* [PATCH 08/14] test_bitmap: remove user bitmap tests
From: Christoph Hellwig @ 2020-09-03 14:22 UTC (permalink / raw)
To: Linus Torvalds, Al Viro, Michael Ellerman, x86
Cc: linux-arch, Kees Cook, linux-kernel, Luis Chamberlain,
linux-fsdevel, linuxppc-dev, Alexey Dobriyan
In-Reply-To: <20200903142242.925828-1-hch@lst.de>
We can't run the tests for userspace bitmap parsing if set_fs() doesn't
exist, and it is about to go away for x86, powerpc with other major
architectures to follow.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
lib/test_bitmap.c | 91 +++++++++++------------------------------------
1 file changed, 21 insertions(+), 70 deletions(-)
diff --git a/lib/test_bitmap.c b/lib/test_bitmap.c
index df903c53952bb9..4425a1dd4ef1c7 100644
--- a/lib/test_bitmap.c
+++ b/lib/test_bitmap.c
@@ -354,50 +354,37 @@ static const struct test_bitmap_parselist parselist_tests[] __initconst = {
};
-static void __init __test_bitmap_parselist(int is_user)
+static void __init test_bitmap_parselist(void)
{
int i;
int err;
ktime_t time;
DECLARE_BITMAP(bmap, 2048);
- char *mode = is_user ? "_user" : "";
for (i = 0; i < ARRAY_SIZE(parselist_tests); i++) {
#define ptest parselist_tests[i]
- if (is_user) {
- mm_segment_t orig_fs = get_fs();
- size_t len = strlen(ptest.in);
-
- set_fs(KERNEL_DS);
- time = ktime_get();
- err = bitmap_parselist_user((__force const char __user *)ptest.in, len,
- bmap, ptest.nbits);
- time = ktime_get() - time;
- set_fs(orig_fs);
- } else {
- time = ktime_get();
- err = bitmap_parselist(ptest.in, bmap, ptest.nbits);
- time = ktime_get() - time;
- }
+ time = ktime_get();
+ err = bitmap_parselist(ptest.in, bmap, ptest.nbits);
+ time = ktime_get() - time;
if (err != ptest.errno) {
- pr_err("parselist%s: %d: input is %s, errno is %d, expected %d\n",
- mode, i, ptest.in, err, ptest.errno);
+ pr_err("parselist: %d: input is %s, errno is %d, expected %d\n",
+ i, ptest.in, err, ptest.errno);
continue;
}
if (!err && ptest.expected
&& !__bitmap_equal(bmap, ptest.expected, ptest.nbits)) {
- pr_err("parselist%s: %d: input is %s, result is 0x%lx, expected 0x%lx\n",
- mode, i, ptest.in, bmap[0],
+ pr_err("parselist: %d: input is %s, result is 0x%lx, expected 0x%lx\n",
+ i, ptest.in, bmap[0],
*ptest.expected);
continue;
}
if (ptest.flags & PARSE_TIME)
- pr_err("parselist%s: %d: input is '%s' OK, Time: %llu\n",
- mode, i, ptest.in, time);
+ pr_err("parselist: %d: input is '%s' OK, Time: %llu\n",
+ i, ptest.in, time);
#undef ptest
}
@@ -443,75 +430,41 @@ static const struct test_bitmap_parselist parse_tests[] __initconst = {
#undef step
};
-static void __init __test_bitmap_parse(int is_user)
+static void __init test_bitmap_parse(void)
{
int i;
int err;
ktime_t time;
DECLARE_BITMAP(bmap, 2048);
- char *mode = is_user ? "_user" : "";
for (i = 0; i < ARRAY_SIZE(parse_tests); i++) {
struct test_bitmap_parselist test = parse_tests[i];
+ size_t len = test.flags & NO_LEN ? UINT_MAX : strlen(test.in);
- if (is_user) {
- size_t len = strlen(test.in);
- mm_segment_t orig_fs = get_fs();
-
- set_fs(KERNEL_DS);
- time = ktime_get();
- err = bitmap_parse_user((__force const char __user *)test.in, len,
- bmap, test.nbits);
- time = ktime_get() - time;
- set_fs(orig_fs);
- } else {
- size_t len = test.flags & NO_LEN ?
- UINT_MAX : strlen(test.in);
- time = ktime_get();
- err = bitmap_parse(test.in, len, bmap, test.nbits);
- time = ktime_get() - time;
- }
+ time = ktime_get();
+ err = bitmap_parse(test.in, len, bmap, test.nbits);
+ time = ktime_get() - time;
if (err != test.errno) {
- pr_err("parse%s: %d: input is %s, errno is %d, expected %d\n",
- mode, i, test.in, err, test.errno);
+ pr_err("parse: %d: input is %s, errno is %d, expected %d\n",
+ i, test.in, err, test.errno);
continue;
}
if (!err && test.expected
&& !__bitmap_equal(bmap, test.expected, test.nbits)) {
- pr_err("parse%s: %d: input is %s, result is 0x%lx, expected 0x%lx\n",
- mode, i, test.in, bmap[0],
+ pr_err("parse: %d: input is %s, result is 0x%lx, expected 0x%lx\n",
+ i, test.in, bmap[0],
*test.expected);
continue;
}
if (test.flags & PARSE_TIME)
- pr_err("parse%s: %d: input is '%s' OK, Time: %llu\n",
- mode, i, test.in, time);
+ pr_err("parse: %d: input is '%s' OK, Time: %llu\n",
+ i, test.in, time);
}
}
-static void __init test_bitmap_parselist(void)
-{
- __test_bitmap_parselist(0);
-}
-
-static void __init test_bitmap_parselist_user(void)
-{
- __test_bitmap_parselist(1);
-}
-
-static void __init test_bitmap_parse(void)
-{
- __test_bitmap_parse(0);
-}
-
-static void __init test_bitmap_parse_user(void)
-{
- __test_bitmap_parse(1);
-}
-
#define EXP1_IN_BITS (sizeof(exp1) * 8)
static void __init test_bitmap_arr32(void)
@@ -675,9 +628,7 @@ static void __init selftest(void)
test_replace();
test_bitmap_arr32();
test_bitmap_parse();
- test_bitmap_parse_user();
test_bitmap_parselist();
- test_bitmap_parselist_user();
test_mem_optimisations();
test_for_each_set_clump8();
test_bitmap_cut();
--
2.28.0
^ permalink raw reply related
* [PATCH 06/14] fs: don't allow splice read/write without explicit ops
From: Christoph Hellwig @ 2020-09-03 14:22 UTC (permalink / raw)
To: Linus Torvalds, Al Viro, Michael Ellerman, x86
Cc: linux-arch, Kees Cook, linux-kernel, Luis Chamberlain,
linux-fsdevel, linuxppc-dev, Alexey Dobriyan
In-Reply-To: <20200903142242.925828-1-hch@lst.de>
default_file_splice_write is the last piece of generic code that uses
set_fs to make the uaccess routines operate on kernel pointers. It
implements a "fallback loop" for splicing from files that do not actually
provide a proper splice_read method. The usual file systems and other
high bandwidth instances all provide a ->splice_read, so this just removes
support for various device drivers and procfs/debugfs files. If splice
support for any of those turns out to be important it can be added back
by switching them to the iter ops and using generic_file_splice_read.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Kees Cook <keescook@chromium.org>
---
fs/read_write.c | 2 +-
fs/splice.c | 130 +++++----------------------------------------
include/linux/fs.h | 2 -
3 files changed, 15 insertions(+), 119 deletions(-)
diff --git a/fs/read_write.c b/fs/read_write.c
index 702c4301d9eb6b..8c61f67453e3d3 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -1077,7 +1077,7 @@ ssize_t vfs_iter_write(struct file *file, struct iov_iter *iter, loff_t *ppos,
}
EXPORT_SYMBOL(vfs_iter_write);
-ssize_t vfs_readv(struct file *file, const struct iovec __user *vec,
+static ssize_t vfs_readv(struct file *file, const struct iovec __user *vec,
unsigned long vlen, loff_t *pos, rwf_t flags)
{
struct iovec iovstack[UIO_FASTIOV];
diff --git a/fs/splice.c b/fs/splice.c
index d7c8a7c4db07ff..412df7b48f9eb7 100644
--- a/fs/splice.c
+++ b/fs/splice.c
@@ -342,89 +342,6 @@ const struct pipe_buf_operations nosteal_pipe_buf_ops = {
};
EXPORT_SYMBOL(nosteal_pipe_buf_ops);
-static ssize_t kernel_readv(struct file *file, const struct kvec *vec,
- unsigned long vlen, loff_t offset)
-{
- mm_segment_t old_fs;
- loff_t pos = offset;
- ssize_t res;
-
- old_fs = get_fs();
- set_fs(KERNEL_DS);
- /* The cast to a user pointer is valid due to the set_fs() */
- res = vfs_readv(file, (const struct iovec __user *)vec, vlen, &pos, 0);
- set_fs(old_fs);
-
- return res;
-}
-
-static ssize_t default_file_splice_read(struct file *in, loff_t *ppos,
- struct pipe_inode_info *pipe, size_t len,
- unsigned int flags)
-{
- struct kvec *vec, __vec[PIPE_DEF_BUFFERS];
- struct iov_iter to;
- struct page **pages;
- unsigned int nr_pages;
- unsigned int mask;
- size_t offset, base, copied = 0;
- ssize_t res;
- int i;
-
- if (pipe_full(pipe->head, pipe->tail, pipe->max_usage))
- return -EAGAIN;
-
- /*
- * Try to keep page boundaries matching to source pagecache ones -
- * it probably won't be much help, but...
- */
- offset = *ppos & ~PAGE_MASK;
-
- iov_iter_pipe(&to, READ, pipe, len + offset);
-
- res = iov_iter_get_pages_alloc(&to, &pages, len + offset, &base);
- if (res <= 0)
- return -ENOMEM;
-
- nr_pages = DIV_ROUND_UP(res + base, PAGE_SIZE);
-
- vec = __vec;
- if (nr_pages > PIPE_DEF_BUFFERS) {
- vec = kmalloc_array(nr_pages, sizeof(struct kvec), GFP_KERNEL);
- if (unlikely(!vec)) {
- res = -ENOMEM;
- goto out;
- }
- }
-
- mask = pipe->ring_size - 1;
- pipe->bufs[to.head & mask].offset = offset;
- pipe->bufs[to.head & mask].len -= offset;
-
- for (i = 0; i < nr_pages; i++) {
- size_t this_len = min_t(size_t, len, PAGE_SIZE - offset);
- vec[i].iov_base = page_address(pages[i]) + offset;
- vec[i].iov_len = this_len;
- len -= this_len;
- offset = 0;
- }
-
- res = kernel_readv(in, vec, nr_pages, *ppos);
- if (res > 0) {
- copied = res;
- *ppos += res;
- }
-
- if (vec != __vec)
- kfree(vec);
-out:
- for (i = 0; i < nr_pages; i++)
- put_page(pages[i]);
- kvfree(pages);
- iov_iter_advance(&to, copied); /* truncates and discards */
- return res;
-}
-
/*
* Send 'sd->len' bytes to socket from 'sd->file' at position 'sd->pos'
* using sendpage(). Return the number of bytes sent.
@@ -788,33 +705,6 @@ iter_file_splice_write(struct pipe_inode_info *pipe, struct file *out,
EXPORT_SYMBOL(iter_file_splice_write);
-static int write_pipe_buf(struct pipe_inode_info *pipe, struct pipe_buffer *buf,
- struct splice_desc *sd)
-{
- int ret;
- void *data;
- loff_t tmp = sd->pos;
-
- data = kmap(buf->page);
- ret = __kernel_write(sd->u.file, data + buf->offset, sd->len, &tmp);
- kunmap(buf->page);
-
- return ret;
-}
-
-static ssize_t default_file_splice_write(struct pipe_inode_info *pipe,
- struct file *out, loff_t *ppos,
- size_t len, unsigned int flags)
-{
- ssize_t ret;
-
- ret = splice_from_pipe(pipe, out, ppos, len, flags, write_pipe_buf);
- if (ret > 0)
- *ppos += ret;
-
- return ret;
-}
-
/**
* generic_splice_sendpage - splice data from a pipe to a socket
* @pipe: pipe to splice from
@@ -836,15 +726,23 @@ ssize_t generic_splice_sendpage(struct pipe_inode_info *pipe, struct file *out,
EXPORT_SYMBOL(generic_splice_sendpage);
+static int warn_unsupported(struct file *file, const char *op)
+{
+ pr_debug_ratelimited(
+ "splice %s not supported for file %pD4 (pid: %d comm: %.20s)\n",
+ op, file, current->pid, current->comm);
+ return -EINVAL;
+}
+
/*
* Attempt to initiate a splice from pipe to file.
*/
static long do_splice_from(struct pipe_inode_info *pipe, struct file *out,
loff_t *ppos, size_t len, unsigned int flags)
{
- if (out->f_op->splice_write)
- return out->f_op->splice_write(pipe, out, ppos, len, flags);
- return default_file_splice_write(pipe, out, ppos, len, flags);
+ if (unlikely(!out->f_op->splice_write))
+ return warn_unsupported(out, "write");
+ return out->f_op->splice_write(pipe, out, ppos, len, flags);
}
/*
@@ -866,9 +764,9 @@ static long do_splice_to(struct file *in, loff_t *ppos,
if (unlikely(len > MAX_RW_COUNT))
len = MAX_RW_COUNT;
- if (in->f_op->splice_read)
- return in->f_op->splice_read(in, ppos, pipe, len, flags);
- return default_file_splice_read(in, ppos, pipe, len, flags);
+ if (unlikely(!in->f_op->splice_read))
+ return warn_unsupported(in, "read");
+ return in->f_op->splice_read(in, ppos, pipe, len, flags);
}
/**
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 7519ae003a082c..839eeccf10174b 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1894,8 +1894,6 @@ ssize_t rw_copy_check_uvector(int type, const struct iovec __user * uvector,
extern ssize_t vfs_read(struct file *, char __user *, size_t, loff_t *);
extern ssize_t vfs_write(struct file *, const char __user *, size_t, loff_t *);
-extern ssize_t vfs_readv(struct file *, const struct iovec __user *,
- unsigned long, loff_t *, rwf_t);
extern ssize_t vfs_copy_file_range(struct file *, loff_t , struct file *,
loff_t, size_t, unsigned int);
extern ssize_t generic_copy_file_range(struct file *file_in, loff_t pos_in,
--
2.28.0
^ permalink raw reply related
* [PATCH 05/14] fs: don't allow kernel reads and writes without iter ops
From: Christoph Hellwig @ 2020-09-03 14:22 UTC (permalink / raw)
To: Linus Torvalds, Al Viro, Michael Ellerman, x86
Cc: linux-arch, Kees Cook, linux-kernel, Luis Chamberlain,
linux-fsdevel, linuxppc-dev, Alexey Dobriyan
In-Reply-To: <20200903142242.925828-1-hch@lst.de>
Don't allow calling ->read or ->write with set_fs as a preparation for
killing off set_fs. All the instances that we use kernel_read/write on
are using the iter ops already.
If a file has both the regular ->read/->write methods and the iter
variants those could have different semantics for messed up enough
drivers. Also fails the kernel access to them in that case.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Kees Cook <keescook@chromium.org>
---
fs/read_write.c | 67 +++++++++++++++++++++++++++++++------------------
1 file changed, 42 insertions(+), 25 deletions(-)
diff --git a/fs/read_write.c b/fs/read_write.c
index 5db58b8c78d0dd..702c4301d9eb6b 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -419,27 +419,41 @@ static ssize_t new_sync_read(struct file *filp, char __user *buf, size_t len, lo
return ret;
}
+static int warn_unsupported(struct file *file, const char *op)
+{
+ pr_warn_ratelimited(
+ "kernel %s not supported for file %pD4 (pid: %d comm: %.20s)\n",
+ op, file, current->pid, current->comm);
+ return -EINVAL;
+}
+
ssize_t __kernel_read(struct file *file, void *buf, size_t count, loff_t *pos)
{
- mm_segment_t old_fs = get_fs();
+ struct kvec iov = {
+ .iov_base = buf,
+ .iov_len = min_t(size_t, count, MAX_RW_COUNT),
+ };
+ struct kiocb kiocb;
+ struct iov_iter iter;
ssize_t ret;
if (WARN_ON_ONCE(!(file->f_mode & FMODE_READ)))
return -EINVAL;
if (!(file->f_mode & FMODE_CAN_READ))
return -EINVAL;
+ /*
+ * Also fail if ->read_iter and ->read are both wired up as that
+ * implies very convoluted semantics.
+ */
+ if (unlikely(!file->f_op->read_iter || file->f_op->read))
+ return warn_unsupported(file, "read");
- if (count > MAX_RW_COUNT)
- count = MAX_RW_COUNT;
- set_fs(KERNEL_DS);
- if (file->f_op->read)
- ret = file->f_op->read(file, (void __user *)buf, count, pos);
- else if (file->f_op->read_iter)
- ret = new_sync_read(file, (void __user *)buf, count, pos);
- else
- ret = -EINVAL;
- set_fs(old_fs);
+ init_sync_kiocb(&kiocb, file);
+ kiocb.ki_pos = *pos;
+ iov_iter_kvec(&iter, READ, &iov, 1, iov.iov_len);
+ ret = file->f_op->read_iter(&kiocb, &iter);
if (ret > 0) {
+ *pos = kiocb.ki_pos;
fsnotify_access(file);
add_rchar(current, ret);
}
@@ -510,28 +524,31 @@ static ssize_t new_sync_write(struct file *filp, const char __user *buf, size_t
/* caller is responsible for file_start_write/file_end_write */
ssize_t __kernel_write(struct file *file, const void *buf, size_t count, loff_t *pos)
{
- mm_segment_t old_fs;
- const char __user *p;
+ struct kvec iov = {
+ .iov_base = (void *)buf,
+ .iov_len = min_t(size_t, count, MAX_RW_COUNT),
+ };
+ struct kiocb kiocb;
+ struct iov_iter iter;
ssize_t ret;
if (WARN_ON_ONCE(!(file->f_mode & FMODE_WRITE)))
return -EBADF;
if (!(file->f_mode & FMODE_CAN_WRITE))
return -EINVAL;
+ /*
+ * Also fail if ->write_iter and ->write are both wired up as that
+ * implies very convoluted semantics.
+ */
+ if (unlikely(!file->f_op->write_iter || file->f_op->write))
+ return warn_unsupported(file, "write");
- old_fs = get_fs();
- set_fs(KERNEL_DS);
- p = (__force const char __user *)buf;
- if (count > MAX_RW_COUNT)
- count = MAX_RW_COUNT;
- if (file->f_op->write)
- ret = file->f_op->write(file, p, count, pos);
- else if (file->f_op->write_iter)
- ret = new_sync_write(file, p, count, pos);
- else
- ret = -EINVAL;
- set_fs(old_fs);
+ init_sync_kiocb(&kiocb, file);
+ kiocb.ki_pos = *pos;
+ iov_iter_kvec(&iter, WRITE, &iov, 1, iov.iov_len);
+ ret = file->f_op->write_iter(&kiocb, &iter);
if (ret > 0) {
+ *pos = kiocb.ki_pos;
fsnotify_modify(file);
add_wchar(current, ret);
}
--
2.28.0
^ permalink raw reply related
* [PATCH 03/14] proc: add a read_iter method to proc proc_ops
From: Christoph Hellwig @ 2020-09-03 14:22 UTC (permalink / raw)
To: Linus Torvalds, Al Viro, Michael Ellerman, x86
Cc: linux-arch, Kees Cook, linux-kernel, Luis Chamberlain,
linux-fsdevel, linuxppc-dev, Alexey Dobriyan
In-Reply-To: <20200903142242.925828-1-hch@lst.de>
This will allow proc files to implement iter read semantics.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/proc/inode.c | 53 ++++++++++++++++++++++++++++++++++++++---
include/linux/proc_fs.h | 1 +
2 files changed, 51 insertions(+), 3 deletions(-)
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index 93dd2045737504..58c075e2a452d6 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -297,6 +297,21 @@ static loff_t proc_reg_llseek(struct file *file, loff_t offset, int whence)
return rv;
}
+static ssize_t proc_reg_read_iter(struct kiocb *iocb, struct iov_iter *iter)
+{
+ struct proc_dir_entry *pde = PDE(file_inode(iocb->ki_filp));
+ ssize_t ret;
+
+ if (pde_is_permanent(pde))
+ return pde->proc_ops->proc_read_iter(iocb, iter);
+
+ if (!use_pde(pde))
+ return -EIO;
+ ret = pde->proc_ops->proc_read_iter(iocb, iter);
+ unuse_pde(pde);
+ return ret;
+}
+
static ssize_t pde_read(struct proc_dir_entry *pde, struct file *file, char __user *buf, size_t count, loff_t *ppos)
{
typeof_member(struct proc_ops, proc_read) read;
@@ -578,6 +593,18 @@ static const struct file_operations proc_reg_file_ops = {
.release = proc_reg_release,
};
+static const struct file_operations proc_iter_file_ops = {
+ .llseek = proc_reg_llseek,
+ .read_iter = proc_reg_read_iter,
+ .write = proc_reg_write,
+ .poll = proc_reg_poll,
+ .unlocked_ioctl = proc_reg_unlocked_ioctl,
+ .mmap = proc_reg_mmap,
+ .get_unmapped_area = proc_reg_get_unmapped_area,
+ .open = proc_reg_open,
+ .release = proc_reg_release,
+};
+
#ifdef CONFIG_COMPAT
static const struct file_operations proc_reg_file_ops_compat = {
.llseek = proc_reg_llseek,
@@ -591,6 +618,19 @@ static const struct file_operations proc_reg_file_ops_compat = {
.open = proc_reg_open,
.release = proc_reg_release,
};
+
+static const struct file_operations proc_iter_file_ops_compat = {
+ .llseek = proc_reg_llseek,
+ .read_iter = proc_reg_read_iter,
+ .write = proc_reg_write,
+ .poll = proc_reg_poll,
+ .unlocked_ioctl = proc_reg_unlocked_ioctl,
+ .compat_ioctl = proc_reg_compat_ioctl,
+ .mmap = proc_reg_mmap,
+ .get_unmapped_area = proc_reg_get_unmapped_area,
+ .open = proc_reg_open,
+ .release = proc_reg_release,
+};
#endif
static void proc_put_link(void *p)
@@ -642,10 +682,17 @@ struct inode *proc_get_inode(struct super_block *sb, struct proc_dir_entry *de)
if (S_ISREG(inode->i_mode)) {
inode->i_op = de->proc_iops;
- inode->i_fop = &proc_reg_file_ops;
+ if (de->proc_ops->proc_read_iter)
+ inode->i_fop = &proc_iter_file_ops;
+ else
+ inode->i_fop = &proc_reg_file_ops;
#ifdef CONFIG_COMPAT
- if (de->proc_ops->proc_compat_ioctl)
- inode->i_fop = &proc_reg_file_ops_compat;
+ if (de->proc_ops->proc_compat_ioctl) {
+ if (de->proc_ops->proc_read_iter)
+ inode->i_fop = &proc_iter_file_ops_compat;
+ else
+ inode->i_fop = &proc_reg_file_ops_compat;
+ }
#endif
} else if (S_ISDIR(inode->i_mode)) {
inode->i_op = de->proc_iops;
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index 2df965cd09742d..270cab43ca3dad 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -30,6 +30,7 @@ struct proc_ops {
unsigned int proc_flags;
int (*proc_open)(struct inode *, struct file *);
ssize_t (*proc_read)(struct file *, char __user *, size_t, loff_t *);
+ ssize_t (*proc_read_iter)(struct kiocb *, struct iov_iter *);
ssize_t (*proc_write)(struct file *, const char __user *, size_t, loff_t *);
loff_t (*proc_lseek)(struct file *, loff_t, int);
int (*proc_release)(struct inode *, struct file *);
--
2.28.0
^ permalink raw reply related
* remove the last set_fs() in common code, and remove it for x86 and powerpc v3
From: Christoph Hellwig @ 2020-09-03 14:22 UTC (permalink / raw)
To: Linus Torvalds, Al Viro, Michael Ellerman, x86
Cc: linux-arch, Kees Cook, linux-kernel, Luis Chamberlain,
linux-fsdevel, linuxppc-dev, Alexey Dobriyan
Hi all,
this series removes the last set_fs() used to force a kernel address
space for the uaccess code in the kernel read/write/splice code, and then
stops implementing the address space overrides entirely for x86 and
powerpc.
[Note to Linus: I'd like to get this into linux-next rather earlier
than later. Do you think it is ok to add this tree to linux-next?]
The file system part has been posted a few times, and the read/write side
has been pretty much unchanced. For splice this series drops the
conversion of the seq_file and sysctl code to the iter ops, and thus loses
the splice support for them. The reasons for that is that it caused a lot
of churn for not much use - splice for these small files really isn't much
of a win, even if existing userspace uses it. All callers I found do the
proper fallback, but if this turns out to be an issue the conversion can
be resurrected.
Besides x86 and powerpc I plan to eventually convert all other
architectures, although this will be a slow process, starting with the
easier ones once the infrastructure is merged. The process to convert
architectures is roughtly:
(1) ensure there is no set_fs(KERNEL_DS) left in arch specific code
(2) implement __get_kernel_nofault and __put_kernel_nofault
(3) remove the arch specific address limitation functionality
Changes since v2:
- add back the patch to support splice through read_iter/write iter
on /proc/sys/*
- entirely remove the tests that depend on set_fs. Note that for
lkdtm the maintainer (Kees) disagrees with this request from Linus
- fix a wrong check in the powerpc access_ok, and drop a few spurious
cleanups there
Changes since v1:
- drop the patch to remove the non-iter ops for /dev/zero and
/dev/null as they caused a performance regression
- don't enable user access in __get_kernel on powerpc
- xfail the set_fs() based lkdtm tests
Diffstat:
^ permalink raw reply
* [PATCH 04/14] sysctl: Convert to iter interfaces
From: Christoph Hellwig @ 2020-09-03 14:22 UTC (permalink / raw)
To: Linus Torvalds, Al Viro, Michael Ellerman, x86
Cc: linux-arch, Kees Cook, linux-kernel, Matthew Wilcox (Oracle),
Luis Chamberlain, linux-fsdevel, linuxppc-dev, Alexey Dobriyan
In-Reply-To: <20200903142242.925828-1-hch@lst.de>
From: "Matthew Wilcox (Oracle)" <willy@infradead.org>
Using the read_iter/write_iter interfaces allows for in-kernel users
to set sysctls without using set_fs(). Also, the buffer is a string,
so give it the real type of 'char *', not void *.
Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/proc/proc_sysctl.c | 46 ++++++++++++++++++--------------------
include/linux/bpf-cgroup.h | 2 +-
kernel/bpf/cgroup.c | 2 +-
3 files changed, 24 insertions(+), 26 deletions(-)
diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 6c1166ccdaea57..a4a3122f8a584a 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -12,6 +12,7 @@
#include <linux/cred.h>
#include <linux/namei.h>
#include <linux/mm.h>
+#include <linux/uio.h>
#include <linux/module.h>
#include <linux/bpf-cgroup.h>
#include <linux/mount.h>
@@ -540,13 +541,14 @@ static struct dentry *proc_sys_lookup(struct inode *dir, struct dentry *dentry,
return err;
}
-static ssize_t proc_sys_call_handler(struct file *filp, void __user *ubuf,
- size_t count, loff_t *ppos, int write)
+static ssize_t proc_sys_call_handler(struct kiocb *iocb, struct iov_iter *iter,
+ int write)
{
- struct inode *inode = file_inode(filp);
+ struct inode *inode = file_inode(iocb->ki_filp);
struct ctl_table_header *head = grab_header(inode);
struct ctl_table *table = PROC_I(inode)->sysctl_entry;
- void *kbuf;
+ size_t count = iov_iter_count(iter);
+ char *kbuf;
ssize_t error;
if (IS_ERR(head))
@@ -569,32 +571,30 @@ static ssize_t proc_sys_call_handler(struct file *filp, void __user *ubuf,
error = -ENOMEM;
if (count >= KMALLOC_MAX_SIZE)
goto out;
+ kbuf = kzalloc(count + 1, GFP_KERNEL);
+ if (!kbuf)
+ goto out;
if (write) {
- kbuf = memdup_user_nul(ubuf, count);
- if (IS_ERR(kbuf)) {
- error = PTR_ERR(kbuf);
- goto out;
- }
- } else {
- kbuf = kzalloc(count, GFP_KERNEL);
- if (!kbuf)
- goto out;
+ error = -EFAULT;
+ if (!copy_from_iter_full(kbuf, count, iter))
+ goto out_free_buf;
+ kbuf[count] = '\0';
}
error = BPF_CGROUP_RUN_PROG_SYSCTL(head, table, write, &kbuf, &count,
- ppos);
+ &iocb->ki_pos);
if (error)
goto out_free_buf;
/* careful: calling conventions are nasty here */
- error = table->proc_handler(table, write, kbuf, &count, ppos);
+ error = table->proc_handler(table, write, kbuf, &count, &iocb->ki_pos);
if (error)
goto out_free_buf;
if (!write) {
error = -EFAULT;
- if (copy_to_user(ubuf, kbuf, count))
+ if (copy_to_iter(kbuf, count, iter) < count)
goto out_free_buf;
}
@@ -607,16 +607,14 @@ static ssize_t proc_sys_call_handler(struct file *filp, void __user *ubuf,
return error;
}
-static ssize_t proc_sys_read(struct file *filp, char __user *buf,
- size_t count, loff_t *ppos)
+static ssize_t proc_sys_read(struct kiocb *iocb, struct iov_iter *iter)
{
- return proc_sys_call_handler(filp, (void __user *)buf, count, ppos, 0);
+ return proc_sys_call_handler(iocb, iter, 0);
}
-static ssize_t proc_sys_write(struct file *filp, const char __user *buf,
- size_t count, loff_t *ppos)
+static ssize_t proc_sys_write(struct kiocb *iocb, struct iov_iter *iter)
{
- return proc_sys_call_handler(filp, (void __user *)buf, count, ppos, 1);
+ return proc_sys_call_handler(iocb, iter, 1);
}
static int proc_sys_open(struct inode *inode, struct file *filp)
@@ -853,8 +851,8 @@ static int proc_sys_getattr(const struct path *path, struct kstat *stat,
static const struct file_operations proc_sys_file_operations = {
.open = proc_sys_open,
.poll = proc_sys_poll,
- .read = proc_sys_read,
- .write = proc_sys_write,
+ .read_iter = proc_sys_read,
+ .write_iter = proc_sys_write,
.llseek = default_llseek,
};
diff --git a/include/linux/bpf-cgroup.h b/include/linux/bpf-cgroup.h
index 64f367044e25f4..82b26a1386d85e 100644
--- a/include/linux/bpf-cgroup.h
+++ b/include/linux/bpf-cgroup.h
@@ -136,7 +136,7 @@ int __cgroup_bpf_check_dev_permission(short dev_type, u32 major, u32 minor,
int __cgroup_bpf_run_filter_sysctl(struct ctl_table_header *head,
struct ctl_table *table, int write,
- void **buf, size_t *pcount, loff_t *ppos,
+ char **buf, size_t *pcount, loff_t *ppos,
enum bpf_attach_type type);
int __cgroup_bpf_run_filter_setsockopt(struct sock *sock, int *level,
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index e21de4f1754c2c..6ec088a96302f9 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -1226,7 +1226,7 @@ const struct bpf_verifier_ops cg_dev_verifier_ops = {
*/
int __cgroup_bpf_run_filter_sysctl(struct ctl_table_header *head,
struct ctl_table *table, int write,
- void **buf, size_t *pcount, loff_t *ppos,
+ char **buf, size_t *pcount, loff_t *ppos,
enum bpf_attach_type type)
{
struct bpf_sysctl_kern ctx = {
--
2.28.0
^ permalink raw reply related
* [PATCH 01/14] proc: remove a level of indentation in proc_get_inode
From: Christoph Hellwig @ 2020-09-03 14:22 UTC (permalink / raw)
To: Linus Torvalds, Al Viro, Michael Ellerman, x86
Cc: linux-arch, Kees Cook, linux-kernel, Luis Chamberlain,
linux-fsdevel, linuxppc-dev, Alexey Dobriyan
In-Reply-To: <20200903142242.925828-1-hch@lst.de>
Just return early on inode allocation failure.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/proc/inode.c | 72 +++++++++++++++++++++++++------------------------
1 file changed, 37 insertions(+), 35 deletions(-)
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index 28d6105e908e4c..016b1302cbabc0 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -619,42 +619,44 @@ struct inode *proc_get_inode(struct super_block *sb, struct proc_dir_entry *de)
{
struct inode *inode = new_inode(sb);
- if (inode) {
- inode->i_ino = de->low_ino;
- inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode);
- PROC_I(inode)->pde = de;
-
- if (is_empty_pde(de)) {
- make_empty_dir_inode(inode);
- return inode;
- }
- if (de->mode) {
- inode->i_mode = de->mode;
- inode->i_uid = de->uid;
- inode->i_gid = de->gid;
- }
- if (de->size)
- inode->i_size = de->size;
- if (de->nlink)
- set_nlink(inode, de->nlink);
-
- if (S_ISREG(inode->i_mode)) {
- inode->i_op = de->proc_iops;
- inode->i_fop = &proc_reg_file_ops;
+ if (!inode) {
+ pde_put(de);
+ return NULL;
+ }
+
+ inode->i_ino = de->low_ino;
+ inode->i_mtime = inode->i_atime = inode->i_ctime = current_time(inode);
+ PROC_I(inode)->pde = de;
+ if (is_empty_pde(de)) {
+ make_empty_dir_inode(inode);
+ return inode;
+ }
+
+ if (de->mode) {
+ inode->i_mode = de->mode;
+ inode->i_uid = de->uid;
+ inode->i_gid = de->gid;
+ }
+ if (de->size)
+ inode->i_size = de->size;
+ if (de->nlink)
+ set_nlink(inode, de->nlink);
+
+ if (S_ISREG(inode->i_mode)) {
+ inode->i_op = de->proc_iops;
+ inode->i_fop = &proc_reg_file_ops;
#ifdef CONFIG_COMPAT
- if (!de->proc_ops->proc_compat_ioctl) {
- inode->i_fop = &proc_reg_file_ops_no_compat;
- }
+ if (!de->proc_ops->proc_compat_ioctl)
+ inode->i_fop = &proc_reg_file_ops_no_compat;
#endif
- } else if (S_ISDIR(inode->i_mode)) {
- inode->i_op = de->proc_iops;
- inode->i_fop = de->proc_dir_ops;
- } else if (S_ISLNK(inode->i_mode)) {
- inode->i_op = de->proc_iops;
- inode->i_fop = NULL;
- } else
- BUG();
- } else
- pde_put(de);
+ } else if (S_ISDIR(inode->i_mode)) {
+ inode->i_op = de->proc_iops;
+ inode->i_fop = de->proc_dir_ops;
+ } else if (S_ISLNK(inode->i_mode)) {
+ inode->i_op = de->proc_iops;
+ inode->i_fop = NULL;
+ } else {
+ BUG();
+ }
return inode;
}
--
2.28.0
^ permalink raw reply related
* [PATCH 02/14] proc: cleanup the compat vs no compat file ops
From: Christoph Hellwig @ 2020-09-03 14:22 UTC (permalink / raw)
To: Linus Torvalds, Al Viro, Michael Ellerman, x86
Cc: linux-arch, Kees Cook, linux-kernel, Luis Chamberlain,
linux-fsdevel, linuxppc-dev, Alexey Dobriyan
In-Reply-To: <20200903142242.925828-1-hch@lst.de>
Instead of providing a special no-compat version provide a special
compat version for operations with ->compat_ioctl.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/proc/inode.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index 016b1302cbabc0..93dd2045737504 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -572,9 +572,6 @@ static const struct file_operations proc_reg_file_ops = {
.write = proc_reg_write,
.poll = proc_reg_poll,
.unlocked_ioctl = proc_reg_unlocked_ioctl,
-#ifdef CONFIG_COMPAT
- .compat_ioctl = proc_reg_compat_ioctl,
-#endif
.mmap = proc_reg_mmap,
.get_unmapped_area = proc_reg_get_unmapped_area,
.open = proc_reg_open,
@@ -582,12 +579,13 @@ static const struct file_operations proc_reg_file_ops = {
};
#ifdef CONFIG_COMPAT
-static const struct file_operations proc_reg_file_ops_no_compat = {
+static const struct file_operations proc_reg_file_ops_compat = {
.llseek = proc_reg_llseek,
.read = proc_reg_read,
.write = proc_reg_write,
.poll = proc_reg_poll,
.unlocked_ioctl = proc_reg_unlocked_ioctl,
+ .compat_ioctl = proc_reg_compat_ioctl,
.mmap = proc_reg_mmap,
.get_unmapped_area = proc_reg_get_unmapped_area,
.open = proc_reg_open,
@@ -646,8 +644,8 @@ struct inode *proc_get_inode(struct super_block *sb, struct proc_dir_entry *de)
inode->i_op = de->proc_iops;
inode->i_fop = &proc_reg_file_ops;
#ifdef CONFIG_COMPAT
- if (!de->proc_ops->proc_compat_ioctl)
- inode->i_fop = &proc_reg_file_ops_no_compat;
+ if (de->proc_ops->proc_compat_ioctl)
+ inode->i_fop = &proc_reg_file_ops_compat;
#endif
} else if (S_ISDIR(inode->i_mode)) {
inode->i_op = de->proc_iops;
--
2.28.0
^ permalink raw reply related
* Re: [PATCH v2] ASoC: fsl_sai: Set SAI Channel Mode to Output Mode
From: Mark Brown @ 2020-09-03 13:48 UTC (permalink / raw)
To: alsa-devel, tiwai, timur, festevam, nicoleotsuka, Xiubo.Lee,
lgirdwood, perex, Shengjiu Wang
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1599112427-22038-1-git-send-email-shengjiu.wang@nxp.com>
On Thu, 3 Sep 2020 13:53:47 +0800, Shengjiu Wang wrote:
> Transmit data pins will output zero when slots are masked or channels
> are disabled. In CHMOD TDM mode, transmit data pins are tri-stated when
> slots are masked or channels are disabled. When data pins are tri-stated,
> there is noise on some channels when FS clock value is high and data is
> read while fsclk is transitioning from high to low.
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: fsl_sai: Set SAI Channel Mode to Output Mode
commit: f4c4b1bb2f5a7f034f039c302b56f82344a6dc8c
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] ASoC: fsl_sai: Support multiple data channel enable bits
From: Mark Brown @ 2020-09-03 13:48 UTC (permalink / raw)
To: alsa-devel, tiwai, timur, festevam, nicoleotsuka, Xiubo.Lee,
lgirdwood, perex, Shengjiu Wang
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <1598958068-10552-1-git-send-email-shengjiu.wang@nxp.com>
On Tue, 1 Sep 2020 19:01:08 +0800, Shengjiu Wang wrote:
> One data channel is one data line. From imx7ulp, the SAI IP is
> enhanced to support multiple data channels.
>
> If there is only two channels input and slots is 2, then enable one
> data channel is enough for data transfer. So enable the TCE/RCE and
> transmit/receive mask register according to the input channels and
> slots configuration.
>
> [...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/1] ASoC: fsl_sai: Support multiple data channel enable bits
commit: 770f58d7d2c58b8ff31d3694ce14a785c2e75009
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] selftests/powerpc: Skip PROT_SAO test in guests/LPARS
From: Michael Ellerman @ 2020-09-03 12:27 UTC (permalink / raw)
To: linuxppc-dev, Michael Ellerman
In-Reply-To: <20200901124653.523182-1-mpe@ellerman.id.au>
On Tue, 1 Sep 2020 22:46:53 +1000, Michael Ellerman wrote:
> In commit 9b725a90a8f1 ("powerpc/64s: Disallow PROT_SAO in LPARs by
> default") PROT_SAO was disabled in guests/LPARs by default. So skip
> the test if we are running in a guest to avoid a spurious failure.
Applied to powerpc/fixes.
[1/1] selftests/powerpc: Skip PROT_SAO test in guests/LPARS
https://git.kernel.org/powerpc/c/fc1f178cdb31783ff37296ecae817a1045a1a513
cheers
^ permalink raw reply
* Re: [PATCH v3] powerpc/mm: Remove DEBUG_VM_PGTABLE support on powerpc
From: Michael Ellerman @ 2020-09-03 12:26 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev, mpe; +Cc: Anshuman Khandual
In-Reply-To: <20200902040122.136414-1-aneesh.kumar@linux.ibm.com>
On Wed, 2 Sep 2020 09:31:22 +0530, Aneesh Kumar K.V wrote:
> The test is broken w.r.t page table update rules and results in kernel
> crash as below. Disable the support until we get the tests updated.
>
> [ 21.083519] kernel BUG at arch/powerpc/mm/pgtable.c:304!
> cpu 0x0: Vector: 700 (Program Check) at [c000000c6d1e76c0]
> pc: c00000000009a5ec: assert_pte_locked+0x14c/0x380
> lr: c0000000005eeeec: pte_update+0x11c/0x190
> sp: c000000c6d1e7950
> msr: 8000000002029033
> current = 0xc000000c6d172c80
> paca = 0xc000000003ba0000 irqmask: 0x03 irq_happened: 0x01
> pid = 1, comm = swapper/0
> kernel BUG at arch/powerpc/mm/pgtable.c:304!
> [link register ] c0000000005eeeec pte_update+0x11c/0x190
> [c000000c6d1e7950] 0000000000000001 (unreliable)
> [c000000c6d1e79b0] c0000000005eee14 pte_update+0x44/0x190
> [c000000c6d1e7a10] c000000001a2ca9c pte_advanced_tests+0x160/0x3d8
> [c000000c6d1e7ab0] c000000001a2d4fc debug_vm_pgtable+0x7e8/0x1338
> [c000000c6d1e7ba0] c0000000000116ec do_one_initcall+0xac/0x5f0
> [c000000c6d1e7c80] c0000000019e4fac kernel_init_freeable+0x4dc/0x5a4
> [c000000c6d1e7db0] c000000000012474 kernel_init+0x24/0x160
> [c000000c6d1e7e20] c00000000000cbd0 ret_from_kernel_thread+0x5c/0x6c
>
> [...]
Applied to powerpc/fixes.
[1/1] powerpc/mm: Remove DEBUG_VM_PGTABLE support on powerpc
https://git.kernel.org/powerpc/c/675bceb097e6fb9769309093ec6f3d33a2fed9cc
cheers
^ permalink raw reply
* Re: [PATCH v2] powerpc/book3s64/radix: Fix boot failure with large amount of guest memory
From: Michael Ellerman @ 2020-09-03 12:26 UTC (permalink / raw)
To: Aneesh Kumar K.V, linuxppc-dev, mpe; +Cc: Shirisha Ganta, Hari Bathini
In-Reply-To: <20200828100852.426575-1-aneesh.kumar@linux.ibm.com>
On Fri, 28 Aug 2020 15:38:52 +0530, Aneesh Kumar K.V wrote:
> If the hypervisor doesn't support hugepages, the kernel ends up allocating a large
> number of page table pages. The early page table allocation was wrongly
> setting the max memblock limit to ppc64_rma_size with radix translation
> which resulted in boot failure as shown below.
>
> Kernel panic - not syncing:
> early_alloc_pgtable: Failed to allocate 16777216 bytes align=0x1000000 nid=-1 from=0x0000000000000000 max_addr=0xffffffffffffffff
> CPU: 0 PID: 0 Comm: swapper Not tainted 5.8.0-24.9-default+ #2
> Call Trace:
> [c0000000016f3d00] [c0000000007c6470] dump_stack+0xc4/0x114 (unreliable)
> [c0000000016f3d40] [c00000000014c78c] panic+0x164/0x418
> [c0000000016f3dd0] [c000000000098890] early_alloc_pgtable+0xe0/0xec
> [c0000000016f3e60] [c0000000010a5440] radix__early_init_mmu+0x360/0x4b4
> [c0000000016f3ef0] [c000000001099bac] early_init_mmu+0x1c/0x3c
> [c0000000016f3f10] [c00000000109a320] early_setup+0x134/0x170
>
> [...]
Applied to powerpc/fixes.
[1/1] powerpc/book3s64/radix: Fix boot failure with large amount of guest memory
https://git.kernel.org/powerpc/c/103a8542cb35b5130f732d00b0419a594ba1b517
cheers
^ permalink raw reply
* Re: [PATCH 1/2] dma-mapping: introduce dma_get_seg_boundary_nr_pages()
From: Andy Shevchenko @ 2020-09-03 10:57 UTC (permalink / raw)
To: Nicolin Chen
Cc: linux-ia64, James Bottomley, Paul Mackerras, H. Peter Anvin,
sparclinux, Christoph Hellwig, Stephen Rothwell, Helge Deller,
maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
Christian Borntraeger, Ingo Molnar, Matt Turner, Fenghua Yu,
Vasily Gorbik, schnelle, hca, ink, Thomas Gleixner,
gerald.schaefer, rth, Tony Luck, linux-parisc, linux-s390,
Linux Kernel Mailing List, linux-alpha, Borislav Petkov,
open list:LINUX FOR POWERPC PA SEMI PWRFICIENT, David S. Miller
In-Reply-To: <20200901221646.26491-2-nicoleotsuka@gmail.com>
On Wed, Sep 2, 2020 at 1:20 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
>
> We found that callers of dma_get_seg_boundary mostly do an ALIGN
> with page mask and then do a page shift to get number of pages:
> ALIGN(boundary + 1, 1 << shift) >> shift
>
> However, the boundary might be as large as ULONG_MAX, which means
> that a device has no specific boundary limit. So either "+ 1" or
> passing it to ALIGN() would potentially overflow.
>
> According to kernel defines:
> #define ALIGN_MASK(x, mask) (((x) + (mask)) & ~(mask))
> #define ALIGN(x, a) ALIGN_MASK(x, (typeof(x))(a) - 1)
>
> We can simplify the logic here into a helper function doing:
> ALIGN(boundary + 1, 1 << shift) >> shift
> = ALIGN_MASK(b + 1, (1 << s) - 1) >> s
> = {[b + 1 + (1 << s) - 1] & ~[(1 << s) - 1]} >> s
> = [b + 1 + (1 << s) - 1] >> s
> = [b + (1 << s)] >> s
> = (b >> s) + 1
>
> This patch introduces and applies dma_get_seg_boundary_nr_pages()
> as an overflow-free helper for the dma_get_seg_boundary() callers
> to get numbers of pages. It also takes care of the NULL dev case
> for non-DMA API callers.
...
> +static inline unsigned long dma_get_seg_boundary_nr_pages(struct device *dev,
> + unsigned int page_shift)
> +{
> + if (!dev)
> + return (U32_MAX >> page_shift) + 1;
> + return (dma_get_seg_boundary(dev) >> page_shift) + 1;
Can it be better to do something like
unsigned long boundary = dev ? dma_get_seg_boundary(dev) : U32_MAX;
return (boundary >> page_shift) + 1;
?
> +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* [PATCH v2] cpuidle-pseries: Fix CEDE latency conversion from tb to us
From: Gautham R. Shenoy @ 2020-09-03 9:27 UTC (permalink / raw)
To: Michael Ellerman, Rafael J. Wysocki, Vaidyanathan Srinivasan,
Joel Stanley
Cc: Gautham R. Shenoy, linuxppc-dev, linux-kernel, linux-pm
From: "Gautham R. Shenoy" <ego@linux.vnet.ibm.com>
commit d947fb4c965c ("cpuidle: pseries: Fixup exit latency for
CEDE(0)") sets the exit latency of CEDE(0) based on the latency values
of the Extended CEDE states advertised by the platform. The values
advertised by the platform are in timebase ticks. However the cpuidle
framework requires the latency values in microseconds.
If the tb-ticks value advertised by the platform correspond to a value
smaller than 1us, during the conversion from tb-ticks to microseconds,
in the current code, the result becomes zero. This is incorrect as it
puts a CEDE state on par with the snooze state.
This patch fixes this by rounding up the result obtained while
converting the latency value from tb-ticks to microseconds. It also
prints a warning in case we discover an extended-cede state with
wakeup latency to be 0. In such a case, ensure that CEDE(0) has a
non-zero wakeup latency.
Fixes: commit d947fb4c965c ("cpuidle: pseries: Fixup exit latency for
CEDE(0)")
Signed-off-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>
---
v1-->v2: Added a warning if a CEDE state has 0 wakeup latency (Suggested by Joel Stanley)
Also added code to ensure that CEDE(0) has a non-zero wakeup latency.
drivers/cpuidle/cpuidle-pseries.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c
index ff6d99e..a2b5c6f 100644
--- a/drivers/cpuidle/cpuidle-pseries.c
+++ b/drivers/cpuidle/cpuidle-pseries.c
@@ -361,7 +361,10 @@ static void __init fixup_cede0_latency(void)
for (i = 0; i < nr_xcede_records; i++) {
struct xcede_latency_record *record = &payload->records[i];
u64 latency_tb = be64_to_cpu(record->latency_ticks);
- u64 latency_us = tb_to_ns(latency_tb) / NSEC_PER_USEC;
+ u64 latency_us = DIV_ROUND_UP_ULL(tb_to_ns(latency_tb), NSEC_PER_USEC);
+
+ if (latency_us == 0)
+ pr_warn("cpuidle: xcede record %d has an unrealistic latency of 0us.\n", i);
if (latency_us < min_latency_us)
min_latency_us = latency_us;
@@ -378,10 +381,14 @@ static void __init fixup_cede0_latency(void)
* Perform the fix-up.
*/
if (min_latency_us < dedicated_states[1].exit_latency) {
- u64 cede0_latency = min_latency_us - 1;
+ /*
+ * We set a minimum of 1us wakeup latency for cede0 to
+ * distinguish it from snooze
+ */
+ u64 cede0_latency = 1;
- if (cede0_latency <= 0)
- cede0_latency = min_latency_us;
+ if (min_latency_us > cede0_latency)
+ cede0_latency = min_latency_us - 1;
dedicated_states[1].exit_latency = cede0_latency;
dedicated_states[1].target_residency = 10 * (cede0_latency);
--
1.9.4
^ permalink raw reply related
* Re: [PATCH 10/10] powerpc: remove address space overrides using set_fs()
From: Christophe Leroy @ 2020-09-03 8:55 UTC (permalink / raw)
To: Christoph Hellwig, Linus Torvalds
Cc: linux-arch, Kees Cook, the arch/x86 maintainers,
Linux Kernel Mailing List, Al Viro, linux-fsdevel, linuxppc-dev
In-Reply-To: <20200903071144.GA19247@lst.de>
Le 03/09/2020 à 09:11, Christoph Hellwig a écrit :
>
> Except that we do not actually have such a patch. For normal user
> writes we only use ->write_iter if ->write is not present. But what
> shows up in the profile is that /dev/zero only has a read_iter op and
> not a normal read. I've added a patch below that implements a normal
> read which might help a tad with this workload, but should not be part
> of a regression.
>
With that patch below, throughput is 113.5MB/s (instead of 99.9MB/s).
So a 14% improvement. That's not bad.
Christophe
>
> ---
> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> index abd4ffdc8cdebc..1dc99ab158457a 100644
> --- a/drivers/char/mem.c
> +++ b/drivers/char/mem.c
> @@ -726,6 +726,27 @@ static ssize_t read_iter_zero(struct kiocb *iocb, struct iov_iter *iter)
> return written;
> }
>
> +static ssize_t read_zero(struct file *file, char __user *buf,
> + size_t count, loff_t *ppos)
> +{
> + size_t cleared = 0;
> +
> + while (count) {
> + size_t chunk = min_t(size_t, count, PAGE_SIZE);
> +
> + if (clear_user(buf + cleared, chunk))
> + return cleared ? cleared : -EFAULT;
> + cleared += chunk;
> + count -= chunk;
> +
> + if (signal_pending(current))
> + return cleared ? cleared : -ERESTARTSYS;
> + cond_resched();
> + }
> +
> + return cleared;
> +}
> +
> static int mmap_zero(struct file *file, struct vm_area_struct *vma)
> {
> #ifndef CONFIG_MMU
> @@ -921,6 +942,7 @@ static const struct file_operations zero_fops = {
> .llseek = zero_lseek,
> .write = write_zero,
> .read_iter = read_iter_zero,
> + .read = read_zero,
> .write_iter = write_iter_zero,
> .mmap = mmap_zero,
> .get_unmapped_area = get_unmapped_area_zero,
>
^ permalink raw reply
* Re: [PATCH 10/10] powerpc: remove address space overrides using set_fs()
From: Christophe Leroy @ 2020-09-03 7:27 UTC (permalink / raw)
To: Christoph Hellwig, Linus Torvalds
Cc: linux-arch, Kees Cook, the arch/x86 maintainers,
Linux Kernel Mailing List, Al Viro, linux-fsdevel, linuxppc-dev
In-Reply-To: <20200903071144.GA19247@lst.de>
Le 03/09/2020 à 09:11, Christoph Hellwig a écrit :
> On Wed, Sep 02, 2020 at 11:02:22AM -0700, Linus Torvalds wrote:
>> I don't see why this change would make any difference.
>
> Me neither, but while looking at a different project I did spot places
> that actually do an access_ok with len 0, that's why I wanted him to
> try.
>
> That being said: Christophe are these number stables? Do you get
> similar numbers with multiple runs?
Yes the numbers are similar with multiple runs and multiple reboots.
>
>> And btw, why do the 32-bit and 64-bit checks even differ? It's not
>> like the extra (single) instruction should even matter. I think the
>> main reason is that the simpler 64-bit case could stay as a macro
>> (because it only uses "addr" and "size" once), but honestly, that
>> "simplification" doesn't help when you then need to have that #ifdef
>> for the 32-bit case and an inline function anyway.
>
> I'll have to leave that to the powerpc folks. The intent was to not
> change the behavior (and I even fucked that up for the the size == 0
> case).
>
>> However, I suspect a bigger reason for the actual performance
>> degradation would be the patch that makes things use "write_iter()"
>> for writing, even when a simpler "write()" exists.
>
> Except that we do not actually have such a patch. For normal user
> writes we only use ->write_iter if ->write is not present. But what
> shows up in the profile is that /dev/zero only has a read_iter op and
> not a normal read. I've added a patch below that implements a normal
> read which might help a tad with this workload, but should not be part
> of a regression.
>
> Also Christophe: can you bisect which patch starts this? Is it really
> this last patch in the series?
5.9-rc2: 91.5MB/s
Patch 1: 74.9MB/s
Patch 2: 97.9MB/s
Patch 3: 97.7MB/s
Patch 4 to 9: 97.9MB/s
Patch 10: 85.3MB/s
Patch 11: 75.4MB/s
See my other mail, when removing CONFIG_STACKPROTECTOR, I get a stable
99.8MB/s throughput.
Christophe
^ permalink raw reply
* Re: [PATCH 10/10] powerpc: remove address space overrides using set_fs()
From: Christophe Leroy @ 2020-09-03 7:20 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-arch, Kees Cook, the arch/x86 maintainers,
Linux Kernel Mailing List, Al Viro, linux-fsdevel, linuxppc-dev,
Christoph Hellwig
In-Reply-To: <CAHk-=wiDCcxuHgENo3UtdFi2QW9B7yXvNpG5CtF=A6bc6PTTgA@mail.gmail.com>
Le 02/09/2020 à 20:02, Linus Torvalds a écrit :
> On Wed, Sep 2, 2020 at 8:17 AM Christophe Leroy
> <christophe.leroy@csgroup.eu> wrote:
>>
>>
>> With this fix, I get
>>
>> root@vgoippro:~# time dd if=/dev/zero of=/dev/null count=1M
>> 536870912 bytes (512.0MB) copied, 6.776327 seconds, 75.6MB/s
>>
>> That's still far from the 91.7MB/s I get with 5.9-rc2, but better than
>> the 65.8MB/s I got yesterday with your series. Still some way to go thought.
>
> I don't see why this change would make any difference.
>
Neither do I.
Looks like nowadays, CONFIG_STACKPROTECTOR has become a default.
I rebuilt the kernel without it, I now get a throughput of 99.8MB/s both
without and with this series.
Looking at the generated code (GCC 10.1), a small change in a function
seems to make large changes in the generated code when
CONFIG_STACKPROTECTOR is set.
In addition to that, trivial functions which don't use the stack at all
get a stack frame anyway when CONFIG_STACKPROTECTOR is set, allthough
that's only -fstack-protector-strong. And there is no canary check.
Without CONFIG_STACKPROTECTOR:
c01572a0 <no_llseek>:
c01572a0: 38 60 ff ff li r3,-1
c01572a4: 38 80 ff e3 li r4,-29
c01572a8: 4e 80 00 20 blr
With CONFIG_STACKPROTECTOR (regardless of CONFIG_STACKPROTECTOR_STRONG
or not):
c0164e08 <no_llseek>:
c0164e08: 94 21 ff f0 stwu r1,-16(r1)
c0164e0c: 38 60 ff ff li r3,-1
c0164e10: 38 80 ff e3 li r4,-29
c0164e14: 38 21 00 10 addi r1,r1,16
c0164e18: 4e 80 00 20 blr
Wondering why CONFIG_STACKPROTECTOR has become the default. It seems to
imply a 10% performance loss even in the best case (91.7MB/s versus
99.8MB/s)
Note that without CONFIG_STACKPROTECTOR_STRONG, I'm at 99.3MB/s, so
that's really the _STRONG alternative that hurts.
Christophe
^ permalink raw reply
* Re: [PATCH 10/10] powerpc: remove address space overrides using set_fs()
From: Christoph Hellwig @ 2020-09-03 7:11 UTC (permalink / raw)
To: Linus Torvalds
Cc: linux-arch, Kees Cook, the arch/x86 maintainers,
Linux Kernel Mailing List, Al Viro, linux-fsdevel, linuxppc-dev,
Christoph Hellwig
In-Reply-To: <CAHk-=wiDCcxuHgENo3UtdFi2QW9B7yXvNpG5CtF=A6bc6PTTgA@mail.gmail.com>
On Wed, Sep 02, 2020 at 11:02:22AM -0700, Linus Torvalds wrote:
> I don't see why this change would make any difference.
Me neither, but while looking at a different project I did spot places
that actually do an access_ok with len 0, that's why I wanted him to
try.
That being said: Christophe are these number stables? Do you get
similar numbers with multiple runs?
> And btw, why do the 32-bit and 64-bit checks even differ? It's not
> like the extra (single) instruction should even matter. I think the
> main reason is that the simpler 64-bit case could stay as a macro
> (because it only uses "addr" and "size" once), but honestly, that
> "simplification" doesn't help when you then need to have that #ifdef
> for the 32-bit case and an inline function anyway.
I'll have to leave that to the powerpc folks. The intent was to not
change the behavior (and I even fucked that up for the the size == 0
case).
> However, I suspect a bigger reason for the actual performance
> degradation would be the patch that makes things use "write_iter()"
> for writing, even when a simpler "write()" exists.
Except that we do not actually have such a patch. For normal user
writes we only use ->write_iter if ->write is not present. But what
shows up in the profile is that /dev/zero only has a read_iter op and
not a normal read. I've added a patch below that implements a normal
read which might help a tad with this workload, but should not be part
of a regression.
Also Christophe: can you bisect which patch starts this? Is it really
this last patch in the series?
---
diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index abd4ffdc8cdebc..1dc99ab158457a 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -726,6 +726,27 @@ static ssize_t read_iter_zero(struct kiocb *iocb, struct iov_iter *iter)
return written;
}
+static ssize_t read_zero(struct file *file, char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ size_t cleared = 0;
+
+ while (count) {
+ size_t chunk = min_t(size_t, count, PAGE_SIZE);
+
+ if (clear_user(buf + cleared, chunk))
+ return cleared ? cleared : -EFAULT;
+ cleared += chunk;
+ count -= chunk;
+
+ if (signal_pending(current))
+ return cleared ? cleared : -ERESTARTSYS;
+ cond_resched();
+ }
+
+ return cleared;
+}
+
static int mmap_zero(struct file *file, struct vm_area_struct *vma)
{
#ifndef CONFIG_MMU
@@ -921,6 +942,7 @@ static const struct file_operations zero_fops = {
.llseek = zero_lseek,
.write = write_zero,
.read_iter = read_iter_zero,
+ .read = read_zero,
.write_iter = write_iter_zero,
.mmap = mmap_zero,
.get_unmapped_area = get_unmapped_area_zero,
^ permalink raw reply related
* [PATCH v5] soc: fsl: enable acpi support in RCPM driver
From: Ran Wang @ 2020-09-03 7:01 UTC (permalink / raw)
To: Li Yang, Christophe Leroy
Cc: Peng Ma, Ran Wang, linuxppc-dev, linux-kernel, linux-arm-kernel
From: Peng Ma <peng.ma@nxp.com>
This patch enables ACPI support in RCPM driver.
Signed-off-by: Peng Ma <peng.ma@nxp.com>
Signed-off-by: Ran Wang <ran.wang_1@nxp.com>
---
Change in v5:
- Fix panic when dev->of_node is null
Change in v4:
- Make commit subject more accurate
- Remove unrelated new blank line
Change in v3:
- Add #ifdef CONFIG_ACPI for acpi_device_id
- Rename rcpm_acpi_imx_ids to rcpm_acpi_ids
Change in v2:
- Update acpi_device_id to fix conflict with other driver
drivers/soc/fsl/rcpm.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/soc/fsl/rcpm.c b/drivers/soc/fsl/rcpm.c
index a093dbe..58870f4 100644
--- a/drivers/soc/fsl/rcpm.c
+++ b/drivers/soc/fsl/rcpm.c
@@ -2,7 +2,7 @@
//
// rcpm.c - Freescale QorIQ RCPM driver
//
-// Copyright 2019 NXP
+// Copyright 2019-2020 NXP
//
// Author: Ran Wang <ran.wang_1@nxp.com>
@@ -13,6 +13,7 @@
#include <linux/slab.h>
#include <linux/suspend.h>
#include <linux/kernel.h>
+#include <linux/acpi.h>
#define RCPM_WAKEUP_CELL_MAX_SIZE 7
@@ -56,10 +57,14 @@ static int rcpm_pm_prepare(struct device *dev)
"fsl,rcpm-wakeup", value,
rcpm->wakeup_cells + 1);
- /* Wakeup source should refer to current rcpm device */
- if (ret || (np->phandle != value[0]))
+ if (ret)
continue;
+ if (is_of_node(dev->fwnode))
+ /* Should refer to current rcpm device */
+ if (np->phandle != value[0])
+ continue;
+
/* Property "#fsl,rcpm-wakeup-cells" of rcpm node defines the
* number of IPPDEXPCR register cells, and "fsl,rcpm-wakeup"
* of wakeup source IP contains an integer array: <phandle to
@@ -139,10 +144,19 @@ static const struct of_device_id rcpm_of_match[] = {
};
MODULE_DEVICE_TABLE(of, rcpm_of_match);
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id rcpm_acpi_ids[] = {
+ {"NXP0015",},
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, rcpm_acpi_ids);
+#endif
+
static struct platform_driver rcpm_driver = {
.driver = {
.name = "rcpm",
.of_match_table = rcpm_of_match,
+ .acpi_match_table = ACPI_PTR(rcpm_acpi_ids),
.pm = &rcpm_pm_ops,
},
.probe = rcpm_probe,
--
2.7.4
^ permalink raw reply related
* [PATCH v2] ASoC: fsl_sai: Set SAI Channel Mode to Output Mode
From: Shengjiu Wang @ 2020-09-03 5:53 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
alsa-devel, lgirdwood
Cc: linuxppc-dev, linux-kernel
Transmit data pins will output zero when slots are masked or channels
are disabled. In CHMOD TDM mode, transmit data pins are tri-stated when
slots are masked or channels are disabled. When data pins are tri-stated,
there is noise on some channels when FS clock value is high and data is
read while fsclk is transitioning from high to low.
Signed-off-by: Cosmin-Gabriel Samoila <cosmin.samoila@nxp.com>
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
---
changes in v2
- update note
- add acked-by Nicolin
sound/soc/fsl/fsl_sai.c | 10 ++++++++--
sound/soc/fsl/fsl_sai.h | 2 ++
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index 38c7bcbb361d..b2d65e53dbc4 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -489,6 +489,10 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
val_cr4 |= FSL_SAI_CR4_FRSZ(slots);
+ /* Set to output mode to avoid tri-stated data pins */
+ if (tx)
+ val_cr4 |= FSL_SAI_CR4_CHMOD;
+
/*
* For SAI master mode, when Tx(Rx) sync with Rx(Tx) clock, Rx(Tx) will
* generate bclk and frame clock for Tx(Rx), we should set RCR4(TCR4),
@@ -497,7 +501,8 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
if (!sai->is_slave_mode && fsl_sai_dir_is_synced(sai, adir)) {
regmap_update_bits(sai->regmap, FSL_SAI_xCR4(!tx, ofs),
- FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK,
+ FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK |
+ FSL_SAI_CR4_CHMOD_MASK,
val_cr4);
regmap_update_bits(sai->regmap, FSL_SAI_xCR5(!tx, ofs),
FSL_SAI_CR5_WNW_MASK | FSL_SAI_CR5_W0W_MASK |
@@ -508,7 +513,8 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
FSL_SAI_CR3_TRCE_MASK,
FSL_SAI_CR3_TRCE((1 << pins) - 1));
regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx, ofs),
- FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK,
+ FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK |
+ FSL_SAI_CR4_CHMOD_MASK,
val_cr4);
regmap_update_bits(sai->regmap, FSL_SAI_xCR5(tx, ofs),
FSL_SAI_CR5_WNW_MASK | FSL_SAI_CR5_W0W_MASK |
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
index 5f630be74853..736a437450c8 100644
--- a/sound/soc/fsl/fsl_sai.h
+++ b/sound/soc/fsl/fsl_sai.h
@@ -119,6 +119,8 @@
#define FSL_SAI_CR4_FRSZ_MASK (0x1f << 16)
#define FSL_SAI_CR4_SYWD(x) (((x) - 1) << 8)
#define FSL_SAI_CR4_SYWD_MASK (0x1f << 8)
+#define FSL_SAI_CR4_CHMOD BIT(5)
+#define FSL_SAI_CR4_CHMOD_MASK BIT(5)
#define FSL_SAI_CR4_MF BIT(4)
#define FSL_SAI_CR4_FSE BIT(3)
#define FSL_SAI_CR4_FSP BIT(1)
--
2.27.0
^ permalink raw reply related
* Re: [PATCH] ASoC: fsl_sai: Set SAI Channel Mode to Output Mode
From: Shengjiu Wang @ 2020-09-03 5:38 UTC (permalink / raw)
To: Nicolin Chen
Cc: Linux-ALSA, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Liam Girdwood, Takashi Iwai, Mark Brown, linuxppc-dev,
linux-kernel
In-Reply-To: <20200903034018.GC4517@Asurada-Nvidia>
On Thu, Sep 3, 2020 at 11:42 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
>
> On Thu, Sep 03, 2020 at 11:09:15AM +0800, Shengjiu Wang wrote:
> > Transmit data pins will output zero when slots are masked or channels
> > are disabled. In CHMOD TDM mode, transmit data pins are tri-stated when
> > slots are masked or channels are disabled. When data pins are tri-stated,
> > there is noise on some channels when FS clock value is high and data is
> > read while fsclk is transitioning from high to low.
> >
> > Signed-off-by: Cosmin-Gabriel Samoila <cosmin.samoila@nxp.com>
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
>
> Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
>
> Though one nit inline:
>
> > ---
> > sound/soc/fsl/fsl_sai.c | 12 ++++++++++--
> > sound/soc/fsl/fsl_sai.h | 2 ++
> > 2 files changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> > index 62c5fdb678fc..33b194a5c1dc 100644
> > --- a/sound/soc/fsl/fsl_sai.c
> > +++ b/sound/soc/fsl/fsl_sai.c
> > @@ -486,6 +486,12 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream,
> >
> > val_cr4 |= FSL_SAI_CR4_FRSZ(slots);
> >
> > + /* Output Mode - data pins transmit 0 when slots are masked
> > + * or channels are disabled
> > + */
>
> Coding style for multi-line comments. Yet, probably can simplify?
>
> /* Set to output mode to avoid tri-stated data pins */
Ok, will update in v2.
best regards
wang shengjiu
^ permalink raw reply
* Re: [PATCH v2] scsi: ibmvfc: interface updates for future FPIN and MQ support
From: Tyrel Datwyler @ 2020-09-03 5:31 UTC (permalink / raw)
To: Martin K. Petersen
Cc: james.bottomley, brking, linuxppc-dev, linux-kernel, linux-scsi
In-Reply-To: <yq1y2lrd2ys.fsf@ca-mkp.ca.oracle.com>
On 9/2/20 7:11 PM, Martin K. Petersen wrote:
>
> Tyrel,
>
>> Fixup complier errors from neglected commit --amend
>
> Bunch of formatting-related checkpatch warnings. Please fix.
>
> Thanks!
>
So, I stuck to the existing style already in that header. If I'm going to fixup
to make checkpatch happy I imagine it makes sense to send a prerequisite patch
that fixes up the rest of the header.
-Tyrel
^ permalink raw reply
* Re: [PATCH v1 02/10] powerpc/kernel/iommu: Align size for IOMMU_PAGE_SIZE on iommu_*_coherent()
From: Alexey Kardashevskiy @ 2020-09-03 4:41 UTC (permalink / raw)
To: Leonardo Bras, Michael Ellerman, Benjamin Herrenschmidt,
Paul Mackerras, Christophe Leroy, Joel Stanley,
Thiago Jung Bauermann, Ram Pai, Brian King,
Murilo Fossa Vicentini, David Dai
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <39ad3a9c103faf9c5fc2fd5700d8606eb4a2b67e.camel@gmail.com>
On 02/09/2020 08:34, Leonardo Bras wrote:
> On Mon, 2020-08-31 at 10:47 +1000, Alexey Kardashevskiy wrote:
>>>
>>> Maybe testing with host 64k pagesize and IOMMU 16MB pagesize in qemu
>>> should be enough, is there any chance to get indirect mapping in qemu
>>> like this? (DDW but with smaller DMA window available)
>>
>> You will have to hack the guest kernel to always do indirect mapping or
>> hack QEMU's rtas_ibm_query_pe_dma_window() to return a small number of
>> available TCEs. But you will be testing QEMU/KVM which behave quite
>> differently to pHyp in this particular case.
>>
>
> As you suggested before, building for 4k cpu pagesize should be the
> best approach. It would allow testing for both pHyp and qemu scenarios.
>
>>>>>> Because if we want the former (==support), then we'll have to align the
>>>>>> size up to the bigger page size when allocating/zeroing system pages,
>>>>>> etc.
>>>>>
>>>>> This part I don't understand. Why do we need to align everything to the
>>>>> bigger pagesize?
>>>>>
>>>>> I mean, is not that enough that the range [ret, ret + size[ is both
>>>>> allocated by mm and mapped on a iommu range?
>>>>>
>>>>> Suppose a iommu_alloc_coherent() of 16kB on PAGESIZE = 4k and
>>>>> IOMMU_PAGE_SIZE() == 64k.
>>>>> Why 4 * cpu_pages mapped by a 64k IOMMU page is not enough?
>>>>> All the space the user asked for is allocated and mapped for DMA.
>>>>
>>>> The user asked to map 16K, the rest - 48K - is used for something else
>>>> (may be even mapped to another device) but you are making all 64K
>>>> accessible by the device which only should be able to access 16K.
>>>>
>>>> In practice, if this happens, H_PUT_TCE will simply fail.
>>>
>>> I have noticed mlx5 driver getting a few bytes in a buffer, and using
>>> iommu_map_page(). It does map a whole page for as few bytes as the user
>>
>> Whole 4K system page or whole 64K iommu page?
>
> I tested it in 64k system page + 64k iommu page.
>
> The 64K system page may be used for anything, and a small portion of it
> (say 128 bytes) needs to be used for DMA.
> The whole page is mapped by IOMMU, and the driver gets info of the
> memory range it should access / modify.
This works because the whole system page belongs to the same memory
context and IOMMU allows a device to access that page. You can still
have problems if there is a bug within the page but it will go mostly
unnoticed as it will be memory corruption.
If you system page is smaller (4K) than IOMMU page (64K), then the
device gets wider access than it should but it is still going to be
silent memory corruption.
>
>>
>>> wants mapped, and the other bytes get used for something else, or just
>>> mapped on another DMA page.
>>> It seems to work fine.
>>
>>
>> With 4K system page and 64K IOMMU page? In practice it would take an
>> effort or/and bad luck to see it crashing. Thanks,
>
> I haven't tested it yet. On a 64k system page and 4k/64k iommu page, it
> works as described above.
>
> I am new to this, so I am trying to understand how a memory page mapped
> as DMA, and used for something else could be a problem.
From the device prospective, there is PCI space and everything from 0
till 1<<64 is accessible and what is that mapped to - the device does
not know. PHB's IOMMU is the thing to notice invalid access and raise
EEH but PHB only knows about PCI->physical memory mapping (with IOMMU
pages) but nothing about the host kernel pages. Does this help? Thanks,
>
> Thanks!
>
>>
>>>>
>>>>>> Bigger pages are not the case here as I understand it.
>>>>>
>>>>> I did not get this part, what do you mean?
>>>>
>>>> Possible IOMMU page sizes are 4K, 64K, 2M, 16M, 256M, 1GB, and the
>>>> supported set of sizes is different for P8/P9 and type of IO (PHB,
>>>> NVLink/CAPI).
>>>>
>>>>
>>>>>>> Update those functions to guarantee alignment with requested size
>>>>>>> using IOMMU_PAGE_ALIGN() before doing iommu_alloc() / iommu_free().
>>>>>>>
>>>>>>> Also, on iommu_range_alloc(), replace ALIGN(n, 1 << tbl->it_page_shift)
>>>>>>> with IOMMU_PAGE_ALIGN(n, tbl), which seems easier to read.
>>>>>>>
>>>>>>> Signed-off-by: Leonardo Bras <leobras.c@gmail.com>
>>>>>>> ---
>>>>>>> arch/powerpc/kernel/iommu.c | 17 +++++++++--------
>>>>>>> 1 file changed, 9 insertions(+), 8 deletions(-)
>>>>>>>
>>>>>>> diff --git a/arch/powerpc/kernel/iommu.c b/arch/powerpc/kernel/iommu.c
>>>>>>> index 9704f3f76e63..d7086087830f 100644
>>>>>>> --- a/arch/powerpc/kernel/iommu.c
>>>>>>> +++ b/arch/powerpc/kernel/iommu.c
>>>>>>> @@ -237,10 +237,9 @@ static unsigned long iommu_range_alloc(struct device *dev,
>>>>>>> }
>>>>>>>
>>>>>>> if (dev)
>>>>>>> - boundary_size = ALIGN(dma_get_seg_boundary(dev) + 1,
>>>>>>> - 1 << tbl->it_page_shift);
>>>>>>> + boundary_size = IOMMU_PAGE_ALIGN(dma_get_seg_boundary(dev) + 1, tbl);
>>>>>>
>>>>>> Run checkpatch.pl, should complain about a long line.
>>>>>
>>>>> It's 86 columns long, which is less than the new limit of 100 columns
>>>>> Linus announced a few weeks ago. checkpatch.pl was updated too:
>>>>> https://www.phoronix.com/scan.php?page=news_item&px=Linux-Kernel-Deprecates-80-Col
>>>>
>>>> Yay finally :) Thanks,
>>>
>>> :)
>>>
>>>>
>>>>>>> else
>>>>>>> - boundary_size = ALIGN(1UL << 32, 1 << tbl->it_page_shift);
>>>>>>> + boundary_size = IOMMU_PAGE_ALIGN(1UL << 32, tbl);
>>>>>>> /* 4GB boundary for iseries_hv_alloc and iseries_hv_map */
>>>>>>>
>>>>>>> n = iommu_area_alloc(tbl->it_map, limit, start, npages, tbl->it_offset,
>>>>>>> @@ -858,6 +857,7 @@ void *iommu_alloc_coherent(struct device *dev, struct iommu_table *tbl,
>>>>>>> unsigned int order;
>>>>>>> unsigned int nio_pages, io_order;
>>>>>>> struct page *page;
>>>>>>> + size_t size_io = size;
>>>>>>>
>>>>>>> size = PAGE_ALIGN(size);
>>>>>>> order = get_order(size);
>>>>>>> @@ -884,8 +884,9 @@ void *iommu_alloc_coherent(struct device *dev, struct iommu_table *tbl,
>>>>>>> memset(ret, 0, size);
>>>>>>>
>>>>>>> /* Set up tces to cover the allocated range */
>>>>>>> - nio_pages = size >> tbl->it_page_shift;
>>>>>>> - io_order = get_iommu_order(size, tbl);
>>>>>>> + size_io = IOMMU_PAGE_ALIGN(size_io, tbl);
>>>>>>> + nio_pages = size_io >> tbl->it_page_shift;
>>>>>>> + io_order = get_iommu_order(size_io, tbl);
>>>>>>> mapping = iommu_alloc(dev, tbl, ret, nio_pages, DMA_BIDIRECTIONAL,
>>>>>>> mask >> tbl->it_page_shift, io_order, 0);
>>>>>>> if (mapping == DMA_MAPPING_ERROR) {
>>>>>>> @@ -900,11 +901,11 @@ void iommu_free_coherent(struct iommu_table *tbl, size_t size,
>>>>>>> void *vaddr, dma_addr_t dma_handle)
>>>>>>> {
>>>>>>> if (tbl) {
>>>>>>> - unsigned int nio_pages;
>>>>>>> + size_t size_io = IOMMU_PAGE_ALIGN(size, tbl);
>>>>>>> + unsigned int nio_pages = size_io >> tbl->it_page_shift;
>>>>>>>
>>>>>>> - size = PAGE_ALIGN(size);
>>>>>>> - nio_pages = size >> tbl->it_page_shift;
>>>>>>> iommu_free(tbl, dma_handle, nio_pages);
>>>>>>> +
>>>>>>
>>>>>> Unrelated new line.
>>>>>
>>>>> Will be removed. Thanks!
>>>>>
>>>>>>> size = PAGE_ALIGN(size);
>>>>>>> free_pages((unsigned long)vaddr, get_order(size));
>>>>>>> }
>>>>>>>
>
--
Alexey
^ permalink raw reply
* Re: [PATCH v1 01/10] powerpc/pseries/iommu: Replace hard-coded page shift
From: Alexey Kardashevskiy @ 2020-09-03 4:26 UTC (permalink / raw)
To: Leonardo Bras, Oliver O'Halloran
Cc: Christophe Leroy, David Dai, Ram Pai, Linux Kernel Mailing List,
Murilo Fossa Vicentini, Paul Mackerras, Joel Stanley, Brian King,
linuxppc-dev, Thiago Jung Bauermann
In-Reply-To: <c381d7e60d0924e432b0f36dce9a44b89733a129.camel@gmail.com>
On 02/09/2020 07:38, Leonardo Bras wrote:
> On Mon, 2020-08-31 at 13:48 +1000, Alexey Kardashevskiy wrote:
>>>>> Well, I created this TCE_RPN_BITS = 52 because the previous mask was a
>>>>> hardcoded 40-bit mask (0xfffffffffful), for hard-coded 12-bit (4k)
>>>>> pagesize, and on PAPR+/LoPAR also defines TCE as having bits 0-51
>>>>> described as RPN, as described before.
>>>>>
>>>>> IODA3 Revision 3.0_prd1 (OpenPowerFoundation), Figure 3.4 and 3.5.
>>>>> shows system memory mapping into a TCE, and the TCE also has bits 0-51
>>>>> for the RPN (52 bits). "Table 3.6. TCE Definition" also shows it.
>>>>> In fact, by the looks of those figures, the RPN_MASK should always be a
>>>>> 52-bit mask, and RPN = (page >> tceshift) & RPN_MASK.
>>>>
>>>> I suspect the mask is there in the first place for extra protection
>>>> against too big addresses going to the TCE table (or/and for virtial vs
>>>> physical addresses). Using 52bit mask makes no sense for anything, you
>>>> could just drop the mask and let c compiler deal with 64bit "uint" as it
>>>> is basically a 4K page address anywhere in the 64bit space. Thanks,
>>>
>>> Assuming 4K pages you need 52 RPN bits to cover the whole 64bit
>>> physical address space. The IODA3 spec does explicitly say the upper
>>> bits are optional and the implementation only needs to support enough
>>> to cover up to the physical address limit, which is 56bits of P9 /
>>> PHB4. If you want to validate that the address will fit inside of
>>> MAX_PHYSMEM_BITS then fine, but I think that should be done as a
>>> WARN_ON or similar rather than just silently masking off the bits.
>>
>> We can do this and probably should anyway but I am also pretty sure we
>> can just ditch the mask and have the hypervisor return an error which
>> will show up in dmesg.
>
> Ok then, ditching the mask.
Well, you could run a little experiment and set some bits above that old
mask and see how phyp reacts :)
> Thanks!
>
--
Alexey
^ 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