Re: [PATCH v3 1/3] KVM: arm64: skip pKVM cache flushes for non cacheable mappings
Bradley Morgan
include at grrlz.net
Wed Jul 1 09:57:48 PDT 2026
On July 1, 2026 5:56:37 PM GMT+01:00, Leonardo Bras <leo.bras at arm.com>
wrote:
>On Wed, Jul 01, 2026 at 05:54:40PM +0100, Bradley Morgan wrote:
>> On July 1, 2026 5:53:34 PM GMT+01:00, Leonardo Bras <leo.bras at arm.com>
>> wrote:
>> >On Wed, Jul 01, 2026 at 05:40:46PM +0100, Bradley Morgan wrote:
>> >> On July 1, 2026 5:05:53 PM GMT+01:00, Leonardo Bras
><leo.bras at arm.com>
>> >> wrote:
>> >> >On Wed, Jun 24, 2026 at 04:00:26PM +0000, Bradley Morgan wrote:
>> >> >> pKVM keeps its own mapping list for stage 2 operations. Its flush
>> >path
>> >> >> uses that list directly, so it lost the PTE attribute check done
>by
>> >the
>> >> >> generic stage 2 walker.
>> >> >>
>> >> >> Record whether a mapping is cacheable and skip cache maintenance
>for
>> >> >> mappings that are not cacheable.
>> >> >>
>> >> >> Fixes: e912efed485a ("KVM: arm64: Introduce the EL1 pKVM MMU")
>> >> >> Signed-off-by: Bradley Morgan <include at grrlz.net>
>> >> >> ---
>> >> >> arch/arm64/kvm/pkvm.c | 51
>> >++++++++++++++++++++++++++++++++++---------
>> >> >> 1 file changed, 41 insertions(+), 10 deletions(-)
>> >> >>
>> >> >> diff --git a/arch/arm64/kvm/pkvm.c b/arch/arm64/kvm/pkvm.c
>> >> >> index 428723b1b0f5..ca6e823028c2 100644
>> >> >> --- a/arch/arm64/kvm/pkvm.c
>> >> >> +++ b/arch/arm64/kvm/pkvm.c
>> >> >> @@ -302,9 +302,32 @@ static u64 __pkvm_mapping_start(struct
>> >pkvm_mapping
>> >> >*m)
>> >> >> return m->gfn * PAGE_SIZE;
>> >> >> }
>> >> >>
>> >> >> +#define PKVM_MAPPING_NR_PAGES_MASK GENMASK_ULL(47, 0)
>> >> >> +#define PKVM_MAPPING_CACHEABLE BIT_ULL(48)
>> >> >
>> >> >Out of curiosity here, why do you choose to use bit 48 here instead
>of,
>> >> >let's say, bit 63?
>> >> >
>> >> >(I know it makes absolutely no difference to inner working here, as
>> >there
>> >> >should probably not be 2^48 pages in one mapping.)
>> >> >
>> >> >Thanks!
>> >> >Leo
>> >>
>> >>
>> >> sup Leo, here's a quote from maz
>> >
>> >Hi Bradley,
>> >
>> >>
>> >> "This thing is already big enough, let's not add a bool right in the
>> >> middle (use pahole to find out why this is bad).
>> >
>> >I suppose you proposed to add a bool into a struct, maybe?
>> >It would screw the struct alignment.
>>
>> yep, crappy old me
>>
>
>Hah, you were probably focused on the big picture.
>
>>
>> >> Given that nr_pages
>> >> is for a range, and that the minimum page size uses 12 bits, the
>> >> largest number of pages you can have here is 56-12=48 bit wide.
>That's
>> >> another 16 bits worth of flags you can use."
>> >
>> >Humm, makes sense.
>> >And since he mentions 16 bits worth of flags, you start by using the
>48th
>> >bit. Ok, got your rationale.
>> >
>> >(I would possibly start with the 63, though, but that's more on
>personal
>> >taste)
>>
>> 48 won't make the world blow up :)
>
>yeap,
Would you like to be CCed on v4 or nahhhh?
>>
>> >>
>> >> this should just clarify things, any questions, feel more than free
>to
>> >ask!
>> >>
>> >> (btw V4 is coming soon)
>> >
>> >Thanks!
>> >Leo
>> >
>> >>
>> >> >> +
>> >> >> +static u64 pkvm_mapping_nr_pages(struct pkvm_mapping *m)
>> >> >> +{
>> >> >> + return m->nr_pages & PKVM_MAPPING_NR_PAGES_MASK;
>> >> >> +}
>> >> >> +
>> >> >> +static bool pkvm_mapping_is_cacheable(struct pkvm_mapping *m)
>> >> >> +{
>> >> >> + return m->nr_pages & PKVM_MAPPING_CACHEABLE;
>> >> >> +}
>> >> >> +
>> >> >> +static void pkvm_mapping_set_nr_pages(struct pkvm_mapping *m, u64
>> >> >nr_pages,
>> >> >> + bool cacheable)
>> >> >> +{
>> >> >> + WARN_ON_ONCE(nr_pages & ~PKVM_MAPPING_NR_PAGES_MASK);
>> >> >> +
>> >> >> + m->nr_pages = nr_pages & PKVM_MAPPING_NR_PAGES_MASK;
>> >> >> + if (cacheable)
>> >> >> + m->nr_pages |= PKVM_MAPPING_CACHEABLE;
>> >> >> +}
>> >> >> +
>> >> >> static u64 __pkvm_mapping_end(struct pkvm_mapping *m)
>> >> >> {
>> >> >> - return (m->gfn + m->nr_pages) * PAGE_SIZE - 1;
>> >> >> + return (m->gfn + pkvm_mapping_nr_pages(m)) * PAGE_SIZE - 1;
>> >> >> }
>> >> >>
>> >> >> INTERVAL_TREE_DEFINE(struct pkvm_mapping, node, u64,
>__subtree_last,
>> >> >> @@ -350,7 +373,7 @@ static int
>__pkvm_pgtable_stage2_reclaim(struct
>> >> >kvm_pgtable *pgt, u64 start, u64
>> >> >> continue;
>> >> >>
>> >> >> page = pfn_to_page(mapping->pfn);
>> >> >> - WARN_ON_ONCE(mapping->nr_pages != 1);
>> >> >> + WARN_ON_ONCE(pkvm_mapping_nr_pages(mapping) != 1);
>> >> >> unpin_user_pages_dirty_lock(&page, 1, true);
>> >> >> account_locked_vm(kvm->mm, 1, false);
>> >> >> pkvm_mapping_remove(mapping, &pgt->pkvm_mappings);
>> >> >> @@ -369,7 +392,7 @@ static int
>__pkvm_pgtable_stage2_unshare(struct
>> >> >kvm_pgtable *pgt, u64 start, u64
>> >> >>
>> >> >> for_each_mapping_in_range_safe(pgt, start, end, mapping) {
>> >> >> ret = kvm_call_hyp_nvhe(__pkvm_host_unshare_guest, handle, mapping->gfn,
>> >> >> - mapping->nr_pages);
>> >> >> + pkvm_mapping_nr_pages(mapping));
>> >> >> if (WARN_ON(ret))
>> >> >> return ret;
>> >> >> pkvm_mapping_remove(mapping, &pgt->pkvm_mappings);
>> >> >> @@ -448,7 +471,7 @@ int pkvm_pgtable_stage2_map(struct kvm_pgtable
>> >*pgt,
>> >> >u64 addr, u64 size,
>> >> >> * permission faults are handled in the relax_perms() path.
>> >> >> */
>> >> >> if (mapping) {
>> >> >> - if (size == (mapping->nr_pages * PAGE_SIZE))
>> >> >> + if (size == (pkvm_mapping_nr_pages(mapping) * PAGE_SIZE))
>> >> >> return -EAGAIN;
>> >> >>
>> >> >> /*
>> >> >> @@ -472,7 +495,9 @@ int pkvm_pgtable_stage2_map(struct kvm_pgtable
>> >*pgt,
>> >> >u64 addr, u64 size,
>> >> >> swap(mapping, cache->mapping);
>> >> >> mapping->gfn = gfn;
>> >> >> mapping->pfn = pfn;
>> >> >> - mapping->nr_pages = size / PAGE_SIZE;
>> >> >> + pkvm_mapping_set_nr_pages(mapping, size / PAGE_SIZE,
>> >> >> + !(prot & (KVM_PGTABLE_PROT_DEVICE |
>> >> >> + KVM_PGTABLE_PROT_NORMAL_NC)));
>> >> >> pkvm_mapping_insert(mapping, &pgt->pkvm_mappings);
>> >> >>
>> >> >> return ret;
>> >> >> @@ -503,7 +528,7 @@ int pkvm_pgtable_stage2_wrprotect(struct
>> >kvm_pgtable
>> >> >*pgt, u64 addr, u64 size)
>> >> >> lockdep_assert_held(&kvm->mmu_lock);
>> >> >> for_each_mapping_in_range_safe(pgt, addr, addr + size, mapping) {
>> >> >> ret = kvm_call_hyp_nvhe(__pkvm_host_wrprotect_guest, handle, mapping->gfn,
>> >> >> - mapping->nr_pages);
>> >> >> + pkvm_mapping_nr_pages(mapping));
>> >> >> if (WARN_ON(ret))
>> >> >> break;
>> >> >> }
>> >> >> @@ -517,9 +542,13 @@ int pkvm_pgtable_stage2_flush(struct
>kvm_pgtable
>> >> >*pgt, u64 addr, u64 size)
>> >> >> struct pkvm_mapping *mapping;
>> >> >>
>> >> >> lockdep_assert_held(&kvm->mmu_lock);
>> >> >> - for_each_mapping_in_range_safe(pgt, addr, addr + size, mapping)
>> >> >> + for_each_mapping_in_range_safe(pgt, addr, addr + size, mapping) {
>> >> >> + if (!pkvm_mapping_is_cacheable(mapping))
>> >> >> + continue;
>> >> >> +
>> >> >> __clean_dcache_guest_page(pfn_to_kaddr(mapping->pfn),
>> >> >> - PAGE_SIZE * mapping->nr_pages);
>> >> >> + PAGE_SIZE * pkvm_mapping_nr_pages(mapping));
>> >> >> + }
>> >> >>
>> >> >> return 0;
>> >> >> }
>> >> >> @@ -536,8 +565,10 @@ bool
>pkvm_pgtable_stage2_test_clear_young(struct
>> >> >kvm_pgtable *pgt, u64 addr, u64
>> >> >>
>> >> >> lockdep_assert_held(&kvm->mmu_lock);
>> >> >> for_each_mapping_in_range_safe(pgt, addr, addr + size, mapping)
>> >> >> - young |= kvm_call_hyp_nvhe(__pkvm_host_test_clear_young_guest, handle, mapping->gfn,
>> >> >> - mapping->nr_pages, mkold);
>> >> >> + young |= kvm_call_hyp_nvhe(__pkvm_host_test_clear_young_guest,
>> >> >> + handle, mapping->gfn,
>> >> >> + pkvm_mapping_nr_pages(mapping),
>> >> >> + mkold);
>> >> >>
>> >> >> return young;
>> >> >> }
>> >> >> --
>> >> >> 2.53.0
>> >> >>
>> >> >
>> >>
>> >> Thanks!
>> >
>>
>> Thanks!
>
>Thanks!
>
Thanks!
More information about the linux-arm-kernel
mailing list