qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PULL 0/8] target-mips queue
@ 2017-08-03 14:45 Yongbok Kim
  2017-08-03 14:45 ` [Qemu-devel] [PULL 1/8] target-mips: Don't stop on [d]mtc0 DESAVE/KScratch Yongbok Kim
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Yongbok Kim @ 2017-08-03 14:45 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell

The following changes since commit aaaec6acad7cf97372d48c1b09126a09697519c8:

  Update version for v2.10.0-rc1 release (2017-08-02 16:36:32 +0100)

are available in the git repository at:

  git://github.com/yongbok/upstream-qemu.git tags/mips-20170803

for you to fetch changes up to d673a68db6963e86536b125af464bb6ed03eba33:

  target/mips: Fix RDHWR CC with icount (2017-08-02 22:18:13 +0100)

----------------------------------------------------------------
MIPS patches 2017-08-03

Changes:
KVM T&E segment support for TCG
malta: leave space for the bootmap after the initrd
Apply CP0.PageMask before writing into TLB entry
Fix fallout from indirect branch optimisation

----------------------------------------------------------------

Aurelien Jarno (1):
  mips/malta: leave space for the bootmap after the initrd

James Hogan (6):
  target-mips: Don't stop on [d]mtc0 DESAVE/KScratch
  mips: Improve segment defs for KVM T&E guests
  mips: Add KVM T&E segment support for TCG
  target/mips: Use BS_EXCP where interrupts are expected
  target/mips: Drop redundant gen_io_start/stop()
  target/mips: Fix RDHWR CC with icount

Leon Alrae (1):
  target-mips: apply CP0.PageMask before writing into TLB entry

 hw/mips/addr.c            | 12 ++++++++
 hw/mips/mips_malta.c      | 24 +++++++--------
 include/hw/mips/cpudevs.h |  5 ++--
 target/mips/helper.c      | 27 +++++++++--------
 target/mips/op_helper.c   |  5 ++--
 target/mips/translate.c   | 74 ++++++++++++++++++++++++++++++-----------------
 6 files changed, 90 insertions(+), 57 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PULL 1/8] target-mips: Don't stop on [d]mtc0 DESAVE/KScratch
  2017-08-03 14:45 [Qemu-devel] [PULL 0/8] target-mips queue Yongbok Kim
@ 2017-08-03 14:45 ` Yongbok Kim
  2017-08-03 14:45 ` [Qemu-devel] [PULL 2/8] mips/malta: leave space for the bootmap after the initrd Yongbok Kim
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Yongbok Kim @ 2017-08-03 14:45 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, James Hogan, Aurelien Jarno

From: James Hogan <james.hogan@imgtec.com>

Writing to the MIPS DESAVE register (and now the KScratch registers)
will stop translation, supposedly due to risk of execution mode
switches. However these registers are basically RW scratch registers
with no side effects so there is no risk of them triggering execution
mode changes.

Drop the bstate = BS_STOP for these registers for both mtc0 and dmtc0.

Fixes: 7a387fffce50 ("Add MIPS32R2 instructions, and generally straighten out the instruction decoding. This is also the first percent towards MIPS64 support.")
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Yongbok Kim <yongbok.kim@imgtec.com>
Reviewed-by: Yongbok Kim <yongbok.kim@imgtec.com>
Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com>
---
 target/mips/translate.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 51626ae..0bca700 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -6386,8 +6386,6 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int reg, int sel)
         default:
             goto cp0_unimplemented;
         }
-        /* Stop translation as we may have switched the execution mode */
-        ctx->bstate = BS_STOP;
         break;
     default:
        goto cp0_unimplemented;
@@ -7714,8 +7712,6 @@ static void gen_dmtc0(DisasContext *ctx, TCGv arg, int reg, int sel)
         default:
             goto cp0_unimplemented;
         }
-        /* Stop translation as we may have switched the execution mode */
-        ctx->bstate = BS_STOP;
         break;
     default:
         goto cp0_unimplemented;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PULL 2/8] mips/malta: leave space for the bootmap after the initrd
  2017-08-03 14:45 [Qemu-devel] [PULL 0/8] target-mips queue Yongbok Kim
  2017-08-03 14:45 ` [Qemu-devel] [PULL 1/8] target-mips: Don't stop on [d]mtc0 DESAVE/KScratch Yongbok Kim
@ 2017-08-03 14:45 ` Yongbok Kim
  2017-08-03 14:45 ` [Qemu-devel] [PULL 3/8] mips: Improve segment defs for KVM T&E guests Yongbok Kim
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Yongbok Kim @ 2017-08-03 14:45 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Aurelien Jarno

