Re: [Qemu-devel] [PATCH] ppc: fix ppc_set_compat() with KVM PR

[David Gibson <david@gibson.dropbear.id.au>]

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

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
advertised list based on what KVM supports, but that seems pretty
awkward.

> 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 <groug@kaod.org>

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

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