x86: xsave/xrstor support, ucontext_t extensions

hi,

Appended patch adds the support for xsave/xrstor infrastructure for x86. xsave/xrstor manages the existing and future processor extended states in x86 architecutre.

More info on xsave/xrstor can be found in the Intel SDM's located at
http://www.intel.com/products/processor/manuals/index.htm

Please let me know your feedback and comments. Specifically, I am not sure if I break anything or make anyone's life harder with the ucontext_t extensions that are proposed in the patch.

BTW, Traditionally glibc has this definition for struct ucontext.

include/asm-x86/ucontext.h +0 -0

@@ -6,7 +6,10 @@
 	struct ucontext  *uc_link;

include/asm-x86/sigcontext.h +13 -0

@@ -202,4 +202,26 @@
 
 #endif /* !__i386__ */
 
+struct _xsave_hdr_struct {
+ 	u64 xstate_bv;
+ 	u64 reserved1[2];
+ 	u64 reserved2[5];
+} __attribute__((packed));
+
+struct _xstate {
+	/*
+	 * Applications need to refer to fpstate through fpstate pointer
+	 * in sigcontext. Not here directly.
+	 */
+ 	struct _fpstate fpstate;
+ 	struct _xsave_hdr_struct xsave_hdr;

arch/x86/kernel/signal_64.c +0 -0

arch/x86/kernel/signal_32.c +0 -0

arch/x86/kernel/signal_32.c +2 -1

-get_sigframe(struct k_sigaction *ka, struct pt_regs *regs, size_t frame_size)
+get_sigframe(struct k_sigaction *ka, struct pt_regs *regs, size_t frame_size,
+	     struct _fpstate **fpstate, struct xsave_struct **xstate)

arch/x86/kernel/xsave.c new file +0 -0

This is a large patch, and somewhat difficult to review since it mixes kernel-private and user-visible changes.

I'm going to focus on the user-visible changes.

Quoted 1 line

BTW, Traditionally glibc has this definition for struct ucontext.

glibc's definition is irrelevant, in part because glibc can and does lie about kernel types to applications, and in part because glibc is not the only user-space consumer of kernel types: there are other libcs, and there are user-space virtualisation tools (my interest in this matter) that care deeply about kernel types and sigframe layouts. In particular, user-space needs to be able to copy and assemble sigframes.

So however the xsave support ends up looking, user-space must have a sensible way of detecting and handling the layout changes.

Quoted 13 lines

include/asm-x86/ucontext.h +4 -1

@@ -6,7 +6,10 @@
 	struct ucontext  *uc_link;
 	stack_t		  uc_stack;
 	struct sigcontext uc_mcontext;
-	sigset_t	  uc_sigmask;	/* mask last for extensibility */
+	sigset_t	  uc_sigmask;
+	/* Allow for uc_sigmask growth.  Glibc uses a 1024-bit sigset_t.  */
+	int		  __unused[32 - (sizeof (sigset_t) / sizeof (int))];
+	struct xstate_cntxt  uc_xstate;
 };
 

include/asm-x86/ucontext.h line 15

Quoted 1 line

include/asm-x86/ucontext.h +0 -0

 #endif /* _ASM_X86_UCONTEXT_H */

You're changing the layout of struct ucontext in two ways: uc_mcontext changes elsewhere, and you're adding __unused and uc_xstate.

How is user-space supposed to know whether it's looking at a current layout ucontext or an xsave-layout ucontext?

It seems that uc_flags is unused and always zero. Could you define a flag bit (e.g. 1) for uc_flags to indicate the xsave layout?

Quoted 6 lines

arch/x86/kernel/sigframe.h +0 -0

@@ -3,9 +3,10 @@
 	char __user *pretcode;
 	int sig;
 	struct sigcontext sc;

arch/x86/kernel/sigframe.h line 10

Quoted 6 lines

arch/x86/kernel/sigframe.h +2 -1

-	struct _fpstate fpstate;
+	struct xstate_cntxt xst_cnxt;
 	unsigned long extramask[_NSIG_WORDS-1];
 	char retcode[8];
+	/* fp and rest of the extended context state follows here */
 };

Offset to extramask[] and retcode changes, as well as the size of the structure.

Why contract xstate_cntxt to xst_cnxt? That just obscures things.

Quoted 26 lines

include/asm-x86/sigcontext.h +19 -0

@@ -202,4 +202,26 @@
 
 #endif /* !__i386__ */
 
+struct _xsave_hdr_struct {
+ 	u64 xstate_bv;
+ 	u64 reserved1[2];
+ 	u64 reserved2[5];
+} __attribute__((packed));
+
+struct _xstate {
+	/*
+	 * Applications need to refer to fpstate through fpstate pointer
+	 * in sigcontext. Not here directly.
+	 */
+ 	struct _fpstate fpstate;
+ 	struct _xsave_hdr_struct xsave_hdr;
+ 	/* new processor state extensions will go here */
+} __attribute__ ((aligned (64)));
+
+struct xstate_cntxt {
+	struct  _xstate __user *xstate;
+	u32	size;

include/asm-x86/sigcontext.h line 227

Quoted 4 lines

include/asm-x86/sigcontext.h +3 -0

+	u32 	lmask;
+	u32	hmask;
+};
 #endif

What is the purpose of the xstate pointer in xstate_cntxt? As far as I can tell, it's redundant and can alway be derived from the ucontext's uc_mcontext.fpstate pointer.

And why does x86-64 make xstate == fpstate while on x86-32 they're at an offset from each other?

The fpstate pointer may be pointing into a struct _xstate. How is user-space supposed to know if this is the case?

struct _fpstate has a 'magic' field which distinguishes x87-only from x87+FXSR structs. Could that field also be used to indicate XSAVE?

How is user-space supposed to know how large the _xstate struct is? Is that the size field in the struct xstate_cntxt?

Quoted 3 lines

arch/x86/kernel/xsave.c new file +0 -0

