linux-kernel - Re: [PATCH v3 0/6] platform/x86: firmware_attributes

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: <DASS0MF0SCE9.1AU3QG8U6A6WV@gmail.com>
Date: Sun, 22 Jun 2025 01:27:33 -0300
From: "Kurt Borja" <kuurtb@...il.com>
To: "Derek J. Clark" <derekjohn.clark@...il.com>, "Hans de Goede"
 <hdegoede@...hat.com>, Ilpo Järvinen
 <ilpo.jarvinen@...ux.intel.com>, Thomas Weißschuh
 <linux@...ssschuh.net>, "Joshua Grisham" <josh@...huagrisham.com>, "Mark
 Pearson" <mpearson-lenovo@...ebb.ca>, "Armin Wolf" <W_Armin@....de>, "Mario
 Limonciello" <mario.limonciello@....com>
Cc: "Antheas Kapenekakis" <lkml@...heas.dev>, "Prasanth Ksr"
 <prasanth.ksr@...l.com>, "Jorge Lopez" <jorge.lopez2@...com>,
 <platform-driver-x86@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
 <Dell.Client.Kernel@...l.com>
Subject: Re: [PATCH v3 0/6] platform/x86: firmware_attributes_class: Add a
 high level API

On Sun Jun 22, 2025 at 12:42 AM -03, Derek J. Clark wrote:
>
>
> On June 21, 2025 8:27:06 PM PDT, Kurt Borja <kuurtb@...il.com> wrote:
>>On Sun Jun 22, 2025 at 12:01 AM -03, Derek J. Clark wrote:
>>>
>>>
>>> On June 21, 2025 5:04:03 PM PDT, Kurt Borja <kuurtb@...il.com> wrote:
>>>>Hi all,
>>>>
>>>>I apologize for taking so long. I've been a bit busy these last few
>>>>weeks.
>>>>
>>>>After my discussion with Joshua on v2, I realized the API I made was not
>>>>ergonomic at all and it didn't exactly respond to driver needs. In this
>>>>version I tried a completely different approach and IMO it's much much
>>>>better now.
>>>>
>>>>First of all I adopted standard sysfs terminology for everything. A
>>>>"firmware attribute" is just an attribute_group under the attributes/
>>>>directory so everything related to this concept is just called "group"
>>>>now. Everything refered as properties in the previous patch are now just
>>>>plain "attributes".
>>>>
>>>>This new API revolves around the `fwat_{bool,enum,int,str}_data`
>>>>structs. These hold all the metadata a "firmware_attribute" of that
>>>>given type needs.
>>>>
>>>>These structs also hold `read` and `write` callbacks for the
>>>>current_value attribute, because obviously that value is always dynamic.
>>>>However the rest of attributes (default_value, display_name, min, max,
>>>>etc) are constant.
>>>
>>> Hi Kurt,
>>>
>>> In the lenovo-wmi drivers the min/max for multiple attributes are actually dynamic based on if power is AC connected or on battery. Looking at patch 2 I might be able to do some pointer manipulation with the attribute's "data" member for those events to make this work, but it would be a lot easier if there was a simple way for me to call my own functions here instead. Perhaps a function pointer could be used to override the default method here? 
>>
>>Hi Derek,
>>
>>All attributes in a given group have the same show method. Maybe we can
>>let users override this with their own show method, i.e. Add a
>>
>>	ssize_t (*attr_show)(struct device *dev, const struct fwat_attribute *attr, const char *buf)
>>
>>to struct fwat_group_data. That should be fairly simple to implement.
>>
>>Did you have another solution in mind?
>>
>>
>
> That should work, yeah. 
> - Derek

Just realized that's exactly what you said :)

It was indeed easy to implement. It will be there for next version.


-- 
 ~ Kurt