[libvirt] [PATCH] cpu_ppc64: Add support for host-model on POWER9

Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_ppc64.c | 8 ++++---- .../qemuxml2argv-pseries-cpu-compat-power9.args | 24 ++++++++++++++++++++++ .../qemuxml2argv-pseries-cpu-compat-power9.xml | 21 +++++++++++++++++++ tests/qemuxml2argvtest.c | 7 +++++++ tests/testutilsqemu.c | 13 +++++++++++- tests/testutilsqemu.h | 1 + 6 files changed, 69 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat-power9.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat-power9.xml diff --git a/src/cpu/cpu_ppc64.c b/src/cpu/cpu_ppc64.c index f64592b55..bf0859904 100644 --- a/src/cpu/cpu_ppc64.c +++ b/src/cpu/cpu_ppc64.c @@ -92,22 +92,22 @@ ppc64CheckCompatibilityMode(const char *host_model, if (!compat_mode) return VIR_CPU_COMPARE_IDENTICAL; - /* Valid host CPUs: POWER6, POWER7, POWER8 */ + /* Valid host CPUs: POWER6, POWER7, POWER8, POWER9 */ if (!STRPREFIX(host_model, "POWER") || !(tmp = (char *) host_model + strlen("POWER")) || virStrToLong_i(tmp, NULL, 10, &host) < 0 || - host < 6 || host > 8) { + host < 6 || host > 9) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Host CPU does not support compatibility modes")); goto out; } - /* Valid compatibility modes: power6, power7, power8 */ + /* Valid compatibility modes: power6, power7, power8, power9 */ if (!STRPREFIX(compat_mode, "power") || !(tmp = (char *) compat_mode + strlen("power")) || virStrToLong_i(tmp, NULL, 10, &compat) < 0 || - compat < 6 || compat > 8) { + compat < 6 || compat > 9) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Unknown compatibility mode %s"), compat_mode); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat-power9.args b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat-power9.args new file mode 100644 index 000000000..af93d63dc --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat-power9.args @@ -0,0 +1,24 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-ppc64 \ +-name QEMUGuest1 \ +-S \ +-M pseries \ +-cpu host,compat=power9 \ +-m 256 \ +-smp 4,sockets=4,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-nographic \ +-nodefconfig \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=readline \ +-boot c \ +-usb \ +-chardev pty,id=charserial0 \ +-device spapr-vty,chardev=charserial0,reg=0x30000000 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat-power9.xml b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat-power9.xml new file mode 100644 index 000000000..30ab5c267 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat-power9.xml @@ -0,0 +1,21 @@ +<domain type='kvm'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>4</vcpu> + <os> + <type arch='ppc64' machine='pseries'>hvm</type> + </os> + <cpu mode='host-model'> + <model>power9</model> + </cpu> + <clock offset='utc'/> + <devices> + <emulator>/usr/bin/qemu-system-ppc64</emulator> + <console type='pty'> + <address type="spapr-vio"/> + </console> + <memballoon model="none"/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 426959857..669caa0e4 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1673,6 +1673,13 @@ mymain(void) QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG); DO_TEST("pseries-cpu-le", QEMU_CAPS_KVM, QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG); + DO_TEST_FAILURE("pseries-cpu-compat-power9", QEMU_CAPS_KVM); + + qemuTestSetHostCPU(driver.caps, cpuPower9); + DO_TEST("pseries-cpu-compat-power9", + QEMU_CAPS_KVM, QEMU_CAPS_CHARDEV, QEMU_CAPS_NODEFCONFIG); + qemuTestSetHostCPU(driver.caps, NULL); + qemuTestSetHostArch(driver.caps, VIR_ARCH_NONE); DO_TEST("pseries-panic-missing", diff --git a/tests/testutilsqemu.c b/tests/testutilsqemu.c index 709e291bd..ee4853841 100644 --- a/tests/testutilsqemu.c +++ b/tests/testutilsqemu.c @@ -17,6 +17,7 @@ virCPUDefPtr cpuDefault; virCPUDefPtr cpuHaswell; virCPUDefPtr cpuPower8; +virCPUDefPtr cpuPower9; static virCPUFeatureDef cpuDefaultFeatures[] = { { (char *) "ds", -1 }, @@ -94,6 +95,15 @@ static virCPUDef cpuPower8Data = { .threads = 8, }; +static virCPUDef cpuPower9Data = { + .type = VIR_CPU_TYPE_HOST, + .arch = VIR_ARCH_PPC64, + .model = (char *) "POWER9", + .sockets = 1, + .cores = 16, + .threads = 1, +}; + typedef enum { TEST_UTILS_QEMU_BIN_I686, TEST_UTILS_QEMU_BIN_X86_64, @@ -467,7 +477,8 @@ virCapsPtr testQemuCapsInit(void) if (!(cpuDefault = virCPUDefCopy(&cpuDefaultData)) || !(cpuHaswell = virCPUDefCopy(&cpuHaswellData)) || - !(cpuPower8 = virCPUDefCopy(&cpuPower8Data))) + !(cpuPower8 = virCPUDefCopy(&cpuPower8Data)) || + !(cpuPower9 = virCPUDefCopy(&cpuPower9Data))) goto cleanup; qemuTestSetHostCPU(caps, NULL); diff --git a/tests/testutilsqemu.h b/tests/testutilsqemu.h index 3393f5eb7..05e5651ae 100644 --- a/tests/testutilsqemu.h +++ b/tests/testutilsqemu.h @@ -21,6 +21,7 @@ virQEMUCapsPtr qemuTestParseCapabilities(virCapsPtr caps, extern virCPUDefPtr cpuDefault; extern virCPUDefPtr cpuHaswell; extern virCPUDefPtr cpuPower8; +extern virCPUDefPtr cpuPower9; void qemuTestSetHostArch(virCapsPtr caps, virArch arch); -- 2.13.0

On Thu, 2017-05-18 at 14:40 +0200, Jiri Denemark wrote:
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_ppc64.c | 8 ++++---- .../qemuxml2argv-pseries-cpu-compat-power9.args | 24 ++++++++++++++++++++++ .../qemuxml2argv-pseries-cpu-compat-power9.xml | 21 +++++++++++++++++++ tests/qemuxml2argvtest.c | 7 +++++++ tests/testutilsqemu.c | 13 +++++++++++- tests/testutilsqemu.h | 1 + 6 files changed, 69 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat-power9.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat-power9.xml
Finally had a chance to test this. The patch itself looks okay; however, on an actual POWER9 host, I get qemu-kvm: can't apply global host-powerpc64-cpu.compat=power9: Invalid compatibility mode "power9" and qemu-kvm: Compatibility PVR 0x0f000004 not valid for CPU when trying power9 and power8 compat modes respectively. David, are these known issues in QEMU? Are they being tracked? I don't see much point in updating libvirt to cope with these new compat modes when QEMU itself doesn't. -- Andrea Bolognani / Red Hat / Virtualization

On Wed, Jun 07, 2017 at 03:05:53PM +0200, Andrea Bolognani wrote:
On Thu, 2017-05-18 at 14:40 +0200, Jiri Denemark wrote:
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_ppc64.c | 8 ++++---- .../qemuxml2argv-pseries-cpu-compat-power9.args | 24 ++++++++++++++++++++++ .../qemuxml2argv-pseries-cpu-compat-power9.xml | 21 +++++++++++++++++++ tests/qemuxml2argvtest.c | 7 +++++++ tests/testutilsqemu.c | 13 +++++++++++- tests/testutilsqemu.h | 1 + 6 files changed, 69 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat-power9.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat-power9.xml
Finally had a chance to test this. The patch itself looks okay; however, on an actual POWER9 host, I get
qemu-kvm: can't apply global host-powerpc64-cpu.compat=power9: Invalid compatibility mode "power9"
and
qemu-kvm: Compatibility PVR 0x0f000004 not valid for CPU
when trying power9 and power8 compat modes respectively.
David, are these known issues in QEMU? Are they being tracked?
It's not an issue in qemu, it's an issue in the hardware. The various bugs in DD1 silicon mean it really isn't compliant with the 3.00 architecture (nor, quite, with the earlier ones). For that reason we explicitly disable compatibility modes on a DD1 host. More specifically advertising architected mode compatibility causes the guest kernel not to turn on some vital DD1 workarounds, which makes it break horribly.
I don't see much point in updating libvirt to cope with these new compat modes when QEMU itself doesn't.
Try TCG, it should work there. -- 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

On Thu, 2017-06-08 at 14:12 +1000, David Gibson wrote:
David, are these known issues in QEMU? Are they being tracked? It's not an issue in qemu, it's an issue in the hardware. The various bugs in DD1 silicon mean it really isn't compliant with the 3.00 architecture (nor, quite, with the earlier ones). For that reason we explicitly disable compatibility modes on a DD1 host. More specifically advertising architected mode compatibility causes the guest kernel not to turn on some vital DD1 workarounds, which makes it break horribly.
Fair enough. That still doesn't account for the lack of power9 compatibility mode, which AFAICT is simply not defined at all in QEMU. Are there plans to introduce it? Should I file a bug to track it?
I don't see much point in updating libvirt to cope with these new compat modes when QEMU itself doesn't. Try TCG, it should work there.
Not quite: $ /usr/libexec/qemu-kvm \ -nodefaults \ -M pseries,accel=tcg \ -cpu host,compat=power8 qemu-kvm: Unable to find CPU definition: host I *can* make it work with $ /usr/libexec/qemu-kvm \ -nodefaults \ -M pseries,accel=tcg \ -cpu POWER9,compat=power8 VNC server running on ::1:5900 but that's not helpful because <cpu mode='host-model'/> uses the former. The same was true with POWER8, though. One more issue I've noticed: $ /usr/libexec/qemu-kvm \ -nodefaults \ -M pseries,accel=kvm \ -cpu POWER9 qemu-kvm: Unable to find sPAPR CPU Core definition Since the recommended configuration for POWER8 guests is to use <cpu mode='custom'> <model>POWER8</model> </cpu> I would expect the above command line to work so that POWER9 guests can be configured in a similar manner. That seems bug-worthy to me, should I go ahead and file it? -- Andrea Bolognani / Red Hat / Virtualization

On Thu, Jun 08, 2017 at 10:21:44AM +0200, Andrea Bolognani wrote:
On Thu, 2017-06-08 at 14:12 +1000, David Gibson wrote:
David, are these known issues in QEMU? Are they being tracked? It's not an issue in qemu, it's an issue in the hardware. The various bugs in DD1 silicon mean it really isn't compliant with the 3.00 architecture (nor, quite, with the earlier ones). For that reason we explicitly disable compatibility modes on a DD1 host. More specifically advertising architected mode compatibility causes the guest kernel not to turn on some vital DD1 workarounds, which makes it break horribly.
Fair enough. That still doesn't account for the lack of power9 compatibility mode, which AFAICT is simply not defined at all in QEMU.
Umm.. what? It's right there in target/ppc/compat.c. Or are you looking at downstream - maybe it hasn't been backported yet.
Are there plans to introduce it? Should I file a bug to track it?
I don't see much point in updating libvirt to cope with these new compat modes when QEMU itself doesn't. Try TCG, it should work there.
Not quite:
$ /usr/libexec/qemu-kvm \ -nodefaults \ -M pseries,accel=tcg \ -cpu host,compat=power8 qemu-kvm: Unable to find CPU definition: host
Oh, right, of course, the "host" cpu only exists with KVM.
I *can* make it work with
$ /usr/libexec/qemu-kvm \ -nodefaults \ -M pseries,accel=tcg \ -cpu POWER9,compat=power8 VNC server running on ::1:5900
but that's not helpful because <cpu mode='host-model'/> uses the former. The same was true with POWER8, though.
One more issue I've noticed:
$ /usr/libexec/qemu-kvm \ -nodefaults \ -M pseries,accel=kvm \ -cpu POWER9 qemu-kvm: Unable to find sPAPR CPU Core definition
Since the recommended configuration for POWER8 guests is to use
<cpu mode='custom'> <model>POWER8</model> </cpu>
I would expect the above command line to work so that POWER9 guests can be configured in a similar manner. That seems bug-worthy to me, should I go ahead and file it?
Uh, yeah. Again, I'm pretty sure that's already working upstream. -- 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

On Fri, 2017-06-09 at 01:06 +1000, David Gibson wrote:
power9 compatibility mode, which AFAICT is simply not defined at all in QEMU. Umm.. what? It's right there in target/ppc/compat.c. Or are you looking at downstream - maybe it hasn't been backported yet.
[...]
One more issue I've noticed: $ /usr/libexec/qemu-kvm \ -nodefaults \ -M pseries,accel=kvm \ -cpu POWER9 qemu-kvm: Unable to find sPAPR CPU Core definition Since the recommended configuration for POWER8 guests is to use <cpu mode='custom'> <model>POWER8</model> </cpu> I would expect the above command line to work so that POWER9 guests can be configured in a similar manner. That seems bug-worthy to me, should I go ahead and file it? Uh, yeah. Again, I'm pretty sure that's already working upstream.
Yeah, I've been looking at downstream. I'll try again with upstream and file bugs as needed. -- Andrea Bolognani / Red Hat / Virtualization

On Thu, 2017-06-08 at 18:40 +0200, Andrea Bolognani wrote:
Yeah, I've been looking at downstream. I'll try again with upstream and file bugs as needed.
Okay, I've tried again with upstream QEMU master (with your CPU compatibility patches applied on top), here are the results for both POWER8 and POWER9. <nothing> ========= * POWER8 (architected) * POWER9 (raw) -cpu POWER8 =========== * POWER8 (architected) * qemu-system-ppc64: Register sync failed... If you're using kvm-hv.ko, only "-cpu host" is possible kvm_init_vcpu failed: Invalid argument -cpu POWER9 =========== * qemu-system-ppc64: Register sync failed... If you're using kvm-hv.ko, only "-cpu host" is possible kvm_init_vcpu failed: Invalid argument * qemu-system-ppc64: Register sync failed... If you're using kvm-hv.ko, only "-cpu host" is possible kvm_init_vcpu failed: Invalid argument -cpu host ========= * POWER8 (architected) * POWER9 (raw) -cpu host,compat=power8 ======================= * POWER8 (architected) * Unexpected error in ppc_set_compat() at target/ppc/compat.c:135: qemu-system-ppc64: Compatibility PVR 0x0f000004 not valid for CPU Aborted -cpu host,compat=power9 ======================= * Unexpected error in ppc_set_compat() at target/ppc/compat.c:135: qemu-system-ppc64: Compatibility PVR 0x0f000005 not valid for CPU Aborted * Unexpected error in ppc_set_compat() at target/ppc/compat.c:135: qemu-system-ppc64: Compatibility PVR 0x0f000005 not valid for CPU Aborted AIUI, the fact that POWER9 is using raw mode rather than architected mode for all cases where the guest actually manages to boot is because compatibility modes are not enabled for early silicon; this is consistent with QEMU refusing to use the compatibility modes when explicitly asked to do so. However, the fact that '-cpu POWER9' can't be used on the POWER9 machine seems like a genuine bug. Even on POWER8, the error message is pretty misleading. Last but not least, using an unsupported compatibility mode results in QEMU abort()ing, which seems a bit excessive for a very reasonable guest configuration error. Do you want me to go ahead and file bugs for the last two items? -- Andrea Bolognani / Red Hat / Virtualization

On Mon, Jun 12, 2017 at 05:06:27PM +0800, Andrea Bolognani wrote:
On Thu, 2017-06-08 at 18:40 +0200, Andrea Bolognani wrote:
Yeah, I've been looking at downstream. I'll try again with upstream and file bugs as needed.
Okay, I've tried again with upstream QEMU master (with your CPU compatibility patches applied on top), here are the results for both POWER8 and POWER9.
Note that this is for POWER9 DD1 - different results are expected for POWER9 DD2, when it actually gets out of the fab.
<nothing> =========
* POWER8 (architected) * POWER9 (raw)
As expected.
-cpu POWER8 ===========
* POWER8 (architected) * qemu-system-ppc64: Register sync failed... If you're using kvm-hv.ko, only "-cpu host" is possible kvm_init_vcpu failed: Invalid argument
As expected.
-cpu POWER9 ===========
* qemu-system-ppc64: Register sync failed... If you're using kvm-hv.ko, only "-cpu host" is possible kvm_init_vcpu failed: Invalid argument
As expected.
* qemu-system-ppc64: Register sync failed... If you're using kvm-hv.ko, only "-cpu host" is possible kvm_init_vcpu failed: Invalid argument
Huh.. that's a bit weird.
-cpu host =========
* POWER8 (architected) * POWER9 (raw)
As expected (no parameters defaults to this).
-cpu host,compat=power8 =======================
* POWER8 (architected) * Unexpected error in ppc_set_compat() at target/ppc/compat.c:135: qemu-system-ppc64: Compatibility PVR 0x0f000004 not valid for CPU Aborted
Expected for DD1.
-cpu host,compat=power9 =======================
* Unexpected error in ppc_set_compat() at target/ppc/compat.c:135: qemu-system-ppc64: Compatibility PVR 0x0f000005 not valid for CPU Aborted
Expected.
* Unexpected error in ppc_set_compat() at target/ppc/compat.c:135: qemu-system-ppc64: Compatibility PVR 0x0f000005 not valid for CPU Aborted
Expected for DD1.
AIUI, the fact that POWER9 is using raw mode rather than architected mode for all cases where the guest actually manages to boot is because compatibility modes are not enabled for early silicon; this is consistent with QEMU refusing to use the compatibility modes when explicitly asked to do so.
Right. The compatibility modes are "disabled" in qemu, not in the silicon for DD1. The compatibility control registers are still there in DD1, and will have some effect. It's just that turning on compat mode won't make the DD1 bugs go away, so we're still not architecture compliant. Hence, we don't advertise them.
However, the fact that '-cpu POWER9' can't be used on the POWER9 machine seems like a genuine bug. Even on POWER8, the error message is pretty misleading.
Yes, that does look like a bug.
Last but not least, using an unsupported compatibility mode results in QEMU abort()ing, which seems a bit excessive for a very reasonable guest configuration error.
Ah, yes. Probably just an &error_abort where we should have an &error_fatal.
Do you want me to go ahead and file bugs for the last two items?
Sure, sonuds good. -- 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

On Thu, 2017-05-18 at 14:40 +0200, Jiri Denemark wrote:
Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/cpu/cpu_ppc64.c | 8 ++++---- .../qemuxml2argv-pseries-cpu-compat-power9.args | 24 ++++++++++++++++++++++ .../qemuxml2argv-pseries-cpu-compat-power9.xml | 21 +++++++++++++++++++ tests/qemuxml2argvtest.c | 7 +++++++ tests/testutilsqemu.c | 13 +++++++++++- tests/testutilsqemu.h | 1 + 6 files changed, 69 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat-power9.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pseries-cpu-compat-power9.xml
Since all my concerns turned out to be just due to temporary issues in QEMU, we can go ahead and merge this. Reviewed-by: Andrea Bolognani <abologna@redhat.com> -- Andrea Bolognani / Red Hat / Virtualization
participants (3)
-
Andrea Bolognani
-
David Gibson
-
Jiri Denemark