FreeBSD Bugzilla – Attachment 242928 Details for
Bug 272018
pci_read_vpd() does not always load VPD even when valid VPD exists because of state machine bug
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Updated VPD patch
pci-VPD.diff (text/plain), 14.97 KB, created by
Stefan Eßer
on 2023-06-21 16:53:51 UTC
(
hide
)
Description:
Updated VPD patch
Filename:
MIME Type:
Creator:
Stefan Eßer
Created:
2023-06-21 16:53:51 UTC
Size:
14.97 KB
patch
obsolete
>diff --git a/sys/dev/pci/pci.c b/sys/dev/pci/pci.c >index 24348ecf14eb..cbdfb3acec70 100644 >--- a/sys/dev/pci/pci.c >+++ b/sys/dev/pci/pci.c >@@ -1063,6 +1063,7 @@ struct vpd_readstate { > uint8_t cksum; > }; > >+/* return 0 and one byte in *data if no read error, -1 else */ > static int > vpd_nextbyte(struct vpd_readstate *vrs, uint8_t *data) > { >@@ -1071,7 +1072,7 @@ vpd_nextbyte(struct vpd_readstate *vrs, uint8_t *data) > > if (vrs->bytesinval == 0) { > if (pci_read_vpd_reg(vrs->pcib, vrs->cfg, vrs->off, ®)) >- return (ENXIO); >+ return (-1); > vrs->val = le32toh(reg); > vrs->off += 4; > byte = vrs->val & 0xff; >@@ -1087,303 +1088,284 @@ vpd_nextbyte(struct vpd_readstate *vrs, uint8_t *data) > return (0); > } > >+/* return 0 on match, -1 and "unget" byte on no match */ >+static int >+vpd_expectbyte(struct vpd_readstate *vrs, uint8_t expected) >+{ >+ uint8_t data; >+ >+ if (vpd_nextbyte(vrs, &data) != 0) >+ return (-1); >+ >+ if (data == expected) >+ return (0); >+ >+ vrs->cksum -= data; >+ vrs->val = (vrs->val << 8) + data; >+ vrs->bytesinval++; >+ return (-1); >+} >+ >+/* return size if tag matches, -1 on no match, -2 on read error */ >+static int >+vpd_read_tag_size(struct vpd_readstate *vrs, uint8_t vpd_tag) >+{ >+ uint8_t byte1, byte2; >+ >+ if (vpd_expectbyte(vrs, vpd_tag) != 0) >+ return (-1); >+ >+ if ((vpd_tag & 0x80) == 0) >+ return (vpd_tag & 0x07); >+ >+ if (vpd_nextbyte(vrs, &byte1) != 0) >+ return (-2); >+ if (vpd_nextbyte(vrs, &byte2) != 0) >+ return (-2); >+ >+ return ((byte2 << 8) + byte1); >+} >+ >+/* (re)allocate buffer in multiples of 8 elements */ >+static void* >+alloc_buffer(void* buffer, size_t element_size, int needed) >+{ >+ int alloc, new_alloc; >+ >+ alloc = roundup2(needed, 8); >+ new_alloc = roundup2(needed + 1, 8); >+ if (alloc != new_alloc) { >+ buffer = reallocf(buffer, >+ new_alloc * element_size, M_DEVBUF, M_WAITOK | M_ZERO); >+ } >+ >+ return (buffer); >+} >+ >+/* read VPD keyword and return element size, return -1 on read error */ >+static int >+vpd_read_elem_head(struct vpd_readstate *vrs, char keyword[2]) >+{ >+ uint8_t data; >+ >+ if (vpd_nextbyte(vrs, &keyword[0]) != 0) >+ return (-1); >+ if (vpd_nextbyte(vrs, &keyword[1]) != 0) >+ return (-1); >+ if (vpd_nextbyte(vrs, &data) != 0) >+ return (-1); >+ >+ return (data); >+} >+ >+/* read VPD data element of given size into allocated buffer */ >+static char * >+vpd_read_value(struct vpd_readstate *vrs, int size) >+{ >+ int i; >+ char char1; >+ char *value; >+ >+ value = malloc(size + 1, M_DEVBUF, M_WAITOK); >+ for (i = 0; i < size; i++) { >+ if (vpd_nextbyte(vrs, &char1) != 0) { >+ free(value, M_DEVBUF); >+ return (NULL); >+ } >+ value[i] = char1; >+ } >+ value[size] = '\0'; >+ >+ return (value); >+} >+ >+/* read VPD into *keyword and *value, return length of data element */ >+static int >+vpd_read_elem_data(struct vpd_readstate *vrs, char keyword[2], char **value, int maxlen) >+{ >+ int len; >+ >+ len = vpd_read_elem_head(vrs, keyword); >+ if (len > maxlen) >+ return (-1); >+ *value = vpd_read_value(vrs, len); >+ >+ return (len); >+} >+ >+/* subtract all data following first byte from checksum of RV element */ > static void >-pci_read_vpd(device_t pcib, pcicfgregs *cfg) >+vpd_fixup_cksum(struct vpd_readstate *vrs, char *rvstring, int len) > { >- struct vpd_readstate vrs; >- int state; >- int name; >- int remain; > int i; >- int alloc, off; /* alloc/off for RO/W arrays */ >- int cksumvalid; >- int dflen; >- int firstrecord; >- uint8_t byte; >- uint8_t byte2; >+ uint8_t fixup; > >- /* init vpd reader */ >- vrs.bytesinval = 0; >- vrs.off = 0; >- vrs.pcib = pcib; >- vrs.cfg = cfg; >- vrs.cksum = 0; >+ fixup = 0; >+ for (i = 1; i < len; i++) >+ fixup += rvstring[i]; >+ vrs->cksum -= fixup; >+} > >- state = 0; >- name = remain = i = 0; /* shut up stupid gcc */ >- alloc = off = 0; /* shut up stupid gcc */ >- dflen = 0; /* shut up stupid gcc */ >- cksumvalid = -1; >- firstrecord = 1; >- while (state >= 0) { >- if (vpd_nextbyte(&vrs, &byte)) { >- pci_printf(cfg, "VPD read timed out\n"); >- state = -2; >- break; >+/* fetch one read-only element and return size of heading + data */ >+static size_t >+next_vpd_ro_elem(struct vpd_readstate *vrs, int maxsize) >+{ >+ struct pcicfg_vpd *vpd; >+ pcicfgregs *cfg; >+ struct vpd_readonly *vpd_ros; >+ int len; >+ >+ cfg = vrs->cfg; >+ vpd = &cfg->vpd; >+ >+ if (maxsize < 3) >+ return (-1); >+ vpd->vpd_ros = alloc_buffer(vpd->vpd_ros, sizeof(*vpd->vpd_ros), vpd->vpd_rocnt); >+ vpd_ros = &vpd->vpd_ros[vpd->vpd_rocnt]; >+ maxsize -= 3; >+ len = vpd_read_elem_data(vrs, vpd_ros->keyword, &vpd_ros->value, maxsize); >+ if (vpd_ros->value == NULL) >+ return (-1); >+ vpd_ros->len = len; >+ if (vpd_ros->keyword[0] == 'R' && vpd_ros->keyword[1] == 'V') { >+ vpd_fixup_cksum(vrs, vpd_ros->value, len); >+ if (vrs->cksum != 0) { >+ pci_printf(cfg, >+ "invalid VPD checksum %#hhx\n", vrs->cksum); >+ return (-1); > } >-#if 0 >- pci_printf(cfg, "vpd: val: %#x, off: %d, bytesinval: %d, byte: " >- "%#hhx, state: %d, remain: %d, name: %#x, i: %d\n", vrs.val, >- vrs.off, vrs.bytesinval, byte, state, remain, name, i); >-#endif >- switch (state) { >- case 0: /* item name */ >- if (byte & 0x80) { >- if (vpd_nextbyte(&vrs, &byte2)) { >- state = -2; >- break; >- } >- remain = byte2; >- if (vpd_nextbyte(&vrs, &byte2)) { >- state = -2; >- break; >- } >- remain |= byte2 << 8; >- name = byte & 0x7f; >- } else { >- remain = byte & 0x7; >- name = (byte >> 3) & 0xf; >- } >- if (firstrecord) { >- if (name != 0x2) { >- pci_printf(cfg, "VPD data does not " \ >- "start with ident (%#x)\n", name); >- state = -2; >- break; >- } >- firstrecord = 0; >- } >- if (vrs.off + remain - vrs.bytesinval > 0x8000) { >- pci_printf(cfg, >- "VPD data overflow, remain %#x\n", remain); >- state = -1; >- break; >- } >- switch (name) { >- case 0x2: /* String */ >- if (cfg->vpd.vpd_ident != NULL) { >- pci_printf(cfg, >- "duplicate VPD ident record\n"); >- state = -2; >- break; >- } >- if (remain > 255) { >- pci_printf(cfg, >- "VPD ident length %d exceeds 255\n", >- remain); >- state = -2; >- break; >- } >- cfg->vpd.vpd_ident = malloc(remain + 1, >- M_DEVBUF, M_WAITOK); >- i = 0; >- state = 1; >- break; >- case 0xf: /* End */ >- state = -1; >- break; >- case 0x10: /* VPD-R */ >- alloc = 8; >- off = 0; >- cfg->vpd.vpd_ros = malloc(alloc * >- sizeof(*cfg->vpd.vpd_ros), M_DEVBUF, >- M_WAITOK | M_ZERO); >- state = 2; >- break; >- case 0x11: /* VPD-W */ >- alloc = 8; >- off = 0; >- cfg->vpd.vpd_w = malloc(alloc * >- sizeof(*cfg->vpd.vpd_w), M_DEVBUF, >- M_WAITOK | M_ZERO); >- state = 5; >- break; >- default: /* Invalid data, abort */ >- pci_printf(cfg, "invalid VPD name: %#x\n", name); >- state = -2; >- break; >- } >- break; >+ } >+ vpd->vpd_rocnt++; > >- case 1: /* Identifier String */ >- cfg->vpd.vpd_ident[i++] = byte; >- remain--; >- if (remain == 0) { >- cfg->vpd.vpd_ident[i] = '\0'; >- state = 0; >- } >- break; >+ return (len + 3); >+} > >- case 2: /* VPD-R Keyword Header */ >- if (off == alloc) { >- cfg->vpd.vpd_ros = reallocf(cfg->vpd.vpd_ros, >- (alloc *= 2) * sizeof(*cfg->vpd.vpd_ros), >- M_DEVBUF, M_WAITOK | M_ZERO); >- } >- cfg->vpd.vpd_ros[off].keyword[0] = byte; >- if (vpd_nextbyte(&vrs, &byte2)) { >- state = -2; >- break; >- } >- cfg->vpd.vpd_ros[off].keyword[1] = byte2; >- if (vpd_nextbyte(&vrs, &byte2)) { >- state = -2; >- break; >- } >- cfg->vpd.vpd_ros[off].len = dflen = byte2; >- if (dflen == 0 && >- strncmp(cfg->vpd.vpd_ros[off].keyword, "RV", >- 2) == 0) { >- /* >- * if this happens, we can't trust the rest >- * of the VPD. >- */ >- pci_printf(cfg, "invalid VPD RV record"); >- cksumvalid = 0; >- state = -1; >- break; >- } else if (dflen == 0) { >- cfg->vpd.vpd_ros[off].value = malloc(1 * >- sizeof(*cfg->vpd.vpd_ros[off].value), >- M_DEVBUF, M_WAITOK); >- cfg->vpd.vpd_ros[off].value[0] = '\x00'; >- } else >- cfg->vpd.vpd_ros[off].value = malloc( >- (dflen + 1) * >- sizeof(*cfg->vpd.vpd_ros[off].value), >- M_DEVBUF, M_WAITOK); >- remain -= 3; >- i = 0; >- /* keep in sync w/ state 3's transitions */ >- if (dflen == 0 && remain == 0) >- state = 0; >- else if (dflen == 0) >- state = 2; >- else >- state = 3; >- break; >+/* fetch one writable element and return size of heading + data */ >+static size_t >+next_vpd_rw_elem(struct vpd_readstate *vrs, int maxsize) >+{ >+ struct pcicfg_vpd *vpd; >+ pcicfgregs *cfg; >+ struct vpd_write *vpd_w; >+ int len; > >- case 3: /* VPD-R Keyword Value */ >- cfg->vpd.vpd_ros[off].value[i++] = byte; >- if (strncmp(cfg->vpd.vpd_ros[off].keyword, >- "RV", 2) == 0 && cksumvalid == -1) { >- if (vrs.cksum == 0) >- cksumvalid = 1; >- else { >- if (bootverbose) >- pci_printf(cfg, >- "bad VPD cksum, remain %hhu\n", >- vrs.cksum); >- cksumvalid = 0; >- state = -1; >- break; >- } >- } >- dflen--; >- remain--; >- /* keep in sync w/ state 2's transitions */ >- if (dflen == 0) >- cfg->vpd.vpd_ros[off++].value[i++] = '\0'; >- if (dflen == 0 && remain == 0) { >- cfg->vpd.vpd_rocnt = off; >- cfg->vpd.vpd_ros = reallocf(cfg->vpd.vpd_ros, >- off * sizeof(*cfg->vpd.vpd_ros), >- M_DEVBUF, M_WAITOK | M_ZERO); >- state = 0; >- } else if (dflen == 0) >- state = 2; >- break; >+ cfg = vrs->cfg; >+ vpd = &cfg->vpd; > >- case 4: >- remain--; >- if (remain == 0) >- state = 0; >- break; >+ if (maxsize < 3) >+ return (-1); >+ vpd->vpd_w = alloc_buffer(vpd->vpd_w, sizeof(*vpd->vpd_w), vpd->vpd_wcnt); >+ if (vpd->vpd_w == NULL) { >+ pci_printf(cfg, "out of memory"); >+ return (-1); >+ } >+ vpd_w = &vpd->vpd_w[vpd->vpd_wcnt]; >+ maxsize -= 3; >+ vpd_w->start = vrs->off + 3 - vrs->bytesinval; >+ len = vpd_read_elem_data(vrs, vpd_w->keyword, &vpd_w->value, maxsize); >+ if (vpd_w->value == NULL) >+ return (-1); >+ vpd_w->len = len; >+ vpd->vpd_wcnt++; > >- case 5: /* VPD-W Keyword Header */ >- if (off == alloc) { >- cfg->vpd.vpd_w = reallocf(cfg->vpd.vpd_w, >- (alloc *= 2) * sizeof(*cfg->vpd.vpd_w), >- M_DEVBUF, M_WAITOK | M_ZERO); >- } >- cfg->vpd.vpd_w[off].keyword[0] = byte; >- if (vpd_nextbyte(&vrs, &byte2)) { >- state = -2; >- break; >- } >- cfg->vpd.vpd_w[off].keyword[1] = byte2; >- if (vpd_nextbyte(&vrs, &byte2)) { >- state = -2; >- break; >- } >- cfg->vpd.vpd_w[off].len = dflen = byte2; >- cfg->vpd.vpd_w[off].start = vrs.off - vrs.bytesinval; >- cfg->vpd.vpd_w[off].value = malloc((dflen + 1) * >- sizeof(*cfg->vpd.vpd_w[off].value), >- M_DEVBUF, M_WAITOK); >- remain -= 3; >- i = 0; >- /* keep in sync w/ state 6's transitions */ >- if (dflen == 0 && remain == 0) >- state = 0; >- else if (dflen == 0) >- state = 5; >- else >- state = 6; >- break; >+ return (len + 3); >+} > >- case 6: /* VPD-W Keyword Value */ >- cfg->vpd.vpd_w[off].value[i++] = byte; >- dflen--; >- remain--; >- /* keep in sync w/ state 5's transitions */ >- if (dflen == 0) >- cfg->vpd.vpd_w[off++].value[i++] = '\0'; >- if (dflen == 0 && remain == 0) { >- cfg->vpd.vpd_wcnt = off; >- cfg->vpd.vpd_w = reallocf(cfg->vpd.vpd_w, >- off * sizeof(*cfg->vpd.vpd_w), >- M_DEVBUF, M_WAITOK | M_ZERO); >- state = 0; >- } else if (dflen == 0) >- state = 5; >- break; >+/* free all memory allocated for VPD data */ >+static void >+vpd_free(struct pcicfg_vpd *vpd) >+{ >+ int i; > >- default: >- pci_printf(cfg, "invalid state: %d\n", state); >- state = -1; >- break; >- } >+ free(vpd->vpd_ident, M_DEVBUF); >+ for (i = 0; i < vpd->vpd_rocnt; i++) >+ free(vpd->vpd_ros[i].value, M_DEVBUF); >+ free(vpd->vpd_ros, M_DEVBUF); >+ vpd->vpd_rocnt = 0; >+ for (i = 0; i < vpd->vpd_wcnt; i++) >+ free(vpd->vpd_w[i].value, M_DEVBUF); >+ free(vpd->vpd_w, M_DEVBUF); >+ vpd->vpd_wcnt = 0; >+} > >- if (cfg->vpd.vpd_ident == NULL || cfg->vpd.vpd_ident[0] == '\0') { >- pci_printf(cfg, "no valid vpd ident found\n"); >- state = -2; >- } >+#define VPD_TAG_END ((0x0f << 3) | 0) /* small tag, len == 0 */ >+#define VPD_TAG_IDENT (0x02 | 0x80) /* large tag */ >+#define VPD_TAG_RO (0x10 | 0x80) /* large tag */ >+#define VPD_TAG_RW (0x11 | 0x80) /* large tag */ >+ >+static int >+pci_parse_vpd(device_t pcib, pcicfgregs *cfg) >+{ >+ struct vpd_readstate vrs; >+ int cksumvalid; >+ int size, elem_size; >+ >+ /* init vpd reader */ >+ vrs.bytesinval = 0; >+ vrs.off = 0; >+ vrs.pcib = pcib; >+ vrs.cfg = cfg; >+ vrs.cksum = 0; >+ >+ /* read VPD ident element - mandatory */ >+ size = vpd_read_tag_size(&vrs, VPD_TAG_IDENT); >+ if (size <= 0) { >+ pci_printf(cfg, "no VPD ident found\n"); >+ return (0); >+ } >+ cfg->vpd.vpd_ident = vpd_read_value(&vrs, size); >+ if (cfg->vpd.vpd_ident == NULL) { >+ pci_printf(cfg, "error accessing VPD ident data\n"); >+ return (0); > } > >- if (cksumvalid <= 0 || state < -1) { >- /* read-only data bad, clean up */ >- if (cfg->vpd.vpd_ros != NULL) { >- for (off = 0; cfg->vpd.vpd_ros[off].value; off++) >- free(cfg->vpd.vpd_ros[off].value, M_DEVBUF); >- free(cfg->vpd.vpd_ros, M_DEVBUF); >- cfg->vpd.vpd_ros = NULL; >- } >+ /* read VPD RO elements - mandatory */ >+ size = vpd_read_tag_size(&vrs, VPD_TAG_RO); >+ if (size <= 0) { >+ pci_printf(cfg, "no read-only VPD data found\n"); >+ return (0); > } >- if (state < -1) { >- /* I/O error, clean up */ >- pci_printf(cfg, "failed to read VPD data.\n"); >- if (cfg->vpd.vpd_ident != NULL) { >- free(cfg->vpd.vpd_ident, M_DEVBUF); >- cfg->vpd.vpd_ident = NULL; >+ while (size > 0) { >+ elem_size = next_vpd_ro_elem(&vrs, size); >+ if (elem_size < 0) { >+ pci_printf(cfg, "error accessing read-only VPD data\n"); >+ return (-1); > } >- if (cfg->vpd.vpd_w != NULL) { >- for (off = 0; cfg->vpd.vpd_w[off].value; off++) >- free(cfg->vpd.vpd_w[off].value, M_DEVBUF); >- free(cfg->vpd.vpd_w, M_DEVBUF); >- cfg->vpd.vpd_w = NULL; >+ size -= elem_size; >+ } >+ cksumvalid = (vrs.cksum == 0); >+ if (!cksumvalid) >+ return (-1); >+ >+ /* read VPD RW elements - optional */ >+ size = vpd_read_tag_size(&vrs, VPD_TAG_RW); >+ if (size == -2) >+ return (-1); >+ while (size > 0) { >+ elem_size = next_vpd_rw_elem(&vrs, size); >+ if (elem_size < 0) { >+ pci_printf(cfg, "error accessing writeable VPD data\n"); >+ return (-1); > } >+ size -= elem_size; > } >+ >+ /* read empty END tag - mandatory */ >+ size = vpd_read_tag_size(&vrs, VPD_TAG_END); >+ if (size != 0) { >+ pci_printf(cfg, "No valid VPD end tag found\n"); >+ } >+ return (0); >+} >+ >+static void >+pci_read_vpd(device_t pcib, pcicfgregs *cfg) >+{ >+ int status; >+ >+ status = pci_parse_vpd(pcib, cfg); >+ if (status < 0) >+ vpd_free(&cfg->vpd); > cfg->vpd.vpd_cached = 1; > #undef REG > #undef WREG >@@ -2777,19 +2759,12 @@ pci_freecfg(struct pci_devinfo *dinfo) > { > struct devlist *devlist_head; > struct pci_map *pm, *next; >- int i; > > devlist_head = &pci_devq; > >- if (dinfo->cfg.vpd.vpd_reg) { >- free(dinfo->cfg.vpd.vpd_ident, M_DEVBUF); >- for (i = 0; i < dinfo->cfg.vpd.vpd_rocnt; i++) >- free(dinfo->cfg.vpd.vpd_ros[i].value, M_DEVBUF); >- free(dinfo->cfg.vpd.vpd_ros, M_DEVBUF); >- for (i = 0; i < dinfo->cfg.vpd.vpd_wcnt; i++) >- free(dinfo->cfg.vpd.vpd_w[i].value, M_DEVBUF); >- free(dinfo->cfg.vpd.vpd_w, M_DEVBUF); >- } >+ if (dinfo->cfg.vpd.vpd_reg) >+ vpd_free(&dinfo->cfg.vpd); >+ > STAILQ_FOREACH_SAFE(pm, &dinfo->cfg.maps, pm_link, next) { > free(pm, M_DEVBUF); > }
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
Attachments on
bug 272018
:
242810
|
242927
| 242928