[PATCH v2 00/18] virtio: pci: Add and fix consistency checks

The virtio PCI drivers forgo a number of consistency checks, particularly around pointer validation and bounds checking. This series focuses on the modern driver to add those checks.
The start of the series adds and fixes some basic bounds checks. Later patches ensure PCI addresses fall within the expected regions rather than any arbitrary address. This is acheived by introducing range parameters to a few of the dm_pci_* functions that allow the ranges to be checked.
The series also adds a few new configs to allow parts of virtio and PCI to be disabled where the features may be unused and the current implementations don't have the needed consistencty checks.
Changelog: v2: - Refactor dm_pci_* functions, not adding new ones - Reorder patches per request in v1
Andrew Scull (18): virtio: pci: Allow exclusion of legacy driver virtio: pci: Fix discovery of device config length virtio: pci: Bounds check device config access virtio: pci: Bounds check notification writes virtio: pci: Check virtio common config size virtio: pci: Check virtio capability is in bounds virtio: pci: Read entire capability into memory pci: Fix use of flags in dm_pci_map_bar() pci: Add config for Enhanced Allocation pci: Check region ranges are addressable pci: Range check address conversions test: pci: Test PCI address conversion functions pci: Map bars with offset and length pci: Match region flags using a mask pci: Update dm_pci_bus_to_virt() parameters pci: Add mask parameter to dm_pci_map_bar() virtio: pci: Check virtio configs are mapped virtio: pci: Make use of dm_pci_map_bar()
arch/sandbox/dts/test.dts | 2 +- arch/x86/cpu/baytrail/cpu.c | 2 +- drivers/ata/ahci.c | 9 +- drivers/bios_emulator/atibios.c | 4 +- drivers/gpio/octeon_gpio.c | 2 +- drivers/i2c/designware_i2c_pci.c | 3 +- drivers/i2c/intel_i2c.c | 2 +- drivers/i2c/octeon_i2c.c | 2 +- drivers/mmc/octeontx_hsmmc.c | 2 +- drivers/mmc/pci_mmc.c | 2 +- drivers/mtd/nand/raw/octeontx_bch.c | 6 +- drivers/mtd/nand/raw/octeontx_nand.c | 2 +- drivers/net/bnxt/bnxt.c | 9 +- drivers/net/fsl_enetc.c | 2 +- drivers/net/fsl_enetc_mdio.c | 2 +- drivers/net/mscc_eswitch/felix_switch.c | 4 +- drivers/net/octeontx/bgx.c | 2 +- drivers/net/octeontx/nic_main.c | 2 +- drivers/net/octeontx/nicvf_main.c | 2 +- drivers/net/octeontx/smi.c | 2 +- drivers/net/octeontx2/cgx.c | 2 +- drivers/net/octeontx2/rvu_af.c | 2 +- drivers/net/octeontx2/rvu_pf.c | 3 +- drivers/net/pch_gbe.c | 2 +- drivers/nvme/nvme_pci.c | 4 +- drivers/pci/Kconfig | 7 + drivers/pci/pci-uclass.c | 197 +++++++++++------------- drivers/spi/octeon_spi.c | 2 +- drivers/usb/host/ohci-pci.c | 2 +- drivers/virtio/Kconfig | 9 ++ drivers/virtio/Makefile | 3 +- drivers/virtio/virtio_pci_legacy.c | 3 +- drivers/virtio/virtio_pci_modern.c | 143 +++++++++++------ include/pci.h | 49 ++++-- test/dm/pci.c | 118 +++++++++++++- 35 files changed, 399 insertions(+), 210 deletions(-)

Add a new config to control whether the driver for legacy virtio PCI devices is included in the build. VIRTIO_PCI_LEGACY is included by default when VIRTIO_PCI is selected, but it can also be independently toggled.
Signed-off-by: Andrew Scull ascull@google.com Reviewed-by: Bin Meng bmeng.cn@gmail.com --- drivers/virtio/Kconfig | 9 +++++++++ drivers/virtio/Makefile | 3 ++- 2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/virtio/Kconfig b/drivers/virtio/Kconfig index 863c3fbe02..586263ec88 100644 --- a/drivers/virtio/Kconfig +++ b/drivers/virtio/Kconfig @@ -37,6 +37,15 @@ config VIRTIO_PCI This driver provides support for virtio based paravirtual device drivers over PCI.
+config VIRTIO_PCI_LEGACY + bool "PCI driver for legacy virtio devices" + depends on PCI + select VIRTIO + default VIRTIO_PCI + help + This driver provides support for legacy virtio based paravirtual + device drivers over PCI. + config VIRTIO_SANDBOX bool "Sandbox driver for virtio devices" depends on SANDBOX diff --git a/drivers/virtio/Makefile b/drivers/virtio/Makefile index dc8880937a..4c63a6c690 100644 --- a/drivers/virtio/Makefile +++ b/drivers/virtio/Makefile @@ -5,7 +5,8 @@
obj-y += virtio-uclass.o virtio_ring.o obj-$(CONFIG_VIRTIO_MMIO) += virtio_mmio.o -obj-$(CONFIG_VIRTIO_PCI) += virtio_pci_legacy.o virtio_pci_modern.o +obj-$(CONFIG_VIRTIO_PCI) += virtio_pci_modern.o +obj-$(CONFIG_VIRTIO_PCI_LEGACY) += virtio_pci_legacy.o obj-$(CONFIG_VIRTIO_SANDBOX) += virtio_sandbox.o obj-$(CONFIG_VIRTIO_NET) += virtio_net.o obj-$(CONFIG_VIRTIO_BLK) += virtio_blk.o

The length of the device config was erroneously being taken from the notify capability. Correct this by finding the length in the device capability.
Fixes: 550435edf810 ("virtio: pci: Support non-legacy PCI transport device") Signed-off-by: Andrew Scull ascull@google.com Reviewed-by: Bin Meng bmeng.cn@gmail.com --- drivers/virtio/virtio_pci_modern.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c index fd8a1f3fec..55d25cb81b 100644 --- a/drivers/virtio/virtio_pci_modern.c +++ b/drivers/virtio/virtio_pci_modern.c @@ -497,7 +497,7 @@ static int virtio_pci_probe(struct udevice *udev) */ device = virtio_pci_find_capability(udev, VIRTIO_PCI_CAP_DEVICE_CFG); if (device) { - offset = notify + offsetof(struct virtio_pci_cap, length); + offset = device + offsetof(struct virtio_pci_cap, length); dm_pci_read_config32(udev, offset, &priv->device_len); }

The device config is optional, so check it was present and mapped before trying to use the pointer. Bounds violations are an error, not just a warning, so bail if the checks fail.
Signed-off-by: Andrew Scull ascull@google.com Reviewed-by: Bin Meng bmeng.cn@gmail.com --- drivers/virtio/virtio_pci_modern.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c index 55d25cb81b..bcf9f18997 100644 --- a/drivers/virtio/virtio_pci_modern.c +++ b/drivers/virtio/virtio_pci_modern.c @@ -114,7 +114,11 @@ static int virtio_pci_get_config(struct udevice *udev, unsigned int offset, __le16 w; __le32 l;
- WARN_ON(offset + len > priv->device_len); + if (!priv->device) + return -ENOSYS; + + if (offset + len > priv->device_len) + return -EINVAL;
switch (len) { case 1: @@ -136,7 +140,7 @@ static int virtio_pci_get_config(struct udevice *udev, unsigned int offset, memcpy(buf + sizeof(l), &l, sizeof(l)); break; default: - WARN_ON(true); + return -EINVAL; }
return 0; @@ -150,7 +154,11 @@ static int virtio_pci_set_config(struct udevice *udev, unsigned int offset, __le16 w; __le32 l;
- WARN_ON(offset + len > priv->device_len); + if (!priv->device) + return -ENOSYS; + + if (offset + len > priv->device_len) + return -EINVAL;
switch (len) { case 1: @@ -172,7 +180,7 @@ static int virtio_pci_set_config(struct udevice *udev, unsigned int offset, iowrite32(le32_to_cpu(l), priv->device + offset + sizeof(l)); break; default: - WARN_ON(true); + return -EINVAL; }
return 0;

Make sure virtio notifications are written within their allocated buffer.
Signed-off-by: Andrew Scull ascull@google.com Reviewed-by: Bin Meng bmeng.cn@gmail.com --- drivers/virtio/virtio_pci_modern.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c index bcf9f18997..7dd58aa0f4 100644 --- a/drivers/virtio/virtio_pci_modern.c +++ b/drivers/virtio/virtio_pci_modern.c @@ -94,6 +94,7 @@ * * @common: pci transport device common register block base * @notify_base: pci transport device notify register block base + * @notify_len: pci transport device notify register block length * @device: pci transport device device-specific register block base * @device_len: pci transport device device-specific register block length * @notify_offset_multiplier: multiply queue_notify_off by this value @@ -101,6 +102,7 @@ struct virtio_pci_priv { struct virtio_pci_common_cfg __iomem *common; void __iomem *notify_base; + u32 notify_len; void __iomem *device; u32 device_len; u32 notify_offset_multiplier; @@ -372,12 +374,20 @@ static int virtio_pci_notify(struct udevice *udev, struct virtqueue *vq) /* get offset of notification word for this vq */ off = ioread16(&priv->common->queue_notify_off);
+ /* + * Check the effective offset is in bounds and leaves space for the + * notification, which is just a single 16-bit value since + * VIRTIO_F_NOTIFICATION_DATA isn't negotiated by the drivers. + */ + off *= priv->notify_offset_multiplier; + if (off > priv->notify_len - sizeof(u16)) + return -EIO; + /* * We write the queue's selector into the notification register * to signal the other end */ - iowrite16(vq->index, - priv->notify_base + off * priv->notify_offset_multiplier); + iowrite16(vq->index, priv->notify_base + off);
return 0; } @@ -499,6 +509,9 @@ static int virtio_pci_probe(struct udevice *udev) return -EINVAL; }
+ offset = notify + offsetof(struct virtio_pci_cap, length); + dm_pci_read_config32(udev, offset, &priv->notify_len); + /* * Device capability is only mandatory for devices that have * device-specific configuration.

Check that the common config is at least as large as the struct it is expected to contain. Only then is it safe to cast the pointer and be safe from out-of-bounds accesses.
Signed-off-by: Andrew Scull ascull@google.com Reviewed-by: Bin Meng bmeng.cn@gmail.com --- drivers/virtio/virtio_pci_modern.c | 8 ++++++++ 1 file changed, 8 insertions(+)
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c index 7dd58aa0f4..2c1b0ebfce 100644 --- a/drivers/virtio/virtio_pci_modern.c +++ b/drivers/virtio/virtio_pci_modern.c @@ -480,6 +480,7 @@ static int virtio_pci_probe(struct udevice *udev) u16 subvendor; u8 revision; int common, notify, device; + u32 common_length; int offset;
/* We only own devices >= 0x1040 and <= 0x107f: leave the rest. */ @@ -501,6 +502,13 @@ static int virtio_pci_probe(struct udevice *udev) 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)) { + 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); if (!notify) {

Ensure the virtio PCI capabilities are contained within the bounds of the device's configuration space. The expected size of the capability is passed when searching for the capability to enforce this check.
Signed-off-by: Andrew Scull ascull@google.com --- drivers/virtio/virtio_pci_modern.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c index 2c1b0ebfce..6fe5e76572 100644 --- a/drivers/virtio/virtio_pci_modern.c +++ b/drivers/virtio/virtio_pci_modern.c @@ -397,18 +397,27 @@ 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 * * Return: offset of the configuration structure */ -static int virtio_pci_find_capability(struct udevice *udev, u8 cfg_type) +static int virtio_pci_find_capability(struct udevice *udev, u8 cfg_type, + size_t cap_size) { int pos; int offset; u8 type, bar;
+ assert(cap_size >= sizeof(struct virtio_pci_cap)); + assert(cap_size <= PCI_CFG_SPACE_SIZE); + 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)) { + /* Ensure the capability is within bounds */ + if (PCI_CFG_SPACE_SIZE - cap_size < pos) + return 0; + offset = pos + offsetof(struct virtio_pci_cap, cfg_type); dm_pci_read_config8(udev, offset, &type); offset = pos + offsetof(struct virtio_pci_cap, bar); @@ -496,7 +505,8 @@ static int virtio_pci_probe(struct udevice *udev) uc_priv->vendor = subvendor;
/* Check for a common config: if not, use legacy mode (bar 0) */ - common = virtio_pci_find_capability(udev, VIRTIO_PCI_CAP_COMMON_CFG); + common = virtio_pci_find_capability(udev, VIRTIO_PCI_CAP_COMMON_CFG, + sizeof(struct virtio_pci_cap)); if (!common) { printf("(%s): leaving for legacy driver\n", udev->name); return -ENODEV; @@ -510,7 +520,8 @@ static int virtio_pci_probe(struct udevice *udev) }
/* If common is there, notify should be too */ - notify = virtio_pci_find_capability(udev, VIRTIO_PCI_CAP_NOTIFY_CFG); + notify = virtio_pci_find_capability(udev, VIRTIO_PCI_CAP_NOTIFY_CFG, + sizeof(struct virtio_pci_notify_cap)); if (!notify) { printf("(%s): missing capabilities %i/%i\n", udev->name, common, notify); @@ -524,7 +535,8 @@ static int virtio_pci_probe(struct udevice *udev) * Device capability is only mandatory for devices that have * device-specific configuration. */ - device = virtio_pci_find_capability(udev, VIRTIO_PCI_CAP_DEVICE_CFG); + 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);

On Wed, Mar 30, 2022 at 12:59 AM Andrew Scull ascull@google.com wrote:
Ensure the virtio PCI capabilities are contained within the bounds of the device's configuration space. The expected size of the capability is passed when searching for the capability to enforce this check.
Signed-off-by: Andrew Scull ascull@google.com
drivers/virtio/virtio_pci_modern.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.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 | 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);

On Wed, Mar 30, 2022 at 12:59 AM Andrew Scull ascull@google.com wrote:
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 | 72 ++++++++++++++++-------------- 1 file changed, 38 insertions(+), 34 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

The flags parameter of dm_pci_map_bar() is used for PCI region flags rather than memory mapping flags. Fix the type to match that of the region flags and stop using the regions flags as memory mapping flags.
Signed-off-by: Andrew Scull ascull@google.com --- drivers/pci/pci-uclass.c | 10 +++++----- include/pci.h | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 33dda00002..8bbeb62f2e 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -1533,8 +1533,8 @@ static phys_addr_t dm_pci_map_ea_virt(struct udevice *dev, int ea_off, return addr; }
-static void *dm_pci_map_ea_bar(struct udevice *dev, int bar, int flags, - int ea_off, struct pci_child_plat *pdata) +static void *dm_pci_map_ea_bar(struct udevice *dev, int bar, int ea_off, + struct pci_child_plat *pdata) { int ea_cnt, i, entry_size; int bar_id = (bar - PCI_BASE_ADDRESS_0) >> 2; @@ -1577,13 +1577,13 @@ static void *dm_pci_map_ea_bar(struct udevice *dev, int bar, int flags, addr += dm_pci_map_ea_virt(dev, ea_off, pdata);
/* size ignored for now */ - return map_physmem(addr, 0, flags); + return map_physmem(addr, 0, MAP_NOCACHE); }
return 0; }
-void *dm_pci_map_bar(struct udevice *dev, int bar, int flags) +void *dm_pci_map_bar(struct udevice *dev, int bar, unsigned long flags) { struct pci_child_plat *pdata = dev_get_parent_plat(dev); struct udevice *udev = dev; @@ -1608,7 +1608,7 @@ void *dm_pci_map_bar(struct udevice *dev, int bar, int flags) */ ea_off = dm_pci_find_capability(udev, PCI_CAP_ID_EA); if (ea_off) - return dm_pci_map_ea_bar(udev, bar, flags, ea_off, pdata); + return dm_pci_map_ea_bar(udev, bar, ea_off, pdata);
/* read BAR address */ dm_pci_read_config32(udev, bar, &bar_response); diff --git a/include/pci.h b/include/pci.h index 673c95c6bb..5ba82826f4 100644 --- a/include/pci.h +++ b/include/pci.h @@ -1472,7 +1472,7 @@ pci_addr_t dm_pci_phys_to_bus(struct udevice *dev, phys_addr_t addr, * @flags: Flags for the region type (PCI_REGION_...) * @return: pointer to the virtual address to use or 0 on error */ -void *dm_pci_map_bar(struct udevice *dev, int bar, int flags); +void *dm_pci_map_bar(struct udevice *dev, int bar, unsigned long flags);
/** * dm_pci_find_next_capability() - find a capability starting from an offset

On Wed, Mar 30, 2022 at 12:59 AM Andrew Scull ascull@google.com wrote:
The flags parameter of dm_pci_map_bar() is used for PCI region flags rather than memory mapping flags. Fix the type to match that of the region flags and stop using the regions flags as memory mapping flags.
Signed-off-by: Andrew Scull ascull@google.com
drivers/pci/pci-uclass.c | 10 +++++----- include/pci.h | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 33dda00002..8bbeb62f2e 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -1533,8 +1533,8 @@ static phys_addr_t dm_pci_map_ea_virt(struct udevice *dev, int ea_off, return addr; }
-static void *dm_pci_map_ea_bar(struct udevice *dev, int bar, int flags,
int ea_off, struct pci_child_plat *pdata)
+static void *dm_pci_map_ea_bar(struct udevice *dev, int bar, int ea_off,
struct pci_child_plat *pdata)
{ int ea_cnt, i, entry_size; int bar_id = (bar - PCI_BASE_ADDRESS_0) >> 2; @@ -1577,13 +1577,13 @@ static void *dm_pci_map_ea_bar(struct udevice *dev, int bar, int flags, addr += dm_pci_map_ea_virt(dev, ea_off, pdata);
/* size ignored for now */
return map_physmem(addr, 0, flags);
return map_physmem(addr, 0, MAP_NOCACHE); } return 0;
}
-void *dm_pci_map_bar(struct udevice *dev, int bar, int flags) +void *dm_pci_map_bar(struct udevice *dev, int bar, unsigned long flags)
Why is this change (int => unsigned long) necessary?
{ struct pci_child_plat *pdata = dev_get_parent_plat(dev); struct udevice *udev = dev; @@ -1608,7 +1608,7 @@ void *dm_pci_map_bar(struct udevice *dev, int bar, int flags) */ ea_off = dm_pci_find_capability(udev, PCI_CAP_ID_EA); if (ea_off)
return dm_pci_map_ea_bar(udev, bar, flags, ea_off, pdata);
return dm_pci_map_ea_bar(udev, bar, ea_off, pdata); /* read BAR address */ dm_pci_read_config32(udev, bar, &bar_response);
diff --git a/include/pci.h b/include/pci.h index 673c95c6bb..5ba82826f4 100644 --- a/include/pci.h +++ b/include/pci.h @@ -1472,7 +1472,7 @@ pci_addr_t dm_pci_phys_to_bus(struct udevice *dev, phys_addr_t addr,
- @flags: Flags for the region type (PCI_REGION_...)
- @return: pointer to the virtual address to use or 0 on error
*/ -void *dm_pci_map_bar(struct udevice *dev, int bar, int flags); +void *dm_pci_map_bar(struct udevice *dev, int bar, unsigned long flags);
/**
- dm_pci_find_next_capability() - find a capability starting from an offset
--
Regards, Bin

diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 33dda00002..8bbeb62f2e 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -1533,8 +1533,8 @@ static phys_addr_t dm_pci_map_ea_virt(struct udevice *dev, int ea_off, return addr; }
-static void *dm_pci_map_ea_bar(struct udevice *dev, int bar, int flags,
int ea_off, struct pci_child_plat *pdata)
+static void *dm_pci_map_ea_bar(struct udevice *dev, int bar, int ea_off,
struct pci_child_plat *pdata)
{ int ea_cnt, i, entry_size; int bar_id = (bar - PCI_BASE_ADDRESS_0) >> 2; @@ -1577,13 +1577,13 @@ static void *dm_pci_map_ea_bar(struct udevice *dev, int bar, int flags, addr += dm_pci_map_ea_virt(dev, ea_off, pdata);
/* size ignored for now */
return map_physmem(addr, 0, flags);
return map_physmem(addr, 0, MAP_NOCACHE); } return 0;
}
-void *dm_pci_map_bar(struct udevice *dev, int bar, int flags) +void *dm_pci_map_bar(struct udevice *dev, int bar, unsigned long flags)
Why is this change (int => unsigned long) necessary?
The flags are used with pci_region::flags which is an unsigned long and it seemed sensible to use the same type, size and sign, to avoid anything unexpected.

Add a config to control whether Enhanced Allocation is supported by the driver.
Signed-off-by: Andrew Scull ascull@google.com --- drivers/pci/Kconfig | 7 +++++++ drivers/pci/pci-uclass.c | 25 +++++++++++++++---------- 2 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig index 47cd074aa1..fd2203420c 100644 --- a/drivers/pci/Kconfig +++ b/drivers/pci/Kconfig @@ -67,6 +67,13 @@ config PCI_SRIOV if available on a PCI Physical Function device and probe for applicable drivers.
+config PCI_ENHANCED_ALLOCATION + bool "Enable support for Enhanced Allocation of resources" + default y + help + Enable support for Enhanced Allocation which can be used by supported + devices in place of traditional BARS for allocation of resources. + config PCI_ARID bool "Enable Alternate Routing-ID support for PCI" help diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 8bbeb62f2e..719656eb3a 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -645,7 +645,11 @@ int dm_pci_hose_probe_bus(struct udevice *bus) return log_msg_ret("probe", -EINVAL); }
- ea_pos = dm_pci_find_capability(bus, PCI_CAP_ID_EA); + if (IS_ENABLED(CONFIG_PCI_ENHANCED_ALLOCATION)) + ea_pos = dm_pci_find_capability(bus, PCI_CAP_ID_EA); + else + ea_pos = 0; + if (ea_pos) { dm_pci_read_config8(bus, ea_pos + sizeof(u32) + sizeof(u8), ®); @@ -1600,15 +1604,16 @@ void *dm_pci_map_bar(struct udevice *dev, int bar, unsigned long flags) udev = pdata->pfdev; }
- /* - * if the function supports Enhanced Allocation use that instead of - * BARs - * Incase of virtual functions, pdata will help read VF BEI - * and EA entry size. - */ - ea_off = dm_pci_find_capability(udev, PCI_CAP_ID_EA); - if (ea_off) - return dm_pci_map_ea_bar(udev, bar, ea_off, pdata); + if (IS_ENABLED(CONFIG_PCI_ENHANCED_ALLOCATION)) { + /* + * If the function supports Enhanced Allocation use that + * instead of BARs. Incase of virtual functions, pdata will + * help read VF BEI and EA entry size. + */ + ea_off = dm_pci_find_capability(udev, PCI_CAP_ID_EA); + if (ea_off) + return dm_pci_map_ea_bar(udev, bar, ea_off, pdata); + }
/* read BAR address */ dm_pci_read_config32(udev, bar, &bar_response);

On Wed, Mar 30, 2022 at 12:59 AM Andrew Scull ascull@google.com wrote:
Add a config to control whether Enhanced Allocation is supported by the driver.
Signed-off-by: Andrew Scull ascull@google.com
drivers/pci/Kconfig | 7 +++++++ drivers/pci/pci-uclass.c | 25 +++++++++++++++---------- 2 files changed, 22 insertions(+), 10 deletions(-)
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig index 47cd074aa1..fd2203420c 100644 --- a/drivers/pci/Kconfig +++ b/drivers/pci/Kconfig @@ -67,6 +67,13 @@ config PCI_SRIOV if available on a PCI Physical Function device and probe for applicable drivers.
+config PCI_ENHANCED_ALLOCATION
bool "Enable support for Enhanced Allocation of resources"
default y
help
Enable support for Enhanced Allocation which can be used by supported
devices in place of traditional BARS for allocation of resources.
Why do we need a config option for EA as it can be figured out in the run time?
config PCI_ARID bool "Enable Alternate Routing-ID support for PCI" help diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 8bbeb62f2e..719656eb3a 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -645,7 +645,11 @@ int dm_pci_hose_probe_bus(struct udevice *bus) return log_msg_ret("probe", -EINVAL); }
ea_pos = dm_pci_find_capability(bus, PCI_CAP_ID_EA);
if (IS_ENABLED(CONFIG_PCI_ENHANCED_ALLOCATION))
ea_pos = dm_pci_find_capability(bus, PCI_CAP_ID_EA);
else
ea_pos = 0;
if (ea_pos) { dm_pci_read_config8(bus, ea_pos + sizeof(u32) + sizeof(u8), ®);
@@ -1600,15 +1604,16 @@ void *dm_pci_map_bar(struct udevice *dev, int bar, unsigned long flags) udev = pdata->pfdev; }
/*
* if the function supports Enhanced Allocation use that instead of
* BARs
* Incase of virtual functions, pdata will help read VF BEI
* and EA entry size.
*/
ea_off = dm_pci_find_capability(udev, PCI_CAP_ID_EA);
if (ea_off)
return dm_pci_map_ea_bar(udev, bar, ea_off, pdata);
if (IS_ENABLED(CONFIG_PCI_ENHANCED_ALLOCATION)) {
/*
* If the function supports Enhanced Allocation use that
* instead of BARs. Incase of virtual functions, pdata will
* help read VF BEI and EA entry size.
*/
ea_off = dm_pci_find_capability(udev, PCI_CAP_ID_EA);
if (ea_off)
return dm_pci_map_ea_bar(udev, bar, ea_off, pdata);
} /* read BAR address */ dm_pci_read_config32(udev, bar, &bar_response);
--
Regards, Bin

diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig index 47cd074aa1..fd2203420c 100644 --- a/drivers/pci/Kconfig +++ b/drivers/pci/Kconfig @@ -67,6 +67,13 @@ config PCI_SRIOV if available on a PCI Physical Function device and probe for applicable drivers.
+config PCI_ENHANCED_ALLOCATION
bool "Enable support for Enhanced Allocation of resources"
default y
help
Enable support for Enhanced Allocation which can be used by supported
devices in place of traditional BARS for allocation of resources.
Why do we need a config option for EA as it can be figured out in the run time?
I wanted to have the option to disable the logic for handling EA at runtime. A glance at the code suggested there's a whole other rabbit hole of missing safety checks that I've opted to leave for another day.

On Thu, Apr 14, 2022 at 12:30 AM Andrew Scull ascull@google.com wrote:
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig index 47cd074aa1..fd2203420c 100644 --- a/drivers/pci/Kconfig +++ b/drivers/pci/Kconfig @@ -67,6 +67,13 @@ config PCI_SRIOV if available on a PCI Physical Function device and probe for applicable drivers.
+config PCI_ENHANCED_ALLOCATION
bool "Enable support for Enhanced Allocation of resources"
default y
help
Enable support for Enhanced Allocation which can be used by supported
devices in place of traditional BARS for allocation of resources.
Why do we need a config option for EA as it can be figured out in the run time?
I wanted to have the option to disable the logic for handling EA at runtime. A glance at the code suggested there's a whole other rabbit hole of missing safety checks that I've opted to leave for another day.
We cannot have a single global option to work for one device using EA but another device using BAR.
Regards, Bin

On Thu, 14 Apr 2022 at 01:42, Bin Meng bmeng.cn@gmail.com wrote:
On Thu, Apr 14, 2022 at 12:30 AM Andrew Scull ascull@google.com wrote:
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig index 47cd074aa1..fd2203420c 100644 --- a/drivers/pci/Kconfig +++ b/drivers/pci/Kconfig @@ -67,6 +67,13 @@ config PCI_SRIOV if available on a PCI Physical Function device and probe for applicable drivers.
+config PCI_ENHANCED_ALLOCATION
bool "Enable support for Enhanced Allocation of resources"
default y
help
Enable support for Enhanced Allocation which can be used by supported
devices in place of traditional BARS for allocation of resources.
Why do we need a config option for EA as it can be figured out in the run time?
I wanted to have the option to disable the logic for handling EA at runtime. A glance at the code suggested there's a whole other rabbit hole of missing safety checks that I've opted to leave for another day.
We cannot have a single global option to work for one device using EA but another device using BAR.
Sorry, I don't understand what you mean. This config will just mean the u-boot doesn't look for, or handle, the EA capability; it disables u-boot's support for this feature. If a system relies on EA, they'll need to have it enabled which is the default state. If a system doesn't then it can be turned off and the suspect logic disabled.
Could you help me understand your concern?

