Re: [PATCH] x86: harden use of calc_ler_msr()

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Mon, May 30, 2022 at 05:48:51PM +0200, Jan Beulich wrote:
> Avoid calling the function more than once, thus making sure we won't,
> under any unusual circumstances, attempt to enable XEN_LER late (which
> can't work, for setup_force_cpu_cap() being __init. In turn this then
> allows making the function itself __init, too.
> 
> While fiddling with section attributes in this area, also move the two
> involved variables to .data.ro_after_init.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -126,11 +126,11 @@ DEFINE_PER_CPU_PAGE_ALIGNED(struct tss_p
>  static int debug_stack_lines = 20;
>  integer_param("debug_stack_lines", debug_stack_lines);
>  
> -static bool opt_ler;
> +static bool __ro_after_init opt_ler;
>  boolean_param("ler", opt_ler);
>  
>  /* LastExceptionFromIP on this hardware.  Zero if LER is not in use. */
> -unsigned int __read_mostly ler_msr;
> +unsigned int __ro_after_init ler_msr;
>  
>  const unsigned int nmi_cpu;
>  
> @@ -2133,7 +2133,7 @@ static void __init set_intr_gate(unsigne
>      __set_intr_gate(n, 0, addr);
>  }
>  
> -static unsigned int calc_ler_msr(void)
> +static unsigned int noinline __init calc_ler_msr(void)
>  {
>      switch ( boot_cpu_data.x86_vendor )
>      {
> @@ -2171,8 +2171,17 @@ void percpu_traps_init(void)
>      if ( !opt_ler )
>          return;
>  
> -    if ( !ler_msr && (ler_msr = calc_ler_msr()) )
> +    if ( !ler_msr )
> +    {
> +        ler_msr = calc_ler_msr();
> +        if ( !ler_msr )
> +        {

While doing this rework it might make sense to print some message
here, like: "LER option requested but no LBR support available" or
similar IMO.

The rest LGTM:

Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks, Roger.