From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from alexa-out-sd-01.qualcomm.com (alexa-out-sd-01.qualcomm.com [199.106.114.38]) by mx.groups.io with SMTP id smtpd.web08.355.1664385012845530854 for ; Wed, 28 Sep 2022 10:10:12 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@quicinc.com header.s=qcdkim header.b=w1hcOlEQ; spf=permerror, err=parse error for token &{10 18 %{ir}.%{v}.%{d}.spf.has.pphosted.com}: invalid domain name (domain: quicinc.com, ip: 199.106.114.38, mailfrom: quic_llindhol@quicinc.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; i=@quicinc.com; q=dns/txt; s=qcdkim; t=1664385012; x=1695921012; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=p/89KymPO6viwbroe57FiUxmuZWu7MqXOvvN27rUT6s=; b=w1hcOlEQho3X6gup4xa2X+QY+uDnxPPY1VU4Xlb+V4PXElMlQLQsuEQ6 2zG/7WD8q6n0J/N4q9adVxWPeO3/ibeiT97HsDGMG3mN38eJ/7dBqgSmj SFed8tkOniMX4aNpa5ndZqkEn9bgTBfCqqPp28n4Jd57ivzhzkaOO9Jz8 Y=; Received: from unknown (HELO ironmsg-SD-alpha.qualcomm.com) ([10.53.140.30]) by alexa-out-sd-01.qualcomm.com with ESMTP; 28 Sep 2022 10:10:12 -0700 X-QCInternal: smtphost Received: from nasanex01c.na.qualcomm.com ([10.45.79.139]) by ironmsg-SD-alpha.qualcomm.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Sep 2022 10:10:11 -0700 Received: from [10.110.46.134] (10.80.80.8) by nasanex01c.na.qualcomm.com (10.45.79.139) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.986.29; Wed, 28 Sep 2022 10:10:11 -0700 Message-ID: <07de359d-9aff-4c09-629a-ba47e6a23964@quicinc.com> Date: Wed, 28 Sep 2022 10:10:11 -0700 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.13.0 Subject: Re: [PATCH v5 09/21] MdePkg/BaseRngLib: Rename ArmReadIdIsar0() to ArmGetFeatRng() To: , CC: Sami Mujawar , Ard Biesheuvel , Rebecca Cran , Michael D Kinney , Liming Gao , Jiewen Yao , Jian J Wang References: <20220919192207.637786-1-Pierre.Gondois@arm.com> <20220919192207.637786-10-Pierre.Gondois@arm.com> From: "Leif Lindholm" In-Reply-To: <20220919192207.637786-10-Pierre.Gondois@arm.com> Return-Path: quic_llindhol@quicinc.com X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01a.na.qualcomm.com (10.52.223.231) To nasanex01c.na.qualcomm.com (10.45.79.139) Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 7bit On 2022-09-19 12:21, Pierre.Gondois@arm.com wrote: > From: Pierre Gondois > > The MdePkg must be self contained and not have external dependencies. > ArmReadIdIsar0() is defined in MdePkg/Library/BaseRngLib and is > limited to the scope of this library. > > The same function will be required to check the FEAT_AES and FEAT_RNG > extensions in other libraries. As this function is Arm specific, it > cannot be added to a library interface in MdePkg. It should be part of > ArmPkg/ArmLib. Ultimately, ArmPkg has no justification for existing. It is a legacy of ARM support being added as a prototype, bolted onto the side of the rest of EDK2. Over time, it needs to be integrated into MdePkg, UefiCpuPkg, and possibly MdeModulePkg. It makes sense to break the ID_ISAR0 read out into a separate library, but we're not making the world a better place if we then hide random direct-asm-accesses in various libraries. There are similar things in other architectures - i.e. CPUID. But I don't like this patch. I'd prefer to keep this code as-is until the separate library has been added - in MdePkg - and we can switch straight to that. / Leif > To avoid having mutiple definitions/prototypes of ArmReadIdIsar0(), > and as BaseRngLib only requires to check the RNG capability bits, > rename the MdePkg/Library/BaseRngLib implementation to ArmGetFeatRng(). > > Signed-off-by: Pierre Gondois > --- > .../AArch64/{ArmReadIdIsar0.S => ArmGetFeatRng.S} | 8 ++++---- > .../AArch64/{ArmReadIdIsar0.asm => ArmGetFeatRng.asm} | 8 ++++---- > MdePkg/Library/BaseRngLib/AArch64/ArmRng.h | 2 +- > MdePkg/Library/BaseRngLib/AArch64/Rndr.c | 2 +- > MdePkg/Library/BaseRngLib/BaseRngLib.inf | 4 ++-- > 5 files changed, 12 insertions(+), 12 deletions(-) > rename MdePkg/Library/BaseRngLib/AArch64/{ArmReadIdIsar0.S => ArmGetFeatRng.S} (78%) > rename MdePkg/Library/BaseRngLib/AArch64/{ArmReadIdIsar0.asm => ArmGetFeatRng.asm} (81%) > > diff --git a/MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.S b/MdePkg/Library/BaseRngLib/AArch64/ArmGetFeatRng.S > similarity index 78% > rename from MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.S > rename to MdePkg/Library/BaseRngLib/AArch64/ArmGetFeatRng.S > index 82a00d362212..c42d60513077 100644 > --- a/MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.S > +++ b/MdePkg/Library/BaseRngLib/AArch64/ArmGetFeatRng.S > @@ -1,6 +1,6 @@ > #------------------------------------------------------------------------------ > # > -# ArmReadIdIsar0() for AArch64 > +# ArmGetFeatRng() for AArch64 > # > # Copyright (c) 2021, NUVIA Inc. All rights reserved.
> # > @@ -10,7 +10,7 @@ > > .text > .p2align 2 > -GCC_ASM_EXPORT(ArmReadIdIsar0) > +GCC_ASM_EXPORT(ArmGetFeatRng) > > #/** > # Reads the ID_AA64ISAR0 Register. > @@ -20,11 +20,11 @@ GCC_ASM_EXPORT(ArmReadIdIsar0) > #**/ > #UINT64 > #EFIAPI > -#ArmReadIdIsar0 ( > +#ArmGetFeatRng ( > # VOID > # ); > # > -ASM_PFX(ArmReadIdIsar0): > +ASM_PFX(ArmGetFeatRng): > mrs x0, id_aa64isar0_el1 // Read ID_AA64ISAR0 Register > ret > > diff --git a/MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.asm b/MdePkg/Library/BaseRngLib/AArch64/ArmGetFeatRng.asm > similarity index 81% > rename from MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.asm > rename to MdePkg/Library/BaseRngLib/AArch64/ArmGetFeatRng.asm > index 1d9f9a808c0c..947adfcd2749 100644 > --- a/MdePkg/Library/BaseRngLib/AArch64/ArmReadIdIsar0.asm > +++ b/MdePkg/Library/BaseRngLib/AArch64/ArmGetFeatRng.asm > @@ -1,6 +1,6 @@ > ;------------------------------------------------------------------------------ > ; > -; ArmReadIdIsar0() for AArch64 > +; ArmGetFeatRng() for AArch64 > ; > ; Copyright (c) 2021, NUVIA Inc. All rights reserved.
> ; > @@ -8,7 +8,7 @@ > ; > ;------------------------------------------------------------------------------ > > - EXPORT ArmReadIdIsar0 > + EXPORT ArmGetFeatRng > AREA BaseLib_LowLevel, CODE, READONLY > > ;/** > @@ -19,11 +19,11 @@ > ;**/ > ;UINT64 > ;EFIAPI > -;ArmReadIdIsar0 ( > +;ArmGetFeatRng ( > ; VOID > ; ); > ; > -ArmReadIdIsar0 > +ArmGetFeatRng > mrs x0, id_aa64isar0_el1 // Read ID_AA64ISAR0 Register > ret > > diff --git a/MdePkg/Library/BaseRngLib/AArch64/ArmRng.h b/MdePkg/Library/BaseRngLib/AArch64/ArmRng.h > index 2d6ef48ab941..b35cba3c063a 100644 > --- a/MdePkg/Library/BaseRngLib/AArch64/ArmRng.h > +++ b/MdePkg/Library/BaseRngLib/AArch64/ArmRng.h > @@ -35,7 +35,7 @@ ArmRndr ( > **/ > UINT64 > EFIAPI > -ArmReadIdIsar0 ( > +ArmGetFeatRng ( > VOID > ); > > diff --git a/MdePkg/Library/BaseRngLib/AArch64/Rndr.c b/MdePkg/Library/BaseRngLib/AArch64/Rndr.c > index 20811bf3ebf3..0cfdf4c37149 100644 > --- a/MdePkg/Library/BaseRngLib/AArch64/Rndr.c > +++ b/MdePkg/Library/BaseRngLib/AArch64/Rndr.c > @@ -47,7 +47,7 @@ BaseRngLibConstructor ( > // Determine RNDR support by examining bits 63:60 of the ISAR0 register returned by > // MSR. A non-zero value indicates that the processor supports the RNDR instruction. > // > - Isar0 = ArmReadIdIsar0 (); > + Isar0 = ArmGetFeatRng (); > ASSERT ((Isar0 & RNDR_MASK) != 0); > > mRndrSupported = ((Isar0 & RNDR_MASK) != 0); > diff --git a/MdePkg/Library/BaseRngLib/BaseRngLib.inf b/MdePkg/Library/BaseRngLib/BaseRngLib.inf > index 1fcceb941495..d6eccb07d469 100644 > --- a/MdePkg/Library/BaseRngLib/BaseRngLib.inf > +++ b/MdePkg/Library/BaseRngLib/BaseRngLib.inf > @@ -37,10 +37,10 @@ [Sources.AARCH64] > AArch64/Rndr.c > AArch64/ArmRng.h > > - AArch64/ArmReadIdIsar0.S | GCC > + AArch64/ArmGetFeatRng.S | GCC > AArch64/ArmRng.S | GCC > > - AArch64/ArmReadIdIsar0.asm | MSFT > + AArch64/ArmGetFeatRng.asm | MSFT > AArch64/ArmRng.asm | MSFT > > [Packages]