From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754047AbdCTK1j (ORCPT ); Mon, 20 Mar 2017 06:27:39 -0400 Received: from szxga03-in.huawei.com ([45.249.212.189]:4431 "EHLO dggrg03-dlp.huawei.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1753492AbdCTK1c (ORCPT ); Mon, 20 Mar 2017 06:27:32 -0400 Subject: Re: [PATCH 03/20] asm-generic/io.h: add PCI config space remap interface To: "Luis R. Rodriguez" , Bjorn Helgaas References: <20170227151436.18698-1-lorenzo.pieralisi@arm.com> <20170227151436.18698-4-lorenzo.pieralisi@arm.com> <20170316211243.GE9739@bhelgaas-glaptop.roam.corp.google.com> <20170317000803.GA28800@wotan.suse.de> CC: Lorenzo Pieralisi , "Paul E. McKenney" , Andy Lutomirski , , , , , Arnd Bergmann , Will Deacon , Bjorn Helgaas , Russell King , Catalin Marinas , Pratyush Anand , "Jingoo Han" , Mingkai Hu , "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 , Linuxarm From: John Garry Message-ID: Date: Mon, 20 Mar 2017 10:22:59 +0000 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <20170317000803.GA28800@wotan.suse.de> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.203.181.153] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A020203.58CFADA9.0538,ss=1,re=0.000,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2014-11-16 11:51:01, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 3cb489c61959bb7e25c5d7de862d2cfb Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 17/03/2017 00:08, Luis R. Rodriguez wrote: > 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. > > So... I take it this is actually fixing a series of odd issues also, > do we at least have some reports or actual issues ? > We (Huawei) originally raised the concern [1], but have not actually experienced any related issue. Thanks, John [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2017-January/477353.html >>> 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. > > Seems reasonable to me -- but are there other buses that could use this already > as well ? Wouldn't these other buses also run into similar issues ? Can someone > also bounce me a copy of the patches that use this ? > > While at it, please add some documentation too, the above commit log is huge, > and yet for the person eyeballing the code they won't have any clue why this > was added exactly. Since this is about helping with picking the right > ioremap due to certain semantics / requirements on the PCI config space, > we should clarify then what exactly are the expectations here. The clearer > you are the less in trouble we can get later. > > Luis > > . >