[PATCH 0/8] ppc: Update targets for Power machines (spapr and pnv)

[PATCH 0/8] ppc: Update targets for Power machines (spapr and pnv)

[Nicholas Piggin]

These are some patches to move defaults to Power10 CPU, update
some feature bits, and also update skiboot to 7.1. I would like
to do this for 9.0 if possible.

Thanks,
Nick

Nicholas Piggin (8):
  target/ppc: POWER10 does not have transactional memory
  ppc/spapr|pnv: Remove SAO from pa-features when running MTTCG
  ppc/spapr: Remove copy-paste from pa-features under TCG
  ppc/spapr: Adjust ibm,pa-features for POWER9
  ppc/spapr: Add pa-features for POWER10 machines
  ppc/pnv: Permit ibm,pa-features set per machine variant
  ppc/pnv: Set POWER9, POWER10 ibm,pa-features bits
  ppc/pnv: Update skiboot to v7.1

 hw/ppc/pnv.c          | 110 +++++++++++++++++++++++++++++++++++++-----
 hw/ppc/spapr.c        |  72 +++++++++++++++++++++++++--
 target/ppc/cpu_init.c |   4 +-
 pc-bios/skiboot.lid   | Bin 2527240 -> 2527328 bytes
 roms/skiboot          |   2 +-
 5 files changed, 168 insertions(+), 20 deletions(-)

-- 
2.42.0

[PATCH 1/8] target/ppc: POWER10 does not have transactional memory

[Nicholas Piggin]

POWER10 hardware implements a degenerate transactional memory facility
in POWER8/9 PCR compatibility modes to permit migration from older
CPUs, but POWER10 / ISA v3.1 mode does not support it so the CPU model
should not support it.

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 9df606d523..5c1d0adca8 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -6576,7 +6576,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
                         PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
                         PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
                         PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
-                        PPC2_TM | PPC2_ISA300 | PPC2_PRCNTL | PPC2_ISA310 |
+                        PPC2_ISA300 | PPC2_PRCNTL | PPC2_ISA310 |
                         PPC2_MEM_LWSYNC | PPC2_BCDA_ISA206;
     pcc->msr_mask = (1ull << MSR_SF) |
                     (1ull << MSR_HV) |
@@ -6620,7 +6620,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
     pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
                  POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
                  POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR |
-                 POWERPC_FLAG_VSX | POWERPC_FLAG_TM | POWERPC_FLAG_SCV;
+                 POWERPC_FLAG_VSX | POWERPC_FLAG_SCV;
     pcc->l1_dcache_size = 0x8000;
     pcc->l1_icache_size = 0x8000;
 }
-- 
2.42.0

[PATCH 2/8] ppc/spapr|pnv: Remove SAO from pa-features when running MTTCG

[Nicholas Piggin]

SAO is a page table attribute that strengthens the memory ordering of
accesses. QEMU with MTTCG does not implement this, so clear it in
ibm,pa-features. There is a complication with spapr migration that is
addressed with comments, it is not a new problem here.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/ppc/pnv.c   |  5 +++++
 hw/ppc/spapr.c | 15 +++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index b949398689..4969fbdb05 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -158,6 +158,11 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
     char *nodename;
     int cpus_offset = get_cpus_node(fdt);
 
+    if (qemu_tcg_mttcg_enabled()) {
+        /* SSO (SAO) ordering is not supported under MTTCG. */
+        pa_features[4 + 2] &= ~0x80;
+    }
+
     nodename = g_strdup_printf("%s@%x", dc->fw_name, pc->pir);
     offset = fdt_add_subnode(fdt, cpus_offset, nodename);
     _FDT(offset);
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 021b1a00e1..1c79d5670d 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -284,6 +284,21 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
         return;
     }
 
