[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMj1kXEg=mFe0pJyiLNVV2Am8NJadPR3M-2iAYYj1+jNyaHgBg@mail.gmail.com>
Date: Fri, 1 Mar 2024 14:10:28 +0100
From: Ard Biesheuvel <ardb@...nel.org>
To: Uros Bizjak <ubizjak@...il.com>
Cc: linux-kernel@...r.kernel.org, linux-tip-commits@...r.kernel.org,
Ingo Molnar <mingo@...nel.org>, Andy Lutomirski <luto@...nel.org>, Brian Gerst <brgerst@...il.com>,
Denys Vlasenko <dvlasenk@...hat.com>, "H. Peter Anvin" <hpa@...or.com>,
Linus Torvalds <torvalds@...ux-foundation.org>, Josh Poimboeuf <jpoimboe@...hat.com>, x86@...nel.org
Subject: Re: [tip: x86/boot] x86/boot: Use 32-bit XOR to clear registers
On Fri, 1 Mar 2024 at 13:51, Uros Bizjak <ubizjak@...il.com> wrote:
>
> On Fri, Mar 1, 2024 at 1:45 PM Ard Biesheuvel <ardb@...nel.org> wrote:
> >
> > On Fri, 1 Mar 2024 at 13:39, tip-bot2 for Uros Bizjak
> > <tip-bot2@...utronix.de> wrote:
> > >
> > > The following commit has been merged into the x86/boot branch of tip:
> > >
> > > Commit-ID: 721f791ce1cddfa5f2bf524ac14741bfa0f72697
> > > Gitweb: https://git.kernel.org/tip/721f791ce1cddfa5f2bf524ac14741bfa0f72697
> > > Author: Uros Bizjak <ubizjak@...il.com>
> > > AuthorDate: Wed, 24 Jan 2024 11:38:59 +01:00
> > > Committer: Ingo Molnar <mingo@...nel.org>
> > > CommitterDate: Fri, 01 Mar 2024 12:47:37 +01:00
> > >
> > > x86/boot: Use 32-bit XOR to clear registers
> > >
> > > x86_64 zero extends 32-bit operations, so for 64-bit operands,
> > > XORL r32,r32 is functionally equal to XORQ r64,r64, but avoids
> > > a REX prefix byte when legacy registers are used.
> > >
> >
> > ... and so this change is pointless churn when not using legacy
> > registers, right?
>
> Although there is no code size change with REX registers, it would
> look weird to use XORQ with REX registers and XORL with legacy regs.
You are changing an isolated occurrence of XORQ into XORL on the basis
that XORQ 'looks weird', and would produce a longer opcode if the
occurrence in question would be using a different register than it
actually uses.
Apologies for the bluntness, but in my book, this really falls firmly
into the 'pointless churn' territory. The startup code is not
performance critical, neither in terms of size nor in speed, and so
I'd prefer to avoid these kinds of changes. Just my 2c, though - Ingo
has already merged the patch.
Powered by blists - more mailing lists