* Some SSDT tables are not loading with kernel >= 5.12 @ 2021-06-03 17:26 Hans de Goede 2021-06-07 9:43 ` Hans de Goede 0 siblings, 1 reply; 15+ messages in thread From: Hans de Goede @ 2021-06-03 17:26 UTC (permalink / raw) To: Rafael J . Wysocki; +Cc: linux-acpi Hi Rafael, I've been helping some users with trying to get to the bottom of some new ACPI errors with kernel 5.12, see: https://bugzilla.kernel.org/show_bug.cgi?id=213023 After looking at dmesg output; and after editing the dmesg output a bit so that I could do diff -u on it, the following stands out: --- dmesg_5.10.38-1-lts 2021-06-03 16:29:41.372922210 +0200 +++ dmesg_linux-5.12.5-arch1-1 2021-06-03 16:30:01.013031634 +0200 @@ -92,7 +92,7 @@ ACPI: IRQ9 used by override. Using ACPI (MADT) for SMP configuration information ACPI: HPET id: 0x8086a201 base: 0xfed00000 -ACPI: Core revision 20200925 +ACPI: Core revision 20210105 PM: Registering ACPI NVS region [mem 0x7156c000-0x7156cfff] (4096 bytes) PM: Registering ACPI NVS region [mem 0x8a88f000-0x8af7efff] (7274496 bytes) ACPI FADT declares the system doesn't support PCIe ASPM, so disable it @@ -113,10 +113,6 @@ ACPI: Dynamic OEM Table Load: ACPI: SSDT 0xFFFF... 0003FF (v02 PmRef Cpu0Cst 00003001 INTL 20160527) ACPI: Dynamic OEM Table Load: -ACPI: SSDT 0xFFFF... 0000BA (v02 PmRef Cpu0Hwp 00003000 INTL 20160527) -ACPI: Dynamic OEM Table Load: -ACPI: SSDT 0xFFFF... 000628 (v02 PmRef HwpLvt 00003000 INTL 20160527) -ACPI: Dynamic OEM Table Load: ACPI: SSDT 0xFFFF... 000D14 (v02 PmRef ApIst 00003000 INTL 20160527) ACPI: Dynamic OEM Table Load: ACPI: SSDT 0xFFFF... 000317 (v02 PmRef ApHwp 00003000 INTL 20160527) Note how for some reason the kernel is no longer loading the Cpu0Hwp and HwpLvt SSDT-s ? Do you have any ideas what might be causing this ? Regards, Hans ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Some SSDT tables are not loading with kernel >= 5.12 2021-06-03 17:26 Some SSDT tables are not loading with kernel >= 5.12 Hans de Goede @ 2021-06-07 9:43 ` Hans de Goede 2021-06-07 10:05 ` Hans de Goede 0 siblings, 1 reply; 15+ messages in thread From: Hans de Goede @ 2021-06-07 9:43 UTC (permalink / raw) To: Rafael J . Wysocki; +Cc: linux-acpi Hi, On 6/3/21 7:26 PM, Hans de Goede wrote: > Hi Rafael, > > I've been helping some users with trying to get to the bottom of some > new ACPI errors with kernel 5.12, see: > > https://bugzilla.kernel.org/show_bug.cgi?id=213023 > > After looking at dmesg output; and after editing the dmesg output > a bit so that I could do diff -u on it, the following stands out: > > --- dmesg_5.10.38-1-lts 2021-06-03 16:29:41.372922210 +0200 > +++ dmesg_linux-5.12.5-arch1-1 2021-06-03 16:30:01.013031634 +0200 > @@ -92,7 +92,7 @@ > ACPI: IRQ9 used by override. > Using ACPI (MADT) for SMP configuration information > ACPI: HPET id: 0x8086a201 base: 0xfed00000 > -ACPI: Core revision 20200925 > +ACPI: Core revision 20210105 > PM: Registering ACPI NVS region [mem 0x7156c000-0x7156cfff] (4096 bytes) > PM: Registering ACPI NVS region [mem 0x8a88f000-0x8af7efff] (7274496 bytes) > ACPI FADT declares the system doesn't support PCIe ASPM, so disable it > @@ -113,10 +113,6 @@ > ACPI: Dynamic OEM Table Load: > ACPI: SSDT 0xFFFF... 0003FF (v02 PmRef Cpu0Cst 00003001 INTL 20160527) > ACPI: Dynamic OEM Table Load: > -ACPI: SSDT 0xFFFF... 0000BA (v02 PmRef Cpu0Hwp 00003000 INTL 20160527) > -ACPI: Dynamic OEM Table Load: > -ACPI: SSDT 0xFFFF... 000628 (v02 PmRef HwpLvt 00003000 INTL 20160527) > -ACPI: Dynamic OEM Table Load: > ACPI: SSDT 0xFFFF... 000D14 (v02 PmRef ApIst 00003000 INTL 20160527) > ACPI: Dynamic OEM Table Load: > ACPI: SSDT 0xFFFF... 000317 (v02 PmRef ApHwp 00003000 INTL 20160527) > > Note how for some reason the kernel is no longer loading the Cpu0Hwp and > HwpLvt SSDT-s ? > > Do you have any ideas what might be causing this ? Good news, a very similar bug is being tracked here: https://bugzilla.redhat.com/show_bug.cgi?id=1963717 And one of the reporters there has done a git bisect and has found the commit which is causing the problem for them: """ git-bisect points to 719e1f561afbe020ed175825a9bd25ed62ed1697 : "ACPI: Execute platform _OSC also with query bit clear". Tested 5.12.9 kernel with the commit reverted, and confirmed that the error messages are gone. (I had to revert 5a6a2c0f0f43676df27632d657a3f18b151a7ef8 for dependency too.) It also brings back the /sys/devices/system/cpu/cpu0/acpi_cppc which is absent in the stable 5.12.x Hope this helps """ I've asked the reporters of: https://bugzilla.kernel.org/show_bug.cgi?id=213023 To also try reverting 719e1f561afbe020ed175825a9bd25ed62ed1697 and see if that helps (I expect it will, I believe the 2 bugs are the same issue). Either way we need to do something about this to fix this for the reporter of https://bugzilla.redhat.com/show_bug.cgi?id=1963717 Any ideas on how to fix this? Regards, Hans ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Some SSDT tables are not loading with kernel >= 5.12 2021-06-07 9:43 ` Hans de Goede @ 2021-06-07 10:05 ` Hans de Goede 2021-06-07 11:13 ` Rafael J. Wysocki 0 siblings, 1 reply; 15+ messages in thread From: Hans de Goede @ 2021-06-07 10:05 UTC (permalink / raw) To: Rafael J . Wysocki, Mario Limonciello; +Cc: linux-acpi [-- Attachment #1: Type: text/plain, Size: 2712 bytes --] HI, On 6/7/21 11:43 AM, Hans de Goede wrote: > Hi, > > On 6/3/21 7:26 PM, Hans de Goede wrote: >> Hi Rafael, >> >> I've been helping some users with trying to get to the bottom of some >> new ACPI errors with kernel 5.12, see: >> >> https://bugzilla.kernel.org/show_bug.cgi?id=213023 >> >> After looking at dmesg output; and after editing the dmesg output >> a bit so that I could do diff -u on it, the following stands out: >> >> --- dmesg_5.10.38-1-lts 2021-06-03 16:29:41.372922210 +0200 >> +++ dmesg_linux-5.12.5-arch1-1 2021-06-03 16:30:01.013031634 +0200 >> @@ -92,7 +92,7 @@ >> ACPI: IRQ9 used by override. >> Using ACPI (MADT) for SMP configuration information >> ACPI: HPET id: 0x8086a201 base: 0xfed00000 >> -ACPI: Core revision 20200925 >> +ACPI: Core revision 20210105 >> PM: Registering ACPI NVS region [mem 0x7156c000-0x7156cfff] (4096 bytes) >> PM: Registering ACPI NVS region [mem 0x8a88f000-0x8af7efff] (7274496 bytes) >> ACPI FADT declares the system doesn't support PCIe ASPM, so disable it >> @@ -113,10 +113,6 @@ >> ACPI: Dynamic OEM Table Load: >> ACPI: SSDT 0xFFFF... 0003FF (v02 PmRef Cpu0Cst 00003001 INTL 20160527) >> ACPI: Dynamic OEM Table Load: >> -ACPI: SSDT 0xFFFF... 0000BA (v02 PmRef Cpu0Hwp 00003000 INTL 20160527) >> -ACPI: Dynamic OEM Table Load: >> -ACPI: SSDT 0xFFFF... 000628 (v02 PmRef HwpLvt 00003000 INTL 20160527) >> -ACPI: Dynamic OEM Table Load: >> ACPI: SSDT 0xFFFF... 000D14 (v02 PmRef ApIst 00003000 INTL 20160527) >> ACPI: Dynamic OEM Table Load: >> ACPI: SSDT 0xFFFF... 000317 (v02 PmRef ApHwp 00003000 INTL 20160527) >> >> Note how for some reason the kernel is no longer loading the Cpu0Hwp and >> HwpLvt SSDT-s ? >> >> Do you have any ideas what might be causing this ? > > Good news, a very similar bug is being tracked here: > > https://bugzilla.redhat.com/show_bug.cgi?id=1963717 > > And one of the reporters there has done a git bisect and has found the commit which is causing the problem for them: > > """ > git-bisect points to 719e1f561afbe020ed175825a9bd25ed62ed1697 : > "ACPI: Execute platform _OSC also with query bit clear". > > Tested 5.12.9 kernel with the commit reverted, and confirmed that the error > messages are gone. (I had to revert > 5a6a2c0f0f43676df27632d657a3f18b151a7ef8 for dependency too.) > > It also brings back the /sys/devices/system/cpu/cpu0/acpi_cppc which is absent > in the stable 5.12.x > > Hope this helps > """ I've taken a quick look at commit 719e1f561afb ("ACPI: Execute platform _OSC also with query bit clear") and I think I may have found the problem. I've attached a patch which I think may fix this (and I've asked the reporters of the bugs to test this). Regards, Hans [-- Attachment #2: 0001-ACPI-Use-_OSC-query-results-to-determine-caps-rather.patch --] [-- Type: text/x-patch, Size: 2517 bytes --] From d2ee8bc4ee24456fddef264f71628e2588e3144c Mon Sep 17 00:00:00 2001 From: Hans de Goede <hdegoede@redhat.com> Date: Mon, 7 Jun 2021 11:55:57 +0200 Subject: [PATCH] ACPI: Use _OSC query results to determine caps rather then the commit results Commit 719e1f561afb ("ACPI: Execute platform _OSC also with query bit clear") makes acpi_bus_osc_negotiate_platform_control() not only query the platforms capabilities but it also commits the result back to the firmware to let the firmware know which capabilities are supported back by the OS. This also moved the code to set the osc_sb_apei_support_acked, osc_pc_lpi_support_confirmed and osc_sb_native_usb4_support_confirmed flags to after the commit. But it seems that some BIOS-es clear the OSC_SUPPORT_DWORD on return when the query bit is not set. Move the checking of the capabilities back to after the _OSC call with the query bit set to restore the old behavior. BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=213023 BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1963717 Fixes: 719e1f561afb ("ACPI: Execute platform _OSC also with query bit clear") Cc: Mario Limonciello <mario.limonciello@outlook.com> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/acpi/bus.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index be7da23fad76..1e4953c0d5c2 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -336,6 +336,10 @@ static void acpi_bus_osc_negotiate_platform_control(void) return; } + osc_sb_apei_support_acked = capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT; + osc_pc_lpi_support_confirmed = capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT; + osc_sb_native_usb4_support_confirmed = capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_NATIVE_USB4_SUPPORT; + /* * Now run _OSC again with query flag clear and with the caps * supported by both the OS and the platform. @@ -347,16 +351,6 @@ static void acpi_bus_osc_negotiate_platform_control(void) if (ACPI_FAILURE(acpi_run_osc(handle, &context))) return; - capbuf_ret = context.ret.pointer; - if (context.ret.length > OSC_SUPPORT_DWORD) { - osc_sb_apei_support_acked = - capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT; - osc_pc_lpi_support_confirmed = - capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT; - osc_sb_native_usb4_support_confirmed = - capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_NATIVE_USB4_SUPPORT; - } - kfree(context.ret.pointer); } -- 2.31.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: Some SSDT tables are not loading with kernel >= 5.12 2021-06-07 10:05 ` Hans de Goede @ 2021-06-07 11:13 ` Rafael J. Wysocki [not found] ` <FR1PR80MB5051E91269FD36681BB357A7E1389@FR1PR80MB5051.lamprd80.prod.outlook.com> 0 siblings, 1 reply; 15+ messages in thread From: Rafael J. Wysocki @ 2021-06-07 11:13 UTC (permalink / raw) To: Hans de Goede; +Cc: Rafael J . Wysocki, Mario Limonciello, linux-acpi On Mon, Jun 7, 2021 at 12:05 PM Hans de Goede <hdegoede@redhat.com> wrote: > > HI, > > On 6/7/21 11:43 AM, Hans de Goede wrote: > > Hi, > > > > On 6/3/21 7:26 PM, Hans de Goede wrote: > >> Hi Rafael, > >> > >> I've been helping some users with trying to get to the bottom of some > >> new ACPI errors with kernel 5.12, see: > >> > >> https://bugzilla.kernel.org/show_bug.cgi?id=213023 > >> > >> After looking at dmesg output; and after editing the dmesg output > >> a bit so that I could do diff -u on it, the following stands out: > >> > >> --- dmesg_5.10.38-1-lts 2021-06-03 16:29:41.372922210 +0200 > >> +++ dmesg_linux-5.12.5-arch1-1 2021-06-03 16:30:01.013031634 +0200 > >> @@ -92,7 +92,7 @@ > >> ACPI: IRQ9 used by override. > >> Using ACPI (MADT) for SMP configuration information > >> ACPI: HPET id: 0x8086a201 base: 0xfed00000 > >> -ACPI: Core revision 20200925 > >> +ACPI: Core revision 20210105 > >> PM: Registering ACPI NVS region [mem 0x7156c000-0x7156cfff] (4096 bytes) > >> PM: Registering ACPI NVS region [mem 0x8a88f000-0x8af7efff] (7274496 bytes) > >> ACPI FADT declares the system doesn't support PCIe ASPM, so disable it > >> @@ -113,10 +113,6 @@ > >> ACPI: Dynamic OEM Table Load: > >> ACPI: SSDT 0xFFFF... 0003FF (v02 PmRef Cpu0Cst 00003001 INTL 20160527) > >> ACPI: Dynamic OEM Table Load: > >> -ACPI: SSDT 0xFFFF... 0000BA (v02 PmRef Cpu0Hwp 00003000 INTL 20160527) > >> -ACPI: Dynamic OEM Table Load: > >> -ACPI: SSDT 0xFFFF... 000628 (v02 PmRef HwpLvt 00003000 INTL 20160527) > >> -ACPI: Dynamic OEM Table Load: > >> ACPI: SSDT 0xFFFF... 000D14 (v02 PmRef ApIst 00003000 INTL 20160527) > >> ACPI: Dynamic OEM Table Load: > >> ACPI: SSDT 0xFFFF... 000317 (v02 PmRef ApHwp 00003000 INTL 20160527) > >> > >> Note how for some reason the kernel is no longer loading the Cpu0Hwp and > >> HwpLvt SSDT-s ? > >> > >> Do you have any ideas what might be causing this ? > > > > Good news, a very similar bug is being tracked here: > > > > https://bugzilla.redhat.com/show_bug.cgi?id=1963717 > > > > And one of the reporters there has done a git bisect and has found the commit which is causing the problem for them: > > > > """ > > git-bisect points to 719e1f561afbe020ed175825a9bd25ed62ed1697 : > > "ACPI: Execute platform _OSC also with query bit clear". > > > > Tested 5.12.9 kernel with the commit reverted, and confirmed that the error > > messages are gone. (I had to revert > > 5a6a2c0f0f43676df27632d657a3f18b151a7ef8 for dependency too.) > > > > It also brings back the /sys/devices/system/cpu/cpu0/acpi_cppc which is absent > > in the stable 5.12.x > > > > Hope this helps > > """ > > I've taken a quick look at commit 719e1f561afb ("ACPI: Execute platform _OSC also with query bit clear") and I think I may have found the problem. > > I've attached a patch which I think may fix this (and I've asked the reporters of the bugs to test this). Thank you, the patch looks reasonable to me. It looks like commit 719e1f561afb went a bit too far. ^ permalink raw reply [flat|nested] 15+ messages in thread
[parent not found: <FR1PR80MB5051E91269FD36681BB357A7E1389@FR1PR80MB5051.lamprd80.prod.outlook.com>]
* Re: Some SSDT tables are not loading with kernel >= 5.12 [not found] ` <FR1PR80MB5051E91269FD36681BB357A7E1389@FR1PR80MB5051.lamprd80.prod.outlook.com> @ 2021-06-07 16:08 ` Mika Westerberg 2021-06-07 19:18 ` Hans de Goede 0 siblings, 1 reply; 15+ messages in thread From: Mika Westerberg @ 2021-06-07 16:08 UTC (permalink / raw) To: Mario Limonciello Cc: Rafael J. Wysocki, Hans de Goede, Rafael J . Wysocki, linux-acpi Hi, Tried now on ADL-P and TGL systems and the _OSC still works properly. Thanks Hans for fixing! Feel free to add my Tested-by. On Mon, Jun 07, 2021 at 01:01:59PM +0000, Mario Limonciello wrote: > Mika, > > Can you have a try and make sure this modification still works properly > on the series in the hardware we originally did it for? > __________________________________________________________________ > > From: Rafael J. Wysocki <rafael@kernel.org> > Sent: Monday, June 7, 2021 6:13:21 AM > To: Hans de Goede <hdegoede@redhat.com> > Cc: Rafael J . Wysocki <rjw@rjwysocki.net>; Mario Limonciello > <mario.limonciello@outlook.com>; linux-acpi > <linux-acpi@vger.kernel.org> > Subject: Re: Some SSDT tables are not loading with kernel >= 5.12 > > On Mon, Jun 7, 2021 at 12:05 PM Hans de Goede <hdegoede@redhat.com> > wrote: > > > > HI, > > > > On 6/7/21 11:43 AM, Hans de Goede wrote: > > > Hi, > > > > > > On 6/3/21 7:26 PM, Hans de Goede wrote: > > >> Hi Rafael, > > >> > > >> I've been helping some users with trying to get to the bottom of > some > > >> new ACPI errors with kernel 5.12, see: > > >> > > >> [1]https://bugzilla.kernel.org/show_bug.cgi?id=213023 > > >> > > >> After looking at dmesg output; and after editing the dmesg output > > >> a bit so that I could do diff -u on it, the following stands out: > > >> > > >> --- dmesg_5.10.38-1-lts 2021-06-03 16:29:41.372922210 +0200 > > >> +++ dmesg_linux-5.12.5-arch1-1 2021-06-03 16:30:01.013031634 > +0200 > > >> @@ -92,7 +92,7 @@ > > >> ACPI: IRQ9 used by override. > > >> Using ACPI (MADT) for SMP configuration information > > >> ACPI: HPET id: 0x8086a201 base: 0xfed00000 > > >> -ACPI: Core revision 20200925 > > >> +ACPI: Core revision 20210105 > > >> PM: Registering ACPI NVS region [mem 0x7156c000-0x7156cfff] (4096 > bytes) > > >> PM: Registering ACPI NVS region [mem 0x8a88f000-0x8af7efff] > (7274496 bytes) > > >> ACPI FADT declares the system doesn't support PCIe ASPM, so > disable it > > >> @@ -113,10 +113,6 @@ > > >> ACPI: Dynamic OEM Table Load: > > >> ACPI: SSDT 0xFFFF... 0003FF (v02 PmRef Cpu0Cst 00003001 INTL > 20160527) > > >> ACPI: Dynamic OEM Table Load: > > >> -ACPI: SSDT 0xFFFF... 0000BA (v02 PmRef Cpu0Hwp 00003000 INTL > 20160527) > > >> -ACPI: Dynamic OEM Table Load: > > >> -ACPI: SSDT 0xFFFF... 000628 (v02 PmRef HwpLvt 00003000 INTL > 20160527) > > >> -ACPI: Dynamic OEM Table Load: > > >> ACPI: SSDT 0xFFFF... 000D14 (v02 PmRef ApIst 00003000 INTL > 20160527) > > >> ACPI: Dynamic OEM Table Load: > > >> ACPI: SSDT 0xFFFF... 000317 (v02 PmRef ApHwp 00003000 INTL > 20160527) > > >> > > >> Note how for some reason the kernel is no longer loading the > Cpu0Hwp and > > >> HwpLvt SSDT-s ? > > >> > > >> Do you have any ideas what might be causing this ? > > > > > > Good news, a very similar bug is being tracked here: > > > > > > [2]https://bugzilla.redhat.com/show_bug.cgi?id=1963717 > > > > > > And one of the reporters there has done a git bisect and has found > the commit which is causing the problem for them: > > > > > > """ > > > git-bisect points to 719e1f561afbe020ed175825a9bd25ed62ed1697 : > > > "ACPI: Execute platform _OSC also with query bit clear". > > > > > > Tested 5.12.9 kernel with the commit reverted, and confirmed that > the error > > > messages are gone. (I had to revert > > > 5a6a2c0f0f43676df27632d657a3f18b151a7ef8 for dependency too.) > > > > > > It also brings back the /sys/devices/system/cpu/cpu0/acpi_cppc > which is absent > > > in the stable 5.12.x > > > > > > Hope this helps > > > """ > > > > I've taken a quick look at commit 719e1f561afb ("ACPI: Execute > platform _OSC also with query bit clear") and I think I may have found > the problem. > > > > I've attached a patch which I think may fix this (and I've asked the > reporters of the bugs to test this). > Thank you, the patch looks reasonable to me. > It looks like commit 719e1f561afb went a bit too far. > > References > > 1. https://bugzilla.kernel.org/show_bug.cgi?id=213023 > 2. https://bugzilla.redhat.com/show_bug.cgi?id=1963717 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Some SSDT tables are not loading with kernel >= 5.12 2021-06-07 16:08 ` Mika Westerberg @ 2021-06-07 19:18 ` Hans de Goede 2021-06-08 9:50 ` Hans de Goede 0 siblings, 1 reply; 15+ messages in thread From: Hans de Goede @ 2021-06-07 19:18 UTC (permalink / raw) To: Mika Westerberg, Mario Limonciello Cc: Rafael J. Wysocki, Rafael J . Wysocki, linux-acpi Hi, On 6/7/21 6:08 PM, Mika Westerberg wrote: > Hi, > > Tried now on ADL-P and TGL systems and the _OSC still works properly. > > Thanks Hans for fixing! > > Feel free to add my Tested-by. Thank you for testing, unfortunately so far from the comments here: https://bugzilla.kernel.org/show_bug.cgi?id=213023 it seems that my patch does not help resolve the issues caused by commit 719e1f561afb ("ACPI: Execute platform _OSC also with query bit clear"), where as reverting that commit does resolve them :| Does anyone have any other ideas how to fix this ? Regards, Hans > > On Mon, Jun 07, 2021 at 01:01:59PM +0000, Mario Limonciello wrote: >> Mika, >> >> Can you have a try and make sure this modification still works properly >> on the series in the hardware we originally did it for? >> __________________________________________________________________ >> >> From: Rafael J. Wysocki <rafael@kernel.org> >> Sent: Monday, June 7, 2021 6:13:21 AM >> To: Hans de Goede <hdegoede@redhat.com> >> Cc: Rafael J . Wysocki <rjw@rjwysocki.net>; Mario Limonciello >> <mario.limonciello@outlook.com>; linux-acpi >> <linux-acpi@vger.kernel.org> >> Subject: Re: Some SSDT tables are not loading with kernel >= 5.12 >> >> On Mon, Jun 7, 2021 at 12:05 PM Hans de Goede <hdegoede@redhat.com> >> wrote: >> > >> > HI, >> > >> > On 6/7/21 11:43 AM, Hans de Goede wrote: >> > > Hi, >> > > >> > > On 6/3/21 7:26 PM, Hans de Goede wrote: >> > >> Hi Rafael, >> > >> >> > >> I've been helping some users with trying to get to the bottom of >> some >> > >> new ACPI errors with kernel 5.12, see: >> > >> >> > >> [1]https://bugzilla.kernel.org/show_bug.cgi?id=213023 >> > >> >> > >> After looking at dmesg output; and after editing the dmesg output >> > >> a bit so that I could do diff -u on it, the following stands out: >> > >> >> > >> --- dmesg_5.10.38-1-lts 2021-06-03 16:29:41.372922210 +0200 >> > >> +++ dmesg_linux-5.12.5-arch1-1 2021-06-03 16:30:01.013031634 >> +0200 >> > >> @@ -92,7 +92,7 @@ >> > >> ACPI: IRQ9 used by override. >> > >> Using ACPI (MADT) for SMP configuration information >> > >> ACPI: HPET id: 0x8086a201 base: 0xfed00000 >> > >> -ACPI: Core revision 20200925 >> > >> +ACPI: Core revision 20210105 >> > >> PM: Registering ACPI NVS region [mem 0x7156c000-0x7156cfff] (4096 >> bytes) >> > >> PM: Registering ACPI NVS region [mem 0x8a88f000-0x8af7efff] >> (7274496 bytes) >> > >> ACPI FADT declares the system doesn't support PCIe ASPM, so >> disable it >> > >> @@ -113,10 +113,6 @@ >> > >> ACPI: Dynamic OEM Table Load: >> > >> ACPI: SSDT 0xFFFF... 0003FF (v02 PmRef Cpu0Cst 00003001 INTL >> 20160527) >> > >> ACPI: Dynamic OEM Table Load: >> > >> -ACPI: SSDT 0xFFFF... 0000BA (v02 PmRef Cpu0Hwp 00003000 INTL >> 20160527) >> > >> -ACPI: Dynamic OEM Table Load: >> > >> -ACPI: SSDT 0xFFFF... 000628 (v02 PmRef HwpLvt 00003000 INTL >> 20160527) >> > >> -ACPI: Dynamic OEM Table Load: >> > >> ACPI: SSDT 0xFFFF... 000D14 (v02 PmRef ApIst 00003000 INTL >> 20160527) >> > >> ACPI: Dynamic OEM Table Load: >> > >> ACPI: SSDT 0xFFFF... 000317 (v02 PmRef ApHwp 00003000 INTL >> 20160527) >> > >> >> > >> Note how for some reason the kernel is no longer loading the >> Cpu0Hwp and >> > >> HwpLvt SSDT-s ? >> > >> >> > >> Do you have any ideas what might be causing this ? >> > > >> > > Good news, a very similar bug is being tracked here: >> > > >> > > [2]https://bugzilla.redhat.com/show_bug.cgi?id=1963717 >> > > >> > > And one of the reporters there has done a git bisect and has found >> the commit which is causing the problem for them: >> > > >> > > """ >> > > git-bisect points to 719e1f561afbe020ed175825a9bd25ed62ed1697 : >> > > "ACPI: Execute platform _OSC also with query bit clear". >> > > >> > > Tested 5.12.9 kernel with the commit reverted, and confirmed that >> the error >> > > messages are gone. (I had to revert >> > > 5a6a2c0f0f43676df27632d657a3f18b151a7ef8 for dependency too.) >> > > >> > > It also brings back the /sys/devices/system/cpu/cpu0/acpi_cppc >> which is absent >> > > in the stable 5.12.x >> > > >> > > Hope this helps >> > > """ >> > >> > I've taken a quick look at commit 719e1f561afb ("ACPI: Execute >> platform _OSC also with query bit clear") and I think I may have found >> the problem. >> > >> > I've attached a patch which I think may fix this (and I've asked the >> reporters of the bugs to test this). >> Thank you, the patch looks reasonable to me. >> It looks like commit 719e1f561afb went a bit too far. >> >> References >> >> 1. https://bugzilla.kernel.org/show_bug.cgi?id=213023 >> 2. https://bugzilla.redhat.com/show_bug.cgi?id=1963717 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Some SSDT tables are not loading with kernel >= 5.12 2021-06-07 19:18 ` Hans de Goede @ 2021-06-08 9:50 ` Hans de Goede 2021-06-08 11:44 ` Mika Westerberg 0 siblings, 1 reply; 15+ messages in thread From: Hans de Goede @ 2021-06-08 9:50 UTC (permalink / raw) To: Mika Westerberg, Mario Limonciello Cc: Rafael J. Wysocki, Rafael J . Wysocki, linux-acpi [-- Attachment #1: Type: text/plain, Size: 5602 bytes --] Hi, On 6/7/21 9:18 PM, Hans de Goede wrote: > Hi, > > On 6/7/21 6:08 PM, Mika Westerberg wrote: >> Hi, >> >> Tried now on ADL-P and TGL systems and the _OSC still works properly. >> >> Thanks Hans for fixing! >> >> Feel free to add my Tested-by. > > Thank you for testing, unfortunately so far from the comments here: > > https://bugzilla.kernel.org/show_bug.cgi?id=213023 > > it seems that my patch does not help resolve the issues caused > by commit 719e1f561afb ("ACPI: Execute platform _OSC also with query > bit clear"), where as reverting that commit does resolve them :| > > Does anyone have any other ideas how to fix this ? The reporter who has done the bisect has commented out the new/second _OSC call and that fixes things for them. So I've written a new fix (attached), note just as before this is untested ATM. Mika, if you can test this one (it replaces the previous one) on machines with native USB4 support to check those don't regress then that would be great. I've asked the various reporters from the 2 bugzilla's for this to also test this new patch. I'll let you know how that goes. Regards, Hans >> On Mon, Jun 07, 2021 at 01:01:59PM +0000, Mario Limonciello wrote: >>> Mika, >>> >>> Can you have a try and make sure this modification still works properly >>> on the series in the hardware we originally did it for? >>> __________________________________________________________________ >>> >>> From: Rafael J. Wysocki <rafael@kernel.org> >>> Sent: Monday, June 7, 2021 6:13:21 AM >>> To: Hans de Goede <hdegoede@redhat.com> >>> Cc: Rafael J . Wysocki <rjw@rjwysocki.net>; Mario Limonciello >>> <mario.limonciello@outlook.com>; linux-acpi >>> <linux-acpi@vger.kernel.org> >>> Subject: Re: Some SSDT tables are not loading with kernel >= 5.12 >>> >>> On Mon, Jun 7, 2021 at 12:05 PM Hans de Goede <hdegoede@redhat.com> >>> wrote: >>> > >>> > HI, >>> > >>> > On 6/7/21 11:43 AM, Hans de Goede wrote: >>> > > Hi, >>> > > >>> > > On 6/3/21 7:26 PM, Hans de Goede wrote: >>> > >> Hi Rafael, >>> > >> >>> > >> I've been helping some users with trying to get to the bottom of >>> some >>> > >> new ACPI errors with kernel 5.12, see: >>> > >> >>> > >> [1]https://bugzilla.kernel.org/show_bug.cgi?id=213023 >>> > >> >>> > >> After looking at dmesg output; and after editing the dmesg output >>> > >> a bit so that I could do diff -u on it, the following stands out: >>> > >> >>> > >> --- dmesg_5.10.38-1-lts 2021-06-03 16:29:41.372922210 +0200 >>> > >> +++ dmesg_linux-5.12.5-arch1-1 2021-06-03 16:30:01.013031634 >>> +0200 >>> > >> @@ -92,7 +92,7 @@ >>> > >> ACPI: IRQ9 used by override. >>> > >> Using ACPI (MADT) for SMP configuration information >>> > >> ACPI: HPET id: 0x8086a201 base: 0xfed00000 >>> > >> -ACPI: Core revision 20200925 >>> > >> +ACPI: Core revision 20210105 >>> > >> PM: Registering ACPI NVS region [mem 0x7156c000-0x7156cfff] (4096 >>> bytes) >>> > >> PM: Registering ACPI NVS region [mem 0x8a88f000-0x8af7efff] >>> (7274496 bytes) >>> > >> ACPI FADT declares the system doesn't support PCIe ASPM, so >>> disable it >>> > >> @@ -113,10 +113,6 @@ >>> > >> ACPI: Dynamic OEM Table Load: >>> > >> ACPI: SSDT 0xFFFF... 0003FF (v02 PmRef Cpu0Cst 00003001 INTL >>> 20160527) >>> > >> ACPI: Dynamic OEM Table Load: >>> > >> -ACPI: SSDT 0xFFFF... 0000BA (v02 PmRef Cpu0Hwp 00003000 INTL >>> 20160527) >>> > >> -ACPI: Dynamic OEM Table Load: >>> > >> -ACPI: SSDT 0xFFFF... 000628 (v02 PmRef HwpLvt 00003000 INTL >>> 20160527) >>> > >> -ACPI: Dynamic OEM Table Load: >>> > >> ACPI: SSDT 0xFFFF... 000D14 (v02 PmRef ApIst 00003000 INTL >>> 20160527) >>> > >> ACPI: Dynamic OEM Table Load: >>> > >> ACPI: SSDT 0xFFFF... 000317 (v02 PmRef ApHwp 00003000 INTL >>> 20160527) >>> > >> >>> > >> Note how for some reason the kernel is no longer loading the >>> Cpu0Hwp and >>> > >> HwpLvt SSDT-s ? >>> > >> >>> > >> Do you have any ideas what might be causing this ? >>> > > >>> > > Good news, a very similar bug is being tracked here: >>> > > >>> > > [2]https://bugzilla.redhat.com/show_bug.cgi?id=1963717 >>> > > >>> > > And one of the reporters there has done a git bisect and has found >>> the commit which is causing the problem for them: >>> > > >>> > > """ >>> > > git-bisect points to 719e1f561afbe020ed175825a9bd25ed62ed1697 : >>> > > "ACPI: Execute platform _OSC also with query bit clear". >>> > > >>> > > Tested 5.12.9 kernel with the commit reverted, and confirmed that >>> the error >>> > > messages are gone. (I had to revert >>> > > 5a6a2c0f0f43676df27632d657a3f18b151a7ef8 for dependency too.) >>> > > >>> > > It also brings back the /sys/devices/system/cpu/cpu0/acpi_cppc >>> which is absent >>> > > in the stable 5.12.x >>> > > >>> > > Hope this helps >>> > > """ >>> > >>> > I've taken a quick look at commit 719e1f561afb ("ACPI: Execute >>> platform _OSC also with query bit clear") and I think I may have found >>> the problem. >>> > >>> > I've attached a patch which I think may fix this (and I've asked the >>> reporters of the bugs to test this). >>> Thank you, the patch looks reasonable to me. >>> It looks like commit 719e1f561afb went a bit too far. >>> >>> References >>> >>> 1. https://bugzilla.kernel.org/show_bug.cgi?id=213023 >>> 2. https://bugzilla.redhat.com/show_bug.cgi?id=1963717 >> [-- Attachment #2: 0001-ACPI-Only-run-_OSC-without-the-query-flag-on-machine.patch --] [-- Type: text/x-patch, Size: 3740 bytes --] From b758a6c48d418a89f8d2bc08c5daa778b162b5b3 Mon Sep 17 00:00:00 2001 From: Hans de Goede <hdegoede@redhat.com> Date: Mon, 7 Jun 2021 11:55:57 +0200 Subject: [PATCH] ACPI: Only run _OSC without the query flag on machines with native USB4 support Commit 719e1f561afb ("ACPI: Execute platform _OSC also with query bit clear") makes acpi_bus_osc_negotiate_platform_control() not only query the platforms capabilities but it also commits the result back to the firmware to report which capabilities are supported by the OS back to the firmware. This reporting back of the OS supported capabilities is necessary for USB4 support, but it is causing problems on some other devices, causing some SSDTs to no longer load on these devices. Make the reporting back of the capabilities (the _OSC call with the query flag cleared) conditional on the queried capabilities including native USB4 support, skipping it when there is no native USB4 support to fix the regressions caused by doing this on some machines without native USB4 support. BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=213023 BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1963717 Fixes: 719e1f561afb ("ACPI: Execute platform _OSC also with query bit clear") Cc: Mario Limonciello <mario.limonciello@outlook.com> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/acpi/bus.c | 41 +++++++++++++++++++---------------------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index be7da23fad76..9a56a6f653cc 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -286,7 +286,7 @@ EXPORT_SYMBOL_GPL(osc_sb_native_usb4_support_confirmed); static u8 sb_uuid_str[] = "0811B06E-4A27-44F9-8D60-3CBBC22E7B48"; static void acpi_bus_osc_negotiate_platform_control(void) { - u32 capbuf[2], *capbuf_ret; + u32 capbuf[2], caps = 0; struct acpi_osc_context context = { .uuid_str = sb_uuid_str, .rev = 1, @@ -330,34 +330,31 @@ static void acpi_bus_osc_negotiate_platform_control(void) if (ACPI_FAILURE(acpi_run_osc(handle, &context))) return; - capbuf_ret = context.ret.pointer; - if (context.ret.length <= OSC_SUPPORT_DWORD) { - kfree(context.ret.pointer); - return; + if (context.ret.length > OSC_SUPPORT_DWORD) { + u32 *capbuf_ret = context.ret.pointer; + + caps = capbuf_ret[OSC_SUPPORT_DWORD]; } + kfree(context.ret.pointer); + + osc_sb_apei_support_acked = caps & OSC_SB_APEI_SUPPORT; + osc_pc_lpi_support_confirmed = caps & OSC_SB_PCLPI_SUPPORT; + osc_sb_native_usb4_support_confirmed = caps & OSC_SB_NATIVE_USB4_SUPPORT; /* - * Now run _OSC again with query flag clear and with the caps - * supported by both the OS and the platform. + * On machines with native USB4 support, run _OSC again with the query + * flag cleared and with the caps supported by both the OS and the + * platform to let the firmware know we support native USB4. */ - capbuf[OSC_QUERY_DWORD] = 0; - capbuf[OSC_SUPPORT_DWORD] = capbuf_ret[OSC_SUPPORT_DWORD]; - kfree(context.ret.pointer); + if (osc_sb_native_usb4_support_confirmed) { + capbuf[OSC_QUERY_DWORD] = 0; + capbuf[OSC_SUPPORT_DWORD] = caps; - if (ACPI_FAILURE(acpi_run_osc(handle, &context))) - return; + if (ACPI_FAILURE(acpi_run_osc(handle, &context))) + return; - capbuf_ret = context.ret.pointer; - if (context.ret.length > OSC_SUPPORT_DWORD) { - osc_sb_apei_support_acked = - capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT; - osc_pc_lpi_support_confirmed = - capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT; - osc_sb_native_usb4_support_confirmed = - capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_NATIVE_USB4_SUPPORT; + kfree(context.ret.pointer); } - - kfree(context.ret.pointer); } /* -- 2.31.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: Some SSDT tables are not loading with kernel >= 5.12 2021-06-08 9:50 ` Hans de Goede @ 2021-06-08 11:44 ` Mika Westerberg 2021-06-08 13:01 ` Mika Westerberg 2021-06-08 13:24 ` Hans de Goede 0 siblings, 2 replies; 15+ messages in thread From: Mika Westerberg @ 2021-06-08 11:44 UTC (permalink / raw) To: Hans de Goede Cc: Mario Limonciello, Rafael J. Wysocki, Rafael J . Wysocki, linux-acpi Hi, On Tue, Jun 08, 2021 at 11:50:15AM +0200, Hans de Goede wrote: > Hi, > > On 6/7/21 9:18 PM, Hans de Goede wrote: > > Hi, > > > > On 6/7/21 6:08 PM, Mika Westerberg wrote: > >> Hi, > >> > >> Tried now on ADL-P and TGL systems and the _OSC still works properly. > >> > >> Thanks Hans for fixing! > >> > >> Feel free to add my Tested-by. > > > > Thank you for testing, unfortunately so far from the comments here: > > > > https://bugzilla.kernel.org/show_bug.cgi?id=213023 > > > > it seems that my patch does not help resolve the issues caused > > by commit 719e1f561afb ("ACPI: Execute platform _OSC also with query > > bit clear"), where as reverting that commit does resolve them :| > > > > Does anyone have any other ideas how to fix this ? > > The reporter who has done the bisect has commented out the new/second > _OSC call and that fixes things for them. So I've written a new fix > (attached), note just as before this is untested ATM. > > Mika, if you can test this one (it replaces the previous one) > on machines with native USB4 support to check those don't regress then > that would be great. I can test it sure, but first let's try to understand what the problem is :) > I've asked the various reporters from the 2 bugzilla's for this to also > test this new patch. I'll let you know how that goes. The _OSC on at least one of the affected platforms look like this: If ((Arg0 == ToUUID ("0811b06e-4a27-44f9-8d60-3cbbc22e7b48") /* Platform-wide Capabilities */)) { If ((Arg1 == One)) { OSCP = CAP0 /* \_SB_._OSC.CAP0 */ If ((CAP0 & 0x04)) { OSCO = 0x04 If (((SGMD & 0x0F) != 0x02)) { If ((RTD3 == Zero)) { CAP0 &= 0x3B STS0 |= 0x10 } } } } Else { STS0 &= 0xFFFFFF00 STS0 |= 0x0A } } Else { STS0 &= 0xFFFFFF00 STS0 |= 0x06 } Probably it is fine to call it several times but the issue is with the mask that it does: CAP0 &= 0x3B This clears out the upper bits. I think this is actually a BIOS bug as it ends up clearing OSC_SB_PCLPI_SUPPORT which is probably not intented, and that seems to cause skipping of the LPI tables or something like that. The alternative is to pass the original caps to the second _OSC call. I think this is safe too. While looking at the code, I found a couple of other issues that should be fixed with the below hack patch. What do you think about this approach? diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index be7da23fad76..80ff81bb668b 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -290,7 +290,7 @@ static void acpi_bus_osc_negotiate_platform_control(void) struct acpi_osc_context context = { .uuid_str = sb_uuid_str, .rev = 1, - .cap.length = 8, + .cap.length = sizeof(capbuf), .cap.pointer = capbuf, }; acpi_handle handle; @@ -330,32 +330,21 @@ static void acpi_bus_osc_negotiate_platform_control(void) if (ACPI_FAILURE(acpi_run_osc(handle, &context))) return; - capbuf_ret = context.ret.pointer; - if (context.ret.length <= OSC_SUPPORT_DWORD) { - kfree(context.ret.pointer); - return; - } + kfree(context.ret.pointer); - /* - * Now run _OSC again with query flag clear and with the caps - * supported by both the OS and the platform. - */ + /* Now run _OSC again with query flag clear */ capbuf[OSC_QUERY_DWORD] = 0; - capbuf[OSC_SUPPORT_DWORD] = capbuf_ret[OSC_SUPPORT_DWORD]; - kfree(context.ret.pointer); if (ACPI_FAILURE(acpi_run_osc(handle, &context))) return; capbuf_ret = context.ret.pointer; - if (context.ret.length > OSC_SUPPORT_DWORD) { - osc_sb_apei_support_acked = - capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT; - osc_pc_lpi_support_confirmed = - capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT; - osc_sb_native_usb4_support_confirmed = - capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_NATIVE_USB4_SUPPORT; - } + osc_sb_apei_support_acked = + capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT; + osc_pc_lpi_support_confirmed = + capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT; + osc_sb_native_usb4_support_confirmed = + capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_NATIVE_USB4_SUPPORT; kfree(context.ret.pointer); } ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: Some SSDT tables are not loading with kernel >= 5.12 2021-06-08 11:44 ` Mika Westerberg @ 2021-06-08 13:01 ` Mika Westerberg 2021-06-08 13:24 ` Hans de Goede 1 sibling, 0 replies; 15+ messages in thread From: Mika Westerberg @ 2021-06-08 13:01 UTC (permalink / raw) To: Hans de Goede Cc: Mario Limonciello, Rafael J. Wysocki, Rafael J . Wysocki, linux-acpi On Tue, Jun 08, 2021 at 02:45:03PM +0300, Mika Westerberg wrote: > Hi, > > On Tue, Jun 08, 2021 at 11:50:15AM +0200, Hans de Goede wrote: > > Hi, > > > > On 6/7/21 9:18 PM, Hans de Goede wrote: > > > Hi, > > > > > > On 6/7/21 6:08 PM, Mika Westerberg wrote: > > >> Hi, > > >> > > >> Tried now on ADL-P and TGL systems and the _OSC still works properly. > > >> > > >> Thanks Hans for fixing! > > >> > > >> Feel free to add my Tested-by. > > > > > > Thank you for testing, unfortunately so far from the comments here: > > > > > > https://bugzilla.kernel.org/show_bug.cgi?id=213023 > > > > > > it seems that my patch does not help resolve the issues caused > > > by commit 719e1f561afb ("ACPI: Execute platform _OSC also with query > > > bit clear"), where as reverting that commit does resolve them :| > > > > > > Does anyone have any other ideas how to fix this ? > > > > The reporter who has done the bisect has commented out the new/second > > _OSC call and that fixes things for them. So I've written a new fix > > (attached), note just as before this is untested ATM. > > > > Mika, if you can test this one (it replaces the previous one) > > on machines with native USB4 support to check those don't regress then > > that would be great. > > I can test it sure, but first let's try to understand what the problem is :) FYI, I also tested your patch and it still works on my test system so if we decided to go with that then feel free to add my Tested-by to the patch too. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Some SSDT tables are not loading with kernel >= 5.12 2021-06-08 11:44 ` Mika Westerberg 2021-06-08 13:01 ` Mika Westerberg @ 2021-06-08 13:24 ` Hans de Goede 2021-06-08 15:03 ` Mika Westerberg 1 sibling, 1 reply; 15+ messages in thread From: Hans de Goede @ 2021-06-08 13:24 UTC (permalink / raw) To: Mika Westerberg Cc: Mario Limonciello, Rafael J. Wysocki, Rafael J . Wysocki, linux-acpi Hi, On 6/8/21 1:44 PM, Mika Westerberg wrote: > Hi, > > On Tue, Jun 08, 2021 at 11:50:15AM +0200, Hans de Goede wrote: >> Hi, >> >> On 6/7/21 9:18 PM, Hans de Goede wrote: >>> Hi, >>> >>> On 6/7/21 6:08 PM, Mika Westerberg wrote: >>>> Hi, >>>> >>>> Tried now on ADL-P and TGL systems and the _OSC still works properly. >>>> >>>> Thanks Hans for fixing! >>>> >>>> Feel free to add my Tested-by. >>> >>> Thank you for testing, unfortunately so far from the comments here: >>> >>> https://bugzilla.kernel.org/show_bug.cgi?id=213023 >>> >>> it seems that my patch does not help resolve the issues caused >>> by commit 719e1f561afb ("ACPI: Execute platform _OSC also with query >>> bit clear"), where as reverting that commit does resolve them :| >>> >>> Does anyone have any other ideas how to fix this ? >> >> The reporter who has done the bisect has commented out the new/second >> _OSC call and that fixes things for them. So I've written a new fix >> (attached), note just as before this is untested ATM. >> >> Mika, if you can test this one (it replaces the previous one) >> on machines with native USB4 support to check those don't regress then >> that would be great. > > I can test it sure, but first let's try to understand what the problem is :) > >> I've asked the various reporters from the 2 bugzilla's for this to also >> test this new patch. I'll let you know how that goes. > > The _OSC on at least one of the affected platforms look like this: > > If ((Arg0 == ToUUID ("0811b06e-4a27-44f9-8d60-3cbbc22e7b48") /* Platform-wide Capabilities */)) > { > If ((Arg1 == One)) > { > OSCP = CAP0 /* \_SB_._OSC.CAP0 */ > If ((CAP0 & 0x04)) > { > OSCO = 0x04 > If (((SGMD & 0x0F) != 0x02)) > { > If ((RTD3 == Zero)) > { > CAP0 &= 0x3B > STS0 |= 0x10 > } > } > } > } > Else > { > STS0 &= 0xFFFFFF00 > STS0 |= 0x0A > } > } > Else > { > STS0 &= 0xFFFFFF00 > STS0 |= 0x06 > } > > Probably it is fine to call it several times but the issue is with the mask > that it does: > > CAP0 &= 0x3B > > This clears out the upper bits. I think this is actually a BIOS bug as it ends > up clearing OSC_SB_PCLPI_SUPPORT which is probably not intented, and that seems > to cause skipping of the LPI tables or something like that. > > The alternative is to pass the original caps to the second _OSC call. I think > this is safe too. While looking at the code, I found a couple of other issues > that should be fixed with the below hack patch. > > What do you think about this approach? I think you might be on to something, quoting from the spec: """ 6.2.11.1.3 Sequence of _OSC calls The following rules govern sequences of calls to _OSC that are issued to the same host bridge and occur within the same boot. • The OS is permitted to evaluate _OSC an arbitrary number of times. • If the OS declares support of a feature in the Status Field in one call to _OSC, then it must preserve the set state of that bit (declaring support for that feature) in all subsequent calls. • If the OS is granted control of a feature in the Control Field in one call to _OSC, then it must preserve the set state of that bit (requesting that feature) in all subsequent calls. """ So the spec is saying that we should indeed keep all the flags which set during the first call also set during subsequent calls. If you can turn this into a proper patch then I can ask the reporters of the 2 bugs to test that patch. Regards, Hans > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c > index be7da23fad76..80ff81bb668b 100644 > --- a/drivers/acpi/bus.c > +++ b/drivers/acpi/bus.c > @@ -290,7 +290,7 @@ static void acpi_bus_osc_negotiate_platform_control(void) > struct acpi_osc_context context = { > .uuid_str = sb_uuid_str, > .rev = 1, > - .cap.length = 8, > + .cap.length = sizeof(capbuf), > .cap.pointer = capbuf, > }; > acpi_handle handle; > @@ -330,32 +330,21 @@ static void acpi_bus_osc_negotiate_platform_control(void) > if (ACPI_FAILURE(acpi_run_osc(handle, &context))) > return; > > - capbuf_ret = context.ret.pointer; > - if (context.ret.length <= OSC_SUPPORT_DWORD) { > - kfree(context.ret.pointer); > - return; > - } > + kfree(context.ret.pointer); > > - /* > - * Now run _OSC again with query flag clear and with the caps > - * supported by both the OS and the platform. > - */ > + /* Now run _OSC again with query flag clear */ > capbuf[OSC_QUERY_DWORD] = 0; > - capbuf[OSC_SUPPORT_DWORD] = capbuf_ret[OSC_SUPPORT_DWORD]; > - kfree(context.ret.pointer); > > if (ACPI_FAILURE(acpi_run_osc(handle, &context))) > return; > > capbuf_ret = context.ret.pointer; > - if (context.ret.length > OSC_SUPPORT_DWORD) { > - osc_sb_apei_support_acked = > - capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT; > - osc_pc_lpi_support_confirmed = > - capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT; > - osc_sb_native_usb4_support_confirmed = > - capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_NATIVE_USB4_SUPPORT; > - } > + osc_sb_apei_support_acked = > + capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT; > + osc_pc_lpi_support_confirmed = > + capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT; > + osc_sb_native_usb4_support_confirmed = > + capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_NATIVE_USB4_SUPPORT; > > kfree(context.ret.pointer); > } > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: Some SSDT tables are not loading with kernel >= 5.12 2021-06-08 13:24 ` Hans de Goede @ 2021-06-08 15:03 ` Mika Westerberg 2021-06-08 16:38 ` [PATCH] ACPI: Pass the same capabilities to the _OSC regardless of the query flag Mika Westerberg 0 siblings, 1 reply; 15+ messages in thread From: Mika Westerberg @ 2021-06-08 15:03 UTC (permalink / raw) To: Hans de Goede Cc: Mario Limonciello, Rafael J. Wysocki, Rafael J . Wysocki, linux-acpi Hi Hans, On Tue, Jun 08, 2021 at 03:24:28PM +0200, Hans de Goede wrote: > Hi, > > On 6/8/21 1:44 PM, Mika Westerberg wrote: > > Hi, > > > > On Tue, Jun 08, 2021 at 11:50:15AM +0200, Hans de Goede wrote: > >> Hi, > >> > >> On 6/7/21 9:18 PM, Hans de Goede wrote: > >>> Hi, > >>> > >>> On 6/7/21 6:08 PM, Mika Westerberg wrote: > >>>> Hi, > >>>> > >>>> Tried now on ADL-P and TGL systems and the _OSC still works properly. > >>>> > >>>> Thanks Hans for fixing! > >>>> > >>>> Feel free to add my Tested-by. > >>> > >>> Thank you for testing, unfortunately so far from the comments here: > >>> > >>> https://bugzilla.kernel.org/show_bug.cgi?id=213023 > >>> > >>> it seems that my patch does not help resolve the issues caused > >>> by commit 719e1f561afb ("ACPI: Execute platform _OSC also with query > >>> bit clear"), where as reverting that commit does resolve them :| > >>> > >>> Does anyone have any other ideas how to fix this ? > >> > >> The reporter who has done the bisect has commented out the new/second > >> _OSC call and that fixes things for them. So I've written a new fix > >> (attached), note just as before this is untested ATM. > >> > >> Mika, if you can test this one (it replaces the previous one) > >> on machines with native USB4 support to check those don't regress then > >> that would be great. > > > > I can test it sure, but first let's try to understand what the problem is :) > > > >> I've asked the various reporters from the 2 bugzilla's for this to also > >> test this new patch. I'll let you know how that goes. > > > > The _OSC on at least one of the affected platforms look like this: > > > > If ((Arg0 == ToUUID ("0811b06e-4a27-44f9-8d60-3cbbc22e7b48") /* Platform-wide Capabilities */)) > > { > > If ((Arg1 == One)) > > { > > OSCP = CAP0 /* \_SB_._OSC.CAP0 */ > > If ((CAP0 & 0x04)) > > { > > OSCO = 0x04 > > If (((SGMD & 0x0F) != 0x02)) > > { > > If ((RTD3 == Zero)) > > { > > CAP0 &= 0x3B > > STS0 |= 0x10 > > } > > } > > } > > } > > Else > > { > > STS0 &= 0xFFFFFF00 > > STS0 |= 0x0A > > } > > } > > Else > > { > > STS0 &= 0xFFFFFF00 > > STS0 |= 0x06 > > } > > > > Probably it is fine to call it several times but the issue is with the mask > > that it does: > > > > CAP0 &= 0x3B > > > > This clears out the upper bits. I think this is actually a BIOS bug as it ends > > up clearing OSC_SB_PCLPI_SUPPORT which is probably not intented, and that seems > > to cause skipping of the LPI tables or something like that. > > > > The alternative is to pass the original caps to the second _OSC call. I think > > this is safe too. While looking at the code, I found a couple of other issues > > that should be fixed with the below hack patch. > > > > What do you think about this approach? > > I think you might be on to something, quoting from the spec: > > """ > 6.2.11.1.3 Sequence of _OSC calls > The following rules govern sequences of calls to _OSC that are issued to the same host bridge and > occur within the same boot. > • The OS is permitted to evaluate _OSC an arbitrary number of times. > • If the OS declares support of a feature in the Status Field in one call to _OSC, then it must > preserve the set state of that bit (declaring support for that feature) in all subsequent calls. > • If the OS is granted control of a feature in the Control Field in one call to _OSC, then it must > preserve the set state of that bit (requesting that feature) in all subsequent calls. > """ > > So the spec is saying that we should indeed keep all the flags which set during > the first call also set during subsequent calls. > > If you can turn this into a proper patch then I can ask the reporters of > the 2 bugs to test that patch. Sure I will. First I think I figured why this happens. The BIOS loads the HWP tables dynamically (in ssdt9.dsl) like this: \_PR.PRxx.GCAP(): If ((OSYS >= 0x07DF)) { If (((CFGD & 0x00400000) && !(SDTL & 0x40))) { If ((\_SB.OSCP & 0x40)) { SDTL |= 0x40 OperationRegion (HWP0, SystemMemory, DerefOf (SSDT [0x0D]), DerefOf (SSDT [0x0E])) Load (HWP0, HW0) /* \_PR_.PR00.HW0_ */ If ((CFGD & 0x00800000)) { OperationRegion (HWPL, SystemMemory, DerefOf (SSDT [0x13]), DerefOf (SSDT [0x14])) Load (HWPL, HW2) /* \_PR_.PR00.HW2_ */ } } Note it checks the \_SB.OSCP which is set in _OSC to the value of the "support" buffer that Linux populates. However, in _OSC it also clears that particular bit (when RTD3 is set to 0): CAP0 &= 0x3B STS0 |= 0x10 Since Linux calls the _OSC again with the cleared bit the \_SB.OSCP also does not have that bit set anymore and that makes GCAP() to skip the Load() operation resulting the errors users have reported. This looks like that the BIOS expects the same set of "support" bits to be set on each call, or alternatively it only expects the _OSC to be run once. In any case, I will make a proper patch soon with the above added to the commit log too. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH] ACPI: Pass the same capabilities to the _OSC regardless of the query flag 2021-06-08 15:03 ` Mika Westerberg @ 2021-06-08 16:38 ` Mika Westerberg 2021-06-08 17:10 ` Hans de Goede 0 siblings, 1 reply; 15+ messages in thread From: Mika Westerberg @ 2021-06-08 16:38 UTC (permalink / raw) To: Hans de Goede, Rafael J. Wysocki Cc: Mario Limonciello, Mika Westerberg, linux-acpi Commit 719e1f561afb ("ACPI: Execute platform _OSC also with query bit clear") makes acpi_bus_osc_negotiate_platform_control() not only query the platforms capabilities but it also commits the result back to the firmware to report which capabilities are supported by the OS back to the firmware On certain systems the BIOS loads SSDT tables dynamically based on the capabilities the OS claims to support. However, on these systems the _OSC actually clears some of the bits (under certain conditions) so what happens is that now when we call the _OSC twice the second time we pass the cleared values and that results errors like below to appear on the system log: ACPI BIOS Error (bug): Could not resolve symbol [\_PR.PR00._CPC], AE_NOT_FOUND (20210105/psargs-330) ACPI Error: Aborting method \_PR.PR01._CPC due to previous error (AE_NOT_FOUND) (20210105/psparse-529) In addition the ACPI 6.4 spec says following [1]: If the OS declares support of a feature in the Support Field in one call to _OSC, then it must preserve the set state of that bit (declaring support for that feature) in all subsequent calls. Based on the above we can fix the issue by passing the same set of capabilities to the platform wide _OSC in both calls regardless of the query flag. While there drop the context.ret.length check which was wrong to begin with (as the length is number of bytes not elements). This is already checked in acpi_run_osc() that also returns an error in that case. [1] https://uefi.org/specs/ACPI/6.4/06_Device_Configuration/Device_Configuration.html#sequence-of-osc-calls BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=213023 BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1963717 Fixes: 719e1f561afb ("ACPI: Execute platform _OSC also with query bit clear") Cc: Mario Limonciello <mario.limonciello@outlook.com> cc: Hans de Goede <hdegoede@redhat.com> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> --- drivers/acpi/bus.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index be7da23fad76..61e8c02595ac 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -336,26 +336,19 @@ static void acpi_bus_osc_negotiate_platform_control(void) return; } - /* - * Now run _OSC again with query flag clear and with the caps - * supported by both the OS and the platform. - */ + /* Now run _OSC again with query flag clear */ capbuf[OSC_QUERY_DWORD] = 0; - capbuf[OSC_SUPPORT_DWORD] = capbuf_ret[OSC_SUPPORT_DWORD]; - kfree(context.ret.pointer); if (ACPI_FAILURE(acpi_run_osc(handle, &context))) return; capbuf_ret = context.ret.pointer; - if (context.ret.length > OSC_SUPPORT_DWORD) { - osc_sb_apei_support_acked = - capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT; - osc_pc_lpi_support_confirmed = - capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT; - osc_sb_native_usb4_support_confirmed = - capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_NATIVE_USB4_SUPPORT; - } + osc_sb_apei_support_acked = + capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT; + osc_pc_lpi_support_confirmed = + capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT; + osc_sb_native_usb4_support_confirmed = + capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_NATIVE_USB4_SUPPORT; kfree(context.ret.pointer); } -- 2.30.2 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] ACPI: Pass the same capabilities to the _OSC regardless of the query flag 2021-06-08 16:38 ` [PATCH] ACPI: Pass the same capabilities to the _OSC regardless of the query flag Mika Westerberg @ 2021-06-08 17:10 ` Hans de Goede 2021-06-09 7:58 ` Hans de Goede 0 siblings, 1 reply; 15+ messages in thread From: Hans de Goede @ 2021-06-08 17:10 UTC (permalink / raw) To: Mika Westerberg, Rafael J. Wysocki; +Cc: Mario Limonciello, linux-acpi Hi, On 6/8/21 6:38 PM, Mika Westerberg wrote: > Commit 719e1f561afb ("ACPI: Execute platform _OSC also with query bit > clear") makes acpi_bus_osc_negotiate_platform_control() not only query > the platforms capabilities but it also commits the result back to the > firmware to report which capabilities are supported by the OS back to > the firmware > > On certain systems the BIOS loads SSDT tables dynamically based on the > capabilities the OS claims to support. However, on these systems the > _OSC actually clears some of the bits (under certain conditions) so what > happens is that now when we call the _OSC twice the second time we pass > the cleared values and that results errors like below to appear on the > system log: > > ACPI BIOS Error (bug): Could not resolve symbol [\_PR.PR00._CPC], AE_NOT_FOUND (20210105/psargs-330) > ACPI Error: Aborting method \_PR.PR01._CPC due to previous error (AE_NOT_FOUND) (20210105/psparse-529) > > In addition the ACPI 6.4 spec says following [1]: > > If the OS declares support of a feature in the Support Field in one > call to _OSC, then it must preserve the set state of that bit > (declaring support for that feature) in all subsequent calls. > > Based on the above we can fix the issue by passing the same set of > capabilities to the platform wide _OSC in both calls regardless of the > query flag. > > While there drop the context.ret.length check which was wrong to begin > with (as the length is number of bytes not elements). This is already > checked in acpi_run_osc() that also returns an error in that case. > > [1] https://uefi.org/specs/ACPI/6.4/06_Device_Configuration/Device_Configuration.html#sequence-of-osc-calls > > BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=213023 > BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1963717 > Fixes: 719e1f561afb ("ACPI: Execute platform _OSC also with query bit clear") > Cc: Mario Limonciello <mario.limonciello@outlook.com> > cc: Hans de Goede <hdegoede@redhat.com> > Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > --- > drivers/acpi/bus.c | 21 +++++++-------------- > 1 file changed, 7 insertions(+), 14 deletions(-) > > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c > index be7da23fad76..61e8c02595ac 100644 > --- a/drivers/acpi/bus.c > +++ b/drivers/acpi/bus.c > @@ -336,26 +336,19 @@ static void acpi_bus_osc_negotiate_platform_control(void) > return; > } > > - /* > - * Now run _OSC again with query flag clear and with the caps > - * supported by both the OS and the platform. > - */ > + /* Now run _OSC again with query flag clear */ > capbuf[OSC_QUERY_DWORD] = 0; > - capbuf[OSC_SUPPORT_DWORD] = capbuf_ret[OSC_SUPPORT_DWORD]; > - kfree(context.ret.pointer); This kfree needs to be moved up, rather then be completely removed and you are still leaving 1 of the unnecessary length checks in place. I've added this fixup on top, to fix both these issues: --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -330,11 +330,7 @@ static void acpi_bus_osc_negotiate_platform_control(void) if (ACPI_FAILURE(acpi_run_osc(handle, &context))) return; - capbuf_ret = context.ret.pointer; - if (context.ret.length <= OSC_SUPPORT_DWORD) { - kfree(context.ret.pointer); - return; - } + kfree(context.ret.pointer); /* Now run _OSC again with query flag clear */ capbuf[OSC_QUERY_DWORD] = 0; I'll ask the reporters of: https://bugzilla.kernel.org/show_bug.cgi?id=213023 https://bugzilla.redhat.com/show_bug.cgi?id=1963717 To test the (fixed-up) patch, so that they can confirm if this indeed fixes things. Regards, Hans > > if (ACPI_FAILURE(acpi_run_osc(handle, &context))) > return; > > capbuf_ret = context.ret.pointer; > - if (context.ret.length > OSC_SUPPORT_DWORD) { > - osc_sb_apei_support_acked = > - capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT; > - osc_pc_lpi_support_confirmed = > - capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT; > - osc_sb_native_usb4_support_confirmed = > - capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_NATIVE_USB4_SUPPORT; > - } > + osc_sb_apei_support_acked = > + capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT; > + osc_pc_lpi_support_confirmed = > + capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT; > + osc_sb_native_usb4_support_confirmed = > + capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_NATIVE_USB4_SUPPORT; > > kfree(context.ret.pointer); > } > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ACPI: Pass the same capabilities to the _OSC regardless of the query flag 2021-06-08 17:10 ` Hans de Goede @ 2021-06-09 7:58 ` Hans de Goede 2021-06-09 9:24 ` Mika Westerberg 0 siblings, 1 reply; 15+ messages in thread From: Hans de Goede @ 2021-06-09 7:58 UTC (permalink / raw) To: Mika Westerberg, Rafael J. Wysocki; +Cc: Mario Limonciello, linux-acpi Hi, On 6/8/21 7:10 PM, Hans de Goede wrote: > Hi, > > On 6/8/21 6:38 PM, Mika Westerberg wrote: >> Commit 719e1f561afb ("ACPI: Execute platform _OSC also with query bit >> clear") makes acpi_bus_osc_negotiate_platform_control() not only query >> the platforms capabilities but it also commits the result back to the >> firmware to report which capabilities are supported by the OS back to >> the firmware >> >> On certain systems the BIOS loads SSDT tables dynamically based on the >> capabilities the OS claims to support. However, on these systems the >> _OSC actually clears some of the bits (under certain conditions) so what >> happens is that now when we call the _OSC twice the second time we pass >> the cleared values and that results errors like below to appear on the >> system log: >> >> ACPI BIOS Error (bug): Could not resolve symbol [\_PR.PR00._CPC], AE_NOT_FOUND (20210105/psargs-330) >> ACPI Error: Aborting method \_PR.PR01._CPC due to previous error (AE_NOT_FOUND) (20210105/psparse-529) >> >> In addition the ACPI 6.4 spec says following [1]: >> >> If the OS declares support of a feature in the Support Field in one >> call to _OSC, then it must preserve the set state of that bit >> (declaring support for that feature) in all subsequent calls. >> >> Based on the above we can fix the issue by passing the same set of >> capabilities to the platform wide _OSC in both calls regardless of the >> query flag. >> >> While there drop the context.ret.length check which was wrong to begin >> with (as the length is number of bytes not elements). This is already >> checked in acpi_run_osc() that also returns an error in that case. >> >> [1] https://uefi.org/specs/ACPI/6.4/06_Device_Configuration/Device_Configuration.html#sequence-of-osc-calls >> >> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=213023 >> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1963717 >> Fixes: 719e1f561afb ("ACPI: Execute platform _OSC also with query bit clear") >> Cc: Mario Limonciello <mario.limonciello@outlook.com> >> cc: Hans de Goede <hdegoede@redhat.com> >> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> >> --- >> drivers/acpi/bus.c | 21 +++++++-------------- >> 1 file changed, 7 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c >> index be7da23fad76..61e8c02595ac 100644 >> --- a/drivers/acpi/bus.c >> +++ b/drivers/acpi/bus.c >> @@ -336,26 +336,19 @@ static void acpi_bus_osc_negotiate_platform_control(void) >> return; >> } >> >> - /* >> - * Now run _OSC again with query flag clear and with the caps >> - * supported by both the OS and the platform. >> - */ >> + /* Now run _OSC again with query flag clear */ >> capbuf[OSC_QUERY_DWORD] = 0; >> - capbuf[OSC_SUPPORT_DWORD] = capbuf_ret[OSC_SUPPORT_DWORD]; >> - kfree(context.ret.pointer); > > This kfree needs to be moved up, rather then be completely removed > and you are still leaving 1 of the unnecessary length checks in place. > > I've added this fixup on top, to fix both these issues: > > --- a/drivers/acpi/bus.c > +++ b/drivers/acpi/bus.c > @@ -330,11 +330,7 @@ static void acpi_bus_osc_negotiate_platform_control(void) > if (ACPI_FAILURE(acpi_run_osc(handle, &context))) > return; > > - capbuf_ret = context.ret.pointer; > - if (context.ret.length <= OSC_SUPPORT_DWORD) { > - kfree(context.ret.pointer); > - return; > - } > + kfree(context.ret.pointer); > > /* Now run _OSC again with query flag clear */ > capbuf[OSC_QUERY_DWORD] = 0; > > I'll ask the reporters of: > https://bugzilla.kernel.org/show_bug.cgi?id=213023 > https://bugzilla.redhat.com/show_bug.cgi?id=1963717 > > To test the (fixed-up) patch, so that they can confirm if this indeed > fixes things. I've received confirmation from 2 users that this patch (with the fixup) fixes this. Can send a v2 with the fixup squashed in for Rafael to pick up? Regards, Hans >> >> if (ACPI_FAILURE(acpi_run_osc(handle, &context))) >> return; >> >> capbuf_ret = context.ret.pointer; >> - if (context.ret.length > OSC_SUPPORT_DWORD) { >> - osc_sb_apei_support_acked = >> - capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT; >> - osc_pc_lpi_support_confirmed = >> - capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT; >> - osc_sb_native_usb4_support_confirmed = >> - capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_NATIVE_USB4_SUPPORT; >> - } >> + osc_sb_apei_support_acked = >> + capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_APEI_SUPPORT; >> + osc_pc_lpi_support_confirmed = >> + capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_PCLPI_SUPPORT; >> + osc_sb_native_usb4_support_confirmed = >> + capbuf_ret[OSC_SUPPORT_DWORD] & OSC_SB_NATIVE_USB4_SUPPORT; >> >> kfree(context.ret.pointer); >> } >> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] ACPI: Pass the same capabilities to the _OSC regardless of the query flag 2021-06-09 7:58 ` Hans de Goede @ 2021-06-09 9:24 ` Mika Westerberg 0 siblings, 0 replies; 15+ messages in thread From: Mika Westerberg @ 2021-06-09 9:24 UTC (permalink / raw) To: Hans de Goede; +Cc: Rafael J. Wysocki, Mario Limonciello, linux-acpi On Wed, Jun 09, 2021 at 09:58:31AM +0200, Hans de Goede wrote: > Hi, > > On 6/8/21 7:10 PM, Hans de Goede wrote: > > Hi, > > > > On 6/8/21 6:38 PM, Mika Westerberg wrote: > >> Commit 719e1f561afb ("ACPI: Execute platform _OSC also with query bit > >> clear") makes acpi_bus_osc_negotiate_platform_control() not only query > >> the platforms capabilities but it also commits the result back to the > >> firmware to report which capabilities are supported by the OS back to > >> the firmware > >> > >> On certain systems the BIOS loads SSDT tables dynamically based on the > >> capabilities the OS claims to support. However, on these systems the > >> _OSC actually clears some of the bits (under certain conditions) so what > >> happens is that now when we call the _OSC twice the second time we pass > >> the cleared values and that results errors like below to appear on the > >> system log: > >> > >> ACPI BIOS Error (bug): Could not resolve symbol [\_PR.PR00._CPC], AE_NOT_FOUND (20210105/psargs-330) > >> ACPI Error: Aborting method \_PR.PR01._CPC due to previous error (AE_NOT_FOUND) (20210105/psparse-529) > >> > >> In addition the ACPI 6.4 spec says following [1]: > >> > >> If the OS declares support of a feature in the Support Field in one > >> call to _OSC, then it must preserve the set state of that bit > >> (declaring support for that feature) in all subsequent calls. > >> > >> Based on the above we can fix the issue by passing the same set of > >> capabilities to the platform wide _OSC in both calls regardless of the > >> query flag. > >> > >> While there drop the context.ret.length check which was wrong to begin > >> with (as the length is number of bytes not elements). This is already > >> checked in acpi_run_osc() that also returns an error in that case. > >> > >> [1] https://uefi.org/specs/ACPI/6.4/06_Device_Configuration/Device_Configuration.html#sequence-of-osc-calls > >> > >> BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=213023 > >> BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1963717 > >> Fixes: 719e1f561afb ("ACPI: Execute platform _OSC also with query bit clear") > >> Cc: Mario Limonciello <mario.limonciello@outlook.com> > >> cc: Hans de Goede <hdegoede@redhat.com> > >> Signed-off-by: Mika Westerberg <mika.westerberg@linux.intel.com> > >> --- > >> drivers/acpi/bus.c | 21 +++++++-------------- > >> 1 file changed, 7 insertions(+), 14 deletions(-) > >> > >> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c > >> index be7da23fad76..61e8c02595ac 100644 > >> --- a/drivers/acpi/bus.c > >> +++ b/drivers/acpi/bus.c > >> @@ -336,26 +336,19 @@ static void acpi_bus_osc_negotiate_platform_control(void) > >> return; > >> } > >> > >> - /* > >> - * Now run _OSC again with query flag clear and with the caps > >> - * supported by both the OS and the platform. > >> - */ > >> + /* Now run _OSC again with query flag clear */ > >> capbuf[OSC_QUERY_DWORD] = 0; > >> - capbuf[OSC_SUPPORT_DWORD] = capbuf_ret[OSC_SUPPORT_DWORD]; > >> - kfree(context.ret.pointer); > > > > This kfree needs to be moved up, rather then be completely removed > > and you are still leaving 1 of the unnecessary length checks in place. > > > > I've added this fixup on top, to fix both these issues: > > > > --- a/drivers/acpi/bus.c > > +++ b/drivers/acpi/bus.c > > @@ -330,11 +330,7 @@ static void acpi_bus_osc_negotiate_platform_control(void) > > if (ACPI_FAILURE(acpi_run_osc(handle, &context))) > > return; > > > > - capbuf_ret = context.ret.pointer; > > - if (context.ret.length <= OSC_SUPPORT_DWORD) { > > - kfree(context.ret.pointer); > > - return; > > - } > > + kfree(context.ret.pointer); > > > > /* Now run _OSC again with query flag clear */ > > capbuf[OSC_QUERY_DWORD] = 0; > > > > I'll ask the reporters of: > > https://bugzilla.kernel.org/show_bug.cgi?id=213023 > > https://bugzilla.redhat.com/show_bug.cgi?id=1963717 > > > > To test the (fixed-up) patch, so that they can confirm if this indeed > > fixes things. > > I've received confirmation from 2 users that this patch (with the fixup) > fixes this. Can send a v2 with the fixup squashed in for Rafael to pick up? Thanks Hans for the fixup and checking with the users! I will send the v2 with your fixup soon. ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-06-09 9:24 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-03 17:26 Some SSDT tables are not loading with kernel >= 5.12 Hans de Goede
2021-06-07 9:43 ` Hans de Goede
2021-06-07 10:05 ` Hans de Goede
2021-06-07 11:13 ` Rafael J. Wysocki
[not found] ` <FR1PR80MB5051E91269FD36681BB357A7E1389@FR1PR80MB5051.lamprd80.prod.outlook.com>
2021-06-07 16:08 ` Mika Westerberg
2021-06-07 19:18 ` Hans de Goede
2021-06-08 9:50 ` Hans de Goede
2021-06-08 11:44 ` Mika Westerberg
2021-06-08 13:01 ` Mika Westerberg
2021-06-08 13:24 ` Hans de Goede
2021-06-08 15:03 ` Mika Westerberg
2021-06-08 16:38 ` [PATCH] ACPI: Pass the same capabilities to the _OSC regardless of the query flag Mika Westerberg
2021-06-08 17:10 ` Hans de Goede
2021-06-09 7:58 ` Hans de Goede
2021-06-09 9:24 ` Mika Westerberg
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.