KVM: arm64: Honour KVM_VM_TYPE_ARM_IPA_SIZE under pKVM

Hi folks,

pKVM has ignored KVM_VM_TYPE_ARM_IPA_SIZE since it started running guest stage 2 at EL2 (v6.2): every VM gets the host's IPA limit, whatever its VMM requested, and an out-of-range request that every other host rejects with -EINVAL is accepted. This series makes pKVM handle the type the way KVM does everywhere else, fixes the bugs that causes, and two found on the way.

Ignoring the type has two consequences I hit while testing the ITS table save series [1]: vgic_init fails on any pKVM host whose limit exceeds 40 bits, because the addresses it expects a 40-bit VM to reject are in range; and on a 64K kernel built for 48-bit PAs on a 52-bit part the host sizes a VM at 52 bits while EL2 gives it 48, so a memslot above 2^48 the host accepts can't be mapped.

Patch 3 is the fix. EL2 takes the IPA size from the VTCR the host sized the VM with, bounded by the same rule as the host's limit, and the host-side override goes: the request is range-checked and the VM gets the size it requested, so the VMM owns the guest's IPA layout on a pKVM host too, and the host and EL2 agree on it.

The rest supports that. Patch 1 moves the limit rule into a helper EL2 can share. Patch 2 rejects a pgd donation that's page-aligned but not aligned to its own size, which crashes EL2 today; patch 3 changes how that size is computed, so the check goes in first. Patch 4 frees a VM vgic_init and vgic_v5 leak when the GIC probe fails.

A VMM that requests less than the host limit under pKVM, the 40-bit default type included, now gets that size, and on a host whose limit is below 40 bits the default type is refused, as it is without pKVM. kvmtool requests the smallest size its RAM needs, for protected VMs too, and crosvm requests the limit, so neither places anything beyond its request.

vgic_init passes with this, its range checks untouched, on QEMU under pKVM and VHE at 52- and 48-bit host IPA limits. On a 64K/PA_BITS_48 pKVM host a guest mapping above 2^48 fails without patch 3 and works with it. On an M4, whose limit is 40 bits, pKVM used to create a VM for a 44-bit request and give a 36-bit VM a 40-bit stage 2; with this it refuses the first and bounds the second, as the same machine does without pKVM.

Based on Linux 7.3-rc3 (fd73f4a665989; also applies to kvmarm/next 8476ce1f1e949).

Cheers,
/fuad

[1] https://lore.kernel.org/all/[email protected]/

Fuad Tabba (3):
KVM: arm64: Move the IPA limit rule into kvm_get_ipa_max()
KVM: arm64: Honour the requested IPA size under pKVM
KVM: arm64: selftests: Free the VM when the GIC device probe fails

Quentin Perret (1):
KVM: arm64: Check PGD alignment when creating a pVM

 arch/arm64/include/asm/kvm_pgtable.h          | 15 ++++++++++++
 arch/arm64/kvm/hyp/nvhe/mem_protect.c         |  2 +-
 arch/arm64/kvm/hyp/nvhe/pkvm.c                | 24 +++++++++++++++----
 arch/arm64/kvm/mmu.c                          |  4 +---
 arch/arm64/kvm/reset.c                        | 12 +---------
 tools/testing/selftests/kvm/arm64/vgic_init.c |  4 +++-
 tools/testing/selftests/kvm/arm64/vgic_v5.c   |  4 +++-
 7 files changed, 44 insertions(+), 21 deletions(-)

base-commit: fd73f4a6659897191fa0d40695fe370925dd3780

2.39.5

Move the rule kvm_set_ipa_limit() applies to PARange into a helper beside kvm_get_parange(), which bounds the same field by a different rule, so the hypervisor can size a guest's IPA space by it.

No functional change intended.

---
 arch/arm64/include/asm/kvm_pgtable.h | 15 +++++++++++++++
 arch/arm64/kvm/reset.c               | 12 +-----------
 2 files changed, 16 insertions(+), 11 deletions(-)

index 41a8687938eb6..8096b0bf381c2 100644
@@ -48,6 +48,21 @@ static inline u64 kvm_get_parange(u64 mmfr0)
 	return parange;
 }
 
+/*
+ * The largest IPA size a VM can be given. Unlike VTCR_EL2.PS
+ * (kvm_get_parange()), it isn't clamped to the kernel's PA configuration.
+ */
+static inline u32 kvm_get_ipa_max(u64 mmfr0)
+{
+	unsigned int parange = cpuid_feature_extract_unsigned_field(mmfr0,
+				ID_AA64MMFR0_EL1_PARANGE_SHIFT);
+
+	if (!kvm_lpa2_is_enabled() && PAGE_SIZE != SZ_64K)
+		parange = min(parange, (unsigned int)ID_AA64MMFR0_EL1_PARANGE_48);
+
+	return id_aa64mmfr0_parange_to_phys_shift(parange);
+}
+
 typedef u64 kvm_pte_t;
 
 #define KVM_PTE_VALID			BIT(0)

