Received-SPF: pass (google.com: domain of linux-kernel+bounces-88710-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Precedence: bulk
MIME-Version: 1.0
References: <20240124103859.611372-1-ubizjak@gmail.com> <170929679278.398.4143931058196373559.tip-bot2@tip-bot2>
 <CAMj1kXGc5_AM2AkE+h8EcvYjAGHqeDWRyVRYUGfBjeubeUmJ6Q@mail.gmail.com>
 <CAFULd4a1sv7LZyT4CMCCsCxi+F9Rm0qAmSV-s=1re_1h71SB7g@mail.gmail.com>
 <CAMj1kXEg=mFe0pJyiLNVV2Am8NJadPR3M-2iAYYj1+jNyaHgBg@mail.gmail.com> <2fadcf62-e77d-49b4-b194-1cfa987d8709@citrix.com>
In-Reply-To: <2fadcf62-e77d-49b4-b194-1cfa987d8709@citrix.com>
From: Ard Biesheuvel <ardb@kernel.org>
Date: Fri, 1 Mar 2024 17:35:44 +0100
Message-ID: <CAMj1kXEAYy0vaA-es7ug9a8=YBj2dX8BMMi-=K3=j03VaYtdPQ@mail.gmail.com>
Subject: Re: [tip: x86/boot] x86/boot: Use 32-bit XOR to clear registers
To: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Uros Bizjak <ubizjak@gmail.com>, linux-kernel@vger.kernel.org, 
	Ingo Molnar <mingo@kernel.org>, Andy Lutomirski <luto@kernel.org>, Brian Gerst <brgerst@gmail.com>, 
	Denys Vlasenko <dvlasenk@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>, 
	Linus Torvalds <torvalds@linux-foundation.org>, Josh Poimboeuf <jpoimboe@redhat.com>, x86@kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Fri, 1 Mar 2024 at 17:02, Andrew Cooper <andrew.cooper3@citrix.com> wrot=
e:
>
> On 01/03/2024 1:10 pm, Ard Biesheuvel wrote:
> > On Fri, 1 Mar 2024 at 13:51, Uros Bizjak <ubizjak@gmail.com> wrote:
> >> On Fri, Mar 1, 2024 at 1:45=E2=80=AFPM Ard Biesheuvel <ardb@kernel.org=
> wrote:
> >>> On Fri, 1 Mar 2024 at 13:39, tip-bot2 for Uros Bizjak
> >>> <tip-bot2@linutronix.de> wrote:
> >>>> The following commit has been merged into the x86/boot branch of tip=
:
> >>>>
> >>>> Commit-ID:     721f791ce1cddfa5f2bf524ac14741bfa0f72697
> >>>> Gitweb:        https://git.kernel.org/tip/721f791ce1cddfa5f2bf524ac1=
4741bfa0f72697
> >>>> Author:        Uros Bizjak <ubizjak@gmail.com>
> >>>> AuthorDate:    Wed, 24 Jan 2024 11:38:59 +01:00
> >>>> Committer:     Ingo Molnar <mingo@kernel.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.
>
> Without trying to get into an argument here...
>

No worries, we're all friends here :-)

> The better reason is that Silvermont Atoms don't recognise the 64bit
> form as a zeroing idiom.  They only recognise the 32bit form of the idiom=
.
>
> Therefore in fastpaths it is (marginally) important to xorl %r15d, %r15d
> rather than xorq %r15, %r15.
>
> But this instance is not a fastpath, and it also doesn't save any
> encoding space, so I'm not sure it was really worth changing.
>

Yeah, that is my point, really. But let's move on. And apologies to
Uros for the tone - it didn't sound as grumpy in my head when I typed
it as it does now reading back the thread.