KVM: arm64: Allow KVM to be disabled from the command line
Subsystems (4)
Marc Zyngier
maintainerPATCH 1/1 KVM: arm64: Allow KVM to be disabled from the command line
Although KVM can be compiled out of the kernel, it cannot be disabled at runtime. Allow this possibility by introducing a new mode that will prevent KVM from initialising.
This is useful in the (limited) circumstances where you don't want KVM to be available (what is wrong with you?), or when you want to install another hypervisor instead (good luck with that).
--- Documentation/admin-guide/kernel-parameters.txt | 3 +++ arch/arm64/include/asm/kvm_host.h | 1 + arch/arm64/kernel/idreg-override.c | 1 + arch/arm64/kvm/arm.c | 14 +++++++++++++- 4 files changed, 18 insertions(+), 1 deletion(-)
Documentation/admin-guide/kernel-parameters.txt +3 -0
@@ -2365,6 +2365,9 @@ kvm-arm.mode= [KVM,ARM] Select one of KVM/arm64's modes of operation. + none: Forcefully disable KVM and run in nVHE mode, + preventing KVM from ever initialising. + nvhe: Standard nVHE-based mode, without support for protected guests.
arch/arm64/include/asm/kvm_host.h +1 -0
@@ -58,6 +58,7 @@enum kvm_mode {KVM_MODE_DEFAULT ,KVM_MODE_PROTECTED , +KVM_MODE_NONE , };enum kvm_mode kvm_get_mode (void );
arch/arm64/kernel/idreg-override.c +1 -0
@@ -95,6 +95,7 @@ static const struct {char alias[FTR_ALIAS_NAME_LEN ];char feature[FTR_ALIAS_OPTION_LEN ]; } aliases[] __initconst = { + { "kvm-arm.mode=none", "id_aa64mmfr1.vh=0" }, { "kvm-arm.mode=nvhe", "id_aa64mmfr1.vh=0" }, { "kvm-arm.mode=protected", "id_aa64mmfr1.vh=0" }, { "arm64.nobti", "id_aa64pfr1.bt=0" },
arch/arm64/kvm/arm.c +13 -1
@@ -2064,6 +2064,11 @@ int kvm_arch_init(void *opaque)return -ENODEV ; } +if (kvm_get_mode () ==KVM_MODE_NONE ) { +kvm_info ("KVM disabled from command line\n" ); +return -ENODEV ; + } + in_hyp_mode =is_kernel_in_hyp_mode (); if (cpus_have_final_cap(ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE) ||
@@ -2137,8 +2142,15 @@ static int __init early_kvm_mode_cfg(char *arg)return 0 ; } -if (strcmp (arg,"nvhe" ) ==0 && !WARN_ON (is_kernel_in_hyp_mode ())) +if (strcmp (arg,"nvhe" ) ==0 && !WARN_ON (is_kernel_in_hyp_mode ())) { + kvm_mode =KVM_MODE_DEFAULT ;return 0 ; + } + +if (strcmp (arg,"none" ) ==0 && !WARN_ON (is_kernel_in_hyp_mode ())) { + kvm_mode =KVM_MODE_NONE ; +return 0 ; + }return -EINVAL ; }
Signature · 6 lines
2.30.2 _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Hey Marc,
that all makes sense:
arch/arm64/kvm/arm.c static int __init early_kvm_mode_cfg(char *arg) · line 2150
Quoted 11 lines
arch/arm64/kvm/arm.c +5
-1@@ -2137,8 +2142,15 @@ static int __init early_kvm_mode_cfg(char *arg)return 0 ; } -if (strcmp (arg,"nvhe" ) ==0 && !WARN_ON (is_kernel_in_hyp_mode ())) +if (strcmp (arg,"nvhe" ) ==0 && !WARN_ON (is_kernel_in_hyp_mode ())) { + kvm_mode =KVM_MODE_DEFAULT ;return 0 ; + } + +if (strcmp (arg,"none" ) ==0 && !WARN_ON (is_kernel_in_hyp_mode ())) {
nit: I noticed we check is_kernel_in_hyp_mode here for nvhe/none but for protected it is checked in is_kvm_protected_mode. May be worth unifying?
Marc Zyngier
maintainerHi David,
Quoted 10 lines · David Brazdil
Hey Marc,
that all makes sense:Reviewed-by David Brazdil <[email protected]>Quoted 4 lines
arch/arm64/kvm/arm.c +0
-0@@ -2137,8 +2142,15 @@ static int __init early_kvm_mode_cfg(char *arg)return 0 ; }
Quoted 9 lines
arch/arm64/kvm/arm.c static int __init early_kvm_mode_cfg(char *arg) · line 2150
Quoted 7 lines
arch/arm64/kvm/arm.c +5
-1-if (strcmp (arg,"nvhe" ) ==0 && !WARN_ON (is_kernel_in_hyp_mode ())) +if (strcmp (arg,"nvhe" ) ==0 && !WARN_ON (is_kernel_in_hyp_mode ())) { + kvm_mode =KVM_MODE_DEFAULT ;return 0 ; + } + +if (strcmp (arg,"none" ) ==0 && !WARN_ON (is_kernel_in_hyp_mode ())) {nit: I noticed we check
is_kernel_in_hyp_modehere for nvhe/none but for protected it is checked inis_kvm_protected_mode. May be worth unifying?
is_kvm_protected_mode() drives a capability (as we rely on the associated static key), and we don't need this with either 'nvhe' nor 'none'.
So I'm unsure what we can unify, to be honest. Can you suggest a patch?
Thanks,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Quoted 8 lines · Marc Zyngier
Although KVM can be compiled out of the kernel, it cannot be disabled at runtime. Allow this possibility by introducing a new mode that will prevent KVM from initialising.
This is useful in the (limited) circumstances where you don't want KVM to be available (what is wrong with you?), or when you want to install another hypervisor instead (good luck with that).
Quoted 7 lines
Signed-off-by Marc Zyngier <[email protected]>--- Documentation/admin-guide/kernel-parameters.txt | 3 +++ arch/arm64/include/asm/kvm_host.h | 1 + arch/arm64/kernel/idreg-override.c | 1 + arch/arm64/kvm/arm.c | 14 +++++++++++++- 4 files changed, 18 insertions(+), 1 deletion(-)
Will
Quoted 71 lines · Marc Zyngier
Although KVM can be compiled out of the kernel, it cannot be disabled at runtime. Allow this possibility by introducing a new mode that will prevent KVM from initialising.
This is useful in the (limited) circumstances where you don't want KVM to be available (what is wrong with you?), or when you want to install another hypervisor instead (good luck with that).
Signed-off-by Marc Zyngier <[email protected]>--- Documentation/admin-guide/kernel-parameters.txt | 3 +++ arch/arm64/include/asm/kvm_host.h | 1 + arch/arm64/kernel/idreg-override.c | 1 + arch/arm64/kvm/arm.c | 14 +++++++++++++- 4 files changed, 18 insertions(+), 1 deletion(-)Documentation/admin-guide/kernel-parameters.txt +3
-0@@ -2365,6 +2365,9 @@ kvm-arm.mode= [KVM,ARM] Select one of KVM/arm64's modes of operation. + none: Forcefully disable KVM and run in nVHE mode, + preventing KVM from ever initialising. + nvhe: Standard nVHE-based mode, without support for protected guests.arch/arm64/include/asm/kvm_host.h +1
-0@@ -58,6 +58,7 @@enum kvm_mode {KVM_MODE_DEFAULT ,KVM_MODE_PROTECTED , +KVM_MODE_NONE , };enum kvm_mode kvm_get_mode (void );arch/arm64/kernel/idreg-override.c +1
-0@@ -95,6 +95,7 @@ static const struct {char alias[FTR_ALIAS_NAME_LEN ];char feature[FTR_ALIAS_OPTION_LEN ]; } aliases[] __initconst = { + { "kvm-arm.mode=none", "id_aa64mmfr1.vh=0" }, { "kvm-arm.mode=nvhe", "id_aa64mmfr1.vh=0" }, { "kvm-arm.mode=protected", "id_aa64mmfr1.vh=0" }, { "arm64.nobti", "id_aa64pfr1.bt=0" },arch/arm64/kvm/arm.c +5
-0@@ -2064,6 +2064,11 @@ int kvm_arch_init(void *opaque)return -ENODEV ; } +if (kvm_get_mode () ==KVM_MODE_NONE ) { +kvm_info ("KVM disabled from command line\n" ); +return -ENODEV ; + } + in_hyp_mode =is_kernel_in_hyp_mode (); if (cpus_have_final_cap(ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE) ||
arch/arm64/kvm/arm.c static int __init early_kvm_mode_cfg(char *arg) · line 2150
Quoted 10 lines
arch/arm64/kvm/arm.c +5
-1@@ -2137,8 +2142,15 @@ static int __init early_kvm_mode_cfg(char *arg)return 0 ; } -if (strcmp (arg,"nvhe" ) ==0 && !WARN_ON (is_kernel_in_hyp_mode ())) +if (strcmp (arg,"nvhe" ) ==0 && !WARN_ON (is_kernel_in_hyp_mode ())) { + kvm_mode =KVM_MODE_DEFAULT ;return 0 ; + } + +if (strcmp (arg,"none" ) ==0 && !WARN_ON (is_kernel_in_hyp_mode ())) {
nit: Does this really need to WARN here ? Unlike the "nvhe" case, if the user wants to keep the KVM out of the picture for, say debugging something, it is perfectly Ok to allow the kernel to be running at EL2 without having to change the Firmware to alter the landing EL for the kernel ?
Otherwise,
Marc Zyngier
maintainerQuoted 80 lines · Suzuki K Poulose
Quoted 78 lines · Marc Zyngier
Although KVM can be compiled out of the kernel, it cannot be disabled at runtime. Allow this possibility by introducing a new mode that will prevent KVM from initialising.
This is useful in the (limited) circumstances where you don't want KVM to be available (what is wrong with you?), or when you want to install another hypervisor instead (good luck with that).
Signed-off-by Marc Zyngier <[email protected]>--- Documentation/admin-guide/kernel-parameters.txt | 3 +++ arch/arm64/include/asm/kvm_host.h | 1 + arch/arm64/kernel/idreg-override.c | 1 + arch/arm64/kvm/arm.c | 14 +++++++++++++- 4 files changed, 18 insertions(+), 1 deletion(-)Documentation/admin-guide/kernel-parameters.txt +2
-0@@ -2365,6 +2365,9 @@ kvm-arm.mode= [KVM,ARM] Select one of KVM/arm64's modes of operation. + none: Forcefully disable KVM and run in nVHE mode, + preventing KVM from ever initialising. + nvhe: Standard nVHE-based mode, without support for protected guests. diff --git a/arch/arm64/include/asm/kvm_host.h
b/arch/arm64/include/asm/kvm_host.h
indexf8be56d5342b..019490c67976100644arch/arm64/include/asm/kvm_host.h +1
-0@@ -58,6 +58,7 @@enum kvm_mode {KVM_MODE_DEFAULT ,KVM_MODE_PROTECTED , +KVM_MODE_NONE , };enum kvm_mode kvm_get_mode (void ); diff --git a/arch/arm64/kernel/idreg-override.c
b/arch/arm64/kernel/idreg-override.c
indexd8e606fe3c21..57013c1b6552100644arch/arm64/kernel/idreg-override.c +1
-0@@ -95,6 +95,7 @@ static const struct {char alias[FTR_ALIAS_NAME_LEN ];char feature[FTR_ALIAS_OPTION_LEN ]; } aliases[] __initconst = { + { "kvm-arm.mode=none", "id_aa64mmfr1.vh=0" }, { "kvm-arm.mode=nvhe", "id_aa64mmfr1.vh=0" }, { "kvm-arm.mode=protected", "id_aa64mmfr1.vh=0" }, { "arm64.nobti", "id_aa64pfr1.bt=0" },arch/arm64/kvm/arm.c +4
-0@@ -2064,6 +2064,11 @@ int kvm_arch_init(void *opaque)return -ENODEV ; } + if (kvm_get_mode() == KVM_MODE_NONE) { +kvm_info ("KVM disabled from command line\n" ); +return -ENODEV ; + } + in_hyp_mode =is_kernel_in_hyp_mode (); if (cpus_have_final_cap(ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE)||
arch/arm64/kvm/arm.c +3
-0@@ -2137,8 +2142,15 @@ static int __init early_kvm_mode_cfg(char *arg)return 0 ; } - if (strcmp(arg, "nvhe") == 0 && !WARN_ON(is_kernel_in_hyp_mode())) +if (strcmp (arg,"nvhe" ) ==0 && !WARN_ON (is_kernel_in_hyp_mode ())) { + kvm_mode =KVM_MODE_DEFAULT ;return 0 ; + }
Quoted 8 lines
arch/arm64/kvm/arm.c static int __init early_kvm_mode_cfg(char *arg) · line 2150
Quoted 2 lines
arch/arm64/kvm/arm.c +2
-0+ +if (strcmp (arg,"none" ) ==0 && !WARN_ON (is_kernel_in_hyp_mode ())) {nit: Does this really need to WARN here ? Unlike the "nvhe" case, if the user wants to keep the KVM out of the picture for, say debugging something, it is perfectly Ok to allow the kernel to be running at EL2 without having to change the Firmware to alter the landing EL for the kernel ?
Well, the doc says "run in nVHE mode" and the option forces id_aa64mmfr1.vh=0. The WARN_ON() will only fires on broken^Wfruity HW that is VHE only. Note that this doesn't rely on any firmware change (we drop from EL2 to EL1 and stay there).
We could add another option (none-vhe?) that stays at EL2 and still disables KVM if there is an appetite for it.
Quoted 4 lines
Otherwise,
Acked-by Suzuki K Poulose <[email protected]>
Thanks,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
Suzuki K Poulose
reviewerQuoted 91 lines · Marc Zyngier
Quoted 80 lines · Suzuki K Poulose
Quoted 78 lines · Marc Zyngier
Although KVM can be compiled out of the kernel, it cannot be disabled at runtime. Allow this possibility by introducing a new mode that will prevent KVM from initialising.
This is useful in the (limited) circumstances where you don't want KVM to be available (what is wrong with you?), or when you want to install another hypervisor instead (good luck with that).
Signed-off-by Marc Zyngier <[email protected]>--- Documentation/admin-guide/kernel-parameters.txt | 3 +++ arch/arm64/include/asm/kvm_host.h | 1 + arch/arm64/kernel/idreg-override.c | 1 + arch/arm64/kvm/arm.c | 14 +++++++++++++- 4 files changed, 18 insertions(+), 1 deletion(-)Documentation/admin-guide/kernel-parameters.txt +2
-0@@ -2365,6 +2365,9 @@ kvm-arm.mode= [KVM,ARM] Select one of KVM/arm64's modes of operation. + none: Forcefully disable KVM and run in nVHE mode, + preventing KVM from ever initialising. + nvhe: Standard nVHE-based mode, without support for protected guests. diff --git a/arch/arm64/include/asm/kvm_host.h
b/arch/arm64/include/asm/kvm_host.h
indexf8be56d5342b..019490c67976100644arch/arm64/include/asm/kvm_host.h +1
-0@@ -58,6 +58,7 @@enum kvm_mode {KVM_MODE_DEFAULT ,KVM_MODE_PROTECTED , +KVM_MODE_NONE , };enum kvm_mode kvm_get_mode (void ); diff --git a/arch/arm64/kernel/idreg-override.c
b/arch/arm64/kernel/idreg-override.c
indexd8e606fe3c21..57013c1b6552100644arch/arm64/kernel/idreg-override.c +1
-0@@ -95,6 +95,7 @@ static const struct {char alias[FTR_ALIAS_NAME_LEN ];char feature[FTR_ALIAS_OPTION_LEN ]; } aliases[] __initconst = { + { "kvm-arm.mode=none", "id_aa64mmfr1.vh=0" }, { "kvm-arm.mode=nvhe", "id_aa64mmfr1.vh=0" }, { "kvm-arm.mode=protected", "id_aa64mmfr1.vh=0" }, { "arm64.nobti", "id_aa64pfr1.bt=0" },arch/arm64/kvm/arm.c +4
-0@@ -2064,6 +2064,11 @@ int kvm_arch_init(void *opaque)return -ENODEV ; } + if (kvm_get_mode() == KVM_MODE_NONE) { +kvm_info ("KVM disabled from command line\n" ); +return -ENODEV ; + } + in_hyp_mode =is_kernel_in_hyp_mode (); if (cpus_have_final_cap(ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE)||
arch/arm64/kvm/arm.c +3
-0@@ -2137,8 +2142,15 @@ static int __init early_kvm_mode_cfg(char *arg)return 0 ; } - if (strcmp(arg, "nvhe") == 0 && !WARN_ON(is_kernel_in_hyp_mode())) +if (strcmp (arg,"nvhe" ) ==0 && !WARN_ON (is_kernel_in_hyp_mode ())) { + kvm_mode =KVM_MODE_DEFAULT ;return 0 ; + }Quoted 8 lines
arch/arm64/kvm/arm.c static int __init early_kvm_mode_cfg(char *arg) · line 2150
Quoted 2 lines
arch/arm64/kvm/arm.c +2
-0+ +if (strcmp (arg,"none" ) ==0 && !WARN_ON (is_kernel_in_hyp_mode ())) {nit: Does this really need to WARN here ? Unlike the "nvhe" case, if the user wants to keep the KVM out of the picture for, say debugging something, it is perfectly Ok to allow the kernel to be running at EL2 without having to change the Firmware to alter the landing EL for the kernel ?
Quoted 4 lines
Well, the doc says "run in nVHE mode" and the option forces id_aa64mmfr1.vh=0. The
WARN_ON()will only fires on broken^Wfruity HW that is VHE only. Note that this doesn't rely on any firmware change (we drop from EL2 to EL1 and stay there).
Ah, ok. So the "none" is in fact "nvhe + no-kvm". Thats the bit I missed. TBH, that name to me sounds like "no KVM" at all, which is what we want. The question is, do we really need "none" to force vh == 0 ? I understand this is only a problem on a rare set of HWs. But the generic option looks deceiving.
That said, I am happy to leave this as is and the doc says so.
Quoted 3 lines
We could add another option (none-vhe?) that stays at EL2 and still disables KVM if there is an appetite for it.
Na. Don't think that is necessary.
Quoted 4 lines
Quoted 4 lines
Otherwise,
Acked-by Suzuki K Poulose <[email protected]>
Suzuki
Marc Zyngier
maintainerQuoted 18 lines · Suzuki K Poulose
Quoted 11 lines · Marc Zyngier
Quoted 8 lines · Suzuki K Poulose
arch/arm64/kvm/arm.c static int __init early_kvm_mode_cfg(char *arg) · line 2150
Quoted 2 lines
arch/arm64/kvm/arm.c +1
-0+if (strcmp (arg,"none" ) ==0 && !WARN_ON (is_kernel_in_hyp_mode ())) {nit: Does this really need to WARN here ? Unlike the "nvhe" case, if the user wants to keep the KVM out of the picture for, say debugging something, it is perfectly Ok to allow the kernel to be running at EL2 without having to change the Firmware to alter the landing EL for the kernel ?
Quoted 4 lines
Well, the doc says "run in nVHE mode" and the option forces id_aa64mmfr1.vh=0. The
WARN_ON()will only fires on broken^Wfruity HW that is VHE only. Note that this doesn't rely on any firmware change (we drop from EL2 to EL1 and stay there).
Quoted 7 lines
Ah, ok. So the "none" is in fact "nvhe + no-kvm". Thats the bit I missed. TBH, that name to me sounds like "no KVM" at all, which is what we want. The question is, do we really need "none" to force vh == 0 ? I understand this is only a problem on a rare set of HWs. But the generic option looks deceiving.
That said, I am happy to leave this as is and the doc says so.
I think you have a point here. Conflating the two things is a bit odd, and we might as well let the user pick the configuration they want (they can always pass 'id_aa64mmfr1.vh=0' themselves).
I'll respin the patch with this change.
Thanks,
M.
Signature · 1 line
Without deviation from the norm, progress is not possible.