[dmidecode] [patch] UEFI support on FreeBSD

[Jean Delvare]

Hi Alexey,

Currently, dmidecode(8) does not work on FreeBSD booted in UEFI mode.
Previously it was understandable, since there are no things like Linuxish
/proc/efi/systab or /sys/firmware/efi/systab to read from under FreeBSD.
https://svnweb.freebsd.org/base?view=revision&revision=307326
I've patched dmidecode.c to try to get the address from hint.smbios.0.mem
and fall back to traditional address space scanning. I've tested it both
on EFI (amd64 laptop) and non-EFI (i386 desktop) machines.
--- dmidecode.c.orig 2017-05-23 13:34:14 UTC
+++ dmidecode.c
@@ -58,6 +58,10 @@
* https://trustedcomputinggroup.org/pc-client-platform-tpm-profile-ptp-specification/
*/
+#ifdef __FreeBSD__
+#include <errno.h>
+#include <kenv.h>
+#endif

I think I'd prefer them after the common ones, and with a separating
blank line.

#include <stdio.h>
#include <string.h>
#include <strings.h>
@@ -4934,13 +4938,18 @@ static int legacy_decode(u8 *buf, const
#define EFI_NO_SMBIOS (-2)
static int address_from_efi(off_t *address)
{
+#if defined(__linux__)
FILE *efi_systab;
const char *filename;
char linebuf[64];
+#elif defined(__FreeBSD__)
+ char addrstr[KENV_MVALLEN + 1];
+#endif
int ret;
*address = 0; /* Prevent compiler warning */
+#if defined(__linux__)
/*
* Linux up to 2.6.6: /proc/efi/systab
* Linux 2.6.7 and up: /sys/firmware/efi/systab
@@ -4973,6 +4982,25 @@ static int address_from_efi(off_t *addre
if (ret == EFI_NO_SMBIOS)
fprintf(stderr, "%s: SMBIOS entry point missing\n", filename);
return ret;
+#elif defined(__FreeBSD__)
+ /*
+ * On FreeBSD, SMBIOS anchor base address in UEFI mode is exposed
+ * https://svnweb.freebsd.org/base?view=revision&revision=307326
+ */
+ ret = kenv(KENV_GET, "hint.smbios.0.mem", addrstr, sizeof(addrstr));
+ if (ret == -1) {
+ if (errno != ENOENT)
+ perror("kenv");
+ return EFI_NOT_FOUND;
+ }
+
+ *address = strtoull(addrstr, NULL, 0);
+ if (!(opt.flags & FLAG_QUIET))
+ printf("# SMBIOS entry point at 0x%08llx\n",
+ (unsigned long long)*address);
+ return 0;

This part of the code is the same as in the __linux__ case. It would be
nice to avoid duplicating it. Maybe it could be moved to the end of the
function, or moved to a separate function altogether (possibly easier.)

+#endif
}
int main(int argc, char * const argv[])

While it makes sense to restrict each section to its OS, I see two
problems with your patch:

1* If building neither on Linux nor on FreeBSD, you get 2 warnings:

dmidecode.c: In function ‘address_from_efi’:
dmidecode.c:4948:6: warning: unused variable ‘ret’ [-Wunused-variable]
int ret;
^
dmidecode.c:5004:1: warning: no return statement in function returning non-void [-Wreturn-type]
}
^

This needs to be addressed.

2* I know that sysfs is Linux-specific, but /proc is not. Are you
certain that no other operating system implements /proc/efi/systab?
Well that can be sorted out later, if needed, not a big problem.

Thanks,

[Jean Delvare]

Hi Alexey,

Post by Jean Delvare

Currently, dmidecode(8) does not work on FreeBSD booted in UEFI mode.
Previously it was understandable, since there are no things like Linuxish
/proc/efi/systab or /sys/firmware/efi/systab to read from under FreeBSD.
https://svnweb.freebsd.org/base?view=revision&revision=307326
I've patched dmidecode.c to try to get the address from hint.smbios.0.mem
and fall back to traditional address space scanning. I've tested it both
on EFI (amd64 laptop) and non-EFI (i386 desktop) machines.
--- dmidecode.c.orig 2017-05-23 13:34:14 UTC
+++ dmidecode.c
@@ -58,6 +58,10 @@
* https://trustedcomputinggroup.org/pc-client-platform-tpm-profile-ptp-specification/
*/
+#ifdef __FreeBSD__
+#include <errno.h>
+#include <kenv.h>
+#endif

