Re: [XEN PATCH v2 06/15] x86/p2m: guard altp2m code with CONFIG_ALTP2M option

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 15 May 2024, Sergiy Kibrik wrote:
> Instead of using generic CONFIG_HVM option switch to a bit more specific
> CONFIG_ALTP2M option for altp2m support. Also guard altp2m routines, so that
> they can be disabled completely in the build -- when target platform does not
> actually support altp2m (AMD-V & ARM as of now).
> 
> Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@xxxxxxxx>
> CC: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
> ---
> changes in v2:
>  - use separate CONFIG_ALTP2M option instead of CONFIG_VMX
> ---
>  xen/arch/x86/include/asm/altp2m.h  |  5 ++++-
>  xen/arch/x86/include/asm/hvm/hvm.h |  2 +-
>  xen/arch/x86/include/asm/p2m.h     | 17 ++++++++++++++++-
>  xen/arch/x86/mm/Makefile           |  2 +-
>  4 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/x86/include/asm/altp2m.h 
> b/xen/arch/x86/include/asm/altp2m.h
> index e5e59cbd68..092b13e231 100644
> --- a/xen/arch/x86/include/asm/altp2m.h
> +++ b/xen/arch/x86/include/asm/altp2m.h
> @@ -7,7 +7,7 @@
>  #ifndef __ASM_X86_ALTP2M_H
>  #define __ASM_X86_ALTP2M_H
>  
> -#ifdef CONFIG_HVM
> +#ifdef CONFIG_ALTP2M
>  
>  #include <xen/types.h>
>  #include <xen/sched.h>         /* for struct vcpu, struct domain */
> @@ -38,7 +38,10 @@ static inline bool altp2m_active(const struct domain *d)
>  }
>  
>  /* Only declaration is needed. DCE will optimise it out when linking. */
> +void altp2m_vcpu_initialise(struct vcpu *v);
> +void altp2m_vcpu_destroy(struct vcpu *v);
>  uint16_t altp2m_vcpu_idx(const struct vcpu *v);
> +int altp2m_vcpu_enable_ve(struct vcpu *v, gfn_t gfn);
>  void altp2m_vcpu_disable_ve(struct vcpu *v);
>  
>  #endif
> diff --git a/xen/arch/x86/include/asm/hvm/hvm.h 
> b/xen/arch/x86/include/asm/hvm/hvm.h
> index 0c9e6f1564..4f03dd7af8 100644
> --- a/xen/arch/x86/include/asm/hvm/hvm.h
> +++ b/xen/arch/x86/include/asm/hvm/hvm.h
> @@ -670,7 +670,7 @@ static inline bool hvm_hap_supported(void)
>  /* returns true if hardware supports alternate p2m's */
>  static inline bool hvm_altp2m_supported(void)
>  {
> -    return hvm_funcs.caps.altp2m;
> +    return IS_ENABLED(CONFIG_ALTP2M) && hvm_funcs.caps.altp2m;
>  }
>  
>  /* Returns true if we have the minimum hardware requirements for nested virt 
> */
> diff --git a/xen/arch/x86/include/asm/p2m.h b/xen/arch/x86/include/asm/p2m.h
> index 111badf89a..855e69d24a 100644
> --- a/xen/arch/x86/include/asm/p2m.h
> +++ b/xen/arch/x86/include/asm/p2m.h
> @@ -581,9 +581,9 @@ 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

Is it necessary? Can't we just replace CONFIG_HVM with CONFIG_ALTP2M on
the same line