From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web10.9672.1674023834400465375 for ; Tue, 17 Jan 2023 22:37:16 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=eNf3dh35; spf=pass (domain: redhat.com, ip: 170.10.133.124, mailfrom: osteffen@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1674023832; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=Bp/el6QWN0d+ZyUhhJV4BrRP//qFRXcWiAEwc1u2A0U=; b=eNf3dh35S0oIlvW+g732z/T+TaJAh3Ct4132mgzNunYng04XB3xlrBeosUYlVQKbclcRWR GBSEMReGVXHJkBzH1shUoLP1rYKC4pWyJ08Q5KTcXCuz0LrDnj5XkrKZDK4fYNh9HTjQIl fl8gVWZ564dDbIHxIf+5qnPeYzc+Tko= Received: from mail-lj1-f197.google.com (mail-lj1-f197.google.com [209.85.208.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-264-KMk5-Hv0PZeC21zs8Wt2Zg-1; Wed, 18 Jan 2023 01:37:09 -0500 X-MC-Unique: KMk5-Hv0PZeC21zs8Wt2Zg-1 Received: by mail-lj1-f197.google.com with SMTP id y9-20020a05651c154900b0028571631915so7331443ljp.18 for ; Tue, 17 Jan 2023 22:37:09 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=Bp/el6QWN0d+ZyUhhJV4BrRP//qFRXcWiAEwc1u2A0U=; b=QHsXQ+drseW4s9tR/+rbr49nsd0cUk20+IqL7I8pvEkbdqLYNH4wvdHz5mZTKw0Twc SlBtDhEIelK6PLziBiuiARzsO6ckkElYht5Q872byS5gjjqzzyf1eE8JFkjtP21H1cwX ImYvzHpQvQC1qPVjqo88BZf7B9er2K0x3Z/1Q9FsrRaY9Atm5lc5RwozrmX9cTH2WpY2 WIwy2AFz3aP00olYOhEjF9h+T8vxtrSIOPfKspZt9HMxaJmk98MoTSRi7FtZC0PRSOkG Tnrp2u1Pb2GPCk5a8Du1z0dUQ2SLaFBHc2FMmXx+/jrNTrGx98pSj+y0qndcgm3mPL3P C+cw== X-Gm-Message-State: AFqh2ko6KorjuobvokdZpv6R+yCSWqKcyjIA9xCZUvt8C6WCFXCsIqpl b/a7UEge/pSTG3m0SSVHiJEQJvO7p+WnlHbgs7GxL/apbKWscCyYnY+q2N33VDzMCmOuBK/PCpC V/z6PZRcD2wwnon/wG/68QwD6TGbTAw== X-Received: by 2002:a2e:9316:0:b0:27f:b787:fd31 with SMTP id e22-20020a2e9316000000b0027fb787fd31mr406928ljh.438.1674023827945; Tue, 17 Jan 2023 22:37:07 -0800 (PST) X-Google-Smtp-Source: AMrXdXuPNzzqycL5+tPVTTK+RMB/bSgYImlSvFm8A8CyXc7Fbzg02p3fjAe09CIiV04/cd2AXcIa1o8Woj7Eg5rUpKM= X-Received: by 2002:a2e:9316:0:b0:27f:b787:fd31 with SMTP id e22-20020a2e9316000000b0027fb787fd31mr406924ljh.438.1674023827670; Tue, 17 Jan 2023 22:37:07 -0800 (PST) MIME-Version: 1.0 References: <20230105162528.1430368-1-ardb@kernel.org> <20230105162528.1430368-2-ardb@kernel.org> In-Reply-To: From: "Oliver Steffen" Date: Wed, 18 Jan 2023 07:36:56 +0100 Message-ID: Subject: Re: [edk2-devel] [PATCH v2 2/2] ArmVirtPkg/ArmVirtQemu: Avoid early ID map on ThunderX To: Ard Biesheuvel Cc: devel@edk2.groups.io, dann.frazier@canonical.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: multipart/alternative; boundary="00000000000098266605f2840b0f" --00000000000098266605f2840b0f Content-Type: text/plain; charset="UTF-8" On Tue, Jan 17, 2023 at 3:57 PM Ard Biesheuvel wrote: > On Tue, 17 Jan 2023 at 13:48, Oliver Steffen wrote: > > > > Hi Ard, Hi everyone, > > > > Thanks for the work! > > > > But somehow this patch (as it was merged into master branch) does not > > work for me on the ThunderX box we have. > > > > Any idea what could be wrong? > > I'm not sure I understand the question. The patch targets ThunderX, > and you are using a ThunderX2. > > What were you expecting to happen, and what is happening instead? Firmware does not start at all when using KVM. Please excuse my limited knowledge of Arm processor variants. I assumed that ThunderX and ThunderX2 are very similar and hoped the fix would also work for this case. The issue was introduced by the same commit that Dann reported (07be1d34d95460a238fcd0f6693efb747c28b329): "ArmVirtPkg/ArmVirtQemu: enable initial ID map at early boot". > I enabled the erratum during build ;-) > > > > CPU Info: > > # lscpu > > Architecture: aarch64 > > CPU op-mode(s): 64-bit > > Byte Order: Little Endian > > CPU(s): 224 > > On-line CPU(s) list: 0-223 > > Vendor ID: Cavium > > BIOS Vendor ID: Cavium Inc. > > Model name: ThunderX2 99xx > > BIOS Model name: Cavium ThunderX2(R) CPU CN9975 v2.2 @ 2.0GHz > > Model: 2 > > Thread(s) per core: 4 > > Core(s) per socket: 28 > > Socket(s): 2 > > Stepping: 0x1 > > BogoMIPS: 400.00 > > Flags: fp asimd evtstrm aes pmull sha1 sha2 crc32 > atomics cpuid asimdrdm > > Caches (sum of all): > > L1d: 1.8 MiB (56 instances) > > L1i: 1.8 MiB (56 instances) > > L2: 14 MiB (56 instances) > > L3: 64 MiB (2 instances) > > [...] > > > > Thanks a lot! > > - Oliver > > > > > > On Tue, Jan 10, 2023 at 1:08 AM dann frazier > wrote: > >> > >> On Thu, Jan 05, 2023 at 05:25:28PM +0100, Ard Biesheuvel wrote: > >> > The early ID map used by ArmVirtQemu uses ASID scoped non-global > >> > mappings, as this allows us to switch to the permanent ID map > seamlessly > >> > without the need for explicit TLB maintenance. > >> > > >> > However, this triggers a known erratum on ThunderX, which does not > >> > tolerate non-global mappings that are executable at EL1, as this > appears > >> > to result in I-cache corruption. (Linux disables the KPTI based > Meltdown > >> > mitigation on ThunderX for the same reason) > >> > > >> > So work around this, by detecting the CPU implementor and part number, > >> > and proceeding without the early ID map if a ThunderX CPU is detected. > >> > > >> > Note that this requires the C code to be built with strict alignment > >> > again, as we may end up executing it with the MMU and caches off. > >> > > >> > Signed-off-by: Ard Biesheuvel > >> > --- > >> > ArmVirtPkg/ArmVirtQemu.dsc | > 5 +++++ > >> > ArmVirtPkg/Library/ArmPlatformLibQemu/AArch64/ArmPlatformHelper.S | > 15 +++++++++++++++ > >> > 2 files changed, 20 insertions(+) > >> > >> FTR, this v2 series also worked for me. > >> > >> -dann > >> > >> > diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc > >> > index f77443229e8e..5dd8b6104cca 100644 > >> > --- a/ArmVirtPkg/ArmVirtQemu.dsc > >> > +++ b/ArmVirtPkg/ArmVirtQemu.dsc > >> > @@ -31,6 +31,7 @@ [Defines] > >> > DEFINE SECURE_BOOT_ENABLE = FALSE > >> > DEFINE TPM2_ENABLE = FALSE > >> > DEFINE TPM2_CONFIG_ENABLE = FALSE > >> > + DEFINE CAVIUM_ERRATUM_27456 = FALSE > >> > > >> > # > >> > # Network definition > >> > @@ -117,7 +118,11 @@ [LibraryClasses.common.UEFI_DRIVER] > >> > UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf > >> > > >> > [BuildOptions] > >> > +!if $(CAVIUM_ERRATUM_27456) == TRUE > >> > + GCC:*_*_AARCH64_PP_FLAGS = -DCAVIUM_ERRATUM_27456 > >> > +!else > >> > GCC:*_*_AARCH64_CC_XIPFLAGS == > >> > +!endif > >> > > >> > !include NetworkPkg/NetworkBuildOptions.dsc.inc > >> > > >> > diff --git > a/ArmVirtPkg/Library/ArmPlatformLibQemu/AArch64/ArmPlatformHelper.S > b/ArmVirtPkg/Library/ArmPlatformLibQemu/AArch64/ArmPlatformHelper.S > >> > index 1787d52fbf51..5ac7c732f6ec 100644 > >> > --- > a/ArmVirtPkg/Library/ArmPlatformLibQemu/AArch64/ArmPlatformHelper.S > >> > +++ > b/ArmVirtPkg/Library/ArmPlatformLibQemu/AArch64/ArmPlatformHelper.S > >> > @@ -42,6 +42,21 @@ > >> > > >> > > >> > ASM_FUNC(ArmPlatformPeiBootAction) > >> > +#ifdef CAVIUM_ERRATUM_27456 > >> > + /* > >> > + * On Cavium ThunderX, using non-global mappings that are > executable at EL1 > >> > + * results in I-cache corruption. So just avoid the early ID > mapping there. > >> > + * > >> > + * MIDR implementor 0x43 > >> > + * MIDR part numbers 0xA1 0xA2 (but not 0xAF) > >> > + */ > >> > + mrs x0, midr_el1 // read the MIDR into X0 > >> > + ubfx x1, x0, #24, #8 // grab implementor id > >> > + ubfx x0, x0, #7, #9 // grab part number bits [11:3] > >> > + cmp x1, #0x43 // compare implementor id > >> > + ccmp x0, #0xA0 >> 3, #0, eq // compare part# bits [11:3] > >> > + b.eq 0f > >> > +#endif > >> > mrs x0, CurrentEL // check current exception level > >> > tbnz x0, #3, 0f // omit early ID map if above EL1 > >> > > > --00000000000098266605f2840b0f Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
On Tue, Jan 17, 2023 at 3:57 PM Ard Biesheuvel <ardb@kernel.org> wrote:
On Tue, 17 Jan 2023 at 13:48, Oliver Steff= en <osteffen@re= dhat.com> wrote:
>
> Hi Ard, Hi everyone,
>
> Thanks for the work!
>
> But somehow this patch (as it was merged into master branch) does not<= br> > work for me on the ThunderX box we have.
>
> Any idea what could be wrong?

I'm not sure I understand the question. The patch targets ThunderX,
and you are using a ThunderX2.

What were you expecting to happen, and what is happening instead?
=C2=A0
Firmware does not start at all when using KVM.

Please excuse my limited knowledge of Arm proces= sor variants.
I= assumed that ThunderX and ThunderX2 are very similar and hoped
the fix would also work fo= r this case.
The issue wa= s introduced by the same commit that Dann
reported (07be1d34d95460a238fcd0f6693efb747c= 28b329):
"= ArmVirtPkg/ArmVirtQemu: enable initial ID map at early boot".

> I enabled the erratum during build ;-)
>
> CPU Info:
> # lscpu
> Architecture:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0aarch64
>=C2=A0 =C2=A0CPU op-mode(s):=C2=A0 =C2=A0 =C2=A0 =C2=A064-bit
>=C2=A0 =C2=A0Byte Order:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Little= Endian
> CPU(s):=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A02= 24
>=C2=A0 =C2=A0On-line CPU(s) list:=C2=A0 0-223
> Vendor ID:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 Cavium
>=C2=A0 =C2=A0BIOS Vendor ID:=C2=A0 =C2=A0 =C2=A0 =C2=A0Cavium Inc.
>=C2=A0 =C2=A0Model name:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0Thunde= rX2 99xx
>=C2=A0 =C2=A0 =C2=A0BIOS Model name:=C2=A0 =C2=A0 Cavium ThunderX2(R) C= PU CN9975 v2.2 @ 2.0GHz
>=C2=A0 =C2=A0 =C2=A0Model:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 2
>=C2=A0 =C2=A0 =C2=A0Thread(s) per core: 4
>=C2=A0 =C2=A0 =C2=A0Core(s) per socket: 28
>=C2=A0 =C2=A0 =C2=A0Socket(s):=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 2
>=C2=A0 =C2=A0 =C2=A0Stepping:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A00= x1
>=C2=A0 =C2=A0 =C2=A0BogoMIPS:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A04= 00.00
>=C2=A0 =C2=A0 =C2=A0Flags:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics cpuid asimdrdm
> Caches (sum of all):
>=C2=A0 =C2=A0L1d:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 1.8 MiB (56 instances)
>=C2=A0 =C2=A0L1i:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 1.8 MiB (56 instances)
>=C2=A0 =C2=A0L2:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A014 MiB (56 instances)
>=C2=A0 =C2=A0L3:=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A064 MiB (2 instances)
> [...]
>
> Thanks a lot!
> - Oliver
>
>
> On Tue, Jan 10, 2023 at 1:08 AM dann frazier <dann.frazier@canonical.com&g= t; wrote:
>>
>> On Thu, Jan 05, 2023 at 05:25:28PM +0100, Ard Biesheuvel wrote: >> > The early ID map used by ArmVirtQemu uses ASID scoped non-glo= bal
>> > mappings, as this allows us to switch to the permanent ID map= seamlessly
>> > without the need for explicit TLB maintenance.
>> >
>> > However, this triggers a known erratum on ThunderX, which doe= s not
>> > tolerate non-global mappings that are executable at EL1, as t= his appears
>> > to result in I-cache corruption. (Linux disables the KPTI bas= ed Meltdown
>> > mitigation on ThunderX for the same reason)
>> >
>> > So work around this, by detecting the CPU implementor and par= t number,
>> > and proceeding without the early ID map if a ThunderX CPU is = detected.
>> >
>> > Note that this requires the C code to be built with strict al= ignment
>> > again, as we may end up executing it with the MMU and caches = off.
>> >
>> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>> > ---
>> >=C2=A0 ArmVirtPkg/ArmVirtQemu.dsc=C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 |=C2=A0 5 +++++
>> >=C2=A0 ArmVirtPkg/Library/ArmPlatformLibQemu/AArch64/ArmPlatfo= rmHelper.S | 15 +++++++++++++++
>> >=C2=A0 2 files changed, 20 insertions(+)
>>
>> FTR, this v2 series also worked for me.
>>
>>=C2=A0 =C2=A0-dann
>>
>> > diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQ= emu.dsc
>> > index f77443229e8e..5dd8b6104cca 100644
>> > --- a/ArmVirtPkg/ArmVirtQemu.dsc
>> > +++ b/ArmVirtPkg/ArmVirtQemu.dsc
>> > @@ -31,6 +31,7 @@ [Defines]
>> >=C2=A0 =C2=A0 DEFINE SECURE_BOOT_ENABLE=C2=A0 =C2=A0 =C2=A0 = =3D FALSE
>> >=C2=A0 =C2=A0 DEFINE TPM2_ENABLE=C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0=3D FALSE
>> >=C2=A0 =C2=A0 DEFINE TPM2_CONFIG_ENABLE=C2=A0 =C2=A0 =C2=A0 = =3D FALSE
>> > +=C2=A0 DEFINE CAVIUM_ERRATUM_27456=C2=A0 =C2=A0 =3D FALSE >> >
>> >=C2=A0 =C2=A0 #
>> >=C2=A0 =C2=A0 # Network definition
>> > @@ -117,7 +118,11 @@ [LibraryClasses.common.UEFI_DRIVER]
>> >=C2=A0 =C2=A0 UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiL= ib.inf
>> >
>> >=C2=A0 [BuildOptions]
>> > +!if $(CAVIUM_ERRATUM_27456) =3D=3D TRUE
>> > +=C2=A0 GCC:*_*_AARCH64_PP_FLAGS =3D -DCAVIUM_ERRATUM_27456 >> > +!else
>> >=C2=A0 =C2=A0 GCC:*_*_AARCH64_CC_XIPFLAGS =3D=3D
>> > +!endif
>> >
>> >=C2=A0 !include NetworkPkg/NetworkBuildOptions.dsc.inc
>> >
>> > diff --git a/ArmVirtPkg/Library/ArmPlatformLibQemu/AArch64/Ar= mPlatformHelper.S b/ArmVirtPkg/Library/ArmPlatformLibQemu/AArch64/ArmPlatfo= rmHelper.S
>> > index 1787d52fbf51..5ac7c732f6ec 100644
>> > --- a/ArmVirtPkg/Library/ArmPlatformLibQemu/AArch64/ArmPlatfo= rmHelper.S
>> > +++ b/ArmVirtPkg/Library/ArmPlatformLibQemu/AArch64/ArmPlatfo= rmHelper.S
>> > @@ -42,6 +42,21 @@
>> >
>> >
>> >=C2=A0 ASM_FUNC(ArmPlatformPeiBootAction)
>> > +#ifdef CAVIUM_ERRATUM_27456
>> > +=C2=A0 /*
>> > +=C2=A0 =C2=A0* On Cavium ThunderX, using non-global mappings= that are executable at EL1
>> > +=C2=A0 =C2=A0* results in I-cache corruption. So just avoid = the early ID mapping there.
>> > +=C2=A0 =C2=A0*
>> > +=C2=A0 =C2=A0* MIDR implementor=C2=A0 =C2=A00x43
>> > +=C2=A0 =C2=A0* MIDR part numbers=C2=A0 0xA1 0xA2 (but not 0x= AF)
>> > +=C2=A0 =C2=A0*/
>> > +=C2=A0 mrs=C2=A0 =C2=A0 x0, midr_el1=C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 // read the MIDR into X0
>> > +=C2=A0 ubfx=C2=A0 =C2=A0x1, x0, #24, #8=C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0// grab implementor id
>> > +=C2=A0 ubfx=C2=A0 =C2=A0x0, x0, #7, #9=C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 // grab part number bits [11:3]
>> > +=C2=A0 cmp=C2=A0 =C2=A0 x1, #0x43=C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0// compare implementor id
>> > +=C2=A0 ccmp=C2=A0 =C2=A0x0, #0xA0 >> 3, #0, eq=C2=A0 /= / compare part# bits [11:3]
>> > +=C2=A0 b.eq=C2=A0 =C2=A00f
>> > +#endif
>> >=C2=A0 =C2=A0 mrs=C2=A0 =C2=A0 x0, CurrentEL=C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0// check current exception level
>> >=C2=A0 =C2=A0 tbnz=C2=A0 =C2=A0x0, #3, 0f=C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 // omit early ID map if above EL1
>> >

--00000000000098266605f2840b0f--