Re: [PATCH 15/22] x86/traps: Introduce opt_fred

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.08.2025 22:23, Andrew Cooper wrote:
> ... disabled by default.  There is a lot of work before FRED can be enabled by
> default.
> 
> One part of FRED, the LKGS (Load Kernel GS) instruction, is enumerated
> separately but is mandatory as FRED disallows the SWAPGS instruction.
> Therefore, both CPUID bits must be checked.

See my (further) reply to patch 13 - I think FRED simply ought to depend on
LKGS.

> @@ -20,6 +22,9 @@ unsigned int __ro_after_init ler_msr;
>  static bool __initdata opt_ler;
>  boolean_param("ler", opt_ler);
>  
> +int8_t __ro_after_init opt_fred = 0; /* -1 when supported. */

I'm a little puzzled by the comment? DYM "once default-enabled"? Then ...

> @@ -305,6 +310,32 @@ void __init traps_init(void)
>      /* Replace early pagefault with real pagefault handler. */
>      _update_gate_addr_lower(&bsp_idt[X86_EXC_PF], entry_PF);
>  
> +    if ( !cpu_has_fred || !cpu_has_lkgs )
> +    {
> +        if ( opt_fred )

... this won't work anymore once the initializer is changed.

> +            printk(XENLOG_WARNING "FRED not available, ignoring\n");
> +        opt_fred = false;

Better use 0 here?

> +    }
> +
> +    if ( opt_fred == -1 )
> +        opt_fred = !pv_shim;

Imo it would be better to have the initializer be -1 right away, and comment
out the "!pv_shim" here, until we mean it to be default-enabled.

> +    if ( opt_fred )
> +    {
> +#ifdef CONFIG_PV32
> +        if ( opt_pv32 )
> +        {
> +            opt_pv32 = 0;
> +            printk(XENLOG_INFO "Disabling PV32 due to FRED\n");
> +        }
> +#endif
> +        printk("Using FRED event delivery\n");
> +    }
> +    else
> +    {
> +        printk("Using IDT event delivery\n");
> +    }

Could I talk you into omitting the figure braces here? Hmm, or perhaps you
mean to later move code here.

Jan