Re: [PATCH] x86/cpu: revert opt_allow_unsafe from __ro_after_init to __read_mostly

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

On Mon, Sep 02, 2024 at 05:16:05PM +0200, Jan Beulich wrote:
> On 02.09.2024 17:00, Roger Pau Monne wrote:
> > Making opt_allow_unsafe read only after init requires changes to the logic 
> > in
> > init_amd(), otherwise the following #PF happens on CPU hotplug:
> > 
> > ----[ Xen-4.20.0-1-d  x86_64  debug=y  Tainted:     H  ]----
> > CPU:    1
> > RIP:    e008:[<ffff82d040291081>] arch/x86/cpu/amd.c#init_amd+0x37f/0x993
> > [...]
> > Xen call trace:
> >    [<ffff82d040291081>] R arch/x86/cpu/amd.c#init_amd+0x37f/0x993
> >    [<ffff82d040291fbe>] F identify_cpu+0x2d4/0x4db
> >    [<ffff82d04032eeaa>] F start_secondary+0x22e/0x3cf
> >    [<ffff82d040203327>] F __high_start+0x87/0xa0
> > 
> > Pagetable walk from ffff82d0404011ea:
> >  L4[0x105] = 000000006fc2e063 ffffffffffffffff
> >  L3[0x141] = 000000006fc2b063 ffffffffffffffff
> >  L2[0x002] = 000000807c7ca063 ffffffffffffffff
> >  L1[0x001] = 800000006f801121 ffffffffffffffff
> > 
> > ****************************************
> > Panic on CPU 1:
> > FATAL PAGE FAULT
> > [error_code=0003]
> > Faulting linear address: ffff82d0404011ea
> > ****************************************
> 
> Hmm, I specifically looked at that code, but I can see how I screwed up.
> 
> > For the time being revert opt_allow_unsafe to be __read_mostly.
> 
> There's exactly one write that an AP can hit. Is it really worth moving
> back, rather than just doing
> 
>       if (opt_allow_unsafe <= 0 && !cpu_has_amd_erratum(c, AMD_ERRATUM_121))
>               opt_allow_unsafe = 1;
>       else if ...
> 
> ?

I would rather move this logic so it's only the BSP that can set
opt_allow_unsafe, and the APs check they match the configuration set
by the BSP.  I think the resulting logic would be cleaner, but I
didn't want to do such a change as part of this fix.

Thanks, Roger.