linux-kernel - Re: [PATCH v2 6/6] vfio/type1: Use mapping page mask for pfnmaps

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHTA-ubjxqs7Rh8ERXqFXCRAm7WoMMRQftSPLmrK61jra7+gYg@mail.gmail.com>
Date: Thu, 27 Feb 2025 14:22:07 -0600
From: Mitchell Augustin <mitchell.augustin@...onical.com>
To: Alex Williamson <alex.williamson@...hat.com>
Cc: Peter Xu <peterx@...hat.com>, kvm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	clg@...hat.com, jgg@...dia.com, willy@...radead.org
Subject: Re: [PATCH v2 6/6] vfio/type1: Use mapping page mask for pfnmaps

Thanks Alex, that's super helpful and makes sense to me now!

-Mitchell

On Wed, Feb 26, 2025 at 11:55 AM Alex Williamson
<alex.williamson@...hat.com> wrote:
>
> On Wed, 19 Feb 2025 14:32:35 -0600
> Mitchell Augustin <mitchell.augustin@...onical.com> wrote:
>
> > > Thanks for the review and testing!
> >
> > Sure thing, thanks for the patch set!
> >
> > If you happen to have a few minutes, I'm struggling to understand the
> > epfn computation and would appreciate some insight.
>
> Sorry, this slipped off my todo list for a few days.
>
> > My current understanding (very possibly incorrect):
> > - epfn is intended to be the last page frame number that can be
> > represented at the mapping level corresponding to addr_mask. (so, if
> > addr_mask == PUD_MASK, epfn would be the highest pfn still in PUD
> > level).
>
> Actually epfn is the first pfn of the next addr_mask level page.  The
> value in the parens (*pfn | (~addr_mask >> PAGE_SHIFT)) is the last pfn
> within the same level page.  We could do it either way, it's just a
> matter of where the +1 gets added.
>
> > - ret should be == npages if all pfns in the requested vma are within
> > the memory hierarchy level denoted by addr_mask. If npages is more
> > than can be represented at that level, ret == the max number of page
> > frames representable at addr_mask level.
>
> Yes.
>
> > - - (if the second case is true, that means we were not able to obtain
> > all requested pages due to running out of PFNs at the current mapping
> > level)
>
> vaddr_get_pfns() is called again if we haven't reached npage.
> Specifically, from vfio_pin_pages_remote() we hit the added continue
> under the !batch->size branch.  If the pfnmaps are fully PUD aligned,
> we'll call vaddr_get_pfns() once per PUD_SIZE, vfio_pin_pages_remote()
> will only return with the full requested npage value, and we'll only
> call vfio_iommu_map() once.  The latter has always been true, the
> difference is the number of times we iterate calling vaddr_get_pfns().
>
> > If the above is all correct, what is confusing me is where the "(*pfn)
> > | " comes into this equation. If epfn is meant to be the last pfn
> > representable at addr_mask level of the hierarchy, wouldn't that be
> > represented by (~pgmask >> PAGE_SHIFT) alone?
>
> (~addr_mask >> PAGE_SHIFT) gives us the last pfn relative to zero.  We
> want the last pfn relative to *pfn, therefore we OR in *pfn.  The OR
> handles any offset that *pfn might have within the addr_mask page, so
> this operation always provides the last pfn of the addr_mask page
> relative to *pfn.  +1 because we want to calculate the number of pfns
> until the next page.  Thanks,
>
> Alex
>


-- 
Mitchell Augustin
Software Engineer - Ubuntu Partner Engineering