On Thu, 14 Apr 2022 at 08:58, Andrew Scull ascull@google.com wrote:
On Thu, 14 Apr 2022 at 01:42, Bin Meng bmeng.cn@gmail.com wrote:
On Thu, Apr 14, 2022 at 12:30 AM Andrew Scull ascull@google.com wrote:
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig index 47cd074aa1..fd2203420c 100644 --- a/drivers/pci/Kconfig +++ b/drivers/pci/Kconfig @@ -67,6 +67,13 @@ config PCI_SRIOV if available on a PCI Physical Function device and probe for applicable drivers.
+config PCI_ENHANCED_ALLOCATION
bool "Enable support for Enhanced Allocation of resources"
default y
help
Enable support for Enhanced Allocation which can be used by supported
devices in place of traditional BARS for allocation of resources.
Why do we need a config option for EA as it can be figured out in the run time?
I wanted to have the option to disable the logic for handling EA at runtime. A glance at the code suggested there's a whole other rabbit hole of missing safety checks that I've opted to leave for another day.
We cannot have a single global option to work for one device using EA but another device using BAR.
Sorry, I don't understand what you mean. This config will just mean the u-boot doesn't look for, or handle, the EA capability; it disables u-boot's support for this feature. If a system relies on EA, they'll need to have it enabled which is the default state. If a system doesn't then it can be turned off and the suspect logic disabled.
Could you help me understand your concern?
A neater version of the last hunk is below. But if you still don't think it belongs upstream, I'll remove it from the series to unblock the rest.
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 8bbeb62f2e..c4fe4e481a 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -645,7 +645,11 @@ int dm_pci_hose_probe_bus(struct udevice *bus) return log_msg_ret("probe", -EINVAL); }
- ea_pos = dm_pci_find_capability(bus, PCI_CAP_ID_EA); + if (IS_ENABLED(CONFIG_PCI_ENHANCED_ALLOCATION)) + ea_pos = dm_pci_find_capability(bus, PCI_CAP_ID_EA); + else + ea_pos = 0; + if (ea_pos) { dm_pci_read_config8(bus, ea_pos + sizeof(u32) + sizeof(u8), ®); @@ -1606,7 +1610,11 @@ void *dm_pci_map_bar(struct udevice *dev, int bar, unsigned long flags) * Incase of virtual functions, pdata will help read VF BEI * and EA entry size. */ - ea_off = dm_pci_find_capability(udev, PCI_CAP_ID_EA); + if (IS_ENABLED(CONFIG_PCI_ENHANCED_ALLOCATION)) + ea_off = dm_pci_find_capability(udev, PCI_CAP_ID_EA); + else + ea_off = 0; + if (ea_off) return dm_pci_map_ea_bar(udev, bar, ea_off, pdata);

On Thu, Apr 14, 2022 at 3:59 PM Andrew Scull ascull@google.com wrote:
On Thu, 14 Apr 2022 at 01:42, Bin Meng bmeng.cn@gmail.com wrote:
On Thu, Apr 14, 2022 at 12:30 AM Andrew Scull ascull@google.com wrote:
diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig index 47cd074aa1..fd2203420c 100644 --- a/drivers/pci/Kconfig +++ b/drivers/pci/Kconfig @@ -67,6 +67,13 @@ config PCI_SRIOV if available on a PCI Physical Function device and probe for applicable drivers.
+config PCI_ENHANCED_ALLOCATION
bool "Enable support for Enhanced Allocation of resources"
default y
help
Enable support for Enhanced Allocation which can be used by supported
devices in place of traditional BARS for allocation of resources.
Why do we need a config option for EA as it can be figured out in the run time?
I wanted to have the option to disable the logic for handling EA at runtime. A glance at the code suggested there's a whole other rabbit hole of missing safety checks that I've opted to leave for another day.
We cannot have a single global option to work for one device using EA but another device using BAR.
Sorry, I don't understand what you mean. This config will just mean the u-boot doesn't look for, or handle, the EA capability; it disables u-boot's support for this feature. If a system relies on EA, they'll need to have it enabled which is the default state. If a system doesn't then it can be turned off and the suspect logic disabled.
Could you help me understand your concern?
I was thinking of 2 PCI devices, one uses the normal BAR mechanism while the other one uses the EA. In such a case, this config option is mandated to be turned on. I just see no value in providing a config option while it can be determined at runtime.
Is that for footprint or some other concern why you wanted to introduce it?
Regards, Bin

When parsing the `ranges` DT node, check that both extremes of the regions are addressable without overflow. This assumption can then be safely made when processing the regions.
Signed-off-by: Andrew Scull ascull@google.com Reviewed-by: Bin Meng bmeng.cn@gmail.com --- drivers/pci/pci-uclass.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 719656eb3a..d84d97d676 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -1017,7 +1017,22 @@ static void decode_regions(struct pci_controller *hose, ofnode parent_node,
if (!IS_ENABLED(CONFIG_SYS_PCI_64BIT) && type == PCI_REGION_MEM && upper_32_bits(pci_addr)) { - debug(" - beyond the 32-bit boundary, ignoring\n"); + debug(" - pci_addr beyond the 32-bit boundary, ignoring\n"); + continue; + } + + if (!IS_ENABLED(CONFIG_PHYS_64BIT) && upper_32_bits(addr)) { + debug(" - addr beyond the 32-bit boundary, ignoring\n"); + continue; + } + + if (~((pci_addr_t)0) - pci_addr < size) { + debug(" - PCI range exceeds max address, ignoring\n"); + continue; + } + + if (~((phys_addr_t)0) - addr < size) { + debug(" - phys range exceeds max address, ignoring\n"); continue; }

Hi Andrew,
On 2022-03-29 18:58, Andrew Scull wrote:
When parsing the `ranges` DT node, check that both extremes of the regions are addressable without overflow. This assumption can then be safely made when processing the regions.
I've bisected this patch to break USB support on 32-bit Raspberry Pi 4 builds.
We've been using U-Boot v2022.01 so far with USB boot support. With the migration to U-Boot v2023.01, stopped booting with the following output:
U-Boot 2023.01+ (Apr 24 2023 - 15:11:45 +0200)
DRAM: 948 MiB (effective 7.9 GiB) RPI 4 Model B (0xd03114) Core: 207 devices, 15 uclasses, devicetree: board MMC: mmcnr@7e300000: 1, mmc@7e340000: 0 Loading Environment from nowhere... OK In: serial Out: serial Err: serial Net: eth0: ethernet@7d580000 PCIe BRCM: link up, 5.0 Gbps x1 (SSC) PCI Autoconfig: Found P2P bridge, device 0 PCI Autoconfig: BAR 0, Prf64, size=0x1000, No resource PCI: Failed autoconfig bar 10
starting USB... Bus xhci_pci: <freeze>
In this builds I've set #define DEBUG in drivers/pci/pci_auto(_common).c.
Reverting this commit leads to:
U-Boot 2023.01+ (Apr 24 2023 - 15:47:58 +0200)
DRAM: 948 MiB (effective 7.9 GiB) RPI 4 Model B (0xd03114) Core: 207 devices, 15 uclasses, devicetree: board MMC: mmcnr@7e300000: 1, mmc@7e340000: 0 Loading Environment from nowhere... OK In: serial Out: serial Err: serial Net: eth0: ethernet@7d580000 PCIe BRCM: link up, 5.0 Gbps x1 (SSC) PCI Autoconfig: Bus Memory region: [c0000000-ffffffff], Physical Memory [600000000-63fffffff] PCI Autoconfig: Found P2P bridge, device 0 PCI Autoconfig: BAR 0, Mem64, size=0x1000, address=0xc0000000 bus_lower=0xc0001000
starting USB... Bus xhci_pci: Register 5000420 NbrPorts 5 Starting the controller USB XHCI 1.00 scanning bus xhci_pci for devices... 3 USB Device(s) found scanning usb for storage devices... 1 Storage Device(s) found ...
The system boot successfully a bootable system.
Note that this needs CONFIG_USB_XHCI_BRCM=y which is not set by default in rpi_4_32b_defconfig.
I am guessing that the freeze is just a subsequent issue of not handling PCI: Failed autoconfig bar 10 properly. But in any case, I'd prefer to have a working PCIe in general.
Is maybe the Raspberry Pi device tree not suited for 32-bit operation? Can't we use some sensible range in this case?
-- Stefan
Signed-off-by: Andrew Scull ascull@google.com Reviewed-by: Bin Meng bmeng.cn@gmail.com
drivers/pci/pci-uclass.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 719656eb3a..d84d97d676 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -1017,7 +1017,22 @@ static void decode_regions(struct pci_controller *hose, ofnode parent_node,
if (!IS_ENABLED(CONFIG_SYS_PCI_64BIT) && type == PCI_REGION_MEM && upper_32_bits(pci_addr)) {
debug(" - beyond the 32-bit boundary, ignoring\n");
debug(" - pci_addr beyond the 32-bit boundary, ignoring\n");
continue;
}
if (!IS_ENABLED(CONFIG_PHYS_64BIT) && upper_32_bits(addr)) {
debug(" - addr beyond the 32-bit boundary, ignoring\n");
continue;
}
if (~((pci_addr_t)0) - pci_addr < size) {
debug(" - PCI range exceeds max address, ignoring\n");
continue;
}
if (~((phys_addr_t)0) - addr < size) {
}debug(" - phys range exceeds max address, ignoring\n"); continue;

When converting between PCI bus and physical addresses, include a length parameter that can be used to check that the entire range fits within one of the PCI regions. This prevents an address being returned that might be only partially valid for the range it is going to be used for.
Where the range check is not wanted, passing a length of 0 will have the same behaviour as before this change.
Signed-off-by: Andrew Scull ascull@google.com --- drivers/pci/pci-uclass.c | 57 ++++++++++++++++++++++++---------------- include/pci.h | 20 +++++++------- 2 files changed, 46 insertions(+), 31 deletions(-)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index d84d97d676..ba210bc91f 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -1394,12 +1394,13 @@ void dm_pci_write_bar32(struct udevice *dev, int barnum, u32 addr) dm_pci_write_config32(dev, bar, addr); }
-static int _dm_pci_bus_to_phys(struct udevice *ctlr, - pci_addr_t bus_addr, unsigned long flags, +static int _dm_pci_bus_to_phys(struct udevice *ctlr, pci_addr_t bus_addr, + size_t len, unsigned long flags, unsigned long skip_mask, phys_addr_t *pa) { struct pci_controller *hose = dev_get_uclass_priv(ctlr); struct pci_region *res; + pci_addr_t offset; int i;
if (hose->region_count == 0) { @@ -1416,18 +1417,25 @@ static int _dm_pci_bus_to_phys(struct udevice *ctlr, if (res->flags & skip_mask) continue;
- if (bus_addr >= res->bus_start && - (bus_addr - res->bus_start) < res->size) { - *pa = (bus_addr - res->bus_start + res->phys_start); - return 0; - } + if (bus_addr < res->bus_start) + continue; + + offset = bus_addr - res->bus_start; + if (offset >= res->size) + continue; + + if (len > res->size - offset) + continue; + + *pa = res->phys_start + offset; + return 0; }
return 1; }
phys_addr_t dm_pci_bus_to_phys(struct udevice *dev, pci_addr_t bus_addr, - unsigned long flags) + size_t len, unsigned long flags) { phys_addr_t phys_addr = 0; struct udevice *ctlr; @@ -1441,14 +1449,14 @@ phys_addr_t dm_pci_bus_to_phys(struct udevice *dev, pci_addr_t bus_addr, * on matches that don't have PCI_REGION_SYS_MEMORY set */ if ((flags & PCI_REGION_TYPE) == PCI_REGION_MEM) { - ret = _dm_pci_bus_to_phys(ctlr, bus_addr, + ret = _dm_pci_bus_to_phys(ctlr, bus_addr, len, flags, PCI_REGION_SYS_MEMORY, &phys_addr); if (!ret) return phys_addr; }
- ret = _dm_pci_bus_to_phys(ctlr, bus_addr, flags, 0, &phys_addr); + ret = _dm_pci_bus_to_phys(ctlr, bus_addr, len, flags, 0, &phys_addr);
if (ret) puts("pci_hose_bus_to_phys: invalid physical address\n"); @@ -1457,12 +1465,12 @@ phys_addr_t dm_pci_bus_to_phys(struct udevice *dev, pci_addr_t bus_addr, }
static int _dm_pci_phys_to_bus(struct udevice *dev, phys_addr_t phys_addr, - unsigned long flags, unsigned long skip_mask, - pci_addr_t *ba) + size_t len, unsigned long flags, + unsigned long skip_mask, pci_addr_t *ba) { struct pci_region *res; struct udevice *ctlr; - pci_addr_t bus_addr; + phys_addr_t offset; int i; struct pci_controller *hose;
@@ -1484,20 +1492,25 @@ static int _dm_pci_phys_to_bus(struct udevice *dev, phys_addr_t phys_addr, if (res->flags & skip_mask) continue;
- bus_addr = phys_addr - res->phys_start + res->bus_start; + if (phys_addr < res->phys_start) + continue;
- if (bus_addr >= res->bus_start && - (bus_addr - res->bus_start) < res->size) { - *ba = bus_addr; - return 0; - } + offset = phys_addr - res->phys_start; + if (offset >= res->size) + continue; + + if (len > res->size - offset) + continue; + + *ba = res->bus_start + offset; + return 0; }
return 1; }
pci_addr_t dm_pci_phys_to_bus(struct udevice *dev, phys_addr_t phys_addr, - unsigned long flags) + size_t len, unsigned long flags) { pci_addr_t bus_addr = 0; int ret; @@ -1507,13 +1520,13 @@ pci_addr_t dm_pci_phys_to_bus(struct udevice *dev, phys_addr_t phys_addr, * on matches that don't have PCI_REGION_SYS_MEMORY set */ if ((flags & PCI_REGION_TYPE) == PCI_REGION_MEM) { - ret = _dm_pci_phys_to_bus(dev, phys_addr, flags, + ret = _dm_pci_phys_to_bus(dev, phys_addr, len, flags, PCI_REGION_SYS_MEMORY, &bus_addr); if (!ret) return bus_addr; }
- ret = _dm_pci_phys_to_bus(dev, phys_addr, flags, 0, &bus_addr); + ret = _dm_pci_phys_to_bus(dev, phys_addr, len, flags, 0, &bus_addr);
if (ret) puts("pci_hose_phys_to_bus: invalid physical address\n"); diff --git a/include/pci.h b/include/pci.h index 5ba82826f4..e5fbe03cb9 100644 --- a/include/pci.h +++ b/include/pci.h @@ -1436,14 +1436,15 @@ void dm_pci_write_bar32(struct udevice *dev, int barnum, u32 addr); u32 dm_pci_read_bar32(const struct udevice *dev, int barnum);
/** - * dm_pci_bus_to_phys() - convert a PCI bus address to a physical address + * dm_pci_bus_to_phys() - convert a PCI bus address range to a physical address * * @dev: Device containing the PCI address * @addr: PCI address to convert + * @len: Length of the address range * @flags: Flags for the region type (PCI_REGION_...) * Return: physical address corresponding to that PCI bus address */ -phys_addr_t dm_pci_bus_to_phys(struct udevice *dev, pci_addr_t addr, +phys_addr_t dm_pci_bus_to_phys(struct udevice *dev, pci_addr_t addr, size_t len, unsigned long flags);
/** @@ -1451,10 +1452,11 @@ phys_addr_t dm_pci_bus_to_phys(struct udevice *dev, pci_addr_t addr, * * @dev: Device containing the bus address * @addr: Physical address to convert + * @len: Length of the address range * @flags: Flags for the region type (PCI_REGION_...) * Return: PCI bus address corresponding to that physical address */ -pci_addr_t dm_pci_phys_to_bus(struct udevice *dev, phys_addr_t addr, +pci_addr_t dm_pci_phys_to_bus(struct udevice *dev, phys_addr_t addr, size_t len, unsigned long flags);
/** @@ -1576,19 +1578,19 @@ int dm_pci_find_ext_capability(struct udevice *dev, int cap); int dm_pci_flr(struct udevice *dev);
#define dm_pci_virt_to_bus(dev, addr, flags) \ - dm_pci_phys_to_bus(dev, (virt_to_phys(addr)), (flags)) + dm_pci_phys_to_bus(dev, (virt_to_phys(addr)), 0, (flags)) #define dm_pci_bus_to_virt(dev, addr, flags, len, map_flags) \ - map_physmem(dm_pci_bus_to_phys(dev, (addr), (flags)), \ + map_physmem(dm_pci_bus_to_phys(dev, (addr), (len), (flags)), \ (len), (map_flags))
#define dm_pci_phys_to_mem(dev, addr) \ - dm_pci_phys_to_bus((dev), (addr), PCI_REGION_MEM) + dm_pci_phys_to_bus((dev), (addr), 0, PCI_REGION_MEM) #define dm_pci_mem_to_phys(dev, addr) \ - dm_pci_bus_to_phys((dev), (addr), PCI_REGION_MEM) + dm_pci_bus_to_phys((dev), (addr), 0, PCI_REGION_MEM) #define dm_pci_phys_to_io(dev, addr) \ - dm_pci_phys_to_bus((dev), (addr), PCI_REGION_IO) + dm_pci_phys_to_bus((dev), (addr), 0, PCI_REGION_IO) #define dm_pci_io_to_phys(dev, addr) \ - dm_pci_bus_to_phys((dev), (addr), PCI_REGION_IO) + dm_pci_bus_to_phys((dev), (addr), 0, PCI_REGION_IO)
#define dm_pci_virt_to_mem(dev, addr) \ dm_pci_virt_to_bus((dev), (addr), PCI_REGION_MEM)

On Wed, Mar 30, 2022 at 12:59 AM Andrew Scull ascull@google.com wrote:
When converting between PCI bus and physical addresses, include a length parameter that can be used to check that the entire range fits within one of the PCI regions. This prevents an address being returned that might be only partially valid for the range it is going to be used for.
Where the range check is not wanted, passing a length of 0 will have the same behaviour as before this change.
Signed-off-by: Andrew Scull ascull@google.com
drivers/pci/pci-uclass.c | 57 ++++++++++++++++++++++++---------------- include/pci.h | 20 +++++++------- 2 files changed, 46 insertions(+), 31 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

Add tests for the functions dm_pci_bus_to_phys() and dm_pci_phys_to_bus() which convert between PCI bus addresses and physical addresses based on the ranges declared for the PCI controller.
The ranges of bus#1 are used for the tests, adding a translation to one of the ranges to cover more cases.
Signed-off-by: Andrew Scull ascull@google.com --- arch/sandbox/dts/test.dts | 2 +- test/dm/pci.c | 102 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 1 deletion(-)
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 48ca3e1e47..76c75e08e7 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -979,7 +979,7 @@ #address-cells = <3>; #size-cells = <2>; ranges = <0x02000000 0 0x30000000 0x30000000 0 0x2000 // MEM0 - 0x02000000 0 0x31000000 0x31000000 0 0x2000 // MEM1 + 0x02000000 0 0x31000000 0x3e000000 0 0x2000 // MEM1 0x01000000 0 0x40000000 0x40000000 0 0x2000>; sandbox,dev-info = <0x08 0x00 0x1234 0x5678 0x0c 0x00 0x1234 0x5678 diff --git a/test/dm/pci.c b/test/dm/pci.c index 00e4440a9d..9789103c7d 100644 --- a/test/dm/pci.c +++ b/test/dm/pci.c @@ -376,3 +376,105 @@ static int dm_test_pci_region_multi(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_pci_region_multi, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT); + +/* + * Test the translation of PCI bus addresses to physical addresses using the + * ranges from bus#1. + */ +static int dm_test_pci_bus_to_phys(struct unit_test_state *uts) +{ + struct udevice *dev; + phys_addr_t phys_addr; + + ut_assertok(dm_pci_bus_find_bdf(PCI_BDF(1, 0x08, 0), &dev)); + + /* Before any of the ranges. */ + phys_addr = dm_pci_bus_to_phys(dev, 0x20000000, 0x400, PCI_REGION_MEM); + ut_asserteq(0, phys_addr); + + /* Identity range: whole, start, mid, end */ + phys_addr = dm_pci_bus_to_phys(dev, 0x2fff0000, 0x2000, PCI_REGION_MEM); + ut_asserteq(0, phys_addr); + phys_addr = dm_pci_bus_to_phys(dev, 0x30000000, 0x2000, PCI_REGION_MEM); + ut_asserteq(0x30000000, phys_addr); + phys_addr = dm_pci_bus_to_phys(dev, 0x30000000, 0x1000, PCI_REGION_MEM); + ut_asserteq(0x30000000, phys_addr); + phys_addr = dm_pci_bus_to_phys(dev, 0x30000abc, 0x12, PCI_REGION_MEM); + ut_asserteq(0x30000abc, phys_addr); + phys_addr = dm_pci_bus_to_phys(dev, 0x30000800, 0x1800, PCI_REGION_MEM); + ut_asserteq(0x30000800, phys_addr); + phys_addr = dm_pci_bus_to_phys(dev, 0x30008000, 0x1801, PCI_REGION_MEM); + ut_asserteq(0, phys_addr); + + /* Translated range: whole, start, mid, end */ + phys_addr = dm_pci_bus_to_phys(dev, 0x30ff0000, 0x2000, PCI_REGION_MEM); + ut_asserteq(0, phys_addr); + phys_addr = dm_pci_bus_to_phys(dev, 0x31000000, 0x2000, PCI_REGION_MEM); + ut_asserteq(0x3e000000, phys_addr); + phys_addr = dm_pci_bus_to_phys(dev, 0x31000000, 0x1000, PCI_REGION_MEM); + ut_asserteq(0x3e000000, phys_addr); + phys_addr = dm_pci_bus_to_phys(dev, 0x31000abc, 0x12, PCI_REGION_MEM); + ut_asserteq(0x3e000abc, phys_addr); + phys_addr = dm_pci_bus_to_phys(dev, 0x31000800, 0x1800, PCI_REGION_MEM); + ut_asserteq(0x3e000800, phys_addr); + phys_addr = dm_pci_bus_to_phys(dev, 0x31008000, 0x1801, PCI_REGION_MEM); + ut_asserteq(0, phys_addr); + + /* Beyond all of the ranges. */ + phys_addr = dm_pci_bus_to_phys(dev, 0x32000000, 0x400, PCI_REGION_MEM); + ut_asserteq(0, phys_addr); + + return 0; +} +DM_TEST(dm_test_pci_bus_to_phys, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT); + +/* + * Test the translation of physical addresses to PCI bus addresses using the + * ranges from bus#1. + */ +static int dm_test_pci_phys_to_bus(struct unit_test_state *uts) +{ + struct udevice *dev; + phys_addr_t phys_addr; + + ut_assertok(dm_pci_bus_find_bdf(PCI_BDF(1, 0x08, 0), &dev)); + + /* Before any of the ranges. */ + phys_addr = dm_pci_phys_to_bus(dev, 0x20000000, 0x400, PCI_REGION_MEM); + ut_asserteq(0, phys_addr); + + /* Identity range: whole, start, mid, end */ + phys_addr = dm_pci_phys_to_bus(dev, 0x2fff0000, 0x2000, PCI_REGION_MEM); + ut_asserteq(0, phys_addr); + phys_addr = dm_pci_phys_to_bus(dev, 0x30000000, 0x2000, PCI_REGION_MEM); + ut_asserteq(0x30000000, phys_addr); + phys_addr = dm_pci_phys_to_bus(dev, 0x30000000, 0x1000, PCI_REGION_MEM); + ut_asserteq(0x30000000, phys_addr); + phys_addr = dm_pci_phys_to_bus(dev, 0x30000abc, 0x12, PCI_REGION_MEM); + ut_asserteq(0x30000abc, phys_addr); + phys_addr = dm_pci_phys_to_bus(dev, 0x30000800, 0x1800, PCI_REGION_MEM); + ut_asserteq(0x30000800, phys_addr); + phys_addr = dm_pci_phys_to_bus(dev, 0x30008000, 0x1801, PCI_REGION_MEM); + ut_asserteq(0, phys_addr); + + /* Translated range: whole, start, mid, end */ + phys_addr = dm_pci_phys_to_bus(dev, 0x3dff0000, 0x2000, PCI_REGION_MEM); + ut_asserteq(0, phys_addr); + phys_addr = dm_pci_phys_to_bus(dev, 0x3e000000, 0x2000, PCI_REGION_MEM); + ut_asserteq(0x31000000, phys_addr); + phys_addr = dm_pci_phys_to_bus(dev, 0x3e000000, 0x1000, PCI_REGION_MEM); + ut_asserteq(0x31000000, phys_addr); + phys_addr = dm_pci_phys_to_bus(dev, 0x3e000abc, 0x12, PCI_REGION_MEM); + ut_asserteq(0x31000abc, phys_addr); + phys_addr = dm_pci_phys_to_bus(dev, 0x3e000800, 0x1800, PCI_REGION_MEM); + ut_asserteq(0x31000800, phys_addr); + phys_addr = dm_pci_phys_to_bus(dev, 0x3e008000, 0x1801, PCI_REGION_MEM); + ut_asserteq(0, phys_addr); + + /* Beyond all of the ranges. */ + phys_addr = dm_pci_phys_to_bus(dev, 0x3f000000, 0x400, PCI_REGION_MEM); + ut_asserteq(0, phys_addr); + + return 0; +} +DM_TEST(dm_test_pci_phys_to_bus, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);

On Wed, Mar 30, 2022 at 12:59 AM Andrew Scull ascull@google.com wrote:
Add tests for the functions dm_pci_bus_to_phys() and dm_pci_phys_to_bus() which convert between PCI bus addresses and physical addresses based on the ranges declared for the PCI controller.
The ranges of bus#1 are used for the tests, adding a translation to one of the ranges to cover more cases.
Signed-off-by: Andrew Scull ascull@google.com
arch/sandbox/dts/test.dts | 2 +- test/dm/pci.c | 102 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 1 deletion(-)
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 48ca3e1e47..76c75e08e7 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -979,7 +979,7 @@ #address-cells = <3>; #size-cells = <2>; ranges = <0x02000000 0 0x30000000 0x30000000 0 0x2000 // MEM0
0x02000000 0 0x31000000 0x31000000 0 0x2000 // MEM1
0x02000000 0 0x31000000 0x3e000000 0 0x2000 // MEM1 0x01000000 0 0x40000000 0x40000000 0 0x2000>; sandbox,dev-info = <0x08 0x00 0x1234 0x5678 0x0c 0x00 0x1234 0x5678
diff --git a/test/dm/pci.c b/test/dm/pci.c index 00e4440a9d..9789103c7d 100644 --- a/test/dm/pci.c +++ b/test/dm/pci.c @@ -376,3 +376,105 @@ static int dm_test_pci_region_multi(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_pci_region_multi, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+/*
- Test the translation of PCI bus addresses to physical addresses using the
- ranges from bus#1.
- */
+static int dm_test_pci_bus_to_phys(struct unit_test_state *uts) +{
struct udevice *dev;
phys_addr_t phys_addr;
ut_assertok(dm_pci_bus_find_bdf(PCI_BDF(1, 0x08, 0), &dev));
/* Before any of the ranges. */
phys_addr = dm_pci_bus_to_phys(dev, 0x20000000, 0x400, PCI_REGION_MEM);
ut_asserteq(0, phys_addr);
/* Identity range: whole, start, mid, end */
phys_addr = dm_pci_bus_to_phys(dev, 0x2fff0000, 0x2000, PCI_REGION_MEM);
I guess you wanted to test 0x2ffff000 with 0x2000 range, for checking the overlap of the start address?
ut_asserteq(0, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x30000000, 0x2000, PCI_REGION_MEM);
ut_asserteq(0x30000000, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x30000000, 0x1000, PCI_REGION_MEM);
ut_asserteq(0x30000000, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x30000abc, 0x12, PCI_REGION_MEM);
ut_asserteq(0x30000abc, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x30000800, 0x1800, PCI_REGION_MEM);
ut_asserteq(0x30000800, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x30008000, 0x1801, PCI_REGION_MEM);
ut_asserteq(0, phys_addr);
/* Translated range: whole, start, mid, end */
phys_addr = dm_pci_bus_to_phys(dev, 0x30ff0000, 0x2000, PCI_REGION_MEM);
0x30fff000?
ut_asserteq(0, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x31000000, 0x2000, PCI_REGION_MEM);
ut_asserteq(0x3e000000, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x31000000, 0x1000, PCI_REGION_MEM);
ut_asserteq(0x3e000000, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x31000abc, 0x12, PCI_REGION_MEM);
ut_asserteq(0x3e000abc, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x31000800, 0x1800, PCI_REGION_MEM);
ut_asserteq(0x3e000800, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x31008000, 0x1801, PCI_REGION_MEM);
ut_asserteq(0, phys_addr);
/* Beyond all of the ranges. */
phys_addr = dm_pci_bus_to_phys(dev, 0x32000000, 0x400, PCI_REGION_MEM);
ut_asserteq(0, phys_addr);
return 0;
+} +DM_TEST(dm_test_pci_bus_to_phys, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+/*
- Test the translation of physical addresses to PCI bus addresses using the
- ranges from bus#1.
- */
+static int dm_test_pci_phys_to_bus(struct unit_test_state *uts) +{
struct udevice *dev;
phys_addr_t phys_addr;
This should be pci_addr_t bus_addr
ut_assertok(dm_pci_bus_find_bdf(PCI_BDF(1, 0x08, 0), &dev));
/* Before any of the ranges. */
phys_addr = dm_pci_phys_to_bus(dev, 0x20000000, 0x400, PCI_REGION_MEM);
ut_asserteq(0, phys_addr);
/* Identity range: whole, start, mid, end */
phys_addr = dm_pci_phys_to_bus(dev, 0x2fff0000, 0x2000, PCI_REGION_MEM);
ut_asserteq(0, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x30000000, 0x2000, PCI_REGION_MEM);
ut_asserteq(0x30000000, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x30000000, 0x1000, PCI_REGION_MEM);
ut_asserteq(0x30000000, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x30000abc, 0x12, PCI_REGION_MEM);
ut_asserteq(0x30000abc, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x30000800, 0x1800, PCI_REGION_MEM);
ut_asserteq(0x30000800, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x30008000, 0x1801, PCI_REGION_MEM);
ut_asserteq(0, phys_addr);
/* Translated range: whole, start, mid, end */
phys_addr = dm_pci_phys_to_bus(dev, 0x3dff0000, 0x2000, PCI_REGION_MEM);
ut_asserteq(0, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x3e000000, 0x2000, PCI_REGION_MEM);
ut_asserteq(0x31000000, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x3e000000, 0x1000, PCI_REGION_MEM);
ut_asserteq(0x31000000, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x3e000abc, 0x12, PCI_REGION_MEM);
ut_asserteq(0x31000abc, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x3e000800, 0x1800, PCI_REGION_MEM);
ut_asserteq(0x31000800, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x3e008000, 0x1801, PCI_REGION_MEM);
ut_asserteq(0, phys_addr);
/* Beyond all of the ranges. */
phys_addr = dm_pci_phys_to_bus(dev, 0x3f000000, 0x400, PCI_REGION_MEM);
ut_asserteq(0, phys_addr);
return 0;
+}
+DM_TEST(dm_test_pci_phys_to_bus, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
Regards, Bin

On Wed, 13 Apr 2022 at 15:03, Bin Meng bmeng.cn@gmail.com wrote:
On Wed, Mar 30, 2022 at 12:59 AM Andrew Scull ascull@google.com wrote:
Add tests for the functions dm_pci_bus_to_phys() and dm_pci_phys_to_bus() which convert between PCI bus addresses and physical addresses based on the ranges declared for the PCI controller.
The ranges of bus#1 are used for the tests, adding a translation to one of the ranges to cover more cases.
Signed-off-by: Andrew Scull ascull@google.com
arch/sandbox/dts/test.dts | 2 +- test/dm/pci.c | 102 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 103 insertions(+), 1 deletion(-)
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts index 48ca3e1e47..76c75e08e7 100644 --- a/arch/sandbox/dts/test.dts +++ b/arch/sandbox/dts/test.dts @@ -979,7 +979,7 @@ #address-cells = <3>; #size-cells = <2>; ranges = <0x02000000 0 0x30000000 0x30000000 0 0x2000 // MEM0
0x02000000 0 0x31000000 0x31000000 0 0x2000 // MEM1
0x02000000 0 0x31000000 0x3e000000 0 0x2000 // MEM1 0x01000000 0 0x40000000 0x40000000 0 0x2000>; sandbox,dev-info = <0x08 0x00 0x1234 0x5678 0x0c 0x00 0x1234 0x5678
diff --git a/test/dm/pci.c b/test/dm/pci.c index 00e4440a9d..9789103c7d 100644 --- a/test/dm/pci.c +++ b/test/dm/pci.c @@ -376,3 +376,105 @@ static int dm_test_pci_region_multi(struct unit_test_state *uts) return 0; } DM_TEST(dm_test_pci_region_multi, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+/*
- Test the translation of PCI bus addresses to physical addresses using the
- ranges from bus#1.
- */
+static int dm_test_pci_bus_to_phys(struct unit_test_state *uts) +{
struct udevice *dev;
phys_addr_t phys_addr;
ut_assertok(dm_pci_bus_find_bdf(PCI_BDF(1, 0x08, 0), &dev));
/* Before any of the ranges. */
phys_addr = dm_pci_bus_to_phys(dev, 0x20000000, 0x400, PCI_REGION_MEM);
ut_asserteq(0, phys_addr);
/* Identity range: whole, start, mid, end */
phys_addr = dm_pci_bus_to_phys(dev, 0x2fff0000, 0x2000, PCI_REGION_MEM);
I guess you wanted to test 0x2ffff000 with 0x2000 range, for checking the overlap of the start address?
I did indeed, great spot!
ut_asserteq(0, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x30000000, 0x2000, PCI_REGION_MEM);
ut_asserteq(0x30000000, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x30000000, 0x1000, PCI_REGION_MEM);
ut_asserteq(0x30000000, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x30000abc, 0x12, PCI_REGION_MEM);
ut_asserteq(0x30000abc, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x30000800, 0x1800, PCI_REGION_MEM);
ut_asserteq(0x30000800, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x30008000, 0x1801, PCI_REGION_MEM);
ut_asserteq(0, phys_addr);
/* Translated range: whole, start, mid, end */
phys_addr = dm_pci_bus_to_phys(dev, 0x30ff0000, 0x2000, PCI_REGION_MEM);
0x30fff000?
Yep, and I've done the same thing in dm_test_pci_bus_to_phys.
ut_asserteq(0, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x31000000, 0x2000, PCI_REGION_MEM);
ut_asserteq(0x3e000000, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x31000000, 0x1000, PCI_REGION_MEM);
ut_asserteq(0x3e000000, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x31000abc, 0x12, PCI_REGION_MEM);
ut_asserteq(0x3e000abc, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x31000800, 0x1800, PCI_REGION_MEM);
ut_asserteq(0x3e000800, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x31008000, 0x1801, PCI_REGION_MEM);
ut_asserteq(0, phys_addr);
/* Beyond all of the ranges. */
phys_addr = dm_pci_bus_to_phys(dev, 0x32000000, 0x400, PCI_REGION_MEM);
ut_asserteq(0, phys_addr);
return 0;
+} +DM_TEST(dm_test_pci_bus_to_phys, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+/*
- Test the translation of physical addresses to PCI bus addresses using the
- ranges from bus#1.
- */
+static int dm_test_pci_phys_to_bus(struct unit_test_state *uts) +{
struct udevice *dev;
phys_addr_t phys_addr;
This should be pci_addr_t bus_addr
Will fix in the next version.
ut_assertok(dm_pci_bus_find_bdf(PCI_BDF(1, 0x08, 0), &dev));
/* Before any of the ranges. */
phys_addr = dm_pci_phys_to_bus(dev, 0x20000000, 0x400, PCI_REGION_MEM);
ut_asserteq(0, phys_addr);
/* Identity range: whole, start, mid, end */
phys_addr = dm_pci_phys_to_bus(dev, 0x2fff0000, 0x2000, PCI_REGION_MEM);
ut_asserteq(0, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x30000000, 0x2000, PCI_REGION_MEM);
ut_asserteq(0x30000000, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x30000000, 0x1000, PCI_REGION_MEM);
ut_asserteq(0x30000000, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x30000abc, 0x12, PCI_REGION_MEM);
ut_asserteq(0x30000abc, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x30000800, 0x1800, PCI_REGION_MEM);
ut_asserteq(0x30000800, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x30008000, 0x1801, PCI_REGION_MEM);
ut_asserteq(0, phys_addr);
/* Translated range: whole, start, mid, end */
phys_addr = dm_pci_phys_to_bus(dev, 0x3dff0000, 0x2000, PCI_REGION_MEM);
ut_asserteq(0, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x3e000000, 0x2000, PCI_REGION_MEM);
ut_asserteq(0x31000000, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x3e000000, 0x1000, PCI_REGION_MEM);
ut_asserteq(0x31000000, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x3e000abc, 0x12, PCI_REGION_MEM);
ut_asserteq(0x31000abc, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x3e000800, 0x1800, PCI_REGION_MEM);
ut_asserteq(0x31000800, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x3e008000, 0x1801, PCI_REGION_MEM);
ut_asserteq(0, phys_addr);
/* Beyond all of the ranges. */
phys_addr = dm_pci_phys_to_bus(dev, 0x3f000000, 0x400, PCI_REGION_MEM);
ut_asserteq(0, phys_addr);
return 0;
+}
+DM_TEST(dm_test_pci_phys_to_bus, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
Regards, Bin

Evolve dm_pci_map_bar() to include an offset and length parameter. These allow a portion of the memory to be mapped and range checks to be applied.
Passing both the offset and length as zero results in the previous behaviour and this is used to migtate the previous callers.
Signed-off-by: Andrew Scull ascull@google.com --- arch/x86/cpu/baytrail/cpu.c | 2 +- drivers/ata/ahci.c | 8 +++---- drivers/gpio/octeon_gpio.c | 2 +- drivers/i2c/designware_i2c_pci.c | 2 +- drivers/i2c/intel_i2c.c | 2 +- drivers/i2c/octeon_i2c.c | 2 +- drivers/mmc/octeontx_hsmmc.c | 2 +- drivers/mmc/pci_mmc.c | 2 +- drivers/mtd/nand/raw/octeontx_bch.c | 4 ++-- drivers/mtd/nand/raw/octeontx_nand.c | 2 +- drivers/net/bnxt/bnxt.c | 6 +++--- drivers/net/fsl_enetc.c | 2 +- drivers/net/fsl_enetc_mdio.c | 2 +- drivers/net/mscc_eswitch/felix_switch.c | 4 ++-- drivers/net/octeontx/bgx.c | 2 +- drivers/net/octeontx/nic_main.c | 2 +- drivers/net/octeontx/nicvf_main.c | 2 +- drivers/net/octeontx/smi.c | 2 +- drivers/net/octeontx2/cgx.c | 2 +- drivers/net/octeontx2/rvu_af.c | 2 +- drivers/net/octeontx2/rvu_pf.c | 2 +- drivers/net/pch_gbe.c | 2 +- drivers/nvme/nvme_pci.c | 4 ++-- drivers/pci/pci-uclass.c | 28 +++++++++++++++++-------- drivers/spi/octeon_spi.c | 2 +- drivers/usb/host/ohci-pci.c | 2 +- drivers/virtio/virtio_pci_legacy.c | 2 +- include/pci.h | 5 ++++- test/dm/pci.c | 12 +++++------ 29 files changed, 63 insertions(+), 50 deletions(-)
diff --git a/arch/x86/cpu/baytrail/cpu.c b/arch/x86/cpu/baytrail/cpu.c index 309a50a116..6f1b05175c 100644 --- a/arch/x86/cpu/baytrail/cpu.c +++ b/arch/x86/cpu/baytrail/cpu.c @@ -55,7 +55,7 @@ int arch_cpu_init_dm(void) for (i = 0; i < 2; i++) { ret = dm_pci_bus_find_bdf(PCI_BDF(0, 0x1e, 3 + i), &dev); if (!ret) { - base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, + base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM); hsuart_clock_set(base); } diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index 2062197afc..3925807d55 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -416,8 +416,8 @@ static int ahci_init_one(struct ahci_uc_priv *uc_priv, struct udevice *dev) uc_priv->udma_mask = 0x7f; /*Fixme,assume to support UDMA6 */
#if !defined(CONFIG_DM_SCSI) - uc_priv->mmio_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_5, - PCI_REGION_MEM); + uc_priv->mmio_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_5, 0, 0, + PCI_REGION_MEM);
/* Take from kernel: * JMicron-specific fixup: @@ -1148,7 +1148,7 @@ int ahci_probe_scsi_pci(struct udevice *ahci_dev) ulong base; u16 vendor, device;
- base = (ulong)dm_pci_map_bar(ahci_dev, PCI_BASE_ADDRESS_5, + base = (ulong)dm_pci_map_bar(ahci_dev, PCI_BASE_ADDRESS_5, 0, 0, PCI_REGION_MEM);
/* @@ -1163,7 +1163,7 @@ int ahci_probe_scsi_pci(struct udevice *ahci_dev)
if (vendor == PCI_VENDOR_ID_CAVIUM && device == PCI_DEVICE_ID_CAVIUM_SATA) - base = (uintptr_t)dm_pci_map_bar(ahci_dev, PCI_BASE_ADDRESS_0, + base = (uintptr_t)dm_pci_map_bar(ahci_dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM); return ahci_probe_scsi(ahci_dev, base); } diff --git a/drivers/gpio/octeon_gpio.c b/drivers/gpio/octeon_gpio.c index 42eae79d8c..e6a8e1a521 100644 --- a/drivers/gpio/octeon_gpio.c +++ b/drivers/gpio/octeon_gpio.c @@ -183,7 +183,7 @@ static int octeon_gpio_probe(struct udevice *dev) priv->data = (const struct octeon_gpio_data *)dev_get_driver_data(dev);
if (priv->data->probe == PROBE_PCI) { - priv->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, + priv->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM); uc_priv->gpio_count = readq(priv->base + priv->data->reg_offs + GPIO_CONST) & diff --git a/drivers/i2c/designware_i2c_pci.c b/drivers/i2c/designware_i2c_pci.c index 9e387737b6..51f1357d10 100644 --- a/drivers/i2c/designware_i2c_pci.c +++ b/drivers/i2c/designware_i2c_pci.c @@ -59,7 +59,7 @@ static int designware_i2c_pci_of_to_plat(struct udevice *dev) priv->regs = (struct i2c_regs *)dm_pci_read_bar32(dev, 0); } else { priv->regs = (struct i2c_regs *) - dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, PCI_REGION_MEM); + dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM); } if (!priv->regs) return -EINVAL; diff --git a/drivers/i2c/intel_i2c.c b/drivers/i2c/intel_i2c.c index 52f7a528ef..7b5b62e3eb 100644 --- a/drivers/i2c/intel_i2c.c +++ b/drivers/i2c/intel_i2c.c @@ -251,7 +251,7 @@ static int intel_i2c_probe(struct udevice *dev) ulong base;
/* Save base address from PCI BAR */ - priv->base = (ulong)dm_pci_map_bar(dev, PCI_BASE_ADDRESS_4, + priv->base = (ulong)dm_pci_map_bar(dev, PCI_BASE_ADDRESS_4, 0, 0, PCI_REGION_IO); base = priv->base;
diff --git a/drivers/i2c/octeon_i2c.c b/drivers/i2c/octeon_i2c.c index 50199ff46e..74fd5c3d2e 100644 --- a/drivers/i2c/octeon_i2c.c +++ b/drivers/i2c/octeon_i2c.c @@ -792,7 +792,7 @@ static int octeon_i2c_probe(struct udevice *dev)
debug("TWSI PCI device: %x\n", bdf);
- twsi->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, + twsi->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM); } else { twsi->base = dev_remap_addr(dev); diff --git a/drivers/mmc/octeontx_hsmmc.c b/drivers/mmc/octeontx_hsmmc.c index f0519f0cf8..0bf38945a1 100644 --- a/drivers/mmc/octeontx_hsmmc.c +++ b/drivers/mmc/octeontx_hsmmc.c @@ -3822,7 +3822,7 @@ static int octeontx_mmc_host_probe(struct udevice *dev)
/* Octeon TX & TX2 use PCI based probing */ if (device_is_compatible(dev, "cavium,thunder-8890-mmc")) { - host->base_addr = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, + host->base_addr = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM); if (!host->base_addr) { pr_err("%s: Error: MMC base address not found\n", diff --git a/drivers/mmc/pci_mmc.c b/drivers/mmc/pci_mmc.c index b9ab064b60..1bc2fbcfdf 100644 --- a/drivers/mmc/pci_mmc.c +++ b/drivers/mmc/pci_mmc.c @@ -50,7 +50,7 @@ static int pci_mmc_probe(struct udevice *dev) desc = mmc_get_blk_desc(&plat->mmc); desc->removable = !(plat->cfg.host_caps & MMC_CAP_NONREMOVABLE);
- host->ioaddr = (void *)dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, + host->ioaddr = (void *)dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM); host->name = dev->name; host->cd_gpio = priv->cd_gpio; diff --git a/drivers/mtd/nand/raw/octeontx_bch.c b/drivers/mtd/nand/raw/octeontx_bch.c index 24ffa5105f..c1cc5fa187 100644 --- a/drivers/mtd/nand/raw/octeontx_bch.c +++ b/drivers/mtd/nand/raw/octeontx_bch.c @@ -176,7 +176,7 @@ static int octeontx_pci_bchpf_probe(struct udevice *dev) if (!bch) return -ENOMEM;
- bch->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, PCI_REGION_MEM); + bch->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM); bch->dev = dev;
debug("%s: base address: %p\n", __func__, bch->reg_base); @@ -361,7 +361,7 @@ static int octeontx_pci_bchvf_probe(struct udevice *dev) vf->dev = dev;
/* Map PF's configuration registers */ - vf->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, PCI_REGION_MEM); + vf->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM); debug("%s: reg base: %p\n", __func__, vf->reg_base);
err = octeontx_cmd_queue_initialize(dev, QID_BCH, QDEPTH - 1, 0, diff --git a/drivers/mtd/nand/raw/octeontx_nand.c b/drivers/mtd/nand/raw/octeontx_nand.c index ff363a56b5..3e84bb2fc0 100644 --- a/drivers/mtd/nand/raw/octeontx_nand.c +++ b/drivers/mtd/nand/raw/octeontx_nand.c @@ -2098,7 +2098,7 @@ static int octeontx_pci_nand_probe(struct udevice *dev) tn->dev = dev; INIT_LIST_HEAD(&tn->chips);
- tn->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, PCI_REGION_MEM); + tn->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM); if (!tn->base) { ret = -EINVAL; goto release; diff --git a/drivers/net/bnxt/bnxt.c b/drivers/net/bnxt/bnxt.c index 9844e96072..a24f965ec1 100644 --- a/drivers/net/bnxt/bnxt.c +++ b/drivers/net/bnxt/bnxt.c @@ -28,9 +28,9 @@ static void bnxt_bring_pci(struct bnxt *bp) dm_pci_read_config16(bp->pdev, PCI_SUBSYSTEM_ID, &bp->subsystem_device); dm_pci_read_config16(bp->pdev, PCI_COMMAND, &bp->cmd_reg); dm_pci_read_config8(bp->pdev, PCI_INTERRUPT_LINE, &bp->irq); - bp->bar0 = dm_pci_map_bar(bp->pdev, PCI_BASE_ADDRESS_0, PCI_REGION_MEM); - bp->bar1 = dm_pci_map_bar(bp->pdev, PCI_BASE_ADDRESS_2, PCI_REGION_MEM); - bp->bar2 = dm_pci_map_bar(bp->pdev, PCI_BASE_ADDRESS_4, PCI_REGION_MEM); + bp->bar0 = dm_pci_map_bar(bp->pdev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM); + bp->bar1 = dm_pci_map_bar(bp->pdev, PCI_BASE_ADDRESS_2, 0, 0, PCI_REGION_MEM); + bp->bar2 = dm_pci_map_bar(bp->pdev, PCI_BASE_ADDRESS_4, 0, 0, PCI_REGION_MEM); cmd_reg = bp->cmd_reg | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER; cmd_reg |= PCI_COMMAND_INTX_DISABLE; /* disable intr */ dm_pci_write_config16(bp->pdev, PCI_COMMAND, cmd_reg); diff --git a/drivers/net/fsl_enetc.c b/drivers/net/fsl_enetc.c index 915c7c8025..60a7b27c81 100644 --- a/drivers/net/fsl_enetc.c +++ b/drivers/net/fsl_enetc.c @@ -344,7 +344,7 @@ static int enetc_probe(struct udevice *dev) }
/* initialize register */ - priv->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0); + priv->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, 0); if (!priv->regs_base) { enetc_dbg(dev, "failed to map BAR0\n"); return -EINVAL; diff --git a/drivers/net/fsl_enetc_mdio.c b/drivers/net/fsl_enetc_mdio.c index 3eb6ac9fc8..f025c2255c 100644 --- a/drivers/net/fsl_enetc_mdio.c +++ b/drivers/net/fsl_enetc_mdio.c @@ -125,7 +125,7 @@ static int enetc_mdio_probe(struct udevice *dev) { struct enetc_mdio_priv *priv = dev_get_priv(dev);
- priv->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0); + priv->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, 0); if (!priv->regs_base) { enetc_dbg(dev, "failed to map BAR0\n"); return -EINVAL; diff --git a/drivers/net/mscc_eswitch/felix_switch.c b/drivers/net/mscc_eswitch/felix_switch.c index 60b2e8f32d..0badb23828 100644 --- a/drivers/net/mscc_eswitch/felix_switch.c +++ b/drivers/net/mscc_eswitch/felix_switch.c @@ -292,13 +292,13 @@ static int felix_probe(struct udevice *dev) return -ENODEV; }
- priv->imdio_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0); + priv->imdio_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, 0); if (!priv->imdio_base) { dev_err(dev, "failed to map BAR0\n"); return -EINVAL; }
- priv->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_4, 0); + priv->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_4, 0, 0, 0); if (!priv->regs_base) { dev_err(dev, "failed to map BAR4\n"); return -EINVAL; diff --git a/drivers/net/octeontx/bgx.c b/drivers/net/octeontx/bgx.c index a5c0c9fe2b..cc8ef099c2 100644 --- a/drivers/net/octeontx/bgx.c +++ b/drivers/net/octeontx/bgx.c @@ -1458,7 +1458,7 @@ int octeontx_bgx_probe(struct udevice *dev) int bgx_idx, node; int inc = 1;
- bgx->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, + bgx->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM); if (!bgx->reg_base) { debug("No PCI region found\n"); diff --git a/drivers/net/octeontx/nic_main.c b/drivers/net/octeontx/nic_main.c index 0f36f2586e..4754c042f1 100644 --- a/drivers/net/octeontx/nic_main.c +++ b/drivers/net/octeontx/nic_main.c @@ -713,7 +713,7 @@ int nic_initialize(struct udevice *dev) return -ENOMEM;
/* MAP PF's configuration registers */ - nic->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, + nic->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM); if (!nic->reg_base) { printf("Cannot map config register space, aborting\n"); diff --git a/drivers/net/octeontx/nicvf_main.c b/drivers/net/octeontx/nicvf_main.c index c30ba49c27..097df6df1e 100644 --- a/drivers/net/octeontx/nicvf_main.c +++ b/drivers/net/octeontx/nicvf_main.c @@ -509,7 +509,7 @@ int nicvf_initialize(struct udevice *dev) /* Enable TSO support */ nicvf->hw_tso = true;
- nicvf->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, + nicvf->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM);
debug("nicvf->reg_base: %p\n", nicvf->reg_base); diff --git a/drivers/net/octeontx/smi.c b/drivers/net/octeontx/smi.c index d70fa820c7..2d521bd3ca 100644 --- a/drivers/net/octeontx/smi.c +++ b/drivers/net/octeontx/smi.c @@ -322,7 +322,7 @@ int octeontx_smi_probe(struct udevice *dev) u64 baseaddr;
debug("SMI PCI device: %x\n", bdf); - if (!dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, PCI_REGION_MEM)) { + if (!dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM)) { printf("Failed to map PCI region for bdf %x\n", bdf); return -1; } diff --git a/drivers/net/octeontx2/cgx.c b/drivers/net/octeontx2/cgx.c index d139029f4e..eed31a9579 100644 --- a/drivers/net/octeontx2/cgx.c +++ b/drivers/net/octeontx2/cgx.c @@ -253,7 +253,7 @@ int cgx_probe(struct udevice *dev) struct cgx *cgx = dev_get_priv(dev); int err;
- cgx->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, + cgx->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM); cgx->dev = dev; cgx->cgx_id = ((u64)(cgx->reg_base) >> 24) & 0x7; diff --git a/drivers/net/octeontx2/rvu_af.c b/drivers/net/octeontx2/rvu_af.c index d2f9654861..47c1502ef8 100644 --- a/drivers/net/octeontx2/rvu_af.c +++ b/drivers/net/octeontx2/rvu_af.c @@ -127,7 +127,7 @@ int rvu_af_probe(struct udevice *dev) { struct rvu_af *af_ptr = dev_get_priv(dev);
- af_ptr->af_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, + af_ptr->af_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM); debug("%s RVU AF BAR %p\n", __func__, af_ptr->af_base); af_ptr->dev = dev; diff --git a/drivers/net/octeontx2/rvu_pf.c b/drivers/net/octeontx2/rvu_pf.c index 4b00178989..024e17e748 100644 --- a/drivers/net/octeontx2/rvu_pf.c +++ b/drivers/net/octeontx2/rvu_pf.c @@ -58,7 +58,7 @@ int rvu_pf_probe(struct udevice *dev)
debug("%s: name: %s\n", __func__, dev->name);
- rvu->pf_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_2, PCI_REGION_MEM); + rvu->pf_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_2, 0, 0, PCI_REGION_MEM); rvu->pfid = dev_seq(dev) + 1; // RVU PF's start from 1; rvu->dev = dev; if (!rvu_af_dev) { diff --git a/drivers/net/pch_gbe.c b/drivers/net/pch_gbe.c index fabcf85c0d..c93cc2ead2 100644 --- a/drivers/net/pch_gbe.c +++ b/drivers/net/pch_gbe.c @@ -449,7 +449,7 @@ static int pch_gbe_probe(struct udevice *dev)
priv->dev = dev;
- iobase = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_1, PCI_REGION_MEM); + iobase = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_1, 0, 0, PCI_REGION_MEM);
plat->iobase = (ulong)iobase; priv->mac_regs = (struct pch_gbe_regs *)iobase; diff --git a/drivers/nvme/nvme_pci.c b/drivers/nvme/nvme_pci.c index 5f60fb884f..3499a7b6e7 100644 --- a/drivers/nvme/nvme_pci.c +++ b/drivers/nvme/nvme_pci.c @@ -28,8 +28,8 @@ static int nvme_probe(struct udevice *udev) sprintf(ndev->vendor, "0x%.4x", pplat->vendor);
ndev->instance = trailing_strtol(udev->name); - ndev->bar = dm_pci_map_bar(udev, PCI_BASE_ADDRESS_0, - PCI_REGION_MEM); + ndev->bar = dm_pci_map_bar(udev, PCI_BASE_ADDRESS_0, 0, 0, + PCI_REGION_MEM); return nvme_init(udev); }
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index ba210bc91f..033c52bb4e 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -1565,7 +1565,8 @@ static phys_addr_t dm_pci_map_ea_virt(struct udevice *dev, int ea_off, return addr; }
-static void *dm_pci_map_ea_bar(struct udevice *dev, int bar, int ea_off, +static void *dm_pci_map_ea_bar(struct udevice *dev, int bar, size_t offset, + size_t len, int ea_off, struct pci_child_plat *pdata) { int ea_cnt, i, entry_size; @@ -1608,14 +1609,18 @@ static void *dm_pci_map_ea_bar(struct udevice *dev, int bar, int ea_off, if (IS_ENABLED(CONFIG_PCI_SRIOV)) addr += dm_pci_map_ea_virt(dev, ea_off, pdata);
+ if (~((phys_addr_t)0) - addr < offset) + return NULL; + /* size ignored for now */ - return map_physmem(addr, 0, MAP_NOCACHE); + return map_physmem(addr + offset, len, MAP_NOCACHE); }
return 0; }
-void *dm_pci_map_bar(struct udevice *dev, int bar, unsigned long flags) +void *dm_pci_map_bar(struct udevice *dev, int bar, size_t offset, size_t len, + unsigned long flags) { struct pci_child_plat *pdata = dev_get_parent_plat(dev); struct udevice *udev = dev; @@ -1640,20 +1645,25 @@ void *dm_pci_map_bar(struct udevice *dev, int bar, unsigned long flags) */ ea_off = dm_pci_find_capability(udev, PCI_CAP_ID_EA); if (ea_off) - return dm_pci_map_ea_bar(udev, bar, ea_off, pdata); + return dm_pci_map_ea_bar(udev, bar, offset, len, ea_off, + pdata); }
/* read BAR address */ dm_pci_read_config32(udev, bar, &bar_response); pci_bus_addr = (pci_addr_t)(bar_response & ~0xf);
+ if (~((pci_addr_t)0) - pci_bus_addr < offset) + return NULL; + /* - * Pass "0" as the length argument to pci_bus_to_virt. The arg - * isn't actually used on any platform because U-Boot assumes a static - * linear mapping. In the future, this could read the BAR size - * and pass that as the size if needed. + * Forward the length argument to dm_pci_bus_to_virt. The length will + * be used to check that the entire address range has been declared as + * a PCI range, but a better check would be to probe for the size of + * the bar and prevent overflow more locally. */ - return dm_pci_bus_to_virt(udev, pci_bus_addr, flags, 0, MAP_NOCACHE); + return dm_pci_bus_to_virt(udev, pci_bus_addr + offset, flags, len, + MAP_NOCACHE); }
static int _dm_pci_find_next_capability(struct udevice *dev, u8 pos, int cap) diff --git a/drivers/spi/octeon_spi.c b/drivers/spi/octeon_spi.c index fcabc112d2..2f8a8a8649 100644 --- a/drivers/spi/octeon_spi.c +++ b/drivers/spi/octeon_spi.c @@ -568,7 +568,7 @@ static int octeon_spi_probe(struct udevice *dev) pci_dev_t bdf = dm_pci_get_bdf(dev);
debug("SPI PCI device: %x\n", bdf); - priv->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, + priv->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM); /* Add base offset */ priv->base += 0x1000; diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c index 6ddc9da704..eab0d96637 100644 --- a/drivers/usb/host/ohci-pci.c +++ b/drivers/usb/host/ohci-pci.c @@ -18,7 +18,7 @@ static int ohci_pci_probe(struct udevice *dev) { struct ohci_regs *regs;
- regs = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, PCI_REGION_MEM); + regs = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM); return ohci_register(dev, regs); }
diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c index 03fa5cb608..504a7ff7b9 100644 --- a/drivers/virtio/virtio_pci_legacy.c +++ b/drivers/virtio/virtio_pci_legacy.c @@ -319,7 +319,7 @@ static int virtio_pci_probe(struct udevice *udev) uc_priv->device = subdevice; uc_priv->vendor = subvendor;
- priv->ioaddr = dm_pci_map_bar(udev, PCI_BASE_ADDRESS_0, PCI_REGION_IO); + priv->ioaddr = dm_pci_map_bar(udev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_IO); if (!priv->ioaddr) return -ENXIO; debug("(%s): virtio legacy device reg base %04lx\n", diff --git a/include/pci.h b/include/pci.h index e5fbe03cb9..d137debb68 100644 --- a/include/pci.h +++ b/include/pci.h @@ -1471,10 +1471,13 @@ pci_addr_t dm_pci_phys_to_bus(struct udevice *dev, phys_addr_t addr, size_t len, * * @dev: Device to check * @bar: Bar register offset (PCI_BASE_ADDRESS_...) + * @offset: Offset from the base to map + * @len: Length to map * @flags: Flags for the region type (PCI_REGION_...) * @return: pointer to the virtual address to use or 0 on error */ -void *dm_pci_map_bar(struct udevice *dev, int bar, unsigned long flags); +void *dm_pci_map_bar(struct udevice *dev, int bar, size_t offset, size_t len, + unsigned long flags);
/** * dm_pci_find_next_capability() - find a capability starting from an offset diff --git a/test/dm/pci.c b/test/dm/pci.c index 9789103c7d..c8598e4c17 100644 --- a/test/dm/pci.c +++ b/test/dm/pci.c @@ -268,27 +268,27 @@ static int dm_test_pci_ea(struct unit_test_state *uts) ut_asserteq(PCI_CAP_ID_EA_OFFSET, cap);
/* test swap case in BAR 1 */ - bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_0, 0); + bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_0, 0, 0, 0); ut_assertnonnull(bar); *(int *)bar = 2; /* swap upper/lower */
- bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_1, 0); + bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_1, 0, 0, 0); ut_assertnonnull(bar); strcpy(bar, "ea TEST"); unmap_sysmem(bar); - bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_1, 0); + bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_1, 0, 0, 0); ut_assertnonnull(bar); ut_asserteq_str("EA test", bar);
/* test magic values in BARs2, 4; BAR 3 is n/a */ - bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_2, 0); + bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_2, 0, 0, 0); ut_assertnonnull(bar); ut_asserteq(PCI_EA_BAR2_MAGIC, *(u32 *)bar);
- bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_3, 0); + bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_3, 0, 0, 0); ut_assertnull(bar);
- bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_4, 0); + bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_4, 0, 0, 0); ut_assertnonnull(bar); ut_asserteq(PCI_EA_BAR4_MAGIC, *(u32 *)bar);

