Re: Kernel Shadow Stacks Versus XFEATURE_MASK_GUEST_SUPERVISOR

[Chao Gao <chao.gao@intel.com>]

>> Can you replace fpu_kernel_cfg.max_features with some fpstate->xyz
>> (sorry too late to think it through :)) that has CET_KERNEL for the
>> guest FPU and not for the regular one?
>
>Sure I can give that a shot.
>
>There's one minor issue in that we need to init dynamic features which
>_aren't_ in fpstate->xfeatures when context switching. There's a little
>note about this in restore_fpregs_from_fpstate().
>
>I think what you're getting at is that we need _something_ to say that
>KVM fpstates should muck with XFEATURE_CET_KERNEL and task fpstates
>should not, even though the task ones have space for XFEATURE_CET_KERNEL.
>
>That's certainly doable.

Sounds like you are talking about something like this, which Sean
suggested in [1]:

diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
index 1015af1ae562..8e7599e3b923 100644
--- a/arch/x86/kernel/fpu/core.c
+++ b/arch/x86/kernel/fpu/core.c
@@ -167,6 +167,16 @@ void restore_fpregs_from_fpstate(struct fpstate *fpstate, u64 mask)
                 */
                xfd_update_state(fpstate);
 
+               /*
+                * Leave supervisor CET state as-is when loading host state
+                * (kernel or userspace).  Supervisor CET state is managed via
+                * XSTATE for KVM guests, but the host never consumes said
+                * state (doesn't support supervisor shadow stacks), i.e. it's
+                * safe to keep guest state loaded into hardware.
+                */
+               if (!fpstate->is_guest)
+                       mask &= ~XFEATURE_MASK_CET_KERNEL;
+

We first looked at enabling CET_KERNEL for all tasks, and that version
included this optimization so that restoring a non-KVM fpstate doesn't
touch CET_KERNEL.

We then switched to the guest-only approach to avoid allocating CET_KERNEL
space in non-KVM fpstates. The optimization could still be applied on top
of that, but it was dropped after its benefit was questioned off-list:

"""
1. It hacks in an optimization with conditionals around the edges,
instead of integrating with the way the rest of the FPU works.
2. There is no proven benefit of this narrow optimization and x86
maintainers are already skeptical.
"""

This optimization can be added back if it benefits kernel shadow stacks.
But os_xrstor_supervisor() and os_xrstor_safe() would need the same
handling.

[1]: https://lore.kernel.org/all/ZM1jV3UPL0AMpVDI@google.com/