Re: [PATCH v4 2/5] x86/asm: add enqcmds() to support ENQCMDS instruction

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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@xxxxxxxxx>
Reviewed-by: Tony Luck <tony.luck@xxxxxxxxx>
---
  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!



[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux PCI]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux