lkml.org 
[lkml]   [2019]   [Nov]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH RFC 01/14] x86/asm: add iosubmit_cmds512() based on movdir64b CPU instruction
From
Date


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!

\
 
 \ /
  Last update: 2019-11-21 01:11    [W:0.135 / U:5.708 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site