On Tue, Mar 29, 2022 at 04:58:55PM +0000, Andrew Scull wrote:
Evolve dm_pci_map_bar() to include an offset and length parameter. These allow a portion of the memory to be mapped and range checks to be applied.
Passing both the offset and length as zero results in the previous behaviour and this is used to migtate the previous callers.
Signed-off-by: Andrew Scull ascull@google.com
This breaks rpi_arm64 building, and possibly others. It may be helpful to read https://u-boot.readthedocs.io/en/latest/develop/ci_testing.html and get a CI run completed for the whole series to shake out any other failures. Thanks!

On Tue, 12 Apr 2022 at 17:42, Tom Rini trini@konsulko.com wrote:
On Tue, Mar 29, 2022 at 04:58:55PM +0000, Andrew Scull wrote:
Evolve dm_pci_map_bar() to include an offset and length parameter. These allow a portion of the memory to be mapped and range checks to be applied.
Passing both the offset and length as zero results in the previous behaviour and this is used to migtate the previous callers.
Signed-off-by: Andrew Scull ascull@google.com
This breaks rpi_arm64 building, and possibly others. It may be helpful to read https://u-boot.readthedocs.io/en/latest/develop/ci_testing.html and get a CI run completed for the whole series to shake out any other failures. Thanks!
That CI sounds like it will be useful to know how to use, thanks for the pointers.
Aside from just building, I'd also be interested in feedback as to whether this evolution of the API is a suitable direction to be taking.

