Re: [PATCH] x86/cpufeatures: Add enumeration for serialize instruction

[Ricardo Neri]

On Sat, Apr 04, 2020 at 02:15:57PM -0700, Andy Lutomirski wrote:
> On Fri, Apr 3, 2020 at 10:19 PM Ricardo Neri
> <ricardo.neri-calderon@xxxxxxxxxxxxxxx> wrote:
> >
> > On Fri, Apr 03, 2020 at 10:12:17AM +0200, Borislav Petkov wrote:
> > > On Thu, Apr 02, 2020 at 06:40:26PM -0700, Ricardo Neri wrote:
> > > > The serialize instruction ensures that before the next instruction is
> > > > fetched and executed, all the modifications to flags, registers, and memory
> > > > made by previous instructions are completed, draining all buffered writes
> > > > to memory.
> > > >
> > > > Importantly, the serialize instruction does not modify registers,
> > > > arithmetic flags or memory.
> > > >
> > > > Hence, the serialize instructions provides a better way for software
> > > > to serialize execution than using instructions such as cpuid, which does
> > > > modify registers and, in virtual machines, causes a VM exit.
> > > >
> > > > This instruction is supported by the CPU if CPUID.7H.EDX[bit 14] is
> > > > set.
> > > >
> > > > Cc: x86@xxxxxxxxxx
> > > > Cc: "Ravi V. Shankar" <ravi.v.shankar@xxxxxxxxx>
> > > > Signed-off-by: Ricardo Neri <ricardo.neri-calderon@xxxxxxxxxxxxxxx>
> > > > ---
> > > > This new instruction is documented in the latest version of the Intel
> > > > Architecture Instruction Set Extensions and Future Features Programming
> > > > Reference Chapter 2.1 located at
> > > > https://software.intel.com/sites/default/files/managed/c5/15/architecture-instruction-set-extensions-programming-reference.pdf
> > > > ---
> > > >  arch/x86/include/asm/cpufeatures.h | 1 +
> > > >  1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> > > > index db189945e9b0..cd9b1ec022ec 100644
> > > > --- a/arch/x86/include/asm/cpufeatures.h
> > > > +++ b/arch/x86/include/asm/cpufeatures.h
> > > > @@ -364,6 +364,7 @@
> > > >  #define X86_FEATURE_AVX512_VP2INTERSECT (18*32+ 8) /* AVX-512 Intersect for D/Q */
> > > >  #define X86_FEATURE_MD_CLEAR               (18*32+10) /* VERW clears CPU buffers */
> > > >  #define X86_FEATURE_TSX_FORCE_ABORT        (18*32+13) /* "" TSX_FORCE_ABORT */
> > > > +#define X86_FEATURE_SERIALIZE              (18*32+14) /* SERIALIZE instruction */
> > > >  #define X86_FEATURE_PCONFIG                (18*32+18) /* Intel PCONFIG */
> > > >  #define X86_FEATURE_SPEC_CTRL              (18*32+26) /* "" Speculation Control (IBRS + IBPB) */
> > > >  #define X86_FEATURE_INTEL_STIBP            (18*32+27) /* "" Single Thread Indirect Branch Predictors */
> > > > --
> > >
> > > Send this together with code which is using it, pls.
> >
> > Do you mean code in the kernel using this instructions. Thus far, I
> > don't have any kernel use cases for this instruction. My intention is to expose
> > this instruction to user space via /proc/cpuinfo. Is that not
> > acceptable?
> 
> Presumably sync_core() should do, roughly:
> 
> if (static_cpu_has(X86_FEATURE_SERIALIZE)) {
>   asm volatile("serialize");
>   return;
> }

Sure Andy, I will look at implementing something as you propose.

> 
> but with the appropriate magic to build it on older binutils.  

But old binutils will not be aware of this new instruction, right? How
could they be impacted?

> should make sure that the in-kernel instruction decoder doesn't
> explode when it sees serialize, presumably.

Sure Andy. I will also test for this.

Thanks and BR,
Ricardo