1. devel@edk2.groups.io
  2. Messages
   Date   
[PATCH v4 0/5] Build cache enhancement

Steven Shi
 

From: "Shi, Steven" <steven.shi@...>

Enhance the edk2 build cache with below patches:
Patch 01/05: Improve the cache hit rate through new cache checkpoint and hash algorithm
Patch 02/05: Print more info to explain why a module build cache miss
Patch 03/05: Fix the unsafe [self.Arch][self.Name] key usage in build cache
Patch 04/05 Add the GenFds multi-thread support in build cache
Patch 05/05 Improve the file saving and copying functions reliability in build cache


You can directly try this patch set in the branch:
https://github.com/shijunjing/edk2/tree/build_cache_improve_v4

V4:
Change single global lock into two locks, which are cache_lock and file_lock,
for better cache performance and IO reliability in windows

V3:
Add patch 5. To improve the autogen CopyFileOnChange() and SaveFileOnChange()
functions reliability for build cache

V2:
Enhance the SaveHashChainFileToCache() function in ModuleAutoGen.py and
not need to call f.close() in the "with open(xxx) as f:" block. The
with block will close the file automatically

V1:
Initial patch set

Shi, Steven (5):
BaseTools: Improve the cache hit in the edk2 build cache
BaseTools: Print first cache missing file for build cachle
BaseTools: Change the [Arch][Name] module key in Build cache
BaseTools: Add GenFds multi-thread support in build cache
BaseTools: Improve the file saving and copying reliability

.../Source/Python/AutoGen/AutoGenWorker.py | 27 +-
BaseTools/Source/Python/AutoGen/CacheIR.py | 28 +
BaseTools/Source/Python/AutoGen/DataPipe.py | 8 +
BaseTools/Source/Python/AutoGen/GenC.py | 0
BaseTools/Source/Python/AutoGen/GenMake.py | 229 +++---
.../Source/Python/AutoGen/ModuleAutoGen.py | 738 ++++++++++++++++--
BaseTools/Source/Python/Common/GlobalData.py | 11 +
BaseTools/Source/Python/Common/Misc.py | 29 +-
BaseTools/Source/Python/build/build.py | 182 +++--
9 files changed, 1011 insertions(+), 241 deletions(-)
mode change 100644 => 100755 BaseTools/Source/Python/AutoGen/AutoGenWorker.py
create mode 100755 BaseTools/Source/Python/AutoGen/CacheIR.py
mode change 100644 => 100755 BaseTools/Source/Python/AutoGen/DataPipe.py
mode change 100644 => 100755 BaseTools/Source/Python/AutoGen/GenC.py
mode change 100644 => 100755 BaseTools/Source/Python/AutoGen/GenMake.py
mode change 100644 => 100755 BaseTools/Source/Python/AutoGen/ModuleAutoGen.py
mode change 100644 => 100755 BaseTools/Source/Python/Common/GlobalData.py
mode change 100644 => 100755 BaseTools/Source/Python/Common/Misc.py
mode change 100644 => 100755 BaseTools/Source/Python/build/build.py

--
2.17.1
Re: [PATCH v1 0/1] Added GOP driver for DisplayLink-based Universal USB Docks to edk2-platforms

Leif Lindholm
 

Hi Andy,

A follow-up - a few more things need changing:

UsbDisplayLink.h (well, any .h file) should only contain the headers
it itself requires.
Moreover, the inclusion of wchar.h must disappear completely.
Instead, use CHAR16 * in the two locations in UsbDisplayLink.c.

Removing wchar.h will then trip you up in Edid.h, at
#define EDID_HEADER_SIZE ((size_t)8)
Please change the size_t to UINTN.

Finally, you need to (and now can after the above changes) get rid of
#undef _DEBUG

#undef NULL
#undef _ASSERT
in UsbDisplayLink.h.

With these additional changes, the driver builds successfully across
GCC 8.3 and CLANG 7.0 for AARCH64/ARM/IA32/X64.

Best Regards,

Leif

On Wed, Aug 14, 2019 at 06:03:51PM +0100, Leif Lindholm wrote:
Hi Andy,

Many thanks for this submission.

I am finding a few issues when building with gcc/clang (I expect you
use Visual Studio).

The RELEASE target barfs with
.../edk2-platforms/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/Gop.c:363:1:
error: unused function 'CalculateRefreshRate'
since the function is only called from a DEBUG statement.
Could be resolved by putting the function inside #ifndef MDEPKG_NDEBUG

Clang triggers an error on the
USB_DISPLAYLINK_DEV_FROM_GRAPHICS_OUTPUT_PROTOCOL macro which seems to
go away if the USB_DISPLAYLINK_DEV_SIGNATURE macro is moved below the
USB_DISPLAYLINK_DEV definition. (I haven't bothered to figure out why
this helps.)

The NOOPT build fails for IA32/X64 on clang with
.../edk2-platforms/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbDescriptors.c:35:
undefined reference to `memset'
You could get rid of this by doing the assignment separate from the
definition. (Indeed, I believe this is one of the reasons for the
rule.)

On X64/clang, the build fails with
.../edk2-platforms/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbDisplayLink.c:410:3:
error: '__builtin_ms_va_start' used in System V ABI function
Adding EFIAPI to the definition/declaration of DlGopPrintTextToScreen
makes this one go away.

Best Regards,

Leif

On Wed, Aug 14, 2019 at 02:43:43PM +0000, Andy Hayes wrote:
From 4a42eb997aeb3699217b40bf3bc47dec56458847 Mon Sep 17 00:00:00 2001
From: "Andy Hayes" < andy.hayes@... >
Date: Wed, 14 Aug 2019 15:30:20 +0100
Subject: [PATCH v1 0/1] Added GOP graphics driver for DisplayLink-based Universal USB Docks to edk2-platforms

andy.hayes@... (1):
Added GOP driver for USB Docks which use DisplayLink chips.

.../DisplayLinkPkg/DisplayLinkPkg.dsc | 61 +
.../DisplayLinkGop/DisplayLinkGopDxe.inf | 63 +
.../DisplayLinkPkg/DisplayLinkGop/Edid.h | 129 ++
.../DisplayLinkGop/UsbDescriptors.h | 109 ++
.../DisplayLinkGop/UsbDisplayLink.h | 284 +++++
.../DisplayLinkGop/CapabilityDescriptor.c | 137 ++
.../DisplayLinkGop/ComponentName.c | 235 ++++
.../DisplayLinkPkg/DisplayLinkGop/Edid.c | 598 +++++++++
.../DisplayLinkPkg/DisplayLinkGop/Gop.c | 587 +++++++++
.../DisplayLinkGop/UsbDescriptors.c | 144 +++
.../DisplayLinkGop/UsbDisplayLink.c | 1109 +++++++++++++++++
.../DisplayLinkGop/UsbTransfer.c | 180 +++
.../DisplayLinkGop/VideoModes.c | 254 ++++
Drivers/DisplayLink/DisplayLinkPkg/ReadMe.md | 77 ++
Maintainers.txt | 5 +
15 files changed, 3972 insertions(+)
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkPkg.dsc
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/DisplayLinkGopDxe.inf
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/Edid.h
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbDescriptors.h
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbDisplayLink.h
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/CapabilityDescriptor.c
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/ComponentName.c
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/Edid.c
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/Gop.c
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbDescriptors.c
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbDisplayLink.c
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/VideoModes.c
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/ReadMe.md

--
2.17.1
Re: [PATCH 1/1] MdeModulePkg/DxeIplPeim: Initialize pointer PageMapLevel5Entry

Laszlo Ersek
 

On 08/14/19 09:37, Zhang, Shenglei wrote:
Initialize PageMapLevel5Entry at the beginning of the function.

This commit will fix a GCC 4.8.5 build failure introduced by commit
b3527dedc3951f061c5a73cb4fb2b0f95f47e08b.

OvmfPkg build failure wtih gcc 4.8.5 still exists at latest edk2 version.
The commit 46f8a6891606746ca8b1e684ac379ce271306dc0 seems not to fix
the build failure completely.

Cc: Dandan Bi <dandan.bi@...>
Cc: Liming Gao <liming.gao@...>
Cc: Hao A Wu <hao.a.wu@...>
Signed-off-by: Shenglei Zhang <shenglei.zhang@...>
---
MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
index 2389f3eb485b..aae80536ac3d 100644
--- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
+++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c
@@ -652,6 +652,8 @@ CreateIdentityMappingPageTables (
UINT64 AddressEncMask;
IA32_CR4 Cr4;

+ PageMapLevel5Entry = NULL;
+
//
// Make sure AddressEncMask is contained to smallest supported address field
//
If you are convinced that we need this assignment *only* for suppressing
an invalid compiler warning, then please add a comment about it:

//
// set PageMapLevel5Entry to suppress incorrect compiler/analyzer
// warnings
//

Related documentation BZ:

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

... Just a suggestion from my side; I defer to the MdeModulePkg maintainers.

Thanks
Laszlo
Re: [PATCH v1 0/1] Added GOP driver for DisplayLink-based Universal USB Docks to edk2-platforms

Leif Lindholm
 

Hi Andy,

Many thanks for this submission.

I am finding a few issues when building with gcc/clang (I expect you
use Visual Studio).

The RELEASE target barfs with
.../edk2-platforms/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/Gop.c:363:1:
error: unused function 'CalculateRefreshRate'
since the function is only called from a DEBUG statement.
Could be resolved by putting the function inside #ifndef MDEPKG_NDEBUG

Clang triggers an error on the
USB_DISPLAYLINK_DEV_FROM_GRAPHICS_OUTPUT_PROTOCOL macro which seems to
go away if the USB_DISPLAYLINK_DEV_SIGNATURE macro is moved below the
USB_DISPLAYLINK_DEV definition. (I haven't bothered to figure out why
this helps.)

The NOOPT build fails for IA32/X64 on clang with
.../edk2-platforms/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbDescriptors.c:35:
undefined reference to `memset'
You could get rid of this by doing the assignment separate from the
definition. (Indeed, I believe this is one of the reasons for the
rule.)

