Re: [PATCH v5 4/7] x86: introduce x86_seg_sys

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 04/09/2024 1:29 pm, Jan Beulich wrote:
> To represent the USER-MSR bitmap access, a new segment type needs
> introducing, behaving like x86_seg_none in terms of address treatment,
> but behaving like a system segment for page walk purposes (implicit
> supervisor-mode access).
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> This feels a little fragile: Of course I did look through uses of the
> enumerators, and I didn't find further places which would need
> adjustment, but I'm not really sure I didn't miss any place.

It does feel a bit fragile, but it may help to consider the other
related cases.

Here, we need a linear access with implicit-supervisor paging
properties.  From what I can tell, it needs to be exactly like other
implicit supervisor accesses.

For CET, we get two new cases.

The legacy bitmap has a pointer out of MSR_[U,S]_CET, but otherwise
obeys CPL rules, so wants to be x86_seg_none.

However, WRUSS is both a CPL0 instruction, and generates implicit-user
accesses.  It's the first instruction of it's like, that I'm aware of. 
If we're going down this x86_seg_sys route, we'd need x86_seg_user too.


Really, this is a consequence of the memory APIs we've got.  It's the
intermediate layers which generate PFEC_* for the pagewalk, and we're
(ab)using segment at the top level to encode "skip segmentation but I
still want certain properties".

But, there's actually a 3rd case we get from CET, and it breaks everything.

Shstk accesses are a new type, architecturally expressed as a new input
(and output) to the pagewalk, but are also regular user-segment relative.

We either do the same trick of expressing fetch() in terms of
read(PFEC_insn) and implement new shstk_{read,write}() accessors which
wrap {read,write}(PFEC_shstk), or we need to plumb the PFEC parameters
higher in the call tree.

It's worth noting that alignment restrictions make things even more
complicated.  Generally, shstk accesses should be 8 or 4 byte aligned
(based on osize), and the pseudocode for WR{U}SS calls this out; after
all they're converting from arbitrary memory operands.

However, there's a fun corner case where a 64bit code segment can use
INCSSPD to misalign SSP, then CALL to generate a misaligned store.  This
combines with an erratum in Zen3 and possibly Zen4 where there's a
missing #GP check on LRET and you can forge a return address formed of
two misaligned addresses.

So misaligned stores are definitely possible (I checked this on both
vendors at the time), so it wouldn't be appropriate to have in a general
shstk_*() helper.  In turn, this means that the implementation of
WR{U}SS would need a way to linearise it's operand manually to insert
the additional check before then making a regular memory access.

And I can't see a way of doing this without exposing PFEC inputs at the
top level.

Thoughts?

~Andrew