linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC 0/2]  Stop the abuse of Linux-* _OSI strings
@ 2022-08-19 14:25 Mario Limonciello
  2022-08-19 14:25 ` [RFC 1/2] ACPI: OSI: Remove Linux-Dell-Video _OSI string Mario Limonciello
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Mario Limonciello @ 2022-08-19 14:25 UTC (permalink / raw)
  To: linux-kernel, linux-acpi, rafael, Len Brown
  Cc: kherbst, nouveau, hdegoede, ddadap, kai.heng.feng,
	Dell.Client.Kernel, Mario Limonciello

3 _OSI strings were introduced in recent years that were intended
to workaround very specific problems found on specific systems.

The idea was supposed to be that these quirks were only used on
those systems, but this proved to be a bad assumption.  I've found
at least one system in the wild where the vendor using the _OSI
string doesn't match the _OSI string and the neither does the use.

So this brings a good time to review keeping those strings in the kernel.
There are 3 strings that were introduced:

Linux-Dell-Video
-> Intended for systems with NVIDIA cards that didn't support RTD3
Linux-Lenovo-NV-HDMI-Audio
-> Intended for powering on NVIDIA HDMI device
Linux-HPI-Hybrid-Graphics
-> Intended for changing dGPU output

AFAIK the first string is no longer relevant as nouveau now supports
RTD3.  If that's wrong, this can be changed for the series.

The second two strings appear to be non-scalable workarounds.  For
accomplishing these tasks, registers can be written from kernel drivers
or custom ASL can be put behind a _DSM.  By forcing either of these two
solutions it will better let the Linux kernel control the behavior.

Based on the above this series drops the first string and marks the second
two strings to only apply to older systems.

Link: https://lore.kernel.org/all/54add026bb6f45fd94a2dc2bae4adf9f@AUSX13MPC101.AMER.DELL.COM/T/

Mario Limonciello (2):
  ACPI: OSI: Remove Linux-Dell-Video _OSI string
  ACPI: OSI: Deprecate some abused _OSI strings

 Documentation/firmware-guide/acpi/osi.rst | 24 ++++++++----------
 drivers/acpi/osi.c                        | 31 ++++++++++++-----------
 2 files changed, 26 insertions(+), 29 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [RFC 1/2] ACPI: OSI: Remove Linux-Dell-Video _OSI string
  2022-08-19 14:25 [RFC 0/2] Stop the abuse of Linux-* _OSI strings Mario Limonciello
@ 2022-08-19 14:25 ` Mario Limonciello
  2022-08-19 14:25 ` [RFC 2/2] ACPI: OSI: Deprecate some abused _OSI strings Mario Limonciello
  2022-08-19 15:44 ` [RFC 0/2] Stop the abuse of Linux-* " Karol Herbst
  2 siblings, 0 replies; 12+ messages in thread
From: Mario Limonciello @ 2022-08-19 14:25 UTC (permalink / raw)
  To: linux-kernel, linux-acpi, rafael, Len Brown
  Cc: kherbst, nouveau, hdegoede, ddadap, kai.heng.feng,
	Dell.Client.Kernel, Mario Limonciello

