* [PATCH] powerpc/vdso32: inline __get_datapage()
From: Christophe Leroy @ 2019-08-16 14:48 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
__get_datapage() is only a few instructions to retrieve the
address of the page where the kernel stores data to the VDSO.
By inlining this function into its users, a bl/blr pair and
a mflr/mtlr pair is avoided, plus a few reg moves.
The improvement is noticeable (about 55 nsec/call on an 8xx)
vdsotest before the patch:
gettimeofday: vdso: 731 nsec/call
clock-gettime-realtime-coarse: vdso: 668 nsec/call
clock-gettime-monotonic-coarse: vdso: 745 nsec/call
vdsotest after the patch:
gettimeofday: vdso: 677 nsec/call
clock-gettime-realtime-coarse: vdso: 613 nsec/call
clock-gettime-monotonic-coarse: vdso: 690 nsec/call
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/kernel/vdso32/cacheflush.S | 10 +++++-----
arch/powerpc/kernel/vdso32/datapage.S | 29 ++++-------------------------
arch/powerpc/kernel/vdso32/datapage.h | 12 ++++++++++++
arch/powerpc/kernel/vdso32/gettimeofday.S | 11 +++++------
4 files changed, 26 insertions(+), 36 deletions(-)
create mode 100644 arch/powerpc/kernel/vdso32/datapage.h
diff --git a/arch/powerpc/kernel/vdso32/cacheflush.S b/arch/powerpc/kernel/vdso32/cacheflush.S
index 7f882e7b9f43..e9453837e4ee 100644
--- a/arch/powerpc/kernel/vdso32/cacheflush.S
+++ b/arch/powerpc/kernel/vdso32/cacheflush.S
@@ -10,6 +10,8 @@
#include <asm/vdso.h>
#include <asm/asm-offsets.h>
+#include "datapage.h"
+
.text
/*
@@ -24,14 +26,12 @@ V_FUNCTION_BEGIN(__kernel_sync_dicache)
.cfi_startproc
mflr r12
.cfi_register lr,r12
- mr r11,r3
- bl __get_datapage@local
+ get_datapage r10, r0
mtlr r12
- mr r10,r3
lwz r7,CFG_DCACHE_BLOCKSZ(r10)
addi r5,r7,-1
- andc r6,r11,r5 /* round low to line bdy */
+ andc r6,r3,r5 /* round low to line bdy */
subf r8,r6,r4 /* compute length */
add r8,r8,r5 /* ensure we get enough */
lwz r9,CFG_DCACHE_LOGBLOCKSZ(r10)
@@ -48,7 +48,7 @@ V_FUNCTION_BEGIN(__kernel_sync_dicache)
lwz r7,CFG_ICACHE_BLOCKSZ(r10)
addi r5,r7,-1
- andc r6,r11,r5 /* round low to line bdy */
+ andc r6,r3,r5 /* round low to line bdy */
subf r8,r6,r4 /* compute length */
add r8,r8,r5
lwz r9,CFG_ICACHE_LOGBLOCKSZ(r10)
diff --git a/arch/powerpc/kernel/vdso32/datapage.S b/arch/powerpc/kernel/vdso32/datapage.S
index 6984125b9fc0..d480d2d4a3fe 100644
--- a/arch/powerpc/kernel/vdso32/datapage.S
+++ b/arch/powerpc/kernel/vdso32/datapage.S
@@ -11,34 +11,13 @@
#include <asm/unistd.h>
#include <asm/vdso.h>
+#include "datapage.h"
+
.text
.global __kernel_datapage_offset;
__kernel_datapage_offset:
.long 0
-V_FUNCTION_BEGIN(__get_datapage)
- .cfi_startproc
- /* We don't want that exposed or overridable as we want other objects
- * to be able to bl directly to here
- */
- .protected __get_datapage
- .hidden __get_datapage
-
- mflr r0
- .cfi_register lr,r0
-
- bcl 20,31,data_page_branch
-data_page_branch:
- mflr r3
- mtlr r0
- addi r3, r3, __kernel_datapage_offset-data_page_branch
- lwz r0,0(r3)
- .cfi_restore lr
- add r3,r0,r3
- blr
- .cfi_endproc
-V_FUNCTION_END(__get_datapage)
-
/*
* void *__kernel_get_syscall_map(unsigned int *syscall_count) ;
*
@@ -53,7 +32,7 @@ V_FUNCTION_BEGIN(__kernel_get_syscall_map)
mflr r12
.cfi_register lr,r12
mr r4,r3
- bl __get_datapage@local
+ get_datapage r3, r0
mtlr r12
addi r3,r3,CFG_SYSCALL_MAP32
cmpli cr0,r4,0
@@ -74,7 +53,7 @@ V_FUNCTION_BEGIN(__kernel_get_tbfreq)
.cfi_startproc
mflr r12
.cfi_register lr,r12
- bl __get_datapage@local
+ get_datapage r3, r0
lwz r4,(CFG_TB_TICKS_PER_SEC + 4)(r3)
lwz r3,CFG_TB_TICKS_PER_SEC(r3)
mtlr r12
diff --git a/arch/powerpc/kernel/vdso32/datapage.h b/arch/powerpc/kernel/vdso32/datapage.h
new file mode 100644
index 000000000000..ad96256be090
--- /dev/null
+++ b/arch/powerpc/kernel/vdso32/datapage.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+.macro get_datapage ptr, tmp
+ bcl 20,31,888f
+888:
+ mflr \ptr
+ addi \ptr, \ptr, __kernel_datapage_offset - 888b
+ lwz \tmp, 0(\ptr)
+ add \ptr, \tmp, \ptr
+.endm
+
+
diff --git a/arch/powerpc/kernel/vdso32/gettimeofday.S b/arch/powerpc/kernel/vdso32/gettimeofday.S
index e10098cde89c..91a58f01dcd5 100644
--- a/arch/powerpc/kernel/vdso32/gettimeofday.S
+++ b/arch/powerpc/kernel/vdso32/gettimeofday.S
@@ -12,6 +12,8 @@
#include <asm/asm-offsets.h>
#include <asm/unistd.h>
+#include "datapage.h"
+
/* Offset for the low 32-bit part of a field of long type */
#ifdef CONFIG_PPC64
#define LOPART 4
@@ -35,8 +37,7 @@ V_FUNCTION_BEGIN(__kernel_gettimeofday)
mr r10,r3 /* r10 saves tv */
mr r11,r4 /* r11 saves tz */
- bl __get_datapage@local /* get data page */
- mr r9, r3 /* datapage ptr in r9 */
+ get_datapage r9, r0
cmplwi r10,0 /* check if tv is NULL */
beq 3f
lis r7,1000000@ha /* load up USEC_PER_SEC */
@@ -82,8 +83,7 @@ V_FUNCTION_BEGIN(__kernel_clock_gettime)
mflr r12 /* r12 saves lr */
.cfi_register lr,r12
mr r11,r4 /* r11 saves tp */
- bl __get_datapage@local /* get data page */
- mr r9,r3 /* datapage ptr in r9 */
+ get_datapage r9, r0
lis r7,NSEC_PER_SEC@h /* want nanoseconds */
ori r7,r7,NSEC_PER_SEC@l
beq cr5,70f
@@ -235,8 +235,7 @@ V_FUNCTION_BEGIN(__kernel_time)
.cfi_register lr,r12
mr r11,r3 /* r11 holds t */
- bl __get_datapage@local
- mr r9, r3 /* datapage ptr in r9 */
+ get_datapage r9, r0
lwz r3,STAMP_XTIME+TSPEC_TV_SEC(r9)
--
2.13.3
^ permalink raw reply related
* Re: 5.2.7 kernel doesn't boot on G5
From: Christian Marillat @ 2019-08-16 14:52 UTC (permalink / raw)
To: Andreas Schwab; +Cc: Mathieu Malaterre, Christian Marillat, linuxppc-dev
In-Reply-To: <87ef1ljjct.fsf@igel.home>
On 16 août 2019 16:05, Andreas Schwab <schwab@linux-m68k.org> wrote:
> On Aug 16 2019, Christian Marillat <marillat@debian.org> wrote:
>
>> On 15 août 2019 19:50, christophe leroy <christophe.leroy@c-s.fr> wrote:
>>
>> [...]
>>
>>> Can you test with latest stable version, ie 5.2.8 ?
>>
>> Built from my G5 with make-kpkg and still doesn't boot :
>
> FWIW, 5.2.0 is working fine on my G5 (PowerMac7,3).
Mine is a PowerMac11,2 "Quadcore" and / is on a RAID0
As 4.19.5 boot I don't think is a hardware problem.
Christian
^ permalink raw reply
* RE: [PATCH 01/10] PCI: designware-ep: Add multiple PFs support for DWC
From: Xiaowei Bao @ 2019-08-16 15:11 UTC (permalink / raw)
To: Andrew Murray
Cc: mark.rutland@arm.com, Roy Zang, lorenzo.pieralisi@arm.com,
arnd@arndb.de, gregkh@linuxfoundation.org, jingoohan1@gmail.com,
Z.q. Hou, linuxppc-dev@lists.ozlabs.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
kishon@ti.com, M.h. Lian, devicetree@vger.kernel.org,
gustavo.pimentel@synopsys.com, Leo Li, shawnguo@kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190816123524.GE14111@e119886-lin.cambridge.arm.com>
> -----Original Message-----
> From: Andrew Murray <andrew.murray@arm.com>
> Sent: 2019年8月16日 20:35
> To: Xiaowei Bao <xiaowei.bao@nxp.com>
> Cc: jingoohan1@gmail.com; gustavo.pimentel@synopsys.com;
> mark.rutland@arm.com; shawnguo@kernel.org; Leo Li
> <leoyang.li@nxp.com>; kishon@ti.com; lorenzo.pieralisi@arm.com;
> arnd@arndb.de; gregkh@linuxfoundation.org; M.h. Lian
> <minghuan.lian@nxp.com>; Roy Zang <roy.zang@nxp.com>;
> linux-pci@vger.kernel.org; devicetree@vger.kernel.org;
> linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> linuxppc-dev@lists.ozlabs.org; Z.q. Hou <zhiqiang.hou@nxp.com>
> Subject: Re: [PATCH 01/10] PCI: designware-ep: Add multiple PFs support for
> DWC
>
> On Fri, Aug 16, 2019 at 11:00:01AM +0000, Xiaowei Bao wrote:
> >
> >
> > > -----Original Message-----
> > > From: Andrew Murray <andrew.murray@arm.com>
> > > Sent: 2019年8月16日 17:45
> > > To: Xiaowei Bao <xiaowei.bao@nxp.com>
> > > Cc: jingoohan1@gmail.com; gustavo.pimentel@synopsys.com;
> > > mark.rutland@arm.com; shawnguo@kernel.org; Leo Li
> > > <leoyang.li@nxp.com>; kishon@ti.com; lorenzo.pieralisi@arm.com;
> > > arnd@arndb.de; gregkh@linuxfoundation.org; M.h. Lian
> > > <minghuan.lian@nxp.com>; Roy Zang <roy.zang@nxp.com>;
> > > linux-pci@vger.kernel.org; devicetree@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> > > linuxppc-dev@lists.ozlabs.org; Z.q. Hou <zhiqiang.hou@nxp.com>
> > > Subject: Re: [PATCH 01/10] PCI: designware-ep: Add multiple PFs
> > > support for DWC
> > >
> > > On Fri, Aug 16, 2019 at 02:55:41AM +0000, Xiaowei Bao wrote:
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: Andrew Murray <andrew.murray@arm.com>
> > > > > Sent: 2019年8月15日 19:32
> > > > > To: Xiaowei Bao <xiaowei.bao@nxp.com>
> > > > > Cc: jingoohan1@gmail.com; gustavo.pimentel@synopsys.com;
> > > > > bhelgaas@google.com; robh+dt@kernel.org; mark.rutland@arm.com;
> > > > > shawnguo@kernel.org; Leo Li <leoyang.li@nxp.com>; kishon@ti.com;
> > > > > lorenzo.pieralisi@arm.com; arnd@arndb.de;
> > > > > gregkh@linuxfoundation.org; M.h. Lian <minghuan.lian@nxp.com>;
> > > > > Mingkai Hu <mingkai.hu@nxp.com>; Roy Zang <roy.zang@nxp.com>;
> > > > > linux-pci@vger.kernel.org; devicetree@vger.kernel.org;
> > > > > linux-kernel@vger.kernel.org;
> > > > > linux-arm-kernel@lists.infradead.org;
> > > > > linuxppc-dev@lists.ozlabs.org
> > > > > Subject: Re: [PATCH 01/10] PCI: designware-ep: Add multiple PFs
> > > > > support for DWC
> > > > >
> > > > > On Thu, Aug 15, 2019 at 04:37:07PM +0800, Xiaowei Bao wrote:
> > > > > > Add multiple PFs support for DWC, different PF have different
> > > > > > config space, we use pf-offset property which get from the DTS
> > > > > > to access the different pF config space.
> > > > >
> > > > > Thanks for the patch. I haven't seen a cover letter for this
> > > > > series, is there one missing?
> > > > Maybe I miss, I will add you to review next time, thanks a lot for
> > > > your
> > > comments.
> > > > >
> > > > >
> > > > > >
> > > > > > Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> > > > > > ---
> > > > > > drivers/pci/controller/dwc/pcie-designware-ep.c | 97
> > > > > +++++++++++++---------
> > > > > > drivers/pci/controller/dwc/pcie-designware.c | 105
> > > > > ++++++++++++++++++++++--
> > > > > > drivers/pci/controller/dwc/pcie-designware.h | 10 ++-
> > > > > > include/linux/pci-epc.h | 1 +
> > > > > > 4 files changed, 164 insertions(+), 49 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > > > b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > > > index 2bf5a35..75e2955 100644
> > > > > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > > > @@ -19,12 +19,14 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep
> > > *ep)
> > > > > > pci_epc_linkup(epc);
> > > > > > }
> > > > > >
> > > > > > -static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum
> > > > > > pci_barno
> > > > > bar,
> > > > > > - int flags)
> > > > > > +static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, u8
> func_no,
> > > > > > + enum pci_barno bar, int flags)
> > > > > > {
> > > > > > u32 reg;
> > > > > > + struct pci_epc *epc = pci->ep.epc;
> > > > > > + u32 pf_base = func_no * epc->pf_offset;
> > > > > >
> > > > > > - reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> > > > > > + reg = pf_base + PCI_BASE_ADDRESS_0 + (4 * bar);
> > > > >
> > > > > I think I'd rather see this arithmetic (and the one for
> > > > > determining
> > > > > pf_base) inside a macro or inline header function. This would
> > > > > make this code more readable and reduce the chances of an error
> > > > > by avoiding
> > > duplication of code.
> > > > >
> > > > > For example look at cdns_pcie_ep_fn_writeb and
> > > > > ROCKCHIP_PCIE_EP_FUNC_BASE for examples of other EP drivers that
> > > > > do this.
> > > > Agree, this looks fine, thanks a lot for your comments, I will use
> > > > this way to access the registers in next version patch.
> > > > >
> > > > >
> > > > > > dw_pcie_dbi_ro_wr_en(pci);
> > > > > > dw_pcie_writel_dbi2(pci, reg, 0x0);
> > > > > > dw_pcie_writel_dbi(pci, reg, 0x0); @@ -37,7 +39,12 @@ static
> > > > > > void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum
> > > > > > pci_barno bar,
> > > > > >
> > > > > > void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno
> > > > > > bar)
> > > {
> > > > > > - __dw_pcie_ep_reset_bar(pci, bar, 0);
> > > > > > + u8 func_no, funcs;
> > > > > > +
> > > > > > + funcs = pci->ep.epc->max_functions;
> > > > > > +
> > > > > > + for (func_no = 0; func_no < funcs; func_no++)
> > > > > > + __dw_pcie_ep_reset_bar(pci, func_no, bar, 0);
> > > > > > }
> > > > > >
> > > > > > static u8 __dw_pcie_ep_find_next_cap(struct dw_pcie *pci, u8
> > > > > > cap_ptr, @@ -78,28 +85,29 @@ static int
> > > > > > dw_pcie_ep_write_header(struct pci_epc *epc, u8 func_no, {
> > > > > > struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > > > > > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > > > + u32 pf_base = func_no * epc->pf_offset;
> > > > > >
> > > > > > dw_pcie_dbi_ro_wr_en(pci);
> > > > > > - dw_pcie_writew_dbi(pci, PCI_VENDOR_ID, hdr->vendorid);
> > > > > > - dw_pcie_writew_dbi(pci, PCI_DEVICE_ID, hdr->deviceid);
> > > > > > - dw_pcie_writeb_dbi(pci, PCI_REVISION_ID, hdr->revid);
> > > > > > - dw_pcie_writeb_dbi(pci, PCI_CLASS_PROG, hdr->progif_code);
> > > > > > - dw_pcie_writew_dbi(pci, PCI_CLASS_DEVICE,
> > > > > > + dw_pcie_writew_dbi(pci, pf_base + PCI_VENDOR_ID,
> > > hdr->vendorid);
> > > > > > + dw_pcie_writew_dbi(pci, pf_base + PCI_DEVICE_ID,
> hdr->deviceid);
> > > > > > + dw_pcie_writeb_dbi(pci, pf_base + PCI_REVISION_ID,
> hdr->revid);
> > > > > > + dw_pcie_writeb_dbi(pci, pf_base + PCI_CLASS_PROG,
> > > hdr->progif_code);
> > > > > > + dw_pcie_writew_dbi(pci, pf_base + PCI_CLASS_DEVICE,
> > > > > > hdr->subclass_code | hdr->baseclass_code << 8);
> > > > > > - dw_pcie_writeb_dbi(pci, PCI_CACHE_LINE_SIZE,
> > > > > > + dw_pcie_writeb_dbi(pci, pf_base + PCI_CACHE_LINE_SIZE,
> > > > > > hdr->cache_line_size);
> > > > > > - dw_pcie_writew_dbi(pci, PCI_SUBSYSTEM_VENDOR_ID,
> > > > > > + dw_pcie_writew_dbi(pci, pf_base +
> PCI_SUBSYSTEM_VENDOR_ID,
> > > > > > hdr->subsys_vendor_id);
> > > > > > - dw_pcie_writew_dbi(pci, PCI_SUBSYSTEM_ID,
> hdr->subsys_id);
> > > > > > - dw_pcie_writeb_dbi(pci, PCI_INTERRUPT_PIN,
> > > > > > + dw_pcie_writew_dbi(pci, pf_base + PCI_SUBSYSTEM_ID,
> > > > > hdr->subsys_id);
> > > > > > + dw_pcie_writeb_dbi(pci, pf_base + PCI_INTERRUPT_PIN,
> > > > > > hdr->interrupt_pin);
> > > > > > dw_pcie_dbi_ro_wr_dis(pci);
> > > > > >
> > > > > > return 0;
> > > > > > }
> > > > > >
> > > > > > -static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, enum
> > > > > pci_barno bar,
> > > > > > - dma_addr_t cpu_addr,
> > > > > > +static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8
> > > func_no,
> > > > > > + enum pci_barno bar, dma_addr_t cpu_addr,
> > > > > > enum dw_pcie_as_type as_type) {
> > > > > > int ret;
> > > > > > @@ -112,7 +120,7 @@ static int dw_pcie_ep_inbound_atu(struct
> > > > > dw_pcie_ep *ep, enum pci_barno bar,
> > > > > > return -EINVAL;
> > > > > > }
> > > > > >
> > > > > > - ret = dw_pcie_prog_inbound_atu(pci, free_win, bar, cpu_addr,
> > > > > > + ret = dw_pcie_prog_inbound_atu(pci, func_no, free_win, bar,
> > > > > > +cpu_addr,
> > > > > > as_type);
> > > > > > if (ret < 0) {
> > > > > > dev_err(pci->dev, "Failed to program IB window\n"); @@
> > > -125,7
> > > > > > +133,8 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep
> > > > > > +*ep,
> > > > > enum pci_barno bar,
> > > > > > return 0;
> > > > > > }
> > > > > >
> > > > > > -static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep,
> > > > > > phys_addr_t phys_addr,
> > > > > > +static int dw_pcie_ep_outbound_atu(struct dw_pcie_ep *ep, u8
> > > func_no,
> > > > > > + phys_addr_t phys_addr,
> > > > > > u64 pci_addr, size_t size) {
> > > > > > u32 free_win;
> > > > > > @@ -137,8 +146,8 @@ static int dw_pcie_ep_outbound_atu(struct
> > > > > dw_pcie_ep *ep, phys_addr_t phys_addr,
> > > > > > return -EINVAL;
> > > > > > }
> > > > > >
> > > > > > - dw_pcie_prog_outbound_atu(pci, free_win,
> PCIE_ATU_TYPE_MEM,
> > > > > > - phys_addr, pci_addr, size);
> > > > > > + dw_pcie_prog_ep_outbound_atu(pci, func_no, free_win,
> > > > > PCIE_ATU_TYPE_MEM,
> > > > > > + phys_addr, pci_addr, size);
> > > > > >
> > > > > > set_bit(free_win, ep->ob_window_map);
> > > > > > ep->outbound_addr[free_win] = phys_addr; @@ -154,7 +163,7
> > > @@
> > > > > static
> > > > > > void dw_pcie_ep_clear_bar(struct pci_epc *epc, u8 func_no,
> > > > > > enum pci_barno bar = epf_bar->barno;
> > > > > > u32 atu_index = ep->bar_to_atu[bar];
> > > > > >
> > > > > > - __dw_pcie_ep_reset_bar(pci, bar, epf_bar->flags);
> > > > > > + __dw_pcie_ep_reset_bar(pci, func_no, bar, epf_bar->flags);
> > > > > >
> > > > > > dw_pcie_disable_atu(pci, atu_index,
> > > DW_PCIE_REGION_INBOUND);
> > > > > > clear_bit(atu_index, ep->ib_window_map); @@ -170,14
> +179,16
> > > @@
> > > > > > static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no,
> > > > > > size_t size = epf_bar->size;
> > > > > > int flags = epf_bar->flags;
> > > > > > enum dw_pcie_as_type as_type;
> > > > > > - u32 reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> > > > > > + u32 pf_base = func_no * epc->pf_offset;
> > > > > > + u32 reg = PCI_BASE_ADDRESS_0 + (4 * bar) + pf_base;
> > > > > >
> > > > > > if (!(flags & PCI_BASE_ADDRESS_SPACE))
> > > > > > as_type = DW_PCIE_AS_MEM;
> > > > > > else
> > > > > > as_type = DW_PCIE_AS_IO;
> > > > > >
> > > > > > - ret = dw_pcie_ep_inbound_atu(ep, bar, epf_bar->phys_addr,
> > > as_type);
> > > > > > + ret = dw_pcie_ep_inbound_atu(ep, func_no, bar,
> > > > > > + epf_bar->phys_addr, as_type);
> > > > > > if (ret)
> > > > > > return ret;
> > > > > >
> > > > > > @@ -235,7 +246,7 @@ static int dw_pcie_ep_map_addr(struct
> > > > > > pci_epc
> > > > > *epc, u8 func_no,
> > > > > > struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > > > > > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > > >
> > > > > > - ret = dw_pcie_ep_outbound_atu(ep, addr, pci_addr, size);
> > > > > > + ret = dw_pcie_ep_outbound_atu(ep, func_no, addr, pci_addr,
> > > > > > +size);
> > > > > > if (ret) {
> > > > > > dev_err(pci->dev, "Failed to enable address\n");
> > > > > > return ret;
> > > > > > @@ -248,12 +259,13 @@ static int dw_pcie_ep_get_msi(struct
> > > > > > pci_epc *epc, u8 func_no) {
> > > > > > struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > > > > > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > > > + u32 pf_base = func_no * epc->pf_offset;
> > > > > > u32 val, reg;
> > > > > >
> > > > > > if (!ep->msi_cap)
> > > > > > return -EINVAL;
> > > > > >
> > > > > > - reg = ep->msi_cap + PCI_MSI_FLAGS;
> > > > > > + reg = ep->msi_cap + pf_base + PCI_MSI_FLAGS;
> > > > > > val = dw_pcie_readw_dbi(pci, reg);
> > > > > > if (!(val & PCI_MSI_FLAGS_ENABLE))
> > > > > > return -EINVAL;
> > > > > > @@ -267,12 +279,13 @@ static int dw_pcie_ep_set_msi(struct
> > > > > > pci_epc *epc, u8 func_no, u8 interrupts) {
> > > > > > struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > > > > > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > > > + u32 pf_base = func_no * epc->pf_offset;
> > > > > > u32 val, reg;
> > > > > >
> > > > > > if (!ep->msi_cap)
> > > > > > return -EINVAL;
> > > > > >
> > > > > > - reg = ep->msi_cap + PCI_MSI_FLAGS;
> > > > > > + reg = ep->msi_cap + pf_base + PCI_MSI_FLAGS;
> > > > > > val = dw_pcie_readw_dbi(pci, reg);
> > > > > > val &= ~PCI_MSI_FLAGS_QMASK;
> > > > > > val |= (interrupts << 1) & PCI_MSI_FLAGS_QMASK; @@
> -287,12
> > > > > > +300,13 @@ static int dw_pcie_ep_get_msix(struct pci_epc *epc,
> > > > > > +u8
> > > func_no) {
> > > > > > struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > > > > > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > > > + u32 pf_base = func_no * epc->pf_offset;
> > > > > > u32 val, reg;
> > > > > >
> > > > > > if (!ep->msix_cap)
> > > > > > return -EINVAL;
> > > > > >
> > > > > > - reg = ep->msix_cap + PCI_MSIX_FLAGS;
> > > > > > + reg = ep->msix_cap + pf_base + PCI_MSIX_FLAGS;
> > > > > > val = dw_pcie_readw_dbi(pci, reg);
> > > > > > if (!(val & PCI_MSIX_FLAGS_ENABLE))
> > > > > > return -EINVAL;
> > > > > > @@ -306,12 +320,13 @@ static int dw_pcie_ep_set_msix(struct
> > > > > > pci_epc *epc, u8 func_no, u16 interrupts) {
> > > > > > struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> > > > > > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > > > + u32 pf_base = func_no * epc->pf_offset;
> > > > > > u32 val, reg;
> > > > > >
> > > > > > if (!ep->msix_cap)
> > > > > > return -EINVAL;
> > > > > >
> > > > > > - reg = ep->msix_cap + PCI_MSIX_FLAGS;
> > > > > > + reg = ep->msix_cap + pf_base + PCI_MSIX_FLAGS;
> > > > > > val = dw_pcie_readw_dbi(pci, reg);
> > > > > > val &= ~PCI_MSIX_FLAGS_QSIZE;
> > > > > > val |= interrupts;
> > > > > > @@ -400,6 +415,7 @@ int dw_pcie_ep_raise_msi_irq(struct
> > > dw_pcie_ep
> > > > > *ep, u8 func_no,
> > > > > > unsigned int aligned_offset;
> > > > > > u16 msg_ctrl, msg_data;
> > > > > > u32 msg_addr_lower, msg_addr_upper, reg;
> > > > > > + u32 pf_base = func_no * epc->pf_offset;
> > > > > > u64 msg_addr;
> > > > > > bool has_upper;
> > > > > > int ret;
> > > > > > @@ -408,19 +424,19 @@ int dw_pcie_ep_raise_msi_irq(struct
> > > > > dw_pcie_ep *ep, u8 func_no,
> > > > > > return -EINVAL;
> > > > > >
> > > > > > /* Raise MSI per the PCI Local Bus Specification Revision 3.0,
> 6.8.1.
> > > */
> > > > > > - reg = ep->msi_cap + PCI_MSI_FLAGS;
> > > > > > + reg = ep->msi_cap + pf_base + PCI_MSI_FLAGS;
> > > > > > msg_ctrl = dw_pcie_readw_dbi(pci, reg);
> > > > > > has_upper = !!(msg_ctrl & PCI_MSI_FLAGS_64BIT);
> > > > > > - reg = ep->msi_cap + PCI_MSI_ADDRESS_LO;
> > > > > > + reg = ep->msi_cap + pf_base + PCI_MSI_ADDRESS_LO;
> > > > > > msg_addr_lower = dw_pcie_readl_dbi(pci, reg);
> > > > > > if (has_upper) {
> > > > > > - reg = ep->msi_cap + PCI_MSI_ADDRESS_HI;
> > > > > > + reg = ep->msi_cap + pf_base + PCI_MSI_ADDRESS_HI;
> > > > > > msg_addr_upper = dw_pcie_readl_dbi(pci, reg);
> > > > > > - reg = ep->msi_cap + PCI_MSI_DATA_64;
> > > > > > + reg = ep->msi_cap + pf_base + PCI_MSI_DATA_64;
> > > > > > msg_data = dw_pcie_readw_dbi(pci, reg);
> > > > > > } else {
> > > > > > msg_addr_upper = 0;
> > > > > > - reg = ep->msi_cap + PCI_MSI_DATA_32;
> > > > > > + reg = ep->msi_cap + pf_base + PCI_MSI_DATA_32;
> > > > > > msg_data = dw_pcie_readw_dbi(pci, reg);
> > > > > > }
> > > > > > aligned_offset = msg_addr_lower & (epc->mem->page_size -
> 1);
> > > @@
> > > > > > -439,7 +455,7 @@ int dw_pcie_ep_raise_msi_irq(struct
> > > > > > dw_pcie_ep *ep,
> > > > > > u8 func_no, }
> > > > > >
> > > > > > int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8
> func_no,
> > > > > > - u16 interrupt_num)
> > > > > > + u16 interrupt_num)
> > > > > > {
> > > > > > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > > > struct pci_epc *epc = ep->epc; @@ -447,16 +463,17 @@ int
> > > > > > dw_pcie_ep_raise_msix_irq(struct
> > > > > dw_pcie_ep *ep, u8 func_no,
> > > > > > u32 bar_addr_upper, bar_addr_lower;
> > > > > > u32 msg_addr_upper, msg_addr_lower;
> > > > > > u32 reg, msg_data, vec_ctrl;
> > > > > > + u32 pf_base = func_no * epc->pf_offset;
> > > > > > u64 tbl_addr, msg_addr, reg_u64;
> > > > > > void __iomem *msix_tbl;
> > > > > > int ret;
> > > > > >
> > > > > > - reg = ep->msix_cap + PCI_MSIX_TABLE;
> > > > > > + reg = ep->msix_cap + pf_base + PCI_MSIX_TABLE;
> > > > > > tbl_offset = dw_pcie_readl_dbi(pci, reg);
> > > > > > bir = (tbl_offset & PCI_MSIX_TABLE_BIR);
> > > > > > tbl_offset &= PCI_MSIX_TABLE_OFFSET;
> > > > > >
> > > > > > - reg = PCI_BASE_ADDRESS_0 + (4 * bir);
> > > > > > + reg = PCI_BASE_ADDRESS_0 + pf_base + (4 * bir);
> > > > > > bar_addr_upper = 0;
> > > > > > bar_addr_lower = dw_pcie_readl_dbi(pci, reg);
> > > > > > reg_u64 = (bar_addr_lower &
> > > PCI_BASE_ADDRESS_MEM_TYPE_MASK);
> > > > > @@
> > > > > > -592,13 +609,17 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> > > > > > ep->epc = epc;
> > > > > > epc_set_drvdata(epc, ep);
> > > > > >
> > > > > > - if (ep->ops->ep_init)
> > > > > > - ep->ops->ep_init(ep);
> > > > > > -
> > > > > > ret = of_property_read_u8(np, "max-functions",
> > > &epc->max_functions);
> > > > > > if (ret < 0)
> > > > > > epc->max_functions = 1;
> > > > > >
> > > > > > + ret = of_property_read_u32(np, "pf-offset", &epc->pf_offset);
> > > > > > + if (ret < 0)
> > > > > > + epc->pf_offset = 0;
> > > > >
> > > > > Bad things will likely happen if max_functions > 1 and pf-offset isn't
> set.
> > > > > I think the driver should bail in this situation. It would be
> > > > > very easy for someone to misconfigure this.
> > > > Yes, you are right, but if the max-functions have defined in DTS,
> > > > require the pf-offset must define in DTS, I am not sure the
> > > > correct value of pf-offsetfor other platforms, so I think the
> > > > max-functions and
> > > pf-offset should not have the dependence.
> > >
> > > Yes you're correct. I hadn't really thought about this beyond
> > > layerscape. It's also possible that other hardware could support
> > > multiple PFs without relying on an offset and perhaps employ some
> > > other mechanism to access different functions. So whilst this
> > > property can be optional for the majority of dwc controllers - it must be
> set and cannot be zero for layerscape.
> > >
> > > Perhaps inside ls_pcie_ep_init, you can set max_functions to 1 if
> > > pf_offset is
> > > 0 and print a WARN to explain why? (Or ls_pcie_ep_init returns
> > > failure and dw_pcie_ep_init checks it and bails).
> > >
> > > The assumption is being made here that future dw controllers may
> > > also use pf_offset (is this likely?) - otherwise why is this in
> > > pcie-designware-ep.c and not pci-layerscape-ep.c and why is this value
> not just hard-coded for lp?
> >
> > Thanks a lot for your detail comments, this give me a lot of help.
> > Yes, I agree your point, and I will seriously consider a best way to fix this
> potential issue.
> > Based on your experience, how do other platforms implement the multiple
> functions?
> > The DWC core difference the different PF by signal
> "client0_tlp_func_num[(PF_WD-1):0]"
>
> I don't know, though looking at the kernel drivers suggests that the existing EP
> controllers have a large address space which contains multiple PFs. They are
> accessed via macros (ROCKCHIP_PCIE_EP_FUNC_BASE(fn),
> CDNS_PCIE_EP_FUNC_BASE(fn)). It would be possible, but probably not
> desirable to have a smaller address space (window) and a register that selects
> which function the window refers to. This is why I'm slight nervous of
> assuming that a pf-offset will cover all future dw drivers - I may be wrong.
OK, thanks, maybe other people have good advice. I will use the macro to implement
the multiple function in v2 patch.
>
> > >
> > >
> > > > even though I didn't define pf-offset when I defined
> > > > max-functions, the pf-offset is 0, the DWC ep driver can continue
> > > > run the progress of INIT but not return, of course, thus the PF1
> > > > will not work, I don't know which
> > > way is better.
> > Hi Andrew,
> > > > >
> > > > >
> > > > > > +
> > > > > > + if (ep->ops->ep_init)
> > > > > > + ep->ops->ep_init(ep);
> > > > > > +
> > > > > > ret = __pci_epc_mem_init(epc, ep->phys_base, ep->addr_size,
> > > > > > ep->page_size);
> > > > > > if (ret < 0) {
> > > > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.c
> > > > > > b/drivers/pci/controller/dwc/pcie-designware.c
> > > > > > index 7d25102..c99cee4 100644
> > > > > > --- a/drivers/pci/controller/dwc/pcie-designware.c
> > > > > > +++ b/drivers/pci/controller/dwc/pcie-designware.c
> > > > > > @@ -158,6 +158,43 @@ static void
> > > > > > dw_pcie_writel_ob_unroll(struct
> > > > > dw_pcie *pci, u32 index, u32 reg,
> > > > > > dw_pcie_writel_atu(pci, offset + reg, val); }
> > > > > >
> > > > > > +static void dw_pcie_prog_ep_outbound_atu_unroll(struct
> > > > > > +dw_pcie *pci, u8
> > > > > func_no,
> > > > > > + int index, int type,
> > > > > > + u64 cpu_addr, u64 pci_addr,
> > > > > > + u32 size)
> > > > > > +{
> > > > > > + u32 retries, val;
> > > > > > +
> > > > > > + dw_pcie_writel_ob_unroll(pci, index,
> > > PCIE_ATU_UNR_LOWER_BASE,
> > > > > > + lower_32_bits(cpu_addr));
> > > > > > + dw_pcie_writel_ob_unroll(pci, index,
> > > PCIE_ATU_UNR_UPPER_BASE,
> > > > > > + upper_32_bits(cpu_addr));
> > > > > > + dw_pcie_writel_ob_unroll(pci, index, PCIE_ATU_UNR_LIMIT,
> > > > > > + lower_32_bits(cpu_addr + size - 1));
> > > > > > + dw_pcie_writel_ob_unroll(pci, index,
> > > PCIE_ATU_UNR_LOWER_TARGET,
> > > > > > + lower_32_bits(pci_addr));
> > > > > > + dw_pcie_writel_ob_unroll(pci, index,
> > > PCIE_ATU_UNR_UPPER_TARGET,
> > > > > > + upper_32_bits(pci_addr));
> > > > > > + dw_pcie_writel_ob_unroll(pci, index,
> > > PCIE_ATU_UNR_REGION_CTRL1,
> > > > > > + type | PCIE_ATU_FUNC_NUM(func_no));
> > > > >
> > > > > With the exception of this line, the rest of this function is
> > > > > identical to dw_pcie_prog_outbound_atu_unroll.
> > > > Yes, I can integrate the same code, but I think we'd better use
> > > > the different outbound window set function between RC and EP,
> > > > because the RC
> > > don't need the func_num parameter.
> > >
> > >
> > >
> > > > >
> > > > > > + dw_pcie_writel_ob_unroll(pci, index,
> > > PCIE_ATU_UNR_REGION_CTRL2,
> > > > > > + PCIE_ATU_ENABLE);
> > > > > > +
> > > > > > + /*
> > > > > > + * Make sure ATU enable takes effect before any subsequent
> config
> > > > > > + * and I/O accesses.
> > > > > > + */
> > > > > > + for (retries = 0; retries < LINK_WAIT_MAX_IATU_RETRIES;
> > > > > > +retries++)
> > > {
> > > > > > + val = dw_pcie_readl_ob_unroll(pci, index,
> > > > > > + PCIE_ATU_UNR_REGION_CTRL2);
> > > > > > + if (val & PCIE_ATU_ENABLE)
> > > > > > + return;
> > > > > > +
> > > > > > + mdelay(LINK_WAIT_IATU);
> > > > > > + }
> > > > > > + dev_err(pci->dev, "Outbound iATU is not being enabled\n"); }
> > > > > > +
> > > > > > static void dw_pcie_prog_outbound_atu_unroll(struct dw_pcie
> > > > > > *pci, int
> > > > > index,
> > > > > > int type, u64 cpu_addr,
> > > > > > u64 pci_addr, u32 size) @@ -194,6
> > > +231,51 @@ static
> > > > > > void
> > > > > dw_pcie_prog_outbound_atu_unroll(struct dw_pcie *pci, int index,
> > > > > > dev_err(pci->dev, "Outbound iATU is not being enabled\n");
> > > > > > }
> > > > > >
> > > > > > +void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8
> > > > > > +func_no, int
> > > > > index,
> > > > > > + int type, u64 cpu_addr, u64 pci_addr,
> > > > > > + u32 size)
> > > > > > +{
> > > > > > + u32 retries, val;
> > > > > > +
> > > > > > + if (pci->ops->cpu_addr_fixup)
> > > > > > + cpu_addr = pci->ops->cpu_addr_fixup(pci, cpu_addr);
> > > > > > +
> > > > > > + if (pci->iatu_unroll_enabled) {
> > > > > > + dw_pcie_prog_ep_outbound_atu_unroll(pci, func_no,
> index,
> > > type,
> > > > > > + cpu_addr, pci_addr, size);
> > > > > > + return;
> > > > > > + }
> > > > > > +
> > > > > > + dw_pcie_writel_dbi(pci, PCIE_ATU_VIEWPORT,
> > > > > > + PCIE_ATU_REGION_OUTBOUND | index);
> > > > > > + dw_pcie_writel_dbi(pci, PCIE_ATU_LOWER_BASE,
> > > > > > + lower_32_bits(cpu_addr));
> > > > > > + dw_pcie_writel_dbi(pci, PCIE_ATU_UPPER_BASE,
> > > > > > + upper_32_bits(cpu_addr));
> > > > > > + dw_pcie_writel_dbi(pci, PCIE_ATU_LIMIT,
> > > > > > + lower_32_bits(cpu_addr + size - 1));
> > > > > > + dw_pcie_writel_dbi(pci, PCIE_ATU_LOWER_TARGET,
> > > > > > + lower_32_bits(pci_addr));
> > > > > > + dw_pcie_writel_dbi(pci, PCIE_ATU_UPPER_TARGET,
> > > > > > + upper_32_bits(pci_addr));
> > > > > > + dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, type |
> > > > > > + PCIE_ATU_FUNC_NUM(func_no));
> > > > >
> > > > > The same here, this is identical to dw_pcie_prog_outbound_atu
> > > > > with the exception of this line.
> > > > >
> > > > > Is there a way you can avoid all of this duplicated code?
> > > > As above, I can integrate the same code, but I keep to think the
> > > > different outbound Window set function should be used between RC and
> EP.
> > >
> > > Or, is it possible to keep and use the existing functions, but use
> > > them differently, e.g:
> > >
> > >
> > > @@ -137,8 +146,8 @@ static int dw_pcie_ep_outbound_atu(struct
> > > dw_pcie_ep *ep, phys_addr_t phys_addr,
> > > return -EINVAL;
> > > }
> > >
> > > - dw_pcie_prog_outbound_atu(pci, free_win,
> PCIE_ATU_TYPE_MEM,
> > > - phys_addr, pci_addr, size);
> > > + dw_pcie_prog_outbound_atu(pci, free_win,
> > > PCIE_ATU_TYPE_MEM_FUNC(func_no),
> > > + phys_addr, pci_addr, size);
> > >
> > > set_bit(free_win, ep->ob_window_map);
> > > ep->outbound_addr[free_win] = phys_addr;
> > >
> > >
> > > Supported with:
> > >
> > > #define PCIE_ATU_TYPE_MEM 0x0
> > > #define PCIE_ATU_TYPE_MEM_FUNC(func_no) (PCIE_ATU_TYPE_MEM |
> > > PCIE_ATU_FUNC_NUM(func_no))
> > >
> > >
> > > This is just a suggestion, but I'm keen to avoid code duplication.
> > Thanks, I have think of a way as follow:
> >
> > This is a good way, but I think PCIE_ATU_TYPE_MEM_FUNC(func_no) will
> > give Someone confused meaning, because PCIE_ATU_TYPE_MEM indicate
> the
> > type of TLP, and the location in the bit[0:3] of register CR1, but the
> > PCIE_ATU_FUNC_NUM is bit[20:24], I have another way:
> > @@ -137,8 +146,8 @@ static int dw_pcie_ep_outbound_atu(struct
> > dw_pcie_ep *ep, phys_addr_t phys_addr,
> > return -EINVAL;
> > }
> >
> > dw_pcie_prog_outbound_atu(pci, free_win, PCIE_ATU_TYPE_MEM,
> > phys_addr, pci_addr, size);
> > + val = dw_pcie_readl_dbi(pci, PCIE_ATU_CR1);
> > + dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, val |
> > +PCIE_ATU_FUNC_NUM(func_no));
> > or
> > +void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int
> index,
> > + int type, u64 cpu_addr, u64
> pci_addr,
> > + u32 size) {
> > + dw_pcie_prog_outbound_atu(pci, index, type, cpu_addr, pci_addr,
> size);
> > + val = dw_pcie_readl_dbi(pci, PCIE_ATU_CR1);
> > + dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, val |
> > +PCIE_ATU_FUNC_NUM(func_no)); }
> >
> > Which do you think is better of these three ways?
>
> Building upon your idea, how about:
>
>
> @@ -137,8 +146,8 @@ static int dw_pcie_ep_outbound_atu(struct
> dw_pcie_ep *ep, phys_addr_t phys_addr,
> return -EINVAL;
> }
>
> - dw_pcie_prog_outbound_atu(pci, free_win,
> PCIE_ATU_TYPE_MEM,
> - phys_addr, pci_addr, size);
> + dw_pcie_prog_ep_outbound_atu(pci, func_no, free_win,
> PCIE_ATU_TYPE_MEM
> + phys_addr, pci_addr, size);
>
> set_bit(free_win, ep->ob_window_map);
> ep->outbound_addr[free_win] = phys_addr;
>
>
> +void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8 func_no, int
> index,
> + int type, u64 cpu_addr, u64
> pci_addr,
> + u32 size)
> +{
> + __dw_pcie_prog_outbound_atu(pci, func_no, index, type, cpu_addr,
> pci_addr, size);
> +}
> +
> +void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, u8 func_no, int
> index,
> + int type, u64 cpu_addr, u64
> pci_addr,
> + u32 size)
> +{
> + __dw_pcie_prog_outbound_atu(pci, 0, index, type, cpu_addr,
> pci_addr, size);
> +}
>
> In other words dw_pcie_prog_outbound_atu is updated (and renamed) to
> always take a func_no and for host controllers this is always set to zero. Or
> you could follow the approach taken in the cadence drivers for their
> implementation of cdns_pcie_set_outbound_region - this always takes a
> func_no and is used by host controller and endpoint drivers (except they don't
> have the helper wrapper functions above thus exposing fn=0 to host
> controllers).
You're correct, I think this way is better, thanks.
>
> > >
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Andrew Murray
> > > > >
> > > > > > + dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, PCIE_ATU_ENABLE);
> > > > > > +
> > > > > > + /*
> > > > > > + * Make sure ATU enable takes effect before any subsequent
> config
> > > > > > + * and I/O accesses.
> > > > > > + */
> > > > > > + for (retries = 0; retries < LINK_WAIT_MAX_IATU_RETRIES;
> > > > > > +retries++)
> > > {
> > > > > > + val = dw_pcie_readl_dbi(pci, PCIE_ATU_CR2);
> > > > > > + if (val & PCIE_ATU_ENABLE)
> > > > > > + return;
> > > > > > +
> > > > > > + mdelay(LINK_WAIT_IATU);
> > > > > > + }
> > > > > > + dev_err(pci->dev, "Outbound iATU is not being enabled\n"); }
> > > > > > +
> > > > > > void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int
> > > > > > index, int
> > > type,
> > > > > > u64 cpu_addr, u64 pci_addr, u32 size)
> { @@
> > > -252,8
> > > > > +334,8
> > > > > > @@ static void dw_pcie_writel_ib_unroll(struct dw_pcie *pci,
> > > > > > u32 index,
> > > > > u32 reg,
> > > > > > dw_pcie_writel_atu(pci, offset + reg, val); }
> > > > > >
> > > > > > -static int dw_pcie_prog_inbound_atu_unroll(struct dw_pcie
> > > > > > *pci, int
> > > index,
> > > > > > - int bar, u64 cpu_addr,
> > > > > > +static int dw_pcie_prog_inbound_atu_unroll(struct dw_pcie
> > > > > > +*pci,
> > > > > > +u8
> > > > > func_no,
> > > > > > + int index, int bar, u64 cpu_addr,
> > > > > > enum dw_pcie_as_type as_type) {
> > > > > > int type;
> > > > > > @@ -275,8 +357,10 @@ static int
> > > > > > dw_pcie_prog_inbound_atu_unroll(struct
> > > > > dw_pcie *pci, int index,
> > > > > > return -EINVAL;
> > > > > > }
> > > > > >
> > > > > > - dw_pcie_writel_ib_unroll(pci, index,
> > > PCIE_ATU_UNR_REGION_CTRL1,
> > > > > type);
> > > > > > + dw_pcie_writel_ib_unroll(pci, index,
> > > PCIE_ATU_UNR_REGION_CTRL1,
> > > > > type |
> > > > > > + PCIE_ATU_FUNC_NUM(func_no));
> > > > > > dw_pcie_writel_ib_unroll(pci, index,
> > > PCIE_ATU_UNR_REGION_CTRL2,
> > > > > > + PCIE_ATU_FUNC_NUM_MATCH_EN |
> > > > > > PCIE_ATU_ENABLE |
> > > > > > PCIE_ATU_BAR_MODE_ENABLE | (bar << 8));
> > > > > >
> > > > > > @@ -297,14 +381,15 @@ static int
> > > > > dw_pcie_prog_inbound_atu_unroll(struct dw_pcie *pci, int index,
> > > > > > return -EBUSY;
> > > > > > }
> > > > > >
> > > > > > -int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int
> bar,
> > > > > > - u64 cpu_addr, enum dw_pcie_as_type
> as_type)
> > > > > > +int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, u8 func_no,
> > > > > > +int
> > > index,
> > > > > > + int bar, u64 cpu_addr,
> > > > > > + enum dw_pcie_as_type as_type)
> > > > > > {
> > > > > > int type;
> > > > > > u32 retries, val;
> > > > > >
> > > > > > if (pci->iatu_unroll_enabled)
> > > > > > - return dw_pcie_prog_inbound_atu_unroll(pci, index, bar,
> > > > > > + return dw_pcie_prog_inbound_atu_unroll(pci, func_no,
> index,
> > > > > > +bar,
> > > > > > cpu_addr, as_type);
> > > > > >
> > > > > > dw_pcie_writel_dbi(pci, PCIE_ATU_VIEWPORT,
> > > > > PCIE_ATU_REGION_INBOUND |
> > > > > > @@ -323,9 +408,11 @@ int dw_pcie_prog_inbound_atu(struct
> > > > > > dw_pcie
> > > > > *pci, int index, int bar,
> > > > > > return -EINVAL;
> > > > > > }
> > > > > >
> > > > > > - dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, type);
> > > > > > - dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, PCIE_ATU_ENABLE
> > > > > > - | PCIE_ATU_BAR_MODE_ENABLE | (bar << 8));
> > > > > > + dw_pcie_writel_dbi(pci, PCIE_ATU_CR1, type |
> > > > > > + PCIE_ATU_FUNC_NUM(func_no));
> > > > > > + dw_pcie_writel_dbi(pci, PCIE_ATU_CR2, PCIE_ATU_ENABLE |
> > > > > > + PCIE_ATU_FUNC_NUM_MATCH_EN |
> > > > > > + PCIE_ATU_BAR_MODE_ENABLE | (bar << 8));
> > > > > >
> > > > > > /*
> > > > > > * Make sure ATU enable takes effect before any subsequent
> > > > > > config diff --git
> > > > > > a/drivers/pci/controller/dwc/pcie-designware.h
> > > > > > b/drivers/pci/controller/dwc/pcie-designware.h
> > > > > > index ffed084..2b291e8 100644
> > > > > > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > > > > > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > > > > > @@ -71,9 +71,11 @@
> > > > > > #define PCIE_ATU_TYPE_IO 0x2
> > > > > > #define PCIE_ATU_TYPE_CFG0 0x4
> > > > > > #define PCIE_ATU_TYPE_CFG1 0x5
> > > > > > +#define PCIE_ATU_FUNC_NUM(pf) (pf << 20)
> > > > > > #define PCIE_ATU_CR2 0x908
> > > > > > #define PCIE_ATU_ENABLE BIT(31)
> > > > > > #define PCIE_ATU_BAR_MODE_ENABLE BIT(30)
> > > > > > +#define PCIE_ATU_FUNC_NUM_MATCH_EN BIT(19)
> > > > > > #define PCIE_ATU_LOWER_BASE 0x90C
> > > > > > #define PCIE_ATU_UPPER_BASE 0x910
> > > > > > #define PCIE_ATU_LIMIT 0x914
> > > > > > @@ -265,8 +267,12 @@ int dw_pcie_wait_for_link(struct dw_pcie
> > > > > > *pci); void dw_pcie_prog_outbound_atu(struct dw_pcie *pci, int
> index,
> > > > > > int type, u64 cpu_addr, u64 pci_addr,
> > > > > > u32 size);
> > > > > > -int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, int index, int
> bar,
> > > > > > - u64 cpu_addr, enum dw_pcie_as_type
> as_type);
> > > > > > +void dw_pcie_prog_ep_outbound_atu(struct dw_pcie *pci, u8
> > > > > > +func_no, int
> > > > > index,
> > > > > > + int type, u64 cpu_addr, u64 pci_addr,
> > > > > > + u32 size);
> > > > > > +int dw_pcie_prog_inbound_atu(struct dw_pcie *pci, u8 func_no,
> > > > > > +int
> > > index,
> > > > > > + int bar, u64 cpu_addr,
> > > > > > + enum dw_pcie_as_type as_type);
> > > > > > void dw_pcie_disable_atu(struct dw_pcie *pci, int index,
> > > > > > enum dw_pcie_region_type type); void
> > > dw_pcie_setup(struct
> > > > > > dw_pcie *pci); diff --git a/include/linux/pci-epc.h
> > > > > > b/include/linux/pci-epc.h index f641bad..fc2feee 100644
> > > > > > --- a/include/linux/pci-epc.h
> > > > > > +++ b/include/linux/pci-epc.h
> > > > > > @@ -96,6 +96,7 @@ struct pci_epc {
> > > > > > const struct pci_epc_ops *ops;
> > > > > > struct pci_epc_mem *mem;
> > > > > > u8 max_functions;
> > > > > > + u32 pf_offset;
> > >
> > > Also pf_offset is an implementation detail needed only by the driver
> > > to calculate where the PF is - it doesn't seem right that we share
> > > this with the EP controller framework (whereas max_functions is used
> > > as a bounds check for func_no in the framework calls).
> > >
> > > I'd suggest that pf_offset is moved to a dwc structure, perhaps
> dw_pcie_ep?
> > I add the variable to this struct is consider that all PF is belong to
> > a PCIe controller, and the pci_epc indicate a PCIe controller, so I
> > add this variable to this struct, what do you think about this? I am not sure
> whether I should add this variable to dw_pcie_ep.
>
> The EPC framework won't use the pf_offset and doesn't need it. It abstracts
> the complexity of writing to the config address space (and similar) through the
> pci_epc_ops. I'd suggest that the EPC framework (and pci_epc struct) only
> needs to contain what *it* needs. Especially given that not all ep drivers have
> a pf_offset or similar.
>
> I understand the logic that pci_epc represents a EP controller, but I think you
> should consider that it actually represents a *generic* EP controller in the
> context of a framework which solely serves the purpose of connecting
> controllers with functions. Whereas the dw_pcie_ep represents a specific
> type of controller (DW) - as the pf_offset is (so far) relating to only DW
> controllers (and as confirmed by the DT mapping) then it makes sense to not
> move pf_offset from the specialised specific controller to the generic
> controller. (Or at least this is how I rationalise it, though the EPC framework is
> something quite unfamiliar to me).
I think this is more reasonable by your detail explaining, I will move pf_offset
to the struct dw_pcie_ep, thanks again!
>
> Thanks,
>
> Andrew Murray
>
> > >
> > > Thanks,
> > >
> > > Andrew Murray
> > >
> > > > > > struct config_group *group;
> > > > > > /* spinlock to protect against concurrent access of EP
> controller */
> > > > > > spinlock_t lock;
> > > > > > --
> > > > > > 2.9.5
> > > > > >
> > > > > >
> > > > > > _______________________________________________
> > > > > > linux-arm-kernel mailing list
> > > > > > linux-arm-kernel@lists.infradead.org
> > > > > > http://lists
> > > > > > .infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-kernel&dat
> > > > > > a=0
> > > 2
> > > > > > %
> > > > > 7C0
> > > > > >
> > > > >
> > >
> 1%7Cxiaowei.bao%40nxp.com%7C0e39168f6f144db6840308d721742040%7
> > > > > C686ea1d
> > > > > >
> > > > >
> > >
> 3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637014654998524452&sd
> > > > > ata=bP7eh
> > > > > > cjlGXCMVFE2b4f12Q6fGV7lQ%2F5i9qIi9FoPlbI%3D&reserved=0
^ permalink raw reply
* [Bug 204371] BUG kmalloc-4k (Tainted: G W ): Object padding overwritten
From: bugzilla-daemon @ 2019-08-16 15:20 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-204371-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=204371
--- Comment #35 from Christophe Leroy (christophe.leroy@c-s.fr) ---
Le 16/08/2019 à 16:38, bugzilla-daemon@bugzilla.kernel.org a écrit :
> https://bugzilla.kernel.org/show_bug.cgi?id=204371
>
> --- Comment #34 from Erhard F. (erhard_f@mailbox.org) ---
> On Fri, 16 Aug 2019 08:22:31 +0000
> bugzilla-daemon@bugzilla.kernel.org wrote:
>
>> https://bugzilla.kernel.org/show_bug.cgi?id=204371
>>
>> --- Comment #32 from Christophe Leroy (christophe.leroy@c-s.fr) ---
>> Then see if the WARNING on kfree() in btrfs_free_dummy_fs_info() is still
>> there.
> With latest changes there are no complaints of the kernel any longer. btrfs
> selftests pass, mounting and unmounting a btrfs partition works without any
> suspicious dmesg output.
>
That's good news. Will you handle submitting the patch to BTRFS file
system ?
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply
* [RFC PATCH] powerpc: Convert ____flush_dcache_icache_phys() to C
From: Christophe Leroy @ 2019-08-16 15:52 UTC (permalink / raw)
To: Alastair D'Silva; +Cc: linuxppc-dev, linux-kernel
Resulting code (8xx with 16 bytes per cacheline and 16k pages)
0000016c <__flush_dcache_icache_phys>:
16c: 54 63 00 22 rlwinm r3,r3,0,0,17
170: 7d 20 00 a6 mfmsr r9
174: 39 40 04 00 li r10,1024
178: 55 28 07 34 rlwinm r8,r9,0,28,26
17c: 7c 67 1b 78 mr r7,r3
180: 7d 49 03 a6 mtctr r10
184: 7d 00 01 24 mtmsr r8
188: 4c 00 01 2c isync
18c: 7c 00 18 6c dcbst 0,r3
190: 38 63 00 10 addi r3,r3,16
194: 42 00 ff f8 bdnz 18c <__flush_dcache_icache_phys+0x20>
198: 7c 00 04 ac hwsync
19c: 7d 49 03 a6 mtctr r10
1a0: 7c 00 3f ac icbi 0,r7
1a4: 38 e7 00 10 addi r7,r7,16
1a8: 42 00 ff f8 bdnz 1a0 <__flush_dcache_icache_phys+0x34>
1ac: 7c 00 04 ac hwsync
1b0: 7d 20 01 24 mtmsr r9
1b4: 4c 00 01 2c isync
1b8: 4e 80 00 20 blr
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
This patch is on top of Alastair's series "powerpc: convert cache asm to C"
Patch 3 of that series should touch __flush_dcache_icache_phys and this
patch could come just after patch 3.
arch/powerpc/include/asm/cacheflush.h | 8 +++++
arch/powerpc/mm/mem.c | 55 ++++++++++++++++++++++++++++-------
2 files changed, 53 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/include/asm/cacheflush.h b/arch/powerpc/include/asm/cacheflush.h
index 1826bf2cc137..bf4f2dc4eb76 100644
--- a/arch/powerpc/include/asm/cacheflush.h
+++ b/arch/powerpc/include/asm/cacheflush.h
@@ -47,6 +47,14 @@ void flush_icache_user_range(struct vm_area_struct *vma,
struct page *page, unsigned long addr,
int len);
void flush_dcache_icache_page(struct page *page);
+#if defined(CONFIG_PPC32) && !defined(CONFIG_BOOKE)
+void __flush_dcache_icache_phys(unsigned long physaddr);
+#else
+static inline void __flush_dcache_icache_phys(unsigned long physaddr)
+{
+ BUG();
+}
+#endif
/**
* flush_dcache_range(): Write any modified data cache blocks out to memory and invalidate them.
diff --git a/arch/powerpc/mm/mem.c b/arch/powerpc/mm/mem.c
index 43be99de7c9a..43009f9227c4 100644
--- a/arch/powerpc/mm/mem.c
+++ b/arch/powerpc/mm/mem.c
@@ -402,6 +402,50 @@ void flush_dcache_page(struct page *page)
}
EXPORT_SYMBOL(flush_dcache_page);
+#if defined(CONFIG_PPC32) && !defined(CONFIG_BOOKE)
+void __flush_dcache_icache_phys(unsigned long physaddr)
+{
+ unsigned long bytes = l1_dcache_bytes();
+ unsigned long nb = PAGE_SIZE / bytes;
+ unsigned long addr = physaddr & PAGE_MASK;
+ unsigned long msr, msr0;
+ unsigned long loop1 = addr, loop2 = addr;
+
+ if (cpu_has_feature(CPU_FTR_COHERENT_ICACHE)) {
+ /* For a snooping icache, we still need a dummy icbi to purge all the
+ * prefetched instructions from the ifetch buffers. We also need a sync
+ * before the icbi to order the the actual stores to memory that might
+ * have modified instructions with the icbi.
+ */
+ mb(); /* sync */
+ icbi((void *)addr);
+ mb(); /* sync */
+ isync();
+ return;
+ }
+ msr0 = mfmsr();
+ msr = msr0 & ~MSR_DR;
+ asm volatile(
+ " mtctr %2;"
+ " mtmsr %3;"
+ " isync;"
+ "0: dcbst 0, %0;"
+ " addi %0, %0, %4;"
+ " bdnz 0b;"
+ " sync;"
+ " mtctr %2;"
+ "1: icbi 0, %1;"
+ " addi %1, %1, %4;"
+ " bdnz 1b;"
+ " sync;"
+ " mtmsr %5;"
+ " isync;"
+ : "+r" (loop1), "+r" (loop2)
+ : "r" (nb), "r" (msr), "i" (bytes), "r" (msr0)
+ : "ctr", "memory");
+}
+#endif
+
void flush_dcache_icache_page(struct page *page)
{
#ifdef CONFIG_HUGETLB_PAGE
@@ -419,16 +463,7 @@ void flush_dcache_icache_page(struct page *page)
__flush_dcache_icache(start);
kunmap_atomic(start);
} else {
- unsigned long msr = mfmsr();
-
- /* Clear the DR bit so that we operate on physical
- * rather than virtual addresses
- */
- mtmsr(msr & ~(MSR_DR));
-
- __flush_dcache_icache((void *)physaddr);
-
- mtmsr(msr);
+ __flush_dcache_icache_phys(page_to_pfn(page) << PAGE_SHIFT);
}
#endif
}
--
2.13.3
^ permalink raw reply related
* Re: [PATCH 3/6] powerpc: Convert flush_icache_range & friends to C
From: Christophe Leroy @ 2019-08-16 15:57 UTC (permalink / raw)
To: Alastair D'Silva, alastair
Cc: Michal Hocko, Greg Kroah-Hartman, David Hildenbrand, linux-kernel,
Nicholas Piggin, Mike Rapoport, Qian Cai, Paul Mackerras,
Thomas Gleixner, linuxppc-dev, Andrew Morton, Allison Randal
In-Reply-To: <8a86bccf-ae4d-6d2c-72b1-db136cec9d10@c-s.fr>
Le 15/08/2019 à 09:29, christophe leroy a écrit :
>
>
> Le 15/08/2019 à 06:10, Alastair D'Silva a écrit :
>> From: Alastair D'Silva <alastair@d-silva.org>
>>
>> Similar to commit 22e9c88d486a
>> ("powerpc/64: reuse PPC32 static inline flush_dcache_range()")
>> this patch converts flush_icache_range() to C, and reimplements the
>> following functions as wrappers around it:
>> __flush_dcache_icache
>> __flush_dcache_icache_phys
>
> Not sure you can do that for __flush_dcache_icache_phys(), see detailed
> comments below
>
I just sent you an RFC patch that could be the way to convert
__flush_dcache_icache_phys() to C.
Feel free to modify it as wished and include it in your series.
Christophe
^ permalink raw reply
* [PATCH v6 0/5] powerpc/powernv/pci: Make hotplug self-sufficient, independent of FW and DT
From: Sergey Miroshnichenko @ 2019-08-16 16:16 UTC (permalink / raw)
To: linuxppc-dev
Cc: Sam Bobroff, Sergey Miroshnichenko, Oliver O'Halloran, linux
Allow switching from the pnv_php module to the standard pciehp driver for
PowerNV, if the platform supports it: it can be a server working on top of
the skiboot with the [1] patchset applied.
Add the ability to discover hot-added devices which weren't added to the
Device Tree (by the pnv_php via an explicit OPAL call when a hotplug event
was intercepted) by direct access to the bus.
Sync the changes in PCIe topology (bus numbers and PEs) with the skiboot.
Tested on POWER8 PowerNV+PHB3 ppc64le (our Vesnin server) with:
- the pciehp driver active;
- the pnv_php driver disabled;
- the "pci=pcie_bus_peer2peer,realloc" kernel command line argument;
- controlled hotplug of a network card with SR-IOV works;
- activating of SR-IOV on a network card works;
- [with extra patches for movable BARs and bus numbers] manually initiated
(via sysfs) rescan has found and turned on a hotplugged bridge.
[1] https://lists.ozlabs.org/pipermail/skiboot/2019-August/015140.html
[Skiboot] [PATCH v3 0/5] core/pci: Track changes of topology by an OS
Change since v5:
- Activates on "ibm,supported-movable-bdfs" property in DT from skiboot
instead of the "pci=realloc" flag;
- Removed the code refactoring patches - will send them separately.
Changes since v4:
- Fixed failing build when EEH is disabled in a kernel config;
- Unfreeze the bus on EEH_IO_ERROR_VALUE(size), not only 0xffffffff;
- Replaced the 0xff magic constant with phb->ioda.reserved_pe_idx;
- Renamed create_pdn() -> pci_create_pdn_from_dev();
- Renamed add_one_dev_pci_data(..., vf_index, ...) -> pci_alloc_pdn();
- Renamed add_dev_pci_data() -> pci_create_vf_pdns();
- Renamed remove_dev_pci_data() -> pci_destroy_vf_pdns();
- Removed the patch fixing uninitialized IOMMU group - now it is fixed in
commit 8f5b27347e88 ("powerpc/powernv/sriov: Register IOMMU groups for
VFs")
Changes since v3:
- Subject changed;
- Don't disable EEH during rescan anymore - instead just unfreeze the
target buses deliberately;
- Add synchronization with the firmware when changing the PCIe topology;
- Fixed for VFs;
- Code cleanup.
Changes since v2:
- Don't reassign bus numbers on PowerNV by default (to retain the default
behavior), but only when pci=realloc is passed;
- Less code affected;
- pci_add_device_node_info is refactored with add_one_dev_pci_data;
- Minor code cleanup.
Changes since v1:
- Fixed build for ppc64le and ppc64be when CONFIG_PCI_IOV is disabled;
- Fixed build for ppc64e when CONFIG_EEH is disabled;
- Fixed code style warnings.
Sergey Miroshnichenko (5):
powerpc/pci: Access PCI config space directly w/o pci_dn
powerpc/powernv/pci: Suppress an EEH error when reading an empty slot
powerpc/pci: Create pci_dn on demand
powerpc/powernv/pci: Hook up the writes to PCI_SECONDARY_BUS register
powerpc/pci: Enable assigning bus numbers instead of reading them from
DT
arch/powerpc/include/asm/ppc-pci.h | 1 +
arch/powerpc/kernel/pci_dn.c | 95 +++++++--
arch/powerpc/kernel/rtas_pci.c | 97 ++++++---
arch/powerpc/platforms/powernv/eeh-powernv.c | 2 +-
arch/powerpc/platforms/powernv/pci.c | 205 +++++++++++++++++--
5 files changed, 331 insertions(+), 69 deletions(-)
--
2.21.0
^ permalink raw reply
* [PATCH v6 1/5] powerpc/pci: Access PCI config space directly w/o pci_dn
From: Sergey Miroshnichenko @ 2019-08-16 16:16 UTC (permalink / raw)
To: linuxppc-dev
Cc: Sam Bobroff, Sergey Miroshnichenko, Oliver O'Halloran, linux
In-Reply-To: <20190816161614.32344-1-s.miroshnichenko@yadro.com>
To fetch an updated DT for the newly hotplugged device, OS must explicitly
request it from the firmware via the pnv_php driver.
If pnv_php wasn't triggered/loaded, it is still possible to discover new
devices if PCIe I/O will not stop in absence of the pci_dn structure.
Reviewed-by: Oliver O'Halloran <oohall@gmail.com>
Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
arch/powerpc/kernel/rtas_pci.c | 97 +++++++++++++++++++---------
arch/powerpc/platforms/powernv/pci.c | 64 ++++++++++++------
2 files changed, 109 insertions(+), 52 deletions(-)
diff --git a/arch/powerpc/kernel/rtas_pci.c b/arch/powerpc/kernel/rtas_pci.c
index ae5e43eaca48..912da28b3737 100644
--- a/arch/powerpc/kernel/rtas_pci.c
+++ b/arch/powerpc/kernel/rtas_pci.c
@@ -42,10 +42,26 @@ static inline int config_access_valid(struct pci_dn *dn, int where)
return 0;
}
-int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 *val)
+static int rtas_read_raw_config(unsigned long buid, int busno, unsigned int devfn,
+ int where, int size, u32 *val)
{
int returnval = -1;
- unsigned long buid, addr;
+ unsigned long addr = rtas_config_addr(busno, devfn, where);
+ int ret;
+
+ if (buid) {
+ ret = rtas_call(ibm_read_pci_config, 4, 2, &returnval,
+ addr, BUID_HI(buid), BUID_LO(buid), size);
+ } else {
+ ret = rtas_call(read_pci_config, 2, 2, &returnval, addr, size);
+ }
+ *val = returnval;
+
+ return ret;
+}
+
+int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 *val)
+{
int ret;
if (!pdn)
@@ -58,16 +74,8 @@ int rtas_read_config(struct pci_dn *pdn, int where, int size, u32 *val)
return PCIBIOS_SET_FAILED;
#endif
- addr = rtas_config_addr(pdn->busno, pdn->devfn, where);
- buid = pdn->phb->buid;
- if (buid) {
- ret = rtas_call(ibm_read_pci_config, 4, 2, &returnval,
- addr, BUID_HI(buid), BUID_LO(buid), size);
- } else {
- ret = rtas_call(read_pci_config, 2, 2, &returnval, addr, size);
- }
- *val = returnval;
-
+ ret = rtas_read_raw_config(pdn->phb->buid, pdn->busno, pdn->devfn,
+ where, size, val);
if (ret)
return PCIBIOS_DEVICE_NOT_FOUND;
@@ -85,18 +93,44 @@ static int rtas_pci_read_config(struct pci_bus *bus,
pdn = pci_get_pdn_by_devfn(bus, devfn);
- /* Validity of pdn is checked in here */
- ret = rtas_read_config(pdn, where, size, val);
- if (*val == EEH_IO_ERROR_VALUE(size) &&
- eeh_dev_check_failure(pdn_to_eeh_dev(pdn)))
- return PCIBIOS_DEVICE_NOT_FOUND;
+ if (pdn) {
+ /* Validity of pdn is checked in here */
+ ret = rtas_read_config(pdn, where, size, val);
+
+ if (*val == EEH_IO_ERROR_VALUE(size) &&
+ eeh_dev_check_failure(pdn_to_eeh_dev(pdn)))
+ ret = PCIBIOS_DEVICE_NOT_FOUND;
+ } else {
+ struct pci_controller *phb = pci_bus_to_host(bus);
+
+ ret = rtas_read_raw_config(phb->buid, bus->number, devfn,
+ where, size, val);
+ }
return ret;
}
+static int rtas_write_raw_config(unsigned long buid, int busno, unsigned int devfn,
+ int where, int size, u32 val)
+{
+ unsigned long addr = rtas_config_addr(busno, devfn, where);
+ int ret;
+
+ if (buid) {
+ ret = rtas_call(ibm_write_pci_config, 5, 1, NULL, addr,
+ BUID_HI(buid), BUID_LO(buid), size, (ulong)val);
+ } else {
+ ret = rtas_call(write_pci_config, 3, 1, NULL, addr, size, (ulong)val);
+ }
+
+ if (ret)
+ return PCIBIOS_DEVICE_NOT_FOUND;
+
+ return PCIBIOS_SUCCESSFUL;
+}
+
int rtas_write_config(struct pci_dn *pdn, int where, int size, u32 val)
{
- unsigned long buid, addr;
int ret;
if (!pdn)
@@ -109,15 +143,8 @@ int rtas_write_config(struct pci_dn *pdn, int where, int size, u32 val)
return PCIBIOS_SET_FAILED;
#endif
- addr = rtas_config_addr(pdn->busno, pdn->devfn, where);
- buid = pdn->phb->buid;
- if (buid) {
- ret = rtas_call(ibm_write_pci_config, 5, 1, NULL, addr,
- BUID_HI(buid), BUID_LO(buid), size, (ulong) val);
- } else {
- ret = rtas_call(write_pci_config, 3, 1, NULL, addr, size, (ulong)val);
- }
-
+ ret = rtas_write_raw_config(pdn->phb->buid, pdn->busno, pdn->devfn,
+ where, size, val);
if (ret)
return PCIBIOS_DEVICE_NOT_FOUND;
@@ -128,12 +155,20 @@ static int rtas_pci_write_config(struct pci_bus *bus,
unsigned int devfn,
int where, int size, u32 val)
{
- struct pci_dn *pdn;
+ struct pci_dn *pdn = pci_get_pdn_by_devfn(bus, devfn);
+ int ret;
- pdn = pci_get_pdn_by_devfn(bus, devfn);
+ if (pdn) {
+ /* Validity of pdn is checked in here. */
+ ret = rtas_write_config(pdn, where, size, val);
+ } else {
+ struct pci_controller *phb = pci_bus_to_host(bus);
- /* Validity of pdn is checked in here. */
- return rtas_write_config(pdn, where, size, val);
+ ret = rtas_write_raw_config(phb->buid, bus->number, devfn,
+ where, size, val);
+ }
+
+ return ret;
}
static struct pci_ops rtas_pci_ops = {
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index 6104418c9ad5..8d6c094f074e 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -647,30 +647,29 @@ static void pnv_pci_config_check_eeh(struct pci_dn *pdn)
}
}
-int pnv_pci_cfg_read(struct pci_dn *pdn,
- int where, int size, u32 *val)
+static int pnv_pci_cfg_read_raw(u64 phb_id, int busno, unsigned int devfn,
+ int where, int size, u32 *val)
{
- struct pnv_phb *phb = pdn->phb->private_data;
- u32 bdfn = (pdn->busno << 8) | pdn->devfn;
+ u32 bdfn = (busno << 8) | devfn;
s64 rc;
switch (size) {
case 1: {
u8 v8;
- rc = opal_pci_config_read_byte(phb->opal_id, bdfn, where, &v8);
+ rc = opal_pci_config_read_byte(phb_id, bdfn, where, &v8);
*val = (rc == OPAL_SUCCESS) ? v8 : 0xff;
break;
}
case 2: {
__be16 v16;
- rc = opal_pci_config_read_half_word(phb->opal_id, bdfn, where,
- &v16);
+ rc = opal_pci_config_read_half_word(phb_id, bdfn, where,
+ &v16);
*val = (rc == OPAL_SUCCESS) ? be16_to_cpu(v16) : 0xffff;
break;
}
case 4: {
__be32 v32;
- rc = opal_pci_config_read_word(phb->opal_id, bdfn, where, &v32);
+ rc = opal_pci_config_read_word(phb_id, bdfn, where, &v32);
*val = (rc == OPAL_SUCCESS) ? be32_to_cpu(v32) : 0xffffffff;
break;
}
@@ -679,27 +678,28 @@ int pnv_pci_cfg_read(struct pci_dn *pdn,
}
pr_devel("%s: bus: %x devfn: %x +%x/%x -> %08x\n",
- __func__, pdn->busno, pdn->devfn, where, size, *val);
+ __func__, busno, devfn, where, size, *val);
+
return PCIBIOS_SUCCESSFUL;
}
-int pnv_pci_cfg_write(struct pci_dn *pdn,
- int where, int size, u32 val)
+static int pnv_pci_cfg_write_raw(u64 phb_id, int busno, unsigned int devfn,
+ int where, int size, u32 val)
{
- struct pnv_phb *phb = pdn->phb->private_data;
- u32 bdfn = (pdn->busno << 8) | pdn->devfn;
+ u32 bdfn = (busno << 8) | devfn;
pr_devel("%s: bus: %x devfn: %x +%x/%x -> %08x\n",
- __func__, pdn->busno, pdn->devfn, where, size, val);
+ __func__, busno, devfn, where, size, val);
+
switch (size) {
case 1:
- opal_pci_config_write_byte(phb->opal_id, bdfn, where, val);
+ opal_pci_config_write_byte(phb_id, bdfn, where, val);
break;
case 2:
- opal_pci_config_write_half_word(phb->opal_id, bdfn, where, val);
+ opal_pci_config_write_half_word(phb_id, bdfn, where, val);
break;
case 4:
- opal_pci_config_write_word(phb->opal_id, bdfn, where, val);
+ opal_pci_config_write_word(phb_id, bdfn, where, val);
break;
default:
return PCIBIOS_FUNC_NOT_SUPPORTED;
@@ -708,6 +708,24 @@ int pnv_pci_cfg_write(struct pci_dn *pdn,
return PCIBIOS_SUCCESSFUL;
}
+int pnv_pci_cfg_read(struct pci_dn *pdn,
+ int where, int size, u32 *val)
+{
+ struct pnv_phb *phb = pdn->phb->private_data;
+
+ return pnv_pci_cfg_read_raw(phb->opal_id, pdn->busno, pdn->devfn,
+ where, size, val);
+}
+
+int pnv_pci_cfg_write(struct pci_dn *pdn,
+ int where, int size, u32 val)
+{
+ struct pnv_phb *phb = pdn->phb->private_data;
+
+ return pnv_pci_cfg_write_raw(phb->opal_id, pdn->busno, pdn->devfn,
+ where, size, val);
+}
+
#if CONFIG_EEH
static bool pnv_pci_cfg_check(struct pci_dn *pdn)
{
@@ -743,13 +761,15 @@ static int pnv_pci_read_config(struct pci_bus *bus,
int where, int size, u32 *val)
{
struct pci_dn *pdn;
- struct pnv_phb *phb;
+ struct pci_controller *hose = pci_bus_to_host(bus);
+ struct pnv_phb *phb = hose->private_data;
int ret;
*val = 0xFFFFFFFF;
pdn = pci_get_pdn_by_devfn(bus, devfn);
if (!pdn)
- return PCIBIOS_DEVICE_NOT_FOUND;
+ return pnv_pci_cfg_read_raw(phb->opal_id, bus->number, devfn,
+ where, size, val);
if (!pnv_pci_cfg_check(pdn))
return PCIBIOS_DEVICE_NOT_FOUND;
@@ -772,12 +792,14 @@ static int pnv_pci_write_config(struct pci_bus *bus,
int where, int size, u32 val)
{
struct pci_dn *pdn;
- struct pnv_phb *phb;
+ struct pci_controller *hose = pci_bus_to_host(bus);
+ struct pnv_phb *phb = hose->private_data;
int ret;
pdn = pci_get_pdn_by_devfn(bus, devfn);
if (!pdn)
- return PCIBIOS_DEVICE_NOT_FOUND;
+ return pnv_pci_cfg_write_raw(phb->opal_id, bus->number, devfn,
+ where, size, val);
if (!pnv_pci_cfg_check(pdn))
return PCIBIOS_DEVICE_NOT_FOUND;
--
2.21.0
^ permalink raw reply related
* [PATCH v6 2/5] powerpc/powernv/pci: Suppress an EEH error when reading an empty slot
From: Sergey Miroshnichenko @ 2019-08-16 16:16 UTC (permalink / raw)
To: linuxppc-dev
Cc: Sam Bobroff, Sergey Miroshnichenko, Oliver O'Halloran, linux
In-Reply-To: <20190816161614.32344-1-s.miroshnichenko@yadro.com>
Reading an empty slot returns all ones, which triggers a false
EEH error event on PowerNV. This patch unfreezes the bus where
it has happened.
Reviewed-by: Oliver O'Halloran <oohall@gmail.com>
Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
arch/powerpc/include/asm/ppc-pci.h | 1 +
arch/powerpc/kernel/pci_dn.c | 2 +-
arch/powerpc/platforms/powernv/pci.c | 31 +++++++++++++++++++++++++---
3 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/include/asm/ppc-pci.h b/arch/powerpc/include/asm/ppc-pci.h
index cec2d6409515..8b51c8577b94 100644
--- a/arch/powerpc/include/asm/ppc-pci.h
+++ b/arch/powerpc/include/asm/ppc-pci.h
@@ -36,6 +36,7 @@ void *traverse_pci_dn(struct pci_dn *root,
void *(*fn)(struct pci_dn *, void *),
void *data);
extern void pci_devs_phb_init_dynamic(struct pci_controller *phb);
+struct pci_dn *pci_bus_to_pdn(struct pci_bus *bus);
/* From rtas_pci.h */
extern void init_pci_config_tokens (void);
diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index c4c8c237a106..e1a0ab2caafe 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -27,7 +27,7 @@
* one of PF's bridge. For other devices, their firmware
* data is linked to that of their bridge.
*/
-static struct pci_dn *pci_bus_to_pdn(struct pci_bus *bus)
+struct pci_dn *pci_bus_to_pdn(struct pci_bus *bus)
{
struct pci_bus *pbus;
struct device_node *dn;
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index 8d6c094f074e..a5b04410c8b4 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -756,6 +756,21 @@ static inline pnv_pci_cfg_check(struct pci_dn *pdn)
}
#endif /* CONFIG_EEH */
+static int get_bus_pe_number(struct pci_bus *bus)
+{
+ struct pci_dn *pdn = pci_bus_to_pdn(bus);
+ struct pci_dn *child;
+
+ if (!pdn)
+ return IODA_INVALID_PE;
+
+ list_for_each_entry(child, &pdn->child_list, list)
+ if (child->pe_number != IODA_INVALID_PE)
+ return child->pe_number;
+
+ return IODA_INVALID_PE;
+}
+
static int pnv_pci_read_config(struct pci_bus *bus,
unsigned int devfn,
int where, int size, u32 *val)
@@ -767,9 +782,19 @@ static int pnv_pci_read_config(struct pci_bus *bus,
*val = 0xFFFFFFFF;
pdn = pci_get_pdn_by_devfn(bus, devfn);
- if (!pdn)
- return pnv_pci_cfg_read_raw(phb->opal_id, bus->number, devfn,
- where, size, val);
+ if (!pdn) {
+ int pe_number = get_bus_pe_number(bus);
+
+ ret = pnv_pci_cfg_read_raw(phb->opal_id, bus->number, devfn,
+ where, size, val);
+
+ if (!ret && (*val == EEH_IO_ERROR_VALUE(size)) && phb->unfreeze_pe)
+ phb->unfreeze_pe(phb, (pe_number == IODA_INVALID_PE) ?
+ phb->ioda.reserved_pe_idx : pe_number,
+ OPAL_EEH_ACTION_CLEAR_FREEZE_ALL);
+
+ return ret;
+ }
if (!pnv_pci_cfg_check(pdn))
return PCIBIOS_DEVICE_NOT_FOUND;
--
2.21.0
^ permalink raw reply related
* [PATCH v6 3/5] powerpc/pci: Create pci_dn on demand
From: Sergey Miroshnichenko @ 2019-08-16 16:16 UTC (permalink / raw)
To: linuxppc-dev
Cc: Sam Bobroff, Sergey Miroshnichenko, Oliver O'Halloran, linux
In-Reply-To: <20190816161614.32344-1-s.miroshnichenko@yadro.com>
If a struct pci_dn hasn't yet been created for the PCIe device (there was
no DT node for it), allocate this structure and fill with info read from
the device directly.
Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
arch/powerpc/kernel/pci_dn.c | 88 ++++++++++++++++++++++++++++++------
1 file changed, 74 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index e1a0ab2caafe..261d61460eac 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -20,6 +20,9 @@
#include <asm/firmware.h>
#include <asm/eeh.h>
+static struct pci_dn *pci_create_pdn_from_dev(struct pci_dev *pdev,
+ struct pci_dn *parent);
+
/*
* The function is used to find the firmware data of one
* specific PCI device, which is attached to the indicated
@@ -52,6 +55,9 @@ struct pci_dn *pci_bus_to_pdn(struct pci_bus *bus)
dn = pci_bus_to_OF_node(pbus);
pdn = dn ? PCI_DN(dn) : NULL;
+ if (!pdn && pbus->self)
+ pdn = pbus->self->dev.archdata.pci_data;
+
return pdn;
}
@@ -61,10 +67,13 @@ struct pci_dn *pci_get_pdn_by_devfn(struct pci_bus *bus,
struct device_node *dn = NULL;
struct pci_dn *parent, *pdn;
struct pci_dev *pdev = NULL;
+ bool pdev_found = false;
/* Fast path: fetch from PCI device */
list_for_each_entry(pdev, &bus->devices, bus_list) {
if (pdev->devfn == devfn) {
+ pdev_found = true;
+
if (pdev->dev.archdata.pci_data)
return pdev->dev.archdata.pci_data;
@@ -73,6 +82,9 @@ struct pci_dn *pci_get_pdn_by_devfn(struct pci_bus *bus,
}
}
+ if (!pdev_found)
+ pdev = NULL;
+
/* Fast path: fetch from device node */
pdn = dn ? PCI_DN(dn) : NULL;
if (pdn)
@@ -85,9 +97,12 @@ struct pci_dn *pci_get_pdn_by_devfn(struct pci_bus *bus,
list_for_each_entry(pdn, &parent->child_list, list) {
if (pdn->busno == bus->number &&
- pdn->devfn == devfn)
- return pdn;
- }
+ pdn->devfn == devfn) {
+ if (pdev)
+ pdev->dev.archdata.pci_data = pdn;
+ return pdn;
+ }
+ }
return NULL;
}
@@ -117,17 +132,17 @@ struct pci_dn *pci_get_pdn(struct pci_dev *pdev)
list_for_each_entry(pdn, &parent->child_list, list) {
if (pdn->busno == pdev->bus->number &&
- pdn->devfn == pdev->devfn)
+ pdn->devfn == pdev->devfn) {
+ pdev->dev.archdata.pci_data = pdn;
return pdn;
+ }
}
- return NULL;
+ return pci_create_pdn_from_dev(pdev, parent);
}
-#ifdef CONFIG_PCI_IOV
-static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
- int vf_index,
- int busno, int devfn)
+static struct pci_dn *pci_alloc_pdn(struct pci_dn *parent,
+ int busno, int devfn)
{
struct pci_dn *pdn;
@@ -143,7 +158,6 @@ static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
pdn->parent = parent;
pdn->busno = busno;
pdn->devfn = devfn;
- pdn->vf_index = vf_index;
pdn->pe_number = IODA_INVALID_PE;
INIT_LIST_HEAD(&pdn->child_list);
INIT_LIST_HEAD(&pdn->list);
@@ -151,7 +165,51 @@ static struct pci_dn *add_one_dev_pci_data(struct pci_dn *parent,
return pdn;
}
-#endif
+
+static struct pci_dn *pci_create_pdn_from_dev(struct pci_dev *pdev,
+ struct pci_dn *parent)
+{
+ struct pci_dn *pdn = NULL;
+ u32 class_code;
+ u16 device_id;
+ u16 vendor_id;
+
+ if (!parent)
+ return NULL;
+
+ pdn = pci_alloc_pdn(parent, pdev->bus->busn_res.start, pdev->devfn);
+ pci_info(pdev, "Create a new pdn for devfn %2x\n", pdev->devfn / 8);
+
+ if (!pdn) {
+ pci_err(pdev, "%s: Failed to allocate pdn\n", __func__);
+ return NULL;
+ }
+
+ #ifdef CONFIG_EEH
+ if (!eeh_dev_init(pdn)) {
+ kfree(pdn);
+ pci_err(pdev, "%s: Failed to allocate edev\n", __func__);
+ return NULL;
+ }
+ #endif /* CONFIG_EEH */
+
+ pci_bus_read_config_word(pdev->bus, pdev->devfn,
+ PCI_VENDOR_ID, &vendor_id);
+ pdn->vendor_id = vendor_id;
+
+ pci_bus_read_config_word(pdev->bus, pdev->devfn,
+ PCI_DEVICE_ID, &device_id);
+ pdn->device_id = device_id;
+
+ pci_bus_read_config_dword(pdev->bus, pdev->devfn,
+ PCI_CLASS_REVISION, &class_code);
+ class_code >>= 8;
+ pdn->class_code = class_code;
+
+ pdev->dev.archdata.pci_data = pdn;
+
+ return pdn;
+}
struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
{
@@ -176,15 +234,17 @@ struct pci_dn *add_dev_pci_data(struct pci_dev *pdev)
for (i = 0; i < pci_sriov_get_totalvfs(pdev); i++) {
struct eeh_dev *edev __maybe_unused;
- pdn = add_one_dev_pci_data(parent, i,
- pci_iov_virtfn_bus(pdev, i),
- pci_iov_virtfn_devfn(pdev, i));
+ pdn = pci_alloc_pdn(parent,
+ pci_iov_virtfn_bus(pdev, i),
+ pci_iov_virtfn_devfn(pdev, i));
if (!pdn) {
dev_warn(&pdev->dev, "%s: Cannot create firmware data for VF#%d\n",
__func__, i);
return NULL;
}
+ pdn->vf_index = i;
+
#ifdef CONFIG_EEH
/* Create the EEH device for the VF */
edev = eeh_dev_init(pdn);
--
2.21.0
^ permalink raw reply related
* [PATCH v6 4/5] powerpc/powernv/pci: Hook up the writes to PCI_SECONDARY_BUS register
From: Sergey Miroshnichenko @ 2019-08-16 16:16 UTC (permalink / raw)
To: linuxppc-dev
Cc: Sam Bobroff, Sergey Miroshnichenko, Oliver O'Halloran, linux
In-Reply-To: <20190816161614.32344-1-s.miroshnichenko@yadro.com>
Writing a new value to the PCI_SECONDARY_BUS register of the bridge means
that its children will become addressable on another address (new B in BDF)
or even un-addressable if the secondary bus is set to zero.
On PowerNV, device PEs are heavily BDF-dependent, so they must be updated
on every such change of its address.
Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
arch/powerpc/platforms/powernv/pci.c | 118 ++++++++++++++++++++++++++-
1 file changed, 116 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/pci.c b/arch/powerpc/platforms/powernv/pci.c
index a5b04410c8b4..e9b4ed0f97a3 100644
--- a/arch/powerpc/platforms/powernv/pci.c
+++ b/arch/powerpc/platforms/powernv/pci.c
@@ -717,13 +717,127 @@ int pnv_pci_cfg_read(struct pci_dn *pdn,
where, size, val);
}
+static void invalidate_children_pes(struct pci_dn *pdn)
+{
+ struct pnv_phb *phb = pdn->phb->private_data;
+ struct pci_dn *child;
+ bool found_pe = false;
+ int pe_num;
+ int pe_bus;
+
+ list_for_each_entry(child, &pdn->child_list, list) {
+ struct pnv_ioda_pe *pe = (child->pe_number != IODA_INVALID_PE) ?
+ &phb->ioda.pe_array[child->pe_number] :
+ NULL;
+
+ if (!child->busno)
+ continue;
+
+ if ((child->class_code >> 8) == PCI_CLASS_BRIDGE_PCI)
+ invalidate_children_pes(child);
+
+ if (pe) {
+ u8 rid_bus = (pe->rid >> 8) & 0xff;
+
+ if (rid_bus) {
+ pe_num = child->pe_number;
+ pe_bus = rid_bus;
+ found_pe = true;
+ }
+
+ pe->rid &= 0xff;
+ }
+
+ child->busno = 0;
+ }
+
+ if (found_pe) {
+ u16 rid = pe_bus << 8;
+
+ opal_pci_set_pe(phb->opal_id, pe_num, rid, 7, 0, 0, OPAL_UNMAP_PE);
+ }
+}
+
+static u8 pre_hook_new_sec_bus(struct pci_dn *pdn, u8 new_secondary_bus)
+{
+ u32 old_secondary_bus = 0;
+
+ if ((pdn->class_code >> 8) != PCI_CLASS_BRIDGE_PCI)
+ return 0;
+
+ pnv_pci_cfg_read(pdn, PCI_SECONDARY_BUS, 1, &old_secondary_bus);
+ old_secondary_bus &= 0xff;
+
+ if (old_secondary_bus != new_secondary_bus)
+ invalidate_children_pes(pdn);
+
+ return old_secondary_bus;
+}
+
+static void update_children_pes(struct pci_dn *pdn, u8 new_secondary_bus)
+{
+ struct pnv_phb *phb = pdn->phb->private_data;
+ struct pci_dn *child;
+ bool found_pe = false;
+ int pe_num;
+
+ if (!new_secondary_bus)
+ return;
+
+ list_for_each_entry(child, &pdn->child_list, list) {
+ struct pnv_ioda_pe *pe = (child->pe_number != IODA_INVALID_PE) ?
+ &phb->ioda.pe_array[child->pe_number] :
+ NULL;
+
+ if (child->busno)
+ continue;
+
+ child->busno = new_secondary_bus;
+
+ if (pe) {
+ pe->rid |= (child->busno << 8);
+ pe_num = child->pe_number;
+ found_pe = true;
+ }
+ }
+
+ if (found_pe) {
+ u16 rid = new_secondary_bus << 8;
+
+ opal_pci_set_pe(phb->opal_id, pe_num, rid, 7, 0, 0, OPAL_MAP_PE);
+ }
+}
+
+static void post_hook_new_sec_bus(struct pci_dn *pdn, u8 new_secondary_bus)
+{
+ if ((pdn->class_code >> 8) != PCI_CLASS_BRIDGE_PCI)
+ return;
+
+ update_children_pes(pdn, new_secondary_bus);
+}
+
int pnv_pci_cfg_write(struct pci_dn *pdn,
int where, int size, u32 val)
{
struct pnv_phb *phb = pdn->phb->private_data;
+ u8 old_secondary_bus = 0, new_secondary_bus = 0;
+ int rc;
+
+ if (where == PCI_SECONDARY_BUS) {
+ new_secondary_bus = val & 0xff;
+ old_secondary_bus = pre_hook_new_sec_bus(pdn, new_secondary_bus);
+ } else if (where == PCI_PRIMARY_BUS && size > 1) {
+ new_secondary_bus = (val >> 8) & 0xff;
+ old_secondary_bus = pre_hook_new_sec_bus(pdn, new_secondary_bus);
+ }
- return pnv_pci_cfg_write_raw(phb->opal_id, pdn->busno, pdn->devfn,
- where, size, val);
+ rc = pnv_pci_cfg_write_raw(phb->opal_id, pdn->busno, pdn->devfn,
+ where, size, val);
+
+ if (new_secondary_bus && old_secondary_bus != new_secondary_bus)
+ post_hook_new_sec_bus(pdn, new_secondary_bus);
+
+ return rc;
}
#if CONFIG_EEH
--
2.21.0
^ permalink raw reply related
* [PATCH v6 5/5] powerpc/pci: Enable assigning bus numbers instead of reading them from DT
From: Sergey Miroshnichenko @ 2019-08-16 16:16 UTC (permalink / raw)
To: linuxppc-dev
Cc: Sam Bobroff, Sergey Miroshnichenko, Oliver O'Halloran, linux
In-Reply-To: <20190816161614.32344-1-s.miroshnichenko@yadro.com>
If the firmware indicates support of reassigning bus numbers via the PHB's
"ibm,supported-movable-bdfs" property in DT, PowerNV will not depend on PCI
topology info from DT anymore.
This makes possible to re-enumerate the fabric, assign the new bus numbers
and switch from the pnv_php module to the standard pciehp driver for PCI
hotplug functionality.
Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
arch/powerpc/kernel/pci_dn.c | 5 +++++
arch/powerpc/platforms/powernv/eeh-powernv.c | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/pci_dn.c b/arch/powerpc/kernel/pci_dn.c
index 261d61460eac..90f8d46550df 100644
--- a/arch/powerpc/kernel/pci_dn.c
+++ b/arch/powerpc/kernel/pci_dn.c
@@ -542,6 +542,11 @@ void pci_devs_phb_init_dynamic(struct pci_controller *phb)
phb->pci_data = pdn;
}
+ if (of_get_property(dn, "ibm,supported-movable-bdfs", NULL)) {
+ pci_add_flags(PCI_REASSIGN_ALL_BUS);
+ return;
+ }
+
/* Update dn->phb ptrs for new phb and children devices */
pci_traverse_device_nodes(dn, add_pdn, phb);
}
diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c b/arch/powerpc/platforms/powernv/eeh-powernv.c
index 620a986209f5..eb01f16c4e60 100644
--- a/arch/powerpc/platforms/powernv/eeh-powernv.c
+++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
@@ -41,7 +41,7 @@ void pnv_pcibios_bus_add_device(struct pci_dev *pdev)
{
struct pci_dn *pdn = pci_get_pdn(pdev);
- if (!pdev->is_virtfn)
+ if (!pdev->is_virtfn && !pci_has_flag(PCI_REASSIGN_ALL_BUS))
return;
/*
--
2.21.0
^ permalink raw reply related
* Re: [PATCH] powerpc/vdso32: Add support for CLOCK_{REALTIME/MONOTONIC}_COARSE
From: Segher Boessenkool @ 2019-08-16 16:44 UTC (permalink / raw)
To: Christophe Leroy
Cc: Santosh Sivaraj, linux-kernel, Paul Mackerras, Naveen N. Rao,
linuxppc-dev
In-Reply-To: <1eb059dcb634c48980e5e43f465aabd3d35ba7f7.1565960416.git.christophe.leroy@c-s.fr>
On Fri, Aug 16, 2019 at 01:01:50PM +0000, Christophe Leroy wrote:
> - add r3,r3,r5
> +78: add r3,r3,r5
You can use actual names for the labels as well... .Lsomething if you
want it to stay a local symbol only.
Segher
^ permalink raw reply
* [PATCH v5 00/23] PCI: Allow BAR movement during hotplug
From: Sergey Miroshnichenko @ 2019-08-16 16:50 UTC (permalink / raw)
To: linux-pci, linuxppc-dev; +Cc: Sergey Miroshnichenko, Bjorn Helgaas, linux
If the firmware or kernel has arranged memory for PCIe devices in a way
that doesn't provide enough space for BARs of a new hotplugged device, the
kernel can pause the drivers of the "obstructing" devices and move their
BARs, so the new BARs can fit into the freed spaces.
To rearrange the BARs and bridge windows these patches releases all of them
after a rescan and re-assigns in the same way as during the initial PCIe
topology scan at system boot.
When a driver is un-paused by the kernel after the PCIe rescan, it should
check if its BARs had moved, and ioremap() them.
Drivers indicate their support of the feature by implementing the new hooks
.rescan_prepare() and .rescan_done() in the struct pci_driver. If a driver
doesn't yet support the feature, BARs of its devices will be considered as
immovable (by checking the pci_dev_movable_bars_supported(dev)) and handled
in the same way as resources with the IORESOURCE_PCI_FIXED flag.
If a driver doesn't yet support the feature, its devices are guaranteed to
have their BARs remaining untouched.
Tested on:
- x86_64 with "pci=realloc,assign-busses,use_crs,pcie_bus_peer2peer";
- POWER8 PowerNV+OPAL+PHB3 ppc64le with [1] applied and the following:
"pci=realloc,pcie_bus_peer2peer";
- both platforms [with extra pacthes (yet to be submitted) for movable bus
numbers]: manually initiated (via sysfs) rescan has found and turned on
a hotplugged bridge.
Not so many platforms and test cases were covered, so all who are
interested are highly welcome to test on your setups - the more exotic the
better!
This patchset is a part of our work on adding support for hotplugging
bridges full of other bridges, NVME drives, SAS HBAs and GPUs without
special requirements such as Hot-Plug Controller, reservation of bus
numbers or memory regions by firmware, etc. The next patchset to submit
will implement the movable bus numbers.
[1] https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-August/195272.html
[PATCH v6 0/5] powerpc/powernv/pci: Make hotplug self-sufficient, independent of FW and DT
Changes since v4:
- Feature is enabled by default (turned on by one of the latest patches);
- Add pci_dev_movable_bars_supported(dev) instead of marking the immovable
BARs with the IORESOURCE_PCI_FIXED flag;
- Set up PCIe bridges during rescan via sysfs, so MPS settings are now
configured not only during system boot or pcihp events;
- Allow movement of switch's BARs if claimed by portdrv;
- Update EEH address caches after rescan for powerpc;
- Don't disable completely hot-added devices which can't have BARs being
fit - just disable their BARs, so they are still visible in lspci etc;
- Clearer names: fixed_range_hard -> immovable_range, fixed_range_soft ->
realloc_range;
- Drop the patch for pci_restore_config_space() - fixed by properly using
the runtime PM.
Changes since v3:
- Rebased to the upstream, so the patches apply cleanly again.
Changes since v2:
- Fixed double-assignment of bridge windows;
- Fixed assignment of fixed prefetched resources;
- Fixed releasing of fixed resources;
- Fixed a debug message;
- Removed auto-enabling the movable BARs for x86 - let's rely on the
"pcie_movable_bars=force" option for now;
- Reordered the patches - bugfixes first.
Changes since v1:
- Add a "pcie_movable_bars={ off | force }" command line argument;
- Handle the IORESOURCE_PCI_FIXED flag properly;
- Don't move BARs of devices which don't support the feature;
- Guarantee that new hotplugged devices will not steal memory from working
devices by ignoring the failing new devices with the new PCI_DEV_IGNORE
flag;
- Add rescan_prepare()+rescan_done() to the struct pci_driver instead of
using the reset_prepare()+reset_done() from struct pci_error_handlers;
- Add a bugfix of a race condition;
- Fixed hotplug in a non-pre-enabled (by BIOS/firmware) bridge;
- Fix the compatibility of the feature with pm_runtime and D3-state;
- Hotplug events from pciehp also can move BARs;
- Add support of the feature to the NVME driver.
Sergey Miroshnichenko (23):
PCI: Fix race condition in pci_enable/disable_device()
PCI: Enable bridge's I/O and MEM access for hotplugged devices
PCI: hotplug: Add a flag for the movable BARs feature
PCI: Define PCI-specific version of the release_child_resources()
PCI: hotplug: movable BARs: Fix reassigning the released bridge
windows
PCI: hotplug: movable BARs: Recalculate all bridge windows during
rescan
PCI: hotplug: movable BARs: Don't allow added devices to steal
resources
PCI: Include fixed and immovable BARs into the bus size calculating
PCI: Prohibit assigning BARs and bridge windows to non-direct parents
PCI: hotplug: movable BARs: Try to assign unassigned resources only
once
PCI: hotplug: movable BARs: Calculate immovable parts of bridge
windows
PCI: hotplug: movable BARs: Compute limits for relocated bridge
windows
PCI: Make sure bridge windows include their fixed BARs
PCI: Fix assigning the fixed prefetchable resources
PCI: hotplug: movable BARs: Assign fixed and immovable BARs before
others
PCI: hotplug: movable BARs: Don't reserve IO/mem bus space
powerpc/pci: Fix crash with enabled movable BARs
powerpc/pci: Handle BAR movement
PCI: hotplug: Configure MPS for hot-added bridges during bus rescan
PCI: hotplug: movable BARs: Enable the feature by default
nvme-pci: Handle movable BARs
PCI/portdrv: Declare support of movable BARs
PCI: pciehp: movable BARs: Trigger a domain rescan on hp events
.../admin-guide/kernel-parameters.txt | 7 +
arch/powerpc/kernel/pci-hotplug.c | 10 +
arch/powerpc/platforms/powernv/pci-ioda.c | 3 +-
drivers/nvme/host/pci.c | 21 +-
drivers/pci/bus.c | 2 +-
drivers/pci/hotplug/pciehp_pci.c | 5 +
drivers/pci/pci.c | 58 +++-
drivers/pci/pci.h | 30 ++
drivers/pci/pcie/portdrv_pci.c | 11 +
drivers/pci/probe.c | 295 +++++++++++++++++-
drivers/pci/setup-bus.c | 276 +++++++++++++---
drivers/pci/setup-res.c | 48 ++-
include/linux/pci.h | 21 ++
13 files changed, 739 insertions(+), 48 deletions(-)
--
2.21.0
^ permalink raw reply
* [PATCH v5 02/23] PCI: Enable bridge's I/O and MEM access for hotplugged devices
From: Sergey Miroshnichenko @ 2019-08-16 16:50 UTC (permalink / raw)
To: linux-pci, linuxppc-dev; +Cc: Sergey Miroshnichenko, Bjorn Helgaas, linux
In-Reply-To: <20190816165101.911-1-s.miroshnichenko@yadro.com>
The PCI_COMMAND_IO and PCI_COMMAND_MEMORY bits of the bridge must be
updated not only when enabling the bridge for the first time, but also if a
hotplugged device requests these types of resources.
Originally these bits were set by the pci_enable_device_flags() only, which
exits early if the bridge is already pci_is_enabled(). So if the bridge was
empty initially (an edge case), then hotplugged devices fail to IO/MEM.
Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
drivers/pci/pci.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index e7f8c354e644..61d951766087 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1652,6 +1652,14 @@ static void pci_enable_bridge(struct pci_dev *dev)
pci_enable_bridge(bridge);
if (pci_is_enabled(dev)) {
+ int i, bars = 0;
+
+ for (i = PCI_BRIDGE_RESOURCES; i < DEVICE_COUNT_RESOURCE; i++) {
+ if (dev->resource[i].flags & (IORESOURCE_MEM | IORESOURCE_IO))
+ bars |= (1 << i);
+ }
+ do_pci_enable_device(dev, bars);
+
if (!dev->is_busmaster)
pci_set_master(dev);
mutex_unlock(&dev->enable_mutex);
--
2.21.0
^ permalink raw reply related
* [PATCH v5 01/23] PCI: Fix race condition in pci_enable/disable_device()
From: Sergey Miroshnichenko @ 2019-08-16 16:50 UTC (permalink / raw)
To: linux-pci, linuxppc-dev
Cc: Marta Rybczynska, Sergey Miroshnichenko, Srinath Mannam,
Bjorn Helgaas, linux
In-Reply-To: <20190816165101.911-1-s.miroshnichenko@yadro.com>
This is a yet another approach to fix an old [1-2] concurrency issue, when:
- two or more devices are being hot-added into a bridge which was
initially empty;
- a bridge with two or more devices is being hot-added;
- during boot, if BIOS/bootloader/firmware doesn't pre-enable bridges.
The problem is that a bridge is reported as enabled before the MEM/IO bits
are actually written to the PCI_COMMAND register, so another driver thread
starts memory requests through the not-yet-enabled bridge:
CPU0 CPU1
pci_enable_device_mem() pci_enable_device_mem()
pci_enable_bridge() pci_enable_bridge()
pci_is_enabled()
return false;
atomic_inc_return(enable_cnt)
Start actual enabling the bridge
... pci_is_enabled()
... return true;
... Start memory requests <-- FAIL
...
Set the PCI_COMMAND_MEMORY bit <-- Must wait for this
Protect the pci_enable/disable_device() and pci_enable_bridge(), which is
similar to the previous solution from commit 40f11adc7cd9 ("PCI: Avoid race
while enabling upstream bridges"), but adding a per-device mutexes and
preventing the dev->enable_cnt from from incrementing early.
CC: Srinath Mannam <srinath.mannam@broadcom.com>
CC: Marta Rybczynska <mrybczyn@kalray.eu>
Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
[1] https://lore.kernel.org/linux-pci/1501858648-22228-1-git-send-email-srinath.mannam@broadcom.com/T/#u
[RFC PATCH v3] pci: Concurrency issue during pci enable bridge
[2] https://lore.kernel.org/linux-pci/744877924.5841545.1521630049567.JavaMail.zimbra@kalray.eu/T/#u
[RFC PATCH] nvme: avoid race-conditions when enabling devices
---
drivers/pci/pci.c | 26 ++++++++++++++++++++++----
drivers/pci/probe.c | 1 +
include/linux/pci.h | 1 +
3 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 1b27b5af3d55..e7f8c354e644 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -1645,6 +1645,8 @@ static void pci_enable_bridge(struct pci_dev *dev)
struct pci_dev *bridge;
int retval;
+ mutex_lock(&dev->enable_mutex);
+
bridge = pci_upstream_bridge(dev);
if (bridge)
pci_enable_bridge(bridge);
@@ -1652,6 +1654,7 @@ static void pci_enable_bridge(struct pci_dev *dev)
if (pci_is_enabled(dev)) {
if (!dev->is_busmaster)
pci_set_master(dev);
+ mutex_unlock(&dev->enable_mutex);
return;
}
@@ -1660,11 +1663,14 @@ static void pci_enable_bridge(struct pci_dev *dev)
pci_err(dev, "Error enabling bridge (%d), continuing\n",
retval);
pci_set_master(dev);
+ mutex_unlock(&dev->enable_mutex);
}
static int pci_enable_device_flags(struct pci_dev *dev, unsigned long flags)
{
struct pci_dev *bridge;
+ /* Enable-locking of bridges is performed within the pci_enable_bridge() */
+ bool need_lock = !dev->subordinate;
int err;
int i, bars = 0;
@@ -1680,8 +1686,13 @@ static int pci_enable_device_flags(struct pci_dev *dev, unsigned long flags)
dev->current_state = (pmcsr & PCI_PM_CTRL_STATE_MASK);
}
- if (atomic_inc_return(&dev->enable_cnt) > 1)
+ if (need_lock)
+ mutex_lock(&dev->enable_mutex);
+ if (pci_is_enabled(dev)) {
+ if (need_lock)
+ mutex_unlock(&dev->enable_mutex);
return 0; /* already enabled */
+ }
bridge = pci_upstream_bridge(dev);
if (bridge)
@@ -1696,8 +1707,10 @@ static int pci_enable_device_flags(struct pci_dev *dev, unsigned long flags)
bars |= (1 << i);
err = do_pci_enable_device(dev, bars);
- if (err < 0)
- atomic_dec(&dev->enable_cnt);
+ if (err >= 0)
+ atomic_inc(&dev->enable_cnt);
+ if (need_lock)
+ mutex_unlock(&dev->enable_mutex);
return err;
}
@@ -1941,15 +1954,20 @@ void pci_disable_device(struct pci_dev *dev)
if (dr)
dr->enabled = 0;
+ mutex_lock(&dev->enable_mutex);
dev_WARN_ONCE(&dev->dev, atomic_read(&dev->enable_cnt) <= 0,
"disabling already-disabled device");
- if (atomic_dec_return(&dev->enable_cnt) != 0)
+ if (atomic_dec_return(&dev->enable_cnt) != 0) {
+ mutex_unlock(&dev->enable_mutex);
return;
+ }
do_pci_disable_device(dev);
dev->is_busmaster = 0;
+
+ mutex_unlock(&dev->enable_mutex);
}
EXPORT_SYMBOL(pci_disable_device);
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index a3c7338fad86..2e58ece820e8 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2427,6 +2427,7 @@ struct pci_dev *pci_alloc_dev(struct pci_bus *bus)
INIT_LIST_HEAD(&dev->bus_list);
dev->dev.type = &pci_dev_type;
dev->bus = pci_bus_get(bus);
+ mutex_init(&dev->enable_mutex);
return dev;
}
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 9e700d9f9f28..d3a72159722d 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -425,6 +425,7 @@ struct pci_dev {
unsigned int no_vf_scan:1; /* Don't scan for VFs after IOV enablement */
pci_dev_flags_t dev_flags;
atomic_t enable_cnt; /* pci_enable_device has been called */
+ struct mutex enable_mutex;
u32 saved_config_space[16]; /* Config space saved at suspend time */
struct hlist_head saved_cap_space;
--
2.21.0
^ permalink raw reply related
* [PATCH v5 03/23] PCI: hotplug: Add a flag for the movable BARs feature
From: Sergey Miroshnichenko @ 2019-08-16 16:50 UTC (permalink / raw)
To: linux-pci, linuxppc-dev
Cc: David Laight, Sam Bobroff, Sergey Miroshnichenko, linux,
Lukas Wunner, Bjorn Helgaas, Oliver O'Halloran, Rajat Jain
In-Reply-To: <20190816165101.911-1-s.miroshnichenko@yadro.com>
When hot-adding a device, the bridge may have windows not big enough (or
fragmented too much) for newly requested BARs to fit in. And expanding
these bridge windows may be impossible because blocked by "neighboring"
BARs and bridge windows.
Still, it may be possible to allocate a memory region for new BARs with the
following procedure:
1) notify all the drivers which support movable BARs to pause and release
the BARs; the rest of the drivers are guaranteed that their devices will
not get BARs moved;
2) release all the bridge windows except of root bridges;
3) try to recalculate new bridge windows that will fit all the BAR types:
- fixed;
- immovable;
- movable;
- newly requested by hot-added devices;
4) if the previous step fails, disable BARs for one of the hot-added
devices and retry from step 3;
5) notify the drivers, so they remap BARs and resume.
This makes the prior reservation of memory by BIOS/bootloader/firmware not
required anymore for the PCI hotplug.
Drivers indicate their support of movable BARs by implementing the new
.rescan_prepare() and .rescan_done() hooks in the struct pci_driver. All
device's activity must be paused during a rescan, and iounmap()+ioremap()
must be applied to every used BAR.
The platform also may need to prepare to BAR movement, so new hooks added:
pcibios_rescan_prepare(pci_dev) and pcibios_rescan_prepare(pci_dev).
This patch is a preparation for future patches with actual implementation,
and for now it just does the following:
- declares the feature;
- defines pci_movable_bars_enabled(), pci_dev_movable_bars_supported(dev);
- invokes the .rescan_prepare() and .rescan_done() driver notifiers;
- declares and invokes the pcibios_rescan_prepare()/_done() hooks;
- adds the PCI_IMMOVABLE_BARS flag.
The feature is disabled by default (via PCI_IMMOVABLE_BARS) until the final
patch of the series. It can be overridden per-arch using this flag or by
the following command line option:
pcie_movable_bars={ off | force }
CC: Sam Bobroff <sbobroff@linux.ibm.com>
CC: Rajat Jain <rajatja@google.com>
CC: Lukas Wunner <lukas@wunner.de>
CC: Oliver O'Halloran <oohall@gmail.com>
CC: David Laight <David.Laight@ACULAB.COM>
Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
.../admin-guide/kernel-parameters.txt | 7 ++
drivers/pci/pci-driver.c | 2 +
drivers/pci/pci.c | 24 ++++++
drivers/pci/pci.h | 2 +
drivers/pci/probe.c | 86 ++++++++++++++++++-
include/linux/pci.h | 7 ++
6 files changed, 126 insertions(+), 2 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 47d981a86e2f..e2274ee87a35 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3526,6 +3526,13 @@
nomsi Do not use MSI for native PCIe PME signaling (this makes
all PCIe root ports use INTx for all services).
+ pcie_movable_bars=[PCIE]
+ Override the movable BARs support detection:
+ off
+ Disable even if supported by the platform
+ force
+ Enable even if not explicitly declared as supported
+
pcmv= [HW,PCMCIA] BadgePAD 4
pd_ignore_unused
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index a8124e47bf6e..d11909e79263 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -1688,6 +1688,8 @@ static int __init pci_driver_init(void)
{
int ret;
+ pci_add_flags(PCI_IMMOVABLE_BARS);
+
ret = bus_register(&pci_bus_type);
if (ret)
return ret;
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 61d951766087..3a504f58ac60 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -139,6 +139,30 @@ static int __init pcie_port_pm_setup(char *str)
}
__setup("pcie_port_pm=", pcie_port_pm_setup);
+static bool pcie_movable_bars_off;
+static bool pcie_movable_bars_force;
+static int __init pcie_movable_bars_setup(char *str)
+{
+ if (!strcmp(str, "off"))
+ pcie_movable_bars_off = true;
+ else if (!strcmp(str, "force"))
+ pcie_movable_bars_force = true;
+ return 1;
+}
+__setup("pcie_movable_bars=", pcie_movable_bars_setup);
+
+bool pci_movable_bars_enabled(void)
+{
+ if (pcie_movable_bars_off)
+ return false;
+
+ if (pcie_movable_bars_force)
+ return true;
+
+ return !pci_has_flag(PCI_IMMOVABLE_BARS);
+}
+EXPORT_SYMBOL(pci_movable_bars_enabled);
+
/* Time to wait after a reset for device to become responsive */
#define PCIE_RESET_READY_POLL_MS 60000
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index d22d1b807701..be7acc477c64 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -257,6 +257,8 @@ bool pci_bus_clip_resource(struct pci_dev *dev, int idx);
void pci_reassigndev_resource_alignment(struct pci_dev *dev);
void pci_disable_bridge_window(struct pci_dev *dev);
+bool pci_dev_movable_bars_supported(struct pci_dev *dev);
+
/* PCIe link information */
#define PCIE_SPEED2STR(speed) \
((speed) == PCIE_SPEED_16_0GT ? "16 GT/s" : \
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 2e58ece820e8..60e3b48d2251 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -3406,6 +3406,74 @@ unsigned int pci_rescan_bus_bridge_resize(struct pci_dev *bridge)
return max;
}
+bool pci_dev_movable_bars_supported(struct pci_dev *dev)
+{
+ if (!dev)
+ return false;
+
+ if (dev->driver && dev->driver->rescan_prepare)
+ return true;
+
+ if ((dev->class >> 8) == PCI_CLASS_DISPLAY_VGA)
+ return false;
+
+ return !dev->driver;
+}
+
+void __weak pcibios_rescan_prepare(struct pci_dev *dev)
+{
+}
+
+void __weak pcibios_rescan_done(struct pci_dev *dev)
+{
+}
+
+static void pci_bus_rescan_prepare(struct pci_bus *bus)
+{
+ struct pci_dev *dev;
+
+ if (bus->self)
+ pci_config_pm_runtime_get(bus->self);
+
+ list_for_each_entry(dev, &bus->devices, bus_list) {
+ struct pci_bus *child = dev->subordinate;
+
+ if (child)
+ pci_bus_rescan_prepare(child);
+
+ if (dev->driver &&
+ dev->driver->rescan_prepare) {
+ dev->driver->rescan_prepare(dev);
+ pcibios_rescan_prepare(dev);
+ } else if (pci_dev_movable_bars_supported(dev)) {
+ pcibios_rescan_prepare(dev);
+ }
+ }
+}
+
+static void pci_bus_rescan_done(struct pci_bus *bus)
+{
+ struct pci_dev *dev;
+
+ list_for_each_entry(dev, &bus->devices, bus_list) {
+ struct pci_bus *child = dev->subordinate;
+
+ if (dev->driver &&
+ dev->driver->rescan_done) {
+ pcibios_rescan_done(dev);
+ dev->driver->rescan_done(dev);
+ } else if (pci_dev_movable_bars_supported(dev)) {
+ pcibios_rescan_done(dev);
+ }
+
+ if (child)
+ pci_bus_rescan_done(child);
+ }
+
+ if (bus->self)
+ pci_config_pm_runtime_put(bus->self);
+}
+
/**
* pci_rescan_bus - Scan a PCI bus for devices
* @bus: PCI bus to scan
@@ -3418,9 +3486,23 @@ unsigned int pci_rescan_bus_bridge_resize(struct pci_dev *bridge)
unsigned int pci_rescan_bus(struct pci_bus *bus)
{
unsigned int max;
+ struct pci_bus *root = bus;
+
+ while (!pci_is_root_bus(root))
+ root = root->parent;
+
+ if (pci_movable_bars_enabled()) {
+ pci_bus_rescan_prepare(root);
+
+ max = pci_scan_child_bus(root);
+ pci_assign_unassigned_root_bus_resources(root);
+
+ pci_bus_rescan_done(root);
+ } else {
+ max = pci_scan_child_bus(bus);
+ pci_assign_unassigned_bus_resources(bus);
+ }
- max = pci_scan_child_bus(bus);
- pci_assign_unassigned_bus_resources(bus);
pci_bus_add_devices(bus);
return max;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index d3a72159722d..e5b5eff05744 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -838,6 +838,8 @@ struct pci_driver {
int (*resume)(struct pci_dev *dev); /* Device woken up */
void (*shutdown)(struct pci_dev *dev);
int (*sriov_configure)(struct pci_dev *dev, int num_vfs); /* On PF */
+ void (*rescan_prepare)(struct pci_dev *dev);
+ void (*rescan_done)(struct pci_dev *dev);
const struct pci_error_handlers *err_handler;
const struct attribute_group **groups;
struct device_driver driver;
@@ -924,6 +926,7 @@ enum {
PCI_ENABLE_PROC_DOMAINS = 0x00000010, /* Enable domains in /proc */
PCI_COMPAT_DOMAIN_0 = 0x00000020, /* ... except domain 0 */
PCI_SCAN_ALL_PCIE_DEVS = 0x00000040, /* Scan all, not just dev 0 */
+ PCI_IMMOVABLE_BARS = 0x00000080, /* Disable runtime BAR reassign */
};
/* These external functions are only available when PCI support is enabled */
@@ -1266,6 +1269,9 @@ unsigned int pci_rescan_bus(struct pci_bus *bus);
void pci_lock_rescan_remove(void);
void pci_unlock_rescan_remove(void);
+void pcibios_rescan_prepare(struct pci_dev *dev);
+void pcibios_rescan_done(struct pci_dev *dev);
+
/* Vital Product Data routines */
ssize_t pci_read_vpd(struct pci_dev *dev, loff_t pos, size_t count, void *buf);
ssize_t pci_write_vpd(struct pci_dev *dev, loff_t pos, size_t count, const void *buf);
@@ -1402,6 +1408,7 @@ unsigned char pci_bus_max_busnr(struct pci_bus *bus);
void pci_setup_bridge(struct pci_bus *bus);
resource_size_t pcibios_window_alignment(struct pci_bus *bus,
unsigned long type);
+bool pci_movable_bars_enabled(void);
#define PCI_VGA_STATE_CHANGE_BRIDGE (1 << 0)
#define PCI_VGA_STATE_CHANGE_DECODES (1 << 1)
--
2.21.0
^ permalink raw reply related
* [PATCH v5 04/23] PCI: Define PCI-specific version of the release_child_resources()
From: Sergey Miroshnichenko @ 2019-08-16 16:50 UTC (permalink / raw)
To: linux-pci, linuxppc-dev; +Cc: Sergey Miroshnichenko, Bjorn Helgaas, linux
In-Reply-To: <20190816165101.911-1-s.miroshnichenko@yadro.com>
If release the bridge resources with standard release_child_resources(), it
drops the .start field of children's BARs to zero, but with the STARTALIGN
flag remaining set, which makes the resource invalid for reassignment.
Some resources must preserve their offset and size: those marked with the
PCI_FIXED and the immovable ones - which are bound by drivers without
support of the movable BARs feature.
Add the pci_release_child_resources() to replace release_child_resources()
in handling the described PCI-specific cases.
Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
drivers/pci/setup-bus.c | 54 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 53 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 79b1fa6519be..6cb8b293c576 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1482,6 +1482,55 @@ static void __pci_bridge_assign_resources(const struct pci_dev *bridge,
(IORESOURCE_IO | IORESOURCE_MEM | IORESOURCE_PREFETCH |\
IORESOURCE_MEM_64)
+/*
+ * Similar to generic release_child_resources(), but aware of immovable BARs and
+ * PCI_FIXED and STARTALIGN flags
+ */
+static void pci_release_child_resources(struct pci_bus *bus, struct resource *r)
+{
+ struct pci_dev *dev;
+
+ if (!bus || !r)
+ return;
+
+ if (r->flags & IORESOURCE_PCI_FIXED)
+ return;
+
+ r->child = NULL;
+
+ list_for_each_entry(dev, &bus->devices, bus_list) {
+ int i;
+
+ for (i = 0; i < PCI_NUM_RESOURCES; i++) {
+ struct resource *tmp = &dev->resource[i];
+ resource_size_t size = resource_size(tmp);
+
+ if (!tmp->flags || tmp->parent != r)
+ continue;
+
+ tmp->parent = NULL;
+ tmp->sibling = NULL;
+
+ pci_release_child_resources(dev->subordinate, tmp);
+
+ if ((tmp->flags & IORESOURCE_PCI_FIXED) ||
+ !pci_dev_movable_bars_supported(dev)) {
+ pci_dbg(dev, "release immovable %pR (%s), keep its flags, base and size\n",
+ tmp, tmp->name);
+ continue;
+ }
+
+ pci_dbg(dev, "release %pR (%s)\n", tmp, tmp->name);
+
+ tmp->start = 0;
+ tmp->end = size - 1;
+
+ tmp->flags &= ~IORESOURCE_STARTALIGN;
+ tmp->flags |= IORESOURCE_SIZEALIGN;
+ }
+ }
+}
+
static void pci_bridge_release_resources(struct pci_bus *bus,
unsigned long type)
{
@@ -1522,7 +1571,10 @@ static void pci_bridge_release_resources(struct pci_bus *bus,
return;
/* If there are children, release them all */
- release_child_resources(r);
+ if (pci_movable_bars_enabled())
+ pci_release_child_resources(bus, r);
+ else
+ release_child_resources(r);
if (!release_resource(r)) {
type = old_flags = r->flags & PCI_RES_TYPE_MASK;
pci_info(dev, "resource %d %pR released\n",
--
2.21.0
^ permalink raw reply related
* [PATCH v5 05/23] PCI: hotplug: movable BARs: Fix reassigning the released bridge windows
From: Sergey Miroshnichenko @ 2019-08-16 16:50 UTC (permalink / raw)
To: linux-pci, linuxppc-dev; +Cc: Sergey Miroshnichenko, Bjorn Helgaas, linux
In-Reply-To: <20190816165101.911-1-s.miroshnichenko@yadro.com>
When a bridge window is temporarily released during the rescan, its old
size is not relevant anymore - it will be recreated from pbus_size_*(), so
it's start value should be zero.
If such window can't be reassigned, don't apply reset_resource(), so the
next retry may succeed.
Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
drivers/pci/setup-bus.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 6cb8b293c576..7c2c57f77c6f 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -295,7 +295,8 @@ static void assign_requested_resources_sorted(struct list_head *head,
0 /* don't care */,
0 /* don't care */);
}
- reset_resource(res);
+ if (!pci_movable_bars_enabled())
+ reset_resource(res);
}
}
}
@@ -1579,8 +1580,8 @@ static void pci_bridge_release_resources(struct pci_bus *bus,
type = old_flags = r->flags & PCI_RES_TYPE_MASK;
pci_info(dev, "resource %d %pR released\n",
PCI_BRIDGE_RESOURCES + idx, r);
- /* Keep the old size */
- r->end = resource_size(r) - 1;
+ /* Don't keep the old size if the bridge will be recalculated */
+ r->end = pci_movable_bars_enabled() ? 0 : (resource_size(r) - 1);
r->start = 0;
r->flags = 0;
--
2.21.0
^ permalink raw reply related
* [PATCH v5 06/23] PCI: hotplug: movable BARs: Recalculate all bridge windows during rescan
From: Sergey Miroshnichenko @ 2019-08-16 16:50 UTC (permalink / raw)
To: linux-pci, linuxppc-dev; +Cc: Sergey Miroshnichenko, Bjorn Helgaas, linux
In-Reply-To: <20190816165101.911-1-s.miroshnichenko@yadro.com>
When the movable BARs feature is enabled and a rescan has been requested,
release all the bridge windows and recalculate them from scratch, taking
into account all kinds for BARs: fixed, immovable, movable, new.
This increases the chances to find a memory space to fit BARs for newly
hotplugged devices, especially if no/not enough gaps were reserved by the
BIOS/bootloader/firmware.
The last step of writing the recalculated windows to the bridges is done
by the new pci_setup_bridges() function.
Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
drivers/pci/pci.h | 1 +
drivers/pci/probe.c | 22 ++++++++++++++++++++++
drivers/pci/setup-bus.c | 16 ++++++++++++++++
3 files changed, 39 insertions(+)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index be7acc477c64..a0ec696512eb 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -253,6 +253,7 @@ void __pci_bus_assign_resources(const struct pci_bus *bus,
struct list_head *realloc_head,
struct list_head *fail_head);
bool pci_bus_clip_resource(struct pci_dev *dev, int idx);
+void pci_bus_release_root_bridge_resources(struct pci_bus *bus);
void pci_reassigndev_resource_alignment(struct pci_dev *dev);
void pci_disable_bridge_window(struct pci_dev *dev);
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 60e3b48d2251..a26bf740e9ab 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -3474,6 +3474,25 @@ static void pci_bus_rescan_done(struct pci_bus *bus)
pci_config_pm_runtime_put(bus->self);
}
+static void pci_setup_bridges(struct pci_bus *bus)
+{
+ struct pci_dev *dev;
+
+ list_for_each_entry(dev, &bus->devices, bus_list) {
+ struct pci_bus *child;
+
+ if (!pci_dev_is_added(dev) || pci_dev_is_ignored(dev))
+ continue;
+
+ child = dev->subordinate;
+ if (child)
+ pci_setup_bridges(child);
+ }
+
+ if (bus->self)
+ pci_setup_bridge(bus);
+}
+
/**
* pci_rescan_bus - Scan a PCI bus for devices
* @bus: PCI bus to scan
@@ -3495,8 +3514,11 @@ unsigned int pci_rescan_bus(struct pci_bus *bus)
pci_bus_rescan_prepare(root);
max = pci_scan_child_bus(root);
+
+ pci_bus_release_root_bridge_resources(root);
pci_assign_unassigned_root_bus_resources(root);
+ pci_setup_bridges(root);
pci_bus_rescan_done(root);
} else {
max = pci_scan_child_bus(bus);
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 7c2c57f77c6f..04f626e1ac18 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1635,6 +1635,22 @@ static void pci_bus_release_bridge_resources(struct pci_bus *bus,
pci_bridge_release_resources(bus, type);
}
+void pci_bus_release_root_bridge_resources(struct pci_bus *root_bus)
+{
+ int i;
+ struct resource *r;
+
+ pci_bus_release_bridge_resources(root_bus, IORESOURCE_IO, whole_subtree);
+ pci_bus_release_bridge_resources(root_bus, IORESOURCE_MEM, whole_subtree);
+ pci_bus_release_bridge_resources(root_bus,
+ IORESOURCE_MEM_64 | IORESOURCE_PREFETCH,
+ whole_subtree);
+
+ pci_bus_for_each_resource(root_bus, r, i) {
+ pci_release_child_resources(root_bus, r);
+ }
+}
+
static void pci_bus_dump_res(struct pci_bus *bus)
{
struct resource *res;
--
2.21.0
^ permalink raw reply related
* [PATCH v5 08/23] PCI: Include fixed and immovable BARs into the bus size calculating
From: Sergey Miroshnichenko @ 2019-08-16 16:50 UTC (permalink / raw)
To: linux-pci, linuxppc-dev; +Cc: Sergey Miroshnichenko, Bjorn Helgaas, linux
In-Reply-To: <20190816165101.911-1-s.miroshnichenko@yadro.com>
The only difference between the fixed/immovable and movable BARs is a size
and offset preservation after they are released (the corresponding struct
resource* detached from a bridge window for a while during a bus rescan).
Include fixed/immovable BARs into result of pbus_size_mem() and prohibit
assigning them to non-direct parents.
Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
drivers/pci/setup-bus.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 1a731002ce18..2c250efca512 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1011,12 +1011,21 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
struct resource *r = &dev->resource[i];
resource_size_t r_size;
- if (r->parent || (r->flags & IORESOURCE_PCI_FIXED) ||
+ if (r->parent ||
((r->flags & mask) != type &&
(r->flags & mask) != type2 &&
(r->flags & mask) != type3))
continue;
r_size = resource_size(r);
+
+ if ((r->flags & IORESOURCE_PCI_FIXED) ||
+ !pci_dev_movable_bars_supported(dev)) {
+ if (pci_movable_bars_enabled())
+ size += r_size;
+
+ continue;
+ }
+
#ifdef CONFIG_PCI_IOV
/* Put SRIOV requested res to the optional list */
if (realloc_head && i >= PCI_IOV_RESOURCES &&
--
2.21.0
^ permalink raw reply related
* [PATCH v5 07/23] PCI: hotplug: movable BARs: Don't allow added devices to steal resources
From: Sergey Miroshnichenko @ 2019-08-16 16:50 UTC (permalink / raw)
To: linux-pci, linuxppc-dev; +Cc: Sergey Miroshnichenko, Bjorn Helgaas, linux
In-Reply-To: <20190816165101.911-1-s.miroshnichenko@yadro.com>
When movable BARs are enabled, the PCI subsystem at first releases all the
bridge windows and then attempts to assign resources both to previously
working devices and to the newly hotplugged ones, with the same priority.
If a hotplugged device gets its BARs first, this may lead to lack of space
for already working devices, which is unacceptable. If that happens, mark
one of the new devices with the newly introduced flag PCI_DEV_DISABLED_BARS
(if it is not yet marked) and retry the BAR recalculation.
The worst case would be no BARs for hotplugged devices, while all the rest
just continue working.
The algorithm is simple and it doesn't retry different subsets of hot-added
devices in case of a failure, e.g. if there are no space to allocate BARs
for both hotplugged devices A and B, but is enough for just A, the A will
be marked with PCI_DEV_DISABLED_BARS first, then (after the next failure) -
B. As a result, A will not get BARs while it could. This issue is only
relevant when hotplugging two and more devices simultaneously.
Add a new res_mask bitmask to the struct pci_dev for storing the indices of
assigned BARs.
Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
drivers/pci/pci.h | 11 +++++
drivers/pci/probe.c | 101 ++++++++++++++++++++++++++++++++++++++--
drivers/pci/setup-bus.c | 15 ++++++
include/linux/pci.h | 1 +
4 files changed, 125 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index a0ec696512eb..53249cbc21b6 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -373,6 +373,7 @@ static inline bool pci_dev_is_disconnected(const struct pci_dev *dev)
/* pci_dev priv_flags */
#define PCI_DEV_ADDED 0
+#define PCI_DEV_DISABLED_BARS 1
static inline void pci_dev_assign_added(struct pci_dev *dev, bool added)
{
@@ -384,6 +385,16 @@ static inline bool pci_dev_is_added(const struct pci_dev *dev)
return test_bit(PCI_DEV_ADDED, &dev->priv_flags);
}
+static inline void pci_dev_disable_bars(struct pci_dev *dev)
+{
+ assign_bit(PCI_DEV_DISABLED_BARS, &dev->priv_flags, true);
+}
+
+static inline bool pci_dev_bars_enabled(const struct pci_dev *dev)
+{
+ return !test_bit(PCI_DEV_DISABLED_BARS, &dev->priv_flags);
+}
+
#ifdef CONFIG_PCIEAER
#include <linux/aer.h>
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index a26bf740e9ab..bf0a7d1c5d09 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -3428,6 +3428,23 @@ void __weak pcibios_rescan_done(struct pci_dev *dev)
{
}
+static unsigned int pci_dev_count_res_mask(struct pci_dev *dev)
+{
+ unsigned int res_mask = 0;
+ int i;
+
+ for (i = 0; i < PCI_BRIDGE_RESOURCES; i++) {
+ struct resource *r = &dev->resource[i];
+
+ if (!r->flags || (r->flags & IORESOURCE_UNSET) || !r->parent)
+ continue;
+
+ res_mask |= (1 << i);
+ }
+
+ return res_mask;
+}
+
static void pci_bus_rescan_prepare(struct pci_bus *bus)
{
struct pci_dev *dev;
@@ -3438,6 +3455,8 @@ static void pci_bus_rescan_prepare(struct pci_bus *bus)
list_for_each_entry(dev, &bus->devices, bus_list) {
struct pci_bus *child = dev->subordinate;
+ dev->res_mask = pci_dev_count_res_mask(dev);
+
if (child)
pci_bus_rescan_prepare(child);
@@ -3481,7 +3500,7 @@ static void pci_setup_bridges(struct pci_bus *bus)
list_for_each_entry(dev, &bus->devices, bus_list) {
struct pci_bus *child;
- if (!pci_dev_is_added(dev) || pci_dev_is_ignored(dev))
+ if (!pci_dev_is_added(dev) || !pci_dev_bars_enabled(dev))
continue;
child = dev->subordinate;
@@ -3493,6 +3512,83 @@ static void pci_setup_bridges(struct pci_bus *bus)
pci_setup_bridge(bus);
}
+static struct pci_dev *pci_find_next_new_device(struct pci_bus *bus)
+{
+ struct pci_dev *dev;
+
+ if (!bus)
+ return NULL;
+
+ list_for_each_entry(dev, &bus->devices, bus_list) {
+ struct pci_bus *child_bus = dev->subordinate;
+
+ if (!pci_dev_is_added(dev) && pci_dev_bars_enabled(dev))
+ return dev;
+
+ if (child_bus) {
+ struct pci_dev *next_new_dev;
+
+ next_new_dev = pci_find_next_new_device(child_bus);
+ if (next_new_dev)
+ return next_new_dev;
+ }
+ }
+
+ return NULL;
+}
+
+static bool pci_bus_check_all_bars_reassigned(struct pci_bus *bus)
+{
+ struct pci_dev *dev;
+ bool ret = true;
+
+ if (!bus)
+ return false;
+
+ list_for_each_entry(dev, &bus->devices, bus_list) {
+ struct pci_bus *child = dev->subordinate;
+ unsigned int res_mask = pci_dev_count_res_mask(dev);
+
+ if (!pci_dev_bars_enabled(dev))
+ continue;
+
+ if (dev->res_mask & ~res_mask) {
+ pci_err(dev, "Non-re-enabled resources found: 0x%x -> 0x%x\n",
+ dev->res_mask, res_mask);
+ ret = false;
+ }
+
+ if (child && !pci_bus_check_all_bars_reassigned(child))
+ ret = false;
+ }
+
+ return ret;
+}
+
+static void pci_reassign_root_bus_resources(struct pci_bus *root)
+{
+ do {
+ struct pci_dev *next_new_dev;
+
+ pci_bus_release_root_bridge_resources(root);
+ pci_assign_unassigned_root_bus_resources(root);
+
+ if (pci_bus_check_all_bars_reassigned(root))
+ break;
+
+ next_new_dev = pci_find_next_new_device(root);
+ if (!next_new_dev) {
+ dev_err(&root->dev, "failed to re-assign resources even after ignoring all the hotplugged devices\n");
+ break;
+ }
+
+ dev_warn(&root->dev, "failed to re-assign resources, disable the next hotplugged device %s and retry\n",
+ dev_name(&next_new_dev->dev));
+
+ pci_dev_disable_bars(next_new_dev);
+ } while (true);
+}
+
/**
* pci_rescan_bus - Scan a PCI bus for devices
* @bus: PCI bus to scan
@@ -3515,8 +3611,7 @@ unsigned int pci_rescan_bus(struct pci_bus *bus)
max = pci_scan_child_bus(root);
- pci_bus_release_root_bridge_resources(root);
- pci_assign_unassigned_root_bus_resources(root);
+ pci_reassign_root_bus_resources(root);
pci_setup_bridges(root);
pci_bus_rescan_done(root);
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 04f626e1ac18..1a731002ce18 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -128,6 +128,9 @@ static void pdev_sort_resources(struct pci_dev *dev, struct list_head *head)
{
int i;
+ if (!pci_dev_bars_enabled(dev))
+ return;
+
for (i = 0; i < PCI_NUM_RESOURCES; i++) {
struct resource *r;
struct pci_dev_resource *dev_res, *tmp;
@@ -177,6 +180,9 @@ static void __dev_sort_resources(struct pci_dev *dev, struct list_head *head)
{
u16 class = dev->class >> 8;
+ if (!pci_dev_bars_enabled(dev))
+ return;
+
/* Don't touch classless devices or host bridges or IOAPICs */
if (class == PCI_CLASS_NOT_DEFINED || class == PCI_CLASS_BRIDGE_HOST)
return;
@@ -278,6 +284,9 @@ static void assign_requested_resources_sorted(struct list_head *head,
int idx;
list_for_each_entry(dev_res, head, list) {
+ if (!pci_dev_bars_enabled(dev_res->dev))
+ continue;
+
res = dev_res->res;
idx = res - &dev_res->dev->resource[0];
if (resource_size(res) &&
@@ -995,6 +1004,9 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
list_for_each_entry(dev, &bus->devices, bus_list) {
int i;
+ if (!pci_dev_bars_enabled(dev))
+ continue;
+
for (i = 0; i < PCI_NUM_RESOURCES; i++) {
struct resource *r = &dev->resource[i];
resource_size_t r_size;
@@ -1349,6 +1361,9 @@ void __pci_bus_assign_resources(const struct pci_bus *bus,
pbus_assign_resources_sorted(bus, realloc_head, fail_head);
list_for_each_entry(dev, &bus->devices, bus_list) {
+ if (!pci_dev_bars_enabled(dev))
+ continue;
+
pdev_assign_fixed_resources(dev);
b = dev->subordinate;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index e5b5eff05744..95a8113c2157 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -375,6 +375,7 @@ struct pci_dev {
*/
unsigned int irq;
struct resource resource[DEVICE_COUNT_RESOURCE]; /* I/O and memory regions + expansion ROMs */
+ unsigned int res_mask; /* Bitmask of assigned resources */
bool match_driver; /* Skip attaching driver */
--
2.21.0
^ permalink raw reply related
* [PATCH v5 10/23] PCI: hotplug: movable BARs: Try to assign unassigned resources only once
From: Sergey Miroshnichenko @ 2019-08-16 16:50 UTC (permalink / raw)
To: linux-pci, linuxppc-dev; +Cc: Sergey Miroshnichenko, Bjorn Helgaas, linux
In-Reply-To: <20190816165101.911-1-s.miroshnichenko@yadro.com>
With enabled BAR movement, BARs and bridge windows can only be assigned to
their direct parents, so there can be only one variant of resource tree,
thus every retry within the pci_assign_unassigned_root_bus_resources() will
result in the same tree, and it is enough to try just once.
In case of failures the pci_reassign_root_bus_resources() disables BARs for
one of the hotplugged devices and tries the assignment again.
Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
drivers/pci/setup-bus.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index aee330047121..33f709095675 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1819,6 +1819,13 @@ void pci_assign_unassigned_root_bus_resources(struct pci_bus *bus)
int pci_try_num = 1;
enum enable_type enable_local;
+ if (pci_movable_bars_enabled()) {
+ __pci_bus_size_bridges(bus, NULL);
+ __pci_bus_assign_resources(bus, NULL, NULL);
+
+ goto dump;
+ }
+
/* Don't realloc if asked to do so */
enable_local = pci_realloc_detect(bus, pci_realloc_enable);
if (pci_realloc_enabled(enable_local)) {
--
2.21.0
^ permalink raw reply related
* [PATCH v5 09/23] PCI: Prohibit assigning BARs and bridge windows to non-direct parents
From: Sergey Miroshnichenko @ 2019-08-16 16:50 UTC (permalink / raw)
To: linux-pci, linuxppc-dev; +Cc: Sergey Miroshnichenko, Bjorn Helgaas, linux
In-Reply-To: <20190816165101.911-1-s.miroshnichenko@yadro.com>
When movable BARs are enabled, the feature of resource relocating from
commit 2bbc6942273b5 ("PCI : ability to relocate assigned pci-resources")
is not used. Instead, inability to assign a resource is used as a signal
to retry BAR assignment with other configuration of bridge windows.
Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
drivers/pci/setup-bus.c | 2 ++
drivers/pci/setup-res.c | 12 ++++++++++++
2 files changed, 14 insertions(+)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 2c250efca512..aee330047121 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1356,6 +1356,8 @@ static void pdev_assign_fixed_resources(struct pci_dev *dev)
while (b && !r->parent) {
assign_fixed_resource_on_bus(b, r);
b = b->parent;
+ if (!r->parent && pci_movable_bars_enabled())
+ break;
}
}
}
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index d8ca40a97693..732d18f60f1b 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -298,6 +298,18 @@ static int _pci_assign_resource(struct pci_dev *dev, int resno,
bus = dev->bus;
while ((ret = __pci_assign_resource(bus, dev, resno, size, min_align))) {
+ if (pci_movable_bars_enabled()) {
+ if (resno >= PCI_BRIDGE_RESOURCES &&
+ resno <= PCI_BRIDGE_RESOURCE_END) {
+ struct resource *res = dev->resource + resno;
+
+ res->start = 0;
+ res->end = 0;
+ res->flags = 0;
+ }
+ break;
+ }
+
if (!bus->parent || !bus->self->transparent)
break;
bus = bus->parent;
--
2.21.0
^ permalink raw reply related
* [PATCH v5 11/23] PCI: hotplug: movable BARs: Calculate immovable parts of bridge windows
From: Sergey Miroshnichenko @ 2019-08-16 16:50 UTC (permalink / raw)
To: linux-pci, linuxppc-dev; +Cc: Sergey Miroshnichenko, Bjorn Helgaas, linux
In-Reply-To: <20190816165101.911-1-s.miroshnichenko@yadro.com>
When movable BARs are enabled, and if a bridge contains a device with fixed
(IORESOURCE_PCI_FIXED) or immovable BARs, the corresponing windows can't be
moved too far away from their original positions - they must still contain
all the fixed/immovable BARs, like that:
1) Window position before a bus rescan:
| <-- root bridge window --> |
| |
| | <-- bridge window --> | |
| | movable BARs | **fixed BAR** | |
2) Possible valid outcome after rescan and move:
| <-- root bridge window --> |
| |
| | <-- bridge window --> | |
| | **fixed BAR** | Movable BARs | |
An immovable area of a bridge (separare for IO, MEM and MEM64 window types)
is a range that covers all the fixed and immovable BARs of direct children,
and all the fixed area of children bridges:
| <-- root bridge window --> |
| |
| | <-- bridge window level 1 --> | |
| | ******** immovable area of this bridge window ******** | |
| | | |
| | **fixed BAR** | <-- bridge window level 2 --> | BARs | |
| | | ***** fixed area of this bridge ***** | | |
| | | | | |
| | | ***fixed BAR*** | | ***fixed BAR*** | | |
To store these areas, the .immovable_range field has been added to struct
pci_bus. It is filled recursively from leaves to the root before a rescan.
Also make pbus_size_io() and pbus_size_mem() return their usual result OR
the size of an immovable range of according type, depending on which one is
larger.
Signed-off-by: Sergey Miroshnichenko <s.miroshnichenko@yadro.com>
---
drivers/pci/pci.h | 14 +++++++
drivers/pci/probe.c | 88 +++++++++++++++++++++++++++++++++++++++++
drivers/pci/setup-bus.c | 17 ++++++++
include/linux/pci.h | 6 +++
4 files changed, 125 insertions(+)
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 53249cbc21b6..12add575faf1 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -371,6 +371,20 @@ static inline bool pci_dev_is_disconnected(const struct pci_dev *dev)
return dev->error_state == pci_channel_io_perm_failure;
}
+static inline int pci_get_bridge_resource_idx(struct resource *r)
+{
+ int idx = 1;
+
+ if (r->flags & IORESOURCE_IO)
+ idx = 0;
+ else if (!(r->flags & IORESOURCE_PREFETCH))
+ idx = 1;
+ else if (r->flags & IORESOURCE_MEM_64)
+ idx = 2;
+
+ return idx;
+}
+
/* pci_dev priv_flags */
#define PCI_DEV_ADDED 0
#define PCI_DEV_DISABLED_BARS 1
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index bf0a7d1c5d09..5f52a19738aa 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -550,6 +550,7 @@ void pci_read_bridge_bases(struct pci_bus *child)
static struct pci_bus *pci_alloc_bus(struct pci_bus *parent)
{
struct pci_bus *b;
+ int idx;
b = kzalloc(sizeof(*b), GFP_KERNEL);
if (!b)
@@ -566,6 +567,11 @@ static struct pci_bus *pci_alloc_bus(struct pci_bus *parent)
if (parent)
b->domain_nr = parent->domain_nr;
#endif
+ for (idx = 0; idx < PCI_BRIDGE_RESOURCE_NUM; ++idx) {
+ b->immovable_range[idx].start = 0;
+ b->immovable_range[idx].end = 0;
+ }
+
return b;
}
@@ -3512,6 +3518,87 @@ static void pci_setup_bridges(struct pci_bus *bus)
pci_setup_bridge(bus);
}
+static void pci_bus_update_immovable_range(struct pci_bus *bus)
+{
+ struct pci_dev *dev;
+ int idx;
+ resource_size_t start, end;
+
+ for (idx = 0; idx < PCI_BRIDGE_RESOURCE_NUM; ++idx) {
+ bus->immovable_range[idx].start = 0;
+ bus->immovable_range[idx].end = 0;
+ }
+
+ list_for_each_entry(dev, &bus->devices, bus_list)
+ if (dev->subordinate)
+ pci_bus_update_immovable_range(dev->subordinate);
+
+ list_for_each_entry(dev, &bus->devices, bus_list) {
+ int i;
+ bool dev_is_movable = pci_dev_movable_bars_supported(dev);
+ struct pci_bus *child = dev->subordinate;
+
+ for (i = 0; i < PCI_BRIDGE_RESOURCES; ++i) {
+ struct resource *r = &dev->resource[i];
+
+ if (!r->flags || (r->flags & IORESOURCE_UNSET) || !r->parent)
+ continue;
+
+ if (!dev_is_movable || (r->flags & IORESOURCE_PCI_FIXED)) {
+ idx = pci_get_bridge_resource_idx(r);
+ start = bus->immovable_range[idx].start;
+ end = bus->immovable_range[idx].end;
+
+ if (!start || start > r->start)
+ start = r->start;
+ if (end < r->end)
+ end = r->end;
+
+ if (bus->immovable_range[idx].start != start ||
+ bus->immovable_range[idx].end != end) {
+ dev_dbg(&bus->dev, "Found fixed 0x%llx-0x%llx in %s, expand the fixed bridge window %d to 0x%llx-0x%llx\n",
+ (unsigned long long)r->start,
+ (unsigned long long)r->end,
+ dev_name(&dev->dev), idx,
+ (unsigned long long)start,
+ (unsigned long long)end);
+ bus->immovable_range[idx].start = start;
+ bus->immovable_range[idx].end = end;
+ }
+ }
+ }
+
+ if (child) {
+ for (idx = 0; idx < PCI_BRIDGE_RESOURCE_NUM; ++idx) {
+ struct resource *child_immovable_range =
+ &child->immovable_range[idx];
+
+ if (child_immovable_range->start >=
+ child_immovable_range->end)
+ continue;
+
+ start = bus->immovable_range[idx].start;
+ end = bus->immovable_range[idx].end;
+
+ if (!start || start > child_immovable_range->start)
+ start = child_immovable_range->start;
+ if (end < child_immovable_range->end)
+ end = child_immovable_range->end;
+
+ if (start < bus->immovable_range[idx].start ||
+ end > bus->immovable_range[idx].end) {
+ dev_dbg(&bus->dev, "Expand the fixed bridge window %d from %s to 0x%llx-0x%llx\n",
+ idx, dev_name(&child->dev),
+ (unsigned long long)start,
+ (unsigned long long)end);
+ bus->immovable_range[idx].start = start;
+ bus->immovable_range[idx].end = end;
+ }
+ }
+ }
+ }
+}
+
static struct pci_dev *pci_find_next_new_device(struct pci_bus *bus)
{
struct pci_dev *dev;
@@ -3610,6 +3697,7 @@ unsigned int pci_rescan_bus(struct pci_bus *bus)
pci_bus_rescan_prepare(root);
max = pci_scan_child_bus(root);
+ pci_bus_update_immovable_range(root);
pci_reassign_root_bus_resources(root);
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 33f709095675..420510a1a257 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -882,9 +882,17 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size,
resource_size_t children_add_size = 0;
resource_size_t min_align, align;
+ resource_size_t fixed_start = bus->immovable_range[0].start;
+ resource_size_t fixed_end = bus->immovable_range[0].end;
+ resource_size_t fixed_size = (fixed_start < fixed_end) ?
+ (fixed_end - fixed_start + 1) : 0;
+
if (!b_res)
return;
+ if (min_size < fixed_size)
+ min_size = fixed_size;
+
min_align = window_alignment(bus, IORESOURCE_IO);
list_for_each_entry(dev, &bus->devices, bus_list) {
int i;
@@ -993,6 +1001,15 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
resource_size_t children_add_size = 0;
resource_size_t children_add_align = 0;
resource_size_t add_align = 0;
+ bool is_mem64 = (mask & IORESOURCE_MEM_64);
+
+ resource_size_t fixed_start = bus->immovable_range[is_mem64 ? 2 : 1].start;
+ resource_size_t fixed_end = bus->immovable_range[is_mem64 ? 2 : 1].end;
+ resource_size_t fixed_size = (fixed_start < fixed_end) ?
+ (fixed_end - fixed_start + 1) : 0;
+
+ if (min_size < fixed_size)
+ min_size = fixed_size;
if (!b_res)
return -ENOSPC;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 95a8113c2157..efafbf816fe6 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -581,6 +581,12 @@ struct pci_bus {
struct list_head resources; /* Address space routed to this bus */
struct resource busn_res; /* Bus numbers routed to this bus */
+ /*
+ * If there are fixed or immovable resources in the bridge window, this range
+ * contains the lowest start address and highest end address of them.
+ */
+ struct resource immovable_range[PCI_BRIDGE_RESOURCE_NUM];
+
struct pci_ops *ops; /* Configuration access functions */
struct msi_controller *msi; /* MSI controller */
void *sysdata; /* Hook for sys-specific extension */
--
2.21.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox