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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 15.05.2024 11:10, Sergiy Kibrik wrote:
> @@ -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);

These additions look unrelated, as long as the description says nothing in
this regard.

> --- 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;

Which in turn raises the question whether the altp2m struct field shouldn't
become conditional upon CONFIG_ALTP2M too (or rather: instead, as the change
here then would need to be done differently). Yet maybe that would entail
further changes elsewhere, so may well better be left for later.

> --- a/xen/arch/x86/mm/Makefile
> +++ b/xen/arch/x86/mm/Makefile
> @@ -1,7 +1,7 @@
>  obj-y += shadow/
>  obj-$(CONFIG_HVM) += hap/
>  
> -obj-$(CONFIG_HVM) += altp2m.o
> +obj-$(CONFIG_ALTP2M) += altp2m.o

This change I think wants to move to patch 5.

Jan