index 10eb7249aa9e8..757adbce2faac 100644
@@ -231,19 +231,9 @@ u32 get_kvm_ipa_limit(void)
 
 int __init kvm_set_ipa_limit(void)
 {
-	unsigned int parange;
 	u64 mmfr0;
 
 	mmfr0 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1);
-	parange = cpuid_feature_extract_unsigned_field(mmfr0,
-				ID_AA64MMFR0_EL1_PARANGE_SHIFT);
-	/*
-	 * IPA size beyond 48 bits for 4K and 16K page size is only supported
-	 * when LPA2 is available. So if we have LPA2, enable it, else cap to 48
-	 * bits, in case it's reported as larger on the system.
-	 */
-	if (!kvm_lpa2_is_enabled() && PAGE_SIZE != SZ_64K)
-		parange = min(parange, (unsigned int)ID_AA64MMFR0_EL1_PARANGE_48);
 
 	/*
 	 * Check with ARMv8.5-GTG that our PAGE_SIZE is supported at

@@ -264,7 +254,7 @@ int __init kvm_set_ipa_limit(void)
 		return -EINVAL;
 	}
 
-	kvm_ipa_limit = id_aa64mmfr0_parange_to_phys_shift(parange);
+	kvm_ipa_limit = kvm_get_ipa_max(mmfr0);
 	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)" : ""));

2.39.5

From: Quentin Perret <[email protected]>

Martijn reported a hypervisor crash when providing pKVM with an undersized PGD allocation. Indeed, although the size of the PGD allocation at EL2 is not under host control, a smaller host-side allocation can lead to providing pKVM with a misaligned PGD, which will cause the guest stage-2 init to fail in a bad way. Specifically, guest_s2_zalloc_pages_exact() expects a successful allocation from hyp_alloc_pages(), which can only happen if the pool has been pre-filled with a physically aligned high-order page.

In order to guarantee allocation success in this path, check the host-provided PGD alignment early on.

[Fuad: rejected with -EINVAL before any donation is mapped, as the
other input checks in __pkvm_init_vm() do]

---
 arch/arm64/kvm/hyp/nvhe/mem_protect.c | 2 +-
 arch/arm64/kvm/hyp/nvhe/pkvm.c        | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

index 39aa8911f62c1..805dade1d4510 100644
@@ -186,7 +186,7 @@ static void *guest_s2_zalloc_pages_exact(size_t size)
 {
 	void *addr = hyp_alloc_pages(&current_vm->pool, get_order(size));
 
-	WARN_ON(size != (PAGE_SIZE << get_order(size)));
+	WARN_ON(!addr || size != (PAGE_SIZE << get_order(size)));
 	hyp_split_page(hyp_virt_to_page(addr));
 
 	return addr;

index 459bd9eb7e4bc..3770315b50361 100644
@@ -846,6 +846,10 @@ int __pkvm_init_vm(struct kvm *host_kvm, unsigned long vm_hva,
 
 	vm_size = pkvm_get_hyp_vm_size(nr_vcpus);
 	pgd_size = kvm_pgtable_stage2_pgd_size(host_mmu.arch.mmu.vtcr);
+	if (!IS_ALIGNED(pgd_hva, pgd_size)) {
+		ret = -EINVAL;
+		goto err_unpin_kvm;
+	}
 
 	ret = -ENOMEM;
 

2.39.5

pKVM gives every VM the host's IPA limit, whatever KVM_VM_TYPE_ARM_IPA_SIZE requested, so vgic_init fails on any pKVM host whose limit exceeds 40 bits: the addresses it expects a 40-bit VM to reject are in range. EL2 also sizes the guest's stage 2 from the host stage 2's VTCR, which on a 64K kernel built for 48-bit PAs on a 52-bit part is 48 bits where the limit is 52: a memslot above 2^48 the host accepts returns -ERANGE on the guest's first access.

Have EL2 take the IPA size from the VTCR the host sized the VM and the donated pgd with, bounded by kvm_get_ipa_max(), and drop the host-side override, so the request is checked and honoured as on any other host.

---
 arch/arm64/kvm/hyp/nvhe/pkvm.c | 20 ++++++++++++++++----
 arch/arm64/kvm/mmu.c           |  4 +---
 2 files changed, 17 insertions(+), 7 deletions(-)

index 3770315b50361..85fc14b7a5bc3 100644
@@ -423,7 +423,8 @@ static void unpin_host_vcpus(struct pkvm_hyp_vcpu *hyp_vcpus[],
 }
 
 static void init_pkvm_hyp_vm(struct kvm *host_kvm, struct pkvm_hyp_vm *hyp_vm,
-			     unsigned int nr_vcpus, pkvm_handle_t handle)
+			     unsigned int nr_vcpus, pkvm_handle_t handle,
+			     u64 vtcr)
 {
 	struct kvm_s2_mmu *mmu = &hyp_vm->kvm.arch.mmu;
 	int idx = vm_handle_to_idx(handle);

@@ -439,7 +440,7 @@ static void init_pkvm_hyp_vm(struct kvm *host_kvm, struct pkvm_hyp_vm *hyp_vm,
 	/* VMID 0 is reserved for the host */
 	atomic64_set(&mmu->vmid.id, idx + 1);
 
