Date   

回复: [PATCH v2 01/11] MdePkg, OvmfPkg: Clean up GHCB field offsets and save area

gaoliming
 

Tom:

-----邮件原件-----
发件人: Tom Lendacky <thomas.lendacky@...>
发送时间: 2020年10月17日 0:09
收件人: devel@edk2.groups.io
抄送: Brijesh Singh <brijesh.singh@...>; Michael D Kinney
<michael.d.kinney@...>; Liming Gao <gaoliming@...>;
Zhiguang Liu <zhiguang.liu@...>; Jordan Justen
<jordan.l.justen@...>; Laszlo Ersek <lersek@...>; Ard
Biesheuvel <ard.biesheuvel@...>
主题: [PATCH v2 01/11] MdePkg, OvmfPkg: Clean up GHCB field offsets and
save area

From: Tom Lendacky <thomas.lendacky@...>

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

Use OFFSET_OF () and sizeof () to calculate the GHCB register field
offsets instead of hardcoding the values in the GHCB_REGISTER enum.
Rename GHCB_REGISTER to GHCB_QWORD_OFFSET to more appropriately
describe
the enum. While redefining the values, only include (and add) fields that
are used per the GHCB specification.

Also, remove the DR7 field from the GHCB_SAVE_AREA structure since it is
not used/defined in the GHCB specification and then rename the reserved
fields as appropriate.

Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Brijesh Singh <brijesh.singh@...>
Signed-off-by: Tom Lendacky <thomas.lendacky@...>
---
MdePkg/Include/Register/Amd/Ghcb.h | 40
+++++++-------------
OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 4 +-
2 files changed, 16 insertions(+), 28 deletions(-)
Please separate the patch for the change in OvmfPkg.
One commit can't cross the different packages.
I understand this is the incompatible change. But, we still need to follow
this rule.

diff --git a/MdePkg/Include/Register/Amd/Ghcb.h
b/MdePkg/Include/Register/Amd/Ghcb.h
index 54a80da0f6d7..93fb6e3cb0fc 100644
--- a/MdePkg/Include/Register/Amd/Ghcb.h
+++ b/MdePkg/Include/Register/Amd/Ghcb.h
@@ -82,35 +82,10 @@
#define IOIO_SEG_DS (BIT11 | BIT10)


