* [PATCH v2 for-4.9 1/2] x86/emul: Use explicit __attribute__((packed)) rather than __packed
@ 2016-10-25 18:10 Andrew Cooper
2016-10-25 18:10 ` [PATCH v2 for-4.9 2/2] x86/emul: Reorder the user segments in x86_segment to match SReg3 encoding Andrew Cooper
2016-10-26 8:54 ` [PATCH v2 for-4.9 1/2] x86/emul: Use explicit __attribute__((packed)) rather than __packed Jan Beulich
0 siblings, 2 replies; 12+ messages in thread
From: Andrew Cooper @ 2016-10-25 18:10 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v2 for-4.9 2/2] x86/emul: Reorder the user segments in x86_segment to match SReg3 encoding
2016-10-25 18:10 [PATCH v2 for-4.9 1/2] x86/emul: Use explicit __attribute__((packed)) rather than __packed Andrew Cooper
@ 2016-10-25 18:10 ` Andrew Cooper
2016-10-26 8:57 ` Jan Beulich
2016-10-26 8:54 ` [PATCH v2 for-4.9 1/2] x86/emul: Use explicit __attribute__((packed)) rather than __packed Jan Beulich
1 sibling, 1 reply; 12+ messages in thread
From: Andrew Cooper @ 2016-10-25 18:10 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v2 for-4.9 1/2] x86/emul: Use explicit __attribute__((packed)) rather than __packed
2016-10-25 18:10 [PATCH v2 for-4.9 1/2] x86/emul: Use explicit __attribute__((packed)) rather than __packed Andrew Cooper
2016-10-25 18:10 ` [PATCH v2 for-4.9 2/2] x86/emul: Reorder the user segments in x86_segment to match SReg3 encoding Andrew Cooper
@ 2016-10-26 8:54 ` Jan Beulich
2016-10-26 9:39 ` Andrew Cooper
1 sibling, 1 reply; 12+ messages in thread
From: Jan Beulich @ 2016-10-26 8:54 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Wei Liu, Xen-devel
>>> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 for-4.9 2/2] x86/emul: Reorder the user segments in x86_segment to match SReg3 encoding
2016-10-25 18:10 ` [PATCH v2 for-4.9 2/2] x86/emul: Reorder the user segments in x86_segment to match SReg3 encoding Andrew Cooper
@ 2016-10-26 8:57 ` Jan Beulich
2016-10-26 9:40 ` Andrew Cooper
0 siblings, 1 reply; 12+ messages in thread
From: Jan Beulich @ 2016-10-26 8:57 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Wei Liu, Xen-devel
>>> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 for-4.9 1/2] x86/emul: Use explicit __attribute__((packed)) rather than __packed
2016-10-26 8:54 ` [PATCH v2 for-4.9 1/2] x86/emul: Use explicit __attribute__((packed)) rather than __packed Jan Beulich
@ 2016-10-26 9:39 ` Andrew Cooper
0 siblings, 0 replies; 12+ messages in thread
From: Andrew Cooper @ 2016-10-26 9:39 UTC (permalink / raw)
To: Jan Beulich; +Cc: Wei Liu, Xen-devel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 for-4.9 2/2] x86/emul: Reorder the user segments in x86_segment to match SReg3 encoding
2016-10-26 8:57 ` Jan Beulich
@ 2016-10-26 9:40 ` Andrew Cooper
2016-10-26 9:48 ` Andrew Cooper
0 siblings, 1 reply; 12+ messages in thread
From: Andrew Cooper @ 2016-10-26 9:40 UTC (permalink / raw)
To: Jan Beulich; +Cc: Wei Liu, Xen-devel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 for-4.9 2/2] x86/emul: Reorder the user segments in x86_segment to match SReg3 encoding
2016-10-26 9:40 ` Andrew Cooper
@ 2016-10-26 9:48 ` Andrew Cooper
2016-10-26 9:54 ` Jan Beulich
0 siblings, 1 reply; 12+ messages in thread
From: Andrew Cooper @ 2016-10-26 9:48 UTC (permalink / raw)
To: Wei Liu; +Cc: Jan Beulich, Xen-devel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 for-4.9 2/2] x86/emul: Reorder the user segments in x86_segment to match SReg3 encoding
2016-10-26 9:48 ` Andrew Cooper
@ 2016-10-26 9:54 ` Jan Beulich
2016-10-26 9:57 ` Andrew Cooper
0 siblings, 1 reply; 12+ messages in thread
From: Jan Beulich @ 2016-10-26 9:54 UTC (permalink / raw)
To: Andrew Cooper, Wei Liu; +Cc: Xen-devel
>>> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 for-4.9 2/2] x86/emul: Reorder the user segments in x86_segment to match SReg3 encoding
2016-10-26 9:54 ` Jan Beulich
@ 2016-10-26 9:57 ` Andrew Cooper
2016-10-26 10:16 ` Jan Beulich
0 siblings, 1 reply; 12+ messages in thread
From: Andrew Cooper @ 2016-10-26 9:57 UTC (permalink / raw)
To: Jan Beulich, Wei Liu; +Cc: Xen-devel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 for-4.9 2/2] x86/emul: Reorder the user segments in x86_segment to match SReg3 encoding
2016-10-26 9:57 ` Andrew Cooper
@ 2016-10-26 10:16 ` Jan Beulich
2016-10-26 11:26 ` Andrew Cooper
0 siblings, 1 reply; 12+ messages in thread
From: Jan Beulich @ 2016-10-26 10:16 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Wei Liu, Xen-devel
>>> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 for-4.9 2/2] x86/emul: Reorder the user segments in x86_segment to match SReg3 encoding
2016-10-26 10:16 ` Jan Beulich
@ 2016-10-26 11:26 ` Andrew Cooper
2016-10-26 12:19 ` Jan Beulich
0 siblings, 1 reply; 12+ messages in thread
From: Andrew Cooper @ 2016-10-26 11:26 UTC (permalink / raw)
To: Jan Beulich; +Cc: Wei Liu, Xen-devel
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2 for-4.9 2/2] x86/emul: Reorder the user segments in x86_segment to match SReg3 encoding
2016-10-26 11:26 ` Andrew Cooper
@ 2016-10-26 12:19 ` Jan Beulich
0 siblings, 0 replies; 12+ messages in thread
From: Jan Beulich @ 2016-10-26 12:19 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Wei Liu, Xen-devel
>>> 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
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-10-26 12:19 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-25 18:10 [PATCH v2 for-4.9 1/2] x86/emul: Use explicit __attribute__((packed)) rather than __packed Andrew Cooper
2016-10-25 18:10 ` [PATCH v2 for-4.9 2/2] x86/emul: Reorder the user segments in x86_segment to match SReg3 encoding Andrew Cooper
2016-10-26 8:57 ` Jan Beulich
2016-10-26 9:40 ` Andrew Cooper
2016-10-26 9:48 ` Andrew Cooper
2016-10-26 9:54 ` Jan Beulich
2016-10-26 9:57 ` Andrew Cooper
2016-10-26 10:16 ` Jan Beulich
2016-10-26 11:26 ` Andrew Cooper
2016-10-26 12:19 ` Jan Beulich
2016-10-26 8:54 ` [PATCH v2 for-4.9 1/2] x86/emul: Use explicit __attribute__((packed)) rather than __packed Jan Beulich
2016-10-26 9:39 ` Andrew Cooper
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.