From e7870c2cdccef3a3a8659995407280900323b9f5 Mon Sep 17 00:00:00 2001 From: Andrew Scull Date: Thu, 21 Apr 2022 16:11:05 +0000 Subject: [PATCH] virtio: pci: Read entire capability into memory 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 Reviewed-by: Bin Meng --- drivers/virtio/virtio_pci_modern.c | 72 ++++++++++++++++-------------- 1 file changed, 38 insertions(+), 34 deletions(-) diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c index 6fe5e76572..bf92d9d3ba 100644 --- a/drivers/virtio/virtio_pci_modern.c +++ b/drivers/virtio/virtio_pci_modern.c @@ -398,19 +398,23 @@ 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; assert(cap_size >= sizeof(struct virtio_pci_cap)); assert(cap_size <= PCI_CFG_SPACE_SIZE); + 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)) { @@ -418,16 +422,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; } @@ -438,33 +452,24 @@ 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) - 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; } @@ -489,7 +494,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. */ @@ -506,46 +511,45 @@ 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), + ¬ify_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)); + sizeof(struct virtio_pci_cap), + &device_cap); if (device) { - offset = device + offsetof(struct virtio_pci_cap, length); - dm_pci_read_config32(udev, offset, &priv->device_len); + priv->device_len = device_cap.length; + priv->device = virtio_pci_map_capability(udev, &device_cap); } /* 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, ¬ify_cap); debug("(%p): common @ %p, notify base @ %p, device @ %p\n", udev, priv->common, priv->notify_base, priv->device); -- 2.39.5