linux-kernel - Re: [PATCH 08/13] x86, libnvdimm, dax: stop abusing __copy_user

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAPcyv4h963B=NuKga+ZuE51=xWFbgAnvGWP2f0KhukcmF6orbQ@mail.gmail.com>
Date:   Tue, 28 Mar 2017 09:26:46 -0700
From:   Dan Williams <dan.j.williams@...el.com>
To:     Ross Zwisler <ross.zwisler@...ux.intel.com>,
        Dan Williams <dan.j.williams@...el.com>,
        "linux-nvdimm@...ts.01.org" <linux-nvdimm@...ts.01.org>,
        Tony Luck <tony.luck@...el.com>, Jan Kara <jack@...e.cz>,
        Toshi Kani <toshi.kani@....com>,
        Matthew Wilcox <mawilcox@...rosoft.com>,
        X86 ML <x86@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Christoph Hellwig <hch@....de>,
        Brian Boylston <brian.boylston@....com>,
        Jeff Moyer <jmoyer@...hat.com>, Ingo Molnar <mingo@...hat.com>,
        Al Viro <viro@...iv.linux.org.uk>,
        "H. Peter Anvin" <hpa@...or.com>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH 08/13] x86, libnvdimm, dax: stop abusing __copy_user_nocache

On Tue, Mar 28, 2017 at 9:21 AM, Ross Zwisler
<ross.zwisler@...ux.intel.com> wrote:
> On Thu, Jan 19, 2017 at 07:50:56PM -0800, Dan Williams wrote:
> <>
>> diff --git a/drivers/nvdimm/x86-asm.S b/drivers/nvdimm/x86-asm.S
>> new file mode 100644
>> index 000000000000..23c5ec94e896
>> --- /dev/null
>> +++ b/drivers/nvdimm/x86-asm.S
>> @@ -0,0 +1,71 @@
>> +/*
>> + * Copyright (c) 2017, Intel Corporation.
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms and conditions of the GNU General Public License,
>> + * version 2, as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope 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.
>> + */
>> +#include <linux/linkage.h>
>> +
>> +/*
>> + * __arch_memcpy_to_pmem - non-temporal + unordered memory copy
>> + *
>> + * 8-byte alignment for destination, source, and len. The results of
>> + * this transfer are not persistent or globally visible until a
>> + * sub-sequent sfence (REQ_FLUSH) to the pmem driver.
>> + *
>> + * Derived from __copy_user_nocache.
>> + */
>> +ENTRY(__arch_memcpy_to_pmem)
>> +     /* Set 4x8-byte copy count and remainder */
>> +     movl %edx,%ecx
>> +     andl $63,%edx
>> +     shrl $6,%ecx
>> +     jz .L_8b_pmem_copy_entry        /* jump if count is 0 */
>> +
>> +     /* Perform 4x8-byte pmem loop-copy */
>> +.L_4x8b_pmem_copy_loop:
>> +     movq (%rsi),%r8
>> +     movq 1*8(%rsi),%r9
>> +     movq 2*8(%rsi),%r10
>> +     movq 3*8(%rsi),%r11
>> +     movnti %r8,(%rdi)
>> +     movnti %r9,1*8(%rdi)
>> +     movnti %r10,2*8(%rdi)
>> +     movnti %r11,3*8(%rdi)
>> +     movq 4*8(%rsi),%r8
>> +     movq 5*8(%rsi),%r9
>> +     movq 6*8(%rsi),%r10
>> +     movq 7*8(%rsi),%r11
>> +     movnti %r8,4*8(%rdi)
>> +     movnti %r9,5*8(%rdi)
>> +     movnti %r10,6*8(%rdi)
>> +     movnti %r11,7*8(%rdi)
>> +     leaq 64(%rsi),%rsi
>> +     leaq 64(%rdi),%rdi
>> +     decl %ecx
>> +     jnz .L_4x8b_pmem_copy_loop
>> +
>> +     /* Set 8-byte copy count and remainder */
>> +.L_8b_pmem_copy_entry:
>> +     movl %edx,%ecx
>> +     andl $7,%edx
>
> I don't think you need to andl %edx here - in __copy_user_nocache() %edx was
> used to keep the remaining count that couldn't be handled with the size of
> transfers we were doing in a given loop, so in .L_8b_nocache_copy_entry we
> mask with 7 so we can use the remaining count in .L_4b_nocache_copy_entry
> and/or .L_1b_cache_copy_entry.
>
> In the PMEM case, though, the 8 byte loop is the end of the line, so we just
> ignore any trailing data that isn't 8 byte aligned.
>
> I'm not sure if it's important to use %ecx as your local loop variable - is
> this a widely held convention?  If not, you could just leave %ecx out of it
> and use %edx directly in the 8 byte copy, i.e.:

My current plan is to ditch this assembly and do an inline asm
implementation in C, because the assembly is really only needed for
the exception handling case.