[PATCH 0/4] ppc/pnv: SMT support for powernv

[PATCH 0/4] ppc/pnv: SMT support for powernv

[Nicholas Piggin]

These patches implement enough to install a distro, boot, run SMP KVM
guests with libvirt with good performance using MTTCG (as reported by
Cedric).

There are a few more SPRs that need to be done, and per-LPAR SPRs are
mostly not annotated yet so it can't run in 1 LPAR mode. But those can
be added in time, it will take a bit of time to get everything exactly
as hardware does so I consider this good enough to run common
software usefully.

Thanks Joel and Cedric for reviews and testing.

Since RFC:
- Rebased against ppc-next (no conflicts vs upstream anyway).
- Add patch 4 avocado boot test with SMT, as was added with pseries SMT.
- Renamed POWERPC_FLAG_1LPAR to POWERPC_FLAG_SMT_1LPAR since it implies
  SMT.
- Fixed typos, patch 1, 3 changelogs improvement (hopefully).

Thanks,
Nick

Nicholas Piggin (4):
  target/ppc: Add LPAR-per-core vs per-thread mode flag
  target/ppc: SMT support for the HID SPR
  ppc/pnv: SMT support for powernv
  tests/avocado: Add powernv machine test script

 hw/ppc/pnv.c                 | 12 +++++
 hw/ppc/pnv_core.c            | 13 +++---
 hw/ppc/spapr_cpu_core.c      |  2 +
 target/ppc/cpu.h             |  3 ++
 target/ppc/cpu_init.c        | 14 +++++-
 target/ppc/helper.h          |  1 +
 target/ppc/misc_helper.c     | 21 +++++++++
 target/ppc/spr_common.h      |  1 +
 target/ppc/translate.c       | 32 ++++++++++++--
 tests/avocado/ppc_powernv.py | 86 ++++++++++++++++++++++++++++++++++++
 10 files changed, 173 insertions(+), 12 deletions(-)
 create mode 100644 tests/avocado/ppc_powernv.py

-- 
2.40.1

[PATCH 1/4] target/ppc: Add LPAR-per-core vs per-thread mode flag

[Nicholas Piggin]

The Power ISA has the concept of sub-processors:

  Hardware is allowed to sub-divide a multi-threaded processor into
  "sub-processors" that appear to privileged programs as multi-threaded
  processors with fewer threads.

POWER9 and POWER10 have two modes, either every thread is a
sub-processor or all threads appear as one multi-threaded processor. In
the user manuals these are known as "LPAR per thread" / "Thread LPAR",
and "LPAR per core" / "1 LPAR", respectively.

The practical difference is: in thread LPAR mode, non-hypervisor SPRs
are not shared between threads and msgsndp can not be used to message
siblings. In 1 LPAR mode, some SPRs are shared and msgsndp is usable.
Thrad LPAR allows multiple partitions to run concurrently on the same
core, and is a requirement for KVM to run on POWER9/10 (which does not
gang-schedule an LPAR on all threads of a core like POWER8 KVM).

Traditionally, SMT in PAPR environments including PowerVM and the
pseries QEMU machine with KVM acceleration behaves as in 1 LPAR mode.
In OPAL systems, Thread LPAR is used. When adding SMT to the powernv
machine, it is therefore preferable to emulate Thread LPAR.

To account for this difference between pseries and powernv, an LPAR mode
flag is added such that SPRs can be implemented as per-LPAR shared, and
that becomes either per-thread or per-core depending on the flag.

Reviewed-by: Joel Stanley <joel@jms.id.au>
Tested-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/ppc/spapr_cpu_core.c |  2 ++
 target/ppc/cpu.h        |  3 +++
 target/ppc/cpu_init.c   | 12 ++++++++++++
 target/ppc/translate.c  | 16 +++++++++++++---
 4 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index a4e3c2fadd..b482d9754a 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -270,6 +270,8 @@ static bool spapr_realize_vcpu(PowerPCCPU *cpu, SpaprMachineState *spapr,
     env->spr_cb[SPR_PIR].default_value = cs->cpu_index;
     env->spr_cb[SPR_TIR].default_value = thread_index;
 
+    cpu_ppc_set_1lpar(cpu);
+
     /* Set time-base frequency to 512 MHz. vhyp must be set first. */
     cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ);
 
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index af12c93ebc..951f73d89d 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -674,6 +674,8 @@ enum {
     POWERPC_FLAG_SCV      = 0x00200000,
     /* Has >1 thread per core                                                */
     POWERPC_FLAG_SMT      = 0x00400000,
+    /* Using "LPAR per core" mode  (as opposed to per-thread)                */
+    POWERPC_FLAG_SMT_1LPAR= 0x00800000,
 };
 
 /*
@@ -1437,6 +1439,7 @@ void store_booke_tsr(CPUPPCState *env, target_ulong val);
 void ppc_tlb_invalidate_all(CPUPPCState *env);
 void ppc_tlb_invalidate_one(CPUPPCState *env, target_ulong addr);
 void cpu_ppc_set_vhyp(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp);
+void cpu_ppc_set_1lpar(PowerPCCPU *cpu);
 int ppcmas_tlb_check(CPUPPCState *env, ppcmas_tlb_t *tlb, hwaddr *raddrp,
                      target_ulong address, uint32_t pid);
 int ppcemb_tlb_search(CPUPPCState *env, target_ulong address, uint32_t pid);
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 5f4969664e..905a59aea9 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -6629,6 +6629,18 @@ void cpu_ppc_set_vhyp(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp)
     env->msr_mask &= ~MSR_HVB;
 }
 
+void cpu_ppc_set_1lpar(PowerPCCPU *cpu)
+{
+    CPUPPCState *env = &cpu->env;
+
+    /*
+     * pseries SMT means "LPAR per core" mode, e.g., msgsndp is usable
+     * between threads.
+     */
+    if (env->flags & POWERPC_FLAG_SMT) {
+        env->flags |= POWERPC_FLAG_SMT_1LPAR;
+    }
+}
 #endif /* !defined(CONFIG_USER_ONLY) */
 
 #endif /* defined(TARGET_PPC64) */
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index db0ba49bdc..10598cde40 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -256,6 +256,16 @@ static inline bool gen_serialize_core(DisasContext *ctx)
 }
 #endif
 
+static inline bool gen_serialize_core_lpar(DisasContext *ctx)
+{
+    /* 1LPAR implies SMT */
+    if (ctx->flags & POWERPC_FLAG_SMT_1LPAR) {
+        return gen_serialize(ctx);
+    }
+
+    return true;
+}
+
 /* SPR load/store helpers */
 static inline void gen_load_spr(TCGv t, int reg)
 {
@@ -451,7 +461,7 @@ static void spr_write_CTRL_ST(DisasContext *ctx, int sprn, int gprn)
 
 void spr_write_CTRL(DisasContext *ctx, int sprn, int gprn)
 {
-    if (!(ctx->flags & POWERPC_FLAG_SMT)) {
+    if (!(ctx->flags & POWERPC_FLAG_SMT_1LPAR)) {
         spr_write_CTRL_ST(ctx, sprn, gprn);
         goto out;
     }
@@ -815,7 +825,7 @@ void spr_write_pcr(DisasContext *ctx, int sprn, int gprn)
 /* DPDES */
 void spr_read_dpdes(DisasContext *ctx, int gprn, int sprn)
 {
-    if (!gen_serialize_core(ctx)) {
+    if (!gen_serialize_core_lpar(ctx)) {
         return;
     }
 
@@ -824,7 +834,7 @@ void spr_read_dpdes(DisasContext *ctx, int gprn, int sprn)
 
 void spr_write_dpdes(DisasContext *ctx, int sprn, int gprn)
 {
-    if (!gen_serialize_core(ctx)) {
+    if (!gen_serialize_core_lpar(ctx)) {
         return;
     }
 
-- 
2.40.1

[PATCH 2/4] target/ppc: SMT support for the HID SPR

[Nicholas Piggin]

HID is a per-core shared register, skiboot sets this (e.g., setting
HILE) on one thread and that must affect all threads of the core.

Tested-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/cpu_init.c    |  2 +-
 target/ppc/helper.h      |  1 +
 target/ppc/misc_helper.c | 21 +++++++++++++++++++++
 target/ppc/spr_common.h  |  1 +
 target/ppc/translate.c   | 16 ++++++++++++++++
 5 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 905a59aea9..720aad9e05 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -5638,7 +5638,7 @@ static void register_power_common_book4_sprs(CPUPPCState *env)
     spr_register_hv(env, SPR_HID0, "HID0",
                  SPR_NOACCESS, SPR_NOACCESS,
                  SPR_NOACCESS, SPR_NOACCESS,
-                 &spr_read_generic, &spr_write_generic,
+                 &spr_read_generic, &spr_core_write_generic,
                  0x00000000);
     spr_register_hv(env, SPR_TSCR, "TSCR",
                  SPR_NOACCESS, SPR_NOACCESS,
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 828f7844c8..abec6fe341 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -704,6 +704,7 @@ DEF_HELPER_3(store_dcr, void, env, tl, tl)
 
 DEF_HELPER_2(load_dump_spr, void, env, i32)
 DEF_HELPER_2(store_dump_spr, void, env, i32)
+DEF_HELPER_3(spr_core_write_generic, void, env, i32, tl)
 DEF_HELPER_3(spr_write_CTRL, void, env, i32, tl)
 
 DEF_HELPER_4(fscr_facility_check, void, env, i32, i32, i32)
diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
index 1f1af21f33..0da335472e 100644
--- a/target/ppc/misc_helper.c
+++ b/target/ppc/misc_helper.c
@@ -43,6 +43,27 @@ void helper_store_dump_spr(CPUPPCState *env, uint32_t sprn)
              env->spr[sprn]);
 }
 
+void helper_spr_core_write_generic(CPUPPCState *env, uint32_t sprn,
+                                   target_ulong val)
+{
+    CPUState *cs = env_cpu(env);
+    CPUState *ccs;
+    uint32_t nr_threads = cs->nr_threads;
+    uint32_t core_id = env->spr[SPR_PIR] & ~(nr_threads - 1);
+
+    assert(core_id == env->spr[SPR_PIR] - env->spr[SPR_TIR]);
+
+    if (nr_threads == 1) {
+        env->spr[sprn] = val;
+        return;
+    }
+
+    THREAD_SIBLING_FOREACH(cs, ccs) {
+        CPUPPCState *cenv = &POWERPC_CPU(ccs)->env;
+        cenv->spr[sprn] = val;
+    }
+}
+
 void helper_spr_write_CTRL(CPUPPCState *env, uint32_t sprn,
                            target_ulong val)
 {
diff --git a/target/ppc/spr_common.h b/target/ppc/spr_common.h
index fbf52123b5..5995070eaf 100644
--- a/target/ppc/spr_common.h
+++ b/target/ppc/spr_common.h
@@ -82,6 +82,7 @@ void spr_noaccess(DisasContext *ctx, int gprn, int sprn);
 void spr_read_generic(DisasContext *ctx, int gprn, int sprn);
 void spr_write_generic(DisasContext *ctx, int sprn, int gprn);
 void spr_write_generic32(DisasContext *ctx, int sprn, int gprn);
+void spr_core_write_generic(DisasContext *ctx, int sprn, int gprn);
 void spr_write_MMCR0(DisasContext *ctx, int sprn, int gprn);
 void spr_write_MMCR1(DisasContext *ctx, int sprn, int gprn);
 void spr_write_PMC(DisasContext *ctx, int sprn, int gprn);
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 10598cde40..07c491df62 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -448,6 +448,22 @@ void spr_write_generic32(DisasContext *ctx, int sprn, int gprn)
 #endif
 }
 
+void spr_core_write_generic(DisasContext *ctx, int sprn, int gprn)
+{
+    if (!(ctx->flags & POWERPC_FLAG_SMT)) {
+        spr_write_generic(ctx, sprn, gprn);
+        return;
+    }
+
+    if (!gen_serialize(ctx)) {
+        return;
+    }
+
+    gen_helper_spr_core_write_generic(cpu_env, tcg_constant_i32(sprn),
+                                      cpu_gpr[gprn]);
+    spr_store_dump_spr(sprn);
+}
+
 static void spr_write_CTRL_ST(DisasContext *ctx, int sprn, int gprn)
 {
     /* This does not implement >1 thread */
-- 
2.40.1

[PATCH 3/4] ppc/pnv: SMT support for powernv

[Nicholas Piggin]

Set the TIR default value with the SMT thread index, and place some
standard limits on SMT configurations. Now powernv is able to boot
skiboot and Linux with a SMT topology, including booting a KVM guest.

There are several SPRs and other features (e.g., broadcast msgsnd)
that are not implemented, but not used by OPAL or Linux and can be
added incrementally.

Reviewed-by: Cédric Le Goater <clg@kaod.org>
Tested-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/ppc/pnv.c      | 12 ++++++++++++
 hw/ppc/pnv_core.c | 13 +++++--------
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index fc083173f3..f599ccad1d 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -887,6 +887,18 @@ static void pnv_init(MachineState *machine)
 
     pnv->num_chips =
         machine->smp.max_cpus / (machine->smp.cores * machine->smp.threads);
+
+    if (machine->smp.threads > 8) {
+        error_report("Cannot support more than 8 threads/core "
+                     "on a powernv machine");
+        exit(1);
+    }
+    if (!is_power_of_2(machine->smp.threads)) {
+        error_report("Cannot support %d threads/core on a powernv"
+                     "machine because it must be a power of 2",
+                     machine->smp.threads);
+        exit(1);
+    }
     /*
      * TODO: should we decide on how many chips we can create based
      * on #cores and Venice vs. Murano vs. Naples chip type etc...,
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index 0bc3ad41c8..acd83caee8 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -167,12 +167,13 @@ static const MemoryRegionOps pnv_core_power9_xscom_ops = {
     .endianness = DEVICE_BIG_ENDIAN,
 };
 
-static void pnv_core_cpu_realize(PnvCore *pc, PowerPCCPU *cpu, Error **errp)
+static void pnv_core_cpu_realize(PnvCore *pc, PowerPCCPU *cpu, Error **errp,
+                                 int thread_index)
 {
     CPUPPCState *env = &cpu->env;
     int core_pir;
-    int thread_index = 0; /* TODO: TCG supports only one thread */
     ppc_spr_t *pir = &env->spr_cb[SPR_PIR];
+    ppc_spr_t *tir = &env->spr_cb[SPR_TIR];
     Error *local_err = NULL;
     PnvChipClass *pcc = PNV_CHIP_GET_CLASS(pc->chip);
 
@@ -188,11 +189,7 @@ static void pnv_core_cpu_realize(PnvCore *pc, PowerPCCPU *cpu, Error **errp)
 
     core_pir = object_property_get_uint(OBJECT(pc), "pir", &error_abort);
 
-    /*
-     * The PIR of a thread is the core PIR + the thread index. We will
-     * need to find a way to get the thread index when TCG supports
-     * more than 1. We could use the object name ?
-     */
+    tir->default_value = thread_index;
     pir->default_value = core_pir + thread_index;
 
     /* Set time-base frequency to 512 MHz */
@@ -241,7 +238,7 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
     }
 
     for (j = 0; j < cc->nr_threads; j++) {
-        pnv_core_cpu_realize(pc, pc->threads[j], &local_err);
+        pnv_core_cpu_realize(pc, pc->threads[j], &local_err, j);
         if (local_err) {
             goto err;
         }
-- 
2.40.1

[PATCH 4/4] tests/avocado: Add powernv machine test script

[Nicholas Piggin]

This copies ppc_pseries.py to start a set of powernv tests, including
a Linux boot test for the newly added SMT mode.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 tests/avocado/ppc_powernv.py | 86 ++++++++++++++++++++++++++++++++++++
 1 file changed, 86 insertions(+)
 create mode 100644 tests/avocado/ppc_powernv.py

diff --git a/tests/avocado/ppc_powernv.py b/tests/avocado/ppc_powernv.py
new file mode 100644
index 0000000000..f72e87bc70
--- /dev/null
+++ b/tests/avocado/ppc_powernv.py
@@ -0,0 +1,86 @@
+# Test that Linux kernel boots on ppc powernv machines and check the console
+#
+# Copyright (c) 2018, 2020 Red Hat, Inc.
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+
+from avocado.utils import archive
+from avocado_qemu import QemuSystemTest
+from avocado_qemu import wait_for_console_pattern
+
+class powernvMachine(QemuSystemTest):
+
+    timeout = 90
+    KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 '
+    panic_message = 'Kernel panic - not syncing'
+    good_message = 'VFS: Cannot open root device'
+
+    def do_test_linux_boot(self):
+        self.require_accelerator("tcg")
+        kernel_url = ('https://archives.fedoraproject.org/pub/archive'
+                      '/fedora-secondary/releases/29/Everything/ppc64le/os'
+                      '/ppc/ppc64/vmlinuz')
+        kernel_hash = '3fe04abfc852b66653b8c3c897a59a689270bc77'
+        kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
+
+        self.vm.set_console()
+        kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'console=hvc0'
+        self.vm.add_args('-kernel', kernel_path,
+                         '-append', kernel_command_line)
+        self.vm.launch()
+
+    def test_linux_boot(self):
+        """
+        :avocado: tags=arch:ppc64
+        :avocado: tags=machine:powernv
+        :avocado: tags=accel:tcg
+        """
+
+        self.do_test_linux_boot()
+        console_pattern = 'VFS: Cannot open root device'
+        wait_for_console_pattern(self, console_pattern, self.panic_message)
+
+    def test_linux_smp_boot(self):
+        """
+        :avocado: tags=arch:ppc64
+        :avocado: tags=machine:powernv
+        :avocado: tags=accel:tcg
+        """
+
+        self.vm.add_args('-smp', '4')
+        self.do_test_linux_boot()
+        console_pattern = 'smp: Brought up 1 node, 4 CPUs'
+        wait_for_console_pattern(self, console_pattern, self.panic_message)
+        wait_for_console_pattern(self, self.good_message, self.panic_message)
+
+    def test_linux_smt_boot(self):
+        """
+        :avocado: tags=arch:ppc64
+        :avocado: tags=machine:powernv
+        :avocado: tags=accel:tcg
+        """
+
+        self.vm.add_args('-smp', '4,threads=4')
+        self.do_test_linux_boot()
+        console_pattern = 'CPU maps initialized for 4 threads per core'
+        wait_for_console_pattern(self, console_pattern, self.panic_message)
+        console_pattern = 'smp: Brought up 1 node, 4 CPUs'
+        wait_for_console_pattern(self, console_pattern, self.panic_message)
+        wait_for_console_pattern(self, self.good_message, self.panic_message)
+
+    def test_linux_big_boot(self):
+        """
+        :avocado: tags=arch:ppc64
+        :avocado: tags=machine:powernv
+        :avocado: tags=accel:tcg
+        """
+
+        self.vm.add_args('-smp', '8,threads=4,cores=2,sockets=1')
+        # powernv does not support NUMA
+        self.do_test_linux_boot()
+        console_pattern = 'CPU maps initialized for 4 threads per core'
+        wait_for_console_pattern(self, console_pattern, self.panic_message)
+        console_pattern = 'smp: Brought up 1 node, 8 CPUs'
+        wait_for_console_pattern(self, console_pattern, self.panic_message)
+        wait_for_console_pattern(self, self.good_message, self.panic_message)
-- 
2.40.1

Re: [PATCH 4/4] tests/avocado: Add powernv machine test script

[Cédric Le Goater]

On 7/3/23 12:17, Nicholas Piggin wrote:
> This copies ppc_pseries.py to start a set of powernv tests, including
> a Linux boot test for the newly added SMT mode.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

A couple of suggestions below,

  


> ---
>   tests/avocado/ppc_powernv.py | 86 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 86 insertions(+)
>   create mode 100644 tests/avocado/ppc_powernv.py
> 
> diff --git a/tests/avocado/ppc_powernv.py b/tests/avocado/ppc_powernv.py
> new file mode 100644
> index 0000000000..f72e87bc70
> --- /dev/null
> +++ b/tests/avocado/ppc_powernv.py
> @@ -0,0 +1,86 @@
> +# Test that Linux kernel boots on ppc powernv machines and check the console
> +#
> +# Copyright (c) 2018, 2020 Red Hat, Inc.
> +#
> +# This work is licensed under the terms of the GNU GPL, version 2 or
> +# later.  See the COPYING file in the top-level directory.
> +
> +from avocado.utils import archive
> +from avocado_qemu import QemuSystemTest
> +from avocado_qemu import wait_for_console_pattern
> +
> +class powernvMachine(QemuSystemTest):
> +
> +    timeout = 90
> +    KERNEL_COMMON_COMMAND_LINE = 'printk.time=0 '
> +    panic_message = 'Kernel panic - not syncing'
> +    good_message = 'VFS: Cannot open root device'
> +
> +    def do_test_linux_boot(self):
> +        self.require_accelerator("tcg")
> +        kernel_url = ('https://archives.fedoraproject.org/pub/archive'
> +                      '/fedora-secondary/releases/29/Everything/ppc64le/os'
> +                      '/ppc/ppc64/vmlinuz')

f29 does support P10 :

   Linux version 4.18.16-300.fc29.ppc64le (mockbuild@buildvm-ppc64le-03.ppc.fedoraproject.org) (gcc version 8.2.1 20180801 (Red Hat 8.2.1-2) (GCC)) #1 SMP Sat Oct 20 23:28:27 UTC 2018

What about :

   https://archives.fedoraproject.org/pub/archive/fedora-secondary/releases/36/Everything/ppc64le/os/ppc/ppc64/

> +        kernel_hash = '3fe04abfc852b66653b8c3c897a59a689270bc77'
> +        kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
> +
> +        self.vm.set_console()
> +        kernel_command_line = self.KERNEL_COMMON_COMMAND_LINE + 'console=hvc0'
> +        self.vm.add_args('-kernel', kernel_path,
> +                         '-append', kernel_command_line)
> +        self.vm.launch()
> +
> +    def test_linux_boot(self):
> +        """
> +        :avocado: tags=arch:ppc64
> +        :avocado: tags=machine:powernv

This defaults to POWER9 cpus. No POWER10 ?

> +        :avocado: tags=accel:tcg
> +        """
> +
> +        self.do_test_linux_boot()
> +        console_pattern = 'VFS: Cannot open root device'
> +        wait_for_console_pattern(self, console_pattern, self.panic_message)
> +
> +    def test_linux_smp_boot(self):
> +        """
> +        :avocado: tags=arch:ppc64
> +        :avocado: tags=machine:powernv
> +        :avocado: tags=accel:tcg
> +        """
> +
> +        self.vm.add_args('-smp', '4')
> +        self.do_test_linux_boot()
> +        console_pattern = 'smp: Brought up 1 node, 4 CPUs'
> +        wait_for_console_pattern(self, console_pattern, self.panic_message)
> +        wait_for_console_pattern(self, self.good_message, self.panic_message)
> +
> +    def test_linux_smt_boot(self):
> +        """
> +        :avocado: tags=arch:ppc64
> +        :avocado: tags=machine:powernv
> +        :avocado: tags=accel:tcg
> +        """
> +
> +        self.vm.add_args('-smp', '4,threads=4')
> +        self.do_test_linux_boot()
> +        console_pattern = 'CPU maps initialized for 4 threads per core'
> +        wait_for_console_pattern(self, console_pattern, self.panic_message)
> +        console_pattern = 'smp: Brought up 1 node, 4 CPUs'
> +        wait_for_console_pattern(self, console_pattern, self.panic_message)
> +        wait_for_console_pattern(self, self.good_message, self.panic_message)
> +
> +    def test_linux_big_boot(self):
> +        """
> +        :avocado: tags=arch:ppc64
> +        :avocado: tags=machine:powernv
> +        :avocado: tags=accel:tcg
> +        """
> +
> +        self.vm.add_args('-smp', '8,threads=4,cores=2,sockets=1')

I would test 2 sockets.

Thanks,

C.

> +        # powernv does not support NUMA
> +        self.do_test_linux_boot()
> +        console_pattern = 'CPU maps initialized for 4 threads per core'
> +        wait_for_console_pattern(self, console_pattern, self.panic_message)
> +        console_pattern = 'smp: Brought up 1 node, 8 CPUs'
> +        wait_for_console_pattern(self, console_pattern, self.panic_message)
> +        wait_for_console_pattern(self, self.good_message, self.panic_message)

Re: [PATCH 2/4] target/ppc: SMT support for the HID SPR

[Cédric Le Goater]

On 7/3/23 12:16, Nicholas Piggin wrote:
> HID is a per-core shared register, skiboot sets this (e.g., setting
> HILE) on one thread and that must affect all threads of the core.
> 
> Tested-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.


> ---
>   target/ppc/cpu_init.c    |  2 +-
>   target/ppc/helper.h      |  1 +
>   target/ppc/misc_helper.c | 21 +++++++++++++++++++++
>   target/ppc/spr_common.h  |  1 +
>   target/ppc/translate.c   | 16 ++++++++++++++++
>   5 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 905a59aea9..720aad9e05 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -5638,7 +5638,7 @@ static void register_power_common_book4_sprs(CPUPPCState *env)
>       spr_register_hv(env, SPR_HID0, "HID0",
>                    SPR_NOACCESS, SPR_NOACCESS,
>                    SPR_NOACCESS, SPR_NOACCESS,
> -                 &spr_read_generic, &spr_write_generic,
> +                 &spr_read_generic, &spr_core_write_generic,
>                    0x00000000);
>       spr_register_hv(env, SPR_TSCR, "TSCR",
>                    SPR_NOACCESS, SPR_NOACCESS,
> diff --git a/target/ppc/helper.h b/target/ppc/helper.h
> index 828f7844c8..abec6fe341 100644
> --- a/target/ppc/helper.h
> +++ b/target/ppc/helper.h
> @@ -704,6 +704,7 @@ DEF_HELPER_3(store_dcr, void, env, tl, tl)
>   
>   DEF_HELPER_2(load_dump_spr, void, env, i32)
>   DEF_HELPER_2(store_dump_spr, void, env, i32)
> +DEF_HELPER_3(spr_core_write_generic, void, env, i32, tl)
>   DEF_HELPER_3(spr_write_CTRL, void, env, i32, tl)
>   
>   DEF_HELPER_4(fscr_facility_check, void, env, i32, i32, i32)
> diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c
> index 1f1af21f33..0da335472e 100644
> --- a/target/ppc/misc_helper.c
> +++ b/target/ppc/misc_helper.c
> @@ -43,6 +43,27 @@ void helper_store_dump_spr(CPUPPCState *env, uint32_t sprn)
>                env->spr[sprn]);
>   }
>   
> +void helper_spr_core_write_generic(CPUPPCState *env, uint32_t sprn,
> +                                   target_ulong val)
> +{
> +    CPUState *cs = env_cpu(env);
> +    CPUState *ccs;
> +    uint32_t nr_threads = cs->nr_threads;
> +    uint32_t core_id = env->spr[SPR_PIR] & ~(nr_threads - 1);
> +
> +    assert(core_id == env->spr[SPR_PIR] - env->spr[SPR_TIR]);
> +
> +    if (nr_threads == 1) {
> +        env->spr[sprn] = val;
> +        return;
> +    }
> +
> +    THREAD_SIBLING_FOREACH(cs, ccs) {
> +        CPUPPCState *cenv = &POWERPC_CPU(ccs)->env;
> +        cenv->spr[sprn] = val;
> +    }
> +}
> +
>   void helper_spr_write_CTRL(CPUPPCState *env, uint32_t sprn,
>                              target_ulong val)
>   {
> diff --git a/target/ppc/spr_common.h b/target/ppc/spr_common.h
> index fbf52123b5..5995070eaf 100644
> --- a/target/ppc/spr_common.h
> +++ b/target/ppc/spr_common.h
> @@ -82,6 +82,7 @@ void spr_noaccess(DisasContext *ctx, int gprn, int sprn);
>   void spr_read_generic(DisasContext *ctx, int gprn, int sprn);
>   void spr_write_generic(DisasContext *ctx, int sprn, int gprn);
>   void spr_write_generic32(DisasContext *ctx, int sprn, int gprn);
> +void spr_core_write_generic(DisasContext *ctx, int sprn, int gprn);
>   void spr_write_MMCR0(DisasContext *ctx, int sprn, int gprn);
>   void spr_write_MMCR1(DisasContext *ctx, int sprn, int gprn);
>   void spr_write_PMC(DisasContext *ctx, int sprn, int gprn);
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 10598cde40..07c491df62 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -448,6 +448,22 @@ void spr_write_generic32(DisasContext *ctx, int sprn, int gprn)
>   #endif
>   }
>   
> +void spr_core_write_generic(DisasContext *ctx, int sprn, int gprn)
> +{
> +    if (!(ctx->flags & POWERPC_FLAG_SMT)) {
> +        spr_write_generic(ctx, sprn, gprn);
> +        return;
> +    }
> +
> +    if (!gen_serialize(ctx)) {
> +        return;
> +    }
> +
> +    gen_helper_spr_core_write_generic(cpu_env, tcg_constant_i32(sprn),
> +                                      cpu_gpr[gprn]);
> +    spr_store_dump_spr(sprn);
> +}
> +
>   static void spr_write_CTRL_ST(DisasContext *ctx, int sprn, int gprn)
>   {
>       /* This does not implement >1 thread */

Re: [PATCH 1/4] target/ppc: Add LPAR-per-core vs per-thread mode flag

[Cédric Le Goater]

On 7/3/23 12:16, Nicholas Piggin wrote:
> The Power ISA has the concept of sub-processors:
> 
>    Hardware is allowed to sub-divide a multi-threaded processor into
>    "sub-processors" that appear to privileged programs as multi-threaded
>    processors with fewer threads.
> 
> POWER9 and POWER10 have two modes, either every thread is a
> sub-processor or all threads appear as one multi-threaded processor. In
> the user manuals these are known as "LPAR per thread" / "Thread LPAR",
> and "LPAR per core" / "1 LPAR", respectively.
> 
> The practical difference is: in thread LPAR mode, non-hypervisor SPRs
> are not shared between threads and msgsndp can not be used to message
> siblings. In 1 LPAR mode, some SPRs are shared and msgsndp is usable.
> Thrad LPAR allows multiple partitions to run concurrently on the same
> core, and is a requirement for KVM to run on POWER9/10 (which does not
> gang-schedule an LPAR on all threads of a core like POWER8 KVM).
> 
> Traditionally, SMT in PAPR environments including PowerVM and the
> pseries QEMU machine with KVM acceleration behaves as in 1 LPAR mode.
> In OPAL systems, Thread LPAR is used. When adding SMT to the powernv
> machine, it is therefore preferable to emulate Thread LPAR.
> 
> To account for this difference between pseries and powernv, an LPAR mode
> flag is added such that SPRs can be implemented as per-LPAR shared, and
> that becomes either per-thread or per-core depending on the flag.
> 
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> Tested-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>




Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> ---
>   hw/ppc/spapr_cpu_core.c |  2 ++
>   target/ppc/cpu.h        |  3 +++
>   target/ppc/cpu_init.c   | 12 ++++++++++++
>   target/ppc/translate.c  | 16 +++++++++++++---
>   4 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index a4e3c2fadd..b482d9754a 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -270,6 +270,8 @@ static bool spapr_realize_vcpu(PowerPCCPU *cpu, SpaprMachineState *spapr,
>       env->spr_cb[SPR_PIR].default_value = cs->cpu_index;
>       env->spr_cb[SPR_TIR].default_value = thread_index;
>   
> +    cpu_ppc_set_1lpar(cpu);
> +
>       /* Set time-base frequency to 512 MHz. vhyp must be set first. */
>       cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ);
>   
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index af12c93ebc..951f73d89d 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -674,6 +674,8 @@ enum {
>       POWERPC_FLAG_SCV      = 0x00200000,
>       /* Has >1 thread per core                                                */
>       POWERPC_FLAG_SMT      = 0x00400000,
> +    /* Using "LPAR per core" mode  (as opposed to per-thread)                */
> +    POWERPC_FLAG_SMT_1LPAR= 0x00800000,
>   };
>   
>   /*
> @@ -1437,6 +1439,7 @@ void store_booke_tsr(CPUPPCState *env, target_ulong val);
>   void ppc_tlb_invalidate_all(CPUPPCState *env);
>   void ppc_tlb_invalidate_one(CPUPPCState *env, target_ulong addr);
>   void cpu_ppc_set_vhyp(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp);
> +void cpu_ppc_set_1lpar(PowerPCCPU *cpu);
>   int ppcmas_tlb_check(CPUPPCState *env, ppcmas_tlb_t *tlb, hwaddr *raddrp,
>                        target_ulong address, uint32_t pid);
>   int ppcemb_tlb_search(CPUPPCState *env, target_ulong address, uint32_t pid);
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 5f4969664e..905a59aea9 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -6629,6 +6629,18 @@ void cpu_ppc_set_vhyp(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp)
>       env->msr_mask &= ~MSR_HVB;
>   }
>   
> +void cpu_ppc_set_1lpar(PowerPCCPU *cpu)
> +{
> +    CPUPPCState *env = &cpu->env;
> +
> +    /*
> +     * pseries SMT means "LPAR per core" mode, e.g., msgsndp is usable
> +     * between threads.
> +     */
> +    if (env->flags & POWERPC_FLAG_SMT) {
> +        env->flags |= POWERPC_FLAG_SMT_1LPAR;
> +    }
> +}
>   #endif /* !defined(CONFIG_USER_ONLY) */
>   
>   #endif /* defined(TARGET_PPC64) */
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index db0ba49bdc..10598cde40 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -256,6 +256,16 @@ static inline bool gen_serialize_core(DisasContext *ctx)
>   }
>   #endif
>   
> +static inline bool gen_serialize_core_lpar(DisasContext *ctx)
> +{
> +    /* 1LPAR implies SMT */
> +    if (ctx->flags & POWERPC_FLAG_SMT_1LPAR) {
> +        return gen_serialize(ctx);
> +    }
> +
> +    return true;
> +}
> +
>   /* SPR load/store helpers */
>   static inline void gen_load_spr(TCGv t, int reg)
>   {
> @@ -451,7 +461,7 @@ static void spr_write_CTRL_ST(DisasContext *ctx, int sprn, int gprn)
>   
>   void spr_write_CTRL(DisasContext *ctx, int sprn, int gprn)
>   {
> -    if (!(ctx->flags & POWERPC_FLAG_SMT)) {
> +    if (!(ctx->flags & POWERPC_FLAG_SMT_1LPAR)) {
>           spr_write_CTRL_ST(ctx, sprn, gprn);
>           goto out;
>       }
> @@ -815,7 +825,7 @@ void spr_write_pcr(DisasContext *ctx, int sprn, int gprn)
>   /* DPDES */
>   void spr_read_dpdes(DisasContext *ctx, int gprn, int sprn)
>   {
> -    if (!gen_serialize_core(ctx)) {
> +    if (!gen_serialize_core_lpar(ctx)) {
>           return;
>       }
>   
> @@ -824,7 +834,7 @@ void spr_read_dpdes(DisasContext *ctx, int gprn, int sprn)
>   
>   void spr_write_dpdes(DisasContext *ctx, int sprn, int gprn)
>   {
> -    if (!gen_serialize_core(ctx)) {
> +    if (!gen_serialize_core_lpar(ctx)) {
>           return;
>       }
>   

Re: [PATCH 1/4] target/ppc: Add LPAR-per-core vs per-thread mode flag

[Daniel Henrique Barboza]

Hi,

I'm afraid this patch breaks two Gitlab runners in different manners.


- the 'tsan-build' runner:

https://gitlab.com/danielhb/qemu/-/jobs/4583483251

[2170/3531] Compiling C object libqemu-ppc64-softmmu.fa.p/target_ppc
_translate.c.o
FAILED: libqemu-ppc64-softmmu.fa.p/target_ppc_translate.c.o
clang -m64 -mcx16 -Ilibqemu-ppc64-softmmu.fa.p -I. -I.. -Itarget/ppc -I../target/ppc -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/pixman-1 -I/usr/include/capstone -I/usr/include/spice-server -I/usr/include/spice-1 -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -fcolor-diagnostics -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -fsanitize=thread -fstack-protector-strong -Wundef -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wmissing-format-attribute -Wno-initializer-overrides -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-string-plus-int -Wno-typedef-redefinition -Wno-tautological-type-limit-compare -Wno-psabi -Wno-gnu-variable-sized-type-not-at-end -Wthread-safety -isystem /builds/danielhb/qemu/linux-headers -isystem linux-headers -iquote . -iquote /builds/danielhb/qemu -iquote /builds/danielhb/qemu/include -iquote /builds/danielhb/qemu/host/include/x86_64 -iquote /builds/danielhb/qemu/host/include/generic -iquote /builds/danielhb/qemu/tcg/i386 -pthread -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -fPIE -isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="ppc64-softmmu-config-target.h"' '-DCONFIG_DEVICES="ppc64-softmmu-config-devices.h"' -MD -MQ libqemu-ppc64-softmmu.fa.p/target_ppc_translate.c.o -MF libqemu-ppc64-softmmu.fa.p/target_ppc_translate.c.o.d -o libqemu-ppc64-softmmu.fa.p/target_ppc_translate.c.o -c ../target/ppc/translate.c
../target/ppc/translate.c:249:20: error: unused function 'gen_serialize_core' [-Werror,-Wunused-function]
static inline bool gen_serialize_core(DisasContext *ctx)
                    ^
1 error generated.

And in fact, after this patch, gen_serialize_core() is now unused:


$ git grep 'gen_serialize_core'
target/ppc/translate.c:static inline bool gen_serialize_core(DisasContext *ctx)
target/ppc/translate.c:static inline bool gen_serialize_core_lpar(DisasContext *ctx)
target/ppc/translate.c:    if (!gen_serialize_core_lpar(ctx)) {
target/ppc/translate.c:    if (!gen_serialize_core_lpar(ctx)) {
$

I'm not sure why this was the only runner that catched this error. Anyway, this is easily
fixable by removing gen_serialize_core().



- the 'clang-user' Gitlab runner:

https://gitlab.com/danielhb/qemu/-/jobs/4583483235

[1331/2617] Compiling C object libqemu-ppc-linux-user.fa.p/target_ppc_translate.c.o
FAILED: libqemu-ppc-linux-user.fa.p/target_ppc_translate.c.o
clang -m64 -mcx16 -Ilibqemu-ppc-linux-user.fa.p -I. -I.. -Itarget/ppc -I../target/ppc -I../common-user/host/x86_64 -I../linux-user/include/host/x86_64 -I../linux-user/include -Ilinux-user -I../linux-user -Ilinux-user/ppc -I../linux-user/ppc -Iqapi -Itrace -Iui/shader -I/usr/include/capstone -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -fcolor-diagnostics -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -fstack-protector-strong -Wundef -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wmissing-format-attribute -Wno-initializer-overrides -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-string-plus-int -Wno-typedef-redefinition -Wno-tautological-type-limit-compare -Wno-psabi -Wno-gnu-variable-sized-type-not-at-end -Wthread-safety -isystem /builds/danielhb/qemu/linux-headers -isystem linux-headers -iquote . -iquote /builds/danielhb/qemu -iquote /builds/danielhb/qemu/include -iquote /builds/danielhb/qemu/host/include/x86_64 -iquote /builds/danielhb/qemu/host/include/generic -iquote /builds/danielhb/qemu/tcg/i386 -pthread -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -fsanitize=undefined -fno-sanitize-recover=undefined -fPIE -isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="ppc-linux-user-config-target.h"' '-DCONFIG_DEVICES="ppc-linux-user-config-devices.h"' -MD -MQ libqemu-ppc-linux-user.fa.p/target_ppc_translate.c.o -MF libqemu-ppc-linux-user.fa.p/target_ppc_translate.c.o.d -o libqemu-ppc-linux-user.fa.p/target_ppc_translate.c.o -c ../target/ppc/translate.c
../target/ppc/translate.c:259:20: error: unused function 'gen_serialize_core_lpar' [-Werror,-Wunused-function]
static inline bool gen_serialize_core_lpar(DisasContext *ctx)
                    ^
1 error generated.


Given that the error was thrown when building the 32 bit linux user binary I think this
is a matter of wrapping the function with the adequate #ifdefs. Thanks,


Daniel



On 7/3/23 07:16, Nicholas Piggin wrote:
> The Power ISA has the concept of sub-processors:
> 
>    Hardware is allowed to sub-divide a multi-threaded processor into
>    "sub-processors" that appear to privileged programs as multi-threaded
>    processors with fewer threads.
> 
> POWER9 and POWER10 have two modes, either every thread is a
> sub-processor or all threads appear as one multi-threaded processor. In
> the user manuals these are known as "LPAR per thread" / "Thread LPAR",
> and "LPAR per core" / "1 LPAR", respectively.
> 
> The practical difference is: in thread LPAR mode, non-hypervisor SPRs
> are not shared between threads and msgsndp can not be used to message
> siblings. In 1 LPAR mode, some SPRs are shared and msgsndp is usable.
> Thrad LPAR allows multiple partitions to run concurrently on the same
> core, and is a requirement for KVM to run on POWER9/10 (which does not
> gang-schedule an LPAR on all threads of a core like POWER8 KVM).
> 
> Traditionally, SMT in PAPR environments including PowerVM and the
> pseries QEMU machine with KVM acceleration behaves as in 1 LPAR mode.
> In OPAL systems, Thread LPAR is used. When adding SMT to the powernv
> machine, it is therefore preferable to emulate Thread LPAR.
> 
> To account for this difference between pseries and powernv, an LPAR mode
> flag is added such that SPRs can be implemented as per-LPAR shared, and
> that becomes either per-thread or per-core depending on the flag.
> 
> Reviewed-by: Joel Stanley <joel@jms.id.au>
> Tested-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   hw/ppc/spapr_cpu_core.c |  2 ++
>   target/ppc/cpu.h        |  3 +++
>   target/ppc/cpu_init.c   | 12 ++++++++++++
>   target/ppc/translate.c  | 16 +++++++++++++---
>   4 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index a4e3c2fadd..b482d9754a 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -270,6 +270,8 @@ static bool spapr_realize_vcpu(PowerPCCPU *cpu, SpaprMachineState *spapr,
>       env->spr_cb[SPR_PIR].default_value = cs->cpu_index;
>       env->spr_cb[SPR_TIR].default_value = thread_index;
>   
> +    cpu_ppc_set_1lpar(cpu);
> +
>       /* Set time-base frequency to 512 MHz. vhyp must be set first. */
>       cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ);
>   
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index af12c93ebc..951f73d89d 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -674,6 +674,8 @@ enum {
>       POWERPC_FLAG_SCV      = 0x00200000,
>       /* Has >1 thread per core                                                */
>       POWERPC_FLAG_SMT      = 0x00400000,
> +    /* Using "LPAR per core" mode  (as opposed to per-thread)                */
> +    POWERPC_FLAG_SMT_1LPAR= 0x00800000,
>   };
>   
>   /*
> @@ -1437,6 +1439,7 @@ void store_booke_tsr(CPUPPCState *env, target_ulong val);
>   void ppc_tlb_invalidate_all(CPUPPCState *env);
>   void ppc_tlb_invalidate_one(CPUPPCState *env, target_ulong addr);
>   void cpu_ppc_set_vhyp(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp);
> +void cpu_ppc_set_1lpar(PowerPCCPU *cpu);
>   int ppcmas_tlb_check(CPUPPCState *env, ppcmas_tlb_t *tlb, hwaddr *raddrp,
>                        target_ulong address, uint32_t pid);
>   int ppcemb_tlb_search(CPUPPCState *env, target_ulong address, uint32_t pid);
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 5f4969664e..905a59aea9 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -6629,6 +6629,18 @@ void cpu_ppc_set_vhyp(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp)
>       env->msr_mask &= ~MSR_HVB;
>   }
>   
> +void cpu_ppc_set_1lpar(PowerPCCPU *cpu)
> +{
> +    CPUPPCState *env = &cpu->env;
> +
> +    /*
> +     * pseries SMT means "LPAR per core" mode, e.g., msgsndp is usable
> +     * between threads.
> +     */
> +    if (env->flags & POWERPC_FLAG_SMT) {
> +        env->flags |= POWERPC_FLAG_SMT_1LPAR;
> +    }
> +}
>   #endif /* !defined(CONFIG_USER_ONLY) */
>   
>   #endif /* defined(TARGET_PPC64) */
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index db0ba49bdc..10598cde40 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -256,6 +256,16 @@ static inline bool gen_serialize_core(DisasContext *ctx)
>   }
>   #endif
>   
> +static inline bool gen_serialize_core_lpar(DisasContext *ctx)
> +{
> +    /* 1LPAR implies SMT */
> +    if (ctx->flags & POWERPC_FLAG_SMT_1LPAR) {
> +        return gen_serialize(ctx);
> +    }
> +
> +    return true;
> +}
> +
>   /* SPR load/store helpers */
>   static inline void gen_load_spr(TCGv t, int reg)
>   {
> @@ -451,7 +461,7 @@ static void spr_write_CTRL_ST(DisasContext *ctx, int sprn, int gprn)
>   
>   void spr_write_CTRL(DisasContext *ctx, int sprn, int gprn)
>   {
> -    if (!(ctx->flags & POWERPC_FLAG_SMT)) {
> +    if (!(ctx->flags & POWERPC_FLAG_SMT_1LPAR)) {
>           spr_write_CTRL_ST(ctx, sprn, gprn);
>           goto out;
>       }
> @@ -815,7 +825,7 @@ void spr_write_pcr(DisasContext *ctx, int sprn, int gprn)
>   /* DPDES */
>   void spr_read_dpdes(DisasContext *ctx, int gprn, int sprn)
>   {
> -    if (!gen_serialize_core(ctx)) {
> +    if (!gen_serialize_core_lpar(ctx)) {
>           return;
>       }
>   
> @@ -824,7 +834,7 @@ void spr_read_dpdes(DisasContext *ctx, int gprn, int sprn)
>   
>   void spr_write_dpdes(DisasContext *ctx, int sprn, int gprn)
>   {
> -    if (!gen_serialize_core(ctx)) {
> +    if (!gen_serialize_core_lpar(ctx)) {
>           return;
>       }
>   

Re: [PATCH 1/4] target/ppc: Add LPAR-per-core vs per-thread mode flag

[Cédric Le Goater]

On 7/3/23 14:23, Daniel Henrique Barboza wrote:
> Hi,
> 
> I'm afraid this patch breaks two Gitlab runners in different manners.
> 
> 
> - the 'tsan-build' runner:
> 
> https://gitlab.com/danielhb/qemu/-/jobs/4583483251
> 
> [2170/3531] Compiling C object libqemu-ppc64-softmmu.fa.p/target_ppc
> _translate.c.o
> FAILED: libqemu-ppc64-softmmu.fa.p/target_ppc_translate.c.o
> clang -m64 -mcx16 -Ilibqemu-ppc64-softmmu.fa.p -I. -I.. -Itarget/ppc -I../target/ppc -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/pixman-1 -I/usr/include/capstone -I/usr/include/spice-server -I/usr/include/spice-1 -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -fcolor-diagnostics -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -fsanitize=thread -fstack-protector-strong -Wundef -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wmissing-format-attribute -Wno-initializer-overrides -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-string-plus-int -Wno-typedef-redefinition -Wno-tautological-type-limit-compare -Wno-psabi -Wno-gnu-variable-sized-type-not-at-end -Wthread-safety -isystem /builds/danielhb/qemu/linux-headers -isystem linux-headers -iquote . -iquote 
> /builds/danielhb/qemu -iquote /builds/danielhb/qemu/include -iquote /builds/danielhb/qemu/host/include/x86_64 -iquote /builds/danielhb/qemu/host/include/generic -iquote /builds/danielhb/qemu/tcg/i386 -pthread -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -fPIE -isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="ppc64-softmmu-config-target.h"' '-DCONFIG_DEVICES="ppc64-softmmu-config-devices.h"' -MD -MQ libqemu-ppc64-softmmu.fa.p/target_ppc_translate.c.o -MF libqemu-ppc64-softmmu.fa.p/target_ppc_translate.c.o.d -o libqemu-ppc64-softmmu.fa.p/target_ppc_translate.c.o -c ../target/ppc/translate.c
> ../target/ppc/translate.c:249:20: error: unused function 'gen_serialize_core' [-Werror,-Wunused-function]
> static inline bool gen_serialize_core(DisasContext *ctx)
>                     ^
> 1 error generated.
> 
> And in fact, after this patch, gen_serialize_core() is now unused:
> 
> 
> $ git grep 'gen_serialize_core'
> target/ppc/translate.c:static inline bool gen_serialize_core(DisasContext *ctx)
> target/ppc/translate.c:static inline bool gen_serialize_core_lpar(DisasContext *ctx)
> target/ppc/translate.c:    if (!gen_serialize_core_lpar(ctx)) {
> target/ppc/translate.c:    if (!gen_serialize_core_lpar(ctx)) {
> $
> 
> I'm not sure why this was the only runner that catched this error. Anyway, this is easily
> fixable by removing gen_serialize_core().
> 
> 
> 
> - the 'clang-user' Gitlab runner:
> 
> https://gitlab.com/danielhb/qemu/-/jobs/4583483235
> 
> [1331/2617] Compiling C object libqemu-ppc-linux-user.fa.p/target_ppc_translate.c.o
> FAILED: libqemu-ppc-linux-user.fa.p/target_ppc_translate.c.o
> clang -m64 -mcx16 -Ilibqemu-ppc-linux-user.fa.p -I. -I.. -Itarget/ppc -I../target/ppc -I../common-user/host/x86_64 -I../linux-user/include/host/x86_64 -I../linux-user/include -Ilinux-user -I../linux-user -Ilinux-user/ppc -I../linux-user/ppc -Iqapi -Itrace -Iui/shader -I/usr/include/capstone -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -fcolor-diagnostics -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -fstack-protector-strong -Wundef -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wmissing-format-attribute -Wno-initializer-overrides -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-string-plus-int -Wno-typedef-redefinition -Wno-tautological-type-limit-compare -Wno-psabi -Wno-gnu-variable-sized-type-not-at-end -Wthread-safety -isystem 
> /builds/danielhb/qemu/linux-headers -isystem linux-headers -iquote . -iquote /builds/danielhb/qemu -iquote /builds/danielhb/qemu/include -iquote /builds/danielhb/qemu/host/include/x86_64 -iquote /builds/danielhb/qemu/host/include/generic -iquote /builds/danielhb/qemu/tcg/i386 -pthread -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -fsanitize=undefined -fno-sanitize-recover=undefined -fPIE -isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="ppc-linux-user-config-target.h"' '-DCONFIG_DEVICES="ppc-linux-user-config-devices.h"' -MD -MQ libqemu-ppc-linux-user.fa.p/target_ppc_translate.c.o -MF libqemu-ppc-linux-user.fa.p/target_ppc_translate.c.o.d -o libqemu-ppc-linux-user.fa.p/target_ppc_translate.c.o -c ../target/ppc/translate.c
> ../target/ppc/translate.c:259:20: error: unused function 'gen_serialize_core_lpar' [-Werror,-Wunused-function]
> static inline bool gen_serialize_core_lpar(DisasContext *ctx)
>                     ^
> 1 error generated.
> 
> 
> Given that the error was thrown when building the 32 bit linux user binary I think this
> is a matter of wrapping the function with the adequate #ifdefs. Thanks,
> 
> 


I added a fix in b769d4c8f4c6 for a similar issue.

Nick, could you please take a look and compile with a recent clang ?

Thanks,

C.



> Daniel
> 
> 
> 
> On 7/3/23 07:16, Nicholas Piggin wrote:
>> The Power ISA has the concept of sub-processors:
>>
>>    Hardware is allowed to sub-divide a multi-threaded processor into
>>    "sub-processors" that appear to privileged programs as multi-threaded
>>    processors with fewer threads.
>>
>> POWER9 and POWER10 have two modes, either every thread is a
>> sub-processor or all threads appear as one multi-threaded processor. In
>> the user manuals these are known as "LPAR per thread" / "Thread LPAR",
>> and "LPAR per core" / "1 LPAR", respectively.
>>
>> The practical difference is: in thread LPAR mode, non-hypervisor SPRs
>> are not shared between threads and msgsndp can not be used to message
>> siblings. In 1 LPAR mode, some SPRs are shared and msgsndp is usable.
>> Thrad LPAR allows multiple partitions to run concurrently on the same
>> core, and is a requirement for KVM to run on POWER9/10 (which does not
>> gang-schedule an LPAR on all threads of a core like POWER8 KVM).
>>
>> Traditionally, SMT in PAPR environments including PowerVM and the
>> pseries QEMU machine with KVM acceleration behaves as in 1 LPAR mode.
>> In OPAL systems, Thread LPAR is used. When adding SMT to the powernv
>> machine, it is therefore preferable to emulate Thread LPAR.
>>
>> To account for this difference between pseries and powernv, an LPAR mode
>> flag is added such that SPRs can be implemented as per-LPAR shared, and
>> that becomes either per-thread or per-core depending on the flag.
>>
>> Reviewed-by: Joel Stanley <joel@jms.id.au>
>> Tested-by: Cédric Le Goater <clg@kaod.org>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>>   hw/ppc/spapr_cpu_core.c |  2 ++
>>   target/ppc/cpu.h        |  3 +++
>>   target/ppc/cpu_init.c   | 12 ++++++++++++
>>   target/ppc/translate.c  | 16 +++++++++++++---
>>   4 files changed, 30 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
>> index a4e3c2fadd..b482d9754a 100644
>> --- a/hw/ppc/spapr_cpu_core.c
>> +++ b/hw/ppc/spapr_cpu_core.c
>> @@ -270,6 +270,8 @@ static bool spapr_realize_vcpu(PowerPCCPU *cpu, SpaprMachineState *spapr,
>>       env->spr_cb[SPR_PIR].default_value = cs->cpu_index;
>>       env->spr_cb[SPR_TIR].default_value = thread_index;
>> +    cpu_ppc_set_1lpar(cpu);
>> +
>>       /* Set time-base frequency to 512 MHz. vhyp must be set first. */
>>       cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ);
>> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
>> index af12c93ebc..951f73d89d 100644
>> --- a/target/ppc/cpu.h
>> +++ b/target/ppc/cpu.h
>> @@ -674,6 +674,8 @@ enum {
>>       POWERPC_FLAG_SCV      = 0x00200000,
>>       /* Has >1 thread per core                                                */
>>       POWERPC_FLAG_SMT      = 0x00400000,
>> +    /* Using "LPAR per core" mode  (as opposed to per-thread)                */
>> +    POWERPC_FLAG_SMT_1LPAR= 0x00800000,
>>   };
>>   /*
>> @@ -1437,6 +1439,7 @@ void store_booke_tsr(CPUPPCState *env, target_ulong val);
>>   void ppc_tlb_invalidate_all(CPUPPCState *env);
>>   void ppc_tlb_invalidate_one(CPUPPCState *env, target_ulong addr);
>>   void cpu_ppc_set_vhyp(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp);
>> +void cpu_ppc_set_1lpar(PowerPCCPU *cpu);
>>   int ppcmas_tlb_check(CPUPPCState *env, ppcmas_tlb_t *tlb, hwaddr *raddrp,
>>                        target_ulong address, uint32_t pid);
>>   int ppcemb_tlb_search(CPUPPCState *env, target_ulong address, uint32_t pid);
>> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
>> index 5f4969664e..905a59aea9 100644
>> --- a/target/ppc/cpu_init.c
>> +++ b/target/ppc/cpu_init.c
>> @@ -6629,6 +6629,18 @@ void cpu_ppc_set_vhyp(PowerPCCPU *cpu, PPCVirtualHypervisor *vhyp)
>>       env->msr_mask &= ~MSR_HVB;
>>   }
>> +void cpu_ppc_set_1lpar(PowerPCCPU *cpu)
>> +{
>> +    CPUPPCState *env = &cpu->env;
>> +
>> +    /*
>> +     * pseries SMT means "LPAR per core" mode, e.g., msgsndp is usable
>> +     * between threads.
>> +     */
>> +    if (env->flags & POWERPC_FLAG_SMT) {
>> +        env->flags |= POWERPC_FLAG_SMT_1LPAR;
>> +    }
>> +}
>>   #endif /* !defined(CONFIG_USER_ONLY) */
>>   #endif /* defined(TARGET_PPC64) */
>> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
>> index db0ba49bdc..10598cde40 100644
>> --- a/target/ppc/translate.c
>> +++ b/target/ppc/translate.c
>> @@ -256,6 +256,16 @@ static inline bool gen_serialize_core(DisasContext *ctx)
>>   }
>>   #endif
>> +static inline bool gen_serialize_core_lpar(DisasContext *ctx)
>> +{
>> +    /* 1LPAR implies SMT */
>> +    if (ctx->flags & POWERPC_FLAG_SMT_1LPAR) {
>> +        return gen_serialize(ctx);
>> +    }
>> +
>> +    return true;
>> +}
>> +
>>   /* SPR load/store helpers */
>>   static inline void gen_load_spr(TCGv t, int reg)
>>   {
>> @@ -451,7 +461,7 @@ static void spr_write_CTRL_ST(DisasContext *ctx, int sprn, int gprn)
>>   void spr_write_CTRL(DisasContext *ctx, int sprn, int gprn)
>>   {
>> -    if (!(ctx->flags & POWERPC_FLAG_SMT)) {
>> +    if (!(ctx->flags & POWERPC_FLAG_SMT_1LPAR)) {
>>           spr_write_CTRL_ST(ctx, sprn, gprn);
>>           goto out;
>>       }
>> @@ -815,7 +825,7 @@ void spr_write_pcr(DisasContext *ctx, int sprn, int gprn)
>>   /* DPDES */
>>   void spr_read_dpdes(DisasContext *ctx, int gprn, int sprn)
>>   {
>> -    if (!gen_serialize_core(ctx)) {
>> +    if (!gen_serialize_core_lpar(ctx)) {
>>           return;
>>       }
>> @@ -824,7 +834,7 @@ void spr_read_dpdes(DisasContext *ctx, int gprn, int sprn)
>>   void spr_write_dpdes(DisasContext *ctx, int sprn, int gprn)
>>   {
>> -    if (!gen_serialize_core(ctx)) {
>> +    if (!gen_serialize_core_lpar(ctx)) {
>>           return;
>>       }

Re: [PATCH 0/4] ppc/pnv: SMT support for powernv

[Cédric Le Goater]

On 7/3/23 12:16, Nicholas Piggin wrote:
> These patches implement enough to install a distro, boot, run SMP KVM
> guests with libvirt with good performance using MTTCG (as reported by
> Cedric).
> 
> There are a few more SPRs that need to be done, and per-LPAR SPRs are
> mostly not annotated yet so it can't run in 1 LPAR mode. But those can
> be added in time, it will take a bit of time to get everything exactly
> as hardware does so I consider this good enough to run common
> software usefully.
> 
> Thanks Joel and Cedric for reviews and testing.
> 
> Since RFC:
> - Rebased against ppc-next (no conflicts vs upstream anyway).
> - Add patch 4 avocado boot test with SMT, as was added with pseries SMT.
> - Renamed POWERPC_FLAG_1LPAR to POWERPC_FLAG_SMT_1LPAR since it implies
>    SMT.
> - Fixed typos, patch 1, 3 changelogs improvement (hopefully).

Since you should resend for the clang breakage, could you please update
the CAVEATS section in docs/system/ppc/powernv.rst also ?

pseries documentation will need a similar update. Not sure where though.

Thanks,

C.


> 
> Thanks,
> Nick
> 
> Nicholas Piggin (4):
>    target/ppc: Add LPAR-per-core vs per-thread mode flag
>    target/ppc: SMT support for the HID SPR
>    ppc/pnv: SMT support for powernv
>    tests/avocado: Add powernv machine test script
> 
>   hw/ppc/pnv.c                 | 12 +++++
>   hw/ppc/pnv_core.c            | 13 +++---
>   hw/ppc/spapr_cpu_core.c      |  2 +
>   target/ppc/cpu.h             |  3 ++
>   target/ppc/cpu_init.c        | 14 +++++-
>   target/ppc/helper.h          |  1 +
>   target/ppc/misc_helper.c     | 21 +++++++++
>   target/ppc/spr_common.h      |  1 +
>   target/ppc/translate.c       | 32 ++++++++++++--
>   tests/avocado/ppc_powernv.py | 86 ++++++++++++++++++++++++++++++++++++
>   10 files changed, 173 insertions(+), 12 deletions(-)
>   create mode 100644 tests/avocado/ppc_powernv.py
> 

Re: [PATCH 1/4] target/ppc: Add LPAR-per-core vs per-thread mode flag

[Nicholas Piggin]

On Mon Jul 3, 2023 at 10:23 PM AEST, Daniel Henrique Barboza wrote:
> Hi,
>
> I'm afraid this patch breaks two Gitlab runners in different manners.
>
>
> - the 'tsan-build' runner:
>
> https://gitlab.com/danielhb/qemu/-/jobs/4583483251
>
> [2170/3531] Compiling C object libqemu-ppc64-softmmu.fa.p/target_ppc
> _translate.c.o
> FAILED: libqemu-ppc64-softmmu.fa.p/target_ppc_translate.c.o
> clang -m64 -mcx16 -Ilibqemu-ppc64-softmmu.fa.p -I. -I.. -Itarget/ppc -I../target/ppc -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/pixman-1 -I/usr/include/capstone -I/usr/include/spice-server -I/usr/include/spice-1 -I/usr/include/glib-2.0 -I/usr/lib/x86_64-linux-gnu/glib-2.0/include -fcolor-diagnostics -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -fsanitize=thread -fstack-protector-strong -Wundef -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wmissing-format-attribute -Wno-initializer-overrides -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-string-plus-int -Wno-typedef-redefinition -Wno-tautological-type-limit-compare -Wno-psabi -Wno-gnu-variable-sized-type-not-at-end -Wthread-safety -isystem /builds/danielhb/qemu/linux-headers -isystem linux-headers -iquote . -iquote /builds/danielhb/qemu -iquote /builds/danielhb/qemu/include -iquote /builds/danielhb/qemu/host/include/x86_64 -iquote /builds/danielhb/qemu/host/include/generic -iquote /builds/danielhb/qemu/tcg/i386 -pthread -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -fPIE -isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="ppc64-softmmu-config-target.h"' '-DCONFIG_DEVICES="ppc64-softmmu-config-devices.h"' -MD -MQ libqemu-ppc64-softmmu.fa.p/target_ppc_translate.c.o -MF libqemu-ppc64-softmmu.fa.p/target_ppc_translate.c.o.d -o libqemu-ppc64-softmmu.fa.p/target_ppc_translate.c.o -c ../target/ppc/translate.c
> ../target/ppc/translate.c:249:20: error: unused function 'gen_serialize_core' [-Werror,-Wunused-function]
> static inline bool gen_serialize_core(DisasContext *ctx)
>                     ^
> 1 error generated.
>
> And in fact, after this patch, gen_serialize_core() is now unused:

Sorry Daniel :( I keep losing my test config. I'll have to set up
something a bit more permanent and reliable. Will resubmit.

Thanks,
Nick