MIME-Version: 1.0
References: <20230226110802.103134-1-usama.arif@bytedance.com>
 <20230226110802.103134-8-usama.arif@bytedance.com> <878rghmrn2.ffs@tglx>
 <35264451eabdf967eb31069cb814e8a05ee3179b.camel@infradead.org>
 <cf7a1657159219cea61ffef567280e2e88d1f670.camel@infradead.org>
 <CAMzpN2hQArxf2mAVq55uMx9VhTjUD-VDEVAD406RScfsrjdAjQ@mail.gmail.com> <0E3AFDDA-039B-40F3-92A4-85CD96B1BB64@infradead.org>
In-Reply-To: <0E3AFDDA-039B-40F3-92A4-85CD96B1BB64@infradead.org>
From:   Brian Gerst <brgerst@gmail.com>
Date:   Tue, 28 Feb 2023 19:03:19 -0500
Message-ID: <CAMzpN2gEhC5q-FPv2iGmEzuk+LiRHF+Mn7EKWw7KbQ5NQUmG2g@mail.gmail.com>
Subject: Re: [PATCH v12 07/11] x86/smpboot: Remove early_gdt_descr on 64-bit
To:     David Woodhouse <dwmw2@infradead.org>
Cc:     Thomas Gleixner <tglx@linutronix.de>,
        Usama Arif <usama.arif@bytedance.com>, kim.phillips@amd.com,
        piotrgorski@cachyos.org, oleksandr@natalenko.name,
        arjan@linux.intel.com, mingo@redhat.com, bp@alien8.de,
        dave.hansen@linux.intel.com, hpa@zytor.com, x86@kernel.org,
        pbonzini@redhat.com, paulmck@kernel.org,
        linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        rcu@vger.kernel.org, mimoja@mimoja.de, hewenliang4@huawei.com,
        thomas.lendacky@amd.com, seanjc@google.com, pmenzel@molgen.mpg.de,
        fam.zheng@bytedance.com, punit.agrawal@bytedance.com,
        simon.evans@bytedance.com, liangma@liangbit.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Tue, Feb 28, 2023 at 6:43=E2=80=AFPM David Woodhouse <dwmw2@infradead.or=
g> wrote:
>
>
>
> On 28 February 2023 22:48:42 GMT, Brian Gerst <brgerst@gmail.com> wrote:
> >On Tue, Feb 28, 2023 at 5:41=E2=80=AFPM David Woodhouse <dwmw2@infradead=
.org> wrote:
> >>
> >> On Tue, 2023-02-28 at 21:57 +0000, David Woodhouse wrote:
> >> >
> >> > ----------------
> >> > IN:
> >> > 0xffffffffa20000b2:  48 31 d2                 xorq     %rdx, %rdx
> >> > 0xffffffffa20000b5:  48 8b 82 c0 74 d5 a3     movq     -0x5c2a8b40(%=
rdx), %rax
> >> > 0xffffffffa20000bc:  48 8b a0 58 14 00 00     movq     0x1458(%rax),=
 %rsp
> >> > 0xffffffffa20000c3:  48 83 ec 10              subq     $0x10, %rsp
> >> > 0xffffffffa20000c7:  66 c7 04 24 7f 00        movw     $0x7f, (%rsp)
> >> > 0xffffffffa20000cd:  48 8d 82 00 10 81 a3     leaq     -0x5c7ef000(%=
rdx), %rax
> >> > 0xffffffffa20000d4:  48 89 44 24 02           movq     %rax, 2(%rsp)
> >> > 0xffffffffa20000d9:  0f 01 14 24              lgdtq    (%rsp)
> >> > 0xffffffffa20000dd:  48 83 c4 10              addq     $0x10, %rsp
> >> > 0xffffffffa20000e1:  31 c0                    xorl     %eax, %eax
> >> > 0xffffffffa20000e3:  8e d8                    movl     %eax, %ds
> >> >
> >> > I cannot work out where the value -0x5c7ef000 comes from, but it
> >> > doesn't seem to be the 0xb000 you claimed, and my brain is hurting
> >> > again...
> >>
> >> Turning off CONFIG_RANDOMIZE_BASE (or just looking at the vmlinux
> >> disassembly instead as Brian did) helps to resolve that FWIW.
> >>
> >> I've changed it to zero all of %rdx and pushed it back to the v12bis
> >> branch.
> >
> >xorl %edx, %edx is preferred, as a 32-bit operation zero-extends to
> >the full 64-bit register.  Using xorq to clear any of the lower 8
> >registers adds an unnecessary REX prefix.  Just one of many quirks of
> >the x86 instruction set...
>
> Ewww. Couldn't the assembler choose to omit the REX prefix then? It does =
more tricksy things than that already.
>
> I almost prefer having the prefix but (in the morning) if you prefer I ca=
n put it back as it was with a comment about the zero-extension.

commit a7bea8308933aaeea76dad7d42a6e51000417626
Author: Jan Beulich <JBeulich@suse.com>
Date:   Mon Jul 2 04:31:54 2018 -0600

    x86/asm/64: Use 32-bit XOR to zero registers

    Some Intel CPUs don't recognize 64-bit XORs as zeroing idioms. Zeroing
    idioms don't require execution bandwidth, as they're being taken care
    of in the frontend (through register renaming). Use 32-bit XORs instead=
.

Not that speed is important here, but it's good to be consistent
across the whole kernel.  Someone will eventually come by and fix it
up anyways, as there have been a few of these patches already in the
git history.

--
Brian Gerst