...

arch/x86/kernel/xsave.c line 179

Quoted 5 lines

arch/x86/kernel/xsave.c new file +5 -0

+	/*
+	 * FP and SSE state can't be restored directly from the userspace
+	 * because of legacy reasons. Lets restore it to the fpstate
+	 * in the task struct.
+	 */

Can you please explain what those 'legacy reasons' are?

I know xsave will be needed once AVX is released to the masses. Any idea on when that will happen?

Quoted 2 lines · Mikael Pettersson

This is a large patch, and somewhat difficult to review since it mixes kernel-private and user-visible changes.

Sorry. I wanted to give the complete picture. Will see if I can split it up while posting for upstream.

Quoted 8 lines

Quoted 1 line

BTW, Traditionally glibc has this definition for struct ucontext.

glibc's definition is irrelevant, in part because glibc can and does lie about kernel types to applications, and in part because glibc is not the only user-space consumer of kernel types: there are other libcs, and there are user-space virtualisation tools (my interest in this matter) that care deeply about kernel types and sigframe layouts. In particular, user-space needs to be able to copy and assemble sigframes.

Ok. I hope user-space is doing this copy and assemble in a (sane) way that allows the kernel to modify the sigframe with out breaking old apps.

Do you know how the user-space is determining how much to copy today?

I have to probably use some magic values(or other flags), indicating the presence of extended state context information in the ucontext.

cpuid information (cpuid.0x1.ecx.osxsave) indicates whether the OS supports xsave or not. This can be one way, that signal handlers can use to interpret the ucontext.

Given that it is more than signal handlers, I can add a flag also, representing ucontext extensions.

Quoted 2 lines

You're changing the layout of struct ucontext in two ways: uc_mcontext changes elsewhere, and you're adding __unused and uc_xstate.

I am not changing the uc_mcontext (struct sigcontext). Just extending the ucontext.

Quoted 5 lines

How is user-space supposed to know whether it's looking at a current layout ucontext or an xsave-layout ucontext?

It seems that uc_flags is unused and always zero. Could you define a flag bit (e.g. 1) for uc_flags to indicate the xsave layout?

Sure. I can even add a magic word to indicate the xsave presence, so that sigreturn can be double sure and not break older apps.

Quoted 13 lines

Quoted 12 lines

arch/x86/kernel/sigframe.h +2 -1

@@ -3,9 +3,10 @@
 	char __user *pretcode;
 	int sig;
 	struct sigcontext sc;
-	struct _fpstate fpstate;
+	struct xstate_cntxt xst_cnxt;
 	unsigned long extramask[_NSIG_WORDS-1];
 	char retcode[8];
+	/* fp and rest of the extended context state follows here */
 };

Quoted 1 line

Offset to extramask[] and retcode changes, as well as the size of the structure.

hm yes. this will break the restore of extramask[], for apps which set their own sig return frames.

I can leave _fpstate as it is(and unused) and move xstate context (which will contain fpstate + the extended state) after extramask[].

Quoted 1 line

Why contract xstate_cntxt to xst_cnxt? That just obscures things.

ok.

Quoted 3 lines

What is the purpose of the xstate pointer in xstate_cntxt? As far as I can tell, it's redundant and can alway be derived from the ucontext's uc_mcontext.fpstate pointer.

This is true in 64bit.

While doing sigreturn, there is no way, kernel can know if the uc_mcontext.fpstate is pointing to just the legacy fpstate(512 bytes) or the extended state pointer (unless we incorporate
some magic word at the end of fpstate image, see below)

during sigreturn, fpstate will be restored from the uc_mcontext.fpstate and extended state will be restored from xstate pointer.

Quoted 2 lines

And why does x86-64 make xstate == fpstate while on x86-32 they're at an offset from each other?

because sigcontext's 32bit fpstate is different from 64bit fpstate. 32bit fp state is fsave frame followed by fxsave frame. Whereas 64bit fp state is just fxsave frame. To maintain 32bit legacy compatibility, 32bit xstate is different from 32bit fpstate.

Quoted 2 lines

struct _fpstate has a 'magic' field which distinguishes x87-only from x87+FXSR structs. Could that field also be used to indicate XSAVE?

I don't think we can use the existing 'magic' field.

But we can
use some what similar magic, if the fxsave/fxrstor give away
some of the fields at the end of fxsave image (today it is reserved and ignored during fxsave/fxrstor) for software use.
We can then use these fields at the end of fpstate, to indicate the presence of xstate. But this requires some architecture changes like giving away this space for SW use. We can take this to architects and
see what they think.

The de-facto ABI for signal delivery and sigreturn is unfortunately based on fairly fixed-layout structs on the stack (sigframes). The only flexibility there that I've found is the sigcontext's fpstate pointer which allows the fpstate to be located elsewhere.

Another idea is that marking non-siginfo sigframes could be done by adding a sys_xsave_sigreturn() and letting those sigframes have a return address pointing to a vsyscall that invokes this new syscall.

My problem with the OSXAVE flag is that it's a very indirect way of communicating the layout of sigframes and sigcontexts. These structures should, if at all possible, be self-describing. A single flag bit in the sigcontext could handle both structures (since a sigframe always includes a sigcontext).

Quoted 4 lines

Quoted 3 lines

struct _fpstate has a 'magic' field which distinguishes x87-only from x87+FXSR structs. Could that field also be used to indicate XSAVE?

I don't think we can use the existing 'magic' field.

Hmm, right now it seems this field has a de-facto ABI of being either 0xffff (plain) or 0x0000 (fxsr). Using other values would confuse at least one application I know of. Sad.

Quoted 8 lines

But we can
use some what similar magic, if the fxsave/fxrstor give away
some of the fields at the end of fxsave image (today it is reserved and ignored during fxsave/fxrstor) for software use.
We can then use these fields at the end of fpstate, to indicate the presence of xstate. But this requires some architecture changes like giving away this space for SW use. We can take this to architects and
see what they think.

