* [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL
@ 2017-04-12 18:56 Leo Duran
2017-04-12 18:56 ` Leo Duran
0 siblings, 1 reply; 13+ messages in thread
From: Leo Duran @ 2017-04-12 18:56 UTC (permalink / raw)
To: edk2-devel; +Cc: Leo Duran
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 handled
by DxeMain, and as such leverages the existing DxeProtocolNotify framework.
I will follow this patch with "proof-of-concept" OvmfPkg driver that
installs the protocol to take pertinent actions based on GCD notifications.
Leo Duran (1):
MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL
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
--
2.7.4
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL 2017-04-12 18:56 [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL Leo Duran @ 2017-04-12 18:56 ` Leo Duran 2017-04-12 19:39 ` Kinney, Michael D 0 siblings, 1 reply; 13+ messages in thread From: Leo Duran @ 2017-04-12 18:56 UTC (permalink / raw) To: edk2-devel; +Cc: Leo Duran, Feng Tian, Star Zeng, Laszlo Ersek, Brijesh Singh 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 handled by DxeMain, and as such leverages the existing DxeProtocolNotify framework. Cc: Feng Tian <feng.tian@intel.com> Cc: Star Zeng <star.zeng@intel.com> Cc: Laszlo Ersek <lersek@redhat.com> Cc: Brijesh Singh <brijesh.singh@amd.com> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Leo Duran <leo.duran@amd.com> --- 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/DxeMain.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.<BR> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> + 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 @@ -17,7 +19,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #define _DXE_MAIN_H_ - #include <PiDxe.h> #include <Protocol/LoadedImage.h> @@ -53,6 +54,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #include <Protocol/TcgService.h> #include <Protocol/HiiPackageList.h> #include <Protocol/SmmBase2.h> +#include <Protocol/GcdMemorySpaceNotify.h> + #include <Guid/MemoryTypeInformation.h> #include <Guid/FirmwareFileSystem2.h> #include <Guid/FirmwareFileSystem3.h> @@ -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 *gGcdMemorySpaceNotify; + // // 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 DXE CIS. # # Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR> +# Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> +# # 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.<BR> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> + 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 @@ -42,6 +44,11 @@ EFI_GUID *gDxeCoreFileName; EFI_LOADED_IMAGE_PROTOCOL *gDxeCoreLoadedImage; // +// DXE Core global for GCD notification protocol +// +EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL *gGcdMemorySpaceNotify = NULL; + +// // DXE Core Module Variables // EFI_BOOT_SERVICES mBootServices = { 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.<BR> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> + 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 @@ -45,9 +47,10 @@ EFI_CORE_PROTOCOL_NOTIFY_ENTRY mArchProtocols[] = { // Optional protocols that the DXE Core will use if they are present // EFI_CORE_PROTOCOL_NOTIFY_ENTRY mOptionalProtocols[] = { - { &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 **)&gGcdMemorySpaceNotify, NULL, NULL, FALSE }, + { NULL, (VOID **)NULL, NULL, NULL, FALSE } }; // diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/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.<BR> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> + 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 @@ -896,6 +898,9 @@ CoreConvertSpace ( } else { Entry->Capabilities = Capabilities | EFI_MEMORY_RUNTIME; } + if (gGcdMemorySpaceNotify) { + gGcdMemorySpaceNotify->MemorySpaceAddNotify (GcdMemoryType, BaseAddress, Length, Entry->Capabilities); + } break; case GCD_ADD_IO_OPERATION: Entry->GcdIoType = GcdIoType; @@ -914,6 +919,9 @@ CoreConvertSpace ( case GCD_REMOVE_MEMORY_OPERATION: Entry->GcdMemoryType = EfiGcdMemoryTypeNonExistent; Entry->Capabilities = 0; + if (gGcdMemorySpaceNotify) { + gGcdMemorySpaceNotify->MemorySpaceRemoveNotify (BaseAddress, Length); + } break; case GCD_REMOVE_IO_OPERATION: Entry->GcdIoType = 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/REMOVE operations. + + Copyright (c) 2017, AMD Inc. All rights reserved.<BR> + + 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 + http://opensource.org/licenses/bsd-license.php + + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 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 = { 0x0d51905b, 0xb77e, 0x452a, {0xa2, 0xc0, 0xec, 0xa0, 0xcc, 0x8d, 0x51, 0x4a } } + ## Include/Protocol/GcdMemorySpaceNotify.h + gEfiGcdMemorySpaceNotifyProtocolGuid = { 0xc842db69, 0x610e, 0x401a, {0x90, 0xd0, 0x88, 0x41, 0xf1, 0xdc, 0x53, 0x79 } } + # # [Error.gEfiMdeModulePkgTokenSpaceGuid] # 0x80000001 | Invalid value provided. -- 2.7.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL 2017-04-12 18:56 ` Leo Duran @ 2017-04-12 19:39 ` Kinney, Michael D 2017-04-12 20:32 ` Laszlo Ersek 2017-04-12 20:34 ` Duran, Leo 0 siblings, 2 replies; 13+ messages in thread From: Kinney, Michael D @ 2017-04-12 19:39 UTC (permalink / raw) To: Leo Duran, edk2-devel@ml01.01.org, Kinney, Michael D Cc: Laszlo Ersek, Tian, Feng, Brijesh Singh, Zeng, Star Hi Leo, DXE Main is supposed to use Arch Protocols for platform specific behavior. In this case, can the CPU Arch Protocol SetMemoryAttributes() function be used to customize behavior? In other modules that add/remove/modify MMIO, gDS->SetMemorySpaceAttributes() 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? 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 <lersek@redhat.com>; Tian, Feng <feng.tian@intel.com>; Leo Duran > <leo.duran@amd.com>; Brijesh Singh <brijesh.singh@amd.com>; Zeng, Star > <star.zeng@intel.com> > 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 handled > by DxeMain, and as such leverages the existing DxeProtocolNotify framework. > > Cc: Feng Tian <feng.tian@intel.com> > Cc: Star Zeng <star.zeng@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com> > Cc: Brijesh Singh <brijesh.singh@amd.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Leo Duran <leo.duran@amd.com> > --- > 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/DxeMain.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.<BR> > +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > + > 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 > @@ -17,7 +19,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER > EXPRESS OR IMPLIED. > #define _DXE_MAIN_H_ > > > - > #include <PiDxe.h> > > #include <Protocol/LoadedImage.h> > @@ -53,6 +54,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER > EXPRESS OR IMPLIED. > #include <Protocol/TcgService.h> > #include <Protocol/HiiPackageList.h> > #include <Protocol/SmmBase2.h> > +#include <Protocol/GcdMemorySpaceNotify.h> > + > #include <Guid/MemoryTypeInformation.h> > #include <Guid/FirmwareFileSystem2.h> > #include <Guid/FirmwareFileSystem3.h> > @@ -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 *gGcdMemorySpaceNotify; > + > // > // 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 DXE CIS. > # > # Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR> > +# Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > +# > # 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.<BR> > +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > + > 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 > @@ -42,6 +44,11 @@ EFI_GUID *gDxeCoreFileName; > EFI_LOADED_IMAGE_PROTOCOL *gDxeCoreLoadedImage; > > // > +// DXE Core global for GCD notification protocol > +// > +EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL *gGcdMemorySpaceNotify = NULL; > + > +// > // DXE Core Module Variables > // > EFI_BOOT_SERVICES mBootServices = { > 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.<BR> > +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > + > 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 > @@ -45,9 +47,10 @@ EFI_CORE_PROTOCOL_NOTIFY_ENTRY mArchProtocols[] = { > // Optional protocols that the DXE Core will use if they are present > // > EFI_CORE_PROTOCOL_NOTIFY_ENTRY mOptionalProtocols[] = { > - { &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 **)&gGcdMemorySpaceNotify, > NULL, NULL, FALSE }, > + { NULL, (VOID **)NULL, > NULL, NULL, FALSE } > }; > > // > diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/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.<BR> > +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > + > 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 > @@ -896,6 +898,9 @@ CoreConvertSpace ( > } else { > Entry->Capabilities = Capabilities | EFI_MEMORY_RUNTIME; > } > + if (gGcdMemorySpaceNotify) { > + gGcdMemorySpaceNotify->MemorySpaceAddNotify (GcdMemoryType, BaseAddress, > Length, Entry->Capabilities); > + } > break; > case GCD_ADD_IO_OPERATION: > Entry->GcdIoType = GcdIoType; > @@ -914,6 +919,9 @@ CoreConvertSpace ( > case GCD_REMOVE_MEMORY_OPERATION: > Entry->GcdMemoryType = EfiGcdMemoryTypeNonExistent; > Entry->Capabilities = 0; > + if (gGcdMemorySpaceNotify) { > + gGcdMemorySpaceNotify->MemorySpaceRemoveNotify (BaseAddress, Length); > + } > break; > case GCD_REMOVE_IO_OPERATION: > Entry->GcdIoType = 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/REMOVE > operations. > + > + Copyright (c) 2017, AMD Inc. All rights reserved.<BR> > + > + 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 > + http://opensource.org/licenses/bsd-license.php > + > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 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 = { 0x0d51905b, 0xb77e, 0x452a, {0xa2, > 0xc0, 0xec, 0xa0, 0xcc, 0x8d, 0x51, 0x4a } } > > + ## Include/Protocol/GcdMemorySpaceNotify.h > + gEfiGcdMemorySpaceNotifyProtocolGuid = { 0xc842db69, 0x610e, 0x401a, {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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL 2017-04-12 19:39 ` Kinney, Michael D @ 2017-04-12 20:32 ` Laszlo Ersek 2017-04-13 0:58 ` Kinney, Michael D 2017-04-12 20:34 ` Duran, Leo 1 sibling, 1 reply; 13+ messages in thread From: Laszlo Ersek @ 2017-04-12 20:32 UTC (permalink / raw) To: Kinney, Michael D, Leo Duran, edk2-devel@ml01.01.org Cc: Tian, Feng, Brijesh Singh, Zeng, Star Mike, On 04/12/17 21:39, Kinney, Michael D wrote: > Hi Leo, > > DXE Main is supposed to use Arch Protocols for platform specific behavior. > > In this case, can the CPU Arch Protocol SetMemoryAttributes() function > be used to customize behavior? > > In other modules that add/remove/modify MMIO, gDS->SetMemorySpaceAttributes() > 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? 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). 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. What would be the best general location or API to hook the "C" bit manipulation into MMIO GCD memory space addition / removal? Thank you, Laszlo > > 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 <lersek@redhat.com>; Tian, Feng <feng.tian@intel.com>; Leo Duran >> <leo.duran@amd.com>; Brijesh Singh <brijesh.singh@amd.com>; Zeng, Star >> <star.zeng@intel.com> >> 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 handled >> by DxeMain, and as such leverages the existing DxeProtocolNotify framework. >> >> Cc: Feng Tian <feng.tian@intel.com> >> Cc: Star Zeng <star.zeng@intel.com> >> Cc: Laszlo Ersek <lersek@redhat.com> >> Cc: Brijesh Singh <brijesh.singh@amd.com> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Leo Duran <leo.duran@amd.com> >> --- >> 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/DxeMain.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.<BR> >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> >> + >> 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 >> @@ -17,7 +19,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER >> EXPRESS OR IMPLIED. >> #define _DXE_MAIN_H_ >> >> >> - >> #include <PiDxe.h> >> >> #include <Protocol/LoadedImage.h> >> @@ -53,6 +54,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER >> EXPRESS OR IMPLIED. >> #include <Protocol/TcgService.h> >> #include <Protocol/HiiPackageList.h> >> #include <Protocol/SmmBase2.h> >> +#include <Protocol/GcdMemorySpaceNotify.h> >> + >> #include <Guid/MemoryTypeInformation.h> >> #include <Guid/FirmwareFileSystem2.h> >> #include <Guid/FirmwareFileSystem3.h> >> @@ -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 *gGcdMemorySpaceNotify; >> + >> // >> // 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 DXE CIS. >> # >> # Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR> >> +# Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> >> +# >> # 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.<BR> >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> >> + >> 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 >> @@ -42,6 +44,11 @@ EFI_GUID *gDxeCoreFileName; >> EFI_LOADED_IMAGE_PROTOCOL *gDxeCoreLoadedImage; >> >> // >> +// DXE Core global for GCD notification protocol >> +// >> +EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL *gGcdMemorySpaceNotify = NULL; >> + >> +// >> // DXE Core Module Variables >> // >> EFI_BOOT_SERVICES mBootServices = { >> 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.<BR> >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> >> + >> 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 >> @@ -45,9 +47,10 @@ EFI_CORE_PROTOCOL_NOTIFY_ENTRY mArchProtocols[] = { >> // Optional protocols that the DXE Core will use if they are present >> // >> EFI_CORE_PROTOCOL_NOTIFY_ENTRY mOptionalProtocols[] = { >> - { &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 **)&gGcdMemorySpaceNotify, >> NULL, NULL, FALSE }, >> + { NULL, (VOID **)NULL, >> NULL, NULL, FALSE } >> }; >> >> // >> diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/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.<BR> >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> >> + >> 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 >> @@ -896,6 +898,9 @@ CoreConvertSpace ( >> } else { >> Entry->Capabilities = Capabilities | EFI_MEMORY_RUNTIME; >> } >> + if (gGcdMemorySpaceNotify) { >> + gGcdMemorySpaceNotify->MemorySpaceAddNotify (GcdMemoryType, BaseAddress, >> Length, Entry->Capabilities); >> + } >> break; >> case GCD_ADD_IO_OPERATION: >> Entry->GcdIoType = GcdIoType; >> @@ -914,6 +919,9 @@ CoreConvertSpace ( >> case GCD_REMOVE_MEMORY_OPERATION: >> Entry->GcdMemoryType = EfiGcdMemoryTypeNonExistent; >> Entry->Capabilities = 0; >> + if (gGcdMemorySpaceNotify) { >> + gGcdMemorySpaceNotify->MemorySpaceRemoveNotify (BaseAddress, Length); >> + } >> break; >> case GCD_REMOVE_IO_OPERATION: >> Entry->GcdIoType = 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/REMOVE >> operations. >> + >> + Copyright (c) 2017, AMD Inc. All rights reserved.<BR> >> + >> + 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 >> + http://opensource.org/licenses/bsd-license.php >> + >> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, >> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 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 = { 0x0d51905b, 0xb77e, 0x452a, {0xa2, >> 0xc0, 0xec, 0xa0, 0xcc, 0x8d, 0x51, 0x4a } } >> >> + ## Include/Protocol/GcdMemorySpaceNotify.h >> + gEfiGcdMemorySpaceNotifyProtocolGuid = { 0xc842db69, 0x610e, 0x401a, {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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL 2017-04-12 20:32 ` Laszlo Ersek @ 2017-04-13 0:58 ` Kinney, Michael D 2017-04-13 2:13 ` Zeng, Star 2017-04-13 2:27 ` Duran, Leo 0 siblings, 2 replies; 13+ messages in thread From: Kinney, Michael D @ 2017-04-13 0:58 UTC (permalink / raw) To: Laszlo Ersek, Leo Duran, edk2-devel@ml01.01.org, Kinney, Michael D Cc: Tian, Feng, Brijesh Singh, Zeng, Star Hi Laszlo and Leo, If you look at the CpuSetMemoryAttributes() function in UefiCpuPkg/CpuDxe/CpuDxe.c, you will see it now calls AssignMemoryPageAttributes() at the end. This function 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 <michael.d.kinney@intel.com>; Leo Duran > <leo.duran@amd.com>; edk2-devel@ml01.01.org > Cc: Tian, Feng <feng.tian@intel.com>; Brijesh Singh <brijesh.singh@amd.com>; > Zeng, Star <star.zeng@intel.com> > Subject: Re: [edk2] [PATCH] MdeModulePkg: Add > EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL > > Mike, > > On 04/12/17 21:39, Kinney, Michael D wrote: > > Hi Leo, > > > > DXE Main is supposed to use Arch Protocols for platform specific behavior. > > > > In this case, can the CPU Arch Protocol SetMemoryAttributes() function > > be used to customize behavior? > > > > In other modules that add/remove/modify MMIO, gDS->SetMemorySpaceAttributes() > > 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? > > 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). > > 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. > > What would be the best general location or API to hook the "C" bit > manipulation into MMIO GCD memory space addition / removal? > > Thank you, > Laszlo > > > > > > 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 <lersek@redhat.com>; Tian, Feng <feng.tian@intel.com>; Leo > Duran > >> <leo.duran@amd.com>; Brijesh Singh <brijesh.singh@amd.com>; Zeng, Star > >> <star.zeng@intel.com> > >> 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 handled > >> by DxeMain, and as such leverages the existing DxeProtocolNotify framework. > >> > >> Cc: Feng Tian <feng.tian@intel.com> > >> Cc: Star Zeng <star.zeng@intel.com> > >> Cc: Laszlo Ersek <lersek@redhat.com> > >> Cc: Brijesh Singh <brijesh.singh@amd.com> > >> Contributed-under: TianoCore Contribution Agreement 1.0 > >> Signed-off-by: Leo Duran <leo.duran@amd.com> > >> --- > >> 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/DxeMain.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.<BR> > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > >> + > >> 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 > >> @@ -17,7 +19,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER > >> EXPRESS OR IMPLIED. > >> #define _DXE_MAIN_H_ > >> > >> > >> - > >> #include <PiDxe.h> > >> > >> #include <Protocol/LoadedImage.h> > >> @@ -53,6 +54,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER > >> EXPRESS OR IMPLIED. > >> #include <Protocol/TcgService.h> > >> #include <Protocol/HiiPackageList.h> > >> #include <Protocol/SmmBase2.h> > >> +#include <Protocol/GcdMemorySpaceNotify.h> > >> + > >> #include <Guid/MemoryTypeInformation.h> > >> #include <Guid/FirmwareFileSystem2.h> > >> #include <Guid/FirmwareFileSystem3.h> > >> @@ -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 *gGcdMemorySpaceNotify; > >> + > >> // > >> // 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 DXE CIS. > >> # > >> # Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR> > >> +# Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > >> +# > >> # 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.<BR> > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > >> + > >> 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 > >> @@ -42,6 +44,11 @@ EFI_GUID *gDxeCoreFileName; > >> EFI_LOADED_IMAGE_PROTOCOL *gDxeCoreLoadedImage; > >> > >> // > >> +// DXE Core global for GCD notification protocol > >> +// > >> +EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL *gGcdMemorySpaceNotify = NULL; > >> + > >> +// > >> // DXE Core Module Variables > >> // > >> EFI_BOOT_SERVICES mBootServices = { > >> 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.<BR> > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > >> + > >> 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 > >> @@ -45,9 +47,10 @@ EFI_CORE_PROTOCOL_NOTIFY_ENTRY mArchProtocols[] = { > >> // Optional protocols that the DXE Core will use if they are present > >> // > >> EFI_CORE_PROTOCOL_NOTIFY_ENTRY mOptionalProtocols[] = { > >> - { &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 **)&gGcdMemorySpaceNotify, > >> NULL, NULL, FALSE }, > >> + { NULL, (VOID **)NULL, > >> NULL, NULL, FALSE } > >> }; > >> > >> // > >> diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/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.<BR> > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > >> + > >> 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 > >> @@ -896,6 +898,9 @@ CoreConvertSpace ( > >> } else { > >> Entry->Capabilities = Capabilities | EFI_MEMORY_RUNTIME; > >> } > >> + if (gGcdMemorySpaceNotify) { > >> + gGcdMemorySpaceNotify->MemorySpaceAddNotify (GcdMemoryType, > BaseAddress, > >> Length, Entry->Capabilities); > >> + } > >> break; > >> case GCD_ADD_IO_OPERATION: > >> Entry->GcdIoType = GcdIoType; > >> @@ -914,6 +919,9 @@ CoreConvertSpace ( > >> case GCD_REMOVE_MEMORY_OPERATION: > >> Entry->GcdMemoryType = EfiGcdMemoryTypeNonExistent; > >> Entry->Capabilities = 0; > >> + if (gGcdMemorySpaceNotify) { > >> + gGcdMemorySpaceNotify->MemorySpaceRemoveNotify (BaseAddress, Length); > >> + } > >> break; > >> case GCD_REMOVE_IO_OPERATION: > >> Entry->GcdIoType = 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/REMOVE > >> operations. > >> + > >> + Copyright (c) 2017, AMD Inc. All rights reserved.<BR> > >> + > >> + 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 > >> + http://opensource.org/licenses/bsd-license.php > >> + > >> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > >> + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR > 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 = { 0x0d51905b, 0xb77e, 0x452a, > {0xa2, > >> 0xc0, 0xec, 0xa0, 0xcc, 0x8d, 0x51, 0x4a } } > >> > >> + ## Include/Protocol/GcdMemorySpaceNotify.h > >> + gEfiGcdMemorySpaceNotifyProtocolGuid = { 0xc842db69, 0x610e, 0x401a, > {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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL 2017-04-13 0:58 ` Kinney, Michael D @ 2017-04-13 2:13 ` Zeng, Star 2017-04-13 2:35 ` Duran, Leo 2017-04-13 2:27 ` Duran, Leo 1 sibling, 1 reply; 13+ messages in thread From: Zeng, Star @ 2017-04-13 2:13 UTC (permalink / raw) To: Leo Duran, Laszlo Ersek, Kinney, Michael D, edk2-devel@ml01.01.org Cc: Tian, Feng, Brijesh Singh, Yao, Jiewen, Zeng, Star Does the solution Jiewen proposed at https://lists.01.org/pipermail/edk2-devel/2017-March/008987.html also work for this case similarly? Thanks, Star -----Original Message----- From: Kinney, Michael D Sent: Thursday, April 13, 2017 8:59 AM To: Laszlo Ersek <lersek@redhat.com>; Leo Duran <leo.duran@amd.com>; edk2-devel@ml01.01.org; Kinney, Michael D <michael.d.kinney@intel.com> Cc: Tian, Feng <feng.tian@intel.com>; Brijesh Singh <brijesh.singh@amd.com>; Zeng, Star <star.zeng@intel.com> Subject: RE: [edk2] [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL Hi Laszlo and Leo, If you look at the CpuSetMemoryAttributes() function in UefiCpuPkg/CpuDxe/CpuDxe.c, you will see it now calls AssignMemoryPageAttributes() at the end. This function 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 <michael.d.kinney@intel.com>; Leo Duran > <leo.duran@amd.com>; edk2-devel@ml01.01.org > Cc: Tian, Feng <feng.tian@intel.com>; Brijesh Singh > <brijesh.singh@amd.com>; Zeng, Star <star.zeng@intel.com> > Subject: Re: [edk2] [PATCH] MdeModulePkg: Add > EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL > > Mike, > > On 04/12/17 21:39, Kinney, Michael D wrote: > > Hi Leo, > > > > DXE Main is supposed to use Arch Protocols for platform specific behavior. > > > > In this case, can the CPU Arch Protocol SetMemoryAttributes() > > function be used to customize behavior? > > > > In other modules that add/remove/modify MMIO, > > gDS->SetMemorySpaceAttributes() 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 > > gDS-> perform > > extra actions? > > 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). > > 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. > > What would be the best general location or API to hook the "C" bit > manipulation into MMIO GCD memory space addition / removal? > > Thank you, > Laszlo > > > > > > 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 <lersek@redhat.com>; Tian, Feng > >> <feng.tian@intel.com>; Leo > Duran > >> <leo.duran@amd.com>; Brijesh Singh <brijesh.singh@amd.com>; Zeng, > >> Star <star.zeng@intel.com> > >> 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 > >> handled by DxeMain, and as such leverages the existing DxeProtocolNotify framework. > >> > >> Cc: Feng Tian <feng.tian@intel.com> > >> Cc: Star Zeng <star.zeng@intel.com> > >> Cc: Laszlo Ersek <lersek@redhat.com> > >> Cc: Brijesh Singh <brijesh.singh@amd.com> > >> Contributed-under: TianoCore Contribution Agreement 1.0 > >> Signed-off-by: Leo Duran <leo.duran@amd.com> > >> --- > >> 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/DxeMain.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.<BR> > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > >> + > >> 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 > >> @@ -17,7 +19,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY > >> KIND, EITHER EXPRESS OR IMPLIED. > >> #define _DXE_MAIN_H_ > >> > >> > >> - > >> #include <PiDxe.h> > >> > >> #include <Protocol/LoadedImage.h> > >> @@ -53,6 +54,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY > >> KIND, EITHER EXPRESS OR IMPLIED. > >> #include <Protocol/TcgService.h> > >> #include <Protocol/HiiPackageList.h> #include > >> <Protocol/SmmBase2.h> > >> +#include <Protocol/GcdMemorySpaceNotify.h> > >> + > >> #include <Guid/MemoryTypeInformation.h> #include > >> <Guid/FirmwareFileSystem2.h> #include <Guid/FirmwareFileSystem3.h> > >> @@ -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 *gGcdMemorySpaceNotify; > >> + > >> // > >> // 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 DXE CIS. > >> # > >> # Copyright (c) 2006 - 2017, Intel Corporation. All rights > >> reserved.<BR> > >> +# Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > >> +# > >> # 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.<BR> > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > >> + > >> 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 > >> @@ -42,6 +44,11 @@ EFI_GUID *gDxeCoreFileName; > >> EFI_LOADED_IMAGE_PROTOCOL *gDxeCoreLoadedImage; > >> > >> // > >> +// DXE Core global for GCD notification protocol // > >> +EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL *gGcdMemorySpaceNotify = > >> +NULL; > >> + > >> +// > >> // DXE Core Module Variables > >> // > >> EFI_BOOT_SERVICES mBootServices = { 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.<BR> > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > >> + > >> 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 > >> @@ -45,9 +47,10 @@ EFI_CORE_PROTOCOL_NOTIFY_ENTRY mArchProtocols[] > >> = { // Optional protocols that the DXE Core will use if they are > >> present // EFI_CORE_PROTOCOL_NOTIFY_ENTRY mOptionalProtocols[] = > >> { > >> - { &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 > >> + **)&gGcdMemorySpaceNotify, > >> NULL, NULL, FALSE }, > >> + { NULL, (VOID **)NULL, > >> NULL, NULL, FALSE } > >> }; > >> > >> // > >> diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > >> b/MdeModulePkg/Core/Dxe/Gcd/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.<BR> > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > >> + > >> 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 > >> @@ -896,6 +898,9 @@ CoreConvertSpace ( > >> } else { > >> Entry->Capabilities = Capabilities | EFI_MEMORY_RUNTIME; > >> } > >> + if (gGcdMemorySpaceNotify) { > >> + gGcdMemorySpaceNotify->MemorySpaceAddNotify > >> + (GcdMemoryType, > BaseAddress, > >> Length, Entry->Capabilities); > >> + } > >> break; > >> case GCD_ADD_IO_OPERATION: > >> Entry->GcdIoType = GcdIoType; @@ -914,6 +919,9 @@ > >> CoreConvertSpace ( > >> case GCD_REMOVE_MEMORY_OPERATION: > >> Entry->GcdMemoryType = EfiGcdMemoryTypeNonExistent; > >> Entry->Capabilities = 0; > >> + if (gGcdMemorySpaceNotify) { > >> + gGcdMemorySpaceNotify->MemorySpaceRemoveNotify (BaseAddress, Length); > >> + } > >> break; > >> case GCD_REMOVE_IO_OPERATION: > >> Entry->GcdIoType = 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/REMOVE > >> operations. > >> + > >> + Copyright (c) 2017, AMD Inc. All rights reserved.<BR> > >> + > >> + 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 > >> + http://opensource.org/licenses/bsd-license.php > >> + > >> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" > >> + BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER > >> + EXPRESS OR > 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 = { 0x0d51905b, 0xb77e, > >> 0x452a, > {0xa2, > >> 0xc0, 0xec, 0xa0, 0xcc, 0x8d, 0x51, 0x4a } } > >> > >> + ## Include/Protocol/GcdMemorySpaceNotify.h > >> + gEfiGcdMemorySpaceNotifyProtocolGuid = { 0xc842db69, 0x610e, > >> + 0x401a, > {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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL 2017-04-13 2:13 ` Zeng, Star @ 2017-04-13 2:35 ` Duran, Leo 2017-04-13 2:40 ` Yao, Jiewen 0 siblings, 1 reply; 13+ messages in thread From: Duran, Leo @ 2017-04-13 2:35 UTC (permalink / raw) To: Zeng, Star, Laszlo Ersek, Kinney, Michael D, edk2-devel@ml01.01.org Cc: Tian, Feng, Singh, Brijesh, Yao, Jiewen Hi Star, At DxeIpl time we only have the MMIO regions published by descriptor HOB's built during PEI. But we also need to capture MMIO regions that may be 'added' by DXE drivers (e.g., PciHostBridgeDxe), ideally without making changes to those drivers. So our thinking is that trapping the 'Add' MMIO requests that come into GCD would be a good place to capture what we need. Thanks, Leo. > -----Original Message----- > From: Zeng, Star [mailto:star.zeng@intel.com] > Sent: Wednesday, April 12, 2017 9:14 PM > To: Duran, Leo <leo.duran@amd.com>; Laszlo Ersek <lersek@redhat.com>; > Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@ml01.01.org > Cc: Tian, Feng <feng.tian@intel.com>; Singh, Brijesh > <brijesh.singh@amd.com>; Yao, Jiewen <jiewen.yao@intel.com>; Zeng, Star > <star.zeng@intel.com> > Subject: RE: [edk2] [PATCH] MdeModulePkg: Add > EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL > > Does the solution Jiewen proposed at https://lists.01.org/pipermail/edk2- > devel/2017-March/008987.html also work for this case similarly? > > Thanks, > Star > -----Original Message----- > From: Kinney, Michael D > Sent: Thursday, April 13, 2017 8:59 AM > To: Laszlo Ersek <lersek@redhat.com>; Leo Duran <leo.duran@amd.com>; > edk2-devel@ml01.01.org; Kinney, Michael D <michael.d.kinney@intel.com> > Cc: Tian, Feng <feng.tian@intel.com>; Brijesh Singh > <brijesh.singh@amd.com>; Zeng, Star <star.zeng@intel.com> > Subject: RE: [edk2] [PATCH] MdeModulePkg: Add > EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL > > Hi Laszlo and Leo, > > If you look at the CpuSetMemoryAttributes() function in > UefiCpuPkg/CpuDxe/CpuDxe.c, you will see it now calls > AssignMemoryPageAttributes() at the end. This function 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 <michael.d.kinney@intel.com>; Leo Duran > > <leo.duran@amd.com>; edk2-devel@ml01.01.org > > Cc: Tian, Feng <feng.tian@intel.com>; Brijesh Singh > > <brijesh.singh@amd.com>; Zeng, Star <star.zeng@intel.com> > > Subject: Re: [edk2] [PATCH] MdeModulePkg: Add > > EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL > > > > Mike, > > > > On 04/12/17 21:39, Kinney, Michael D wrote: > > > Hi Leo, > > > > > > DXE Main is supposed to use Arch Protocols for platform specific > behavior. > > > > > > In this case, can the CPU Arch Protocol SetMemoryAttributes() > > > function be used to customize behavior? > > > > > > In other modules that add/remove/modify MMIO, > > > gDS->SetMemorySpaceAttributes() 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 > > > gDS-> perform > > > extra actions? > > > > 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). > > > > 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. > > > > What would be the best general location or API to hook the "C" bit > > manipulation into MMIO GCD memory space addition / removal? > > > > Thank you, > > Laszlo > > > > > > > > > > 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 <lersek@redhat.com>; Tian, Feng > > >> <feng.tian@intel.com>; Leo > > Duran > > >> <leo.duran@amd.com>; Brijesh Singh <brijesh.singh@amd.com>; Zeng, > > >> Star <star.zeng@intel.com> > > >> 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 > > >> handled by DxeMain, and as such leverages the existing > DxeProtocolNotify framework. > > >> > > >> Cc: Feng Tian <feng.tian@intel.com> > > >> Cc: Star Zeng <star.zeng@intel.com> > > >> Cc: Laszlo Ersek <lersek@redhat.com> > > >> Cc: Brijesh Singh <brijesh.singh@amd.com> > > >> Contributed-under: TianoCore Contribution Agreement 1.0 > > >> Signed-off-by: Leo Duran <leo.duran@amd.com> > > >> --- > > >> 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/DxeMain.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.<BR> > > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> + > > >> 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 > > >> @@ -17,7 +19,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF > ANY > > >> KIND, EITHER EXPRESS OR IMPLIED. > > >> #define _DXE_MAIN_H_ > > >> > > >> > > >> - > > >> #include <PiDxe.h> > > >> > > >> #include <Protocol/LoadedImage.h> > > >> @@ -53,6 +54,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF > ANY > > >> KIND, EITHER EXPRESS OR IMPLIED. > > >> #include <Protocol/TcgService.h> > > >> #include <Protocol/HiiPackageList.h> #include > > >> <Protocol/SmmBase2.h> > > >> +#include <Protocol/GcdMemorySpaceNotify.h> > > >> + > > >> #include <Guid/MemoryTypeInformation.h> #include > > >> <Guid/FirmwareFileSystem2.h> #include > <Guid/FirmwareFileSystem3.h> > > >> @@ -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 > *gGcdMemorySpaceNotify; > > >> + > > >> // > > >> // 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 DXE > CIS. > > >> # > > >> # Copyright (c) 2006 - 2017, Intel Corporation. All rights > > >> reserved.<BR> > > >> +# Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> +# > > >> # 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.<BR> > > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> + > > >> 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 > > >> @@ -42,6 +44,11 @@ EFI_GUID *gDxeCoreFileName; > > >> EFI_LOADED_IMAGE_PROTOCOL *gDxeCoreLoadedImage; > > >> > > >> // > > >> +// DXE Core global for GCD notification protocol // > > >> +EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL > *gGcdMemorySpaceNotify = > > >> +NULL; > > >> + > > >> +// > > >> // DXE Core Module Variables > > >> // > > >> EFI_BOOT_SERVICES mBootServices = { 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.<BR> > > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> + > > >> 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 > > >> @@ -45,9 +47,10 @@ EFI_CORE_PROTOCOL_NOTIFY_ENTRY > mArchProtocols[] > > >> = { // Optional protocols that the DXE Core will use if they are > > >> present // EFI_CORE_PROTOCOL_NOTIFY_ENTRY > mOptionalProtocols[] = > > >> { > > >> - { &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 > > >> + **)&gGcdMemorySpaceNotify, > > >> NULL, NULL, FALSE }, > > >> + { NULL, (VOID **)NULL, > > >> NULL, NULL, FALSE } > > >> }; > > >> > > >> // > > >> diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > > >> b/MdeModulePkg/Core/Dxe/Gcd/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.<BR> > > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> + > > >> 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 > > >> @@ -896,6 +898,9 @@ CoreConvertSpace ( > > >> } else { > > >> Entry->Capabilities = Capabilities | EFI_MEMORY_RUNTIME; > > >> } > > >> + if (gGcdMemorySpaceNotify) { > > >> + gGcdMemorySpaceNotify->MemorySpaceAddNotify > > >> + (GcdMemoryType, > > BaseAddress, > > >> Length, Entry->Capabilities); > > >> + } > > >> break; > > >> case GCD_ADD_IO_OPERATION: > > >> Entry->GcdIoType = GcdIoType; @@ -914,6 +919,9 @@ > > >> CoreConvertSpace ( > > >> case GCD_REMOVE_MEMORY_OPERATION: > > >> Entry->GcdMemoryType = EfiGcdMemoryTypeNonExistent; > > >> Entry->Capabilities = 0; > > >> + if (gGcdMemorySpaceNotify) { > > >> + gGcdMemorySpaceNotify->MemorySpaceRemoveNotify > (BaseAddress, Length); > > >> + } > > >> break; > > >> case GCD_REMOVE_IO_OPERATION: > > >> Entry->GcdIoType = 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/REMOVE > > >> operations. > > >> + > > >> + Copyright (c) 2017, AMD Inc. All rights reserved.<BR> > > >> + > > >> + 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 > > >> + http://opensource.org/licenses/bsd-license.php > > >> + > > >> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS > IS" > > >> + BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, > EITHER > > >> + EXPRESS OR > > 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 = { 0x0d51905b, 0xb77e, > > >> 0x452a, > > {0xa2, > > >> 0xc0, 0xec, 0xa0, 0xcc, 0x8d, 0x51, 0x4a } } > > >> > > >> + ## Include/Protocol/GcdMemorySpaceNotify.h > > >> + gEfiGcdMemorySpaceNotifyProtocolGuid = { 0xc842db69, 0x610e, > > >> + 0x401a, > > {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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL 2017-04-13 2:35 ` Duran, Leo @ 2017-04-13 2:40 ` Yao, Jiewen 2017-04-13 2:44 ` Duran, Leo 0 siblings, 1 reply; 13+ messages in thread From: Yao, Jiewen @ 2017-04-13 2:40 UTC (permalink / raw) To: Duran, Leo, Zeng, Star, Laszlo Ersek, Kinney, Michael D, edk2-devel@ml01.01.org Cc: Tian, Feng, Singh, Brijesh, Yao, Jiewen Hi Leo Can we clear SEC for all non-DRAM region, instead of MMIO region? Then we do not need worried about 'ADD' MMIO. Thank you Yao Jiewen From: Duran, Leo [mailto:leo.duran@amd.com] Sent: Thursday, April 13, 2017 10:35 AM To: Zeng, Star <star.zeng@intel.com>; Laszlo Ersek <lersek@redhat.com>; Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@ml01.01.org Cc: Tian, Feng <feng.tian@intel.com>; Singh, Brijesh <brijesh.singh@amd.com>; Yao, Jiewen <jiewen.yao@intel.com> Subject: RE: [edk2] [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL Hi Star, At DxeIpl time we only have the MMIO regions published by descriptor HOB's built during PEI. But we also need to capture MMIO regions that may be 'added' by DXE drivers (e.g., PciHostBridgeDxe), ideally without making changes to those drivers. So our thinking is that trapping the 'Add' MMIO requests that come into GCD would be a good place to capture what we need. Thanks, Leo. > -----Original Message----- > From: Zeng, Star [mailto:star.zeng@intel.com] > Sent: Wednesday, April 12, 2017 9:14 PM > To: Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; > Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> > Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; Singh, Brijesh > <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Zeng, Star > <star.zeng@intel.com<mailto:star.zeng@intel.com>> > Subject: RE: [edk2] [PATCH] MdeModulePkg: Add > EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL > > Does the solution Jiewen proposed at https://lists.01.org/pipermail/edk2- > devel/2017-March/008987.html also work for this case similarly? > > Thanks, > Star > -----Original Message----- > From: Kinney, Michael D > Sent: Thursday, April 13, 2017 8:59 AM > To: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com>>; > edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>> > Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; Brijesh Singh > <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>> > Subject: RE: [edk2] [PATCH] MdeModulePkg: Add > EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL > > Hi Laszlo and Leo, > > If you look at the CpuSetMemoryAttributes() function in > UefiCpuPkg/CpuDxe/CpuDxe.c, you will see it now calls > AssignMemoryPageAttributes() at the end. This function 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 <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Leo Duran > > <leo.duran@amd.com<mailto:leo.duran@amd.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> > > Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; Brijesh Singh > > <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>> > > Subject: Re: [edk2] [PATCH] MdeModulePkg: Add > > EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL > > > > Mike, > > > > On 04/12/17 21:39, Kinney, Michael D wrote: > > > Hi Leo, > > > > > > DXE Main is supposed to use Arch Protocols for platform specific > behavior. > > > > > > In this case, can the CPU Arch Protocol SetMemoryAttributes() > > > function be used to customize behavior? > > > > > > In other modules that add/remove/modify MMIO, > > > gDS->SetMemorySpaceAttributes() 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 > > > gDS-> perform > > > extra actions? > > > > 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). > > > > 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. > > > > What would be the best general location or API to hook the "C" bit > > manipulation into MMIO GCD memory space addition / removal? > > > > Thank you, > > Laszlo > > > > > > > > > > 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<mailto:edk2-devel@ml01.01.org> > > >> Cc: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Tian, Feng > > >> <feng.tian@intel.com<mailto:feng.tian@intel.com>>; Leo > > Duran > > >> <leo.duran@amd.com<mailto:leo.duran@amd.com>>; Brijesh Singh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>>; Zeng, > > >> Star <star.zeng@intel.com<mailto:star.zeng@intel.com>> > > >> 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 > > >> handled by DxeMain, and as such leverages the existing > DxeProtocolNotify framework. > > >> > > >> Cc: Feng Tian <feng.tian@intel.com<mailto:feng.tian@intel.com>> > > >> Cc: Star Zeng <star.zeng@intel.com<mailto:star.zeng@intel.com>> > > >> Cc: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>> > > >> Cc: Brijesh Singh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>> > > >> Contributed-under: TianoCore Contribution Agreement 1.0 > > >> Signed-off-by: Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com>> > > >> --- > > >> 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/DxeMain.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.<BR> > > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> + > > >> 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 > > >> @@ -17,7 +19,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF > ANY > > >> KIND, EITHER EXPRESS OR IMPLIED. > > >> #define _DXE_MAIN_H_ > > >> > > >> > > >> - > > >> #include <PiDxe.h> > > >> > > >> #include <Protocol/LoadedImage.h> > > >> @@ -53,6 +54,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF > ANY > > >> KIND, EITHER EXPRESS OR IMPLIED. > > >> #include <Protocol/TcgService.h> > > >> #include <Protocol/HiiPackageList.h> #include > > >> <Protocol/SmmBase2.h> > > >> +#include <Protocol/GcdMemorySpaceNotify.h> > > >> + > > >> #include <Guid/MemoryTypeInformation.h> #include > > >> <Guid/FirmwareFileSystem2.h> #include > <Guid/FirmwareFileSystem3.h> > > >> @@ -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 > *gGcdMemorySpaceNotify; > > >> + > > >> // > > >> // 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 DXE > CIS. > > >> # > > >> # Copyright (c) 2006 - 2017, Intel Corporation. All rights > > >> reserved.<BR> > > >> +# Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> +# > > >> # 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.<BR> > > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> + > > >> 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 > > >> @@ -42,6 +44,11 @@ EFI_GUID *gDxeCoreFileName; > > >> EFI_LOADED_IMAGE_PROTOCOL *gDxeCoreLoadedImage; > > >> > > >> // > > >> +// DXE Core global for GCD notification protocol // > > >> +EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL > *gGcdMemorySpaceNotify = > > >> +NULL; > > >> + > > >> +// > > >> // DXE Core Module Variables > > >> // > > >> EFI_BOOT_SERVICES mBootServices = { 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.<BR> > > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> + > > >> 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 > > >> @@ -45,9 +47,10 @@ EFI_CORE_PROTOCOL_NOTIFY_ENTRY > mArchProtocols[] > > >> = { // Optional protocols that the DXE Core will use if they are > > >> present // EFI_CORE_PROTOCOL_NOTIFY_ENTRY > mOptionalProtocols[] = > > >> { > > >> - { &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 > > >> + **)&gGcdMemorySpaceNotify, > > >> NULL, NULL, FALSE }, > > >> + { NULL, (VOID **)NULL, > > >> NULL, NULL, FALSE } > > >> }; > > >> > > >> // > > >> diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > > >> b/MdeModulePkg/Core/Dxe/Gcd/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.<BR> > > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> + > > >> 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 > > >> @@ -896,6 +898,9 @@ CoreConvertSpace ( > > >> } else { > > >> Entry->Capabilities = Capabilities | EFI_MEMORY_RUNTIME; > > >> } > > >> + if (gGcdMemorySpaceNotify) { > > >> + gGcdMemorySpaceNotify->MemorySpaceAddNotify > > >> + (GcdMemoryType, > > BaseAddress, > > >> Length, Entry->Capabilities); > > >> + } > > >> break; > > >> case GCD_ADD_IO_OPERATION: > > >> Entry->GcdIoType = GcdIoType; @@ -914,6 +919,9 @@ > > >> CoreConvertSpace ( > > >> case GCD_REMOVE_MEMORY_OPERATION: > > >> Entry->GcdMemoryType = EfiGcdMemoryTypeNonExistent; > > >> Entry->Capabilities = 0; > > >> + if (gGcdMemorySpaceNotify) { > > >> + gGcdMemorySpaceNotify->MemorySpaceRemoveNotify > (BaseAddress, Length); > > >> + } > > >> break; > > >> case GCD_REMOVE_IO_OPERATION: > > >> Entry->GcdIoType = 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/REMOVE > > >> operations. > > >> + > > >> + Copyright (c) 2017, AMD Inc. All rights reserved.<BR> > > >> + > > >> + 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 > > >> + http://opensource.org/licenses/bsd-license.php > > >> + > > >> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS > IS" > > >> + BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, > EITHER > > >> + EXPRESS OR > > 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 = { 0x0d51905b, 0xb77e, > > >> 0x452a, > > {0xa2, > > >> 0xc0, 0xec, 0xa0, 0xcc, 0x8d, 0x51, 0x4a } } > > >> > > >> + ## Include/Protocol/GcdMemorySpaceNotify.h > > >> + gEfiGcdMemorySpaceNotifyProtocolGuid = { 0xc842db69, 0x610e, > > >> + 0x401a, > > {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<mailto:edk2-devel@lists.01.org> > > >> https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL 2017-04-13 2:40 ` Yao, Jiewen @ 2017-04-13 2:44 ` Duran, Leo 2017-04-13 3:05 ` Yao, Jiewen 0 siblings, 1 reply; 13+ messages in thread From: Duran, Leo @ 2017-04-13 2:44 UTC (permalink / raw) To: Yao, Jiewen, Zeng, Star, Laszlo Ersek, Kinney, Michael D, edk2-devel@ml01.01.org Cc: Tian, Feng, Singh, Brijesh From: Yao, Jiewen [mailto:jiewen.yao@intel.com] Sent: Wednesday, April 12, 2017 9:40 PM To: Duran, Leo <leo.duran@amd.com>; Zeng, Star <star.zeng@intel.com>; Laszlo Ersek <lersek@redhat.com>; Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@ml01.01.org Cc: Tian, Feng <feng.tian@intel.com>; Singh, Brijesh <brijesh.singh@amd.com>; Yao, Jiewen <jiewen.yao@intel.com> Subject: RE: [edk2] [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL Hi Leo Can we clear SEC for all non-DRAM region, instead of MMIO region? Then we do not need worried about 'ADD' MMIO. [Duran, Leo] Ummh... Let me check with Brijesh. I guess you're suggesting a one-time deal, done once page tables are created. Thank you Yao Jiewen From: Duran, Leo [mailto:leo.duran@amd.com] Sent: Thursday, April 13, 2017 10:35 AM To: Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; Singh, Brijesh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> Subject: RE: [edk2] [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL Hi Star, At DxeIpl time we only have the MMIO regions published by descriptor HOB's built during PEI. But we also need to capture MMIO regions that may be 'added' by DXE drivers (e.g., PciHostBridgeDxe), ideally without making changes to those drivers. So our thinking is that trapping the 'Add' MMIO requests that come into GCD would be a good place to capture what we need. Thanks, Leo. > -----Original Message----- > From: Zeng, Star [mailto:star.zeng@intel.com] > Sent: Wednesday, April 12, 2017 9:14 PM > To: Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; > Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> > Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; Singh, Brijesh > <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Zeng, Star > <star.zeng@intel.com<mailto:star.zeng@intel.com>> > Subject: RE: [edk2] [PATCH] MdeModulePkg: Add > EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL > > Does the solution Jiewen proposed at https://lists.01.org/pipermail/edk2- > devel/2017-March/008987.html also work for this case similarly? > > Thanks, > Star > -----Original Message----- > From: Kinney, Michael D > Sent: Thursday, April 13, 2017 8:59 AM > To: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com>>; > edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>> > Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; Brijesh Singh > <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>> > Subject: RE: [edk2] [PATCH] MdeModulePkg: Add > EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL > > Hi Laszlo and Leo, > > If you look at the CpuSetMemoryAttributes() function in > UefiCpuPkg/CpuDxe/CpuDxe.c, you will see it now calls > AssignMemoryPageAttributes() at the end. This function 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 <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Leo Duran > > <leo.duran@amd.com<mailto:leo.duran@amd.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> > > Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; Brijesh Singh > > <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>> > > Subject: Re: [edk2] [PATCH] MdeModulePkg: Add > > EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL > > > > Mike, > > > > On 04/12/17 21:39, Kinney, Michael D wrote: > > > Hi Leo, > > > > > > DXE Main is supposed to use Arch Protocols for platform specific > behavior. > > > > > > In this case, can the CPU Arch Protocol SetMemoryAttributes() > > > function be used to customize behavior? > > > > > > In other modules that add/remove/modify MMIO, > > > gDS->SetMemorySpaceAttributes() 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 > > > gDS-> perform > > > extra actions? > > > > 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). > > > > 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. > > > > What would be the best general location or API to hook the "C" bit > > manipulation into MMIO GCD memory space addition / removal? > > > > Thank you, > > Laszlo > > > > > > > > > > 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<mailto:edk2-devel@ml01.01.org> > > >> Cc: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Tian, Feng > > >> <feng.tian@intel.com<mailto:feng.tian@intel.com>>; Leo > > Duran > > >> <leo.duran@amd.com<mailto:leo.duran@amd.com>>; Brijesh Singh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>>; Zeng, > > >> Star <star.zeng@intel.com<mailto:star.zeng@intel.com>> > > >> 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 > > >> handled by DxeMain, and as such leverages the existing > DxeProtocolNotify framework. > > >> > > >> Cc: Feng Tian <feng.tian@intel.com<mailto:feng.tian@intel.com>> > > >> Cc: Star Zeng <star.zeng@intel.com<mailto:star.zeng@intel.com>> > > >> Cc: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>> > > >> Cc: Brijesh Singh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>> > > >> Contributed-under: TianoCore Contribution Agreement 1.0 > > >> Signed-off-by: Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com>> > > >> --- > > >> 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/DxeMain.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.<BR> > > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> + > > >> 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 > > >> @@ -17,7 +19,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF > ANY > > >> KIND, EITHER EXPRESS OR IMPLIED. > > >> #define _DXE_MAIN_H_ > > >> > > >> > > >> - > > >> #include <PiDxe.h> > > >> > > >> #include <Protocol/LoadedImage.h> > > >> @@ -53,6 +54,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF > ANY > > >> KIND, EITHER EXPRESS OR IMPLIED. > > >> #include <Protocol/TcgService.h> > > >> #include <Protocol/HiiPackageList.h> #include > > >> <Protocol/SmmBase2.h> > > >> +#include <Protocol/GcdMemorySpaceNotify.h> > > >> + > > >> #include <Guid/MemoryTypeInformation.h> #include > > >> <Guid/FirmwareFileSystem2.h> #include > <Guid/FirmwareFileSystem3.h> > > >> @@ -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 > *gGcdMemorySpaceNotify; > > >> + > > >> // > > >> // 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 DXE > CIS. > > >> # > > >> # Copyright (c) 2006 - 2017, Intel Corporation. All rights > > >> reserved.<BR> > > >> +# Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> +# > > >> # 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.<BR> > > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> + > > >> 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 > > >> @@ -42,6 +44,11 @@ EFI_GUID *gDxeCoreFileName; > > >> EFI_LOADED_IMAGE_PROTOCOL *gDxeCoreLoadedImage; > > >> > > >> // > > >> +// DXE Core global for GCD notification protocol // > > >> +EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL > *gGcdMemorySpaceNotify = > > >> +NULL; > > >> + > > >> +// > > >> // DXE Core Module Variables > > >> // > > >> EFI_BOOT_SERVICES mBootServices = { 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.<BR> > > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> + > > >> 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 > > >> @@ -45,9 +47,10 @@ EFI_CORE_PROTOCOL_NOTIFY_ENTRY > mArchProtocols[] > > >> = { // Optional protocols that the DXE Core will use if they are > > >> present // EFI_CORE_PROTOCOL_NOTIFY_ENTRY > mOptionalProtocols[] = > > >> { > > >> - { &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 > > >> + **)&gGcdMemorySpaceNotify, > > >> NULL, NULL, FALSE }, > > >> + { NULL, (VOID **)NULL, > > >> NULL, NULL, FALSE } > > >> }; > > >> > > >> // > > >> diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > > >> b/MdeModulePkg/Core/Dxe/Gcd/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.<BR> > > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> + > > >> 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 > > >> @@ -896,6 +898,9 @@ CoreConvertSpace ( > > >> } else { > > >> Entry->Capabilities = Capabilities | EFI_MEMORY_RUNTIME; > > >> } > > >> + if (gGcdMemorySpaceNotify) { > > >> + gGcdMemorySpaceNotify->MemorySpaceAddNotify > > >> + (GcdMemoryType, > > BaseAddress, > > >> Length, Entry->Capabilities); > > >> + } > > >> break; > > >> case GCD_ADD_IO_OPERATION: > > >> Entry->GcdIoType = GcdIoType; @@ -914,6 +919,9 @@ > > >> CoreConvertSpace ( > > >> case GCD_REMOVE_MEMORY_OPERATION: > > >> Entry->GcdMemoryType = EfiGcdMemoryTypeNonExistent; > > >> Entry->Capabilities = 0; > > >> + if (gGcdMemorySpaceNotify) { > > >> + gGcdMemorySpaceNotify->MemorySpaceRemoveNotify > (BaseAddress, Length); > > >> + } > > >> break; > > >> case GCD_REMOVE_IO_OPERATION: > > >> Entry->GcdIoType = 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/REMOVE > > >> operations. > > >> + > > >> + Copyright (c) 2017, AMD Inc. All rights reserved.<BR> > > >> + > > >> + 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 > > >> + http://opensource.org/licenses/bsd-license.php > > >> + > > >> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS > IS" > > >> + BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, > EITHER > > >> + EXPRESS OR > > 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 = { 0x0d51905b, 0xb77e, > > >> 0x452a, > > {0xa2, > > >> 0xc0, 0xec, 0xa0, 0xcc, 0x8d, 0x51, 0x4a } } > > >> > > >> + ## Include/Protocol/GcdMemorySpaceNotify.h > > >> + gEfiGcdMemorySpaceNotifyProtocolGuid = { 0xc842db69, 0x610e, > > >> + 0x401a, > > {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<mailto:edk2-devel@lists.01.org> > > >> https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL 2017-04-13 2:44 ` Duran, Leo @ 2017-04-13 3:05 ` Yao, Jiewen 2017-04-14 14:10 ` Duran, Leo 0 siblings, 1 reply; 13+ messages in thread From: Yao, Jiewen @ 2017-04-13 3:05 UTC (permalink / raw) To: Duran, Leo, Zeng, Star, Laszlo Ersek, Kinney, Michael D, edk2-devel@ml01.01.org Cc: Tian, Feng, Singh, Brijesh, Yao, Jiewen Yes, exactly. I hope the default path be as simple as possible. Then we only need update the page table in the corner case. Thank you Yao Jiewen From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Duran, Leo Sent: Thursday, April 13, 2017 10:44 AM To: Yao, Jiewen <jiewen.yao@intel.com>; Zeng, Star <star.zeng@intel.com>; Laszlo Ersek <lersek@redhat.com>; Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@ml01.01.org Cc: Tian, Feng <feng.tian@intel.com>; Singh, Brijesh <brijesh.singh@amd.com> Subject: Re: [edk2] [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL From: Yao, Jiewen [mailto:jiewen.yao@intel.com] Sent: Wednesday, April 12, 2017 9:40 PM To: Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; Singh, Brijesh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> Subject: RE: [edk2] [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL Hi Leo Can we clear SEC for all non-DRAM region, instead of MMIO region? Then we do not need worried about 'ADD' MMIO. [Duran, Leo] Ummh... Let me check with Brijesh. I guess you're suggesting a one-time deal, done once page tables are created. Thank you Yao Jiewen From: Duran, Leo [mailto:leo.duran@amd.com] Sent: Thursday, April 13, 2017 10:35 AM To: Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com<mailto:star.zeng@intel.com%3cmailto:star.zeng@intel.com>>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3cmailto:lersek@redhat.com>>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>> Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com<mailto:feng.tian@intel.com%3cmailto:feng.tian@intel.com>>>; Singh, Brijesh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com<mailto:brijesh.singh@amd.com%3cmailto:brijesh.singh@amd.com>>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>> Subject: RE: [edk2] [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL Hi Star, At DxeIpl time we only have the MMIO regions published by descriptor HOB's built during PEI. But we also need to capture MMIO regions that may be 'added' by DXE drivers (e.g., PciHostBridgeDxe), ideally without making changes to those drivers. So our thinking is that trapping the 'Add' MMIO requests that come into GCD would be a good place to capture what we need. Thanks, Leo. > -----Original Message----- > From: Zeng, Star [mailto:star.zeng@intel.com] > Sent: Wednesday, April 12, 2017 9:14 PM > To: Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com>>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3cmailto:lersek@redhat.com>>>; > Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>> > Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com<mailto:feng.tian@intel.com%3cmailto:feng.tian@intel.com>>>; Singh, Brijesh > <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com<mailto:brijesh.singh@amd.com%3cmailto:brijesh.singh@amd.com>>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>>; Zeng, Star > <star.zeng@intel.com<mailto:star.zeng@intel.com<mailto:star.zeng@intel.com%3cmailto:star.zeng@intel.com>>> > Subject: RE: [edk2] [PATCH] MdeModulePkg: Add > EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL > > Does the solution Jiewen proposed at https://lists.01.org/pipermail/edk2- > devel/2017-March/008987.html also work for this case similarly? > > Thanks, > Star > -----Original Message----- > From: Kinney, Michael D > Sent: Thursday, April 13, 2017 8:59 AM > To: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3cmailto:lersek@redhat.com>>>; Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com>>>; > edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>> > Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com<mailto:feng.tian@intel.com%3cmailto:feng.tian@intel.com>>>; Brijesh Singh > <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com<mailto:brijesh.singh@amd.com%3cmailto:brijesh.singh@amd.com>>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com<mailto:star.zeng@intel.com%3cmailto:star.zeng@intel.com>>> > Subject: RE: [edk2] [PATCH] MdeModulePkg: Add > EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL > > Hi Laszlo and Leo, > > If you look at the CpuSetMemoryAttributes() function in > UefiCpuPkg/CpuDxe/CpuDxe.c, you will see it now calls > AssignMemoryPageAttributes() at the end. This function 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 <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>; Leo Duran > > <leo.duran@amd.com<mailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com>>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>> > > Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com<mailto:feng.tian@intel.com%3cmailto:feng.tian@intel.com>>>; Brijesh Singh > > <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com<mailto:brijesh.singh@amd.com%3cmailto:brijesh.singh@amd.com>>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com<mailto:star.zeng@intel.com%3cmailto:star.zeng@intel.com>>> > > Subject: Re: [edk2] [PATCH] MdeModulePkg: Add > > EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL > > > > Mike, > > > > On 04/12/17 21:39, Kinney, Michael D wrote: > > > Hi Leo, > > > > > > DXE Main is supposed to use Arch Protocols for platform specific > behavior. > > > > > > In this case, can the CPU Arch Protocol SetMemoryAttributes() > > > function be used to customize behavior? > > > > > > In other modules that add/remove/modify MMIO, > > > gDS->SetMemorySpaceAttributes() 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 > > > gDS-> perform > > > extra actions? > > > > 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). > > > > 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. > > > > What would be the best general location or API to hook the "C" bit > > manipulation into MMIO GCD memory space addition / removal? > > > > Thank you, > > Laszlo > > > > > > > > > > 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<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>> > > >> Cc: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3cmailto:lersek@redhat.com>>>; Tian, Feng > > >> <feng.tian@intel.com<mailto:feng.tian@intel.com<mailto:feng.tian@intel.com%3cmailto:feng.tian@intel.com>>>; Leo > > Duran > > >> <leo.duran@amd.com<mailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com>>>; Brijesh Singh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com<mailto:brijesh.singh@amd.com%3cmailto:brijesh.singh@amd.com>>>; Zeng, > > >> Star <star.zeng@intel.com<mailto:star.zeng@intel.com<mailto:star.zeng@intel.com%3cmailto:star.zeng@intel.com>>> > > >> 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 > > >> handled by DxeMain, and as such leverages the existing > DxeProtocolNotify framework. > > >> > > >> Cc: Feng Tian <feng.tian@intel.com<mailto:feng.tian@intel.com<mailto:feng.tian@intel.com%3cmailto:feng.tian@intel.com>>> > > >> Cc: Star Zeng <star.zeng@intel.com<mailto:star.zeng@intel.com<mailto:star.zeng@intel.com%3cmailto:star.zeng@intel.com>>> > > >> Cc: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3cmailto:lersek@redhat.com>>> > > >> Cc: Brijesh Singh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com<mailto:brijesh.singh@amd.com%3cmailto:brijesh.singh@amd.com>>> > > >> Contributed-under: TianoCore Contribution Agreement 1.0 > > >> Signed-off-by: Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com>>> > > >> --- > > >> 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/DxeMain.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.<BR> > > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> + > > >> 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 > > >> @@ -17,7 +19,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF > ANY > > >> KIND, EITHER EXPRESS OR IMPLIED. > > >> #define _DXE_MAIN_H_ > > >> > > >> > > >> - > > >> #include <PiDxe.h> > > >> > > >> #include <Protocol/LoadedImage.h> > > >> @@ -53,6 +54,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF > ANY > > >> KIND, EITHER EXPRESS OR IMPLIED. > > >> #include <Protocol/TcgService.h> > > >> #include <Protocol/HiiPackageList.h> #include > > >> <Protocol/SmmBase2.h> > > >> +#include <Protocol/GcdMemorySpaceNotify.h> > > >> + > > >> #include <Guid/MemoryTypeInformation.h> #include > > >> <Guid/FirmwareFileSystem2.h> #include > <Guid/FirmwareFileSystem3.h> > > >> @@ -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 > *gGcdMemorySpaceNotify; > > >> + > > >> // > > >> // 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 DXE > CIS. > > >> # > > >> # Copyright (c) 2006 - 2017, Intel Corporation. All rights > > >> reserved.<BR> > > >> +# Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> +# > > >> # 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.<BR> > > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> + > > >> 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 > > >> @@ -42,6 +44,11 @@ EFI_GUID *gDxeCoreFileName; > > >> EFI_LOADED_IMAGE_PROTOCOL *gDxeCoreLoadedImage; > > >> > > >> // > > >> +// DXE Core global for GCD notification protocol // > > >> +EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL > *gGcdMemorySpaceNotify = > > >> +NULL; > > >> + > > >> +// > > >> // DXE Core Module Variables > > >> // > > >> EFI_BOOT_SERVICES mBootServices = { 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.<BR> > > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> + > > >> 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 > > >> @@ -45,9 +47,10 @@ EFI_CORE_PROTOCOL_NOTIFY_ENTRY > mArchProtocols[] > > >> = { // Optional protocols that the DXE Core will use if they are > > >> present // EFI_CORE_PROTOCOL_NOTIFY_ENTRY > mOptionalProtocols[] = > > >> { > > >> - { &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 > > >> + **)&gGcdMemorySpaceNotify, > > >> NULL, NULL, FALSE }, > > >> + { NULL, (VOID **)NULL, > > >> NULL, NULL, FALSE } > > >> }; > > >> > > >> // > > >> diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > > >> b/MdeModulePkg/Core/Dxe/Gcd/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.<BR> > > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> + > > >> 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 > > >> @@ -896,6 +898,9 @@ CoreConvertSpace ( > > >> } else { > > >> Entry->Capabilities = Capabilities | EFI_MEMORY_RUNTIME; > > >> } > > >> + if (gGcdMemorySpaceNotify) { > > >> + gGcdMemorySpaceNotify->MemorySpaceAddNotify > > >> + (GcdMemoryType, > > BaseAddress, > > >> Length, Entry->Capabilities); > > >> + } > > >> break; > > >> case GCD_ADD_IO_OPERATION: > > >> Entry->GcdIoType = GcdIoType; @@ -914,6 +919,9 @@ > > >> CoreConvertSpace ( > > >> case GCD_REMOVE_MEMORY_OPERATION: > > >> Entry->GcdMemoryType = EfiGcdMemoryTypeNonExistent; > > >> Entry->Capabilities = 0; > > >> + if (gGcdMemorySpaceNotify) { > > >> + gGcdMemorySpaceNotify->MemorySpaceRemoveNotify > (BaseAddress, Length); > > >> + } > > >> break; > > >> case GCD_REMOVE_IO_OPERATION: > > >> Entry->GcdIoType = 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/REMOVE > > >> operations. > > >> + > > >> + Copyright (c) 2017, AMD Inc. All rights reserved.<BR> > > >> + > > >> + 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 > > >> + http://opensource.org/licenses/bsd-license.php > > >> + > > >> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS > IS" > > >> + BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, > EITHER > > >> + EXPRESS OR > > 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 = { 0x0d51905b, 0xb77e, > > >> 0x452a, > > {0xa2, > > >> 0xc0, 0xec, 0xa0, 0xcc, 0x8d, 0x51, 0x4a } } > > >> > > >> + ## Include/Protocol/GcdMemorySpaceNotify.h > > >> + gEfiGcdMemorySpaceNotifyProtocolGuid = { 0xc842db69, 0x610e, > > >> + 0x401a, > > {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<mailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>> > > >> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL 2017-04-13 3:05 ` Yao, Jiewen @ 2017-04-14 14:10 ` Duran, Leo 0 siblings, 0 replies; 13+ messages in thread From: Duran, Leo @ 2017-04-14 14:10 UTC (permalink / raw) To: 'Yao, Jiewen', Zeng, Star, Laszlo Ersek, Kinney, Michael D, edk2-devel@ml01.01.org Cc: Tian, Feng, Singh, Brijesh All, Let's please drop this patch for now... We're evaluating Yao's suggestion (which looks OK so far). Thanks, Leo. From: Yao, Jiewen [mailto:jiewen.yao@intel.com] Sent: Wednesday, April 12, 2017 10:06 PM To: Duran, Leo <leo.duran@amd.com>; Zeng, Star <star.zeng@intel.com>; Laszlo Ersek <lersek@redhat.com>; Kinney, Michael D <michael.d.kinney@intel.com>; edk2-devel@ml01.01.org Cc: Tian, Feng <feng.tian@intel.com>; Singh, Brijesh <brijesh.singh@amd.com>; Yao, Jiewen <jiewen.yao@intel.com> Subject: RE: [edk2] [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL Yes, exactly. I hope the default path be as simple as possible. Then we only need update the page table in the corner case. Thank you Yao Jiewen From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Duran, Leo Sent: Thursday, April 13, 2017 10:44 AM To: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; Singh, Brijesh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>> Subject: Re: [edk2] [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL From: Yao, Jiewen [mailto:jiewen.yao@intel.com] Sent: Wednesday, April 12, 2017 9:40 PM To: Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org> Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com>>; Singh, Brijesh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> Subject: RE: [edk2] [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL Hi Leo Can we clear SEC for all non-DRAM region, instead of MMIO region? Then we do not need worried about 'ADD' MMIO. [Duran, Leo] Ummh... Let me check with Brijesh. I guess you're suggesting a one-time deal, done once page tables are created. Thank you Yao Jiewen From: Duran, Leo [mailto:leo.duran@amd.com] Sent: Thursday, April 13, 2017 10:35 AM To: Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com<mailto:star.zeng@intel.com%3cmailto:star.zeng@intel.com>>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3cmailto:lersek@redhat.com>>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>> Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com<mailto:feng.tian@intel.com%3cmailto:feng.tian@intel.com>>>; Singh, Brijesh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com<mailto:brijesh.singh@amd.com%3cmailto:brijesh.singh@amd.com>>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>> Subject: RE: [edk2] [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL Hi Star, At DxeIpl time we only have the MMIO regions published by descriptor HOB's built during PEI. But we also need to capture MMIO regions that may be 'added' by DXE drivers (e.g., PciHostBridgeDxe), ideally without making changes to those drivers. So our thinking is that trapping the 'Add' MMIO requests that come into GCD would be a good place to capture what we need. Thanks, Leo. > -----Original Message----- > From: Zeng, Star [mailto:star.zeng@intel.com] > Sent: Wednesday, April 12, 2017 9:14 PM > To: Duran, Leo <leo.duran@amd.com<mailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com>>>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3cmailto:lersek@redhat.com>>>; > Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>> > Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com<mailto:feng.tian@intel.com%3cmailto:feng.tian@intel.com>>>; Singh, Brijesh > <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com<mailto:brijesh.singh@amd.com%3cmailto:brijesh.singh@amd.com>>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com<mailto:jiewen.yao@intel.com%3cmailto:jiewen.yao@intel.com>>>; Zeng, Star > <star.zeng@intel.com<mailto:star.zeng@intel.com<mailto:star.zeng@intel.com%3cmailto:star.zeng@intel.com>>> > Subject: RE: [edk2] [PATCH] MdeModulePkg: Add > EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL > > Does the solution Jiewen proposed at https://lists.01.org/pipermail/edk2- > devel/2017-March/008987.html also work for this case similarly? > > Thanks, > Star > -----Original Message----- > From: Kinney, Michael D > Sent: Thursday, April 13, 2017 8:59 AM > To: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3cmailto:lersek@redhat.com>>>; Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com>>>; > edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>> > Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com<mailto:feng.tian@intel.com%3cmailto:feng.tian@intel.com>>>; Brijesh Singh > <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com<mailto:brijesh.singh@amd.com%3cmailto:brijesh.singh@amd.com>>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com<mailto:star.zeng@intel.com%3cmailto:star.zeng@intel.com>>> > Subject: RE: [edk2] [PATCH] MdeModulePkg: Add > EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL > > Hi Laszlo and Leo, > > If you look at the CpuSetMemoryAttributes() function in > UefiCpuPkg/CpuDxe/CpuDxe.c, you will see it now calls > AssignMemoryPageAttributes() at the end. This function 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 <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com%3cmailto:michael.d.kinney@intel.com>>>; Leo Duran > > <leo.duran@amd.com<mailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com>>>; edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>> > > Cc: Tian, Feng <feng.tian@intel.com<mailto:feng.tian@intel.com<mailto:feng.tian@intel.com%3cmailto:feng.tian@intel.com>>>; Brijesh Singh > > <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com<mailto:brijesh.singh@amd.com%3cmailto:brijesh.singh@amd.com>>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com<mailto:star.zeng@intel.com%3cmailto:star.zeng@intel.com>>> > > Subject: Re: [edk2] [PATCH] MdeModulePkg: Add > > EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL > > > > Mike, > > > > On 04/12/17 21:39, Kinney, Michael D wrote: > > > Hi Leo, > > > > > > DXE Main is supposed to use Arch Protocols for platform specific > behavior. > > > > > > In this case, can the CPU Arch Protocol SetMemoryAttributes() > > > function be used to customize behavior? > > > > > > In other modules that add/remove/modify MMIO, > > > gDS->SetMemorySpaceAttributes() 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 > > > gDS-> perform > > > extra actions? > > > > 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). > > > > 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. > > > > What would be the best general location or API to hook the "C" bit > > manipulation into MMIO GCD memory space addition / removal? > > > > Thank you, > > Laszlo > > > > > > > > > > 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<mailto:edk2-devel@ml01.01.org<mailto:edk2-devel@ml01.01.org%3cmailto:edk2-devel@ml01.01.org>> > > >> Cc: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3cmailto:lersek@redhat.com>>>; Tian, Feng > > >> <feng.tian@intel.com<mailto:feng.tian@intel.com<mailto:feng.tian@intel.com%3cmailto:feng.tian@intel.com>>>; Leo > > Duran > > >> <leo.duran@amd.com<mailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com>>>; Brijesh Singh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com<mailto:brijesh.singh@amd.com%3cmailto:brijesh.singh@amd.com>>>; Zeng, > > >> Star <star.zeng@intel.com<mailto:star.zeng@intel.com<mailto:star.zeng@intel.com%3cmailto:star.zeng@intel.com>>> > > >> 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 > > >> handled by DxeMain, and as such leverages the existing > DxeProtocolNotify framework. > > >> > > >> Cc: Feng Tian <feng.tian@intel.com<mailto:feng.tian@intel.com<mailto:feng.tian@intel.com%3cmailto:feng.tian@intel.com>>> > > >> Cc: Star Zeng <star.zeng@intel.com<mailto:star.zeng@intel.com<mailto:star.zeng@intel.com%3cmailto:star.zeng@intel.com>>> > > >> Cc: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com<mailto:lersek@redhat.com%3cmailto:lersek@redhat.com>>> > > >> Cc: Brijesh Singh <brijesh.singh@amd.com<mailto:brijesh.singh@amd.com<mailto:brijesh.singh@amd.com%3cmailto:brijesh.singh@amd.com>>> > > >> Contributed-under: TianoCore Contribution Agreement 1.0 > > >> Signed-off-by: Leo Duran <leo.duran@amd.com<mailto:leo.duran@amd.com<mailto:leo.duran@amd.com%3cmailto:leo.duran@amd.com>>> > > >> --- > > >> 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/DxeMain.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.<BR> > > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> + > > >> 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 > > >> @@ -17,7 +19,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF > ANY > > >> KIND, EITHER EXPRESS OR IMPLIED. > > >> #define _DXE_MAIN_H_ > > >> > > >> > > >> - > > >> #include <PiDxe.h> > > >> > > >> #include <Protocol/LoadedImage.h> > > >> @@ -53,6 +54,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF > ANY > > >> KIND, EITHER EXPRESS OR IMPLIED. > > >> #include <Protocol/TcgService.h> > > >> #include <Protocol/HiiPackageList.h> #include > > >> <Protocol/SmmBase2.h> > > >> +#include <Protocol/GcdMemorySpaceNotify.h> > > >> + > > >> #include <Guid/MemoryTypeInformation.h> #include > > >> <Guid/FirmwareFileSystem2.h> #include > <Guid/FirmwareFileSystem3.h> > > >> @@ -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 > *gGcdMemorySpaceNotify; > > >> + > > >> // > > >> // 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 DXE > CIS. > > >> # > > >> # Copyright (c) 2006 - 2017, Intel Corporation. All rights > > >> reserved.<BR> > > >> +# Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> +# > > >> # 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.<BR> > > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> + > > >> 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 > > >> @@ -42,6 +44,11 @@ EFI_GUID *gDxeCoreFileName; > > >> EFI_LOADED_IMAGE_PROTOCOL *gDxeCoreLoadedImage; > > >> > > >> // > > >> +// DXE Core global for GCD notification protocol // > > >> +EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL > *gGcdMemorySpaceNotify = > > >> +NULL; > > >> + > > >> +// > > >> // DXE Core Module Variables > > >> // > > >> EFI_BOOT_SERVICES mBootServices = { 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.<BR> > > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> + > > >> 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 > > >> @@ -45,9 +47,10 @@ EFI_CORE_PROTOCOL_NOTIFY_ENTRY > mArchProtocols[] > > >> = { // Optional protocols that the DXE Core will use if they are > > >> present // EFI_CORE_PROTOCOL_NOTIFY_ENTRY > mOptionalProtocols[] = > > >> { > > >> - { &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 > > >> + **)&gGcdMemorySpaceNotify, > > >> NULL, NULL, FALSE }, > > >> + { NULL, (VOID **)NULL, > > >> NULL, NULL, FALSE } > > >> }; > > >> > > >> // > > >> diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > > >> b/MdeModulePkg/Core/Dxe/Gcd/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.<BR> > > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> + > > >> 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 > > >> @@ -896,6 +898,9 @@ CoreConvertSpace ( > > >> } else { > > >> Entry->Capabilities = Capabilities | EFI_MEMORY_RUNTIME; > > >> } > > >> + if (gGcdMemorySpaceNotify) { > > >> + gGcdMemorySpaceNotify->MemorySpaceAddNotify > > >> + (GcdMemoryType, > > BaseAddress, > > >> Length, Entry->Capabilities); > > >> + } > > >> break; > > >> case GCD_ADD_IO_OPERATION: > > >> Entry->GcdIoType = GcdIoType; @@ -914,6 +919,9 @@ > > >> CoreConvertSpace ( > > >> case GCD_REMOVE_MEMORY_OPERATION: > > >> Entry->GcdMemoryType = EfiGcdMemoryTypeNonExistent; > > >> Entry->Capabilities = 0; > > >> + if (gGcdMemorySpaceNotify) { > > >> + gGcdMemorySpaceNotify->MemorySpaceRemoveNotify > (BaseAddress, Length); > > >> + } > > >> break; > > >> case GCD_REMOVE_IO_OPERATION: > > >> Entry->GcdIoType = 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/REMOVE > > >> operations. > > >> + > > >> + Copyright (c) 2017, AMD Inc. All rights reserved.<BR> > > >> + > > >> + 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 > > >> + http://opensource.org/licenses/bsd-license.php > > >> + > > >> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS > IS" > > >> + BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, > EITHER > > >> + EXPRESS OR > > 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 = { 0x0d51905b, 0xb77e, > > >> 0x452a, > > {0xa2, > > >> 0xc0, 0xec, 0xa0, 0xcc, 0x8d, 0x51, 0x4a } } > > >> > > >> + ## Include/Protocol/GcdMemorySpaceNotify.h > > >> + gEfiGcdMemorySpaceNotifyProtocolGuid = { 0xc842db69, 0x610e, > > >> + 0x401a, > > {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<mailto:edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org%3cmailto:edk2-devel@lists.01.org>> > > >> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL 2017-04-13 0:58 ` Kinney, Michael D 2017-04-13 2:13 ` Zeng, Star @ 2017-04-13 2:27 ` Duran, Leo 1 sibling, 0 replies; 13+ messages in thread From: Duran, Leo @ 2017-04-13 2:27 UTC (permalink / raw) To: Kinney, Michael D, Laszlo Ersek, edk2-devel@ml01.01.org Cc: Tian, Feng, Singh, Brijesh, Zeng, Star Mike, The a ranges we're interest in are those that invoke GCD with an 'Add' request for MMIO. Before any DXE drivers are dispatched the 'Add' requests are simply the MMIO regions reported by resource descriptor HOB's built prior to DxeIpl. However, once DXE drivers are dispatched GCD may get other 'Add' requests for MMIO (e.g., from PciHostBridgeDxe), so we'd interested in those as well. So it seems like by having GCD issue a notify on 'Add' requests would cover all cases, without requiring any surgery in DXE components that may issue 'Add' requests. Leo. > -----Original Message----- > From: Kinney, Michael D [mailto:michael.d.kinney@intel.com] > Sent: Wednesday, April 12, 2017 7:59 PM > To: Laszlo Ersek <lersek@redhat.com>; Duran, Leo <leo.duran@amd.com>; > edk2-devel@ml01.01.org; Kinney, Michael D <michael.d.kinney@intel.com> > Cc: Tian, Feng <feng.tian@intel.com>; Singh, Brijesh > <brijesh.singh@amd.com>; Zeng, Star <star.zeng@intel.com> > Subject: RE: [edk2] [PATCH] MdeModulePkg: Add > EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL > > Hi Laszlo and Leo, > > If you look at the CpuSetMemoryAttributes() function in > UefiCpuPkg/CpuDxe/CpuDxe.c, you will see it now calls > AssignMemoryPageAttributes() at the end. This function 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 <michael.d.kinney@intel.com>; Leo Duran > > <leo.duran@amd.com>; edk2-devel@ml01.01.org > > Cc: Tian, Feng <feng.tian@intel.com>; Brijesh Singh > > <brijesh.singh@amd.com>; Zeng, Star <star.zeng@intel.com> > > Subject: Re: [edk2] [PATCH] MdeModulePkg: Add > > EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL > > > > Mike, > > > > On 04/12/17 21:39, Kinney, Michael D wrote: > > > Hi Leo, > > > > > > DXE Main is supposed to use Arch Protocols for platform specific > behavior. > > > > > > In this case, can the CPU Arch Protocol SetMemoryAttributes() > > > function be used to customize behavior? > > > > > > In other modules that add/remove/modify MMIO, > > > gDS->SetMemorySpaceAttributes() 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 > > > gDS-> perform > > > extra actions? > > > > 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). > > > > 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. > > > > What would be the best general location or API to hook the "C" bit > > manipulation into MMIO GCD memory space addition / removal? > > > > Thank you, > > Laszlo > > > > > > > > > > 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 <lersek@redhat.com>; Tian, Feng > > >> <feng.tian@intel.com>; Leo > > Duran > > >> <leo.duran@amd.com>; Brijesh Singh <brijesh.singh@amd.com>; Zeng, > > >> Star <star.zeng@intel.com> > > >> 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 > > >> handled by DxeMain, and as such leverages the existing > DxeProtocolNotify framework. > > >> > > >> Cc: Feng Tian <feng.tian@intel.com> > > >> Cc: Star Zeng <star.zeng@intel.com> > > >> Cc: Laszlo Ersek <lersek@redhat.com> > > >> Cc: Brijesh Singh <brijesh.singh@amd.com> > > >> Contributed-under: TianoCore Contribution Agreement 1.0 > > >> Signed-off-by: Leo Duran <leo.duran@amd.com> > > >> --- > > >> 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/DxeMain.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.<BR> > > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> + > > >> 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 > > >> @@ -17,7 +19,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF > ANY > > >> KIND, EITHER EXPRESS OR IMPLIED. > > >> #define _DXE_MAIN_H_ > > >> > > >> > > >> - > > >> #include <PiDxe.h> > > >> > > >> #include <Protocol/LoadedImage.h> > > >> @@ -53,6 +54,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF > ANY > > >> KIND, EITHER EXPRESS OR IMPLIED. > > >> #include <Protocol/TcgService.h> > > >> #include <Protocol/HiiPackageList.h> #include > > >> <Protocol/SmmBase2.h> > > >> +#include <Protocol/GcdMemorySpaceNotify.h> > > >> + > > >> #include <Guid/MemoryTypeInformation.h> #include > > >> <Guid/FirmwareFileSystem2.h> #include > <Guid/FirmwareFileSystem3.h> > > >> @@ -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 > *gGcdMemorySpaceNotify; > > >> + > > >> // > > >> // 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 DXE > CIS. > > >> # > > >> # Copyright (c) 2006 - 2017, Intel Corporation. All rights > > >> reserved.<BR> > > >> +# Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> +# > > >> # 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.<BR> > > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> + > > >> 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 > > >> @@ -42,6 +44,11 @@ EFI_GUID *gDxeCoreFileName; > > >> EFI_LOADED_IMAGE_PROTOCOL *gDxeCoreLoadedImage; > > >> > > >> // > > >> +// DXE Core global for GCD notification protocol // > > >> +EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL > *gGcdMemorySpaceNotify = > > >> +NULL; > > >> + > > >> +// > > >> // DXE Core Module Variables > > >> // > > >> EFI_BOOT_SERVICES mBootServices = { 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.<BR> > > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> + > > >> 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 > > >> @@ -45,9 +47,10 @@ EFI_CORE_PROTOCOL_NOTIFY_ENTRY > mArchProtocols[] > > >> = { // Optional protocols that the DXE Core will use if they are > > >> present // EFI_CORE_PROTOCOL_NOTIFY_ENTRY > mOptionalProtocols[] = > > >> { > > >> - { &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 > > >> + **)&gGcdMemorySpaceNotify, > > >> NULL, NULL, FALSE }, > > >> + { NULL, (VOID **)NULL, > > >> NULL, NULL, FALSE } > > >> }; > > >> > > >> // > > >> diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > > >> b/MdeModulePkg/Core/Dxe/Gcd/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.<BR> > > >> +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > >> + > > >> 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 > > >> @@ -896,6 +898,9 @@ CoreConvertSpace ( > > >> } else { > > >> Entry->Capabilities = Capabilities | EFI_MEMORY_RUNTIME; > > >> } > > >> + if (gGcdMemorySpaceNotify) { > > >> + gGcdMemorySpaceNotify->MemorySpaceAddNotify > > >> + (GcdMemoryType, > > BaseAddress, > > >> Length, Entry->Capabilities); > > >> + } > > >> break; > > >> case GCD_ADD_IO_OPERATION: > > >> Entry->GcdIoType = GcdIoType; @@ -914,6 +919,9 @@ > > >> CoreConvertSpace ( > > >> case GCD_REMOVE_MEMORY_OPERATION: > > >> Entry->GcdMemoryType = EfiGcdMemoryTypeNonExistent; > > >> Entry->Capabilities = 0; > > >> + if (gGcdMemorySpaceNotify) { > > >> + gGcdMemorySpaceNotify->MemorySpaceRemoveNotify > (BaseAddress, Length); > > >> + } > > >> break; > > >> case GCD_REMOVE_IO_OPERATION: > > >> Entry->GcdIoType = 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/REMOVE > > >> operations. > > >> + > > >> + Copyright (c) 2017, AMD Inc. All rights reserved.<BR> > > >> + > > >> + 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 > > >> + http://opensource.org/licenses/bsd-license.php > > >> + > > >> + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS > IS" > > >> + BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, > EITHER > > >> + EXPRESS OR > > 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 = { 0x0d51905b, 0xb77e, > > >> 0x452a, > > {0xa2, > > >> 0xc0, 0xec, 0xa0, 0xcc, 0x8d, 0x51, 0x4a } } > > >> > > >> + ## Include/Protocol/GcdMemorySpaceNotify.h > > >> + gEfiGcdMemorySpaceNotifyProtocolGuid = { 0xc842db69, 0x610e, > > >> + 0x401a, > > {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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL 2017-04-12 19:39 ` Kinney, Michael D 2017-04-12 20:32 ` Laszlo Ersek @ 2017-04-12 20:34 ` Duran, Leo 1 sibling, 0 replies; 13+ messages in thread From: Duran, Leo @ 2017-04-12 20:34 UTC (permalink / raw) To: Kinney, Michael D, edk2-devel@ml01.01.org Cc: Laszlo Ersek, Tian, Feng, Singh, Brijesh, Zeng, Star Hi Mike, Please see my replies below. Leo. > -----Original Message----- > From: Kinney, Michael D [mailto:michael.d.kinney@intel.com] > Sent: Wednesday, April 12, 2017 2:39 PM > To: Duran, Leo <leo.duran@amd.com>; edk2-devel@ml01.01.org; Kinney, > Michael D <michael.d.kinney@intel.com> > Cc: Laszlo Ersek <lersek@redhat.com>; Tian, Feng <feng.tian@intel.com>; > Singh, Brijesh <brijesh.singh@amd.com>; Zeng, Star <star.zeng@intel.com> > Subject: RE: [edk2] [PATCH] MdeModulePkg: Add > EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL > > Hi Leo, > > DXE Main is supposed to use Arch Protocols for platform specific behavior. > > In this case, can the CPU Arch Protocol SetMemoryAttributes() function be > used to customize behavior? [Duran, Leo] DxeMain supports a list of Arch protocols and also a list of optional protocols. SetMemoryAttributes() is a service provided by GCD, via CoreConvertSpace(), just as Add/RemoveMemorySpace(). The behavior we need is a notification into OvmfPkg as GCD gets called to Add/RemoveMemorySpace(), not just if/when GCD gets called to SetMemoryAttributes(). BTW, SetMemoryAttributes() may get called on any range, without specifying type (whereas we're interested in MMIO ranges, which are specified in ADD operations) > > In other modules that add/remove/modify MMIO, gDS- > >SetMemorySpaceAttributes() 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? > > 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 <lersek@redhat.com>; Tian, Feng > > <feng.tian@intel.com>; Leo Duran <leo.duran@amd.com>; Brijesh Singh > > <brijesh.singh@amd.com>; Zeng, Star <star.zeng@intel.com> > > 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 > > handled by DxeMain, and as such leverages the existing DxeProtocolNotify > framework. > > > > Cc: Feng Tian <feng.tian@intel.com> > > Cc: Star Zeng <star.zeng@intel.com> > > Cc: Laszlo Ersek <lersek@redhat.com> > > Cc: Brijesh Singh <brijesh.singh@amd.com> > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Leo Duran <leo.duran@amd.com> > > --- > > 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/DxeMain.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.<BR> > > +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > + > > 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 @@ -17,7 +19,6 @@ WITHOUT WARRANTIES OR > REPRESENTATIONS OF > > ANY KIND, EITHER EXPRESS OR IMPLIED. > > #define _DXE_MAIN_H_ > > > > > > - > > #include <PiDxe.h> > > > > #include <Protocol/LoadedImage.h> > > @@ -53,6 +54,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF > ANY KIND, > > EITHER EXPRESS OR IMPLIED. > > #include <Protocol/TcgService.h> > > #include <Protocol/HiiPackageList.h> > > #include <Protocol/SmmBase2.h> > > +#include <Protocol/GcdMemorySpaceNotify.h> > > + > > #include <Guid/MemoryTypeInformation.h> #include > > <Guid/FirmwareFileSystem2.h> #include <Guid/FirmwareFileSystem3.h> > @@ > > -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 > *gGcdMemorySpaceNotify; > > + > > // > > // 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 DXE > CIS. > > # > > # Copyright (c) 2006 - 2017, Intel Corporation. All rights > > reserved.<BR> > > +# Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> # > > # 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.<BR> > > +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > + > > 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 > > @@ -42,6 +44,11 @@ EFI_GUID *gDxeCoreFileName; > > EFI_LOADED_IMAGE_PROTOCOL *gDxeCoreLoadedImage; > > > > // > > +// DXE Core global for GCD notification protocol // > > +EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL > *gGcdMemorySpaceNotify = NULL; > > + > > +// > > // DXE Core Module Variables > > // > > EFI_BOOT_SERVICES mBootServices = { > > 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.<BR> > > +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > + > > 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 @@ -45,9 +47,10 @@ EFI_CORE_PROTOCOL_NOTIFY_ENTRY > > mArchProtocols[] = { // Optional protocols that the DXE Core will use > > if they are present // EFI_CORE_PROTOCOL_NOTIFY_ENTRY > > mOptionalProtocols[] = { > > - { &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 > > + **)&gGcdMemorySpaceNotify, > > NULL, NULL, FALSE }, > > + { NULL, (VOID **)NULL, > > NULL, NULL, FALSE } > > }; > > > > // > > diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > > b/MdeModulePkg/Core/Dxe/Gcd/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.<BR> > > +Copyright (c) 2017, AMD Incorporated. All rights reserved.<BR> > > + > > 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 @@ -896,6 +898,9 @@ CoreConvertSpace ( > > } else { > > Entry->Capabilities = Capabilities | EFI_MEMORY_RUNTIME; > > } > > + if (gGcdMemorySpaceNotify) { > > + gGcdMemorySpaceNotify->MemorySpaceAddNotify > (GcdMemoryType, > > + BaseAddress, > > Length, Entry->Capabilities); > > + } > > break; > > case GCD_ADD_IO_OPERATION: > > Entry->GcdIoType = GcdIoType; > > @@ -914,6 +919,9 @@ CoreConvertSpace ( > > case GCD_REMOVE_MEMORY_OPERATION: > > Entry->GcdMemoryType = EfiGcdMemoryTypeNonExistent; > > Entry->Capabilities = 0; > > + if (gGcdMemorySpaceNotify) { > > + gGcdMemorySpaceNotify->MemorySpaceRemoveNotify > (BaseAddress, Length); > > + } > > break; > > case GCD_REMOVE_IO_OPERATION: > > Entry->GcdIoType = 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/REMOVE > > operations. > > + > > + Copyright (c) 2017, AMD Inc. All rights reserved.<BR> > > + > > + 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 > > + http://opensource.org/licenses/bsd-license.php > > + > > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" > > + BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, > EITHER EXPRESS OR 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 = { 0x0d51905b, 0xb77e, > > 0x452a, {0xa2, 0xc0, 0xec, 0xa0, 0xcc, 0x8d, 0x51, 0x4a } } > > > > + ## Include/Protocol/GcdMemorySpaceNotify.h > > + gEfiGcdMemorySpaceNotifyProtocolGuid = { 0xc842db69, 0x610e, > > + 0x401a, {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 ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2017-04-14 14:10 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-04-12 18:56 [PATCH] MdeModulePkg: Add EFI_GCD_MEMORY_SPACE_NOTIFY_PROTOCOL Leo Duran 2017-04-12 18:56 ` Leo Duran 2017-04-12 19:39 ` Kinney, Michael D 2017-04-12 20:32 ` Laszlo Ersek 2017-04-13 0:58 ` Kinney, Michael D 2017-04-13 2:13 ` Zeng, Star 2017-04-13 2:35 ` Duran, Leo 2017-04-13 2:40 ` Yao, Jiewen 2017-04-13 2:44 ` Duran, Leo 2017-04-13 3:05 ` Yao, Jiewen 2017-04-14 14:10 ` Duran, Leo 2017-04-13 2:27 ` Duran, Leo 2017-04-12 20:34 ` Duran, Leo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox