[PATCH 1/1] ppc/pnv.c: add a friendly warning when accel=kvm is used

[PATCH 1/1] ppc/pnv.c: add a friendly warning when accel=kvm is used

[Daniel Henrique Barboza]

If one tries to use -machine powernv9,accel=kvm in a Power9 host, a
cryptic error will be shown:

qemu-system-ppc64: Register sync failed... If you're using kvm-hv.ko, only "-cpu host" is possible
qemu-system-ppc64: kvm_init_vcpu: kvm_arch_init_vcpu failed (0): Invalid argument

Appending '-cpu host' will throw another error:

qemu-system-ppc64: invalid chip model 'host' for powernv9 machine

The root cause is that in IBM PowerPC we have different specs for the bare-metal
and the guests. The bare-metal follows OPAL, the guests follow PAPR. The kernel
KVM modules presented in the ppc kernels implements PAPR. This means that we
can't use KVM accel when using the powernv machine, which is the emulation of
the bare-metal host.

All that said, let's give a more informative error in this case.

Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
 hw/ppc/pnv.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 71e45515f1..e5b87e8730 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -742,6 +742,11 @@ static void pnv_init(MachineState *machine)
     DriveInfo *pnor = drive_get(IF_MTD, 0, 0);
     DeviceState *dev;
 
