Re: [PATCH] Introduce support for Systems Management Driver over WMI for Dell Systems

[Hans de Goede]

Hi,

On 9/22/20 10:57 AM, Hans de Goede wrote:
> Hi,
>
> On 9/21/20 5:26 PM, Limonciello, Mario wrote:
>
> <snip>
>
> I will do another more detailed reply in another email, but I would like to focus
> at the main pain point here, which is the using a generic sysfs-ABI/class vs using
> a Dell specific sysfs-ABI.
>
> > > I guess a could way to look at the generic sysfs firmware attributes
> > > class I'm proposing is looking at it as a lowest common denominator
> > > solution. With the addition of vendor specific extensions so that
> > > vendors (e.g. Dell) are not limited to only offering functionality
> > > offered by the generic, shared ABI. Does that make sense ?
> > >
> > > Regards,
> >
> > I really think that trying to fit all the vendors into the same interface is going
> > to stifle areas for innovation in the firmware and kernel space in the name of
> > "simplicity" which really only goes as far as the kernel side.  Userspace has
> > to carry delta between vendors no matter what, so why introduce a LCD then?
> >
> > Just as easily we could have:
> > /sys/devices/platform/dell-wmi-sysman/attributes/
> >
> > Which works 90% the same as:
> > /sys/devices/platform/lenovo-wmi-sysman/attributes/
>
> So the reason why I want a class interface for this is to allow say
> FleetCommander to have a generic plugin implementing that 90%, so
> no deps, only support plain admin-password authentication.
>
> Allowing such a generic plugin requires 2 things:
>
> 1) Ensuring that the 90% overlapping functionality offers a 100%
> identical userspace ABI, thus a shared sysfs ABI definition
>
> 2) That userspace has a generic way to enumerate devices/drivers
> implementing this shared sysfs ABI, and we have a standard
> mechanism for enumerating drivers which implement a standard ABI,
> that is we make them register class devices under /sys/class/<abi-name>.
>
> I have not heard any convincing arguments against why would
> should not or can not have these 2 things. All I'm hearing is
> a vague fear that this may "stifle areas for innovation in the firmware
> and kernel space".
>
> Honestly I have the feeling we are going in circles in this discussion
> and I really do not understand why you are so dead set against having
> a common sysfs ABI/class for this?
>
> In part of the snipped text you write "Having to de-feature the sysfs
> interface", but I have not asked you to remove any features anywhere in
> this thread!
>
> So I really do not understand where this fear of not being able to
> implement certain, possibly Dell specific, features comes from?
>
> You mentioned that the way the dependencies are expressed are
> highly Dell specific, so I suggested allowing having vendor
> extensions like dell-modifiers and dell-value_modifiers. The whole
> idea behind allowing vendor-extensions is actually the exact
> opposite of de-featuring the sysfs interface.


So I've been thinking more about this and to me this whole argument
sounds a lot like we just want to have our own little corner to
play in, without needing to worry about what other vendors do.

And then Lenovo, and HP and who knows else will all want the same
and we and up with at least 5 different interfaces.

It is bad enough that we already have to deal with having 5+
different firmware interfaces for this and worse even for silly
things like setting the brightness level for the kbd backlight,
which is such a trivial thing that you would think PC vendors
should be able to sit down and agree on a single ACPI API for it.

We are NOT going to take this lets all do our own thing approach and
also let this trickle up in the stack to the kernel <-> userspace API!

One of the tasks of the kernel is to act as a HAL and this clearly
falls under that. Imagine if userspace code would need to use different
kernel APIs for storage/filesystem accesses depending on if they were
running on a Dell or a Lenovo machines. Or having different APIs to
access the network depending on the machine vendor...

So I'm sorry, but I'm drawing a line in the sand here, unless you can
come up with some really convincing NEW arguments why this needs to
be a Dell specific interface, the Dell firmware-attributes code must
use a generic sysfs-ABI/class to get accepted upstream.

Note that I think the currently suggested private Dell ABI is actually
pretty suitable for such a generic sysfs-ABI/class, so I'm not asking
you to make a lot of changes here.

Regards,

Hans