[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