linux-kernel - Re: [PATCH RFC 01/14] x86/asm: add iosubmit_cmds512() based on movdir64b CPU instruction

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] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 20 Nov 2019 17:10:41 -0700
From:   Dave Jiang <dave.jiang@...el.com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     "dmaengine@...r.kernel.org" <dmaengine@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "vkoul@...nel.org" <vkoul@...nel.org>,
        "Williams, Dan J" <dan.j.williams@...el.com>,
        "Luck, Tony" <tony.luck@...el.com>,
        "Lin, Jing" <jing.lin@...el.com>,
        "Raj, Ashok" <ashok.raj@...el.com>,
        "Kumar, Sanjay K" <sanjay.k.kumar@...el.com>,
        "Dey, Megha" <megha.dey@...el.com>,
        "Pan, Jacob jun" <jacob.jun.pan@...el.com>,
        "Liu, Yi L" <yi.l.liu@...el.com>,
        "axboe@...nel.dk" <axboe@...nel.dk>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "Yu, Fenghua" <fenghua.yu@...el.com>,
        "hpa@...or.com" <hpa@...or.com>
Subject: Re: [PATCH RFC 01/14] x86/asm: add iosubmit_cmds512() based on
 movdir64b CPU instruction



On 11/20/19 2:53 PM, Borislav Petkov wrote:
> On Wed, Nov 20, 2019 at 02:23:49PM -0700, Dave Jiang wrote:
>> +/**
>> + * iosubmit_cmds512 - copy data to single MMIO location, in 512-bit units
> 
> Where is the alignment check on that data before doing the copying?

I'll add the check on the destination address. The call is modeled after 
__iowrite64_copy() / __iowrite32_copy() in lib/iomap_copy.c. Looks like 
those functions do not check for the alignment requirements either.

> 
>> + * @dst: destination, in MMIO space (must be 512-bit aligned)
>> + * @src: source
>> + * @count: number of 512 bits quantities to submit
> 
> Where's that check on the data?

I don't follow?

> 
>> + *
>> + * Submit data from kernel space to MMIO space, in units of 512 bits at a
>> + * time.  Order of access is not guaranteed, nor is a memory barrier
>> + * performed afterwards.
>> + */
>> +static inline void iosubmit_cmds512(void __iomem *dst, const void *src,
>> +				    size_t count)
> 
> An iosubmit function which returns void and doesn't tell its callers
> whether it succeeded or not? That looks non-optimal to say the least.
> 
> Why isn't there a fallback function which to call when the CPU doesn't
> support movdir64b?
> 
> Because then you can use alternative_call() and have the thing work
> regardless of hardware support for MOVDIR*.

Looks like Tony answered this part.

> 
>> +{
>> +	const u8 *from = src;
>> +	const u8 *end = from + count * 64;
>> +
>> +	if (!cpu_has_write512())
> 
> If anything, that thing needs to go and you should use
> 
>    static_cpu_has(X86_FEATURE_MOVDIR64B)
> 
> as it looks to me like you would care about speed on this fast path?
> Yes, no?
> 

Yes thank you!