[PULL 00/10] Misc fixes for 2023-07-25

[PULL 00/10] Misc fixes for 2023-07-25

[Philippe Mathieu-Daudé]

The following changes since commit 3ee44ec72753ec0ff05ad1569dfa609203d722b2:

  Merge tag 'pull-request-2023-07-24' of https://gitlab.com/thuth/qemu into staging (2023-07-24 18:06:36 +0100)

are available in the Git repository at:

  https://github.com/philmd/qemu.git tags/misc-fixes-20230725

for you to fetch changes up to f8cfdd2038c1823301e6df753242e465b1dc8539:

  target/tricore: Rename tricore_feature (2023-07-25 14:42:00 +0200)

----------------------------------------------------------------
Misc patches queue

hw/sd/sdhci: Default I/O ops to little endian
hw/mips/loongson3-virt: Only use default USB if available
hw/char/escc: Implement loopback mode to allow self-testing
target/mips: Avoid overruns and shifts by negative number
target/sparc: Handle FPRS correctly on big-endian hosts
target/tricore: Rename tricore_feature to avoid clash with libcapstone

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

Bastian Koppelmann (1):
  target/tricore: Rename tricore_feature

Bernhard Beschow (1):
  hw/sd/sdhci: Do not force sdhci_mmio_*_ops onto all SD controllers

Peter Maydell (2):
  target/mips: Avoid shift by negative number in
    page_table_walk_refill()
  target/sparc: Handle FPRS correctly on big-endian hosts

Philippe Mathieu-Daudé (4):
  target/mips/mxu: Replace magic array size by its definition
  target/mips/mxu: Avoid overrun in gen_mxu_S32SLT()
  target/mips/mxu: Avoid overrun in gen_mxu_q8adde()
  target/mips: Pass directory/leaf shift values to walk_directory()

Thomas Huth (2):
  hw/mips: Improve the default USB settings in the loongson3-virt
    machine
  hw/char/escc: Implement loopback mode

 target/sparc/cpu.h                  |  2 +-
 target/tricore/cpu.h                |  2 +-
 hw/char/escc.c                      |  4 ++-
 hw/mips/loongson3_virt.c            |  2 +-
 hw/sd/sdhci.c                       |  8 ++++-
 target/mips/tcg/mxu_translate.c     | 36 +++++++++++++++-------
 target/mips/tcg/sysemu/tlb_helper.c | 48 ++++++++++++++---------------
 target/sparc/cpu.c                  |  4 +--
 target/sparc/machine.c              |  3 +-
 target/sparc/monitor.c              |  2 +-
 target/tricore/cpu.c                |  8 ++---
 target/tricore/helper.c             |  4 +--
 target/tricore/op_helper.c          |  4 +--
 13 files changed, 75 insertions(+), 52 deletions(-)

-- 
2.38.1

[PULL 01/10] hw/sd/sdhci: Do not force sdhci_mmio_*_ops onto all SD controllers

[Philippe Mathieu-Daudé]

From: Bernhard Beschow <shentey@gmail.com>

Since commit c0a55a0c9da2 "hw/sd/sdhci: Support big endian SD host controller
interfaces" sdhci_common_realize() forces all SD card controllers to use either
sdhci_mmio_le_ops or sdhci_mmio_be_ops, depending on the "endianness" property.
However, there are device models which use different MMIO ops: TYPE_IMX_USDHC
uses usdhc_mmio_ops and TYPE_S3C_SDHCI uses sdhci_s3c_mmio_ops.

Forcing sdhci_mmio_le_ops breaks SD card handling on the "sabrelite" board, for
example. Fix this by defaulting the io_ops to little endian and switch to big
endian in sdhci_common_realize() only if there is a matchig big endian variant
available.

Fixes: c0a55a0c9da2 ("hw/sd/sdhci: Support big endian SD host controller
interfaces")

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
Tested-by: Guenter Roeck <linux@roeck-us.net>
Message-Id: <20230709080950.92489-1-shentey@gmail.com>
---
 hw/sd/sdhci.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 6811f0f1a8..362c2c86aa 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1382,6 +1382,8 @@ void sdhci_initfn(SDHCIState *s)
 
     s->insert_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sdhci_raise_insertion_irq, s);
     s->transfer_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sdhci_data_transfer, s);
+
+    s->io_ops = &sdhci_mmio_le_ops;
 }
 
 void sdhci_uninitfn(SDHCIState *s)
@@ -1399,9 +1401,13 @@ void sdhci_common_realize(SDHCIState *s, Error **errp)
 
     switch (s->endianness) {
     case DEVICE_LITTLE_ENDIAN:
-        s->io_ops = &sdhci_mmio_le_ops;
+        /* s->io_ops is little endian by default */
         break;
     case DEVICE_BIG_ENDIAN:
+        if (s->io_ops != &sdhci_mmio_le_ops) {
+            error_setg(errp, "SD controller doesn't support big endianness");
+            return;
+        }
         s->io_ops = &sdhci_mmio_be_ops;
         break;
     default:
-- 
2.38.1

Re: [PULL 01/10] hw/sd/sdhci: Do not force sdhci_mmio_*_ops onto all SD controllers

[Bernhard Beschow]

Am 25. Juli 2023 14:58:20 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>:
>From: Bernhard Beschow <shentey@gmail.com>
>
>Since commit c0a55a0c9da2 "hw/sd/sdhci: Support big endian SD host controller
>interfaces" sdhci_common_realize() forces all SD card controllers to use either
>sdhci_mmio_le_ops or sdhci_mmio_be_ops, depending on the "endianness" property.
>However, there are device models which use different MMIO ops: TYPE_IMX_USDHC
>uses usdhc_mmio_ops and TYPE_S3C_SDHCI uses sdhci_s3c_mmio_ops.
>
>Forcing sdhci_mmio_le_ops breaks SD card handling on the "sabrelite" board, for
>example. Fix this by defaulting the io_ops to little endian and switch to big
>endian in sdhci_common_realize() only if there is a matchig big endian variant
>available.
>
>Fixes: c0a55a0c9da2 ("hw/sd/sdhci: Support big endian SD host controller
>interfaces")
>
>Signed-off-by: Bernhard Beschow <shentey@gmail.com>
>Tested-by: Guenter Roeck <linux@roeck-us.net>
>Message-Id: <20230709080950.92489-1-shentey@gmail.com>

Ping qemu-stable

AFAIU the regression happens since 8.0, thus would be great to have a fix there.

Best regards,
Bernhard

>---
> hw/sd/sdhci.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
>diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
>index 6811f0f1a8..362c2c86aa 100644
>--- a/hw/sd/sdhci.c
>+++ b/hw/sd/sdhci.c
>@@ -1382,6 +1382,8 @@ void sdhci_initfn(SDHCIState *s)
> 
>     s->insert_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sdhci_raise_insertion_irq, s);
>     s->transfer_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sdhci_data_transfer, s);
>+
>+    s->io_ops = &sdhci_mmio_le_ops;
> }
> 
> void sdhci_uninitfn(SDHCIState *s)
>@@ -1399,9 +1401,13 @@ void sdhci_common_realize(SDHCIState *s, Error **errp)
> 
>     switch (s->endianness) {
>     case DEVICE_LITTLE_ENDIAN:
>-        s->io_ops = &sdhci_mmio_le_ops;
>+        /* s->io_ops is little endian by default */
>         break;
>     case DEVICE_BIG_ENDIAN:
>+        if (s->io_ops != &sdhci_mmio_le_ops) {
>+            error_setg(errp, "SD controller doesn't support big endianness");
>+            return;
>+        }
>         s->io_ops = &sdhci_mmio_be_ops;
>         break;
>     default:

[PULL 02/10] hw/mips: Improve the default USB settings in the loongson3-virt machine

[Philippe Mathieu-Daudé]

From: Thomas Huth <thuth@redhat.com>

It's possible to compile QEMU without the USB devices (e.g. when using
"--without-default-devices" as option for the "configure" script).
To be still able to run the loongson3-virt machine in default mode with
such a QEMU binary, we have to check here for the availability of the
OHCI controller first before instantiating the USB devices.

Signed-off-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20230714104903.284845-1-thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/mips/loongson3_virt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/mips/loongson3_virt.c b/hw/mips/loongson3_virt.c
index 4018b8c1d3..3ad0a223df 100644
--- a/hw/mips/loongson3_virt.c
+++ b/hw/mips/loongson3_virt.c
@@ -447,7 +447,7 @@ static inline void loongson3_virt_devices_init(MachineState *machine,
 
     pci_vga_init(pci_bus);
 
-    if (defaults_enabled()) {
+    if (defaults_enabled() && object_class_by_name("pci-ohci")) {
         pci_create_simple(pci_bus, -1, "pci-ohci");
         usb_create_simple(usb_bus_find(-1), "usb-kbd");
         usb_create_simple(usb_bus_find(-1), "usb-tablet");
-- 
2.38.1

[PULL 03/10] hw/char/escc: Implement loopback mode

[Philippe Mathieu-Daudé]

From: Thomas Huth <huth@tuxfamily.org>

The firmware of the m68k next-cube machine uses the loopback mode
for self-testing the hardware and currently fails during this step.
By implementing the loopback mode, we can make the firmware pass
to the next step.

Signed-off-by: Thomas Huth <huth@tuxfamily.org>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20230716153519.31722-1-huth@tuxfamily.org>
---
 hw/char/escc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/char/escc.c b/hw/char/escc.c
index 4f3872bfe9..4be66053c1 100644
--- a/hw/char/escc.c
+++ b/hw/char/escc.c
@@ -653,7 +653,9 @@ static void escc_mem_write(void *opaque, hwaddr addr,
         escc_update_irq(s);
         s->tx = val;
         if (s->wregs[W_TXCTRL2] & TXCTRL2_TXEN) { /* tx enabled */
-            if (qemu_chr_fe_backend_connected(&s->chr)) {
+            if (s->wregs[W_MISC2] & MISC2_LCL_LOOP) {
+                serial_receive_byte(s, s->tx);
+            } else if (qemu_chr_fe_backend_connected(&s->chr)) {
                 /*
                  * XXX this blocks entire thread. Rewrite to use
                  * qemu_chr_fe_write and background I/O callbacks
-- 
2.38.1

[PULL 04/10] target/mips/mxu: Replace magic array size by its definition

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20230712060806.82323-2-philmd@linaro.org>
---
 target/mips/tcg/mxu_translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/mips/tcg/mxu_translate.c b/target/mips/tcg/mxu_translate.c
index deb8060a17..b007948a73 100644
--- a/target/mips/tcg/mxu_translate.c
+++ b/target/mips/tcg/mxu_translate.c
@@ -609,7 +609,7 @@ enum {
 static TCGv mxu_gpr[NUMBER_OF_MXU_REGISTERS - 1];
 static TCGv mxu_CR;
 
-static const char mxuregnames[][4] = {
+static const char mxuregnames[NUMBER_OF_MXU_REGISTERS][4] = {
     "XR1",  "XR2",  "XR3",  "XR4",  "XR5",  "XR6",  "XR7",  "XR8",
     "XR9",  "XR10", "XR11", "XR12", "XR13", "XR14", "XR15", "XCR",
 };
-- 
2.38.1

[PULL 05/10] target/mips/mxu: Avoid overrun in gen_mxu_S32SLT()

[Philippe Mathieu-Daudé]

Coverity reports a potential overrun (CID 1517769):

  Overrunning array "mxu_gpr" of 15 8-byte elements at
  element index 4294967295 (byte offset 34359738367)
  using index "XRb - 1U" (which evaluates to 4294967295).

Use gen_load_mxu_gpr() to safely load MXU registers.

Fixes: ff7936f009 ("target/mips/mxu: Add S32SLT ... insns")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20230712060806.82323-3-philmd@linaro.org>
---
 target/mips/tcg/mxu_translate.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/target/mips/tcg/mxu_translate.c b/target/mips/tcg/mxu_translate.c
index b007948a73..520747a597 100644
--- a/target/mips/tcg/mxu_translate.c
+++ b/target/mips/tcg/mxu_translate.c
@@ -2434,8 +2434,12 @@ static void gen_mxu_S32SLT(DisasContext *ctx)
         tcg_gen_movi_tl(mxu_gpr[XRa - 1], 0);
     } else {
         /* the most general case */
-        tcg_gen_setcond_tl(TCG_COND_LT, mxu_gpr[XRa - 1],
-                           mxu_gpr[XRb - 1], mxu_gpr[XRc - 1]);
+        TCGv t0 = tcg_temp_new();
+        TCGv t1 = tcg_temp_new();
+
+        gen_load_mxu_gpr(t0, XRb);
+        gen_load_mxu_gpr(t1, XRc);
+        tcg_gen_setcond_tl(TCG_COND_LT, mxu_gpr[XRa - 1], t0, t1);
     }
 }
 
-- 
2.38.1

[PULL 06/10] target/mips/mxu: Avoid overrun in gen_mxu_q8adde()

[Philippe Mathieu-Daudé]

Coverity reports a potential overruns (CID 1517770):

  Overrunning array "mxu_gpr" of 15 8-byte elements at
  element index 4294967295 (byte offset 34359738367)
  using index "XRb - 1U" (which evaluates to 4294967295).

Add a gen_extract_mxu_gpr() helper similar to
gen_load_mxu_gpr() to safely extract MXU registers.

Fixes: eb79951ab6 ("target/mips/mxu: Add Q8ADDE ... insns")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20230712060806.82323-4-philmd@linaro.org>
---
 target/mips/tcg/mxu_translate.c | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/target/mips/tcg/mxu_translate.c b/target/mips/tcg/mxu_translate.c
index 520747a597..e662acd5df 100644
--- a/target/mips/tcg/mxu_translate.c
+++ b/target/mips/tcg/mxu_translate.c
@@ -644,6 +644,16 @@ static inline void gen_store_mxu_gpr(TCGv t, unsigned int reg)
     }
 }
 
+static inline void gen_extract_mxu_gpr(TCGv t, unsigned int reg,
+                                       unsigned int ofs, unsigned int len)
+{
+    if (reg == 0) {
+        tcg_gen_movi_tl(t, 0);
+    } else if (reg <= 15) {
+        tcg_gen_extract_tl(t, mxu_gpr[reg - 1], ofs, len);
+    }
+}
+
 /* MXU control register moves. */
 static inline void gen_load_mxu_cr(TCGv t)
 {
@@ -3004,10 +3014,10 @@ static void gen_mxu_q8adde(DisasContext *ctx, bool accumulate)
         TCGv t5 = tcg_temp_new();
 
         if (XRa != 0) {
-            tcg_gen_extract_tl(t0, mxu_gpr[XRb - 1], 16, 8);
-            tcg_gen_extract_tl(t1, mxu_gpr[XRc - 1], 16, 8);
-            tcg_gen_extract_tl(t2, mxu_gpr[XRb - 1], 24, 8);
-            tcg_gen_extract_tl(t3, mxu_gpr[XRc - 1], 24, 8);
+            gen_extract_mxu_gpr(t0, XRb, 16, 8);
+            gen_extract_mxu_gpr(t1, XRc, 16, 8);
+            gen_extract_mxu_gpr(t2, XRb, 24, 8);
+            gen_extract_mxu_gpr(t3, XRc, 24, 8);
             if (aptn2 & 2) {
                 tcg_gen_sub_tl(t0, t0, t1);
                 tcg_gen_sub_tl(t2, t2, t3);
@@ -3027,10 +3037,10 @@ static void gen_mxu_q8adde(DisasContext *ctx, bool accumulate)
             tcg_gen_or_tl(t4, t2, t0);
         }
         if (XRd != 0) {
-            tcg_gen_extract_tl(t0, mxu_gpr[XRb - 1], 0, 8);
-            tcg_gen_extract_tl(t1, mxu_gpr[XRc - 1], 0, 8);
-            tcg_gen_extract_tl(t2, mxu_gpr[XRb - 1], 8, 8);
-            tcg_gen_extract_tl(t3, mxu_gpr[XRc - 1], 8, 8);
+            gen_extract_mxu_gpr(t0, XRb, 0, 8);
+            gen_extract_mxu_gpr(t1, XRc, 0, 8);
+            gen_extract_mxu_gpr(t2, XRb, 8, 8);
+            gen_extract_mxu_gpr(t3, XRc, 8, 8);
             if (aptn2 & 1) {
                 tcg_gen_sub_tl(t0, t0, t1);
                 tcg_gen_sub_tl(t2, t2, t3);
-- 
2.38.1

[PULL 07/10] target/mips: Pass directory/leaf shift values to walk_directory()

[Philippe Mathieu-Daudé]

We already evaluated directory_shift and leaf_shift in
page_table_walk_refill(), no need to do that again: pass
as argument.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20230717213504.24777-2-philmd@linaro.org>
---
 target/mips/tcg/sysemu/tlb_helper.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/target/mips/tcg/sysemu/tlb_helper.c b/target/mips/tcg/sysemu/tlb_helper.c
index e5e1e9dd3f..e7be649b02 100644
--- a/target/mips/tcg/sysemu/tlb_helper.c
+++ b/target/mips/tcg/sysemu/tlb_helper.c
@@ -623,18 +623,13 @@ static uint64_t get_tlb_entry_layout(CPUMIPSState *env, uint64_t entry,
 
 static int walk_directory(CPUMIPSState *env, uint64_t *vaddr,
         int directory_index, bool *huge_page, bool *hgpg_directory_hit,
-        uint64_t *pw_entrylo0, uint64_t *pw_entrylo1)
+        uint64_t *pw_entrylo0, uint64_t *pw_entrylo1,
+        int directory_shift, int leaf_shift)
 {
     int dph = (env->CP0_PWCtl >> CP0PC_DPH) & 0x1;
     int psn = (env->CP0_PWCtl >> CP0PC_PSN) & 0x3F;
     int hugepg = (env->CP0_PWCtl >> CP0PC_HUGEPG) & 0x1;
     int pf_ptew = (env->CP0_PWField >> CP0PF_PTEW) & 0x3F;
-    int ptew = (env->CP0_PWSize >> CP0PS_PTEW) & 0x3F;
-    int native_shift = (((env->CP0_PWSize >> CP0PS_PS) & 1) == 0) ? 2 : 3;
-    int directory_shift = (ptew > 1) ? -1 :
-            (hugepg && (ptew == 1)) ? native_shift + 1 : native_shift;
-    int leaf_shift = (ptew > 1) ? -1 :
-            (ptew == 1) ? native_shift + 1 : native_shift;
     uint32_t direntry_size = 1 << (directory_shift + 3);
     uint32_t leafentry_size = 1 << (leaf_shift + 3);
     uint64_t entry;
@@ -779,7 +774,8 @@ static bool page_table_walk_refill(CPUMIPSState *env, vaddr address,
     if (gdw > 0) {
         vaddr |= goffset;
         switch (walk_directory(env, &vaddr, pf_gdw, &huge_page, &hgpg_gdhit,
-                               &pw_entrylo0, &pw_entrylo1))
+                               &pw_entrylo0, &pw_entrylo1,
+                               directory_shift, leaf_shift))
         {
         case 0:
             return false;
@@ -795,7 +791,8 @@ static bool page_table_walk_refill(CPUMIPSState *env, vaddr address,
     if (udw > 0) {
         vaddr |= uoffset;
         switch (walk_directory(env, &vaddr, pf_udw, &huge_page, &hgpg_udhit,
-                               &pw_entrylo0, &pw_entrylo1))
+                               &pw_entrylo0, &pw_entrylo1,
+                               directory_shift, leaf_shift))
         {
         case 0:
             return false;
@@ -811,7 +808,8 @@ static bool page_table_walk_refill(CPUMIPSState *env, vaddr address,
     if (mdw > 0) {
         vaddr |= moffset;
         switch (walk_directory(env, &vaddr, pf_mdw, &huge_page, &hgpg_mdhit,
-                               &pw_entrylo0, &pw_entrylo1))
+                               &pw_entrylo0, &pw_entrylo1,
+                               directory_shift, leaf_shift))
         {
         case 0:
             return false;
-- 
2.38.1

[PULL 08/10] target/mips: Avoid shift by negative number in page_table_walk_refill()

[Philippe Mathieu-Daudé]

From: Peter Maydell <peter.maydell@linaro.org>

Coverity points out that in page_table_walk_refill() we can
shift by a negative number, which is undefined behaviour
(CID 1452918, 1452920, 1452922).  We already catch the
negative directory_shift and leaf_shift as being a "bail
out early" case, but not until we've already used them to
calculated some offset values.

The shifts can be negative only if ptew > 1, so make the
bail-out-early check look directly at that, and only
calculate the shift amounts and the offsets based on them
after we have done that check. This allows
us to simplify the expressions used to calculate the
shift amounts, use an unsigned type, and avoids the
undefined behaviour.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
[PMD: Check for ptew > 1, use unsigned type]
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20230717213504.24777-3-philmd@linaro.org>
---
 target/mips/tcg/sysemu/tlb_helper.c | 32 +++++++++++++++--------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/target/mips/tcg/sysemu/tlb_helper.c b/target/mips/tcg/sysemu/tlb_helper.c
index e7be649b02..7dbc2e24c4 100644
--- a/target/mips/tcg/sysemu/tlb_helper.c
+++ b/target/mips/tcg/sysemu/tlb_helper.c
@@ -624,7 +624,7 @@ static uint64_t get_tlb_entry_layout(CPUMIPSState *env, uint64_t entry,
 static int walk_directory(CPUMIPSState *env, uint64_t *vaddr,
         int directory_index, bool *huge_page, bool *hgpg_directory_hit,
         uint64_t *pw_entrylo0, uint64_t *pw_entrylo1,
-        int directory_shift, int leaf_shift)
+        unsigned directory_shift, unsigned leaf_shift)
 {
     int dph = (env->CP0_PWCtl >> CP0PC_DPH) & 0x1;
     int psn = (env->CP0_PWCtl >> CP0PC_PSN) & 0x3F;
@@ -730,21 +730,11 @@ static bool page_table_walk_refill(CPUMIPSState *env, vaddr address,
 
     /* Other HTW configs */
     int hugepg = (env->CP0_PWCtl >> CP0PC_HUGEPG) & 0x1;
-
-    /* HTW Shift values (depend on entry size) */
-    int directory_shift = (ptew > 1) ? -1 :
-            (hugepg && (ptew == 1)) ? native_shift + 1 : native_shift;
-    int leaf_shift = (ptew > 1) ? -1 :
-            (ptew == 1) ? native_shift + 1 : native_shift;
+    unsigned directory_shift, leaf_shift;
 
     /* Offsets into tables */
-    int goffset = gindex << directory_shift;
-    int uoffset = uindex << directory_shift;
-    int moffset = mindex << directory_shift;
-    int ptoffset0 = (ptindex >> 1) << (leaf_shift + 1);
-    int ptoffset1 = ptoffset0 | (1 << (leaf_shift));
-
-    uint32_t leafentry_size = 1 << (leaf_shift + 3);
+    unsigned goffset, uoffset, moffset, ptoffset0, ptoffset1;
+    uint32_t leafentry_size;
 
     /* Starting address - Page Table Base */
     uint64_t vaddr = env->CP0_PWBase;
@@ -766,10 +756,22 @@ static bool page_table_walk_refill(CPUMIPSState *env, vaddr address,
         /* no structure to walk */
         return false;
     }
-    if ((directory_shift == -1) || (leaf_shift == -1)) {
+    if (ptew > 1) {
         return false;
     }
 
+    /* HTW Shift values (depend on entry size) */
+    directory_shift = (hugepg && (ptew == 1)) ? native_shift + 1 : native_shift;
+    leaf_shift = (ptew == 1) ? native_shift + 1 : native_shift;
+
+    goffset = gindex << directory_shift;
+    uoffset = uindex << directory_shift;
+    moffset = mindex << directory_shift;
+    ptoffset0 = (ptindex >> 1) << (leaf_shift + 1);
+    ptoffset1 = ptoffset0 | (1 << (leaf_shift));
+
+    leafentry_size = 1 << (leaf_shift + 3);
+
     /* Global Directory */
     if (gdw > 0) {
         vaddr |= goffset;
-- 
2.38.1

[PULL 09/10] target/sparc: Handle FPRS correctly on big-endian hosts

[Philippe Mathieu-Daudé]

From: Peter Maydell <peter.maydell@linaro.org>

In CPUSparcState we define the fprs field as uint64_t.  However we
then refer to it in translate.c via a TCGv_i32 which we set up with
tcg_global_mem_new_ptr().  This means that on a big-endian host when
the guest does something to writo te the FPRS register this value
ends up in the wrong half of the uint64_t, and the QEMU C code that
refers to env->fprs sees the wrong value.  The effect of this is that
guest code that enables the FPU crashes with spurious FPU Disabled
exceptions.  In particular, this is why
 tests/avocado/machine_sparc64_sun4u.py:Sun4uMachine.test_sparc64_sun4u
times out on an s390 host.

There are multiple ways we could fix this; since there are actually
only three bits in the FPRS register and the code in translate.c
would be a bit painful to convert to dealing with a TCGv_i64, change
the type of the CPU state struct field to match what translate.c is
expecting.

(None of the other fields referenced by the r32[] array in
sparc_tcg_init() have the wrong type.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Message-Id: <20230717103544.637453-1-peter.maydell@linaro.org>
---
 target/sparc/cpu.h     | 2 +-
 target/sparc/cpu.c     | 4 ++--
 target/sparc/machine.c | 3 ++-
 target/sparc/monitor.c | 2 +-
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/target/sparc/cpu.h b/target/sparc/cpu.h
index 95d2d0da71..98044572f2 100644
--- a/target/sparc/cpu.h
+++ b/target/sparc/cpu.h
@@ -521,7 +521,7 @@ struct CPUArchState {
     uint64_t igregs[8]; /* interrupt general registers */
     uint64_t mgregs[8]; /* mmu general registers */
     uint64_t glregs[8 * MAXTL_MAX];
-    uint64_t fprs;
+    uint32_t fprs;
     uint64_t tick_cmpr, stick_cmpr;
     CPUTimer *tick, *stick;
 #define TICK_NPT_MASK        0x8000000000000000ULL
diff --git a/target/sparc/cpu.c b/target/sparc/cpu.c
index e329a7aece..130ab8f578 100644
--- a/target/sparc/cpu.c
+++ b/target/sparc/cpu.c
@@ -673,8 +673,8 @@ static void sparc_cpu_dump_state(CPUState *cs, FILE *f, int flags)
                  "cleanwin: %d cwp: %d\n",
                  env->cansave, env->canrestore, env->otherwin, env->wstate,
                  env->cleanwin, env->nwindows - 1 - env->cwp);
-    qemu_fprintf(f, "fsr: " TARGET_FMT_lx " y: " TARGET_FMT_lx " fprs: "
-                 TARGET_FMT_lx "\n", env->fsr, env->y, env->fprs);
+    qemu_fprintf(f, "fsr: " TARGET_FMT_lx " y: " TARGET_FMT_lx " fprs: %016x\n",
+                 env->fsr, env->y, env->fprs);
 
 #else
     qemu_fprintf(f, "psr: %08x (icc: ", cpu_get_psr(env));
diff --git a/target/sparc/machine.c b/target/sparc/machine.c
index 44b9e7d75d..274e1217df 100644
--- a/target/sparc/machine.c
+++ b/target/sparc/machine.c
@@ -168,7 +168,8 @@ const VMStateDescription vmstate_sparc_cpu = {
         VMSTATE_UINT64_ARRAY(env.bgregs, SPARCCPU, 8),
         VMSTATE_UINT64_ARRAY(env.igregs, SPARCCPU, 8),
         VMSTATE_UINT64_ARRAY(env.mgregs, SPARCCPU, 8),
-        VMSTATE_UINT64(env.fprs, SPARCCPU),
+        VMSTATE_UNUSED(4), /* was unused high half of uint64_t fprs */
+        VMSTATE_UINT32(env.fprs, SPARCCPU),
         VMSTATE_UINT64(env.tick_cmpr, SPARCCPU),
         VMSTATE_UINT64(env.stick_cmpr, SPARCCPU),
         VMSTATE_CPU_TIMER(env.tick, SPARCCPU),
diff --git a/target/sparc/monitor.c b/target/sparc/monitor.c
index 318413686a..73f15aa272 100644
--- a/target/sparc/monitor.c
+++ b/target/sparc/monitor.c
@@ -154,7 +154,7 @@ const MonitorDef monitor_defs[] = {
     { "otherwin", offsetof(CPUSPARCState, otherwin) },
     { "wstate", offsetof(CPUSPARCState, wstate) },
     { "cleanwin", offsetof(CPUSPARCState, cleanwin) },
-    { "fprs", offsetof(CPUSPARCState, fprs) },
+    { "fprs", offsetof(CPUSPARCState, fprs), NULL, MD_I32 },
 #endif
     { NULL },
 };
-- 
2.38.1

[PULL 10/10] target/tricore: Rename tricore_feature

[Philippe Mathieu-Daudé]

From: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>

this name is used by capstone and will lead to a build failure of QEMU,
when capstone is enabled. So we rename it to tricore_has_feature(), to
match has_feature() in translate.c.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1774
Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20230721060605.76636-1-kbastian@mail.uni-paderborn.de>
---
 target/tricore/cpu.h       | 2 +-
 target/tricore/cpu.c       | 8 ++++----
 target/tricore/helper.c    | 4 ++--
 target/tricore/op_helper.c | 4 ++--
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/target/tricore/cpu.h b/target/tricore/cpu.h
index a50b91cc36..3708405be8 100644
--- a/target/tricore/cpu.h
+++ b/target/tricore/cpu.h
@@ -277,7 +277,7 @@ enum tricore_features {
     TRICORE_FEATURE_162,
 };
 
-static inline int tricore_feature(CPUTriCoreState *env, int feature)
+static inline int tricore_has_feature(CPUTriCoreState *env, int feature)
 {
     return (env->features & (1ULL << feature)) != 0;
 }
diff --git a/target/tricore/cpu.c b/target/tricore/cpu.c
index f15169bd1b..133a9ac70e 100644
--- a/target/tricore/cpu.c
+++ b/target/tricore/cpu.c
@@ -104,18 +104,18 @@ static void tricore_cpu_realizefn(DeviceState *dev, Error **errp)
     }
 
     /* Some features automatically imply others */
-    if (tricore_feature(env, TRICORE_FEATURE_162)) {
+    if (tricore_has_feature(env, TRICORE_FEATURE_162)) {
         set_feature(env, TRICORE_FEATURE_161);
     }
 
-    if (tricore_feature(env, TRICORE_FEATURE_161)) {
+    if (tricore_has_feature(env, TRICORE_FEATURE_161)) {
         set_feature(env, TRICORE_FEATURE_16);
     }
 
-    if (tricore_feature(env, TRICORE_FEATURE_16)) {
+    if (tricore_has_feature(env, TRICORE_FEATURE_16)) {
         set_feature(env, TRICORE_FEATURE_131);
     }
-    if (tricore_feature(env, TRICORE_FEATURE_131)) {
+    if (tricore_has_feature(env, TRICORE_FEATURE_131)) {
         set_feature(env, TRICORE_FEATURE_13);
     }
     cpu_reset(cs);
diff --git a/target/tricore/helper.c b/target/tricore/helper.c
index 951024d491..731a6e9cb6 100644
--- a/target/tricore/helper.c
+++ b/target/tricore/helper.c
@@ -155,7 +155,7 @@ void psw_write(CPUTriCoreState *env, uint32_t val)
 #define FIELD_GETTER_WITH_FEATURE(NAME, REG, FIELD, FEATURE)     \
 uint32_t NAME(CPUTriCoreState *env)                             \
 {                                                                \
-    if (tricore_feature(env, TRICORE_FEATURE_##FEATURE)) {       \
+    if (tricore_has_feature(env, TRICORE_FEATURE_##FEATURE)) {   \
         return FIELD_EX32(env->REG, REG, FIELD ## _ ## FEATURE); \
     }                                                            \
     return FIELD_EX32(env->REG, REG, FIELD ## _13);              \
@@ -170,7 +170,7 @@ uint32_t NAME(CPUTriCoreState *env)         \
 #define FIELD_SETTER_WITH_FEATURE(NAME, REG, FIELD, FEATURE)              \
 void NAME(CPUTriCoreState *env, uint32_t val)                            \
 {                                                                         \
-    if (tricore_feature(env, TRICORE_FEATURE_##FEATURE)) {                \
+    if (tricore_has_feature(env, TRICORE_FEATURE_##FEATURE)) {            \
         env->REG = FIELD_DP32(env->REG, REG, FIELD ## _ ## FEATURE, val); \
     }                                                                     \
     env->REG = FIELD_DP32(env->REG, REG, FIELD ## _13, val);              \
diff --git a/target/tricore/op_helper.c b/target/tricore/op_helper.c
index 821a4b67cb..89be1ed648 100644
--- a/target/tricore/op_helper.c
+++ b/target/tricore/op_helper.c
@@ -2584,7 +2584,7 @@ void helper_ret(CPUTriCoreState *env)
     /* PCXI = new_PCXI; */
     env->PCXI = new_PCXI;
 
-    if (tricore_feature(env, TRICORE_FEATURE_131)) {
+    if (tricore_has_feature(env, TRICORE_FEATURE_131)) {
         /* PSW = {new_PSW[31:26], PSW[25:24], new_PSW[23:0]}; */
         psw_write(env, (new_PSW & ~(0x3000000)) + (psw & (0x3000000)));
     } else { /* TRICORE_FEATURE_13 only */
@@ -2695,7 +2695,7 @@ void helper_rfm(CPUTriCoreState *env)
     env->gpr_a[10] = cpu_ldl_data(env, env->DCX+8);
     env->gpr_a[11] = cpu_ldl_data(env, env->DCX+12);
 
-    if (tricore_feature(env, TRICORE_FEATURE_131)) {
+    if (tricore_has_feature(env, TRICORE_FEATURE_131)) {
         env->DBGTCR = 0;
     }
 }
-- 
2.38.1

Re: [PULL 10/10] target/tricore: Rename tricore_feature

[Bastian Koppelmann]

Hi Phil,

On Tue, Jul 25, 2023 at 04:58:29PM +0200, Philippe Mathieu-Daudé wrote:
> From: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> 
> this name is used by capstone and will lead to a build failure of QEMU,
> when capstone is enabled. So we rename it to tricore_has_feature(), to
> match has_feature() in translate.c.
> 
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1774
> Signed-off-by: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Message-Id: <20230721060605.76636-1-kbastian@mail.uni-paderborn.de>
> ---
>  target/tricore/cpu.h       | 2 +-
>  target/tricore/cpu.c       | 8 ++++----
>  target/tricore/helper.c    | 4 ++--
>  target/tricore/op_helper.c | 4 ++--
>  4 files changed, 9 insertions(+), 9 deletions(-)

+CC: mjt@tls.msk.ru

Michael Tokarev has already picked it up. See https://lore.kernel.org/qemu-devel/20230725145829.37782-11-philmd@linaro.org/T/#u

Cheers,
Bastian

Re: [PULL 10/10] target/tricore: Rename tricore_feature

[Michael Tokarev]

25.07.2023 22:11, Bastian Koppelmann wrote:
...
> Michael Tokarev has already picked it up. See https://lore.kernel.org/qemu-devel/20230725145829.37782-11-philmd@linaro.org/T/#u

I noticed that too, we did it almost at the same time.

But there's nothing wrong with that.  It doesn't matter
how a particular change enters the tree. When pulling
the same change for the 2nd time, git will notice the
change is already there and do nothing.

/mjt

Re: [PULL 00/10] Misc fixes for 2023-07-25

[Peter Maydell]

On Tue, 25 Jul 2023 at 15:58, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> The following changes since commit 3ee44ec72753ec0ff05ad1569dfa609203d722b2:
>
>   Merge tag 'pull-request-2023-07-24' of https://gitlab.com/thuth/qemu into staging (2023-07-24 18:06:36 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/philmd/qemu.git tags/misc-fixes-20230725
>
> for you to fetch changes up to f8cfdd2038c1823301e6df753242e465b1dc8539:
>
>   target/tricore: Rename tricore_feature (2023-07-25 14:42:00 +0200)
>
> ----------------------------------------------------------------
> Misc patches queue
>
> hw/sd/sdhci: Default I/O ops to little endian
> hw/mips/loongson3-virt: Only use default USB if available
> hw/char/escc: Implement loopback mode to allow self-testing
> target/mips: Avoid overruns and shifts by negative number
> target/sparc: Handle FPRS correctly on big-endian hosts
> target/tricore: Rename tricore_feature to avoid clash with libcapstone
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/8.1
for any user-visible changes.

-- PMM