On X64/clang, the build fails with
.../edk2-platforms/Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbDisplayLink.c:410:3:
error: '__builtin_ms_va_start' used in System V ABI function
Adding EFIAPI to the definition/declaration of DlGopPrintTextToScreen
makes this one go away.

Best Regards,

Leif

On Wed, Aug 14, 2019 at 02:43:43PM +0000, Andy Hayes wrote:
From 4a42eb997aeb3699217b40bf3bc47dec56458847 Mon Sep 17 00:00:00 2001
From: "Andy Hayes" < andy.hayes@... >
Date: Wed, 14 Aug 2019 15:30:20 +0100
Subject: [PATCH v1 0/1] Added GOP graphics driver for DisplayLink-based Universal USB Docks to edk2-platforms

andy.hayes@... (1):
Added GOP driver for USB Docks which use DisplayLink chips.

.../DisplayLinkPkg/DisplayLinkPkg.dsc | 61 +
.../DisplayLinkGop/DisplayLinkGopDxe.inf | 63 +
.../DisplayLinkPkg/DisplayLinkGop/Edid.h | 129 ++
.../DisplayLinkGop/UsbDescriptors.h | 109 ++
.../DisplayLinkGop/UsbDisplayLink.h | 284 +++++
.../DisplayLinkGop/CapabilityDescriptor.c | 137 ++
.../DisplayLinkGop/ComponentName.c | 235 ++++
.../DisplayLinkPkg/DisplayLinkGop/Edid.c | 598 +++++++++
.../DisplayLinkPkg/DisplayLinkGop/Gop.c | 587 +++++++++
.../DisplayLinkGop/UsbDescriptors.c | 144 +++
.../DisplayLinkGop/UsbDisplayLink.c | 1109 +++++++++++++++++
.../DisplayLinkGop/UsbTransfer.c | 180 +++
.../DisplayLinkGop/VideoModes.c | 254 ++++
Drivers/DisplayLink/DisplayLinkPkg/ReadMe.md | 77 ++
Maintainers.txt | 5 +
15 files changed, 3972 insertions(+)
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkPkg.dsc
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/DisplayLinkGopDxe.inf
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/Edid.h
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbDescriptors.h
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbDisplayLink.h
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/CapabilityDescriptor.c
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/ComponentName.c
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/Edid.c
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/Gop.c
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbDescriptors.c
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbDisplayLink.c
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/VideoModes.c
create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/ReadMe.md

--
2.17.1
Re: static data in dxe_runtime modules

Andrew Fish
 



On Aug 14, 2019, at 8:16 AM, Laszlo Ersek <lersek@...> wrote:

On 08/13/19 13:23, Roman Kagan wrote:
On Tue, Aug 13, 2019 at 11:10:27AM +0200, Laszlo Ersek wrote:
On 08/12/19 20:43, Roman Kagan wrote:
On Fri, Aug 09, 2019 at 04:07:00PM +0000, Roman Kagan via Groups.Io wrote:
On Thu, Aug 08, 2019 at 07:39:14PM +0200, Laszlo Ersek wrote:
On 08/07/19 19:41, Andrew Fish wrote:
On Aug 7, 2019, at 10:29 AM, Laszlo Ersek <lersek@...> wrote:
On 08/05/19 12:18, Roman Kagan wrote:
On Sat, Aug 03, 2019 at 04:03:04AM +0200, Laszlo Ersek via Groups.Io wrote:
On 08/01/19 21:16, Roman Kagan wrote:
I'm convinced that OpenSSL needs to expose a new API for this particular
problem.

Since, as you point out below, the problem only affects the essentially
broken configuration (SECURE_BOOT_ENABLE && !SMM_REQUIRE), I'm fine with
saving time and effort and sticking to the hack-ish approach proposed in
the bugzilla issue, which is to iterate over "thread-local" pointers and
EfiConvertPointer() on each.  (As long as it fixes the problem of
course; I'll test and report back.)

It doesn't :(  It just gets slightly further and hits another static
pointer variable which is not part of the thread-local array:

...
 Pkcs7Verify
   EVP_add_digest
     OBJ_NAME_add

this one uses a few static pointer variables that are also initialized
on demand and become stale upon SetVirtualAddressMap().

So it looks like the issue can't be solved without making OpenSSL aware
of this use case.

Is reloading the module from scratch ruled out completely?

Not my place to say authoritatively, but:
- it would be a first, as much as I can say,
- it would duplicate (in purpose) an existing facility.

Personally I'd expect it to be rejected, but it's not up to me. If
you're willing to "build one to (possibly) throw away", that could be
the most direct way to get authoritative (= maintainer) feedback.


Laszlo,

I thunk it is more likely to get rejected as it would not work. 

Every runtime driver I've every seen usually works like this:
1) Loads as an EFI driver and uses EFI Boot Services in its constructor (gBS, gDS, AllocatePool(), etc.)
2) You use the EFI Boot Service to register the ExitBootServices Event. 
3) SetVirtualAddressMap event fires and converts all the pointers 
4) After all the ExitBootServices events have been processed the DXE Runtime driver re-relocates the Runtime Driver
5) The next code that runs is a call from the kernel virtual mapping, and the system is at runtime 

