linux-kernel - Re: [PATCH 1/3] x86: Use MOVL when reading segment registers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAFULd4Z=ZkOR5qiRxE-5LTyn=FrjtDTbjRRQP2n08kDEbc4_aA@mail.gmail.com>
Date: Fri, 21 Nov 2025 09:09:40 +0100
From: Uros Bizjak <ubizjak@...il.com>
To: Michael Kelley <mhklinux@...look.com>
Cc: "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>, "x86@...nel.org" <x86@...nel.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Thomas Gleixner <tglx@...utronix.de>, 
	Ingo Molnar <mingo@...nel.org>, Borislav Petkov <bp@...en8.de>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, "H. Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH 1/3] x86: Use MOVL when reading segment registers

On Fri, Nov 21, 2025 at 2:03 AM Michael Kelley <mhklinux@...look.com> wrote:
>
> From: Uros Bizjak <ubizjak@...il.com>
> >
> > Use MOVL when reading segment registers to avoid 0x66 operand-size
> > override insn prefix. The segment value is always 16-bit and gets
> > zero-extended to the full 32-bit size.
> >
> > Signed-off-by: Uros Bizjak <ubizjak@...il.com>
> > Cc: Thomas Gleixner <tglx@...utronix.de>
> > Cc: Ingo Molnar <mingo@...nel.org>
> > Cc: Borislav Petkov <bp@...en8.de>
> > Cc: Dave Hansen <dave.hansen@...ux.intel.com>
> > Cc: "H. Peter Anvin" <hpa@...or.com>
> > ---
> >  arch/x86/include/asm/segment.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/include/asm/segment.h b/arch/x86/include/asm/segment.h
> > index f59ae7186940..9f5be2bbd291 100644
> > --- a/arch/x86/include/asm/segment.h
> > +++ b/arch/x86/include/asm/segment.h
> > @@ -348,7 +348,7 @@ static inline void __loadsegment_fs(unsigned short value)
> >   * Save a segment register away:
> >   */
> >  #define savesegment(seg, value)                              \
> > -     asm("mov %%" #seg ",%0":"=r" (value) : : "memory")
> > +     asm("movl %%" #seg ",%k0" : "=r" (value) : : "memory")
> >
> >  #endif /* !__ASSEMBLER__ */
> >  #endif /* __KERNEL__ */
> > --
> > 2.51.1
> >
>
> I've built a linux-next20251119 kernel plus the three patches in this
> series, and tested it in an SEV-SNP VM in the Azure public cloud. The VM
> is a Standard DC16ads v5 (16 vcpus, 64 GiB memory) running Ubuntu
> 20.04. It boots and does basic smoke tests with no issues. So, for all
> three patches,
>
> Tested-by: Michael Kelley <mhklinux@...look.com>
>
> I do have a comment on the commit message for Patch 3 (I would have
> replied there, but for unknown reasons the third patch didn't show up
> in my LKML feed). The commit message says "VMMCALL ... may be inserted
> before the frame pointer". This text was somewhat ambiguous to me.
> I initially read it as "the compiler might insert VMCALL before the
> frame pointer is set up". But I think you meant "it's OK to allow the
> compiler to insert the VMMCALL before the frame point is set up".
> Maybe the intended meaning is obvious to experts in this area,
> but I'm new enough that it was confusing to me. ;-)
>
> To avoid any future confusion, I'd suggest this wording, which is explicit
> about why this patch is appropriate:
>
> Unlike the CALL instruction, VMMCALL does not push to the stack, so
> it's OK to allow the compiler to insert it before the frame pointer gets
> set up by the containing function. ASM_CALL_CONSTRAINT is for CALLs
> that must be after the frame pointer is set up, so it is over-constraining
> here and can be removed.
>
> FWIW, removing the ASM_CALL_CONSTRAINT does not change
> the generated code for hv_snp_hypercall().

Michael,

thanks for your testing and rewording suggestion! After reading it
again a couple of times, I agree that the original text is a bit too
terse, and adding your words indeed clear it up considerably! I have
changed the patch description as you proposed in v2.

BR,
Uros.