-typedef enum {
- GhcbCpl = 25,
- GhcbRflags = 46,
- GhcbRip,
- GhcbRsp = 59,
- GhcbRax = 63,
- GhcbRcx = 97,
- GhcbRdx,
- GhcbRbx,
- GhcbRbp = 101,
- GhcbRsi,
- GhcbRdi,
- GhcbR8,
- GhcbR9,
- GhcbR10,
- GhcbR11,
- GhcbR12,
- GhcbR13,
- GhcbR14,
- GhcbR15,
- GhcbXCr0 = 125,
-} GHCB_REGISTER;
-
typedef PACKED struct {
UINT8 Reserved1[203];
UINT8 Cpl;
- UINT8 Reserved2[148];
- UINT64 Dr7;
- UINT8 Reserved3[144];
+ UINT8 Reserved8[300];
UINT64 Rax;
UINT8 Reserved4[264];
UINT64 Rcx;
@@ -136,6 +111,19 @@ typedef PACKED struct {
UINT32 GhcbUsage;
} GHCB;

+typedef enum {
+ GhcbCpl = OFFSET_OF (GHCB, SaveArea.Cpl) / sizeof (UINT64),
OFFSET_OF (GHCB, SaveArea.Cpl) is 204. But, it can't fully divide 8
(sizeof(UINT64)). Is it correct?

Thanks
Liming
+ GhcbRax = OFFSET_OF (GHCB, SaveArea.Rax) / sizeof
(UINT64),
+ GhcbRbx = OFFSET_OF (GHCB, SaveArea.Rbx) / sizeof
(UINT64),
+ GhcbRcx = OFFSET_OF (GHCB, SaveArea.Rcx) / sizeof
(UINT64),
+ GhcbRdx = OFFSET_OF (GHCB, SaveArea.Rdx) / sizeof
(UINT64),
+ GhcbXCr0 = OFFSET_OF (GHCB, SaveArea.XCr0) / sizeof
(UINT64),
+ GhcbSwExitCode = OFFSET_OF (GHCB, SaveArea.SwExitCode) / sizeof
(UINT64),
+ GhcbSwExitInfo1 = OFFSET_OF (GHCB, SaveArea.SwExitInfo1) / sizeof
(UINT64),
+ GhcbSwExitInfo2 = OFFSET_OF (GHCB, SaveArea.SwExitInfo2) / sizeof
(UINT64),
+ GhcbSwScratch = OFFSET_OF (GHCB, SaveArea.SwScratch) / sizeof
(UINT64),
+} GHCB_QWORD_OFFSET;
+
typedef union {
struct {
UINT32 Lower32Bits;
diff --git a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
index 8e42b305e83c..c5484a3f478c 100644
--- a/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
+++ b/OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c
@@ -153,7 +153,7 @@ STATIC
BOOLEAN
GhcbIsRegValid (
IN GHCB *Ghcb,
- IN GHCB_REGISTER Reg
+ IN GHCB_QWORD_OFFSET Reg
)
{
UINT32 RegIndex;
@@ -179,7 +179,7 @@ STATIC
VOID
GhcbSetRegValid (
IN OUT GHCB *Ghcb,
- IN GHCB_REGISTER Reg
+ IN GHCB_QWORD_OFFSET Reg
)
{
UINT32 RegIndex;
--
2.28.0


回复: [edk2-devel] [PATCH v4 0/6] Extend Last Attempt Status Usage

gaoliming
 

Michael:
I have no other comments. Mike knows the more detail about FMP module.

For this patch set, Acked-by: Liming Gao <gaoliming@...>

Thanks
Liming

-----邮件原件-----
发件人: bounce+27952+66342+4905953+8761045@groups.io
<bounce+27952+66342+4905953+8761045@groups.io> 代表 Michael
Kubacki
发送时间: 2020年10月17日 3:46
收件人: Kinney, Michael D <michael.d.kinney@...>;
devel@edk2.groups.io
抄送: Liming Gao <gaoliming@...>; Jiang, Guomin
<guomin.jiang@...>; Xu, Wei6 <wei6.xu@...>; Liu, Zhiguang
<zhiguang.liu@...>
主题: Re: [edk2-devel] [PATCH v4 0/6] Extend Last Attempt Status Usage

I can definitely make those changes.

I haven't heard much from others (Liming, Wei, Zhiguang). If you could
please indicate your status on the patch series (use v5) with a reply of
either acceptance or suggestions that would be great before I send out
v6 with these updates.

Thanks,
Michael

On 10/16/2020 8:33 AM, Kinney, Michael D wrote:
Hi Michael,

Thank you for the updates. A couple minor comments:

1) FmpDevicePkg/Library/FmpDeviceLibNull

In order to demonstrate the preferred implementation I think we
should
have FmpDeviceCheckImage() call FmpDeviceCheckImageWithStatus()
and
FmpDeviceSetImage() call FmpDeviceSetImageWithStatus(). This
way, it
will be clear to FmpDeviceLib developers that they only need to
implement the *WithStatus() version.

2) FmpDevicePkg/PrivateInclude/FmpLastAttemptStatus.h

FmpDependencyCheckLib comment block should use /// instead of //
to be
consistent with the rest of the include file.

Thanks,

Mike


-----Original Message-----
From: Michael Kubacki <@michael.kubacki>
Sent: Thursday, October 15, 2020 2:20 PM
To: devel@edk2.groups.io
Cc: Liming Gao <gaoliming@...>; Kinney, Michael D
<michael.d.kinney@...>; Jiang, Guomin <guomin.jiang@...>;
Xu, Wei6 <wei6.xu@...>; Liu, Zhiguang <zhiguang.liu@...>
Subject: Re: [PATCH v4 0/6] Extend Last Attempt Status Usage

Hi all,

It's been about two weeks since this email and I still haven't seen any
feedback on v4.

I made a very small update that resulted in a v5 series today:
https://edk2.groups.io/g/devel/message/66287

If you could please provide timely feedback on v5 it would be appreciated.

Also, there's an issue building FmpDevicePkg at the moment that you'll
need this patch to fix:
https://edk2.groups.io/g/devel/message/66286

That patch needs review as well.

Thanks,
Michael

On 10/2/2020 9:26 AM, Michael Kubacki wrote:
It is going on a week now and I haven't seen a response to this patch
series yet. Please review it when possible.

On a somewhat related note, I made the changes that should be
necessary
in edk2-platforms for backward compatibility in this patch:
https://edk2.groups.io/g/devel/message/65821

Thanks,
Michael

On 9/25/2020 7:19 PM, @michael.kubacki wrote:
From: Michael Kubacki <michael.kubacki@...>

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

This patch series adds more granularity to Last Attempt Status
codes reported during FMP check image and set image operations
that greatly improve precision of the status codes.

The unsuccessful vendor range (0x1000 - 0x4000) was introduced
in UEFI Specification 2.8. At a high-level, two subranges are
defined within that range in this patch series:
1. The FMP Reserved range - reserved for components
implemented
in FmpDevicePkg.
2. The FMP Device Library Reserved range - reserved for
FmpDeviceLib instance-specific usage.

The ranges are described in a public header file LastAttemptStatus.h
while the specific codes used within FmpDevicePkg implementation
are defined in a private header file FmpLastAttemptStatus.h.

FmpDeviceLib instances should use the range definition from the
public header file to define Last Attempt Status codes local to
their library instance.

Of note, there's multiple approaches to assigning private status
codes in the FMP Reserved range. For example, individual components
could define their last attempt status codes locally with the
range allocated to the component defined in a package-wide private
header file. However, one goal of the granularity being introduced
is to provide straightforward traceability to an error source.

For that reason, it was chosen to define a constant set of codes at
the package level in FmpLastAttemptStatus.h. For example, if a new
FmpDependencyLib instance is added, it would not be able to reassign
status code values in the pre-existing FMP Dependency range; it
would reuse codes for the same error source and be able to add new
codes onto the range for its usage.

V4 changes:
1. Simplified range value definitions in LastAttemptStatus.h.
Directly assign the values in the macro definition instead
of using calculations.
2. Adjusted range sizes to leave more room for future expansion.

OLD:
START | END | Usage
------------------------------------------------|
0x1000 | 0x1FFF |
FmpDevicePkg |
0x1000 | 0x107F | FmpDxe driver |
0x1080 | 0x109F | FMP dependency Libs |
0x2000 | 0x3FFF | FmpDeviceLib instances |

NEW:
START | END | Usage
----------------------------------------------------------------|
0x1000 | 0x17FF |
FmpDevicePkg |
0x1000 | 0x107F | FmpDxe
driver |
0x1080 | 0x109F |
FmpDependencyLib |
0x10A0 | 0x10BF |
FmpDependencyCheckLib |
0x10C0 | 0x17FF | Unused. Available for future
expansion. |
0x1800 | 0x1FFF | FmpDeviceLib instances
implementation |
0x2000 | 0x3FFF | Unused. Available for future
expansion. |

3. Broke the single range in v3 for FMP Dependency libraries into
separate ranges.
4. Clarified LastAttemptStatus return values in each function
description.
5. Returned an expected LastAttemptStatus value for some
functions
that previously did not return a value.
6. Reverted changes in FmpDxe to call the new FmpDeviceLib APIs
for FmpDeviceCheckImage () and FmpDeviceSetImage ().
These will
be added in a future series after impacted platforms in
edk2-platforms are updated to use the new APIs.
7. Instead of directly changing the pre-existing APIs in
FmpDeviceLib to add a LastAttemptStatus parameter, the
new
functions were added to the library interface:
* FmpDeviceCheckImageWithStatus ()
* FmpDeviceSetImageWithStatus ()

V3 changes:
1. Enhanced range definitions in LastAttemptStatus.h with more
completeness providing length, min, and max values.
2. Moved the actual Last Attempt Status code assignments to a
private header file PrivateInclude/FmpLastAttemptStatus.h.
3. Changed the value of
LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL_VENDOR_
RANGE_MAX
to 0x3FFF instead of 0x4000 even though 0x4000 is defined in
the UEFI specification. The length is 0x4000 but the max
allowed value should be 0x3FFF. This change was made now
to
prevent implementation compatibility issues in the future.
4. Included "DEVICE" in the following macro name to clearly
associate it with the FmpDeviceLib library class:
LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_ERROR_xxx
5. Included a map to help the reader better visualize the range
definitions in LastAttemptStatus.h.
6. Included additional documentation describing the enum in
FmpLastAttemptStatus.h. An explicit statement stating that
new
codes should be added onto the end of ranges to preserve the
values was added.
7. Simplified error handling logic in FmpDxe for FmpDeviceLib
calls that return Last Attempt Status.
8. V2 had a single memory allocation failure code used for
different memory allocations in CheckFmpDependency () in
FmpDependencyLib. Each potential allocation failure was
assigned a unique code.

V2 changes:
1. Consolidate all previous incremental updates to
LastAttemptStatus.h into one patch (patch 2)
2. Move LastAttemptStatus.h from Include to PrivateInclude
3. Correct patch 1 subject from "FmpDevicePkg" to "MdePkg"

Cc: Liming Gao <gaoliming@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Guomin Jiang <guomin.jiang@...>
Cc: Wei6 Xu <wei6.xu@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>

Michael Kubacki (6):
MdePkg/SystemResourceTable.h: Add vendor range values
FmpDevicePkg: Add Last Attempt Status header files
FmpDevicePkg/FmpDxe: Add check image path Last Attempt
Status
capability
FmpDevicePkg/FmpDxe: Improve set image path Last Attempt
Status
granularity
FmpDevicePkg: Add Last Attempt Status support to dependency
libs
FmpDevicePkg/FmpDeviceLib: Add Last Attempt Status to
Check/Set API


FmpDevicePkg/FmpDxe/FmpDxe.c
| 146 +++++++++++++++++---

FmpDevicePkg/Library/FmpDependencyCheckLib/FmpDependencyCheckLib.
c
| 39 ++++--

FmpDevicePkg/Library/FmpDependencyCheckLibNull/FmpDependencyCheck
LibNull.c
| 14 +-

FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c
| 93 +++++++++++--

FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c
| 132 +++++++++++++++++-

FmpDevicePkg/Test/UnitTest/Library/FmpDependencyLib/EvaluateDependen
cyUnitTest.c
| 7 +-

FmpDevicePkg/FmpDxe/FmpDxe.h
| 4 +-

FmpDevicePkg/Include/LastAttemptStatus.h
| 81 +++++++++++

FmpDevicePkg/Include/Library/FmpDependencyCheckLib.h
| 8 +-

FmpDevicePkg/Include/Library/FmpDependencyLib.h
| 44 ++++--

FmpDevicePkg/Include/Library/FmpDeviceLib.h
| 121 +++++++++++++++-

FmpDevicePkg/PrivateInclude/FmpLastAttemptStatus.h
| 81 +++++++++++

MdePkg/Include/Guid/SystemResourceTable.h
| 13 ++
13 files changed, 718 insertions(+), 65 deletions(-)
create mode 100644 FmpDevicePkg/Include/LastAttemptStatus.h
create mode 100644
FmpDevicePkg/PrivateInclude/FmpLastAttemptStatus.h



FW: [PATCH 0/3] MdeModulePkg: use pool allocations for ACPI tables

Wu, Hao A
 

Forward to Liming's latest mail address.

Best Regards,
Hao Wu

-----Original Message-----
From: Ard Biesheuvel <ard.biesheuvel@...>
Sent: Friday, October 16, 2020 11:49 PM
To: devel@edk2.groups.io
Cc: Ard Biesheuvel <ard.biesheuvel@...>; Bi, Dandan <dandan.bi@...>; Liming Gao <gaoliming@...>; Wang, Jian J <jian.j.wang@...>; Wu, Hao A <hao.a.wu@...>; Sami Mujawar <sami.mujawar@...>; Laszlo Ersek <lersek@...>; Leif Lindholm <leif@...>
Subject: [PATCH 0/3] MdeModulePkg: use pool allocations for ACPI tables

Currently, the AcpiTableDxe memory allocator uses page based allocations, for which the only reason seems to be that it permits the use of a memory limit, which is necessary for ACPI 1.0 tables that need to reside in the first 4 GB of memory.

That requirement does not exist on AArch64, and since page based allocations are rounded up to 64 KB multiples, this wastes some memory in a way that can easily be avoided. So let's use the existing 'mAcpiTableAllocType'
policy variable, and switch to pool allocations if it is set to 'AllocateAnyPages'

Example output from Linux booting on ArmVirtQemu:

Before:
ACPI: RSDP 0x0000000078510000 000024 (v02 BOCHS )
ACPI: XSDT 0x0000000078500000 00004C (v01 BOCHS BXPCFACP 00000001 01000013)
ACPI: FACP 0x00000000784C0000 00010C (v05 BOCHS BXPCFACP 00000001 BXPC 00000001)
ACPI: DSDT 0x00000000784D0000 0014BB (v02 BOCHS BXPCDSDT 00000001 BXPC 00000001)
ACPI: APIC 0x00000000784B0000 0000A8 (v03 BOCHS BXPCAPIC 00000001 BXPC 00000001)
ACPI: GTDT 0x00000000784A0000 000060 (v02 BOCHS BXPCGTDT 00000001 BXPC 00000001)
ACPI: MCFG 0x0000000078490000 00003C (v01 BOCHS BXPCMCFG 00000001 BXPC 00000001)
ACPI: SPCR 0x0000000078480000 000050 (v02 BOCHS BXPCSPCR 00000001 BXPC 00000001)

After:
ACPI: RSDP 0x000000007C030018 000024 (v02 BOCHS )
ACPI: XSDT 0x000000007C03FE98 00004C (v01 BOCHS BXPCFACP 00000001 01000013)
ACPI: FACP 0x000000007C03FA98 00010C (v05 BOCHS BXPCFACP 00000001 BXPC 00000001)
ACPI: DSDT 0x000000007C037518 0014BB (v02 BOCHS BXPCDSDT 00000001 BXPC 00000001)
ACPI: APIC 0x000000007C03FC18 0000A8 (v03 BOCHS BXPCAPIC 00000001 BXPC 00000001)
ACPI: GTDT 0x000000007C03FD18 000060 (v02 BOCHS BXPCGTDT 00000001 BXPC 00000001)
ACPI: MCFG 0x000000007C03FE18 00003C (v01 BOCHS BXPCMCFG 00000001 BXPC 00000001)
ACPI: SPCR 0x000000007C03FF98 000050 (v02 BOCHS BXPCSPCR 00000001 BXPC 00000001)

Cc: Dandan Bi <dandan.bi@...>
Cc: Liming Gao <gaoliming@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Sami Mujawar <sami.mujawar@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Leif Lindholm <leif@...>

Ard Biesheuvel (3):
MdeModulePkg/AcpiTableDxe: use pool allocations when possible
MdeModulePkg/AcpiTableDxe: use pool allocation for RSDT/XSDT if
possible
MdeModulePkg/AcpiTableDxe: use pool allocation for RSDP if possible

.../Universal/Acpi/AcpiTableDxe/AcpiTable.h | 4 +-
.../Universal/Acpi/AcpiTableDxe/AcpiSdt.c | 4 +-
.../Acpi/AcpiTableDxe/AcpiTableProtocol.c | 216 +++++++++++-------
3 files changed, 143 insertions(+), 81 deletions(-)

--
2.17.1


回复: [edk2-devel] CI test on copyright

gaoliming
 

Bob:
Now, the problem is the inconsistent behavior. The same copyright style ((C) Copyright 2020 Hewlett Packard Enterprise Development LP ) in .c can pass ECC, but in .inf file can't pass ECC. I want to know the reason.

Thanks
Liming

-----邮件原件-----
发件人: Feng, Bob C <bob.c.feng@...>
发送时间: 2020年10月19日 9:25
收件人: devel@edk2.groups.io; gaoliming@...;
spbrogan@...; shenglei.zhang@...; abner.chang@...
抄送: 'Bret Barkelew' <Bret.Barkelew@...>; 'Sean Brogan'
<sean.brogan@...>
主题: RE: [edk2-devel] CI test on copyright

In the ECC, the rule to check Copyright is to see if the Copyright is the first
word of a line, and the Copyright should be followed by a (

Liming, for your reference, the code in ECC

## _IsCopyrightLine
# check whether current line is copyright line, the criteria is whether there is
case insensitive keyword "Copyright"
# followed by zero or more white space characters followed by a "(" character
#
# @param LineContent: the line need to be checked
# @return: True if current line is copyright line, False else
#

-Bob

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of gaoliming
Sent: Monday, October 19, 2020 9:01 AM
To: devel@edk2.groups.io; spbrogan@...;
shenglei.zhang@...; abner.chang@...
Cc: 'Bret Barkelew' <Bret.Barkelew@...>; 'Sean Brogan'
<sean.brogan@...>
Subject: 回复: [edk2-devel] CI test on copyright

I will check this issue first.

Thanks
Liming

-----邮件原件-----
发件人: bounce+27952+66349+4905953+8761045@groups.io
<bounce+27952+66349+4905953+8761045@groups.io> 代表 Sean
发送时间: 2020年10月17日 12:46
收件人: devel@edk2.groups.io; shenglei.zhang@...;
abner.chang@...
抄送: Bret Barkelew <Bret.Barkelew@...>; Sean Brogan
<sean.brogan@...>
主题: Re: [edk2-devel] CI test on copyright

ECC stuff was done by Intel. I would defer to them.

Adding shenglei.zhang@...

On 10/15/2020 9:21 AM, Abner Chang wrote:
Hi Sean and Bret,
I got the CI test error which says the first line in file header
section
must
have the copyright information, however the copyright is there and
looks
to
me fine as below,
(C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>

[cid:image001.png@...]


The interesting part is CI seems happy with below format of
copyright,
and
this issue only happens on INF file but not on the *.c and *.h.
Copyright (C) 2020 Hewlett Packard Enterprise Development LP<BR>

Any idea on this?
Abner













Re: CI test on copyright

Bob Feng
 

In the ECC, the rule to check Copyright is to see if the Copyright is the first word of a line, and the Copyright should be followed by a (

Liming, for your reference, the code in ECC

## _IsCopyrightLine
# check whether current line is copyright line, the criteria is whether there is case insensitive keyword "Copyright"
# followed by zero or more white space characters followed by a "(" character
#
# @param LineContent: the line need to be checked
# @return: True if current line is copyright line, False else
#

-Bob

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of gaoliming
Sent: Monday, October 19, 2020 9:01 AM
To: devel@edk2.groups.io; spbrogan@...; shenglei.zhang@...; abner.chang@...
Cc: 'Bret Barkelew' <Bret.Barkelew@...>; 'Sean Brogan' <sean.brogan@...>
Subject: 回复: [edk2-devel] CI test on copyright

I will check this issue first.

Thanks
Liming

-----邮件原件-----
发件人: bounce+27952+66349+4905953+8761045@groups.io
<bounce+27952+66349+4905953+8761045@groups.io> 代表 Sean
发送时间: 2020年10月17日 12:46
收件人: devel@edk2.groups.io; shenglei.zhang@...;
abner.chang@...
抄送: Bret Barkelew <Bret.Barkelew@...>; Sean Brogan
<sean.brogan@...>
主题: Re: [edk2-devel] CI test on copyright

ECC stuff was done by Intel. I would defer to them.

Adding shenglei.zhang@...

On 10/15/2020 9:21 AM, Abner Chang wrote:
Hi Sean and Bret,
I got the CI test error which says the first line in file header
section
must
have the copyright information, however the copyright is there and
looks
to
me fine as below,
(C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>

[cid:image001.png@...]


The interesting part is CI seems happy with below format of
copyright,
and
this issue only happens on INF file but not on the *.c and *.h.
Copyright (C) 2020 Hewlett Packard Enterprise Development LP<BR>

Any idea on this?
Abner








回复: [edk2-devel] [Patch] BaseTools: Fix PcdValueInit tool build issue with VS compiler x64

gaoliming
 

Sean and Bob:
How verify this change?

Thanks
Liming

-----邮件原件-----
发件人: Sean Brogan <spbrogan@...>
发送时间: 2020年10月17日 6:17
收件人: devel@edk2.groups.io; bob.c.feng@...
抄送: Liming Gao <gaoliming@...>; Yuwei Chen
<yuwei.chen@...>; Michael D Kinney <michael.d.kinney@...>;
Sean Brogan <sean.brogan@...>
主题: Re: [edk2-devel] [Patch] BaseTools: Fix PcdValueInit tool build issue
with VS compiler x64

Reviewed-by: Sean Brogan <sean.brogan@...>

On 10/15/2020 4:20 AM, Bob Feng wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3001

When the 64-bit version of VS compiler is used, the generated
PcdValueInit tool will be failed to compile.

This patch is going to fix that issue.

Signed-off-by: Bob Feng <bob.c.feng@...>
Cc: Liming Gao <gaoliming@...>
Cc: Yuwei Chen <yuwei.chen@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Sean Brogan <sean.brogan@...>

---
BaseTools/Source/C/Common/PcdValueCommon.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Source/C/Common/PcdValueCommon.h
b/BaseTools/Source/C/Common/PcdValueCommon.h
index cfd3bb76e1..1652bd5430 100644
--- a/BaseTools/Source/C/Common/PcdValueCommon.h
+++ b/BaseTools/Source/C/Common/PcdValueCommon.h
@@ -12,11 +12,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Common/UefiBaseTypes.h>
#include <Common/UefiInternalFormRepresentation.h>

#define __FIELD_SIZE(TYPE, Field) (sizeof((TYPE *)0)->Field)
#define __ARRAY_ELEMENT_SIZE(TYPE, Field) (sizeof((TYPE
*)0)->Field[0])
-#define __OFFSET_OF(TYPE, Field) ((UINT32) &(((TYPE *)0)->Field))
+#define __OFFSET_OF(TYPE, Field) ((UINT32)(size_t) &(((TYPE *)0)->Field))
#define __FLEXIBLE_SIZE(Size, TYPE, Field, MaxIndex) if
(__FIELD_SIZE(TYPE, Field) == 0) Size = MAX((__OFFSET_OF(TYPE, Field) +
__ARRAY_ELEMENT_SIZE(TYPE, Field) * (MaxIndex)), Size)
#define __ARRAY_SIZE(Array) (sizeof(Array)/sizeof(Array[0]))

#if defined(_MSC_EXTENSIONS)
#define __STATIC_ASSERT static_assert


回复: [edk2-devel] CI test on copyright

gaoliming
 

I will check this issue first.

Thanks
Liming

-----邮件原件-----
发件人: bounce+27952+66349+4905953+8761045@groups.io
<bounce+27952+66349+4905953+8761045@groups.io> 代表 Sean
发送时间: 2020年10月17日 12:46
收件人: devel@edk2.groups.io; shenglei.zhang@...;
abner.chang@...
抄送: Bret Barkelew <Bret.Barkelew@...>; Sean Brogan
<sean.brogan@...>
主题: Re: [edk2-devel] CI test on copyright

ECC stuff was done by Intel. I would defer to them.

Adding shenglei.zhang@...

On 10/15/2020 9:21 AM, Abner Chang wrote:
Hi Sean and Bret,
I got the CI test error which says the first line in file header section
must
have the copyright information, however the copyright is there and looks
to
me fine as below,
(C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>

[cid:image001.png@...]


The interesting part is CI seems happy with below format of copyright,
and
this issue only happens on INF file but not on the *.c and *.h.
Copyright (C) 2020 Hewlett Packard Enterprise Development LP<BR>

Any idea on this?
Abner








Re: [PATCH v3 0/2] CryptoPkg/OpensslLib: Add native instruction support for X64

Yao, Jiewen
 

Thanks Zurcher.

Comment:
1) I do not see the copy right header and license header for uefi-asm.conf. Please add it.

2) Is "ApiHooks.c" only for Visual Studio? Do we need it for GCC and LLVM?
If it is only for visual studio, I suggest we rename to MsftApiHooks.c, and add "| MSFT" in INF.

3) Since nasm is only for X64¸ please use [Source.X64] for them.

Please refer to https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BaseLib/BaseLib.inf, on 2) and 3).