It is important to remember that when gRT->SetVirtualAddressMap() is called by the OS Loader (or early kernel) that gBS->ExitBootServices() has already been called. So by the time you get 3) almost all of EFI is gone. The only services that remain are gRT. Note you find the location of gRT by using the gST passed into the entry point of the driver. So here is lies the problem the entry point is passed a Handle (EFI Boot Services concept) and a pointer to gST (another boot services table). So you can't really reload a module and expect it to work. 

In EFI the transition from Boot Service time to Runtime already requires a lot of coding discipline to not call services at runtime that will go away after ExitBootServices. Having C code that can be called in Physical mode, and then with a virtual mapping is a very unnatural and what EFI does today is the minimum required to make things work. 

Also remember dumping more into EFI runtime is stealing memory and usually kernel virtual address space from the OS. 

Thanks,

Andrew Fish


Thanks
Laszlo

I'd try to cook up a patch for that unless there's a strong no-go.

Thanks,
Roman.
Re: [PATCH v2 1/1] MdePkg: Add STATIC_ASSERT macro

Vitaly Cheptsov
 

Michael, Liming, Laszlo,

Static assertions via _Static_assert are standard C11 functionality, thus any at least C11 (ISO/IEC 9899 2011) conforming compiler is required to support the second argument with the diagnostic description.
The notation without the message currently is only present in C++, not in C, thus the two-argument notation is the only allowed notation for _Static_assert for at least C17 (ISO/IEC 9899 2018) and below.
In the bottom of this message I included a quote from C17 for the relevant section (6.7.10).

GCC and CLANG (including Xcode) appear to be conforming to the standard for this section, and MSVC compiler static_assert extension also supports the diagnostic message argument. This is pretty much all we care about.

As for examples, I see little reason to clarify STATIC_ASSERT behaviour outside of the standard reference in its description and actual usage in the source code, but can do that just fine if you think that it may help somebody.

I fully agree that VERIFY_SIZE_OF usage should be converted to STATIC_ASSERT, and in fact I also suggest VERIFY_SIZE_OF to be entirely removed from Base.h. This should be fairly costless, as apparently it is only used in Base.h and MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c, which I can replace in the same patch set.

As for select ASSERT usage switching to STATIC_ASSERT, this would also be great, as let us be honest, the use of ASSERT in EDK II codebase is very questioning. In fact, this was one of the reasons we introduced our own static assertions some time ago. However, fixing up all broken assertions is unlikely a best place to fit into this patchset, but I can surely add a few examples, in case somebody points them out. This will be useful for reference purposes and may help the maintainers to get a better idea when static assertions are to be used.

Looking forward to hearing your opinions.

Best regards,
Vitaly


6.7.10 Static assertions

Syntax
1 static_assert-declaration:
_Static_assert ( constant-expression , string-literal ) ;

Constraints
2 The constant expression shall compare unequal to 0.

Semantics
3 The constant expression shall be an integer constant expression. If the value of the constant expression compares unequal to 0, the declaration has no effect. Otherwise, the constraint is violated and the implementation shall produce a diagnostic message that includes the text of the string literal, except that characters not in the basic source character set are not required to appear in the message.
Forward references: diagnostics (7.2).

7.2 Diagnostics <assert. h>

3 The macro
static_assert
expands to _Static_assert.

14 авг. 2019 г., в 18:47, Kinney, Michael D <michael.d.kinney@...> написал(а):


Liming,

I think a good candidate to demonstrate this
feature are the checks made in MdePkg/Include/Base.h.
The current implementation forces a divide by 0
in the C pre-processor to break the build.
STATIC_ASSERT() would be a better way to do this.
I would also remove unused externs from the builds.

/**
Verifies the storage size of a given data type.

This macro generates a divide by zero error or a zero size array declaration in
the preprocessor if the size is incorrect. These are declared as "extern" so
the space for these arrays will not be in the modules.

@param TYPE The date type to determine the size of.
@param Size The expected size for the TYPE.

**/
#define VERIFY_SIZE_OF(TYPE, Size) extern UINT8 _VerifySizeof##TYPE[(sizeof(TYPE) == (Size)) / (sizeof(TYPE) == (Size))]

//
// Verify that ProcessorBind.h produced UEFI Data Types that are compliant with
// Section 2.3.1 of the UEFI 2.3 Specification.
//
VERIFY_SIZE_OF (BOOLEAN, 1);
VERIFY_SIZE_OF (INT8, 1);
VERIFY_SIZE_OF (UINT8, 1);
VERIFY_SIZE_OF (INT16, 2);
VERIFY_SIZE_OF (UINT16, 2);
VERIFY_SIZE_OF (INT32, 4);
VERIFY_SIZE_OF (UINT32, 4);
VERIFY_SIZE_OF (INT64, 8);
VERIFY_SIZE_OF (UINT64, 8);
VERIFY_SIZE_OF (CHAR8, 1);
VERIFY_SIZE_OF (CHAR16, 2);

//
// The following three enum types are used to verify that the compiler
// configuration for enum types is compliant with Section 2.3.1 of the
// UEFI 2.3 Specification. These enum types and enum values are not
// intended to be used. A prefix of '__' is used avoid conflicts with
// other types.
//
typedef enum {
__VerifyUint8EnumValue = 0xff
} __VERIFY_UINT8_ENUM_SIZE;

typedef enum {
__VerifyUint16EnumValue = 0xffff
} __VERIFY_UINT16_ENUM_SIZE;

typedef enum {
__VerifyUint32EnumValue = 0xffffffff
} __VERIFY_UINT32_ENUM_SIZE;

VERIFY_SIZE_OF (__VERIFY_UINT8_ENUM_SIZE, 4);
VERIFY_SIZE_OF (__VERIFY_UINT16_ENUM_SIZE, 4);
VERIFY_SIZE_OF (__VERIFY_UINT32_ENUM_SIZE, 4);

A couple examples. Do all the compilers support the message parameter too?

STATIC_ASSERT (sizeof (BOOLEAN) == 1, "sizeof (BOOLEAN) does not meet UEFI Specification Data Type requirements")
STATIC_ASSERT (sizeof (UINT16) == 2, "sizeof (UINT16) does not meet UEFI Specification Data Type requirements")
STATIC_ASSERT (sizeof (INT32) == 4, "sizeof (INT32) does not meet UEFI Specification Data Type requirements")
STATIC_ASSERT (sizeof (CHAR16) == 2, "sizeof (CHAR16) does not meet UEFI Specification Data Type requirements")
STATIC_ASSERT (sizeof (__VERIFY_UINT8_ENUM_SIZE) == 4, "Size of enum does not meet UEFI Specification Data Type requirements")
STATIC_ASSERT (sizeof (__VERIFY_UINT32_ENUM_SIZE) == 4, "Size of enum does not meet UEFI Specification Data Type requirements")

Thanks,

Mike

-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io]
On Behalf Of Liming Gao
Sent: Wednesday, August 14, 2019 6:50 AM
To: devel@edk2.groups.io; vit9696@...
Subject: Re: [edk2-devel] [PATCH v2 1/1] MdePkg: Add
STATIC_ASSERT macro

Can you add the sample usage of new macro STATIC_ASSERT?

Or, give the link of static_assert or _Static_assert.

If so, the developer knows how to use them in source
code.

Thanks
Liming
-----Original Message-----
From: devel@edk2.groups.io
[mailto:devel@edk2.groups.io] On Behalf Of
vit9696 via Groups.Io
Sent: Tuesday, August 13, 2019 4:17 PM
To: devel@edk2.groups.io
Subject: [edk2-devel] [PATCH v2 1/1] MdePkg: Add
STATIC_ASSERT macro

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

