* [Bug 204371] BUG kmalloc-4k (Tainted: G W ): Object padding overwritten
From: bugzilla-daemon @ 2019-08-16 8:22 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 #32 from Christophe Leroy (christophe.leroy@c-s.fr) ---
I think first thing is to fix test_add_free_space_entry() :
- replace the map = kzalloc(...) by map = (void *)get_zeroed_page(...) like in
other places.
- replace the kfree(map); by free_page((unsigned long)map);
Then see if the WARNING on kfree() in btrfs_free_dummy_fs_info() is still
there.
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply
* Re: [PATCH 01/10] PCI: designware-ep: Add multiple PFs support for DWC
From: Andrew Murray @ 2019-08-16 9:44 UTC (permalink / raw)
To: Xiaowei Bao
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: <AM5PR04MB3299396885AC4B54B9E8D0AEF5AF0@AM5PR04MB3299.eurprd04.prod.outlook.com>
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?
> 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.
> >
> >
> > > +
> > > + 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,
> >
> > 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?
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
> > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists
> > > .infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-kernel&data=02%
> > 7C0
> > >
> > 1%7Cxiaowei.bao%40nxp.com%7C0e39168f6f144db6840308d721742040%7
> > C686ea1d
> > >
> > 3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637014654998524452&sd
> > ata=bP7eh
> > > cjlGXCMVFE2b4f12Q6fGV7lQ%2F5i9qIi9FoPlbI%3D&reserved=0
^ permalink raw reply
* Re: [PATCH 02/10] PCI: designware-ep: Add the doorbell mode of MSI-X in EP mode
From: Andrew Murray @ 2019-08-16 10:20 UTC (permalink / raw)
To: Xiaowei Bao
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
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, Leo Li, M.h. Lian,
robh+dt@kernel.org, linux-arm-kernel@lists.infradead.org,
gustavo.pimentel@synopsys.com, bhelgaas@google.com, kishon@ti.com,
shawnguo@kernel.org, Mingkai Hu
In-Reply-To: <AM5PR04MB329973845D6396624AFDE547F5AF0@AM5PR04MB3299.eurprd04.prod.outlook.com>
On Fri, Aug 16, 2019 at 02:58:31AM +0000, Xiaowei Bao wrote:
>
>
> > -----Original Message-----
> > From: Andrew Murray <andrew.murray@arm.com>
> > Sent: 2019年8月15日 19:54
> > 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 02/10] PCI: designware-ep: Add the doorbell mode of
> > MSI-X in EP mode
> >
> > On Thu, Aug 15, 2019 at 04:37:08PM +0800, Xiaowei Bao wrote:
> > > Add the doorbell mode of MSI-X in EP mode.
> > >
> > > Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> > > ---
> > > drivers/pci/controller/dwc/pcie-designware-ep.c | 14 ++++++++++++++
> > > drivers/pci/controller/dwc/pcie-designware.h | 14 ++++++++++++++
> > > 2 files changed, 28 insertions(+)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > index 75e2955..e3a7cdf 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > @@ -454,6 +454,20 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep
> > *ep, u8 func_no,
> > > return 0;
> > > }
> > >
> > > +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8
> > func_no,
> > > + u16 interrupt_num)
> > > +{
> > > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > + u32 msg_data;
> > > +
> > > + msg_data = (func_no << PCIE_MSIX_DOORBELL_PF_SHIFT) |
> > > + (interrupt_num - 1);
> > > +
> > > + dw_pcie_writel_dbi(pci, PCIE_MSIX_DOORBELL, msg_data);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> > > u16 interrupt_num)
> >
> > Have I understood correctly that the hardware provides an alternative
> > mechanism that allows for raising MSI-X interrupts without the bother of
> > reading the capabilities registers?
> Yes, the hardware provide two way to MSI-X, please check the page 492 of
> DWC_pcie_dm_registers_4.30 Menu.
> MSIX_DOORBELL_OFF on page 492 0x948 Description: MSI-X Doorbell Register....>
Thanks for the reference.
> >
> > If so is there any good reason to keep dw_pcie_ep_raise_msix_irq? (And thus
> > use it in dw_plat_pcie_ep_raise_irq also)?
> I am not sure, but I think the dw_pcie_ep_raise_msix_irq function is not correct,
> because I think we can't get the MSIX table from the address ep->phys_base + tbl_addr,
> but I also don't know where I can get the correct MSIX table.
Well it looks like this function is used by snps,dw-pcie-ep and snps,dw-pcie,
perhaps the doorbell mode isn't available on that hardware.
> >
> >
> > > {
> > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h
> > > b/drivers/pci/controller/dwc/pcie-designware.h
> > > index 2b291e8..cd903e9 100644
> > > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > > @@ -88,6 +88,11 @@
> > > #define PCIE_MISC_CONTROL_1_OFF 0x8BC
> > > #define PCIE_DBI_RO_WR_EN BIT(0)
> > >
> > > +#define PCIE_MSIX_DOORBELL 0x948
> > > +#define PCIE_MSIX_DOORBELL_PF_SHIFT 24
> > > +#define PCIE_MSIX_DOORBELL_VF_SHIFT 16
> > > +#define PCIE_MSIX_DOORBELL_VF_ACTIVE BIT(15)
> >
> > The _VF defines are not used, I'd suggest removing them.
> In fact, I will add the SRIOV support in this file, the SRIOV feature have verified
> In my board, but I need wait the EP framework SRIOV patch merge,
> so I defined these two macros.
I'd suggest adding the VF macros along with the SRIOV feature.
Thanks,
Andrew Murray
> >
> > Thanks,
> >
> > Andrew Murray
> >
> > > +
> > > /*
> > > * iATU Unroll-specific register definitions
> > > * From 4.80 core version the address translation will be made by
> > > unroll @@ -399,6 +404,8 @@ int dw_pcie_ep_raise_msi_irq(struct
> > dw_pcie_ep *ep, u8 func_no,
> > > u8 interrupt_num);
> > > int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> > > u16 interrupt_num);
> > > +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8
> > func_no,
> > > + u16 interrupt_num);
> > > void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar);
> > > #else static inline void dw_pcie_ep_linkup(struct dw_pcie_ep *ep) @@
> > > -431,6 +438,13 @@ static inline int dw_pcie_ep_raise_msix_irq(struct
> > dw_pcie_ep *ep, u8 func_no,
> > > return 0;
> > > }
> > >
> > > +static inline int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep
> > *ep,
> > > + u8 func_no,
> > > + u16 interrupt_num)
> > > +{
> > > + return 0;
> > > +}
> > > +
> > > static inline void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum
> > > pci_barno bar) { }
> > > --
> > > 2.9.5
> > >
> > >
> > > _______________________________________________
> > > linux-arm-kernel mailing list
> > > linux-arm-kernel@lists.infradead.org
> > > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Flists
> > > .infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-kernel&data=02%
> > 7C0
> > >
> > 1%7Cxiaowei.bao%40nxp.com%7C8489493003bb48a0139d08d721773972%
> > 7C686ea1d
> > >
> > 3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637014668369499298&sd
> > ata=dyrXB
> > >
> > avljJBFUSNXW7K%2FRoXvwfWTE%2FoU2KMd1bZkJow%3D&reserved=0
^ permalink raw reply
* Re: [PATCH 05/10] PCI: layerscape: Modify the way of getting capability with different PEX
From: Andrew Murray @ 2019-08-16 10:25 UTC (permalink / raw)
To: Xiaowei Bao
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, Leo Li,
M.h. Lian, devicetree@vger.kernel.org, robh+dt@kernel.org,
linux-arm-kernel@lists.infradead.org,
gustavo.pimentel@synopsys.com, bhelgaas@google.com, kishon@ti.com,
shawnguo@kernel.org, Mingkai Hu
In-Reply-To: <AM5PR04MB329966792C66E9AAB6C0B30DF5AF0@AM5PR04MB3299.eurprd04.prod.outlook.com>
On Fri, Aug 16, 2019 at 03:00:00AM +0000, Xiaowei Bao wrote:
>
>
> > -----Original Message-----
> > From: Andrew Murray <andrew.murray@arm.com>
> > Sent: 2019年8月15日 20:51
> > 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 05/10] PCI: layerscape: Modify the way of getting
> > capability with different PEX
> >
> > On Thu, Aug 15, 2019 at 04:37:11PM +0800, Xiaowei Bao wrote:
> > > The different PCIe controller in one board may be have different
> > > capability of MSI or MSIX, so change the way of getting the MSI
> > > capability, make it more flexible.
> > >
> > > Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> > > ---
> > > drivers/pci/controller/dwc/pci-layerscape-ep.c | 28
> > > +++++++++++++++++++-------
> > > 1 file changed, 21 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > index be61d96..9404ca0 100644
> > > --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > @@ -22,6 +22,7 @@
> > >
> > > struct ls_pcie_ep {
> > > struct dw_pcie *pci;
> > > + struct pci_epc_features *ls_epc;
> > > };
> > >
> > > #define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
> > > @@ -40,25 +41,26 @@ static const struct of_device_id
> > ls_pcie_ep_of_match[] = {
> > > { },
> > > };
> > >
> > > -static const struct pci_epc_features ls_pcie_epc_features = {
> > > - .linkup_notifier = false,
> > > - .msi_capable = true,
> > > - .msix_capable = false,
> > > -};
> > > -
> > > static const struct pci_epc_features* ls_pcie_ep_get_features(struct
> > > dw_pcie_ep *ep) {
> > > - return &ls_pcie_epc_features;
> > > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > + struct ls_pcie_ep *pcie = to_ls_pcie_ep(pci);
> > > +
> > > + return pcie->ls_epc;
> > > }
> > >
> > > static void ls_pcie_ep_init(struct dw_pcie_ep *ep) {
> > > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > + struct ls_pcie_ep *pcie = to_ls_pcie_ep(pci);
> > > enum pci_barno bar;
> > >
> > > for (bar = BAR_0; bar <= BAR_5; bar++)
> > > dw_pcie_ep_reset_bar(pci, bar);
> > > +
> > > + pcie->ls_epc->msi_capable = ep->msi_cap ? true : false;
> > > + pcie->ls_epc->msix_capable = ep->msix_cap ? true : false;
> > > }
> > >
> > > static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no, @@
> > > -118,6 +120,7 @@ static int __init ls_pcie_ep_probe(struct platform_device
> > *pdev)
> > > struct device *dev = &pdev->dev;
> > > struct dw_pcie *pci;
> > > struct ls_pcie_ep *pcie;
> > > + struct pci_epc_features *ls_epc;
> > > struct resource *dbi_base;
> > > int ret;
> > >
> > > @@ -129,6 +132,10 @@ static int __init ls_pcie_ep_probe(struct
> > platform_device *pdev)
> > > if (!pci)
> > > return -ENOMEM;
> > >
> > > + ls_epc = devm_kzalloc(dev, sizeof(*ls_epc), GFP_KERNEL);
> > > + if (!ls_epc)
> > > + return -ENOMEM;
> > > +
> > > dbi_base = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> > "regs");
> > > pci->dbi_base = devm_pci_remap_cfg_resource(dev, dbi_base);
> > > if (IS_ERR(pci->dbi_base))
> > > @@ -139,6 +146,13 @@ static int __init ls_pcie_ep_probe(struct
> > platform_device *pdev)
> > > pci->ops = &ls_pcie_ep_ops;
> > > pcie->pci = pci;
> > >
> > > + ls_epc->linkup_notifier = false,
> > > + ls_epc->msi_capable = true,
> > > + ls_epc->msix_capable = true,
> >
> > As [msi,msix]_capable is shortly set from ls_pcie_ep_init - is there any reason
> > to set them here (to potentially incorrect values)?
> This is a INIT value, maybe false is better for msi_capable and msix_capable,
> of course, we don't need to set it.
ls_epc is kzalloc'd and so all zeros, so you get false for free. I think you
can remove these two lines (or all three if you don't care that linkup_notifier
isn't explicitly set).
Thanks,
Andrew Murray
> >
> > Thanks,
> >
> > Andrew Murray
> >
> > > + ls_epc->bar_fixed_64bit = (1 << BAR_2) | (1 << BAR_4),
> > > +
> > > + pcie->ls_epc = ls_epc;
> > > +
> > > platform_set_drvdata(pdev, pcie);
> > >
> > > ret = ls_add_pcie_ep(pcie, pdev);
> > > --
> > > 2.9.5
> > >
^ permalink raw reply
* Re: [PATCH 02/10] PCI: designware-ep: Add the doorbell mode of MSI-X in EP mode
From: Kishon Vijay Abraham I @ 2019-08-16 10:49 UTC (permalink / raw)
To: Xiaowei Bao, Andrew Murray
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
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, Leo Li, M.h. Lian,
robh+dt@kernel.org, linux-arm-kernel@lists.infradead.org,
gustavo.pimentel@synopsys.com, bhelgaas@google.com,
shawnguo@kernel.org, Mingkai Hu
In-Reply-To: <AM5PR04MB329973845D6396624AFDE547F5AF0@AM5PR04MB3299.eurprd04.prod.outlook.com>
Hi,
On 16/08/19 8:28 AM, Xiaowei Bao wrote:
>
>
>> -----Original Message-----
>> From: Andrew Murray <andrew.murray@arm.com>
>> Sent: 2019年8月15日 19:54
>> 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 02/10] PCI: designware-ep: Add the doorbell mode of
>> MSI-X in EP mode
>>
>> On Thu, Aug 15, 2019 at 04:37:08PM +0800, Xiaowei Bao wrote:
>>> Add the doorbell mode of MSI-X in EP mode.
>>>
>>> Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
>>> ---
>>> drivers/pci/controller/dwc/pcie-designware-ep.c | 14 ++++++++++++++
>>> drivers/pci/controller/dwc/pcie-designware.h | 14 ++++++++++++++
>>> 2 files changed, 28 insertions(+)
>>>
>>> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c
>>> b/drivers/pci/controller/dwc/pcie-designware-ep.c
>>> index 75e2955..e3a7cdf 100644
>>> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
>>> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
>>> @@ -454,6 +454,20 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep
>> *ep, u8 func_no,
>>> return 0;
>>> }
>>>
>>> +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8
>> func_no,
>>> + u16 interrupt_num)
>>> +{
>>> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>>> + u32 msg_data;
>>> +
>>> + msg_data = (func_no << PCIE_MSIX_DOORBELL_PF_SHIFT) |
>>> + (interrupt_num - 1);
>>> +
>>> + dw_pcie_writel_dbi(pci, PCIE_MSIX_DOORBELL, msg_data);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
>>> u16 interrupt_num)
>>
>> Have I understood correctly that the hardware provides an alternative
>> mechanism that allows for raising MSI-X interrupts without the bother of
>> reading the capabilities registers?
> Yes, the hardware provide two way to MSI-X, please check the page 492 of
> DWC_pcie_dm_registers_4.30 Menu.
> MSIX_DOORBELL_OFF on page 492 0x948 Description: MSI-X Doorbell Register....>
>>
>> If so is there any good reason to keep dw_pcie_ep_raise_msix_irq? (And thus
>> use it in dw_plat_pcie_ep_raise_irq also)?
> I am not sure, but I think the dw_pcie_ep_raise_msix_irq function is not correct,
> because I think we can't get the MSIX table from the address ep->phys_base + tbl_addr,
> but I also don't know where I can get the correct MSIX table.
Sometime back when I tried raising MSI-X from EP, it was failing. It's quite
possible dw_pcie_ep_raise_msix_irq function is not correct.
MSI-X table can be obtained from the inbound ATU corresponding to the MSIX bar.
IMO MSI-X support in EP mode needs rework. For instance set_msix should also
take BAR number as input to be configured in the MSI-X capability. The function
driver (pci-epf-test.c) should allocate memory taking into account the MSI-X table.
Thanks
Kishon
^ permalink raw reply
* RE: [PATCH 01/10] PCI: designware-ep: Add multiple PFs support for DWC
From: Xiaowei Bao @ 2019-08-16 11:00 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: <20190816094449.GA14111@e119886-lin.cambridge.arm.com>
> -----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]"
>
>
> > 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?
>
> > >
> > > 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.
>
> 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&data=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
* RE: [PATCH 02/10] PCI: designware-ep: Add the doorbell mode of MSI-X in EP mode
From: Xiaowei Bao @ 2019-08-16 11:01 UTC (permalink / raw)
To: Andrew Murray
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
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, Leo Li, M.h. Lian,
robh+dt@kernel.org, linux-arm-kernel@lists.infradead.org,
gustavo.pimentel@synopsys.com, bhelgaas@google.com, kishon@ti.com,
shawnguo@kernel.org, Mingkai Hu
In-Reply-To: <20190816102025.GB14111@e119886-lin.cambridge.arm.com>
> -----Original Message-----
> From: Andrew Murray <andrew.murray@arm.com>
> Sent: 2019年8月16日 18:20
> 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>;
> 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 02/10] PCI: designware-ep: Add the doorbell mode of
> MSI-X in EP mode
>
> On Fri, Aug 16, 2019 at 02:58:31AM +0000, Xiaowei Bao wrote:
> >
> >
> > > -----Original Message-----
> > > From: Andrew Murray <andrew.murray@arm.com>
> > > Sent: 2019年8月15日 19:54
> > > 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 02/10] PCI: designware-ep: Add the doorbell mode
> > > of MSI-X in EP mode
> > >
> > > On Thu, Aug 15, 2019 at 04:37:08PM +0800, Xiaowei Bao wrote:
> > > > Add the doorbell mode of MSI-X in EP mode.
> > > >
> > > > Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> > > > ---
> > > > drivers/pci/controller/dwc/pcie-designware-ep.c | 14
> ++++++++++++++
> > > > drivers/pci/controller/dwc/pcie-designware.h | 14
> ++++++++++++++
> > > > 2 files changed, 28 insertions(+)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > index 75e2955..e3a7cdf 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > @@ -454,6 +454,20 @@ int dw_pcie_ep_raise_msi_irq(struct
> > > > dw_pcie_ep
> > > *ep, u8 func_no,
> > > > return 0;
> > > > }
> > > >
> > > > +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8
> > > func_no,
> > > > + u16 interrupt_num)
> > > > +{
> > > > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > + u32 msg_data;
> > > > +
> > > > + msg_data = (func_no << PCIE_MSIX_DOORBELL_PF_SHIFT) |
> > > > + (interrupt_num - 1);
> > > > +
> > > > + dw_pcie_writel_dbi(pci, PCIE_MSIX_DOORBELL, msg_data);
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> > > > u16 interrupt_num)
> > >
> > > Have I understood correctly that the hardware provides an
> > > alternative mechanism that allows for raising MSI-X interrupts
> > > without the bother of reading the capabilities registers?
> > Yes, the hardware provide two way to MSI-X, please check the page 492
> > of
> > DWC_pcie_dm_registers_4.30 Menu.
> > MSIX_DOORBELL_OFF on page 492 0x948 Description: MSI-X Doorbell
> > Register....>
>
> Thanks for the reference.
>
> > >
> > > If so is there any good reason to keep dw_pcie_ep_raise_msix_irq?
> > > (And thus use it in dw_plat_pcie_ep_raise_irq also)?
> > I am not sure, but I think the dw_pcie_ep_raise_msix_irq function is
> > not correct, because I think we can't get the MSIX table from the
> > address ep->phys_base + tbl_addr, but I also don't know where I can get the
> correct MSIX table.
>
> Well it looks like this function is used by snps,dw-pcie-ep and snps,dw-pcie,
> perhaps the doorbell mode isn't available on that hardware.
>
> > >
> > >
> > > > {
> > > > diff --git a/drivers/pci/controller/dwc/pcie-designware.h
> > > > b/drivers/pci/controller/dwc/pcie-designware.h
> > > > index 2b291e8..cd903e9 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > > > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > > > @@ -88,6 +88,11 @@
> > > > #define PCIE_MISC_CONTROL_1_OFF 0x8BC
> > > > #define PCIE_DBI_RO_WR_EN BIT(0)
> > > >
> > > > +#define PCIE_MSIX_DOORBELL 0x948
> > > > +#define PCIE_MSIX_DOORBELL_PF_SHIFT 24
> > > > +#define PCIE_MSIX_DOORBELL_VF_SHIFT 16
> > > > +#define PCIE_MSIX_DOORBELL_VF_ACTIVE BIT(15)
> > >
> > > The _VF defines are not used, I'd suggest removing them.
> > In fact, I will add the SRIOV support in this file, the SRIOV feature
> > have verified In my board, but I need wait the EP framework SRIOV
> > patch merge, so I defined these two macros.
>
> I'd suggest adding the VF macros along with the SRIOV feature.
OK, I will remove these two macros. Thanks.
>
> Thanks,
>
> Andrew Murray
>
> > >
> > > Thanks,
> > >
> > > Andrew Murray
> > >
> > > > +
> > > > /*
> > > > * iATU Unroll-specific register definitions
> > > > * From 4.80 core version the address translation will be made by
> > > > unroll @@ -399,6 +404,8 @@ int dw_pcie_ep_raise_msi_irq(struct
> > > dw_pcie_ep *ep, u8 func_no,
> > > > u8 interrupt_num);
> > > > int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> > > > u16 interrupt_num);
> > > > +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8
> > > func_no,
> > > > + u16 interrupt_num);
> > > > void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno
> > > > bar); #else static inline void dw_pcie_ep_linkup(struct
> > > > dw_pcie_ep *ep) @@
> > > > -431,6 +438,13 @@ static inline int
> > > > dw_pcie_ep_raise_msix_irq(struct
> > > dw_pcie_ep *ep, u8 func_no,
> > > > return 0;
> > > > }
> > > >
> > > > +static inline int dw_pcie_ep_raise_msix_irq_doorbell(struct
> > > > +dw_pcie_ep
> > > *ep,
> > > > + u8 func_no,
> > > > + u16 interrupt_num)
> > > > +{
> > > > + return 0;
> > > > +}
> > > > +
> > > > static inline void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum
> > > > pci_barno bar) { }
> > > > --
> > > > 2.9.5
> > > >
> > > >
> > > > _______________________________________________
> > > > linux-arm-kernel mailing list
> > > > linux-arm-kernel@lists.infradead.org
> > > > http://lists
> > > > .infradead.org%2Fmailman%2Flistinfo%2Flinux-arm-kernel&data=0
> 2
> > > > %
> > > 7C0
> > > >
> > >
> 1%7Cxiaowei.bao%40nxp.com%7C8489493003bb48a0139d08d721773972%
> > > 7C686ea1d
> > > >
> > >
> 3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637014668369499298&sd
> > > ata=dyrXB
> > > >
> > >
> avljJBFUSNXW7K%2FRoXvwfWTE%2FoU2KMd1bZkJow%3D&reserved=0
^ permalink raw reply
* RE: [PATCH 05/10] PCI: layerscape: Modify the way of getting capability with different PEX
From: Xiaowei Bao @ 2019-08-16 11:03 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, Leo Li,
M.h. Lian, devicetree@vger.kernel.org, robh+dt@kernel.org,
linux-arm-kernel@lists.infradead.org,
gustavo.pimentel@synopsys.com, bhelgaas@google.com, kishon@ti.com,
shawnguo@kernel.org, Mingkai Hu
In-Reply-To: <20190816102545.GC14111@e119886-lin.cambridge.arm.com>
> -----Original Message-----
> From: Andrew Murray <andrew.murray@arm.com>
> Sent: 2019年8月16日 18:26
> 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; Z.q. Hou
> <zhiqiang.hou@nxp.com>
> Subject: Re: [PATCH 05/10] PCI: layerscape: Modify the way of getting
> capability with different PEX
>
> On Fri, Aug 16, 2019 at 03:00:00AM +0000, Xiaowei Bao wrote:
> >
> >
> > > -----Original Message-----
> > > From: Andrew Murray <andrew.murray@arm.com>
> > > Sent: 2019年8月15日 20:51
> > > 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 05/10] PCI: layerscape: Modify the way of
> > > getting capability with different PEX
> > >
> > > On Thu, Aug 15, 2019 at 04:37:11PM +0800, Xiaowei Bao wrote:
> > > > The different PCIe controller in one board may be have different
> > > > capability of MSI or MSIX, so change the way of getting the MSI
> > > > capability, make it more flexible.
> > > >
> > > > Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> > > > ---
> > > > drivers/pci/controller/dwc/pci-layerscape-ep.c | 28
> > > > +++++++++++++++++++-------
> > > > 1 file changed, 21 insertions(+), 7 deletions(-)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > > b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > > index be61d96..9404ca0 100644
> > > > --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > > +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> > > > @@ -22,6 +22,7 @@
> > > >
> > > > struct ls_pcie_ep {
> > > > struct dw_pcie *pci;
> > > > + struct pci_epc_features *ls_epc;
> > > > };
> > > >
> > > > #define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
> > > > @@ -40,25 +41,26 @@ static const struct of_device_id
> > > ls_pcie_ep_of_match[] = {
> > > > { },
> > > > };
> > > >
> > > > -static const struct pci_epc_features ls_pcie_epc_features = {
> > > > - .linkup_notifier = false,
> > > > - .msi_capable = true,
> > > > - .msix_capable = false,
> > > > -};
> > > > -
> > > > static const struct pci_epc_features*
> > > > ls_pcie_ep_get_features(struct dw_pcie_ep *ep) {
> > > > - return &ls_pcie_epc_features;
> > > > + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > + struct ls_pcie_ep *pcie = to_ls_pcie_ep(pci);
> > > > +
> > > > + return pcie->ls_epc;
> > > > }
> > > >
> > > > static void ls_pcie_ep_init(struct dw_pcie_ep *ep) {
> > > > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > + struct ls_pcie_ep *pcie = to_ls_pcie_ep(pci);
> > > > enum pci_barno bar;
> > > >
> > > > for (bar = BAR_0; bar <= BAR_5; bar++)
> > > > dw_pcie_ep_reset_bar(pci, bar);
> > > > +
> > > > + pcie->ls_epc->msi_capable = ep->msi_cap ? true : false;
> > > > + pcie->ls_epc->msix_capable = ep->msix_cap ? true : false;
> > > > }
> > > >
> > > > static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8
> > > > func_no, @@
> > > > -118,6 +120,7 @@ static int __init ls_pcie_ep_probe(struct
> > > > platform_device
> > > *pdev)
> > > > struct device *dev = &pdev->dev;
> > > > struct dw_pcie *pci;
> > > > struct ls_pcie_ep *pcie;
> > > > + struct pci_epc_features *ls_epc;
> > > > struct resource *dbi_base;
> > > > int ret;
> > > >
> > > > @@ -129,6 +132,10 @@ static int __init ls_pcie_ep_probe(struct
> > > platform_device *pdev)
> > > > if (!pci)
> > > > return -ENOMEM;
> > > >
> > > > + ls_epc = devm_kzalloc(dev, sizeof(*ls_epc), GFP_KERNEL);
> > > > + if (!ls_epc)
> > > > + return -ENOMEM;
> > > > +
> > > > dbi_base = platform_get_resource_byname(pdev,
> IORESOURCE_MEM,
> > > "regs");
> > > > pci->dbi_base = devm_pci_remap_cfg_resource(dev, dbi_base);
> > > > if (IS_ERR(pci->dbi_base))
> > > > @@ -139,6 +146,13 @@ static int __init ls_pcie_ep_probe(struct
> > > platform_device *pdev)
> > > > pci->ops = &ls_pcie_ep_ops;
> > > > pcie->pci = pci;
> > > >
> > > > + ls_epc->linkup_notifier = false,
> > > > + ls_epc->msi_capable = true,
> > > > + ls_epc->msix_capable = true,
> > >
> > > As [msi,msix]_capable is shortly set from ls_pcie_ep_init - is there
> > > any reason to set them here (to potentially incorrect values)?
> > This is a INIT value, maybe false is better for msi_capable and
> > msix_capable, of course, we don't need to set it.
>
> ls_epc is kzalloc'd and so all zeros, so you get false for free. I think you can
> remove these two lines (or all three if you don't care that linkup_notifier isn't
> explicitly set).
Agree, This is correct, thanks a lot.
>
> Thanks,
>
> Andrew Murray
>
> > >
> > > Thanks,
> > >
> > > Andrew Murray
> > >
> > > > + ls_epc->bar_fixed_64bit = (1 << BAR_2) | (1 << BAR_4),
> > > > +
> > > > + pcie->ls_epc = ls_epc;
> > > > +
> > > > platform_set_drvdata(pdev, pcie);
> > > >
> > > > ret = ls_add_pcie_ep(pcie, pdev);
> > > > --
> > > > 2.9.5
> > > >
^ permalink raw reply
* [PATCH] powerpc/32: Add VDSO version of getcpu
From: Christophe Leroy @ 2019-08-16 11:08 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
Commit 18ad51dd342a ("powerpc: Add VDSO version of getcpu") added
getcpu() for PPC64 only, by making use of a user readable general
purpose SPR.
PPC32 doesn't have any such SPR, a full system call can still be
avoided by implementing a fast system call which reads the CPU id
in the task struct and returns immediately without going back in
virtual mode.
Before the patch, vdsotest reported:
getcpu: syscall: 1572 nsec/call
getcpu: libc: 1787 nsec/call
getcpu: vdso: not tested
Now, vdsotest reports:
getcpu: syscall: 1582 nsec/call
getcpu: libc: 667 nsec/call
getcpu: vdso: 368 nsec/call
For non SMP, just return CPU id 0 from the VDSO directly.
PPC32 doesn't support CONFIG_NUMA so NUMA node is always 0.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
arch/powerpc/include/asm/vdso.h | 2 ++
arch/powerpc/kernel/head_32.h | 13 +++++++++++++
arch/powerpc/kernel/head_booke.h | 11 +++++++++++
arch/powerpc/kernel/vdso32/Makefile | 4 +---
arch/powerpc/kernel/vdso32/getcpu.S | 7 +++++++
arch/powerpc/kernel/vdso32/vdso32.lds.S | 2 --
6 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/vdso.h b/arch/powerpc/include/asm/vdso.h
index b5e1f8f8a05c..adb54782df5f 100644
--- a/arch/powerpc/include/asm/vdso.h
+++ b/arch/powerpc/include/asm/vdso.h
@@ -16,6 +16,8 @@
/* Define if 64 bits VDSO has procedure descriptors */
#undef VDS64_HAS_DESCRIPTORS
+#define NR_MAGIC_FAST_VDSO_SYSCALL 0x789a
+
#ifndef __ASSEMBLY__
/* Offsets relative to thread->vdso_base */
diff --git a/arch/powerpc/kernel/head_32.h b/arch/powerpc/kernel/head_32.h
index 4a692553651f..a2e38b59785a 100644
--- a/arch/powerpc/kernel/head_32.h
+++ b/arch/powerpc/kernel/head_32.h
@@ -3,6 +3,8 @@
#define __HEAD_32_H__
#include <asm/ptrace.h> /* for STACK_FRAME_REGS_MARKER */
+#include <asm/vdso.h>
+#include <asm/asm-offsets.h>
/*
* MSR_KERNEL is > 0x8000 on 4xx/Book-E since it include MSR_CE.
@@ -74,7 +76,13 @@
.endm
.macro SYSCALL_ENTRY trapno
+#ifdef CONFIG_SMP
+ cmplwi cr0, r0, NR_MAGIC_FAST_VDSO_SYSCALL
+#endif
mfspr r12,SPRN_SPRG_THREAD
+#ifdef CONFIG_SMP
+ beq- 1f
+#endif
mfcr r10
lwz r11,TASK_STACK-THREAD(r12)
mflr r9
@@ -152,6 +160,11 @@
mtspr SPRN_SRR0,r11
SYNC
RFI /* jump to handler, enable MMU */
+#ifdef CONFIG_SMP
+1:
+ lwz r5, TASK_CPU - THREAD(r12)
+ RFI
+#endif
.endm
/*
diff --git a/arch/powerpc/kernel/head_booke.h b/arch/powerpc/kernel/head_booke.h
index 2ae635df9026..c534e87cac84 100644
--- a/arch/powerpc/kernel/head_booke.h
+++ b/arch/powerpc/kernel/head_booke.h
@@ -3,6 +3,8 @@
#define __HEAD_BOOKE_H__
#include <asm/ptrace.h> /* for STACK_FRAME_REGS_MARKER */
+#include <asm/vdso.h>
+#include <asm/asm-offsets.h>
#include <asm/kvm_asm.h>
#include <asm/kvm_booke_hv_asm.h>
@@ -104,6 +106,10 @@ FTR_SECTION_ELSE
#ifdef CONFIG_KVM_BOOKE_HV
ALT_FTR_SECTION_END_IFSET(CPU_FTR_EMB_HV)
#endif
+#ifdef CONFIG_SMP
+ cmplwi cr0, r0, NR_MAGIC_FAST_VDSO_SYSCALL
+ beq- 1f
+#endif
BOOKE_CLEAR_BTB(r11)
lwz r11, TASK_STACK - THREAD(r10)
rlwinm r12,r12,0,4,2 /* Clear SO bit in CR */
@@ -176,6 +182,11 @@ ALT_FTR_SECTION_END_IFSET(CPU_FTR_EMB_HV)
mtspr SPRN_SRR0,r11
SYNC
RFI /* jump to handler, enable MMU */
+#ifdef CONFIG_SMP
+1:
+ lwz r5, TASK_CPU - THREAD(r10)
+ RFI
+#endif
.endm
/* To handle the additional exception priority levels on 40x and Book-E
diff --git a/arch/powerpc/kernel/vdso32/Makefile b/arch/powerpc/kernel/vdso32/Makefile
index 06f54d947057..e147bbdc12cd 100644
--- a/arch/powerpc/kernel/vdso32/Makefile
+++ b/arch/powerpc/kernel/vdso32/Makefile
@@ -2,9 +2,7 @@
# List of files in the vdso, has to be asm only for now
-obj-vdso32-$(CONFIG_PPC64) = getcpu.o
-obj-vdso32 = sigtramp.o gettimeofday.o datapage.o cacheflush.o note.o \
- $(obj-vdso32-y)
+obj-vdso32 = sigtramp.o gettimeofday.o datapage.o cacheflush.o note.o getcpu.o
# Build rules
diff --git a/arch/powerpc/kernel/vdso32/getcpu.S b/arch/powerpc/kernel/vdso32/getcpu.S
index 63e914539e1a..bd67a0c25c86 100644
--- a/arch/powerpc/kernel/vdso32/getcpu.S
+++ b/arch/powerpc/kernel/vdso32/getcpu.S
@@ -17,7 +17,14 @@
*/
V_FUNCTION_BEGIN(__kernel_getcpu)
.cfi_startproc
+#if defined(CONFIG_PPC64)
mfspr r5,SPRN_SPRG_VDSO_READ
+#elif defined (CONFIG_SMP)*/
+ li r0, NR_MAGIC_FAST_VDSO_SYSCALL
+ sc /* returns cpuid in r5, clobbers cr0 and r10-r13 */
+#else
+ li r5, 0
+#endif
cmpwi cr0,r3,0
cmpwi cr1,r4,0
clrlwi r6,r5,16
diff --git a/arch/powerpc/kernel/vdso32/vdso32.lds.S b/arch/powerpc/kernel/vdso32/vdso32.lds.S
index 099a6db14e67..663880671e20 100644
--- a/arch/powerpc/kernel/vdso32/vdso32.lds.S
+++ b/arch/powerpc/kernel/vdso32/vdso32.lds.S
@@ -152,9 +152,7 @@ VERSION
__kernel_sync_dicache_p5;
__kernel_sigtramp32;
__kernel_sigtramp_rt32;
-#ifdef CONFIG_PPC64
__kernel_getcpu;
-#endif
__kernel_time;
local: *;
--
2.13.3
^ permalink raw reply related
* Oops in blk_mq_get_request() (was Re: ppc64le kernel panic on 5.2.9-rc1)
From: Michael Ellerman @ 2019-08-16 11:14 UTC (permalink / raw)
To: Major Hayden, linuxppc-dev, Jens Axboe, linux-block
In-Reply-To: <2ff51999-cf99-2190-2707-acf2f2ca3dc5@redhat.com>
Major Hayden <major@redhat.com> writes:
> Hello there,
>
> The CKI Project just found a kernel panic while running the blktests
> test suite on stable 5.2.9-rc1[0]. Michael Ellerman requested for this
> list to be copied on these ppc64le failures.
>
> We have some logs[1] for these failures and they start with
> "ppc64le_host_2_Storage_blktests*". We hope this helps!
>
> [0] https://lore.kernel.org/stable/255f9af4-6087-7f56-5860-5aa0397a7631@redhat.com/T/#t
> [1] https://artifacts.cki-project.org/pipelines/100875/logs/
Thanks for the report.
It looks like you tested the stable queue yesterday, which AFAICS
results in the exact same source tree as you tested above, and yet
yesterday you didn't see the failure. So it's intermittent, which is
annoying.
Looking at the oops:
[ 7101.930385] NIP: c00000000067b230 LR: c00000000067b1d4 CTR: c000000000029140
[ 7101.930400] REGS: c00020391ccc35c0 TRAP: 0300 Not tainted (5.2.9-rc1-2440e48.cki)
[ 7101.930413] MSR: 900000000280b033 <SF,HV,VEC,VSX,EE,FP,ME,IR,DR,RI,LE> CR: 44002228 XER: 20040000
[ 7101.930433] CFAR: c0000000001d9e28 DAR: 800a00066b9e7e28 DSISR: 40000000 IRQMASK: 0
GPR00: c00000000067b1d4 c00020391ccc3850 c0000000016cdb00 0000067578a4ab31
GPR04: 0000000000000000 ffffffffffffffff 0000000001f3fb0e 0000000000000001
GPR08: 800a00066b9e7d80 800a00066b9e7d80 0000000000000001 c00800000cd5db88
r9
GPR12: c000000000029140 c000203fff6b9800 0000000000000008 c00800000d5751e0
GPR16: c000203985a8a6c0 c00800000e013278 c000203985a8a700 0000000000000038
GPR20: 0000000000000030 0000000000000028 0000000000000020 fffffffffffff000
GPR24: 0000000000000001 0000000000000400 0000000000000000 0000000000000023
GPR28: 0000000000000000 0000000000000000 c00020391ccc38c8 c000003ef1b00000
[ 7101.930544] NIP [c00000000067b230] blk_mq_get_request+0x260/0x4b0
[ 7101.930557] LR [c00000000067b1d4] blk_mq_get_request+0x204/0x4b0
[ 7101.930569] Call Trace:
[ 7101.930577] [c00020391ccc3850] [c00000000067b1d4] blk_mq_get_request+0x204/0x4b0 (unreliable)
[ 7101.930594] [c00020391ccc38a0] [c00000000067b688] blk_mq_alloc_request_hctx+0x108/0x1b0
[ 7101.930617] [c00020391ccc3910] [c00800000cd51aac] nvme_alloc_request+0x54/0xe0 [nvme_core]
[ 7101.930633] [c00020391ccc3940] [c00800000cd5641c] __nvme_submit_sync_cmd+0x64/0x290 [nvme_core]
[ 7101.930651] [c00020391ccc39c0] [c00800000d571650] nvmf_connect_io_queue+0x148/0x1e0 [nvme_fabrics]
[ 7101.930668] [c00020391ccc3ab0] [c00800000e0106b0] nvme_loop_connect_io_queues+0x98/0xf8 [nvme_loop]
[ 7101.930684] [c00020391ccc3af0] [c00800000e01116c] nvme_loop_create_ctrl+0x434/0x6a0 [nvme_loop]
[ 7101.930700] [c00020391ccc3bd0] [c00800000d5724f0] nvmf_dev_write+0xd38/0x124c [nvme_fabrics]
[ 7101.930719] [c00020391ccc3d60] [c000000000421e58] __vfs_write+0x38/0x70
[ 7101.930731] [c00020391ccc3d80] [c000000000426188] vfs_write+0xd8/0x250
[ 7101.930744] [c00020391ccc3dd0] [c000000000426558] ksys_write+0x78/0x130
[ 7101.930758] [c00020391ccc3e20] [c00000000000bde4] system_call+0x5c/0x70
And then the disassembly:
x = op_is_sync(op)
r27 = op = 0x0000000000000023
c00000000067b1e0: 3e 06 67 57 clrlwi r7,r27,24 # r7 = r27 & REQ_OP_MASK
c00000000067b1e4: 00 00 07 2c cmpwi r7,0 # if r7 == REQ_OP_READ, x = 1 then goto label2
x = 1
c00000000067b1e8: 01 00 20 39 li r9,1
...
r30 = data = c00020391ccc38c8
r8 = data->ctx = 800a00066b9e7d80
c00000000067b208: 18 00 1e e9 ld r8,24(r30)
c00000000067b20c: 18 00 82 41 beq c00000000067b224 <blk_mq_get_request+0x254> -> label2
(op & (REQ_SYNC | REQ_FUA | REQ_PREFLUSH))
c00000000067b210: 1c 05 6a 57 rlwinm r10,r27,0,20,14
c00000000067b214: 68 03 4a 55 rlwinm r10,r10,0,13,20
c00000000067b218: 34 00 4a 7d cntlzw r10,r10
c00000000067b21c: 7e d9 4a 55 rlwinm r10,r10,27,5,31
c00000000067b220: 01 00 49 69 xori r9,r10,1
c00000000067b224: 24 1f 29 79 rldicr r9,r9,3,60 <- label2
r9 = x * 8
c00000000067b228: 01 00 e0 38 li r7,1 # for refcount_set
r9 = data->ctx + x
c00000000067b22c: 14 4a 28 7d add r9,r8,r9
data->ctx->rq_dispatched[op_is_sync(op)]++;
r10 = data->ctx->rq_dispatched[x]
c00000000067b230: a8 00 49 e9 ld r10,168(r9) <- NIP
x++
c00000000067b234: 01 00 4a 39 addi r10,r10,1
data->ctx->rq_dispatched[x] = r10
c00000000067b238: a8 00 49 f9 std r10,168(r9)
refcount_set(&rq->ref, 1);
c00000000067b23c: d4 00 ff 90 stw r7,212(r31)
So we're oopsing at data->ctx->rq_dispatched[op_is_sync(op)]++.
data->ctx looks completely bogus, ie. 800a00066b9e7d80, that's not
anything like a valid kernel address.
And also op doesn't look like a valid op value, it's 0x23, which has no
flag bits set, but also doesn't match any of the values in req_opf.
So I suspect data is pointing somewhere bogus. Or possibly it used to
point at a blk_mq_alloc_data but doesn't anymore.
Why that's happened I have no idea. I can't see any obvious commits in
mainline or stable that mention anything similar, maybe someone on
linux-block recognises it?
cheers
^ permalink raw reply
* RE: [PATCH 02/10] PCI: designware-ep: Add the doorbell mode of MSI-X in EP mode
From: Xiaowei Bao @ 2019-08-16 11:14 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Andrew Murray
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
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, Leo Li, M.h. Lian,
robh+dt@kernel.org, linux-arm-kernel@lists.infradead.org,
gustavo.pimentel@synopsys.com, bhelgaas@google.com,
shawnguo@kernel.org, Mingkai Hu
In-Reply-To: <02cf2f3d-336c-85bb-1fb5-a141c5a9cf79@ti.com>
> -----Original Message-----
> From: Kishon Vijay Abraham I <kishon@ti.com>
> Sent: 2019年8月16日 18:50
> To: Xiaowei Bao <xiaowei.bao@nxp.com>; Andrew Murray
> <andrew.murray@arm.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>;
> lorenzo.pieralisi@arm.com; arnd@arndb.de; gregkh@linuxfoundation.org;
> M.h. Lian <minghuan.lian@nxp.com>; Mingkai Hu <mingkai.hu@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 02/10] PCI: designware-ep: Add the doorbell mode of
> MSI-X in EP mode
>
> Hi,
>
> On 16/08/19 8:28 AM, Xiaowei Bao wrote:
> >
> >
> >> -----Original Message-----
> >> From: Andrew Murray <andrew.murray@arm.com>
> >> Sent: 2019年8月15日 19:54
> >> 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 02/10] PCI: designware-ep: Add the doorbell mode
> >> of MSI-X in EP mode
> >>
> >> On Thu, Aug 15, 2019 at 04:37:08PM +0800, Xiaowei Bao wrote:
> >>> Add the doorbell mode of MSI-X in EP mode.
> >>>
> >>> Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> >>> ---
> >>> drivers/pci/controller/dwc/pcie-designware-ep.c | 14
> ++++++++++++++
> >>> drivers/pci/controller/dwc/pcie-designware.h | 14
> ++++++++++++++
> >>> 2 files changed, 28 insertions(+)
> >>>
> >>> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c
> >>> b/drivers/pci/controller/dwc/pcie-designware-ep.c
> >>> index 75e2955..e3a7cdf 100644
> >>> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> >>> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> >>> @@ -454,6 +454,20 @@ int dw_pcie_ep_raise_msi_irq(struct
> dw_pcie_ep
> >> *ep, u8 func_no,
> >>> return 0;
> >>> }
> >>>
> >>> +int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8
> >> func_no,
> >>> + u16 interrupt_num)
> >>> +{
> >>> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> >>> + u32 msg_data;
> >>> +
> >>> + msg_data = (func_no << PCIE_MSIX_DOORBELL_PF_SHIFT) |
> >>> + (interrupt_num - 1);
> >>> +
> >>> + dw_pcie_writel_dbi(pci, PCIE_MSIX_DOORBELL, msg_data);
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> >>> u16 interrupt_num)
> >>
> >> Have I understood correctly that the hardware provides an alternative
> >> mechanism that allows for raising MSI-X interrupts without the bother
> >> of reading the capabilities registers?
> > Yes, the hardware provide two way to MSI-X, please check the page 492
> > of
> > DWC_pcie_dm_registers_4.30 Menu.
> > MSIX_DOORBELL_OFF on page 492 0x948 Description: MSI-X Doorbell
> > Register....>
> >>
> >> If so is there any good reason to keep dw_pcie_ep_raise_msix_irq?
> >> (And thus use it in dw_plat_pcie_ep_raise_irq also)?
> > I am not sure, but I think the dw_pcie_ep_raise_msix_irq function is
> > not correct, because I think we can't get the MSIX table from the
> > address ep->phys_base + tbl_addr, but I also don't know where I can get the
> correct MSIX table.
>
> Sometime back when I tried raising MSI-X from EP, it was failing. It's quite
> possible dw_pcie_ep_raise_msix_irq function is not correct.
>
> MSI-X table can be obtained from the inbound ATU corresponding to the MSIX
> bar.
> IMO MSI-X support in EP mode needs rework. For instance set_msix should
> also take BAR number as input to be configured in the MSI-X capability. The
> function driver (pci-epf-test.c) should allocate memory taking into account the
> MSI-X table.
Hi Kishon,
Thanks a lot for your explain, yes, we can get the MSI-X table from the inbound ATU of
the MSIX BAR.
>
> Thanks
> Kishon
^ permalink raw reply
* [PATCH v2] powerpc/32: Add VDSO version of getcpu
From: Christophe Leroy @ 2019-08-16 11:16 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
Commit 18ad51dd342a ("powerpc: Add VDSO version of getcpu") added
getcpu() for PPC64 only, by making use of a user readable general
purpose SPR.
PPC32 doesn't have any such SPR, a full system call can still be
avoided by implementing a fast system call which reads the CPU id
in the task struct and returns immediately without going back in
virtual mode.
Before the patch, vdsotest reported:
getcpu: syscall: 1572 nsec/call
getcpu: libc: 1787 nsec/call
getcpu: vdso: not tested
Now, vdsotest reports:
getcpu: syscall: 1582 nsec/call
getcpu: libc: 667 nsec/call
getcpu: vdso: 368 nsec/call
For non SMP, just return CPU id 0 from the VDSO directly.
PPC32 doesn't support CONFIG_NUMA so NUMA node is always 0.
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
v2: fixed build error in getcpu.S
---
arch/powerpc/include/asm/vdso.h | 2 ++
arch/powerpc/kernel/head_32.h | 13 +++++++++++++
arch/powerpc/kernel/head_booke.h | 11 +++++++++++
arch/powerpc/kernel/vdso32/Makefile | 4 +---
arch/powerpc/kernel/vdso32/getcpu.S | 7 +++++++
arch/powerpc/kernel/vdso32/vdso32.lds.S | 2 --
6 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/vdso.h b/arch/powerpc/include/asm/vdso.h
index b5e1f8f8a05c..adb54782df5f 100644
--- a/arch/powerpc/include/asm/vdso.h
+++ b/arch/powerpc/include/asm/vdso.h
@@ -16,6 +16,8 @@
/* Define if 64 bits VDSO has procedure descriptors */
#undef VDS64_HAS_DESCRIPTORS
+#define NR_MAGIC_FAST_VDSO_SYSCALL 0x789a
+
#ifndef __ASSEMBLY__
/* Offsets relative to thread->vdso_base */
diff --git a/arch/powerpc/kernel/head_32.h b/arch/powerpc/kernel/head_32.h
index 4a692553651f..a2e38b59785a 100644
--- a/arch/powerpc/kernel/head_32.h
+++ b/arch/powerpc/kernel/head_32.h
@@ -3,6 +3,8 @@
#define __HEAD_32_H__
#include <asm/ptrace.h> /* for STACK_FRAME_REGS_MARKER */
+#include <asm/vdso.h>
+#include <asm/asm-offsets.h>
/*
* MSR_KERNEL is > 0x8000 on 4xx/Book-E since it include MSR_CE.
@@ -74,7 +76,13 @@
.endm
.macro SYSCALL_ENTRY trapno
+#ifdef CONFIG_SMP
+ cmplwi cr0, r0, NR_MAGIC_FAST_VDSO_SYSCALL
+#endif
mfspr r12,SPRN_SPRG_THREAD
+#ifdef CONFIG_SMP
+ beq- 1f
+#endif
mfcr r10
lwz r11,TASK_STACK-THREAD(r12)
mflr r9
@@ -152,6 +160,11 @@
mtspr SPRN_SRR0,r11
SYNC
RFI /* jump to handler, enable MMU */
+#ifdef CONFIG_SMP
+1:
+ lwz r5, TASK_CPU - THREAD(r12)
+ RFI
+#endif
.endm
/*
diff --git a/arch/powerpc/kernel/head_booke.h b/arch/powerpc/kernel/head_booke.h
index 2ae635df9026..c534e87cac84 100644
--- a/arch/powerpc/kernel/head_booke.h
+++ b/arch/powerpc/kernel/head_booke.h
@@ -3,6 +3,8 @@
#define __HEAD_BOOKE_H__
#include <asm/ptrace.h> /* for STACK_FRAME_REGS_MARKER */
+#include <asm/vdso.h>
+#include <asm/asm-offsets.h>
#include <asm/kvm_asm.h>
#include <asm/kvm_booke_hv_asm.h>
@@ -104,6 +106,10 @@ FTR_SECTION_ELSE
#ifdef CONFIG_KVM_BOOKE_HV
ALT_FTR_SECTION_END_IFSET(CPU_FTR_EMB_HV)
#endif
+#ifdef CONFIG_SMP
+ cmplwi cr0, r0, NR_MAGIC_FAST_VDSO_SYSCALL
+ beq- 1f
+#endif
BOOKE_CLEAR_BTB(r11)
lwz r11, TASK_STACK - THREAD(r10)
rlwinm r12,r12,0,4,2 /* Clear SO bit in CR */
@@ -176,6 +182,11 @@ ALT_FTR_SECTION_END_IFSET(CPU_FTR_EMB_HV)
mtspr SPRN_SRR0,r11
SYNC
RFI /* jump to handler, enable MMU */
+#ifdef CONFIG_SMP
+1:
+ lwz r5, TASK_CPU - THREAD(r10)
+ RFI
+#endif
.endm
/* To handle the additional exception priority levels on 40x and Book-E
diff --git a/arch/powerpc/kernel/vdso32/Makefile b/arch/powerpc/kernel/vdso32/Makefile
index 06f54d947057..e147bbdc12cd 100644
--- a/arch/powerpc/kernel/vdso32/Makefile
+++ b/arch/powerpc/kernel/vdso32/Makefile
@@ -2,9 +2,7 @@
# List of files in the vdso, has to be asm only for now
-obj-vdso32-$(CONFIG_PPC64) = getcpu.o
-obj-vdso32 = sigtramp.o gettimeofday.o datapage.o cacheflush.o note.o \
- $(obj-vdso32-y)
+obj-vdso32 = sigtramp.o gettimeofday.o datapage.o cacheflush.o note.o getcpu.o
# Build rules
diff --git a/arch/powerpc/kernel/vdso32/getcpu.S b/arch/powerpc/kernel/vdso32/getcpu.S
index 63e914539e1a..bde226ad904d 100644
--- a/arch/powerpc/kernel/vdso32/getcpu.S
+++ b/arch/powerpc/kernel/vdso32/getcpu.S
@@ -17,7 +17,14 @@
*/
V_FUNCTION_BEGIN(__kernel_getcpu)
.cfi_startproc
+#if defined(CONFIG_PPC64)
mfspr r5,SPRN_SPRG_VDSO_READ
+#elif defined(CONFIG_SMP)
+ li r0, NR_MAGIC_FAST_VDSO_SYSCALL
+ sc /* returns cpuid in r5, clobbers cr0 and r10-r13 */
+#else
+ li r5, 0
+#endif
cmpwi cr0,r3,0
cmpwi cr1,r4,0
clrlwi r6,r5,16
diff --git a/arch/powerpc/kernel/vdso32/vdso32.lds.S b/arch/powerpc/kernel/vdso32/vdso32.lds.S
index 099a6db14e67..663880671e20 100644
--- a/arch/powerpc/kernel/vdso32/vdso32.lds.S
+++ b/arch/powerpc/kernel/vdso32/vdso32.lds.S
@@ -152,9 +152,7 @@ VERSION
__kernel_sync_dicache_p5;
__kernel_sigtramp32;
__kernel_sigtramp_rt32;
-#ifdef CONFIG_PPC64
__kernel_getcpu;
-#endif
__kernel_time;
local: *;
--
2.13.3
^ permalink raw reply related
* Applied "ASoC: imx-audmux: Add driver suspend and resume to support MEGA Fast" to the asoc tree
From: Mark Brown @ 2019-08-16 12:14 UTC (permalink / raw)
To: Shengjiu Wang
Cc: alsa-devel, timur, Xiubo.Lee, linuxppc-dev, s.hauer, tiwai,
lgirdwood, linux-kernel, nicoleotsuka, Mark Brown, linux-imx,
kernel, shawnguo, perex, festevam, linux-arm-kernel
In-Reply-To: <1565931794-7218-1-git-send-email-shengjiu.wang@nxp.com>
The patch
ASoC: imx-audmux: Add driver suspend and resume to support MEGA Fast
has been applied to the asoc tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.3
All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying
to this mail.
Thanks,
Mark
From 8661ab5b23d6d30d8687fc05bc1dba8f9a64b444 Mon Sep 17 00:00:00 2001
From: Shengjiu Wang <shengjiu.wang@nxp.com>
Date: Fri, 16 Aug 2019 01:03:14 -0400
Subject: [PATCH] ASoC: imx-audmux: Add driver suspend and resume to support
MEGA Fast
For i.MX6 SoloX, there is a mode of the SoC to shutdown all power
source of modules during system suspend and resume procedure.
Thus, AUDMUX needs to save all the values of registers before the
system suspend and restore them after the system resume.
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Link: https://lore.kernel.org/r/1565931794-7218-1-git-send-email-shengjiu.wang@nxp.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
sound/soc/fsl/imx-audmux.c | 54 +++++++++++++++++++++++++++++++++++++-
1 file changed, 53 insertions(+), 1 deletion(-)
diff --git a/sound/soc/fsl/imx-audmux.c b/sound/soc/fsl/imx-audmux.c
index b2351cd33b0f..16ede3b5cb32 100644
--- a/sound/soc/fsl/imx-audmux.c
+++ b/sound/soc/fsl/imx-audmux.c
@@ -23,6 +23,8 @@
static struct clk *audmux_clk;
static void __iomem *audmux_base;
+static u32 *regcache;
+static u32 reg_max;
#define IMX_AUDMUX_V2_PTCR(x) ((x) * 8)
#define IMX_AUDMUX_V2_PDCR(x) ((x) * 8 + 4)
@@ -317,8 +319,23 @@ static int imx_audmux_probe(struct platform_device *pdev)
if (of_id)
pdev->id_entry = of_id->data;
audmux_type = pdev->id_entry->driver_data;
- if (audmux_type == IMX31_AUDMUX)
+
+ switch (audmux_type) {
+ case IMX31_AUDMUX:
audmux_debugfs_init();
+ reg_max = 14;
+ break;
+ case IMX21_AUDMUX:
+ reg_max = 6;
+ break;
+ default:
+ dev_err(&pdev->dev, "unsupported version!\n");
+ return -EINVAL;
+ }
+
+ regcache = devm_kzalloc(&pdev->dev, sizeof(u32) * reg_max, GFP_KERNEL);
+ if (!regcache)
+ return -ENOMEM;
if (of_id)
imx_audmux_parse_dt_defaults(pdev, pdev->dev.of_node);
@@ -334,12 +351,47 @@ static int imx_audmux_remove(struct platform_device *pdev)
return 0;
}
+#ifdef CONFIG_PM_SLEEP
+static int imx_audmux_suspend(struct device *dev)
+{
+ int i;
+
+ clk_prepare_enable(audmux_clk);
+
+ for (i = 0; i < reg_max; i++)
+ regcache[i] = readl(audmux_base + i * 4);
+
+ clk_disable_unprepare(audmux_clk);
+
+ return 0;
+}
+
+static int imx_audmux_resume(struct device *dev)
+{
+ int i;
+
+ clk_prepare_enable(audmux_clk);
+
+ for (i = 0; i < reg_max; i++)
+ writel(regcache[i], audmux_base + i * 4);
+
+ clk_disable_unprepare(audmux_clk);
+
+ return 0;
+}
+#endif /* CONFIG_PM_SLEEP */
+
+static const struct dev_pm_ops imx_audmux_pm = {
+ SET_SYSTEM_SLEEP_PM_OPS(imx_audmux_suspend, imx_audmux_resume)
+};
+
static struct platform_driver imx_audmux_driver = {
.probe = imx_audmux_probe,
.remove = imx_audmux_remove,
.id_table = imx_audmux_ids,
.driver = {
.name = DRIVER_NAME,
+ .pm = &imx_audmux_pm,
.of_match_table = imx_audmux_dt_ids,
}
};
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] ASoC: imx-audmux: Add driver suspend and resume to support MEGA Fast
From: Mark Brown @ 2019-08-16 12:16 UTC (permalink / raw)
To: Shengjiu Wang
Cc: linux-arm-kernel, alsa-devel, linuxppc-dev, timur, Xiubo.Lee,
shawnguo, s.hauer, tiwai, lgirdwood, perex, nicoleotsuka,
linux-imx, kernel, festevam, linux-kernel
In-Reply-To: <1565931794-7218-1-git-send-email-shengjiu.wang@nxp.com>
[-- Attachment #1: Type: text/plain, Size: 217 bytes --]
On Fri, Aug 16, 2019 at 01:03:14AM -0400, Shengjiu Wang wrote:
> + for (i = 0; i < reg_max; i++)
> + regcache[i] = readl(audmux_base + i * 4);
If only there were some framework which provided a register cache! :P
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: Oops in blk_mq_get_request() (was Re: ppc64le kernel panic on 5.2.9-rc1)
From: Ming Lei @ 2019-08-16 12:34 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Major Hayden, Jens Axboe, linuxppc-dev, linux-block
In-Reply-To: <87blwp746d.fsf@concordia.ellerman.id.au>
On Fri, Aug 16, 2019 at 7:15 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>
> Major Hayden <major@redhat.com> writes:
> > Hello there,
> >
> > The CKI Project just found a kernel panic while running the blktests
> > test suite on stable 5.2.9-rc1[0]. Michael Ellerman requested for this
> > list to be copied on these ppc64le failures.
> >
> > We have some logs[1] for these failures and they start with
> > "ppc64le_host_2_Storage_blktests*". We hope this helps!
> >
> > [0] https://lore.kernel.org/stable/255f9af4-6087-7f56-5860-5aa0397a7631@redhat.com/T/#t
> > [1] https://artifacts.cki-project.org/pipelines/100875/logs/
>
> Thanks for the report.
>
> It looks like you tested the stable queue yesterday, which AFAICS
> results in the exact same source tree as you tested above, and yet
> yesterday you didn't see the failure. So it's intermittent, which is
> annoying.
>
> Looking at the oops:
>
> [ 7101.930385] NIP: c00000000067b230 LR: c00000000067b1d4 CTR: c000000000029140
> [ 7101.930400] REGS: c00020391ccc35c0 TRAP: 0300 Not tainted (5.2.9-rc1-2440e48.cki)
> [ 7101.930413] MSR: 900000000280b033 <SF,HV,VEC,VSX,EE,FP,ME,IR,DR,RI,LE> CR: 44002228 XER: 20040000
> [ 7101.930433] CFAR: c0000000001d9e28 DAR: 800a00066b9e7e28 DSISR: 40000000 IRQMASK: 0
> GPR00: c00000000067b1d4 c00020391ccc3850 c0000000016cdb00 0000067578a4ab31
> GPR04: 0000000000000000 ffffffffffffffff 0000000001f3fb0e 0000000000000001
> GPR08: 800a00066b9e7d80 800a00066b9e7d80 0000000000000001 c00800000cd5db88
> r9
> GPR12: c000000000029140 c000203fff6b9800 0000000000000008 c00800000d5751e0
> GPR16: c000203985a8a6c0 c00800000e013278 c000203985a8a700 0000000000000038
> GPR20: 0000000000000030 0000000000000028 0000000000000020 fffffffffffff000
> GPR24: 0000000000000001 0000000000000400 0000000000000000 0000000000000023
> GPR28: 0000000000000000 0000000000000000 c00020391ccc38c8 c000003ef1b00000
> [ 7101.930544] NIP [c00000000067b230] blk_mq_get_request+0x260/0x4b0
> [ 7101.930557] LR [c00000000067b1d4] blk_mq_get_request+0x204/0x4b0
> [ 7101.930569] Call Trace:
> [ 7101.930577] [c00020391ccc3850] [c00000000067b1d4] blk_mq_get_request+0x204/0x4b0 (unreliable)
> [ 7101.930594] [c00020391ccc38a0] [c00000000067b688] blk_mq_alloc_request_hctx+0x108/0x1b0
> [ 7101.930617] [c00020391ccc3910] [c00800000cd51aac] nvme_alloc_request+0x54/0xe0 [nvme_core]
> [ 7101.930633] [c00020391ccc3940] [c00800000cd5641c] __nvme_submit_sync_cmd+0x64/0x290 [nvme_core]
> [ 7101.930651] [c00020391ccc39c0] [c00800000d571650] nvmf_connect_io_queue+0x148/0x1e0 [nvme_fabrics]
> [ 7101.930668] [c00020391ccc3ab0] [c00800000e0106b0] nvme_loop_connect_io_queues+0x98/0xf8 [nvme_loop]
> [ 7101.930684] [c00020391ccc3af0] [c00800000e01116c] nvme_loop_create_ctrl+0x434/0x6a0 [nvme_loop]
> [ 7101.930700] [c00020391ccc3bd0] [c00800000d5724f0] nvmf_dev_write+0xd38/0x124c [nvme_fabrics]
> [ 7101.930719] [c00020391ccc3d60] [c000000000421e58] __vfs_write+0x38/0x70
> [ 7101.930731] [c00020391ccc3d80] [c000000000426188] vfs_write+0xd8/0x250
> [ 7101.930744] [c00020391ccc3dd0] [c000000000426558] ksys_write+0x78/0x130
> [ 7101.930758] [c00020391ccc3e20] [c00000000000bde4] system_call+0x5c/0x70
>
> And then the disassembly:
>
> x = op_is_sync(op)
> r27 = op = 0x0000000000000023
> c00000000067b1e0: 3e 06 67 57 clrlwi r7,r27,24 # r7 = r27 & REQ_OP_MASK
>
> c00000000067b1e4: 00 00 07 2c cmpwi r7,0 # if r7 == REQ_OP_READ, x = 1 then goto label2
> x = 1
> c00000000067b1e8: 01 00 20 39 li r9,1
>
> ...
>
> r30 = data = c00020391ccc38c8
> r8 = data->ctx = 800a00066b9e7d80
> c00000000067b208: 18 00 1e e9 ld r8,24(r30)
> c00000000067b20c: 18 00 82 41 beq c00000000067b224 <blk_mq_get_request+0x254> -> label2
>
> (op & (REQ_SYNC | REQ_FUA | REQ_PREFLUSH))
> c00000000067b210: 1c 05 6a 57 rlwinm r10,r27,0,20,14
> c00000000067b214: 68 03 4a 55 rlwinm r10,r10,0,13,20
> c00000000067b218: 34 00 4a 7d cntlzw r10,r10
> c00000000067b21c: 7e d9 4a 55 rlwinm r10,r10,27,5,31
> c00000000067b220: 01 00 49 69 xori r9,r10,1
>
> c00000000067b224: 24 1f 29 79 rldicr r9,r9,3,60 <- label2
> r9 = x * 8
>
> c00000000067b228: 01 00 e0 38 li r7,1 # for refcount_set
>
> r9 = data->ctx + x
> c00000000067b22c: 14 4a 28 7d add r9,r8,r9
>
> data->ctx->rq_dispatched[op_is_sync(op)]++;
>
> r10 = data->ctx->rq_dispatched[x]
> c00000000067b230: a8 00 49 e9 ld r10,168(r9) <- NIP
>
> x++
> c00000000067b234: 01 00 4a 39 addi r10,r10,1
>
> data->ctx->rq_dispatched[x] = r10
> c00000000067b238: a8 00 49 f9 std r10,168(r9)
>
> refcount_set(&rq->ref, 1);
> c00000000067b23c: d4 00 ff 90 stw r7,212(r31)
>
>
> So we're oopsing at data->ctx->rq_dispatched[op_is_sync(op)]++.
>
> data->ctx looks completely bogus, ie. 800a00066b9e7d80, that's not
> anything like a valid kernel address.
>
> And also op doesn't look like a valid op value, it's 0x23, which has no
> flag bits set, but also doesn't match any of the values in req_opf.
>
> So I suspect data is pointing somewhere bogus. Or possibly it used to
> point at a blk_mq_alloc_data but doesn't anymore.
>
> Why that's happened I have no idea. I can't see any obvious commits in
> mainline or stable that mention anything similar, maybe someone on
> linux-block recognises it?
>
> cheers
Please try:
https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git/commit/?h=for-5.4/block&id=556f36e90dbe7dded81f4fac084d2bc8a2458330
Strictly speaking, it is still a workaround, but it works in case that
CPU hotplug isn't
involved.
Thanks,
Ming Lei
^ permalink raw reply
* Re: [PATCH 01/10] PCI: designware-ep: Add multiple PFs support for DWC
From: Andrew Murray @ 2019-08-16 12:35 UTC (permalink / raw)
To: Xiaowei Bao
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: <AM5PR04MB32997C516770A0555970330BF5AF0@AM5PR04MB3299.eurprd04.prod.outlook.com>
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.
> >
> >
> > > 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).
> >
> > > >
> > > > 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).
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&data=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
* Re: [PATCH] powerpc/futex: fix warning: 'oldval' may be used uninitialized in this function
From: Michael Ellerman @ 2019-08-16 12:35 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <86b72f0c134367b214910b27b9a6dd3321af93bb.1565774657.git.christophe.leroy@c-s.fr>
Christophe Leroy <christophe.leroy@c-s.fr> writes:
> CC kernel/futex.o
> kernel/futex.c: In function 'do_futex':
> kernel/futex.c:1676:17: warning: 'oldval' may be used uninitialized in this function [-Wmaybe-uninitialized]
> return oldval == cmparg;
> ^
> kernel/futex.c:1651:6: note: 'oldval' was declared here
> int oldval, ret;
> ^
>
> This is because arch_futex_atomic_op_inuser() only sets *oval
> if ret is NUL and GCC doesn't see that it will use it only when
I prefer 0 to "NUL", as ret is an int. I'll reword it. But otherwise
this looks OK.
cheers
> ret is NUL.
>
> Anyway, the non-NUL ret path is an error path that won't suffer from
> setting *oval, and as *oval is a local var in futex_atomic_op_inuser()
> it will have no impact.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> ---
> arch/powerpc/include/asm/futex.h | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/futex.h b/arch/powerpc/include/asm/futex.h
> index 3a6aa57b9d90..eea28ca679db 100644
> --- a/arch/powerpc/include/asm/futex.h
> +++ b/arch/powerpc/include/asm/futex.h
> @@ -60,8 +60,7 @@ static inline int arch_futex_atomic_op_inuser(int op, int oparg, int *oval,
>
> pagefault_enable();
>
> - if (!ret)
> - *oval = oldval;
> + *oval = oldval;
>
> prevent_write_to_user(uaddr, sizeof(*uaddr));
> return ret;
> --
> 2.13.3
^ permalink raw reply
* Re: 5.2.7 kernel doesn't boot on G5
From: Christian Marillat @ 2019-08-16 13:00 UTC (permalink / raw)
To: christophe leroy; +Cc: Mathieu Malaterre, Christian Marillat, linuxppc-dev
In-Reply-To: <e582fcf0-a311-07f7-5445-c3471ec5c783@c-s.fr>
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 :
https://www.deb-multimedia.org/tests/20190816_142333.jpg
Christian
^ permalink raw reply
* [PATCH] powerpc/vdso32: Add support for CLOCK_{REALTIME/MONOTONIC}_COARSE
From: Christophe Leroy @ 2019-08-16 13:01 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: Naveen N. Rao, Santosh Sivaraj, linuxppc-dev, linux-kernel
This is copied and adapted from commit 5c929885f1bb ("powerpc/vdso64:
Add support for CLOCK_{REALTIME/MONOTONIC}_COARSE")
from Santosh Sivaraj <santosh@fossix.org>
Benchmark from vdsotest:
clock-gettime-realtime: syscall: 3601 nsec/call
clock-gettime-realtime: libc: 1072 nsec/call
clock-gettime-realtime: vdso: 931 nsec/call
clock-gettime-monotonic: syscall: 4034 nsec/call
clock-gettime-monotonic: libc: 1213 nsec/call
clock-gettime-monotonic: vdso: 1076 nsec/call
clock-gettime-realtime-coarse: syscall: 2722 nsec/call
clock-gettime-realtime-coarse: libc: 805 nsec/call
clock-gettime-realtime-coarse: vdso: 668 nsec/call
clock-gettime-monotonic-coarse: syscall: 2949 nsec/call
clock-gettime-monotonic-coarse: libc: 882 nsec/call
clock-gettime-monotonic-coarse: vdso: 745 nsec/call
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Cc: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Cc: Santosh Sivaraj <santosh@fossix.org>
Link: https://github.com/linuxppc/issues/issues/41
---
arch/powerpc/kernel/vdso32/gettimeofday.S | 48 ++++++++++++++++++++++++++++++-
1 file changed, 47 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/vdso32/gettimeofday.S b/arch/powerpc/kernel/vdso32/gettimeofday.S
index becd9f8767ed..e10098cde89c 100644
--- a/arch/powerpc/kernel/vdso32/gettimeofday.S
+++ b/arch/powerpc/kernel/vdso32/gettimeofday.S
@@ -71,6 +71,12 @@ V_FUNCTION_BEGIN(__kernel_clock_gettime)
cmpli cr0,r3,CLOCK_REALTIME
cmpli cr1,r3,CLOCK_MONOTONIC
cror cr0*4+eq,cr0*4+eq,cr1*4+eq
+
+ cmpli cr5,r3,CLOCK_REALTIME_COARSE
+ cmpli cr6,r3,CLOCK_MONOTONIC_COARSE
+ cror cr5*4+eq,cr5*4+eq,cr6*4+eq
+
+ cror cr0*4+eq,cr0*4+eq,cr5*4+eq
bne cr0,99f
mflr r12 /* r12 saves lr */
@@ -80,6 +86,7 @@ V_FUNCTION_BEGIN(__kernel_clock_gettime)
mr r9,r3 /* datapage ptr in r9 */
lis r7,NSEC_PER_SEC@h /* want nanoseconds */
ori r7,r7,NSEC_PER_SEC@l
+ beq cr5,70f
50: bl __do_get_tspec@local /* get sec/nsec from tb & kernel */
bne cr1,80f /* not monotonic -> all done */
@@ -106,12 +113,51 @@ V_FUNCTION_BEGIN(__kernel_clock_gettime)
lwz r0,(CFG_TB_UPDATE_COUNT+LOPART)(r9)
cmpl cr0,r8,r0 /* check if updated */
bne- 50b
+ b 78f
+
+ /*
+ * For coarse clocks we get data directly from the vdso data page, so
+ * we don't need to call __do_get_tspec, but we still need to do the
+ * counter trick.
+ */
+70: lwz r8,(CFG_TB_UPDATE_COUNT+LOPART)(r9)
+ andi. r0,r8,1 /* pending update ? loop */
+ bne- 70b
+ add r9,r9,r0 /* r0 is already 0 */
+
+ /*
+ * CLOCK_REALTIME_COARSE, below values are needed for MONOTONIC_COARSE
+ * too
+ */
+ lwz r3,STAMP_XTIME+TSPC32_TV_SEC(r9)
+ lwz r4,STAMP_XTIME+TSPC32_TV_NSEC(r9)
+ bne cr6,75f
+
+ /* CLOCK_MONOTONIC_COARSE */
+ lwz r5,(WTOM_CLOCK_SEC+LOPART)(r9)
+ lwz r6,WTOM_CLOCK_NSEC(r9)
+
+ /* check if counter has updated */
+ or r0,r6,r5
+75: or r0,r0,r3
+ or r0,r0,r4
+ xor r0,r0,r0
+ add r3,r3,r0
+ lwz r0,CFG_TB_UPDATE_COUNT+LOPART(r9)
+ cmpl cr0,r0,r8 /* check if updated */
+ bne- 70b
+
+ /* Counter has not updated, so continue calculating proper values for
+ * sec and nsec if monotonic coarse, or just return with the proper
+ * values for realtime.
+ */
+ bne cr6,80f
/* Calculate and store result. Note that this mimics the C code,
* which may cause funny results if nsec goes negative... is that
* possible at all ?
*/
- add r3,r3,r5
+78: add r3,r3,r5
add r4,r4,r6
cmpw cr0,r4,r7
cmpwi cr1,r4,0
--
2.13.3
^ permalink raw reply related
* [PATCH] crypto: vmx/xts - use fallback for ciphertext stealing
From: Ard Biesheuvel @ 2019-08-16 14:06 UTC (permalink / raw)
To: linux-crypto
Cc: herbert, Ard Biesheuvel, nayna, ebiggers, pfsmorigo, leitao,
linuxppc-dev
For correctness and compliance with the XTS-AES specification, we are
adding support for ciphertext stealing to XTS implementations, even
though no use cases are known that will be enabled by this.
Since the Power8 implementation already has a fallback skcipher standby
for other purposes, let's use it for this purpose as well. If ciphertext
stealing use cases ever become a bottleneck, we can always revisit this.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
drivers/crypto/vmx/aes_xts.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/crypto/vmx/aes_xts.c b/drivers/crypto/vmx/aes_xts.c
index 49f7258045fa..d59e736882f6 100644
--- a/drivers/crypto/vmx/aes_xts.c
+++ b/drivers/crypto/vmx/aes_xts.c
@@ -84,7 +84,7 @@ static int p8_aes_xts_crypt(struct skcipher_request *req, int enc)
u8 tweak[AES_BLOCK_SIZE];
int ret;
- if (!crypto_simd_usable()) {
+ if (!crypto_simd_usable() || (req->cryptlen % XTS_BLOCK_SIZE) != 0) {
struct skcipher_request *subreq = skcipher_request_ctx(req);
*subreq = *req;
--
2.17.1
^ permalink raw reply related
* Re: 5.2.7 kernel doesn't boot on G5
From: Andreas Schwab @ 2019-08-16 14:05 UTC (permalink / raw)
To: Christian Marillat; +Cc: Mathieu Malaterre, linuxppc-dev
In-Reply-To: <87zhk9tgdb.fsf__22033.2117214397$1565960670$gmane$org@christian.marillat.net>
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).
Andreas.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply
* [PATCH] powerpc: Set right value of Speculation_Store_Bypass in /proc/<pid>/status
From: Gustavo Walbon @ 2019-08-16 14:30 UTC (permalink / raw)
To: benh, paulus, mpe
Cc: mikey, jkosina, linux-kernel, diana.craciun, gustavowalbon,
jpoimboe, leitao, tglx, linuxppc-dev
The issue has showed the value of status of Speculation_Store_Bypass in the
/proc/<pid>/status as `unknown` for PowerPC systems.
The patch fix the checking of the mitigation status of Speculation, and
can be reported as "not vulnerable", "globally mitigated" or "vulnerable".
Link: https://github.com/linuxppc/issues/issues/255
Signed-off-by: Gustavo Walbon <gwalbon@linux.ibm.com>
---
arch/powerpc/kernel/security.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/security.c b/arch/powerpc/kernel/security.c
index e1c9cf079503..754ae4238d4e 100644
--- a/arch/powerpc/kernel/security.c
+++ b/arch/powerpc/kernel/security.c
@@ -14,7 +14,7 @@
#include <asm/debugfs.h>
#include <asm/security_features.h>
#include <asm/setup.h>
-
+#include <linux/prctl.h>
unsigned long powerpc_security_features __read_mostly = SEC_FTR_DEFAULT;
@@ -339,6 +339,29 @@ ssize_t cpu_show_spec_store_bypass(struct device *dev, struct device_attribute *
return sprintf(buf, "Vulnerable\n");
}
+static int ssb_prctl_get(struct task_struct *task)
+{
+ if (stf_barrier) {
+ if (stf_enabled_flush_types == STF_BARRIER_NONE)
+ return PR_SPEC_NOT_AFFECTED;
+ else
+ return PR_SPEC_DISABLE;
+ } else
+ return PR_SPEC_DISABLE_NOEXEC;
+
+ return -EINVAL;
+}
+
+int arch_prctl_spec_ctrl_get(struct task_struct *task, unsigned long which)
+{
+ switch (which) {
+ case PR_SPEC_STORE_BYPASS:
+ return ssb_prctl_get(task);
+ default:
+ return -ENODEV;
+ }
+}
+
#ifdef CONFIG_DEBUG_FS
static int stf_barrier_set(void *data, u64 val)
{
--
2.19.1
^ permalink raw reply related
* [Bug 204371] BUG kmalloc-4k (Tainted: G W ): Object padding overwritten
From: bugzilla-daemon @ 2019-08-16 14:35 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 #33 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) ---
> I think first thing is to fix test_add_free_space_entry() :
> - replace the map = kzalloc(...) by map = (void *)get_zeroed_page(...) like
> in
> other places.
> - replace the kfree(map); by free_page((unsigned long)map);
diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
index 062be9dde4c6..ed15645b4321 100644
--- a/fs/btrfs/free-space-cache.c
+++ b/fs/btrfs/free-space-cache.c
@@ -764,7 +764,7 @@ static int __load_free_space_cache(struct btrfs_root *root,
struct inode *inode,
} else {
ASSERT(num_bitmaps);
num_bitmaps--;
- e->bitmap = kzalloc(PAGE_SIZE, GFP_NOFS);
+ e->bitmap = (void *)get_zeroed_page(GFP_NOFS);
if (!e->bitmap) {
kmem_cache_free(
btrfs_free_space_cachep, e);
@@ -1881,7 +1881,7 @@ static void free_bitmap(struct btrfs_free_space_ctl *ctl,
struct btrfs_free_space *bitmap_info)
{
unlink_free_space(ctl, bitmap_info);
- kfree(bitmap_info->bitmap);
+ free_page((unsigned long)bitmap_info->bitmap);
kmem_cache_free(btrfs_free_space_cachep, bitmap_info);
ctl->total_bitmaps--;
ctl->op->recalc_thresholds(ctl);
@@ -2135,7 +2135,7 @@ static int insert_into_bitmap(struct btrfs_free_space_ctl
*ctl,
}
/* allocate the bitmap */
- info->bitmap = kzalloc(PAGE_SIZE, GFP_NOFS);
+ info->bitmap = (void *)get_zeroed_page(GFP_NOFS);
spin_lock(&ctl->tree_lock);
if (!info->bitmap) {
ret = -ENOMEM;
@@ -2146,7 +2146,7 @@ static int insert_into_bitmap(struct btrfs_free_space_ctl
*ctl,
out:
if (info) {
- kfree(info->bitmap);
+ free_page((unsigned long)info->bitmap);
kmem_cache_free(btrfs_free_space_cachep, info);
}
@@ -2802,7 +2802,7 @@ u64 btrfs_alloc_from_cluster(struct
btrfs_block_group_cache *block_group,
if (entry->bytes == 0) {
ctl->free_extents--;
if (entry->bitmap) {
- kfree(entry->bitmap);
+ free_page((unsigned long)entry->bitmap);
ctl->total_bitmaps--;
ctl->op->recalc_thresholds(ctl);
}
@@ -3606,7 +3606,7 @@ int test_add_free_space_entry(struct
btrfs_block_group_cache *cache,
}
if (!map) {
- map = kzalloc(PAGE_SIZE, GFP_NOFS);
+ map = (void *)get_zeroed_page(GFP_NOFS);
if (!map) {
kmem_cache_free(btrfs_free_space_cachep, info);
return -ENOMEM;
@@ -3635,7 +3635,7 @@ int test_add_free_space_entry(struct
btrfs_block_group_cache *cache,
if (info)
kmem_cache_free(btrfs_free_space_cachep, info);
- kfree(map);
+ free_page((unsigned long)map);
return 0;
}
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply related
* [Bug 204371] BUG kmalloc-4k (Tainted: G W ): Object padding overwritten
From: bugzilla-daemon @ 2019-08-16 14:38 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 #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.
--
You are receiving this mail because:
You are on the CC list for the bug.
^ permalink raw reply
* [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
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