[Qemu-devel] [PATCH 00/11] target/hppa patches

[Qemu-devel] [PATCH 00/11] target/hppa patches

[Sven Schnelle]

Hi Richard,

here are a few fixes for the HPPA architecture. Some fixes for
problems reported by diagnostics software, some TLB fixes to make
HP-UX 10.20 work with TB chaining in QEMU. I'm not sure whether the
'call eval_interrupt() after ssm' fix is really the right way - please
check.

I haven't seen any problems with these HP-UX 10.20 anymore, and Linux also
still works.

Thanks
Sven

Sven Schnelle (11):
  target/hppa: fix overwriting source reg in addb
  target/hppa: fix TLB handling for page 0
  target/hppa: report ITLB_EXCP_MISS for ITLB misses
  target/hppa: add TLB trace events
  target/hppa: remove PSW I/R/Q bit check
  target/hppa: ignore DIAG opcode
  target/hppa: fix b,gate instruction
  target/hppa: allow multiple itlbp without itlba
  target/hppa: add TLB protection id check
  target/hppa: exit TB if either Data or Instruction TLB changes
  target/hppa: call eval_interrupt() after ssm

 Makefile.objs            |  1 +
 target/hppa/cpu.h        |  5 +++
 target/hppa/insns.decode |  3 ++
 target/hppa/int_helper.c |  2 +-
 target/hppa/mem_helper.c | 68 +++++++++++++++++++++++++++++++---------
 target/hppa/op_helper.c  | 13 +++++---
 target/hppa/trace-events | 18 +++++++++++
 target/hppa/translate.c  | 23 +++++++++-----
 8 files changed, 106 insertions(+), 27 deletions(-)
 create mode 100644 target/hppa/trace-events

-- 
2.20.1

[Qemu-devel] [PATCH 01/11] target/hppa: fix overwriting source reg in addb

[Sven Schnelle]

When one of the source registers is the same as the destination register,
the source register gets overwritten with the destionation value before
do_add_sv() is called, which leads to unexpection condition matches.

Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
 target/hppa/translate.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index dc5636fe94..7001c2eb80 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -3033,7 +3033,7 @@ static bool do_addb(DisasContext *ctx, unsigned r, TCGv_reg in1,
     DisasCond cond;
 
     in2 = load_gpr(ctx, r);
-    dest = dest_gpr(ctx, r);
+    dest = tcg_temp_new();
     sv = NULL;
     cb_msb = NULL;
 
@@ -3049,6 +3049,8 @@ static bool do_addb(DisasContext *ctx, unsigned r, TCGv_reg in1,
     }
 
     cond = do_cond(c * 2 + f, dest, cb_msb, sv);
+    save_gpr(ctx, r, dest);
+    tcg_temp_free(dest);
     return do_cbranch(ctx, disp, n, &cond);
 }
 
-- 
2.20.1

[Qemu-devel] [PATCH 02/11] target/hppa: fix TLB handling for page 0

[Sven Schnelle]

Assume the following sequence:

pitlbe r0(sr0,r0)
iitlba r4,(sr0,r0)
ldil L%3000000,r5
iitlbp r5,(sr0,r0)

This will purge the whole TLB and add an entry for page 0. However
the current TLB implementation in helper_iitlba() will store to
the last empty TLB entry, while helper_iitlbp() will write to the
first empty entry. That is because an empty entry will match address
0 in helper_iitlba()

Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
 target/hppa/mem_helper.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