I think I'd prefer them after the common ones, and with a separating
blank line.

Be my guest; I'm not trying to interfere with you coding style in any way;
feel free to reorder #include's to your liking.

Done.

Post by Jean Delvare

+ *address = strtoull(addrstr, NULL, 0);
+ if (!(opt.flags & FLAG_QUIET))
+ printf("# SMBIOS entry point at 0x%08llx\n",
+ (unsigned long long)*address);
+ return 0;

This part of the code is the same as in the __linux__ case. It would be
nice to avoid duplicating it. Maybe it could be moved to the end of the
function, or moved to a separate function altogether (possibly easier.)

Well, again, I wanted to have a working implementation, and if you can
refactor it to a better shape, I'll happily test the final patch to make
sure it works on FreeBSD for both BIOS and UEFI cases.

I'll write a separate patch, and post it so you can test it.

Post by Jean Delvare
dmidecode.c:4948:6: warning: unused variable ???ret??? [-Wunused-variable]
int ret;
^
dmidecode.c:5004:1: warning: no return statement in function returning non-void [-Wreturn-type]
}
^
This needs to be addressed.

Right, I've seen this warning, I think. Do you want me to fix it, or
rather address it on your own?

I fixed it.

Post by Jean Delvare
2* I know that sysfs is Linux-specific, but /proc is not. Are you
certain that no other operating system implements /proc/efi/systab?

Traditionally /proc was solely about processes, and FreeBSD adheres
to this, being Unix. Linux, on the other hand, had decided to abuse
/proc for a number of other, unrelated things, but than again: Linux
is not strictly a Unix, just a look-alike.
That said, I do believe that /proc/efi/systab path is Linux-specific
and won't count on it being present and/or expectedly populated in
some other operating system.

OK, thanks. Can you please test the following patch and confirm it
still works for you?

---
dmidecode.c | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)

--- dmidecode.c.orig 2017-05-23 10:15:36.397386352 +0200
+++ dmidecode.c 2017-09-26 08:42:08.339871831 +0200
@@ -64,6 +64,11 @@
#include <stdlib.h>
#include <unistd.h>

+#ifdef __FreeBSD__
+#include <errno.h>
+#include <kenv.h>
+#endif
+
#include "version.h"
#include "config.h"
#include "types.h"
@@ -4934,13 +4939,18 @@ static int legacy_decode(u8 *buf, const
#define EFI_NO_SMBIOS (-2)
static int address_from_efi(off_t *address)
{
+#if defined(__linux__)
FILE *efi_systab;
const char *filename;
char linebuf[64];
+#elif defined(__FreeBSD__)
+ char addrstr[KENV_MVALLEN + 1];
+#endif
int ret;

*address = 0; /* Prevent compiler warning */

+#if defined(__linux__)
/*
* Linux up to 2.6.6: /proc/efi/systab
* Linux 2.6.7 and up: /sys/firmware/efi/systab
@@ -4972,6 +4982,27 @@ static int address_from_efi(off_t *addre

if (ret == EFI_NO_SMBIOS)
fprintf(stderr, "%s: SMBIOS entry point missing\n", filename);
+#elif defined(__FreeBSD__)
+ /*
+ * On FreeBSD, SMBIOS anchor base address in UEFI mode is exposed
+ * via kernel environment:
+ * https://svnweb.freebsd.org/base?view=revision&revision=307326
+ */
+ ret = kenv(KENV_GET, "hint.smbios.0.mem", addrstr, sizeof(addrstr));
+ if (ret == -1)
+ {
+ if (errno != ENOENT)
+ perror("kenv");
+ return EFI_NOT_FOUND;
+ }
+
+ *address = strtoull(addrstr, NULL, 0);
+ if (!(opt.flags & FLAG_QUIET))
+ printf("# SMBIOS entry point at 0x%08llx\n",
+ (unsigned long long)*address);
+#else
+ ret = EFI_NOT_FOUND;
+#endif
return ret;
}