* Re: [PATCH v12 01/12] lib: introduce copy_struct_{to,from}_user helpers
From: Peter Zijlstra @ 2019-09-05 10:57 UTC (permalink / raw)
To: Aleksa Sarai
Cc: linux-ia64, linux-sh, Alexander Shishkin, Rasmus Villemoes,
Alexei Starovoitov, linux-kernel, David Howells, linux-kselftest,
sparclinux, Jiri Olsa, linux-arch, linux-s390, Tycho Andersen,
Aleksa Sarai, Shuah Khan, Ingo Molnar, linux-arm-kernel,
linux-mips, linux-xtensa, Kees Cook, Arnd Bergmann, Jann Horn,
linuxppc-dev, linux-m68k, Al Viro, Andy Lutomirski, Shuah Khan,
Namhyung Kim, David Drysdale, Christian Brauner, J. Bruce Fields,
linux-parisc, linux-api, Chanho Min, Jeff Layton, Oleg Nesterov,
patrick.bellasi, Eric Biederman, linux-alpha, linux-fsdevel,
Andrew Morton, Linus Torvalds, containers
In-Reply-To: <20190905094305.GJ2349@hirez.programming.kicks-ass.net>
On Thu, Sep 05, 2019 at 11:43:05AM +0200, Peter Zijlstra wrote:
> On Thu, Sep 05, 2019 at 07:26:22PM +1000, Aleksa Sarai wrote:
> > On 2019-09-05, Peter Zijlstra <peterz@infradead.org> wrote:
> > > On Thu, Sep 05, 2019 at 06:19:22AM +1000, Aleksa Sarai wrote:
> > > > +/**
> > > > + * copy_struct_to_user: copy a struct to user space
> > > > + * @dst: Destination address, in user space.
> > > > + * @usize: Size of @dst struct.
> > > > + * @src: Source address, in kernel space.
> > > > + * @ksize: Size of @src struct.
> > > > + *
> > > > + * Copies a struct from kernel space to user space, in a way that guarantees
> > > > + * backwards-compatibility for struct syscall arguments (as long as future
> > > > + * struct extensions are made such that all new fields are *appended* to the
> > > > + * old struct, and zeroed-out new fields have the same meaning as the old
> > > > + * struct).
> > > > + *
> > > > + * @ksize is just sizeof(*dst), and @usize should've been passed by user space.
> > > > + * The recommended usage is something like the following:
> > > > + *
> > > > + * SYSCALL_DEFINE2(foobar, struct foo __user *, uarg, size_t, usize)
> > > > + * {
> > > > + * int err;
> > > > + * struct foo karg = {};
> > > > + *
> > > > + * // do something with karg
> > > > + *
> > > > + * err = copy_struct_to_user(uarg, usize, &karg, sizeof(karg));
> > > > + * if (err)
> > > > + * return err;
> > > > + *
> > > > + * // ...
> > > > + * }
> > > > + *
> > > > + * There are three cases to consider:
> > > > + * * If @usize == @ksize, then it's copied verbatim.
> > > > + * * If @usize < @ksize, then kernel space is "returning" a newer struct to an
> > > > + * older user space. In order to avoid user space getting incomplete
> > > > + * information (new fields might be important), all trailing bytes in @src
> > > > + * (@ksize - @usize) must be zerored
> > >
> > > s/zerored/zero/, right?
> >
> > It should've been "zeroed".
>
> That reads wrong to me; that way it reads like this function must take
> that action and zero out the 'rest'; which is just wrong.
>
> This function must verify those bytes are zero, not make them zero.
>
> > > > , otherwise -EFBIG is returned.
> > >
> > > 'Funny' that, copy_struct_from_user() below seems to use E2BIG.
> >
> > This is a copy of the semantics that sched_[sg]etattr(2) uses -- E2BIG for
> > a "too big" struct passed to the kernel, and EFBIG for a "too big"
> > struct passed to user-space. I would personally have preferred EMSGSIZE
> > instead of EFBIG, but felt using the existing error codes would be less
> > confusing.
>
> Sadly a recent commit:
>
> 1251201c0d34 ("sched/core: Fix uclamp ABI bug, clean up and robustify sched_read_attr() ABI logic and code")
>
> Made the situation even 'worse'.
And thinking more about things; I'm not convinced the above patch is
actually right.
Do we really want to simply truncate all the attributes of the task?
And should we not at least set sched_flags when there are non-default
clamp values applied?
See; that is I think the primary bug that had chrt failing; we tried to
publish the default clamp values as !0.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* RE: [EXT] Re: coresight: Add ETM4.1 support for ThunderX2
From: Tanmay Vilas Kumar Jagdale @ 2019-09-05 10:59 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Tomasz Nowicki, Jayachandran Chandrasekharan Nair,
Ganapatrao Kulkarni, linux-arm-kernel@lists.infradead.org,
suzuki.poulose@arm.com
In-Reply-To: <20190819203325.GB8268@xps15>
Hi Mathieu,
> Same comment as the previous patch along with the following...
>
> On Thu, Aug 15, 2019 at 01:53:46PM +0000, Tanmay Vilas Kumar Jagdale wrote:
> > Add ETM4.1 periperhal ID for Marvell's ThunderX2 chip.
> >
> > Signed-off-by: Tanmay Jagdale <tanmay@marvell.com>
> > ---
> > drivers/hwtracing/coresight/coresight-etm4x.c | 2 ++
> > drivers/hwtracing/coresight/coresight-etm4x.h | 1 +
> > 2 files changed, 3 insertions(+)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-etm4x.c
> > b/drivers/hwtracing/coresight/coresight-etm4x.c
> > index 7bcac8896fc1..ac3bd617907b 100644
> > --- a/drivers/hwtracing/coresight/coresight-etm4x.c
> > +++ b/drivers/hwtracing/coresight/coresight-etm4x.c
> > @@ -58,6 +58,7 @@ static bool etm4_arch_supported(u8 arch)
> > /* Mask out the minor version number */
> > switch (arch & 0xf0) {
> > case ETM_ARCH_V4:
> > + case ETM_ARCH_V4_1:
>
> Why add ETM_ARCH_V4_1 when the switch statement strips off the first byte?
>
> Look at[1], someone already added support for 4.2.
>
> [1]. 5666dfd1d8a4 coresight: etm4x: Add support to enable ETMv4.2
>
>
> > break;
> > default:
> > return false;
> > @@ -1196,6 +1197,7 @@ static const struct amba_id etm4_ids[] = {
> > CS_AMBA_ID(0x000bb95e), /* Cortex-A57 */
> > CS_AMBA_ID(0x000bb95a), /* Cortex-A72 */
> > CS_AMBA_ID(0x000bb959), /* Cortex-A73 */
> > + CS_AMBA_ID(0x000cc0af), /* Marvell ThunderX2 */
>
> I suspect this processor also has "coresight-cpu-debug" IPs. If that is the
> case it is very possible they both have the same CID and a UCI (see next line)
> is required.
>
Yes, our processor has the coresight-cpu-debug IP. Currently I am working with
the hardware team to test this feature. Once that is done I will post a patch
that supports it. In the meantime I will post a v2 patch for ETMv4 with UCI.
Hope that is okay.
> > CS_AMBA_UCI_ID(0x000bb9da, uci_id_etm4), /* Cortex-A35 */
> > {},
> > };
>
> Thanks,
> Mathieu
>
> > diff --git a/drivers/hwtracing/coresight/coresight-etm4x.h
> > b/drivers/hwtracing/coresight/coresight-etm4x.h
> > index 4523f10ddd0f..03369e56b2eb 100644
> > --- a/drivers/hwtracing/coresight/coresight-etm4x.h
> > +++ b/drivers/hwtracing/coresight/coresight-etm4x.h
> > @@ -137,6 +137,7 @@
> > #define ETM_MAX_SS_CMP 8
> >
> > #define ETM_ARCH_V4 0x40
> > +#define ETM_ARCH_V4_1 0x41
> > #define ETMv4_SYNC_MASK 0x1F
> > #define ETM_CYC_THRESHOLD_MASK 0xFFF
> > #define ETM_CYC_THRESHOLD_DEFAULT 0x100
> > --
> > 2.17.1
> >
Thanks,
Tanmay
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v12 01/12] lib: introduce copy_struct_{to,from}_user helpers
From: Christian Brauner @ 2019-09-05 11:05 UTC (permalink / raw)
To: Aleksa Sarai
Cc: linux-ia64, linux-sh, Peter Zijlstra, Rasmus Villemoes,
Alexei Starovoitov, linux-kernel, David Howells, linux-kselftest,
sparclinux, Shuah Khan, linux-arch, linux-s390, Tycho Andersen,
Aleksa Sarai, Jiri Olsa, Alexander Shishkin, Ingo Molnar,
linux-arm-kernel, linux-mips, linux-xtensa, Kees Cook,
Arnd Bergmann, Jann Horn, linuxppc-dev, linux-m68k, Al Viro,
Andy Lutomirski, Shuah Khan, Namhyung Kim, David Drysdale,
Christian Brauner, J. Bruce Fields, linux-parisc, linux-api,
Chanho Min, Jeff Layton, Oleg Nesterov, Eric Biederman,
linux-alpha, linux-fsdevel, Andrew Morton, Linus Torvalds,
containers
In-Reply-To: <20190904201933.10736-2-cyphar@cyphar.com>
On Thu, Sep 05, 2019 at 06:19:22AM +1000, Aleksa Sarai wrote:
> A common pattern for syscall extensions is increasing the size of a
> struct passed from userspace, such that the zero-value of the new fields
> result in the old kernel behaviour (allowing for a mix of userspace and
> kernel vintages to operate on one another in most cases). This is done
> in both directions -- hence two helpers -- though it's more common to
> have to copy user space structs into kernel space.
>
> Previously there was no common lib/ function that implemented
> the necessary extension-checking semantics (and different syscalls
> implemented them slightly differently or incompletely[1]). A future
> patch replaces all of the common uses of this pattern to use the new
> copy_struct_{to,from}_user() helpers.
>
> [1]: For instance {sched_setattr,perf_event_open,clone3}(2) all do do
> similar checks to copy_struct_from_user() while rt_sigprocmask(2)
> always rejects differently-sized struct arguments.
>
> Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
> ---
> include/linux/uaccess.h | 5 ++
> lib/Makefile | 2 +-
> lib/struct_user.c | 182 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 188 insertions(+), 1 deletion(-)
> create mode 100644 lib/struct_user.c
>
> diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
> index 34a038563d97..0ad9544a1aee 100644
> --- a/include/linux/uaccess.h
> +++ b/include/linux/uaccess.h
> @@ -230,6 +230,11 @@ static inline unsigned long __copy_from_user_inatomic_nocache(void *to,
>
> #endif /* ARCH_HAS_NOCACHE_UACCESS */
>
> +extern int copy_struct_to_user(void __user *dst, size_t usize,
> + const void *src, size_t ksize);
> +extern int copy_struct_from_user(void *dst, size_t ksize,
> + const void __user *src, size_t usize);
> +
> /*
> * probe_kernel_read(): safely attempt to read from a location
> * @dst: pointer to the buffer that shall take the data
> diff --git a/lib/Makefile b/lib/Makefile
> index 29c02a924973..d86c71feaf0a 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -28,7 +28,7 @@ endif
> CFLAGS_string.o := $(call cc-option, -fno-stack-protector)
> endif
>
> -lib-y := ctype.o string.o vsprintf.o cmdline.o \
> +lib-y := ctype.o string.o struct_user.o vsprintf.o cmdline.o \
> rbtree.o radix-tree.o timerqueue.o xarray.o \
> idr.o extable.o \
> sha1.o chacha.o irq_regs.o argv_split.o \
> diff --git a/lib/struct_user.c b/lib/struct_user.c
> new file mode 100644
> index 000000000000..7301ab1bbe98
> --- /dev/null
> +++ b/lib/struct_user.c
> @@ -0,0 +1,182 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2019 SUSE LLC
> + * Copyright (C) 2019 Aleksa Sarai <cyphar@cyphar.com>
> + */
> +
> +#include <linux/types.h>
> +#include <linux/export.h>
> +#include <linux/uaccess.h>
> +#include <linux/kernel.h>
> +#include <linux/string.h>
> +
> +#define BUFFER_SIZE 64
> +
> +/*
> + * "memset(p, 0, size)" but for user space buffers. Caller must have already
> + * checked access_ok(p, size).
> + */
> +static int __memzero_user(void __user *p, size_t s)
> +{
> + const char zeros[BUFFER_SIZE] = {};
> + while (s > 0) {
> + size_t n = min(s, sizeof(zeros));
> +
> + if (__copy_to_user(p, zeros, n))
> + return -EFAULT;
> +
> + p += n;
> + s -= n;
> + }
> + return 0;
> +}
> +
> +/**
> + * copy_struct_to_user: copy a struct to user space
> + * @dst: Destination address, in user space.
> + * @usize: Size of @dst struct.
> + * @src: Source address, in kernel space.
> + * @ksize: Size of @src struct.
> + *
> + * Copies a struct from kernel space to user space, in a way that guarantees
> + * backwards-compatibility for struct syscall arguments (as long as future
> + * struct extensions are made such that all new fields are *appended* to the
> + * old struct, and zeroed-out new fields have the same meaning as the old
> + * struct).
> + *
> + * @ksize is just sizeof(*dst), and @usize should've been passed by user space.
> + * The recommended usage is something like the following:
> + *
> + * SYSCALL_DEFINE2(foobar, struct foo __user *, uarg, size_t, usize)
> + * {
> + * int err;
> + * struct foo karg = {};
> + *
> + * // do something with karg
> + *
> + * err = copy_struct_to_user(uarg, usize, &karg, sizeof(karg));
> + * if (err)
> + * return err;
> + *
> + * // ...
> + * }
> + *
> + * There are three cases to consider:
> + * * If @usize == @ksize, then it's copied verbatim.
> + * * If @usize < @ksize, then kernel space is "returning" a newer struct to an
> + * older user space. In order to avoid user space getting incomplete
> + * information (new fields might be important), all trailing bytes in @src
> + * (@ksize - @usize) must be zerored, otherwise -EFBIG is returned.
> + * * If @usize > @ksize, then the kernel is "returning" an older struct to a
> + * newer user space. The trailing bytes in @dst (@usize - @ksize) will be
> + * zero-filled.
> + *
> + * Returns (in all cases, some data may have been copied):
> + * * -EFBIG: (@usize < @ksize) and there are non-zero trailing bytes in @src.
> + * * -EFAULT: access to user space failed.
> + */
> +int copy_struct_to_user(void __user *dst, size_t usize,
> + const void *src, size_t ksize)
> +{
> + size_t size = min(ksize, usize);
> + size_t rest = abs(ksize - usize);
> +
> + if (unlikely(usize > PAGE_SIZE))
> + return -EFAULT;
Looks like this should be -EFBIG.
> + if (unlikely(!access_ok(dst, usize)))
> + return -EFAULT;
> +
> + /* Deal with trailing bytes. */
> + if (usize < ksize) {
> + if (memchr_inv(src + size, 0, rest))
> + return -EFBIG;
> + } else if (usize > ksize) {
> + if (__memzero_user(dst + size, rest))
> + return -EFAULT;
Is zeroing that memory really our job? Seems to me we should just check
it is zeroed.
> + }
> + /* Copy the interoperable parts of the struct. */
> + if (__copy_to_user(dst, src, size))
> + return -EFAULT;
> + return 0;
> +}
> +EXPORT_SYMBOL(copy_struct_to_user);
> +
> +/**
> + * copy_struct_from_user: copy a struct from user space
> + * @dst: Destination address, in kernel space. This buffer must be @ksize
> + * bytes long.
> + * @ksize: Size of @dst struct.
> + * @src: Source address, in user space.
> + * @usize: (Alleged) size of @src struct.
> + *
> + * Copies a struct from user space to kernel space, in a way that guarantees
> + * backwards-compatibility for struct syscall arguments (as long as future
> + * struct extensions are made such that all new fields are *appended* to the
> + * old struct, and zeroed-out new fields have the same meaning as the old
> + * struct).
> + *
> + * @ksize is just sizeof(*dst), and @usize should've been passed by user space.
> + * The recommended usage is something like the following:
> + *
> + * SYSCALL_DEFINE2(foobar, const struct foo __user *, uarg, size_t, usize)
> + * {
> + * int err;
> + * struct foo karg = {};
> + *
> + * err = copy_struct_from_user(&karg, sizeof(karg), uarg, size);
> + * if (err)
> + * return err;
> + *
> + * // ...
> + * }
> + *
> + * There are three cases to consider:
> + * * If @usize == @ksize, then it's copied verbatim.
> + * * If @usize < @ksize, then the user space has passed an old struct to a
> + * newer kernel. The rest of the trailing bytes in @dst (@ksize - @usize)
> + * are to be zero-filled.
> + * * If @usize > @ksize, then the user space has passed a new struct to an
> + * older kernel. The trailing bytes unknown to the kernel (@usize - @ksize)
> + * are checked to ensure they are zeroed, otherwise -E2BIG is returned.
> + *
> + * Returns (in all cases, some data may have been copied):
> + * * -E2BIG: (@usize > @ksize) and there are non-zero trailing bytes in @src.
> + * * -E2BIG: @usize is "too big" (at time of writing, >PAGE_SIZE).
> + * * -EFAULT: access to user space failed.
> + */
> +int copy_struct_from_user(void *dst, size_t ksize,
> + const void __user *src, size_t usize)
> +{
> + size_t size = min(ksize, usize);
> + size_t rest = abs(ksize - usize);
> +
> + if (unlikely(usize > PAGE_SIZE))
> + return -EFAULT;
That should be -E2BIG.
> + if (unlikely(!access_ok(src, usize)))
> + return -EFAULT;
> +
> + /* Deal with trailing bytes. */
> + if (usize < ksize)
> + memset(dst + size, 0, rest);
I think kernel style mandates that if one branch in an if-else ladder
requires {} all other must use {} as well. So this should be:
if () {
// one line
} else {
// one line
// another line
}
That's a change in behavior for clone3() and sched at least, no? Unless
- which I guess you might have done - you have moved the "error out when
the struct is too small" part before the call to copy_struct_from_user()
for them.
> + else if (usize > ksize) {
> + const void __user *addr = src + size;
> + char buffer[BUFFER_SIZE] = {};
> +
> + while (rest > 0) {
> + size_t bufsize = min(rest, sizeof(buffer));
> +
> + if (__copy_from_user(buffer, addr, bufsize))
> + return -EFAULT;
> + if (memchr_inv(buffer, 0, bufsize))
> + return -E2BIG;
> +
> + addr += bufsize;
> + rest -= bufsize;
> + }
> + }
> + /* Copy the interoperable parts of the struct. */
> + if (__copy_from_user(dst, src, size))
> + return -EFAULT;
> + return 0;
> +}
> +EXPORT_SYMBOL(copy_struct_from_user);
> --
> 2.23.0
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH -next 12/15] thermal: tango: use devm_platform_ioremap_resource() to simplify code
From: Måns Rullgård @ 2019-09-05 11:06 UTC (permalink / raw)
To: YueHaibing
Cc: mmayer, eric, miquel.raynal, linux-stm32, heiko, amit.kucheria,
f.fainelli, daniel.lezcano, phil, linux-rockchip, agross,
bcm-kernel-feedback-list, linux-arm-msm, rui.zhang,
david.hernandezsanchez, alexandre.torgue, marc.w.gonzalez, rjui,
edubezval, linux-mediatek, linux-rpi-kernel, gregory.0xf0,
matthias.bgg, horms+renesas, talel, linux-arm-kernel, sbranden,
wsa+renesas, gregkh, linux-pm, linux-kernel, wahrenst,
mcoquelin.stm32, jun.nie, computersforpeace, shawnguo
In-Reply-To: <20190904122939.23780-13-yuehaibing@huawei.com>
YueHaibing <yuehaibing@huawei.com> writes:
> Use devm_platform_ioremap_resource() to simplify the code a bit.
> This is detected by coccinelle.
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Acked-by: Mans Rullgard <mans@mansr.com>
> ---
> drivers/thermal/tango_thermal.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/thermal/tango_thermal.c b/drivers/thermal/tango_thermal.c
> index 304b461..f44441b 100644
> --- a/drivers/thermal/tango_thermal.c
> +++ b/drivers/thermal/tango_thermal.c
> @@ -73,7 +73,6 @@ static void tango_thermal_init(struct tango_thermal_priv *priv)
>
> static int tango_thermal_probe(struct platform_device *pdev)
> {
> - struct resource *res;
> struct tango_thermal_priv *priv;
> struct thermal_zone_device *tzdev;
>
> @@ -81,8 +80,7 @@ static int tango_thermal_probe(struct platform_device *pdev)
> if (!priv)
> return -ENOMEM;
>
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - priv->base = devm_ioremap_resource(&pdev->dev, res);
> + priv->base = devm_platform_ioremap_resource(pdev, 0);
> if (IS_ERR(priv->base))
> return PTR_ERR(priv->base);
>
> --
> 2.7.4
>
--
Måns Rullgård
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v12 01/12] lib: introduce copy_struct_{to,from}_user helpers
From: Christian Brauner @ 2019-09-05 11:09 UTC (permalink / raw)
To: Aleksa Sarai
Cc: linux-ia64, linux-sh, Peter Zijlstra, Rasmus Villemoes,
Alexei Starovoitov, linux-kernel, David Howells, linux-kselftest,
sparclinux, Shuah Khan, linux-arch, linux-s390, Tycho Andersen,
Aleksa Sarai, Jiri Olsa, Alexander Shishkin, Ingo Molnar,
linux-arm-kernel, linux-mips, linux-xtensa, Kees Cook,
Arnd Bergmann, Jann Horn, linuxppc-dev, linux-m68k, Al Viro,
Andy Lutomirski, Shuah Khan, Namhyung Kim, David Drysdale,
Christian Brauner, J. Bruce Fields, linux-parisc, linux-api,
Chanho Min, Jeff Layton, Oleg Nesterov, Eric Biederman,
linux-alpha, linux-fsdevel, Andrew Morton, Linus Torvalds,
containers
In-Reply-To: <20190904201933.10736-2-cyphar@cyphar.com>
On Thu, Sep 05, 2019 at 06:19:22AM +1000, Aleksa Sarai wrote:
> A common pattern for syscall extensions is increasing the size of a
> struct passed from userspace, such that the zero-value of the new fields
> result in the old kernel behaviour (allowing for a mix of userspace and
> kernel vintages to operate on one another in most cases). This is done
> in both directions -- hence two helpers -- though it's more common to
> have to copy user space structs into kernel space.
>
> Previously there was no common lib/ function that implemented
> the necessary extension-checking semantics (and different syscalls
> implemented them slightly differently or incompletely[1]). A future
> patch replaces all of the common uses of this pattern to use the new
> copy_struct_{to,from}_user() helpers.
>
> [1]: For instance {sched_setattr,perf_event_open,clone3}(2) all do do
> similar checks to copy_struct_from_user() while rt_sigprocmask(2)
> always rejects differently-sized struct arguments.
>
> Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
I would probably split this out into a separate patchset. It can very
well go in before openat2(). Thoughts?
Christian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v12 01/12] lib: introduce copy_struct_{to,from}_user helpers
From: Rasmus Villemoes @ 2019-09-05 11:17 UTC (permalink / raw)
To: Christian Brauner, Aleksa Sarai
Cc: linux-ia64, linux-sh, Peter Zijlstra, Alexei Starovoitov,
linux-kernel, David Howells, linux-kselftest, sparclinux,
Jiri Olsa, linux-arch, linux-s390, Tycho Andersen, Aleksa Sarai,
Shuah Khan, Alexander Shishkin, Ingo Molnar, linux-arm-kernel,
linux-mips, linux-xtensa, Kees Cook, Arnd Bergmann, Jann Horn,
linuxppc-dev, linux-m68k, Al Viro, Andy Lutomirski, Shuah Khan,
Namhyung Kim, David Drysdale, Christian Brauner, J. Bruce Fields,
linux-parisc, linux-api, Chanho Min, Jeff Layton, Oleg Nesterov,
Eric Biederman, linux-alpha, linux-fsdevel, Andrew Morton,
Linus Torvalds, containers
In-Reply-To: <20190905110544.d6c5t7rx25kvywmi@wittgenstein>
On 05/09/2019 13.05, Christian Brauner wrote:
> On Thu, Sep 05, 2019 at 06:19:22AM +1000, Aleksa Sarai wrote:
>> + if (unlikely(!access_ok(dst, usize)))
>> + return -EFAULT;
>> +
>> + /* Deal with trailing bytes. */
>> + if (usize < ksize) {
>> + if (memchr_inv(src + size, 0, rest))
>> + return -EFBIG;
>> + } else if (usize > ksize) {
>> + if (__memzero_user(dst + size, rest))
>> + return -EFAULT;
>
> Is zeroing that memory really our job? Seems to me we should just check
> it is zeroed.
Of course it is, otherwise you'd require userspace to clear the output
buffer it gives us, which in the majority of cases is wasted work. It's
much easier to reason about if we just say "the kernel populates [uaddr,
uaddr + usize)".
It's completely symmetric to copy_struct_from_user doing a memset() of
the tail of the kernel buffer in case of ksize>usize - you wouldn't want
to require the kernel callers to pass a zeroed buffer to
copy_struct_from_user() - it's just that when we memset(__user*),
there's an error check to do.
Rasmus
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH V4 0/6] PCI: tegra: Enable PCIe C5 controller of Tegra194 in p2972-0000 platform
From: Lorenzo Pieralisi @ 2019-09-05 11:20 UTC (permalink / raw)
To: Vidya Sagar
Cc: devicetree, mperttunen, mmaddireddy, kthota, gustavo.pimentel,
linux-kernel, kishon, linux-tegra, robh+dt, thierry.reding,
linux-pci, bhelgaas, andrew.murray, digetx, jonathanh,
linux-arm-kernel, sagar.tv
In-Reply-To: <20190905104553.2884-1-vidyas@nvidia.com>
On Thu, Sep 05, 2019 at 04:15:47PM +0530, Vidya Sagar wrote:
> This patch series enables Tegra194's C5 controller which owns x16 slot in
> p2972-0000 platform. C5 controller's PERST# and CLKREQ# are not configured as
> output and bi-directional signals by default and hence they need to be
> configured explicitly. Also, x16 slot's 3.3V and 12V supplies are controlled
> through GPIOs and hence they need to be enabled through regulator framework.
> This patch series adds required infrastructural support to address both the
> aforementioned requirements.
> Testing done on p2972-0000 platform
> - Able to enumerate devices connected to x16 slot (owned by C5 controller)
> - Enumerated device's functionality verified
> - Suspend-Resume sequence is verified with device connected to x16 slot
>
> V4:
> * Rebased (Patch-4/6 particularly) on top of Lorenzo's pci/tegra branch
>
> V3:
> * Addressed some more review comments from Andrew Murray and Thierry Reding
>
> V2:
> * Changed the order of patches in the series for easy merging
> * Addressed review comments from Thierry Reding and Andrew Murray
>
> Vidya Sagar (6):
> dt-bindings: PCI: tegra: Add sideband pins configuration entries
> dt-bindings: PCI: tegra: Add PCIe slot supplies regulator entries
> PCI: tegra: Add support to configure sideband pins
> PCI: tegra: Add support to enable slot regulators
> arm64: tegra: Add configuration for PCIe C5 sideband signals
> arm64: tegra: Add PCIe slot supply information in p2972-0000 platform
>
> .../bindings/pci/nvidia,tegra194-pcie.txt | 16 ++++
> .../arm64/boot/dts/nvidia/tegra194-p2888.dtsi | 24 +++++
> .../boot/dts/nvidia/tegra194-p2972-0000.dts | 4 +-
> arch/arm64/boot/dts/nvidia/tegra194.dtsi | 38 +++++++-
> drivers/pci/controller/dwc/pcie-tegra194.c | 94 ++++++++++++++++++-
> 5 files changed, 172 insertions(+), 4 deletions(-)
Applied to pci/tegra for v5.4, thanks.
Lorenzo
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v1 0/3] clk: rockchip: support clock controller for rk3308 SoC
From: Heiko Stuebner @ 2019-09-05 11:22 UTC (permalink / raw)
To: Finley Xiao
Cc: huangtao, sboyd, mturquette, zhangqing, linux-kernel,
linux-rockchip, tony.xie, andy.yan, linux-clk, linux-arm-kernel
In-Reply-To: <20190903115947.26618-1-finley.xiao@rock-chips.com>
Hi Finley,
Am Dienstag, 3. September 2019, 13:59:44 CEST schrieb Finley Xiao:
> Finley Xiao (3):
> dt-bindings: Add bindings for rk3308 clock controller
> clk: rockchip: Add dt-binding header for rk3308
> clk: rockchip: Add clock controller for the RK3308
applied for (hopefully still) 5.4.
I did change the binding patch to name the i2s mclk inputs explicitly
as you can see in [0] as placeholders in dt-bindings do not necessarily
work that great.
Heiko
[0] https://git.kernel.org/pub/scm/linux/kernel/git/mmind/linux-rockchip.git/commit/?h=v5.4-clk/next&id=2d1fb8e983dc0669f276b176142798a228dc0f38
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 03/10] arm64: atomics: avoid out-of-line ll/sc atomics
From: Andrew Murray @ 2019-09-05 11:25 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Mark Rutland, Peter Zijlstra, Catalin Marinas, Robin Murphy,
Ard.Biesheuvel, Nathan Chancellor, Will Deacon, Linux ARM
In-Reply-To: <CAKwvOdkVatgMBLiuKV1bLdDKj_czsaGXuXWXp-9VR6zLyv+U4g@mail.gmail.com>
On Wed, Sep 04, 2019 at 10:28:14AM -0700, Nick Desaulniers wrote:
> On Tue, Sep 3, 2019 at 3:04 PM Andrew Murray <andrew.murray@arm.com> wrote:
> >
> > On Tue, Sep 03, 2019 at 05:37:55PM +0100, Will Deacon wrote:
> > > On Tue, Sep 03, 2019 at 04:31:20PM +0100, Andrew Murray wrote:
> > > > On Tue, Sep 03, 2019 at 04:15:44PM +0100, Andrew Murray wrote:
> > > > > On Tue, Sep 03, 2019 at 03:45:34PM +0100, Will Deacon wrote:
> > > > > > Does it work if the only thing you change is the toolchain, and use GCC
> > > > > > instead?
> > > > >
> > > > > Yup.
> > > >
> > > > Also this is Clang generation:
> > > >
> > > > ffff8000100f2700 <__ptrace_link>:
> > > > ffff8000100f2700: f9426009 ldr x9, [x0, #1216]
> > > > ffff8000100f2704: 91130008 add x8, x0, #0x4c0
> > > > ffff8000100f2708: eb09011f cmp x8, x9
> > > > ffff8000100f270c: 540002a1 b.ne ffff8000100f2760 <__ptrace_link+0x60> // b.any
> > > > ffff8000100f2710: f9425829 ldr x9, [x1, #1200]
> > > > ffff8000100f2714: 9112c02a add x10, x1, #0x4b0
> > > > ffff8000100f2718: f9000528 str x8, [x9, #8]
> > > > ffff8000100f271c: f9026009 str x9, [x0, #1216]
> > > > ffff8000100f2720: f902640a str x10, [x0, #1224]
> > > > ffff8000100f2724: f9025828 str x8, [x1, #1200]
> > > > ffff8000100f2728: f9024001 str x1, [x0, #1152]
> > > > ffff8000100f272c: b4000162 cbz x2, ffff8000100f2758 <__ptrace_link+0x58>
> > > > ffff8000100f2730: b900985f str wzr, [x2, #152]
> > > > ffff8000100f2734: 14000004 b ffff8000100f2744 <__ptrace_link+0x44>
> > > > ffff8000100f2738: 14000001 b ffff8000100f273c <__ptrace_link+0x3c>
> > > > ffff8000100f273c: 14000006 b ffff8000100f2754 <__ptrace_link+0x54>
> > > > ffff8000100f2740: 14000001 b ffff8000100f2744 <__ptrace_link+0x44>
> > > > ffff8000100f2744: 52800028 mov w8, #0x1 // #1
> > > > ffff8000100f2748: b828005f stadd w8, [x2]
> > > > ffff8000100f274c: f9030002 str x2, [x0, #1536]
> > > > ffff8000100f2750: d65f03c0 ret
> > > > ffff8000100f2754: 140007fd b ffff8000100f4748 <ptrace_check_attach+0xf8>
> > > > ...
> > > >
> > > > This looks like the default path (before we write over it) will take you to
> > > > the LSE code (e.g. ffff8000100f2734). I'm pretty sure this is wrong, or at
> > > > least not what we expected to see. Also why 4 branches?
> > >
> > > So I reproduced this with a silly atomic_inc wrapper:
> > >
> > > void will_atomic_inc(atomic_t *v)
> > > {
> > > atomic_inc(v);
> > > }
> > >
> > > Compiles to:
> > >
> > > 0000000000000018 <will_atomic_inc>:
> > > 18: 14000004 b 28 <will_atomic_inc+0x10>
> > > 1c: 14000001 b 20 <will_atomic_inc+0x8>
> > > 20: 14000005 b 34 <will_atomic_inc+0x1c>
> > > 24: 14000001 b 28 <will_atomic_inc+0x10>
> > > 28: 52800028 mov w8, #0x1 // #1
> > > 2c: b828001f stadd w8, [x0]
> > > 30: d65f03c0 ret
> > > 34: 14000027 b d0 <dump_kernel_offset+0x60>
> > > 38: d65f03c0 ret
> > >
> > > which is going to explode.
> >
> > I've come up with a simple reproducer for this issue:
> >
> > static bool branch_jump()
> > {
> > asm_volatile_goto(
> > "1: b %l[l_yes2]"
> > : : : : l_yes2);
> >
> > return false;
> > l_yes2:
> > return true;
> > }
> >
> > static bool branch_test()
> > {
> > return (!branch_jump() && !branch_jump());
> > }
> >
> > void andy_test(int *v)
> > {
> > if (branch_test())
> > *v = 0xff;
> > }
> >
> > This leads to the following (it shouldn't do anything):
> >
> > 0000000000000000 <andy_test>:
> > 0: 14000004 b 10 <andy_test+0x10>
> > 4: 14000001 b 8 <andy_test+0x8>
> > 8: 14000004 b 18 <andy_test+0x18>
> > c: 14000001 b 10 <andy_test+0x10>
> > 10: 52801fe8 mov w8, #0xff // #255
> > 14: b9000008 str w8, [x0]
> > 18: d65f03c0 ret
> >
> > The issue goes away with any of the following hunks:
> >
> >
> > @@ -55,7 +55,7 @@ static bool branch_jump()
> >
> > static bool branch_test()
> > {
> > - return (!branch_jump() && !branch_jump());
> > + return (!branch_jump());
> > }
> >
> > void andy_test(int *v)
> >
> >
> > or:
> >
> >
> > @@ -53,14 +53,10 @@ static bool branch_jump()
> > return true;
> > }
> >
> > -static bool branch_test()
> > -{
> > - return (!branch_jump() && !branch_jump());
> > -}
> >
> > void andy_test(int *v)
> > {
> > - if (branch_test())
> > + if (!branch_jump() && !branch_jump())
> > *v = 0xff;
> > }
>
> Indeed, playing with the definition of `__lse_ll_sc_body`, I can get
> the kernel to boot again.
Thanks for investigating this.
Did it boot to a prompt? I played with the structure of the code and
too was able to get it to boot, but I found that it hung later-on during
boot. Thus I lost a bit of confidence in it.
>
> So I think your very helpful test cases are illustrating two different problems:
> https://godbolt.org/z/dMf7x-
> See the disassembly of `andy_test2`. Reference to the correct label
> is emitted in the inline asm, but there's some silly unconditional
> branches to the next instruction. That's issue #1 and part of the
> reason you see superfluous branches. With that fixed, `andy_test2`
> would match between GCC and Clang. I think that can be a very late
> peephole optimization (and further, we could probably combine labels
> that refer to the same location, oh and .Lfunc_endX could just use
> `.`, too!). LLVM devs noted that the x86 backend doesn't have this
> issue, but this is a curiously recurring pattern I'm noticing in LLVM
> where some arch agnostic optimization is only implemented for x86...
> I'm reading through our Branch Folding pass which I think should
> handle this, but I'll need to fire up a debugger.
>
> Issue #2 is the more critical issue, but may be conflated with issue
> #1. Issue #2 is the nonsensical control flow with one level of
> inlining. See how in the disassembly of `andy_test`, the first label
> referenced from inline assembly is *before* the mov/str when it should
> have been *after*. Not sure where we could be going wrong, but it's
> straightforward for me to observe the code change as its transformed
> through LLVM, and I've debugged and fixed issues related to inlining
> asm goto before.
You may also be interested in this:
https://godbolt.org/z/8OthP2
void andy_test3(int *v)
{
if (!branch_jump())
return;
if (!branch_jump())
return;
*v = 0xff;
}
(I used a similar approach with system_uses_lse_atomics to get the
kernel to boot a bit more).
This generated code does the right thing here (in comparison to andy_test2).
I felt like this gave an insight as to what is going on, but I don't
have the knowledge to know what. It's as if the early return prevents the
compiler from getting confused when it should otherwise jump to the second
goto.
Thanks,
Andrew Murray
> --
> Thanks,
> ~Nick Desaulniers
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v12 01/12] lib: introduce copy_struct_{to,from}_user helpers
From: Aleksa Sarai @ 2019-09-05 11:27 UTC (permalink / raw)
To: Christian Brauner
Cc: linux-ia64, linux-sh, Peter Zijlstra, Rasmus Villemoes,
Alexei Starovoitov, linux-kernel, David Howells, linux-kselftest,
sparclinux, Shuah Khan, linux-arch, linux-s390, Tycho Andersen,
Aleksa Sarai, Jiri Olsa, Alexander Shishkin, Ingo Molnar,
linux-arm-kernel, linux-mips, linux-xtensa, Kees Cook,
Arnd Bergmann, Jann Horn, linuxppc-dev, linux-m68k, Al Viro,
Andy Lutomirski, Shuah Khan, Namhyung Kim, David Drysdale,
Christian Brauner, J. Bruce Fields, linux-parisc, linux-api,
Chanho Min, Jeff Layton, Oleg Nesterov, Eric Biederman,
linux-alpha, linux-fsdevel, Andrew Morton, Linus Torvalds,
containers
In-Reply-To: <20190905110915.4vvhicg4ldmpi5u6@wittgenstein>
[-- Attachment #1.1: Type: text/plain, Size: 1778 bytes --]
On 2019-09-05, Christian Brauner <christian.brauner@ubuntu.com> wrote:
> On Thu, Sep 05, 2019 at 06:19:22AM +1000, Aleksa Sarai wrote:
> > A common pattern for syscall extensions is increasing the size of a
> > struct passed from userspace, such that the zero-value of the new fields
> > result in the old kernel behaviour (allowing for a mix of userspace and
> > kernel vintages to operate on one another in most cases). This is done
> > in both directions -- hence two helpers -- though it's more common to
> > have to copy user space structs into kernel space.
> >
> > Previously there was no common lib/ function that implemented
> > the necessary extension-checking semantics (and different syscalls
> > implemented them slightly differently or incompletely[1]). A future
> > patch replaces all of the common uses of this pattern to use the new
> > copy_struct_{to,from}_user() helpers.
> >
> > [1]: For instance {sched_setattr,perf_event_open,clone3}(2) all do do
> > similar checks to copy_struct_from_user() while rt_sigprocmask(2)
> > always rejects differently-sized struct arguments.
> >
> > Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> > Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
>
> I would probably split this out into a separate patchset. It can very
> well go in before openat2(). Thoughts?
Yeah, I'll split this and the related patches out -- though I will admit
I'm not sure how you're supposed to deal with multiple independent
patchsets that depend on each other. How will folks reviewing openat2(2)
know to include the lib/struct_user.c changes?
Also, whose tree should it go through?
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v12 01/12] lib: introduce copy_struct_{to,from}_user helpers
From: Christian Brauner @ 2019-09-05 11:29 UTC (permalink / raw)
To: Rasmus Villemoes
Cc: linux-ia64, linux-sh, Peter Zijlstra, Alexei Starovoitov,
linux-kernel, David Howells, linux-kselftest, sparclinux,
Shuah Khan, linux-arch, linux-s390, Tycho Andersen, Aleksa Sarai,
Jiri Olsa, Alexander Shishkin, Ingo Molnar, linux-arm-kernel,
linux-mips, linux-xtensa, Kees Cook, Arnd Bergmann, Jann Horn,
linuxppc-dev, Aleksa Sarai, Al Viro, Andy Lutomirski, Shuah Khan,
Namhyung Kim, David Drysdale, Christian Brauner, J. Bruce Fields,
linux-parisc, linux-m68k, linux-api, Chanho Min, Jeff Layton,
Oleg Nesterov, Eric Biederman, linux-alpha, linux-fsdevel,
Andrew Morton, Linus Torvalds, containers
In-Reply-To: <ae415ea8-4442-d81c-3b46-2ae5fb35bbdf@rasmusvillemoes.dk>
On Thu, Sep 05, 2019 at 01:17:38PM +0200, Rasmus Villemoes wrote:
> On 05/09/2019 13.05, Christian Brauner wrote:
> > On Thu, Sep 05, 2019 at 06:19:22AM +1000, Aleksa Sarai wrote:
>
> >> + if (unlikely(!access_ok(dst, usize)))
> >> + return -EFAULT;
> >> +
> >> + /* Deal with trailing bytes. */
> >> + if (usize < ksize) {
> >> + if (memchr_inv(src + size, 0, rest))
> >> + return -EFBIG;
> >> + } else if (usize > ksize) {
> >> + if (__memzero_user(dst + size, rest))
> >> + return -EFAULT;
> >
> > Is zeroing that memory really our job? Seems to me we should just check
> > it is zeroed.
>
> Of course it is, otherwise you'd require userspace to clear the output
> buffer it gives us, which in the majority of cases is wasted work. It's
> much easier to reason about if we just say "the kernel populates [uaddr,
> uaddr + usize)".
I don't really mind either way so sure. :)
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v3 4/7] serial: fsl_linflexuart: Be consistent with the name
From: Stefan-gabriel Mirea @ 2019-09-05 11:32 UTC (permalink / raw)
To: gregkh@linuxfoundation.org
Cc: mark.rutland@arm.com, devicetree@vger.kernel.org, corbet@lwn.net,
catalin.marinas@arm.com, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, Leo Li, robh+dt@kernel.org,
linux-serial@vger.kernel.org, jslaby@suse.com,
shawnguo@kernel.org, will@kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20190904075259.GA10983@kroah.com>
Hello Greg,
On 9/4/2019 10:53 AM, gregkh@linuxfoundation.org wrote:
> On Fri, Aug 23, 2019 at 07:11:37PM +0000, Stefan-gabriel Mirea wrote:
>> For consistency reasons, spell the controller name as "LINFlexD" in
>> comments and documentation.
>>
>> Signed-off-by: Stefan-Gabriel Mirea <stefan-gabriel.mirea@nxp.com>
>> ---
>> Documentation/admin-guide/kernel-parameters.txt | 2 +-
>> drivers/tty/serial/Kconfig | 8 ++++----
>> drivers/tty/serial/fsl_linflexuart.c | 4 ++--
>> include/uapi/linux/serial_core.h | 4 ++--
>> 4 files changed, 9 insertions(+), 9 deletions(-)
>
> Doesn't apply to my tty-next tree :(
Thanks for letting me know; I will fix this in v4.
Regards,
Stefan
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [V2, 1/2] media: i2c: dw9768: Add DT support and MAINTAINERS entry
From: Andy Shevchenko @ 2019-09-05 11:35 UTC (permalink / raw)
To: Sakari Ailus
Cc: mark.rutland, devicetree, drinkcat, srv_heupstream, sam.hung,
shengnan.wang, tfiga, sj.huang, robh+dt, linux-mediatek,
dongchun.zhu, matthias.bgg, bingbu.cao, mchehab, linux-arm-kernel,
linux-media
In-Reply-To: <20190905104829.GB5475@paasikivi.fi.intel.com>
On Thu, Sep 05, 2019 at 01:48:30PM +0300, Sakari Ailus wrote:
> On Thu, Sep 05, 2019 at 01:14:06PM +0300, Andy Shevchenko wrote:
> > On Thu, Sep 05, 2019 at 03:21:41PM +0800, dongchun.zhu@mediatek.com wrote:
> > > From: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > >
> > > This patch is to add the Devicetree binding documentation and
> > > MAINTAINERS entry for dw9768 actuator.
> > >
> > > Signed-off-by: Dongchun Zhu <dongchun.zhu@mediatek.com>
> > > ---
> > > Documentation/devicetree/bindings/media/i2c/dongwoon,dw9768.txt | 9 +++++++++
> > > MAINTAINERS | 7 +++++++
> >
> > This should be:
> > 1) two separate patches
>
> Why? The MAINTAINERS entry is usually added in the first patch needing it,
> isn't it?
Bindings are required to be a separate patch.
Rob, is it still the case or am I mistaken?
--
With Best Regards,
Andy Shevchenko
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* swiotlb-xen cleanups v4
From: Christoph Hellwig @ 2019-09-05 11:33 UTC (permalink / raw)
To: Stefano Stabellini, Konrad Rzeszutek Wilk, gross, boris.ostrovsky
Cc: xen-devel, iommu, x86, linux-kernel, linux-arm-kernel
Hi Xen maintainers and friends,
please take a look at this series that cleans up the parts of swiotlb-xen
that deal with non-coherent caches.
Changes since v3:
- don't use dma_direct_alloc on x86
Changes since v2:
- further dma_cache_maint improvements
- split the previous patch 1 into 3 patches
Changes since v1:
- rewrite dma_cache_maint to be much simpler
- improve various comments and commit logs
- remove page-coherent.h entirely
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [V2, 2/2] media: i2c: Add DW9768 VCM driver
From: Andy Shevchenko @ 2019-09-05 11:36 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: mark.rutland, devicetree, drinkcat, srv_heupstream, sam.hung,
shengnan.wang, tfiga, sj.huang, robh+dt, linux-mediatek,
dongchun.zhu, Sakari Ailus, matthias.bgg, bingbu.cao, mchehab,
linux-arm-kernel, linux-media
In-Reply-To: <ad357e27-3e51-6922-1924-5d2c2daf1934@redhat.com>
On Thu, Sep 05, 2019 at 12:57:34PM +0200, Javier Martinez Canillas wrote:
> On 9/5/19 12:40 PM, Sakari Ailus wrote:
> > On Thu, Sep 05, 2019 at 01:19:08PM +0300, Andy Shevchenko wrote:
> >> On Thu, Sep 05, 2019 at 11:21:34AM +0300, Sakari Ailus wrote:
> >>> On Thu, Sep 05, 2019 at 03:21:42PM +0800, dongchun.zhu@mediatek.com wrote:
> >>>> From: Dongchun Zhu <dongchun.zhu@mediatek.com>
> >>
> >>>> +static const struct i2c_device_id dw9768_id_table[] = {
> >>>> + { DW9768_NAME, 0 },
> >>>> + { },
> >>>
> >>> Could you drop the I²C ID table?
> >>
> >> But why?
> >> It will allow you to instanciate the device from user space.
>
> Yes, the I2C device table is still needed if the device can be instantiated
> from user-space using the sysfs interface, or otherwise the module won't be
> automatically loaded.
>
> Kieran posted a "[PATCH RFC] modpost: Support I2C Aliases from OF tables"
> patch that adds a MODULE_DEVICE_TABLE(i2c_of, ..) macro so modpost could
> add legacy I2C modalias using the information in the OF device ID tables:
>
> https://patchwork.kernel.org/patch/11038861/
>
> If that lands, then we could get rid of the I2C device tables altogether
> for non-legacy I2C drivers.
>
> >
> > The device is supposed to be present in DT (or ACPI tables) already.
> >
>
> Agreed. Also by looking at the driver's probe function I see that the
> device lookups a 'vin' and 'vdd' regulators supplies and it fails if
> aren't defined, so it can't be instantiated from user-space anyways.
Thank you for clarifications!
So, it can use ->probe_new() in that case.
>
> BTW, these two regulators supplies should be listed as 'vin-supply'
> and 'vdd-supply' as required properties in the DT binding document.
--
With Best Regards,
Andy Shevchenko
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 01/11] xen/arm: use dma-noncoherent.h calls for xen-swiotlb cache maintainance
From: Christoph Hellwig @ 2019-09-05 11:33 UTC (permalink / raw)
To: Stefano Stabellini, Konrad Rzeszutek Wilk, gross, boris.ostrovsky
Cc: xen-devel, iommu, x86, linux-kernel, linux-arm-kernel
In-Reply-To: <20190905113408.3104-1-hch@lst.de>
Copy the arm64 code that uses the dma-direct/swiotlb helpers for DMA
on-coherent devices.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/arm/include/asm/device.h | 3 -
arch/arm/include/asm/xen/page-coherent.h | 72 +++++++++---------------
arch/arm/mm/dma-mapping.c | 8 +--
drivers/xen/swiotlb-xen.c | 20 -------
4 files changed, 28 insertions(+), 75 deletions(-)
diff --git a/arch/arm/include/asm/device.h b/arch/arm/include/asm/device.h
index f6955b55c544..c675bc0d5aa8 100644
--- a/arch/arm/include/asm/device.h
+++ b/arch/arm/include/asm/device.h
@@ -14,9 +14,6 @@ struct dev_archdata {
#endif
#ifdef CONFIG_ARM_DMA_USE_IOMMU
struct dma_iommu_mapping *mapping;
-#endif
-#ifdef CONFIG_XEN
- const struct dma_map_ops *dev_dma_ops;
#endif
unsigned int dma_coherent:1;
unsigned int dma_ops_setup:1;
diff --git a/arch/arm/include/asm/xen/page-coherent.h b/arch/arm/include/asm/xen/page-coherent.h
index 2c403e7c782d..602ac02f154c 100644
--- a/arch/arm/include/asm/xen/page-coherent.h
+++ b/arch/arm/include/asm/xen/page-coherent.h
@@ -6,23 +6,37 @@
#include <asm/page.h>
#include <xen/arm/page-coherent.h>
-static inline const struct dma_map_ops *xen_get_dma_ops(struct device *dev)
-{
- if (dev && dev->archdata.dev_dma_ops)
- return dev->archdata.dev_dma_ops;
- return get_arch_dma_ops(NULL);
-}
-
static inline void *xen_alloc_coherent_pages(struct device *hwdev, size_t size,
dma_addr_t *dma_handle, gfp_t flags, unsigned long attrs)
{
- return xen_get_dma_ops(hwdev)->alloc(hwdev, size, dma_handle, flags, attrs);
+ return dma_direct_alloc(hwdev, size, dma_handle, flags, attrs);
}
static inline void xen_free_coherent_pages(struct device *hwdev, size_t size,
void *cpu_addr, dma_addr_t dma_handle, unsigned long attrs)
{
- xen_get_dma_ops(hwdev)->free(hwdev, size, cpu_addr, dma_handle, attrs);
+ dma_direct_free(hwdev, size, cpu_addr, dma_handle, attrs);
+}
+
+static inline void xen_dma_sync_single_for_cpu(struct device *hwdev,
+ dma_addr_t handle, size_t size, enum dma_data_direction dir)
+{
+ unsigned long pfn = PFN_DOWN(handle);
+
+ if (pfn_valid(pfn))
+ dma_direct_sync_single_for_cpu(hwdev, handle, size, dir);
+ else
+ __xen_dma_sync_single_for_cpu(hwdev, handle, size, dir);
+}
+
+static inline void xen_dma_sync_single_for_device(struct device *hwdev,
+ dma_addr_t handle, size_t size, enum dma_data_direction dir)
+{
+ unsigned long pfn = PFN_DOWN(handle);
+ if (pfn_valid(pfn))
+ dma_direct_sync_single_for_device(hwdev, handle, size, dir);
+ else
+ __xen_dma_sync_single_for_device(hwdev, handle, size, dir);
}
static inline void xen_dma_map_page(struct device *hwdev, struct page *page,
@@ -36,17 +50,8 @@ static inline void xen_dma_map_page(struct device *hwdev, struct page *page,
bool local = (page_pfn <= dev_pfn) &&
(dev_pfn - page_pfn < compound_pages);
- /*
- * Dom0 is mapped 1:1, while the Linux page can span across
- * multiple Xen pages, it's not possible for it to contain a
- * mix of local and foreign Xen pages. So if the first xen_pfn
- * == mfn the page is local otherwise it's a foreign page
- * grant-mapped in dom0. If the page is local we can safely
- * call the native dma_ops function, otherwise we call the xen
- * specific function.
- */
if (local)
- xen_get_dma_ops(hwdev)->map_page(hwdev, page, offset, size, dir, attrs);
+ dma_direct_map_page(hwdev, page, offset, size, dir, attrs);
else
__xen_dma_map_page(hwdev, page, dev_addr, offset, size, dir, attrs);
}
@@ -63,33 +68,10 @@ static inline void xen_dma_unmap_page(struct device *hwdev, dma_addr_t handle,
* safely call the native dma_ops function, otherwise we call the xen
* specific function.
*/
- if (pfn_valid(pfn)) {
- if (xen_get_dma_ops(hwdev)->unmap_page)
- xen_get_dma_ops(hwdev)->unmap_page(hwdev, handle, size, dir, attrs);
- } else
+ if (pfn_valid(pfn))
+ dma_direct_unmap_page(hwdev, handle, size, dir, attrs);
+ else
__xen_dma_unmap_page(hwdev, handle, size, dir, attrs);
}
-static inline void xen_dma_sync_single_for_cpu(struct device *hwdev,
- dma_addr_t handle, size_t size, enum dma_data_direction dir)
-{
- unsigned long pfn = PFN_DOWN(handle);
- if (pfn_valid(pfn)) {
- if (xen_get_dma_ops(hwdev)->sync_single_for_cpu)
- xen_get_dma_ops(hwdev)->sync_single_for_cpu(hwdev, handle, size, dir);
- } else
- __xen_dma_sync_single_for_cpu(hwdev, handle, size, dir);
-}
-
-static inline void xen_dma_sync_single_for_device(struct device *hwdev,
- dma_addr_t handle, size_t size, enum dma_data_direction dir)
-{
- unsigned long pfn = PFN_DOWN(handle);
- if (pfn_valid(pfn)) {
- if (xen_get_dma_ops(hwdev)->sync_single_for_device)
- xen_get_dma_ops(hwdev)->sync_single_for_device(hwdev, handle, size, dir);
- } else
- __xen_dma_sync_single_for_device(hwdev, handle, size, dir);
-}
-
#endif /* _ASM_ARM_XEN_PAGE_COHERENT_H */
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index d42557ee69c2..738097396445 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -1132,10 +1132,6 @@ static const struct dma_map_ops *arm_get_dma_map_ops(bool coherent)
* 32-bit DMA.
* Use the generic dma-direct / swiotlb ops code in that case, as that
* handles bounce buffering for us.
- *
- * Note: this checks CONFIG_ARM_LPAE instead of CONFIG_SWIOTLB as the
- * latter is also selected by the Xen code, but that code for now relies
- * on non-NULL dev_dma_ops. To be cleaned up later.
*/
if (IS_ENABLED(CONFIG_ARM_LPAE))
return NULL;
@@ -2363,10 +2359,8 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
set_dma_ops(dev, dma_ops);
#ifdef CONFIG_XEN
- if (xen_initial_domain()) {
- dev->archdata.dev_dma_ops = dev->dma_ops;
+ if (xen_initial_domain())
dev->dma_ops = xen_dma_ops;
- }
#endif
dev->archdata.dma_ops_setup = true;
}
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index ae1df496bf38..eee86cc7046b 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -557,11 +557,6 @@ xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma,
void *cpu_addr, dma_addr_t dma_addr, size_t size,
unsigned long attrs)
{
-#ifdef CONFIG_ARM
- if (xen_get_dma_ops(dev)->mmap)
- return xen_get_dma_ops(dev)->mmap(dev, vma, cpu_addr,
- dma_addr, size, attrs);
-#endif
return dma_common_mmap(dev, vma, cpu_addr, dma_addr, size, attrs);
}
@@ -574,21 +569,6 @@ xen_swiotlb_get_sgtable(struct device *dev, struct sg_table *sgt,
void *cpu_addr, dma_addr_t handle, size_t size,
unsigned long attrs)
{
-#ifdef CONFIG_ARM
- if (xen_get_dma_ops(dev)->get_sgtable) {
-#if 0
- /*
- * This check verifies that the page belongs to the current domain and
- * is not one mapped from another domain.
- * This check is for debug only, and should not go to production build
- */
- unsigned long bfn = PHYS_PFN(dma_to_phys(dev, handle));
- BUG_ON (!page_is_ram(bfn));
-#endif
- return xen_get_dma_ops(dev)->get_sgtable(dev, sgt, cpu_addr,
- handle, size, attrs);
- }
-#endif
return dma_common_get_sgtable(dev, sgt, cpu_addr, handle, size, attrs);
}
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 02/11] xen/arm: consolidate page-coherent.h
From: Christoph Hellwig @ 2019-09-05 11:33 UTC (permalink / raw)
To: Stefano Stabellini, Konrad Rzeszutek Wilk, gross, boris.ostrovsky
Cc: xen-devel, iommu, x86, linux-kernel, linux-arm-kernel
In-Reply-To: <20190905113408.3104-1-hch@lst.de>
Shared the duplicate arm/arm64 code in include/xen/arm/page-coherent.h.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
arch/arm/include/asm/xen/page-coherent.h | 75 --------------------
arch/arm64/include/asm/xen/page-coherent.h | 75 --------------------
include/xen/arm/page-coherent.h | 80 ++++++++++++++++++++++
3 files changed, 80 insertions(+), 150 deletions(-)
diff --git a/arch/arm/include/asm/xen/page-coherent.h b/arch/arm/include/asm/xen/page-coherent.h
index 602ac02f154c..27e984977402 100644
--- a/arch/arm/include/asm/xen/page-coherent.h
+++ b/arch/arm/include/asm/xen/page-coherent.h
@@ -1,77 +1,2 @@
/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef _ASM_ARM_XEN_PAGE_COHERENT_H
-#define _ASM_ARM_XEN_PAGE_COHERENT_H
-
-#include <linux/dma-mapping.h>
-#include <asm/page.h>
#include <xen/arm/page-coherent.h>
-
-static inline void *xen_alloc_coherent_pages(struct device *hwdev, size_t size,
- dma_addr_t *dma_handle, gfp_t flags, unsigned long attrs)
-{
- return dma_direct_alloc(hwdev, size, dma_handle, flags, attrs);
-}
-
-static inline void xen_free_coherent_pages(struct device *hwdev, size_t size,
- void *cpu_addr, dma_addr_t dma_handle, unsigned long attrs)
-{
- dma_direct_free(hwdev, size, cpu_addr, dma_handle, attrs);
-}
-
-static inline void xen_dma_sync_single_for_cpu(struct device *hwdev,
- dma_addr_t handle, size_t size, enum dma_data_direction dir)
-{
- unsigned long pfn = PFN_DOWN(handle);
-
- if (pfn_valid(pfn))
- dma_direct_sync_single_for_cpu(hwdev, handle, size, dir);
- else
- __xen_dma_sync_single_for_cpu(hwdev, handle, size, dir);
-}
-
-static inline void xen_dma_sync_single_for_device(struct device *hwdev,
- dma_addr_t handle, size_t size, enum dma_data_direction dir)
-{
- unsigned long pfn = PFN_DOWN(handle);
- if (pfn_valid(pfn))
- dma_direct_sync_single_for_device(hwdev, handle, size, dir);
- else
- __xen_dma_sync_single_for_device(hwdev, handle, size, dir);
-}
-
-static inline void xen_dma_map_page(struct device *hwdev, struct page *page,
- dma_addr_t dev_addr, unsigned long offset, size_t size,
- enum dma_data_direction dir, unsigned long attrs)
-{
- unsigned long page_pfn = page_to_xen_pfn(page);
- unsigned long dev_pfn = XEN_PFN_DOWN(dev_addr);
- unsigned long compound_pages =
- (1<<compound_order(page)) * XEN_PFN_PER_PAGE;
- bool local = (page_pfn <= dev_pfn) &&
- (dev_pfn - page_pfn < compound_pages);
-
- if (local)
- dma_direct_map_page(hwdev, page, offset, size, dir, attrs);
- else
- __xen_dma_map_page(hwdev, page, dev_addr, offset, size, dir, attrs);
-}
-
-static inline void xen_dma_unmap_page(struct device *hwdev, dma_addr_t handle,
- size_t size, enum dma_data_direction dir, unsigned long attrs)
-{
- unsigned long pfn = PFN_DOWN(handle);
- /*
- * Dom0 is mapped 1:1, while the Linux page can be spanned accross
- * multiple Xen page, it's not possible to have a mix of local and
- * foreign Xen page. Dom0 is mapped 1:1, so calling pfn_valid on a
- * foreign mfn will always return false. If the page is local we can
- * safely call the native dma_ops function, otherwise we call the xen
- * specific function.
- */
- if (pfn_valid(pfn))
- dma_direct_unmap_page(hwdev, handle, size, dir, attrs);
- else
- __xen_dma_unmap_page(hwdev, handle, size, dir, attrs);
-}
-
-#endif /* _ASM_ARM_XEN_PAGE_COHERENT_H */
diff --git a/arch/arm64/include/asm/xen/page-coherent.h b/arch/arm64/include/asm/xen/page-coherent.h
index d88e56b90b93..27e984977402 100644
--- a/arch/arm64/include/asm/xen/page-coherent.h
+++ b/arch/arm64/include/asm/xen/page-coherent.h
@@ -1,77 +1,2 @@
/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef _ASM_ARM64_XEN_PAGE_COHERENT_H
-#define _ASM_ARM64_XEN_PAGE_COHERENT_H
-
-#include <linux/dma-mapping.h>
-#include <asm/page.h>
#include <xen/arm/page-coherent.h>
-
-static inline void *xen_alloc_coherent_pages(struct device *hwdev, size_t size,
- dma_addr_t *dma_handle, gfp_t flags, unsigned long attrs)
-{
- return dma_direct_alloc(hwdev, size, dma_handle, flags, attrs);
-}
-
-static inline void xen_free_coherent_pages(struct device *hwdev, size_t size,
- void *cpu_addr, dma_addr_t dma_handle, unsigned long attrs)
-{
- dma_direct_free(hwdev, size, cpu_addr, dma_handle, attrs);
-}
-
-static inline void xen_dma_sync_single_for_cpu(struct device *hwdev,
- dma_addr_t handle, size_t size, enum dma_data_direction dir)
-{
- unsigned long pfn = PFN_DOWN(handle);
-
- if (pfn_valid(pfn))
- dma_direct_sync_single_for_cpu(hwdev, handle, size, dir);
- else
- __xen_dma_sync_single_for_cpu(hwdev, handle, size, dir);
-}
-
-static inline void xen_dma_sync_single_for_device(struct device *hwdev,
- dma_addr_t handle, size_t size, enum dma_data_direction dir)
-{
- unsigned long pfn = PFN_DOWN(handle);
- if (pfn_valid(pfn))
- dma_direct_sync_single_for_device(hwdev, handle, size, dir);
- else
- __xen_dma_sync_single_for_device(hwdev, handle, size, dir);
-}
-
-static inline void xen_dma_map_page(struct device *hwdev, struct page *page,
- dma_addr_t dev_addr, unsigned long offset, size_t size,
- enum dma_data_direction dir, unsigned long attrs)
-{
- unsigned long page_pfn = page_to_xen_pfn(page);
- unsigned long dev_pfn = XEN_PFN_DOWN(dev_addr);
- unsigned long compound_pages =
- (1<<compound_order(page)) * XEN_PFN_PER_PAGE;
- bool local = (page_pfn <= dev_pfn) &&
- (dev_pfn - page_pfn < compound_pages);
-
- if (local)
- dma_direct_map_page(hwdev, page, offset, size, dir, attrs);
- else
- __xen_dma_map_page(hwdev, page, dev_addr, offset, size, dir, attrs);
-}
-
-static inline void xen_dma_unmap_page(struct device *hwdev, dma_addr_t handle,
- size_t size, enum dma_data_direction dir, unsigned long attrs)
-{
- unsigned long pfn = PFN_DOWN(handle);
- /*
- * Dom0 is mapped 1:1, while the Linux page can be spanned accross
- * multiple Xen page, it's not possible to have a mix of local and
- * foreign Xen page. Dom0 is mapped 1:1, so calling pfn_valid on a
- * foreign mfn will always return false. If the page is local we can
- * safely call the native dma_ops function, otherwise we call the xen
- * specific function.
- */
- if (pfn_valid(pfn))
- dma_direct_unmap_page(hwdev, handle, size, dir, attrs);
- else
- __xen_dma_unmap_page(hwdev, handle, size, dir, attrs);
-}
-
-#endif /* _ASM_ARM64_XEN_PAGE_COHERENT_H */
diff --git a/include/xen/arm/page-coherent.h b/include/xen/arm/page-coherent.h
index 2ca9164a79bf..a840d6949a87 100644
--- a/include/xen/arm/page-coherent.h
+++ b/include/xen/arm/page-coherent.h
@@ -2,6 +2,9 @@
#ifndef _XEN_ARM_PAGE_COHERENT_H
#define _XEN_ARM_PAGE_COHERENT_H
+#include <linux/dma-mapping.h>
+#include <asm/page.h>
+
void __xen_dma_map_page(struct device *hwdev, struct page *page,
dma_addr_t dev_addr, unsigned long offset, size_t size,
enum dma_data_direction dir, unsigned long attrs);
@@ -13,4 +16,81 @@ void __xen_dma_sync_single_for_cpu(struct device *hwdev,
void __xen_dma_sync_single_for_device(struct device *hwdev,
dma_addr_t handle, size_t size, enum dma_data_direction dir);
+static inline void *xen_alloc_coherent_pages(struct device *hwdev, size_t size,
+ dma_addr_t *dma_handle, gfp_t flags, unsigned long attrs)
+{
+ return dma_direct_alloc(hwdev, size, dma_handle, flags, attrs);
+}
+
+static inline void xen_free_coherent_pages(struct device *hwdev, size_t size,
+ void *cpu_addr, dma_addr_t dma_handle, unsigned long attrs)
+{
+ dma_direct_free(hwdev, size, cpu_addr, dma_handle, attrs);
+}
+
+static inline void xen_dma_sync_single_for_cpu(struct device *hwdev,
+ dma_addr_t handle, size_t size, enum dma_data_direction dir)
+{
+ unsigned long pfn = PFN_DOWN(handle);
+
+ if (pfn_valid(pfn))
+ dma_direct_sync_single_for_cpu(hwdev, handle, size, dir);
+ else
+ __xen_dma_sync_single_for_cpu(hwdev, handle, size, dir);
+}
+
+static inline void xen_dma_sync_single_for_device(struct device *hwdev,
+ dma_addr_t handle, size_t size, enum dma_data_direction dir)
+{
+ unsigned long pfn = PFN_DOWN(handle);
+ if (pfn_valid(pfn))
+ dma_direct_sync_single_for_device(hwdev, handle, size, dir);
+ else
+ __xen_dma_sync_single_for_device(hwdev, handle, size, dir);
+}
+
+static inline void xen_dma_map_page(struct device *hwdev, struct page *page,
+ dma_addr_t dev_addr, unsigned long offset, size_t size,
+ enum dma_data_direction dir, unsigned long attrs)
+{
+ unsigned long page_pfn = page_to_xen_pfn(page);
+ unsigned long dev_pfn = XEN_PFN_DOWN(dev_addr);
+ unsigned long compound_pages =
+ (1<<compound_order(page)) * XEN_PFN_PER_PAGE;
+ bool local = (page_pfn <= dev_pfn) &&
+ (dev_pfn - page_pfn < compound_pages);
+
+ /*
+ * Dom0 is mapped 1:1, while the Linux page can span across
+ * multiple Xen pages, it's not possible for it to contain a
+ * mix of local and foreign Xen pages. So if the first xen_pfn
+ * == mfn the page is local otherwise it's a foreign page
+ * grant-mapped in dom0. If the page is local we can safely
+ * call the native dma_ops function, otherwise we call the xen
+ * specific function.
+ */
+ if (local)
+ dma_direct_map_page(hwdev, page, offset, size, dir, attrs);
+ else
+ __xen_dma_map_page(hwdev, page, dev_addr, offset, size, dir, attrs);
+}
+
+static inline void xen_dma_unmap_page(struct device *hwdev, dma_addr_t handle,
+ size_t size, enum dma_data_direction dir, unsigned long attrs)
+{
+ unsigned long pfn = PFN_DOWN(handle);
+ /*
+ * Dom0 is mapped 1:1, while the Linux page can be spanned accross
+ * multiple Xen page, it's not possible to have a mix of local and
+ * foreign Xen page. Dom0 is mapped 1:1, so calling pfn_valid on a
+ * foreign mfn will always return false. If the page is local we can
+ * safely call the native dma_ops function, otherwise we call the xen
+ * specific function.
+ */
+ if (pfn_valid(pfn))
+ dma_direct_unmap_page(hwdev, handle, size, dir, attrs);
+ else
+ __xen_dma_unmap_page(hwdev, handle, size, dir, attrs);
+}
+
#endif /* _XEN_ARM_PAGE_COHERENT_H */
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 03/11] xen/arm: use dev_is_dma_coherent
From: Christoph Hellwig @ 2019-09-05 11:34 UTC (permalink / raw)
To: Stefano Stabellini, Konrad Rzeszutek Wilk, gross, boris.ostrovsky
Cc: x86, Julien Grall, linux-kernel, iommu, xen-devel,
linux-arm-kernel
In-Reply-To: <20190905113408.3104-1-hch@lst.de>
Use the dma-noncoherent dev_is_dma_coherent helper instead of the home
grown variant. Note that both are always initialized to the same
value in arch_setup_dma_ops.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
arch/arm/include/asm/dma-mapping.h | 6 ------
arch/arm/xen/mm.c | 12 ++++++------
arch/arm64/include/asm/dma-mapping.h | 9 ---------
3 files changed, 6 insertions(+), 21 deletions(-)
diff --git a/arch/arm/include/asm/dma-mapping.h b/arch/arm/include/asm/dma-mapping.h
index dba9355e2484..bdd80ddbca34 100644
--- a/arch/arm/include/asm/dma-mapping.h
+++ b/arch/arm/include/asm/dma-mapping.h
@@ -91,12 +91,6 @@ static inline dma_addr_t virt_to_dma(struct device *dev, void *addr)
}
#endif
-/* do not use this function in a driver */
-static inline bool is_device_dma_coherent(struct device *dev)
-{
- return dev->archdata.dma_coherent;
-}
-
/**
* arm_dma_alloc - allocate consistent memory for DMA
* @dev: valid struct device pointer, or NULL for ISA and EISA-like devices
diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
index d33b77e9add3..90574d89d0d4 100644
--- a/arch/arm/xen/mm.c
+++ b/arch/arm/xen/mm.c
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: GPL-2.0-only
#include <linux/cpu.h>
-#include <linux/dma-mapping.h>
+#include <linux/dma-noncoherent.h>
#include <linux/gfp.h>
#include <linux/highmem.h>
#include <linux/export.h>
@@ -99,7 +99,7 @@ void __xen_dma_map_page(struct device *hwdev, struct page *page,
dma_addr_t dev_addr, unsigned long offset, size_t size,
enum dma_data_direction dir, unsigned long attrs)
{
- if (is_device_dma_coherent(hwdev))
+ if (dev_is_dma_coherent(hwdev))
return;
if (attrs & DMA_ATTR_SKIP_CPU_SYNC)
return;
@@ -112,7 +112,7 @@ void __xen_dma_unmap_page(struct device *hwdev, dma_addr_t handle,
unsigned long attrs)
{
- if (is_device_dma_coherent(hwdev))
+ if (dev_is_dma_coherent(hwdev))
return;
if (attrs & DMA_ATTR_SKIP_CPU_SYNC)
return;
@@ -123,7 +123,7 @@ void __xen_dma_unmap_page(struct device *hwdev, dma_addr_t handle,
void __xen_dma_sync_single_for_cpu(struct device *hwdev,
dma_addr_t handle, size_t size, enum dma_data_direction dir)
{
- if (is_device_dma_coherent(hwdev))
+ if (dev_is_dma_coherent(hwdev))
return;
__xen_dma_page_dev_to_cpu(hwdev, handle, size, dir);
}
@@ -131,7 +131,7 @@ void __xen_dma_sync_single_for_cpu(struct device *hwdev,
void __xen_dma_sync_single_for_device(struct device *hwdev,
dma_addr_t handle, size_t size, enum dma_data_direction dir)
{
- if (is_device_dma_coherent(hwdev))
+ if (dev_is_dma_coherent(hwdev))
return;
__xen_dma_page_cpu_to_dev(hwdev, handle, size, dir);
}
@@ -159,7 +159,7 @@ bool xen_arch_need_swiotlb(struct device *dev,
* memory and we are not able to flush the cache.
*/
return (!hypercall_cflush && (xen_pfn != bfn) &&
- !is_device_dma_coherent(dev));
+ !dev_is_dma_coherent(dev));
}
int xen_create_contiguous_region(phys_addr_t pstart, unsigned int order,
diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h
index bdcb0922a40c..67243255a858 100644
--- a/arch/arm64/include/asm/dma-mapping.h
+++ b/arch/arm64/include/asm/dma-mapping.h
@@ -18,14 +18,5 @@ static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type *bus)
return NULL;
}
-/*
- * Do not use this function in a driver, it is only provided for
- * arch/arm/mm/xen.c, which is used by arm64 as well.
- */
-static inline bool is_device_dma_coherent(struct device *dev)
-{
- return dev->dma_coherent;
-}
-
#endif /* __KERNEL__ */
#endif /* __ASM_DMA_MAPPING_H */
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 04/11] xen/arm: simplify dma_cache_maint
From: Christoph Hellwig @ 2019-09-05 11:34 UTC (permalink / raw)
To: Stefano Stabellini, Konrad Rzeszutek Wilk, gross, boris.ostrovsky
Cc: xen-devel, iommu, x86, linux-kernel, linux-arm-kernel
In-Reply-To: <20190905113408.3104-1-hch@lst.de>
Calculate the required operation in the caller, and pass it directly
instead of recalculating it for each page, and use simple arithmetics
to get from the physical address to Xen page size aligned chunks.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
arch/arm/xen/mm.c | 61 ++++++++++++++++-------------------------------
1 file changed, 21 insertions(+), 40 deletions(-)
diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
index 90574d89d0d4..2fde161733b0 100644
--- a/arch/arm/xen/mm.c
+++ b/arch/arm/xen/mm.c
@@ -35,64 +35,45 @@ unsigned long xen_get_swiotlb_free_pages(unsigned int order)
return __get_free_pages(flags, order);
}
-enum dma_cache_op {
- DMA_UNMAP,
- DMA_MAP,
-};
static bool hypercall_cflush = false;
-/* functions called by SWIOTLB */
-
-static void dma_cache_maint(dma_addr_t handle, unsigned long offset,
- size_t size, enum dma_data_direction dir, enum dma_cache_op op)
+/* buffers in highmem or foreign pages cannot cross page boundaries */
+static void dma_cache_maint(dma_addr_t handle, size_t size, u32 op)
{
struct gnttab_cache_flush cflush;
- unsigned long xen_pfn;
- size_t left = size;
- xen_pfn = (handle >> XEN_PAGE_SHIFT) + offset / XEN_PAGE_SIZE;
- offset %= XEN_PAGE_SIZE;
+ cflush.a.dev_bus_addr = handle & XEN_PAGE_MASK;
+ cflush.offset = xen_offset_in_page(handle);
+ cflush.op = op;
do {
- size_t len = left;
-
- /* buffers in highmem or foreign pages cannot cross page
- * boundaries */
- if (len + offset > XEN_PAGE_SIZE)
- len = XEN_PAGE_SIZE - offset;
-
- cflush.op = 0;
- cflush.a.dev_bus_addr = xen_pfn << XEN_PAGE_SHIFT;
- cflush.offset = offset;
- cflush.length = len;
-
- if (op == DMA_UNMAP && dir != DMA_TO_DEVICE)
- cflush.op = GNTTAB_CACHE_INVAL;
- if (op == DMA_MAP) {
- if (dir == DMA_FROM_DEVICE)
- cflush.op = GNTTAB_CACHE_INVAL;
- else
- cflush.op = GNTTAB_CACHE_CLEAN;
- }
- if (cflush.op)
- HYPERVISOR_grant_table_op(GNTTABOP_cache_flush, &cflush, 1);
+ if (size + cflush.offset > XEN_PAGE_SIZE)
+ cflush.length = XEN_PAGE_SIZE - cflush.offset;
+ else
+ cflush.length = size;
+
+ HYPERVISOR_grant_table_op(GNTTABOP_cache_flush, &cflush, 1);
- offset = 0;
- xen_pfn++;
- left -= len;
- } while (left);
+ cflush.offset = 0;
+ cflush.a.dev_bus_addr += cflush.length;
+ size -= cflush.length;
+ } while (size);
}
static void __xen_dma_page_dev_to_cpu(struct device *hwdev, dma_addr_t handle,
size_t size, enum dma_data_direction dir)
{
- dma_cache_maint(handle & PAGE_MASK, handle & ~PAGE_MASK, size, dir, DMA_UNMAP);
+ if (dir != DMA_TO_DEVICE)
+ dma_cache_maint(handle, size, GNTTAB_CACHE_INVAL);
}
static void __xen_dma_page_cpu_to_dev(struct device *hwdev, dma_addr_t handle,
size_t size, enum dma_data_direction dir)
{
- dma_cache_maint(handle & PAGE_MASK, handle & ~PAGE_MASK, size, dir, DMA_MAP);
+ if (dir == DMA_FROM_DEVICE)
+ dma_cache_maint(handle, size, GNTTAB_CACHE_INVAL);
+ else
+ dma_cache_maint(handle, size, GNTTAB_CACHE_CLEAN);
}
void __xen_dma_map_page(struct device *hwdev, struct page *page,
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 05/11] xen/arm: remove xen_dma_ops
From: Christoph Hellwig @ 2019-09-05 11:34 UTC (permalink / raw)
To: Stefano Stabellini, Konrad Rzeszutek Wilk, gross, boris.ostrovsky
Cc: x86, Julien Grall, linux-kernel, iommu, xen-devel,
linux-arm-kernel
In-Reply-To: <20190905113408.3104-1-hch@lst.de>
arm and arm64 can just use xen_swiotlb_dma_ops directly like x86, no
need for a pointer indirection.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Julien Grall <julien.grall@arm.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
arch/arm/mm/dma-mapping.c | 3 ++-
arch/arm/xen/mm.c | 4 ----
arch/arm64/mm/dma-mapping.c | 3 ++-
include/xen/arm/hypervisor.h | 2 --
4 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 738097396445..2661cad36359 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -35,6 +35,7 @@
#include <asm/mach/map.h>
#include <asm/system_info.h>
#include <asm/dma-contiguous.h>
+#include <xen/swiotlb-xen.h>
#include "dma.h"
#include "mm.h"
@@ -2360,7 +2361,7 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
#ifdef CONFIG_XEN
if (xen_initial_domain())
- dev->dma_ops = xen_dma_ops;
+ dev->dma_ops = &xen_swiotlb_dma_ops;
#endif
dev->archdata.dma_ops_setup = true;
}
diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
index 2fde161733b0..11d5ad26fcfe 100644
--- a/arch/arm/xen/mm.c
+++ b/arch/arm/xen/mm.c
@@ -162,16 +162,12 @@ void xen_destroy_contiguous_region(phys_addr_t pstart, unsigned int order)
}
EXPORT_SYMBOL_GPL(xen_destroy_contiguous_region);
-const struct dma_map_ops *xen_dma_ops;
-EXPORT_SYMBOL(xen_dma_ops);
-
int __init xen_mm_init(void)
{
struct gnttab_cache_flush cflush;
if (!xen_initial_domain())
return 0;
xen_swiotlb_init(1, false);
- xen_dma_ops = &xen_swiotlb_dma_ops;
cflush.op = 0;
cflush.a.dev_bus_addr = 0;
diff --git a/arch/arm64/mm/dma-mapping.c b/arch/arm64/mm/dma-mapping.c
index bd2b039f43a6..4b244a037349 100644
--- a/arch/arm64/mm/dma-mapping.c
+++ b/arch/arm64/mm/dma-mapping.c
@@ -8,6 +8,7 @@
#include <linux/cache.h>
#include <linux/dma-noncoherent.h>
#include <linux/dma-iommu.h>
+#include <xen/swiotlb-xen.h>
#include <asm/cacheflush.h>
@@ -64,6 +65,6 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
#ifdef CONFIG_XEN
if (xen_initial_domain())
- dev->dma_ops = xen_dma_ops;
+ dev->dma_ops = &xen_swiotlb_dma_ops;
#endif
}
diff --git a/include/xen/arm/hypervisor.h b/include/xen/arm/hypervisor.h
index 2982571f7cc1..43ef24dd030e 100644
--- a/include/xen/arm/hypervisor.h
+++ b/include/xen/arm/hypervisor.h
@@ -19,8 +19,6 @@ static inline enum paravirt_lazy_mode paravirt_get_lazy_mode(void)
return PARAVIRT_LAZY_NONE;
}
-extern const struct dma_map_ops *xen_dma_ops;
-
#ifdef CONFIG_XEN
void __init xen_early_init(void);
#else
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH v12 01/12] lib: introduce copy_struct_{to,from}_user helpers
From: Christian Brauner @ 2019-09-05 11:40 UTC (permalink / raw)
To: Aleksa Sarai
Cc: linux-ia64, linux-sh, Peter Zijlstra, Rasmus Villemoes,
Alexei Starovoitov, linux-kernel, David Howells, linux-kselftest,
sparclinux, Shuah Khan, linux-arch, linux-s390, Tycho Andersen,
Aleksa Sarai, Jiri Olsa, Alexander Shishkin, Ingo Molnar,
linux-arm-kernel, linux-mips, linux-xtensa, Kees Cook,
Arnd Bergmann, Jann Horn, linuxppc-dev, linux-m68k, Al Viro,
Andy Lutomirski, Shuah Khan, Namhyung Kim, David Drysdale,
Christian Brauner, J. Bruce Fields, linux-parisc, linux-api,
Chanho Min, Jeff Layton, Oleg Nesterov, Eric Biederman,
linux-alpha, linux-fsdevel, Andrew Morton, Linus Torvalds,
containers
In-Reply-To: <20190905112718.ojg3znly6x3m4mjq@yavin.dot.cyphar.com>
On Thu, Sep 05, 2019 at 09:27:18PM +1000, Aleksa Sarai wrote:
> On 2019-09-05, Christian Brauner <christian.brauner@ubuntu.com> wrote:
> > On Thu, Sep 05, 2019 at 06:19:22AM +1000, Aleksa Sarai wrote:
> > > A common pattern for syscall extensions is increasing the size of a
> > > struct passed from userspace, such that the zero-value of the new fields
> > > result in the old kernel behaviour (allowing for a mix of userspace and
> > > kernel vintages to operate on one another in most cases). This is done
> > > in both directions -- hence two helpers -- though it's more common to
> > > have to copy user space structs into kernel space.
> > >
> > > Previously there was no common lib/ function that implemented
> > > the necessary extension-checking semantics (and different syscalls
> > > implemented them slightly differently or incompletely[1]). A future
> > > patch replaces all of the common uses of this pattern to use the new
> > > copy_struct_{to,from}_user() helpers.
> > >
> > > [1]: For instance {sched_setattr,perf_event_open,clone3}(2) all do do
> > > similar checks to copy_struct_from_user() while rt_sigprocmask(2)
> > > always rejects differently-sized struct arguments.
> > >
> > > Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> > > Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
> >
> > I would probably split this out into a separate patchset. It can very
> > well go in before openat2(). Thoughts?
>
> Yeah, I'll split this and the related patches out -- though I will admit
> I'm not sure how you're supposed to deal with multiple independent
> patchsets that depend on each other. How will folks reviewing openat2(2)
> know to include the lib/struct_user.c changes?
The way I usually deal with this is to make two branches. One with the
changes the other depends on and then merge this branch into the other
and put the changes on top. Then you can provide a complete branch that
people can test when you send the patchset out by just linking to it in
the cover letter.
(But if it's too much hazzle just leave it.)
>
> Also, whose tree should it go through?
If people think splitting it out makes sense and we can settle the
technical details I can take it and let it stew in linux-next at least
for a little while.
I have changes to clone3() in there that touch
copy_clone_args_from_user() anyway and there are tests for clone3()
struct copying so we'd catch regressions (for clone3() at least) pretty
quickly.
If we don't see any major issues in the next two weeks it might even be
ok to send for 5.4.
Christian
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 06/11] xen: remove the exports for xen_{create, destroy}_contiguous_region
From: Christoph Hellwig @ 2019-09-05 11:34 UTC (permalink / raw)
To: Stefano Stabellini, Konrad Rzeszutek Wilk, gross, boris.ostrovsky
Cc: xen-devel, iommu, x86, linux-kernel, linux-arm-kernel
In-Reply-To: <20190905113408.3104-1-hch@lst.de>
These routines are only used by swiotlb-xen, which cannot be modular.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
arch/arm/xen/mm.c | 2 --
arch/x86/xen/mmu_pv.c | 2 --
2 files changed, 4 deletions(-)
diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
index 11d5ad26fcfe..9d73fa4a5991 100644
--- a/arch/arm/xen/mm.c
+++ b/arch/arm/xen/mm.c
@@ -154,13 +154,11 @@ int xen_create_contiguous_region(phys_addr_t pstart, unsigned int order,
*dma_handle = pstart;
return 0;
}
-EXPORT_SYMBOL_GPL(xen_create_contiguous_region);
void xen_destroy_contiguous_region(phys_addr_t pstart, unsigned int order)
{
return;
}
-EXPORT_SYMBOL_GPL(xen_destroy_contiguous_region);
int __init xen_mm_init(void)
{
diff --git a/arch/x86/xen/mmu_pv.c b/arch/x86/xen/mmu_pv.c
index 26e8b326966d..c8dbee62ec2a 100644
--- a/arch/x86/xen/mmu_pv.c
+++ b/arch/x86/xen/mmu_pv.c
@@ -2625,7 +2625,6 @@ int xen_create_contiguous_region(phys_addr_t pstart, unsigned int order,
*dma_handle = virt_to_machine(vstart).maddr;
return success ? 0 : -ENOMEM;
}
-EXPORT_SYMBOL_GPL(xen_create_contiguous_region);
void xen_destroy_contiguous_region(phys_addr_t pstart, unsigned int order)
{
@@ -2660,7 +2659,6 @@ void xen_destroy_contiguous_region(phys_addr_t pstart, unsigned int order)
spin_unlock_irqrestore(&xen_reservation_lock, flags);
}
-EXPORT_SYMBOL_GPL(xen_destroy_contiguous_region);
static noinline void xen_flush_tlb_all(void)
{
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 07/11] swiotlb-xen: remove xen_swiotlb_dma_mmap and xen_swiotlb_dma_get_sgtable
From: Christoph Hellwig @ 2019-09-05 11:34 UTC (permalink / raw)
To: Stefano Stabellini, Konrad Rzeszutek Wilk, gross, boris.ostrovsky
Cc: xen-devel, iommu, x86, linux-kernel, linux-arm-kernel
In-Reply-To: <20190905113408.3104-1-hch@lst.de>
There is no need to wrap the common version, just wire them up directly.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
drivers/xen/swiotlb-xen.c | 29 ++---------------------------
1 file changed, 2 insertions(+), 27 deletions(-)
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index eee86cc7046b..b8808677ae1d 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -547,31 +547,6 @@ xen_swiotlb_dma_supported(struct device *hwdev, u64 mask)
return xen_virt_to_bus(xen_io_tlb_end - 1) <= mask;
}
-/*
- * Create userspace mapping for the DMA-coherent memory.
- * This function should be called with the pages from the current domain only,
- * passing pages mapped from other domains would lead to memory corruption.
- */
-static int
-xen_swiotlb_dma_mmap(struct device *dev, struct vm_area_struct *vma,
- void *cpu_addr, dma_addr_t dma_addr, size_t size,
- unsigned long attrs)
-{
- return dma_common_mmap(dev, vma, cpu_addr, dma_addr, size, attrs);
-}
-
-/*
- * This function should be called with the pages from the current domain only,
- * passing pages mapped from other domains would lead to memory corruption.
- */
-static int
-xen_swiotlb_get_sgtable(struct device *dev, struct sg_table *sgt,
- void *cpu_addr, dma_addr_t handle, size_t size,
- unsigned long attrs)
-{
- return dma_common_get_sgtable(dev, sgt, cpu_addr, handle, size, attrs);
-}
-
const struct dma_map_ops xen_swiotlb_dma_ops = {
.alloc = xen_swiotlb_alloc_coherent,
.free = xen_swiotlb_free_coherent,
@@ -584,6 +559,6 @@ const struct dma_map_ops xen_swiotlb_dma_ops = {
.map_page = xen_swiotlb_map_page,
.unmap_page = xen_swiotlb_unmap_page,
.dma_supported = xen_swiotlb_dma_supported,
- .mmap = xen_swiotlb_dma_mmap,
- .get_sgtable = xen_swiotlb_get_sgtable,
+ .mmap = dma_common_mmap,
+ .get_sgtable = dma_common_get_sgtable,
};
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 08/11] swiotlb-xen: use the same foreign page check everywhere
From: Christoph Hellwig @ 2019-09-05 11:34 UTC (permalink / raw)
To: Stefano Stabellini, Konrad Rzeszutek Wilk, gross, boris.ostrovsky
Cc: xen-devel, iommu, x86, linux-kernel, linux-arm-kernel
In-Reply-To: <20190905113408.3104-1-hch@lst.de>
xen_dma_map_page uses a different and more complicated check for foreign
pages than the other three cache maintainance helpers. Switch it to the
simpler pfn_valid method a well, and document the scheme with a single
improved comment in xen_dma_map_page.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
include/xen/arm/page-coherent.h | 31 +++++++++----------------------
1 file changed, 9 insertions(+), 22 deletions(-)
diff --git a/include/xen/arm/page-coherent.h b/include/xen/arm/page-coherent.h
index a840d6949a87..a8d9c0678c27 100644
--- a/include/xen/arm/page-coherent.h
+++ b/include/xen/arm/page-coherent.h
@@ -53,23 +53,17 @@ static inline void xen_dma_map_page(struct device *hwdev, struct page *page,
dma_addr_t dev_addr, unsigned long offset, size_t size,
enum dma_data_direction dir, unsigned long attrs)
{
- unsigned long page_pfn = page_to_xen_pfn(page);
- unsigned long dev_pfn = XEN_PFN_DOWN(dev_addr);
- unsigned long compound_pages =
- (1<<compound_order(page)) * XEN_PFN_PER_PAGE;
- bool local = (page_pfn <= dev_pfn) &&
- (dev_pfn - page_pfn < compound_pages);
+ unsigned long pfn = PFN_DOWN(dev_addr);
/*
- * Dom0 is mapped 1:1, while the Linux page can span across
- * multiple Xen pages, it's not possible for it to contain a
- * mix of local and foreign Xen pages. So if the first xen_pfn
- * == mfn the page is local otherwise it's a foreign page
- * grant-mapped in dom0. If the page is local we can safely
- * call the native dma_ops function, otherwise we call the xen
- * specific function.
+ * Dom0 is mapped 1:1, and while the Linux page can span across multiple
+ * Xen pages, it is not possible for it to contain a mix of local and
+ * foreign Xen pages. Calling pfn_valid on a foreign mfn will always
+ * return false, so if pfn_valid returns true the pages is local and we
+ * can use the native dma-direct functions, otherwise we call the Xen
+ * specific version.
*/
- if (local)
+ if (pfn_valid(pfn))
dma_direct_map_page(hwdev, page, offset, size, dir, attrs);
else
__xen_dma_map_page(hwdev, page, dev_addr, offset, size, dir, attrs);
@@ -79,14 +73,7 @@ static inline void xen_dma_unmap_page(struct device *hwdev, dma_addr_t handle,
size_t size, enum dma_data_direction dir, unsigned long attrs)
{
unsigned long pfn = PFN_DOWN(handle);
- /*
- * Dom0 is mapped 1:1, while the Linux page can be spanned accross
- * multiple Xen page, it's not possible to have a mix of local and
- * foreign Xen page. Dom0 is mapped 1:1, so calling pfn_valid on a
- * foreign mfn will always return false. If the page is local we can
- * safely call the native dma_ops function, otherwise we call the xen
- * specific function.
- */
+
if (pfn_valid(pfn))
dma_direct_unmap_page(hwdev, handle, size, dir, attrs);
else
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 09/11] swiotlb-xen: simplify cache maintainance
From: Christoph Hellwig @ 2019-09-05 11:34 UTC (permalink / raw)
To: Stefano Stabellini, Konrad Rzeszutek Wilk, gross, boris.ostrovsky
Cc: xen-devel, iommu, x86, linux-kernel, linux-arm-kernel
In-Reply-To: <20190905113408.3104-1-hch@lst.de>
Now that we know we always have the dma-noncoherent.h helpers available
if we are on an architecture with support for non-coherent devices,
we can just call them directly, and remove the calls to the dma-direct
routines, including the fact that we call the dma_direct_map_page
routines but ignore the value returned from it. Instead we now have
Xen wrappers for the arch_sync_dma_for_{device,cpu} helpers that call
the special Xen versions of those routines for foreign pages.
Note that the new helpers get the physical address passed in addition
to the dma address to avoid another translation for the local cache
maintainance. The pfn_valid checks remain on the dma address as in
the old code, even if that looks a little funny.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
---
arch/arm/xen/mm.c | 64 +++++++-----------------
arch/x86/include/asm/xen/page-coherent.h | 14 ------
drivers/xen/swiotlb-xen.c | 20 ++++----
include/xen/arm/page-coherent.h | 63 -----------------------
include/xen/swiotlb-xen.h | 5 ++
5 files changed, 32 insertions(+), 134 deletions(-)
diff --git a/arch/arm/xen/mm.c b/arch/arm/xen/mm.c
index 9d73fa4a5991..2b2c208408bb 100644
--- a/arch/arm/xen/mm.c
+++ b/arch/arm/xen/mm.c
@@ -60,63 +60,33 @@ static void dma_cache_maint(dma_addr_t handle, size_t size, u32 op)
} while (size);
}
-static void __xen_dma_page_dev_to_cpu(struct device *hwdev, dma_addr_t handle,
- size_t size, enum dma_data_direction dir)
+/*
+ * Dom0 is mapped 1:1, and while the Linux page can span across multiple Xen
+ * pages, it is not possible for it to contain a mix of local and foreign Xen
+ * pages. Calling pfn_valid on a foreign mfn will always return false, so if
+ * pfn_valid returns true the pages is local and we can use the native
+ * dma-direct functions, otherwise we call the Xen specific version.
+ */
+void xen_dma_sync_for_cpu(struct device *dev, dma_addr_t handle,
+ phys_addr_t paddr, size_t size, enum dma_data_direction dir)
{
- if (dir != DMA_TO_DEVICE)
+ if (pfn_valid(PFN_DOWN(handle)))
+ arch_sync_dma_for_cpu(dev, paddr, size, dir);
+ else if (dir != DMA_TO_DEVICE)
dma_cache_maint(handle, size, GNTTAB_CACHE_INVAL);
}
-static void __xen_dma_page_cpu_to_dev(struct device *hwdev, dma_addr_t handle,
- size_t size, enum dma_data_direction dir)
+void xen_dma_sync_for_device(struct device *dev, dma_addr_t handle,
+ phys_addr_t paddr, size_t size, enum dma_data_direction dir)
{
- if (dir == DMA_FROM_DEVICE)
+ if (pfn_valid(PFN_DOWN(handle)))
+ arch_sync_dma_for_device(dev, paddr, size, dir);
+ else if (dir == DMA_FROM_DEVICE)
dma_cache_maint(handle, size, GNTTAB_CACHE_INVAL);
else
dma_cache_maint(handle, size, GNTTAB_CACHE_CLEAN);
}
-void __xen_dma_map_page(struct device *hwdev, struct page *page,
- dma_addr_t dev_addr, unsigned long offset, size_t size,
- enum dma_data_direction dir, unsigned long attrs)
-{
- if (dev_is_dma_coherent(hwdev))
- return;
- if (attrs & DMA_ATTR_SKIP_CPU_SYNC)
- return;
-
- __xen_dma_page_cpu_to_dev(hwdev, dev_addr, size, dir);
-}
-
-void __xen_dma_unmap_page(struct device *hwdev, dma_addr_t handle,
- size_t size, enum dma_data_direction dir,
- unsigned long attrs)
-
-{
- if (dev_is_dma_coherent(hwdev))
- return;
- if (attrs & DMA_ATTR_SKIP_CPU_SYNC)
- return;
-
- __xen_dma_page_dev_to_cpu(hwdev, handle, size, dir);
-}
-
-void __xen_dma_sync_single_for_cpu(struct device *hwdev,
- dma_addr_t handle, size_t size, enum dma_data_direction dir)
-{
- if (dev_is_dma_coherent(hwdev))
- return;
- __xen_dma_page_dev_to_cpu(hwdev, handle, size, dir);
-}
-
-void __xen_dma_sync_single_for_device(struct device *hwdev,
- dma_addr_t handle, size_t size, enum dma_data_direction dir)
-{
- if (dev_is_dma_coherent(hwdev))
- return;
- __xen_dma_page_cpu_to_dev(hwdev, handle, size, dir);
-}
-
bool xen_arch_need_swiotlb(struct device *dev,
phys_addr_t phys,
dma_addr_t dev_addr)
diff --git a/arch/x86/include/asm/xen/page-coherent.h b/arch/x86/include/asm/xen/page-coherent.h
index 116777e7f387..63cd41b2e17a 100644
--- a/arch/x86/include/asm/xen/page-coherent.h
+++ b/arch/x86/include/asm/xen/page-coherent.h
@@ -21,18 +21,4 @@ static inline void xen_free_coherent_pages(struct device *hwdev, size_t size,
free_pages((unsigned long) cpu_addr, get_order(size));
}
-static inline void xen_dma_map_page(struct device *hwdev, struct page *page,
- dma_addr_t dev_addr, unsigned long offset, size_t size,
- enum dma_data_direction dir, unsigned long attrs) { }
-
-static inline void xen_dma_unmap_page(struct device *hwdev, dma_addr_t handle,
- size_t size, enum dma_data_direction dir,
- unsigned long attrs) { }
-
-static inline void xen_dma_sync_single_for_cpu(struct device *hwdev,
- dma_addr_t handle, size_t size, enum dma_data_direction dir) { }
-
-static inline void xen_dma_sync_single_for_device(struct device *hwdev,
- dma_addr_t handle, size_t size, enum dma_data_direction dir) { }
-
#endif /* _ASM_X86_XEN_PAGE_COHERENT_H */
diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index b8808677ae1d..f81031f0c1c7 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -28,6 +28,7 @@
#include <linux/memblock.h>
#include <linux/dma-direct.h>
+#include <linux/dma-noncoherent.h>
#include <linux/export.h>
#include <xen/swiotlb-xen.h>
#include <xen/page.h>
@@ -391,6 +392,7 @@ static dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
if (map == (phys_addr_t)DMA_MAPPING_ERROR)
return DMA_MAPPING_ERROR;
+ phys = map;
dev_addr = xen_phys_to_bus(map);
/*
@@ -402,14 +404,9 @@ static dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
return DMA_MAPPING_ERROR;
}
- page = pfn_to_page(map >> PAGE_SHIFT);
- offset = map & ~PAGE_MASK;
done:
- /*
- * we are not interested in the dma_addr returned by xen_dma_map_page,
- * only in the potential cache flushes executed by the function.
- */
- xen_dma_map_page(dev, page, dev_addr, offset, size, dir, attrs);
+ if (!dev_is_dma_coherent(dev) && !(attrs & DMA_ATTR_SKIP_CPU_SYNC))
+ xen_dma_sync_for_device(dev, dev_addr, phys, size, dir);
return dev_addr;
}
@@ -429,7 +426,8 @@ static void xen_unmap_single(struct device *hwdev, dma_addr_t dev_addr,
BUG_ON(dir == DMA_NONE);
- xen_dma_unmap_page(hwdev, dev_addr, size, dir, attrs);
+ if (!dev_is_dma_coherent(hwdev) && !(attrs & DMA_ATTR_SKIP_CPU_SYNC))
+ xen_dma_sync_for_cpu(hwdev, dev_addr, paddr, size, dir);
/* NOTE: We use dev_addr here, not paddr! */
if (is_xen_swiotlb_buffer(dev_addr))
@@ -449,7 +447,8 @@ xen_swiotlb_sync_single_for_cpu(struct device *dev, dma_addr_t dma_addr,
{
phys_addr_t paddr = xen_bus_to_phys(dma_addr);
- xen_dma_sync_single_for_cpu(dev, dma_addr, size, dir);
+ if (!dev_is_dma_coherent(dev))
+ xen_dma_sync_for_cpu(dev, dma_addr, paddr, size, dir);
if (is_xen_swiotlb_buffer(dma_addr))
swiotlb_tbl_sync_single(dev, paddr, size, dir, SYNC_FOR_CPU);
@@ -464,7 +463,8 @@ xen_swiotlb_sync_single_for_device(struct device *dev, dma_addr_t dma_addr,
if (is_xen_swiotlb_buffer(dma_addr))
swiotlb_tbl_sync_single(dev, paddr, size, dir, SYNC_FOR_DEVICE);
- xen_dma_sync_single_for_device(dev, dma_addr, size, dir);
+ if (!dev_is_dma_coherent(dev))
+ xen_dma_sync_for_device(dev, dma_addr, paddr, size, dir);
}
/*
diff --git a/include/xen/arm/page-coherent.h b/include/xen/arm/page-coherent.h
index a8d9c0678c27..b9cc11e887ed 100644
--- a/include/xen/arm/page-coherent.h
+++ b/include/xen/arm/page-coherent.h
@@ -5,17 +5,6 @@
#include <linux/dma-mapping.h>
#include <asm/page.h>
-void __xen_dma_map_page(struct device *hwdev, struct page *page,
- dma_addr_t dev_addr, unsigned long offset, size_t size,
- enum dma_data_direction dir, unsigned long attrs);
-void __xen_dma_unmap_page(struct device *hwdev, dma_addr_t handle,
- size_t size, enum dma_data_direction dir,
- unsigned long attrs);
-void __xen_dma_sync_single_for_cpu(struct device *hwdev,
- dma_addr_t handle, size_t size, enum dma_data_direction dir);
-void __xen_dma_sync_single_for_device(struct device *hwdev,
- dma_addr_t handle, size_t size, enum dma_data_direction dir);
-
static inline void *xen_alloc_coherent_pages(struct device *hwdev, size_t size,
dma_addr_t *dma_handle, gfp_t flags, unsigned long attrs)
{
@@ -28,56 +17,4 @@ static inline void xen_free_coherent_pages(struct device *hwdev, size_t size,
dma_direct_free(hwdev, size, cpu_addr, dma_handle, attrs);
}
-static inline void xen_dma_sync_single_for_cpu(struct device *hwdev,
- dma_addr_t handle, size_t size, enum dma_data_direction dir)
-{
- unsigned long pfn = PFN_DOWN(handle);
-
- if (pfn_valid(pfn))
- dma_direct_sync_single_for_cpu(hwdev, handle, size, dir);
- else
- __xen_dma_sync_single_for_cpu(hwdev, handle, size, dir);
-}
-
-static inline void xen_dma_sync_single_for_device(struct device *hwdev,
- dma_addr_t handle, size_t size, enum dma_data_direction dir)
-{
- unsigned long pfn = PFN_DOWN(handle);
- if (pfn_valid(pfn))
- dma_direct_sync_single_for_device(hwdev, handle, size, dir);
- else
- __xen_dma_sync_single_for_device(hwdev, handle, size, dir);
-}
-
-static inline void xen_dma_map_page(struct device *hwdev, struct page *page,
- dma_addr_t dev_addr, unsigned long offset, size_t size,
- enum dma_data_direction dir, unsigned long attrs)
-{
- unsigned long pfn = PFN_DOWN(dev_addr);
-
- /*
- * Dom0 is mapped 1:1, and while the Linux page can span across multiple
- * Xen pages, it is not possible for it to contain a mix of local and
- * foreign Xen pages. Calling pfn_valid on a foreign mfn will always
- * return false, so if pfn_valid returns true the pages is local and we
- * can use the native dma-direct functions, otherwise we call the Xen
- * specific version.
- */
- if (pfn_valid(pfn))
- dma_direct_map_page(hwdev, page, offset, size, dir, attrs);
- else
- __xen_dma_map_page(hwdev, page, dev_addr, offset, size, dir, attrs);
-}
-
-static inline void xen_dma_unmap_page(struct device *hwdev, dma_addr_t handle,
- size_t size, enum dma_data_direction dir, unsigned long attrs)
-{
- unsigned long pfn = PFN_DOWN(handle);
-
- if (pfn_valid(pfn))
- dma_direct_unmap_page(hwdev, handle, size, dir, attrs);
- else
- __xen_dma_unmap_page(hwdev, handle, size, dir, attrs);
-}
-
#endif /* _XEN_ARM_PAGE_COHERENT_H */
diff --git a/include/xen/swiotlb-xen.h b/include/xen/swiotlb-xen.h
index 5e4b83f83dbc..d71380f6ed0b 100644
--- a/include/xen/swiotlb-xen.h
+++ b/include/xen/swiotlb-xen.h
@@ -4,6 +4,11 @@
#include <linux/swiotlb.h>
+void xen_dma_sync_for_cpu(struct device *dev, dma_addr_t handle,
+ phys_addr_t paddr, size_t size, enum dma_data_direction dir);
+void xen_dma_sync_for_device(struct device *dev, dma_addr_t handle,
+ phys_addr_t paddr, size_t size, enum dma_data_direction dir);
+
extern int xen_swiotlb_init(int verbose, bool early);
extern const struct dma_map_ops xen_swiotlb_dma_ops;
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
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