[edk2] [Patch] Maintainers.txt: Update BaseTools maintainers

2018-11-29 Thread Yonghong Zhu
As Yonghong has some other focus, change him from maintainer
to reviewer, Bob will be the new maintainer.

Cc: Bob Feng 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yonghong Zhu 
---
 Maintainers.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Maintainers.txt b/Maintainers.txt
index 8a9f8df..2031db6 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -82,12 +82,13 @@ M: Laszlo Ersek 
 M: Ard Biesheuvel 
 R: Julien Grall 
 
 BaseTools
 W: https://github.com/tianocore/tianocore.github.io/wiki/BaseTools
-M: Yonghong Zhu 
+M: Bob Feng 
 M: Liming Gao 
+R: Yonghong Zhu 
 
 BeagleBoardPkg
 W: https://github.com/tianocore/tianocore.github.io/wiki/BeagleBoardPkg
 M: Leif Lindholm 
 M: Ard Biesheuvel 
-- 
2.6.1.windows.1

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


[edk2] [edk2-test][Patch 2/3] uefi-sct/SctPkg:Add VerifySignature() Func Test

2018-11-29 Thread Eric Jin
Cc: Supreeth Venkatesh 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Jin 
---
 .../EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c   |2 +
 .../EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h   |7 +
 .../PKCS7Verify/BlackBoxTest/Pkcs7BBTestData.c | 1466 +---
 .../PKCS7Verify/BlackBoxTest/Pkcs7BBTestFunction.c |   87 +-
 .../PKCS7Verify/BlackBoxTest/Pkcs7BBTestMain.c |   52 +-
 .../PKCS7Verify/BlackBoxTest/Pkcs7BBTestMain.h |   22 +-
 6 files changed, 1080 insertions(+), 556 deletions(-)

diff --git 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c
index 142f6d4..4d433c3 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c
@@ -42,3 +42,5 @@ EFI_GUID gPkcs7BBTestFunctionAssertionGuid001 = 
EFI_TEST_PKCS7BBTESTFUNCTION_ASS
 EFI_GUID gPkcs7BBTestFunctionAssertionGuid002 = 
EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_002_GUID;
 EFI_GUID gPkcs7BBTestFunctionAssertionGuid003 = 
EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_003_GUID;
 EFI_GUID gPkcs7BBTestFunctionAssertionGuid004 = 
EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_004_GUID;
+EFI_GUID gPkcs7BBTestFunctionAssertionGuid005 = 
EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_005_GUID;
+EFI_GUID gPkcs7BBTestFunctionAssertionGuid006 = 
EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_006_GUID;
diff --git 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h
index 2b5cd85..32a00f6 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h
@@ -82,4 +82,11 @@ extern EFI_GUID gPkcs7BBTestFunctionAssertionGuid003;
 { 0x912e23ef, 0x299c, 0x41ab, {0xa0, 0xf5, 0xfc, 0xbc, 0xf6, 0xfd, 0xd3, 0x32 
}}
 extern EFI_GUID gPkcs7BBTestFunctionAssertionGuid004;
 
+#define EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_005_GUID \
+{ 0x93740b06, 0xa186, 0x47ff, { 0xba, 0xc3, 0xdd, 0xa8, 0xcb, 0x7b, 0x18, 0x5e 
}}
+extern EFI_GUID gPkcs7BBTestFunctionAssertionGuid005;
+
+#define EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_006_GUID \
+{ 0x37253616, 0xca42, 0x4082, { 0x90, 0xda, 0xdb, 0x69, 0x98, 0x22, 0xa0, 0xe6 
}}
+extern EFI_GUID gPkcs7BBTestFunctionAssertionGuid006;
 
diff --git 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Pkcs7BBTestData.c
 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Pkcs7BBTestData.c
index 0511e00..9b66938 100644
--- 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Pkcs7BBTestData.c
+++ 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Pkcs7BBTestData.c
@@ -25,541 +25,979 @@ Abstract:
 --*/
 
 //
+// Test Root Certificate ("TestRoot.cer")
+//
+GLOBAL_REMOVE_IF_UNREFERENCED UINT8 TestRootCert[781] = {
+  0x30, 0x82, 0x03, 0x09, 0x30, 0x82, 0x01, 0xF1, 0xA0, 0x03, 0x02, 0x01,
+  0x02, 0x02, 0x10, 0xDE, 0x9F, 0x42, 0x91, 0x68, 0x16, 0xEA, 0x97, 0x4D,
+  0xA1, 0x8A, 0x32, 0x25, 0xD6, 0xEE, 0x8D, 0x30, 0x0D, 0x06, 0x09, 0x2A,
+  0x86, 0x48, 0x86, 0xF7, 0x0D, 0x01, 0x01, 0x0B, 0x05, 0x00, 0x30, 0x13,
+  0x31, 0x11, 0x30, 0x0F, 0x06, 0x03, 0x55, 0x04, 0x03, 0x13, 0x08, 0x54,
+  0x65, 0x73, 0x74, 0x52, 0x6F, 0x6F, 0x74, 0x30, 0x1E, 0x17, 0x0D, 0x31,
+  0x38, 0x30, 0x31, 0x32, 0x35, 0x30, 0x32, 0x30, 0x35, 0x35, 0x30, 0x5A,
+  0x17, 0x0D, 0x33, 0x39, 0x31, 0x32, 0x33, 0x31, 0x32, 0x33, 0x35, 0x39,
+  0x35, 0x39, 0x5A, 0x30, 0x13, 0x31, 0x11, 0x30, 0x0F, 0x06, 0x03, 0x55,
+  0x04, 0x03, 0x13, 0x08, 0x54, 0x65, 0x73, 0x74, 0x52, 0x6F, 0x6F, 0x74,
+  0x30, 0x82, 0x01, 0x22, 0x30, 0x0D, 0x06, 0x09, 0x2A, 0x86, 0x48, 0x86,
+  0xF7, 0x0D, 0x01, 0x01, 0x01, 0x05, 0x00, 0x03, 0x82, 0x01, 0x0F, 0x00,
+  0x30, 0x82, 0x01, 0x0A, 0x02, 0x82, 0x01, 0x01, 0x00, 0xA5, 0x97, 0x23,
+  0x48, 0xBE, 0xCA, 0xC8, 0xE0, 0x88, 0xC6, 0xA2, 0xAF, 0x78, 0x60, 0x94,
+  0x48, 0x3E, 0x82, 0xE7, 0xD5, 0x62, 0x01, 0x73, 0x00, 0xEA, 0x42, 0x7A,
+  0x32, 0x0A, 0xD7, 0x3F, 0x4D, 0x0B, 0x71, 0x6D, 0xD3, 0x50, 0x5E, 0x26,
+  0x20, 0xE8, 0xCC, 0xB6, 0x0A, 0xAF, 0xD9, 0x07, 0x22, 0x17, 0x45, 0xD8,
+  0x91, 0x75, 0x75, 0x52, 0xD8, 0x8C, 0xAB, 0x63, 0x0A, 0xF0, 0x23, 0x14,
+  0x34, 0x92, 0x3F, 0xE0, 0x05, 0x24, 0x28, 0xED, 0x74, 0x8E, 0x4D, 0x3E,
+  0xE9, 0xBA, 0xA8, 0x92, 0xB1, 0x0D, 0xCB, 0x60, 0x4B, 0xEA, 0x42, 0x84,
+  0x20, 0x25, 0x8E, 0x80, 0x1D, 0x57, 0xEE, 0xE7, 0xF4, 0xB8, 0x11, 0x3C,
+  0xDE, 0x51, 0x2F, 0xE1, 0xE8, 0xC0, 0x8B, 0x85, 0x7D, 0x54, 0x0F, 0x05,
+  0xBE, 0xD5, 0xAA, 0x70, 0xDD, 0xDE, 0x55, 0xEC, 0x78, 0x1F, 0x0B, 0xE6,
+  0x0D, 0xEF, 0xDB, 0xCD, 0x7A, 0x33, 0x17, 0xA3, 0xD5, 0xDB, 0x20, 0xC6,
+  0xF1, 0x48, 0xD6, 0x6E, 0x1F, 0xA1, 0x72, 0xD1, 0xA1, 0xF0, 0x47, 0x2D,
+  0xA2, 0xDC, 0x42, 0x11, 0x65, 0x3A, 0xE1, 0x32, 0x3E, 0x15, 0x25, 0xCA,
+  0x1F, 0x99,

[edk2] [edk2-test][Patch 3/3] uefi-sct/SctPkg:Add VerifySignature() Conf Test

2018-11-29 Thread Eric Jin
Cc: Supreeth Venkatesh 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Jin 
---
 .../EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c   |   5 +-
 .../EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h   |  16 ++
 .../BlackBoxTest/Pkcs7BBTestConformance.c  | 303 -
 .../PKCS7Verify/BlackBoxTest/Pkcs7BBTestMain.c |   2 -
 4 files changed, 319 insertions(+), 7 deletions(-)

diff --git 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c
index 4d433c3..8f6546a 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.c
@@ -36,7 +36,10 @@ EFI_GUID gPkcs7BBTestConformanceAssertionGuid007 = 
EFI_TEST_PKCS7BBTESTCONFORMAN
 EFI_GUID gPkcs7BBTestConformanceAssertionGuid008 = 
EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_008_GUID;
 EFI_GUID gPkcs7BBTestConformanceAssertionGuid009 = 
EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_009_GUID;
 EFI_GUID gPkcs7BBTestConformanceAssertionGuid010 = 
EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_010_GUID;
-
+EFI_GUID gPkcs7BBTestConformanceAssertionGuid011 = 
EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_011_GUID;
+EFI_GUID gPkcs7BBTestConformanceAssertionGuid012 = 
EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_012_GUID;
+EFI_GUID gPkcs7BBTestConformanceAssertionGuid013 = 
EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_013_GUID;
+EFI_GUID gPkcs7BBTestConformanceAssertionGuid014 = 
EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_014_GUID;
 
 EFI_GUID gPkcs7BBTestFunctionAssertionGuid001 = 
EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_001_GUID;
 EFI_GUID gPkcs7BBTestFunctionAssertionGuid002 = 
EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_002_GUID;
diff --git 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h
index 32a00f6..f8d1b8f 100644
--- a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h
+++ b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Guid.h
@@ -65,6 +65,22 @@ extern EFI_GUID gPkcs7BBTestConformanceAssertionGuid009;
 { 0xb136e016, 0x4f80, 0x44bd, {0xba, 0xb0, 0x1c, 0x34, 0x8a, 0x2d, 0xa1, 0x8a 
}}
 extern EFI_GUID gPkcs7BBTestConformanceAssertionGuid010;
 
+#define EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_011_GUID \
+{ 0xa58f3626, 0xf16e, 0x4cd5, { 0xba, 0x12, 0x7a, 0x9d, 0xd6, 0x9a, 0x7a, 0x71 
}}
+extern EFI_GUID gPkcs7BBTestConformanceAssertionGuid011;
+
+#define EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_012_GUID \
+{ 0xbe4a0bf2, 0x2d46, 0x4d96, { 0xa6, 0x11, 0x21, 0x99, 0xa5, 0x5f, 0xa4, 0xee 
}}
+extern EFI_GUID gPkcs7BBTestConformanceAssertionGuid012;
+
+#define EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_013_GUID \
+{ 0xc0536a27, 0x304e, 0x497a, { 0xa5, 0xe3, 0x94, 0x11, 0x38, 0x53, 0x6e, 0x40 
}}
+extern EFI_GUID gPkcs7BBTestConformanceAssertionGuid013;
+
+#define EFI_TEST_PKCS7BBTESTCONFORMANCE_ASSERTION_014_GUID \
+{ 0x8c5aa0e8, 0x17ff, 0x49cd, { 0x8f, 0xec, 0x37, 0xc3, 0xfd, 0x5f, 0x77, 0x0 
}}
+extern EFI_GUID gPkcs7BBTestConformanceAssertionGuid014;
+
 
 #define EFI_TEST_PKCS7BBTESTFUNCTION_ASSERTION_001_GUID \
 { 0x5c0eec50, 0xa6ea, 0x413c, {0x8a, 0x46, 0x4a, 0xd1, 0x4a, 0x77, 0x76, 0xf1 
}}
diff --git 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Pkcs7BBTestConformance.c
 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Pkcs7BBTestConformance.c
index b7bc19b..ce7d5bb 100644
--- 
a/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Pkcs7BBTestConformance.c
+++ 
b/uefi-sct/SctPkg/TestCase/UEFI/EFI/Protocol/PKCS7Verify/BlackBoxTest/Pkcs7BBTestConformance.c
@@ -278,10 +278,6 @@ BBTestVerifyBufferConformanceTest (
  Status
  );
 
-
-
-
-
   // Signed data embedded in SignedData but InData is not NULL
   AllowedDb[0] = DbEntry1;
   RevokedDb[0] = NULL;
@@ -507,4 +503,303 @@ BBTestVerifyBufferConformanceTest (
   return EFI_SUCCESS;
 }
 
+EFI_STATUS
+BBTestVerifySignatureConformanceTest (
+  IN EFI_BB_TEST_PROTOCOL*This,
+  IN VOID*ClientInterface,
+  IN EFI_TEST_LEVEL  TestLevel,
+  IN EFI_HANDLE  SupportHandle
+  )
+{
+  EFI_STANDARD_TEST_LIBRARY_PROTOCOL*StandardLib;
+  EFI_STATUSStatus;
+  EFI_PKCS7_VERIFY_PROTOCOL *Pkcs7Verify;
+  EFI_TEST_ASSERTIONAssertionType;
 
+  Pkcs7Verify = (EFI_PKCS7_VERIFY_PROTOCOL*)ClientInterface;
+  if (Pkcs7Verify == NULL)
+return EFI_UNSUPPORTED;
+
+  //
+  // Get the Standard Library Interface
+  //
+  Status = gtBS->HandleProtocol (
+   SupportHandle,
+   &gEfiStandardTestLibraryGuid,
+   (VOID **) &StandardLib
+   );
+  if (EFI_ERROR(Status)) {
+return Status;
+  }
+
+  AllowedDb[0] = DbEn

Re: [edk2] [edk2-announce] Research Request

2018-11-29 Thread Laszlo Ersek
On 11/29/18 02:07, Jeremiah Cox wrote:
> I did a further experiment for you:
> https://github.com/lersek/edk2/pull/2

Thanks!

> I cannot rebase away my history from PRs...
> Hopefully you have a nice email trail too.

The emails are coming in nice, but I'm not universally pleased with the
contents. I listed some issues regarding that in
, but I guess I should write them
up sometime more readably, at the end of this experiment.

Thanks!
Laszlo

> -Original Message-
> From: Laszlo Ersek  
> Sent: Wednesday, November 28, 2018 2:02 PM
> To: Jeremiah Cox ; Brian J. Johnson 
> ; stephano 
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [edk2-announce] Research Request
> 
> On 11/28/18 19:31, Jeremiah Cox wrote:
>> Test PR submitted
> 
> Thanks!
> Laszlo
> 

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


Re: [edk2] Help on boot manager 'Boot Manager Menu' and direct boot

2018-11-29 Thread Laszlo Ersek
On 11/29/18 04:09, Udit Kumar wrote:
> Thanks for pointers Andrew
> But if I use emulated runtime variables or say efi=noruntime, I am getting 
> same behavior.
> On device tree, I see device tree same in both cases.
> 
> In case of direct boot, only user space console is corrupted whereas kernel 
> space console
> already prints good characters.
> 
> FYI, 
> Both user space and kernel space are on same kermit session

I can only think of some terminal control sequences that are *not*
printed to the terminal when you don't enter UiApp manually. I don't
understand how that could cause the exact symptom you describe, but I
have no better explanation.

Can you try other serial communication programs on your desktop? Such as
"minicom" or "screen"?

Also, can you try changing your "console=..." kernel param(s)?

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


Re: [edk2] [PATCH v3 05/16] MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits

2018-11-29 Thread Ard Biesheuvel
On Wed, 28 Nov 2018 at 19:41, Laszlo Ersek  wrote:
>
> On 11/28/18 15:33, Ard Biesheuvel wrote:
> > AArch64 supports the use of more than 48 bits for physical and/or
> > virtual addressing, but only if the page size is set to 64 KB,
> > which is not supported by UEFI. So redefine MAX_ADDRESS to cover
> > only 48 address bits.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel 
> > Reviewed-by: Leif Lindholm 
> > ---
> >  MdePkg/Include/AArch64/ProcessorBind.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/MdePkg/Include/AArch64/ProcessorBind.h 
> > b/MdePkg/Include/AArch64/ProcessorBind.h
> > index 968c18f915ae..dad75df1c579 100644
> > --- a/MdePkg/Include/AArch64/ProcessorBind.h
> > +++ b/MdePkg/Include/AArch64/ProcessorBind.h
> > @@ -138,9 +138,9 @@ typedef INT64   INTN;
> >  #define MAX_2_BITS  0xC000ULL
> >
> >  ///
> > -/// Maximum legal AARCH64  address
> > +/// Maximum legal AARCH64  address (48 bits for 4 KB page size)
> >  ///
> > -#define MAX_ADDRESS   0xULL
> > +#define MAX_ADDRESS   0xULL
> >
> >  ///
> >  /// Maximum legal AArch64 INTN and UINTN values.
> >
>
> Hmmm.
>
> I bit the bullet and grepped the tree for MAX_ADDRESS.
>
> The amount of hits is staggering. I can't audit all of them.
>
> Generally, MAX_ADDRESS seems to be used in checks that prevent address
> wrap-around. In that regard, this change looks valid.
>
> I can't guarantee this change won't regress anything though. In the
> previous posting of this patch, I asked Liming some questions (IIRC):
>
> 6f1209fb-bb89-a70f-ba0e-3ebf2e12e459@redhat.com">http://mid.mail-archive.com/6f1209fb-bb89-a70f-ba0e-3ebf2e12e459@redhat.com
>
> It would be nice to see answers. :)
>

Yep

> In addition:
>
> (a) in "BaseTools/Source/C/Include/AArch64/ProcessorBind.h", we have
> another instance of the macro definition. I suspect it should be kept in
> sync.
>

Indeed.

> (b) in "BaseTools/Source/C/Common/CommonLib.h", we have:
>
> #define MAX_UINTN MAX_ADDRESS
>
> which I think relies on (a), and hence it will be amusingly wrong after
> we synchronize (a) with MdePkg.
>
> (BTW, (b) is exactly the kind of assumption that scares me about this
> patch.)
>

That doesn't make any sense at all. What does 'native' mean in the
context of BaseTools anyway?

> We're not much past the last stable tag (edk2-stable201811), so let's
> hope there's going to be enough time to catch any regressions.
>
> With (a) and (b) investigated / fixed up, I'd be willing to A-b this.
> Cautiously :)
>

Thanks

> Anyway, this is for MdePkg, so my review is not required. (I certainly
> do not intend to *oppose* this patch.)
>
> Thanks
> Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH v3 05/16] MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits

2018-11-29 Thread Laszlo Ersek
On 11/29/18 11:40, Ard Biesheuvel wrote:
> On Wed, 28 Nov 2018 at 19:41, Laszlo Ersek  wrote:
>>
>> On 11/28/18 15:33, Ard Biesheuvel wrote:
>>> AArch64 supports the use of more than 48 bits for physical and/or
>>> virtual addressing, but only if the page size is set to 64 KB,
>>> which is not supported by UEFI. So redefine MAX_ADDRESS to cover
>>> only 48 address bits.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Ard Biesheuvel 
>>> Reviewed-by: Leif Lindholm 
>>> ---
>>>  MdePkg/Include/AArch64/ProcessorBind.h | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/MdePkg/Include/AArch64/ProcessorBind.h 
>>> b/MdePkg/Include/AArch64/ProcessorBind.h
>>> index 968c18f915ae..dad75df1c579 100644
>>> --- a/MdePkg/Include/AArch64/ProcessorBind.h
>>> +++ b/MdePkg/Include/AArch64/ProcessorBind.h
>>> @@ -138,9 +138,9 @@ typedef INT64   INTN;
>>>  #define MAX_2_BITS  0xC000ULL
>>>
>>>  ///
>>> -/// Maximum legal AARCH64  address
>>> +/// Maximum legal AARCH64  address (48 bits for 4 KB page size)
>>>  ///
>>> -#define MAX_ADDRESS   0xULL
>>> +#define MAX_ADDRESS   0xULL
>>>
>>>  ///
>>>  /// Maximum legal AArch64 INTN and UINTN values.
>>>
>>
>> Hmmm.
>>
>> I bit the bullet and grepped the tree for MAX_ADDRESS.
>>
>> The amount of hits is staggering. I can't audit all of them.
>>
>> Generally, MAX_ADDRESS seems to be used in checks that prevent address
>> wrap-around. In that regard, this change looks valid.
>>
>> I can't guarantee this change won't regress anything though. In the
>> previous posting of this patch, I asked Liming some questions (IIRC):
>>
>> 6f1209fb-bb89-a70f-ba0e-3ebf2e12e459@redhat.com">http://mid.mail-archive.com/6f1209fb-bb89-a70f-ba0e-3ebf2e12e459@redhat.com
>>
>> It would be nice to see answers. :)
>>
> 
> Yep
> 
>> In addition:
>>
>> (a) in "BaseTools/Source/C/Include/AArch64/ProcessorBind.h", we have
>> another instance of the macro definition. I suspect it should be kept in
>> sync.
>>
> 
> Indeed.
> 
>> (b) in "BaseTools/Source/C/Common/CommonLib.h", we have:
>>
>> #define MAX_UINTN MAX_ADDRESS
>>
>> which I think relies on (a), and hence it will be amusingly wrong after
>> we synchronize (a) with MdePkg.
>>
>> (BTW, (b) is exactly the kind of assumption that scares me about this
>> patch.)
>>
> 
> That doesn't make any sense at all. What does 'native' mean in the
> context of BaseTools anyway?

I can't tell whether this MAX_UINTN definition is for BaseTools itself
(i.e., "native") or for the build target arch.

"CommonLib.c" has a number of instances of MAX_UINTN... Hm, they are in
the following two functions:

- StrDecimalToUintnS() -- wrapped by StrDecimalToUintn(),
  StrToIpv4Address(), and StrToIpv6Address())

- StrHexToUintnS() -- wrapped by StrHexToUintn(), and
  StrToIpv6Address().

I tried to look at where those are called from, and the picture remains
muddled.

For example, StrHexToUintn() is called from
"BaseTools/Source/C/DevicePath/DevicePathFromText.c", functions
EisaIdFromText() and DevPathFromTextNVMe(). These functions seem to
compose binary devpath nodes from text *during the build*, likely for
the firmware-under-build to consume as-is. Hence, this use of MAX_UINTN
-- through StrHexToUintn() -- qualifies as *native* use. And for that,
the MAX_UINTN definition in "CommonLib.h" looks wrong, independently of
your patch series.

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


[edk2] [PATCH 3/6] BaseTools/DevicePath: use explicit 64-bit number parsing routines

