linux-kernel - Re: [PATCH v5 00/20] Add support for binding ACPI platform profile to multiple drivers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d08d40f5-5482-4e7f-99b4-a5d9e403b828@gmx.de>
Date: Thu, 7 Nov 2024 10:06:07 +0100
From: Armin Wolf <W_Armin@....de>
To: Mario Limonciello <mario.limonciello@....com>,
 Hans de Goede <hdegoede@...hat.com>,
 Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
Cc: "Rafael J . Wysocki" <rafael@...nel.org>, Len Brown <lenb@...nel.org>,
 Maximilian Luz <luzmaximilian@...il.com>, Lee Chun-Yi <jlee@...e.com>,
 Shyam Sundar S K <Shyam-sundar.S-k@....com>,
 Corentin Chary <corentin.chary@...il.com>, "Luke D . Jones"
 <luke@...nes.dev>, Ike Panhc <ike.pan@...onical.com>,
 Henrique de Moraes Holschuh <hmh@....eng.br>,
 Alexis Belmonte <alexbelm48@...il.com>,
 Uwe Kleine-König <u.kleine-koenig@...gutronix.de>,
 Ai Chao <aichao@...inos.cn>, Gergo Koteles <soyer@....hu>,
 open list <linux-kernel@...r.kernel.org>,
 "open list:ACPI" <linux-acpi@...r.kernel.org>,
 "open list:MICROSOFT SURFACE PLATFORM PROFILE DRIVER"
 <platform-driver-x86@...r.kernel.org>,
 "open list:THINKPAD ACPI EXTRAS DRIVER"
 <ibm-acpi-devel@...ts.sourceforge.net>,
 Mark Pearson <mpearson-lenovo@...ebb.ca>,
 Matthew Schwartz <matthew.schwartz@...ux.dev>
Subject: Re: [PATCH v5 00/20] Add support for binding ACPI platform profile to
 multiple drivers

Am 07.11.24 um 07:02 schrieb Mario Limonciello:

> Currently there are a number of ASUS products on the market that happen to
> have ACPI objects for amd-pmf to bind to as well as an ACPI platform
> profile provided by asus-wmi.
>
> The ACPI platform profile support created by amd-pmf on these ASUS
> products is "Function 9" which is specifically for "BIOS or EC
> notification" of power slider position. This feature is actively used
> by some designs such as Framework 13 and Framework 16.
>
> On these ASUS designs we keep on quirking more and more of them to turn
> off this notification so that asus-wmi can bind.
>
> This however isn't how Windows works.  "Multiple" things are notified for
> the power slider position. This series adjusts Linux to behave similarly.
>
> Multiple drivers can now register an ACPI platform profile and will react
> to set requests.
>
> To avoid chaos, only positions that are common to both drivers are
> accepted when the legacy /sys/firmware/acpi/platform_profile interface
> is used.
>
> This series also adds a new concept of a "custom" profile.  This allows
> userspace to discover that there are multiple driver handlers that are
> configured differently.
>
> This series also allows dropping all of the PMF quirks from amd-pmf.

Thank you for this patch series. The overall design seems good to me, but i think
you forgot to extend platform_profile_notify().

Thanks,
Armin Wolf

> ---
> v5:
>   * Adjust mutex handling
>   * Add missing error handling
>   * Drop dev member
>   * Add cleanup handling for module unload
>   * Fix crash on accessing legacy files after all drivers unloaded
>
> Mario Limonciello (20):
>    ACPI: platform-profile: Add a name member to handlers
>    platform/x86/dell: dell-pc: Create platform device
>    ACPI: platform_profile: Add platform handler argument to
>      platform_profile_remove()
>    ACPI: platform_profile: Move sanity check out of the mutex
>    ACPI: platform_profile: Move matching string for new profile out of
>      mutex
>    ACPI: platform_profile: Use guard(mutex) for register/unregister
>    ACPI: platform_profile: Use `scoped_cond_guard`
>    ACPI: platform_profile: Create class for ACPI platform profile
>    ACPI: platform_profile: Unregister class and sysfs group on module
>      unload
>    ACPI: platform_profile: Add name attribute to class interface
>    ACPI: platform_profile: Add choices attribute for class interface
>    ACPI: platform_profile: Add profile attribute for class interface
>    ACPI: platform_profile: Notify change events on register and
>      unregister
>    ACPI: platform_profile: Only show profiles common for all handlers
>    ACPI: platform_profile: Add concept of a "custom" profile
>    ACPI: platform_profile: Make sure all profile handlers agree on
>      profile
>    ACPI: platform_profile: Check all profile handler to calculate next
>    ACPI: platform_profile: Allow multiple handlers
>    platform/x86/amd: pmf: Drop all quirks
>    Documentation: Add documentation about class interface for platform
>      profiles
>
>   .../ABI/testing/sysfs-platform_profile        |   5 +
>   .../userspace-api/sysfs-platform_profile.rst  |  28 +
>   drivers/acpi/platform_profile.c               | 494 ++++++++++++++----
>   .../surface/surface_platform_profile.c        |   7 +-
>   drivers/platform/x86/acer-wmi.c               |   5 +-
>   drivers/platform/x86/amd/pmf/Makefile         |   2 +-
>   drivers/platform/x86/amd/pmf/core.c           |   1 -
>   drivers/platform/x86/amd/pmf/pmf-quirks.c     |  66 ---
>   drivers/platform/x86/amd/pmf/pmf.h            |   3 -
>   drivers/platform/x86/amd/pmf/sps.c            |   3 +-
>   drivers/platform/x86/asus-wmi.c               |   5 +-
>   drivers/platform/x86/dell/alienware-wmi.c     |   3 +-
>   drivers/platform/x86/dell/dell-pc.c           |  35 +-
>   drivers/platform/x86/hp/hp-wmi.c              |   3 +-
>   drivers/platform/x86/ideapad-laptop.c         |   3 +-
>   .../platform/x86/inspur_platform_profile.c    |   6 +-
>   drivers/platform/x86/thinkpad_acpi.c          |   3 +-
>   include/linux/platform_profile.h              |   6 +-
>   18 files changed, 488 insertions(+), 190 deletions(-)
>   delete mode 100644 drivers/platform/x86/amd/pmf/pmf-quirks.c
>
>
> base-commit: d68cb6023356af3bd3193983ad4ec03954a0b3e2