On Tue, Apr 12, 2022 at 11:54:33PM +0100, Andrew Scull wrote:
On Tue, 12 Apr 2022 at 17:42, Tom Rini trini@konsulko.com wrote:
On Tue, Mar 29, 2022 at 04:58:55PM +0000, Andrew Scull wrote:
Evolve dm_pci_map_bar() to include an offset and length parameter. These allow a portion of the memory to be mapped and range checks to be applied.
Passing both the offset and length as zero results in the previous behaviour and this is used to migtate the previous callers.
Signed-off-by: Andrew Scull ascull@google.com
This breaks rpi_arm64 building, and possibly others. It may be helpful to read https://u-boot.readthedocs.io/en/latest/develop/ci_testing.html and get a CI run completed for the whole series to shake out any other failures. Thanks!
That CI sounds like it will be useful to know how to use, thanks for the pointers.
Aside from just building, I'd also be interested in feedback as to whether this evolution of the API is a suitable direction to be taking.
Ah. To be clear, I was preparing to merge the series and ran in to this problem. So, I think it's fine once CI passes.

On Wed, Mar 30, 2022 at 12:59 AM Andrew Scull ascull@google.com wrote:
Evolve dm_pci_map_bar() to include an offset and length parameter. These allow a portion of the memory to be mapped and range checks to be applied.
Passing both the offset and length as zero results in the previous behaviour and this is used to migtate the previous callers.
migtate?
Signed-off-by: Andrew Scull ascull@google.com
arch/x86/cpu/baytrail/cpu.c | 2 +- drivers/ata/ahci.c | 8 +++---- drivers/gpio/octeon_gpio.c | 2 +- drivers/i2c/designware_i2c_pci.c | 2 +- drivers/i2c/intel_i2c.c | 2 +- drivers/i2c/octeon_i2c.c | 2 +- drivers/mmc/octeontx_hsmmc.c | 2 +- drivers/mmc/pci_mmc.c | 2 +- drivers/mtd/nand/raw/octeontx_bch.c | 4 ++-- drivers/mtd/nand/raw/octeontx_nand.c | 2 +- drivers/net/bnxt/bnxt.c | 6 +++--- drivers/net/fsl_enetc.c | 2 +- drivers/net/fsl_enetc_mdio.c | 2 +- drivers/net/mscc_eswitch/felix_switch.c | 4 ++-- drivers/net/octeontx/bgx.c | 2 +- drivers/net/octeontx/nic_main.c | 2 +- drivers/net/octeontx/nicvf_main.c | 2 +- drivers/net/octeontx/smi.c | 2 +- drivers/net/octeontx2/cgx.c | 2 +- drivers/net/octeontx2/rvu_af.c | 2 +- drivers/net/octeontx2/rvu_pf.c | 2 +- drivers/net/pch_gbe.c | 2 +- drivers/nvme/nvme_pci.c | 4 ++-- drivers/pci/pci-uclass.c | 28 +++++++++++++++++-------- drivers/spi/octeon_spi.c | 2 +- drivers/usb/host/ohci-pci.c | 2 +- drivers/virtio/virtio_pci_legacy.c | 2 +- include/pci.h | 5 ++++- test/dm/pci.c | 12 +++++------ 29 files changed, 63 insertions(+), 50 deletions(-)
Regards, Bin

On Wed, 13 Apr 2022 at 15:05, Bin Meng bmeng.cn@gmail.com wrote:
On Wed, Mar 30, 2022 at 12:59 AM Andrew Scull ascull@google.com wrote:
Evolve dm_pci_map_bar() to include an offset and length parameter. These allow a portion of the memory to be mapped and range checks to be applied.
Passing both the offset and length as zero results in the previous behaviour and this is used to migtate the previous callers.
migtate?
*migrate -- will fix
Signed-off-by: Andrew Scull ascull@google.com
arch/x86/cpu/baytrail/cpu.c | 2 +- drivers/ata/ahci.c | 8 +++---- drivers/gpio/octeon_gpio.c | 2 +- drivers/i2c/designware_i2c_pci.c | 2 +- drivers/i2c/intel_i2c.c | 2 +- drivers/i2c/octeon_i2c.c | 2 +- drivers/mmc/octeontx_hsmmc.c | 2 +- drivers/mmc/pci_mmc.c | 2 +- drivers/mtd/nand/raw/octeontx_bch.c | 4 ++-- drivers/mtd/nand/raw/octeontx_nand.c | 2 +- drivers/net/bnxt/bnxt.c | 6 +++--- drivers/net/fsl_enetc.c | 2 +- drivers/net/fsl_enetc_mdio.c | 2 +- drivers/net/mscc_eswitch/felix_switch.c | 4 ++-- drivers/net/octeontx/bgx.c | 2 +- drivers/net/octeontx/nic_main.c | 2 +- drivers/net/octeontx/nicvf_main.c | 2 +- drivers/net/octeontx/smi.c | 2 +- drivers/net/octeontx2/cgx.c | 2 +- drivers/net/octeontx2/rvu_af.c | 2 +- drivers/net/octeontx2/rvu_pf.c | 2 +- drivers/net/pch_gbe.c | 2 +- drivers/nvme/nvme_pci.c | 4 ++-- drivers/pci/pci-uclass.c | 28 +++++++++++++++++-------- drivers/spi/octeon_spi.c | 2 +- drivers/usb/host/ohci-pci.c | 2 +- drivers/virtio/virtio_pci_legacy.c | 2 +- include/pci.h | 5 ++++- test/dm/pci.c | 12 +++++------ 29 files changed, 63 insertions(+), 50 deletions(-)
Regards, Bin