This string was introduced because drivers for NVIDIA hardware
didn't support RTD3 in the past.  This is no longer the case, and
so vendors shouldn't be using this string to modify ASL anymore.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/acpi/osi.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/acpi/osi.c b/drivers/acpi/osi.c
index 9f6853809138..c2f6b2f553d9 100644
--- a/drivers/acpi/osi.c
+++ b/drivers/acpi/osi.c
@@ -44,15 +44,6 @@ osi_setup_entries[OSI_STRING_ENTRIES_MAX] __initdata = {
 	{"Processor Device", true},
 	{"3.0 _SCP Extensions", true},
 	{"Processor Aggregator Device", true},
-	/*
-	 * Linux-Dell-Video is used by BIOS to disable RTD3 for NVidia graphics
-	 * cards as RTD3 is not supported by drivers now.  Systems with NVidia
-	 * cards will hang without RTD3 disabled.
-	 *
-	 * Once NVidia drivers officially support RTD3, this _OSI strings can
-	 * be removed if both new and old graphics cards are supported.
-	 */
-	{"Linux-Dell-Video", true},
 	/*
 	 * Linux-Lenovo-NV-HDMI-Audio is used by BIOS to power on NVidia's HDMI
 	 * audio device which is turned off for power-saving in Windows OS.
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [RFC 2/2] ACPI: OSI: Deprecate some abused _OSI strings
  2022-08-19 14:25 [RFC 0/2] Stop the abuse of Linux-* _OSI strings Mario Limonciello
  2022-08-19 14:25 ` [RFC 1/2] ACPI: OSI: Remove Linux-Dell-Video _OSI string Mario Limonciello
@ 2022-08-19 14:25 ` Mario Limonciello
  2022-08-19 22:02   ` Daniel Dadap
  2022-08-19 15:44 ` [RFC 0/2] Stop the abuse of Linux-* " Karol Herbst
  2 siblings, 1 reply; 12+ messages in thread
From: Mario Limonciello @ 2022-08-19 14:25 UTC (permalink / raw)
  To: linux-kernel, linux-acpi, rafael, Len Brown
  Cc: kherbst, nouveau, hdegoede, ddadap, kai.heng.feng,
	Dell.Client.Kernel, Mario Limonciello

The Linux-Lenovo-NV-HDMI-Audio and Linux-HPI-Hybrid-Graphics have
been seen in the wild being abused by other vendors.  If these use
cases are still needed for modern laptops, they should be done via
kernel drivers instead.

As we can't have nice things, mark these strings to only be applied
to laptops from 2022 or earlier.  This should avoid breaking any
older laptops.  In the future if the kernel drivers need to call
Linux-only ASL for any reason, it could be a custom _DSM used only
for Linux or something similar. This approach allows kernel developers
to control whether to stop calling the ASL when the deficiency by
the kernel is resolved.

Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 Documentation/firmware-guide/acpi/osi.rst | 24 ++++++++++-------------
 drivers/acpi/osi.c                        | 22 +++++++++++++++------
 2 files changed, 26 insertions(+), 20 deletions(-)

diff --git a/Documentation/firmware-guide/acpi/osi.rst b/Documentation/firmware-guide/acpi/osi.rst
index 05869c0045d7..392b982741fe 100644
--- a/Documentation/firmware-guide/acpi/osi.rst
+++ b/Documentation/firmware-guide/acpi/osi.rst
@@ -41,26 +41,22 @@ But it is likely that they will all eventually be added.
 What should an OEM do if they want to support Linux and Windows
 using the same BIOS image?  Often they need to do something different
 for Linux to deal with how Linux is different from Windows.
-Here the BIOS should ask exactly what it wants to know:
 
+In this case, the OEM should create custom ASL to be executed by the
+Linux kernel and changes to Linux kernel drivers to execute this custom
+ASL.  The easiest way to accomplish this is to introduce a device specific
+method (_DSM) that is called from the Linux kernel.
+
+In the past the kernel used to support something like:
 _OSI("Linux-OEM-my_interface_name")
 where 'OEM' is needed if this is an OEM-specific hook,
 and 'my_interface_name' describes the hook, which could be a
 quirk, a bug, or a bug-fix.
 
-In addition, the OEM should send a patch to upstream Linux
-via the linux-acpi@vger.kernel.org mailing list.  When that patch
-is checked into Linux, the OS will answer "YES" when the BIOS
-on the OEM's system uses _OSI to ask if the interface is supported
-by the OS.  Linux distributors can back-port that patch for Linux
-pre-installs, and it will be included by all distributions that
-re-base to upstream.  If the distribution can not update the kernel binary,
-they can also add an acpi_osi=Linux-OEM-my_interface_name
-cmdline parameter to the boot loader, as needed.
-
-If the string refers to a feature where the upstream kernel
-eventually grows support, a patch should be sent to remove
-the string when that support is added to the kernel.
+However this was discovered to be abused by other BIOS vendors to change
+completely unrelated code on completely unrelated systems.  As such it's
+been deprecated and any old hooks will not be activated on systems from
+2023 or later.
 
 That was easy.  Read on, to find out how to do it wrong.
 
diff --git a/drivers/acpi/osi.c b/drivers/acpi/osi.c
index c2f6b2f553d9..18c339c3f277 100644
--- a/drivers/acpi/osi.c
+++ b/drivers/acpi/osi.c
@@ -25,6 +25,7 @@
 struct acpi_osi_entry {
 	char string[OSI_STRING_LENGTH_MAX];
 	bool enable;
+	unsigned int max_bios_year;
 };
 
 static struct acpi_osi_config {
@@ -40,25 +41,29 @@ static struct acpi_osi_config {
 static struct acpi_osi_config osi_config;
 static struct acpi_osi_entry
 osi_setup_entries[OSI_STRING_ENTRIES_MAX] __initdata = {
-	{"Module Device", true},
-	{"Processor Device", true},
-	{"3.0 _SCP Extensions", true},
-	{"Processor Aggregator Device", true},
+	{"Module Device", true, 0},
+	{"Processor Device", true, 0},
+	{"3.0 _SCP Extensions", true, 0},
+	{"Processor Aggregator Device", true, 0},
 	/*
 	 * Linux-Lenovo-NV-HDMI-Audio is used by BIOS to power on NVidia's HDMI
 	 * audio device which is turned off for power-saving in Windows OS.
 	 * This power management feature observed on some Lenovo Thinkpad
 	 * systems which will not be able to output audio via HDMI without
 	 * a BIOS workaround.
+	 *
+	 * This _OSI string is only applied to systems from 2022 or earlier.
 	 */
-	{"Linux-Lenovo-NV-HDMI-Audio", true},
+	{"Linux-Lenovo-NV-HDMI-Audio", true, 2022},
 	/*
 	 * Linux-HPI-Hybrid-Graphics is used by BIOS to enable dGPU to
 	 * output video directly to external monitors on HP Inc. mobile
 	 * workstations as Nvidia and AMD VGA drivers provide limited
 	 * hybrid graphics supports.
+	 *
+	 * This _OSI string is only applied to systems from 2022 or earlier.
 	 */
-	{"Linux-HPI-Hybrid-Graphics", true},
+	{"Linux-HPI-Hybrid-Graphics", true, 2022},
 };
 
 static u32 acpi_osi_handler(acpi_string interface, u32 supported)
@@ -122,9 +127,11 @@ void __init acpi_osi_setup(char *str)
 		osi = &osi_setup_entries[i];
 		if (!strcmp(osi->string, str)) {
 			osi->enable = enable;
+			osi->max_bios_year = 0;
 			break;
 		} else if (osi->string[0] == '\0') {
 			osi->enable = enable;
+			osi->max_bios_year = 0;
 			strncpy(osi->string, str, OSI_STRING_LENGTH_MAX);
 			break;
 		}
@@ -225,6 +232,9 @@ static void __init acpi_osi_setup_late(void)
 		str = osi->string;
 		if (*str == '\0')
 			break;
