From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Maximilian Luz <luzmaximilian@gmail.com>,
"H. Peter Anvin" <hpa@zytor.com>, Sachi King <nakato@nakato.io>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
David Laight <David.Laight@aculab.com>,
"x86@kernel.org" <x86@kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
stable <stable@vger.kernel.org>, Sasha Levin <sashal@kernel.org>,
Tom Lendacky <thomas.lendacky@amd.com>
Subject: Re: [PATCH] x86/i8259: Work around buggy legacy PIC
Date: Tue, 18 May 2021 11:28:47 +0300 [thread overview]
Message-ID: <YKN6vwGE2XAg3kY7@smile.fi.intel.com> (raw)
In-Reply-To: <87lf8ddjqx.ffs@nanos.tec.linutronix.de>
On Tue, May 18, 2021 at 01:27:02AM +0200, Thomas Gleixner wrote:
> On Mon, May 17 2021 at 21:25, Maximilian Luz wrote:
> > On 5/17/21 8:40 PM, Thomas Gleixner wrote:
> >> Can you please add "apic=verbose" to the kernel command line and provide
> >> full dmesg output for a kernel w/o your patch and one with your patch
> >> applied?
> >
> > I don't actually own an affected device, but I'm sure Sachi can provide
> > you with that.
>
> Ok.
>
> > As far as we can tell, due to the NULL PIC being chosen nr_legacy_irqs()
> > returns 0. That in turn causes mp_check_pin_attr() to return false
> > because is_level and active_low don't seem to match the expected
> > values.
>
> Ok.
>
> > That check is essentially ignored if nr_legacy_irqs() returns a high
> > enough value.
>
> Close enough.
>
> > I guess that might also be a firmware bug here? Not sure where the
> > expected values come from.
>
> They come from the interrupt override ACPI table and if not supplied
> then irq 0-15 is preset with default values, which are type=edge and
> polarity=high, i.e. the opposite of what the failing driver wants.
>
> The ACPI table lacks an override entry for IRQ7. I looked at one of the
> dmesg files in that github thread and that has overrides:
>
> [ 0.111674] ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl)
> [ 0.111681] ACPI: INT_SRC_OVR (bus 0 bus_irq 9 global_irq 9 low level)
> [ 0.111688] ACPI: IRQ0 used by override.
> [ 0.111692] ACPI: IRQ9 used by override.
>
> IRQ7 should have a corresponding entry as IRQ9 has:
>
> https://github.com/linux-surface/acpidumps/blob/4da0148744164cea0c924dab92f45842fde03177/surface_laptop_4_amd/apic.dsl#L178
>
> Subtable Type : 02 [Interrupt Source Override]
> Length : 0A
> Bus : 00
> Source : 07
> Interrupt : 00000007
> Flags (decoded below) : 000F
> Polarity : 3
> Trigger Mode : 3
>
> > Sachi can probably walk you through this a bit better as she's the one
> > who tracked this down. See also [1, 2] and following comments.
>
> Impressive detective work!
>
> Sachi, can you please try the hack below to confirm the above?
>
> It's not meant to be a solution, but it's the most trivial way to
> validate this.
>
> I'm pretty sure that Windows on Surface does not care about the PIC at
> all. Whether that's on purpose to safe power or just because Windows
> ignores the PIC completely by now does not matter at all. No idea how
> that repeated poking on the PIC makes it come alive either and TBH, I
> don't care too much about it simply because Linux is able to cope with a
> missing PIC as long as the ACPI tables are correct.
>
> I'm way too tired to think about a proper solution for that problem and
> I noticed another related issue in that dmesg output:
>
> [ 0.272448] Failed to register legacy timer interrupt
>
> It's not a problem which causes failures, but it's related to the
> missing PIC.
But ACPI has a pretty nice means about missing legacy hardware, it's called
Hardware Reduced mode. It excludes automatically the (legacy) PIC, PIT, etc.
OTOH it excludes ACPI power chip as well. I haven't looked into this, just
share my thoughts what else can be checked. (On Intel the MID devices use
that approach)
> Needs some more thoughts with brain awake...
--
With Best Regards,
Andy Shevchenko
next prev parent reply other threads:[~2021-05-18 8:28 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-12 21:04 [PATCH] x86/i8259: Work around buggy legacy PIC Maximilian Luz
2021-05-13 8:10 ` David Laight
2021-05-13 10:11 ` Maximilian Luz
2021-05-13 10:36 ` David Laight
2021-05-14 13:01 ` Thomas Gleixner
2021-05-14 13:13 ` David Laight
2021-05-14 16:19 ` Ingo Molnar
2021-05-14 19:41 ` Sachi King
2021-05-14 10:51 ` David Laight
2021-05-14 11:58 ` Maximilian Luz
2021-05-14 17:32 ` Thomas Gleixner
2021-05-14 17:35 ` H. Peter Anvin
2021-05-14 22:47 ` Maximilian Luz
2021-05-17 18:40 ` Thomas Gleixner
2021-05-17 19:25 ` Maximilian Luz
2021-05-17 23:27 ` Thomas Gleixner
2021-05-18 8:28 ` Andy Shevchenko [this message]
2021-05-18 19:58 ` Sachi King
2021-05-18 15:45 ` Thomas Gleixner
2021-05-14 13:44 ` Thomas Gleixner
2021-05-14 16:12 ` David Laight
2021-05-14 17:28 ` H. Peter Anvin
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=YKN6vwGE2XAg3kY7@smile.fi.intel.com \
--to=andriy.shevchenko@linux.intel.com \
--cc=David.Laight@aculab.com \
--cc=bp@alien8.de \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luzmaximilian@gmail.com \
--cc=mingo@redhat.com \
--cc=nakato@nakato.io \
--cc=sashal@kernel.org \
--cc=stable@vger.kernel.org \
--cc=tglx@linutronix.de \
--cc=thomas.lendacky@amd.com \
--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).