I want to get ACK from Mike Kinney, to see if it is right way to manually add nasm and ApiHooks.c. Or if we need some extra documentation for that.
Please do add him as reviewer in next version patch.

I treat the ApiHooks.c is a work-around and it should be removed in the future, once openssl resolve it. Maybe we need a Bugzilla to track this.


Thank you
Yao Jiewen

-----Original Message-----
From: Christopher J Zurcher <christopher.j.zurcher@...>
Sent: Thursday, October 15, 2020 4:49 AM
To: devel@edk2.groups.io
Cc: Yao, Jiewen <jiewen.yao@...>; Wang, Jian J
<jian.j.wang@...>; Lu, XiaoyuX <xiaoyux.lu@...>; Ard
Biesheuvel <ard.biesheuvel@...>
Subject: [PATCH v3 0/2] CryptoPkg/OpensslLib: Add native instruction
support for X64

V3 Changes:
Added definitions for ptrdiff_t and wchar_t to CrtLibSupport.h for
LLVM/Clang build support.
Added -UWIN32 to GCC Flags for LLVM/Clang build support.
Added missing AES GCM assembly file.

V2 Changes:
Limit scope of assembly config to SHA and AES functions.
Removed IA32 native support (reduced config was causing build failure and
can be added in a later patch).
Removed XMM instructions from assembly generation.
Added automatic copyright header porting for generated assembly files.

This patch adds support for building the native instruction algorithms for
the X64 architecture in OpensslLib. The process_files.pl script was modified
to parse the .asm file targets from the OpenSSL build config data struct, and
generate the necessary assembly files for the EDK2 build environment.

For the X64 variant, OpenSSL includes calls to a Windows error handling API,
and that function has been stubbed out in ApiHooks.c.

For all variants, a constructor is added to call the required CPUID function
within OpenSSL to facilitate processor capability checks in the native
algorithms.

Additional native architecture variants should be simple to add by following
the changes made for this architecture.

The OpenSSL assembly files are traditionally generated at build time using a
perl script. To avoid that burden on EDK2 users, these end-result assembly
files are generated during the configuration steps performed by the package
maintainer (through process_files.pl). The perl generator scripts inside
OpenSSL do not parse file comments as they are only meant to create
intermediate build files, so process_files.pl contains additional hooks to
preserve the copyright headers as well as clean up tabs and line endings to
comply with EDK2 coding standards. The resulting file headers align with
the generated .h files which are already included in the EDK2 repository.

Cc: Jiewen Yao <jiewen.yao@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Xiaoyu Lu <xiaoyux.lu@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>

Christopher J Zurcher (2):
CryptoPkg/OpensslLib: Add native instruction support for X64
CryptoPkg/OpensslLib: Commit the auto-generated assembly files for X64

CryptoPkg/Library/OpensslLib/OpensslLib.inf | 2 +-
CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf | 2 +-
CryptoPkg/Library/OpensslLib/OpensslLibX64.inf | 657 +++
CryptoPkg/Library/Include/CrtLibSupport.h | 2 +
CryptoPkg/Library/Include/openssl/opensslconf.h | 3 -
CryptoPkg/Library/OpensslLib/ApiHooks.c | 18 +
CryptoPkg/Library/OpensslLib/OpensslLibConstructor.c | 34 +
CryptoPkg/Library/OpensslLib/X64/crypto/aes/aesni-mb-x86_64.nasm |
732 +++
CryptoPkg/Library/OpensslLib/X64/crypto/aes/aesni-sha1-x86_64.nasm |
1916 ++++++++
CryptoPkg/Library/OpensslLib/X64/crypto/aes/aesni-sha256-x86_64.nasm |
78 +
CryptoPkg/Library/OpensslLib/X64/crypto/aes/aesni-x86_64.nasm |
5103 ++++++++++++++++++++
CryptoPkg/Library/OpensslLib/X64/crypto/aes/vpaes-x86_64.nasm |
1173 +++++
CryptoPkg/Library/OpensslLib/X64/crypto/modes/aesni-gcm-x86_64.nasm |
34 +
CryptoPkg/Library/OpensslLib/X64/crypto/modes/ghash-x86_64.nasm |
1569 ++++++
CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha1-mb-x86_64.nasm |
3137 ++++++++++++
CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha1-x86_64.nasm |
2884 +++++++++++
CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha256-mb-x86_64.nasm |
3461 +++++++++++++
CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha256-x86_64.nasm |
3313 +++++++++++++
CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha512-x86_64.nasm |
1938 ++++++++
CryptoPkg/Library/OpensslLib/X64/crypto/x86_64cpuid.nasm | 491
++
CryptoPkg/Library/OpensslLib/process_files.pl | 223 +-
CryptoPkg/Library/OpensslLib/uefi-asm.conf | 15 +
22 files changed, 26735 insertions(+), 50 deletions(-)
create mode 100644 CryptoPkg/Library/OpensslLib/OpensslLibX64.inf
create mode 100644 CryptoPkg/Library/OpensslLib/ApiHooks.c
create mode 100644 CryptoPkg/Library/OpensslLib/OpensslLibConstructor.c
create mode 100644 CryptoPkg/Library/OpensslLib/X64/crypto/aes/aesni-
mb-x86_64.nasm
create mode 100644 CryptoPkg/Library/OpensslLib/X64/crypto/aes/aesni-
sha1-x86_64.nasm
create mode 100644 CryptoPkg/Library/OpensslLib/X64/crypto/aes/aesni-
sha256-x86_64.nasm
create mode 100644 CryptoPkg/Library/OpensslLib/X64/crypto/aes/aesni-
x86_64.nasm
create mode 100644 CryptoPkg/Library/OpensslLib/X64/crypto/aes/vpaes-
x86_64.nasm
create mode 100644
CryptoPkg/Library/OpensslLib/X64/crypto/modes/aesni-gcm-x86_64.nasm
create mode 100644
CryptoPkg/Library/OpensslLib/X64/crypto/modes/ghash-x86_64.nasm
create mode 100644 CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha1-
mb-x86_64.nasm
create mode 100644 CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha1-
x86_64.nasm
create mode 100644 CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha256-
mb-x86_64.nasm
create mode 100644 CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha256-
x86_64.nasm
create mode 100644 CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha512-
x86_64.nasm
create mode 100644
CryptoPkg/Library/OpensslLib/X64/crypto/x86_64cpuid.nasm
create mode 100644 CryptoPkg/Library/OpensslLib/uefi-asm.conf

--
2.28.0.windows.1


Re: [PATCH] SecurityPkg/Tcg2PhysicalPresenceLib: Fix incorrect TCG VER comparision

Yao, Jiewen
 