index aecf3075f6..f30824f4e1 100644
--- a/target/hppa/mem_helper.c
+++ b/target/hppa/mem_helper.c
@@ -238,15 +238,17 @@ void HELPER(itlba)(CPUHPPAState *env, target_ulong addr, target_ureg reg)
 {
     hppa_tlb_entry *empty = NULL;
     int i;
-
     /* Zap any old entries covering ADDR; notice empty entries on the way.  */
     for (i = 0; i < ARRAY_SIZE(env->tlb); ++i) {
         hppa_tlb_entry *ent = &env->tlb[i];
-        if (!ent->entry_valid) {
-            empty = ent;
-        } else if (ent->va_b <= addr && addr <= ent->va_e) {
-            hppa_flush_tlb_ent(env, ent);
-            empty = ent;
+        if (ent->va_b <= addr && addr <= ent->va_e) {
+            if (ent->entry_valid) {
+                hppa_flush_tlb_ent(env, ent);
+            }
+
+            if (!empty) {
+                empty = ent;
+            }
         }
     }
 
-- 
2.20.1

[Qemu-devel] [PATCH 03/11] target/hppa: report ITLB_EXCP_MISS for ITLB misses

[Sven Schnelle]

Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
 target/hppa/mem_helper.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
index f30824f4e1..07ecfaf092 100644
--- a/target/hppa/mem_helper.c
+++ b/target/hppa/mem_helper.c
@@ -96,9 +96,7 @@ int hppa_get_physical_address(CPUHPPAState *env, vaddr addr, int mmu_idx,
     if (ent == NULL || !ent->entry_valid) {
         phys = 0;
         prot = 0;
-        /* ??? Unconditionally report data tlb miss,
-           even if this is an instruction fetch.  */
-        ret = EXCP_DTLB_MISS;
+        ret = (type == PAGE_EXEC) ? EXCP_ITLB_MISS : EXCP_DTLB_MISS;
         goto egress;
     }
 
-- 
2.20.1

[Qemu-devel] [PATCH 04/11] target/hppa: add TLB trace events

[Sven Schnelle]

To ease TLB debugging add a few trace events, which are disabled
by default so that there's no performance impact.

Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
 Makefile.objs            |  1 +
 target/hppa/mem_helper.c | 20 ++++++++++++++++++--
 target/hppa/op_helper.c  |  2 ++
 target/hppa/trace-events | 18 ++++++++++++++++++
 4 files changed, 39 insertions(+), 2 deletions(-)
 create mode 100644 target/hppa/trace-events

diff --git a/Makefile.objs b/Makefile.objs
index ef65a6c12e..4df63e1633 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -183,6 +183,7 @@ trace-events-subdirs += qapi
 trace-events-subdirs += qom
 trace-events-subdirs += scsi
 trace-events-subdirs += target/arm
+trace-events-subdirs += target/hppa
 trace-events-subdirs += target/i386
 trace-events-subdirs += target/mips
 trace-events-subdirs += target/ppc
diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
index 07ecfaf092..26da953185 100644
--- a/target/hppa/mem_helper.c
+++ b/target/hppa/mem_helper.c
@@ -22,6 +22,7 @@
 #include "exec/exec-all.h"
 #include "exec/helper-proto.h"
 #include "qom/cpu.h"
+#include "trace.h"
 
 #ifdef CONFIG_USER_ONLY
 int hppa_cpu_handle_mmu_fault(CPUState *cs, vaddr address,
@@ -43,9 +44,11 @@ static hppa_tlb_entry *hppa_find_tlb(CPUHPPAState *env, vaddr addr)
     for (i = 0; i < ARRAY_SIZE(env->tlb); ++i) {
         hppa_tlb_entry *ent = &env->tlb[i];
         if (ent->va_b <= addr && addr <= ent->va_e) {
+            trace_hppa_tlb_find_entry(env, ent + i, ent->entry_valid, ent->va_b, ent->va_e, ent->pa);
             return ent;
         }
     }
+    trace_hppa_tlb_find_entry_not_found(env, addr);
     return NULL;
 }
 
@@ -55,6 +58,8 @@ static void hppa_flush_tlb_ent(CPUHPPAState *env, hppa_tlb_entry *ent)
     unsigned i, n = 1 << (2 * ent->page_size);
     uint64_t addr = ent->va_b;
 
+    trace_hppa_tlb_flush_ent(env, ent, ent->va_b, ent->va_e, ent->pa);
+
     for (i = 0; i < n; ++i, addr += TARGET_PAGE_SIZE) {
         /* Do not flush MMU_PHYS_IDX.  */
         tlb_flush_page_by_mmuidx(cs, addr, 0xf);
@@ -169,6 +174,7 @@ int hppa_get_physical_address(CPUHPPAState *env, vaddr addr, int mmu_idx,
  egress:
     *pphys = phys;
     *pprot = prot;
+    trace_hppa_tlb_get_physical_address(env, ret, prot, addr, phys);
     return ret;
 }
 
@@ -198,6 +204,7 @@ void tlb_fill(CPUState *cs, target_ulong addr, int size,
               MMUAccessType type, int mmu_idx, uintptr_t retaddr)
 {
     HPPACPU *cpu = HPPA_CPU(cs);
+    CPUHPPAState *env = &cpu->env;
     int prot, excp, a_prot;
     hwaddr phys;
 
@@ -213,9 +220,10 @@ void tlb_fill(CPUState *cs, target_ulong addr, int size,
         break;
     }
 
-    excp = hppa_get_physical_address(&cpu->env, addr, mmu_idx,
+    excp = hppa_get_physical_address(env, addr, mmu_idx,
                                      a_prot, &phys, &prot);
     if (unlikely(excp >= 0)) {
+        trace_hppa_tlb_fill_excp(env, addr, size, type, mmu_idx);
         /* Failure.  Raise the indicated exception.  */
         cs->exception_index = excp;
         if (cpu->env.psw & PSW_Q) {
@@ -226,6 +234,8 @@ void tlb_fill(CPUState *cs, target_ulong addr, int size,
         cpu_loop_exit_restore(cs, retaddr);
     }
 
+    trace_hppa_tlb_fill_success(env, addr & TARGET_PAGE_MASK,
+            phys & TARGET_PAGE_MASK, size, type, mmu_idx);
     /* Success!  Store the translation into the QEMU TLB.  */
     tlb_set_page(cs, addr & TARGET_PAGE_MASK, phys & TARGET_PAGE_MASK,
                  prot, mmu_idx, TARGET_PAGE_SIZE);
@@ -259,6 +269,7 @@ void HELPER(itlba)(CPUHPPAState *env, target_ulong addr, target_ureg reg)
     empty->va_b = addr & TARGET_PAGE_MASK;
     empty->va_e = empty->va_b + TARGET_PAGE_SIZE - 1;
     empty->pa = extract32(reg, 5, 20) << TARGET_PAGE_BITS;
+    trace_hppa_tlb_itlba(env, empty, empty->va_b, empty->va_e, empty->pa);
 }
 
 /* Insert (Insn/Data) TLB Protection.  Note this is PA 1.1 only.  */
@@ -280,6 +291,8 @@ void HELPER(itlbp)(CPUHPPAState *env, target_ulong addr, target_ureg reg)
     ent->d = extract32(reg, 28, 1);
     ent->t = extract32(reg, 29, 1);
     ent->entry_valid = 1;
+    trace_hppa_tlb_itlbp(env, ent, ent->access_id, ent->u, ent->ar_pl2,
+                         ent->ar_pl1, ent->ar_type, ent->b, ent->d, ent->t);
 }
 
 /* Purge (Insn/Data) TLB.  This is explicitly page-based, and is
@@ -299,6 +312,7 @@ void HELPER(ptlb)(CPUHPPAState *env, target_ulong addr)
 {
     CPUState *src = CPU(hppa_env_get_cpu(env));
     CPUState *cpu;
+    trace_hppa_tlb_ptlb(env);
     run_on_cpu_data data = RUN_ON_CPU_TARGET_PTR(addr);
 
     CPU_FOREACH(cpu) {
@@ -314,7 +328,7 @@ void HELPER(ptlb)(CPUHPPAState *env, target_ulong addr)
 void HELPER(ptlbe)(CPUHPPAState *env)
 {
     CPUState *src = CPU(hppa_env_get_cpu(env));
-
+    trace_hppa_tlb_ptlbe(env);
     memset(env->tlb, 0, sizeof(env->tlb));
     tlb_flush_by_mmuidx(src, 0xf);
 }
@@ -335,8 +349,10 @@ target_ureg HELPER(lpa)(CPUHPPAState *env, target_ulong addr)
         if (excp == EXCP_DTLB_MISS) {
             excp = EXCP_NA_DTLB_MISS;
         }
+        trace_hppa_tlb_lpa_failed(env, addr);
         hppa_dynamic_excp(env, excp, GETPC());
     }
+    trace_hppa_tlb_lpa_success(env, addr, phys);
     return phys;
 }
 
diff --git a/target/hppa/op_helper.c b/target/hppa/op_helper.c
index 268caaaa20..a05681d480 100644
--- a/target/hppa/op_helper.c
+++ b/target/hppa/op_helper.c
@@ -25,6 +25,7 @@
 #include "sysemu/sysemu.h"
 #include "qemu/timer.h"
 #include "fpu/softfloat.h"
+#include "trace.h"
 
 void QEMU_NORETURN HELPER(excp)(CPUHPPAState *env, int excp)
 {
@@ -165,6 +166,7 @@ target_ureg HELPER(probe)(CPUHPPAState *env, target_ulong addr,
     int prot, excp;
     hwaddr phys;
 
+    trace_hppa_tlb_probe(addr, level, want);
     /* Fail if the requested privilege level is higher than current.  */
     if (level < (env->iaoq_f & 3)) {
         return 0;
diff --git a/target/hppa/trace-events b/target/hppa/trace-events
new file mode 100644
index 0000000000..80dae5bd8b
--- /dev/null
+++ b/target/hppa/trace-events
@@ -0,0 +1,18 @@
+# See docs/devel/tracing.txt for syntax documentation.
+
+# target/hppa/mem_helper.c
+disable hppa_tlb_flush_ent(void *env, void *ent, uint64_t va_b, uint64_t va_e, uint64_t pa) "env=%p ent=%p va_b=0x%lx va_e=0x%lx pa=0x%lx"
+disable hppa_tlb_find_entry(void *env, void *ent, int valid, uint64_t va_b, uint64_t va_e, uint64_t pa) "env=%p ent=%p valid=%d va_b=0x%lx va_e=0x%lx pa=0x%lx"
+disable hppa_tlb_find_entry_not_found(void *env, uint64_t addr) "env=%p addr=%08lx"
+disable hppa_tlb_get_physical_address(void *env, int ret, int prot, uint64_t addr, uint64_t phys) "env=%p ret=%d prot=%d addr=0x%lx phys=0x%lx"
+disable hppa_tlb_fill_excp(void *env, uint64_t addr, int size, int type, int mmu_idx) "env=%p addr=0x%lx size=%d type=%d mmu_idx=%d"
+disable hppa_tlb_fill_success(void *env, uint64_t addr, uint64_t phys, int size, int type, int mmu_idx) "env=%p addr=0x%lx phys=0x%lx size=%d type=%d mmu_idx=%d"
+disable hppa_tlb_itlba(void *env, void *ent, uint64_t va_b, uint64_t va_e, uint64_t pa) "env=%p ent=%p va_b=0x%lx va_e=0x%lx pa=0x%lx"
+disable hppa_tlb_itlbp(void *env, void *ent, int access_id, int u, int pl2, int pl1, int type, int b, int d, int t) "env=%p ent=%p access_id=%x u=%d pl2=%d pl1=%d type=%d b=%d d=%d t=%d"
+disable hppa_tlb_ptlb(void *env) "env=%p"
+disable hppa_tlb_ptlbe(void *env) "env=%p"
+disable hppa_tlb_lpa_success(void *env, uint64_t addr, uint64_t phys) "env=%p addr=0x%lx phys=0x%lx"
+disable hppa_tlb_lpa_failed(void *env, uint64_t addr) "env=%p addr=0x%lx"
+
+# target/hppa/op_helper.c
+disable hppa_tlb_probe(uint64_t addr, int level, int want) "addr=0x%lx level=%d want=%d"
-- 
2.20.1

[Qemu-devel] [PATCH 05/11] target/hppa: remove PSW I/R/Q bit check

[Sven Schnelle]

HP ODE use rfi to set the Q bit, and i don't see anything in the
documentation that this is forbidden. So remove it.

Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
 target/hppa/op_helper.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/target/hppa/op_helper.c b/target/hppa/op_helper.c
index a05681d480..a55a5dfc02 100644
--- a/target/hppa/op_helper.c
+++ b/target/hppa/op_helper.c
@@ -678,11 +678,6 @@ target_ureg HELPER(swap_system_mask)(CPUHPPAState *env, target_ureg nsm)
 
 void HELPER(rfi)(CPUHPPAState *env)
 {
-    /* ??? On second reading this condition simply seems
-       to be undefined rather than a diagnosed trap.  */
-    if (env->psw & (PSW_I | PSW_R | PSW_Q)) {
-        helper_excp(env, EXCP_ILL);
-    }
     env->iasq_f = (uint64_t)env->cr[CR_IIASQ] << 32;
     env->iasq_b = (uint64_t)env->cr_back[0] << 32;
     env->iaoq_f = env->cr[CR_IIAOQ];
-- 
2.20.1

[Qemu-devel] [PATCH 06/11] target/hppa: ignore DIAG opcode

[Sven Schnelle]

DIAG is usually only used by diagnostics software as it's CPU
specific. In most of the cases it's better to ignore it and log
a message that it's not implemented.

Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
 target/hppa/insns.decode | 3 +++
 target/hppa/translate.c  | 6 ++++++
 2 files changed, 9 insertions(+)

diff --git a/target/hppa/insns.decode b/target/hppa/insns.decode
index 55ff39dd05..098370c2f0 100644
--- a/target/hppa/insns.decode
+++ b/target/hppa/insns.decode
@@ -525,3 +525,6 @@ fmpy_d          001110 ..... ..... 010 ..... ... .....  @f0e_d_3
 fdiv_d          001110 ..... ..... 011 ..... ... .....  @f0e_d_3
 
 xmpyu           001110 ..... ..... 010 .0111 .00 t:5    r1=%ra64 r2=%rb64
+
+# diag
+diag            000101 ----- ----- ---- ---- ---- ----
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 7001c2eb80..441f0ea9d6 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -4292,3 +4292,9 @@ void restore_state_to_opc(CPUHPPAState *env, TranslationBlock *tb,
        that the instruction was not nullified.  */
     env->psw_n = 0;
 }
+
+static bool trans_diag(DisasContext *ctx, arg_diag *a)
+{
+    qemu_log_mask(LOG_UNIMP, "DIAG opcode ignored\n");
+    return true;
+}
-- 
2.20.1

[Qemu-devel] [PATCH 07/11] target/hppa: fix b,gate instruction

[Sven Schnelle]

b,gate does GR[t] ← cat(GR[t]{0..29},IAOQ_Front{30..31});
instead of saving the link address to register t.

Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
 target/hppa/translate.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 441f0ea9d6..a393a12252 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -3464,6 +3464,7 @@ static bool trans_b_gate(DisasContext *ctx, arg_b_gate *a)
     }
 
 #ifndef CONFIG_USER_ONLY
+    TCGv_reg tmp;
     if (ctx->tb_flags & PSW_C) {
         CPUHPPAState *env = ctx->cs->env_ptr;
         int type = hppa_artype_for_page(env, ctx->base.pc_next);
@@ -3480,12 +3481,13 @@ static bool trans_b_gate(DisasContext *ctx, arg_b_gate *a)
         if (type >= 4 && type - 4 < ctx->privilege) {
             dest = deposit32(dest, 0, 2, type - 4);
         }
+        tmp = dest_gpr(ctx, a->l);
+        tcg_gen_deposit_reg(tmp, tmp, cpu_iaoq_f, 0, 2);
     } else {
         dest &= -4;  /* priv = 0 */
     }
 #endif
-
-    return do_dbranch(ctx, dest, a->l, a->n);
+    return do_dbranch(ctx, dest, 0, a->n);
 }
 
 static bool trans_blr(DisasContext *ctx, arg_blr *a)
-- 
2.20.1

[Qemu-devel] [PATCH 08/11] target/hppa: allow multiple itlbp without itlba

[Sven Schnelle]

The ODE software calls itlbp on existing TLB entries without
calling itlba first, so this seems to be valid.

Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
 target/hppa/mem_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
index 26da953185..fc1b6a4fcd 100644
--- a/target/hppa/mem_helper.c
+++ b/target/hppa/mem_helper.c
@@ -277,7 +277,7 @@ void HELPER(itlbp)(CPUHPPAState *env, target_ulong addr, target_ureg reg)
 {
     hppa_tlb_entry *ent = hppa_find_tlb(env, addr);
 
-    if (unlikely(ent == NULL || ent->entry_valid)) {
+    if (unlikely(ent == NULL)) {
         qemu_log_mask(LOG_GUEST_ERROR, "ITLBP not following ITLBA\n");
         return;
     }
-- 
2.20.1

[Qemu-devel] [PATCH 09/11] target/hppa: add TLB protection id check

[Sven Schnelle]

Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
 target/hppa/cpu.h        |  4 ++++
 target/hppa/mem_helper.c | 28 ++++++++++++++++++++++++++--
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index 861bbb1f16..d808796ee3 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -143,6 +143,10 @@
 #endif
 
 #define CR_RC            0
+#define CR_PID1          8
+#define CR_PID2          9
+#define CR_PID3          12
+#define CR_PID4          13
 #define CR_SCRCCR        10
 #define CR_SAR           11
 #define CR_IVA           14
diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
index fc1b6a4fcd..a52d691d15 100644
--- a/target/hppa/mem_helper.c
+++ b/target/hppa/mem_helper.c
@@ -85,7 +85,7 @@ int hppa_get_physical_address(CPUHPPAState *env, vaddr addr, int mmu_idx,
                               int type, hwaddr *pphys, int *pprot)
 {
     hwaddr phys;
-    int prot, r_prot, w_prot, x_prot;
+    int prot, r_prot, w_prot, x_prot, wd;
     hppa_tlb_entry *ent;
     int ret = -1;
 
@@ -130,7 +130,31 @@ int hppa_get_physical_address(CPUHPPAState *env, vaddr addr, int mmu_idx,
         break;
     }
 
-    /* ??? Check PSW_P and ent->access_prot.  This can remove PAGE_WRITE.  */
+    /* access_id == 0 means public page and no check is performed */
+    if ((env->psw & PSW_P) && ent->access_id) {
+            wd = 1;
+
+            if (ent->access_id == (env->cr[CR_PID1] >> 1)) {
+                wd &= env->cr[CR_PID1];
+            }
+
+            if (ent->access_id == (env->cr[CR_PID2] >> 1)) {
+                wd &= env->cr[CR_PID2];
+            }
+
+            if (ent->access_id == (env->cr[CR_PID3] >> 1)) {
+                wd &= env->cr[CR_PID3];
+            }
+
+            if (ent->access_id == (env->cr[CR_PID4] >> 1)) {
+                wd &= env->cr[CR_PID4];
+            }
+
+            if (wd && (type & w_prot)) {
+                ret = EXCP_DMPI;
+                goto egress;
+            }
+    }
 
     /* No guest access type indicates a non-architectural access from
        within QEMU.  Bypass checks for access, D, B and T bits.  */
-- 
2.20.1

[Qemu-devel] [PATCH 10/11] target/hppa: exit TB if either Data or Instruction TLB changes

[Sven Schnelle]

The current code assumes that we don't need to exit the TB
if a Data Cache Flush or Insert has happend. However, as we
have a shared Data/Instruction TLB, a Data cache flush also
flushes Instruction TLB entries, and a Data cache TLB insert
might also evict a Instruction TLB entry.

So exit the TB in all cases if Instruction translation is enabled.

Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
 target/hppa/translate.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index a393a12252..fcacff963e 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -2474,9 +2474,8 @@ static bool trans_ixtlbx(DisasContext *ctx, arg_ixtlbx *a)
         gen_helper_itlbp(cpu_env, addr, reg);
     }
 
-    /* Exit TB for ITLB change if mmu is enabled.  This *should* not be
-       the case, since the OS TLB fill handler runs with mmu disabled.  */
-    if (!a->data && (ctx->tb_flags & PSW_C)) {
+    /* Exit TB for TLB change if mmu is enabled.  */
+    if (ctx->tb_flags & PSW_C) {
         ctx->base.is_jmp = DISAS_IAQ_N_STALE;
     }
     return nullify_end(ctx);
@@ -2503,7 +2502,7 @@ static bool trans_pxtlbx(DisasContext *ctx, arg_pxtlbx *a)
     }
 
     /* Exit TB for TLB change if mmu is enabled.  */
-    if (!a->data && (ctx->tb_flags & PSW_C)) {
+    if (ctx->tb_flags & PSW_C) {
         ctx->base.is_jmp = DISAS_IAQ_N_STALE;
     }
     return nullify_end(ctx);
-- 
2.20.1

[Qemu-devel] [PATCH 11/11] target/hppa: call eval_interrupt() after ssm

[Sven Schnelle]

HP-UX (all versions) is losing timer interrupts, which leads to
hangs. Pressing a key on the console fixes this, so it looks like
QEMU is just looping trough TBs without checking for interrupts.
Further investion showed that this happens when interrupts are
triggered, without PSW_I enabled. Calling eval_interrupt() after
PSW_I is set seems to fix this.

Signed-off-by: Sven Schnelle <svens@stackframe.org>
---
 target/hppa/cpu.h        | 1 +
 target/hppa/int_helper.c | 2 +-
 target/hppa/op_helper.c  | 6 ++++++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index d808796ee3..3440ccad28 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -366,5 +366,6 @@ void hppa_cpu_alarm_timer(void *);
 int hppa_artype_for_page(CPUHPPAState *env, target_ulong vaddr);
 #endif
 void QEMU_NORETURN hppa_dynamic_excp(CPUHPPAState *env, int excp, uintptr_t ra);
+void eval_interrupt(HPPACPU *cpu);
 
 #endif /* HPPA_CPU_H */
diff --git a/target/hppa/int_helper.c b/target/hppa/int_helper.c
index 8d5edd3a20..e3acaa39eb 100644
--- a/target/hppa/int_helper.c
+++ b/target/hppa/int_helper.c
@@ -25,7 +25,7 @@
 #include "qom/cpu.h"
 
 #ifndef CONFIG_USER_ONLY
-static void eval_interrupt(HPPACPU *cpu)
+void eval_interrupt(HPPACPU *cpu)
 {
     CPUState *cs = CPU(cpu);
     if (cpu->env.cr[CR_EIRR] & cpu->env.cr[CR_EIEM]) {
diff --git a/target/hppa/op_helper.c b/target/hppa/op_helper.c
index a55a5dfc02..f93211c84f 100644
--- a/target/hppa/op_helper.c
+++ b/target/hppa/op_helper.c
@@ -662,6 +662,7 @@ void HELPER(reset)(CPUHPPAState *env)
 
 target_ureg HELPER(swap_system_mask)(CPUHPPAState *env, target_ureg nsm)
 {
+    HPPACPU *cpu = hppa_env_get_cpu(env);
     target_ulong psw = env->psw;
     /*
      * Setting the PSW Q bit to 1, if it was not already 1, is an
@@ -673,6 +674,11 @@ target_ureg HELPER(swap_system_mask)(CPUHPPAState *env, target_ureg nsm)
      * so let this go without comment.
      */
     env->psw = (psw & ~PSW_SM) | (nsm & PSW_SM);
+    if (!(psw & PSW_I) && (nsm & PSW_I)) {
+        qemu_mutex_lock_iothread();
+        eval_interrupt(cpu);
+        qemu_mutex_unlock_iothread();
+    }
     return psw & PSW_SM;
 }
 
-- 
2.20.1

Re: [Qemu-devel] [PATCH 00/11] target/hppa patches

[no-reply]

Patchew URL: https://patchew.org/QEMU/20190311191602.25796-1-svens@stackframe.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20190311191602.25796-1-svens@stackframe.org
Subject: [Qemu-devel] [PATCH 00/11] target/hppa patches

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]               patchew/20190311191602.25796-1-svens@stackframe.org -> patchew/20190311191602.25796-1-svens@stackframe.org
Switched to a new branch 'test'
40ceb6e6ae target/hppa: call eval_interrupt() after ssm
c5988388b3 target/hppa: exit TB if either Data or Instruction TLB changes
2fe19345de target/hppa: add TLB protection id check
c6466d97af target/hppa: allow multiple itlbp without itlba
278b4cc1e3 target/hppa: fix b,gate instruction
362d5b268b target/hppa: ignore DIAG opcode
00d4779b37 target/hppa: remove PSW I/R/Q bit check
d51423679b target/hppa: add TLB trace events
92887610fa target/hppa: report ITLB_EXCP_MISS for ITLB misses
95cf25833a target/hppa: fix TLB handling for page 0
7772997bae target/hppa: fix overwriting source reg in addb

=== OUTPUT BEGIN ===
1/11 Checking commit 7772997bae4b (target/hppa: fix overwriting source reg in addb)
2/11 Checking commit 95cf25833af4 (target/hppa: fix TLB handling for page 0)
3/11 Checking commit 92887610fa42 (target/hppa: report ITLB_EXCP_MISS for ITLB misses)
4/11 Checking commit d51423679b65 (target/hppa: add TLB trace events)
ERROR: line over 90 characters
#41: FILE: target/hppa/mem_helper.c:47:
+            trace_hppa_tlb_find_entry(env, ent + i, ent->entry_valid, ent->va_b, ent->va_e, ent->pa);

WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#161: 
new file mode 100644

ERROR: Hex numbers must be prefixed with '0x'
#171: FILE: target/hppa/trace-events:6:
+disable hppa_tlb_find_entry_not_found(void *env, uint64_t addr) "env=%p addr=%08lx"

ERROR: Hex numbers must be prefixed with '0x'
#176: FILE: target/hppa/trace-events:11:
+disable hppa_tlb_itlbp(void *env, void *ent, int access_id, int u, int pl2, int pl1, int type, int b, int d, int t) "env=%p ent=%p access_id=%x u=%d pl2=%d pl1=%d type=%d b=%d d=%d t=%d"

total: 3 errors, 1 warnings, 138 lines checked

Patch 4/11 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

5/11 Checking commit 00d4779b3785 (target/hppa: remove PSW I/R/Q bit check)
6/11 Checking commit 362d5b268bd8 (target/hppa: ignore DIAG opcode)
7/11 Checking commit 278b4cc1e332 (target/hppa: fix b,gate instruction)
8/11 Checking commit c6466d97af03 (target/hppa: allow multiple itlbp without itlba)
9/11 Checking commit 2fe19345de05 (target/hppa: add TLB protection id check)
10/11 Checking commit c5988388b3de (target/hppa: exit TB if either Data or Instruction TLB changes)
11/11 Checking commit 40ceb6e6aed2 (target/hppa: call eval_interrupt() after ssm)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190311191602.25796-1-svens@stackframe.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH 06/11] target/hppa: ignore DIAG opcode

[Richard Henderson]

On 3/11/19 12:15 PM, Sven Schnelle wrote:
> +static bool trans_diag(DisasContext *ctx, arg_diag *a)
> +{
> +    qemu_log_mask(LOG_UNIMP, "DIAG opcode ignored\n");
> +    return true;

This needs to free the nullify condition, as with trans_nop.
I've fixed this up while applying.


r~

Re: [Qemu-devel] [PATCH 07/11] target/hppa: fix b,gate instruction

[Richard Henderson]

On 3/11/19 12:15 PM, Sven Schnelle wrote:
> b,gate does GR[t] ← cat(GR[t]{0..29},IAOQ_Front{30..31});
> instead of saving the link address to register t.
> 

Quite right.  Silly mistake.

>  #ifndef CONFIG_USER_ONLY
> +    TCGv_reg tmp;
>      if (ctx->tb_flags & PSW_C) {
>          CPUHPPAState *env = ctx->cs->env_ptr;
>          int type = hppa_artype_for_page(env, ctx->base.pc_next);
> @@ -3480,12 +3481,13 @@ static bool trans_b_gate(DisasContext *ctx, arg_b_gate *a)
>          if (type >= 4 && type - 4 < ctx->privilege) {
>              dest = deposit32(dest, 0, 2, type - 4);
>          }
> +        tmp = dest_gpr(ctx, a->l);
> +        tcg_gen_deposit_reg(tmp, tmp, cpu_iaoq_f, 0, 2);
>      } else {
>          dest &= -4;  /* priv = 0 */
>      }
>  #endif
> -
> -    return do_dbranch(ctx, dest, a->l, a->n);
> +    return do_dbranch(ctx, dest, 0, a->n);

This change needs to be outside the CONFIG_USER_ONLY.  It needs to handle
nullification (which was previously all handled in do_dbranch).  I'm thinking
of something like the following.


r~


diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index f3e78b8e22..6ac196804b 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -3446,6 +3446,8 @@ static bool trans_b_gate
 {
     target_ureg dest = iaoq_dest(ctx, a->disp);

+    nullify_over(ctx);
+
     /* Make sure the caller hasn't done something weird with the queue.
      * ??? This is not quite the same as the PSW[B] bit, which would be
      * expensive to track.  Real hardware will trap for
@@ -3483,7 +3485,16 @@ static bool trans_b_gate
     }
 #endif

-    return do_dbranch(ctx, dest, a->l, a->n);
+    if (a->l) {
+        TCGv_reg tmp = dest_gpr(ctx, a->l);
+        if (ctx->privilege < 3) {
+            tcg_gen_andi_reg(tmp, tmp, -4);
+        }
+        tcg_gen_ori_reg(tmp, tmp, ctx->privilege);
+        save_gpr(ctx, a->l, tmp);
+    }
+
+    return do_dbranch(ctx, dest, 0, a->n);
 }

 static bool trans_blr(DisasContext *ctx, arg_blr *a)

Re: [Qemu-devel] [PATCH 08/11] target/hppa: allow multiple itlbp without itlba

[Richard Henderson]

On 3/11/19 12:15 PM, Sven Schnelle wrote:
> The ODE software calls itlbp on existing TLB entries without
> calling itlba first, so this seems to be valid.
> 
> Signed-off-by: Sven Schnelle <svens@stackframe.org>
> ---
>  target/hppa/mem_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
> index 26da953185..fc1b6a4fcd 100644
> --- a/target/hppa/mem_helper.c
> +++ b/target/hppa/mem_helper.c
> @@ -277,7 +277,7 @@ void HELPER(itlbp)(CPUHPPAState *env, target_ulong addr, target_ureg reg)
>  {
>      hppa_tlb_entry *ent = hppa_find_tlb(env, addr);
>  
> -    if (unlikely(ent == NULL || ent->entry_valid)) {
> +    if (unlikely(ent == NULL)) {
>          qemu_log_mask(LOG_GUEST_ERROR, "ITLBP not following ITLBA\n");
>          return;
>      }
> 

Hmm.  Do you have a broader context for this?  Like maybe the software has just
flushed the entire TLB?  If the entry is valid, and we're not relaxing
permissions, then we might need to flush the softtlb page as well.


r~

Re: [Qemu-devel] [PATCH 09/11] target/hppa: add TLB protection id check

[Richard Henderson]

[-- Attachment #1: Type: text/plain, Size: 1133 bytes --]

On 3/11/19 12:16 PM, Sven Schnelle wrote:
> +    /* access_id == 0 means public page and no check is performed */
> +    if ((env->psw & PSW_P) && ent->access_id) {
> +            wd = 1;
> +
> +            if (ent->access_id == (env->cr[CR_PID1] >> 1)) {
> +                wd &= env->cr[CR_PID1];
> +            }
> +
> +            if (ent->access_id == (env->cr[CR_PID2] >> 1)) {
> +                wd &= env->cr[CR_PID2];
> +            }
> +
> +            if (ent->access_id == (env->cr[CR_PID3] >> 1)) {
> +                wd &= env->cr[CR_PID3];
> +            }
> +
> +            if (ent->access_id == (env->cr[CR_PID4] >> 1)) {
> +                wd &= env->cr[CR_PID4];
> +            }
> +
> +            if (wd && (type & w_prot)) {
> +                ret = EXCP_DMPI;
> +                goto egress;
> +            }
> +    }

This is insufficient.

(1) The softmmu tlb much be flushed when PSW_P,
    or any of the PID registers change.
(2) If type != PAGE_WRITE, you need to remove PAGE_WRITE
    from prot so that the next write doesn't see wrong permissions.

I'll be testing something like the following.


r~



[-- Attachment #2: z --]
[-- Type: text/plain, Size: 6078 bytes --]

diff --git a/target/hppa/cpu.h b/target/hppa/cpu.h
index 861bbb1f16..c062c7969c 100644
--- a/target/hppa/cpu.h
+++ b/target/hppa/cpu.h
@@ -143,6 +143,10 @@
 #endif
 
 #define CR_RC            0
+#define CR_PID1          8
+#define CR_PID2          9
+#define CR_PID3          12
+#define CR_PID4          13
 #define CR_SCRCCR        10
 #define CR_SAR           11
 #define CR_IVA           14
@@ -341,6 +345,12 @@ target_ureg cpu_hppa_get_psw(CPUHPPAState *env);
 void cpu_hppa_put_psw(CPUHPPAState *env, target_ureg);
 void cpu_hppa_loaded_fr0(CPUHPPAState *env);
 
+#ifdef CONFIG_USER_ONLY
+static inline void cpu_hppa_change_prot_id(CPUHPPAState *env) { }
+#else
+void cpu_hppa_change_prot_id(CPUHPPAState *env);
+#endif
+
 #define cpu_signal_handler cpu_hppa_signal_handler
 
 int cpu_hppa_signal_handler(int host_signum, void *pinfo, void *puc);
diff --git a/target/hppa/helper.h b/target/hppa/helper.h
index bfe0dd1db1..38d834ef6b 100644
--- a/target/hppa/helper.h
+++ b/target/hppa/helper.h
@@ -92,4 +92,5 @@ DEF_HELPER_FLAGS_3(itlbp, TCG_CALL_NO_RWG, void, env, tl, tr)
 DEF_HELPER_FLAGS_2(ptlb, TCG_CALL_NO_RWG, void, env, tl)
 DEF_HELPER_FLAGS_1(ptlbe, TCG_CALL_NO_RWG, void, env)
 DEF_HELPER_FLAGS_2(lpa, TCG_CALL_NO_WG, tr, env, tl)
+DEF_HELPER_FLAGS_1(change_prot_id, TCG_CALL_NO_RWG, void, env)
 #endif
diff --git a/target/hppa/gdbstub.c b/target/hppa/gdbstub.c
index 3157a690f2..983bf92aaf 100644
--- a/target/hppa/gdbstub.c
+++ b/target/hppa/gdbstub.c
@@ -93,19 +93,19 @@ int hppa_cpu_gdb_read_register(CPUState *cs, uint8_t *mem_buf, int n)
         val = env->cr[CR_RC];
         break;
     case 52:
-        val = env->cr[8];
+        val = env->cr[CR_PID1];
         break;
     case 53:
-        val = env->cr[9];
+        val = env->cr[CR_PID2];
         break;
     case 54:
         val = env->cr[CR_SCRCCR];
         break;
     case 55:
-        val = env->cr[12];
+        val = env->cr[CR_PID3];
         break;
     case 56:
-        val = env->cr[13];
+        val = env->cr[CR_PID4];
         break;
     case 57:
         val = env->cr[24];
@@ -224,19 +224,23 @@ int hppa_cpu_gdb_write_register(CPUState *cs, uint8_t *mem_buf, int n)
         env->cr[CR_RC] = val;
         break;
     case 52:
-        env->cr[8] = val;
+        env->cr[CR_PID1] = val;
+        cpu_hppa_change_prot_id(env);
         break;
     case 53:
-        env->cr[9] = val;
+        env->cr[CR_PID2] = val;
+        cpu_hppa_change_prot_id(env);
         break;
     case 54:
         env->cr[CR_SCRCCR] = val;
         break;
     case 55:
-        env->cr[12] = val;
+        env->cr[CR_PID3] = val;
+        cpu_hppa_change_prot_id(env);
         break;
     case 56:
-        env->cr[13] = val;
+        env->cr[CR_PID4] = val;
+        cpu_hppa_change_prot_id(env);
         break;
     case 57:
         env->cr[24] = val;
diff --git a/target/hppa/helper.c b/target/hppa/helper.c
index 6539061e52..ac750b62ef 100644
--- a/target/hppa/helper.c
+++ b/target/hppa/helper.c
@@ -21,6 +21,7 @@
 
 #include "cpu.h"
 #include "fpu/softfloat.h"
+#include "exec/exec-all.h"
 #include "exec/helper-proto.h"
 
 target_ureg cpu_hppa_get_psw(CPUHPPAState *env)
@@ -49,6 +50,7 @@ target_ureg cpu_hppa_get_psw(CPUHPPAState *env)
 
 void cpu_hppa_put_psw(CPUHPPAState *env, target_ureg psw)
 {
+    target_ureg old_psw = env->psw;
     target_ureg cb = 0;
 
     env->psw = psw & ~(PSW_N | PSW_V | PSW_CB);
@@ -64,6 +66,14 @@ void cpu_hppa_put_psw(CPUHPPAState *env, target_ureg psw)
     cb |= ((psw >>  9) & 1) <<  8;
     cb |= ((psw >>  8) & 1) <<  4;
     env->psw_cb = cb;
+
+    /* If PSW_P changes, it affects how we translate addresses.  */
+    if ((psw ^ old_psw) & PSW_P) {
+#ifndef CONFIG_USER_ONLY
+        CPUState *src = CPU(hppa_env_get_cpu(env));
+        tlb_flush_by_mmuidx(src, 0xf);
+#endif
+    }
 }
 
 void hppa_cpu_dump_state(CPUState *cs, FILE *f,
diff --git a/target/hppa/mem_helper.c b/target/hppa/mem_helper.c
index 64743bf425..9fc26d3c22 100644
--- a/target/hppa/mem_helper.c
+++ b/target/hppa/mem_helper.c
@@ -130,7 +130,20 @@ int hppa_get_physical_address(CPUHPPAState *env, vaddr addr, int mmu_idx,
         break;
     }
 
-    /* ??? Check PSW_P and ent->access_prot.  This can remove PAGE_WRITE.  */
+    /* access_id == 0 means public page and no check is performed */
+    if ((env->psw & PSW_P) && ent->access_id) {
+        /* If bits [31:1] match, and bit 0 is set, suppress write.  */
+        int match = ent->access_id * 2 + 1;
+
+        if (match == env->cr[CR_PID1] || match == env->cr[CR_PID2] ||
+            match == env->cr[CR_PID3] || match == env->cr[CR_PID4]) {
+            prot &= PAGE_READ | PAGE_EXEC;
+            if (type == PAGE_WRITE) {
+                ret = EXCP_DMPI;
+                goto egress;
+            }
+        }
+    }
 
     /* No guest access type indicates a non-architectural access from
        within QEMU.  Bypass checks for access, D, B and T bits.  */
@@ -333,6 +346,19 @@ void HELPER(ptlbe)(CPUHPPAState *env)
     tlb_flush_by_mmuidx(src, 0xf);
 }
 
+void cpu_hppa_change_prot_id(CPUHPPAState *env)
+{
+    if (env->psw & PSW_P) {
+        CPUState *src = CPU(hppa_env_get_cpu(env));
+        tlb_flush_by_mmuidx(src, 0xf);
+    }
+}
+
+void HELPER(change_prot_id)(CPUHPPAState *env)
+{
+    cpu_hppa_change_prot_id(env);
+}
+
 target_ureg HELPER(lpa)(CPUHPPAState *env, target_ulong addr)
 {
     hwaddr phys;
diff --git a/target/hppa/translate.c b/target/hppa/translate.c
index 6ac196804b..70a7cd4a89 100644
--- a/target/hppa/translate.c
+++ b/target/hppa/translate.c
@@ -2256,6 +2256,16 @@ static bool trans_mtctl(DisasContext *ctx, arg_mtctl *a)
                        offsetof(CPUHPPAState, cr_back[ctl - CR_IIASQ]));
         break;
 
+    case CR_PID1:
+    case CR_PID2:
+    case CR_PID3:
+    case CR_PID4:
+        tcg_gen_st_reg(reg, cpu_env, offsetof(CPUHPPAState, cr[ctl]));
+#ifndef CONFIG_USER_ONLY
+        gen_helper_change_prot_id(cpu_env);
+#endif
+        break;
+
     default:
         tcg_gen_st_reg(reg, cpu_env, offsetof(CPUHPPAState, cr[ctl]));
         break;

Re: [Qemu-devel] [PATCH 01/11] target/hppa: fix overwriting source reg in addb

[Richard Henderson]

On 3/11/19 12:15 PM, Sven Schnelle wrote:
> When one of the source registers is the same as the destination register,
> the source register gets overwritten with the destionation value before
> do_add_sv() is called, which leads to unexpection condition matches.
> 
> Signed-off-by: Sven Schnelle <svens@stackframe.org>
> ---
>  target/hppa/translate.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

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


r~

Re: [Qemu-devel] [PATCH 02/11] target/hppa: fix TLB handling for page 0

[Richard Henderson]

On 3/11/19 12:15 PM, Sven Schnelle wrote:
> Assume the following sequence:
> 
> pitlbe r0(sr0,r0)
> iitlba r4,(sr0,r0)
> ldil L%3000000,r5
> iitlbp r5,(sr0,r0)
> 
> This will purge the whole TLB and add an entry for page 0. However
> the current TLB implementation in helper_iitlba() will store to
> the last empty TLB entry, while helper_iitlbp() will write to the
> first empty entry. That is because an empty entry will match address
> 0 in helper_iitlba()
> 
> Signed-off-by: Sven Schnelle <svens@stackframe.org>
> ---
>  target/hppa/mem_helper.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)

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


r~

Re: [Qemu-devel] [PATCH 03/11] target/hppa: report ITLB_EXCP_MISS for ITLB misses

[Richard Henderson]

On 3/11/19 12:15 PM, Sven Schnelle wrote:
> Signed-off-by: Sven Schnelle <svens@stackframe.org>
> ---
>  target/hppa/mem_helper.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

At one point this didn't boot linux, but perhaps there was a second bug.

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


r~

Re: [Qemu-devel] [PATCH 04/11] target/hppa: add TLB trace events

[Richard Henderson]

On 3/11/19 12:15 PM, Sven Schnelle wrote:
> To ease TLB debugging add a few trace events, which are disabled
> by default so that there's no performance impact.
> 
> Signed-off-by: Sven Schnelle <svens@stackframe.org>
> ---
>  Makefile.objs            |  1 +
>  target/hppa/mem_helper.c | 20 ++++++++++++++++++--
>  target/hppa/op_helper.c  |  2 ++
>  target/hppa/trace-events | 18 ++++++++++++++++++
>  4 files changed, 39 insertions(+), 2 deletions(-)
>  create mode 100644 target/hppa/trace-events

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


r~

Re: [Qemu-devel] [PATCH 05/11] target/hppa: remove PSW I/R/Q bit check

[Richard Henderson]

On 3/11/19 12:15 PM, Sven Schnelle wrote:
> HP ODE use rfi to set the Q bit, and i don't see anything in the
> documentation that this is forbidden. So remove it.
> 
> Signed-off-by: Sven Schnelle <svens@stackframe.org>
> ---
>  target/hppa/op_helper.c | 5 -----
>  1 file changed, 5 deletions(-)

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


r~

Re: [Qemu-devel] [PATCH 10/11] target/hppa: exit TB if either Data or Instruction TLB changes

[Richard Henderson]

On 3/11/19 12:16 PM, Sven Schnelle wrote:
> The current code assumes that we don't need to exit the TB
> if a Data Cache Flush or Insert has happend. However, as we
> have a shared Data/Instruction TLB, a Data cache flush also
> flushes Instruction TLB entries, and a Data cache TLB insert
> might also evict a Instruction TLB entry.
> 
> So exit the TB in all cases if Instruction translation is enabled.
> 
> Signed-off-by: Sven Schnelle <svens@stackframe.org>
> ---
>  target/hppa/translate.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)

I suppose since we advertise a unified i/d tlb, the os feels that
either i/d flush should be sufficient.

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


r~

Re: [Qemu-devel] [PATCH 11/11] target/hppa: call eval_interrupt() after ssm

[Richard Henderson]

On 3/11/19 12:16 PM, Sven Schnelle wrote:
> HP-UX (all versions) is losing timer interrupts, which leads to
> hangs. Pressing a key on the console fixes this, so it looks like
> QEMU is just looping trough TBs without checking for interrupts.
> Further investion showed that this happens when interrupts are
> triggered, without PSW_I enabled. Calling eval_interrupt() after
> PSW_I is set seems to fix this.
> 
> Signed-off-by: Sven Schnelle <svens@stackframe.org>
> ---
>  target/hppa/cpu.h        | 1 +
>  target/hppa/int_helper.c | 2 +-
>  target/hppa/op_helper.c  | 6 ++++++
>  3 files changed, 8 insertions(+), 1 deletion(-)

The correct fix is to exit to the main loop.


r~

Re: [Qemu-devel] [PATCH 11/11] target/hppa: call eval_interrupt() after ssm

[Richard Henderson]

On 3/11/19 8:28 PM, Richard Henderson wrote:
> On 3/11/19 12:16 PM, Sven Schnelle wrote:
>> HP-UX (all versions) is losing timer interrupts, which leads to
>> hangs. Pressing a key on the console fixes this, so it looks like
>> QEMU is just looping trough TBs without checking for interrupts.
>> Further investion showed that this happens when interrupts are
>> triggered, without PSW_I enabled. Calling eval_interrupt() after
>> PSW_I is set seems to fix this.
>>
>> Signed-off-by: Sven Schnelle <svens@stackframe.org>
>> ---
>>  target/hppa/cpu.h        | 1 +
>>  target/hppa/int_helper.c | 2 +-
>>  target/hppa/op_helper.c  | 6 ++++++
>>  3 files changed, 8 insertions(+), 1 deletion(-)
> 
> The correct fix is to exit to the main loop.

... except what we're already doing that.  So I don't see what
can be changed to help.  This doesn't seem to make a difference.


r~