Messages in this thread Patch in this message | | | Subject | Re: [RFC 2/2] ACPI: OSI: Deprecate some abused _OSI strings | From | Daniel Dadap <> | Date | Fri, 19 Aug 2022 17:02:47 -0500 |
| |
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))
| |