[systemd-devel] Fwd / FYI: [PATCH kernel] firmware/dmi: Move product_sku info to the end of the modalias

[Hans de Goede]

Hi,

On 8/31/21 3:08 PM, Hans de Goede wrote:
> Hi,
> 
> I BCc-ed systemd-devel on this, to avoid reply-to-all from kernel folks
> getting bounces because of them not being subscribed, but it seems the
> list does not like being in the BCc, so hence this forward.
> 
> Note this is just FYI, as mentioned I'll also prepare a systemd pull-req
> (also) fixing this on the systemd/hwdb side.
> 
> Regards,
> 
> Hans
> 
> 
> -------- Forwarded Message --------
> Subject: [PATCH regression fix] firmware/dmi: Move product_sku info to the end of the modalias
> Date: Tue, 31 Aug 2021 15:05:08 +0200
> From: Hans de Goede <hdegoede at redhat.com>
> To: Jean Delvare <jdelvare at suse.com>
> CC: Hans de Goede <hdegoede at redhat.com>, linux-kernel at vger.kernel.org, stable at vger.kernel.org, Kai-Chuan Hsieh <kaichuan.hsieh at canonical.com>, Erwan Velu <e.velu at criteo.com>
> 
> Commit e26f023e01ef ("firmware/dmi: Include product_sku info to modalias")
> added a new field to the modalias in the middle of the modalias, breaking
> some existing udev/hwdb matches on the whole modalias without a wildcard
> ('*') in between the pvr and rvn fields.
> 
> All modalias matches in e.g. :
> https://github.com/systemd/systemd/blob/main/hwdb.d/60-sensor.hwdb
> deliberately end in ':*' so that new fields can be added at *the end* of
> the modalias, but adding a new field in the middle like this breaks things.
> 
> Move the new sku field to the end of the modalias to fix some hwdb
> entries no longer matching.
> 
> The new sku field has already been put to use in 2 new hwdb entries:
> 
>  sensor:modalias:platform:HID-SENSOR-200073:dmi:*svnDell*:sku0A3E:*
>   ACCEL_LOCATION=base
> 
>  sensor:modalias:platform:HID-SENSOR-200073:dmi:*svnDell*:sku0B0B:*
>   ACCEL_LOCATION=base
> 
> The wildcard use before and after the sku in these matches means that they
> should keep working with the sku moved to the end.
> 
> Note that there is a second instance of in essence the same problem,
> commit f5152f4ded3c ("firmware/dmi: Report DMI Bios & EC firmware release")
> 
> Added 2 new br and efr fields in the middle of the modalias. This too
> breaks some hwdb modalias matches, but this has gone unnoticed for over
> a year. So some newer hwdb modalias matches actually depend on these
> fields being in the middle of the string. Moving these to the end now
> would break 3 hwdb entries, while fixing 8 entries.
> 
> Since there is no good answer for the new br and efr fields I have chosen
> to leave these as is. Instead I'll submit a hwdb update to put a wildcard
> at the place where these fields may or may not be present depending on the
> kernel version.

In case anyone is interested here is the systemd pull-req fixing this from
the hwdb side: https://github.com/systemd/systemd/pull/20599

Regards,

Hans





> BugLink: https://github.com/systemd/systemd/issues/20550
> Link: https://github.com/systemd/systemd/pull/20562
> Fixes: e26f023e01ef ("firmware/dmi: Include product_sku info to modalias")
> Cc: stable at vger.kernel.org
> Cc: Kai-Chuan Hsieh <kaichuan.hsieh at canonical.com>
> Cc: Erwan Velu <e.velu at criteo.com>
> Signed-off-by: Hans de Goede <hdegoede at redhat.com>
> ---
>  drivers/firmware/dmi-id.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/firmware/dmi-id.c b/drivers/firmware/dmi-id.c
> index 4d5421d14a41..940ddf916202 100644
> --- a/drivers/firmware/dmi-id.c
> +++ b/drivers/firmware/dmi-id.c
> @@ -73,6 +73,10 @@ static void ascii_filter(char *d, const char *s)
>  
>  static ssize_t get_modalias(char *buffer, size_t buffer_size)
>  {
> +	/*
> +	 * Note new fields need to be added at the end to keep compatibility
> +	 * with udev's hwdb which does matches on "`cat dmi/id/modalias`*".
> +	 */
>  	static const struct mafield {
>  		const char *prefix;
>  		int field;
> @@ -85,13 +89,13 @@ static ssize_t get_modalias(char *buffer, size_t buffer_size)
>  		{ "svn", DMI_SYS_VENDOR },
>  		{ "pn",  DMI_PRODUCT_NAME },
>  		{ "pvr", DMI_PRODUCT_VERSION },
> -		{ "sku", DMI_PRODUCT_SKU },
>  		{ "rvn", DMI_BOARD_VENDOR },
>  		{ "rn",  DMI_BOARD_NAME },
>  		{ "rvr", DMI_BOARD_VERSION },
>  		{ "cvn", DMI_CHASSIS_VENDOR },
>  		{ "ct",  DMI_CHASSIS_TYPE },
>  		{ "cvr", DMI_CHASSIS_VERSION },
> +		{ "sku", DMI_PRODUCT_SKU },
>  		{ NULL,  DMI_NONE }
>  	};
>  
>