Re: [PATCH 09/11] pci: Add function to validate PCI address range

Bin Meng
2022-03-25T07:14:41Z

On Sun, Mar 20, 2022 at 7:42 PM Andrew Scull <ascull@google.com> wrote:
>
> Add a function to convert a PCI address range to a physical address
> range. The address range is validated to ensure it is contained within
> one of the PCI address regions and that the whole range can be indexed
> from the resulting physical address.

I am not sure if we need to provide such an API given there is already
dm_pci_bus_to_phys() and friends. Not sure we really need this new API
to add a check against the range.

>
> Signed-off-by: Andrew Scull <ascull@google.com>
> ---
> drivers/pci/pci-uclass.c | 30 ++++++++++++++++++++++++++++++
> include/pci.h | 16 ++++++++++++++++
> 2 files changed, 46 insertions(+)
>
> diff --git a/drivers/pci/pci-uclass.c b/drivers/pci/pci-uclass.c
> index 54a05d7567..efab8916e2 100644
> --- a/drivers/pci/pci-uclass.c
> +++ b/drivers/pci/pci-uclass.c
> @@ -1452,6 +1452,36 @@ phys_addr_t dm_pci_bus_to_phys(struct udevice *dev, pci_addr_t bus_addr,
> return phys_addr;
> }
>
> +phys_addr_t dm_pci_bus_range_to_phys(struct udevice *dev, pci_addr_t bus_addr,
> + size_t size, unsigned long mask,
> + unsigned long flags)
> +{
> + struct udevice *ctlr = pci_get_controller(dev);
> + struct pci_controller *hose = dev_get_uclass_priv(ctlr);
> + struct pci_region *res;
> + size_t offset;
> + int i;
> +
> + for (i = 0; i < hose->region_count; i++) {
> + res = &hose->regions[i];
> +
> + if ((res->flags & mask) != flags)
> + continue;
> +
> + if (bus_addr < res->bus_start)
> + continue;
> +
> + offset = bus_addr - res->bus_start;
> + if (offset >= res->size || size > res->size - offset)
> + continue;
> +
> + return res->phys_start + offset;
> + }
> +
> + puts("pci_bus_range_to_phys: invalid address range\n");
> + return 0;
> +}

If we really need this, I believe we can do some refactoring on the
existing _dm_pci_bus_to_phys() to provide the new capability you
wanted.

> +
> 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)
> diff --git a/include/pci.h b/include/pci.h
> index 673c95c6bb..15b3031c1f 100644
> --- a/include/pci.h
> +++ b/include/pci.h
> @@ -1446,6 +1446,22 @@ u32 dm_pci_read_bar32(const struct udevice *dev, int barnum);
> phys_addr_t dm_pci_bus_to_phys(struct udevice *dev, pci_addr_t addr,
> unsigned long flags);
>
> +/**
> + * dm_pci_bus_range_to_phys() - convert a PCI bus address range to a physical
> + * address.
> + *
> + * @dev: Device containing the PCI address
> + * @addr: PCI address to convert
> + * @size: Size of the addressrange

need a space before 'range'

> + * @mask: Mask of flags to match
> + * @flags: Flags for the region type (PCI_REGION_...)
> + * Return: physical address corresponding to that PCI bus address range or 0 if
> + * the range could not be converted
> + */
> +phys_addr_t dm_pci_bus_range_to_phys(struct udevice *dev, pci_addr_t bus_addr,
> + size_t size, unsigned long mask,
> + unsigned long flags);
> +
> /**
> * dm_pci_phys_to_bus() - convert a physical address to a PCI bus address
> *

Please add a test case in test/dm/pci.c to cover this new API.

Regards,
Bin