* [PATCH] KVM: PPC: Book3S HV: Fix L2 guest reboot failure due to empty 'arch_compat' @ 2024-01-18 9:56 Amit Machhiwal 2024-01-23 7:54 ` Gautam Menghani 2024-01-24 7:36 ` Aneesh Kumar K.V 0 siblings, 2 replies; 4+ messages in thread From: Amit Machhiwal @ 2024-01-18 9:56 UTC (permalink / raw) To: linuxppc-dev, kvm, kvm-ppc Cc: Jordan Niethe, linux-kernel, Nicholas Piggin, Vaidyanathan Srinivasan, Aneesh Kumar K . V, Amit Machhiwal, Naveen N . Rao, Vaibhav Jain, Amit Machhiwal 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@linux.ibm.com> --- 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; } 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); break; } -- 2.43.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: PPC: Book3S HV: Fix L2 guest reboot failure due to empty 'arch_compat' 2024-01-18 9:56 [PATCH] KVM: PPC: Book3S HV: Fix L2 guest reboot failure due to empty 'arch_compat' Amit Machhiwal @ 2024-01-23 7:54 ` Gautam Menghani 2024-01-24 7:36 ` Aneesh Kumar K.V 1 sibling, 0 replies; 4+ messages in thread From: Gautam Menghani @ 2024-01-23 7:54 UTC (permalink / raw) To: Amit Machhiwal Cc: kvm, linux-kernel, kvm-ppc, Vaidyanathan Srinivasan, Aneesh Kumar K . V, Amit Machhiwal, Nicholas Piggin, Jordan Niethe, Naveen N . Rao, Vaibhav Jain, linuxppc-dev On Thu, Jan 18, 2024 at 03:26:53PM +0530, Amit Machhiwal wrote: > 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@linux.ibm.com> > --- > 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; > } > 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); > break; > } > > -- > 2.43.0 > I tested this patch on pseries Power 10 machine with KVM support : Without this patch, with the latest mainline as host,the kvm guest on pseries/powervm fails to reboot and with this patch, reboot works fine. Tested-by: Gautam Menghani <gautam@linux.ibm.com> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: PPC: Book3S HV: Fix L2 guest reboot failure due to empty 'arch_compat' 2024-01-18 9:56 [PATCH] KVM: PPC: Book3S HV: Fix L2 guest reboot failure due to empty 'arch_compat' Amit Machhiwal 2024-01-23 7:54 ` Gautam Menghani @ 2024-01-24 7:36 ` Aneesh Kumar K.V 2024-01-29 10:47 ` Amit Machhiwal 1 sibling, 1 reply; 4+ messages in thread From: Aneesh Kumar K.V @ 2024-01-24 7:36 UTC (permalink / raw) To: Amit Machhiwal, linuxppc-dev, kvm, kvm-ppc Cc: Jordan Niethe, linux-kernel, Nicholas Piggin, Vaidyanathan Srinivasan, Amit Machhiwal, Naveen N . Rao, Vaibhav Jain, Amit Machhiwal Amit Machhiwal <amachhiw@linux.ibm.com> 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@linux.ibm.com> > --- > 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Re: [PATCH] KVM: PPC: Book3S HV: Fix L2 guest reboot failure due to empty 'arch_compat' 2024-01-24 7:36 ` Aneesh Kumar K.V @ 2024-01-29 10:47 ` Amit Machhiwal 0 siblings, 0 replies; 4+ messages in thread From: Amit Machhiwal @ 2024-01-29 10:47 UTC (permalink / raw) To: Aneesh Kumar K.V, linuxppc-dev, kvm, kvm-ppc Cc: Jordan Niethe, linux-kernel, Nicholas Piggin, Vaidyanathan Srinivasan, Amit Machhiwal, Naveen N . Rao, Vaibhav Jain Hi Aneesh, Thanks for looking into the patch. My comments are inline below. On 2024/01/24 01:06 PM, Aneesh Kumar K.V wrote: > Amit Machhiwal <amachhiw@linux.ibm.com> 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@linux.ibm.com> > > --- > > 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. The above condition check will cause problems when we would try to boot a machine below Power 9. For example, if we passed the arch_compat == PVR_ARCH_207, cap will remain 0 resulting the above check into a false condition. Consequently, we would never return an -EINVAL in that case resulting the arch compatilbility request succeed when it doesn't support nested papr guest. > > > > 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? > No, an arch_compat == 0 won't work in nested API v2. That's because the guest wide PVR cannot be 0, and if arch_compat == 0, then suppported host PVR value should be mentioned. If we were to skip this hunk (keeping the arch_compat == 0), a system reboot of L2 guest would fail and result into a kernel trap as below: [ 22.106360] reboot: Restarting system KVM: unknown exit, hardware reason ffffffffffffffea NIP 0000000000000100 LR 000000000000fe44 CTR 0000000000000000 XER 0000000020040092 CPU#0 MSR 0000000000001000 HID0 0000000000000000 HF 6c000000 iidx 3 didx 3 TB 00000000 00000000 DECR 0 GPR00 0000000000000000 0000000000000000 c000000002a8c300 000000007fe00000 GPR04 0000000000000000 0000000000000000 0000000000001002 8000000002803033 GPR08 000000000a000000 0000000000000000 0000000000000004 000000002fff0000 GPR12 0000000000000000 c000000002e10000 0000000105639200 0000000000000004 GPR16 0000000000000000 000000010563a090 0000000000000000 0000000000000000 GPR20 0000000105639e20 00000001056399c8 00007fffe54abab0 0000000105639288 GPR24 0000000000000000 0000000000000001 0000000000000001 0000000000000000 GPR28 0000000000000000 0000000000000000 c000000002b30840 0000000000000000 CR 00000000 [ - - - - - - - - ] RES 000@ffffffffffffffff SRR0 0000000000000000 SRR1 0000000000000000 PVR 0000000000800200 VRSAVE 0000000000000000 SPRG0 0000000000000000 SPRG1 0000000000000000 SPRG2 0000000000000000 SPRG3 0000000000000000 SPRG4 0000000000000000 SPRG5 0000000000000000 SPRG6 0000000000000000 SPRG7 0000000000000000 HSRR0 0000000000000000 HSRR1 0000000000000000 CFAR 0000000000000000 LPCR 0000000000020400 PTCR 0000000000000000 DAR 0000000000000000 DSISR 0000000000000000 Message from syslogd@ltcd48-lp1 at Jan 24 08:02:16 ... kernel:trap=0xffffffea | pc=0x100 | msr=0x1000 > > > break; > > } > > > > -- > > 2.43.0 > > -aneesh ~Amit ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-01-29 10:48 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-01-18 9:56 [PATCH] KVM: PPC: Book3S HV: Fix L2 guest reboot failure due to empty 'arch_compat' Amit Machhiwal 2024-01-23 7:54 ` Gautam Menghani 2024-01-24 7:36 ` Aneesh Kumar K.V 2024-01-29 10:47 ` Amit Machhiwal
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).