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
| ||
|
Date: Mon, 23 Mar 2015 12:20:47 -0500 From: Bjorn Helgaas <bhelgaas@...gle.com> To: "Luis R. Rodriguez" <mcgrof@...not-panic.com> Cc: Andy Lutomirski <luto@...capital.net>, Ingo Molnar <mingo@...hat.com>, Thomas Gleixner <tglx@...utronix.de>, "H. Peter Anvin" <hpa@...or.com>, jgross@...e.com, Jan Beulich <JBeulich@...e.com>, Borislav Petkov <bp@...e.de>, Suresh Siddha <suresh.b.siddha@...el.com>, venkatesh.pallipadi@...el.com, Dave Airlie <airlied@...hat.com>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, linux-fbdev@...r.kernel.org, "x86@...nel.org" <x86@...nel.org>, "xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>, "Luis R. Rodriguez" <mcgrof@...e.com>, Ingo Molnar <mingo@...e.hu>, Daniel Vetter <daniel.vetter@...ll.ch>, Antonino Daplas <adaplas@...il.com>, Jean-Christophe Plagniol-Villard <plagnioj@...osoft.com>, Tomi Valkeinen <tomi.valkeinen@...com>, Dave Hansen <dave.hansen@...ux.intel.com>, Arnd Bergmann <arnd@...db.de>, "Michael S. Tsirkin" <mst@...hat.com>, Stefan Bader <stefan.bader@...onical.com>, Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>, Ville Syrjälä <ville.syrjala@...ux.intel.com>, David Vrabel <david.vrabel@...rix.com>, Toshi Kani <toshi.kani@...com>, Roger Pau Monné <roger.pau@...rix.com>, xen-devel <xen-devel@...ts.xensource.com> Subject: Re: [PATCH v1 05/47] pci: add pci_iomap_wc() variants Hi Luis, This seems OK to me, but I'm curious about a few things. On Fri, Mar 20, 2015 at 6:17 PM, Luis R. Rodriguez <mcgrof@...not-panic.com> wrote: > From: "Luis R. Rodriguez" <mcgrof@...e.com> > > This allows drivers to take advantage of write-combining > when possible. Ideally we'd have pci_read_bases() just > peg an IORESOURCE_WC flag for us We do set IORESOURCE_PREFETCH. Do you mean something different? > but where exactly > video devices memory lie varies *largely* and at times things > are mixed with MMIO registers, sometimes we can address > the changes in drivers, other times the change requires > intrusive changes. What does a video device address have to do with this? I do see that if a BAR maps only a frame buffer, the device might be able to mark it prefetchable, while if the BAR mapped both a frame buffer and some registers, it might not be able to make it prefetchable. But that doesn't seem like it depends on the *address*. pci_iomap_range() already makes a cacheable mapping if IORESOURCE_CACHEABLE; I'm guessing that you would like it to automatically use WC if the BAR if IORESOURCE_PREFETCH, e.g., if (flags & IORESOURCE_CACHEABLE) return ioremap(start, len); if (flags & IORESOURCE_PREFETCH) return ioremap_wc(start, len); return ioremap_nocache(start, len); Is there a reason not to do that? > Although there is also arch_phys_wc_add() that makes use of > architecture specific write-combinging alternatives (MTRR on > x86 when a system does not have PAT) we void polluting > pci_iomap() space with it and force drivers and subsystems > that want to use it to be explicit. > > There are a few motivations for this: > > a) Take advantage of PAT when available > > b) Help bury MTRR code away, MTRR is architecture specific and on > x86 its replaced by PAT > > c) Help with the goal of eventually using _PAGE_CACHE_UC over > _PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (de33c442e) > ... > +void __iomem *pci_iomap_wc_range(struct pci_dev *dev, > + int bar, > + unsigned long offset, > + unsigned long maxlen) > +{ > + resource_size_t start = pci_resource_start(dev, bar); > + resource_size_t len = pci_resource_len(dev, bar); > + unsigned long flags = pci_resource_flags(dev, bar); > + > + if (len <= offset || !start) > + return NULL; > + len -= offset; > + start += offset; > + if (maxlen && len > maxlen) > + len = maxlen; > + if (flags & IORESOURCE_IO) > + return __pci_ioport_map(dev, start, len); > + if (flags & IORESOURCE_MEM) Should we log a note in dmesg if the BAR is *not* IORESOURCE_PREFETCH? I know the driver might know it's safe even if the device didn't mark the BAR as prefetchable, but it does seem like an easy way for a driver to shoot itself in the foot. > + return ioremap_wc(start, len); > + /* What? */ > + return NULL; > +} -- 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