[PULL 0/8] ppc-for-9.0-3 queue

[PULL 0/8] ppc-for-9.0-3 queue

[Nicholas Piggin]

The following changes since commit 5012e522aca161be5c141596c66e5cc6082538a9:

  Update version for v9.0.0-rc1 release (2024-03-26 19:46:55 +0000)

are available in the Git repository at:

  https://gitlab.com/npiggin/qemu.git tags/pull-ppc-for-9.0-3-20240331

for you to fetch changes up to b07a5bb736ca08d55cc3ada8ca309943b55d4b70:

  tests/avocado: ppc_hv_tests.py set alpine time before setup-alpine (2024-03-30 18:50:26 +1000)

----------------------------------------------------------------
* Various fixes for recent regressions and new code.

----------------------------------------------------------------
Benjamin Gray (2):
      target/ppc: Restore [H]DEXCR to 64-bits
      target/ppc: Fix GDB register indexing on secondary CPUs

Harsh Prateek Bora (1):
      spapr: nested: use bitwise NOT operator for flags check

Nicholas Piggin (3):
      target/ppc: Do not clear MSR[ME] on MCE interrupts to supervisor
      tests/avocado: Fix ppc_hv_tests.py xorriso dependency guard
      tests/avocado: ppc_hv_tests.py set alpine time before setup-alpine

Philippe Mathieu-Daudé (2):
      hw/ppc/spapr: Include missing 'sysemu/tcg.h' header
      target/ppc/mmu-radix64: Use correct string format in walk_tree()

 hw/ppc/spapr.c                |  1 +
 hw/ppc/spapr_nested.c         |  2 +-
 target/ppc/cpu_init.c         |  4 ++--
 target/ppc/excp_helper.c      |  5 +++--
 target/ppc/gdbstub.c          | 31 ++++++++++++++++++++-----------
 target/ppc/mmu-radix64.c      |  8 ++++----
 tests/avocado/ppc_hv_tests.py |  6 +++++-
 7 files changed, 36 insertions(+), 21 deletions(-)

[PULL 1/8] spapr: nested: use bitwise NOT operator for flags check

[Nicholas Piggin]

From: Harsh Prateek Bora <harshpb@linux.ibm.com>

Check for flag bit in H_GUEST_GETSET_STATE_FLAG_GUEST_WIDE need to use
bitwise NOT operator to ensure no other flag bits are set.

