Amit Machhiwal <amachhiw@xxxxxxxxxxxxx> writes:
> Currently, rebooting a pseries nested qemu-kvm guest (L2) results in
> below error as L1 qemu sends PVR value 'arch_compat' == 0 via
> ppc_set_compat ioctl. This triggers a condition failure in
> kvmppc_set_arch_compat() resulting in an EINVAL.
>
> qemu-system-ppc64: Unable to set CPU compatibility mode in KVM: Invalid
>
> This patch updates kvmppc_set_arch_compat() to use the host PVR value if
> 'compat_pvr' == 0 indicating that qemu doesn't want to enforce any
> specific PVR compat mode.
>
> Signed-off-by: Amit Machhiwal <amachhiw@xxxxxxxxxxxxx>
> ---
> arch/powerpc/kvm/book3s_hv.c | 2 +-
> arch/powerpc/kvm/book3s_hv_nestedv2.c | 12 ++++++++++--
> 2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 1ed6ec140701..9573d7f4764a 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -439,7 +439,7 @@ static int kvmppc_set_arch_compat(struct kvm_vcpu *vcpu, u32 arch_compat)
> if (guest_pcr_bit > host_pcr_bit)
> return -EINVAL;
>
> - if (kvmhv_on_pseries() && kvmhv_is_nestedv2()) {
> + if (kvmhv_on_pseries() && kvmhv_is_nestedv2() && arch_compat) {
> if (!(cap & nested_capabilities))
> return -EINVAL;
> }
>
Instead of that arch_compat check, would it better to do
if (kvmhv_on_pseries() && kvmhv_is_nestedv2()) {
if (cap && !(cap & nested_capabilities))
return -EINVAL;
}
ie, if a capability is requested, then check against nested_capbilites
to see if the capability exist.
> diff --git a/arch/powerpc/kvm/book3s_hv_nestedv2.c b/arch/powerpc/kvm/book3s_hv_nestedv2.c
> index fd3c4f2d9480..069a1fcfd782 100644
> --- a/arch/powerpc/kvm/book3s_hv_nestedv2.c
> +++ b/arch/powerpc/kvm/book3s_hv_nestedv2.c
> @@ -138,6 +138,7 @@ static int gs_msg_ops_vcpu_fill_info(struct kvmppc_gs_buff *gsb,
> vector128 v;
> int rc, i;
> u16 iden;
> + u32 arch_compat = 0;
>
> vcpu = gsm->data;
>
> @@ -347,8 +348,15 @@ static int gs_msg_ops_vcpu_fill_info(struct kvmppc_gs_buff *gsb,
> break;
> }
> case KVMPPC_GSID_LOGICAL_PVR:
> - rc = kvmppc_gse_put_u32(gsb, iden,
> - vcpu->arch.vcore->arch_compat);
> + if (!vcpu->arch.vcore->arch_compat) {
> + if (cpu_has_feature(CPU_FTR_ARCH_31))
> + arch_compat = PVR_ARCH_31;
> + else if (cpu_has_feature(CPU_FTR_ARCH_300))
> + arch_compat = PVR_ARCH_300;
> + } else {
> + arch_compat = vcpu->arch.vcore->arch_compat;
> + }
> + rc = kvmppc_gse_put_u32(gsb, iden, arch_compat);
>
Won't a arch_compat = 0 work here?. ie, where you observing the -EINVAL from
the first hunk or does this hunk have an impact?
> break;
> }
>
> --
> 2.43.0
-aneesh