When converting addresses, apply a mask to the region flags during lookup. This allows the caller to specify which flags are important and which are not, for example to exclude system memory regions.
The behaviour of the function is changed such that they don't preferentially search for a non-system memory region. However, system memory regions are added after other regions in decode_regions() leading to a similar outcome.
Signed-off-by: Andrew Scull ascull@google.com --- drivers/pci/pci-uclass.c | 110 +++++++++------------------------------ include/pci.h | 18 ++++--- test/dm/pci.c | 60 +++++++++++---------- 3 files changed, 67 insertions(+), 121 deletions(-)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 033c52bb4e..5069ada66d 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -1394,27 +1394,27 @@ void dm_pci_write_bar32(struct udevice *dev, int barnum, u32 addr) dm_pci_write_config32(dev, bar, addr); }
-static int _dm_pci_bus_to_phys(struct udevice *ctlr, pci_addr_t bus_addr, - size_t len, unsigned long flags, - unsigned long skip_mask, phys_addr_t *pa) +phys_addr_t dm_pci_bus_to_phys(struct udevice *dev, pci_addr_t bus_addr, + size_t len, unsigned long mask, + unsigned long flags) { - struct pci_controller *hose = dev_get_uclass_priv(ctlr); + struct udevice *ctlr; + struct pci_controller *hose; struct pci_region *res; pci_addr_t offset; int i;
- if (hose->region_count == 0) { - *pa = bus_addr; - return 0; - } + /* The root controller has the region information */ + ctlr = pci_get_controller(dev); + hose = dev_get_uclass_priv(ctlr); + + if (hose->region_count == 0) + return bus_addr;
for (i = 0; i < hose->region_count; i++) { res = &hose->regions[i];
- if (((res->flags ^ flags) & PCI_REGION_TYPE) != 0) - continue; - - if (res->flags & skip_mask) + if ((res->flags & mask) != flags) continue;
if (bus_addr < res->bus_start) @@ -1427,69 +1427,34 @@ static int _dm_pci_bus_to_phys(struct udevice *ctlr, pci_addr_t bus_addr, if (len > res->size - offset) continue;
- *pa = res->phys_start + offset; - return 0; + return res->phys_start + offset; }
- return 1; + puts("pci_hose_bus_to_phys: invalid physical address\n"); + return 0; }
-phys_addr_t dm_pci_bus_to_phys(struct udevice *dev, pci_addr_t bus_addr, - size_t len, unsigned long flags) +pci_addr_t dm_pci_phys_to_bus(struct udevice *dev, phys_addr_t phys_addr, + size_t len, unsigned long mask, + unsigned long flags) { - phys_addr_t phys_addr = 0; struct udevice *ctlr; - int ret; - - /* The root controller has the region information */ - ctlr = pci_get_controller(dev); - - /* - * if PCI_REGION_MEM is set we do a two pass search with preference - * on matches that don't have PCI_REGION_SYS_MEMORY set - */ - if ((flags & PCI_REGION_TYPE) == PCI_REGION_MEM) { - ret = _dm_pci_bus_to_phys(ctlr, bus_addr, len, - flags, PCI_REGION_SYS_MEMORY, - &phys_addr); - if (!ret) - return phys_addr; - } - - ret = _dm_pci_bus_to_phys(ctlr, bus_addr, len, flags, 0, &phys_addr); - - if (ret) - puts("pci_hose_bus_to_phys: invalid physical address\n"); - - return phys_addr; -} - -static int _dm_pci_phys_to_bus(struct udevice *dev, phys_addr_t phys_addr, - size_t len, unsigned long flags, - unsigned long skip_mask, pci_addr_t *ba) -{ + struct pci_controller *hose; struct pci_region *res; - struct udevice *ctlr; phys_addr_t offset; int i; - struct pci_controller *hose;
/* The root controller has the region information */ ctlr = pci_get_controller(dev); hose = dev_get_uclass_priv(ctlr);
- if (hose->region_count == 0) { - *ba = phys_addr; - return 0; - } + if (hose->region_count == 0) + return phys_addr;
for (i = 0; i < hose->region_count; i++) { res = &hose->regions[i];
- if (((res->flags ^ flags) & PCI_REGION_TYPE) != 0) - continue; - - if (res->flags & skip_mask) + if ((res->flags & mask) != flags) continue;
if (phys_addr < res->phys_start) @@ -1502,36 +1467,11 @@ static int _dm_pci_phys_to_bus(struct udevice *dev, phys_addr_t phys_addr, if (len > res->size - offset) continue;
- *ba = res->bus_start + offset; - return 0; - } - - return 1; -} - -pci_addr_t dm_pci_phys_to_bus(struct udevice *dev, phys_addr_t phys_addr, - size_t len, unsigned long flags) -{ - pci_addr_t bus_addr = 0; - int ret; - - /* - * if PCI_REGION_MEM is set we do a two pass search with preference - * on matches that don't have PCI_REGION_SYS_MEMORY set - */ - if ((flags & PCI_REGION_TYPE) == PCI_REGION_MEM) { - ret = _dm_pci_phys_to_bus(dev, phys_addr, len, flags, - PCI_REGION_SYS_MEMORY, &bus_addr); - if (!ret) - return bus_addr; + return res->bus_start + offset; }
- ret = _dm_pci_phys_to_bus(dev, phys_addr, len, flags, 0, &bus_addr); - - if (ret) - puts("pci_hose_phys_to_bus: invalid physical address\n"); - - return bus_addr; + puts("pci_hose_phys_to_bus: invalid physical address\n"); + return 0; }
static phys_addr_t dm_pci_map_ea_virt(struct udevice *dev, int ea_off, diff --git a/include/pci.h b/include/pci.h index d137debb68..8198265269 100644 --- a/include/pci.h +++ b/include/pci.h @@ -1441,11 +1441,12 @@ u32 dm_pci_read_bar32(const struct udevice *dev, int barnum); * @dev: Device containing the PCI address * @addr: PCI address to convert * @len: Length of the address range + * @mask: Mask to match flags for the region type * @flags: Flags for the region type (PCI_REGION_...) * Return: physical address corresponding to that PCI bus address */ phys_addr_t dm_pci_bus_to_phys(struct udevice *dev, pci_addr_t addr, size_t len, - unsigned long flags); + unsigned long mask, unsigned long flags);
/** * dm_pci_phys_to_bus() - convert a physical address to a PCI bus address @@ -1453,11 +1454,12 @@ phys_addr_t dm_pci_bus_to_phys(struct udevice *dev, pci_addr_t addr, size_t len, * @dev: Device containing the bus address * @addr: Physical address to convert * @len: Length of the address range + * @mask: Mask to match flags for the region type * @flags: Flags for the region type (PCI_REGION_...) * Return: PCI bus address corresponding to that physical address */ pci_addr_t dm_pci_phys_to_bus(struct udevice *dev, phys_addr_t addr, size_t len, - unsigned long flags); + unsigned long mask, unsigned long flags);
/** * dm_pci_map_bar() - get a virtual address associated with a BAR region @@ -1581,19 +1583,19 @@ int dm_pci_find_ext_capability(struct udevice *dev, int cap); int dm_pci_flr(struct udevice *dev);
#define dm_pci_virt_to_bus(dev, addr, flags) \ - dm_pci_phys_to_bus(dev, (virt_to_phys(addr)), 0, (flags)) + dm_pci_phys_to_bus(dev, (virt_to_phys(addr)), 0, PCI_REGION_TYPE, (flags)) #define dm_pci_bus_to_virt(dev, addr, flags, len, map_flags) \ - map_physmem(dm_pci_bus_to_phys(dev, (addr), (len), (flags)), \ + map_physmem(dm_pci_bus_to_phys(dev, (addr), (len), PCI_REGION_TYPE, (flags)), \ (len), (map_flags))
#define dm_pci_phys_to_mem(dev, addr) \ - dm_pci_phys_to_bus((dev), (addr), 0, PCI_REGION_MEM) + dm_pci_phys_to_bus((dev), (addr), 0, PCI_REGION_TYPE, PCI_REGION_MEM) #define dm_pci_mem_to_phys(dev, addr) \ - dm_pci_bus_to_phys((dev), (addr), 0, PCI_REGION_MEM) + dm_pci_bus_to_phys((dev), (addr), 0, PCI_REGION_TYPE, PCI_REGION_MEM) #define dm_pci_phys_to_io(dev, addr) \ - dm_pci_phys_to_bus((dev), (addr), 0, PCI_REGION_IO) + dm_pci_phys_to_bus((dev), (addr), 0, PCI_REGION_TYPE, PCI_REGION_IO) #define dm_pci_io_to_phys(dev, addr) \ - dm_pci_bus_to_phys((dev), (addr), 0, PCI_REGION_IO) + dm_pci_bus_to_phys((dev), (addr), 0, PCI_REGION_TYPE, PCI_REGION_IO)
#define dm_pci_virt_to_mem(dev, addr) \ dm_pci_virt_to_bus((dev), (addr), PCI_REGION_MEM) diff --git a/test/dm/pci.c b/test/dm/pci.c index c8598e4c17..edc407f9d3 100644 --- a/test/dm/pci.c +++ b/test/dm/pci.c @@ -383,45 +383,47 @@ DM_TEST(dm_test_pci_region_multi, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT); */ static int dm_test_pci_bus_to_phys(struct unit_test_state *uts) { + unsigned long mask = PCI_REGION_TYPE; + unsigned long flags = PCI_REGION_MEM; struct udevice *dev; phys_addr_t phys_addr;
ut_assertok(dm_pci_bus_find_bdf(PCI_BDF(1, 0x08, 0), &dev));
/* Before any of the ranges. */ - phys_addr = dm_pci_bus_to_phys(dev, 0x20000000, 0x400, PCI_REGION_MEM); + phys_addr = dm_pci_bus_to_phys(dev, 0x20000000, 0x400, mask, flags); ut_asserteq(0, phys_addr);
/* Identity range: whole, start, mid, end */ - phys_addr = dm_pci_bus_to_phys(dev, 0x2fff0000, 0x2000, PCI_REGION_MEM); + phys_addr = dm_pci_bus_to_phys(dev, 0x2fff0000, 0x2000, mask, flags); ut_asserteq(0, phys_addr); - phys_addr = dm_pci_bus_to_phys(dev, 0x30000000, 0x2000, PCI_REGION_MEM); + phys_addr = dm_pci_bus_to_phys(dev, 0x30000000, 0x2000, mask, flags); ut_asserteq(0x30000000, phys_addr); - phys_addr = dm_pci_bus_to_phys(dev, 0x30000000, 0x1000, PCI_REGION_MEM); + phys_addr = dm_pci_bus_to_phys(dev, 0x30000000, 0x1000, mask, flags); ut_asserteq(0x30000000, phys_addr); - phys_addr = dm_pci_bus_to_phys(dev, 0x30000abc, 0x12, PCI_REGION_MEM); + phys_addr = dm_pci_bus_to_phys(dev, 0x30000abc, 0x12, mask, flags); ut_asserteq(0x30000abc, phys_addr); - phys_addr = dm_pci_bus_to_phys(dev, 0x30000800, 0x1800, PCI_REGION_MEM); + phys_addr = dm_pci_bus_to_phys(dev, 0x30000800, 0x1800, mask, flags); ut_asserteq(0x30000800, phys_addr); - phys_addr = dm_pci_bus_to_phys(dev, 0x30008000, 0x1801, PCI_REGION_MEM); + phys_addr = dm_pci_bus_to_phys(dev, 0x30008000, 0x1801, mask, flags); ut_asserteq(0, phys_addr);
/* Translated range: whole, start, mid, end */ - phys_addr = dm_pci_bus_to_phys(dev, 0x30ff0000, 0x2000, PCI_REGION_MEM); + phys_addr = dm_pci_bus_to_phys(dev, 0x30ff0000, 0x2000, mask, flags); ut_asserteq(0, phys_addr); - phys_addr = dm_pci_bus_to_phys(dev, 0x31000000, 0x2000, PCI_REGION_MEM); + phys_addr = dm_pci_bus_to_phys(dev, 0x31000000, 0x2000, mask, flags); ut_asserteq(0x3e000000, phys_addr); - phys_addr = dm_pci_bus_to_phys(dev, 0x31000000, 0x1000, PCI_REGION_MEM); + phys_addr = dm_pci_bus_to_phys(dev, 0x31000000, 0x1000, mask, flags); ut_asserteq(0x3e000000, phys_addr); - phys_addr = dm_pci_bus_to_phys(dev, 0x31000abc, 0x12, PCI_REGION_MEM); + phys_addr = dm_pci_bus_to_phys(dev, 0x31000abc, 0x12, mask, flags); ut_asserteq(0x3e000abc, phys_addr); - phys_addr = dm_pci_bus_to_phys(dev, 0x31000800, 0x1800, PCI_REGION_MEM); + phys_addr = dm_pci_bus_to_phys(dev, 0x31000800, 0x1800, mask, flags); ut_asserteq(0x3e000800, phys_addr); - phys_addr = dm_pci_bus_to_phys(dev, 0x31008000, 0x1801, PCI_REGION_MEM); + phys_addr = dm_pci_bus_to_phys(dev, 0x31008000, 0x1801, mask, flags); ut_asserteq(0, phys_addr);
/* Beyond all of the ranges. */ - phys_addr = dm_pci_bus_to_phys(dev, 0x32000000, 0x400, PCI_REGION_MEM); + phys_addr = dm_pci_bus_to_phys(dev, 0x32000000, 0x400, mask, flags); ut_asserteq(0, phys_addr);
return 0; @@ -434,45 +436,47 @@ DM_TEST(dm_test_pci_bus_to_phys, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT); */ static int dm_test_pci_phys_to_bus(struct unit_test_state *uts) { + unsigned long mask = PCI_REGION_TYPE; + unsigned long flags = PCI_REGION_MEM; struct udevice *dev; phys_addr_t phys_addr;
ut_assertok(dm_pci_bus_find_bdf(PCI_BDF(1, 0x08, 0), &dev));
/* Before any of the ranges. */ - phys_addr = dm_pci_phys_to_bus(dev, 0x20000000, 0x400, PCI_REGION_MEM); + phys_addr = dm_pci_phys_to_bus(dev, 0x20000000, 0x400, mask, flags); ut_asserteq(0, phys_addr);
/* Identity range: whole, start, mid, end */ - phys_addr = dm_pci_phys_to_bus(dev, 0x2fff0000, 0x2000, PCI_REGION_MEM); + phys_addr = dm_pci_phys_to_bus(dev, 0x2fff0000, 0x2000, mask, flags); ut_asserteq(0, phys_addr); - phys_addr = dm_pci_phys_to_bus(dev, 0x30000000, 0x2000, PCI_REGION_MEM); + phys_addr = dm_pci_phys_to_bus(dev, 0x30000000, 0x2000, mask, flags); ut_asserteq(0x30000000, phys_addr); - phys_addr = dm_pci_phys_to_bus(dev, 0x30000000, 0x1000, PCI_REGION_MEM); + phys_addr = dm_pci_phys_to_bus(dev, 0x30000000, 0x1000, mask, flags); ut_asserteq(0x30000000, phys_addr); - phys_addr = dm_pci_phys_to_bus(dev, 0x30000abc, 0x12, PCI_REGION_MEM); + phys_addr = dm_pci_phys_to_bus(dev, 0x30000abc, 0x12, mask, flags); ut_asserteq(0x30000abc, phys_addr); - phys_addr = dm_pci_phys_to_bus(dev, 0x30000800, 0x1800, PCI_REGION_MEM); + phys_addr = dm_pci_phys_to_bus(dev, 0x30000800, 0x1800, mask, flags); ut_asserteq(0x30000800, phys_addr); - phys_addr = dm_pci_phys_to_bus(dev, 0x30008000, 0x1801, PCI_REGION_MEM); + phys_addr = dm_pci_phys_to_bus(dev, 0x30008000, 0x1801, mask, flags); ut_asserteq(0, phys_addr);
/* Translated range: whole, start, mid, end */ - phys_addr = dm_pci_phys_to_bus(dev, 0x3dff0000, 0x2000, PCI_REGION_MEM); + phys_addr = dm_pci_phys_to_bus(dev, 0x3dff0000, 0x2000, mask, flags); ut_asserteq(0, phys_addr); - phys_addr = dm_pci_phys_to_bus(dev, 0x3e000000, 0x2000, PCI_REGION_MEM); + phys_addr = dm_pci_phys_to_bus(dev, 0x3e000000, 0x2000, mask, flags); ut_asserteq(0x31000000, phys_addr); - phys_addr = dm_pci_phys_to_bus(dev, 0x3e000000, 0x1000, PCI_REGION_MEM); + phys_addr = dm_pci_phys_to_bus(dev, 0x3e000000, 0x1000, mask, flags); ut_asserteq(0x31000000, phys_addr); - phys_addr = dm_pci_phys_to_bus(dev, 0x3e000abc, 0x12, PCI_REGION_MEM); + phys_addr = dm_pci_phys_to_bus(dev, 0x3e000abc, 0x12, mask, flags); ut_asserteq(0x31000abc, phys_addr); - phys_addr = dm_pci_phys_to_bus(dev, 0x3e000800, 0x1800, PCI_REGION_MEM); + phys_addr = dm_pci_phys_to_bus(dev, 0x3e000800, 0x1800, mask, flags); ut_asserteq(0x31000800, phys_addr); - phys_addr = dm_pci_phys_to_bus(dev, 0x3e008000, 0x1801, PCI_REGION_MEM); + phys_addr = dm_pci_phys_to_bus(dev, 0x3e008000, 0x1801, mask, flags); ut_asserteq(0, phys_addr);
/* Beyond all of the ranges. */ - phys_addr = dm_pci_phys_to_bus(dev, 0x3f000000, 0x400, PCI_REGION_MEM); + phys_addr = dm_pci_phys_to_bus(dev, 0x3f000000, 0x400, mask, flags); ut_asserteq(0, phys_addr);
return 0;

On Wed, Mar 30, 2022 at 12:59 AM Andrew Scull ascull@google.com wrote:
When converting addresses, apply a mask to the region flags during lookup. This allows the caller to specify which flags are important and which are not, for example to exclude system memory regions.
The behaviour of the function is changed such that they don't preferentially search for a non-system memory region. However, system memory regions are added after other regions in decode_regions() leading to a similar outcome.
Signed-off-by: Andrew Scull ascull@google.com
drivers/pci/pci-uclass.c | 110 +++++++++------------------------------ include/pci.h | 18 ++++--- test/dm/pci.c | 60 +++++++++++---------- 3 files changed, 67 insertions(+), 121 deletions(-)
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 033c52bb4e..5069ada66d 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -1394,27 +1394,27 @@ void dm_pci_write_bar32(struct udevice *dev, int barnum, u32 addr) dm_pci_write_config32(dev, bar, addr); }
-static int _dm_pci_bus_to_phys(struct udevice *ctlr, pci_addr_t bus_addr,
size_t len, unsigned long flags,
unsigned long skip_mask, phys_addr_t *pa)
+phys_addr_t dm_pci_bus_to_phys(struct udevice *dev, pci_addr_t bus_addr,
size_t len, unsigned long mask,
unsigned long flags)
{
struct pci_controller *hose = dev_get_uclass_priv(ctlr);
struct udevice *ctlr;
struct pci_controller *hose; struct pci_region *res; pci_addr_t offset; int i;
if (hose->region_count == 0) {
*pa = bus_addr;
return 0;
}
/* The root controller has the region information */
ctlr = pci_get_controller(dev);
hose = dev_get_uclass_priv(ctlr);
if (hose->region_count == 0)
return bus_addr; for (i = 0; i < hose->region_count; i++) { res = &hose->regions[i];
if (((res->flags ^ flags) & PCI_REGION_TYPE) != 0)
continue;
if (res->flags & skip_mask)
if ((res->flags & mask) != flags) continue; if (bus_addr < res->bus_start)
@@ -1427,69 +1427,34 @@ static int _dm_pci_bus_to_phys(struct udevice *ctlr, pci_addr_t bus_addr, if (len > res->size - offset) continue;
*pa = res->phys_start + offset;
return 0;
return res->phys_start + offset; }
return 1;
puts("pci_hose_bus_to_phys: invalid physical address\n");
return 0;
}
-phys_addr_t dm_pci_bus_to_phys(struct udevice *dev, pci_addr_t bus_addr,
size_t len, unsigned long flags)
+pci_addr_t dm_pci_phys_to_bus(struct udevice *dev, phys_addr_t phys_addr,
size_t len, unsigned long mask,
unsigned long flags)
{
phys_addr_t phys_addr = 0; struct udevice *ctlr;
int ret;
/* The root controller has the region information */
ctlr = pci_get_controller(dev);
/*
* if PCI_REGION_MEM is set we do a two pass search with preference
* on matches that don't have PCI_REGION_SYS_MEMORY set
*/
if ((flags & PCI_REGION_TYPE) == PCI_REGION_MEM) {
ret = _dm_pci_bus_to_phys(ctlr, bus_addr, len,
flags, PCI_REGION_SYS_MEMORY,
&phys_addr);
if (!ret)
return phys_addr;
}
ret = _dm_pci_bus_to_phys(ctlr, bus_addr, len, flags, 0, &phys_addr);
if (ret)
puts("pci_hose_bus_to_phys: invalid physical address\n");
return phys_addr;
-}
-static int _dm_pci_phys_to_bus(struct udevice *dev, phys_addr_t phys_addr,
size_t len, unsigned long flags,
unsigned long skip_mask, pci_addr_t *ba)
-{
struct pci_controller *hose; struct pci_region *res;
struct udevice *ctlr; phys_addr_t offset; int i;
struct pci_controller *hose; /* The root controller has the region information */ ctlr = pci_get_controller(dev); hose = dev_get_uclass_priv(ctlr);
if (hose->region_count == 0) {
*ba = phys_addr;
return 0;
}
if (hose->region_count == 0)
return phys_addr; for (i = 0; i < hose->region_count; i++) { res = &hose->regions[i];
if (((res->flags ^ flags) & PCI_REGION_TYPE) != 0)
continue;
if (res->flags & skip_mask)
if ((res->flags & mask) != flags) continue; if (phys_addr < res->phys_start)
@@ -1502,36 +1467,11 @@ static int _dm_pci_phys_to_bus(struct udevice *dev, phys_addr_t phys_addr, if (len > res->size - offset) continue;
*ba = res->bus_start + offset;
return 0;
}
return 1;
-}
-pci_addr_t dm_pci_phys_to_bus(struct udevice *dev, phys_addr_t phys_addr,
size_t len, unsigned long flags)
-{
pci_addr_t bus_addr = 0;
int ret;
/*
* if PCI_REGION_MEM is set we do a two pass search with preference
* on matches that don't have PCI_REGION_SYS_MEMORY set
*/
if ((flags & PCI_REGION_TYPE) == PCI_REGION_MEM) {
ret = _dm_pci_phys_to_bus(dev, phys_addr, len, flags,
PCI_REGION_SYS_MEMORY, &bus_addr);
if (!ret)
return bus_addr;
return res->bus_start + offset; }
ret = _dm_pci_phys_to_bus(dev, phys_addr, len, flags, 0, &bus_addr);
if (ret)
puts("pci_hose_phys_to_bus: invalid physical address\n");
return bus_addr;
puts("pci_hose_phys_to_bus: invalid physical address\n");
return 0;
}
static phys_addr_t dm_pci_map_ea_virt(struct udevice *dev, int ea_off, diff --git a/include/pci.h b/include/pci.h index d137debb68..8198265269 100644 --- a/include/pci.h +++ b/include/pci.h @@ -1441,11 +1441,12 @@ u32 dm_pci_read_bar32(const struct udevice *dev, int barnum);
- @dev: Device containing the PCI address
- @addr: PCI address to convert
- @len: Length of the address range
- @mask: Mask to match flags for the region type
nits: Mask should be left aligned ...
- @flags: Flags for the region type (PCI_REGION_...)
- Return: physical address corresponding to that PCI bus address
*/ phys_addr_t dm_pci_bus_to_phys(struct udevice *dev, pci_addr_t addr, size_t len,
unsigned long flags);
unsigned long mask, unsigned long flags);
/**
- dm_pci_phys_to_bus() - convert a physical address to a PCI bus address
@@ -1453,11 +1454,12 @@ phys_addr_t dm_pci_bus_to_phys(struct udevice *dev, pci_addr_t addr, size_t len,
- @dev: Device containing the bus address
- @addr: Physical address to convert
- @len: Length of the address range
- @mask: Mask to match flags for the region type
ditto
- @flags: Flags for the region type (PCI_REGION_...)
- Return: PCI bus address corresponding to that physical address
*/ pci_addr_t dm_pci_phys_to_bus(struct udevice *dev, phys_addr_t addr, size_t len,
unsigned long flags);
unsigned long mask, unsigned long flags);
/**
- dm_pci_map_bar() - get a virtual address associated with a BAR region
@@ -1581,19 +1583,19 @@ int dm_pci_find_ext_capability(struct udevice *dev, int cap); int dm_pci_flr(struct udevice *dev);
#define dm_pci_virt_to_bus(dev, addr, flags) \
dm_pci_phys_to_bus(dev, (virt_to_phys(addr)), 0, (flags))
dm_pci_phys_to_bus(dev, (virt_to_phys(addr)), 0, PCI_REGION_TYPE, (flags))
#define dm_pci_bus_to_virt(dev, addr, flags, len, map_flags) \
map_physmem(dm_pci_bus_to_phys(dev, (addr), (len), (flags)), \
map_physmem(dm_pci_bus_to_phys(dev, (addr), (len), PCI_REGION_TYPE, (flags)), \ (len), (map_flags))
#define dm_pci_phys_to_mem(dev, addr) \
dm_pci_phys_to_bus((dev), (addr), 0, PCI_REGION_MEM)
dm_pci_phys_to_bus((dev), (addr), 0, PCI_REGION_TYPE, PCI_REGION_MEM)
#define dm_pci_mem_to_phys(dev, addr) \
dm_pci_bus_to_phys((dev), (addr), 0, PCI_REGION_MEM)
dm_pci_bus_to_phys((dev), (addr), 0, PCI_REGION_TYPE, PCI_REGION_MEM)
#define dm_pci_phys_to_io(dev, addr) \
dm_pci_phys_to_bus((dev), (addr), 0, PCI_REGION_IO)
dm_pci_phys_to_bus((dev), (addr), 0, PCI_REGION_TYPE, PCI_REGION_IO)
#define dm_pci_io_to_phys(dev, addr) \
dm_pci_bus_to_phys((dev), (addr), 0, PCI_REGION_IO)
dm_pci_bus_to_phys((dev), (addr), 0, PCI_REGION_TYPE, PCI_REGION_IO)
#define dm_pci_virt_to_mem(dev, addr) \ dm_pci_virt_to_bus((dev), (addr), PCI_REGION_MEM) diff --git a/test/dm/pci.c b/test/dm/pci.c index c8598e4c17..edc407f9d3 100644 --- a/test/dm/pci.c +++ b/test/dm/pci.c @@ -383,45 +383,47 @@ DM_TEST(dm_test_pci_region_multi, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT); */ static int dm_test_pci_bus_to_phys(struct unit_test_state *uts) {
unsigned long mask = PCI_REGION_TYPE;
unsigned long flags = PCI_REGION_MEM; struct udevice *dev; phys_addr_t phys_addr; ut_assertok(dm_pci_bus_find_bdf(PCI_BDF(1, 0x08, 0), &dev)); /* Before any of the ranges. */
phys_addr = dm_pci_bus_to_phys(dev, 0x20000000, 0x400, PCI_REGION_MEM);
phys_addr = dm_pci_bus_to_phys(dev, 0x20000000, 0x400, mask, flags); ut_asserteq(0, phys_addr); /* Identity range: whole, start, mid, end */
phys_addr = dm_pci_bus_to_phys(dev, 0x2fff0000, 0x2000, PCI_REGION_MEM);
phys_addr = dm_pci_bus_to_phys(dev, 0x2fff0000, 0x2000, mask, flags); ut_asserteq(0, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x30000000, 0x2000, PCI_REGION_MEM);
phys_addr = dm_pci_bus_to_phys(dev, 0x30000000, 0x2000, mask, flags); ut_asserteq(0x30000000, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x30000000, 0x1000, PCI_REGION_MEM);
phys_addr = dm_pci_bus_to_phys(dev, 0x30000000, 0x1000, mask, flags); ut_asserteq(0x30000000, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x30000abc, 0x12, PCI_REGION_MEM);
phys_addr = dm_pci_bus_to_phys(dev, 0x30000abc, 0x12, mask, flags); ut_asserteq(0x30000abc, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x30000800, 0x1800, PCI_REGION_MEM);
phys_addr = dm_pci_bus_to_phys(dev, 0x30000800, 0x1800, mask, flags); ut_asserteq(0x30000800, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x30008000, 0x1801, PCI_REGION_MEM);
phys_addr = dm_pci_bus_to_phys(dev, 0x30008000, 0x1801, mask, flags); ut_asserteq(0, phys_addr); /* Translated range: whole, start, mid, end */
phys_addr = dm_pci_bus_to_phys(dev, 0x30ff0000, 0x2000, PCI_REGION_MEM);
phys_addr = dm_pci_bus_to_phys(dev, 0x30ff0000, 0x2000, mask, flags); ut_asserteq(0, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x31000000, 0x2000, PCI_REGION_MEM);
phys_addr = dm_pci_bus_to_phys(dev, 0x31000000, 0x2000, mask, flags); ut_asserteq(0x3e000000, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x31000000, 0x1000, PCI_REGION_MEM);
phys_addr = dm_pci_bus_to_phys(dev, 0x31000000, 0x1000, mask, flags); ut_asserteq(0x3e000000, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x31000abc, 0x12, PCI_REGION_MEM);
phys_addr = dm_pci_bus_to_phys(dev, 0x31000abc, 0x12, mask, flags); ut_asserteq(0x3e000abc, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x31000800, 0x1800, PCI_REGION_MEM);
phys_addr = dm_pci_bus_to_phys(dev, 0x31000800, 0x1800, mask, flags); ut_asserteq(0x3e000800, phys_addr);
phys_addr = dm_pci_bus_to_phys(dev, 0x31008000, 0x1801, PCI_REGION_MEM);
phys_addr = dm_pci_bus_to_phys(dev, 0x31008000, 0x1801, mask, flags); ut_asserteq(0, phys_addr); /* Beyond all of the ranges. */
phys_addr = dm_pci_bus_to_phys(dev, 0x32000000, 0x400, PCI_REGION_MEM);
phys_addr = dm_pci_bus_to_phys(dev, 0x32000000, 0x400, mask, flags); ut_asserteq(0, phys_addr); return 0;
@@ -434,45 +436,47 @@ DM_TEST(dm_test_pci_bus_to_phys, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT); */ static int dm_test_pci_phys_to_bus(struct unit_test_state *uts) {
unsigned long mask = PCI_REGION_TYPE;
unsigned long flags = PCI_REGION_MEM; struct udevice *dev; phys_addr_t phys_addr; ut_assertok(dm_pci_bus_find_bdf(PCI_BDF(1, 0x08, 0), &dev)); /* Before any of the ranges. */
phys_addr = dm_pci_phys_to_bus(dev, 0x20000000, 0x400, PCI_REGION_MEM);
phys_addr = dm_pci_phys_to_bus(dev, 0x20000000, 0x400, mask, flags); ut_asserteq(0, phys_addr); /* Identity range: whole, start, mid, end */
phys_addr = dm_pci_phys_to_bus(dev, 0x2fff0000, 0x2000, PCI_REGION_MEM);
phys_addr = dm_pci_phys_to_bus(dev, 0x2fff0000, 0x2000, mask, flags); ut_asserteq(0, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x30000000, 0x2000, PCI_REGION_MEM);
phys_addr = dm_pci_phys_to_bus(dev, 0x30000000, 0x2000, mask, flags); ut_asserteq(0x30000000, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x30000000, 0x1000, PCI_REGION_MEM);
phys_addr = dm_pci_phys_to_bus(dev, 0x30000000, 0x1000, mask, flags); ut_asserteq(0x30000000, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x30000abc, 0x12, PCI_REGION_MEM);
phys_addr = dm_pci_phys_to_bus(dev, 0x30000abc, 0x12, mask, flags); ut_asserteq(0x30000abc, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x30000800, 0x1800, PCI_REGION_MEM);
phys_addr = dm_pci_phys_to_bus(dev, 0x30000800, 0x1800, mask, flags); ut_asserteq(0x30000800, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x30008000, 0x1801, PCI_REGION_MEM);
phys_addr = dm_pci_phys_to_bus(dev, 0x30008000, 0x1801, mask, flags); ut_asserteq(0, phys_addr); /* Translated range: whole, start, mid, end */
phys_addr = dm_pci_phys_to_bus(dev, 0x3dff0000, 0x2000, PCI_REGION_MEM);
phys_addr = dm_pci_phys_to_bus(dev, 0x3dff0000, 0x2000, mask, flags); ut_asserteq(0, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x3e000000, 0x2000, PCI_REGION_MEM);
phys_addr = dm_pci_phys_to_bus(dev, 0x3e000000, 0x2000, mask, flags); ut_asserteq(0x31000000, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x3e000000, 0x1000, PCI_REGION_MEM);
phys_addr = dm_pci_phys_to_bus(dev, 0x3e000000, 0x1000, mask, flags); ut_asserteq(0x31000000, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x3e000abc, 0x12, PCI_REGION_MEM);
phys_addr = dm_pci_phys_to_bus(dev, 0x3e000abc, 0x12, mask, flags); ut_asserteq(0x31000abc, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x3e000800, 0x1800, PCI_REGION_MEM);
phys_addr = dm_pci_phys_to_bus(dev, 0x3e000800, 0x1800, mask, flags); ut_asserteq(0x31000800, phys_addr);
phys_addr = dm_pci_phys_to_bus(dev, 0x3e008000, 0x1801, PCI_REGION_MEM);
phys_addr = dm_pci_phys_to_bus(dev, 0x3e008000, 0x1801, mask, flags); ut_asserteq(0, phys_addr); /* Beyond all of the ranges. */
phys_addr = dm_pci_phys_to_bus(dev, 0x3f000000, 0x400, PCI_REGION_MEM);
phys_addr = dm_pci_phys_to_bus(dev, 0x3f000000, 0x400, mask, flags); ut_asserteq(0, phys_addr); return 0;
Otherwise, Reviewed-by: Bin Meng bmeng.cn@gmail.com

Add mask parameter and reorder length parameter to match the other PCI address conversion functions. Using PCI_REGION_TYPE as the mask gives the old behaviour.
It's converted from a macro to an inline function as the length parameter is now used twice, but should only be calculated once.
Signed-off-by: Andrew Scull ascull@google.com --- drivers/bios_emulator/atibios.c | 4 ++-- drivers/pci/pci-uclass.c | 4 ++-- include/pci.h | 19 ++++++++++++++----- 3 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/drivers/bios_emulator/atibios.c b/drivers/bios_emulator/atibios.c index 9547470a2f..cdc5ba6ad9 100644 --- a/drivers/bios_emulator/atibios.c +++ b/drivers/bios_emulator/atibios.c @@ -368,8 +368,8 @@ void *PCI_mapBIOSImage(struct udevice *pcidev) return NULL; }
- BIOSImage = dm_pci_bus_to_virt(pcidev, BIOSImageBus, - PCI_REGION_MEM, 0, MAP_NOCACHE); + BIOSImage = dm_pci_bus_to_virt(pcidev, BIOSImageBus, 0, PCI_REGION_TYPE, + PCI_REGION_MEM, MAP_NOCACHE);
/*Change the PCI BAR registers to map it onto the bus.*/ dm_pci_write_config32(pcidev, BIOSImageBAR, 0); diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 5069ada66d..676052090a 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -1602,8 +1602,8 @@ void *dm_pci_map_bar(struct udevice *dev, int bar, size_t offset, size_t len, * a PCI range, but a better check would be to probe for the size of * the bar and prevent overflow more locally. */ - return dm_pci_bus_to_virt(udev, pci_bus_addr + offset, flags, len, - MAP_NOCACHE); + return dm_pci_bus_to_virt(udev, pci_bus_addr + offset, len, + PCI_REGION_TYPE, flags, MAP_NOCACHE); }
static int _dm_pci_find_next_capability(struct udevice *dev, u8 pos, int cap) diff --git a/include/pci.h b/include/pci.h index 8198265269..683efcd1be 100644 --- a/include/pci.h +++ b/include/pci.h @@ -11,6 +11,8 @@ #ifndef _PCI_H #define _PCI_H
+#include <asm/io.h> + #define PCI_CFG_SPACE_SIZE 256 #define PCI_CFG_SPACE_EXP_SIZE 4096
@@ -1584,9 +1586,14 @@ int dm_pci_flr(struct udevice *dev);
#define dm_pci_virt_to_bus(dev, addr, flags) \ dm_pci_phys_to_bus(dev, (virt_to_phys(addr)), 0, PCI_REGION_TYPE, (flags)) -#define dm_pci_bus_to_virt(dev, addr, flags, len, map_flags) \ - map_physmem(dm_pci_bus_to_phys(dev, (addr), (len), PCI_REGION_TYPE, (flags)), \ - (len), (map_flags)) + +static inline void *dm_pci_bus_to_virt(struct udevice *dev, pci_addr_t addr, + size_t len, unsigned long mask, + unsigned long flags, + unsigned long map_flags) { + return map_physmem(dm_pci_bus_to_phys(dev, addr, len, mask, flags), + len, map_flags); +}
#define dm_pci_phys_to_mem(dev, addr) \ dm_pci_phys_to_bus((dev), (addr), 0, PCI_REGION_TYPE, PCI_REGION_MEM) @@ -1600,11 +1607,13 @@ int dm_pci_flr(struct udevice *dev); #define dm_pci_virt_to_mem(dev, addr) \ dm_pci_virt_to_bus((dev), (addr), PCI_REGION_MEM) #define dm_pci_mem_to_virt(dev, addr, len, map_flags) \ - dm_pci_bus_to_virt((dev), (addr), PCI_REGION_MEM, (len), (map_flags)) + dm_pci_bus_to_virt((dev), (addr), (len), PCI_REGION_TYPE, \ + PCI_REGION_MEM, (map_flags)) #define dm_pci_virt_to_io(dev, addr) \ dm_pci_virt_to_bus((dev), (addr), PCI_REGION_IO) #define dm_pci_io_to_virt(dev, addr, len, map_flags) \ - dm_pci_bus_to_virt((dev), (addr), PCI_REGION_IO, (len), (map_flags)) + dm_pci_bus_to_virt((dev), (addr), (len), PCI_REGION_TYPE, \ + PCI_REGION_IO, (map_flags))
/** * dm_pci_find_device() - find a device by vendor/device ID

On Wed, Mar 30, 2022 at 12:59 AM Andrew Scull ascull@google.com wrote:
Add mask parameter and reorder length parameter to match the other PCI address conversion functions. Using PCI_REGION_TYPE as the mask gives the old behaviour.
It's converted from a macro to an inline function as the length parameter is now used twice, but should only be calculated once.
Signed-off-by: Andrew Scull ascull@google.com
drivers/bios_emulator/atibios.c | 4 ++-- drivers/pci/pci-uclass.c | 4 ++-- include/pci.h | 19 ++++++++++++++----- 3 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/drivers/bios_emulator/atibios.c b/drivers/bios_emulator/atibios.c index 9547470a2f..cdc5ba6ad9 100644 --- a/drivers/bios_emulator/atibios.c +++ b/drivers/bios_emulator/atibios.c @@ -368,8 +368,8 @@ void *PCI_mapBIOSImage(struct udevice *pcidev) return NULL; }
BIOSImage = dm_pci_bus_to_virt(pcidev, BIOSImageBus,
PCI_REGION_MEM, 0, MAP_NOCACHE);
BIOSImage = dm_pci_bus_to_virt(pcidev, BIOSImageBus, 0, PCI_REGION_TYPE,
PCI_REGION_MEM, MAP_NOCACHE); /*Change the PCI BAR registers to map it onto the bus.*/ dm_pci_write_config32(pcidev, BIOSImageBAR, 0);
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 5069ada66d..676052090a 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -1602,8 +1602,8 @@ void *dm_pci_map_bar(struct udevice *dev, int bar, size_t offset, size_t len, * a PCI range, but a better check would be to probe for the size of * the bar and prevent overflow more locally. */
return dm_pci_bus_to_virt(udev, pci_bus_addr + offset, flags, len,
MAP_NOCACHE);
return dm_pci_bus_to_virt(udev, pci_bus_addr + offset, len,
PCI_REGION_TYPE, flags, MAP_NOCACHE);
}
static int _dm_pci_find_next_capability(struct udevice *dev, u8 pos, int cap) diff --git a/include/pci.h b/include/pci.h index 8198265269..683efcd1be 100644 --- a/include/pci.h +++ b/include/pci.h @@ -11,6 +11,8 @@ #ifndef _PCI_H #define _PCI_H
+#include <asm/io.h>
#define PCI_CFG_SPACE_SIZE 256 #define PCI_CFG_SPACE_EXP_SIZE 4096
@@ -1584,9 +1586,14 @@ int dm_pci_flr(struct udevice *dev);
#define dm_pci_virt_to_bus(dev, addr, flags) \ dm_pci_phys_to_bus(dev, (virt_to_phys(addr)), 0, PCI_REGION_TYPE, (flags)) -#define dm_pci_bus_to_virt(dev, addr, flags, len, map_flags) \
map_physmem(dm_pci_bus_to_phys(dev, (addr), (len), PCI_REGION_TYPE, (flags)), \
(len), (map_flags))
+static inline void *dm_pci_bus_to_virt(struct udevice *dev, pci_addr_t addr,
size_t len, unsigned long mask,
unsigned long flags,
unsigned long map_flags) {
return map_physmem(dm_pci_bus_to_phys(dev, addr, len, mask, flags),
len, map_flags);
+}
#define dm_pci_phys_to_mem(dev, addr) \ dm_pci_phys_to_bus((dev), (addr), 0, PCI_REGION_TYPE, PCI_REGION_MEM) @@ -1600,11 +1607,13 @@ int dm_pci_flr(struct udevice *dev); #define dm_pci_virt_to_mem(dev, addr) \ dm_pci_virt_to_bus((dev), (addr), PCI_REGION_MEM) #define dm_pci_mem_to_virt(dev, addr, len, map_flags) \
dm_pci_bus_to_virt((dev), (addr), PCI_REGION_MEM, (len), (map_flags))
dm_pci_bus_to_virt((dev), (addr), (len), PCI_REGION_TYPE, \
PCI_REGION_MEM, (map_flags))
#define dm_pci_virt_to_io(dev, addr) \ dm_pci_virt_to_bus((dev), (addr), PCI_REGION_IO) #define dm_pci_io_to_virt(dev, addr, len, map_flags) \
dm_pci_bus_to_virt((dev), (addr), PCI_REGION_IO, (len), (map_flags))
dm_pci_bus_to_virt((dev), (addr), (len), PCI_REGION_TYPE, \
PCI_REGION_IO, (map_flags))
/**
- dm_pci_find_device() - find a device by vendor/device ID
--
I believe this patch should be squashed into patch #14 for bisectablitly.
Regards, Bin

On Wed, 13 Apr 2022 at 16:03, Bin Meng bmeng.cn@gmail.com wrote:
On Wed, Mar 30, 2022 at 12:59 AM Andrew Scull ascull@google.com wrote:
Add mask parameter and reorder length parameter to match the other PCI address conversion functions. Using PCI_REGION_TYPE as the mask gives the old behaviour.
It's converted from a macro to an inline function as the length parameter is now used twice, but should only be calculated once.
Signed-off-by: Andrew Scull ascull@google.com
drivers/bios_emulator/atibios.c | 4 ++-- drivers/pci/pci-uclass.c | 4 ++-- include/pci.h | 19 ++++++++++++++----- 3 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/drivers/bios_emulator/atibios.c b/drivers/bios_emulator/atibios.c index 9547470a2f..cdc5ba6ad9 100644 --- a/drivers/bios_emulator/atibios.c +++ b/drivers/bios_emulator/atibios.c @@ -368,8 +368,8 @@ void *PCI_mapBIOSImage(struct udevice *pcidev) return NULL; }
BIOSImage = dm_pci_bus_to_virt(pcidev, BIOSImageBus,
PCI_REGION_MEM, 0, MAP_NOCACHE);
BIOSImage = dm_pci_bus_to_virt(pcidev, BIOSImageBus, 0, PCI_REGION_TYPE,
PCI_REGION_MEM, MAP_NOCACHE); /*Change the PCI BAR registers to map it onto the bus.*/ dm_pci_write_config32(pcidev, BIOSImageBAR, 0);
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 5069ada66d..676052090a 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -1602,8 +1602,8 @@ void *dm_pci_map_bar(struct udevice *dev, int bar, size_t offset, size_t len, * a PCI range, but a better check would be to probe for the size of * the bar and prevent overflow more locally. */
return dm_pci_bus_to_virt(udev, pci_bus_addr + offset, flags, len,
MAP_NOCACHE);
return dm_pci_bus_to_virt(udev, pci_bus_addr + offset, len,
PCI_REGION_TYPE, flags, MAP_NOCACHE);
}
static int _dm_pci_find_next_capability(struct udevice *dev, u8 pos, int cap) diff --git a/include/pci.h b/include/pci.h index 8198265269..683efcd1be 100644 --- a/include/pci.h +++ b/include/pci.h @@ -11,6 +11,8 @@ #ifndef _PCI_H #define _PCI_H
+#include <asm/io.h>
#define PCI_CFG_SPACE_SIZE 256 #define PCI_CFG_SPACE_EXP_SIZE 4096
@@ -1584,9 +1586,14 @@ int dm_pci_flr(struct udevice *dev);
#define dm_pci_virt_to_bus(dev, addr, flags) \ dm_pci_phys_to_bus(dev, (virt_to_phys(addr)), 0, PCI_REGION_TYPE, (flags)) -#define dm_pci_bus_to_virt(dev, addr, flags, len, map_flags) \
map_physmem(dm_pci_bus_to_phys(dev, (addr), (len), PCI_REGION_TYPE, (flags)), \
(len), (map_flags))
+static inline void *dm_pci_bus_to_virt(struct udevice *dev, pci_addr_t addr,
size_t len, unsigned long mask,
unsigned long flags,
unsigned long map_flags) {
return map_physmem(dm_pci_bus_to_phys(dev, addr, len, mask, flags),
len, map_flags);
+}
#define dm_pci_phys_to_mem(dev, addr) \ dm_pci_phys_to_bus((dev), (addr), 0, PCI_REGION_TYPE, PCI_REGION_MEM) @@ -1600,11 +1607,13 @@ int dm_pci_flr(struct udevice *dev); #define dm_pci_virt_to_mem(dev, addr) \ dm_pci_virt_to_bus((dev), (addr), PCI_REGION_MEM) #define dm_pci_mem_to_virt(dev, addr, len, map_flags) \
dm_pci_bus_to_virt((dev), (addr), PCI_REGION_MEM, (len), (map_flags))
dm_pci_bus_to_virt((dev), (addr), (len), PCI_REGION_TYPE, \
PCI_REGION_MEM, (map_flags))
#define dm_pci_virt_to_io(dev, addr) \ dm_pci_virt_to_bus((dev), (addr), PCI_REGION_IO) #define dm_pci_io_to_virt(dev, addr, len, map_flags) \
dm_pci_bus_to_virt((dev), (addr), PCI_REGION_IO, (len), (map_flags))
dm_pci_bus_to_virt((dev), (addr), (len), PCI_REGION_TYPE, \
PCI_REGION_IO, (map_flags))
/**
- dm_pci_find_device() - find a device by vendor/device ID
--
I believe this patch should be squashed into patch #14 for bisectablitly.
I made sure it at least bisects on sandbox. #14 adds the mask parameter and the callers were made to pass PCI_REGION_TYPE as the mask. dm_pci_bus_to_virt() was one of those callers and this patch then goes on to expose the mask with a new parameter to dm_pci_bus_to_virt().
I am having to change this back from an inline function to a macro using expression statements as some platforms ran into problems with inclusion orders.

Add a mask parameter to control the lookup of the PCI region from which the mapping can be made.
Signed-off-by: Andrew Scull ascull@google.com --- arch/x86/cpu/baytrail/cpu.c | 2 +- drivers/ata/ahci.c | 7 ++++--- drivers/gpio/octeon_gpio.c | 2 +- drivers/i2c/designware_i2c_pci.c | 3 ++- drivers/i2c/intel_i2c.c | 2 +- drivers/i2c/octeon_i2c.c | 2 +- drivers/mmc/octeontx_hsmmc.c | 2 +- drivers/mmc/pci_mmc.c | 2 +- drivers/mtd/nand/raw/octeontx_bch.c | 6 ++++-- drivers/mtd/nand/raw/octeontx_nand.c | 2 +- drivers/net/bnxt/bnxt.c | 9 ++++++--- drivers/net/fsl_enetc.c | 2 +- drivers/net/fsl_enetc_mdio.c | 2 +- drivers/net/mscc_eswitch/felix_switch.c | 4 ++-- drivers/net/octeontx/bgx.c | 2 +- drivers/net/octeontx/nic_main.c | 2 +- drivers/net/octeontx/nicvf_main.c | 2 +- drivers/net/octeontx/smi.c | 2 +- drivers/net/octeontx2/cgx.c | 2 +- drivers/net/octeontx2/rvu_af.c | 2 +- drivers/net/octeontx2/rvu_pf.c | 3 ++- drivers/net/pch_gbe.c | 2 +- drivers/nvme/nvme_pci.c | 2 +- drivers/pci/pci-uclass.c | 6 +++--- drivers/spi/octeon_spi.c | 2 +- drivers/usb/host/ohci-pci.c | 2 +- drivers/virtio/virtio_pci_legacy.c | 3 ++- include/pci.h | 3 ++- test/dm/pci.c | 12 ++++++------ 29 files changed, 52 insertions(+), 42 deletions(-)
diff --git a/arch/x86/cpu/baytrail/cpu.c b/arch/x86/cpu/baytrail/cpu.c index 6f1b05175c..3eb10b53bb 100644 --- a/arch/x86/cpu/baytrail/cpu.c +++ b/arch/x86/cpu/baytrail/cpu.c @@ -55,7 +55,7 @@ int arch_cpu_init_dm(void) for (i = 0; i < 2; i++) { ret = dm_pci_bus_find_bdf(PCI_BDF(0, 0x1e, 3 + i), &dev); if (!ret) { - base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, + base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); hsuart_clock_set(base); } diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index 3925807d55..de6131f1d9 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -417,7 +417,7 @@ static int ahci_init_one(struct ahci_uc_priv *uc_priv, struct udevice *dev)
#if !defined(CONFIG_DM_SCSI) uc_priv->mmio_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_5, 0, 0, - PCI_REGION_MEM); + PCI_REGION_TYPE, PCI_REGION_MEM);
/* Take from kernel: * JMicron-specific fixup: @@ -1149,7 +1149,7 @@ int ahci_probe_scsi_pci(struct udevice *ahci_dev) u16 vendor, device;
base = (ulong)dm_pci_map_bar(ahci_dev, PCI_BASE_ADDRESS_5, 0, 0, - PCI_REGION_MEM); + PCI_REGION_TYPE, PCI_REGION_MEM);
/* * Note: @@ -1163,7 +1163,8 @@ int ahci_probe_scsi_pci(struct udevice *ahci_dev)
if (vendor == PCI_VENDOR_ID_CAVIUM && device == PCI_DEVICE_ID_CAVIUM_SATA) - base = (uintptr_t)dm_pci_map_bar(ahci_dev, PCI_BASE_ADDRESS_0, 0, 0, + base = (uintptr_t)dm_pci_map_bar(ahci_dev, PCI_BASE_ADDRESS_0, + 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); return ahci_probe_scsi(ahci_dev, base); } diff --git a/drivers/gpio/octeon_gpio.c b/drivers/gpio/octeon_gpio.c index e6a8e1a521..2b2465b1bc 100644 --- a/drivers/gpio/octeon_gpio.c +++ b/drivers/gpio/octeon_gpio.c @@ -183,7 +183,7 @@ static int octeon_gpio_probe(struct udevice *dev) priv->data = (const struct octeon_gpio_data *)dev_get_driver_data(dev);
if (priv->data->probe == PROBE_PCI) { - priv->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, + priv->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); uc_priv->gpio_count = readq(priv->base + priv->data->reg_offs + GPIO_CONST) & diff --git a/drivers/i2c/designware_i2c_pci.c b/drivers/i2c/designware_i2c_pci.c index 51f1357d10..1572c2c6bc 100644 --- a/drivers/i2c/designware_i2c_pci.c +++ b/drivers/i2c/designware_i2c_pci.c @@ -59,7 +59,8 @@ static int designware_i2c_pci_of_to_plat(struct udevice *dev) priv->regs = (struct i2c_regs *)dm_pci_read_bar32(dev, 0); } else { priv->regs = (struct i2c_regs *) - dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM); + dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, + PCI_REGION_TYPE, PCI_REGION_MEM); } if (!priv->regs) return -EINVAL; diff --git a/drivers/i2c/intel_i2c.c b/drivers/i2c/intel_i2c.c index 7b5b62e3eb..dc26fa8c54 100644 --- a/drivers/i2c/intel_i2c.c +++ b/drivers/i2c/intel_i2c.c @@ -251,7 +251,7 @@ static int intel_i2c_probe(struct udevice *dev) ulong base;
/* Save base address from PCI BAR */ - priv->base = (ulong)dm_pci_map_bar(dev, PCI_BASE_ADDRESS_4, 0, 0, + priv->base = (ulong)dm_pci_map_bar(dev, PCI_BASE_ADDRESS_4, 0, 0, PCI_REGION_TYPE, PCI_REGION_IO); base = priv->base;
diff --git a/drivers/i2c/octeon_i2c.c b/drivers/i2c/octeon_i2c.c index 74fd5c3d2e..e54ef18e51 100644 --- a/drivers/i2c/octeon_i2c.c +++ b/drivers/i2c/octeon_i2c.c @@ -792,7 +792,7 @@ static int octeon_i2c_probe(struct udevice *dev)
debug("TWSI PCI device: %x\n", bdf);
- twsi->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, + twsi->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); } else { twsi->base = dev_remap_addr(dev); diff --git a/drivers/mmc/octeontx_hsmmc.c b/drivers/mmc/octeontx_hsmmc.c index 0bf38945a1..6e9acf7310 100644 --- a/drivers/mmc/octeontx_hsmmc.c +++ b/drivers/mmc/octeontx_hsmmc.c @@ -3822,7 +3822,7 @@ static int octeontx_mmc_host_probe(struct udevice *dev)
/* Octeon TX & TX2 use PCI based probing */ if (device_is_compatible(dev, "cavium,thunder-8890-mmc")) { - host->base_addr = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, + host->base_addr = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); if (!host->base_addr) { pr_err("%s: Error: MMC base address not found\n", diff --git a/drivers/mmc/pci_mmc.c b/drivers/mmc/pci_mmc.c index 1bc2fbcfdf..cba2ea8cf3 100644 --- a/drivers/mmc/pci_mmc.c +++ b/drivers/mmc/pci_mmc.c @@ -50,7 +50,7 @@ static int pci_mmc_probe(struct udevice *dev) desc = mmc_get_blk_desc(&plat->mmc); desc->removable = !(plat->cfg.host_caps & MMC_CAP_NONREMOVABLE);
- host->ioaddr = (void *)dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, + host->ioaddr = (void *)dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); host->name = dev->name; host->cd_gpio = priv->cd_gpio; diff --git a/drivers/mtd/nand/raw/octeontx_bch.c b/drivers/mtd/nand/raw/octeontx_bch.c index c1cc5fa187..c1d721cabf 100644 --- a/drivers/mtd/nand/raw/octeontx_bch.c +++ b/drivers/mtd/nand/raw/octeontx_bch.c @@ -176,7 +176,8 @@ static int octeontx_pci_bchpf_probe(struct udevice *dev) if (!bch) return -ENOMEM;
- bch->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM); + bch->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, + PCI_REGION_TYPE, PCI_REGION_MEM); bch->dev = dev;
debug("%s: base address: %p\n", __func__, bch->reg_base); @@ -361,7 +362,8 @@ static int octeontx_pci_bchvf_probe(struct udevice *dev) vf->dev = dev;
/* Map PF's configuration registers */ - vf->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM); + vf->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, + PCI_REGION_TYPE, PCI_REGION_MEM); debug("%s: reg base: %p\n", __func__, vf->reg_base);
err = octeontx_cmd_queue_initialize(dev, QID_BCH, QDEPTH - 1, 0, diff --git a/drivers/mtd/nand/raw/octeontx_nand.c b/drivers/mtd/nand/raw/octeontx_nand.c index 3e84bb2fc0..b338b204f3 100644 --- a/drivers/mtd/nand/raw/octeontx_nand.c +++ b/drivers/mtd/nand/raw/octeontx_nand.c @@ -2098,7 +2098,7 @@ static int octeontx_pci_nand_probe(struct udevice *dev) tn->dev = dev; INIT_LIST_HEAD(&tn->chips);
- tn->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM); + tn->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); if (!tn->base) { ret = -EINVAL; goto release; diff --git a/drivers/net/bnxt/bnxt.c b/drivers/net/bnxt/bnxt.c index a24f965ec1..1c9a996240 100644 --- a/drivers/net/bnxt/bnxt.c +++ b/drivers/net/bnxt/bnxt.c @@ -28,9 +28,12 @@ static void bnxt_bring_pci(struct bnxt *bp) dm_pci_read_config16(bp->pdev, PCI_SUBSYSTEM_ID, &bp->subsystem_device); dm_pci_read_config16(bp->pdev, PCI_COMMAND, &bp->cmd_reg); dm_pci_read_config8(bp->pdev, PCI_INTERRUPT_LINE, &bp->irq); - bp->bar0 = dm_pci_map_bar(bp->pdev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM); - bp->bar1 = dm_pci_map_bar(bp->pdev, PCI_BASE_ADDRESS_2, 0, 0, PCI_REGION_MEM); - bp->bar2 = dm_pci_map_bar(bp->pdev, PCI_BASE_ADDRESS_4, 0, 0, PCI_REGION_MEM); + bp->bar0 = dm_pci_map_bar(bp->pdev, PCI_BASE_ADDRESS_0, 0, 0, + PCI_REGION_TYPE, PCI_REGION_MEM); + bp->bar1 = dm_pci_map_bar(bp->pdev, PCI_BASE_ADDRESS_2, 0, 0, + PCI_REGION_TYPE, PCI_REGION_MEM); + bp->bar2 = dm_pci_map_bar(bp->pdev, PCI_BASE_ADDRESS_4, 0, 0, + PCI_REGION_TYPE, PCI_REGION_MEM); cmd_reg = bp->cmd_reg | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER; cmd_reg |= PCI_COMMAND_INTX_DISABLE; /* disable intr */ dm_pci_write_config16(bp->pdev, PCI_COMMAND, cmd_reg); diff --git a/drivers/net/fsl_enetc.c b/drivers/net/fsl_enetc.c index 60a7b27c81..3aa835329e 100644 --- a/drivers/net/fsl_enetc.c +++ b/drivers/net/fsl_enetc.c @@ -344,7 +344,7 @@ static int enetc_probe(struct udevice *dev) }
/* initialize register */ - priv->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, 0); + priv->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, 0); if (!priv->regs_base) { enetc_dbg(dev, "failed to map BAR0\n"); return -EINVAL; diff --git a/drivers/net/fsl_enetc_mdio.c b/drivers/net/fsl_enetc_mdio.c index f025c2255c..50ad76dfeb 100644 --- a/drivers/net/fsl_enetc_mdio.c +++ b/drivers/net/fsl_enetc_mdio.c @@ -125,7 +125,7 @@ static int enetc_mdio_probe(struct udevice *dev) { struct enetc_mdio_priv *priv = dev_get_priv(dev);
- priv->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, 0); + priv->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, 0); if (!priv->regs_base) { enetc_dbg(dev, "failed to map BAR0\n"); return -EINVAL; diff --git a/drivers/net/mscc_eswitch/felix_switch.c b/drivers/net/mscc_eswitch/felix_switch.c index 0badb23828..709c9e3ef5 100644 --- a/drivers/net/mscc_eswitch/felix_switch.c +++ b/drivers/net/mscc_eswitch/felix_switch.c @@ -292,13 +292,13 @@ static int felix_probe(struct udevice *dev) return -ENODEV; }
- priv->imdio_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, 0); + priv->imdio_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, 0); if (!priv->imdio_base) { dev_err(dev, "failed to map BAR0\n"); return -EINVAL; }
- priv->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_4, 0, 0, 0); + priv->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_4, 0, 0, PCI_REGION_TYPE, 0); if (!priv->regs_base) { dev_err(dev, "failed to map BAR4\n"); return -EINVAL; diff --git a/drivers/net/octeontx/bgx.c b/drivers/net/octeontx/bgx.c index cc8ef099c2..b6592ff2ce 100644 --- a/drivers/net/octeontx/bgx.c +++ b/drivers/net/octeontx/bgx.c @@ -1458,7 +1458,7 @@ int octeontx_bgx_probe(struct udevice *dev) int bgx_idx, node; int inc = 1;
- bgx->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, + bgx->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); if (!bgx->reg_base) { debug("No PCI region found\n"); diff --git a/drivers/net/octeontx/nic_main.c b/drivers/net/octeontx/nic_main.c index 4754c042f1..99886e3afc 100644 --- a/drivers/net/octeontx/nic_main.c +++ b/drivers/net/octeontx/nic_main.c @@ -713,7 +713,7 @@ int nic_initialize(struct udevice *dev) return -ENOMEM;
/* MAP PF's configuration registers */ - nic->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, + nic->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); if (!nic->reg_base) { printf("Cannot map config register space, aborting\n"); diff --git a/drivers/net/octeontx/nicvf_main.c b/drivers/net/octeontx/nicvf_main.c index 097df6df1e..6e4d0a0512 100644 --- a/drivers/net/octeontx/nicvf_main.c +++ b/drivers/net/octeontx/nicvf_main.c @@ -509,7 +509,7 @@ int nicvf_initialize(struct udevice *dev) /* Enable TSO support */ nicvf->hw_tso = true;
- nicvf->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, + nicvf->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM);
debug("nicvf->reg_base: %p\n", nicvf->reg_base); diff --git a/drivers/net/octeontx/smi.c b/drivers/net/octeontx/smi.c index 2d521bd3ca..233c26f731 100644 --- a/drivers/net/octeontx/smi.c +++ b/drivers/net/octeontx/smi.c @@ -322,7 +322,7 @@ int octeontx_smi_probe(struct udevice *dev) u64 baseaddr;
debug("SMI PCI device: %x\n", bdf); - if (!dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM)) { + if (!dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM)) { printf("Failed to map PCI region for bdf %x\n", bdf); return -1; } diff --git a/drivers/net/octeontx2/cgx.c b/drivers/net/octeontx2/cgx.c index eed31a9579..c6ec3200c0 100644 --- a/drivers/net/octeontx2/cgx.c +++ b/drivers/net/octeontx2/cgx.c @@ -253,7 +253,7 @@ int cgx_probe(struct udevice *dev) struct cgx *cgx = dev_get_priv(dev); int err;
- cgx->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, + cgx->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); cgx->dev = dev; cgx->cgx_id = ((u64)(cgx->reg_base) >> 24) & 0x7; diff --git a/drivers/net/octeontx2/rvu_af.c b/drivers/net/octeontx2/rvu_af.c index 47c1502ef8..0d3a9ffe9e 100644 --- a/drivers/net/octeontx2/rvu_af.c +++ b/drivers/net/octeontx2/rvu_af.c @@ -127,7 +127,7 @@ int rvu_af_probe(struct udevice *dev) { struct rvu_af *af_ptr = dev_get_priv(dev);
- af_ptr->af_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, + af_ptr->af_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); debug("%s RVU AF BAR %p\n", __func__, af_ptr->af_base); af_ptr->dev = dev; diff --git a/drivers/net/octeontx2/rvu_pf.c b/drivers/net/octeontx2/rvu_pf.c index 024e17e748..5f3ea1f8ea 100644 --- a/drivers/net/octeontx2/rvu_pf.c +++ b/drivers/net/octeontx2/rvu_pf.c @@ -58,7 +58,8 @@ int rvu_pf_probe(struct udevice *dev)
debug("%s: name: %s\n", __func__, dev->name);
- rvu->pf_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_2, 0, 0, PCI_REGION_MEM); + rvu->pf_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_2, 0, 0, + PCI_REGION_TYPE, PCI_REGION_MEM); rvu->pfid = dev_seq(dev) + 1; // RVU PF's start from 1; rvu->dev = dev; if (!rvu_af_dev) { diff --git a/drivers/net/pch_gbe.c b/drivers/net/pch_gbe.c index c93cc2ead2..6b251ab63f 100644 --- a/drivers/net/pch_gbe.c +++ b/drivers/net/pch_gbe.c @@ -449,7 +449,7 @@ static int pch_gbe_probe(struct udevice *dev)
priv->dev = dev;
- iobase = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_1, 0, 0, PCI_REGION_MEM); + iobase = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_1, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM);
plat->iobase = (ulong)iobase; priv->mac_regs = (struct pch_gbe_regs *)iobase; diff --git a/drivers/nvme/nvme_pci.c b/drivers/nvme/nvme_pci.c index 3499a7b6e7..36bf9c5ffb 100644 --- a/drivers/nvme/nvme_pci.c +++ b/drivers/nvme/nvme_pci.c @@ -29,7 +29,7 @@ static int nvme_probe(struct udevice *udev)
ndev->instance = trailing_strtol(udev->name); ndev->bar = dm_pci_map_bar(udev, PCI_BASE_ADDRESS_0, 0, 0, - PCI_REGION_MEM); + PCI_REGION_TYPE, PCI_REGION_MEM); return nvme_init(udev); }
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 676052090a..b92ef23a7a 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -1560,7 +1560,7 @@ static void *dm_pci_map_ea_bar(struct udevice *dev, int bar, size_t offset, }
void *dm_pci_map_bar(struct udevice *dev, int bar, size_t offset, size_t len, - unsigned long flags) + unsigned long mask, unsigned long flags) { struct pci_child_plat *pdata = dev_get_parent_plat(dev); struct udevice *udev = dev; @@ -1602,8 +1602,8 @@ void *dm_pci_map_bar(struct udevice *dev, int bar, size_t offset, size_t len, * a PCI range, but a better check would be to probe for the size of * the bar and prevent overflow more locally. */ - return dm_pci_bus_to_virt(udev, pci_bus_addr + offset, len, - PCI_REGION_TYPE, flags, MAP_NOCACHE); + return dm_pci_bus_to_virt(udev, pci_bus_addr + offset, len, mask, flags, + MAP_NOCACHE); }
static int _dm_pci_find_next_capability(struct udevice *dev, u8 pos, int cap) diff --git a/drivers/spi/octeon_spi.c b/drivers/spi/octeon_spi.c index 2f8a8a8649..c2a7ee232b 100644 --- a/drivers/spi/octeon_spi.c +++ b/drivers/spi/octeon_spi.c @@ -568,7 +568,7 @@ static int octeon_spi_probe(struct udevice *dev) pci_dev_t bdf = dm_pci_get_bdf(dev);
debug("SPI PCI device: %x\n", bdf); - priv->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, + priv->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); /* Add base offset */ priv->base += 0x1000; diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c index eab0d96637..f061aec289 100644 --- a/drivers/usb/host/ohci-pci.c +++ b/drivers/usb/host/ohci-pci.c @@ -18,7 +18,7 @@ static int ohci_pci_probe(struct udevice *dev) { struct ohci_regs *regs;
- regs = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM); + regs = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); return ohci_register(dev, regs); }
diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c index 504a7ff7b9..cf5dfb17a9 100644 --- a/drivers/virtio/virtio_pci_legacy.c +++ b/drivers/virtio/virtio_pci_legacy.c @@ -319,7 +319,8 @@ static int virtio_pci_probe(struct udevice *udev) uc_priv->device = subdevice; uc_priv->vendor = subvendor;
- priv->ioaddr = dm_pci_map_bar(udev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_IO); + priv->ioaddr = dm_pci_map_bar(udev, PCI_BASE_ADDRESS_0, 0, 0, + PCI_REGION_TYPE, PCI_REGION_IO); if (!priv->ioaddr) return -ENXIO; debug("(%s): virtio legacy device reg base %04lx\n", diff --git a/include/pci.h b/include/pci.h index 683efcd1be..99122ed54f 100644 --- a/include/pci.h +++ b/include/pci.h @@ -1477,11 +1477,12 @@ pci_addr_t dm_pci_phys_to_bus(struct udevice *dev, phys_addr_t addr, size_t len, * @bar: Bar register offset (PCI_BASE_ADDRESS_...) * @offset: Offset from the base to map * @len: Length to map + * @mask: Mask to match flags for the region type * @flags: Flags for the region type (PCI_REGION_...) * @return: pointer to the virtual address to use or 0 on error */ void *dm_pci_map_bar(struct udevice *dev, int bar, size_t offset, size_t len, - unsigned long flags); + unsigned long mask, unsigned long flags);
/** * dm_pci_find_next_capability() - find a capability starting from an offset diff --git a/test/dm/pci.c b/test/dm/pci.c index edc407f9d3..4bbda4c407 100644 --- a/test/dm/pci.c +++ b/test/dm/pci.c @@ -268,27 +268,27 @@ static int dm_test_pci_ea(struct unit_test_state *uts) ut_asserteq(PCI_CAP_ID_EA_OFFSET, cap);
/* test swap case in BAR 1 */ - bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_0, 0, 0, 0); + bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, 0); ut_assertnonnull(bar); *(int *)bar = 2; /* swap upper/lower */
- bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_1, 0, 0, 0); + bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_1, 0, 0, PCI_REGION_TYPE, 0); ut_assertnonnull(bar); strcpy(bar, "ea TEST"); unmap_sysmem(bar); - bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_1, 0, 0, 0); + bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_1, 0, 0, PCI_REGION_TYPE, 0); ut_assertnonnull(bar); ut_asserteq_str("EA test", bar);
/* test magic values in BARs2, 4; BAR 3 is n/a */ - bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_2, 0, 0, 0); + bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_2, 0, 0, PCI_REGION_TYPE, 0); ut_assertnonnull(bar); ut_asserteq(PCI_EA_BAR2_MAGIC, *(u32 *)bar);
- bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_3, 0, 0, 0); + bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_3, 0, 0, PCI_REGION_TYPE, 0); ut_assertnull(bar);
- bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_4, 0, 0, 0); + bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_4, 0, 0, PCI_REGION_TYPE, 0); ut_assertnonnull(bar); ut_asserteq(PCI_EA_BAR4_MAGIC, *(u32 *)bar);

On Wed, Mar 30, 2022 at 12:59 AM Andrew Scull ascull@google.com wrote:
Add a mask parameter to control the lookup of the PCI region from which the mapping can be made.
Signed-off-by: Andrew Scull ascull@google.com
arch/x86/cpu/baytrail/cpu.c | 2 +- drivers/ata/ahci.c | 7 ++++--- drivers/gpio/octeon_gpio.c | 2 +- drivers/i2c/designware_i2c_pci.c | 3 ++- drivers/i2c/intel_i2c.c | 2 +- drivers/i2c/octeon_i2c.c | 2 +- drivers/mmc/octeontx_hsmmc.c | 2 +- drivers/mmc/pci_mmc.c | 2 +- drivers/mtd/nand/raw/octeontx_bch.c | 6 ++++-- drivers/mtd/nand/raw/octeontx_nand.c | 2 +- drivers/net/bnxt/bnxt.c | 9 ++++++--- drivers/net/fsl_enetc.c | 2 +- drivers/net/fsl_enetc_mdio.c | 2 +- drivers/net/mscc_eswitch/felix_switch.c | 4 ++-- drivers/net/octeontx/bgx.c | 2 +- drivers/net/octeontx/nic_main.c | 2 +- drivers/net/octeontx/nicvf_main.c | 2 +- drivers/net/octeontx/smi.c | 2 +- drivers/net/octeontx2/cgx.c | 2 +- drivers/net/octeontx2/rvu_af.c | 2 +- drivers/net/octeontx2/rvu_pf.c | 3 ++- drivers/net/pch_gbe.c | 2 +- drivers/nvme/nvme_pci.c | 2 +- drivers/pci/pci-uclass.c | 6 +++--- drivers/spi/octeon_spi.c | 2 +- drivers/usb/host/ohci-pci.c | 2 +- drivers/virtio/virtio_pci_legacy.c | 3 ++- include/pci.h | 3 ++- test/dm/pci.c | 12 ++++++------ 29 files changed, 52 insertions(+), 42 deletions(-)
Again I believe this patch should be squashed into previous patches for bisectability.
diff --git a/arch/x86/cpu/baytrail/cpu.c b/arch/x86/cpu/baytrail/cpu.c index 6f1b05175c..3eb10b53bb 100644 --- a/arch/x86/cpu/baytrail/cpu.c +++ b/arch/x86/cpu/baytrail/cpu.c @@ -55,7 +55,7 @@ int arch_cpu_init_dm(void) for (i = 0; i < 2; i++) { ret = dm_pci_bus_find_bdf(PCI_BDF(0, 0x1e, 3 + i), &dev); if (!ret) {
base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); hsuart_clock_set(base); }
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index 3925807d55..de6131f1d9 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -417,7 +417,7 @@ static int ahci_init_one(struct ahci_uc_priv *uc_priv, struct udevice *dev)
#if !defined(CONFIG_DM_SCSI) uc_priv->mmio_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_5, 0, 0,
PCI_REGION_MEM);
PCI_REGION_TYPE, PCI_REGION_MEM); /* Take from kernel: * JMicron-specific fixup:
@@ -1149,7 +1149,7 @@ int ahci_probe_scsi_pci(struct udevice *ahci_dev) u16 vendor, device;
base = (ulong)dm_pci_map_bar(ahci_dev, PCI_BASE_ADDRESS_5, 0, 0,
PCI_REGION_MEM);
PCI_REGION_TYPE, PCI_REGION_MEM); /* * Note:
@@ -1163,7 +1163,8 @@ int ahci_probe_scsi_pci(struct udevice *ahci_dev)
if (vendor == PCI_VENDOR_ID_CAVIUM && device == PCI_DEVICE_ID_CAVIUM_SATA)
base = (uintptr_t)dm_pci_map_bar(ahci_dev, PCI_BASE_ADDRESS_0, 0, 0,
base = (uintptr_t)dm_pci_map_bar(ahci_dev, PCI_BASE_ADDRESS_0,
0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); return ahci_probe_scsi(ahci_dev, base);
} diff --git a/drivers/gpio/octeon_gpio.c b/drivers/gpio/octeon_gpio.c index e6a8e1a521..2b2465b1bc 100644 --- a/drivers/gpio/octeon_gpio.c +++ b/drivers/gpio/octeon_gpio.c @@ -183,7 +183,7 @@ static int octeon_gpio_probe(struct udevice *dev) priv->data = (const struct octeon_gpio_data *)dev_get_driver_data(dev);
if (priv->data->probe == PROBE_PCI) {
priv->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
priv->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); uc_priv->gpio_count = readq(priv->base + priv->data->reg_offs + GPIO_CONST) &
diff --git a/drivers/i2c/designware_i2c_pci.c b/drivers/i2c/designware_i2c_pci.c index 51f1357d10..1572c2c6bc 100644 --- a/drivers/i2c/designware_i2c_pci.c +++ b/drivers/i2c/designware_i2c_pci.c @@ -59,7 +59,8 @@ static int designware_i2c_pci_of_to_plat(struct udevice *dev) priv->regs = (struct i2c_regs *)dm_pci_read_bar32(dev, 0); } else { priv->regs = (struct i2c_regs *)
dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM);
dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
PCI_REGION_TYPE, PCI_REGION_MEM); } if (!priv->regs) return -EINVAL;
diff --git a/drivers/i2c/intel_i2c.c b/drivers/i2c/intel_i2c.c index 7b5b62e3eb..dc26fa8c54 100644 --- a/drivers/i2c/intel_i2c.c +++ b/drivers/i2c/intel_i2c.c @@ -251,7 +251,7 @@ static int intel_i2c_probe(struct udevice *dev) ulong base;
/* Save base address from PCI BAR */
priv->base = (ulong)dm_pci_map_bar(dev, PCI_BASE_ADDRESS_4, 0, 0,
priv->base = (ulong)dm_pci_map_bar(dev, PCI_BASE_ADDRESS_4, 0, 0, PCI_REGION_TYPE, PCI_REGION_IO); base = priv->base;
diff --git a/drivers/i2c/octeon_i2c.c b/drivers/i2c/octeon_i2c.c index 74fd5c3d2e..e54ef18e51 100644 --- a/drivers/i2c/octeon_i2c.c +++ b/drivers/i2c/octeon_i2c.c @@ -792,7 +792,7 @@ static int octeon_i2c_probe(struct udevice *dev)
debug("TWSI PCI device: %x\n", bdf);
twsi->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
twsi->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); } else { twsi->base = dev_remap_addr(dev);
diff --git a/drivers/mmc/octeontx_hsmmc.c b/drivers/mmc/octeontx_hsmmc.c index 0bf38945a1..6e9acf7310 100644 --- a/drivers/mmc/octeontx_hsmmc.c +++ b/drivers/mmc/octeontx_hsmmc.c @@ -3822,7 +3822,7 @@ static int octeontx_mmc_host_probe(struct udevice *dev)
/* Octeon TX & TX2 use PCI based probing */ if (device_is_compatible(dev, "cavium,thunder-8890-mmc")) {
host->base_addr = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
host->base_addr = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); if (!host->base_addr) { pr_err("%s: Error: MMC base address not found\n",
diff --git a/drivers/mmc/pci_mmc.c b/drivers/mmc/pci_mmc.c index 1bc2fbcfdf..cba2ea8cf3 100644 --- a/drivers/mmc/pci_mmc.c +++ b/drivers/mmc/pci_mmc.c @@ -50,7 +50,7 @@ static int pci_mmc_probe(struct udevice *dev) desc = mmc_get_blk_desc(&plat->mmc); desc->removable = !(plat->cfg.host_caps & MMC_CAP_NONREMOVABLE);
host->ioaddr = (void *)dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
host->ioaddr = (void *)dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); host->name = dev->name; host->cd_gpio = priv->cd_gpio;
diff --git a/drivers/mtd/nand/raw/octeontx_bch.c b/drivers/mtd/nand/raw/octeontx_bch.c index c1cc5fa187..c1d721cabf 100644 --- a/drivers/mtd/nand/raw/octeontx_bch.c +++ b/drivers/mtd/nand/raw/octeontx_bch.c @@ -176,7 +176,8 @@ static int octeontx_pci_bchpf_probe(struct udevice *dev) if (!bch) return -ENOMEM;
bch->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM);
bch->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
PCI_REGION_TYPE, PCI_REGION_MEM); bch->dev = dev; debug("%s: base address: %p\n", __func__, bch->reg_base);
@@ -361,7 +362,8 @@ static int octeontx_pci_bchvf_probe(struct udevice *dev) vf->dev = dev;
/* Map PF's configuration registers */
vf->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM);
vf->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
PCI_REGION_TYPE, PCI_REGION_MEM); debug("%s: reg base: %p\n", __func__, vf->reg_base); err = octeontx_cmd_queue_initialize(dev, QID_BCH, QDEPTH - 1, 0,
diff --git a/drivers/mtd/nand/raw/octeontx_nand.c b/drivers/mtd/nand/raw/octeontx_nand.c index 3e84bb2fc0..b338b204f3 100644 --- a/drivers/mtd/nand/raw/octeontx_nand.c +++ b/drivers/mtd/nand/raw/octeontx_nand.c @@ -2098,7 +2098,7 @@ static int octeontx_pci_nand_probe(struct udevice *dev) tn->dev = dev; INIT_LIST_HEAD(&tn->chips);
tn->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM);
tn->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); if (!tn->base) { ret = -EINVAL; goto release;
diff --git a/drivers/net/bnxt/bnxt.c b/drivers/net/bnxt/bnxt.c index a24f965ec1..1c9a996240 100644 --- a/drivers/net/bnxt/bnxt.c +++ b/drivers/net/bnxt/bnxt.c @@ -28,9 +28,12 @@ static void bnxt_bring_pci(struct bnxt *bp) dm_pci_read_config16(bp->pdev, PCI_SUBSYSTEM_ID, &bp->subsystem_device); dm_pci_read_config16(bp->pdev, PCI_COMMAND, &bp->cmd_reg); dm_pci_read_config8(bp->pdev, PCI_INTERRUPT_LINE, &bp->irq);
bp->bar0 = dm_pci_map_bar(bp->pdev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM);
bp->bar1 = dm_pci_map_bar(bp->pdev, PCI_BASE_ADDRESS_2, 0, 0, PCI_REGION_MEM);
bp->bar2 = dm_pci_map_bar(bp->pdev, PCI_BASE_ADDRESS_4, 0, 0, PCI_REGION_MEM);
bp->bar0 = dm_pci_map_bar(bp->pdev, PCI_BASE_ADDRESS_0, 0, 0,
PCI_REGION_TYPE, PCI_REGION_MEM);
bp->bar1 = dm_pci_map_bar(bp->pdev, PCI_BASE_ADDRESS_2, 0, 0,
PCI_REGION_TYPE, PCI_REGION_MEM);
bp->bar2 = dm_pci_map_bar(bp->pdev, PCI_BASE_ADDRESS_4, 0, 0,
PCI_REGION_TYPE, PCI_REGION_MEM); cmd_reg = bp->cmd_reg | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER; cmd_reg |= PCI_COMMAND_INTX_DISABLE; /* disable intr */ dm_pci_write_config16(bp->pdev, PCI_COMMAND, cmd_reg);
diff --git a/drivers/net/fsl_enetc.c b/drivers/net/fsl_enetc.c index 60a7b27c81..3aa835329e 100644 --- a/drivers/net/fsl_enetc.c +++ b/drivers/net/fsl_enetc.c @@ -344,7 +344,7 @@ static int enetc_probe(struct udevice *dev) }
/* initialize register */
priv->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, 0);
priv->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, 0); if (!priv->regs_base) { enetc_dbg(dev, "failed to map BAR0\n"); return -EINVAL;
diff --git a/drivers/net/fsl_enetc_mdio.c b/drivers/net/fsl_enetc_mdio.c index f025c2255c..50ad76dfeb 100644 --- a/drivers/net/fsl_enetc_mdio.c +++ b/drivers/net/fsl_enetc_mdio.c @@ -125,7 +125,7 @@ static int enetc_mdio_probe(struct udevice *dev) { struct enetc_mdio_priv *priv = dev_get_priv(dev);
priv->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, 0);
priv->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, 0); if (!priv->regs_base) { enetc_dbg(dev, "failed to map BAR0\n"); return -EINVAL;
diff --git a/drivers/net/mscc_eswitch/felix_switch.c b/drivers/net/mscc_eswitch/felix_switch.c index 0badb23828..709c9e3ef5 100644 --- a/drivers/net/mscc_eswitch/felix_switch.c +++ b/drivers/net/mscc_eswitch/felix_switch.c @@ -292,13 +292,13 @@ static int felix_probe(struct udevice *dev) return -ENODEV; }
priv->imdio_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, 0);
priv->imdio_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, 0); if (!priv->imdio_base) { dev_err(dev, "failed to map BAR0\n"); return -EINVAL; }
priv->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_4, 0, 0, 0);
priv->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_4, 0, 0, PCI_REGION_TYPE, 0); if (!priv->regs_base) { dev_err(dev, "failed to map BAR4\n"); return -EINVAL;
diff --git a/drivers/net/octeontx/bgx.c b/drivers/net/octeontx/bgx.c index cc8ef099c2..b6592ff2ce 100644 --- a/drivers/net/octeontx/bgx.c +++ b/drivers/net/octeontx/bgx.c @@ -1458,7 +1458,7 @@ int octeontx_bgx_probe(struct udevice *dev) int bgx_idx, node; int inc = 1;
bgx->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
bgx->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); if (!bgx->reg_base) { debug("No PCI region found\n");
diff --git a/drivers/net/octeontx/nic_main.c b/drivers/net/octeontx/nic_main.c index 4754c042f1..99886e3afc 100644 --- a/drivers/net/octeontx/nic_main.c +++ b/drivers/net/octeontx/nic_main.c @@ -713,7 +713,7 @@ int nic_initialize(struct udevice *dev) return -ENOMEM;
/* MAP PF's configuration registers */
nic->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
nic->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); if (!nic->reg_base) { printf("Cannot map config register space, aborting\n");
diff --git a/drivers/net/octeontx/nicvf_main.c b/drivers/net/octeontx/nicvf_main.c index 097df6df1e..6e4d0a0512 100644 --- a/drivers/net/octeontx/nicvf_main.c +++ b/drivers/net/octeontx/nicvf_main.c @@ -509,7 +509,7 @@ int nicvf_initialize(struct udevice *dev) /* Enable TSO support */ nicvf->hw_tso = true;
nicvf->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
nicvf->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); debug("nicvf->reg_base: %p\n", nicvf->reg_base);
diff --git a/drivers/net/octeontx/smi.c b/drivers/net/octeontx/smi.c index 2d521bd3ca..233c26f731 100644 --- a/drivers/net/octeontx/smi.c +++ b/drivers/net/octeontx/smi.c @@ -322,7 +322,7 @@ int octeontx_smi_probe(struct udevice *dev) u64 baseaddr;
debug("SMI PCI device: %x\n", bdf);
if (!dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM)) {
if (!dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM)) { printf("Failed to map PCI region for bdf %x\n", bdf); return -1; }
diff --git a/drivers/net/octeontx2/cgx.c b/drivers/net/octeontx2/cgx.c index eed31a9579..c6ec3200c0 100644 --- a/drivers/net/octeontx2/cgx.c +++ b/drivers/net/octeontx2/cgx.c @@ -253,7 +253,7 @@ int cgx_probe(struct udevice *dev) struct cgx *cgx = dev_get_priv(dev); int err;
cgx->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
cgx->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); cgx->dev = dev; cgx->cgx_id = ((u64)(cgx->reg_base) >> 24) & 0x7;
diff --git a/drivers/net/octeontx2/rvu_af.c b/drivers/net/octeontx2/rvu_af.c index 47c1502ef8..0d3a9ffe9e 100644 --- a/drivers/net/octeontx2/rvu_af.c +++ b/drivers/net/octeontx2/rvu_af.c @@ -127,7 +127,7 @@ int rvu_af_probe(struct udevice *dev) { struct rvu_af *af_ptr = dev_get_priv(dev);
af_ptr->af_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
af_ptr->af_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); debug("%s RVU AF BAR %p\n", __func__, af_ptr->af_base); af_ptr->dev = dev;
diff --git a/drivers/net/octeontx2/rvu_pf.c b/drivers/net/octeontx2/rvu_pf.c index 024e17e748..5f3ea1f8ea 100644 --- a/drivers/net/octeontx2/rvu_pf.c +++ b/drivers/net/octeontx2/rvu_pf.c @@ -58,7 +58,8 @@ int rvu_pf_probe(struct udevice *dev)
debug("%s: name: %s\n", __func__, dev->name);
rvu->pf_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_2, 0, 0, PCI_REGION_MEM);
rvu->pf_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_2, 0, 0,
PCI_REGION_TYPE, PCI_REGION_MEM); rvu->pfid = dev_seq(dev) + 1; // RVU PF's start from 1; rvu->dev = dev; if (!rvu_af_dev) {
diff --git a/drivers/net/pch_gbe.c b/drivers/net/pch_gbe.c index c93cc2ead2..6b251ab63f 100644 --- a/drivers/net/pch_gbe.c +++ b/drivers/net/pch_gbe.c @@ -449,7 +449,7 @@ static int pch_gbe_probe(struct udevice *dev)
priv->dev = dev;
iobase = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_1, 0, 0, PCI_REGION_MEM);
iobase = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_1, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); plat->iobase = (ulong)iobase; priv->mac_regs = (struct pch_gbe_regs *)iobase;
diff --git a/drivers/nvme/nvme_pci.c b/drivers/nvme/nvme_pci.c index 3499a7b6e7..36bf9c5ffb 100644 --- a/drivers/nvme/nvme_pci.c +++ b/drivers/nvme/nvme_pci.c @@ -29,7 +29,7 @@ static int nvme_probe(struct udevice *udev)
ndev->instance = trailing_strtol(udev->name); ndev->bar = dm_pci_map_bar(udev, PCI_BASE_ADDRESS_0, 0, 0,
PCI_REGION_MEM);
PCI_REGION_TYPE, PCI_REGION_MEM); return nvme_init(udev);
}
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 676052090a..b92ef23a7a 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -1560,7 +1560,7 @@ static void *dm_pci_map_ea_bar(struct udevice *dev, int bar, size_t offset, }
void *dm_pci_map_bar(struct udevice *dev, int bar, size_t offset, size_t len,
unsigned long flags)
unsigned long mask, unsigned long flags)
{ struct pci_child_plat *pdata = dev_get_parent_plat(dev); struct udevice *udev = dev; @@ -1602,8 +1602,8 @@ void *dm_pci_map_bar(struct udevice *dev, int bar, size_t offset, size_t len, * a PCI range, but a better check would be to probe for the size of * the bar and prevent overflow more locally. */
return dm_pci_bus_to_virt(udev, pci_bus_addr + offset, len,
PCI_REGION_TYPE, flags, MAP_NOCACHE);
return dm_pci_bus_to_virt(udev, pci_bus_addr + offset, len, mask, flags,
MAP_NOCACHE);
}
static int _dm_pci_find_next_capability(struct udevice *dev, u8 pos, int cap) diff --git a/drivers/spi/octeon_spi.c b/drivers/spi/octeon_spi.c index 2f8a8a8649..c2a7ee232b 100644 --- a/drivers/spi/octeon_spi.c +++ b/drivers/spi/octeon_spi.c @@ -568,7 +568,7 @@ static int octeon_spi_probe(struct udevice *dev) pci_dev_t bdf = dm_pci_get_bdf(dev);
debug("SPI PCI device: %x\n", bdf);
priv->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
priv->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); /* Add base offset */ priv->base += 0x1000;
diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c index eab0d96637..f061aec289 100644 --- a/drivers/usb/host/ohci-pci.c +++ b/drivers/usb/host/ohci-pci.c @@ -18,7 +18,7 @@ static int ohci_pci_probe(struct udevice *dev) { struct ohci_regs *regs;
regs = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM);
regs = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); return ohci_register(dev, regs);
}
diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c index 504a7ff7b9..cf5dfb17a9 100644 --- a/drivers/virtio/virtio_pci_legacy.c +++ b/drivers/virtio/virtio_pci_legacy.c @@ -319,7 +319,8 @@ static int virtio_pci_probe(struct udevice *udev) uc_priv->device = subdevice; uc_priv->vendor = subvendor;
priv->ioaddr = dm_pci_map_bar(udev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_IO);
priv->ioaddr = dm_pci_map_bar(udev, PCI_BASE_ADDRESS_0, 0, 0,
PCI_REGION_TYPE, PCI_REGION_IO); if (!priv->ioaddr) return -ENXIO; debug("(%s): virtio legacy device reg base %04lx\n",
diff --git a/include/pci.h b/include/pci.h index 683efcd1be..99122ed54f 100644 --- a/include/pci.h +++ b/include/pci.h @@ -1477,11 +1477,12 @@ pci_addr_t dm_pci_phys_to_bus(struct udevice *dev, phys_addr_t addr, size_t len,
- @bar: Bar register offset (PCI_BASE_ADDRESS_...)
- @offset: Offset from the base to map
- @len: Length to map
*/
- @mask: Mask to match flags for the region type
- @flags: Flags for the region type (PCI_REGION_...)
- @return: pointer to the virtual address to use or 0 on error
void *dm_pci_map_bar(struct udevice *dev, int bar, size_t offset, size_t len,
unsigned long flags);
unsigned long mask, unsigned long flags);
/**
- dm_pci_find_next_capability() - find a capability starting from an offset
diff --git a/test/dm/pci.c b/test/dm/pci.c index edc407f9d3..4bbda4c407 100644 --- a/test/dm/pci.c +++ b/test/dm/pci.c @@ -268,27 +268,27 @@ static int dm_test_pci_ea(struct unit_test_state *uts) ut_asserteq(PCI_CAP_ID_EA_OFFSET, cap);
/* test swap case in BAR 1 */
bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_0, 0, 0, 0);
bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, 0); ut_assertnonnull(bar); *(int *)bar = 2; /* swap upper/lower */
bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_1, 0, 0, 0);
bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_1, 0, 0, PCI_REGION_TYPE, 0); ut_assertnonnull(bar); strcpy(bar, "ea TEST"); unmap_sysmem(bar);
bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_1, 0, 0, 0);
bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_1, 0, 0, PCI_REGION_TYPE, 0); ut_assertnonnull(bar); ut_asserteq_str("EA test", bar); /* test magic values in BARs2, 4; BAR 3 is n/a */
bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_2, 0, 0, 0);
bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_2, 0, 0, PCI_REGION_TYPE, 0); ut_assertnonnull(bar); ut_asserteq(PCI_EA_BAR2_MAGIC, *(u32 *)bar);
bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_3, 0, 0, 0);
bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_3, 0, 0, PCI_REGION_TYPE, 0); ut_assertnull(bar);
bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_4, 0, 0, 0);
bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_4, 0, 0, PCI_REGION_TYPE, 0); ut_assertnonnull(bar); ut_asserteq(PCI_EA_BAR4_MAGIC, *(u32 *)bar);
--
Regards, Bin

