On 9/15/2023 1:40 AM, Dave Hansen wrote:
On 9/13/23 23:33, Yang Weijiang wrote:
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1636,9 +1636,17 @@ static int __xstate_request_perm(u64 permitted, u64 requested, bool guest)
/* Calculate the resulting kernel state size */
mask = permitted | requested;
- /* Take supervisor states into account on the host */
+ /*
+ * Take supervisor states into account on the host. And add
+ * kernel dynamic xfeatures to guest since guest kernel may
+ * enable corresponding CPU feaures and the xstate registers
+ * need to be saved/restored properly.
+ */
if (!guest)
mask |= xfeatures_mask_supervisor();
+ else
+ mask |= fpu_kernel_dynamic_xfeatures;
+
ksize = xstate_calculate_size(mask, compacted);
Heh, you changed the "guest" naming in "fpu_kernel_dynamic_xfeatures"
but didn't change the logic.
As it's coded at the moment *ALL* "fpu_kernel_dynamic_xfeatures" are
guest xfeatures. So, they're different in name only.
If you want to change the rules for guests, we have *ONE* place that's
done: fpstate_reset(). It establishes the permissions and the sizes for
the default guest FPU. Start there. If you want to make the guest
defaults include XFEATURE_CET_USER, then you need to put the bit in *there*.
Yeah, fpstate_reset() is the right place to hold the guest init permits and propagate
them here, thanks for the suggestion!
Nit, did you actually mean XFEATURE_CET_KERNEL instead of XFEATURE_CET_USER above?
because the latter is already supported by upstream kernel.
The other option is to have the KVM code actually go and "request" that
the dynamic states get added to 'fpu->guest_perm'.
Yes, compared with above option, it will change current userspace handling logic, i.e.,
only user xstates are dynamically requested. So I'd try above option first.
Would there ever be
any reason for KVM to be on a system which supports a dynamic kernel
feature but where it doesn't get enabled for guest use, or at least
shouldn't have the FPU space allocated?
I haven't heard of that kind of usage for other features so far, CET supervisor xstate is the
only dynamic kernel feature now, not sure whether other CPU features having supervisor
xstate would share the handling logic like CET does one day.
