RE: [PATCH 3/6] PCI: layerscape: Add the EP mode support
-Original Message- From: Li Yang Sent: 2018年10月27日 4:29 To: Xiaowei Bao Cc: Arnd Bergmann ; Rob Herring ; Bjorn Helgaas ; Mark Rutland ; Shawn Guo ; kis...@ti.com; lorenzo.pieral...@arm.com; Greg Kroah-Hartman ; M.h. Lian ; Mingkai Hu ; Roy Zang ; Kate Stewart ; cyrille.pitc...@free-electrons.com; Philippe Ombredanne ; shawn@rock-chips.com; niklas.cas...@axis.com; linux-...@vger.kernel.org; open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS ; lkml ; moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE ; linuxppc-dev Subject: Re: [PATCH 3/6] PCI: layerscape: Add the EP mode support On Fri, Oct 26, 2018 at 2:43 AM Xiaowei Bao wrote: > > > > -Original Message- > From: arndbergm...@gmail.com On Behalf Of > Arnd Bergmann > Sent: 2018年10月26日 15:01 > To: Xiaowei Bao > Cc: Rob Herring ; bhelg...@google.com; > mark.rutl...@arm.com; shawn...@kernel.org; Leo Li > ; kis...@ti.com; lorenzo.pieral...@arm.com; > 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: [PATCH 3/6] PCI: layerscape: Add the EP mode support > > On 10/26/18, Xiaowei Bao wrote: > > From: Rob Herring > >> On Thu, Oct 25, 2018 at 07:08:58PM +0800, Xiaowei Bao wrote: > >>> "fsl,ls2080a-pcie", "fsl,ls2085a-pcie", "snps,dw-pcie" > >>> "fsl,ls2088a-pcie" > >>> "fsl,ls1088a-pcie" > >>> "fsl,ls1046a-pcie" > >>> "fsl,ls1012a-pcie > >>> + EP mode: > >>> +"fsl,ls-pcie-ep" > >> > > > You need SoC specific compatibles for the same reasons as the RC. > > > > [Xiaowei Bao] I want to contains all layerscape platform use one > > compatible if the PCIe controller work in EP mode. > > Do you mean only one of the SoCs that support RC mode has EP mode? > I think you still need a SoC specific compatible as Rob explained, in case > there will be a second one in the future. > > If you want to ensure that you don't have to update the device driver for > each new chip that comes in when the EP mode is compatible, the way this is > handled is to list multiple values in the compatible property, listing the > first SoC that introduced the specific version of that IP block as the most > generic type, e.g. > > copatible = "fsl,ls2088a-pcie-ep", "fsl,ls1012a-pcie-ep", > "snps,dw-pcie-ep"; > > For consistency, it probably is best to match each RC mode value with the > corresponding EP mode string for each device that can support both (if there > is more than one). > > Arnd > [Xiaowei Bao] My mean is that the ls-pcie-ep compatibles will contain all > layerscape SOCs of NXP, e.g: ls1046a-pcie-ep, fsl,ls2088a-pcie-ep, > ls2088a-pcie-ep and so on, other layerscape SOCs have not test except the > ls1046a, I think it is compatible if the new chip or other SOCs use the DW > core, OK, I will discuss this issue internally, and reply to you later. You can define a generic compatible string for the EP mode of all these platforms. But like Rob and Arnd mentioned, it is good to also define the SoC specific compatible strings just in case that we need special treatment for certain SoCs in the future. Regards, Leo [Xiaowei Bao] Hi Leo, OK, I will add the SoC specific compatible strings in patch-v2, thanks a lot.
Re: [PATCH 2/5] powerpc/futex: GUAP support for futex ops
On Fri, 2018-10-26 at 18:32 +0200, LEROY Christophe wrote:
> Russell Currey a écrit :
>
> > Wrap the futex operations in GUAP locks and unlocks.
>
> Does it means futex doesn't work anymore once only patch 1 is
> applied
> ? If so, then you should split patch 1 in two parts and reorder
> patches so that guap can only be activated once all necessary
> changes
> are done. Otherwise the serie won't be bisectable
Yeah, I agree. I just wanted to remove some amount of breadth from
what already is one gigantic patch. Bisectability is more important
than that, however.
- Russell
>
> Christophe
>
> > Signed-off-by: Russell Currey
> > ---
> > arch/powerpc/include/asm/futex.h | 4
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/arch/powerpc/include/asm/futex.h
> > b/arch/powerpc/include/asm/futex.h
> > index 94542776a62d..3aed640ee9ef 100644
> > --- a/arch/powerpc/include/asm/futex.h
> > +++ b/arch/powerpc/include/asm/futex.h
> > @@ -35,6 +35,7 @@ static inline int
> > arch_futex_atomic_op_inuser(int
> > op, int oparg, int *oval,
> > {
> > int oldval = 0, ret;
> >
> > + unlock_user_access();
> > pagefault_disable();
> >
> > switch (op) {
> > @@ -62,6 +63,7 @@ static inline int
> > arch_futex_atomic_op_inuser(int
> > op, int oparg, int *oval,
> > if (!ret)
> > *oval = oldval;
> >
> > + lock_user_access();
> > return ret;
> > }
> >
> > @@ -75,6 +77,7 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32
> > __user *uaddr,
> > if (!access_ok(VERIFY_WRITE, uaddr, sizeof(u32)))
> > return -EFAULT;
> >
> > + unlock_user_access();
> > __asm__ __volatile__ (
> > PPC_ATOMIC_ENTRY_BARRIER
> > "1: lwarx %1,0,%3 # futex_atomic_cmpxchg_inatomic\n\
> > @@ -95,6 +98,7 @@ futex_atomic_cmpxchg_inatomic(u32 *uval, u32
> > __user *uaddr,
> > : "cc", "memory");
> >
> > *uval = prev;
> > + lock_user_access();
> > return ret;
> > }
> >
> > --
> > 2.19.1
>
>
Re: [PATCH 5/5] powerpc/64s: Document that PPC supports nosmap
On Fri, 2018-10-26 at 18:35 +0200, LEROY Christophe wrote: > Why not call our new functionnality SMAP instead of calling it GUAP ? mpe wasn't a fan of using the same terminology as other architectures. Having a separate term does avoid some assumptions about how things work or are implemented, but sharing compatibility with an existing parameter is nice. Personally I don't really care too much about the name. - Russell > > Christophe > > Russell Currey a écrit : > > > Signed-off-by: Russell Currey > > --- > > Documentation/admin-guide/kernel-parameters.txt | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/Documentation/admin-guide/kernel-parameters.txt > > b/Documentation/admin-guide/kernel-parameters.txt > > index a5ad67d5cb16..8f78e75965f0 100644 > > --- a/Documentation/admin-guide/kernel-parameters.txt > > +++ b/Documentation/admin-guide/kernel-parameters.txt > > @@ -2764,7 +2764,7 @@ > > noexec=on: enable non-executable mappings > > (default) > > noexec=off: disable non-executable mappings > > > > - nosmap [X86] > > + nosmap [X86,PPC] > > Disable SMAP (Supervisor Mode Access > > Prevention) > > even if it is supported by processor. > > > > -- > > 2.19.1 > >
Re: [PATCH 2/4] mm: speed up mremap by 500x on large regions (v2)
On Sat, Oct 27, 2018 at 12:39:17PM -0700, Joel Fernandes wrote: > Hi Balbir, > > On Sat, Oct 27, 2018 at 09:21:02PM +1100, Balbir Singh wrote: > > On Wed, Oct 24, 2018 at 07:13:50PM -0700, Joel Fernandes wrote: > > > On Wed, Oct 24, 2018 at 10:57:33PM +1100, Balbir Singh wrote: > > > [...] > > > > > > + pmd_t pmd; > > > > > > + > > > > > > + new_ptl = pmd_lockptr(mm, new_pmd); > > > > > > > > > > > > Looks like this is largely inspired by move_huge_pmd(), I guess a lot of > > > > the code applies, why not just reuse as much as possible? The same > > > > comments > > > > w.r.t mmap_sem helping protect against lock order issues applies as > > > > well. > > > > > > I thought about this and when I looked into it, it seemed there are subtle > > > differences that make such sharing not worth it (or not possible). > > > > > > > Could you elaborate on them? > > The move_huge_page function is defined only for CONFIG_TRANSPARENT_HUGEPAGE > so we cannot reuse it to begin with, since we have it disabled on our > systems. I am not sure if it is a good idea to split that out and refactor it > for reuse especially since our case is quite simple compared to huge pages. > > There are also a couple of subtle differences between the move_normal_pmd and > the move_huge_pmd. Atleast 2 of them are: > > 1. We don't concern ourself with the PMD dirty bit, since the pages being > moved are normal pages and at the soft-dirty bit accounting is at the PTE > level, since we are not moving PTEs, we don't need to do that. > > 2. The locking is simpler as Kirill pointed, pmd_lock cannot fail however > __pmd_trans_huge_lock can. > > I feel it is not super useful to refactor move_huge_pmd to support our case > especially since move_normal_pmd is quite small, so IMHO the benefit of code > reuse isn't there very much. > My big concern is that any bug fixes will need to monitor both paths. Do you see a big overhead in checking the soft dirty bit? The locking is a little different. Having said that, I am not strictly opposed to the extra code, just concerned about missing fixes/updates as we find them. > Do let me know your thoughts and thanks for your interest in this. > > Balbir Singh.
Re: ethernet "bus" number in DTS ?
On Fri, 26 Oct 2018 15:57:15 -0700 Florian Fainelli wrote: > On 10/23/18 11:22 PM, Michal Suchánek wrote: > > On Tue, 23 Oct 2018 11:20:36 -0700 > > Florian Fainelli wrote: > > > >> On 10/23/18 11:02 AM, Joakim Tjernlund wrote: > >>> On Tue, 2018-10-23 at 10:03 -0700, Florian Fainelli wrote: > > > >>> > >>> I also noted that using status = "disabled" didn't work either to > >>> create a fix name scheme. Even worse, all the eth I/F after gets > >>> renumbered. It seems to me there is value in having stability in > >>> eth I/F naming at boot. Then userspace(udev) can rename if need > >>> be. > >>> > >>> Sure would like to known more about why this feature is not > >>> wanted ? > >>> > >>> I found > >>> https://patchwork.kernel.org/patch/4122441/ > >>> You quote policy as reason but surely it must be better to > >>> have something stable, connected to the hardware name, than > >>> semirandom naming? > >> > >> If the Device Tree nodes are ordered by ascending base register > >> address, my understanding is that you get the same order as far as > >> platform_device creation goes, this may not be true in the future > >> if Rob decides to randomize that, but AFAICT this is still true. > >> This may not work well with status = disabled properties being > >> inserted here and there, but we have used that here and it has > >> worked for as far as I can remember doing it. > > > > So this is unstable in several respects. First is changing the > > enabled/disabled status in the deivecetrees provided by the kernel. > > > > Second is if you have hardware hotplug mechanism either by firmware > > or by loading device overlays. > > > > Third is the case when the devicetree is not built as part of the > > kernel but is instead provided by firmware that initializes the > > low-level hardware details. Then the ordering by address is not > > guaranteed nor is that the same address will be used to access the > > same interface every time. There might be multiple ways to > > configure the hardware depending on firmware configuration and/or > > version. > > > > > >> Second, you might want to name network devices ethX, but what if I > >> want to name them ethernetX or fooX or barX? Should we be > >> accepting a mechanism in the kernel that would allow someone to > >> name the interfaces the way they want straight from a name being > >> provided in Device Tree? > > > > Clearly if there is text Ethernet1 printed above the Ethernet port > > we should provide a mechanism to name the port Ethernet1 by > > default. > > Yes, but then have a specific property that is flexible enough to > retrieve things like "vital product information". For DSA switches, we > have an optional "label" property which names the network device > directly like it would be found on the product's case. Ideally this > should be much more flexible such that it can point to the appropriate > node/firmware service/whatever to get that name, which is what some > people have been working on lately, see [1]. > > [1]: https://lkml.org/lkml/2018/8/14/1039 That's nice for something unique per-device like MAC address. However, for something per-model like port labels DT properties should suffice. > > The point is don't re-purpose the aliases which is something that > exists within Device Tree to basically provide a shorter path to a > specific set of nodes for the boot program to mangle and muck with > instead of having to resolve a full path to a node. At least that is > how I conceive it. > > Now what complicates the matter is that some OSes like Linux tend to > use it to also generate seemingly stable index for peripherals that > are numbered by index such as SPI, I2C, etc buses, which is why we are > having this conversation here, see below for more. > > > > >> > >> Aliases are fine for providing relative stability within the Device > >> Tree itself and boot programs that might need to modify the Device > >> Tree (e.g: inserting MAC addresses) such that you don't have to > >> encode logic to search for nodes by compatible strings etc. but > >> outside of that use case, it seems to me that you can resolve every > >> naming decision in user-space. > > > > However, this is pushing platform-specific knowledge to userspace. > > The way the Ethernet interface is attached and hence the device > > properties usable for identifying the device uniquely are > > platform-specific. > > There is always going to be some amount of platform specific knowledge > to user-space, what matters is the level of abstraction that is > presented to you. > > > > > On the other hand, aliases are universal when provided. If they are > > good enough to assign a MAC address they are good enough to provide > > a stable default name. > > We are not talking about the same aliases then. The special Device > Tree node named "aliases" is a way to create shorted Device Tree node > paths, it is not by any means an equivalent for what I would rather > c
Re: [PATCH 1/5] powerpc/64s: Guarded Userspace Access Prevention
Russell Currey a écrit :
Guarded Userspace Access Prevention (GUAP) utilises a feature of
the Radix MMU which disallows read and write access to userspace
addresses. By utilising this, the kernel is prevented from accessing
user data from outside of trusted paths that perform proper safety checks,
such as copy_{to/from}_user() and friends.
Userspace access is disabled from early boot and is only enabled when:
- exiting the kernel and entering userspace
- performing an operation like copy_{to/from}_user()
- context switching to a process that has access enabled
and similarly, access is disabled again when exiting userspace and entering
the kernel.
This feature has a slight performance impact which I roughly measured to be
3% slower in the worst case (performing 1GB of 1 byte read()/write()
syscalls), and is gated behind the CONFIG_PPC_RADIX_GUAP option for
performance-critical builds.
This feature can be tested by using the lkdtm driver (CONFIG_LKDTM=y) and
performing the following:
echo ACCESS_USERSPACE > [debugfs]/provoke-crash/DIRECT
if enabled, this should send SIGSEGV to the thread.
Signed-off-by: Russell Currey
I think this patch should be split in at least two parts:
First part for implementing the generic part, including the changes to
futex and csum, and a second part implementing the radix part.
---
Since the previous version of this patchset (named KHRAP) there have been
several changes, some of which include:
- macro naming, suggested by Nick
- builds should be fixed outside of 64s
- no longer unlock heading out to userspace
- removal of unnecessary isyncs
- more config option testing
- removal of save/restore
- use pr_crit() and reword message on fault
arch/powerpc/include/asm/exception-64e.h | 3 ++
arch/powerpc/include/asm/exception-64s.h | 19 +++-
arch/powerpc/include/asm/mmu.h | 7 +++
arch/powerpc/include/asm/paca.h | 3 ++
arch/powerpc/include/asm/reg.h | 1 +
arch/powerpc/include/asm/uaccess.h | 57
arch/powerpc/kernel/asm-offsets.c| 1 +
arch/powerpc/kernel/dt_cpu_ftrs.c| 4 ++
arch/powerpc/kernel/entry_64.S | 17 ++-
arch/powerpc/mm/fault.c | 12 +
arch/powerpc/mm/pgtable-radix.c | 2 +
arch/powerpc/mm/pkeys.c | 7 ++-
arch/powerpc/platforms/Kconfig.cputype | 15 +++
13 files changed, 135 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/include/asm/exception-64e.h
b/arch/powerpc/include/asm/exception-64e.h
index 555e22d5e07f..bf25015834ee 100644
--- a/arch/powerpc/include/asm/exception-64e.h
+++ b/arch/powerpc/include/asm/exception-64e.h
@@ -215,5 +215,8 @@ exc_##label##_book3e:
#define RFI_TO_USER\
rfi
+#define UNLOCK_USER_ACCESS(reg)
+#define LOCK_USER_ACCESS(reg)
+
#endif /* _ASM_POWERPC_EXCEPTION_64E_H */
diff --git a/arch/powerpc/include/asm/exception-64s.h
b/arch/powerpc/include/asm/exception-64s.h
index 3b4767ed3ec5..0cac5bd380ca 100644
--- a/arch/powerpc/include/asm/exception-64s.h
+++ b/arch/powerpc/include/asm/exception-64s.h
@@ -264,6 +264,19 @@ BEGIN_FTR_SECTION_NESTED(943)
\
std ra,offset(r13); \
END_FTR_SECTION_NESTED(ftr,ftr,943)
+#define LOCK_USER_ACCESS(reg)
\
+BEGIN_MMU_FTR_SECTION_NESTED(944) \
+ LOAD_REG_IMMEDIATE(reg,AMR_LOCKED); \
+ mtspr SPRN_AMR,reg; \
+END_MMU_FTR_SECTION_NESTED(MMU_FTR_RADIX_GUAP,MMU_FTR_RADIX_GUAP,944)
+
+#define UNLOCK_USER_ACCESS(reg)
\
+BEGIN_MMU_FTR_SECTION_NESTED(945) \
+ li reg,0; \
+ mtspr SPRN_AMR,reg; \
+ isync \
+END_MMU_FTR_SECTION_NESTED(MMU_FTR_RADIX_GUAP,MMU_FTR_RADIX_GUAP,945)
+
#define EXCEPTION_PROLOG_0(area) \
GET_PACA(r13); \
std r9,area+EX_R9(r13); /* save r9 */ \
@@ -500,7 +513,11 @@ END_FTR_SECTION_NESTED(ftr,ftr,943)
beq 4f; /* if from kernel mode */ \
ACCOUNT_CPU_USER_ENTRY(r13, r9, r10); \
SAVE_PPR(area, r9);\
-4: EXCEPTION_PROLOG_COMMON_2(area)\
+4: lbz r9,PACA_USER_ACCESS_ALLOWED(r13); \
+ cmpwi cr1,r9,0;
[PATCH] hwmon: (ibmpowernv) Remove bogus __init annotations
If gcc decides not to inline make_sensor_label():
WARNING: vmlinux.o(.text+0x4df549c): Section mismatch in reference from the
function .create_device_attrs() to the function .init.text:.make_sensor_label()
The function .create_device_attrs() references
the function __init .make_sensor_label().
This is often because .create_device_attrs lacks a __init
annotation or the annotation of .make_sensor_label is wrong.
As .probe() can be called after freeing of __init memory, all __init
annotiations in the driver are bogus, and should be removed.
Signed-off-by: Geert Uytterhoeven
---
Compile-tested only.
---
drivers/hwmon/ibmpowernv.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/hwmon/ibmpowernv.c b/drivers/hwmon/ibmpowernv.c
index 0ccca87f527191dc..293dd1c6c7b36ef2 100644
--- a/drivers/hwmon/ibmpowernv.c
+++ b/drivers/hwmon/ibmpowernv.c
@@ -181,7 +181,7 @@ static ssize_t show_label(struct device *dev, struct
device_attribute *devattr,
return sprintf(buf, "%s\n", sdata->label);
}
-static int __init get_logical_cpu(int hwcpu)
+static int get_logical_cpu(int hwcpu)
{
int cpu;
@@ -192,9 +192,8 @@ static int __init get_logical_cpu(int hwcpu)
return -ENOENT;
}
-static void __init make_sensor_label(struct device_node *np,
-struct sensor_data *sdata,
-const char *label)
+static void make_sensor_label(struct device_node *np,
+ struct sensor_data *sdata, const char *label)
{
u32 id;
size_t n;
--
2.17.1
NXP P50XX/e5500: SMP doesn't work anymore with the latest Git kernel
Hello, SMP doesn't work anymore with the latest Git kernel (28/10/18 11:12AM GMT) on my P5020 board and on virtual e5500 QEMU machines. Board with P5020 dual core CPU: [ 0.00] - [ 0.00] phys_mem_size = 0x2 [ 0.00] dcache_bsize = 0x40 [ 0.00] icache_bsize = 0x40 [ 0.00] cpu_features = 0x0003008003b4 [ 0.00] possible = 0x0003009003b4 [ 0.00] always = 0x0003008003b4 [ 0.00] cpu_user_features = 0xcc008000 0x0800 [ 0.00] mmu_features = 0x000a0010 [ 0.00] firmware_features = 0x [ 0.00] - [ 0.00] CoreNet Generic board ... [ 0.002161] smp: Bringing up secondary CPUs ... [ 0.002339] No cpu-release-addr for cpu 1 [ 0.002347] smp: failed starting cpu 1 (rc -2) [ 0.002401] smp: Brought up 1 node, 1 CPU Virtual e5500 quad core QEMU machine: [ 0.026394] smp: Bringing up secondary CPUs ... [ 0.027831] No cpu-release-addr for cpu 1 [ 0.027989] smp: failed starting cpu 1 (rc -2) [ 0.030143] No cpu-release-addr for cpu 2 [ 0.030304] smp: failed starting cpu 2 (rc -2) [ 0.032400] No cpu-release-addr for cpu 3 [ 0.032533] smp: failed starting cpu 3 (rc -2) [ 0.033117] smp: Brought up 1 node, 1 CPU QEMU command: ./qemu-system-ppc64 -M ppce500 -cpu e5500 -m 2048 -kernel /home/christian/Downloads/vmlinux-4.20-alpha4-AmigaOne_X1000_X5000/X5000_and_QEMU_e5500/uImage-4.20 -drive format=raw,file=/home/christian/Downloads/MATE_PowerPC_Remix_2017_0.9.img,index=0,if=virtio -nic user,model=e1000 -append "rw root=/dev/vda" -device virtio-vga -device virtio-mouse-pci -device virtio-keyboard-pci -usb -soundhw es1370 -smp 4 .config: ... CONFIG_SMP=y CONFIG_NR_CPUS=4 ... Please test the latest Git kernel on your NXP P50XX boards. Thanks, Christian
[PATCH] ppc4xx: ocm: fix errnous "failed to create file" errors
Previously, the kernel would complain:
| debugfs ppc4xx ocm: failed to create file
But the "info" file was still created and working. This
is because debugfs_create_file() returns a pointer to a
struct *dentry on success or -ENODEV when debugfs isn't
available. This patch fixes both the debugfs_create_dir()
and debugfs_create_file() check, so this will work as
expected.
Signed-off-by: Christian Lamparter
---
arch/powerpc/platforms/4xx/ocm.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/platforms/4xx/ocm.c b/arch/powerpc/platforms/4xx/ocm.c
index 69d9f60d9fe5..e616c8636303 100644
--- a/arch/powerpc/platforms/4xx/ocm.c
+++ b/arch/powerpc/platforms/4xx/ocm.c
@@ -293,13 +293,14 @@ static int ocm_debugfs_init(void)
{
struct dentry *junk;
- junk = debugfs_create_dir("ppc4xx_ocm", 0);
- if (!junk) {
+ junk = debugfs_create_dir("ppc4xx_ocm", NULL);
+ if (IS_ERR_OR_NULL(junk)) {
printk(KERN_ALERT "debugfs ppc4xx ocm: failed to create dir\n");
return -1;
}
- if (debugfs_create_file("info", 0644, junk, NULL, &ocm_debugfs_fops)) {
+ if (IS_ERR_OR_NULL(debugfs_create_file("info", 0644, junk, NULL,
+ &ocm_debugfs_fops))) {
printk(KERN_ALERT "debugfs ppc4xx ocm: failed to create
file\n");
return -1;
}
--
2.19.1
