[PATCH v2 14/18] pci: Match region flags using a mask
Bin Meng
bmeng.cn at gmail.com
Wed Apr 13 16:59:27 CEST 2022
On Wed, Mar 30, 2022 at 12:59 AM Andrew Scull <ascull at 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 at 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 at gmail.com>
More information about the U-Boot
mailing list