[PATCH v2] efi: dmi: List SMBIOS3 table before SMBIOS table

[PATCH v2] efi: dmi: List SMBIOS3 table before SMBIOS table

[Jean Delvare]

The SMBIOS3 table should appear before the SMBIOS table in
/sys/firmware/efi/systab. This allows user-space utilities which
support both to pick the SMBIOS3 table with a single pass on systems
where both are implemented. The SMBIOS3 entry point is more capable
than the SMBIOS entry point so it should be preferred.

This follows the same logic as the ACPI20 table being listed before
the ACPI table.

Signed-off-by: Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org>
Cc: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Leif Lindholm <leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
Changes since v1:
 * Added a comment explaining why SMBIOS3 must be listed before SMBIOS,
   so that it doesn't get accidentally broken in the future.

 drivers/firmware/efi/efi.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

--- linux-4.1-rc1.orig/drivers/firmware/efi/efi.c	2015-04-28 16:31:46.614397242 +0200
+++ linux-4.1-rc1/drivers/firmware/efi/efi.c	2015-04-30 15:15:46.414862079 +0200
@@ -85,10 +85,15 @@ static ssize_t systab_show(struct kobjec
 		str += sprintf(str, "ACPI20=0x%lx\n", efi.acpi20);
 	if (efi.acpi != EFI_INVALID_TABLE_ADDR)
 		str += sprintf(str, "ACPI=0x%lx\n", efi.acpi);
-	if (efi.smbios != EFI_INVALID_TABLE_ADDR)
-		str += sprintf(str, "SMBIOS=0x%lx\n", efi.smbios);
+	/*
+	 * If both SMBIOS and SMBIOS3 entry points are implemented, the
+	 * SMBIOS3 entry point shall be preferred, so we list it first to
+	 * let applications stop parsing after the first match.
+	 */
 	if (efi.smbios3 != EFI_INVALID_TABLE_ADDR)
 		str += sprintf(str, "SMBIOS3=0x%lx\n", efi.smbios3);
+	if (efi.smbios != EFI_INVALID_TABLE_ADDR)
+		str += sprintf(str, "SMBIOS=0x%lx\n", efi.smbios);
 	if (efi.hcdp != EFI_INVALID_TABLE_ADDR)
 		str += sprintf(str, "HCDP=0x%lx\n", efi.hcdp);
 	if (efi.boot_info != EFI_INVALID_TABLE_ADDR)


-- 
Jean Delvare
SUSE L3 Support

Re: [PATCH v2] efi: dmi: List SMBIOS3 table before SMBIOS table

[Matt Fleming]

On Thu, 30 Apr, at 03:23:05PM, Jean Delvare wrote:
> The SMBIOS3 table should appear before the SMBIOS table in
> /sys/firmware/efi/systab. This allows user-space utilities which
> support both to pick the SMBIOS3 table with a single pass on systems
> where both are implemented. The SMBIOS3 entry point is more capable
> than the SMBIOS entry point so it should be preferred.
> 
> This follows the same logic as the ACPI20 table being listed before
> the ACPI table.
> 
> Signed-off-by: Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org>
> Cc: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Leif Lindholm <leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
> Changes since v1:
>  * Added a comment explaining why SMBIOS3 must be listed before SMBIOS,
>    so that it doesn't get accidentally broken in the future.
> 
>  drivers/firmware/efi/efi.c |    9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)

Thanks Jean. This looks OK to me but I'd really like to see some ACKs
from Leif, Ard or Ivan.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH v2] efi: dmi: List SMBIOS3 table before SMBIOS table

[Ard Biesheuvel]

On 30 April 2015 at 18:04, Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org> wrote:
> On Thu, 30 Apr, at 03:23:05PM, Jean Delvare wrote:
>> The SMBIOS3 table should appear before the SMBIOS table in
>> /sys/firmware/efi/systab. This allows user-space utilities which
>> support both to pick the SMBIOS3 table with a single pass on systems
>> where both are implemented. The SMBIOS3 entry point is more capable
>> than the SMBIOS entry point so it should be preferred.
>>
>> This follows the same logic as the ACPI20 table being listed before
>> the ACPI table.
>>
>> Signed-off-by: Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org>
>> Cc: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> Cc: Leif Lindholm <leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> Cc: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> ---
>> Changes since v1:
>>  * Added a comment explaining why SMBIOS3 must be listed before SMBIOS,
>>    so that it doesn't get accidentally broken in the future.
>>
>>  drivers/firmware/efi/efi.c |    9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> Thanks Jean. This looks OK to me but I'd really like to see some ACKs
> from Leif, Ard or Ivan.
>

