Message ID | 20170227151436.18698-3-lorenzo.pieralisi@arm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
[+cc Luis] On Mon, Feb 27, 2017 at 03:14:13PM +0000, Lorenzo Pieralisi wrote: > According to the PCI local bus specifications (Revision 3.0, 3.2.5), > I/O Address space transactions are non-posted. On architectures where > I/O space is implemented through a chunk of memory mapped space mapped > to PCI address space (ie IA64/ARM/ARM64) the memory mapping for the > region backing I/O Address Space transactions determines the I/O > transactions attributes (before the transactions actually reaches the > PCI bus where it is handled according to the PCI specifications). > > Current pci_remap_iospace() interface, that is used to map the PCI I/O > Address Space into virtual address space, use pgprot_device() as memory > attribute for the virtual address mapping, that in some architectures > (ie ARM64) provides non-cacheable but write bufferable mappings (ie > posted writes), which clash with the non-posted write behaviour for I/O > Address Space mandated by the PCI specifications. > > Update the prot ioremap_page_range() parameter in pci_remap_iospace() > to pgprot_noncached to ensure that the virtual mapping backing > I/O Address Space guarantee non-posted write transactions issued > when addressing I/O Address Space through the MMIO mapping. > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: Will Deacon <will.deacon@arm.com> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Russell King <linux@armlinux.org.uk> > Cc: Catalin Marinas <catalin.marinas@arm.com> > --- > drivers/pci/pci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index bd98674..bfb3c6e 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -3375,7 +3375,7 @@ int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr) > return -EINVAL; > > return ioremap_page_range(vaddr, vaddr + resource_size(res), phys_addr, > - pgprot_device(PAGE_KERNEL)); > + pgprot_noncached(PAGE_KERNEL)); pgprot_device() is equivalent to pgprot_noncached() on all arches except ARM64, and I trust you're doing the right thing on ARM64, so I'm fine with this from a PCI perspective. I do find this puzzling because I naively expected pgprot_noncached() to match up with ioremap_nocache(), and apparently it doesn't. For example, ARM64 ioremap_nocache() uses PROT_DEVICE_nGnRE, which doesn't match the MT_DEVICE_nGnRnE in pgprot_noncached(). The point of these patches is to use non-posted mappings. Apparently you can do that with pgprot_noncached() here, but ioremap_nocache() isn't enough for the config space mappings? I suppose that's a consequence of the pgprot_noncached() vs ioremap_nocache() mismatch, but this is all extremely confusing. > #else > /* this architecture does not have memory mapped I/O space, > so this function should never be called */ > -- > 2.10.0 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Thu, Mar 16, 2017 at 04:48:44PM -0500, Bjorn Helgaas wrote: > [+cc Luis] > > On Mon, Feb 27, 2017 at 03:14:13PM +0000, Lorenzo Pieralisi wrote: > > According to the PCI local bus specifications (Revision 3.0, 3.2.5), > > I/O Address space transactions are non-posted. On architectures where > > I/O space is implemented through a chunk of memory mapped space mapped > > to PCI address space (ie IA64/ARM/ARM64) the memory mapping for the > > region backing I/O Address Space transactions determines the I/O > > transactions attributes (before the transactions actually reaches the > > PCI bus where it is handled according to the PCI specifications). > > > > Current pci_remap_iospace() interface, that is used to map the PCI I/O > > Address Space into virtual address space, use pgprot_device() as memory > > attribute for the virtual address mapping, that in some architectures > > (ie ARM64) provides non-cacheable but write bufferable mappings (ie > > posted writes), <sarcasm> Gee wiz, I am glad this is so well documented. </sarcasm> > > which clash with the non-posted write behaviour for I/O > > Address Space mandated by the PCI specifications. > > > > Update the prot ioremap_page_range() parameter in pci_remap_iospace() > > to pgprot_noncached to ensure that the virtual mapping backing > > I/O Address Space guarantee non-posted write transactions issued > > when addressing I/O Address Space through the MMIO mapping. How did we end up with pgprot_device() then in the first place Liviu Dudau [0] ? I ask for two reasons: a) should we then use a Fixes tag for this patch ? b) it does not seem clear what the semantics for pgprot_device() or even pgprot_noncached(). Can you add some ? 8b921acfeffdb ("PCI: Add pci_remap_iospace() to map bus I/O resources") Also this patch claims archs can override this call alone, as its __weak. So is the right thing to do to change pci_remap_iospace() to pgprot_noncached() or is it for archs to add their own pci_remap_iospace()? If so why ? Without proper semantics defined for these helpers this is all fuzzy. > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > Cc: Arnd Bergmann <arnd@arndb.de> > > Cc: Will Deacon <will.deacon@arm.com> > > Cc: Bjorn Helgaas <bhelgaas@google.com> > > Cc: Russell King <linux@armlinux.org.uk> > > Cc: Catalin Marinas <catalin.marinas@arm.com> > > --- > > drivers/pci/pci.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > > index bd98674..bfb3c6e 100644 > > --- a/drivers/pci/pci.c > > +++ b/drivers/pci/pci.c > > @@ -3375,7 +3375,7 @@ int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr) > > return -EINVAL; > > > > return ioremap_page_range(vaddr, vaddr + resource_size(res), phys_addr, > > - pgprot_device(PAGE_KERNEL)); > > + pgprot_noncached(PAGE_KERNEL)); > > pgprot_device() is equivalent to pgprot_noncached() on all arches > except ARM64, and I trust you're doing the right thing on ARM64, so > I'm fine with this from a PCI perspective. > > I do find this puzzling because I naively expected pgprot_noncached() > to match up with ioremap_nocache(), and apparently it doesn't. > > For example, ARM64 ioremap_nocache() uses PROT_DEVICE_nGnRE, which > doesn't match the MT_DEVICE_nGnRnE in pgprot_noncached(). > > The point of these patches is to use non-posted mappings. Apparently > you can do that with pgprot_noncached() here, but ioremap_nocache() > isn't enough for the config space mappings? This is for iospace it seems, so the other patch I think was for config space. Luis > I suppose that's a consequence of the pgprot_noncached() vs > ioremap_nocache() mismatch, but this is all extremely confusing. > > > #else > > /* this architecture does not have memory mapped I/O space, > > so this function should never be called */ > > -- > > 2.10.0 > > > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
On Fri, Mar 17, 2017 at 01:33:21AM +0100, Luis R. Rodriguez wrote: > On Thu, Mar 16, 2017 at 04:48:44PM -0500, Bjorn Helgaas wrote: > > [+cc Luis] > > > > On Mon, Feb 27, 2017 at 03:14:13PM +0000, Lorenzo Pieralisi wrote: > > > According to the PCI local bus specifications (Revision 3.0, 3.2.5), > > > I/O Address space transactions are non-posted. On architectures where > > > I/O space is implemented through a chunk of memory mapped space mapped > > > to PCI address space (ie IA64/ARM/ARM64) the memory mapping for the > > > region backing I/O Address Space transactions determines the I/O > > > transactions attributes (before the transactions actually reaches the > > > PCI bus where it is handled according to the PCI specifications). > > > > > > Current pci_remap_iospace() interface, that is used to map the PCI I/O > > > Address Space into virtual address space, use pgprot_device() as memory > > > attribute for the virtual address mapping, that in some architectures > > > (ie ARM64) provides non-cacheable but write bufferable mappings (ie > > > posted writes), > > <sarcasm> > Gee wiz, I am glad this is so well documented. > </sarcasm> > > > > which clash with the non-posted write behaviour for I/O > > > Address Space mandated by the PCI specifications. > > > > > > Update the prot ioremap_page_range() parameter in pci_remap_iospace() > > > to pgprot_noncached to ensure that the virtual mapping backing > > > I/O Address Space guarantee non-posted write transactions issued > > > when addressing I/O Address Space through the MMIO mapping. > > How did we end up with pgprot_device() then in the first place Liviu Dudau [0] ? > I ask for two reasons: [replying using personal email as the corporate email system is taking its sweet time to deliver the email to my inbox] I've asked the people with the right knowledge about the correct API to use (Hi Catalin!), and during the review it did not throw any red flags. I guess, given Bjorn's comment, that everyone assumed AArch64 is the same as all other architectures and pgprot_device is synonymous to pgprot_noncached. > > a) should we then use a Fixes tag for this patch ? I'm not aware of issues being reported, but Lorenzo might have more info on this. > b) it does not seem clear what the semantics for pgprot_device() or even > pgprot_noncached(). Can you add some ? > > 8b921acfeffdb ("PCI: Add pci_remap_iospace() to map bus I/O resources") > > Also this patch claims archs can override this call alone, as its __weak. > So is the right thing to do to change pci_remap_iospace() to pgprot_noncached() > or is it for archs to add their own pci_remap_iospace()? If so why ? Without > proper semantics defined for these helpers this is all fuzzy. That was the initial intention, to let arches / platforms overwrite the whole pci_remap_iospace(). I guess the reality is that no one needs to overwrite it except for the AArch64 quirk, so probably easier to remove the __weak and fix the attributes for arm64. Best regards, Liviu > > > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > > Cc: Arnd Bergmann <arnd@arndb.de> > > > Cc: Will Deacon <will.deacon@arm.com> > > > Cc: Bjorn Helgaas <bhelgaas@google.com> > > > Cc: Russell King <linux@armlinux.org.uk> > > > Cc: Catalin Marinas <catalin.marinas@arm.com> > > > --- > > > drivers/pci/pci.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > > > index bd98674..bfb3c6e 100644 > > > --- a/drivers/pci/pci.c > > > +++ b/drivers/pci/pci.c > > > @@ -3375,7 +3375,7 @@ int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr) > > > return -EINVAL; > > > > > > return ioremap_page_range(vaddr, vaddr + resource_size(res), phys_addr, > > > - pgprot_device(PAGE_KERNEL)); > > > + pgprot_noncached(PAGE_KERNEL)); > > > > pgprot_device() is equivalent to pgprot_noncached() on all arches > > except ARM64, and I trust you're doing the right thing on ARM64, so > > I'm fine with this from a PCI perspective. > > > > I do find this puzzling because I naively expected pgprot_noncached() > > to match up with ioremap_nocache(), and apparently it doesn't. > > > > For example, ARM64 ioremap_nocache() uses PROT_DEVICE_nGnRE, which > > doesn't match the MT_DEVICE_nGnRnE in pgprot_noncached(). > > > > The point of these patches is to use non-posted mappings. Apparently > > you can do that with pgprot_noncached() here, but ioremap_nocache() > > isn't enough for the config space mappings? > > This is for iospace it seems, so the other patch I think was for > config space. > > Luis > > > I suppose that's a consequence of the pgprot_noncached() vs > > ioremap_nocache() mismatch, but this is all extremely confusing. > > > > > #else > > > /* this architecture does not have memory mapped I/O space, > > > so this function should never be called */ > > > -- > > > 2.10.0 > > > > > > > > > _______________________________________________ > > > linux-arm-kernel mailing list > > > linux-arm-kernel@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > > > > -- > Luis Rodriguez, SUSE LINUX GmbH > Maxfeldstrasse 5; D-90409 Nuernberg
On Fri, Mar 17, 2017 at 10:43:39AM +0000, Liviu Dudau wrote: > On Fri, Mar 17, 2017 at 01:33:21AM +0100, Luis R. Rodriguez wrote: > > a) should we then use a Fixes tag for this patch ? > > I'm not aware of issues being reported, but Lorenzo might have more info on this. Lorenzo ? If not what exactly made you discover this ? If it is a fix, and only ARM64 is implicated, seems like a worthy change to consider for stable for the sake of stable ARM64 kernels. But, that would leave the PCI config space without a simple 1 liner fix too -- so maybe its not worth it. Distributions wanting to support ARM64 however would like to carry these changes, so some annotations such as Fixes should help. > > b) it does not seem clear what the semantics for pgprot_device() or even > > pgprot_noncached(). Can you add some ? > > > > 8b921acfeffdb ("PCI: Add pci_remap_iospace() to map bus I/O resources") > > > > Also this patch claims archs can override this call alone, as its __weak. > > So is the right thing to do to change pci_remap_iospace() to pgprot_noncached() > > or is it for archs to add their own pci_remap_iospace()? If so why ? Without > > proper semantics defined for these helpers this is all fuzzy. > > That was the initial intention, to let arches / platforms overwrite the whole > pci_remap_iospace(). I guess the reality is that no one needs to overwrite it except > for the AArch64 quirk, so probably easier to remove the __weak and fix the attributes for arm64. Sounds much more reasonable to me. Luis
On Fri, Mar 17, 2017 at 01:33:21AM +0100, Luis R. Rodriguez wrote: > On Thu, Mar 16, 2017 at 04:48:44PM -0500, Bjorn Helgaas wrote: > > [+cc Luis] > > > > On Mon, Feb 27, 2017 at 03:14:13PM +0000, Lorenzo Pieralisi wrote: > > > According to the PCI local bus specifications (Revision 3.0, 3.2.5), > > > I/O Address space transactions are non-posted. On architectures where > > > I/O space is implemented through a chunk of memory mapped space mapped > > > to PCI address space (ie IA64/ARM/ARM64) the memory mapping for the > > > region backing I/O Address Space transactions determines the I/O > > > transactions attributes (before the transactions actually reaches the > > > PCI bus where it is handled according to the PCI specifications). > > > > > > Current pci_remap_iospace() interface, that is used to map the PCI I/O > > > Address Space into virtual address space, use pgprot_device() as memory > > > attribute for the virtual address mapping, that in some architectures > > > (ie ARM64) provides non-cacheable but write bufferable mappings (ie > > > posted writes), > > <sarcasm> > Gee wiz, I am glad this is so well documented. > </sarcasm> I'm not sure this is actionable feedback. The two paragraphs above seem clear and useful to me. How would you like to see them improved? > > > ... > > > @@ -3375,7 +3375,7 @@ int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr) > > > return -EINVAL; > > > > > > return ioremap_page_range(vaddr, vaddr + resource_size(res), phys_addr, > > > - pgprot_device(PAGE_KERNEL)); > > > + pgprot_noncached(PAGE_KERNEL)); > > > > ... > > I do find this puzzling because I naively expected pgprot_noncached() > > to match up with ioremap_nocache(), and apparently it doesn't. > > > > For example, ARM64 ioremap_nocache() uses PROT_DEVICE_nGnRE, which > > doesn't match the MT_DEVICE_nGnRnE in pgprot_noncached(). > > > > The point of these patches is to use non-posted mappings. Apparently > > you can do that with pgprot_noncached() here, but ioremap_nocache() > > isn't enough for the config space mappings? > > This is for iospace it seems, so the other patch I think was for > config space. I understand that 02/20 is for PCI I/O port space and 03/20 is for PCI config space. I'm confused because I thought we wanted the same non-posted mapping for both, but looks like they're different. Patch 02/20 uses ioremap_page_range(..., pgprot_noncached(PAGE_KERNEL)) to map PCI I/O port space: #define pgprot_noncached(prot) \ __pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_DEVICE_nGnRnE) | PTE_PXN | PTE_UXN) Patch 03/20 uses ioremap_nocache() to map PCI config space: #define ioremap_nocache(addr, size) __ioremap((addr), (size), __pgprot(PROT_DEVICE_nGnRE)) So the I/O port mapping uses MT_DEVICE_nGnRnE, while the config space mapping uses PROT_DEVICE_nGnRE, which looks different. Bjorn
On Fri, Mar 17, 2017 at 05:26:18PM +0100, Luis R. Rodriguez wrote: > On Fri, Mar 17, 2017 at 10:43:39AM +0000, Liviu Dudau wrote: > > On Fri, Mar 17, 2017 at 01:33:21AM +0100, Luis R. Rodriguez wrote: > > > a) should we then use a Fixes tag for this patch ? > > > > I'm not aware of issues being reported, but Lorenzo might have more info on this. > > Lorenzo ? If not what exactly made you discover this ? If it is a fix, and only > ARM64 is implicated, seems like a worthy change to consider for stable for the > sake of stable ARM64 kernels. But, that would leave the PCI config space without > a simple 1 liner fix too -- so maybe its not worth it. Distributions wanting > to support ARM64 however would like to carry these changes, so some annotations > such as Fixes should help. It started with this thread: http://lists.infradead.org/pipermail/linux-arm-kernel/2017-January/477353.html this series is not fixing any current issue I am aware of (but I am not keen on leaving code as-is either) hence adding a Fixes: tag is problematic. I would leave stable kernels alone for the time being. Lorenzo > > > b) it does not seem clear what the semantics for pgprot_device() or even > > > pgprot_noncached(). Can you add some ? > > > > > > 8b921acfeffdb ("PCI: Add pci_remap_iospace() to map bus I/O resources") > > > > > > Also this patch claims archs can override this call alone, as its __weak. > > > So is the right thing to do to change pci_remap_iospace() to pgprot_noncached() > > > or is it for archs to add their own pci_remap_iospace()? If so why ? Without > > > proper semantics defined for these helpers this is all fuzzy. > > > > That was the initial intention, to let arches / platforms overwrite the whole > > pci_remap_iospace(). I guess the reality is that no one needs to overwrite it except > > for the AArch64 quirk, so probably easier to remove the __weak and fix the attributes for arm64. > > Sounds much more reasonable to me. > > Luis
On Mon, Mar 20, 2017 at 11:06:36AM -0500, Bjorn Helgaas wrote: [...] > > > > @@ -3375,7 +3375,7 @@ int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr) > > > > return -EINVAL; > > > > > > > > return ioremap_page_range(vaddr, vaddr + resource_size(res), phys_addr, > > > > - pgprot_device(PAGE_KERNEL)); > > > > + pgprot_noncached(PAGE_KERNEL)); > > > > > > ... > > > I do find this puzzling because I naively expected pgprot_noncached() > > > to match up with ioremap_nocache(), and apparently it doesn't. > > > > > > For example, ARM64 ioremap_nocache() uses PROT_DEVICE_nGnRE, which > > > doesn't match the MT_DEVICE_nGnRnE in pgprot_noncached(). > > > > > > The point of these patches is to use non-posted mappings. Apparently > > > you can do that with pgprot_noncached() here, but ioremap_nocache() > > > isn't enough for the config space mappings? > > > > This is for iospace it seems, so the other patch I think was for > > config space. > > I understand that 02/20 is for PCI I/O port space and 03/20 is for PCI > config space. I'm confused because I thought we wanted the same > non-posted mapping for both, but looks like they're different. > > Patch 02/20 uses ioremap_page_range(..., pgprot_noncached(PAGE_KERNEL)) > to map PCI I/O port space: > > #define pgprot_noncached(prot) \ > __pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_DEVICE_nGnRnE) | PTE_PXN | PTE_UXN) > > Patch 03/20 uses ioremap_nocache() to map PCI config space: > > #define ioremap_nocache(addr, size) __ioremap((addr), (size), __pgprot(PROT_DEVICE_nGnRE)) > > So the I/O port mapping uses MT_DEVICE_nGnRnE, while the config space > mapping uses PROT_DEVICE_nGnRE, which looks different. On ARM64 (PATCH 4) and ARM (PATCH 5) we override pci_remap_cfgspace() with implementations that provide non-posted writes bus attributes, PATCH 3 is just there to provide a "safe" (well, I need input on that) fall-back. Thanks, Lorenzo
On Mon, Mar 20, 2017 at 04:26:27PM +0000, Lorenzo Pieralisi wrote: > On Mon, Mar 20, 2017 at 11:06:36AM -0500, Bjorn Helgaas wrote: > > [...] > > > > > > @@ -3375,7 +3375,7 @@ int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr) > > > > > return -EINVAL; > > > > > > > > > > return ioremap_page_range(vaddr, vaddr + resource_size(res), phys_addr, > > > > > - pgprot_device(PAGE_KERNEL)); > > > > > + pgprot_noncached(PAGE_KERNEL)); > > > > > > > > ... > > > > I do find this puzzling because I naively expected pgprot_noncached() > > > > to match up with ioremap_nocache(), and apparently it doesn't. > > > > > > > > For example, ARM64 ioremap_nocache() uses PROT_DEVICE_nGnRE, which > > > > doesn't match the MT_DEVICE_nGnRnE in pgprot_noncached(). > > > > > > > > The point of these patches is to use non-posted mappings. Apparently > > > > you can do that with pgprot_noncached() here, but ioremap_nocache() > > > > isn't enough for the config space mappings? > > > > > > This is for iospace it seems, so the other patch I think was for > > > config space. > > > > I understand that 02/20 is for PCI I/O port space and 03/20 is for PCI > > config space. I'm confused because I thought we wanted the same > > non-posted mapping for both, but looks like they're different. > > > > Patch 02/20 uses ioremap_page_range(..., pgprot_noncached(PAGE_KERNEL)) > > to map PCI I/O port space: > > > > #define pgprot_noncached(prot) \ > > __pgprot_modify(prot, PTE_ATTRINDX_MASK, PTE_ATTRINDX(MT_DEVICE_nGnRnE) | PTE_PXN | PTE_UXN) > > > > Patch 03/20 uses ioremap_nocache() to map PCI config space: > > > > #define ioremap_nocache(addr, size) __ioremap((addr), (size), __pgprot(PROT_DEVICE_nGnRE)) > > > > So the I/O port mapping uses MT_DEVICE_nGnRnE, while the config space > > mapping uses PROT_DEVICE_nGnRE, which looks different. > > On ARM64 (PATCH 4) and ARM (PATCH 5) we override pci_remap_cfgspace() > with implementations that provide non-posted writes bus attributes, > PATCH 3 is just there to provide a "safe" (well, I need input on that) > fall-back. Ah, thanks, that makes sense to me finally! After patch 4, pci_remap_cfgspace() uses PROT_DEVICE_nGnRnE, which does look like the MT_DEVICE_nGnRnE used in the ioremap_page_range() path. I wonder if we can get rid of pgprot_device() altogether? Patch 2 removed the use in pci_remap_iospace(), and patch 18 removes the use in tegra_pcie_bus_alloc(). Bjorn
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index bd98674..bfb3c6e 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -3375,7 +3375,7 @@ int pci_remap_iospace(const struct resource *res, phys_addr_t phys_addr) return -EINVAL; return ioremap_page_range(vaddr, vaddr + resource_size(res), phys_addr, - pgprot_device(PAGE_KERNEL)); + pgprot_noncached(PAGE_KERNEL)); #else /* this architecture does not have memory mapped I/O space, so this function should never be called */
According to the PCI local bus specifications (Revision 3.0, 3.2.5), I/O Address space transactions are non-posted. On architectures where I/O space is implemented through a chunk of memory mapped space mapped to PCI address space (ie IA64/ARM/ARM64) the memory mapping for the region backing I/O Address Space transactions determines the I/O transactions attributes (before the transactions actually reaches the PCI bus where it is handled according to the PCI specifications). Current pci_remap_iospace() interface, that is used to map the PCI I/O Address Space into virtual address space, use pgprot_device() as memory attribute for the virtual address mapping, that in some architectures (ie ARM64) provides non-cacheable but write bufferable mappings (ie posted writes), which clash with the non-posted write behaviour for I/O Address Space mandated by the PCI specifications. Update the prot ioremap_page_range() parameter in pci_remap_iospace() to pgprot_noncached to ensure that the virtual mapping backing I/O Address Space guarantee non-posted write transactions issued when addressing I/O Address Space through the MMIO mapping. Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: Will Deacon <will.deacon@arm.com> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Russell King <linux@armlinux.org.uk> Cc: Catalin Marinas <catalin.marinas@arm.com> --- drivers/pci/pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)