Resolves: Coverity CID 1540008
Resolves: Coverity CID 1540009
Reported-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off by: Harsh Prateek Bora <harshpb@linux.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/ppc/spapr_nested.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/spapr_nested.c b/hw/ppc/spapr_nested.c
index 936659b4c0..c02785756c 100644
--- a/hw/ppc/spapr_nested.c
+++ b/hw/ppc/spapr_nested.c
@@ -1511,7 +1511,7 @@ static target_ulong h_guest_getset_state(PowerPCCPU *cpu,
     if (flags & H_GUEST_GETSET_STATE_FLAG_GUEST_WIDE) {
         gsr.flags |= GUEST_STATE_REQUEST_GUEST_WIDE;
     }
-    if (flags & !H_GUEST_GETSET_STATE_FLAG_GUEST_WIDE) {
+    if (flags & ~H_GUEST_GETSET_STATE_FLAG_GUEST_WIDE) {
         return H_PARAMETER; /* flag not supported yet */
     }
 
-- 
2.43.0

[PULL 2/8] hw/ppc/spapr: Include missing 'sysemu/tcg.h' header

[Nicholas Piggin]

From: Philippe Mathieu-Daudé <philmd@linaro.org>

"sysemu/tcg.h" declares tcg_enabled(), and is implicitly included.
Include it explicitly to avoid the following error when refactoring
headers:

  hw/ppc/spapr.c:2612:9: error: call to undeclared function 'tcg_enabled'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    if (tcg_enabled()) {
        ^

Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/ppc/spapr.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index c417f9dd52..e9bc97fee0 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -35,6 +35,7 @@
 #include "sysemu/sysemu.h"
 #include "sysemu/hostmem.h"
 #include "sysemu/numa.h"
+#include "sysemu/tcg.h"
 #include "sysemu/qtest.h"
 #include "sysemu/reset.h"
 #include "sysemu/runstate.h"
-- 
2.43.0

[PULL 3/8] target/ppc/mmu-radix64: Use correct string format in walk_tree()

[Nicholas Piggin]

From: Philippe Mathieu-Daudé <philmd@linaro.org>

'mask', 'nlb' and 'base_addr' are all uin64_t types.
Use the corresponding PRIx64 format.

Fixes: d2066bc50d ("target/ppc: Check page dir/table base alignment")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/mmu-radix64.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
index 5823e039e6..690dff7a49 100644
--- a/target/ppc/mmu-radix64.c
+++ b/target/ppc/mmu-radix64.c
@@ -300,8 +300,8 @@ static int ppc_radix64_next_level(AddressSpace *as, vaddr eaddr,
 
         if (nlb & mask) {
             qemu_log_mask(LOG_GUEST_ERROR,
-                "%s: misaligned page dir/table base: 0x"TARGET_FMT_lx
-                " page dir size: 0x"TARGET_FMT_lx"\n",
+                "%s: misaligned page dir/table base: 0x%" PRIx64
+                " page dir size: 0x%" PRIx64 "\n",
                 __func__, nlb, mask + 1);
             nlb &= ~mask;
         }
@@ -324,8 +324,8 @@ static int ppc_radix64_walk_tree(AddressSpace *as, vaddr eaddr,
 
     if (base_addr & mask) {
         qemu_log_mask(LOG_GUEST_ERROR,
-            "%s: misaligned page dir base: 0x"TARGET_FMT_lx
-            " page dir size: 0x"TARGET_FMT_lx"\n",
+            "%s: misaligned page dir base: 0x%" PRIx64
+            " page dir size: 0x%" PRIx64 "\n",
             __func__, base_addr, mask + 1);
         base_addr &= ~mask;
     }
-- 
2.43.0

[PULL 4/8] target/ppc: Restore [H]DEXCR to 64-bits

[Nicholas Piggin]

From: Benjamin Gray <bgray@linux.ibm.com>

The DEXCR emulation was recently changed to a 32-bit register, possibly
because it does have a 32-bit read-only view. It is a full 64-bit
SPR though, so use the corresponding 64-bit write functions.

Fixes: fbda88f7abdee ("target/ppc: Fix width of some 32-bit SPRs")
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/cpu_init.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 7e65f08147..22fdea093b 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -5820,7 +5820,7 @@ static void register_power10_dexcr_sprs(CPUPPCState *env)
 {
     spr_register(env, SPR_DEXCR, "DEXCR",
             SPR_NOACCESS, SPR_NOACCESS,
-            &spr_read_generic, &spr_write_generic32,
+            &spr_read_generic, &spr_write_generic,
             0);
 
     spr_register(env, SPR_UDEXCR, "UDEXCR",
@@ -5831,7 +5831,7 @@ static void register_power10_dexcr_sprs(CPUPPCState *env)
     spr_register_hv(env, SPR_HDEXCR, "HDEXCR",
             SPR_NOACCESS, SPR_NOACCESS,
             SPR_NOACCESS, SPR_NOACCESS,
-            &spr_read_generic, &spr_write_generic32,
+            &spr_read_generic, &spr_write_generic,
             0);
 
     spr_register(env, SPR_UHDEXCR, "UHDEXCR",
-- 
2.43.0

[PULL 5/8] target/ppc: Fix GDB register indexing on secondary CPUs

[Nicholas Piggin]

From: Benjamin Gray <bgray@linux.ibm.com>

The GDB server protocol assigns an arbitrary numbering of the SPRs.
We track this correspondence on each SPR with gdb_id, using it to
resolve any SPR requests GDB makes.

Early on we generate an XML representation of the SPRs to give GDB,
including this numbering. However the XML is cached globally, and we
skip setting the SPR gdb_id values on subsequent threads if we detect
it is cached. This causes QEMU to fail to resolve SPR requests against
secondary CPUs because it cannot find the matching gdb_id value on that
thread's SPRs.

This is a minimal fix to first assign the gdb_id values, then return
early if the XML is cached. Otherwise we generate the XML using the
now already initialised gdb_id values.

Fixes: 1b53948ff8f7 ("target/ppc: Use GDBFeature for dynamic XML")
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/gdbstub.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/target/ppc/gdbstub.c b/target/ppc/gdbstub.c
index 3f1e61bdb7..3b28d4e21c 100644
--- a/target/ppc/gdbstub.c
+++ b/target/ppc/gdbstub.c
@@ -305,14 +305,6 @@ static void gdb_gen_spr_feature(CPUState *cs)
     unsigned int num_regs = 0;
     int i;
 
-    if (pcc->gdb_spr.xml) {
-        return;
-    }
-
-    gdb_feature_builder_init(&builder, &pcc->gdb_spr,
-                             "org.qemu.power.spr", "power-spr.xml",
-                             cs->gdb_num_regs);
-
     for (i = 0; i < ARRAY_SIZE(env->spr_cb); i++) {
         ppc_spr_t *spr = &env->spr_cb[i];
 
@@ -320,9 +312,6 @@ static void gdb_gen_spr_feature(CPUState *cs)
             continue;
         }
 
-        gdb_feature_builder_append_reg(&builder, g_ascii_strdown(spr->name, -1),
-                                       TARGET_LONG_BITS, num_regs,
-                                       "int", "spr");
         /*
          * GDB identifies registers based on the order they are
          * presented in the XML. These ids will not match QEMU's
@@ -335,6 +324,26 @@ static void gdb_gen_spr_feature(CPUState *cs)
         num_regs++;
     }
 
+    if (pcc->gdb_spr.xml) {
+        return;
+    }
+
+    gdb_feature_builder_init(&builder, &pcc->gdb_spr,
+                             "org.qemu.power.spr", "power-spr.xml",
+                             cs->gdb_num_regs);
+
+    for (i = 0; i < ARRAY_SIZE(env->spr_cb); i++) {
+        ppc_spr_t *spr = &env->spr_cb[i];
+
+        if (!spr->name) {
+            continue;
+        }
+
+        gdb_feature_builder_append_reg(&builder, g_ascii_strdown(spr->name, -1),
+                                       TARGET_LONG_BITS, spr->gdb_id,
+                                       "int", "spr");
+    }
+
     gdb_feature_builder_end(&builder);
 }
 #endif
-- 
2.43.0

[PULL 6/8] target/ppc: Do not clear MSR[ME] on MCE interrupts to supervisor

[Nicholas Piggin]

Hardware clears the MSR[ME] bit when delivering a machine check
interrupt, so that is what QEMU does.

The spapr environment runs in supervisor mode though, and receives
machine check interrupts after they are processed by the hypervisor,
and MSR[ME] must always be enabled in supervisor mode (otherwise it
could checkstop the system). So MSR[ME] must not be cleared when
delivering machine checks to the supervisor.

The fix to prevent supervisor mode from modifying MSR[ME] also
prevented it from re-enabling the incorrectly cleared MSR[ME] bit
when returning from handling the interrupt. Before that fix, the
problem was not very noticable with well-behaved code. So the
Fixes tag is not strictly correct, but practically they go together.

Found by kvm-unit-tests machine check tests (not yet upstream).

Fixes: 678b6f1af75ef ("target/ppc: Prevent supervisor from modifying MSR[ME]")
Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/excp_helper.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 80f584f933..674c05a2ce 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1345,9 +1345,10 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
              * clear (e.g., see FWNMI in PAPR).
              */
             new_msr |= (target_ulong)MSR_HVB;
+
+            /* HV machine check exceptions don't have ME set */
+            new_msr &= ~((target_ulong)1 << MSR_ME);
         }
-        /* machine check exceptions don't have ME set */
-        new_msr &= ~((target_ulong)1 << MSR_ME);
 
         msr |= env->error_code;
         break;
-- 
2.43.0

[PULL 7/8] tests/avocado: Fix ppc_hv_tests.py xorriso dependency guard

[Nicholas Piggin]

For some reason the skipIf missing_deps() check fails to skip the test
if it comes after the skipUnless lines, causing an error running on
systems without xorriso.

Avocado implements skipUnless is just an inverted skipIf, so it's not
clear what the bug is or why this fixes it. For now it's enough to
get things working.

Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2246
Fixes: c9cb496710758 ("tests/avocado: ppc add hypervisor tests")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 tests/avocado/ppc_hv_tests.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/avocado/ppc_hv_tests.py b/tests/avocado/ppc_hv_tests.py
index 5080358e25..2c8ddd9257 100644
--- a/tests/avocado/ppc_hv_tests.py
+++ b/tests/avocado/ppc_hv_tests.py
@@ -42,10 +42,11 @@ def missing_deps():
 # QEMU packages are downloaded and installed on each test. That's not a
 # large download, but it may be more polite to create qcow2 image with
 # QEMU already installed and use that.
+# XXX: The order of these tests seems to matter, see git blame.
+@skipIf(missing_deps(), 'dependencies (%s) not installed' % ','.join(deps))
 @skipUnless(os.getenv('QEMU_TEST_FLAKY_TESTS'), 'Test sometimes gets stuck due to console handling problem')
 @skipUnless(os.getenv('AVOCADO_ALLOW_LARGE_STORAGE'), 'storage limited')
 @skipUnless(os.getenv('SPEED') == 'slow', 'runtime limited')
-@skipIf(missing_deps(), 'dependencies (%s) not installed' % ','.join(deps))
 class HypervisorTest(QemuSystemTest):
 
     timeout = 1000
-- 
2.43.0

[PULL 8/8] tests/avocado: ppc_hv_tests.py set alpine time before setup-alpine

[Nicholas Piggin]

If the time is wrong, setup-alpine SSL certificate checks can fail.
setup-alpine is used to bring up the network, but it doesn't seem
to to set NTP time before the failing SSL checks. This test has
recently started failing presumably because the default time has
now fallen too far behind.

Fix this by setting time from the host time before running setup-alpine.

Fixes: c9cb496710758 ("tests/avocado: ppc add hypervisor tests")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 tests/avocado/ppc_hv_tests.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/avocado/ppc_hv_tests.py b/tests/avocado/ppc_hv_tests.py
index 2c8ddd9257..bf8822bb97 100644
--- a/tests/avocado/ppc_hv_tests.py
+++ b/tests/avocado/ppc_hv_tests.py
@@ -14,6 +14,7 @@
 import os
 import time
 import subprocess
+from datetime import datetime
 
 deps = ["xorriso"] # dependent tools needed in the test setup/box.
 
@@ -107,6 +108,8 @@ def do_start_alpine(self):
         exec_command(self, 'root')
         wait_for_console_pattern(self, 'localhost login:')
         wait_for_console_pattern(self, 'You may change this message by editing /etc/motd.')
+        # If the time is wrong, SSL certificates can fail.
+        exec_command(self, 'date -s "' + datetime.utcnow().strftime('%Y-%m-%d %H:%M:%S' + '"'))
         exec_command(self, 'setup-alpine -qe')
         wait_for_console_pattern(self, 'Updating repository indexes... done.')
 
-- 
2.43.0

Re: [PULL 0/8] ppc-for-9.0-3 queue

[Peter Maydell]

On Sun, 31 Mar 2024 at 08:34, Nicholas Piggin <npiggin@gmail.com> wrote:
>
> The following changes since commit 5012e522aca161be5c141596c66e5cc6082538a9:
>
>   Update version for v9.0.0-rc1 release (2024-03-26 19:46:55 +0000)
>
> are available in the Git repository at:
>
>   https://gitlab.com/npiggin/qemu.git tags/pull-ppc-for-9.0-3-20240331
>
> for you to fetch changes up to b07a5bb736ca08d55cc3ada8ca309943b55d4b70:
>
>   tests/avocado: ppc_hv_tests.py set alpine time before setup-alpine (2024-03-30 18:50:26 +1000)
>
> ----------------------------------------------------------------
> * Various fixes for recent regressions and new code.
>
> ----------------------------------------------------------------


Applied, thanks.

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

-- PMM