From: Aurelien Jarno <aurelien@aurel32.net>

Since commit 9768e2abf7 the initrd is loaded at the end of the low
memory to avoid clash for the kernel relocation when kaslr is used.

However this in turn conflicts with the bootmap memory that the kernel
tries to place after initrd, but in low memory. The bootmap spans the
whole usable physical address space. The machine can have at most 2GiB
of memory, 256MiB of low memory mapped at 0x00000000, and 1792MiB of
high memory mapped at 0x90000000. The biggest bootmap therefore
corresponds to the adresses 0x00000000 -> 0xffffffff, which at 1 bit
per 4kiB page corresponds to 128kiB in memory.

Therefore reserve 128kiB after the initrd.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
Tested-by: Yongbok Kim <yongbok.kim@imgtec.com>
Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com>
---
 hw/mips/mips_malta.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 8ecd544..9dcec27 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -843,7 +843,10 @@ static int64_t load_kernel (void)
     if (loaderparams.initrd_filename) {
         initrd_size = get_image_size (loaderparams.initrd_filename);
         if (initrd_size > 0) {
-            initrd_offset = (loaderparams.ram_low_size - initrd_size
+            /* The kernel allocates the bootmap memory in the low memory after
+               the initrd.  It takes at most 128kiB for 2GB RAM and 4kiB
+               pages.  */
+            initrd_offset = (loaderparams.ram_low_size - initrd_size - 131072
                              - ~INITRD_PAGE_MASK) & INITRD_PAGE_MASK;
             if (kernel_high >= initrd_offset) {
                 fprintf(stderr,
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PULL 3/8] mips: Improve segment defs for KVM T&E guests
  2017-08-03 14:45 [Qemu-devel] [PULL 0/8] target-mips queue Yongbok Kim
  2017-08-03 14:45 ` [Qemu-devel] [PULL 1/8] target-mips: Don't stop on [d]mtc0 DESAVE/KScratch Yongbok Kim
  2017-08-03 14:45 ` [Qemu-devel] [PULL 2/8] mips/malta: leave space for the bootmap after the initrd Yongbok Kim
@ 2017-08-03 14:45 ` Yongbok Kim
  2017-08-03 14:45 ` [Qemu-devel] [PULL 4/8] mips: Add KVM T&E segment support for TCG Yongbok Kim
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Yongbok Kim @ 2017-08-03 14:45 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, James Hogan, Aurelien Jarno, Paolo Bonzini, kvm

From: James Hogan <james.hogan@imgtec.com>

Improve the segment definitions used by get_physical_address() to yield
target_ulong types, e.g. 0xffffffff80000000 instead of 0x80000000. This
is in preparation for enabling emulation of MIPS KVM T&E segments in TCG
MIPS targets, which unlike KVM could potentially have 64-bit
target_ulong. In such a case the offset guest KSEG0 address ends up at
e.g. 0x000000008xxxxxxx instead of 0xffffffff8xxxxxxx.

This also allows the casts to int32_t that force sign extension to be
removed, which removes any confusion due to relational comparison of
unsigned (target_ulong) and signed (int32_t) types.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Yongbok Kim <yongbok.kim@imgtec.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: kvm@vger.kernel.org
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com>
---
 target/mips/helper.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/target/mips/helper.c b/target/mips/helper.c
index a2b79e8..05883b9 100644
--- a/target/mips/helper.c
+++ b/target/mips/helper.c
@@ -216,14 +216,14 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical,
     /* effective address (modified for KVM T&E kernel segments) */
     target_ulong address = real_address;
 
-#define USEG_LIMIT      0x7FFFFFFFUL
-#define KSEG0_BASE      0x80000000UL
-#define KSEG1_BASE      0xA0000000UL
-#define KSEG2_BASE      0xC0000000UL
-#define KSEG3_BASE      0xE0000000UL
+#define USEG_LIMIT      ((target_ulong)(int32_t)0x7FFFFFFFUL)
+#define KSEG0_BASE      ((target_ulong)(int32_t)0x80000000UL)
+#define KSEG1_BASE      ((target_ulong)(int32_t)0xA0000000UL)
+#define KSEG2_BASE      ((target_ulong)(int32_t)0xC0000000UL)
+#define KSEG3_BASE      ((target_ulong)(int32_t)0xE0000000UL)
 
-#define KVM_KSEG0_BASE  0x40000000UL
-#define KVM_KSEG2_BASE  0x60000000UL
+#define KVM_KSEG0_BASE  ((target_ulong)(int32_t)0x40000000UL)
+#define KVM_KSEG2_BASE  ((target_ulong)(int32_t)0x60000000UL)
 
     if (kvm_enabled()) {
         /* KVM T&E adds guest kernel segments in useg */
@@ -307,17 +307,17 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical,
             ret = TLBRET_BADADDR;
         }
 #endif
-    } else if (address < (int32_t)KSEG1_BASE) {
+    } else if (address < KSEG1_BASE) {
         /* kseg0 */
         ret = get_segctl_physical_address(env, physical, prot, real_address, rw,
                                           access_type, mmu_idx,
                                           env->CP0_SegCtl1 >> 16, 0x1FFFFFFF);
-    } else if (address < (int32_t)KSEG2_BASE) {
+    } else if (address < KSEG2_BASE) {
         /* kseg1 */
         ret = get_segctl_physical_address(env, physical, prot, real_address, rw,
                                           access_type, mmu_idx,
                                           env->CP0_SegCtl1, 0x1FFFFFFF);
-    } else if (address < (int32_t)KSEG3_BASE) {
+    } else if (address < KSEG3_BASE) {
         /* sseg (kseg2) */
         ret = get_segctl_physical_address(env, physical, prot, real_address, rw,
                                           access_type, mmu_idx,
@@ -974,8 +974,7 @@ void mips_cpu_do_interrupt(CPUState *cs)
         } else if (cause == 30 && !(env->CP0_Config3 & (1 << CP0C3_SC) &&
                                     env->CP0_Config5 & (1 << CP0C5_CV))) {
             /* Force KSeg1 for cache errors */
-            env->active_tc.PC = (int32_t)KSEG1_BASE |
-                                (env->CP0_EBase & 0x1FFFF000);
+            env->active_tc.PC = KSEG1_BASE | (env->CP0_EBase & 0x1FFFF000);
         } else {
             env->active_tc.PC = env->CP0_EBase & ~0xfff;
         }
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PULL 4/8] mips: Add KVM T&E segment support for TCG
  2017-08-03 14:45 [Qemu-devel] [PULL 0/8] target-mips queue Yongbok Kim
                   ` (2 preceding siblings ...)
  2017-08-03 14:45 ` [Qemu-devel] [PULL 3/8] mips: Improve segment defs for KVM T&E guests Yongbok Kim
@ 2017-08-03 14:45 ` Yongbok Kim
  2017-08-03 14:45 ` [Qemu-devel] [PULL 5/8] target-mips: apply CP0.PageMask before writing into TLB entry Yongbok Kim
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Yongbok Kim @ 2017-08-03 14:45 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, James Hogan, Aurelien Jarno, Paolo Bonzini, kvm

From: James Hogan <james.hogan@imgtec.com>

MIPS KVM trap & emulate guest kernels have a different segment layout
compared with traditional MIPS kernels, to allow both the user and
kernel code to run from the user address segment without repeatedly
trapping to KVM.

QEMU currently supports this layout only for KVM, but its sometimes
useful to be able to run these kernels in QEMU on a PC, so enable it for
TCG too.

This also paves the way for MIPS KVM VZ support (which uses the normal
virtual memory layout) by abstracting whether user mode kernel segments
are in use.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Yongbok Kim <yongbok.kim@imgtec.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: kvm@vger.kernel.org
Reviewed-by: Richard Henderson <rth@twiddle.net>
[Yongbok Kim:
  minor change]
Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com>
---
 hw/mips/addr.c            | 12 ++++++++++++
 hw/mips/mips_malta.c      | 19 ++++++++-----------
 include/hw/mips/cpudevs.h |  5 +++--
 target/mips/helper.c      |  4 ++--
 target/mips/translate.c   |  4 ++--
 5 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/hw/mips/addr.c b/hw/mips/addr.c
index e4e86b4..4da46e1 100644
--- a/hw/mips/addr.c
+++ b/hw/mips/addr.c
@@ -24,6 +24,8 @@
 #include "hw/hw.h"
 #include "hw/mips/cpudevs.h"
 
+static int mips_um_ksegs;
+
 uint64_t cpu_mips_kseg0_to_phys(void *opaque, uint64_t addr)
 {
     return addr & 0x1fffffffll;
@@ -38,3 +40,13 @@ uint64_t cpu_mips_kvm_um_phys_to_kseg0(void *opaque, uint64_t addr)
 {
     return addr | 0x40000000ll;
 }
+
+bool mips_um_ksegs_enabled(void)
+{
+    return mips_um_ksegs;
+}
+
+void mips_um_ksegs_enable(void)
+{
+    mips_um_ksegs = 1;
+}
diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 9dcec27..af678f5 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -818,23 +818,20 @@ static int64_t load_kernel (void)
         exit(1);
     }
 
-    /* Sanity check where the kernel has been linked */
-    if (kvm_enabled()) {
-        if (kernel_entry & 0x80000000ll) {
+    /* Check where the kernel has been linked */
+    if (kernel_entry & 0x80000000ll) {
+        if (kvm_enabled()) {
             error_report("KVM guest kernels must be linked in useg. "
                          "Did you forget to enable CONFIG_KVM_GUEST?");
             exit(1);
         }
 
-        xlate_to_kseg0 = cpu_mips_kvm_um_phys_to_kseg0;
-    } else {
-        if (!(kernel_entry & 0x80000000ll)) {
-            error_report("KVM guest kernels aren't supported with TCG. "
-                         "Did you unintentionally enable CONFIG_KVM_GUEST?");
-            exit(1);
-        }
-
         xlate_to_kseg0 = cpu_mips_phys_to_kseg0;
+    } else {
+        /* if kernel entry is in useg it is probably a KVM T&E kernel */
+        mips_um_ksegs_enable();
+
+        xlate_to_kseg0 = cpu_mips_kvm_um_phys_to_kseg0;
     }
 
     /* load initrd */
diff --git a/include/hw/mips/cpudevs.h b/include/hw/mips/cpudevs.h
index 698339b..291f592 100644
--- a/include/hw/mips/cpudevs.h
+++ b/include/hw/mips/cpudevs.h
@@ -5,11 +5,12 @@
 
 /* Definitions for MIPS CPU internal devices.  */
 
-/* mips_addr.c */
+/* addr.c */
 uint64_t cpu_mips_kseg0_to_phys(void *opaque, uint64_t addr);
 uint64_t cpu_mips_phys_to_kseg0(void *opaque, uint64_t addr);
 uint64_t cpu_mips_kvm_um_phys_to_kseg0(void *opaque, uint64_t addr);
-
+bool mips_um_ksegs_enabled(void);
+void mips_um_ksegs_enable(void);
 
 /* mips_int.c */
 void cpu_mips_irq_init_cpu(MIPSCPU *cpu);
diff --git a/target/mips/helper.c b/target/mips/helper.c
index 05883b9..ca39aca 100644
--- a/target/mips/helper.c
+++ b/target/mips/helper.c
@@ -19,10 +19,10 @@
 #include "qemu/osdep.h"
 
 #include "cpu.h"
-#include "sysemu/kvm.h"
 #include "exec/exec-all.h"
 #include "exec/cpu_ldst.h"
 #include "exec/log.h"
+#include "hw/mips/cpudevs.h"
 
 enum {
     TLBRET_XI = -6,
@@ -225,7 +225,7 @@ static int get_physical_address (CPUMIPSState *env, hwaddr *physical,
 #define KVM_KSEG0_BASE  ((target_ulong)(int32_t)0x40000000UL)
 #define KVM_KSEG2_BASE  ((target_ulong)(int32_t)0x60000000UL)
 
-    if (kvm_enabled()) {
+    if (mips_um_ksegs_enabled()) {
         /* KVM T&E adds guest kernel segments in useg */
         if (real_address >= KVM_KSEG0_BASE) {
             if (real_address < KVM_KSEG2_BASE) {
diff --git a/target/mips/translate.c b/target/mips/translate.c
index 0bca700..88f518b 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -27,10 +27,10 @@
 #include "exec/exec-all.h"
 #include "tcg-op.h"
 #include "exec/cpu_ldst.h"
+#include "hw/mips/cpudevs.h"
 
 #include "exec/helper-proto.h"
 #include "exec/helper-gen.h"
-#include "sysemu/kvm.h"
 #include "exec/semihost.h"
 
 #include "target/mips/trace.h"
@@ -20635,7 +20635,7 @@ void cpu_state_reset(CPUMIPSState *env)
     env->CP0_Wired = 0;
     env->CP0_GlobalNumber = (cs->cpu_index & 0xFF) << CP0GN_VPId;
     env->CP0_EBase = (cs->cpu_index & 0x3FF);
-    if (kvm_enabled()) {
+    if (mips_um_ksegs_enabled()) {
         env->CP0_EBase |= 0x40000000;
     } else {
         env->CP0_EBase |= (int32_t)0x80000000;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PULL 5/8] target-mips: apply CP0.PageMask before writing into TLB entry
  2017-08-03 14:45 [Qemu-devel] [PULL 0/8] target-mips queue Yongbok Kim
                   ` (3 preceding siblings ...)
  2017-08-03 14:45 ` [Qemu-devel] [PULL 4/8] mips: Add KVM T&E segment support for TCG Yongbok Kim
@ 2017-08-03 14:45 ` Yongbok Kim
  2017-08-03 14:45 ` [Qemu-devel] [PULL 6/8] target/mips: Use BS_EXCP where interrupts are expected Yongbok Kim
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Yongbok Kim @ 2017-08-03 14:45 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, Leon Alrae

From: Leon Alrae <leon.alrae@imgtec.com>

PFN0 and PFN1 have to be masked out with PageMask_Mask.

Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
Reviewed-by: Yongbok Kim <yongbok.kim@imgtec.com>
[Yongbok Kim:
  Added commit message]
Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com>
---
 target/mips/op_helper.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
index 526f8e4..320f2b0 100644
--- a/target/mips/op_helper.c
+++ b/target/mips/op_helper.c
@@ -2008,6 +2008,7 @@ static inline uint64_t get_tlb_pfn_from_entrylo(uint64_t entrylo)
 static void r4k_fill_tlb(CPUMIPSState *env, int idx)
 {
     r4k_tlb_t *tlb;
+    uint64_t mask = env->CP0_PageMask >> (TARGET_PAGE_BITS + 1);
 
     /* XXX: detect conflicting TLBs and raise a MCHECK exception when needed */
     tlb = &env->tlb->mmu.r4k.tlb[idx];
@@ -2028,13 +2029,13 @@ static void r4k_fill_tlb(CPUMIPSState *env, int idx)
     tlb->C0 = (env->CP0_EntryLo0 >> 3) & 0x7;
     tlb->XI0 = (env->CP0_EntryLo0 >> CP0EnLo_XI) & 1;
     tlb->RI0 = (env->CP0_EntryLo0 >> CP0EnLo_RI) & 1;
-    tlb->PFN[0] = get_tlb_pfn_from_entrylo(env->CP0_EntryLo0) << 12;
+    tlb->PFN[0] = (get_tlb_pfn_from_entrylo(env->CP0_EntryLo0) & ~mask) << 12;
     tlb->V1 = (env->CP0_EntryLo1 & 2) != 0;
     tlb->D1 = (env->CP0_EntryLo1 & 4) != 0;
     tlb->C1 = (env->CP0_EntryLo1 >> 3) & 0x7;
     tlb->XI1 = (env->CP0_EntryLo1 >> CP0EnLo_XI) & 1;
     tlb->RI1 = (env->CP0_EntryLo1 >> CP0EnLo_RI) & 1;
-    tlb->PFN[1] = get_tlb_pfn_from_entrylo(env->CP0_EntryLo1) << 12;
+    tlb->PFN[1] = (get_tlb_pfn_from_entrylo(env->CP0_EntryLo1) & ~mask) << 12;
 }
 
 void r4k_helper_tlbinv(CPUMIPSState *env)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PULL 6/8] target/mips: Use BS_EXCP where interrupts are expected
  2017-08-03 14:45 [Qemu-devel] [PULL 0/8] target-mips queue Yongbok Kim
                   ` (4 preceding siblings ...)
  2017-08-03 14:45 ` [Qemu-devel] [PULL 5/8] target-mips: apply CP0.PageMask before writing into TLB entry Yongbok Kim
@ 2017-08-03 14:45 ` Yongbok Kim
  2017-08-03 14:45 ` [Qemu-devel] [PULL 7/8] target/mips: Drop redundant gen_io_start/stop() Yongbok Kim
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Yongbok Kim @ 2017-08-03 14:45 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, James Hogan, Aurelien Jarno, Richard Henderson

From: James Hogan <james.hogan@imgtec.com>

Commit e350d8ca3ac7 ("target/mips: optimize indirect branches") made
indirect branches able to directly find the next TB and jump straight to
it without breaking out of translated code and going around the main
execution loop. This breaks the assumption in target/mips/translate.c
that BS_STOP is sufficient to cause pending interrupts to be handled,
since interrupts are only checked in the main loop.

Fix a few of these assumptions by using gen_save_pc to update the saved
PC and using BS_EXCP instead of BS_STOP:

 - [D]MFC0 CP0_Count may trigger a timer interrupt which should be
   immediately handled.

 - [D]MTC0 CP0_Cause may trigger an interrupt (but in fact translation
   was only even being stopped in the DMTC0 case).

 - [D]MTC0 CP0_<any> when icount is used is assumed could potentially
   cause interrupts.

 - EI may trigger an interrupt which was pending. I specifically hit
   this case when running KVM nested in mipsel-softmmu. A timer
   interrupt while the 2nd guest was executing is caught by KVM which
   switches back to the normal Linux exception base and re-enables
   interrupts with EI. Since the above commit QEMU doesn't leave
   translated code until the nested KVM has already restored the KVM
   exception base and returned to the 2nd guest, at which point it is
   too late to check for pending interrupts and it gets stuck in an
   infinite loop of unhandled interrupts.

Something similar was needed for ARM in commit b29fd33db578
("target/arm: use DISAS_EXIT for eret handling").

Fixes: e350d8ca3ac7 ("target/mips: optimize indirect branches")
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Yongbok Kim <yongbok.kim@imgtec.com>
Cc: Richard Henderson <rth@twiddle.net>
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com>
---
 target/mips/translate.c | 47 ++++++++++++++++++++++++++++++++++-------------
 1 file changed, 34 insertions(+), 13 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index 88f518b..ba6b8f5 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -5334,8 +5334,10 @@ static void gen_mfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
                 gen_io_end();
             }
             /* Break the TB to be able to take timer interrupts immediately
-               after reading count.  */
-            ctx->bstate = BS_STOP;
+               after reading count. BS_STOP isn't sufficient, we need to ensure
+               we break completely out of translated code.  */
+            gen_save_pc(ctx->pc + 4);
+            ctx->bstate = BS_EXCP;
             rn = "Count";
             break;
         /* 6,7 are implementation dependent */
@@ -6061,6 +6063,11 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int reg, int sel)
         case 0:
             save_cpu_state(ctx, 1);
             gen_helper_mtc0_cause(cpu_env, arg);
+            /* Stop translation as we may have triggered an interrupt. BS_STOP
+             * isn't sufficient, we need to ensure we break out of translated
+             * code to check for pending interrupts.  */
+            gen_save_pc(ctx->pc + 4);
+            ctx->bstate = BS_EXCP;
             rn = "Cause";
             break;
         default:
@@ -6395,7 +6402,10 @@ static void gen_mtc0(DisasContext *ctx, TCGv arg, int reg, int sel)
     /* For simplicity assume that all writes can cause interrupts.  */
     if (ctx->tb->cflags & CF_USE_ICOUNT) {
         gen_io_end();
-        ctx->bstate = BS_STOP;
+        /* BS_STOP isn't sufficient, we need to ensure we break out of
+         * translated code to check for pending interrupts.  */
+        gen_save_pc(ctx->pc + 4);
+        ctx->bstate = BS_EXCP;
     }
     return;
 
@@ -6676,8 +6686,10 @@ static void gen_dmfc0(DisasContext *ctx, TCGv arg, int reg, int sel)
                 gen_io_end();
             }
             /* Break the TB to be able to take timer interrupts immediately
-               after reading count.  */
-            ctx->bstate = BS_STOP;
+               after reading count. BS_STOP isn't sufficient, we need to ensure
+               we break completely out of translated code.  */
+            gen_save_pc(ctx->pc + 4);
+            ctx->bstate = BS_EXCP;
             rn = "Count";
             break;
         /* 6,7 are implementation dependent */
@@ -7398,8 +7410,11 @@ static void gen_dmtc0(DisasContext *ctx, TCGv arg, int reg, int sel)
             if (ctx->tb->cflags & CF_USE_ICOUNT) {
                 gen_io_end();
             }
-            /* Stop translation as we may have triggered an intetrupt */
-            ctx->bstate = BS_STOP;
+            /* Stop translation as we may have triggered an intetrupt. BS_STOP
+             * isn't sufficient, we need to ensure we break out of translated
+             * code to check for pending interrupts.  */
+            gen_save_pc(ctx->pc + 4);
+            ctx->bstate = BS_EXCP;
             rn = "Cause";
             break;
         default:
@@ -7721,7 +7736,10 @@ static void gen_dmtc0(DisasContext *ctx, TCGv arg, int reg, int sel)
     /* For simplicity assume that all writes can cause interrupts.  */
     if (ctx->tb->cflags & CF_USE_ICOUNT) {
         gen_io_end();
-        ctx->bstate = BS_STOP;
+        /* BS_STOP isn't sufficient, we need to ensure we break out of
+         * translated code to check for pending interrupts.  */
+        gen_save_pc(ctx->pc + 4);
+        ctx->bstate = BS_EXCP;
     }
     return;
 
@@ -13565,8 +13583,10 @@ static void gen_pool32axf (CPUMIPSState *env, DisasContext *ctx, int rt, int rs)
                 save_cpu_state(ctx, 1);
                 gen_helper_ei(t0, cpu_env);
                 gen_store_gpr(t0, rs);
-                /* Stop translation as we may have switched the execution mode */
-                ctx->bstate = BS_STOP;
+                /* BS_STOP isn't sufficient, we need to ensure we break out
+                   of translated code to check for pending interrupts.  */
+                gen_save_pc(ctx->pc + 4);
+                ctx->bstate = BS_EXCP;
                 tcg_temp_free(t0);
             }
             break;
@@ -19688,9 +19708,10 @@ static void decode_opc(CPUMIPSState *env, DisasContext *ctx)
                     save_cpu_state(ctx, 1);
                     gen_helper_ei(t0, cpu_env);
                     gen_store_gpr(t0, rt);
-                    /* Stop translation as we may have switched
-                       the execution mode.  */
-                    ctx->bstate = BS_STOP;
+                    /* BS_STOP isn't sufficient, we need to ensure we break out
+                       of translated code to check for pending interrupts.  */
+                    gen_save_pc(ctx->pc + 4);
+                    ctx->bstate = BS_EXCP;
                     break;
                 default:            /* Invalid */
                     MIPS_INVAL("mfmc0");
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PULL 7/8] target/mips: Drop redundant gen_io_start/stop()
  2017-08-03 14:45 [Qemu-devel] [PULL 0/8] target-mips queue Yongbok Kim
                   ` (5 preceding siblings ...)
  2017-08-03 14:45 ` [Qemu-devel] [PULL 6/8] target/mips: Use BS_EXCP where interrupts are expected Yongbok Kim
@ 2017-08-03 14:45 ` Yongbok Kim
  2017-08-03 14:45 ` [Qemu-devel] [PULL 8/8] target/mips: Fix RDHWR CC with icount Yongbok Kim
  2017-08-04 12:46 ` [Qemu-devel] [PULL 0/8] target-mips queue Peter Maydell
  8 siblings, 0 replies; 10+ messages in thread
From: Yongbok Kim @ 2017-08-03 14:45 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, James Hogan, Aurelien Jarno

From: James Hogan <james.hogan@imgtec.com>

DMTC0 CP0_Cause does a redundant gen_io_start() and gen_io_end() pair,
even though this is done for all DMTC0 operations outside of the switch
statement. Remove these redundant calls.

Fixes: 5dc5d9f055c5 ("mips: more fixes to the MIPS interrupt glue logic")
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Yongbok Kim <yongbok.kim@imgtec.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com>
---
 target/mips/translate.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index ba6b8f5..bcea2a1 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -7401,15 +7401,7 @@ static void gen_dmtc0(DisasContext *ctx, TCGv arg, int reg, int sel)
         switch (sel) {
         case 0:
             save_cpu_state(ctx, 1);
-            /* Mark as an IO operation because we may trigger a software
-               interrupt.  */
-            if (ctx->tb->cflags & CF_USE_ICOUNT) {
-                gen_io_start();
-            }
             gen_helper_mtc0_cause(cpu_env, arg);
-            if (ctx->tb->cflags & CF_USE_ICOUNT) {
-                gen_io_end();
-            }
             /* Stop translation as we may have triggered an intetrupt. BS_STOP
              * isn't sufficient, we need to ensure we break out of translated
              * code to check for pending interrupts.  */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [Qemu-devel] [PULL 8/8] target/mips: Fix RDHWR CC with icount
  2017-08-03 14:45 [Qemu-devel] [PULL 0/8] target-mips queue Yongbok Kim
                   ` (6 preceding siblings ...)
  2017-08-03 14:45 ` [Qemu-devel] [PULL 7/8] target/mips: Drop redundant gen_io_start/stop() Yongbok Kim
@ 2017-08-03 14:45 ` Yongbok Kim
  2017-08-04 12:46 ` [Qemu-devel] [PULL 0/8] target-mips queue Peter Maydell
  8 siblings, 0 replies; 10+ messages in thread
From: Yongbok Kim @ 2017-08-03 14:45 UTC (permalink / raw)
  To: qemu-devel; +Cc: Peter Maydell, James Hogan, Aurelien Jarno

From: James Hogan <james.hogan@imgtec.com>

RDHWR CC reads the CPU timer like MFC0 CP0_Count, so with icount enabled
it must set can_do_io while it calls the helper to avoid the "Bad icount
read" error. It should also break out of the translation loop to ensure
that timer interrupts are immediately handled.

Fixes: 2e70f6efa8b9 ("Add instruction counter.")
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Aurelien Jarno <aurelien@aurel32.net>
Cc: Yongbok Kim <yongbok.kim@imgtec.com>
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Yongbok Kim <yongbok.kim@imgtec.com>
---
 target/mips/translate.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/target/mips/translate.c b/target/mips/translate.c
index bcea2a1..c78d272 100644
--- a/target/mips/translate.c
+++ b/target/mips/translate.c
@@ -10755,8 +10755,19 @@ static void gen_rdhwr(DisasContext *ctx, int rt, int rd, int sel)
         gen_store_gpr(t0, rt);
         break;
     case 2:
+        if (ctx->tb->cflags & CF_USE_ICOUNT) {
+            gen_io_start();
+        }
         gen_helper_rdhwr_cc(t0, cpu_env);
+        if (ctx->tb->cflags & CF_USE_ICOUNT) {
+            gen_io_end();
+        }
         gen_store_gpr(t0, rt);
+        /* Break the TB to be able to take timer interrupts immediately
+           after reading count. BS_STOP isn't sufficient, we need to ensure
+           we break completely out of translated code.  */
+        gen_save_pc(ctx->pc + 4);
+        ctx->bstate = BS_EXCP;
         break;
     case 3:
         gen_helper_rdhwr_ccres(t0, cpu_env);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [Qemu-devel] [PULL 0/8] target-mips queue
  2017-08-03 14:45 [Qemu-devel] [PULL 0/8] target-mips queue Yongbok Kim
                   ` (7 preceding siblings ...)
  2017-08-03 14:45 ` [Qemu-devel] [PULL 8/8] target/mips: Fix RDHWR CC with icount Yongbok Kim
@ 2017-08-04 12:46 ` Peter Maydell
  8 siblings, 0 replies; 10+ messages in thread
From: Peter Maydell @ 2017-08-04 12:46 UTC (permalink / raw)
  To: Yongbok Kim; +Cc: QEMU Developers

On 3 August 2017 at 15:45, Yongbok Kim <yongbok.kim@imgtec.com> wrote:
> The following changes since commit aaaec6acad7cf97372d48c1b09126a09697519c8:
>
>   Update version for v2.10.0-rc1 release (2017-08-02 16:36:32 +0100)
>
> are available in the git repository at:
>
>   git://github.com/yongbok/upstream-qemu.git tags/mips-20170803
>
> for you to fetch changes up to d673a68db6963e86536b125af464bb6ed03eba33:
>
>   target/mips: Fix RDHWR CC with icount (2017-08-02 22:18:13 +0100)
>
> ----------------------------------------------------------------
> MIPS patches 2017-08-03
>
> Changes:
> KVM T&E segment support for TCG
> malta: leave space for the bootmap after the initrd
> Apply CP0.PageMask before writing into TLB entry
> Fix fallout from indirect branch optimisation
>

Applied, thanks.

-- PMM

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-08-04 13:22 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-03 14:45 [Qemu-devel] [PULL 0/8] target-mips queue Yongbok Kim
2017-08-03 14:45 ` [Qemu-devel] [PULL 1/8] target-mips: Don't stop on [d]mtc0 DESAVE/KScratch Yongbok Kim
2017-08-03 14:45 ` [Qemu-devel] [PULL 2/8] mips/malta: leave space for the bootmap after the initrd Yongbok Kim
2017-08-03 14:45 ` [Qemu-devel] [PULL 3/8] mips: Improve segment defs for KVM T&E guests Yongbok Kim
2017-08-03 14:45 ` [Qemu-devel] [PULL 4/8] mips: Add KVM T&E segment support for TCG Yongbok Kim
2017-08-03 14:45 ` [Qemu-devel] [PULL 5/8] target-mips: apply CP0.PageMask before writing into TLB entry Yongbok Kim
2017-08-03 14:45 ` [Qemu-devel] [PULL 6/8] target/mips: Use BS_EXCP where interrupts are expected Yongbok Kim
2017-08-03 14:45 ` [Qemu-devel] [PULL 7/8] target/mips: Drop redundant gen_io_start/stop() Yongbok Kim
2017-08-03 14:45 ` [Qemu-devel] [PULL 8/8] target/mips: Fix RDHWR CC with icount Yongbok Kim
2017-08-04 12:46 ` [Qemu-devel] [PULL 0/8] target-mips queue Peter Maydell

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).