Re: [PATCH v4] KVM: PPC: Book3S HV: Fix L2 guest reboot failure due to empty 'arch_compat'

2024-02-20 Thread Michael Ellerman
On Wed, 07 Feb 2024 11:15:26 +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
> argument
> 
> [...]

Applied to powerpc/fixes.

[1/1] KVM: PPC: Book3S HV: Fix L2 guest reboot failure due to empty 
'arch_compat'
  https://git.kernel.org/powerpc/c/20c8c4dafe93e82441583e93bd68c0d256d7bed4

cheers


[PATCH v4] KVM: PPC: Book3S HV: Fix L2 guest reboot failure due to empty 'arch_compat'

2024-02-06 Thread 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
argument

Also, a value of 0 for arch_compat generally refers the default
compatibility of the host. But, arch_compat, being a Guest Wide Element
in nested API v2, cannot be set to 0 in GSB as PowerVM (L0) expects a
non-zero value. A value of 0 triggers a kernel trap during a reboot and
consequently causes it to fail:

[   22.106360] reboot: Restarting system
KVM: unknown exit, hardware reason ffea
NIP 0100   LR fe44 CTR  XER 
20040092 CPU#0
MSR 1000 HID0   HF 6c00 iidx 3 didx 3
TB   DECR 0
GPR00   c2a8c300 7fe0
GPR04   1002 82803033
GPR08 0a00  0004 2fff
GPR12  c2e1 000105639200 0004
GPR16  00010563a090  
GPR20 000105639e20 0001056399c8 7fffe54abab0 000105639288
GPR24  0001 0001 
GPR28   c2b30840 
CR   [ -  -  -  -  -  -  -  -  ] RES 000@
 SRR0   SRR1 PVR 00800200 VRSAVE 

SPRG0  SPRG1   SPRG2   SPRG3 

SPRG4  SPRG5   SPRG6   SPRG7 

HSRR0  HSRR1 
 CFAR 
 LPCR 00020400
 PTCR    DAR   DSISR 

 kernel:trap=0xffea | pc=0x100 | msr=0x1000

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.

The relevant part of the code might need a rework if PowerVM implements
a support for `arch_compat == 0` in nestedv2 API.

Fixes: 19d31c5f1157 ("KVM: PPC: Add support for nestedv2 guests")
Reviewed-by: Aneesh Kumar K.V (IBM) 
Reviewed-by: Vaibhav Jain 
Signed-off-by: Amit Machhiwal 
---

Changes v3 -> v4:
- Moved part of a code comment around implementation of `arch_compat
  == 0` in PowerVM to the patch description based on an off mailing
  list discussion

Changes v2 -> v3:
- Vaibhav: Use a 'break' instead of switch-case fallthrough
- v2: 
https://lore.kernel.org/all/20240205132607.2776637-1-amach...@linux.ibm.com/

Changes v1 -> v2:
- Added descriptive error log in the patch description when
  `arch_compat == 0` passed in GSB
- Added a helper function for PCR to capabilities mapping
- Added relevant comments around the changes being made
- v1: 
https://lore.kernel.org/lkml/20240118095653.2588129-1-amach...@linux.ibm.com/

 arch/powerpc/kvm/book3s_hv.c  | 26 --
 arch/powerpc/kvm/book3s_hv_nestedv2.c | 20 ++--
 2 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 52427fc2a33f..0b921704da45 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -391,6 +391,24 @@ static void kvmppc_set_pvr_hv(struct kvm_vcpu *vcpu, u32 
pvr)
 /* Dummy value used in computing PCR value below */
 #define PCR_ARCH_31(PCR_ARCH_300 << 1)
 
+static inline unsigned long map_pcr_to_cap(unsigned long pcr)
+{
+   unsigned long cap = 0;
+
+   switch (pcr) {
+   case PCR_ARCH_300:
+   cap = H_GUEST_CAP_POWER9;
+   break;
+   case PCR_ARCH_31:
+   cap = H_GUEST_CAP_POWER10;
+   break;
+   default:
+   break;
+   }
+
+   return cap;
+}
+
 static int kvmppc_set_arch_compat(struct kvm_vcpu *vcpu, u32 arch_compat)
 {
unsigned long host_pcr_bit = 0, guest_pcr_bit = 0, cap = 0;
@@ -424,11 +442,9 @@ static int kvmppc_set_arch_compat(struct kvm_vcpu *vcpu, 
u32 arch_compat)
break;
case PVR_ARCH_300:
guest_pcr_bit = PCR_ARCH_300;
-   cap = H_GUEST_CAP_POWER9;
break;
case PVR_ARCH_31:
guest_pcr_bit = PCR_ARCH_31;
-   cap = H_GUEST_CAP_POWER10;
break;
default:
return -EINVAL;
@@ -440,6 +456,12 @@ static int kvmppc_set_arch_compat(struct kvm_vcpu *vcpu, 
u32 arch_compat)
return -EINVAL;
 
if (kvmhv_on_pseries() &&