> Date: Thu, 15 Sep 2022 11:32:02 +0200
> From: Paweł Cichowski <pawel.cichow...@3mdeb.com>
>
> I am trying it out now, unfortunately I'm running into this error:
> fatal privileged instruction fault in supervisor mode
> [...]
> kernel: privileged instruction fault trap, code=0
> Stopped in pid 0.0 (system) at netbsd:xrstor+0xa: fxsave1
> This happens in efi_runtime_exit, more specifically - during
> fpu_kern_leave. It's probably due to me naively debugging by just
> calling it during efi_init, so now I'll try to do it properly and
> add efi_gettime to /dev/efi.
efi_init is probably too early to call this stuff -- not sure exactly
at what point fpu_kern_enter/leave becomes safe but efi_init may be
too early.
> How did you go about testing it?
I just wrote a little userland program, attached, to list the
variables and query their values using /dev/efi, which I've been
running in qemu booted with OVMF:
qemu-system-x86_64 \
-machine pc,accel=nvmm \
-bios /usr/pkg/share/ovmf/OVMFX64.fd \
...
> My main concern right now is not knowing where to try and start
> contributing to NetBSD, as it really sparked my interest. I found
> this project list https://wiki.netbsd.org/projects/all/#index4h1 and
> will probably look for some features of other BSDs which could be
> ported. Could you tell me where or how I could find some entry-level
> projects?
That's a reasonable place to start, although it's a good idea to ask
about any particular projects first -- many of those are a bit
outdated.
Also always a good strategy to try starting with something you want to
use or adjacent to something you want to use like this EFI business.
I noticed that fwupd needs to get at EFI configuration tables too so I
drafted a patch to expose this through /dev/efi (meant to be source-
compatible with FreeBSD) but I haven't tested it and it probably won't
work on the first try.
#include <sys/cdefs.h>
#include <sys/efiio.h>
#include <sys/ioctl.h>
#include <err.h>
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <uuid.h>
#define _PATH_EFI "/dev/efi"
static const struct uuid EFI_GLOBAL_VARIABLE = {
0x8BE4DF61,0x93CA,0x11d2,
0xAA,0x0D,{0x00,0xE0,0x98,0x03,0x2B,0x8C},
};
static const unsigned char *const stdvars[] = {
"AuditMode",
"Boot0000",
"Boot0001",
"Boot0002",
"Boot0003",
"Boot0004",
"Boot0005",
"Boot0006",
"Boot0007",
"BootCurrent",
"BootNext",
"BootOrder",
"BootOptionSupport",
"ConIn",
"ConInDev",
"ConOut",
"ConOutDev",
"dbDefault",
"dbrDefault",
"dbtDefault",
"dbxDefault",
"DeployedMode",
"Driver0000",
"Driver0001",
"Driver0002",
"Driver0003",
"DriverOrder",
"ErrOut",
"ErrOutDev",
"HwErrRecSupport",
"KEK",
"KEKDefault",
"Key0000",
"Key0001",
"Key0002",
"Key0003",
"Lang",
"LangCodes",
"OsIndications",
"OsIndicationsSupported",
"OsRecoveryOrder",
"PK",
"PKDefault",
"PlatformLangCodes",
"PlatformLang",
"PlatformRecovery0000",
"PlatformRecovery0001",
"PlatformRecovery0002",
"PlatformRecovery0003",
"SignatureSupport",
"SecureBoot",
"SetupMode",
"SysPrep0000",
"SysPrep0001",
"SysPrep0002",
"SysPrep0003",
"SysPrepOrder",
"Timeout",
"VendorKeys",
};
int
main(void)
{
uint16_t name[128] = {0};
uint8_t buf[1024];
struct efi_var_ioc e = {
.name = name,
.namesize = __arraycount(name),
};
struct efi_var_ioc e1;
int fd;
unsigned i, j;
if ((fd = open(_PATH_EFI, O_RDWR)) == -1)
err(1, "open");
for (;;) {
if (ioctl(fd, EFIIOC_VAR_NEXT, &e) == -1) {
warn("ioctl(EFIIOC_VAR_NEXT)");
break;
}
if (e.name == NULL)
break;
char *uuidstr;
uint32_t uuidstatus;
uuid_to_string(&e.vendor, &uuidstr, &uuidstatus);
if (uuidstatus == uuid_s_ok) {
printf("[%s] ", uuidstr);
free(uuidstr);
} else {
printf("[err 0x%x] ", uuidstatus);
}
for (i = 0; i < __arraycount(name); i++) {
if (name[i] == 0)
break;
if (name[i] >= 256)
printf("\\U+%04x", name[i]);
else
printf("%c", name[i]);
}
memset(buf, 0, sizeof(buf));
e1 = e;
e1.data = buf;
e1.datasize = sizeof(buf);
if (ioctl(fd, EFIIOC_VAR_GET, &e1) == -1) {
printf("\n");
fflush(stdout);
warn("ioctl(EFIIOC_VAR_GET)");
continue;
}
printf(" 0x%x\n", e1.attrib);
for (i = 0; i < e1.datasize; i++) {
if ((i % 16) == 8)
printf(" ");
printf(" %02x", buf[i]);
if ((i % 16) == 15)
printf("\n");
}
if (e1.datasize % 16)
printf("\n");
}
for (i = 0; i < __arraycount(stdvars); i++) {
printf("%s", stdvars[i]);
for (j = 0; stdvars[i][j] != '\0'; j++)
name[j] = stdvars[i][j];
name[j] = 0;
e.name = name;
e.namesize = 2*(j + 1);
e.vendor = EFI_GLOBAL_VARIABLE;
memset(buf, 0, sizeof(buf));
e.data = buf;
e.datasize = sizeof(buf);
if (ioctl(fd, EFIIOC_VAR_GET, &e) == -1) {
printf("\n");
fflush(stdout);
warn("ioctl(EFIIOC_VAR_GET)");
continue;
}
printf(" 0x%x\n", e.attrib);
for (j = 0; j < e.datasize; j++) {
if ((j % 16) == 0)
printf(" ");
printf(" %02x", buf[j]);
if ((j % 16) == 15)
printf("\n");
}
if (e.datasize % 16)
printf("\n");
}
fflush(stdout);
return ferror(stdout);
}
>From cb0f9e34b035742fa323c43867fea1262e4b373c Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastr...@netbsd.org>
Date: Thu, 15 Sep 2022 07:54:19 +0000
Subject: [PATCH 1/2] efi(4): Implement MI parts of EFIIOC_GET_TABLE.
Intended to be compatible with FreeBSD.
Not yet supported on any architectures.
---
sys/dev/efi.c | 200 +++++++++++++++++++++++++++++++++++++++++++++++
sys/dev/efivar.h | 8 +-
sys/sys/efiio.h | 8 ++
3 files changed, 214 insertions(+), 2 deletions(-)
diff --git a/sys/dev/efi.c b/sys/dev/efi.c
index 49a1f2e387e5..67c9b17ddde4 100644
--- a/sys/dev/efi.c
+++ b/sys/dev/efi.c
@@ -40,7 +40,10 @@ __KERNEL_RCSID(0, "$NetBSD: efi.c,v 1.3 2022/04/01 06:51:12
skrll Exp $");
#include <sys/atomic.h>
#include <sys/efiio.h>
+#include <uvm/uvm_extern.h>
+
#include <dev/efivar.h>
+#include <dev/mm.h>
#ifdef _LP64
#define EFIERR(x) (0x8000000000000000 | x)
@@ -149,6 +152,201 @@ efi_status_to_error(efi_status status)
}
}
+/* XXX move to efi.h */
+#define EFI_SYSTEM_RESOURCE_TABLE_GUID
\
+ {0xb122a263,0x3661,0x4f68,0x99,0x29,{0x78,0xf8,0xb0,0xd6,0x21,0x80}}
+#define EFI_PROPERTIES_TABLE
\
+ {0x880aaca3,0x4adc,0x4a04,0x90,0x79,{0xb7,0x47,0x34,0x08,0x25,0xe5}}
+
+#define EFI_SYSTEM_RESOURCE_TABLE_FIRMWARE_RESOURCE_VERSION 1
+
+struct EFI_SYSTEM_RESOURCE_ENTRY {
+ struct uuid FwClass;
+ uint32_t FwType;
+ uint32_t FwVersion;
+ uint32_t LowestSupportedFwVersion;
+ uint32_t CapsuleFlags;
+ uint32_t LastAttemptVersion;
+ uint32_t LastAttemptStatus;
+};
+
+struct EFI_SYSTEM_RESOURCE_TABLE {
+ uint32_t FwResourceCount;
+ uint32_t FwResourceCountMax;
+ uint64_t FwResourceVersion;
+ struct EFI_SYSTEM_RESOURCE_ENTRY Entries[];
+};
+
+static void *
+efi_map_pa(uint64_t addr, bool *directp)
+{
+ paddr_t pa = addr;
+ vaddr_t va;
+
+ /*
+ * Verify the address is not truncated by conversion to
+ * paddr_t. This might happen with a 64-bit EFI booting a
+ * 32-bit OS.
+ */
+ if (pa != addr)
+ return NULL;
+
+ /*
+ * Try direct-map if we have it. If it works, note that it was
+ * direct-mapped for efi_unmap.
+ */
+#ifdef __HAVE_MM_MD_DIRECT_MAPPED_PHYS
+ if (mm_md_direct_mapped_phys(pa, &va)) {
+ *directp = true;
+ return (void *)va;
+ }
+#endif
+
+ /*
+ * No direct map. Reserve a page of kernel virtual address
+ * space, with no backing, to map to the physical address.
+ */
+ va = uvm_km_alloc(kernel_map, PAGE_SIZE, 0,
+ UVM_KMF_VAONLY|UVM_KMF_WAITVA);
+ KASSERT(va != 0);
+
+ /*
+ * Map the kva page to the physical address and update the
+ * kernel pmap so we can use it.
+ */
+ pmap_kenter_pa(va, pa, VM_PROT_READ, 0);
+ pmap_update(pmap_kernel());
+
+ /*
+ * Success! Return the VA and note that it was not
+ * direct-mapped for efi_unmap.
+ */
+ *directp = false;
+ return (void *)va;
+}
+
+static void
+efi_unmap(void *ptr, bool direct)
+{
+ vaddr_t va = (vaddr_t)ptr;
+
+ /*
+ * If it was direct-mapped, nothing to do here.
+ */
+ if (direct)
+ return;
+
+ /*
+ * First remove the mapping from the kernel pmap so that it can
+ * be reused, before we free the kva and let anyone else reuse
+ * it.
+ */
+ pmap_kremove(va, PAGE_SIZE);
+ pmap_update(pmap_kernel());
+
+ /*
+ * Next free the kva so it can be reused by someone else.
+ */
+ uvm_km_free(kernel_map, va, PAGE_SIZE, UVM_KMF_VAONLY);
+}
+
+static int
+efi_ioctl_got_table(struct efi_get_table_ioc *ioc, void *ptr, size_t len)
+{
+
+ /*
+ * Return the actual table length.
+ */
+ ioc->table_len = len;
+
+ /*
+ * Copy out as much as we can into the user's allocated buffer.
+ */
+ return copyout(ioc->buf, ptr, MIN(ioc->buf_len, len));
+}
+
+static int
+efi_ioctl_get_esrt(struct efi_get_table_ioc *ioc,
+ struct EFI_SYSTEM_RESOURCE_TABLE *tab)
+{
+
+ /*
+ * Verify the firmware resource version is one we understand.
+ */
+ if (tab->FwResourceVersion !=
+ EFI_SYSTEM_RESOURCE_TABLE_FIRMWARE_RESOURCE_VERSION)
+ return ENOENT;
+
+ /*
+ * Verify the resource count fits within the single page we
+ * have mapped.
+ *
+ * XXX What happens if it doesn't? Are we expected to map more
+ * than one page, according to the table header? The UEFI spec
+ * is unclear on this.
+ */
+ const size_t entry_space = PAGE_SIZE -
+ offsetof(struct EFI_SYSTEM_RESOURCE_TABLE, Entries);
+ if (tab->FwResourceCount > entry_space/sizeof(tab->Entries[0]))
+ return ENOENT;
+
+ /*
+ * Success! Return everything through the last table entry.
+ */
+ const size_t len = offsetof(struct EFI_SYSTEM_RESOURCE_TABLE,
+ Entries[tab->FwResourceCount]);
+ return efi_ioctl_got_table(ioc, tab, len);
+}
+
+static int
+efi_ioctl_get_table(struct efi_get_table_ioc *ioc)
+{
+ uint64_t addr;
+ bool direct;
+ efi_status status;
+ int error;
+
+ /*
+ * If the platform doesn't support it yet, fail now.
+ */
+ if (efi_ops->efi_gettab == NULL)
+ return ENODEV;
+
+ /*
+ * Get the address of the requested table out of the EFI
+ * configuration table.
+ */
+ status = efi_ops->efi_gettab(&ioc->uuid, &addr);
+ if (status != EFI_SUCCESS)
+ return efi_status_to_error(status);
+
+ /*
+ * UEFI provides no generic way to identify the size of the
+ * table, so we have to bake knowledge of every vendor GUID
+ * into this code to safely expose the right amount of data to
+ * userland.
+ *
+ * We even have to bake knowledge of which ones are physically
+ * addressed and which ones might be virtually addressed
+ * according to the vendor GUID into this code, although for
+ * the moment we never use RT->SetVirtualAddressMap so we only
+ * ever have to deal with physical addressing.
+ */
+ if (memcmp(&ioc->uuid, &(struct uuid)EFI_SYSTEM_RESOURCE_TABLE_GUID,
+ sizeof(ioc->uuid)) == 0) {
+ struct EFI_SYSTEM_RESOURCE_TABLE *tab;
+
+ if ((tab = efi_map_pa(addr, &direct)) == NULL)
+ return ENOENT;
+ error = efi_ioctl_get_esrt(ioc, tab);
+ efi_unmap(tab, direct);
+ } else {
+ error = ENOENT;
+ }
+
+ return error;
+}
+
static int
efi_ioctl_var_get(struct efi_var_ioc *var)
{
@@ -289,6 +487,8 @@ efi_ioctl(dev_t dev, u_long cmd, void *data, int flags,
struct lwp *l)
KASSERT(efi_ops != NULL);
switch (cmd) {
+ case EFIIOC_GET_TABLE:
+ return efi_ioctl_get_table(data);
case EFIIOC_VAR_GET:
return efi_ioctl_var_get(data);
case EFIIOC_VAR_NEXT:
diff --git a/sys/dev/efivar.h b/sys/dev/efivar.h
index 21d6d61fd26a..72aeb8c6fbae 100644
--- a/sys/dev/efivar.h
+++ b/sys/dev/efivar.h
@@ -29,16 +29,20 @@
#ifndef _DEV_EFIVAR_H
#define _DEV_EFIVAR_H
+#include <sys/uuid.h>
+#include <sys/types.h>
+
#include <machine/efi.h>
struct efi_ops {
efi_status (*efi_gettime)(struct efi_tm *, struct efi_tmcap *);
efi_status (*efi_settime)(struct efi_tm *);
efi_status (*efi_getvar)(uint16_t *, struct uuid *, uint32_t *,
- u_long *, void *);
+ u_long *, void *);
efi_status (*efi_setvar)(uint16_t *, struct uuid *, uint32_t,
- u_long, void *);
+ u_long, void *);
efi_status (*efi_nextvar)(u_long *, uint16_t *, struct uuid *);
+ efi_status (*efi_gettab)(const struct uuid *, uint64_t *);
};
void efi_register_ops(const struct efi_ops *);
diff --git a/sys/sys/efiio.h b/sys/sys/efiio.h
index 8f3a9a2d54e9..c50c2c416fa9 100644
--- a/sys/sys/efiio.h
+++ b/sys/sys/efiio.h
@@ -48,6 +48,13 @@
#define EFI_VARIABLE_APPEND_WRITE
0x00000040
#define EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS
0x00000080
+struct efi_get_table_ioc {
+ void * buf;
+ struct uuid uuid;
+ size_t table_len;
+ size_t buf_len;
+};
+
struct efi_var_ioc {
uint16_t * name; /* vendor's variable name */
size_t namesize; /* size in bytes of the name buffer */
@@ -57,6 +64,7 @@ struct efi_var_ioc {
size_t datasize; /* size in bytes of the data buffer */
};
+#define EFIIOC_GET_TABLE _IOWR('e', 1, struct efi_get_table_ioc)
#define EFIIOC_VAR_GET _IOWR('e', 4, struct efi_var_ioc)
#define EFIIOC_VAR_NEXT _IOWR('e', 5, struct efi_var_ioc)
#define EFIIOC_VAR_SET _IOWR('e', 7, struct efi_var_ioc)
>From f4b317934d0f1604190d8cc1cfe5dd8406bef5fe Mon Sep 17 00:00:00 2001
From: Taylor R Campbell <riastr...@netbsd.org>
Date: Thu, 15 Sep 2022 07:55:02 +0000
Subject: [PATCH 2/2] efi(4): Implement EFIIOC_GET_TABLE on x86.
---
sys/arch/x86/x86/efi_machdep.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/sys/arch/x86/x86/efi_machdep.c b/sys/arch/x86/x86/efi_machdep.c
index 22c96e460d6c..6f29511c484b 100644
--- a/sys/arch/x86/x86/efi_machdep.c
+++ b/sys/arch/x86/x86/efi_machdep.c
@@ -589,8 +589,10 @@ efi_get_e820memmap(void)
#define EFIERR(x) (0x80000000ul | (x))
#endif
+#define EFI_SUCCESS EFIERR(0)
#define EFI_UNSUPPORTED EFIERR(3)
#define EFI_DEVICE_ERROR EFIERR(7)
+#define EFI_NOT_FOUND EFIERR(14)
/*
* efi_runtime_init()
@@ -963,12 +965,29 @@ efi_runtime_setvar(efi_char *name, struct uuid *vendor,
uint32_t attrib,
return status;
}
+static efi_status
+efi_runtime_gettab(const struct uuid *vendor, uint64_t *addrp)
+{
+ struct efi_cfgtbl *cfgtbl = efi_getcfgtblhead();
+ paddr_t pa;
+
+ if (cfgtbl == NULL)
+ return EFI_UNSUPPORTED;
+
+ pa = efi_getcfgtblpa(vendor);
+ if (pa == 0)
+ return EFI_NOT_FOUND;
+ *addrp = pa;
+ return EFI_SUCCESS;
+}
+
static struct efi_ops efi_runtime_ops = {
.efi_gettime = efi_runtime_gettime,
.efi_settime = efi_runtime_settime,
.efi_getvar = efi_runtime_getvar,
.efi_setvar = efi_runtime_setvar,
.efi_nextvar = efi_runtime_nextvar,
+ .efi_gettab = efi_runtime_gettab,
};
#endif /* EFI_RUNTIME */
