* [PATCH v3 1/4] target/ppc: Fix masked PVR matching
2022-03-07 6:55 [PATCH v3 0/4] Fix PVR matching, add AIL cap compatibility Nicholas Piggin
@ 2022-03-07 6:55 ` Nicholas Piggin
2022-03-10 17:46 ` Cédric Le Goater
` (2 more replies)
2022-03-07 6:55 ` [PATCH v3 2/4] target/ppc: Remove chip type field from POWER9 DD2.0 PVR Nicholas Piggin
` (2 subsequent siblings)
3 siblings, 3 replies; 9+ messages in thread
From: Nicholas Piggin @ 2022-03-07 6:55 UTC (permalink / raw)
To: qemu-ppc
Cc: Alexey Kardashevskiy, Cédric Le Goater, qemu-devel,
Nicholas Piggin, David Gibson
The pvr_match for a CPU class is not supposed to just match for any
CPU in the family, but rather whether this particular CPU class is the
best match in the family.
Prior to this fix, e.g., a POWER9 DD2.3 KVM host matches to the
power9_v1.0 class (because that's first in the list). After the patch,
it matches the power9_v2.0 class.
Fixes: 03ae4133ab8 ("target-ppc: Add pvr_match() callback")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
target/ppc/cpu_init.c | 51 ++++++++++++++++++++++++++++---------------
1 file changed, 34 insertions(+), 17 deletions(-)
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 073fd10168..83ca741bea 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -5910,13 +5910,14 @@ static void init_proc_POWER7(CPUPPCState *env)
static bool ppc_pvr_match_power7(PowerPCCPUClass *pcc, uint32_t pvr)
{
- if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER7P_BASE) {
- return true;
- }
- if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER7_BASE) {
- return true;
+ uint32_t base = pvr & CPU_POWERPC_POWER_SERVER_MASK;
+ uint32_t pcc_base = pcc->pvr & CPU_POWERPC_POWER_SERVER_MASK;
+
+ if (base != pcc_base) {
+ return false;
}
- return false;
+
+ return true;
}
static bool cpu_has_work_POWER7(CPUState *cs)
@@ -6070,16 +6071,14 @@ static void init_proc_POWER8(CPUPPCState *env)
static bool ppc_pvr_match_power8(PowerPCCPUClass *pcc, uint32_t pvr)
{
- if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER8NVL_BASE) {
- return true;
- }
- if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER8E_BASE) {
- return true;
- }
- if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER8_BASE) {
- return true;
+ uint32_t base = pvr & CPU_POWERPC_POWER_SERVER_MASK;
+ uint32_t pcc_base = pcc->pvr & CPU_POWERPC_POWER_SERVER_MASK;
+
+ if (base != pcc_base) {
+ return false;
}
- return false;
+
+ return true;
}
static bool cpu_has_work_POWER8(CPUState *cs)
@@ -6277,9 +6276,18 @@ static void init_proc_POWER9(CPUPPCState *env)
static bool ppc_pvr_match_power9(PowerPCCPUClass *pcc, uint32_t pvr)
{
- if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER9_BASE) {
+ uint32_t base = pvr & CPU_POWERPC_POWER_SERVER_MASK;
+ uint32_t pcc_base = pcc->pvr & CPU_POWERPC_POWER_SERVER_MASK;
+
+ if (base != pcc_base) {
+ return false;
+ }
+
+ if ((pvr & 0x0f00) == (pcc->pvr & 0x0f00)) {
+ /* Major DD version matches to power9_v1.0 and power9_v2.0 */
return true;
}
+
return false;
}
@@ -6489,9 +6497,18 @@ static void init_proc_POWER10(CPUPPCState *env)
static bool ppc_pvr_match_power10(PowerPCCPUClass *pcc, uint32_t pvr)
{
- if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER10_BASE) {
+ uint32_t base = pvr & CPU_POWERPC_POWER_SERVER_MASK;
+ uint32_t pcc_base = pcc->pvr & CPU_POWERPC_POWER_SERVER_MASK;
+
+ if (base != pcc_base) {
+ return false;
+ }
+
+ if ((pvr & 0x0f00) == (pcc->pvr & 0x0f00)) {
+ /* Major DD version matches to power10_v1.0 and power10_v2.0 */
return true;
}
+
return false;
}
--
2.23.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v3 1/4] target/ppc: Fix masked PVR matching
2022-03-07 6:55 ` [PATCH v3 1/4] target/ppc: Fix masked PVR matching Nicholas Piggin
@ 2022-03-10 17:46 ` Cédric Le Goater
2022-03-11 3:03 ` Alexey Kardashevskiy
2022-03-12 8:45 ` David Gibson
2 siblings, 0 replies; 9+ messages in thread
From: Cédric Le Goater @ 2022-03-10 17:46 UTC (permalink / raw)
To: Nicholas Piggin, qemu-ppc
Cc: Alexey Kardashevskiy, Cédric Le Goater, qemu-devel,
David Gibson
Hello Nick,
On 3/7/22 07:55, Nicholas Piggin wrote:
> The pvr_match for a CPU class is not supposed to just match for any
> CPU in the family, but rather whether this particular CPU class is the
> best match in the family.
>
> Prior to this fix, e.g., a POWER9 DD2.3 KVM host matches to the
> power9_v1.0 class (because that's first in the list). After the patch,
> it matches the power9_v2.0 class.
>
> Fixes: 03ae4133ab8 ("target-ppc: Add pvr_match() callback")
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>> ---
> target/ppc/cpu_init.c | 51 ++++++++++++++++++++++++++++---------------
> 1 file changed, 34 insertions(+), 17 deletions(-)
make check-qtest-ppc64 fails with :
―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――― ✀ ――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
stderr:
qemu-system-ppc64: invalid CPU model 'power8nvl_v1.0-powerpc64-cpu' for powernv8 machine
Broken pipe
TAP parsing error: Too few tests run (expected 6, got 1)
(test program exited with status code -6)
――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――
# QTEST_QEMU_BINARY=build/ppc64-softmmu/qemu-system-ppc64 QTEST_QEMU_IMG=qemu-img build/tests/qtest/pnv-xscom-test
1..6
# Start of ppc64 tests
# Start of pnv-xscom tests
# Start of cfam_id tests
# starting QEMU: exec build/ppc64-softmmu/qemu-system-ppc64 -qtest unix:/tmp/qtest-2994797.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-2994797.qmp,id=char0 -mon chardev=char0,mode=control -display none -M powernv8 -accel tcg -cpu POWER8 -accel qtest
ok 1 /ppc64/pnv-xscom/cfam_id/POWER8
# starting QEMU: exec build/ppc64-softmmu/qemu-system-ppc64 -qtest unix:/tmp/qtest-2994797.sock -qtest-log /dev/null -chardev socket,path=/tmp/qtest-2994797.qmp,id=char0 -mon chardev=char0,mode=control -display none -M powernv8 -accel tcg -cpu POWER8NVL -accel qtest
qemu-system-ppc64: invalid CPU model 'power8nvl_v1.0-powerpc64-cpu' for powernv8 machine
Broken pipe
Aborted (core dumped)
Thanks,
C.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v3 1/4] target/ppc: Fix masked PVR matching
2022-03-07 6:55 ` [PATCH v3 1/4] target/ppc: Fix masked PVR matching Nicholas Piggin
2022-03-10 17:46 ` Cédric Le Goater
@ 2022-03-11 3:03 ` Alexey Kardashevskiy
2022-03-12 8:45 ` David Gibson
2 siblings, 0 replies; 9+ messages in thread
From: Alexey Kardashevskiy @ 2022-03-11 3:03 UTC (permalink / raw)
To: Nicholas Piggin, qemu-ppc; +Cc: Cédric Le Goater, qemu-devel, David Gibson
On 3/7/22 17:55, Nicholas Piggin wrote:
> The pvr_match for a CPU class is not supposed to just match for any
> CPU in the family, but rather whether this particular CPU class is the
> best match in the family.
>
> Prior to this fix, e.g., a POWER9 DD2.3 KVM host matches to the
> power9_v1.0 class (because that's first in the list). After the patch,
> it matches the power9_v2.0 class.
So if we get now another revision of p10, this just won't work at all
instead of matching DD1. Not that we have a reasonable chance of this
happening though...
> Fixes: 03ae4133ab8 ("target-ppc: Add pvr_match() callback")
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> target/ppc/cpu_init.c | 51 ++++++++++++++++++++++++++++---------------
> 1 file changed, 34 insertions(+), 17 deletions(-)
>
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 073fd10168..83ca741bea 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -5910,13 +5910,14 @@ static void init_proc_POWER7(CPUPPCState *env)
>
> static bool ppc_pvr_match_power7(PowerPCCPUClass *pcc, uint32_t pvr)
> {
> - if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER7P_BASE) {
> - return true;
> - }
> - if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER7_BASE) {
> - return true;
> + uint32_t base = pvr & CPU_POWERPC_POWER_SERVER_MASK;
> + uint32_t pcc_base = pcc->pvr & CPU_POWERPC_POWER_SERVER_MASK;
> +
> + if (base != pcc_base) {
> + return false;
> }
> - return false;
> +
> + return true;
> }
>
> static bool cpu_has_work_POWER7(CPUState *cs)
> @@ -6070,16 +6071,14 @@ static void init_proc_POWER8(CPUPPCState *env)
>
> static bool ppc_pvr_match_power8(PowerPCCPUClass *pcc, uint32_t pvr)
> {
> - if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER8NVL_BASE) {
> - return true;
> - }
> - if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER8E_BASE) {
> - return true;
> - }
> - if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER8_BASE) {
> - return true;
> + uint32_t base = pvr & CPU_POWERPC_POWER_SERVER_MASK;
> + uint32_t pcc_base = pcc->pvr & CPU_POWERPC_POWER_SERVER_MASK;
(after looking at failing
"QTEST_QEMU_BINARY=~/pbuild/qemu-localhost-ppc64/qemu-system-ppc64
~/pbuild/qemu-localhost-ppc64/tests/qtest/pnv-xscom-test")
So this breaks using different family masks for the same machine type,
like, 0x004[BCD]xxxx for pnv_machine_power8_class_init()'s
POWERPC_CPU_TYPE_NAME("power8_v2.0").
So I'd keep P7/P8 where they are today and create a new machine type for
every family mask (higher 16 bits of PVR). Thanks,
> +
> + if (base != pcc_base) {
> + return false;
> }
> - return false;
> +
> + return true;
> }
>
> static bool cpu_has_work_POWER8(CPUState *cs)
> @@ -6277,9 +6276,18 @@ static void init_proc_POWER9(CPUPPCState *env)
>
> static bool ppc_pvr_match_power9(PowerPCCPUClass *pcc, uint32_t pvr)
> {
> - if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER9_BASE) {
> + uint32_t base = pvr & CPU_POWERPC_POWER_SERVER_MASK;
> + uint32_t pcc_base = pcc->pvr & CPU_POWERPC_POWER_SERVER_MASK;
> +
> + if (base != pcc_base) {
> + return false;
> + }
> +
> + if ((pvr & 0x0f00) == (pcc->pvr & 0x0f00)) {
> + /* Major DD version matches to power9_v1.0 and power9_v2.0 */
> return true;
> }
> +
> return false;
> }
>
> @@ -6489,9 +6497,18 @@ static void init_proc_POWER10(CPUPPCState *env)
>
> static bool ppc_pvr_match_power10(PowerPCCPUClass *pcc, uint32_t pvr)
> {
> - if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER10_BASE) {
> + uint32_t base = pvr & CPU_POWERPC_POWER_SERVER_MASK;
> + uint32_t pcc_base = pcc->pvr & CPU_POWERPC_POWER_SERVER_MASK;
> +
> + if (base != pcc_base) {
> + return false;
> + }
> +
> + if ((pvr & 0x0f00) == (pcc->pvr & 0x0f00)) {
> + /* Major DD version matches to power10_v1.0 and power10_v2.0 */
> return true;
> }
> +
> return false;
> }
>
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v3 1/4] target/ppc: Fix masked PVR matching
2022-03-07 6:55 ` [PATCH v3 1/4] target/ppc: Fix masked PVR matching Nicholas Piggin
2022-03-10 17:46 ` Cédric Le Goater
2022-03-11 3:03 ` Alexey Kardashevskiy
@ 2022-03-12 8:45 ` David Gibson
2 siblings, 0 replies; 9+ messages in thread
From: David Gibson @ 2022-03-12 8:45 UTC (permalink / raw)
To: Nicholas Piggin
Cc: Alexey Kardashevskiy, Cédric Le Goater, qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 4071 bytes --]
On Mon, Mar 07, 2022 at 04:55:24PM +1000, Nicholas Piggin wrote:
> The pvr_match for a CPU class is not supposed to just match for any
> CPU in the family, but rather whether this particular CPU class is the
> best match in the family.
Ok... but I don't see how that question can possibly be answered
without reference to all the available options.
> Prior to this fix, e.g., a POWER9 DD2.3 KVM host matches to the
> power9_v1.0 class (because that's first in the list). After the patch,
> it matches the power9_v2.0 class.
.. so, doesn't this indicate a problem in the check order, rather than
a problem with the matching function?
>
> Fixes: 03ae4133ab8 ("target-ppc: Add pvr_match() callback")
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> target/ppc/cpu_init.c | 51 ++++++++++++++++++++++++++++---------------
> 1 file changed, 34 insertions(+), 17 deletions(-)
>
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index 073fd10168..83ca741bea 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -5910,13 +5910,14 @@ static void init_proc_POWER7(CPUPPCState *env)
>
> static bool ppc_pvr_match_power7(PowerPCCPUClass *pcc, uint32_t pvr)
> {
> - if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER7P_BASE) {
> - return true;
> - }
> - if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER7_BASE) {
> - return true;
> + uint32_t base = pvr & CPU_POWERPC_POWER_SERVER_MASK;
> + uint32_t pcc_base = pcc->pvr & CPU_POWERPC_POWER_SERVER_MASK;
> +
> + if (base != pcc_base) {
> + return false;
> }
> - return false;
> +
> + return true;
> }
>
> static bool cpu_has_work_POWER7(CPUState *cs)
> @@ -6070,16 +6071,14 @@ static void init_proc_POWER8(CPUPPCState *env)
>
> static bool ppc_pvr_match_power8(PowerPCCPUClass *pcc, uint32_t pvr)
> {
> - if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER8NVL_BASE) {
> - return true;
> - }
> - if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER8E_BASE) {
> - return true;
> - }
> - if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER8_BASE) {
> - return true;
> + uint32_t base = pvr & CPU_POWERPC_POWER_SERVER_MASK;
> + uint32_t pcc_base = pcc->pvr & CPU_POWERPC_POWER_SERVER_MASK;
> +
> + if (base != pcc_base) {
> + return false;
> }
> - return false;
> +
> + return true;
> }
>
> static bool cpu_has_work_POWER8(CPUState *cs)
> @@ -6277,9 +6276,18 @@ static void init_proc_POWER9(CPUPPCState *env)
>
> static bool ppc_pvr_match_power9(PowerPCCPUClass *pcc, uint32_t pvr)
> {
> - if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER9_BASE) {
> + uint32_t base = pvr & CPU_POWERPC_POWER_SERVER_MASK;
> + uint32_t pcc_base = pcc->pvr & CPU_POWERPC_POWER_SERVER_MASK;
> +
> + if (base != pcc_base) {
> + return false;
> + }
> +
> + if ((pvr & 0x0f00) == (pcc->pvr & 0x0f00)) {
> + /* Major DD version matches to power9_v1.0 and power9_v2.0 */
> return true;
> }
> +
> return false;
> }
>
> @@ -6489,9 +6497,18 @@ static void init_proc_POWER10(CPUPPCState *env)
>
> static bool ppc_pvr_match_power10(PowerPCCPUClass *pcc, uint32_t pvr)
> {
> - if ((pvr & CPU_POWERPC_POWER_SERVER_MASK) == CPU_POWERPC_POWER10_BASE) {
> + uint32_t base = pvr & CPU_POWERPC_POWER_SERVER_MASK;
> + uint32_t pcc_base = pcc->pvr & CPU_POWERPC_POWER_SERVER_MASK;
> +
> + if (base != pcc_base) {
> + return false;
> + }
> +
> + if ((pvr & 0x0f00) == (pcc->pvr & 0x0f00)) {
> + /* Major DD version matches to power10_v1.0 and power10_v2.0 */
> return true;
> }
> +
> return false;
> }
>
--
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 --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 2/4] target/ppc: Remove chip type field from POWER9 DD2.0 PVR
2022-03-07 6:55 [PATCH v3 0/4] Fix PVR matching, add AIL cap compatibility Nicholas Piggin
2022-03-07 6:55 ` [PATCH v3 1/4] target/ppc: Fix masked PVR matching Nicholas Piggin
@ 2022-03-07 6:55 ` Nicholas Piggin
2022-03-12 8:50 ` David Gibson
2022-03-07 6:55 ` [PATCH v3 3/4] target/ppc: Add POWER9 DD2.2 model Nicholas Piggin
2022-03-07 6:55 ` [PATCH v3 4/4] spapr: Add SPAPR_CAP_AIL_MODE_3 for AIL mode 3 support for H_SET_MODE hcall Nicholas Piggin
3 siblings, 1 reply; 9+ messages in thread
From: Nicholas Piggin @ 2022-03-07 6:55 UTC (permalink / raw)
To: qemu-ppc
Cc: Alexey Kardashevskiy, Cédric Le Goater, qemu-devel,
Nicholas Piggin, David Gibson
The POWER9 DD2.0 PVR does not follow the same format as the other
POWER9/10 PVRs, it includes a non-zero value in the "chip type" field.
This does not cause problems because the pvr check is masks it out and
matches against the base, but it's a small inconsistency. Zero the
field.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
target/ppc/cpu-models.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/target/ppc/cpu-models.h b/target/ppc/cpu-models.h
index 76775a74a9..b42f2ab162 100644
--- a/target/ppc/cpu-models.h
+++ b/target/ppc/cpu-models.h
@@ -349,7 +349,7 @@ enum {
CPU_POWERPC_POWER8NVL_v10 = 0x004C0100,
CPU_POWERPC_POWER9_BASE = 0x004E0000,
CPU_POWERPC_POWER9_DD1 = 0x004E0100,
- CPU_POWERPC_POWER9_DD20 = 0x004E1200,
+ CPU_POWERPC_POWER9_DD20 = 0x004E0200,
CPU_POWERPC_POWER10_BASE = 0x00800000,
CPU_POWERPC_POWER10_DD1 = 0x00800100,
CPU_POWERPC_POWER10_DD20 = 0x00800200,
--
2.23.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v3 2/4] target/ppc: Remove chip type field from POWER9 DD2.0 PVR
2022-03-07 6:55 ` [PATCH v3 2/4] target/ppc: Remove chip type field from POWER9 DD2.0 PVR Nicholas Piggin
@ 2022-03-12 8:50 ` David Gibson
0 siblings, 0 replies; 9+ messages in thread
From: David Gibson @ 2022-03-12 8:50 UTC (permalink / raw)
To: Nicholas Piggin
Cc: Alexey Kardashevskiy, Cédric Le Goater, qemu-ppc, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 1549 bytes --]
On Mon, Mar 07, 2022 at 04:55:25PM +1000, Nicholas Piggin wrote:
> The POWER9 DD2.0 PVR does not follow the same format as the other
> POWER9/10 PVRs, it includes a non-zero value in the "chip type" field.
I'm unclear whether you're describing the hardware PVR here, or the
value in qemu.
> This does not cause problems because the pvr check is masks it out and
> matches against the base, but it's a small inconsistency. Zero the
> field.
I assume this is making the qemu model match the hardware, but that's
not entirely clear to me from the commit message.
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> target/ppc/cpu-models.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/ppc/cpu-models.h b/target/ppc/cpu-models.h
> index 76775a74a9..b42f2ab162 100644
> --- a/target/ppc/cpu-models.h
> +++ b/target/ppc/cpu-models.h
> @@ -349,7 +349,7 @@ enum {
> CPU_POWERPC_POWER8NVL_v10 = 0x004C0100,
> CPU_POWERPC_POWER9_BASE = 0x004E0000,
> CPU_POWERPC_POWER9_DD1 = 0x004E0100,
> - CPU_POWERPC_POWER9_DD20 = 0x004E1200,
> + CPU_POWERPC_POWER9_DD20 = 0x004E0200,
> CPU_POWERPC_POWER10_BASE = 0x00800000,
> CPU_POWERPC_POWER10_DD1 = 0x00800100,
> CPU_POWERPC_POWER10_DD20 = 0x00800200,
--
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 --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v3 3/4] target/ppc: Add POWER9 DD2.2 model
2022-03-07 6:55 [PATCH v3 0/4] Fix PVR matching, add AIL cap compatibility Nicholas Piggin
2022-03-07 6:55 ` [PATCH v3 1/4] target/ppc: Fix masked PVR matching Nicholas Piggin
2022-03-07 6:55 ` [PATCH v3 2/4] target/ppc: Remove chip type field from POWER9 DD2.0 PVR Nicholas Piggin
@ 2022-03-07 6:55 ` Nicholas Piggin
2022-03-07 6:55 ` [PATCH v3 4/4] spapr: Add SPAPR_CAP_AIL_MODE_3 for AIL mode 3 support for H_SET_MODE hcall Nicholas Piggin
3 siblings, 0 replies; 9+ messages in thread
From: Nicholas Piggin @ 2022-03-07 6:55 UTC (permalink / raw)
To: qemu-ppc
Cc: Alexey Kardashevskiy, Cédric Le Goater, qemu-devel,
Nicholas Piggin, David Gibson
POWER9 DD2.1 and earlier had significant limitations when running KVM,
including lack of "mixed mode" MMU support (ability to run HPT and RPT
mode on threads of the same core), and a translation prefetch issue
which is worked around by disabling "AIL" mode for the guest.
These processors are not widely available, and it's difficult to deal
with all these quirks in qemu +/- KVM, so create a POWER9 DD2.2 CPU
and make it the default POWER9 CPU.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
hw/ppc/pnv.c | 2 +-
hw/ppc/pnv_core.c | 2 +-
hw/ppc/spapr.c | 2 +-
hw/ppc/spapr_cpu_core.c | 1 +
include/hw/ppc/pnv.h | 2 +-
target/ppc/cpu-models.c | 4 +++-
target/ppc/cpu-models.h | 1 +
target/ppc/cpu_init.c | 21 +++++++++++++++++++--
tests/qtest/device-plug-test.c | 4 ++--
9 files changed, 30 insertions(+), 9 deletions(-)
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 0ac86e104f..a7217b6ffd 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -2144,7 +2144,7 @@ static void pnv_machine_power9_class_init(ObjectClass *oc, void *data)
static const char compat[] = "qemu,powernv9\0ibm,powernv";
mc->desc = "IBM PowerNV (Non-Virtualized) POWER9";
- mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power9_v2.0");
+ mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power9_v2.2");
xfc->match_nvt = pnv_match_nvt;
mc->alias = "powernv";
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index 19e8eb885f..a350cfc0b6 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -346,7 +346,7 @@ static const TypeInfo pnv_core_infos[] = {
DEFINE_PNV_CORE_TYPE(power8, "power8e_v2.1"),
DEFINE_PNV_CORE_TYPE(power8, "power8_v2.0"),
DEFINE_PNV_CORE_TYPE(power8, "power8nvl_v1.0"),
- DEFINE_PNV_CORE_TYPE(power9, "power9_v2.0"),
+ DEFINE_PNV_CORE_TYPE(power9, "power9_v2.2"),
DEFINE_PNV_CORE_TYPE(power10, "power10_v2.0"),
};
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 4cc204f90d..69b0a6f6d6 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4599,7 +4599,7 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
smc->dr_lmb_enabled = true;
smc->update_dt_enabled = true;
- mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power9_v2.0");
+ mc->default_cpu_type = POWERPC_CPU_TYPE_NAME("power9_v2.2");
mc->has_hotpluggable_cpus = true;
mc->nvdimm_supported = true;
smc->resize_hpt_default = SPAPR_RESIZE_HPT_ENABLED;
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index ed84713960..fe18127d1d 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -387,6 +387,7 @@ static const TypeInfo spapr_cpu_core_type_infos[] = {
DEFINE_SPAPR_CPU_CORE_TYPE("power8nvl_v1.0"),
DEFINE_SPAPR_CPU_CORE_TYPE("power9_v1.0"),
DEFINE_SPAPR_CPU_CORE_TYPE("power9_v2.0"),
+ DEFINE_SPAPR_CPU_CORE_TYPE("power9_v2.2"),
DEFINE_SPAPR_CPU_CORE_TYPE("power10_v1.0"),
DEFINE_SPAPR_CPU_CORE_TYPE("power10_v2.0"),
#ifdef CONFIG_KVM
diff --git a/include/hw/ppc/pnv.h b/include/hw/ppc/pnv.h
index 1e34ddd502..7f7b8ec4df 100644
--- a/include/hw/ppc/pnv.h
+++ b/include/hw/ppc/pnv.h
@@ -180,7 +180,7 @@ DECLARE_INSTANCE_CHECKER(PnvChip, PNV_CHIP_POWER8,
DECLARE_INSTANCE_CHECKER(PnvChip, PNV_CHIP_POWER8NVL,
TYPE_PNV_CHIP_POWER8NVL)
-#define TYPE_PNV_CHIP_POWER9 PNV_CHIP_TYPE_NAME("power9_v2.0")
+#define TYPE_PNV_CHIP_POWER9 PNV_CHIP_TYPE_NAME("power9_v2.2")
DECLARE_INSTANCE_CHECKER(PnvChip, PNV_CHIP_POWER9,
TYPE_PNV_CHIP_POWER9)
diff --git a/target/ppc/cpu-models.c b/target/ppc/cpu-models.c
index 976be5e0d1..3d136859f0 100644
--- a/target/ppc/cpu-models.c
+++ b/target/ppc/cpu-models.c
@@ -732,6 +732,8 @@
"POWER9 v1.0")
POWERPC_DEF("power9_v2.0", CPU_POWERPC_POWER9_DD20, POWER9,
"POWER9 v2.0")
+ POWERPC_DEF("power9_v2.2", CPU_POWERPC_POWER9_DD22, POWER9,
+ "POWER9 v2.2")
POWERPC_DEF("power10_v1.0", CPU_POWERPC_POWER10_DD1, POWER10,
"POWER10 v1.0")
POWERPC_DEF("power10_v2.0", CPU_POWERPC_POWER10_DD20, POWER10,
@@ -908,7 +910,7 @@ PowerPCCPUAlias ppc_cpu_aliases[] = {
{ "power8e", "power8e_v2.1" },
{ "power8", "power8_v2.0" },
{ "power8nvl", "power8nvl_v1.0" },
- { "power9", "power9_v2.0" },
+ { "power9", "power9_v2.2" },
{ "power10", "power10_v2.0" },
#endif
diff --git a/target/ppc/cpu-models.h b/target/ppc/cpu-models.h
index b42f2ab162..20be2a4348 100644
--- a/target/ppc/cpu-models.h
+++ b/target/ppc/cpu-models.h
@@ -350,6 +350,7 @@ enum {
CPU_POWERPC_POWER9_BASE = 0x004E0000,
CPU_POWERPC_POWER9_DD1 = 0x004E0100,
CPU_POWERPC_POWER9_DD20 = 0x004E0200,
+ CPU_POWERPC_POWER9_DD22 = 0x004E0202,
CPU_POWERPC_POWER10_BASE = 0x00800000,
CPU_POWERPC_POWER10_DD1 = 0x00800100,
CPU_POWERPC_POWER10_DD20 = 0x00800200,
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index 83ca741bea..eee5d9cffb 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -6283,9 +6283,26 @@ static bool ppc_pvr_match_power9(PowerPCCPUClass *pcc, uint32_t pvr)
return false;
}
- if ((pvr & 0x0f00) == (pcc->pvr & 0x0f00)) {
- /* Major DD version matches to power9_v1.0 and power9_v2.0 */
+ if ((pvr & 0x0f00) != (pcc->pvr & 0x0f00)) {
+ /* Major DD version does not match */
+ return false;
+ }
+
+ if ((pvr & 0x0f00) == 0x100) {
+ /* DD1.x always matches power9_v1.0 */
return true;
+ } else if ((pvr & 0x0f00) == 0x200) {
+ if ((pvr & 0xf) < 2) {
+ /* DD2.0, DD2.1 match power9_v2.0 */
+ if ((pcc->pvr & 0xf) == 0) {
+ return true;
+ }
+ } else {
+ /* DD2.2, DD2.3 (and any higher) match power9_v2.2 */
+ if ((pcc->pvr & 0xf) == 2) {
+ return true;
+ }
+ }
}
return false;
diff --git a/tests/qtest/device-plug-test.c b/tests/qtest/device-plug-test.c
index 404a92e132..30adc91d12 100644
--- a/tests/qtest/device-plug-test.c
+++ b/tests/qtest/device-plug-test.c
@@ -124,8 +124,8 @@ static void test_spapr_cpu_unplug_request(void)
{
QTestState *qtest;
- qtest = qtest_initf("-cpu power9_v2.0 -smp 1,maxcpus=2 "
- "-device power9_v2.0-spapr-cpu-core,core-id=1,id=dev0");
+ qtest = qtest_initf("-cpu power9_v2.2 -smp 1,maxcpus=2 "
+ "-device power9_v2.2-spapr-cpu-core,core-id=1,id=dev0");
/* similar to test_pci_unplug_request */
device_del(qtest, "dev0");
--
2.23.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v3 4/4] spapr: Add SPAPR_CAP_AIL_MODE_3 for AIL mode 3 support for H_SET_MODE hcall
2022-03-07 6:55 [PATCH v3 0/4] Fix PVR matching, add AIL cap compatibility Nicholas Piggin
` (2 preceding siblings ...)
2022-03-07 6:55 ` [PATCH v3 3/4] target/ppc: Add POWER9 DD2.2 model Nicholas Piggin
@ 2022-03-07 6:55 ` Nicholas Piggin
3 siblings, 0 replies; 9+ messages in thread
From: Nicholas Piggin @ 2022-03-07 6:55 UTC (permalink / raw)
To: qemu-ppc
Cc: Alexey Kardashevskiy, Cédric Le Goater, qemu-devel,
Nicholas Piggin, David Gibson
The behaviour of the Address Translation Mode on Interrupt resource is
not consistently supported by all CPU versions or all KVM versions:
KVM-HV does not support mode 2, and does not support mode 3 on POWER7 or
early POWER9 processesors. KVM PR only supports mode 0. TCG supports all
modes (0, 2, 3). This leads to inconsistencies in guest behaviour and
could cause problems migrating guests.
This was not noticable for Linux guests for a long time because the
kernel only uses modes 0 and 3, and it used to consider AIL-3 to be
advisory in that it would always keep the AIL-0 vectors around. Recent
Linux guests depend on the AIL mode working as specified in order to
support the SCV facility interrupt. If AIL-3 can not be provided, then
Linux must be given an error so it can disable the SCV facility, rather
than silently failing.
Add the ail-mode-3 capability to specify that AIL-3 is supported. AIL-0
is implied as the baseline, and AIL-2 is no longer supported by spapr.
AIL-2 is not known to be used by any software, but support in TCG could
be restored with an ail-mode-2 capability quite easily if a regression
is reported.
Modify the H_SET_MODE Address Translation Mode on Interrupt resource
handler to check capabilities and correctly return error if not
supported.
A heuristic is added for KVM to determine AIL-3 support before the
introduction of a new KVM CAP, because blanket disabling AIL-3 has too
much performance cost.
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
hw/ppc/spapr.c | 5 +++++
hw/ppc/spapr_caps.c | 37 +++++++++++++++++++++++++++++++++++++
hw/ppc/spapr_hcall.c | 24 +++++++++++++-----------
include/hw/ppc/spapr.h | 4 +++-
target/ppc/kvm.c | 32 ++++++++++++++++++++++++++++++++
target/ppc/kvm_ppc.h | 6 ++++++
6 files changed, 96 insertions(+), 12 deletions(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 69b0a6f6d6..6789c8a38d 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -4641,6 +4641,11 @@ static void spapr_machine_class_init(ObjectClass *oc, void *data)
smc->default_caps.caps[SPAPR_CAP_CCF_ASSIST] = SPAPR_CAP_ON;
smc->default_caps.caps[SPAPR_CAP_FWNMI] = SPAPR_CAP_ON;
smc->default_caps.caps[SPAPR_CAP_RPT_INVALIDATE] = SPAPR_CAP_OFF;
+
+ /* This cap specifies whether the AIL 3 mode for H_SET_RESOURCE is
+ * supported. The default is modified by default_caps_with_cpu().
+ */
+ smc->default_caps.caps[SPAPR_CAP_AIL_MODE_3] = SPAPR_CAP_ON;
spapr_caps_add_properties(smc);
smc->irq = &spapr_irq_dual;
smc->dr_phb_enabled = true;
diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c
index 655ab856a0..fe9a04b638 100644
--- a/hw/ppc/spapr_caps.c
+++ b/hw/ppc/spapr_caps.c
@@ -614,6 +614,33 @@ static void cap_rpt_invalidate_apply(SpaprMachineState *spapr,
}
}
+static void cap_ail_mode_3_apply(SpaprMachineState *spapr,
+ uint8_t val, Error **errp)
+{
+ ERRP_GUARD();
+ PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
+ PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+
+ if (!val) {
+ return;
+ }
+
+ if (tcg_enabled()) {
+ /* AIL-3 is only supported on POWER8 and above CPUs. */
+ if (!(pcc->insns_flags2 & PPC2_ISA207S)) {
+ error_setg(errp, "TCG only supports cap-ail-mode-3 on POWER8 and later CPUs");
+ error_append_hint(errp, "Try appending -machine cap-ail-mode-3=off\n");
+ return;
+ }
+ } else if (kvm_enabled()) {
+ if (!kvmppc_supports_ail_3()) {
+ error_setg(errp, "KVM implementation does not support cap-ail-mode-3");
+ error_append_hint(errp, "Try appending -machine cap-ail-mode-3=off\n");
+ return;
+ }
+ }
+}
+
SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
[SPAPR_CAP_HTM] = {
.name = "htm",
@@ -731,6 +758,15 @@ SpaprCapabilityInfo capability_table[SPAPR_CAP_NUM] = {
.type = "bool",
.apply = cap_rpt_invalidate_apply,
},
+ [SPAPR_CAP_AIL_MODE_3] = {
+ .name = "ail-mode-3",
+ .description = "Alternate Interrupt Location (AIL) mode 3 support",
+ .index = SPAPR_CAP_AIL_MODE_3,
+ .get = spapr_cap_get_bool,
+ .set = spapr_cap_set_bool,
+ .type = "bool",
+ .apply = cap_ail_mode_3_apply,
+ },
};
static SpaprCapabilities default_caps_with_cpu(SpaprMachineState *spapr,
@@ -750,6 +786,7 @@ static SpaprCapabilities default_caps_with_cpu(SpaprMachineState *spapr,
0, spapr->max_compat_pvr)) {
caps.caps[SPAPR_CAP_HTM] = SPAPR_CAP_OFF;
caps.caps[SPAPR_CAP_CFPC] = SPAPR_CAP_BROKEN;
+ caps.caps[SPAPR_CAP_AIL_MODE_3] = SPAPR_CAP_OFF;
}
if (!ppc_type_check_compat(cputype, CPU_POWERPC_LOGICAL_2_06_PLUS,
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index f008290787..e183892287 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -812,30 +812,32 @@ static target_ulong h_set_mode_resource_le(PowerPCCPU *cpu,
}
static target_ulong h_set_mode_resource_addr_trans_mode(PowerPCCPU *cpu,
+ SpaprMachineState *spapr,
target_ulong mflags,
target_ulong value1,
target_ulong value2)
{
- PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
-
- if (!(pcc->insns_flags2 & PPC2_ISA207S)) {
- return H_P2;
- }
if (value1) {
return H_P3;
}
+
if (value2) {
return H_P4;
}
- if (mflags == 1) {
- /* AIL=1 is reserved in POWER8/POWER9/POWER10 */
+ /*
+ * AIL-1 is not architected, and AIL-2 is not supported by QEMU spapr.
+ * It is supported for faithful emulation of bare metal systems, but for
+ * compatibility concerns we leave it out of the pseries machine.
+ */
+ if (mflags != 0 && mflags != 3) {
return H_UNSUPPORTED_FLAG;
}
- if (mflags == 2 && (pcc->insns_flags2 & PPC2_ISA310)) {
- /* AIL=2 is reserved in POWER10 (ISA v3.1) */
- return H_UNSUPPORTED_FLAG;
+ if (mflags == 3) {
+ if (!spapr_get_cap(spapr, SPAPR_CAP_AIL_MODE_3)) {
+ return H_UNSUPPORTED_FLAG;
+ }
}
spapr_set_all_lpcrs(mflags << LPCR_AIL_SHIFT, LPCR_AIL);
@@ -854,7 +856,7 @@ static target_ulong h_set_mode(PowerPCCPU *cpu, SpaprMachineState *spapr,
ret = h_set_mode_resource_le(cpu, spapr, args[0], args[2], args[3]);
break;
case H_SET_MODE_RESOURCE_ADDR_TRANS_MODE:
- ret = h_set_mode_resource_addr_trans_mode(cpu, args[0],
+ ret = h_set_mode_resource_addr_trans_mode(cpu, spapr, args[0],
args[2], args[3]);
break;
}
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index f5c33dcc86..7c090d54b0 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -77,8 +77,10 @@ typedef enum {
#define SPAPR_CAP_FWNMI 0x0A
/* Support H_RPT_INVALIDATE */
#define SPAPR_CAP_RPT_INVALIDATE 0x0B
+/* Support for AIL modes */
+#define SPAPR_CAP_AIL_MODE_3 0x0C
/* Num Caps */
-#define SPAPR_CAP_NUM (SPAPR_CAP_RPT_INVALIDATE + 1)
+#define SPAPR_CAP_NUM (SPAPR_CAP_AIL_MODE_3 + 1)
/*
* Capability Values
diff --git a/target/ppc/kvm.c b/target/ppc/kvm.c
index dc93b99189..e984474608 100644
--- a/target/ppc/kvm.c
+++ b/target/ppc/kvm.c
@@ -2563,6 +2563,38 @@ int kvmppc_has_cap_rpt_invalidate(void)
return cap_rpt_invalidate;
}
+bool kvmppc_supports_ail_3(void)
+{
+ PowerPCCPUClass *pcc = kvm_ppc_get_host_cpu_class();
+
+ /*
+ * KVM PR only supports AIL-0
+ */
+ if (kvmppc_is_pr(kvm_state)) {
+ return false;
+ }
+
+ /*
+ * KVM HV hosts support AIL-3 on POWER8 and above, except for radix
+ * mode on some early POWER9s.
+ */
+ if (!(pcc->insns_flags2 & PPC2_ISA207S)) {
+ return false;
+ }
+
+ /*
+ * These tests match the CPU_FTR_P9_RADIX_PREFETCH_BUG flag in Linux.
+ * DD2.0 and 2.1 has it, DD2.2 and 2.3 does not, but we have no 2.1 or
+ * 2.3 CPU model.
+ */
+ if (((pcc->pvr & 0xffff0fff) == CPU_POWERPC_POWER9_DD1) ||
+ ((pcc->pvr & 0xffff0fff) == CPU_POWERPC_POWER9_DD20)) {
+ return false;
+ }
+
+ return true;
+}
+
PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void)
{
uint32_t host_pvr = mfpvr();
diff --git a/target/ppc/kvm_ppc.h b/target/ppc/kvm_ppc.h
index ee9325bf9a..7bba26d1da 100644
--- a/target/ppc/kvm_ppc.h
+++ b/target/ppc/kvm_ppc.h
@@ -73,6 +73,7 @@ int kvmppc_set_cap_nested_kvm_hv(int enable);
int kvmppc_get_cap_large_decr(void);
int kvmppc_enable_cap_large_decr(PowerPCCPU *cpu, int enable);
int kvmppc_has_cap_rpt_invalidate(void);
+bool kvmppc_supports_ail_3(void);
int kvmppc_enable_hwrng(void);
int kvmppc_put_books_sregs(PowerPCCPU *cpu);
PowerPCCPUClass *kvm_ppc_get_host_cpu_class(void);
@@ -393,6 +394,11 @@ static inline int kvmppc_has_cap_rpt_invalidate(void)
return false;
}
+static inline bool kvmppc_supports_ail_3(void)
+{
+ return false;
+}
+
static inline int kvmppc_enable_hwrng(void)
{
return -1;
--
2.23.0
^ permalink raw reply related [flat|nested] 9+ messages in thread