| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20150403205201.GF10892@google.com>
Date: Fri, 3 Apr 2015 15:52:01 -0500
From: Bjorn Helgaas <bhelgaas@...gle.com>
To: Yinghai Lu <yinghai@...nel.org>
Cc: David Miller <davem@...emloft.net>,
David Ahern <david.ahern@...cle.com>,
linux-pci@...r.kernel.org, sparclinux@...r.kernel.org,
linux-kernel@...r.kernel.org, stable@...r.kernel.org,
Sam Ravnborg <sam@...nborg.org>,
Russell King <linux@....linux.org.uk>,
linux-arm-kernel@...ts.infradead.org,
Ralf Baechle <ralf@...ux-mips.org>, linux-mips@...ux-mips.org,
Benjamin Herrenschmidt <benh@...nel.crashing.org>,
linuxppc-dev@...ts.ozlabs.org, x86@...nel.org
Subject: Re: [PATCH 1/3] PCI: Introduce pci_bus_addr_t
[+cc Sam (commented on previous versions), Russell, linux-arm-kernel, Ralf,
linux-mips, Ben, linuxppc-dev, x86]
On Fri, Apr 03, 2015 at 02:32:34PM -0500, Bjorn Helgaas wrote:
> 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+m9Wuw@mail.gmail.com
> > Reported-by: David Ahern <david.ahern@...cle.com>
> > Tested-by: David Ahern <david.ahern@...cle.com>
> > Signed-off-by: Yinghai Lu <yinghai@...nel.org>
> > Cc: <stable@...r.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 majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists