All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Scull <ascull@google.com>
To: u-boot@lists.denx.de
Cc: sjg@chromium.org, bmeng.cn@gmail.com, adelva@google.com,
	keirf@google.com,  ptosi@google.com,
	Andrew Scull <ascull@google.com>
Subject: [PATCH 06/11] virtio: pci: Read entire capability into memory
Date: Sun, 20 Mar 2022 11:41:13 +0000	[thread overview]
Message-ID: <20220320114118.2237795-7-ascull@google.com> (raw)
In-Reply-To: <20220320114118.2237795-1-ascull@google.com>

Read the virtio PCI capability out of the device configuration space to
a struct rather than accessing fields directly from the configuration
space as they are needed. This both makes access to the fields easier
and avoids re-reading fields.

Re-reading fields could result in time-of-check to time-of-use problems,
should the value in the configuration space change. The range check of
the `bar` field and the later call to `dm_pci_read_bar32()` is an
example of where this could happen.

Signed-off-by: Andrew Scull <ascull@google.com>
---
 drivers/virtio/virtio_pci_modern.c | 74 ++++++++++++++++--------------
 1 file changed, 40 insertions(+), 34 deletions(-)

diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c
index 4b346be257..38a0da0a84 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -393,15 +393,16 @@ static int virtio_pci_notify(struct udevice *udev, struct virtqueue *vq)
  * @udev:	the transport device
  * @cfg_type:	the VIRTIO_PCI_CAP_* value we seek
  * @cap_size:	expected size of the capability
