Re: [PATCH v2 1/2] x86/p2m: make p2m_get_page_from_gfn() handle grant case correctly

[George Dunlap <george.dunlap@xxxxxxxxx>]

On Thu, Feb 8, 2024 at 4:02 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 08.02.2024 07:32, George Dunlap wrote:
> > On Thu, Jun 23, 2022 at 7:54 PM Jan Beulich <jbeulich@xxxxxxxx> wrote:
> >
> >> Grant P2M entries, which are covered by p2m_is_any_ram(), wouldn't pass
> >> the get_page() unless the grant was a local one. These need to take the
> >> same path as foreign entries. Just the assertion there is not valid for
> >> local grants, and hence it triggering needs to be avoided.
> >>
> >
> > I think I'd say:
> >
> > ---
> > The 'fast' path of p2m_get_page_from_gfn handles three cases: normal ram,
> > foreign p2m entries, and grant map entries.  For normal ram and grant table
> > entries, get_page() is called, but for foreign entries,
> > page_get_owner_and_reference() is called, since the current domain is
> > expected not to be the owner.
> >
> > Unfortunately, grant maps are *also* generally expected to be owned by
> > foreign domains; so this function will fail for any p2m entry containing a
> > grant map that doesn't happen to be local.
> >
> > Have grant maps take the same path as foreign entries.  Since grants may
> > actually be either foreign or local, adjust the assertion to allow for this.
> > ---
>
> Sure, thanks, I can use this, but then I'd perhaps ought to add your
> S-o-b instead of ...
<snip>
> ... R-b, requiring yet someone else's ack?

Legally I think the SoB is more for the provenance of the code than
the commit messages; so it would mainly be to credit me, which I'm not
particularly fussed by.

That said, we did just put something in MAINTAINERS about how to deal
with this situation; You sending the patch implicitly approves all the
changes I made, so then if I give an R-b, that approves all the
changes you made, satisfying the requirements.

 -George