+    if (qemu_tcg_mttcg_enabled()) {
+        /*
+         * SSO (SAO) ordering is not supported under MTTCG, so disable it.
+         * There is no cap for this, so there is a migration bug here.
+         * However don't disable it entirely, to allow it to be used under
+         * KVM. This is a minor concern because:
+         * - SAO is an obscure an rarely (if ever) used feature.
+         * - SAO is removed from POWER10 / v3.1, so there is already a
+         *   migration problem today.
+         * - Linux does not test this pa-features bit today anyway, so it's
+         *   academic.
+         */
+        pa_features[4 + 2] &= ~0x80;
+    }
+
     if (ppc_hash64_has(cpu, PPC_HASH64_CI_LARGEPAGE)) {
         /*
          * Note: we keep CI large pages off by default because a 64K capable
-- 
2.42.0

[PATCH 3/8] ppc/spapr: Remove copy-paste from pa-features under TCG

[Nicholas Piggin]

TCG does not support copy/paste instructions. Remove it from
ibm,pa-features when running TCG. Similarly to SAO, there is a
migration issue here, but this doesn't really make things worse,
a cap would be required to fix it.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/ppc/spapr.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 1c79d5670d..ce969be770 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -299,6 +299,18 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
         pa_features[4 + 2] &= ~0x80;
     }
 
+    if (tcg_enabled()) {
+        /*
+         * TCG does not implement copy/paste instructions. This causes a
+         * migration issue similarly to SAO, but there is no avoiding that
+         * since there is no KVM cap or way to disable the instructions in
+         * hardware. It's also quite an obscure instruction and is not really
+         * used in KVM guests since KVM does not support accelerator pass-
+         * through anyway.
+         */
+        pa_features[38 + 2] &= ~0x80;
+    }
+
     if (ppc_hash64_has(cpu, PPC_HASH64_CI_LARGEPAGE)) {
         /*
          * Note: we keep CI large pages off by default because a 64K capable
-- 
2.42.0

[PATCH 4/8] ppc/spapr: Adjust ibm,pa-features for POWER9

[Nicholas Piggin]

"MMR" and "SPR SO" are not implemented in POWER9, so clear those bits.
HTM is not set by default, and only later if the cap is set, so remove
the comment that suggests otherwise.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/ppc/spapr.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index ce969be770..1e1946de59 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -248,14 +248,14 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
         0x80, 0x00, 0x00, 0x00, 0x00, 0x00, /* 6 - 11 */
         /* 16: Vector */
         0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 12 - 17 */
-        /* 18: Vec. Scalar, 20: Vec. XOR, 22: HTM */
+        /* 18: Vec. Scalar, 20: Vec. XOR */
         0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 18 - 23 */
         /* 24: Ext. Dec, 26: 64 bit ftrs, 28: PM ftrs */
         0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 24 - 29 */
-        /* 30: MMR, 32: LE atomic, 34: EBB + ext EBB */
-        0x80, 0x00, 0x80, 0x00, 0xC0, 0x00, /* 30 - 35 */
-        /* 36: SPR SO, 38: Copy/Paste, 40: Radix MMU */
-        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 36 - 41 */
+        /* 32: LE atomic, 34: EBB + ext EBB */
+        0x00, 0x00, 0x80, 0x00, 0xC0, 0x00, /* 30 - 35 */
+        /* 38: Copy/Paste, 40: Radix MMU */
+        0x00, 0x00, 0x80, 0x00, 0x80, 0x00, /* 36 - 41 */
         /* 42: PM, 44: PC RA, 46: SC vec'd */
         0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 42 - 47 */
         /* 48: SIMD, 50: QP BFP, 52: String */
-- 
2.42.0

[PATCH 5/8] ppc/spapr: Add pa-features for POWER10 machines

[Nicholas Piggin]

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/ppc/spapr.c | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 1e1946de59..2f58d8f292 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -265,6 +265,36 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
         /* 60: NM atomic, 62: RNG */
         0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
     };
+    /* 3.1 removes SAO, HTM support */
+    uint8_t pa_features_31[] = { 74, 0,
+        /* 0: MMU|FPU|SLB|RUN|DABR|NX, 1: fri[nzpm]|DABRX|SPRG3|SLB0|PP110 */
+        /* 2: VPM|DS205|PPR|DS202|DS206, 3: LSD|URG, 5: LE|CFAR|EB|LSQ */
+        0xf6, 0x1f, 0xc7, 0xc0, 0x00, 0xf0, /* 0 - 5 */
+        /* 6: DS207 */
+        0x80, 0x00, 0x00, 0x00, 0x00, 0x00, /* 6 - 11 */
+        /* 16: Vector */
+        0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 12 - 17 */
+        /* 18: Vec. Scalar, 20: Vec. XOR */
+        0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 18 - 23 */
+        /* 24: Ext. Dec, 26: 64 bit ftrs, 28: PM ftrs */
+        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 24 - 29 */
+        /* 32: LE atomic, 34: EBB + ext EBB */
+        0x00, 0x00, 0x80, 0x00, 0xC0, 0x00, /* 30 - 35 */
+        /* 38: Copy/Paste, 40: Radix MMU */
+        0x00, 0x00, 0x80, 0x00, 0x80, 0x00, /* 36 - 41 */
+        /* 42: PM, 44: PC RA, 46: SC vec'd */
+        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 42 - 47 */
+        /* 48: SIMD, 50: QP BFP, 52: String */
+        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 48 - 53 */
+        /* 54: DecFP, 56: DecI, 58: SHA */
+        0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 54 - 59 */
+        /* 60: NM atomic, 62: RNG */
+        0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
+        /* 68: DEXCR[SBHE|IBRTPDUS|SRAPD|NPHIE|PHIE] */
+        0x00, 0x00, 0xce, 0x00, 0x00, 0x00, /* 66 - 71 */
+        /* 72: Hash Check */
+        0x80, 0x00, /* 73 */
+    };
     uint8_t *pa_features = NULL;
     size_t pa_size;
 
