On Tue, Mar 31, 2015 at 07:57:47PM -0700, Yinghai Lu wrote:
> David Ahern found commit d63e2e1f3df9 ("sparc/PCI: Clip bridge windows
> to fit in upstream windows") broke booting on sparc/T5-8.
> 
> In the boot log, there is
>   pci 0000:06:00.0: reg 0x184: can't handle BAR above 4GB (bus address
>   0x110204000)
> but that only could happen when dma_addr_t is 32-bit.
> 
> According to David Miller, all DMA occurs behind an IOMMU and these
> IOMMUs only support 32-bit addressing, therefore dma_addr_t is
> 32-bit on sparc64.
> 
> Let's introduce pci_bus_addr_t instead of using dma_addr_t,
> and pci_bus_addr_t will be 64-bit on 64-bit platform or X86_PAE.
> 
> Fixes: commit d63e2e1f3df9 ("sparc/PCI: Clip bridge windows to fit in 
> upstream windows")
> Fixes: commit 23b13bc76f35 ("PCI: Fail safely if we can't handle BARs larger 
> than 4GB")
> Link: 
> http://lkml.kernel.org/r/cae9fiqu1gjy1lyrxs+ma5lcteee4xmtjrg0axj9k_tsu+m9...@mail.gmail.com
> Reported-by: David Ahern <david.ah...@oracle.com>
> Tested-by: David Ahern <david.ah...@oracle.com>
> Signed-off-by: Yinghai Lu <ying...@kernel.org>
> Cc: <sta...@vger.kernel.org> #3.19
> ---
>  drivers/pci/Kconfig |  4 ++++
>  drivers/pci/bus.c   | 10 +++++-----
>  drivers/pci/probe.c | 12 ++++++------
>  include/linux/pci.h | 12 +++++++++---
>  4 files changed, 24 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> index 7a8f1c5..6a5a269 100644
> --- a/drivers/pci/Kconfig
> +++ b/drivers/pci/Kconfig
> @@ -1,6 +1,10 @@
>  #
>  # PCI configuration
>  #
> +config PCI_BUS_ADDR_T_64BIT
> +     def_bool y if (64BIT || X86_PAE)
> +     depends on PCI

We're going to use pci_bus_addr_t in some places where we previously used
dma_addr_t, which means pci_bus_addr_t should be at least as large as
dma_addr_t.  Can you enforce that directly, e.g., with something like this?

    def_bool y if (ARCH_DMA_ADDR_T_64BIT || 64BIT || X86_PAE)

Unless we do something like that, I think this may break these arches by
changing the addresses in struct pci_bus_region from 64 bits to 32:

    arch/arm/Kconfig:
        XEN selects ARCH_DMA_ADDR_T_64BIT but not 64BIT, so it would not
        set PCI_BUS_ADDR_T_64BIT

    arch/arm/mach-axxia/Kconfig:
        ARCH_AXXIA selects ARCH_DMA_ADDR_T_64BIT but not 64BIT, so it
        would not set PCI_BUS_ADDR_T_64BIT

    arch/arm/mach-exynos/Kconfig:
        SOC_EXYNOS5440 selects ARCH_DMA_ADDR_T_64BIT if ARM_LPAE, but not
        64BIT, so it would not set PCI_BUS_ADDR_T_64BIT

    arch/arm/mach-highbank/Kconfig:
        ARCH_HIGHBANK selects ARCH_DMA_ADDR_T_64BIT if ARM_LPAE, but not
        64BIT, so it would not set PCI_BUS_ADDR_T_64BIT

    arch/arm/mach-shmobile/Kconfig:
        ARCH_SHMOBILE_MULTI selects ARCH_DMA_ADDR_T_64BIT if ARM_LPAE, but
        not 64BIT, so it would not set PCI_BUS_ADDR_T_64BIT

    arch/mips/Kconfig:
        sets ARCH_DMA_ADDR_T_64BIT if (HIGHMEM && ARCH_PHYS_ADDR_T_64BIT)
        || 64BIT, so if we have (HIGHMEM && ARCH_PHYS_ADDR_T_64BIT) but not
        64BIT, it would not set PCI_BUS_ADDR_T_64BIT
        
    arch/powerpc/Kconfig:
        sets ARCH_DMA_ADDR_T_64BIT if ARCH_PHYS_ADDR_T_64BIT, which isn't
        trivially identical to (64BIT || X86_PAE), so we may not set
        PCI_BUS_ADDR_T_64BIT in all cases where we set
        ARCH_DMA_ADDR_T_64BIT

    arch/x86/Kconfig:
        sets ARCH_DMA_ADDR_T_64BIT if X86_64 || HIGHMEM64G, which isn't
        trivially identical to (64BIT || X86_PAE), so we may not set
        PCI_BUS_ADDR_T_64BIT in all cases where we set
        ARCH_DMA_ADDR_T_64BIT

>  config PCI_MSI
>       bool "Message Signaled Interrupts (MSI and MSI-X)"
>       depends on PCI
> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> index 90fa3a7..6fbd3f2 100644
> --- a/drivers/pci/bus.c
> +++ b/drivers/pci/bus.c
> @@ -92,11 +92,11 @@ void pci_bus_remove_resources(struct pci_bus *bus)
>  }
>  
>  static struct pci_bus_region pci_32_bit = {0, 0xffffffffULL};
> -#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> +#ifdef CONFIG_PCI_BUS_ADDR_T_64BIT
>  static struct pci_bus_region pci_64_bit = {0,
> -                             (dma_addr_t) 0xffffffffffffffffULL};
> -static struct pci_bus_region pci_high = {(dma_addr_t) 0x100000000ULL,
> -                             (dma_addr_t) 0xffffffffffffffffULL};
> +                             (pci_bus_addr_t) 0xffffffffffffffffULL};
> +static struct pci_bus_region pci_high = {(pci_bus_addr_t) 0x100000000ULL,
> +                             (pci_bus_addr_t) 0xffffffffffffffffULL};
>  #endif
>  
>  /*
> @@ -200,7 +200,7 @@ int pci_bus_alloc_resource(struct pci_bus *bus, struct 
> resource *res,
>                                         resource_size_t),
>               void *alignf_data)
>  {
> -#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> +#ifdef CONFIG_PCI_BUS_ADDR_T_64BIT
>       int rc;
>  
>       if (res->flags & IORESOURCE_MEM_64) {
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 8d2f400..f71cb7c 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -253,8 +253,8 @@ int __pci_read_base(struct pci_dev *dev, enum 
> pci_bar_type type,
>       }
>  
>       if (res->flags & IORESOURCE_MEM_64) {
> -             if ((sizeof(dma_addr_t) < 8 || sizeof(resource_size_t) < 8) &&
> -                 sz64 > 0x100000000ULL) {
> +             if ((sizeof(pci_bus_addr_t) < 8 || sizeof(resource_size_t) < 8)
> +                 && sz64 > 0x100000000ULL) {
>                       res->flags |= IORESOURCE_UNSET | IORESOURCE_DISABLED;
>                       res->start = 0;
>                       res->end = 0;
> @@ -263,7 +263,7 @@ int __pci_read_base(struct pci_dev *dev, enum 
> pci_bar_type type,
>                       goto out;
>               }
>  
> -             if ((sizeof(dma_addr_t) < 8) && l) {
> +             if ((sizeof(pci_bus_addr_t) < 8) && l) {
>                       /* Above 32-bit boundary; try to reallocate */
>                       res->flags |= IORESOURCE_UNSET;
>                       res->start = 0;
> @@ -398,7 +398,7 @@ static void pci_read_bridge_mmio_pref(struct pci_bus 
> *child)
>       struct pci_dev *dev = child->self;
>       u16 mem_base_lo, mem_limit_lo;
>       u64 base64, limit64;
> -     dma_addr_t base, limit;
> +     pci_bus_addr_t base, limit;
>       struct pci_bus_region region;
>       struct resource *res;
>  
> @@ -425,8 +425,8 @@ static void pci_read_bridge_mmio_pref(struct pci_bus 
> *child)
>               }
>       }
>  
> -     base = (dma_addr_t) base64;
> -     limit = (dma_addr_t) limit64;
> +     base = (pci_bus_addr_t) base64;
> +     limit = (pci_bus_addr_t) limit64;
>  
>       if (base != base64) {
>               dev_err(&dev->dev, "can't handle bridge window above 4GB (bus 
> address %#010llx)\n",
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 211e9da..6021bbe 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -573,9 +573,15 @@ int raw_pci_read(unsigned int domain, unsigned int bus, 
> unsigned int devfn,
>  int raw_pci_write(unsigned int domain, unsigned int bus, unsigned int devfn,
>                 int reg, int len, u32 val);
>  
> +#ifdef CONFIG_PCI_BUS_ADDR_T_64BIT
> +typedef u64 pci_bus_addr_t;
> +#else
> +typedef u32 pci_bus_addr_t;
> +#endif
> +
>  struct pci_bus_region {
> -     dma_addr_t start;
> -     dma_addr_t end;
> +     pci_bus_addr_t start;
> +     pci_bus_addr_t end;
>  };
>  
>  struct pci_dynids {
> @@ -1124,7 +1130,7 @@ int __must_check pci_bus_alloc_resource(struct pci_bus 
> *bus,
>  
>  int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr);
>  
> -static inline dma_addr_t pci_bus_address(struct pci_dev *pdev, int bar)
> +static inline pci_bus_addr_t pci_bus_address(struct pci_dev *pdev, int bar)
>  {
>       struct pci_bus_region region;
>  
> -- 
> 1.8.4.5
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to