Re: [PATCH 1/2] KVM: arm64: nv: Allocate the shadow S2 MMUs individually

On Tue, 04 Aug 2026 22:54:30 +0100,
Karl Mehltretter <[email protected]> wrote:
> 
> On Tue, Aug 04, 2026 at 03:56:47PM +0100, Marc Zyngier wrote:
> > > The other thing is that reallocating the pointer array isn't great. It
> > > adds complexity, and makes everything more fragile than it should be.
> > > 
> > > See the hack below that seems to work OK.
> > > 
> 
> Your draft is indeed a bit simpler.
> 
> Should I send a cleaned-up version as v2?

Yes, please.

> 
> I would allocate the table on the first call to kvm_vcpu_init_nested()
> and not in kvm_init_nested(), so it cannot leak if VM creation
> fails.

I'd rather keep it on the VM creation path. This is VM-wide data, by
definition, and given that its size doesn't depend on the number of
vcpus anymore (we allocate the maximum once and for all), it is right
where it belongs. See the untested hack below for the freeing on error.

> There's another issue in the current code: the ptdump debugfs file
> keeps the raw mmu pointer as its private data. I would add that to the
> commit message.

Why is that a problem? With this approach, the mmu pointers are always
expected to be valid, irrespective of the allocation pattern, and we
only publish pointers to the dumper when the S2_mmu is actively being
used.

We're about to rip out the NV ptdump anyway as it has many other
problems, but I'd like to understand what you see that I don't.

Thanks,

	M.

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 9e42e92dbc7b1..e883e45382fb2 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -223,10 +223,6 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 	mutex_unlock(&kvm->lock);
 #endif
 
-	ret = kvm_init_nested(kvm);
-	if (ret)
-		return ret;
-
 	ret = kvm_share_hyp(kvm, kvm + 1);
 	if (ret)
 		return ret;
@@ -241,6 +237,10 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 	if (ret)
 		goto err_free_cpumask;
 
+	ret = kvm_init_nested(kvm);
+	if (ret)
+		goto err_uninit_mmu;
+
 	if (is_protected_kvm_enabled()) {
 		/*
 		 * If any failures occur after this is successful, make sure to
@@ -269,6 +269,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 
 err_uninit_mmu:
 	kvm_uninit_stage2_mmu(kvm);
+	kvfree(kvm->arch.nested_mmus);
 err_free_cpumask:
 	free_cpumask_var(kvm->arch.supported_cpus);
 err_unshare_kvm:

-- 
Without deviation from the norm, progress is not possible.