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

[David Gibson]

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

signature.asc
Description: PGP signature