Re: [PATCH v18] arm64: mm: Handle Granule Protection Faults (GPFs)

[Suzuki K Poulose <suzuki.poulose@arm.com>]

On 17/09/2026 11:36, Catalin Marinas wrote:
> Hi Suzuki,
> 
> On Thu, Sep 17, 2026 at 10:03:14AM +0100, Suzuki K Poulose wrote:
>> On 16/09/2026 17:39, Catalin Marinas wrote:
>>> On Sun, Sep 13, 2026 at 08:04:58AM +0100, Suzuki K Poulose wrote:
>>>> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
>>>> index 75c3e463df2ef..dc3a87902a60c 100644
>>>> --- a/arch/arm64/mm/fault.c
>>>> +++ b/arch/arm64/mm/fault.c
>>>> @@ -914,6 +914,24 @@ static int do_tag_check_fault(unsigned long far, unsigned long esr,
>>>>    	return 0;
>>>>    }
>>>> +static int do_gpf_ptw(unsigned long far, unsigned long esr, struct pt_regs *regs)
>>>> +{
>>>> +	const struct fault_info *inf = esr_to_fault_info(esr);
>>>> +	unsigned long addr = untagged_addr(far);
>>>> +
>>>> +	die_kernel_fault(inf->name, addr, esr, regs);
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static int do_gpf(unsigned long far, unsigned long esr, struct pt_regs *regs)
>>>> +{
>>>> +	if (!user_mode(regs) && !is_el1_instruction_abort(esr) &&
>>>> +	    fixup_exception(regs, esr))
>>>> +		return 0;
>>>> +
>>>> +	return 1;
>>>> +}
>>>
>>> We discussed briefly offline. With the latest patches around, would we
>>> ever end up with private memory mapped in the VMM and hence the GPF? If
>>> not, I would still keep this handling but add a
>>> WARN_ON_ONCE(user_mode(regs)).
>>>
>>> However, can we end up delegating a non-guest_memfd memslot page as
>>> protected?
>>>
>>> I played a bit with codex and it reckons it's possible if a guest_memfd
>>> memslot is deleted after its IPA range has been initialised with
>>> RIPAS=RAM. Removing the memslot unmaps and undelegates any data pages
>>> but leaves the RMM state as RAM. The VMM can then install an ordinary
>>> memslot over the same GPA range.
>>
>> This should be prevented by the following predicates:
>>
>> 1) Realms only support guest_memfd backed memslots for mappable memory.
>> 2) Memslots cannot be created after the Realm is created, as is with the
>> protected VMs. (This check seems to have been lost over the iterations,
>>   but should be reinstated).
> 
> If that's the intended model, I think it should work. But v18 doesn't
> enforce either of them. I noticed the second predicate for pKVM only -
> your 'Widen the scope of "protected" VMs' patch makes this restriction
> explicit to pKVM.
> 
> For the first one, if !kvm_slot_has_gmem(), it simply continues with the
> registration.
> 
>>> A subsequent private-IPA S2 fault sees the non-guest_memfd slot, takes
>>> user_mem_abort(), GUPs the user page and passes it to
>>> realm_map_protected(). The userspace mapping remains present, so a later
>>> EL0 access can generate a GPF.
>>
>> The Realm mem abort code should prevent this by ensuring that the
>> memslot is backed by gmem for private_faults. With the mandate of
>> in-place conversion, even the shared pages must come from the
>> gmem backed memslots.
> 
> IIUC this only works if the memslot is gmem but I can't see what
> prevents ordinary slots from being assigned to realms. I think we can
> enter the user_mem_abort() -> realm_map_ipa() for ordinary slots unless
> we prevent the deletion of the original slots and enforce gmem only
> slots early.


I had another look and we could handle this via 
kvm_fault_is_gmem_abort() see in arch/arm64/kvm/mmu.c:


diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 87e49251e0447..af5a4bf961aae 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -1731,6 +1731,9 @@ static int gmem_abort(const struct 
kvm_s2_fault_desc *s2fd)
         gfn_t gfn;
         int ret;

+       if (!kvm_slot_has_gmem(s2fd->memslot))
+               return -EINVAL;
+
         if (!perm_fault) {
                 memcache = get_mmu_memcache(vcpu);
                 ret = topup_mmu_memcache(vcpu, memcache);
@@ -2277,10 +2280,12 @@ static bool private_ipa_fault(struct kvm *kvm, 
phys_addr_t fault_ipa);
  static bool kvm_fault_is_gmem_abort(struct kvm *kvm,
                                     const struct kvm_s2_fault_desc *s2fd)
  {
-       if (!kvm_slot_has_gmem(s2fd->memslot))
-               return false;
         if (kvm_memslot_is_gmem_only(s2fd->memslot))
                 return true;
+       /*
+        * For Realms, all private faults must be backed by GMEM.
+        * TODO: Handle Trusted device private memory mappings.
+        */
         if (private_ipa_fault(kvm, s2fd->fault_ipa))
                 return true;
         return false;




Also, I have the following hunk for preventing memslot modifications.
I will add this to v20 integration branch, which is almost ready ;-)

diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 582b48e34486b..87e49251e0447 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -2783,6 +2783,18 @@ void kvm_arch_commit_memory_region(struct kvm *kvm,
         }
  }

+static bool kvm_prevents_memslot_change(struct kvm *kvm, enum 
kvm_mr_change change)
+{
+       /* Cannot modify memslots once a pVM has run or Realm created */
+       if (change != KVM_MR_DELETE && change != KVM_MR_MOVE)
+               return false;
+
+       if ((kvm_vm_is_protected_pkvm(kvm) && 
pkvm_hyp_vm_is_created(kvm)) ||
+            kvm_realm_is_created(kvm))
+               return true;
+       return false;
+}
+
  int kvm_arch_prepare_memory_region(struct kvm *kvm,
                                    const struct kvm_memory_slot *old,
                                    struct kvm_memory_slot *new,
@@ -2791,12 +2803,9 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,
         hva_t hva, reg_end;
         int ret = 0;

-       if (kvm_vm_is_protected_pkvm(kvm)) {
-               /* Cannot modify memslots once a pVM has run. */
-               if (pkvm_hyp_vm_is_created(kvm) &&
-                   (change == KVM_MR_DELETE || change == KVM_MR_MOVE)) {
+       if (kvm_vm_is_protected(kvm)) {
+               if (kvm_prevents_memslot_change(kvm, change))
                         return -EPERM;
-               }

                 if (new &&
                     new->flags & (KVM_MEM_LOG_DIRTY_PAGES | 
KVM_MEM_READONLY)) {
diff --git a/arch/arm64/kvm/rmi.c b/arch/arm64/kvm/rmi.c
index fc0297103f08b..6ec4e4487dff9 100644
--- a/arch/arm64/kvm/rmi.c
+++ b/arch/arm64/kvm/rmi.c
@@ -1596,6 +1596,7 @@ int kvm_activate_realm(struct kvm *kvm)
         if (kvm_realm_state(kvm) >= REALM_STATE_ACTIVE)
                 return 0;

+       guard(mutex)(&kvm->slots_lock);
         guard(mutex)(&kvm->arch.config_lock);
         /* Check again with the lock held */
         if (kvm_realm_state(kvm) >= REALM_STATE_ACTIVE)


Cheers
Suzuki