KVM: arm64: Assorted IPA size fixes
Subsystems (5)
Marc Zyngier
maintainerv3 cover KVM: arm64: Assorted IPA size fixes
This is a rework of an initial patch posted a couple of days back[1]
While working on enabling KVM on "reduced IPA size" systems, I realise we have a couple of issues, some of while do impact userspace.
The first issue is that we accept the creation of a "default IPA size" VM (40 bits) even when the HW doesn't support it. Not good.
The second one is that we disallow a memslot to end right where the IPA limit is. One page less and you're good, but that's not quite what it should be.
I intend for both patches to be backported to -stable.
Thanks,
M.
* From v2 [2]:
- Fix silly printk blunder
- Added Cc-stable and Fixes tags
* From v1 [1]:
- Don't try to cap the default IPA size. If userspace uses 0 with an expectation that it will get 40bits, we should abide by it and return an error immediately (noticed by Andrew)
- Added a new patch to fix the exclusive nature of the IPA limit
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
Marc Zyngier (2):
KVM: arm64: Reject VM creation when the default IPA size is
unsupported
KVM: arm64: Fix exclusive limit for IPA size
Documentation/virt/kvm/api.rst | 3 +++ arch/arm64/kvm/mmu.c | 3 +-- arch/arm64/kvm/reset.c | 12 ++++++++---- 3 files changed, 12 insertions(+), 6 deletions(-)
Signature · 6 lines
2.29.2 _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Marc Zyngier
maintainerPATCH v3 1/2 KVM: arm64: Reject VM creation when the default IPA size is unsupported
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.
--- Documentation/virt/kvm/api.rst | 3 +++ arch/arm64/kvm/reset.c | 12 ++++++++---- 2 files changed, 11 insertions(+), 4 deletions(-)
Documentation/virt/kvm/api.rst +3 -0
@@ -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
arch/arm64/kvm/reset.c +8 -4
@@ -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); +return -EINVAL ; + } } mmfr0 =read_sanitised_ftr_reg (SYS_ID_AA64MMFR0_EL1 );
Signature · 6 lines
2.29.2 _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v3 1/2] KVM: arm64: Reject VM creation when the default IPA size is unsupported
Quoted 9 lines · Marc Zyngier
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).
Quoted 4 lines
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
Quoted 41 lines
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 <[email protected]>--- Documentation/virt/kvm/api.rst | 3 +++ arch/arm64/kvm/reset.c | 12 ++++++++---- 2 files changed, 11 insertions(+), 4 deletions(-)Documentation/virt/kvm/api.rst +3
-0@@ -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 toarch/arm64/kvm/reset.c +0
-0
arch/arm64/kvm/reset.c int kvm_set_ipa_limit(void) · line 329
Quoted 10 lines
arch/arm64/kvm/reset.c +3
-4@@ -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 ()
Quoted 18 lines
arch/arm64/kvm/reset.c +5
-0return 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 );Signature · 1 line
2.29.2
Thanks,
drew
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v3 1/2] KVM: arm64: Reject VM creation when the default IPA size is unsupported
In reply to Marc ZyngierHi Marc,
Quoted 23 lines · Marc Zyngier
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.
Quoted 7 lines
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 <[email protected]>
Thanks
Eric
Quoted 40 lines
--- Documentation/virt/kvm/api.rst | 3 +++ arch/arm64/kvm/reset.c | 12 ++++++++---- 2 files changed, 11 insertions(+), 4 deletions(-)Documentation/virt/kvm/api.rst +3
-0@@ -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 toarch/arm64/kvm/reset.c +3
-4@@ -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 ;
arch/arm64/kvm/reset.c int kvm_arm_setup_stage2(struct kvm *kvm, unsigned long type) · line 365
Quoted 10 lines
arch/arm64/kvm/reset.c +5
-0}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 );
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Marc Zyngier
maintainerPATCH v3 2/2 KVM: arm64: Fix exclusive limit for IPA size
In reply to Marc ZyngierWhen registering a memslot, we check the size and location of that memslot against the IPA size to ensure that we can provide guest access to the whole of the memory.
Unfortunately, this check rejects memslot that end-up at the exact limit of the addressing capability for a given IPA size. For example, it refuses the creation of a 2GB memslot at 0x8000000 with a 32bit IPA space.
Fix it by relaxing the check to accept a memslot reaching the limit of the IPA space.
--- arch/arm64/kvm/mmu.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
arch/arm64/kvm/mmu.c +1 -2
@@ -1312,8 +1312,7 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,* Prevent userspace from creating a memory region outside of the IPA * space addressable by the KVM guest IPA space. */ -if (memslot->base_gfn + memslot->npages >= - (kvm_phys_size (kvm) >>PAGE_SHIFT )) +if ((memslot->base_gfn + memslot->npages) > (kvm_phys_size (kvm) >>PAGE_SHIFT ))return -EFAULT ;mmap_read_lock (current->mm);
Signature · 6 lines
2.29.2 _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v3 2/2] KVM: arm64: Fix exclusive limit for IPA size
Quoted 9 lines · Marc Zyngier
When registering a memslot, we check the size and location of that memslot against the IPA size to ensure that we can provide guest access to the whole of the memory.
Unfortunately, this check rejects memslot that end-up at the exact limit of the addressing capability for a given IPA size. For example, it refuses the creation of a 2GB memslot at
0x8000000with a 32bit IPA space.
Quoted 4 lines
Fix it by relaxing the check to accept a memslot reaching the limit of the IPA space.
Fixes e55cac5bf2a9 ("kvm: arm/arm64: Prepare for VM specific stage2 translations")
Isn't this actually fixing commit c3058d5da222 ("arm/arm64: KVM: Ensure memslots are within KVM_PHYS_SIZE") ?
Quoted 24 lines
Reviewed-by Eric Auger <[email protected]>Signed-off-by Marc Zyngier <[email protected]>--- arch/arm64/kvm/mmu.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)arch/arm64/kvm/mmu.c +1
-2@@ -1312,8 +1312,7 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,* Prevent userspace from creating a memory region outside of the IPA * space addressable by the KVM guest IPA space. */ -if (memslot->base_gfn + memslot->npages >= - (kvm_phys_size (kvm) >>PAGE_SHIFT )) +if ((memslot->base_gfn + memslot->npages) > (kvm_phys_size (kvm) >>PAGE_SHIFT ))return -EFAULT ;mmap_read_lock (current->mm);Signature · 1 line
2.29.2
Otherwise
Thanks,
drew
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Marc Zyngier
maintainerRe: [PATCH v3 2/2] KVM: arm64: Fix exclusive limit for IPA size
Quoted 10 lines · Andrew Jones
Quoted 9 lines · Marc Zyngier
When registering a memslot, we check the size and location of that memslot against the IPA size to ensure that we can provide guest access to the whole of the memory.
Unfortunately, this check rejects memslot that end-up at the exact limit of the addressing capability for a given IPA size. For example, it refuses the creation of a 2GB memslot at
0x8000000with a 32bit IPA space.
Quoted 7 lines
Quoted 4 lines
Fix it by relaxing the check to accept a memslot reaching the limit of the IPA space.
Fixes e55cac5bf2a9 ("kvm: arm/arm64: Prepare for VM specific stage2 translations")Isn't this actually fixing commit
c3058d5da222("arm/arm64: KVM: Ensure memslots are withinKVM_PHYS_SIZE") ?
Ah, yes, that's indeed better (more backport work... ;-)
Thanks,
M.
Signature · 5 lines
Jazz is not dead. It just smells funny... _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Quoted 9 lines · Marc Zyngier
This is a rework of an initial patch posted a couple of days back[1]
While working on enabling KVM on "reduced IPA size" systems, I realise we have a couple of issues, some of while do impact userspace.
The first issue is that we accept the creation of a "default IPA size" VM (40 bits) even when the HW doesn't support it. Not good.
[...]
Applied to fixes, thanks!
[1/2] KVM: arm64: Reject VM creation when the default IPA size is unsupported
commit: 7d717558dd5ef10d28866750d5c24ff892ea3778
[2/2] KVM: arm64: Fix exclusive limit for IPA size
commit: 262b003d059c6671601a19057e9fe1a5e7f23722Cheers,
M.
Signature · 7 lines
Without deviation from the norm, progress is not possible. _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm