[Xen-devel] [PATCH for-4.11] x86/traps: Improve code generation for set_ist()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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 <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Juergen Gross <jgross@xxxxxxxx>

I'd like this to be considered for 4.11 at this point, but it won't be the end
of the world if it is delayed until 4.12
---
 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 4093c65..58efc1d 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 db9988a..1bc2f90 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)
-- 
2.1.4


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