If the HW doesn't store anything valuable there, we could store SW flags/cookies there on signal delivery, and clear them before fxrstor (unless the HW is known to ignore those fields). But it depends on how forgiving the HW is.

Quoted 7 lines · Mikael Pettersson

My problem with the OSXAVE flag is that it's a very indirect way of communicating the layout of sigframes and sigcontexts. These structures should, if at all possible, be self-describing. A single flag bit in the sigcontext could handle both structures (since a sigframe always includes a sigcontext).

It's also wrong, since OSXSAVE indicates that the CPU can do it, not that the kernel can.

Quoted 4 lines

Quoted 3 lines

Quoted 2 lines

struct _fpstate has a 'magic' field which distinguishes x87-only from x87+FXSR structs. Could that field also be used to indicate XSAVE?

I don't think we can use the existing 'magic' field.

Quoted 4 lines

Hmm, right now it seems this field has a de-facto ABI of being either 0xffff (plain) or 0x0000 (fxsr). Using other values would confuse at least one application I know of. Sad.

Well, arguably it is the right thing to use since we're talking about a new format. The difference is that the new format *does* extend backwards to match the old format.

Quoted 10 lines

Quoted 9 lines

But we can
use some what similar magic, if the fxsave/fxrstor give away some of the fields at the end of fxsave image (today it is reserved and ignored during fxsave/fxrstor) for software use.
We can then use these fields at the end of fpstate, to indicate the presence of
xstate. But this requires some architecture changes like giving away this space for SW use. We can take this to architects and see what they think.

Quoted 4 lines

If the HW doesn't store anything valuable there, we could store SW flags/cookies there on signal delivery, and clear them before fxrstor (unless the HW is known to ignore those fields). But it depends on how forgiving the HW is.

All we need is a single field -- a single byte -- reserved indefinitely for software use. Existing FXSAVE kernels will have set it to zero.

There might be fields the existing FXSAVE format which can be equally abused, even. I will do some looking.

Quoted 2 lines

Quoted 1 line

It's also wrong, since OSXSAVE indicates that the CPU can do it, not that the kernel can.

OSXSAVE indicates the OS support and XSAVE indicates the cpu support.

Quoted 3 lines

All we need is a single field -- a single byte -- reserved indefinitely for software use. Existing FXSAVE kernels will have set it to zero.

There might be fields the existing FXSAVE format which can be equally abused, even. I will do some looking.

All the reserved fields at the end of fxsave format are zeroed and presented as such to the user. If HW makes some of these fields SW available, then we can use those (will check). If there is any scope with the existing format it self, that will be much better.

Quoted 8 lines

But we can
use some what similar magic, if the fxsave/fxrstor give away
some of the fields at the end of fxsave image (today it is reserved and ignored during fxsave/fxrstor) for software use.
We can then use these fields at the end of fpstate, to indicate the presence of xstate. But this requires some architecture changes like giving away this space for SW use. We can take this to architects and
see what they think.

If the HW doesn't store anything valuable there, we could store SW flags/cookies there on signal delivery, and clear them before fxrstor (unless the HW is known to ignore those fields). But it depends on how forgiving the HW is.

Quoted 9 lines · Mikael Pettersson

Quoted 8 lines

But we can
use some what similar magic, if the fxsave/fxrstor give away
some of the fields at the end of fxsave image (today it is reserved and ignored during fxsave/fxrstor) for software use.
We can then use these fields at the end of fpstate, to indicate the presence of xstate. But this requires some architecture changes like giving away this space for SW use. We can take this to architects and
see what they think.

Quoted 4 lines

If the HW doesn't store anything valuable there, we could store SW flags/cookies there on signal delivery, and clear them before fxrstor (unless the HW is known to ignore those fields). But it depends on how forgiving the HW is.

Ok. CPU folks are planning to make some of the bytes at the end of fxsave image, SW usable.

We can use some of these fields, to represent the extended state presence with a cookie, save area size, mask of the state stored. If needed, we can include the start address of the fpstate pointer (also as part of the cookie), so that we can detect the situation, where apps are just memcopying sizeof(struct _fpstate) from the fpstate pointer (but not aware of the extended state).

we don't need any ucontext_t extensions any more and just use the fpstate pointer to indicate the extended state aswell, right?

In addition, we need to make sure that for 32bit non-rt sigframes, we don't modify the extramask[] offset.

Quoted 8 lines · Mikael Pettersson

Quoted 8 lines

But we can
use some what similar magic, if the fxsave/fxrstor give away
some of the fields at the end of fxsave image (today it is reserved and ignored during fxsave/fxrstor) for software use.
We can then use these fields at the end of fpstate, to indicate the presence of xstate. But this requires some architecture changes like giving away this space for SW use. We can take this to architects and
see what they think.

Quoted 4 lines

If the HW doesn't store anything valuable there, we could store SW flags/cookies there on signal delivery, and clear them before fxrstor (unless the HW is known to ignore those fields). But it depends on how forgiving the HW is.

Ok. CPU folks are planning to make some of the bytes at the end of fxsave image, SW usable.

Quoted 14 lines · Suresh Siddha

Quoted 12 lines · Mikael Pettersson

Quoted 8 lines

But we can
use some what similar magic, if the fxsave/fxrstor give away
some of the fields at the end of fxsave image (today it is reserved and ignored during fxsave/fxrstor) for software use.
We can then use these fields at the end of fpstate, to indicate the presence of xstate. But this requires some architecture changes like giving away this space for SW use. We can take this to architects and
see what they think.

If the HW doesn't store anything valuable there, we could store SW flags/cookies there on signal delivery, and clear them before fxrstor (unless the HW is known to ignore those fields). But it depends on how forgiving the HW is.

Quoted 2 lines

Ok. CPU folks are planning to make some of the bytes at the end of fxsave image, SW usable.

Are they always zeroed in earlier CPUs though? If not that wouldn't work 100% reliably because whatever cookie you put in could have been there before by chance.

