Re: [XEN PATCH v3 04/16] x86: introduce CONFIG_ALTP2M Kconfig option

[Sergiy Kibrik <sergiy_kibrik@xxxxxxxx>]

07.06.24 10:47, Jan Beulich:
> On 03.06.2024 13:13, Sergiy Kibrik wrote:
> > --- a/xen/arch/x86/include/asm/p2m.h
> > +++ b/xen/arch/x86/include/asm/p2m.h
> > @@ -577,10 +577,10 @@ static inline gfn_t mfn_to_gfn(const struct domain *d, 
> > mfn_t mfn)
> >           return _gfn(mfn_x(mfn));
> >   }
> >   
> > -#ifdef CONFIG_HVM
> >   #define AP2MGET_prepopulate true
> >   #define AP2MGET_query false
> >   
> > +#ifdef CONFIG_ALTP2M
> >   /*
> >    * Looks up altp2m entry. If the entry is not found it looks up the entry in
> >    * hostp2m.
> In principle this #ifdef shouldn't need moving. It's just that the
> three use sites need taking care of a little differently. E.g. ...
>
> > @@ -589,6 +589,15 @@ static inline gfn_t mfn_to_gfn(const struct domain *d, 
> > mfn_t mfn)
> >   int altp2m_get_effective_entry(struct p2m_domain *ap2m, gfn_t gfn, mfn_t *mfn,
> >                                  p2m_type_t *t, p2m_access_t *a,
> >                                  bool prepopulate);
> > +#else
> > +static inline int altp2m_get_effective_entry(struct p2m_domain *ap2m,
> > +                                             gfn_t gfn, mfn_t *mfn,
> > +                                             p2m_type_t *t, p2m_access_t *a,
> > +                                             bool prepopulate)
> > +{
> > +    ASSERT_UNREACHABLE();
> > +    return -EOPNOTSUPP;
> > +}
> static inline int altp2m_get_effective_entry(struct p2m_domain *ap2m,
>                                               gfn_t gfn, mfn_t *mfn,
>                                               p2m_type_t *t, p2m_access_t *a)
> {
>      ASSERT_UNREACHABLE();
>      return -EOPNOTSUPP;
> }
> #define altp2m_get_effective_entry(ap2m, gfn, mfn, t, a, prepopulate) \
>          altp2m_get_effective_entry(ap2m, gfn, mfn, t, a)
>
> Misra doesn't like such shadowing, so the inline function may want
> naming slightly differently, e.g. _ap2m_get_effective_entry().

I can do that, sure.
Though here I'm curious what benefits we're getting from little 
complication of an indirect call to an empty stub -- is avoiding of 
AP2MGET_* defines worth it?
  -Sergiy