From eb7db33c6cf4172551fe0f9f7cf4aa047dc16d88 Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
Date: Wed, 20 Jun 2018 14:27:11 -0400
Subject: [PATCH 10/39] Improve ACPI device path formatting
This factors a bunch of the duplication out to another function, which
also does a better job of it.
Signed-off-by: Peter Jones <pjones@redhat.com>
---
src/dp-acpi.c | 296 ++++++++++++++++++---------------
src/include/efivar/efivar-dp.h | 2 +-
2 files changed, 159 insertions(+), 139 deletions(-)
diff --git a/src/dp-acpi.c b/src/dp-acpi.c
index 70162f320dc..a49ef38488c 100644
--- a/src/dp-acpi.c
+++ b/src/dp-acpi.c
@@ -44,6 +44,59 @@ _format_acpi_adr(char *buf, size_t size,
#define format_acpi_adr(buf, size, off, dp) \
format_helper(_format_acpi_adr, buf, size, off, "AcpiAdr", dp)
+static ssize_t
+_format_acpi_hid_ex(char *buf, size_t size, const char *dp_type UNUSED,
+ const_efidp dp,
+ const char *hidstr, const char *cidstr, const char *uidstr)
+{
+ ssize_t off = 0;
+
+ debug(DEBUG, "hid:0x%08x hidstr:\"%s\"", dp->acpi_hid_ex.hid, hidstr);
+ debug(DEBUG, "cid:0x%08x cidstr:\"%s\"", dp->acpi_hid_ex.cid, cidstr);
+ debug(DEBUG, "uid:0x%08x uidstr:\"%s\"", dp->acpi_hid_ex.uid, uidstr);
+
+ if (!hidstr && !cidstr && (uidstr || dp->acpi_hid_ex.uid)) {
+ format(buf, size, off, "AcpiExp",
+ "AcpiExp(0x%"PRIx32",0x%"PRIx32",",
+ dp->acpi_hid_ex.hid, dp->acpi_hid_ex.cid);
+ if (uidstr) {
+ format(buf, size, off, "AcpiExp", "%s)", uidstr);
+ } else {
+ format(buf, size, off, "AcpiExp", "0x%"PRIx32")",
+ dp->acpi_hid_ex.uid);
+ }
+ return off;
+ }
+
+ format(buf, size, off, "AcpiEx", "AcpiEx(");
+ if (hidstr) {
+ format(buf, size, off, "AcpiEx", "%s,", hidstr);
+ } else {
+ format(buf, size, off, "AcpiEx", "0x%"PRIx32",",
+ dp->acpi_hid_ex.hid);
+ }
+
+ if (cidstr) {
+ format(buf, size, off, "AcpiEx", "%s,", cidstr);
+ } else {
+ format(buf, size, off, "AcpiEx", "0x%"PRIx32",",
+ dp->acpi_hid_ex.cid);
+ }
+
+ if (uidstr) {
+ format(buf, size, off, "AcpiEx", "%s)", uidstr);
+ } else {
+ format(buf, size, off, "AcpiEx", "0x%"PRIx32")",
+ dp->acpi_hid_ex.uid);
+ }
+
+ return off;
+}
+
+#define format_acpi_hid_ex(buf, size, off, dp, hidstr, cidstr, uidstr) \
+ format_helper(_format_acpi_hid_ex, buf, size, off, "AcpiEx", dp,\
+ hidstr, cidstr, uidstr)
+
ssize_t
_format_acpi_dn(char *buf, size_t size, const_efidp dp)
{
@@ -53,13 +106,15 @@ _format_acpi_dn(char *buf, size_t size, const_efidp dp)
const char *uidstr = NULL;
size_t uidlen = 0;
const char *cidstr = NULL;
- size_t cidlen = 0;
+ // size_t cidlen = 0;
if (dp->subtype == EFIDP_ACPI_ADR) {
+ debug(DEBUG, "formatting ACPI _ADR");
format_acpi_adr(buf, size, off, dp);
return off;
} else if (dp->subtype != EFIDP_ACPI_HID_EX &&
dp->subtype != EFIDP_ACPI_HID) {
+ debug(DEBUG, "DP subtype %d, formatting as ACPI Path", dp->subtype);
format(buf, size, off, "AcpiPath", "AcpiPath(%d,", dp->subtype);
format_hex(buf, size, off, "AcpiPath", (uint8_t *)dp+4,
(efidp_node_size(dp)-4) / 2);
@@ -69,6 +124,7 @@ _format_acpi_dn(char *buf, size_t size, const_efidp dp)
ssize_t limit = efidp_node_size(dp)
- offsetof(efidp_acpi_hid_ex, hidstr);
+ debug(DEBUG, "formatting ACPI HID EX");
hidstr = dp->acpi_hid_ex.hidstr;
hidlen = strnlen(hidstr, limit);
limit -= hidlen + 1;
@@ -81,7 +137,7 @@ _format_acpi_dn(char *buf, size_t size, const_efidp dp)
if (limit) {
cidstr = uidstr + uidlen + 1;
- cidlen = strnlen(cidstr, limit);
+ // cidlen = strnlen(cidstr, limit);
// limit -= cidlen + 1;
}
@@ -96,143 +152,102 @@ _format_acpi_dn(char *buf, size_t size, const_efidp dp)
"PcieRoot(%s)", uidstr);
return off;
default:
- format(buf, size, off, "AcpiEx", "AcpiEx(");
- if (hidlen)
- format(buf, size, off, "AcpiEx", "%s",
- hidstr);
- else
- format(buf, size, off, "AcpiEx", "0x%"PRIx32,
- dp->acpi_hid_ex.hid);
- if (cidlen)
- format(buf, size, off, "AcpiEx", ",%s",
- cidstr);
- else
- format(buf, size, off, "AcpiEx", ",0x%"PRIx32,
- dp->acpi_hid_ex.cid);
- if (uidlen)
- format(buf, size, off, "AcpiEx", ",%s",
- uidstr);
- else
- format(buf, size, off, "AcpiEx", ",0x%"PRIx32,
- dp->acpi_hid_ex.uid);
- format(buf, size, off, "AcpiEx", ")");
- break;
+ format_acpi_hid_ex(buf, size, off, dp,
+ hidstr, cidstr, uidstr);
+ return off;
}
}
- }
-
- switch (dp->acpi_hid.hid) {
- case EFIDP_ACPI_PCI_ROOT_HID:
- format(buf, size, off, "PciRoot", "PciRoot(0x%"PRIx32")",
- dp->acpi_hid.uid);
- break;
- case EFIDP_ACPI_PCIE_ROOT_HID:
- format(buf, size, off, "PcieRoot", "PcieRoot(0x%"PRIx32")",
- dp->acpi_hid.uid);
- break;
- case EFIDP_ACPI_FLOPPY_HID:
- format(buf, size, off, "Floppy", "Floppy(0x%"PRIx32")",
- dp->acpi_hid.uid);
- break;
- case EFIDP_ACPI_KEYBOARD_HID:
- format(buf, size, off, "Keyboard", "Keyboard(0x%"PRIx32")",
- dp->acpi_hid.uid);
- break;
- case EFIDP_ACPI_SERIAL_HID:
- format(buf, size, off, "Keyboard", "Serial(0x%"PRIx32")",
- dp->acpi_hid.uid);
- break;
- case EFIDP_ACPI_NVDIMM_HID: {
- int rc;
- const_efidp next = NULL;
- efidp_acpi_adr *adrdp;
- int end;
-
- format(buf, size, off, "NvRoot()", "NvRoot()");
-
- rc = efidp_next_node(dp, &next);
- if (rc < 0 || !next) {
- efi_error("could not format DP");
- return rc;
- }
+ } else if (dp->subtype == EFIDP_ACPI_HID_EX) {
+ switch (dp->acpi_hid.hid) {
+ case EFIDP_ACPI_PCI_ROOT_HID:
+ format(buf, size, off, "PciRoot",
+ "PciRoot(0x%"PRIx32")",
+ dp->acpi_hid.uid);
+ break;
+ case EFIDP_ACPI_PCIE_ROOT_HID:
+ format(buf, size, off, "PcieRoot",
+ "PcieRoot(0x%"PRIx32")",
+ dp->acpi_hid.uid);
+ break;
+ case EFIDP_ACPI_FLOPPY_HID:
+ format(buf, size, off, "Floppy",
+ "Floppy(0x%"PRIx32")",
+ dp->acpi_hid.uid);
+ break;
+ case EFIDP_ACPI_KEYBOARD_HID:
+ format(buf, size, off, "Keyboard",
+ "Keyboard(0x%"PRIx32")",
+ dp->acpi_hid.uid);
+ break;
+ case EFIDP_ACPI_SERIAL_HID:
+ format(buf, size, off, "Serial",
+ "Serial(0x%"PRIx32")",
+ dp->acpi_hid.uid);
+ break;
+ case EFIDP_ACPI_NVDIMM_HID: {
+ int rc;
+ const_efidp next = NULL;
+ efidp_acpi_adr *adrdp;
+ int end;
- if (efidp_type(next) != EFIDP_ACPI_TYPE ||
- efidp_subtype(next) != EFIDP_ACPI_ADR) {
- efi_error("Invalid child node type (0x%02x,0x%02x)",
- efidp_type(next), efidp_subtype(next));
- return -EINVAL;
- }
- adrdp = (efidp_acpi_adr *)next;
+ format(buf, size, off, "NvRoot()", "NvRoot()");
- end = efidp_size_after(adrdp, header)
- / sizeof(adrdp->adr[0]);
+ rc = efidp_next_node(dp, &next);
+ if (rc < 0 || !next) {
+ efi_error("could not format DP");
+ return rc;
+ }
- for (int i = 0; i < end; i++) {
- uint32_t node_controller, socket, memory_controller;
- uint32_t memory_channel, dimm;
- uint32_t adr = adrdp->adr[i];
+ if (efidp_type(next) != EFIDP_ACPI_TYPE ||
+ efidp_subtype(next) != EFIDP_ACPI_ADR) {
+ efi_error("Invalid child node type (0x%02x,0x%02x)",
+ efidp_type(next), efidp_subtype(next));
+ return -EINVAL;
+ }
+ adrdp = (efidp_acpi_adr *)next;
- efidp_decode_acpi_nvdimm_adr(adr, &node_controller,
- &socket,
- &memory_controller,
- &memory_channel, &dimm);
+ end = efidp_size_after(adrdp, header)
+ / sizeof(adrdp->adr[0]);
- if (i != 0)
- format(buf, size, off, "NvDimm", ",");
+ for (int i = 0; i < end; i++) {
+ uint32_t node_controller, socket, memory_controller;
+ uint32_t memory_channel, dimm;
+ uint32_t adr = adrdp->adr[i];
- format(buf, size, off, "NvDimm",
- "NvDimm(0x%03x,0x%01x,0x%01x,0x%01x,0x%01x)",
- node_controller, socket, memory_controller,
- memory_channel, dimm);
- }
- break;
- }
- default:
- switch (dp->subtype) {
- case EFIDP_ACPI_HID_EX:
- if (!hidstr && !cidstr &&
- (uidstr || dp->acpi_hid_ex.uid)){
- format(buf, size, off, "AcpiExp",
- "AcpiExp(0x%"PRIx32",0x%"PRIx32",",
- dp->acpi_hid_ex.hid,
- dp->acpi_hid_ex.cid);
- if (uidstr) {
- format(buf, size, off, "AcpiExp",
- "%s)", uidstr);
- } else {
- format(buf, size, off, "AcpiExp",
- "0x%"PRIx32")",
- dp->acpi_hid.uid);
- }
- break;
- }
- format(buf, size, off, "AcpiEx", "AcpiEx(");
- if (hidstr) {
- format(buf, size, off, "AcpiEx", "%s,", hidstr);
- } else {
- format(buf, size, off, "AcpiEx", "0x%"PRIx32",",
- dp->acpi_hid.hid);
- }
+ efidp_decode_acpi_nvdimm_adr(adr,
+ &node_controller, &socket,
+ &memory_controller, &memory_channel,
+ &dimm);
- if (cidstr) {
- format(buf, size, off, "AcpiEx", "%s,", cidstr);
- } else {
- format(buf, size, off, "AcpiEx", "0x%"PRIx32",",
- dp->acpi_hid_ex.cid);
- }
+ if (i != 0)
+ format(buf, size, off, "NvDimm", ",");
- if (uidstr) {
- format(buf, size, off, "AcpiEx", "%s)", uidstr);
- } else {
- format(buf, size, off, "AcpiEx", "0x%"PRIx32")",
- dp->acpi_hid.uid);
+ format(buf, size, off, "NvDimm",
+ "NvDimm(0x%03x,0x%01x,0x%01x,0x%01x,0x%01x)",
+ node_controller, socket, memory_controller,
+ memory_channel, dimm);
}
break;
- case EFIDP_ACPI_HID:
- format(buf, size, off, "Acpi",
- "Acpi(0x%"PRIx32",0x%"PRIx32")",
- dp->acpi_hid.hid, dp->acpi_hid.uid);
- break;
+ }
+ default:
+ debug(DEBUG, "Decoding non-well-known HID");
+ switch (dp->subtype) {
+ case EFIDP_ACPI_HID_EX:
+ format_acpi_hid_ex(buf, size, off, dp,
+ hidstr, cidstr, uidstr);
+ break;
+ case EFIDP_ACPI_HID:
+ debug(DEBUG, "Decoding ACPI HID");
+ format(buf, size, off, "Acpi",
+ "Acpi(0x%08x,0x%"PRIx32")",
+ dp->acpi_hid.hid, dp->acpi_hid.uid);
+ break;
+ default:
+ debug(DEBUG, "ACPI subtype %d???",
+ dp->subtype);
+ errno = EINVAL;
+ return -1;
+ }
}
}
@@ -259,7 +274,7 @@ efidp_make_acpi_hid(uint8_t *buf, ssize_t size, uint32_t hid, uint32_t uid)
return sz;
}
-ssize_t PUBLIC NONNULL(6, 7, 8)
+ssize_t PUBLIC
efidp_make_acpi_hid_ex(uint8_t *buf, ssize_t size,
uint32_t hid, uint32_t uid, uint32_t cid,
const char *hidstr, const char *uidstr,
@@ -268,21 +283,26 @@ efidp_make_acpi_hid_ex(uint8_t *buf, ssize_t size,
efidp_acpi_hid_ex *acpi_hid = (efidp_acpi_hid_ex *)buf;
ssize_t req;
ssize_t sz;
+ size_t hidlen = hidstr ? strlen(hidstr) : 0;
+ size_t uidlen = uidstr ? strlen(uidstr) : 0;
+ size_t cidlen = cidstr ? strlen(cidstr) : 0;
- req = sizeof (*acpi_hid) + 3 +
- strlen(hidstr) + strlen(uidstr) + strlen(cidstr);
+ req = sizeof (*acpi_hid) + 3 + hidlen + uidlen + cidlen;
sz = efidp_make_generic(buf, size, EFIDP_ACPI_TYPE, EFIDP_ACPI_HID_EX,
req);
if (size && sz == req) {
- acpi_hid->uid = uid;
- acpi_hid->hid = hid;
- acpi_hid->cid = cid;
+ acpi_hid->hid = hidlen ? 0 : hid;
+ acpi_hid->uid = uidlen ? 0 : uid;
+ acpi_hid->cid = cidlen ? 0 : cid;
char *next = (char *)buf+offsetof(efidp_acpi_hid_ex, hidstr);
- strcpy(next, hidstr);
- next += strlen(hidstr) + 1;
- strcpy(next, uidstr);
- next += strlen(uidstr) + 1;
- strcpy(next, cidstr);
+ if (hidlen)
+ strcpy(next, hidstr);
+ next += hidlen + 1;
+ if (uidlen)
+ strcpy(next, uidstr);
+ next += uidlen + 1;
+ if (cidlen)
+ strcpy(next, cidstr);
}
if (sz < 0)
diff --git a/src/include/efivar/efivar-dp.h b/src/include/efivar/efivar-dp.h
index 106d3645e21..1b05775ae7e 100644
--- a/src/include/efivar/efivar-dp.h
+++ b/src/include/efivar/efivar-dp.h
@@ -133,7 +133,7 @@ typedef struct {
/* three ascii string fields follow */
char hidstr[];
} EFIVAR_PACKED efidp_acpi_hid_ex;
-extern ssize_t __attribute__((__nonnull__ (6,7,8)))
+extern ssize_t
efidp_make_acpi_hid_ex(uint8_t *buf, ssize_t size,
uint32_t hid, uint32_t uid, uint32_t cid,
const char *hidstr, const char *uidstr,
--
2.17.1