Quoted 4 lines

Are they always zeroed in earlier CPUs though? If not that wouldn't work 100% reliably because whatever cookie you put in could have been there before by chance.

I wrote a test program (fill an area with zeroes, fxsave, inspect reserved fields, then fill it with ones, fxsave, inspect again), and all processors appear to just not write anything to the reserved fields after the last xmm register. (Tested on an old Mobile Athlon64, Opteron 280, P4 Xeon, Pentium-D, and C2 Xeon E5345.)

So the question now is what if anything has the Linux kernel written to those reserved fields. (Looking..) Hmm, signal delivery on x86-64 seems to do fxsave directly to the fxsave area in the user's sigframe, which would imply that the reserved fields have unpredictable values.

Quoted 11 lines · Mikael Pettersson

Quoted 4 lines

Are they always zeroed in earlier CPUs though? If not that wouldn't work 100% reliably because whatever cookie you put in could have been there before by chance.

I wrote a test program (fill an area with zeroes, fxsave, inspect reserved fields, then fill it with ones, fxsave, inspect again), and all processors appear to just not write anything to the reserved fields after the last xmm register. (Tested on an old Mobile Athlon64, Opteron 280, P4 Xeon, Pentium-D, and C2 Xeon E5345.)

Quoted 5 lines

So the question now is what if anything has the Linux kernel written to those reserved fields. (Looking..) Hmm, signal delivery on x86-64 seems to do fxsave directly to the fxsave area in the user's sigframe, which would imply that the reserved fields have unpredictable values.

OK, so that's not a usable path unless we can find some area in the existing data set to put a flag. Groan.

Quoted 9 lines · H. Peter Anvin

Quoted 5 lines · Mikael Pettersson

So the question now is what if anything has the Linux kernel written to those reserved fields. (Looking..) Hmm, signal delivery on x86-64 seems to do fxsave directly to the fxsave area in the user's sigframe, which would imply that the reserved fields have unpredictable values.

OK, so that's not a usable path unless we can find some area in the existing data set to put a flag. Groan.

An ugly workaround could be to start clearing one of these fields, and say that the data there is only valid for kernels >= 2.6.26. (I said it was ugly...)

Or we go back to stashing a flag in uc_flags (which is kosher), and try to figure out how to mark non-rt sigframes.

This issue of not-zeroing, is present in only 64bit kernels and for 64bit apps, right?

64bit app signal handling uses only rt_frame, so we can add an uc_flag for them and for 32bit apps, kernel was always zero'ing the reserved bits at the end of _fpstate.

In short, for non-rt frames, they can check the reserved bits at the end of fpstate frame and for rt-frames (perhaps even for 32bit rt frame handling) apps can check for uc_flag aswell, for extended state presence. Is this good enough?

Quoted 7 lines

An ugly workaround could be to start clearing one of these fields, and say that the data there is only valid for kernels >= 2.6.26. (I said it was ugly...)

Or we go back to stashing a flag in uc_flags (which is kosher), and try to figure out how to mark non-rt sigframes.

This issue of not-zeroing, is present in only 64bit kernels and for 64bit apps, right?

64bit app signal handling uses only rt_frame, so we can add an uc_flag for them and for 32bit apps, kernel was always zero'ing the reserved bits at the end of _fpstate.

In short, for non-rt frames, they can check the reserved bits at the end of fpstate frame and for rt-frames (perhaps even for 32bit rt frame handling) apps can check for uc_flag aswell, for extended state presence. Is this good enough?

The biggest potential problem with this that I see is that relying on CPUID can mess with certain virtualization solutions. Another option to accomplish the same thing would be to have a system call (preferrably a prctl, since it is at least in theory personality-dependent) to query what information is included in the fpstate data - since it will always be the same for any particular kernel.

Quoted 1 line

The biggest potential problem with this that I see is that relying on CPUID can mess with certain virtualization solutions. Another option to accomplish the same thing would be to have a system call (preferrably a prctl, since it is at least in theory personality-dependent) to query what information is included in the fpstate data - since it will always be the same for any particular kernel.

It's easy and cheap to add an indicator to the vDSO for this.

Yes, but I suspect for legacy apps running without vDSO might matter.

Quoted 1 line

Yes, but I suspect for legacy apps running without vDSO might matter.

"Legacy" apps want to be changed to make a new kind of kernel query and care about new details of signal frame layout for new features they never used before, but don't want to handle the vDSO?

Quoted 6 lines

The biggest potential problem with this that I see is that relying on CPUID can mess with certain virtualization solutions. Another option to accomplish the same thing would be to have a system call (preferrably a prctl, since it is at least in theory personality-dependent) to query what information is included in the fpstate data - since it will always be the same for any particular kernel.

It's easy and cheap to add an indicator to the vDSO for this.

Quoted 11 lines · Suresh Siddha

Quoted 7 lines

An ugly workaround could be to start clearing one of these fields, and say that the data there is only valid for kernels >= 2.6.26. (I said it was ugly...)

Or we go back to stashing a flag in uc_flags (which is kosher), and try to figure out how to mark non-rt sigframes.

This issue of not-zeroing, is present in only 64bit kernels and for 64bit apps, right?

Quoted 9 lines

64bit app signal handling uses only rt_frame, so we can add an uc_flag for them and for 32bit apps, kernel was always zero'ing the reserved bits at the end of _fpstate.

In short, for non-rt frames, they can check the reserved bits at the end of fpstate frame and for rt-frames (perhaps even for 32bit rt frame handling) apps can check for uc_flag aswell, for extended state presence. Is this good enough?

Okay, trying to close on this :)

I would suggest using the uc_flag for the rt frames, and simply rely on the OSXSAVE flag for non-rt signal frames. It a rather sucky approach (as previously discussed), but since any sane user of these fields (as opposed to just relying on the kernel to save/restore) should use the SIGINFO frames, I don't see a problem *as long as it's possible to get the information* -- any solution which demands performance should just turn on SIGINFO and be happy.

