Re: [PATCH 1/3] KVM: arm64: Clear the whole hugetlb folio when sanitising MTE tags

[Lorenzo Stoakes (ARM)]

On Tue, Oct 20, 2026 at 02:09:45PM +0100, Fuad Tabba wrote:
> Hi Lorenzo,
>
> On Tue, 20 Oct 2026 12:52:54 +0100, Lorenzo Stoakes (ARM)
> <ljs@xxxxxxxxxx> wrote:
> [...]
> > On Mon, Oct 19, 2026 at 02:21:01PM +0100, Fuad Tabba wrote:
> > > sanitise_mte_tags() clears the tags of the pages about to be mapped at
> > > stage-2 before marking them tagged. For a hugetlb folio the flag covers
> > > every page in the folio, but the loop only covers the mapping being
> > > installed. Stage-2 maps a folio in pieces for the contiguous hugetlb
> > > sizes, and for any size under dirty logging. The first fault then marks
> > > the whole folio after clearing only its own piece, and the rest keeps
> > > its previous owner's tags for the guest to read.
> >
> > I'm guessing the folio size is always >= size here?
>
> Yes, the stage-2 mapping of a hugetlb VMA is never larger than its
> hugetlb page size, so it always sits within one folio.

OK good! :)

>
> [...]
> > > diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> [...]
> > > @@ -1514,7 +1514,8 @@ static void sanitise_mte_tags(struct kvm *kvm, kvm_pfn_t pfn,
> > >          if (folio_test_hugetlb(folio)) {
> > >                  /* Hugetlb has MTE flags set on head page only */
> >
> > Is this comment redundant now?
>
> I'd keep it: it's why the whole folio has to be cleared, as the flags
> on the head page cover every page in it.

OK, but there's an inconsistency between the comments in 1/3 and 2/3 (you update
one in 2/3 to reflect what's happening), so maybe worth having the comment from
2/3 on both, rather than this one?

(the PG_arch1,2,3 flags are folio flags unless there's something weird so it's
sort of redundant I think?)

>
> > >                  if (folio_try_hugetlb_mte_tagging(folio)) {
> > > -                        for (i = 0; i < nr_pages; i++, page++)
> > > +                        page = folio_page(folio, 0);
> > > +                        for (i = 0; i < folio_nr_pages(folio); i++, page++)
> >
> > I know it's a stable fix, but the fact that this pattern exists in multiple
> > places really argues for a mte_clear_folio_tags(), which would make this
> > trivially:
> >
> >         mte_clear_folio_tags(folio);
> >
> > And avoid the re-use of page etc.
> >
> > It's not a huge change and makes the next patch in the series simpler too so
> > arguably could be part of the series?
>
> I think you're right about the helper, but I'd rather keep the fixes
> as small as possible for the backports. I'll send
> mte_clear_folio_tags() as a separate patch on top of these, using it
> in mte_sync_tags() too. In v2 the loop uses folio_page(folio, i)
> rather than reusing page.

Sure makes sense, though honestly I think the duplication there means delta of
lines of code is small. But then again, adding a helper in a Fixes: is iffy
itself so fair enough :)

--
Cheers, Lorenzo