On Thu, Jun 05, 2008 at 05:28:15PM -0700, H. Peter Anvin wrote:
quoted text
> Sorry for not getting back on this for so long.

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

quoted text
> 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.

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 text
> Given that, it would have been much better if there had 
> been separate invocations of XSAVE for each substate region.

It's primarily designed for context switch handling, with
fxsave's interoperability in mind. Even though the substates are
laidout one after another, xsave/xrstor can operate on individual
states based on the edx:eax mask.

Each substate will probably also have different instructions to
save/restore/init their substate.

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

yes.

quoted text
> 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.

As previously agreed.

quoted text
> - 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)

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 text
>   - 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 text
>   - 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 text
>   - 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.

In my RFC, we had the bit mask also saved in the SW-reserved area,
which represents the extended state saved in the signal frame.
This is in addition to the bit mask represeted by the xstate_bv
in the header. xstate_bv indicates the current status (init/non-init)
of the sub-states for the bit mask saved in the SW-reserved area.

While restoring, kernel can also use the same bit mask in SW area to restore
the state and init the other state not referred by the bit mask.

quoted text
> 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.

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 text
>   - 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?

thanks,
suresh
--
unsubscribe notice
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/