The biggest potential problem with this that I see is that relying on CPUID can mess with certain virtualization solutions. Another option to accomplish the same thing would be to have a system call (preferrably a prctl, since it is at least in theory personality-dependent) to query what information is included in the fpstate data - since it will always be the same for any particular kernel.

Thoughts?

Quoted 9 lines · H. Peter Anvin

Quoted 5 lines · Suresh Siddha

In short, for non-rt frames, they can check the reserved bits at the end of fpstate frame and for rt-frames (perhaps even for 32bit rt frame handling) apps can check for uc_flag aswell, for extended state presence. Is this good enough?

Okay, trying to close on this :)

Quoted 7 lines

I would suggest using the uc_flag for the rt frames, and simply rely on the OSXSAVE flag for non-rt signal frames. It a rather sucky approach (as previously discussed), but since any sane user of these fields (as opposed to just relying on the kernel to save/restore) should use the SIGINFO frames, I don't see a problem *as long as it's possible to get the information* -- any solution which demands performance should just turn on SIGINFO and be happy.

I don't have the luxury to unconditionally change non-rt signal delivery to rt signal delivery, but using uc_flags plus OSXSAVE or prctl() to announce these layout changes would work for us. Of course, any existing sigframe mangling (as opposed to just reading it) code must be updated to avoid breakage, but that's unavoidable.

Quoted 2 lines

The biggest potential problem with this that I see is that relying on CPUID can mess with certain virtualization solutions. Another option to

Quoted 6 lines

accomplish the same thing would be to have a system call (preferrably a prctl, since it is at least in theory personality-dependent) to query what information is included in the fpstate data - since it will always be the same for any particular kernel.

Thoughts?

Ok for me.

/Mikael

hpa, What is the virtualization problem? Are you referring to perf problem? As you noted, regular non-rt signal handlers won't need this cpuid check. It's needed only for those who manually look at non-rt signal frames and interpret it. And also, they can do this check only once and not everytime.

To me, prtcl() just seems to be an overkill.

While restoring from the user, kernel also need to find out what layout the user is passing. So it's bi-directional. I prefer the same mechanism (using cookies/magic numbers etc inaddition to uc_flags or cpuid checks) to interpret the fpstate for both user/kernel.

ARM also seem to be using similar things while extending their ucontext_t, with out other kernel interfaces to indicate the layout.

BTW, how come 32bit kernel doesn't have the X86_FXSR_MAGIC checks, while restoring the extended fxsave data from _fpstate?

Quoted 8 lines · Suresh Siddha

hpa, What is the virtualization problem? Are you referring to perf problem? As you noted, regular non-rt signal handlers won't need this cpuid check. It's
needed only for those who manually look at non-rt signal frames and interpret it.
And also, they can do this check only once and not everytime.

No, relying on CPUID and vdso both have implications for virtualization.

I don't think it is ... it's not overkill but rather "underkill"... it's a low-performance solution but it's guaranteed to be safe in the presence of virtualization of all its various ilk. Note that you don't need to be able to *set* the format via prctl(), just *query* (get) it.

Using prctl() allows us to make this personality-dependent if we ever need to.

Quoted 4 lines

While restoring from the user, kernel also need to find out what layout the user is passing. So it's bi-directional. I prefer the same mechanism (using cookies/magic numbers etc inaddition to uc_flags or cpuid checks) to interpret the fpstate for both user/kernel.

No, it really doesn't: the kernel only needs to be able to read the same format as it itself wrote.

Quoted 7 lines

ARM also seem to be using similar things while extending their ucontext_t, with out other kernel interfaces to indicate the layout.

BTW, how come 32bit kernel doesn't have the X86_FXSR_MAGIC checks, while restoring
the extended fxsave data from _fpstate?

Again, the kernel already knows the format, so it doesn't need to check.

can you please elaborate? even in presence of virtualization, appropriate cpuid bits need to be set/visible for application, for xsave/xrstor to work properly.

Quoted 9 lines

I don't think it is ... it's not overkill but rather "underkill"... it's a low-performance solution but it's guaranteed to be safe in the presence of virtualization of all its various ilk. Note that you don't need to be able to *set* the format via prctl(), just *query* (get) it.

Using prctl() allows us to make this personality-dependent if we ever need to.

Quoted 4 lines

While restoring from the user, kernel also need to find out what layout the user is passing. So it's bi-directional. I prefer the same mechanism (using cookies/magic numbers etc inaddition to uc_flags or cpuid checks) to interpret the fpstate for both user/kernel.

No, it really doesn't: the kernel only needs to be able to read the same format as it itself wrote.

But user can potentially change the _fpstate pointer in sigcontext struct.

Quoted 6 lines

Quoted 5 lines

ARM also seem to be using similar things while extending their ucontext_t, with out other kernel interfaces to indicate the layout.

BTW, how come 32bit kernel doesn't have the X86_FXSR_MAGIC checks, while restoring the extended fxsave data from _fpstate?

Again, the kernel already knows the format, so it doesn't need to check.

What happens with some old applications which change the _fpstate pointer. they may or may not be fxsave aware...

Quoted 1 line

But user can potentially change the _fpstate pointer in sigcontext struct.

If so, they get what they bargained for... I am not even sure the kernel will successfully clean up the stack frame if they do that.

Quoted 5 lines · Suresh Siddha

can you please elaborate? even in presence of virtualization, appropriate cpuid bits need to be set/visible for application, for xsave/xrstor to work properly.

For many paravirtualization solutions, CPUID "leak" from the hypervisor.
The fact that CPUID cannot be disabled (made ring 0 only) is a major
flaw in the architecture.

Therefore, relying on CPUID is too dangerous.

hmm.. so the kernel needs to export all the cpuid info (that the kernel enables and supports) to the user through some mechanism then?

Quoted 2 lines