I don't have any objections to this patch per se, but I am not crazy
about the fact that the way the output is ordered is kernel<->userland
ABI now
But since there appears to be a precedent with ACPI20 before ACPI,
that was already the case, apparently.
So,

Acked-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Re: [PATCH v2] efi: dmi: List SMBIOS3 table before SMBIOS table

[Matt Fleming]

On Thu, 30 Apr, at 06:10:48PM, Ard Biesheuvel wrote:
> 
> I don't have any objections to this patch per se, but I am not crazy
> about the fact that the way the output is ordered is kernel<->userland
> ABI now
> But since there appears to be a precedent with ACPI20 before ACPI,
> that was already the case, apparently.
> So,
> 
> Acked-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

Thanks Ard.

I've thought about this for a long while and we always kinda treat the
results of sysfs files as ABI anyway. If SMBIOS 3 wasn't so new I
wouldn't entertain doing this, but I think it's OK in this situation.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH v2] efi: dmi: List SMBIOS3 table before SMBIOS table

[Ard Biesheuvel]

On 26 May 2015 at 20:11, Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org> wrote:
> On Thu, 30 Apr, at 06:10:48PM, Ard Biesheuvel wrote:
>>
>> I don't have any objections to this patch per se, but I am not crazy
>> about the fact that the way the output is ordered is kernel<->userland
>> ABI now
>> But since there appears to be a precedent with ACPI20 before ACPI,
>> that was already the case, apparently.
>> So,
>>
>> Acked-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>
> Thanks Ard.
>
> I've thought about this for a long while and we always kinda treat the
> results of sysfs files as ABI anyway. If SMBIOS 3 wasn't so new I
> wouldn't entertain doing this, but I think it's OK in this situation.
>

OK. Then I guess we should at least document in
Documentation/ABI/stable/sysfs-firmware-efi-systab that the order in
which the entries are enumerated is part of the ABI.

Regards,
Ard.

Re: [PATCH v2] efi: dmi: List SMBIOS3 table before SMBIOS table

[Matt Fleming]

On Wed, 27 May, at 12:30:37PM, Ard Biesheuvel wrote:
> 
> OK. Then I guess we should at least document in
> Documentation/ABI/stable/sysfs-firmware-efi-systab that the order in
> which the entries are enumerated is part of the ABI.

Makes perfect sense to me. How about this?

---

>From ccb5f08bb71ae4199c75668c6beacd6b47fa186c Mon Sep 17 00:00:00 2001
From: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Date: Wed, 27 May 2015 14:14:22 +0100
Subject: [PATCH] efi: Add 'systab' information to Documentation/ABI

It's not clear right now that the order in which entries are displayed
in /sys/firmware/efi/systab actually forms an ABI that userspace tools
rely upon.

Document the ABI along with the userspace tool that cares.

Suggested-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org>
Cc: Leif Lindholm <leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: Ivan Khoronzhuk <ivan.khoronzhuk-hExfYMNmJl/Cnp4W7fqMDg@public.gmane.org>
Signed-off-by: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 Documentation/ABI/testing/sysfs-firmware-efi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-firmware-efi b/Documentation/ABI/testing/sysfs-firmware-efi
index 05874da7ce80..e794eac32a90 100644
--- a/Documentation/ABI/testing/sysfs-firmware-efi
+++ b/Documentation/ABI/testing/sysfs-firmware-efi
@@ -18,3 +18,13 @@ Contact:	Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
 Description:	It shows the physical address of config table entry in the EFI
 		system table.
 Users:		Kexec
+
+What:		/sys/firmware/efi/systab
+Date:		April 2005
+Contact:	linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
+Description:	Displays the physical addresses of all EFI Configuration
+		Tables found via the EFI System Table. The order in
+		which the tables are printed forms an ABI and newer
+		versions are always printed first, i.e. ACPI20 comes
+		before ACPI.
+Users:		dmidecode
-- 
2.1.0

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH v2] efi: dmi: List SMBIOS3 table before SMBIOS table

[Ard Biesheuvel]