Provide a macro for compile time assertions.
Equivalent to C11 static_assert macro from assert.h.

Signed-off-by: Vitaly Cheptsov
<vit9696@...>
---
MdePkg/Include/Base.h | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/MdePkg/Include/Base.h
b/MdePkg/Include/Base.h index
ce20b5f01dce..f85f7028a262 100644
--- a/MdePkg/Include/Base.h
+++ b/MdePkg/Include/Base.h
@@ -843,6 +843,17 @@ typedef UINTN *BASE_LIST;
#define
OFFSET_OF(TYPE, Field) ((UINTN) &(((TYPE *)0)->Field))
#endif

+///
+/// Portable definition for compile time assertions.
+/// Equivalent to C11 static_assert macro from
assert.h.
+/// Takes condtion and error message as its
arguments.
+///
+#ifdef _MSC_EXTENSIONS
+ #define STATIC_ASSERT static_assert #else
+ #define STATIC_ASSERT _Static_assert #endif
+
/**
Macro that returns a pointer to the data structure
that contains a specified field of
that data structure. This is a lightweight method
to hide
information by placing a
--
2.20.1 (Apple Git-117)


-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this
group.

View/Reply Online (#45503):
https://edk2.groups.io/g/devel/message/45503
Mute This Topic: https://groups.io/mt/32850582/1759384
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub
[liming.gao@...] -=-=-=-=-=-=

?
signature.asc

Re: [RFC] BZ 1837 Enable Windows Firmware Update Driver Tool in Edk2/BaseTools for 201908 stable tag

Eric Jin
 

Hi Leif,

-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
Leif Lindholm
Sent: Wednesday, August 14, 2019 2:47 AM
To: Gao, Liming <liming.gao@...>
Cc: Jin, Eric <eric.jin@...>; rfc@edk2.groups.io; devel@edk2.groups.io;
Feng, Bob C <bob.c.feng@...>; Cetola, Stephano
<stephano.cetola@...>; Laszlo Ersek (lersek@...)
<lersek@...>; afish@...; Kinney, Michael D
<michael.d.kinney@...>
Subject: Re: [edk2-devel] [RFC] BZ 1837 Enable Windows Firmware Update
Driver Tool in Edk2/BaseTools for 201908 stable tag

Hi Liming,

This is the other one. As I said, the fact that we are slipping multiple scripts in
*just* before freeze is a concern for me.
I realize it is my fault that I don't notify the release plan as early as possible even the BZ1837 is created in May and want to catch 201908 stable tag.


I have no objection to the code here though.
Thanks.


I would however request that Sean is set as author on patch 1/2 as he was
the original author of the script. (This was easy for me to find out because
the commit message was exemplary.)
In the patch series V2, Sean has been set as author on patch 1/2. Thank you for this valuable suggestion.

Best Regards
Eric


Best Regards,

Leif

On Tue, Aug 13, 2019 at 01:49:24PM +0000, Gao, Liming wrote:
I see this patch was sent a week ago. This is a standalone tool. There is no
impact on normal build and boot. I am OK to add it for 201908 stable tag.

Thanks
Liming
From: Jin, Eric
Sent: Monday, August 12, 2019 3:09 PM
To: rfc@edk2.groups.io
Cc: Gao, Liming <liming.gao@...>; Jin, Eric
<eric.jin@...>; devel@edk2.groups.io; Feng, Bob C
<bob.c.feng@...>
Subject: [RFC] BZ 1837 Enable Windows Firmware Update Driver Tool in
Edk2/BaseTools for 201908 stable tag

Hi All,

It is the request to Enable Windows Firmware Update Driver Tool in
Edk2/BaseTools and catch the Q3 tag.
The new tool will leverage the edk2-pytool-library to generate the cat/inf
file based on the cap file. The output driver package can be trigged in
Windows OS to complete capsule update.

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

Patch link: https://edk2.groups.io/g/devel/topic/32780378#44992

Best Regards
Eric
Re: [PATCH v2] BaseTools/Scripts: Add GetUtcDateTime script.

Chiu, Chasel
 

Yes, as pylama currently not requirement yet we will evaluate this later.

Thanks!

Regards,
Chasel

-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
rebecca@...
Sent: Wednesday, August 14, 2019 7:51 PM
To: devel@edk2.groups.io; Chiu, Chasel <chasel.chiu@...>
Cc: Feng, Bob C <bob.c.feng@...>; Gao, Liming <liming.gao@...>;
Leif Lindholm <leif.lindholm@...>
Subject: Re: [edk2-devel] [PATCH v2] BaseTools/Scripts: Add GetUtcDateTime
script.

On 2019-08-14 04:21, Chiu, Chasel wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2067

A script that can return UTC date and time in ascii format which is
convenient for patching build time information in any binary.

I know it's not a required tool to be run before committing, but could
you consider the following issues pylama reported, please?

BaseTools/Scripts/GetUtcDateTime.py:1:1: E266 too many leading '#' for
block comment [pycodestyle]
BaseTools/Scripts/GetUtcDateTime.py:10:1: E402 module level import not
at top of file [pycodestyle]
BaseTools/Scripts/GetUtcDateTime.py:11:1: E402 module level import not
at top of file [pycodestyle]
BaseTools/Scripts/GetUtcDateTime.py:12:1: E402 module level import not
at top of file [pycodestyle]
BaseTools/Scripts/GetUtcDateTime.py:14:1: E302 expected 2 blank lines,
found 1 [pycodestyle]
BaseTools/Scripts/GetUtcDateTime.py:29:14: E211 whitespace before '('
[pycodestyle]
BaseTools/Scripts/GetUtcDateTime.py:35:14: E211 whitespace before '('
[pycodestyle]
BaseTools/Scripts/GetUtcDateTime.py:38:14: E211 whitespace before '('
[pycodestyle]
BaseTools/Scripts/GetUtcDateTime.py:41:14: E211 whitespace before '('
[pycodestyle]
BaseTools/Scripts/GetUtcDateTime.py:43:1: E305 expected 2 blank lines
after class or function definition, found 1 [pycodestyle]

--
Rebecca Cran
Re: [Patch] MdeModulePkg DxeCore: Fix for missing MAT update

Liming Gao
 

Laszlo:

-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Laszlo Ersek
Sent: Wednesday, August 14, 2019 11:13 PM
To: Gao, Liming <liming.gao@...>; Kinney, Michael D <michael.d.kinney@...>; devel@edk2.groups.io
Cc: Mike Turner <miketur@...>; Wang, Jian J <jian.j.wang@...>; Wu, Hao A <hao.a.wu@...>; Bi, Dandan
<dandan.bi@...>
Subject: Re: [edk2-devel] [Patch] MdeModulePkg DxeCore: Fix for missing MAT update

On 08/14/19 16:00, Gao, Liming wrote:
Laszlo:

I cherry pick this patch from Mu project with the minimal change.
I can update the comment style.
Yes, please. Thanks!

The gEfiMemoryTypeInformationGuid HOB is supposed to be built -- if it
is built at all -- no later than in the DXE IPL PEIM (if VariablePei is
included in the platform, and the underlying UEFI variable exists). Is
that correct?
gEfiMemoryTypeInformationGuid HOB is installed by platform PEI.
Yes, that's what I meant by "no later than in the DXE IPL PEIM".

If the platform PEI doesn't install this HOB, it means this feature is disabled.
PlatformPei is supposed to build the HOB in the first place, yes.

However, if it doesn't, then there still may be a feedback loop between
the DXE IPL PEIM and BDS. The former builds the HOB from the UEFI
variable, and the latter updates the variable (as I understand) for
future boots.
UEFI variable is created by BDS only when HOB can be found.
If Platform PEIM doesn't build HOB, DxeIpl will not build HOB,
then BDS will not create UEFI variable. If so, there is no HOB in
every boot.

Thanks
Liming
Thanks
Laszlo
Re: [PATCH v2 1/1] MdePkg: Add STATIC_ASSERT macro

Michael D Kinney
 

Liming,

I think a good candidate to demonstrate this
feature are the checks made in MdePkg/Include/Base.h.
The current implementation forces a divide by 0
in the C pre-processor to break the build.
STATIC_ASSERT() would be a better way to do this.
I would also remove unused externs from the builds.

/**
Verifies the storage size of a given data type.

This macro generates a divide by zero error or a zero size array declaration in
the preprocessor if the size is incorrect. These are declared as "extern" so
the space for these arrays will not be in the modules.

@param TYPE The date type to determine the size of.
@param Size The expected size for the TYPE.

**/
#define VERIFY_SIZE_OF(TYPE, Size) extern UINT8 _VerifySizeof##TYPE[(sizeof(TYPE) == (Size)) / (sizeof(TYPE) == (Size))]

