| Message ID | 20190621093843.220980-4-marc.zyngier@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: ARMv8.3 Nested Virtualization support | expand |
On 21/06/2019 10:37, Marc Zyngier wrote: > From: Jintack Lim <jintack.lim@linaro.org> > > Add a new ARM64_HAS_NESTED_VIRT feature to indicate that the > CPU has the ARMv8.3 nested virtualization capability. > > This will be used to support nested virtualization in KVM. > > Signed-off-by: Jintack Lim <jintack.lim@linaro.org> > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > Signed-off-by: Christoffer Dall <christoffer.dall@arm.com> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > --- > .../admin-guide/kernel-parameters.txt | 4 +++ > arch/arm64/include/asm/cpucaps.h | 3 ++- > arch/arm64/include/asm/sysreg.h | 1 + > arch/arm64/kernel/cpufeature.c | 26 +++++++++++++++++++ > 4 files changed, 33 insertions(+), 1 deletion(-) > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt > index 138f6664b2e2..202bb2115d83 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -2046,6 +2046,10 @@ > [KVM,ARM] Allow use of GICv4 for direct injection of > LPIs. > > + kvm-arm.nested= > + [KVM,ARM] Allow nested virtualization in KVM/ARM. > + Default is 0 (disabled) > + Once the kernel has been built with nested guest support, what do we gain from having it disabled by default? It seems a bit odd since the guests have to opt-in for the capability of running guests of their own. Is it it likely to have negative impact a negative impact on the host kernel? Or on guests that do not request use of nested virt? If not I feel that this kernel parameter should be dropped. Cheers,
On 21/06/2019 14:08, Julien Thierry wrote: > > > On 21/06/2019 10:37, Marc Zyngier wrote: >> From: Jintack Lim <jintack.lim@linaro.org> >> >> Add a new ARM64_HAS_NESTED_VIRT feature to indicate that the >> CPU has the ARMv8.3 nested virtualization capability. >> >> This will be used to support nested virtualization in KVM. >> >> Signed-off-by: Jintack Lim <jintack.lim@linaro.org> >> Signed-off-by: Andre Przywara <andre.przywara@arm.com> >> Signed-off-by: Christoffer Dall <christoffer.dall@arm.com> >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> >> --- >> .../admin-guide/kernel-parameters.txt | 4 +++ >> arch/arm64/include/asm/cpucaps.h | 3 ++- >> arch/arm64/include/asm/sysreg.h | 1 + >> arch/arm64/kernel/cpufeature.c | 26 +++++++++++++++++++ >> 4 files changed, 33 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt >> index 138f6664b2e2..202bb2115d83 100644 >> --- a/Documentation/admin-guide/kernel-parameters.txt >> +++ b/Documentation/admin-guide/kernel-parameters.txt >> @@ -2046,6 +2046,10 @@ >> [KVM,ARM] Allow use of GICv4 for direct injection of >> LPIs. >> >> + kvm-arm.nested= >> + [KVM,ARM] Allow nested virtualization in KVM/ARM. >> + Default is 0 (disabled) >> + > > Once the kernel has been built with nested guest support, what do we > gain from having it disabled by default? We have a bunch of fast paths almost everywhere when NV isn't enabled. It makes a real difference at the moment. > It seems a bit odd since the guests have to opt-in for the capability of > running guests of their own. > > Is it it likely to have negative impact a negative impact on the host > kernel? Or on guests that do not request use of nested virt? > > If not I feel that this kernel parameter should be dropped. It really does. Speed is one, but also security is another. NV adds all kind of new paths and complexity. Having a central knob to control it and having it OFF by default helps me sleep at night... This is also what x86 had for multiple years until it was deemed safe enough to be on by default. Thanks, M.
On 06/21/2019 10:37 AM, Marc Zyngier wrote: > From: Jintack Lim <jintack.lim@linaro.org> > > Add a new ARM64_HAS_NESTED_VIRT feature to indicate that the > CPU has the ARMv8.3 nested virtualization capability. > > This will be used to support nested virtualization in KVM. > > Signed-off-by: Jintack Lim <jintack.lim@linaro.org> > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > Signed-off-by: Christoffer Dall <christoffer.dall@arm.com> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>
On Fri, Jun 21, 2019 at 10:37:47AM +0100, Marc Zyngier wrote: > From: Jintack Lim <jintack.lim@linaro.org> > > Add a new ARM64_HAS_NESTED_VIRT feature to indicate that the > CPU has the ARMv8.3 nested virtualization capability. > > This will be used to support nested virtualization in KVM. > > Signed-off-by: Jintack Lim <jintack.lim@linaro.org> > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > Signed-off-by: Christoffer Dall <christoffer.dall@arm.com> > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > --- > .../admin-guide/kernel-parameters.txt | 4 +++ > arch/arm64/include/asm/cpucaps.h | 3 ++- > arch/arm64/include/asm/sysreg.h | 1 + > arch/arm64/kernel/cpufeature.c | 26 +++++++++++++++++++ > 4 files changed, 33 insertions(+), 1 deletion(-) > > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt > index 138f6664b2e2..202bb2115d83 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -2046,6 +2046,10 @@ > [KVM,ARM] Allow use of GICv4 for direct injection of > LPIs. > > + kvm-arm.nested= > + [KVM,ARM] Allow nested virtualization in KVM/ARM. > + Default is 0 (disabled) > + In light of the discussion on this patch, is it worth making 0 not guarantee that nested is allowed, rather than guaranteeing to disable nested? This would allow the option to be turned into a no-op later once the NV code is considered mature enough to rip out all the conditionality. [...] Cheers ---Dave
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 138f6664b2e2..202bb2115d83 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -2046,6 +2046,10 @@ [KVM,ARM] Allow use of GICv4 for direct injection of LPIs. + kvm-arm.nested= + [KVM,ARM] Allow nested virtualization in KVM/ARM. + Default is 0 (disabled) + kvm-intel.ept= [KVM,Intel] Disable extended page tables (virtualized MMU) support on capable Intel chips. Default is 1 (enabled) diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h index 33401ebc187c..faa13c1f1f65 100644 --- a/arch/arm64/include/asm/cpucaps.h +++ b/arch/arm64/include/asm/cpucaps.h @@ -63,7 +63,8 @@ #define ARM64_HAS_IRQ_PRIO_MASKING 42 #define ARM64_HAS_DCPODP 43 #define ARM64_WORKAROUND_1463225 44 +#define ARM64_HAS_NESTED_VIRT 45 -#define ARM64_NCAPS 45 +#define ARM64_NCAPS 46 #endif /* __ASM_CPUCAPS_H */ diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h index 434cf53d527b..f3ca7e4796ab 100644 --- a/arch/arm64/include/asm/sysreg.h +++ b/arch/arm64/include/asm/sysreg.h @@ -693,6 +693,7 @@ /* id_aa64mmfr2 */ #define ID_AA64MMFR2_FWB_SHIFT 40 #define ID_AA64MMFR2_AT_SHIFT 32 +#define ID_AA64MMFR2_NV_SHIFT 24 #define ID_AA64MMFR2_LVA_SHIFT 16 #define ID_AA64MMFR2_IESB_SHIFT 12 #define ID_AA64MMFR2_LSM_SHIFT 8 diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c index 80babf451519..2f8e7d4e8e45 100644 --- a/arch/arm64/kernel/cpufeature.c +++ b/arch/arm64/kernel/cpufeature.c @@ -224,6 +224,7 @@ static const struct arm64_ftr_bits ftr_id_aa64mmfr1[] = { static const struct arm64_ftr_bits ftr_id_aa64mmfr2[] = { ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_FWB_SHIFT, 4, 0), ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_AT_SHIFT, 4, 0), + ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_NV_SHIFT, 4, 0), ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_LVA_SHIFT, 4, 0), ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_IESB_SHIFT, 4, 0), ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64MMFR2_LSM_SHIFT, 4, 0), @@ -1161,6 +1162,21 @@ static void cpu_copy_el2regs(const struct arm64_cpu_capabilities *__unused) if (!alternative_is_applied(ARM64_HAS_VIRT_HOST_EXTN)) write_sysreg(read_sysreg(tpidr_el1), tpidr_el2); } + +static bool nested_param; +static bool has_nested_virt_support(const struct arm64_cpu_capabilities *cap, + int scope) +{ + return has_cpuid_feature(cap, scope) && + nested_param; +} + +static int __init kvmarm_nested_cfg(char *buf) +{ + return strtobool(buf, &nested_param); +} + +early_param("kvm-arm.nested", kvmarm_nested_cfg); #endif static void cpu_has_fwb(const struct arm64_cpu_capabilities *__unused) @@ -1331,6 +1347,16 @@ static const struct arm64_cpu_capabilities arm64_features[] = { .matches = runs_at_el2, .cpu_enable = cpu_copy_el2regs, }, + { + .desc = "Nested Virtualization Support", + .capability = ARM64_HAS_NESTED_VIRT, + .type = ARM64_CPUCAP_SYSTEM_FEATURE, + .matches = has_nested_virt_support, + .sys_reg = SYS_ID_AA64MMFR2_EL1, + .sign = FTR_UNSIGNED, + .field_pos = ID_AA64MMFR2_NV_SHIFT, + .min_field_value = 1, + }, #endif /* CONFIG_ARM64_VHE */ { .desc = "32-bit EL0 Support",