On 27 May 2015 at 15:18, Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org> wrote:
> On Wed, 27 May, at 12:30:37PM, Ard Biesheuvel wrote:
>>
>> OK. Then I guess we should at least document in
>> Documentation/ABI/stable/sysfs-firmware-efi-systab that the order in
>> which the entries are enumerated is part of the ABI.
>
> Makes perfect sense to me. How about this?
>
> ---
>
> From ccb5f08bb71ae4199c75668c6beacd6b47fa186c Mon Sep 17 00:00:00 2001
> From: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Date: Wed, 27 May 2015 14:14:22 +0100
> Subject: [PATCH] efi: Add 'systab' information to Documentation/ABI
>
> It's not clear right now that the order in which entries are displayed
> in /sys/firmware/efi/systab actually forms an ABI that userspace tools
> rely upon.
>
> Document the ABI along with the userspace tool that cares.
>
> Suggested-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org>
> Cc: Leif Lindholm <leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Ivan Khoronzhuk <ivan.khoronzhuk-hExfYMNmJl/Cnp4W7fqMDg@public.gmane.org>
> Signed-off-by: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Still not crazy about it but here it goes :-)

Acked-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>


> ---
>  Documentation/ABI/testing/sysfs-firmware-efi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-firmware-efi b/Documentation/ABI/testing/sysfs-firmware-efi
> index 05874da7ce80..e794eac32a90 100644
> --- a/Documentation/ABI/testing/sysfs-firmware-efi
> +++ b/Documentation/ABI/testing/sysfs-firmware-efi
> @@ -18,3 +18,13 @@ Contact:     Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>  Description:   It shows the physical address of config table entry in the EFI
>                 system table.
>  Users:         Kexec
> +
> +What:          /sys/firmware/efi/systab
> +Date:          April 2005
> +Contact:       linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> +Description:   Displays the physical addresses of all EFI Configuration
> +               Tables found via the EFI System Table. The order in
> +               which the tables are printed forms an ABI and newer
> +               versions are always printed first, i.e. ACPI20 comes
> +               before ACPI.
> +Users:         dmidecode
> --
> 2.1.0
>
> --
> Matt Fleming, Intel Open Source Technology Center

Re: [PATCH v2] efi: dmi: List SMBIOS3 table before SMBIOS table

[Jean Delvare]

On Wed, 27 May 2015 14:18:57 +0100, Matt Fleming wrote:
> On Wed, 27 May, at 12:30:37PM, Ard Biesheuvel wrote:
> > 
> > OK. Then I guess we should at least document in
> > Documentation/ABI/stable/sysfs-firmware-efi-systab that the order in
> > which the entries are enumerated is part of the ABI.
> 
> Makes perfect sense to me. How about this?
> 
> ---
> 
> From ccb5f08bb71ae4199c75668c6beacd6b47fa186c Mon Sep 17 00:00:00 2001
> From: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Date: Wed, 27 May 2015 14:14:22 +0100
> Subject: [PATCH] efi: Add 'systab' information to Documentation/ABI
> 
> It's not clear right now that the order in which entries are displayed
> in /sys/firmware/efi/systab actually forms an ABI that userspace tools
> rely upon.
> 
> Document the ABI along with the userspace tool that cares.
> 
> Suggested-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org>
> Cc: Leif Lindholm <leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Ivan Khoronzhuk <ivan.khoronzhuk-hExfYMNmJl/Cnp4W7fqMDg@public.gmane.org>
> Signed-off-by: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
>  Documentation/ABI/testing/sysfs-firmware-efi | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-firmware-efi b/Documentation/ABI/testing/sysfs-firmware-efi
> index 05874da7ce80..e794eac32a90 100644
> --- a/Documentation/ABI/testing/sysfs-firmware-efi
> +++ b/Documentation/ABI/testing/sysfs-firmware-efi
> @@ -18,3 +18,13 @@ Contact:	Dave Young <dyoung-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>  Description:	It shows the physical address of config table entry in the EFI
>  		system table.
>  Users:		Kexec
> +
> +What:		/sys/firmware/efi/systab
> +Date:		April 2005
> +Contact:	linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> +Description:	Displays the physical addresses of all EFI Configuration
> +		Tables found via the EFI System Table. The order in
> +		which the tables are printed forms an ABI and newer
> +		versions are always printed first, i.e. ACPI20 comes
> +		before ACPI.
> +Users:		dmidecode

Reviewed-by: Jean Delvare <jdelvare-l3A5Bk7waGM@public.gmane.org>

Thank you,
-- 
Jean Delvare
SUSE L3 Support