On Thu, Mar 11, 2021 at 10:00:15AM +0000, 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.
provided
>
> 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.
>
> Fixes: 233a7cb23531 ("kvm: arm64: Allow tuning the physical address size for VM")
> Signed-off-by: Marc Zyngier <maz@xxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> ---
> 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..9d3d09a89894 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)" : ""));
nit: there's a couple pair of unnecessary ()
>
> 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);
> + return -EINVAL;
> + }
> }
>
> mmfr0 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1);
> --
> 2.29.2
>
Reviewed-by: Andrew Jones <drjones@xxxxxxxxxx>
Thanks,
drew
