Re: [PATCH] x86/MTRR: avoid several indirect calls

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 17/01/2024 9:32 am, Jan Beulich wrote:
> The use of (supposedly) vendor-specific hooks is a relic from the days
> when Xen was still possible to build as 32-bit binary. There's no
> expectation that a new need for such an abstraction would arise. Convert
> mttr_if to a mere boolean and all prior calls through it to direct ones,
> thus allowing to eliminate 6 ENDBR from .text.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

> ---
> Strictly speaking mtrr_if could go apparently away as well, for
> effectively mirroring cpu_has_mtrr now (once mtrr_bp_init() has run).
> Replacing the variable uses would be further code churn, though.

There's loads more cleanup I'd like to do, but that doesn't invalidate
this as an improvement.

Bloat-o-meter reports:

    add/remove: 7/8 grow/shrink: 1/14 up/down: 1354/-1803 (-449)

for this change, which definitely welcome.  That said ...

> --- a/xen/arch/x86/cpu/mtrr/generic.c
> +++ b/xen/arch/x86/cpu/mtrr/generic.c
> @@ -586,21 +586,9 @@ int cf_check generic_validate_add_page(
>  }
>  
>  
> -static int cf_check generic_have_wrcomb(void)
> +bool mtrr_have_wrcomb(void)
>  {
>       unsigned long config;
>       rdmsrl(MSR_MTRRcap, config);
>       return (config & (1ULL << 10));
>  }

... I do have half a mind to fix this as a followon.  We really don't
want to be re-reading the caps MSR for every call to mtrr_add_page().

~Andrew