* [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* 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
* [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 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