@@ -280,6 +310,10 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
         pa_features = pa_features_300;
         pa_size = sizeof(pa_features_300);
     }
+    if (ppc_check_compat(cpu, CPU_POWERPC_LOGICAL_3_10, 0, cpu->compat_pvr)) {
+        pa_features = pa_features_31;
+        pa_size = sizeof(pa_features_31);
+    }
     if (!pa_features) {
         return;
     }
-- 
2.42.0

[PATCH 6/8] ppc/pnv: Permit ibm,pa-features set per machine variant

[Nicholas Piggin]

This allows different pa-features for powernv8/9/10.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/ppc/pnv.c | 41 +++++++++++++++++++++++++++++------------
 1 file changed, 29 insertions(+), 12 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 4969fbdb05..0a144402d7 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -133,7 +133,7 @@ static int get_cpus_node(void *fdt)
  * device tree, used in XSCOM to address cores and in interrupt
  * servers.
  */
-static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
+static int pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
 {
     PowerPCCPU *cpu = pc->threads[0];
     CPUState *cs = CPU(cpu);
@@ -149,11 +149,6 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
     uint32_t cpufreq = 1000000000;
     uint32_t page_sizes_prop[64];
     size_t page_sizes_prop_size;
-    const uint8_t pa_features[] = { 24, 0,
-                                    0xf6, 0x3f, 0xc7, 0xc0, 0x80, 0xf0,
-                                    0x80, 0x00, 0x00, 0x00, 0x00, 0x00,
-                                    0x00, 0x00, 0x00, 0x00, 0x80, 0x00,
-                                    0x80, 0x00, 0x80, 0x00, 0x80, 0x00 };
     int offset;
     char *nodename;
     int cpus_offset = get_cpus_node(fdt);
@@ -241,15 +236,14 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
                            page_sizes_prop, page_sizes_prop_size)));
     }
 
-    _FDT((fdt_setprop(fdt, offset, "ibm,pa-features",
-                       pa_features, sizeof(pa_features))));
-
     /* Build interrupt servers properties */
     for (i = 0; i < smt_threads; i++) {
         servers_prop[i] = cpu_to_be32(pc->pir + i);
     }
     _FDT((fdt_setprop(fdt, offset, "ibm,ppc-interrupt-server#s",
                        servers_prop, sizeof(*servers_prop) * smt_threads)));
+
+    return offset;
 }
 
 static void pnv_dt_icp(PnvChip *chip, void *fdt, uint32_t pir,
@@ -304,6 +298,17 @@ PnvChip *pnv_chip_add_phb(PnvChip *chip, PnvPHB *phb)
     return chip;
 }
 
+/*
+ * Same as spapr pa_features_207 except pnv always enables CI largepages bit.
+ * HTM is always enabled because TCG does implement HTM, it's just a
+ * degenerate implementation.
+ */
+static const uint8_t pa_features_207[] = { 24, 0,
+                 0xf6, 0x3f, 0xc7, 0xc0, 0x80, 0xf0,
+                 0x80, 0x00, 0x00, 0x00, 0x00, 0x00,
+                 0x00, 0x00, 0x00, 0x00, 0x80, 0x00,
+                 0x80, 0x00, 0x80, 0x00, 0x80, 0x00 };
+
 static void pnv_chip_power8_dt_populate(PnvChip *chip, void *fdt)
 {
     static const char compat[] = "ibm,power8-xscom\0ibm,xscom";
@@ -316,8 +321,12 @@ static void pnv_chip_power8_dt_populate(PnvChip *chip, void *fdt)
 
     for (i = 0; i < chip->nr_cores; i++) {
         PnvCore *pnv_core = chip->cores[i];
+        int offset;
+
+        offset = pnv_dt_core(chip, pnv_core, fdt);
 
-        pnv_dt_core(chip, pnv_core, fdt);
+        _FDT((fdt_setprop(fdt, offset, "ibm,pa-features",
+                           pa_features_207, sizeof(pa_features_207))));
 
         /* Interrupt Control Presenters (ICP). One per core. */
         pnv_dt_icp(chip, fdt, pnv_core->pir, CPU_CORE(pnv_core)->nr_threads);
@@ -340,8 +349,12 @@ static void pnv_chip_power9_dt_populate(PnvChip *chip, void *fdt)
 
     for (i = 0; i < chip->nr_cores; i++) {
         PnvCore *pnv_core = chip->cores[i];
+        int offset;
 
-        pnv_dt_core(chip, pnv_core, fdt);
+        offset = pnv_dt_core(chip, pnv_core, fdt);
+
+        _FDT((fdt_setprop(fdt, offset, "ibm,pa-features",
+                           pa_features_207, sizeof(pa_features_207))));
     }
 
     if (chip->ram_size) {
@@ -363,8 +376,12 @@ static void pnv_chip_power10_dt_populate(PnvChip *chip, void *fdt)
 
     for (i = 0; i < chip->nr_cores; i++) {
         PnvCore *pnv_core = chip->cores[i];
+        int offset;
+
+        offset = pnv_dt_core(chip, pnv_core, fdt);
 
-        pnv_dt_core(chip, pnv_core, fdt);
+        _FDT((fdt_setprop(fdt, offset, "ibm,pa-features",
+                           pa_features_207, sizeof(pa_features_207))));
     }
 
     if (chip->ram_size) {
-- 
2.42.0

[PATCH 7/8] ppc/pnv: Set POWER9, POWER10 ibm,pa-features bits

[Nicholas Piggin]

Copy the pa-features arrays from spapr, adjusting slightly as
described in comments.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/ppc/pnv.c   | 68 ++++++++++++++++++++++++++++++++++++++++++++++++--
 hw/ppc/spapr.c |  1 +
 2 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 0a144402d7..9db8fcd19e 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -337,6 +337,36 @@ static void pnv_chip_power8_dt_populate(PnvChip *chip, void *fdt)
     }
 }
 
+/*
+ * Same as spapr pa_features_300 except pnv always enables CI largepages bit,
+ * always disables copy/paste.
+ */
+static const uint8_t pa_features_300[] = { 66, 0,
+    /* 0: MMU|FPU|SLB|RUN|DABR|NX, 1: CILRG|fri[nzpm]|DABRX|SPRG3|SLB0|PP110 */
+    /* 2: VPM|DS205|PPR|DS202|DS206, 3: LSD|URG, 5: LE|CFAR|EB|LSQ */
+    0xf6, 0x3f, 0xc7, 0xc0, 0x00, 0xf0, /* 0 - 5 */
+    /* 6: DS207 */
+    0x80, 0x00, 0x00, 0x00, 0x00, 0x00, /* 6 - 11 */
+    /* 16: Vector */
+    0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 12 - 17 */
+    /* 18: Vec. Scalar, 20: Vec. XOR, 22: HTM */
+    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 18 - 23 */
+    /* 24: Ext. Dec, 26: 64 bit ftrs, 28: PM ftrs */
+    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 24 - 29 */
+    /* 32: LE atomic, 34: EBB + ext EBB */
+    0x00, 0x00, 0x80, 0x00, 0xC0, 0x00, /* 30 - 35 */
+    /* 40: Radix MMU */
+    0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 36 - 41 */
+    /* 42: PM, 44: PC RA, 46: SC vec'd */
+    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 42 - 47 */
+    /* 48: SIMD, 50: QP BFP, 52: String */
+    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 48 - 53 */
+    /* 54: DecFP, 56: DecI, 58: SHA */
+    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 54 - 59 */
+    /* 60: NM atomic, 62: RNG */
+    0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
+};
+
 static void pnv_chip_power9_dt_populate(PnvChip *chip, void *fdt)
 {
     static const char compat[] = "ibm,power9-xscom\0ibm,xscom";
@@ -354,7 +384,7 @@ static void pnv_chip_power9_dt_populate(PnvChip *chip, void *fdt)
         offset = pnv_dt_core(chip, pnv_core, fdt);
 
         _FDT((fdt_setprop(fdt, offset, "ibm,pa-features",
-                           pa_features_207, sizeof(pa_features_207))));
+                           pa_features_300, sizeof(pa_features_300))));
     }
 
     if (chip->ram_size) {
@@ -364,6 +394,40 @@ static void pnv_chip_power9_dt_populate(PnvChip *chip, void *fdt)
     pnv_dt_lpc(chip, fdt, 0, PNV9_LPCM_BASE(chip), PNV9_LPCM_SIZE);
 }
 
