From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-ot1-f45.google.com (mail-ot1-f45.google.com [209.85.210.45]) by mx.groups.io with SMTP id smtpd.web12.2087.1620337627000817963 for ; Thu, 06 May 2021 14:47:07 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20150623.gappssmtp.com header.s=20150623 header.b=C/EQ2L9s; spf=pass (domain: nuviainc.com, ip: 209.85.210.45, mailfrom: rebecca@nuviainc.com) Received: by mail-ot1-f45.google.com with SMTP id g15-20020a9d128f0000b02902a7d7a7bb6eso6211733otg.9 for ; Thu, 06 May 2021 14:47:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=/edyytXzqQdILDq2PYUgIjAWn3QjZ8J7ilEtkGRM5Dg=; b=C/EQ2L9sukOJDM1gA+tpEf2EHc9TULnLdm3nqFHQmGg53JcpkTQmhlS79whTShg6oi X+vqFNuvnBoTSwgAB4KXMFVzX6CijpfOzT4x5xLzK0/OR7AxzgcCWu3iQ589c2gNjZUB FqpEwyb1ao9RmEzpk5RW+hkJ7nAqFU73dFcKDBXrI0FPKEaA0HUF7NzYpdsp4jkbPOI+ 2ZNxZ11MMbGRc89/aEqrYeOfLIee6lJchBlyEKSvvZCnck5cvk8Lv/ZZrQjvT4GnFYnK pGrpbDAqRiXBKKnX9WR5QfAkMWW1vymwMyX3t8zRhxSfRai+GVaXdKakn6ZgWJdO8Pzo KoKg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=/edyytXzqQdILDq2PYUgIjAWn3QjZ8J7ilEtkGRM5Dg=; b=Hm3ON+VzJcwxScvjhOKlsrFRkkxekxclXPCi72Hn+nmAvXL4i9EZXjClzt9yHsZXC/ y2J05A4/kQYrmy2CCZyth7IYUD9hwvK+lxri/iqQYklMkPw9IgfmGTwdkU4IxaE3ZomT WdilGttXRcmFijORwMpZEUUQFIuEBHXwmgGD59Bagv2e46ALpDaYWtXpiyYeQzfIMqrL HKISybcTRf1iS+jFcNAzMNq7t1iVzZmZiXaO/RKNbk8jVJfF4QpOWYIjbZgp8sgVeXx8 qL3S5QImDpN3hDeAELOe2XKtz3EmTRwx6VqLXzXqWm8iAnhsc8EDvpgw5lX1IHUw0kk0 tlRQ== X-Gm-Message-State: AOAM531jOdxUaScSWL97CaGidDOyr7YBgkZF41hcp9aDrG1gHHdb88S7 cOnyUmoUr6dnRvj0ssYPws/C6Q== X-Google-Smtp-Source: ABdhPJx2g6SqWKYcwq8045b2ex5aOZ4R674hozGqaGFl0zIdfK3uRVBay48CcEpC+zE9i4OYveXGoA== X-Received: by 2002:a9d:4911:: with SMTP id e17mr5521756otf.38.1620337626316; Thu, 06 May 2021 14:47:06 -0700 (PDT) Return-Path: Received: from [10.0.10.142] (c-174-52-16-57.hsd1.ut.comcast.net. [174.52.16.57]) by smtp.gmail.com with ESMTPSA id c95sm776932otb.80.2021.05.06.14.47.05 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 06 May 2021 14:47:05 -0700 (PDT) Subject: Re: [edk2-devel] [PATCH 2/3] MdePkg: Refactor BaseRngLib to support AARCH64 in addition to X86 To: devel@edk2.groups.io, sami.mujawar@arm.com Cc: Jiewen Yao , Jian J Wang , Michael D Kinney , Liming Gao , Zhiguang Liu , Ard Biesheuvel , nd@arm.com References: <20210428204415.25454-1-rebecca@nuviainc.com> <20210428204415.25454-3-rebecca@nuviainc.com> From: "Rebecca Cran" Message-ID: <17a133b7-c3e0-773d-4aa8-78ce05050d33@nuviainc.com> Date: Thu, 6 May 2021 15:47:04 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit On 5/4/21 3:09 PM, Sami Mujawar wrote: >> +EFIAPI >> +BaseRngLibConstructor ( >> + VOID >> + ) >> +{ >> + UINT64 Isar0; >> + // >> + // 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 (); >> + ASSERT ((Isar0 & RNDR_MASK) != 0); >> + (void)Isar0; > [SAMI] ASSERTs will vanish in the release builds. So, I think this needs > to be an if condition. If RNDR is not supported RETURN_UNSUPPORTED > should be returned. > However, it appears thatthe auto generated function > ProcessLibraryConstructorList() disregards the error code returned by > the constructor (see Build\...\AutoGen.c files). So it looks like the > loading operation would continue in release builds despite of an error. > I am not aware if this is the desired behavior or why the status code > returned by the constructor is disregarded. > > However, this would be a probem in the current case as subsequent calls > to generate random numbers will result in an undefined instruction > exception. > To prevent this, I think the above check should be done in either >    - ArmRndr()/ArmRndrrs() >   or >    - preferably in ArchGetRandomNumberXX(), which should return an > error code EFI_UNSUPPORTED, EFI_NOT_READY or EFI_SUCCESS. However, the > impact on IA32/x64 code needs to be evaluated. I've updated both the x86 and aarch64 code to add checks that the RDRAND and RNDR instructions are supported before trying to use them in GetRandomNumber[16,32,64,128]. However, it causes the X64 CryptoPkg host-based unit tests to fail because UnitTestHostBaseLibAsmCpuid just sets all the OUT parameters to 0, which causes RngDxe to think RDRAND isn't supported. Should the unit tests be using BaseRngLibTimerLib instead of BaseRngLib? Or should I leave the x86 code as it was, with the ASSERT in the constructor and no further checks at the time of use? -- Rebecca Cran