diff mbox series

x86/emul: Simplify segment override prefix decoding

Message ID 20240411152314.1755561-1-andrew.cooper3@citrix.com (mailing list archive)
State New
Headers show
Series x86/emul: Simplify segment override prefix decoding | expand

Commit Message

Andrew Cooper April 11, 2024, 3:23 p.m. UTC
x86_seg_* uses architectural encodings.  Therefore, we can fold the prefix
handling cases together and derive the segment from the prefix byte itself.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>

This makes a marginal saving of 47 bytes, indicating (not unexpectedly) that
the optimiser cannot fold the case statements automatically.

fs/gs is weirder.  The expression is 4 + (b & 1), and for some reason this
adds +264 bytes to the function.  Even if the logical expression is larger
than two simple stores (and it probably is), it's not +264 bytes larger...
---
 xen/arch/x86/x86_emulate/decode.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

Comments

Jan Beulich April 18, 2024, 10:27 a.m. UTC | #1
On 11.04.2024 17:23, Andrew Cooper wrote:
> x86_seg_* uses architectural encodings.  Therefore, we can fold the prefix
> handling cases together and derive the segment from the prefix byte itself.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

I notice we already have suitable BUILD_BUG_ON()s, as we use similar logic
already for PUSH/POP of the selector registers.

Jan
diff mbox series

Patch

diff --git a/xen/arch/x86/x86_emulate/decode.c b/xen/arch/x86/x86_emulate/decode.c
index de836068fdd8..ee4cbdc0002c 100644
--- a/xen/arch/x86/x86_emulate/decode.c
+++ b/xen/arch/x86/x86_emulate/decode.c
@@ -1043,17 +1043,12 @@  int x86emul_decode(struct x86_emulate_state *s,
         case 0x67: /* address-size override */
             ad_bytes = def_ad_bytes ^ (mode_64bit() ? 12 : 6);
             break;
-        case 0x2e: /* CS override / ignored in 64-bit mode */
+        case 0x26: /* ES override */
+        case 0x2e: /* CS override */
+        case 0x36: /* SS override */
+        case 0x3e: /* DS override, all ignored in 64-bit mode */
             if ( !mode_64bit() )
-                override_seg = x86_seg_cs;
-            break;
-        case 0x3e: /* DS override / ignored in 64-bit mode */
-            if ( !mode_64bit() )
-                override_seg = x86_seg_ds;
-            break;
-        case 0x26: /* ES override / ignored in 64-bit mode */
-            if ( !mode_64bit() )
-                override_seg = x86_seg_es;
+                override_seg = (b >> 3) & 3;
             break;
         case 0x64: /* FS override */
             override_seg = x86_seg_fs;
@@ -1061,10 +1056,6 @@  int x86emul_decode(struct x86_emulate_state *s,
         case 0x65: /* GS override */
             override_seg = x86_seg_gs;
             break;
-        case 0x36: /* SS override / ignored in 64-bit mode */
-            if ( !mode_64bit() )
-                override_seg = x86_seg_ss;
-            break;
         case 0xf0: /* LOCK */
             s->lock_prefix = true;
             break;