2018-11-29 Thread Ard Biesheuvel
Replace invocations of StrHexToUintn() with StrHexToUint64(), so
that we can drop the former.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 BaseTools/Source/C/DevicePath/DevicePathFromText.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/C/DevicePath/DevicePathFromText.c 
b/BaseTools/Source/C/DevicePath/DevicePathFromText.c
index 555efa1acdde..6151926af9aa 100644
--- a/BaseTools/Source/C/DevicePath/DevicePathFromText.c
+++ b/BaseTools/Source/C/DevicePath/DevicePathFromText.c
@@ -520,7 +520,7 @@ EisaIdFromText (
   return (((Text[0] - 'A' + 1) & 0x1f) << 10)
+ (((Text[1] - 'A' + 1) & 0x1f) <<  5)
+ (((Text[2] - 'A' + 1) & 0x1f) <<  0)
-   + (UINT32) (StrHexToUintn (&Text[3]) << 16)
+   + (UINT32) (StrHexToUint64 (&Text[3]) << 16)
;
 }
 
@@ -1506,7 +1506,7 @@ DevPathFromTextNVMe (
 
   Index = sizeof (Nvme->NamespaceUuid) / sizeof (UINT8);
   while (Index-- != 0) {
-Uuid[Index] = (UINT8) StrHexToUintn (SplitStr (&NamespaceUuidStr, L'-'));
+Uuid[Index] = (UINT8) StrHexToUint64 (SplitStr (&NamespaceUuidStr, L'-'));
   }
 
   return (EFI_DEVICE_PATH_PROTOCOL *) Nvme;
-- 
2.19.1

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


[edk2] [PATCH 6/6] BaseTools/CommonLib: drop definition of MAX_UINTN

2018-11-29 Thread Ard Biesheuvel
The maximum value that can be represented by the native word size
of the *target* should be irrelevant when compiling tools that
run on the build *host*. So drop the definition of MAX_UINTN, now
that we no longer use it.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 BaseTools/Source/C/Common/CommonLib.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/BaseTools/Source/C/Common/CommonLib.h 
b/BaseTools/Source/C/Common/CommonLib.h
index 6930d9227b87..b1c6c00a3478 100644
--- a/BaseTools/Source/C/Common/CommonLib.h
+++ b/BaseTools/Source/C/Common/CommonLib.h
@@ -22,7 +22,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 
 #define MAX_LONG_FILE_PATH 500
 
-#define MAX_UINTN MAX_ADDRESS
 #define MAX_UINT64 ((UINT64)0xULL)
 #define MAX_UINT16  ((UINT16)0x)
 #define MAX_UINT8   ((UINT8)0xFF)
-- 
2.19.1

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


[edk2] [PATCH 4/6] BaseTools/DevicePath: use MAX_UINT16 as default device path max size

2018-11-29 Thread Ard Biesheuvel
Replace the default size limit of IsDevicePathValid() with a value
that does not depend on the native word size of the build host.

64 KB seems sufficient as the upper bound of a device path handled
by UEFI.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 BaseTools/Source/C/DevicePath/DevicePathUtilities.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/C/DevicePath/DevicePathUtilities.c 
b/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
index d4ec2742b7c8..ba7f83e53070 100644
--- a/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
+++ b/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
@@ -62,7 +62,7 @@ IsDevicePathValid (
   ASSERT (DevicePath != NULL);
 
   if (MaxSize == 0) {
-MaxSize = MAX_UINTN;
+MaxSize = MAX_UINT16;
  }
 
   //
@@ -78,7 +78,7 @@ IsDevicePathValid (
   return FALSE;
 }
 
-if (NodeLength > MAX_UINTN - Size) {
+if (NodeLength > MAX_UINT16 - Size) {
   return FALSE;
 }
 Size += NodeLength;
-- 
2.19.1

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


[edk2] [PATCH 2/6] BaseTools/CommonLib: use explicit 64-bit type in Strtoi()

2018-11-29 Thread Ard Biesheuvel
Don't use the native word size string to number parsing routines,
but instead, use the 64-bit one and cast to UINTN.

Currently, the only user is in Source/C/DevicePath/DevicePathFromText.c
which takes care to use Strtoi64 () unless it assumes the value fits
in 32-bit, so this change is a no-op even on 32-bit build hosts.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 BaseTools/Source/C/Common/CommonLib.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/BaseTools/Source/C/Common/CommonLib.c 
b/BaseTools/Source/C/Common/CommonLib.c
index bea6af0a45b1..c5e32b1292e0 100644
--- a/BaseTools/Source/C/Common/CommonLib.c
+++ b/BaseTools/Source/C/Common/CommonLib.c
@@ -2252,9 +2252,9 @@ Strtoi (
   )
 {
   if (IsHexStr (Str)) {
-return StrHexToUintn (Str);
+return (UINTN)StrHexToUint64 (Str);
   } else {
-return StrDecimalToUintn (Str);
+return (UINTN)StrDecimalToUint64 (Str);
   }
 }
 
-- 
2.19.1

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


[edk2] [PATCH 5/6] BaseTools/CommonLib: get rid of 'native' type string parsing routines

2018-11-29 Thread Ard Biesheuvel
Parsing a string into an integer variable of the native word size
is not defined for the BaseTools, since the same tools may be used
to build firmware for different targets with different native word
sizes.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 BaseTools/Source/C/Common/CommonLib.h |  24 ---
 BaseTools/Source/C/Common/CommonLib.c | 174 +---
 2 files changed, 5 insertions(+), 193 deletions(-)

diff --git a/BaseTools/Source/C/Common/CommonLib.h 
b/BaseTools/Source/C/Common/CommonLib.h
index fa10fac4682a..6930d9227b87 100644
--- a/BaseTools/Source/C/Common/CommonLib.h
+++ b/BaseTools/Source/C/Common/CommonLib.h
@@ -250,16 +250,6 @@ StrSize (
   CONST CHAR16  *String
   );
 
-UINTN
-StrHexToUintn (
-  CONST CHAR16  *String
-  );
-
-UINTN
-StrDecimalToUintn (
-  CONST CHAR16  *String
-  );
-
 UINT64
 StrHexToUint64 (
   CONST CHAR16 *String
@@ -277,13 +267,6 @@ StrHexToUint64S (
 UINT64 *Data
   );
 
-RETURN_STATUS
-StrHexToUintnS (
-CONST CHAR16 *String,
- CHAR16 **EndPointer,  OPTIONAL
- UINTN  *Data
-  );
-
 RETURN_STATUS
 StrDecimalToUint64S (
 CONST CHAR16 *String,
@@ -291,13 +274,6 @@ StrDecimalToUint64S (
  UINT64 *Data
   );
 
-RETURN_STATUS
-StrDecimalToUintnS (
-CONST CHAR16 *String,
- CHAR16 **EndPointer,  OPTIONAL
- UINTN  *Data
-  );
-
 VOID *
 ReallocatePool (
UINTN  OldSize,
diff --git a/BaseTools/Source/C/Common/CommonLib.c 
b/BaseTools/Source/C/Common/CommonLib.c
index c5e32b1292e0..9142a9a7eda3 100644
--- a/BaseTools/Source/C/Common/CommonLib.c
+++ b/BaseTools/Source/C/Common/CommonLib.c
@@ -882,72 +882,6 @@ InternalSafeStringNoStrOverlap (
   return !InternalSafeStringIsOverlap (Str1, Size1 * sizeof(CHAR16), Str2, 
Size2 * sizeof(CHAR16));
 }
 
-RETURN_STATUS
-StrDecimalToUintnS (
-CONST CHAR16 *String,
- CHAR16 **EndPointer,  OPTIONAL
- UINTN  *Data
-  )
-{
-  ASSERT (((UINTN) String & BIT0) == 0);
-
-  //
-  // 1. Neither String nor Data shall be a null pointer.
-  //
-  SAFE_STRING_CONSTRAINT_CHECK ((String != NULL), RETURN_INVALID_PARAMETER);
-  SAFE_STRING_CONSTRAINT_CHECK ((Data != NULL), RETURN_INVALID_PARAMETER);
-
-  //
-  // 2. The length of String shall not be greater than RSIZE_MAX.
-  //
-  if (RSIZE_MAX != 0) {
-SAFE_STRING_CONSTRAINT_CHECK ((StrnLenS (String, RSIZE_MAX + 1) <= 
RSIZE_MAX), RETURN_INVALID_PARAMETER);
-  }
-
-  if (EndPointer != NULL) {
-*EndPointer = (CHAR16 *) String;
-  }
-
-  //
-  // Ignore the pad spaces (space or tab)
-  //
-  while ((*String == L' ') || (*String == L'\t')) {
-String++;
-  }
-
-  //
-  // Ignore leading Zeros after the spaces
-  //
-  while (*String == L'0') {
-String++;
-  }
-
-  *Data = 0;
-
-  while (InternalIsDecimalDigitCharacter (*String)) {
-//
-// If the number represented by String overflows according to the range
-// defined by UINTN, then MAX_UINTN is stored in *Data and
-// RETURN_UNSUPPORTED is returned.
-//
-if (*Data > ((MAX_UINTN - (*String - L'0')) / 10)) {
-  *Data = MAX_UINTN;
-  if (EndPointer != NULL) {
-*EndPointer = (CHAR16 *) String;
-  }
-  return RETURN_UNSUPPORTED;
-}
-
-*Data = *Data * 10 + (*String - L'0');
-String++;
-  }
-
-  if (EndPointer != NULL) {
-*EndPointer = (CHAR16 *) String;
-  }
-  return RETURN_SUCCESS;
-}
-
 /**
   Convert a Null-terminated Unicode decimal string to a value of type UINT64.
 
@@ -1064,9 +998,9 @@ StrDecimalToUint64S (
 
 /**
   Convert a Null-terminated Unicode hexadecimal string to a value of type
-  UINTN.
+  UINT64.
 
-  This function outputs a value of type UINTN by interpreting the contents of
+  This function outputs a value of type UINT64 by interpreting the contents of
   the Unicode string specified by String as a hexadecimal number. The format of
   the input Unicode string String is:
 
@@ -1091,8 +1025,8 @@ StrDecimalToUint64S (
 
   If String has no valid hexadecimal digits in the above format, then 0 is
   stored at the location pointed to by Data.
-  If the number represented by String exceeds the range defined by UINTN, then
-  MAX_UINTN is stored at the location pointed to by Data.
+  If the number represented by String exceeds the range defined by UINT64, then
+  MAX_UINT64 is stored at the location pointed to by Data.
 
   If EndPointer is not NULL, a pointer to the character that stopped the scan
   is stored at the location pointed to by EndPointer. If String has no valid
@@ -1112,86 +1046,10 @@ StrDecimalToUint64S (
characters, not including the
Null-terminator.
   @retval RETURN_UNSUPPORTED   If the number represented by String exceeds
- 

[edk2] [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling

2018-11-29 Thread Ard Biesheuvel
In the context of the BaseTools, there is no such thing as a native word
size, given that the same set of tools may be used to build a firmware
image consisting of both 32-bit and 64-bit modules.

So update StrToIpv4Address() and StrToIpv6Address() to use UINT64
types instead of UINTN types when parsing strings.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 BaseTools/Source/C/Common/CommonLib.c | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/BaseTools/Source/C/Common/CommonLib.c 
b/BaseTools/Source/C/Common/CommonLib.c
index 618aadac781a..bea6af0a45b1 100644
--- a/BaseTools/Source/C/Common/CommonLib.c
+++ b/BaseTools/Source/C/Common/CommonLib.c
@@ -1785,7 +1785,7 @@ StrToIpv4Address (
 {
   RETURN_STATUS  Status;
   UINTN  AddressIndex;
-  UINTN  Uintn;
+  UINTN  Uint64;
   EFI_IPv4_ADDRESS   LocalAddress;
   UINT8  LocalPrefixLength;
   CHAR16 *Pointer;
@@ -1812,7 +1812,7 @@ StrToIpv4Address (
 //
 // Get D or P.
 //
-Status = StrDecimalToUintnS ((CONST CHAR16 *) Pointer, &Pointer, &Uintn);
+Status = StrDecimalToUint64S ((CONST CHAR16 *) Pointer, &Pointer, &Uint64);
 if (RETURN_ERROR (Status)) {
   return RETURN_UNSUPPORTED;
 }
@@ -1820,18 +1820,18 @@ StrToIpv4Address (
   //
   // It's P.
   //
-  if (Uintn > 32) {
+  if (Uint64 > 32) {
 return RETURN_UNSUPPORTED;
   }
-  LocalPrefixLength = (UINT8) Uintn;
+  LocalPrefixLength = (UINT8) Uint64;
 } else {
   //
   // It's D.
   //
-  if (Uintn > MAX_UINT8) {
+  if (Uint64 > MAX_UINT8) {
 return RETURN_UNSUPPORTED;
   }
-  LocalAddress.Addr[AddressIndex] = (UINT8) Uintn;
+  LocalAddress.Addr[AddressIndex] = (UINT8) Uint64;
   AddressIndex++;
 }
 
@@ -1888,7 +1888,7 @@ StrToIpv6Address (
 {
   RETURN_STATUS  Status;
   UINTN  AddressIndex;
-  UINTN  Uintn;
+  UINT64 Uint64;
   EFI_IPv6_ADDRESS   LocalAddress;
   UINT8  LocalPrefixLength;
   CONST CHAR16   *Pointer;
@@ -1969,7 +1969,7 @@ StrToIpv6Address (
 //
 // Get X.
 //
-Status = StrHexToUintnS (Pointer, &End, &Uintn);
+Status = StrHexToUint64S (Pointer, &End, &Uint64);
 if (RETURN_ERROR (Status) || End - Pointer > 4) {
   //
   // Number of hexadecimal digit characters is no more than 4.
@@ -1978,24 +1978,24 @@ StrToIpv6Address (
 }
 Pointer = End;
 //
-// Uintn won't exceed MAX_UINT16 if number of hexadecimal digit 
characters is no more than 4.
+// Uint64 won't exceed MAX_UINT16 if number of hexadecimal digit 
characters is no more than 4.
 //
 ASSERT (AddressIndex + 1 < ARRAY_SIZE (Address->Addr));
-LocalAddress.Addr[AddressIndex] = (UINT8) ((UINT16) Uintn >> 8);
-LocalAddress.Addr[AddressIndex + 1] = (UINT8) Uintn;
+LocalAddress.Addr[AddressIndex] = (UINT8) ((UINT16) Uint64 >> 8);
+LocalAddress.Addr[AddressIndex + 1] = (UINT8) Uint64;
 AddressIndex += 2;
   } else {
 //
 // Get P, then exit the loop.
 //
-Status = StrDecimalToUintnS (Pointer, &End, &Uintn);
-if (RETURN_ERROR (Status) || End == Pointer || Uintn > 128) {
+Status = StrDecimalToUint64S (Pointer, &End, &Uint64);
+if (RETURN_ERROR (Status) || End == Pointer || Uint64 > 128) {
   //
   // Prefix length should not exceed 128.
   //
   return RETURN_UNSUPPORTED;
 }
-LocalPrefixLength = (UINT8) Uintn;
+LocalPrefixLength = (UINT8) Uint64;
 Pointer = End;
 break;
   }
-- 
2.19.1

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


[edk2] [PATCH 0/6] BaseTools: get rid of MAX_UINTN

2018-11-29 Thread Ard Biesheuvel
There should be no reason for the build tools to care about the native
word size of a particular target, so relying on a definition of MAX_UINTN
is definitely wrong, and most likely inaccurate on 32-bit build hosts.

So refactor the code in CommonLib and DevicePath so we no longer rely
on this definition.

Cc: Laszlo Ersek 
Cc: Yonghong Zhu 
Cc: Liming Gao 
Cc: Bob Feng 

Ard Biesheuvel (6):
  BaseTools/CommonLib: avoid using 'native' word size in IP address
handling
  BaseTools/CommonLib: use explicit 64-bit type in Strtoi()
  BaseTools/DevicePath: use explicit 64-bit number parsing routines
  BaseTools/DevicePath: use MAX_UINT16 as default device path max size
  BaseTools/CommonLib: get rid of 'native' type string parsing routines
  BaseTools/CommonLib: drop definition of MAX_UINTN

 BaseTools/Source/C/Common/CommonLib.h |  25 ---
 BaseTools/Source/C/Common/CommonLib.c | 206 ++
 .../Source/C/DevicePath/DevicePathFromText.c  |   4 +-
 .../Source/C/DevicePath/DevicePathUtilities.c |   4 +-
 4 files changed, 25 insertions(+), 214 deletions(-)

-- 
2.19.1

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


Re: [edk2] Help on boot manager 'Boot Manager Menu' and direct boot

2018-11-29 Thread Udit Kumar
Thanks Laszlo,

 
> I can only think of some terminal control sequences that are *not* printed to
> the terminal when you don't enter UiApp manually. I don't understand how that
> could cause the exact symptom you describe, but I have no better explanation.
> 
> Can you try other serial communication programs on your desktop? Such as
> "minicom" or "screen"?

Screen didn't help.
Moreover , using different OS distributions show same similar behavior !!
 
> Also, can you try changing your "console=..." kernel param(s)?

You meant baud-rate ? 

On uefi side,  could you help me if there is some extra information passed to 
OS in path 
UiApp -> BootDevice, 

I could see , some of additional protocols are installed in above path, I am 
not sure if those are
used  by  OS or OS Loader (grub in my case)  somehow.


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


Re: [edk2] [PATCH] MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits

2018-11-29 Thread Gao, Liming
Laszlo:
  I add my comments. 

Thanks
Liming
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Tuesday, November 27, 2018 10:52 PM
> To: Ard Biesheuvel ; edk2-devel@lists.01.org
> Cc: Gao, Liming ; Kinney, Michael D 
> ; leif.lindh...@linaro.org;
> phi...@redhat.com
> Subject: Re: [PATCH] MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 
> bits
> 
> On 11/27/18 13:27, Ard Biesheuvel wrote:
> > AArch64 support the use of more than 48 bits for physical and/or
> > virtual addressing, but only if the page size is set to 64 KB,
> > which is not supported by UEFI/EDK2. So redefine MAX_ADDRESS to
> > cover only 48 address bits.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel 
> > ---
> >  MdePkg/Include/AArch64/ProcessorBind.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/MdePkg/Include/AArch64/ProcessorBind.h 
> > b/MdePkg/Include/AArch64/ProcessorBind.h
> > index 968c18f915ae..dad75df1c579 100644
> > --- a/MdePkg/Include/AArch64/ProcessorBind.h
> > +++ b/MdePkg/Include/AArch64/ProcessorBind.h
> > @@ -138,9 +138,9 @@ typedef INT64   INTN;
> >  #define MAX_2_BITS  0xC000ULL
> >
> >  ///
> > -/// Maximum legal AARCH64  address
> > +/// Maximum legal AARCH64  address (48 bits for 4 KB page size)
> >  ///
> > -#define MAX_ADDRESS   0xULL
> > +#define MAX_ADDRESS   0xULL
> >
> >  ///
> >  /// Maximum legal AArch64 INTN and UINTN values.
> >
> 
> H. I'm worried about this change. I think it could open a can of
> worms. I have no clue what *all* the things are that we use MAX_ADDRESS
> for. Does it give the maximum value of the canonical address *format*?
> Or is it the maximum address that the processor could ever access?
> 
> Let's look at the X64 situation... For X64, MAX_ADDRESS is
> 0x____ULL (MdePkg/Include/X64/ProcessorBind.h). However,
> on X64, even considering the recently introduced 5-level paging
> , the "useful"
> number of address bits is up to just 57 -- it used to be 48, with
> 4-level paging. That is: not 64. Yet we have MAX_UINT64 for MAX_ADDRESS!
> 
> Which in turn means that, with X64 5-level paging in mind, the issue
> affects X64 as well -- there could be RAM in the system that the 64-bit
> DXE phase couldn't access (because edk2 doesn't support 5-level paging,
> AIUI), but the OS could.
> 
> That officially turns the question into a multi-architectural one: how
> should the UEFI memmap describe the highest RAM range, such that it be
> exposed to the OS, but not exposed to the firmware itself? (Because, the
> firmware doesn't support the necessary paging mode, or processor mode.)
> 
> Liming, can you share what Intel plans, in edk2, for supporting 5-level
> paging?
So far, I have no more to be shared. I don't know whether it is necessary to 
support 5-level paging with the max memory. 
The firmware can report [2^48 .. 2^57) RAM with the allocated status. So, those 
region memory can't be allocated in firmware. 
They will be visible to OS. If OS enables 5-level paging, it can access them. 
> 
> And, on such physical X64 systems today that support 57-bit paging, how
> does the UEFI memmap describe the [2^48 .. 2^57) RAM?
> 
> And how does the firmware allocate and use memory from that area?
> 
> Thanks,
> Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling

2018-11-29 Thread Gao, Liming
Ard:
  I mean the build error. Besides, what test have you done with this patch set?

CommonLib.c(1651): error C2220: warning treated as error - no 'object' file 
generated
CommonLib.c(1651): warning C4133: 'function': incompatible types - from 'UINTN 
*' to 'UINT64 *'
NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio 
14.0\VC\BIN\cl.exe"' : return code '0x2'

Below > +  UINTN  Uint64; ==> > +  UINT64  
Uint64;
> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Thursday, November 29, 2018 8:31 PM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel ; Laszlo Ersek 
> ; Zhu, Yonghong ; Gao,
> Liming ; Feng, Bob C 
> Subject: [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word size in 
> IP address handling
> 
> In the context of the BaseTools, there is no such thing as a native word
> size, given that the same set of tools may be used to build a firmware
> image consisting of both 32-bit and 64-bit modules.
> 
> So update StrToIpv4Address() and StrToIpv6Address() to use UINT64
> types instead of UINTN types when parsing strings.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  BaseTools/Source/C/Common/CommonLib.c | 28 ++--
>  1 file changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/BaseTools/Source/C/Common/CommonLib.c 
> b/BaseTools/Source/C/Common/CommonLib.c
> index 618aadac781a..bea6af0a45b1 100644
> --- a/BaseTools/Source/C/Common/CommonLib.c
> +++ b/BaseTools/Source/C/Common/CommonLib.c
> @@ -1785,7 +1785,7 @@ StrToIpv4Address (
>  {
>RETURN_STATUS  Status;
>UINTN  AddressIndex;
> -  UINTN  Uintn;
> +  UINTN  Uint64;
>EFI_IPv4_ADDRESS   LocalAddress;
>UINT8  LocalPrefixLength;
>CHAR16 *Pointer;
> @@ -1812,7 +1812,7 @@ StrToIpv4Address (
>  //
>  // Get D or P.
>  //
> -Status = StrDecimalToUintnS ((CONST CHAR16 *) Pointer, &Pointer, &Uintn);
> +Status = StrDecimalToUint64S ((CONST CHAR16 *) Pointer, &Pointer, 
> &Uint64);
>  if (RETURN_ERROR (Status)) {
>return RETURN_UNSUPPORTED;
>  }
> @@ -1820,18 +1820,18 @@ StrToIpv4Address (
>//
>// It's P.
>//
> -  if (Uintn > 32) {
> +  if (Uint64 > 32) {
>  return RETURN_UNSUPPORTED;
>}
> -  LocalPrefixLength = (UINT8) Uintn;
> +  LocalPrefixLength = (UINT8) Uint64;
>  } else {
>//
>// It's D.
>//
> -  if (Uintn > MAX_UINT8) {
> +  if (Uint64 > MAX_UINT8) {
>  return RETURN_UNSUPPORTED;
>}
> -  LocalAddress.Addr[AddressIndex] = (UINT8) Uintn;
> +  LocalAddress.Addr[AddressIndex] = (UINT8) Uint64;
>AddressIndex++;
>  }
> 
> @@ -1888,7 +1888,7 @@ StrToIpv6Address (
>  {
>RETURN_STATUS  Status;
>UINTN  AddressIndex;
> -  UINTN  Uintn;
> +  UINT64 Uint64;
>EFI_IPv6_ADDRESS   LocalAddress;
>UINT8  LocalPrefixLength;
>CONST CHAR16   *Pointer;
> @@ -1969,7 +1969,7 @@ StrToIpv6Address (
>  //
>  // Get X.
>  //
> -Status = StrHexToUintnS (Pointer, &End, &Uintn);
> +Status = StrHexToUint64S (Pointer, &End, &Uint64);
>  if (RETURN_ERROR (Status) || End - Pointer > 4) {
>//
>// Number of hexadecimal digit characters is no more than 4.
> @@ -1978,24 +1978,24 @@ StrToIpv6Address (
>  }
>  Pointer = End;
>  //
> -// Uintn won't exceed MAX_UINT16 if number of hexadecimal digit 
> characters is no more than 4.
> +// Uint64 won't exceed MAX_UINT16 if number of hexadecimal digit 
> characters is no more than 4.
>  //
>  ASSERT (AddressIndex + 1 < ARRAY_SIZE (Address->Addr));
> -LocalAddress.Addr[AddressIndex] = (UINT8) ((UINT16) Uintn >> 8);
> -LocalAddress.Addr[AddressIndex + 1] = (UINT8) Uintn;
> +LocalAddress.Addr[AddressIndex] = (UINT8) ((UINT16) Uint64 >> 8);
> +LocalAddress.Addr[AddressIndex + 1] = (UINT8) Uint64;
>  AddressIndex += 2;
>} else {
>  //
>  // Get P, then exit the loop.
>  //
> -Status = StrDecimalToUintnS (Pointer, &End, &Uintn);
> -if (RETURN_ERROR (Status) || End == Pointer || Uintn > 128) {
> +Status = StrDecimalToUint64S (Pointer, &End, &Uint64);
> +if (RETURN_ERROR (Status) || End == Pointer || Uint64 > 128) {
>//
>// Prefix length should not exceed 128.
>//
>return RETURN_UNSUPPORTED;
>  }
> -LocalPrefixLength = (UINT8) Uintn;
> +LocalPrefixLength = (UINT8) Uint64;
>  Pointer = End;
>  break;
>}
> --
> 2.19.1

___
edk2-devel mail

Re: [edk2] [PATCH v3 edk2-platforms 0/6] Platform/ARM/Sgi: Add support for Clark.Ares and Clark.Helios platforms

2018-11-29 Thread Leif Lindholm
So, I'm actually mostly happy with this set, but this cover letter is
incorrect.

On Wed, Nov 21, 2018 at 06:14:51PM +0530, Chandni Cherukuri wrote:
> Changes since v1:
> - No code changes, posting this series again with correct patch subject

Plenty of code changes. Stopped the masking and shifting of
platform/config IDs.

> This patch series adds support for two new Arm's SGI platforms - 
> SGI-Clark.Ares and SGI-Clark.Helios. The first patch in this
> series adds support to use a new binding added to the system-id
> node for Platform Identification. The second patch refactors the
> ACPI tables to prevent duplication of ACPI tables. The common
> ACPI tables for SGI platforms are put in the AcpiTables folder.
> Dsdt.asl and Madt.aslc remain in the platform specific folder.
> The rest of the patches add support for the two new SGI platforms.

I am not super happy that the Change from COM1 -> \\_SB.COM0 got
folded into Patch number 2, but I'm not going to ask for a v4 over it.

For the series:
Reviewed-by: Leif Lindholm 
Pushed as 86c75fc51f..77ae8610df.

> Chandni Cherukuri (6):
>   Platform/ARM/Sgi: Adapt to changes in system-id DT node.
>   Platform/ARM/Sgi: Refactor ACPI tables for SGI platforms
>   Platform/ARM/Sgi: Add ACPI tables for SGI-Clark.Ares platform
>   Platform/ARM/Sgi: Add initial support for SGI-Clark.Ares platform
>   Platform/ARM/Sgi: Add ACPI tables for SGI-Clark.Helios platform
>   Platform/ARM/Sgi: Add initial support for SGI-Clark.Helios platform
> 
>  Platform/ARM/SgiPkg/SgiPlatform.dec  |   2 +
>  Platform/ARM/SgiPkg/SgiPlatform.dsc  |   4 +-
>  Platform/ARM/SgiPkg/SgiPlatform.fdf  |   4 +-
>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf |  58 -
>  Platform/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf  |  58 +
>  Platform/ARM/SgiPkg/AcpiTables/SgiClarkAresAcpiTables.inf|  58 +
>  Platform/ARM/SgiPkg/AcpiTables/SgiClarkHeliosAcpiTables.inf  |  58 +
>  Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf  |   2 +
>  Platform/ARM/SgiPkg/Include/SgiAcpiHeader.h  |   2 +-
>  Platform/ARM/SgiPkg/Include/SgiPlatform.h|   6 +
>  Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.c|  33 +--
>  Platform/ARM/SgiPkg/Library/SgiPlatformPei/SgiPlatformPeim.c |  50 ++--
>  Platform/ARM/SgiPkg/AcpiTables/Dbg2.aslc |  88 +++
>  Platform/ARM/SgiPkg/AcpiTables/Fadt.aslc |  87 +++
>  Platform/ARM/SgiPkg/AcpiTables/Gtdt.aslc | 152 
> +++
>  Platform/ARM/SgiPkg/AcpiTables/Iort.aslc | 106 
>  Platform/ARM/SgiPkg/AcpiTables/Mcfg.aslc |  59 +
>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc  |  90 ---
>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dsdt.asl   |   2 +-
>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Fadt.aslc  |  87 ---
>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Gtdt.aslc  | 152 
> ---
>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Iort.aslc  | 106 
>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Mcfg.aslc  |  59 -
>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Spcr.aslc  |  77 --
>  Platform/ARM/SgiPkg/AcpiTables/Sgi575/Ssdt.asl   |  95 ---
>  Platform/ARM/SgiPkg/AcpiTables/SgiClarkAres/Dsdt.asl | 116 +
>  Platform/ARM/SgiPkg/AcpiTables/SgiClarkAres/Madt.aslc| 171 
> +
>  Platform/ARM/SgiPkg/AcpiTables/SgiClarkHelios/Dsdt.asl   | 262 
> +++
>  Platform/ARM/SgiPkg/AcpiTables/SgiClarkHelios/Madt.aslc  | 266 
> 
>  Platform/ARM/SgiPkg/AcpiTables/Spcr.aslc |  77 ++
>  Platform/ARM/SgiPkg/AcpiTables/Ssdt.asl  |  93 +++
>  31 files changed, 1722 insertions(+), 758 deletions(-)
>  delete mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/AcpiTables.inf
>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575AcpiTables.inf
>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/SgiClarkAresAcpiTables.inf
>  create mode 100644 
> Platform/ARM/SgiPkg/AcpiTables/SgiClarkHeliosAcpiTables.inf
>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Dbg2.aslc
>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Fadt.aslc
>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Gtdt.aslc
>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Iort.aslc
>  create mode 100644 Platform/ARM/SgiPkg/AcpiTables/Mcfg.aslc
>  delete mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Dbg2.aslc
>  delete mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Fadt.aslc
>  delete mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Gtdt.aslc
>  delete mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Iort.aslc
>  delete mode 100644 Platform/ARM/SgiPkg/AcpiTables/Sgi575/Mcfg.aslc
>  delete mode 100644 Platform/ARM/SgiPkg/AcpiTa

Re: [edk2] [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling

2018-11-29 Thread Ard Biesheuvel
On Thu, 29 Nov 2018 at 16:11, Gao, Liming  wrote:
>
> Ard:
>   I mean the build error. Besides, what test have you done with this patch 
> set?
>
> CommonLib.c(1651): error C2220: warning treated as error - no 'object' file 
> generated
> CommonLib.c(1651): warning C4133: 'function': incompatible types - from 
> 'UINTN *' to 'UINT64 *'
> NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio 
> 14.0\VC\BIN\cl.exe"' : return code '0x2'
>

Apologies, i missed this change at line 1624

-  UINTN  Uint64;
+  UINT64 Uint64;

It builds fine with GCC though.

> Below > +  UINTN  Uint64; ==> > +  UINT64  
> Uint64;
> > -Original Message-
> > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > Sent: Thursday, November 29, 2018 8:31 PM
> > To: edk2-devel@lists.01.org
> > Cc: Ard Biesheuvel ; Laszlo Ersek 
> > ; Zhu, Yonghong ; Gao,
> > Liming ; Feng, Bob C 
> > Subject: [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word size in 
> > IP address handling
> >
> > In the context of the BaseTools, there is no such thing as a native word
> > size, given that the same set of tools may be used to build a firmware
> > image consisting of both 32-bit and 64-bit modules.
> >
> > So update StrToIpv4Address() and StrToIpv6Address() to use UINT64
> > types instead of UINTN types when parsing strings.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel 
> > ---
> >  BaseTools/Source/C/Common/CommonLib.c | 28 ++--
> >  1 file changed, 14 insertions(+), 14 deletions(-)
> >
> > diff --git a/BaseTools/Source/C/Common/CommonLib.c 
> > b/BaseTools/Source/C/Common/CommonLib.c
> > index 618aadac781a..bea6af0a45b1 100644
> > --- a/BaseTools/Source/C/Common/CommonLib.c
> > +++ b/BaseTools/Source/C/Common/CommonLib.c
> > @@ -1785,7 +1785,7 @@ StrToIpv4Address (
> >  {
> >RETURN_STATUS  Status;
> >UINTN  AddressIndex;
> > -  UINTN  Uintn;
> > +  UINTN  Uint64;
> >EFI_IPv4_ADDRESS   LocalAddress;
> >UINT8  LocalPrefixLength;
> >CHAR16 *Pointer;
> > @@ -1812,7 +1812,7 @@ StrToIpv4Address (
> >  //
> >  // Get D or P.
> >  //
> > -Status = StrDecimalToUintnS ((CONST CHAR16 *) Pointer, &Pointer, 
> > &Uintn);
> > +Status = StrDecimalToUint64S ((CONST CHAR16 *) Pointer, &Pointer, 
> > &Uint64);
> >  if (RETURN_ERROR (Status)) {
> >return RETURN_UNSUPPORTED;
> >  }
> > @@ -1820,18 +1820,18 @@ StrToIpv4Address (
> >//
> >// It's P.
> >//
> > -  if (Uintn > 32) {
> > +  if (Uint64 > 32) {
> >  return RETURN_UNSUPPORTED;
> >}
> > -  LocalPrefixLength = (UINT8) Uintn;
> > +  LocalPrefixLength = (UINT8) Uint64;
> >  } else {
> >//
> >// It's D.
> >//
> > -  if (Uintn > MAX_UINT8) {
> > +  if (Uint64 > MAX_UINT8) {
> >  return RETURN_UNSUPPORTED;
> >}
> > -  LocalAddress.Addr[AddressIndex] = (UINT8) Uintn;
> > +  LocalAddress.Addr[AddressIndex] = (UINT8) Uint64;
> >AddressIndex++;
> >  }
> >
> > @@ -1888,7 +1888,7 @@ StrToIpv6Address (
> >  {
> >RETURN_STATUS  Status;
> >UINTN  AddressIndex;
> > -  UINTN  Uintn;
> > +  UINT64 Uint64;
> >EFI_IPv6_ADDRESS   LocalAddress;
> >UINT8  LocalPrefixLength;
> >CONST CHAR16   *Pointer;
> > @@ -1969,7 +1969,7 @@ StrToIpv6Address (
> >  //
> >  // Get X.
> >  //
> > -Status = StrHexToUintnS (Pointer, &End, &Uintn);
> > +Status = StrHexToUint64S (Pointer, &End, &Uint64);
> >  if (RETURN_ERROR (Status) || End - Pointer > 4) {
> >//
> >// Number of hexadecimal digit characters is no more than 4.
> > @@ -1978,24 +1978,24 @@ StrToIpv6Address (
> >  }
> >  Pointer = End;
> >  //
> > -// Uintn won't exceed MAX_UINT16 if number of hexadecimal digit 
> > characters is no more than 4.
> > +// Uint64 won't exceed MAX_UINT16 if number of hexadecimal digit 
> > characters is no more than 4.
> >  //
> >  ASSERT (AddressIndex + 1 < ARRAY_SIZE (Address->Addr));
> > -LocalAddress.Addr[AddressIndex] = (UINT8) ((UINT16) Uintn >> 8);
> > -LocalAddress.Addr[AddressIndex + 1] = (UINT8) Uintn;
> > +LocalAddress.Addr[AddressIndex] = (UINT8) ((UINT16) Uint64 >> 8);
> > +LocalAddress.Addr[AddressIndex + 1] = (UINT8) Uint64;
> >  AddressIndex += 2;
> >} else {
> >  //
> >  // Get P, then exit the loop.
> >  //
> > -Status = StrDecimalToUintnS (Pointer, &End, &Uintn);
> > -if (RETURN_ERROR (Status) || End == Pointer || Uintn > 128) {
> > +Status = StrDecimalToUint64S (Pointer, &End, &U

Re: [edk2] [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling

2018-11-29 Thread Gao, Liming
Do you verify which GCC arch? 32bit or 64bit or ARM?

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Thursday, November 29, 2018 11:14 PM
> To: Gao, Liming 
> Cc: edk2-devel@lists.01.org; Laszlo Ersek ; Zhu, Yonghong 
> ; Feng, Bob C
> 
> Subject: Re: [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word size 
> in IP address handling
> 
> On Thu, 29 Nov 2018 at 16:11, Gao, Liming  wrote:
> >
> > Ard:
> >   I mean the build error. Besides, what test have you done with this patch 
> > set?
> >
> > CommonLib.c(1651): error C2220: warning treated as error - no 'object' file 
> > generated
> > CommonLib.c(1651): warning C4133: 'function': incompatible types - from 
> > 'UINTN *' to 'UINT64 *'
> > NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual Studio 
> > 14.0\VC\BIN\cl.exe"' : return code '0x2'
> >
> 
> Apologies, i missed this change at line 1624
> 
> -  UINTN  Uint64;
> +  UINT64 Uint64;
> 
> It builds fine with GCC though.
> 
> > Below > +  UINTN  Uint64; ==> > +  UINT64  
> > Uint64;
> > > -Original Message-
> > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > Sent: Thursday, November 29, 2018 8:31 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Ard Biesheuvel ; Laszlo Ersek 
> > > ; Zhu, Yonghong ;
> Gao,
> > > Liming ; Feng, Bob C 
> > > Subject: [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word size 
> > > in IP address handling
> > >
> > > In the context of the BaseTools, there is no such thing as a native word
> > > size, given that the same set of tools may be used to build a firmware
> > > image consisting of both 32-bit and 64-bit modules.
> > >
> > > So update StrToIpv4Address() and StrToIpv6Address() to use UINT64
> > > types instead of UINTN types when parsing strings.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Ard Biesheuvel 
> > > ---
> > >  BaseTools/Source/C/Common/CommonLib.c | 28 ++--
> > >  1 file changed, 14 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/BaseTools/Source/C/Common/CommonLib.c 
> > > b/BaseTools/Source/C/Common/CommonLib.c
> > > index 618aadac781a..bea6af0a45b1 100644
> > > --- a/BaseTools/Source/C/Common/CommonLib.c
> > > +++ b/BaseTools/Source/C/Common/CommonLib.c
> > > @@ -1785,7 +1785,7 @@ StrToIpv4Address (
> > >  {
> > >RETURN_STATUS  Status;
> > >UINTN  AddressIndex;
> > > -  UINTN  Uintn;
> > > +  UINTN  Uint64;
> > >EFI_IPv4_ADDRESS   LocalAddress;
> > >UINT8  LocalPrefixLength;
> > >CHAR16 *Pointer;
> > > @@ -1812,7 +1812,7 @@ StrToIpv4Address (
> > >  //
> > >  // Get D or P.
> > >  //
> > > -Status = StrDecimalToUintnS ((CONST CHAR16 *) Pointer, &Pointer, 
> > > &Uintn);
> > > +Status = StrDecimalToUint64S ((CONST CHAR16 *) Pointer, &Pointer, 
> > > &Uint64);
> > >  if (RETURN_ERROR (Status)) {
> > >return RETURN_UNSUPPORTED;
> > >  }
> > > @@ -1820,18 +1820,18 @@ StrToIpv4Address (
> > >//
> > >// It's P.
> > >//
> > > -  if (Uintn > 32) {
> > > +  if (Uint64 > 32) {
> > >  return RETURN_UNSUPPORTED;
> > >}
> > > -  LocalPrefixLength = (UINT8) Uintn;
> > > +  LocalPrefixLength = (UINT8) Uint64;
> > >  } else {
> > >//
> > >// It's D.
> > >//
> > > -  if (Uintn > MAX_UINT8) {
> > > +  if (Uint64 > MAX_UINT8) {
> > >  return RETURN_UNSUPPORTED;
> > >}
> > > -  LocalAddress.Addr[AddressIndex] = (UINT8) Uintn;
> > > +  LocalAddress.Addr[AddressIndex] = (UINT8) Uint64;
> > >AddressIndex++;
> > >  }
> > >
> > > @@ -1888,7 +1888,7 @@ StrToIpv6Address (
> > >  {
> > >RETURN_STATUS  Status;
> > >UINTN  AddressIndex;
> > > -  UINTN  Uintn;
> > > +  UINT64 Uint64;
> > >EFI_IPv6_ADDRESS   LocalAddress;
> > >UINT8  LocalPrefixLength;
> > >CONST CHAR16   *Pointer;
> > > @@ -1969,7 +1969,7 @@ StrToIpv6Address (
> > >  //
> > >  // Get X.
> > >  //
> > > -Status = StrHexToUintnS (Pointer, &End, &Uintn);
> > > +Status = StrHexToUint64S (Pointer, &End, &Uint64);
> > >  if (RETURN_ERROR (Status) || End - Pointer > 4) {
> > >//
> > >// Number of hexadecimal digit characters is no more than 4.
> > > @@ -1978,24 +1978,24 @@ StrToIpv6Address (
> > >  }
> > >  Pointer = End;
> > >  //
> > > -// Uintn won't exceed MAX_UINT16 if number of hexadecimal digit 
> > > characters is no more than 4.
> > > +// Uint64 won't exceed MAX_UINT16 if number of hexadecimal digit 
> > > characters is no more than 4.
> > >  //
> > >  ASSERT (AddressIndex + 1 < ARRAY_SI

Re: [edk2] [PATCH 3/6] BaseTools/DevicePath: use explicit 64-bit number parsing routines

2018-11-29 Thread Carsey, Jaben
Reviewed-by: Jaben Carsey 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Thursday, November 29, 2018 4:31 AM
> To: edk2-devel@lists.01.org
> Cc: Laszlo Ersek ; Gao, Liming 
> Subject: [edk2] [PATCH 3/6] BaseTools/DevicePath: use explicit 64-bit
> number parsing routines
> 
> Replace invocations of StrHexToUintn() with StrHexToUint64(), so
> that we can drop the former.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  BaseTools/Source/C/DevicePath/DevicePathFromText.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/BaseTools/Source/C/DevicePath/DevicePathFromText.c
> b/BaseTools/Source/C/DevicePath/DevicePathFromText.c
> index 555efa1acdde..6151926af9aa 100644
> --- a/BaseTools/Source/C/DevicePath/DevicePathFromText.c
> +++ b/BaseTools/Source/C/DevicePath/DevicePathFromText.c
> @@ -520,7 +520,7 @@ EisaIdFromText (
>return (((Text[0] - 'A' + 1) & 0x1f) << 10)
> + (((Text[1] - 'A' + 1) & 0x1f) <<  5)
> + (((Text[2] - 'A' + 1) & 0x1f) <<  0)
> -   + (UINT32) (StrHexToUintn (&Text[3]) << 16)
> +   + (UINT32) (StrHexToUint64 (&Text[3]) << 16)
> ;
>  }
> 
> @@ -1506,7 +1506,7 @@ DevPathFromTextNVMe (
> 
>Index = sizeof (Nvme->NamespaceUuid) / sizeof (UINT8);
>while (Index-- != 0) {
> -Uuid[Index] = (UINT8) StrHexToUintn (SplitStr (&NamespaceUuidStr, L'-
> '));
> +Uuid[Index] = (UINT8) StrHexToUint64 (SplitStr (&NamespaceUuidStr, L'-
> '));
>}
> 
>return (EFI_DEVICE_PATH_PROTOCOL *) Nvme;
> --
> 2.19.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH 2/6] BaseTools/CommonLib: use explicit 64-bit type in Strtoi()

2018-11-29 Thread Carsey, Jaben
Reviewed-by: Jaben Carsey 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Thursday, November 29, 2018 4:31 AM
> To: edk2-devel@lists.01.org
> Cc: Laszlo Ersek ; Gao, Liming 
> Subject: [edk2] [PATCH 2/6] BaseTools/CommonLib: use explicit 64-bit type
> in Strtoi()
> 
> Don't use the native word size string to number parsing routines,
> but instead, use the 64-bit one and cast to UINTN.
> 
> Currently, the only user is in Source/C/DevicePath/DevicePathFromText.c
> which takes care to use Strtoi64 () unless it assumes the value fits
> in 32-bit, so this change is a no-op even on 32-bit build hosts.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  BaseTools/Source/C/Common/CommonLib.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/BaseTools/Source/C/Common/CommonLib.c
> b/BaseTools/Source/C/Common/CommonLib.c
> index bea6af0a45b1..c5e32b1292e0 100644
> --- a/BaseTools/Source/C/Common/CommonLib.c
> +++ b/BaseTools/Source/C/Common/CommonLib.c
> @@ -2252,9 +2252,9 @@ Strtoi (
>)
>  {
>if (IsHexStr (Str)) {
> -return StrHexToUintn (Str);
> +return (UINTN)StrHexToUint64 (Str);
>} else {
> -return StrDecimalToUintn (Str);
> +return (UINTN)StrDecimalToUint64 (Str);
>}
>  }
> 
> --
> 2.19.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word size in IP address handling

2018-11-29 Thread Ard Biesheuvel
On Thu, 29 Nov 2018 at 16:15, Gao, Liming  wrote:
>
> Do you verify which GCC arch? 32bit or 64bit or ARM?
>

64-bit ARM

> > -Original Message-
> > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > Sent: Thursday, November 29, 2018 11:14 PM
> > To: Gao, Liming 
> > Cc: edk2-devel@lists.01.org; Laszlo Ersek ; Zhu, 
> > Yonghong ; Feng, Bob C
> > 
> > Subject: Re: [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word 
> > size in IP address handling
> >
> > On Thu, 29 Nov 2018 at 16:11, Gao, Liming  wrote:
> > >
> > > Ard:
> > >   I mean the build error. Besides, what test have you done with this 
> > > patch set?
> > >
> > > CommonLib.c(1651): error C2220: warning treated as error - no 'object' 
> > > file generated
> > > CommonLib.c(1651): warning C4133: 'function': incompatible types - from 
> > > 'UINTN *' to 'UINT64 *'
> > > NMAKE : fatal error U1077: '"C:\Program Files (x86)\Microsoft Visual 
> > > Studio 14.0\VC\BIN\cl.exe"' : return code '0x2'
> > >
> >
> > Apologies, i missed this change at line 1624
> >
> > -  UINTN  Uint64;
> > +  UINT64 Uint64;
> >
> > It builds fine with GCC though.
> >
> > > Below > +  UINTN  Uint64; ==> > +  UINT64 
> > >  Uint64;
> > > > -Original Message-
> > > > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > > > Sent: Thursday, November 29, 2018 8:31 PM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: Ard Biesheuvel ; Laszlo Ersek 
> > > > ; Zhu, Yonghong ;
> > Gao,
> > > > Liming ; Feng, Bob C 
> > > > Subject: [PATCH 1/6] BaseTools/CommonLib: avoid using 'native' word 
> > > > size in IP address handling
> > > >
> > > > In the context of the BaseTools, there is no such thing as a native word
> > > > size, given that the same set of tools may be used to build a firmware
> > > > image consisting of both 32-bit and 64-bit modules.
> > > >
> > > > So update StrToIpv4Address() and StrToIpv6Address() to use UINT64
> > > > types instead of UINTN types when parsing strings.
> > > >
> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > Signed-off-by: Ard Biesheuvel 
> > > > ---
> > > >  BaseTools/Source/C/Common/CommonLib.c | 28 ++--
> > > >  1 file changed, 14 insertions(+), 14 deletions(-)
> > > >
> > > > diff --git a/BaseTools/Source/C/Common/CommonLib.c 
> > > > b/BaseTools/Source/C/Common/CommonLib.c
> > > > index 618aadac781a..bea6af0a45b1 100644
> > > > --- a/BaseTools/Source/C/Common/CommonLib.c
> > > > +++ b/BaseTools/Source/C/Common/CommonLib.c
> > > > @@ -1785,7 +1785,7 @@ StrToIpv4Address (
> > > >  {
> > > >RETURN_STATUS  Status;
> > > >UINTN  AddressIndex;
> > > > -  UINTN  Uintn;
> > > > +  UINTN  Uint64;
> > > >EFI_IPv4_ADDRESS   LocalAddress;
> > > >UINT8  LocalPrefixLength;
> > > >CHAR16 *Pointer;
> > > > @@ -1812,7 +1812,7 @@ StrToIpv4Address (
> > > >  //
> > > >  // Get D or P.
> > > >  //
> > > > -Status = StrDecimalToUintnS ((CONST CHAR16 *) Pointer, &Pointer, 
> > > > &Uintn);
> > > > +Status = StrDecimalToUint64S ((CONST CHAR16 *) Pointer, &Pointer, 
> > > > &Uint64);
> > > >  if (RETURN_ERROR (Status)) {
> > > >return RETURN_UNSUPPORTED;
> > > >  }
> > > > @@ -1820,18 +1820,18 @@ StrToIpv4Address (
> > > >//
> > > >// It's P.
> > > >//
> > > > -  if (Uintn > 32) {
> > > > +  if (Uint64 > 32) {
> > > >  return RETURN_UNSUPPORTED;
> > > >}
> > > > -  LocalPrefixLength = (UINT8) Uintn;
> > > > +  LocalPrefixLength = (UINT8) Uint64;
> > > >  } else {
> > > >//
> > > >// It's D.
> > > >//
> > > > -  if (Uintn > MAX_UINT8) {
> > > > +  if (Uint64 > MAX_UINT8) {
> > > >  return RETURN_UNSUPPORTED;
> > > >}
> > > > -  LocalAddress.Addr[AddressIndex] = (UINT8) Uintn;
> > > > +  LocalAddress.Addr[AddressIndex] = (UINT8) Uint64;
> > > >AddressIndex++;
> > > >  }
> > > >
> > > > @@ -1888,7 +1888,7 @@ StrToIpv6Address (
> > > >  {
> > > >RETURN_STATUS  Status;
> > > >UINTN  AddressIndex;
> > > > -  UINTN  Uintn;
> > > > +  UINT64 Uint64;
> > > >EFI_IPv6_ADDRESS   LocalAddress;
> > > >UINT8  LocalPrefixLength;
> > > >CONST CHAR16   *Pointer;
> > > > @@ -1969,7 +1969,7 @@ StrToIpv6Address (
> > > >  //
> > > >  // Get X.
> > > >  //
> > > > -Status = StrHexToUintnS (Pointer, &End, &Uintn);
> > > > +Status = StrHexToUint64S (Pointer, &End, &Uint64);
> > > >  if (RETURN_ERROR (Status) || End - Pointer > 4) {
> > > >//
> > > >// Number of hexadecimal digit characters is no more than 4.
> > > > @@ -1978,24 +1978,24 @@ StrToIpv6Address (
> > > >  }
> > > >  Pointer = End;
> > 

Re: [edk2] [PATCH 4/6] BaseTools/DevicePath: use MAX_UINT16 as default device path max size

2018-11-29 Thread Carsey, Jaben
Reviewed-by: Jaben Carsey 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Thursday, November 29, 2018 4:31 AM
> To: edk2-devel@lists.01.org
> Cc: Laszlo Ersek ; Gao, Liming 
> Subject: [edk2] [PATCH 4/6] BaseTools/DevicePath: use MAX_UINT16 as
> default device path max size
> 
> Replace the default size limit of IsDevicePathValid() with a value
> that does not depend on the native word size of the build host.
> 
> 64 KB seems sufficient as the upper bound of a device path handled
> by UEFI.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  BaseTools/Source/C/DevicePath/DevicePathUtilities.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
> b/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
> index d4ec2742b7c8..ba7f83e53070 100644
> --- a/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
> +++ b/BaseTools/Source/C/DevicePath/DevicePathUtilities.c
> @@ -62,7 +62,7 @@ IsDevicePathValid (
>ASSERT (DevicePath != NULL);
> 
>if (MaxSize == 0) {
> -MaxSize = MAX_UINTN;
> +MaxSize = MAX_UINT16;
>   }
> 
>//
> @@ -78,7 +78,7 @@ IsDevicePathValid (
>return FALSE;
>  }
> 
> -if (NodeLength > MAX_UINTN - Size) {
> +if (NodeLength > MAX_UINT16 - Size) {
>return FALSE;
>  }
>  Size += NodeLength;
> --
> 2.19.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH 6/6] BaseTools/CommonLib: drop definition of MAX_UINTN

2018-11-29 Thread Carsey, Jaben
Reviewed-by: Jaben Carsey 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Thursday, November 29, 2018 4:31 AM
> To: edk2-devel@lists.01.org
> Cc: Laszlo Ersek ; Gao, Liming 
> Subject: [edk2] [PATCH 6/6] BaseTools/CommonLib: drop definition of
> MAX_UINTN
> 
> The maximum value that can be represented by the native word size
> of the *target* should be irrelevant when compiling tools that
> run on the build *host*. So drop the definition of MAX_UINTN, now
> that we no longer use it.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  BaseTools/Source/C/Common/CommonLib.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/BaseTools/Source/C/Common/CommonLib.h
> b/BaseTools/Source/C/Common/CommonLib.h
> index 6930d9227b87..b1c6c00a3478 100644
> --- a/BaseTools/Source/C/Common/CommonLib.h
> +++ b/BaseTools/Source/C/Common/CommonLib.h
> @@ -22,7 +22,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
> 
>  #define MAX_LONG_FILE_PATH 500
> 
> -#define MAX_UINTN MAX_ADDRESS
>  #define MAX_UINT64 ((UINT64)0xULL)
>  #define MAX_UINT16  ((UINT16)0x)
>  #define MAX_UINT8   ((UINT8)0xFF)
> --
> 2.19.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH v3 05/16] MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits

2018-11-29 Thread Gao, Liming



> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
> Ersek
> Sent: Thursday, November 29, 2018 7:34 PM
> To: Ard Biesheuvel 
> Cc: Andrew Jones ; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH v3 05/16] MdePkg/ProcessorBind.h AARCH64: limit 
> MAX_ADDRESS to 48 bits
> 
> On 11/29/18 11:40, Ard Biesheuvel wrote:
> > On Wed, 28 Nov 2018 at 19:41, Laszlo Ersek  wrote:
> >>
> >> On 11/28/18 15:33, Ard Biesheuvel wrote:
> >>> AArch64 supports the use of more than 48 bits for physical and/or
> >>> virtual addressing, but only if the page size is set to 64 KB,
> >>> which is not supported by UEFI. So redefine MAX_ADDRESS to cover
> >>> only 48 address bits.
> >>>
> >>> Contributed-under: TianoCore Contribution Agreement 1.1
> >>> Signed-off-by: Ard Biesheuvel 
> >>> Reviewed-by: Leif Lindholm 
> >>> ---
> >>>  MdePkg/Include/AArch64/ProcessorBind.h | 4 ++--
> >>>  1 file changed, 2 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/MdePkg/Include/AArch64/ProcessorBind.h 
> >>> b/MdePkg/Include/AArch64/ProcessorBind.h
> >>> index 968c18f915ae..dad75df1c579 100644
> >>> --- a/MdePkg/Include/AArch64/ProcessorBind.h
> >>> +++ b/MdePkg/Include/AArch64/ProcessorBind.h
> >>> @@ -138,9 +138,9 @@ typedef INT64   INTN;
> >>>  #define MAX_2_BITS  0xC000ULL
> >>>
> >>>  ///
> >>> -/// Maximum legal AARCH64  address
> >>> +/// Maximum legal AARCH64  address (48 bits for 4 KB page size)
> >>>  ///
> >>> -#define MAX_ADDRESS   0xULL
> >>> +#define MAX_ADDRESS   0xULL
> >>>
> >>>  ///
> >>>  /// Maximum legal AArch64 INTN and UINTN values.
> >>>
> >>
> >> Hmmm.
> >>
> >> I bit the bullet and grepped the tree for MAX_ADDRESS.
> >>
> >> The amount of hits is staggering. I can't audit all of them.
> >>
> >> Generally, MAX_ADDRESS seems to be used in checks that prevent address
> >> wrap-around. In that regard, this change looks valid.
> >>
> >> I can't guarantee this change won't regress anything though. In the
> >> previous posting of this patch, I asked Liming some questions (IIRC):
> >>
> >> 6f1209fb-bb89-a70f-ba0e-3ebf2e12e459@redhat.com">http://mid.mail-archive.com/6f1209fb-bb89-a70f-ba0e-3ebf2e12e459@redhat.com
> >>
> >> It would be nice to see answers. :)
> >>
> >
> > Yep
> >
> >> In addition:
> >>
> >> (a) in "BaseTools/Source/C/Include/AArch64/ProcessorBind.h", we have
> >> another instance of the macro definition. I suspect it should be kept in
> >> sync.
> >>
> >
> > Indeed.
> >
> >> (b) in "BaseTools/Source/C/Common/CommonLib.h", we have:
> >>
> >> #define MAX_UINTN MAX_ADDRESS
> >>
> >> which I think relies on (a), and hence it will be amusingly wrong after
> >> we synchronize (a) with MdePkg.
> >>
> >> (BTW, (b) is exactly the kind of assumption that scares me about this
> >> patch.)
> >>
> >
> > That doesn't make any sense at all. What does 'native' mean in the
> > context of BaseTools anyway?
> 
> I can't tell whether this MAX_UINTN definition is for BaseTools itself
> (i.e., "native") or for the build target arch.
> 
> "CommonLib.c" has a number of instances of MAX_UINTN... Hm, they are in
> the following two functions:
> 
> - StrDecimalToUintnS() -- wrapped by StrDecimalToUintn(),
>   StrToIpv4Address(), and StrToIpv6Address())
> 
> - StrHexToUintnS() -- wrapped by StrHexToUintn(), and
>   StrToIpv6Address().
> 
> I tried to look at where those are called from, and the picture remains
> muddled.
> 
> For example, StrHexToUintn() is called from
> "BaseTools/Source/C/DevicePath/DevicePathFromText.c", functions
> EisaIdFromText() and DevPathFromTextNVMe(). These functions seem to
> compose binary devpath nodes from text *during the build*, likely for
> the firmware-under-build to consume as-is. Hence, this use of MAX_UINTN
> -- through StrHexToUintn() -- qualifies as *native* use. And for that,
> the MAX_UINTN definition in "CommonLib.h" looks wrong, independently of
> your patch series.

I agree this is an issue in BaseTools. BaseTools should generate the same 
result no matter on the host with the different arch. 

For this change in MdePkg, it makes sense to me. Reviewed-by: Liming Gao 

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


Re: [edk2] [PATCH v3 01/16] EmbeddedPkg/TemplateSec: remove unused module

2018-11-29 Thread Leif Lindholm
On Wed, Nov 28, 2018 at 03:33:42PM +0100, Ard Biesheuvel wrote:
> Remove this module: it is unused, and should not be used as an
> example going forward.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Leif Lindholm 

> ---
>  EmbeddedPkg/TemplateSec/TemplateSec.inf | 65 -
>  EmbeddedPkg/TemplateSec/TemplateSec.c   | 76 
>  2 files changed, 141 deletions(-)
> 
> diff --git a/EmbeddedPkg/TemplateSec/TemplateSec.inf 
> b/EmbeddedPkg/TemplateSec/TemplateSec.inf
> deleted file mode 100644
> index 3a63e59294d3..
> --- a/EmbeddedPkg/TemplateSec/TemplateSec.inf
> +++ /dev/null
> @@ -1,65 +0,0 @@
> -#/** @file
> -#
> -#Component description file for DxeIpl module
> -#
> -#   The responsibility of this module is to load the DXE Core from a 
> Firmware Volume. This implementation i used to load a 32-bit DXE Core.
> -#
> -#  Copyright (c) 2006 - 2008, 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
> -#  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.
> -#
> -#**/
> -
> -[Defines]
> -  INF_VERSION= 0x00010005
> -  BASE_NAME  = TemplateSec
> -  FILE_GUID  = 1D6F730F-5A55-4078-869B-E0A18324BDC8
> -  MODULE_TYPE= SEC
> -  VERSION_STRING = 1.0
> -
> -
> -#
> -# The following information is for reference only and not required by the 
> build tools.
> -#
> -#  VALID_ARCHITECTURES   = IA32 X64 ARM
> -#
> -
> -[Sources.common]
> -  TemplateSec.c
> -
> -[Sources.Ia32]
> -#  Ia32/ResetVector.asm | MSFT
> -#  Ia32/ResetVector.S   | GCC
> -
> -[Sources.X64]
> -#  X64/ResetVector.asm | MSFT
> -#  X64/ResetVector.S   | GCC
> -
> -[Sources.ARM]
> -#  Arm/ResetVector.asm | RVCT
> -#  Arm/ResetVector.S   | GCC
> -
> -[Packages]
> -  MdePkg/MdePkg.dec
> -  MdeModulePkg/MdeModulePkg.dec
> -  EmbeddedPkg/EmbeddedPkg.dec
> -
> -
> -[LibraryClasses]
> -  BaseLib
> -  DebugLib
> -  BaseMemoryLib
> -  UefiDecompressLib
> -  PeCoffLib
> -  CacheMaintenanceLib
> -  PrePiLib
> -
> -[Pcd]
> -  gEmbeddedTokenSpaceGuid.PcdEmbeddedFdBaseAddress
> -  gEmbeddedTokenSpaceGuid.PcdEmbeddedFdSize
> -
> diff --git a/EmbeddedPkg/TemplateSec/TemplateSec.c 
> b/EmbeddedPkg/TemplateSec/TemplateSec.c
> deleted file mode 100644
> index c63adbb6f90f..
> --- a/EmbeddedPkg/TemplateSec/TemplateSec.c
> +++ /dev/null
> @@ -1,76 +0,0 @@
> -/** @file
> -
> -  Copyright (c) 2008 - 2009, Apple Inc. 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
> -  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.
> -
> -**/
> -
> -#include 
> -
> -#include 
> -#include 
> -#include 
> -
> -#include 
> -
> -VOID
> -_ModuleEntryPoint (
> -  VOID
> -  )
> -{
> -}
> -
> -VOID
> -CEntryPoint (
> -  VOID*MemoryBase,
> -  UINTN   MemorySize,
> -  VOID*StackBase,
> -  UINTN   StackSize
> -  )
> -{
> -  EFI_PHYSICAL_ADDRESS  MemoryBegin;
> -  UINT64MemoryLength;
> -  VOID  *HobBase;
> -
> -  //
> -  // Boot strap the C environment so the other library services will work 
> properly.
> -  //
> -  MemoryBegin  = (EFI_PHYSICAL_ADDRESS)(UINTN)MemoryBase;
> -  MemoryLength = (UINT64)MemorySize;
> -  HobBase  = (VOID *)(UINTN)(FixedPcdGet32(PcdEmbeddedFdBaseAddress) + 
> FixedPcdGet32(PcdEmbeddedFdSize));
> -  CreateHobList (MemoryBase, MemorySize, HobBase, StackBase);
> -
> -  MemoryBegin  = (EFI_PHYSICAL_ADDRESS)(UINTN)StackBase;
> -  MemoryLength = (UINT64)StackSize;
> -  UpdateStackHob (MemoryBegin, MemoryLength);
> -
> -  DEBUG ((DEBUG_ERROR, "CEntryPoint (%x,%x,%x,%x)\n", MemoryBase, 
> MemorySize, StackBase, StackSize));
> -
> -  //
> -  // Add your C code stuff here
> -  //
> -
> -
> -  //
> -  // Load the DXE Core and transfer control to it
> -  //
> -
> -  // Give the DXE Core access to our DEBUG and ASSERT infrastructure so this 
> will work prior
> -  // to the DXE version being loaded. Thus we close the debugging gap 
> between phases.
> -  AddDxeCoreReportStatusCodeCallback ();
> -
> -  //BuildFvHobs (PcdBfvBase, PcdBfvSize, NULL);
> -
> -  LoadDxeCoreFromFv (NULL, 0);
> -
> -  // DXE Core should always load and never return
> -  ASSERT (FALSE);
> -}
> -
> -- 
> 2.19.

Re: [edk2] [PATCH v3 02/16] EmbeddedPkg/PrePiHobLib: drop CreateHobList() from library

2018-11-29 Thread Leif Lindholm
On Wed, Nov 28, 2018 at 03:33:43PM +0100, Ard Biesheuvel wrote:
> Drop the declaration and the implementation of CreateHoblist(),
> which is not used anywhere.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Leif Lindholm 

> ---
>  EmbeddedPkg/Include/Library/PrePiLib.h | 18 -
>  EmbeddedPkg/Library/PrePiHobLib/Hob.c  | 41 
>  2 files changed, 59 deletions(-)
> 
> diff --git a/EmbeddedPkg/Include/Library/PrePiLib.h 
> b/EmbeddedPkg/Include/Library/PrePiLib.h
> index cf70fca3b619..a857308ecec2 100644
> --- a/EmbeddedPkg/Include/Library/PrePiLib.h
> +++ b/EmbeddedPkg/Include/Library/PrePiLib.h
> @@ -274,24 +274,6 @@ HobConstructor (
>IN VOID   *EfiFreeMemoryTop
>);
>  
> -/**
> -  Retrieves the magic value from the PE/COFF header.
> -
> -  @param  Hdr The buffer in which to return the PE32, PE32+, or 
> TE header.
> -
> -  @return EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC - Image is PE32
> -  @return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC - Image is PE32+
> -
> -**/
> -VOID
> -CreateHobList (
> -  IN VOID   *MemoryBegin,
> -  IN UINTN  MemoryLength,
> -  IN VOID   *HobBase,
> -  IN VOID   *StackBase
> -  );
> -
> -
>  /**
>This service enables PEIMs to create various types of HOBs.
>  
> diff --git a/EmbeddedPkg/Library/PrePiHobLib/Hob.c 
> b/EmbeddedPkg/Library/PrePiHobLib/Hob.c
> index aff8ea05797b..ba16899a9184 100644
> --- a/EmbeddedPkg/Library/PrePiHobLib/Hob.c
> +++ b/EmbeddedPkg/Library/PrePiHobLib/Hob.c
> @@ -175,47 +175,6 @@ BuildResourceDescriptorHob (
>Hob->ResourceLength= NumberOfBytes;
>  }
>  
> -/**
> -
> -
> -**/
> -VOID
> -CreateHobList (
> -  IN VOID   *MemoryBegin,
> -  IN UINTN  MemoryLength,
> -  IN VOID   *HobBase,
> -  IN VOID   *StackBase
> -  )
> -{
> -  EFI_HOB_HANDOFF_INFO_TABLE  *Hob;
> -  EFI_RESOURCE_ATTRIBUTE_TYPE Attributes;
> -
> -  Hob = HobConstructor (MemoryBegin,MemoryLength,HobBase,StackBase);
> -  SetHobList (Hob);
> -
> -  BuildCpuHob (PcdGet8 (PcdPrePiCpuMemorySize), PcdGet8 (PcdPrePiCpuIoSize));
> -
> -  Attributes =(
> -EFI_RESOURCE_ATTRIBUTE_PRESENT |
> -EFI_RESOURCE_ATTRIBUTE_INITIALIZED |
> -EFI_RESOURCE_ATTRIBUTE_TESTED |
> -EFI_RESOURCE_ATTRIBUTE_UNCACHEABLE |
> -EFI_RESOURCE_ATTRIBUTE_WRITE_COMBINEABLE |
> -EFI_RESOURCE_ATTRIBUTE_WRITE_THROUGH_CACHEABLE |
> -EFI_RESOURCE_ATTRIBUTE_WRITE_BACK_CACHEABLE
> -  );
> -
> -  BuildResourceDescriptorHob (EFI_RESOURCE_SYSTEM_MEMORY, Attributes, 
> (UINTN)MemoryBegin, MemoryLength);
> -
> -  BuildStackHob ((EFI_PHYSICAL_ADDRESS)(UINTN)StackBase, ((UINTN)MemoryBegin 
> + MemoryLength) - (UINTN)StackBase);
> -
> -  if (FeaturePcdGet (PcdPrePiProduceMemoryTypeInformationHob)) {
> -// Optional feature that helps prevent EFI memory map fragmentation.
> -BuildMemoryTypeInformationHob ();
> -  }
> -}
> -
> -
>  VOID
>  EFIAPI
>  BuildFvHobs (
> -- 
> 2.19.1
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH v3 06/16] ArmPkg/ArmLib: add support for reading the max physical address space size

2018-11-29 Thread Leif Lindholm
On Wed, Nov 28, 2018 at 03:33:47PM +0100, Ard Biesheuvel wrote:
> Add a helper function that returns the maximum physical address space
> size as supported by the current CPU.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Leif Lindholm 

> ---
>  ArmPkg/Include/Library/ArmLib.h   |  6 ++
>  ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S | 17 +
>  ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S |  8 
>  ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm   |  8 
>  4 files changed, 39 insertions(+)
> 
> diff --git a/ArmPkg/Include/Library/ArmLib.h b/ArmPkg/Include/Library/ArmLib.h
> index ffda50e9d767..9a804c15fdb6 100644
> --- a/ArmPkg/Include/Library/ArmLib.h
> +++ b/ArmPkg/Include/Library/ArmLib.h
> @@ -733,4 +733,10 @@ ArmWriteCntvOff (
>UINT64   Val
>);
>  
> +UINTN
> +EFIAPI
> +ArmGetPhysicalAddressBits (
> +  VOID
> +  );
> +
>  #endif // __ARM_LIB__
> diff --git a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S 
> b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> index 1ef2f61f5979..b7173e00b039 100644
> --- a/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> +++ b/ArmPkg/Library/ArmLib/AArch64/ArmLibSupport.S
> @@ -196,4 +196,21 @@ ASM_FUNC(ArmWriteSctlr)
>  3:msr   sctlr_el3, x0
>  4:ret
>  
> +ASM_FUNC(ArmGetPhysicalAddressBits)
> +  mrs   x0, id_aa64mmfr0_el1
> +  adr   x1, .LPARanges
> +  and   x0, x0, #0xf
> +  ldrb  w0, [x1, x0]
> +  ret
> +
> +//
> +// Bits 0..3 of the AA64MFR0_EL1 system register encode the size of the
> +// physical address space support on this CPU:
> +// 0 == 32 bits, 1 == 36 bits, etc etc
> +// 7 and up are reserved
> +//
> +.LPARanges:
> +  .byte 32, 36, 40, 42, 44, 48, 52,  0
> +  .byte  0,  0,  0,  0,  0,  0,  0,  0
> +
>  ASM_FUNCTION_REMOVE_IF_UNREFERENCED
> diff --git a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S 
> b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
> index f2a517671f0a..0e9f9d0453e4 100644
> --- a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
> +++ b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.S
> @@ -165,4 +165,12 @@ ASM_FUNC(ArmWriteCpuActlr)
>isb
>bx  lr
>  
> +ASM_FUNC (ArmGetPhysicalAddressBits)
> +  mrc p15, 0, r0, c0, c1, 4   // MMFR0
> +  and r0, r0, #0xf// VMSA [3:0]
> +  cmp r0, #5  // >= 5 implies LPAE support
> +  movlt   r0, #32 // 32 bits if no LPAE
> +  movge   r0, #40 // 40 bits if LPAE
> +  bx  lr
> +
>  ASM_FUNCTION_REMOVE_IF_UNREFERENCED
> diff --git a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm 
> b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm
> index 219140c22b13..3eb52875971d 100644
> --- a/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm
> +++ b/ArmPkg/Library/ArmLib/Arm/ArmLibSupport.asm
> @@ -169,4 +169,12 @@
>isb
>bx  lr
>  
> + RVCT_ASM_EXPORT ArmGetPhysicalAddressBits
> +  mrc p15, 0, r0, c0, c1, 4   ; MMFR0
> +  and r0, r0, #0xf; VMSA [3:0]
> +  cmp r0, #5  ; >= 5 implies LPAE support
> +  movlt   r0, #32 ; 32 bits if no LPAE
> +  movge   r0, #40 ; 40 bits if LPAE
> +  bx  lr
> +
>END
> -- 
> 2.19.1
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH v3 08/16] ArmPkg/ArmMmuLib: take the CPU supported maximum PA space into account

2018-11-29 Thread Leif Lindholm
On Wed, Nov 28, 2018 at 03:33:49PM +0100, Ard Biesheuvel wrote:
> In preparation of dropping PcdPrePiCpuMemorySize entirely, base the
> maximum size of the identity map on the capabilities of the CPU.
> Since that may exceed what is architecturally permitted when using
> 4 KB pages, take MAX_ADDRESS into account as well.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Leif Lindholm 

> ---
>  ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf   |  3 ---
>  ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf|  3 ---
>  ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 11 +--
>  3 files changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf 
> b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> index b9f264de8d26..246963361e45 100644
> --- a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> +++ b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
> @@ -40,8 +40,5 @@ [LibraryClasses]
>CacheMaintenanceLib
>MemoryAllocationLib
>  
> -[Pcd.AARCH64]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
> -
>  [Pcd.ARM]
>gArmTokenSpaceGuid.PcdNormalMemoryNonshareableOverride
> diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf 
> b/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
> index ecf13f790734..f689c193b862 100644
> --- a/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
> +++ b/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf
> @@ -35,6 +35,3 @@ [LibraryClasses]
>ArmLib
>CacheMaintenanceLib
>MemoryAllocationLib
> -
> -[Pcd.AARCH64]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
> diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c 
> b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> index 4b62ecb6a476..5403b8d4070e 100644
> --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c
> @@ -604,8 +604,15 @@ ArmConfigureMmu (
>  return EFI_INVALID_PARAMETER;
>}
>  
> -  // Cover the entire GCD memory space
> -  MaxAddress = (1UL << PcdGet8 (PcdPrePiCpuMemorySize)) - 1;
> +  //
> +  // Limit the virtual address space to what we can actually use: UEFI
> +  // mandates a 1:1 mapping, so no point in making the virtual address
> +  // space larger than the physical address space. We also have to take
> +  // into account the architectural limitations that result from UEFI's
> +  // use of 4 KB pages.
> +  //
> +  MaxAddress = MIN (LShiftU64 (1ULL, ArmGetPhysicalAddressBits ()) - 1,
> +MAX_ADDRESS);
>  
>// Lookup the Table Level to get the information
>LookupAddresstoRootTable (MaxAddress, &T0SZ, &RootTableEntryCount);
> -- 
> 2.19.1
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH v3 10/16] ArmPlatformPkg/PrePi: base GCD memory space size on CPU's PA range

2018-11-29 Thread Leif Lindholm
On Wed, Nov 28, 2018 at 03:33:51PM +0100, Ard Biesheuvel wrote:
> Derive the size of the GCD memory space map directly from the CPU's
> information registers rather than from the PcdPrePiCpuMemorySize PCD,
> which will be removed.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Leif Lindholm 

> ---
>  ArmPlatformPkg/PrePi/PeiMPCore.inf  | 1 -
>  ArmPlatformPkg/PrePi/PeiUniCore.inf | 1 -
>  ArmPlatformPkg/PrePi/PrePi.c| 2 +-
>  3 files changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/ArmPlatformPkg/PrePi/PeiMPCore.inf 
> b/ArmPlatformPkg/PrePi/PeiMPCore.inf
> index 242b03175536..7e2ad6fc483d 100644
> --- a/ArmPlatformPkg/PrePi/PeiMPCore.inf
> +++ b/ArmPlatformPkg/PrePi/PeiMPCore.inf
> @@ -97,7 +97,6 @@ [FixedPcd]
>  
>gArmPlatformTokenSpaceGuid.PcdCoreCount
>  
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
>gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
>  
>gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
> diff --git a/ArmPlatformPkg/PrePi/PeiUniCore.inf 
> b/ArmPlatformPkg/PrePi/PeiUniCore.inf
> index a45cdef4ed91..26328b7e8f67 100644
> --- a/ArmPlatformPkg/PrePi/PeiUniCore.inf
> +++ b/ArmPlatformPkg/PrePi/PeiUniCore.inf
> @@ -90,7 +90,6 @@ [FixedPcd]
>  
>gArmPlatformTokenSpaceGuid.PcdCoreCount
>  
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
>gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
>  
>gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
> diff --git a/ArmPlatformPkg/PrePi/PrePi.c b/ArmPlatformPkg/PrePi/PrePi.c
> index 02cff7ddc204..245bdded1eb3 100644
> --- a/ArmPlatformPkg/PrePi/PrePi.c
> +++ b/ArmPlatformPkg/PrePi/PrePi.c
> @@ -115,7 +115,7 @@ PrePiMain (
>BuildStackHob (StacksBase, StacksSize);
>  
>//TODO: Call CpuPei as a library
> -  BuildCpuHob (PcdGet8 (PcdPrePiCpuMemorySize), PcdGet8 (PcdPrePiCpuIoSize));
> +  BuildCpuHob (ArmGetPhysicalAddressBits (), PcdGet8 (PcdPrePiCpuIoSize));
>  
>if (ArmIsMpCore ()) {
>  // Only MP Core platform need to produce gArmMpCoreInfoPpiGuid
> -- 
> 2.19.1
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH v3 09/16] ArmPkg/CpuPei: base GCD memory space size on CPU's PA range

2018-11-29 Thread Leif Lindholm
On Wed, Nov 28, 2018 at 03:33:50PM +0100, Ard Biesheuvel wrote:
> Derive the size of the GCD memory space map directly from the CPU's
> information registers rather than from the PcdPrePiCpuMemorySize PCD,
> which will be removed.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Leif Lindholm 

> ---
>  ArmPkg/Drivers/CpuPei/CpuPei.inf | 1 -
>  ArmPkg/Drivers/CpuPei/CpuPei.c   | 2 +-
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/ArmPkg/Drivers/CpuPei/CpuPei.inf 
> b/ArmPkg/Drivers/CpuPei/CpuPei.inf
> index eafccd600983..dcea012fd8f9 100644
> --- a/ArmPkg/Drivers/CpuPei/CpuPei.inf
> +++ b/ArmPkg/Drivers/CpuPei/CpuPei.inf
> @@ -50,7 +50,6 @@ [Guids]
>gArmMpCoreInfoGuid
>  
>  [FixedPcd]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
>gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
>  
>  [Depex]
> diff --git a/ArmPkg/Drivers/CpuPei/CpuPei.c b/ArmPkg/Drivers/CpuPei/CpuPei.c
> index d54f42acfcc8..e63519ff6481 100644
> --- a/ArmPkg/Drivers/CpuPei/CpuPei.c
> +++ b/ArmPkg/Drivers/CpuPei/CpuPei.c
> @@ -73,7 +73,7 @@ InitializeCpuPeim (
>ArmEnableBranchPrediction ();
>  
>// Publish the CPU memory and io spaces sizes
> -  BuildCpuHob (PcdGet8 (PcdPrePiCpuMemorySize), PcdGet8 (PcdPrePiCpuIoSize));
> +  BuildCpuHob (ArmGetPhysicalAddressBits (), PcdGet8 (PcdPrePiCpuIoSize));
>  
>// Only MP Core platform need to produce gArmMpCoreInfoPpiGuid
>Status = PeiServicesLocatePpi (&gArmMpCoreInfoPpiGuid, 0, NULL, 
> (VOID**)&ArmMpCoreInfoPpi);
> -- 
> 2.19.1
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH v3 13/16] ArmPlatformPkg/PlatformPei: drop unused PCD references

2018-11-29 Thread Leif Lindholm
On Wed, Nov 28, 2018 at 03:33:54PM +0100, Ard Biesheuvel wrote:
> Drop some PCD references that are not actually referenced from the
> PlatformPei code.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Leif Lindholm 

> ---
>  ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf | 3 ---
>  ArmPlatformPkg/PlatformPei/PlatformPeim.inf   | 3 ---
>  2 files changed, 6 deletions(-)
> 
> diff --git a/ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf 
> b/ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
> index 314789d0a990..23bb3f37e766 100644
> --- a/ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
> +++ b/ArmPlatformPkg/PlatformPei/PlatformPeiLib.inf
> @@ -46,8 +46,5 @@ [FixedPcd]
>gArmTokenSpaceGuid.PcdFvBaseAddress
>gArmTokenSpaceGuid.PcdFvSize
>  
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
> -
>  [depex]
>TRUE
> diff --git a/ArmPlatformPkg/PlatformPei/PlatformPeim.inf 
> b/ArmPlatformPkg/PlatformPei/PlatformPeim.inf
> index 423b9ab858d1..4934baa838e1 100644
> --- a/ArmPlatformPkg/PlatformPei/PlatformPeim.inf
> +++ b/ArmPlatformPkg/PlatformPei/PlatformPeim.inf
> @@ -57,9 +57,6 @@ [FixedPcd]
>gArmTokenSpaceGuid.PcdFvBaseAddress
>gArmTokenSpaceGuid.PcdFvSize
>  
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
> -
>  [Depex]
>TRUE
>  
> -- 
> 2.19.1
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH v3 12/16] BeagleBoardPkg/PrePi: base GCD memory space size on CPU's PA range

2018-11-29 Thread Leif Lindholm
On Wed, Nov 28, 2018 at 03:33:53PM +0100, Ard Biesheuvel wrote:
> Derive the size of the GCD memory space map directly from the CPU's
> information registers rather than from the PcdPrePiCpuMemorySize PCD,
> which will be removed.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Leif Lindholm 

> ---
>  BeagleBoardPkg/PrePi/PeiUniCore.inf | 1 -
>  BeagleBoardPkg/PrePi/PrePi.c| 2 +-
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/BeagleBoardPkg/PrePi/PeiUniCore.inf 
> b/BeagleBoardPkg/PrePi/PeiUniCore.inf
> index 3d72bc5b46e1..53c71d8eafc2 100644
> --- a/BeagleBoardPkg/PrePi/PeiUniCore.inf
> +++ b/BeagleBoardPkg/PrePi/PeiUniCore.inf
> @@ -86,7 +86,6 @@ [FixedPcd]
>  
>gArmPlatformTokenSpaceGuid.PcdCoreCount
>  
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
>gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
>  
>gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
> diff --git a/BeagleBoardPkg/PrePi/PrePi.c b/BeagleBoardPkg/PrePi/PrePi.c
> index 46f63f40c46e..bc9b0c80b84c 100644
> --- a/BeagleBoardPkg/PrePi/PrePi.c
> +++ b/BeagleBoardPkg/PrePi/PrePi.c
> @@ -110,7 +110,7 @@ PrePiMain (
>BuildStackHob (StacksBase, StacksSize);
>  
>//TODO: Call CpuPei as a library
> -  BuildCpuHob (PcdGet8 (PcdPrePiCpuMemorySize), PcdGet8 (PcdPrePiCpuIoSize));
> +  BuildCpuHob (ArmGetPhysicalAddressBits (), PcdGet8 (PcdPrePiCpuIoSize));
>  
>// Store timer value logged at the beginning of firmware image execution
>Performance.ResetEnd = GetTimeInNanoSecond (StartTimeStamp);
> -- 
> 2.19.1
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH v3 14/16] EmbeddedPkg/PrePiLib: drop unused PCD reference

2018-11-29 Thread Leif Lindholm
On Wed, Nov 28, 2018 at 03:33:55PM +0100, Ard Biesheuvel wrote:
> Drop the reference to gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
> which is never used.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Leif Lindholm 

> ---
>  EmbeddedPkg/Library/PrePiLib/PrePiLib.inf | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/EmbeddedPkg/Library/PrePiLib/PrePiLib.inf 
> b/EmbeddedPkg/Library/PrePiLib/PrePiLib.inf
> index de68405098c0..3dba884b1f31 100644
> --- a/EmbeddedPkg/Library/PrePiLib/PrePiLib.inf
> +++ b/EmbeddedPkg/Library/PrePiLib/PrePiLib.inf
> @@ -69,7 +69,6 @@ [Protocols]
>  
>  
>  [FixedPcd.common]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
>gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize
>  
>gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory
> -- 
> 2.19.1
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH v3 16/16] EmbeddedPkg/EmbeddedPkg.dec: drop PcdPrePiCpuMemorySize declarations

2018-11-29 Thread Leif Lindholm
On Wed, Nov 28, 2018 at 03:33:57PM +0100, Ard Biesheuvel wrote:
> PcdPrePiCpuMemorySize is no longer used so drop the declarations from
> the package DEC file.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 

Reviewed-by: Leif Lindholm 

> ---
>  EmbeddedPkg/EmbeddedPkg.dec | 4 
>  1 file changed, 4 deletions(-)
> 
> diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
> index 28a143865d0e..ff5aab07d745 100644
> --- a/EmbeddedPkg/EmbeddedPkg.dec
> +++ b/EmbeddedPkg/EmbeddedPkg.dec
> @@ -170,22 +170,18 @@ [PcdsFixedAtBuild.common]
>gEmbeddedTokenSpaceGuid.PcdAndroidBootDevicePath|L""|VOID*|0x0057
>  
>  [PcdsFixedAtBuild.ARM]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|32|UINT8|0x0010
>gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|0|UINT8|0x0011
>  
># ISP1761 USB OTG Controller
>gEmbeddedTokenSpaceGuid.PcdIsp1761BaseAddress|0|UINT32|0x0021
>  
>  [PcdsFixedAtBuild.AARCH64]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|48|UINT8|0x0010
>gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|0|UINT8|0x0011
>  
>  [PcdsFixedAtBuild.IA32]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|36|UINT8|0x0010
>gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16|UINT8|0x0011
>  
>  [PcdsFixedAtBuild.X64]
> -  gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|52|UINT8|0x0010
>gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16|UINT8|0x0011
>  
>  [PcdsFixedAtBuild.common, PcdsDynamic.common]
> -- 
> 2.19.1
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [RFC PATCH v3 00/11] Extend secure variable service to be usable from Standalone MM

2018-11-29 Thread Gao, Liming
My comment is below. 

1. Please don't update MemoryFence() implementation. It will impact all 
consumer code. AsmLfence() is X86 specific API. You can implement the internal 
function in the arch specific source file to call AsmLfence() for X86 and call 
MemoryFence() for ARM. This internal function will be called in the common 
logic. 
2. On StandaloneMmServicesTableLib.h, I suggest to add it into MdePkg, and add 
StandaloneMmRuntimeDxe library into MdePkg. This library sets gMmst is NULL, 
and always return FALSE in InMm(). 
3. On PcdStandaloneMmEnable, I also suggest to add it into MdePkg. It can be 
used to control the driver logic in the different packages. 

With 2 & 3, other edk2 packages don't need to depend on 
StandaloneMmPkg/StandaloneMmPkg.dec

> -Original Message-
> From: Jagadeesh Ujja [mailto:jagadeesh.u...@arm.com]
> Sent: Wednesday, November 28, 2018 5:35 PM
> To: edk2-devel@lists.01.org; Gao, Liming ; Zhang, Chao 
> B ; leif.lindh...@linaro.org;
> ard.biesheu...@linaro.org
> Subject: [RFC PATCH v3 00/11] Extend secure variable service to be usable 
> from Standalone MM
> 
> Changes since v2:
> - Added 'Contributed-under' tag, removed Change-ID tag and
>   maintained a single signed-off-by for the all the patches.
> 
> Changes since v1:
> - Addressed all the comments from Liming Gao
>   - Removed the use of #ifdef/#else/#endif and used a Pcd instead to
> select between MM and non-MM paths.
>   - Removed all dependencies on edk2-platforms.
>   - Dropped the use of mMmst and used gSmst instead.
>   - Added a dummy implementation UefiRuntimeServiceTableLib for
> MM_STANDALONE usage
> - Replaced all uses of AsmLfence with MemoryFence from variable
>   service code.
> - Add a new StandaloneMmRuntimeDxe library to for use by non-MM code.
> 
> This RFC patch series extends the existing secure variable service support for
> use with Standalone MM. This is applicable to paltforms that use Standalone
> Management Mode to protect access to non-volatile memory (NOR flash in case
> of these patches) used to store the secure EFI variables.
> 
> The first patch pulls in additional libraries from the staging branch of
> StandaloneMmPkg into the edk2's StandaloneMmPkg. The existing secure variable
> service implementation supports only the traditional MM mode and so the rest
> of the patches extends the existing secure variable service support to be
> useable with Standalone MM mode as well.
> 
> This patch series is being posted as an RFC to get feedback on the approach 
> taken
> in these patches.
> 
> Jagadeesh Ujja (11):
>   MdeModulePkg/Variable: replace all uses of AsmLfence with MemoryFence
>   StandaloneMmPkg: Pull in additonal libraries from staging branch
>   MdeModulePkg/Library: Add StandaloneMmRuntimeDxe library
>   ArmPlatformPkg/NorFlashDxe: allow reusability as a MM driver
>   MdeModulePkg/FaultTolerantWriteDxe: allow reusability as a MM driver
>   MdeModulePkg/Variable/RuntimeDxe: adapt for usability with MM
> Standalone
>   MdeModulePkg/Variable/RuntimeDxe: adapt as a MM Standalone driver
>   SecurityPkg/AuthVariableLib: allow MM_STANDALONE drivers to use this
> library
>   MdeModulePkg/VarCheckLib: allow MM_STANDALONE drivers to use this
> library
>   CryptoPkg/BaseCryptLib: allow MM_STANDALONE drivers to use this
> library
>   CryptoPkg/BaseCryptLib: Hack to get time in MM Standalone mode
> 
>  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
> |   3 +
>  ArmPlatformPkg/Drivers/NorFlashDxe/{NorFlashDxe.inf => 
> NorFlashStandaloneMm.inf}
> |  28 +-
>  CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> |   8 +-
>  CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> |   5 +
>  MdeModulePkg/Library/{VarCheckLib/VarCheckLib.inf => 
> StandaloneMmRuntimeDxe/StandaloneMmRuntimeDxe.inf}
> |  22 +-
>  MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
> |   5 +-
>  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> |   2 +
>  MdeModulePkg/Universal/FaultTolerantWriteDxe/{FaultTolerantWriteDxe.inf => 
> FaultTolerantWriteStandaloneMm.inf}   |
> 53 +-
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> |   2 +
>  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
> |   4 +
>  MdeModulePkg/Universal/Variable/RuntimeDxe/{VariableRuntimeDxe.inf => 
> VariableStandaloneMm.inf}
> | 107 ++-
>  SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> |   5 +-
>  StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf
> |   2 +-
>  StandaloneMmPkg/Library/{StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf =>
> StandaloneMmHobLib/StandaloneMmHobLib.inf} |  11 +-
>  
> StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf
> |  45 ++
>  
> StandaloneMmPkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf
> |  36 +
>  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h
> |   5 +-
>  MdeModulePkg/Include/Library/StandaloneMmRuntimeDxe.h
>

[edk2] [RFC] Proposal to add edk2-apps repository

2018-11-29 Thread Kinney, Michael D
Hello,

I would like to propose the creation of a new
repository called edk2-apps.  This repository
would initially be used to host the following
packages from the edk2 repository:

* AppPkg
* StdLib
* StdLibPrivateInternalFiles

These 3 packages provide support for the libc along
with applications that depend on libc.  None of the
other packages in the edk2 repository use these
packages, so these 3 package can be safely moved
without any impacts to platform firmware builds.
Build configurations that do use libc features can
clone the edk2-apps repository and add it to
PACKAGES_PATH.

The history of these 3 packages would be preserved
when importing the content into edk2-apps.  After
The import is verified, these 3 packages would be
deleted from the edk2 repository.

This proposal helps reduce the size of the edk2
repository and focuses edk2 repository on packages
used to provide UEFI/PI conformant firmware.

If there are no concerns with this proposal, I will
enter a Tianocore BZs for the two steps.

Best regards,

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


Re: [edk2] [PATCH v3 00/16] [Arm|ArmVirt|MdePkg|Embedded]Pkg: lift 40-bit IPA space limit

2018-11-29 Thread Ard Biesheuvel
On Wed, 28 Nov 2018 at 15:34, Ard Biesheuvel  wrote:
>
> This v3 subsumes and/or supersedes
>
> [PATCH v2 00/13] ArmPkg, ArmVirtPkg: lift 40-bit IPA space limit
> [PATCH] MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits
> [PATCH v2 0/2] ArmVirtPkg: remove high peripheral space mapping
>
> The ArmVirtQemu targets currently limit the size of the IPA space to
> 40 bits because that is all what KVM supports. However, this is about
> to change, and so we need to update the code if we want to ensure that
> our UEFI firmware builds can keep running on systems that set values
> other than 40 (which could be > 40 or < 40)
>
> This series refactors how we handle the maximum size of the physical
> address space supported by the CPU in relation with the size of UEFI's
> 1:1 mapping and the size of the GCD memory space map, taking the following
> observations into account:
> - the range of the linear mapping can be tied to whatever the CPU supports
>   (as long as it doesn't exceed what the architecture permits for 4k pages)
>   since we mostly already use the maximum of 4 levels anyway, and there is
>   no memory cost involved beyond that
> - there is usually no point in mapping the entire address space, which does
>   involve a memory cost
> - the GCD memory space may be required to cover more than what UEFI can
>   address itself, since it is the based for the UEFI memory map that is
>   provided to the OS
>
> Patches #1 and #2 remove some unused code to avoid having to fix it.
>
> Patches #3 and #4 update ArmVirtQemu and ArmVirtQemuKernel to drop the high
> peripheral space mapping, and map whatever may reside there explicitly
> (currently only the ECAM space in practice, but the MMIO view of the PCI
> I/O space is mapped explicitly as well)
>
> Patch #5 was sent out before individually, and sets MAX_ADDRESS to the
> maximum value AArch64 can map in UEFI which runs with 4k pages.
>
> Patch #6 adds a helper to ArmLib to read the number of supported address
> bits and take this into account in the page table code (#8), which allows
> PcdPrePiCpuMemorySize to assume a value that exceeds the capabilities of
> the CPU.
>
> Patch #7 is mostly a cleanup patch, to switch to the new helper added in
> patch #6. No functional changes intended.
>
> Patches #9 to #12 modify building of the CPU hob (and thus the size of the
> GCD memory space) based on the CPU capabilities rather than the value of
> PcdPrePiCpuMemorySize, which is dropped in the last patch.
>
> Pacthes #13 and #14 remove some needless references to PcdPrePiCpuMemorySize
>
> Patch #15 drops the overrides of PcdPrePiCpuMemorySize from all ArmVirtPkg
> platforms.
>
> Cc: Laszlo Ersek 
> Cc: Leif Lindholm 
> Cc: Eric Auger 
> Cc: Andrew Jones 
> Cc: Philippe Mathieu-Daude 
> Cc: Julien Grall 
>
> Ard Biesheuvel (16):
>   EmbeddedPkg/TemplateSec: remove unused module
>   EmbeddedPkg/PrePiHobLib: drop CreateHobList() from library
>   ArmVirtPkg/FdtPciHostBridgeLib: map ECAM and I/O spaces in GCD memory
> map
>   ArmVirtPkg/QemuVirtMemInfoLib: remove 1:1 mapping of top of PA range
>   MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits
>   ArmPkg/ArmLib: add support for reading the max physical address space
> size
>   ArmVirtPkg/XenVirtMemInfoLib: refactor reading of the PA space size
>   ArmPkg/ArmMmuLib: take the CPU supported maximum PA space into account
>   ArmPkg/CpuPei: base GCD memory space size on CPU's PA range
>   ArmPlatformPkg/PrePi: base GCD memory space size on CPU's PA range
>   ArmVirtPkg/PrePi: base GCD memory space size on CPU's PA range
>   BeagleBoardPkg/PrePi: base GCD memory space size on CPU's PA range
>   ArmPlatformPkg/PlatformPei: drop unused PCD references
>   EmbeddedPkg/PrePiLib: drop unused PCD reference
>   ArmVirtPkg: drop PcdPrePiCpuMemorySize assignments from all platforms
>   EmbeddedPkg/EmbeddedPkg.dec: drop PcdPrePiCpuMemorySize declarations
>

Thanks all for the reviews.

Patches #1 .. #15 pushed as e979ea74aa14..55342094fb86

Patch #16 needs to wait until edk2-platforms is brought up to date
with the removal of PcdPrePiCpuMemorySize
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] Help on boot manager 'Boot Manager Menu' and direct boot

2018-11-29 Thread Laszlo Ersek
On 11/29/18 14:12, Udit Kumar wrote:
> Thanks Laszlo,
> 
>  
>> I can only think of some terminal control sequences that are *not* printed to
>> the terminal when you don't enter UiApp manually. I don't understand how that
>> could cause the exact symptom you describe, but I have no better explanation.
>>
>> Can you try other serial communication programs on your desktop? Such as
>> "minicom" or "screen"?
> 
> Screen didn't help.
> Moreover , using different OS distributions show same similar behavior !!
>  
>> Also, can you try changing your "console=..." kernel param(s)?
> 
> You meant baud-rate ? 

Yes, and more. The options that the "console=" kernel parameter takes.

> 
> On uefi side,  could you help me if there is some extra information passed to 
> OS in path 
> UiApp -> BootDevice, 

I don't think so. Nothing comes to my mind anyway.

> I could see , some of additional protocols are installed in above path, I am 
> not sure if those are
> used  by  OS or OS Loader (grub in my case)  somehow.

Well, UiApp generally connects all drivers to all devices -- normally a
platform BDS would not want to do this, for the sake of booting quickly
--, which likely results in more protocol instances being installed in
the system. That shouldn't cause a difference for how serial behaves
once the OS has booted.

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


Re: [edk2] [PATCH] MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits

2018-11-29 Thread Laszlo Ersek
On 11/29/18 15:59, Gao, Liming wrote:
> Laszlo:
>   I add my comments. 
> 
> Thanks
> Liming
>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Tuesday, November 27, 2018 10:52 PM
>> To: Ard Biesheuvel ; edk2-devel@lists.01.org
>> Cc: Gao, Liming ; Kinney, Michael D 
>> ; leif.lindh...@linaro.org;
>> phi...@redhat.com
>> Subject: Re: [PATCH] MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 
>> bits
>>
>> On 11/27/18 13:27, Ard Biesheuvel wrote:
>>> AArch64 support the use of more than 48 bits for physical and/or
>>> virtual addressing, but only if the page size is set to 64 KB,
>>> which is not supported by UEFI/EDK2. So redefine MAX_ADDRESS to
>>> cover only 48 address bits.
>>>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Ard Biesheuvel 
>>> ---
>>>  MdePkg/Include/AArch64/ProcessorBind.h | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/MdePkg/Include/AArch64/ProcessorBind.h 
>>> b/MdePkg/Include/AArch64/ProcessorBind.h
>>> index 968c18f915ae..dad75df1c579 100644
>>> --- a/MdePkg/Include/AArch64/ProcessorBind.h
>>> +++ b/MdePkg/Include/AArch64/ProcessorBind.h
>>> @@ -138,9 +138,9 @@ typedef INT64   INTN;
>>>  #define MAX_2_BITS  0xC000ULL
>>>
>>>  ///
>>> -/// Maximum legal AARCH64  address
>>> +/// Maximum legal AARCH64  address (48 bits for 4 KB page size)
>>>  ///
>>> -#define MAX_ADDRESS   0xULL
>>> +#define MAX_ADDRESS   0xULL
>>>
>>>  ///
>>>  /// Maximum legal AArch64 INTN and UINTN values.
>>>
>>
>> H. I'm worried about this change. I think it could open a can of
>> worms. I have no clue what *all* the things are that we use MAX_ADDRESS
>> for. Does it give the maximum value of the canonical address *format*?
>> Or is it the maximum address that the processor could ever access?
>>
>> Let's look at the X64 situation... For X64, MAX_ADDRESS is
>> 0x____ULL (MdePkg/Include/X64/ProcessorBind.h). However,
>> on X64, even considering the recently introduced 5-level paging
>> , the "useful"
>> number of address bits is up to just 57 -- it used to be 48, with
>> 4-level paging. That is: not 64. Yet we have MAX_UINT64 for MAX_ADDRESS!
>>
>> Which in turn means that, with X64 5-level paging in mind, the issue
>> affects X64 as well -- there could be RAM in the system that the 64-bit
>> DXE phase couldn't access (because edk2 doesn't support 5-level paging,
>> AIUI), but the OS could.
>>
>> That officially turns the question into a multi-architectural one: how
>> should the UEFI memmap describe the highest RAM range, such that it be
>> exposed to the OS, but not exposed to the firmware itself? (Because, the
>> firmware doesn't support the necessary paging mode, or processor mode.)
>>
>> Liming, can you share what Intel plans, in edk2, for supporting 5-level
>> paging?
> So far, I have no more to be shared. I don't know whether it is necessary to 
> support 5-level paging with the max memory. 
> The firmware can report [2^48 .. 2^57) RAM with the allocated status. So, 
> those region memory can't be allocated in firmware. 
> They will be visible to OS. If OS enables 5-level paging, it can access them. 

Great, thank you!
Laszlo

>>
>> And, on such physical X64 systems today that support 57-bit paging, how
>> does the UEFI memmap describe the [2^48 .. 2^57) RAM?
>>
>> And how does the firmware allocate and use memory from that area?
>>
>> Thanks,
>> Laszlo

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


[edk2] [PATCH v5 2/2] MdeModulePkg/SdMmcPciHcDxe: Add SDMMC HC v4 and above Support.

2018-11-29 Thread Ashish Singhal
Add SDMA, ADMA2 and 26b data length support.

If V4 64 bit address mode is enabled in compatibility register,
program controller to enable V4 host mode and use appropriate
SDMA registers supporting 64 bit addresses.

If V4 64 bit address mode is enabled in compatibility register,
program controller to enable V4 host mode and use appropriate
ADMA descriptors supporting 64 bit addresses.

If host controller version is above V4.0, enable ADMA2 with 26b data
length support for better performance. HC 2 register is configured to
use 26 bit data lengths and ADMA2 descriptors are configured appropriately.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1359

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal 
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h |   4 +-
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   | 328 ++---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h   |  48 ++-
 3 files changed, 322 insertions(+), 58 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
index 8c1a589..3af7c95 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h
@@ -2,6 +2,7 @@
 
   Provides some data structure definitions used by the SD/MMC host controller 
driver.
 
+Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
 Copyright (c) 2015, 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
@@ -150,7 +151,8 @@ typedef struct {
   BOOLEAN Started;
   UINT64  Timeout;
 
-  SD_MMC_HC_ADMA_DESC_LINE*AdmaDesc;
+  SD_MMC_HC_ADMA_32_DESC_LINE *Adma32Desc;
+  SD_MMC_HC_ADMA_64_DESC_LINE *Adma64Desc;
   EFI_PHYSICAL_ADDRESSAdmaDescPhy;
   VOID*AdmaMap;
   UINT32  AdmaPages;
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index 598b6a3..debc3be 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -4,6 +4,7 @@
 
   It would expose EFI_SD_MMC_PASS_THRU_PROTOCOL for upper layer use.
 
+  Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved.
   Copyright (c) 2015 - 2017, 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
@@ -418,6 +419,36 @@ SdMmcHcWaitMmioSet (
 }
 
 /**
+  Get the controller version information from the specified slot.
+
+  @param[in]  PciIo   The PCI IO protocol instance.
+  @param[in]  SlotThe slot number of the SD card to send the 
command to.
+  @param[out] Version The buffer to store the version information.
+
+  @retval EFI_SUCCESS The operation executes successfully.
+  @retval Others  The operation fails.
+
+**/
+EFI_STATUS
+SdMmcHcGetControllerVersion (
+  IN EFI_PCI_IO_PROTOCOL  *PciIo,
+  IN UINT8Slot,
+ OUT UINT16   *Version
+  )
+{
+  EFI_STATUSStatus;
+
+  Status = SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_CTRL_VER, TRUE, sizeof 
(UINT16), Version);
+  if (EFI_ERROR (Status)) {
+return Status;
+  }
+
+  *Version &= 0xFF;
+
+  return EFI_SUCCESS;
+}
+
+/**
   Software reset the specified SD/MMC host controller and enable all 
interrupts.
 
   @param[in] PrivateA pointer to the SD_MMC_HC_PRIVATE_DATA instance.
@@ -776,18 +807,19 @@ SdMmcHcClockSupply (
 
   DEBUG ((DEBUG_INFO, "BaseClkFreq %dMHz Divisor %d ClockFreq %dKhz\n", 
BaseClkFreq, Divisor, ClockFreq));
 
-  Status = SdMmcHcRwMmio (PciIo, Slot, SD_MMC_HC_CTRL_VER, TRUE, sizeof 
(ControllerVer), &ControllerVer);
+  Status = SdMmcHcGetControllerVersion (PciIo, Slot, &ControllerVer);
   if (EFI_ERROR (Status)) {
 return Status;
   }
   //
   // Set SDCLK Frequency Select and Internal Clock Enable fields in Clock 
Control register.
   //
-  if (((ControllerVer & 0xFF) >= SD_MMC_HC_CTRL_VER_300) &&
-  ((ControllerVer & 0xFF) <= SD_MMC_HC_CTRL_VER_420)) {
+  if ((ControllerVer >= SD_MMC_HC_CTRL_VER_300) &&
+  (ControllerVer <= SD_MMC_HC_CTRL_VER_420)) {
 ASSERT (Divisor <= 0x3FF);
 ClockCtrl = ((Divisor & 0xFF) << 8) | ((Divisor & 0x300) >> 2);
-  } else if (((ControllerVer & 0xFF) == 0) || ((ControllerVer & 0xFF) == 1)) {
+  } else if ((ControllerVer == SD_MMC_HC_CTRL_VER_100) ||
+ (ControllerVer == SD_MMC_HC_CTRL_VER_200)) {
 //
 // Only the most significant bit can be used as divisor.
 //
@@ -935,6 +967,60 @@ SdMmcHcSetBusWidth (
 }
 
 /**
+  Configure V4 controller enhancements at initialization.
+
+  @param[in] PciIo  The PCI IO protocol inst

[edk2] [PATCH v5 1/2] MdeModulePkg/SdMmcPciHcDxe: Declare V4 64 bit address capability

2018-11-29 Thread Ashish Singhal
Add capability declaration for V4.x 64 bit system address support.
This would be used for host controllers working in version 4. Enable
64 bit DMA support in PCI layer if V3 or V4 64 bit support is
enabled in host capability register.

The usage of this new field does not need a guard for version check as
spec for previous SDMMC versions defines this field as reserved with
default value of 0.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1359

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal 
---
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c |  9 -
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   |  3 ++-
 MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h   | 10 +-
 3 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
index a87f8de..2bfd758 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c
@@ -649,7 +649,14 @@ SdMmcPciHcDriverBindingStart (
   Private->BaseClkFreq[Slot]
   ));
 
-Support64BitDma &= Private->Capability[Slot].SysBus64;
+//
+// If any of the slots does not support 64b system bus
+// do not enable 64b DMA in the PCI layer.
+//
+if (Private->Capability[Slot].SysBus64V3 == 0 &&^M
+Private->Capability[Slot].SysBus64V4 == 0) {^M
+  Support64BitDma &= FALSE;^M
+}
 
 Status = SdMmcHcGetMaxCurrent (PciIo, Slot, &Private->MaxCurrent[Slot]);
 if (EFI_ERROR (Status)) {
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
index ddf6dcf..598b6a3 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c
@@ -45,7 +45,8 @@ DumpCapabilityReg (
   DEBUG ((DEBUG_INFO, "   Voltage 3.3   %a\n", Capability->Voltage33 ? 
"TRUE" : "FALSE"));
   DEBUG ((DEBUG_INFO, "   Voltage 3.0   %a\n", Capability->Voltage30 ? 
"TRUE" : "FALSE"));
   DEBUG ((DEBUG_INFO, "   Voltage 1.8   %a\n", Capability->Voltage18 ? 
"TRUE" : "FALSE"));
-  DEBUG ((DEBUG_INFO, "   64-bit Sys Bus%a\n", Capability->SysBus64 ? 
"TRUE" : "FALSE"));
+  DEBUG ((DEBUG_INFO, "   V4 64-bit Sys Bus %a\n", Capability->SysBus64V4 ? 
"TRUE" : "FALSE"));
+  DEBUG ((DEBUG_INFO, "   V3 64-bit Sys Bus %a\n", Capability->SysBus64V3 ? 
"TRUE" : "FALSE"));
   DEBUG ((DEBUG_INFO, "   Async Interrupt   %a\n", Capability->AsyncInt ? 
"TRUE" : "FALSE"));
   DEBUG ((DEBUG_INFO, "   SlotType  "));
   if (Capability->SlotType == 0x00) {
diff --git a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h 
b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
index dd45cbd..ad9ce64 100644
--- a/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
+++ b/MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h
@@ -129,24 +129,24 @@ typedef struct {
   UINT32   Voltage33:1;   // bit 24
   UINT32   Voltage30:1;   // bit 25
   UINT32   Voltage18:1;   // bit 26
-  UINT32   Reserved3:1;   // bit 27
-  UINT32   SysBus64:1;// bit 28
+  UINT32   SysBus64V4:1;  // bit 27
+  UINT32   SysBus64V3:1;  // bit 28
   UINT32   AsyncInt:1;// bit 29
   UINT32   SlotType:2;// bit 30:31
   UINT32   Sdr50:1;   // bit 32
   UINT32   Sdr104:1;  // bit 33
   UINT32   Ddr50:1;   // bit 34
-  UINT32   Reserved4:1;   // bit 35
+  UINT32   Reserved3:1;   // bit 35
   UINT32   DriverTypeA:1; // bit 36
   UINT32   DriverTypeC:1; // bit 37
   UINT32   DriverTypeD:1; // bit 38
   UINT32   DriverType4:1; // bit 39
   UINT32   TimerCount:4;  // bit 40:43
-  UINT32   Reserved5:1;   // bit 44
+  UINT32   Reserved4:1;   // bit 44
   UINT32   TuningSDR50:1; // bit 45
   UINT32   RetuningMod:2; // bit 46:47
   UINT32   ClkMultiplier:8;   // bit 48:55
-  UINT32   Reserved6:7;   // bit 56:62
+  UINT32   Reserved5:7;   // bit 56:62
   UINT32   Hs400:1;   // bit 63
 } SD_MMC_HC_SLOT_CAP;
 
-- 
2.7.4

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


[edk2] [RFC] Change EDK II to an Apache 2.0 License

2018-11-29 Thread Kinney, Michael D
Hello,

This RFC follows up on the proposal from Mark Doran to change the 
EDK II Project to an Apache 2.0 License.

https://lists.01.org/pipermail/edk2-devel/2018-October/030385.html


  ** Please provide feedback on the proposal by Friday 12/7/18. **

I will be following up with pointers to public GitHub branches that
contain the initial set of changes in steps (1) and (2) below for 
review. 

The proposal is to perform this change to edk2/master in the steps listed
below. The license change will not be applied to any of the other existing
branches in the edk2 repository.

1) Change all files with a BSD 2-Clause license and only a single 
   copyright statement by Intel Corporation to an Apache 2.0 license
   and add an SPDX-License-Identifier statement.

   ==
   SPDX-License-Identifier: Apache-2.0

   Licensed under the Apache License, Version 2.0 (the "License");
   you may not use this file except in compliance with the License.
   You may obtain a copy of the License at

   http://www.apache.org/licenses/LICENSE-2.0

   Unless required by applicable law or agreed to in writing, software
   distributed under the License is distributed on an "AS IS" BASIS,
   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
   See the License for the specific language governing permissions and
   limitations under the License.
   ==

2) Update Readme.md and License.txt in the root of the edk2 repository to
   state that content is covered by a mix of BSD 2-Clause and Apache 2.0
   licenses. 

3) Update all documentation to state that content submitted under the 
   Apache 2.0 license no longer requires the Tianocore Contribution
   Agreement which means the following line is not required in commit
   messages for changes to files that are covered by an Apache 2.0 License.

   Contributed-under: TianoCore Contribution Agreement 1.1

4) Create Wiki page(s) that provide the details of the Apache 2.0 License
   change and provide the status of the license change for each package
   in the edk2 repository.  Also provide a list of the additional copyright
   holders that need to be contacted to accept the change to an Apache 2.0
   License along with the status of that acceptance.

5) After all copyright holders have accepted the change to an Apache 2.0
   License, change the remaining files from BSD 2-Clause to Apache 2.0.

6) Update Readme.md and License.txt in the edk2 repository to state that
   Apache 2.0 is the preferred license for the EDK II project.

Best regards,

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


Re: [edk2] [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 64bit SDMA and ADMA2 support.

2018-11-29 Thread Ashish Singhal
Hello Hao,

Answers to your comments:

1. Patch 5 fixes the issue with accessing SD as well as eMMC files.
2. Using Private variable for controller version would mean I can only access 
it in functions having an instance of the installed protocol which is not 
possible in some functions for example where we set clock dividers. We either 
need to use what I have or pass Private or controller version as an argument to 
functions using it.
3. I think there is a bigger issue here. During host controller initialization 
we need to setup 64b addressing which happens before we initialize 64b DMA in 
PCI layer. So what you are suggesting may not be that helpful. Also, what we 
are doing right now is that we go through all slots and if controller on any 
slot does not support 64b, we do not enable 64b DMA in PCI layer. What is the 
likelihood of all controllers on an SOC not supporting similar address width? 
Also, shouldn't we be enabling 64b DMA in PCI layer if any of the controller 
supports 64b addressing? Wouldn't 64b DMA enabled mean 32b is enabled by 
default?
4. Patch 5 has been rebased on tip of edk2.

Thanks
Ashish

-Original Message-
From: Wu, Hao A  
Sent: Wednesday, November 28, 2018 12:25 AM
To: Ashish Singhal ; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 64bit 
SDMA and ADMA2 support.

Hello,

Sorry for the delayed response.

Apart from inserting the Bugzilla tracker information, several more general 
level
comments:

1. Cannot access the files (content) on SD & eMMC devices

After applying (rebasing onto the latest codes) your patches, I found that 
though the SD & eMMC devices can be detected, yet the content cannot be 
accessed.

There are 1 SD card and 1 embedded eMMC device within the system. Under Shell, 
I can see 2 file systems on SD & eMMC devices being detected, but when I try to 
access the files on those file systems by using the 'ls' command, it fails, 
saying "ls: File Not Found - 'FS0:\'". I can confirm that files can be listed 
successfully without applying this patch.

I also tried the 'dblk' command for display the data via BlockIO protocols 
created on those devices. I found that for SD, the command works fine. But for 
eMMC, the command fails with message "dblk: Read file error - 'BlockIo'".

For the hardware I own, the controllers are all version 3.0. I tested on both
IA32 and X64 arch image. The results were the same as described above.

Unfortunately, I do not have access to boards with SD controller with version 
newer than 3.0. So I am not able to perform those tests on my side for Version
4.0 or greater SD controllers.

Do you have any hardware with SD controller of version 3.0? Is it working on 
your side?

Please let me know if additional information is needed.


2. On SdMmcHcGetControllerVersion()

I found that there is a 'ControllerVersion' version in the data structure 
SD_MMC_HC_PRIVATE_DATA. But currently, this field seems never used.

I think we can get the controller version information only once and use this 
field to store it. Hence, this new function can be eliminated and also can 
avoid calling it multiple times.


3. Setting the SD_MMC_HC_64_ADDR_EN bit in SdMmcHcV4Init64BitSupport()

The setting of the SD_MMC_HC_64_ADDR_EN bit is decided by the 'SysBus64V4'
field in the CAP register. For me, additional dependency on the 64-bit DMA 
support in the PCI layer is needed as well.

In function SdMmcPciHcDriverBindingStart():

  //
  // Enable 64-bit DMA support in the PCI layer if this controller
  // supports it.
  //
  if (Support64BitDma) {
Status = PciIo->Attributes (
  PciIo,
  EfiPciIoAttributeOperationEnable,
  EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE,
  NULL
  );
if (EFI_ERROR (Status)) {
  DEBUG ((DEBUG_WARN, "SdMmcPciHcDriverBindingStart: failed to enable 
64-bit DMA (%r)\n", Status));
}
  }

If the above PCI attribute setting fails, the PCI layer will not support the 
64-bit DMA.

Hence, I am thinking to introduce a new BOOLEAN field in the 
"SD_MMC_HC_PRIVATE_DATA" data structure (maybe putting the 'Support64BitDma'
into the data structure). If the above PCI operation succeeds, the BOOLEAN 
field will be set to TRUE, otherwise, FALSE.

And the setting of the SD_MMC_HC_64_ADDR_EN bit should depend on the BOOLEAN 
field as well.


4. Please help to rebase the patch upon the latest master branch.

Some changes within the SdMmcPciHcDxe have been pushed recently. Could you help 
to rebase this patch upon the latest changes. Thanks in advance.


Also, some inline comments below.


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Ashish Singhal
> Sent: Tuesday, November 20, 2018 4:59 AM
> To: edk2-devel@lists.01.org
> Cc: Ashish Singhal
> Subject: [edk2] [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 
> 64bit SDMA and ADMA2 support.

Re: [edk2] [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 64bit SDMA and ADMA2 support.

2018-11-29 Thread Ashish Singhal
Missed one of your suggested compiler related change in Patch 5. Have posted 
patch 6.

Thanks
Ashish Singhal

-Original Message-
From: Ashish Singhal 
Sent: Thursday, November 29, 2018 11:48 AM
To: 'Wu, Hao A' ; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 64bit 
SDMA and ADMA2 support.

Hello Hao,

Answers to your comments:

1. Patch 5 fixes the issue with accessing SD as well as eMMC files.
2. Using Private variable for controller version would mean I can only access 
it in functions having an instance of the installed protocol which is not 
possible in some functions for example where we set clock dividers. We either 
need to use what I have or pass Private or controller version as an argument to 
functions using it.
3. I think there is a bigger issue here. During host controller initialization 
we need to setup 64b addressing which happens before we initialize 64b DMA in 
PCI layer. So what you are suggesting may not be that helpful. Also, what we 
are doing right now is that we go through all slots and if controller on any 
slot does not support 64b, we do not enable 64b DMA in PCI layer. What is the 
likelihood of all controllers on an SOC not supporting similar address width? 
Also, shouldn't we be enabling 64b DMA in PCI layer if any of the controller 
supports 64b addressing? Wouldn't 64b DMA enabled mean 32b is enabled by 
default?
4. Patch 5 has been rebased on tip of edk2.

Thanks
Ashish

-Original Message-
From: Wu, Hao A 
Sent: Wednesday, November 28, 2018 12:25 AM
To: Ashish Singhal ; edk2-devel@lists.01.org
Subject: RE: [edk2] [PATCH v3 2/2] MdeModulePkg/SdMmcPciHcDxe: Add V4 64bit 
SDMA and ADMA2 support.

Hello,

Sorry for the delayed response.

Apart from inserting the Bugzilla tracker information, several more general 
level
comments:

1. Cannot access the files (content) on SD & eMMC devices

After applying (rebasing onto the latest codes) your patches, I found that 
though the SD & eMMC devices can be detected, yet the content cannot be 
accessed.

There are 1 SD card and 1 embedded eMMC device within the system. Under Shell, 
I can see 2 file systems on SD & eMMC devices being detected, but when I try to 
access the files on those file systems by using the 'ls' command, it fails, 
saying "ls: File Not Found - 'FS0:\'". I can confirm that files can be listed 
successfully without applying this patch.

I also tried the 'dblk' command for display the data via BlockIO protocols 
created on those devices. I found that for SD, the command works fine. But for 
eMMC, the command fails with message "dblk: Read file error - 'BlockIo'".

For the hardware I own, the controllers are all version 3.0. I tested on both
IA32 and X64 arch image. The results were the same as described above.

Unfortunately, I do not have access to boards with SD controller with version 
newer than 3.0. So I am not able to perform those tests on my side for Version
4.0 or greater SD controllers.

Do you have any hardware with SD controller of version 3.0? Is it working on 
your side?

Please let me know if additional information is needed.


2. On SdMmcHcGetControllerVersion()

I found that there is a 'ControllerVersion' version in the data structure 
SD_MMC_HC_PRIVATE_DATA. But currently, this field seems never used.

I think we can get the controller version information only once and use this 
field to store it. Hence, this new function can be eliminated and also can 
avoid calling it multiple times.


3. Setting the SD_MMC_HC_64_ADDR_EN bit in SdMmcHcV4Init64BitSupport()

The setting of the SD_MMC_HC_64_ADDR_EN bit is decided by the 'SysBus64V4'
field in the CAP register. For me, additional dependency on the 64-bit DMA 
support in the PCI layer is needed as well.

In function SdMmcPciHcDriverBindingStart():

  //
  // Enable 64-bit DMA support in the PCI layer if this controller
  // supports it.
  //
  if (Support64BitDma) {
Status = PciIo->Attributes (
  PciIo,
  EfiPciIoAttributeOperationEnable,
  EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE,
  NULL
  );
if (EFI_ERROR (Status)) {
  DEBUG ((DEBUG_WARN, "SdMmcPciHcDriverBindingStart: failed to enable 
64-bit DMA (%r)\n", Status));
}
  }

If the above PCI attribute setting fails, the PCI layer will not support the 
64-bit DMA.

Hence, I am thinking to introduce a new BOOLEAN field in the 
"SD_MMC_HC_PRIVATE_DATA" data structure (maybe putting the 'Support64BitDma'
into the data structure). If the above PCI operation succeeds, the BOOLEAN 
field will be set to TRUE, otherwise, FALSE.

And the setting of the SD_MMC_HC_64_ADDR_EN bit should depend on the BOOLEAN 
field as well.


4. Please help to rebase the patch upon the latest master branch.

Some changes within the SdMmcPciHcDxe have been pushed recently. Could you help 
to rebase this patch upon the latest changes. Thanks in advance.


Also,

Re: [edk2] [edk2-announce] Research Request

2018-11-29 Thread Rebecca Cran via edk2-devel
Would you be interested in going through this process with Phabricator, too? 

Rebecca


On November 29, 2018 at 2:48:18 AM, Laszlo Ersek 
(ler...@redhat.com(mailto:ler...@redhat.com)) wrote:

> On 11/29/18 02:07, Jeremiah Cox wrote:
> > I did a further experiment for you:
> > https://github.com/lersek/edk2/pull/2
> 
> Thanks!
> 
> > I cannot rebase away my history from PRs...
> > Hopefully you have a nice email trail too.
> 
> The emails are coming in nice, but I'm not universally pleased with the
> contents. I listed some issues regarding that in
> , but I guess I should write them
> up sometime more readably, at the end of this experiment.
> 
> Thanks!
> Laszlo
> 
> > -Original Message-
> > From: Laszlo Ersek 
> > Sent: Wednesday, November 28, 2018 2:02 PM
> > To: Jeremiah Cox ; Brian J. Johnson 
> > ; stephano 
> > Cc: edk2-devel@lists.01.org
> > Subject: Re: [edk2] [edk2-announce] Research Request
> > 
> > On 11/28/18 19:31, Jeremiah Cox wrote:
> > > Test PR submitted
> > 
> > Thanks!
> > Laszlo
> > 
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [RFC] Proposal to add edk2-apps repository

2018-11-29 Thread Leif Lindholm
On Thu, Nov 29, 2018 at 05:58:08PM +, Kinney, Michael D wrote:
> Hello,
> 
> I would like to propose the creation of a new
> repository called edk2-apps.  This repository
> would initially be used to host the following
> packages from the edk2 repository:
> 
> * AppPkg
> * StdLib
> * StdLibPrivateInternalFiles

Let me start by saying I 100% back moving these out of the main edk2
repository.

> These 3 packages provide support for the libc along
> with applications that depend on libc.  None of the
> other packages in the edk2 repository use these
> packages, so these 3 package can be safely moved
> without any impacts to platform firmware builds.
> Build configurations that do use libc features can
> clone the edk2-apps repository and add it to
> PACKAGES_PATH.

I must confess to never having properly understood the scope of AppPkg
to begin with.

AppPkg/Applications/Hello does not appear to have any further (real)
dependency on libc than MdeModulePkg/Application/HelloWorld/, and .

And certainly MdeModulePkg/Applications contain plenty of
... applications.

So, if the purpose is simply to provide some examples of application
written to libc rather than UEFI - should this be edk2-libc instead?

Best Regards,

Leif

> The history of these 3 packages would be preserved
> when importing the content into edk2-apps.  After
> The import is verified, these 3 packages would be
> deleted from the edk2 repository.
> 
> This proposal helps reduce the size of the edk2
> repository and focuses edk2 repository on packages
> used to provide UEFI/PI conformant firmware.
> 
> If there are no concerns with this proposal, I will
> enter a Tianocore BZs for the two steps.
> 
> Best regards,
> 
> Mike
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [RFC] Change EDK II to an Apache 2.0 License

2018-11-29 Thread Leif Lindholm
On Thu, Nov 29, 2018 at 06:39:28PM +, Kinney, Michael D wrote:
> Hello,
> 
> This RFC follows up on the proposal from Mark Doran to change the 
> EDK II Project to an Apache 2.0 License.
> 
> https://lists.01.org/pipermail/edk2-devel/2018-October/030385.html
> 
> 
>   ** Please provide feedback on the proposal by Friday 12/7/18. **

7 December 2018 to anyone outside the US :)
 
> I will be following up with pointers to public GitHub branches that
> contain the initial set of changes in steps (1) and (2) below for 
> review. 
> 
> The proposal is to perform this change to edk2/master in the steps listed
> below. The license change will not be applied to any of the other existing
> branches in the edk2 repository.
> 
> 1) Change all files with a BSD 2-Clause license and only a single 
>copyright statement by Intel Corporation to an Apache 2.0 license
>and add an SPDX-License-Identifier statement.
> 
>==
>SPDX-License-Identifier: Apache-2.0
> 
>Licensed under the Apache License, Version 2.0 (the "License");
>you may not use this file except in compliance with the License.
>You may obtain a copy of the License at
> 
>http://www.apache.org/licenses/LICENSE-2.0
> 
>Unless required by applicable law or agreed to in writing, software
>distributed under the License is distributed on an "AS IS" BASIS,
>WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>See the License for the specific language governing permissions and
>limitations under the License.
>==
> 
> 2) Update Readme.md and License.txt in the root of the edk2 repository to
>state that content is covered by a mix of BSD 2-Clause and Apache 2.0
>licenses. 
> 
> 3) Update all documentation to state that content submitted under the 
>Apache 2.0 license no longer requires the Tianocore Contribution
>Agreement which means the following line is not required in commit
>messages for changes to files that are covered by an Apache 2.0 License.

Presumably this also applies to files _added_ with an Apache 2.0
License? (If so, the above could benefit from minor rewording.)

>Contributed-under: TianoCore Contribution Agreement 1.1
> 
> 4) Create Wiki page(s) that provide the details of the Apache 2.0 License
>change and provide the status of the license change for each package
>in the edk2 repository.  Also provide a list of the additional copyright
>holders that need to be contacted to accept the change to an Apache 2.0
>License along with the status of that acceptance.
> 
> 5) After all copyright holders have accepted the change to an Apache 2.0
>License, change the remaining files from BSD 2-Clause to Apache 2.0.
> 
> 6) Update Readme.md and License.txt in the edk2 repository to state that
>Apache 2.0 is the preferred license for the EDK II project.

>From Linaro's side, we have no concern beyond that we'd like to hear
ARM's opinion. (Adding cc.)

Regards,

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


[edk2] [Patch] UefiCpuPkg/MpLib: Fix PEI Services Table pointer on AP

2018-11-29 Thread Felix Polyudov
According to PI specification PEI Services table pointer is stored
right before ITD base. Starting from commit c563077a380437c1
BSP and AP have different IDT instances.
PEI Services table pointer was not initialized in the AP IDT instance.
As a result, any attempt to use functions from
PeiServicesTablePointerLib or PeiServicesLib on AP caused CPU exception.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Felix Polyudov 
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 7f4d6e6..0e3e362 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -1567,6 +1567,7 @@ MpInitLibInitialize (
   BufferSize  = ApStackSize * MaxLogicalProcessorNumber;
   BufferSize += MonitorFilterSize * MaxLogicalProcessorNumber;
   BufferSize += ApResetVectorSize;
+  BufferSize += sizeof(UINTN);
   BufferSize  = ALIGN_VALUE (BufferSize, 8);
   BufferSize += VolatileRegisters.Idtr.Limit + 1;
   BufferSize += sizeof (CPU_MP_DATA);
@@ -1587,6 +1588,8 @@ MpInitLibInitialize (
   // Backup Buffer
   //++
   //   Padding
+  //++
+  //PEI Services Table Pointer
   //++ <-- ApIdtBase (8-byte boundary)
   //   AP IDT  All APs share one separate IDT. So AP can get 
address of CPU_MP_DATA from IDT Base.
   //++ <-- CpuMpData
@@ -1599,7 +1602,7 @@ MpInitLibInitialize (
   //
   MonitorBuffer= (UINT8 *) (Buffer + ApStackSize * 
MaxLogicalProcessorNumber);
   BackupBufferAddr = (UINTN) MonitorBuffer + MonitorFilterSize * 
MaxLogicalProcessorNumber;
-  ApIdtBase= ALIGN_VALUE (BackupBufferAddr + ApResetVectorSize, 8);
+  ApIdtBase= ALIGN_VALUE (BackupBufferAddr + ApResetVectorSize + 
sizeof(UINTN), 8);
   CpuMpData= (CPU_MP_DATA *) (ApIdtBase + VolatileRegisters.Idtr.Limit 
+ 1);
   CpuMpData->Buffer   = Buffer;
   CpuMpData->CpuApStackSize   = ApStackSize;
@@ -1653,6 +1656,11 @@ MpInitLibInitialize (
   Buffer + BufferSize);
 
   //
+  // Initialize PEI Services table pointer. Copy the address from BSP.
+  //
+  *(UINTN*)(ApIdtBase - sizeof(UINTN)) = *(UINTN*)(VolatileRegisters.Idtr.Base 
- sizeof (UINTN));
+
+  //
   // Duplicate BSP's IDT to APs.
   // All APs share one separate IDT. So AP can get the address of CpuMpData by 
using IDTR.BASE + IDTR.LIMIT + 1
   //
-- 
2.10.0.windows.1



Please consider the environment before printing this email.

The information contained in this message may be confidential and proprietary 
to American Megatrends, Inc.  This communication is intended to be read only by 
the individual or entity to whom it is addressed or by their designee. If the 
reader of this message is not the intended recipient, you are on notice that 
any distribution of this message, in any form, is strictly prohibited.  Please 
promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and 
then delete or destroy all copies of the transmission.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [RFC] Proposal to add edk2-apps repository

2018-11-29 Thread Kinney, Michael D
Leif,

I did consider the edk2-libc name.  The port of Python 2.7 
is in the AppPkg as well and it uses libc.

So the content of this new package is a combination of libc
And apps that use libc.

I am definitely open to alternate names.  2 options so far:

* edk2-apps
* edk2-libc

Thanks,

Mike

> -Original Message-
> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> Sent: Thursday, November 29, 2018 2:41 PM
> To: Kinney, Michael D 
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [RFC] Proposal to add edk2-apps
> repository
> 
> On Thu, Nov 29, 2018 at 05:58:08PM +, Kinney, Michael
> D wrote:
> > Hello,
> >
> > I would like to propose the creation of a new
> > repository called edk2-apps.  This repository
> > would initially be used to host the following
> > packages from the edk2 repository:
> >
> > * AppPkg
> > * StdLib
> > * StdLibPrivateInternalFiles
> 
> Let me start by saying I 100% back moving these out of the
> main edk2
> repository.
> 
> > These 3 packages provide support for the libc along
> > with applications that depend on libc.  None of the
> > other packages in the edk2 repository use these
> > packages, so these 3 package can be safely moved
> > without any impacts to platform firmware builds.
> > Build configurations that do use libc features can
> > clone the edk2-apps repository and add it to
> > PACKAGES_PATH.
> 
> I must confess to never having properly understood the
> scope of AppPkg
> to begin with.
> 
> AppPkg/Applications/Hello does not appear to have any
> further (real)
> dependency on libc than
> MdeModulePkg/Application/HelloWorld/, and .
> 
> And certainly MdeModulePkg/Applications contain plenty of
> ... applications.
> 
> So, if the purpose is simply to provide some examples of
> application
> written to libc rather than UEFI - should this be edk2-
> libc instead?
> 
> Best Regards,
> 
> Leif
> 
> > The history of these 3 packages would be preserved
> > when importing the content into edk2-apps.  After
> > The import is verified, these 3 packages would be
> > deleted from the edk2 repository.
> >
> > This proposal helps reduce the size of the edk2
> > repository and focuses edk2 repository on packages
> > used to provide UEFI/PI conformant firmware.
> >
> > If there are no concerns with this proposal, I will
> > enter a Tianocore BZs for the two steps.
> >
> > Best regards,
> >
> > Mike
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [RFC] Proposal to add edk2-apps repository

2018-11-29 Thread krishnaLee
Kinney,
I always think there may be two kinds of apps:
1,some apps have dependency on uefi_shell(shell-lib,efi_shell_protocol,...they 
usually execute under uefi_shell),I would call them "uefi_shell_application";
2,some apps have no dependency on uefi_shell(such as apps in 
MdeModulePkg/Application),I would call them "standard_uefi_application".

The "AppPkg / StdLib / StdLibPrivateInternalFiles" packages are usually used by 
uefi_shell_application,I think they can all move to ShellPkg,no need to create 
new package ?


Thanks,
krishna.

At 2018-11-30 08:46:58, "Kinney, Michael D"  wrote:
>Leif,
>
>I did consider the edk2-libc name.  The port of Python 2.7 
>is in the AppPkg as well and it uses libc.
>
>So the content of this new package is a combination of libc
>And apps that use libc.
>
>I am definitely open to alternate names.  2 options so far:
>
>* edk2-apps
>* edk2-libc
>
>Thanks,
>
>Mike
>
>> -Original Message-
>> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
>> Sent: Thursday, November 29, 2018 2:41 PM
>> To: Kinney, Michael D 
>> Cc: edk2-devel@lists.01.org
>> Subject: Re: [edk2] [RFC] Proposal to add edk2-apps
>> repository
>> 
>> On Thu, Nov 29, 2018 at 05:58:08PM +, Kinney, Michael
>> D wrote:
>> > Hello,
>> >
>> > I would like to propose the creation of a new
>> > repository called edk2-apps.  This repository
>> > would initially be used to host the following
>> > packages from the edk2 repository:
>> >
>> > * AppPkg
>> > * StdLib
>> > * StdLibPrivateInternalFiles
>> 
>> Let me start by saying I 100% back moving these out of the
>> main edk2
>> repository.
>> 
>> > These 3 packages provide support for the libc along
>> > with applications that depend on libc.  None of the
>> > other packages in the edk2 repository use these
>> > packages, so these 3 package can be safely moved
>> > without any impacts to platform firmware builds.
>> > Build configurations that do use libc features can
>> > clone the edk2-apps repository and add it to
>> > PACKAGES_PATH.
>> 
>> I must confess to never having properly understood the
>> scope of AppPkg
>> to begin with.
>> 
>> AppPkg/Applications/Hello does not appear to have any
>> further (real)
>> dependency on libc than
>> MdeModulePkg/Application/HelloWorld/, and .
>> 
>> And certainly MdeModulePkg/Applications contain plenty of
>> ... applications.
>> 
>> So, if the purpose is simply to provide some examples of
>> application
>> written to libc rather than UEFI - should this be edk2-
>> libc instead?
>> 
>> Best Regards,
>> 
>> Leif
>> 
>> > The history of these 3 packages would be preserved
>> > when importing the content into edk2-apps.  After
>> > The import is verified, these 3 packages would be
>> > deleted from the edk2 repository.
>> >
>> > This proposal helps reduce the size of the edk2
>> > repository and focuses edk2 repository on packages
>> > used to provide UEFI/PI conformant firmware.
>> >
>> > If there are no concerns with this proposal, I will
>> > enter a Tianocore BZs for the two steps.
>> >
>> > Best regards,
>> >
>> > Mike
>> > ___
>> > edk2-devel mailing list
>> > edk2-devel@lists.01.org
>> > https://lists.01.org/mailman/listinfo/edk2-devel
>___
>edk2-devel mailing list
>edk2-devel@lists.01.org
>https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [RFC] Proposal to add edk2-apps repository

2018-11-29 Thread Wang, Jian J
Hi Krishna and Mike,

There might be applications which don't depend on ShellPkg or StdLib. I think
an extra repo is necessary. For example, the MicroPythonPkg (currently in 
staging
branch) doesn't depend on ShellPkg and StdLib. Maybe we can consider to put it
in edk2-apps but not edk2-libc.

Regards,
Jian


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> krishnaLee
> Sent: Friday, November 30, 2018 9:45 AM
> To: edk2-devel@lists.01.org
> Subject: Re: [edk2] [RFC] Proposal to add edk2-apps repository
> 
> Kinney,
> I always think there may be two kinds of apps:
> 1,some apps have dependency on uefi_shell(shell-lib,efi_shell_protocol,...they
> usually execute under uefi_shell),I would call them "uefi_shell_application";
> 2,some apps have no dependency on uefi_shell(such as apps in
> MdeModulePkg/Application),I would call them "standard_uefi_application".
> 
> The "AppPkg / StdLib / StdLibPrivateInternalFiles" packages are usually used 
> by
> uefi_shell_application,I think they can all move to ShellPkg,no need to create
> new package ?
> 
> 
> Thanks,
> krishna.
> 
> At 2018-11-30 08:46:58, "Kinney, Michael D" 
> wrote:
> >Leif,
> >
> >I did consider the edk2-libc name.  The port of Python 2.7
> >is in the AppPkg as well and it uses libc.
> >
> >So the content of this new package is a combination of libc
> >And apps that use libc.
> >
> >I am definitely open to alternate names.  2 options so far:
> >
> >* edk2-apps
> >* edk2-libc
> >
> >Thanks,
> >
> >Mike
> >
> >> -Original Message-
> >> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> >> Sent: Thursday, November 29, 2018 2:41 PM
> >> To: Kinney, Michael D 
> >> Cc: edk2-devel@lists.01.org
> >> Subject: Re: [edk2] [RFC] Proposal to add edk2-apps
> >> repository
> >>
> >> On Thu, Nov 29, 2018 at 05:58:08PM +, Kinney, Michael
> >> D wrote:
> >> > Hello,
> >> >
> >> > I would like to propose the creation of a new
> >> > repository called edk2-apps.  This repository
> >> > would initially be used to host the following
> >> > packages from the edk2 repository:
> >> >
> >> > * AppPkg
> >> > * StdLib
> >> > * StdLibPrivateInternalFiles
> >>
> >> Let me start by saying I 100% back moving these out of the
> >> main edk2
> >> repository.
> >>
> >> > These 3 packages provide support for the libc along
> >> > with applications that depend on libc.  None of the
> >> > other packages in the edk2 repository use these
> >> > packages, so these 3 package can be safely moved
> >> > without any impacts to platform firmware builds.
> >> > Build configurations that do use libc features can
> >> > clone the edk2-apps repository and add it to
> >> > PACKAGES_PATH.
> >>
> >> I must confess to never having properly understood the
> >> scope of AppPkg
> >> to begin with.
> >>
> >> AppPkg/Applications/Hello does not appear to have any
> >> further (real)
> >> dependency on libc than
> >> MdeModulePkg/Application/HelloWorld/, and .
> >>
> >> And certainly MdeModulePkg/Applications contain plenty of
> >> ... applications.
> >>
> >> So, if the purpose is simply to provide some examples of
> >> application
> >> written to libc rather than UEFI - should this be edk2-
> >> libc instead?
> >>
> >> Best Regards,
> >>
> >> Leif
> >>
> >> > The history of these 3 packages would be preserved
> >> > when importing the content into edk2-apps.  After
> >> > The import is verified, these 3 packages would be
> >> > deleted from the edk2 repository.
> >> >
> >> > This proposal helps reduce the size of the edk2
> >> > repository and focuses edk2 repository on packages
> >> > used to provide UEFI/PI conformant firmware.
> >> >
> >> > If there are no concerns with this proposal, I will
> >> > enter a Tianocore BZs for the two steps.
> >> >
> >> > Best regards,
> >> >
> >> > Mike
> >> > ___
> >> > edk2-devel mailing list
> >> > edk2-devel@lists.01.org
> >> > https://lists.01.org/mailman/listinfo/edk2-devel
> >___
> >edk2-devel mailing list
> >edk2-devel@lists.01.org
> >https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH v3 0/2] Remove DuetPkg and unused tools

2018-11-29 Thread Shenglei Zhang
DuetPkg depends on Legacy BIOS to provide a UEFI environment.
It was invented in the era when UEFI environment is hard to find.
Since now UEFI is very popular in PC area, we could stop the
official support of this package and remove it from the master.
And moreover, the tools only used by DuetPkg can also be removed.
https://bugzilla.tianocore.org/show_bug.cgi?id=1322

The changes are placed in
https://github.com/shenglei10/edk2/commits/duet

v2:Remove these tools in Makefile and GNUmakefile.

v3:Change the commit order.

Cc: Ruiyu Ni 
Cc: Hao Wu 
Cc: Yonghong Zhu 
Cc: Liming Gao 
Shenglei Zhang (2):
  DuetPkg: Remove DuetPkg
  BaseTools: Remove tools only used by DuetPkg

 BaseTools/Source/BinaryFiles.txt  |4 -
 BaseTools/Source/C/BootSectImage/GNUmakefile  |   21 -
 BaseTools/Source/C/BootSectImage/Makefile |   22 -
 .../Source/C/BootSectImage/bootsectimage.c|  955 --
 BaseTools/Source/C/BootSectImage/fat.h|  152 -
 BaseTools/Source/C/BootSectImage/mbr.h|   58 -
 BaseTools/Source/C/EfiLdrImage/EfiLdrImage.c  |  319 --
 BaseTools/Source/C/EfiLdrImage/GNUmakefile|   21 -
 BaseTools/Source/C/EfiLdrImage/Makefile   |   22 -
 BaseTools/Source/C/GNUmakefile|5 -
 BaseTools/Source/C/GenBootSector/FatFormat.h  |  152 -
 .../Source/C/GenBootSector/GenBootSector.c|  823 -
 .../Source/C/GenBootSector/GetDrvNumOffset.c  |   73 -
 BaseTools/Source/C/GenBootSector/Makefile |   22 -
 BaseTools/Source/C/GenPage/GNUmakefile|   21 -
 BaseTools/Source/C/GenPage/GenPage.c  |  441 ---
 BaseTools/Source/C/GenPage/Makefile   |   22 -
 BaseTools/Source/C/GenPage/VirtualMemory.h|  122 -
 .../Source/C/GnuGenBootSector/FatFormat.h |  152 -
 .../Source/C/GnuGenBootSector/GNUmakefile |   21 -
 .../C/GnuGenBootSector/GnuGenBootSector.c |  455 ---
 BaseTools/Source/C/Makefile   |4 -
 BaseTools/toolsetup.bat   |4 -
 DuetPkg/AcpiResetDxe/Reset.c  |  212 --
 DuetPkg/AcpiResetDxe/Reset.inf|   47 -
 DuetPkg/BiosVideoThunkDxe/BiosVideo.c | 2822 -
 DuetPkg/BiosVideoThunkDxe/BiosVideo.h |  504 ---
 DuetPkg/BiosVideoThunkDxe/BiosVideo.inf   |   50 -
 DuetPkg/BiosVideoThunkDxe/ComponentName.c |  166 -
 DuetPkg/BiosVideoThunkDxe/LegacyBiosThunk.c   |  220 --
 .../BiosVideoThunkDxe/VesaBiosExtensions.h|  457 ---
 DuetPkg/BootSector/BootSector.inf |   79 -
 DuetPkg/BootSector/FILE.LST   |   39 -
 DuetPkg/BootSector/GNUmakefile|  140 -
 DuetPkg/BootSector/Gpt.S  |  297 --
 DuetPkg/BootSector/Gpt.asm|  294 --
 DuetPkg/BootSector/Makefile   |  173 -
 DuetPkg/BootSector/Mbr.S  |  262 --
 DuetPkg/BootSector/Mbr.asm|  261 --
 DuetPkg/BootSector/bin/Gpt.com|  Bin 512 -> 0 bytes
 DuetPkg/BootSector/bin/Mbr.com|  Bin 512 -> 0 bytes
 DuetPkg/BootSector/bin/Readme.txt |8 -
 DuetPkg/BootSector/bin/St16_64.com|  Bin 4096 -> 0 bytes
 DuetPkg/BootSector/bin/St32_64.com|  Bin 4096 -> 0 bytes
 DuetPkg/BootSector/bin/Start.com  |  Bin 4096 -> 0 bytes
 DuetPkg/BootSector/bin/Start16.com|  Bin 4096 -> 0 bytes
 DuetPkg/BootSector/bin/Start32.com|  Bin 4096 -> 0 bytes
 DuetPkg/BootSector/bin/Start64.com|  Bin 4096 -> 0 bytes
 DuetPkg/BootSector/bin/bootsect.com   |  Bin 512 -> 0 bytes
 DuetPkg/BootSector/bin/bs16.com   |  Bin 512 -> 0 bytes
 DuetPkg/BootSector/bin/bs32.com   |  Bin 512 -> 0 bytes
 DuetPkg/BootSector/bin/efi32.com  |  Bin 139264 -> 0 bytes
 DuetPkg/BootSector/bin/efi32.com2 |  Bin 4096 -> 0 bytes
 DuetPkg/BootSector/bin/efi64.com  |  Bin 139264 -> 0 bytes
 DuetPkg/BootSector/bin/efi64.com2 |  Bin 4096 -> 0 bytes
 DuetPkg/BootSector/bootsect.S |  303 --
 DuetPkg/BootSector/bootsect.asm   |  301 --
 DuetPkg/BootSector/bs16.S |  291 --
 DuetPkg/BootSector/bs16.asm   |  288 --
 DuetPkg/BootSector/bs32.S |  312 --
 DuetPkg/BootSector/bs32.asm   |  310 --
 DuetPkg/BootSector/efi32.S| 1176 ---
 DuetPkg/BootSector/efi32.asm  |  582 
 DuetPkg/BootSector/efi64.S| 1385 
 DuetPkg/BootSector/efi64.asm  |  787 -
 DuetPkg/BootSector/st16_64.S  | 1142 ---
 DuetPkg/BootSector/st16_64.asm| 1140 ---
 DuetPkg/BootSector/st32_64.S  | 1157 ---
 DuetPkg/BootSector/st32_64.asm| 1156 ---
 DuetPkg/BootSector/start.S|  919 --
 DuetPkg/BootSector/start.asm  |  916 --
 DuetPkg/BootSector/start16.S   

Re: [edk2] [RFC] Proposal to add edk2-apps repository

2018-11-29 Thread Ni, Ruiyu
Mike,
It's a great idea to have another edk2-apps repo.

I have a question not quite related to this new repo but related to 
edk2-platforms repo.
There are many platforms in edk2 repo as well, like Ovmf, Emulator, 
Vlv2TbltDevicePkg.
What's your opinion/plan about these platforms?


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Kinney, Michael D
> Sent: Friday, November 30, 2018 1:58 AM
> To: edk2-devel@lists.01.org; Kinney, Michael D 
> Subject: [edk2] [RFC] Proposal to add edk2-apps repository
> 
> Hello,
> 
> I would like to propose the creation of a new repository called edk2-apps.  
> This
> repository would initially be used to host the following packages from the 
> edk2
> repository:
> 
> * AppPkg
> * StdLib
> * StdLibPrivateInternalFiles
> 
> These 3 packages provide support for the libc along with applications that
> depend on libc.  None of the other packages in the edk2 repository use these
> packages, so these 3 package can be safely moved without any impacts to
> platform firmware builds.
> Build configurations that do use libc features can clone the edk2-apps 
> repository
> and add it to PACKAGES_PATH.
> 
> The history of these 3 packages would be preserved when importing the content
> into edk2-apps.  After The import is verified, these 3 packages would be 
> deleted
> from the edk2 repository.
> 
> This proposal helps reduce the size of the edk2 repository and focuses edk2
> repository on packages used to provide UEFI/PI conformant firmware.
> 
> If there are no concerns with this proposal, I will enter a Tianocore BZs for 
> the
> two steps.
> 
> Best regards,
> 
> Mike
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [RFC] Proposal to add edk2-apps repository

2018-11-29 Thread Ni, Ruiyu
Krishna,
The reason there are applications inside MdeModulePkg/Application is that the 
shell protocol was in ShellPkg when the app was developed and MdeModulePkg 
cannot depend on ShellPkg (rule).
Now since shell protocol is moved to MdePkg, any apps can depend on shell 
protocol. (In fact they wanted to but just wasn't allowed due to reason above.)

I even prefer to move the ShellPkg to the edk2-app repo Mike proposed. Instead 
of enlarge the ShellPkg:)

I don't prefer edk2-libc unless we have a strategy/plan to make ordinary C 
developer easy by promoting the std-c pkg.
The other reason I prefer edk2-app is then ShellPkg might be moved to that new 
repo.


> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> krishnaLee
> Sent: Friday, November 30, 2018 9:45 AM
> To: edk2-devel@lists.01.org
> Subject: Re: [edk2] [RFC] Proposal to add edk2-apps repository
> 
> Kinney,
> I always think there may be two kinds of apps:
> 1,some apps have dependency on uefi_shell(shell-lib,efi_shell_protocol,...they
> usually execute under uefi_shell),I would call them "uefi_shell_application";
> 2,some apps have no dependency on uefi_shell(such as apps in
> MdeModulePkg/Application),I would call them "standard_uefi_application".
> 
> The "AppPkg / StdLib / StdLibPrivateInternalFiles" packages are usually used 
> by
> uefi_shell_application,I think they can all move to ShellPkg,no need to create
> new package ?
> 
> 
> Thanks,
> krishna.
> 
> At 2018-11-30 08:46:58, "Kinney, Michael D" 
> wrote:
> >Leif,
> >
> >I did consider the edk2-libc name.  The port of Python 2.7 is in the
> >AppPkg as well and it uses libc.
> >
> >So the content of this new package is a combination of libc And apps
> >that use libc.
> >
> >I am definitely open to alternate names.  2 options so far:
> >
> >* edk2-apps
> >* edk2-libc
> >
> >Thanks,
> >
> >Mike
> >
> >> -Original Message-
> >> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> >> Sent: Thursday, November 29, 2018 2:41 PM
> >> To: Kinney, Michael D 
> >> Cc: edk2-devel@lists.01.org
> >> Subject: Re: [edk2] [RFC] Proposal to add edk2-apps repository
> >>
> >> On Thu, Nov 29, 2018 at 05:58:08PM +, Kinney, Michael D wrote:
> >> > Hello,
> >> >
> >> > I would like to propose the creation of a new repository called
> >> > edk2-apps.  This repository would initially be used to host the
> >> > following packages from the edk2 repository:
> >> >
> >> > * AppPkg
> >> > * StdLib
> >> > * StdLibPrivateInternalFiles
> >>
> >> Let me start by saying I 100% back moving these out of the main edk2
> >> repository.
> >>
> >> > These 3 packages provide support for the libc along with
> >> > applications that depend on libc.  None of the other packages in
> >> > the edk2 repository use these packages, so these 3 package can be
> >> > safely moved without any impacts to platform firmware builds.
> >> > Build configurations that do use libc features can clone the
> >> > edk2-apps repository and add it to PACKAGES_PATH.
> >>
> >> I must confess to never having properly understood the scope of
> >> AppPkg to begin with.
> >>
> >> AppPkg/Applications/Hello does not appear to have any further (real)
> >> dependency on libc than MdeModulePkg/Application/HelloWorld/, and .
> >>
> >> And certainly MdeModulePkg/Applications contain plenty of ...
> >> applications.
> >>
> >> So, if the purpose is simply to provide some examples of application
> >> written to libc rather than UEFI - should this be edk2- libc instead?
> >>
> >> Best Regards,
> >>
> >> Leif
> >>
> >> > The history of these 3 packages would be preserved when importing
> >> > the content into edk2-apps.  After The import is verified, these 3
> >> > packages would be deleted from the edk2 repository.
> >> >
> >> > This proposal helps reduce the size of the edk2 repository and
> >> > focuses edk2 repository on packages used to provide UEFI/PI
> >> > conformant firmware.
> >> >
> >> > If there are no concerns with this proposal, I will enter a
> >> > Tianocore BZs for the two steps.
> >> >
> >> > Best regards,
> >> >
> >> > Mike
> >> > ___
> >> > edk2-devel mailing list
> >> > edk2-devel@lists.01.org
> >> > https://lists.01.org/mailman/listinfo/edk2-devel
> >___
> >edk2-devel mailing list
> >edk2-devel@lists.01.org
> >https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [Patch] UefiCpuPkg/MpLib: Fix PEI Services Table pointer on AP

2018-11-29 Thread Ni, Ruiyu
Felix,
I disagree:) Sorry about that. :)

The commit you mentioned might be made by me (didn't checked).
Because I aimed to avoid calling PEI services from AP. That's a violation of PI 
spec and not safe by design.

The AP calling standard services concern was raised by Andrew initially.

Thanks,
Ray

> -Original Message-
> From: Felix Polyudov [mailto:fel...@ami.com]
> Sent: Friday, November 30, 2018 8:36 AM
> To: edk2-devel@lists.01.org
> Cc: Dong, Eric ; Ni, Ruiyu ;
> ler...@redhat.com
> Subject: [Patch] UefiCpuPkg/MpLib: Fix PEI Services Table pointer on AP
> 
> According to PI specification PEI Services table pointer is stored right 
> before ITD
> base. Starting from commit c563077a380437c1 BSP and AP have different IDT
> instances.
> PEI Services table pointer was not initialized in the AP IDT instance.
> As a result, any attempt to use functions from PeiServicesTablePointerLib or
> PeiServicesLib on AP caused CPU exception.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Felix Polyudov 
> ---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 7f4d6e6..0e3e362 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -1567,6 +1567,7 @@ MpInitLibInitialize (
>BufferSize  = ApStackSize * MaxLogicalProcessorNumber;
>BufferSize += MonitorFilterSize * MaxLogicalProcessorNumber;
>BufferSize += ApResetVectorSize;
> +  BufferSize += sizeof(UINTN);
>BufferSize  = ALIGN_VALUE (BufferSize, 8);
>BufferSize += VolatileRegisters.Idtr.Limit + 1;
>BufferSize += sizeof (CPU_MP_DATA);
> @@ -1587,6 +1588,8 @@ MpInitLibInitialize (
>// Backup Buffer
>//++
>//   Padding
> +  //++
> +  //PEI Services Table Pointer
>//++ <-- ApIdtBase (8-byte boundary)
>//   AP IDT  All APs share one separate IDT. So AP can get 
> address of
> CPU_MP_DATA from IDT Base.
>//++ <-- CpuMpData
> @@ -1599,7 +1602,7 @@ MpInitLibInitialize (
>//
>MonitorBuffer= (UINT8 *) (Buffer + ApStackSize *
> MaxLogicalProcessorNumber);
>BackupBufferAddr = (UINTN) MonitorBuffer + MonitorFilterSize *
> MaxLogicalProcessorNumber;
> -  ApIdtBase= ALIGN_VALUE (BackupBufferAddr + ApResetVectorSize, 8);
> +  ApIdtBase= ALIGN_VALUE (BackupBufferAddr + ApResetVectorSize +
> sizeof(UINTN), 8);
>CpuMpData= (CPU_MP_DATA *) (ApIdtBase + 
> VolatileRegisters.Idtr.Limit +
> 1);
>CpuMpData->Buffer   = Buffer;
>CpuMpData->CpuApStackSize   = ApStackSize;
> @@ -1653,6 +1656,11 @@ MpInitLibInitialize (
>Buffer + BufferSize);
> 
>//
> +  // Initialize PEI Services table pointer. Copy the address from BSP.
> +  //
> +  *(UINTN*)(ApIdtBase - sizeof(UINTN)) =
> + *(UINTN*)(VolatileRegisters.Idtr.Base - sizeof (UINTN));
> +
> +  //
>// Duplicate BSP's IDT to APs.
>// All APs share one separate IDT. So AP can get the address of CpuMpData 
> by
> using IDTR.BASE + IDTR.LIMIT + 1
>//
> --
> 2.10.0.windows.1
> 
> 
> 
> Please consider the environment before printing this email.
> 
> The information contained in this message may be confidential and proprietary
> to American Megatrends, Inc.  This communication is intended to be read only
> by the individual or entity to whom it is addressed or by their designee. If 
> the
> reader of this message is not the intended recipient, you are on notice that 
> any
> distribution of this message, in any form, is strictly prohibited.  Please 
> promptly
> notify the sender by reply e-mail or by telephone at 770-246-8600, and then
> delete or destroy all copies of the transmission.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [RFC] Proposal to add edk2-apps repository

2018-11-29 Thread Kinney, Michael D
Ray,

Real HW platforms like Vlv2* and Quark* should be moved to
edk2-platforms.  Virtual platforms such as Ovmf and EmulatorPkg
should stay in edk2 repo to support validation.

We should enter Tianocore BZ to move the real HW platforms.

Thanks,

Mike

> -Original Message-
> From: Ni, Ruiyu
> Sent: Thursday, November 29, 2018 7:32 PM
> To: edk2-devel@lists.01.org; Kinney, Michael D
> 
> Subject: RE: [RFC] Proposal to add edk2-apps repository
> 
> Mike,
> It's a great idea to have another edk2-apps repo.
> 
> I have a question not quite related to this new repo but
> related to edk2-platforms repo.
> There are many platforms in edk2 repo as well, like Ovmf,
> Emulator, Vlv2TbltDevicePkg.
> What's your opinion/plan about these platforms?
> 
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-
> boun...@lists.01.org] On Behalf Of
> > Kinney, Michael D
> > Sent: Friday, November 30, 2018 1:58 AM
> > To: edk2-devel@lists.01.org; Kinney, Michael D
> 
> > Subject: [edk2] [RFC] Proposal to add edk2-apps
> repository
> >
> > Hello,
> >
> > I would like to propose the creation of a new repository
> called edk2-apps.  This
> > repository would initially be used to host the following
> packages from the edk2
> > repository:
> >
> > * AppPkg
> > * StdLib
> > * StdLibPrivateInternalFiles
> >
> > These 3 packages provide support for the libc along with
> applications that
> > depend on libc.  None of the other packages in the edk2
> repository use these
> > packages, so these 3 package can be safely moved without
> any impacts to
> > platform firmware builds.
> > Build configurations that do use libc features can clone
> the edk2-apps repository
> > and add it to PACKAGES_PATH.
> >
> > The history of these 3 packages would be preserved when
> importing the content
> > into edk2-apps.  After The import is verified, these 3
> packages would be deleted
> > from the edk2 repository.
> >
> > This proposal helps reduce the size of the edk2
> repository and focuses edk2
> > repository on packages used to provide UEFI/PI
> conformant firmware.
> >
> > If there are no concerns with this proposal, I will
> enter a Tianocore BZs for the
> > two steps.
> >
> > Best regards,
> >
> > Mike
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [RFC] Proposal to add edk2-apps repository

2018-11-29 Thread Andrew Fish
Mike,

As Krishna points out there are flavors of Apps. Do we want to have different 
packages for different flavor of apps, or different dirs in a more generic App 
package? Maybe we should define classes of UEFI Applications in the README.md 
and give them a place to live. 

I don't want to get too pedantic so we can have a 1st level of if you depend on 
X you go here. Maybe something if you depend on the clib you go in the Clib 
dir, if you depend on Shell you go in Shell. With a priority to the list so 
clib always means clib dir. etc.?

Thanks,

Andrew Fish

> On Nov 29, 2018, at 5:44 PM, krishnaLee  wrote:
> 
> Kinney,
> I always think there may be two kinds of apps:
> 1,some apps have dependency on 
> uefi_shell(shell-lib,efi_shell_protocol,...they usually execute under 
> uefi_shell),I would call them "uefi_shell_application";
> 2,some apps have no dependency on uefi_shell(such as apps in 
> MdeModulePkg/Application),I would call them "standard_uefi_application".
> 
> The "AppPkg / StdLib / StdLibPrivateInternalFiles" packages are usually used 
> by uefi_shell_application,I think they can all move to ShellPkg,no need to 
> create new package ?
> 
> 
> Thanks,
> krishna.
> 
> At 2018-11-30 08:46:58, "Kinney, Michael D"  
> wrote:
>> Leif,
>> 
>> I did consider the edk2-libc name.  The port of Python 2.7 
>> is in the AppPkg as well and it uses libc.
>> 
>> So the content of this new package is a combination of libc
>> And apps that use libc.
>> 
>> I am definitely open to alternate names.  2 options so far:
>> 
>> * edk2-apps
>> * edk2-libc
>> 
>> Thanks,
>> 
>> Mike
>> 
>>> -Original Message-
>>> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
>>> Sent: Thursday, November 29, 2018 2:41 PM
>>> To: Kinney, Michael D 
>>> Cc: edk2-devel@lists.01.org
>>> Subject: Re: [edk2] [RFC] Proposal to add edk2-apps
>>> repository
>>> 
>>> On Thu, Nov 29, 2018 at 05:58:08PM +, Kinney, Michael
>>> D wrote:
 Hello,
 
 I would like to propose the creation of a new
 repository called edk2-apps.  This repository
 would initially be used to host the following
 packages from the edk2 repository:
 
 * AppPkg
 * StdLib
 * StdLibPrivateInternalFiles
>>> 
>>> Let me start by saying I 100% back moving these out of the
>>> main edk2
>>> repository.
>>> 
 These 3 packages provide support for the libc along
 with applications that depend on libc.  None of the
 other packages in the edk2 repository use these
 packages, so these 3 package can be safely moved
 without any impacts to platform firmware builds.
 Build configurations that do use libc features can
 clone the edk2-apps repository and add it to
 PACKAGES_PATH.
>>> 
>>> I must confess to never having properly understood the
>>> scope of AppPkg
>>> to begin with.
>>> 
>>> AppPkg/Applications/Hello does not appear to have any
>>> further (real)
>>> dependency on libc than
>>> MdeModulePkg/Application/HelloWorld/, and .
>>> 
>>> And certainly MdeModulePkg/Applications contain plenty of
>>> ... applications.
>>> 
>>> So, if the purpose is simply to provide some examples of
>>> application
>>> written to libc rather than UEFI - should this be edk2-
>>> libc instead?
>>> 
>>> Best Regards,
>>> 
>>> Leif
>>> 
 The history of these 3 packages would be preserved
 when importing the content into edk2-apps.  After
 The import is verified, these 3 packages would be
 deleted from the edk2 repository.
 
 This proposal helps reduce the size of the edk2
 repository and focuses edk2 repository on packages
 used to provide UEFI/PI conformant firmware.
 
 If there are no concerns with this proposal, I will
 enter a Tianocore BZs for the two steps.
 
 Best regards,
 
 Mike
 ___
 edk2-devel mailing list
 edk2-devel@lists.01.org
 https://lists.01.org/mailman/listinfo/edk2-devel
>> ___
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

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