+		if (osi->max_bios_year &&
+		    dmi_get_bios_year() > osi->max_bios_year)
+			continue;
 		if (osi->enable) {
 			status = acpi_install_interface(str);
 			if (ACPI_SUCCESS(status))
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [RFC 0/2] Stop the abuse of Linux-* _OSI strings
  2022-08-19 14:25 [RFC 0/2] Stop the abuse of Linux-* _OSI strings Mario Limonciello
  2022-08-19 14:25 ` [RFC 1/2] ACPI: OSI: Remove Linux-Dell-Video _OSI string Mario Limonciello
  2022-08-19 14:25 ` [RFC 2/2] ACPI: OSI: Deprecate some abused _OSI strings Mario Limonciello
@ 2022-08-19 15:44 ` Karol Herbst
  2022-08-19 16:00   ` Limonciello, Mario
  2022-08-22 21:18   ` Lyude Paul
  2 siblings, 2 replies; 12+ messages in thread
From: Karol Herbst @ 2022-08-19 15:44 UTC (permalink / raw)
  To: Mario Limonciello, Lyude Paul
  Cc: linux-kernel, linux-acpi, rafael, Len Brown, nouveau, hdegoede,
	ddadap, kai.heng.feng, Dell.Client.Kernel

On Fri, Aug 19, 2022 at 4:25 PM Mario Limonciello
<mario.limonciello@amd.com> wrote:
>
> 3 _OSI strings were introduced in recent years that were intended
> to workaround very specific problems found on specific systems.
>
> The idea was supposed to be that these quirks were only used on
> those systems, but this proved to be a bad assumption.  I've found
> at least one system in the wild where the vendor using the _OSI
> string doesn't match the _OSI string and the neither does the use.
>
> So this brings a good time to review keeping those strings in the kernel.
> There are 3 strings that were introduced:
>
> Linux-Dell-Video
> -> Intended for systems with NVIDIA cards that didn't support RTD3
> Linux-Lenovo-NV-HDMI-Audio
> -> Intended for powering on NVIDIA HDMI device
> Linux-HPI-Hybrid-Graphics
> -> Intended for changing dGPU output
>
> AFAIK the first string is no longer relevant as nouveau now supports
> RTD3.  If that's wrong, this can be changed for the series.
>

Nouveau always supported RTD3, because that's mainly a kernel feature.
When those were introduced we simply had a bug only hit on a few
systems. And instead of helping us to debug this, this workaround was
added :( We were not even asked about this.

I am a bit curious about the other two though as I am not even sure
they are needed at all as we put other work arounds in place. @Lyude
Paul might know more about these.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC 0/2] Stop the abuse of Linux-* _OSI strings
  2022-08-19 15:44 ` [RFC 0/2] Stop the abuse of Linux-* " Karol Herbst
@ 2022-08-19 16:00   ` Limonciello, Mario
  2022-08-19 16:37     ` Karol Herbst
  2022-08-22 21:18   ` Lyude Paul
  1 sibling, 1 reply; 12+ messages in thread
From: Limonciello, Mario @ 2022-08-19 16:00 UTC (permalink / raw)
  To: Karol Herbst, Lyude Paul
  Cc: linux-kernel, linux-acpi, rafael, Len Brown, nouveau, hdegoede,
	ddadap, kai.heng.feng, Dell.Client.Kernel

On 8/19/2022 10:44, Karol Herbst wrote:
> On Fri, Aug 19, 2022 at 4:25 PM Mario Limonciello
> <mario.limonciello@amd.com> wrote:
>>
>> 3 _OSI strings were introduced in recent years that were intended
>> to workaround very specific problems found on specific systems.
>>
>> The idea was supposed to be that these quirks were only used on
>> those systems, but this proved to be a bad assumption.  I've found
>> at least one system in the wild where the vendor using the _OSI
>> string doesn't match the _OSI string and the neither does the use.
>>
>> So this brings a good time to review keeping those strings in the kernel.
>> There are 3 strings that were introduced:
>>
>> Linux-Dell-Video
>> -> Intended for systems with NVIDIA cards that didn't support RTD3
>> Linux-Lenovo-NV-HDMI-Audio
>> -> Intended for powering on NVIDIA HDMI device
>> Linux-HPI-Hybrid-Graphics
>> -> Intended for changing dGPU output
>>
>> AFAIK the first string is no longer relevant as nouveau now supports
>> RTD3.  If that's wrong, this can be changed for the series.
>>
> 
> Nouveau always supported RTD3, because that's mainly a kernel feature.
> When those were introduced we simply had a bug only hit on a few
> systems. And instead of helping us to debug this, this workaround was
> added :( We were not even asked about this.

My apologies, I was certainly part of the impetus for this W/A in the 
first place while I was at my previous employer.  Your comment 
re-affirms to me that at least the first patch is correct.

> 
> I am a bit curious about the other two though as I am not even sure
> they are needed at all as we put other work arounds in place. @Lyude
> Paul might know more about these.
> 

If the other two really aren't needed anymore, then yeah we should just 
tear all 3 out.  If that's the direction we go, I would appreciate some 
commit IDs to reference in the commit message for tearing them out so 
that if they end up backporting to stable we know how far they should go.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC 0/2] Stop the abuse of Linux-* _OSI strings
  2022-08-19 16:00   ` Limonciello, Mario
@ 2022-08-19 16:37     ` Karol Herbst
  2022-08-19 16:43       ` Limonciello, Mario
  0 siblings, 1 reply; 12+ messages in thread
From: Karol Herbst @ 2022-08-19 16:37 UTC (permalink / raw)
  To: Limonciello, Mario
  Cc: Lyude Paul, linux-kernel, linux-acpi, rafael, Len Brown, nouveau,
	hdegoede, ddadap, kai.heng.feng, Dell.Client.Kernel

On Fri, Aug 19, 2022 at 6:00 PM Limonciello, Mario
<mario.limonciello@amd.com> wrote:
>
> On 8/19/2022 10:44, Karol Herbst wrote:
> > On Fri, Aug 19, 2022 at 4:25 PM Mario Limonciello
> > <mario.limonciello@amd.com> wrote:
> >>
> >> 3 _OSI strings were introduced in recent years that were intended
> >> to workaround very specific problems found on specific systems.
> >>
> >> The idea was supposed to be that these quirks were only used on
> >> those systems, but this proved to be a bad assumption.  I've found
> >> at least one system in the wild where the vendor using the _OSI
> >> string doesn't match the _OSI string and the neither does the use.
> >>
> >> So this brings a good time to review keeping those strings in the kernel.
> >> There are 3 strings that were introduced:
> >>
> >> Linux-Dell-Video
> >> -> Intended for systems with NVIDIA cards that didn't support RTD3
> >> Linux-Lenovo-NV-HDMI-Audio
> >> -> Intended for powering on NVIDIA HDMI device
> >> Linux-HPI-Hybrid-Graphics
> >> -> Intended for changing dGPU output
> >>
> >> AFAIK the first string is no longer relevant as nouveau now supports
> >> RTD3.  If that's wrong, this can be changed for the series.
> >>
> >
> > Nouveau always supported RTD3, because that's mainly a kernel feature.
> > When those were introduced we simply had a bug only hit on a few
> > systems. And instead of helping us to debug this, this workaround was
> > added :( We were not even asked about this.
>
> My apologies, I was certainly part of the impetus for this W/A in the
> first place while I was at my previous employer.  Your comment
> re-affirms to me that at least the first patch is correct.
>

Yeah, no worries. I just hope that people in the future will
communicate such things.

Anyway, there are a few issues with the runpm stuff left, and looking
at what nvidia does in their open driver makes me wonder if we might
need a bigger overhaul of runpm. They do apply bridge/host controller
specific workarounds and I suspect some of them are related here as
the workaround I came up with in nouveau can be seen in 434fdb51513bf.

But also having access to documentation/specification from what Nvidia
is doing would be quite helpful. We know that on some really new AMD
systems we run into new issues and this needs some investigation. I
simply don't access to any laptops where this problem can be seen.

> >
> > I am a bit curious about the other two though as I am not even sure
> > they are needed at all as we put other work arounds in place. @Lyude
> > Paul might know more about these.
> >
>
> If the other two really aren't needed anymore, then yeah we should just
> tear all 3 out.  If that's the direction we go, I would appreciate some
> commit IDs to reference in the commit message for tearing them out so
> that if they end up backporting to stable we know how far they should go.
>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC 0/2] Stop the abuse of Linux-* _OSI strings
  2022-08-19 16:37     ` Karol Herbst
@ 2022-08-19 16:43       ` Limonciello, Mario
  2022-08-19 16:47         ` Karol Herbst
  0 siblings, 1 reply; 12+ messages in thread
From: Limonciello, Mario @ 2022-08-19 16:43 UTC (permalink / raw)
  To: Karol Herbst
  Cc: Lyude Paul, linux-kernel, linux-acpi, rafael, Len Brown, nouveau,
	hdegoede, ddadap, kai.heng.feng, Dell.Client.Kernel

On 8/19/2022 11:37, Karol Herbst wrote:
> On Fri, Aug 19, 2022 at 6:00 PM Limonciello, Mario
> <mario.limonciello@amd.com> wrote:
>>
>> On 8/19/2022 10:44, Karol Herbst wrote:
>>> On Fri, Aug 19, 2022 at 4:25 PM Mario Limonciello
>>> <mario.limonciello@amd.com> wrote:
>>>>
>>>> 3 _OSI strings were introduced in recent years that were intended
>>>> to workaround very specific problems found on specific systems.
>>>>
>>>> The idea was supposed to be that these quirks were only used on
>>>> those systems, but this proved to be a bad assumption.  I've found
>>>> at least one system in the wild where the vendor using the _OSI
>>>> string doesn't match the _OSI string and the neither does the use.
>>>>
>>>> So this brings a good time to review keeping those strings in the kernel.
>>>> There are 3 strings that were introduced:
>>>>
>>>> Linux-Dell-Video
>>>> -> Intended for systems with NVIDIA cards that didn't support RTD3
>>>> Linux-Lenovo-NV-HDMI-Audio
>>>> -> Intended for powering on NVIDIA HDMI device
>>>> Linux-HPI-Hybrid-Graphics
>>>> -> Intended for changing dGPU output
>>>>
>>>> AFAIK the first string is no longer relevant as nouveau now supports
>>>> RTD3.  If that's wrong, this can be changed for the series.
>>>>
>>>
>>> Nouveau always supported RTD3, because that's mainly a kernel feature.
>>> When those were introduced we simply had a bug only hit on a few
>>> systems. And instead of helping us to debug this, this workaround was
>>> added :( We were not even asked about this.
>>
>> My apologies, I was certainly part of the impetus for this W/A in the
>> first place while I was at my previous employer.  Your comment
>> re-affirms to me that at least the first patch is correct.
>>
> 
> Yeah, no worries. I just hope that people in the future will
> communicate such things.
> 
> Anyway, there are a few issues with the runpm stuff left, and looking
> at what nvidia does in their open driver makes me wonder if we might
> need a bigger overhaul of runpm. They do apply bridge/host controller
> specific workarounds and I suspect some of them are related here as
> the workaround I came up with in nouveau can be seen in 434fdb51513bf.

But this overhaul shouldn't gate removing this _OSI string, or you think 
it should?

> 
> But also having access to documentation/specification from what Nvidia
> is doing would be quite helpful. We know that on some really new AMD
> systems we run into new issues and this needs some investigation. I
> simply don't access to any laptops where this problem can be seen.
> 

Do you mean there are specifically remaining issues on AMD APU + NVIDIA 
dGPU systems?  Any public bugs by chance?

Depending on what these are I'm happy to try to help with at least 
access.  If we have them maybe we can try to make the right connections 
to get some hardware to you, or at least remotely access it.

>>>
>>> I am a bit curious about the other two though as I am not even sure
>>> they are needed at all as we put other work arounds in place. @Lyude
>>> Paul might know more about these.
>>>
>>
>> If the other two really aren't needed anymore, then yeah we should just
>> tear all 3 out.  If that's the direction we go, I would appreciate some
>> commit IDs to reference in the commit message for tearing them out so
>> that if they end up backporting to stable we know how far they should go.
>>
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC 0/2] Stop the abuse of Linux-* _OSI strings
  2022-08-19 16:43       ` Limonciello, Mario
@ 2022-08-19 16:47         ` Karol Herbst
  0 siblings, 0 replies; 12+ messages in thread
From: Karol Herbst @ 2022-08-19 16:47 UTC (permalink / raw)
  To: Limonciello, Mario
  Cc: Lyude Paul, linux-kernel, linux-acpi, rafael, Len Brown, nouveau,
	hdegoede, ddadap, kai.heng.feng, Dell.Client.Kernel

On Fri, Aug 19, 2022 at 6:43 PM Limonciello, Mario
<mario.limonciello@amd.com> wrote:
>
> On 8/19/2022 11:37, Karol Herbst wrote:
> > On Fri, Aug 19, 2022 at 6:00 PM Limonciello, Mario
> > <mario.limonciello@amd.com> wrote:
> >>
> >> On 8/19/2022 10:44, Karol Herbst wrote:
> >>> On Fri, Aug 19, 2022 at 4:25 PM Mario Limonciello
> >>> <mario.limonciello@amd.com> wrote:
> >>>>
> >>>> 3 _OSI strings were introduced in recent years that were intended
> >>>> to workaround very specific problems found on specific systems.
> >>>>
> >>>> The idea was supposed to be that these quirks were only used on
> >>>> those systems, but this proved to be a bad assumption.  I've found
> >>>> at least one system in the wild where the vendor using the _OSI
> >>>> string doesn't match the _OSI string and the neither does the use.
> >>>>
> >>>> So this brings a good time to review keeping those strings in the kernel.
> >>>> There are 3 strings that were introduced:
> >>>>
> >>>> Linux-Dell-Video
> >>>> -> Intended for systems with NVIDIA cards that didn't support RTD3
> >>>> Linux-Lenovo-NV-HDMI-Audio
> >>>> -> Intended for powering on NVIDIA HDMI device
> >>>> Linux-HPI-Hybrid-Graphics
> >>>> -> Intended for changing dGPU output
> >>>>
> >>>> AFAIK the first string is no longer relevant as nouveau now supports
> >>>> RTD3.  If that's wrong, this can be changed for the series.
> >>>>
> >>>
> >>> Nouveau always supported RTD3, because that's mainly a kernel feature.
> >>> When those were introduced we simply had a bug only hit on a few
> >>> systems. And instead of helping us to debug this, this workaround was
> >>> added :( We were not even asked about this.
> >>
> >> My apologies, I was certainly part of the impetus for this W/A in the
> >> first place while I was at my previous employer.  Your comment
> >> re-affirms to me that at least the first patch is correct.
> >>
> >
> > Yeah, no worries. I just hope that people in the future will
> > communicate such things.
> >
> > Anyway, there are a few issues with the runpm stuff left, and looking
> > at what nvidia does in their open driver makes me wonder if we might
> > need a bigger overhaul of runpm. They do apply bridge/host controller
> > specific workarounds and I suspect some of them are related here as
> > the workaround I came up with in nouveau can be seen in 434fdb51513bf.
>
> But this overhaul shouldn't gate removing this _OSI string, or you think
> it should?
>

Hard to tell. If there are affected systems but have those _OSI
strings in place so it's hidden, this would be annoying, but then we
might have more pointers on what's actually broken. Anyway, we don't
need those workarounds and rather a real fix for all those issues. And
I suspect the real fix is to apply specific workarounds for specific
systems.

> >
> > But also having access to documentation/specification from what Nvidia
> > is doing would be quite helpful. We know that on some really new AMD
> > systems we run into new issues and this needs some investigation. I
> > simply don't access to any laptops where this problem can be seen.
> >
>
> Do you mean there are specifically remaining issues on AMD APU + NVIDIA
> dGPU systems?  Any public bugs by chance?
>
> Depending on what these are I'm happy to try to help with at least
> access.  If we have them maybe we can try to make the right connections
> to get some hardware to you, or at least remotely access it.
>

https://gitlab.freedesktop.org/drm/nouveau/-/issues/108

there might be more though, but this should be a good start.

> >>>
> >>> I am a bit curious about the other two though as I am not even sure
> >>> they are needed at all as we put other work arounds in place. @Lyude
> >>> Paul might know more about these.
> >>>
> >>
> >> If the other two really aren't needed anymore, then yeah we should just
> >> tear all 3 out.  If that's the direction we go, I would appreciate some
> >> commit IDs to reference in the commit message for tearing them out so
> >> that if they end up backporting to stable we know how far they should go.
> >>
> >
>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC 2/2] ACPI: OSI: Deprecate some abused _OSI strings
  2022-08-19 14:25 ` [RFC 2/2] ACPI: OSI: Deprecate some abused _OSI strings Mario Limonciello
@ 2022-08-19 22:02   ` Daniel Dadap
  0 siblings, 0 replies; 12+ messages in thread
From: Daniel Dadap @ 2022-08-19 22:02 UTC (permalink / raw)
  To: Mario Limonciello, linux-kernel, linux-acpi, rafael, Len Brown
  Cc: kherbst, nouveau, hdegoede, kai.heng.feng, Dell.Client.Kernel


On 8/19/22 9:25 AM, Mario Limonciello wrote:
> The Linux-Lenovo-NV-HDMI-Audio and Linux-HPI-Hybrid-Graphics have
> been seen in the wild being abused by other vendors.  If these use
> cases are still needed for modern laptops, they should be done via
> kernel drivers instead.
>
> As we can't have nice things, mark these strings to only be applied
> to laptops from 2022 or earlier.  This should avoid breaking any
> older laptops.  In the future if the kernel drivers need to call
> Linux-only ASL for any reason, it could be a custom _DSM used only
> for Linux or something similar. This approach allows kernel developers
> to control whether to stop calling the ASL when the deficiency by
> the kernel is resolved.
>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>   Documentation/firmware-guide/acpi/osi.rst | 24 ++++++++++-------------
>   drivers/acpi/osi.c                        | 22 +++++++++++++++------
>   2 files changed, 26 insertions(+), 20 deletions(-)
>
> diff --git a/Documentation/firmware-guide/acpi/osi.rst b/Documentation/firmware-guide/acpi/osi.rst
> index 05869c0045d7..392b982741fe 100644
> --- a/Documentation/firmware-guide/acpi/osi.rst
> +++ b/Documentation/firmware-guide/acpi/osi.rst
> @@ -41,26 +41,22 @@ But it is likely that they will all eventually be added.
>   What should an OEM do if they want to support Linux and Windows
>   using the same BIOS image?  Often they need to do something different
>   for Linux to deal with how Linux is different from Windows.
> -Here the BIOS should ask exactly what it wants to know:
>   
> +In this case, the OEM should create custom ASL to be executed by the
> +Linux kernel and changes to Linux kernel drivers to execute this custom
> +ASL.  The easiest way to accomplish this is to introduce a device specific
> +method (_DSM) that is called from the Linux kernel.
> +
> +In the past the kernel used to support something like:
>   _OSI("Linux-OEM-my_interface_name")
>   where 'OEM' is needed if this is an OEM-specific hook,
>   and 'my_interface_name' describes the hook, which could be a
>   quirk, a bug, or a bug-fix.
>   
> -In addition, the OEM should send a patch to upstream Linux
> -via the linux-acpi@vger.kernel.org mailing list.  When that patch
> -is checked into Linux, the OS will answer "YES" when the BIOS
> -on the OEM's system uses _OSI to ask if the interface is supported
> -by the OS.  Linux distributors can back-port that patch for Linux
> -pre-installs, and it will be included by all distributions that
> -re-base to upstream.  If the distribution can not update the kernel binary,
> -they can also add an acpi_osi=Linux-OEM-my_interface_name
> -cmdline parameter to the boot loader, as needed.
> -
> -If the string refers to a feature where the upstream kernel
> -eventually grows support, a patch should be sent to remove
> -the string when that support is added to the kernel.
> +However this was discovered to be abused by other BIOS vendors to change
> +completely unrelated code on completely unrelated systems.  As such it's
> +been deprecated and any old hooks will not be activated on systems from
> +2023 or later.
>   
>   That was easy.  Read on, to find out how to do it wrong.
>   
> diff --git a/drivers/acpi/osi.c b/drivers/acpi/osi.c
> index c2f6b2f553d9..18c339c3f277 100644
> --- a/drivers/acpi/osi.c
> +++ b/drivers/acpi/osi.c
> @@ -25,6 +25,7 @@
>   struct acpi_osi_entry {
>   	char string[OSI_STRING_LENGTH_MAX];
>   	bool enable;
> +	unsigned int max_bios_year;
>   };
>   
>   static struct acpi_osi_config {
> @@ -40,25 +41,29 @@ static struct acpi_osi_config {
>   static struct acpi_osi_config osi_config;
>   static struct acpi_osi_entry
>   osi_setup_entries[OSI_STRING_ENTRIES_MAX] __initdata = {
> -	{"Module Device", true},
> -	{"Processor Device", true},
> -	{"3.0 _SCP Extensions", true},
> -	{"Processor Aggregator Device", true},
> +	{"Module Device", true, 0},
> +	{"Processor Device", true, 0},
> +	{"3.0 _SCP Extensions", true, 0},
> +	{"Processor Aggregator Device", true, 0},


Since you're going to have to update this whole table anyway, maybe it 
would be more self-documenting if you used named initializers? e.g.:

{ .string = "Module Device", .enable = true, .max_bios_year = 0 },

For that matter, do we really need to explicitly set max_bios_year in 
entries that don't use it? They're all zero-initialized anyway due to 
the static declaration.


>   	/*
>   	 * Linux-Lenovo-NV-HDMI-Audio is used by BIOS to power on NVidia's HDMI
>   	 * audio device which is turned off for power-saving in Windows OS.
>   	 * This power management feature observed on some Lenovo Thinkpad
>   	 * systems which will not be able to output audio via HDMI without
>   	 * a BIOS workaround.
> +	 *
> +	 * This _OSI string is only applied to systems from 2022 or earlier.
>   	 */
> -	{"Linux-Lenovo-NV-HDMI-Audio", true},
> +	{"Linux-Lenovo-NV-HDMI-Audio", true, 2022},


Oof. I remember this one. Setting it to this year seems a bit generous, 
as IIUC systems haven't shipped with this feature in a few years, so 
there shouldn't be anything legitimately needing this key in recent 
years, but if it's been found being abused in the wild as you say, I 
suppose it's safer to not break things, even if they're being naughty.


>   	/*
>   	 * Linux-HPI-Hybrid-Graphics is used by BIOS to enable dGPU to
>   	 * output video directly to external monitors on HP Inc. mobile
>   	 * workstations as Nvidia and AMD VGA drivers provide limited
>   	 * hybrid graphics supports.
> +	 *
> +	 * This _OSI string is only applied to systems from 2022 or earlier.
>   	 */
> -	{"Linux-HPI-Hybrid-Graphics", true},
> +	{"Linux-HPI-Hybrid-Graphics", true, 2022},
>   };
>   
>   static u32 acpi_osi_handler(acpi_string interface, u32 supported)
> @@ -122,9 +127,11 @@ void __init acpi_osi_setup(char *str)
>   		osi = &osi_setup_entries[i];
>   		if (!strcmp(osi->string, str)) {
>   			osi->enable = enable;
> +			osi->max_bios_year = 0;
>   			break;
>   		} else if (osi->string[0] == '\0') {
>   			osi->enable = enable;
> +			osi->max_bios_year = 0;
>   			strncpy(osi->string, str, OSI_STRING_LENGTH_MAX);
>   			break;
>   		}
> @@ -225,6 +232,9 @@ static void __init acpi_osi_setup_late(void)
>   		str = osi->string;
>   		if (*str == '\0')
>   			break;
> +		if (osi->max_bios_year &&
> +		    dmi_get_bios_year() > osi->max_bios_year)
> +			continue;
>   		if (osi->enable) {
>   			status = acpi_install_interface(str);
>   			if (ACPI_SUCCESS(status))

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC 0/2] Stop the abuse of Linux-* _OSI strings
  2022-08-19 15:44 ` [RFC 0/2] Stop the abuse of Linux-* " Karol Herbst
  2022-08-19 16:00   ` Limonciello, Mario
@ 2022-08-22 21:18   ` Lyude Paul
  2022-08-23  3:47     ` Kai-Heng Feng
  1 sibling, 1 reply; 12+ messages in thread
From: Lyude Paul @ 2022-08-22 21:18 UTC (permalink / raw)
  To: Karol Herbst, Mario Limonciello
  Cc: linux-kernel, linux-acpi, rafael, Len Brown, nouveau, hdegoede,
	ddadap, kai.heng.feng, Dell.Client.Kernel

On Fri, 2022-08-19 at 17:44 +0200, Karol Herbst wrote:
> On Fri, Aug 19, 2022 at 4:25 PM Mario Limonciello
> <mario.limonciello@amd.com> wrote:
> > 
> > 3 _OSI strings were introduced in recent years that were intended
> > to workaround very specific problems found on specific systems.
> > 
> > The idea was supposed to be that these quirks were only used on
> > those systems, but this proved to be a bad assumption.  I've found
> > at least one system in the wild where the vendor using the _OSI
> > string doesn't match the _OSI string and the neither does the use.
> > 
> > So this brings a good time to review keeping those strings in the kernel.
> > There are 3 strings that were introduced:
> > 
> > Linux-Dell-Video
> > -> Intended for systems with NVIDIA cards that didn't support RTD3
> > Linux-Lenovo-NV-HDMI-Audio
> > -> Intended for powering on NVIDIA HDMI device
> > Linux-HPI-Hybrid-Graphics
> > -> Intended for changing dGPU output
> > 
> > AFAIK the first string is no longer relevant as nouveau now supports
> > RTD3.  If that's wrong, this can be changed for the series.
> > 
> 
> Nouveau always supported RTD3, because that's mainly a kernel feature.
> When those were introduced we simply had a bug only hit on a few
> systems. And instead of helping us to debug this, this workaround was
> added :( We were not even asked about this.
> 
> I am a bit curious about the other two though as I am not even sure
> they are needed at all as we put other work arounds in place. @Lyude
> Paul might know more about these.

Some of the _OSI strings are totally fine. From my recollection:

[    0.242993] ACPI: Added _OSI(Linux-Lenovo-NV-HDMI-Audio)
^ this one is needed to do a couple of ACPI tricks at startup to get the PCIe
device for audio on nvidia's GPU to be detected properly

[    0.242993] ACPI: Added _OSI(Linux-HPI-Hybrid-Graphics)

I don't actually know how necessary this is, but I'm hesistant to call this
one bad as it may be related to the funny mux configurations that I'm learning
may exist on HP machines.

> 

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC 0/2] Stop the abuse of Linux-* _OSI strings
  2022-08-22 21:18   ` Lyude Paul
@ 2022-08-23  3:47     ` Kai-Heng Feng
  2022-08-23 17:05       ` Lyude Paul
  0 siblings, 1 reply; 12+ messages in thread
From: Kai-Heng Feng @ 2022-08-23  3:47 UTC (permalink / raw)
  To: Lyude Paul
  Cc: Karol Herbst, Mario Limonciello, linux-kernel, linux-acpi,
	rafael, Len Brown, nouveau, hdegoede, ddadap, Dell.Client.Kernel,
	Aaron Ma

[+Cc Aaron]

On Tue, Aug 23, 2022 at 5:18 AM Lyude Paul <lyude@redhat.com> wrote:
>
> On Fri, 2022-08-19 at 17:44 +0200, Karol Herbst wrote:
> > On Fri, Aug 19, 2022 at 4:25 PM Mario Limonciello
> > <mario.limonciello@amd.com> wrote:
> > >
> > > 3 _OSI strings were introduced in recent years that were intended
> > > to workaround very specific problems found on specific systems.
> > >
> > > The idea was supposed to be that these quirks were only used on
> > > those systems, but this proved to be a bad assumption.  I've found
> > > at least one system in the wild where the vendor using the _OSI
> > > string doesn't match the _OSI string and the neither does the use.
> > >
> > > So this brings a good time to review keeping those strings in the kernel.
> > > There are 3 strings that were introduced:
> > >
> > > Linux-Dell-Video
> > > -> Intended for systems with NVIDIA cards that didn't support RTD3
> > > Linux-Lenovo-NV-HDMI-Audio
> > > -> Intended for powering on NVIDIA HDMI device
> > > Linux-HPI-Hybrid-Graphics
> > > -> Intended for changing dGPU output
> > >
> > > AFAIK the first string is no longer relevant as nouveau now supports
> > > RTD3.  If that's wrong, this can be changed for the series.
> > >
> >
> > Nouveau always supported RTD3, because that's mainly a kernel feature.
> > When those were introduced we simply had a bug only hit on a few
> > systems. And instead of helping us to debug this, this workaround was
> > added :( We were not even asked about this.
> >
> > I am a bit curious about the other two though as I am not even sure
> > they are needed at all as we put other work arounds in place. @Lyude
> > Paul might know more about these.
>
> Some of the _OSI strings are totally fine. From my recollection:
>
> [    0.242993] ACPI: Added _OSI(Linux-Lenovo-NV-HDMI-Audio)
> ^ this one is needed to do a couple of ACPI tricks at startup to get the PCIe
> device for audio on nvidia's GPU to be detected properly

This should be fixed by commit b516ea586d71 ("PCI: Enable NVIDIA HDA
controllers").
Aaron worked on more Lenovo systems than me, so he may be more sure of it.

>
> [    0.242993] ACPI: Added _OSI(Linux-HPI-Hybrid-Graphics)
>
> I don't actually know how necessary this is, but I'm hesistant to call this
> one bad as it may be related to the funny mux configurations that I'm learning
> may exist on HP machines.

Should be fixed by commit 8e55f99c510f ("drm/i915: Invoke another _DSM
to enable MUX on HP Workstation laptops").

And for "Linux-Dell-Video", it should be fixed by 5775b843a619 ("PCI:
Restore config space on runtime resume despite being unbound").

So actually I am in favor of removing them all.

Kai-Heng

>
> >
>
> --
> Cheers,
>  Lyude Paul (she/her)
>  Software Engineer at Red Hat
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC 0/2] Stop the abuse of Linux-* _OSI strings
  2022-08-23  3:47     ` Kai-Heng Feng
@ 2022-08-23 17:05       ` Lyude Paul
  0 siblings, 0 replies; 12+ messages in thread
From: Lyude Paul @ 2022-08-23 17:05 UTC (permalink / raw)
  To: Kai-Heng Feng
  Cc: Karol Herbst, Mario Limonciello, linux-kernel, linux-acpi,
	rafael, Len Brown, nouveau, hdegoede, ddadap, Dell.Client.Kernel,
	Aaron Ma

On Tue, 2022-08-23 at 11:47 +0800, Kai-Heng Feng wrote:
> [+Cc Aaron]
> 
> On Tue, Aug 23, 2022 at 5:18 AM Lyude Paul <lyude@redhat.com> wrote:
> > 
> > On Fri, 2022-08-19 at 17:44 +0200, Karol Herbst wrote:
> > > On Fri, Aug 19, 2022 at 4:25 PM Mario Limonciello
> > > <mario.limonciello@amd.com> wrote:
> > > > 
> > > > 3 _OSI strings were introduced in recent years that were intended
> > > > to workaround very specific problems found on specific systems.
> > > > 
> > > > The idea was supposed to be that these quirks were only used on
> > > > those systems, but this proved to be a bad assumption.  I've found
> > > > at least one system in the wild where the vendor using the _OSI
> > > > string doesn't match the _OSI string and the neither does the use.
> > > > 
> > > > So this brings a good time to review keeping those strings in the kernel.
> > > > There are 3 strings that were introduced:
> > > > 
> > > > Linux-Dell-Video
> > > > -> Intended for systems with NVIDIA cards that didn't support RTD3
> > > > Linux-Lenovo-NV-HDMI-Audio
> > > > -> Intended for powering on NVIDIA HDMI device
> > > > Linux-HPI-Hybrid-Graphics
> > > > -> Intended for changing dGPU output
> > > > 
> > > > AFAIK the first string is no longer relevant as nouveau now supports
> > > > RTD3.  If that's wrong, this can be changed for the series.
> > > > 
> > > 
> > > Nouveau always supported RTD3, because that's mainly a kernel feature.
> > > When those were introduced we simply had a bug only hit on a few
> > > systems. And instead of helping us to debug this, this workaround was
> > > added :( We were not even asked about this.
> > > 
> > > I am a bit curious about the other two though as I am not even sure
> > > they are needed at all as we put other work arounds in place. @Lyude
> > > Paul might know more about these.
> > 
> > Some of the _OSI strings are totally fine. From my recollection:
> > 
> > [    0.242993] ACPI: Added _OSI(Linux-Lenovo-NV-HDMI-Audio)
> > ^ this one is needed to do a couple of ACPI tricks at startup to get the PCIe
> > device for audio on nvidia's GPU to be detected properly
> 
> This should be fixed by commit b516ea586d71 ("PCI: Enable NVIDIA HDA
> controllers").
> Aaron worked on more Lenovo systems than me, so he may be more sure of it.
> 
> > 
> > [    0.242993] ACPI: Added _OSI(Linux-HPI-Hybrid-Graphics)
> > 
> > I don't actually know how necessary this is, but I'm hesistant to call this
> > one bad as it may be related to the funny mux configurations that I'm learning
> > may exist on HP machines.
> 
> Should be fixed by commit 8e55f99c510f ("drm/i915: Invoke another _DSM
> to enable MUX on HP Workstation laptops").
> 
> And for "Linux-Dell-Video", it should be fixed by 5775b843a619 ("PCI:
> Restore config space on runtime resume despite being unbound").
> 
> So actually I am in favor of removing them all.

Woo! Thank you for the help with this :3. Since this all seems to be in order:

Reviewed-by: Lyude Paul <lyude@redhat.com>

> 
> Kai-Heng
> 
> > 
> > > 
> > 
> > --
> > Cheers,
> >  Lyude Paul (she/her)
> >  Software Engineer at Red Hat
> > 
> 

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-08-23 18:42 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-19 14:25 [RFC 0/2] Stop the abuse of Linux-* _OSI strings Mario Limonciello
2022-08-19 14:25 ` [RFC 1/2] ACPI: OSI: Remove Linux-Dell-Video _OSI string Mario Limonciello
2022-08-19 14:25 ` [RFC 2/2] ACPI: OSI: Deprecate some abused _OSI strings Mario Limonciello
2022-08-19 22:02   ` Daniel Dadap
2022-08-19 15:44 ` [RFC 0/2] Stop the abuse of Linux-* " Karol Herbst
2022-08-19 16:00   ` Limonciello, Mario
2022-08-19 16:37     ` Karol Herbst
2022-08-19 16:43       ` Limonciello, Mario
2022-08-19 16:47         ` Karol Herbst
2022-08-22 21:18   ` Lyude Paul
2022-08-23  3:47     ` Kai-Heng Feng
2022-08-23 17:05       ` Lyude Paul

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).