[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] ppc: fix ppc_set_compat() with KVM PR
From: |
Greg Kurz |
Subject: |
Re: [Qemu-devel] [PATCH] ppc: fix ppc_set_compat() with KVM PR |
Date: |
Tue, 15 Aug 2017 12:09:00 +0200 |
On Tue, 15 Aug 2017 14:04:09 +1000
David Gibson <address@hidden> wrote:
> On Mon, Aug 14, 2017 at 07:49:16PM +0200, Greg Kurz wrote:
> > When running in KVM PR mode, kvmppc_set_compat() always fail because the
> > current PR implementation doesn't handle KVM_REG_PPC_ARCH_COMPAT. Now that
> > the machine code inconditionally calls ppc_set_compat_all() at reset time
> > to restore the compat mode default value (commit 66d5c492dd3a9), it is
> > impossible to start a guest with PR:
> >
> > qemu-system-ppc64: Unable to set CPU compatibility mode in KVM:
> > Invalid argument
> >
> > A tentative patch [1] was recently sent by Suraj to address the issue, but
> > it would prevent the compat mode to be turned off on reset. And we really
> > don't want to explicitely check for KVM PR. During the patch's review,
> > David suggested that we should only call the KVM ioctl() if the compat
> > PVR changes. This allows at least to run with KVM PR, provided no compat
> > mode is requested from the command line (which should be the case when
> > running PR nested). This is what this patch does.
>
> I'm not sure that's true any more, since we now prefer compat modes
> during CAS. I guess we could exclude the compat modes from the
Oops, I wanted to talk about CAS and I... forgot :(
So, indeed, CAS fails to apply compat mode and immediately returns H_HARDWARE
to the guest, ie the rest of CAS isn't done :-\. Linux guests print a warning:
WARNING: ibm,client-architecture-support call FAILED!
done
and (luckily?) continue to boot anyway (at least that's what happens with
RHEL7 guests).
> advertised list based on what KVM supports, but that seems pretty
> awkward.
>
I'll re-read the details in LoPAPR and try to come up with something.
Cheers,
--
Greg
> > While here, we also fix the side effect where KVM would fail but we would
> > change the CPU state in QEMU anyway.
> >
> > [1] http://patchwork.ozlabs.org/patch/782039/
> >
> > Signed-off-by: Greg Kurz <address@hidden>
>
> In any case, this makes things better than they were, so I'm applying.
>
> > ---
> > target/ppc/compat.c | 9 +++++----
> > 1 file changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/target/ppc/compat.c b/target/ppc/compat.c
> > index f1b67faa97e3..f8729fe46d61 100644
> > --- a/target/ppc/compat.c
> > +++ b/target/ppc/compat.c
> > @@ -140,16 +140,17 @@ void ppc_set_compat(PowerPCCPU *cpu, uint32_t
> > compat_pvr, Error **errp)
> >
> > cpu_synchronize_state(CPU(cpu));
> >
> > - cpu->compat_pvr = compat_pvr;
> > - env->spr[SPR_PCR] = pcr & pcc->pcr_mask;
> > -
> > - if (kvm_enabled()) {
> > + if (kvm_enabled() && cpu->compat_pvr != compat_pvr) {
> > int ret = kvmppc_set_compat(cpu, cpu->compat_pvr);
> > if (ret < 0) {
> > error_setg_errno(errp, -ret,
> > "Unable to set CPU compatibility mode in
> > KVM");
> > + return;
> > }
> > }
> > +
> > + cpu->compat_pvr = compat_pvr;
> > + env->spr[SPR_PCR] = pcr & pcc->pcr_mask;
> > }
> >
> > typedef struct {
> >
>
pgp9X8xgUuUTa.pgp
Description: OpenPGP digital signature