diff mbox series

[kvm-unit-tests,1/2] x86: Do not assign values to unaligned pointer to 128 bits

Message ID 20210511015016.815461-1-jacobhxu@google.com (mailing list archive)
State New, archived
Headers show
Series [kvm-unit-tests,1/2] x86: Do not assign values to unaligned pointer to 128 bits | expand

Commit Message

Jacob Xu May 11, 2021, 1:50 a.m. UTC
When compiled with clang, the following statement gets converted into a
movaps instructions.
mem->u[0] = 5; mem->u[1] = 6; mem->u[2] = 7; mem->u[3] = 8;

Since mem is an unaligned pointer to sse_union, we get a GP when
running. Let's avoid using a pointer to sse_union at all, since doing so
implies that the pointer is aligned to 128 bits.

Fixes: e5e76263b5 ("x86: add additional test cases for sse exceptions to
emulator.c")

Signed-off-by: Jacob Xu <jacobhxu@google.com>
---
 x86/emulator.c | 67 +++++++++++++++++++++++++-------------------------
 1 file changed, 33 insertions(+), 34 deletions(-)

Comments

Jacob Xu June 10, 2021, 4:14 p.m. UTC | #1
ping (previous ping just now contained html subpart, oops).


On Mon, May 10, 2021 at 6:50 PM Jacob Xu <jacobhxu@google.com> wrote:
>
> When compiled with clang, the following statement gets converted into a
> movaps instructions.
> mem->u[0] = 5; mem->u[1] = 6; mem->u[2] = 7; mem->u[3] = 8;
>
> Since mem is an unaligned pointer to sse_union, we get a GP when
> running. Let's avoid using a pointer to sse_union at all, since doing so
> implies that the pointer is aligned to 128 bits.
>
> Fixes: e5e76263b5 ("x86: add additional test cases for sse exceptions to
> emulator.c")
>
> Signed-off-by: Jacob Xu <jacobhxu@google.com>
> ---
>  x86/emulator.c | 67 +++++++++++++++++++++++++-------------------------
>  1 file changed, 33 insertions(+), 34 deletions(-)
>
> diff --git a/x86/emulator.c b/x86/emulator.c
> index 9705073..1d5c172 100644
> --- a/x86/emulator.c
> +++ b/x86/emulator.c
> @@ -645,37 +645,34 @@ static void test_muldiv(long *mem)
>
>  typedef unsigned __attribute__((vector_size(16))) sse128;
>
> -typedef union {
> -    sse128 sse;
> -    unsigned u[4];
> -} sse_union;
> -
> -static bool sseeq(sse_union *v1, sse_union *v2)
> +static bool sseeq(uint32_t *v1, uint32_t *v2)
>  {
>      bool ok = true;
>      int i;
>
>      for (i = 0; i < 4; ++i) {
> -       ok &= v1->u[i] == v2->u[i];
> +       ok &= v1[i] == v2[i];
>      }
>
>      return ok;
>  }
>
> -static __attribute__((target("sse2"))) void test_sse(sse_union *mem)
> +static __attribute__((target("sse2"))) void test_sse(uint32_t *mem)
>  {
> -       sse_union v;
> +       sse128 vv;
> +       uint32_t *v = (uint32_t *)&vv;
>
>         write_cr0(read_cr0() & ~6); /* EM, TS */
>         write_cr4(read_cr4() | 0x200); /* OSFXSR */
> +       memset(&vv, 0, sizeof(vv));
>
>  #define TEST_RW_SSE(insn) do { \
> -               v.u[0] = 1; v.u[1] = 2; v.u[2] = 3; v.u[3] = 4; \
> -               asm(insn " %1, %0" : "=m"(*mem) : "x"(v.sse)); \
> -               report(sseeq(&v, mem), insn " (read)"); \
> -               mem->u[0] = 5; mem->u[1] = 6; mem->u[2] = 7; mem->u[3] = 8; \
> -               asm(insn " %1, %0" : "=x"(v.sse) : "m"(*mem)); \
> -               report(sseeq(&v, mem), insn " (write)"); \
> +               v[0] = 1; v[1] = 2; v[2] = 3; v[3] = 4; \
> +               asm(insn " %1, %0" : "=m"(*mem) : "x"(vv) : "memory"); \
> +               report(sseeq(v, mem), insn " (read)"); \
> +               mem[0] = 5; mem[1] = 6; mem[2] = 7; mem[3] = 8; \
> +               asm(insn " %1, %0" : "=x"(vv) : "m"(*mem) : "memory"); \
> +               report(sseeq(v, mem), insn " (write)"); \
>  } while (0)
>
>         TEST_RW_SSE("movdqu");
> @@ -704,40 +701,41 @@ static void cross_movups_handler(struct ex_regs *regs)
>
>  static __attribute__((target("sse2"))) void test_sse_exceptions(void *cross_mem)
>  {
> -       sse_union v;
> -       sse_union *mem;
> +       sse128 vv;
> +       uint32_t *v = (uint32_t *)&vv;
> +       uint32_t *mem;
>         uint8_t *bytes = cross_mem; // aligned on PAGE_SIZE*2
>         void *page2 = (void *)(&bytes[4096]);
>         struct pte_search search;
>         pteval_t orig_pte;
>
>         // setup memory for unaligned access
> -       mem = (sse_union *)(&bytes[8]);
> +       mem = (uint32_t *)(&bytes[8]);
>
>         // test unaligned access for movups, movupd and movaps
> -       v.u[0] = 1; v.u[1] = 2; v.u[2] = 3; v.u[3] = 4;
> -       mem->u[0] = 5; mem->u[1] = 6; mem->u[2] = 7; mem->u[3] = 8;
> -       asm("movups %1, %0" : "=m"(*mem) : "x"(v.sse));
> -       report(sseeq(&v, mem), "movups unaligned");
> -
> -       v.u[0] = 1; v.u[1] = 2; v.u[2] = 3; v.u[3] = 4;
> -       mem->u[0] = 5; mem->u[1] = 6; mem->u[2] = 7; mem->u[3] = 8;
> -       asm("movupd %1, %0" : "=m"(*mem) : "x"(v.sse));
> -       report(sseeq(&v, mem), "movupd unaligned");
> +       v[0] = 1; v[1] = 2; v[2] = 3; v[3] = 4;
> +       mem[0] = 5; mem[1] = 6; mem[2] = 8; mem[3] = 9;
> +       asm("movups %1, %0" : "=m"(*mem) : "x"(vv) : "memory");
> +       report(sseeq(v, mem), "movups unaligned");
> +
> +       v[0] = 1; v[1] = 2; v[2] = 3; v[3] = 4;
> +       mem[0] = 5; mem[1] = 6; mem[2] = 7; mem[3] = 8;
> +       asm("movupd %1, %0" : "=m"(*mem) : "x"(vv) : "memory");
> +       report(sseeq(v, mem), "movupd unaligned");
>         exceptions = 0;
>         handle_exception(GP_VECTOR, unaligned_movaps_handler);
>         asm("movaps %1, %0\n\t unaligned_movaps_cont:"
> -                       : "=m"(*mem) : "x"(v.sse));
> +                       : "=m"(*mem) : "x"(vv));
>         handle_exception(GP_VECTOR, 0);
>         report(exceptions == 1, "unaligned movaps exception");
>
>         // setup memory for cross page access
> -       mem = (sse_union *)(&bytes[4096-8]);
> -       v.u[0] = 1; v.u[1] = 2; v.u[2] = 3; v.u[3] = 4;
> -       mem->u[0] = 5; mem->u[1] = 6; mem->u[2] = 7; mem->u[3] = 8;
> +       mem = (uint32_t *)(&bytes[4096-8]);
> +       v[0] = 1; v[1] = 2; v[2] = 3; v[3] = 4;
> +       mem[0] = 5; mem[1] = 6; mem[2] = 7; mem[3] = 8;
>
> -       asm("movups %1, %0" : "=m"(*mem) : "x"(v.sse));
> -       report(sseeq(&v, mem), "movups unaligned crosspage");
> +       asm("movups %1, %0" : "=m"(*mem) : "x"(vv) : "memory");
> +       report(sseeq(v, mem), "movups unaligned crosspage");
>
>         // invalidate second page
>         search = find_pte_level(current_page_table(), page2, 1);
> @@ -747,7 +745,8 @@ static __attribute__((target("sse2"))) void test_sse_exceptions(void *cross_mem)
>
>         exceptions = 0;
>         handle_exception(PF_VECTOR, cross_movups_handler);
> -       asm("movups %1, %0\n\t cross_movups_cont:" : "=m"(*mem) : "x"(v.sse));
> +       asm("movups %1, %0\n\t cross_movups_cont:" : "=m"(*mem) : "x"(vv) :
> +                       "memory");
>         handle_exception(PF_VECTOR, 0);
>         report(exceptions == 1, "movups crosspage exception");
>
> --
> 2.31.1.607.g51e8a6a459-goog
>
Paolo Bonzini June 10, 2021, 4:55 p.m. UTC | #2
On 10/06/21 18:14, Jacob Xu wrote:
> ping (previous ping just now contained html subpart, oops).

Queued, thanks!

Paolo

> On Mon, May 10, 2021 at 6:50 PM Jacob Xu <jacobhxu@google.com> wrote:
>>
>> When compiled with clang, the following statement gets converted into a
>> movaps instructions.
>> mem->u[0] = 5; mem->u[1] = 6; mem->u[2] = 7; mem->u[3] = 8;
>>
>> Since mem is an unaligned pointer to sse_union, we get a GP when
>> running. Let's avoid using a pointer to sse_union at all, since doing so
>> implies that the pointer is aligned to 128 bits.
>>
>> Fixes: e5e76263b5 ("x86: add additional test cases for sse exceptions to
>> emulator.c")
>>
>> Signed-off-by: Jacob Xu <jacobhxu@google.com>
>> ---
>>   x86/emulator.c | 67 +++++++++++++++++++++++++-------------------------
>>   1 file changed, 33 insertions(+), 34 deletions(-)
>>
>> diff --git a/x86/emulator.c b/x86/emulator.c
>> index 9705073..1d5c172 100644
>> --- a/x86/emulator.c
>> +++ b/x86/emulator.c
>> @@ -645,37 +645,34 @@ static void test_muldiv(long *mem)
>>
>>   typedef unsigned __attribute__((vector_size(16))) sse128;
>>
>> -typedef union {
>> -    sse128 sse;
>> -    unsigned u[4];
>> -} sse_union;
>> -
>> -static bool sseeq(sse_union *v1, sse_union *v2)
>> +static bool sseeq(uint32_t *v1, uint32_t *v2)
>>   {
>>       bool ok = true;
>>       int i;
>>
>>       for (i = 0; i < 4; ++i) {
>> -       ok &= v1->u[i] == v2->u[i];
>> +       ok &= v1[i] == v2[i];
>>       }
>>
>>       return ok;
>>   }
>>
>> -static __attribute__((target("sse2"))) void test_sse(sse_union *mem)
>> +static __attribute__((target("sse2"))) void test_sse(uint32_t *mem)
>>   {
>> -       sse_union v;
>> +       sse128 vv;
>> +       uint32_t *v = (uint32_t *)&vv;
>>
>>          write_cr0(read_cr0() & ~6); /* EM, TS */
>>          write_cr4(read_cr4() | 0x200); /* OSFXSR */
>> +       memset(&vv, 0, sizeof(vv));
>>
>>   #define TEST_RW_SSE(insn) do { \
>> -               v.u[0] = 1; v.u[1] = 2; v.u[2] = 3; v.u[3] = 4; \
>> -               asm(insn " %1, %0" : "=m"(*mem) : "x"(v.sse)); \
>> -               report(sseeq(&v, mem), insn " (read)"); \
>> -               mem->u[0] = 5; mem->u[1] = 6; mem->u[2] = 7; mem->u[3] = 8; \
>> -               asm(insn " %1, %0" : "=x"(v.sse) : "m"(*mem)); \
>> -               report(sseeq(&v, mem), insn " (write)"); \
>> +               v[0] = 1; v[1] = 2; v[2] = 3; v[3] = 4; \
>> +               asm(insn " %1, %0" : "=m"(*mem) : "x"(vv) : "memory"); \
>> +               report(sseeq(v, mem), insn " (read)"); \
>> +               mem[0] = 5; mem[1] = 6; mem[2] = 7; mem[3] = 8; \
>> +               asm(insn " %1, %0" : "=x"(vv) : "m"(*mem) : "memory"); \
>> +               report(sseeq(v, mem), insn " (write)"); \
>>   } while (0)
>>
>>          TEST_RW_SSE("movdqu");
>> @@ -704,40 +701,41 @@ static void cross_movups_handler(struct ex_regs *regs)
>>
>>   static __attribute__((target("sse2"))) void test_sse_exceptions(void *cross_mem)
>>   {
>> -       sse_union v;
>> -       sse_union *mem;
>> +       sse128 vv;
>> +       uint32_t *v = (uint32_t *)&vv;
>> +       uint32_t *mem;
>>          uint8_t *bytes = cross_mem; // aligned on PAGE_SIZE*2
>>          void *page2 = (void *)(&bytes[4096]);
>>          struct pte_search search;
>>          pteval_t orig_pte;
>>
>>          // setup memory for unaligned access
>> -       mem = (sse_union *)(&bytes[8]);
>> +       mem = (uint32_t *)(&bytes[8]);
>>
>>          // test unaligned access for movups, movupd and movaps
>> -       v.u[0] = 1; v.u[1] = 2; v.u[2] = 3; v.u[3] = 4;
>> -       mem->u[0] = 5; mem->u[1] = 6; mem->u[2] = 7; mem->u[3] = 8;
>> -       asm("movups %1, %0" : "=m"(*mem) : "x"(v.sse));
>> -       report(sseeq(&v, mem), "movups unaligned");
>> -
>> -       v.u[0] = 1; v.u[1] = 2; v.u[2] = 3; v.u[3] = 4;
>> -       mem->u[0] = 5; mem->u[1] = 6; mem->u[2] = 7; mem->u[3] = 8;
>> -       asm("movupd %1, %0" : "=m"(*mem) : "x"(v.sse));
>> -       report(sseeq(&v, mem), "movupd unaligned");
>> +       v[0] = 1; v[1] = 2; v[2] = 3; v[3] = 4;
>> +       mem[0] = 5; mem[1] = 6; mem[2] = 8; mem[3] = 9;
>> +       asm("movups %1, %0" : "=m"(*mem) : "x"(vv) : "memory");
>> +       report(sseeq(v, mem), "movups unaligned");
>> +
>> +       v[0] = 1; v[1] = 2; v[2] = 3; v[3] = 4;
>> +       mem[0] = 5; mem[1] = 6; mem[2] = 7; mem[3] = 8;
>> +       asm("movupd %1, %0" : "=m"(*mem) : "x"(vv) : "memory");
>> +       report(sseeq(v, mem), "movupd unaligned");
>>          exceptions = 0;
>>          handle_exception(GP_VECTOR, unaligned_movaps_handler);
>>          asm("movaps %1, %0\n\t unaligned_movaps_cont:"
>> -                       : "=m"(*mem) : "x"(v.sse));
>> +                       : "=m"(*mem) : "x"(vv));
>>          handle_exception(GP_VECTOR, 0);
>>          report(exceptions == 1, "unaligned movaps exception");
>>
>>          // setup memory for cross page access
>> -       mem = (sse_union *)(&bytes[4096-8]);
>> -       v.u[0] = 1; v.u[1] = 2; v.u[2] = 3; v.u[3] = 4;
>> -       mem->u[0] = 5; mem->u[1] = 6; mem->u[2] = 7; mem->u[3] = 8;
>> +       mem = (uint32_t *)(&bytes[4096-8]);
>> +       v[0] = 1; v[1] = 2; v[2] = 3; v[3] = 4;
>> +       mem[0] = 5; mem[1] = 6; mem[2] = 7; mem[3] = 8;
>>
>> -       asm("movups %1, %0" : "=m"(*mem) : "x"(v.sse));
>> -       report(sseeq(&v, mem), "movups unaligned crosspage");
>> +       asm("movups %1, %0" : "=m"(*mem) : "x"(vv) : "memory");
>> +       report(sseeq(v, mem), "movups unaligned crosspage");
>>
>>          // invalidate second page
>>          search = find_pte_level(current_page_table(), page2, 1);
>> @@ -747,7 +745,8 @@ static __attribute__((target("sse2"))) void test_sse_exceptions(void *cross_mem)
>>
>>          exceptions = 0;
>>          handle_exception(PF_VECTOR, cross_movups_handler);
>> -       asm("movups %1, %0\n\t cross_movups_cont:" : "=m"(*mem) : "x"(v.sse));
>> +       asm("movups %1, %0\n\t cross_movups_cont:" : "=m"(*mem) : "x"(vv) :
>> +                       "memory");
>>          handle_exception(PF_VECTOR, 0);
>>          report(exceptions == 1, "movups crosspage exception");
>>
>> --
>> 2.31.1.607.g51e8a6a459-goog
>>
>
diff mbox series

Patch

diff --git a/x86/emulator.c b/x86/emulator.c
index 9705073..1d5c172 100644
--- a/x86/emulator.c
+++ b/x86/emulator.c
@@ -645,37 +645,34 @@  static void test_muldiv(long *mem)
 
 typedef unsigned __attribute__((vector_size(16))) sse128;
 
-typedef union {
-    sse128 sse;
-    unsigned u[4];
-} sse_union;
-
-static bool sseeq(sse_union *v1, sse_union *v2)
+static bool sseeq(uint32_t *v1, uint32_t *v2)
 {
     bool ok = true;
     int i;
 
     for (i = 0; i < 4; ++i) {
-	ok &= v1->u[i] == v2->u[i];
+	ok &= v1[i] == v2[i];
     }
 
     return ok;
 }
 
-static __attribute__((target("sse2"))) void test_sse(sse_union *mem)
+static __attribute__((target("sse2"))) void test_sse(uint32_t *mem)
 {
-	sse_union v;
+	sse128 vv;
+	uint32_t *v = (uint32_t *)&vv;
 
 	write_cr0(read_cr0() & ~6); /* EM, TS */
 	write_cr4(read_cr4() | 0x200); /* OSFXSR */
+	memset(&vv, 0, sizeof(vv));
 
 #define TEST_RW_SSE(insn) do { \
-		v.u[0] = 1; v.u[1] = 2; v.u[2] = 3; v.u[3] = 4; \
-		asm(insn " %1, %0" : "=m"(*mem) : "x"(v.sse)); \
-		report(sseeq(&v, mem), insn " (read)"); \
-		mem->u[0] = 5; mem->u[1] = 6; mem->u[2] = 7; mem->u[3] = 8; \
-		asm(insn " %1, %0" : "=x"(v.sse) : "m"(*mem)); \
-		report(sseeq(&v, mem), insn " (write)"); \
+		v[0] = 1; v[1] = 2; v[2] = 3; v[3] = 4; \
+		asm(insn " %1, %0" : "=m"(*mem) : "x"(vv) : "memory"); \
+		report(sseeq(v, mem), insn " (read)"); \
+		mem[0] = 5; mem[1] = 6; mem[2] = 7; mem[3] = 8; \
+		asm(insn " %1, %0" : "=x"(vv) : "m"(*mem) : "memory"); \
+		report(sseeq(v, mem), insn " (write)"); \
 } while (0)
 
 	TEST_RW_SSE("movdqu");
@@ -704,40 +701,41 @@  static void cross_movups_handler(struct ex_regs *regs)
 
 static __attribute__((target("sse2"))) void test_sse_exceptions(void *cross_mem)
 {
-	sse_union v;
-	sse_union *mem;
+	sse128 vv;
+	uint32_t *v = (uint32_t *)&vv;
+	uint32_t *mem;
 	uint8_t *bytes = cross_mem; // aligned on PAGE_SIZE*2
 	void *page2 = (void *)(&bytes[4096]);
 	struct pte_search search;
 	pteval_t orig_pte;
 
 	// setup memory for unaligned access
-	mem = (sse_union *)(&bytes[8]);
+	mem = (uint32_t *)(&bytes[8]);
 
 	// test unaligned access for movups, movupd and movaps
-	v.u[0] = 1; v.u[1] = 2; v.u[2] = 3; v.u[3] = 4;
-	mem->u[0] = 5; mem->u[1] = 6; mem->u[2] = 7; mem->u[3] = 8;
-	asm("movups %1, %0" : "=m"(*mem) : "x"(v.sse));
-	report(sseeq(&v, mem), "movups unaligned");
-
-	v.u[0] = 1; v.u[1] = 2; v.u[2] = 3; v.u[3] = 4;
-	mem->u[0] = 5; mem->u[1] = 6; mem->u[2] = 7; mem->u[3] = 8;
-	asm("movupd %1, %0" : "=m"(*mem) : "x"(v.sse));
-	report(sseeq(&v, mem), "movupd unaligned");
+	v[0] = 1; v[1] = 2; v[2] = 3; v[3] = 4;
+	mem[0] = 5; mem[1] = 6; mem[2] = 8; mem[3] = 9;
+	asm("movups %1, %0" : "=m"(*mem) : "x"(vv) : "memory");
+	report(sseeq(v, mem), "movups unaligned");
+
+	v[0] = 1; v[1] = 2; v[2] = 3; v[3] = 4;
+	mem[0] = 5; mem[1] = 6; mem[2] = 7; mem[3] = 8;
+	asm("movupd %1, %0" : "=m"(*mem) : "x"(vv) : "memory");
+	report(sseeq(v, mem), "movupd unaligned");
 	exceptions = 0;
 	handle_exception(GP_VECTOR, unaligned_movaps_handler);
 	asm("movaps %1, %0\n\t unaligned_movaps_cont:"
-			: "=m"(*mem) : "x"(v.sse));
+			: "=m"(*mem) : "x"(vv));
 	handle_exception(GP_VECTOR, 0);
 	report(exceptions == 1, "unaligned movaps exception");
 
 	// setup memory for cross page access
-	mem = (sse_union *)(&bytes[4096-8]);
-	v.u[0] = 1; v.u[1] = 2; v.u[2] = 3; v.u[3] = 4;
-	mem->u[0] = 5; mem->u[1] = 6; mem->u[2] = 7; mem->u[3] = 8;
+	mem = (uint32_t *)(&bytes[4096-8]);
+	v[0] = 1; v[1] = 2; v[2] = 3; v[3] = 4;
+	mem[0] = 5; mem[1] = 6; mem[2] = 7; mem[3] = 8;
 
-	asm("movups %1, %0" : "=m"(*mem) : "x"(v.sse));
-	report(sseeq(&v, mem), "movups unaligned crosspage");
+	asm("movups %1, %0" : "=m"(*mem) : "x"(vv) : "memory");
+	report(sseeq(v, mem), "movups unaligned crosspage");
 
 	// invalidate second page
 	search = find_pte_level(current_page_table(), page2, 1);
@@ -747,7 +745,8 @@  static __attribute__((target("sse2"))) void test_sse_exceptions(void *cross_mem)
 
 	exceptions = 0;
 	handle_exception(PF_VECTOR, cross_movups_handler);
-	asm("movups %1, %0\n\t cross_movups_cont:" : "=m"(*mem) : "x"(v.sse));
+	asm("movups %1, %0\n\t cross_movups_cont:" : "=m"(*mem) : "x"(vv) :
+			"memory");
 	handle_exception(PF_VECTOR, 0);
 	report(exceptions == 1, "movups crosspage exception");