+ * @cap:	capability read from the config space
  *
  * Return: offset of the configuration structure
  */
 static int virtio_pci_find_capability(struct udevice *udev, u8 cfg_type,
-				      size_t cap_size)
+				      size_t cap_size,
+				      struct virtio_pci_cap *cap)
 {
 	int pos;
 	int offset;
-	u8 type, bar;
 
 	if (cap_size < sizeof(struct virtio_pci_cap))
 		return 0;
@@ -409,6 +410,9 @@ static int virtio_pci_find_capability(struct udevice *udev, u8 cfg_type,
 	if (cap_size > PCI_CFG_SPACE_SIZE)
 		return 0;
 
+	if (!cap)
+		return 0;
+
 	for (pos = dm_pci_find_capability(udev, PCI_CAP_ID_VNDR);
 	     pos > 0;
 	     pos = dm_pci_find_next_capability(udev, pos, PCI_CAP_ID_VNDR)) {
@@ -416,16 +420,26 @@ static int virtio_pci_find_capability(struct udevice *udev, u8 cfg_type,
 		if (PCI_CFG_SPACE_SIZE - cap_size < pos)
 			return 0;
 
+		offset = pos + offsetof(struct virtio_pci_cap, cap_vndr);
+		dm_pci_read_config8(udev, offset, &cap->cap_vndr);
+		offset = pos + offsetof(struct virtio_pci_cap, cap_next);
+		dm_pci_read_config8(udev, offset, &cap->cap_next);
+		offset = pos + offsetof(struct virtio_pci_cap, cap_len);
+		dm_pci_read_config8(udev, offset, &cap->cap_len);
 		offset = pos + offsetof(struct virtio_pci_cap, cfg_type);
-		dm_pci_read_config8(udev, offset, &type);
+		dm_pci_read_config8(udev, offset, &cap->cfg_type);
 		offset = pos + offsetof(struct virtio_pci_cap, bar);
-		dm_pci_read_config8(udev, offset, &bar);
+		dm_pci_read_config8(udev, offset, &cap->bar);
+		offset = pos + offsetof(struct virtio_pci_cap, offset);
+		dm_pci_read_config32(udev, offset, &cap->offset);
+		offset = pos + offsetof(struct virtio_pci_cap, length);
+		dm_pci_read_config32(udev, offset, &cap->length);
 
 		/* Ignore structures with reserved BAR values */
-		if (bar > 0x5)
+		if (cap->bar > 0x5)
 			continue;
 
-		if (type == cfg_type)
+		if (cap->cfg_type == cfg_type)
 			return pos;
 	}
 
@@ -436,33 +450,27 @@ static int virtio_pci_find_capability(struct udevice *udev, u8 cfg_type,
  * virtio_pci_map_capability - map base address of the capability
  *
  * @udev:	the transport device
- * @off:	offset of the configuration structure
+ * @cap:	capability to map
  *
  * Return: base address of the capability
  */
-static void __iomem *virtio_pci_map_capability(struct udevice *udev, int off)
+static void __iomem *virtio_pci_map_capability(struct udevice *udev,
+					       const struct virtio_pci_cap *cap)
 {
-	u8 bar;
-	u32 offset;
 	ulong base;
 	void __iomem *p;
 
-	if (!off)
+	if (!cap)
 		return NULL;
 
-	offset = off + offsetof(struct virtio_pci_cap, bar);
-	dm_pci_read_config8(udev, offset, &bar);
-	offset = off + offsetof(struct virtio_pci_cap, offset);
-	dm_pci_read_config32(udev, offset, &offset);
-
 	/*
 	 * TODO: adding 64-bit BAR support
 	 *
 	 * Per spec, the BAR is permitted to be either 32-bit or 64-bit.
 	 * For simplicity, only read the BAR address as 32-bit.
 	 */
-	base = dm_pci_read_bar32(udev, bar);
-	p = (void __iomem *)base + offset;
+	base = dm_pci_read_bar32(udev, cap->bar);
+	p = (void __iomem *)base + cap->offset;
 
 	return p;
 }
@@ -487,7 +495,7 @@ static int virtio_pci_probe(struct udevice *udev)
 	u16 subvendor;
 	u8 revision;
 	int common, notify, device;
-	u32 common_length;
+	struct virtio_pci_cap common_cap, notify_cap, device_cap;
 	int offset;
 
 	/* We only own devices >= 0x1040 and <= 0x107f: leave the rest. */
@@ -504,46 +512,44 @@ static int virtio_pci_probe(struct udevice *udev)
 
 	/* Check for a common config: if not, use legacy mode (bar 0) */
 	common = virtio_pci_find_capability(udev, VIRTIO_PCI_CAP_COMMON_CFG,
-					    sizeof(struct virtio_pci_cap));
+					    sizeof(struct virtio_pci_cap),
+					    &common_cap);
 	if (!common) {
 		printf("(%s): leaving for legacy driver\n", udev->name);
 		return -ENODEV;
 	}
 
-	offset = common + offsetof(struct virtio_pci_cap, length);
-	dm_pci_read_config32(udev, offset, &common_length);
-	if (common_length < sizeof(struct virtio_pci_common_cfg)) {
+	if (common_cap.length < sizeof(struct virtio_pci_common_cfg)) {
 		printf("(%s): virtio common config too small\n", udev->name);
 		return -EINVAL;
 	}
 
 	/* If common is there, notify should be too */
 	notify = virtio_pci_find_capability(udev, VIRTIO_PCI_CAP_NOTIFY_CFG,
-					    sizeof(struct virtio_pci_notify_cap));
+					    sizeof(struct virtio_pci_notify_cap),
+					    &notify_cap);
 	if (!notify) {
 		printf("(%s): missing capabilities %i/%i\n", udev->name,
 		       common, notify);
 		return -EINVAL;
 	}
 
-	offset = notify + offsetof(struct virtio_pci_cap, length);
-	dm_pci_read_config32(udev, offset, &priv->notify_len);
+	priv->notify_len = notify_cap.length;
 
 	/*
 	 * Device capability is only mandatory for devices that have
 	 * device-specific configuration.
 	 */
 	device = virtio_pci_find_capability(udev, VIRTIO_PCI_CAP_DEVICE_CFG,
-					    sizeof(struct virtio_pci_cap));
-	if (device) {
-		offset = device + offsetof(struct virtio_pci_cap, length);
-		dm_pci_read_config32(udev, offset, &priv->device_len);
-	}
+					    sizeof(struct virtio_pci_cap),
+					    &device_cap);
+	if (device)
+		priv->device_len = device_cap.length;
 
 	/* Map configuration structures */
-	priv->common = virtio_pci_map_capability(udev, common);
-	priv->notify_base = virtio_pci_map_capability(udev, notify);
-	priv->device = virtio_pci_map_capability(udev, device);
+	priv->common = virtio_pci_map_capability(udev, &common_cap);
+	priv->notify_base = virtio_pci_map_capability(udev, &notify_cap);
+	priv->device = virtio_pci_map_capability(udev, &device_cap);
 	debug("(%p): common @ %p, notify base @ %p, device @ %p\n",
 	      udev, priv->common, priv->notify_base, priv->device);
 
-- 
2.35.1.894.gb6a874cedc-goog


  parent reply	other threads:[~2022-03-20 11:44 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-20 11:41 [PATCH 00/11] virtio: pci: Add and fix consistency checks Andrew Scull
2022-03-20 11:41 ` [PATCH 01/11] virtio: pci: Fix discovery of device config length Andrew Scull
2022-03-24  9:32   ` Bin Meng
2022-03-20 11:41 ` [PATCH 02/11] virtio: pci: Bounds check device config access Andrew Scull
2022-03-24 14:07   ` Bin Meng
2022-03-20 11:41 ` [PATCH 03/11] virtio: pci: Bounds check notification writes Andrew Scull
2022-03-24 14:18   ` Bin Meng
2022-03-24 16:24     ` Andrew Scull
2022-03-25  1:41       ` Bin Meng
2022-03-20 11:41 ` [PATCH 04/11] virtio: pci: Check virtio common config size Andrew Scull
2022-03-24 14:22   ` Bin Meng
2022-03-20 11:41 ` [PATCH 05/11] virtio: pci: Check virtio capability is in bounds Andrew Scull
2022-03-24 15:24   ` Bin Meng
2022-03-24 16:27     ` Andrew Scull
2022-03-25  1:27       ` Bin Meng
2022-03-20 11:41 ` Andrew Scull [this message]
2022-03-25  4:31   ` [PATCH 06/11] virtio: pci: Read entire capability into memory Bin Meng
2022-03-25  7:03     ` Andrew Scull
2022-03-25  7:51       ` Bin Meng
2022-03-25  9:18         ` Andrew Scull
2022-03-25 10:25           ` Bin Meng
2022-03-28 14:28             ` Andrew Scull
2022-03-20 11:41 ` [PATCH 07/11] virtio: pci: Check virtio configs are mapped Andrew Scull
2022-03-25  4:38   ` Bin Meng
2022-03-25  7:07     ` Andrew Scull
2022-03-25  7:19       ` Bin Meng
2022-03-20 11:41 ` [PATCH 08/11] pci: Check region ranges are addressable Andrew Scull
2022-03-25  7:14   ` Bin Meng
2022-03-20 11:41 ` [PATCH 09/11] pci: Add function to validate PCI address range Andrew Scull
2022-03-25  7:14   ` Bin Meng
2022-03-25 10:26     ` Andrew Scull
2022-03-20 11:41 ` [PATCH 10/11] virtio: pci: Check mapped range is in a PCI region Andrew Scull
2022-03-25  7:14   ` Bin Meng
2022-03-20 11:41 ` [PATCH 11/11] virtio: pci: Allow exclusion of legacy driver Andrew Scull
2022-03-25  7:14   ` Bin Meng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220320114118.2237795-7-ascull@google.com \
    --to=ascull@google.com \
    --cc=adelva@google.com \
    --cc=bmeng.cn@gmail.com \
    --cc=keirf@google.com \
    --cc=ptosi@google.com \
    --cc=sjg@chromium.org \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.