[PATCH v2 for-4.9 1/2] x86/emul: Use explicit __attribute__((packed)) rather than __packed

[PATCH v2 for-4.9 1/2] x86/emul: Use explicit __attribute__((packed)) rather than __packed

[Andrew Cooper]

x86_emulate.h is included by the userspace test harness.  Avoid using
constructs which don't come from standard header files.

Reposition the test harnesses inclusion of x86_emulate.h to avoid relying on
any definitons intended for use by x86_emulate.c alone.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
---
 tools/tests/x86_emulator/test_x86_emulator.c | 2 --
 tools/tests/x86_emulator/x86_emulate.c       | 6 ++----
 xen/arch/x86/x86_emulate/x86_emulate.h       | 4 ++--
 3 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/tools/tests/x86_emulator/test_x86_emulator.c b/tools/tests/x86_emulator/test_x86_emulator.c
index 15d100a..6f6d70f 100644
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -8,8 +8,6 @@
 #include <xen/xen.h>
 #include <sys/mman.h>
 
-#define __packed __attribute__((packed))
-
 #include "x86_emulate/x86_emulate.h"
 #include "blowfish.h"
 
diff --git a/tools/tests/x86_emulator/x86_emulate.c b/tools/tests/x86_emulator/x86_emulate.c
index 50a4262..af90b6e 100644
--- a/tools/tests/x86_emulator/x86_emulate.c
+++ b/tools/tests/x86_emulator/x86_emulate.c
@@ -6,6 +6,8 @@
 #include <string.h>
 #include <xen/xen.h>
 
+#include "x86_emulate/x86_emulate.h"
+
 typedef bool bool_t;
 
 #define is_canonical_address(x) (((int64_t)(x) >> 47) == ((int64_t)(x) >> 63))
@@ -23,8 +25,6 @@ typedef bool bool_t;
 #define cpu_has_amd_erratum(nr) 0
 #define mark_regs_dirty(r) ((void)(r))
 
-#define __packed __attribute__((packed))
-
 /* For generic assembly code: use macros to define operation/operand sizes. */
 #ifdef __i386__
 # define __OS          "l"  /* Operation Suffix */
@@ -34,8 +34,6 @@ typedef bool bool_t;
 # define __OP          "r"  /* Operand Prefix */
 #endif
 
