[edk2] [Patch][edk2-platforms/devel-MinnowBoard3-UDK2017] Add Memory test code.

2018-04-02 Thread zwei4 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: zwei4 
---
 .../PlatformBootManagerLib/PlatformBootManager.c   | 23 +-
 .../PlatformBootManagerLib.inf |  1 +
 2 files changed, 23 insertions(+), 1 deletion(-)

diff --git 
a/Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootManager.c
 
b/Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootManager.c
index 7c7a98e2b9..6715d9073b 100644
--- 
a/Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootManager.c
+++ 
b/Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootManager.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1000,10 +1001,30 @@ PlatformBootManagerAfterConsole (
   VOID
   )
 {
-  EFI_BOOT_MODE LocalBootMode;
+  EFI_STATUSStatus;
+  EFI_BOOT_MODE LocalBootMode;
+  BOOLEAN   RequireSoftECCInit;
+  EFI_GENERIC_MEMORY_TEST_PROTOCOL  *GenMemoryTest;
   
   DEBUG ((EFI_D_INFO, "PlatformBootManagerAfterConsole\n"));
 
+  //
+  // Run memory test code at this point.
+  //
+  Status = gBS->LocateProtocol (
+  ,
+  NULL,
+  (VOID **) 
+  );
+
+  if (!EFI_ERROR (Status)) {
+Status = GenMemoryTest->MemoryTestInit (
+  GenMemoryTest,
+  IGNORE,
+  
+  );
+  } 
+
   //
   // Get current Boot Mode.
   //
diff --git 
a/Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
 
b/Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index 8e429fbf9f..9f476a14d0 100644
--- 
a/Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ 
b/Platform/BroxtonPlatformPkg/Common/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -92,6 +92,7 @@
   gEfiFormBrowser2ProtocolGuid
   gExitPmAuthProtocolGuid
   gEfiGraphicsOutputProtocolGuid
+  gEfiGenericMemTestProtocolGuid
   
 [Guids]
   gEfiGlobalVariableGuid
-- 
2.14.1.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute conversion

2018-04-02 Thread Zeng, Star 
Just thought more about the case.

If a memory range has WB attribute, and code is to set RUNTIME attribute, then 
the WB attribute will be clear in GCD database, it seems wrong. :(
For this case, need have OR operation ( | RUNTIME) to original attribute in GCD 
database?


Thanks,
Star
-Original Message-
From: Yao, Jiewen 
Sent: Tuesday, April 3, 2018 10:34 AM
To: Zeng, Star ; Wang, Jian J ; 
Kinney, Michael D ; Heyi Guo ; 
edk2-devel@lists.01.org
Cc: Ni, Ruiyu ; Yi Li ; Gao, 
Liming ; Dong, Eric ; Renhao Liang 

Subject: RE: [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute conversion

That is good idea.

Just add more description as the code comment, please.

It is easy for code review later.

Thank you
Yao Jiewen


> -Original Message-
> From: Zeng, Star
> Sent: Tuesday, April 3, 2018 10:33 AM
> To: Yao, Jiewen ; Wang, Jian J 
> ; Kinney, Michael D 
> ; Heyi Guo ; 
> edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Yi Li ; 
> Gao, Liming ; Dong, Eric ; 
> Renhao Liang ; Zeng, Star 
> 
> Subject: RE: [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute 
> conversion
> 
> Agree.
> 
> And I agree we need the fix at
> https://lists.01.org/pipermail/edk2-devel/2018-April/023364.html from 
> Mike for compatibility.
> And I think more precious and precise information need to be added 
> into the commit log or code comments since we have been crazy about 
> handling attributes in the GCD service. :)
> 
> 
> Thanks,
> Star
> -Original Message-
> From: Yao, Jiewen
> Sent: Tuesday, April 3, 2018 10:26 AM
> To: Wang, Jian J ; Zeng, Star 
> ; Kinney, Michael D ; 
> Heyi Guo ; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Yi Li ; 
> Gao, Liming ; Dong, Eric ; 
> Renhao Liang 
> Subject: RE: [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute 
> conversion
> 
> I have a discussion with Star/Jian.
> 
> The expectation for the CPU driver is to handle PageAttribute.
> The expectation for the platform driver is to use GetAttribute(), 
> AND/OR attribute, then call SetAttribute().
> Because the DRAM always has a cache attribute (no matter UC or WB), 0 
> should not happen. (0 might happen for a GCD reserved memory, but 
> there is no need to set page attribute)
> 
> If all drivers use above way, there won't be any issue. There is no 
> need to introduce a new protocol so far.
> 
> We did encounter some error, because the platform/silicon/CPU code is 
> not updated yet. (For example, the MinnowMax which is using binary) 
> The fix to filter 0 seems a workable way to resolve the compatibility issue.
> 
> Later, I suggest we update MinnowMax binary - Add paging handling for 
> CPU driver, and use GetAttribute()/SetAttribute() for platform/silicon code.
> 
> 
> Thank you
> Yao Jiewen
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf 
> > Of Wang, Jian J
> > Sent: Tuesday, April 3, 2018 9:24 AM
> > To: Zeng, Star ; Kinney, Michael D 
> > ; Heyi Guo ; 
> > edk2-devel@lists.01.org
> > Cc: Ni, Ruiyu ; Yi Li ; 
> > Gao, Liming ; Dong, Eric 
> > ; Renhao Liang 
> > Subject: Re: [edk2] [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of 
> > attribute conversion
> >
> > The NX Memory Protection and Heap Guard features need to clear 
> > paging attributes.
> > Allowing 0 attribute is the current only choice without changing arch 
> > protocol.
> > Maybe
> > It's time to introduce a new protocol.
> >
> > Regards,
> > Jian
> >
> >
> > > -Original Message-
> > > From: Zeng, Star
> > > Sent: Tuesday, April 03, 2018 9:16 AM
> > > To: Kinney, Michael D ; Heyi Guo 
> > > ; edk2-devel@lists.01.org
> > > Cc: Yi Li ; Renhao Liang 
> > > ; Dong, Eric ; Gao, 
> > > Liming ; Wang, Jian J 
> > > ; Ni, Ruiyu ; Zeng, 
> > > Star 
> > > Subject: RE: [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute 
> > > conversion
> > >
> > > Current gCpu->SetMemoryAttributes does not support getting memory 
> > > attributes, and has no clear description about clearing memory attributes.
> > >
>

Re: [edk2] [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute conversion

2018-04-02 Thread Yao, Jiewen 
That is good idea.

Just add more description as the code comment, please.

It is easy for code review later.

Thank you
Yao Jiewen


> -Original Message-
> From: Zeng, Star
> Sent: Tuesday, April 3, 2018 10:33 AM
> To: Yao, Jiewen ; Wang, Jian J ;
> Kinney, Michael D ; Heyi Guo
> ; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Yi Li ; Gao,
> Liming ; Dong, Eric ; Renhao
> Liang ; Zeng, Star 
> Subject: RE: [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute conversion
> 
> Agree.
> 
> And I agree we need the fix at
> https://lists.01.org/pipermail/edk2-devel/2018-April/023364.html from Mike
> for compatibility.
> And I think more precious and precise information need to be added into the
> commit log or code comments since we have been crazy about handling
> attributes in the GCD service. :)
> 
> 
> Thanks,
> Star
> -Original Message-
> From: Yao, Jiewen
> Sent: Tuesday, April 3, 2018 10:26 AM
> To: Wang, Jian J ; Zeng, Star ;
> Kinney, Michael D ; Heyi Guo
> ; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Yi Li ; Gao,
> Liming ; Dong, Eric ; Renhao
> Liang 
> Subject: RE: [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute conversion
> 
> I have a discussion with Star/Jian.
> 
> The expectation for the CPU driver is to handle PageAttribute.
> The expectation for the platform driver is to use GetAttribute(), AND/OR
> attribute, then call SetAttribute().
> Because the DRAM always has a cache attribute (no matter UC or WB), 0 should
> not happen. (0 might happen for a GCD reserved memory, but there is no need
> to set page attribute)
> 
> If all drivers use above way, there won't be any issue. There is no need to
> introduce a new protocol so far.
> 
> We did encounter some error, because the platform/silicon/CPU code is not
> updated yet. (For example, the MinnowMax which is using binary) The fix to
> filter 0 seems a workable way to resolve the compatibility issue.
> 
> Later, I suggest we update MinnowMax binary - Add paging handling for CPU
> driver, and use GetAttribute()/SetAttribute() for platform/silicon code.
> 
> 
> Thank you
> Yao Jiewen
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Wang, Jian J
> > Sent: Tuesday, April 3, 2018 9:24 AM
> > To: Zeng, Star ; Kinney, Michael D
> > ; Heyi Guo ;
> > edk2-devel@lists.01.org
> > Cc: Ni, Ruiyu ; Yi Li ;
> > Gao, Liming ; Dong, Eric ;
> > Renhao Liang 
> > Subject: Re: [edk2] [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute
> > conversion
> >
> > The NX Memory Protection and Heap Guard features need to clear paging
> > attributes.
> > Allowing 0 attribute is the current only choice without changing arch 
> > protocol.
> > Maybe
> > It's time to introduce a new protocol.
> >
> > Regards,
> > Jian
> >
> >
> > > -Original Message-
> > > From: Zeng, Star
> > > Sent: Tuesday, April 03, 2018 9:16 AM
> > > To: Kinney, Michael D ; Heyi Guo
> > > ; edk2-devel@lists.01.org
> > > Cc: Yi Li ; Renhao Liang
> > > ; Dong, Eric ; Gao,
> > > Liming ; Wang, Jian J ;
> > > Ni, Ruiyu ; Zeng, Star 
> > > Subject: RE: [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute
> > > conversion
> > >
> > > 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\SmmMemoryAttribut
> > e.
> > > 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 ; Heyi Guo
> > > ; edk2-devel@lists.01.org
> > > Cc: Yi Li ; Renhao Liang
> > > ; Dong, Eric ; Gao,
> > > Liming ; Wang, Jian J ;
> > > Ni, Ruiyu ; Zeng, Star 
> > > Subject: RE: [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute
> > >

Re: [edk2] [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute conversion

2018-04-02 Thread Zeng, Star 
Agree.

And I agree we need the fix at 
https://lists.01.org/pipermail/edk2-devel/2018-April/023364.html from Mike for 
compatibility.
And I think more precious and precise information need to be added into the 
commit log or code comments since we have been crazy about handling attributes 
in the GCD service. :)


Thanks,
Star
-Original Message-
From: Yao, Jiewen 
Sent: Tuesday, April 3, 2018 10:26 AM
To: Wang, Jian J ; Zeng, Star ; 
Kinney, Michael D ; Heyi Guo ; 
edk2-devel@lists.01.org
Cc: Ni, Ruiyu ; Yi Li ; Gao, 
Liming ; Dong, Eric ; Renhao Liang 

Subject: RE: [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute conversion

I have a discussion with Star/Jian.

The expectation for the CPU driver is to handle PageAttribute.
The expectation for the platform driver is to use GetAttribute(), AND/OR 
attribute, then call SetAttribute().
Because the DRAM always has a cache attribute (no matter UC or WB), 0 should 
not happen. (0 might happen for a GCD reserved memory, but there is no need to 
set page attribute)

If all drivers use above way, there won't be any issue. There is no need to 
introduce a new protocol so far.

We did encounter some error, because the platform/silicon/CPU code is not 
updated yet. (For example, the MinnowMax which is using binary) The fix to 
filter 0 seems a workable way to resolve the compatibility issue.

Later, I suggest we update MinnowMax binary - Add paging handling for CPU 
driver, and use GetAttribute()/SetAttribute() for platform/silicon code.


Thank you
Yao Jiewen

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Wang, Jian J
> Sent: Tuesday, April 3, 2018 9:24 AM
> To: Zeng, Star ; Kinney, Michael D 
> ; Heyi Guo ; 
> edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Yi Li ; 
> Gao, Liming ; Dong, Eric ; 
> Renhao Liang 
> Subject: Re: [edk2] [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute 
> conversion
> 
> The NX Memory Protection and Heap Guard features need to clear paging 
> attributes.
> Allowing 0 attribute is the current only choice without changing arch 
> protocol.
> Maybe
> It's time to introduce a new protocol.
> 
> Regards,
> Jian
> 
> 
> > -Original Message-
> > From: Zeng, Star
> > Sent: Tuesday, April 03, 2018 9:16 AM
> > To: Kinney, Michael D ; Heyi Guo 
> > ; edk2-devel@lists.01.org
> > Cc: Yi Li ; Renhao Liang 
> > ; Dong, Eric ; Gao, 
> > Liming ; Wang, Jian J ; 
> > Ni, Ruiyu ; Zeng, Star 
> > Subject: RE: [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute 
> > conversion
> >
> > 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\SmmMemoryAttribut
> e.
> > 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 ; Heyi Guo 
> > ; edk2-devel@lists.01.org
> > Cc: Yi Li ; Renhao Liang 
> > ; Dong, Eric ; Gao, 
> > Liming ; Wang, Jian J ; 
> > Ni, Ruiyu ; Zeng, Star 
> > 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 ; Heyi Guo 
> > ; edk2- de...@lists.01.org; Kinney, Michael D 
> > 
> > Cc: Yi Li

Re: [edk2] [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute conversion

2018-04-02 Thread Yao, Jiewen 
I have a discussion with Star/Jian.

The expectation for the CPU driver is to handle PageAttribute.
The expectation for the platform driver is to use GetAttribute(), AND/OR 
attribute, then call SetAttribute().
Because the DRAM always has a cache attribute (no matter UC or WB), 0 should 
not happen. (0 might happen for a GCD reserved memory, but there is no need to 
set page attribute)

If all drivers use above way, there won't be any issue. There is no need to 
introduce a new protocol so far.

We did encounter some error, because the platform/silicon/CPU code is not 
updated yet. (For example, the MinnowMax which is using binary)
The fix to filter 0 seems a workable way to resolve the compatibility issue.

Later, I suggest we update MinnowMax binary - Add paging handling for CPU 
driver, and use GetAttribute()/SetAttribute() for platform/silicon code.


Thank you
Yao Jiewen

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Wang,
> Jian J
> Sent: Tuesday, April 3, 2018 9:24 AM
> To: Zeng, Star ; Kinney, Michael D
> ; Heyi Guo ;
> edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Yi Li ; Gao,
> Liming ; Dong, Eric ; Renhao
> Liang 
> Subject: Re: [edk2] [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute
> conversion
> 
> The NX Memory Protection and Heap Guard features need to clear paging
> attributes.
> Allowing 0 attribute is the current only choice without changing arch 
> protocol.
> Maybe
> It's time to introduce a new protocol.
> 
> Regards,
> Jian
> 
> 
> > -Original Message-
> > From: Zeng, Star
> > Sent: Tuesday, April 03, 2018 9:16 AM
> > To: Kinney, Michael D ; Heyi Guo
> > ; edk2-devel@lists.01.org
> > Cc: Yi Li ; Renhao Liang
> > ; Dong, Eric ; Gao, Liming
> > ; Wang, Jian J ; Ni, Ruiyu
> > ; Zeng, Star 
> > Subject: RE: [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute conversion
> >
> > 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\SmmMemoryAttribut
> e.
> > 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 ; Heyi Guo
> > ; edk2-devel@lists.01.org
> > Cc: Yi Li ; Renhao Liang
> > ; Dong, Eric ; Gao, Liming
> > ; Wang, Jian J ; Ni, Ruiyu
> > ; Zeng, Star 
> > 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 ; Heyi Guo ; edk2-
> > de...@lists.01.org; Kinney, Michael D 
> > Cc: Yi Li ; Renhao Liang
> > ; Dong, Eric ; Gao, Liming
> > ; Wang, Jian J ; Ni, Ruiyu
> > 
> > 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

Re: [edk2] [PATCH 0/2] Refine some comments about SmmMemoryAttribute

2018-04-02 Thread Wang, Jian J 

For this series:
Reviewed-by: Jian J Wang 

> -Original Message-
> From: Zeng, Star
> Sent: Tuesday, April 03, 2018 9:52 AM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star ; Wang, Jian J ;
> Yao, Jiewen ; Dong, Eric ; Laszlo
> Ersek 
> Subject: [PATCH 0/2] Refine some comments about SmmMemoryAttribute
> 
> 1. Fix some "support" to "supported".
> 2. Fix some "set" to "clear" in ClearMemoryAttributes interface.
> 3. Remove redundant comments for GetMemoryAttributes interface.
> 
> Cc: Jian J Wang 
> Cc: Jiewen Yao 
> Cc: Eric Dong 
> Cc: Laszlo Ersek 
> 
> Star Zeng (2):
>   MdeModulePkg SmmMemoryAttribute.h: Refine some comments
>   UefiCpuPkg PiSmmCpuDxeSmm: Refine some comments about
> SmmMemoryAttribute
> 
>  MdeModulePkg/Include/Protocol/SmmMemoryAttribute.h | 15 ++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 15 ++-
>  UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 21
> +
>  3 files changed, 21 insertions(+), 30 deletions(-)
> 
> --
> 2.7.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 1/2] MdeModulePkg SmmMemoryAttribute.h: Refine some comments

2018-04-02 Thread Star Zeng 
1. Fix some "support" to "supported".
2. Fix some "set" to "clear" in ClearMemoryAttributes interface.
3. Remove redundant comments for GetMemoryAttributes interface.

Cc: Jian J Wang 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 MdeModulePkg/Include/Protocol/SmmMemoryAttribute.h | 15 ++-
 1 file changed, 6 insertions(+), 9 deletions(-)

diff --git a/MdeModulePkg/Include/Protocol/SmmMemoryAttribute.h 
b/MdeModulePkg/Include/Protocol/SmmMemoryAttribute.h
index 0700eb51d6e1..012fa2aaec6e 100644
--- a/MdeModulePkg/Include/Protocol/SmmMemoryAttribute.h
+++ b/MdeModulePkg/Include/Protocol/SmmMemoryAttribute.h
@@ -2,7 +2,7 @@
   SMM Memory Attribute Protocol provides retrieval and update service
   for memory attributes in EFI SMM environment.
 
-  Copyright (c) 2017, Intel Corporation. All rights reserved.
+  Copyright (c) 2017 - 2018, Intel Corporation. All rights reserved.
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
   which accompanies this distribution.  The full text of the license may be 
found at
@@ -42,7 +42,7 @@ typedef struct _EDKII_SMM_MEMORY_ATTRIBUTE_PROTOCOL 
EDKII_SMM_MEMORY_ATTRIBUTE_P
   @retval EFI_UNSUPPORTED   The processor does not support one or more
 bytes of the memory resource range specified
 by BaseAddress and Length.
-The bit mask of attributes is not support for
+The bit mask of attributes is not supported for
 the memory resource range specified by
 BaseAddress and Length.
 
@@ -64,17 +64,17 @@ EFI_STATUS
   @param  BaseAddress   The physical address that is the start address of
 a memory region.
   @param  LengthThe size in bytes of the memory region.
-  @param  AttributesThe bit mask of attributes to set for the memory
+  @param  AttributesThe bit mask of attributes to clear for the memory
 region.
 
-  @retval EFI_SUCCESS   The attributes were set for the memory region.
+  @retval EFI_SUCCESS   The attributes were clear for the memory 
region.
   @retval EFI_INVALID_PARAMETER Length is zero.
 Attributes specified an illegal combination of
 attributes that cannot be set together.
   @retval EFI_UNSUPPORTED   The processor does not support one or more
 bytes of the memory resource range specified
 by BaseAddress and Length.
-The bit mask of attributes is not support for
+The bit mask of attributes is not supported for
 the memory resource range specified by
 BaseAddress and Length.
 
@@ -89,7 +89,7 @@ EFI_STATUS
   );
 
 /**
-  This function retrieve the attributes of the memory region specified by
+  This function retrieves the attributes of the memory region specified by
   BaseAddress and Length. If different attributes are got from different part
   of the memory region, EFI_NO_MAPPING will be returned.
 
@@ -107,9 +107,6 @@ EFI_STATUS
   @retval EFI_UNSUPPORTED   The processor does not support one or more
 bytes of the memory resource range specified
 by BaseAddress and Length.
-The bit mask of attributes is not support for
-the memory resource range specified by
-BaseAddress and Length.
 
 **/
 typedef
-- 
2.7.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 0/2] Refine some comments about SmmMemoryAttribute

2018-04-02 Thread Star Zeng 
1. Fix some "support" to "supported".
2. Fix some "set" to "clear" in ClearMemoryAttributes interface.
3. Remove redundant comments for GetMemoryAttributes interface.

Cc: Jian J Wang 
Cc: Jiewen Yao 
Cc: Eric Dong 
Cc: Laszlo Ersek 

Star Zeng (2):
  MdeModulePkg SmmMemoryAttribute.h: Refine some comments
  UefiCpuPkg PiSmmCpuDxeSmm: Refine some comments about
SmmMemoryAttribute

 MdeModulePkg/Include/Protocol/SmmMemoryAttribute.h | 15 ++-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 15 ++-
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 21 +
 3 files changed, 21 insertions(+), 30 deletions(-)

-- 
2.7.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH 2/2] UefiCpuPkg PiSmmCpuDxeSmm: Refine some comments about SmmMemoryAttribute

2018-04-02 Thread Star Zeng 
1. Fix some "support" to "supported".
2. Fix some "set" to "clear" in ClearMemoryAttributes interface.
3. Remove redundant comments for GetMemoryAttributes interface.

Cc: Jian J Wang 
Cc: Jiewen Yao 
Cc: Eric Dong 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 15 ++-
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 21 +
 2 files changed, 15 insertions(+), 21 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index a2babb987732..af6437ee822b 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -1079,7 +1079,7 @@ TransferApToSafeState (
   @retval EFI_UNSUPPORTED   The processor does not support one or more
 bytes of the memory resource range specified
 by BaseAddress and Length.
-The bit mask of attributes is not support for
+The bit mask of attributes is not supported for
 the memory resource range specified by
 BaseAddress and Length.
 
@@ -1101,17 +1101,17 @@ EdkiiSmmSetMemoryAttributes (
   @param  BaseAddress   The physical address that is the start address of
 a memory region.
   @param  LengthThe size in bytes of the memory region.
-  @param  AttributesThe bit mask of attributes to set for the memory
+  @param  AttributesThe bit mask of attributes to clear for the memory
 region.
 
-  @retval EFI_SUCCESS   The attributes were set for the memory region.
+  @retval EFI_SUCCESS   The attributes were clear for the memory 
region.
   @retval EFI_INVALID_PARAMETER Length is zero.
 Attributes specified an illegal combination of
-attributes that cannot be set together.
+attributes that cannot be clear together.
   @retval EFI_UNSUPPORTED   The processor does not support one or more
 bytes of the memory resource range specified
 by BaseAddress and Length.
-The bit mask of attributes is not support for
+The bit mask of attributes is not supported for
 the memory resource range specified by
 BaseAddress and Length.
 
@@ -1126,7 +1126,7 @@ EdkiiSmmClearMemoryAttributes (
   );
 
 /**
-  This function retrieve the attributes of the memory region specified by
+  This function retrieves the attributes of the memory region specified by
   BaseAddress and Length. If different attributes are got from different part
   of the memory region, EFI_NO_MAPPING will be returned.
 
@@ -1144,9 +1144,6 @@ EdkiiSmmClearMemoryAttributes (
   @retval EFI_UNSUPPORTED   The processor does not support one or more
 bytes of the memory resource range specified
 by BaseAddress and Length.
-The bit mask of attributes is not support for
-the memory resource range specified by
-BaseAddress and Length.
 
 **/
 EFI_STATUS
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index 2a4a29899862..17515fbe079c 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -604,7 +604,7 @@ SmmClearMemoryAttributesEx (
 the memory resource range.
   @retval EFI_UNSUPPORTED   The processor does not support one or more 
bytes of the memory
 resource range specified by BaseAddress and 
Length.
-The bit mask of attributes is not support for 
the memory resource
+The bit mask of attributes is not supported 
for the memory resource
 range specified by BaseAddress and Length.
 
 **/
@@ -632,12 +632,12 @@ SmmSetMemoryAttributes (
 BaseAddress and Length cannot be modified.
   @retval EFI_INVALID_PARAMETER Length is zero.
 Attributes specified an illegal combination of 
attributes that
-cannot be set together.
+cannot be clear together.
   @retval EFI_OUT_OF_RESOURCES  There are not enough system resources to 
modify the attributes of

Re: [edk2] [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute conversion

2018-04-02 Thread Wang, Jian J 
The NX Memory Protection and Heap Guard features need to clear paging 
attributes.
Allowing 0 attribute is the current only choice without changing arch protocol. 
Maybe
It's time to introduce a new protocol.

Regards,
Jian


> -Original Message-
> From: Zeng, Star
> Sent: Tuesday, April 03, 2018 9:16 AM
> To: Kinney, Michael D ; Heyi Guo
> ; edk2-devel@lists.01.org
> Cc: Yi Li ; Renhao Liang
> ; Dong, Eric ; Gao, Liming
> ; Wang, Jian J ; Ni, Ruiyu
> ; Zeng, Star 
> Subject: RE: [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute conversion
> 
> 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 ; Heyi Guo
> ; edk2-devel@lists.01.org
> Cc: Yi Li ; Renhao Liang
> ; Dong, Eric ; Gao, Liming
> ; Wang, Jian J ; Ni, Ruiyu
> ; Zeng, Star 
> 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 ; Heyi Guo ; edk2-
> de...@lists.01.org; Kinney, Michael D 
> Cc: Yi Li ; Renhao Liang
> ; Dong, Eric ; Gao, Liming
> ; Wang, Jian J ; Ni, Ruiyu
> 
> 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 ; edk2- de...@lists.01.org
> > Cc: Yi Li ; Renhao Liang
> > ; Dong, Eric ; Kinney,
> > Michael D ; Gao, Liming
> > ; Wang, Jian J ; Ni,
> > Ruiyu ; Zeng, Star 
> > Subject: RE: [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute
> > conversion
> >
> > Pushed at 5b91bf82c67b586b9588cbe4bbffa1588f6b5926.
> >
> > Thanks,
> > Star
> > -Original Message-
> > From: Heyi Guo [mailto:heyi@linaro.org]
> > Sent: Thursday, March 29, 2018 4:20 PM
> > To: edk2-devel@lists.01.org
> > Cc: Heyi Guo ; Yi Li ;
> > Renhao Liang ; Zeng, Star
> > ; Dong, Eric ; Kinney,
> > Michael D ; Gao, Liming
> > ; Wang, Jian J ; Ni,
> > Ruiyu 
> > 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

Re: [edk2] [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute conversion

2018-04-02 Thread Zeng, Star 
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 ; Heyi Guo 
; edk2-devel@lists.01.org
Cc: Yi Li ; Renhao Liang ; 
Dong, Eric ; Gao, Liming ; Wang, 
Jian J ; Ni, Ruiyu ; Zeng, Star 

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 ; Heyi Guo ; 
edk2-devel@lists.01.org; Kinney, Michael D 
Cc: Yi Li ; Renhao Liang ; 
Dong, Eric ; Gao, Liming ; Wang, 
Jian J ; Ni, Ruiyu 
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 ; edk2- de...@lists.01.org
> Cc: Yi Li ; Renhao Liang 
> ; Dong, Eric ; Kinney, 
> Michael D ; Gao, Liming 
> ; Wang, Jian J ; Ni, 
> Ruiyu ; Zeng, Star 
> Subject: RE: [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute 
> conversion
> 
> Pushed at 5b91bf82c67b586b9588cbe4bbffa1588f6b5926.
> 
> Thanks,
> Star
> -Original Message-
> From: Heyi Guo [mailto:heyi@linaro.org]
> Sent: Thursday, March 29, 2018 4:20 PM
> To: edk2-devel@lists.01.org
> Cc: Heyi Guo ; Yi Li ; 
> Renhao Liang ; Zeng, Star 
> ; Dong, Eric ; Kinney, 
> Michael D ; Gao, Liming 
> ; Wang, Jian J ; Ni, 
> Ruiyu 
> 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 
> Signed-off-by: Yi Li 
> Signed-off-by: Renhao Liang 
> Cc: Star Zeng 
> Cc: Eric Dong 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Jian J Wang 
> Cc: Ruiyu Ni

Re: [edk2] [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute conversion

2018-04-02 Thread Zeng, Star 
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 ; Heyi Guo ; 
edk2-devel@lists.01.org; Kinney, Michael D 
Cc: Yi Li ; Renhao Liang ; 
Dong, Eric ; Gao, Liming ; Wang, 
Jian J ; Ni, Ruiyu 
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 ; edk2- de...@lists.01.org
> Cc: Yi Li ; Renhao Liang 
> ; Dong, Eric ; Kinney, 
> Michael D ; Gao, Liming 
> ; Wang, Jian J ; Ni, 
> Ruiyu ; Zeng, Star 
> Subject: RE: [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute 
> conversion
> 
> Pushed at 5b91bf82c67b586b9588cbe4bbffa1588f6b5926.
> 
> Thanks,
> Star
> -Original Message-
> From: Heyi Guo [mailto:heyi@linaro.org]
> Sent: Thursday, March 29, 2018 4:20 PM
> To: edk2-devel@lists.01.org
> Cc: Heyi Guo ; Yi Li ; 
> Renhao Liang ; Zeng, Star 
> ; Dong, Eric ; Kinney, 
> Michael D ; Gao, Liming 
> ; Wang, Jian J ; Ni, 
> Ruiyu 
> 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 
> Signed-off-by: Yi Li 
> Signed-off-by: Renhao Liang 
> Cc: Star Zeng 
> Cc: Eric Dong 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Jian J Wang 
> Cc: Ruiyu Ni 
> ---
>  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
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch] ShellPkg/dblk: Avoid overwriting Media->IoAlign

2018-04-02 Thread Kornel Pal 
According to the UEFI Specification, data values in EFI_BLOCK_IO_MEDIA 
are read-only.
This patch introduces a local variable to avoid overwriting IoAlign and 
reduces unnecessary overallocation.


Cc: Jaben Carsey 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Kornel Pal 
---
 ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c 
b/ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c

index 3632ca8a7a..b2cbae84f3 100644
--- a/ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c
+++ b/ShellPkg/Library/UefiShellDebug1CommandsLib/Dblk.c
@@ -39,6 +39,7 @@ DisplayTheBlocks(
   UINT8 *Buffer;
   UINT8 *OriginalBuffer;
   UINTN BufferSize;
+  UINT32    IoAlign;

   ShellStatus = SHELL_SUCCESS;

@@ -53,13 +54,14 @@ DisplayTheBlocks(
   }

   BufferSize = BlockIo->Media->BlockSize * BlockCount;
-  if(BlockIo->Media->IoAlign == 0) {
-    BlockIo->Media->IoAlign = 1;
+  IoAlign    = BlockIo->Media->IoAlign;
+  if (IoAlign == 0) {
+    IoAlign = 1;
   }

   if (BufferSize > 0) {
-    OriginalBuffer = AllocateZeroPool(BufferSize + 
BlockIo->Media->IoAlign);
-    Buffer = ALIGN_POINTER 
(OriginalBuffer,BlockIo->Media->IoAlign);

+    OriginalBuffer = AllocateZeroPool(BufferSize + IoAlign - 1);
+    Buffer = ALIGN_POINTER (OriginalBuffer, IoAlign);
   } else {
 ShellPrintEx(-1,-1,L"  BlockSize: 0x%08x, BlockCount: 0x%08x\r\n", 
BlockIo->Media->BlockSize, BlockCount);

 OriginalBuffer = NULL;
--
2.15.1.windows.2

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch] MdeModulePkg/Gcd: Filter gCpu->SetMemoryAttributes() calls

2018-04-02 Thread Kinney, Michael D 
This patch fixes an issue with VlvTbltDevicePkg introduced
by commit:

https://github.com/tianocore/edk2/commit/5b91bf82c67b586b9588cbe4bbffa1588f6b5926

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 
Cc: Yi Li 
Cc: Renhao Liang 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Liming Gao 
Cc: Jian J Wang 
Cc: Ruiyu Ni 
Signed-off-by: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1
---
 MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c
index 907245a3f5..45ed6280db 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   0x
-
 //
 // Module Variables
 //
@@ -873,7 +871,7 @@ CoreConvertSpace (
 // Call CPU Arch Protocol to attempt to set attributes on the range
 //
 CpuArchAttributes = ConverToCpuArchAttributes (Attributes);
-if (CpuArchAttributes != INVALID_CPU_ARCH_ATTRIBUTES) {
+if (CpuArchAttributes != 0) {
   if (gCpu == NULL) {
 Status = EFI_NOT_AVAILABLE_YET;
   } else {
-- 
2.14.2.windows.3

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] OVMF UsbBulkTransfer returns EFI_OUT_OF_RESOURCES

2018-04-02 Thread Rob Taglang 

Hello,

I can pass a host USB device to QEMU boot with OVMF, and it shows up as 
a EFI_USB_IO_PROTOCOL device and the interface descriptors and 
endpoints are detected correctly. A UsbControlTransfer operation 
succeeds. However, UsbBulkTransfer returns EFI_OUT_OF_RESOURCES 
regardless of how much memory I allocate for QEMU.


This application does work correctly on real hardware. Is this expected 
behavior in OVMF?


Thanks,
Rob
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of attribute conversion

2018-04-02 Thread Kinney, Michael D 
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 ; edk2-
> de...@lists.01.org
> Cc: Yi Li ; Renhao Liang
> ; Dong, Eric
> ; Kinney, Michael D
> ; Gao, Liming
> ; Wang, Jian J
> ; Ni, Ruiyu ;
> Zeng, Star 
> Subject: RE: [PATCH 1/1] MdeModulePkg/Gcd: Fix bug of
> attribute conversion
> 
> Pushed at 5b91bf82c67b586b9588cbe4bbffa1588f6b5926.
> 
> Thanks,
> Star
> -Original Message-
> From: Heyi Guo [mailto:heyi@linaro.org]
> Sent: Thursday, March 29, 2018 4:20 PM
> To: edk2-devel@lists.01.org
> Cc: Heyi Guo ; Yi Li
> ; Renhao Liang
> ; Zeng, Star
> ; Dong, Eric ;
> Kinney, Michael D ; Gao,
> Liming ; Wang, Jian J
> ; Ni, Ruiyu 
> 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 
> Signed-off-by: Yi Li 
> Signed-off-by: Renhao Liang 
> Cc: Star Zeng 
> Cc: Eric Dong 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Jian J Wang 
> Cc: Ruiyu Ni 
> ---
>  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
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch] Build Spec: update chapter 2.7's format

2018-04-02 Thread Gao, Liming 
Reviewed-by: Liming Gao 

>-Original Message-
>From: Zhu, Yonghong
>Sent: Monday, April 02, 2018 4:47 PM
>To: edk2-devel@lists.01.org
>Cc: Gao, Liming ; Kinney, Michael D
>; Shaw, Kevin W 
>Subject: [Patch] Build Spec: update chapter 2.7's format
>
>This patch update chapter 2.7's format since we found the PDF of this
>section is ugly.
>
>Cc: Liming Gao 
>Cc: Michael Kinney 
>Cc: Kevin W Shaw 
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Yonghong Zhu 
>---
> 2_design_discussion/27_sku_support.md | 130
>--
> 1 file changed, 92 insertions(+), 38 deletions(-)
>
>diff --git a/2_design_discussion/27_sku_support.md
>b/2_design_discussion/27_sku_support.md
>index 891fc93..83f9f87 100644
>--- a/2_design_discussion/27_sku_support.md
>+++ b/2_design_discussion/27_sku_support.md
>@@ -31,115 +31,169 @@
>
> ## 2.7 SKU Support
>
> The EDK II build system provides the capability of supporting multiple SKUs in
> a single firmware image. SKU selection is a runtime option that can be set
>from
>-a platform driver. The build system also supports building a specific SKU. The
>-SKU enabled PCD sections (defined by a PCD section tag in the DSC file that
>+a platform driver. The build system also supports building a specific SKU.
>+
>+The SKU enabled PCD sections (defined by a PCD section tag in the DSC file
>that
> contains a SKUID modifier that is not DEFAULT) are a sparsely populated set
>of
> configuration settings. The platform developer may specify one or more PCDs
>that
> will have different values than the PCD values specified by the default SKU.
> Additional PCDs not listed in a default PCD section may also be specified
>under
>-a section with a SKUID modifier.
>+a section with a SKUID modifier.
>+
> During runtime, the PCD drivers will automatically return a default SKU value
> if no specific SKU value was specified after a platform driver calls SetSku().
> The configuration elements, PCDs, must be accessed using either Dynamic or
> DynamicEx PCD access methods. When building and image that supports
>multiple
> SKUs, the Feature Flag, Fixed At Build and Patchable In Module PCDs will only
> use the default SKU configuration settings. The default configuration settings
> are identified by PCD section tags that have either a Default SKUID modifier
> or have not SKUID modifier. The set of SKUs that can be included is
>configurable
>-either through setting the list in the DSC file [Defines] section's
>SKU_IDENTIFIER
>-element or through setting one or more -x SKUID command-line options to
>the build
>-command.
>+either through setting the list in the DSC file `[Defines]` section's
>+`SKU_IDENTIFIER` element or through setting one or more -x SKUID
>command-line
>+options to the build command.
>+
> When building a single SKU, it is possible to use SKU specific Feature Flag,
>Fixed
> At Build and Patchable In Module configuration elements along with the
>Dynamic and
> DynamicEx PCD for the specific SKU. The build tools will automatically adjust
>the
> SKU specific Dynamic and DynamicEx PCD values overriding the default values.
>This
> is equivalent of running SetSku immediately on reset.
>
>+**
> **Note:** If there are no PcdsDynamic or PcdsDynamicEx section tags that
>use a
> SkuId modifier, then only the DEFAULT values will be placed into the PCD
>Database.
> The platform drivers must not call SetSku() for this single SKU, as the
>'DEFAULT'
> SKU will be the only SKU available when built by this method.
>+**
>
> The following examples show the three types of builds.
>-DEFAULT SKUID Build
>+
>+**DEFAULT SKUID Build**
> One or more SKUID | SKUIDENTIFIER entries may appear in the DSC file's
>[SkuIds]
> section as in the following example:
>+
>+```
> [SkuIds]
>   0 | DEFAULT
>   1 | ScsiSku
>   2 | SataSku
>   3 | iScsiSku
>-Only the DEFAULT SKU will be built as identified by a single entry in the
>-SKU_IDENTIFIER in the DSC file's [Defines] section as in the following
>example:
>-  SKU_IDENTIFIER = DEFAULT
>+```
>+
>+Only the `DEFAULT` SKU will be built as identified by a single entry in the
>+`SKU_IDENTIFIER` in the DSC file's `[Defines]` section as in the following
>+example:
>+
>+  `SKU_IDENTIFIER = DEFAULT`
>+
> The user is not required to specify:
>-  -x DEFAULT
>+
>+  `-x DEFAULT`
>+
> FeatureFlag, PatchableInModule and FixedAtBuild PCD values from the
>values in
>-the default PCD sections. Dynamic and DynamicEx PCD values from the
>default
>-PCD sections (sections that do not contain a SkuId modifier in the section tag
>-or that contain a SkuId modifier of DEFAULT) must be used. These values will
>-then be placed in the DEFAULT table of the PCD Database.
>+the default PCD sections.
>+
>+Dynamic and DynamicEx PCD values from the default PCD

Re: [edk2] [Patch] Build spec: Add some clarification and clean up for build report

2018-04-02 Thread Gao, Liming 
Reviewed-by: Liming Gao 

>-Original Message-
>From: Zhu, Yonghong
>Sent: Monday, April 02, 2018 11:30 AM
>To: edk2-devel@lists.01.org
>Cc: Gao, Liming ; Kinney, Michael D
>; Shaw, Kevin W 
>Subject: [Patch] Build spec: Add some clarification and clean up for build
>report
>
>Add some clarification and clean up for build report section.
>
>Cc: Liming Gao 
>Cc: Michael Kinney 
>Cc: Kevin W Shaw 
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Yonghong Zhu 
>---
> 13_build_reports/134_platform_summary.md   | 44 +++-
>--
> 13_build_reports/136_global_pcd_section.md |  7 +++--
> 13_build_reports/138_module_section.md | 11 +---
> 3 files changed, 27 insertions(+), 35 deletions(-)
>
>diff --git a/13_build_reports/134_platform_summary.md
>b/13_build_reports/134_platform_summary.md
>index 09b8db6..23964af 100644
>--- a/13_build_reports/134_platform_summary.md
>+++ b/13_build_reports/134_platform_summary.md
>@@ -1,9 +1,9 @@
> 2.6.1.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [Patch] Build Spec: update chapter 2.7's format

2018-04-02 Thread Yonghong Zhu 
This patch update chapter 2.7's format since we found the PDF of this
section is ugly.

Cc: Liming Gao 
Cc: Michael Kinney 
Cc: Kevin W Shaw 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
---
 2_design_discussion/27_sku_support.md | 130 --
 1 file changed, 92 insertions(+), 38 deletions(-)

diff --git a/2_design_discussion/27_sku_support.md 
b/2_design_discussion/27_sku_support.md
index 891fc93..83f9f87 100644
--- a/2_design_discussion/27_sku_support.md
+++ b/2_design_discussion/27_sku_support.md
@@ -31,115 +31,169 @@
 
 ## 2.7 SKU Support
 
 The EDK II build system provides the capability of supporting multiple SKUs in
 a single firmware image. SKU selection is a runtime option that can be set from
-a platform driver. The build system also supports building a specific SKU. The
-SKU enabled PCD sections (defined by a PCD section tag in the DSC file that
+a platform driver. The build system also supports building a specific SKU.
+
+The SKU enabled PCD sections (defined by a PCD section tag in the DSC file that
 contains a SKUID modifier that is not DEFAULT) are a sparsely populated set of
 configuration settings. The platform developer may specify one or more PCDs 
that
 will have different values than the PCD values specified by the default SKU.
 Additional PCDs not listed in a default PCD section may also be specified under
-a section with a SKUID modifier. 
+a section with a SKUID modifier.
+
 During runtime, the PCD drivers will automatically return a default SKU value
 if no specific SKU value was specified after a platform driver calls SetSku().
 The configuration elements, PCDs, must be accessed using either Dynamic or
 DynamicEx PCD access methods. When building and image that supports multiple
 SKUs, the Feature Flag, Fixed At Build and Patchable In Module PCDs will only
 use the default SKU configuration settings. The default configuration settings
 are identified by PCD section tags that have either a Default SKUID modifier
 or have not SKUID modifier. The set of SKUs that can be included is 
configurable
-either through setting the list in the DSC file [Defines] section's 
SKU_IDENTIFIER
-element or through setting one or more -x SKUID command-line options to the 
build
-command.
+either through setting the list in the DSC file `[Defines]` section's 
+`SKU_IDENTIFIER` element or through setting one or more -x SKUID command-line
+options to the build command.
+
 When building a single SKU, it is possible to use SKU specific Feature Flag, 
Fixed
 At Build and Patchable In Module configuration elements along with the Dynamic 
and
 DynamicEx PCD for the specific SKU. The build tools will automatically adjust 
the
 SKU specific Dynamic and DynamicEx PCD values overriding the default values. 
This
 is equivalent of running SetSku immediately on reset.
 
+**
 **Note:** If there are no PcdsDynamic or PcdsDynamicEx section tags that use a
 SkuId modifier, then only the DEFAULT values will be placed into the PCD 
Database.
 The platform drivers must not call SetSku() for this single SKU, as the 
'DEFAULT'
 SKU will be the only SKU available when built by this method.
+**
 
 The following examples show the three types of builds.
-DEFAULT SKUID Build
+
+**DEFAULT SKUID Build**
 One or more SKUID | SKUIDENTIFIER entries may appear in the DSC file's [SkuIds]
 section as in the following example:
+
+```
 [SkuIds]
   0 | DEFAULT
   1 | ScsiSku
   2 | SataSku
   3 | iScsiSku
-Only the DEFAULT SKU will be built as identified by a single entry in the
-SKU_IDENTIFIER in the DSC file's [Defines] section as in the following example:
-  SKU_IDENTIFIER = DEFAULT
+```
+
+Only the `DEFAULT` SKU will be built as identified by a single entry in the
+`SKU_IDENTIFIER` in the DSC file's `[Defines]` section as in the following
+example:
+
+  `SKU_IDENTIFIER = DEFAULT`
+
 The user is not required to specify:
-  -x DEFAULT
+
+  `-x DEFAULT`
+
 FeatureFlag, PatchableInModule and FixedAtBuild PCD values from the values in
-the default PCD sections. Dynamic and DynamicEx PCD values from the default
-PCD sections (sections that do not contain a SkuId modifier in the section tag
-or that contain a SkuId modifier of DEFAULT) must be used. These values will
-then be placed in the DEFAULT table of the PCD Database.
+the default PCD sections. 
+
+Dynamic and DynamicEx PCD values from the default PCD sections (sections that
+do not contain a SkuId modifier in the section tag or that contain a SkuId
+modifier of DEFAULT) must be used. These values will then be placed in the
+DEFAULT table of the PCD Database.
+
+**
 **WARNING:** The platform drivers must not call SetSku() for this single SKU, 
as
 the 'DEFAULT' SKU will be the only SKU available when built by this method.
+**
+
 
-Single SKUID Build
+**Single SKUID Build**
 This method is

Re: [edk2] OVMF package : Question about Qemu/Xen support

2018-04-02 Thread Tiger Liu(BJ-RD) 
Hi, Laszlo:
I have a question you would like to ask you.
Could I study pcie native hot plug feature with ovmf?

It seems qemu emulates Q35 chipset.
And ovmf provides PciHotPlugInitDxe driver.

Thanks

Best wishes,
-邮件原件-
发件人: Laszlo Ersek [mailto:ler...@redhat.com]
发送时间: 2018年3月5日 17:15
收件人: Tiger Liu(BJ-RD) 
抄送: 'edk2-devel@lists.01.org' ; Anthony Perard 
; Julien Grall 
主题: Re: [edk2] OVMF package : Question about Qemu/Xen support

On 03/05/18 06:49, Tiger Liu(BJ-RD) wrote:
> Hi, experts:
> I have a question about Ovmf.
>
> Must Ovmf be run with qemu tool?
> Or It could be run with Xen without needed qemu software.
>
> It seems Xen began to support uefi boot from 4.4 version.

- From the Xen wiki:

https://wiki.xenproject.org/wiki/OVMF

> One thing to have in mind is Xen supports both its own QEMU fork
> called qemu-traditional and upstream QEMU called qemu-xen. OVMF only
> supports the latter. Xen 4.4 has upstream QEMU configured for all HVM
> guests by default, so it is fine to not specify which QEMU to use in
> guest config file. But if you have already configured qemu-traditional
> for your guest you would need to delete / comment out that line.

See also

https://wiki.xenproject.org/wiki/Xen_Project_Release_Features#Device_Models_and_Virtual_Firmware

- I'm also CC'ing the OVMF reviewers for Xen (from the "Maintainers.txt"
file).

Thanks
Laszlo


保密声明：
本邮件含有保密或专有信息，仅供指定收件人使用。严禁对本邮件或其内容做任何未经授权的查阅、使用、复制或转发。
CONFIDENTIAL NOTE:
This email contains confidential or legally privileged information and is for 
the sole use of its intended recipient. Any unauthorized review, use, copying 
or forwarding of this email or the content of this email is strictly prohibited.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel