Messages in this thread | | | Subject | Re: [PATCH RFC 01/14] x86/asm: add iosubmit_cmds512() based on movdir64b CPU instruction | From | Dave Jiang <> | Date | Wed, 20 Nov 2019 17:10:41 -0700 |
| |
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!
| |