On Wed, 13 Apr 2022 at 16:11, Bin Meng bmeng.cn@gmail.com wrote:
On Wed, Mar 30, 2022 at 12:59 AM Andrew Scull ascull@google.com wrote:
Add a mask parameter to control the lookup of the PCI region from which the mapping can be made.
Signed-off-by: Andrew Scull ascull@google.com
arch/x86/cpu/baytrail/cpu.c | 2 +- drivers/ata/ahci.c | 7 ++++--- drivers/gpio/octeon_gpio.c | 2 +- drivers/i2c/designware_i2c_pci.c | 3 ++- drivers/i2c/intel_i2c.c | 2 +- drivers/i2c/octeon_i2c.c | 2 +- drivers/mmc/octeontx_hsmmc.c | 2 +- drivers/mmc/pci_mmc.c | 2 +- drivers/mtd/nand/raw/octeontx_bch.c | 6 ++++-- drivers/mtd/nand/raw/octeontx_nand.c | 2 +- drivers/net/bnxt/bnxt.c | 9 ++++++--- drivers/net/fsl_enetc.c | 2 +- drivers/net/fsl_enetc_mdio.c | 2 +- drivers/net/mscc_eswitch/felix_switch.c | 4 ++-- drivers/net/octeontx/bgx.c | 2 +- drivers/net/octeontx/nic_main.c | 2 +- drivers/net/octeontx/nicvf_main.c | 2 +- drivers/net/octeontx/smi.c | 2 +- drivers/net/octeontx2/cgx.c | 2 +- drivers/net/octeontx2/rvu_af.c | 2 +- drivers/net/octeontx2/rvu_pf.c | 3 ++- drivers/net/pch_gbe.c | 2 +- drivers/nvme/nvme_pci.c | 2 +- drivers/pci/pci-uclass.c | 6 +++--- drivers/spi/octeon_spi.c | 2 +- drivers/usb/host/ohci-pci.c | 2 +- drivers/virtio/virtio_pci_legacy.c | 3 ++- include/pci.h | 3 ++- test/dm/pci.c | 12 ++++++------ 29 files changed, 52 insertions(+), 42 deletions(-)
Again I believe this patch should be squashed into previous patches for bisectability.
Same reasoning here, dm_pci_map_bar() migrated internally in #14 and is now exposing the mask in its interface in this patch.
diff --git a/arch/x86/cpu/baytrail/cpu.c b/arch/x86/cpu/baytrail/cpu.c index 6f1b05175c..3eb10b53bb 100644 --- a/arch/x86/cpu/baytrail/cpu.c +++ b/arch/x86/cpu/baytrail/cpu.c @@ -55,7 +55,7 @@ int arch_cpu_init_dm(void) for (i = 0; i < 2; i++) { ret = dm_pci_bus_find_bdf(PCI_BDF(0, 0x1e, 3 + i), &dev); if (!ret) {
base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); hsuart_clock_set(base); }
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index 3925807d55..de6131f1d9 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -417,7 +417,7 @@ static int ahci_init_one(struct ahci_uc_priv *uc_priv, struct udevice *dev)
#if !defined(CONFIG_DM_SCSI) uc_priv->mmio_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_5, 0, 0,
PCI_REGION_MEM);
PCI_REGION_TYPE, PCI_REGION_MEM); /* Take from kernel: * JMicron-specific fixup:
@@ -1149,7 +1149,7 @@ int ahci_probe_scsi_pci(struct udevice *ahci_dev) u16 vendor, device;
base = (ulong)dm_pci_map_bar(ahci_dev, PCI_BASE_ADDRESS_5, 0, 0,
PCI_REGION_MEM);
PCI_REGION_TYPE, PCI_REGION_MEM); /* * Note:
@@ -1163,7 +1163,8 @@ int ahci_probe_scsi_pci(struct udevice *ahci_dev)
if (vendor == PCI_VENDOR_ID_CAVIUM && device == PCI_DEVICE_ID_CAVIUM_SATA)
base = (uintptr_t)dm_pci_map_bar(ahci_dev, PCI_BASE_ADDRESS_0, 0, 0,
base = (uintptr_t)dm_pci_map_bar(ahci_dev, PCI_BASE_ADDRESS_0,
0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); return ahci_probe_scsi(ahci_dev, base);
} diff --git a/drivers/gpio/octeon_gpio.c b/drivers/gpio/octeon_gpio.c index e6a8e1a521..2b2465b1bc 100644 --- a/drivers/gpio/octeon_gpio.c +++ b/drivers/gpio/octeon_gpio.c @@ -183,7 +183,7 @@ static int octeon_gpio_probe(struct udevice *dev) priv->data = (const struct octeon_gpio_data *)dev_get_driver_data(dev);
if (priv->data->probe == PROBE_PCI) {
priv->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
priv->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); uc_priv->gpio_count = readq(priv->base + priv->data->reg_offs + GPIO_CONST) &
diff --git a/drivers/i2c/designware_i2c_pci.c b/drivers/i2c/designware_i2c_pci.c index 51f1357d10..1572c2c6bc 100644 --- a/drivers/i2c/designware_i2c_pci.c +++ b/drivers/i2c/designware_i2c_pci.c @@ -59,7 +59,8 @@ static int designware_i2c_pci_of_to_plat(struct udevice *dev) priv->regs = (struct i2c_regs *)dm_pci_read_bar32(dev, 0); } else { priv->regs = (struct i2c_regs *)
dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM);
dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
PCI_REGION_TYPE, PCI_REGION_MEM); } if (!priv->regs) return -EINVAL;
diff --git a/drivers/i2c/intel_i2c.c b/drivers/i2c/intel_i2c.c index 7b5b62e3eb..dc26fa8c54 100644 --- a/drivers/i2c/intel_i2c.c +++ b/drivers/i2c/intel_i2c.c @@ -251,7 +251,7 @@ static int intel_i2c_probe(struct udevice *dev) ulong base;
/* Save base address from PCI BAR */
priv->base = (ulong)dm_pci_map_bar(dev, PCI_BASE_ADDRESS_4, 0, 0,
priv->base = (ulong)dm_pci_map_bar(dev, PCI_BASE_ADDRESS_4, 0, 0, PCI_REGION_TYPE, PCI_REGION_IO); base = priv->base;
diff --git a/drivers/i2c/octeon_i2c.c b/drivers/i2c/octeon_i2c.c index 74fd5c3d2e..e54ef18e51 100644 --- a/drivers/i2c/octeon_i2c.c +++ b/drivers/i2c/octeon_i2c.c @@ -792,7 +792,7 @@ static int octeon_i2c_probe(struct udevice *dev)
debug("TWSI PCI device: %x\n", bdf);
twsi->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
twsi->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); } else { twsi->base = dev_remap_addr(dev);
diff --git a/drivers/mmc/octeontx_hsmmc.c b/drivers/mmc/octeontx_hsmmc.c index 0bf38945a1..6e9acf7310 100644 --- a/drivers/mmc/octeontx_hsmmc.c +++ b/drivers/mmc/octeontx_hsmmc.c @@ -3822,7 +3822,7 @@ static int octeontx_mmc_host_probe(struct udevice *dev)
/* Octeon TX & TX2 use PCI based probing */ if (device_is_compatible(dev, "cavium,thunder-8890-mmc")) {
host->base_addr = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
host->base_addr = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); if (!host->base_addr) { pr_err("%s: Error: MMC base address not found\n",
diff --git a/drivers/mmc/pci_mmc.c b/drivers/mmc/pci_mmc.c index 1bc2fbcfdf..cba2ea8cf3 100644 --- a/drivers/mmc/pci_mmc.c +++ b/drivers/mmc/pci_mmc.c @@ -50,7 +50,7 @@ static int pci_mmc_probe(struct udevice *dev) desc = mmc_get_blk_desc(&plat->mmc); desc->removable = !(plat->cfg.host_caps & MMC_CAP_NONREMOVABLE);
host->ioaddr = (void *)dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
host->ioaddr = (void *)dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); host->name = dev->name; host->cd_gpio = priv->cd_gpio;
diff --git a/drivers/mtd/nand/raw/octeontx_bch.c b/drivers/mtd/nand/raw/octeontx_bch.c index c1cc5fa187..c1d721cabf 100644 --- a/drivers/mtd/nand/raw/octeontx_bch.c +++ b/drivers/mtd/nand/raw/octeontx_bch.c @@ -176,7 +176,8 @@ static int octeontx_pci_bchpf_probe(struct udevice *dev) if (!bch) return -ENOMEM;
bch->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM);
bch->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
PCI_REGION_TYPE, PCI_REGION_MEM); bch->dev = dev; debug("%s: base address: %p\n", __func__, bch->reg_base);
@@ -361,7 +362,8 @@ static int octeontx_pci_bchvf_probe(struct udevice *dev) vf->dev = dev;
/* Map PF's configuration registers */
vf->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM);
vf->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
PCI_REGION_TYPE, PCI_REGION_MEM); debug("%s: reg base: %p\n", __func__, vf->reg_base); err = octeontx_cmd_queue_initialize(dev, QID_BCH, QDEPTH - 1, 0,
diff --git a/drivers/mtd/nand/raw/octeontx_nand.c b/drivers/mtd/nand/raw/octeontx_nand.c index 3e84bb2fc0..b338b204f3 100644 --- a/drivers/mtd/nand/raw/octeontx_nand.c +++ b/drivers/mtd/nand/raw/octeontx_nand.c @@ -2098,7 +2098,7 @@ static int octeontx_pci_nand_probe(struct udevice *dev) tn->dev = dev; INIT_LIST_HEAD(&tn->chips);
tn->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM);
tn->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); if (!tn->base) { ret = -EINVAL; goto release;
diff --git a/drivers/net/bnxt/bnxt.c b/drivers/net/bnxt/bnxt.c index a24f965ec1..1c9a996240 100644 --- a/drivers/net/bnxt/bnxt.c +++ b/drivers/net/bnxt/bnxt.c @@ -28,9 +28,12 @@ static void bnxt_bring_pci(struct bnxt *bp) dm_pci_read_config16(bp->pdev, PCI_SUBSYSTEM_ID, &bp->subsystem_device); dm_pci_read_config16(bp->pdev, PCI_COMMAND, &bp->cmd_reg); dm_pci_read_config8(bp->pdev, PCI_INTERRUPT_LINE, &bp->irq);
bp->bar0 = dm_pci_map_bar(bp->pdev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM);
bp->bar1 = dm_pci_map_bar(bp->pdev, PCI_BASE_ADDRESS_2, 0, 0, PCI_REGION_MEM);
bp->bar2 = dm_pci_map_bar(bp->pdev, PCI_BASE_ADDRESS_4, 0, 0, PCI_REGION_MEM);
bp->bar0 = dm_pci_map_bar(bp->pdev, PCI_BASE_ADDRESS_0, 0, 0,
PCI_REGION_TYPE, PCI_REGION_MEM);
bp->bar1 = dm_pci_map_bar(bp->pdev, PCI_BASE_ADDRESS_2, 0, 0,
PCI_REGION_TYPE, PCI_REGION_MEM);
bp->bar2 = dm_pci_map_bar(bp->pdev, PCI_BASE_ADDRESS_4, 0, 0,
PCI_REGION_TYPE, PCI_REGION_MEM); cmd_reg = bp->cmd_reg | PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER; cmd_reg |= PCI_COMMAND_INTX_DISABLE; /* disable intr */ dm_pci_write_config16(bp->pdev, PCI_COMMAND, cmd_reg);
diff --git a/drivers/net/fsl_enetc.c b/drivers/net/fsl_enetc.c index 60a7b27c81..3aa835329e 100644 --- a/drivers/net/fsl_enetc.c +++ b/drivers/net/fsl_enetc.c @@ -344,7 +344,7 @@ static int enetc_probe(struct udevice *dev) }
/* initialize register */
priv->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, 0);
priv->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, 0); if (!priv->regs_base) { enetc_dbg(dev, "failed to map BAR0\n"); return -EINVAL;
diff --git a/drivers/net/fsl_enetc_mdio.c b/drivers/net/fsl_enetc_mdio.c index f025c2255c..50ad76dfeb 100644 --- a/drivers/net/fsl_enetc_mdio.c +++ b/drivers/net/fsl_enetc_mdio.c @@ -125,7 +125,7 @@ static int enetc_mdio_probe(struct udevice *dev) { struct enetc_mdio_priv *priv = dev_get_priv(dev);
priv->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, 0);
priv->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, 0); if (!priv->regs_base) { enetc_dbg(dev, "failed to map BAR0\n"); return -EINVAL;
diff --git a/drivers/net/mscc_eswitch/felix_switch.c b/drivers/net/mscc_eswitch/felix_switch.c index 0badb23828..709c9e3ef5 100644 --- a/drivers/net/mscc_eswitch/felix_switch.c +++ b/drivers/net/mscc_eswitch/felix_switch.c @@ -292,13 +292,13 @@ static int felix_probe(struct udevice *dev) return -ENODEV; }
priv->imdio_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, 0);
priv->imdio_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, 0); if (!priv->imdio_base) { dev_err(dev, "failed to map BAR0\n"); return -EINVAL; }
priv->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_4, 0, 0, 0);
priv->regs_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_4, 0, 0, PCI_REGION_TYPE, 0); if (!priv->regs_base) { dev_err(dev, "failed to map BAR4\n"); return -EINVAL;
diff --git a/drivers/net/octeontx/bgx.c b/drivers/net/octeontx/bgx.c index cc8ef099c2..b6592ff2ce 100644 --- a/drivers/net/octeontx/bgx.c +++ b/drivers/net/octeontx/bgx.c @@ -1458,7 +1458,7 @@ int octeontx_bgx_probe(struct udevice *dev) int bgx_idx, node; int inc = 1;
bgx->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
bgx->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); if (!bgx->reg_base) { debug("No PCI region found\n");
diff --git a/drivers/net/octeontx/nic_main.c b/drivers/net/octeontx/nic_main.c index 4754c042f1..99886e3afc 100644 --- a/drivers/net/octeontx/nic_main.c +++ b/drivers/net/octeontx/nic_main.c @@ -713,7 +713,7 @@ int nic_initialize(struct udevice *dev) return -ENOMEM;
/* MAP PF's configuration registers */
nic->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
nic->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); if (!nic->reg_base) { printf("Cannot map config register space, aborting\n");
diff --git a/drivers/net/octeontx/nicvf_main.c b/drivers/net/octeontx/nicvf_main.c index 097df6df1e..6e4d0a0512 100644 --- a/drivers/net/octeontx/nicvf_main.c +++ b/drivers/net/octeontx/nicvf_main.c @@ -509,7 +509,7 @@ int nicvf_initialize(struct udevice *dev) /* Enable TSO support */ nicvf->hw_tso = true;
nicvf->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
nicvf->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); debug("nicvf->reg_base: %p\n", nicvf->reg_base);
diff --git a/drivers/net/octeontx/smi.c b/drivers/net/octeontx/smi.c index 2d521bd3ca..233c26f731 100644 --- a/drivers/net/octeontx/smi.c +++ b/drivers/net/octeontx/smi.c @@ -322,7 +322,7 @@ int octeontx_smi_probe(struct udevice *dev) u64 baseaddr;
debug("SMI PCI device: %x\n", bdf);
if (!dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM)) {
if (!dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM)) { printf("Failed to map PCI region for bdf %x\n", bdf); return -1; }
diff --git a/drivers/net/octeontx2/cgx.c b/drivers/net/octeontx2/cgx.c index eed31a9579..c6ec3200c0 100644 --- a/drivers/net/octeontx2/cgx.c +++ b/drivers/net/octeontx2/cgx.c @@ -253,7 +253,7 @@ int cgx_probe(struct udevice *dev) struct cgx *cgx = dev_get_priv(dev); int err;
cgx->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
cgx->reg_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); cgx->dev = dev; cgx->cgx_id = ((u64)(cgx->reg_base) >> 24) & 0x7;
diff --git a/drivers/net/octeontx2/rvu_af.c b/drivers/net/octeontx2/rvu_af.c index 47c1502ef8..0d3a9ffe9e 100644 --- a/drivers/net/octeontx2/rvu_af.c +++ b/drivers/net/octeontx2/rvu_af.c @@ -127,7 +127,7 @@ int rvu_af_probe(struct udevice *dev) { struct rvu_af *af_ptr = dev_get_priv(dev);
af_ptr->af_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
af_ptr->af_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); debug("%s RVU AF BAR %p\n", __func__, af_ptr->af_base); af_ptr->dev = dev;
diff --git a/drivers/net/octeontx2/rvu_pf.c b/drivers/net/octeontx2/rvu_pf.c index 024e17e748..5f3ea1f8ea 100644 --- a/drivers/net/octeontx2/rvu_pf.c +++ b/drivers/net/octeontx2/rvu_pf.c @@ -58,7 +58,8 @@ int rvu_pf_probe(struct udevice *dev)
debug("%s: name: %s\n", __func__, dev->name);
rvu->pf_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_2, 0, 0, PCI_REGION_MEM);
rvu->pf_base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_2, 0, 0,
PCI_REGION_TYPE, PCI_REGION_MEM); rvu->pfid = dev_seq(dev) + 1; // RVU PF's start from 1; rvu->dev = dev; if (!rvu_af_dev) {
diff --git a/drivers/net/pch_gbe.c b/drivers/net/pch_gbe.c index c93cc2ead2..6b251ab63f 100644 --- a/drivers/net/pch_gbe.c +++ b/drivers/net/pch_gbe.c @@ -449,7 +449,7 @@ static int pch_gbe_probe(struct udevice *dev)
priv->dev = dev;
iobase = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_1, 0, 0, PCI_REGION_MEM);
iobase = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_1, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); plat->iobase = (ulong)iobase; priv->mac_regs = (struct pch_gbe_regs *)iobase;
diff --git a/drivers/nvme/nvme_pci.c b/drivers/nvme/nvme_pci.c index 3499a7b6e7..36bf9c5ffb 100644 --- a/drivers/nvme/nvme_pci.c +++ b/drivers/nvme/nvme_pci.c @@ -29,7 +29,7 @@ static int nvme_probe(struct udevice *udev)
ndev->instance = trailing_strtol(udev->name); ndev->bar = dm_pci_map_bar(udev, PCI_BASE_ADDRESS_0, 0, 0,
PCI_REGION_MEM);
PCI_REGION_TYPE, PCI_REGION_MEM); return nvme_init(udev);
}
diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c index 676052090a..b92ef23a7a 100644 --- a/drivers/pci/pci-uclass.c +++ b/drivers/pci/pci-uclass.c @@ -1560,7 +1560,7 @@ static void *dm_pci_map_ea_bar(struct udevice *dev, int bar, size_t offset, }
void *dm_pci_map_bar(struct udevice *dev, int bar, size_t offset, size_t len,
unsigned long flags)
unsigned long mask, unsigned long flags)
{ struct pci_child_plat *pdata = dev_get_parent_plat(dev); struct udevice *udev = dev; @@ -1602,8 +1602,8 @@ void *dm_pci_map_bar(struct udevice *dev, int bar, size_t offset, size_t len, * a PCI range, but a better check would be to probe for the size of * the bar and prevent overflow more locally. */
return dm_pci_bus_to_virt(udev, pci_bus_addr + offset, len,
PCI_REGION_TYPE, flags, MAP_NOCACHE);
return dm_pci_bus_to_virt(udev, pci_bus_addr + offset, len, mask, flags,
MAP_NOCACHE);
}
static int _dm_pci_find_next_capability(struct udevice *dev, u8 pos, int cap) diff --git a/drivers/spi/octeon_spi.c b/drivers/spi/octeon_spi.c index 2f8a8a8649..c2a7ee232b 100644 --- a/drivers/spi/octeon_spi.c +++ b/drivers/spi/octeon_spi.c @@ -568,7 +568,7 @@ static int octeon_spi_probe(struct udevice *dev) pci_dev_t bdf = dm_pci_get_bdf(dev);
debug("SPI PCI device: %x\n", bdf);
priv->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0,
priv->base = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); /* Add base offset */ priv->base += 0x1000;
diff --git a/drivers/usb/host/ohci-pci.c b/drivers/usb/host/ohci-pci.c index eab0d96637..f061aec289 100644 --- a/drivers/usb/host/ohci-pci.c +++ b/drivers/usb/host/ohci-pci.c @@ -18,7 +18,7 @@ static int ohci_pci_probe(struct udevice *dev) { struct ohci_regs *regs;
regs = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_MEM);
regs = dm_pci_map_bar(dev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, PCI_REGION_MEM); return ohci_register(dev, regs);
}
diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c index 504a7ff7b9..cf5dfb17a9 100644 --- a/drivers/virtio/virtio_pci_legacy.c +++ b/drivers/virtio/virtio_pci_legacy.c @@ -319,7 +319,8 @@ static int virtio_pci_probe(struct udevice *udev) uc_priv->device = subdevice; uc_priv->vendor = subvendor;
priv->ioaddr = dm_pci_map_bar(udev, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_IO);
priv->ioaddr = dm_pci_map_bar(udev, PCI_BASE_ADDRESS_0, 0, 0,
PCI_REGION_TYPE, PCI_REGION_IO); if (!priv->ioaddr) return -ENXIO; debug("(%s): virtio legacy device reg base %04lx\n",
diff --git a/include/pci.h b/include/pci.h index 683efcd1be..99122ed54f 100644 --- a/include/pci.h +++ b/include/pci.h @@ -1477,11 +1477,12 @@ pci_addr_t dm_pci_phys_to_bus(struct udevice *dev, phys_addr_t addr, size_t len,
- @bar: Bar register offset (PCI_BASE_ADDRESS_...)
- @offset: Offset from the base to map
- @len: Length to map
*/
- @mask: Mask to match flags for the region type
- @flags: Flags for the region type (PCI_REGION_...)
- @return: pointer to the virtual address to use or 0 on error
void *dm_pci_map_bar(struct udevice *dev, int bar, size_t offset, size_t len,
unsigned long flags);
unsigned long mask, unsigned long flags);
/**
- dm_pci_find_next_capability() - find a capability starting from an offset
diff --git a/test/dm/pci.c b/test/dm/pci.c index edc407f9d3..4bbda4c407 100644 --- a/test/dm/pci.c +++ b/test/dm/pci.c @@ -268,27 +268,27 @@ static int dm_test_pci_ea(struct unit_test_state *uts) ut_asserteq(PCI_CAP_ID_EA_OFFSET, cap);
/* test swap case in BAR 1 */
bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_0, 0, 0, 0);
bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_0, 0, 0, PCI_REGION_TYPE, 0); ut_assertnonnull(bar); *(int *)bar = 2; /* swap upper/lower */
bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_1, 0, 0, 0);
bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_1, 0, 0, PCI_REGION_TYPE, 0); ut_assertnonnull(bar); strcpy(bar, "ea TEST"); unmap_sysmem(bar);
bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_1, 0, 0, 0);
bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_1, 0, 0, PCI_REGION_TYPE, 0); ut_assertnonnull(bar); ut_asserteq_str("EA test", bar); /* test magic values in BARs2, 4; BAR 3 is n/a */
bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_2, 0, 0, 0);
bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_2, 0, 0, PCI_REGION_TYPE, 0); ut_assertnonnull(bar); ut_asserteq(PCI_EA_BAR2_MAGIC, *(u32 *)bar);
bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_3, 0, 0, 0);
bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_3, 0, 0, PCI_REGION_TYPE, 0); ut_assertnull(bar);
bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_4, 0, 0, 0);
bar = dm_pci_map_bar(swap, PCI_BASE_ADDRESS_4, 0, 0, PCI_REGION_TYPE, 0); ut_assertnonnull(bar); ut_asserteq(PCI_EA_BAR4_MAGIC, *(u32 *)bar);
--
Regards, Bin

