Message-ID: <916b7da1eb8941e383d7d73936cb8d7a@ausx13mpc120.AMER.DELL.COM>
Date:   Mon, 22 May 2017 19:53:29 +0000
From:   <Mario.Limonciello@...l.com>
To:     <jdelvare@...e.de>
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

> > 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.)

So far we've been lucky and systems that we know get OEM'ed
have only needed to be quirked one time that I know of.  That quirk
ended up being done off of the PCI host controller 
subsystem vendor and product ID.

More on that as an option later.  You're entirely right though.  
We were only solving one half of the problem with the solution 
we proposed so far.

> 
> > > 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.

I agree that pushing a requirement in this area will probably break "someone".
I guess let's table that for now and figure out the right solution for "current".

> 
> > > (...)
> > > 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.

Yes, sorry I did test this and it works properly.  I think this is good for
dmidecode. 

> 
> > 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.
> 

This is true.  It's subjective where it's "easy" since for factory installed stuff
we work with our various Linux OS vendors to build images with backported
stuff.

For the general use case, yeah I agree installing a newer userspace application
is easier than backporting a kernel patch and rebuilding a 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?

Not just for udev triggering, but also internal use within the kernel.
There's plenty of places that drivers special case on DMI data using stuff like this:
DMI_MATCH(DMI_PRODUCT_NAME,

> 
> 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.
>
Yeah I agree.
 
> 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?
> 

I think this is actually a pretty good proposal.  Once extension I'd like
to add though is "ospn" for "Original System Product Name".

There is a hexadecimal string used to indicate the system identifier.  
It's the same thing you would see if you looked at PCI subsystem
device ID across the various PCI devices in the box.

Between that combination of both keys you can do modaliasing or quirking
accurately and to the same granularity you can today with 
DMI_SYS_VENDOR/DMI_PRODUCT_NAME.

> 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?

The hardware will be identical to a Dell branded machine, but as I said
above the model number will be changed.

> 
> 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?

Dell internal teams and partners are given direction not to depend 
anything upon the marketing model ID (DMI_PRODUCT_NAME) because
this rebranding can be applied to any product.  They instead depend upon
the value that I mentioned above that would be that original system
produce name.
For example on my  Precision T7600 this would be "0495":
        String 1: Dell System
        String 2: 1[0495]

Of course you don't really have an idea of what scale this happens, but
in some business segments more machines are sold as OEM'ed than Dell
branded.  Newly branded machines can be introduced at any time.

As a result, it's not really a manageable approach to document every
single machine that has been OEM'ed.  (I suspect the team that works on 
OEM'ing these would consider it proprietary information too.)


So all that said, maybe is the simpler approach to export the information
from PCI subsystem vendor and device out to the system as a pair of new
DMI match variables?   I think both X & Y could be solved in the same solution
to get this information if exported to the right places.

Example, my same system you'll get:
00:00.0 Host bridge [0600]: Intel Corporation Xeon E5/Core i7 DMI2 [8086:3c00] (rev 05)
        Subsystem: Dell Xeon E5/Core i7 DMI2 [1028:0495]

1028 is Dell
0495 is the system product ID I referred to above.

Powered by blists - more mailing lists