* [PATCH v7 0/4] KVM: PPC: Expose CPU compatibility modes for nested guests
@ 2026-08-06 17:06 Amit Machhiwal
2026-08-06 17:06 ` [PATCH v7 1/4] KVM: PPC: Introduce KVM_CAP_PPC_COMPAT_CAPS and wire up ioctl Amit Machhiwal
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Amit Machhiwal @ 2026-08-06 17:06 UTC (permalink / raw)
To: linuxppc-dev, Madhavan Srinivasan
Cc: Vaibhav Jain, Amit Machhiwal, Anushree Mathur, Paolo Bonzini,
Nicholas Piggin, Michael Ellerman, Christophe Leroy (CS GROUP),
Jonathan Corbet, Shuah Khan, Ritesh Harjani, kvm, linux-kernel,
linux-doc
On POWER systems, newer processor generations can operate in compatibility
modes corresponding to earlier generations (e.g., a Power11 system running
in Power10 compatibility mode). In such cases, the effective CPU level
exposed to guests differs from the physical processor generation.
This creates a problem for nested virtualization. When booting a nested KVM
guest (L2) inside a host KVM guest (L1) running in a compatibility mode,
userspace (e.g., QEMU) may derive the CPU model from the raw hardware PVR
and attempt to configure the nested guest accordingly. However, the L1
partition is constrained by the compatibility level negotiated with the
hypervisor (L0), and requests exceeding that level are rejected, leading to
guest boot failures such as:
KVM-NESTEDv2: couldn't set guest wide elements
This series provides a mechanism for userspace to query the effective CPU
compatibility modes supported by the host, so it can select an appropriate
CPU model for nested guests.
To achieve this, the series introduces a new KVM capability and ioctl
(KVM_CAP_PPC_COMPAT_CAPS / KVM_PPC_GET_COMPAT_CAPS) that expose the
compatibility modes supported by the host.
Why a new UAPI?
===============
While cpu-version is available in /proc/device-tree/cpus/<cpu#>/cpu-version
on both L1 booted on PowerNV and PowerVM LPARs, the UAPI approach is
preferable for several reasons:
1. pHYP (L0) capabilities: On PowerVM, we need to rely on capabilities
negotiated with pHYP in KVM, not just device tree properties. The
cpu-version property depicts the current compat mode but doesn't point
to what all compat modes are supported for the nested guest.
2. procfs dependency: Not all systems run with procfs enabled (CONFIG_PROC_FS
is optional). Minimal configurations like buildroot might disable it, but
KVM ioctl works regardless since it accesses kernel data structures
directly.
3. Kernel validation: The kernel validates and normalizes the compatibility
information, ensuring userspace gets validated, consistent data.
4. Abstraction & stability: /proc/device-tree is an implementation detail.
The UAPI provides a stable interface that won't break if the underlying
mechanism changes.
5. Semantic clarity: KVM_PPC_GET_COMPAT_CAPS clearly expresses what
compatibility modes can be used for KVM guests, vs. parsing device tree
which requires understanding the semantic meaning of cpu-version.
The implementation supports both:
- KVM on PowerVM (nested API v2), where compatibility information is
served from the cached nested_capabilities value, originally obtained
via the H_GUEST_GET_CAPABILITIES hypercall at module init.
- KVM on PowerNV (nested API v1), where compatibility is derived from the
device tree ("cpu-version") representing the effective processor
compatibility level.
This allows userspace (e.g., QEMU) to select a CPU model consistent with
the host compatibility mode, avoiding mismatches and enabling successful
nested guest boot.
Note: This series is built on top of patch [1] which must be applied first.
Patch [1] ensures arch_compat is validated against the host compatibility
mode before this series adds the capability query mechanism.
Commit e4de1b9cb3b5 ("powerpc/dt_cpu_ftrs: Set CPU_FTR_P11_PVR for Power11
and later processors") which was also a prerequisite has been merged upstream.
Changes in v7:
- KVM_CAP_PPC_COMPAT_CAPS: add hv_enabled guard to align the capability check
with ioctl availability; a PR KVM guest on pseries now correctly returns 0
for the capability [Sashiko]
Patch summary:
[1/4] Introduce KVM_CAP_PPC_COMPAT_CAPS and wire up ioctl
[2/4] Implement capability retrieval for KVM on PowerVM (API v2)
[3/4] Add KVM on PowerNV support (API v1)
[4/4] Document the new ioctl
Testing (with QEMU v5 patches and on top of patch [1]):
KVM APIv1 Testing
=================
On P10 PowerNV machine (L0)
---------------------------
- P10 L1 KVM guest -> works
- P10 nested L2 KVM guest -> works
- P9 compat nested L2 KVM guest -> works
- P9 compat L1 KVM guest -> works
- P9 nested L2 KVM guest -> works
On Powernv11 TCG Guest (L0)
---------------------------
- P11 PowerNV TCG L0 guest -> works
- P11 L1 KVM guest -> works
- P11 L2 KVM guest -> works
- P10 compat L1 KVM guest -> works
- P10 L2 KVM guest -> works
- P9 compat L1 KVM guest -> works
- P9 L2 KVM guest -> works
KVM APIv2 Testing
=================
On P11 PowerVM LPAR (L1)
------------------------
- P11 L2 KVM guest -> works
- P10 compat L2 KVM guest -> works
- P9 compat L2 KVM guest fails to boot as expected
- Without QEMU patches but Linux patches
- P11 L2 KVM guest -> works
- P10 compat L2 KVM guest -> works
- P9 compat L2 KVM guest fails to boot as expected
- Without Linux patches but QEMU patches
- P11 L2 KVM guest -> works
- P10 compat L2 KVM guest -> works
On P11 LPAR in P10 compat (L1)
------------------------------
- P10 (host compat) L2 KVM guest -> works
- Without QEMU patch but Linux patches
- P10 guest fails to boot as expected (error: kvm run failed Invalid argument)
- Without Linux patch but QEMU patches
- P10 guest fails to boot as expected (KVM: unknown exit, hardware reason ffffffffffffffea)
On P10 PowerVM LPAR (L1)
------------------------
- P10 L2 KVM guest -> works
- P9 compat L2 KVM guest fails to boot as expected
TCG pSeries Guest
=================
- P11 (default) pSeries guest boots fine
ABI Extensibility Testing (struct size 32, extra member)
=========================================================
- Newer struct on QEMU, older kernel -> works (kernel returns -E2BIG,
QEMU retries with correct size)
- New struct on Linux kernel, older QEMU -> works (kernel zero-pads
trailing fields, QEMU gets correct data)
With this series, nested guests boot successfully in configurations where
they previously failed due to compatibility mismatches.
Related QEMU series:
====================
QEMU v5 series:
https://lore.kernel.org/all/20260804182914.83091-1-amachhiw@linux.ibm.com/
Previous QEMU versions:
v4: https://lore.kernel.org/all/20260701052341.62289-1-amachhiw@linux.ibm.com/
v3: https://lore.kernel.org/all/20260616113915.25589-1-amachhiw@linux.ibm.com/
v2: https://lore.kernel.org/all/20260502140021.69712-1-amachhiw@linux.ibm.com/
v1: https://lore.kernel.org/all/20260430061333.37905-1-amachhiw@linux.ibm.com/
Previous versions:
==================
v6: https://lore.kernel.org/linuxppc-dev/20260804180705.59160-1-amachhiw@linux.ibm.com/
v5: https://lore.kernel.org/linuxppc-dev/20260701051409.51820-1-amachhiw@linux.ibm.com/
v4: https://lore.kernel.org/linuxppc-dev/20260616123314.82721-1-amachhiw@linux.ibm.com/
v3: https://lore.kernel.org/linuxppc-dev/20260522152744.55251-1-amachhiw@linux.ibm.com/
v2: https://lore.kernel.org/linuxppc-dev/20260513100755.83195-1-amachhiw@linux.ibm.com/
v1: https://lore.kernel.org/linuxppc-dev/20260430054906.94431-1-amachhiw@linux.ibm.com/
References:
===========
[1] https://lore.kernel.org/all/20260714175432.86388-1-amachhiw@linux.ibm.com/
Amit Machhiwal (4):
KVM: PPC: Introduce KVM_CAP_PPC_COMPAT_CAPS and wire up ioctl
KVM: PPC: Book3S HV: Implement compat CPU capability retrieval for KVM
on PowerVM
KVM: PPC: Book3S HV: Add support for compat CPU capabilities for KVM
on PowerNV
KVM: PPC: Document KVM_PPC_GET_COMPAT_CAPS ioctl
Documentation/virt/kvm/api.rst | 79 +++++++++++++++++++++++++++++
arch/powerpc/include/asm/kvm_ppc.h | 1 +
arch/powerpc/include/uapi/asm/kvm.h | 18 +++++++
arch/powerpc/kvm/book3s_hv.c | 56 ++++++++++++++++++++
arch/powerpc/kvm/powerpc.c | 71 ++++++++++++++++++++++++++
include/uapi/linux/kvm.h | 3 ++
6 files changed, 228 insertions(+)
base-commit: fcaeecb8b0cd44f77d03b28de0671258d4db18f8
prerequisite-patch-id: 7755786f0e4f415e47065ff1972765008727fe10
--
2.50.1 (Apple Git-155)
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v7 1/4] KVM: PPC: Introduce KVM_CAP_PPC_COMPAT_CAPS and wire up ioctl
2026-08-06 17:06 [PATCH v7 0/4] KVM: PPC: Expose CPU compatibility modes for nested guests Amit Machhiwal
@ 2026-08-06 17:06 ` Amit Machhiwal
2026-08-07 3:08 ` Ritesh Harjani
2026-08-06 17:06 ` [PATCH v7 2/4] KVM: PPC: Book3S HV: Implement compat CPU capability retrieval for KVM on PowerVM Amit Machhiwal
` (2 subsequent siblings)
3 siblings, 1 reply; 17+ messages in thread
From: Amit Machhiwal @ 2026-08-06 17:06 UTC (permalink / raw)
To: linuxppc-dev, Madhavan Srinivasan
Cc: Vaibhav Jain, Amit Machhiwal, Anushree Mathur, Paolo Bonzini,
Nicholas Piggin, Michael Ellerman, Christophe Leroy (CS GROUP),
Jonathan Corbet, Shuah Khan, Ritesh Harjani, kvm, linux-kernel,
linux-doc, Gautam Menghani
Introduce a new capability and ioctl to expose CPU compatibility modes
supported by the host processor for nested guests.
On IBM POWER systems, newer processor generations (N) can operate in
compatibility modes corresponding to earlier generations, like (N-1) and
(N-2). This is particularly relevant for nested virtualization, where
nested KVM guests may need to run with a specific processor compatibility
level.
Introduce KVM_CAP_PPC_COMPAT_CAPS capability and the corresponding
KVM_PPC_GET_COMPAT_CAPS vm ioctl. The ioctl returns a bitmap describing
the compatibility modes supported by the host in respective bit numbers,
allowing userspace (e.g., QEMU) to select an appropriate compatibility
level when configuring nested KVM guests.
The ioctl handling is added in kvm_arch_vm_ioctl() and retrieves host
CPU compatibility capabilities via a PowerPC-specific backend
implementation when available.
The struct kvm_ppc_compat_caps places the 'size' field first so it can
be read alone via get_user() before copy_struct_from_user() is called,
avoiding pointer arithmetic to locate the size field.
The ioctl is defined using _IO so the ioctl number remains stable even if
the struct grows in future versions. It uses copy_struct_from_user() and
copy_struct_to_user() to provide forward- and backward-compatible
extensibility: older userspace passing a smaller struct to a newer kernel
gets zero-padded trailing fields, while newer userspace passing a larger
struct to an older kernel (usize > ksize) gets sizeof(struct
kvm_ppc_compat_caps) written back to host_caps.size so it can retry with the
older kernel-supported size, after which the kernel returns -E2BIG.
KVM_PPC_COMPAT_CAPS_SIZE_VER0 is defined as a frozen integer constant
(24) marking the size of the initial struct version, used as the
minimum floor for size field validation, similar to other versioned
struct interfaces in the kernel.
The 'flags' field is reserved for future use. The kernel rejects any
call where flags is non-zero with -EINVAL, preventing garbage values
from being baked into ABI permanently.
The ioctl returns appropriate error codes: EINVAL for an invalid size
or non-zero reserved fields, E2BIG if new userspace provides a larger
struct than the kernel knows about (with ksize written back into
host_caps.size for the retry), EFAULT for failed copy operations, and
ENOTTY if the backend doesn't implement get_compat_caps.
Suggested-by: Vaibhav Jain <vaibhav@linux.ibm.com>
Tested-by: Gautam Menghani <gautam@linux.ibm.com>
Reviewed-by: Gautam Menghani <gautam@linux.ibm.com>
Tested-by: Anushree Mathur <anushree.mathur@linux.ibm.com>
Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com>
---
Changes in this version:
- KVM_CAP_PPC_COMPAT_CAPS: add hv_enabled guard to align the capability
check with ioctl availability; a PR KVM VM on pseries now correctly
returns 0 for the capability [Sashiko]
arch/powerpc/include/asm/kvm_ppc.h | 1 +
arch/powerpc/include/uapi/asm/kvm.h | 8 ++++
arch/powerpc/kvm/powerpc.c | 71 +++++++++++++++++++++++++++++
include/uapi/linux/kvm.h | 3 ++
4 files changed, 83 insertions(+)
diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
index 0953f2daa466..169ea6a7fbad 100644
--- a/arch/powerpc/include/asm/kvm_ppc.h
+++ b/arch/powerpc/include/asm/kvm_ppc.h
@@ -319,6 +319,7 @@ struct kvmppc_ops {
bool (*hash_v3_possible)(void);
int (*create_vm_debugfs)(struct kvm *kvm);
int (*create_vcpu_debugfs)(struct kvm_vcpu *vcpu, struct dentry *debugfs_dentry);
+ int (*get_compat_caps)(struct kvm_ppc_compat_caps *host_caps);
};
extern struct kvmppc_ops *kvmppc_hv_ops;
diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
index 077c5437f521..19e53d5ae540 100644
--- a/arch/powerpc/include/uapi/asm/kvm.h
+++ b/arch/powerpc/include/uapi/asm/kvm.h
@@ -437,6 +437,14 @@ struct kvm_ppc_cpu_char {
__u64 behaviour_mask; /* valid bits in behaviour */
};
+/* For KVM_PPC_GET_COMPAT_CAPS */
+struct kvm_ppc_compat_caps {
+ __u64 size; /* Size of this structure */
+ __u64 flags; /* Reserved for future use */
+ __u64 compat_capabilities; /* Capabilities supported by the host */
+};
+#define KVM_PPC_COMPAT_CAPS_SIZE_VER0 24 /* sizeof first published struct */
+
/*
* Values for character and character_mask.
* These are identical to the values used by H_GET_CPU_CHARACTERISTICS.
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index b6b83fe3233f..e64b3cfadd3a 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -703,6 +703,13 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
}
}
break;
+#if defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE)
+ case KVM_CAP_PPC_COMPAT_CAPS:
+ r = 0;
+ if (hv_enabled && kvmhv_on_pseries())
+ r = 1;
+ break;
+#endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */
default:
r = 0;
break;
@@ -2469,6 +2476,70 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
r = kvm->arch.kvm_ops->svm_off(kvm);
break;
}
+ case KVM_PPC_GET_COMPAT_CAPS: {
+ struct kvm_ppc_compat_caps host_caps = {};
+ u64 usize;
+
+ /*
+ * Read the size field first to drive copy_struct_from_user.
+ * size must be the first field of the struct.
+ */
+ r = -EFAULT;
+ if (get_user(usize, (__u64 __user *)argp))
+ goto out;
+
+ /*
+ * Enforce a minimum: reject buffers smaller than the initial
+ * struct version (VER0). This allows old userspace compiled
+ * against the original struct to still work on a newer kernel
+ * that has grown the struct with appended fields.
+ */
+ r = -EINVAL;
+ if (usize < KVM_PPC_COMPAT_CAPS_SIZE_VER0)
+ goto out;
+
+ /*
+ * New userspace with a larger struct called an older kernel.
+ * Write back ksize in host_caps.size so userspace knows which
+ * older struct to retry with, then fail with -E2BIG.
+ */
+ if (usize > sizeof(host_caps)) {
+ host_caps.size = sizeof(host_caps);
+ r = -EFAULT;
+ if (put_user(host_caps.size, (__u64 __user *)argp))
+ goto out;
+ r = -E2BIG;
+ goto out;
+ }
+
+ /*
+ * copy_struct_from_user() handles forward/backward compat:
+ * usize == ksize: verbatim copy
+ * usize < ksize: zero-pad trailing (old userspace, new kernel)
+ */
+ r = copy_struct_from_user(&host_caps, sizeof(host_caps),
+ argp, usize);
+ if (r)
+ goto out;
+
+ /* Reserved fields must be zero */
+ r = -EINVAL;
+ if (host_caps.flags)
+ goto out;
+
+ r = -ENOTTY;
+ if (!kvm->arch.kvm_ops->get_compat_caps)
+ goto out;
+
+ r = kvm->arch.kvm_ops->get_compat_caps(&host_caps);
+ if (r)
+ goto out;
+
+ host_caps.size = sizeof(host_caps);
+ r = copy_struct_to_user(argp, usize, &host_caps,
+ sizeof(host_caps), NULL);
+ break;
+ }
default: {
struct kvm *kvm = filp->private_data;
r = kvm->arch.kvm_ops->arch_vm_ioctl(filp, ioctl, arg);
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 419011097fa8..70e36e6a0ad4 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -997,6 +997,7 @@ struct kvm_enable_cap {
#define KVM_CAP_S390_KEYOP 247
#define KVM_CAP_S390_VSIE_ESAMODE 248
#define KVM_CAP_S390_HPAGE_2G 249
+#define KVM_CAP_PPC_COMPAT_CAPS 250
struct kvm_irq_routing_irqchip {
__u32 irqchip;
@@ -1341,6 +1342,8 @@ struct kvm_s390_keyop {
/* Available with KVM_CAP_COUNTER_OFFSET */
#define KVM_ARM_SET_COUNTER_OFFSET _IOW(KVMIO, 0xb5, struct kvm_arm_counter_offset)
#define KVM_ARM_GET_REG_WRITABLE_MASKS _IOR(KVMIO, 0xb6, struct reg_mask_range)
+/* Available with KVM_CAP_PPC_COMPAT_CAPS */
+#define KVM_PPC_GET_COMPAT_CAPS _IO(KVMIO, 0xb8)
/* ioctl for vm fd */
#define KVM_CREATE_DEVICE _IOWR(KVMIO, 0xe0, struct kvm_create_device)
--
2.50.1 (Apple Git-155)
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v7 2/4] KVM: PPC: Book3S HV: Implement compat CPU capability retrieval for KVM on PowerVM
2026-08-06 17:06 [PATCH v7 0/4] KVM: PPC: Expose CPU compatibility modes for nested guests Amit Machhiwal
2026-08-06 17:06 ` [PATCH v7 1/4] KVM: PPC: Introduce KVM_CAP_PPC_COMPAT_CAPS and wire up ioctl Amit Machhiwal
@ 2026-08-06 17:06 ` Amit Machhiwal
2026-08-07 4:31 ` Ritesh Harjani
2026-08-06 17:06 ` [PATCH v7 3/4] KVM: PPC: Book3S HV: Add support for compat CPU capabilities for KVM on PowerNV Amit Machhiwal
2026-08-06 17:06 ` [PATCH v7 4/4] KVM: PPC: Document KVM_PPC_GET_COMPAT_CAPS ioctl Amit Machhiwal
3 siblings, 1 reply; 17+ messages in thread
From: Amit Machhiwal @ 2026-08-06 17:06 UTC (permalink / raw)
To: linuxppc-dev, Madhavan Srinivasan
Cc: Vaibhav Jain, Amit Machhiwal, Anushree Mathur, Paolo Bonzini,
Nicholas Piggin, Michael Ellerman, Christophe Leroy (CS GROUP),
Jonathan Corbet, Shuah Khan, Ritesh Harjani, kvm, linux-kernel,
linux-doc, Gautam Menghani
On POWER systems, the host CPU may run in a compatibility mode (e.g., a
Power11 processor operating in Power10 compatibility mode). In such
cases, the effective CPU level exposed to guests differs from the
physical processor generation.
When running nested KVM guests, QEMU derives the host CPU type using
mfpvr(), which reflects the physical processor version. This can result
in a mismatch between the CPU model selected by QEMU and the
compatibility mode enforced by the host, leading to guest boot failures.
For example, booting a nested guest on a Power11 LPAR configured in
Power10 compatibility mode fails with:
KVM-NESTEDv2: couldn't set guest wide elements
[..KVM reg dump..]
This occurs because QEMU selects a CPU model corresponding to the
physical processor (via mfpvr()), while the host operates in a lower
compatibility mode. As a result, KVM rejects the requested compatibility
level during guest initialization.
On pseries nestedv2 systems, add support for retrieving host CPU
compatibility capabilities for nested guests on PowerVM. The capability
bitmap reflects the processor modes negotiated between the Power
hypervisor (L0) and the host partition (L1) via the
H_GUEST_GET_CAPABILITIES hcall, but is retrieved from the cached
nested_capabilities value populated during module initialization,
avoiding repeated hypervisor calls. A WARN_ON_ONCE() flags the
unexpected case where nested_capabilities is zero on a nestedv2 system.
The implementation defines KVM-specific capability constants
(KVM_PPC_COMPAT_CAP_POWER9/10/11), masks unsupported bits, and exposes
the result through the KVM_PPC_GET_COMPAT_CAPS ioctl.
Hook the implementation into the Book3S HV kvmppc_ops so that it can be
invoked by the generic KVM ioctl handling code.
Suggested-by: Vaibhav Jain <vaibhav@linux.ibm.com>
Tested-by: Gautam Menghani <gautam@linux.ibm.com>
Reviewed-by: Gautam Menghani <gautam@linux.ibm.com>
Tested-by: Anushree Mathur <anushree.mathur@linux.ibm.com>
Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com>
---
arch/powerpc/include/uapi/asm/kvm.h | 10 ++++++++++
arch/powerpc/kvm/book3s_hv.c | 20 ++++++++++++++++++++
2 files changed, 30 insertions(+)
diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
index 19e53d5ae540..913a64b901a3 100644
--- a/arch/powerpc/include/uapi/asm/kvm.h
+++ b/arch/powerpc/include/uapi/asm/kvm.h
@@ -445,6 +445,16 @@ struct kvm_ppc_compat_caps {
};
#define KVM_PPC_COMPAT_CAPS_SIZE_VER0 24 /* sizeof first published struct */
+/*
+ * Capability bits for compat_capabilities field in kvm_ppc_compat_caps.
+ * These bits indicate which processor compatibility modes are supported.
+ */
+#define KVM_PPC_COMPAT_CAP_POWER9 (1ULL << 62)
+#define KVM_PPC_COMPAT_CAP_POWER10 (1ULL << 61)
+#define KVM_PPC_COMPAT_CAP_POWER11 (1ULL << 60)
+#define KVM_PPC_COMPAT_BITMASK (KVM_PPC_COMPAT_CAP_POWER9 | \
+ KVM_PPC_COMPAT_CAP_POWER10 | \
+ KVM_PPC_COMPAT_CAP_POWER11)
/*
* Values for character and character_mask.
* These are identical to the values used by H_GET_CPU_CHARACTERISTICS.
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index f9380ef65750..152cd08a5b38 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -6523,6 +6523,25 @@ static bool kvmppc_hash_v3_possible(void)
return true;
}
+
+static int kvmppc_get_compat_caps(struct kvm_ppc_compat_caps *host_caps)
+{
+ unsigned long capabilities = 0;
+ long rc = -EINVAL;
+
+ if (kvmhv_on_pseries()) {
+ if (kvmhv_is_nestedv2()) {
+ WARN_ON_ONCE(!nested_capabilities);
+ capabilities = nested_capabilities;
+ rc = 0;
+ }
+ }
+
+ host_caps->compat_capabilities = capabilities & KVM_PPC_COMPAT_BITMASK;
+
+ return rc;
+}
+
static struct kvmppc_ops kvm_ops_hv = {
.get_sregs = kvm_arch_vcpu_ioctl_get_sregs_hv,
.set_sregs = kvm_arch_vcpu_ioctl_set_sregs_hv,
@@ -6565,6 +6584,7 @@ static struct kvmppc_ops kvm_ops_hv = {
.hash_v3_possible = kvmppc_hash_v3_possible,
.create_vcpu_debugfs = kvmppc_arch_create_vcpu_debugfs_hv,
.create_vm_debugfs = kvmppc_arch_create_vm_debugfs_hv,
+ .get_compat_caps = kvmppc_get_compat_caps,
};
static int kvm_init_subcore_bitmap(void)
--
2.50.1 (Apple Git-155)
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v7 3/4] KVM: PPC: Book3S HV: Add support for compat CPU capabilities for KVM on PowerNV
2026-08-06 17:06 [PATCH v7 0/4] KVM: PPC: Expose CPU compatibility modes for nested guests Amit Machhiwal
2026-08-06 17:06 ` [PATCH v7 1/4] KVM: PPC: Introduce KVM_CAP_PPC_COMPAT_CAPS and wire up ioctl Amit Machhiwal
2026-08-06 17:06 ` [PATCH v7 2/4] KVM: PPC: Book3S HV: Implement compat CPU capability retrieval for KVM on PowerVM Amit Machhiwal
@ 2026-08-06 17:06 ` Amit Machhiwal
2026-08-07 4:54 ` Ritesh Harjani
2026-08-06 17:06 ` [PATCH v7 4/4] KVM: PPC: Document KVM_PPC_GET_COMPAT_CAPS ioctl Amit Machhiwal
3 siblings, 1 reply; 17+ messages in thread
From: Amit Machhiwal @ 2026-08-06 17:06 UTC (permalink / raw)
To: linuxppc-dev, Madhavan Srinivasan
Cc: Vaibhav Jain, Amit Machhiwal, Anushree Mathur, Paolo Bonzini,
Nicholas Piggin, Michael Ellerman, Christophe Leroy (CS GROUP),
Jonathan Corbet, Shuah Khan, Ritesh Harjani, kvm, linux-kernel,
linux-doc, Gautam Menghani
Currently, when booting a compatibility-mode KVM guest (L1) on a PowerNV
hypervisor (L0), the guest runs with the expected processor
compatibility level. However, when booting a nested KVM guest (L2)
inside the L1, QEMU derives the CPU model from the raw host PVR and
attempts to run the nested guest at that level, instead of honoring the
compatibility mode of the L1.
Extend host CPU compatibility capability reporting to support nested
virtualization on PowerNV systems (PAPR nested API v1).
For nested API v2 (PowerVM), compatibility capabilities are served from
the cached nested_capabilities value (populated at module init via
kvmhv_nested_init() using the H_GUEST_GET_CAPABILITIES hcall). This
information is not available on PowerNV systems.
For nested API v1, derive the compatibility capabilities from the L1
guest by reading the "cpu-version" property from the device tree, which
reflects the effective (logical) processor compatibility level. Map this
value to the corresponding compatibility capability bitmap using
KVM-specific constants.
The mapping is cumulative: a system running at a given compatibility
level is assumed to also support older generations down the supported
chain. Note that unlike KVM on PowerVM (nested API v2), KVM on PowerNV
currently does not strictly enforce older generation compatibility modes
for nested guests - the reported capabilities reflect what the host CPU
can present, not what the hypervisor independently validates.
Introduce a helper kvmppc_map_compat_capabilities() to translate CPU
version values into KVM_PPC_COMPAT_CAP bits using a fallthrough switch,
and integrate it into kvmppc_get_compat_caps(). The implementation
applies masking to ensure only supported processor modes are exposed.
This allows userspace to query host CPU compatibility modes on both
KVM on PowerVM and on PowerNV platforms via the KVM_PPC_GET_COMPAT_CAPS
ioctl.
Suggested-by: Vaibhav Jain <vaibhav@linux.ibm.com>
Tested-by: Gautam Menghani <gautam@linux.ibm.com>
Reviewed-by: Gautam Menghani <gautam@linux.ibm.com>
Tested-by: Anushree Mathur <anushree.mathur@linux.ibm.com>
Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com>
---
arch/powerpc/kvm/book3s_hv.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 152cd08a5b38..4d0307f55e06 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -6523,20 +6523,56 @@ static bool kvmppc_hash_v3_possible(void)
return true;
}
+static int kvmppc_map_compat_capabilities(u32 cpu_version,
+ unsigned long *capabilities)
+{
+ switch (cpu_version) {
+ case PVR_ARCH_31_P11:
+ *capabilities |= KVM_PPC_COMPAT_CAP_POWER11;
+ fallthrough;
+ case PVR_ARCH_31:
+ *capabilities |= KVM_PPC_COMPAT_CAP_POWER10;
+ fallthrough;
+ case PVR_ARCH_300:
+ *capabilities |= KVM_PPC_COMPAT_CAP_POWER9;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
static int kvmppc_get_compat_caps(struct kvm_ppc_compat_caps *host_caps)
{
+ struct device_node *np;
unsigned long capabilities = 0;
long rc = -EINVAL;
+ u32 cpu_version = 0;
if (kvmhv_on_pseries()) {
if (kvmhv_is_nestedv2()) {
WARN_ON_ONCE(!nested_capabilities);
capabilities = nested_capabilities;
rc = 0;
+ } else {
+ for_each_node_by_type(np, "cpu") {
+ if (!of_property_read_u32(np, "cpu-version",
+ &cpu_version)) {
+ of_node_put(np);
+ break;
+ }
+ }
+ if (!cpu_version)
+ return -EINVAL;
+ rc = kvmppc_map_compat_capabilities(cpu_version,
+ &capabilities);
}
}
+ if (rc < 0)
+ return rc;
+
host_caps->compat_capabilities = capabilities & KVM_PPC_COMPAT_BITMASK;
return rc;
--
2.50.1 (Apple Git-155)
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v7 4/4] KVM: PPC: Document KVM_PPC_GET_COMPAT_CAPS ioctl
2026-08-06 17:06 [PATCH v7 0/4] KVM: PPC: Expose CPU compatibility modes for nested guests Amit Machhiwal
` (2 preceding siblings ...)
2026-08-06 17:06 ` [PATCH v7 3/4] KVM: PPC: Book3S HV: Add support for compat CPU capabilities for KVM on PowerNV Amit Machhiwal
@ 2026-08-06 17:06 ` Amit Machhiwal
2026-08-07 4:35 ` Ritesh Harjani
3 siblings, 1 reply; 17+ messages in thread
From: Amit Machhiwal @ 2026-08-06 17:06 UTC (permalink / raw)
To: linuxppc-dev, Madhavan Srinivasan
Cc: Vaibhav Jain, Amit Machhiwal, Anushree Mathur, Paolo Bonzini,
Nicholas Piggin, Michael Ellerman, Christophe Leroy (CS GROUP),
Jonathan Corbet, Shuah Khan, Ritesh Harjani, kvm, linux-kernel,
linux-doc, Gautam Menghani
Add documentation for the KVM_PPC_GET_COMPAT_CAPS ioctl to the KVM API
documentation.
The ioctl exposes host processor compatibility modes supported for
nested KVM guests on PowerPC systems. The documentation covers error
code descriptions including E2BIG for forward compatibility, the
extensible size-based versioning contract using
KVM_PPC_COMPAT_CAPS_SIZE_VER0, the rationale for rejecting non-zero
reserved fields to prevent ABI ambiguity, bit numbering clarification
for IBM MSB-0 convention, and KVM-specific capability bit constants.
Tested-by: Gautam Menghani <gautam@linux.ibm.com>
Reviewed-by: Gautam Menghani <gautam@linux.ibm.com>
Tested-by: Anushree Mathur <anushree.mathur@linux.ibm.com>
Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com>
---
Documentation/virt/kvm/api.rst | 79 ++++++++++++++++++++++++++++++++++
1 file changed, 79 insertions(+)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index e3003a241d5b..22fedb0aa34b 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -6566,6 +6566,85 @@ KVM_S390_KEYOP_SSKE
Sets the storage key for the guest address ``guest_addr`` to the key
specified in ``key``, returning the previous value in ``key``.
+4.145 KVM_PPC_GET_COMPAT_CAPS
+-----------------------------
+:Capability: KVM_CAP_PPC_COMPAT_CAPS
+:Architectures: powerpc
+:Type: vm ioctl
+:Parameters: struct kvm_ppc_compat_caps (in/out)
+:Returns: 0 on success, negative value on failure
+
+Errors include:
+
+ ======== ============================================================
+ EFAULT if ``struct kvm_ppc_compat_caps`` cannot be read from or
+ written to userspace
+ EINVAL if the ``size`` field is smaller than
+ ``KVM_PPC_COMPAT_CAPS_SIZE_VER0``, if the ``flags`` field
+ is non-zero, or if the backend fails to retrieve or map
+ CPU compatibility capabilities
+ E2BIG if ``size`` is larger than the kernel's struct size
+ (new userspace on old kernel); the kernel writes back its
+ own struct size into the ``size`` field so userspace can
+ retry with the correct size
+ ENOTTY if the backend does not implement the ``get_compat_caps``
+ operation (e.g., on non-HV KVM implementations where the
+ required KVM operations are not available)
+ ======== ============================================================
+
+IBM POWER system server-based processors provide a compatibility mode feature
+where an Nth generation processor can operate in modes consistent with earlier
+generations such as (N-1) and (N-2).
+
+This ioctl provides userspace with information about the CPU compatibility modes
+supported by the current host processor for booting the nested KVM guests on
+KVM on PowerNV (nested API v1) and KVM on PowerVM (nested API v2) platforms.
+
+::
+
+ struct kvm_ppc_compat_caps {
+ __u64 size; /* Size of this structure */
+ __u64 flags; /* Reserved for future use, must be 0 */
+ __u64 compat_capabilities; /* Capabilities supported by the host */
+ };
+
+Before calling this ioctl, userspace must set the ``size`` field to
+``sizeof(struct kvm_ppc_compat_caps)`` and zero the ``flags`` field.
+The kernel rejects non-zero ``flags`` with ``-EINVAL`` to prevent
+uninitialized stack values from being silently accepted, keeping the
+field available for future use without ABI ambiguity.
+
+The ioctl uses ``copy_struct_from_user()`` and ``copy_struct_to_user()``
+to support extensible versioning: if userspace passes a struct smaller
+than the current kernel version (``size >= KVM_PPC_COMPAT_CAPS_SIZE_VER0``),
+the kernel zero-pads unknown trailing fields. If userspace passes a larger
+struct (``size > sizeof(struct kvm_ppc_compat_caps)``), the kernel writes
+back its own struct size into the ``size`` field and returns ``-E2BIG``,
+allowing userspace to discover the kernel's struct size and retry.
+``KVM_PPC_COMPAT_CAPS_SIZE_VER0`` (24) is a frozen constant marking the
+size of the initial struct version.
+
+The ``compat_capabilities`` bit field describes the processor compatibility
+modes supported by the host. The following bits indicate support for specific
+processor modes (using IBM's MSB-0 convention where bit 0 is the most
+significant bit):
+
+- ``KVM_PPC_COMPAT_CAP_POWER9`` (bit 1) -- KVM guests can run in Power9 processor mode
+- ``KVM_PPC_COMPAT_CAP_POWER10`` (bit 2) -- KVM guests can run in Power10 processor mode
+- ``KVM_PPC_COMPAT_CAP_POWER11`` (bit 3) -- KVM guests can run in Power11 processor mode
+
+.. note::
+
+ The bit numbering above uses IBM's MSB-0 convention (bit 0 is the most
+ significant bit). In the actual implementation, these are defined as:
+
+ - ``KVM_PPC_COMPAT_CAP_POWER9`` = ``(1ULL << 62)``
+ - ``KVM_PPC_COMPAT_CAP_POWER10`` = ``(1ULL << 61)``
+ - ``KVM_PPC_COMPAT_CAP_POWER11`` = ``(1ULL << 60)``
+
+ Userspace should use the defined constants from ``<linux/kvm.h>`` rather
+ than hardcoding bit positions.
+
.. _kvm_run:
5. The kvm_run structure
--
2.50.1 (Apple Git-155)
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v7 1/4] KVM: PPC: Introduce KVM_CAP_PPC_COMPAT_CAPS and wire up ioctl
2026-08-06 17:06 ` [PATCH v7 1/4] KVM: PPC: Introduce KVM_CAP_PPC_COMPAT_CAPS and wire up ioctl Amit Machhiwal
@ 2026-08-07 3:08 ` Ritesh Harjani
2026-08-07 10:55 ` Amit Machhiwal
0 siblings, 1 reply; 17+ messages in thread
From: Ritesh Harjani @ 2026-08-07 3:08 UTC (permalink / raw)
To: Amit Machhiwal, linuxppc-dev, Madhavan Srinivasan
Cc: Vaibhav Jain, Amit Machhiwal, Anushree Mathur, Paolo Bonzini,
Nicholas Piggin, Michael Ellerman, Christophe Leroy (CS GROUP),
Jonathan Corbet, Shuah Khan, kvm, linux-kernel, linux-doc,
Gautam Menghani
Amit Machhiwal <amachhiw@linux.ibm.com> writes:
> Introduce a new capability and ioctl to expose CPU compatibility modes
> supported by the host processor for nested guests.
>
> On IBM POWER systems, newer processor generations (N) can operate in
> compatibility modes corresponding to earlier generations, like (N-1) and
> (N-2). This is particularly relevant for nested virtualization, where
> nested KVM guests may need to run with a specific processor compatibility
> level.
>
> Introduce KVM_CAP_PPC_COMPAT_CAPS capability and the corresponding
> KVM_PPC_GET_COMPAT_CAPS vm ioctl. The ioctl returns a bitmap describing
> the compatibility modes supported by the host in respective bit numbers,
> allowing userspace (e.g., QEMU) to select an appropriate compatibility
> level when configuring nested KVM guests.
>
> The ioctl handling is added in kvm_arch_vm_ioctl() and retrieves host
> CPU compatibility capabilities via a PowerPC-specific backend
> implementation when available.
>
> The struct kvm_ppc_compat_caps places the 'size' field first so it can
> be read alone via get_user() before copy_struct_from_user() is called,
> avoiding pointer arithmetic to locate the size field.
>
> The ioctl is defined using _IO so the ioctl number remains stable even if
> the struct grows in future versions. It uses copy_struct_from_user() and
> copy_struct_to_user() to provide forward- and backward-compatible
> extensibility: older userspace passing a smaller struct to a newer kernel
> gets zero-padded trailing fields, while newer userspace passing a larger
> struct to an older kernel (usize > ksize) gets sizeof(struct
> kvm_ppc_compat_caps) written back to host_caps.size so it can retry with the
> older kernel-supported size, after which the kernel returns -E2BIG.
>
> KVM_PPC_COMPAT_CAPS_SIZE_VER0 is defined as a frozen integer constant
> (24) marking the size of the initial struct version, used as the
> minimum floor for size field validation, similar to other versioned
> struct interfaces in the kernel.
>
> The 'flags' field is reserved for future use. The kernel rejects any
> call where flags is non-zero with -EINVAL, preventing garbage values
> from being baked into ABI permanently.
>
> The ioctl returns appropriate error codes: EINVAL for an invalid size
> or non-zero reserved fields, E2BIG if new userspace provides a larger
> struct than the kernel knows about (with ksize written back into
> host_caps.size for the retry), EFAULT for failed copy operations, and
> ENOTTY if the backend doesn't implement get_compat_caps.
>
> Suggested-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> Tested-by: Gautam Menghani <gautam@linux.ibm.com>
> Reviewed-by: Gautam Menghani <gautam@linux.ibm.com>
> Tested-by: Anushree Mathur <anushree.mathur@linux.ibm.com>
> Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com>
> ---
> Changes in this version:
> - KVM_CAP_PPC_COMPAT_CAPS: add hv_enabled guard to align the capability
> check with ioctl availability; a PR KVM VM on pseries now correctly
> returns 0 for the capability [Sashiko]
>
> arch/powerpc/include/asm/kvm_ppc.h | 1 +
> arch/powerpc/include/uapi/asm/kvm.h | 8 ++++
> arch/powerpc/kvm/powerpc.c | 71 +++++++++++++++++++++++++++++
> include/uapi/linux/kvm.h | 3 ++
> 4 files changed, 83 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
> index 0953f2daa466..169ea6a7fbad 100644
> --- a/arch/powerpc/include/asm/kvm_ppc.h
> +++ b/arch/powerpc/include/asm/kvm_ppc.h
> @@ -319,6 +319,7 @@ struct kvmppc_ops {
> bool (*hash_v3_possible)(void);
> int (*create_vm_debugfs)(struct kvm *kvm);
> int (*create_vcpu_debugfs)(struct kvm_vcpu *vcpu, struct dentry *debugfs_dentry);
> + int (*get_compat_caps)(struct kvm_ppc_compat_caps *host_caps);
> };
>
> extern struct kvmppc_ops *kvmppc_hv_ops;
> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
> index 077c5437f521..19e53d5ae540 100644
> --- a/arch/powerpc/include/uapi/asm/kvm.h
> +++ b/arch/powerpc/include/uapi/asm/kvm.h
> @@ -437,6 +437,14 @@ struct kvm_ppc_cpu_char {
> __u64 behaviour_mask; /* valid bits in behaviour */
> };
>
> +/* For KVM_PPC_GET_COMPAT_CAPS */
> +struct kvm_ppc_compat_caps {
> + __u64 size; /* Size of this structure */
> + __u64 flags; /* Reserved for future use */
> + __u64 compat_capabilities; /* Capabilities supported by the host */
> +};
> +#define KVM_PPC_COMPAT_CAPS_SIZE_VER0 24 /* sizeof first published struct */
> +
> /*
> * Values for character and character_mask.
> * These are identical to the values used by H_GET_CPU_CHARACTERISTICS.
> diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> index b6b83fe3233f..e64b3cfadd3a 100644
> --- a/arch/powerpc/kvm/powerpc.c
> +++ b/arch/powerpc/kvm/powerpc.c
> @@ -703,6 +703,13 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> }
> }
> break;
> +#if defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE)
> + case KVM_CAP_PPC_COMPAT_CAPS:
> + r = 0;
> + if (hv_enabled && kvmhv_on_pseries())
I think sashiko is just complaining in the 1st patch because we have not
yet wired up the kvmppc_hv_ops->get_compat_caps() yet in patch-1. I
think it is expecting..
if (hv_enabled && kvmhv_on_pseries() && kvmppc_hv_ops->get_compat_caps)
But either way is fine.
> + r = 1;
> + break;
> +#endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */
> default:
> r = 0;
> break;
> @@ -2469,6 +2476,70 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
> r = kvm->arch.kvm_ops->svm_off(kvm);
> break;
> }
> + case KVM_PPC_GET_COMPAT_CAPS: {
> + struct kvm_ppc_compat_caps host_caps = {};
> + u64 usize;
> +
> + /*
> + * Read the size field first to drive copy_struct_from_user.
> + * size must be the first field of the struct.
> + */
> + r = -EFAULT;
> + if (get_user(usize, (__u64 __user *)argp))
> + goto out;
> +
> + /*
> + * Enforce a minimum: reject buffers smaller than the initial
> + * struct version (VER0). This allows old userspace compiled
> + * against the original struct to still work on a newer kernel
> + * that has grown the struct with appended fields.
> + */
> + r = -EINVAL;
> + if (usize < KVM_PPC_COMPAT_CAPS_SIZE_VER0)
> + goto out;
> +
> + /*
> + * New userspace with a larger struct called an older kernel.
> + * Write back ksize in host_caps.size so userspace knows which
> + * older struct to retry with, then fail with -E2BIG.
> + */
> + if (usize > sizeof(host_caps)) {
> + host_caps.size = sizeof(host_caps);
> + r = -EFAULT;
> + if (put_user(host_caps.size, (__u64 __user *)argp))
> + goto out;
> + r = -E2BIG;
> + goto out;
> + }
You anyways mentioned copy_struct_from_user() is taking care of both
forward and backward compat. Then what is the point of this check?
shouldn't we get rid of this complete if logic? I don't see a point of
this if we are anyway using copy_struct_from_user().
> +
> + /*
> + * copy_struct_from_user() handles forward/backward compat:
> + * usize == ksize: verbatim copy
> + * usize < ksize: zero-pad trailing (old userspace, new kernel)
+ * usize > ksize: succeed iff the trailing bytes userspace
+ * sent are zero, else -E2BIG
shouldn't we add usize > ksize case details, like ^^^, which
copy_struct_from_user() handles since we are already adding the details
of other 2 cases.
</btw snip from copy_struct_from_user kdoc>
* There are three cases to consider:
* * If @usize == @ksize, then it's copied verbatim.
* * If @usize < @ksize, then the userspace has passed an old struct to a
* newer kernel. The rest of the trailing bytes in @dst (@ksize - @usize)
* are to be zero-filled.
* * If @usize > @ksize, then the userspace has passed a new struct to an
* older kernel. The trailing bytes unknown to the kernel (@usize - @ksize)
* are checked to ensure they are zeroed, otherwise -E2BIG is returned.
*
* Returns (in all cases, some data may have been copied):
* * -E2BIG: (@usize > @ksize) and there are non-zero trailing bytes in @src.
* * -EFAULT: access to userspace failed.
> + */
> + r = copy_struct_from_user(&host_caps, sizeof(host_caps),
> + argp, usize);
> + if (r)
> + goto out;
> +
> + /* Reserved fields must be zero */
> + r = -EINVAL;
> + if (host_caps.flags)
> + goto out;
> +
> + r = -ENOTTY;
> + if (!kvm->arch.kvm_ops->get_compat_caps)
> + goto out;
> +
> + r = kvm->arch.kvm_ops->get_compat_caps(&host_caps);
> + if (r)
> + goto out;
> +
> + host_caps.size = sizeof(host_caps);
shouldn't this be, since we don't want to be reporting a larger size to
the user?
host_caps.size = min_t(u64, usize, sizeof(host_caps));
> + r = copy_struct_to_user(argp, usize, &host_caps,
> + sizeof(host_caps), NULL);
> + break;
> + }
> default: {
> struct kvm *kvm = filp->private_data;
> r = kvm->arch.kvm_ops->arch_vm_ioctl(filp, ioctl, arg);
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 419011097fa8..70e36e6a0ad4 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -997,6 +997,7 @@ struct kvm_enable_cap {
> #define KVM_CAP_S390_KEYOP 247
> #define KVM_CAP_S390_VSIE_ESAMODE 248
> #define KVM_CAP_S390_HPAGE_2G 249
> +#define KVM_CAP_PPC_COMPAT_CAPS 250
>
> struct kvm_irq_routing_irqchip {
> __u32 irqchip;
> @@ -1341,6 +1342,8 @@ struct kvm_s390_keyop {
> /* Available with KVM_CAP_COUNTER_OFFSET */
> #define KVM_ARM_SET_COUNTER_OFFSET _IOW(KVMIO, 0xb5, struct kvm_arm_counter_offset)
> #define KVM_ARM_GET_REG_WRITABLE_MASKS _IOR(KVMIO, 0xb6, struct reg_mask_range)
> +/* Available with KVM_CAP_PPC_COMPAT_CAPS */
> +#define KVM_PPC_GET_COMPAT_CAPS _IO(KVMIO, 0xb8)
>
> /* ioctl for vm fd */
> #define KVM_CREATE_DEVICE _IOWR(KVMIO, 0xe0, struct kvm_create_device)
> --
> 2.50.1 (Apple Git-155)
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 2/4] KVM: PPC: Book3S HV: Implement compat CPU capability retrieval for KVM on PowerVM
2026-08-06 17:06 ` [PATCH v7 2/4] KVM: PPC: Book3S HV: Implement compat CPU capability retrieval for KVM on PowerVM Amit Machhiwal
@ 2026-08-07 4:31 ` Ritesh Harjani
2026-08-07 10:58 ` Amit Machhiwal
0 siblings, 1 reply; 17+ messages in thread
From: Ritesh Harjani @ 2026-08-07 4:31 UTC (permalink / raw)
To: Amit Machhiwal, linuxppc-dev, Madhavan Srinivasan
Cc: Vaibhav Jain, Amit Machhiwal, Anushree Mathur, Paolo Bonzini,
Nicholas Piggin, Michael Ellerman, Christophe Leroy (CS GROUP),
Jonathan Corbet, Shuah Khan, kvm, linux-kernel, linux-doc,
Gautam Menghani
Amit Machhiwal <amachhiw@linux.ibm.com> writes:
> On POWER systems, the host CPU may run in a compatibility mode (e.g., a
> Power11 processor operating in Power10 compatibility mode). In such
> cases, the effective CPU level exposed to guests differs from the
> physical processor generation.
>
> When running nested KVM guests, QEMU derives the host CPU type using
> mfpvr(), which reflects the physical processor version. This can result
> in a mismatch between the CPU model selected by QEMU and the
> compatibility mode enforced by the host, leading to guest boot failures.
>
> For example, booting a nested guest on a Power11 LPAR configured in
> Power10 compatibility mode fails with:
>
> KVM-NESTEDv2: couldn't set guest wide elements
> [..KVM reg dump..]
>
> This occurs because QEMU selects a CPU model corresponding to the
> physical processor (via mfpvr()), while the host operates in a lower
> compatibility mode. As a result, KVM rejects the requested compatibility
> level during guest initialization.
>
> On pseries nestedv2 systems, add support for retrieving host CPU
> compatibility capabilities for nested guests on PowerVM. The capability
> bitmap reflects the processor modes negotiated between the Power
> hypervisor (L0) and the host partition (L1) via the
> H_GUEST_GET_CAPABILITIES hcall, but is retrieved from the cached
> nested_capabilities value populated during module initialization,
> avoiding repeated hypervisor calls. A WARN_ON_ONCE() flags the
> unexpected case where nested_capabilities is zero on a nestedv2 system.
> The implementation defines KVM-specific capability constants
> (KVM_PPC_COMPAT_CAP_POWER9/10/11), masks unsupported bits, and exposes
> the result through the KVM_PPC_GET_COMPAT_CAPS ioctl.
>
> Hook the implementation into the Book3S HV kvmppc_ops so that it can be
> invoked by the generic KVM ioctl handling code.
>
> Suggested-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> Tested-by: Gautam Menghani <gautam@linux.ibm.com>
> Reviewed-by: Gautam Menghani <gautam@linux.ibm.com>
> Tested-by: Anushree Mathur <anushree.mathur@linux.ibm.com>
> Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com>
> ---
> arch/powerpc/include/uapi/asm/kvm.h | 10 ++++++++++
> arch/powerpc/kvm/book3s_hv.c | 20 ++++++++++++++++++++
> 2 files changed, 30 insertions(+)
Looks good to me. Please feel free to add:
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 4/4] KVM: PPC: Document KVM_PPC_GET_COMPAT_CAPS ioctl
2026-08-06 17:06 ` [PATCH v7 4/4] KVM: PPC: Document KVM_PPC_GET_COMPAT_CAPS ioctl Amit Machhiwal
@ 2026-08-07 4:35 ` Ritesh Harjani
2026-08-07 13:36 ` Amit Machhiwal
0 siblings, 1 reply; 17+ messages in thread
From: Ritesh Harjani @ 2026-08-07 4:35 UTC (permalink / raw)
To: Amit Machhiwal, linuxppc-dev, Madhavan Srinivasan
Cc: Vaibhav Jain, Amit Machhiwal, Anushree Mathur, Paolo Bonzini,
Nicholas Piggin, Michael Ellerman, Christophe Leroy (CS GROUP),
Jonathan Corbet, Shuah Khan, kvm, linux-kernel, linux-doc,
Gautam Menghani
Amit Machhiwal <amachhiw@linux.ibm.com> writes:
> Add documentation for the KVM_PPC_GET_COMPAT_CAPS ioctl to the KVM API
> documentation.
>
> The ioctl exposes host processor compatibility modes supported for
> nested KVM guests on PowerPC systems. The documentation covers error
> code descriptions including E2BIG for forward compatibility, the
> extensible size-based versioning contract using
> KVM_PPC_COMPAT_CAPS_SIZE_VER0, the rationale for rejecting non-zero
> reserved fields to prevent ABI ambiguity, bit numbering clarification
> for IBM MSB-0 convention, and KVM-specific capability bit constants.
>
> Tested-by: Gautam Menghani <gautam@linux.ibm.com>
> Reviewed-by: Gautam Menghani <gautam@linux.ibm.com>
> Tested-by: Anushree Mathur <anushree.mathur@linux.ibm.com>
> Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com>
> ---
> Documentation/virt/kvm/api.rst | 79 ++++++++++++++++++++++++++++++++++
> 1 file changed, 79 insertions(+)
>
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index e3003a241d5b..22fedb0aa34b 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -6566,6 +6566,85 @@ KVM_S390_KEYOP_SSKE
> Sets the storage key for the guest address ``guest_addr`` to the key
> specified in ``key``, returning the previous value in ``key``.
>
> +4.145 KVM_PPC_GET_COMPAT_CAPS
> +-----------------------------
> +:Capability: KVM_CAP_PPC_COMPAT_CAPS
> +:Architectures: powerpc
> +:Type: vm ioctl
> +:Parameters: struct kvm_ppc_compat_caps (in/out)
> +:Returns: 0 on success, negative value on failure
> +
> +Errors include:
> +
> + ======== ============================================================
> + EFAULT if ``struct kvm_ppc_compat_caps`` cannot be read from or
> + written to userspace
> + EINVAL if the ``size`` field is smaller than
> + ``KVM_PPC_COMPAT_CAPS_SIZE_VER0``, if the ``flags`` field
> + is non-zero, or if the backend fails to retrieve or map
> + CPU compatibility capabilities
> + E2BIG if ``size`` is larger than the kernel's struct size
> + (new userspace on old kernel); the kernel writes back its
> + own struct size into the ``size`` field so userspace can
> + retry with the correct size
> + ENOTTY if the backend does not implement the ``get_compat_caps``
> + operation (e.g., on non-HV KVM implementations where the
> + required KVM operations are not available)
Amit, this may not be true anymore right after your changes in v7?
Can we please update the documentation accordingly as well.
> + ======== ============================================================
> +
> +IBM POWER system server-based processors provide a compatibility mode feature
> +where an Nth generation processor can operate in modes consistent with earlier
> +generations such as (N-1) and (N-2).
> +
> +This ioctl provides userspace with information about the CPU compatibility modes
> +supported by the current host processor for booting the nested KVM guests on
> +KVM on PowerNV (nested API v1) and KVM on PowerVM (nested API v2) platforms.
> +
> +::
> +
> + struct kvm_ppc_compat_caps {
> + __u64 size; /* Size of this structure */
> + __u64 flags; /* Reserved for future use, must be 0 */
> + __u64 compat_capabilities; /* Capabilities supported by the host */
> + };
> +
> +Before calling this ioctl, userspace must set the ``size`` field to
> +``sizeof(struct kvm_ppc_compat_caps)`` and zero the ``flags`` field.
> +The kernel rejects non-zero ``flags`` with ``-EINVAL`` to prevent
> +uninitialized stack values from being silently accepted, keeping the
> +field available for future use without ABI ambiguity.
> +
> +The ioctl uses ``copy_struct_from_user()`` and ``copy_struct_to_user()``
> +to support extensible versioning: if userspace passes a struct smaller
> +than the current kernel version (``size >= KVM_PPC_COMPAT_CAPS_SIZE_VER0``),
> +the kernel zero-pads unknown trailing fields. If userspace passes a larger
So I already requested that we should fix this. We cannot write more
bytes than requested by the user, since that memory may not be allocated
for this struct in userspace.
On checking Sashiko comments in reply to this patch - I think that is
also complaining of the same thing that it could cause buffer overflow.
> +struct (``size > sizeof(struct kvm_ppc_compat_caps)``), the kernel writes
> +back its own struct size into the ``size`` field and returns ``-E2BIG``,
> +allowing userspace to discover the kernel's struct size and retry.
> +``KVM_PPC_COMPAT_CAPS_SIZE_VER0`` (24) is a frozen constant marking the
> +size of the initial struct version.
Once we update the comments in patch-1 - I think we should correct this
documentation too accordingly. We should just simply use
copy_to|from_user_struct() style for doing this.
> +
> +The ``compat_capabilities`` bit field describes the processor compatibility
> +modes supported by the host. The following bits indicate support for specific
> +processor modes (using IBM's MSB-0 convention where bit 0 is the most
> +significant bit):
> +
> +- ``KVM_PPC_COMPAT_CAP_POWER9`` (bit 1) -- KVM guests can run in Power9 processor mode
> +- ``KVM_PPC_COMPAT_CAP_POWER10`` (bit 2) -- KVM guests can run in Power10 processor mode
> +- ``KVM_PPC_COMPAT_CAP_POWER11`` (bit 3) -- KVM guests can run in Power11 processor mode
> +
> +.. note::
> +
> + The bit numbering above uses IBM's MSB-0 convention (bit 0 is the most
> + significant bit). In the actual implementation, these are defined as:
> +
> + - ``KVM_PPC_COMPAT_CAP_POWER9`` = ``(1ULL << 62)``
> + - ``KVM_PPC_COMPAT_CAP_POWER10`` = ``(1ULL << 61)``
> + - ``KVM_PPC_COMPAT_CAP_POWER11`` = ``(1ULL << 60)``
> +
> + Userspace should use the defined constants from ``<linux/kvm.h>`` rather
> + than hardcoding bit positions.
> +
> .. _kvm_run:
>
> 5. The kvm_run structure
> --
> 2.50.1 (Apple Git-155)
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 3/4] KVM: PPC: Book3S HV: Add support for compat CPU capabilities for KVM on PowerNV
2026-08-06 17:06 ` [PATCH v7 3/4] KVM: PPC: Book3S HV: Add support for compat CPU capabilities for KVM on PowerNV Amit Machhiwal
@ 2026-08-07 4:54 ` Ritesh Harjani
2026-08-07 12:07 ` Amit Machhiwal
0 siblings, 1 reply; 17+ messages in thread
From: Ritesh Harjani @ 2026-08-07 4:54 UTC (permalink / raw)
To: Amit Machhiwal, linuxppc-dev, Madhavan Srinivasan
Cc: Vaibhav Jain, Amit Machhiwal, Anushree Mathur, Paolo Bonzini,
Nicholas Piggin, Michael Ellerman, Christophe Leroy (CS GROUP),
Jonathan Corbet, Shuah Khan, kvm, linux-kernel, linux-doc,
Gautam Menghani
Amit Machhiwal <amachhiw@linux.ibm.com> writes:
> Currently, when booting a compatibility-mode KVM guest (L1) on a PowerNV
> hypervisor (L0), the guest runs with the expected processor
> compatibility level. However, when booting a nested KVM guest (L2)
> inside the L1, QEMU derives the CPU model from the raw host PVR and
> attempts to run the nested guest at that level, instead of honoring the
> compatibility mode of the L1.
>
> Extend host CPU compatibility capability reporting to support nested
> virtualization on PowerNV systems (PAPR nested API v1).
>
> For nested API v2 (PowerVM), compatibility capabilities are served from
> the cached nested_capabilities value (populated at module init via
> kvmhv_nested_init() using the H_GUEST_GET_CAPABILITIES hcall). This
> information is not available on PowerNV systems.
>
> For nested API v1, derive the compatibility capabilities from the L1
> guest by reading the "cpu-version" property from the device tree, which
> reflects the effective (logical) processor compatibility level. Map this
> value to the corresponding compatibility capability bitmap using
> KVM-specific constants.
>
> The mapping is cumulative: a system running at a given compatibility
> level is assumed to also support older generations down the supported
> chain. Note that unlike KVM on PowerVM (nested API v2), KVM on PowerNV
> currently does not strictly enforce older generation compatibility modes
> for nested guests - the reported capabilities reflect what the host CPU
> can present, not what the hypervisor independently validates.
>
> Introduce a helper kvmppc_map_compat_capabilities() to translate CPU
> version values into KVM_PPC_COMPAT_CAP bits using a fallthrough switch,
> and integrate it into kvmppc_get_compat_caps(). The implementation
> applies masking to ensure only supported processor modes are exposed.
>
> This allows userspace to query host CPU compatibility modes on both
> KVM on PowerVM and on PowerNV platforms via the KVM_PPC_GET_COMPAT_CAPS
> ioctl.
>
> Suggested-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> Tested-by: Gautam Menghani <gautam@linux.ibm.com>
> Reviewed-by: Gautam Menghani <gautam@linux.ibm.com>
> Tested-by: Anushree Mathur <anushree.mathur@linux.ibm.com>
> Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com>
> ---
> arch/powerpc/kvm/book3s_hv.c | 36 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 36 insertions(+)
>
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 152cd08a5b38..4d0307f55e06 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -6523,20 +6523,56 @@ static bool kvmppc_hash_v3_possible(void)
> return true;
> }
>
> +static int kvmppc_map_compat_capabilities(u32 cpu_version,
> + unsigned long *capabilities)
> +{
> + switch (cpu_version) {
> + case PVR_ARCH_31_P11:
> + *capabilities |= KVM_PPC_COMPAT_CAP_POWER11;
> + fallthrough;
> + case PVR_ARCH_31:
> + *capabilities |= KVM_PPC_COMPAT_CAP_POWER10;
> + fallthrough;
> + case PVR_ARCH_300:
> + *capabilities |= KVM_PPC_COMPAT_CAP_POWER9;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
>
> static int kvmppc_get_compat_caps(struct kvm_ppc_compat_caps *host_caps)
> {
> + struct device_node *np;
> unsigned long capabilities = 0;
> long rc = -EINVAL;
> + u32 cpu_version = 0;
>
> if (kvmhv_on_pseries()) {
> if (kvmhv_is_nestedv2()) {
> WARN_ON_ONCE(!nested_capabilities);
> capabilities = nested_capabilities;
> rc = 0;
> + } else {
> + for_each_node_by_type(np, "cpu") {
> + if (!of_property_read_u32(np, "cpu-version",
> + &cpu_version)) {
> + of_node_put(np);
> + break;
> + }
> + }
What happens when we don't have "cpu-version" DT property?
Check this commit
5a61ef74f269f2 ("powerpc/64s: Support new device tree binding for discovering CPU features")
And also why do we need to parse the DT properties again?
Shouldn't we check something like this?
if (cpu_has_feature(CPU_FTR_P11_PVR))
capabilities |= KVM_PPC_COMPAT_CAP_POWER11;
if (cpu_has_feature(CPU_FTR_ARCH_31))
capabilities |= KVM_PPC_COMPAT_CAP_POWER10;
if (cpu_has_feature(CPU_FTR_ARCH_300))
capabilities |= KVM_PPC_COMPAT_CAP_POWER9;
-ritesh
> + if (!cpu_version)
> + return -EINVAL;
> + rc = kvmppc_map_compat_capabilities(cpu_version,
> + &capabilities);
> }
> }
>
> + if (rc < 0)
> + return rc;
> +
> host_caps->compat_capabilities = capabilities & KVM_PPC_COMPAT_BITMASK;
>
> return rc;
> --
> 2.50.1 (Apple Git-155)
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 1/4] KVM: PPC: Introduce KVM_CAP_PPC_COMPAT_CAPS and wire up ioctl
2026-08-07 3:08 ` Ritesh Harjani
@ 2026-08-07 10:55 ` Amit Machhiwal
2026-08-07 11:36 ` Ritesh Harjani
0 siblings, 1 reply; 17+ messages in thread
From: Amit Machhiwal @ 2026-08-07 10:55 UTC (permalink / raw)
To: Ritesh Harjani
Cc: Amit Machhiwal, linuxppc-dev, Madhavan Srinivasan, Vaibhav Jain,
Anushree Mathur, Paolo Bonzini, Nicholas Piggin, Michael Ellerman,
Christophe Leroy (CS GROUP), Jonathan Corbet, Shuah Khan, kvm,
linux-kernel, linux-doc, Gautam Menghani
Hi Ritesh,
Thanks for reviewing this patch. Please find my response inline below.
On 2026/08/07 08:38 AM, Ritesh Harjani wrote:
> Amit Machhiwal <amachhiw@linux.ibm.com> writes:
>
> > Introduce a new capability and ioctl to expose CPU compatibility modes
> > supported by the host processor for nested guests.
> >
> > On IBM POWER systems, newer processor generations (N) can operate in
> > compatibility modes corresponding to earlier generations, like (N-1) and
> > (N-2). This is particularly relevant for nested virtualization, where
> > nested KVM guests may need to run with a specific processor compatibility
> > level.
> >
> > Introduce KVM_CAP_PPC_COMPAT_CAPS capability and the corresponding
> > KVM_PPC_GET_COMPAT_CAPS vm ioctl. The ioctl returns a bitmap describing
> > the compatibility modes supported by the host in respective bit numbers,
> > allowing userspace (e.g., QEMU) to select an appropriate compatibility
> > level when configuring nested KVM guests.
> >
> > The ioctl handling is added in kvm_arch_vm_ioctl() and retrieves host
> > CPU compatibility capabilities via a PowerPC-specific backend
> > implementation when available.
> >
> > The struct kvm_ppc_compat_caps places the 'size' field first so it can
> > be read alone via get_user() before copy_struct_from_user() is called,
> > avoiding pointer arithmetic to locate the size field.
> >
> > The ioctl is defined using _IO so the ioctl number remains stable even if
> > the struct grows in future versions. It uses copy_struct_from_user() and
> > copy_struct_to_user() to provide forward- and backward-compatible
> > extensibility: older userspace passing a smaller struct to a newer kernel
> > gets zero-padded trailing fields, while newer userspace passing a larger
> > struct to an older kernel (usize > ksize) gets sizeof(struct
> > kvm_ppc_compat_caps) written back to host_caps.size so it can retry with the
> > older kernel-supported size, after which the kernel returns -E2BIG.
> >
> > KVM_PPC_COMPAT_CAPS_SIZE_VER0 is defined as a frozen integer constant
> > (24) marking the size of the initial struct version, used as the
> > minimum floor for size field validation, similar to other versioned
> > struct interfaces in the kernel.
> >
> > The 'flags' field is reserved for future use. The kernel rejects any
> > call where flags is non-zero with -EINVAL, preventing garbage values
> > from being baked into ABI permanently.
> >
> > The ioctl returns appropriate error codes: EINVAL for an invalid size
> > or non-zero reserved fields, E2BIG if new userspace provides a larger
> > struct than the kernel knows about (with ksize written back into
> > host_caps.size for the retry), EFAULT for failed copy operations, and
> > ENOTTY if the backend doesn't implement get_compat_caps.
> >
> > Suggested-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> > Tested-by: Gautam Menghani <gautam@linux.ibm.com>
> > Reviewed-by: Gautam Menghani <gautam@linux.ibm.com>
> > Tested-by: Anushree Mathur <anushree.mathur@linux.ibm.com>
> > Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com>
> > ---
> > Changes in this version:
> > - KVM_CAP_PPC_COMPAT_CAPS: add hv_enabled guard to align the capability
> > check with ioctl availability; a PR KVM VM on pseries now correctly
> > returns 0 for the capability [Sashiko]
> >
> > arch/powerpc/include/asm/kvm_ppc.h | 1 +
> > arch/powerpc/include/uapi/asm/kvm.h | 8 ++++
> > arch/powerpc/kvm/powerpc.c | 71 +++++++++++++++++++++++++++++
> > include/uapi/linux/kvm.h | 3 ++
> > 4 files changed, 83 insertions(+)
> >
> > diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
> > index 0953f2daa466..169ea6a7fbad 100644
> > --- a/arch/powerpc/include/asm/kvm_ppc.h
> > +++ b/arch/powerpc/include/asm/kvm_ppc.h
> > @@ -319,6 +319,7 @@ struct kvmppc_ops {
> > bool (*hash_v3_possible)(void);
> > int (*create_vm_debugfs)(struct kvm *kvm);
> > int (*create_vcpu_debugfs)(struct kvm_vcpu *vcpu, struct dentry *debugfs_dentry);
> > + int (*get_compat_caps)(struct kvm_ppc_compat_caps *host_caps);
> > };
> >
> > extern struct kvmppc_ops *kvmppc_hv_ops;
> > diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
> > index 077c5437f521..19e53d5ae540 100644
> > --- a/arch/powerpc/include/uapi/asm/kvm.h
> > +++ b/arch/powerpc/include/uapi/asm/kvm.h
> > @@ -437,6 +437,14 @@ struct kvm_ppc_cpu_char {
> > __u64 behaviour_mask; /* valid bits in behaviour */
> > };
> >
> > +/* For KVM_PPC_GET_COMPAT_CAPS */
> > +struct kvm_ppc_compat_caps {
> > + __u64 size; /* Size of this structure */
> > + __u64 flags; /* Reserved for future use */
> > + __u64 compat_capabilities; /* Capabilities supported by the host */
> > +};
> > +#define KVM_PPC_COMPAT_CAPS_SIZE_VER0 24 /* sizeof first published struct */
> > +
> > /*
> > * Values for character and character_mask.
> > * These are identical to the values used by H_GET_CPU_CHARACTERISTICS.
> > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> > index b6b83fe3233f..e64b3cfadd3a 100644
> > --- a/arch/powerpc/kvm/powerpc.c
> > +++ b/arch/powerpc/kvm/powerpc.c
> > @@ -703,6 +703,13 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> > }
> > }
> > break;
> > +#if defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE)
> > + case KVM_CAP_PPC_COMPAT_CAPS:
> > + r = 0;
> > + if (hv_enabled && kvmhv_on_pseries())
>
> I think sashiko is just complaining in the 1st patch because we have not
> yet wired up the kvmppc_hv_ops->get_compat_caps() yet in patch-1. I
> think it is expecting..
>
> if (hv_enabled && kvmhv_on_pseries() && kvmppc_hv_ops->get_compat_caps)
>
> But either way is fine.
>
>
> > + r = 1;
> > + break;
> > +#endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */
> > default:
> > r = 0;
> > break;
> > @@ -2469,6 +2476,70 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
> > r = kvm->arch.kvm_ops->svm_off(kvm);
> > break;
> > }
> > + case KVM_PPC_GET_COMPAT_CAPS: {
> > + struct kvm_ppc_compat_caps host_caps = {};
> > + u64 usize;
> > +
> > + /*
> > + * Read the size field first to drive copy_struct_from_user.
> > + * size must be the first field of the struct.
> > + */
> > + r = -EFAULT;
> > + if (get_user(usize, (__u64 __user *)argp))
> > + goto out;
> > +
> > + /*
> > + * Enforce a minimum: reject buffers smaller than the initial
> > + * struct version (VER0). This allows old userspace compiled
> > + * against the original struct to still work on a newer kernel
> > + * that has grown the struct with appended fields.
> > + */
> > + r = -EINVAL;
> > + if (usize < KVM_PPC_COMPAT_CAPS_SIZE_VER0)
> > + goto out;
> > +
> > + /*
> > + * New userspace with a larger struct called an older kernel.
> > + * Write back ksize in host_caps.size so userspace knows which
> > + * older struct to retry with, then fail with -E2BIG.
> > + */
> > + if (usize > sizeof(host_caps)) {
> > + host_caps.size = sizeof(host_caps);
> > + r = -EFAULT;
> > + if (put_user(host_caps.size, (__u64 __user *)argp))
> > + goto out;
> > + r = -E2BIG;
> > + goto out;
> > + }
>
> You anyways mentioned copy_struct_from_user() is taking care of both
> forward and backward compat. Then what is the point of this check?
> shouldn't we get rid of this complete if logic? I don't see a point of
> this if we are anyway using copy_struct_from_user().
The pre-check is intentional and serves a purpose that
copy_struct_from_user() alone cannot provide: explicit kernel struct
size discovery.
copy_struct_from_user() returns -E2BIG when usize > ksize and trailing
bytes are non-zero — but it gives userspace no way to know what ksize to
retry with. It also silently succeeds when trailing bytes are zero,
which means a new userspace on an old kernel would never learn the
kernel's struct size at all.
This design is different by intent: when new userspace passes a larger
struct, we always return -E2BIG and write back sizeof(host_caps) into
the size field so userspace learns the exact kernel-supported size and
can retry with it. This explicit negotiation is verified in the ABI
extensibility testing in the cover letter ("Newer struct on QEMU, older
kernel -> works"). The explicit -E2BIG + ksize writeback makes the
version negotiation unambiguous.
>
> > +
> > + /*
> > + * copy_struct_from_user() handles forward/backward compat:
> > + * usize == ksize: verbatim copy
> > + * usize < ksize: zero-pad trailing (old userspace, new kernel)
>
> + * usize > ksize: succeed iff the trailing bytes userspace
> + * sent are zero, else -E2BIG
>
> shouldn't we add usize > ksize case details, like ^^^, which
> copy_struct_from_user() handles since we are already adding the details
> of other 2 cases.
The comment is intentionally limited to two cases. By the time we reach
copy_struct_from_user(), the usize > sizeof(host_caps) case has already
been caught by the pre-check above and returned -E2BIG. So
copy_struct_from_user() can only ever see usize == ksize or usize <
ksize at this point — those are the only two cases worth documenting
here.
Adding the usize > ksize case would be misleading since that code path
is unreachable for this call site.
>
>
> </btw snip from copy_struct_from_user kdoc>
>
> * There are three cases to consider:
> * * If @usize == @ksize, then it's copied verbatim.
> * * If @usize < @ksize, then the userspace has passed an old struct to a
> * newer kernel. The rest of the trailing bytes in @dst (@ksize - @usize)
> * are to be zero-filled.
> * * If @usize > @ksize, then the userspace has passed a new struct to an
> * older kernel. The trailing bytes unknown to the kernel (@usize - @ksize)
> * are checked to ensure they are zeroed, otherwise -E2BIG is returned.
> *
> * Returns (in all cases, some data may have been copied):
> * * -E2BIG: (@usize > @ksize) and there are non-zero trailing bytes in @src.
> * * -EFAULT: access to userspace failed.
>
> > + */
> > + r = copy_struct_from_user(&host_caps, sizeof(host_caps),
> > + argp, usize);
> > + if (r)
> > + goto out;
> > +
> > + /* Reserved fields must be zero */
> > + r = -EINVAL;
> > + if (host_caps.flags)
> > + goto out;
> > +
> > + r = -ENOTTY;
> > + if (!kvm->arch.kvm_ops->get_compat_caps)
> > + goto out;
> > +
> > + r = kvm->arch.kvm_ops->get_compat_caps(&host_caps);
> > + if (r)
> > + goto out;
> > +
> > + host_caps.size = sizeof(host_caps);
>
> shouldn't this be, since we don't want to be reporting a larger size to
> the user?
>
> host_caps.size = min_t(u64, usize, sizeof(host_caps));
At this point in the code, usize has already been bounds-checked to
[KVM_PPC_COMPAT_CAPS_SIZE_VER0, sizeof(host_caps)], so min_t(u64, usize,
sizeof(host_caps)) is always equal to usize here — the min_t never
actually clamps anything.
Thanks,
Amit
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 2/4] KVM: PPC: Book3S HV: Implement compat CPU capability retrieval for KVM on PowerVM
2026-08-07 4:31 ` Ritesh Harjani
@ 2026-08-07 10:58 ` Amit Machhiwal
0 siblings, 0 replies; 17+ messages in thread
From: Amit Machhiwal @ 2026-08-07 10:58 UTC (permalink / raw)
To: Ritesh Harjani
Cc: Amit Machhiwal, linuxppc-dev, Madhavan Srinivasan, Vaibhav Jain,
Anushree Mathur, Paolo Bonzini, Nicholas Piggin, Michael Ellerman,
Christophe Leroy (CS GROUP), Jonathan Corbet, Shuah Khan, kvm,
linux-kernel, linux-doc, Gautam Menghani
On 2026/08/07 10:01 AM, Ritesh Harjani wrote:
> Amit Machhiwal <amachhiw@linux.ibm.com> writes:
>
> > On POWER systems, the host CPU may run in a compatibility mode (e.g., a
> > Power11 processor operating in Power10 compatibility mode). In such
> > cases, the effective CPU level exposed to guests differs from the
> > physical processor generation.
> >
> > When running nested KVM guests, QEMU derives the host CPU type using
> > mfpvr(), which reflects the physical processor version. This can result
> > in a mismatch between the CPU model selected by QEMU and the
> > compatibility mode enforced by the host, leading to guest boot failures.
> >
> > For example, booting a nested guest on a Power11 LPAR configured in
> > Power10 compatibility mode fails with:
> >
> > KVM-NESTEDv2: couldn't set guest wide elements
> > [..KVM reg dump..]
> >
> > This occurs because QEMU selects a CPU model corresponding to the
> > physical processor (via mfpvr()), while the host operates in a lower
> > compatibility mode. As a result, KVM rejects the requested compatibility
> > level during guest initialization.
> >
> > On pseries nestedv2 systems, add support for retrieving host CPU
> > compatibility capabilities for nested guests on PowerVM. The capability
> > bitmap reflects the processor modes negotiated between the Power
> > hypervisor (L0) and the host partition (L1) via the
> > H_GUEST_GET_CAPABILITIES hcall, but is retrieved from the cached
> > nested_capabilities value populated during module initialization,
> > avoiding repeated hypervisor calls. A WARN_ON_ONCE() flags the
> > unexpected case where nested_capabilities is zero on a nestedv2 system.
> > The implementation defines KVM-specific capability constants
> > (KVM_PPC_COMPAT_CAP_POWER9/10/11), masks unsupported bits, and exposes
> > the result through the KVM_PPC_GET_COMPAT_CAPS ioctl.
> >
> > Hook the implementation into the Book3S HV kvmppc_ops so that it can be
> > invoked by the generic KVM ioctl handling code.
> >
> > Suggested-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> > Tested-by: Gautam Menghani <gautam@linux.ibm.com>
> > Reviewed-by: Gautam Menghani <gautam@linux.ibm.com>
> > Tested-by: Anushree Mathur <anushree.mathur@linux.ibm.com>
> > Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com>
> > ---
> > arch/powerpc/include/uapi/asm/kvm.h | 10 ++++++++++
> > arch/powerpc/kvm/book3s_hv.c | 20 ++++++++++++++++++++
> > 2 files changed, 30 insertions(+)
>
> Looks good to me. Please feel free to add:
>
> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
Thanks for the review, Ritesh!
~Amit
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 1/4] KVM: PPC: Introduce KVM_CAP_PPC_COMPAT_CAPS and wire up ioctl
2026-08-07 10:55 ` Amit Machhiwal
@ 2026-08-07 11:36 ` Ritesh Harjani
2026-08-07 12:15 ` Ritesh Harjani
2026-08-07 13:04 ` Amit Machhiwal
0 siblings, 2 replies; 17+ messages in thread
From: Ritesh Harjani @ 2026-08-07 11:36 UTC (permalink / raw)
To: Amit Machhiwal
Cc: Amit Machhiwal, linuxppc-dev, Madhavan Srinivasan, Vaibhav Jain,
Anushree Mathur, Paolo Bonzini, Nicholas Piggin, Michael Ellerman,
Christophe Leroy (CS GROUP), Jonathan Corbet, Shuah Khan, kvm,
linux-kernel, linux-doc, Gautam Menghani
Amit Machhiwal <amachhiw@linux.ibm.com> writes:
> Hi Ritesh,
>
> Thanks for reviewing this patch. Please find my response inline below.
>
> On 2026/08/07 08:38 AM, Ritesh Harjani wrote:
>> Amit Machhiwal <amachhiw@linux.ibm.com> writes:
>>
>> > Introduce a new capability and ioctl to expose CPU compatibility modes
>> > supported by the host processor for nested guests.
>> >
>> > On IBM POWER systems, newer processor generations (N) can operate in
>> > compatibility modes corresponding to earlier generations, like (N-1) and
>> > (N-2). This is particularly relevant for nested virtualization, where
>> > nested KVM guests may need to run with a specific processor compatibility
>> > level.
>> >
>> > Introduce KVM_CAP_PPC_COMPAT_CAPS capability and the corresponding
>> > KVM_PPC_GET_COMPAT_CAPS vm ioctl. The ioctl returns a bitmap describing
>> > the compatibility modes supported by the host in respective bit numbers,
>> > allowing userspace (e.g., QEMU) to select an appropriate compatibility
>> > level when configuring nested KVM guests.
>> >
>> > The ioctl handling is added in kvm_arch_vm_ioctl() and retrieves host
>> > CPU compatibility capabilities via a PowerPC-specific backend
>> > implementation when available.
>> >
>> > The struct kvm_ppc_compat_caps places the 'size' field first so it can
>> > be read alone via get_user() before copy_struct_from_user() is called,
>> > avoiding pointer arithmetic to locate the size field.
>> >
>> > The ioctl is defined using _IO so the ioctl number remains stable even if
>> > the struct grows in future versions. It uses copy_struct_from_user() and
>> > copy_struct_to_user() to provide forward- and backward-compatible
>> > extensibility: older userspace passing a smaller struct to a newer kernel
>> > gets zero-padded trailing fields, while newer userspace passing a larger
>> > struct to an older kernel (usize > ksize) gets sizeof(struct
>> > kvm_ppc_compat_caps) written back to host_caps.size so it can retry with the
>> > older kernel-supported size, after which the kernel returns -E2BIG.
>> >
>> > KVM_PPC_COMPAT_CAPS_SIZE_VER0 is defined as a frozen integer constant
>> > (24) marking the size of the initial struct version, used as the
>> > minimum floor for size field validation, similar to other versioned
>> > struct interfaces in the kernel.
>> >
>> > The 'flags' field is reserved for future use. The kernel rejects any
>> > call where flags is non-zero with -EINVAL, preventing garbage values
>> > from being baked into ABI permanently.
>> >
>> > The ioctl returns appropriate error codes: EINVAL for an invalid size
>> > or non-zero reserved fields, E2BIG if new userspace provides a larger
>> > struct than the kernel knows about (with ksize written back into
>> > host_caps.size for the retry), EFAULT for failed copy operations, and
>> > ENOTTY if the backend doesn't implement get_compat_caps.
>> >
>> > Suggested-by: Vaibhav Jain <vaibhav@linux.ibm.com>
>> > Tested-by: Gautam Menghani <gautam@linux.ibm.com>
>> > Reviewed-by: Gautam Menghani <gautam@linux.ibm.com>
>> > Tested-by: Anushree Mathur <anushree.mathur@linux.ibm.com>
>> > Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com>
>> > ---
>> > Changes in this version:
>> > - KVM_CAP_PPC_COMPAT_CAPS: add hv_enabled guard to align the capability
>> > check with ioctl availability; a PR KVM VM on pseries now correctly
>> > returns 0 for the capability [Sashiko]
>> >
>> > arch/powerpc/include/asm/kvm_ppc.h | 1 +
>> > arch/powerpc/include/uapi/asm/kvm.h | 8 ++++
>> > arch/powerpc/kvm/powerpc.c | 71 +++++++++++++++++++++++++++++
>> > include/uapi/linux/kvm.h | 3 ++
>> > 4 files changed, 83 insertions(+)
>> >
>> > diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
>> > index 0953f2daa466..169ea6a7fbad 100644
>> > --- a/arch/powerpc/include/asm/kvm_ppc.h
>> > +++ b/arch/powerpc/include/asm/kvm_ppc.h
>> > @@ -319,6 +319,7 @@ struct kvmppc_ops {
>> > bool (*hash_v3_possible)(void);
>> > int (*create_vm_debugfs)(struct kvm *kvm);
>> > int (*create_vcpu_debugfs)(struct kvm_vcpu *vcpu, struct dentry *debugfs_dentry);
>> > + int (*get_compat_caps)(struct kvm_ppc_compat_caps *host_caps);
>> > };
>> >
>> > extern struct kvmppc_ops *kvmppc_hv_ops;
>> > diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
>> > index 077c5437f521..19e53d5ae540 100644
>> > --- a/arch/powerpc/include/uapi/asm/kvm.h
>> > +++ b/arch/powerpc/include/uapi/asm/kvm.h
>> > @@ -437,6 +437,14 @@ struct kvm_ppc_cpu_char {
>> > __u64 behaviour_mask; /* valid bits in behaviour */
>> > };
>> >
>> > +/* For KVM_PPC_GET_COMPAT_CAPS */
>> > +struct kvm_ppc_compat_caps {
>> > + __u64 size; /* Size of this structure */
>> > + __u64 flags; /* Reserved for future use */
>> > + __u64 compat_capabilities; /* Capabilities supported by the host */
>> > +};
>> > +#define KVM_PPC_COMPAT_CAPS_SIZE_VER0 24 /* sizeof first published struct */
>> > +
>> > /*
>> > * Values for character and character_mask.
>> > * These are identical to the values used by H_GET_CPU_CHARACTERISTICS.
>> > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
>> > index b6b83fe3233f..e64b3cfadd3a 100644
>> > --- a/arch/powerpc/kvm/powerpc.c
>> > +++ b/arch/powerpc/kvm/powerpc.c
>> > @@ -703,6 +703,13 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>> > }
>> > }
>> > break;
>> > +#if defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE)
>> > + case KVM_CAP_PPC_COMPAT_CAPS:
>> > + r = 0;
>> > + if (hv_enabled && kvmhv_on_pseries())
>>
>> I think sashiko is just complaining in the 1st patch because we have not
>> yet wired up the kvmppc_hv_ops->get_compat_caps() yet in patch-1. I
>> think it is expecting..
>>
>> if (hv_enabled && kvmhv_on_pseries() && kvmppc_hv_ops->get_compat_caps)
>>
>> But either way is fine.
>>
>>
>> > + r = 1;
>> > + break;
>> > +#endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */
>> > default:
>> > r = 0;
>> > break;
>> > @@ -2469,6 +2476,70 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
>> > r = kvm->arch.kvm_ops->svm_off(kvm);
>> > break;
>> > }
>> > + case KVM_PPC_GET_COMPAT_CAPS: {
>> > + struct kvm_ppc_compat_caps host_caps = {};
>> > + u64 usize;
>> > +
>> > + /*
>> > + * Read the size field first to drive copy_struct_from_user.
>> > + * size must be the first field of the struct.
>> > + */
>> > + r = -EFAULT;
>> > + if (get_user(usize, (__u64 __user *)argp))
>> > + goto out;
>> > +
>> > + /*
>> > + * Enforce a minimum: reject buffers smaller than the initial
>> > + * struct version (VER0). This allows old userspace compiled
>> > + * against the original struct to still work on a newer kernel
>> > + * that has grown the struct with appended fields.
>> > + */
>> > + r = -EINVAL;
>> > + if (usize < KVM_PPC_COMPAT_CAPS_SIZE_VER0)
>> > + goto out;
>> > +
>> > + /*
>> > + * New userspace with a larger struct called an older kernel.
>> > + * Write back ksize in host_caps.size so userspace knows which
>> > + * older struct to retry with, then fail with -E2BIG.
>> > + */
>> > + if (usize > sizeof(host_caps)) {
>> > + host_caps.size = sizeof(host_caps);
>> > + r = -EFAULT;
>> > + if (put_user(host_caps.size, (__u64 __user *)argp))
>> > + goto out;
>> > + r = -E2BIG;
>> > + goto out;
>> > + }
>>
>> You anyways mentioned copy_struct_from_user() is taking care of both
>> forward and backward compat. Then what is the point of this check?
>> shouldn't we get rid of this complete if logic? I don't see a point of
>> this if we are anyway using copy_struct_from_user().
>
> The pre-check is intentional and serves a purpose that
> copy_struct_from_user() alone cannot provide: explicit kernel struct
> size discovery.
>
> copy_struct_from_user() returns -E2BIG when usize > ksize and trailing
> bytes are non-zero — but it gives userspace no way to know what ksize to
> retry with. It also silently succeeds when trailing bytes are zero,
> which means a new userspace on an old kernel would never learn the
> kernel's struct size at all.
>
> This design is different by intent: when new userspace passes a larger
> struct, we always return -E2BIG and write back sizeof(host_caps) into
> the size field so userspace learns the exact kernel-supported size and
> can retry with it. This explicit negotiation is verified in the ABI
> extensibility testing in the cover letter ("Newer struct on QEMU, older
> kernel -> works"). The explicit -E2BIG + ksize writeback makes the
> version negotiation unambiguous.
>
yup, my bad, should have seen the comment in the code above.
But isn't this a bad design? Tomorrow, say we have a v2, a larger
version of this struct and if your newer Qemu (with larger struct size)
is running on an older kernel with a smaller struct size, then you will
fail it because usize > ksize even though the userspace has zero filled
the rest of the trailing bytes...
if (usize > sizeof(host_caps))
Instead wouldn't it be better if we do something like this?
r = copy_struct_from_user(&host_caps, sizeof(host_caps),
argp, usize);
if (r) {
if (r == -E2BIG)
put_user(sizeof(host_caps), &argp->size);
goto out;
}
So, we only fail if the userspace has passed a newer struct and the
trailing bytes, which are unknown to this older kernel, are zeroed,
otherwise we return -E2BIG. We also write the ksize back, so the user
knows what ksize it supports.
This would avoid an unncessary round trip and will simplify the
userspace design, isn't it?
>>
>> > +
>> > + /*
>> > + * copy_struct_from_user() handles forward/backward compat:
>> > + * usize == ksize: verbatim copy
>> > + * usize < ksize: zero-pad trailing (old userspace, new kernel)
>>
>> + * usize > ksize: succeed iff the trailing bytes userspace
>> + * sent are zero, else -E2BIG
>>
>> shouldn't we add usize > ksize case details, like ^^^, which
>> copy_struct_from_user() handles since we are already adding the details
>> of other 2 cases.
>
> The comment is intentionally limited to two cases. By the time we reach
> copy_struct_from_user(), the usize > sizeof(host_caps) case has already
> been caught by the pre-check above and returned -E2BIG. So
> copy_struct_from_user() can only ever see usize == ksize or usize <
> ksize at this point — those are the only two cases worth documenting
> here.
>
> Adding the usize > ksize case would be misleading since that code path
> is unreachable for this call site.
>
>>
>>
>> </btw snip from copy_struct_from_user kdoc>
>>
>> * There are three cases to consider:
>> * * If @usize == @ksize, then it's copied verbatim.
>> * * If @usize < @ksize, then the userspace has passed an old struct to a
>> * newer kernel. The rest of the trailing bytes in @dst (@ksize - @usize)
>> * are to be zero-filled.
>> * * If @usize > @ksize, then the userspace has passed a new struct to an
>> * older kernel. The trailing bytes unknown to the kernel (@usize - @ksize)
>> * are checked to ensure they are zeroed, otherwise -E2BIG is returned.
>> *
>> * Returns (in all cases, some data may have been copied):
>> * * -E2BIG: (@usize > @ksize) and there are non-zero trailing bytes in @src.
>> * * -EFAULT: access to userspace failed.
>>
>> > + */
>> > + r = copy_struct_from_user(&host_caps, sizeof(host_caps),
>> > + argp, usize);
>> > + if (r)
>> > + goto out;
>> > +
>> > + /* Reserved fields must be zero */
>> > + r = -EINVAL;
>> > + if (host_caps.flags)
>> > + goto out;
>> > +
>> > + r = -ENOTTY;
>> > + if (!kvm->arch.kvm_ops->get_compat_caps)
>> > + goto out;
>> > +
>> > + r = kvm->arch.kvm_ops->get_compat_caps(&host_caps);
>> > + if (r)
>> > + goto out;
>> > +
>> > + host_caps.size = sizeof(host_caps);
>>
>> shouldn't this be, since we don't want to be reporting a larger size to
>> the user?
>>
>> host_caps.size = min_t(u64, usize, sizeof(host_caps));
>
> At this point in the code, usize has already been bounds-checked to
> [KVM_PPC_COMPAT_CAPS_SIZE_VER0, sizeof(host_caps)], so min_t(u64, usize,
> sizeof(host_caps)) is always equal to usize here — the min_t never
> actually clamps anything.
>
> Thanks,
> Amit
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 3/4] KVM: PPC: Book3S HV: Add support for compat CPU capabilities for KVM on PowerNV
2026-08-07 4:54 ` Ritesh Harjani
@ 2026-08-07 12:07 ` Amit Machhiwal
2026-08-07 12:13 ` Ritesh Harjani
0 siblings, 1 reply; 17+ messages in thread
From: Amit Machhiwal @ 2026-08-07 12:07 UTC (permalink / raw)
To: Ritesh Harjani
Cc: Amit Machhiwal, linuxppc-dev, Madhavan Srinivasan, Vaibhav Jain,
Anushree Mathur, Paolo Bonzini, Nicholas Piggin, Michael Ellerman,
Christophe Leroy (CS GROUP), Jonathan Corbet, Shuah Khan, kvm,
linux-kernel, linux-doc, Gautam Menghani
On 2026/08/07 10:24 AM, Ritesh Harjani wrote:
> Amit Machhiwal <amachhiw@linux.ibm.com> writes:
<snip>
> >
> > static int kvmppc_get_compat_caps(struct kvm_ppc_compat_caps *host_caps)
> > {
> > + struct device_node *np;
> > unsigned long capabilities = 0;
> > long rc = -EINVAL;
> > + u32 cpu_version = 0;
> >
> > if (kvmhv_on_pseries()) {
> > if (kvmhv_is_nestedv2()) {
> > WARN_ON_ONCE(!nested_capabilities);
> > capabilities = nested_capabilities;
> > rc = 0;
> > + } else {
> > + for_each_node_by_type(np, "cpu") {
> > + if (!of_property_read_u32(np, "cpu-version",
> > + &cpu_version)) {
> > + of_node_put(np);
> > + break;
> > + }
> > + }
>
> What happens when we don't have "cpu-version" DT property?
> Check this commit
> 5a61ef74f269f2 ("powerpc/64s: Support new device tree binding for discovering CPU features")
>
> And also why do we need to parse the DT properties again?
> Shouldn't we check something like this?
>
> if (cpu_has_feature(CPU_FTR_P11_PVR))
> capabilities |= KVM_PPC_COMPAT_CAP_POWER11;
> if (cpu_has_feature(CPU_FTR_ARCH_31))
> capabilities |= KVM_PPC_COMPAT_CAP_POWER10;
> if (cpu_has_feature(CPU_FTR_ARCH_300))
> capabilities |= KVM_PPC_COMPAT_CAP_POWER9;
>
Thanks for the suggestion, Ritesh! After closer analysis, cpu_has_feature()
would give the same result on pseries, but I believe that using 'cpu-version'
directly is both more correct and more explicit for this context.
Here's why:
1. 'ibm,powerpc-cpu-features' / dt-cpu-ftrs is baremetal (powernv/OPAL) only.
SLOF firmware used by pseries guests never provides that node. On pseries,
dt_cpu_ftrs_in_use() is always false, and CPU feature bits including
CPU_FTR_ARCH_300, CPU_FTR_ARCH_31, CPU_FTR_P11_PVR are set by identify_cpu()
from the cpu-version DT property at prom.c:423:
if (!dt_cpu_ftrs_in_use()) {
prop = of_get_flat_dt_prop(node, "cpu-version", NULL);
if (prop && (be32_to_cpup(prop) & 0xff000000) == 0x0f000000) {
identify_cpu(0, be32_to_cpup(prop));
So cpu_has_feature() on pseries is just an indirect readback of what
identify_cpu() already derived from 'cpu-version' — the source of truth is
still cpu-version.
2. Commit e4de1b9cb3b5 ("powerpc/dt_cpu_ftrs: Set CPU_FTR_P11_PVR for Power11
and later processors") explicitly documents this split:
"This issue does not affect pseries guests, where SLOF firmware
does not provide this node, causing the kernel to fall back to
the traditional cputable path (identify_cpu) which correctly
sets CPU_FTR_P11_PVR during PVR-based CPU identification."
That fix was needed on powernv only — and our code is in the
kvmhv_on_pseries() branch, so the dt-cpu-ftrs path is never taken.
3. 'cpu-version' is the PAPR-defined compat level indicator — it is what PHYP
and QEMU explicitly set to communicate the negotiated compat mode. Reading it
directly is semantically correct: we are reporting the compat level the
hypervisor advertised, not a kernel-internal feature bit derived from it.
I also responded to a similar concern from Sashiko covering points 1 and 3 here:
https://lore.kernel.org/all/20260806214117.8a2ca150-97-amachhiw@linux.ibm.com/
So I'd prefer to keep the direct cpu-version lookup.
Thanks,
Amit
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 3/4] KVM: PPC: Book3S HV: Add support for compat CPU capabilities for KVM on PowerNV
2026-08-07 12:07 ` Amit Machhiwal
@ 2026-08-07 12:13 ` Ritesh Harjani
0 siblings, 0 replies; 17+ messages in thread
From: Ritesh Harjani @ 2026-08-07 12:13 UTC (permalink / raw)
To: Amit Machhiwal
Cc: Amit Machhiwal, linuxppc-dev, Madhavan Srinivasan, Vaibhav Jain,
Anushree Mathur, Paolo Bonzini, Nicholas Piggin, Michael Ellerman,
Christophe Leroy (CS GROUP), Jonathan Corbet, Shuah Khan, kvm,
linux-kernel, linux-doc, Gautam Menghani
Amit Machhiwal <amachhiw@linux.ibm.com> writes:
> On 2026/08/07 10:24 AM, Ritesh Harjani wrote:
>> Amit Machhiwal <amachhiw@linux.ibm.com> writes:
>
> <snip>
>
>> >
>> > static int kvmppc_get_compat_caps(struct kvm_ppc_compat_caps *host_caps)
>> > {
>> > + struct device_node *np;
>> > unsigned long capabilities = 0;
>> > long rc = -EINVAL;
>> > + u32 cpu_version = 0;
>> >
>> > if (kvmhv_on_pseries()) {
>> > if (kvmhv_is_nestedv2()) {
>> > WARN_ON_ONCE(!nested_capabilities);
>> > capabilities = nested_capabilities;
>> > rc = 0;
>> > + } else {
>> > + for_each_node_by_type(np, "cpu") {
>> > + if (!of_property_read_u32(np, "cpu-version",
>> > + &cpu_version)) {
>> > + of_node_put(np);
>> > + break;
>> > + }
>> > + }
>>
>> What happens when we don't have "cpu-version" DT property?
>> Check this commit
>> 5a61ef74f269f2 ("powerpc/64s: Support new device tree binding for discovering CPU features")
>>
>> And also why do we need to parse the DT properties again?
>> Shouldn't we check something like this?
>>
>> if (cpu_has_feature(CPU_FTR_P11_PVR))
>> capabilities |= KVM_PPC_COMPAT_CAP_POWER11;
>> if (cpu_has_feature(CPU_FTR_ARCH_31))
>> capabilities |= KVM_PPC_COMPAT_CAP_POWER10;
>> if (cpu_has_feature(CPU_FTR_ARCH_300))
>> capabilities |= KVM_PPC_COMPAT_CAP_POWER9;
>>
>
> Thanks for the suggestion, Ritesh! After closer analysis, cpu_has_feature()
> would give the same result on pseries, but I believe that using 'cpu-version'
> directly is both more correct and more explicit for this context.
>
> Here's why:
>
> 1. 'ibm,powerpc-cpu-features' / dt-cpu-ftrs is baremetal (powernv/OPAL) only.
Arghh. That make sense. Thanks for checking.
The design with "cpu-version" sounds good in that case.
Please feel free to add:
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 1/4] KVM: PPC: Introduce KVM_CAP_PPC_COMPAT_CAPS and wire up ioctl
2026-08-07 11:36 ` Ritesh Harjani
@ 2026-08-07 12:15 ` Ritesh Harjani
2026-08-07 13:04 ` Amit Machhiwal
1 sibling, 0 replies; 17+ messages in thread
From: Ritesh Harjani @ 2026-08-07 12:15 UTC (permalink / raw)
To: Amit Machhiwal
Cc: Amit Machhiwal, linuxppc-dev, Madhavan Srinivasan, Vaibhav Jain,
Anushree Mathur, Paolo Bonzini, Nicholas Piggin, Michael Ellerman,
Christophe Leroy (CS GROUP), Jonathan Corbet, Shuah Khan, kvm,
linux-kernel, linux-doc, Gautam Menghani
Ritesh Harjani (IBM) <ritesh.list@gmail.com> writes:
> Amit Machhiwal <amachhiw@linux.ibm.com> writes:
>
>> Hi Ritesh,
>>
>> Thanks for reviewing this patch. Please find my response inline below.
>>
>> On 2026/08/07 08:38 AM, Ritesh Harjani wrote:
>>> Amit Machhiwal <amachhiw@linux.ibm.com> writes:
>>>
>>> > Introduce a new capability and ioctl to expose CPU compatibility modes
>>> > supported by the host processor for nested guests.
>>> >
>>> > On IBM POWER systems, newer processor generations (N) can operate in
>>> > compatibility modes corresponding to earlier generations, like (N-1) and
>>> > (N-2). This is particularly relevant for nested virtualization, where
>>> > nested KVM guests may need to run with a specific processor compatibility
>>> > level.
>>> >
>>> > Introduce KVM_CAP_PPC_COMPAT_CAPS capability and the corresponding
>>> > KVM_PPC_GET_COMPAT_CAPS vm ioctl. The ioctl returns a bitmap describing
>>> > the compatibility modes supported by the host in respective bit numbers,
>>> > allowing userspace (e.g., QEMU) to select an appropriate compatibility
>>> > level when configuring nested KVM guests.
>>> >
>>> > The ioctl handling is added in kvm_arch_vm_ioctl() and retrieves host
>>> > CPU compatibility capabilities via a PowerPC-specific backend
>>> > implementation when available.
>>> >
>>> > The struct kvm_ppc_compat_caps places the 'size' field first so it can
>>> > be read alone via get_user() before copy_struct_from_user() is called,
>>> > avoiding pointer arithmetic to locate the size field.
>>> >
>>> > The ioctl is defined using _IO so the ioctl number remains stable even if
>>> > the struct grows in future versions. It uses copy_struct_from_user() and
>>> > copy_struct_to_user() to provide forward- and backward-compatible
>>> > extensibility: older userspace passing a smaller struct to a newer kernel
>>> > gets zero-padded trailing fields, while newer userspace passing a larger
>>> > struct to an older kernel (usize > ksize) gets sizeof(struct
>>> > kvm_ppc_compat_caps) written back to host_caps.size so it can retry with the
>>> > older kernel-supported size, after which the kernel returns -E2BIG.
>>> >
>>> > KVM_PPC_COMPAT_CAPS_SIZE_VER0 is defined as a frozen integer constant
>>> > (24) marking the size of the initial struct version, used as the
>>> > minimum floor for size field validation, similar to other versioned
>>> > struct interfaces in the kernel.
>>> >
>>> > The 'flags' field is reserved for future use. The kernel rejects any
>>> > call where flags is non-zero with -EINVAL, preventing garbage values
>>> > from being baked into ABI permanently.
>>> >
>>> > The ioctl returns appropriate error codes: EINVAL for an invalid size
>>> > or non-zero reserved fields, E2BIG if new userspace provides a larger
>>> > struct than the kernel knows about (with ksize written back into
>>> > host_caps.size for the retry), EFAULT for failed copy operations, and
>>> > ENOTTY if the backend doesn't implement get_compat_caps.
>>> >
>>> > Suggested-by: Vaibhav Jain <vaibhav@linux.ibm.com>
>>> > Tested-by: Gautam Menghani <gautam@linux.ibm.com>
>>> > Reviewed-by: Gautam Menghani <gautam@linux.ibm.com>
>>> > Tested-by: Anushree Mathur <anushree.mathur@linux.ibm.com>
>>> > Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com>
>>> > ---
>>> > Changes in this version:
>>> > - KVM_CAP_PPC_COMPAT_CAPS: add hv_enabled guard to align the capability
>>> > check with ioctl availability; a PR KVM VM on pseries now correctly
>>> > returns 0 for the capability [Sashiko]
>>> >
>>> > arch/powerpc/include/asm/kvm_ppc.h | 1 +
>>> > arch/powerpc/include/uapi/asm/kvm.h | 8 ++++
>>> > arch/powerpc/kvm/powerpc.c | 71 +++++++++++++++++++++++++++++
>>> > include/uapi/linux/kvm.h | 3 ++
>>> > 4 files changed, 83 insertions(+)
>>> >
>>> > diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
>>> > index 0953f2daa466..169ea6a7fbad 100644
>>> > --- a/arch/powerpc/include/asm/kvm_ppc.h
>>> > +++ b/arch/powerpc/include/asm/kvm_ppc.h
>>> > @@ -319,6 +319,7 @@ struct kvmppc_ops {
>>> > bool (*hash_v3_possible)(void);
>>> > int (*create_vm_debugfs)(struct kvm *kvm);
>>> > int (*create_vcpu_debugfs)(struct kvm_vcpu *vcpu, struct dentry *debugfs_dentry);
>>> > + int (*get_compat_caps)(struct kvm_ppc_compat_caps *host_caps);
>>> > };
>>> >
>>> > extern struct kvmppc_ops *kvmppc_hv_ops;
>>> > diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
>>> > index 077c5437f521..19e53d5ae540 100644
>>> > --- a/arch/powerpc/include/uapi/asm/kvm.h
>>> > +++ b/arch/powerpc/include/uapi/asm/kvm.h
>>> > @@ -437,6 +437,14 @@ struct kvm_ppc_cpu_char {
>>> > __u64 behaviour_mask; /* valid bits in behaviour */
>>> > };
>>> >
>>> > +/* For KVM_PPC_GET_COMPAT_CAPS */
>>> > +struct kvm_ppc_compat_caps {
>>> > + __u64 size; /* Size of this structure */
>>> > + __u64 flags; /* Reserved for future use */
>>> > + __u64 compat_capabilities; /* Capabilities supported by the host */
>>> > +};
>>> > +#define KVM_PPC_COMPAT_CAPS_SIZE_VER0 24 /* sizeof first published struct */
>>> > +
>>> > /*
>>> > * Values for character and character_mask.
>>> > * These are identical to the values used by H_GET_CPU_CHARACTERISTICS.
>>> > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
>>> > index b6b83fe3233f..e64b3cfadd3a 100644
>>> > --- a/arch/powerpc/kvm/powerpc.c
>>> > +++ b/arch/powerpc/kvm/powerpc.c
>>> > @@ -703,6 +703,13 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>>> > }
>>> > }
>>> > break;
>>> > +#if defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE)
>>> > + case KVM_CAP_PPC_COMPAT_CAPS:
>>> > + r = 0;
>>> > + if (hv_enabled && kvmhv_on_pseries())
>>>
>>> I think sashiko is just complaining in the 1st patch because we have not
>>> yet wired up the kvmppc_hv_ops->get_compat_caps() yet in patch-1. I
>>> think it is expecting..
>>>
>>> if (hv_enabled && kvmhv_on_pseries() && kvmppc_hv_ops->get_compat_caps)
>>>
>>> But either way is fine.
>>>
>>>
>>> > + r = 1;
>>> > + break;
>>> > +#endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */
>>> > default:
>>> > r = 0;
>>> > break;
>>> > @@ -2469,6 +2476,70 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
>>> > r = kvm->arch.kvm_ops->svm_off(kvm);
>>> > break;
>>> > }
>>> > + case KVM_PPC_GET_COMPAT_CAPS: {
>>> > + struct kvm_ppc_compat_caps host_caps = {};
>>> > + u64 usize;
>>> > +
>>> > + /*
>>> > + * Read the size field first to drive copy_struct_from_user.
>>> > + * size must be the first field of the struct.
>>> > + */
>>> > + r = -EFAULT;
>>> > + if (get_user(usize, (__u64 __user *)argp))
>>> > + goto out;
>>> > +
>>> > + /*
>>> > + * Enforce a minimum: reject buffers smaller than the initial
>>> > + * struct version (VER0). This allows old userspace compiled
>>> > + * against the original struct to still work on a newer kernel
>>> > + * that has grown the struct with appended fields.
>>> > + */
>>> > + r = -EINVAL;
>>> > + if (usize < KVM_PPC_COMPAT_CAPS_SIZE_VER0)
>>> > + goto out;
>>> > +
>>> > + /*
>>> > + * New userspace with a larger struct called an older kernel.
>>> > + * Write back ksize in host_caps.size so userspace knows which
>>> > + * older struct to retry with, then fail with -E2BIG.
>>> > + */
>>> > + if (usize > sizeof(host_caps)) {
>>> > + host_caps.size = sizeof(host_caps);
>>> > + r = -EFAULT;
>>> > + if (put_user(host_caps.size, (__u64 __user *)argp))
>>> > + goto out;
>>> > + r = -E2BIG;
>>> > + goto out;
>>> > + }
>>>
>>> You anyways mentioned copy_struct_from_user() is taking care of both
>>> forward and backward compat. Then what is the point of this check?
>>> shouldn't we get rid of this complete if logic? I don't see a point of
>>> this if we are anyway using copy_struct_from_user().
>>
>> The pre-check is intentional and serves a purpose that
>> copy_struct_from_user() alone cannot provide: explicit kernel struct
>> size discovery.
>>
>> copy_struct_from_user() returns -E2BIG when usize > ksize and trailing
>> bytes are non-zero — but it gives userspace no way to know what ksize to
>> retry with. It also silently succeeds when trailing bytes are zero,
>> which means a new userspace on an old kernel would never learn the
>> kernel's struct size at all.
>>
>> This design is different by intent: when new userspace passes a larger
>> struct, we always return -E2BIG and write back sizeof(host_caps) into
>> the size field so userspace learns the exact kernel-supported size and
>> can retry with it. This explicit negotiation is verified in the ABI
>> extensibility testing in the cover letter ("Newer struct on QEMU, older
>> kernel -> works"). The explicit -E2BIG + ksize writeback makes the
>> version negotiation unambiguous.
>>
>
> yup, my bad, should have seen the comment in the code above.
>
> But isn't this a bad design? Tomorrow, say we have a v2, a larger
> version of this struct and if your newer Qemu (with larger struct size)
> is running on an older kernel with a smaller struct size, then you will
> fail it because usize > ksize even though the userspace has zero filled
> the rest of the trailing bytes...
> if (usize > sizeof(host_caps))
>
> Instead wouldn't it be better if we do something like this?
>
> r = copy_struct_from_user(&host_caps, sizeof(host_caps),
> argp, usize);
> if (r) {
> if (r == -E2BIG)
> put_user(sizeof(host_caps), &argp->size);
> goto out;
> }
>
> So, we only fail if the userspace has passed a newer struct and the
> trailing bytes, which are unknown to this older kernel, are zeroed,
> otherwise we return -E2BIG.
Sorry made some typos in above paragraph, what I meant is -
So we only fail with -E2BIG if the userspace has passed a newer struct
and the trailing bytes, which are unknown to this older kernel, are
non-zero. if they're all zero, the call succeeds, since those bytes are
safe to ignore. We also write the ksize back on the failure path, so the
user knows what ksize this kernel supports.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 1/4] KVM: PPC: Introduce KVM_CAP_PPC_COMPAT_CAPS and wire up ioctl
2026-08-07 11:36 ` Ritesh Harjani
2026-08-07 12:15 ` Ritesh Harjani
@ 2026-08-07 13:04 ` Amit Machhiwal
1 sibling, 0 replies; 17+ messages in thread
From: Amit Machhiwal @ 2026-08-07 13:04 UTC (permalink / raw)
To: Ritesh Harjani
Cc: Amit Machhiwal, linuxppc-dev, Madhavan Srinivasan, Vaibhav Jain,
Anushree Mathur, Paolo Bonzini, Nicholas Piggin, Michael Ellerman,
Christophe Leroy (CS GROUP), Jonathan Corbet, Shuah Khan, kvm,
linux-kernel, linux-doc, Gautam Menghani
On 2026/08/07 05:06 PM, Ritesh Harjani wrote:
> Amit Machhiwal <amachhiw@linux.ibm.com> writes:
>
> > Hi Ritesh,
> >
> > Thanks for reviewing this patch. Please find my response inline below.
> >
> > On 2026/08/07 08:38 AM, Ritesh Harjani wrote:
> >> Amit Machhiwal <amachhiw@linux.ibm.com> writes:
> >>
> >> > Introduce a new capability and ioctl to expose CPU compatibility modes
> >> > supported by the host processor for nested guests.
> >> >
> >> > On IBM POWER systems, newer processor generations (N) can operate in
> >> > compatibility modes corresponding to earlier generations, like (N-1) and
> >> > (N-2). This is particularly relevant for nested virtualization, where
> >> > nested KVM guests may need to run with a specific processor compatibility
> >> > level.
> >> >
> >> > Introduce KVM_CAP_PPC_COMPAT_CAPS capability and the corresponding
> >> > KVM_PPC_GET_COMPAT_CAPS vm ioctl. The ioctl returns a bitmap describing
> >> > the compatibility modes supported by the host in respective bit numbers,
> >> > allowing userspace (e.g., QEMU) to select an appropriate compatibility
> >> > level when configuring nested KVM guests.
> >> >
> >> > The ioctl handling is added in kvm_arch_vm_ioctl() and retrieves host
> >> > CPU compatibility capabilities via a PowerPC-specific backend
> >> > implementation when available.
> >> >
> >> > The struct kvm_ppc_compat_caps places the 'size' field first so it can
> >> > be read alone via get_user() before copy_struct_from_user() is called,
> >> > avoiding pointer arithmetic to locate the size field.
> >> >
> >> > The ioctl is defined using _IO so the ioctl number remains stable even if
> >> > the struct grows in future versions. It uses copy_struct_from_user() and
> >> > copy_struct_to_user() to provide forward- and backward-compatible
> >> > extensibility: older userspace passing a smaller struct to a newer kernel
> >> > gets zero-padded trailing fields, while newer userspace passing a larger
> >> > struct to an older kernel (usize > ksize) gets sizeof(struct
> >> > kvm_ppc_compat_caps) written back to host_caps.size so it can retry with the
> >> > older kernel-supported size, after which the kernel returns -E2BIG.
> >> >
> >> > KVM_PPC_COMPAT_CAPS_SIZE_VER0 is defined as a frozen integer constant
> >> > (24) marking the size of the initial struct version, used as the
> >> > minimum floor for size field validation, similar to other versioned
> >> > struct interfaces in the kernel.
> >> >
> >> > The 'flags' field is reserved for future use. The kernel rejects any
> >> > call where flags is non-zero with -EINVAL, preventing garbage values
> >> > from being baked into ABI permanently.
> >> >
> >> > The ioctl returns appropriate error codes: EINVAL for an invalid size
> >> > or non-zero reserved fields, E2BIG if new userspace provides a larger
> >> > struct than the kernel knows about (with ksize written back into
> >> > host_caps.size for the retry), EFAULT for failed copy operations, and
> >> > ENOTTY if the backend doesn't implement get_compat_caps.
> >> >
> >> > Suggested-by: Vaibhav Jain <vaibhav@linux.ibm.com>
> >> > Tested-by: Gautam Menghani <gautam@linux.ibm.com>
> >> > Reviewed-by: Gautam Menghani <gautam@linux.ibm.com>
> >> > Tested-by: Anushree Mathur <anushree.mathur@linux.ibm.com>
> >> > Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com>
> >> > ---
> >> > Changes in this version:
> >> > - KVM_CAP_PPC_COMPAT_CAPS: add hv_enabled guard to align the capability
> >> > check with ioctl availability; a PR KVM VM on pseries now correctly
> >> > returns 0 for the capability [Sashiko]
> >> >
> >> > arch/powerpc/include/asm/kvm_ppc.h | 1 +
> >> > arch/powerpc/include/uapi/asm/kvm.h | 8 ++++
> >> > arch/powerpc/kvm/powerpc.c | 71 +++++++++++++++++++++++++++++
> >> > include/uapi/linux/kvm.h | 3 ++
> >> > 4 files changed, 83 insertions(+)
> >> >
> >> > diff --git a/arch/powerpc/include/asm/kvm_ppc.h b/arch/powerpc/include/asm/kvm_ppc.h
> >> > index 0953f2daa466..169ea6a7fbad 100644
> >> > --- a/arch/powerpc/include/asm/kvm_ppc.h
> >> > +++ b/arch/powerpc/include/asm/kvm_ppc.h
> >> > @@ -319,6 +319,7 @@ struct kvmppc_ops {
> >> > bool (*hash_v3_possible)(void);
> >> > int (*create_vm_debugfs)(struct kvm *kvm);
> >> > int (*create_vcpu_debugfs)(struct kvm_vcpu *vcpu, struct dentry *debugfs_dentry);
> >> > + int (*get_compat_caps)(struct kvm_ppc_compat_caps *host_caps);
> >> > };
> >> >
> >> > extern struct kvmppc_ops *kvmppc_hv_ops;
> >> > diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
> >> > index 077c5437f521..19e53d5ae540 100644
> >> > --- a/arch/powerpc/include/uapi/asm/kvm.h
> >> > +++ b/arch/powerpc/include/uapi/asm/kvm.h
> >> > @@ -437,6 +437,14 @@ struct kvm_ppc_cpu_char {
> >> > __u64 behaviour_mask; /* valid bits in behaviour */
> >> > };
> >> >
> >> > +/* For KVM_PPC_GET_COMPAT_CAPS */
> >> > +struct kvm_ppc_compat_caps {
> >> > + __u64 size; /* Size of this structure */
> >> > + __u64 flags; /* Reserved for future use */
> >> > + __u64 compat_capabilities; /* Capabilities supported by the host */
> >> > +};
> >> > +#define KVM_PPC_COMPAT_CAPS_SIZE_VER0 24 /* sizeof first published struct */
> >> > +
> >> > /*
> >> > * Values for character and character_mask.
> >> > * These are identical to the values used by H_GET_CPU_CHARACTERISTICS.
> >> > diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
> >> > index b6b83fe3233f..e64b3cfadd3a 100644
> >> > --- a/arch/powerpc/kvm/powerpc.c
> >> > +++ b/arch/powerpc/kvm/powerpc.c
> >> > @@ -703,6 +703,13 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> >> > }
> >> > }
> >> > break;
> >> > +#if defined(CONFIG_KVM_BOOK3S_HV_POSSIBLE)
> >> > + case KVM_CAP_PPC_COMPAT_CAPS:
> >> > + r = 0;
> >> > + if (hv_enabled && kvmhv_on_pseries())
> >>
> >> I think sashiko is just complaining in the 1st patch because we have not
> >> yet wired up the kvmppc_hv_ops->get_compat_caps() yet in patch-1. I
> >> think it is expecting..
> >>
> >> if (hv_enabled && kvmhv_on_pseries() && kvmppc_hv_ops->get_compat_caps)
> >>
> >> But either way is fine.
> >>
> >>
> >> > + r = 1;
> >> > + break;
> >> > +#endif /* CONFIG_KVM_BOOK3S_HV_POSSIBLE */
> >> > default:
> >> > r = 0;
> >> > break;
> >> > @@ -2469,6 +2476,70 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
> >> > r = kvm->arch.kvm_ops->svm_off(kvm);
> >> > break;
> >> > }
> >> > + case KVM_PPC_GET_COMPAT_CAPS: {
> >> > + struct kvm_ppc_compat_caps host_caps = {};
> >> > + u64 usize;
> >> > +
> >> > + /*
> >> > + * Read the size field first to drive copy_struct_from_user.
> >> > + * size must be the first field of the struct.
> >> > + */
> >> > + r = -EFAULT;
> >> > + if (get_user(usize, (__u64 __user *)argp))
> >> > + goto out;
> >> > +
> >> > + /*
> >> > + * Enforce a minimum: reject buffers smaller than the initial
> >> > + * struct version (VER0). This allows old userspace compiled
> >> > + * against the original struct to still work on a newer kernel
> >> > + * that has grown the struct with appended fields.
> >> > + */
> >> > + r = -EINVAL;
> >> > + if (usize < KVM_PPC_COMPAT_CAPS_SIZE_VER0)
> >> > + goto out;
> >> > +
> >> > + /*
> >> > + * New userspace with a larger struct called an older kernel.
> >> > + * Write back ksize in host_caps.size so userspace knows which
> >> > + * older struct to retry with, then fail with -E2BIG.
> >> > + */
> >> > + if (usize > sizeof(host_caps)) {
> >> > + host_caps.size = sizeof(host_caps);
> >> > + r = -EFAULT;
> >> > + if (put_user(host_caps.size, (__u64 __user *)argp))
> >> > + goto out;
> >> > + r = -E2BIG;
> >> > + goto out;
> >> > + }
> >>
> >> You anyways mentioned copy_struct_from_user() is taking care of both
> >> forward and backward compat. Then what is the point of this check?
> >> shouldn't we get rid of this complete if logic? I don't see a point of
> >> this if we are anyway using copy_struct_from_user().
> >
> > The pre-check is intentional and serves a purpose that
> > copy_struct_from_user() alone cannot provide: explicit kernel struct
> > size discovery.
> >
> > copy_struct_from_user() returns -E2BIG when usize > ksize and trailing
> > bytes are non-zero — but it gives userspace no way to know what ksize to
> > retry with. It also silently succeeds when trailing bytes are zero,
> > which means a new userspace on an old kernel would never learn the
> > kernel's struct size at all.
> >
> > This design is different by intent: when new userspace passes a larger
> > struct, we always return -E2BIG and write back sizeof(host_caps) into
> > the size field so userspace learns the exact kernel-supported size and
> > can retry with it. This explicit negotiation is verified in the ABI
> > extensibility testing in the cover letter ("Newer struct on QEMU, older
> > kernel -> works"). The explicit -E2BIG + ksize writeback makes the
> > version negotiation unambiguous.
> >
>
> yup, my bad, should have seen the comment in the code above.
No worries...
>
> But isn't this a bad design? Tomorrow, say we have a v2, a larger
> version of this struct and if your newer Qemu (with larger struct size)
> is running on an older kernel with a smaller struct size, then you will
> fail it because usize > ksize even though the userspace has zero filled
> the rest of the trailing bytes...
You're right and thanks for pointing this out. A newer userspace that
zero-initialises the new fields (e.g. struct caps = {}; caps.size =
sizeof(caps);) is a valid forward-compat call — zero means "use default
for unknown fields", which is exactly the contract
copy_struct_from_user() is designed to honour. Forcing a round-trip in
that case is wrong.
I'll drop the manual usize > sizeof(host_caps) pre-check and let
copy_struct_from_user() own the usize > ksize path entirely.
One small correction to your suggested snippet though: put_user()
return value is ignored there. put_user() returns 0 on success or
-EFAULT on failure, so if the write-back fails we'd silently return
-E2BIG instead of -EFAULT. The fix is straightforward:
diff --git a/arch/powerpc/kvm/powerpc.c b/arch/powerpc/kvm/powerpc.c
index e64b3cfadd3a..ed48f069fb76 100644
--- a/arch/powerpc/kvm/powerpc.c
+++ b/arch/powerpc/kvm/powerpc.c
@@ -2498,29 +2498,28 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
if (usize < KVM_PPC_COMPAT_CAPS_SIZE_VER0)
goto out;
- /*
- * New userspace with a larger struct called an older kernel.
- * Write back ksize in host_caps.size so userspace knows which
- * older struct to retry with, then fail with -E2BIG.
- */
- if (usize > sizeof(host_caps)) {
- host_caps.size = sizeof(host_caps);
- r = -EFAULT;
- if (put_user(host_caps.size, (__u64 __user *)argp))
- goto out;
- r = -E2BIG;
- goto out;
- }
-
/*
* copy_struct_from_user() handles forward/backward compat:
* usize == ksize: verbatim copy
* usize < ksize: zero-pad trailing (old userspace, new kernel)
+ * usize > ksize: succeed iff trailing bytes are zero, else -E2BIG
*/
r = copy_struct_from_user(&host_caps, sizeof(host_caps),
argp, usize);
- if (r)
+ if (r) {
+ /*
+ * New userspace with a larger struct called an older
+ * kernel. Write back ksize in host_caps.size so
+ * userspace knows which older struct to retry with,
+ * then fail with -E2BIG.
+ */
+ if (r == -E2BIG)
+ if (put_user((__u64)sizeof(host_caps),
+ (__u64 __user *)argp))
+ r = -EFAULT;
goto out;
+ }
/* Reserved fields must be zero */
r = -EINVAL;
This preserves -EFAULT priority if put_user() fails, consistent with
how the get_user() guard earlier in the same handler works.
Will post v8 with this fix.
Thanks,
Amit
> if (usize > sizeof(host_caps))
>
> Instead wouldn't it be better if we do something like this?
>
> r = copy_struct_from_user(&host_caps, sizeof(host_caps),
> argp, usize);
> if (r) {
> if (r == -E2BIG)
> put_user(sizeof(host_caps), &argp->size);
> goto out;
> }
>
> So, we only fail if the userspace has passed a newer struct and the
> trailing bytes, which are unknown to this older kernel, are zeroed,
> otherwise we return -E2BIG. We also write the ksize back, so the user
> knows what ksize it supports.
>
> This would avoid an unncessary round trip and will simplify the
> userspace design, isn't it?
>
> >>
<snip>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v7 4/4] KVM: PPC: Document KVM_PPC_GET_COMPAT_CAPS ioctl
2026-08-07 4:35 ` Ritesh Harjani
@ 2026-08-07 13:36 ` Amit Machhiwal
0 siblings, 0 replies; 17+ messages in thread
From: Amit Machhiwal @ 2026-08-07 13:36 UTC (permalink / raw)
To: Ritesh Harjani
Cc: Amit Machhiwal, linuxppc-dev, Madhavan Srinivasan, Vaibhav Jain,
Anushree Mathur, Paolo Bonzini, Nicholas Piggin, Michael Ellerman,
Christophe Leroy (CS GROUP), Jonathan Corbet, Shuah Khan, kvm,
linux-kernel, linux-doc, Gautam Menghani
On 2026/08/07 10:05 AM, Ritesh Harjani wrote:
> Amit Machhiwal <amachhiw@linux.ibm.com> writes:
>
> > Add documentation for the KVM_PPC_GET_COMPAT_CAPS ioctl to the KVM API
> > documentation.
> >
> > The ioctl exposes host processor compatibility modes supported for
> > nested KVM guests on PowerPC systems. The documentation covers error
> > code descriptions including E2BIG for forward compatibility, the
> > extensible size-based versioning contract using
> > KVM_PPC_COMPAT_CAPS_SIZE_VER0, the rationale for rejecting non-zero
> > reserved fields to prevent ABI ambiguity, bit numbering clarification
> > for IBM MSB-0 convention, and KVM-specific capability bit constants.
> >
> > Tested-by: Gautam Menghani <gautam@linux.ibm.com>
> > Reviewed-by: Gautam Menghani <gautam@linux.ibm.com>
> > Tested-by: Anushree Mathur <anushree.mathur@linux.ibm.com>
> > Signed-off-by: Amit Machhiwal <amachhiw@linux.ibm.com>
> > ---
> > Documentation/virt/kvm/api.rst | 79 ++++++++++++++++++++++++++++++++++
> > 1 file changed, 79 insertions(+)
> >
> > diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> > index e3003a241d5b..22fedb0aa34b 100644
> > --- a/Documentation/virt/kvm/api.rst
> > +++ b/Documentation/virt/kvm/api.rst
> > @@ -6566,6 +6566,85 @@ KVM_S390_KEYOP_SSKE
> > Sets the storage key for the guest address ``guest_addr`` to the key
> > specified in ``key``, returning the previous value in ``key``.
> >
> > +4.145 KVM_PPC_GET_COMPAT_CAPS
> > +-----------------------------
> > +:Capability: KVM_CAP_PPC_COMPAT_CAPS
> > +:Architectures: powerpc
> > +:Type: vm ioctl
> > +:Parameters: struct kvm_ppc_compat_caps (in/out)
> > +:Returns: 0 on success, negative value on failure
> > +
> > +Errors include:
> > +
> > + ======== ============================================================
> > + EFAULT if ``struct kvm_ppc_compat_caps`` cannot be read from or
> > + written to userspace
> > + EINVAL if the ``size`` field is smaller than
> > + ``KVM_PPC_COMPAT_CAPS_SIZE_VER0``, if the ``flags`` field
> > + is non-zero, or if the backend fails to retrieve or map
> > + CPU compatibility capabilities
> > + E2BIG if ``size`` is larger than the kernel's struct size
> > + (new userspace on old kernel); the kernel writes back its
> > + own struct size into the ``size`` field so userspace can
> > + retry with the correct size
> > + ENOTTY if the backend does not implement the ``get_compat_caps``
> > + operation (e.g., on non-HV KVM implementations where the
> > + required KVM operations are not available)
>
> Amit, this may not be true anymore right after your changes in v7?
> Can we please update the documentation accordingly as well.
Agreed. After dropping the manual pre-check in patch-1 and delegating to
copy_struct_from_user(), -E2BIG is no longer unconditional when usize >
ksize — it only fires if the unknown trailing bytes are non-zero. Will
update the E2BIG entry to:
E2BIG if ``size`` is larger than the kernel's struct size and
the unknown trailing bytes are non-zero (new userspace on
old kernel with non-default fields set); the kernel writes
back its own struct size into the ``size`` field so
userspace can retry with the correct size
>
> > + ======== ============================================================
> > +
> > +IBM POWER system server-based processors provide a compatibility mode feature
> > +where an Nth generation processor can operate in modes consistent with earlier
> > +generations such as (N-1) and (N-2).
> > +
> > +This ioctl provides userspace with information about the CPU compatibility modes
> > +supported by the current host processor for booting the nested KVM guests on
> > +KVM on PowerNV (nested API v1) and KVM on PowerVM (nested API v2) platforms.
> > +
> > +::
> > +
> > + struct kvm_ppc_compat_caps {
> > + __u64 size; /* Size of this structure */
> > + __u64 flags; /* Reserved for future use, must be 0 */
> > + __u64 compat_capabilities; /* Capabilities supported by the host */
> > + };
> > +
> > +Before calling this ioctl, userspace must set the ``size`` field to
> > +``sizeof(struct kvm_ppc_compat_caps)`` and zero the ``flags`` field.
> > +The kernel rejects non-zero ``flags`` with ``-EINVAL`` to prevent
> > +uninitialized stack values from being silently accepted, keeping the
> > +field available for future use without ABI ambiguity.
> > +
> > +The ioctl uses ``copy_struct_from_user()`` and ``copy_struct_to_user()``
> > +to support extensible versioning: if userspace passes a struct smaller
> > +than the current kernel version (``size >= KVM_PPC_COMPAT_CAPS_SIZE_VER0``),
> > +the kernel zero-pads unknown trailing fields. If userspace passes a larger
>
> So I already requested that we should fix this. We cannot write more
> bytes than requested by the user, since that memory may not be allocated
> for this struct in userspace.
>
> On checking Sashiko comments in reply to this patch - I think that is
> also complaining of the same thing that it could cause buffer overflow.
copy_struct_to_user() itself is safe — it caps its write to min(ksize,
usize) bytes so it never writes past the user's buffer. However, the
problem is in the value written back in the size field: if usize <
sizeof(host_caps) (old userspace, new kernel), we'd write size =
sizeof(host_caps) into the first 8 bytes of the user's smaller buffer.
If userspace then reuses the struct naively, it would pass usize =
sizeof(host_caps) against its smaller allocation, which would cause an
actual overflow on the next call.
The fix is to write back usize instead:
host_caps.size = usize;
r = copy_struct_to_user(argp, usize, &host_caps,
sizeof(host_caps), NULL);
This tells userspace "I filled exactly as many bytes as you gave me",
which is the correct contract for copy_struct_to_user().
Will update both the code in patch-1 and the versioning paragraph in the
documentation accordingly in v8.
Thanks,
Amit
>
>
> > +struct (``size > sizeof(struct kvm_ppc_compat_caps)``), the kernel writes
> > +back its own struct size into the ``size`` field and returns ``-E2BIG``,
> > +allowing userspace to discover the kernel's struct size and retry.
> > +``KVM_PPC_COMPAT_CAPS_SIZE_VER0`` (24) is a frozen constant marking the
> > +size of the initial struct version.
>
> Once we update the comments in patch-1 - I think we should correct this
> documentation too accordingly. We should just simply use
> copy_to|from_user_struct() style for doing this.
>
> > +
> > +The ``compat_capabilities`` bit field describes the processor compatibility
> > +modes supported by the host. The following bits indicate support for specific
> > +processor modes (using IBM's MSB-0 convention where bit 0 is the most
> > +significant bit):
> > +
> > +- ``KVM_PPC_COMPAT_CAP_POWER9`` (bit 1) -- KVM guests can run in Power9 processor mode
> > +- ``KVM_PPC_COMPAT_CAP_POWER10`` (bit 2) -- KVM guests can run in Power10 processor mode
> > +- ``KVM_PPC_COMPAT_CAP_POWER11`` (bit 3) -- KVM guests can run in Power11 processor mode
> > +
> > +.. note::
> > +
> > + The bit numbering above uses IBM's MSB-0 convention (bit 0 is the most
> > + significant bit). In the actual implementation, these are defined as:
> > +
> > + - ``KVM_PPC_COMPAT_CAP_POWER9`` = ``(1ULL << 62)``
> > + - ``KVM_PPC_COMPAT_CAP_POWER10`` = ``(1ULL << 61)``
> > + - ``KVM_PPC_COMPAT_CAP_POWER11`` = ``(1ULL << 60)``
> > +
> > + Userspace should use the defined constants from ``<linux/kvm.h>`` rather
> > + than hardcoding bit positions.
> > +
> > .. _kvm_run:
> >
> > 5. The kvm_run structure
> > --
> > 2.50.1 (Apple Git-155)
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2026-08-07 13:36 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-06 17:06 [PATCH v7 0/4] KVM: PPC: Expose CPU compatibility modes for nested guests Amit Machhiwal
2026-08-06 17:06 ` [PATCH v7 1/4] KVM: PPC: Introduce KVM_CAP_PPC_COMPAT_CAPS and wire up ioctl Amit Machhiwal
2026-08-07 3:08 ` Ritesh Harjani
2026-08-07 10:55 ` Amit Machhiwal
2026-08-07 11:36 ` Ritesh Harjani
2026-08-07 12:15 ` Ritesh Harjani
2026-08-07 13:04 ` Amit Machhiwal
2026-08-06 17:06 ` [PATCH v7 2/4] KVM: PPC: Book3S HV: Implement compat CPU capability retrieval for KVM on PowerVM Amit Machhiwal
2026-08-07 4:31 ` Ritesh Harjani
2026-08-07 10:58 ` Amit Machhiwal
2026-08-06 17:06 ` [PATCH v7 3/4] KVM: PPC: Book3S HV: Add support for compat CPU capabilities for KVM on PowerNV Amit Machhiwal
2026-08-07 4:54 ` Ritesh Harjani
2026-08-07 12:07 ` Amit Machhiwal
2026-08-07 12:13 ` Ritesh Harjani
2026-08-06 17:06 ` [PATCH v7 4/4] KVM: PPC: Document KVM_PPC_GET_COMPAT_CAPS ioctl Amit Machhiwal
2026-08-07 4:35 ` Ritesh Harjani
2026-08-07 13:36 ` Amit Machhiwal
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox