On Tue, 2019-03-12 at 18:50 -0400, Andrea Arcangeli wrote:
> On Tue, Mar 12, 2019 at 03:02:54PM -0700, James Bottomley wrote:
> > I'm sure there must be workarounds elsewhere in the other arch code
> > otherwise things like this, which appear all over drivers/,
> > wouldn't
> > work:
> >
> > drivers/scsi/isci/request.c:1430
> >
> > kaddr = kmap_atomic(page);
> > memcpy(kaddr + sg->offset, src_addr, copy_len);
> > kunmap_atomic(kaddr);
> >
>
> Are you sure "page" is an userland page with an alias address?
>
> sg->page_link = (unsigned long)virt_to_page(addr);
Yes, it's an element of a scatter gather list, which may be either a
kernel page or a user page, but is usually the latter.
> page_link seems to point to kernel memory.
>
> I found an apparent solution like parisc on arm 32bit:
>
> void __kunmap_atomic(void *kvaddr)
> {
> unsigned long vaddr = (unsigned long) kvaddr & PAGE_MASK;
> int idx, type;
>
> if (kvaddr >= (void *)FIXADDR_START) {
> type = kmap_atomic_idx();
> idx = FIX_KMAP_BEGIN + type + KM_TYPE_NR *
> smp_processor_id();
>
> if (cache_is_vivt())
> __cpuc_flush_dcache_area((void *)vaddr,
> PAGE_SIZE);
>
> However on arm 64bit kunmap_atomic is not implemented at all and
> other 32bit implementations don't do it, for example sparc seems to
> do the cache flush too if the kernel is built with
> CONFIG_DEBUG_HIGHMEM (which makes the flushing conditional to the
> debug option).
>
> The kunmap_atomic where fixmap is used, is flushing the tlb lazily so
> even on 32bit you can't even be sure if there was a tlb flush for
> each single page you unmapped, so it's hard to see how the above can
> work safe, is "page" would have been an userland page mapped with
> aliased CPU cache.
>
> > the sequence dirties the kernel virtual address but doesn't flush
> > before doing kunmap. There are hundreds of other examples which is
> > why I think adding flush_kernel_dcache_page() is an already lost
> > cause.
>
> In lots of cases kmap is needed to just modify kernel memory not to
> modify userland memory (where get/put_user is more commonly used
> instead..), there's no cache aliasing in such case.
That's why I picked drivers/ The use case in there is mostly kmap to
put a special value into a scatter gather list entry.
> > Actually copy_user_page() is unused in the main kernel. The big
> > problem is copy_user_highpage() but that's mostly highly optimised
> > by the VIPT architectures (in other words you can fiddle with kmap
> > without impacting it).
>
> copy_user_page is not unused, it's called precisely by
> copy_user_highpage, which is why the cache flushes are done inside
> copy_user_page.
>
> static inline void copy_user_highpage(struct page *to, struct page
> *from,
> unsigned long vaddr, struct vm_area_struct *vma)
> {
> char *vfrom, *vto;
>
> vfrom = kmap_atomic(from);
> vto = kmap_atomic(to);
> copy_user_page(vto, vfrom, vaddr, to);
> kunmap_atomic(vto);
> kunmap_atomic(vfrom);
> }
That's the asm/generic implementation. Most VIPT architectures
override it.
James
