Re: [PATCH v3] x86/fpu: copy MXCSR & MXCSR_FLAGS with SSE/YMM state

From: Ingo Molnar
Date: Sat Feb 11 2017 - 05:02:50 EST

* Rik van Riel <riel@xxxxxxxxxx> wrote:

> /*
> + * Weird legacy quirk: SSE and YMM states store information in the
> + * MXCSR and MXCSR_FLAGS fields of the FP area. That means if the FP
> + * area is marked is unused in the xfeatures header, we need to copy
> + * MXCSR and MXCSR_FLAGS if either SSE or YMM are in use.
> + */
> +static inline bool xfeatures_need_mxcsr_copy(u64 xfeatures)
> +{
> + if (!(xfeatures & (XFEATURE_MASK_SSE|XFEATURE_MASK_YMM)))
> + return 0;
> +
> + if (xfeatures & XFEATURE_MASK_FP)
> + return 0;
> +
> + return 1;
> +}

Applied to tip:WIP.x86/fpu and will try to get that branch into final shape ASAP,
thanks Rik!

BTW., a different approach: could we also implement this quirk via setting the
xfeatures bits accordingly? In particular, we could set FP to 1 if we see that

I.e. instead of:

header.xfeatures = xsave->header.xfeatures;

We could do something like:

header.xfeatures = xfeatures_quirk(xsave->header.xfeatures);


xfeatures_quirk() would do the obvious:

static u64 xfeatures_mxcsr_quirk(u64 xfeatures)
return xfeatures | XFEATURE_MASK_FP;

return xfeatures;

This means we'd copy the whole FP area, not just the MXCSR* fields, but I think
overall it's a cleaner and easier to maintain approach - assuming it works and I'm
missing something.

Such as us then sticking that enabled FP bit into the hardware and it could get
confused or reject the state if other FP fields have random values?

In any case I've applied your fix with minor edits: I fixed a typo, renamed the
quirk function which was a bit long, removed marginal linebreaks and twiddled the
changelog. Edited version is attached below.

BTW., would you be interested in adding your FPU user ABI tests to
tools/tests/selftests/x86? If there's many tests then I wouldn't mind if it got a
new, separate subdirectory, under tools/tests/selftests/x86/fpu/ or so.