On Wed, Mar 30, 2022 at 12:59 AM Andrew Scull <asc...@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 <asc...@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...@gmail.com>