From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 2AEDD1A1F52 for ; Thu, 8 Sep 2016 01:38:59 -0700 (PDT) Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id ACF9D3F720; Thu, 8 Sep 2016 08:38:58 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-57.phx2.redhat.com [10.3.116.57]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u888cv3H023606; Thu, 8 Sep 2016 04:38:57 -0400 To: Joseph Shifflett , edk2-devel@ml01.01.org, Paolo Bonzini References: <1473309015-26017-1-git-send-email-joseph.shifflett@hpe.com> <1473309015-26017-5-git-send-email-joseph.shifflett@hpe.com> From: Laszlo Ersek Message-ID: <658e188d-7ac9-0166-c799-1f28fc80d292@redhat.com> Date: Thu, 8 Sep 2016 10:38:56 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <1473309015-26017-5-git-send-email-joseph.shifflett@hpe.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Thu, 08 Sep 2016 08:38:58 +0000 (UTC) Subject: Re: [PATCH v1 4/5] OvmfPkg/SmmCpuFeaturesLib: Abstact processor features X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 08 Sep 2016 08:38:59 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Adding Paolo. Paolo, here's the root of the thread: https://lists.01.org/pipermail/edk2-devel/2016-September/001500.html On 09/08/16 06:30, Joseph Shifflett wrote: > Create new functions to abstract how XD/NX is detected, enabled, and > disabled. Also, create a new function to determine if Branch Trace > Storage is supported. Existing code is specific to Intel processors. > > This provides NULL implementations of the new functions. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Joseph Shifflett > --- > OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c | 44 ++++++++++++++++++++ > 1 file changed, 44 insertions(+) > > diff --git a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c > index a307f64c9c61..4c8c7fb7b25f 100644 > --- a/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c > +++ b/OvmfPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c > @@ -2,6 +2,7 @@ > The CPU specific programming for PiSmmCpuDxeSmm module. > > Copyright (c) 2010 - 2015, Intel Corporation. All rights reserved.
> +(C) Copyright 2016 Hewlett Packard Enterprise Development LP
> This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD License > which accompanies this distribution. The full text of the license may be found at > @@ -832,3 +833,46 @@ SmmCpuFeaturesAllocatePageTableMemory ( > return NULL; > } > > +/** > + This API provides a method to determine if XD/NX support has been forced off in > + the non-SMM execution environment. It will enable XD/NX support while in SMM > + > + @retval TRUE XD/NX was disabled when runningn in the non-SMM execution environment > + @retval FALSE XD/NX was enabled when runningn in the non-SMM execution environment > + > +**/ > +BOOLEAN > +EFIAPI > +SmmCpuFeaturesCheckAndEnableXdSupport ( > + VOID > + ) > +{ > + return FALSE; > +} > + > +/** > + This API provides a method to disable XD/NX support before exiting SMM > +**/ > +VOID > +EFIAPI > +SmmCpuFeaturesDisableXdSupport ( > + VOID > + ) > +{ > +} > + > +/** > + This API determines if Branch Trace Storage Support is currently available > + > + @retval TRUE BTS is available > + @retval FALSE BTS is disabled > + > +**/ > +BOOLEAN > +EFIAPI > +SmmCpuFeaturesConfirmBranchTraceStorageSupport ( > + VOID > + ) > +{ > + return FALSE; > +} > I think it would be best to check these with Paolo, as he authored OvmfPkg's SmmCpuFeaturesLib. * The XD manipulation (wrt. the first two functions) is definitely supported by KVM, so at least for those functions, I think the same functionality as in the UefiCpuPkg library instance would be appropriate. (See some additional pondering at [1] below.) * For the last function, I think KVM does not implement the branch recording MSRs. I also don't know if it even implements the "feature query" MSR for the branch recording feature. Here too, the UefiCpuPkg variant might be more appropriate for OvmfPkg than a constant FALSE -- please consult with Paolo. Most importantly (I should have asked this first): what are you going to use these functions for? I assume you are abstracting PiSmmCpuDxeSmm from Ia32/X64, bringing it closer to ARM/AARCH64. It would be nice to see the exact actions in PiSmmCpuDxeSmm that you plan to rebase on these new library APIs. If you post a version 2 of the patch set, - please spell out the intended *concrete* use of each function in the commit message of patch #1 (that is, where you modify the library class header), - please CC Paolo on the entire set. ( [1] I recall that there was some nastiness wrt. the AMD SMM save state area *not* saving and restoring XD state when entering and leaving SMM *on Ia32*, but correctly saving and restoring XD state when doing the same on X64. Therefore our current recommendation for running a 32-bit OVMF binary (that was built with -D SMM_REQUIRE) on qemu-system-i386 is to pass -cpu XXXX,-nx because if XD is presented to the guest, then SMM will break. IOW, currently we ask the QEMU user to hide the XD processor feature on 32-bit processors. I don't know if we could get rid of the user-side tweak with the help of this library. I guess you might be able to add MDE_CPU_IA32 vs. MDE_CPU_X64 specific code to the first two functions in OvmfPkg; reporting "XD invariably absent" in the MDE_CPU_IA32 case. However I'm worried that some other component in the firmware would still query MSR_IA32_MISC_ENABLE, and then be inconsistent with the constant retval from here. Thus we'll likely have to keep the -nx tweak in OvmfPkg/README anyway, and in turn these functions should not be specific to Ia32/X64 in OvmfPkg. I believe the first two functions should just copy the UefiCpuPkg implementation, and possibly even the third one. But, check with Paolo please. ) Thanks! Laszlo