From: Thomas Gleixner <tglx@linutronix.de> To: Mark Rutland <mark.rutland@arm.com> Cc: Peter Zijlstra <peterz@infradead.org>, Juergen Gross <jgross@suse.com>, xen-devel@lists.xenproject.org, x86@kernel.org, linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org, luto@kernel.org, Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>, "H. Peter Anvin" <hpa@zytor.com>, Deep Shah <sdeep@vmware.com>, "VMware\, Inc." <pv-drivers@vmware.com>, Boris Ostrovsky <boris.ostrovsky@oracle.com>, Stefano Stabellini <sstabellini@kernel.org> Subject: x86/ioapic: Cleanup the timer_works() irqflags mess Date: Thu, 10 Dec 2020 21:15:04 +0100 [thread overview] Message-ID: <87k0tpju47.fsf@nanos.tec.linutronix.de> (raw) In-Reply-To: <20201210111008.GB88655@C02TD0UTHF1T.local> Mark tripped over the creative irqflags handling in the IO-APIC timer delivery check which ends up doing: local_irq_save(flags); local_irq_enable(); local_irq_restore(flags); which triggered a new consistency check he's working on required for replacing the POPF based restore with a conditional STI. That code is a historical mess and none of this is needed. Make it straightforward use local_irq_disable()/enable() as that's all what is required. It is invoked from interrupt enabled code nowadays. Reported-by: Mark Rutland <mark.rutland@arm.com> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Tested-by: Mark Rutland <mark.rutland@arm.com> --- arch/x86/kernel/apic/io_apic.c | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) --- a/arch/x86/kernel/apic/io_apic.c +++ b/arch/x86/kernel/apic/io_apic.c @@ -1618,21 +1618,16 @@ static void __init delay_without_tsc(voi static int __init timer_irq_works(void) { unsigned long t1 = jiffies; - unsigned long flags; if (no_timer_check) return 1; - local_save_flags(flags); local_irq_enable(); - if (boot_cpu_has(X86_FEATURE_TSC)) delay_with_tsc(); else delay_without_tsc(); - local_irq_restore(flags); - /* * Expect a few ticks at least, to be sure some possible * glue logic does not lock up after one or two first @@ -1641,10 +1636,10 @@ static int __init timer_irq_works(void) * least one tick may be lost due to delays. */ - /* jiffies wrap? */ - if (time_after(jiffies, t1 + 4)) - return 1; - return 0; + local_irq_disable(); + + /* Did jiffies advance? */ + return time_after(jiffies, t1 + 4); } /* @@ -2117,13 +2112,12 @@ static inline void __init check_timer(vo struct irq_cfg *cfg = irqd_cfg(irq_data); int node = cpu_to_node(0); int apic1, pin1, apic2, pin2; - unsigned long flags; int no_pin1 = 0; if (!global_clock_event) return; - local_irq_save(flags); + local_irq_disable(); /* * get/set the timer IRQ vector: @@ -2191,7 +2185,6 @@ static inline void __init check_timer(vo goto out; } panic_if_irq_remap("timer doesn't work through Interrupt-remapped IO-APIC"); - local_irq_disable(); clear_IO_APIC_pin(apic1, pin1); if (!no_pin1) apic_printk(APIC_QUIET, KERN_ERR "..MP-BIOS bug: " @@ -2215,7 +2208,6 @@ static inline void __init check_timer(vo /* * Cleanup, just in case ... */ - local_irq_disable(); legacy_pic->mask(0); clear_IO_APIC_pin(apic2, pin2); apic_printk(APIC_QUIET, KERN_INFO "....... failed.\n"); @@ -2232,7 +2224,6 @@ static inline void __init check_timer(vo apic_printk(APIC_QUIET, KERN_INFO "..... works.\n"); goto out; } - local_irq_disable(); legacy_pic->mask(0); apic_write(APIC_LVT0, APIC_LVT_MASKED | APIC_DM_FIXED | cfg->vector); apic_printk(APIC_QUIET, KERN_INFO "..... failed.\n"); @@ -2251,7 +2242,6 @@ static inline void __init check_timer(vo apic_printk(APIC_QUIET, KERN_INFO "..... works.\n"); goto out; } - local_irq_disable(); apic_printk(APIC_QUIET, KERN_INFO "..... failed :(.\n"); if (apic_is_x2apic_enabled()) apic_printk(APIC_QUIET, KERN_INFO @@ -2260,7 +2250,7 @@ static inline void __init check_timer(vo panic("IO-APIC + timer doesn't work! Boot with apic=debug and send a " "report. Then try booting with the 'noapic' option.\n"); out: - local_irq_restore(flags); + local_irq_enable(); } /*
WARNING: multiple messages have this Message-ID (diff)
From: Thomas Gleixner <tglx@linutronix.de> To: Mark Rutland <mark.rutland@arm.com> Cc: Juergen Gross <jgross@suse.com>, Stefano Stabellini <sstabellini@kernel.org>, Peter Zijlstra <peterz@infradead.org>, x86@kernel.org, linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org, "VMware, Inc." <pv-drivers@vmware.com>, Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>, luto@kernel.org, "H. Peter Anvin" <hpa@zytor.com>, xen-devel@lists.xenproject.org, Boris Ostrovsky <boris.ostrovsky@oracle.com> Subject: x86/ioapic: Cleanup the timer_works() irqflags mess Date: Thu, 10 Dec 2020 21:15:04 +0100 [thread overview] Message-ID: <87k0tpju47.fsf@nanos.tec.linutronix.de> (raw) In-Reply-To: <20201210111008.GB88655@C02TD0UTHF1T.local> Mark tripped over the creative irqflags handling in the IO-APIC timer delivery check which ends up doing: local_irq_save(flags); local_irq_enable(); local_irq_restore(flags); which triggered a new consistency check he's working on required for replacing the POPF based restore with a conditional STI. That code is a historical mess and none of this is needed. Make it straightforward use local_irq_disable()/enable() as that's all what is required. It is invoked from interrupt enabled code nowadays. Reported-by: Mark Rutland <mark.rutland@arm.com> Signed-off-by: Thomas Gleixner <tglx@linutronix.de> Tested-by: Mark Rutland <mark.rutland@arm.com> --- arch/x86/kernel/apic/io_apic.c | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) --- a/arch/x86/kernel/apic/io_apic.c +++ b/arch/x86/kernel/apic/io_apic.c @@ -1618,21 +1618,16 @@ static void __init delay_without_tsc(voi static int __init timer_irq_works(void) { unsigned long t1 = jiffies; - unsigned long flags; if (no_timer_check) return 1; - local_save_flags(flags); local_irq_enable(); - if (boot_cpu_has(X86_FEATURE_TSC)) delay_with_tsc(); else delay_without_tsc(); - local_irq_restore(flags); - /* * Expect a few ticks at least, to be sure some possible * glue logic does not lock up after one or two first @@ -1641,10 +1636,10 @@ static int __init timer_irq_works(void) * least one tick may be lost due to delays. */ - /* jiffies wrap? */ - if (time_after(jiffies, t1 + 4)) - return 1; - return 0; + local_irq_disable(); + + /* Did jiffies advance? */ + return time_after(jiffies, t1 + 4); } /* @@ -2117,13 +2112,12 @@ static inline void __init check_timer(vo struct irq_cfg *cfg = irqd_cfg(irq_data); int node = cpu_to_node(0); int apic1, pin1, apic2, pin2; - unsigned long flags; int no_pin1 = 0; if (!global_clock_event) return; - local_irq_save(flags); + local_irq_disable(); /* * get/set the timer IRQ vector: @@ -2191,7 +2185,6 @@ static inline void __init check_timer(vo goto out; } panic_if_irq_remap("timer doesn't work through Interrupt-remapped IO-APIC"); - local_irq_disable(); clear_IO_APIC_pin(apic1, pin1); if (!no_pin1) apic_printk(APIC_QUIET, KERN_ERR "..MP-BIOS bug: " @@ -2215,7 +2208,6 @@ static inline void __init check_timer(vo /* * Cleanup, just in case ... */ - local_irq_disable(); legacy_pic->mask(0); clear_IO_APIC_pin(apic2, pin2); apic_printk(APIC_QUIET, KERN_INFO "....... failed.\n"); @@ -2232,7 +2224,6 @@ static inline void __init check_timer(vo apic_printk(APIC_QUIET, KERN_INFO "..... works.\n"); goto out; } - local_irq_disable(); legacy_pic->mask(0); apic_write(APIC_LVT0, APIC_LVT_MASKED | APIC_DM_FIXED | cfg->vector); apic_printk(APIC_QUIET, KERN_INFO "..... failed.\n"); @@ -2251,7 +2242,6 @@ static inline void __init check_timer(vo apic_printk(APIC_QUIET, KERN_INFO "..... works.\n"); goto out; } - local_irq_disable(); apic_printk(APIC_QUIET, KERN_INFO "..... failed :(.\n"); if (apic_is_x2apic_enabled()) apic_printk(APIC_QUIET, KERN_INFO @@ -2260,7 +2250,7 @@ static inline void __init check_timer(vo panic("IO-APIC + timer doesn't work! Boot with apic=debug and send a " "report. Then try booting with the 'noapic' option.\n"); out: - local_irq_restore(flags); + local_irq_enable(); } /* _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
next prev parent reply other threads:[~2020-12-10 20:16 UTC|newest] Thread overview: 107+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-11-20 11:46 [PATCH v2 00/12] x86: major paravirt cleanup Juergen Gross 2020-11-20 11:46 ` Juergen Gross via Virtualization 2020-11-20 11:46 ` [PATCH v2 01/12] x86/xen: use specific Xen pv interrupt entry for MCE Juergen Gross 2020-12-09 21:03 ` Thomas Gleixner 2020-11-20 11:46 ` [PATCH v2 02/12] x86/xen: use specific Xen pv interrupt entry for DF Juergen Gross 2020-12-09 21:03 ` Thomas Gleixner 2020-11-20 11:46 ` [PATCH v2 03/12] x86/pv: switch SWAPGS to ALTERNATIVE Juergen Gross 2020-11-20 11:46 ` Juergen Gross via Virtualization 2020-11-27 11:31 ` Borislav Petkov 2020-11-27 11:31 ` Borislav Petkov 2020-12-09 21:07 ` Thomas Gleixner 2020-12-09 21:07 ` Thomas Gleixner 2020-11-20 11:46 ` [PATCH v2 04/12] x86/xen: drop USERGS_SYSRET64 paravirt call Juergen Gross 2020-11-20 11:46 ` Juergen Gross via Virtualization 2020-12-02 12:32 ` Borislav Petkov 2020-12-02 12:32 ` Borislav Petkov 2020-12-02 14:48 ` Jürgen Groß 2020-12-02 14:48 ` Jürgen Groß via Virtualization 2020-12-02 17:08 ` Borislav Petkov 2020-12-02 17:08 ` Borislav Petkov 2020-11-20 11:46 ` [PATCH v2 05/12] x86: rework arch_local_irq_restore() to not use popf Juergen Gross 2020-11-20 11:46 ` Juergen Gross via Virtualization 2020-11-20 11:59 ` Peter Zijlstra 2020-11-20 11:59 ` Peter Zijlstra 2020-11-20 12:05 ` Jürgen Groß 2020-11-20 12:05 ` Jürgen Groß via Virtualization 2020-11-22 6:55 ` Jürgen Groß 2020-11-22 6:55 ` Jürgen Groß via Virtualization 2020-11-22 21:44 ` Andy Lutomirski 2020-11-22 21:44 ` Andy Lutomirski 2020-11-22 21:44 ` Andy Lutomirski 2020-11-23 5:21 ` Jürgen Groß 2020-11-23 5:21 ` Jürgen Groß via Virtualization 2020-11-23 15:15 ` Andy Lutomirski 2020-11-23 15:15 ` Andy Lutomirski 2020-12-09 13:27 ` Mark Rutland 2020-12-09 13:27 ` Mark Rutland 2020-12-09 14:02 ` Mark Rutland 2020-12-09 14:02 ` Mark Rutland 2020-12-09 14:05 ` Jürgen Groß 2020-12-09 14:05 ` Jürgen Groß via Virtualization 2020-12-09 18:15 ` Mark Rutland 2020-12-09 18:15 ` Mark Rutland 2020-12-09 18:54 ` Thomas Gleixner 2020-12-09 18:54 ` Thomas Gleixner 2020-12-10 11:10 ` Mark Rutland 2020-12-10 11:10 ` Mark Rutland 2020-12-10 20:15 ` Thomas Gleixner [this message] 2020-12-10 20:15 ` x86/ioapic: Cleanup the timer_works() irqflags mess Thomas Gleixner 2020-12-10 22:04 ` [tip: x86/apic] " tip-bot2 for Thomas Gleixner 2020-12-11 5:10 ` Jürgen Groß 2020-12-11 5:10 ` Jürgen Groß via Virtualization 2020-11-27 2:20 ` [x86] 97e8f0134a: fio.write_iops 8.6% improvement kernel test robot 2020-11-20 11:46 ` [PATCH v2 06/12] x86/paravirt: switch time pvops functions to use static_call() Juergen Gross 2020-11-20 11:46 ` Juergen Gross via Virtualization 2020-11-20 12:01 ` Peter Zijlstra 2020-11-20 12:01 ` Peter Zijlstra 2020-11-20 12:07 ` Jürgen Groß 2020-11-20 12:07 ` Jürgen Groß via Virtualization 2020-11-20 11:46 ` [PATCH v2 07/12] x86: add new features for paravirt patching Juergen Gross 2020-12-08 18:43 ` Borislav Petkov 2020-12-09 7:30 ` Jürgen Groß 2020-12-09 12:03 ` Borislav Petkov 2020-12-09 12:22 ` Jürgen Groß 2020-12-10 17:58 ` Borislav Petkov 2020-11-20 11:46 ` [PATCH v2 08/12] x86/paravirt: remove no longer needed 32-bit pvops cruft Juergen Gross 2020-11-20 11:46 ` Juergen Gross via Virtualization 2020-11-20 12:08 ` Peter Zijlstra 2020-11-20 12:08 ` Peter Zijlstra 2020-11-20 12:16 ` Jürgen Groß 2020-11-20 12:16 ` Jürgen Groß via Virtualization 2020-11-20 16:52 ` Arvind Sankar 2020-11-20 11:46 ` [PATCH v2 09/12] x86/paravirt: switch iret pvops to ALTERNATIVE Juergen Gross 2020-11-20 11:46 ` Juergen Gross via Virtualization 2020-11-20 11:46 ` [PATCH v2 10/12] x86/paravirt: add new macros PVOP_ALT* supporting pvops in ALTERNATIVEs Juergen Gross 2020-11-20 11:46 ` Juergen Gross via Virtualization 2020-11-20 11:46 ` [PATCH v2 11/12] x86/paravirt: switch functions with custom code to ALTERNATIVE Juergen Gross 2020-11-20 11:46 ` Juergen Gross via Virtualization 2020-11-20 15:46 ` kernel test robot 2020-11-20 15:46 ` kernel test robot 2020-11-20 15:46 ` kernel test robot 2020-11-25 15:46 ` [x86/paravirt] fd8d46a7a2: kernel-selftests.livepatch.test-callbacks.sh.fail kernel test robot 2020-11-20 11:46 ` [PATCH v2 12/12] x86/paravirt: have only one paravirt patch function Juergen Gross 2020-11-20 11:46 ` Juergen Gross via Virtualization 2020-11-20 14:18 ` kernel test robot 2020-11-20 14:18 ` kernel test robot 2020-11-20 14:18 ` kernel test robot 2020-11-20 12:53 ` [PATCH v2 00/12] x86: major paravirt cleanup Peter Zijlstra 2020-11-20 12:53 ` Peter Zijlstra 2020-11-23 13:43 ` Peter Zijlstra 2020-11-23 13:43 ` Peter Zijlstra 2020-12-15 11:42 ` Jürgen Groß 2020-12-15 11:42 ` Jürgen Groß via Virtualization 2020-12-15 14:18 ` Peter Zijlstra 2020-12-15 14:18 ` Peter Zijlstra 2020-12-15 14:54 ` Peter Zijlstra 2020-12-15 14:54 ` Peter Zijlstra 2020-12-15 15:07 ` Jürgen Groß 2020-12-15 15:07 ` Jürgen Groß via Virtualization 2020-12-16 0:38 ` Josh Poimboeuf 2020-12-16 0:38 ` Josh Poimboeuf 2020-12-16 8:40 ` Peter Zijlstra 2020-12-16 8:40 ` Peter Zijlstra 2020-12-16 16:56 ` Josh Poimboeuf 2020-12-16 16:56 ` Josh Poimboeuf 2020-12-16 17:58 ` Peter Zijlstra 2020-12-16 17:58 ` Peter Zijlstra
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=87k0tpju47.fsf@nanos.tec.linutronix.de \ --to=tglx@linutronix.de \ --cc=boris.ostrovsky@oracle.com \ --cc=bp@alien8.de \ --cc=hpa@zytor.com \ --cc=jgross@suse.com \ --cc=linux-kernel@vger.kernel.org \ --cc=luto@kernel.org \ --cc=mark.rutland@arm.com \ --cc=mingo@redhat.com \ --cc=peterz@infradead.org \ --cc=pv-drivers@vmware.com \ --cc=sdeep@vmware.com \ --cc=sstabellini@kernel.org \ --cc=virtualization@lists.linux-foundation.org \ --cc=x86@kernel.org \ --cc=xen-devel@lists.xenproject.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.