| Message ID | 20211129200150.351436-8-maz@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: ARMv8.3/8.4 Nested Virtualization support | expand |
Hi Marc, On 30-11-2021 01:30 am, Marc Zyngier wrote: > From: Christoffer Dall <christoffer.dall@arm.com> > > Introduce the feature bit and a primitive that checks if the feature is > set behind a static key check based on the cpus_have_const_cap check. > > Checking nested_virt_in_use() on systems without nested virt enabled > should have neglgible overhead. Typo: negligible > > We don't yet allow userspace to actually set this feature. > > Signed-off-by: Christoffer Dall <christoffer.dall@arm.com> > Signed-off-by: Marc Zyngier <maz@kernel.org> > --- > arch/arm64/include/asm/kvm_nested.h | 14 ++++++++++++++ > arch/arm64/include/uapi/asm/kvm.h | 1 + > 2 files changed, 15 insertions(+) > create mode 100644 arch/arm64/include/asm/kvm_nested.h > > diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h > new file mode 100644 > index 000000000000..1028ac65a897 > --- /dev/null > +++ b/arch/arm64/include/asm/kvm_nested.h > @@ -0,0 +1,14 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef __ARM64_KVM_NESTED_H > +#define __ARM64_KVM_NESTED_H > + > +#include <linux/kvm_host.h> > + > +static inline bool nested_virt_in_use(const struct kvm_vcpu *vcpu) > +{ > + return (!__is_defined(__KVM_NVHE_HYPERVISOR__) && > + cpus_have_final_cap(ARM64_HAS_NESTED_VIRT) && > + test_bit(KVM_ARM_VCPU_HAS_EL2, vcpu->arch.features)); > +} > + > +#endif /* __ARM64_KVM_NESTED_H */ > diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h > index b3edde68bc3e..395a4c039bcc 100644 > --- a/arch/arm64/include/uapi/asm/kvm.h > +++ b/arch/arm64/include/uapi/asm/kvm.h > @@ -106,6 +106,7 @@ struct kvm_regs { > #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 */ > > struct kvm_vcpu_init { > __u32 target; It looks good to me. Please feel free to add. Reviewed-by: Ganapatrao Kulkarni <gankulkarni@os.amperecomputing.com> Thanks, Ganapat
Hi Marc, On Mon, Nov 29, 2021 at 08:00:48PM +0000, Marc Zyngier wrote: > From: Christoffer Dall <christoffer.dall@arm.com> > > Introduce the feature bit and a primitive that checks if the feature is > set behind a static key check based on the cpus_have_const_cap check. > > Checking nested_virt_in_use() on systems without nested virt enabled > should have neglgible overhead. > > We don't yet allow userspace to actually set this feature. > > Signed-off-by: Christoffer Dall <christoffer.dall@arm.com> > Signed-off-by: Marc Zyngier <maz@kernel.org> > --- > arch/arm64/include/asm/kvm_nested.h | 14 ++++++++++++++ > arch/arm64/include/uapi/asm/kvm.h | 1 + > 2 files changed, 15 insertions(+) > create mode 100644 arch/arm64/include/asm/kvm_nested.h > > diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h > new file mode 100644 > index 000000000000..1028ac65a897 > --- /dev/null > +++ b/arch/arm64/include/asm/kvm_nested.h > @@ -0,0 +1,14 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +#ifndef __ARM64_KVM_NESTED_H > +#define __ARM64_KVM_NESTED_H > + > +#include <linux/kvm_host.h> > + > +static inline bool nested_virt_in_use(const struct kvm_vcpu *vcpu) > +{ > + return (!__is_defined(__KVM_NVHE_HYPERVISOR__) && > + cpus_have_final_cap(ARM64_HAS_NESTED_VIRT) && > + test_bit(KVM_ARM_VCPU_HAS_EL2, vcpu->arch.features)); kvm_vcpu_init_nested() checks the ARM64_HAS_NESTED_VIRT cap before setting the features bit, so I guess you can drop this check here if you're interested in correctness. But the reason the cap check done is performance, right? Same with the NVHE define. Thanks, Alex > +} > + > +#endif /* __ARM64_KVM_NESTED_H */ > diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h > index b3edde68bc3e..395a4c039bcc 100644 > --- a/arch/arm64/include/uapi/asm/kvm.h > +++ b/arch/arm64/include/uapi/asm/kvm.h > @@ -106,6 +106,7 @@ struct kvm_regs { > #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 */ > > struct kvm_vcpu_init { > __u32 target; > -- > 2.30.2 >
On Thu, 13 Jan 2022 14:10:39 +0000, Alexandru Elisei <alexandru.elisei@arm.com> wrote: > > Hi Marc, > > On Mon, Nov 29, 2021 at 08:00:48PM +0000, Marc Zyngier wrote: > > From: Christoffer Dall <christoffer.dall@arm.com> > > > > Introduce the feature bit and a primitive that checks if the feature is > > set behind a static key check based on the cpus_have_const_cap check. > > > > Checking nested_virt_in_use() on systems without nested virt enabled > > should have neglgible overhead. > > > > We don't yet allow userspace to actually set this feature. > > > > Signed-off-by: Christoffer Dall <christoffer.dall@arm.com> > > Signed-off-by: Marc Zyngier <maz@kernel.org> > > --- > > arch/arm64/include/asm/kvm_nested.h | 14 ++++++++++++++ > > arch/arm64/include/uapi/asm/kvm.h | 1 + > > 2 files changed, 15 insertions(+) > > create mode 100644 arch/arm64/include/asm/kvm_nested.h > > > > diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h > > new file mode 100644 > > index 000000000000..1028ac65a897 > > --- /dev/null > > +++ b/arch/arm64/include/asm/kvm_nested.h > > @@ -0,0 +1,14 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +#ifndef __ARM64_KVM_NESTED_H > > +#define __ARM64_KVM_NESTED_H > > + > > +#include <linux/kvm_host.h> > > + > > +static inline bool nested_virt_in_use(const struct kvm_vcpu *vcpu) > > +{ > > + return (!__is_defined(__KVM_NVHE_HYPERVISOR__) && > > + cpus_have_final_cap(ARM64_HAS_NESTED_VIRT) && > > + test_bit(KVM_ARM_VCPU_HAS_EL2, vcpu->arch.features)); > > kvm_vcpu_init_nested() checks the ARM64_HAS_NESTED_VIRT cap before setting > the features bit, so I guess you can drop this check here if you're > interested in correctness. > > But the reason the cap check done is performance, right? Same with the NVHE > define. Exactly. The capability check allows us to use a static key to bypass the test_bit() when NV isn't enable, which is 100% of our use cases. Given that this checked fairly often on some of the fast paths, it is worth doing it. The NVHE check goes even further by allowing some dead code removal in the nVHE-specific code, which cannot use NV by construction. Thanks, M.
On Mon, Nov 29, 2021 at 08:00:48PM +0000, Marc Zyngier wrote: > From: Christoffer Dall <christoffer.dall@arm.com> > > Introduce the feature bit and a primitive that checks if the feature is > set behind a static key check based on the cpus_have_const_cap check. > > Checking nested_virt_in_use() on systems without nested virt enabled > should have neglgible overhead. > > We don't yet allow userspace to actually set this feature. > > Signed-off-by: Christoffer Dall <christoffer.dall@arm.com> > Signed-off-by: Marc Zyngier <maz@kernel.org> Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
diff --git a/arch/arm64/include/asm/kvm_nested.h b/arch/arm64/include/asm/kvm_nested.h new file mode 100644 index 000000000000..1028ac65a897 --- /dev/null +++ b/arch/arm64/include/asm/kvm_nested.h @@ -0,0 +1,14 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef __ARM64_KVM_NESTED_H +#define __ARM64_KVM_NESTED_H + +#include <linux/kvm_host.h> + +static inline bool nested_virt_in_use(const struct kvm_vcpu *vcpu) +{ + return (!__is_defined(__KVM_NVHE_HYPERVISOR__) && + cpus_have_final_cap(ARM64_HAS_NESTED_VIRT) && + test_bit(KVM_ARM_VCPU_HAS_EL2, vcpu->arch.features)); +} + +#endif /* __ARM64_KVM_NESTED_H */ diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h index b3edde68bc3e..395a4c039bcc 100644 --- a/arch/arm64/include/uapi/asm/kvm.h +++ b/arch/arm64/include/uapi/asm/kvm.h @@ -106,6 +106,7 @@ struct kvm_regs { #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 */ struct kvm_vcpu_init { __u32 target;