Re: [PATCH 6/6] arm64: uaccess: Implement *_flushcache variants

[Catalin Marinas]

On Thu, Aug 10, 2017 at 12:58:45PM +0200, Arnd Bergmann wrote:
> On Tue, Jul 25, 2017 at 12:55 PM, Robin Murphy  wrote:
> > Implement the set of copy functions with guarantees of a clean cache
> > upon completion necessary to support the pmem driver.
> >
> > Signed-off-by: Robin Murphy 
> > ---
> >  arch/arm64/Kconfig  |  1 +
> >  arch/arm64/include/asm/string.h |  4 
> >  arch/arm64/include/asm/uaccess.h| 12 ++
> >  arch/arm64/lib/Makefile |  2 ++
> >  arch/arm64/lib/uaccess_flushcache.c | 47 
> > +
> >  5 files changed, 66 insertions(+)
> >  create mode 100644 arch/arm64/lib/uaccess_flushcache.c
> 
> It looks like Catalin applied part of this patch but forgot to add
> arch/arm64/lib/uaccess_flushcache.c

Added it now, thanks.

-- 
Catalin
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH 6/6] arm64: uaccess: Implement *_flushcache variants

[Arnd Bergmann]

On Tue, Jul 25, 2017 at 12:55 PM, Robin Murphy  wrote:
> Implement the set of copy functions with guarantees of a clean cache
> upon completion necessary to support the pmem driver.
>
> Signed-off-by: Robin Murphy 
> ---
>  arch/arm64/Kconfig  |  1 +
>  arch/arm64/include/asm/string.h |  4 
>  arch/arm64/include/asm/uaccess.h| 12 ++
>  arch/arm64/lib/Makefile |  2 ++
>  arch/arm64/lib/uaccess_flushcache.c | 47 
> +
>  5 files changed, 66 insertions(+)
>  create mode 100644 arch/arm64/lib/uaccess_flushcache.c

It looks like Catalin applied part of this patch but forgot to add
arch/arm64/lib/uaccess_flushcache.c

  Arnd
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm

Re: [PATCH 6/6] arm64: uaccess: Implement *_flushcache variants

[Will Deacon]

On Tue, Jul 25, 2017 at 11:55:43AM +0100, Robin Murphy wrote:
> Implement the set of copy functions with guarantees of a clean cache
> upon completion necessary to support the pmem driver.
> 
> Signed-off-by: Robin Murphy 
> ---
>  arch/arm64/Kconfig  |  1 +
>  arch/arm64/include/asm/string.h |  4 
>  arch/arm64/include/asm/uaccess.h| 12 ++
>  arch/arm64/lib/Makefile |  2 ++
>  arch/arm64/lib/uaccess_flushcache.c | 47 
> +
>  5 files changed, 66 insertions(+)
>  create mode 100644 arch/arm64/lib/uaccess_flushcache.c

[...]

> diff --git a/arch/arm64/lib/uaccess_flushcache.c 
> b/arch/arm64/lib/uaccess_flushcache.c
> new file mode 100644
> index ..b6ceafdb8b72
> --- /dev/null
> +++ b/arch/arm64/lib/uaccess_flushcache.c
> @@ -0,0 +1,47 @@
> +/*
> + * Copyright (C) 2017 ARM Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see .
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +void memcpy_flushcache(void *dst, const void *src, size_t cnt)
> +{
> + /*
> +  * We assume this should not be called with @dst pointing to
> +  * non-cacheable memory, such that we don't need an explicit
> +  * barrier to order the cache maintenance against the memcpy.
> +  */
> + memcpy(dst, src, cnt);
> + __clean_dcache_area_pop(dst, cnt);
> +}
> +EXPORT_SYMBOL_GPL(memcpy_flushcache);
> +
> +void memcpy_page_flushcache(char *to, struct page *page, size_t offset,
> + size_t len)
> +{
> + memcpy_flushcache(to, page_address(page) + offset, len);
> +}
> +
> +unsigned long __copy_user_flushcache(void *to, const void __user *from,
> +  unsigned long n)
> +{
> + unsigned long rc = __arch_copy_from_user(to, from, n);

I'm a bit nervous calling the bare user accessor here without an access_ok
check beforehand. Can we rely on the caller having done the check for us? I
tried to follow the breadcrumbs back out, but I noticed that other iov
iterators (such as copy_from_iter) *do* do the bounds check, whereas the
pmem version (copy_from_iter_nocache) doesn't appear to check the address.

Is that right?

Will
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm