[PATCH] KVM: arm64: Allow KVM to be disabled from the command line

[Suzuki K Poulose]

On 30/09/2021 11:29, Marc Zyngier wrote:
> On Wed, 29 Sep 2021 11:35:46 +0100,
> Suzuki K Poulose <suzuki.poulose at arm.com> wrote:
>>
>> On 03/09/2021 10:16, Marc Zyngier wrote:
>>> 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 <maz at kernel.org>
>>> ---
>>>    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(-)
>>>
>>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>>> index 91ba391f9b32..cc5f68846434 100644
>>> --- a/Documentation/admin-guide/kernel-parameters.txt
>>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>>> @@ -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
>>> index f8be56d5342b..019490c67976 100644
>>> --- a/arch/arm64/include/asm/kvm_host.h
>>> +++ b/arch/arm64/include/asm/kvm_host.h
>>> @@ -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
>>> index d8e606fe3c21..57013c1b6552 100644
>>> --- a/arch/arm64/kernel/idreg-override.c
>>> +++ b/arch/arm64/kernel/idreg-override.c
>>> @@ -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" },
>>> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
>>> index fe102cd2e518..cdc70e238316 100644
>>> --- a/arch/arm64/kvm/arm.c
>>> +++ b/arch/arm64/kvm/arm.c
>>> @@ -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())) {
>>
>> 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).

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.

> 
> 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.

> 
>> Otherwise,
>>
>> Acked-by: Suzuki K Poulose <suzuki.poulose at arm.com>

Suzuki