Pushed
https://github.com/tianocore/edk2/pull/1027
git-hash: 709b163940c55604b983400eb49dad144a2aa091

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao,
Jiewen
Sent: Friday, October 16, 2020 1:55 PM
To: Lee, Terry <terry.lee@...>; devel@edk2.groups.io;
stefanb@...; lersek@...; Gao, Zhichao
<zhichao.gao@...>
Cc: Wang, Jian J <jian.j.wang@...>; Zhang, Chao B
<chao.b.zhang@...>; Marc-André Lureau
<marcandre.lureau@...>
Subject: Re: [edk2-devel] [PATCH] SecurityPkg/Tcg2PhysicalPresenceLib: Fix
incorrect TCG VER comparision

Fair enough.

Reviewed-by: Jiewen Yao <Jiewen.yao@...>

I will wait for a few more days to see if there is any feedback from Laszlo or
Marc-Andre.




-----Original Message-----
From: Lee, Terry <terry.lee@...>
Sent: Friday, October 16, 2020 1:32 PM
To: Yao, Jiewen <jiewen.yao@...>; devel@edk2.groups.io;
stefanb@...; lersek@...; Gao, Zhichao
<zhichao.gao@...>
Cc: Wang, Jian J <jian.j.wang@...>; Zhang, Chao B
<chao.b.zhang@...>; Marc-André Lureau
<marcandre.lureau@...>
Subject: RE: [edk2-devel] [PATCH] SecurityPkg/Tcg2PhysicalPresenceLib: Fix
incorrect TCG VER comparision

Jiewen,

I have only PP1.3 configuration. The only WHCK test failure is a known
Windows issue that I believe is unrelated to PP.

Terry

-----Original Message-----
From: Yao, Jiewen [mailto:jiewen.yao@...]
Sent: Thursday, October 15, 2020 7:31 PM
To: Lee, Terry <terry.lee@...>; devel@edk2.groups.io;
stefanb@...; lersek@...; Gao, Zhichao
<zhichao.gao@...>
Cc: Wang, Jian J <jian.j.wang@...>; Zhang, Chao B
<chao.b.zhang@...>; Marc-André Lureau
<marcandre.lureau@...>
Subject: RE: [edk2-devel] [PATCH] SecurityPkg/Tcg2PhysicalPresenceLib: Fix
incorrect TCG VER comparision

Thanks Terry.
I tend to give R-B. I read the code it seems no impact.

Would you please confirm you have tested both PP1.2 and PP1.3
configuration, with windows WHCK test pass?

Thank you
Yao Jiewen

-----Original Message-----
From: Lee, Terry <terry.lee@...>
Sent: Friday, October 16, 2020 10:25 AM
To: Yao, Jiewen <jiewen.yao@...>; devel@edk2.groups.io;
stefanb@...; lersek@...; Gao, Zhichao
<zhichao.gao@...>
Cc: Wang, Jian J <jian.j.wang@...>; Zhang, Chao B
<chao.b.zhang@...>; Marc-André Lureau
<marcandre.lureau@...>
Subject: RE: [edk2-devel] [PATCH] SecurityPkg/Tcg2PhysicalPresenceLib:
Fix incorrect TCG VER comparision

Jiewen,

I tested this patch on HPE Superdome Flex with both Linux and Windows.

Terry

-----Original Message-----
From: Yao, Jiewen [mailto:jiewen.yao@...]
Sent: Thursday, October 15, 2020 6:09 PM
To: Lee, Terry <terry.lee@...>; devel@edk2.groups.io;
stefanb@...; lersek@...; Gao, Zhichao
<zhichao.gao@...>
Cc: Wang, Jian J <jian.j.wang@...>; Zhang, Chao B
<chao.b.zhang@...>; Marc-André Lureau
<marcandre.lureau@...>
Subject: RE: [edk2-devel] [PATCH] SecurityPkg/Tcg2PhysicalPresenceLib:
Fix incorrect TCG VER comparision

Hello
Is there any one can share the information on what test has been done
for this ?

Thank you
Yao Jiewen

-----Original Message-----
From: Lee, Terry <terry.lee@...>
Sent: Friday, October 16, 2020 12:59 AM
To: devel@edk2.groups.io; stefanb@...; lersek@...;
Gao, Zhichao <zhichao.gao@...>
Cc: Yao, Jiewen <jiewen.yao@...>; Wang, Jian J
<jian.j.wang@...>; Zhang, Chao B <chao.b.zhang@...>;
Marc- André Lureau <marcandre.lureau@...>
Subject: RE: [edk2-devel] [PATCH] SecurityPkg/Tcg2PhysicalPresenceLib:
Fix incorrect TCG VER comparision

Could the package maintainer merge this patch? Thanks.

Terry

-----Original Message-----
From: Stefan Berger [mailto:stefanb@...]
Sent: Friday, July 10, 2020 7:27 AM
To: devel@edk2.groups.io; lersek@...; Gao, Zhichao
<zhichao.gao@...>
Cc: Lee, Terry <terry.lee@...>; Yao, Jiewen
<jiewen.yao@...>; Wang, Jian J <jian.j.wang@...>; Zhang,
Chao B <chao.b.zhang@...>; Marc-André Lureau
<marcandre.lureau@...>
Subject: Re: [edk2-devel] [PATCH] SecurityPkg/Tcg2PhysicalPresenceLib:
Fix incorrect TCG VER comparision

On 7/10/20 9:53 AM, Stefan Berger wrote:
On 7/10/20 1:43 AM, Laszlo Ersek wrote:
(+Marc-André, Stefan)

On 07/10/20 02:44, Gao, Zhichao wrote:
This bug is not obeserved by me. But I view the code. The
condition is incorrect and it would affect the TCG operation:
if (!mIsTcg2PPVerLowerThan_1_3) {
if (OperationRequest <
TCG2_PHYSICAL_PRESENCE_VENDOR_SPECIFIC_OPERATION) {
//
// TCG2 PP1.3 spec defined operations that are
reserved or un-implemented
//
return
TCG_PP_GET_USER_CONFIRMATION_NOT_IMPLEMENTED;
}
} else {
//
// TCG PP lower than 1.3. (1.0, 1.1, 1.2)
//
if (OperationRequest <=
TCG2_PHYSICAL_PRESENCE_NO_ACTION_MAX) {
RequestConfirmed = TRUE;
} else if (OperationRequest <
TCG2_PHYSICAL_PRESENCE_VENDOR_SPECIFIC_OPERATION) {
return
TCG_PP_GET_USER_CONFIRMATION_NOT_IMPLEMENTED;
}
}
I've found that code myself, but I'm not familiar enough with TPM
PPI stuff to understand immediately the effects of this change. I
can see that where we used to return
TCG_PP_GET_USER_CONFIRMATION_NOT_IMPLEMENTED before, we
could now
assign "RequestConfirmed = TRUE", and vice versa, due to
"mIsTcg2PPVerLowerThan_1_3" being potentially inverted.

But what does that *mean*? What is the behavioral change that
human end-users, or software components, will experience?

The above code snipped is located in a default branch of a large
switch statement that handles most of the common PPI operations
independent of this change, so that at least is good.

I would say that in the worst case some of the operations not
otherwise handled may have mistakenly failed or could have been
executed without user confirmation/interaction. On Linux at least
PPI requests can only be sent by root.

I am running a somewhat dated version of edk2 (Fedora 31). The
operations advertised are: 0,5,14,21,22,23,24,33,96,97. All of these
are individually handled in the switch statement, so there should no
be any impact. I am currently not aware of whether this list can be
extended with some sort of module.





Thanks
Laszlo

So I think it should be fixed.

Thanks,
Zhichao

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf
Of
Laszlo Ersek
Sent: Thursday, July 9, 2020 6:02 PM
To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@...>
Cc: Terry Lee <terry.lee@...>; Yao, Jiewen
<jiewen.yao@...>; Wang, Jian J <jian.j.wang@...>;
Zhang, Chao B <chao.b.zhang@...>
Subject: Re: [edk2-devel] [PATCH]
SecurityPkg/Tcg2PhysicalPresenceLib: Fix incorrect TCG VER
comparision

On 07/09/20 04:46, Gao, Zhichao wrote:
From: Terry Lee <terry.lee@...>

REF:
https://urldefense.proofpoint.com/v2/url?u=https-3A__bugzilla.
ti
an
ocore.org_show-5Fbug.cgi-3Fid-
3D2697&d=DwIDaQ&c=C5b8zRQO1miGmBeVZ2
LFWg&r=Jlc0Jxr620EZ-
CppyrjGotnxH9DrT0KvwcLjekZ9Dow&m=WPv3vn5VEelRC
s-
W8pfNM00wMOfpKBesXnAhRfylF7g&s=iFUYthUCfHLeeQAvr_OhTPHTiA9hZvw
48
Bj8YhXhQAI&e=

Tcg2PhysicalPresenceLibConstructor set the module variable
mIsTcg2PPVerLowerThan_1_3 with incorrect TCG version
comparision.

Cc: Jiewen Yao <jiewen.yao@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Chao Zhang <chao.b.zhang@...>
Signed-off-by: Zhichao Gao <zhichao.gao@...>
---
.../SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c |
2
+-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git
a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPr
esen

ceLib.c
b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPr
esen

ceLib.c
index 1c46d5e69d..8afaa0a785 100644
---
a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPr
esen

ceLib.c
+++
b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPr
+++ esenceLib.c
@@ -387,7 +387,7 @@ Tcg2PhysicalPresenceLibConstructor ( {
EFI_STATUS Status;

- if (AsciiStrnCmp(PP_INF_VERSION_1_2, (CHAR8
*)PcdGetPtr(PcdTcgPhysicalPresenceInterfaceVer),
sizeof(PP_INF_VERSION_1_2) - 1) <= 0) {
+ if (AsciiStrnCmp(PP_INF_VERSION_1_2, (CHAR8
+*)PcdGetPtr(PcdTcgPhysicalPresenceInterfaceVer),
+ sizeof(PP_INF_VERSION_1_2) - 1) >= 0) {
mIsTcg2PPVerLowerThan_1_3 = TRUE;
}

What is the practical impact of this bug / fix?

Thanks
Laszlo









Re: [PATCH] CryptoPkg/BaseCryptLib: fix NULL dereference (CVE-2019-14584)

Yao, Jiewen
 

Reviewed-by: Jiewen Yao <Jiewen.yao@...>

-----Original Message-----
From: Wang, Jian J <jian.j.wang@...>
Sent: Friday, October 16, 2020 1:15 PM
To: devel@edk2.groups.io
Cc: Lu, XiaoyuX <xiaoyux.lu@...>; Jiang, Guomin
<guomin.jiang@...>; Yao, Jiewen <jiewen.yao@...>; Laszlo
Ersek <lersek@...>
Subject: [PATCH] CryptoPkg/BaseCryptLib: fix NULL dereference (CVE-2019-
14584)

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



AuthenticodeVerify() calls OpenSSLs d2i_PKCS7() API to parse asn encoded

signed authenticode pkcs#7 data. when this successfully returns, a type

check is done by calling PKCS7_type_is_signed() and then

Pkcs7->d.sign->contents->type is used. It is possible to construct an asn1

blob that successfully decodes and have d2i_PKCS7() return a valid pointer

and have PKCS7_type_is_signed() also return success but have Pkcs7->d.sign

be a NULL pointer.



Looking at how PKCS7_verify() [inside of OpenSSL] implements checking for

pkcs7 structs it does the following:

- call PKCS7_type_is_signed()

- call PKCS7_get_detached()

Looking into how PKCS7_get_detatched() is implemented, it checks to see if

p7->d.sign is NULL or if p7->d.sign->contents->d.ptr is NULL.



As such, the fix is to do the same as OpenSSL after calling d2i_PKCS7().

- Add call to PKS7_get_detached() to existing error handling



Cc: Xiaoyu Lu <xiaoyux.lu@...>

Cc: Guomin Jiang <guomin.jiang@...>

Cc: Jiewen Yao <jiewen.yao@...>

Cc: Laszlo Ersek <lersek@...>
Signed-off-by: Jian J Wang <jian.j.wang@...>

Reviewed-by: Laszlo Ersek <lersek@...>
---
CryptoPkg/Library/BaseCryptLib/Pk/CryptAuthenticode.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/CryptoPkg/Library/BaseCryptLib/Pk/CryptAuthenticode.c
b/CryptoPkg/Library/BaseCryptLib/Pk/CryptAuthenticode.c
index 2772b1e2be..ae0ee61fb6 100644
--- a/CryptoPkg/Library/BaseCryptLib/Pk/CryptAuthenticode.c
+++ b/CryptoPkg/Library/BaseCryptLib/Pk/CryptAuthenticode.c
@@ -9,7 +9,7 @@
AuthenticodeVerify() will get PE/COFF Authenticode and will do basic check
for

data structure.



-Copyright (c) 2011 - 2015, Intel Corporation. All rights reserved.<BR>

+Copyright (c) 2011 - 2019, Intel Corporation. All rights reserved.<BR>

SPDX-License-Identifier: BSD-2-Clause-Patent



**/

@@ -100,7 +100,7 @@ AuthenticodeVerify (
//

// Check if it's PKCS#7 Signed Data (for Authenticode Scenario)

//

- if (!PKCS7_type_is_signed (Pkcs7)) {

+ if (!PKCS7_type_is_signed (Pkcs7) || PKCS7_get_detached (Pkcs7)) {

goto _Exit;

}



--
2.19.0.windows.1


Re: [PATCH v6 0/2] Add Unit Tests for BaseCryptLib to CryptoPkg

Yao, Jiewen
 

Pushed.
https://github.com/tianocore/edk2/pull/1026
git-hash: 694bfd6ff5b9a8352b4ca8634ed4ce449f505991, 73e3cb6c7eea4f5db81c87574dcefe1282de4772

-----Original Message-----
From: matthewfcarlson@... <matthewfcarlson@...>
Sent: Friday, October 9, 2020 6:38 AM
To: devel@edk2.groups.io
Cc: Wang, Jian J <jian.j.wang@...>; Lu, XiaoyuX
<xiaoyux.lu@...>; Yao, Jiewen <jiewen.yao@...>; Jiang,
Guomin <guomin.jiang@...>; Sean Brogan
<sean.brogan@...>; Bret Barkelew
<Bret.Barkelew@...>; Kinney, Michael D
<michael.d.kinney@...>; Liming Gao <liming.gao@...>
Subject: [PATCH v6 0/2] Add Unit Tests for BaseCryptLib to CryptoPkg

From: Matthew Carlson <matthewfcarlson@...>

This turns adds Host Based Unit Tests for CryptoPkg, adds a new
BaseCryptLib
implementation that is meant for unit testing and turns on HBUT for
CryptoPkg
CI.

Changes for V6:
Told ECC to ignore unit test and new CRT wrapper

Changes for V5:
Remove BaseTimerLibPosix as it is no longer needed
Cleaned up a bit of the documentation around the tests

Cc: Jian J Wang <jian.j.wang@...>
Cc: Xiaoyu Lu <xiaoyux.lu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Guomin Jiang <guomin.jiang@...>
Cc: Sean Brogan <sean.brogan@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <liming.gao@...>

*** BLURB HERE ***

Matthew Carlson (2):
CryptoPkg: BaseCryptLib: Add unit tests (Host and Shell based)
AzurePipelines : Pr Gate: Turn on HBUT for CryptoPkg

CryptoPkg/Library/BaseCryptLib/SysCall/UnitTestHostCrtWrapper.c
| 93 ++
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/AuthenticodeTests.c
| 1002 ++++++++++++++++++++
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/BaseCryptLibUnitTests.c
| 66 ++
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/BlockCipherTests.c
| 293 ++++++
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/DhTests.c
| 106 +++
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/HashTests.c
| 197 ++++
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/HmacTests.c
| 184 ++++
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/OaepEncryptTests.c
| 308 ++++++
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/Pkcs5Pbkdf2Tests.c
| 71 ++
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/Pkcs7EkuTests.c
| 524 ++++++++++
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/RandTests.c
| 51 +
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/RsaPkcs7Tests.c
| 415 ++++++++
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/RsaTests.c
| 310 ++++++
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TSTests.c
| 335 +++++++
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/UnitTestMain.c
| 81 ++
.azurepipelines/templates/pr-gate-build-job.yml
| 2 +-
CryptoPkg/CryptoPkg.ci.yaml | 8 +
CryptoPkg/CryptoPkg.dsc | 23 +
CryptoPkg/Library/BaseCryptLib/UnitTestHostBaseCryptLib.inf
| 90 ++
CryptoPkg/Test/CryptoPkgHostUnitTest.dsc
| 35 +
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/Pkcs7EkuTestSignatures.h
| 789 +++++++++++++++
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLib.h
| 121 +++
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibHost.inf
| 46 +
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibShell.inf
| 49 +

CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/ChainCreationI
nstructions.txt | 92 ++

CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/CreateTestCert
s.cmd | 11 +

CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/SignFirmwareW
ithEKUs.cmd | 76 ++

CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/TestEKUParsing
IssuingCA.ini | 45 +

CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/TestEKUParsing
LeafSigner.ini | 25 +

CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/TestEKUParsing
LeafSignerPid1.ini | 24 +

CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/TestEKUParsing
LeafSignerPid12345.ini | 27 +

CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/TestEKUParsing
NoEKUsInSigner.ini | 16 +

CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/TestEKUParsing
PolicyCA.ini | 28 +

CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/TestEKUParsing
Root.ini | 28 +
34 files changed, 5570 insertions(+), 1 deletion(-)
create mode 100644
CryptoPkg/Library/BaseCryptLib/SysCall/UnitTestHostCrtWrapper.c
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/AuthenticodeTests.c
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/BaseCryptLibUnitTests.c
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/BlockCipherTests.c
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/DhTests.c
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/HashTests.c
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/HmacTests.c
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/OaepEncryptTests.c
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/Pkcs5Pbkdf2Tests.c
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/Pkcs7EkuTests.c
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/RandTests.c
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/RsaPkcs7Tests.c
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/RsaTests.c
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TSTests.c
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/UnitTestMain.c
create mode 100644
CryptoPkg/Library/BaseCryptLib/UnitTestHostBaseCryptLib.inf
create mode 100644 CryptoPkg/Test/CryptoPkgHostUnitTest.dsc
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/Pkcs7EkuTestSignatures.h
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLib.h
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibHost.inf
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibShell.inf
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/ChainCreationI
nstructions.txt
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/CreateTestCert
s.cmd
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/SignFirmwareW
ithEKUs.cmd
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/TestEKUParsing
IssuingCA.ini
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/TestEKUParsing
LeafSigner.ini
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/TestEKUParsing
LeafSignerPid1.ini
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/TestEKUParsing
LeafSignerPid12345.ini
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/TestEKUParsing
NoEKUsInSigner.ini
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/TestEKUParsing
PolicyCA.ini
create mode 100644
CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestEKUCerts/TestEKUParsing
Root.ini

--
2.28.0.vfs.0.0


Re: Issues with edk2 compilation with xcode5 on mac OS - files compiled but strange behaviors

Andrew Fish
 



On Oct 17, 2020, at 5:19 AM, Daniele Crudo <crudo.daniele@...> wrote:

Finally I found the culprit.

I tested also different toolchains; here the details.

On Big Sur beta 10:

Xcode 12.0.1
Build version 12A7300

1- Toolchain 1:
Apple clang version 12.0.0 (clang-1200.0.32.2)
Target: x86_64-apple-darwin20.1.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

2- Toolchain 2:
clang version 9.0.0 (git://github.com/llvm/llvm-project.git 0399d5a9682b3cef71c653373e38890c63c4c365)
Target: x86_64-apple-darwin20.1.0
Thread model: posix
InstalledDir: /Users/daniele/Downloads/clang+llvm-9.0.0-x86_64-darwin-apple/bin

On Catalina 10.15.7:

Xcode 12.0.1
Build version 12A7300

1- Toolchain 1:
Apple clang version 12.0.0 (clang-1200.0.32.2)
Target: x86_64-apple-darwin19.6.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

2- Toolchain 2:
clang version 9.0.0 (git://github.com/llvm/llvm-project.git 0399d5a9682b3cef71c653373e38890c63c4c365)
Target: x86_64-apple-darwin19.6.0
Thread model: posix
InstalledDir: /Users/daniele/Downloads/clang+llvm-9.0.0-x86_64-darwin-apple/bin

3- Toolchain 3:
Apple clang version 7.0.0  (based on LLVM 7.0.0)
Target: x86_64-apple-darwin19.6.0
Thread model: posix
InstalledDir: /Library/Developer/Toolchains/swift-latest.xctoolchain/usr/bin
------------------------------------------------------------------------------

I noticed that the issue was with Catalina and Toolchain 3.
Commands to compile the files with GPU issue (Toolchain 3):

$ git clone https://github.com/tianocore/edk2.git
$ cd edk2
$ git clean -ffdx
$ git reset --hard
$ git submodule deinit --force --all
$ git checkout edk2-stable202008
$ git submodule update --init --force
$ source edksetup.sh
$ nice make -C "$EDK_TOOLS_PATH" -j $(getconf _NPROCESSORS_ONLN)
$ build -a X64 -b RELEASE -p OvmfPkg/OvmfPkgX64.dsc -t XCODE5

These commands build files successfully, but OVMF_VARS.fd and OVMF_CODE.fd give the described issue (cannot boot if hdmi is not plugged in).

Other toolchains (both on Big Sur and Catalina), with the same commands reported above, give an error if:

$ make -C BaseTools/

is not run after:

$ git submodule update --init --force

All the other toolchains build files without the described issue (I can boot with only dvi attached).

So, I thought, let's run:

$ make -C BaseTools/

with toolchain 3 (clang 7, on Catalina) and boom, I'm able to boot with only dvi attached.

I'm not sure where the problem is, because if you take a look at the first mac os log I attached (where I did not explicitly run $ make -C BaseTools/), BaseTools seem to have compiled:

"Finished building BaseTools C Tools with HOST_ARCH=X64"

Anyway, if I don't explicitly run:

$ make -C BaseTools/

OVMF_CODE.fd and OVMF_VARS.fd don't work properly.

Summary for tianocore logo (logo is shown: yes/no):

Clang 12.0: NO
Clang 9.0: YES
Clang 7.0: YES


Lattner (clang was his PhD project) has always been aggressive about optimizing away undefined behavior [1]. So generally when a newer version of clang starts showing random behavior my 1st thought is the optimize has found some new undefined behavior and optimized It away. This generally ends up with hard to track down bugs :(.  

One tick to debug undefined behavior  is to turn off optimization for a given driver or library. You can turn off optimizations by adding an entry like this to the drivers 

/Volumes/Case/edk2-github(master)>git diff
diff --git a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
index fe8befd51d3c..e897a5da925b 100644
--- a/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
+++ b/OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf
@@ -64,3 +64,6 @@ [Protocols]
 [Pcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
   gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask
+
+[BuildOptions]
+  XCODE:*_*_*_CC_FLAGS = -O0 -fno-lto

Then see if the issue goes away. This at leasts get you pointed in the correct direction. 


Summary for building (can compile without errors, without explicitly run "make -C BaseTools/": yes/no):
Clang 12.0: NO
Clang 9.0: NO
Clang 7.0: YES


In general clang is aggressive about promoting new warning to -Wall every release 

Thanks,

Andrew Fish

Best regards

Il giorno ven 16 ott 2020 alle ore 22:05 Andrew Fish <afish@...> ha scritto:


On Oct 16, 2020, at 12:24 PM, Daniele Crudo <crudo.daniele@...> wrote:


Sorry I missed the edk2-devel in cc, resending.

OK so the DEBUG and RELEASE both build with -Os. This is firmware things have to fit in the ROM. The big difference between DEBUG and release is the DEBUG prints get stripped out, so that usually means timing issues :(. 

The compiler flags live in  BaseTools/Conf/tools_def.template and you can edit them in Conf/tools_def.txt

ok, thanks for this, I was hoping in optimization issue but it seems it's not the case (?)

What logo are your talking about? I see the TianoCore logo failed in your macOS debug log as I would expect [1]d:
"HII Image Package with logo not found in PE/COFF resource section”
I don’t see that error message in the kali log, so that matches what I would expect. 

Yes, I was referring to the tianocore logo; I was writing that in the kali compiled log there are additional lines relating to tftpDynamicCommand.efi and LinuxInitrdDynamicShellCommand.efi, which are not in the mac compiled log.

It sounds like you are talking about a logo coming from your Hackintosh EFI boot shim? Or the macOS booter?
This is all the Hackintosh stuff that is loading. 
[Bds] Expand PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0xFFFF,0x0) -> PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0xFFFF,0x0)/HD(1,GPT,58F0B968-76EA-4F98-BD18-44DC6D57AEBA,0x800,0x7F7DF)/\EFI\BOOT\BOOTX64.EFI
Loading driver at 0x0007DDEB000 EntryPoint=0x0007DDEFE36 Bootstrap.efi
Loading driver at 0x0007DC77000 EntryPoint=0x0007DCED3A5 OpenCore.efi
Loading driver at 0x0007DDDA000 EntryPoint=0x0007DDDA5C9 AE4C11C8-1D6C-F24E-A183-E1CA36D1A8A9.efi
Loading driver at 0x0007F88E000 EntryPoint=0x0007F8920BD OpenRuntime.efi
Loading driver at 0x0007DD45000 EntryPoint=0x0007DD56C4B OpenCanopy.efi
Loading driver at 0x0007DD66000 EntryPoint=0x0007DD6FC53 AudioDxe.efi
Loading driver at 0x0007DDCE000 EntryPoint=0x0007DDD17C9 UsbMouseDxe.efi
Loading driver at 0x0007D054000 EntryPoint=0x0007D0A220C apfs.efi

And this is the macOS booter (1st stage OS loader, the thing Mac EFI firmware loads directly) 
Loading driver at 0x0007CE48000 EntryPoint=0x0007CE51673 boot.efi
 
Yes, I know and this is expected behavior, I was not referring to that logo.

There was some work on a CLANGPDB toolchain that worked on all platforms. 

Thanks for this, I rapidly had a look at the github page; so these commands:
$ cd edk2
$ git clean -ffdx
$ git reset --hard
$ git submodule deinit --force --all
$ git checkout edk2-stable202008
$ git checkout CLANGPDB
$ git submodule update --init --force
$ source edksetup.sh
$ export CLANG_BIN=~/my/local/path/to/clang+llvm-9.0.0-x86_64-darwin-apple/bin/
$ build -a X64 -b RELEASE -p OvmfPkg/OvmfPkgX64.dsc -t CLANGPDB

will let me install the clang 9.0 toolchain in my mac os and build edk2 with that toolchain?

So it sounds like you have some kind of graphics issue on a RELEASE build? What happens if on a RELEASE XCODE build you just boot to the UEFI Shell?

Maybe I misunderstood, but I cannot boot to a shell, as soon as I type "virsh start mymv" the screen goes black (with hdmi unplugged).


I’m wondering what happens if you boot another guest OS. At least on OVMF if you don’t have a guest OS to boot then you drop into an UEFI Shell (EFI Application) that is built into the EFI ROM. 

Thanks,

Andrew Fish




Re: Issues with edk2 compilation with xcode5 on mac OS - files compiled but strange behaviors

Daniele Crudo
 

Finally I found the culprit.

I tested also different toolchains; here the details.

On Big Sur beta 10:

Xcode 12.0.1
Build version 12A7300

1- Toolchain 1:
Apple clang version 12.0.0 (clang-1200.0.32.2)
Target: x86_64-apple-darwin20.1.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

2- Toolchain 2:
clang version 9.0.0 (git://github.com/llvm/llvm-project.git 0399d5a9682b3cef71c653373e38890c63c4c365)
Target: x86_64-apple-darwin20.1.0
Thread model: posix
InstalledDir: /Users/daniele/Downloads/clang+llvm-9.0.0-x86_64-darwin-apple/bin

On Catalina 10.15.7:

Xcode 12.0.1
Build version 12A7300

1- Toolchain 1:
Apple clang version 12.0.0 (clang-1200.0.32.2)
Target: x86_64-apple-darwin19.6.0
Thread model: posix
InstalledDir: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin

2- Toolchain 2:
clang version 9.0.0 (git://github.com/llvm/llvm-project.git 0399d5a9682b3cef71c653373e38890c63c4c365)
Target: x86_64-apple-darwin19.6.0
Thread model: posix
InstalledDir: /Users/daniele/Downloads/clang+llvm-9.0.0-x86_64-darwin-apple/bin

3- Toolchain 3:
Apple clang version 7.0.0  (based on LLVM 7.0.0)
Target: x86_64-apple-darwin19.6.0
Thread model: posix
InstalledDir: /Library/Developer/Toolchains/swift-latest.xctoolchain/usr/bin
------------------------------------------------------------------------------

I noticed that the issue was with Catalina and Toolchain 3.
Commands to compile the files with GPU issue (Toolchain 3):

$ git clone https://github.com/tianocore/edk2.git
$ cd edk2
$ git clean -ffdx
$ git reset --hard
$ git submodule deinit --force --all
$ git checkout edk2-stable202008
$ git submodule update --init --force
$ source edksetup.sh
$ nice make -C "$EDK_TOOLS_PATH" -j $(getconf _NPROCESSORS_ONLN)
$ build -a X64 -b RELEASE -p OvmfPkg/OvmfPkgX64.dsc -t XCODE5

These commands build files successfully, but OVMF_VARS.fd and OVMF_CODE.fd give the described issue (cannot boot if hdmi is not plugged in).

Other toolchains (both on Big Sur and Catalina), with the same commands reported above, give an error if:

$ make -C BaseTools/

is not run after:

$ git submodule update --init --force

All the other toolchains build files without the described issue (I can boot with only dvi attached).

So, I thought, let's run:

$ make -C BaseTools/

with toolchain 3 (clang 7, on Catalina) and boom, I'm able to boot with only dvi attached.

I'm not sure where the problem is, because if you take a look at the first mac os log I attached (where I did not explicitly run $ make -C BaseTools/), BaseTools seem to have compiled:

"Finished building BaseTools C Tools with HOST_ARCH=X64"

Anyway, if I don't explicitly run:

$ make -C BaseTools/

OVMF_CODE.fd and OVMF_VARS.fd don't work properly.

Summary for tianocore logo (logo is shown: yes/no):

Clang 12.0: NO
Clang 9.0: YES
Clang 7.0: YES

Summary for building (can compile without errors, without explicitly run "make -C BaseTools/": yes/no):
Clang 12.0: NO
Clang 9.0: NO
Clang 7.0: YES

Best regards

Il giorno ven 16 ott 2020 alle ore 22:05 Andrew Fish <afish@...> ha scritto:


On Oct 16, 2020, at 12:24 PM, Daniele Crudo <crudo.daniele@...> wrote:


Sorry I missed the edk2-devel in cc, resending.

OK so the DEBUG and RELEASE both build with -Os. This is firmware things have to fit in the ROM. The big difference between DEBUG and release is the DEBUG prints get stripped out, so that usually means timing issues :(. 

The compiler flags live in  BaseTools/Conf/tools_def.template and you can edit them in Conf/tools_def.txt

ok, thanks for this, I was hoping in optimization issue but it seems it's not the case (?)

What logo are your talking about? I see the TianoCore logo failed in your macOS debug log as I would expect [1]d:
"HII Image Package with logo not found in PE/COFF resource section”
I don’t see that error message in the kali log, so that matches what I would expect. 

Yes, I was referring to the tianocore logo; I was writing that in the kali compiled log there are additional lines relating to tftpDynamicCommand.efi and LinuxInitrdDynamicShellCommand.efi, which are not in the mac compiled log.

It sounds like you are talking about a logo coming from your Hackintosh EFI boot shim? Or the macOS booter?
This is all the Hackintosh stuff that is loading. 
[Bds] Expand PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0xFFFF,0x0) -> PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0xFFFF,0x0)/HD(1,GPT,58F0B968-76EA-4F98-BD18-44DC6D57AEBA,0x800,0x7F7DF)/\EFI\BOOT\BOOTX64.EFI
Loading driver at 0x0007DDEB000 EntryPoint=0x0007DDEFE36 Bootstrap.efi
Loading driver at 0x0007DC77000 EntryPoint=0x0007DCED3A5 OpenCore.efi
Loading driver at 0x0007DDDA000 EntryPoint=0x0007DDDA5C9 AE4C11C8-1D6C-F24E-A183-E1CA36D1A8A9.efi
Loading driver at 0x0007F88E000 EntryPoint=0x0007F8920BD OpenRuntime.efi
Loading driver at 0x0007DD45000 EntryPoint=0x0007DD56C4B OpenCanopy.efi
Loading driver at 0x0007DD66000 EntryPoint=0x0007DD6FC53 AudioDxe.efi
Loading driver at 0x0007DDCE000 EntryPoint=0x0007DDD17C9 UsbMouseDxe.efi
Loading driver at 0x0007D054000 EntryPoint=0x0007D0A220C apfs.efi

And this is the macOS booter (1st stage OS loader, the thing Mac EFI firmware loads directly) 
Loading driver at 0x0007CE48000 EntryPoint=0x0007CE51673 boot.efi
 
Yes, I know and this is expected behavior, I was not referring to that logo.

There was some work on a CLANGPDB toolchain that worked on all platforms. 

Thanks for this, I rapidly had a look at the github page; so these commands:
$ cd edk2
$ git clean -ffdx
$ git reset --hard
$ git submodule deinit --force --all
$ git checkout edk2-stable202008
$ git checkout CLANGPDB
$ git submodule update --init --force
$ source edksetup.sh
$ export CLANG_BIN=~/my/local/path/to/clang+llvm-9.0.0-x86_64-darwin-apple/bin/
$ build -a X64 -b RELEASE -p OvmfPkg/OvmfPkgX64.dsc -t CLANGPDB

will let me install the clang 9.0 toolchain in my mac os and build edk2 with that toolchain?

So it sounds like you have some kind of graphics issue on a RELEASE build? What happens if on a RELEASE XCODE build you just boot to the UEFI Shell?

Maybe I misunderstood, but I cannot boot to a shell, as soon as I type "virsh start mymv" the screen goes black (with hdmi unplugged).


I’m wondering what happens if you boot another guest OS. At least on OVMF if you don’t have a guest OS to boot then you drop into an UEFI Shell (EFI Application) that is built into the EFI ROM. 

Thanks,

Andrew Fish



Re: CI test on copyright

Sean
 

ECC stuff was done by Intel. I would defer to them.

Adding shenglei.zhang@...

On 10/15/2020 9:21 AM, Abner Chang wrote:
Hi Sean and Bret,
I got the CI test error which says the first line in file header section must have the copyright information, however the copyright is there and looks to me fine as below,
(C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
[cid:image001.png@...]
The interesting part is CI seems happy with below format of copyright, and this issue only happens on INF file but not on the *.c and *.h.
Copyright (C) 2020 Hewlett Packard Enterprise Development LP<BR>
Any idea on this?
Abner


Re: [PATCH v5 0/6] Extend Last Attempt Status Usage

Michael Kubacki
 

Hi Mike,

These changes will be made on top of v5.

Thanks,
Michael

On 10/16/2020 3:57 PM, Michael D Kinney wrote:
Hi Michael,
Thank you for the updates. This feedback was meant for V5, and not V4.
A couple minor comments:
1) FmpDevicePkg/Library/FmpDeviceLibNull
In order to demonstrate the preferred implementation I think we should
have FmpDeviceCheckImage() call FmpDeviceCheckImageWithStatus() and
FmpDeviceSetImage() call FmpDeviceSetImageWithStatus(). This way, it
will be clear to FmpDeviceLib developers that they only need to
implement the *WithStatus() version.
2) FmpDevicePkg/PrivateInclude/FmpLastAttemptStatus.h
FmpDependencyCheckLib comment block should use /// instead of // to be
consistent with the rest of the include file.
Thanks,
Mike

-----Original Message-----
From: @michael.kubacki <@michael.kubacki>
Sent: Thursday, October 15, 2020 2:11 PM
To: devel@edk2.groups.io
Cc: Liming Gao <gaoliming@...>; Kinney, Michael D <michael.d.kinney@...>; Jiang, Guomin <guomin.jiang@...>;
Xu, Wei6 <wei6.xu@...>; Liu, Zhiguang <zhiguang.liu@...>
Subject: [PATCH v5 0/6] Extend Last Attempt Status Usage

From: Michael Kubacki <michael.kubacki@...>

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

This patch series adds more granularity to Last Attempt Status
codes reported during FMP check image and set image operations
that greatly improve precision of the status codes.

The unsuccessful vendor range (0x1000 - 0x4000) was introduced
in UEFI Specification 2.8. At a high-level, two subranges are
defined within that range in this patch series:
1. The FMP Reserved range - reserved for components implemented
in FmpDevicePkg.
2. The FMP Device Library Reserved range - reserved for
FmpDeviceLib instance-specific usage.

The ranges are described in a public header file LastAttemptStatus.h
while the specific codes used within FmpDevicePkg implementation
are defined in a private header file FmpLastAttemptStatus.h.

FmpDeviceLib instances should use the range definition from the
public header file to define Last Attempt Status codes local to
their library instance.

Of note, there's multiple approaches to assigning private status
codes in the FMP Reserved range. For example, individual components
could define their last attempt status codes locally with the
range allocated to the component defined in a package-wide private
header file. However, one goal of the granularity being introduced
is to provide straightforward traceability to an error source.

For that reason, it was chosen to define a constant set of codes at
the package level in FmpLastAttemptStatus.h. For example, if a new
FmpDependencyLib instance is added, it would not be able to reassign
status code values in the pre-existing FMP Dependency range; it
would reuse codes for the same error source and be able to add new
codes onto the range for its usage.

V5 changes:
1. Fixed an issue where
LAST_ATTEMPT_STATUS_DRIVER_ERROR_INVALID_CERTIFICATE is changed
to LAST_ATTEMPT_STATUS_DRIVER_ERROR_IMAGE_AUTH_FAILURE in the
logic to return the last attempt status code in
CheckTheImageInternal().

V4 changes:
1. Simplified range value definitions in LastAttemptStatus.h.
Directly assign the values in the macro definition instead
of using calculations.
2. Adjusted range sizes to leave more room for future expansion.

OLD:
START | END | Usage
------------------------------------------------|
0x1000 | 0x1FFF | FmpDevicePkg |
0x1000 | 0x107F | FmpDxe driver |
0x1080 | 0x109F | FMP dependency Libs |
0x2000 | 0x3FFF | FmpDeviceLib instances |

NEW:
START | END | Usage
----------------------------------------------------------------|
0x1000 | 0x17FF | FmpDevicePkg |
0x1000 | 0x107F | FmpDxe driver |
0x1080 | 0x109F | FmpDependencyLib |
0x10A0 | 0x10BF | FmpDependencyCheckLib |
0x10C0 | 0x17FF | Unused. Available for future expansion. |
0x1800 | 0x1FFF | FmpDeviceLib instances implementation |
0x2000 | 0x3FFF | Unused. Available for future expansion. |

3. Broke the single range in v3 for FMP Dependency libraries into
separate ranges.
4. Clarified LastAttemptStatus return values in each function
description.
5. Returned an expected LastAttemptStatus value for some functions
that previously did not return a value.
6. Reverted changes in FmpDxe to call the new FmpDeviceLib APIs
for FmpDeviceCheckImage () and FmpDeviceSetImage (). These will
be added in a future series after impacted platforms in
edk2-platforms are updated to use the new APIs.
7. Instead of directly changing the pre-existing APIs in
FmpDeviceLib to add a LastAttemptStatus parameter, the new
functions were added to the library interface:
* FmpDeviceCheckImageWithStatus ()
* FmpDeviceSetImageWithStatus ()

V3 changes:
1. Enhanced range definitions in LastAttemptStatus.h with more
completeness providing length, min, and max values.
2. Moved the actual Last Attempt Status code assignments to a
private header file PrivateInclude/FmpLastAttemptStatus.h.
3. Changed the value of
LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL_VENDOR_RANGE_MAX
to 0x3FFF instead of 0x4000 even though 0x4000 is defined in
the UEFI specification. The length is 0x4000 but the max
allowed value should be 0x3FFF. This change was made now to
prevent implementation compatibility issues in the future.
4. Included "DEVICE" in the following macro name to clearly
associate it with the FmpDeviceLib library class:
LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_ERROR_xxx
5. Included a map to help the reader better visualize the range
definitions in LastAttemptStatus.h.
6. Included additional documentation describing the enum in
FmpLastAttemptStatus.h. An explicit statement stating that new
codes should be added onto the end of ranges to preserve the
values was added.
7. Simplified error handling logic in FmpDxe for FmpDeviceLib
calls that return Last Attempt Status.
8. V2 had a single memory allocation failure code used for
different memory allocations in CheckFmpDependency () in
FmpDependencyLib. Each potential allocation failure was
assigned a unique code.

V2 changes:
1. Consolidate all previous incremental updates to
LastAttemptStatus.h into one patch (patch 2)
2. Move LastAttemptStatus.h from Include to PrivateInclude
3. Correct patch 1 subject from "FmpDevicePkg" to "MdePkg"

Cc: Liming Gao <gaoliming@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Guomin Jiang <guomin.jiang@...>
Cc: Wei6 Xu <wei6.xu@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>

Michael Kubacki (6):
MdePkg/SystemResourceTable.h: Add vendor range values
FmpDevicePkg: Add Last Attempt Status header files
FmpDevicePkg/FmpDxe: Add check image path Last Attempt Status
capability
FmpDevicePkg/FmpDxe: Improve set image path Last Attempt Status
granularity
FmpDevicePkg: Add Last Attempt Status support to dependency libs
FmpDevicePkg/FmpDeviceLib: Add Last Attempt Status to Check/Set API

FmpDevicePkg/FmpDxe/FmpDxe.c | 146 +++++++++++++++++---
FmpDevicePkg/Library/FmpDependencyCheckLib/FmpDependencyCheckLib.c | 39 ++++--
FmpDevicePkg/Library/FmpDependencyCheckLibNull/FmpDependencyCheckLibNull.c | 14 +-
FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c | 93 +++++++++++--
FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c | 132 +++++++++++++++++-
FmpDevicePkg/Test/UnitTest/Library/FmpDependencyLib/EvaluateDependencyUnitTest.c | 7 +-
FmpDevicePkg/FmpDxe/FmpDxe.h | 4 +-
FmpDevicePkg/Include/LastAttemptStatus.h | 81 +++++++++++
FmpDevicePkg/Include/Library/FmpDependencyCheckLib.h | 8 +-
FmpDevicePkg/Include/Library/FmpDependencyLib.h | 44 ++++--
FmpDevicePkg/Include/Library/FmpDeviceLib.h | 121 +++++++++++++++-
FmpDevicePkg/PrivateInclude/FmpLastAttemptStatus.h | 81 +++++++++++
MdePkg/Include/Guid/SystemResourceTable.h | 13 ++
13 files changed, 718 insertions(+), 65 deletions(-)
create mode 100644 FmpDevicePkg/Include/LastAttemptStatus.h
create mode 100644 FmpDevicePkg/PrivateInclude/FmpLastAttemptStatus.h

--
2.28.0.windows.1


Re: [PATCH v5 0/6] Extend Last Attempt Status Usage

Michael D Kinney
 

Hi Michael,

Thank you for the updates. This feedback was meant for V5, and not V4.

A couple minor comments:

1) FmpDevicePkg/Library/FmpDeviceLibNull

In order to demonstrate the preferred implementation I think we should
have FmpDeviceCheckImage() call FmpDeviceCheckImageWithStatus() and
FmpDeviceSetImage() call FmpDeviceSetImageWithStatus(). This way, it
will be clear to FmpDeviceLib developers that they only need to
implement the *WithStatus() version.

2) FmpDevicePkg/PrivateInclude/FmpLastAttemptStatus.h

FmpDependencyCheckLib comment block should use /// instead of // to be
consistent with the rest of the include file.

Thanks,

Mike

-----Original Message-----
From: @michael.kubacki <@michael.kubacki>
Sent: Thursday, October 15, 2020 2:11 PM
To: devel@edk2.groups.io
Cc: Liming Gao <gaoliming@...>; Kinney, Michael D <michael.d.kinney@...>; Jiang, Guomin <guomin.jiang@...>;
Xu, Wei6 <wei6.xu@...>; Liu, Zhiguang <zhiguang.liu@...>
Subject: [PATCH v5 0/6] Extend Last Attempt Status Usage

From: Michael Kubacki <michael.kubacki@...>

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

This patch series adds more granularity to Last Attempt Status
codes reported during FMP check image and set image operations
that greatly improve precision of the status codes.

The unsuccessful vendor range (0x1000 - 0x4000) was introduced
in UEFI Specification 2.8. At a high-level, two subranges are
defined within that range in this patch series:
1. The FMP Reserved range - reserved for components implemented
in FmpDevicePkg.
2. The FMP Device Library Reserved range - reserved for
FmpDeviceLib instance-specific usage.

The ranges are described in a public header file LastAttemptStatus.h
while the specific codes used within FmpDevicePkg implementation
are defined in a private header file FmpLastAttemptStatus.h.

FmpDeviceLib instances should use the range definition from the
public header file to define Last Attempt Status codes local to
their library instance.

Of note, there's multiple approaches to assigning private status
codes in the FMP Reserved range. For example, individual components
could define their last attempt status codes locally with the
range allocated to the component defined in a package-wide private
header file. However, one goal of the granularity being introduced
is to provide straightforward traceability to an error source.

For that reason, it was chosen to define a constant set of codes at
the package level in FmpLastAttemptStatus.h. For example, if a new
FmpDependencyLib instance is added, it would not be able to reassign
status code values in the pre-existing FMP Dependency range; it
would reuse codes for the same error source and be able to add new
codes onto the range for its usage.

V5 changes:
1. Fixed an issue where
LAST_ATTEMPT_STATUS_DRIVER_ERROR_INVALID_CERTIFICATE is changed
to LAST_ATTEMPT_STATUS_DRIVER_ERROR_IMAGE_AUTH_FAILURE in the
logic to return the last attempt status code in
CheckTheImageInternal().

V4 changes:
1. Simplified range value definitions in LastAttemptStatus.h.
Directly assign the values in the macro definition instead
of using calculations.
2. Adjusted range sizes to leave more room for future expansion.

OLD:
START | END | Usage
------------------------------------------------|
0x1000 | 0x1FFF | FmpDevicePkg |
0x1000 | 0x107F | FmpDxe driver |
0x1080 | 0x109F | FMP dependency Libs |
0x2000 | 0x3FFF | FmpDeviceLib instances |

NEW:
START | END | Usage
----------------------------------------------------------------|
0x1000 | 0x17FF | FmpDevicePkg |
0x1000 | 0x107F | FmpDxe driver |
0x1080 | 0x109F | FmpDependencyLib |
0x10A0 | 0x10BF | FmpDependencyCheckLib |
0x10C0 | 0x17FF | Unused. Available for future expansion. |
0x1800 | 0x1FFF | FmpDeviceLib instances implementation |
0x2000 | 0x3FFF | Unused. Available for future expansion. |

3. Broke the single range in v3 for FMP Dependency libraries into
separate ranges.
4. Clarified LastAttemptStatus return values in each function
description.
5. Returned an expected LastAttemptStatus value for some functions
that previously did not return a value.
6. Reverted changes in FmpDxe to call the new FmpDeviceLib APIs
for FmpDeviceCheckImage () and FmpDeviceSetImage (). These will
be added in a future series after impacted platforms in
edk2-platforms are updated to use the new APIs.
7. Instead of directly changing the pre-existing APIs in
FmpDeviceLib to add a LastAttemptStatus parameter, the new
functions were added to the library interface:
* FmpDeviceCheckImageWithStatus ()
* FmpDeviceSetImageWithStatus ()

V3 changes:
1. Enhanced range definitions in LastAttemptStatus.h with more
completeness providing length, min, and max values.
2. Moved the actual Last Attempt Status code assignments to a
private header file PrivateInclude/FmpLastAttemptStatus.h.
3. Changed the value of
LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL_VENDOR_RANGE_MAX
to 0x3FFF instead of 0x4000 even though 0x4000 is defined in
the UEFI specification. The length is 0x4000 but the max
allowed value should be 0x3FFF. This change was made now to
prevent implementation compatibility issues in the future.
4. Included "DEVICE" in the following macro name to clearly
associate it with the FmpDeviceLib library class:
LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_ERROR_xxx
5. Included a map to help the reader better visualize the range
definitions in LastAttemptStatus.h.
6. Included additional documentation describing the enum in
FmpLastAttemptStatus.h. An explicit statement stating that new
codes should be added onto the end of ranges to preserve the
values was added.
7. Simplified error handling logic in FmpDxe for FmpDeviceLib
calls that return Last Attempt Status.
8. V2 had a single memory allocation failure code used for
different memory allocations in CheckFmpDependency () in
FmpDependencyLib. Each potential allocation failure was
assigned a unique code.

V2 changes:
1. Consolidate all previous incremental updates to
LastAttemptStatus.h into one patch (patch 2)
2. Move LastAttemptStatus.h from Include to PrivateInclude
3. Correct patch 1 subject from "FmpDevicePkg" to "MdePkg"

Cc: Liming Gao <gaoliming@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Guomin Jiang <guomin.jiang@...>
Cc: Wei6 Xu <wei6.xu@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>

Michael Kubacki (6):
MdePkg/SystemResourceTable.h: Add vendor range values
FmpDevicePkg: Add Last Attempt Status header files
FmpDevicePkg/FmpDxe: Add check image path Last Attempt Status
capability
FmpDevicePkg/FmpDxe: Improve set image path Last Attempt Status
granularity
FmpDevicePkg: Add Last Attempt Status support to dependency libs
FmpDevicePkg/FmpDeviceLib: Add Last Attempt Status to Check/Set API

FmpDevicePkg/FmpDxe/FmpDxe.c | 146 +++++++++++++++++---
FmpDevicePkg/Library/FmpDependencyCheckLib/FmpDependencyCheckLib.c | 39 ++++--
FmpDevicePkg/Library/FmpDependencyCheckLibNull/FmpDependencyCheckLibNull.c | 14 +-
FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c | 93 +++++++++++--
FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c | 132 +++++++++++++++++-
FmpDevicePkg/Test/UnitTest/Library/FmpDependencyLib/EvaluateDependencyUnitTest.c | 7 +-
FmpDevicePkg/FmpDxe/FmpDxe.h | 4 +-
FmpDevicePkg/Include/LastAttemptStatus.h | 81 +++++++++++
FmpDevicePkg/Include/Library/FmpDependencyCheckLib.h | 8 +-
FmpDevicePkg/Include/Library/FmpDependencyLib.h | 44 ++++--
FmpDevicePkg/Include/Library/FmpDeviceLib.h | 121 +++++++++++++++-
FmpDevicePkg/PrivateInclude/FmpLastAttemptStatus.h | 81 +++++++++++
MdePkg/Include/Guid/SystemResourceTable.h | 13 ++
13 files changed, 718 insertions(+), 65 deletions(-)
create mode 100644 FmpDevicePkg/Include/LastAttemptStatus.h
create mode 100644 FmpDevicePkg/PrivateInclude/FmpLastAttemptStatus.h

--
2.28.0.windows.1


Re: Should we have an AsciiStrnSizeS

Yao, Jiewen
 

Thanks Tim.

I think we have ASCII version - AsciiStrnSizeS and AsciiStrnLenS. :-)

Please see - https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BaseLib/SafeString.c#L1702

 

Thank you

Yao Jiewen

 

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Tim Lewis
Sent: Saturday, October 17, 2020 6:11 AM
To: devel@edk2.groups.io
Subject: [edk2-devel] Should we have an AsciiStrnSizeS

 

We have StrnSizeS, and we have StrnLenS, but not AsciiStrnSizeS.

 

Thanks,

 

Tim


Re: [Patch] BaseTools: Fix PcdValueInit tool build issue with VS compiler x64

Sean
 

Reviewed-by: Sean Brogan <sean.brogan@...>

On 10/15/2020 4:20 AM, Bob Feng wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3001
When the 64-bit version of VS compiler is used, the generated
PcdValueInit tool will be failed to compile.
This patch is going to fix that issue.
Signed-off-by: Bob Feng <bob.c.feng@...>
Cc: Liming Gao <gaoliming@...>
Cc: Yuwei Chen <yuwei.chen@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Sean Brogan <sean.brogan@...>
---
BaseTools/Source/C/Common/PcdValueCommon.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/BaseTools/Source/C/Common/PcdValueCommon.h b/BaseTools/Source/C/Common/PcdValueCommon.h
index cfd3bb76e1..1652bd5430 100644
--- a/BaseTools/Source/C/Common/PcdValueCommon.h
+++ b/BaseTools/Source/C/Common/PcdValueCommon.h
@@ -12,11 +12,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#include <Common/UefiBaseTypes.h>
#include <Common/UefiInternalFormRepresentation.h>
#define __FIELD_SIZE(TYPE, Field) (sizeof((TYPE *)0)->Field)
#define __ARRAY_ELEMENT_SIZE(TYPE, Field) (sizeof((TYPE *)0)->Field[0])
-#define __OFFSET_OF(TYPE, Field) ((UINT32) &(((TYPE *)0)->Field))
+#define __OFFSET_OF(TYPE, Field) ((UINT32)(size_t) &(((TYPE *)0)->Field))
#define __FLEXIBLE_SIZE(Size, TYPE, Field, MaxIndex) if (__FIELD_SIZE(TYPE, Field) == 0) Size = MAX((__OFFSET_OF(TYPE, Field) + __ARRAY_ELEMENT_SIZE(TYPE, Field) * (MaxIndex)), Size)
#define __ARRAY_SIZE(Array) (sizeof(Array)/sizeof(Array[0]))
#if defined(_MSC_EXTENSIONS)
#define __STATIC_ASSERT static_assert


Should we have an AsciiStrnSizeS

Tim Lewis
 

We have StrnSizeS, and we have StrnLenS, but not AsciiStrnSizeS.

 

Thanks,

 

Tim


Re: Issues with edk2 compilation with xcode5 on mac OS - files compiled but strange behaviors

Andrew Fish
 



On Oct 16, 2020, at 12:24 PM, Daniele Crudo <crudo.daniele@...> wrote:


Sorry I missed the edk2-devel in cc, resending.

OK so the DEBUG and RELEASE both build with -Os. This is firmware things have to fit in the ROM. The big difference between DEBUG and release is the DEBUG prints get stripped out, so that usually means timing issues :(. 

The compiler flags live in  BaseTools/Conf/tools_def.template and you can edit them in Conf/tools_def.txt

ok, thanks for this, I was hoping in optimization issue but it seems it's not the case (?)

What logo are your talking about? I see the TianoCore logo failed in your macOS debug log as I would expect [1]d:
"HII Image Package with logo not found in PE/COFF resource section”
I don’t see that error message in the kali log, so that matches what I would expect. 

Yes, I was referring to the tianocore logo; I was writing that in the kali compiled log there are additional lines relating to tftpDynamicCommand.efi and LinuxInitrdDynamicShellCommand.efi, which are not in the mac compiled log.

It sounds like you are talking about a logo coming from your Hackintosh EFI boot shim? Or the macOS booter?
This is all the Hackintosh stuff that is loading. 
[Bds] Expand PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0xFFFF,0x0) -> PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0xFFFF,0x0)/HD(1,GPT,58F0B968-76EA-4F98-BD18-44DC6D57AEBA,0x800,0x7F7DF)/\EFI\BOOT\BOOTX64.EFI
Loading driver at 0x0007DDEB000 EntryPoint=0x0007DDEFE36 Bootstrap.efi
Loading driver at 0x0007DC77000 EntryPoint=0x0007DCED3A5 OpenCore.efi
Loading driver at 0x0007DDDA000 EntryPoint=0x0007DDDA5C9 AE4C11C8-1D6C-F24E-A183-E1CA36D1A8A9.efi
Loading driver at 0x0007F88E000 EntryPoint=0x0007F8920BD OpenRuntime.efi
Loading driver at 0x0007DD45000 EntryPoint=0x0007DD56C4B OpenCanopy.efi
Loading driver at 0x0007DD66000 EntryPoint=0x0007DD6FC53 AudioDxe.efi
Loading driver at 0x0007DDCE000 EntryPoint=0x0007DDD17C9 UsbMouseDxe.efi
Loading driver at 0x0007D054000 EntryPoint=0x0007D0A220C apfs.efi

And this is the macOS booter (1st stage OS loader, the thing Mac EFI firmware loads directly) 
Loading driver at 0x0007CE48000 EntryPoint=0x0007CE51673 boot.efi
 
Yes, I know and this is expected behavior, I was not referring to that logo.

There was some work on a CLANGPDB toolchain that worked on all platforms. 

Thanks for this, I rapidly had a look at the github page; so these commands:
$ cd edk2
$ git clean -ffdx
$ git reset --hard
$ git submodule deinit --force --all
$ git checkout edk2-stable202008
$ git checkout CLANGPDB
$ git submodule update --init --force
$ source edksetup.sh
$ export CLANG_BIN=~/my/local/path/to/clang+llvm-9.0.0-x86_64-darwin-apple/bin/
$ build -a X64 -b RELEASE -p OvmfPkg/OvmfPkgX64.dsc -t CLANGPDB

will let me install the clang 9.0 toolchain in my mac os and build edk2 with that toolchain?

So it sounds like you have some kind of graphics issue on a RELEASE build? What happens if on a RELEASE XCODE build you just boot to the UEFI Shell?

Maybe I misunderstood, but I cannot boot to a shell, as soon as I type "virsh start mymv" the screen goes black (with hdmi unplugged).


I’m wondering what happens if you boot another guest OS. At least on OVMF if you don’t have a guest OS to boot then you drop into an UEFI Shell (EFI Application) that is built into the EFI ROM. 

Thanks,

Andrew Fish