[Xen-devel] [PATCH] x86emul: correct segment override decode for 64-bit mode

[Xen-devel] [PATCH] x86emul: correct segment override decode for 64-bit mode

[Jan Beulich]

The legacy / compatibility mode ES, CS, SS, and DS overrides are null
prefixes in 64-bit mode, i.e. they in particular don't cancel an
earlier FS or GS one.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -2830,14 +2830,17 @@ x86_decode(
         case 0x67: /* address-size override */
             ad_bytes = def_ad_bytes ^ (mode_64bit() ? 12 : 6);
             break;
-        case 0x2e: /* CS override */
-            override_seg = x86_seg_cs;
+        case 0x2e: /* CS override / null prefix in 64-bit mode */
+            if ( !mode_64bit() )
+                override_seg = x86_seg_cs;
             break;
-        case 0x3e: /* DS override */
-            override_seg = x86_seg_ds;
+        case 0x3e: /* DS override / null prefix in 64-bit mode */
+            if ( !mode_64bit() )
+                override_seg = x86_seg_ds;
             break;
-        case 0x26: /* ES override */
-            override_seg = x86_seg_es;
+        case 0x26: /* ES override / null prefix in 64-bit mode */
+            if ( !mode_64bit() )
+                override_seg = x86_seg_es;
             break;
         case 0x64: /* FS override */
             override_seg = x86_seg_fs;
@@ -2845,8 +2848,9 @@ x86_decode(
         case 0x65: /* GS override */
             override_seg = x86_seg_gs;
             break;
-        case 0x36: /* SS override */
-            override_seg = x86_seg_ss;
+        case 0x36: /* SS override / null prefix in 64-bit mode */
+            if ( !mode_64bit() )
+                override_seg = x86_seg_ss;
             break;
         case 0xf0: /* LOCK */
             lock_prefix = 1;
@@ -2871,10 +2875,6 @@ x86_decode(
     }
  done_prefixes:
 
-    /* %{e,c,s,d}s overrides are ignored in 64bit mode. */
-    if ( mode_64bit() && override_seg < x86_seg_fs )
-        override_seg = x86_seg_none;
-
     if ( rex_prefix & REX_W )
         op_bytes = 8;
 

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

Re: [Xen-devel] [PATCH] x86emul: correct segment override decode for 64-bit mode

[Andrew Cooper]

On 11/12/2019 09:27, Jan Beulich wrote:
> The legacy / compatibility mode ES, CS, SS, and DS overrides are null
> prefixes in 64-bit mode, i.e. they in particular don't cancel an
> earlier FS or GS one.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

null is a very overloaded term.  What you mean here is simply "ignored".

In attempting to confirm/test this, I've found yet another curiosity
with instruction length calculations when reordering a rex prefix and
legacy prefix.  Objdump gets it wrong, but the instruction boundaries
according to singlestep are weird.

~Andrew

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

Re: [Xen-devel] [PATCH] x86emul: correct segment override decode for 64-bit mode

[Jan Beulich]

On 11.12.2019 21:51, Andrew Cooper wrote:
> On 11/12/2019 09:27, Jan Beulich wrote:
>> The legacy / compatibility mode ES, CS, SS, and DS overrides are null
>> prefixes in 64-bit mode, i.e. they in particular don't cancel an
>> earlier FS or GS one.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> null is a very overloaded term.  What you mean here is simply "ignored".

The AMD PM has "Instead, they are treated as null prefixes." This is
what I've taken to use here. I'm happy to take whatever other
sensible wording you like better (including "ignored"). But I'd like
you to explicitly clarify that you're not okay with me using a term
from vendor documentation here.

> In attempting to confirm/test this, I've found yet another curiosity
> with instruction length calculations when reordering a rex prefix and
> legacy prefix.  Objdump gets it wrong, but the instruction boundaries
> according to singlestep are weird.

Objdump getting it wrong is no surprise at all to me (which is one
of the reasons why I prefer to use my own disassembler wherever
possible). Yet without you spelling out what specific anomalies
you've observed (or what weirdness there is with single stepping)
I won't know whether I may want to make an attempt at fixing
objdump. Nor can I see what this comment's implication is on the
patch here, i.e. what changes you mean me to make.

Jan

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

Re: [Xen-devel] [PATCH] x86emul: correct segment override decode for 64-bit mode

[Andrew Cooper]

On 12/12/2019 10:04, Jan Beulich wrote:
> On 11.12.2019 21:51, Andrew Cooper wrote:
>> On 11/12/2019 09:27, Jan Beulich wrote:
>>> The legacy / compatibility mode ES, CS, SS, and DS overrides are null
>>> prefixes in 64-bit mode, i.e. they in particular don't cancel an
>>> earlier FS or GS one.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> null is a very overloaded term.  What you mean here is simply "ignored".
> The AMD PM has "Instead, they are treated as null prefixes." This is
> what I've taken to use here. I'm happy to take whatever other
> sensible wording you like better (including "ignored"). But I'd like
> you to explicitly clarify that you're not okay with me using a term
> from vendor documentation here.

"Ignored" is the more descriptive term, matches 2 different parts of the
APM, and most importantly, more obviously matches the code.

I can't even spot mention of this behaviour in the SDM.

>
>> In attempting to confirm/test this, I've found yet another curiosity
>> with instruction length calculations when reordering a rex prefix and
>> legacy prefix.  Objdump gets it wrong, but the instruction boundaries
>> according to singlestep are weird.
> Objdump getting it wrong is no surprise at all to me (which is one
> of the reasons why I prefer to use my own disassembler wherever
> possible). Yet without you spelling out what specific anomalies
> you've observed (or what weirdness there is with single stepping)
> I won't know whether I may want to make an attempt at fixing
> objdump. Nor can I see what this comment's implication is on the
> patch here, i.e. what changes you mean me to make.

The sequence in question is:

1048a1:    48                       rex.W
1048a2:    2e 8b 32                 mov    %cs:(%rdx),%esi

which was deliberately permuting the rex and %cs prefix to see what
happened.

The instruction boundary issue was a mistake in my code and with it
fixed, both Intel and AMD processors agree that the above 4 bytes is a
single instruction with 32bit operand size.  x86_emulate() also agrees,
which was the point of the test.

As I've resolved the instruction length ambiguity, Acked/Tested-by:
Andrew Cooper <andrew.cooper3@citrix.com>

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