* Re: [PATCH v1 1/2] open: add close_range()
From: Christian Brauner @ 2019-05-23 14:37 UTC (permalink / raw)
To: Jann Horn
Cc: linux-ia64, linux-sh, kernel list, David Howells,
open list:KERNEL SELFTEST FRAMEWORK, sparclinux, Shuah Khan,
linux-arch, linux-s390, Miklos Szeredi, the arch/x86 maintainers,
Linus Torvalds, linux-xtensa, Todd Kjos, Arnd Bergmann,
linux-m68k, Al Viro, Oleg Nesterov, Thomas Gleixner,
Dmitry V. Levin, linux-arm-kernel, Florian Weimer, linux-parisc,
Linux API, linux-mips, linux-alpha, linux-fsdevel, linuxppc-dev
In-Reply-To: <CAG48ez0Uq2GQnQsuPkNrDdJVku_6GPeZ_5F_-5J3iy2CULr0_Q@mail.gmail.com>
On Thu, May 23, 2019 at 04:32:14PM +0200, Jann Horn wrote:
> On Thu, May 23, 2019 at 1:51 PM Christian Brauner <christian@brauner.io> wrote:
> [...]
> > I kept it dumb and was about to reply that your solution introduces more
> > code when it seemed we wanted to keep this very simple for now.
> > But then I saw that find_next_opened_fd() already exists as
> > find_next_fd(). So it's actually not bad compared to what I sent in v1.
> > So - with some small tweaks (need to test it and all now) - how do we
> > feel about?:
> [...]
> > static int __close_next_open_fd(struct files_struct *files, unsigned *curfd, unsigned maxfd)
> > {
> > struct file *file = NULL;
> > unsigned fd;
> > struct fdtable *fdt;
> >
> > spin_lock(&files->file_lock);
> > fdt = files_fdtable(files);
> > fd = find_next_fd(fdt, *curfd);
>
> find_next_fd() finds free fds, not used ones.
>
> > if (fd >= fdt->max_fds || fd > maxfd)
> > goto out_unlock;
> >
> > file = fdt->fd[fd];
> > rcu_assign_pointer(fdt->fd[fd], NULL);
> > __put_unused_fd(files, fd);
>
> You can't do __put_unused_fd() if the old pointer in fdt->fd[fd] was
> NULL - because that means that the fd has been reserved by another
> thread that is about to put a file pointer in there, and if you
> release the fd here, that messes up the refcounting (or hits the
> BUG_ON() in __fd_install()).
>
> > out_unlock:
> > spin_unlock(&files->file_lock);
> >
> > if (!file)
> > return -EBADF;
> >
> > *curfd = fd;
> > filp_close(file, files);
> > return 0;
> > }
> >
> > int __close_range(struct files_struct *files, unsigned fd, unsigned max_fd)
> > {
> > if (fd > max_fd)
> > return -EINVAL;
> >
> > while (fd <= max_fd) {
>
> Note that with a pattern like this, you have to be careful about what
> happens if someone gives you max_fd==0xffffffff - then this condition
> is always true and the loop can not terminate this way.
>
> > if (__close_next_fd(files, &fd, maxfd))
> > break;
>
> (obviously it can still terminate this way)
Yup, this was only a quick draft.
I think the dumb simple thing that I did before was the best way to do
it for now.
I first thought that the find_next_open_fd() function already exists but
when I went to write a POC for testing realized it doesn't anyway.
^ permalink raw reply
* Re: [PATCH v1 1/2] open: add close_range()
From: Jann Horn @ 2019-05-23 14:32 UTC (permalink / raw)
To: Christian Brauner
Cc: linux-ia64, linux-sh, kernel list, David Howells,
open list:KERNEL SELFTEST FRAMEWORK, sparclinux, Shuah Khan,
linux-arch, linux-s390, Miklos Szeredi, the arch/x86 maintainers,
Linus Torvalds, linux-xtensa, Todd Kjos, Arnd Bergmann,
linux-m68k, Al Viro, Oleg Nesterov, Thomas Gleixner,
Dmitry V. Levin, linux-arm-kernel, Florian Weimer, linux-parisc,
Linux API, linux-mips, linux-alpha, linux-fsdevel, linuxppc-dev
In-Reply-To: <20190523115118.pmscbd6kaqy37dym@brauner.io>
On Thu, May 23, 2019 at 1:51 PM Christian Brauner <christian@brauner.io> wrote:
[...]
> I kept it dumb and was about to reply that your solution introduces more
> code when it seemed we wanted to keep this very simple for now.
> But then I saw that find_next_opened_fd() already exists as
> find_next_fd(). So it's actually not bad compared to what I sent in v1.
> So - with some small tweaks (need to test it and all now) - how do we
> feel about?:
[...]
> static int __close_next_open_fd(struct files_struct *files, unsigned *curfd, unsigned maxfd)
> {
> struct file *file = NULL;
> unsigned fd;
> struct fdtable *fdt;
>
> spin_lock(&files->file_lock);
> fdt = files_fdtable(files);
> fd = find_next_fd(fdt, *curfd);
find_next_fd() finds free fds, not used ones.
> if (fd >= fdt->max_fds || fd > maxfd)
> goto out_unlock;
>
> file = fdt->fd[fd];
> rcu_assign_pointer(fdt->fd[fd], NULL);
> __put_unused_fd(files, fd);
You can't do __put_unused_fd() if the old pointer in fdt->fd[fd] was
NULL - because that means that the fd has been reserved by another
thread that is about to put a file pointer in there, and if you
release the fd here, that messes up the refcounting (or hits the
BUG_ON() in __fd_install()).
> out_unlock:
> spin_unlock(&files->file_lock);
>
> if (!file)
> return -EBADF;
>
> *curfd = fd;
> filp_close(file, files);
> return 0;
> }
>
> int __close_range(struct files_struct *files, unsigned fd, unsigned max_fd)
> {
> if (fd > max_fd)
> return -EINVAL;
>
> while (fd <= max_fd) {
Note that with a pattern like this, you have to be careful about what
happens if someone gives you max_fd==0xffffffff - then this condition
is always true and the loop can not terminate this way.
> if (__close_next_fd(files, &fd, maxfd))
> break;
(obviously it can still terminate this way)
> cond_resched();
> fd++;
> }
>
> return 0;
> }
^ permalink raw reply
* Re: [PATCH v1 1/2] open: add close_range()
From: Christian Brauner @ 2019-05-23 14:28 UTC (permalink / raw)
To: Oleg Nesterov
Cc: linux-ia64, linux-sh, ldv, dhowells, linux-kselftest, sparclinux,
shuah, linux-arch, linux-s390, miklos, x86, torvalds, linux-mips,
linux-xtensa, tkjos, arnd, jannh, linux-m68k, viro, tglx,
linux-arm-kernel, fweimer, linux-parisc, linux-api, linux-kernel,
linux-alpha, linux-fsdevel, linuxppc-dev
In-Reply-To: <20190523141447.34s3kc3fuwmoeq7n@brauner.io>
On Thu, May 23, 2019 at 04:14:47PM +0200, Christian Brauner wrote:
> On Thu, May 23, 2019 at 01:51:18PM +0200, Christian Brauner wrote:
> > On Wed, May 22, 2019 at 06:57:37PM +0200, Oleg Nesterov wrote:
> > > On 05/22, Christian Brauner wrote:
> > > >
> > > > +static struct file *pick_file(struct files_struct *files, unsigned fd)
> > > > {
> > > > - struct file *file;
> > > > + struct file *file = NULL;
> > > > struct fdtable *fdt;
> > > >
> > > > spin_lock(&files->file_lock);
> > > > @@ -632,15 +629,65 @@ int __close_fd(struct files_struct *files, unsigned fd)
> > > > goto out_unlock;
> > > > rcu_assign_pointer(fdt->fd[fd], NULL);
> > > > __put_unused_fd(files, fd);
> > > > - spin_unlock(&files->file_lock);
> > > > - return filp_close(file, files);
> > > >
> > > > out_unlock:
> > > > spin_unlock(&files->file_lock);
> > > > - return -EBADF;
> > > > + return file;
> > >
> > > ...
> > >
> > > > +int __close_range(struct files_struct *files, unsigned fd, unsigned max_fd)
> > > > +{
> > > > + unsigned int cur_max;
> > > > +
> > > > + if (fd > max_fd)
> > > > + return -EINVAL;
> > > > +
> > > > + rcu_read_lock();
> > > > + cur_max = files_fdtable(files)->max_fds;
> > > > + rcu_read_unlock();
> > > > +
> > > > + /* cap to last valid index into fdtable */
> > > > + if (max_fd >= cur_max)
> > > > + max_fd = cur_max - 1;
> > > > +
> > > > + while (fd <= max_fd) {
> > > > + struct file *file;
> > > > +
> > > > + file = pick_file(files, fd++);
> > >
> > > Well, how about something like
> > >
> > > static unsigned int find_next_opened_fd(struct fdtable *fdt, unsigned start)
> > > {
> > > unsigned int maxfd = fdt->max_fds;
> > > unsigned int maxbit = maxfd / BITS_PER_LONG;
> > > unsigned int bitbit = start / BITS_PER_LONG;
> > >
> > > bitbit = find_next_bit(fdt->full_fds_bits, maxbit, bitbit) * BITS_PER_LONG;
> > > if (bitbit > maxfd)
> > > return maxfd;
> > > if (bitbit > start)
> > > start = bitbit;
> > > return find_next_bit(fdt->open_fds, maxfd, start);
> > > }
> >
> > >
> > > unsigned close_next_fd(struct files_struct *files, unsigned start, unsigned maxfd)
> > > {
> > > unsigned fd;
> > > struct file *file;
> > > struct fdtable *fdt;
> > >
> > > spin_lock(&files->file_lock);
> > > fdt = files_fdtable(files);
> > > fd = find_next_opened_fd(fdt, start);
> > > if (fd >= fdt->max_fds || fd > maxfd) {
> > > fd = -1;
> > > goto out;
> > > }
> > >
> > > file = fdt->fd[fd];
> > > rcu_assign_pointer(fdt->fd[fd], NULL);
> > > __put_unused_fd(files, fd);
> > > out:
> > > spin_unlock(&files->file_lock);
> > >
> > > if (fd == -1u)
> > > return fd;
> > >
> > > filp_close(file, files);
> > > return fd + 1;
> > > }
> >
> > Thanks, Oleg!
> >
> > I kept it dumb and was about to reply that your solution introduces more
> > code when it seemed we wanted to keep this very simple for now.
> > But then I saw that find_next_opened_fd() already exists as
> > find_next_fd(). So it's actually not bad compared to what I sent in v1.
> > So - with some small tweaks (need to test it and all now) - how do we
> > feel about?:
>
> That's obviously not correct atm but I'll send out a tweaked version in
> a bit.
So given that we would really need another find_next_open_fd() I think
sticking to the simple cond_resched() version I sent before is better
for now until we see real-world performance issues.
I was however missing a test for close_range(fd, fd, 0) anyway so I'll
need to send a v2 with this test added.
Christian
^ permalink raw reply
* Re: [PATCH v1 1/2] open: add close_range()
From: Christian Brauner @ 2019-05-23 14:14 UTC (permalink / raw)
To: Oleg Nesterov
Cc: linux-ia64, linux-sh, ldv, dhowells, linux-kselftest, sparclinux,
shuah, linux-arch, linux-s390, miklos, x86, torvalds, linux-mips,
linux-xtensa, tkjos, arnd, jannh, linux-m68k, viro, tglx,
linux-arm-kernel, fweimer, linux-parisc, linux-api, linux-kernel,
linux-alpha, linux-fsdevel, linuxppc-dev
In-Reply-To: <20190523115118.pmscbd6kaqy37dym@brauner.io>
On Thu, May 23, 2019 at 01:51:18PM +0200, Christian Brauner wrote:
> On Wed, May 22, 2019 at 06:57:37PM +0200, Oleg Nesterov wrote:
> > On 05/22, Christian Brauner wrote:
> > >
> > > +static struct file *pick_file(struct files_struct *files, unsigned fd)
> > > {
> > > - struct file *file;
> > > + struct file *file = NULL;
> > > struct fdtable *fdt;
> > >
> > > spin_lock(&files->file_lock);
> > > @@ -632,15 +629,65 @@ int __close_fd(struct files_struct *files, unsigned fd)
> > > goto out_unlock;
> > > rcu_assign_pointer(fdt->fd[fd], NULL);
> > > __put_unused_fd(files, fd);
> > > - spin_unlock(&files->file_lock);
> > > - return filp_close(file, files);
> > >
> > > out_unlock:
> > > spin_unlock(&files->file_lock);
> > > - return -EBADF;
> > > + return file;
> >
> > ...
> >
> > > +int __close_range(struct files_struct *files, unsigned fd, unsigned max_fd)
> > > +{
> > > + unsigned int cur_max;
> > > +
> > > + if (fd > max_fd)
> > > + return -EINVAL;
> > > +
> > > + rcu_read_lock();
> > > + cur_max = files_fdtable(files)->max_fds;
> > > + rcu_read_unlock();
> > > +
> > > + /* cap to last valid index into fdtable */
> > > + if (max_fd >= cur_max)
> > > + max_fd = cur_max - 1;
> > > +
> > > + while (fd <= max_fd) {
> > > + struct file *file;
> > > +
> > > + file = pick_file(files, fd++);
> >
> > Well, how about something like
> >
> > static unsigned int find_next_opened_fd(struct fdtable *fdt, unsigned start)
> > {
> > unsigned int maxfd = fdt->max_fds;
> > unsigned int maxbit = maxfd / BITS_PER_LONG;
> > unsigned int bitbit = start / BITS_PER_LONG;
> >
> > bitbit = find_next_bit(fdt->full_fds_bits, maxbit, bitbit) * BITS_PER_LONG;
> > if (bitbit > maxfd)
> > return maxfd;
> > if (bitbit > start)
> > start = bitbit;
> > return find_next_bit(fdt->open_fds, maxfd, start);
> > }
>
> >
> > unsigned close_next_fd(struct files_struct *files, unsigned start, unsigned maxfd)
> > {
> > unsigned fd;
> > struct file *file;
> > struct fdtable *fdt;
> >
> > spin_lock(&files->file_lock);
> > fdt = files_fdtable(files);
> > fd = find_next_opened_fd(fdt, start);
> > if (fd >= fdt->max_fds || fd > maxfd) {
> > fd = -1;
> > goto out;
> > }
> >
> > file = fdt->fd[fd];
> > rcu_assign_pointer(fdt->fd[fd], NULL);
> > __put_unused_fd(files, fd);
> > out:
> > spin_unlock(&files->file_lock);
> >
> > if (fd == -1u)
> > return fd;
> >
> > filp_close(file, files);
> > return fd + 1;
> > }
>
> Thanks, Oleg!
>
> I kept it dumb and was about to reply that your solution introduces more
> code when it seemed we wanted to keep this very simple for now.
> But then I saw that find_next_opened_fd() already exists as
> find_next_fd(). So it's actually not bad compared to what I sent in v1.
> So - with some small tweaks (need to test it and all now) - how do we
> feel about?:
That's obviously not correct atm but I'll send out a tweaked version in
a bit.
Christian
^ permalink raw reply
* Applied "spi: spi-fsl-spi: call spi_finalize_current_message() at the end" to the spi tree
From: Mark Brown @ 2019-05-23 13:49 UTC (permalink / raw)
To: Christophe Leroy; +Cc: Mark Brown, linuxppc-dev, linux-kernel, linux-spi
In-Reply-To: <9c1b9545e6683666a795cd89070fa9491f33b9da.1558522754.git.christophe.leroy@c-s.fr>
The patch
spi: spi-fsl-spi: call spi_finalize_current_message() at the end
has been applied to the spi tree at
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-5.2
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
From 44a042182cb1e9f7916e015c836967bf638b33c4 Mon Sep 17 00:00:00 2001
From: Christophe Leroy <christophe.leroy@c-s.fr>
Date: Wed, 22 May 2019 11:00:36 +0000
Subject: [PATCH] spi: spi-fsl-spi: call spi_finalize_current_message() at the
end
spi_finalize_current_message() shall be called once all
actions are finished, otherwise the last actions might
step over a newly started transfer.
Fixes: c592becbe704 ("spi: fsl-(e)spi: migrate to generic master queueing")
Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
drivers/spi/spi-fsl-spi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/spi/spi-fsl-spi.c b/drivers/spi/spi-fsl-spi.c
index b36ac6aa3b1f..7fbdaf066719 100644
--- a/drivers/spi/spi-fsl-spi.c
+++ b/drivers/spi/spi-fsl-spi.c
@@ -432,7 +432,6 @@ static int fsl_spi_do_one_msg(struct spi_master *master,
}
m->status = status;
- spi_finalize_current_message(master);
if (status || !cs_change) {
ndelay(nsecs);
@@ -440,6 +439,7 @@ static int fsl_spi_do_one_msg(struct spi_master *master,
}
fsl_spi_setup_transfer(spi, NULL);
+ spi_finalize_current_message(master);
return 0;
}
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] powerpc/powernv: Show checkstop reason for NPU2 HMIs
From: Michael Ellerman @ 2019-05-23 13:45 UTC (permalink / raw)
To: Frederic Barrat, linuxppc-dev, ajd, arbab, aik; +Cc: clombard, groug
In-Reply-To: <20190523122804.4801-1-fbarrat@linux.ibm.com>
Frederic Barrat <fbarrat@linux.ibm.com> writes:
> If the kernel is notified of an HMI caused by the NPU2, it's currently
> not being recognized and it logs the default message:
>
> Unknown Malfunction Alert of type 3
>
> The NPU on Power 9 has 3 Fault Isolation Registers, so that's a lot of
> possible causes, but we should at least log that it's an NPU problem
> and report which FIR and which bit were raised if opal gave us the
> information.
>
> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
> ---
>
> Could be merged independently from (the opal-api.h change is already
> in the skiboot tree), but works better with, the matching skiboot
> change:
> http://patchwork.ozlabs.org/patch/1104076/
Well it *must* work with or without the skiboot change, because old/new
kernels will run on old/new skiboots.
It looks like it will work fine, we just won't get any extra information
in xstop_reason, right?
cheers
> diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
> index e1577cfa7186..2492fe248e1e 100644
> --- a/arch/powerpc/include/asm/opal-api.h
> +++ b/arch/powerpc/include/asm/opal-api.h
> @@ -568,6 +568,7 @@ enum OpalHMI_XstopType {
> CHECKSTOP_TYPE_UNKNOWN = 0,
> CHECKSTOP_TYPE_CORE = 1,
> CHECKSTOP_TYPE_NX = 2,
> + CHECKSTOP_TYPE_NPU = 3
> };
>
> enum OpalHMI_CoreXstopReason {
> diff --git a/arch/powerpc/platforms/powernv/opal-hmi.c b/arch/powerpc/platforms/powernv/opal-hmi.c
> index 586ec71a4e17..de12a240b477 100644
> --- a/arch/powerpc/platforms/powernv/opal-hmi.c
> +++ b/arch/powerpc/platforms/powernv/opal-hmi.c
> @@ -149,6 +149,43 @@ static void print_nx_checkstop_reason(const char *level,
> xstop_reason[i].description);
> }
>
> +static void print_npu_checkstop_reason(const char *level,
> + struct OpalHMIEvent *hmi_evt)
> +{
> + uint8_t reason, reason_count, i;
> +
> + /*
> + * We may not have a checkstop reason on some combination of
> + * hardware and/or skiboot version
> + */
> + if (!hmi_evt->u.xstop_error.xstop_reason) {
> + printk("%s NPU checkstop on chip %x\n", level,
> + be32_to_cpu(hmi_evt->u.xstop_error.u.chip_id));
> + return;
> + }
> +
> + /*
> + * NPU2 has 3 FIRs. Reason encoded on a byte as:
> + * 2 bits for the FIR number
> + * 6 bits for the bit number
> + * It may be possible to find several reasons.
> + *
> + * We don't display a specific message per FIR bit as there
> + * are too many and most are meaningless without the workbook
> + * and/or hw team help anyway.
> + */
> + reason_count = sizeof(hmi_evt->u.xstop_error.xstop_reason) /
> + sizeof(reason);
> + for (i = 0; i < reason_count; i++) {
> + reason = (hmi_evt->u.xstop_error.xstop_reason >> (8 * i)) & 0xFF;
> + if (reason)
> + printk("%s NPU checkstop on chip %x: FIR%d bit %d is set\n",
> + level,
> + be32_to_cpu(hmi_evt->u.xstop_error.u.chip_id),
> + reason >> 6, reason & 0x3F);
> + }
> +}
> +
> static void print_checkstop_reason(const char *level,
> struct OpalHMIEvent *hmi_evt)
> {
> @@ -160,6 +197,9 @@ static void print_checkstop_reason(const char *level,
> case CHECKSTOP_TYPE_NX:
> print_nx_checkstop_reason(level, hmi_evt);
> break;
> + case CHECKSTOP_TYPE_NPU:
> + print_npu_checkstop_reason(level, hmi_evt);
> + break;
> default:
> printk("%s Unknown Malfunction Alert of type %d\n",
> level, type);
> --
> 2.21.0
^ permalink raw reply
* [PATCH] powerpc/powernv: Show checkstop reason for NPU2 HMIs
From: Frederic Barrat @ 2019-05-23 12:28 UTC (permalink / raw)
To: linuxppc-dev, ajd, arbab, aik; +Cc: clombard, groug
If the kernel is notified of an HMI caused by the NPU2, it's currently
not being recognized and it logs the default message:
Unknown Malfunction Alert of type 3
The NPU on Power 9 has 3 Fault Isolation Registers, so that's a lot of
possible causes, but we should at least log that it's an NPU problem
and report which FIR and which bit were raised if opal gave us the
information.
Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com>
---
Could be merged independently from (the opal-api.h change is already
in the skiboot tree), but works better with, the matching skiboot
change:
http://patchwork.ozlabs.org/patch/1104076/
arch/powerpc/include/asm/opal-api.h | 1 +
arch/powerpc/platforms/powernv/opal-hmi.c | 40 +++++++++++++++++++++++
2 files changed, 41 insertions(+)
diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
index e1577cfa7186..2492fe248e1e 100644
--- a/arch/powerpc/include/asm/opal-api.h
+++ b/arch/powerpc/include/asm/opal-api.h
@@ -568,6 +568,7 @@ enum OpalHMI_XstopType {
CHECKSTOP_TYPE_UNKNOWN = 0,
CHECKSTOP_TYPE_CORE = 1,
CHECKSTOP_TYPE_NX = 2,
+ CHECKSTOP_TYPE_NPU = 3
};
enum OpalHMI_CoreXstopReason {
diff --git a/arch/powerpc/platforms/powernv/opal-hmi.c b/arch/powerpc/platforms/powernv/opal-hmi.c
index 586ec71a4e17..de12a240b477 100644
--- a/arch/powerpc/platforms/powernv/opal-hmi.c
+++ b/arch/powerpc/platforms/powernv/opal-hmi.c
@@ -149,6 +149,43 @@ static void print_nx_checkstop_reason(const char *level,
xstop_reason[i].description);
}
+static void print_npu_checkstop_reason(const char *level,
+ struct OpalHMIEvent *hmi_evt)
+{
+ uint8_t reason, reason_count, i;
+
+ /*
+ * We may not have a checkstop reason on some combination of
+ * hardware and/or skiboot version
+ */
+ if (!hmi_evt->u.xstop_error.xstop_reason) {
+ printk("%s NPU checkstop on chip %x\n", level,
+ be32_to_cpu(hmi_evt->u.xstop_error.u.chip_id));
+ return;
+ }
+
+ /*
+ * NPU2 has 3 FIRs. Reason encoded on a byte as:
+ * 2 bits for the FIR number
+ * 6 bits for the bit number
+ * It may be possible to find several reasons.
+ *
+ * We don't display a specific message per FIR bit as there
+ * are too many and most are meaningless without the workbook
+ * and/or hw team help anyway.
+ */
+ reason_count = sizeof(hmi_evt->u.xstop_error.xstop_reason) /
+ sizeof(reason);
+ for (i = 0; i < reason_count; i++) {
+ reason = (hmi_evt->u.xstop_error.xstop_reason >> (8 * i)) & 0xFF;
+ if (reason)
+ printk("%s NPU checkstop on chip %x: FIR%d bit %d is set\n",
+ level,
+ be32_to_cpu(hmi_evt->u.xstop_error.u.chip_id),
+ reason >> 6, reason & 0x3F);
+ }
+}
+
static void print_checkstop_reason(const char *level,
struct OpalHMIEvent *hmi_evt)
{
@@ -160,6 +197,9 @@ static void print_checkstop_reason(const char *level,
case CHECKSTOP_TYPE_NX:
print_nx_checkstop_reason(level, hmi_evt);
break;
+ case CHECKSTOP_TYPE_NPU:
+ print_npu_checkstop_reason(level, hmi_evt);
+ break;
default:
printk("%s Unknown Malfunction Alert of type %d\n",
level, type);
--
2.21.0
^ permalink raw reply related
* Re: [PATCH] powerpc/power: Expose pfn_is_nosave prototype
From: Christophe Leroy @ 2019-05-23 12:02 UTC (permalink / raw)
To: Mathieu Malaterre, Michael Ellerman
Cc: Len Brown, linux-pm, Rafael J. Wysocki, linux-kernel,
Paul Mackerras, Pavel Machek, linuxppc-dev
In-Reply-To: <20190523114736.30268-1-malat@debian.org>
Le 23/05/2019 à 13:47, Mathieu Malaterre a écrit :
> The declaration for pfn_is_nosave is only available in
> kernel/power/power.h. Since this function can be override in arch,
> expose it globally. Having a prototype will make sure to avoid warning
> (sometime treated as error with W=1) such as:
>
> arch/powerpc/kernel/suspend.c:18:5: error: no previous prototype for 'pfn_is_nosave' [-Werror=missing-prototypes]
>
> This moves the declaration into a globally visible header file and add
> missing include to avoid a warning in powerpc.
Then you should also drop it from kernel/power/power.h and
arch/s390/kernel/entry.h
Christophe
>
> Signed-off-by: Mathieu Malaterre <malat@debian.org>
> ---
> arch/powerpc/kernel/suspend.c | 1 +
> include/linux/suspend.h | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/arch/powerpc/kernel/suspend.c b/arch/powerpc/kernel/suspend.c
> index a531154cc0f3..9e1b6b894245 100644
> --- a/arch/powerpc/kernel/suspend.c
> +++ b/arch/powerpc/kernel/suspend.c
> @@ -8,6 +8,7 @@
> */
>
> #include <linux/mm.h>
> +#include <linux/suspend.h>
> #include <asm/page.h>
> #include <asm/sections.h>
>
> diff --git a/include/linux/suspend.h b/include/linux/suspend.h
> index 3f529ad9a9d2..2660bbdf5230 100644
> --- a/include/linux/suspend.h
> +++ b/include/linux/suspend.h
> @@ -395,6 +395,7 @@ extern bool system_entering_hibernation(void);
> extern bool hibernation_available(void);
> asmlinkage int swsusp_save(void);
> extern struct pbe *restore_pblist;
> +int pfn_is_nosave(unsigned long pfn);
> #else /* CONFIG_HIBERNATION */
> static inline void register_nosave_region(unsigned long b, unsigned long e) {}
> static inline void register_nosave_region_late(unsigned long b, unsigned long e) {}
>
^ permalink raw reply
* Re: [PATCH v1 1/2] open: add close_range()
From: Christian Brauner @ 2019-05-23 11:51 UTC (permalink / raw)
To: Oleg Nesterov
Cc: linux-ia64, linux-sh, ldv, dhowells, linux-kselftest, sparclinux,
shuah, linux-arch, linux-s390, miklos, x86, torvalds, linux-mips,
linux-xtensa, tkjos, arnd, jannh, linux-m68k, viro, tglx,
linux-arm-kernel, fweimer, linux-parisc, linux-api, linux-kernel,
linux-alpha, linux-fsdevel, linuxppc-dev
In-Reply-To: <20190522165737.GC4915@redhat.com>
On Wed, May 22, 2019 at 06:57:37PM +0200, Oleg Nesterov wrote:
> On 05/22, Christian Brauner wrote:
> >
> > +static struct file *pick_file(struct files_struct *files, unsigned fd)
> > {
> > - struct file *file;
> > + struct file *file = NULL;
> > struct fdtable *fdt;
> >
> > spin_lock(&files->file_lock);
> > @@ -632,15 +629,65 @@ int __close_fd(struct files_struct *files, unsigned fd)
> > goto out_unlock;
> > rcu_assign_pointer(fdt->fd[fd], NULL);
> > __put_unused_fd(files, fd);
> > - spin_unlock(&files->file_lock);
> > - return filp_close(file, files);
> >
> > out_unlock:
> > spin_unlock(&files->file_lock);
> > - return -EBADF;
> > + return file;
>
> ...
>
> > +int __close_range(struct files_struct *files, unsigned fd, unsigned max_fd)
> > +{
> > + unsigned int cur_max;
> > +
> > + if (fd > max_fd)
> > + return -EINVAL;
> > +
> > + rcu_read_lock();
> > + cur_max = files_fdtable(files)->max_fds;
> > + rcu_read_unlock();
> > +
> > + /* cap to last valid index into fdtable */
> > + if (max_fd >= cur_max)
> > + max_fd = cur_max - 1;
> > +
> > + while (fd <= max_fd) {
> > + struct file *file;
> > +
> > + file = pick_file(files, fd++);
>
> Well, how about something like
>
> static unsigned int find_next_opened_fd(struct fdtable *fdt, unsigned start)
> {
> unsigned int maxfd = fdt->max_fds;
> unsigned int maxbit = maxfd / BITS_PER_LONG;
> unsigned int bitbit = start / BITS_PER_LONG;
>
> bitbit = find_next_bit(fdt->full_fds_bits, maxbit, bitbit) * BITS_PER_LONG;
> if (bitbit > maxfd)
> return maxfd;
> if (bitbit > start)
> start = bitbit;
> return find_next_bit(fdt->open_fds, maxfd, start);
> }
>
> unsigned close_next_fd(struct files_struct *files, unsigned start, unsigned maxfd)
> {
> unsigned fd;
> struct file *file;
> struct fdtable *fdt;
>
> spin_lock(&files->file_lock);
> fdt = files_fdtable(files);
> fd = find_next_opened_fd(fdt, start);
> if (fd >= fdt->max_fds || fd > maxfd) {
> fd = -1;
> goto out;
> }
>
> file = fdt->fd[fd];
> rcu_assign_pointer(fdt->fd[fd], NULL);
> __put_unused_fd(files, fd);
> out:
> spin_unlock(&files->file_lock);
>
> if (fd == -1u)
> return fd;
>
> filp_close(file, files);
> return fd + 1;
> }
Thanks, Oleg!
I kept it dumb and was about to reply that your solution introduces more
code when it seemed we wanted to keep this very simple for now.
But then I saw that find_next_opened_fd() already exists as
find_next_fd(). So it's actually not bad compared to what I sent in v1.
So - with some small tweaks (need to test it and all now) - how do we
feel about?:
/**
* __close_next_open_fd() - Close the nearest open fd.
*
* @curfd: lowest file descriptor to consider
* @maxfd: highest file descriptor to consider
*
* This function will close the nearest open fd, i.e. it will either
* close @curfd if it is open or the closest open file descriptor
* c greater than @curfd that
* is smaller or equal to maxfd.
* If the function found a file descriptor to close it will return 0 and
* place the file descriptor it closed in @curfd. If it did not find a
* file descriptor to close it will return -EBADF.
*/
static int __close_next_open_fd(struct files_struct *files, unsigned *curfd, unsigned maxfd)
{
struct file *file = NULL;
unsigned fd;
struct fdtable *fdt;
spin_lock(&files->file_lock);
fdt = files_fdtable(files);
fd = find_next_fd(fdt, *curfd);
if (fd >= fdt->max_fds || fd > maxfd)
goto out_unlock;
file = fdt->fd[fd];
rcu_assign_pointer(fdt->fd[fd], NULL);
__put_unused_fd(files, fd);
out_unlock:
spin_unlock(&files->file_lock);
if (!file)
return -EBADF;
*curfd = fd;
filp_close(file, files);
return 0;
}
int __close_range(struct files_struct *files, unsigned fd, unsigned max_fd)
{
if (fd > max_fd)
return -EINVAL;
while (fd <= max_fd) {
if (__close_next_fd(files, &fd, maxfd))
break;
cond_resched();
fd++;
}
return 0;
}
SYSCALL_DEFINE3(close_range, unsigned int, fd, unsigned int, max_fd,
unsigned int, flags)
{
if (flags)
return -EINVAL;
return __close_range(current->files, fd, max_fd);
}
^ permalink raw reply
* Re: [PATCH v2] powerpc/32: sstep: Move variable `rc` within CONFIG_PPC64 sentinels
From: Mathieu Malaterre @ 2019-05-23 11:49 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Paul Mackerras, linuxppc-dev, LKML
In-Reply-To: <20190312212318.17822-1-malat@debian.org>
ping ?
On Tue, Mar 12, 2019 at 10:23 PM Mathieu Malaterre <malat@debian.org> wrote:
>
> Fix warnings treated as errors with W=1:
>
> arch/powerpc/lib/sstep.c:1172:31: error: variable 'rc' set but not used [-Werror=unused-but-set-variable]
>
> Suggested-by: Christophe Leroy <christophe.leroy@c-s.fr>
> Signed-off-by: Mathieu Malaterre <malat@debian.org>
> ---
> v2: as suggested prefer CONFIG_PPC64 sentinel instead of unused keyword
>
> arch/powerpc/lib/sstep.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> index 3d33fb509ef4..9996dc7a0b46 100644
> --- a/arch/powerpc/lib/sstep.c
> +++ b/arch/powerpc/lib/sstep.c
> @@ -1169,7 +1169,10 @@ static nokprobe_inline int trap_compare(long v1, long v2)
> int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
> unsigned int instr)
> {
> - unsigned int opcode, ra, rb, rc, rd, spr, u;
> + unsigned int opcode, ra, rb, rd, spr, u;
> +#ifdef CONFIG_PPC64
> + unsigned int rc;
> +#endif
> unsigned long int imm;
> unsigned long int val, val2;
> unsigned int mb, me, sh;
> @@ -1292,7 +1295,9 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
> rd = (instr >> 21) & 0x1f;
> ra = (instr >> 16) & 0x1f;
> rb = (instr >> 11) & 0x1f;
> +#ifdef CONFIG_PPC64
> rc = (instr >> 6) & 0x1f;
> +#endif
>
> switch (opcode) {
> #ifdef __powerpc64__
> --
> 2.20.1
>
^ permalink raw reply
* [PATCH] powerpc/power: Expose pfn_is_nosave prototype
From: Mathieu Malaterre @ 2019-05-23 11:47 UTC (permalink / raw)
To: Michael Ellerman
Cc: Len Brown, linux-pm, Mathieu Malaterre, Rafael J. Wysocki,
linux-kernel, Paul Mackerras, Pavel Machek, linuxppc-dev
The declaration for pfn_is_nosave is only available in
kernel/power/power.h. Since this function can be override in arch,
expose it globally. Having a prototype will make sure to avoid warning
(sometime treated as error with W=1) such as:
arch/powerpc/kernel/suspend.c:18:5: error: no previous prototype for 'pfn_is_nosave' [-Werror=missing-prototypes]
This moves the declaration into a globally visible header file and add
missing include to avoid a warning in powerpc.
Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
arch/powerpc/kernel/suspend.c | 1 +
include/linux/suspend.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/arch/powerpc/kernel/suspend.c b/arch/powerpc/kernel/suspend.c
index a531154cc0f3..9e1b6b894245 100644
--- a/arch/powerpc/kernel/suspend.c
+++ b/arch/powerpc/kernel/suspend.c
@@ -8,6 +8,7 @@
*/
#include <linux/mm.h>
+#include <linux/suspend.h>
#include <asm/page.h>
#include <asm/sections.h>
diff --git a/include/linux/suspend.h b/include/linux/suspend.h
index 3f529ad9a9d2..2660bbdf5230 100644
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -395,6 +395,7 @@ extern bool system_entering_hibernation(void);
extern bool hibernation_available(void);
asmlinkage int swsusp_save(void);
extern struct pbe *restore_pblist;
+int pfn_is_nosave(unsigned long pfn);
#else /* CONFIG_HIBERNATION */
static inline void register_nosave_region(unsigned long b, unsigned long e) {}
static inline void register_nosave_region_late(unsigned long b, unsigned long e) {}
--
2.20.1
^ permalink raw reply related
* Patch "x86/mpx, mm/core: Fix recursive munmap() corruption" has been added to the 5.1-stable tree
From: gregkh @ 2019-05-23 11:26 UTC (permalink / raw)
To: 20190419194747.5E1AD6DC, akpm, anton.ivanov, benh, bp,
dave.hansen, gregkh, gxt, hjl.tools, hpa, jdike, linux-mm,
linux-um, linuxppc-dev, luto, mhocko, mingo, mpe, paulus, peterz,
rguenther, richard, riel, tglx, torvalds, vbabka, yang.shi
Cc: stable-commits
This is a note to let you know that I've just added the patch titled
x86/mpx, mm/core: Fix recursive munmap() corruption
to the 5.1-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
x86-mpx-mm-core-fix-recursive-munmap-corruption.patch
and it can be found in the queue-5.1 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.
From 5a28fc94c9143db766d1ba5480cae82d856ad080 Mon Sep 17 00:00:00 2001
From: Dave Hansen <dave.hansen@linux.intel.com>
Date: Fri, 19 Apr 2019 12:47:47 -0700
Subject: x86/mpx, mm/core: Fix recursive munmap() corruption
From: Dave Hansen <dave.hansen@linux.intel.com>
commit 5a28fc94c9143db766d1ba5480cae82d856ad080 upstream.
This is a bit of a mess, to put it mildly. But, it's a bug
that only seems to have showed up in 4.20 but wasn't noticed
until now, because nobody uses MPX.
MPX has the arch_unmap() hook inside of munmap() because MPX
uses bounds tables that protect other areas of memory. When
memory is unmapped, there is also a need to unmap the MPX
bounds tables. Barring this, unused bounds tables can eat 80%
of the address space.
But, the recursive do_munmap() that gets called vi arch_unmap()
wreaks havoc with __do_munmap()'s state. It can result in
freeing populated page tables, accessing bogus VMA state,
double-freed VMAs and more.
See the "long story" further below for the gory details.
To fix this, call arch_unmap() before __do_unmap() has a chance
to do anything meaningful. Also, remove the 'vma' argument
and force the MPX code to do its own, independent VMA lookup.
== UML / unicore32 impact ==
Remove unused 'vma' argument to arch_unmap(). No functional
change.
I compile tested this on UML but not unicore32.
== powerpc impact ==
powerpc uses arch_unmap() well to watch for munmap() on the
VDSO and zeroes out 'current->mm->context.vdso_base'. Moving
arch_unmap() makes this happen earlier in __do_munmap(). But,
'vdso_base' seems to only be used in perf and in the signal
delivery that happens near the return to userspace. I can not
find any likely impact to powerpc, other than the zeroing
happening a little earlier.
powerpc does not use the 'vma' argument and is unaffected by
its removal.
I compile-tested a 64-bit powerpc defconfig.
== x86 impact ==
For the common success case this is functionally identical to
what was there before. For the munmap() failure case, it's
possible that some MPX tables will be zapped for memory that
continues to be in use. But, this is an extraordinarily
unlikely scenario and the harm would be that MPX provides no
protection since the bounds table got reset (zeroed).
I can't imagine anyone doing this:
ptr = mmap();
// use ptr
ret = munmap(ptr);
if (ret)
// oh, there was an error, I'll
// keep using ptr.
Because if you're doing munmap(), you are *done* with the
memory. There's probably no good data in there _anyway_.
This passes the original reproducer from Richard Biener as
well as the existing mpx selftests/.
The long story:
munmap() has a couple of pieces:
1. Find the affected VMA(s)
2. Split the start/end one(s) if neceesary
3. Pull the VMAs out of the rbtree
4. Actually zap the memory via unmap_region(), including
freeing page tables (or queueing them to be freed).
5. Fix up some of the accounting (like fput()) and actually
free the VMA itself.
This specific ordering was actually introduced by:
dd2283f2605e ("mm: mmap: zap pages with read mmap_sem in munmap")
during the 4.20 merge window. The previous __do_munmap() code
was actually safe because the only thing after arch_unmap() was
remove_vma_list(). arch_unmap() could not see 'vma' in the
rbtree because it was detached, so it is not even capable of
doing operations unsafe for remove_vma_list()'s use of 'vma'.
Richard Biener reported a test that shows this in dmesg:
[1216548.787498] BUG: Bad rss-counter state mm:0000000017ce560b idx:1 val:551
[1216548.787500] BUG: non-zero pgtables_bytes on freeing mm: 24576
What triggered this was the recursive do_munmap() called via
arch_unmap(). It was freeing page tables that has not been
properly zapped.
But, the problem was bigger than this. For one, arch_unmap()
can free VMAs. But, the calling __do_munmap() has variables
that *point* to VMAs and obviously can't handle them just
getting freed while the pointer is still in use.
I tried a couple of things here. First, I tried to fix the page
table freeing problem in isolation, but I then found the VMA
issue. I also tried having the MPX code return a flag if it
modified the rbtree which would force __do_munmap() to re-walk
to restart. That spiralled out of control in complexity pretty
fast.
Just moving arch_unmap() and accepting that the bonkers failure
case might eat some bounds tables seems like the simplest viable
fix.
This was also reported in the following kernel bugzilla entry:
https://bugzilla.kernel.org/show_bug.cgi?id=203123
There are some reports that this commit triggered this bug:
dd2283f2605 ("mm: mmap: zap pages with read mmap_sem in munmap")
While that commit certainly made the issues easier to hit, I believe
the fundamental issue has been with us as long as MPX itself, thus
the Fixes: tag below is for one of the original MPX commits.
[ mingo: Minor edits to the changelog and the patch. ]
Reported-by: Richard Biener <rguenther@suse.de>
Reported-by: H.J. Lu <hjl.tools@gmail.com>
Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Reviewed-by Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Yang Shi <yang.shi@linux.alibaba.com>
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Anton Ivanov <anton.ivanov@cambridgegreys.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Guan Xuetao <gxt@pku.edu.cn>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Jeff Dike <jdike@addtoit.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Richard Weinberger <richard@nod.at>
Cc: Rik van Riel <riel@surriel.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: linux-arch@vger.kernel.org
Cc: linux-mm@kvack.org
Cc: linux-um@lists.infradead.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: stable@vger.kernel.org
Fixes: dd2283f2605e ("mm: mmap: zap pages with read mmap_sem in munmap")
Link: http://lkml.kernel.org/r/20190419194747.5E1AD6DC@viggo.jf.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
arch/powerpc/include/asm/mmu_context.h | 1 -
arch/um/include/asm/mmu_context.h | 1 -
arch/unicore32/include/asm/mmu_context.h | 1 -
arch/x86/include/asm/mmu_context.h | 6 +++---
arch/x86/include/asm/mpx.h | 15 ++++++++-------
arch/x86/mm/mpx.c | 10 ++++++----
include/asm-generic/mm_hooks.h | 1 -
mm/mmap.c | 15 ++++++++-------
8 files changed, 25 insertions(+), 25 deletions(-)
--- a/arch/powerpc/include/asm/mmu_context.h
+++ b/arch/powerpc/include/asm/mmu_context.h
@@ -237,7 +237,6 @@ extern void arch_exit_mmap(struct mm_str
#endif
static inline void arch_unmap(struct mm_struct *mm,
- struct vm_area_struct *vma,
unsigned long start, unsigned long end)
{
if (start <= mm->context.vdso_base && mm->context.vdso_base < end)
--- a/arch/um/include/asm/mmu_context.h
+++ b/arch/um/include/asm/mmu_context.h
@@ -22,7 +22,6 @@ static inline int arch_dup_mmap(struct m
}
extern void arch_exit_mmap(struct mm_struct *mm);
static inline void arch_unmap(struct mm_struct *mm,
- struct vm_area_struct *vma,
unsigned long start, unsigned long end)
{
}
--- a/arch/unicore32/include/asm/mmu_context.h
+++ b/arch/unicore32/include/asm/mmu_context.h
@@ -88,7 +88,6 @@ static inline int arch_dup_mmap(struct m
}
static inline void arch_unmap(struct mm_struct *mm,
- struct vm_area_struct *vma,
unsigned long start, unsigned long end)
{
}
--- a/arch/x86/include/asm/mmu_context.h
+++ b/arch/x86/include/asm/mmu_context.h
@@ -277,8 +277,8 @@ static inline void arch_bprm_mm_init(str
mpx_mm_init(mm);
}
-static inline void arch_unmap(struct mm_struct *mm, struct vm_area_struct *vma,
- unsigned long start, unsigned long end)
+static inline void arch_unmap(struct mm_struct *mm, unsigned long start,
+ unsigned long end)
{
/*
* mpx_notify_unmap() goes and reads a rarely-hot
@@ -298,7 +298,7 @@ static inline void arch_unmap(struct mm_
* consistently wrong.
*/
if (unlikely(cpu_feature_enabled(X86_FEATURE_MPX)))
- mpx_notify_unmap(mm, vma, start, end);
+ mpx_notify_unmap(mm, start, end);
}
/*
--- a/arch/x86/include/asm/mpx.h
+++ b/arch/x86/include/asm/mpx.h
@@ -64,12 +64,15 @@ struct mpx_fault_info {
};
#ifdef CONFIG_X86_INTEL_MPX
-int mpx_fault_info(struct mpx_fault_info *info, struct pt_regs *regs);
-int mpx_handle_bd_fault(void);
+
+extern int mpx_fault_info(struct mpx_fault_info *info, struct pt_regs *regs);
+extern int mpx_handle_bd_fault(void);
+
static inline int kernel_managing_mpx_tables(struct mm_struct *mm)
{
return (mm->context.bd_addr != MPX_INVALID_BOUNDS_DIR);
}
+
static inline void mpx_mm_init(struct mm_struct *mm)
{
/*
@@ -78,11 +81,10 @@ static inline void mpx_mm_init(struct mm
*/
mm->context.bd_addr = MPX_INVALID_BOUNDS_DIR;
}
-void mpx_notify_unmap(struct mm_struct *mm, struct vm_area_struct *vma,
- unsigned long start, unsigned long end);
-unsigned long mpx_unmapped_area_check(unsigned long addr, unsigned long len,
- unsigned long flags);
+extern void mpx_notify_unmap(struct mm_struct *mm, unsigned long start, unsigned long end);
+extern unsigned long mpx_unmapped_area_check(unsigned long addr, unsigned long len, unsigned long flags);
+
#else
static inline int mpx_fault_info(struct mpx_fault_info *info, struct pt_regs *regs)
{
@@ -100,7 +102,6 @@ static inline void mpx_mm_init(struct mm
{
}
static inline void mpx_notify_unmap(struct mm_struct *mm,
- struct vm_area_struct *vma,
unsigned long start, unsigned long end)
{
}
--- a/arch/x86/mm/mpx.c
+++ b/arch/x86/mm/mpx.c
@@ -881,9 +881,10 @@ static int mpx_unmap_tables(struct mm_st
* the virtual address region start...end have already been split if
* necessary, and the 'vma' is the first vma in this range (start -> end).
*/
-void mpx_notify_unmap(struct mm_struct *mm, struct vm_area_struct *vma,
- unsigned long start, unsigned long end)
+void mpx_notify_unmap(struct mm_struct *mm, unsigned long start,
+ unsigned long end)
{
+ struct vm_area_struct *vma;
int ret;
/*
@@ -902,11 +903,12 @@ void mpx_notify_unmap(struct mm_struct *
* which should not occur normally. Being strict about it here
* helps ensure that we do not have an exploitable stack overflow.
*/
- do {
+ vma = find_vma(mm, start);
+ while (vma && vma->vm_start < end) {
if (vma->vm_flags & VM_MPX)
return;
vma = vma->vm_next;
- } while (vma && vma->vm_start < end);
+ }
ret = mpx_unmap_tables(mm, start, end);
if (ret)
--- a/include/asm-generic/mm_hooks.h
+++ b/include/asm-generic/mm_hooks.h
@@ -18,7 +18,6 @@ static inline void arch_exit_mmap(struct
}
static inline void arch_unmap(struct mm_struct *mm,
- struct vm_area_struct *vma,
unsigned long start, unsigned long end)
{
}
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2735,9 +2735,17 @@ int __do_munmap(struct mm_struct *mm, un
return -EINVAL;
len = PAGE_ALIGN(len);
+ end = start + len;
if (len == 0)
return -EINVAL;
+ /*
+ * arch_unmap() might do unmaps itself. It must be called
+ * and finish any rbtree manipulation before this code
+ * runs and also starts to manipulate the rbtree.
+ */
+ arch_unmap(mm, start, end);
+
/* Find the first overlapping VMA */
vma = find_vma(mm, start);
if (!vma)
@@ -2746,7 +2754,6 @@ int __do_munmap(struct mm_struct *mm, un
/* we have start < vma->vm_end */
/* if it doesn't overlap, we have nothing.. */
- end = start + len;
if (vma->vm_start >= end)
return 0;
@@ -2816,12 +2823,6 @@ int __do_munmap(struct mm_struct *mm, un
/* Detach vmas from rbtree */
detach_vmas_to_be_unmapped(mm, vma, prev, end);
- /*
- * mpx unmap needs to be called with mmap_sem held for write.
- * It is safe to call it before unmap_region().
- */
- arch_unmap(mm, vma, start, end);
-
if (downgrade)
downgrade_write(&mm->mmap_sem);
Patches currently in stable-queue which might be from dave.hansen@linux.intel.com are
queue-5.1/x86-mpx-mm-core-fix-recursive-munmap-corruption.patch
^ permalink raw reply
* Patch "x86/mpx, mm/core: Fix recursive munmap() corruption" has been added to the 5.0-stable tree
From: gregkh @ 2019-05-23 11:26 UTC (permalink / raw)
To: 20190419194747.5E1AD6DC, akpm, anton.ivanov, benh, bp,
dave.hansen, gregkh, gxt, hjl.tools, hpa, jdike, linux-mm,
linux-um, linuxppc-dev, luto, mhocko, mingo, mpe, paulus, peterz,
rguenther, richard, riel, tglx, torvalds, vbabka, yang.shi
Cc: stable-commits
This is a note to let you know that I've just added the patch titled
x86/mpx, mm/core: Fix recursive munmap() corruption
to the 5.0-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
x86-mpx-mm-core-fix-recursive-munmap-corruption.patch
and it can be found in the queue-5.0 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.
From 5a28fc94c9143db766d1ba5480cae82d856ad080 Mon Sep 17 00:00:00 2001
From: Dave Hansen <dave.hansen@linux.intel.com>
Date: Fri, 19 Apr 2019 12:47:47 -0700
Subject: x86/mpx, mm/core: Fix recursive munmap() corruption
From: Dave Hansen <dave.hansen@linux.intel.com>
commit 5a28fc94c9143db766d1ba5480cae82d856ad080 upstream.
This is a bit of a mess, to put it mildly. But, it's a bug
that only seems to have showed up in 4.20 but wasn't noticed
until now, because nobody uses MPX.
MPX has the arch_unmap() hook inside of munmap() because MPX
uses bounds tables that protect other areas of memory. When
memory is unmapped, there is also a need to unmap the MPX
bounds tables. Barring this, unused bounds tables can eat 80%
of the address space.
But, the recursive do_munmap() that gets called vi arch_unmap()
wreaks havoc with __do_munmap()'s state. It can result in
freeing populated page tables, accessing bogus VMA state,
double-freed VMAs and more.
See the "long story" further below for the gory details.
To fix this, call arch_unmap() before __do_unmap() has a chance
to do anything meaningful. Also, remove the 'vma' argument
and force the MPX code to do its own, independent VMA lookup.
== UML / unicore32 impact ==
Remove unused 'vma' argument to arch_unmap(). No functional
change.
I compile tested this on UML but not unicore32.
== powerpc impact ==
powerpc uses arch_unmap() well to watch for munmap() on the
VDSO and zeroes out 'current->mm->context.vdso_base'. Moving
arch_unmap() makes this happen earlier in __do_munmap(). But,
'vdso_base' seems to only be used in perf and in the signal
delivery that happens near the return to userspace. I can not
find any likely impact to powerpc, other than the zeroing
happening a little earlier.
powerpc does not use the 'vma' argument and is unaffected by
its removal.
I compile-tested a 64-bit powerpc defconfig.
== x86 impact ==
For the common success case this is functionally identical to
what was there before. For the munmap() failure case, it's
possible that some MPX tables will be zapped for memory that
continues to be in use. But, this is an extraordinarily
unlikely scenario and the harm would be that MPX provides no
protection since the bounds table got reset (zeroed).
I can't imagine anyone doing this:
ptr = mmap();
// use ptr
ret = munmap(ptr);
if (ret)
// oh, there was an error, I'll
// keep using ptr.
Because if you're doing munmap(), you are *done* with the
memory. There's probably no good data in there _anyway_.
This passes the original reproducer from Richard Biener as
well as the existing mpx selftests/.
The long story:
munmap() has a couple of pieces:
1. Find the affected VMA(s)
2. Split the start/end one(s) if neceesary
3. Pull the VMAs out of the rbtree
4. Actually zap the memory via unmap_region(), including
freeing page tables (or queueing them to be freed).
5. Fix up some of the accounting (like fput()) and actually
free the VMA itself.
This specific ordering was actually introduced by:
dd2283f2605e ("mm: mmap: zap pages with read mmap_sem in munmap")
during the 4.20 merge window. The previous __do_munmap() code
was actually safe because the only thing after arch_unmap() was
remove_vma_list(). arch_unmap() could not see 'vma' in the
rbtree because it was detached, so it is not even capable of
doing operations unsafe for remove_vma_list()'s use of 'vma'.
Richard Biener reported a test that shows this in dmesg:
[1216548.787498] BUG: Bad rss-counter state mm:0000000017ce560b idx:1 val:551
[1216548.787500] BUG: non-zero pgtables_bytes on freeing mm: 24576
What triggered this was the recursive do_munmap() called via
arch_unmap(). It was freeing page tables that has not been
properly zapped.
But, the problem was bigger than this. For one, arch_unmap()
can free VMAs. But, the calling __do_munmap() has variables
that *point* to VMAs and obviously can't handle them just
getting freed while the pointer is still in use.
I tried a couple of things here. First, I tried to fix the page
table freeing problem in isolation, but I then found the VMA
issue. I also tried having the MPX code return a flag if it
modified the rbtree which would force __do_munmap() to re-walk
to restart. That spiralled out of control in complexity pretty
fast.
Just moving arch_unmap() and accepting that the bonkers failure
case might eat some bounds tables seems like the simplest viable
fix.
This was also reported in the following kernel bugzilla entry:
https://bugzilla.kernel.org/show_bug.cgi?id=203123
There are some reports that this commit triggered this bug:
dd2283f2605 ("mm: mmap: zap pages with read mmap_sem in munmap")
While that commit certainly made the issues easier to hit, I believe
the fundamental issue has been with us as long as MPX itself, thus
the Fixes: tag below is for one of the original MPX commits.
[ mingo: Minor edits to the changelog and the patch. ]
Reported-by: Richard Biener <rguenther@suse.de>
Reported-by: H.J. Lu <hjl.tools@gmail.com>
Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Reviewed-by Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Yang Shi <yang.shi@linux.alibaba.com>
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Anton Ivanov <anton.ivanov@cambridgegreys.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Guan Xuetao <gxt@pku.edu.cn>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Jeff Dike <jdike@addtoit.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Richard Weinberger <richard@nod.at>
Cc: Rik van Riel <riel@surriel.com>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: linux-arch@vger.kernel.org
Cc: linux-mm@kvack.org
Cc: linux-um@lists.infradead.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: stable@vger.kernel.org
Fixes: dd2283f2605e ("mm: mmap: zap pages with read mmap_sem in munmap")
Link: http://lkml.kernel.org/r/20190419194747.5E1AD6DC@viggo.jf.intel.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
arch/powerpc/include/asm/mmu_context.h | 1 -
arch/um/include/asm/mmu_context.h | 1 -
arch/unicore32/include/asm/mmu_context.h | 1 -
arch/x86/include/asm/mmu_context.h | 6 +++---
arch/x86/include/asm/mpx.h | 15 ++++++++-------
arch/x86/mm/mpx.c | 10 ++++++----
include/asm-generic/mm_hooks.h | 1 -
mm/mmap.c | 15 ++++++++-------
8 files changed, 25 insertions(+), 25 deletions(-)
--- a/arch/powerpc/include/asm/mmu_context.h
+++ b/arch/powerpc/include/asm/mmu_context.h
@@ -237,7 +237,6 @@ extern void arch_exit_mmap(struct mm_str
#endif
static inline void arch_unmap(struct mm_struct *mm,
- struct vm_area_struct *vma,
unsigned long start, unsigned long end)
{
if (start <= mm->context.vdso_base && mm->context.vdso_base < end)
--- a/arch/um/include/asm/mmu_context.h
+++ b/arch/um/include/asm/mmu_context.h
@@ -22,7 +22,6 @@ static inline int arch_dup_mmap(struct m
}
extern void arch_exit_mmap(struct mm_struct *mm);
static inline void arch_unmap(struct mm_struct *mm,
- struct vm_area_struct *vma,
unsigned long start, unsigned long end)
{
}
--- a/arch/unicore32/include/asm/mmu_context.h
+++ b/arch/unicore32/include/asm/mmu_context.h
@@ -88,7 +88,6 @@ static inline int arch_dup_mmap(struct m
}
static inline void arch_unmap(struct mm_struct *mm,
- struct vm_area_struct *vma,
unsigned long start, unsigned long end)
{
}
--- a/arch/x86/include/asm/mmu_context.h
+++ b/arch/x86/include/asm/mmu_context.h
@@ -277,8 +277,8 @@ static inline void arch_bprm_mm_init(str
mpx_mm_init(mm);
}
-static inline void arch_unmap(struct mm_struct *mm, struct vm_area_struct *vma,
- unsigned long start, unsigned long end)
+static inline void arch_unmap(struct mm_struct *mm, unsigned long start,
+ unsigned long end)
{
/*
* mpx_notify_unmap() goes and reads a rarely-hot
@@ -298,7 +298,7 @@ static inline void arch_unmap(struct mm_
* consistently wrong.
*/
if (unlikely(cpu_feature_enabled(X86_FEATURE_MPX)))
- mpx_notify_unmap(mm, vma, start, end);
+ mpx_notify_unmap(mm, start, end);
}
/*
--- a/arch/x86/include/asm/mpx.h
+++ b/arch/x86/include/asm/mpx.h
@@ -64,12 +64,15 @@ struct mpx_fault_info {
};
#ifdef CONFIG_X86_INTEL_MPX
-int mpx_fault_info(struct mpx_fault_info *info, struct pt_regs *regs);
-int mpx_handle_bd_fault(void);
+
+extern int mpx_fault_info(struct mpx_fault_info *info, struct pt_regs *regs);
+extern int mpx_handle_bd_fault(void);
+
static inline int kernel_managing_mpx_tables(struct mm_struct *mm)
{
return (mm->context.bd_addr != MPX_INVALID_BOUNDS_DIR);
}
+
static inline void mpx_mm_init(struct mm_struct *mm)
{
/*
@@ -78,11 +81,10 @@ static inline void mpx_mm_init(struct mm
*/
mm->context.bd_addr = MPX_INVALID_BOUNDS_DIR;
}
-void mpx_notify_unmap(struct mm_struct *mm, struct vm_area_struct *vma,
- unsigned long start, unsigned long end);
-unsigned long mpx_unmapped_area_check(unsigned long addr, unsigned long len,
- unsigned long flags);
+extern void mpx_notify_unmap(struct mm_struct *mm, unsigned long start, unsigned long end);
+extern unsigned long mpx_unmapped_area_check(unsigned long addr, unsigned long len, unsigned long flags);
+
#else
static inline int mpx_fault_info(struct mpx_fault_info *info, struct pt_regs *regs)
{
@@ -100,7 +102,6 @@ static inline void mpx_mm_init(struct mm
{
}
static inline void mpx_notify_unmap(struct mm_struct *mm,
- struct vm_area_struct *vma,
unsigned long start, unsigned long end)
{
}
--- a/arch/x86/mm/mpx.c
+++ b/arch/x86/mm/mpx.c
@@ -881,9 +881,10 @@ static int mpx_unmap_tables(struct mm_st
* the virtual address region start...end have already been split if
* necessary, and the 'vma' is the first vma in this range (start -> end).
*/
-void mpx_notify_unmap(struct mm_struct *mm, struct vm_area_struct *vma,
- unsigned long start, unsigned long end)
+void mpx_notify_unmap(struct mm_struct *mm, unsigned long start,
+ unsigned long end)
{
+ struct vm_area_struct *vma;
int ret;
/*
@@ -902,11 +903,12 @@ void mpx_notify_unmap(struct mm_struct *
* which should not occur normally. Being strict about it here
* helps ensure that we do not have an exploitable stack overflow.
*/
- do {
+ vma = find_vma(mm, start);
+ while (vma && vma->vm_start < end) {
if (vma->vm_flags & VM_MPX)
return;
vma = vma->vm_next;
- } while (vma && vma->vm_start < end);
+ }
ret = mpx_unmap_tables(mm, start, end);
if (ret)
--- a/include/asm-generic/mm_hooks.h
+++ b/include/asm-generic/mm_hooks.h
@@ -18,7 +18,6 @@ static inline void arch_exit_mmap(struct
}
static inline void arch_unmap(struct mm_struct *mm,
- struct vm_area_struct *vma,
unsigned long start, unsigned long end)
{
}
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2736,9 +2736,17 @@ int __do_munmap(struct mm_struct *mm, un
return -EINVAL;
len = PAGE_ALIGN(len);
+ end = start + len;
if (len == 0)
return -EINVAL;
+ /*
+ * arch_unmap() might do unmaps itself. It must be called
+ * and finish any rbtree manipulation before this code
+ * runs and also starts to manipulate the rbtree.
+ */
+ arch_unmap(mm, start, end);
+
/* Find the first overlapping VMA */
vma = find_vma(mm, start);
if (!vma)
@@ -2747,7 +2755,6 @@ int __do_munmap(struct mm_struct *mm, un
/* we have start < vma->vm_end */
/* if it doesn't overlap, we have nothing.. */
- end = start + len;
if (vma->vm_start >= end)
return 0;
@@ -2817,12 +2824,6 @@ int __do_munmap(struct mm_struct *mm, un
/* Detach vmas from rbtree */
detach_vmas_to_be_unmapped(mm, vma, prev, end);
- /*
- * mpx unmap needs to be called with mmap_sem held for write.
- * It is safe to call it before unmap_region().
- */
- arch_unmap(mm, vma, start, end);
-
if (downgrade)
downgrade_write(&mm->mmap_sem);
Patches currently in stable-queue which might be from dave.hansen@linux.intel.com are
queue-5.0/x86-mpx-mm-core-fix-recursive-munmap-corruption.patch
^ permalink raw reply
* [PATCH v4 3/3] kselftest: Extend vDSO selftest to clock_getres
From: Vincenzo Frascino @ 2019-05-23 11:21 UTC (permalink / raw)
To: linux-arch, linuxppc-dev, linux-s390, linux-kselftest
Cc: Arnd Bergmann, Heiko Carstens, Paul Mackerras, Martin Schwidefsky,
Thomas Gleixner, vincenzo.frascino, Shuah Khan
In-Reply-To: <20190523112116.19233-1-vincenzo.frascino@arm.com>
The current version of the multiarch vDSO selftest verifies only
gettimeofday.
Extend the vDSO selftest to clock_getres, to verify that the
syscall and the vDSO library function return the same information.
The extension has been used to verify the hrtimer_resoltion fix.
Cc: Shuah Khan <shuah@kernel.org>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
---
Note: This patch is independent from the others in this series, hence it
can be merged singularly by the kselftest maintainers.
tools/testing/selftests/vDSO/Makefile | 2 +
.../selftests/vDSO/vdso_clock_getres.c | 124 ++++++++++++++++++
2 files changed, 126 insertions(+)
create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile
index 9e03d61f52fd..d5c5bfdf1ac1 100644
--- a/tools/testing/selftests/vDSO/Makefile
+++ b/tools/testing/selftests/vDSO/Makefile
@@ -5,6 +5,7 @@ uname_M := $(shell uname -m 2>/dev/null || echo not)
ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/)
TEST_GEN_PROGS := $(OUTPUT)/vdso_test
+TEST_GEN_PROGS += $(OUTPUT)/vdso_clock_getres
ifeq ($(ARCH),x86)
TEST_GEN_PROGS += $(OUTPUT)/vdso_standalone_test_x86
endif
@@ -18,6 +19,7 @@ endif
all: $(TEST_GEN_PROGS)
$(OUTPUT)/vdso_test: parse_vdso.c vdso_test.c
+$(OUTPUT)/vdso_clock_getres: vdso_clock_getres.c
$(OUTPUT)/vdso_standalone_test_x86: vdso_standalone_test_x86.c parse_vdso.c
$(CC) $(CFLAGS) $(CFLAGS_vdso_standalone_test_x86) \
vdso_standalone_test_x86.c parse_vdso.c \
diff --git a/tools/testing/selftests/vDSO/vdso_clock_getres.c b/tools/testing/selftests/vDSO/vdso_clock_getres.c
new file mode 100644
index 000000000000..b62d8d4f7c38
--- /dev/null
+++ b/tools/testing/selftests/vDSO/vdso_clock_getres.c
@@ -0,0 +1,124 @@
+// SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note
+/*
+ * vdso_clock_getres.c: Sample code to test clock_getres.
+ * Copyright (c) 2019 Arm Ltd.
+ *
+ * Compile with:
+ * gcc -std=gnu99 vdso_clock_getres.c
+ *
+ * Tested on ARM, ARM64, MIPS32, x86 (32-bit and 64-bit),
+ * Power (32-bit and 64-bit), S390x (32-bit and 64-bit).
+ * Might work on other architectures.
+ */
+
+#define _GNU_SOURCE
+#include <elf.h>
+#include <err.h>
+#include <fcntl.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <time.h>
+#include <sys/auxv.h>
+#include <sys/mman.h>
+#include <sys/time.h>
+#include <unistd.h>
+#include <sys/syscall.h>
+
+#include "../kselftest.h"
+
+static long syscall_clock_getres(clockid_t _clkid, struct timespec *_ts)
+{
+ long ret;
+
+ ret = syscall(SYS_clock_getres, _clkid, _ts);
+
+ return ret;
+}
+
+const char *vdso_clock_name[12] = {
+ "CLOCK_REALTIME",
+ "CLOCK_MONOTONIC",
+ "CLOCK_PROCESS_CPUTIME_ID",
+ "CLOCK_THREAD_CPUTIME_ID",
+ "CLOCK_MONOTONIC_RAW",
+ "CLOCK_REALTIME_COARSE",
+ "CLOCK_MONOTONIC_COARSE",
+ "CLOCK_BOOTTIME",
+ "CLOCK_REALTIME_ALARM",
+ "CLOCK_BOOTTIME_ALARM",
+ "CLOCK_SGI_CYCLE",
+ "CLOCK_TAI",
+};
+
+/*
+ * This function calls clock_getres in vdso and by system call
+ * with different values for clock_id.
+ *
+ * Example of output:
+ *
+ * clock_id: CLOCK_REALTIME [PASS]
+ * clock_id: CLOCK_BOOTTIME [PASS]
+ * clock_id: CLOCK_TAI [PASS]
+ * clock_id: CLOCK_REALTIME_COARSE [PASS]
+ * clock_id: CLOCK_MONOTONIC [PASS]
+ * clock_id: CLOCK_MONOTONIC_RAW [PASS]
+ * clock_id: CLOCK_MONOTONIC_COARSE [PASS]
+ */
+static inline int vdso_test_clock(unsigned int clock_id)
+{
+ struct timespec x, y;
+
+ printf("clock_id: %s", vdso_clock_name[clock_id]);
+ clock_getres(clock_id, &x);
+ syscall_clock_getres(clock_id, &y);
+
+ if ((x.tv_sec != y.tv_sec) || (x.tv_sec != y.tv_sec)) {
+ printf(" [FAIL]\n");
+ return KSFT_FAIL;
+ }
+
+ printf(" [PASS]\n");
+ return KSFT_PASS;
+}
+
+int main(int argc, char **argv)
+{
+ int ret;
+
+#if _POSIX_TIMERS > 0
+
+#ifdef CLOCK_REALTIME
+ ret = vdso_test_clock(CLOCK_REALTIME);
+#endif
+
+#ifdef CLOCK_BOOTTIME
+ ret += vdso_test_clock(CLOCK_BOOTTIME);
+#endif
+
+#ifdef CLOCK_TAI
+ ret += vdso_test_clock(CLOCK_TAI);
+#endif
+
+#ifdef CLOCK_REALTIME_COARSE
+ ret += vdso_test_clock(CLOCK_REALTIME_COARSE);
+#endif
+
+#ifdef CLOCK_MONOTONIC
+ ret += vdso_test_clock(CLOCK_MONOTONIC);
+#endif
+
+#ifdef CLOCK_MONOTONIC_RAW
+ ret += vdso_test_clock(CLOCK_MONOTONIC_RAW);
+#endif
+
+#ifdef CLOCK_MONOTONIC_COARSE
+ ret += vdso_test_clock(CLOCK_MONOTONIC_COARSE);
+#endif
+
+#endif
+ if (ret > 0)
+ return KSFT_FAIL;
+
+ return KSFT_PASS;
+}
--
2.21.0
^ permalink raw reply related
* [PATCH v4 2/3] s390: Fix vDSO clock_getres()
From: Vincenzo Frascino @ 2019-05-23 11:21 UTC (permalink / raw)
To: linux-arch, linuxppc-dev, linux-s390, linux-kselftest
Cc: Arnd Bergmann, Heiko Carstens, Paul Mackerras, Martin Schwidefsky,
Thomas Gleixner, vincenzo.frascino, Shuah Khan
In-Reply-To: <20190523112116.19233-1-vincenzo.frascino@arm.com>
clock_getres in the vDSO library has to preserve the same behaviour
of posix_get_hrtimer_res().
In particular, posix_get_hrtimer_res() does:
sec = 0;
ns = hrtimer_resolution;
and hrtimer_resolution depends on the enablement of the high
resolution timers that can happen either at compile or at run time.
Fix the s390 vdso implementation of clock_getres keeping a copy of
hrtimer_resolution in vdso data and using that directly.
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Acked-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---
Note: This patch is independent from the others in this series, hence it
can be merged singularly by the s390 maintainers.
arch/s390/include/asm/vdso.h | 1 +
arch/s390/kernel/asm-offsets.c | 2 +-
arch/s390/kernel/time.c | 1 +
arch/s390/kernel/vdso32/clock_getres.S | 12 +++++++-----
arch/s390/kernel/vdso64/clock_getres.S | 10 +++++-----
5 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/arch/s390/include/asm/vdso.h b/arch/s390/include/asm/vdso.h
index 169d7604eb80..f3ba84fa9bd1 100644
--- a/arch/s390/include/asm/vdso.h
+++ b/arch/s390/include/asm/vdso.h
@@ -36,6 +36,7 @@ struct vdso_data {
__u32 tk_shift; /* Shift used for xtime_nsec 0x60 */
__u32 ts_dir; /* TOD steering direction 0x64 */
__u64 ts_end; /* TOD steering end 0x68 */
+ __u32 hrtimer_res; /* hrtimer resolution 0x70 */
};
struct vdso_per_cpu_data {
diff --git a/arch/s390/kernel/asm-offsets.c b/arch/s390/kernel/asm-offsets.c
index 41ac4ad21311..4a229a60b24a 100644
--- a/arch/s390/kernel/asm-offsets.c
+++ b/arch/s390/kernel/asm-offsets.c
@@ -76,6 +76,7 @@ int main(void)
OFFSET(__VDSO_TK_SHIFT, vdso_data, tk_shift);
OFFSET(__VDSO_TS_DIR, vdso_data, ts_dir);
OFFSET(__VDSO_TS_END, vdso_data, ts_end);
+ OFFSET(__VDSO_CLOCK_REALTIME_RES, vdso_data, hrtimer_res);
OFFSET(__VDSO_ECTG_BASE, vdso_per_cpu_data, ectg_timer_base);
OFFSET(__VDSO_ECTG_USER, vdso_per_cpu_data, ectg_user_time);
OFFSET(__VDSO_CPU_NR, vdso_per_cpu_data, cpu_nr);
@@ -87,7 +88,6 @@ int main(void)
DEFINE(__CLOCK_REALTIME_COARSE, CLOCK_REALTIME_COARSE);
DEFINE(__CLOCK_MONOTONIC_COARSE, CLOCK_MONOTONIC_COARSE);
DEFINE(__CLOCK_THREAD_CPUTIME_ID, CLOCK_THREAD_CPUTIME_ID);
- DEFINE(__CLOCK_REALTIME_RES, MONOTONIC_RES_NSEC);
DEFINE(__CLOCK_COARSE_RES, LOW_RES_NSEC);
BLANK();
/* idle data offsets */
diff --git a/arch/s390/kernel/time.c b/arch/s390/kernel/time.c
index e8766beee5ad..8ea9db599d38 100644
--- a/arch/s390/kernel/time.c
+++ b/arch/s390/kernel/time.c
@@ -310,6 +310,7 @@ void update_vsyscall(struct timekeeper *tk)
vdso_data->tk_mult = tk->tkr_mono.mult;
vdso_data->tk_shift = tk->tkr_mono.shift;
+ vdso_data->hrtimer_res = hrtimer_resolution;
smp_wmb();
++vdso_data->tb_update_count;
}
diff --git a/arch/s390/kernel/vdso32/clock_getres.S b/arch/s390/kernel/vdso32/clock_getres.S
index eaf9cf1417f6..fecd7684c645 100644
--- a/arch/s390/kernel/vdso32/clock_getres.S
+++ b/arch/s390/kernel/vdso32/clock_getres.S
@@ -18,20 +18,22 @@
__kernel_clock_getres:
CFI_STARTPROC
basr %r1,0
- la %r1,4f-.(%r1)
+10: al %r1,4f-10b(%r1)
+ l %r0,__VDSO_CLOCK_REALTIME_RES(%r1)
chi %r2,__CLOCK_REALTIME
je 0f
chi %r2,__CLOCK_MONOTONIC
je 0f
- la %r1,5f-4f(%r1)
+ basr %r1,0
+ la %r1,5f-.(%r1)
+ l %r0,0(%r1)
chi %r2,__CLOCK_REALTIME_COARSE
je 0f
chi %r2,__CLOCK_MONOTONIC_COARSE
jne 3f
0: ltr %r3,%r3
jz 2f /* res == NULL */
-1: l %r0,0(%r1)
- xc 0(4,%r3),0(%r3) /* set tp->tv_sec to zero */
+1: xc 0(4,%r3),0(%r3) /* set tp->tv_sec to zero */
st %r0,4(%r3) /* store tp->tv_usec */
2: lhi %r2,0
br %r14
@@ -39,6 +41,6 @@ __kernel_clock_getres:
svc 0
br %r14
CFI_ENDPROC
-4: .long __CLOCK_REALTIME_RES
+4: .long _vdso_data - 10b
5: .long __CLOCK_COARSE_RES
.size __kernel_clock_getres,.-__kernel_clock_getres
diff --git a/arch/s390/kernel/vdso64/clock_getres.S b/arch/s390/kernel/vdso64/clock_getres.S
index 081435398e0a..022b58c980db 100644
--- a/arch/s390/kernel/vdso64/clock_getres.S
+++ b/arch/s390/kernel/vdso64/clock_getres.S
@@ -17,12 +17,14 @@
.type __kernel_clock_getres,@function
__kernel_clock_getres:
CFI_STARTPROC
- larl %r1,4f
+ larl %r1,3f
+ lg %r0,0(%r1)
cghi %r2,__CLOCK_REALTIME_COARSE
je 0f
cghi %r2,__CLOCK_MONOTONIC_COARSE
je 0f
- larl %r1,3f
+ larl %r1,_vdso_data
+ l %r0,__VDSO_CLOCK_REALTIME_RES(%r1)
cghi %r2,__CLOCK_REALTIME
je 0f
cghi %r2,__CLOCK_MONOTONIC
@@ -36,7 +38,6 @@ __kernel_clock_getres:
jz 2f
0: ltgr %r3,%r3
jz 1f /* res == NULL */
- lg %r0,0(%r1)
xc 0(8,%r3),0(%r3) /* set tp->tv_sec to zero */
stg %r0,8(%r3) /* store tp->tv_usec */
1: lghi %r2,0
@@ -45,6 +46,5 @@ __kernel_clock_getres:
svc 0
br %r14
CFI_ENDPROC
-3: .quad __CLOCK_REALTIME_RES
-4: .quad __CLOCK_COARSE_RES
+3: .quad __CLOCK_COARSE_RES
.size __kernel_clock_getres,.-__kernel_clock_getres
--
2.21.0
^ permalink raw reply related
* [PATCH v4 1/3] powerpc: Fix vDSO clock_getres()
From: Vincenzo Frascino @ 2019-05-23 11:21 UTC (permalink / raw)
To: linux-arch, linuxppc-dev, linux-s390, linux-kselftest
Cc: Arnd Bergmann, Heiko Carstens, stable, Paul Mackerras,
Martin Schwidefsky, Thomas Gleixner, vincenzo.frascino,
Shuah Khan
In-Reply-To: <20190523112116.19233-1-vincenzo.frascino@arm.com>
clock_getres in the vDSO library has to preserve the same behaviour
of posix_get_hrtimer_res().
In particular, posix_get_hrtimer_res() does:
sec = 0;
ns = hrtimer_resolution;
and hrtimer_resolution depends on the enablement of the high
resolution timers that can happen either at compile or at run time.
Fix the powerpc vdso implementation of clock_getres keeping a copy of
hrtimer_resolution in vdso data and using that directly.
Fixes: a7f290dad32e ("[PATCH] powerpc: Merge vdso's and add vdso support
to 32 bits kernel")
Cc: stable@vger.kernel.org
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Reviewed-by: Christophe Leroy <christophe.leroy@c-s.fr>
---
Note: This patch is independent from the others in this series, hence it
can be merged singularly by the powerpc maintainers.
arch/powerpc/include/asm/vdso_datapage.h | 2 ++
arch/powerpc/kernel/asm-offsets.c | 2 +-
arch/powerpc/kernel/time.c | 1 +
arch/powerpc/kernel/vdso32/gettimeofday.S | 7 +++++--
arch/powerpc/kernel/vdso64/gettimeofday.S | 7 +++++--
5 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/include/asm/vdso_datapage.h b/arch/powerpc/include/asm/vdso_datapage.h
index bbc06bd72b1f..4333b9a473dc 100644
--- a/arch/powerpc/include/asm/vdso_datapage.h
+++ b/arch/powerpc/include/asm/vdso_datapage.h
@@ -86,6 +86,7 @@ struct vdso_data {
__s32 wtom_clock_nsec; /* Wall to monotonic clock nsec */
__s64 wtom_clock_sec; /* Wall to monotonic clock sec */
struct timespec stamp_xtime; /* xtime as at tb_orig_stamp */
+ __u32 hrtimer_res; /* hrtimer resolution */
__u32 syscall_map_64[SYSCALL_MAP_SIZE]; /* map of syscalls */
__u32 syscall_map_32[SYSCALL_MAP_SIZE]; /* map of syscalls */
};
@@ -107,6 +108,7 @@ struct vdso_data {
__s32 wtom_clock_nsec;
struct timespec stamp_xtime; /* xtime as at tb_orig_stamp */
__u32 stamp_sec_fraction; /* fractional seconds of stamp_xtime */
+ __u32 hrtimer_res; /* hrtimer resolution */
__u32 syscall_map_32[SYSCALL_MAP_SIZE]; /* map of syscalls */
__u32 dcache_block_size; /* L1 d-cache block size */
__u32 icache_block_size; /* L1 i-cache block size */
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index 8e02444e9d3d..dfc40f29f2b9 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -389,6 +389,7 @@ int main(void)
OFFSET(WTOM_CLOCK_NSEC, vdso_data, wtom_clock_nsec);
OFFSET(STAMP_XTIME, vdso_data, stamp_xtime);
OFFSET(STAMP_SEC_FRAC, vdso_data, stamp_sec_fraction);
+ OFFSET(CLOCK_REALTIME_RES, vdso_data, hrtimer_res);
OFFSET(CFG_ICACHE_BLOCKSZ, vdso_data, icache_block_size);
OFFSET(CFG_DCACHE_BLOCKSZ, vdso_data, dcache_block_size);
OFFSET(CFG_ICACHE_LOGBLOCKSZ, vdso_data, icache_log_block_size);
@@ -419,7 +420,6 @@ int main(void)
DEFINE(CLOCK_REALTIME_COARSE, CLOCK_REALTIME_COARSE);
DEFINE(CLOCK_MONOTONIC_COARSE, CLOCK_MONOTONIC_COARSE);
DEFINE(NSEC_PER_SEC, NSEC_PER_SEC);
- DEFINE(CLOCK_REALTIME_RES, MONOTONIC_RES_NSEC);
#ifdef CONFIG_BUG
DEFINE(BUG_ENTRY_SIZE, sizeof(struct bug_entry));
diff --git a/arch/powerpc/kernel/time.c b/arch/powerpc/kernel/time.c
index 325d60633dfa..4ea4e9d7a58e 100644
--- a/arch/powerpc/kernel/time.c
+++ b/arch/powerpc/kernel/time.c
@@ -963,6 +963,7 @@ void update_vsyscall(struct timekeeper *tk)
vdso_data->wtom_clock_nsec = tk->wall_to_monotonic.tv_nsec;
vdso_data->stamp_xtime = xt;
vdso_data->stamp_sec_fraction = frac_sec;
+ vdso_data->hrtimer_res = hrtimer_resolution;
smp_wmb();
++(vdso_data->tb_update_count);
}
diff --git a/arch/powerpc/kernel/vdso32/gettimeofday.S b/arch/powerpc/kernel/vdso32/gettimeofday.S
index afd516b572f8..2b5f9e83c610 100644
--- a/arch/powerpc/kernel/vdso32/gettimeofday.S
+++ b/arch/powerpc/kernel/vdso32/gettimeofday.S
@@ -160,12 +160,15 @@ V_FUNCTION_BEGIN(__kernel_clock_getres)
cror cr0*4+eq,cr0*4+eq,cr1*4+eq
bne cr0,99f
+ mflr r12
+ .cfi_register lr,r12
+ bl __get_datapage@local
+ lwz r5,CLOCK_REALTIME_RES(r3)
+ mtlr r12
li r3,0
cmpli cr0,r4,0
crclr cr0*4+so
beqlr
- lis r5,CLOCK_REALTIME_RES@h
- ori r5,r5,CLOCK_REALTIME_RES@l
stw r3,TSPC32_TV_SEC(r4)
stw r5,TSPC32_TV_NSEC(r4)
blr
diff --git a/arch/powerpc/kernel/vdso64/gettimeofday.S b/arch/powerpc/kernel/vdso64/gettimeofday.S
index 1f324c28705b..f07730f73d5e 100644
--- a/arch/powerpc/kernel/vdso64/gettimeofday.S
+++ b/arch/powerpc/kernel/vdso64/gettimeofday.S
@@ -190,12 +190,15 @@ V_FUNCTION_BEGIN(__kernel_clock_getres)
cror cr0*4+eq,cr0*4+eq,cr1*4+eq
bne cr0,99f
+ mflr r12
+ .cfi_register lr,r12
+ bl V_LOCAL_FUNC(__get_datapage)
+ lwz r5,CLOCK_REALTIME_RES(r3)
+ mtlr r12
li r3,0
cmpldi cr0,r4,0
crclr cr0*4+so
beqlr
- lis r5,CLOCK_REALTIME_RES@h
- ori r5,r5,CLOCK_REALTIME_RES@l
std r3,TSPC64_TV_SEC(r4)
std r5,TSPC64_TV_NSEC(r4)
blr
--
2.21.0
^ permalink raw reply related
* [PATCH v4 0/3] Fix vDSO clock_getres()
From: Vincenzo Frascino @ 2019-05-23 11:21 UTC (permalink / raw)
To: linux-arch, linuxppc-dev, linux-s390, linux-kselftest
Cc: Arnd Bergmann, Heiko Carstens, Paul Mackerras, Martin Schwidefsky,
Thomas Gleixner, vincenzo.frascino, Shuah Khan
clock_getres in the vDSO library has to preserve the same behaviour
of posix_get_hrtimer_res().
In particular, posix_get_hrtimer_res() does:
sec = 0;
ns = hrtimer_resolution;
and hrtimer_resolution depends on the enablement of the high
resolution timers that can happen either at compile or at run time.
A possible fix is to change the vdso implementation of clock_getres,
keeping a copy of hrtimer_resolution in vdso data and using that
directly [1].
This patchset implements the proposed fix for arm64, powerpc, s390,
nds32 and adds a test to verify that the syscall and the vdso library
implementation of clock_getres return the same values.
Even if these patches are unified by the same topic, there is no
dependency between them, hence they can be merged singularly by each
arch maintainer.
Note: arm64 and nds32 respective fixes have been merged in 5.2-rc1,
hence they have been removed from this series.
[1] https://marc.info/?l=linux-arm-kernel&m=155110381930196&w=2
Changes:
--------
v4:
- Address review comments.
v3:
- Rebased on 5.2-rc1.
- Address review comments.
v2:
- Rebased on 5.1-rc5.
- Addressed review comments.
Cc: Christophe Leroy <christophe.leroy@c-s.fr>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Vincenzo Frascino (3):
powerpc: Fix vDSO clock_getres()
s390: Fix vDSO clock_getres()
kselftest: Extend vDSO selftest to clock_getres
arch/powerpc/include/asm/vdso_datapage.h | 2 +
arch/powerpc/kernel/asm-offsets.c | 2 +-
arch/powerpc/kernel/time.c | 1 +
arch/powerpc/kernel/vdso32/gettimeofday.S | 7 +-
arch/powerpc/kernel/vdso64/gettimeofday.S | 7 +-
arch/s390/include/asm/vdso.h | 1 +
arch/s390/kernel/asm-offsets.c | 2 +-
arch/s390/kernel/time.c | 1 +
arch/s390/kernel/vdso32/clock_getres.S | 12 +-
arch/s390/kernel/vdso64/clock_getres.S | 10 +-
tools/testing/selftests/vDSO/Makefile | 2 +
.../selftests/vDSO/vdso_clock_getres.c | 124 ++++++++++++++++++
12 files changed, 155 insertions(+), 16 deletions(-)
create mode 100644 tools/testing/selftests/vDSO/vdso_clock_getres.c
--
2.21.0
^ permalink raw reply
* Re: [PATCH] ASoC: fsl_esai: fix the channel swap issue after xrun
From: S.j. Wang @ 2019-05-23 11:04 UTC (permalink / raw)
To: Nicolin Chen, broonie@kernel.org
Cc: alsa-devel@alsa-project.org, timur@kernel.org,
Xiubo.Lee@gmail.com, festevam@gmail.com,
linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
Hi
> On Thu, May 23, 2019 at 09:53:42AM +0000, S.j. Wang wrote:
> > > > + /*
> > > > + * Add fifo reset here, because the regcache_sync will
> > > > + * write one more data to ETDR.
> > > > + * Which will cause channel shift.
> > >
> > > Sounds like a bug to me...should fix it first by marking the data
> > > registers as volatile.
> > >
> > The ETDR is a writable register, it is not volatile. Even we change it
> > to Volatile, I don't think we can't avoid this issue. for the
> > regcache_sync Just to write this register, it is correct behavior.
>
> Is that so? Quoting the comments of regcache_sync():
> "* regcache_sync - Sync the register cache with the hardware.
> *
> * @map: map to configure.
> *
> * Any registers that should not be synced should be marked as
> * volatile."
>
> If regcache_sync() does sync volatile registers too as you said, I don't mind
> having this FIFO reset WAR for now, though I think this mismatch between
> the comments and the actual behavior then should get people's attention.
>
> Thank you
ETDR is not volatile, if we mark it is volatile, is it correct?
Bets regards
Wang shengjiu
^ permalink raw reply
* [PATCH] powerpc: Remove variable ‘path’ since not used
From: Mathieu Malaterre @ 2019-05-23 10:25 UTC (permalink / raw)
To: Michael Ellerman
Cc: Mathieu Malaterre, Paul Mackerras, linuxppc-dev, linux-kernel
In commit eab00a208eb6 ("powerpc: Move `path` variable inside
DEBUG_PROM") DEBUG_PROM sentinels were added to silence a warning
(treated as error with W=1):
arch/powerpc/kernel/prom_init.c:1388:8: error: variable ‘path’ set but not used [-Werror=unused-but-set-variable]
Rework the original patch and simplify the code, by removing the
variable ‘path’ completely. Fix line over 90 characters.
Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Mathieu Malaterre <malat@debian.org>
---
arch/powerpc/kernel/prom_init.c | 13 ++++---------
1 file changed, 4 insertions(+), 9 deletions(-)
diff --git a/arch/powerpc/kernel/prom_init.c b/arch/powerpc/kernel/prom_init.c
index 7edb23861162..f6df4ddebb82 100644
--- a/arch/powerpc/kernel/prom_init.c
+++ b/arch/powerpc/kernel/prom_init.c
@@ -1566,9 +1566,6 @@ static void __init reserve_mem(u64 base, u64 size)
static void __init prom_init_mem(void)
{
phandle node;
-#ifdef DEBUG_PROM
- char *path;
-#endif
char type[64];
unsigned int plen;
cell_t *p, *endp;
@@ -1590,9 +1587,6 @@ static void __init prom_init_mem(void)
prom_debug("root_size_cells: %x\n", rsc);
prom_debug("scanning memory:\n");
-#ifdef DEBUG_PROM
- path = prom_scratch;
-#endif
for (node = 0; prom_next_node(&node); ) {
type[0] = 0;
@@ -1617,9 +1611,10 @@ static void __init prom_init_mem(void)
endp = p + (plen / sizeof(cell_t));
#ifdef DEBUG_PROM
- memset(path, 0, sizeof(prom_scratch));
- call_prom("package-to-path", 3, 1, node, path, sizeof(prom_scratch) - 1);
- prom_debug(" node %s :\n", path);
+ memset(prom_scratch, 0, sizeof(prom_scratch));
+ call_prom("package-to-path", 3, 1, node, prom_scratch,
+ sizeof(prom_scratch) - 1);
+ prom_debug(" node %s :\n", prom_scratch);
#endif /* DEBUG_PROM */
while ((endp - p) >= (rac + rsc)) {
--
2.20.1
^ permalink raw reply related
* Re: Failure to boot G4: dt_headr_start=0x01501000
From: Christophe Leroy @ 2019-05-23 10:18 UTC (permalink / raw)
To: Mathieu Malaterre; +Cc: linuxppc-dev, Daniel Axtens
In-Reply-To: <158e1855-62ef-baf6-4fff-f28131a7e095@c-s.fr>
On 05/23/2019 10:05 AM, Christophe Leroy wrote:
>
>
> On 05/23/2019 09:59 AM, Christophe Leroy wrote:
>>
>>
>> On 05/23/2019 09:45 AM, Christophe Leroy wrote:
>>>
>>>
>>> Le 23/05/2019 à 10:53, Mathieu Malaterre a écrit :
>>>>> Commit id is:
>>>>>
>>>>> e93c9c99a629 (tag: v5.1) Linux 5.1
>>>>>
>>>>>> Did you try latest powerpc/merge branch ?
>>>>>
>>>>> Will try that next.
>>>>
>>>> I confirm powerpc/merge does not boot for me (same config). Commit id:
>>>>
>>>> a27eaa62326d (powerpc/merge) Automatic merge of branches 'master',
>>>> 'next' and 'fixes' into merge
>>>
>>> I see in the config you sent me that you have selected CONFIG_KASAN,
>>> which is a big new stuff.
>>>
>>> Can you try without it ?
>>
>> While building with your config, I get a huge amount of:
>>
>> ppc-linux-ld: warning: orphan section `.data..LASAN0' from
>> `lib/xarray.o' being placed in section `.data..LASAN0'.
>> SORTEX vmlinux
>>
>>
>>
>> I see you have also selected CONFIG_LD_DEAD_CODE_DATA_ELIMINATION=y
>>
>> I guess nobody have never tried both this and CONFIG_KASAN together on
>> ppc32. I'll give it a try.
>
>
> And you also have CONFIG_FTRACE.
>
> In a recent patch implementing KASAN on PPC64, Daniel says that KASAN
> and FTRACE don't go together well
> (https://patchwork.ozlabs.org/patch/1103826/)
>
> If you find out that it works without KASAN, can you then try with KASAN
> but without FTRACE ?
>
I tried your config in Qemu, looks I'm getting a recursive Oops:
#50 0xc0066af0 in do_exit (code=0xb) at kernel/exit.c:787
#51 0xc0013984 in oops_end (flags=<optimized out>, regs=<optimized out>,
signr=0xb) at arch/powerpc/kernel/traps.c:253
#52 0xc001c30c in handle_page_fault () at arch/powerpc/kernel/entry_32.S:637
#53 0x20302e30 in ?? ()
#54 0xc001cb60 in btext_drawchar (c=0x0) at arch/powerpc/kernel/btext.c:522
#55 0xc00167cc in udbg_write (s=0xc113ae22 <text+2> " 0.000000] CPU: 0
PID: 0 Comm: swapper Not tainted 5.1.0+ #1647\n0\n", n=0x37) at
arch/powerpc/kernel/udbg.c:114
#56 0xc00d43f0 in call_console_drivers (ext_text=<optimized out>,
text=<optimized out>, len=<optimized out>, ext_len=<optimized out>) at
kernel/printk/printk.c:1780
#57 console_unlock () at kernel/printk/printk.c:2462
#58 0xc00d6630 in console_flush_on_panic () at kernel/printk/printk.c:2552
#59 0xc00618a0 in panic (fmt=0xc10f459f <buf+31> "!") at kernel/panic.c:280
#60 0xc0066af0 in do_exit (code=0xb) at kernel/exit.c:787
#61 0xc0013984 in oops_end (flags=<optimized out>, regs=<optimized out>,
signr=0xb) at arch/powerpc/kernel/traps.c:253
#62 0xc001c30c in handle_page_fault () at arch/powerpc/kernel/entry_32.S:637
#63 0x20302e30 in ?? ()
#64 0xc001cb60 in btext_drawchar (c=0x0) at arch/powerpc/kernel/btext.c:522
#65 0xc00167cc in udbg_write (s=0xc113ae22 <text+2> " 0.000000] CPU: 0
PID: 0 Comm: swapper Not tainted 5.1.0+ #1647\n0\n", n=0x45) at
arch/powerpc/kernel/udbg.c:114
#66 0xc00d43f0 in call_console_drivers (ext_text=<optimized out>,
text=<optimized out>, len=<optimized out>, ext_len=<optimized out>) at
kernel/printk/printk.c:1780
#67 console_unlock () at kernel/printk/printk.c:2462
#68 0xc00d6630 in console_flush_on_panic () at kernel/printk/printk.c:2552
#69 0xc00618a0 in panic (fmt=0xc10f459f <buf+31> "!") at kernel/panic.c:280
#70 0xc0066af0 in do_exit (code=0xb) at kernel/exit.c:787
#71 0xc0013984 in oops_end (flags=<optimized out>, regs=<optimized out>,
signr=0xb) at arch/powerpc/kernel/traps.c:253
#72 0xc001c30c in handle_page_fault () at arch/powerpc/kernel/entry_32.S:637
#73 0x20302e30 in ?? ()
#74 0xc001cb60 in btext_drawchar (c=0x0) at arch/powerpc/kernel/btext.c:522
#75 0xc00167cc in udbg_write (s=0xc113ae22 <text+2> " 0.000000] CPU: 0
PID: 0 Comm: swapper Not tainted 5.1.0+ #1647\n0\n", n=0x32) at
arch/powerpc/kernel/udbg.c:114
#76 0xc00d43f0 in call_console_drivers (ext_text=<optimized out>,
text=<optimized out>, len=<optimized out>, ext_len=<optimized out>) at
kernel/printk/printk.c:1780
#77 console_unlock () at kernel/printk/printk.c:2462
#78 0xc00d68d8 in vprintk_emit (facility=<optimized out>,
level=<optimized out>, dict=0x0, dictlen=0x0, fmt=0xc085e4c0
"\001\066printk: %sconsole [%s%d] enabled\n",
args=0xc10cff30) at kernel/printk/printk.c:1985
#79 0xc00d69d8 in vprintk_default (fmt=<optimized out>, args=<optimized
out>) at kernel/printk/printk.c:2012
#80 0xc00d7a40 in vprintk_func (fmt=<optimized out>, args=<optimized
out>) at kernel/printk/printk_safe.c:398
#81 0xc00d2638 in printk (fmt=<optimized out>) at
kernel/printk/printk.c:2045
#82 0xc00d4ef8 in register_console (newcon=0xc0cb9a20 <udbg_console>) at
kernel/printk/printk.c:2777
#83 0xc0b79ed0 in machine_init (dt_ptr=<optimized out>) at
arch/powerpc/kernel/setup_32.c:83
#84 0xc000347c in start_here () at arch/powerpc/kernel/head_32.S:901
Christophe
^ permalink raw reply
* Re: Failure to boot G4: dt_headr_start=0x01501000
From: Mathieu Malaterre @ 2019-05-23 10:16 UTC (permalink / raw)
To: Christophe Leroy; +Cc: linuxppc-dev
In-Reply-To: <9e5ef44d-259a-1f1a-bd6a-98abdae85da0@c-s.fr>
On Thu, May 23, 2019 at 11:45 AM Christophe Leroy
<christophe.leroy@c-s.fr> wrote:
>
>
>
> Le 23/05/2019 à 10:53, Mathieu Malaterre a écrit :
> > On Thu, May 23, 2019 at 10:29 AM Mathieu Malaterre <malat@debian.org> wrote:
> >>
> >> On Thu, May 23, 2019 at 8:39 AM Christophe Leroy
> >> <christophe.leroy@c-s.fr> wrote:
> >>>
> >>> Salut Mathieu,
> >>>
> >>> Le 23/05/2019 à 08:24, Mathieu Malaterre a écrit :
> >>>> Salut Christophe,
> >>>>
> >>>> On Wed, May 22, 2019 at 2:20 PM Christophe Leroy
> >>>> <christophe.leroy@c-s.fr> wrote:
> >>>>>
> >>>>>
> >>>>>
> >>>>> Le 22/05/2019 à 14:15, Mathieu Malaterre a écrit :
> >>>>>> Hi all,
> >>>>>>
> >>>>>> I have not boot my G4 in a while, today using master here is what I see:
> >>>>>>
> >>>>>> done
> >>>>>> Setting btext !
> >>>>>> W=640 H=488 LB=768 addr=0x9c008000
> >>>>>> copying OF device tree...
> >>>>>> starting device tree allocs at 01401000
> >>>>>> otloc_up(00100000, 0013d948)
> >>>>>> trying: 0x01401000
> >>>>>> trying: 0x01501000
> >>>>>> -› 01501000
> >>>>>> alloc_bottom : 01601000
> >>>>>> alloc_top : 20000000
> >>>>>> alloc_top_hi : 20000000
> >>>>>> nmo_top : 20000000
> >>>>>> ram_top : 20000000
> >>>>>> Building dt strings...
> >>>>>> Building dt structure...
> >>>>>> reserved memory map:
> >>>>>> 00d40000 - 006c1000
> >>>>>> Device tree strings 0x01502000 -> 0x00000007
> >>>>>> Device tree struct 0x01503000 -> 0x00000007
> >>>>>> Quiescing Open Firmware ...
> >>>>>> Booting Linux via __start() @ 0x001400000
> >>>>>> ->dt_headr_start=0x01501000
> >>>>>>
> >>>>>> Any suggestions before I start a bisect ?
> >>>>>>
> >>>>>
> >>>>> Have you tried without CONFIG_PPC_KUEP and CONFIG_PPC_KUAP ?
> >>>>
> >>>> Using locally:
> >>>>
> >>>> diff --git a/arch/powerpc/configs/g4_defconfig
> >>>> b/arch/powerpc/configs/g4_defconfig
> >>>> index 14d0376f637d..916bce8ce9c3 100644
> >>>> --- a/arch/powerpc/configs/g4_defconfig
> >>>> +++ b/arch/powerpc/configs/g4_defconfig
> >>>> @@ -32,6 +32,8 @@ CONFIG_USERFAULTFD=y
> >>>> # CONFIG_COMPAT_BRK is not set
> >>>> CONFIG_PROFILING=y
> >>>> CONFIG_G4_CPU=y
> >>>> +# CONFIG_PPC_KUEP is not set
> >>>> +# CONFIG_PPC_KUAP is not set
> >>>> CONFIG_PANIC_TIMEOUT=0
> >>>> # CONFIG_PPC_CHRP is not set
> >>>> CONFIG_CPU_FREQ=y
> >>>>
> >>>>
> >>>> Leads to almost the same error (some values have changed):
> >>>
> >>> Ok.
> >>>
> >>> When you say you are using 'master', what do you mean ? Can you give the
> >>> commit Id ?
> >>>
> >>> Does it boots with Kernel 5.1.4 ?
> >>
> >> I was able to boot v5.1:
> >>
> >> $ dmesg | head
> >> [ 0.000000] printk: bootconsole [udbg0] enabled
> >> [ 0.000000] Total memory = 512MB; using 1024kB for hash table (at (ptrval))
> >> [ 0.000000] Linux version 5.1.0+ (malat@debian.org) (gcc version
> >> 8.3.0 (Debian 8.3.0-7)) #8 Thu May 23 06:26:38 UTC 2019
> >>
> >> Commit id is:
> >>
> >> e93c9c99a629 (tag: v5.1) Linux 5.1
> >>
> >>> Did you try latest powerpc/merge branch ?
> >>
> >> Will try that next.
> >
> > I confirm powerpc/merge does not boot for me (same config). Commit id:
> >
> > a27eaa62326d (powerpc/merge) Automatic merge of branches 'master',
> > 'next' and 'fixes' into merge
>
> I see in the config you sent me that you have selected CONFIG_KASAN,
> which is a big new stuff.
>
> Can you try without it ?
With same config but CONFIG_KASAN=n (on top of a27eaa62326d), I can
reproduce the boot failure (no change).
Time for bisect ?
> Christophe
>
> >
> >
> >>> Can you send your full .config ?
> >>
> >> Config is attached.
> >>
> >> Thanks,
> >>
> >>> Christophe
> >>>
> >>>>
> >>>> done
> >>>> Setting btext !
> >>>> W=640 H=488 LB=768 addr=0x9c008000
> >>>> copying OF device tree...
> >>>> starting device tree allocs at 01300000
> >>>> alloc_up(00100000, 0013d948)
> >>>> trying: 0x01300000
> >>>> trying: 0x01400000
> >>>> -› 01400000
> >>>> alloc_bottom : 01500000
> >>>> alloc_top : 20000000
> >>>> alloc_top_hi : 20000000
> >>>> nmo_top : 20000000
> >>>> ram_top : 20000000
> >>>> Building dt strings...
> >>>> Building dt structure...
> >>>> reserved memory map:
> >>>> 00c40000 - 006c0000
> >>>> Device tree strings 0x01401000 -> 0x00000007
> >>>> Device tree struct 0x01402000 -> 0x00000007
> >>>> Quiescing Open Firmware ...
> >>>> Booting Linux via __start() @ 0x001400000
> >>>> ->dt_headr_start=0x01400000
> >>>>
> >>>> Thanks anyway,
> >>>>
^ permalink raw reply
* Re: [PATCH] ASoC: fsl_esai: fix the channel swap issue after xrun
From: Nicolin Chen @ 2019-05-23 10:10 UTC (permalink / raw)
To: broonie@kernel.org, S.j. Wang
Cc: alsa-devel@alsa-project.org, timur@kernel.org,
Xiubo.Lee@gmail.com, festevam@gmail.com,
linux-kernel@vger.kernel.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <VE1PR04MB647934199C3AA60759BED888E3010@VE1PR04MB6479.eurprd04.prod.outlook.com>
Hello Shengjiu,
On Thu, May 23, 2019 at 09:53:42AM +0000, S.j. Wang wrote:
> > > + /*
> > > + * Add fifo reset here, because the regcache_sync will
> > > + * write one more data to ETDR.
> > > + * Which will cause channel shift.
> >
> > Sounds like a bug to me...should fix it first by marking the data registers as
> > volatile.
> >
> The ETDR is a writable register, it is not volatile. Even we change it to
> Volatile, I don't think we can't avoid this issue. for the regcache_sync
> Just to write this register, it is correct behavior.
Is that so? Quoting the comments of regcache_sync():
"* regcache_sync - Sync the register cache with the hardware.
*
* @map: map to configure.
*
* Any registers that should not be synced should be marked as
* volatile."
If regcache_sync() does sync volatile registers too as you said,
I don't mind having this FIFO reset WAR for now, though I think
this mismatch between the comments and the actual behavior then
should get people's attention.
Thank you
^ permalink raw reply
* Re: Failure to boot G4: dt_headr_start=0x01501000
From: Christophe Leroy @ 2019-05-23 10:05 UTC (permalink / raw)
To: Mathieu Malaterre; +Cc: linuxppc-dev, Daniel Axtens
In-Reply-To: <ba3a1c25-72ce-cfb4-67ac-df07584f57f5@c-s.fr>
On 05/23/2019 09:59 AM, Christophe Leroy wrote:
>
>
> On 05/23/2019 09:45 AM, Christophe Leroy wrote:
>>
>>
>> Le 23/05/2019 à 10:53, Mathieu Malaterre a écrit :
>>>> Commit id is:
>>>>
>>>> e93c9c99a629 (tag: v5.1) Linux 5.1
>>>>
>>>>> Did you try latest powerpc/merge branch ?
>>>>
>>>> Will try that next.
>>>
>>> I confirm powerpc/merge does not boot for me (same config). Commit id:
>>>
>>> a27eaa62326d (powerpc/merge) Automatic merge of branches 'master',
>>> 'next' and 'fixes' into merge
>>
>> I see in the config you sent me that you have selected CONFIG_KASAN,
>> which is a big new stuff.
>>
>> Can you try without it ?
>
> While building with your config, I get a huge amount of:
>
> ppc-linux-ld: warning: orphan section `.data..LASAN0' from
> `lib/xarray.o' being placed in section `.data..LASAN0'.
> SORTEX vmlinux
>
>
>
> I see you have also selected CONFIG_LD_DEAD_CODE_DATA_ELIMINATION=y
>
> I guess nobody have never tried both this and CONFIG_KASAN together on
> ppc32. I'll give it a try.
And you also have CONFIG_FTRACE.
In a recent patch implementing KASAN on PPC64, Daniel says that KASAN
and FTRACE don't go together well
(https://patchwork.ozlabs.org/patch/1103826/)
If you find out that it works without KASAN, can you then try with KASAN
but without FTRACE ?
Christophe
^ permalink raw reply
* Re: Failure to boot G4: dt_headr_start=0x01501000
From: Christophe Leroy @ 2019-05-23 9:59 UTC (permalink / raw)
To: Mathieu Malaterre; +Cc: linuxppc-dev
In-Reply-To: <9e5ef44d-259a-1f1a-bd6a-98abdae85da0@c-s.fr>
On 05/23/2019 09:45 AM, Christophe Leroy wrote:
>
>
> Le 23/05/2019 à 10:53, Mathieu Malaterre a écrit :
>>> Commit id is:
>>>
>>> e93c9c99a629 (tag: v5.1) Linux 5.1
>>>
>>>> Did you try latest powerpc/merge branch ?
>>>
>>> Will try that next.
>>
>> I confirm powerpc/merge does not boot for me (same config). Commit id:
>>
>> a27eaa62326d (powerpc/merge) Automatic merge of branches 'master',
>> 'next' and 'fixes' into merge
>
> I see in the config you sent me that you have selected CONFIG_KASAN,
> which is a big new stuff.
>
> Can you try without it ?
While building with your config, I get a huge amount of:
ppc-linux-ld: warning: orphan section `.data..LASANLOC10' from
`lib/vsprintf.o' being placed in section `.data..LASANLOC10'.
ppc-linux-ld: warning: orphan section `.data..LASANLOC11' from
`lib/vsprintf.o' being placed in section `.data..LASANLOC11'.
ppc-linux-ld: warning: orphan section `.data..LASANLOC12' from
`lib/vsprintf.o' being placed in section `.data..LASANLOC12'.
ppc-linux-ld: warning: orphan section `.data..LASANLOC13' from
`lib/vsprintf.o' being placed in section `.data..LASANLOC13'.
ppc-linux-ld: warning: orphan section `.data..LASANLOC14' from
`lib/vsprintf.o' being placed in section `.data..LASANLOC14'.
ppc-linux-ld: warning: orphan section `.data..LASANLOC15' from
`lib/vsprintf.o' being placed in section `.data..LASANLOC15'.
ppc-linux-ld: warning: orphan section `.data..LASANLOC16' from
`lib/vsprintf.o' being placed in section `.data..LASANLOC16'.
ppc-linux-ld: warning: orphan section `.data..LASANLOC1' from
`lib/vsprintf.o' being placed in section `.data..LASANLOC1'.
ppc-linux-ld: warning: orphan section `.data..LASANLOC2' from
`lib/vsprintf.o' being placed in section `.data..LASANLOC2'.
ppc-linux-ld: warning: orphan section `.data..LASANLOC3' from
`lib/vsprintf.o' being placed in section `.data..LASANLOC3'.
ppc-linux-ld: warning: orphan section `.data..LASANLOC4' from
`lib/vsprintf.o' being placed in section `.data..LASANLOC4'.
ppc-linux-ld: warning: orphan section `.data..LASANLOC5' from
`lib/vsprintf.o' being placed in section `.data..LASANLOC5'.
ppc-linux-ld: warning: orphan section `.data..LASANLOC6' from
`lib/vsprintf.o' being placed in section `.data..LASANLOC6'.
ppc-linux-ld: warning: orphan section `.data..LASANLOC7' from
`lib/vsprintf.o' being placed in section `.data..LASANLOC7'.
ppc-linux-ld: warning: orphan section `.data..LASANLOC8' from
`lib/vsprintf.o' being placed in section `.data..LASANLOC8'.
ppc-linux-ld: warning: orphan section `.data..LASANLOC9' from
`lib/vsprintf.o' being placed in section `.data..LASANLOC9'.
ppc-linux-ld: warning: orphan section `.data..LASAN0' from
`lib/xarray.o' being placed in section `.data..LASAN0'.
SORTEX vmlinux
I see you have also selected CONFIG_LD_DEAD_CODE_DATA_ELIMINATION=y
I guess nobody have never tried both this and CONFIG_KASAN together on
ppc32. I'll give it a try.
Christophe
>
> Christophe
>
>>
>>
>>>> Can you send your full .config ?
>>>
>>> Config is attached.
>>>
>>> Thanks,
>>>
>>>> Christophe
>>>>
>>>>>
>>>>> done
>>>>> Setting btext !
>>>>> W=640 H=488 LB=768 addr=0x9c008000
>>>>> copying OF device tree...
>>>>> starting device tree allocs at 01300000
>>>>> alloc_up(00100000, 0013d948)
>>>>> trying: 0x01300000
>>>>> trying: 0x01400000
>>>>> -› 01400000
>>>>> alloc_bottom : 01500000
>>>>> alloc_top : 20000000
>>>>> alloc_top_hi : 20000000
>>>>> nmo_top : 20000000
>>>>> ram_top : 20000000
>>>>> Building dt strings...
>>>>> Building dt structure...
>>>>> reserved memory map:
>>>>> 00c40000 - 006c0000
>>>>> Device tree strings 0x01401000 -> 0x00000007
>>>>> Device tree struct 0x01402000 -> 0x00000007
>>>>> Quiescing Open Firmware ...
>>>>> Booting Linux via __start() @ 0x001400000
>>>>> ->dt_headr_start=0x01400000
>>>>>
>>>>> Thanks anyway,
>>>>>
^ permalink raw reply
* Re: [PATCH] ASoC: fsl_esai: fix the channel swap issue after xrun
From: S.j. Wang @ 2019-05-23 9:53 UTC (permalink / raw)
To: Nicolin Chen
Cc: alsa-devel@alsa-project.org, timur@kernel.org,
Xiubo.Lee@gmail.com, festevam@gmail.com,
linux-kernel@vger.kernel.org, broonie@kernel.org,
linuxppc-dev@lists.ozlabs.org
Hi
> > + /*
> > + * Add fifo reset here, because the regcache_sync will
> > + * write one more data to ETDR.
> > + * Which will cause channel shift.
>
> Sounds like a bug to me...should fix it first by marking the data registers as
> volatile.
>
The ETDR is a writable register, it is not volatile. Even we change it to
Volatile, I don't think we can't avoid this issue. for the regcache_sync
Just to write this register, it is correct behavior.
Best regards
Wang shengjiu
^ 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