Attachment 242810 Details for Bug 272018 – Simpiified VPD parser

[patch] Simpiified VPD parser

pci-VPD.diff (text/plain), 15.27 KB, created by Stefan Eßer on 2023-06-16 15:04:07 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Stefan Eßer

Created: 2023-06-16 15:04:07 UTC

Size: 15.27 KB

patch

obsolete

>diff --git a/sys/dev/pci/pci.c b/sys/dev/pci/pci.c
>index 24348ecf14eb..cd91bd42ef3f 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, &reg))
>-			return (ENXIO);
>+			return (-1);
> 		vrs->val = le32toh(reg);
> 		vrs->off += 4;
> 		byte = vrs->val & 0xff;
>@@ -1087,307 +1088,296 @@ 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;
>+
>+	if (size < 0 || size > 255) // maximum element size supported is 255
>+		return (NULL);
>+	value = malloc(size + 1, M_DEVBUF, M_WAITOK);
>+	if (value == NULL)
>+		return (NULL);
>+	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);
>+	if (vpd->vpd_ros == NULL) {
>+		pci_printf(cfg, "out of memory");
>+		return (-1);
>+	}
>+	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
>-}
> 
> int
> pci_get_vpd_ident_method(device_t dev, device_t child, const char **identptr)
>@@ -2777,19 +2767,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);
> 	}

Actions: View | Diff

Attachments on bug 272018: 242810 | 242927 | 242928