Re: [RFC PATCH 0/1] platform: x86: tuxi: Implement TUXEDO TUXI ACPI TFAN as thermal subsystem

[Armin Wolf <W_Armin@gmx.de>]

Am 27.11.24 um 12:21 schrieb Werner Sembach:

> Hi,
>
> Following up to https://lore.kernel.org/all/172b7acd-4313-4924-bcbc-41b73b39ada0@tuxedocomputers.com/ and https://lore.kernel.org/all/f26d867e-f247-43bb-a78b-be0bce35c973@roeck-us.net/ I experimented with the thermal subsystem and these are my results so far, but I'm hitting a bit of a wall:
>
> As far as I can tell to implement "2. As long as GTMP is > 80°C fan speed must be at least 30%." I would need to add a new gevenor, lets call it "user_space_with_safeguards". I would be nice when the temp <-> fanspeed relation could be passed via the thermal_trip structure. And safeguarding the hardware from userspace only works when I can restrict userspace from just selecting the preexisting "user_space" govenor.
>
> So my ideas/questions:
> - Add a field "min_fanspeed_percent" to the thermal_trip struct that will only be used by the "user_space_with_safeguards" govenor
> - Add a "user_space_with_safeguards" govenor that is the same as the "user_space" govenor, but on trip, a minimum speed is applied
> - How can i ensure that on further speed updates the min speed is applied to? I could just implement it directly in the cdev, but that would be spagetti coding around the govenor.
> - Can I somehow restrict userspace from using certain govenors?
> - I'm a litte bit confused about the thermal zone "mode" sysfs switch, here it says deactivate for userspace control: https://elixir.bootlin.com/linux/v6.12/source/Documentation/ABI/testing/sysfs-class-thermal#L20, but what about the user_space govenor then?

Hi,

i am having little experience with the thermal subsystem, but i suggest that policy decisions like "min_fanspeed_percent" should either:

- come from the hardware/firmware itself
- come from userspace

Effectively this driver tries to enforce a Tuxedo-specific policy that is not directly based on hardware limits. The book "Linux Device Drivers"
says:

	"the role of a device driver is providing///mechanism/, not/policy/."

Furthermore:

	"When/writing/ drivers, a programmer should pay particular attention to this fundamental concept: write kernel code to access the hardware,
	 but don't force particular policies on the user, since different users have different needs. The driver should deal with making the hardware
	 available, leaving all the issues about/how/ to use the hardware to the applications. A driver, then, is flexible if it offers access to the
	 hardware capabilities without adding constraints."

The issue is that the Tuxedo-specific policy is not directly connected with the hardware. Some hardware might need a bigger minimum fan speed than
other hardware for example.

The hardware (or rather firmware in this case) should communicate those constraints to the linux driver so that we do not need to rely on random
temperatures for hardware protection.

This ACPI interface however basically provides us with a hwmon interface and Tuxedo now wants the kernel to enforce their policy on it. I suspect that
happens for warranty reasons, right?

Maybe there is a way to enforce this policy through userspace applications?

Thanks,
Armin Wolf

> Best regards,
> Werner
>
> Werner Sembach (1):
>    platform: x86: tuxi: Implement TUXEDO TUXI ACPI TFAN as thermal
>      subsystem
>
>   drivers/platform/x86/Makefile                 |   3 +
>   drivers/platform/x86/tuxedo/Kbuild            |   6 +
>   drivers/platform/x86/tuxedo/nbxx/Kbuild       |   8 +
>   drivers/platform/x86/tuxedo/nbxx/Kconfig      |   9 +
>   drivers/platform/x86/tuxedo/nbxx/acpi_tuxi.c  |  96 +++++++
>   drivers/platform/x86/tuxedo/nbxx/acpi_tuxi.h  |  84 ++++++
>   .../x86/tuxedo/nbxx/acpi_tuxi_thermal.c       | 241 ++++++++++++++++++
>   .../x86/tuxedo/nbxx/acpi_tuxi_thermal.h       |  14 +
>   8 files changed, 461 insertions(+)
>   create mode 100644 drivers/platform/x86/tuxedo/Kbuild
>   create mode 100644 drivers/platform/x86/tuxedo/nbxx/Kbuild
>   create mode 100644 drivers/platform/x86/tuxedo/nbxx/Kconfig
>   create mode 100644 drivers/platform/x86/tuxedo/nbxx/acpi_tuxi.c
>   create mode 100644 drivers/platform/x86/tuxedo/nbxx/acpi_tuxi.h
>   create mode 100644 drivers/platform/x86/tuxedo/nbxx/acpi_tuxi_thermal.c
>   create mode 100644 drivers/platform/x86/tuxedo/nbxx/acpi_tuxi_thermal.h
>