Re: [Xen-devel] [PATCH 1/3] tests/x86emul: Helpers to save and restore FPU state
On 09/03/18 13:38, Jan Beulich wrote:
On 09.03.18 at 13:37, wrote:
> On 06.03.18 at 21:24, wrote:
>>> +void emul_save_fpu_state(void)
>>> +{
>>> +if ( use_xsave )
>>> +asm volatile ( "xsave" __OS " %[ptr]"
>>> + : [ptr] "=m" (fpu_save_area)
>>> + : "a" (~0ull), "d" (~0ull) );
>> Wait, this doesn't build as 32-bit binary. Needs to be ~0ul, and
>> __OS also can't be used here.
>>
>>> +else
>>> +asm volatile ( "fxsave %0" : "=m" (fpu_save_area) );
>> Whereas if you want something like __OS above, you'd want the
>> same here.
> Here's the full incremental diff I've used for now, so that things
> would build everywhere I've tried:
>
> --- unstable.orig/tools/tests/x86_emulator/x86-emulate.c
> +++ unstable/tools/tests/x86_emulator/x86-emulate.c
> @@ -32,20 +32,22 @@ static bool use_xsave;
> void emul_save_fpu_state(void)
> {
> if ( use_xsave )
> -asm volatile ( "xsave" __OS " %[ptr]"
> +asm volatile ( "xsave %[ptr]"
> : [ptr] "=m" (fpu_save_area)
> - : "a" (~0ull), "d" (~0ull) );
> + : "a" (~0ul), "d" (~0ul) );
> else
> asm volatile ( "fxsave %0" : "=m" (fpu_save_area) );
> }
>
> void emul_restore_fpu_state(void)
> {
> +/* Older gcc can't deal with "m" array inputs; make them outputs
> instead. */
> if ( use_xsave )
> -asm volatile ( "xrstor" __OS " %[ptr]"
> - :: [ptr] "m" (fpu_save_area), "a" (~0ull), "d"
> (~0ull) );
> +asm volatile ( "xrstor %[ptr]"
> + : [ptr] "+m" (fpu_save_area)
> + : "a" (~0ul), "d" (~0ul) );
> else
> -asm volatile ( "fxrstor %0" :: "m" (fpu_save_area) );
> +asm volatile ( "fxrstor %0" : "+m" (fpu_save_area) );
> }
>
> bool emul_test_init(void)
Ok - I'll merge this in.
My worry with the __OS was to make sure that we matched how the kernel
would save and restore context, so the exception pointers don't get
lost. However, that only matters at the point that we attempt to
memcmp(), and thinking about it, we'd need a better algorithm anyway.
~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/3] tests/x86emul: Helpers to save and restore FPU state
>>> On 09.03.18 at 13:37, wrote:
On 06.03.18 at 21:24, wrote:
>> +void emul_save_fpu_state(void)
>> +{
>> +if ( use_xsave )
>> +asm volatile ( "xsave" __OS " %[ptr]"
>> + : [ptr] "=m" (fpu_save_area)
>> + : "a" (~0ull), "d" (~0ull) );
>
> Wait, this doesn't build as 32-bit binary. Needs to be ~0ul, and
> __OS also can't be used here.
>
>> +else
>> +asm volatile ( "fxsave %0" : "=m" (fpu_save_area) );
>
> Whereas if you want something like __OS above, you'd want the
> same here.
Here's the full incremental diff I've used for now, so that things
would build everywhere I've tried:
--- unstable.orig/tools/tests/x86_emulator/x86-emulate.c
+++ unstable/tools/tests/x86_emulator/x86-emulate.c
@@ -32,20 +32,22 @@ static bool use_xsave;
void emul_save_fpu_state(void)
{
if ( use_xsave )
-asm volatile ( "xsave" __OS " %[ptr]"
+asm volatile ( "xsave %[ptr]"
: [ptr] "=m" (fpu_save_area)
- : "a" (~0ull), "d" (~0ull) );
+ : "a" (~0ul), "d" (~0ul) );
else
asm volatile ( "fxsave %0" : "=m" (fpu_save_area) );
}
void emul_restore_fpu_state(void)
{
+/* Older gcc can't deal with "m" array inputs; make them outputs instead.
*/
if ( use_xsave )
-asm volatile ( "xrstor" __OS " %[ptr]"
- :: [ptr] "m" (fpu_save_area), "a" (~0ull), "d" (~0ull)
);
+asm volatile ( "xrstor %[ptr]"
+ : [ptr] "+m" (fpu_save_area)
+ : "a" (~0ul), "d" (~0ul) );
else
-asm volatile ( "fxrstor %0" :: "m" (fpu_save_area) );
+asm volatile ( "fxrstor %0" : "+m" (fpu_save_area) );
}
bool emul_test_init(void)
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/3] tests/x86emul: Helpers to save and restore FPU state
>>> On 06.03.18 at 21:24, wrote:
> +void emul_save_fpu_state(void)
> +{
> +if ( use_xsave )
> +asm volatile ( "xsave" __OS " %[ptr]"
> + : [ptr] "=m" (fpu_save_area)
> + : "a" (~0ull), "d" (~0ull) );
Wait, this doesn't build as 32-bit binary. Needs to be ~0ul, and
__OS also can't be used here.
> +else
> +asm volatile ( "fxsave %0" : "=m" (fpu_save_area) );
Whereas if you want something like __OS above, you'd want the
same here.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Re: [Xen-devel] [PATCH 1/3] tests/x86emul: Helpers to save and restore FPU state
>>> On 06.03.18 at 21:24, wrote: > Introduce common helpers for saving and restoring FPU state. During > emul_test_init(), calculate whether to use xsave or fxsave, and tweak the > existing mxcsr_mask logic to avoid using another large static buffer. > > Signed-off-by: Andrew Cooper Reviewed-by: Jan Beulich ___ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel
[Xen-devel] [PATCH 1/3] tests/x86emul: Helpers to save and restore FPU state
Introduce common helpers for saving and restoring FPU state. During
emul_test_init(), calculate whether to use xsave or fxsave, and tweak the
existing mxcsr_mask logic to avoid using another large static buffer.
Signed-off-by: Andrew Cooper
---
CC: Jan Beulich
---
tools/tests/x86_emulator/x86-emulate.c | 70 +++---
tools/tests/x86_emulator/x86-emulate.h | 4 ++
2 files changed, 60 insertions(+), 14 deletions(-)
diff --git a/tools/tests/x86_emulator/x86-emulate.c
b/tools/tests/x86_emulator/x86-emulate.c
index 9056610..47e503d 100644
--- a/tools/tests/x86_emulator/x86-emulate.c
+++ b/tools/tests/x86_emulator/x86-emulate.c
@@ -26,26 +26,68 @@
uint32_t mxcsr_mask = 0xffbf;
+static char fpu_save_area[4096] __attribute__((__aligned__((64;
+static bool use_xsave;
+
+void emul_save_fpu_state(void)
+{
+if ( use_xsave )
+asm volatile ( "xsave" __OS " %[ptr]"
+ : [ptr] "=m" (fpu_save_area)
+ : "a" (~0ull), "d" (~0ull) );
+else
+asm volatile ( "fxsave %0" : "=m" (fpu_save_area) );
+}
+
+void emul_restore_fpu_state(void)
+{
+if ( use_xsave )
+asm volatile ( "xrstor" __OS " %[ptr]"
+ :: [ptr] "m" (fpu_save_area), "a" (~0ull), "d" (~0ull)
);
+else
+asm volatile ( "fxrstor %0" :: "m" (fpu_save_area) );
+}
+
bool emul_test_init(void)
{
+union {
+char x[464];
+struct {
+uint32_t other[6];
+uint32_t mxcsr;
+uint32_t mxcsr_mask;
+/* ... */
+};
+} *fxs = (void *)fpu_save_area;
+
unsigned long sp;
-if ( cpu_has_fxsr )
+if ( cpu_has_xsave )
{
-static union __attribute__((__aligned__(16))) {
-char x[464];
-struct {
-uint32_t other[6];
-uint32_t mxcsr;
-uint32_t mxcsr_mask;
-/* ... */
-};
-} fxs;
-
-asm ( "fxsave %0" : "=m" (fxs) );
-if ( fxs.mxcsr_mask )
-mxcsr_mask = fxs.mxcsr_mask;
+unsigned int tmp, ebx;
+
+asm ( "cpuid"
+ : "=a" (tmp), "=b" (ebx), "=c" (tmp), "=d" (tmp)
+ : "a" (0xd), "c" (0) );
+
+/*
+ * Sanity check that fpu_save_area[] is large enough. This assertion
+ * will trip eventually, at which point fpu_save_area[] needs to get
+ * larger.
+ */
+assert(ebx < sizeof(fpu_save_area));
+
+/* Use xsave if available... */
+use_xsave = true;
}
+else
+/* But use fxsave if xsave isn't available. */
+assert(cpu_has_fxsr);
+
+/* Reuse the save state buffer to find mcxsr_mask. */
+asm ( "fxsave %0" : "=m" (*fxs) );
+if ( fxs->mxcsr_mask )
+mxcsr_mask = fxs->mxcsr_mask;
/*
* Mark the entire stack executable so that the stub executions
diff --git a/tools/tests/x86_emulator/x86-emulate.h
b/tools/tests/x86_emulator/x86-emulate.h
index a1c4774..45acea4 100644
--- a/tools/tests/x86_emulator/x86-emulate.h
+++ b/tools/tests/x86_emulator/x86-emulate.h
@@ -52,6 +52,10 @@ extern uint32_t mxcsr_mask;
#define MMAP_SZ 16384
bool emul_test_init(void);
+/* Must save and restore FPU state between any call into libc. */
+void emul_save_fpu_state(void);
+void emul_restore_fpu_state(void);
+
#include "x86_emulate/x86_emulate.h"
static inline uint64_t xgetbv(uint32_t xcr)
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
