[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] x86/idt: Minor improvements to _update_gate_addr_lower()



After some experimentation, using .a/.b makes far better logic than using the
named fields, as both Clang and GCC spill idte to the stack when named fields
are used.

GCC seems to do something very daft for the addr1 field.  It takes addr,
shifts it by 32, then ANDs with 0xffff0000000000000UL, which requires
manifesting a MOVABS.

Clang follows the C, whereby it ANDs with $imm32, then shifts, avoiding the
MOVABS entirely.

No functional change.

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

I'm disappointed about how poor the code generation is when assigning to named
fields, but I suppose it is a harder problem for the compiler to figure out.

add/remove: 0/0 grow/shrink: 0/2 up/down: 0/-24 (-24)
Function                                     old     new   delta
machine_kexec                                356     348      -8
traps_init                                   434     418     -16
---
 xen/arch/x86/include/asm/idt.h | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/include/asm/idt.h b/xen/arch/x86/include/asm/idt.h
index f613d5693e0e..b5e570a77fae 100644
--- a/xen/arch/x86/include/asm/idt.h
+++ b/xen/arch/x86/include/asm/idt.h
@@ -92,15 +92,16 @@ static inline void _set_gate_lower(idt_entry_t *gate, 
unsigned long type,
  * Update the lower half handler of an IDT entry, without changing any other
  * configuration.
  */
-static inline void _update_gate_addr_lower(idt_entry_t *gate, void *addr)
+static inline void _update_gate_addr_lower(idt_entry_t *gate, void *_addr)
 {
+    unsigned long addr = (unsigned long)_addr;
+    unsigned int addr1 = addr & 0xffff0000U; /* GCC force better codegen. */
     idt_entry_t idte;
-    idte.a = gate->a;
 
-    idte.b = ((unsigned long)(addr) >> 32);
-    idte.a &= 0x0000FFFFFFFF0000ULL;
-    idte.a |= (((unsigned long)(addr) & 0xFFFF0000UL) << 32) |
-        ((unsigned long)(addr) & 0xFFFFUL);
+    idte.b = addr >> 32;
+    idte.a = gate->a & 0x0000ffffffff0000UL;
+    idte.a |= (unsigned long)addr1 << 32;
+    idte.a |= addr & 0xffff;
 
     _write_gate_lower(gate, &idte);
 }
-- 
2.39.5




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.