//
// Verify that ProcessorBind.h produced UEFI Data Types that are compliant with
// Section 2.3.1 of the UEFI 2.3 Specification.
//
VERIFY_SIZE_OF (BOOLEAN, 1);
VERIFY_SIZE_OF (INT8, 1);
VERIFY_SIZE_OF (UINT8, 1);
VERIFY_SIZE_OF (INT16, 2);
VERIFY_SIZE_OF (UINT16, 2);
VERIFY_SIZE_OF (INT32, 4);
VERIFY_SIZE_OF (UINT32, 4);
VERIFY_SIZE_OF (INT64, 8);
VERIFY_SIZE_OF (UINT64, 8);
VERIFY_SIZE_OF (CHAR8, 1);
VERIFY_SIZE_OF (CHAR16, 2);

//
// The following three enum types are used to verify that the compiler
// configuration for enum types is compliant with Section 2.3.1 of the
// UEFI 2.3 Specification. These enum types and enum values are not
// intended to be used. A prefix of '__' is used avoid conflicts with
// other types.
//
typedef enum {
__VerifyUint8EnumValue = 0xff
} __VERIFY_UINT8_ENUM_SIZE;

typedef enum {
__VerifyUint16EnumValue = 0xffff
} __VERIFY_UINT16_ENUM_SIZE;

typedef enum {
__VerifyUint32EnumValue = 0xffffffff
} __VERIFY_UINT32_ENUM_SIZE;

VERIFY_SIZE_OF (__VERIFY_UINT8_ENUM_SIZE, 4);
VERIFY_SIZE_OF (__VERIFY_UINT16_ENUM_SIZE, 4);
VERIFY_SIZE_OF (__VERIFY_UINT32_ENUM_SIZE, 4);

A couple examples. Do all the compilers support the message parameter too?

STATIC_ASSERT (sizeof (BOOLEAN) == 1, "sizeof (BOOLEAN) does not meet UEFI Specification Data Type requirements")
STATIC_ASSERT (sizeof (UINT16) == 2, "sizeof (UINT16) does not meet UEFI Specification Data Type requirements")
STATIC_ASSERT (sizeof (INT32) == 4, "sizeof (INT32) does not meet UEFI Specification Data Type requirements")
STATIC_ASSERT (sizeof (CHAR16) == 2, "sizeof (CHAR16) does not meet UEFI Specification Data Type requirements")
STATIC_ASSERT (sizeof (__VERIFY_UINT8_ENUM_SIZE) == 4, "Size of enum does not meet UEFI Specification Data Type requirements")
STATIC_ASSERT (sizeof (__VERIFY_UINT32_ENUM_SIZE) == 4, "Size of enum does not meet UEFI Specification Data Type requirements")

Thanks,

Mike

-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io]
On Behalf Of Liming Gao
Sent: Wednesday, August 14, 2019 6:50 AM
To: devel@edk2.groups.io; vit9696@...
Subject: Re: [edk2-devel] [PATCH v2 1/1] MdePkg: Add
STATIC_ASSERT macro

Can you add the sample usage of new macro STATIC_ASSERT?

Or, give the link of static_assert or _Static_assert.

If so, the developer knows how to use them in source
code.

Thanks
Liming
-----Original Message-----
From: devel@edk2.groups.io
[mailto:devel@edk2.groups.io] On Behalf Of
vit9696 via Groups.Io
Sent: Tuesday, August 13, 2019 4:17 PM
To: devel@edk2.groups.io
Subject: [edk2-devel] [PATCH v2 1/1] MdePkg: Add
STATIC_ASSERT macro

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

Provide a macro for compile time assertions.
Equivalent to C11 static_assert macro from assert.h.

Signed-off-by: Vitaly Cheptsov
<vit9696@...>
---
MdePkg/Include/Base.h | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/MdePkg/Include/Base.h
b/MdePkg/Include/Base.h index
ce20b5f01dce..f85f7028a262 100644
--- a/MdePkg/Include/Base.h
+++ b/MdePkg/Include/Base.h
@@ -843,6 +843,17 @@ typedef UINTN *BASE_LIST;
#define
OFFSET_OF(TYPE, Field) ((UINTN) &(((TYPE *)0)->Field))
#endif

