RE: [PATCH v2 08/10] PCI: layerscape: Add EP mode support for ls1088a and ls2088a
> -Original Message-
> From: christophe leroy
> Sent: 2019年8月24日 14:45
> To: Xiaowei Bao ; Andrew Murray
>
> Cc: mark.rutl...@arm.com; Roy Zang ;
> lorenzo.pieral...@arm.co; a...@arndb.de; devicet...@vger.kernel.org;
> gre...@linuxfoundation.org; linuxppc-dev@lists.ozlabs.org;
> linux-...@vger.kernel.org; linux-ker...@vger.kernel.org; kis...@ti.com; M.h.
> Lian ; robh...@kernel.org;
> gustavo.pimen...@synopsys.com; jingooh...@gmail.com;
> bhelg...@google.com; Leo Li ; shawn...@kernel.org;
> Mingkai Hu ; linux-arm-ker...@lists.infradead.org
> Subject: Re: [PATCH v2 08/10] PCI: layerscape: Add EP mode support for
> ls1088a and ls2088a
>
>
>
> Le 24/08/2019 à 02:18, Xiaowei Bao a écrit :
> >
> >
> >> -Original Message-
> >> From: Andrew Murray
> >> Sent: 2019年8月23日 22:28
> >> To: Xiaowei Bao
> >> Cc: bhelg...@google.com; robh...@kernel.org; mark.rutl...@arm.com;
> >> shawn...@kernel.org; Leo Li ; kis...@ti.com;
> >> lorenzo.pieral...@arm.co; a...@arndb.de; gre...@linuxfoundation.org;
> M.h.
> >> Lian ; Mingkai Hu ; Roy
> >> Zang ; jingooh...@gmail.com;
> >> gustavo.pimen...@synopsys.com; linux-...@vger.kernel.org;
> >> devicet...@vger.kernel.org; linux-ker...@vger.kernel.org;
> >> linux-arm-ker...@lists.infradead.org; linuxppc-dev@lists.ozlabs.org
> >> Subject: Re: [PATCH v2 08/10] PCI: layerscape: Add EP mode support
> >> for ls1088a and ls2088a
> >>
> >> On Thu, Aug 22, 2019 at 07:22:40PM +0800, Xiaowei Bao wrote:
> >>> Add PCIe EP mode support for ls1088a and ls2088a, there are some
> >>> difference between LS1 and LS2 platform, so refactor the code of the
> >>> EP driver.
> >>>
> >>> Signed-off-by: Xiaowei Bao
> >>> ---
> >>> v2:
> >>> - New mechanism for layerscape EP driver.
> >>
> >> Was there a v1 of this patch?
> >
> > Yes, but I don't know how to comments, ^_^
>
> As far as I can see, in the previous version of the series
> (https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatch
> work.ozlabs.org%2Fproject%2Flinuxppc-dev%2Flist%2F%3Fseries%3D125315
> %26state%3D*data=02%7C01%7Cxiaowei.bao%40nxp.com%7C1befe9
> a67c8046f9535e08d7285eaab6%7C686ea1d3bc2b4c6fa92cd99c5c301635%
> 7C0%7C0%7C637022259387139020sdata=p4wbycd04Z7qRUfAoZtwc
> UP7pR%2FuA3%2FjVcWMz6YyQVQ%3Dreserved=0),
> the 8/10 was something completely different, and I can't find any other patch
> in the series that could have been the v1 of this patch.
Thanks, I will correct it to v1 in next version patch.
>
> Christophe
>
> >
> >>
> >>>
> >>> drivers/pci/controller/dwc/pci-layerscape-ep.c | 76
> >>> --
> >>> 1 file changed, 58 insertions(+), 18 deletions(-)
> >>>
> >>> diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> >>> b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> >>> index 7ca5fe8..2a66f07 100644
> >>> --- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> >>> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> >>> @@ -20,27 +20,29 @@
> >>>
> >>> #define PCIE_DBI2_OFFSET0x1000 /* DBI2 base address*/
> >>>
> >>> -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)
> >>> +
> >>> +struct ls_pcie_ep_drvdata {
> >>> + u32 func_offset;
> >>> + const struct dw_pcie_ep_ops *ops;
> >>> + const struct dw_pcie_ops*dw_pcie_ops;
> >>> };
> >>>
> >>> -#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
> >>> +struct ls_pcie_ep {
> >>> + struct dw_pcie *pci;
> >>> + struct pci_epc_features *ls_epc;
> >>> + const struct ls_pcie_ep_drvdata *drvdata; };
> >>>
> >>> static int ls_pcie_establish_link(struct dw_pcie *pci) {
> >>> return 0;
> >>> }
> >>>
> >>> -static const struct dw_pcie_ops ls_pcie_ep_ops = {
> >>> +static const struct dw_pcie_ops dw_ls_pcie_ep_ops = {
> >>> .start_link = ls_pcie_establish_link, };
> >>>
> >>> -static const struct of_device_id ls_pcie_ep_of_match[] = {
> >>> - { .compatible = "fsl,ls-pcie-ep",},
> >>> - { },
> >>> -};
> >>> -
> >>> static const struct pci_epc_features*
> >>> ls_pcie_ep_get_features(struct dw_pcie_ep *ep) { @@ -82,10 +84,44
> >>> @@ static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> >>> }
> >>> }
> >>>
> >>> -static const struct dw_pcie_ep_ops pcie_ep_ops = {
> >>> +static unsigned int ls_pcie_ep_func_conf_select(struct dw_pcie_ep *ep,
> >>> + u8 func_no)
> >>> +{
> >>> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> >>> + struct ls_pcie_ep *pcie = to_ls_pcie_ep(pci);
> >>> + u8 header_type;
> >>> +
> >>> + header_type = ioread8(pci->dbi_base + PCI_HEADER_TYPE);
> >>> +
> >>> + if (header_type & (1 << 7))
> >>> + return pcie->drvdata->func_offset * func_no;
> >>> + else
> >>> + return 0;
> >>
> >> It looks like there isn't a PCI define for multi function, the
> >> nearest I could find was
Re: [PATCH v2] powerpc/powernv: Add ultravisor message log interface
On 8/23/19 9:48 AM, Michael Ellerman wrote: > Hi Claudio, Hi Michael, > > Claudio Carvalho writes: >> Ultravisor (UV) provides an in-memory console which follows the OPAL >> in-memory console structure. >> >> This patch extends the OPAL msglog code to also initialize the UV memory >> console and provide a sysfs interface (uv_msglog) for userspace to view >> the UV message log. >> >> CC: Madhavan Srinivasan >> CC: Oliver O'Halloran >> Signed-off-by: Claudio Carvalho >> --- >> This patch depends on the "kvmppc: Paravirtualize KVM to support >> ultravisor" patchset submitted by Claudio Carvalho. >> --- >> arch/powerpc/platforms/powernv/opal-msglog.c | 99 ++-- >> 1 file changed, 72 insertions(+), 27 deletions(-) > I think the code changes look mostly OK here. > > But I'm not sure about the end result in sysfs. > > If I'm reading it right this will create: > > /sys/firmware/opal/uv_msglog > > Which I think is a little weird, because the UV is not OPAL. > > So I guess I wonder if the file should be created elsewhere to avoid any > confusion and keep things nicely separated. > > Possibly /sys/firmware/ultravisor/msglog ? Yes, makes sense. I will do that. Currently, the ultravisor memory console DT property is in /ibm,opal/ibm,opal-uv-memcons. I think we should move it to /ibm,ultravisor/ibm,uv-firmware/ibm,uv-memcons. What do you think? Thanks, Claudio > > cheers
Re: cleanup the dma_pgprot handling
On Fri, Aug 23, 2019 at 09:58:04PM +, Paul Burton wrote: > So I believe uncached & uncached accelerated are another case like that > described above - they're 2 different CCAs but the same "access type", > namely uncached. > > Section 4.9 then goes on to forbid mixing access types, but not CCAs. > > It would be nice if the precise mapping from CCA to access type was > provided, but I don't see that anywhere. I can check with the > architecture team to be sure, but to my knowledge we're fine to mix > access via kseg1 (ie. uncached) & mappings with CCA=7 (uncached > accelerated). Ok. Looks like we can keep it then and I'll add a comment to the code with the above reference.
[powerpc]WARN : arch/powerpc/platforms/powernv/smp.c:160
linux-next is currently broken on POWER8 non virtualized. Kernel fails to reach login prompt with following kernel warning repeatedly shown during boot. The problem dates back atleast till next-20190816. [ 40.285606] WARNING: CPU: 1 PID: 0 at arch/powerpc/platforms/powernv/smp.c:160 pnv_smp_cpu_kill_self+0x50/0x2d0 [ 40.285609] Modules linked in: kvm_hv kvm sunrpc dm_mirror dm_region_hash dm_log dm_mod ses enclosure scsi_transport_sas sg ipmi_powernv ipmi_devintf powernv_rng uio_pdrv_genirq uio leds_powernv ipmi_msghandler powernv_op_panel ibmpowernv ip_tables ext4 mbcache jbd2 sd_mod ipr tg3 libata ptp pps_core [ 40.285643] CPU: 1 PID: 0 Comm: swapper/1 Not tainted 5.3.0-rc5-next-20190823-autotest-autotest #1 [ 40.285644] NIP: c00b5f40 LR: c0055498 CTR: c00b5ef0 [ 40.285646] REGS: c007f5527980 TRAP: 0700 Not tainted (5.3.0-rc5-next-20190823-autotest-autotest) [ 40.285646] MSR: 90029033 CR: 24004028 XER: [ 40.285650] CFAR: c0055494 IRQMASK: 1 [ 40.285650] GPR00: c0055498 c007f5527c10 c148b200 [ 40.285650] GPR04: c007fa897d80 c007fa90c800 0007f998 [ 40.285650] GPR08: 0001 c007fa90c800 [ 40.285650] GPR12: c00b5ef0 c007ee00 0800 c00c11d0 [ 40.285650] GPR16: 0001 c1035280 c15303c0 [ 40.285650] GPR20: c0052d60 0001 c007f54cd800 c007f54cd880 [ 40.285650] GPR24: 0008 c007f54cd800 c14bdf78 c14c20d8 [ 40.285650] GPR28: 0002 c14c2538 0001 c007f54cd800 [ 40.285662] NIP [c00b5f40] pnv_smp_cpu_kill_self+0x50/0x2d0 [ 40.285664] LR [c0055498] cpu_die+0x48/0x64 [ 40.285665] Call Trace: [ 40.285667] [c007f5527c10] [c0f85f10] ppc64_tlb_batch+0x0/0x1220 (unreliable) [ 40.285669] [c007f5527df0] [c0055498] cpu_die+0x48/0x64 [ 40.285672] [c007f5527e10] [c00226a0] arch_cpu_idle_dead+0x20/0x40 [ 40.285674] [c007f5527e30] [c016bd2c] do_idle+0x37c/0x3f0 [ 40.285676] [c007f5527ed0] [c016bfac] cpu_startup_entry+0x3c/0x50 [ 40.285678] [c007f5527f00] [c0055198] start_secondary+0x638/0x680 [ 40.285680] [c007f5527f90] [c000ac5c] start_secondary_prolog+0x10/0x14 [ 40.285680] Instruction dump: [ 40.285681] fb61ffd8 fb81ffe0 fba1ffe8 fbc1fff0 fbe1fff8 f8010010 f821fe21 e90d1178 [ 40.285684] f9010198 3900 892d0988 792907e0 <0b09> 3922 7d210164 3923 [ 40.285687] ---[ end trace 72c90a064122d9e4 ]— Relevant code snippet : 156 /* 157 * This hard disables local interurpts, ensuring we have no lazy 158 * irqs pending. 159 */ 160 WARN_ON(irqs_disabled()); <<=== 161 hard_irq_disable(); 162 WARN_ON(lazy_irq_pending()); Thanks -Sachin
Re: [PATCH] powerpc/64: Fix stacktrace on BE when function_graph is enabled
Michael Ellerman wrote:
Currently if we oops or warn while function_graph is active the stack
trace looks like:
.trace_graph_return+0xac/0x100
.ftrace_return_to_handler+0x98/0x140
.return_to_handler+0x20/0x40
.return_to_handler+0x0/0x40
.return_to_handler+0x0/0x40
.return_to_handler+0x0/0x40
.return_to_handler+0x0/0x40
.return_to_handler+0x0/0x40
.return_to_handler+0x0/0x40
.cpu_startup_entry+0x34/0x40
.start_secondary+0x680/0x6f0
start_secondary_prolog+0x10/0x14
Notice the multiple entries that just show .return_to_handler.
There is logic in show_stack() to detect this case and print the
traced function, but we inadvertently broke it in commit
7d56c65a6ff9 ("powerpc/ftrace: Remove mod_return_to_handler") (2014),
because that commit accidentally removed the dereference of rth which
gets the text address from the function descriptor. Hence this is only
broken on big endian (or technically ELFv1).
Fix it by using the proper accessor, which is ppc_function_entry().
Result is we get a stack trace such as:
.trace_graph_return+0x134/0x160
.ftrace_return_to_handler+0x94/0x140
.return_to_handler+0x20/0x40
.return_to_handler+0x0/0x40 (.shared_cede_loop+0x48/0x130)
.return_to_handler+0x0/0x40 (.cpuidle_enter_state+0xa0/0x690)
.return_to_handler+0x0/0x40 (.cpuidle_enter+0x44/0x70)
.return_to_handler+0x0/0x40 (.call_cpuidle+0x68/0xc0)
.return_to_handler+0x0/0x40 (.do_idle+0x37c/0x400)
.return_to_handler+0x0/0x40 (.cpu_startup_entry+0x30/0x50)
.rest_init+0x224/0x348
Fixes: 7d56c65a6ff9 ("powerpc/ftrace: Remove mod_return_to_handler")
Signed-off-by: Michael Ellerman
---
arch/powerpc/kernel/process.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 8fc4de0d22b4..1601d7cfe45e 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -2048,7 +2048,7 @@ void show_stack(struct task_struct *tsk, unsigned long
*stack)
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
struct ftrace_ret_stack *ret_stack;
extern void return_to_handler(void);
- unsigned long rth = (unsigned long)return_to_handler;
+ unsigned long rth = ppc_function_entry(return_to_handler);
Thanks! This looks good to me. A small suggestion though -- can we use
dereference_kernel_function_descriptor() instead? It will be a nop for
ABIv2, which would be nice, but not really a major deal.
In either case:
Reviewed-by: Naveen N. Rao
- Naveen
Re: [PATCH v2 08/10] PCI: layerscape: Add EP mode support for ls1088a and ls2088a
Le 24/08/2019 à 02:18, Xiaowei Bao a écrit :
-Original Message-
From: Andrew Murray
Sent: 2019年8月23日 22:28
To: Xiaowei Bao
Cc: bhelg...@google.com; robh...@kernel.org; mark.rutl...@arm.com;
shawn...@kernel.org; Leo Li ; kis...@ti.com;
lorenzo.pieral...@arm.co; a...@arndb.de; gre...@linuxfoundation.org; M.h.
Lian ; Mingkai Hu ; Roy
Zang ; jingooh...@gmail.com;
gustavo.pimen...@synopsys.com; linux-...@vger.kernel.org;
devicet...@vger.kernel.org; linux-ker...@vger.kernel.org;
linux-arm-ker...@lists.infradead.org; linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v2 08/10] PCI: layerscape: Add EP mode support for
ls1088a and ls2088a
On Thu, Aug 22, 2019 at 07:22:40PM +0800, Xiaowei Bao wrote:
Add PCIe EP mode support for ls1088a and ls2088a, there are some
difference between LS1 and LS2 platform, so refactor the code of the
EP driver.
Signed-off-by: Xiaowei Bao
---
v2:
- New mechanism for layerscape EP driver.
Was there a v1 of this patch?
Yes, but I don't know how to comments, ^_^
As far as I can see, in the previous version of the series
(https://patchwork.ozlabs.org/project/linuxppc-dev/list/?series=125315=*),
the 8/10 was something completely different, and I can't find any other
patch in the series that could have been the v1 of this patch.
Christophe
drivers/pci/controller/dwc/pci-layerscape-ep.c | 76
--
1 file changed, 58 insertions(+), 18 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-layerscape-ep.c
b/drivers/pci/controller/dwc/pci-layerscape-ep.c
index 7ca5fe8..2a66f07 100644
--- a/drivers/pci/controller/dwc/pci-layerscape-ep.c
+++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
@@ -20,27 +20,29 @@
#define PCIE_DBI2_OFFSET 0x1000 /* DBI2 base address*/
-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)
+
+struct ls_pcie_ep_drvdata {
+ u32 func_offset;
+ const struct dw_pcie_ep_ops *ops;
+ const struct dw_pcie_ops*dw_pcie_ops;
};
-#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
+struct ls_pcie_ep {
+ struct dw_pcie *pci;
+ struct pci_epc_features *ls_epc;
+ const struct ls_pcie_ep_drvdata *drvdata; };
static int ls_pcie_establish_link(struct dw_pcie *pci) {
return 0;
}
-static const struct dw_pcie_ops ls_pcie_ep_ops = {
+static const struct dw_pcie_ops dw_ls_pcie_ep_ops = {
.start_link = ls_pcie_establish_link, };
-static const struct of_device_id ls_pcie_ep_of_match[] = {
- { .compatible = "fsl,ls-pcie-ep",},
- { },
-};
-
static const struct pci_epc_features* ls_pcie_ep_get_features(struct
dw_pcie_ep *ep) { @@ -82,10 +84,44 @@ static int
ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
}
}
-static const struct dw_pcie_ep_ops pcie_ep_ops = {
+static unsigned int ls_pcie_ep_func_conf_select(struct dw_pcie_ep *ep,
+ u8 func_no)
+{
+ struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+ struct ls_pcie_ep *pcie = to_ls_pcie_ep(pci);
+ u8 header_type;
+
+ header_type = ioread8(pci->dbi_base + PCI_HEADER_TYPE);
+
+ if (header_type & (1 << 7))
+ return pcie->drvdata->func_offset * func_no;
+ else
+ return 0;
It looks like there isn't a PCI define for multi function, the nearest I could
find
was PCI_HEADER_TYPE_MULTIDEVICE in hotplug/ibmphp.h. A comment
above the test might be helpful to explain the test.
Yes, I have not find the PCI_HEADER_TYPE_MULTIDEVICE define. OK, I will add
The comments in next version patch.
As the ls_pcie_ep_drvdata structures are static, the unset .func_offset will be
initialised to 0, so you could just drop the test above.
OK, thanks
However something to the effect of the following may help spot
misconfiguration:
WARN_ON(func_no && !pcie->drvdata->func_offset); return
pcie->drvdata->func_offset * func_no;
Thanks a lot, this looks better.
The WARN is probably quite useful as if you are attempting to use non-zero
functions and func_offset isn't set - then things may appear to work normally
but actually will break horribly.
got it, thanks.
Thanks,
Andrew Murray
+}
+
+static const struct dw_pcie_ep_ops ls_pcie_ep_ops = {
.ep_init = ls_pcie_ep_init,
.raise_irq = ls_pcie_ep_raise_irq,
.get_features = ls_pcie_ep_get_features,
+ .func_conf_select = ls_pcie_ep_func_conf_select, };
+
+static const struct ls_pcie_ep_drvdata ls1_ep_drvdata = {
+ .ops = _pcie_ep_ops,
+ .dw_pcie_ops = _ls_pcie_ep_ops,
+};
+
+static const struct ls_pcie_ep_drvdata ls2_ep_drvdata = {
+ .func_offset = 0x2,
+ .ops = _pcie_ep_ops,
+ .dw_pcie_ops = _ls_pcie_ep_ops,
+};
+
+static const struct of_device_id ls_pcie_ep_of_match[] = {
+ { .compatible =
