* [PATCH v1 0/4] KVM: arm64: Update KVM_VCPU_MAX_FEATURES and refactor to avoid same issue
@ 2024-10-14 16:58 Fuad Tabba
2024-10-14 16:58 ` [PATCH v1 1/4] KVM: arm64: Update the value of KVM_VCPU_MAX_FEATURES Fuad Tabba
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Fuad Tabba @ 2024-10-14 16:58 UTC (permalink / raw)
To: kvmarm
Cc: maz, oliver.upton, catalin.marinas, joey.gouly, suzuki.poulose,
yuzenghui, will, christoffer.dall, tabba
The value of KVM_VCPU_MAX_FEATURES has not been updated since
adding new features in commit 89b0e7de3451 ("KVM: arm64: nv:
Introduce nested virtualization VCPU feature").
This patch series updates its value and refactors VCPU feature
values as well as KVM arch flags to reduce the chances of this
happening again.
Based on Linux 6.12-rc3 (8e929cb546ee).
Cheers,
/fuad
Fuad Tabba (4):
KVM: arm64: Update the value of KVM_VCPU_MAX_FEATURES
KVM: arm64: Move KVM_VCPU_MAX_FEATURES to the features it is counting
KVM: arm64: Convert KVM_ARM_VCPU_* features into an enum
KVM: arm64: Convert KVM_ARCH_FLAG_* into an enum
arch/arm64/include/asm/kvm_host.h | 58 +++++++++++++++++--------------
arch/arm64/include/uapi/asm/kvm.h | 22 +++++++-----
arch/arm64/kvm/arm.c | 12 ++++---
3 files changed, 52 insertions(+), 40 deletions(-)
base-commit: 8e929cb546ee42c9a61d24fae60605e9e3192354
--
2.47.0.rc1.288.g06298d1525-goog
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v1 1/4] KVM: arm64: Update the value of KVM_VCPU_MAX_FEATURES
2024-10-14 16:58 [PATCH v1 0/4] KVM: arm64: Update KVM_VCPU_MAX_FEATURES and refactor to avoid same issue Fuad Tabba
@ 2024-10-14 16:58 ` Fuad Tabba
2024-10-14 16:58 ` [PATCH v1 2/4] KVM: arm64: Move KVM_VCPU_MAX_FEATURES to the features it is counting Fuad Tabba
` (3 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Fuad Tabba @ 2024-10-14 16:58 UTC (permalink / raw)
To: kvmarm
Cc: maz, oliver.upton, catalin.marinas, joey.gouly, suzuki.poulose,
yuzenghui, will, christoffer.dall, tabba
There are currently 8 CPU features, but the value of
KVM_VCPU_MAX_FEATURES hasn't been updated to reflect that.
Fixes: 89b0e7de3451 ("KVM: arm64: nv: Introduce nested virtualization VCPU feature")
Signed-off-by: Fuad Tabba <tabba@google.com>
---
arch/arm64/include/asm/kvm_host.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 94cff508874b..78c068592d73 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -39,7 +39,7 @@
#define KVM_MAX_VCPUS VGIC_V3_MAX_CPUS
-#define KVM_VCPU_MAX_FEATURES 7
+#define KVM_VCPU_MAX_FEATURES 8
#define KVM_VCPU_VALID_FEATURES (BIT(KVM_VCPU_MAX_FEATURES) - 1)
#define KVM_REQ_SLEEP \
--
2.47.0.rc1.288.g06298d1525-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v1 2/4] KVM: arm64: Move KVM_VCPU_MAX_FEATURES to the features it is counting
2024-10-14 16:58 [PATCH v1 0/4] KVM: arm64: Update KVM_VCPU_MAX_FEATURES and refactor to avoid same issue Fuad Tabba
2024-10-14 16:58 ` [PATCH v1 1/4] KVM: arm64: Update the value of KVM_VCPU_MAX_FEATURES Fuad Tabba
@ 2024-10-14 16:58 ` Fuad Tabba
2024-10-14 16:58 ` [PATCH v1 3/4] KVM: arm64: Convert KVM_ARM_VCPU_* features into an enum Fuad Tabba
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Fuad Tabba @ 2024-10-14 16:58 UTC (permalink / raw)
To: kvmarm
Cc: maz, oliver.upton, catalin.marinas, joey.gouly, suzuki.poulose,
yuzenghui, will, christoffer.dall, tabba
Move KVM_VCPU_MAX_FEATURES to the header file that has the
features it is counting. Having it there reduces the chances of
missing to update it.
No functional change intended.
Signed-off-by: Fuad Tabba <tabba@google.com>
---
arch/arm64/include/asm/kvm_host.h | 3 ---
arch/arm64/include/uapi/asm/kvm.h | 3 +++
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 78c068592d73..85901afeb332 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -39,9 +39,6 @@
#define KVM_MAX_VCPUS VGIC_V3_MAX_CPUS
-#define KVM_VCPU_MAX_FEATURES 8
-#define KVM_VCPU_VALID_FEATURES (BIT(KVM_VCPU_MAX_FEATURES) - 1)
-
#define KVM_REQ_SLEEP \
KVM_ARCH_REQ_FLAGS(0, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP)
#define KVM_REQ_IRQ_PENDING KVM_ARCH_REQ(1)
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index 964df31da975..2d5fd0ed7dff 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -109,6 +109,9 @@ struct kvm_regs {
#define KVM_ARM_VCPU_PTRAUTH_GENERIC 6 /* VCPU uses generic authentication */
#define KVM_ARM_VCPU_HAS_EL2 7 /* Support nested virtualization */
+#define KVM_VCPU_MAX_FEATURES 8
+#define KVM_VCPU_VALID_FEATURES (BIT(KVM_VCPU_MAX_FEATURES) - 1)
+
struct kvm_vcpu_init {
__u32 target;
__u32 features[7];
--
2.47.0.rc1.288.g06298d1525-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v1 3/4] KVM: arm64: Convert KVM_ARM_VCPU_* features into an enum
2024-10-14 16:58 [PATCH v1 0/4] KVM: arm64: Update KVM_VCPU_MAX_FEATURES and refactor to avoid same issue Fuad Tabba
2024-10-14 16:58 ` [PATCH v1 1/4] KVM: arm64: Update the value of KVM_VCPU_MAX_FEATURES Fuad Tabba
2024-10-14 16:58 ` [PATCH v1 2/4] KVM: arm64: Move KVM_VCPU_MAX_FEATURES to the features it is counting Fuad Tabba
@ 2024-10-14 16:58 ` Fuad Tabba
2024-10-14 17:13 ` Marc Zyngier
2024-10-14 16:58 ` [PATCH v1 4/4] KVM: arm64: Convert KVM_ARCH_FLAG_* " Fuad Tabba
2024-10-14 17:17 ` [PATCH v1 0/4] KVM: arm64: Update KVM_VCPU_MAX_FEATURES and refactor to avoid same issue Marc Zyngier
4 siblings, 1 reply; 9+ messages in thread
From: Fuad Tabba @ 2024-10-14 16:58 UTC (permalink / raw)
To: kvmarm
Cc: maz, oliver.upton, catalin.marinas, joey.gouly, suzuki.poulose,
yuzenghui, will, christoffer.dall, tabba
Instead of using compile time defines, convert the KVM_ARM_VCPU_*
features into an enum. Among other things, this reduces the
chances of missing to update KVM_ARM_VCPU_MAX_FEATURES.
Also rename KVM_VCPU_MAX_FEATURES and KVM_ARM_VCPU_VALID_FEATURES
to KVM_ARM_VCPU_MAX_FEATURES and KVM_ARM_VCPU_VALID_FEATURES in
order to match the features they're counting.
No functional change intended.
Signed-off-by: Fuad Tabba <tabba@google.com>
---
arch/arm64/include/asm/kvm_host.h | 2 +-
arch/arm64/include/uapi/asm/kvm.h | 25 ++++++++++++++-----------
arch/arm64/kvm/arm.c | 10 +++++-----
3 files changed, 20 insertions(+), 17 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 85901afeb332..9b7bf4ba07a3 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -326,7 +326,7 @@ struct kvm_arch {
unsigned long flags;
/* VM-wide vCPU feature set */
- DECLARE_BITMAP(vcpu_features, KVM_VCPU_MAX_FEATURES);
+ DECLARE_BITMAP(vcpu_features, KVM_ARM_VCPU_MAX_FEATURES);
/* MPIDR to vcpu index mapping, optional */
struct kvm_mpidr_data *mpidr_data;
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index 2d5fd0ed7dff..b6ebd79b8373 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -100,17 +100,20 @@ struct kvm_regs {
#define KVM_VGIC_V3_REDIST_SIZE (2 * SZ_64K)
#define KVM_VGIC_V3_ITS_SIZE (2 * SZ_64K)
-#define KVM_ARM_VCPU_POWER_OFF 0 /* CPU is started in OFF state */
-#define KVM_ARM_VCPU_EL1_32BIT 1 /* CPU running a 32bit VM */
-#define KVM_ARM_VCPU_PSCI_0_2 2 /* CPU uses PSCI v0.2 */
-#define KVM_ARM_VCPU_PMU_V3 3 /* Support guest PMUv3 */
-#define KVM_ARM_VCPU_SVE 4 /* enable SVE for this CPU */
-#define KVM_ARM_VCPU_PTRAUTH_ADDRESS 5 /* VCPU uses address authentication */
-#define KVM_ARM_VCPU_PTRAUTH_GENERIC 6 /* VCPU uses generic authentication */
-#define KVM_ARM_VCPU_HAS_EL2 7 /* Support nested virtualization */
-
-#define KVM_VCPU_MAX_FEATURES 8
-#define KVM_VCPU_VALID_FEATURES (BIT(KVM_VCPU_MAX_FEATURES) - 1)
+enum kvm_arm_vcpu_features {
+ KVM_ARM_VCPU_POWER_OFF = 0, /* CPU is started in OFF state */
+ KVM_ARM_VCPU_EL1_32BIT, /* CPU running a 32bit VM */
+ KVM_ARM_VCPU_PSCI_0_2, /* CPU uses PSCI v0.2 */
+ KVM_ARM_VCPU_PMU_V3, /* Support guest PMUv3 */
+ KVM_ARM_VCPU_SVE, /* enable SVE for this CPU */
+ KVM_ARM_VCPU_PTRAUTH_ADDRESS, /* VCPU uses address authentication */
+ KVM_ARM_VCPU_PTRAUTH_GENERIC, /* VCPU uses generic authentication */
+ KVM_ARM_VCPU_HAS_EL2, /* Support nested virtualization */
+
+ KVM_ARM_VCPU_MAX_FEATURES, /* Must be last */
+};
+
+#define KVM_ARM_VCPU_VALID_FEATURES (BIT(KVM_ARM_VCPU_MAX_FEATURES) - 1)
struct kvm_vcpu_init {
__u32 target;
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index a0d01c46e408..df17d50887d6 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -211,7 +211,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
kvm_arm_init_hypercalls(kvm);
- bitmap_zero(kvm->arch.vcpu_features, KVM_VCPU_MAX_FEATURES);
+ bitmap_zero(kvm->arch.vcpu_features, KVM_ARM_VCPU_MAX_FEATURES);
return 0;
@@ -1407,7 +1407,7 @@ int kvm_vm_ioctl_irq_line(struct kvm *kvm, struct kvm_irq_level *irq_level,
static unsigned long system_supported_vcpu_features(void)
{
- unsigned long features = KVM_VCPU_VALID_FEATURES;
+ unsigned long features = KVM_ARM_VCPU_VALID_FEATURES;
if (!cpus_have_final_cap(ARM64_HAS_32BIT_EL1))
clear_bit(KVM_ARM_VCPU_EL1_32BIT, &features);
@@ -1435,7 +1435,7 @@ static int kvm_vcpu_init_check_features(struct kvm_vcpu *vcpu,
unsigned long features = init->features[0];
int i;
- if (features & ~KVM_VCPU_VALID_FEATURES)
+ if (features & ~KVM_ARM_VCPU_VALID_FEATURES)
return -ENOENT;
for (i = 1; i < ARRAY_SIZE(init->features); i++) {
@@ -1474,7 +1474,7 @@ static bool kvm_vcpu_init_changed(struct kvm_vcpu *vcpu,
unsigned long features = init->features[0];
return !bitmap_equal(vcpu->kvm->arch.vcpu_features, &features,
- KVM_VCPU_MAX_FEATURES);
+ KVM_ARM_VCPU_MAX_FEATURES);
}
static int kvm_setup_vcpu(struct kvm_vcpu *vcpu)
@@ -1509,7 +1509,7 @@ static int __kvm_vcpu_set_target(struct kvm_vcpu *vcpu,
kvm_vcpu_init_changed(vcpu, init))
goto out_unlock;
- bitmap_copy(kvm->arch.vcpu_features, &features, KVM_VCPU_MAX_FEATURES);
+ bitmap_copy(kvm->arch.vcpu_features, &features, KVM_ARM_VCPU_MAX_FEATURES);
ret = kvm_setup_vcpu(vcpu);
if (ret)
--
2.47.0.rc1.288.g06298d1525-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH v1 4/4] KVM: arm64: Convert KVM_ARCH_FLAG_* into an enum
2024-10-14 16:58 [PATCH v1 0/4] KVM: arm64: Update KVM_VCPU_MAX_FEATURES and refactor to avoid same issue Fuad Tabba
` (2 preceding siblings ...)
2024-10-14 16:58 ` [PATCH v1 3/4] KVM: arm64: Convert KVM_ARM_VCPU_* features into an enum Fuad Tabba
@ 2024-10-14 16:58 ` Fuad Tabba
2024-10-14 17:17 ` [PATCH v1 0/4] KVM: arm64: Update KVM_VCPU_MAX_FEATURES and refactor to avoid same issue Marc Zyngier
4 siblings, 0 replies; 9+ messages in thread
From: Fuad Tabba @ 2024-10-14 16:58 UTC (permalink / raw)
To: kvmarm
Cc: maz, oliver.upton, catalin.marinas, joey.gouly, suzuki.poulose,
yuzenghui, will, christoffer.dall, tabba
Instead of using compile time defines, convert the
KVM_ARCH_FLAG_* vm flags into an enum. Add a compile-time check
to ensure it fits in flag.
No functional change intended.
Signed-off-by: Fuad Tabba <tabba@google.com>
---
arch/arm64/include/asm/kvm_host.h | 53 +++++++++++++++++--------------
arch/arm64/kvm/arm.c | 2 ++
2 files changed, 32 insertions(+), 23 deletions(-)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 9b7bf4ba07a3..b55057c3fb2c 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -268,6 +268,35 @@ enum fgt_group_id {
__NR_FGT_GROUP_IDS__
};
+enum kvm_arch_flags {
+ /*
+ * If we encounter a data abort without valid instruction syndrome
+ * information, report this to user space. User space can (and
+ * should) opt in to this feature if KVM_CAP_ARM_NISV_TO_USER is
+ * supported.
+ */
+ KVM_ARCH_FLAG_RETURN_NISV_IO_ABORT_TO_USER = 0,
+ /* Memory Tagging Extension enabled for the guest */
+ KVM_ARCH_FLAG_MTE_ENABLED,
+ /* At least one vCPU has ran in the VM */
+ KVM_ARCH_FLAG_HAS_RAN_ONCE,
+ /* The vCPU feature set for the VM is configured */
+ KVM_ARCH_FLAG_VCPU_FEATURES_CONFIGURED,
+ /* PSCI SYSTEM_SUSPEND enabled for the guest */
+ KVM_ARCH_FLAG_SYSTEM_SUSPEND_ENABLED,
+ /* VM counter offset */
+ KVM_ARCH_FLAG_VM_COUNTER_OFFSET,
+ /* Timer PPIs made immutable */
+ KVM_ARCH_FLAG_TIMER_PPIS_IMMUTABLE,
+ /* Initial ID reg values loaded */
+ KVM_ARCH_FLAG_ID_REGS_INITIALIZED,
+ /* Fine-Grained UNDEF initialised */
+ KVM_ARCH_FLAG_FGU_INITIALIZED,
+
+ /* Must be last */
+ KVM_ARCH_FLAG_MAX
+};
+
struct kvm_arch {
struct kvm_s2_mmu mmu;
@@ -300,29 +329,7 @@ struct kvm_arch {
/* Protects VM-scoped configuration data */
struct mutex config_lock;
- /*
- * If we encounter a data abort without valid instruction syndrome
- * information, report this to user space. User space can (and
- * should) opt in to this feature if KVM_CAP_ARM_NISV_TO_USER is
- * supported.
- */
-#define KVM_ARCH_FLAG_RETURN_NISV_IO_ABORT_TO_USER 0
- /* Memory Tagging Extension enabled for the guest */
-#define KVM_ARCH_FLAG_MTE_ENABLED 1
- /* At least one vCPU has ran in the VM */
-#define KVM_ARCH_FLAG_HAS_RAN_ONCE 2
- /* The vCPU feature set for the VM is configured */
-#define KVM_ARCH_FLAG_VCPU_FEATURES_CONFIGURED 3
- /* PSCI SYSTEM_SUSPEND enabled for the guest */
-#define KVM_ARCH_FLAG_SYSTEM_SUSPEND_ENABLED 4
- /* VM counter offset */
-#define KVM_ARCH_FLAG_VM_COUNTER_OFFSET 5
- /* Timer PPIs made immutable */
-#define KVM_ARCH_FLAG_TIMER_PPIS_IMMUTABLE 6
- /* Initial ID reg values loaded */
-#define KVM_ARCH_FLAG_ID_REGS_INITIALIZED 7
- /* Fine-Grained UNDEF initialised */
-#define KVM_ARCH_FLAG_FGU_INITIALIZED 8
+ /* Bitmap of the set kvm_arch_flags */
unsigned long flags;
/* VM-wide vCPU feature set */
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index df17d50887d6..0d2bcc65cf85 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -117,6 +117,8 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
if (kvm_vm_is_protected(kvm) && !pkvm_ext_allowed(kvm, cap->cap))
return -EINVAL;
+ BUILD_BUG_ON(sizeof(kvm->arch.flags) * __CHAR_BIT__ <= KVM_ARCH_FLAG_MAX);
+
switch (cap->cap) {
case KVM_CAP_ARM_NISV_TO_USER:
r = 0;
--
2.47.0.rc1.288.g06298d1525-goog
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v1 3/4] KVM: arm64: Convert KVM_ARM_VCPU_* features into an enum
2024-10-14 16:58 ` [PATCH v1 3/4] KVM: arm64: Convert KVM_ARM_VCPU_* features into an enum Fuad Tabba
@ 2024-10-14 17:13 ` Marc Zyngier
0 siblings, 0 replies; 9+ messages in thread
From: Marc Zyngier @ 2024-10-14 17:13 UTC (permalink / raw)
To: Fuad Tabba
Cc: kvmarm, oliver.upton, catalin.marinas, joey.gouly, suzuki.poulose,
yuzenghui, will, christoffer.dall
On Mon, 14 Oct 2024 17:58:08 +0100,
Fuad Tabba <tabba@google.com> wrote:
>
> Instead of using compile time defines, convert the KVM_ARM_VCPU_*
> features into an enum. Among other things, this reduces the
> chances of missing to update KVM_ARM_VCPU_MAX_FEATURES.
>
> Also rename KVM_VCPU_MAX_FEATURES and KVM_ARM_VCPU_VALID_FEATURES
> to KVM_ARM_VCPU_MAX_FEATURES and KVM_ARM_VCPU_VALID_FEATURES in
> order to match the features they're counting.
>
> No functional change intended.
>
> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
> arch/arm64/include/asm/kvm_host.h | 2 +-
> arch/arm64/include/uapi/asm/kvm.h | 25 ++++++++++++++-----------
> arch/arm64/kvm/arm.c | 10 +++++-----
> 3 files changed, 20 insertions(+), 17 deletions(-)
>
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index 85901afeb332..9b7bf4ba07a3 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -326,7 +326,7 @@ struct kvm_arch {
> unsigned long flags;
>
> /* VM-wide vCPU feature set */
> - DECLARE_BITMAP(vcpu_features, KVM_VCPU_MAX_FEATURES);
> + DECLARE_BITMAP(vcpu_features, KVM_ARM_VCPU_MAX_FEATURES);
>
> /* MPIDR to vcpu index mapping, optional */
> struct kvm_mpidr_data *mpidr_data;
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 2d5fd0ed7dff..b6ebd79b8373 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -100,17 +100,20 @@ struct kvm_regs {
> #define KVM_VGIC_V3_REDIST_SIZE (2 * SZ_64K)
> #define KVM_VGIC_V3_ITS_SIZE (2 * SZ_64K)
>
> -#define KVM_ARM_VCPU_POWER_OFF 0 /* CPU is started in OFF state */
> -#define KVM_ARM_VCPU_EL1_32BIT 1 /* CPU running a 32bit VM */
> -#define KVM_ARM_VCPU_PSCI_0_2 2 /* CPU uses PSCI v0.2 */
> -#define KVM_ARM_VCPU_PMU_V3 3 /* Support guest PMUv3 */
> -#define KVM_ARM_VCPU_SVE 4 /* enable SVE for this CPU */
> -#define KVM_ARM_VCPU_PTRAUTH_ADDRESS 5 /* VCPU uses address authentication */
> -#define KVM_ARM_VCPU_PTRAUTH_GENERIC 6 /* VCPU uses generic authentication */
> -#define KVM_ARM_VCPU_HAS_EL2 7 /* Support nested virtualization */
> -
> -#define KVM_VCPU_MAX_FEATURES 8
> -#define KVM_VCPU_VALID_FEATURES (BIT(KVM_VCPU_MAX_FEATURES) - 1)
> +enum kvm_arm_vcpu_features {
> + KVM_ARM_VCPU_POWER_OFF = 0, /* CPU is started in OFF state */
> + KVM_ARM_VCPU_EL1_32BIT, /* CPU running a 32bit VM */
> + KVM_ARM_VCPU_PSCI_0_2, /* CPU uses PSCI v0.2 */
> + KVM_ARM_VCPU_PMU_V3, /* Support guest PMUv3 */
> + KVM_ARM_VCPU_SVE, /* enable SVE for this CPU */
> + KVM_ARM_VCPU_PTRAUTH_ADDRESS, /* VCPU uses address authentication */
> + KVM_ARM_VCPU_PTRAUTH_GENERIC, /* VCPU uses generic authentication */
> + KVM_ARM_VCPU_HAS_EL2, /* Support nested virtualization */
> +
> + KVM_ARM_VCPU_MAX_FEATURES, /* Must be last */
> +};
This is going to break any userspace that has code shaped like this:
#ifdef KVM_ARM_VCPU_SVE
[...]
#endif
Not necessarily a good pattern, but nonetheless a valid one. So you
*must* preserve the #defines, even if you define them along the lines
of:
#define KVM_ARM_VCPU_SVE KVM_ARM_VCPU_SVE
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 0/4] KVM: arm64: Update KVM_VCPU_MAX_FEATURES and refactor to avoid same issue
2024-10-14 16:58 [PATCH v1 0/4] KVM: arm64: Update KVM_VCPU_MAX_FEATURES and refactor to avoid same issue Fuad Tabba
` (3 preceding siblings ...)
2024-10-14 16:58 ` [PATCH v1 4/4] KVM: arm64: Convert KVM_ARCH_FLAG_* " Fuad Tabba
@ 2024-10-14 17:17 ` Marc Zyngier
2024-10-14 18:17 ` Fuad Tabba
4 siblings, 1 reply; 9+ messages in thread
From: Marc Zyngier @ 2024-10-14 17:17 UTC (permalink / raw)
To: Fuad Tabba
Cc: kvmarm, oliver.upton, catalin.marinas, joey.gouly, suzuki.poulose,
yuzenghui, will, christoffer.dall
On Mon, 14 Oct 2024 17:58:05 +0100,
Fuad Tabba <tabba@google.com> wrote:
>
> The value of KVM_VCPU_MAX_FEATURES has not been updated since
> adding new features in commit 89b0e7de3451 ("KVM: arm64: nv:
> Introduce nested virtualization VCPU feature").
Could you please expand on what is broken? 89b0e7de3451 makes a point
in *not* exposing this to userspace, as outlined in the commit
message -- this is done on purpose, because NV is not functional yet.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 0/4] KVM: arm64: Update KVM_VCPU_MAX_FEATURES and refactor to avoid same issue
2024-10-14 17:17 ` [PATCH v1 0/4] KVM: arm64: Update KVM_VCPU_MAX_FEATURES and refactor to avoid same issue Marc Zyngier
@ 2024-10-14 18:17 ` Fuad Tabba
2024-10-15 10:15 ` Marc Zyngier
0 siblings, 1 reply; 9+ messages in thread
From: Fuad Tabba @ 2024-10-14 18:17 UTC (permalink / raw)
To: Marc Zyngier
Cc: kvmarm, oliver.upton, catalin.marinas, joey.gouly, suzuki.poulose,
yuzenghui, will, christoffer.dall
Hi Marc,
On Mon, 14 Oct 2024 at 18:17, Marc Zyngier <maz@kernel.org> wrote:
>
> On Mon, 14 Oct 2024 17:58:05 +0100,
> Fuad Tabba <tabba@google.com> wrote:
> >
> > The value of KVM_VCPU_MAX_FEATURES has not been updated since
> > adding new features in commit 89b0e7de3451 ("KVM: arm64: nv:
> > Introduce nested virtualization VCPU feature").
>
> Could you please expand on what is broken? 89b0e7de3451 makes a point
> in *not* exposing this to userspace, as outlined in the commit
> message -- this is done on purpose, because NV is not functional yet.
I understand. I was wondering how you would test NV when this feature
isn't settable, which is why I thought it was broken. That said, maybe
a comment by the definition of KVM_VCPU_MAX_FEATURES to that effect
might make things less confusing for people like me :)
With that, is it worth repinning this patch, or is it not worth the
complexity of trying to hide NV as long as it's not supported?
Thanks for clarifying this,
/fuad
> Thanks,
>
> M.
>
> --
> Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 0/4] KVM: arm64: Update KVM_VCPU_MAX_FEATURES and refactor to avoid same issue
2024-10-14 18:17 ` Fuad Tabba
@ 2024-10-15 10:15 ` Marc Zyngier
0 siblings, 0 replies; 9+ messages in thread
From: Marc Zyngier @ 2024-10-15 10:15 UTC (permalink / raw)
To: Fuad Tabba
Cc: kvmarm, oliver.upton, catalin.marinas, joey.gouly, suzuki.poulose,
yuzenghui, will, christoffer.dall
On Mon, 14 Oct 2024 19:17:19 +0100,
Fuad Tabba <tabba@google.com> wrote:
>
> Hi Marc,
>
> On Mon, 14 Oct 2024 at 18:17, Marc Zyngier <maz@kernel.org> wrote:
> >
> > On Mon, 14 Oct 2024 17:58:05 +0100,
> > Fuad Tabba <tabba@google.com> wrote:
> > >
> > > The value of KVM_VCPU_MAX_FEATURES has not been updated since
> > > adding new features in commit 89b0e7de3451 ("KVM: arm64: nv:
> > > Introduce nested virtualization VCPU feature").
> >
> > Could you please expand on what is broken? 89b0e7de3451 makes a point
> > in *not* exposing this to userspace, as outlined in the commit
> > message -- this is done on purpose, because NV is not functional yet.
>
> I understand. I was wondering how you would test NV when this feature
Testing????? Fool!!! KVM is a *write-only* code base.
More seriously, we have zillions of out-of-tree patches that actually
enable the thing, just like for pKVM.
> isn't settable, which is why I thought it was broken. That said, maybe
> a comment by the definition of KVM_VCPU_MAX_FEATURES to that effect
> might make things less confusing for people like me :)
>
> With that, is it worth repinning this patch, or is it not worth the
> complexity of trying to hide NV as long as it's not supported?
Maybe that's the sort of kick up the bum I need to actually enable
*some* NV support upstream.
Let me see if I can extract something from the current mess.
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2024-10-15 10:15 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-14 16:58 [PATCH v1 0/4] KVM: arm64: Update KVM_VCPU_MAX_FEATURES and refactor to avoid same issue Fuad Tabba
2024-10-14 16:58 ` [PATCH v1 1/4] KVM: arm64: Update the value of KVM_VCPU_MAX_FEATURES Fuad Tabba
2024-10-14 16:58 ` [PATCH v1 2/4] KVM: arm64: Move KVM_VCPU_MAX_FEATURES to the features it is counting Fuad Tabba
2024-10-14 16:58 ` [PATCH v1 3/4] KVM: arm64: Convert KVM_ARM_VCPU_* features into an enum Fuad Tabba
2024-10-14 17:13 ` Marc Zyngier
2024-10-14 16:58 ` [PATCH v1 4/4] KVM: arm64: Convert KVM_ARCH_FLAG_* " Fuad Tabba
2024-10-14 17:17 ` [PATCH v1 0/4] KVM: arm64: Update KVM_VCPU_MAX_FEATURES and refactor to avoid same issue Marc Zyngier
2024-10-14 18:17 ` Fuad Tabba
2024-10-15 10:15 ` Marc Zyngier
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.