+///
+/// Portable definition for compile time assertions.
+/// Equivalent to C11 static_assert macro from
assert.h.
+/// Takes condtion and error message as its
arguments.
+///
+#ifdef _MSC_EXTENSIONS
+ #define STATIC_ASSERT static_assert #else
+ #define STATIC_ASSERT _Static_assert #endif
+
/**
Macro that returns a pointer to the data structure
that contains a specified field of
that data structure. This is a lightweight method
to hide
information by placing a
--
2.20.1 (Apple Git-117)


-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this
group.

View/Reply Online (#45503):
https://edk2.groups.io/g/devel/message/45503
Mute This Topic: https://groups.io/mt/32850582/1759384
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub
[liming.gao@...] -=-=-=-=-=-=
Re: [PATCH] MdePkg: Add STATIC_ASSERT macro

Laszlo Ersek
 

(adding back the list)

On 08/13/19 16:30, vit9696@... wrote:
Laszlo,

GCC implemented _Static_assert in 4.6. See its official wiki for C11 compatibility documentation:

https://gcc.gnu.org/wiki/C11Status
Awesome news!

Once this is upstreamed, we should go through all the ASSERT()s in the
code, and replace those that are compile-time checkable with
STATIC_ASSERT(). I know that I've added quite a few ASSERT()s that I
wished would be checked at build time.

Of course, I'll probably never have time for that. :/

Thanks
Laszlo
Re: [PATCH v1 0/1] Added GOP driver for DisplayLink-based Universal USB Docks to edk2-platforms

Michael D Kinney
 

Hi Andy,

 

Thanks for the contribution.  Is this patch for the edk2-platform repo?  The email subject can help clarify the intended repo.

 

Also, we prefer the patches to be provided inline instead of an attachment.  You can use the

‘git send-email’ feature to do this.  Here are a couple links to useful wikis.

 

https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Development-Process

https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers

 

I also noticed looking at the patch file that there are some inconsistencies with the

EDK II C Coding Standards.  Most are around function headers and indents.  Here is a link

to the latest version:

 

https://www.gitbook.com/download/pdf/book/edk2-docs/edk-ii-c-coding-standards-specification

 

I see a few #if 0 and commented out lines of code and TODO comments.  Can you please clean these up

and if needed add Tianocore Bugzillas for future enhancements/work items for this driver.

 

https://bugzilla.tianocore.org/

 

I also noticed that you are using named fields to initialize C structures:

 

STATIC CONST struct VideoMode ModeData[] = {

+  {

+     // 640 x 480 @ 60Hz

+    .Reserved2 = 2,

+    .PixelClock = 2517,

+    .HActive = 640,

+    .HBlanking = 160,

+    .HSyncOffset = 16,

+    .HSyncWidth = 96,

 

I do like this style because it has better self documentation of the field being

initialized to a value.  However, I do not see this style in the rest of the EDK II

sources, so I am concerned that we may have required compilers for the EDK II

that may not support this syntax.

 

If we find all the supported compilers do support this syntax, this would be

a great addition to the EDK II C Coding Standards.

 

I am curious how this driver interacts with the ConSplitter when displays

are hot added and hot removed.  I see a reference to a feature that appears

to sync the GOP content between frame buffers when a display is hot added.

I would be better if the common code in the ConSplitter handled these types

of operations instead of putting that code in individual GOP drivers. I have

added Ray to this thread who may have ideas on how to handle these hot

add events.

 

Thanks,

 

Mike

 

From: Andy Hayes [mailto:andy.hayes@...]
Sent: Wednesday, August 14, 2019 7:44 AM
To: devel@edk2.groups.io
Cc: Leif Lindholm <leif.lindholm@...>; Kinney, Michael D <michael.d.kinney@...>
Subject: [PATCH v1 0/1] Added GOP driver for DisplayLink-based Universal USB Docks to edk2-platforms

 

From 4a42eb997aeb3699217b40bf3bc47dec56458847 Mon Sep 17 00:00:00 2001

From: "Andy Hayes" < andy.hayes@... >

Date: Wed, 14 Aug 2019 15:30:20 +0100

Subject: [PATCH v1 0/1] Added GOP graphics driver for DisplayLink-based Universal USB Docks to edk2-platforms

 

andy.hayes@... (1):

  Added GOP driver for USB Docks which use DisplayLink chips.

 

.../DisplayLinkPkg/DisplayLinkPkg.dsc         |   61 +

.../DisplayLinkGop/DisplayLinkGopDxe.inf      |   63 +

.../DisplayLinkPkg/DisplayLinkGop/Edid.h      |  129 ++

.../DisplayLinkGop/UsbDescriptors.h           |  109 ++

.../DisplayLinkGop/UsbDisplayLink.h           |  284 +++++

.../DisplayLinkGop/CapabilityDescriptor.c     |  137 ++

.../DisplayLinkGop/ComponentName.c            |  235 ++++

.../DisplayLinkPkg/DisplayLinkGop/Edid.c      |  598 +++++++++

.../DisplayLinkPkg/DisplayLinkGop/Gop.c       |  587 +++++++++

.../DisplayLinkGop/UsbDescriptors.c           |  144 +++

.../DisplayLinkGop/UsbDisplayLink.c           | 1109 +++++++++++++++++

.../DisplayLinkGop/UsbTransfer.c              |  180 +++

.../DisplayLinkGop/VideoModes.c               |  254 ++++

Drivers/DisplayLink/DisplayLinkPkg/ReadMe.md  |   77 ++

Maintainers.txt                               |    5 +

15 files changed, 3972 insertions(+)

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkPkg.dsc

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/DisplayLinkGopDxe.inf

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/Edid.h

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbDescriptors.h

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbDisplayLink.h

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/CapabilityDescriptor.c

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/ComponentName.c

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/Edid.c

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/Gop.c

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbDescriptors.c

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbDisplayLink.c

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/VideoModes.c

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/ReadMe.md

 

--

2.17.1
Re: static data in dxe_runtime modules

Laszlo Ersek
 

On 08/13/19 13:23, Roman Kagan wrote:
On Tue, Aug 13, 2019 at 11:10:27AM +0200, Laszlo Ersek wrote:
On 08/12/19 20:43, Roman Kagan wrote:
On Fri, Aug 09, 2019 at 04:07:00PM +0000, Roman Kagan via Groups.Io wrote:
On Thu, Aug 08, 2019 at 07:39:14PM +0200, Laszlo Ersek wrote:
On 08/07/19 19:41, Andrew Fish wrote:
On Aug 7, 2019, at 10:29 AM, Laszlo Ersek <lersek@...> wrote:
On 08/05/19 12:18, Roman Kagan wrote:
On Sat, Aug 03, 2019 at 04:03:04AM +0200, Laszlo Ersek via Groups.Io wrote:
On 08/01/19 21:16, Roman Kagan wrote:
I'm convinced that OpenSSL needs to expose a new API for this particular
problem.
Since, as you point out below, the problem only affects the essentially
broken configuration (SECURE_BOOT_ENABLE && !SMM_REQUIRE), I'm fine with
saving time and effort and sticking to the hack-ish approach proposed in
the bugzilla issue, which is to iterate over "thread-local" pointers and
EfiConvertPointer() on each. (As long as it fixes the problem of
course; I'll test and report back.)
It doesn't :( It just gets slightly further and hits another static
pointer variable which is not part of the thread-local array:

...
Pkcs7Verify
EVP_add_digest
OBJ_NAME_add

this one uses a few static pointer variables that are also initialized
on demand and become stale upon SetVirtualAddressMap().
So it looks like the issue can't be solved without making OpenSSL aware
of this use case.
Is reloading the module from scratch ruled out completely?
Not my place to say authoritatively, but:
- it would be a first, as much as I can say,
- it would duplicate (in purpose) an existing facility.

Personally I'd expect it to be rejected, but it's not up to me. If
you're willing to "build one to (possibly) throw away", that could be
the most direct way to get authoritative (= maintainer) feedback.

Thanks
Laszlo

I'd try to cook up a patch for that unless there's a strong no-go.

Thanks,
Roman.
Re: [Patch] MdeModulePkg DxeCore: Fix for missing MAT update

Laszlo Ersek
 

On 08/14/19 16:00, Gao, Liming wrote:
Laszlo:

I cherry pick this patch from Mu project with the minimal change.
I can update the comment style.
Yes, please. Thanks!

The gEfiMemoryTypeInformationGuid HOB is supposed to be built -- if it
is built at all -- no later than in the DXE IPL PEIM (if VariablePei is
included in the platform, and the underlying UEFI variable exists). Is
that correct?
gEfiMemoryTypeInformationGuid HOB is installed by platform PEI.
Yes, that's what I meant by "no later than in the DXE IPL PEIM".

If the platform PEI doesn't install this HOB, it means this feature is disabled.
PlatformPei is supposed to build the HOB in the first place, yes.

However, if it doesn't, then there still may be a feedback loop between
the DXE IPL PEIM and BDS. The former builds the HOB from the UEFI
variable, and the latter updates the variable (as I understand) for
future boots.

Thanks
Laszlo
Re: [PATCH V2] ShellPkg/UefiShellDriver1CommandsLib: Make array big enough

Carsey, Jaben
 

Reviewed-by: Jaben Carsey <jaben.carsey@...>

Thanks
-Jaben

-----Original Message-----
From: Augustine, Linson
Sent: Wednesday, August 14, 2019 1:41 AM
To: Gao, Zhichao <zhichao.gao@...>; devel@edk2.groups.io
Cc: Carsey, Jaben <jaben.carsey@...>; Ni, Ray <ray.ni@...>;
oleksiyy@...
Subject: RE: [edk2-devel] [PATCH V2]
ShellPkg/UefiShellDriver1CommandsLib: Make array big enough

Reviewed-by: Linson Augustine <Linson.augustine@...>

Regards,
-Linson

-----Original Message-----
From: Gao, Zhichao
Sent: Wednesday, August 14, 2019 2:03 PM
To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@...>
Cc: Carsey, Jaben <jaben.carsey@...>; Ni, Ray <ray.ni@...>;
oleksiyy@...; Augustine, Linson <linson.augustine@...>
Subject: RE: [edk2-devel] [PATCH V2]
ShellPkg/UefiShellDriver1CommandsLib: Make array big enough

Ping again.

Thanks,
Zhichao

-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
Gao, Zhichao
Sent: Friday, July 26, 2019 3:47 PM
To: devel@edk2.groups.io
Cc: Carsey, Jaben <jaben.carsey@...>; Ni, Ray
<ray.ni@...>; oleksiyy@...
Subject: FW: [edk2-devel] [PATCH V2]
ShellPkg/UefiShellDriver1CommandsLib: Make array big enough

Ping. Please help to review it.

Thanks,
Zhichao

-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
Gao, Zhichao
Sent: Monday, July 22, 2019 2:58 PM
To: devel@edk2.groups.io
Cc: Carsey, Jaben <jaben.carsey@...>; Ni, Ray
<ray.ni@...>; Oleksiy <oleksiyy@...>
Subject: [edk2-devel] [PATCH V2] ShellPkg/UefiShellDriver1CommandsLib:
Make array big enough

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

The two CHAR16 array ChildCountStr and DeviceCountStr is defined to
hold the decimal string data of UINTN. The max of UINTN is
18446744073709551615 and it contain 20 characters.
So make their size to 21 CHAR16s to hold the string data with a null-
terminate.
UnicodeValueToStringS regard the value input as INT64, and
21 CHARs is enough to hold the lowest value with minus '-'.
Although the value shouldn't be such big.

Cc: Jaben Carsey <jaben.carsey@...>
Cc: Ray Ni <ray.ni@...>
Cc: Oleksiy <oleksiyy@...>
Signed-off-by: Zhichao Gao <zhichao.gao@...>
---

V2:
Update the copyright.

ShellPkg/Library/UefiShellDriver1CommandsLib/Drivers.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/Drivers.c
b/ShellPkg/Library/UefiShellDriver1CommandsLib/Drivers.c
index 794b737bd1..27cd278cf0 100644
--- a/ShellPkg/Library/UefiShellDriver1CommandsLib/Drivers.c
+++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/Drivers.c
@@ -2,7 +2,7 @@
Main file for Drivers shell Driver1 function.

(C) Copyright 2012-2015 Hewlett-Packard Development Company,
L.P.<BR>
- Copyright (c) 2010 - 2018, Intel Corporation. All rights
reserved.<BR>
+ Copyright (c) 2010 - 2019, Intel Corporation. All rights
+ reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -263,8 +263,8 @@ ShellCommandRunDrivers (
EFI_HANDLE *HandleWalker;
UINTN ChildCount;
UINTN DeviceCount;
- CHAR16 ChildCountStr[3];
- CHAR16 DeviceCountStr[3];
+ CHAR16 ChildCountStr[21];
+ CHAR16 DeviceCountStr[21];
CHAR16 *Temp2;
CONST CHAR16 *FullDriverName;
CHAR16 *TruncatedDriverName;
--
2.21.0.windows.1
Re: [PATCH 1/1] ShellPkg/UefiShellAcpiViewCommandLib: Remove the variable "Index"

Carsey, Jaben
 

Reviewed-by: Jaben Carsey <jaben.carsey@...>

Thanks
-Jaben

-----Original Message-----
From: Gao, Zhichao
Sent: Tuesday, August 13, 2019 8:37 PM
To: Zhang, Shenglei <shenglei.zhang@...>; devel@edk2.groups.io
Cc: Carsey, Jaben <jaben.carsey@...>; Ni, Ray <ray.ni@...>
Subject: RE: [PATCH 1/1] ShellPkg/UefiShellAcpiViewCommandLib: Remove
the variable "Index"

Reviewed-by: Zhichao Gao <zhichao.gao@...>

Thanks,
Zhichao

-----Original Message-----
From: Zhang, Shenglei
Sent: Wednesday, August 14, 2019 11:07 AM
To: devel@edk2.groups.io
Cc: Carsey, Jaben <jaben.carsey@...>; Ni, Ray <ray.ni@...>;
Gao, Zhichao <zhichao.gao@...>
Subject: [PATCH 1/1] ShellPkg/UefiShellAcpiViewCommandLib: Remove
the
variable "Index"

In IortParser.c ,the variable Index is set but not used in function
DumpIortNodeNamedComponent. This will cause build failure when
building
ShellPkg with GCC.

Cc: Jaben Carsey <jaben.carsey@...>
Cc: Ray Ni <ray.ni@...>
Cc: Zhichao Gao <zhichao.gao@...>
Signed-off-by: Shenglei Zhang <shenglei.zhang@...>
---
.../UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c | 2 --
1 file changed, 2 deletions(-)

diff --git
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
index 8912d415a755..f1cdb9ac01d8 100644
---
a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c
+++
b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortPars
+++ er.c
@@ -472,7 +472,6 @@ DumpIortNodeNamedComponent (
)
{
UINT32 Offset;
- UINT32 Index;

Offset = ParseAcpi (
TRUE,
@@ -485,7 +484,6 @@ DumpIortNodeNamedComponent (

// Estimate the Device Name length
PrintFieldName (2, L"Device Object Name");
- Index = 0;

while ((*(Ptr + Offset) != 0) &&
(Offset < Length)) {
--
2.18.0.windows.1
Re: Cancelled Event: TianoCore Design / Bug Triage - EMEA - Wednesday, 14 August 2019 #cal-cancelled

Stephano Cetola
 

No topics this week.

Also, I'll be at the Open Source Summit next week. Ray will likely run
the design meeting on Thursday, and unless anyone has any topics for
Wednesday, we'll probably cancel that one.

Cheers,
Stephano

On Wed, Aug 14, 2019 at 7:46 AM devel@edk2.groups.io Calendar
<devel@edk2.groups.io> wrote:

Cancelled: TianoCore Design / Bug Triage - EMEA

This event has been cancelled.

When:
Wednesday, 14 August 2019
8:00am to 9:00am
(UTC-07:00) America/Los Angeles

Where:
https://zoom.us/j/695893389

Organizer:
stephano.cetola@...

Description:

Join Zoom Meeting

https://zoom.us/j/695893389



One tap mobile

+17207072699,,695893389# US

+16465588656,,695893389# US (New York)



Dial by your location

+1 720 707 2699 US

+1 646 558 8656 US (New York)

Meeting ID: 695 893 389

Find your local number: https://zoom.us/u/abOtdJckxL
Cancelled Event: TianoCore Design / Bug Triage - EMEA - Wednesday, 14 August 2019 #cal-cancelled

devel@edk2.groups.io Calendar <devel@...>
 

Cancelled: TianoCore Design / Bug Triage - EMEA

This event has been cancelled.

When:
Wednesday, 14 August 2019
8:00am to 9:00am
(UTC-07:00) America/Los Angeles

Where:
https://zoom.us/j/695893389

Organizer:
stephano.cetola@...

Description:

Join Zoom Meeting

https://zoom.us/j/695893389

 

One tap mobile

+17207072699,,695893389# US

+16465588656,,695893389# US (New York)

 

Dial by your location

        +1 720 707 2699 US

        +1 646 558 8656 US (New York)

Meeting ID: 695 893 389

Find your local number: https://zoom.us/u/abOtdJckxL

 

Calendar
Calendar

[PATCH v1 0/1] Added GOP driver for DisplayLink-based Universal USB Docks to edk2-platforms

Andy Hayes
 

From 4a42eb997aeb3699217b40bf3bc47dec56458847 Mon Sep 17 00:00:00 2001

From: "Andy Hayes" < andy.hayes@... >

Date: Wed, 14 Aug 2019 15:30:20 +0100

Subject: [PATCH v1 0/1] Added GOP graphics driver for DisplayLink-based Universal USB Docks to edk2-platforms

 

andy.hayes@... (1):

  Added GOP driver for USB Docks which use DisplayLink chips.

 

.../DisplayLinkPkg/DisplayLinkPkg.dsc         |   61 +

.../DisplayLinkGop/DisplayLinkGopDxe.inf      |   63 +

.../DisplayLinkPkg/DisplayLinkGop/Edid.h      |  129 ++

.../DisplayLinkGop/UsbDescriptors.h           |  109 ++

.../DisplayLinkGop/UsbDisplayLink.h           |  284 +++++

.../DisplayLinkGop/CapabilityDescriptor.c     |  137 ++

.../DisplayLinkGop/ComponentName.c            |  235 ++++

.../DisplayLinkPkg/DisplayLinkGop/Edid.c      |  598 +++++++++

.../DisplayLinkPkg/DisplayLinkGop/Gop.c       |  587 +++++++++

.../DisplayLinkGop/UsbDescriptors.c           |  144 +++

.../DisplayLinkGop/UsbDisplayLink.c           | 1109 +++++++++++++++++

.../DisplayLinkGop/UsbTransfer.c              |  180 +++

.../DisplayLinkGop/VideoModes.c               |  254 ++++

Drivers/DisplayLink/DisplayLinkPkg/ReadMe.md  |   77 ++

Maintainers.txt                               |    5 +

15 files changed, 3972 insertions(+)

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkPkg.dsc

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/DisplayLinkGopDxe.inf

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/Edid.h

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbDescriptors.h

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbDisplayLink.h

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/CapabilityDescriptor.c

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/ComponentName.c

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/Edid.c

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/Gop.c

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbDescriptors.c

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbDisplayLink.c

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/UsbTransfer.c

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/DisplayLinkGop/VideoModes.c

create mode 100644 Drivers/DisplayLink/DisplayLinkPkg/ReadMe.md

 

--

2.17.1

?
0001-Added-GOP-driver-for-USB-Docks-using-DisplayLink-chi.patch

Re: CPU hotplug using SMM with QEMU+OVMF

Paolo Bonzini <pbonzini@...>
 

On 14/08/19 15:20, Yao, Jiewen wrote:
- Does this part require a new branch somewhere in the OVMF SEC code?
How do we determine whether the CPU executing SEC is BSP or
hot-plugged AP?
[Jiewen] I think this is blocked from hardware perspective, since the first instruction.
There are some hardware specific registers can be used to determine if the CPU is new added.
I don’t think this must be same as the real hardware.
You are free to invent some registers in device model to be used in OVMF hot plug driver.
Yes, this would be a new operation mode for QEMU, that only applies to
hot-plugged CPUs. In this mode the AP doesn't reply to INIT or SMI, in
fact it doesn't reply to anything at all.

- How do we tell the hot-plugged AP where to start execution? (I.e. that
it should execute code at a particular pflash location.)
[Jiewen] Same real mode reset vector at FFFF:FFF0.
You do not need a reset vector or INIT/SIPI/SIPI sequence at all in
QEMU. The AP does not start execution at all when it is unplugged, so
no cache-as-RAM etc.

We only need to modify QEMU so that hot-plugged APIs do not reply to
INIT/SIPI/SMI.

I don’t think there is problem for real hardware, who always has CAR.
Can QEMU provide some CPU specific space, such as MMIO region?
Why is a CPU-specific region needed if every other processor is in SMM
and thus trusted.

Does CPU hotplug apply only at the socket level? If the CPU is
multi-core, what is responsible for hot-plugging all cores present in
the socket?
I can answer this: the SMM handler would interact with the hotplug
controller in the same way that ACPI DSDT does normally. This supports
multiple hotplugs already.

Writes to the hotplug controller from outside SMM would be ignored.

(03) New CPU: (Flash) send board message to tell host CPU (GPIO->SCI)
-- I am waiting for hot-add message.
Maybe we can simplify this in QEMU by broadcasting an SMI to existent
processors immediately upon plugging the new CPU.
The QEMU DSDT could be modified (when secure boot is in effect) to OUT
to 0xB2 when hotplug happens. It could write a well-known value to
0xB2, to be read by an SMI handler in edk2.



(NOTE: Host CPU can only
send
instruction in SMM mode. -- The register is SMM only)
Sorry, I don't follow -- what register are we talking about here, and
why is the BSP needed to send anything at all? What "instruction" do you
have in mind?
[Jiewen] The new CPU does not enable SMI at reset.
At some point of time later, the CPU need enable SMI, right?
The "instruction" here means, the host CPUs need tell to CPU to enable SMI.
Right, this would be a write to the CPU hotplug controller

(04) Host CPU: (OS) get message from board that a new CPU is added.
(GPIO -> SCI)

(05) Host CPU: (OS) All CPUs enter SMM (SCI->SWSMI) (NOTE: New CPU
will not enter CPU because SMI is disabled)
I don't understand the OS involvement here. But, again, perhaps QEMU can
force all existent CPUs into SMM immediately upon adding the new CPU.
[Jiewen] OS here means the Host CPU running code in OS environment, not in SMM environment.
See above.

(06) Host CPU: (SMM) Save 38000, Update 38000 -- fill simple SMM
rebase code.

(07) Host CPU: (SMM) Send message to New CPU to Enable SMI.
Aha, so this is the SMM-only register you mention in step (03). Is the
register specified in the Intel SDM?
[Jiewen] Right. That is the register to let host CPU tell new CPU to enable SMI.
It is platform specific register. Not defined in SDM.
You may invent one in device model.
See above.

(10) New CPU: (SMM) Response first SMI at 38000, and rebase SMBASE to
TSEG.
What code does the new CPU execute after it completes step (10)? Does it
halt?
[Jiewen] The new CPU exits SMM and return to original place - where it is
interrupted to enter SMM - running code on the flash.
So in our case we'd need an INIT/SIPI/SIPI sequence between (06) and (07).

(11) Host CPU: (SMM) Restore 38000.
These steps (i.e., (06) through (11)) don't appear RAS-specific. The
only platform-specific feature seems to be SMI masking register, which
could be extracted into a new SmmCpuFeaturesLib API.

Thus, would you please consider open sourcing firmware code for steps
(06) through (11)?

Alternatively -- and in particular because the stack for step (01)
concerns me --, we could approach this from a high-level, functional
perspective. The states that really matter are the relocated SMBASE for
the new CPU, and the state of the full system, right at the end of step
(11).

When the SMM setup quiesces during normal firmware boot, OVMF could
use
existent (finalized) SMBASE infomation to *pre-program* some virtual
QEMU hardware, with such state that would be expected, as "final" state,
of any new hotplugged CPU. Afterwards, if / when the hotplug actually
happens, QEMU could blanket-apply this state to the new CPU, and
broadcast a hardware SMI to all CPUs except the new one.
I'd rather avoid this and stay as close as possible to real hardware.

Paolo
45301 - 45320 of 90922