| Message ID | 20230220194702.2260181-3-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/irq: Perf improvements | expand |
On 20.02.2023 20:47, Andrew Cooper wrote: > --- a/xen/arch/x86/include/asm/system.h > +++ b/xen/arch/x86/include/asm/system.h > @@ -267,13 +267,8 @@ static inline unsigned long array_index_mask_nospec(unsigned long index, > }) > #define local_irq_restore(x) \ > ({ \ > - BUILD_BUG_ON(sizeof(x) != sizeof(long)); \ > - asm volatile ( "pushfq\n\t" \ > - "andq %0, (%%rsp)\n\t" \ > - "orq %1, (%%rsp)\n\t" \ > - "popfq" \ > - : : "i?r" ( ~X86_EFLAGS_IF ), \ > - "ri" ( (x) & X86_EFLAGS_IF ) ); \ > + if ( (x) & X86_EFLAGS_IF ) \ > + local_irq_enable(); \ > }) Without it being written down anywhere that IRQs cannot be turned off this way, and without there being a reference to that documentation in the description, this is introducing a plain bug; I'm sorry to say it that way. With both of the above fulfilled I'd of course be happy to see the improvement take effect. Jan
diff --git a/xen/arch/x86/include/asm/system.h b/xen/arch/x86/include/asm/system.h index 65e63de69a67..4be235472ecd 100644 --- a/xen/arch/x86/include/asm/system.h +++ b/xen/arch/x86/include/asm/system.h @@ -267,13 +267,8 @@ static inline unsigned long array_index_mask_nospec(unsigned long index, }) #define local_irq_restore(x) \ ({ \ - BUILD_BUG_ON(sizeof(x) != sizeof(long)); \ - asm volatile ( "pushfq\n\t" \ - "andq %0, (%%rsp)\n\t" \ - "orq %1, (%%rsp)\n\t" \ - "popfq" \ - : : "i?r" ( ~X86_EFLAGS_IF ), \ - "ri" ( (x) & X86_EFLAGS_IF ) ); \ + if ( (x) & X86_EFLAGS_IF ) \ + local_irq_enable(); \ }) static inline int local_irq_is_enabled(void)
POPF is a horribly expensive instruction, while STI is an optimised fastpath. Switching POPF for a conditional branch and STI caused an 8% perf improvement in various linux measurements. While I don't expect the change to be that dramatic in Xen, there will be an improvement. Furthermore, there is the following code generation improvement: add/remove: 0/0 grow/shrink: 3/52 up/down: 52/-966 (-914) owing to not needing to opencode the restriction to just IF in asm. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> CC: Wei Liu <wl@xen.org> --- xen/arch/x86/include/asm/system.h | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)