[PATCH v2 1/2] KVM: arm64: Reject VM creation when the default IPA size is unsupported

[Marc Zyngier]

Hi Eric,

On 2021-03-10 10:59, Auger Eric wrote:
> Hi Marc,
> 
> On 3/10/21 11:42 AM, Marc Zyngier wrote:
>> KVM/arm64 has forever used a 40bit default IPA space, partially
>> due to its 32bit heritage (where the only choice is 40bit).
>> 
>> However, there are implementations in the wild that have a *cough*
>> much smaller *cough* IPA space, which leads to a misprogramming of
>> VTCR_EL2, and a guest that is stuck on its first memory access
>> if userspace dares to ask for the default IPA setting (which most
>> VMMs do).
>> 
>> Instead, blundly reject the creation of such VM, as we can't
>> satisfy the requirements from userspace (with a one-off warning).
>> Also clarify the boot warning, and document that the VM creation
>> will fail when an unsupported IPA size is probided.
>> 
>> Although this is an ABI change, it doesn't really change much
>> for userspace:
>> 
>> - the guest couldn't run before this change, but no error was
>>   returned. At least userspace knows what is happening.
>> 
>> - a memory slot that was accepted because it did fit the default
>>   IPA space now doesn't even get a chance to be registered.
>> 
>> The other thing that is left doing is to convince userspace to
>> actually use the IPA space setting instead of relying on the
>> antiquated default.
>> 
>> Signed-off-by: Marc Zyngier <maz at kernel.org>
>> ---
>>  Documentation/virt/kvm/api.rst |  3 +++
>>  arch/arm64/kvm/reset.c         | 12 ++++++++----
>>  2 files changed, 11 insertions(+), 4 deletions(-)
>> 
>> diff --git a/Documentation/virt/kvm/api.rst 
>> b/Documentation/virt/kvm/api.rst
>> index 1a2b5210cdbf..38e327d4b479 100644
>> --- a/Documentation/virt/kvm/api.rst
>> +++ b/Documentation/virt/kvm/api.rst
>> @@ -182,6 +182,9 @@ is dependent on the CPU capability and the kernel 
>> configuration. The limit can
>>  be retrieved using KVM_CAP_ARM_VM_IPA_SIZE of the KVM_CHECK_EXTENSION
>>  ioctl() at run-time.
>> 
>> +Creation of the VM will fail if the requested IPA size (whether it is
>> +implicit or explicit) is unsupported on the host.
>> +
>>  Please note that configuring the IPA size does not affect the 
>> capability
>>  exposed by the guest CPUs in ID_AA64MMFR0_EL1[PARange]. It only 
>> affects
>>  size of the address translated by the stage2 level (guest physical to
>> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
>> index 47f3f035f3ea..f1a38405934e 100644
>> --- a/arch/arm64/kvm/reset.c
>> +++ b/arch/arm64/kvm/reset.c
>> @@ -324,10 +324,9 @@ int kvm_set_ipa_limit(void)
>>  	}
>> 
>>  	kvm_ipa_limit = id_aa64mmfr0_parange_to_phys_shift(parange);
>> -	WARN(kvm_ipa_limit < KVM_PHYS_SHIFT,
>> -	     "KVM IPA Size Limit (%d bits) is smaller than default size\n",
>> -	     kvm_ipa_limit);
>> -	kvm_info("IPA Size Limit: %d bits\n", kvm_ipa_limit);
>> +	kvm_info("IPA Size Limit: %d bits%s\n", kvm_ipa_limit,
>> +		 ((kvm_ipa_limit < KVM_PHYS_SHIFT) ?
>> +		  " (Reduced IPA size, limited VM/VMM compatibility)" : ""));
>> 
>>  	return 0;
>>  }
>> @@ -356,6 +355,11 @@ int kvm_arm_setup_stage2(struct kvm *kvm, 
>> unsigned long type)
>>  			return -EINVAL;
>>  	} else {
>>  		phys_shift = KVM_PHYS_SHIFT;
>> +		if (phys_shift > kvm_ipa_limit) {
>> +			pr_warn_once("%s using unsupported default IPA limit, upgrade your 
>> VMM\n",
>> +				     current->comm, kvm_ipa_limit);
> don't you have a trouble with the args here?
> 
> Otherwise looks sensible to me.

This is what happens when you do a last minute change and push it out
without thinking.

My apologies, I'll fix that.

         M.
-- 
Jazz is not dead. It just smells funny...