diff --git a/sys/dev/pci/pci.c b/sys/dev/pci/pci.c
--- a/sys/dev/pci/pci.c
+++ b/sys/dev/pci/pci.c
@@ -1063,6 +1063,7 @@
 	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 @@
 
 	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,303 +1088,284 @@
 	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 @@
 {
 	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);
 	}