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

[PATCH] 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>
---
 drivers/firmware/efi/efi.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--- linux-4.1-rc1.orig/drivers/firmware/efi/efi.c	2015-04-27 02:59:10.000000000 +0200
+++ linux-4.1-rc1/drivers/firmware/efi/efi.c	2015-04-28 09:19:20.910416344 +0200
@@ -85,10 +85,10 @@ 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 (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] efi: dmi: List SMBIOS3 table before SMBIOS table

[Matt Fleming]

On Tue, 28 Apr, at 09:30:12AM, 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.
 
Now we need to ask the reverse question: Do any userspace utils use the
last SMBIOS table entry as the preferred one? I suspect not because
SMBIOS3 is so new but we need to highlight the risk of breaking
functionality since this file forms an ABI.

Anyone aware of such a tool?

Also Jean, this patch could do with a new comment describing the
importance of the table entry order so we don't break it in the future.

> 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>
> ---
>  drivers/firmware/efi/efi.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> --- linux-4.1-rc1.orig/drivers/firmware/efi/efi.c	2015-04-27 02:59:10.000000000 +0200
> +++ linux-4.1-rc1/drivers/firmware/efi/efi.c	2015-04-28 09:19:20.910416344 +0200
> @@ -85,10 +85,10 @@ 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 (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
> --
> To unsubscribe from this list: send the line "unsubscribe linux-efi" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Matt Fleming, Intel Open Source Technology Center

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

[Jean Delvare]

Hi Matt,

Le Thursday 30 April 2015 à 11:30 +0100, Matt Fleming a écrit :
> On Tue, 28 Apr, at 09:30:12AM, 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.
>  
> Now we need to ask the reverse question: Do any userspace utils use the
> last SMBIOS table entry as the preferred one? I suspect not because
> SMBIOS3 is so new but we need to highlight the risk of breaking
> functionality since this file forms an ABI.

I'd be surprised. Either you stop parsing after the first match, or you
parse the whole file and pick what you prefer. Parsing the whole file
and then arbitrarily choosing the last match wouldn't make much sense.

> Anyone aware of such a tool?

I am not, but then again the only tool I am familiar with which parses
this file is dmidecode. Developers more familiar with EFI-based systems
will know better.

> Also Jean, this patch could do with a new comment describing the
> importance of the table entry order so we don't break it in the future.

Good idea, thanks for the suggestion. Version 2 of the patch is coming.

> 
> > 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>
> > ---
> >  drivers/firmware/efi/efi.c |    4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > --- linux-4.1-rc1.orig/drivers/firmware/efi/efi.c	2015-04-27 02:59:10.000000000 +0200
> > +++ linux-4.1-rc1/drivers/firmware/efi/efi.c	2015-04-28 09:19:20.910416344 +0200
> > @@ -85,10 +85,10 @@ 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 (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