[PATCH 09/17] KVM: arm64: Move VMA-related information to kvm_s2_fault_vma_info

[Wei-Lin Chang]

Hi,

On Sat, Mar 21, 2026 at 09:50:40AM +0000, Marc Zyngier wrote:
> On Wed, 18 Mar 2026 16:14:19 +0000,
> Fuad Tabba <tabba at google.com> wrote:
> > 
> > Hi Joey,
> > 
> > First, thanks for the reviews and the comments on my series. You're
> > right about my changes wrongly editing "page". I wanted it to be as
> > mechanical as possible to make it easy to review, but it ended up
> > being too mechanical.
> > 
> > <snip>
> > 
> > > > -     /* Mark the fault->page dirty only if the fault is handled successfully */
> > > > -     if (fault->writable && !ret)
> > > > -             mark_page_dirty_in_slot(kvm, s2fd->memslot, get_canonical_gfn(s2fd, fault));
> > > > +     /* Mark the page dirty only if the fault is handled successfully */
> > > > +     if (fault->writable && !ret) {
> > > > +             phys_addr_t ipa = gfn_to_gpa(get_canonical_gfn(s2fd, s2vi));
> > > > +             ipa &= ~(mapping_size - 1);
> > > > +             mark_page_dirty_in_slot(kvm, s2fd->memslot, gpa_to_gfn(ipa));
> > >
> > > I don't understand this change, why do we need to mask stuff now?
> > 
> > Let me see if _I_ understand it (Marc, please correct me if I'm wrong).
> > 
> > Before this patch, fault->gfn and fault->vma_pagesize were mutable,
> > and transparent_hugepage_adjust() modified both directly. In addition
> > to this being confusing (which gfn is this: the host /canonical or the
> > nested one?), it made it more difficult to separate the logic.
> > 
> > So, to mark a dirty page, it did this:
> > -             mark_page_dirty_in_slot(kvm, s2fd->memslot,
> > get_canonical_gfn(s2fd, fault));
> > 
> > which relied on the old struct fault to calculate the canonical gfn
> > using the (magically) THP adjusted fault->vma_pagesize.
> > 
> > Now that fault (or s2vi, its successor in this case) isn't mutable, we
> > need to get the canonical gfn using the host mapping size.
> 
> It's exactly that, and it is slightly clearer if you look at how
> mapping_size is updated:
> 
> 	mapping_size = transparent_hugepage_adjust(kvm, s2fd->memslot,
> 						   s2fd->hva, &fault->pfn,
> 						   &gfn);
> 
> The faulting IPA is represented by 'gfn', and gets correctly updated
> by the helper. But that doesn't adjust the 'canonical' IPA, which is
> used for any memslot related update.
> 
> So if we need to call into mark_page_dirty_in_slot(), we really need
> to pick the base of the region we are actually marking dirty, hence
> the masking of the bottom bits.
> 
> Does this make sense? This is one of the area where the constification
> results in slightly more complicated code, as we can't update things
> in place anymore.

Sorry for being late, I think I am missing something:
If dirty logging is enabled, then the mapping size would be capped at
PAGE_SIZE, and THP adjust wouldn't happen. Why do we still need to align
the canonical IPA down?

Thanks,
Wei-Lin Chang

> 
> Thanks,
> 
> 	M.
> 
> -- 
> Jazz isn't dead. It just smells funny.