[RFC PATCH] soc: fsl: guts: handle devm_kstrdup() failure
devm_kstrdup() may return NULL if internal allocation failed.
soc_dev_attr.machine should be checked (although its only use
in pr_info() would be safe even with a NULL). Therefor
in the unlikely case of allocation failure, fsl_guts_probe() returns
-ENOMEM as this allocating failing is an indication of something
more serious going wrong at system level.
As machine is from the device tree which I assume to be RO - if
that assumption is always correct - a better alternative would be
to use devm_kstrdup_const() here. That would then simply copy the
reference to the RO data and not perform any allocation at all.
Signed-off-by: Nicholas Mc Guire
Fixes: a6fc3b698130 ("soc: fsl: add GUTS driver for QorIQ platforms")
---
Problem located by experimental coccinelle script
Patch was compile tested with: multi_v7_defconfig (implies FSL_GUTS=y)
Patch is against 4.20-rc4 (localversion-next is next-20181130)
drivers/soc/fsl/guts.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c
index 302e0c8..a0c751b 100644
--- a/drivers/soc/fsl/guts.c
+++ b/drivers/soc/fsl/guts.c
@@ -156,8 +156,11 @@ static int fsl_guts_probe(struct platform_device *pdev)
if (of_property_read_string(root, "model", &machine))
of_property_read_string_index(root, "compatible", 0, &machine);
of_node_put(root);
- if (machine)
+ if (machine) {
soc_dev_attr.machine = devm_kstrdup(dev, machine, GFP_KERNEL);
+ if (!soc_dev_attr.machine)
+ return -ENOMEM;
+ }
svr = fsl_guts_get_svr();
soc_die = fsl_soc_die_match(svr, fsl_soc_die);
--
2.1.4
Re: [1/1] Fix NULL pointer access in PowerPC MSI teardown code
On Wed, 2018-11-28 at 03:20:48 UTC, Radu Rendec wrote: > The arch_teardown_msi_irqs() function assumes that controller ops > pointers were already checked in arch_setup_msi_irqs(), but this > assumption is wrong: arch_teardown_msi_irqs() can be called even when > arch_setup_msi_irqs() returns an error (-ENOSYS). > > This can happen in the following scenario: > > * msi_capability_init() calls pci_msi_setup_msi_irqs() > * pci_msi_setup_msi_irqs() returns -ENOSYS > * msi_capability_init() notices the error and calls free_msi_irqs() > * free_msi_irqs() calls pci_msi_teardown_msi_irqs() > > This is easier to see when CONFIG_PCI_MSI_IRQ_DOMAIN is not set and > pci_msi_setup_msi_irqs() and pci_msi_teardown_msi_irqs() are just > aliases to arch_setup_msi_irqs() and arch_teardown_msi_irqs(). > > The call to free_msi_irqs() upon pci_msi_setup_msi_irqs() failure seems > legit, as it does additional cleanup; e.g. list_del(&entry->list) and > kfree(entry) inside free_msi_irqs() do happen (MSI descriptors are > allocated before pci_msi_setup_msi_irqs() is called and need to be > cleaned up if that fails). > > Signed-off-by: Radu Rendec Applied to powerpc fixes, thanks. https://git.kernel.org/powerpc/c/78e7b15e17ac175e7eed9e21c6f92d cheers
Re: powerpc: Look for "stdout-path" when setting up legacy consoles
On Fri, 2018-11-30 at 03:54:09 UTC, Benjamin Herrenschmidt wrote: > Commit 78e5dfea8 "powerpc: dts: replace 'linux,stdout-path' with > 'stdout-path'" > broke the default console on a number of embedded PowerPC systems, because it > failed to also update the code in arch/powerpc/kernel/legacy_serial.c to > look for that property in addition to the old one. > > This fixes it. > > Signed-off-by: Benjamin Herrenschmidt > Fixes: 78e5dfea8 powerpc: dts: replace 'linux,stdout-path' with 'stdout-path' > Reviewed-by: Rob Herring Applied to powerpc fixes, thanks. https://git.kernel.org/powerpc/c/bf3d6afbb234156749b640b6c50f71 cheers
[PATCH] KVM: PPC: Book3S HV: NULL check before some freeing functions is not needed.
NULL check before some freeing functions is not needed. Signed-off-by: Thomas Meyer --- diff -u -p a/arch/powerpc/kvm/book3s_hv_nested.c b/arch/powerpc/kvm/book3s_hv_nested.c --- a/arch/powerpc/kvm/book3s_hv_nested.c +++ b/arch/powerpc/kvm/book3s_hv_nested.c @@ -1252,8 +1252,7 @@ static long int __kvmhv_nested_page_faul rmapp = &memslot->arch.rmap[gfn - memslot->base_gfn]; ret = kvmppc_create_pte(kvm, gp->shadow_pgtable, pte, n_gpa, level, mmu_seq, gp->shadow_lpid, rmapp, &n_rmap); - if (n_rmap) - kfree(n_rmap); + kfree(n_rmap); if (ret == -EAGAIN) ret = RESUME_GUEST; /* Let the guest try again */
Re: [PATCH] KVM: PPC: Book3S HV: NULL check before some freeing functions is not needed.
On Sun, 2018-12-02 at 21:52 +0100, Thomas Meyer wrote: > NULL check before some freeing functions is not needed. Technically true, however I think a comment should be added then to make it clearer to someone reading the code why this is ok. See below. Suraj. > > Signed-off-by: Thomas Meyer > --- > > diff -u -p a/arch/powerpc/kvm/book3s_hv_nested.c > b/arch/powerpc/kvm/book3s_hv_nested.c > --- a/arch/powerpc/kvm/book3s_hv_nested.c > +++ b/arch/powerpc/kvm/book3s_hv_nested.c > @@ -1252,8 +1252,7 @@ static long int __kvmhv_nested_page_faul > rmapp = &memslot->arch.rmap[gfn - memslot->base_gfn]; > ret = kvmppc_create_pte(kvm, gp->shadow_pgtable, pte, n_gpa, > level, > mmu_seq, gp->shadow_lpid, rmapp, > &n_rmap); > - if (n_rmap) > - kfree(n_rmap); > + kfree(n_rmap); e.g. /* n_rmap set to NULL in kvmppc_create_pte if reference preserved */ > if (ret == -EAGAIN) > ret = RESUME_GUEST; /* Let the guest try > again */ >
[PATCH v4] powerpc/ptrace: replace ptrace_report_syscall() with a tracehook call
From: Elvira Khabirova
Arch code should use tracehook_*() helpers, as documented
in include/linux/tracehook.h,
ptrace_report_syscall() is not expected to be used outside that file.
Co-authored-by: Dmitry V. Levin
Fixes: 5521eb4bca2d ("powerpc/ptrace: Add support for PTRACE_SYSEMU")
Signed-off-by: Elvira Khabirova
Signed-off-by: Dmitry V. Levin
---
v4: rewritten to call tracehook_report_syscall_entry() once, compile-tested
v3: add a descriptive comment
v2: explicitly ignore tracehook_report_syscall_entry() return code
arch/powerpc/kernel/ptrace.c | 54 +++-
1 file changed, 35 insertions(+), 19 deletions(-)
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index afb819f4ca68..59c8c9a3d7ea 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -3263,27 +3263,43 @@ static inline int do_seccomp(struct pt_regs *regs) {
return 0; }
*/
long do_syscall_trace_enter(struct pt_regs *regs)
{
+ struct thread_info *ti;
+ u32 cached_flags;
+
user_exit();
- if (test_thread_flag(TIF_SYSCALL_EMU)) {
- ptrace_report_syscall(regs);
- /*
-* Returning -1 will skip the syscall execution. We want to
-* avoid clobbering any register also, thus, not 'gotoing'
-* skip label.
-*/
- return -1;
- }
+ ti = current_thread_info();
+ cached_flags =
+ READ_ONCE(ti->flags) &
+ (TIF_SYSCALL_EMU | TIF_SYSCALL_TRACE | TIF_SYSCALL_TRACEPOINT);
- /*
-* The tracer may decide to abort the syscall, if so tracehook
-* will return !0. Note that the tracer may also just change
-* regs->gpr[0] to an invalid syscall number, that is handled
-* below on the exit path.
-*/
- if (test_thread_flag(TIF_SYSCALL_TRACE) &&
- tracehook_report_syscall_entry(regs))
- goto skip;
+ if (cached_flags & (TIF_SYSCALL_EMU | TIF_SYSCALL_TRACE)) {
+ int rc = tracehook_report_syscall_entry(regs);
+
+ if (unlikely(cached_flags & _TIF_SYSCALL_EMU)) {
+ /*
+* A nonzero return code from
+* tracehook_report_syscall_entry() tells us
+* to prevent the syscall execution, but
+* we are not going to execute it anyway.
+*
+* Returning -1 will skip the syscall execution.
+* We want to avoid clobbering any register also,
+* thus, not 'gotoing' skip label.
+*/
+ return -1;
+ }
+
+ if (rc) {
+ /*
+* The tracer decided to abort the syscall.
+* Note that the tracer may also just change
+* regs->gpr[0] to an invalid syscall number,
+* that is handled below on the exit path.
+*/
+ goto skip;
+ }
+ }
/* Run seccomp after ptrace; allow it to set gpr[3]. */
if (do_seccomp(regs))
@@ -3293,7 +3309,7 @@ long do_syscall_trace_enter(struct pt_regs *regs)
if (regs->gpr[0] >= NR_syscalls)
goto skip;
- if (unlikely(test_thread_flag(TIF_SYSCALL_TRACEPOINT)))
+ if (unlikely(cached_flags & TIF_SYSCALL_TRACEPOINT))
trace_sys_enter(regs, regs->gpr[0]);
#ifdef CONFIG_PPC64
--
ldv
Re: pkeys: Reserve PKEY_DISABLE_READ
On Thu, Nov 29, 2018 at 12:37:15PM +0100, Florian Weimer wrote:
> * Dave Hansen:
>
> > On 11/27/18 3:57 AM, Florian Weimer wrote:
> >> I would have expected something that translates PKEY_DISABLE_WRITE |
> >> PKEY_DISABLE_READ into PKEY_DISABLE_ACCESS, and also accepts
> >> PKEY_DISABLE_ACCESS | PKEY_DISABLE_READ, for consistency with POWER.
> >>
> >> (My understanding is that PKEY_DISABLE_ACCESS does not disable all
> >> access, but produces execute-only memory.)
> >
> > Correct, it disables all data access, but not execution.
>
> So I would expect something like this (completely untested, I did not
> even compile this):
Ok. I re-read through the entire email thread to understand the problem and
the proposed solution. Let me summarize it below. Lets see if we are on the same
plate.
So the problem is as follows:
Currently the kernel supports 'disable-write' and 'disable-access'.
On x86, cpu supports 'disable-write' and 'disable-access'. This
matches with what the kernel supports. All good.
However on power, cpu supports 'disable-read' too. Since userspace can
program the cpu directly, userspace has the ability to set
'disable-read' too. This can lead to inconsistency between the kernel
and the userspace.
We want the kernel to match userspace on all architectures.
Proposed Solution:
Enhance the kernel to understand 'disable-read', and facilitate architectures
that understand 'disable-read' to allow it.
Also explicitly define the semantics of disable-access as
'disable-read and disable-write'
Did I get this right? Assuming I did, the implementation has to do
the following --
On power, sys_pkey_alloc() should succeed if the init_val
is PKEY_DISABLE_READ, PKEY_DISABLE_WRITE, PKEY_DISABLE_ACCESS
or any combination of the three.
On x86, sys_pkey_alloc() should succeed if the init_val is
PKEY_DISABLE_WRITE or PKEY_DISABLE_ACCESS or PKEY_DISABLE_READ
or any combination of the three, except PKEY_DISABLE_READ
specified all by itself.
On all other arches, none of the flags are supported.
Are we on the same plate?
RP
>
> diff --git a/arch/powerpc/include/asm/pkeys.h
> b/arch/powerpc/include/asm/pkeys.h
> index 20ebf153c871..bed23f9e8336 100644
> --- a/arch/powerpc/include/asm/pkeys.h
> +++ b/arch/powerpc/include/asm/pkeys.h
> @@ -199,6 +199,11 @@ static inline bool arch_pkeys_enabled(void)
> return !static_branch_likely(&pkey_disabled);
> }
>
> +static inline bool arch_pkey_access_rights_valid(unsigned long rights)
> +{
> + return (rights & ~(unsigned long)PKEY_ACCESS_MASK) == 0;
> +}
> +
> extern void pkey_mm_init(struct mm_struct *mm);
> extern bool arch_supports_pkeys(int cap);
> extern unsigned int arch_usable_pkeys(void);
> diff --git a/arch/x86/include/asm/pkeys.h b/arch/x86/include/asm/pkeys.h
> index 19b137f1b3be..e3e1d5a316e8 100644
> --- a/arch/x86/include/asm/pkeys.h
> +++ b/arch/x86/include/asm/pkeys.h
> @@ -14,6 +14,17 @@ static inline bool arch_pkeys_enabled(void)
> return boot_cpu_has(X86_FEATURE_OSPKE);
> }
>
> +static inline bool arch_pkey_access_rights_valid(unsigned long rights)
> +{
> + if (rights & ~(unsigned long)PKEY_ACCESS_MASK)
> + return false;
> + if (rights & PKEY_DISABLE_READ) {
> + /* x86 can only disable read access along with write access. */
> + return rights & (PKEY_DISABLE_WRITE | PKEY_DISABLE_ACCESS);
> + }
> + return true;
> +}
> +
> /*
> * Try to dedicate one of the protection keys to be used as an
> * execute-only protection key.
> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index 87a57b7642d3..b9b78145017f 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -928,7 +928,13 @@ int arch_set_user_pkey_access(struct task_struct *tsk,
> int pkey,
> return -EINVAL;
>
> /* Set the bits we need in PKRU: */
> - if (init_val & PKEY_DISABLE_ACCESS)
> + if (init_val & (PKEY_DISABLE_ACCESS | PKEY_DISABLE_READ))
> + /*
> + * arch_pkey_access_rights_valid checked that
> + * PKEY_DISABLE_READ is actually representable on x86
> + * (that is, it comes with PKEY_DISABLE_ACCESS or
> + * PKEY_DISABLE_WRITE).
> + */
> new_pkru_bits |= PKRU_AD_BIT;
> if (init_val & PKEY_DISABLE_WRITE)
> new_pkru_bits |= PKRU_WD_BIT;
> diff --git a/include/linux/pkeys.h b/include/linux/pkeys.h
> index 2955ba976048..2c330fabbe55 100644
> --- a/include/linux/pkeys.h
> +++ b/include/linux/pkeys.h
> @@ -48,6 +48,11 @@ static inline void copy_init_pkru_to_fpregs(void)
> {
> }
>
> +static inline bool arch_pkey_access_rights_valid(unsigned long rights)
> +{
> + return false;
> +}
> +
> #endif /* ! CONFIG_ARCH_HAS_PKEYS */
>
> #endif /* _LINUX_PKEYS_H */
> diff --git a/mm/mprotect.c b/mm/mprotect.c
> index 6d331620b9e5..f4cefc3540df 100644
>
RE: [PATCHv2 5/6] pci: layerscape: Add the EP mode support.
Hi Lorenzo,
-Original Message-
From: Lorenzo Pieralisi
Sent: 2018年12月1日 0:22
To: Xiaowei Bao
Cc: bhelg...@google.com; robh...@kernel.org; mark.rutl...@arm.com;
shawn...@kernel.org; Leo Li ; kis...@ti.com; a...@arndb.de;
gre...@linuxfoundation.org; M.h. Lian ; Mingkai Hu
; Roy Zang ;
kstew...@linuxfoundation.org; cyrille.pitc...@free-electrons.com;
pombreda...@nexb.com; shawn@rock-chips.com; niklas.cas...@axis.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: [PATCHv2 5/6] pci: layerscape: Add the EP mode support.
On Mon, Nov 05, 2018 at 04:46:52PM +0800, Xiaowei Bao wrote:
> Add the PCIe EP mode support for layerscape platform.
>
> Signed-off-by: Xiaowei Bao
> ---
> v2:
> - remove the EP mode check function.
>
> drivers/pci/controller/dwc/Makefile|2 +-
> drivers/pci/controller/dwc/pci-layerscape-ep.c | 147
>
> 2 files changed, 148 insertions(+), 1 deletions(-) create mode
> 100644 drivers/pci/controller/dwc/pci-layerscape-ep.c
>
> diff --git a/drivers/pci/controller/dwc/Makefile
> b/drivers/pci/controller/dwc/Makefile
> index 5d2ce72..b26d617 100644
> --- a/drivers/pci/controller/dwc/Makefile
> +++ b/drivers/pci/controller/dwc/Makefile
> @@ -8,7 +8,7 @@ obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
> obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
> obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
> obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone-dw.o pci-keystone.o
> -obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
> +obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o pci-layerscape-ep.o
> obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
> obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
> obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o diff --git
> a/drivers/pci/controller/dwc/pci-layerscape-ep.c
> b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> new file mode 100644
> index 000..289618b
> --- /dev/null
> +++ b/drivers/pci/controller/dwc/pci-layerscape-ep.c
> @@ -0,0 +1,147 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PCIe controller EP driver for Freescale Layerscape SoCs
> + *
> + * Copyright (C) 2018 NXP Semiconductor.
> + *
> + * Author: Xiaowei Bao */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include "pcie-designware.h"
> +
> +#define PCIE_DBI2_OFFSET 0x1000 /* DBI2 base address*/
> +
> +struct ls_pcie_ep {
> + struct dw_pcie *pci;
> +};
I am not really sure why you need an additional struct.
[Xiaowei Bao] thanks a lot for your comments, I defined this structure in order
to add NXP's new chip PCIe EP driver in the future, because other platforms may
have some errata to be solved. The structure of defining an LX platform will be
more flexible. I can use the driver of the DW platform directly. Just need to
modify the DTS, but for the future development of all NXP platform EP drivers,
thus adding a new file.
> +#define to_ls_pcie_ep(x) dev_get_drvdata((x)->dev)
Unused.
> +
> +static int ls_pcie_establish_link(struct dw_pcie *pci) {
> + return 0;
> +}
> +
> +static const struct dw_pcie_ops 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 void ls_pcie_ep_init(struct dw_pcie_ep *ep) {
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> + struct pci_epc *epc = ep->epc;
> + enum pci_barno bar;
> +
> + for (bar = BAR_0; bar <= BAR_5; bar++)
> + dw_pcie_ep_reset_bar(pci, bar);
> +
> + epc->features |= EPC_FEATURE_NO_LINKUP_NOTIFIER; }
> +
> +static int ls_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
> + enum pci_epc_irq_type type, u16
> interrupt_num) {
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> +
> + switch (type) {
> + case PCI_EPC_IRQ_LEGACY:
> + return dw_pcie_ep_raise_legacy_irq(ep, func_no);
> + case PCI_EPC_IRQ_MSI:
> + return dw_pcie_ep_raise_msi_irq(ep, func_no, interrupt_num);
> + case PCI_EPC_IRQ_MSIX:
> + return dw_pcie_ep_raise_msix_irq(ep, func_no, interrupt_num);
> + default:
> + dev_err(pci->dev, "UNKNOWN IRQ type\n");
> + }
> +
> + return 0;
So if it falls through to the default, we log an error but return 0 ? This does
not make much sense.
I know you probably copy/pasted code from DWC platform, that code must be fixed
too I suppose.
Lorenzo
[Xiaowei Bao] Thanks a lot for your comments, I will send the v3 patch fix it.
> +}
> +
> +static struct dw_pcie_ep_ops pcie_ep_ops = {
> + .ep_init = ls_pcie_ep_init,
> + .raise_irq = ls_pcie_ep_raise_irq,
> +};
> +
> +static int __init ls_add_pcie_ep(struct ls_pcie_ep *pcie,
> + struct pl
RE: [PATCHv2 3/6] PCI: layerscape: Add the EP mode support
Hi Lorenzo, -Original Message- From: Lorenzo Pieralisi Sent: 2018年11月30日 18:55 To: Xiaowei Bao Cc: bhelg...@google.com; robh...@kernel.org; mark.rutl...@arm.com; shawn...@kernel.org; Leo Li ; kis...@ti.com; a...@arndb.de; gre...@linuxfoundation.org; M.h. Lian ; Mingkai Hu ; Roy Zang ; kstew...@linuxfoundation.org; cyrille.pitc...@free-electrons.com; pombreda...@nexb.com; shawn@rock-chips.com; niklas.cas...@axis.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: [PATCHv2 3/6] PCI: layerscape: Add the EP mode support On Mon, Nov 05, 2018 at 04:46:50PM +0800, Xiaowei Bao wrote: > Add the EP mode support. > > Signed-off-by: Xiaowei Bao > --- > v2: > - Add the SoC specific compatibles. > > .../devicetree/bindings/pci/layerscape-pci.txt |3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) Wrong commit $SUBJECT, this is not PCI code, it is a DT binding update, I will have a look at the rest of the series to see if I can update this patch or you will do it with the next respin. Lorenzo [Xiaowei Bao] HI Lorenzo, thanks a lot, I will send V3 patch to modify the commits. > diff --git a/Documentation/devicetree/bindings/pci/layerscape-pci.txt > b/Documentation/devicetree/bindings/pci/layerscape-pci.txt > index 66df1e8..9c090c7 100644 > --- a/Documentation/devicetree/bindings/pci/layerscape-pci.txt > +++ b/Documentation/devicetree/bindings/pci/layerscape-pci.txt > @@ -13,12 +13,15 @@ information. > > Required properties: > - compatible: should contain the platform identifier such as: > + RC mode: > "fsl,ls1021a-pcie", "snps,dw-pcie" > "fsl,ls2080a-pcie", "fsl,ls2085a-pcie", "snps,dw-pcie" > "fsl,ls2088a-pcie" > "fsl,ls1088a-pcie" > "fsl,ls1046a-pcie" > "fsl,ls1012a-pcie" > + EP mode: > +"fsl,ls1046a-pcie-ep", "fsl,ls-pcie-ep" > - reg: base addresses and lengths of the PCIe controller register blocks. > - interrupts: A list of interrupt outputs of the controller. Must contain an >entry for each entry in the interrupt-names property. > -- > 1.7.1 >
