Re: [PATCH] x86/HVM: get_pat_flags() is needed only by shadow code

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 18/07/2024 11:10 am, Jan Beulich wrote:
> Therefore with SHADOW_PAGING=n this is better compiled out, to avoid
> leaving around unreachable/dead code.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> --- a/xen/arch/x86/hvm/mtrr.c
> +++ b/xen/arch/x86/hvm/mtrr.c
> @@ -271,6 +271,8 @@ int mtrr_get_type(const struct mtrr_stat
>     return overlap_mtrr_pos;
>  }
>  
> +#ifdef CONFIG_SHADOW_PAGING
> +
>  /*
>   * return the memory type from PAT.
>   * NOTE: valid only when paging is enabled.
> @@ -359,6 +361,8 @@ uint32_t get_pat_flags(struct vcpu *v,
>      return pat_type_2_pte_flags(pat_entry_value);
>  }
>  
> +#endif /* CONFIG_SHADOW_PAGING */
> +
>  static inline bool valid_mtrr_type(uint8_t type)
>  {
>      switch ( type )

While I can see this is true, the fact it is indicates that we have
bugs/problems elsewhere.

It is not only the shadow code that has to combine attributes like this,
so we've either got opencoding elsewhere, or a bad abstraction.

(This is an observation, rather than a specific objection.)

~Andrew