From eb7db33c6cf4172551fe0f9f7cf4aa047dc16d88 Mon Sep 17 00:00:00 2001 From: Peter Jones Date: Wed, 20 Jun 2018 14:27:11 -0400 Subject: [PATCH 11/17] 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 --- 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