-	mmu->vtcr = host_mmu.arch.mmu.vtcr;
+	mmu->vtcr = vtcr;
 	mmu->arch = &hyp_vm->kvm.arch;
 	mmu->pgt = &hyp_vm->pgt;
 }

@@ -826,6 +827,8 @@ int __pkvm_init_vm(struct kvm *host_kvm, unsigned long vm_hva,
 	unsigned int nr_vcpus;
 	pkvm_handle_t handle;
 	void *pgd = NULL;
+	u32 phys_shift;
+	u64 vtcr;
 	int ret;
 
 	ret = hyp_pin_shared_mem(host_kvm, host_kvm + 1);

@@ -844,8 +847,17 @@ int __pkvm_init_vm(struct kvm *host_kvm, unsigned long vm_hva,
 		goto err_unpin_kvm;
 	}
 
+	phys_shift = VTCR_EL2_IPA(READ_ONCE(host_kvm->arch.mmu.vtcr));
+	if (phys_shift < ARM64_MIN_PARANGE_BITS ||
+	    phys_shift > kvm_get_ipa_max(id_aa64mmfr0_el1_sys_val)) {
+		ret = -EINVAL;
+		goto err_unpin_kvm;
+	}
+	vtcr = kvm_get_vtcr(id_aa64mmfr0_el1_sys_val, id_aa64mmfr1_el1_sys_val,
+			    phys_shift);
+
 	vm_size = pkvm_get_hyp_vm_size(nr_vcpus);
-	pgd_size = kvm_pgtable_stage2_pgd_size(host_mmu.arch.mmu.vtcr);
+	pgd_size = kvm_pgtable_stage2_pgd_size(vtcr);
 	if (!IS_ALIGNED(pgd_hva, pgd_size)) {
 		ret = -EINVAL;
 		goto err_unpin_kvm;

@@ -861,7 +873,7 @@ int __pkvm_init_vm(struct kvm *host_kvm, unsigned long vm_hva,
 	if (!pgd)
 		goto err_remove_mappings;
 
-	init_pkvm_hyp_vm(host_kvm, hyp_vm, nr_vcpus, handle);
+	init_pkvm_hyp_vm(host_kvm, hyp_vm, nr_vcpus, handle, vtcr);
 
 	ret = kvm_guest_prepare_stage2(hyp_vm, pgd);
 	if (ret)

index 9ba86450fe4af..5a3a4863f3d13 100644
@@ -909,9 +909,7 @@ static int kvm_init_ipa_range(struct kvm_s2_mmu *mmu, unsigned long type)
 	u32 phys_shift;
 
 	phys_shift = KVM_VM_TYPE_ARM_IPA_SIZE(type);
-	if (is_protected_kvm_enabled()) {
-		phys_shift = kvm_ipa_limit;
-	} else if (phys_shift) {
+	if (phys_shift) {
 		if (phys_shift > kvm_ipa_limit ||
 		    phys_shift < ARM64_MIN_PARANGE_BITS)
 			return -EINVAL;

2.39.5

test_kvm_device() creates a VM to probe the GIC version under test and returns without freeing it when the probe fails, in vgic_init and in its vgic_v5 copy. Free it on that path.

---
 tools/testing/selftests/kvm/arm64/vgic_init.c | 4 +++-
 tools/testing/selftests/kvm/arm64/vgic_v5.c   | 4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

index 47e34b43afb29..53894d32b0fc8 100644
@@ -732,8 +732,10 @@ int test_kvm_device(u32 gic_dev_type)
 
 	/* trial mode */
 	ret = __kvm_test_create_device(v.vm, gic_dev_type);
-	if (ret)
+	if (ret) {
+		kvm_vm_free(v.vm);
 		return ret;
+	}
 	v.gic_fd = kvm_create_device(v.vm, gic_dev_type);
 
 	ret = __kvm_create_device(v.vm, gic_dev_type);

index 96cfd6bb32f6f..10fdfe362428b 100644
@@ -187,8 +187,10 @@ int test_kvm_device(u32 gic_dev_type)
 
 	/* trial mode */
 	ret = __kvm_test_create_device(v.vm, gic_dev_type);
-	if (ret)
+	if (ret) {
+		kvm_vm_free(v.vm);
 		return ret;
+	}
 	v.gic_fd = kvm_create_device(v.vm, gic_dev_type);
 
 	ret = __kvm_create_device(v.vm, gic_dev_type);

2.39.5