From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) (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 EF54520D7648E for ; Wed, 12 Apr 2017 17:58:31 -0700 (PDT) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 12 Apr 2017 17:58:31 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.37,192,1488873600"; d="scan'208";a="247970964" Received: from orsmsx105.amr.corp.intel.com ([10.22.225.132]) by fmsmga004.fm.intel.com with ESMTP; 12 Apr 2017 17:58:31 -0700 Received: from orsmsx160.amr.corp.intel.com (10.22.226.43) by ORSMSX105.amr.corp.intel.com (10.22.225.132) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 12 Apr 2017 17:58:31 -0700 Received: from orsmsx113.amr.corp.intel.com ([169.254.9.59]) by ORSMSX160.amr.corp.intel.com ([169.254.13.129]) with mapi id 14.03.0319.002; Wed, 12 Apr 2017 17:58:30 -0700 From: "Kinney, Michael D" To: Laszlo Ersek , Leo Duran , "edk2-devel@ml01.01.org" , "Kinney, Michael D" CC: "Tian, Feng" , Brijesh Singh , "Zeng, Star" Thread-Topic: [edk2] [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL Thread-Index: AQHSs76QLLN8w8aIl0q3JQpKnVQPVqHCiuUA//+P2vCAAIr8gP//0o7w Date: Thu, 13 Apr 2017 00:58:30 +0000 Message-ID: References: <1492023400-16132-1-git-send-email-leo.duran@amd.com> <1492023400-16132-2-git-send-email-leo.duran@amd.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_IC x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMjE5OGQ0ZmUtYWE4Ny00ZjZhLWJkNjEtZmRmMDU3ODAyN2U1IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6IkdBREwrdG1DZk1KY01PaUtXVHQ2aGlQN1dsbGFiSTdHTHlQVERRclU3dmM9In0= dlp-product: dlpe-windows dlp-version: 10.0.102.7 dlp-reaction: no-action x-originating-ip: [10.22.254.138] MIME-Version: 1.0 Subject: Re: [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 13 Apr 2017 00:58:32 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Laszlo and Leo, If you look at the CpuSetMemoryAttributes() function in=20 UefiCpuPkg/CpuDxe/CpuDxe.c, you will see it now calls AssignMemoryPageAttributes() at the end. This function=20 is implemented in UefiCpuPkg/CpuDxe/CpuPageTable.c and it updates page table attributes based on the request. This looks like a very similar action to update PTEs. What MMIO areas are we discussing here? MMIO ranges associated with PCI BARs allocated through PCI Bus Driver and PCI Host Bridge driver? Mike > -----Original Message----- > From: Laszlo Ersek [mailto:lersek@redhat.com] > Sent: Wednesday, April 12, 2017 1:33 PM > To: Kinney, Michael D ; Leo Duran > ; edk2-devel@ml01.01.org > Cc: Tian, Feng ; Brijesh Singh ; > Zeng, Star > Subject: Re: [edk2] [PATCH] MdeModulePkg: Add > EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL >=20 > Mike, >=20 > On 04/12/17 21:39, Kinney, Michael D wrote: > > Hi Leo, > > > > DXE Main is supposed to use Arch Protocols for platform specific behavi= or. > > > > In this case, can the CPU Arch Protocol SetMemoryAttributes() function > > be used to customize behavior? > > > > In other modules that add/remove/modify MMIO, gDS->SetMemorySpaceAttrib= utes() > > Is called to set memory space attributed such as cachability. Can the > > module that is adding/removing memory regions from GCD also call > > gDS-> SetMemorySpaceAttributes() so a CPU specific module can perform > > extra actions? >=20 > The goal is to clear the "C" bit in the PTEs (--> disable encryption in > SEV guests) for all MMIO areas in the GCD memory space map, when they > are added, and to re-set the "C" bit when the area is removed (or the > area's GCD type is changed to something non-MMIO). >=20 > This behavior should cover all possible gDS->AddMemorySpace() calls (for > the MMIO type), regardless of what driver calls gDS->AddMemorySpace(). > In other words, drivers that add MMIO space should remain unaware of SEV > / the "C" bit in PTEs. Otherwise we'd have to enable SEV detection and > "C" bit massaging in every affected driver individually (even future > ones, possibly pulled in from under MdeModulePkg or UefiCpuPkg). > Cacheability and similar attributes for memory ranges are quite > universal on all architectures, and it is okay to expect all drivers to > manage those attributes manually (whenever that's necessary), but (a) > SEV / memory encryption is not that universal, and (b) clearing "C" for > MMIO ranges affects all drivers alike, so it would be good to generalize > it at a low level. >=20 > What would be the best general location or API to hook the "C" bit > manipulation into MMIO GCD memory space addition / removal? >=20 > Thank you, > Laszlo >=20 >=20 > > > > Thanks, > > > > Mike > > > > > > > >> -----Original Message----- > >> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of= Leo > Duran > >> Sent: Wednesday, April 12, 2017 11:57 AM > >> To: edk2-devel@ml01.01.org > >> Cc: Laszlo Ersek ; Tian, Feng = ; Leo > Duran > >> ; Brijesh Singh ; Zeng, Star > >> > >> Subject: [edk2] [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_= PROTOCOL > >> > >> GCD consumes the protocol to issue a Notify() on Add/Remove operations= . > >> > >> The intended use-case is to allow OvmfPkg take actions on behalf of an > >> SEV-enabled guest. > >> > >> The new protocol is simply added to the list of optional protocols han= dled > >> by DxeMain, and as such leverages the existing DxeProtocolNotify frame= work. > >> > >> Cc: Feng Tian > >> Cc: Star Zeng > >> Cc: Laszlo Ersek > >> Cc: Brijesh Singh > >> Contributed-under: TianoCore Contribution Agreement 1.0 > >> Signed-off-by: Leo Duran > >> --- > >> MdeModulePkg/Core/Dxe/DxeMain.h | 10 +++- > >> MdeModulePkg/Core/Dxe/DxeMain.inf | 4 ++ > >> MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c | 7 +++ > >> MdeModulePkg/Core/Dxe/DxeMain/DxeProtocolNotify.c | 9 ++- > >> MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 8 +++ > >> .../Include/Protocol/GcdMemorySpaceNotify.h | 65 > ++++++++++++++++++++++ > >> MdeModulePkg/MdeModulePkg.dec | 3 + > >> 7 files changed, 100 insertions(+), 6 deletions(-) > >> create mode 100644 MdeModulePkg/Include/Protocol/GcdMemorySpaceNotify= .h > >> > >> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/D= xeMain.h > >> index 1a0babb..8a037ff 100644 > >> --- a/MdeModulePkg/Core/Dxe/DxeMain.h > >> +++ b/MdeModulePkg/Core/Dxe/DxeMain.h > >> @@ -3,6 +3,8 @@ > >> internal structure and functions used by DxeCore module. > >> > >> Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved. > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.
> >> + > >> 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 ma= y be > found > >> at > >> @@ -17,7 +19,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, E= ITHER > >> EXPRESS OR IMPLIED. > >> #define _DXE_MAIN_H_ > >> > >> > >> - > >> #include > >> > >> #include > >> @@ -53,6 +54,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, E= ITHER > >> EXPRESS OR IMPLIED. > >> #include > >> #include > >> #include > >> +#include > >> + > >> #include > >> #include > >> #include > >> @@ -296,12 +299,13 @@ extern EFI_RUNTIME_ARCH_PROTOCOL > >> gRuntimeTemplate; > >> > >> extern EFI_LOAD_FIXED_ADDRESS_CONFIGURATION_TABLE > >> gLoadModuleAtFixAddressConfigurationTable; > >> extern BOOLEAN > >> gLoadFixedAddressCodeMemoryReady; > >> + > >> +extern EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL *gGcdMemorySpaceNotif= y; > >> + > >> // > >> // Service Initialization Functions > >> // > >> > >> - > >> - > >> /** > >> Called to initialize the pool. > >> > >> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf > >> b/MdeModulePkg/Core/Dxe/DxeMain.inf > >> index 30d5984..888a16f 100644 > >> --- a/MdeModulePkg/Core/Dxe/DxeMain.inf > >> +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf > >> @@ -4,6 +4,8 @@ > >> # It provides an implementation of DXE Core that is compliant with D= XE CIS. > >> # > >> # Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.=
> >> +# Copyright (c) 2017, AMD Incorporated. All rights reserved.
> >> +# > >> # 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 > >> @@ -162,6 +164,8 @@ > >> gEfiSmmBase2ProtocolGuid ## SOMETIMES_CONSUMES > >> gEfiBlockIoProtocolGuid ## SOMETIMES_CONSUMES > >> > >> + gEfiGcdMemorySpaceNotifyProtocolGuid ## CONSUMES > >> + > >> # Arch Protocols > >> gEfiBdsArchProtocolGuid ## CONSUMES > >> gEfiCpuArchProtocolGuid ## CONSUMES > >> diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c > >> b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c > >> index 91e94a7..46b68da 100644 > >> --- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c > >> +++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c > >> @@ -2,6 +2,8 @@ > >> DXE Core Main Entry Point > >> > >> Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved. > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.
> >> + > >> 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 ma= y be > found > >> at > >> @@ -42,6 +44,11 @@ EFI_GUID *gDxeCoreFileNam= e; > >> EFI_LOADED_IMAGE_PROTOCOL *gDxeCoreLoadedImage; > >> > >> // > >> +// DXE Core global for GCD notification protocol > >> +// > >> +EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL *gGcdMemorySpaceNotify =3D NULL= ; > >> + > >> +// > >> // DXE Core Module Variables > >> // > >> EFI_BOOT_SERVICES mBootServices =3D { > >> diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeProtocolNotify.c > >> b/MdeModulePkg/Core/Dxe/DxeMain/DxeProtocolNotify.c > >> index ea7c610..2314e34 100644 > >> --- a/MdeModulePkg/Core/Dxe/DxeMain/DxeProtocolNotify.c > >> +++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeProtocolNotify.c > >> @@ -4,6 +4,8 @@ > >> events that represent the Architectural Protocols. > >> > >> Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved. > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.
> >> + > >> 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 ma= y be > found > >> at > >> @@ -45,9 +47,10 @@ EFI_CORE_PROTOCOL_NOTIFY_ENTRY mArchProtocols[] = =3D { > >> // Optional protocols that the DXE Core will use if they are present > >> // > >> EFI_CORE_PROTOCOL_NOTIFY_ENTRY mOptionalProtocols[] =3D { > >> - { &gEfiSecurity2ArchProtocolGuid, (VOID **)&gSecurity2, = NULL, > NULL, > >> FALSE }, > >> - { &gEfiSmmBase2ProtocolGuid, (VOID **)&gSmmBase2, = NULL, > NULL, > >> FALSE }, > >> - { NULL, (VOID **)NULL, = NULL, > NULL, > >> FALSE } > >> + { &gEfiSecurity2ArchProtocolGuid, (VOID **)&gSecurity2, > >> NULL, NULL, FALSE }, > >> + { &gEfiSmmBase2ProtocolGuid, (VOID **)&gSmmBase2, > >> NULL, NULL, FALSE }, > >> + { &gEfiGcdMemorySpaceNotifyProtocolGuid, (VOID **)&gGcdMemorySpaceN= otify, > >> NULL, NULL, FALSE }, > >> + { NULL, (VOID **)NULL, > >> NULL, NULL, FALSE } > >> }; > >> > >> // > >> diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/G= cd/Gcd.c > >> index a06f8bb..223fcd8 100644 > >> --- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > >> +++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > >> @@ -4,6 +4,8 @@ > >> are accessible to the CPU that is executing the DXE core. > >> > >> Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved. > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.
> >> + > >> 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 ma= y be > found > >> at > >> @@ -896,6 +898,9 @@ CoreConvertSpace ( > >> } else { > >> Entry->Capabilities =3D Capabilities | EFI_MEMORY_RUNTIME; > >> } > >> + if (gGcdMemorySpaceNotify) { > >> + gGcdMemorySpaceNotify->MemorySpaceAddNotify (GcdMemoryType, > BaseAddress, > >> Length, Entry->Capabilities); > >> + } > >> break; > >> case GCD_ADD_IO_OPERATION: > >> Entry->GcdIoType =3D GcdIoType; > >> @@ -914,6 +919,9 @@ CoreConvertSpace ( > >> case GCD_REMOVE_MEMORY_OPERATION: > >> Entry->GcdMemoryType =3D EfiGcdMemoryTypeNonExistent; > >> Entry->Capabilities =3D 0; > >> + if (gGcdMemorySpaceNotify) { > >> + gGcdMemorySpaceNotify->MemorySpaceRemoveNotify (BaseAddress, = Length); > >> + } > >> break; > >> case GCD_REMOVE_IO_OPERATION: > >> Entry->GcdIoType =3D EfiGcdIoTypeNonExistent; > >> diff --git a/MdeModulePkg/Include/Protocol/GcdMemorySpaceNotify.h > >> b/MdeModulePkg/Include/Protocol/GcdMemorySpaceNotify.h > >> new file mode 100644 > >> index 0000000..9174957 > >> --- /dev/null > >> +++ b/MdeModulePkg/Include/Protocol/GcdMemorySpaceNotify.h > >> @@ -0,0 +1,65 @@ > >> +/** @file > >> + This file declares the GcdMemorySpaceNotify Protocol. > >> + > >> + This Protocol is consumed by GCD to issue notications during ADD/RE= MOVE > >> operations. > >> + > >> + Copyright (c) 2017, AMD Inc. All rights reserved.
> >> + > >> + This program and the accompanying materials > >> + are licensed and made available under the terms and conditions of t= he BSD > >> License > >> + which accompanies this distribution. The full text of the license = may be > >> found at > >> + http://opensource.org/licenses/bsd-license.php > >> + > >> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASI= S, > >> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS O= R > IMPLIED. > >> + > >> +**/ > >> + > >> +#ifndef __EFI_GCD_MEMORY_SPACE_NOTIFY_H__ > >> +#define __EFI_GCD_MEMORY_SPACE_NOTIFY_H__ > >> + > >> +#define EFI_GCD_MEMORY_SPACE_NOTIFY_GUID \ > >> + { \ > >> + 0xc842db69, 0x610e, 0x401a, {0x90, 0xd0, 0x88, 0x41, 0xf1, 0xdc, = 0x53, > 0x79 > >> } \ > >> + } > >> + > >> +/** > >> + Notify on: Add a segment of memory to GCD map. > >> + > >> + @param GcdMemoryType Memory type of the segment. > >> + @param BaseAddress Base address of the segment. > >> + @param Length Length of the segment. > >> + @param Capabilities Alterable attributes of the segment. > >> + > >> +**/ > >> +typedef > >> +VOID > >> +(EFIAPI *GCD_ADD_MEMORY_NOTIFY) ( > >> + IN EFI_GCD_MEMORY_TYPE GcdMemoryType, > >> + IN EFI_PHYSICAL_ADDRESS BaseAddress, > >> + IN UINT64 Length, > >> + IN UINT64 Capabilities > >> +); > >> + > >> +/** > >> + Notify on: Remove a segment of memory to GCD map. > >> + > >> + @param BaseAddress Base address of the segment. > >> + @param Length Length of the segment. > >> + > >> +**/ > >> +typedef > >> +VOID > >> +(EFIAPI *GCD_REMOVE_MEMORY_NOTIFY) ( > >> + IN EFI_PHYSICAL_ADDRESS BaseAddress, > >> + IN UINT64 Length > >> +); > >> + > >> +typedef struct { > >> + GCD_ADD_MEMORY_NOTIFY MemorySpaceAddNotify; > >> + GCD_REMOVE_MEMORY_NOTIFY MemorySpaceRemoveNotify; > >> +} EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL; > >> + > >> +extern EFI_GUID gEfiGcdMemorySpaceNotifyProtocolGuid; > >> + > >> +#endif > >> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg= .dec > >> index ca09cbc..95f9311 100644 > >> --- a/MdeModulePkg/MdeModulePkg.dec > >> +++ b/MdeModulePkg/MdeModulePkg.dec > >> @@ -546,6 +546,9 @@ > >> ## Include/Protocol/NonDiscoverableDevice.h > >> gEdkiiNonDiscoverableDeviceProtocolGuid =3D { 0x0d51905b, 0xb77e, 0= x452a, > {0xa2, > >> 0xc0, 0xec, 0xa0, 0xcc, 0x8d, 0x51, 0x4a } } > >> > >> + ## Include/Protocol/GcdMemorySpaceNotify.h > >> + gEfiGcdMemorySpaceNotifyProtocolGuid =3D { 0xc842db69, 0x610e, 0x4= 01a, > {0x90, > >> 0xd0, 0x88, 0x41, 0xf1, 0xdc, 0x53, 0x79 } } > >> + > >> # > >> # [Error.gEfiMdeModulePkgTokenSpaceGuid] > >> # 0x80000001 | Invalid value provided. > >> -- > >> 2.7.4 > >> > >> _______________________________________________ > >> edk2-devel mailing list > >> edk2-devel@lists.01.org > >> https://lists.01.org/mailman/listinfo/edk2-devel