Re: [PATCH] x86/P2M: correct type use in p2m_put_gfn()

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Wed, Feb 04, 2026 at 08:49:53AM +0100, Jan Beulich wrote:
> On 04.02.2026 08:35, Roger Pau Monné wrote:
> > On Tue, Feb 03, 2026 at 03:01:27PM +0100, Jan Beulich wrote:
> >> Everywhere else gfn_t are passed into respective GFN locking macros: Do so
> >> here as well.
> >>
> >> Amends: 819cdc5a7301 ("x86/p2m: re-arrange {,__}put_gfn()")
> >> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> > 
> > Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> Thanks.
> 
> >> ---
> >> Easy to spot by adding ASSERT(!gfn_eq(g, INVALID_GFN)) to the respective
> >> macros. While imo that should be a correct thing to do (as with
> >> hypothetical split locks a valid GFN would really need passing in, in
> >> order to be able to figure out which lock to use), we can't do so right
> >> now: The lock is acquired ahead of respective checking in a number of
> >> places, e.g. in p2m_get_gfn_type_access().
> > 
> > Could we convert those macros into static inlines?  It's dangerous to
> > use macros like those when the parameters are dropped, as the
> > parameter is not evaluated at all.
> 
> It is. Seeing how the header is used, converting may be possible. There's
> one slight concern I'd have with doing so: It would move us one step
> closer to giving the impression that the arguments passed are correct at
> all use sites (while as long as they're entirely ignored, that's kind of
> a hint that they may need checking). I can't point at it right now, but
> I'm pretty sure I had come across at least one place where they're pretty
> clearly wrong.

Well, having at least the type check is better than not checking
anything at all.  By clearly wrong you mean passing INVALID_GFN, or a
random GFN that had something do to with the context?

Thanks, Roger.