-#include "x86_emulate/x86_emulate.h"
-
 #define get_stub(stb) ((void *)((stb).addr = (uintptr_t)(stb).buf))
 #define put_stub(stb)
 
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h b/xen/arch/x86/x86_emulate/x86_emulate.h
index 641711e..2b39b81 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.h
+++ b/xen/arch/x86/x86_emulate/x86_emulate.h
@@ -71,7 +71,7 @@ enum x86_swint_emulation {
  * Attribute for segment selector. This is a copy of bit 40:47 & 52:55 of the
  * segment descriptor. It happens to match the format of an AMD SVM VMCB.
  */
-typedef union __packed segment_attributes {
+typedef union __attribute__((packed)) segment_attributes {
     uint16_t bytes;
     struct
     {
@@ -91,7 +91,7 @@ typedef union __packed segment_attributes {
  * Full state of a segment register (visible and hidden portions).
  * Again, this happens to match the format of an AMD SVM VMCB.
  */
-struct __packed segment_register {
+struct __attribute__((packed)) segment_register {
     uint16_t   sel;
     segment_attributes_t attr;
     uint32_t   limit;
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[PATCH v2 for-4.9 2/2] x86/emul: Reorder the user segments in x86_segment to match SReg3 encoding

[Andrew Cooper]

This avoids needing a translation table between hardware ordering and Xen's
ordering.

This also fixes a bug whereby an encoding using REX.R wasn't ignored.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>

v2:
 * Mask out REX.R, which is ignored by hardware.
 * Expose the BUILD_BUG_ON()s to the test harness, and fix its build.
---
 tools/tests/x86_emulator/x86_emulate.c | 13 +++++++++++++
 xen/arch/x86/x86_emulate/x86_emulate.c | 35 +++++++++++++++-------------------
 xen/arch/x86/x86_emulate/x86_emulate.h |  4 ++--
 3 files changed, 30 insertions(+), 22 deletions(-)

diff --git a/tools/tests/x86_emulator/x86_emulate.c b/tools/tests/x86_emulator/x86_emulate.c
index af90b6e..c46b7fc 100644
--- a/tools/tests/x86_emulator/x86_emulate.c
+++ b/tools/tests/x86_emulator/x86_emulate.c
@@ -19,6 +19,16 @@ typedef bool bool_t;
 #define ASSERT assert
 #define ASSERT_UNREACHABLE() assert(!__LINE__)
 
+#if __GNUC__ > 4 || (__GNUC__ == 4 && __GNUC_MINOR__ >= 6)
+/* Force a compilation error if condition is true */
+#define BUILD_BUG_ON(cond) ({ _Static_assert(!(cond), "!(" #cond ")"); })
+#define BUILD_BUG_ON_ZERO(cond) \
+    sizeof(struct { _Static_assert(!(cond), "!(" #cond ")"); })
+#else
+#define BUILD_BUG_ON_ZERO(cond) sizeof(struct { int:-!!(cond); })
+#define BUILD_BUG_ON(cond) ((void)BUILD_BUG_ON_ZERO(cond))
+#endif
+
 #define MASK_EXTR(v, m) (((v) & (m)) / ((m) & -(m)))
 #define MASK_INSR(v, m) (((v) * ((m) & -(m))) & (m))
 
@@ -37,4 +47,7 @@ typedef bool bool_t;
 #define get_stub(stb) ((void *)((stb).addr = (uintptr_t)(stb).buf))
 #define put_stub(stb)
 
+#define __init
+#define __maybe_unused __attribute__((__unused__))
+
 #include "x86_emulate/x86_emulate.c"
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c b/xen/arch/x86/x86_emulate/x86_emulate.c
index a1821d5..295907e 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1557,22 +1557,6 @@ decode_register(
     return p;
 }
 
-#define decode_segment_failed x86_seg_tr
-static enum x86_segment
-decode_segment(uint8_t modrm_reg)
-{
-    switch ( modrm_reg )
-    {
-    case 0: return x86_seg_es;
-    case 1: return x86_seg_cs;
-    case 2: return x86_seg_ss;
-    case 3: return x86_seg_ds;
-    case 4: return x86_seg_fs;
-    case 5: return x86_seg_gs;
-    }
-    return decode_segment_failed;
-}
-
 static bool is_aligned(enum x86_segment seg, unsigned long offs,
                        unsigned int size, struct x86_emulate_ctxt *ctxt,
                        const struct x86_emulate_ops *ops)
@@ -2982,8 +2966,8 @@ x86_emulate(
         break;
 
     case 0x8c: /* mov Sreg,r/m */
-        seg = decode_segment(modrm_reg);
-        generate_exception_if(seg == decode_segment_failed, EXC_UD, -1);
+        seg = modrm_reg & 7; /* REX.R is ignored. */
+        generate_exception_if(!is_x86_user_segment(seg), EXC_UD, -1);
     store_selector:
         fail_if(ops->read_segment == NULL);
         if ( (rc = ops->read_segment(seg, &sreg, ctxt)) != 0 )
@@ -2994,8 +2978,8 @@ x86_emulate(
         break;
 
     case 0x8e: /* mov r/m,Sreg */
-        seg = decode_segment(modrm_reg);
-        generate_exception_if(seg == decode_segment_failed, EXC_UD, -1);
+        seg = modrm_reg & 7; /* REX.R is ignored. */
+        generate_exception_if(!is_x86_user_segment(seg), EXC_UD, -1);
         generate_exception_if(seg == x86_seg_cs, EXC_UD, -1);
         if ( (rc = load_seg(seg, src.val, 0, NULL, ctxt, ops)) != 0 )
             goto done;
@@ -5438,6 +5422,17 @@ x86_emulate(
 #undef override_seg
 #undef ea
 
+static void __init __maybe_unused build_assertions(void)
+{
+    /* Check the values against SReg3 encoding in opcode/ModRM bytes. */
+    BUILD_BUG_ON(x86_seg_es != 0);
+    BUILD_BUG_ON(x86_seg_cs != 1);
+    BUILD_BUG_ON(x86_seg_ss != 2);
+    BUILD_BUG_ON(x86_seg_ds != 3);
+    BUILD_BUG_ON(x86_seg_fs != 4);
+    BUILD_BUG_ON(x86_seg_gs != 5);
+}
+
 #ifdef __XEN__
 
 #include <xen/err.h>
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h b/xen/arch/x86/x86_emulate/x86_emulate.h
index 2b39b81..639356a 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.h
+++ b/xen/arch/x86/x86_emulate/x86_emulate.h
@@ -29,11 +29,11 @@ struct x86_emulate_ctxt;
 
 /* Comprehensive enumeration of x86 segment registers. */
 enum x86_segment {
-    /* General purpose. */
+    /* General purpose.  Matches the SReg3 encoding in opcode/ModRM bytes. */
+    x86_seg_es,
     x86_seg_cs,
     x86_seg_ss,
     x86_seg_ds,
-    x86_seg_es,
     x86_seg_fs,
     x86_seg_gs,
     /* System. */
-- 
2.1.4


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 for-4.9 1/2] x86/emul: Use explicit __attribute__((packed)) rather than __packed

[Jan Beulich]

>>> On 25.10.16 at 20:10, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/arch/x86/x86_emulate/x86_emulate.h
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.h
> @@ -71,7 +71,7 @@ enum x86_swint_emulation {
>   * Attribute for segment selector. This is a copy of bit 40:47 & 52:55 of the
>   * segment descriptor. It happens to match the format of an AMD SVM VMCB.
>   */
> -typedef union __packed segment_attributes {
> +typedef union __attribute__((packed)) segment_attributes {
>      uint16_t bytes;
>      struct
>      {
> @@ -91,7 +91,7 @@ typedef union __packed segment_attributes {
>   * Full state of a segment register (visible and hidden portions).
>   * Again, this happens to match the format of an AMD SVM VMCB.
>   */
> -struct __packed segment_register {
> +struct __attribute__((packed)) segment_register {
>      uint16_t   sel;
>      segment_attributes_t attr;
>      uint32_t   limit;

Please use __packed__ in both cases (using the underscore free
variant is better to be restricted to just .c files). With that
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 for-4.9 2/2] x86/emul: Reorder the user segments in x86_segment to match SReg3 encoding

[Jan Beulich]

>>> On 25.10.16 at 20:10, <andrew.cooper3@citrix.com> wrote:
> @@ -2994,8 +2978,8 @@ x86_emulate(
>          break;
>  
>      case 0x8e: /* mov r/m,Sreg */
> -        seg = decode_segment(modrm_reg);
> -        generate_exception_if(seg == decode_segment_failed, EXC_UD, -1);
> +        seg = modrm_reg & 7; /* REX.R is ignored. */
> +        generate_exception_if(!is_x86_user_segment(seg), EXC_UD, -1);
>          generate_exception_if(seg == x86_seg_cs, EXC_UD, -1);

Can I talk you into folding these two generate_exception_if()s, as
they logically belong together?

> @@ -5438,6 +5422,17 @@ x86_emulate(
>  #undef override_seg
>  #undef ea
>  
> +static void __init __maybe_unused build_assertions(void)
> +{
> +    /* Check the values against SReg3 encoding in opcode/ModRM bytes. */
> +    BUILD_BUG_ON(x86_seg_es != 0);
> +    BUILD_BUG_ON(x86_seg_cs != 1);
> +    BUILD_BUG_ON(x86_seg_ss != 2);
> +    BUILD_BUG_ON(x86_seg_ds != 3);
> +    BUILD_BUG_ON(x86_seg_fs != 4);
> +    BUILD_BUG_ON(x86_seg_gs != 5);
> +}

So using an inline function in the header did not work out?

In any event,
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 for-4.9 1/2] x86/emul: Use explicit __attribute__((packed)) rather than __packed

[Andrew Cooper]

On 26/10/16 09:54, Jan Beulich wrote:
>>>> On 25.10.16 at 20:10, <andrew.cooper3@citrix.com> wrote:
>> --- a/xen/arch/x86/x86_emulate/x86_emulate.h
>> +++ b/xen/arch/x86/x86_emulate/x86_emulate.h
>> @@ -71,7 +71,7 @@ enum x86_swint_emulation {
>>   * Attribute for segment selector. This is a copy of bit 40:47 & 52:55 of the
>>   * segment descriptor. It happens to match the format of an AMD SVM VMCB.
>>   */
>> -typedef union __packed segment_attributes {
>> +typedef union __attribute__((packed)) segment_attributes {
>>      uint16_t bytes;
>>      struct
>>      {
>> @@ -91,7 +91,7 @@ typedef union __packed segment_attributes {
>>   * Full state of a segment register (visible and hidden portions).
>>   * Again, this happens to match the format of an AMD SVM VMCB.
>>   */
>> -struct __packed segment_register {
>> +struct __attribute__((packed)) segment_register {
>>      uint16_t   sel;
>>      segment_attributes_t attr;
>>      uint32_t   limit;
> Please use __packed__ in both cases (using the underscore free
> variant is better to be restricted to just .c files). With that
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Will do.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 for-4.9 2/2] x86/emul: Reorder the user segments in x86_segment to match SReg3 encoding

[Andrew Cooper]

On 26/10/16 09:57, Jan Beulich wrote:
>>>> On 25.10.16 at 20:10, <andrew.cooper3@citrix.com> wrote:
>> @@ -2994,8 +2978,8 @@ x86_emulate(
>>          break;
>>  
>>      case 0x8e: /* mov r/m,Sreg */
>> -        seg = decode_segment(modrm_reg);
>> -        generate_exception_if(seg == decode_segment_failed, EXC_UD, -1);
>> +        seg = modrm_reg & 7; /* REX.R is ignored. */
>> +        generate_exception_if(!is_x86_user_segment(seg), EXC_UD, -1);
>>          generate_exception_if(seg == x86_seg_cs, EXC_UD, -1);
> Can I talk you into folding these two generate_exception_if()s, as
> they logically belong together?

Ok.

>
>> @@ -5438,6 +5422,17 @@ x86_emulate(
>>  #undef override_seg
>>  #undef ea
>>  
>> +static void __init __maybe_unused build_assertions(void)
>> +{
>> +    /* Check the values against SReg3 encoding in opcode/ModRM bytes. */
>> +    BUILD_BUG_ON(x86_seg_es != 0);
>> +    BUILD_BUG_ON(x86_seg_cs != 1);
>> +    BUILD_BUG_ON(x86_seg_ss != 2);
>> +    BUILD_BUG_ON(x86_seg_ds != 3);
>> +    BUILD_BUG_ON(x86_seg_fs != 4);
>> +    BUILD_BUG_ON(x86_seg_gs != 5);
>> +}
> So using an inline function in the header did not work out?

No - that would involve making BUILD_BUG_ON() available to x86_emulate.h

In this position, it does also affect the test harness build, as it is
outside an #ifdef __XEN__ block.

>
> In any event,
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Thanks

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 for-4.9 2/2] x86/emul: Reorder the user segments in x86_segment to match SReg3 encoding

[Andrew Cooper]

On 26/10/16 10:40, Andrew Cooper wrote:
> On 26/10/16 09:57, Jan Beulich wrote:
>>>>> On 25.10.16 at 20:10, <andrew.cooper3@citrix.com> wrote:
>>> @@ -2994,8 +2978,8 @@ x86_emulate(
>>>          break;
>>>  
>>>      case 0x8e: /* mov r/m,Sreg */
>>> -        seg = decode_segment(modrm_reg);
>>> -        generate_exception_if(seg == decode_segment_failed, EXC_UD, -1);
>>> +        seg = modrm_reg & 7; /* REX.R is ignored. */
>>> +        generate_exception_if(!is_x86_user_segment(seg), EXC_UD, -1);
>>>          generate_exception_if(seg == x86_seg_cs, EXC_UD, -1);
>> Can I talk you into folding these two generate_exception_if()s, as
>> they logically belong together?
> Ok.
>
>>> @@ -5438,6 +5422,17 @@ x86_emulate(
>>>  #undef override_seg
>>>  #undef ea
>>>  
>>> +static void __init __maybe_unused build_assertions(void)
>>> +{
>>> +    /* Check the values against SReg3 encoding in opcode/ModRM bytes. */
>>> +    BUILD_BUG_ON(x86_seg_es != 0);
>>> +    BUILD_BUG_ON(x86_seg_cs != 1);
>>> +    BUILD_BUG_ON(x86_seg_ss != 2);
>>> +    BUILD_BUG_ON(x86_seg_ds != 3);
>>> +    BUILD_BUG_ON(x86_seg_fs != 4);
>>> +    BUILD_BUG_ON(x86_seg_gs != 5);
>>> +}
>> So using an inline function in the header did not work out?
> No - that would involve making BUILD_BUG_ON() available to x86_emulate.h
>
> In this position, it does also affect the test harness build, as it is
> outside an #ifdef __XEN__ block.
>
>> In any event,
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> Thanks

Wei: Turns out that I typo'd the subject, and I meant "for 4.8" here, as
this is a bugfix in the emulator.  Please can I get a view towards a
release ack?

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 for-4.9 2/2] x86/emul: Reorder the user segments in x86_segment to match SReg3 encoding

[Jan Beulich]

>>> On 26.10.16 at 11:48, <andrew.cooper3@citrix.com> wrote:
> Wei: Turns out that I typo'd the subject, and I meant "for 4.8" here, as
> this is a bugfix in the emulator.  Please can I get a view towards a
> release ack?

Oh, I didn't even realize that v2 has become a bugfix instead of mere
simplification. Somehow I had been under the impression that
decode_segment() behaved correctly. Perhaps the subject should
then be changed?

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 for-4.9 2/2] x86/emul: Reorder the user segments in x86_segment to match SReg3 encoding

[Andrew Cooper]

On 26/10/16 10:54, Jan Beulich wrote:
>>>> On 26.10.16 at 11:48, <andrew.cooper3@citrix.com> wrote:
>> Wei: Turns out that I typo'd the subject, and I meant "for 4.8" here, as
>> this is a bugfix in the emulator.  Please can I get a view towards a
>> release ack?
> Oh, I didn't even realize that v2 has become a bugfix instead of mere
> simplification. Somehow I had been under the impression that
> decode_segment() behaved correctly. Perhaps the subject should
> then be changed?

I was entirely accidentally a bugfix, but a bugfix none the less.

I am open to changing the title.  How about "Correct the decoding of
SReg3 operands"?

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 for-4.9 2/2] x86/emul: Reorder the user segments in x86_segment to match SReg3 encoding

[Jan Beulich]

>>> On 26.10.16 at 11:57, <andrew.cooper3@citrix.com> wrote:
> On 26/10/16 10:54, Jan Beulich wrote:
>>>>> On 26.10.16 at 11:48, <andrew.cooper3@citrix.com> wrote:
>>> Wei: Turns out that I typo'd the subject, and I meant "for 4.8" here, as
>>> this is a bugfix in the emulator.  Please can I get a view towards a
>>> release ack?
>> Oh, I didn't even realize that v2 has become a bugfix instead of mere
>> simplification. Somehow I had been under the impression that
>> decode_segment() behaved correctly. Perhaps the subject should
>> then be changed?
> 
> I was entirely accidentally a bugfix, but a bugfix none the less.
> 
> I am open to changing the title.  How about "Correct the decoding of
> SReg3 operands"?

SGTM, together with some adjustment to the description then (as
the original patch purpose now has become a cleanup side effect).

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 for-4.9 2/2] x86/emul: Reorder the user segments in x86_segment to match SReg3 encoding

[Andrew Cooper]

On 26/10/16 11:16, Jan Beulich wrote:
>>>> On 26.10.16 at 11:57, <andrew.cooper3@citrix.com> wrote:
>> On 26/10/16 10:54, Jan Beulich wrote:
>>>>>> On 26.10.16 at 11:48, <andrew.cooper3@citrix.com> wrote:
>>>> Wei: Turns out that I typo'd the subject, and I meant "for 4.8" here, as
>>>> this is a bugfix in the emulator.  Please can I get a view towards a
>>>> release ack?
>>> Oh, I didn't even realize that v2 has become a bugfix instead of mere
>>> simplification. Somehow I had been under the impression that
>>> decode_segment() behaved correctly. Perhaps the subject should
>>> then be changed?
>> I was entirely accidentally a bugfix, but a bugfix none the less.
>>
>> I am open to changing the title.  How about "Correct the decoding of
>> SReg3 operands"?
> SGTM, together with some adjustment to the description then (as
> the original patch purpose now has become a cleanup side effect).

Text now reads:

x86/emul: Correct the decoding of SReg3 operands

REX.R is ignored when considering segment register operands, and needs
masking
out first.

While fixing this, reorder the user segments in x86_segment to match SReg3
encoding.  This avoids needing a translation table between hardware ordering
and Xen's ordering.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH v2 for-4.9 2/2] x86/emul: Reorder the user segments in x86_segment to match SReg3 encoding

[Jan Beulich]

>>> On 26.10.16 at 13:26, <andrew.cooper3@citrix.com> wrote:
> On 26/10/16 11:16, Jan Beulich wrote:
>>>>> On 26.10.16 at 11:57, <andrew.cooper3@citrix.com> wrote:
>>> On 26/10/16 10:54, Jan Beulich wrote:
>>>>>>> On 26.10.16 at 11:48, <andrew.cooper3@citrix.com> wrote:
>>>>> Wei: Turns out that I typo'd the subject, and I meant "for 4.8" here, as
>>>>> this is a bugfix in the emulator.  Please can I get a view towards a
>>>>> release ack?
>>>> Oh, I didn't even realize that v2 has become a bugfix instead of mere
>>>> simplification. Somehow I had been under the impression that
>>>> decode_segment() behaved correctly. Perhaps the subject should
>>>> then be changed?
>>> I was entirely accidentally a bugfix, but a bugfix none the less.
>>>
>>> I am open to changing the title.  How about "Correct the decoding of
>>> SReg3 operands"?
>> SGTM, together with some adjustment to the description then (as
>> the original patch purpose now has become a cleanup side effect).
> 
> Text now reads:
> 
> x86/emul: Correct the decoding of SReg3 operands
> 
> REX.R is ignored when considering segment register operands, and needs
> masking
> out first.
> 
> While fixing this, reorder the user segments in x86_segment to match SReg3
> encoding.  This avoids needing a translation table between hardware ordering
> and Xen's ordering.

Thanks, sounds good.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel