Current gCpu->SetMemoryAttributes does not support getting memory attributes, and has no clear description about clearing memory attributes.
I noticed we introduced SmmMemoryAttribute(MdeModulePkg\Include\Protocol\SmmMemoryAttribute.h) protocol for heap guard feature in SMM environment. Seemingly, we also need introduce MemoryAttribute protocol for DXE? Thanks, Star -----Original Message----- From: Zeng, Star Sent: Tuesday, April 3, 2018 8:59 AM To: Kinney, Michael D <[email protected]>; Heyi Guo <[email protected]>; [email protected] Cc: Yi Li <[email protected]>; Renhao Liang <[email protected]>; Dong, Eric <[email protected]>; Gao, Liming <[email protected]>; Wang, Jian J <[email protected]>; Ni, Ruiyu <[email protected]>; Zeng, Star <[email protected]> Subject: RE: [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute conversion Mike, Agree the problem. We need support only RUNTIME attribute. We need support only cache attributes. We need support only page attributes. We need support RUNTIME + cache + page attributes. Do we need support the 0 Attributes case to clear page attributes? There was discussion about the 0 Attributes case at https://lists.01.org/pipermail/edk2-devel/2018-March/023315.html. It came to the question that should the 0 Attributes case be handled by SetMemoryAttributes() to clear the page attributes? Thanks, Star -----Original Message----- From: Kinney, Michael D Sent: Tuesday, April 3, 2018 5:43 AM To: Zeng, Star <[email protected]>; Heyi Guo <[email protected]>; [email protected]; Kinney, Michael D <[email protected]> Cc: Yi Li <[email protected]>; Renhao Liang <[email protected]>; Dong, Eric <[email protected]>; Gao, Liming <[email protected]>; Wang, Jian J <[email protected]>; Ni, Ruiyu <[email protected]> Subject: RE: [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute conversion Star, This commit breaks Vlv2TbltDevicePkg. On this platform, there are 2 calls to gDS->SetMemorySpaceAttributes() for an MMIO range to set only the EFI_MEMORY_RUNTIME bit. With this commit, ConverToCpuArchAttributes()returns 0, and 0 is passed to gCpu->SetMemoryAttributes()that returns EFI_INVALID_PARAMETER on Vlv2TbltDevicePkg. Before this commit, ConverToCpuArchAttributes() returns INVALID_CPU_ARCH_ATTRIBUTES which causes the call to gCpu->SetMemoryAttributes()to be skipped so no error is generated. I think the right fix is to skip the call to gCpu->SetMemoryAttributes() if CpuArchAttributes is 0 so a call that only sets the RUNTIME attribute can be supported along with call the set both the RUNTIME bit and other cache related bits. I will send a patch soon with the proposed fix. Mike > -----Original Message----- > From: Zeng, Star > Sent: Sunday, April 1, 2018 10:59 PM > To: Heyi Guo <[email protected]>; edk2- [email protected] > Cc: Yi Li <[email protected]>; Renhao Liang > <[email protected]>; Dong, Eric <[email protected]>; Kinney, > Michael D <[email protected]>; Gao, Liming > <[email protected]>; Wang, Jian J <[email protected]>; Ni, > Ruiyu <[email protected]>; Zeng, Star <[email protected]> > Subject: RE: [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute > conversion > > Pushed at 5b91bf82c67b586b9588cbe4bbffa1588f6b5926. > > Thanks, > Star > -----Original Message----- > From: Heyi Guo [mailto:[email protected]] > Sent: Thursday, March 29, 2018 4:20 PM > To: [email protected] > Cc: Heyi Guo <[email protected]>; Yi Li <[email protected]>; > Renhao Liang <[email protected]>; Zeng, Star > <[email protected]>; Dong, Eric <[email protected]>; Kinney, > Michael D <[email protected]>; Gao, Liming > <[email protected]>; Wang, Jian J <[email protected]>; Ni, > Ruiyu <[email protected]> > Subject: [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute conversion > > For gDS->SetMemorySpaceAttributes(), when user passes a combined > memory attribute including CPU arch attribute and other attributes, > like EFI_MEMORY_RUNTIME, > ConverToCpuArchAttributes() will return INVALID_CPU_ARCH_ATTRIBUTES > and skip setting page/cache attribute for the specified memory space. > > We don't see any reason to forbid combining CPU arch attributes and > non-CPU-arch attributes when calling gDS- > >SetMemorySpaceAttributes(), so we remove the check code > in ConverToCpuArchAttributes(); the remaining code is enough to grab > the interested bits for > Cpu->SetMemoryAttributes(). > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Heyi Guo <[email protected]> > Signed-off-by: Yi Li <[email protected]> > Signed-off-by: Renhao Liang <[email protected]> > Cc: Star Zeng <[email protected]> > Cc: Eric Dong <[email protected]> > Cc: Michael D Kinney <[email protected]> > Cc: Liming Gao <[email protected]> > Cc: Jian J Wang <[email protected]> > Cc: Ruiyu Ni <[email protected]> > --- > MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 5 ----- > 1 file changed, 5 deletions(-) > > diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c index > 77f4adb4bc01..907245a3f512 100644 > --- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > +++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c > @@ -673,11 +673,6 @@ ConverToCpuArchAttributes ( { > UINT64 CpuArchAttributes; > > - if ((Attributes & ~(EXCLUSIVE_MEMORY_ATTRIBUTES | > - NONEXCLUSIVE_MEMORY_ATTRIBUTES)) > != 0) { > - return INVALID_CPU_ARCH_ATTRIBUTES; > - } > - > CpuArchAttributes = Attributes & > NONEXCLUSIVE_MEMORY_ATTRIBUTES; > > if ( (Attributes & EFI_MEMORY_UC) == EFI_MEMORY_UC) { > -- > 2.7.4 _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
