From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935356AbdCVPQo (ORCPT ); Wed, 22 Mar 2017 11:16:44 -0400 Received: from mail-oi0-f68.google.com ([209.85.218.68]:33783 "EHLO mail-oi0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751539AbdCVPQa (ORCPT ); Wed, 22 Mar 2017 11:16:30 -0400 MIME-Version: 1.0 In-Reply-To: <20170322150403.GA4951@red-moon> References: <20170227151436.18698-1-lorenzo.pieralisi@arm.com> <20170227151436.18698-4-lorenzo.pieralisi@arm.com> <20170316211243.GE9739@bhelgaas-glaptop.roam.corp.google.com> <20170322150403.GA4951@red-moon> From: Arnd Bergmann Date: Wed, 22 Mar 2017 16:15:29 +0100 X-Google-Sender-Auth: K5MAVMJUW57pGyEI6GCAljj_H7A Message-ID: Subject: Re: [PATCH 03/20] asm-generic/io.h: add PCI config space remap interface To: Lorenzo Pieralisi Cc: Bjorn Helgaas , linux-pci , Linux ARM , Linux Kernel Mailing List , linux-arch , Will Deacon , Bjorn Helgaas , Russell King , Catalin Marinas , Pratyush Anand , Jingoo Han , Mingkai Hu , John Garry , Tanmay Inamdar , Murali Karicheri , Bharat Kumar Gogada , Ray Jui , Wenrui Li , Shawn Lin , Minghuan Lian , Jon Mason , Gabriele Paoloni , Thomas Petazzoni , Joao Pinto , Thierry Reding , Michal Simek , Stanimir Varbanov , Zhou Wang , Roy Zang , "Luis R. Rodriguez" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Mar 22, 2017 at 4:04 PM, Lorenzo Pieralisi wrote: > Hi Bjorn, Arnd, > > On Thu, Mar 16, 2017 at 04:12:43PM -0500, Bjorn Helgaas wrote: >> [+cc Luis] >> >> On Mon, Feb 27, 2017 at 03:14:14PM +0000, Lorenzo Pieralisi wrote: >> > The PCI specifications (Rev 3.0, 3.2.5 "Transaction Ordering and >> > Posting") mandate non-posted configuration transactions. As further >> > highlighted in the PCIe specifications (4.0 - Rev0.3, "Ordering >> > Considerations for the Enhanced Configuration Access Mechanism"), >> > through ECAM and ECAM-derivative configuration mechanism, the memory >> > mapped transactions from the host CPU into Configuration Requests on the >> > PCI express fabric may create ordering problems for software because >> > writes to memory address are typically posted transactions (unless the >> > architecture can enforce through virtual address mapping non-posted >> > write transactions behaviour) but writes to Configuration Space are not >> > posted on the PCI express fabric. >> > >> > Current DT and ACPI host bridge controllers map PCI configuration space >> > (ECAM and ECAM-derivative) into the virtual address space through >> > ioremap() calls, that are non-cacheable device accesses on most >> > architectures, but may provide "bufferable" or "posted" write semantics >> > in architecture like eg ARM/ARM64 that allow ioremap'ed regions writes >> > to be buffered in the bus connecting the host CPU to the PCI fabric; >> > this behaviour, as underlined in the PCIe specifications, may trigger >> > transactions ordering rules and must be prevented. >> > >> > Introduce a new generic and explicit API to create a memory >> > mapping for ECAM and ECAM-derivative config space area that >> > defaults to ioremap_nocache() (which should provide a sane default >> > behaviour) but still allowing architectures on which ioremap_nocache() >> > results in posted write transactions to override the function >> > call with an arch specific implementation that complies with >> > the PCI specifications for configuration transactions. >> > >> > Signed-off-by: Lorenzo Pieralisi >> > Cc: Arnd Bergmann >> > Cc: Will Deacon >> > Cc: Bjorn Helgaas >> > Cc: Russell King >> > Cc: Catalin Marinas >> > --- >> > include/asm-generic/io.h | 9 +++++++++ >> > 1 file changed, 9 insertions(+) >> > >> > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h >> > index 7ef015e..52dda81 100644 >> > --- a/include/asm-generic/io.h >> > +++ b/include/asm-generic/io.h >> > @@ -915,6 +915,15 @@ extern void ioport_unmap(void __iomem *p); >> > #endif /* CONFIG_GENERIC_IOMAP */ >> > #endif /* CONFIG_HAS_IOPORT_MAP */ >> > >> > +#ifndef pci_remap_cfgspace >> > +#define pci_remap_cfgspace pci_remap_cfgspace >> > +static inline void __iomem *pci_remap_cfgspace(phys_addr_t offset, >> > + size_t size) >> > +{ >> > + return ioremap_nocache(offset, size); >> > +} >> >> I'm fine with this conceptually, but I think it would make more sense >> if the name weren't specific to PCI or config space, e.g., >> ioremap_nopost() or something. > > I would like to respin shortly so I have to make a decision on the > interface, either: > > (1) I keep it a PCI only interface (which means I can even move it to > include/linux/pci.h and arch*/include/asm/pci.h to override it) > > or > > (2) We add it as a generic ioremap_* interface (I am not sure though > that's really useful other than to map PCI config space, actually > the reasoning behind the naming was to limit its usage to PCI > config space mappings) > > I take it as Bjorn is keener on (2), just running a final check before > putting v2 together to make progress. I'm fine with either way. Arnd