Re: [Qemu-devel] [PATCH] target/ppc: Only set PCR in kvm if actually in a compat mode

[Suraj Jitindar Singh <sjitindarsingh@gmail.com>]

On Fri, 2017-06-30 at 14:03 +1000, David Gibson wrote:
> On Thu, Jun 29, 2017 at 02:59:39PM +1000, Suraj Jitindar Singh wrote:
> > The Processor Compatibility Register (PCR) I used to set the
> > compatibility mode of the processor using the SET_ONE_REG ioctl on
> > KVM_REG_PPC_ARCH_COMPAT. Previously this was only called when a
> > compat
> > mode was actually in use, however a recent patch made it
> > unconditional.
> > Calling this in KVM_PR fails as there is no handler for that call
> > and it
> > is thus impossible to start a machine with KVM_PR.
> > 
> > Change ppc_set_compat() so that the ioctl is only actually called
> > if a
> > compat mode is in use. This means that a KVM_PR guest can boot.
> > Additionally the current behaviour for KVM_HV is preserved where a
> > compat
> > mode of 0 set pcr and arch_compat in the vcore struct to zero, both
> > of
> > which are initialised to zero anyway.
> > 
> > Fixes: 37f516defa2e ("pseries: Reset CPU compatibility mode")
> > 
> > Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> 
> This doesn't seem quite right.  With this change, how would we ever
> turn compatibility mode _off_ (which could happen on reset if nothing

Oh yeah, didn't really think about that.

> else).  Really we should add this pseudo-register to KVM PR, although
> I'm fine with also having a qemu workaround to let it work with older
> PR versions.

How do you feel about having a check and only calling the ioctl if the
KVM in use is HV?

> 
> > 
> > ---
> > 
> > Based on: dwg/ppc-for-2.10
> > 
> >  target/ppc/compat.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/target/ppc/compat.c b/target/ppc/compat.c
> > index f1b67fa..4482206 100644
> > --- a/target/ppc/compat.c
> > +++ b/target/ppc/compat.c
> > @@ -143,7 +143,7 @@ void ppc_set_compat(PowerPCCPU *cpu, uint32_t
> > compat_pvr, Error **errp)
> >      cpu->compat_pvr = compat_pvr;
> >      env->spr[SPR_PCR] = pcr & pcc->pcr_mask;
> >  
> > -    if (kvm_enabled()) {
> > +    if (kvm_enabled() && compat_pvr) {
> >          int ret = kvmppc_set_compat(cpu, cpu->compat_pvr);
> >          if (ret < 0) {
> >              error_setg_errno(errp, -ret,
> 
>