diff mbox series

[RFC,2/2] ACPI: OSI: Deprecate some abused _OSI strings

Message ID 20220819142519.5684-3-mario.limonciello@amd.com
State New
Headers show
Series Stop the abuse of Linux-* _OSI strings | expand

Commit Message

Mario Limonciello Aug. 19, 2022, 2:25 p.m. UTC
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(-)

Comments

Daniel Dadap Aug. 19, 2022, 10:02 p.m. UTC | #1
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))
diff mbox series

Patch

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