* [PATCH 7/7] exec: open code copy_string_kernel
From: Christoph Hellwig @ 2020-04-21 15:42 UTC (permalink / raw)
To: Andrew Morton, Alexander Viro
Cc: Arnd Bergmann, linux-kernel, Jeremy Kerr, linux-fsdevel,
linuxppc-dev, Eric W . Biederman
In-Reply-To: <20200421154204.252921-1-hch@lst.de>
Currently copy_string_kernel is just a wrapper around copy_strings that
simplifies the calling conventions and uses set_fs to allow passing a
kernel pointer. But due to the fact the we only need to handle a single
kernel argument pointer, the logic can be sigificantly simplified while
getting rid of the set_fs.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/exec.c | 43 ++++++++++++++++++++++++++++++++++---------
1 file changed, 34 insertions(+), 9 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index b2a77d5acede..ea90af1fb236 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -592,17 +592,42 @@ static int copy_strings(int argc, struct user_arg_ptr argv,
*/
int copy_string_kernel(const char *arg, struct linux_binprm *bprm)
{
- int r;
- mm_segment_t oldfs = get_fs();
- struct user_arg_ptr argv = {
- .ptr.native = (const char __user *const __user *)&arg,
- };
+ int len = strnlen(arg, MAX_ARG_STRLEN) + 1 /* terminating NUL */;
+ unsigned long pos = bprm->p;
+
+ if (len == 0)
+ return -EFAULT;
+ if (!valid_arg_len(bprm, len))
+ return -E2BIG;
+
+ /* We're going to work our way backwards. */
+ arg += len;
+ bprm->p -= len;
+ if (IS_ENABLED(CONFIG_MMU) && bprm->p < bprm->argmin)
+ return -E2BIG;
+
+ while (len > 0) {
+ unsigned int bytes_to_copy = min_t(unsigned int, len,
+ min_not_zero(offset_in_page(pos), PAGE_SIZE));
+ struct page *page;
+ char *kaddr;
- set_fs(KERNEL_DS);
- r = copy_strings(1, argv, bprm);
- set_fs(oldfs);
+ pos -= bytes_to_copy;
+ arg -= bytes_to_copy;
+ len -= bytes_to_copy;
- return r;
+ page = get_arg_page(bprm, pos, 1);
+ if (!page)
+ return -E2BIG;
+ kaddr = kmap_atomic(page);
+ flush_arg_page(bprm, pos & PAGE_MASK, page);
+ memcpy(kaddr + offset_in_page(pos), arg, bytes_to_copy);
+ flush_kernel_dcache_page(page);
+ kunmap_atomic(kaddr);
+ put_arg_page(page);
+ }
+
+ return 0;
}
EXPORT_SYMBOL(copy_string_kernel);
--
2.26.1
^ permalink raw reply related
* Re: [PATCH 2/5] powerpc: Replace _ALIGN_DOWN() by ALIGN_DOWN()
From: Segher Boessenkool @ 2020-04-21 15:55 UTC (permalink / raw)
To: Joel Stanley
Cc: linux-fbdev, kvm, Linux Kernel Mailing List, dri-devel,
Paul Mackerras, alsa-devel, linuxppc-dev
In-Reply-To: <CACPK8XfqnqgkXcBzp=nqd=AJX1MK05eTNiyOdaEuRu3_6RsXSQ@mail.gmail.com>
Hi!
On Tue, Apr 21, 2020 at 01:04:05AM +0000, Joel Stanley wrote:
> On Mon, 20 Apr 2020 at 18:38, Christophe Leroy <christophe.leroy@c-s.fr> wrote:
> > _ALIGN_DOWN() is specific to powerpc
> > ALIGN_DOWN() is generic and does the same
> >
> > Replace _ALIGN_DOWN() by ALIGN_DOWN()
>
> This one is a bit less obvious. It becomes (leaving the typeof's alone
> for clarity):
>
> -((addr)&(~((typeof(addr))(size)-1)))
> +((((addr) - ((size) - 1)) + ((typeof(addr))(size) - 1)) &
> ~((typeof(addr))(size)-1))
>
> Which I assume the compiler will sort out?
[ This is line-wrapped, something in your mailer? Took me a bit to figure
out the - and + are diff -u things :-) ]
In the common case where size is a constant integer power of two, the
compiler will have no problem with this. But why do so complicated?
Why are the casts there, btw?
Segher
^ permalink raw reply
* RE: [musl] Powerpc Linux 'scv' system call ABI proposal take 2
From: David Laight @ 2020-04-21 12:28 UTC (permalink / raw)
To: 'Nicholas Piggin', Adhemerval Zanella, Rich Felker
Cc: libc-dev@lists.llvm.org, libc-alpha@sourceware.org,
linuxppc-dev@lists.ozlabs.org, musl@lists.openwall.com
In-Reply-To: <1587344003.daumxvs1kh.astroid@bobo.none>
From: Nicholas Piggin
> Sent: 20 April 2020 02:10
...
> >> Yes, but does it really matter to optimize this specific usage case
> >> for size? glibc, for instance, tries to leverage the syscall mechanism
> >> by adding some complex pre-processor asm directives. It optimizes
> >> the syscall code size in most cases. For instance, kill in static case
> >> generates on x86_64:
> >>
> >> 0000000000000000 <__kill>:
> >> 0: b8 3e 00 00 00 mov $0x3e,%eax
> >> 5: 0f 05 syscall
> >> 7: 48 3d 01 f0 ff ff cmp $0xfffffffffffff001,%rax
> >> d: 0f 83 00 00 00 00 jae 13 <__kill+0x13>
Hmmm... that cmp + jae is unnecessary here.
It is also a 32bit offset jump.
I also suspect it gets predicted very badly.
> >> 13: c3 retq
> >>
> >> While on musl:
> >>
> >> 0000000000000000 <kill>:
> >> 0: 48 83 ec 08 sub $0x8,%rsp
> >> 4: 48 63 ff movslq %edi,%rdi
> >> 7: 48 63 f6 movslq %esi,%rsi
> >> a: b8 3e 00 00 00 mov $0x3e,%eax
> >> f: 0f 05 syscall
> >> 11: 48 89 c7 mov %rax,%rdi
> >> 14: e8 00 00 00 00 callq 19 <kill+0x19>
> >> 19: 5a pop %rdx
> >> 1a: c3 retq
> >
> > Wow that's some extraordinarily bad codegen going on by gcc... The
> > sign-extension is semantically needed and I don't see a good way
> > around it (glibc's asm is kinda a hack taking advantage of kernel not
> > looking at high bits, I think), but the gratuitous stack adjustment
> > and refusal to generate a tail call isn't. I'll see if we can track
> > down what's going on and get it fixed.
A suitable cast might get rid of the sign extension.
Possibly just (unsigned int).
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* [PATCH v2 00/29] fs: convert remaining docs to ReST file format
From: Mauro Carvalho Chehab @ 2020-04-21 16:54 UTC (permalink / raw)
To: Linux Doc Mailing List
Cc: linux-usb, Jan Kara, Amir Goldstein, J. Bruce Fields,
Christoph Hellwig, Jonathan Corbet, Mauro Carvalho Chehab,
codalist, coda, linux-cachefs, Alexey Dobriyan, Joel Becker,
Jan Harkes, David Howells, Greg Kroah-Hartman, Darrick J. Wong,
Jeff Layton, linux-kernel, linux-xfs, Jeremy Kerr, Jan Kara,
linux-fsdevel, linuxppc-dev, Alexander Viro
This patch series convert the remaining files under Documentation/filesystems
to the ReST file format (except for dax.txt and patch-lookup.txt).
It is based on linux-next (next-2020021).
PS.: I opted to add mainly ML from the output of get_maintainers.pl to the c/c
list of patch 00/34, because otherwise the number of c/c would be too many,
with would very likely cause ML servers to reject it.
The results of those changes (together with other changes from my pending
doc patches) are available at:
https://www.infradead.org/~mchehab/kernel_docs/filesystems/index.html
And the series is also on my git tree:
https://git.linuxtv.org/mchehab/experimental.git/log/?h=fs_docs
If you prefer, feel free to merge the patches via the filesystems git tree(s).
-
v2:
- Removed dax.txt, in order to prevent a merge conflict with a dax patchset;
- Removed patch-lookup.txt conversion, due to patch-lookup.rst.
(I'll revisit this on some future)
- Removed a patch that was already merged;
- Added some Acked-by.
Mauro Carvalho Chehab (29):
docs: filesystems: convert caching/object.txt to ReST
docs: filesystems: convert caching/fscache.txt to ReST format
docs: filesystems: caching/netfs-api.txt: convert it to ReST
docs: filesystems: caching/operations.txt: convert it to ReST
docs: filesystems: caching/cachefiles.txt: convert to ReST
docs: filesystems: caching/backend-api.txt: convert it to ReST
docs: filesystems: convert cifs/cifsroot.txt to ReST
docs: filesystems: convert configfs.txt to ReST
docs: filesystems: convert automount-support.txt to ReST
docs: filesystems: convert coda.txt to ReST
docs: filesystems: convert devpts.txt to ReST
docs: filesystems: convert dnotify.txt to ReST
docs: filesystems: convert fiemap.txt to ReST
docs: filesystems: convert files.txt to ReST
docs: filesystems: convert fuse-io.txt to ReST
docs: filesystems: convert locks.txt to ReST
docs: filesystems: convert mandatory-locking.txt to ReST
docs: filesystems: convert mount_api.txt to ReST
docs: filesystems: convert quota.txt to ReST
docs: filesystems: convert seq_file.txt to ReST
docs: filesystems: convert sharedsubtree.txt to ReST
docs: filesystems: split spufs.txt into 3 separate files
docs: filesystems: convert spufs/spu_create.txt to ReST
docs: filesystems: convert spufs/spufs.txt to ReST
docs: filesystems: convert spufs/spu_run.txt to ReST
docs: filesystems: convert sysfs-pci.txt to ReST
docs: filesystems: convert sysfs-tagging.txt to ReST
docs: filesystems: convert xfs-delayed-logging-design.txt to ReST
docs: filesystems: convert xfs-self-describing-metadata.txt to ReST
Documentation/admin-guide/sysctl/kernel.rst | 2 +-
...ount-support.txt => automount-support.rst} | 23 +-
.../{backend-api.txt => backend-api.rst} | 165 +-
.../{cachefiles.txt => cachefiles.rst} | 139 +-
Documentation/filesystems/caching/fscache.rst | 565 ++++++
Documentation/filesystems/caching/fscache.txt | 448 -----
Documentation/filesystems/caching/index.rst | 14 +
.../caching/{netfs-api.txt => netfs-api.rst} | 172 +-
.../caching/{object.txt => object.rst} | 43 +-
.../{operations.txt => operations.rst} | 45 +-
.../cifs/{cifsroot.txt => cifsroot.rst} | 56 +-
Documentation/filesystems/coda.rst | 1670 ++++++++++++++++
Documentation/filesystems/coda.txt | 1676 -----------------
.../{configfs/configfs.txt => configfs.rst} | 129 +-
Documentation/filesystems/devpts.rst | 36 +
Documentation/filesystems/devpts.txt | 26 -
.../filesystems/{dnotify.txt => dnotify.rst} | 11 +-
.../filesystems/{fiemap.txt => fiemap.rst} | 133 +-
.../filesystems/{files.txt => files.rst} | 15 +-
.../filesystems/{fuse-io.txt => fuse-io.rst} | 6 +
Documentation/filesystems/index.rst | 23 +
.../filesystems/{locks.txt => locks.rst} | 14 +-
...tory-locking.txt => mandatory-locking.rst} | 25 +-
.../{mount_api.txt => mount_api.rst} | 329 ++--
Documentation/filesystems/proc.rst | 2 +-
.../filesystems/{quota.txt => quota.rst} | 41 +-
.../{seq_file.txt => seq_file.rst} | 61 +-
.../{sharedsubtree.txt => sharedsubtree.rst} | 394 ++--
Documentation/filesystems/spufs/index.rst | 13 +
.../filesystems/spufs/spu_create.rst | 131 ++
Documentation/filesystems/spufs/spu_run.rst | 138 ++
.../{spufs.txt => spufs/spufs.rst} | 304 +--
.../{sysfs-pci.txt => sysfs-pci.rst} | 23 +-
.../{sysfs-tagging.txt => sysfs-tagging.rst} | 22 +-
...ign.txt => xfs-delayed-logging-design.rst} | 65 +-
...a.txt => xfs-self-describing-metadata.rst} | 182 +-
Documentation/iio/iio_configfs.rst | 2 +-
Documentation/usb/gadget_configfs.rst | 4 +-
MAINTAINERS | 14 +-
fs/cachefiles/Kconfig | 4 +-
fs/coda/Kconfig | 2 +-
fs/configfs/inode.c | 2 +-
fs/configfs/item.c | 2 +-
fs/fscache/Kconfig | 8 +-
fs/fscache/cache.c | 8 +-
fs/fscache/cookie.c | 2 +-
fs/fscache/object.c | 4 +-
fs/fscache/operation.c | 2 +-
fs/locks.c | 2 +-
include/linux/configfs.h | 2 +-
include/linux/fs_context.h | 2 +-
include/linux/fscache-cache.h | 4 +-
include/linux/fscache.h | 42 +-
include/linux/lsm_hooks.h | 2 +-
54 files changed, 3844 insertions(+), 3405 deletions(-)
rename Documentation/filesystems/{automount-support.txt => automount-support.rst} (92%)
rename Documentation/filesystems/caching/{backend-api.txt => backend-api.rst} (87%)
rename Documentation/filesystems/caching/{cachefiles.txt => cachefiles.rst} (90%)
create mode 100644 Documentation/filesystems/caching/fscache.rst
delete mode 100644 Documentation/filesystems/caching/fscache.txt
create mode 100644 Documentation/filesystems/caching/index.rst
rename Documentation/filesystems/caching/{netfs-api.txt => netfs-api.rst} (91%)
rename Documentation/filesystems/caching/{object.txt => object.rst} (95%)
rename Documentation/filesystems/caching/{operations.txt => operations.rst} (90%)
rename Documentation/filesystems/cifs/{cifsroot.txt => cifsroot.rst} (72%)
create mode 100644 Documentation/filesystems/coda.rst
delete mode 100644 Documentation/filesystems/coda.txt
rename Documentation/filesystems/{configfs/configfs.txt => configfs.rst} (87%)
create mode 100644 Documentation/filesystems/devpts.rst
delete mode 100644 Documentation/filesystems/devpts.txt
rename Documentation/filesystems/{dnotify.txt => dnotify.rst} (90%)
rename Documentation/filesystems/{fiemap.txt => fiemap.rst} (70%)
rename Documentation/filesystems/{files.txt => files.rst} (95%)
rename Documentation/filesystems/{fuse-io.txt => fuse-io.rst} (95%)
rename Documentation/filesystems/{locks.txt => locks.rst} (91%)
rename Documentation/filesystems/{mandatory-locking.txt => mandatory-locking.rst} (91%)
rename Documentation/filesystems/{mount_api.txt => mount_api.rst} (79%)
rename Documentation/filesystems/{quota.txt => quota.rst} (81%)
rename Documentation/filesystems/{seq_file.txt => seq_file.rst} (92%)
rename Documentation/filesystems/{sharedsubtree.txt => sharedsubtree.rst} (72%)
create mode 100644 Documentation/filesystems/spufs/index.rst
create mode 100644 Documentation/filesystems/spufs/spu_create.rst
create mode 100644 Documentation/filesystems/spufs/spu_run.rst
rename Documentation/filesystems/{spufs.txt => spufs/spufs.rst} (57%)
rename Documentation/filesystems/{sysfs-pci.txt => sysfs-pci.rst} (92%)
rename Documentation/filesystems/{sysfs-tagging.txt => sysfs-tagging.rst} (72%)
rename Documentation/filesystems/{xfs-delayed-logging-design.txt => xfs-delayed-logging-design.rst} (97%)
rename Documentation/filesystems/{xfs-self-describing-metadata.txt => xfs-self-describing-metadata.rst} (83%)
--
2.25.2
^ permalink raw reply
* [PATCH v2 24/29] docs: filesystems: convert spufs/spufs.txt to ReST
From: Mauro Carvalho Chehab @ 2020-04-21 16:54 UTC (permalink / raw)
To: Linux Doc Mailing List
Cc: Mauro Carvalho Chehab, Jeremy Kerr, linuxppc-dev, linux-kernel,
Jonathan Corbet
In-Reply-To: <cover.1587487612.git.mchehab+huawei@kernel.org>
This file is at groff output format. Manually convert it to
ReST format, trying to preserve a similar output after parsed.
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
---
Documentation/filesystems/spufs/index.rst | 1 +
.../spufs/{spufs.txt => spufs.rst} | 59 +++++++++----------
MAINTAINERS | 2 +-
3 files changed, 30 insertions(+), 32 deletions(-)
rename Documentation/filesystems/spufs/{spufs.txt => spufs.rst} (95%)
diff --git a/Documentation/filesystems/spufs/index.rst b/Documentation/filesystems/spufs/index.rst
index 39553c6ebefd..939cf59a7d9e 100644
--- a/Documentation/filesystems/spufs/index.rst
+++ b/Documentation/filesystems/spufs/index.rst
@@ -8,4 +8,5 @@ SPU Filesystem
.. toctree::
:maxdepth: 1
+ spufs
spu_create
diff --git a/Documentation/filesystems/spufs/spufs.txt b/Documentation/filesystems/spufs/spufs.rst
similarity index 95%
rename from Documentation/filesystems/spufs/spufs.txt
rename to Documentation/filesystems/spufs/spufs.rst
index caf36aaae804..8a42859bb100 100644
--- a/Documentation/filesystems/spufs/spufs.txt
+++ b/Documentation/filesystems/spufs/spufs.rst
@@ -1,12 +1,18 @@
-SPUFS(2) Linux Programmer's Manual SPUFS(2)
+.. SPDX-License-Identifier: GPL-2.0
+=====
+spufs
+=====
+Name
+====
-NAME
spufs - the SPU file system
-DESCRIPTION
+Description
+===========
+
The SPU file system is used on PowerPC machines that implement the Cell
Broadband Engine Architecture in order to access Synergistic Processor
Units (SPUs).
@@ -21,7 +27,9 @@ DESCRIPTION
ally add or remove files.
-MOUNT OPTIONS
+Mount Options
+=============
+
uid=<uid>
set the user owning the mount point, the default is 0 (root).
@@ -29,7 +37,9 @@ MOUNT OPTIONS
set the group owning the mount point, the default is 0 (root).
-FILES
+Files
+=====
+
The files in spufs mostly follow the standard behavior for regular sys-
tem calls like read(2) or write(2), but often support only a subset of
the operations supported on regular file systems. This list details the
@@ -125,14 +135,12 @@ FILES
space is available for writing.
- /mbox_stat
- /ibox_stat
- /wbox_stat
+ /mbox_stat, /ibox_stat, /wbox_stat
Read-only files that contain the length of the current queue, i.e. how
many words can be read from mbox or ibox or how many words can be
written to wbox without blocking. The files can be read only in 4-byte
units and return a big-endian binary integer number. The possible
- operations on an open *box_stat file are:
+ operations on an open ``*box_stat`` file are:
read(2)
If a count smaller than four is requested, read returns -1 and
@@ -143,12 +151,7 @@ FILES
in EAGAIN.
- /npc
- /decr
- /decr_status
- /spu_tag_mask
- /event_mask
- /srr0
+ /npc, /decr, /decr_status, /spu_tag_mask, /event_mask, /srr0
Internal registers of the SPU. The representation is an ASCII string
with the numeric value of the next instruction to be executed. These
can be used in read/write mode for debugging, but normal operation of
@@ -157,17 +160,14 @@ FILES
The contents of these files are:
+ =================== ===================================
npc Next Program Counter
-
decr SPU Decrementer
-
decr_status Decrementer Status
-
spu_tag_mask MFC tag mask for SPU DMA
-
event_mask Event mask for SPU interrupts
-
srr0 Interrupt Return address register
+ =================== ===================================
The possible operations on an open npc, decr, decr_status,
@@ -206,8 +206,7 @@ FILES
from the data buffer, updating the value of the fpcr register.
- /signal1
- /signal2
+ /signal1, /signal2
The two signal notification channels of an SPU. These are read-write
files that operate on a 32 bit word. Writing to one of these files
triggers an interrupt on the SPU. The value written to the signal
@@ -233,8 +232,7 @@ FILES
file.
- /signal1_type
- /signal2_type
+ /signal1_type, /signal2_type
These two files change the behavior of the signal1 and signal2 notifi-
cation files. The contain a numerical ASCII string which is read as
either "1" or "0". In mode 0 (overwrite), the hardware replaces the
@@ -259,18 +257,17 @@ FILES
the previous setting.
-EXAMPLES
+Examples
+========
/etc/fstab entry
none /spu spufs gid=spu 0 0
-AUTHORS
+Authors
+=======
Arnd Bergmann <arndb@de.ibm.com>, Mark Nutter <mnutter@us.ibm.com>,
Ulrich Weigand <Ulrich.Weigand@de.ibm.com>
-SEE ALSO
+See Also
+========
capabilities(7), close(2), spu_create(2), spu_run(2), spufs(7)
-
-
-
-Linux 2005-09-28 SPUFS(2)
diff --git a/MAINTAINERS b/MAINTAINERS
index ac8ddd91928a..968c97eda352 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15955,7 +15955,7 @@ M: Jeremy Kerr <jk@ozlabs.org>
L: linuxppc-dev@lists.ozlabs.org
S: Supported
W: http://www.ibm.com/developerworks/power/cell/
-F: Documentation/filesystems/spufs.txt
+F: Documentation/filesystems/spufs/spufs.rst
F: arch/powerpc/platforms/cell/spufs/
SQUASHFS FILE SYSTEM
--
2.25.2
^ permalink raw reply related
* [PATCH 0/3] powerpc/module_64: Fix _mcount() stub
From: Naveen N. Rao @ 2020-04-21 17:35 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Qian Cai, Steven Rostedt
This series addresses the crash reported by Qian Cai on ppc64le with
-mprofile-kernel here:
https://lore.kernel.org/r/15AC5B0E-A221-4B8C-9039-FA96B8EF7C88@lca.pw
While fixing patch_instruction() should address the crash, we should
still change the default stub we setup for _mcount() for cases where a
kernel is built without ftrace.
- Naveen
Naveen N. Rao (3):
powerpc/module_64: Consolidate ftrace code
powerpc/module_64: Simplify check for -mprofile-kernel ftrace
relocations
powerpc/module_64: Use special stub for _mcount() with
-mprofile-kernel
arch/powerpc/include/asm/module.h | 3 -
arch/powerpc/kernel/module_64.c | 281 ++++++++++++++----------------
2 files changed, 127 insertions(+), 157 deletions(-)
base-commit: a9aa21d05c33c556e48c5062b6632a9b94906570
--
2.25.1
^ permalink raw reply
* [PATCH 1/3] powerpc/module_64: Consolidate ftrace code
From: Naveen N. Rao @ 2020-04-21 17:35 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Qian Cai, Steven Rostedt
In-Reply-To: <cover.1587488954.git.naveen.n.rao@linux.vnet.ibm.com>
module_trampoline_target() is only used by ftrace. Move the prototype
within the appropriate #ifdef in the header. Also, move the function
body to the end of module_64.c so as to consolidate all ftrace code in
one place.
No functional changes.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
arch/powerpc/include/asm/module.h | 3 --
arch/powerpc/kernel/module_64.c | 69 +++++++++++++++----------------
2 files changed, 33 insertions(+), 39 deletions(-)
diff --git a/arch/powerpc/include/asm/module.h b/arch/powerpc/include/asm/module.h
index 356658711a86..6b99d773f522 100644
--- a/arch/powerpc/include/asm/module.h
+++ b/arch/powerpc/include/asm/module.h
@@ -90,12 +90,9 @@ struct mod_arch_specific {
# ifdef MODULE
asm(".section .ftrace.tramp,\"ax\",@nobits; .align 3; .previous");
# endif /* MODULE */
-#endif
int module_trampoline_target(struct module *mod, unsigned long trampoline,
unsigned long *target);
-
-#ifdef CONFIG_DYNAMIC_FTRACE
int module_finalize_ftrace(struct module *mod, const Elf_Shdr *sechdrs);
#else
static inline int module_finalize_ftrace(struct module *mod, const Elf_Shdr *sechdrs)
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 007606a48fd9..54d9d830f4a4 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -144,42 +144,6 @@ static u32 ppc64_stub_insns[] = {
PPC_INST_BCTR,
};
-#ifdef CONFIG_DYNAMIC_FTRACE
-int module_trampoline_target(struct module *mod, unsigned long addr,
- unsigned long *target)
-{
- struct ppc64_stub_entry *stub;
- func_desc_t funcdata;
- u32 magic;
-
- if (!within_module_core(addr, mod)) {
- pr_err("%s: stub %lx not in module %s\n", __func__, addr, mod->name);
- return -EFAULT;
- }
-
- stub = (struct ppc64_stub_entry *)addr;
-
- if (probe_kernel_read(&magic, &stub->magic, sizeof(magic))) {
- pr_err("%s: fault reading magic for stub %lx for %s\n", __func__, addr, mod->name);
- return -EFAULT;
- }
-
- if (magic != STUB_MAGIC) {
- pr_err("%s: bad magic for stub %lx for %s\n", __func__, addr, mod->name);
- return -EFAULT;
- }
-
- if (probe_kernel_read(&funcdata, &stub->funcdata, sizeof(funcdata))) {
- pr_err("%s: fault reading funcdata for stub %lx for %s\n", __func__, addr, mod->name);
- return -EFAULT;
- }
-
- *target = stub_func_addr(funcdata);
-
- return 0;
-}
-#endif
-
/* Count how many different 24-bit relocations (different symbol,
different addend) */
static unsigned int count_relocs(const Elf64_Rela *rela, unsigned int num)
@@ -745,6 +709,39 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
}
#ifdef CONFIG_DYNAMIC_FTRACE
+int module_trampoline_target(struct module *mod, unsigned long addr,
+ unsigned long *target)
+{
+ struct ppc64_stub_entry *stub;
+ func_desc_t funcdata;
+ u32 magic;
+
+ if (!within_module_core(addr, mod)) {
+ pr_err("%s: stub %lx not in module %s\n", __func__, addr, mod->name);
+ return -EFAULT;
+ }
+
+ stub = (struct ppc64_stub_entry *)addr;
+
+ if (probe_kernel_read(&magic, &stub->magic, sizeof(magic))) {
+ pr_err("%s: fault reading magic for stub %lx for %s\n", __func__, addr, mod->name);
+ return -EFAULT;
+ }
+
+ if (magic != STUB_MAGIC) {
+ pr_err("%s: bad magic for stub %lx for %s\n", __func__, addr, mod->name);
+ return -EFAULT;
+ }
+
+ if (probe_kernel_read(&funcdata, &stub->funcdata, sizeof(funcdata))) {
+ pr_err("%s: fault reading funcdata for stub %lx for %s\n", __func__, addr, mod->name);
+ return -EFAULT;
+ }
+
+ *target = stub_func_addr(funcdata);
+
+ return 0;
+}
#ifdef CONFIG_MPROFILE_KERNEL
--
2.25.1
^ permalink raw reply related
* [PATCH 2/3] powerpc/module_64: Simplify check for -mprofile-kernel ftrace relocations
From: Naveen N. Rao @ 2020-04-21 17:35 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Qian Cai, Steven Rostedt
In-Reply-To: <cover.1587488954.git.naveen.n.rao@linux.vnet.ibm.com>
For -mprofile-kernel, we need special handling when generating stubs for
ftrace calls such as _mcount(). To faciliate this, we check if a
R_PPC64_REL24 relocation is for a symbol named "_mcount()" along with
also checking the instruction sequence. The latter is not really
required since "_mcount()" is an exported symbol and kernel modules
cannot use it. As such, drop the additional checking and simplify the
code. This helps unify stub creation for ftrace stubs with
-mprofile-kernel and aids in code reuse.
Also rename is_mprofile_mcount_callsite() to is_mprofile_ftrace_call()
to reflect the checking being done.
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
arch/powerpc/kernel/module_64.c | 18 ++++--------------
1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 54d9d830f4a4..7f4cc5346387 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -414,19 +414,9 @@ static unsigned long stub_for_addr(const Elf64_Shdr *sechdrs,
}
#ifdef CONFIG_MPROFILE_KERNEL
-static bool is_mprofile_mcount_callsite(const char *name, u32 *instruction)
+static bool is_mprofile_ftrace_call(const char *name)
{
- if (strcmp("_mcount", name))
- return false;
-
- /*
- * Check if this is one of the -mprofile-kernel sequences.
- */
- if (instruction[-1] == PPC_INST_STD_LR &&
- instruction[-2] == PPC_INST_MFLR)
- return true;
-
- if (instruction[-1] == PPC_INST_MFLR)
+ if (!strcmp("_mcount", name))
return true;
return false;
@@ -450,7 +440,7 @@ static void squash_toc_save_inst(const char *name, unsigned long addr)
#else
static void squash_toc_save_inst(const char *name, unsigned long addr) { }
-static bool is_mprofile_mcount_callsite(const char *name, u32 *instruction)
+static bool is_mprofile_ftrace_call(const char *name)
{
return false;
}
@@ -462,7 +452,7 @@ static int restore_r2(const char *name, u32 *instruction, struct module *me)
{
u32 *prev_insn = instruction - 1;
- if (is_mprofile_mcount_callsite(name, prev_insn))
+ if (is_mprofile_ftrace_call(name))
return 1;
/*
--
2.25.1
^ permalink raw reply related
* [PATCH 3/3] powerpc/module_64: Use special stub for _mcount() with -mprofile-kernel
From: Naveen N. Rao @ 2020-04-21 17:35 UTC (permalink / raw)
To: linuxppc-dev; +Cc: Qian Cai, Steven Rostedt
In-Reply-To: <cover.1587488954.git.naveen.n.rao@linux.vnet.ibm.com>
Since commit c55d7b5e64265f ("powerpc: Remove STRICT_KERNEL_RWX
incompatibility with RELOCATABLE"), powerpc kernels with
-mprofile-kernel can crash in certain scenarios with a trace like below:
BUG: Unable to handle kernel instruction fetch (NULL pointer?)
Faulting instruction address: 0x00000000
Oops: Kernel access of bad area, sig: 11 [#1]
LE PAGE_SIZE=64K MMU=Radix SMP NR_CPUS=256 DEBUG_PAGEALLOC NUMA PowerNV
<snip>
NIP [0000000000000000] 0x0
LR [c0080000102c0048] ext4_iomap_end+0x8/0x30 [ext4]
Call Trace:
iomap_apply+0x20c/0x920 (unreliable)
iomap_bmap+0xfc/0x160
ext4_bmap+0xa4/0x180 [ext4]
bmap+0x4c/0x80
jbd2_journal_init_inode+0x44/0x1a0 [jbd2]
ext4_load_journal+0x440/0x860 [ext4]
ext4_fill_super+0x342c/0x3ab0 [ext4]
mount_bdev+0x25c/0x290
ext4_mount+0x28/0x50 [ext4]
legacy_get_tree+0x4c/0xb0
vfs_get_tree+0x4c/0x130
do_mount+0xa18/0xc50
sys_mount+0x158/0x180
system_call+0x5c/0x68
The NIP points to NULL, or a random location (data even), while the LR
always points to the LEP of a function (with an offset of 8), indicating
that something went wrong with ftrace. However, ftrace is not
necessarily active when such crashes occur.
The kernel OOPS sometimes follows a warning from ftrace indicating that
some module functions could not be patched with a nop. Other times, if a
module is loaded early during boot, instruction patching can fail due to
a separate bug, but the error is not reported due to missing error
reporting.
In all the above cases when instruction patching fails, ftrace will be
disabled but certain kernel module functions will be left with default
calls to _mcount(). This is not a problem with ELFv1. However, with
-mprofile-kernel, the default stub is problematic since it depends on a
valid module TOC in r2. If the kernel (or a different module) calls into
a function that does not use the TOC, the function won't have a prologue
to setup the module TOC. When that function calls into _mcount(), we
will end up in the relocation stub that will use the previous TOC, and
end up trying to jump into a random location. From the above trace:
iomap_apply+0x20c/0x920 [kernel TOC]
|
V
ext4_iomap_end+0x8/0x30 [no GEP == kernel TOC]
|
V
_mcount() stub
[uses kernel TOC -> random entry]
To address this, let's change over to using the special stub that is
used for ftrace_[regs_]caller() for _mcount(). This ensures that we are
not dependent on a valid module TOC in r2 for default _mcount()
handling.
Reported-by: Qian Cai <cai@lca.pw>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
arch/powerpc/kernel/module_64.c | 222 +++++++++++++++-----------------
1 file changed, 104 insertions(+), 118 deletions(-)
diff --git a/arch/powerpc/kernel/module_64.c b/arch/powerpc/kernel/module_64.c
index 7f4cc5346387..cc7e990e8376 100644
--- a/arch/powerpc/kernel/module_64.c
+++ b/arch/powerpc/kernel/module_64.c
@@ -348,6 +348,92 @@ int module_frob_arch_sections(Elf64_Ehdr *hdr,
return 0;
}
+#ifdef CONFIG_MPROFILE_KERNEL
+
+#define PACATOC offsetof(struct paca_struct, kernel_toc)
+
+/*
+ * ld r12,PACATOC(r13)
+ * addis r12,r12,<high>
+ * addi r12,r12,<low>
+ * mtctr r12
+ * bctr
+ */
+static u32 stub_insns[] = {
+ PPC_INST_LD | __PPC_RT(R12) | __PPC_RA(R13) | PACATOC,
+ PPC_INST_ADDIS | __PPC_RT(R12) | __PPC_RA(R12),
+ PPC_INST_ADDI | __PPC_RT(R12) | __PPC_RA(R12),
+ PPC_INST_MTCTR | __PPC_RS(R12),
+ PPC_INST_BCTR,
+};
+
+/*
+ * For mprofile-kernel we use a special stub for ftrace_caller() because we
+ * can't rely on r2 containing this module's TOC when we enter the stub.
+ *
+ * That can happen if the function calling us didn't need to use the toc. In
+ * that case it won't have setup r2, and the r2 value will be either the
+ * kernel's toc, or possibly another modules toc.
+ *
+ * To deal with that this stub uses the kernel toc, which is always accessible
+ * via the paca (in r13). The target (ftrace_caller()) is responsible for
+ * saving and restoring the toc before returning.
+ */
+static inline int create_ftrace_stub(struct ppc64_stub_entry *entry,
+ unsigned long addr,
+ struct module *me)
+{
+ long reladdr;
+
+ memcpy(entry->jump, stub_insns, sizeof(stub_insns));
+
+ /* Stub uses address relative to kernel toc (from the paca) */
+ reladdr = addr - kernel_toc_addr();
+ if (reladdr > 0x7FFFFFFF || reladdr < -(0x80000000L)) {
+ pr_err("%s: Address of %ps out of range of kernel_toc.\n",
+ me->name, (void *)addr);
+ return 0;
+ }
+
+ entry->jump[1] |= PPC_HA(reladdr);
+ entry->jump[2] |= PPC_LO(reladdr);
+
+ /* Eventhough we don't use funcdata in the stub, it's needed elsewhere. */
+ entry->funcdata = func_desc(addr);
+ entry->magic = STUB_MAGIC;
+
+ return 1;
+}
+
+static bool is_mprofile_ftrace_call(const char *name)
+{
+ if (!strcmp("_mcount", name))
+ return true;
+#ifdef CONFIG_DYNAMIC_FTRACE
+ if (!strcmp("ftrace_caller", name))
+ return true;
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+ if (!strcmp("ftrace_regs_caller", name))
+ return true;
+#endif
+#endif
+
+ return false;
+}
+#else
+static inline int create_ftrace_stub(struct ppc64_stub_entry *entry,
+ unsigned long addr,
+ struct module *me)
+{
+ return 0;
+}
+
+static bool is_mprofile_ftrace_call(const char *name)
+{
+ return false;
+}
+#endif
+
/*
* r2 is the TOC pointer: it actually points 0x8000 into the TOC (this gives the
* value maximum span in an instruction which uses a signed offset). Round down
@@ -363,10 +449,14 @@ static inline unsigned long my_r2(const Elf64_Shdr *sechdrs, struct module *me)
static inline int create_stub(const Elf64_Shdr *sechdrs,
struct ppc64_stub_entry *entry,
unsigned long addr,
- struct module *me)
+ struct module *me,
+ const char *name)
{
long reladdr;
+ if (is_mprofile_ftrace_call(name))
+ return create_ftrace_stub(entry, addr, me);
+
memcpy(entry->jump, ppc64_stub_insns, sizeof(ppc64_stub_insns));
/* Stub uses address relative to r2. */
@@ -390,7 +480,8 @@ static inline int create_stub(const Elf64_Shdr *sechdrs,
stub to set up the TOC ptr (r2) for the function. */
static unsigned long stub_for_addr(const Elf64_Shdr *sechdrs,
unsigned long addr,
- struct module *me)
+ struct module *me,
+ const char *name)
{
struct ppc64_stub_entry *stubs;
unsigned int i, num_stubs;
@@ -407,45 +498,12 @@ static unsigned long stub_for_addr(const Elf64_Shdr *sechdrs,
return (unsigned long)&stubs[i];
}
- if (!create_stub(sechdrs, &stubs[i], addr, me))
+ if (!create_stub(sechdrs, &stubs[i], addr, me, name))
return 0;
return (unsigned long)&stubs[i];
}
-#ifdef CONFIG_MPROFILE_KERNEL
-static bool is_mprofile_ftrace_call(const char *name)
-{
- if (!strcmp("_mcount", name))
- return true;
-
- return false;
-}
-
-/*
- * In case of _mcount calls, do not save the current callee's TOC (in r2) into
- * the original caller's stack frame. If we did we would clobber the saved TOC
- * value of the original caller.
- */
-static void squash_toc_save_inst(const char *name, unsigned long addr)
-{
- struct ppc64_stub_entry *stub = (struct ppc64_stub_entry *)addr;
-
- /* Only for calls to _mcount */
- if (strcmp("_mcount", name) != 0)
- return;
-
- stub->jump[2] = PPC_INST_NOP;
-}
-#else
-static void squash_toc_save_inst(const char *name, unsigned long addr) { }
-
-static bool is_mprofile_ftrace_call(const char *name)
-{
- return false;
-}
-#endif
-
/* We expect a noop next: if it is, replace it with instruction to
restore r2. */
static int restore_r2(const char *name, u32 *instruction, struct module *me)
@@ -590,14 +648,13 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
if (sym->st_shndx == SHN_UNDEF ||
sym->st_shndx == SHN_LIVEPATCH) {
/* External: go via stub */
- value = stub_for_addr(sechdrs, value, me);
+ value = stub_for_addr(sechdrs, value, me,
+ strtab + sym->st_name);
if (!value)
return -ENOENT;
if (!restore_r2(strtab + sym->st_name,
(u32 *)location + 1, me))
return -ENOEXEC;
-
- squash_toc_save_inst(strtab + sym->st_name, value);
} else
value += local_entry_offset(sym);
@@ -733,88 +790,17 @@ int module_trampoline_target(struct module *mod, unsigned long addr,
return 0;
}
-#ifdef CONFIG_MPROFILE_KERNEL
-
-#define PACATOC offsetof(struct paca_struct, kernel_toc)
-
-/*
- * For mprofile-kernel we use a special stub for ftrace_caller() because we
- * can't rely on r2 containing this module's TOC when we enter the stub.
- *
- * That can happen if the function calling us didn't need to use the toc. In
- * that case it won't have setup r2, and the r2 value will be either the
- * kernel's toc, or possibly another modules toc.
- *
- * To deal with that this stub uses the kernel toc, which is always accessible
- * via the paca (in r13). The target (ftrace_caller()) is responsible for
- * saving and restoring the toc before returning.
- */
-static unsigned long create_ftrace_stub(const Elf64_Shdr *sechdrs,
- struct module *me, unsigned long addr)
-{
- struct ppc64_stub_entry *entry;
- unsigned int i, num_stubs;
- /*
- * ld r12,PACATOC(r13)
- * addis r12,r12,<high>
- * addi r12,r12,<low>
- * mtctr r12
- * bctr
- */
- static u32 stub_insns[] = {
- PPC_INST_LD | __PPC_RT(R12) | __PPC_RA(R13) | PACATOC,
- PPC_INST_ADDIS | __PPC_RT(R12) | __PPC_RA(R12),
- PPC_INST_ADDI | __PPC_RT(R12) | __PPC_RA(R12),
- PPC_INST_MTCTR | __PPC_RS(R12),
- PPC_INST_BCTR,
- };
- long reladdr;
-
- num_stubs = sechdrs[me->arch.stubs_section].sh_size / sizeof(*entry);
-
- /* Find the next available stub entry */
- entry = (void *)sechdrs[me->arch.stubs_section].sh_addr;
- for (i = 0; i < num_stubs && stub_func_addr(entry->funcdata); i++, entry++);
-
- if (i >= num_stubs) {
- pr_err("%s: Unable to find a free slot for ftrace stub.\n", me->name);
- return 0;
- }
-
- memcpy(entry->jump, stub_insns, sizeof(stub_insns));
-
- /* Stub uses address relative to kernel toc (from the paca) */
- reladdr = addr - kernel_toc_addr();
- if (reladdr > 0x7FFFFFFF || reladdr < -(0x80000000L)) {
- pr_err("%s: Address of %ps out of range of kernel_toc.\n",
- me->name, (void *)addr);
- return 0;
- }
-
- entry->jump[1] |= PPC_HA(reladdr);
- entry->jump[2] |= PPC_LO(reladdr);
-
- /* Eventhough we don't use funcdata in the stub, it's needed elsewhere. */
- entry->funcdata = func_desc(addr);
- entry->magic = STUB_MAGIC;
-
- return (unsigned long)entry;
-}
-#else
-static unsigned long create_ftrace_stub(const Elf64_Shdr *sechdrs,
- struct module *me, unsigned long addr)
-{
- return stub_for_addr(sechdrs, addr, me);
-}
-#endif
-
int module_finalize_ftrace(struct module *mod, const Elf_Shdr *sechdrs)
{
- mod->arch.tramp = create_ftrace_stub(sechdrs, mod,
- (unsigned long)ftrace_caller);
+ mod->arch.tramp = stub_for_addr(sechdrs,
+ (unsigned long)ftrace_caller,
+ mod,
+ "ftrace_caller");
#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
- mod->arch.tramp_regs = create_ftrace_stub(sechdrs, mod,
- (unsigned long)ftrace_regs_caller);
+ mod->arch.tramp_regs = stub_for_addr(sechdrs,
+ (unsigned long)ftrace_regs_caller,
+ mod,
+ "ftrace_regs_caller");
if (!mod->arch.tramp_regs)
return -ENOENT;
#endif
--
2.25.1
^ permalink raw reply related
* Re: [PATCH] KVM: PPC: Book3S HV: read ibm,secure-memory nodes
From: Laurent Dufour @ 2020-04-21 17:39 UTC (permalink / raw)
To: Oliver O'Halloran, Michael Ellerman
Cc: Alexey Kardashevskiy, Paul Mackerras, linuxppc-dev,
Linux Kernel Mailing List, kvm-ppc
In-Reply-To: <CAOSf1CF2YEG1U_1XP_Vvk3Bn1RCiNa1DAKEbemWu00JimoPsUQ@mail.gmail.com>
Le 21/04/2020 à 15:43, Oliver O'Halloran a écrit :
> On Tue, Apr 21, 2020 at 11:37 PM Michael Ellerman <mpe@ellerman.id.au> wrote:
>>
>> Hi Laurent,
>>
>> Laurent Dufour <ldufour@linux.ibm.com> writes:
>>> The newly introduced ibm,secure-memory nodes supersede the
>>> ibm,uv-firmware's property secure-memory-ranges.
>>
>> Is either documented in a device tree binding document anywhere?
>>
>> cheers
>>
>>> Firmware will no more expose the secure-memory-ranges property so first
>>> read the new one and if not found rollback to the older one.
>
> There's some in Ryan's UV support series for skiboot:
>
> https://patchwork.ozlabs.org/project/skiboot/patch/20200227204023.22125-2-grimm@linux.ibm.com/
>
> ...which is also marked RFC. Cool.
Thanks Oliver for this pointer.
Yes this is an RFC but this documentation details the secure memory nodes
created by skiboot and parsed by this patch.
Michael, is that enough for you?
Laurent.
^ permalink raw reply
* Re: [PATCH 1/2] powerpc: Add base support for ISA v3.1
From: Segher Boessenkool @ 2020-04-21 17:50 UTC (permalink / raw)
To: Oliver O'Halloran
Cc: Alistair Popple, Michael Neuling, linuxppc-dev, Nicholas Piggin
In-Reply-To: <CAOSf1CGP02VW9adNyBcHF4_BZiv-jKohH4qihj7BmEgAif0VLA@mail.gmail.com>
Hi,
On Tue, Apr 21, 2020 at 11:58:31AM +1000, Oliver O'Halloran wrote:
> On Tue, Apr 21, 2020 at 11:53 AM Alistair Popple <alistair@popple.id.au> wrote:
> >
> > On Tuesday, 21 April 2020 11:30:52 AM AEST Alistair Popple wrote:
> > > On Saturday, 4 April 2020 2:32:08 AM AEST Segher Boessenkool wrote:
> > > > Hi!
> > > >
> > > > On Fri, Apr 03, 2020 at 03:10:54PM +1100, Alistair Popple wrote:
> > > > > +#define PCR_ARCH_300 0x10 /* Architecture 3.00 */
> > > >
> > > > It's called 3.0, not 3.00?
> > >
> > > Thanks. I'll fix that up.
> >
> > Actually we have already defined it upstream as CPU_FTR_ARCH_300 in arch/
> > powerpc/include/asm/cputable.h and PVR_ARCH_300 in arch/powerpc/include/asm/
> > reg.h so for consistency we should make it the same here. So either we leave
> > this patch as is or we change it to PCR_ARCH_30 along with the existing
> > upstream #defines. Thoughts?
>
> I used 300 for consistency with the existing three digit definitions
> when I added the CPU_FTR macro. It doesn't really matter since these
> are all internal definitions, but I SAY THE BIKESHED SHOULD BE THREE
> DIGITS LONG.
You can do in your own symbols whatever you want, but the comment is
refering to an existing real-world version of the ISA, namely, 3.0 :-)
(It's important to get names right, and especially in the canonical
places, like this one. IMO of course).
Segher
^ permalink raw reply
* Re: [PATCH v8 0/7] ASoC: Add new module driver for new ASRC
From: Mark Brown @ 2020-04-21 18:22 UTC (permalink / raw)
To: devicetree, Xiubo.Lee, festevam, perex, nicoleotsuka, alsa-devel,
Shengjiu Wang, timur, robh+dt, mark.rutland, lgirdwood, tiwai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1586845137.git.shengjiu.wang@nxp.com>
On Tue, 14 Apr 2020 14:56:00 +0800, Shengjiu Wang wrote:
> Add new module driver for new ASRC in i.MX8MN, several commits
> are added for new property fsl,asrc-format
>
> Shengjiu Wang (7):
> ASoC: fsl_asrc: rename asrc_priv to asrc
> ASoC: dt-bindings: fsl_asrc: Add new property fsl,asrc-format
> ASoC: fsl-asoc-card: Support new property fsl,asrc-format
> ASoC: fsl_asrc: Support new property fsl,asrc-format
> ASoC: fsl_asrc: Move common definition to fsl_asrc_common
> ASoC: dt-bindings: fsl_easrc: Add document for EASRC
> ASoC: fsl_easrc: Add EASRC ASoC CPU DAI drivers
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.8
Thanks!
[1/7] ASoC: fsl_asrc: rename asrc_priv to asrc
commit: 7470704d8b425c4c7045884690f92cf015563aac
[2/7] ASoC: dt-bindings: fsl_asrc: Add new property fsl, asrc-format
commit: b84b4c9a688d803b0e7cf91fec9a5d8b3ba47768
[3/7] ASoC: fsl-asoc-card: Support new property fsl, asrc-format
commit: 859e364302c510cfdd9abda13a3c4c1d1bc68c57
[4/7] ASoC: fsl_asrc: Support new property fsl,asrc-format
commit: 4520af41fd21863d026d53c7e1eb987509cb3c24
[5/7] ASoC: fsl_asrc: Move common definition to fsl_asrc_common
commit: be7bd03f0201b5a513ced98c08444a140eab92ea
[6/7] ASoC: dt-bindings: fsl_easrc: Add document for EASRC
commit: a960de4da241d409a73e318ab19e6b5fdcd95a83
[7/7] ASoC: fsl_easrc: Add EASRC ASoC CPU DAI drivers
commit: 955ac624058f91172b3b8820280556e699e1e0ff
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
^ permalink raw reply
* Re: [PATCH v9 0/7] ASoC: Add new module driver for new ASRC
From: Mark Brown @ 2020-04-21 18:22 UTC (permalink / raw)
To: Xiubo.Lee, devicetree, nicoleotsuka, perex, lgirdwood, alsa-devel,
Shengjiu Wang, timur, robh+dt, mark.rutland, festevam, tiwai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <cover.1587038908.git.shengjiu.wang@nxp.com>
On Thu, 16 Apr 2020 20:25:30 +0800, Shengjiu Wang wrote:
> Add new module driver for new ASRC in i.MX8MN, several commits
> are added for new property fsl,asrc-format
>
> Shengjiu Wang (7):
> ASoC: fsl_asrc: rename asrc_priv to asrc
> ASoC: dt-bindings: fsl_asrc: Add new property fsl,asrc-format
> ASoC: fsl-asoc-card: Support new property fsl,asrc-format
> ASoC: fsl_asrc: Support new property fsl,asrc-format
> ASoC: fsl_asrc: Move common definition to fsl_asrc_common
> ASoC: dt-bindings: fsl_easrc: Add document for EASRC
> ASoC: fsl_easrc: Add EASRC ASoC CPU DAI drivers
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.8
Thanks!
[1/7] ASoC: fsl_asrc: rename asrc_priv to asrc
commit: 7470704d8b425c4c7045884690f92cf015563aac
[2/7] ASoC: dt-bindings: fsl_asrc: Add new property fsl, asrc-format
commit: b84b4c9a688d803b0e7cf91fec9a5d8b3ba47768
[3/7] ASoC: fsl-asoc-card: Support new property fsl, asrc-format
commit: 859e364302c510cfdd9abda13a3c4c1d1bc68c57
[4/7] ASoC: fsl_asrc: Support new property fsl,asrc-format
commit: 4520af41fd21863d026d53c7e1eb987509cb3c24
[5/7] ASoC: fsl_asrc: Move common definition to fsl_asrc_common
commit: be7bd03f0201b5a513ced98c08444a140eab92ea
[6/7] ASoC: dt-bindings: fsl_easrc: Add document for EASRC
commit: a960de4da241d409a73e318ab19e6b5fdcd95a83
[7/7] ASoC: fsl_easrc: Add EASRC ASoC CPU DAI drivers
commit: 955ac624058f91172b3b8820280556e699e1e0ff
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
^ permalink raw reply
* Re: [PATCH v2 5/5] uaccess: Rename user_access_begin/end() to user_full_access_begin/end()
From: Linus Torvalds @ 2020-04-21 18:34 UTC (permalink / raw)
To: Al Viro
Cc: linux-arch, Kees Cook, Dave Airlie, intel-gfx, Peter Anvin,
Linux Kernel Mailing List, Russell King, Linux-MM, Paul Mackerras,
Daniel Vetter, Andrew Morton, linuxppc-dev
In-Reply-To: <20200421024919.GA23230@ZenIV.linux.org.uk>
On Mon, Apr 20, 2020 at 7:49 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> The only source I'd been able to find speaks of >= 60 cycles
> (and possibly much more) for non-pipelined coprocessor instructions;
> the list of such does contain loads and stores to a bunch of registers.
> However, the register in question (p15/c3) has only store mentioned there,
> so loads might be cheap; no obvious reasons for those to be slow.
> That's a question to arm folks, I'm afraid... rmk?
_If_ it turns out to be expensive, is there any reason we couldn't
just cache the value in general?
That's what x86 tends to do with expensive system registers. One
example would be "msr_misc_features_shadow".
But maybe that's something to worry about when/if it turns out to
actually be a problem?
Linus
^ permalink raw reply
* Re: [PATCH 1/7] powerpc/spufs: simplify spufs core dumping
From: Al Viro @ 2020-04-21 18:49 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Arnd Bergmann, linux-kernel, Jeremy Kerr, linux-fsdevel,
Andrew Morton, linuxppc-dev, Eric W . Biederman
In-Reply-To: <20200421154204.252921-2-hch@lst.de>
On Tue, Apr 21, 2020 at 05:41:58PM +0200, Christoph Hellwig wrote:
> static ssize_t spufs_proxydma_info_read(struct file *file, char __user *buf,
> size_t len, loff_t *pos)
> {
> struct spu_context *ctx = file->private_data;
> + struct spu_proxydma_info info;
> int ret;
>
> + if (len < sizeof(info))
> + return -EINVAL;
> + if (!access_ok(buf, len))
> + return -EFAULT;
> +
> ret = spu_acquire_saved(ctx);
> if (ret)
> return ret;
> spin_lock(&ctx->csa.register_lock);
> - ret = __spufs_proxydma_info_read(ctx, buf, len, pos);
> + __spufs_proxydma_info_read(ctx, &info);
> + ret = simple_read_from_buffer(buf, len, pos, &info, sizeof(info));
IDGI... What's that access_ok() for? If you are using simple_read_from_buffer(),
the damn thing goes through copy_to_user(). Why bother with separate access_ok()
here?
^ permalink raw reply
* Re: [PATCH 1/7] powerpc/spufs: simplify spufs core dumping
From: Christoph Hellwig @ 2020-04-21 19:01 UTC (permalink / raw)
To: Al Viro
Cc: Arnd Bergmann, linux-kernel, Eric W . Biederman, linux-fsdevel,
Andrew Morton, linuxppc-dev, Christoph Hellwig, Jeremy Kerr
In-Reply-To: <20200421184941.GD23230@ZenIV.linux.org.uk>
On Tue, Apr 21, 2020 at 07:49:41PM +0100, Al Viro wrote:
> > spin_lock(&ctx->csa.register_lock);
> > - ret = __spufs_proxydma_info_read(ctx, buf, len, pos);
> > + __spufs_proxydma_info_read(ctx, &info);
> > + ret = simple_read_from_buffer(buf, len, pos, &info, sizeof(info));
>
> IDGI... What's that access_ok() for? If you are using simple_read_from_buffer(),
> the damn thing goes through copy_to_user(). Why bother with separate access_ok()
> here?
I have no idea at all, this just refactors the code.
^ permalink raw reply
* Re: [PATCH 1/7] powerpc/spufs: simplify spufs core dumping
From: Al Viro @ 2020-04-21 19:19 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Arnd Bergmann, linux-kernel, Jeremy Kerr, linux-fsdevel,
Andrew Morton, linuxppc-dev, Eric W . Biederman
In-Reply-To: <20200421190148.GA26071@lst.de>
On Tue, Apr 21, 2020 at 09:01:48PM +0200, Christoph Hellwig wrote:
> On Tue, Apr 21, 2020 at 07:49:41PM +0100, Al Viro wrote:
> > > spin_lock(&ctx->csa.register_lock);
> > > - ret = __spufs_proxydma_info_read(ctx, buf, len, pos);
> > > + __spufs_proxydma_info_read(ctx, &info);
> > > + ret = simple_read_from_buffer(buf, len, pos, &info, sizeof(info));
> >
> > IDGI... What's that access_ok() for? If you are using simple_read_from_buffer(),
> > the damn thing goes through copy_to_user(). Why bother with separate access_ok()
> > here?
>
> I have no idea at all, this just refactors the code.
Wait a bloody minute, it's doing *what* under a spinlock?
^ permalink raw reply
* Re: [PATCH 1/7] powerpc/spufs: simplify spufs core dumping
From: Al Viro @ 2020-04-21 19:25 UTC (permalink / raw)
To: Christoph Hellwig
Cc: Arnd Bergmann, linux-kernel, Jeremy Kerr, linux-fsdevel,
Andrew Morton, linuxppc-dev, Eric W . Biederman
In-Reply-To: <20200421191909.GF23230@ZenIV.linux.org.uk>
On Tue, Apr 21, 2020 at 08:19:09PM +0100, Al Viro wrote:
> On Tue, Apr 21, 2020 at 09:01:48PM +0200, Christoph Hellwig wrote:
> > On Tue, Apr 21, 2020 at 07:49:41PM +0100, Al Viro wrote:
> > > > spin_lock(&ctx->csa.register_lock);
> > > > - ret = __spufs_proxydma_info_read(ctx, buf, len, pos);
> > > > + __spufs_proxydma_info_read(ctx, &info);
> > > > + ret = simple_read_from_buffer(buf, len, pos, &info, sizeof(info));
> > >
> > > IDGI... What's that access_ok() for? If you are using simple_read_from_buffer(),
> > > the damn thing goes through copy_to_user(). Why bother with separate access_ok()
> > > here?
> >
> > I have no idea at all, this just refactors the code.
>
> Wait a bloody minute, it's doing *what* under a spinlock?
... and yes, I realize that it's been broken the same way. It still needs fixing.
AFAICS, that got broken in commit bf1ab978be23 "[POWERPC] coredump: Add SPU elf
notes to coredump." when spufs_proxydma_info_read() had copy_to_user() (until
then done after dropping the spinlock) moved into an area where blocking is very
much *not* allowed.
^ permalink raw reply
* Re: [PATCH v2 1/2] powerpc/eeh: fix pseries_eeh_configure_bridge()
From: Nathan Lynch @ 2020-04-21 23:33 UTC (permalink / raw)
To: Sam Bobroff; +Cc: linuxppc-dev, Oliver O'Halloran
In-Reply-To: <074529df859e2aae5ee1683e567f708b65e3558d.1587361657.git.sbobroff@linux.ibm.com>
Sam Bobroff <sbobroff@linux.ibm.com> writes:
> If a device is hot unplgged during EEH recovery, it's possible for the
> RTAS call to ibm,configure-pe in pseries_eeh_configure() to return
> parameter error (-3), however negative return values are not checked
> for and this leads to an infinite loop.
>
> Fix this by correctly bailing out on negative values.
>
> Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
> ---
> arch/powerpc/platforms/pseries/eeh_pseries.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
> index 893ba3f562c4..c4ef03bec0de 100644
> --- a/arch/powerpc/platforms/pseries/eeh_pseries.c
> +++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
> @@ -605,7 +605,7 @@ static int pseries_eeh_configure_bridge(struct eeh_pe *pe)
> config_addr, BUID_HI(pe->phb->buid),
> BUID_LO(pe->phb->buid));
>
> - if (!ret)
> + if (ret <= 0)
> return ret;
Note that this returns the firmware error value (e.g. -3 parameter
error) without converting it to a Linux errno. Nothing checks the error
value of this function as best I can tell, but -EINVAL would be better
than an implicit -ESRCH here.
And while this will behave correctly, the pr_warn() at the end of
pseries_eeh_configure_bridge() hints that someone had the intention
that this code should log a message on such an error:
static int pseries_eeh_configure_bridge(struct eeh_pe *pe)
{
int config_addr;
int ret;
/* Waiting 0.2s maximum before skipping configuration */
int max_wait = 200;
/* Figure out the PE address */
config_addr = pe->config_addr;
if (pe->addr)
config_addr = pe->addr;
while (max_wait > 0) {
ret = rtas_call(ibm_configure_pe, 3, 1, NULL,
config_addr, BUID_HI(pe->phb->buid),
BUID_LO(pe->phb->buid));
if (!ret)
return ret;
/*
* If RTAS returns a delay value that's above 100ms, cut it
* down to 100ms in case firmware made a mistake. For more
* on how these delay values work see rtas_busy_delay_time
*/
if (ret > RTAS_EXTENDED_DELAY_MIN+2 &&
ret <= RTAS_EXTENDED_DELAY_MAX)
ret = RTAS_EXTENDED_DELAY_MIN+2;
max_wait -= rtas_busy_delay_time(ret);
if (max_wait < 0)
break;
rtas_busy_delay(ret);
}
pr_warn("%s: Unable to configure bridge PHB#%x-PE#%x (%d)\n",
__func__, pe->phb->global_number, pe->addr, ret);
return ret;
}
So perhaps the error path should be made to break out of the loop
instead of returning. Or is the parameter error result simply
uninteresting in this scenario?
^ permalink raw reply
* [PATCH V2 0/2] mm/thp: Rename pmd_mknotpresent() as pmd_mkinvalid()
From: Anshuman Khandual @ 2020-04-22 1:52 UTC (permalink / raw)
To: linux-mm
Cc: Peter Zijlstra, Catalin Marinas, Dave Hansen, linux-kernel,
Paul Mackerras, H. Peter Anvin, Will Deacon, x86, Russell King,
Ingo Molnar, linux-snps-arc, Anshuman Khandual, Steven Rostedt,
Borislav Petkov, Andy Lutomirski, nouveau, Thomas Gleixner,
linux-arm-kernel, Thomas Bogendoerfer, Vineet Gupta, linux-mips,
Andrew Morton, linuxppc-dev
This series renames pmd_mknotpresent() as pmd_mkinvalid(). Before that it
drops an existing pmd_mknotpresent() definition from powerpc platform which
was never required as it defines it's pmdp_invalidate() through subscribing
__HAVE_ARCH_PMDP_INVALIDATE. This does not create any functional change.
This rename was suggested by Catalin during a previous discussion while we
were trying to change the THP helpers on arm64 platform for migration.
https://patchwork.kernel.org/patch/11019637/
This series is based on v5.7-rc2.
Boot tested on arm64 and x86 platforms.
Built tested on many other platforms including the ones changed here.
Changes in V2:
- Changed pmd_mknotvalid() as pmd_mkinvalid() per Will
Changes in V1: (https://patchwork.kernel.org/project/linux-mm/list/?series=259139)
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Vineet Gupta <vgupta@synopsys.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: nouveau@lists.freedesktop.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-snps-arc@lists.infradead.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-mips@vger.kernel.org
Cc: x86@kernel.org
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Anshuman Khandual (2):
powerpc/mm: Drop platform defined pmd_mknotpresent()
mm/thp: Rename pmd_mknotpresent() as pmd_mkinvalid()
arch/arc/include/asm/hugepage.h | 2 +-
arch/arm/include/asm/pgtable-3level.h | 2 +-
arch/arm64/include/asm/pgtable.h | 2 +-
arch/mips/include/asm/pgtable.h | 2 +-
arch/powerpc/include/asm/book3s/64/pgtable.h | 4 ----
arch/x86/include/asm/pgtable.h | 2 +-
arch/x86/mm/kmmio.c | 2 +-
mm/pgtable-generic.c | 2 +-
8 files changed, 7 insertions(+), 11 deletions(-)
--
2.20.1
^ permalink raw reply
* [PATCH V2 1/2] powerpc/mm: Drop platform defined pmd_mknotpresent()
From: Anshuman Khandual @ 2020-04-22 1:52 UTC (permalink / raw)
To: linux-mm; +Cc: Anshuman Khandual, linux-kernel, Paul Mackerras, linuxppc-dev
In-Reply-To: <1587520326-10099-1-git-send-email-anshuman.khandual@arm.com>
Platform needs to define pmd_mknotpresent() for generic pmdp_invalidate()
only when __HAVE_ARCH_PMDP_INVALIDATE is not subscribed. Otherwise platform
specific pmd_mknotpresent() is not required. Hence just drop it.
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Paul Mackerras <paulus@samba.org>
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
arch/powerpc/include/asm/book3s/64/pgtable.h | 4 ----
1 file changed, 4 deletions(-)
diff --git a/arch/powerpc/include/asm/book3s/64/pgtable.h b/arch/powerpc/include/asm/book3s/64/pgtable.h
index 368b136517e0..d6438659926c 100644
--- a/arch/powerpc/include/asm/book3s/64/pgtable.h
+++ b/arch/powerpc/include/asm/book3s/64/pgtable.h
@@ -1168,10 +1168,6 @@ static inline int pmd_large(pmd_t pmd)
return !!(pmd_raw(pmd) & cpu_to_be64(_PAGE_PTE));
}
-static inline pmd_t pmd_mknotpresent(pmd_t pmd)
-{
- return __pmd(pmd_val(pmd) & ~_PAGE_PRESENT);
-}
/*
* For radix we should always find H_PAGE_HASHPTE zero. Hence
* the below will work for radix too
--
2.20.1
^ permalink raw reply related
* Re: [PATCH v2 1/2] powerpc/eeh: fix pseries_eeh_configure_bridge()
From: Sam Bobroff @ 2020-04-22 3:30 UTC (permalink / raw)
To: Nathan Lynch; +Cc: linuxppc-dev, Oliver O'Halloran
In-Reply-To: <874ktc2z3j.fsf@linux.ibm.com>
[-- Attachment #1: Type: text/plain, Size: 3225 bytes --]
On Tue, Apr 21, 2020 at 06:33:36PM -0500, Nathan Lynch wrote:
> Sam Bobroff <sbobroff@linux.ibm.com> writes:
> > If a device is hot unplgged during EEH recovery, it's possible for the
> > RTAS call to ibm,configure-pe in pseries_eeh_configure() to return
> > parameter error (-3), however negative return values are not checked
> > for and this leads to an infinite loop.
> >
> > Fix this by correctly bailing out on negative values.
> >
> > Signed-off-by: Sam Bobroff <sbobroff@linux.ibm.com>
> > ---
> > arch/powerpc/platforms/pseries/eeh_pseries.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c b/arch/powerpc/platforms/pseries/eeh_pseries.c
> > index 893ba3f562c4..c4ef03bec0de 100644
> > --- a/arch/powerpc/platforms/pseries/eeh_pseries.c
> > +++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
> > @@ -605,7 +605,7 @@ static int pseries_eeh_configure_bridge(struct eeh_pe *pe)
> > config_addr, BUID_HI(pe->phb->buid),
> > BUID_LO(pe->phb->buid));
> >
> > - if (!ret)
> > + if (ret <= 0)
> > return ret;
>
> Note that this returns the firmware error value (e.g. -3 parameter
> error) without converting it to a Linux errno. Nothing checks the error
> value of this function as best I can tell, but -EINVAL would be better
> than an implicit -ESRCH here.
Right, it's never used but I agree. I'll change it for v3.
> And while this will behave correctly, the pr_warn() at the end of
> pseries_eeh_configure_bridge() hints that someone had the intention
> that this code should log a message on such an error:
>
> static int pseries_eeh_configure_bridge(struct eeh_pe *pe)
> {
> int config_addr;
> int ret;
> /* Waiting 0.2s maximum before skipping configuration */
> int max_wait = 200;
>
> /* Figure out the PE address */
> config_addr = pe->config_addr;
> if (pe->addr)
> config_addr = pe->addr;
>
> while (max_wait > 0) {
> ret = rtas_call(ibm_configure_pe, 3, 1, NULL,
> config_addr, BUID_HI(pe->phb->buid),
> BUID_LO(pe->phb->buid));
>
> if (!ret)
> return ret;
>
> /*
> * If RTAS returns a delay value that's above 100ms, cut it
> * down to 100ms in case firmware made a mistake. For more
> * on how these delay values work see rtas_busy_delay_time
> */
> if (ret > RTAS_EXTENDED_DELAY_MIN+2 &&
> ret <= RTAS_EXTENDED_DELAY_MAX)
> ret = RTAS_EXTENDED_DELAY_MIN+2;
>
> max_wait -= rtas_busy_delay_time(ret);
>
> if (max_wait < 0)
> break;
>
> rtas_busy_delay(ret);
> }
>
> pr_warn("%s: Unable to configure bridge PHB#%x-PE#%x (%d)\n",
> __func__, pe->phb->global_number, pe->addr, ret);
> return ret;
> }
>
> So perhaps the error path should be made to break out of the loop
> instead of returning. Or is the parameter error result simply
> uninteresting in this scenario?
Sounds reasonable to me, and given that the only way I know to trigger
the error path (see the commit message) is not going to be common, I
think a message is a good idea. (And, as one of the people likely to
debug a future issue here, I'll probably appreciate it.)
Cheers,
Sam.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* linux-next: build failure after merge of the powerpc tree
From: Stephen Rothwell @ 2020-04-22 5:41 UTC (permalink / raw)
To: Michael Ellerman, PowerPC
Cc: Linux Next Mailing List, Haren Myneni, Linux Kernel Mailing List
[-- Attachment #1: Type: text/plain, Size: 1685 bytes --]
Hi all,
After merging the powerpc tree, today's linux-next build (powerpc
allyesconfig) failed like this:
In file included from <command-line>:32:
./usr/include/asm/vas-api.h:15:2: error: unknown type name '__u32'
15 | __u32 version;
| ^~~~~
./usr/include/asm/vas-api.h:16:2: error: unknown type name '__s16'
16 | __s16 vas_id; /* specific instance of vas or -1 for default */
| ^~~~~
./usr/include/asm/vas-api.h:17:2: error: unknown type name '__u16'
17 | __u16 reserved1;
| ^~~~~
./usr/include/asm/vas-api.h:18:2: error: unknown type name '__u64'
18 | __u64 flags; /* Future use */
| ^~~~~
./usr/include/asm/vas-api.h:19:2: error: unknown type name '__u64'
19 | __u64 reserved2[6];
| ^~~~~
Caused by commit
45f25a79fe50 ("powerpc/vas: Define VAS_TX_WIN_OPEN ioctl API")
uapi headers should be self contained. I have added the following patch
for today:
From: Stephen Rothwell <sfr@canb.auug.org.au>
Date: Wed, 22 Apr 2020 15:28:26 +1000
Subject: [PATCH] powerpc/vas: uapi headers should be self contained
Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
---
arch/powerpc/include/uapi/asm/vas-api.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/powerpc/include/uapi/asm/vas-api.h b/arch/powerpc/include/uapi/asm/vas-api.h
index fe95d67e3bab..ebd4b2424785 100644
--- a/arch/powerpc/include/uapi/asm/vas-api.h
+++ b/arch/powerpc/include/uapi/asm/vas-api.h
@@ -6,6 +6,8 @@
#ifndef _UAPI_MISC_VAS_H
#define _UAPI_MISC_VAS_H
+#include <linux/types.h>
+
#include <asm/ioctl.h>
#define VAS_MAGIC 'v'
--
2.25.1
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply related
* Re: [musl] Powerpc Linux 'scv' system call ABI proposal take 2
From: Nicholas Piggin @ 2020-04-22 6:18 UTC (permalink / raw)
To: Rich Felker; +Cc: libc-dev, libc-alpha, linuxppc-dev, Adhemerval Zanella, musl
In-Reply-To: <20200420172715.GC11469@brightrain.aerifal.cx>
Excerpts from Rich Felker's message of April 21, 2020 3:27 am:
> On Mon, Apr 20, 2020 at 02:31:58PM +1000, Nicholas Piggin wrote:
>> Excerpts from Rich Felker's message of April 20, 2020 2:09 pm:
>> > On Mon, Apr 20, 2020 at 12:32:21PM +1000, Nicholas Piggin wrote:
>> >> Excerpts from Rich Felker's message of April 20, 2020 11:34 am:
>> >> > On Mon, Apr 20, 2020 at 11:10:25AM +1000, Nicholas Piggin wrote:
>> >> >> Excerpts from Rich Felker's message of April 17, 2020 4:31 am:
>> >> >> > Note that because lr is clobbered we need at least once normally
>> >> >> > call-clobbered register that's not syscall clobbered to save lr in.
>> >> >> > Otherwise stack frame setup is required to spill it.
>> >> >>
>> >> >> The kernel would like to use r9-r12 for itself. We could do with fewer
>> >> >> registers, but we have some delay establishing the stack (depends on a
>> >> >> load which depends on a mfspr), and entry code tends to be quite store
>> >> >> heavy whereas on the caller side you have r1 set up (modulo stack
>> >> >> updates), and the system call is a long delay during which time the
>> >> >> store queue has significant time to drain.
>> >> >>
>> >> >> My feeling is it would be better for kernel to have these scratch
>> >> >> registers.
>> >> >
>> >> > If your new kernel syscall mechanism requires the caller to make a
>> >> > whole stack frame it otherwise doesn't need and spill registers to it,
>> >> > it becomes a lot less attractive. Some of those 90 cycles saved are
>> >> > immediately lost on the userspace side, plus you either waste icache
>> >> > at the call point or require the syscall to go through a
>> >> > userspace-side helper function that performs the spill and restore.
>> >>
>> >> You would be surprised how few cycles that takes on a high end CPU. Some
>> >> might be a couple of %. I am one for counting cycles mind you, I'm not
>> >> being flippant about it. If we can come up with something faster I'd be
>> >> up for it.
>> >
>> > If the cycle count is trivial then just do it on the kernel side.
>>
>> The cycle count for user is, because you have r1 ready. Kernel does not
>> have its stack ready, it has to mfspr rX ; ld rY,N(rX); to get stack to
>> save into.
>>
>> Which is also wasted work for a userspace.
>>
>> Now that I think about it, no stack frame is even required! lr is saved
>> into the caller's stack when its clobbered with an asm, just as when
>> it's used for a function call.
>
> No. If there is a non-clobbered register, lr can be moved to the
> non-clobbered register rather than saved to the stack. However it
> looks like (1) gcc doesn't take advantage of that possibility, but (2)
> the caller already arranged for there to be space on the stack to save
> lr, so the cost is only one store and one load, not any stack
> adjustment or other frame setup. So it's probably not a really big
> deal. However, just adding "lr" clobber to existing syscall in musl
> increased the size of a simple syscall function (getuid) from 20 bytes
> to 36 bytes.
Yeah I had a bit of a play around with musl (which is very nice code I
must say). The powerpc64 syscall asm is missing ctr clobber by the way.
Fortunately adding it doesn't change code generation for me, but it
should be fixed. glibc had the same bug at one point I think (probably
due to syscall ABI documentation not existing -- something now lives in
linux/Documentation/powerpc/syscall64-abi.rst).
Yes lr needs to be saved, I didn't see any new requirement for stack
frames, and it was often already saved, but it does hurt the small
wrapper functions.
I did look at entirely replacing sc with scv though, just as an
experiment. One day you might make sc optional! Text size impoves by
about 3kB with the proposed ABI.
Mostly seems to be the bns+ ; neg sequence. __syscall1/2/3 get
out-of-lined by the compiler in a lot of cases. Linux's bloat-o-meter
says:
add/remove: 0/5 grow/shrink: 24/260 up/down: 220/-3428 (-3208)
Function old new delta
fcntl 400 424 +24
popen 600 620 +20
times 32 40 +8
[...]
alloc_rev 816 784 -32
alloc_fwd 812 780 -32
__syscall1.constprop 32 - -32
__fdopen 504 472 -32
__expand_heap 628 592 -36
__syscall2 40 - -40
__syscall3 44 - -44
fchmodat 372 324 -48
__wake.constprop 408 360 -48
child 1116 1064 -52
checker 220 156 -64
__bin_chunk 1576 1512 -64
malloc 1940 1860 -80
__syscall3.constprop 96 - -96
__syscall1 108 - -108
Total: Before=613379, After=610171, chg -0.52%
Now if we go a step further we could preserve r0,r4-r8. That gives the
kernel r9-r12 as scratch while leaving userspace with some spare
volatile GPRs except in the uncommon syscall6 case.
static inline long __syscall0(long n)
{
register long r0 __asm__("r0") = n;
register long r3 __asm__("r3");
__asm__ __volatile__("scv 0"
: "=r"(r3)
: "r"(r0)
: "memory", "cr0", "cr1", "cr5", "cr6", "cr7", "lr", "ctr", "r9", "r10", "r11", "r12"
return r3;
}
That saves another ~400 bytes, reducing some of the register shuffling
for futex loops etc:
[...]
__pthread_cond_timedwait 964 944 -20
__expand_heap 592 572 -20
socketpair 292 268 -24
__wake.constprop 360 336 -24
malloc 1860 1828 -32
__bin_chunk 1512 1472 -40
fcntl 424 376 -48
Total: Before=610171, After=609723, chg -0.07%
As you say, the compiler doesn't do a good job of saving lr in a spare
GPR unfortunately. Saving it ourselves to eliminate the lr clobber is no
good because it's almost always already saved. At least having
non-clobbered volatile GPRs could let a future smarter compiler take
advantage.
If we go further and try to preserve r3 as well by putting the return
value in r9 or r0, we go backwards about 300 bytes. It's good for the
lock loops and complex functions, but hurts a lot of simpler functions
that have to add 'mr r3,r9' etc.
Most of the time there are saved non-volatile GPRs around anyway though,
so not sure which way to go on this. Text size savings can't be ignored
and it's pretty easy for the kernel to do (we already save r3-r8 and
zero them on exit, so we could load them instead from cache line that's
should be hot).
So I may be inclined to go this way, even if we won't see benefit now.
Thanks,
Nick
^ permalink raw reply
* Re: [musl] Powerpc Linux 'scv' system call ABI proposal take 2
From: Nicholas Piggin @ 2020-04-22 6:29 UTC (permalink / raw)
To: Rich Felker; +Cc: libc-dev, libc-alpha, linuxppc-dev, Adhemerval Zanella, musl
In-Reply-To: <1587531042.1qvc287tsc.astroid@bobo.none>
Excerpts from Nicholas Piggin's message of April 22, 2020 4:18 pm:
> If we go further and try to preserve r3 as well by putting the return
> value in r9 or r0, we go backwards about 300 bytes. It's good for the
> lock loops and complex functions, but hurts a lot of simpler functions
> that have to add 'mr r3,r9' etc.
>
> Most of the time there are saved non-volatile GPRs around anyway though,
> so not sure which way to go on this. Text size savings can't be ignored
> and it's pretty easy for the kernel to do (we already save r3-r8 and
> zero them on exit, so we could load them instead from cache line that's
> should be hot).
>
> So I may be inclined to go this way, even if we won't see benefit now.
By, "this way" I don't mean r9 or r0 return value (which is larger code),
but r3 return value with r0,r4-r8 preserved.
Thanks,
Nick
^ permalink raw reply
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