Re: [PATCH] x86/i8259: Work around buggy legacy PIC

From: Maximilian Luz <luzmaximilian@gmail.com>
To: "H. Peter Anvin" <hpa@zytor.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Sachi King <nakato@nakato.io>, Ingo Molnar <mingo@redhat.com>,
	Borislav Petkov <bp@alien8.de>,
	David Laight <David.Laight@aculab.com>
Cc: "x86@kernel.org" <x86@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"stable@vger.kernel.org" <stable@vger.kernel.org>
Subject: Re: [PATCH] x86/i8259: Work around buggy legacy PIC
Date: Sat, 15 May 2021 00:47:44 +0200	[thread overview]
Message-ID: <087c418e-53fb-b3a6-0d9b-e738e4c821c7@gmail.com> (raw)
In-Reply-To: <c26954e1-bb2f-086a-9c7f-68382978efe7@zytor.com>

On 5/14/21 7:35 PM, H. Peter Anvin wrote:
> On 5/14/21 10:32 AM, Thomas Gleixner wrote:
>>
>> That's a valid assumption. As I said, we can make IOAPIC work even w/o
>> PIC. I'll have a look how much PIC assumptions are still around.
>>
> 
> As far as I read, the problem isn't actually the absence of a PIC (we definitely boot systems without PICs all the time now), but rather that the PIC is advertised in ACPI but is buggy or absent; a similar platform with different firmware doesn't have problem.
> 
> If my understanding of the thread is correct, it's quirk fodder.

I believe the theory was that, while the PIC is advertised in ACPI, it
might be expected to not be used and only present for some legacy reason
(therefore untested and buggy). Which I believe led to the question
whether we shouldn't prefer IOAPIC on systems like that in general. So I
guess it comes down to how you define "systems like that". By Tomas'
comment, I guess it should be possible to implement this as "systems
that should prefer IOAPIC over legacy PIC" quirk.

I guess all modern machines should have an IOAPIC, so it might also be
preferable to expand that definition, maybe over time and with enough
testing.

If possible, I think that would be preferable to the "just retry until
it works" kind of workaround.

As Sachi mentioned in her reply:

> I'm not sure the i8259 is needed on the device, as the interrupts
> appear to function on the device if I bypass the nr_legacy_irqs() check
> while the legacy_pic is set to the null_legacy_pic.
> 
> The null_legacy_pic however specifies having 0 irqs, and the io_apic
> does not allow us to set the pin attributes unless the pin we are
> attempting to set is less than nr_legacy_irqs.
> 
> The IOAPIC seems to take responsibility for the 0-15 interrupts on this
> specific hardware, should we maybe be ignoring the i8259 and looking
> into allowing interrupts 0-15 to be setup even when the legacy_pic is
> not available?

Just my two cents, please keep in mind that I'm out of my depth here.

Regards,
Max