+    if (kvm_enabled()) {
+        error_report("The powernv machine does not work with KVM acceleration");
+        exit(EXIT_FAILURE);
+    }
+
     /* allocate RAM */
     if (machine->ram_size < mc->default_ram_size) {
         char *sz = size_to_str(mc->default_ram_size);
-- 
2.31.1

Re: [PATCH 1/1] ppc/pnv.c: add a friendly warning when accel=kvm is used

[David Gibson]

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

On Thu, Nov 25, 2021 at 07:42:02PM -0300, Daniel Henrique Barboza wrote:
> If one tries to use -machine powernv9,accel=kvm in a Power9 host, a
> cryptic error will be shown:
> 
> qemu-system-ppc64: Register sync failed... If you're using kvm-hv.ko, only "-cpu host" is possible
> qemu-system-ppc64: kvm_init_vcpu: kvm_arch_init_vcpu failed (0): Invalid argument
> 
> Appending '-cpu host' will throw another error:
> 
> qemu-system-ppc64: invalid chip model 'host' for powernv9 machine
> 
> The root cause is that in IBM PowerPC we have different specs for the bare-metal
> and the guests. The bare-metal follows OPAL, the guests follow PAPR. The kernel
> KVM modules presented in the ppc kernels implements PAPR. This means that we
> can't use KVM accel when using the powernv machine, which is the emulation of
> the bare-metal host.
> 
> All that said, let's give a more informative error in this case.
> 
> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> ---
>  hw/ppc/pnv.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 71e45515f1..e5b87e8730 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -742,6 +742,11 @@ static void pnv_init(MachineState *machine)
>      DriveInfo *pnor = drive_get(IF_MTD, 0, 0);
>      DeviceState *dev;
>  
> +    if (kvm_enabled()) {
> +        error_report("The powernv machine does not work with KVM acceleration");
> +        exit(EXIT_FAILURE);
> +    }


Hmm.. my only concern here is that powernv could, at least
theoretically, work with KVM PR.  I don't think it does right now,
though.

> +
>      /* allocate RAM */
>      if (machine->ram_size < mc->default_ram_size) {
>          char *sz = size_to_str(mc->default_ram_size);

-- 
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 1/1] ppc/pnv.c: add a friendly warning when accel=kvm is used

[Cédric Le Goater]

On 11/26/21 02:11, David Gibson wrote:
> On Thu, Nov 25, 2021 at 07:42:02PM -0300, Daniel Henrique Barboza wrote:
>> If one tries to use -machine powernv9,accel=kvm in a Power9 host, a
>> cryptic error will be shown:
>>
>> qemu-system-ppc64: Register sync failed... If you're using kvm-hv.ko, only "-cpu host" is possible
>> qemu-system-ppc64: kvm_init_vcpu: kvm_arch_init_vcpu failed (0): Invalid argument
>>
>> Appending '-cpu host' will throw another error:
>>
>> qemu-system-ppc64: invalid chip model 'host' for powernv9 machine
>>
>> The root cause is that in IBM PowerPC we have different specs for the bare-metal
>> and the guests. The bare-metal follows OPAL, the guests follow PAPR. The kernel
>> KVM modules presented in the ppc kernels implements PAPR. This means that we
>> can't use KVM accel when using the powernv machine, which is the emulation of
>> the bare-metal host.
>>
>> All that said, let's give a more informative error in this case.
>>
>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>> ---
>>   hw/ppc/pnv.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>> index 71e45515f1..e5b87e8730 100644
>> --- a/hw/ppc/pnv.c
>> +++ b/hw/ppc/pnv.c
>> @@ -742,6 +742,11 @@ static void pnv_init(MachineState *machine)
>>       DriveInfo *pnor = drive_get(IF_MTD, 0, 0);
>>       DeviceState *dev;
>>   
>> +    if (kvm_enabled()) {
>> +        error_report("The powernv machine does not work with KVM acceleration");
>> +        exit(EXIT_FAILURE);
>> +    }
> 
> 
> Hmm.. my only concern here is that powernv could, at least
> theoretically, work with KVM PR.  I don't think it does right now,
> though.

At the same time, it is nice to not let the user think that it could work
in its current state. Don't you think so ?

C.

> 
>> +
>>       /* allocate RAM */
>>       if (machine->ram_size < mc->default_ram_size) {
>>           char *sz = size_to_str(mc->default_ram_size);
> 

Re: [PATCH 1/1] ppc/pnv.c: add a friendly warning when accel=kvm is used

[David Gibson]

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

On Fri, Nov 26, 2021 at 06:51:38PM +0100, Cédric le Goater wrote:
> On 11/26/21 02:11, David Gibson wrote:
> > On Thu, Nov 25, 2021 at 07:42:02PM -0300, Daniel Henrique Barboza wrote:
> > > If one tries to use -machine powernv9,accel=kvm in a Power9 host, a
> > > cryptic error will be shown:
> > > 
> > > qemu-system-ppc64: Register sync failed... If you're using kvm-hv.ko, only "-cpu host" is possible
> > > qemu-system-ppc64: kvm_init_vcpu: kvm_arch_init_vcpu failed (0): Invalid argument
> > > 
> > > Appending '-cpu host' will throw another error:
> > > 
> > > qemu-system-ppc64: invalid chip model 'host' for powernv9 machine
> > > 
> > > The root cause is that in IBM PowerPC we have different specs for the bare-metal
> > > and the guests. The bare-metal follows OPAL, the guests follow PAPR. The kernel
> > > KVM modules presented in the ppc kernels implements PAPR. This means that we
> > > can't use KVM accel when using the powernv machine, which is the emulation of
> > > the bare-metal host.
> > > 
> > > All that said, let's give a more informative error in this case.
> > > 
> > > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> > > ---
> > >   hw/ppc/pnv.c | 5 +++++
> > >   1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > > index 71e45515f1..e5b87e8730 100644
> > > --- a/hw/ppc/pnv.c
> > > +++ b/hw/ppc/pnv.c
> > > @@ -742,6 +742,11 @@ static void pnv_init(MachineState *machine)
> > >       DriveInfo *pnor = drive_get(IF_MTD, 0, 0);
> > >       DeviceState *dev;
> > > +    if (kvm_enabled()) {
> > > +        error_report("The powernv machine does not work with KVM acceleration");
> > > +        exit(EXIT_FAILURE);
> > > +    }
> > 
> > 
> > Hmm.. my only concern here is that powernv could, at least
> > theoretically, work with KVM PR.  I don't think it does right now,
> > though.
> 
> At the same time, it is nice to not let the user think that it could work
> in its current state. Don't you think so ?

Right, I'm thinking of the implication if you have an old qemu but a
new KVM which let it work.  Chances of KVM actually implementing this
probably aren't good though, so requiring the qemu update if we ever
do is probably the better deal.

-- 
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 1/1] ppc/pnv.c: add a friendly warning when accel=kvm is used

[Daniel Henrique Barboza]

On 11/27/21 02:14, David Gibson wrote:
> On Fri, Nov 26, 2021 at 06:51:38PM +0100, Cédric le Goater wrote:
>> On 11/26/21 02:11, David Gibson wrote:
>>> On Thu, Nov 25, 2021 at 07:42:02PM -0300, Daniel Henrique Barboza wrote:
>>>> If one tries to use -machine powernv9,accel=kvm in a Power9 host, a
>>>> cryptic error will be shown:
>>>>
>>>> qemu-system-ppc64: Register sync failed... If you're using kvm-hv.ko, only "-cpu host" is possible
>>>> qemu-system-ppc64: kvm_init_vcpu: kvm_arch_init_vcpu failed (0): Invalid argument
>>>>
>>>> Appending '-cpu host' will throw another error:
>>>>
>>>> qemu-system-ppc64: invalid chip model 'host' for powernv9 machine
>>>>
>>>> The root cause is that in IBM PowerPC we have different specs for the bare-metal
>>>> and the guests. The bare-metal follows OPAL, the guests follow PAPR. The kernel
>>>> KVM modules presented in the ppc kernels implements PAPR. This means that we
>>>> can't use KVM accel when using the powernv machine, which is the emulation of
>>>> the bare-metal host.
>>>>
>>>> All that said, let's give a more informative error in this case.
>>>>
>>>> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
>>>> ---
>>>>    hw/ppc/pnv.c | 5 +++++
>>>>    1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>>>> index 71e45515f1..e5b87e8730 100644
>>>> --- a/hw/ppc/pnv.c
>>>> +++ b/hw/ppc/pnv.c
>>>> @@ -742,6 +742,11 @@ static void pnv_init(MachineState *machine)
>>>>        DriveInfo *pnor = drive_get(IF_MTD, 0, 0);
>>>>        DeviceState *dev;
>>>> +    if (kvm_enabled()) {
>>>> +        error_report("The powernv machine does not work with KVM acceleration");
>>>> +        exit(EXIT_FAILURE);
>>>> +    }
>>>
>>>
>>> Hmm.. my only concern here is that powernv could, at least
>>> theoretically, work with KVM PR.  I don't think it does right now,
>>> though.
>>
>> At the same time, it is nice to not let the user think that it could work
>> in its current state. Don't you think so ?
> 
> Right, I'm thinking of the implication if you have an old qemu but a
> new KVM which let it work.  Chances of KVM actually implementing this
> probably aren't good though, so requiring the qemu update if we ever
> do is probably the better deal.


If the KVM module implements powernv accel support in the future, I wouldn't
take my the chances with the powernv machine working out of the box with it.

Most likely, if an endeavor of supporting KVM accel for powernv ever takes
place, we'll need QEMU changes to go with it. And when that happens we can
revert this patch and make the other necessary changes/fixes.

All that said, perhaps it's useful to add a note in docs/system/ppc/powernv.rst
explaining the the rationale for what we're doing here.



Thanks,


Daniel



> 

Re: [PATCH 1/1] ppc/pnv.c: add a friendly warning when accel=kvm is used

[David Gibson]

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

On Mon, Nov 29, 2021 at 06:09:41PM -0300, Daniel Henrique Barboza wrote:
> 
> 
> On 11/27/21 02:14, David Gibson wrote:
> > On Fri, Nov 26, 2021 at 06:51:38PM +0100, Cédric le Goater wrote:
> > > On 11/26/21 02:11, David Gibson wrote:
> > > > On Thu, Nov 25, 2021 at 07:42:02PM -0300, Daniel Henrique Barboza wrote:
> > > > > If one tries to use -machine powernv9,accel=kvm in a Power9 host, a
> > > > > cryptic error will be shown:
> > > > > 
> > > > > qemu-system-ppc64: Register sync failed... If you're using kvm-hv.ko, only "-cpu host" is possible
> > > > > qemu-system-ppc64: kvm_init_vcpu: kvm_arch_init_vcpu failed (0): Invalid argument
> > > > > 
> > > > > Appending '-cpu host' will throw another error:
> > > > > 
> > > > > qemu-system-ppc64: invalid chip model 'host' for powernv9 machine
> > > > > 
> > > > > The root cause is that in IBM PowerPC we have different specs for the bare-metal
> > > > > and the guests. The bare-metal follows OPAL, the guests follow PAPR. The kernel
> > > > > KVM modules presented in the ppc kernels implements PAPR. This means that we
> > > > > can't use KVM accel when using the powernv machine, which is the emulation of
> > > > > the bare-metal host.
> > > > > 
> > > > > All that said, let's give a more informative error in this case.
> > > > > 
> > > > > Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
> > > > > ---
> > > > >    hw/ppc/pnv.c | 5 +++++
> > > > >    1 file changed, 5 insertions(+)
> > > > > 
> > > > > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > > > > index 71e45515f1..e5b87e8730 100644
> > > > > --- a/hw/ppc/pnv.c
> > > > > +++ b/hw/ppc/pnv.c
> > > > > @@ -742,6 +742,11 @@ static void pnv_init(MachineState *machine)
> > > > >        DriveInfo *pnor = drive_get(IF_MTD, 0, 0);
> > > > >        DeviceState *dev;
> > > > > +    if (kvm_enabled()) {
> > > > > +        error_report("The powernv machine does not work with KVM acceleration");
> > > > > +        exit(EXIT_FAILURE);
> > > > > +    }
> > > > 
> > > > 
> > > > Hmm.. my only concern here is that powernv could, at least
> > > > theoretically, work with KVM PR.  I don't think it does right now,
> > > > though.
> > > 
> > > At the same time, it is nice to not let the user think that it could work
> > > in its current state. Don't you think so ?
> > 
> > Right, I'm thinking of the implication if you have an old qemu but a
> > new KVM which let it work.  Chances of KVM actually implementing this
> > probably aren't good though, so requiring the qemu update if we ever
> > do is probably the better deal.
> 
> 
> If the KVM module implements powernv accel support in the future, I wouldn't
> take my the chances with the powernv machine working out of the box with it.
> 
> Most likely, if an endeavor of supporting KVM accel for powernv ever takes
> place, we'll need QEMU changes to go with it. And when that happens we can
> revert this patch and make the other necessary changes/fixes.
> 
> All that said, perhaps it's useful to add a note in docs/system/ppc/powernv.rst
> explaining the the rationale for what we're doing here.

Yeah, makes sense to me.

-- 
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 --]