Thanks. Reviewed-by: [email protected]
> -----Original Message----- > From: edk2-devel [mailto:[email protected]] On Behalf Of Star > Zeng > Sent: Wednesday, April 4, 2018 10:08 AM > To: [email protected] > Cc: Ni, Ruiyu <[email protected]>; Yi Li <[email protected]>; Dong, > Eric > <[email protected]>; Renhao Liang <[email protected]>; Gao, Liming > <[email protected]>; Heyi Guo <[email protected]>; Kinney, Michael D > <[email protected]>; Zeng, Star <[email protected]> > Subject: [edk2] [PATCH V2] MdeModulePkg/Gcd: Filter > gCpu->SetMemoryAttributes() calls > > From: "Kinney, Michael D" <[email protected]> > > This patch fixes an issue with VlvTbltDevicePkg introduced > by commit 5b91bf82c67b586b9588cbe4bbffa1588f6b5926. > > The history is as below. > To support heap guard feature, > 14dde9e903bb9a719ebb8f3381da72b19509bc36 > added support for SetMemorySpaceAttributes() to handle page attributes, > but after that, a combination of CPU arch attributes and other attributes > was not allowed anymore, for example, UC + RUNTIME. It is a regression. > Then 5b91bf82c67b586b9588cbe4bbffa1588f6b5926 was to fix the regression, > and we thought 0 CPU arch attributes may be used to clear CPU arch > attributes, so 0 CPU arch attributes was allowed to be sent to > gCpu->SetMemoryAttributes(). > > But some implementation of CPU driver may return error for 0 CPU arch > attributes. That fails the case that caller just calls > SetMemorySpaceAttributes() with none CPU arch attributes (for example, > RUNTIME), and the purpose of the case is not to clear CPU arch attributes. > > This patch filters the call to gCpu->SetMemoryAttributes() > if the requested attributes is 0. It also removes the #define > INVALID_CPU_ARCH_ATTRIBUTES that is no longer used. > > Cc: Heyi Guo <[email protected]> > Cc: Yi Li <[email protected]> > Cc: Renhao Liang <[email protected]> > Cc: Star Zeng <[email protected]> > Cc: Eric Dong <[email protected]> > Cc: Liming Gao <[email protected]> > Cc: Jian J Wang <[email protected]> > Cc: Ruiyu Ni <[email protected]> > Signed-off-by: Michael D Kinney <[email protected]> > Signed-off-by: Star Zeng <[email protected]> > Contributed-under: TianoCore Contribution Agreement 1.1 > --- > MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 25 ++++++++++++++++++++++--- > 1 file changed, 22 insertions(+), 3 deletions(-) > > diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > index 907245a3f512..31ddf9c3bb51 100644 > --- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > +++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > @@ -48,8 +48,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY > KIND, EITHER EXPRESS OR IMPLIED. > #define NONEXCLUSIVE_MEMORY_ATTRIBUTES (EFI_MEMORY_XP | > EFI_MEMORY_RP | \ > EFI_MEMORY_RO) > > -#define INVALID_CPU_ARCH_ATTRIBUTES 0xffffffff > - > // > // Module Variables > // > @@ -873,7 +871,21 @@ CoreConvertSpace ( > // Call CPU Arch Protocol to attempt to set attributes on the range > // > CpuArchAttributes = ConverToCpuArchAttributes (Attributes); > - if (CpuArchAttributes != INVALID_CPU_ARCH_ATTRIBUTES) { > + // > + // CPU arch attributes include page attributes and cache attributes. > + // Only page attributes supports to be cleared, but not cache attributes. > + // Caller is expected to use GetMemorySpaceDescriptor() to get the > current > + // attributes, AND/OR attributes, and then calls > SetMemorySpaceAttributes() > + // to set the new attributes. > + // So 0 CPU arch attributes should not happen as memory should always > have > + // a cache attribute (no matter UC or WB, etc). > + // > + // Here, 0 CPU arch attributes will be filtered to be compatible with the > + // case that caller just calls SetMemorySpaceAttributes() with none CPU > + // arch attributes (for example, RUNTIME) as the purpose of the case is > not > + // to clear CPU arch attributes. > + // > + if (CpuArchAttributes != 0) { > if (gCpu == NULL) { > Status = EFI_NOT_AVAILABLE_YET; > } else { > @@ -936,6 +948,13 @@ CoreConvertSpace ( > // Set attributes operation > // > case GCD_SET_ATTRIBUTES_MEMORY_OPERATION: > + if (CpuArchAttributes == 0) { > + // > + // Keep original CPU arch attributes when caller just calls > + // SetMemorySpaceAttributes() with none CPU arch attributes (for > example, RUNTIME). > + // > + Attributes |= (Entry->Attributes & > (EXCLUSIVE_MEMORY_ATTRIBUTES | NONEXCLUSIVE_MEMORY_ATTRIBUTES)); > + } > Entry->Attributes = Attributes; > break; > // > -- > 2.7.0.windows.1 > > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
