Re: Regression with CET: [PATCH v1] x86/mm: avoid inadvertently degrading a TLB flush to local only

[David Vrabel <dvrabel@xxxxxxxxxx>]

On 27/04/2022 19:03, Andrew Cooper wrote:
> On 19/04/2022 16:03, David Vrabel wrote:
> > From: David Vrabel <dvrabel@xxxxxxxxxxxx>
> >
> > If the direct map is incorrectly modified with interrupts disabled,
> > the required TLB flushes are degraded to flushing the local CPU only.
> >
> > This could lead to very hard to diagnose problems as different CPUs will
> > end up with different views of memory. Although, no such issues have yet
> > been identified.
> >
> > Change the check in the flush_area() macro to look at system_state
> > instead. This defers the switch from local to all later in the boot
> > (see xen/arch/x86/setup.c:__start_xen()). This is fine because
> > additional PCPUs are not brought up until after the system state is
> > SYS_STATE_smp_boot.
> >
> > Signed-off-by: David Vrabel <dvrabel@xxxxxxxxxxxx>
> This explodes on CET systems:
>
> (XEN) Assertion 'local_irq_is_enabled()' failed at arch/x86/smp.c:265
> (XEN) ----[ Xen-4.17.0-10.24-d  x86_64  debug=y  Not tainted ]----
> (XEN) CPU:    0
> (XEN) RIP:    e008:[<ffff82d040345300>] flush_area_mask+0x40/0x13e
> <snip>
> (XEN) Xen call trace:
> (XEN)    [<ffff82d040345300>] R flush_area_mask+0x40/0x13e
> (XEN)    [<ffff82d040338a40>] F modify_xen_mappings+0xc5/0x958
> (XEN)    [<ffff82d0404474f9>] F
> arch/x86/alternative.c#_alternative_instructions+0xb7/0xb9
> (XEN)    [<ffff82d0404476cc>] F alternative_branches+0xf/0x12
> (XEN)    [<ffff82d04044e37d>] F __start_xen+0x1ef4/0x2776
> (XEN)    [<ffff82d040203344>] F __high_start+0x94/0xa0
> (XEN)
> (XEN)
> (XEN) ****************************************
> (XEN) Panic on CPU 0:
> (XEN) Assertion 'local_irq_is_enabled()' failed at arch/x86/smp.c:265
> (XEN) ****************************************
> (XEN)
>
> We really did want a local-only flush here, because we specifically
> intended to make self-modifying changes before bringing secondary CPUs up.
I think the transition to SYS_STATE_smp_boot system state should be 
later. i.e., the last point were only 1 PCPU is guaranteed.
David