* RE: [PATCH V6 4/4] arm64: dts: imx8mm: Enable cpu-idle driver
From: Anson Huang @ 2019-08-16 10:37 UTC (permalink / raw)
To: Daniel Lezcano, robh+dt@kernel.org, mark.rutland@arm.com,
shawnguo@kernel.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, festevam@gmail.com, tglx@linutronix.de,
Leonard Crestez, Daniel Baluta, Jacky Bai, Jun Li,
l.stach@pengutronix.de, Abel Vesa, ccaione@baylibre.com,
andrew.smirnov@gmail.com, angus@akkea.ca, agx@sigxcpu.org,
devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Cc: dl-linux-imx
In-Reply-To: <DB3PR0402MB3916E469219C7CC68D55C90AF5AF0@DB3PR0402MB3916.eurprd04.prod.outlook.com>
Hi, Shawn
> > On 16/08/2019 02:38, Anson Huang wrote:
> > > Enable i.MX8MM cpu-idle using generic ARM cpu-idle driver, 2 states
> > > are supported, details as below:
> > >
> > > root@imx8mmevk:~# cat
> > /sys/devices/system/cpu/cpu0/cpuidle/state0/name
> > > WFI
> > > root@imx8mmevk:~# cat
> > > /sys/devices/system/cpu/cpu0/cpuidle/state0/usage
> > > 3973
> > > root@imx8mmevk:~# cat
> > /sys/devices/system/cpu/cpu0/cpuidle/state1/name
> > > cpu-pd-wait
> > > root@imx8mmevk:~# cat
> > > /sys/devices/system/cpu/cpu0/cpuidle/state1/usage
> > > 6647
> > >
> > > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> >
> > Hi Anson,
> >
> > I've applied the patches 1-3 but this one does not apply.
>
> Thanks.
>
> >
> > You can either respin it against tip/timers/core and take it through
> > Shawn's tree. If the later, you can add my Acked-by.
>
> Hi, Shawn
> Can you take this patch and add below Acked-by? It should can be
> applied to your tree directly.
>
> Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Sorry that I just found this patch can NOT be applied to your for-next tree neither, so
I redo the patch against your for-next tree, so you can just skip this patch series now
(as Daniel already picked the rest 3 patches) and take that patch I just sent, link as below:
https://patchwork.kernel.org/patch/11097471/
Thanks,
Anson
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ 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
> > > >
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] Revert "ARM: dts: rockchip: add startup delay to rk3288-veyron panel-regulators"
From: Heiko Stuebner @ 2019-08-16 11:28 UTC (permalink / raw)
To: Douglas Anderson
Cc: Mark Rutland, devicetree, linux-kernel, Rob Herring,
linux-rockchip, mka, enric.balletbo, linux-arm-kernel
In-Reply-To: <20190620182056.61552-1-dianders@chromium.org>
Am Donnerstag, 20. Juni 2019, 20:20:56 CEST schrieb Douglas Anderson:
> This reverts commit 1f45e8c6d0161f044d679f242fe7514e2625af4a.
>
> This 100 ms mystery delay is not on downstream kernels and no longer
> seems needed on upstream kernels either [1]. Presumably something in the
> meantime has made things better. A few possibilities for patches that
> have landed in the meantime that could have made this better are
> commit 3157694d8c7f ("pwm-backlight: Add support for PWM delays
> proprieties."), commit 5fb5caee92ba ("pwm-backlight: Enable/disable
> the PWM before/after LCD enable toggle."), and commit 6d5922dd0d60
> ("ARM: dts: rockchip: set PWM delay backlight settings for Veyron")
>
> Let's revert and get our 100 ms back.
>
> [1] https://lkml.kernel.org/r/2226970.BAPq4liE1j@diego
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
I've rebased the change on top of Matthias' veyron display reorganization
and applied it for 5.4
Thanks
Heiko
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 2/2] arm64: Don't use KPTI where we have E0PD
From: Mark Brown @ 2019-08-16 11:31 UTC (permalink / raw)
To: Will Deacon; +Cc: Catalin Marinas, linux-arm-kernel, Suzuki K Poulose
In-Reply-To: <20190815180030.GF4841@sirena.co.uk>
[-- Attachment #1.1: Type: text/plain, Size: 1067 bytes --]
On Thu, Aug 15, 2019 at 07:00:30PM +0100, Mark Brown wrote:
> On Thu, Aug 15, 2019 at 05:35:42PM +0100, Will Deacon wrote:
> > Thinking about this further, I think we can simply move all of the
> > 'kaslr_offset() > 0' checks used by the kpti code (i.e. in
> > arm64_kernel_unmapped_at_el0(), kpti_install_ng_mappings() and
> > unmap_kernel_at_el0()) into a helper function which does the check for
> > E0PD as well. Perhaps 'kaslr_requires_kpti()' ?
> > I think that should simplify your patch as well. What do you think?
> Dunno about simplifying the patch particularly, looks very similar but
> in any case it does appear to solve the problem - thanks.
Actually no, it's not quite that simple. They're not all looking for
quite the same thing even if they're all currently doing the same check.
For example kpti_install_ng_mappings() should run on all CPUs unless
none of them have installed global mappings and in particular currently
needs to run on the boot CPU but that's not what we want in for example
unmap_kernel_at_el0(). I'll poke at it some more.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 2/2] arm64: allwinner: h6: add I2C nodes
From: Maxime Ripard @ 2019-08-16 11:33 UTC (permalink / raw)
To: Bhushan Shah
Cc: Mark Rutland, devicetree, Wolfram Sang, Gregory CLEMENT,
linux-kernel, Chen-Yu Tsai, Rob Herring, linux-i2c,
linux-arm-kernel, Icenowy Zheng
In-Reply-To: <20190816084309.27440-3-bshah@kde.org>
[-- Attachment #1.1: Type: text/plain, Size: 354 bytes --]
On Fri, Aug 16, 2019 at 02:13:09PM +0530, Bhushan Shah wrote:
> Add device-tree nodes for i2c0 to i2c2, and also add relevant pinctrl
> nodes.
>
> Suggested-by: Icenowy Zheng <icenowy@aosc.io>
> Signed-off-by: Bhushan Shah <bshah@kde.org>
Applied both, thanks!
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] ARM64: dts: allwinner: Add devicetree for pine H64 modelA evaluation board
From: Maxime Ripard @ 2019-08-16 11:36 UTC (permalink / raw)
To: Corentin Labbe
Cc: mark.rutland, devicetree, linux-sunxi, linux-kernel, wens,
robh+dt, linux-arm-kernel
In-Reply-To: <20190816093513.GA25042@Red>
[-- Attachment #1.1: Type: text/plain, Size: 3950 bytes --]
On Fri, Aug 16, 2019 at 11:35:13AM +0200, Corentin Labbe wrote:
> On Wed, Aug 14, 2019 at 03:33:22PM +0200, Maxime Ripard wrote:
> > On Wed, Aug 14, 2019 at 03:17:41PM +0200, Corentin Labbe wrote:
> > > On Mon, Aug 12, 2019 at 11:40:00AM +0200, Maxime Ripard wrote:
> > > > On Thu, Aug 08, 2019 at 10:42:53AM +0200, Corentin Labbe wrote:
> > > > > This patch adds the evaluation variant of the model A of the PineH64.
> > > > > The model A has the same size of the pine64 and has a PCIE slot.
> > > > >
> > > > > The only devicetree difference with current pineH64, is the PHY
> > > > > regulator.
> > > > >
> > > > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> > > > > ---
> > > > > arch/arm64/boot/dts/allwinner/Makefile | 1 +
> > > > > .../sun50i-h6-pine-h64-modelA-eval.dts | 26 +++++++++++++++++++
> > > > > 2 files changed, 27 insertions(+)
> > > > > create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64-modelA-eval.dts
> > > > >
> > > > > diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile
> > > > > index f6db0611cb85..9a02166cbf72 100644
> > > > > --- a/arch/arm64/boot/dts/allwinner/Makefile
> > > > > +++ b/arch/arm64/boot/dts/allwinner/Makefile
> > > > > @@ -25,3 +25,4 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-3.dtb
> > > > > dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-lite2.dtb
> > > > > dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-one-plus.dtb
> > > > > dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-pine-h64.dtb
> > > > > +dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-pine-h64-modelA-eval.dtb
> > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64-modelA-eval.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64-modelA-eval.dts
> > > > > new file mode 100644
> > > > > index 000000000000..d8ff02747efe
> > > > > --- /dev/null
> > > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64-modelA-eval.dts
> > > > > @@ -0,0 +1,26 @@
> > > > > +// SPDX-License-Identifier: (GPL-2.0+ or MIT)
> > > > > +/*
> > > > > + * Copyright (C) 2019 Corentin Labbe <clabbe.montjoie@gmail.com>
> > > > > + */
> > > > > +
> > > > > +#include "sun50i-h6-pine-h64.dts"
> > > > > +
> > > > > +/ {
> > > > > + model = "Pine H64 model A evaluation board";
> > > > > + compatible = "pine64,pine-h64-modelA-eval", "allwinner,sun50i-h6";
> > > > > +
> > > > > + reg_gmac_3v3: gmac-3v3 {
> > > > > + compatible = "regulator-fixed";
> > > > > + regulator-name = "vcc-gmac-3v3";
> > > > > + regulator-min-microvolt = <3300000>;
> > > > > + regulator-max-microvolt = <3300000>;
> > > > > + startup-delay-us = <100000>;
> > > > > + gpio = <&pio 2 16 GPIO_ACTIVE_HIGH>;
> > > > > + enable-active-high;
> > > > > + };
> > > > > +
> > > > > +};
> > > > > +
> > > > > +&emac {
> > > > > + phy-supply = <®_gmac_3v3>;
> > > > > +};
> > > >
> > > > I might be missing some context here, but I'm pretty sure that the
> > > > initial intent of the pine h64 DTS was to support the model A all
> > > > along.
> > > >
> > >
> > > The regulator changed between modelA and B.
> > > See this old patchset (supporting modelA) https://patchwork.kernel.org/patch/10539149/ for example.
> >
> > I'm not sure what your point is, but mine is that everything about the
> > model A should be in sun50i-h6-pine-h64.dts.
> >
>
> model A and B are different enough for distinct dtb, (see sub-thread
> on HDMI difference for an other difference than PHY regulator)
I don't mind having separate DTBs for model A and model B.
> And clearly, the current dtb is for model B.
That DTS was added almost a year before the model B was announced, and
no commit to that file mention the model B, so it's definitely not
clear.
> So do you mean that we need to create a new dtb for model B ? (and
> hack the current back to model A ?)
I'd prefer not to hack anything, but yes
Maxime
--
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: arm: fsl: Add PHYTEC i.MX6 UL/ULL devicetree bindings
From: Stefan Riedmüller @ 2019-08-16 11:42 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, devicetree, Andrew Smirnov, Fabio Estevam,
Sascha Hauer, linux-kernel@vger.kernel.org, NXP Linux Team,
Manivannan Sadhasivam, Shawn Guo,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE
In-Reply-To: <CAL_JsqJHfVDfpC9Yr7o3HO4wU7QR+sp0mxFLkxwVcGXXLeAyJw@mail.gmail.com>
Hi Rob,
On 15.08.19 15:43, Rob Herring wrote:
> On Thu, Aug 15, 2019 at 4:55 AM Stefan Riedmüller
> <s.riedmueller@phytec.de> wrote:
>>
>> Hi Rob,
>>
>> On 13.08.19 18:04, Rob Herring wrote:
>>> On Wed, Jul 24, 2019 at 09:49:32AM +0200, Stefan Riedmueller wrote:
>>>> Add devicetree bindings for i.MX6 UL/ULL based phyCORE-i.MX6 UL/ULL and
>>>> phyBOARD-Segin.
>>>>
>>>> Signed-off-by: Stefan Riedmueller <s.riedmueller@phytec.de>
>>>> ---
>>>> Documentation/devicetree/bindings/arm/fsl.yaml | 8 ++++++++
>>>> 1 file changed, 8 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/arm/fsl.yaml b/Documentation/devicetree/bindings/arm/fsl.yaml
>>>> index 7294ac36f4c0..40f007859092 100644
>>>> --- a/Documentation/devicetree/bindings/arm/fsl.yaml
>>>> +++ b/Documentation/devicetree/bindings/arm/fsl.yaml
>>>> @@ -161,12 +161,20 @@ properties:
>>>> items:
>>>> - enum:
>>>> - fsl,imx6ul-14x14-evk # i.MX6 UltraLite 14x14 EVK Board
>>>> + - phytec,imx6ul-pbacd10 # PHYTEC phyBOARD-Segin with i.MX6 UL
>>>> + - phytec,imx6ul-pbacd10-emmc # PHYTEC phyBOARD-Segin eMMC Kit
>>>> + - phytec,imx6ul-pbacd10-nand # PHYTEC phyBOARD-Segin NAND Kit
>>>> + - phytec,imx6ul-pcl063 # PHYTEC phyCORE-i.MX 6UL
>>>
>>> This doesn't match what is in the dts files:
>>>
>>> arch/arm/boot/dts/imx6ul-phytec-pcl063.dtsi: compatible = "phytec,imx6ul-pcl063", "fsl,imx6ul";
>>> arch/arm/boot/dts/imx6ul-phytec-phyboard-segin-full.dts: compatible = "phytec,imx6ul-pbacd10", "phytec,imx6ul-pcl063",
>>> "fsl,imx6ul";
>>> arch/arm/boot/dts/imx6ul-phytec-phyboard-segin.dtsi: compatible = "phytec,imx6ul-pbacd-10", "phytec,imx6ul-pcl063",
>>> "fsl,imx6ul";
>>
>> Shawn already applied my patches which rename the compatibles, see
>> https://lkml.org/lkml/2019/7/23/42
>
> In any case, it still doesn't match. For example, from those patches:
>
> + model = "PHYTEC phyBOARD-Segin i.MX6 ULL Full Featured with eMMC";
> + compatible = "phytec,imx6ull-pbacd10-emmc", "phytec,imx6ull-pbacd10",
> + "phytec,imx6ull-pcl063","fsl,imx6ull";
>
> The correct schema for this would be:
>
> items:
> - const: phytec,imx6ull-pbacd10-emmc
> - const: phytec,imx6ull-pbacd10
> - const: phytec,imx6ull-pcl063
> - const: fsl,imx6ull
>
> This defines how many entries (4), what they are, and the order of
> them. Maybe the first entry can be an enum with the -nand compatible
> if those are 2 options.
>
> Run 'make dtbs_check' and make sure there aren't warnings for the root node.
Thanks for your input. I will take another closer look at this and send a
new version.
Stefan
>
> Rob
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] ARM64: dts: allwinner: Add devicetree for pine H64 modelA evaluation board
From: Corentin Labbe @ 2019-08-16 11:57 UTC (permalink / raw)
To: Maxime Ripard
Cc: mark.rutland, devicetree, linux-sunxi, linux-kernel, wens,
robh+dt, linux-arm-kernel
In-Reply-To: <20190816113650.hstbi5ntstx3wh4a@flea>
On Fri, Aug 16, 2019 at 01:36:50PM +0200, Maxime Ripard wrote:
> On Fri, Aug 16, 2019 at 11:35:13AM +0200, Corentin Labbe wrote:
> > On Wed, Aug 14, 2019 at 03:33:22PM +0200, Maxime Ripard wrote:
> > > On Wed, Aug 14, 2019 at 03:17:41PM +0200, Corentin Labbe wrote:
> > > > On Mon, Aug 12, 2019 at 11:40:00AM +0200, Maxime Ripard wrote:
> > > > > On Thu, Aug 08, 2019 at 10:42:53AM +0200, Corentin Labbe wrote:
> > > > > > This patch adds the evaluation variant of the model A of the PineH64.
> > > > > > The model A has the same size of the pine64 and has a PCIE slot.
> > > > > >
> > > > > > The only devicetree difference with current pineH64, is the PHY
> > > > > > regulator.
> > > > > >
> > > > > > Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>
> > > > > > ---
> > > > > > arch/arm64/boot/dts/allwinner/Makefile | 1 +
> > > > > > .../sun50i-h6-pine-h64-modelA-eval.dts | 26 +++++++++++++++++++
> > > > > > 2 files changed, 27 insertions(+)
> > > > > > create mode 100644 arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64-modelA-eval.dts
> > > > > >
> > > > > > diff --git a/arch/arm64/boot/dts/allwinner/Makefile b/arch/arm64/boot/dts/allwinner/Makefile
> > > > > > index f6db0611cb85..9a02166cbf72 100644
> > > > > > --- a/arch/arm64/boot/dts/allwinner/Makefile
> > > > > > +++ b/arch/arm64/boot/dts/allwinner/Makefile
> > > > > > @@ -25,3 +25,4 @@ dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-3.dtb
> > > > > > dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-lite2.dtb
> > > > > > dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-orangepi-one-plus.dtb
> > > > > > dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-pine-h64.dtb
> > > > > > +dtb-$(CONFIG_ARCH_SUNXI) += sun50i-h6-pine-h64-modelA-eval.dtb
> > > > > > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64-modelA-eval.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64-modelA-eval.dts
> > > > > > new file mode 100644
> > > > > > index 000000000000..d8ff02747efe
> > > > > > --- /dev/null
> > > > > > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64-modelA-eval.dts
> > > > > > @@ -0,0 +1,26 @@
> > > > > > +// SPDX-License-Identifier: (GPL-2.0+ or MIT)
> > > > > > +/*
> > > > > > + * Copyright (C) 2019 Corentin Labbe <clabbe.montjoie@gmail.com>
> > > > > > + */
> > > > > > +
> > > > > > +#include "sun50i-h6-pine-h64.dts"
> > > > > > +
> > > > > > +/ {
> > > > > > + model = "Pine H64 model A evaluation board";
> > > > > > + compatible = "pine64,pine-h64-modelA-eval", "allwinner,sun50i-h6";
> > > > > > +
> > > > > > + reg_gmac_3v3: gmac-3v3 {
> > > > > > + compatible = "regulator-fixed";
> > > > > > + regulator-name = "vcc-gmac-3v3";
> > > > > > + regulator-min-microvolt = <3300000>;
> > > > > > + regulator-max-microvolt = <3300000>;
> > > > > > + startup-delay-us = <100000>;
> > > > > > + gpio = <&pio 2 16 GPIO_ACTIVE_HIGH>;
> > > > > > + enable-active-high;
> > > > > > + };
> > > > > > +
> > > > > > +};
> > > > > > +
> > > > > > +&emac {
> > > > > > + phy-supply = <®_gmac_3v3>;
> > > > > > +};
> > > > >
> > > > > I might be missing some context here, but I'm pretty sure that the
> > > > > initial intent of the pine h64 DTS was to support the model A all
> > > > > along.
> > > > >
> > > >
> > > > The regulator changed between modelA and B.
> > > > See this old patchset (supporting modelA) https://patchwork.kernel.org/patch/10539149/ for example.
> > >
> > > I'm not sure what your point is, but mine is that everything about the
> > > model A should be in sun50i-h6-pine-h64.dts.
> > >
> >
> > model A and B are different enough for distinct dtb, (see sub-thread
> > on HDMI difference for an other difference than PHY regulator)
>
> I don't mind having separate DTBs for model A and model B.
>
> > And clearly, the current dtb is for model B.
>
> That DTS was added almost a year before the model B was announced, and
> no commit to that file mention the model B, so it's definitely not
> clear.
Normal it was added for model A (without any ethernet/HDMI support, so nothing distinct from model B), and the modelB ethernet/HDMI support cames after.
>
> > So do you mean that we need to create a new dtb for model B ? (and
> > hack the current back to model A ?)
>
> I'd prefer not to hack anything, but yes
>
Since model A is not public (only evaluations boards exists), the probability of a production model A is low and the current dtb is perfect for model B , could you reconsider this ?
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: aarch64 Kernel Panic Asynchronous SError Interrupt on large file IO
From: Robin Murphy @ 2019-08-16 12:01 UTC (permalink / raw)
To: Philipp Richter
Cc: heiko, catalin.marinas, vicencb, linux-rockchip, andre.przywara,
Will Deacon, linux-arm-kernel
In-Reply-To: <CA+Vb7hqPvDtv0ahjxa_gM68qsws6-dmtiOPmG6-WB+HZEC=4aw@mail.gmail.com>
On 15/08/2019 17:00, Philipp Richter wrote:
> Reading from the raw eMMC block /dev/mmcblkp1 I can also produce a panic :
>
> sudo dd if=/dev/mmcblk1 of=/dev/null bs=1M status=progress
> 2846883840 bytes (2.8 GB, 2.7 GiB) copied, 23 s, 124 MB/s
Hmm, I'm running Arch with the same stock kernel on my RK3328 box, and
that doesn't seem to have an problem:
-----
[root@nemulon-9 ~]# dd if=/dev/mmcblk1 of=/dev/null bs=1M status=progress
15408824320 bytes (15 GB, 14 GiB) copied, 125 s, 123 MB/s
14800+0 records in
14800+0 records out
15518924800 bytes (16 GB, 14 GiB) copied, 125.868 s, 123 MB/s
[root@nemulon-9 ~]# dd if=/dev/mmcblk1 of=/dev/null bs=1M status=progress
15437135872 bytes (15 GB, 14 GiB) copied, 125 s, 123 MB/s
14800+0 records in
14800+0 records out
15518924800 bytes (16 GB, 14 GiB) copied, 125.655 s, 124 MB/s
[root@nemulon-9 ~]# dd if=/dev/mmcblk1 of=/dev/null bs=1M status=progress
15446573056 bytes (15 GB, 14 GiB) copied, 125 s, 124 MB/s
14800+0 records in
14800+0 records out
15518924800 bytes (16 GB, 14 GiB) copied, 125.576 s, 124 MB/s
[root@nemulon-9 ~]# dd if=/dev/mmcblk1 of=/dev/null bs=1M status=progress
15411970048 bytes (15 GB, 14 GiB) copied, 125 s, 123 MB/s
14800+0 records in
14800+0 records out
15518924800 bytes (16 GB, 14 GiB) copied, 125.844 s, 123 MB/s
[root@nemulon-9 ~]#
-----
and FWIW this box is running a far more antique Rock64 firmware:
-----
DDR version 1.06 20170424
In
LPDDR3
786MHz
Bus Width=32 Col=11 Bank=8 Row=15/15 CS=2 Die Bus-Width=32 Size=4096MB
ddrconfig:7
OUT
Boot1 Release Time: 2017-05-18, version: 2.43
ChipType = 0x11, 127
WR_REL_SET is 0 4
SdmmcInit=2 0
BootCapSize=2000
UserCapSize=14800MB
FwPartOffset=2000 , 2000
SdmmcInit=0 2
StorageInit ok = 23274
Raw SecureMode = 0
SecureInit read PBA: 0x4
SecureInit read PBA: 0x404
SecureInit read PBA: 0x804
SecureInit read PBA: 0xc04
SecureInit read PBA: 0x1004
SecureInit ret = 0, SecureMode = 0
LoadTrustBL
No find bl30.bin
No find bl32.bin
Load uboot, ReadLba = 2000
Load OK, addr=0x200000, size=0x9268c
RunBL31 0x10000
\x01NOTICE: BL31: v1.3(debug):f947c7e
NOTICE: BL31: Built : 09:28:45, May 31 2017
NOTICE: BL31:Rockchip release version: v1.3
INFO: ARM GICv2 driver initialized
INFO: Using opteed sec cpu_context!
INFO: boot cpu mask: 1
INFO: plat_rockchip_pmu_init: pd status 0xe
INFO: BL31: Initializing runtime services
WARNING: No OPTEE provided by BL2 boot loader, Booting device without
OPTEE initialization. SMC`s destined for OPTEE will return SMC_UNK
ERROR: Error initializing runtime service opteed_fast
INFO: BL31: Preparing for EL3 exit to normal world
INFO: Entry point address = 0x200000
INFO: SPSR = 0x3c9
U-Boot 2017.09-rc4-g22993de (Sep 14 2017 - 21:57:17 +0000), Build:
jenkins-linux-build-rock-64-118
Model: Pine64 Rock64
DRAM: 4 GiB
MMC: rksdmmc@ff500000: 1, rksdmmc@ff520000: 0
Card did not respond to voltage select!
mmc_init: -95, time 9
*** Warning - No block device, using default environment
----
[ of course it's not a Rock64 at all, but I'm lazy, that was the only
prebuilt image available at the time, and the differences aren't major
enough to prevent SD/eMMC booting :) ]
> ============
> [ 428.794747] dwmmc_rockchip ff520000.dwmmc: Unexpected command
> timeout, state 3
> [ 428.984736] dwmmc_rockchip ff520000.dwmmc: Unexpected command
> timeout, state 3
> [ 429.174738] dwmmc_rockchip ff520000.dwmmc: Unexpected command
> timeout, state 3
> [ 429.179323] Internal error: synchronous external abort: 96000210
> [#1] SMP
> [ 429.179934] Modules linked in: wireguard(O) ip6_udp_tunnel
> udp_tunnel lz4 lz4_compress iptable_filter iptable_raw xt_owner
> iptable_nat xt_connmark iptable_mangle bpfilter rc_cec
> snd_soc_hdmi_codec dw_hdmi_i2s_audio dw_hdmi_cec
> snd_soc_audio_graph_cc
> [ 429.186527] CPU: 0 PID: 1079 Comm: bash Tainted: G O
> 5.2.8-1-ARCH #1
> [ 429.187193] Hardware name: Pine64 Rock64 (DT)
> [ 429.187576] pstate: 20000005 (nzCv daif -PAN -UAO)
> [ 429.188007] pc : copy_page_range+0x124/0x3d0
> [ 429.188386] lr : dup_mm+0x3fc/0x478
> [ 429.188692] sp : ffff00001277bb80
> [ 429.188982] x29: ffff00001277bb80 x28: ffff8000dd17e450
> [ 429.189446] x27: ffff8000dd17e470 x26: ffff8000dd17e460
> [ 429.189912] x25: 0000aaaac4a01000 x24: ffff8000dca92a00
> [ 429.190376] x23: ffff8000dd1fdf80 x22: ffff8000dd30c8a0
> [ 429.190840] x21: ffff8000dca92a00 x20: ffff8000dd30c8a0
> [ 429.191306] x19: ffff8000dd1fdf80 x18: 0000000000000000
> [ 429.191771] x17: 0000000000000000 x16: 0000000000000000
> [ 429.192236] x15: 0000000000000000 x14: ffff8000dd2b86d0
> [ 429.192700] x13: 00000000000000f8 x12: 0000000000000000
> [ 429.193165] x11: 0000000000000000 x10: ffff8000e44bde01
> [ 429.193630] x9 : 0000000000100871 x8 : 0000000000000000
> [ 429.194095] x7 : ffff8000e4481760 x6 : 0000000000000000
> [ 429.194560] x5 : 0000aaaac49fc000 x4 : ffff0000102905c0
> [ 429.195026] x3 : 0000000000000000 x2 : ffff800009c74aa8
> [ 429.195491] x1 : 0000aaaac4a00fff x0 : ffff800009c74aa8
> [ 429.195959] Call trace:
> [ 429.196178] copy_page_range+0x124/0x3d0
> [ 429.196521] dup_mm+0x3fc/0x478
> [ 429.196801] copy_process.isra.4.part.5+0x143c/0x1450
> [ 429.197244] _do_fork+0xec/0x410
> [ 429.197529] __arm64_sys_clone+0x2c/0x38
> [ 429.197877] el0_svc_handler+0xa4/0x180
> [ 429.198215] el0_svc+0x8/0xc
> [ 429.198474] Code: 360812e0 f9403fe0 b4000ac0 f9403fe0 (f9400000)
This one's particularly interesting since it's synchronous. That code
dump implies that an "ldr x0, [x0]" is faulting, when x0 holds a linear
map address (i.e. directly correlated to a physical address).
> [ 429.199008] ---[ end trace 04beba7bac629e3f ]---
> [ 429.200049] SError Interrupt on CPU1, code 0xbf000002 -- SError
> [ 429.200052] CPU: 1 PID: 669 Comm: systemd-journal Tainted: G D
> O 5.2.8-1-ARCH #1
> [ 429.200054] Hardware name: Pine64 Rock64 (DT)
> [ 429.200055] pstate: 20000005 (nzCv daif -PAN -UAO)
> [ 429.200056] pc : allocate_slab+0x1d0/0x570
> [ 429.200058] lr : allocate_slab+0x1e0/0x570
> [ 429.200059] sp : ffff000011d8baa0
> [ 429.200060] x29: ffff000011d8baa0 x28: 0000000000000003
> [ 429.200063] x27: ffff7e0000276800 x26: ffff800009da6e00
> [ 429.200068] x25: 0000000000000009 x24: 0000000000007bc0
> [ 429.200071] x23: 0000000000000003 x22: 0000000000000003
> [ 429.200075] x21: ffff800009da0000 x20: 0000000000005280
> [ 429.200079] x19: ffff8000b3fa3980 x18: 0000000000000000
> [ 429.200082] x17: 0000000000000000 x16: 0000000000000000
> [ 429.200086] x15: 0000000000000000 x14: 0000000000000000
> [ 429.200090] x13: 0000000000000000 x12: 0000000000000000
> [ 429.200094] x11: 0000000000000000 x10: 0000000000000000
> [ 429.200098] x9 : 0000000000000000 x8 : 0000000000000000
> [ 429.200102] x7 : 00000000fee00000 x6 : 0000000000000018
> [ 429.200106] x5 : 0000000000000040 x4 : 0000000000210d00
> [ 429.200110] x3 : 0000000000000dc0 x2 : 0000000005a79795
> [ 429.200112] x1 : 0000000000000000 x0 : ffff8000f2f35a80
> [ 429.200117] Kernel panic - not syncing: Asynchronous SError
> Interrupt
This is less revealing, but the fact that x21 and x26 are holding
pointers to a relatively similar area of RAM does raise an eyebrow.
[...]
>> [12624.268933] SError Interrupt on CPU0, code 0xbf000002 -- SError
>> [12624.268940] CPU: 0 PID: 14170 Comm: kworker/u8:4 Tainted: G
>> O 5.2.8-1-ARCH #1
>> [12624.268942] Hardware name: Pine64 Rock64 (DT)
>> [12624.268944] Workqueue: btrfs-endio btrfs_endio_helper [btrfs]
>> [12624.268946] pstate: 20000005 (nzCv daif -PAN -UAO)
>> [12624.268948] pc : __memcpy+0x118/0x180
>> [12624.268950] lr : btrfs_decompress_buf2page+0x124/0x228 [btrfs]
>> [12624.268951] sp : ffff00001c28bb40
>> [12624.268952] x29: ffff00001c28bb40 x28: ffff8000f2a2b870
>> [12624.268955] x27: 0000000000001000 x26: ffff7e0000270200
>> [12624.268958] x25: 0000000000001000 x24: 000000000001f000
>> [12624.268961] x23: 0000000000000000 x22: 000000000001f000
>> [12624.268964] x21: ffff8000fde46040 x20: 0000000000140000
>> [12624.268967] x19: 0000000000001000 x18: ffff8000e830aef5
>> [12624.268970] x17: 0000000000000ad3 x16: 0000000000000003
>> [12624.268973] x15: 0000000000000002 x14: a8c37bfd9101e042
>> [12624.268976] x13: a9425bf552800021 x12: a94153f3f0000b62
>> [12624.268979] x11: f9400a80900011a4 x10: aa1603e3d63f0260
>> [12624.268982] x9 : 9101c04252800021 x8 : 910003fda9b97bfd
>> [12624.268985] x7 : d61f0080f9475c84 x6 : ffff800009c08390
>> [12624.268988] x5 : ffff800065005050 x4 : 0000000000000000
>> [12624.268990] x3 : 0000000000140000 x2 : 0000000000000c00
>> [12624.268993] x1 : ffff8000dac023d0 x0 : ffff800009c08000
>> [12624.268997] Kernel panic - not syncing: Asynchronous SError Interrupt
In this case, I know that x6 is the destination pointer for the memcpy
routine, and the memcpy from the first log also looks similar:
> [ 334.414179] x7 : 911b47420a1c00a2 x6 : ffff800009c01150
Again, all within a few MB of that same region. Given that we're a few
hundred bytes into the copy both times, this could well represent the
point where the first dirtied cachelines start to get written back,
provoking the bus error from the memory controller and thus an async SError.
This does start to smell like some issue with that particular area of
physical memory - either because it's been marked as Secure-only by
firmware and the controller configured to abort Non-Secure accesses, or
possibly because of an actual DRAM failure. The next thing I'd do is
have a play around with the "memtest=..." kernel parameter to see if
that can consistently find a problem, and see if the firmware change
that Heiko pointed out makes any difference.
Robin.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2 2/2] arm64: Don't use KPTI where we have E0PD
From: Mark Brown @ 2019-08-16 12:10 UTC (permalink / raw)
To: Catalin Marinas; +Cc: Will Deacon, linux-arm-kernel, Suzuki K Poulose
In-Reply-To: <20190816102424.GA28874@arrakis.emea.arm.com>
[-- Attachment #1.1: Type: text/plain, Size: 3404 bytes --]
On Fri, Aug 16, 2019 at 11:24:24AM +0100, Catalin Marinas wrote:
> On Thu, Aug 15, 2019 at 05:35:42PM +0100, Will Deacon wrote:
> > > + if (IS_ENABLED(CONFIG_ARM64_E0PD)) {
> > > + ftr = read_sysreg_s(SYS_ID_AA64MMFR2_EL1);
> > > + if ((ftr >> ID_AA64MMFR2_E0PD_SHIFT) & 0xf)
> > > + return false;
> > > + }
> What I don't particularly like here is that on big.LITTLE this hunk may
> have a different behaviour depending on which CPU you run it on. In
> general, such CPUID access should only be done in a non-preemptible
> context.
> We probably get away with this during early boot (before CPU caps have
> been set up) when arm64_kernel_unmapped_at_el0() is false since we only
> have a single CPU running. Later on at run-time, we either have
> arm64_kernel_unmapped_at_el0() true, meaning that some CPU is missing
> E0PD with kaslr_offset() > 0, or the kernel is mapped at EL0 with all
> CPUs having E0PD. But I find it hard to reason about.
Yes, all this stuff is unfortunately hard to reason about since there's
several environment changes during boot which have a material effect and
also multiple different things that might trigger KPTI. IIRC my thinking
here was that if we turned on KPTI we're turning it on for all CPUs so
by the time we could be prempted we'd be returning true from the earlier
check for arm64_kernel_unmapped_at_el0() but it's possible I missed some
case there. I was trying to avoid disturbing the existing code too much
unless I had a strong reason to on the basis that I might be missing
something about the way it was done.
> Could we move the above hunk in this block:
> } else if (!static_branch_likely(&arm64_const_caps_ready)) {
> ...
> }
> and reshuffle the rest of the function to only rely on
> arm64_kernel_unmapped_at_el0() when the caps are ready (at run-time)?
I've added the check, will look at the reshuffle.
> > Thinking about this further, I think we can simply move all of the
> > 'kaslr_offset() > 0' checks used by the kpti code (i.e. in
> > arm64_kernel_unmapped_at_el0(), kpti_install_ng_mappings() and
> > unmap_kernel_at_el0()) into a helper function which does the check for
> > E0PD as well. Perhaps 'kaslr_requires_kpti()' ?
> I agree, this needs some refactoring as we have this decision in three
> separate places.
> Trying to put my thoughts together. At run-time, with capabilities fully
> enabled, we want:
> arm64_kernel_use_ng_mappings() == arm64_kernel_unmapped_at_el0()
> KPTI is equivalent to arm64_kernel_unmapped_at_el0()
Yes, this bit is simple - once we're up and running everything is clear.
> I think kaslr_requires_kpti() should access the raw CPUID registers (for
> E0PD, TX1 bug) and be called only by unmap_kernel_at_el0() and
> arm64_kernel_use_ng_mappings(), the latter if !arm64_const_caps_ready.
> The boot CPU should store kaslr_requires_kpti() value somewhere and
> kpti_install_ng_mappings() should check this variable before deciding to
> skip the page table rewrite.
We definitely need some variable I think, and I think you're right that
making the decision on the boot CPU would simplify things a lot. The
systems with very large memories that are most affected by the cost of
moving from global to non-global mappings are most likely symmetric
anyway so only looking at the boot CPU should be fine for that.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* 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
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ 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.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 #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v2] arm: dts: rockchip: fix vcc_host_5v regulator for usb3 host
From: Heiko Stuebner @ 2019-08-16 12:24 UTC (permalink / raw)
To: Kever Yang
Cc: Mark Rutland, devicetree, Jonas Karlman, Katsuhiro Suzuki,
linux-kernel, linux-rockchip, Chen-Yu Tsai, Rob Herring,
Tomohiro Mayama, linux-arm-kernel
In-Reply-To: <20190815081252.27405-1-kever.yang@rock-chips.com>
Hi Kever, TL,
[added TL Lim for clarification]
Am Donnerstag, 15. August 2019, 10:12:52 CEST schrieb Kever Yang:
> According to rock64 schemetic V2 and V3, the VCC_HOST_5V output is
> controlled by USB_20_HOST_DRV, which is the same as VCC_HOST1_5V.
The v1 schematics I have do reference the GPIO0_A0 as controlling this
supply, so the big question would be how to handle the different versions.
Because adding this would probably break v1 boards in this function.
@TL: where v1 boards also sold or were they only used during development?
If this were the case, we could just apply the patch, not caring about
v1 boards, but if v1 boards were also sold to customers there would be
more of a problem.
Thanks
Heiko
>
> Signed-off-by: Kever Yang <kever.yang@rock-chips.com>
> ---
>
> Changes in v2:
> - remove enable-active-high property
>
> arch/arm64/boot/dts/rockchip/rk3328-rock64.dts | 11 ++---------
> 1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
> index 7cfd5ca6cc85..62936b432f9a 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3328-rock64.dts
> @@ -34,10 +34,9 @@
>
> vcc_host_5v: vcc-host-5v-regulator {
> compatible = "regulator-fixed";
> - enable-active-high;
> - gpio = <&gpio0 RK_PA0 GPIO_ACTIVE_HIGH>;
> + gpio = <&gpio0 RK_PA2 GPIO_ACTIVE_LOW>;
> pinctrl-names = "default";
> - pinctrl-0 = <&usb30_host_drv>;
> + pinctrl-0 = <&usb20_host_drv>;
> regulator-name = "vcc_host_5v";
> regulator-always-on;
> regulator-boot-on;
> @@ -320,12 +319,6 @@
> rockchip,pins = <0 RK_PA2 RK_FUNC_GPIO &pcfg_pull_none>;
> };
> };
> -
> - usb3 {
> - usb30_host_drv: usb30-host-drv {
> - rockchip,pins = <0 RK_PA0 RK_FUNC_GPIO &pcfg_pull_none>;
> - };
> - };
> };
>
> &sdmmc {
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [RFC v4 00/18] objtool: Add support for arm64
From: Raphael Gault @ 2019-08-16 12:23 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, jpoimboe
Cc: raph.gault+kdev, peterz, catalin.marinas, will.deacon,
Raphael Gault, julien.thierry.kdev
Hi,
Changes since RFC V3:
* Rebased on tip/master: Switch/jump table had been refactored
* Take Catalin Marinas comments into account regarding the asm macro for
marking exceptions.
As of now, objtool only supports the x86_64 architecture but the
groundwork has already been done in order to add support for other
architectures without too much effort.
This series of patches adds support for the arm64 architecture
based on the Armv8.5 Architecture Reference Manual.
Objtool will be a valuable tool to progress and provide more guarentees
on live patching which is a work in progress for arm64.
Once we have the base of objtool working the next steps will be to
port Peter Z's uaccess validation for arm64.
Raphael Gault (18):
objtool: Add abstraction for computation of symbols offsets
objtool: orc: Refactor ORC API for other architectures to implement.
objtool: Move registers and control flow to arch-dependent code
objtool: arm64: Add required implementation for supporting the aarch64
architecture in objtool.
objtool: special: Adapt special section handling
objtool: arm64: Adapt the stack frame checks for arm architecture
objtool: Introduce INSN_UNKNOWN type
objtool: Refactor switch-tables code to support other architectures
gcc-plugins: objtool: Add plugin to detect switch table on arm64
objtool: arm64: Implement functions to add switch tables alternatives
arm64: alternative: Mark .altinstr_replacement as containing
executable instructions
arm64: assembler: Add macro to annotate asm function having non
standard stack-frame.
arm64: sleep: Prevent stack frame warnings from objtool
arm64: kvm: Annotate non-standard stack frame functions
arm64: kernel: Add exception on kuser32 to prevent stack analysis
arm64: crypto: Add exceptions for crypto object to prevent stack
analysis
arm64: kernel: Annotate non-standard stack frame functions
objtool: arm64: Enable stack validation for arm64
arch/arm64/Kconfig | 1 +
arch/arm64/crypto/Makefile | 3 +
arch/arm64/include/asm/alternative.h | 2 +-
arch/arm64/kernel/Makefile | 3 +
arch/arm64/kernel/hyp-stub.S | 3 +
arch/arm64/kernel/sleep.S | 5 +
arch/arm64/kvm/hyp-init.S | 3 +
arch/arm64/kvm/hyp/entry.S | 3 +
include/linux/frame.h | 19 +-
scripts/Makefile.gcc-plugins | 2 +
scripts/gcc-plugins/Kconfig | 9 +
.../arm64_switch_table_detection_plugin.c | 58 +
tools/objtool/Build | 2 -
tools/objtool/arch.h | 19 +
tools/objtool/arch/arm64/Build | 8 +
tools/objtool/arch/arm64/arch_special.c | 165 +
tools/objtool/arch/arm64/bit_operations.c | 67 +
tools/objtool/arch/arm64/decode.c | 2815 +++++++++++++++++
.../objtool/arch/arm64/include/arch_special.h | 52 +
.../arch/arm64/include/asm/orc_types.h | 96 +
.../arch/arm64/include/bit_operations.h | 24 +
tools/objtool/arch/arm64/include/cfi.h | 74 +
.../objtool/arch/arm64/include/insn_decode.h | 210 ++
tools/objtool/arch/arm64/orc_dump.c | 26 +
tools/objtool/arch/arm64/orc_gen.c | 40 +
tools/objtool/arch/x86/Build | 3 +
tools/objtool/arch/x86/arch_special.c | 107 +
tools/objtool/arch/x86/decode.c | 16 +
tools/objtool/arch/x86/include/arch_special.h | 45 +
tools/objtool/{ => arch/x86/include}/cfi.h | 0
tools/objtool/{ => arch/x86}/orc_dump.c | 4 +-
tools/objtool/{ => arch/x86}/orc_gen.c | 104 +-
tools/objtool/check.c | 309 +-
tools/objtool/check.h | 10 +
tools/objtool/elf.c | 3 +-
tools/objtool/orc.h | 4 +-
tools/objtool/special.c | 28 +-
tools/objtool/special.h | 13 +-
38 files changed, 4129 insertions(+), 226 deletions(-)
create mode 100644 scripts/gcc-plugins/arm64_switch_table_detection_plugin.c
create mode 100644 tools/objtool/arch/arm64/Build
create mode 100644 tools/objtool/arch/arm64/arch_special.c
create mode 100644 tools/objtool/arch/arm64/bit_operations.c
create mode 100644 tools/objtool/arch/arm64/decode.c
create mode 100644 tools/objtool/arch/arm64/include/arch_special.h
create mode 100644 tools/objtool/arch/arm64/include/asm/orc_types.h
create mode 100644 tools/objtool/arch/arm64/include/bit_operations.h
create mode 100644 tools/objtool/arch/arm64/include/cfi.h
create mode 100644 tools/objtool/arch/arm64/include/insn_decode.h
create mode 100644 tools/objtool/arch/arm64/orc_dump.c
create mode 100644 tools/objtool/arch/arm64/orc_gen.c
create mode 100644 tools/objtool/arch/x86/arch_special.c
create mode 100644 tools/objtool/arch/x86/include/arch_special.h
rename tools/objtool/{ => arch/x86/include}/cfi.h (100%)
rename tools/objtool/{ => arch/x86}/orc_dump.c (98%)
rename tools/objtool/{ => arch/x86}/orc_gen.c (66%)
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [RFC v4 01/18] objtool: Add abstraction for computation of symbols offsets
From: Raphael Gault @ 2019-08-16 12:23 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, jpoimboe
Cc: raph.gault+kdev, peterz, catalin.marinas, will.deacon,
Raphael Gault, julien.thierry.kdev
In-Reply-To: <20190816122403.14994-1-raphael.gault@arm.com>
The jump destination and relocation offset used previously are only
reliable on x86_64 architecture. We abstract these computations by calling
arch-dependent implementations.
Signed-off-by: Raphael Gault <raphael.gault@arm.com>
---
tools/objtool/arch.h | 6 ++++++
tools/objtool/arch/x86/decode.c | 11 +++++++++++
tools/objtool/check.c | 15 ++++++++++-----
3 files changed, 27 insertions(+), 5 deletions(-)
diff --git a/tools/objtool/arch.h b/tools/objtool/arch.h
index ced3765c4f44..a9a50a25ca66 100644
--- a/tools/objtool/arch.h
+++ b/tools/objtool/arch.h
@@ -66,6 +66,8 @@ struct stack_op {
struct op_src src;
};
+struct instruction;
+
void arch_initial_func_cfi_state(struct cfi_state *state);
int arch_decode_instruction(struct elf *elf, struct section *sec,
@@ -75,4 +77,8 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
bool arch_callee_saved_reg(unsigned char reg);
+unsigned long arch_jump_destination(struct instruction *insn);
+
+unsigned long arch_dest_rela_offset(int addend);
+
#endif /* _ARCH_H */
diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index 0567c47a91b1..fa33b3465722 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -11,6 +11,7 @@
#include "lib/inat.c"
#include "lib/insn.c"
+#include "../../check.h"
#include "../../elf.h"
#include "../../arch.h"
#include "../../warn.h"
@@ -66,6 +67,11 @@ bool arch_callee_saved_reg(unsigned char reg)
}
}
+unsigned long arch_dest_rela_offset(int addend)
+{
+ return addend + 4;
+}
+
int arch_decode_instruction(struct elf *elf, struct section *sec,
unsigned long offset, unsigned int maxlen,
unsigned int *len, enum insn_type *type,
@@ -497,3 +503,8 @@ void arch_initial_func_cfi_state(struct cfi_state *state)
state->regs[16].base = CFI_CFA;
state->regs[16].offset = -8;
}
+
+unsigned long arch_jump_destination(struct instruction *insn)
+{
+ return insn->offset + insn->len + insn->immediate;
+}
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 176f2f084060..479fab46b656 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -563,7 +563,7 @@ static int add_jump_destinations(struct objtool_file *file)
insn->len);
if (!rela) {
dest_sec = insn->sec;
- dest_off = insn->offset + insn->len + insn->immediate;
+ dest_off = arch_jump_destination(insn);
} else if (rela->sym->type == STT_SECTION) {
dest_sec = rela->sym->sec;
dest_off = rela->addend + 4;
@@ -659,7 +659,7 @@ static int add_call_destinations(struct objtool_file *file)
rela = find_rela_by_dest_range(insn->sec, insn->offset,
insn->len);
if (!rela) {
- dest_off = insn->offset + insn->len + insn->immediate;
+ dest_off = arch_jump_destination(insn);
insn->call_dest = find_symbol_by_offset(insn->sec,
dest_off);
@@ -672,14 +672,19 @@ static int add_call_destinations(struct objtool_file *file)
}
} else if (rela->sym->type == STT_SECTION) {
+ /*
+ * the original x86_64 code adds 4 to the rela->addend
+ * which is not needed on arm64 architecture.
+ */
+ dest_off = arch_dest_rela_offset(rela->addend);
insn->call_dest = find_symbol_by_offset(rela->sym->sec,
- rela->addend+4);
+ dest_off);
if (!insn->call_dest ||
insn->call_dest->type != STT_FUNC) {
- WARN_FUNC("can't find call dest symbol at %s+0x%x",
+ WARN_FUNC("can't find call dest symbol at %s+0x%lx",
insn->sec, insn->offset,
rela->sym->sec->name,
- rela->addend + 4);
+ dest_off);
return -1;
}
} else
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [RFC v4 02/18] objtool: orc: Refactor ORC API for other architectures to implement.
From: Raphael Gault @ 2019-08-16 12:23 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, jpoimboe
Cc: raph.gault+kdev, peterz, catalin.marinas, will.deacon,
Raphael Gault, julien.thierry.kdev
In-Reply-To: <20190816122403.14994-1-raphael.gault@arm.com>
The ORC unwinder is only supported on x86 at the moment and should thus be
in the x86 architecture code. In order not to break the whole structure in
case another architecture decides to support the ORC unwinder via objtool
we choose to let the implementation be done in the architecture dependent
code.
Signed-off-by: Raphael Gault <raphael.gault@arm.com>
---
tools/objtool/Build | 2 -
tools/objtool/arch.h | 3 +
tools/objtool/arch/x86/Build | 2 +
tools/objtool/{ => arch/x86}/orc_dump.c | 4 +-
tools/objtool/{ => arch/x86}/orc_gen.c | 104 ++++++++++++++++++++++--
tools/objtool/check.c | 99 +---------------------
tools/objtool/orc.h | 4 +-
7 files changed, 111 insertions(+), 107 deletions(-)
rename tools/objtool/{ => arch/x86}/orc_dump.c (98%)
rename tools/objtool/{ => arch/x86}/orc_gen.c (66%)
diff --git a/tools/objtool/Build b/tools/objtool/Build
index 8dc4f0848362..d069d26d97fa 100644
--- a/tools/objtool/Build
+++ b/tools/objtool/Build
@@ -2,8 +2,6 @@ objtool-y += arch/$(SRCARCH)/
objtool-y += builtin-check.o
objtool-y += builtin-orc.o
objtool-y += check.o
-objtool-y += orc_gen.o
-objtool-y += orc_dump.o
objtool-y += elf.o
objtool-y += special.o
objtool-y += objtool.o
diff --git a/tools/objtool/arch.h b/tools/objtool/arch.h
index a9a50a25ca66..e91e12807678 100644
--- a/tools/objtool/arch.h
+++ b/tools/objtool/arch.h
@@ -10,6 +10,7 @@
#include <linux/list.h>
#include "elf.h"
#include "cfi.h"
+#include "orc.h"
enum insn_type {
INSN_JUMP_CONDITIONAL,
@@ -77,6 +78,8 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
bool arch_callee_saved_reg(unsigned char reg);
+int arch_orc_read_unwind_hints(struct objtool_file *file);
+
unsigned long arch_jump_destination(struct instruction *insn);
unsigned long arch_dest_rela_offset(int addend);
diff --git a/tools/objtool/arch/x86/Build b/tools/objtool/arch/x86/Build
index b998412c017d..1f11b45999d0 100644
--- a/tools/objtool/arch/x86/Build
+++ b/tools/objtool/arch/x86/Build
@@ -1,4 +1,6 @@
objtool-y += decode.o
+objtool-y += orc_dump.o
+objtool-y += orc_gen.o
inat_tables_script = arch/x86/tools/gen-insn-attr-x86.awk
inat_tables_maps = arch/x86/lib/x86-opcode-map.txt
diff --git a/tools/objtool/orc_dump.c b/tools/objtool/arch/x86/orc_dump.c
similarity index 98%
rename from tools/objtool/orc_dump.c
rename to tools/objtool/arch/x86/orc_dump.c
index 13ccf775a83a..cfe8f96bdd68 100644
--- a/tools/objtool/orc_dump.c
+++ b/tools/objtool/arch/x86/orc_dump.c
@@ -4,8 +4,8 @@
*/
#include <unistd.h>
-#include "orc.h"
-#include "warn.h"
+#include "../../orc.h"
+#include "../../warn.h"
static const char *reg_name(unsigned int reg)
{
diff --git a/tools/objtool/orc_gen.c b/tools/objtool/arch/x86/orc_gen.c
similarity index 66%
rename from tools/objtool/orc_gen.c
rename to tools/objtool/arch/x86/orc_gen.c
index 27a4112848c2..b4f285bf5271 100644
--- a/tools/objtool/orc_gen.c
+++ b/tools/objtool/arch/x86/orc_gen.c
@@ -6,11 +6,11 @@
#include <stdlib.h>
#include <string.h>
-#include "orc.h"
-#include "check.h"
-#include "warn.h"
+#include "../../orc.h"
+#include "../../check.h"
+#include "../../warn.h"
-int create_orc(struct objtool_file *file)
+int arch_create_orc(struct objtool_file *file)
{
struct instruction *insn;
@@ -116,7 +116,7 @@ static int create_orc_entry(struct section *u_sec, struct section *ip_relasec,
return 0;
}
-int create_orc_sections(struct objtool_file *file)
+int arch_create_orc_sections(struct objtool_file *file)
{
struct instruction *insn, *prev_insn;
struct section *sec, *u_sec, *ip_relasec;
@@ -209,3 +209,97 @@ int create_orc_sections(struct objtool_file *file)
return 0;
}
+
+int arch_orc_read_unwind_hints(struct objtool_file *file)
+{
+ struct section *sec, *relasec;
+ struct rela *rela;
+ struct unwind_hint *hint;
+ struct instruction *insn;
+ struct cfi_reg *cfa;
+ int i;
+
+ sec = find_section_by_name(file->elf, ".discard.unwind_hints");
+ if (!sec)
+ return 0;
+
+ relasec = sec->rela;
+ if (!relasec) {
+ WARN("missing .rela.discard.unwind_hints section");
+ return -1;
+ }
+
+ if (sec->len % sizeof(struct unwind_hint)) {
+ WARN("struct unwind_hint size mismatch");
+ return -1;
+ }
+
+ file->hints = true;
+
+ for (i = 0; i < sec->len / sizeof(struct unwind_hint); i++) {
+ hint = (struct unwind_hint *)sec->data->d_buf + i;
+
+ rela = find_rela_by_dest(sec, i * sizeof(*hint));
+ if (!rela) {
+ WARN("can't find rela for unwind_hints[%d]", i);
+ return -1;
+ }
+
+ insn = find_insn(file, rela->sym->sec, rela->addend);
+ if (!insn) {
+ WARN("can't find insn for unwind_hints[%d]", i);
+ return -1;
+ }
+
+ cfa = &insn->state.cfa;
+
+ if (hint->type == UNWIND_HINT_TYPE_SAVE) {
+ insn->save = true;
+ continue;
+
+ } else if (hint->type == UNWIND_HINT_TYPE_RESTORE) {
+ insn->restore = true;
+ insn->hint = true;
+ continue;
+ }
+
+ insn->hint = true;
+
+ switch (hint->sp_reg) {
+ case ORC_REG_UNDEFINED:
+ cfa->base = CFI_UNDEFINED;
+ break;
+ case ORC_REG_SP:
+ cfa->base = CFI_SP;
+ break;
+ case ORC_REG_BP:
+ cfa->base = CFI_BP;
+ break;
+ case ORC_REG_SP_INDIRECT:
+ cfa->base = CFI_SP_INDIRECT;
+ break;
+ case ORC_REG_R10:
+ cfa->base = CFI_R10;
+ break;
+ case ORC_REG_R13:
+ cfa->base = CFI_R13;
+ break;
+ case ORC_REG_DI:
+ cfa->base = CFI_DI;
+ break;
+ case ORC_REG_DX:
+ cfa->base = CFI_DX;
+ break;
+ default:
+ WARN_FUNC("unsupported unwind_hint sp base reg %d",
+ insn->sec, insn->offset, hint->sp_reg);
+ return -1;
+ }
+
+ cfa->offset = hint->sp_offset;
+ insn->state.type = hint->type;
+ insn->state.end = hint->end;
+ }
+
+ return 0;
+}
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 479fab46b656..30e147391dcb 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1162,99 +1162,6 @@ static int add_jump_table_alts(struct objtool_file *file)
return 0;
}
-static int read_unwind_hints(struct objtool_file *file)
-{
- struct section *sec, *relasec;
- struct rela *rela;
- struct unwind_hint *hint;
- struct instruction *insn;
- struct cfi_reg *cfa;
- int i;
-
- sec = find_section_by_name(file->elf, ".discard.unwind_hints");
- if (!sec)
- return 0;
-
- relasec = sec->rela;
- if (!relasec) {
- WARN("missing .rela.discard.unwind_hints section");
- return -1;
- }
-
- if (sec->len % sizeof(struct unwind_hint)) {
- WARN("struct unwind_hint size mismatch");
- return -1;
- }
-
- file->hints = true;
-
- for (i = 0; i < sec->len / sizeof(struct unwind_hint); i++) {
- hint = (struct unwind_hint *)sec->data->d_buf + i;
-
- rela = find_rela_by_dest(sec, i * sizeof(*hint));
- if (!rela) {
- WARN("can't find rela for unwind_hints[%d]", i);
- return -1;
- }
-
- insn = find_insn(file, rela->sym->sec, rela->addend);
- if (!insn) {
- WARN("can't find insn for unwind_hints[%d]", i);
- return -1;
- }
-
- cfa = &insn->state.cfa;
-
- if (hint->type == UNWIND_HINT_TYPE_SAVE) {
- insn->save = true;
- continue;
-
- } else if (hint->type == UNWIND_HINT_TYPE_RESTORE) {
- insn->restore = true;
- insn->hint = true;
- continue;
- }
-
- insn->hint = true;
-
- switch (hint->sp_reg) {
- case ORC_REG_UNDEFINED:
- cfa->base = CFI_UNDEFINED;
- break;
- case ORC_REG_SP:
- cfa->base = CFI_SP;
- break;
- case ORC_REG_BP:
- cfa->base = CFI_BP;
- break;
- case ORC_REG_SP_INDIRECT:
- cfa->base = CFI_SP_INDIRECT;
- break;
- case ORC_REG_R10:
- cfa->base = CFI_R10;
- break;
- case ORC_REG_R13:
- cfa->base = CFI_R13;
- break;
- case ORC_REG_DI:
- cfa->base = CFI_DI;
- break;
- case ORC_REG_DX:
- cfa->base = CFI_DX;
- break;
- default:
- WARN_FUNC("unsupported unwind_hint sp base reg %d",
- insn->sec, insn->offset, hint->sp_reg);
- return -1;
- }
-
- cfa->offset = hint->sp_offset;
- insn->state.type = hint->type;
- insn->state.end = hint->end;
- }
-
- return 0;
-}
static int read_retpoline_hints(struct objtool_file *file)
{
@@ -1354,7 +1261,7 @@ static int decode_sections(struct objtool_file *file)
if (ret)
return ret;
- ret = read_unwind_hints(file);
+ ret = arch_orc_read_unwind_hints(file);
if (ret)
return ret;
@@ -2479,11 +2386,11 @@ int check(const char *_objname, bool orc)
}
if (orc) {
- ret = create_orc(&file);
+ ret = arch_create_orc(&file);
if (ret < 0)
goto out;
- ret = create_orc_sections(&file);
+ ret = arch_create_orc_sections(&file);
if (ret < 0)
goto out;
diff --git a/tools/objtool/orc.h b/tools/objtool/orc.h
index ee2832221e62..fd7fa1d34a81 100644
--- a/tools/objtool/orc.h
+++ b/tools/objtool/orc.h
@@ -10,8 +10,8 @@
struct objtool_file;
-int create_orc(struct objtool_file *file);
-int create_orc_sections(struct objtool_file *file);
+int arch_create_orc(struct objtool_file *file);
+int arch_create_orc_sections(struct objtool_file *file);
int orc_dump(const char *objname);
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [RFC v4 03/18] objtool: Move registers and control flow to arch-dependent code
From: Raphael Gault @ 2019-08-16 12:23 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, jpoimboe
Cc: raph.gault+kdev, peterz, catalin.marinas, will.deacon,
Raphael Gault, julien.thierry.kdev
In-Reply-To: <20190816122403.14994-1-raphael.gault@arm.com>
The control flow information and register macro definitions were based on
the x86_64 architecture but should be abstract so that each architecture
can define the correct values for the registers, especially the registers
related to the stack frame (Frame Pointer, Stack Pointer and possibly
Return Address).
Signed-off-by: Raphael Gault <raphael.gault@arm.com>
---
tools/objtool/arch/x86/include/arch_special.h | 36 +++++++++++++++++++
tools/objtool/{ => arch/x86/include}/cfi.h | 0
tools/objtool/check.h | 1 +
tools/objtool/special.c | 19 +---------
4 files changed, 38 insertions(+), 18 deletions(-)
create mode 100644 tools/objtool/arch/x86/include/arch_special.h
rename tools/objtool/{ => arch/x86/include}/cfi.h (100%)
diff --git a/tools/objtool/arch/x86/include/arch_special.h b/tools/objtool/arch/x86/include/arch_special.h
new file mode 100644
index 000000000000..424ce47013e3
--- /dev/null
+++ b/tools/objtool/arch/x86/include/arch_special.h
@@ -0,0 +1,36 @@
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef _X86_ARCH_SPECIAL_H
+#define _X86_ARCH_SPECIAL_H
+
+#define EX_ENTRY_SIZE 12
+#define EX_ORIG_OFFSET 0
+#define EX_NEW_OFFSET 4
+
+#define JUMP_ENTRY_SIZE 16
+#define JUMP_ORIG_OFFSET 0
+#define JUMP_NEW_OFFSET 4
+
+#define ALT_ENTRY_SIZE 13
+#define ALT_ORIG_OFFSET 0
+#define ALT_NEW_OFFSET 4
+#define ALT_FEATURE_OFFSET 8
+#define ALT_ORIG_LEN_OFFSET 10
+#define ALT_NEW_LEN_OFFSET 11
+
+#define X86_FEATURE_POPCNT (4 * 32 + 23)
+#define X86_FEATURE_SMAP (9 * 32 + 20)
+
+#endif /* _X86_ARCH_SPECIAL_H */
diff --git a/tools/objtool/cfi.h b/tools/objtool/arch/x86/include/cfi.h
similarity index 100%
rename from tools/objtool/cfi.h
rename to tools/objtool/arch/x86/include/cfi.h
diff --git a/tools/objtool/check.h b/tools/objtool/check.h
index 6d875ca6fce0..af87b55db454 100644
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -11,6 +11,7 @@
#include "cfi.h"
#include "arch.h"
#include "orc.h"
+#include "arch_special.h"
#include <linux/hashtable.h>
struct insn_state {
diff --git a/tools/objtool/special.c b/tools/objtool/special.c
index fdbaa611146d..b8ccee1b5382 100644
--- a/tools/objtool/special.c
+++ b/tools/objtool/special.c
@@ -14,24 +14,7 @@
#include "builtin.h"
#include "special.h"
#include "warn.h"
-
-#define EX_ENTRY_SIZE 12
-#define EX_ORIG_OFFSET 0
-#define EX_NEW_OFFSET 4
-
-#define JUMP_ENTRY_SIZE 16
-#define JUMP_ORIG_OFFSET 0
-#define JUMP_NEW_OFFSET 4
-
-#define ALT_ENTRY_SIZE 13
-#define ALT_ORIG_OFFSET 0
-#define ALT_NEW_OFFSET 4
-#define ALT_FEATURE_OFFSET 8
-#define ALT_ORIG_LEN_OFFSET 10
-#define ALT_NEW_LEN_OFFSET 11
-
-#define X86_FEATURE_POPCNT (4*32+23)
-#define X86_FEATURE_SMAP (9*32+20)
+#include "arch_special.h"
struct special_entry {
const char *sec;
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [RFC v4 05/18] objtool: special: Adapt special section handling
From: Raphael Gault @ 2019-08-16 12:23 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, jpoimboe
Cc: raph.gault+kdev, peterz, catalin.marinas, will.deacon,
Raphael Gault, julien.thierry.kdev
In-Reply-To: <20190816122403.14994-1-raphael.gault@arm.com>
This patch abstracts the few architecture dependent tests that are
perform when handling special section and switch tables. It enables any
architecture to ignore a particular CPU feature or not to handle switch
tables.
Signed-off-by: Raphael Gault <raphael.gault@arm.com>
---
tools/objtool/arch/arm64/Build | 1 +
tools/objtool/arch/arm64/arch_special.c | 22 +++++++++++++++
.../objtool/arch/arm64/include/arch_special.h | 10 +++++--
tools/objtool/arch/x86/Build | 1 +
tools/objtool/arch/x86/arch_special.c | 28 +++++++++++++++++++
tools/objtool/arch/x86/include/arch_special.h | 9 ++++++
tools/objtool/check.c | 24 ++++++++++++++--
tools/objtool/special.c | 9 ++----
tools/objtool/special.h | 3 ++
9 files changed, 96 insertions(+), 11 deletions(-)
create mode 100644 tools/objtool/arch/arm64/arch_special.c
create mode 100644 tools/objtool/arch/x86/arch_special.c
diff --git a/tools/objtool/arch/arm64/Build b/tools/objtool/arch/arm64/Build
index bf7a32c2b9e9..3d09be745a84 100644
--- a/tools/objtool/arch/arm64/Build
+++ b/tools/objtool/arch/arm64/Build
@@ -1,3 +1,4 @@
+objtool-y += arch_special.o
objtool-y += decode.o
objtool-y += orc_dump.o
objtool-y += orc_gen.o
diff --git a/tools/objtool/arch/arm64/arch_special.c b/tools/objtool/arch/arm64/arch_special.c
new file mode 100644
index 000000000000..a21d28876317
--- /dev/null
+++ b/tools/objtool/arch/arm64/arch_special.c
@@ -0,0 +1,22 @@
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#include "../../special.h"
+#include "arch_special.h"
+
+void arch_force_alt_path(unsigned short feature,
+ bool uaccess,
+ struct special_alt *alt)
+{
+}
diff --git a/tools/objtool/arch/arm64/include/arch_special.h b/tools/objtool/arch/arm64/include/arch_special.h
index 63da775d0581..185103be8a51 100644
--- a/tools/objtool/arch/arm64/include/arch_special.h
+++ b/tools/objtool/arch/arm64/include/arch_special.h
@@ -30,7 +30,13 @@
#define ALT_ORIG_LEN_OFFSET 10
#define ALT_NEW_LEN_OFFSET 11
-#define X86_FEATURE_POPCNT (4 * 32 + 23)
-#define X86_FEATURE_SMAP (9 * 32 + 20)
+static inline bool arch_should_ignore_feature(unsigned short feature)
+{
+ return false;
+}
+static inline bool arch_support_switch_table(void)
+{
+ return false;
+}
#endif /* _ARM64_ARCH_SPECIAL_H */
diff --git a/tools/objtool/arch/x86/Build b/tools/objtool/arch/x86/Build
index 1f11b45999d0..63e167775bc8 100644
--- a/tools/objtool/arch/x86/Build
+++ b/tools/objtool/arch/x86/Build
@@ -1,3 +1,4 @@
+objtool-y += arch_special.o
objtool-y += decode.o
objtool-y += orc_dump.o
objtool-y += orc_gen.o
diff --git a/tools/objtool/arch/x86/arch_special.c b/tools/objtool/arch/x86/arch_special.c
new file mode 100644
index 000000000000..6583a1770bb2
--- /dev/null
+++ b/tools/objtool/arch/x86/arch_special.c
@@ -0,0 +1,28 @@
+/*
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see <http://www.gnu.org/licenses/>.
+ */
+#include "../../special.h"
+#include "arch_special.h"
+
+void arch_force_alt_path(unsigned short feature,
+ bool uaccess,
+ struct special_alt *alt)
+{
+ if (feature == X86_FEATURE_SMAP) {
+ if (uaccess)
+ alt->skip_orig = true;
+ else
+ alt->skip_alt = true;
+ }
+}
diff --git a/tools/objtool/arch/x86/include/arch_special.h b/tools/objtool/arch/x86/include/arch_special.h
index 424ce47013e3..fce2b1193194 100644
--- a/tools/objtool/arch/x86/include/arch_special.h
+++ b/tools/objtool/arch/x86/include/arch_special.h
@@ -33,4 +33,13 @@
#define X86_FEATURE_POPCNT (4 * 32 + 23)
#define X86_FEATURE_SMAP (9 * 32 + 20)
+static inline bool arch_should_ignore_feature(unsigned short feature)
+{
+ return feature == X86_FEATURE_POPCNT;
+}
+
+static inline bool arch_support_switch_table(void)
+{
+ return true;
+}
#endif /* _X86_ARCH_SPECIAL_H */
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 30e147391dcb..4af6422d3428 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -729,7 +729,7 @@ static int handle_group_alt(struct objtool_file *file,
last_orig_insn = insn;
}
- if (next_insn_same_sec(file, last_orig_insn)) {
+ if (last_orig_insn && next_insn_same_sec(file, last_orig_insn)) {
fake_jump = malloc(sizeof(*fake_jump));
if (!fake_jump) {
WARN("malloc failed");
@@ -1061,6 +1061,26 @@ static struct rela *find_jump_table(struct objtool_file *file,
table_rela = find_rela_by_dest(table_sec, table_offset);
if (!table_rela)
continue;
+ /*
+ * If we are on arm64 architecture, we now that we
+ * are in presence of a switch table thanks to
+ * the `br <Xn>` insn. but we can't retrieve it yet.
+ * So we just ignore unreachable for this file.
+ */
+ if (!arch_support_switch_table()) {
+ file->ignore_unreachables = true;
+ return NULL;
+ }
+
+ rodata_rela = find_rela_by_dest(rodata_sec, table_offset);
+ if (rodata_rela) {
+ /*
+ * Use of RIP-relative switch jumps is quite rare, and
+ * indicates a rare GCC quirk/bug which can leave dead
+ * code behind.
+ */
+ if (text_rela->type == R_X86_64_PC32)
+ file->ignore_unreachables = true;
/*
* Use of RIP-relative switch jumps is quite rare, and
@@ -1864,7 +1884,7 @@ static int validate_branch(struct objtool_file *file, struct symbol *func,
insn = first;
sec = insn->sec;
- if (insn->alt_group && list_empty(&insn->alts)) {
+ if (!insn->visited && insn->alt_group && list_empty(&insn->alts)) {
WARN_FUNC("don't know how to handle branch to middle of alternative instruction group",
sec, insn->offset);
return 1;
diff --git a/tools/objtool/special.c b/tools/objtool/special.c
index b8ccee1b5382..7a0092d6e5b3 100644
--- a/tools/objtool/special.c
+++ b/tools/objtool/special.c
@@ -81,7 +81,7 @@ static int get_alt_entry(struct elf *elf, struct special_entry *entry,
* feature path which is a "very very small percentage of
* machines".
*/
- if (feature == X86_FEATURE_POPCNT)
+ if (arch_should_ignore_feature(feature))
alt->skip_orig = true;
/*
@@ -93,12 +93,7 @@ static int get_alt_entry(struct elf *elf, struct special_entry *entry,
* find paths that see the STAC but take the NOP instead of
* CLAC and the other way around.
*/
- if (feature == X86_FEATURE_SMAP) {
- if (uaccess)
- alt->skip_orig = true;
- else
- alt->skip_alt = true;
- }
+ arch_force_alt_path(feature, uaccess, alt);
}
orig_rela = find_rela_by_dest(sec, offset + entry->orig);
diff --git a/tools/objtool/special.h b/tools/objtool/special.h
index 35061530e46e..90626a7e41cf 100644
--- a/tools/objtool/special.h
+++ b/tools/objtool/special.h
@@ -27,5 +27,8 @@ struct special_alt {
};
int special_get_alts(struct elf *elf, struct list_head *alts);
+void arch_force_alt_path(unsigned short feature,
+ bool uaccess,
+ struct special_alt *alt);
#endif /* _SPECIAL_H */
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [RFC v4 06/18] objtool: arm64: Adapt the stack frame checks for arm architecture
From: Raphael Gault @ 2019-08-16 12:23 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, jpoimboe
Cc: raph.gault+kdev, peterz, catalin.marinas, will.deacon,
Raphael Gault, julien.thierry.kdev
In-Reply-To: <20190816122403.14994-1-raphael.gault@arm.com>
Since the way the initial stack frame when entering a function is different
that what is done in the x86_64 architecture, we need to add some more
check to support the different cases. As opposed as for x86_64, the return
address is not stored by the call instruction but is instead loaded in a
register. The initial stack frame is thus empty when entering a function
and 2 push operations are needed to set it up correctly. All the different
combinations need to be taken into account.
Signed-off-by: Raphael Gault <raphael.gault@arm.com>
---
tools/objtool/arch.h | 2 +
tools/objtool/arch/arm64/decode.c | 28 +++++++++
tools/objtool/arch/x86/decode.c | 5 ++
tools/objtool/check.c | 100 ++++++++++++++++++++++++++++--
tools/objtool/elf.c | 3 +-
5 files changed, 131 insertions(+), 7 deletions(-)
diff --git a/tools/objtool/arch.h b/tools/objtool/arch.h
index bb5ce810fb6e..68d6371a24a2 100644
--- a/tools/objtool/arch.h
+++ b/tools/objtool/arch.h
@@ -91,4 +91,6 @@ unsigned long arch_jump_destination(struct instruction *insn);
unsigned long arch_dest_rela_offset(int addend);
+bool arch_is_insn_sibling_call(struct instruction *insn);
+
#endif /* _ARCH_H */
diff --git a/tools/objtool/arch/arm64/decode.c b/tools/objtool/arch/arm64/decode.c
index 395c5777afab..be3d2eb10227 100644
--- a/tools/objtool/arch/arm64/decode.c
+++ b/tools/objtool/arch/arm64/decode.c
@@ -106,6 +106,34 @@ unsigned long arch_dest_rela_offset(int addend)
return addend;
}
+/*
+ * In order to know if we are in presence of a sibling
+ * call and not in presence of a switch table we look
+ * back at the previous instructions and see if we are
+ * jumping inside the same function that we are already
+ * in.
+ */
+bool arch_is_insn_sibling_call(struct instruction *insn)
+{
+ struct instruction *prev;
+ struct list_head *l;
+ struct symbol *sym;
+ list_for_each_prev(l, &insn->list) {
+ prev = list_entry(l, struct instruction, list);
+ if (!prev->func ||
+ prev->func->pfunc != insn->func->pfunc)
+ return false;
+ if (prev->stack_op.src.reg != ADR_SOURCE)
+ continue;
+ sym = find_symbol_containing(insn->sec, insn->immediate);
+ if (!sym || sym->type != STT_FUNC)
+ return false;
+ else if (sym->type == STT_FUNC)
+ return true;
+ break;
+ }
+ return false;
+}
static int is_arm64(struct elf *elf)
{
switch (elf->ehdr.e_machine) {
diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index fa33b3465722..98726990714d 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -72,6 +72,11 @@ unsigned long arch_dest_rela_offset(int addend)
return addend + 4;
}
+bool arch_is_insn_sibling_call(struct instruction *insn)
+{
+ return true;
+}
+
int arch_decode_instruction(struct elf *elf, struct section *sec,
unsigned long offset, unsigned int maxlen,
unsigned int *len, enum insn_type *type,
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 4af6422d3428..519569b0329f 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -566,10 +566,10 @@ static int add_jump_destinations(struct objtool_file *file)
dest_off = arch_jump_destination(insn);
} else if (rela->sym->type == STT_SECTION) {
dest_sec = rela->sym->sec;
- dest_off = rela->addend + 4;
+ dest_off = arch_dest_rela_offset(rela->addend);
} else if (rela->sym->sec->idx) {
dest_sec = rela->sym->sec;
- dest_off = rela->sym->sym.st_value + rela->addend + 4;
+ dest_off = rela->sym->sym.st_value + arch_dest_rela_offset(rela->addend);
} else if (strstr(rela->sym->name, "_indirect_thunk_")) {
/*
* Retpoline jumps are really dynamic jumps in
@@ -1368,8 +1368,8 @@ static void save_reg(struct insn_state *state, unsigned char reg, int base,
static void restore_reg(struct insn_state *state, unsigned char reg)
{
- state->regs[reg].base = CFI_UNDEFINED;
- state->regs[reg].offset = 0;
+ state->regs[reg].base = initial_func_cfi.regs[reg].base;
+ state->regs[reg].offset = initial_func_cfi.regs[reg].offset;
}
/*
@@ -1525,8 +1525,32 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
/* add imm, %rsp */
state->stack_size -= op->src.offset;
- if (cfa->base == CFI_SP)
+ if (cfa->base == CFI_SP) {
cfa->offset -= op->src.offset;
+ if (state->stack_size == 0 &&
+ initial_func_cfi.cfa.base == CFI_CFA) {
+ cfa->base = CFI_CFA;
+ cfa->offset = 0;
+ }
+ }
+ /*
+ * on arm64 the save/restore of sp into fp is not automatic
+ * and the first one can be done without the other so we
+ * need to be careful not to invalidate the stack frame in such
+ * cases.
+ */
+ else if (cfa->base == CFI_BP) {
+ if (state->stack_size == 0 &&
+ initial_func_cfi.cfa.base == CFI_CFA) {
+ cfa->base = CFI_CFA;
+ cfa->offset = 0;
+ restore_reg(state, CFI_BP);
+ }
+ } else if (cfa->base == CFI_CFA) {
+ cfa->base = CFI_SP;
+ if (state->stack_size >= 16)
+ cfa->offset = 16;
+ }
break;
}
@@ -1537,6 +1561,15 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
break;
}
+ if (op->src.reg == CFI_SP && op->dest.reg == CFI_BP &&
+ cfa->base == CFI_SP &&
+ regs[CFI_BP].base == CFI_CFA &&
+ regs[CFI_BP].offset == -cfa->offset) {
+ /* mov %rsp, %rbp */
+ cfa->base = op->dest.reg;
+ state->bp_scratch = false;
+ break;
+ }
if (op->src.reg == CFI_SP && cfa->base == CFI_SP) {
/* drap: lea disp(%rsp), %drap */
@@ -1629,6 +1662,22 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
state->stack_size -= 8;
if (cfa->base == CFI_SP)
cfa->offset -= 8;
+ if (cfa->base == CFI_SP &&
+ cfa->offset == 0 &&
+ initial_func_cfi.cfa.base == CFI_CFA)
+ cfa->base = CFI_CFA;
+
+ if (op->extra.used) {
+ if (regs[op->extra.reg].offset == -state->stack_size)
+ restore_reg(state, op->extra.reg);
+ state->stack_size -= 8;
+ if (cfa->base == CFI_SP)
+ cfa->offset -= 8;
+ if (cfa->base == CFI_SP &&
+ cfa->offset == 0 &&
+ initial_func_cfi.cfa.base == CFI_CFA)
+ cfa->base = CFI_CFA;
+ }
break;
@@ -1648,12 +1697,22 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
/* drap: mov disp(%rbp), %reg */
restore_reg(state, op->dest.reg);
+ if (op->extra.used &&
+ op->src.reg == CFI_BP &&
+ op->extra.offset == regs[op->extra.reg].offset)
+ restore_reg(state, op->extra.reg);
+
} else if (op->src.reg == cfa->base &&
op->src.offset == regs[op->dest.reg].offset + cfa->offset) {
/* mov disp(%rbp), %reg */
/* mov disp(%rsp), %reg */
restore_reg(state, op->dest.reg);
+
+ if (op->extra.used &&
+ op->src.reg == cfa->base &&
+ op->extra.offset == regs[op->extra.reg].offset + cfa->offset)
+ restore_reg(state, op->extra.reg);
}
break;
@@ -1669,6 +1728,8 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
case OP_DEST_PUSH:
case OP_DEST_PUSHF:
state->stack_size += 8;
+ if (cfa->base == CFI_CFA)
+ cfa->base = CFI_SP;
if (cfa->base == CFI_SP)
cfa->offset += 8;
@@ -1702,6 +1763,21 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
save_reg(state, op->src.reg, CFI_CFA, -state->stack_size);
}
+ if (op->extra.used) {
+ state->stack_size += 8;
+ if (cfa->base == CFI_CFA)
+ cfa->base = CFI_SP;
+ if (cfa->base == CFI_SP)
+ cfa->offset += 8;
+ if (!state->drap ||
+ (!(op->extra.reg == cfa->base &&
+ op->extra.reg == state->drap_reg) &&
+ !(op->extra.reg == CFI_BP &&
+ cfa->base == state->drap_reg) &&
+ regs[op->extra.reg].base == CFI_UNDEFINED))
+ save_reg(state, op->extra.reg, CFI_CFA,
+ -state->stack_size);
+ }
/* detect when asm code uses rbp as a scratch register */
if (!no_fp && insn->func && op->src.reg == CFI_BP &&
cfa->base != CFI_BP)
@@ -1720,11 +1796,19 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
/* save drap offset so we know when to restore it */
state->drap_offset = op->dest.offset;
}
+ if (op->extra.used && op->extra.reg == cfa->base &&
+ op->extra.reg == state->drap_reg) {
+ cfa->base = CFI_BP_INDIRECT;
+ cfa->offset = op->extra.offset;
+ }
else if (regs[op->src.reg].base == CFI_UNDEFINED) {
/* drap: mov reg, disp(%rbp) */
save_reg(state, op->src.reg, CFI_BP, op->dest.offset);
+ if (op->extra.used)
+ save_reg(state, op->extra.reg, CFI_BP,
+ op->extra.offset);
}
} else if (op->dest.reg == cfa->base) {
@@ -1733,8 +1817,12 @@ static int update_insn_state(struct instruction *insn, struct insn_state *state)
/* mov reg, disp(%rsp) */
save_reg(state, op->src.reg, CFI_CFA,
op->dest.offset - state->cfa.offset);
+ if (op->extra.used)
+ save_reg(state, op->extra.reg, CFI_CFA,
+ op->extra.offset - state->cfa.offset);
}
+
break;
case OP_DEST_LEAVE:
@@ -1857,7 +1945,7 @@ static int validate_call(struct instruction *insn, struct insn_state *state)
static int validate_sibling_call(struct instruction *insn, struct insn_state *state)
{
- if (has_modified_stack_frame(state)) {
+ if (arch_is_insn_sibling_call(insn) && has_modified_stack_frame(state)) {
WARN_FUNC("sibling call from callable instruction with modified stack frame",
insn->sec, insn->offset);
return 1;
diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
index edba4745f25a..c6ac0b771b73 100644
--- a/tools/objtool/elf.c
+++ b/tools/objtool/elf.c
@@ -62,7 +62,8 @@ struct symbol *find_symbol_by_offset(struct section *sec, unsigned long offset)
struct symbol *sym;
list_for_each_entry(sym, &sec->symbol_list, list)
- if (sym->type != STT_SECTION &&
+ if (sym->type != STT_NOTYPE &&
+ sym->type != STT_SECTION &&
sym->offset == offset)
return sym;
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [RFC v4 08/18] objtool: Refactor switch-tables code to support other architectures
From: Raphael Gault @ 2019-08-16 12:23 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, jpoimboe
Cc: raph.gault+kdev, peterz, catalin.marinas, will.deacon,
Raphael Gault, julien.thierry.kdev
In-Reply-To: <20190816122403.14994-1-raphael.gault@arm.com>
The way to identify switch-tables and retrieves all the data necessary
to handle the different execution branches is not the same on all
architecture. In order to be able to add other architecture support,
this patch defines arch-dependent functions to process jump-tables.
Signed-off-by: Raphael Gault <raphael.gault@arm.com>
---
tools/objtool/arch/arm64/arch_special.c | 15 ++++
tools/objtool/arch/arm64/decode.c | 4 +-
tools/objtool/arch/x86/arch_special.c | 79 ++++++++++++++++++++
tools/objtool/check.c | 95 +------------------------
tools/objtool/check.h | 7 ++
tools/objtool/special.h | 10 ++-
6 files changed, 114 insertions(+), 96 deletions(-)
diff --git a/tools/objtool/arch/arm64/arch_special.c b/tools/objtool/arch/arm64/arch_special.c
index a21d28876317..17a8a06aac2a 100644
--- a/tools/objtool/arch/arm64/arch_special.c
+++ b/tools/objtool/arch/arm64/arch_special.c
@@ -20,3 +20,18 @@ void arch_force_alt_path(unsigned short feature,
struct special_alt *alt)
{
}
+
+int arch_add_jump_table(struct objtool_file *file, struct instruction *insn,
+ struct rela *table, struct rela *next_table)
+{
+ return 0;
+}
+
+struct rela *arch_find_switch_table(struct objtool_file *file,
+ struct rela *text_rela,
+ struct section *rodata_sec,
+ unsigned long table_offset)
+{
+ file->ignore_unreachables = true;
+ return NULL;
+}
diff --git a/tools/objtool/arch/arm64/decode.c b/tools/objtool/arch/arm64/decode.c
index 4cb9402d6fe1..a20725c1bfd7 100644
--- a/tools/objtool/arch/arm64/decode.c
+++ b/tools/objtool/arch/arm64/decode.c
@@ -159,7 +159,7 @@ static int is_arm64(struct elf *elf)
int arch_decode_instruction(struct elf *elf, struct section *sec,
unsigned long offset, unsigned int maxlen,
- unsigned int *len, unsigned char *type,
+ unsigned int *len, enum insn_type *type,
unsigned long *immediate, struct stack_op *op)
{
int arm64 = 0;
@@ -184,7 +184,7 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
insn = *(u32 *)(sec->data->d_buf + offset);
//dispatch according to encoding classes
- return aarch64_insn_class_decode_table[(insn >> 25) & 0xf](insn, type,
+ return aarch64_insn_class_decode_table[(insn >> 25) & 0xf](insn, (unsigned char *)type,
immediate, op);
}
diff --git a/tools/objtool/arch/x86/arch_special.c b/tools/objtool/arch/x86/arch_special.c
index 6583a1770bb2..c097001d805b 100644
--- a/tools/objtool/arch/x86/arch_special.c
+++ b/tools/objtool/arch/x86/arch_special.c
@@ -26,3 +26,82 @@ void arch_force_alt_path(unsigned short feature,
alt->skip_alt = true;
}
}
+
+int arch_add_jump_table(struct objtool_file *file, struct instruction *insn,
+ struct rela *table, struct rela *next_table)
+{
+ struct rela *rela = table;
+ struct instruction *dest_insn;
+ struct alternative *alt;
+ struct symbol *pfunc = insn->func->pfunc;
+ unsigned int prev_offset = 0;
+
+ /*
+ * Each @rela is a switch table relocation which points to the target
+ * instruction.
+ */
+ list_for_each_entry_from(rela, &table->sec->rela_list, list) {
+
+ /* Check for the end of the table: */
+ if (rela != table && rela->jump_table_start)
+ break;
+
+ /* Make sure the table entries are consecutive: */
+ if (prev_offset && rela->offset != prev_offset + 8)
+ break;
+
+ /* Detect function pointers from contiguous objects: */
+ if (rela->sym->sec == pfunc->sec &&
+ rela->addend == pfunc->offset)
+ break;
+
+ dest_insn = find_insn(file, rela->sym->sec, rela->addend);
+ if (!dest_insn)
+ break;
+
+ /* Make sure the destination is in the same function: */
+ if (!dest_insn->func || dest_insn->func->pfunc != pfunc)
+ break;
+
+ alt = malloc(sizeof(*alt));
+ if (!alt) {
+ WARN("malloc failed");
+ return -1;
+ }
+
+ alt->insn = dest_insn;
+ list_add_tail(&alt->list, &insn->alts);
+ prev_offset = rela->offset;
+ }
+
+ if (!prev_offset) {
+ WARN_FUNC("can't find switch jump table",
+ insn->sec, insn->offset);
+ return -1;
+ }
+
+ return 0;
+}
+
+struct rela *arch_find_switch_table(struct objtool_file *file,
+ struct rela *text_rela,
+ struct section *rodata_sec,
+ unsigned long table_offset)
+{
+ struct rela *rodata_rela;
+
+ rodata_rela = find_rela_by_dest(rodata_sec, table_offset);
+ if (rodata_rela) {
+ /*
+ * Use of RIP-relative switch jumps is quite rare, and
+ * indicates a rare GCC quirk/bug which can leave dead
+ * code behind.
+ */
+ if (text_rela->type == R_X86_64_PC32)
+ file->ignore_unreachables = true;
+
+ return rodata_rela;
+ }
+
+ return NULL;
+}
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index baa6a93f37cd..18f7fb47392a 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -20,12 +20,6 @@
#define C_JUMP_TABLE_SECTION ".rodata..c_jump_table"
-struct alternative {
- struct list_head list;
- struct instruction *insn;
- bool skip_orig;
-};
-
const char *objname;
struct cfi_state initial_func_cfi;
@@ -901,62 +895,6 @@ static int add_special_section_alts(struct objtool_file *file)
return ret;
}
-static int add_jump_table(struct objtool_file *file, struct instruction *insn,
- struct rela *table)
-{
- struct rela *rela = table;
- struct instruction *dest_insn;
- struct alternative *alt;
- struct symbol *pfunc = insn->func->pfunc;
- unsigned int prev_offset = 0;
-
- /*
- * Each @rela is a switch table relocation which points to the target
- * instruction.
- */
- list_for_each_entry_from(rela, &table->sec->rela_list, list) {
-
- /* Check for the end of the table: */
- if (rela != table && rela->jump_table_start)
- break;
-
- /* Make sure the table entries are consecutive: */
- if (prev_offset && rela->offset != prev_offset + 8)
- break;
-
- /* Detect function pointers from contiguous objects: */
- if (rela->sym->sec == pfunc->sec &&
- rela->addend == pfunc->offset)
- break;
-
- dest_insn = find_insn(file, rela->sym->sec, rela->addend);
- if (!dest_insn)
- break;
-
- /* Make sure the destination is in the same function: */
- if (!dest_insn->func || dest_insn->func->pfunc != pfunc)
- break;
-
- alt = malloc(sizeof(*alt));
- if (!alt) {
- WARN("malloc failed");
- return -1;
- }
-
- alt->insn = dest_insn;
- list_add_tail(&alt->list, &insn->alts);
- prev_offset = rela->offset;
- }
-
- if (!prev_offset) {
- WARN_FUNC("can't find switch jump table",
- insn->sec, insn->offset);
- return -1;
- }
-
- return 0;
-}
-
/*
* find_jump_table() - Given a dynamic jump, find the switch jump table in
* .rodata associated with it.
@@ -1058,38 +996,9 @@ static struct rela *find_jump_table(struct objtool_file *file,
continue;
/* Each table entry has a rela associated with it. */
- table_rela = find_rela_by_dest(table_sec, table_offset);
+ table_rela = arch_find_switch_table(file, text_rela, table_sec, table_offset);
if (!table_rela)
continue;
- /*
- * If we are on arm64 architecture, we now that we
- * are in presence of a switch table thanks to
- * the `br <Xn>` insn. but we can't retrieve it yet.
- * So we just ignore unreachable for this file.
- */
- if (!arch_support_switch_table()) {
- file->ignore_unreachables = true;
- return NULL;
- }
-
- rodata_rela = find_rela_by_dest(rodata_sec, table_offset);
- if (rodata_rela) {
- /*
- * Use of RIP-relative switch jumps is quite rare, and
- * indicates a rare GCC quirk/bug which can leave dead
- * code behind.
- */
- if (text_rela->type == R_X86_64_PC32)
- file->ignore_unreachables = true;
-
- /*
- * Use of RIP-relative switch jumps is quite rare, and
- * indicates a rare GCC quirk/bug which can leave dead code
- * behind.
- */
- if (text_rela->type == R_X86_64_PC32)
- file->ignore_unreachables = true;
-
return table_rela;
}
@@ -1145,7 +1054,7 @@ static int add_func_jump_tables(struct objtool_file *file,
if (!insn->jump_table)
continue;
- ret = add_jump_table(file, insn, insn->jump_table);
+ ret = arch_add_jump_table(file, insn, insn->jump_table, NULL);
if (ret)
return ret;
}
diff --git a/tools/objtool/check.h b/tools/objtool/check.h
index af87b55db454..267759760a3d 100644
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -13,6 +13,7 @@
#include "orc.h"
#include "arch_special.h"
#include <linux/hashtable.h>
+;
struct insn_state {
struct cfi_reg cfa;
@@ -48,6 +49,12 @@ struct instruction {
struct orc_entry orc;
};
+struct alternative {
+ struct list_head list;
+ struct instruction *insn;
+ bool skip_orig;
+};
+
struct objtool_file {
struct elf *elf;
struct list_head insn_list;
diff --git a/tools/objtool/special.h b/tools/objtool/special.h
index 90626a7e41cf..9b1f968a4325 100644
--- a/tools/objtool/special.h
+++ b/tools/objtool/special.h
@@ -7,7 +7,10 @@
#define _SPECIAL_H
#include <stdbool.h>
+#include <stdlib.h>
+#include "check.h"
#include "elf.h"
+#include "warn.h"
struct special_alt {
struct list_head list;
@@ -30,5 +33,10 @@ int special_get_alts(struct elf *elf, struct list_head *alts);
void arch_force_alt_path(unsigned short feature,
bool uaccess,
struct special_alt *alt);
-
+int arch_add_jump_table(struct objtool_file *file, struct instruction *insn,
+ struct rela *table, struct rela *next_table);
+struct rela *arch_find_switch_table(struct objtool_file *file,
+ struct rela *text_rela,
+ struct section *rodata_sec,
+ unsigned long table_offset);
#endif /* _SPECIAL_H */
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [RFC v4 09/18] gcc-plugins: objtool: Add plugin to detect switch table on arm64
From: Raphael Gault @ 2019-08-16 12:23 UTC (permalink / raw)
To: linux-arm-kernel, linux-kernel, jpoimboe
Cc: raph.gault+kdev, peterz, catalin.marinas, will.deacon,
Raphael Gault, julien.thierry.kdev
In-Reply-To: <20190816122403.14994-1-raphael.gault@arm.com>
This plugins comes into play before the final 2 RTL passes of GCC and
detects switch-tables that are to be outputed in the ELF and writes
information in an "objtool_data" section which will be used by objtool.
Signed-off-by: Raphael Gault <raphael.gault@arm.com>
---
scripts/Makefile.gcc-plugins | 2 +
scripts/gcc-plugins/Kconfig | 9 +++
.../arm64_switch_table_detection_plugin.c | 58 +++++++++++++++++++
3 files changed, 69 insertions(+)
create mode 100644 scripts/gcc-plugins/arm64_switch_table_detection_plugin.c
diff --git a/scripts/Makefile.gcc-plugins b/scripts/Makefile.gcc-plugins
index 5f7df50cfe7a..a56736df9dc2 100644
--- a/scripts/Makefile.gcc-plugins
+++ b/scripts/Makefile.gcc-plugins
@@ -44,6 +44,8 @@ ifdef CONFIG_GCC_PLUGIN_ARM_SSP_PER_TASK
endif
export DISABLE_ARM_SSP_PER_TASK_PLUGIN
+gcc-plugin-$(CONFIG_GCC_PLUGIN_SWITCH_TABLES) += arm64_switch_table_detection_plugin.so
+
# All the plugin CFLAGS are collected here in case a build target needs to
# filter them out of the KBUILD_CFLAGS.
GCC_PLUGINS_CFLAGS := $(strip $(addprefix -fplugin=$(objtree)/scripts/gcc-plugins/, $(gcc-plugin-y)) $(gcc-plugin-cflags-y))
diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig
index d33de0b9f4f5..1daeffb55dce 100644
--- a/scripts/gcc-plugins/Kconfig
+++ b/scripts/gcc-plugins/Kconfig
@@ -113,4 +113,13 @@ config GCC_PLUGIN_ARM_SSP_PER_TASK
bool
depends on GCC_PLUGINS && ARM
+config GCC_PLUGIN_SWITCH_TABLES
+ bool "GCC Plugin: Identify switch tables at compile time"
+ default y
+ depends on STACK_VALIDATION && ARM64
+ help
+ Plugin to identify switch tables generated at compile time and store
+ them in a .objtool_data section. Objtool will then use that section
+ to analyse the different execution path of the switch table.
+
endmenu
diff --git a/scripts/gcc-plugins/arm64_switch_table_detection_plugin.c b/scripts/gcc-plugins/arm64_switch_table_detection_plugin.c
new file mode 100644
index 000000000000..d7f0e13910d5
--- /dev/null
+++ b/scripts/gcc-plugins/arm64_switch_table_detection_plugin.c
@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <stdio.h>
+#include "gcc-common.h"
+
+__visible int plugin_is_GPL_compatible;
+
+static unsigned int arm64_switchtbl_rtl_execute(void)
+{
+ rtx_insn *insn;
+ rtx_insn *labelp = NULL;
+ rtx_jump_table_data *tablep = NULL;
+ section *sec = get_section(".objtool_data", SECTION_STRINGS, NULL);
+ section *curr_sec = current_function_section();
+
+ for (insn = get_insns(); insn; insn = NEXT_INSN(insn)) {
+ /*
+ * Find a tablejump_p INSN (using a dispatch table)
+ */
+ if (!tablejump_p(insn, &labelp, &tablep))
+ continue;
+
+ if (labelp && tablep) {
+ switch_to_section(sec);
+ assemble_integer_with_op(".quad ", gen_rtx_LABEL_REF(Pmode, labelp));
+ assemble_integer_with_op(".quad ", GEN_INT(GET_NUM_ELEM(tablep->get_labels())));
+ assemble_integer_with_op(".quad ", GEN_INT(ADDR_DIFF_VEC_FLAGS(tablep).offset_unsigned));
+ switch_to_section(curr_sec);
+ }
+ }
+ return 0;
+}
+
+#define PASS_NAME arm64_switchtbl_rtl
+
+#define NO_GATE
+#include "gcc-generate-rtl-pass.h"
+
+__visible int plugin_init(struct plugin_name_args *plugin_info,
+ struct plugin_gcc_version *version)
+{
+ const char * const plugin_name = plugin_info->base_name;
+ int tso = 0;
+ int i;
+
+ if (!plugin_default_version_check(version, &gcc_version)) {
+ error(G_("incompatible gcc/plugin versions"));
+ return 1;
+ }
+
+ PASS_INFO(arm64_switchtbl_rtl, "outof_cfglayout", 1,
+ PASS_POS_INSERT_AFTER);
+
+ register_callback(plugin_info->base_name, PLUGIN_PASS_MANAGER_SETUP,
+ NULL, &arm64_switchtbl_rtl_pass_info);
+
+ return 0;
+}
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ 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