hmm.. so the kernel needs to export all the cpuid info (that the kernel enables and supports) to the user through some mechanism then?

For a cheap interface, we use AT_HWCAP for this. Unfortunately that only covers the first 32 bits of CPUID info. (For another cheap interface, giving all the CPUID bits in the vDSO would be easy.)

For a clunky interface that already exists and is "simple" to use, there is /dev/cpu/0/cpuid now. I wonder if having a device node and opening it too much for applications that consider the vDSO too complex.

Quoted 7 lines · Roland McGrath

Quoted 2 lines

hmm.. so the kernel needs to export all the cpuid info (that the kernel enables and supports) to the user through some mechanism then?

For a cheap interface, we use AT_HWCAP for this. Unfortunately that only covers the first 32 bits of CPUID info. (For another cheap interface, giving all the CPUID bits in the vDSO would be easy.)

Quoted 3 lines

For a clunky interface that already exists and is "simple" to use, there is /dev/cpu/0/cpuid now. I wonder if having a device node and opening it too much for applications that consider the vDSO too complex.

I doubt it.

Sorry for not getting back on this for so long.

I have looked at the XSAVE architecture, and it is pretty darn hideous, mostly because it doesn't describe itself in the absence of CPUID information.

Given that, it would have been much better if there had been separate invocations of XSAVE for each substate region.

On the
other hand, normalizing to the current CPU format is probably desirable anyway.

I would like to make this proposal for the signal frames (again, flagged with a uc_flags bit for RT frames):

- The SW-reserved areas at the bottom of the FXSAVE region will be used as follows:
- A magic number (M1)
- A length pointer (L1), giving the length of the entire XSAVE region.

- At the end of the XSAVE region, i.e. at the offset given by the length pointer, we create a secondary structure looking something like this:

- Magic number (M2)

- Descriptor count (DC)
- DC * <EBX, EAX> from CPUID leaf 0Dh

- Possibly a checksum or CRC of this structure

Note that this tail structure will always be the same on a given kernel, so it can be pre-canned at boot time. This tail structure serves two purposes:

- It can be used to verify against truncation of the state. (I.e. if an XSAVE-unaware application tries to copy and save away a state and later restore it, but only copies the first 512 bytes and later just puts a pointer to it.)

- It can be used to verify against an alien state (saved and restored from another CPU, or even just another kernel version with different support.)

If there is a mismatch, we can then take appropriate action:

- No M1 or M2 signature, or L1 or DC are insane:

    -> Reinitialize any non-FXSAVE state.

- M1, M2, L1, and DC make sense, but mismatch on DC or descriptor offsets:
-> Do a region-by-region copy in of the state; reinitialize any
regions not provided.

- Mismatch on descriptor sizes:

    -> Consider that region corrupt and reinitialize?

The region-by-region copy could of course be used even in the same-CPU case, if there turns out to be a negible performance difference over whole-block copy.

I thought we had a closure on the previous thread. But no problem. It's better late than never.

xsave,xrstor are performance senstive instructions as they are used in process context switches. It doesn't have to describe itself and at any time, one can get all the xsave relevant layout information using cpuid. And when needed, SW can always pass extra information with the xsave image.

Quoted 1 line

- Magic number (M2)

As I mentioned earlier, we can avoid this magic number, by including a pointer (which points to start of the fp and xstate on stack) along with M1.

This will catch any one copying the FP state of the frame but not aware of Xstate.

Quoted 2 lines

- Descriptor count (DC)
- DC * <EBX, EAX> from CPUID leaf 0Dh

As you mentioned, this doesn't change after a kernel boot. So do we really need to save this static information on every signal? (also please see below about the compaction).

Quoted 8 lines

- Possibly a checksum or CRC of this structure

Note that this tail structure will always be the same on a given kernel, so it can be pre-canned at boot time. This tail structure serves two purposes:

- It can be used to verify against truncation of the state. (I.e. if an XSAVE-unaware application tries to copy and save away a state and later restore it, but only copies the first 512 bytes and later just puts a pointer to it.)

As I mentioned above, pointer along with M1 should be enough to catch this?

Quoted 3 lines

- It can be used to verify against an alien state (saved and restored from another CPU, or even just another kernel version with different support.)

Though the xsave layout is extendable, save area is not compacted if some features are not supported by processor and/or system software. This is documented in Vol 2b under "xsave" instruction.

Given that the descriptor offsets don't change, we can achieve the same thing with a bit mask representing the state in the xsave layout. xrstor with the approriate bit masks will automatically restore/init the state.

Quoted 4 lines

- Mismatch on descriptor sizes:

    -> Consider that region corrupt and reinitialize?

The region-by-region copy could of course be used even in the same-CPU case, if there turns out to be a negible performance difference over whole-block copy.

Today in 64bit, we directly do fxsave/fxrstor in and out of user-space for signal handlers. I would like to retain this behavior as much as possible with xsave/xrstor aswell (and at the same time, provide as much information as possible for the user to interpret the signal frame). Bit mask representing the state saved in the xsave image, M1, length and some cookie (pointer along with M1) to detect the image truncation can achieve this. Isn't it?

Quoted 5 lines · H. Peter Anvin

Quoted 4 lines · Suresh Siddha

can you please elaborate? even in presence of virtualization, appropriate cpuid bits need to be set/visible for application, for xsave/xrstor to work properly.

Quoted 5 lines

For many paravirtualization solutions, CPUID "leak" from the hypervisor.
The fact that CPUID cannot be disabled (made ring 0 only) is a major
flaw in the architecture.

Therefore, relying on CPUID is too dangerous.

hmm.. so the kernel needs to export all the cpuid info (that the kernel enables and supports) to the user through some mechanism then?

atleast in the xsave case, hypervisor can completely control the OSXSAVE flag. I am still not convinced whether I need to add prctl() to indicate the layout. If I have to add, then it should not just
be about whether xsave information is included in _fpstate or not, it should also be about the whole cpuid information provided by the xsave architecture. Because the user potentially needs all that information, to make sense out of the data layout included in the extended state area.

Hmm, right now it seems this field has a de-facto ABI of being either 0xffff (plain) or 0x0000 (fxsr). Using other values would confuse at least one application I know of. Sad.

Quoted 3 lines · you

Hmm, right now it seems this field has a de-facto ABI of being either 0xffff (plain) or 0x0000 (fxsr). Using other values would confuse at least one application I know of. Sad.

... could you share what application that is? This otherwise would be ideal.

struct _fpstate {
	...
       unsigned short  magic;          /* 0xffff = regular FPU data only */
	...
};

#define X86_FXSR_MAGIC 0x0000

Quoted 6 lines · H. Peter Anvin

Quoted 3 lines · you

Hmm, right now it seems this field has a de-facto ABI of being either 0xffff (plain) or 0x0000 (fxsr). Using other values would confuse at least one application I know of. Sad.

Quoted 2 lines

... could you share what application that is? This otherwise would be ideal.

It's the virtual machine for the Erlang/OTP concurrent functional language. It's open-source and used extensively for telecom and internet server applications.

Quoted 6 lines · Suresh Siddha

hpa, What is the virtualization problem? Are you referring to perf problem? As you noted, regular non-rt signal handlers won't need this cpuid check. It's needed only for those who manually look at non-rt signal frames and interpret it. And also, they can do this check only once and not everytime.

No, relying on CPUID and vdso both have implications for virtualization.

Quoted 8 lines · * H. Peter Anvin ([email protected]) wrote:

Quoted 6 lines · Suresh Siddha

hpa, What is the virtualization problem? Are you referring to perf problem? As you noted, regular non-rt signal handlers won't need this cpuid check. It's needed only for those who manually look at non-rt signal frames and interpret it. And also, they can do this check only once and not everytime.

Quoted 1 line

No, relying on CPUID and vdso both have implications for virtualization.

hmm, what implications? cpuid is dealt with already.

Quoted 1 line · Chris Wright

hmm, what implications? cpuid is dealt with already.

By whom?

Quoted 3 lines · * Mikael Pettersson ([email protected]) wrote:

Quoted 1 line · Chris Wright

hmm, what implications? cpuid is dealt with already.

By whom?

I was thinking specifically vmx non-root.

You're assuming the presence *and use* of VMX.

Quoted 6 lines · Suresh Siddha

hpa, What is the virtualization problem? Are you referring to perf problem? As you noted, regular non-rt signal handlers won't need this cpuid check. It's needed only for those who manually look at non-rt signal frames and interpret it. And also, they can do this check only once and not everytime.

No, relying on CPUID and vdso both have implications for virtualization.

Quoted 1 line

To me, prtcl() just seems to be an overkill.

I don't think it is ... it's not overkill but rather "underkill"... it's a low-performance solution but it's guaranteed to be safe in the presence of virtualization of all its various ilk. Note that you don't need to be able to *set* the format via prctl(), just *query* (get) it.

Quoted 4 lines

While restoring from the user, kernel also need to find out what layout the user is passing. So it's bi-directional. I prefer the same mechanism (using cookies/magic numbers etc inaddition to uc_flags or cpuid checks) to interpret the fpstate for both user/kernel.

No, it really doesn't: the kernel only needs to be able to read the same format as it itself wrote.

Quoted 8 lines

Quoted 5 lines

While restoring from the user, kernel also need to find out what layout the user is passing. So it's bi-directional. I prefer the same mechanism (using cookies/magic numbers etc inaddition to uc_flags or cpuid checks) to interpret the fpstate for both user/kernel.

No, it really doesn't: the kernel only needs to be able to read the same format as it itself wrote.

The kernel needs to accept one(*) of the formats it can produce, which is not necessarily what it last produced. It's not inconceivable that user-space will construct sigframes on the fly (to emulate setcontext), or that it will mangle sigframes (e.g. to map non-rt to rt before sigreturn).

(*) The format is determined by which version of sys_sigreturn the user invokes.

Quoted 18 lines · Mikael Pettersson

Quoted 2 lines

Quoted 2 lines

While restoring from the user, kernel also need to find out what

layout

Quoted 1 line

Quoted 1 line

the user is passing. So it's bi-directional. I prefer the same

mechanism

Quoted 1 line

Quoted 1 line

(using cookies/magic numbers etc inaddition to uc_flags or cpuid

checks) to

Quoted 3 lines

Quoted 1 line

interpret the fpstate for both user/kernel.

No, it really doesn't: the kernel only needs to be able to read the

same > format as it itself wrote.

The kernel needs to accept one(*) of the formats it can produce, which is not necessarily what it last produced. It's not inconceivable that user-space will construct sigframes on the fly (to emulate setcontext), or that it will mangle sigframes (e.g. to map non-rt to rt before sigreturn).

Quoted 3 lines

(*) The format is determined by which version of sys_sigreturn the user invokes.

No. You CANNOT restore from a frame that doesn't have the full state - you don't have enough information to do so!

What I was doing in the RFC is: restore the state what ever that was present and init the state that was not present in the stack frame.

Quoted 2 lines

What I was doing in the RFC is: restore the state what ever that was present and init the state that was not present in the stack frame.

That is consistent in spirit with the existing treatment of FPU data. That is, if the sigcontext.fpstate pointer is NULL, the thread's FPU state is reset to default. (And despite what hpa said about being "supported", the facts in the code are that sigreturn just follows the sigcontext.fpstate pointer, whatever it is. On 32-bit, the pointer is NULL in the context saved when the thread had not used the FPU, so modifying the sigcontext to include FP state when it didn't before requires putting in some user-chosen pointer. There in fact may well be existing code that does user-level coroutine switching using sigreturn and relies on this, for all we know.)

Quoted 14 lines · Roland McGrath

Quoted 2 lines

What I was doing in the RFC is: restore the state what ever that was present and init the state that was not present in the stack frame.

