From: Vitaly Kuznetsov <vkuznets@redhat.com>
To: Thomas Gleixner <tglx@linutronix.de>, x86@kernel.org
Cc: Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
"H. Peter Anvin" <hpa@zytor.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH RFC 1/2] x86/apic: Do not make an exception for PIC_CASCADE_IR when marking legacy irqs in irq_matrix
Date: Thu, 18 Mar 2021 09:47:10 +0100 [thread overview]
Message-ID: <878s6k7ss1.fsf@vitty.brq.redhat.com> (raw)
In-Reply-To: <874kh9xuid.fsf@nanos.tec.linutronix.de>
Thomas Gleixner <tglx@linutronix.de> writes:
> On Wed, Mar 17 2021 at 22:40, Thomas Gleixner wrote:
>> af174783b925 ("x86: I/O APIC: Never configure IRQ2")
>>
>> has a very nice explanation why.
>>
>> Back then the logic was quite different. All legacy PIC interrupts
>> (0-15) were bound to the legacy vectors at boot and never moved away.
>>
>> There was a check in the back then setup routing which prevented the
>> IOAPIC routing of IRQ2 which got lost at some point. Haven't figured out
>> yet where this might be. Still digging in those ancient horrors.
>
> So the commit in question is:
>
> d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical irqdomain interfaces")
>
> For almost 6 years nobody complained about this wreckage, which might
> indicate that we could lift this requirement, but OTOH for any system
> which actually has a PIC IRQ2 is unusable by design.
>
> And my history based paranoia vs. these kind of changes just makes me
> tend to the safe side, so restoring the original behaviour makes more
> sense than just papering over it.
>
> Just to be clear: The reservation of PIC pin2 (aka. IRQ2) as a permanent
> system vector cannot go away for any system which has a PIC. If that
> ever fires then the PIC is hosed and you really want the warning about
> the unhandled vector which is easy to identify because it is fixed.
>
> You might think that's not relevant for modern CPUs where the PIC is
> emulated, but I've dealt with reports where that emulation goes south so
> I rather keep those sanity checks around as long as that legacy mess is
> relevant. No idea why it still exists, but that's a different story.
>
> Thanks,
>
> tglx
> ---
> Subject: x86/ioapic: Ignore IRQ2 again
> From: Thomas Gleixner <tglx@linutronix.de>
> Date: Wed, 17 Mar 2021 23:10:02 +0100
>
> Why did I have to do the archaeology?
>
> < INSERT PROPER CHANGELOG >
>
> Fixes: d32932d02e18 ("x86/irq: Convert IOAPIC to use hierarchical irqdomain interfaces")
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
> arch/x86/kernel/apic/io_apic.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> --- a/arch/x86/kernel/apic/io_apic.c
> +++ b/arch/x86/kernel/apic/io_apic.c
> @@ -1032,6 +1032,16 @@ static int mp_map_pin_to_irq(u32 gsi, in
> if (idx >= 0 && test_bit(mp_irqs[idx].srcbus, mp_bus_not_pci)) {
> irq = mp_irqs[idx].srcbusirq;
> legacy = mp_is_legacy_irq(irq);
> + /*
> + * IRQ2 is unusable for historical reasons on systems which
> + * have a legacy PIC. See the comment vs. IRQ2 further down.
> + *
> + * If this gets removed at some point then the related code
> + * in lapic_assign_system_vectors() needs to be adjusted as
> + * well.
> + */
> + if (legacy && irq == PIC_CASCADE_IR)
> + return -EINVAL;
> }
>
> mutex_lock(&ioapic_mutex);
>
This seems to work well for me, thanks! Please feel free to add
Tested-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Alternatively, just let me know and I'll pick this up, write a changelog and
send out with your Suggested-by:.
--
Vitaly
next prev parent reply other threads:[~2021-03-18 8:47 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-02-19 11:30 [PATCH RFC 0/2] x86/apic: Avoid cm->allocated going negative in irq_matrix Vitaly Kuznetsov
2021-02-19 11:31 ` [PATCH RFC 1/2] x86/apic: Do not make an exception for PIC_CASCADE_IR when marking legacy irqs " Vitaly Kuznetsov
2021-03-17 20:14 ` Thomas Gleixner
2021-03-17 21:18 ` Thomas Gleixner
2021-03-17 21:40 ` Thomas Gleixner
2021-03-17 22:52 ` Thomas Gleixner
2021-03-18 8:47 ` Vitaly Kuznetsov [this message]
2021-03-18 8:29 ` Vitaly Kuznetsov
2021-03-18 10:10 ` Thomas Gleixner
2021-02-19 11:31 ` [PATCH RFC 2/2] genirq/matrix: WARN_ON_ONCE() when cm->allocated/m->total_allocated go negative Vitaly Kuznetsov
2021-03-17 20:38 ` Thomas Gleixner
2021-03-18 7:58 ` Vitaly Kuznetsov
2021-03-18 19:29 ` Thomas Gleixner
2021-03-19 9:59 ` Vitaly Kuznetsov
2021-03-11 10:19 ` [PATCH RFC 0/2] x86/apic: Avoid cm->allocated going negative in irq_matrix Vitaly Kuznetsov
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=878s6k7ss1.fsf@vitty.brq.redhat.com \
--to=vkuznets@redhat.com \
--cc=bp@alien8.de \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).