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

[Jan Beulich <jbeulich@xxxxxxxx>]

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().

> @@ -914,8 +923,14 @@ static inline bool p2m_set_altp2m(struct vcpu *v, 
> unsigned int idx)
>  /* Switch alternate p2m for a single vcpu */
>  bool p2m_switch_vcpu_altp2m_by_id(struct vcpu *v, unsigned int idx);
>  
> +#ifdef CONFIG_ALTP2M
>  /* Check to see if vcpu should be switched to a different p2m. */
>  void p2m_altp2m_check(struct vcpu *v, uint16_t idx);
> +#else
> +static inline void p2m_altp2m_check(struct vcpu *v, uint16_t idx)
> +{
> +}

Btw, no need to put the braces on separate lines for such a stub.

Jan