Re: [PATCH 03/59] arm64: Add ARM64_HAS_NESTED_VIRT cpufeature

2019-06-24 Thread Dave Martin
On Fri, Jun 21, 2019 at 10:37:47AM +0100, Marc Zyngier wrote:
> From: Jintack Lim 
> 
> 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 
> Signed-off-by: Andre Przywara 
> Signed-off-by: Christoffer Dall 
> Signed-off-by: Marc Zyngier 
> ---
>  .../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
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH 03/59] arm64: Add ARM64_HAS_NESTED_VIRT cpufeature

2019-06-21 Thread Marc Zyngier
On 21/06/2019 14:08, Julien Thierry wrote:
> 
> 
> On 21/06/2019 10:37, Marc Zyngier wrote:
>> From: Jintack Lim 
>>
>> 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 
>> Signed-off-by: Andre Przywara 
>> Signed-off-by: Christoffer Dall 
>> Signed-off-by: Marc Zyngier 
>> ---
>>  .../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.
-- 
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


Re: [PATCH 03/59] arm64: Add ARM64_HAS_NESTED_VIRT cpufeature

2019-06-21 Thread Julien Thierry



On 21/06/2019 10:37, Marc Zyngier wrote:
> From: Jintack Lim 
> 
> 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 
> Signed-off-by: Andre Przywara 
> Signed-off-by: Christoffer Dall 
> Signed-off-by: Marc Zyngier 
> ---
>  .../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,

-- 
Julien Thierry
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm


[PATCH 03/59] arm64: Add ARM64_HAS_NESTED_VIRT cpufeature

2019-06-21 Thread Marc Zyngier
From: Jintack Lim 

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 
Signed-off-by: Andre Przywara 
Signed-off-by: Christoffer Dall 
Signed-off-by: Marc Zyngier 
---
 .../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)
+
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_NCAPS45
+#define ARM64_NCAPS46
 
 #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_SHIFT12
 #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, _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",
-- 
2.20.1

___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm