[xen master] x86/traps: Improve code generation for set_ist()

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

[xen master] x86/traps: Improve code generation for set_ist()

patchbot
commit 5b8d8a19228b818d8e4b62ae1b9b5236e9fdbdaa
Author:     Andrew Cooper <[hidden email]>
AuthorDate: Thu Mar 22 11:31:07 2018 +0000
Commit:     Andrew Cooper <[hidden email]>
CommitDate: Fri Apr 27 15:18:36 2018 +0100

    x86/traps: Improve code generation for set_ist()
   
    The IST field in an IDT entry is a 3 bit field, with 5 adjacent reserved bits
    which we always write as zero.  By expressing this as a byte field in a union,
    we turn an invocation of enable_each_ist() from
   
      4b 8b 14 d3                     mov    (%r11,%r10,8),%rdx
      48 b8 ff ff ff ff f8 ff ff ff   movabs $0xfffffff8ffffffff,%rax
      48 be 00 00 00 00 01 00 00 00   movabs $0x100000000,%rsi
      48 8b 8a 80 00 00 00            mov    0x80(%rdx),%rcx
      48 21 c1                        and    %rax,%rcx
      48 09 f1                        or     %rsi,%rcx
      48 be 00 00 00 00 02 00 00 00   movabs $0x200000000,%rsi
      48 89 8a 80 00 00 00            mov    %rcx,0x80(%rdx)
      48 8b 4a 20                     mov    0x20(%rdx),%rcx
      48 21 c1                        and    %rax,%rcx
      48 23 82 20 01 00 00            and    0x120(%rdx),%rax
      48 09 f1                        or     %rsi,%rcx
      48 89 4a 20                     mov    %rcx,0x20(%rdx)
      48 b9 00 00 00 00 03 00 00 00   movabs $0x300000000,%rcx
      48 09 c8                        or     %rcx,%rax
      48 89 82 20 01 00 00            mov    %rax,0x120(%rdx)
   
    into
   
      4b 8b 04 d3                     mov    (%r11,%r10,8),%rax
      c6 80 84 00 00 00 01            movb   $0x1,0x84(%rax)
      c6 40 24 02                     movb   $0x2,0x24(%rax)
      c6 80 24 01 00 00 03            movb   $0x3,0x124(%rax)
   
    which is far more simple.  As the IDT is typically live, this is more
    obviously safe.
   
    The net delta for this change is:
   
      add/remove: 0/0 grow/shrink: 0/7 up/down: 0/-334 (-334)
   
    While making changes here, tidy up the set_ist() declaration.  Drop the
    always_inline (I don't recall why I wrote it like that originally) and the ist
    parameter need not be unsigned long (although it will be const-propagated in
    practice).
   
    Signed-off-by: Andrew Cooper <[hidden email]>
    Reviewed-by: Jan Beulich <[hidden email]>
    Release-acked-by: Juergen Gross <[hidden email]>
---
 xen/include/asm-x86/desc.h      | 15 +++++++++++++--
 xen/include/asm-x86/processor.h | 12 +++++-------
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/xen/include/asm-x86/desc.h b/xen/include/asm-x86/desc.h
index 4093c65faa..58efc1d896 100644
--- a/xen/include/asm-x86/desc.h
+++ b/xen/include/asm-x86/desc.h
@@ -106,8 +106,19 @@ struct desc_struct {
     u32 a, b;
 };
 
-typedef struct {
-    u64 a, b;
+typedef union {
+    struct {
+        uint64_t a, b;
+    };
+    struct {
+        uint16_t addr0;
+        uint16_t cs;
+        uint8_t  ist; /* :3, 5 bits rsvd, but this yields far better code. */
+        uint8_t  type:4, s:1, dpl:2, p:1;
+        uint16_t addr1;
+        uint32_t addr2;
+        /* 32 bits rsvd. */
+    };
 } idt_entry_t;
 
 /* Write the lower 64 bits of an IDT Entry. This relies on the upper 32
diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index db9988ab33..1bc2f90d04 100644
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -406,16 +406,14 @@ struct __packed __cacheline_aligned tss_struct {
 #define IST_MCE  3UL
 #define IST_MAX  3UL
 
-/* Set the interrupt stack table used by a particular interrupt
- * descriptor table entry. */
-static always_inline void set_ist(idt_entry_t *idt, unsigned long ist)
+/* Set the Interrupt Stack Table used by a particular IDT entry. */
+static inline void set_ist(idt_entry_t *idt, unsigned int ist)
 {
-    idt_entry_t new = *idt;
-
     /* IST is a 3 bit field, 32 bits into the IDT entry. */
     ASSERT(ist <= IST_MAX);
-    new.a = (idt->a & ~(7UL << 32)) | (ist << 32);
-    _write_gate_lower(idt, &new);
+
+    /* Typically used on a live idt.  Disuade any clever optimisations. */
+    ACCESS_ONCE(idt->ist) = ist;
 }
 
 static inline void enable_each_ist(idt_entry_t *idt)
--
generated by git-patchbot for /home/xen/git/xen.git#master

_______________________________________________
Xen-changelog mailing list
[hidden email]
https://lists.xenproject.org/xen-changelog