Prepare for calls to `virtio_pci_map_capability()` failing by returning NULL on error. If this happens, later accesses to the pointers would be unsafe so cause the probe to fail if such an error occurs.
Signed-off-by: Andrew Scull ascull@google.com --- drivers/virtio/virtio_pci_modern.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c index bf92d9d3ba..f1e64a3438 100644 --- a/drivers/virtio/virtio_pci_modern.c +++ b/drivers/virtio/virtio_pci_modern.c @@ -533,7 +533,19 @@ static int virtio_pci_probe(struct udevice *udev) return -EINVAL; }
+ /* Map configuration structures */ + priv->common = virtio_pci_map_capability(udev, &common_cap); + if (!priv->common) { + printf("(%s): could not map common config\n", udev->name); + return -EINVAL; + } + priv->notify_len = notify_cap.length; + priv->notify_base = virtio_pci_map_capability(udev, ¬ify_cap); + if (!priv->notify_base) { + printf("(%s): could not map notify config\n", udev->name); + return -EINVAL; + }
/* * Device capability is only mandatory for devices that have @@ -545,11 +557,13 @@ static int virtio_pci_probe(struct udevice *udev) if (device) { priv->device_len = device_cap.length; priv->device = virtio_pci_map_capability(udev, &device_cap); + if (!priv->device) { + printf("(%s): could not map device config\n", + udev->name); + return -EINVAL; + } }
- /* Map configuration structures */ - 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);

On Wed, Mar 30, 2022 at 12:59 AM Andrew Scull ascull@google.com wrote:
Prepare for calls to `virtio_pci_map_capability()` failing by returning NULL on error. If this happens, later accesses to the pointers would be unsafe so cause the probe to fail if such an error occurs.
Signed-off-by: Andrew Scull ascull@google.com
drivers/virtio/virtio_pci_modern.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

The virtio PCI capabilities describe regions of memory that should be mapped. Map those with dm_pci_map_bar() which will ensure they are valid PCI regions.
Signed-off-by: Andrew Scull ascull@google.com --- drivers/virtio/virtio_pci_modern.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c index f1e64a3438..880a12cc28 100644 --- a/drivers/virtio/virtio_pci_modern.c +++ b/drivers/virtio/virtio_pci_modern.c @@ -459,19 +459,17 @@ static int virtio_pci_find_capability(struct udevice *udev, u8 cfg_type, static void __iomem *virtio_pci_map_capability(struct udevice *udev, const struct virtio_pci_cap *cap) { - ulong base; - void __iomem *p; - /* - * 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. + * Find the corresponding memory region that isn't system memory but is + * writable. */ - base = dm_pci_read_bar32(udev, cap->bar); - p = (void __iomem *)base + cap->offset; + unsigned long mask = + PCI_REGION_TYPE | PCI_REGION_SYS_MEMORY | PCI_REGION_RO; + unsigned long flags = PCI_REGION_MEM; + u8 *p = dm_pci_map_bar(udev, PCI_BASE_ADDRESS_0 + cap->bar, cap->offset, + cap->length, mask, flags);
- return p; + return (void __iomem *)p; }
static int virtio_pci_bind(struct udevice *udev)

On Wed, Mar 30, 2022 at 12:59 AM Andrew Scull ascull@google.com wrote:
The virtio PCI capabilities describe regions of memory that should be mapped. Map those with dm_pci_map_bar() which will ensure they are valid PCI regions.
Signed-off-by: Andrew Scull ascull@google.com
drivers/virtio/virtio_pci_modern.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

Hi,
Today I discovered a failure which bisected to this commit.
When running QEMU with libvirt, which created some pcie devices, U-boot fails on autoboot with:
Unhandled exception: Load access fault EPC: 00000000bff7c46e RA: 00000000bff7c46c TVAL: 0000000040200014 EPC: 000000008002446e RA: 000000008002446c reloc adjusted
Code: 8082 1141 e022 e406 842e 40ef dc5f 611c (c783 0147)
I have tried to minimize the command line that makes the difference.
This works:
-device '{"driver":"virtio-blk-pci","drive":"libvirt-2-format","id":"virtio-disk0","bootindex":1}'
This doesn't work:
-device '{"driver":"virtio-blk-pci","bus":"pci.2","addr":"0x0","drive":"libvirt-2-format","id":"virtio-disk0","bootindex":1}'
I have tried this on multiple machines, different U-boot tags, mmode or smode with OpenSBI, all with the same behavior. Latest U-boot master with this commit reverted works fine on both cases.
I am very new to U-boot, please do let me know if more info are to be provided.
Attached the full command line, for reference:
/usr/bin/qemu-system-riscv64 \ -machine virt \ -bios u-boot/u-boot.bin \ -m 1024 \ -device '{"driver":"pcie-root-port","port":8,"chassis":1,"id":"pci.1","bus":"pcie.0","multifunction":true,"addr":"0x1"}' \ -device '{"driver":"pcie-root-port","port":9,"chassis":2,"id":"pci.2","bus":"pcie.0","addr":"0x1.0x1"}' \ -blockdev '{"driver":"file","filename":"archriscv-2022-08-24.qcow2","node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}' \ -blockdev '{"node-name":"libvirt-2-format","read-only":false,"driver":"qcow2","file":"libvirt-2-storage","backing":null}' \ -device '{"driver":"virtio-blk-pci","bus":"pci.2","addr":"0x0","drive":"libvirt-2-format","id":"virtio-disk0","bootindex":1}' \ -nographic

在 2022-08-25星期四的 11:01 +0300,Felix Yan写道:
Hi,
Today I discovered a failure which bisected to this commit.
When running QEMU with libvirt, which created some pcie devices, U-boot fails on autoboot with:
Unhandled exception: Load access fault EPC: 00000000bff7c46e RA: 00000000bff7c46c TVAL: 0000000040200014 EPC: 000000008002446e RA: 000000008002446c reloc adjusted
Code: 8082 1141 e022 e406 842e 40ef dc5f 611c (c783 0147)
I have tried to minimize the command line that makes the difference.
This works:
-device '{"driver":"virtio-blk-pci","drive":"libvirt-2-format","id":"virtio-disk0","bootindex":1}'
This doesn't work:
-device '{"driver":"virtio-blk-pci","bus":"pci.2","addr":"0x0","drive":"libvirt-2-format","id":"virtio-disk0","bootindex":1}'
I have tried this on multiple machines, different U-boot tags, mmode or smode with OpenSBI, all with the same behavior. Latest U-boot master with this commit reverted works fine on both cases.
I am very new to U-boot, please do let me know if more info are to be provided.
Attached the full command line, for reference:
/usr/bin/qemu-system-riscv64 \ -machine virt \ -bios u-boot/u-boot.bin \ -m 1024 \ -device '{"driver":"pcie-root-port","port":8,"chassis":1,"id":"pci.1","bus":"pcie.0","multifunction":true,"addr":"0x1"}' \ -device '{"driver":"pcie-root-port","port":9,"chassis":2,"id":"pci.2","bus":"pcie.0","addr":"0x1.0x1"}' \ -blockdev '{"driver":"file","filename":"archriscv-2022-08-24.qcow2","node-name":"libvirt-2-storage","auto-read-only":true,"discard":"unmap"}' \ -blockdev '{"node-name":"libvirt-2-format","read-only":false,"driver":"qcow2","file":"libvirt-2-storage","backing":null}' \ -device '{"driver":"virtio-blk-pci","bus":"pci.2","addr":"0x0","drive":"libvirt-2-format","id":"virtio-disk0","bootindex":1}' \ -nographic
-- Regards, Felix Yan
Please try my patch and let me know the test results
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c index 880a12cc28..cfde4007f5 100644 --- a/drivers/virtio/virtio_pci_modern.c +++ b/drivers/virtio/virtio_pci_modern.c @@ -466,7 +466,7 @@ static void __iomem *virtio_pci_map_capability(struct udevice *udev, unsigned long mask = PCI_REGION_TYPE | PCI_REGION_SYS_MEMORY | PCI_REGION_RO; unsigned long flags = PCI_REGION_MEM; - u8 *p = dm_pci_map_bar(udev, PCI_BASE_ADDRESS_0 + cap->bar, cap->offset, + u8 *p = dm_pci_map_bar(udev, PCI_BASE_ADDRESS_0 + 4 * cap->bar, cap->offset, cap->length, mask, flags);
return (void __iomem *)p;
Regards, Xiang W

On 8/28/22 19:49, Xiang W wrote:
Please try my patch and let me know the test results
diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c index 880a12cc28..cfde4007f5 100644 --- a/drivers/virtio/virtio_pci_modern.c +++ b/drivers/virtio/virtio_pci_modern.c @@ -466,7 +466,7 @@ static void __iomem *virtio_pci_map_capability(struct udevice *udev, unsigned long mask = PCI_REGION_TYPE | PCI_REGION_SYS_MEMORY | PCI_REGION_RO; unsigned long flags = PCI_REGION_MEM;
u8 *p = dm_pci_map_bar(udev, PCI_BASE_ADDRESS_0 + cap->bar, cap->offset,
u8 *p = dm_pci_map_bar(udev, PCI_BASE_ADDRESS_0 + 4 * cap->bar, cap->offset, cap->length, mask, flags); return (void __iomem *)p;
It fixes the problem for me. Tested on U-Boot master with both mmode and smode, also with the original libvirt use case.
Thanks!

Hi Andrew,
On Wed, Mar 30, 2022 at 12:59 AM Andrew Scull ascull@google.com wrote:
The virtio PCI drivers forgo a number of consistency checks, particularly around pointer validation and bounds checking. This series focuses on the modern driver to add those checks.
The start of the series adds and fixes some basic bounds checks. Later patches ensure PCI addresses fall within the expected regions rather than any arbitrary address. This is acheived by introducing range parameters to a few of the dm_pci_* functions that allow the ranges to be checked.
The series also adds a few new configs to allow parts of virtio and PCI to be disabled where the features may be unused and the current implementations don't have the needed consistencty checks.
Changelog: v2:
- Refactor dm_pci_* functions, not adding new ones
- Reorder patches per request in v1
I believe some patches have bisectability issues. CI does not complain such, but you can use buildman to catch those.
Regards, Bin
participants (6)
-
Andrew Scull
-
Bin Meng
-
Felix Yan
-
Stefan Agner
-
Tom Rini
-
Xiang W