Messages in this thread | | | Subject | Re: [PATCH v4 2/5] x86/asm: add enqcmds() to support ENQCMDS instruction | From | Dave Jiang <> | Date | Wed, 23 Sep 2020 08:47:31 -0700 |
| |
On 9/23/2020 4:08 AM, Borislav Petkov wrote: > On Thu, Sep 17, 2020 at 02:15:23PM -0700, Dave Jiang wrote: >> Add enqcmds() in x86 io.h instead of special_insns.h. > > Why? It is an asm wrapper for a special instruction.
Ok will move.
> >> MOVDIR64B >> instruction can be used for other purposes. A wrapper was introduced >> in io.h for its command submission usage. ENQCMDS has a single >> purpose of submit 64-byte commands to supported devices and should >> be called directly. >> >> Signed-off-by: Dave Jiang <dave.jiang@intel.com> >> Reviewed-by: Tony Luck <tony.luck@intel.com> >> --- >> arch/x86/include/asm/io.h | 29 +++++++++++++++++++++++++++++ >> 1 file changed, 29 insertions(+) >> >> diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h >> index d726459d08e5..b7af0bf8a018 100644 >> --- a/arch/x86/include/asm/io.h >> +++ b/arch/x86/include/asm/io.h >> @@ -424,4 +424,33 @@ static inline void iosubmit_cmds512(void __iomem *dst, const void *src, >> } >> } >> >> +/** >> + * enqcmds - copy a 512 bits data unit to single MMIO location > > Your #319433 doc says > > "ENQCMDS — Enqueue Command Supervisor" > > Now *how* that enqueueing is done you can explain in the comment below.
Ok will add.
> >> + * @dst: destination, in MMIO space (must be 512-bit aligned) >> + * @src: source >> + * >> + * Submit data from kernel space to MMIO space, in a unit of 512 bits. >> + * Order of data access is not guaranteed, nor is a memory barrier >> + * performed afterwards. The command returns false (0) on failure, and true (1) >> + * on success. > > The command or the function?
Function. Will fix. > > From what I see below, the instruction sets ZF=1 to denote that it needs > to be retried and ZF=0 means success, as the doc says. And in good UNIX > tradition, 0 means usually success and !0 failure. > > So why are you flipping that?
Ok will return 0 for success and -ERETRY for failure.
> >> + * Warning: Do not use this helper unless your driver has checked that the CPU >> + * instruction is supported on the platform. >> + */ >> +static inline bool enqcmds(void __iomem *dst, const void *src) >> +{ >> + bool retry; >> + >> + /* ENQCMDS [rdx], rax */ >> + asm volatile(".byte 0xf3, 0x0f, 0x38, 0xf8, 0x02, 0x66, 0x90\t\n" > ^^^^ > No need for those last two chars.
Ok will remove.
> >> + CC_SET(z) >> + : CC_OUT(z) (retry) >> + : "a" (dst), "d" (src)); > > <---- newline here.
Will fix.
> >> + /* Submission failure is indicated via EFLAGS.ZF=1 */ >> + if (retry) >> + return false; >> + >> + return true; >> +} >> + >> #endif /* _ASM_X86_IO_H */ > > Thx. >
Thank you very much for reviewing Boris. Very much appreciated!
| |