+/*
+ * Same as spapr pa_features_31 except pnv always enables CI largepages bit,
+ * always disables copy/paste.
+ */
+static const uint8_t pa_features_31[] = { 74, 0,
+    /* 0: MMU|FPU|SLB|RUN|DABR|NX, 1: CILRG|fri[nzpm]|DABRX|SPRG3|SLB0|PP110 */
+    /* 2: VPM|DS205|PPR|DS202|DS206, 3: LSD|URG, 5: LE|CFAR|EB|LSQ */
+    0xf6, 0x3f, 0xc7, 0xc0, 0x00, 0xf0, /* 0 - 5 */
+    /* 6: DS207 */
+    0x80, 0x00, 0x00, 0x00, 0x00, 0x00, /* 6 - 11 */
+    /* 16: Vector */
+    0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 12 - 17 */
+    /* 18: Vec. Scalar, 20: Vec. XOR */
+    0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 18 - 23 */
+    /* 24: Ext. Dec, 26: 64 bit ftrs, 28: PM ftrs */
+    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 24 - 29 */
+    /* 32: LE atomic, 34: EBB + ext EBB */
+    0x00, 0x00, 0x80, 0x00, 0xC0, 0x00, /* 30 - 35 */
+    /* 40: Radix MMU */
+    0x00, 0x00, 0x00, 0x00, 0x80, 0x00, /* 36 - 41 */
+    /* 42: PM, 44: PC RA, 46: SC vec'd */
+    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 42 - 47 */
+    /* 48: SIMD, 50: QP BFP, 52: String */
+    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 48 - 53 */
+    /* 54: DecFP, 56: DecI, 58: SHA */
+    0x80, 0x00, 0x80, 0x00, 0x80, 0x00, /* 54 - 59 */
+    /* 60: NM atomic, 62: RNG */
+    0x80, 0x00, 0x80, 0x00, 0x00, 0x00, /* 60 - 65 */
+    /* 68: DEXCR[SBHE|IBRTPDUS|SRAPD|NPHIE|PHIE] */
+    0x00, 0x00, 0xce, 0x00, 0x00, 0x00, /* 66 - 71 */
+    /* 72: Hash Check */
+    0x80, 0x00, /* 73 */
+};
+
 static void pnv_chip_power10_dt_populate(PnvChip *chip, void *fdt)
 {
     static const char compat[] = "ibm,power10-xscom\0ibm,xscom";
@@ -381,7 +445,7 @@ static void pnv_chip_power10_dt_populate(PnvChip *chip, void *fdt)
         offset = pnv_dt_core(chip, pnv_core, fdt);
 
         _FDT((fdt_setprop(fdt, offset, "ibm,pa-features",
-                           pa_features_207, sizeof(pa_features_207))));
+                           pa_features_31, sizeof(pa_features_31))));
     }
 
     if (chip->ram_size) {
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 2f58d8f292..40fdbd5d12 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -233,6 +233,7 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
                                  PowerPCCPU *cpu,
                                  void *fdt, int offset)
 {
+    /* These should be kept in sync with pnv */
     uint8_t pa_features_206[] = { 6, 0,
         0xf6, 0x1f, 0xc7, 0x00, 0x80, 0xc0 };
     uint8_t pa_features_207[] = { 24, 0,
-- 
2.42.0

[PATCH 8/8] ppc/pnv: Update skiboot to v7.1

[Nicholas Piggin]

This includes a number of improvements and fixes. Importantly there
is a change for QEMU platforms to permit the ChipTOD to be initialised
if it is present in the device tree. This will facilitate ChipTOD
enablement in pnv.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
[blob omitted, patch is just for comments]

 pc-bios/skiboot.lid | Bin 2527240 -> 2527328 bytes
 roms/skiboot        |   2 +-
 2 files changed, 1 insertion(+), 1 deletion(-)

diff --git a/roms/skiboot b/roms/skiboot
index 24a7eb3596..dbd5de6624 160000
--- a/roms/skiboot
+++ b/roms/skiboot
@@ -1 +1 @@
-Subproject commit 24a7eb35966d93455520bc2debdd7954314b638b
+Subproject commit dbd5de6624d7466bb67d1eb4e57bc3a8e2ad9e87
-- 
2.42.0

Re: [PATCH 2/8] ppc/spapr|pnv: Remove SAO from pa-features when running MTTCG

[David Gibson]

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

On Fri, Jan 19, 2024 at 12:09:36AM +1000, Nicholas Piggin wrote:
> SAO is a page table attribute that strengthens the memory ordering of
> accesses. QEMU with MTTCG does not implement this, so clear it in
> ibm,pa-features. There is a complication with spapr migration that is
> addressed with comments, it is not a new problem here.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  hw/ppc/pnv.c   |  5 +++++
>  hw/ppc/spapr.c | 15 +++++++++++++++
>  2 files changed, 20 insertions(+)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index b949398689..4969fbdb05 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -158,6 +158,11 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
>      char *nodename;
>      int cpus_offset = get_cpus_node(fdt);
>  
> +    if (qemu_tcg_mttcg_enabled()) {
> +        /* SSO (SAO) ordering is not supported under MTTCG. */
> +        pa_features[4 + 2] &= ~0x80;
> +    }
> +
>      nodename = g_strdup_printf("%s@%x", dc->fw_name, pc->pir);
>      offset = fdt_add_subnode(fdt, cpus_offset, nodename);
>      _FDT(offset);
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 021b1a00e1..1c79d5670d 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -284,6 +284,21 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
>          return;
>      }
>  
> +    if (qemu_tcg_mttcg_enabled()) {
> +        /*
> +         * SSO (SAO) ordering is not supported under MTTCG, so disable it.
> +         * There is no cap for this, so there is a migration bug here.
> +         * However don't disable it entirely, to allow it to be used under
> +         * KVM. This is a minor concern because:
> +         * - SAO is an obscure an rarely (if ever) used feature.
> +         * - SAO is removed from POWER10 / v3.1, so there is already a
> +         *   migration problem today.
> +         * - Linux does not test this pa-features bit today anyway, so it's
> +         *   academic.
> +         */
> +        pa_features[4 + 2] &= ~0x80;

Oof.. I see the reasoning but modifying guest visible parameters based
on host capabilities without a cap really worries me nonetheless.

> +    }
> +
>      if (ppc_hash64_has(cpu, PPC_HASH64_CI_LARGEPAGE)) {
>          /*
>           * Note: we keep CI large pages off by default because a 64K capable

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 8/8] ppc/pnv: Update skiboot to v7.1

[Cédric Le Goater]

On 1/18/24 15:09, Nicholas Piggin wrote:
> This includes a number of improvements and fixes. Importantly there
> is a change for QEMU platforms to permit the ChipTOD to be initialised
> if it is present in the device tree. This will facilitate ChipTOD
> enablement in pnv.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>


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

Thanks,

C.


> ---> [blob omitted, patch is just for comments]
> 
>   pc-bios/skiboot.lid | Bin 2527240 -> 2527328 bytes
>   roms/skiboot        |   2 +-
>   2 files changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/roms/skiboot b/roms/skiboot
> index 24a7eb3596..dbd5de6624 160000
> --- a/roms/skiboot
> +++ b/roms/skiboot
> @@ -1 +1 @@
> -Subproject commit 24a7eb35966d93455520bc2debdd7954314b638b
> +Subproject commit dbd5de6624d7466bb67d1eb4e57bc3a8e2ad9e87

Re: [PATCH 2/8] ppc/spapr|pnv: Remove SAO from pa-features when running MTTCG

[Nicholas Piggin]

On Fri Jan 19, 2024 at 10:23 AM AEST, David Gibson wrote:
> On Fri, Jan 19, 2024 at 12:09:36AM +1000, Nicholas Piggin wrote:
> > SAO is a page table attribute that strengthens the memory ordering of
> > accesses. QEMU with MTTCG does not implement this, so clear it in
> > ibm,pa-features. There is a complication with spapr migration that is
> > addressed with comments, it is not a new problem here.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >  hw/ppc/pnv.c   |  5 +++++
> >  hw/ppc/spapr.c | 15 +++++++++++++++
> >  2 files changed, 20 insertions(+)
> > 
> > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > index b949398689..4969fbdb05 100644
> > --- a/hw/ppc/pnv.c
> > +++ b/hw/ppc/pnv.c
> > @@ -158,6 +158,11 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
> >      char *nodename;
> >      int cpus_offset = get_cpus_node(fdt);
> >  
> > +    if (qemu_tcg_mttcg_enabled()) {
> > +        /* SSO (SAO) ordering is not supported under MTTCG. */
> > +        pa_features[4 + 2] &= ~0x80;
> > +    }
> > +
> >      nodename = g_strdup_printf("%s@%x", dc->fw_name, pc->pir);
> >      offset = fdt_add_subnode(fdt, cpus_offset, nodename);
> >      _FDT(offset);
> > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > index 021b1a00e1..1c79d5670d 100644
> > --- a/hw/ppc/spapr.c
> > +++ b/hw/ppc/spapr.c
> > @@ -284,6 +284,21 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
> >          return;
> >      }
> >  
> > +    if (qemu_tcg_mttcg_enabled()) {
> > +        /*
> > +         * SSO (SAO) ordering is not supported under MTTCG, so disable it.
> > +         * There is no cap for this, so there is a migration bug here.
> > +         * However don't disable it entirely, to allow it to be used under
> > +         * KVM. This is a minor concern because:
> > +         * - SAO is an obscure an rarely (if ever) used feature.
> > +         * - SAO is removed from POWER10 / v3.1, so there is already a
> > +         *   migration problem today.
> > +         * - Linux does not test this pa-features bit today anyway, so it's
> > +         *   academic.
> > +         */
> > +        pa_features[4 + 2] &= ~0x80;
>
> Oof.. I see the reasoning but modifying guest visible parameters based
> on host capabilities without a cap really worries me nonetheless.

Yeah :( It's not a new problem, but changing it based on host
does make it look uglier I guess.

Other option could be to just disable it always. I don't mind
but someone did mention experimenting with it when I asked
about removing support from Linux. They could still test with
bare metal, and if ever started actually being used then we
could add a cap for it.

Thanks,
Nick

Re: [PATCH 2/8] ppc/spapr|pnv: Remove SAO from pa-features when running MTTCG

[David Gibson]

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

On Tue, Jan 23, 2024 at 11:57:56AM +1000, Nicholas Piggin wrote:
> On Fri Jan 19, 2024 at 10:23 AM AEST, David Gibson wrote:
> > On Fri, Jan 19, 2024 at 12:09:36AM +1000, Nicholas Piggin wrote:
> > > SAO is a page table attribute that strengthens the memory ordering of
> > > accesses. QEMU with MTTCG does not implement this, so clear it in
> > > ibm,pa-features. There is a complication with spapr migration that is
> > > addressed with comments, it is not a new problem here.
> > > 
> > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > > ---
> > >  hw/ppc/pnv.c   |  5 +++++
> > >  hw/ppc/spapr.c | 15 +++++++++++++++
> > >  2 files changed, 20 insertions(+)
> > > 
> > > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > > index b949398689..4969fbdb05 100644
> > > --- a/hw/ppc/pnv.c
> > > +++ b/hw/ppc/pnv.c
> > > @@ -158,6 +158,11 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
> > >      char *nodename;
> > >      int cpus_offset = get_cpus_node(fdt);
> > >  
> > > +    if (qemu_tcg_mttcg_enabled()) {
> > > +        /* SSO (SAO) ordering is not supported under MTTCG. */
> > > +        pa_features[4 + 2] &= ~0x80;
> > > +    }
> > > +
> > >      nodename = g_strdup_printf("%s@%x", dc->fw_name, pc->pir);
> > >      offset = fdt_add_subnode(fdt, cpus_offset, nodename);
> > >      _FDT(offset);
> > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > index 021b1a00e1..1c79d5670d 100644
> > > --- a/hw/ppc/spapr.c
> > > +++ b/hw/ppc/spapr.c
> > > @@ -284,6 +284,21 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
> > >          return;
> > >      }
> > >  
> > > +    if (qemu_tcg_mttcg_enabled()) {
> > > +        /*
> > > +         * SSO (SAO) ordering is not supported under MTTCG, so disable it.
> > > +         * There is no cap for this, so there is a migration bug here.
> > > +         * However don't disable it entirely, to allow it to be used under
> > > +         * KVM. This is a minor concern because:
> > > +         * - SAO is an obscure an rarely (if ever) used feature.
> > > +         * - SAO is removed from POWER10 / v3.1, so there is already a
> > > +         *   migration problem today.
> > > +         * - Linux does not test this pa-features bit today anyway, so it's
> > > +         *   academic.
> > > +         */
> > > +        pa_features[4 + 2] &= ~0x80;
> >
> > Oof.. I see the reasoning but modifying guest visible parameters based
> > on host capabilities without a cap really worries me nonetheless.
> 
> Yeah :( It's not a new problem, but changing it based on host
> does make it look uglier I guess.

It's not really about whether it looks uglier, it's the fact that any
dependency of guest visible aspects of the VM on host properties is a
potential landmine for migration.

The qemu migration model is - pretty fundamentally - that the VM
should look and behave, from the point of view of the guest, the same
before and after migration.  If the behaviour of the VM changes based
on host properties it breaks that assumption, and it does so in a way
that the user can't control or even easily predict.  Tools such as
libvirt, or even qemu itself, can't verify that the migration is valid
if there are effectively invisible parameters to the VM configuration
that come from the host instead of the command line.

> Other option could be to just disable it always. I don't mind
> but someone did mention experimenting with it when I asked
> about removing support from Linux. They could still test with
> bare metal, and if ever started actually being used then we
> could add a cap for it.

I think that's a better idea.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/8] ppc/spapr|pnv: Remove SAO from pa-features when running MTTCG

[Nicholas Piggin]

On Thu Jan 25, 2024 at 1:11 PM AEST, David Gibson wrote:
> On Tue, Jan 23, 2024 at 11:57:56AM +1000, Nicholas Piggin wrote:
> > On Fri Jan 19, 2024 at 10:23 AM AEST, David Gibson wrote:
> > > On Fri, Jan 19, 2024 at 12:09:36AM +1000, Nicholas Piggin wrote:
> > > > SAO is a page table attribute that strengthens the memory ordering of
> > > > accesses. QEMU with MTTCG does not implement this, so clear it in
> > > > ibm,pa-features. There is a complication with spapr migration that is
> > > > addressed with comments, it is not a new problem here.
> > > > 
> > > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > > > ---
> > > >  hw/ppc/pnv.c   |  5 +++++
> > > >  hw/ppc/spapr.c | 15 +++++++++++++++
> > > >  2 files changed, 20 insertions(+)
> > > > 
> > > > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > > > index b949398689..4969fbdb05 100644
> > > > --- a/hw/ppc/pnv.c
> > > > +++ b/hw/ppc/pnv.c
> > > > @@ -158,6 +158,11 @@ static void pnv_dt_core(PnvChip *chip, PnvCore *pc, void *fdt)
> > > >      char *nodename;
> > > >      int cpus_offset = get_cpus_node(fdt);
> > > >  
> > > > +    if (qemu_tcg_mttcg_enabled()) {
> > > > +        /* SSO (SAO) ordering is not supported under MTTCG. */
> > > > +        pa_features[4 + 2] &= ~0x80;
> > > > +    }
> > > > +
> > > >      nodename = g_strdup_printf("%s@%x", dc->fw_name, pc->pir);
> > > >      offset = fdt_add_subnode(fdt, cpus_offset, nodename);
> > > >      _FDT(offset);
> > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> > > > index 021b1a00e1..1c79d5670d 100644
> > > > --- a/hw/ppc/spapr.c
> > > > +++ b/hw/ppc/spapr.c
> > > > @@ -284,6 +284,21 @@ static void spapr_dt_pa_features(SpaprMachineState *spapr,
> > > >          return;
> > > >      }
> > > >  
> > > > +    if (qemu_tcg_mttcg_enabled()) {
> > > > +        /*
> > > > +         * SSO (SAO) ordering is not supported under MTTCG, so disable it.
> > > > +         * There is no cap for this, so there is a migration bug here.
> > > > +         * However don't disable it entirely, to allow it to be used under
> > > > +         * KVM. This is a minor concern because:
> > > > +         * - SAO is an obscure an rarely (if ever) used feature.
> > > > +         * - SAO is removed from POWER10 / v3.1, so there is already a
> > > > +         *   migration problem today.
> > > > +         * - Linux does not test this pa-features bit today anyway, so it's
> > > > +         *   academic.
> > > > +         */
> > > > +        pa_features[4 + 2] &= ~0x80;
> > >
> > > Oof.. I see the reasoning but modifying guest visible parameters based
> > > on host capabilities without a cap really worries me nonetheless.
> > 
> > Yeah :( It's not a new problem, but changing it based on host
> > does make it look uglier I guess.
>
> It's not really about whether it looks uglier, it's the fact that any
> dependency of guest visible aspects of the VM on host properties is a
> potential landmine for migration.
>
> The qemu migration model is - pretty fundamentally - that the VM
> should look and behave, from the point of view of the guest, the same
> before and after migration.  If the behaviour of the VM changes based
> on host properties it breaks that assumption, and it does so in a way
> that the user can't control or even easily predict.  Tools such as
> libvirt, or even qemu itself, can't verify that the migration is valid
> if there are effectively invisible parameters to the VM configuration
> that come from the host instead of the command line.

I agree, you did manage to drill this lesson in.

What I meant was that -- today we have a problem where a target can
run on a host with buggy implementation, whether boot or migration.
This patch addresses the boot case. Migration case is still broken,
arguably the situation is still improved. Anyway...

> > Other option could be to just disable it always. I don't mind
> > but someone did mention experimenting with it when I asked
> > about removing support from Linux. They could still test with
> > bare metal, and if ever started actually being used then we
> > could add a cap for it.
>
> I think that's a better idea.

Alright we'll go that way.

Thanks,
Nick