Re: [PATCH v2 2/2] x86/dom0: only disable SMAP for the PV dom0 build

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 30/07/2024 4:28 pm, Roger Pau Monne wrote:
> The PVH dom0 builder doesn't switch page tables and has no need to run with
> SMAP disabled.
>
> Use stac() and clac(), as that's safe to use even if events would hit in the
> middle of the region with SMAP disabled.  Nesting stac() and clac() calls is
> not safe, so the stac() call is done strictly after elf_load_binary() because
> that uses raw_{copy_to,clear}_guest() accessors which toggle SMAP.
>
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Summarising a discussion on Matrix.

There are 3 options.

1) Simply reposition the write_cr4()/cr4_pv32_mask adjustments.
2) This form (use stac/clac instead of playing with cr4).
3) Delay the original set_in_cr4(SMAP).

As proved by the confusion thus far, cr4_pv32_mask adjustments are
fragile and can go unnoticed in the general case (need a lucky watchdog
NMI hit to trigger).  Hence I'd prefer to remove the adjustments.

Using stac()/clac() is much easier.  It is fragile because of nesting
(no AC save/restore infrastructure), but any mistake here will be picked
up reliably in Gitlab CI because both adl-* and zen3p-* support SMAP.

Personally I think option 2 is better than 1, hence why I suggested it. 
It's got a fragile corner case but will be spotted reliably.

However, it occurs to me that option 3 exists as well... just delay
setting SMAP until after dom0 is made.  We have form now with only
activating CET-SS on the way out of __start_xen().

Furthermore, option 3 would allow us to move the cr4_pv32_mask
adjustment into set_in_cr4() and never need to opencode it.

(Although this is a bit tricky given the current code layout. 
cr4_pv32_mask also wants to be __ro_after_init and non-existent in a
!PV32 build.)

~Andrew