[Qemu-devel] [PATCH v2 24/36] target-microblaze: mmu: Add R_TBLX_MISS macros

[Richard Henderson]

Add a R_TBLX_MISS MASK and SHIFT macros.
---
target/microblaze/mmu.c | 5 +++--
target/microblaze/mmu.h | 4 ++++
2 files changed, 7 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson <***@linaro.org>


r~

[Richard Henderson]

Convert env_btarget to i64.
No functional change.
---
target/microblaze/cpu.h | 2 +-
target/microblaze/op_helper.c | 3 ++-
target/microblaze/translate.c | 35 ++++++++++++++++++++++-------------
3 files changed, 25 insertions(+), 15 deletions(-)

Merge with previous? They seem to be indivisible.

Why do you want to do this for what appears to be a boolean?


r~

[Richard Henderson]

Cleanup eval_cond_jmp to use tcg_gen_movcond_i64().
No functional change.
---
target/microblaze/translate.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 256acce876..a4f6b307d3 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -1167,12 +1167,9 @@ static inline void eval_cc(DisasContext *dc, unsigned int cc,
static void eval_cond_jmp(DisasContext *dc, TCGv_i64 pc_true, TCGv_i64 pc_false)
{
- TCGLabel *l1 = gen_new_label();
- /* Conditional jmp. */
- tcg_gen_mov_i64(cpu_SR[SR_PC], pc_false);
- tcg_gen_brcondi_i64(TCG_COND_EQ, env_btaken, 0, l1);
- tcg_gen_mov_i64(cpu_SR[SR_PC], pc_true);
- gen_set_label(l1);
+ tcg_gen_movcond_i64(TCG_COND_NE, cpu_SR[SR_PC],
+ env_btaken, tcg_const_i64(0),
+ pc_true, pc_false);

Ah. I would probably just extend env_btaken here instead of the previous patch.


r~

[Edgar E. Iglesias]

Post by Richard Henderson

Cleanup eval_cond_jmp to use tcg_gen_movcond_i64().
No functional change.
---
target/microblaze/translate.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index 256acce876..a4f6b307d3 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -1167,12 +1167,9 @@ static inline void eval_cc(DisasContext *dc, unsigned int cc,
static void eval_cond_jmp(DisasContext *dc, TCGv_i64 pc_true, TCGv_i64 pc_false)
{
- TCGLabel *l1 = gen_new_label();
- /* Conditional jmp. */
- tcg_gen_mov_i64(cpu_SR[SR_PC], pc_false);
- tcg_gen_brcondi_i64(TCG_COND_EQ, env_btaken, 0, l1);
- tcg_gen_mov_i64(cpu_SR[SR_PC], pc_true);
- gen_set_label(l1);
+ tcg_gen_movcond_i64(TCG_COND_NE, cpu_SR[SR_PC],
+ env_btaken, tcg_const_i64(0),
+ pc_true, pc_false);

Ah. I would probably just extend env_btaken here instead of the previous patch.

I've changed it according to your suggestion. I'll post v3 in a moment.

Thanks,
Edgar

[Richard Henderson]

Remove argument b in eval_cc() as it is always set to zero.
No functional change.
---
target/microblaze/translate.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Richard Henderson <***@linaro.org>


r~

[Richard Henderson]

Use a table based conversion to map condition-codes between
MicroBlaze ISA encoding and TCG.
No functional change.
---
target/microblaze/translate.c | 41 ++++++++++++++++++++---------------------
1 file changed, 20 insertions(+), 21 deletions(-)

Reviewed-by: Richard Henderson <***@linaro.org>


r~

[Richard Henderson]

* Avoid long 80+ character lines.
* Remove D() macro and use qemu_log_mask.
* Remove logs that are not very useful
---
target/microblaze/mmu.c | 39 +++++++++++++++++++--------------------
1 file changed, 19 insertions(+), 20 deletions(-)

Reviewed-by: Richard Henderson <***@linaro.org>


r~

[Richard Henderson]

Simplify address computation using tcg_gen_addi_i32().
tcg_gen_addi_i32() already optimizes the case when the
immediate is zero.
No functional change.
---
target/microblaze/translate.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)

Reviewed-by: Richard Henderson <***@linaro.org>


r~

[Richard Henderson]

+ if (to) {
+ gen_helper_mmu_write(cpu_env, tcg_const_i32(extended),
+ tcg_const_i32(sr), cpu_R[dc->ra]);
+ } else {
+ gen_helper_mmu_read(cpu_R[dc->rd], cpu_env,
+ tcg_const_i32(extended), tcg_const_i32(sr));
+ }

These tcg_const_i32 results should be tcg_temp_free'd.


r~

[Edgar E. Iglesias]

Post by Richard Henderson

+ if (to) {
+ gen_helper_mmu_write(cpu_env, tcg_const_i32(extended),
+ tcg_const_i32(sr), cpu_R[dc->ra]);
+ } else {
+ gen_helper_mmu_read(cpu_R[dc->rd], cpu_env,
+ tcg_const_i32(extended), tcg_const_i32(sr));
+ }

These tcg_const_i32 results should be tcg_temp_free'd.

Thanks, I've fixed this for v3.

[Richard Henderson]

Add a configurable output address mask, used to mimic the
configurable physical address bit width.
---
target/microblaze/cpu.c | 1 +
target/microblaze/mmu.c | 1 +
target/microblaze/mmu.h | 1 +
3 files changed, 3 insertions(+)

Reviewed-by: Richard Henderson <***@linaro.org>


r~

[Richard Henderson]

Prepare for 64-bit addresses.
This makes no functional difference as the upper parts of
the 64-bit addresses are not yet reachable.
---
target/microblaze/mmu.c | 14 +++++++-------
target/microblaze/mmu.h | 6 +++---
2 files changed, 10 insertions(+), 10 deletions(-)

Reviewed-by: Richard Henderson <***@linaro.org>


r~

[Richard Henderson]

Add explicit handling for MMU_R_TLBX and log accesses to
invalid MMU registers. We can now remove the state for
all regs but PID, ZPR and TLBX (0 - 2).
---
target/microblaze/mmu.c | 7 +++++--
target/microblaze/mmu.h | 2 +-
2 files changed, 6 insertions(+), 3 deletions(-)

Reviewed-by: Richard Henderson <***@linaro.org>


r~

[Richard Henderson]

Add support for Extended Addressing. Load/stores with EA
enabled concatenate two 32bit registers to form an extended
address.
We don't allow users to enable address sizes larger than
32 bits quite yet though. Once the MMU support is in, we'll
turn it on.
---
target/microblaze/cpu.c | 18 +++++++++++++-
target/microblaze/cpu.h | 2 ++
target/microblaze/translate.c | 55 +++++++++++++++++++++++++++++++++----------
3 files changed, 62 insertions(+), 13 deletions(-)

Reviewed-by: Richard Henderson <***@linaro.org>


r~

[Richard Henderson]

Implement MFSE EAR to enable access to the upper part of EAR.
---
target/microblaze/translate.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson <***@linaro.org>


r~

[Alistair Francis]

On Tue, May 8, 2018 at 10:31 AM, Edgar E. Iglesias

Name special registers we support.

Reviewed-by: Alistair Francis <***@wdc.com>

Alistair

---
target/microblaze/translate.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c
index c971fe3b72..6cc92d09c9 100644
--- a/target/microblaze/translate.c
+++ b/target/microblaze/translate.c
@@ -105,8 +105,8 @@ static const char *regnames[] =
static const char *special_regnames[] =
{
- "rpc", "rmsr", "sr2", "sr3", "sr4", "sr5", "sr6", "sr7",
- "sr8", "sr9", "sr10", "sr11", "sr12", "sr13"
+ "rpc", "rmsr", "sr2", "rear", "sr4", "resr", "sr6", "rfsr",
+ "sr8", "sr9", "sr10", "rbtr", "sr12", "redr"
};
static inline void t_sync_flags(DisasContext *dc)
--
2.14.1

[Richard Henderson]

+ if (cpu->cfg.use_mmu && (env->sregs[SR_MSR] & MSR_VM)
+ && mmu_idx != MMU_NOMMU_IDX) {

For future cleanup, the first condition should be moved to cpu_mmu_index (the
second condition is already there). At which point here you need check nothing
but mmu_idx.


r~

[Edgar E. Iglesias]

Post by Richard Henderson

+ if (cpu->cfg.use_mmu && (env->sregs[SR_MSR] & MSR_VM)
+ && mmu_idx != MMU_NOMMU_IDX) {

For future cleanup, the first condition should be moved to cpu_mmu_index (the
second condition is already there). At which point here you need check nothing
but mmu_idx.

Thanks, I've added a followup patch that does this.

Cheers,
Edgar

[Richard Henderson]

We already have a CPU property to control if a core has
an MMU or not. Remove USE_MMU PVR checks in favor of
looking at the property.
---
target/microblaze/helper.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)

Reviewed-by: Richard Henderson <***@linaro.org>


r~