On 04/08/2020 10:36, Jan Beulich wrote:
> See the code comment that's being extended. Additionally a few more
> zap_fpsel() invocations are needed - whenever we stored state after
> there potentially having been a context switch behind our backs.
>
> Reported-by: Andrew Cooper <andrew.coop...@citrix.com>
> Signed-off-by: Jan Beulich <jbeul...@suse.com>
Acked-by: Andrew Cooper <andrew.coop...@citrix.com>
Tested-by: Andrew Cooper <andrew.coop...@citrix.com>
>
> --- a/tools/tests/x86_emulator/test_x86_emulator.c
> +++ b/tools/tests/x86_emulator/test_x86_emulator.c
> @@ -752,6 +752,13 @@ static struct x86_emulate_ops emulops =
> * 64-bit OSes may not (be able to) properly restore the two selectors in
> * the FPU environment. Zap them so that memcmp() on two saved images will
> * work regardless of whether a context switch occurred in the middle.
> + *
> + * Additionally on AMD-like CPUs FDP/FIP/FOP may get lost across context
> + * switches, when there's no unmasked pending FP exception: With
I think you want a full stop rather than a colon, and ...
> + * CPUID[80000008].EBX[2] clear, the fields don't get written/read by
> + * {F,}XSAVE / {F,}XRSTOR, which OSes often compensate for by invoking an
> + * insn forcing the fields to gain a deterministic value. Whereas with said
... a comma here rather than a full stop.
Having "whereas" at the beginning of a sentence like this is weird,
given that you're contrasting the behaviour of the CPUID bit.
Also, the more usual CPUID syntax would be CPUID.0x80000008.EBX[2].
~Andrew
> + * bit set, zeroes will get written (and hence later restored).
> */
> static void zap_fpsel(unsigned int *env, bool is_32bit)
> {
>
