In DMI type 0, there is several fields that encodes a release.
The dmi_save_release() function have the logic to check if the field is valid.
If so, it reports the actual value.
Signed-off-by: Erwan Velu <[email protected]>
---
drivers/firmware/dmi_scan.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
index 35ed56b9c34f..202bd2c69d0f 100644
--- a/drivers/firmware/dmi_scan.c
+++ b/drivers/firmware/dmi_scan.c
@@ -181,6 +181,32 @@ static void __init dmi_save_ident(const struct dmi_header *dm, int slot,
dmi_ident[slot] = p;
}
+static void __init dmi_save_release(const struct dmi_header *dm, int slot,
+ int index)
+{
+ const u8 *d;
+ char *s;
+
+ // If the table doesn't have the field, let's return
+ if (dmi_ident[slot] || dm->length < index)
+ return;
+
+ d = (u8 *) dm + index;
+
+ // As per the specification,
+ // if the system doesn't have the field, the value is FF
+ if (d[0] == 0xFF)
+ return;
+
+ s = dmi_alloc(4);
+ if (!s)
+ return;
+
+ sprintf(s, "%u", d[0]);
+
+ dmi_ident[slot] = s;
+}
+
static void __init dmi_save_uuid(const struct dmi_header *dm, int slot,
int index)
{
--
2.21.0
Some vendors like HPe or Dell, encodes the release version of their BIOS
in the "System BIOS {Major|Minor} Release" fields of Type 0.
This information is useful to know which release of the bios is actually running.
It could be used for some quirks, debugging sessions or inventory tasks.
This patch extract these 2 fields in DMI_BIOS_MAJOR_RELEASE & DMI_BIOS_MINOR_RELEASE.
A typical output for a Dell system running the 65.27 bios is :
[root@t1700 ~]# cat /sys/devices/virtual/dmi/id/bios_release_major
65
[root@t1700 ~]# cat /sys/devices/virtual/dmi/id/bios_release_minor
27
[root@t1700 ~]#
Signed-off-by: Erwan Velu <[email protected]>
---
drivers/firmware/dmi-id.c | 6 ++++++
drivers/firmware/dmi_scan.c | 2 ++
include/linux/mod_devicetable.h | 2 ++
scripts/mod/file2alias.c | 2 ++
4 files changed, 12 insertions(+)
diff --git a/drivers/firmware/dmi-id.c b/drivers/firmware/dmi-id.c
index ff39f64f2aae..3248c2837a4d 100644
--- a/drivers/firmware/dmi-id.c
+++ b/drivers/firmware/dmi-id.c
@@ -42,6 +42,8 @@ DEFINE_DMI_ATTR_WITH_SHOW(bios_vendor, 0444, DMI_BIOS_VENDOR);
DEFINE_DMI_ATTR_WITH_SHOW(bios_version, 0444, DMI_BIOS_VERSION);
DEFINE_DMI_ATTR_WITH_SHOW(bios_date, 0444, DMI_BIOS_DATE);
DEFINE_DMI_ATTR_WITH_SHOW(sys_vendor, 0444, DMI_SYS_VENDOR);
+DEFINE_DMI_ATTR_WITH_SHOW(bios_release_major, 0444, DMI_BIOS_MAJOR_RELEASE);
+DEFINE_DMI_ATTR_WITH_SHOW(bios_release_minor, 0444, DMI_BIOS_MINOR_RELEASE);
DEFINE_DMI_ATTR_WITH_SHOW(product_name, 0444, DMI_PRODUCT_NAME);
DEFINE_DMI_ATTR_WITH_SHOW(product_version, 0444, DMI_PRODUCT_VERSION);
DEFINE_DMI_ATTR_WITH_SHOW(product_serial, 0400, DMI_PRODUCT_SERIAL);
@@ -78,6 +80,8 @@ static ssize_t get_modalias(char *buffer, size_t buffer_size)
{ "bvn", DMI_BIOS_VENDOR },
{ "bvr", DMI_BIOS_VERSION },
{ "bd", DMI_BIOS_DATE },
+ { "bjr", DMI_BIOS_MAJOR_RELEASE },
+ { "bmr", DMI_BIOS_MINOR_RELEASE },
{ "svn", DMI_SYS_VENDOR },
{ "pn", DMI_PRODUCT_NAME },
{ "pvr", DMI_PRODUCT_VERSION },
@@ -187,6 +191,8 @@ static void __init dmi_id_init_attr_table(void)
ADD_DMI_ATTR(bios_vendor, DMI_BIOS_VENDOR);
ADD_DMI_ATTR(bios_version, DMI_BIOS_VERSION);
ADD_DMI_ATTR(bios_date, DMI_BIOS_DATE);
+ ADD_DMI_ATTR(bios_release_major, DMI_BIOS_MAJOR_RELEASE);
+ ADD_DMI_ATTR(bios_release_minor, DMI_BIOS_MINOR_RELEASE);
ADD_DMI_ATTR(sys_vendor, DMI_SYS_VENDOR);
ADD_DMI_ATTR(product_name, DMI_PRODUCT_NAME);
ADD_DMI_ATTR(product_version, DMI_PRODUCT_VERSION);
diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
index 202bd2c69d0f..886ace54e527 100644
--- a/drivers/firmware/dmi_scan.c
+++ b/drivers/firmware/dmi_scan.c
@@ -464,6 +464,8 @@ static void __init dmi_decode(const struct dmi_header *dm, void *dummy)
dmi_save_ident(dm, DMI_BIOS_VENDOR, 4);
dmi_save_ident(dm, DMI_BIOS_VERSION, 5);
dmi_save_ident(dm, DMI_BIOS_DATE, 8);
+ dmi_save_release(dm, DMI_BIOS_MAJOR_RELEASE, 20);
+ dmi_save_release(dm, DMI_BIOS_MINOR_RELEASE, 21);
break;
case 1: /* System Information */
dmi_save_ident(dm, DMI_SYS_VENDOR, 4);
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 5714fd35a83c..2471de601bd6 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -532,6 +532,8 @@ enum dmi_field {
DMI_BIOS_VENDOR,
DMI_BIOS_VERSION,
DMI_BIOS_DATE,
+ DMI_BIOS_MAJOR_RELEASE,
+ DMI_BIOS_MINOR_RELEASE,
DMI_SYS_VENDOR,
DMI_PRODUCT_NAME,
DMI_PRODUCT_VERSION,
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index e17a29ae2e97..1b4f9bc3b06c 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -931,6 +931,8 @@ static const struct dmifield {
{ "bvn", DMI_BIOS_VENDOR },
{ "bvr", DMI_BIOS_VERSION },
{ "bd", DMI_BIOS_DATE },
+ { "bjr", DMI_BIOS_MAJOR_RELEASE },
+ { "bmr", DMI_BIOS_MINOR_RELEASE },
{ "svn", DMI_SYS_VENDOR },
{ "pn", DMI_PRODUCT_NAME },
{ "pvr", DMI_PRODUCT_VERSION },
--
2.21.0
Servers that have a BMC encodes the release version of their firmware
in the "Embedded Controller Firmware {Major|Minor} Release" fields of Type 0.
This information is useful to know which release of the BMC is actually running.
It could be used for some quirks, debugging sessions or inventory tasks.
This patch extract these 2 fields in DMI_EMBEDDED_FW_MAJOR_RELEASE & DMI_EMBEDDED_FW_MINOR_RELEASE
A typical output for a Dell system running the 3.75 bios is :
[root@t1700 ~]# cat /sys/devices/virtual/dmi/id/fw_release_major
3
[root@t1700 ~]# cat /sys/devices/virtual/dmi/id/fw_release_minor
75
[root@t1700 ~]#
Signed-off-by: Erwan Velu <[email protected]>
---
drivers/firmware/dmi-id.c | 10 ++++++++--
drivers/firmware/dmi_scan.c | 2 ++
include/linux/mod_devicetable.h | 2 ++
scripts/mod/file2alias.c | 2 ++
4 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/dmi-id.c b/drivers/firmware/dmi-id.c
index 3248c2837a4d..5262626bf9f1 100644
--- a/drivers/firmware/dmi-id.c
+++ b/drivers/firmware/dmi-id.c
@@ -42,8 +42,10 @@ DEFINE_DMI_ATTR_WITH_SHOW(bios_vendor, 0444, DMI_BIOS_VENDOR);
DEFINE_DMI_ATTR_WITH_SHOW(bios_version, 0444, DMI_BIOS_VERSION);
DEFINE_DMI_ATTR_WITH_SHOW(bios_date, 0444, DMI_BIOS_DATE);
DEFINE_DMI_ATTR_WITH_SHOW(sys_vendor, 0444, DMI_SYS_VENDOR);
-DEFINE_DMI_ATTR_WITH_SHOW(bios_release_major, 0444, DMI_BIOS_MAJOR_RELEASE);
-DEFINE_DMI_ATTR_WITH_SHOW(bios_release_minor, 0444, DMI_BIOS_MINOR_RELEASE);
+DEFINE_DMI_ATTR_WITH_SHOW(bios_release_major, 0444, DMI_BIOS_MAJOR_RELEASE);
+DEFINE_DMI_ATTR_WITH_SHOW(bios_release_minor, 0444, DMI_BIOS_MINOR_RELEASE);
+DEFINE_DMI_ATTR_WITH_SHOW(fw_release_major, 0444, DMI_EMBEDDED_FW_MAJOR_RELEASE);
+DEFINE_DMI_ATTR_WITH_SHOW(fw_release_minor, 0444, DMI_EMBEDDED_FW_MINOR_RELEASE);
DEFINE_DMI_ATTR_WITH_SHOW(product_name, 0444, DMI_PRODUCT_NAME);
DEFINE_DMI_ATTR_WITH_SHOW(product_version, 0444, DMI_PRODUCT_VERSION);
DEFINE_DMI_ATTR_WITH_SHOW(product_serial, 0400, DMI_PRODUCT_SERIAL);
@@ -82,6 +84,8 @@ static ssize_t get_modalias(char *buffer, size_t buffer_size)
{ "bd", DMI_BIOS_DATE },
{ "bjr", DMI_BIOS_MAJOR_RELEASE },
{ "bmr", DMI_BIOS_MINOR_RELEASE },
+ { "efj", DMI_EMBEDDED_FW_MAJOR_RELEASE },
+ { "efm", DMI_EMBEDDED_FW_MINOR_RELEASE },
{ "svn", DMI_SYS_VENDOR },
{ "pn", DMI_PRODUCT_NAME },
{ "pvr", DMI_PRODUCT_VERSION },
@@ -193,6 +197,8 @@ static void __init dmi_id_init_attr_table(void)
ADD_DMI_ATTR(bios_date, DMI_BIOS_DATE);
ADD_DMI_ATTR(bios_release_major, DMI_BIOS_MAJOR_RELEASE);
ADD_DMI_ATTR(bios_release_minor, DMI_BIOS_MINOR_RELEASE);
+ ADD_DMI_ATTR(fw_release_major, DMI_EMBEDDED_FW_MAJOR_RELEASE);
+ ADD_DMI_ATTR(fw_release_minor, DMI_EMBEDDED_FW_MINOR_RELEASE);
ADD_DMI_ATTR(sys_vendor, DMI_SYS_VENDOR);
ADD_DMI_ATTR(product_name, DMI_PRODUCT_NAME);
ADD_DMI_ATTR(product_version, DMI_PRODUCT_VERSION);
diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
index 886ace54e527..3beec6896a58 100644
--- a/drivers/firmware/dmi_scan.c
+++ b/drivers/firmware/dmi_scan.c
@@ -466,6 +466,8 @@ static void __init dmi_decode(const struct dmi_header *dm, void *dummy)
dmi_save_ident(dm, DMI_BIOS_DATE, 8);
dmi_save_release(dm, DMI_BIOS_MAJOR_RELEASE, 20);
dmi_save_release(dm, DMI_BIOS_MINOR_RELEASE, 21);
+ dmi_save_release(dm, DMI_EMBEDDED_FW_MAJOR_RELEASE, 22);
+ dmi_save_release(dm, DMI_EMBEDDED_FW_MINOR_RELEASE, 23);
break;
case 1: /* System Information */
dmi_save_ident(dm, DMI_SYS_VENDOR, 4);
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 2471de601bd6..e6482fd94bfd 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -534,6 +534,8 @@ enum dmi_field {
DMI_BIOS_DATE,
DMI_BIOS_MAJOR_RELEASE,
DMI_BIOS_MINOR_RELEASE,
+ DMI_EMBEDDED_FW_MAJOR_RELEASE,
+ DMI_EMBEDDED_FW_MINOR_RELEASE,
DMI_SYS_VENDOR,
DMI_PRODUCT_NAME,
DMI_PRODUCT_VERSION,
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index 1b4f9bc3b06c..ce03040271cd 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -933,6 +933,8 @@ static const struct dmifield {
{ "bd", DMI_BIOS_DATE },
{ "bjr", DMI_BIOS_MAJOR_RELEASE },
{ "bmr", DMI_BIOS_MINOR_RELEASE },
+ { "efj", DMI_EMBEDDED_FW_MAJOR_RELEASE },
+ { "efm", DMI_EMBEDDED_FW_MINOR_RELEASE },
{ "svn", DMI_SYS_VENDOR },
{ "pn", DMI_PRODUCT_NAME },
{ "pvr", DMI_PRODUCT_VERSION },
--
2.21.0
Hi Erwan,
Sorry for the late answer.
On Wed, 18 Sep 2019 11:43:19 +0200, Erwan Velu wrote:
> In DMI type 0, there is several fields that encodes a release.
is -> are
encodes -> encode
> The dmi_save_release() function have the logic to check if the field is valid.
> If so, it reports the actual value.
>
> Signed-off-by: Erwan Velu <[email protected]>
> ---
> drivers/firmware/dmi_scan.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
This patch introduces a warning:
drivers/firmware/dmi_scan.c:185:20: warning: ‘dmi_save_release’ defined but not used [-Wunused-function]
static void __init dmi_save_release(const struct dmi_header *dm, int slot,
^~~~~~~~~~~~~~~~
because you add a static function with no user. I understand that you
add a use later in the series, but it's not OK to introduce a warning
even if temporary. It also makes little sense to split the changes that
way as there is no way to cherry-pick one of the patches without the
rest. And it makes things more difficult to review too, as I can't
possibly judge if this function if right without also seeing where is
will be called and how.
So, please merge all the changes into a single patch.
> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
> index 35ed56b9c34f..202bd2c69d0f 100644
> --- a/drivers/firmware/dmi_scan.c
> +++ b/drivers/firmware/dmi_scan.c
> @@ -181,6 +181,32 @@ static void __init dmi_save_ident(const struct dmi_header *dm, int slot,
> dmi_ident[slot] = p;
> }
>
> +static void __init dmi_save_release(const struct dmi_header *dm, int slot,
> + int index)
> +{
> + const u8 *d;
> + char *s;
> +
> + // If the table doesn't have the field, let's return
Please stick to C89-style comments (/* */) as used everywhere else in
this file.
> + if (dmi_ident[slot] || dm->length < index)
> + return;
> +
> + d = (u8 *) dm + index;
> +
> + // As per the specification,
> + // if the system doesn't have the field, the value is FF
> + if (d[0] == 0xFF)
> + return;
That's not exactly what the specification says. It says:
"If the system does not support the use of [the System BIOS Major
Release] field, the value is 0FFh for both this field and the System
BIOS Minor Release field." So unused is when both fields are 0xFF. You
can't test them independently.
Same goes for the Embedded Controller Firmware Release fields, even if
it is worded differently, the logic is the same.
> +
> + s = dmi_alloc(4);
> + if (!s)
> + return;
> +
> + sprintf(s, "%u", d[0]);
> +
> + dmi_ident[slot] = s;
> +}
> +
> static void __init dmi_save_uuid(const struct dmi_header *dm, int slot,
> int index)
> {
--
Jean Delvare
SUSE L3 Support
On Wed, 18 Sep 2019 11:43:20 +0200, Erwan Velu wrote:
> Some vendors like HPe or Dell, encodes the release version of their BIOS
encodes -> encode
> in the "System BIOS {Major|Minor} Release" fields of Type 0.
>
> This information is useful to know which release of the bios is actually running.
> It could be used for some quirks, debugging sessions or inventory tasks.
>
> This patch extract these 2 fields in DMI_BIOS_MAJOR_RELEASE & DMI_BIOS_MINOR_RELEASE.
>
> A typical output for a Dell system running the 65.27 bios is :
>
> [root@t1700 ~]# cat /sys/devices/virtual/dmi/id/bios_release_major
> 65
> [root@t1700 ~]# cat /sys/devices/virtual/dmi/id/bios_release_minor
> 27
> [root@t1700 ~]#
I don't think we want two fields. This adds quite some overhead, and
they are not independent from each other anyway. I'd rather have one
field with the values combined:
[root@t1700 ~]# cat /sys/devices/virtual/dmi/id/bios_release
65.27
[root@t1700 ~]#
This would also be in line with how it was implemented in dmidecode. Is
there any reason to NOT go that route?
--
Jean Delvare
SUSE L3 Support
On Wed, 18 Sep 2019 11:43:21 +0200, Erwan Velu wrote:
> Servers that have a BMC encodes the release version of their firmware
> in the "Embedded Controller Firmware {Major|Minor} Release" fields of Type 0.
>
> This information is useful to know which release of the BMC is actually running.
> It could be used for some quirks, debugging sessions or inventory tasks.
>
> This patch extract these 2 fields in DMI_EMBEDDED_FW_MAJOR_RELEASE & DMI_EMBEDDED_FW_MINOR_RELEASE
>
> A typical output for a Dell system running the 3.75 bios is :
>
> [root@t1700 ~]# cat /sys/devices/virtual/dmi/id/fw_release_major
> 3
> [root@t1700 ~]# cat /sys/devices/virtual/dmi/id/fw_release_minor
> 75
> [root@t1700 ~]#
Same comment here as for previous patch, obviously.
Additionally, please run scripts/checkpatch.pl on your patch before you
resubmit, and address all the problems reported.
Thanks,
--
Jean Delvare
SUSE L3 Support
Got all your points, sending a V2 with the fixes you asked.
On 21/10/2019 16:32, Jean Delvare wrote:
> Hi Erwan,
>
> Sorry for the late answer.
>
> On Wed, 18 Sep 2019 11:43:19 +0200, Erwan Velu wrote:
>> In DMI type 0, there is several fields that encodes a release.
> is -> are
> encodes -> encode
>
>> The dmi_save_release() function have the logic to check if the field is valid.
>> If so, it reports the actual value.
>>
>> Signed-off-by: Erwan Velu <[email protected]>
>> ---
>> drivers/firmware/dmi_scan.c | 26 ++++++++++++++++++++++++++
>> 1 file changed, 26 insertions(+)
> This patch introduces a warning:
>
> drivers/firmware/dmi_scan.c:185:20: warning: ‘dmi_save_release’ defined but not used [-Wunused-function]
> static void __init dmi_save_release(const struct dmi_header *dm, int slot,
> ^~~~~~~~~~~~~~~~
>
> because you add a static function with no user. I understand that you
> add a use later in the series, but it's not OK to introduce a warning
> even if temporary. It also makes little sense to split the changes that
> way as there is no way to cherry-pick one of the patches without the
> rest. And it makes things more difficult to review too, as I can't
> possibly judge if this function if right without also seeing where is
> will be called and how.
>
> So, please merge all the changes into a single patch.
>
>> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
>> index 35ed56b9c34f..202bd2c69d0f 100644
>> --- a/drivers/firmware/dmi_scan.c
>> +++ b/drivers/firmware/dmi_scan.c
>> @@ -181,6 +181,32 @@ static void __init dmi_save_ident(const struct dmi_header *dm, int slot,
>> dmi_ident[slot] = p;
>> }
>>
>> +static void __init dmi_save_release(const struct dmi_header *dm, int slot,
>> + int index)
>> +{
>> + const u8 *d;
>> + char *s;
>> +
>> + // If the table doesn't have the field, let's return
> Please stick to C89-style comments (/* */) as used everywhere else in
> this file.
>
>> + if (dmi_ident[slot] || dm->length < index)
>> + return;
>> +
>> + d = (u8 *) dm + index;
>> +
>> + // As per the specification,
>> + // if the system doesn't have the field, the value is FF
>> + if (d[0] == 0xFF)
>> + return;
> That's not exactly what the specification says. It says:
>
> "If the system does not support the use of [the System BIOS Major
> Release] field, the value is 0FFh for both this field and the System
> BIOS Minor Release field." So unused is when both fields are 0xFF. You
> can't test them independently.
>
> Same goes for the Embedded Controller Firmware Release fields, even if
> it is worded differently, the logic is the same.
>
>> +
>> + s = dmi_alloc(4);
>> + if (!s)
>> + return;
>> +
>> + sprintf(s, "%u", d[0]);
>> +
>> + dmi_ident[slot] = s;
>> +}
>> +
>> static void __init dmi_save_uuid(const struct dmi_header *dm, int slot,
>> int index)
>> {
>
On 21/10/2019 16:53, Jean Delvare wrote:
> This would also be in line with how it was implemented in dmidecode. Is
> there any reason to NOT go that route?
The main reason was I though it would be easier to compare releases by
splitting major & minor.
That is probably overkill, so I'll stick to the usual way to represent a
release in V2.
Thanks for the review,
Erwan,
Some vendors like HPe or Dell, encode the release version of their BIOS
in the "System BIOS {Major|Minor} Release" fields of Type 0.
This information is used to know which bios release actually runs.
It could be used for some quirks, debugging sessions or inventory tasks.
A typical output for a Dell system running the 65.27 bios is :
[root@t1700 ~]# cat /sys/devices/virtual/dmi/id/bios_release
65.27
[root@t1700 ~]#
This commit add dmi_save_release() function have the logic to
check if the field is valid. If so, it reports the actual value.
Signed-off-by: Erwan Velu <[email protected]>
---
drivers/firmware/dmi-id.c | 3 +++
drivers/firmware/dmi_scan.c | 29 +++++++++++++++++++++++++++++
include/linux/mod_devicetable.h | 1 +
scripts/mod/file2alias.c | 1 +
4 files changed, 34 insertions(+)
diff --git a/drivers/firmware/dmi-id.c b/drivers/firmware/dmi-id.c
index ff39f64f2aae..a2aac65ff771 100644
--- a/drivers/firmware/dmi-id.c
+++ b/drivers/firmware/dmi-id.c
@@ -42,6 +42,7 @@ DEFINE_DMI_ATTR_WITH_SHOW(bios_vendor, 0444, DMI_BIOS_VENDOR);
DEFINE_DMI_ATTR_WITH_SHOW(bios_version, 0444, DMI_BIOS_VERSION);
DEFINE_DMI_ATTR_WITH_SHOW(bios_date, 0444, DMI_BIOS_DATE);
DEFINE_DMI_ATTR_WITH_SHOW(sys_vendor, 0444, DMI_SYS_VENDOR);
+DEFINE_DMI_ATTR_WITH_SHOW(bios_release, 0444, DMI_BIOS_RELEASE);
DEFINE_DMI_ATTR_WITH_SHOW(product_name, 0444, DMI_PRODUCT_NAME);
DEFINE_DMI_ATTR_WITH_SHOW(product_version, 0444, DMI_PRODUCT_VERSION);
DEFINE_DMI_ATTR_WITH_SHOW(product_serial, 0400, DMI_PRODUCT_SERIAL);
@@ -78,6 +79,7 @@ static ssize_t get_modalias(char *buffer, size_t buffer_size)
{ "bvn", DMI_BIOS_VENDOR },
{ "bvr", DMI_BIOS_VERSION },
{ "bd", DMI_BIOS_DATE },
+ { "br", DMI_BIOS_RELEASE },
{ "svn", DMI_SYS_VENDOR },
{ "pn", DMI_PRODUCT_NAME },
{ "pvr", DMI_PRODUCT_VERSION },
@@ -187,6 +189,7 @@ static void __init dmi_id_init_attr_table(void)
ADD_DMI_ATTR(bios_vendor, DMI_BIOS_VENDOR);
ADD_DMI_ATTR(bios_version, DMI_BIOS_VERSION);
ADD_DMI_ATTR(bios_date, DMI_BIOS_DATE);
+ ADD_DMI_ATTR(bios_release, DMI_BIOS_RELEASE);
ADD_DMI_ATTR(sys_vendor, DMI_SYS_VENDOR);
ADD_DMI_ATTR(product_name, DMI_PRODUCT_NAME);
ADD_DMI_ATTR(product_version, DMI_PRODUCT_VERSION);
diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
index 1e21fc3e9851..d010c915c1ab 100644
--- a/drivers/firmware/dmi_scan.c
+++ b/drivers/firmware/dmi_scan.c
@@ -181,6 +181,34 @@ static void __init dmi_save_ident(const struct dmi_header *dm, int slot,
dmi_ident[slot] = p;
}
+static void __init dmi_save_release(const struct dmi_header *dm, int slot,
+ int index)
+{
+ const u8 *minor, *major;
+ char *s;
+
+ /* If the table doesn't have the field, let's return */
+ if (dmi_ident[slot] || dm->length < index)
+ return;
+
+ minor = (u8 *) dm + index;
+ major = (u8 *) dm + index - 1;
+
+ /* As per the spec, if the system doesn't support this field,
+ * the value is FF
+ */
+ if (major[0] == 0xFF && minor[0] == 0xFF)
+ return;
+
+ s = dmi_alloc(4);
+ if (!s)
+ return;
+
+ sprintf(s, "%u.%u", major[0], minor[0]);
+
+ dmi_ident[slot] = s;
+}
+
static void __init dmi_save_uuid(const struct dmi_header *dm, int slot,
int index)
{
@@ -438,6 +466,7 @@ static void __init dmi_decode(const struct dmi_header *dm, void *dummy)
dmi_save_ident(dm, DMI_BIOS_VENDOR, 4);
dmi_save_ident(dm, DMI_BIOS_VERSION, 5);
dmi_save_ident(dm, DMI_BIOS_DATE, 8);
+ dmi_save_release(dm, DMI_BIOS_RELEASE, 21);
break;
case 1: /* System Information */
dmi_save_ident(dm, DMI_SYS_VENDOR, 4);
diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
index 5714fd35a83c..618933d770e6 100644
--- a/include/linux/mod_devicetable.h
+++ b/include/linux/mod_devicetable.h
@@ -532,6 +532,7 @@ enum dmi_field {
DMI_BIOS_VENDOR,
DMI_BIOS_VERSION,
DMI_BIOS_DATE,
+ DMI_BIOS_RELEASE,
DMI_SYS_VENDOR,
DMI_PRODUCT_NAME,
DMI_PRODUCT_VERSION,
diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
index c91eba751804..cc48930cc02a 100644
--- a/scripts/mod/file2alias.c
+++ b/scripts/mod/file2alias.c
@@ -936,6 +936,7 @@ static const struct dmifield {
{ "bvn", DMI_BIOS_VENDOR },
{ "bvr", DMI_BIOS_VERSION },
{ "bd", DMI_BIOS_DATE },
+ { "br", DMI_BIOS_RELEASE },
{ "svn", DMI_SYS_VENDOR },
{ "pn", DMI_PRODUCT_NAME },
{ "pvr", DMI_PRODUCT_VERSION },
--
2.23.0
Hi Erwan,
Once again, sorry for the late answer.
On Wed, 27 Nov 2019 16:07:25 +0100, Erwan Velu wrote:
> Some vendors like HPe or Dell, encode the release version of their BIOS
> in the "System BIOS {Major|Minor} Release" fields of Type 0.
>
> This information is used to know which bios release actually runs.
> It could be used for some quirks, debugging sessions or inventory tasks.
>
> A typical output for a Dell system running the 65.27 bios is :
>
> [root@t1700 ~]# cat /sys/devices/virtual/dmi/id/bios_release
> 65.27
> [root@t1700 ~]#
>
> This commit add dmi_save_release() function have the logic to
> check if the field is valid. If so, it reports the actual value.
>
> Signed-off-by: Erwan Velu <[email protected]>
> ---
> drivers/firmware/dmi-id.c | 3 +++
> drivers/firmware/dmi_scan.c | 29 +++++++++++++++++++++++++++++
> include/linux/mod_devicetable.h | 1 +
> scripts/mod/file2alias.c | 1 +
> 4 files changed, 34 insertions(+)
>
> diff --git a/drivers/firmware/dmi-id.c b/drivers/firmware/dmi-id.c
> index ff39f64f2aae..a2aac65ff771 100644
> --- a/drivers/firmware/dmi-id.c
> +++ b/drivers/firmware/dmi-id.c
> @@ -42,6 +42,7 @@ DEFINE_DMI_ATTR_WITH_SHOW(bios_vendor, 0444, DMI_BIOS_VENDOR);
> DEFINE_DMI_ATTR_WITH_SHOW(bios_version, 0444, DMI_BIOS_VERSION);
> DEFINE_DMI_ATTR_WITH_SHOW(bios_date, 0444, DMI_BIOS_DATE);
> DEFINE_DMI_ATTR_WITH_SHOW(sys_vendor, 0444, DMI_SYS_VENDOR);
> +DEFINE_DMI_ATTR_WITH_SHOW(bios_release, 0444, DMI_BIOS_RELEASE);
> DEFINE_DMI_ATTR_WITH_SHOW(product_name, 0444, DMI_PRODUCT_NAME);
> DEFINE_DMI_ATTR_WITH_SHOW(product_version, 0444, DMI_PRODUCT_VERSION);
> DEFINE_DMI_ATTR_WITH_SHOW(product_serial, 0400, DMI_PRODUCT_SERIAL);
> @@ -78,6 +79,7 @@ static ssize_t get_modalias(char *buffer, size_t buffer_size)
> { "bvn", DMI_BIOS_VENDOR },
> { "bvr", DMI_BIOS_VERSION },
> { "bd", DMI_BIOS_DATE },
> + { "br", DMI_BIOS_RELEASE },
> { "svn", DMI_SYS_VENDOR },
> { "pn", DMI_PRODUCT_NAME },
> { "pvr", DMI_PRODUCT_VERSION },
> @@ -187,6 +189,7 @@ static void __init dmi_id_init_attr_table(void)
> ADD_DMI_ATTR(bios_vendor, DMI_BIOS_VENDOR);
> ADD_DMI_ATTR(bios_version, DMI_BIOS_VERSION);
> ADD_DMI_ATTR(bios_date, DMI_BIOS_DATE);
> + ADD_DMI_ATTR(bios_release, DMI_BIOS_RELEASE);
> ADD_DMI_ATTR(sys_vendor, DMI_SYS_VENDOR);
> ADD_DMI_ATTR(product_name, DMI_PRODUCT_NAME);
> ADD_DMI_ATTR(product_version, DMI_PRODUCT_VERSION);
> diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c
> index 1e21fc3e9851..d010c915c1ab 100644
> --- a/drivers/firmware/dmi_scan.c
> +++ b/drivers/firmware/dmi_scan.c
> @@ -181,6 +181,34 @@ static void __init dmi_save_ident(const struct dmi_header *dm, int slot,
> dmi_ident[slot] = p;
> }
>
> +static void __init dmi_save_release(const struct dmi_header *dm, int slot,
> + int index)
> +{
> + const u8 *minor, *major;
> + char *s;
> +
> + /* If the table doesn't have the field, let's return */
> + if (dmi_ident[slot] || dm->length < index)
> + return;
> +
> + minor = (u8 *) dm + index;
> + major = (u8 *) dm + index - 1;
> +
> + /* As per the spec, if the system doesn't support this field,
> + * the value is FF
> + */
> + if (major[0] == 0xFF && minor[0] == 0xFF)
When using a pointer to a single entity, the common practice is to use
*major rather than major[0].
> + return;
> +
> + s = dmi_alloc(4);
4 bytes (3 + 1) were enough when you encoded a single byte. Now that you
encode 2 bytes separates by a dot, you need 8 (3 + 1 + 3 + 1).
> + if (!s)
> + return;
> +
> + sprintf(s, "%u.%u", major[0], minor[0]);
Here too, *major would be preferred.
> +
> + dmi_ident[slot] = s;
> +}
> +
> static void __init dmi_save_uuid(const struct dmi_header *dm, int slot,
> int index)
> {
> @@ -438,6 +466,7 @@ static void __init dmi_decode(const struct dmi_header *dm, void *dummy)
> dmi_save_ident(dm, DMI_BIOS_VENDOR, 4);
> dmi_save_ident(dm, DMI_BIOS_VERSION, 5);
> dmi_save_ident(dm, DMI_BIOS_DATE, 8);
> + dmi_save_release(dm, DMI_BIOS_RELEASE, 21);
> break;
> case 1: /* System Information */
> dmi_save_ident(dm, DMI_SYS_VENDOR, 4);
> diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h
> index 5714fd35a83c..618933d770e6 100644
> --- a/include/linux/mod_devicetable.h
> +++ b/include/linux/mod_devicetable.h
> @@ -532,6 +532,7 @@ enum dmi_field {
> DMI_BIOS_VENDOR,
> DMI_BIOS_VERSION,
> DMI_BIOS_DATE,
> + DMI_BIOS_RELEASE,
> DMI_SYS_VENDOR,
> DMI_PRODUCT_NAME,
> DMI_PRODUCT_VERSION,
> diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c
> index c91eba751804..cc48930cc02a 100644
> --- a/scripts/mod/file2alias.c
> +++ b/scripts/mod/file2alias.c
> @@ -936,6 +936,7 @@ static const struct dmifield {
> { "bvn", DMI_BIOS_VENDOR },
> { "bvr", DMI_BIOS_VERSION },
> { "bd", DMI_BIOS_DATE },
> + { "br", DMI_BIOS_RELEASE },
> { "svn", DMI_SYS_VENDOR },
> { "pn", DMI_PRODUCT_NAME },
> { "pvr", DMI_PRODUCT_VERSION },
--
Jean Delvare
SUSE L3 Support