[PATCH v3 2/3] KVM: arm64: top up pKVM mapping cache for permission faults

[PATCH v3 2/3] KVM: arm64: top up pKVM mapping cache for permission faults

[Bradley Morgan]

Permission faults normally only relax an existing leaf, so the fault path
does not top up the memcache.

With pKVM, a permission fault can also replace page mappings with a
PMD mapping. That path needs a fresh pkvm_mapping object, and can
dereference a NULL cache->mapping if the cache was not topped up.

Allocate just that object for pKVM permission faults.

The issue was discovered [1] by Sashiko.

Link: https://lore.kernel.org/all/20260623161545.EA08E1F000E9@smtp.kernel.org/ [1]

Fixes: db14091d8f75 ("KVM: arm64: Stage-2 huge mappings for np-guests")
Cc: stable@vger.kernel.org
Signed-off-by: Bradley Morgan <include@grrlz.net>
---
 arch/arm64/kvm/mmu.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 6c941aaa10c6..3f57f6825a33 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -1177,17 +1177,26 @@ void free_hyp_memcache(struct kvm_hyp_memcache *mc)
 	__free_hyp_memcache(mc, hyp_mc_free_fn, kvm_host_va, mc);
 }
 
+static int topup_hyp_memcache_mapping(struct kvm_hyp_memcache *mc)
+{
+	if (mc->mapping)
+		return 0;
+
+	mc->mapping = kzalloc_obj(struct pkvm_mapping,
+				  GFP_KERNEL_ACCOUNT);
+	return mc->mapping ? 0 : -ENOMEM;
+}
+
 int topup_hyp_memcache(struct kvm_hyp_memcache *mc, unsigned long min_pages)
 {
+	int ret;
+
 	if (!is_protected_kvm_enabled())
 		return 0;
 
-	if (!mc->mapping) {
-		mc->mapping = kzalloc_obj(struct pkvm_mapping,
-					  GFP_KERNEL_ACCOUNT);
-		if (!mc->mapping)
-			return -ENOMEM;
-	}
+	ret = topup_hyp_memcache_mapping(mc);
+	if (ret)
+		return ret;
 
 	return __topup_hyp_memcache(mc, min_pages, hyp_mc_alloc_fn,
 				    kvm_host_pa, mc);
@@ -2113,7 +2122,9 @@ static int user_mem_abort(const struct kvm_s2_fault_desc *s2fd)
 	 * Permission faults just need to update the existing leaf entry,
 	 * and so normally don't require allocations from the memcache. The
 	 * only exception to this is when dirty logging is enabled at runtime
-	 * and a write fault needs to collapse a block entry into a table.
+	 * and a write fault needs to collapse a block entry into a table. With
+	 * pKVM, they may still need a fresh mapping object if the fault turns
+	 * page entries into a block entry.
 	 */
 	memcache = get_mmu_memcache(s2fd->vcpu);
 	if (!perm_fault || (memslot_is_logging(s2fd->memslot) &&
@@ -2121,6 +2132,10 @@ static int user_mem_abort(const struct kvm_s2_fault_desc *s2fd)
 		ret = topup_mmu_memcache(s2fd->vcpu, memcache);
 		if (ret)
 			return ret;
+	} else if (is_protected_kvm_enabled()) {
+		ret = topup_hyp_memcache_mapping(memcache);
+		if (ret)
+			return ret;
 	}
 
 	/*
-- 
2.53.0

[PATCH v3 3/3] KVM: arm64: top up stage 2 memcache for dirty logging faults

[Bradley Morgan]

Dirty logging forces new stage 2 mappings down to page size, but
it does not always remove an existing block mapping before the next
fault. Eager splitting is best effort and is disabled by default.

A permission fault on such a block can still need a page table page
to install the smaller mapping. Top up the memcache for any permission
fault while dirty logging is active, not only for write faults.

The issue was discovered [1] by Sashiko.

Link: https://lore.kernel.org/all/59984F6D-06F2-4302-BDD7-92DF334E8FA0@grrlz.net/T/#t [1]

Fixes: 6f745f1bb5bf ("KVM: arm64: Convert user_mem_abort() to generic page-table API")
Cc: stable@vger.kernel.org
Signed-off-by: Bradley Morgan <include@grrlz.net>
---
 arch/arm64/kvm/mmu.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
index 3f57f6825a33..8911e319e6fa 100644
--- a/arch/arm64/kvm/mmu.c
+++ b/arch/arm64/kvm/mmu.c
@@ -2122,13 +2122,12 @@ static int user_mem_abort(const struct kvm_s2_fault_desc *s2fd)
 	 * Permission faults just need to update the existing leaf entry,
 	 * and so normally don't require allocations from the memcache. The
 	 * only exception to this is when dirty logging is enabled at runtime
-	 * and a write fault needs to collapse a block entry into a table. With
-	 * pKVM, they may still need a fresh mapping object if the fault turns
-	 * page entries into a block entry.
+	 * and a fault needs to collapse a block entry into a table. With pKVM,
+	 * they may still need a fresh mapping object if the fault turns page
+	 * entries into a block entry.
 	 */
 	memcache = get_mmu_memcache(s2fd->vcpu);
-	if (!perm_fault || (memslot_is_logging(s2fd->memslot) &&
-			    kvm_is_write_fault(s2fd->vcpu))) {
+	if (!perm_fault || memslot_is_logging(s2fd->memslot)) {
 		ret = topup_mmu_memcache(s2fd->vcpu, memcache);
 		if (ret)
 			return ret;
-- 
2.53.0

Re: [PATCH v3 3/3] KVM: arm64: top up stage 2 memcache for dirty logging faults

[Bradley Morgan]

On June 24, 2026 5:00:28 PM GMT+01:00, Bradley Morgan <include@grrlz.net>
wrote:
>Dirty logging forces new stage 2 mappings down to page size, but
>it does not always remove an existing block mapping before the next
>fault. Eager splitting is best effort and is disabled by default.
>
>A permission fault on such a block can still need a page table page
>to install the smaller mapping. Top up the memcache for any permission
>fault while dirty logging is active, not only for write faults.
>
>The issue was discovered [1] by Sashiko.
>
>Link: https://lore.kernel.org/all/59984F6D-06F2-4302-BDD7-92DF334E8FA0@grrlz.net/T/#t [1]
>
>Fixes: 6f745f1bb5bf ("KVM: arm64: Convert user_mem_abort() to generic page-table API")
>Cc: stable@vger.kernel.org
>Signed-off-by: Bradley Morgan <include@grrlz.net>
>---
> arch/arm64/kvm/mmu.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
>diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
>index 3f57f6825a33..8911e319e6fa 100644
>--- a/arch/arm64/kvm/mmu.c
>+++ b/arch/arm64/kvm/mmu.c
>@@ -2122,13 +2122,12 @@ static int user_mem_abort(const struct kvm_s2_fault_desc *s2fd)
> 	 * Permission faults just need to update the existing leaf entry,
> 	 * and so normally don't require allocations from the memcache. The
> 	 * only exception to this is when dirty logging is enabled at runtime
>-	 * and a write fault needs to collapse a block entry into a table. With
>-	 * pKVM, they may still need a fresh mapping object if the fault turns
>-	 * page entries into a block entry.
>+	 * and a fault needs to collapse a block entry into a table. With pKVM,
>+	 * they may still need a fresh mapping object if the fault turns page
>+	 * entries into a block entry.
> 	 */
> 	memcache = get_mmu_memcache(s2fd->vcpu);
>-	if (!perm_fault || (memslot_is_logging(s2fd->memslot) &&
>-			    kvm_is_write_fault(s2fd->vcpu))) {
>+	if (!perm_fault || memslot_is_logging(s2fd->memslot)) {
> 		ret = topup_mmu_memcache(s2fd->vcpu, memcache);
> 		if (ret)
> 			return ret;
>

Note: Patch 3 seems to conflict because of patch 2 (the comments)


Oops! :(

V4 (after people have their review go), will contain one commit (patch
3) with the updated comments.

Patch 1 and 2 applies as usual.

Apologies for my messup. 

Thanks!

Re: [PATCH v3 3/3] KVM: arm64: top up stage 2 memcache for dirty logging faults

[Bradley Morgan]

On June 24, 2026 6:39:16 PM GMT+01:00, Bradley Morgan <include@grrlz.net>
wrote:
>On June 24, 2026 5:00:28 PM GMT+01:00, Bradley Morgan <include@grrlz.net>
>wrote:
>>Dirty logging forces new stage 2 mappings down to page size, but
>>it does not always remove an existing block mapping before the next
>>fault. Eager splitting is best effort and is disabled by default.
>>
>>A permission fault on such a block can still need a page table page
>>to install the smaller mapping. Top up the memcache for any permission
>>fault while dirty logging is active, not only for write faults.
>>
>>The issue was discovered [1] by Sashiko.
>>
>>Link:
>https://lore.kernel.org/all/59984F6D-06F2-4302-BDD7-92DF334E8FA0@grrlz.net/T/#t
>[1]
>>
>>Fixes: 6f745f1bb5bf ("KVM: arm64: Convert user_mem_abort() to generic
>page-table API")
>>Cc: stable@vger.kernel.org
>>Signed-off-by: Bradley Morgan <include@grrlz.net>
>>---
>> arch/arm64/kvm/mmu.c | 9 ++++-----
>> 1 file changed, 4 insertions(+), 5 deletions(-)
>>
>>diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
>>index 3f57f6825a33..8911e319e6fa 100644
>>--- a/arch/arm64/kvm/mmu.c
>>+++ b/arch/arm64/kvm/mmu.c
>>@@ -2122,13 +2122,12 @@ static int user_mem_abort(const struct
>kvm_s2_fault_desc *s2fd)
>> 	 * Permission faults just need to update the existing leaf entry,
>> 	 * and so normally don't require allocations from the memcache. The
>> 	 * only exception to this is when dirty logging is enabled at runtime
>>-	 * and a write fault needs to collapse a block entry into a table. With
>>-	 * pKVM, they may still need a fresh mapping object if the fault turns
>>-	 * page entries into a block entry.
>>+	 * and a fault needs to collapse a block entry into a table. With pKVM,
>>+	 * they may still need a fresh mapping object if the fault turns page
>>+	 * entries into a block entry.
>> 	 */
>> 	memcache = get_mmu_memcache(s2fd->vcpu);
>>-	if (!perm_fault || (memslot_is_logging(s2fd->memslot) &&
>>-			    kvm_is_write_fault(s2fd->vcpu))) {
>>+	if (!perm_fault || memslot_is_logging(s2fd->memslot)) {
>> 		ret = topup_mmu_memcache(s2fd->vcpu, memcache);
>> 		if (ret)
>> 			return ret;
>>
>
>Note: Patch 3 seems to conflict because of patch 2 (the comments)
>
>
>Oops! :(
>
>V4 (after people have their review go), will contain one commit (patch
>3) with the updated comments.
>
>Patch 1 and 2 applies as usual.
>
>Apologies for my messup. 
>
>Thanks!


Actually. Hmm.

I'll just drop patches 2 and 3, I'll do them at a later date, please
disregard patches 2 and 3, patch 1 doesn't rely on 2 and 3..

If you guys wanna have a look feel free! :)

Thanks!

Re: [PATCH v3 3/3] KVM: arm64: top up stage 2 memcache for dirty logging faults

[Marc Zyngier]

On Wed, 24 Jun 2026 18:46:10 +0100,
Bradley Morgan <include@grrlz.net> wrote:
> 
> On June 24, 2026 6:39:16 PM GMT+01:00, Bradley Morgan <include@grrlz.net>
> wrote:
> >
> >Note: Patch 3 seems to conflict because of patch 2 (the comments)
> >
> >
> >Oops! :(
> >
> >V4 (after people have their review go), will contain one commit (patch
> >3) with the updated comments.
> >
> >Patch 1 and 2 applies as usual.
> >
> >Apologies for my messup. 
> >
> >Thanks!
> 
> 
> Actually. Hmm.
> 
> I'll just drop patches 2 and 3, I'll do them at a later date, please
> disregard patches 2 and 3, patch 1 doesn't rely on 2 and 3..
> 
> If you guys wanna have a look feel free! :)

As I suggested in my reply to your hasty v2, taking a few *days*
between versions is generally a good thing. it gives the reviewers
time to chime in, and gives you the opportunity to reflect on what
you've just written (reading your own patches after a few days is a
sure way to go and rewrite them).

Actually, by posting more often, you are guaranteeing that people
*avoid* reviewing your stuff, since odds are that there is a new
version coming in the next 10 minutes, so why bother...

But hey, that's free advice, so it's probably worthless.

	M.

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

Re: [PATCH v3 3/3] KVM: arm64: top up stage 2 memcache for dirty logging faults

[Bradley Morgan]

On June 24, 2026 7:25:04 PM GMT+01:00, Marc Zyngier <maz@kernel.org> wrote:
>On Wed, 24 Jun 2026 18:46:10 +0100,
>Bradley Morgan <include@grrlz.net> wrote:
>> 
>> On June 24, 2026 6:39:16 PM GMT+01:00, Bradley Morgan
><include@grrlz.net>
>> wrote:
>> >
>> >Note: Patch 3 seems to conflict because of patch 2 (the comments)
>> >
>> >
>> >Oops! :(
>> >
>> >V4 (after people have their review go), will contain one commit (patch
>> >3) with the updated comments.
>> >
>> >Patch 1 and 2 applies as usual.
>> >
>> >Apologies for my messup. 
>> >
>> >Thanks!
>> 
>> 
>> Actually. Hmm.
>> 
>> I'll just drop patches 2 and 3, I'll do them at a later date, please
>> disregard patches 2 and 3, patch 1 doesn't rely on 2 and 3..
>> 
>> If you guys wanna have a look feel free! :)
>
>As I suggested in my reply to your hasty v2, taking a few *days*
>between versions is generally a good thing. it gives the reviewers
>time to chime in, and gives you the opportunity to reflect on what
>you've just written (reading your own patches after a few days is a
>sure way to go and rewrite them).

welp, I guess I learned the hard way with the hastiness of my V3, (that's
kinda why I dropped p2 and p3)


>Actually, by posting more often, you are guaranteeing that people
>*avoid* reviewing your stuff, since odds are that there is a new
>version coming in the next 10 minutes, so why bother...

I'm not planning on posting another version, until fuller review concludes


>But hey, that's free advice, so it's probably worthless.

advice from someone with experience isn't worthless. 

just stupid me decided to unpark my V3 and send it :(

Sorry about that..

>	M.
>
>

Thanks!