That is consistent in spirit with the existing treatment of FPU data. That is, if the sigcontext.fpstate pointer is NULL, the thread's FPU state is reset to default. (And despite what hpa said about being "supported", the facts in the code are that sigreturn just follows the sigcontext.fpstate pointer, whatever it is. On 32-bit, the pointer is NULL in the context saved when the thread had not used the FPU, so modifying the sigcontext to include FP state when it didn't before requires putting in some user-chosen pointer. There in fact may well be existing code that does user-level coroutine switching using sigreturn and relies on this, for all we know.)

Okay. Pretty much what it comes down to is that there is no ideal solution. Thus, we're trying to explore the potential tradeoffs. The scenario you describe above will crash horribly for a non-FXSAVE aware application running on an FXSAVE kernel.

Either way, there has been a long time since, and new bad applications have obviously emerged, partially "assisted" by our propensity to not document, and the deep gulf between our kernel and userspace developers.

Let's try another strawman on for size:

- It is clear it is desirable not just for the frame itself but for the fpstate to be self-describing.

- Thus, let's put a magic cookie in one of the reserved fields at the end of the FXSAVE region, and make sure it is long enough to be unlikely to pop up randomly; as well as another magic cookie outside the FXSAVE region.

- The signal delivery code will write the cookie (or zero, for !XSAVE) regardless of any crap ptrace might have written into it.

- We will ALSO set bit 0 in uc_flags for RT sigframes as an additional assurance.

- We will introduce at least a 32-bit field for future use, to be written unconditionally zero for now. We don't want to have to go through this particular torture yet again.

as well as another magic cookie outside the FXSAVE region.

- The XSAVE state beyond the FXSAVE region needs to be self-describing. This may mean adding information not provided by the hardware.

Furthermore, it must be possible for userspace to know the length of the frame, even if it doesn't understand its detailed contents.

None of this is foolproof on older kernels -- there simply *IS* no option for older kernels that is 100% guaranteed, thanks to various assumptions made and design decisions taken over the years. There are a couple of failure scenarious here:

- XSAVE-aware application running on pre-XSAVE kernel:

Such an application will be aware that the XSAVE information may not exist, but needs to know (with high probability) that it isn't present. We have CPUID.OSXSAVE, the uc_flags bit, and the magic cookie to help here. ptrace can introduce the magic cookie falsely into the state, but ptrace can introduce all kinds of failures; either way they would (probablistically) not see the *second* cookie.

The fact that 64-bit kernels don't clear the unused fields is of less concern, since 64-bit kernels get the uc_flags field.

- Pre-XSAVE application running on XSAVE kernel with XSAVE enabled:

Here we have the potential for all kinds of corrupt state, including userspace trying to save away the state and load it later, not knowing the proper size of it. Worse, some sick person might try to save and restore state from different hosts, with potential for all kinds of mayhem.

The saved state, if copied from the original, would contain the first cookie but not the second cookie.

Again, the use of two cookies here adds some amount of assurance;

but that again amounts to probabilistic failure detection. However, I personally don't see any way to avoid that scenario at all.

Quoted 6 lines

Quoted 6 lines

The kernel needs to accept one(*) of the formats it can produce, which is not necessarily what it last produced. It's not inconceivable that user-space will construct sigframes on the fly (to emulate setcontext), or that it will mangle sigframes (e.g. to map non-rt to rt before sigreturn).

Quoted 4 lines

Quoted 3 lines

(*) The format is determined by which version of sys_sigreturn the user invokes.

No. You CANNOT restore from a frame that doesn't have the full state - you don't have enough information to do so!

What I was doing in the RFC is: restore the state what ever that was present and init the state that was not present in the stack frame.

An ugly workaround could be to start clearing one of these fields, and say that the data there is only valid for kernels >= 2.6.26. (I said it was ugly...)

Or we go back to stashing a flag in uc_flags (which is kosher), and try to figure out how to mark non-rt sigframes.

Quoted 2 lines

Ok. CPU folks are planning to make some of the bytes at the end of fxsave image, SW usable.

Are they always zeroed in earlier CPUs though? If not that wouldn't work 100% reliably because whatever cookie you put in could have been there before by chance.

I don't see anything in the SDM guaranteeing zeroing.

Quoted 8 lines · Andi Kleen

Quoted 3 lines

Ok. CPU folks are planning to make some of the bytes at the end of fxsave
image, SW usable.

Are they always zeroed in earlier CPUs though? If not that wouldn't work 100% reliably because whatever cookie you put in could have been there before by chance.

Quoted 2 lines

I don't see anything in the SDM guaranteeing zeroing.

I'm pretty sure they weren't zeroed by the CPUs. If they weren't zeroed *by the kernel*, there might have been an information leak.

I don't think there is one. We never copy fxsave completely out of the kernel. x86-64 does FXSAVE directly in/out user space, but the only leak is what there was before.

Quoted 3 lines

I don't think there is one. We never copy fxsave completely out of the kernel. x86-64 does FXSAVE directly in/out user space, but the only leak is what there was before.

ptrace/user_regset copies out and in the whole fxsave block from the ptrace caller. (Only the mxcsr word is constrained after copy-in.)

1. potential information leak out of the kernel if the memory area isn't zeroed before the first FXSAVE - I haven't verified if so is the case. This would be a (potentially very serious) security hole.

2. Hidden state in the kernel - this means user space can set nonarchitectural state in the kernel. There are a few risks with that:

a. Malware might use it to hide state.

b. The possibility of using the stability or lack thereof of this
state to extract information about kernel internals and/or provide a covert channel in the presence of hardware changes.
c. It is not certain that future architectures will not have
off-limit fields here, like the equivalent of MXCSR. This is somewhat of a tricky judgement, of course, but it seems safer to me if we would explicitly list the modifiable fields.

AFAIK the only reason we control the MXCSR value is because we don't want to handle a #GP in context switch.