Message-ID: <20170522094758.10cb455c@endymion>
Date:   Mon, 22 May 2017 09:47:58 +0200
From:   Jean Delvare <jdelvare@...e.de>
To:     <Mario.Limonciello@...l.com>
Cc:     <gregkh@...uxfoundation.org>, <Allen.Hung@...l.com>,
        <rmk+kernel@....linux.org.uk>, <somlo@....edu>,
        <jens.wiklander@...aro.org>, <agross@...eaurora.org>,
        <arnd@...db.de>, <sudeep.holla@....com>, <eric@...olt.net>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 2/2] dmi-id: add dmi/id/oem group for exporting oem
 strings to sysfs

Hi Mario,

On Mon, 1 May 2017 20:58:13 +0000, Mario.Limonciello@...l.com wrote:
> > That I understand. What I do not understand is how software is supposed
> > to figure out which string represents what, even if it knows how many
> > strings there are in total.  
> 
> At least for Dell, not all of these are useful to general purpose userspace
> software but more so to internal tools.

Well I think it is in the very nature of OEM strings to be only useful
for vendor-specific tools. Nothing Dell-specific here.

> The most useful one is one that indicates "Dell System" and would be
> always indexed to "String 1" on Dell systems.
> 
> If *any* kernel implementation that exports to userspace would be 
> acceptable, recognizing that particular OEM string would be the most 
> valuable.

Understood, more on this below.

> This might seem silly, why not just read /sys/class/dmi/id/sys_vendor
> right?
> 
> Most business client products at Dell have  the ability to be OEM'ed.  
> When this happens the standard SMBIOS strings will be wiped and 
> filled with data related to the OEM that will be re-selling them.
> 
> This unfortunately breaks some presumptions that kernel makes
> about when quirks are applied to particular systems by DMI data.
> Modules that look for "Dell Inc." won't load, quirks that are matched
> on a "Latitude 7140" wouldn't apply etc.
>
> Userspace similarly breaks when presumptions are made.  For example 
> fwupd in displaying the value of the ESRT properly.  It's quirked
> to display how Dell encodes it based upon SMBIOS data in userspace.
> This causes UEFI capsules to not apply on OEM'ed systems.
> 
> Not many systems that are OEM'ed are sold with Linux today (or used
> with to our knowledge), but we're anticipating some changes in this, 
> and that's why we raised this in the first place.

This is rather good news :-)

> So we would really like a way in the kernel to be able to recognize
> this situation and both tell kernel modules and tell userspace.

OK, I get the idea. It should be noted that the patch you initially
proposed, and I declined, did not fully solve this actual problem.
Making the OEM strings available to user-space through sysfs does not
allow for modalias-based auto-loading of kernel drivers based on these
OEM strings. It would only allow user-space tools or scripts to
manually poke at OEM strings to make decisions. Only the data included
in /sys/devices/virtual/dmi/id/modalias can be used to load drivers
automatically, and OEM strings aren't part of that. Nor do I think they
should, as they are non-standard by nature.

>From a kernel driver perspective, your candidate patch did not change
anything, as the OEM strings are already available to kernel drivers
today, using dmi_find_device(DMI_DEV_TYPE_OEM_STRING, ...).

Overall it looks like an XY problem to me. You came to us with a
candidate solution to problem Y (making OEM strings available to
user-space), and we have spent some time debating it (you doing it in
the kernel, me in dmidecode) while in the end neither implementation
will address problem X (exposing the name of the original hardware
manufacturer behind the OEM vendor, ideally in a way which allows
module auto-loading.)

> > Note that I am not blaming Dell here, the SMBIOS specification is
> > really weak in this respect, it would have better defined string pairs,
> > so that the BIOS can specify both string names and values; or defined a
> > standard format to include both in one string (simply defining a
> > standard separator character such as ";" or "=" would have done the
> > trick.) Leaving it to each vendor to sort it out was the best way to
> > ensure there wouldn't be any consistency. Sigh.
> 
> It the DMTF had something better defined, absolutely we would follow
> it for the future.  Do you sit on DMTF?

No, I do not. It would probably make sense, but I have no idea how to
make it happen, nor the requirements and implications.

> > (...)
> > Below is my second proposal of a dmidecode patch implementing what you
> > need. I believe I have addressed all your concerns, but please report
> > if there is anything left to fix [1]. If you are happy with this patch,
> > I will include it in the upcoming release 3.1 of dmidecode.
> 
> I'll do some testing with this later, but I would still really like a way to

Did you test? I would like to release dmidecode 3.1 by the end of the
month, and it would be great if this feature could be included.

> take care of the situation I raised above.  We can set up tools like
> fwupd to have a dependency of dmidecode 3.1+ and then shell out to
> dmidecode and use this functionality to parse the strings and pass up
> whether it's a Dell system, but is that really the best way?

I think it is, yes.

You would have a dependency on something anyway, as the feature doesn't
exist today. If it was implemented in the kernel as you wanted, you
would depend on a specific kernel version (or backport of your commit.)
Most people will find it a lot easier to have to install dmidecode 3.1
(or 3.0 with my commit backported) than to upgrade their kernel.

> It does still leave the kernel modules matching to be problematic too.

I agree, as my own conclusions above indicate.

I think we need to start thinking again from scratch. As I understand
it, what you really need is a way to include the original maker of the
hardware in some modalias string, so that udev can trigger the loading
of the appropriate kernel modules. Is that correct?

In the case of Dell, the information is present in the DMI OEM string
#1. However this is a Dell-specific decision, so just always including
this string in /sys/devices/virtual/dmi/id/modalias would only work for
Dell and not other hardware makers. Therefore I think we need one level
of indirection.

One way to implement this would be to add another key at the end
of /sys/devices/virtual/dmi/id/modalias, maybe "osvn" for "original
system vendor" (tell me if you have a better idea.) The value of this
key would be computed by sequentially testing a number of hypotheses,
starting with checking if OEM string #1 is "Dell System". In which case
osvn would be set to "DellInc." or whatever you think is appropriate.
For over OEM providers, other tests could be added later. If no test
succeeds then the extra key is left empty.

Kernel drivers which should be loaded on every system originally
designed by some hardware maker could then just match the extra key
(which will thus need an internal name and ID as well, something like
DMI_ORIGINAL_SYS_VENDOR, added to enum dmi_field.)

The value could be additionally exposed
as /sys/devices/virtual/dmi/id/original_sys_vendor or some such if you
think that could be useful.

What do you think?

Are we good with one original maker string, or should we envision the
possibility that different parts of the system could be from different
original makers?

Still... Before going down that road, can you convince me that the
above is really needed? I mean, let's assume vendor Magicfoo decides to
base their upcoming laptops on an original Dell design. Why can't we
just add:

	{
		.matches = {
			DMI_MATCH(DMI_SYS_VENDOR, "Magicfoo"),
			DMI_MATCH(DMI_CHASSIS_TYPE, "9"), /*Laptop*/
		},
	},

to drivers/platform/x86/dell-laptop.c and be done with it? The driver
can simply bail out if loaded on a system where it isn't actually
needed. Isn't it good enough?

-- 
Jean Delvare
SUSE L3 Support

Powered by blists - more mailing lists