Date   

Re: [Patch V2] MinPlatformPkg: Check if Acpi table is already installed.

Dong, Eric
 

Reviewed-by: Eric Dong <eric.dong@...>

-----Original Message-----
From: Sravanthi, K KavyaX <k.kavyax.sravanthi@...>
Sent: Monday, December 6, 2021 2:16 PM
To: devel@edk2.groups.io
Cc: Sravanthi, K KavyaX <k.kavyax.sravanthi@...>; Chiu, Chasel <chasel.chiu@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Liming Gao <gaoliming@...>; Dong, Eric <eric.dong@...>
Subject: [Patch V2] MinPlatformPkg: Check if Acpi table is already installed.

Check if Acpi table is already installed by locating the first ACPI table in XSDT/RSDT based on Signature

Cc: Chasel Chiu <chasel.chiu@...>
Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Liming Gao <gaoliming@...>
Cc: Eric Dong <eric.dong@...>
Signed-off-by: kavya <k.kavyax.sravanthi@...>
Reviewed-by: Zhiguang Liu <zhiguang.liu@...>
---
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)

diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index 785cf4c2f9..dcbb8b7c7f 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -1048,12 +1048,21 @@ InstallMcfgFromScratch ( {
EFI_STATUS Status;
EFI_ACPI_MEMORY_MAPPED_CONFIGURATION_BASE_ADDRESS_TABLE_HEADER *McfgTable;
+ EFI_ACPI_COMMON_HEADER *Mcfg;
EFI_ACPI_MEMORY_MAPPED_ENHANCED_CONFIGURATION_SPACE_BASE_ADDRESS_ALLOCATION_STRUCTURE *Segment;
UINTN Index;
UINTN SegmentCount;
PCI_SEGMENT_INFO *PciSegmentInfo;
UINTN TableHandle;

+ Mcfg = (EFI_ACPI_COMMON_HEADER *) EfiLocateFirstAcpiTable (
+ EFI_ACPI_3_0_PCI_EXPRESS_MEMORY_MAPPED_CONFIGURATION_SPACE_BASE_ADDRESS_DESCRIPTION_TABLE_SIGNATURE
+ ); if (Mcfg != NULL) {
+ DEBUG ((DEBUG_INFO, "MCFG table already installed\n"));
+ return EFI_SUCCESS;
+ }
+
PciSegmentInfo = GetPciSegmentInfo (&SegmentCount);

McfgTable = AllocateZeroPool (
@@ -1365,6 +1374,7 @@ UpdateLocalTable ( {
EFI_STATUS Status;
EFI_ACPI_COMMON_HEADER *CurrentTable;
+ EFI_ACPI_COMMON_HEADER *Table;
EFI_ACPI_TABLE_VERSION Version;
UINTN TableHandle;
UINTN Index;
@@ -1372,6 +1382,14 @@ UpdateLocalTable (
for (Index = 0; Index < sizeof(mLocalTable)/sizeof(mLocalTable[0]); Index++) {
CurrentTable = mLocalTable[Index];

+ Table = (EFI_ACPI_COMMON_HEADER *) EfiLocateFirstAcpiTable (CurrentTable->Signature);
+ if (Table != NULL) {
+ DEBUG ((DEBUG_INFO, "Acpi table with signature=%c%c%c%c already installed\n",
+ ((CHAR8*)&CurrentTable->Signature)[0], ((CHAR8*)&CurrentTable->Signature)[1],
+ ((CHAR8*)&CurrentTable->Signature)[2], ((CHAR8*)&CurrentTable->Signature)[3]));
+ continue;
+ }
+
PlatformUpdateTables (CurrentTable, &Version);

TableHandle = 0;
--
2.16.2.windows.1


Re: [PATCH V3 23/29] UefiCpuPkg: Update AddressEncMask in CpuPageTable

Gerd Hoffmann
 

Hi,

Looks like two PCDs for basically the same thing.
Should we create a common CC PCD here?
1. The current situation of PcdPteMemoryEncryptionAddressOrMask is:
1.1 PcdPteMemoryEncryptionAddressOrMask is now set by AmdSev.
1.2 In CreateIdentityMappingPageTables(), this value (AddressEncMask) is set to the page tables in SEV guest.
1.3 This PCD is also used as an indicator in InternalMemEncryptSevStatus() if ReadSevMsr is TRUE or FALSE.
1.4 This PCD is also used in BootScriptExecutorEntryPoint()
Yes. Creating a common CC PCD may require some changes on the SEV side
too. The code (1.3 for example) assumes sev is active when
PcdPteMemoryEncryptionAddressOrMask is set, which will obviously not be
the case any more when tdx uses it too. But there are other ways to
check for sev which can be used instead ...

2. The meaning and usage scenario of PcdTdxSharedBitMask are somehow different from PcdPteMemoryEncryptionAddressOrMask.
2.1 Guest physical address (GPA) space of Td guest is divided into private and shared sub-spaces, determined by the shared bit of GPA.[1]
Well, there are some differences in detail but the underlying concept is
the same. The page table bit says whenever the page is private to the vm
or not. With SEV the bit enables/disables encryption. With TDX the bit
switches between private and shared encryption key.

2.2 PcdTdxSharedBitMask indicates the above shared bit of GPA. And
only the shared GPA has the shared bit set. This breaks 1.2.
Hmm, ok. So the logic is different. SEV enables the bit for private
pages whereas TDX enables the bit for shared pages.

Too bad. That indeed makes it impossible to share a single PCD.

We could still define something generic, like a
"set-this-bit-for-shared-pages" pcd and a
"set-this-bit-for-private-pages" pcd. But at the end of the day that
probably wouldn't be very different from having
PcdPteMemoryEncryptionAddressOrMask + PcdTdxSharedBitMask ...

take care,
Gerd


Re: [Patch V2] MinPlatformPkg: Check if Acpi table is already installed.

Ni, Ray
 

Reviewed-by: Ray Ni ray.ni@...


Re: Soft Feature Freeze will start on 2021-11-08 for edk2-stable202111

Gao, Zhichao
 

Yes. The INF file is required. Before we don’t have script to build it, we keep using the one from ShellBinPkg. I think we can add the two INF file generation to the release generation script.

Here is the content of UefiShell.inf

##  @file

#  This is the UEFI Shell application binary file.

#

#  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>

#

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

#

##

##

 

[Defines]

  INF_VERSION                    = 0x00010006

  BASE_NAME                      = Shell

  FILE_GUID                      = 7C04A583-9E3E-4f1c-AD65-E05268D0B4D1

  MODULE_TYPE                    = UEFI_APPLICATION

  VERSION_STRING                 = 1.0

 

[Binaries.Ia32]

  PE32|Ia32/Shell.efi|*

 

[Binaries.X64]

  PE32|X64/Shell.efi|*

 

[Binaries.Arm]

  PE32|Arm/Shell.efi|*

 

[Binaries.AArch64]

  PE32|AArch64/Shell.efi|*

MinUefiShell.inf

##  @file

#  This is the UEFI Shell application binary file.

#

#  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>

#

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

#

##

##

 

[Defines]

  INF_VERSION                    = 0x00010006

  BASE_NAME                      = Shell

  FILE_GUID                      = 7C04A583-9E3E-4f1c-AD65-E05268D0B4D1

  MODULE_TYPE                    = UEFI_APPLICATION

  VERSION_STRING                 = 1.0

 

[Binaries.Ia32]

  PE32|Ia32/Shell.efi|*

 

[Binaries.X64]

  PE32|X64/Shell.efi|*

 

[Binaries.Arm]

  PE32|Arm/Shell.efi|*

 

[Binaries.AArch64]

  PE32|AArch64/Shell.efi|*

==== End ====

 

For the extended shell, we can make a binary release. But that is not required, because the extended command is not record in the shell spec. The behavior is not documented. That means there is no expected result. But if there are benefits to release them, we should release them.

 

Thanks,

Zhichao

 

From: Kinney, Michael D <michael.d.kinney@...>
Sent: Monday, December 6, 2021 1:30 PM
To: Gao, Zhichao <zhichao.gao@...>; gaoliming <gaoliming@...>; devel@edk2.groups.io; rebecca@...; Kinney, Michael D <michael.d.kinney@...>
Cc: Teng, Lynn L <lynn.l.teng@...>; afish@...; leif@...
Subject: RE: [edk2-devel] Soft Feature Freeze will start on 2021-11-08 for edk2-stable202111

 

The previous release also provide an “As Built” INF file to allow the shell to be easily integrated into platform DSC files.

 

Do you think this is required?  How was that generated?

 

The ShellPkg also contains a number of command extensions.  Should those be build and included in a binary release?

 

Thanks,

 

Mike

 

From: Gao, Zhichao <zhichao.gao@...>
Sent: Sunday, December 5, 2021 9:16 PM
To: gaoliming <gaoliming@...>; Kinney, Michael D <michael.d.kinney@...>; devel@edk2.groups.io; rebecca@...
Cc: Teng, Lynn L <lynn.l.teng@...>; afish@...; leif@...
Subject: RE: [edk2-devel] Soft Feature Freeze will start on 2021-11-08 for edk2-stable202111

 

To generate Full Shell, execute:

  "build -a IA32 -a X64 -p ShellPkg\ShellPkg.dsc -b RELEASE"

To generate Minimal Shell, execute:

  "build -a IA32 -a X64 -p ShellPkg\ShellPkg.dsc -b RELEASE -D NO_SHELL_PROFILES"

 

The binary with name “Shell_7C04A583-9E3E-4f1c-AD65-E05268D0B4D1.efi" Is the one for ShellBinPkg release.

 

Thanks,

Zhichao

 

From: gaoliming <gaoliming@...>
Sent: Friday, December 3, 2021 10:08 AM
To: Kinney, Michael D <michael.d.kinney@...>; devel@edk2.groups.io; rebecca@...; Gao, Zhichao <zhichao.gao@...>
Cc: Teng, Lynn L <lynn.l.teng@...>; afish@...; leif@...
Subject:
回复: [edk2-devel] Soft Feature Freeze will start on 2021-11-08 for edk2-stable202111

 

Mike:

 Previously, ShellPkg maintainer Zhichao Gao helps to generate the binary ShellPkg.

 

Zhichao:

 Can you give the build step to generate the binary ShellPkg with Shell and MinShell?

 

Thanks

Liming

发件人: Kinney, Michael D <michael.d.kinney@...>
发送时间: 2021121 11:16
收件人: devel@edk2.groups.io; gaoliming@...; rebecca@...; Gao, Zhichao <zhichao.gao@...>; Kinney, Michael D <michael.d.kinney@...>
抄送: Teng, Lynn L <lynn.l.teng@...>; afish@...; leif@...
主题: RE: [edk2-devel] Soft Feature Freeze will start on 2021-11-08 for edk2-stable202111

 

Hi Liming,

 

I agree it would be better to have a CI agent create this shell ZIP file.

 

What was the process used to create the previous ZIP files?  When I do a shell build I see some different named files.  MinShell is very small.  What flags were used.  There are also some shell commands in the shell package.  Should we add those to the ZIP file?

 

If you give me the recipe, I can work on a CI task.  We can also add RISCV64 if that is of interest as well.

 

Thanks,

 

Mike

 

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of gaoliming
Sent: Thursday, November 11, 2021 5:02 PM
To: devel@edk2.groups.io; rebecca@...; Gao, Zhichao <zhichao.gao@...>
Cc: Kinney, Michael D <michael.d.kinney@...>; Teng, Lynn L <lynn.l.teng@...>; afish@...; leif@...
Subject:
回复: [edk2-devel] Soft Feature Freeze will start on 2021-11-08 for edk2-stable202111

 

Rebecca:

 Thanks for your reminder. Now, we have open CI. I think we can create the binary ShellPkg based on CI build result.

 

Mike:

 Can open CI upload the generated EFI image as the artifacts? If so, the binary ShellPkg can be created from CI build result.

 

Thanks

Liming

发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Rebecca Cran
发送时间: 20211112 7:44
收件人: devel@edk2.groups.io; gaoliming@...
抄送: 'Kinney, Michael D' <michael.d.kinney@...>; 'Teng, Lynn L' <lynn.l.teng@...>; afish@...; leif@...
主题: Re: [edk2-devel] Soft Feature Freeze will start on 2021-11-08 for edk2-stable202111

 

Do we have a task or reminder to generate the UEFI Shell binaries for this release, and to upload ShellBinPkg.zip to Github?

https://bugzilla.tianocore.org/show_bug.cgi?id=3178 was opened but apparently never resolved, and the latest ShellBinPkg.zip is still from the edk2-stable202002 release.

 

--
Rebecca Cran

 

On 11/4/21 11:53 PM, gaoliming wrote:

Hi, all

We will enter into Soft Feature Freeze phase on 2021-11-08. In this phase, the feature under review will not be allowed to be pushed. The feature passed review can still be merged.

 

The patch review can continue without break in edk2 community. If the patch is sent before Soft Feature Freeze, and plans to catch this stable tag, the patch contributor need reply to his patch and notify edk2 community. If the patch is sent after Soft Feature Freeze, and plans to catch this stable tag, please add edk2-stable202111 key words in the patch title and BZ, so the community know this patch target and give the feedback.

 

Below is edk2-stable202111 tag planning https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning Proposed Schedule

Date (00:00:00 UTC-8) Description

2021-08-30  Beginning of development

2021-11-08  Soft Feature Freeze

2021-11-12  Hard Feature Freeze

2021-11-26  Release

 

Thanks

Liming

 


回复: [edk2-devel] Event: TianoCore Bug Triage - APAC / NAMO - 12/07/2021 #cal-reminder

gaoliming
 

The following issues will be reviewed this week meeting.

 

3721

EDK2

Code

unassigned@...

UNCO

Action is set to 'untested' if image is unsigned and hash not found in DB/DBX

Mon 04:21

Joseph.Hemann@...

3763

EDK2

Code

unassigned@...

UNCO

Cond expression logic is not functioning as expected in VFR

Fri 13:05

kathappan.esakkithevar@inte...

2120

Tianocor

Code

michael.d.kinney@...

UNCO

[PeCoffLib] Provide flexibility for platform to disable TE image support

Fri 12:54

hao.a.wu@...

3331

EDK2

Code

unassigned@...

UNCO

Treat memory protection attributes separately

Fri 02:30

mhaeuser@...

3326

EDK2

Code

mhaeuser@...

UNCO

Enforce W^X design principles

Fri 02:29

mhaeuser@...

3329

EDK2

Code

mhaeuser@...

UNCO

DxeCore: No support for granular PE section permissions

Fri 02:26

mhaeuser@...

3761

EDK2

Code

unassigned@...

UNCO

AhciPei doesn't detect CD ROM device

Thu 22:47

gaoliming@...

3758

EDK2

Code

unassigned@...

UNCO

MemoryAllocationLib fails in clang release builds

Thu 08:02

ross.burton@...

3755

EDK2

Code

unassigned@...

UNCO

Displaying SMBIOS Type38 fields in smbiosview command in formatted manner

2021-11-30

sainadhn@...

3751

EDK2

Code

unassigned@...

UNCO

Check routine from Mm Communication from Arm package fails unnecessarily

2021-11-29

kun.qin@...

3753

EDK2

Code

aaron.li@...

UNCO

BuildAndIntegrationInstructions should contain solution for a common coreboot integration issue

2021-11-29

aaron.li@...

 

Thanks

Liming

发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 devel@edk2.groups.io Calendar
发送时间: 2021127 10:30
收件人: devel@edk2.groups.io
主题: [edk2-devel] Event: TianoCore Bug Triage - APAC / NAMO - 12/07/2021 #cal-reminder

 

Reminder: TianoCore Bug Triage - APAC / NAMO

When:
12/07/2021
6:30pm to 7:30pm
(UTC-08:00) America/Los Angeles

Where:
https://teams.microsoft.com/l/meetup-join/19%3ameeting_OTUyZTg2NjgtNDhlNS00ODVlLTllYTUtYzg1OTNjNjdiZjFh%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%22b286b53a-1218-4db3-bfc9-3d4c5aa7669e%22%7d

Organizer: Liming Gao gaoliming@...

View Event

Description:

TianoCore Bug Triage - APAC / NAMO

Hosted by Liming Gao

 

________________________________________________________________________________

Microsoft Teams meeting

Join on your computer or mobile app

Click here to join the meeting

Join with a video conferencing device

teams@...

Video Conference ID: 116 062 094 0

Alternate VTC dialing instructions

Or call in (audio only)

+1 916-245-6934,,77463821#   United States, Sacramento

Phone Conference ID: 774 638 21#

Find a local number | Reset PIN

Learn More | Meeting options


Re: Uncrustify Conversion Detailed Plan and Extended Hard Freeze Update #4

Chiu, Chasel
 

For below 2 packages, Reviewed-by: Chasel Chiu <chasel.chiu@...>

Thanks,
Chasel

* IntelFsp2Pkg
* IntelFsp2WrapperPkg

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael D
Kinney
Sent: Monday, December 6, 2021 9:18 AM
To: Michael Kubacki <mikuback@...>; devel@edk2.groups.io;
maciej.rabeda@...; Michael Kubacki
<michael.kubacki@...>; Andrew Fish (afish@...)
<afish@...>; Leif Lindholm <leif@...>; Kinney, Michael D
<michael.d.kinney@...>
Subject: Re: [edk2-devel] Uncrustify Conversion Detailed Plan and Extended
Hard Freeze Update #4

Hello EDK II Maintainers,

A detailed evaluation of the DEBUG_CODE() formatting issue has been
completed.
The reason DEBUG_CODE() is a challenge is that this looks like a macro from a
C parsing perspective, but the EDK II usage places C statements or blocks of
C code as the parameter to this macro.

There are actually 2 methods available to mark a statement or a block of code
to be included when the DEBUG_PROPERTY_DEBUG_CODE_ENABLED bit is
enabled in
PcdDebugPropertyMask. One is DEBUG_CODE() and the other is to mark the
beginning
and end of a code block with DEBUG_CODE_BEGIN() and DEBUG_CODE_END().
In fact,
DEBUG_CODE() is implemented using DEBUG_CODE_BEGIN() and
DEBUG_CODE_END() macros.

#define DEBUG_CODE(Expression) \
DEBUG_CODE_BEGIN (); \
Expression \
DEBUG_CODE_END ()

A complete review for the use of these DEBUG_CODE macros was performed on
the
edk2 repo. Uncrustify performs good formatting for code blocks between
DEBUG_CODE_BEGIN() and DEBUG_CODE_END(). This is because these look
like simple
macros calls with no parameters and the lines of C code between these 2
macros
is formatted correctly.

The uncrustify formatting issues are only present with the use of DEBUG_CODE().
Simple use cases of DEBUG_CODE(Expression) where Expression is a single C
statement also look correct. A medium complexity use case where Expression is
a code block of simple statements or even some local variables and simple
statements also look correct. It is only complex code blocks that use C
statements such as if/for/while/case that include the use of braces {} does
uncrustify perform incorrect formatting.

The recommended solution to this issue is to convert the use of DEBUG_CODE()
to DEBUG_CODE_BEGIN() / DEBUG_CODE_END() for cases where the
Expression
passed to DEBUG_CODE() is the complex use case that contains statements that
use braces {}. There are 57 instances of this pattern across 40 files in the
edk2 repo.

I have posted a branch with these additional patches:


https://github.com/mdkinney/edk2/tree/Bug_3737_3760_3739_ApplyUncrustif
yChanges_V7

I have performed CompareBuild tests with this revised patch series with
the DEBUG_CODE changes. It passes 100% showing no binary differences.

https://github.com/mdkinney/edk2/actions/runs/1542454606

I have opened a PR to run this patch series through EDK II CI. It also passes 100%.

https://github.com/tianocore/edk2/pull/2236

The summary of changes made since the V6 review are:

1) Change uncrustify configuration assignment alignment threshold to 0

align_assign_thresh = 0

2) Replace "<param>, OPTIONAL" with "<param> OPTIONAL,"

3) Replace DEBUG_CODE(Expression) with

DEBUG_CODE_BEGIN();
Expression
DEBUG_CODE_END()

if Expression is complex and contains braces {}.

4) No changes to uncrustify tool required.

Please review the differences between the following 2 branches and provide
feedback or a Series Reviewed-by if you agree with these additional changes.


https://github.com/mdkinney/edk2/tree/Bug_3737_3760_3739_ApplyUncrustif
yChanges_V6

https://github.com/mdkinney/edk2/tree/Bug_3737_3760_3739_ApplyUncrustif
yChanges_V7

The goal is to complete the review and get the uncrustify change committed
tomorrow so the extended hard freeze can be lifted.

Thanks,

Mike

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@...>
Sent: Thursday, December 2, 2021 6:23 PM
To: Michael Kubacki <mikuback@...>; devel@edk2.groups.io;
maciej.rabeda@...; Michael Kubacki
<michael.kubacki@...>; Andrew Fish (afish@...)
<afish@...>; Leif Lindholm <leif@...>;
Kinney, Michael D <michael.d.kinney@...>
Subject: RE: [edk2-devel] Uncrustify Conversion Detailed Plan and Extended
Hard Freeze Update #4

Hello EDK II Maintainers,

I have entered BZ 3760 to make the use of the OPTIONAL keyword style
consistent for all of edk2 repo
and to be compatible with uncrustify.

I have posted the following V6 branch that does the EFI_D_* to DEBUG_*
changes, the OPTIONAL keyword
style changes, and the uncrustify changes with the one configuration change
for assignment alignment.

https://github.com/mdkinney/edk2/tree/Bug_3737_3760_3739_ApplyUncrustif
yChanges_V6

Please provide feedback on the code style in this branch with the known
DEBUG_CODE() issue still
present.

If we are able to quickly update uncrustify to handle DEBUG_CODE(), I will
generate a V7.

Thanks,

Mike

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@...>
Sent: Thursday, December 2, 2021 4:53 PM
To: Michael Kubacki <mikuback@...>;
devel@edk2.groups.io; maciej.rabeda@...; Michael Kubacki
<michael.kubacki@...>; Andrew Fish (afish@...)
<afish@...>; Leif Lindholm <leif@...>;
Kinney, Michael D <michael.d.kinney@...>
Subject: RE: [edk2-devel] Uncrustify Conversion Detailed Plan and Extended
Hard Freeze Update #4

Michael,

Yes. Please update the patch series that adds UncrustifyCheck with those
changes.

Thanks,

Mike

-----Original Message-----
From: Michael Kubacki <mikuback@...>
Sent: Thursday, December 2, 2021 4:31 PM
To: Kinney, Michael D <michael.d.kinney@...>;
devel@edk2.groups.io; maciej.rabeda@...; Michael
Kubacki
<michael.kubacki@...>; Andrew Fish (afish@...)
<afish@...>; Leif Lindholm <leif@...>
Subject: Re: [edk2-devel] Uncrustify Conversion Detailed Plan and
Extended Hard Freeze Update #4

Hi Mike,

Thank you for the detailed analysis and recommendations.

I agree with the recommendations and I will try to have an Uncrustify
tool update by tomorrow for (4).

That will require an update in uncrustify_ext_dep.yaml to pull in the
new version. I'm assuming I should also update uncrustify.cfg to set
align_assign_thresh = 0 in that new patch series?

Regards,
Michael

On 12/2/2021 7:18 PM, Kinney, Michael D wrote:
Hi Michael,

CORRECTION: set align_assign_threshold to 0.

Reponses inline below.

I would like to summarize the 4 issues raised in the past day along with
the recommendations.

1) Exclusion feature for UncrustifyCheck. There are 2 directories with 8
files total that
maintainers have noted they would like to see not go through
uncrustify formatting. Today
the only content that is skipped is BaseTools and submodules.

Adding a general purpose exclusion feature would then require all
developers to make
sure their method of using uncrustify also excludes those same areas.
This requires
extra steps for all developers and maintainers.

If we do not add the exclusion feature, then the 8 files will require an
extra step
to sync with the original source of those files. The rate of changes of
these 8 files
is very low today.

RECOMMENDATION: Do not add exclusion feature at this time. Revisit
if the extra work
to maintain the files that would be candidates for exclusions increases
significantly.

2) Alignment of assignments. The threshold of 4 characters appears to
be too low and causes
source files that are already aligned to become unaligned.

RECOMMENDATION: Change threshold to the default value of 0 which
means no limit.

align_assign_thresh= 0

3) Alignment of parameters in function declaration not correct. The root
cause of this
is the use of the OPTIONAL keyword. If the OPTIONAL keyword is
removed, then the
alignment is correct. The alignment is also correct if the OPTIONAL
keyword appears
before the ','. If the OPTIONAL keyword appears after the ',', then the
format is
not correct. The OPTIONAL keyword indicates that the parameter in
the function is
not required and may be passed in as NULL or 0 or some other default
value defined by
the API. It makes more sense for this OPTIONAL keyword that follows
the parameter
names to appear before the ',' so it is scoped to the parameter on that
line. If it
appears after the ',', then C parsers thinks it is a prefix (IN, OUT, CONST,
volatile,
static) for the next parameter in the function.

RECOMMENDATION: Update patch series with a global search and
replace so OPTIONAL
keyword always appears before the ',' on the same line.

RegEx search string: ',( *)OPTIONAL( *)'
RegEx replace string: ' $1OPTIONAL,$2'

4) Format issues with complex blocks in DEBUG_CODE(), or between
DEBUG_CODE_BEGIN() and
DEBUG_CODE_END(). Uncrustify treats these as macros and is not
aware that the
parameter passed into the macro call is a block of C code that needs to
be formatted.
Complex blocks with if/while/for/case statements are impacted the
most.

RECOMMENDATION: Update the uncrustify with an edk2 specific
extension to treat these
macros as a block of code as if they were surrounded by an extra set of
braces {}.


I have posted a branch for testing purposes that implements (2) and (3).

Branch:
https://github.com/mdkinney/edk2/tree/TestOnly_Bug_3737_3739_ApplyUncru
stifyChanges_V6_OPTIONAL_Keyword_Fix
PR: https://github.com/tianocore/edk2/pull/2233
Status: PASS
CompareBuild:
https://github.com/mdkinney/edk2/actions/runs/1532855914
Status: PASS

You can see what changed by fetching and comparing the following 2
branches:

https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyChan
ges_V5
https://github.com/mdkinney/edk2/tree/TestOnly_Bug_3737_3739_ApplyUncru
stifyChanges_V6_OPTIONAL_Keyword_Fix

Please provide feedback on the RECOMMENDATIONS above. I will go
ahead and prepare of V6 version of
the patch series now that that test results are all PASS.

Best regards,

Mike


-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@...>
Sent: Thursday, December 2, 2021 4:15 PM
To: Michael Kubacki <mikuback@...>;
devel@edk2.groups.io; maciej.rabeda@...; Michael
Kubacki
<michael.kubacki@...>; Andrew Fish (afish@...)
<afish@...>; Leif Lindholm
<leif@...>;
Kinney, Michael D <michael.d.kinney@...>
Subject: RE: [edk2-devel] Uncrustify Conversion Detailed Plan and
Extended Hard Freeze Update #4

Hi Michael,

Reponses inline below.

I would like to summarize the 4 issues raised in the past day along with
the recommendations.

1) Exclusion feature for UncrustifyCheck. There are 2 directories with 8
files total that
maintainers have noted they would like to see not go through
uncrustify formatting. Today
the only content that is skipped is BaseTools and submodules.

Adding a general purpose exclusion feature would then require all
developers to make
sure their method of using uncrustify also excludes those same areas.
This requires
extra steps for all developers and maintainers.

If we do not add the exclusion feature, then the 8 files will require an
extra step
to sync with the original source of those files. The rate of changes of
these 8 files
is very low today.

RECOMMENDATION: Do not add exclusion feature at this time.
Revisit if the extra work
to maintain the files that would be candidates for exclusions increases
significantly.

2) Alignment of assignments. The threshold of 4 characters appears to
be too low and causes
source files that are already aligned to become unaligned.

RECOMMENDATION: Change threshold to the default value of 0
which means no limit.

align_assign_thresh= 4

3) Alignment of parameters in function declaration not correct. The
root cause of this
is the use of the OPTIONAL keyword. If the OPTIONAL keyword is
removed, then the
alignment is correct. The alignment is also correct if the OPTIONAL
keyword appears
before the ','. If the OPTIONAL keyword appears after the ',', then the
format is
not correct. The OPTIONAL keyword indicates that the parameter in
the function is
not required and may be passed in as NULL or 0 or some other default
value defined by
the API. It makes more sense for this OPTIONAL keyword that follows
the parameter
names to appear before the ',' so it is scoped to the parameter on
that line. If it
appears after the ',', then C parsers thinks it is a prefix (IN, OUT,
CONST, volatile,
static) for the next parameter in the function.

RECOMMENDATION: Update patch series with a global search and
replace so OPTIONAL
keyword always appears before the ',' on the same line.

RegEx search string: ',( *)OPTIONAL( *)'
RegEx replace string: ' $1OPTIONAL,$2'

4) Format issues with complex blocks in DEBUG_CODE(), or between
DEBUG_CODE_BEGIN() and
DEBUG_CODE_END(). Uncrustify treats these as macros and is not
aware that the
parameter passed into the macro call is a block of C code that needs
to be formatted.
Complex blocks with if/while/for/case statements are impacted the
most.

RECOMMENDATION: Update the uncrustify with an edk2 specific
extension to treat these
macros as a block of code as if they were surrounded by an extra set
of braces {}.


I have posted a branch for testing purposes that implements (2) and (3).

Branch:
https://github.com/mdkinney/edk2/tree/TestOnly_Bug_3737_3739_ApplyUncru
stifyChanges_V6_OPTIONAL_Keyword_Fix
PR: https://github.com/tianocore/edk2/pull/2233
Status: PASS
CompareBuild:
https://github.com/mdkinney/edk2/actions/runs/1532855914
Status: PASS

You can see what changed by fetching and comparing the following 2
branches:

https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyChan
ges_V5
https://github.com/mdkinney/edk2/tree/TestOnly_Bug_3737_3739_ApplyUncru
stifyChanges_V6_OPTIONAL_Keyword_Fix

Please provide feedback on the RECOMMENDATIONS above. I will go
ahead and prepare of V6 version of
the patch series now that that test results are all PASS.

Best regards,

Mike


-----Original Message-----
From: Michael Kubacki <mikuback@...>
Sent: Thursday, December 2, 2021 1:57 PM
To: devel@edk2.groups.io; Kinney, Michael D
<michael.d.kinney@...>; maciej.rabeda@...; Michael
Kubacki
<michael.kubacki@...>; Andrew Fish (afish@...)
<afish@...>; Leif Lindholm
<leif@...>
Subject: Re: [edk2-devel] Uncrustify Conversion Detailed Plan and
Extended Hard Freeze Update #4

My reply is inline.

Regards,
Michael

On 12/2/2021 2:45 PM, Michael D Kinney wrote:
Hi Maciej,

Thanks for the feedback.

* Example #1.This appears to be caused by the following uncrustify
settings:

# The threshold for aligning on '=' in assignments.

# Use a negative number for absolute thresholds.

#

# 0: No limit (default).

align_assign_thresh= 0# number

The edk2 setting for this is:

align_assign_thresh= 4

This means blocks of assignments that are different than more than 4
spaces will be considered a new block.

‘HwAddreLen’ and ‘Xid’ are more than 4 characters in length
different.Same for ‘Xid’ and ‘Reserved’.So

uncrustify treats these as 3 different assignment blocks.

If we change to the default value of 0: No limit, this example is
treated as a single block and all ‘=’ are aligned.

Is there a reason ‘4’ was selected?Is there is any harm in using the
default of 0?
We can certainly change the threshold. '4' was derived by
experimentation. This is an area that is somewhat subjective and every
case is difficult to cover well. Please feel free to suggest any changes.
I recommend we use the default value of 0. That way, any code that
choose to
align assignments will still be aligned. If a developer does not like
short assignments and long assignments in the same code block to use
the
long assignment column, they can always break the block up into
multiple
blocks by adding a carriage return.

For the benefit of others, this file can be a useful refernce
understanding spans, gaps, and thresholds.

https://github.com/uncrustify/uncrustify/blob/master/documentation/htdocs/c
onfiguration.txt

* Example #2: Uncruistfy is confused by the DEBUG_CODE()
macro.This is
not a traditional macro because the

contents of the macro is a block of C code.I do not know how to
convince
uncrustify that the contents of a

macro like function call to be treated as a code block from an indent
perspective.
I believe this would have to be overridden in the Uncrustify fork since
DEBUG_CODE() is being treated as a macro function call and code
blocks
are not expected there. In addition, some special treatment might be
needed for alignment in between
DEBUG_CODE_BEGIN()/DEBUG_CODE_END().

I'm happy to look at this. However, regression validation can take a
while so I'd like to make sure we need this now. In cases that do not
have additional code blocks, it seems to format fairly well. Is this
prevalent and impactful enough it must be fixed now? Or, could we
revisit it with a follow up patch?
How long does regression testing take? I see about 20 instances of
DEBUG_CODE() that are producing bad formatting. If the content inside
DEBUG_CODE() is a single line of a single block of statements without
any if/while/for/case statements that require further indent, then the
format looks ok.

DEBUG_CODE_BEGIN() and DEBUG_CODE_END() would look better if
the
contents between are also indented one level.


* Example #3/#4: Uncrustify is confused by the OPTIONAL
keyword.The
edk2 config declares it as a QUALIFIER

like IN and OUT.However, OPTIONAL only appears at the end of the
line
that declares a parameter to a

function.There are 3 forms. One with comma after OPTIONAL.One
with comma
before OPTIONAL, and

one with no comma if the parameter is the last parameter in the
function
declaration.

TYPE ParamName OPTIONAL,

TYPEParamName, OPTIONAL

TYPEParamName OPTIONAL

OPTIONAL is defined to nothing in edk2 builds.From a uncrustify
perspective, we really want is to be

ignored or more correctly treated it as a token that is attached to
ParamName and the combination of

ParamName and OPTIONAL treated as one unit to determine indents.
"TYPE ParamName, OPTIONAL" seems especially problematic and
presents an
inconsistency with the other formats. Mike, can you please let me
know
if you have the same observation and your thoughts on a consistent
pattern?

Thanks,

Mike

*From:*devel@edk2.groups.io <devel@edk2.groups.io> *On Behalf
Of *Maciej
Rabeda
*Sent:* Thursday, December 2, 2021 10:27 AM
*To:* devel@edk2.groups.io; Kinney, Michael D
<michael.d.kinney@...>; Michael Kubacki
<michael.kubacki@...>; Andrew Fish (afish@...)
<afish@...>; Leif Lindholm <leif@...>
*Subject:* Re: [edk2-devel] Uncrustify Conversion Detailed Plan and
Extended Hard Freeze Update #4

Hey Mike,

While most of the changes related to fixing coding style violations,
there are a couple of changes to NetworkPkg in that PR that make
the
code less readable. Examples below.

Example 1:



Example 2:


Example 3:


Example 4:

On 30-Nov-21 23:34, Michael D Kinney wrote:

Hello,

Thank you for your patience during this extended hard freeze.

Just one more step to go.There has been a delay in the review of

the patch series with the uncrustify source changes.PR(6).This

patch series was not sent out as patch review email because of its

very large size.It only contains source style changes and the

CompareBuild tool and GitHub action has shown there are no
binary

differences introduced with these source style changes.

If you are a package maintainer, then please review the following

branch/PR for your package contents and review the EDK II CI
results

and BuildCompare results.I do not expect a line by line review

because we already had time to provide feedback on the source
style

performed by uncrustify.Instead, a Reviewed-by for your package

indicates that you have reviewed the EDK II CI results and
CompareBuild

tool functionality and results and you accept the source style

changes to your package.

*https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyCha
nges_V5
<https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyCha
nges_V5>

*https://github.com/tianocore/edk2/pull/2229
<https://github.com/tianocore/edk2/pull/2229>

*https://github.com/mdkinney/edk2/actions/runs/1521618836
<https://github.com/mdkinney/edk2/actions/runs/1521618836>

Additional details on this update below.

Thank you,

Mike

Changes from Update #3

----------------------------------------------------------------------------

* Pushed PR (5)

* Added link to PR(6). EDK II CI Status is PASS. Build Compare PASS.

* Waiting for review of PR (6)

* Review of PR (7) completed and waiting for review of PR (6)

----------------------------------------------------------------------------

Changes from Update #2

----------------------------------------------------------------------------

* Changed order of PRs swapping (4) and (5).The PR that activates

increases the max CI agent job time is independent of all the other

PRs and its review is complete, so it can be committed now.

* Pushed PRs (1), (2), (3), (4).

* Waiting for review to complete for PRs (5) and (6)

* Reviews complete for PR (7)

* Identifies steps using git filter-branch to apply uncrustify changes
to a

code review patch series that was generated before the uncrustify
changes

avoiding manual merge.

* Identified steps using git filter-repo to generate an alternate
history of

the edk2 repo with uncrustify changes applied on every
commit.This may

be useful when evaluating changes to files using tools like git
blame

without the large uncrustify patch series.

---------------------------------------------------------------------------

Changes from Update #1

----------------------------------------------------------------------------

* Changed order of PRs swapping (6) and (7).The PR that activates

EDK II CI check UncrustifyCheck has to be last because it
unconditionally

checks all C/H files in all packages.Not just files that have been

modified like some of the other checkers.

* Updated link to the branch with the UncrustifyCheck plugin that
has been

updated with a one line change and Reviewed-by and Tested-by
tags.

https://github.com/mdkinney/edk2/tree/Bug_3748_add_uncrustify_ci_plugin_v
6
<https://github.com/mdkinney/edk2/tree/Bug_3748_add_uncrustify_ci_plugin_
v6>

* Reviews complete for (1), (2), (3), (5), and (7)

---------------------------------------------------------------------------

Michael Kubacki and I have prepared the patches required to apply
the

uncrustify changes and enable EDK II CI to check all submitted

patches have been run through uncrustify.

We have verified through the CompareBuild GitHub Action that the

format changes performed by uncrustify have no functional
changes.

All of the OBJ, LIB, DLL, EFI, FFS, FV, and FD files match 100%

across 70 VS2019/GCC5 builds of all package/platform DSC files in

the edk2 repo.

The hard freeze will be extended after the edk2-stable202111 tag
until

all uncrustify related changes are committed.We do not expect
this

to take more than a few days.Do not push any PRs until the hard

freeze is lifted.

The changes are broken up into 7 patch series/PRs.The PRs are
ordered

so they can be submitted using the normal submission process and
EDK II

CI will pass for each one.Details are listed below.

Uncrustify 73.0.3 for EDK II

=============================

* Sources:https://dev.azure.com/projectmu/_git/Uncrustify
<https://dev.azure.com/projectmu/_git/Uncrustify>

*
Documentation:https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncru
stify.wiki/1/Project-Mu-(EDK-
II)-
Fork-
Readme
<https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Pr
oject-Mu-(EDK-II)-Fork-Readme>

*
Download:https://dev.azure.com/projectmu/Uncrustify/_packaging?_a=packag
e&feed=mu_uncrustify&package=mu-
uncrustify-release&protocolType=NuGet&version=73.0.3
<https://dev.azure.com/projectmu/Uncrustify/_packaging?_a=package&feed=
mu_uncrustify&package=mu-uncrustify-
release&protocolType=NuGet&version=73.0.3>

Installing Uncrustify

======================

The Uncrustify tool is installed automatically when the Pytools

environment is used and the stuart* commands are run to
complete the

environment setup.Please see:

https://github.com/tianocore/edk2/tree/master/.pytool#running-
ci-locally
<https://github.com/tianocore/edk2/tree/master/.pytool#running-ci-
locally>

Uncrustify can also be installed from the download page listed
above

or built from sources from the source link above.

The Documentation link provides instruction on how to run
uncrustify from

the command line or install as a Visual Studio Code plugin.The
main

uncrustify documentation also describes how to integrate with a
few other

editors.

We have also discussed a client side githook.That effort has not
started.

Let us know if that is a feature you would find useful.

Developer impact for new code reviews

======================================

Once the uncrustify checker is active in EDK II CI, developers must

make sure their patches are run through the uncrustify tool before

sending the patches for review.

Developers must install and run uncrustify against changes files
before

sending patch review emails or submitting PR for EDK II CI.If EDK II
CI

detects and differences in source formatting, then EDK II CI will fail

and the developer must run uncrustify and resubmit the patches.

Developer impact to patch series/PRs reviewed during edk2-
stable201121 soft/hard freeze

=================================================================
======================

Developers must rebase their changes after the uncrustify source
changes are

committed.The branch with a preview of the uncrustify changes
can be used

to start this rebase work.

https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyChan
ges_V5
<https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyCha
nges_V5>

The following steps can be used to update an existing branch with
the

required uncrustify format.This is the Windows version.I will add

the Linux version soon.

1) Fetch and checkout and rebase to latest edk2/master

git fetch origin

git checkout master

git rebase origin/master

2) Make a backup copy of plugin UncrustifyCheck outside
WORKSPACE.

(e.g. C:\Temp\UncrustifyCheck) so the uncrustify tool executable
and

EDK II specific uncrustify configuration file available when working

with a branch that does not have those tools in its scope.

xcopy .pytool\Plugin\UncrustifyCheck C:\Temp\UncrustifyCheck

3) Check out the patch series branch (e.g. MyBranch)

git checkout MyBranch

4) Rebase patch series against edk2-stable202111



git rebase edk2-stable202111

5) Create new branch for the uncrustifed version (e.g.
MyBranch_Uncrustified)

git checkout -b MyBranch_Uncrustified

6) Use git filter-branch to uncrustify all the commits in the series

between the rebase target from (2) and HEAD of the branch.A filter

can be used to scope the uncrustify operations to only the C/H files

in the specific package the patch series is against. (e.g.
DynamicTablesPkg).

BaseTools should always be excluded.If the package scoped filter
is

not used, it will still work, but will take longer to run because

uncrustify will rescan every C/H files in the whole repo.

git filter-branch --tree-filter "git ls-files DynamicTablesPkg*.c
DynamicTablesPkg*.h :!BaseTools/* |
c:\\Temp\\UncrustifyCheck\\mu-uncrustify-
release_extdep\\Windows-x86\\uncrustify.exe -c
c:\\Temp\\UncrustifyCheck\\uncrustify.cfg -F - --replace --no-backup -
-if-changed" edk2-stable202111..HEAD

7) Now that all the individual patches in the branch are uncrustified,

rebase against latest edk2/master that is already uncrustified.

git rebase master

8) Verify the patches in this new branch.

Impacts to tracing history across the uncrusity changes

=======================================================

Tools the view file and line history do work with the large
uncrustify

patch series.One impact is that the operations can be very slow
due

to the large uncrustify patches.

One option to provide a faster experience is to provide an
alternate

version of the edk2 repository as "documentation" that has the

entire history re-written with uncrustify run on every commit.

The tool called git-filter-repo can be used to perform this

transformation and runs in a reasonable period of time (a few
hours)

https://github.com/newren/git-filter-repo
<https://github.com/newren/git-filter-repo>

https://github.com/newren/git-filter-
repo/blob/main/contrib/filter-repo-demos/lint-history
<https://github.com/newren/git-filter-repo/blob/main/contrib/filter-
repo-demos/lint-history>

The following steps can be used to perform this transformation.

This is the Windows version. I will add the Linux version soon.

** WARNING **This operation modifies(rewrites) all the commits

in the local copy of the repo.Do not perform

these steps on a local repo you are using for

active development.

1) Clone edk2 into a new directory (see **WARNING**)

git clonehttps://github.com/tianocore/edk2.git
<https://github.com/tianocore/edk2.git> edk2-uncrustified

cd edk2-uncrustified

2) Setup python virtual env, install pytools, and run stuart
commands

to setup build environment which includes installing uncrustify
tools.

https://github.com/tianocore/edk2/tree/master/.pytool#running-
ci-locally
<https://github.com/tianocore/edk2/tree/master/.pytool#running-ci-
locally>

3) Make a backup copy of plugin UncrustifyCheck outside
WORKSPACE.

(e.g. C:\Temp\UncrustifyCheck) so the uncrustify tool executable
and

EDK II specific uncrustify configuration file available when working

with a branch that does not have those tools in its scope.

xcopy .pytool\Plugin\UncrustifyCheck C:\Temp\UncrustifyCheck

4) Use lint-history.py from git-filter-repo examples

https://github.com/newren/git-filter-repo
<https://github.com/newren/git-filter-repo>

https://github.com/newren/git-filter-
repo/blob/main/contrib/filter-repo-demos/lint-history
<https://github.com/newren/git-filter-repo/blob/main/contrib/filter-
repo-demos/lint-history>

Line #127 - Add try except around subprocess.check_call() with
except

being pass.This is required because there are a few commits of C

files in the edk2 repo that have incorrect C syntax and do not

build with a C compiler and break the uncrustify parser.Skip
reformat

of C files that can not be parsed by uncrustify.These rare instances

are addressed in the commit that fixes the C syntax error.

Run this slightly modified version of lint-history.Include only

C/H files and exclude directories that start with 'Tools' or
'BaseTools'.

This step took about 2.2 hours on a laptop.

lint-history.py

--relevant "return (not filename.startswith(b'Tools') and not
filename.startswith(b'BaseTools') and
(filename.endswith(b'.c') or filename.endswith(b'.h')))"

c:\\work\\GitHub\\tianocore\\foo\\UncrustifyCheck\\mu-
uncrustify-release_extdep\\Windows-x86\\uncrustify.exe
-
c
c:\\work\\GitHub\\tianocore\\foo\\UncrustifyCheck\\uncrustify.cfg --
replace --no-backup --if-changed

Order of PRs to apply during extended hard freeze

==================================================

1) Update EmulatorPkg Win Host [BuildOptions] MSFT CC_FLAGS
to not force debug information

*https://bugzilla.tianocore.org/show_bug.cgi?id=3747
<https://bugzilla.tianocore.org/show_bug.cgi?id=3747>

*https://github.com/mdkinney/edk2/tree/Bug_3747_EmulatorPkg_WinHost_Re
producibleBuild
<https://github.com/mdkinney/edk2/tree/Bug_3747_EmulatorPkg_WinHost_Re
producibleBuild>

*https://github.com/tianocore/edk2/pull/2215
<https://github.com/tianocore/edk2/pull/2215>

* Required for EmulatorPkg to pass CompareBuild for VS2019
IA32/X64 builds.

* Status: Review complete.PR pushed.

2) EccCheck should not revert staged and local changes

*https://bugzilla.tianocore.org/show_bug.cgi?id=2986
<https://bugzilla.tianocore.org/show_bug.cgi?id=2986>

*https://github.com/mdkinney/edk2/tree/Bug_2986_EccCheckRemoveGitRever
t_V2
<https://github.com/mdkinney/edk2/tree/Bug_2986_EccCheckRemoveGitRever
t_V2>

*https://github.com/tianocore/edk2/pull/2216
<https://github.com/tianocore/edk2/pull/2216>

* Required for EDK II CI to complete in a reasonable period of time
when

processing the 4000+ source file style changes made by uncrustify.

* Also fixes critical bugs that can potentially corrupt git state when

EccCheck is run locally.

* Status: Review complete.PR pushed.

3) Update pytool LicenseCheck plugin to use temp directory for diff
output file

*https://bugzilla.tianocore.org/show_bug.cgi?id=3746
<https://bugzilla.tianocore.org/show_bug.cgi?id=3746>

*https://github.com/mdkinney/edk2/tree/Bug_3746_LicenseCheckUseDiffOutp
utFile_V2
<https://github.com/mdkinney/edk2/tree/Bug_3746_LicenseCheckUseDiffOutp
utFile_V2>

*https://github.com/tianocore/edk2/pull/2217
<https://github.com/tianocore/edk2/pull/2217>

* Required to reduce EDK II CI build times.

* Status: Review complete.PR pushed.

4) Update max job time from 60 min to 120 minutes
in .azurepipelines/templates

*https://bugzilla.tianocore.org/show_bug.cgi?id=3750
<https://bugzilla.tianocore.org/show_bug.cgi?id=3750>

*https://github.com/mdkinney/edk2/tree/Bug_3750_IncreaseAzurePipelinesTim
eout
<https://github.com/mdkinney/edk2/tree/Bug_3750_IncreaseAzurePipelinesTim
eout>

*https://github.com/tianocore/edk2/pull/2219
<https://github.com/tianocore/edk2/pull/2219>

* Required to allow EccCheck of uncrustify changes to complete
on Azure

Pipelines CI agents without timing out.

* Status: Review complete.PR pushed.

5) Update Package YAML to ignore specific ECC files/errors

*https://bugzilla.tianocore.org/show_bug.cgi?id=3749
<https://bugzilla.tianocore.org/show_bug.cgi?id=3749>

*https://github.com/mdkinney/edk2/tree/Bug_3749_EccCheckIgnoreFilesErrors
<https://github.com/mdkinney/edk2/tree/Bug_3749_EccCheckIgnoreFilesErrors


*https://github.com/tianocore/edk2/pull/2218
<https://github.com/tianocore/edk2/pull/2218>

* Required to pass EccCheck

* Status: Review complete. PR pushed

6) Uncrustify Source Changes

*https://bugzilla.tianocore.org/show_bug.cgi?id=3737
<https://bugzilla.tianocore.org/show_bug.cgi?id=3737>

*https://bugzilla.tianocore.org/show_bug.cgi?id=3739
<https://bugzilla.tianocore.org/show_bug.cgi?id=3739>

*https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyCha
nges_V5
<https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyCha
nges_V5>

*https://github.com/tianocore/edk2/pull/2229
<https://github.com/tianocore/edk2/pull/2229>

* Build comparison result
PASS:https://github.com/mdkinney/edk2/actions/runs/1521618836
<https://github.com/mdkinney/edk2/actions/runs/1521618836>

* EFI_D_ -> DEBUG changes required to pass PatchCheck

* Uncrustify format changes required to pass UncrustifyCheck

* Status:

Waiting for review

7) UncrustifyCheck EDK II CI Plugin

*https://bugzilla.tianocore.org/show_bug.cgi?id=3748
<https://bugzilla.tianocore.org/show_bug.cgi?id=3748>

*https://github.com/mdkinney/edk2/tree/Bug_3748_add_uncrustify_ci_plugin_
v6
<https://github.com/mdkinney/edk2/tree/Bug_3748_add_uncrustify_ci_plugin_
v6>

* Required to enforce all PRs submitted to EDK II CI match
uncrustify format.

* Unconditionally checks all packages.Can not be committed until
all C/H

source files have been updated.

* Status: Review complete

Combined Branch/PR for Review/Test

==================================

* Build Comparison results must pass 100% across the full set of
PRs before

the individual PRs can be pushed in the order listed above.

*
Branch:https://github.com/mdkinney/edk2/tree/TestOnly_Uncrustify_PR_Series
<https://github.com/mdkinney/edk2/tree/TestOnly_Uncrustify_PR_Series>

* PR:https://github.com/tianocore/edk2/pull/2229
<https://github.com/tianocore/edk2/pull/2229>

Status = PASS

* CompareBuild:

Branch:https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrusti
fyChanges_V5
<https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyCha
nges_V5>

--ref1:ef9a059cdb15844fe52a49af2bf7d86b9dd3e9bf

--ref2:Bug_3737_3739_ApplyUncrustifyChanges_V5

Extra Options: -n 4 --quiet

Results:https://github.com/mdkinney/edk2/actions/runs/1521618836
<https://github.com/mdkinney/edk2/actions/runs/1521618836>

30 VS2019 build comparisons PASS

40 GCC5 build comparisons PASS

100% PASS

The following git log shows the set of patches from --ref1 to --ref
2across

which there are no differences in any of the
OBJ/LIB/DLL/EFI/FFS/FV/FD files.

--ref2

b7d4bf0675b7 (HEAD ->
Bug_3737_3739_ApplyUncrustifyChanges_V5) UnitTestFrameworkPkg: Apply
uncrusitify
changes

7f03d25f60e7 UefiPayloadPkg: Apply uncrusitify changes

0bfd8d9b5ac9 UefiCpuPkg: Apply uncrusitify changes

e1cd9bfb9dea StandaloneMmPkg: Apply uncrusitify changes

5da2f65be378 SourceLevelDebugPkg: Apply uncrusitify changes

95b86de07e5d SignedCapsulePkg: Apply uncrusitify changes

fe71d97246c4 ShellPkg: Apply uncrusitify changes

54c21c952992 SecurityPkg: Apply uncrusitify changes

187a3785f12b RedfishPkg: Apply uncrusitify changes

810100002a46 PcAtChipsetPkg: Apply uncrusitify changes

276a695c0cf2 OvmfPkg: Apply uncrusitify changes

303c0a91ab07 NetworkPkg: Apply uncrusitify changes

bc80792cd1b1 MdePkg: Apply uncrusitify changes

3ea86be17a2a MdeModulePkg: Apply uncrusitify changes

c70ef11ed0cd IntelFsp2WrapperPkg: Apply uncrusitify changes

c0291221f252 IntelFsp2Pkg: Apply uncrusitify changes

6a479952a690 FmpDevicePkg: Apply uncrusitify changes

3a7c05b7070d FatPkg: Apply uncrusitify changes

b789f98c8959 EmulatorPkg: Apply uncrusitify changes

952d7a1c9220 EmbeddedPkg: Apply uncrusitify changes

a1cc9881bab6 DynamicTablesPkg: Apply uncrusitify changes

50654dfe5785 CryptoPkg: Apply uncrusitify changes

ed965a02dfa1 ArmVirtPkg: Apply uncrusitify changes

9744023fbc46 ArmPlatformPkg: Apply uncrusitify changes

7a1cde5f5bba ArmPkg: Apply uncrusitify changes

19d17e0913e8 UefiCpuPkg: Change use of EFI_D_* to DEBUG_*

ffa718b4f994 SourceLevelDebugPkg: Change use of EFI_D_* to
DEBUG_*

b86cb3c5e5b4 ShellPkg: Change use of EFI_D_* to DEBUG_*

c7c42204dc07 SecurityPkg: Change use of EFI_D_* to DEBUG_*

16b8e6f958e4 PcAtChipsetPkg: Change use of EFI_D_* to
DEBUG_*

0ac3f8b2dac5 OvmfPkg: Change use of EFI_D_* to DEBUG_*

bc5004b8d294 NetworkPkg: Change use of EFI_D_* to DEBUG_*

6f671a8e2377 MdePkg: Change use of EFI_D_* to DEBUG_*

a10c610ff9a3 MdeModulePkg: Change use of EFI_D_* to
DEBUG_*

09a3bddba390 FatPkg: Change use of EFI_D_* to DEBUG_*

59c61318246a EmulatorPkg: Change use of EFI_D_* to DEBUG_*

3a80367dda3b EmbeddedPkg: Change use of EFI_D_* to DEBUG_*

23eb1aaf80ca ArmVirtPkg: Change use of EFI_D_* to DEBUG_*

875914b45c54 ArmPlatformPkg: Change use of EFI_D_* to
DEBUG_*

eb2eca82b451 ArmPkg: Change use of EFI_D_* to DEBUG_*

f0f3f5aae7c4 (origin/master, origin/HEAD, master)
UnitTestFrameworkPkg: Update YAML to ignore specific ECC
files/errors

c05734797790 UefiPayloadPkg: Update YAML to ignore specific
ECC files/errors

c30c40d6c63d StandaloneMmPkg: Update YAML to ignore specific
ECC files/errors

9944508e85f1 ShellPkg: Update YAML to ignore specific ECC
files/errors

60fa40be458d SecurityPkg: Update YAML to ignore specific ECC
files/errors

df790cd6b37e MdePkg: Update YAML to ignore specific ECC
files/errors

9deb9370766e MdeModulePkg: Update YAML to ignore specific
ECC files/errors

d7d30e8f219f EmulatorPkg: Update YAML to ignore specific ECC
files/errors

d5744ecba813 CryptoPkg: Update YAML to ignore specific ECC
files/errors

c97fee87f0f9 ArmVirtPkg: Update YAML to ignore specific ECC
files/errors

1939fc9569f2 ArmPlatformPkg: Update YAML to ignore specific
ECC files/errors

365dced2c37a ArmPkg: Update YAML to ignore specific ECC
files/errors

76a1ce4d5fec .azurepipelines/templates: Update max pipeline job
time to 2 hours

99f84ff47390 .pytools/Plugin/LicenseCheck: Use temp directory
for git diff output

3019f1bbabf1 .pytool/Plugin/EccCheck: Add performance
optimizations

854462bd3479 .pytool/Plugin/EccCheck: Remove temp directory
on exception

69877614fdee .pytool/Plugin/EccCheck: Remove RevertCode()

--ref1

ef9a059cdb15 EmulatorPkg/Win/Host: Update CC_FLAGS

bb1bba3d7767 (tag: edk2-stable202111) NetworkPkg: Fix invalid
pointer for DNS response token on error

Best regards,

Mike




Re: [PATCH V3 23/29] UefiCpuPkg: Update AddressEncMask in CpuPageTable

Min Xu
 

On November 22, 2021 11:09 AM, Ni Ray wrote:
Gerd, thanks. I am about to raise the same comments...
+ gEfiMdeModulePkgTokenSpaceGuid.PcdTdxSharedBitMask ##
CONSUMES

AddressEncMask = PcdGet64 (PcdPteMemoryEncryptionAddressOrMask)
&
PAGING_1G_ADDRESS_MASK_64;
+ if (AddressEncMask == 0) {
+ AddressEncMask = PcdGet64 (PcdTdxSharedBitMask) &
+ PAGING_1G_ADDRESS_MASK_64; }
Looks like two PCDs for basically the same thing.
Should we create a common CC PCD here?
1. The current situation of PcdPteMemoryEncryptionAddressOrMask is:
1.1 PcdPteMemoryEncryptionAddressOrMask is now set by AmdSev.
1.2 In CreateIdentityMappingPageTables(), this value (AddressEncMask) is set to the page tables in SEV guest.
1.3 This PCD is also used as an indicator in InternalMemEncryptSevStatus() if ReadSevMsr is TRUE or FALSE.
1.4 This PCD is also used in BootScriptExecutorEntryPoint()

2. The meaning and usage scenario of PcdTdxSharedBitMask are somehow different from PcdPteMemoryEncryptionAddressOrMask.
2.1 Guest physical address (GPA) space of Td guest is divided into private and shared sub-spaces, determined by the shared bit of GPA.[1]
2.2 PcdTdxSharedBitMask indicates the above shared bit of GPA. And only the shared GPA has the shared bit set. This breaks 1.2.
2.3 It also breaks above 1.3. Because not all the MSR can be read in Td guest (It will trigger #VE).
2.4 It breaks above 1.4 as well. Because the private GPA doesn't have the shared bit set (2.2). So BootScriptExecutorEntryPoint() has to check Td guest in run-time so that the correct AddressEncMask is used.

Based on above investigation and consideration, I suggest use PcdTdxSharedBitMask for Td guest and PcdPteMemoryEncryptionAddressOrMask for SEV guest. We can re-visit it later.

[1] https://www.intel.com/content/dam/develop/external/us/en/documents/tdx-module-1.0-public-spec-v0.931.pdf Section 2.4.2

Thanks
Min


Re: Uncrustify Conversion Detailed Plan and Extended Hard Freeze Update #4

Wang, Jian J
 

For CryptoPkg, SecurityPkg and SignedCapsulePkg,

Reviewed-by: Jian J Wang <jian.j.wang@...>

Regards,
Jian

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael D
Kinney
Sent: Monday, December 06, 2021 9:18 AM
To: Michael Kubacki <mikuback@...>; devel@edk2.groups.io;
maciej.rabeda@...; Michael Kubacki
<michael.kubacki@...>; Andrew Fish (afish@...)
<afish@...>; Leif Lindholm <leif@...>; Kinney, Michael D
<michael.d.kinney@...>
Subject: Re: [edk2-devel] Uncrustify Conversion Detailed Plan and Extended
Hard Freeze Update #4

Hello EDK II Maintainers,

A detailed evaluation of the DEBUG_CODE() formatting issue has been
completed.
The reason DEBUG_CODE() is a challenge is that this looks like a macro from a
C parsing perspective, but the EDK II usage places C statements or blocks of
C code as the parameter to this macro.

There are actually 2 methods available to mark a statement or a block of code
to be included when the DEBUG_PROPERTY_DEBUG_CODE_ENABLED bit is
enabled in
PcdDebugPropertyMask. One is DEBUG_CODE() and the other is to mark the
beginning
and end of a code block with DEBUG_CODE_BEGIN() and DEBUG_CODE_END().
In fact,
DEBUG_CODE() is implemented using DEBUG_CODE_BEGIN() and
DEBUG_CODE_END() macros.

#define DEBUG_CODE(Expression) \
DEBUG_CODE_BEGIN (); \
Expression \
DEBUG_CODE_END ()

A complete review for the use of these DEBUG_CODE macros was performed on
the
edk2 repo. Uncrustify performs good formatting for code blocks between
DEBUG_CODE_BEGIN() and DEBUG_CODE_END(). This is because these look
like simple
macros calls with no parameters and the lines of C code between these 2
macros
is formatted correctly.

The uncrustify formatting issues are only present with the use of DEBUG_CODE().
Simple use cases of DEBUG_CODE(Expression) where Expression is a single C
statement also look correct. A medium complexity use case where Expression is
a code block of simple statements or even some local variables and simple
statements also look correct. It is only complex code blocks that use C
statements such as if/for/while/case that include the use of braces {} does
uncrustify perform incorrect formatting.

The recommended solution to this issue is to convert the use of DEBUG_CODE()
to DEBUG_CODE_BEGIN() / DEBUG_CODE_END() for cases where the
Expression
passed to DEBUG_CODE() is the complex use case that contains statements that
use braces {}. There are 57 instances of this pattern across 40 files in the
edk2 repo.

I have posted a branch with these additional patches:


https://github.com/mdkinney/edk2/tree/Bug_3737_3760_3739_ApplyUncrustif
yChanges_V7

I have performed CompareBuild tests with this revised patch series with
the DEBUG_CODE changes. It passes 100% showing no binary differences.

https://github.com/mdkinney/edk2/actions/runs/1542454606

I have opened a PR to run this patch series through EDK II CI. It also passes 100%.

https://github.com/tianocore/edk2/pull/2236

The summary of changes made since the V6 review are:

1) Change uncrustify configuration assignment alignment threshold to 0

align_assign_thresh = 0

2) Replace "<param>, OPTIONAL" with "<param> OPTIONAL,"

3) Replace DEBUG_CODE(Expression) with

DEBUG_CODE_BEGIN();
Expression
DEBUG_CODE_END()

if Expression is complex and contains braces {}.

4) No changes to uncrustify tool required.

Please review the differences between the following 2 branches and provide
feedback or a Series Reviewed-by if you agree with these additional changes.


https://github.com/mdkinney/edk2/tree/Bug_3737_3760_3739_ApplyUncrustif
yChanges_V6

https://github.com/mdkinney/edk2/tree/Bug_3737_3760_3739_ApplyUncrustif
yChanges_V7

The goal is to complete the review and get the uncrustify change committed
tomorrow so the extended hard freeze can be lifted.

Thanks,

Mike

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@...>
Sent: Thursday, December 2, 2021 6:23 PM
To: Michael Kubacki <mikuback@...>; devel@edk2.groups.io;
maciej.rabeda@...; Michael Kubacki
<michael.kubacki@...>; Andrew Fish (afish@...)
<afish@...>; Leif Lindholm <leif@...>;
Kinney, Michael D <michael.d.kinney@...>
Subject: RE: [edk2-devel] Uncrustify Conversion Detailed Plan and Extended
Hard Freeze Update #4

Hello EDK II Maintainers,

I have entered BZ 3760 to make the use of the OPTIONAL keyword style
consistent for all of edk2 repo
and to be compatible with uncrustify.

I have posted the following V6 branch that does the EFI_D_* to DEBUG_*
changes, the OPTIONAL keyword
style changes, and the uncrustify changes with the one configuration change
for assignment alignment.

https://github.com/mdkinney/edk2/tree/Bug_3737_3760_3739_ApplyUncrustif
yChanges_V6

Please provide feedback on the code style in this branch with the known
DEBUG_CODE() issue still
present.

If we are able to quickly update uncrustify to handle DEBUG_CODE(), I will
generate a V7.

Thanks,

Mike

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@...>
Sent: Thursday, December 2, 2021 4:53 PM
To: Michael Kubacki <mikuback@...>;
devel@edk2.groups.io; maciej.rabeda@...; Michael Kubacki
<michael.kubacki@...>; Andrew Fish (afish@...)
<afish@...>; Leif Lindholm <leif@...>;
Kinney, Michael D <michael.d.kinney@...>
Subject: RE: [edk2-devel] Uncrustify Conversion Detailed Plan and Extended
Hard Freeze Update #4

Michael,

Yes. Please update the patch series that adds UncrustifyCheck with those
changes.

Thanks,

Mike

-----Original Message-----
From: Michael Kubacki <mikuback@...>
Sent: Thursday, December 2, 2021 4:31 PM
To: Kinney, Michael D <michael.d.kinney@...>;
devel@edk2.groups.io; maciej.rabeda@...; Michael
Kubacki
<michael.kubacki@...>; Andrew Fish (afish@...)
<afish@...>; Leif Lindholm <leif@...>
Subject: Re: [edk2-devel] Uncrustify Conversion Detailed Plan and
Extended Hard Freeze Update #4

Hi Mike,

Thank you for the detailed analysis and recommendations.

I agree with the recommendations and I will try to have an Uncrustify
tool update by tomorrow for (4).

That will require an update in uncrustify_ext_dep.yaml to pull in the
new version. I'm assuming I should also update uncrustify.cfg to set
align_assign_thresh = 0 in that new patch series?

Regards,
Michael

On 12/2/2021 7:18 PM, Kinney, Michael D wrote:
Hi Michael,

CORRECTION: set align_assign_threshold to 0.

Reponses inline below.

I would like to summarize the 4 issues raised in the past day along with
the recommendations.

1) Exclusion feature for UncrustifyCheck. There are 2 directories with 8
files total that
maintainers have noted they would like to see not go through
uncrustify formatting. Today
the only content that is skipped is BaseTools and submodules.

Adding a general purpose exclusion feature would then require all
developers to make
sure their method of using uncrustify also excludes those same areas.
This requires
extra steps for all developers and maintainers.

If we do not add the exclusion feature, then the 8 files will require an
extra step
to sync with the original source of those files. The rate of changes of
these 8 files
is very low today.

RECOMMENDATION: Do not add exclusion feature at this time. Revisit
if the extra work
to maintain the files that would be candidates for exclusions increases
significantly.

2) Alignment of assignments. The threshold of 4 characters appears to
be too low and causes
source files that are already aligned to become unaligned.

RECOMMENDATION: Change threshold to the default value of 0 which
means no limit.

align_assign_thresh= 0

3) Alignment of parameters in function declaration not correct. The root
cause of this
is the use of the OPTIONAL keyword. If the OPTIONAL keyword is
removed, then the
alignment is correct. The alignment is also correct if the OPTIONAL
keyword appears
before the ','. If the OPTIONAL keyword appears after the ',', then the
format is
not correct. The OPTIONAL keyword indicates that the parameter in
the function is
not required and may be passed in as NULL or 0 or some other default
value defined by
the API. It makes more sense for this OPTIONAL keyword that follows
the parameter
names to appear before the ',' so it is scoped to the parameter on that
line. If it
appears after the ',', then C parsers thinks it is a prefix (IN, OUT, CONST,
volatile,
static) for the next parameter in the function.

RECOMMENDATION: Update patch series with a global search and
replace so OPTIONAL
keyword always appears before the ',' on the same line.

RegEx search string: ',( *)OPTIONAL( *)'
RegEx replace string: ' $1OPTIONAL,$2'

4) Format issues with complex blocks in DEBUG_CODE(), or between
DEBUG_CODE_BEGIN() and
DEBUG_CODE_END(). Uncrustify treats these as macros and is not
aware that the
parameter passed into the macro call is a block of C code that needs to
be formatted.
Complex blocks with if/while/for/case statements are impacted the
most.

RECOMMENDATION: Update the uncrustify with an edk2 specific
extension to treat these
macros as a block of code as if they were surrounded by an extra set of
braces {}.


I have posted a branch for testing purposes that implements (2) and (3).

Branch:
https://github.com/mdkinney/edk2/tree/TestOnly_Bug_3737_3739_ApplyUncru
stifyChanges_V6_OPTIONAL_Keyword_Fix
PR: https://github.com/tianocore/edk2/pull/2233
Status: PASS
CompareBuild:
https://github.com/mdkinney/edk2/actions/runs/1532855914
Status: PASS

You can see what changed by fetching and comparing the following 2
branches:

https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyChan
ges_V5
https://github.com/mdkinney/edk2/tree/TestOnly_Bug_3737_3739_ApplyUncru
stifyChanges_V6_OPTIONAL_Keyword_Fix

Please provide feedback on the RECOMMENDATIONS above. I will go
ahead and prepare of V6 version of
the patch series now that that test results are all PASS.

Best regards,

Mike


-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@...>
Sent: Thursday, December 2, 2021 4:15 PM
To: Michael Kubacki <mikuback@...>;
devel@edk2.groups.io; maciej.rabeda@...; Michael
Kubacki
<michael.kubacki@...>; Andrew Fish (afish@...)
<afish@...>; Leif Lindholm
<leif@...>;
Kinney, Michael D <michael.d.kinney@...>
Subject: RE: [edk2-devel] Uncrustify Conversion Detailed Plan and
Extended Hard Freeze Update #4

Hi Michael,

Reponses inline below.

I would like to summarize the 4 issues raised in the past day along with
the recommendations.

1) Exclusion feature for UncrustifyCheck. There are 2 directories with 8
files total that
maintainers have noted they would like to see not go through
uncrustify formatting. Today
the only content that is skipped is BaseTools and submodules.

Adding a general purpose exclusion feature would then require all
developers to make
sure their method of using uncrustify also excludes those same areas.
This requires
extra steps for all developers and maintainers.

If we do not add the exclusion feature, then the 8 files will require an
extra step
to sync with the original source of those files. The rate of changes of
these 8 files
is very low today.

RECOMMENDATION: Do not add exclusion feature at this time.
Revisit if the extra work
to maintain the files that would be candidates for exclusions increases
significantly.

2) Alignment of assignments. The threshold of 4 characters appears to
be too low and causes
source files that are already aligned to become unaligned.

RECOMMENDATION: Change threshold to the default value of 0
which means no limit.

align_assign_thresh= 4

3) Alignment of parameters in function declaration not correct. The
root cause of this
is the use of the OPTIONAL keyword. If the OPTIONAL keyword is
removed, then the
alignment is correct. The alignment is also correct if the OPTIONAL
keyword appears
before the ','. If the OPTIONAL keyword appears after the ',', then the
format is
not correct. The OPTIONAL keyword indicates that the parameter in
the function is
not required and may be passed in as NULL or 0 or some other default
value defined by
the API. It makes more sense for this OPTIONAL keyword that follows
the parameter
names to appear before the ',' so it is scoped to the parameter on
that line. If it
appears after the ',', then C parsers thinks it is a prefix (IN, OUT,
CONST, volatile,
static) for the next parameter in the function.

RECOMMENDATION: Update patch series with a global search and
replace so OPTIONAL
keyword always appears before the ',' on the same line.

RegEx search string: ',( *)OPTIONAL( *)'
RegEx replace string: ' $1OPTIONAL,$2'

4) Format issues with complex blocks in DEBUG_CODE(), or between
DEBUG_CODE_BEGIN() and
DEBUG_CODE_END(). Uncrustify treats these as macros and is not
aware that the
parameter passed into the macro call is a block of C code that needs
to be formatted.
Complex blocks with if/while/for/case statements are impacted the
most.

RECOMMENDATION: Update the uncrustify with an edk2 specific
extension to treat these
macros as a block of code as if they were surrounded by an extra set
of braces {}.


I have posted a branch for testing purposes that implements (2) and (3).

Branch:
https://github.com/mdkinney/edk2/tree/TestOnly_Bug_3737_3739_ApplyUncru
stifyChanges_V6_OPTIONAL_Keyword_Fix
PR: https://github.com/tianocore/edk2/pull/2233
Status: PASS
CompareBuild:
https://github.com/mdkinney/edk2/actions/runs/1532855914
Status: PASS

You can see what changed by fetching and comparing the following 2
branches:

https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyChan
ges_V5
https://github.com/mdkinney/edk2/tree/TestOnly_Bug_3737_3739_ApplyUncru
stifyChanges_V6_OPTIONAL_Keyword_Fix

Please provide feedback on the RECOMMENDATIONS above. I will go
ahead and prepare of V6 version of
the patch series now that that test results are all PASS.

Best regards,

Mike


-----Original Message-----
From: Michael Kubacki <mikuback@...>
Sent: Thursday, December 2, 2021 1:57 PM
To: devel@edk2.groups.io; Kinney, Michael D
<michael.d.kinney@...>; maciej.rabeda@...; Michael
Kubacki
<michael.kubacki@...>; Andrew Fish (afish@...)
<afish@...>; Leif Lindholm
<leif@...>
Subject: Re: [edk2-devel] Uncrustify Conversion Detailed Plan and
Extended Hard Freeze Update #4

My reply is inline.

Regards,
Michael

On 12/2/2021 2:45 PM, Michael D Kinney wrote:
Hi Maciej,

Thanks for the feedback.

* Example #1.This appears to be caused by the following uncrustify
settings:

# The threshold for aligning on '=' in assignments.

# Use a negative number for absolute thresholds.

#

# 0: No limit (default).

align_assign_thresh= 0# number

The edk2 setting for this is:

align_assign_thresh= 4

This means blocks of assignments that are different than more than 4
spaces will be considered a new block.

‘HwAddreLen’ and ‘Xid’ are more than 4 characters in length
different.Same for ‘Xid’ and ‘Reserved’.So

uncrustify treats these as 3 different assignment blocks.

If we change to the default value of 0: No limit, this example is
treated as a single block and all ‘=’ are aligned.

Is there a reason ‘4’ was selected?Is there is any harm in using the
default of 0?
We can certainly change the threshold. '4' was derived by
experimentation. This is an area that is somewhat subjective and every
case is difficult to cover well. Please feel free to suggest any changes.
I recommend we use the default value of 0. That way, any code that
choose to
align assignments will still be aligned. If a developer does not like
short assignments and long assignments in the same code block to use
the
long assignment column, they can always break the block up into
multiple
blocks by adding a carriage return.

For the benefit of others, this file can be a useful refernce
understanding spans, gaps, and thresholds.

https://github.com/uncrustify/uncrustify/blob/master/documentation/htdocs/c
onfiguration.txt

* Example #2: Uncruistfy is confused by the DEBUG_CODE()
macro.This is
not a traditional macro because the

contents of the macro is a block of C code.I do not know how to
convince
uncrustify that the contents of a

macro like function call to be treated as a code block from an indent
perspective.
I believe this would have to be overridden in the Uncrustify fork since
DEBUG_CODE() is being treated as a macro function call and code
blocks
are not expected there. In addition, some special treatment might be
needed for alignment in between
DEBUG_CODE_BEGIN()/DEBUG_CODE_END().

I'm happy to look at this. However, regression validation can take a
while so I'd like to make sure we need this now. In cases that do not
have additional code blocks, it seems to format fairly well. Is this
prevalent and impactful enough it must be fixed now? Or, could we
revisit it with a follow up patch?
How long does regression testing take? I see about 20 instances of
DEBUG_CODE() that are producing bad formatting. If the content inside
DEBUG_CODE() is a single line of a single block of statements without
any if/while/for/case statements that require further indent, then the
format looks ok.

DEBUG_CODE_BEGIN() and DEBUG_CODE_END() would look better if
the
contents between are also indented one level.


* Example #3/#4: Uncrustify is confused by the OPTIONAL
keyword.The
edk2 config declares it as a QUALIFIER

like IN and OUT.However, OPTIONAL only appears at the end of the
line
that declares a parameter to a

function.There are 3 forms. One with comma after OPTIONAL.One
with comma
before OPTIONAL, and

one with no comma if the parameter is the last parameter in the
function
declaration.

TYPE ParamName OPTIONAL,

TYPEParamName, OPTIONAL

TYPEParamName OPTIONAL

OPTIONAL is defined to nothing in edk2 builds.From a uncrustify
perspective, we really want is to be

ignored or more correctly treated it as a token that is attached to
ParamName and the combination of

ParamName and OPTIONAL treated as one unit to determine indents.
"TYPE ParamName, OPTIONAL" seems especially problematic and
presents an
inconsistency with the other formats. Mike, can you please let me
know
if you have the same observation and your thoughts on a consistent
pattern?

Thanks,

Mike

*From:*devel@edk2.groups.io <devel@edk2.groups.io> *On Behalf
Of *Maciej
Rabeda
*Sent:* Thursday, December 2, 2021 10:27 AM
*To:* devel@edk2.groups.io; Kinney, Michael D
<michael.d.kinney@...>; Michael Kubacki
<michael.kubacki@...>; Andrew Fish (afish@...)
<afish@...>; Leif Lindholm <leif@...>
*Subject:* Re: [edk2-devel] Uncrustify Conversion Detailed Plan and
Extended Hard Freeze Update #4

Hey Mike,

While most of the changes related to fixing coding style violations,
there are a couple of changes to NetworkPkg in that PR that make
the
code less readable. Examples below.

Example 1:



Example 2:


Example 3:


Example 4:

On 30-Nov-21 23:34, Michael D Kinney wrote:

Hello,

Thank you for your patience during this extended hard freeze.

Just one more step to go.There has been a delay in the review of

the patch series with the uncrustify source changes.PR(6).This

patch series was not sent out as patch review email because of its

very large size.It only contains source style changes and the

CompareBuild tool and GitHub action has shown there are no
binary

differences introduced with these source style changes.

If you are a package maintainer, then please review the following

branch/PR for your package contents and review the EDK II CI
results

and BuildCompare results.I do not expect a line by line review

because we already had time to provide feedback on the source
style

performed by uncrustify.Instead, a Reviewed-by for your package

indicates that you have reviewed the EDK II CI results and
CompareBuild

tool functionality and results and you accept the source style

changes to your package.

*https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyCha
nges_V5
<https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyCha
nges_V5>

*https://github.com/tianocore/edk2/pull/2229
<https://github.com/tianocore/edk2/pull/2229>

*https://github.com/mdkinney/edk2/actions/runs/1521618836
<https://github.com/mdkinney/edk2/actions/runs/1521618836>

Additional details on this update below.

Thank you,

Mike

Changes from Update #3

----------------------------------------------------------------------------

* Pushed PR (5)

* Added link to PR(6). EDK II CI Status is PASS. Build Compare PASS.

* Waiting for review of PR (6)

* Review of PR (7) completed and waiting for review of PR (6)

----------------------------------------------------------------------------

Changes from Update #2

----------------------------------------------------------------------------

* Changed order of PRs swapping (4) and (5).The PR that activates

increases the max CI agent job time is independent of all the other

PRs and its review is complete, so it can be committed now.

* Pushed PRs (1), (2), (3), (4).

* Waiting for review to complete for PRs (5) and (6)

* Reviews complete for PR (7)

* Identifies steps using git filter-branch to apply uncrustify changes
to a

code review patch series that was generated before the uncrustify
changes

avoiding manual merge.

* Identified steps using git filter-repo to generate an alternate
history of

the edk2 repo with uncrustify changes applied on every
commit.This may

be useful when evaluating changes to files using tools like git
blame

without the large uncrustify patch series.

---------------------------------------------------------------------------

Changes from Update #1

----------------------------------------------------------------------------

* Changed order of PRs swapping (6) and (7).The PR that activates

EDK II CI check UncrustifyCheck has to be last because it
unconditionally

checks all C/H files in all packages.Not just files that have been

modified like some of the other checkers.

* Updated link to the branch with the UncrustifyCheck plugin that
has been

updated with a one line change and Reviewed-by and Tested-by
tags.

https://github.com/mdkinney/edk2/tree/Bug_3748_add_uncrustify_ci_plugin_v
6
<https://github.com/mdkinney/edk2/tree/Bug_3748_add_uncrustify_ci_plugin_
v6>

* Reviews complete for (1), (2), (3), (5), and (7)

---------------------------------------------------------------------------

Michael Kubacki and I have prepared the patches required to apply
the

uncrustify changes and enable EDK II CI to check all submitted

patches have been run through uncrustify.

We have verified through the CompareBuild GitHub Action that the

format changes performed by uncrustify have no functional
changes.

All of the OBJ, LIB, DLL, EFI, FFS, FV, and FD files match 100%

across 70 VS2019/GCC5 builds of all package/platform DSC files in

the edk2 repo.

The hard freeze will be extended after the edk2-stable202111 tag
until

all uncrustify related changes are committed.We do not expect
this

to take more than a few days.Do not push any PRs until the hard

freeze is lifted.

The changes are broken up into 7 patch series/PRs.The PRs are
ordered

so they can be submitted using the normal submission process and
EDK II

CI will pass for each one.Details are listed below.

Uncrustify 73.0.3 for EDK II

=============================

* Sources:https://dev.azure.com/projectmu/_git/Uncrustify
<https://dev.azure.com/projectmu/_git/Uncrustify>

*
Documentation:https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncru
stify.wiki/1/Project-Mu-(EDK-
II)-
Fork-
Readme
<https://dev.azure.com/projectmu/Uncrustify/_wiki/wikis/Uncrustify.wiki/1/Pr
oject-Mu-(EDK-II)-Fork-Readme>

*
Download:https://dev.azure.com/projectmu/Uncrustify/_packaging?_a=packag
e&feed=mu_uncrustify&package=mu-
uncrustify-release&protocolType=NuGet&version=73.0.3
<https://dev.azure.com/projectmu/Uncrustify/_packaging?_a=package&feed=
mu_uncrustify&package=mu-uncrustify-
release&protocolType=NuGet&version=73.0.3>

Installing Uncrustify

======================

The Uncrustify tool is installed automatically when the Pytools

environment is used and the stuart* commands are run to
complete the

environment setup.Please see:

https://github.com/tianocore/edk2/tree/master/.pytool#running-
ci-locally
<https://github.com/tianocore/edk2/tree/master/.pytool#running-ci-
locally>

Uncrustify can also be installed from the download page listed
above

or built from sources from the source link above.

The Documentation link provides instruction on how to run
uncrustify from

the command line or install as a Visual Studio Code plugin.The
main

uncrustify documentation also describes how to integrate with a
few other

editors.

We have also discussed a client side githook.That effort has not
started.

Let us know if that is a feature you would find useful.

Developer impact for new code reviews

======================================

Once the uncrustify checker is active in EDK II CI, developers must

make sure their patches are run through the uncrustify tool before

sending the patches for review.

Developers must install and run uncrustify against changes files
before

sending patch review emails or submitting PR for EDK II CI.If EDK II
CI

detects and differences in source formatting, then EDK II CI will fail

and the developer must run uncrustify and resubmit the patches.

Developer impact to patch series/PRs reviewed during edk2-
stable201121 soft/hard freeze

=================================================================
======================

Developers must rebase their changes after the uncrustify source
changes are

committed.The branch with a preview of the uncrustify changes
can be used

to start this rebase work.

https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyChan
ges_V5
<https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyCha
nges_V5>

The following steps can be used to update an existing branch with
the

required uncrustify format.This is the Windows version.I will add

the Linux version soon.

1) Fetch and checkout and rebase to latest edk2/master

git fetch origin

git checkout master

git rebase origin/master

2) Make a backup copy of plugin UncrustifyCheck outside
WORKSPACE.

(e.g. C:\Temp\UncrustifyCheck) so the uncrustify tool executable
and

EDK II specific uncrustify configuration file available when working

with a branch that does not have those tools in its scope.

xcopy .pytool\Plugin\UncrustifyCheck C:\Temp\UncrustifyCheck

3) Check out the patch series branch (e.g. MyBranch)

git checkout MyBranch

4) Rebase patch series against edk2-stable202111



git rebase edk2-stable202111

5) Create new branch for the uncrustifed version (e.g.
MyBranch_Uncrustified)

git checkout -b MyBranch_Uncrustified

6) Use git filter-branch to uncrustify all the commits in the series

between the rebase target from (2) and HEAD of the branch.A filter

can be used to scope the uncrustify operations to only the C/H files

in the specific package the patch series is against. (e.g.
DynamicTablesPkg).

BaseTools should always be excluded.If the package scoped filter
is

not used, it will still work, but will take longer to run because

uncrustify will rescan every C/H files in the whole repo.

git filter-branch --tree-filter "git ls-files DynamicTablesPkg*.c
DynamicTablesPkg*.h :!BaseTools/* |
c:\\Temp\\UncrustifyCheck\\mu-uncrustify-
release_extdep\\Windows-x86\\uncrustify.exe -c
c:\\Temp\\UncrustifyCheck\\uncrustify.cfg -F - --replace --no-backup -
-if-changed" edk2-stable202111..HEAD

7) Now that all the individual patches in the branch are uncrustified,

rebase against latest edk2/master that is already uncrustified.

git rebase master

8) Verify the patches in this new branch.

Impacts to tracing history across the uncrusity changes

=======================================================

Tools the view file and line history do work with the large
uncrustify

patch series.One impact is that the operations can be very slow
due

to the large uncrustify patches.

One option to provide a faster experience is to provide an
alternate

version of the edk2 repository as "documentation" that has the

entire history re-written with uncrustify run on every commit.

The tool called git-filter-repo can be used to perform this

transformation and runs in a reasonable period of time (a few
hours)

https://github.com/newren/git-filter-repo
<https://github.com/newren/git-filter-repo>

https://github.com/newren/git-filter-
repo/blob/main/contrib/filter-repo-demos/lint-history
<https://github.com/newren/git-filter-repo/blob/main/contrib/filter-
repo-demos/lint-history>

The following steps can be used to perform this transformation.

This is the Windows version. I will add the Linux version soon.

** WARNING **This operation modifies(rewrites) all the commits

in the local copy of the repo.Do not perform

these steps on a local repo you are using for

active development.

1) Clone edk2 into a new directory (see **WARNING**)

git clonehttps://github.com/tianocore/edk2.git
<https://github.com/tianocore/edk2.git> edk2-uncrustified

cd edk2-uncrustified

2) Setup python virtual env, install pytools, and run stuart
commands

to setup build environment which includes installing uncrustify
tools.

https://github.com/tianocore/edk2/tree/master/.pytool#running-
ci-locally
<https://github.com/tianocore/edk2/tree/master/.pytool#running-ci-
locally>

3) Make a backup copy of plugin UncrustifyCheck outside
WORKSPACE.

(e.g. C:\Temp\UncrustifyCheck) so the uncrustify tool executable
and

EDK II specific uncrustify configuration file available when working

with a branch that does not have those tools in its scope.

xcopy .pytool\Plugin\UncrustifyCheck C:\Temp\UncrustifyCheck

4) Use lint-history.py from git-filter-repo examples

https://github.com/newren/git-filter-repo
<https://github.com/newren/git-filter-repo>

https://github.com/newren/git-filter-
repo/blob/main/contrib/filter-repo-demos/lint-history
<https://github.com/newren/git-filter-repo/blob/main/contrib/filter-
repo-demos/lint-history>

Line #127 - Add try except around subprocess.check_call() with
except

being pass.This is required because there are a few commits of C

files in the edk2 repo that have incorrect C syntax and do not

build with a C compiler and break the uncrustify parser.Skip
reformat

of C files that can not be parsed by uncrustify.These rare instances

are addressed in the commit that fixes the C syntax error.

Run this slightly modified version of lint-history.Include only

C/H files and exclude directories that start with 'Tools' or
'BaseTools'.

This step took about 2.2 hours on a laptop.

lint-history.py

--relevant "return (not filename.startswith(b'Tools') and not
filename.startswith(b'BaseTools') and
(filename.endswith(b'.c') or filename.endswith(b'.h')))"

c:\\work\\GitHub\\tianocore\\foo\\UncrustifyCheck\\mu-
uncrustify-release_extdep\\Windows-x86\\uncrustify.exe
-
c
c:\\work\\GitHub\\tianocore\\foo\\UncrustifyCheck\\uncrustify.cfg --
replace --no-backup --if-changed

Order of PRs to apply during extended hard freeze

==================================================

1) Update EmulatorPkg Win Host [BuildOptions] MSFT CC_FLAGS
to not force debug information

*https://bugzilla.tianocore.org/show_bug.cgi?id=3747
<https://bugzilla.tianocore.org/show_bug.cgi?id=3747>

*https://github.com/mdkinney/edk2/tree/Bug_3747_EmulatorPkg_WinHost_Re
producibleBuild
<https://github.com/mdkinney/edk2/tree/Bug_3747_EmulatorPkg_WinHost_Re
producibleBuild>

*https://github.com/tianocore/edk2/pull/2215
<https://github.com/tianocore/edk2/pull/2215>

* Required for EmulatorPkg to pass CompareBuild for VS2019
IA32/X64 builds.

* Status: Review complete.PR pushed.

2) EccCheck should not revert staged and local changes

*https://bugzilla.tianocore.org/show_bug.cgi?id=2986
<https://bugzilla.tianocore.org/show_bug.cgi?id=2986>

*https://github.com/mdkinney/edk2/tree/Bug_2986_EccCheckRemoveGitRever
t_V2
<https://github.com/mdkinney/edk2/tree/Bug_2986_EccCheckRemoveGitRever
t_V2>

*https://github.com/tianocore/edk2/pull/2216
<https://github.com/tianocore/edk2/pull/2216>

* Required for EDK II CI to complete in a reasonable period of time
when

processing the 4000+ source file style changes made by uncrustify.

* Also fixes critical bugs that can potentially corrupt git state when

EccCheck is run locally.

* Status: Review complete.PR pushed.

3) Update pytool LicenseCheck plugin to use temp directory for diff
output file

*https://bugzilla.tianocore.org/show_bug.cgi?id=3746
<https://bugzilla.tianocore.org/show_bug.cgi?id=3746>

*https://github.com/mdkinney/edk2/tree/Bug_3746_LicenseCheckUseDiffOutp
utFile_V2
<https://github.com/mdkinney/edk2/tree/Bug_3746_LicenseCheckUseDiffOutp
utFile_V2>

*https://github.com/tianocore/edk2/pull/2217
<https://github.com/tianocore/edk2/pull/2217>

* Required to reduce EDK II CI build times.

* Status: Review complete.PR pushed.

4) Update max job time from 60 min to 120 minutes
in .azurepipelines/templates

*https://bugzilla.tianocore.org/show_bug.cgi?id=3750
<https://bugzilla.tianocore.org/show_bug.cgi?id=3750>

*https://github.com/mdkinney/edk2/tree/Bug_3750_IncreaseAzurePipelinesTim
eout
<https://github.com/mdkinney/edk2/tree/Bug_3750_IncreaseAzurePipelinesTim
eout>

*https://github.com/tianocore/edk2/pull/2219
<https://github.com/tianocore/edk2/pull/2219>

* Required to allow EccCheck of uncrustify changes to complete
on Azure

Pipelines CI agents without timing out.

* Status: Review complete.PR pushed.

5) Update Package YAML to ignore specific ECC files/errors

*https://bugzilla.tianocore.org/show_bug.cgi?id=3749
<https://bugzilla.tianocore.org/show_bug.cgi?id=3749>

*https://github.com/mdkinney/edk2/tree/Bug_3749_EccCheckIgnoreFilesErrors
<https://github.com/mdkinney/edk2/tree/Bug_3749_EccCheckIgnoreFilesErrors>

*https://github.com/tianocore/edk2/pull/2218
<https://github.com/tianocore/edk2/pull/2218>

* Required to pass EccCheck

* Status: Review complete. PR pushed

6) Uncrustify Source Changes

*https://bugzilla.tianocore.org/show_bug.cgi?id=3737
<https://bugzilla.tianocore.org/show_bug.cgi?id=3737>

*https://bugzilla.tianocore.org/show_bug.cgi?id=3739
<https://bugzilla.tianocore.org/show_bug.cgi?id=3739>

*https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyCha
nges_V5
<https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyCha
nges_V5>

*https://github.com/tianocore/edk2/pull/2229
<https://github.com/tianocore/edk2/pull/2229>

* Build comparison result
PASS:https://github.com/mdkinney/edk2/actions/runs/1521618836
<https://github.com/mdkinney/edk2/actions/runs/1521618836>

* EFI_D_ -> DEBUG changes required to pass PatchCheck

* Uncrustify format changes required to pass UncrustifyCheck

* Status:

Waiting for review

7) UncrustifyCheck EDK II CI Plugin

*https://bugzilla.tianocore.org/show_bug.cgi?id=3748
<https://bugzilla.tianocore.org/show_bug.cgi?id=3748>

*https://github.com/mdkinney/edk2/tree/Bug_3748_add_uncrustify_ci_plugin_
v6
<https://github.com/mdkinney/edk2/tree/Bug_3748_add_uncrustify_ci_plugin_
v6>

* Required to enforce all PRs submitted to EDK II CI match
uncrustify format.

* Unconditionally checks all packages.Can not be committed until
all C/H

source files have been updated.

* Status: Review complete

Combined Branch/PR for Review/Test

==================================

* Build Comparison results must pass 100% across the full set of
PRs before

the individual PRs can be pushed in the order listed above.

*
Branch:https://github.com/mdkinney/edk2/tree/TestOnly_Uncrustify_PR_Series
<https://github.com/mdkinney/edk2/tree/TestOnly_Uncrustify_PR_Series>

* PR:https://github.com/tianocore/edk2/pull/2229
<https://github.com/tianocore/edk2/pull/2229>

Status = PASS

* CompareBuild:

Branch:https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrusti
fyChanges_V5
<https://github.com/mdkinney/edk2/tree/Bug_3737_3739_ApplyUncrustifyCha
nges_V5>

--ref1:ef9a059cdb15844fe52a49af2bf7d86b9dd3e9bf

--ref2:Bug_3737_3739_ApplyUncrustifyChanges_V5

Extra Options: -n 4 --quiet

Results:https://github.com/mdkinney/edk2/actions/runs/1521618836
<https://github.com/mdkinney/edk2/actions/runs/1521618836>

30 VS2019 build comparisons PASS

40 GCC5 build comparisons PASS

100% PASS

The following git log shows the set of patches from --ref1 to --ref
2across

which there are no differences in any of the
OBJ/LIB/DLL/EFI/FFS/FV/FD files.

--ref2

b7d4bf0675b7 (HEAD ->
Bug_3737_3739_ApplyUncrustifyChanges_V5) UnitTestFrameworkPkg: Apply
uncrusitify
changes

7f03d25f60e7 UefiPayloadPkg: Apply uncrusitify changes

0bfd8d9b5ac9 UefiCpuPkg: Apply uncrusitify changes

e1cd9bfb9dea StandaloneMmPkg: Apply uncrusitify changes

5da2f65be378 SourceLevelDebugPkg: Apply uncrusitify changes

95b86de07e5d SignedCapsulePkg: Apply uncrusitify changes

fe71d97246c4 ShellPkg: Apply uncrusitify changes

54c21c952992 SecurityPkg: Apply uncrusitify changes

187a3785f12b RedfishPkg: Apply uncrusitify changes

810100002a46 PcAtChipsetPkg: Apply uncrusitify changes

276a695c0cf2 OvmfPkg: Apply uncrusitify changes

303c0a91ab07 NetworkPkg: Apply uncrusitify changes

bc80792cd1b1 MdePkg: Apply uncrusitify changes

3ea86be17a2a MdeModulePkg: Apply uncrusitify changes

c70ef11ed0cd IntelFsp2WrapperPkg: Apply uncrusitify changes

c0291221f252 IntelFsp2Pkg: Apply uncrusitify changes

6a479952a690 FmpDevicePkg: Apply uncrusitify changes

3a7c05b7070d FatPkg: Apply uncrusitify changes

b789f98c8959 EmulatorPkg: Apply uncrusitify changes

952d7a1c9220 EmbeddedPkg: Apply uncrusitify changes

a1cc9881bab6 DynamicTablesPkg: Apply uncrusitify changes

50654dfe5785 CryptoPkg: Apply uncrusitify changes

ed965a02dfa1 ArmVirtPkg: Apply uncrusitify changes

9744023fbc46 ArmPlatformPkg: Apply uncrusitify changes

7a1cde5f5bba ArmPkg: Apply uncrusitify changes

19d17e0913e8 UefiCpuPkg: Change use of EFI_D_* to DEBUG_*

ffa718b4f994 SourceLevelDebugPkg: Change use of EFI_D_* to
DEBUG_*

b86cb3c5e5b4 ShellPkg: Change use of EFI_D_* to DEBUG_*

c7c42204dc07 SecurityPkg: Change use of EFI_D_* to DEBUG_*

16b8e6f958e4 PcAtChipsetPkg: Change use of EFI_D_* to
DEBUG_*

0ac3f8b2dac5 OvmfPkg: Change use of EFI_D_* to DEBUG_*

bc5004b8d294 NetworkPkg: Change use of EFI_D_* to DEBUG_*

6f671a8e2377 MdePkg: Change use of EFI_D_* to DEBUG_*

a10c610ff9a3 MdeModulePkg: Change use of EFI_D_* to
DEBUG_*

09a3bddba390 FatPkg: Change use of EFI_D_* to DEBUG_*

59c61318246a EmulatorPkg: Change use of EFI_D_* to DEBUG_*

3a80367dda3b EmbeddedPkg: Change use of EFI_D_* to DEBUG_*

23eb1aaf80ca ArmVirtPkg: Change use of EFI_D_* to DEBUG_*

875914b45c54 ArmPlatformPkg: Change use of EFI_D_* to
DEBUG_*

eb2eca82b451 ArmPkg: Change use of EFI_D_* to DEBUG_*

f0f3f5aae7c4 (origin/master, origin/HEAD, master)
UnitTestFrameworkPkg: Update YAML to ignore specific ECC
files/errors

c05734797790 UefiPayloadPkg: Update YAML to ignore specific
ECC files/errors

c30c40d6c63d StandaloneMmPkg: Update YAML to ignore specific
ECC files/errors

9944508e85f1 ShellPkg: Update YAML to ignore specific ECC
files/errors

60fa40be458d SecurityPkg: Update YAML to ignore specific ECC
files/errors

df790cd6b37e MdePkg: Update YAML to ignore specific ECC
files/errors

9deb9370766e MdeModulePkg: Update YAML to ignore specific
ECC files/errors

d7d30e8f219f EmulatorPkg: Update YAML to ignore specific ECC
files/errors

d5744ecba813 CryptoPkg: Update YAML to ignore specific ECC
files/errors

c97fee87f0f9 ArmVirtPkg: Update YAML to ignore specific ECC
files/errors

1939fc9569f2 ArmPlatformPkg: Update YAML to ignore specific
ECC files/errors

365dced2c37a ArmPkg: Update YAML to ignore specific ECC
files/errors

76a1ce4d5fec .azurepipelines/templates: Update max pipeline job
time to 2 hours

99f84ff47390 .pytools/Plugin/LicenseCheck: Use temp directory
for git diff output

3019f1bbabf1 .pytool/Plugin/EccCheck: Add performance
optimizations

854462bd3479 .pytool/Plugin/EccCheck: Remove temp directory
on exception

69877614fdee .pytool/Plugin/EccCheck: Remove RevertCode()

--ref1

ef9a059cdb15 EmulatorPkg/Win/Host: Update CC_FLAGS

bb1bba3d7767 (tag: edk2-stable202111) NetworkPkg: Fix invalid
pointer for DNS response token on error

Best regards,

Mike




Event: TianoCore Bug Triage - APAC / NAMO - 12/07/2021 #cal-reminder

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

Reminder: TianoCore Bug Triage - APAC / NAMO

When:
12/07/2021
6:30pm to 7:30pm
(UTC-08:00) America/Los Angeles

Where:
https://teams.microsoft.com/l/meetup-join/19%3ameeting_OTUyZTg2NjgtNDhlNS00ODVlLTllYTUtYzg1OTNjNjdiZjFh%40thread.v2/0?context=%7b%22Tid%22%3a%2246c98d88-e344-4ed4-8496-4ed7712e255d%22%2c%22Oid%22%3a%22b286b53a-1218-4db3-bfc9-3d4c5aa7669e%22%7d

Organizer: Liming Gao gaoliming@...

View Event

Description:

TianoCore Bug Triage - APAC / NAMO

Hosted by Liming Gao

 

________________________________________________________________________________

Microsoft Teams meeting

Join on your computer or mobile app

Click here to join the meeting

Join with a video conferencing device

teams@...

Video Conference ID: 116 062 094 0

Alternate VTC dialing instructions

Or call in (audio only)

+1 916-245-6934,,77463821#   United States, Sacramento

Phone Conference ID: 774 638 21#

Find a local number | Reset PIN

Learn More | Meeting options


Re: [PATCH V3 15/29] OvmfPkg: Update SecEntry.nasm to support Tdx

Yao, Jiewen
 

-----Original Message-----
From: kraxel@... <kraxel@...>
Sent: Monday, December 6, 2021 10:58 PM
To: Yao, Jiewen <jiewen.yao@...>
Cc: devel@edk2.groups.io; jejb@...; Xu, Min M
<min.m.xu@...>; Ard Biesheuvel <ardb+tianocore@...>; Justen,
Jordan L <jordan.l.justen@...>; Brijesh Singh <brijesh.singh@...>;
Erdem Aktas <erdemaktas@...>; Tom Lendacky
<thomas.lendacky@...>
Subject: Re: [edk2-devel] [PATCH V3 15/29] OvmfPkg: Update SecEntry.nasm to
support Tdx

Hi,

[Jiewen] Again, I feel lost.

Would you please clarify what is your purpose for this discussion?

Yes, Security related stuff in PEI, that is not a problem. For
example, we moved flash lock from DXE to PEI. (I already describe that
in my previous email.)
Well, you tried to make the point that PEI shouldn't do anything beyond
memory initialization. Which might have been correct for the initial
design, but meanwhile it is not true any more.
[Jiewen] No, I am not making this point.
My point is : It is legal to move a feature from PEI to DXE.
My understanding is that: you are making point - "It is problematic that moving a feature from PEI to DXE". I take "feature" as "any generic feature".

I accept the statement that "It *may* be problematic that moving a *specific (such as security)* feature from PEI to DXE".
But I disagree the general statement that moving any feature from PEI to DXE will cause problem.


For PEI, my point is
1) According to PI spec, PEI is minimal so it is legal to move feature from PEI to DXE.
2) We do see some examples that we move from DXE to PEI, but that is case by case. It does not mean you cannot move feature from PEI to DXE.




The key is *privilege*. If you don't need PEI privilege, you don't need move to
PEI.

2) "SMM" support is in DXE today. What do you mean SMM support in PEI ?
ovmf has a pei module for smm support (see
OvmfPkg/SmmAccess/SmmAccessPei.inf).
[Jiewen] The SmmAccessPei is for S3 resume. It is already documented to PI spec.
I don't see anything wrong here.



But I don't see how the examples support your statement on "exposure".
Well, lets stick to the flash lock example. Moving it from DXE to PEI
makes it less exposed. It'll run before DXE starts processing
externally controlled input (efi vars, kbd input, disk reads, option
roms, ...). So trying trick it into not actually locking the flash is
much harder.
[Jiewen] I am tired on using word "exposure". You gave the definition that "exposure == external interface".
And your statement is that we should move "feature" to the component with less exposure. - I take "feature" as "any generic feature".

In my opinion, less exposure != high privilege. There might be some relationship. A high privilege module may have less exposure usually. However, the privilege is *cause*, less exposure is *consequence*. Not verse versa. We made judgement based upon privilege, not the exposure.

I accept the statement that "We should move a *security* feature to component with *high privilege*".
But I disagree the general statement that we should moving any feature to less exposure.
I give a counter example - SMM. SMM has less external interface, but we cannot move "any generic feature" to SMM.



Or, to put it into another way, it reduces the attack surface for the
code with higher privilege.

(it's of course also need to make sure you can't unlock flash with a
suspend-resume cycle).

Well, I want focus on how all this will look like long-term, i.e. once
we have lazy accept implemented. I don't think it makes sense to put
much effort into optimizing a workflow which will only be temporary
anyway.
[Jiewen] This is the long-term solution.
Lazy accept and MP accept are two required feature.

"Lazy accept" just mean you can do things later, but you still need do it.
"MP accept" means you can do things much quicker.

I don't think we can remove MP accept just because we have lazy accept.
I don't want remove it. But with lazy accept you have more options to
implement it. No need to go for assembler code in SEC, you can use MP
service with standard C code in PEI or DXE.

[Jiewen] the "barely enough memory" is 64M and it takes long time to
accept if you don't use MP.
If I remember the numbers correctly (roughly 4 seconds for 2G on a
single processor) that "long time" would be roughly 12 ms for 64M.
[Jiewen] OK, I talked with Min again. 12ms is not right data today.
We have bigger number, but I cannot share the data according to legal reason.

But I agree with your statement that, if the data is small enough, then we don't need MP in sec.

I propose this way:
1) In first patch, we drop MP in SEC.
2) We can revisit if it is really needed later, when the TDX platform is about to launch.
I believe we will have more precise data at that moment.



That is the point where you start re-inventing the wheel though.
You add logic to distribute memory acceptance jobs to the APs.
I'd suggest to add full MP service support to TDX (probably also using
the mailbox), then use MP service to distribute memory acceptance jobs
to the APs. I think you will need that anyway for lazy accept, to do
parallel accept in DXE phase.
[Jiewen] I think I have stated it clearly that this is due to TDX
architecture, we have to rendezvous all APs in SEC. So the MP code
SEC is unavoidable. We have to reinvent the wheel in some way.
Well, adding TDX rendezvous support isn't re-inventing the wheel, you
surely need that, no question.

But then you go create your own job scheduler (also accept job
implementation), all in assembler, instead of using standard edk2 MP
services with more readable C code. *That* is where you re-invent the
wheel.

Now, you can see the benefit to accept PEI memory is not there.
NOTE: PEI memory is ~64M if GPAW is 48, it is ~98M if GPAW is 52, which is
a
big number.

What is all this memory needed for?
[Jiewen] These are initial memory for PEI Core and DXE Core to initialize the
content.
If you don't have initial memory, the core will fail.
Where does the 50% increase for GPAW=52 comes from?
[Jiewen] Yes, this is about page table.
The reason is that UEFI spec requires you to map all memory. You have to create page table for all.


take care,
Gerd


[PATCH][Edk2 Platform] UserAuthFeaturePkg: Add boot menu return status code to trigger function.

Dong, Eric
 

From: Bo Chang Ke <bo-changx.ke@...>

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

BIOS password is not required when overriding boot device via F7 hotkey.
Add boot menu return status code in callback function
for ReportStatusCode() notification.

Signed-off-by: Bo Chang Ke <bo-changx.ke@...>
Cc: Sai Chaganty <rangasai.v.chaganty@...>
Cc: Liming Gao <gaoliming@...>
Cc: Dandan Bi <dandan.bi@...>
---
.../UserAuthenticationDxeSmm/UserAuthenticationDxe.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthentica=
tionDxeSmm/UserAuthenticationDxe.c b/Features/Intel/UserInterface/UserAuthF=
eaturePkg/UserAuthenticationDxeSmm/UserAuthenticationDxe.c
index bba2057a96..4433fb1a20 100644
--- a/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxe=
Smm/UserAuthenticationDxe.c
+++ b/Features/Intel/UserInterface/UserAuthFeaturePkg/UserAuthenticationDxe=
Smm/UserAuthenticationDxe.c
@@ -355,7 +355,8 @@ CheckForPassword (
BOOLEAN PasswordSet;=0D
=0D
if (((CodeType & EFI_STATUS_CODE_TYPE_MASK) =3D=3D EFI_PROGRESS_CODE) &&=
=0D
- (Value =3D=3D (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_PC_USER_SETUP))) =
{=0D
+ ((Value =3D=3D (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_PC_USER_SETUP)) |=
|=0D
+ ((Value =3D=3D (EFI_SOFTWARE_DXE_BS_DRIVER | EFI_SW_DXE_BS_PC_READY_T=
O_BOOT_EVENT))))) {=0D
//=0D
// Check whether enter setup page.=0D
//=0D
--=20
2.26.2.windows.1


Re: 回复: [edk2-devel] [PATCH v1 0/2] MM communicate functionality in variable policy

Kun Qin
 

Hi Liming,

The strict check on MessageLength is only existent in `MmCommunication` driver from ArmPkg (https://github.com/tianocore/edk2/blob/e1e7306b54147e65cb7347b060e94f336d4a82d2/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c#L110). The x86 version does not seem to check MessageLength.

So the behavior does not change on X86 systems, but the MdeModulePkg change alone will fix the failure to proceed with variable policy on Arm system.

We also proposed a change in ArmPkg regarding the MessageLength check to stick closer to PI spec in this patch series.

Regards,
Kun

On 12/06/2021 17:12, gaoliming wrote:
Kun:
Does this change impact current behavior? Seemly, there is no strict check on MessageLength.
Thanks
Liming
-----邮件原件-----
发件人: Kun Qin <kuqin12@...>
发送时间: 2021年12月7日 2:47
收件人: Ard Biesheuvel <ardb@...>
抄送: edk2-devel-groups-io <devel@edk2.groups.io>; Jian J Wang
<jian.j.wang@...>; Liming Gao <gaoliming@...>; Hao A
Wu <hao.a.wu@...>; Leif Lindholm <leif@...>; Ard
Biesheuvel <ardb+tianocore@...>; Bret Barkelew
<Bret.Barkelew@...>; Michael Kubacki
<michael.kubacki@...>
主题: Re: [edk2-devel] [PATCH v1 0/2] MM communicate functionality in
variable policy

Thanks for the information, Ard. I just meant to plan ahead so that I
can work on the feedback for these patches, if any.

I can ping back the thread again once the stable tag is created.

Regards,
Kun

On 12/06/2021 10:41, Ard Biesheuvel wrote:
On Mon, 6 Dec 2021 at 19:35, Kun Qin <kuqin12@...> wrote:

Hi ArmPkg and MdeModulePkg maintainers,

It has been a week since the patches were sent. Could you please review
the changes and let me know if there is any feedback? Any input is
appreciated.
As far as I know, we are still in hard freeze for the upcoming stable tag.


On 11/29/2021 16:39, Kun Qin via groups.io wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3709
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3751

Currently, setups with variable policy operations used together with MM
communicate from ArmPkg could fail with `EFI_INVALID_PARAMETER`.
This was
due to the errors from 2 following aspects:

1. For variable policy implementations in MdeModulePkg, the DXE
runtime
agent would communicate to MM to disable, register or query policies.
However, during these operations, the MessageLength calculation is
including MM communicate header. This could lead to MM agent read
data
across the given buffer boundary and/or trigger other errors.

2. On the other hand, current MM communicate routine from ArmPkg
would
fail the function if the input message length does not equal to input
buffer size.

As defined in PI specification, the `CommSize`, when as input, should
stand for "The size of the data buffer being passed in", which would mean
the maximal number of bytes `CommBuffer` can hold. In turn, the value of
this input parameter can be used for MM handlers to determine whether
the
output data is too large to fit in this buffer. Enforcing the incoming
buffer to hold exactly the number of used bytes mismatches with the PI
spec description.

This change fix MessageLength field calculation from variable policy and
updated input argument inspections from MM communicate routine in
ArmPkg
to match PI spec descriptions.

Patch v1 branch:
https://github.com/kuqin12/edk2/tree/mm_communicate_check

Cc: Jian J Wang <jian.j.wang@...>
Cc: Liming Gao <gaoliming@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Leif Lindholm <leif@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Michael Kubacki <michael.kubacki@...>

Kun Qin (2):
MdeModulePkg: VariableSmmRuntimeDxe: Fix Variable Policy
Message
Length
ArmPkg: MmCommunicationDxe: Update MM communicate input
arguments
checks

ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
| 44 ++++++++++++--------
MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c |
10 ++---
2 files changed, 32 insertions(+), 22 deletions(-)


[PATCH 4/4] IntelSiliconPkg/VTd: Only generate PEI DMA buffer once.

Sheng Wei
 

VTdInfoNotify may be called manay times, PEI DMA buffer should be
generated only once.

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

Cc: Ray Ni <ray.ni@...>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@...>
Cc: Jenny Huang <jenny.huang@...>
Cc: Robert Kowalewski <robert.kowalewski@...>
Signed-off-by: Sheng Wei <w.sheng@...>
---
.../Feature/VTd/IntelVTdDmarPei/DmarTable.c | 405 +++++------------
.../Feature/VTd/IntelVTdDmarPei/IntelVTdDmar.c | 340 ++++++--------
.../Feature/VTd/IntelVTdDmarPei/IntelVTdDmarPei.c | 501 ++++++++++++---------
.../Feature/VTd/IntelVTdDmarPei/IntelVTdDmarPei.h | 109 +++--
.../Feature/VTd/IntelVTdDmarPei/TranslationTable.c | 175 ++++---
5 files changed, 696 insertions(+), 834 deletions(-)

diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/DmarTable.c b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/DmarTable.c
index e9c99d0a..a3ae65c3 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/DmarTable.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/DmarTable.c
@@ -1,6 +1,7 @@
/** @file

- Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2020 - 2021, Intel Corporation. All rights reserved.<BR>
+
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -413,142 +414,53 @@ GetPciBusDeviceFunction (
}

/**
- Return the index of PCI data.
-
- @param[in] VTdUnitInfo The VTd engine unit information.
- @param[in] Segment The Segment used to identify a VTd engine.
- @param[in] SourceId The SourceId used to identify a VTd engine and table entry.
-
- @return The index of the PCI data.
- @retval (UINTN)-1 The PCI data is not found.
-**/
-UINTN
-GetPciDataIndex (
- IN VTD_UNIT_INFO *VTdUnitInfo,
- IN UINT16 Segment,
- IN VTD_SOURCE_ID SourceId
- )
-{
- UINTN Index;
- VTD_SOURCE_ID *PciSourceId;
- PEI_PCI_DEVICE_DATA *PciDeviceDataBase;
-
- if (Segment != VTdUnitInfo->Segment) {
- return (UINTN)-1;
- }
-
- for (Index = 0; Index < VTdUnitInfo->PciDeviceInfo.PciDeviceDataNumber; Index++) {
- PciDeviceDataBase = (PEI_PCI_DEVICE_DATA*) (UINTN) VTdUnitInfo->PciDeviceInfo.PciDeviceData;
- PciSourceId = &PciDeviceDataBase[Index].PciSourceId;
- if ((PciSourceId->Bits.Bus == SourceId.Bits.Bus) &&
- (PciSourceId->Bits.Device == SourceId.Bits.Device) &&
- (PciSourceId->Bits.Function == SourceId.Bits.Function) ) {
- return Index;
- }
- }
-
- return (UINTN)-1;
-}
-
-
-/**
- Register PCI device to VTd engine.
+ Parse DMAR DRHD table.

- @param[in] VTdUnitInfo The VTd engine unit information.
- @param[in] Segment The segment of the source.
- @param[in] SourceId The SourceId of the source.
- @param[in] DeviceType The DMAR device scope type.
- @param[in] CheckExist TRUE: ERROR will be returned if the PCI device is already registered.
- FALSE: SUCCESS will be returned if the PCI device is registered.
+ @param[in] AcpiDmarTable DMAR ACPI table
+ @param[in] Callback Callback function for handle DRHD
+ @param[in] Context Callback function Context

- @retval EFI_SUCCESS The PCI device is registered.
- @retval EFI_OUT_OF_RESOURCES No enough resource to register a new PCI device.
- @retval EFI_ALREADY_STARTED The device is already registered.
+ @return EFI_SUCCESS The DMAR DRHD table is parsed.

**/
EFI_STATUS
-RegisterPciDevice (
- IN VTD_UNIT_INFO *VTdUnitInfo,
- IN UINT16 Segment,
- IN VTD_SOURCE_ID SourceId,
- IN UINT8 DeviceType,
- IN BOOLEAN CheckExist
+ParseDmarAcpiTableDrhd (
+ IN EFI_ACPI_DMAR_HEADER *AcpiDmarTable,
+ IN PROCESS_DRHD_CALLBACK_FUNC Callback,
+ IN VOID *Context
)
{
- PEI_PCI_DEVICE_INFORMATION *PciDeviceInfo;
- VTD_SOURCE_ID *PciSourceId;
- UINTN PciDataIndex;
- UINTN PciDeviceDataSize;
- PEI_PCI_DEVICE_DATA *NewPciDeviceData;
- PEI_PCI_DEVICE_DATA *PciDeviceDataBase;
-
- PciDeviceInfo = &VTdUnitInfo->PciDeviceInfo;
-
- PciDataIndex = GetPciDataIndex (VTdUnitInfo, Segment, SourceId);
- if (PciDataIndex == (UINTN)-1) {
- //
- // Register new
- //
-
- if (PciDeviceInfo->PciDeviceDataNumber >= PciDeviceInfo->PciDeviceDataMaxNumber) {
- //
- // Reallocate
- //
- PciDeviceDataSize = sizeof(*NewPciDeviceData) * (PciDeviceInfo->PciDeviceDataMaxNumber + MAX_VTD_PCI_DATA_NUMBER);
- DEBUG ((DEBUG_INFO, "New PciDeviceDataSize:%d Page:%d\n", PciDeviceDataSize, EFI_SIZE_TO_PAGES (PciDeviceDataSize)));
- NewPciDeviceData = AllocateZeroPages (EFI_SIZE_TO_PAGES(PciDeviceDataSize));
- if (NewPciDeviceData == NULL) {
- return EFI_OUT_OF_RESOURCES;
- }
- PciDeviceInfo->PciDeviceDataMaxNumber += MAX_VTD_PCI_DATA_NUMBER;
- if (PciDeviceInfo->PciDeviceData != 0) {
- CopyMem (NewPciDeviceData, (VOID *) (UINTN) PciDeviceInfo->PciDeviceData, sizeof (*NewPciDeviceData) * PciDeviceInfo->PciDeviceDataNumber);
- FreePages((VOID *) (UINTN) PciDeviceInfo->PciDeviceData, PciDeviceInfo->PciDeviceDataPageSize);
- }
- PciDeviceInfo->PciDeviceData = (UINT32) (UINTN) NewPciDeviceData;
- PciDeviceInfo->PciDeviceDataPageSize = (UINT32) EFI_SIZE_TO_PAGES (PciDeviceDataSize);
- }
-
- ASSERT (PciDeviceInfo->PciDeviceDataNumber < PciDeviceInfo->PciDeviceDataMaxNumber);
-
- PciDeviceDataBase = (PEI_PCI_DEVICE_DATA *) (UINTN) PciDeviceInfo->PciDeviceData;
- PciSourceId = &PciDeviceDataBase[PciDeviceInfo->PciDeviceDataNumber].PciSourceId;
- PciSourceId->Bits.Bus = SourceId.Bits.Bus;
- PciSourceId->Bits.Device = SourceId.Bits.Device;
- PciSourceId->Bits.Function = SourceId.Bits.Function;
-
- DEBUG ((DEBUG_INFO, " RegisterPciDevice: PCI S%04x B%02x D%02x F%02x", Segment, SourceId.Bits.Bus, SourceId.Bits.Device, SourceId.Bits.Function));
-
- PciDeviceDataBase[PciDeviceInfo->PciDeviceDataNumber].DeviceType = DeviceType;
-
- if ((DeviceType != EFI_ACPI_DEVICE_SCOPE_ENTRY_TYPE_PCI_ENDPOINT) &&
- (DeviceType != EFI_ACPI_DEVICE_SCOPE_ENTRY_TYPE_PCI_BRIDGE)) {
- DEBUG ((DEBUG_INFO, " (*)"));
- }
- DEBUG ((DEBUG_INFO, "\n"));
+ EFI_ACPI_DMAR_STRUCTURE_HEADER *DmarHeader;
+ UINT32 VtdIndex;

- PciDeviceInfo->PciDeviceDataNumber++;
- } else {
- if (CheckExist) {
- DEBUG ((DEBUG_INFO, " RegisterPciDevice: PCI S%04x B%02x D%02x F%02x already registered\n", Segment, SourceId.Bits.Bus, SourceId.Bits.Device, SourceId.Bits.Function));
- return EFI_ALREADY_STARTED;
+ VtdIndex = 0;
+ DmarHeader = (EFI_ACPI_DMAR_STRUCTURE_HEADER *) ((UINTN) (AcpiDmarTable + 1));
+ while ((UINTN) DmarHeader < (UINTN) AcpiDmarTable + AcpiDmarTable->Header.Length) {
+ switch (DmarHeader->Type) {
+ case EFI_ACPI_DMAR_TYPE_DRHD:
+ Callback (Context, VtdIndex, (EFI_ACPI_DMAR_DRHD_HEADER *) DmarHeader);
+ VtdIndex++;
+ break;
+ default:
+ break;
}
+ DmarHeader = (EFI_ACPI_DMAR_STRUCTURE_HEADER *) ((UINTN) DmarHeader + DmarHeader->Length);
}

return EFI_SUCCESS;
}

/**
- Process DMAR DRHD table.
+ Parse DMAR RMRR table to get device number.
+
+ @param[in] DmarRmrr The RMRR table.

- @param[in] VTdUnitInfo The VTd engine unit information.
- @param[in] DmarDrhd The DRHD table.
+ @return the device number

**/
-VOID
-ProcessDrhd (
- IN VTD_UNIT_INFO *VTdUnitInfo,
- IN EFI_ACPI_DMAR_DRHD_HEADER *DmarDrhd
+UINTN
+ProcessRmrrGetDeviceNumber (
+ IN EFI_ACPI_DMAR_RMRR_HEADER *DmarRmrr
)
{
EFI_ACPI_DMAR_DEVICE_SCOPE_STRUCTURE_HEADER *DmarDevScopeEntry;
@@ -556,189 +468,83 @@ ProcessDrhd (
UINT8 Device;
UINT8 Function;
EFI_STATUS Status;
- VTD_SOURCE_ID SourceId;
-
- DEBUG ((DEBUG_INFO," VTD BaseAddress - 0x%016lx\n", DmarDrhd->RegisterBaseAddress));
- VTdUnitInfo->VtdUnitBaseAddress = (UINT32) DmarDrhd->RegisterBaseAddress;
-
- VTdUnitInfo->EnableQueuedInvalidation = 0;
-
- DEBUG ((DEBUG_INFO," VTD Segment - %d\n", DmarDrhd->SegmentNumber));
- VTdUnitInfo->Segment = DmarDrhd->SegmentNumber;
-
- VTdUnitInfo->FixedSecondLevelPagingEntry = 0;
- VTdUnitInfo->RmrrSecondLevelPagingEntry = 0;
- VTdUnitInfo->RootEntryTable = 0;
- VTdUnitInfo->ExtRootEntryTable = 0;
- VTdUnitInfo->RootEntryTablePageSize = 0;
- VTdUnitInfo->ExtRootEntryTablePageSize = 0;
-
- VTdUnitInfo->PciDeviceInfo.IncludeAllFlag = 0;
- VTdUnitInfo->PciDeviceInfo.PciDeviceDataMaxNumber = 0;
- VTdUnitInfo->PciDeviceInfo.PciDeviceDataNumber = 0;
- VTdUnitInfo->PciDeviceInfo.PciDeviceDataPageSize = 0;
- VTdUnitInfo->PciDeviceInfo.PciDeviceData = 0;
-
- if ((DmarDrhd->Flags & EFI_ACPI_DMAR_DRHD_FLAGS_INCLUDE_PCI_ALL) != 0) {
- VTdUnitInfo->PciDeviceInfo.IncludeAllFlag = TRUE;
- DEBUG ((DEBUG_INFO," ProcessDrhd: with INCLUDE ALL\n"));
- } else {
- VTdUnitInfo->PciDeviceInfo.IncludeAllFlag = FALSE;
- DEBUG ((DEBUG_INFO," ProcessDrhd: without INCLUDE ALL\n"));
- }
+ UINTN DeviceNumber;

- VTdUnitInfo->PciDeviceInfo.PciDeviceDataNumber = 0;
- VTdUnitInfo->PciDeviceInfo.PciDeviceDataMaxNumber = 0;
- VTdUnitInfo->PciDeviceInfo.PciDeviceDataPageSize = 0;
- VTdUnitInfo->PciDeviceInfo.PciDeviceData = 0;
+ DEBUG ((DEBUG_INFO," PEI RMRR (Base 0x%016lx, Limit 0x%016lx)\n", DmarRmrr->ReservedMemoryRegionBaseAddress, DmarRmrr->ReservedMemoryRegionLimitAddress));

- DmarDevScopeEntry = (EFI_ACPI_DMAR_DEVICE_SCOPE_STRUCTURE_HEADER *) ((UINTN) (DmarDrhd + 1));
- while ((UINTN)DmarDevScopeEntry < (UINTN) DmarDrhd + DmarDrhd->Header.Length) {
+ DeviceNumber = 0;

- Status = GetPciBusDeviceFunction (DmarDrhd->SegmentNumber, DmarDevScopeEntry, &Bus, &Device, &Function);
- if (EFI_ERROR (Status)) {
- return;
- }
+ if ((DmarRmrr->ReservedMemoryRegionBaseAddress == 0) ||
+ (DmarRmrr->ReservedMemoryRegionLimitAddress == 0)) {
+ return DeviceNumber;
+ }

- DEBUG ((DEBUG_INFO," ProcessDrhd: "));
- switch (DmarDevScopeEntry->Type) {
- case EFI_ACPI_DEVICE_SCOPE_ENTRY_TYPE_PCI_ENDPOINT:
- DEBUG ((DEBUG_INFO,"PCI Endpoint"));
- break;
- case EFI_ACPI_DEVICE_SCOPE_ENTRY_TYPE_PCI_BRIDGE:
- DEBUG ((DEBUG_INFO,"PCI-PCI bridge"));
- break;
- case EFI_ACPI_DEVICE_SCOPE_ENTRY_TYPE_IOAPIC:
- DEBUG ((DEBUG_INFO,"IOAPIC"));
- break;
- case EFI_ACPI_DEVICE_SCOPE_ENTRY_TYPE_MSI_CAPABLE_HPET:
- DEBUG ((DEBUG_INFO,"MSI Capable HPET"));
- break;
- case EFI_ACPI_DEVICE_SCOPE_ENTRY_TYPE_ACPI_NAMESPACE_DEVICE:
- DEBUG ((DEBUG_INFO,"ACPI Namespace Device"));
- break;
+ DmarDevScopeEntry = (EFI_ACPI_DMAR_DEVICE_SCOPE_STRUCTURE_HEADER *) ((UINTN) (DmarRmrr + 1));
+ while ((UINTN) DmarDevScopeEntry < (UINTN) DmarRmrr + DmarRmrr->Header.Length) {
+ if (DmarDevScopeEntry->Type != EFI_ACPI_DEVICE_SCOPE_ENTRY_TYPE_PCI_ENDPOINT) {
+ DEBUG ((DEBUG_INFO,"RMRR DevScopeEntryType is not endpoint, type[0x%x] \n", DmarDevScopeEntry->Type));
+ return DeviceNumber;
}
- DEBUG ((DEBUG_INFO," S%04x B%02x D%02x F%02x\n", DmarDrhd->SegmentNumber, Bus, Device, Function));
-
- SourceId.Bits.Bus = Bus;
- SourceId.Bits.Device = Device;
- SourceId.Bits.Function = Function;

- Status = RegisterPciDevice (VTdUnitInfo, DmarDrhd->SegmentNumber, SourceId, DmarDevScopeEntry->Type, TRUE);
+ Status = GetPciBusDeviceFunction (DmarRmrr->SegmentNumber, DmarDevScopeEntry, &Bus, &Device, &Function);
if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR,"RegisterPciDevice Failed !\n"));
+ continue;
}

- DmarDevScopeEntry = (EFI_ACPI_DMAR_DEVICE_SCOPE_STRUCTURE_HEADER *) ((UINTN) DmarDevScopeEntry + DmarDevScopeEntry->Length);
- }
-}
-
-/**
- Dump the PCI device information managed by this VTd engine.
-
- @param[in] VTdInfo The VTd engine context information.
- @param[in] VtdIndex The index of VTd engine.
-
-**/
-VOID
-DumpPciDeviceInfo (
- IN VTD_INFO *VTdInfo,
- IN UINTN VtdIndex
- )
-{
- UINTN Index;
- PEI_PCI_DEVICE_DATA *PciDeviceDataBase;
-
- DEBUG ((DEBUG_INFO,"PCI Device Information (Number 0x%x, IncludeAll - %d):\n",
- VTdInfo->VtdUnitInfo[VtdIndex].PciDeviceInfo.PciDeviceDataNumber,
- VTdInfo->VtdUnitInfo[VtdIndex].PciDeviceInfo.IncludeAllFlag
- ));
+ DEBUG ((DEBUG_INFO,"RMRR S%04x B%02x D%02x F%02x\n", DmarRmrr->SegmentNumber, Bus, Device, Function));

- PciDeviceDataBase = (PEI_PCI_DEVICE_DATA *) (UINTN) VTdInfo->VtdUnitInfo[VtdIndex].PciDeviceInfo.PciDeviceData;
+ DeviceNumber++;

- for (Index = 0; Index < VTdInfo->VtdUnitInfo[VtdIndex].PciDeviceInfo.PciDeviceDataNumber; Index++) {
- DEBUG ((DEBUG_INFO," S%04x B%02x D%02x F%02x\n",
- VTdInfo->VtdUnitInfo[VtdIndex].Segment,
- PciDeviceDataBase[Index].PciSourceId.Bits.Bus,
- PciDeviceDataBase[Index].PciSourceId.Bits.Device,
- PciDeviceDataBase[Index].PciSourceId.Bits.Function
- ));
+ DmarDevScopeEntry = (EFI_ACPI_DMAR_DEVICE_SCOPE_STRUCTURE_HEADER *) ((UINTN) DmarDevScopeEntry + DmarDevScopeEntry->Length);
}
+
+ return DeviceNumber;
}

/**
- Parse DMAR DRHD table.
+ Get VTd DMAR RMRR device number.

@param[in] AcpiDmarTable DMAR ACPI table

- @return EFI_SUCCESS The DMAR DRHD table is parsed.
-
+ @return the device number
**/
-EFI_STATUS
-ParseDmarAcpiTableDrhd (
- IN EFI_ACPI_DMAR_HEADER *AcpiDmarTable
+UINTN
+GetVtdRmrrDeviceNumber (
+ IN EFI_ACPI_DMAR_HEADER *AcpiDmarTable
)
{
- EFI_ACPI_DMAR_STRUCTURE_HEADER *DmarHeader;
- UINTN VtdUnitNumber;
- UINTN VtdIndex;
- VTD_INFO *VTdInfo;
-
- VtdUnitNumber = GetVtdEngineNumber (AcpiDmarTable);
- if (VtdUnitNumber == 0) {
- return EFI_UNSUPPORTED;
- }
+ UINTN DeviceNumber;
+ EFI_ACPI_DMAR_STRUCTURE_HEADER *DmarHeader;

- VTdInfo = BuildGuidHob (&mVTdInfoGuid, sizeof (VTD_INFO) + (VtdUnitNumber - 1) * sizeof (VTD_UNIT_INFO));
- ASSERT(VTdInfo != NULL);
- if (VTdInfo == NULL) {
- return EFI_OUT_OF_RESOURCES;
- }
-
- //
- // Initialize the engine mask to all.
- //
- VTdInfo->AcpiDmarTable = (UINT32) (UINTN) AcpiDmarTable;
- VTdInfo->HostAddressWidth = AcpiDmarTable->HostAddressWidth;
- VTdInfo->VTdEngineCount = (UINT32) VtdUnitNumber;
+ DeviceNumber = 0;

- VtdIndex = 0;
DmarHeader = (EFI_ACPI_DMAR_STRUCTURE_HEADER *) ((UINTN) (AcpiDmarTable + 1));
while ((UINTN) DmarHeader < (UINTN) AcpiDmarTable + AcpiDmarTable->Header.Length) {
switch (DmarHeader->Type) {
- case EFI_ACPI_DMAR_TYPE_DRHD:
- ASSERT (VtdIndex < VtdUnitNumber);
- ProcessDrhd (&VTdInfo->VtdUnitInfo[VtdIndex], (EFI_ACPI_DMAR_DRHD_HEADER *) DmarHeader);
- VtdIndex++;
-
+ case EFI_ACPI_DMAR_TYPE_RMRR:
+ DeviceNumber += ProcessRmrrGetDeviceNumber ((EFI_ACPI_DMAR_RMRR_HEADER *) DmarHeader);
break;
-
default:
break;
}
DmarHeader = (EFI_ACPI_DMAR_STRUCTURE_HEADER *) ((UINTN) DmarHeader + DmarHeader->Length);
}
- ASSERT (VtdIndex == VtdUnitNumber);
-
- for (VtdIndex = 0; VtdIndex < VtdUnitNumber; VtdIndex++) {
- DumpPciDeviceInfo (VTdInfo, VtdIndex);
- }
-
- return EFI_SUCCESS;
+ return DeviceNumber;
}

-
/**
- Process DMAR RMRR table.
+ Parse RMRR table.

- @param[in] VTdInfo The VTd engine context information.
- @param[in] DmarRmrr The RMRR table.
+ @param[in] DmarRmrr RMRR table
+ @param[out] RmrrInfo The VTd RMRR information.
+
+ @return EFI_SUCCESS The DMAR DRHD table is parsed.

**/
VOID
ProcessRmrr (
- IN VTD_INFO *VTdInfo,
- IN EFI_ACPI_DMAR_RMRR_HEADER *DmarRmrr
+ IN EFI_ACPI_DMAR_RMRR_HEADER *DmarRmrr,
+ OUT VTD_RMRR_INFO *RmrrInfo
)
{
EFI_ACPI_DMAR_DEVICE_SCOPE_STRUCTURE_HEADER *DmarDevScopeEntry;
@@ -746,7 +552,7 @@ ProcessRmrr (
UINT8 Device;
UINT8 Function;
EFI_STATUS Status;
- VTD_SOURCE_ID SourceId;
+ UINTN DeviceIndex;

DEBUG ((DEBUG_INFO," PEI RMRR (Base 0x%016lx, Limit 0x%016lx)\n", DmarRmrr->ReservedMemoryRegionBaseAddress, DmarRmrr->ReservedMemoryRegionLimitAddress));

@@ -755,6 +561,11 @@ ProcessRmrr (
return ;
}

+ RmrrInfo->SegmentNumber = DmarRmrr->SegmentNumber;
+ RmrrInfo->ReservedMemoryRegionBaseAddress = DmarRmrr->ReservedMemoryRegionBaseAddress;
+ RmrrInfo->ReservedMemoryRegionLimitAddress = DmarRmrr->ReservedMemoryRegionLimitAddress;
+ DeviceIndex = 0;
+
DmarDevScopeEntry = (EFI_ACPI_DMAR_DEVICE_SCOPE_STRUCTURE_HEADER *) ((UINTN) (DmarRmrr + 1));
while ((UINTN) DmarDevScopeEntry < (UINTN) DmarRmrr + DmarRmrr->Header.Length) {
if (DmarDevScopeEntry->Type != EFI_ACPI_DEVICE_SCOPE_ENTRY_TYPE_PCI_ENDPOINT) {
@@ -766,50 +577,39 @@ ProcessRmrr (
if (EFI_ERROR (Status)) {
continue;
}
+ ASSERT (DeviceIndex < RmrrInfo->DeviceNumber);

DEBUG ((DEBUG_INFO,"RMRR S%04x B%02x D%02x F%02x\n", DmarRmrr->SegmentNumber, Bus, Device, Function));

- SourceId.Bits.Bus = Bus;
- SourceId.Bits.Device = Device;
- SourceId.Bits.Function = Function;
-
- Status = EnableRmrrPageAttribute (
- VTdInfo,
- DmarRmrr->SegmentNumber,
- SourceId,
- DmarRmrr->ReservedMemoryRegionBaseAddress,
- DmarRmrr->ReservedMemoryRegionLimitAddress,
- EDKII_IOMMU_ACCESS_READ | EDKII_IOMMU_ACCESS_WRITE
- );
- if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_INFO, "EnableRmrrPageAttribute : %r\n", Status));
- }
+ RmrrInfo->SourceId[DeviceIndex].Bits.Bus = Bus;
+ RmrrInfo->SourceId[DeviceIndex].Bits.Device = Device;
+ RmrrInfo->SourceId[DeviceIndex].Bits.Function = Function;
+ DeviceIndex++;

DmarDevScopeEntry = (EFI_ACPI_DMAR_DEVICE_SCOPE_STRUCTURE_HEADER *) ((UINTN) DmarDevScopeEntry + DmarDevScopeEntry->Length);
}
}

/**
- Parse DMAR DRHD table.
+ Parse DMAR RMRR table.

- @param[in] VTdInfo The VTd engine context information.
+ @param[in] AcpiDmarTable DMAR ACPI table
+ @param[out] RmrrInfo The VTd RMRR information.

**/
VOID
-ParseDmarAcpiTableRmrr (
- IN VTD_INFO *VTdInfo
+ParseRmrrDmarAcpiTable (
+ IN EFI_ACPI_DMAR_HEADER *AcpiDmarTable,
+ OUT VTD_RMRR_INFO *RmrrInfo
)
{
- EFI_ACPI_DMAR_HEADER *AcpiDmarTable;
EFI_ACPI_DMAR_STRUCTURE_HEADER *DmarHeader;

- AcpiDmarTable = (EFI_ACPI_DMAR_HEADER *) (UINTN) VTdInfo->AcpiDmarTable;
-
DmarHeader = (EFI_ACPI_DMAR_STRUCTURE_HEADER *) ((UINTN) (AcpiDmarTable + 1));
while ((UINTN) DmarHeader < (UINTN) AcpiDmarTable + AcpiDmarTable->Header.Length) {
switch (DmarHeader->Type) {
case EFI_ACPI_DMAR_TYPE_RMRR:
- ProcessRmrr (VTdInfo, (EFI_ACPI_DMAR_RMRR_HEADER *) DmarHeader);
+ ProcessRmrr ((EFI_ACPI_DMAR_RMRR_HEADER *) DmarHeader, RmrrInfo);
break;
default:
break;
@@ -818,3 +618,36 @@ ParseDmarAcpiTableRmrr (
}
}

+/**
+ Get RMRR Info from ACPI DMAR Table.
+
+ @param[in] AcpiDmarTable DMAR ACPI table
+
+ @return the VTd engine number.
+**/
+VTD_RMRR_INFO *
+GetVtdRmrrInfo (
+ IN EFI_ACPI_DMAR_HEADER *AcpiDmarTable
+ )
+{
+ UINTN DeviceNumber;
+ VTD_RMRR_INFO *RmrrInfo;
+ UINTN InfoSize;
+
+ RmrrInfo = NULL;
+ DeviceNumber = GetVtdRmrrDeviceNumber (AcpiDmarTable);
+
+ if (DeviceNumber > 0) {
+ InfoSize = sizeof (VTD_RMRR_INFO) - sizeof (VTD_SOURCE_ID) + sizeof (VTD_SOURCE_ID) * DeviceNumber;
+ RmrrInfo = (VTD_RMRR_INFO *) AllocateZeroPages (EFI_SIZE_TO_PAGES (InfoSize));
+ if (RmrrInfo == NULL) {
+ DEBUG ((DEBUG_ERROR, "GetVtdRmrrInfo - OUT_OF_RESOURCE\n"));
+ ASSERT (FALSE);
+ return NULL;
+ }
+
+ RmrrInfo->DeviceNumber = (UINT16) DeviceNumber;
+ ParseRmrrDmarAcpiTable (AcpiDmarTable, RmrrInfo);
+ }
+ return RmrrInfo;
+}
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/IntelVTdDmar.c b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/IntelVTdDmar.c
index 63397a1a..91694797 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/IntelVTdDmar.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/IntelVTdDmar.c
@@ -1,6 +1,6 @@
/** @file

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

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

@@ -79,7 +79,7 @@ PerpareCacheInvalidationInterface (
IN VTD_UNIT_INFO *VTdUnitInfo
)
{
- UINT16 QueueSize;
+ UINT16 QiDescLength;
UINT64 Reg64;
UINT32 Reg32;
VTD_ECAP_REG ECapReg;
@@ -109,12 +109,6 @@ PerpareCacheInvalidationInterface (
Reg32 = MmioRead32 ((UINTN)VTdUnitInfo->VtdUnitBaseAddress + R_GSTS_REG);
} while ((Reg32 & B_GSTS_REG_QIES) != 0);
MmioWrite64 ((UINTN)VTdUnitInfo->VtdUnitBaseAddress + R_IQA_REG, 0);
-
- if (VTdUnitInfo->QiDesc != NULL) {
- FreePages(VTdUnitInfo->QiDesc, EFI_SIZE_TO_PAGES(sizeof(QI_DESC) * VTdUnitInfo->QiDescLength));
- VTdUnitInfo->QiDesc = NULL;
- VTdUnitInfo->QiDescLength = 0;
- }
}

//
@@ -125,19 +119,19 @@ PerpareCacheInvalidationInterface (
//
// Setup the IQ address, size and descriptor width through the Invalidation Queue Address Register
//
- QueueSize = 0;
- VTdUnitInfo->QiDescLength = 1 << (QueueSize + 8);
- VTdUnitInfo->QiDesc = (QI_DESC *) AllocatePages (EFI_SIZE_TO_PAGES(sizeof(QI_DESC) * VTdUnitInfo->QiDescLength));
-
if (VTdUnitInfo->QiDesc == NULL) {
- VTdUnitInfo->QiDescLength = 0;
- DEBUG ((DEBUG_ERROR,"Could not Alloc Invalidation Queue Buffer.\n"));
- return EFI_OUT_OF_RESOURCES;
+ VTdUnitInfo->QueueSize = 0;
+ QiDescLength = 1 << (VTdUnitInfo->QueueSize + 8);
+ VTdUnitInfo->QiDesc = (QI_DESC *) AllocatePages (EFI_SIZE_TO_PAGES(sizeof(QI_DESC) * QiDescLength));
+ if (VTdUnitInfo->QiDesc == NULL) {
+ DEBUG ((DEBUG_ERROR,"Could not Alloc Invalidation Queue Buffer.\n"));
+ return EFI_OUT_OF_RESOURCES;
+ }
}

- DEBUG ((DEBUG_INFO, "Invalidation Queue Length : %d\n", VTdUnitInfo->QiDescLength));
+ DEBUG ((DEBUG_INFO, "Invalidation Queue Length : %d\n", QiDescLength));
Reg64 = (UINT64)(UINTN)VTdUnitInfo->QiDesc;
- Reg64 |= QueueSize;
+ Reg64 |= VTdUnitInfo->QueueSize;
MmioWrite64 ((UINTN)VTdUnitInfo->VtdUnitBaseAddress + R_IQA_REG, Reg64);

//
@@ -167,7 +161,8 @@ DisableQueuedInvalidationInterface (
IN VTD_UNIT_INFO *VTdUnitInfo
)
{
- UINT32 Reg32;
+ UINT32 Reg32;
+ UINT16 QiDescLength;

if (VTdUnitInfo->EnableQueuedInvalidation != 0) {
Reg32 = MmioRead32 ((UINTN)VTdUnitInfo->VtdUnitBaseAddress + R_GSTS_REG);
@@ -179,9 +174,10 @@ DisableQueuedInvalidationInterface (
} while ((Reg32 & B_GSTS_REG_QIES) != 0);

if (VTdUnitInfo->QiDesc != NULL) {
- FreePages(VTdUnitInfo->QiDesc, EFI_SIZE_TO_PAGES(sizeof(QI_DESC) * VTdUnitInfo->QiDescLength));
+ QiDescLength = 1 << (VTdUnitInfo->QueueSize + 8);
+ FreePages(VTdUnitInfo->QiDesc, EFI_SIZE_TO_PAGES(sizeof(QI_DESC) * QiDescLength));
VTdUnitInfo->QiDesc = NULL;
- VTdUnitInfo->QiDescLength = 0;
+ VTdUnitInfo->QueueSize = 0;
}

VTdUnitInfo->EnableQueuedInvalidation = 0;
@@ -204,24 +200,9 @@ QueuedInvalidationCheckFault (
UINT32 FaultReg;

FaultReg = MmioRead32 ((UINTN)VTdUnitInfo->VtdUnitBaseAddress + R_FSTS_REG);
-
- if (FaultReg & B_FSTS_REG_IQE) {
- DEBUG((DEBUG_ERROR, "Detect Invalidation Queue Error [0x%08x]\n", FaultReg));
- FaultReg |= B_FSTS_REG_IQE;
- MmioWrite32 ((UINTN)VTdUnitInfo->VtdUnitBaseAddress + R_FSTS_REG, FaultReg);
- return RETURN_DEVICE_ERROR;
- }
-
- if (FaultReg & B_FSTS_REG_ITE) {
- DEBUG((DEBUG_ERROR, "Detect Invalidation Time-out Error [0x%08x]\n", FaultReg));
- FaultReg |= B_FSTS_REG_ITE;
- MmioWrite32 ((UINTN)VTdUnitInfo->VtdUnitBaseAddress + R_FSTS_REG, FaultReg);
- return RETURN_DEVICE_ERROR;
- }
-
- if (FaultReg & B_FSTS_REG_ICE) {
- DEBUG((DEBUG_ERROR, "Detect Invalidation Completion Error [0x%08x]\n", FaultReg));
- FaultReg |= B_FSTS_REG_ICE;
+ if (FaultReg & (B_FSTS_REG_IQE | B_FSTS_REG_ITE | B_FSTS_REG_ICE)) {
+ DEBUG((DEBUG_ERROR, "Detect Queue Invalidation Error [0x%08x]\n", FaultReg));
+ FaultReg |= (B_FSTS_REG_IQE | B_FSTS_REG_ITE | B_FSTS_REG_ICE);
MmioWrite32 ((UINTN)VTdUnitInfo->VtdUnitBaseAddress + R_FSTS_REG, FaultReg);
return RETURN_DEVICE_ERROR;
}
@@ -256,7 +237,7 @@ SubmitQueuedInvalidationDescriptor (
return EFI_INVALID_PARAMETER;
}

- QiDescLength = VTdUnitInfo->QiDescLength;
+ QiDescLength = 1 << (VTdUnitInfo->QueueSize + 8);
BaseDesc = VTdUnitInfo->QiDesc;

DEBUG((DEBUG_INFO, "[0x%x] Submit QI Descriptor [0x%08x, 0x%08x]\n", VTdUnitInfo->VtdUnitBaseAddress, Desc->Low, Desc->High));
@@ -391,15 +372,15 @@ InvalidateIOTLB (
**/
EFI_STATUS
EnableDmarPreMem (
- IN UINTN VtdUnitBaseAddress,
- IN UINTN RtaddrRegValue
+ IN UINT32 VtdUnitBaseAddress,
+ IN UINT64 RtaddrRegValue
)
{
UINT32 Reg32;

DEBUG ((DEBUG_INFO, ">>>>>>EnableDmarPreMem() for engine [%x] \n", VtdUnitBaseAddress));

- DEBUG ((DEBUG_INFO, "RTADDR_REG : 0x%x \n", RtaddrRegValue));
+ DEBUG ((DEBUG_INFO, "RTADDR_REG : 0x%016lx \n", RtaddrRegValue));
MmioWrite64 (VtdUnitBaseAddress + R_RTADDR_REG, (UINT64) RtaddrRegValue);

Reg32 = MmioRead32 (VtdUnitBaseAddress + R_GSTS_REG);
@@ -566,139 +547,52 @@ DisableDmar (
}

/**
- Dump VTd version registers.
-
- @param[in] VerReg The version register.
-**/
-VOID
-DumpVtdVerRegs (
- IN VTD_VER_REG *VerReg
- )
-{
- DEBUG ((DEBUG_INFO, " VerReg:\n", VerReg->Uint32));
- DEBUG ((DEBUG_INFO, " Major - 0x%x\n", VerReg->Bits.Major));
- DEBUG ((DEBUG_INFO, " Minor - 0x%x\n", VerReg->Bits.Minor));
-}
+ Enable VTd translation table protection for block DMA

-/**
- Dump VTd capability registers.
+ @param[in] VtdUnitBaseAddress The base address of the VTd engine.

- @param[in] CapReg The capability register.
+ @retval EFI_SUCCESS DMAR translation is enabled.
+ @retval EFI_DEVICE_ERROR DMAR translation is not enabled.
**/
-VOID
-DumpVtdCapRegs (
- IN VTD_CAP_REG *CapReg
- )
-{
- DEBUG ((DEBUG_INFO, " CapReg:\n", CapReg->Uint64));
- DEBUG ((DEBUG_INFO, " ND - 0x%x\n", CapReg->Bits.ND));
- DEBUG ((DEBUG_INFO, " AFL - 0x%x\n", CapReg->Bits.AFL));
- DEBUG ((DEBUG_INFO, " RWBF - 0x%x\n", CapReg->Bits.RWBF));
- DEBUG ((DEBUG_INFO, " PLMR - 0x%x\n", CapReg->Bits.PLMR));
- DEBUG ((DEBUG_INFO, " PHMR - 0x%x\n", CapReg->Bits.PHMR));
- DEBUG ((DEBUG_INFO, " CM - 0x%x\n", CapReg->Bits.CM));
- DEBUG ((DEBUG_INFO, " SAGAW - 0x%x\n", CapReg->Bits.SAGAW));
- DEBUG ((DEBUG_INFO, " MGAW - 0x%x\n", CapReg->Bits.MGAW));
- DEBUG ((DEBUG_INFO, " ZLR - 0x%x\n", CapReg->Bits.ZLR));
- DEBUG ((DEBUG_INFO, " FRO - 0x%x\n", CapReg->Bits.FRO));
- DEBUG ((DEBUG_INFO, " SLLPS - 0x%x\n", CapReg->Bits.SLLPS));
- DEBUG ((DEBUG_INFO, " PSI - 0x%x\n", CapReg->Bits.PSI));
- DEBUG ((DEBUG_INFO, " NFR - 0x%x\n", CapReg->Bits.NFR));
- DEBUG ((DEBUG_INFO, " MAMV - 0x%x\n", CapReg->Bits.MAMV));
- DEBUG ((DEBUG_INFO, " DWD - 0x%x\n", CapReg->Bits.DWD));
- DEBUG ((DEBUG_INFO, " DRD - 0x%x\n", CapReg->Bits.DRD));
- DEBUG ((DEBUG_INFO, " FL1GP - 0x%x\n", CapReg->Bits.FL1GP));
- DEBUG ((DEBUG_INFO, " PI - 0x%x\n", CapReg->Bits.PI));
-}
-
-/**
- Dump VTd extended capability registers.
-
- @param[in] ECapReg The extended capability register.
-**/
-VOID
-DumpVtdECapRegs (
- IN VTD_ECAP_REG *ECapReg
- )
-{
- DEBUG ((DEBUG_INFO, " ECapReg:\n", ECapReg->Uint64));
- DEBUG ((DEBUG_INFO, " C - 0x%x\n", ECapReg->Bits.C));
- DEBUG ((DEBUG_INFO, " QI - 0x%x\n", ECapReg->Bits.QI));
- DEBUG ((DEBUG_INFO, " DT - 0x%x\n", ECapReg->Bits.DT));
- DEBUG ((DEBUG_INFO, " IR - 0x%x\n", ECapReg->Bits.IR));
- DEBUG ((DEBUG_INFO, " EIM - 0x%x\n", ECapReg->Bits.EIM));
- DEBUG ((DEBUG_INFO, " PT - 0x%x\n", ECapReg->Bits.PT));
- DEBUG ((DEBUG_INFO, " SC - 0x%x\n", ECapReg->Bits.SC));
- DEBUG ((DEBUG_INFO, " IRO - 0x%x\n", ECapReg->Bits.IRO));
- DEBUG ((DEBUG_INFO, " MHMV - 0x%x\n", ECapReg->Bits.MHMV));
- DEBUG ((DEBUG_INFO, " MTS - 0x%x\n", ECapReg->Bits.MTS));
- DEBUG ((DEBUG_INFO, " NEST - 0x%x\n", ECapReg->Bits.NEST));
- DEBUG ((DEBUG_INFO, " PASID - 0x%x\n", ECapReg->Bits.PASID));
- DEBUG ((DEBUG_INFO, " PRS - 0x%x\n", ECapReg->Bits.PRS));
- DEBUG ((DEBUG_INFO, " ERS - 0x%x\n", ECapReg->Bits.ERS));
- DEBUG ((DEBUG_INFO, " SRS - 0x%x\n", ECapReg->Bits.SRS));
- DEBUG ((DEBUG_INFO, " NWFS - 0x%x\n", ECapReg->Bits.NWFS));
- DEBUG ((DEBUG_INFO, " EAFS - 0x%x\n", ECapReg->Bits.EAFS));
- DEBUG ((DEBUG_INFO, " PSS - 0x%x\n", ECapReg->Bits.PSS));
- DEBUG ((DEBUG_INFO, " ADMS - 0x%x\n", ECapReg->Bits.ADMS));
-}
-
-
-/**
- Enable VTd translation table protection for all.
-
- @param[in] VTdInfo The VTd engine context information.
- @param[in] EngineMask The mask of the VTd engine to be accessed.
-**/
-VOID
-EnableVTdTranslationProtectionAll (
- IN VTD_INFO *VTdInfo,
- IN UINT64 EngineMask
+EFI_STATUS
+EnableVTdTranslationProtectionBlockDma (
+ IN UINT32 VtdUnitBaseAddress
)
{
- EFI_STATUS Status;
- EDKII_VTD_NULL_ROOT_ENTRY_TABLE_PPI *RootEntryTable;
- UINTN Index;
+ EFI_STATUS Status;
+ VTD_ECAP_REG ECapReg;
+ EDKII_VTD_NULL_ROOT_ENTRY_TABLE_PPI *RootEntryTable;

- DEBUG ((DEBUG_INFO, "EnableVTdTranslationProtectionAll - 0x%lx\n", EngineMask));
+ DEBUG ((DEBUG_INFO, "EnableVTdTranslationProtectionBlockDma - 0x%08x\n", VtdUnitBaseAddress));

- for (Index = 0; Index < VTdInfo->VTdEngineCount; Index++) {
- if ((EngineMask & LShiftU64(1, Index)) == 0) {
- continue;
- }
+ ECapReg.Uint64 = MmioRead64 (VtdUnitBaseAddress + R_ECAP_REG);
+ DEBUG ((DEBUG_INFO, "ECapReg : 0%016lx\n", ECapReg.Uint64));

- VTdInfo->VtdUnitInfo[Index].VerReg.Uint32 = MmioRead32 (VTdInfo->VtdUnitInfo[Index].VtdUnitBaseAddress + R_VER_REG);
- DumpVtdVerRegs (&VTdInfo->VtdUnitInfo[Index].VerReg);
- VTdInfo->VtdUnitInfo[Index].CapReg.Uint64 = MmioRead64 (VTdInfo->VtdUnitInfo[Index].VtdUnitBaseAddress + R_CAP_REG);
- DumpVtdCapRegs (&VTdInfo->VtdUnitInfo[Index].CapReg);
- VTdInfo->VtdUnitInfo[Index].ECapReg.Uint64 = MmioRead64 (VTdInfo->VtdUnitInfo[Index].VtdUnitBaseAddress + R_ECAP_REG);
- DumpVtdECapRegs (&VTdInfo->VtdUnitInfo[Index].ECapReg);
-
- if (VTdInfo->VtdUnitInfo[Index].ECapReg.Bits.ADMS == 1) {
- //
- // Use Abort DMA Mode
- //
- Status = EnableDmarPreMem (VTdInfo->VtdUnitInfo[Index].VtdUnitBaseAddress, V_RTADDR_REG_TTM_ADM);
- } else {
- //
- // Use Null Root Entry Table
- //
- Status = PeiServicesLocatePpi (
- &gEdkiiVTdNullRootEntryTableGuid,
- 0,
- NULL,
- (VOID **)&RootEntryTable
- );
- if (EFI_ERROR(Status)) {
- DEBUG ((DEBUG_ERROR, "Locate Null Root Entry Table Ppi Failed : %r\n", Status));
- ASSERT (FALSE);
- return;
- }
- EnableDmarPreMem (VTdInfo->VtdUnitInfo[Index].VtdUnitBaseAddress, (UINTN) *RootEntryTable);
+ if (ECapReg.Bits.ADMS == 1) {
+ //
+ // Use Abort DMA Mode
+ //
+ DEBUG ((DEBUG_INFO, "Enable abort DMA mode.\n"));
+ Status = EnableDmarPreMem (VtdUnitBaseAddress, V_RTADDR_REG_TTM_ADM);
+ } else {
+ //
+ // Use Null Root Entry Table
+ //
+ Status = PeiServicesLocatePpi (
+ &gEdkiiVTdNullRootEntryTableGuid,
+ 0,
+ NULL,
+ (VOID **)&RootEntryTable
+ );
+ if (EFI_ERROR(Status)) {
+ DEBUG ((DEBUG_ERROR, "Locate Null Root Entry Table Ppi Failed : %r\n", Status));
+ ASSERT (FALSE);
+ return EFI_DEVICE_ERROR;
}
+ Status = EnableDmarPreMem (VtdUnitBaseAddress, (UINT64) *RootEntryTable);
}

- return;
+ return Status;
}

/**
@@ -715,18 +609,25 @@ EnableVTdTranslationProtection (
)
{
EFI_STATUS Status;
- UINTN VtdIndex;
+ UINTN Index;
+ VTD_UNIT_INFO *VtdUnitInfo;
+
+ for (Index = 0; Index < VTdInfo->VTdEngineCount; Index++) {
+ VtdUnitInfo = &VTdInfo->VtdUnitInfo[Index];
+ if (VtdUnitInfo->Done) {
+ DEBUG ((DEBUG_INFO, "EnableVtdDmar (%d) was enabled\n", Index));
+ continue;
+ }

- for (VtdIndex = 0; VtdIndex < VTdInfo->VTdEngineCount; VtdIndex++) {
- if (VTdInfo->VtdUnitInfo[VtdIndex].ExtRootEntryTable != 0) {
- DEBUG ((DEBUG_INFO, "EnableVtdDmar (%d) ExtRootEntryTable 0x%x\n", VtdIndex, VTdInfo->VtdUnitInfo[VtdIndex].ExtRootEntryTable));
- Status = EnableDmar (&VTdInfo->VtdUnitInfo[VtdIndex], VTdInfo->VtdUnitInfo[VtdIndex].ExtRootEntryTable);
+ if (VtdUnitInfo->ExtRootEntryTable != 0) {
+ DEBUG ((DEBUG_INFO, "EnableVtdDmar (%d) ExtRootEntryTable 0x%x\n", Index, VtdUnitInfo->ExtRootEntryTable));
+ Status = EnableDmar (VtdUnitInfo, VtdUnitInfo->ExtRootEntryTable);
} else {
- DEBUG ((DEBUG_INFO, "EnableVtdDmar (%d) RootEntryTable 0x%x\n", VtdIndex, VTdInfo->VtdUnitInfo[VtdIndex].RootEntryTable));
- Status = EnableDmar (&VTdInfo->VtdUnitInfo[VtdIndex], VTdInfo->VtdUnitInfo[VtdIndex].RootEntryTable);
+ DEBUG ((DEBUG_INFO, "EnableVtdDmar (%d) RootEntryTable 0x%x\n", Index, VtdUnitInfo->RootEntryTable));
+ Status = EnableDmar (VtdUnitInfo, VtdUnitInfo->RootEntryTable);
}
if (EFI_ERROR (Status)) {
- DEBUG ((DEBUG_ERROR, "EnableVtdDmar (%d) Failed !\n", VtdIndex));
+ DEBUG ((DEBUG_ERROR, "EnableVtdDmar (%d) Failed !\n", Index));
return Status;
}
}
@@ -737,22 +638,21 @@ EnableVTdTranslationProtection (
Disable VTd translation table protection.

@param[in] VTdInfo The VTd engine context information.
- @param[in] EngineMask The mask of the VTd engine to be accessed.
**/
VOID
DisableVTdTranslationProtection (
- IN VTD_INFO *VTdInfo,
- IN UINT64 EngineMask
+ IN VTD_INFO *VTdInfo
)
{
UINTN Index;

- DEBUG ((DEBUG_INFO, "DisableVTdTranslationProtection - 0x%lx\n", EngineMask));
+ if (VTdInfo == NULL) {
+ return;
+ }
+
+ DEBUG ((DEBUG_INFO, "DisableVTdTranslationProtection - %d Vtd Engine\n", VTdInfo->VTdEngineCount));

for (Index = 0; Index < VTdInfo->VTdEngineCount; Index++) {
- if ((EngineMask & LShiftU64(1, Index)) == 0) {
- continue;
- }
DisableDmar ((UINTN) VTdInfo->VtdUnitInfo[Index].VtdUnitBaseAddress);

DisableQueuedInvalidationInterface(&VTdInfo->VtdUnitInfo[Index]);
@@ -786,6 +686,36 @@ PrepareVtdCacheInvalidationConfig (
return EFI_SUCCESS;
}

+/**
+
+**/
+EFI_STATUS
+VtdCheckUsing5LevelPaging (
+ IN UINT8 HostAddressWidth,
+ IN VTD_UNIT_INFO *VtdUnitInfo,
+ OUT BOOLEAN *Is5LevelPaging
+ )
+{
+ DEBUG((DEBUG_INFO, " CapReg SAGAW bits : 0x%02x\n", VtdUnitInfo->CapReg.Bits.SAGAW));
+
+ *Is5LevelPaging = FALSE;
+ if ((VtdUnitInfo->CapReg.Bits.SAGAW & BIT3) != 0) {
+ *Is5LevelPaging = TRUE;
+ if ((HostAddressWidth <= 48) &&
+ ((VtdUnitInfo->CapReg.Bits.SAGAW & BIT2) != 0)) {
+ *Is5LevelPaging = FALSE;
+ } else {
+ return EFI_UNSUPPORTED;
+ }
+ }
+ if ((VtdUnitInfo->CapReg.Bits.SAGAW & (BIT3 | BIT2)) == 0) {
+ return EFI_UNSUPPORTED;
+ }
+ DEBUG((DEBUG_INFO, " Using %d Level Paging\n", *Is5LevelPaging ? 5 : 4));
+ return EFI_SUCCESS;
+}
+
+
/**
Prepare VTD configuration.

@@ -798,43 +728,35 @@ PrepareVtdConfig (
IN VTD_INFO *VTdInfo
)
{
+ EFI_STATUS Status;
UINTN Index;
- UINTN DomainNumber;
+ VTD_UNIT_INFO *VtdUnitInfo;

for (Index = 0; Index < VTdInfo->VTdEngineCount; Index++) {
- DEBUG ((DEBUG_ERROR, "Dump VTd Capability (%d)\n", Index));
- VTdInfo->VtdUnitInfo[Index].VerReg.Uint32 = MmioRead32 (VTdInfo->VtdUnitInfo[Index].VtdUnitBaseAddress + R_VER_REG);
- DumpVtdVerRegs (&VTdInfo->VtdUnitInfo[Index].VerReg);
- VTdInfo->VtdUnitInfo[Index].CapReg.Uint64 = MmioRead64 (VTdInfo->VtdUnitInfo[Index].VtdUnitBaseAddress + R_CAP_REG);
- DumpVtdCapRegs (&VTdInfo->VtdUnitInfo[Index].CapReg);
- VTdInfo->VtdUnitInfo[Index].ECapReg.Uint64 = MmioRead64 (VTdInfo->VtdUnitInfo[Index].VtdUnitBaseAddress + R_ECAP_REG);
- DumpVtdECapRegs (&VTdInfo->VtdUnitInfo[Index].ECapReg);
-
- VTdInfo->VtdUnitInfo[Index].Is5LevelPaging = FALSE;
- if ((VTdInfo->VtdUnitInfo[Index].CapReg.Bits.SAGAW & BIT2) != 0) {
- DEBUG ((DEBUG_INFO, "Support 4-level page-table on VTD %d\n", Index));
- }
- if ((VTdInfo->VtdUnitInfo[Index].CapReg.Bits.SAGAW & BIT3) != 0) {
- DEBUG((DEBUG_INFO, "Support 5-level page-table on VTD %d\n", Index));
- VTdInfo->VtdUnitInfo[Index].Is5LevelPaging = TRUE;
-
- if ((VTdInfo->HostAddressWidth <= 48) &&
- ((VTdInfo->VtdUnitInfo[Index].CapReg.Bits.SAGAW & BIT2) != 0)) {
- DEBUG ((DEBUG_INFO, "Rollback to 4-level page-table on VTD %d\n", Index));
- VTdInfo->VtdUnitInfo[Index].Is5LevelPaging = FALSE;
- }
+ VtdUnitInfo = &VTdInfo->VtdUnitInfo[Index];
+ if (VtdUnitInfo->Done) {
+ continue;
}
- if ((VTdInfo->VtdUnitInfo[Index].CapReg.Bits.SAGAW & (BIT3 | BIT2)) == 0) {
- DEBUG ((DEBUG_ERROR, "!!!! Page-table type 0x%X is not supported on VTD %d !!!!\n", Index, VTdInfo->VtdUnitInfo[Index].CapReg.Bits.SAGAW));
- return EFI_UNSUPPORTED;
+ DEBUG((DEBUG_INFO, "VTd Engine: 0x%08X\n", VtdUnitInfo->VtdUnitBaseAddress));
+
+ VtdUnitInfo->VerReg.Uint32 = MmioRead32 (VtdUnitInfo->VtdUnitBaseAddress + R_VER_REG);
+ VtdUnitInfo->CapReg.Uint64 = MmioRead64 (VtdUnitInfo->VtdUnitBaseAddress + R_CAP_REG);
+ VtdUnitInfo->ECapReg.Uint64 = MmioRead64 (VtdUnitInfo->VtdUnitBaseAddress + R_ECAP_REG);
+ DEBUG((DEBUG_INFO, " VerReg : 0x%08X\n", VtdUnitInfo->VerReg.Uint32));
+ DEBUG((DEBUG_INFO, " CapReg : 0x%016lX\n", VtdUnitInfo->CapReg.Uint64));
+ DEBUG((DEBUG_INFO, " ECapReg : 0x%016lX\n", VtdUnitInfo->ECapReg.Uint64));
+
+ Status = VtdCheckUsing5LevelPaging (VTdInfo->HostAddressWidth, VtdUnitInfo, &(VtdUnitInfo->Is5LevelPaging));
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "!!!! Page-table type 0x%X is not supported!!!!\n", VtdUnitInfo->CapReg.Bits.SAGAW));
+ return Status;
}

- DomainNumber = (UINTN)1 << (UINT8) ((UINTN) VTdInfo->VtdUnitInfo[Index].CapReg.Bits.ND * 2 + 4);
- if (VTdInfo->VtdUnitInfo[Index].PciDeviceInfo.PciDeviceDataNumber >= DomainNumber) {
- DEBUG ((DEBUG_ERROR, "!!!! Pci device Number(0x%x) >= DomainNumber(0x%x) !!!!\n", VTdInfo->VtdUnitInfo[Index].PciDeviceInfo.PciDeviceDataNumber, DomainNumber));
- return EFI_UNSUPPORTED;
+ Status = PerpareCacheInvalidationInterface(VtdUnitInfo);
+ if (EFI_ERROR (Status)) {
+ return Status;
}
}
+
return EFI_SUCCESS;
}
-
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/IntelVTdDmarPei.c b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/IntelVTdDmarPei.c
index a8f7bfee..fd2f4dd3 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/IntelVTdDmarPei.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/IntelVTdDmarPei.c
@@ -1,6 +1,6 @@
/** @file

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

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

@@ -50,20 +50,20 @@ typedef struct {
the device driver need use SetAttribute() to update the IOMMU
attribute to request DMA access (read and/or write).

- @param[in] This The PPI instance pointer.
- @param[in] DeviceHandle The device who initiates the DMA access request.
- @param[in] Mapping The mapping value returned from Map().
- @param[in] IoMmuAccess The IOMMU access.
-
- @retval EFI_SUCCESS The IoMmuAccess is set for the memory range specified by DeviceAddress and Length.
- @retval EFI_INVALID_PARAMETER Mapping is not a value that was returned by Map().
- @retval EFI_INVALID_PARAMETER IoMmuAccess specified an illegal combination of access.
- @retval EFI_UNSUPPORTED The bit mask of IoMmuAccess is not supported by the IOMMU.
- @retval EFI_UNSUPPORTED The IOMMU does not support the memory range specified by Mapping.
- @retval EFI_OUT_OF_RESOURCES There are not enough resources available to modify the IOMMU access.
- @retval EFI_DEVICE_ERROR The IOMMU device reported an error while attempting the operation.
- @retval EFI_NOT_AVAILABLE_YET DMA protection has been enabled, but DMA buffer are
- not available to be allocated yet.
+ @param[in] This The PPI instance pointer.
+ @param[in] DeviceHandle The device who initiates the DMA access request.
+ @param[in] Mapping The mapping value returned from Map().
+ @param[in] IoMmuAccess The IOMMU access.
+
+ @retval EFI_SUCCESS The IoMmuAccess is set for the memory range specified by DeviceAddress and Length.
+ @retval EFI_INVALID_PARAMETER Mapping is not a value that was returned by Map().
+ @retval EFI_INVALID_PARAMETER IoMmuAccess specified an illegal combination of access.
+ @retval EFI_UNSUPPORTED The bit mask of IoMmuAccess is not supported by the IOMMU.
+ @retval EFI_UNSUPPORTED The IOMMU does not support the memory range specified by Mapping.
+ @retval EFI_OUT_OF_RESOURCES There are not enough resources available to modify the IOMMU access.
+ @retval EFI_DEVICE_ERROR The IOMMU device reported an error while attempting the operation.
+ @retval EFI_NOT_AVAILABLE_YET DMA protection has been enabled, but DMA buffer are
+ not available to be allocated yet.
**/
EFI_STATUS
EFIAPI
@@ -93,22 +93,22 @@ PeiIoMmuSetAttribute (
Provides the controller-specific addresses required to access system memory from a
DMA bus master.

- @param This The PPI instance pointer.
- @param Operation Indicates if the bus master is going to read or write to system memory.
- @param HostAddress The system memory address to map to the PCI controller.
- @param NumberOfBytes On input the number of bytes to map. On output the number of bytes
- that were mapped.
- @param DeviceAddress The resulting map address for the bus master PCI controller to use to
- access the hosts HostAddress.
- @param Mapping A resulting value to pass to Unmap().
-
- @retval EFI_SUCCESS The range was mapped for the returned NumberOfBytes.
- @retval EFI_UNSUPPORTED The HostAddress cannot be mapped as a common buffer.
- @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
- @retval EFI_OUT_OF_RESOURCES The request could not be completed due to a lack of resources.
- @retval EFI_DEVICE_ERROR The system hardware could not map the requested address.
- @retval EFI_NOT_AVAILABLE_YET DMA protection has been enabled, but DMA buffer are
- not available to be allocated yet.
+ @param [in] This The PPI instance pointer.
+ @param [in] Operation Indicates if the bus master is going to read or write to system memory.
+ @param [in] HostAddress The system memory address to map to the PCI controller.
+ @param [in] [out] NumberOfBytes On input the number of bytes to map. On output the number of bytes
+ that were mapped.
+ @param [out] DeviceAddress The resulting map address for the bus master PCI controller to use to
+ access the hosts HostAddress.
+ @param [out] Mapping A resulting value to pass to Unmap().
+
+ @retval EFI_SUCCESS The range was mapped for the returned NumberOfBytes.
+ @retval EFI_UNSUPPORTED The HostAddress cannot be mapped as a common buffer.
+ @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
+ @retval EFI_OUT_OF_RESOURCES The request could not be completed due to a lack of resources.
+ @retval EFI_DEVICE_ERROR The system hardware could not map the requested address.
+ @retval EFI_NOT_AVAILABLE_YET DMA protection has been enabled, but DMA buffer are
+ not available to be allocated yet.
**/
EFI_STATUS
EFIAPI
@@ -184,14 +184,14 @@ PeiIoMmuMap (
/**
Completes the Map() operation and releases any corresponding resources.

- @param This The PPI instance pointer.
- @param Mapping The mapping value returned from Map().
+ @param [in] This The PPI instance pointer.
+ @param [in] Mapping The mapping value returned from Map().

- @retval EFI_SUCCESS The range was unmapped.
- @retval EFI_INVALID_PARAMETER Mapping is not a value that was returned by Map().
- @retval EFI_DEVICE_ERROR The data was not committed to the target system memory.
- @retval EFI_NOT_AVAILABLE_YET DMA protection has been enabled, but DMA buffer are
- not available to be allocated yet.
+ @retval EFI_SUCCESS The range was unmapped.
+ @retval EFI_INVALID_PARAMETER Mapping is not a value that was returned by Map().
+ @retval EFI_DEVICE_ERROR The data was not committed to the target system memory.
+ @retval EFI_NOT_AVAILABLE_YET DMA protection has been enabled, but DMA buffer are
+ not available to be allocated yet.
**/
EFI_STATUS
EFIAPI
@@ -250,21 +250,21 @@ PeiIoMmuUnmap (
Allocates pages that are suitable for an OperationBusMasterCommonBuffer or
OperationBusMasterCommonBuffer64 mapping.

- @param This The PPI instance pointer.
- @param MemoryType The type of memory to allocate, EfiBootServicesData or
- EfiRuntimeServicesData.
- @param Pages The number of pages to allocate.
- @param HostAddress A pointer to store the base system memory address of the
- allocated range.
- @param Attributes The requested bit mask of attributes for the allocated range.
-
- @retval EFI_SUCCESS The requested memory pages were allocated.
- @retval EFI_UNSUPPORTED Attributes is unsupported. The only legal attribute bits are
- MEMORY_WRITE_COMBINE, MEMORY_CACHED and DUAL_ADDRESS_CYCLE.
- @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
- @retval EFI_OUT_OF_RESOURCES The memory pages could not be allocated.
- @retval EFI_NOT_AVAILABLE_YET DMA protection has been enabled, but DMA buffer are
- not available to be allocated yet.
+ @param [in] This The PPI instance pointer.
+ @param [in] MemoryType The type of memory to allocate, EfiBootServicesData or
+ EfiRuntimeServicesData.
+ @param [in] Pages The number of pages to allocate.
+ @param [in] [out] HostAddress A pointer to store the base system memory address of the
+ allocated range.
+ @param [in] Attributes The requested bit mask of attributes for the allocated range.
+
+ @retval EFI_SUCCESS The requested memory pages were allocated.
+ @retval EFI_UNSUPPORTED Attributes is unsupported. The only legal attribute bits are
+ MEMORY_WRITE_COMBINE, MEMORY_CACHED and DUAL_ADDRESS_CYCLE.
+ @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
+ @retval EFI_OUT_OF_RESOURCES The memory pages could not be allocated.
+ @retval EFI_NOT_AVAILABLE_YET DMA protection has been enabled, but DMA buffer are
+ not available to be allocated yet.
**/
EFI_STATUS
EFIAPI
@@ -307,15 +307,15 @@ PeiIoMmuAllocateBuffer (
/**
Frees memory that was allocated with AllocateBuffer().

- @param This The PPI instance pointer.
- @param Pages The number of pages to free.
- @param HostAddress The base system memory address of the allocated range.
+ @param [in] This The PPI instance pointer.
+ @param [in] Pages The number of pages to free.
+ @param [in] HostAddress The base system memory address of the allocated range.

- @retval EFI_SUCCESS The requested memory pages were freed.
- @retval EFI_INVALID_PARAMETER The memory range specified by HostAddress and Pages
- was not allocated with AllocateBuffer().
- @retval EFI_NOT_AVAILABLE_YET DMA protection has been enabled, but DMA buffer are
- not available to be allocated yet.
+ @retval EFI_SUCCESS The requested memory pages were freed.
+ @retval EFI_INVALID_PARAMETER The memory range specified by HostAddress and Pages
+ was not allocated with AllocateBuffer().
+ @retval EFI_NOT_AVAILABLE_YET DMA protection has been enabled, but DMA buffer are
+ not available to be allocated yet.
**/
EFI_STATUS
EFIAPI
@@ -364,52 +364,147 @@ CONST EFI_PEI_PPI_DESCRIPTOR mIoMmuPpiList = {
};

/**
- Release the momery in the Intel VTd Info
+ Get ACPI DMAT Table from EdkiiVTdInfo PPI

- @param[in] VTdInfo The VTd engine context information.
+ @retval Address ACPI DMAT Table address
+ @retval NULL Failed to get ACPI DMAT Table
**/
-VOID
-ReleaseVTdInfo (
- IN VTD_INFO *VTdInfo
+EFI_ACPI_DMAR_HEADER * GetAcpiDmarTable (
+ VOID
)
{
- UINTN Index;
+ EFI_STATUS Status;
+ EFI_ACPI_DMAR_HEADER *AcpiDmarTable;

- for (Index = 0; Index < VTdInfo->VTdEngineCount; Index++) {
- DEBUG ((DEBUG_INFO, "Release momery in VTdInfo[%d]\n", Index));
+ //
+ // Get the DMAR table
+ //
+ Status = PeiServicesLocatePpi (
+ &gEdkiiVTdInfoPpiGuid,
+ 0,
+ NULL,
+ (VOID **)&AcpiDmarTable
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Fail to get ACPI DMAR Table : %r\n", Status));
+ AcpiDmarTable = NULL;
+ } else {
+ DumpAcpiDMAR (AcpiDmarTable);
+ }

- if (VTdInfo->VtdUnitInfo[Index].FixedSecondLevelPagingEntry) {
- FreePages ((VOID *) (UINTN) VTdInfo->VtdUnitInfo[Index].FixedSecondLevelPagingEntry, 1);
- VTdInfo->VtdUnitInfo[Index].FixedSecondLevelPagingEntry = 0;
- }
+ return AcpiDmarTable;
+}

- if (VTdInfo->VtdUnitInfo[Index].RmrrSecondLevelPagingEntry) {
- FreePages ((VOID *) (UINTN) VTdInfo->VtdUnitInfo[Index].RmrrSecondLevelPagingEntry, 1);
- VTdInfo->VtdUnitInfo[Index].RmrrSecondLevelPagingEntry = 0;
- }
+/**
+ Get the VTd engine context information hob.

- if (VTdInfo->VtdUnitInfo[Index].RootEntryTable) {
- FreePages ((VOID *) (UINTN) VTdInfo->VtdUnitInfo[Index].RootEntryTable, VTdInfo->VtdUnitInfo[Index].RootEntryTablePageSize);
- VTdInfo->VtdUnitInfo[Index].RootEntryTable = 0;
- }
+ @retval The VTd engine context information.

- if (VTdInfo->VtdUnitInfo[Index].ExtRootEntryTable) {
- FreePages ((VOID *) (UINTN) VTdInfo->VtdUnitInfo[Index].ExtRootEntryTable, VTdInfo->VtdUnitInfo[Index].ExtRootEntryTablePageSize);
- VTdInfo->VtdUnitInfo[Index].RootEntryTable = 0;
+**/
+VTD_INFO *
+GetVTdInfoHob (
+ VOID
+ )
+{
+ VOID *Hob;
+ VTD_INFO *VTdInfo;
+
+ Hob = GetFirstGuidHob (&mVTdInfoGuid);
+ if (Hob == NULL) {
+ VTdInfo = BuildGuidHob (&mVTdInfoGuid, sizeof (VTD_INFO));
+ if (VTdInfo != NULL) {
+ ZeroMem (VTdInfo, sizeof (VTD_INFO));
}
+ } else {
+ VTdInfo = GET_GUID_HOB_DATA(Hob);
+ }
+ return VTdInfo;
+}
+
+/**
+ Callback function of parse DMAR DRHD table in pre-memory phase.

- if (VTdInfo->VtdUnitInfo[Index].PciDeviceInfo.PciDeviceData) {
- FreePages ((VOID *) (UINTN) VTdInfo->VtdUnitInfo[Index].PciDeviceInfo.PciDeviceData, VTdInfo->VtdUnitInfo[Index].PciDeviceInfo.PciDeviceDataPageSize);
- VTdInfo->VtdUnitInfo[Index].PciDeviceInfo.PciDeviceDataPageSize = 0;
- VTdInfo->VtdUnitInfo[Index].PciDeviceInfo.PciDeviceData = 0;
- VTdInfo->VtdUnitInfo[Index].PciDeviceInfo.PciDeviceDataNumber = 0;
- VTdInfo->VtdUnitInfo[Index].PciDeviceInfo.PciDeviceDataMaxNumber = 0;
+ @param [in] [out] Context Callback function context.
+ @param [in] VTdIndex The VTd engine index.
+ @param [in] DmarDrhd The DRHD table.
+
+**/
+VOID
+ProcessDhrdPreMemory (
+ IN OUT VOID *Context,
+ IN UINT32 VTdIndex,
+ IN EFI_ACPI_DMAR_DRHD_HEADER *DmarDrhd
+ )
+{
+ DEBUG ((DEBUG_INFO,"VTD (%d) BaseAddress - 0x%016lx\n", VTdIndex, DmarDrhd->RegisterBaseAddress));
+
+ EnableVTdTranslationProtectionBlockDma ((UINT32) DmarDrhd->RegisterBaseAddress);
+}
+
+
+/**
+ Compare 2 RMRR contexts are same or not.
+
+ @param[in] Destin Destiny RMRR Info
+ @param[in] Source Source RMRR Info
+
+ @retval Same or not same
+
+**/
+BOOLEAN
+CompareVTdRmrrInfo (
+ IN VTD_RMRR_INFO *Destin,
+ IN VTD_RMRR_INFO *Source
+ )
+{
+ UINT16 InfoSize;
+
+ if ((Destin == NULL) && (Source == NULL)) {
+ return TRUE;
+ }
+
+ if ((Destin != NULL) && (Source != NULL)) {
+ if (Destin->DeviceNumber == Source->DeviceNumber) {
+ InfoSize = sizeof (VTD_RMRR_INFO) - sizeof (VTD_SOURCE_ID) + sizeof (VTD_SOURCE_ID) * Destin->DeviceNumber;
+ if (CompareMem (Destin, Source, InfoSize) == 0) {
+ return TRUE;
+ }
}
}
+ return FALSE;
+}
+
+/**
+ Callback function of parse DMAR DRHD table in post memory phase.
+
+ @param [in] [out] Context Callback function context.
+ @param [in] VTdIndex The VTd engine index.
+ @param [in] DmarDrhd The DRHD table.
+
+**/
+VOID
+ProcessDrhdPostMemory (
+ IN OUT VOID *Context,
+ IN UINT32 VTdIndex,
+ IN EFI_ACPI_DMAR_DRHD_HEADER *DmarDrhd
+ )
+{
+ VTD_UNIT_INFO *VtdUnitInfo;
+
+ VtdUnitInfo = (VTD_UNIT_INFO *) Context;
+ VtdUnitInfo += VTdIndex;
+
+ VtdUnitInfo->Done = FALSE;
+ VtdUnitInfo->VtdUnitBaseAddress = (UINT32) DmarDrhd->RegisterBaseAddress;
+ VtdUnitInfo->Segment = DmarDrhd->SegmentNumber;
+ VtdUnitInfo->Flags = DmarDrhd->Flags;
+
+ DEBUG ((DEBUG_INFO,"VTD (%d) BaseAddress - 0x%016lx\n", VTdIndex, DmarDrhd->RegisterBaseAddress));
+ DEBUG ((DEBUG_INFO," Segment - %d, Flags - 0x%x\n", DmarDrhd->SegmentNumber, DmarDrhd->Flags));
}

/**
- Initializes the Intel VTd Info.
+ Initializes the Intel VTd Info in post memory phase.

@retval EFI_SUCCESS Usb bot driver is successfully initialized.
@retval EFI_OUT_OF_RESOURCES Can't initialize the driver.
@@ -419,89 +514,96 @@ InitVTdInfo (
VOID
)
{
- EFI_STATUS Status;
EFI_ACPI_DMAR_HEADER *AcpiDmarTable;
- VOID *Hob;
+ VTD_RMRR_INFO *RmrrInfo;
+ BOOLEAN RmrrInfoEqual;
VTD_INFO *VTdInfo;
- UINT64 EngineMask;
-
- Status = PeiServicesLocatePpi (
- &gEdkiiVTdInfoPpiGuid,
- 0,
- NULL,
- (VOID **)&AcpiDmarTable
- );
- ASSERT_EFI_ERROR (Status);
+ VTD_UNIT_INFO *VtdUnitInfo;
+ UINTN VtdUnitNumber;
+ EFI_STATUS Status;
+ UINTN i;
+ UINTN j;

- DumpAcpiDMAR (AcpiDmarTable);
+ VTdInfo = GetVTdInfoHob ();
+ ASSERT (VTdInfo != NULL);

- //
- // Clear old VTdInfo Hob.
- //
- Hob = GetFirstGuidHob (&mVTdInfoGuid);
- if (Hob != NULL) {
- DEBUG ((DEBUG_INFO, " Find Hob : mVTdInfoGuid - 0x%x\n", Hob));
+ AcpiDmarTable = GetAcpiDmarTable ();
+ ASSERT (AcpiDmarTable != NULL);

- VTdInfo = GET_GUID_HOB_DATA(Hob);
- EngineMask = LShiftU64 (1, VTdInfo->VTdEngineCount) - 1;
- EnableVTdTranslationProtectionAll (VTdInfo, EngineMask);
-
- ReleaseVTdInfo (VTdInfo);
- VTdInfo->VTdEngineCount = 0;
+ VtdUnitNumber = GetVtdEngineNumber (AcpiDmarTable);
+ if (VtdUnitNumber == 0) {
+ return EFI_UNSUPPORTED;
+ }
+ RmrrInfo = GetVtdRmrrInfo (AcpiDmarTable);
+ RmrrInfoEqual = CompareVTdRmrrInfo (VTdInfo->VtdRmrrInfo, RmrrInfo);

- ZeroMem (&((EFI_HOB_GUID_TYPE *) Hob)->Name, sizeof (EFI_GUID));
+ VtdUnitInfo = AllocateZeroPages (EFI_SIZE_TO_PAGES (sizeof (VTD_UNIT_INFO) * VtdUnitNumber));
+ if (VtdUnitInfo == NULL) {
+ DEBUG ((DEBUG_ERROR, "InitVTdInfo - OUT_OF_RESOURCE\n"));
+ ASSERT (FALSE);
+ return EFI_OUT_OF_RESOURCES;
}

- //
- // Get DMAR information to local VTdInfo
- //
- Status = ParseDmarAcpiTableDrhd (AcpiDmarTable);
- if (EFI_ERROR(Status)) {
- DEBUG ((DEBUG_ERROR, " ParseDmarAcpiTableDrhd : %r\n", Status));
+ Status = ParseDmarAcpiTableDrhd (AcpiDmarTable, ProcessDrhdPostMemory, VtdUnitInfo);
+ if (EFI_ERROR (Status)) {
+ ASSERT_EFI_ERROR (Status);
return Status;
}

//
- // NOTE: Do not parse RMRR here, because RMRR may cause DMAR programming.
+ // Check Host Address Width
//
+ if ((AcpiDmarTable->HostAddressWidth == VTdInfo->HostAddressWidth) && RmrrInfoEqual) {
+ for (i = 0; i < VtdUnitNumber; i++) {
+ //
+ // Scan each new Vtd Unit
+ //
+ for (j = 0; j < VTdInfo->VTdEngineCount; j++) {
+ //
+ // Compare the new Vtd Unit with each exist VTd Unit
+ //
+ if (VtdUnitInfo[i].VtdUnitBaseAddress == VTdInfo->VtdUnitInfo[j].VtdUnitBaseAddress) {
+ DEBUG ((DEBUG_INFO,"Find VTD [0x%08lx] Exist\n", VtdUnitInfo[i].VtdUnitBaseAddress));
+ CopyMem (&VtdUnitInfo[i], &VTdInfo->VtdUnitInfo[j], sizeof (VTD_UNIT_INFO));
+ VtdUnitInfo[i].Done = TRUE;
+
+ break;
+ }
+ }
+ }
+ }
+ VTdInfo->AcpiDmarTable = (UINT32) AcpiDmarTable;
+ VTdInfo->HostAddressWidth = AcpiDmarTable->HostAddressWidth;
+ VTdInfo->VTdEngineCount = VtdUnitNumber;
+ VTdInfo->VtdUnitInfo = VtdUnitInfo;
+ VTdInfo->VtdRmrrInfo = RmrrInfo;

return EFI_SUCCESS;
}

/**
- Initializes the Intel VTd DMAR for all memory.
+ Initializes the Intel VTd DMAR for block all DMA.

@retval EFI_SUCCESS Driver is successfully initialized.
@retval RETURN_NOT_READY Fail to get VTdInfo Hob .
**/
EFI_STATUS
-InitVTdDmarForAll (
+InitVTdDmarBlockAll (
VOID
)
{
- VOID *Hob;
- VTD_INFO *VTdInfo;
- UINT64 EngineMask;
- EFI_STATUS Status;
-
- Hob = GetFirstGuidHob (&mVTdInfoGuid);
- if (Hob == NULL) {
- DEBUG ((DEBUG_ERROR, "Fail to get VTdInfo Hob.\n"));
- return RETURN_NOT_READY;
- }
- VTdInfo = GET_GUID_HOB_DATA (Hob);
- EngineMask = LShiftU64 (1, VTdInfo->VTdEngineCount) - 1;
-
- DEBUG ((DEBUG_INFO, "PrepareVtdConfig\n"));
- Status = PrepareVtdConfig (VTdInfo);
- if (EFI_ERROR (Status)) {
- ASSERT_EFI_ERROR (Status);
- return Status;
- }
+ EFI_ACPI_DMAR_HEADER *AcpiDmarTable;

- EnableVTdTranslationProtectionAll (VTdInfo, EngineMask);
+ //
+ // Get the DMAR table
+ //
+ AcpiDmarTable = GetAcpiDmarTable ();
+ ASSERT (AcpiDmarTable != NULL);

- return EFI_SUCCESS;
+ //
+ // Parse the DMAR table and block all DMA
+ //
+ return ParseDmarAcpiTableDrhd (AcpiDmarTable, ProcessDhrdPreMemory, NULL);
}

/**
@@ -524,8 +626,8 @@ InitDmaBuffer(
DEBUG ((DEBUG_INFO, "InitDmaBuffer :\n"));

Hob = GetFirstGuidHob (&mDmaBufferInfoGuid);
+ ASSERT(Hob != NULL);
DmaBufferInfo = GET_GUID_HOB_DATA (Hob);
- VtdPmrHobPtr = GetFirstGuidHob (&gVtdPmrInfoDataHobGuid);

/**
When gVtdPmrInfoDataHobGuid exists, it means:
@@ -535,7 +637,7 @@ InitDmaBuffer(
4. Protection regions will be conveyed through VTD_PMR_INFO_HOB

When gVtdPmrInfoDataHobGuid dosen't exist, it means:
- 1. IntelVTdDmar driver will calcuate the PMR memory alignment
+ 1. IntelVTdDmarPei driver will calcuate the protected memory alignment
2. Dma buffer is reserved by AllocateAlignedPages()
**/

@@ -545,31 +647,38 @@ InitDmaBuffer(
return EFI_INVALID_PARAMETER;
}

- if (VtdPmrHobPtr == NULL) {
- //
- // Allocate memory for DMA buffer
- //
- DmaBufferInfo->DmaBufferBase = (UINT64) (UINTN) AllocateAlignedPages (EFI_SIZE_TO_PAGES ((UINTN) DmaBufferInfo->DmaBufferSize), 0);
- if (DmaBufferInfo->DmaBufferBase == 0) {
- DEBUG ((DEBUG_ERROR, " InitDmaBuffer : OutOfResource\n"));
- return EFI_OUT_OF_RESOURCES;
+ if (DmaBufferInfo->DmaBufferBase == 0) {
+ VtdPmrHobPtr = GetFirstGuidHob (&gVtdPmrInfoDataHobGuid);
+ if (VtdPmrHobPtr != NULL) {
+ //
+ // Get the protected memory ranges information from the VTd PMR hob
+ //
+ VtdPmrHob = GET_GUID_HOB_DATA (VtdPmrHobPtr);
+
+ if ((VtdPmrHob->ProtectedHighBase - VtdPmrHob->ProtectedLowLimit) < DmaBufferInfo->DmaBufferSize) {
+ DEBUG ((DEBUG_ERROR, " DmaBufferSize not enough\n"));
+ return EFI_INVALID_PARAMETER;
+ }
+ DmaBufferInfo->DmaBufferBase = VtdPmrHob->ProtectedLowLimit;
+ } else {
+ //
+ // Allocate memory for DMA buffer
+ //
+ DmaBufferInfo->DmaBufferBase = (UINT64) (UINTN) AllocateAlignedPages (EFI_SIZE_TO_PAGES ((UINTN) DmaBufferInfo->DmaBufferSize), 0);
+ if (DmaBufferInfo->DmaBufferBase == 0) {
+ DEBUG ((DEBUG_ERROR, " InitDmaBuffer : OutOfResource\n"));
+ return EFI_OUT_OF_RESOURCES;
+ }
+ DEBUG ((DEBUG_INFO, "Alloc DMA buffer success.\n"));
}
- DmaBufferInfo->DmaBufferLimit = DmaBufferInfo->DmaBufferBase + DmaBufferInfo->DmaBufferSize;
- DEBUG ((DEBUG_INFO, "Alloc DMA buffer success.\n"));
- } else {
- //
- // Get the PMR ranges information for the VTd PMR hob
- //
- VtdPmrHob = GET_GUID_HOB_DATA (VtdPmrHobPtr);
- DmaBufferInfo->DmaBufferBase = VtdPmrHob->ProtectedLowLimit;
- DmaBufferInfo->DmaBufferLimit = VtdPmrHob->ProtectedHighBase;
+
+ DmaBufferInfo->DmaBufferCurrentTop = DmaBufferInfo->DmaBufferBase + DmaBufferInfo->DmaBufferSize;
+ DmaBufferInfo->DmaBufferCurrentBottom = DmaBufferInfo->DmaBufferBase;
+
+ DEBUG ((DEBUG_INFO, " DmaBufferSize : 0x%lx\n", DmaBufferInfo->DmaBufferSize));
+ DEBUG ((DEBUG_INFO, " DmaBufferBase : 0x%lx\n", DmaBufferInfo->DmaBufferBase));
}
- DmaBufferInfo->DmaBufferCurrentTop = DmaBufferInfo->DmaBufferBase + DmaBufferInfo->DmaBufferSize;
- DmaBufferInfo->DmaBufferCurrentBottom = DmaBufferInfo->DmaBufferBase;

- DEBUG ((DEBUG_INFO, " DmaBufferSize : 0x%lx\n", DmaBufferInfo->DmaBufferSize));
- DEBUG ((DEBUG_INFO, " DmaBufferBase : 0x%lx\n", DmaBufferInfo->DmaBufferBase));
- DEBUG ((DEBUG_INFO, " DmaBufferLimit : 0x%lx\n", DmaBufferInfo->DmaBufferLimit));
DEBUG ((DEBUG_INFO, " DmaBufferCurrentTop : 0x%lx\n", DmaBufferInfo->DmaBufferCurrentTop));
DEBUG ((DEBUG_INFO, " DmaBufferCurrentBottom : 0x%lx\n", DmaBufferInfo->DmaBufferCurrentBottom));

@@ -588,14 +697,14 @@ InitVTdDmarForDma (
VOID
)
{
- VOID *Hob;
VTD_INFO *VTdInfo;
+
EFI_STATUS Status;
EFI_PEI_PPI_DESCRIPTOR *OldDescriptor;
EDKII_IOMMU_PPI *OldIoMmuPpi;

- Hob = GetFirstGuidHob (&mVTdInfoGuid);
- VTdInfo = GET_GUID_HOB_DATA (Hob);
+ VTdInfo = GetVTdInfoHob ();
+ ASSERT (VTdInfo != NULL);

DEBUG ((DEBUG_INFO, "PrepareVtdConfig\n"));
Status = PrepareVtdConfig (VTdInfo);
@@ -604,13 +713,6 @@ InitVTdDmarForDma (
return Status;
}

- DEBUG ((DEBUG_INFO, "PrepareVtdCacheInvalidationConfig\n"));
- Status = PrepareVtdCacheInvalidationConfig (VTdInfo);
- if (EFI_ERROR (Status)) {
- ASSERT_EFI_ERROR (Status);
- return Status;
- }
-
// create root entry table
DEBUG ((DEBUG_INFO, "SetupTranslationTable\n"));
Status = SetupTranslationTable (VTdInfo);
@@ -620,8 +722,11 @@ InitVTdDmarForDma (
}

// If there is RMRR memory, parse it here.
- DEBUG ((DEBUG_INFO, "PeiParseDmarAcpiTableRmrr\n"));
- ParseDmarAcpiTableRmrr (VTdInfo);
+ Status = SetupRmrr (VTdInfo);
+ if (EFI_ERROR (Status)) {
+ ASSERT_EFI_ERROR (Status);
+ return Status;
+ }

DEBUG ((DEBUG_INFO, "EnableVtdDmar\n"));
Status = EnableVTdTranslationProtection(VTdInfo);
@@ -668,21 +773,10 @@ S3EndOfPeiNotify(
IN VOID *Ppi
)
{
- VOID *Hob;
- VTD_INFO *VTdInfo;
- UINT64 EngineMask;
-
DEBUG((DEBUG_INFO, "VTd DMAR PEI S3EndOfPeiNotify\n"));

if ((PcdGet8(PcdVTdPolicyPropertyMask) & BIT1) == 0) {
- Hob = GetFirstGuidHob (&mVTdInfoGuid);
- if (Hob == NULL) {
- return EFI_SUCCESS;
- }
- VTdInfo = GET_GUID_HOB_DATA(Hob);
-
- EngineMask = LShiftU64 (1, VTdInfo->VTdEngineCount) - 1;
- DisableVTdTranslationProtection (VTdInfo, EngineMask);
+ DisableVTdTranslationProtection (GetVTdInfoHob ());
}
return EFI_SUCCESS;
}
@@ -733,18 +827,13 @@ VTdInfoNotify (

DEBUG ((DEBUG_INFO, "MemoryInitialized - %x\n", MemoryInitialized));

- //
- // NOTE: We need reinit VTdInfo because previous information might be overriden.
- //
- InitVTdInfo ();
-
if (!MemoryInitialized) {
//
// If the memory is not initialized,
// Protect all system memory
//

- InitVTdDmarForAll ();
+ InitVTdDmarBlockAll ();

//
// Install PPI.
@@ -758,9 +847,16 @@ VTdInfoNotify (
//

Status = InitDmaBuffer ();
- ASSERT_EFI_ERROR(Status);
+ ASSERT_EFI_ERROR (Status);

- InitVTdDmarForDma ();
+ //
+ // NOTE: We need reinit VTdInfo because previous information might be overriden.
+ //
+ Status = InitVTdInfo ();
+ ASSERT_EFI_ERROR (Status);
+
+ Status = InitVTdDmarForDma ();
+ ASSERT_EFI_ERROR (Status);
}

return EFI_SUCCESS;
@@ -826,4 +922,3 @@ IntelVTdDmarInitialize (

return EFI_SUCCESS;
}
-
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/IntelVTdDmarPei.h b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/IntelVTdDmarPei.h
index e23a6c8e..74667277 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/IntelVTdDmarPei.h
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/IntelVTdDmarPei.h
@@ -9,68 +9,71 @@
#ifndef __DMA_ACCESS_LIB_H__
#define __DMA_ACCESS_LIB_H__

-#define MAX_VTD_PCI_DATA_NUMBER 0x100
-
#define VTD_64BITS_ADDRESS(Lo, Hi) (LShiftU64 (Lo, 12) | LShiftU64 (Hi, 32))

typedef struct {
- UINT8 DeviceType;
- VTD_SOURCE_ID PciSourceId;
-} PEI_PCI_DEVICE_DATA;
-
-typedef struct {
- BOOLEAN IncludeAllFlag;
- UINT32 PciDeviceDataNumber;
- UINT32 PciDeviceDataMaxNumber;
- UINT32 PciDeviceDataPageSize;
- UINT32 PciDeviceData;
-} PEI_PCI_DEVICE_INFORMATION;
+ UINT16 SegmentNumber;
+ UINT64 ReservedMemoryRegionBaseAddress;
+ UINT64 ReservedMemoryRegionLimitAddress;
+ UINT16 DeviceNumber;
+ VTD_SOURCE_ID SourceId[1];
+} VTD_RMRR_INFO;

typedef struct {
+ BOOLEAN Done;
UINT32 VtdUnitBaseAddress;
UINT16 Segment;
+ UINT8 Flags;
VTD_VER_REG VerReg;
VTD_CAP_REG CapReg;
VTD_ECAP_REG ECapReg;
BOOLEAN Is5LevelPaging;
+ UINT8 EnableQueuedInvalidation;
+ UINT16 QueueSize;
+ QI_DESC *QiDesc;
+ UINT16 QiFreeHead;
UINT32 FixedSecondLevelPagingEntry;
- UINT32 RmrrSecondLevelPagingEntry;
UINT32 RootEntryTable;
UINT32 ExtRootEntryTable;
UINT16 RootEntryTablePageSize;
UINT16 ExtRootEntryTablePageSize;
- PEI_PCI_DEVICE_INFORMATION PciDeviceInfo;
- UINT8 EnableQueuedInvalidation;
- UINT16 QiDescLength;
- QI_DESC *QiDesc;
- UINT16 QiFreeHead;
+ UINT32 RmrrSecondLevelPagingEntry;
} VTD_UNIT_INFO;

typedef struct {
UINT32 AcpiDmarTable;
UINT8 HostAddressWidth;
UINT32 VTdEngineCount;
- VTD_UNIT_INFO VtdUnitInfo[1];
+ VTD_UNIT_INFO *VtdUnitInfo;
+ VTD_RMRR_INFO *VtdRmrrInfo;
} VTD_INFO;

typedef struct {
UINT64 DmaBufferBase;
UINT64 DmaBufferSize;
- UINT64 DmaBufferLimit;
UINT64 DmaBufferCurrentTop;
UINT64 DmaBufferCurrentBottom;
} DMA_BUFFER_INFO;

+typedef
+VOID
+(EFIAPI *PROCESS_DRHD_CALLBACK_FUNC) (
+ IN OUT VOID *Context,
+ IN UINT32 VTdIndex,
+ IN EFI_ACPI_DMAR_DRHD_HEADER *DmarDrhd
+ );
+
/**
- Enable VTd translation table protection.
+ Enable VTd translation table protection for block DMA

- @param[in] VTdInfo The VTd engine context information.
- @param[in] EngineMask The mask of the VTd engine to be accessed.
+ @param[in] VtdUnitBaseAddress The base address of the VTd engine.
+
+ @retval EFI_SUCCESS DMAR translation is enabled.
+ @retval EFI_DEVICE_ERROR DMAR translation is not enabled.
**/
-VOID
-EnableVTdTranslationProtectionAll (
- IN VTD_INFO *VTdInfo,
- IN UINT64 EngineMask
+EFI_STATUS
+EnableVTdTranslationProtectionBlockDma (
+ IN UINT32 VtdUnitBaseAddress
);

/**
@@ -90,34 +93,39 @@ EnableVTdTranslationProtection (
Disable VTd translation table protection.

@param[in] VTdInfo The VTd engine context information.
- @param[in] EngineMask The mask of the VTd engine to be accessed.
**/
VOID
DisableVTdTranslationProtection (
- IN VTD_INFO *VTdInfo,
- IN UINT64 EngineMask
+ IN VTD_INFO *VTdInfo
);

/**
Parse DMAR DRHD table.

@param[in] AcpiDmarTable DMAR ACPI table
+ @param[in] Callback Callback function for handle DRHD
+ @param[in] Context Callback function Context
+
+ @return EFI_SUCCESS The DMAR DRHD table is parsed.

- @return EFI_SUCCESS The DMAR DRHD table is parsed.
**/
EFI_STATUS
ParseDmarAcpiTableDrhd (
- IN EFI_ACPI_DMAR_HEADER *AcpiDmarTable
+ IN EFI_ACPI_DMAR_HEADER *AcpiDmarTable,
+ IN PROCESS_DRHD_CALLBACK_FUNC Callback,
+ IN VOID *Context
);

/**
- Parse DMAR DRHD table.
+ Get RMRR Info from ACPI DMAR Table.

- @param[in] VTdInfo The VTd engine context information.
+ @param[in] AcpiDmarTable DMAR ACPI table
+
+ @return the VTd engine number.
**/
-VOID
-ParseDmarAcpiTableRmrr (
- IN VTD_INFO *VTdInfo
+VTD_RMRR_INFO *
+GetVtdRmrrInfo (
+ IN EFI_ACPI_DMAR_HEADER *AcpiDmarTable
);

/**
@@ -167,6 +175,30 @@ SetupTranslationTable (
IN VTD_INFO *VTdInfo
);

+/**
+ Get VTd engine number.
+
+ @param[in] AcpiDmarTable DMAR ACPI table
+
+ @return the VTd engine number.
+**/
+UINTN
+GetVtdEngineNumber (
+ IN EFI_ACPI_DMAR_HEADER *AcpiDmarTable
+ );
+
+/**
+ Setup VTd RMRR in translation table.
+
+ @param[in] VTdInfo The VTd engine context information.
+
+ @return the VTd engine number.
+**/
+EFI_STATUS
+SetupRmrr (
+ IN VTD_INFO *VTdInfo
+ );
+
/**
Flush VTD page table and context table memory.

@@ -240,4 +272,3 @@ extern EFI_GUID mVTdInfoGuid;
extern EFI_GUID mDmaBufferInfoGuid;

#endif
-
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/TranslationTable.c b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/TranslationTable.c
index a309d566..405ea630 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/TranslationTable.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/TranslationTable.c
@@ -1,6 +1,7 @@
/** @file

Copyright (c) 2020 - 2021, Intel Corporation. All rights reserved.<BR>
+
SPDX-License-Identifier: BSD-2-Clause-Patent

**/
@@ -276,6 +277,10 @@ CreateContextEntry (
VTD_SECOND_LEVEL_PAGING_ENTRY *SecondLevelPagingEntry;
UINT64 Pt;

+ if (VTdUnitInfo->RootEntryTable != 0) {
+ return EFI_SUCCESS;
+ }
+
RootPages = EFI_SIZE_TO_PAGES (sizeof (VTD_ROOT_ENTRY) * VTD_ROOT_ENTRY_NUMBER);
ContextPages = EFI_SIZE_TO_PAGES (sizeof (VTD_CONTEXT_ENTRY) * VTD_CONTEXT_ENTRY_NUMBER);
EntryTablePages = RootPages + ContextPages * (VTD_ROOT_ENTRY_NUMBER);
@@ -359,6 +364,10 @@ CreateExtContextEntry (
VTD_SECOND_LEVEL_PAGING_ENTRY *SecondLevelPagingEntry;
UINT64 Pt;

+ if (VTdUnitInfo->ExtRootEntryTable != 0) {
+ return EFI_SUCCESS;
+ }
+
RootPages = EFI_SIZE_TO_PAGES (sizeof (VTD_EXT_ROOT_ENTRY) * VTD_ROOT_ENTRY_NUMBER);
ContextPages = EFI_SIZE_TO_PAGES (sizeof (VTD_EXT_CONTEXT_ENTRY) * VTD_CONTEXT_ENTRY_NUMBER);
EntryTablePages = RootPages + ContextPages * (VTD_ROOT_ENTRY_NUMBER);
@@ -837,6 +846,10 @@ CreateFixedSecondLevelPagingEntry (
VOID *Hob;
DMA_BUFFER_INFO *DmaBufferInfo;

+ if (VTdUnitInfo->FixedSecondLevelPagingEntry != 0) {
+ return EFI_SUCCESS;
+ }
+
VTdUnitInfo->FixedSecondLevelPagingEntry = (UINT32) (UINTN) CreateSecondLevelPagingEntryTable (VTdUnitInfo, NULL, 0, SIZE_4GB, 0);
if (VTdUnitInfo->FixedSecondLevelPagingEntry == 0) {
DEBUG ((DEBUG_ERROR, "FixedSecondLevelPagingEntry is empty\n"));
@@ -846,7 +859,7 @@ CreateFixedSecondLevelPagingEntry (
Hob = GetFirstGuidHob (&mDmaBufferInfoGuid);
DmaBufferInfo = GET_GUID_HOB_DATA (Hob);
BaseAddress = DmaBufferInfo->DmaBufferBase;
- Length = DmaBufferInfo->DmaBufferLimit - DmaBufferInfo->DmaBufferBase;
+ Length = DmaBufferInfo->DmaBufferSize;
IoMmuAccess = EDKII_IOMMU_ACCESS_READ | EDKII_IOMMU_ACCESS_WRITE;

DEBUG ((DEBUG_INFO, " BaseAddress = 0x%lx\n", BaseAddress));
@@ -877,6 +890,9 @@ SetupTranslationTable (

for (Index = 0; Index < VTdInfo->VTdEngineCount; Index++) {
VtdUnitInfo = &VTdInfo->VtdUnitInfo[Index];
+ if (VtdUnitInfo->Done) {
+ continue;
+ }

Status = CreateFixedSecondLevelPagingEntry (VtdUnitInfo);
if (EFI_ERROR (Status)) {
@@ -912,83 +928,45 @@ SetupTranslationTable (
}

/**
- Find the VTd index by the Segment and SourceId.
+ Setup VTd RMRR in translation table.

@param[in] VTdInfo The VTd engine context information.
- @param[in] Segment The segment of the source.
- @param[in] SourceId The SourceId of the source.
- @param[out] ExtContextEntry The ExtContextEntry of the source.
- @param[out] ContextEntry The ContextEntry of the source.

- @return The index of the VTd engine.
- @retval (UINTN)-1 The VTd engine is not found.
+ @return the VTd engine number.
**/
-UINTN
-FindVtdIndexBySegmentSourceId (
- IN VTD_INFO *VTdInfo,
- IN UINT16 Segment,
- IN VTD_SOURCE_ID SourceId,
- OUT VTD_EXT_CONTEXT_ENTRY **ExtContextEntry,
- OUT VTD_CONTEXT_ENTRY **ContextEntry
+EFI_STATUS
+SetupRmrr (
+ IN VTD_INFO *VTdInfo
)
{
- UINTN VtdIndex;
- VTD_ROOT_ENTRY *RootEntryBase;
- VTD_ROOT_ENTRY *RootEntry;
- VTD_CONTEXT_ENTRY *ContextEntryTable;
- VTD_CONTEXT_ENTRY *ThisContextEntry;
- VTD_EXT_ROOT_ENTRY *ExtRootEntryBase;
- VTD_EXT_ROOT_ENTRY *ExtRootEntry;
- VTD_EXT_CONTEXT_ENTRY *ExtContextEntryTable;
- VTD_EXT_CONTEXT_ENTRY *ThisExtContextEntry;
+ EFI_STATUS Status;
+ UINTN Index;

- for (VtdIndex = 0; VtdIndex < VTdInfo->VTdEngineCount; VtdIndex++) {
- if (GetPciDataIndex (&VTdInfo->VtdUnitInfo[VtdIndex], Segment, SourceId) != (UINTN)-1) {
- DEBUG ((DEBUG_INFO, "Find VtdIndex(0x%x) for S%04x B%02x D%02x F%02x\n", VtdIndex, Segment, SourceId.Bits.Bus, SourceId.Bits.Device, SourceId.Bits.Function));
- break;
- }
+ if (VTdInfo->VtdRmrrInfo == NULL) {
+ return EFI_SUCCESS;
}
- if (VtdIndex >= VTdInfo->VTdEngineCount) {
- for (VtdIndex = 0; VtdIndex < VTdInfo->VTdEngineCount; VtdIndex++) {
- if (Segment != VTdInfo->VtdUnitInfo[VtdIndex].Segment) {
- continue;
- }
- if (VTdInfo->VtdUnitInfo[VtdIndex].PciDeviceInfo.IncludeAllFlag) {
- DEBUG ((DEBUG_INFO, "Find IncludeAllFlag VtdIndex(0x%x) for S%04x B%02x D%02x F%02x\n", VtdIndex, Segment, SourceId.Bits.Bus, SourceId.Bits.Device, SourceId.Bits.Function));
- break;
- }
- }
+
+ if (VTdInfo->VtdRmrrInfo->DeviceNumber == 0) {
+ return EFI_SUCCESS;
}

- if (VtdIndex < VTdInfo->VTdEngineCount) {
- ExtRootEntryBase = (VTD_EXT_ROOT_ENTRY *) (UINTN) VTdInfo->VtdUnitInfo[VtdIndex].ExtRootEntryTable;
- if (ExtRootEntryBase != 0) {
- ExtRootEntry = &ExtRootEntryBase[SourceId.Index.RootIndex];
- ExtContextEntryTable = (VTD_EXT_CONTEXT_ENTRY *) (UINTN) VTD_64BITS_ADDRESS(ExtRootEntry->Bits.LowerContextTablePointerLo, ExtRootEntry->Bits.LowerContextTablePointerHi) ;
- ThisExtContextEntry = &ExtContextEntryTable[SourceId.Index.ContextIndex];
- if (ThisExtContextEntry->Bits.AddressWidth == 0) {
- DEBUG ((DEBUG_INFO, "ExtContextEntry AddressWidth : 0x%x\n", ThisExtContextEntry->Bits.AddressWidth));
- return (UINTN)-1;
- }
- *ExtContextEntry = ThisExtContextEntry;
- *ContextEntry = NULL;
- } else {
- RootEntryBase = (VTD_ROOT_ENTRY*) (UINTN) VTdInfo->VtdUnitInfo[VtdIndex].RootEntryTable;
- RootEntry = &RootEntryBase[SourceId.Index.RootIndex];
- ContextEntryTable = (VTD_CONTEXT_ENTRY *) (UINTN) VTD_64BITS_ADDRESS(RootEntry->Bits.ContextTablePointerLo, RootEntry->Bits.ContextTablePointerHi) ;
- ThisContextEntry = &ContextEntryTable[SourceId.Index.ContextIndex];
- if (ThisContextEntry->Bits.AddressWidth == 0) {
- DEBUG ((DEBUG_INFO, "ContextEntry AddressWidth : 0x%x\n", ThisContextEntry->Bits.AddressWidth));
- return (UINTN)-1;
- }
- *ExtContextEntry = NULL;
- *ContextEntry = ThisContextEntry;
+ DEBUG ((DEBUG_INFO, "Setup Rmrr Info\n"));
+
+ for (Index = 0; Index < VTdInfo->VtdRmrrInfo->DeviceNumber; Index++) {
+ Status = EnableRmrrPageAttribute (
+ VTdInfo,
+ VTdInfo->VtdRmrrInfo->SegmentNumber,
+ VTdInfo->VtdRmrrInfo->SourceId[Index],
+ VTdInfo->VtdRmrrInfo->ReservedMemoryRegionBaseAddress,
+ VTdInfo->VtdRmrrInfo->ReservedMemoryRegionLimitAddress,
+ EDKII_IOMMU_ACCESS_READ | EDKII_IOMMU_ACCESS_WRITE
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "EnableRmrrPageAttribute : %r\n", Status));
}
-
- return VtdIndex;
}

- return (UINTN)-1;
+ return EFI_SUCCESS;
}

/**
@@ -1013,8 +991,9 @@ EnableRmrrPageAttribute (
IN UINT64 IoMmuAccess
)
{
- EFI_STATUS Status;
- UINTN VtdIndex;
+ UINTN Index;
+ VTD_UNIT_INFO *VtdUnitInfo;
+ EFI_STATUS Status;
VTD_EXT_CONTEXT_ENTRY *ExtContextEntry;
VTD_CONTEXT_ENTRY *ContextEntry;
VTD_SECOND_LEVEL_PAGING_ENTRY *SecondLevelPagingEntry;
@@ -1022,39 +1001,41 @@ EnableRmrrPageAttribute (

DEBUG ((DEBUG_INFO, "EnableRmrrPageAttribute (S%04x B%02x D%02x F%02x)\n", Segment, SourceId.Bits.Bus, SourceId.Bits.Device, SourceId.Bits.Function));

- VtdIndex = FindVtdIndexBySegmentSourceId (VTdInfo, Segment, SourceId, &ExtContextEntry, &ContextEntry);
- if (VtdIndex == (UINTN)-1) {
- DEBUG ((DEBUG_ERROR, "EnableRmrrPageAttribute - Can not locate Pci device (S%04x B%02x D%02x F%02x) !\n", Segment, SourceId.Bits.Bus, SourceId.Bits.Device, SourceId.Bits.Function));
- return EFI_DEVICE_ERROR;
- }
-
- if (VTdInfo->VtdUnitInfo[VtdIndex].RmrrSecondLevelPagingEntry == 0) {
- DEBUG ((DEBUG_INFO, "CreateSecondLevelPagingEntry - %d\n", VtdIndex));
- VTdInfo->VtdUnitInfo[VtdIndex].RmrrSecondLevelPagingEntry = (UINT32)(UINTN)CreateSecondLevelPagingEntryTable (&VTdInfo->VtdUnitInfo[VtdIndex], NULL, 0, SIZE_4GB, 0);
- if (VTdInfo->VtdUnitInfo[VtdIndex].RmrrSecondLevelPagingEntry == 0) {
- return EFI_OUT_OF_RESOURCES;
+ for (Index = 0; Index < VTdInfo->VTdEngineCount; Index++) {
+ VtdUnitInfo = &VTdInfo->VtdUnitInfo[Index];
+ if (VtdUnitInfo->Done) {
+ continue;
}

- Status =SetSecondLevelPagingAttribute (&VTdInfo->VtdUnitInfo[VtdIndex], (VTD_SECOND_LEVEL_PAGING_ENTRY*)(UINTN)VTdInfo->VtdUnitInfo[VtdIndex].RmrrSecondLevelPagingEntry, MemoryBase, MemoryLimit + 1 - MemoryBase, IoMmuAccess);
- if (EFI_ERROR (Status)) {
- return Status;
- }
- }
+ if (VtdUnitInfo->RmrrSecondLevelPagingEntry == 0) {
+ DEBUG ((DEBUG_INFO, "CreateSecondLevelPagingEntry - %d\n", Index));
+ VtdUnitInfo->RmrrSecondLevelPagingEntry = (UINT32)(UINTN)CreateSecondLevelPagingEntryTable (VtdUnitInfo, NULL, 0, SIZE_4GB, 0);
+ if (VtdUnitInfo->RmrrSecondLevelPagingEntry == 0) {
+ return EFI_OUT_OF_RESOURCES;
+ }

- SecondLevelPagingEntry = (VTD_SECOND_LEVEL_PAGING_ENTRY *) (UINTN) VTdInfo->VtdUnitInfo[VtdIndex].RmrrSecondLevelPagingEntry;
- Pt = (UINT64) RShiftU64 ((UINT64) (UINTN) SecondLevelPagingEntry, 12);
- if (ExtContextEntry != NULL) {
- ExtContextEntry->Bits.SecondLevelPageTranslationPointerLo = (UINT32) Pt;
- ExtContextEntry->Bits.SecondLevelPageTranslationPointerHi = (UINT32) RShiftU64(Pt, 20);
- ExtContextEntry->Bits.DomainIdentifier = ((1 << (UINT8) ((UINTN) VTdInfo->VtdUnitInfo[VtdIndex].CapReg.Bits.ND * 2 + 4)) - 1);
- ExtContextEntry->Bits.Present = 1;
- FlushPageTableMemory (&VTdInfo->VtdUnitInfo[VtdIndex], (UINTN) ExtContextEntry, sizeof(*ExtContextEntry));
- } else if (ContextEntry != NULL) {
- ContextEntry->Bits.SecondLevelPageTranslationPointerLo = (UINT32) Pt;
- ContextEntry->Bits.SecondLevelPageTranslationPointerHi = (UINT32) RShiftU64 (Pt, 20);
- ContextEntry->Bits.DomainIdentifier = ((1 << (UINT8) ((UINTN) VTdInfo->VtdUnitInfo[VtdIndex].CapReg.Bits.ND * 2 + 4)) - 1);
- ContextEntry->Bits.Present = 1;
- FlushPageTableMemory (&VTdInfo->VtdUnitInfo[VtdIndex], (UINTN) ContextEntry, sizeof (*ContextEntry));
+ Status = SetSecondLevelPagingAttribute (VtdUnitInfo, (VTD_SECOND_LEVEL_PAGING_ENTRY*)(UINTN)VtdUnitInfo->RmrrSecondLevelPagingEntry, MemoryBase, MemoryLimit + 1 - MemoryBase, IoMmuAccess);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ }
+ SecondLevelPagingEntry = (VTD_SECOND_LEVEL_PAGING_ENTRY *) (UINTN) VtdUnitInfo->RmrrSecondLevelPagingEntry;
+ ExtContextEntry = (VTD_EXT_CONTEXT_ENTRY *) VtdUnitInfo->ExtRootEntryTable;
+ ContextEntry = (VTD_CONTEXT_ENTRY *) VtdUnitInfo->RootEntryTable;
+ Pt = (UINT64) RShiftU64 ((UINT64) (UINTN) SecondLevelPagingEntry, 12);
+ if (ExtContextEntry != NULL) {
+ ExtContextEntry->Bits.SecondLevelPageTranslationPointerLo = (UINT32) Pt;
+ ExtContextEntry->Bits.SecondLevelPageTranslationPointerHi = (UINT32) RShiftU64(Pt, 20);
+ ExtContextEntry->Bits.DomainIdentifier = ((1 << (UINT8) ((UINTN) VtdUnitInfo->CapReg.Bits.ND * 2 + 4)) - 1);
+ ExtContextEntry->Bits.Present = 1;
+ FlushPageTableMemory (VtdUnitInfo, (UINTN) ExtContextEntry, sizeof(*ExtContextEntry));
+ } else if (ContextEntry != NULL) {
+ ContextEntry->Bits.SecondLevelPageTranslationPointerLo = (UINT32) Pt;
+ ContextEntry->Bits.SecondLevelPageTranslationPointerHi = (UINT32) RShiftU64 (Pt, 20);
+ ContextEntry->Bits.DomainIdentifier = ((1 << (UINT8) ((UINTN) VtdUnitInfo->CapReg.Bits.ND * 2 + 4)) - 1);
+ ContextEntry->Bits.Present = 1;
+ FlushPageTableMemory (VtdUnitInfo, (UINTN) ContextEntry, sizeof (*ContextEntry));
+ }
}

return EFI_SUCCESS;
--
2.16.2.windows.1


[PATCH 3/4] IntelSiliconPkg/VTd: Support VTd Abort DMA Mode

Sheng Wei
 

If VTd ECAP_REG.ADMS bit is set, abort DMA mode is supported.
When VTd Abort DMA Mode is enabled, hardware will abort all DMA
operations without the need to set up a root-table with each
entry marked as not-present.

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

Cc: Ray Ni <ray.ni@...>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@...>
Cc: Jenny Huang <jenny.huang@...>
Cc: Robert Kowalewski <robert.kowalewski@...>
Signed-off-by: Sheng Wei <w.sheng@...>
---
.../Feature/VTd/IntelVTdDmarPei/IntelVTdDmar.c | 43 +++++++++++++---------
1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/IntelVTdDmar.c b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/IntelVTdDmar.c
index 87ce9716..63397a1a 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/IntelVTdDmar.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/IntelVTdDmar.c
@@ -384,7 +384,7 @@ InvalidateIOTLB (
Enable DMAR translation inpre-mem phase.

@param[in] VtdUnitBaseAddress The base address of the VTd engine.
- @param[in] RootEntryTable The address of the VTd RootEntryTable.
+ @param[in] RtaddrRegValue The value of RTADDR_REG.

@retval EFI_SUCCESS DMAR translation is enabled.
@retval EFI_DEVICE_ERROR DMAR translation is not enabled.
@@ -392,15 +392,15 @@ InvalidateIOTLB (
EFI_STATUS
EnableDmarPreMem (
IN UINTN VtdUnitBaseAddress,
- IN UINTN RootEntryTable
+ IN UINTN RtaddrRegValue
)
{
UINT32 Reg32;

DEBUG ((DEBUG_INFO, ">>>>>>EnableDmarPreMem() for engine [%x] \n", VtdUnitBaseAddress));

- DEBUG ((DEBUG_INFO, "RootEntryTable 0x%x \n", RootEntryTable));
- MmioWrite64 (VtdUnitBaseAddress + R_RTADDR_REG, (UINT64) (UINTN) RootEntryTable);
+ DEBUG ((DEBUG_INFO, "RTADDR_REG : 0x%x \n", RtaddrRegValue));
+ MmioWrite64 (VtdUnitBaseAddress + R_RTADDR_REG, (UINT64) RtaddrRegValue);

Reg32 = MmioRead32 (VtdUnitBaseAddress + R_GSTS_REG);
MmioWrite32 (VtdUnitBaseAddress + R_GCMD_REG, Reg32 | B_GMCD_REG_SRTP);
@@ -662,18 +662,6 @@ EnableVTdTranslationProtectionAll (

DEBUG ((DEBUG_INFO, "EnableVTdTranslationProtectionAll - 0x%lx\n", EngineMask));

- Status = PeiServicesLocatePpi (
- &gEdkiiVTdNullRootEntryTableGuid,
- 0,
- NULL,
- (VOID **)&RootEntryTable
- );
- if (EFI_ERROR(Status)) {
- DEBUG ((DEBUG_ERROR, "Locate Null Root Entry Table Ppi Failed : %r\n", Status));
- ASSERT (FALSE);
- return;
- }
-
for (Index = 0; Index < VTdInfo->VTdEngineCount; Index++) {
if ((EngineMask & LShiftU64(1, Index)) == 0) {
continue;
@@ -686,7 +674,28 @@ EnableVTdTranslationProtectionAll (
VTdInfo->VtdUnitInfo[Index].ECapReg.Uint64 = MmioRead64 (VTdInfo->VtdUnitInfo[Index].VtdUnitBaseAddress + R_ECAP_REG);
DumpVtdECapRegs (&VTdInfo->VtdUnitInfo[Index].ECapReg);

- EnableDmarPreMem (VTdInfo->VtdUnitInfo[Index].VtdUnitBaseAddress, (UINTN) *RootEntryTable);
+ if (VTdInfo->VtdUnitInfo[Index].ECapReg.Bits.ADMS == 1) {
+ //
+ // Use Abort DMA Mode
+ //
+ Status = EnableDmarPreMem (VTdInfo->VtdUnitInfo[Index].VtdUnitBaseAddress, V_RTADDR_REG_TTM_ADM);
+ } else {
+ //
+ // Use Null Root Entry Table
+ //
+ Status = PeiServicesLocatePpi (
+ &gEdkiiVTdNullRootEntryTableGuid,
+ 0,
+ NULL,
+ (VOID **)&RootEntryTable
+ );
+ if (EFI_ERROR(Status)) {
+ DEBUG ((DEBUG_ERROR, "Locate Null Root Entry Table Ppi Failed : %r\n", Status));
+ ASSERT (FALSE);
+ return;
+ }
+ EnableDmarPreMem (VTdInfo->VtdUnitInfo[Index].VtdUnitBaseAddress, (UINTN) *RootEntryTable);
+ }
}

return;
--
2.16.2.windows.1


[PATCH 2/4] IntelSiliconPkg/VTd: Update VTd register structs

Sheng Wei
 

Update VTd register structs accroding to VTd spec ver 3.3

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

Cc: Ray Ni <ray.ni@...>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@...>
Cc: Jenny Huang <jenny.huang@...>
Cc: Robert Kowalewski <robert.kowalewski@...>
Signed-off-by: Sheng Wei <w.sheng@...>
---
.../Feature/VTd/IntelVTdDmarPei/IntelVTdDmar.c | 3 +-
.../Feature/VTd/IntelVTdDmarPei/TranslationTable.c | 23 +++++++++++----
.../Feature/VTd/IntelVTdDxe/TranslationTable.c | 22 ++++++++++++--
.../Feature/VTd/IntelVTdDxe/VtdReg.c | 15 ++++++++--
.../IntelSiliconPkg/Include/IndustryStandard/Vtd.h | 34 +++++++++++++++++-----
5 files changed, 76 insertions(+), 21 deletions(-)

diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/IntelVTdDmar.c b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/IntelVTdDmar.c
index c3a939c9..87ce9716 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/IntelVTdDmar.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/IntelVTdDmar.c
@@ -631,10 +631,8 @@ DumpVtdECapRegs (
DEBUG ((DEBUG_INFO, " SC - 0x%x\n", ECapReg->Bits.SC));
DEBUG ((DEBUG_INFO, " IRO - 0x%x\n", ECapReg->Bits.IRO));
DEBUG ((DEBUG_INFO, " MHMV - 0x%x\n", ECapReg->Bits.MHMV));
- DEBUG ((DEBUG_INFO, " ECS - 0x%x\n", ECapReg->Bits.ECS));
DEBUG ((DEBUG_INFO, " MTS - 0x%x\n", ECapReg->Bits.MTS));
DEBUG ((DEBUG_INFO, " NEST - 0x%x\n", ECapReg->Bits.NEST));
- DEBUG ((DEBUG_INFO, " DIS - 0x%x\n", ECapReg->Bits.DIS));
DEBUG ((DEBUG_INFO, " PASID - 0x%x\n", ECapReg->Bits.PASID));
DEBUG ((DEBUG_INFO, " PRS - 0x%x\n", ECapReg->Bits.PRS));
DEBUG ((DEBUG_INFO, " ERS - 0x%x\n", ECapReg->Bits.ERS));
@@ -642,6 +640,7 @@ DumpVtdECapRegs (
DEBUG ((DEBUG_INFO, " NWFS - 0x%x\n", ECapReg->Bits.NWFS));
DEBUG ((DEBUG_INFO, " EAFS - 0x%x\n", ECapReg->Bits.EAFS));
DEBUG ((DEBUG_INFO, " PSS - 0x%x\n", ECapReg->Bits.PSS));
+ DEBUG ((DEBUG_INFO, " ADMS - 0x%x\n", ECapReg->Bits.ADMS));
}


diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/TranslationTable.c b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/TranslationTable.c
index 6676b2a9..a309d566 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/TranslationTable.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/TranslationTable.c
@@ -884,13 +884,26 @@ SetupTranslationTable (
return Status;
}

- if (VtdUnitInfo->ECapReg.Bits.ECS) {
- DEBUG ((DEBUG_INFO, "CreateExtContextEntry - %d\n", Index));
- Status = CreateExtContextEntry (VtdUnitInfo);
+ if (VtdUnitInfo->ECapReg.Bits.SMTS) {
+ if (VtdUnitInfo->ECapReg.Bits.DEP_24) {
+ DEBUG ((DEBUG_ERROR,"ECapReg.bit24 is not zero\n"));
+ ASSERT(FALSE);
+ Status = EFI_UNSUPPORTED;
+ } else {
+ Status = CreateExtContextEntry (VtdUnitInfo);
+ }
} else {
- DEBUG ((DEBUG_INFO, "CreateContextEntry - %d\n", Index));
- Status = CreateContextEntry (VtdUnitInfo);
+ if (VtdUnitInfo->ECapReg.Bits.DEP_24) {
+ //
+ // To compatible with pervious VTd engine
+ // It was ECS(Extended Context Support) bit.
+ //
+ Status = CreateExtContextEntry (VtdUnitInfo);
+ } else {
+ Status = CreateContextEntry (VtdUnitInfo);
+ }
}
+
if (EFI_ERROR (Status)) {
return Status;
}
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTable.c b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTable.c
index ca5f65a8..48e38d56 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTable.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTable.c
@@ -382,11 +382,27 @@ SetupTranslationTable (

for (Index = 0; Index < mVtdUnitNumber; Index++) {
DEBUG((DEBUG_INFO, "CreateContextEntry - %d\n", Index));
- if (mVtdUnitInformation[Index].ECapReg.Bits.ECS) {
- Status = CreateExtContextEntry (Index);
+
+ if (mVtdUnitInformation[Index].ECapReg.Bits.SMTS) {
+ if (mVtdUnitInformation[Index].ECapReg.Bits.DEP_24) {
+ DEBUG ((DEBUG_ERROR,"ECapReg.bit24 is not zero\n"));
+ ASSERT(FALSE);
+ Status = EFI_UNSUPPORTED;
+ } else {
+ Status = CreateExtContextEntry (Index);
+ }
} else {
- Status = CreateContextEntry (Index);
+ if (mVtdUnitInformation[Index].ECapReg.Bits.DEP_24) {
+ //
+ // To compatible with pervious VTd engine
+ // It was ECS(Extended Context Support) bit.
+ //
+ Status = CreateExtContextEntry (Index);
+ } else {
+ Status = CreateContextEntry (Index);
+ }
}
+
if (EFI_ERROR (Status)) {
return Status;
}
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c
index 1ce9c1c0..7a9af56f 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c
@@ -698,10 +698,8 @@ DumpVtdECapRegs (
DEBUG((DEBUG_INFO, " SC - 0x%x\n", ECapReg->Bits.SC));
DEBUG((DEBUG_INFO, " IRO - 0x%x\n", ECapReg->Bits.IRO));
DEBUG((DEBUG_INFO, " MHMV - 0x%x\n", ECapReg->Bits.MHMV));
- DEBUG((DEBUG_INFO, " ECS - 0x%x\n", ECapReg->Bits.ECS));
DEBUG((DEBUG_INFO, " MTS - 0x%x\n", ECapReg->Bits.MTS));
DEBUG((DEBUG_INFO, " NEST - 0x%x\n", ECapReg->Bits.NEST));
- DEBUG((DEBUG_INFO, " DIS - 0x%x\n", ECapReg->Bits.DIS));
DEBUG((DEBUG_INFO, " PASID - 0x%x\n", ECapReg->Bits.PASID));
DEBUG((DEBUG_INFO, " PRS - 0x%x\n", ECapReg->Bits.PRS));
DEBUG((DEBUG_INFO, " ERS - 0x%x\n", ECapReg->Bits.ERS));
@@ -709,6 +707,8 @@ DumpVtdECapRegs (
DEBUG((DEBUG_INFO, " NWFS - 0x%x\n", ECapReg->Bits.NWFS));
DEBUG((DEBUG_INFO, " EAFS - 0x%x\n", ECapReg->Bits.EAFS));
DEBUG((DEBUG_INFO, " PSS - 0x%x\n", ECapReg->Bits.PSS));
+ DEBUG((DEBUG_INFO, " SMTS - 0x%x\n", ECapReg->Bits.SMTS));
+ DEBUG((DEBUG_INFO, " ADMS - 0x%x\n", ECapReg->Bits.ADMS));
}

/**
@@ -721,12 +721,20 @@ DumpVtdRegs (
IN UINTN VtdIndex
)
{
+ const CHAR8 *TypeName[4] = {
+ "Write",
+ "Page",
+ "Read",
+ "AtomicOp",
+ };
+
UINTN Index;
UINT64 Reg64;
VTD_FRCD_REG FrcdReg;
VTD_CAP_REG CapReg;
UINT32 Reg32;
VTD_SOURCE_ID SourceId;
+ UINT32 TypeId;

DEBUG((DEBUG_INFO, "#### DumpVtdRegs(%d) Begin ####\n", VtdIndex));

@@ -771,7 +779,8 @@ DumpVtdRegs (
DEBUG((DEBUG_INFO, " Fault Info - 0x%016lx\n", VTD_64BITS_ADDRESS(FrcdReg.Bits.FILo, FrcdReg.Bits.FIHi)));
SourceId.Uint16 = (UINT16)FrcdReg.Bits.SID;
DEBUG((DEBUG_INFO, " Source - B%02x D%02x F%02x\n", SourceId.Bits.Bus, SourceId.Bits.Device, SourceId.Bits.Function));
- DEBUG((DEBUG_INFO, " Type - %x (%a)\n", FrcdReg.Bits.T, FrcdReg.Bits.T ? "read" : "write"));
+ TypeId = (FrcdReg.Bits.T1 << 1) | FrcdReg.Bits.T2;
+ DEBUG((DEBUG_INFO, " Type - %x (%a)\n", TypeId, TypeName[TypeId]));
DEBUG((DEBUG_INFO, " Reason - %x (Refer to VTd Spec, Appendix A)\n", FrcdReg.Bits.FR));
}
}
diff --git a/Silicon/Intel/IntelSiliconPkg/Include/IndustryStandard/Vtd.h b/Silicon/Intel/IntelSiliconPkg/Include/IndustryStandard/Vtd.h
index a759ca10..32fbdd02 100644
--- a/Silicon/Intel/IntelSiliconPkg/Include/IndustryStandard/Vtd.h
+++ b/Silicon/Intel/IntelSiliconPkg/Include/IndustryStandard/Vtd.h
@@ -216,6 +216,7 @@ typedef union {
#define B_GSTS_REG_RTPS BIT30
#define B_GSTS_REG_TE BIT31
#define R_RTADDR_REG 0x20
+#define V_RTADDR_REG_TTM_ADM (BIT11|BIT10)
#define R_CCMD_REG 0x28
#define B_CCMD_REG_CIRG_MASK (BIT62|BIT61)
#define V_CCMD_REG_CIRG_GLOBAL BIT61
@@ -334,7 +335,10 @@ typedef union {
UINT8 FL1GP:1; // First Level 1-GByte Page Support
UINT8 Rsvd_57:2;
UINT8 PI:1; // Posted Interrupts Support
- UINT8 Rsvd_60:4;
+ UINT8 FL5LP:1; // First Level 5-level Paging Support
+ UINT8 Rsvd_61:1;
+ UINT8 ESIRTPS:1; // Enhanced Set Interrupt Remap Table Pointer Support
+ UINT8 ESRTPS:1; // Enhanced Set Root Table Pointer Support
} Bits;
UINT64 Uint64;
} VTD_CAP_REG;
@@ -346,7 +350,7 @@ typedef union {
UINT8 DT:1; // Device-TLB support
UINT8 IR:1; // Interrupt Remapping support
UINT8 EIM:1; // Extended Interrupt Mode
- UINT8 Rsvd_5:1;
+ UINT8 DEP_5:1;
UINT8 PT:1; // Pass Through
UINT8 SC:1; // Snoop Control

@@ -354,11 +358,11 @@ typedef union {
UINT16 Rsvd_18:2;
UINT16 MHMV:4; // Maximum Handle Mask Value

- UINT8 ECS:1; // Extended Context Support
+ UINT8 DEP_24:1;
UINT8 MTS:1; // Memory Type Support
UINT8 NEST:1; // Nested Translation Support
- UINT8 DIS:1; // Deferred Invalidate Support
- UINT8 PASID:1; // Process Address Space ID Support
+ UINT8 Rsvd_27:1;
+ UINT8 DEP_28:1;
UINT8 PRS:1; // Page Request Support
UINT8 ERS:1; // Execute Request Support
UINT8 SRS:1; // Supervisor Request Support
@@ -367,7 +371,20 @@ typedef union {
UINT32 NWFS:1; // No Write Flag Support
UINT32 EAFS:1; // Extended Accessed Flag Support
UINT32 PSS:5; // PASID Size Supported
- UINT32 Rsvd_40:24;
+ UINT32 PASID:1; // Process Address Space ID Support
+ UINT32 DIT:1; // Device-TLB Invalidation Throttle
+ UINT32 PDS:1; // Page-request Drain Support
+ UINT32 SMTS:1; // Scalable Mode Translation Support
+ UINT32 VCS:1; // Virtual Command Support
+ UINT32 SLADS:1; // Second-Level Accessed Dirty Support
+ UINT32 SLTS:1; // Second-level Translation Support
+ UINT32 FLTS:1; // First-level Translation Support
+ UINT32 SMPWCS:1; // Scalable-Mode Page-walk Coherency Support
+ UINT32 RPS:1; // RID-PASID Support
+ UINT32 Rsvd_50:2;
+ UINT32 ADMS:1; // Abort DMA Mode Support
+ UINT32 RPRIVS:1; // RID_PRIV Support
+ UINT32 Rsvd_54:10;
} Bits;
UINT64 Uint64;
} VTD_ECAP_REG;
@@ -379,7 +396,8 @@ typedef union {
UINT32 FIHi:32; // FaultInfo

UINT32 SID:16; // Source Identifier
- UINT32 Rsvd_80:13;
+ UINT32 Rsvd_80:12;
+ UINT32 T2:1; // Type bit2 (0: Write/Read, 1: Page/AtomicOp)
UINT32 PRIV:1; // Privilege Mode Requested
UINT32 EXE:1; // Execute Permission Requested
UINT32 PP:1; // PASID Present
@@ -387,7 +405,7 @@ typedef union {
UINT32 FR:8; // Fault Reason
UINT32 PV:20; // PASID Value
UINT32 AT:2; // Address Type
- UINT32 T:1; // Type (0: Write, 1: Read)
+ UINT32 T1:1; // Type bit1 (0: Write/Page, 1: Read/AtomicOp)
UINT32 F:1; // Fault
} Bits;
UINT64 Uint64[2];
--
2.16.2.windows.1


[PATCH 1/4] IntelSiliconPkg/VTd: Fix typos

Sheng Wei
 

It is DRHD(DMA Remapping Hardware Unit Definition).

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

Cc: Ray Ni <ray.ni@...>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@...>
Cc: Jenny Huang <jenny.huang@...>
Cc: Robert Kowalewski <robert.kowalewski@...>
Signed-off-by: Sheng Wei <w.sheng@...>
---
.../IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/DmarTable.c | 12 ++++++------
.../IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c | 12 ++++++------
.../IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/DmarTable.c | 6 +++---
3 files changed, 15 insertions(+), 15 deletions(-)

diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/DmarTable.c b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/DmarTable.c
index 2154690d..e9c99d0a 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/DmarTable.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDmarPei/DmarTable.c
@@ -539,14 +539,14 @@ RegisterPciDevice (
}

/**
- Process DMAR DHRD table.
+ Process DMAR DRHD table.

@param[in] VTdUnitInfo The VTd engine unit information.
@param[in] DmarDrhd The DRHD table.

**/
VOID
-ProcessDhrd (
+ProcessDrhd (
IN VTD_UNIT_INFO *VTdUnitInfo,
IN EFI_ACPI_DMAR_DRHD_HEADER *DmarDrhd
)
@@ -581,10 +581,10 @@ ProcessDhrd (

if ((DmarDrhd->Flags & EFI_ACPI_DMAR_DRHD_FLAGS_INCLUDE_PCI_ALL) != 0) {
VTdUnitInfo->PciDeviceInfo.IncludeAllFlag = TRUE;
- DEBUG ((DEBUG_INFO," ProcessDhrd: with INCLUDE ALL\n"));
+ DEBUG ((DEBUG_INFO," ProcessDrhd: with INCLUDE ALL\n"));
} else {
VTdUnitInfo->PciDeviceInfo.IncludeAllFlag = FALSE;
- DEBUG ((DEBUG_INFO," ProcessDhrd: without INCLUDE ALL\n"));
+ DEBUG ((DEBUG_INFO," ProcessDrhd: without INCLUDE ALL\n"));
}

VTdUnitInfo->PciDeviceInfo.PciDeviceDataNumber = 0;
@@ -600,7 +600,7 @@ ProcessDhrd (
return;
}

- DEBUG ((DEBUG_INFO," ProcessDhrd: "));
+ DEBUG ((DEBUG_INFO," ProcessDrhd: "));
switch (DmarDevScopeEntry->Type) {
case EFI_ACPI_DEVICE_SCOPE_ENTRY_TYPE_PCI_ENDPOINT:
DEBUG ((DEBUG_INFO,"PCI Endpoint"));
@@ -708,7 +708,7 @@ ParseDmarAcpiTableDrhd (
switch (DmarHeader->Type) {
case EFI_ACPI_DMAR_TYPE_DRHD:
ASSERT (VtdIndex < VtdUnitNumber);
- ProcessDhrd (&VTdInfo->VtdUnitInfo[VtdIndex], (EFI_ACPI_DMAR_DRHD_HEADER *) DmarHeader);
+ ProcessDrhd (&VTdInfo->VtdUnitInfo[VtdIndex], (EFI_ACPI_DMAR_DRHD_HEADER *) DmarHeader);
VtdIndex++;

break;
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
index 1ee290b7..75fbd53e 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmarAcpiTable.c
@@ -662,7 +662,7 @@ GetPciBusDeviceFunction (
}

/**
- Process DMAR DHRD table.
+ Process DMAR DRHD table.

@param[in] VtdIndex The index of VTd engine.
@param[in] DmarDrhd The DRHD table.
@@ -670,7 +670,7 @@ GetPciBusDeviceFunction (
@retval EFI_SUCCESS The DRHD table is processed.
**/
EFI_STATUS
-ProcessDhrd (
+ProcessDrhd (
IN UINTN VtdIndex,
IN EFI_ACPI_DMAR_DRHD_HEADER *DmarDrhd
)
@@ -690,7 +690,7 @@ ProcessDhrd (

if ((DmarDrhd->Flags & EFI_ACPI_DMAR_DRHD_FLAGS_INCLUDE_PCI_ALL) != 0) {
mVtdUnitInformation[VtdIndex].PciDeviceInfo.IncludeAllFlag = TRUE;
- DEBUG ((DEBUG_INFO," ProcessDhrd: with INCLUDE ALL\n"));
+ DEBUG ((DEBUG_INFO," ProcessDrhd: with INCLUDE ALL\n"));

Status = ScanAllPciBus((VOID *)VtdIndex, DmarDrhd->SegmentNumber, ScanBusCallbackRegisterPciDevice);
if (EFI_ERROR (Status)) {
@@ -698,7 +698,7 @@ ProcessDhrd (
}
} else {
mVtdUnitInformation[VtdIndex].PciDeviceInfo.IncludeAllFlag = FALSE;
- DEBUG ((DEBUG_INFO," ProcessDhrd: without INCLUDE ALL\n"));
+ DEBUG ((DEBUG_INFO," ProcessDrhd: without INCLUDE ALL\n"));
}

DmarDevScopeEntry = (EFI_ACPI_DMAR_DEVICE_SCOPE_STRUCTURE_HEADER *)((UINTN)(DmarDrhd + 1));
@@ -709,7 +709,7 @@ ProcessDhrd (
return Status;
}

- DEBUG ((DEBUG_INFO," ProcessDhrd: "));
+ DEBUG ((DEBUG_INFO," ProcessDrhd: "));
switch (DmarDevScopeEntry->Type) {
case EFI_ACPI_DEVICE_SCOPE_ENTRY_TYPE_PCI_ENDPOINT:
DEBUG ((DEBUG_INFO,"PCI Endpoint"));
@@ -877,7 +877,7 @@ ParseDmarAcpiTableDrhd (
switch (DmarHeader->Type) {
case EFI_ACPI_DMAR_TYPE_DRHD:
ASSERT (VtdIndex < mVtdUnitNumber);
- Status = ProcessDhrd (VtdIndex, (EFI_ACPI_DMAR_DRHD_HEADER *)DmarHeader);
+ Status = ProcessDrhd (VtdIndex, (EFI_ACPI_DMAR_DRHD_HEADER *)DmarHeader);
if (EFI_ERROR (Status)) {
return Status;
}
diff --git a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/DmarTable.c b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/DmarTable.c
index d920d136..1bb74f40 100644
--- a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/DmarTable.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdPmrPei/DmarTable.c
@@ -356,14 +356,14 @@ GetVtdEngineNumber (
}

/**
- Process DMAR DHRD table.
+ Process DMAR DRHD table.

@param[in] VTdInfo The VTd engine context information.
@param[in] VtdIndex The index of VTd engine.
@param[in] DmarDrhd The DRHD table.
**/
VOID
-ProcessDhrd (
+ProcessDrhd (
IN VTD_INFO *VTdInfo,
IN UINTN VtdIndex,
IN EFI_ACPI_DMAR_DRHD_HEADER *DmarDrhd
@@ -415,7 +415,7 @@ ParseDmarAcpiTableDrhd (
switch (DmarHeader->Type) {
case EFI_ACPI_DMAR_TYPE_DRHD:
ASSERT (VtdIndex < VtdUnitNumber);
- ProcessDhrd (VTdInfo, VtdIndex, (EFI_ACPI_DMAR_DRHD_HEADER *)DmarHeader);
+ ProcessDrhd (VTdInfo, VtdIndex, (EFI_ACPI_DMAR_DRHD_HEADER *)DmarHeader);
VtdIndex++;

break;
--
2.16.2.windows.1


[PATCH 0/4] There are 4 patches for VTd drivers

Sheng Wei
 

[PATCH 1/4] IntelSiliconPkg/VTd: Fix typos
[PATCH 2/4] IntelSiliconPkg/VTd: Update VTd register structs
[PATCH 3/4] IntelSiliconPkg/VTd: Support VTd Abort DMA Mode
[PATCH 4/4] IntelSiliconPkg/VTd: Only generate PEI DMA buffer once.

Cc: Ray Ni <ray.ni@...>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@...>
Cc: Jenny Huang <jenny.huang@...>
Cc: Robert Kowalewski <robert.kowalewski@...>
Signed-off-by: Sheng Wei <w.sheng@...>

Sheng Wei (4):
IntelSiliconPkg/VTd: Fix typos
IntelSiliconPkg/VTd: Update VTd register structs
IntelSiliconPkg/VTd: Support VTd Abort DMA Mode
IntelSiliconPkg/VTd: Only generate PEI DMA buffer once.

.../Feature/VTd/IntelVTdDmarPei/DmarTable.c | 405 +++++------------
.../Feature/VTd/IntelVTdDmarPei/IntelVTdDmar.c | 336 ++++++--------
.../Feature/VTd/IntelVTdDmarPei/IntelVTdDmarPei.c | 501 ++++++++++++---------
.../Feature/VTd/IntelVTdDmarPei/IntelVTdDmarPei.h | 109 +++--
.../Feature/VTd/IntelVTdDmarPei/TranslationTable.c | 198 ++++----
.../Feature/VTd/IntelVTdDxe/DmarAcpiTable.c | 12 +-
.../Feature/VTd/IntelVTdDxe/TranslationTable.c | 22 +-
.../Feature/VTd/IntelVTdDxe/VtdReg.c | 15 +-
.../Feature/VTd/IntelVTdPmrPei/DmarTable.c | 6 +-
.../IntelSiliconPkg/Include/IndustryStandard/Vtd.h | 34 +-
10 files changed, 782 insertions(+), 856 deletions(-)

--
2.16.2.windows.1


Re: [PATCH v13 00/32] Add AMD Secure Nested Paging (SEV-SNP) support

Yao, Jiewen
 

Thanks to remind me.

Yes, I am waiting for hard freeze extend finish last week.

I will merge soon.

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Brijesh
Singh via groups.io
Sent: Monday, December 6, 2021 11:12 PM
To: devel@edk2.groups.io
Cc: brijesh.singh@...; James Bottomley <jejb@...>; Xu, Min M
<min.m.xu@...>; Yao, Jiewen <jiewen.yao@...>; Tom Lendacky
<thomas.lendacky@...>; Justen, Jordan L <jordan.l.justen@...>;
Ard Biesheuvel <ardb+tianocore@...>; Erdem Aktas
<erdemaktas@...>; Michael Roth <Michael.Roth@...>; Gerd
Hoffmann <kraxel@...>; Kinney, Michael D
<michael.d.kinney@...>; Liming Gao <gaoliming@...>; Liu,
Zhiguang <zhiguang.liu@...>; Ni, Ray <ray.ni@...>; Kumar, Rahul1
<rahul1.kumar@...>; Dong, Eric <eric.dong@...>
Subject: Re: [edk2-devel] [PATCH v13 00/32] Add AMD Secure Nested Paging
(SEV-SNP) support

Hi Gerd and Jiewen,

Now that all the patches are ack'ed by Ray, can we plan to merge this
series ?

-Brijesh


On 11/12/21 11:39 AM, Brijesh Singh via groups.io wrote:
---------------------------------------------------------------------------
Hi Ray,

Thanks for your reviews and continuous support; I have updated a couple of
patches
to address your comment. As I said in my previous reply, I will working on a
follow-up series to group some of those Sev specific variables in CpuData.

I hope that is okay with you.

thanks
----------------------------------------------------------------------------

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

SEV-SNP builds upon existing SEV and SEV-ES functionality while adding
new hardware-based memory protections. SEV-SNP adds strong memory
integrity
protection to help prevent malicious hypervisor-based attacks like data
replay, memory re-mapping and more in order to create an isolated memory
encryption environment.

This series provides the basic building blocks to support booting the SEV-SNP
VMs, it does not cover all the security enhancement introduced by the SEV-
SNP
such as interrupt protection.

Many of the integrity guarantees of SEV-SNP are enforced through a new
structure called the Reverse Map Table (RMP). Adding a new page to SEV-SNP
VM requires a 2-step process. First, the hypervisor assigns a page to the
guest using the new RMPUPDATE instruction. This transitions the page to
guest-invalid. Second, the guest validates the page using the new PVALIDATE
instruction. The SEV-SNP VMs can use the new "Page State Change Request
NAE"
defined in the GHCB specification to ask hypervisor to add or remove page
from the RMP table.

Each page assigned to the SEV-SNP VM can either be validated or unvalidated,
as indicated by the Validated flag in the page's RMP entry. There are two
approaches that can be taken for the page validation: Pre-validation and
Lazy Validation.

Under pre-validation, the pages are validated prior to first use. And under
lazy validation, pages are validated when first accessed. An access to a
unvalidated page results in a #VC exception, at which time the exception
handler may validate the page. Lazy validation requires careful tracking of
the validated pages to avoid validating the same GPA more than once. The
recently introduced "Unaccepted" memory type can be used to communicate
the
unvalidated memory ranges to the Guest OS.

At this time we only support the pre-validation. OVMF detects all the available
system RAM in the PEI phase. When SEV-SNP is enabled, the memory is
validated
before it is made available to the EDK2 core.

Now that series contains all the basic support required to launch SEV-SNP
guest. We are still missing the Interrupt security feature provided by the
SNP. The feature will be added after the base support is accepted.

Additional resources
---------------------
SEV-SNP whitepaper
https://www.amd.com/system/files/TechDocs/SEV-SNP-strengthening-vm-
isolation-with-integrity-protection-and-more.pdf

APM 2: https://www.amd.com/system/files/TechDocs/24593.pdf (section
15.36)

The complete source is available at
https://github.com/AMDESE/ovmf/tree/snp-v13

GHCB spec:
https://developer.amd.com/wp-content/resources/56421.pdf

SEV-SNP firmware specification:
https://www.amd.com/system/files/TechDocs/56860.pdf

Change since v12:
* MpLib: Add comment to clarify that SEV-SNP enabled implicitly means SEV
and SEV-ES are active.
* MpLib: Move the extended topology initialization in AmdSev.c

Change since v11:
* rebase to the latest
* fix the UefiCpuPkg PCD definition patch header.

Change since v10:
* fix 'unresolved external symbol __allshl' link error when building I32 for
VS2017.

Changes since v9:
* Move CCAttrs Pcd define in MdePkg
* Add comment to indicate that allocating the identity map PT is temporary
until we get lazy validation

Changes since v8:
* drop the generic metadata and make it specific to SEV.

Changes since v7:
* Move SEV specific changes in MpLib in AmdSev file
* Update the GHCB register function to not restore the GHCB MSR because
we were already in the MSR protocol mode.
* Drop the SNP name from PcdSnpSecPreValidate.
* Add new section for GHCB memory in the OVMF metadata.

Change since v6:
* Drop the SNP boot block GUID and switch to using the Metadata guided
structure
proposed by Min in TDX series.
* Exclude the GHCB page from the pre-validated region. It simplifies the reset
vector code where we do not need to unvalidate the GHCB page.
* Now that GHCB page is not validated so move the VMPL check from reset
vector
code to the MemEncryptSevLib on the first page validation.
* Introduce the ConfidentialComputingGuestAttr PCD to communicate which
memory encryption is active so that MpInitLib can make use of it.
* Drop the SEVES specific PCD as the information can be communicated via
the ConfidentialComputingGuestAttr.
* Move the SNP specific AP creation function in AmdSev.c.
* Define the SNP Blob GUID in a new file.

Change since v5:
* When possible use the CPUID value from CPUID page
* Move the SEV specific functions from SecMain.c in AmdSev.c
* Rebase to the latest code
* Add the review feedback from Yao.

Change since v4:
* Use the correct MSR for the SEV_STATUS
* Add VMPL-0 check

Change since v3:
* ResetVector: move all SEV specific code in AmdSev.asm and add macros to
keep
the code readable.
* Drop extending the EsWorkArea to contain SNP specific state.
* Drop the GhcbGpa library and call the VmgExit directly to register GHCB GPA.
* Install the CC blob config table from AmdSevDxe instead of extending the
AmdSev/SecretsDxe for it.
* Add the separate PCDs for the SNP Secrets.

Changes since v2:
* Add support for the AP creation.
* Use the module-scoping override to make AmdSevDxe use the IO port for
PCI reads.
* Use the reserved memory type for CPUID and Secrets page.
*
Changes since v1:
* Drop the interval tree support to detect the pre-validated overlap region.
* Use an array to keep track of pre-validated regions.
* Add support to query the Hypervisor feature and verify that SNP feature is
supported.
* Introduce MemEncryptSevClearMmioPageEncMask() to clear the C-bit from
MMIO ranges.
* Pull the SevSecretDxe and SevSecretPei into OVMF package build.
* Extend the SevSecretDxe to expose confidential computing blob location
through
EFI configuration table.

Brijesh Singh (28):
OvmfPkg/SecMain: move SEV specific routines in AmdSev.c
UefiCpuPkg/MpInitLib: move SEV specific routines in AmdSev.c
OvmfPkg/ResetVector: move clearing GHCB in SecMain
OvmfPkg/ResetVector: introduce SEV metadata descriptor for VMM use
OvmfPkg: reserve SNP secrets page
OvmfPkg: reserve CPUID page
OvmfPkg/ResetVector: pre-validate the data pages used in SEC phase
OvmfPkg/MemEncryptSevLib: add MemEncryptSevSnpEnabled()
OvmfPkg/SecMain: register GHCB gpa for the SEV-SNP guest
OvmfPkg/PlatformPei: register GHCB gpa for the SEV-SNP guest
OvmfPkg/AmdSevDxe: do not use extended PCI config space
OvmfPkg/MemEncryptSevLib: add support to validate system RAM
OvmfPkg/MemEncryptSevLib: add function to check the VMPL0
OvmfPkg/BaseMemEncryptSevLib: skip the pre-validated system RAM
OvmfPkg/MemEncryptSevLib: add support to validate > 4GB memory in PEI
phase
OvmfPkg/SecMain: validate the memory used for decompressing Fv
OvmfPkg/PlatformPei: validate the system RAM when SNP is active
MdePkg: Define ConfidentialComputingGuestAttr
OvmfPkg/PlatformPei: set PcdConfidentialComputingAttr when SEV is
active
UefiCpuPkg/MpInitLib: use PcdConfidentialComputingAttr to check SEV
status
UefiCpuPkg: add PcdGhcbHypervisorFeatures
OvmfPkg/PlatformPei: set the Hypervisor Features PCD
MdePkg/GHCB: increase the GHCB protocol max version
UefiCpuPkg/MpLib: add support to register GHCB GPA when SEV-SNP is
enabled
OvmfPkg/MemEncryptSevLib: change the page state in the RMP table
OvmfPkg/MemEncryptSevLib: skip page state change for Mmio address
OvmfPkg/PlatformPei: mark cpuid and secrets memory reserved in EFI map
OvmfPkg/AmdSev: expose the SNP reserved pages through configuration
table

Michael Roth (3):
OvmfPkg/ResetVector: use SEV-SNP-validated CPUID values
OvmfPkg/VmgExitLib: use SEV-SNP-validated CPUID values
UefiCpuPkg/MpInitLib: use BSP to do extended topology check

Tom Lendacky (1):
UefiCpuPkg/MpInitLib: Use SEV-SNP AP Creation NAE event to launch APs

MdePkg/MdePkg.dec | 4 +
OvmfPkg/OvmfPkg.dec | 19 +
UefiCpuPkg/UefiCpuPkg.dec | 5 +
OvmfPkg/AmdSev/AmdSevX64.dsc | 8 +-
OvmfPkg/Bhyve/BhyveX64.dsc | 5 +-
OvmfPkg/OvmfPkgIa32.dsc | 4 +
OvmfPkg/OvmfPkgIa32X64.dsc | 9 +-
OvmfPkg/OvmfPkgX64.dsc | 8 +-
OvmfPkg/OvmfXen.dsc | 5 +-
OvmfPkg/OvmfPkgX64.fdf | 6 +
OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 7 +
.../DxeMemEncryptSevLib.inf | 3 +
.../PeiMemEncryptSevLib.inf | 7 +
.../SecMemEncryptSevLib.inf | 3 +
OvmfPkg/Library/VmgExitLib/SecVmgExitLib.inf | 2 +
OvmfPkg/Library/VmgExitLib/VmgExitLib.inf | 3 +
OvmfPkg/PlatformPei/PlatformPei.inf | 7 +
OvmfPkg/ResetVector/ResetVector.inf | 5 +
OvmfPkg/Sec/SecMain.inf | 4 +
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 6 +-
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 6 +-
.../Include/ConfidentialComputingGuestAttr.h | 25 +
MdePkg/Include/Register/Amd/Ghcb.h | 2 +-
.../Guid/ConfidentialComputingSevSnpBlob.h | 33 ++
OvmfPkg/Include/Library/MemEncryptSevLib.h | 26 +
.../X64/SnpPageStateChange.h | 36 ++
.../BaseMemEncryptSevLib/X64/VirtualMemory.h | 24 +
OvmfPkg/PlatformPei/Platform.h | 5 +
OvmfPkg/Sec/AmdSev.h | 95 ++++
UefiCpuPkg/Library/MpInitLib/MpLib.h | 103 ++++
OvmfPkg/AmdSevDxe/AmdSevDxe.c | 23 +
.../DxeMemEncryptSevLibInternal.c | 27 ++
.../Ia32/MemEncryptSevLib.c | 17 +
.../PeiMemEncryptSevLibInternal.c | 27 ++
.../SecMemEncryptSevLibInternal.c | 19 +
.../X64/DxeSnpSystemRamValidate.c | 40 ++
.../X64/PeiDxeVirtualMemory.c | 167 ++++++-
.../X64/PeiSnpSystemRamValidate.c | 127 +++++
.../X64/SecSnpSystemRamValidate.c | 82 ++++
.../X64/SnpPageStateChangeInternal.c | 294 ++++++++++++
OvmfPkg/Library/VmgExitLib/VmgExitVcHandler.c | 444 ++++++++++++++++-
-
OvmfPkg/PlatformPei/AmdSev.c | 231 +++++++++
OvmfPkg/PlatformPei/MemDetect.c | 2 +
OvmfPkg/Sec/AmdSev.c | 298 ++++++++++++
OvmfPkg/Sec/SecMain.c | 158 +------
UefiCpuPkg/Library/MpInitLib/AmdSev.c | 260 ++++++++++
UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 16 +-
UefiCpuPkg/Library/MpInitLib/Ia32/AmdSev.c | 70 +++
UefiCpuPkg/Library/MpInitLib/MpLib.c | 347 +++++---------
UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 4 +-
UefiCpuPkg/Library/MpInitLib/X64/AmdSev.c | 261 ++++++++++
OvmfPkg/FvmainCompactScratchEnd.fdf.inc | 5 +
OvmfPkg/ResetVector/Ia16/ResetVectorVtf0.asm | 14 +
OvmfPkg/ResetVector/Ia32/AmdSev.asm | 86 +++-
OvmfPkg/ResetVector/ResetVector.nasmb | 18 +
OvmfPkg/ResetVector/X64/OvmfSevMetadata.asm | 74 +++
UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 2 +
UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm | 200 ++++++++
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 100 +---
59 files changed, 3360 insertions(+), 528 deletions(-)
create mode 100644 MdePkg/Include/ConfidentialComputingGuestAttr.h
create mode 100644
OvmfPkg/Include/Guid/ConfidentialComputingSevSnpBlob.h
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChange.h
create mode 100644 OvmfPkg/Sec/AmdSev.h
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/DxeSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/PeiSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SecSnpSystemRamValidate.c
create mode 100644
OvmfPkg/Library/BaseMemEncryptSevLib/X64/SnpPageStateChangeInternal.c
create mode 100644 OvmfPkg/Sec/AmdSev.c
create mode 100644 UefiCpuPkg/Library/MpInitLib/AmdSev.c
create mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/AmdSev.c
create mode 100644 UefiCpuPkg/Library/MpInitLib/X64/AmdSev.c
create mode 100644 OvmfPkg/ResetVector/X64/OvmfSevMetadata.asm
create mode 100644 UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm



回复: [edk2-devel] [PATCH v1 0/2] MM communicate functionality in variable policy

gaoliming
 

Kun:
Does this change impact current behavior? Seemly, there is no strict check on MessageLength.

Thanks
Liming

-----邮件原件-----
发件人: Kun Qin <kuqin12@...>
发送时间: 2021年12月7日 2:47
收件人: Ard Biesheuvel <ardb@...>
抄送: edk2-devel-groups-io <devel@edk2.groups.io>; Jian J Wang
<jian.j.wang@...>; Liming Gao <gaoliming@...>; Hao A
Wu <hao.a.wu@...>; Leif Lindholm <leif@...>; Ard
Biesheuvel <ardb+tianocore@...>; Bret Barkelew
<Bret.Barkelew@...>; Michael Kubacki
<michael.kubacki@...>
主题: Re: [edk2-devel] [PATCH v1 0/2] MM communicate functionality in
variable policy

Thanks for the information, Ard. I just meant to plan ahead so that I
can work on the feedback for these patches, if any.

I can ping back the thread again once the stable tag is created.

Regards,
Kun

On 12/06/2021 10:41, Ard Biesheuvel wrote:
On Mon, 6 Dec 2021 at 19:35, Kun Qin <kuqin12@...> wrote:

Hi ArmPkg and MdeModulePkg maintainers,

It has been a week since the patches were sent. Could you please review
the changes and let me know if there is any feedback? Any input is
appreciated.
As far as I know, we are still in hard freeze for the upcoming stable tag.


On 11/29/2021 16:39, Kun Qin via groups.io wrote:
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3709
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3751

Currently, setups with variable policy operations used together with MM
communicate from ArmPkg could fail with `EFI_INVALID_PARAMETER`.
This was
due to the errors from 2 following aspects:

1. For variable policy implementations in MdeModulePkg, the DXE
runtime
agent would communicate to MM to disable, register or query policies.
However, during these operations, the MessageLength calculation is
including MM communicate header. This could lead to MM agent read
data
across the given buffer boundary and/or trigger other errors.

2. On the other hand, current MM communicate routine from ArmPkg
would
fail the function if the input message length does not equal to input
buffer size.

As defined in PI specification, the `CommSize`, when as input, should
stand for "The size of the data buffer being passed in", which would mean
the maximal number of bytes `CommBuffer` can hold. In turn, the value of
this input parameter can be used for MM handlers to determine whether
the
output data is too large to fit in this buffer. Enforcing the incoming
buffer to hold exactly the number of used bytes mismatches with the PI
spec description.

This change fix MessageLength field calculation from variable policy and
updated input argument inspections from MM communicate routine in
ArmPkg
to match PI spec descriptions.

Patch v1 branch:
https://github.com/kuqin12/edk2/tree/mm_communicate_check

Cc: Jian J Wang <jian.j.wang@...>
Cc: Liming Gao <gaoliming@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Leif Lindholm <leif@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Michael Kubacki <michael.kubacki@...>

Kun Qin (2):
MdeModulePkg: VariableSmmRuntimeDxe: Fix Variable Policy
Message
Length
ArmPkg: MmCommunicationDxe: Update MM communicate input
arguments
checks

ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
| 44 ++++++++++++--------
MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c |
10 ++---
2 files changed, 32 insertions(+), 22 deletions(-)


Re: [edk2-platforms][PATCH V1 1/1] WhitleyOpenBoardPkg/BoardAcpiLib: Simplify implementation

Oram, Isaac W
 

Pushed as 41dacdf4fb36b350bd199adeb9036b7d44b3d243

-----Original Message-----
From: Desimone, Nathaniel L <nathaniel.l.desimone@...>
Sent: Wednesday, December 1, 2021 5:09 PM
To: Oram, Isaac W <isaac.w.oram@...>; devel@edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu@...>
Subject: RE: [edk2-devel][edk2-platforms][PATCH V1 1/1] WhitleyOpenBoardPkg/BoardAcpiLib: Simplify implementation

Reviewed-by: Nate DeSimone <nathaniel.l.desimone@...>

-----Original Message-----
From: Oram, Isaac W <isaac.w.oram@...>
Sent: Wednesday, November 10, 2021 6:05 PM
To: devel@edk2.groups.io
Cc: Oram, Isaac W <isaac.w.oram@...>; Desimone, Nathaniel L <nathaniel.l.desimone@...>; Chiu, Chasel <chasel.chiu@...>
Subject: [edk2-devel][edk2-platforms][PATCH V1 1/1] WhitleyOpenBoardPkg/BoardAcpiLib: Simplify implementation

Remove DXE version as the library isn't in use.
Simplify the SMM library. Remove functions calling functions.

Cc: Nate DeSimone <nathaniel.l.desimone@...>
Cc: Chasel Chiu <chasel.chiu@...>
Signed-off-by: Isaac Oram <isaac.w.oram@...>
---
Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/DxeBoardAcpiTableLib.c | 37 ------
Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/DxeBoardAcpiTableLib.inf | 44 -------
Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/DxeMtOlympusAcpiTableLib.c | 54 --------
Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.c | 89 +++++++++++--
Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.inf | 1 -
Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmSiliconAcpiEnableLib.c | 138 --------------------
Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc | 7 +-
7 files changed, 77 insertions(+), 293 deletions(-)

diff --git a/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/DxeBoardAcpiTableLib.c b/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/DxeBoardAcpiTableLib.c
deleted file mode 100644
index dfa0c994dc..0000000000
--- a/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/DxeBoardAcpiTableLib.c
+++ /dev/null
@@ -1,37 +0,0 @@
-/** @file
- Platform Hook Library instances
-
- @copyright
- Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
-
- SPDX-License-Identifier: BSD-2-Clause-Patent -**/
-
-#include <Base.h>
-#include <Uefi.h>
-#include <PiDxe.h>
-#include <Library/BaseLib.h>
-#include <Library/IoLib.h>
-#include <Library/BoardAcpiTableLib.h>
-#include <Library/PcdLib.h>
-#include <Library/DebugLib.h>
-
-EFI_STATUS
-EFIAPI
-MtOlympusBoardUpdateAcpiTable (
- IN OUT EFI_ACPI_COMMON_HEADER *Table,
- IN OUT EFI_ACPI_TABLE_VERSION *Version
- );
-
-EFI_STATUS
-EFIAPI
-BoardUpdateAcpiTable (
- IN OUT EFI_ACPI_COMMON_HEADER *Table,
- IN OUT EFI_ACPI_TABLE_VERSION *Version
- )
-{
- MtOlympusBoardUpdateAcpiTable (Table, Version);
-
- return EFI_SUCCESS;
-}
-
diff --git a/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/DxeBoardAcpiTableLib.inf b/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/DxeBoardAcpiTableLib.inf
deleted file mode 100644
index 3186c6c91e..0000000000
--- a/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/DxeBoardAcpiTableLib.inf
+++ /dev/null
@@ -1,44 +0,0 @@
-### @file
-# Platform Hook Library instance for SandyBridge Mobile/Desktop CRB.
-#
-# @copyright
-# Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR> -# -# SPDX-License-Identifier: BSD-2-Clause-Patent -# -##
-
-[Defines]
- INF_VERSION = 0x00010017
- BASE_NAME = DxeBoardAcpiTableLib
- FILE_GUID = 6562E0AE-90D8-4D41-8C97-81286B4BE7D2
- VERSION_STRING = 1.0
- MODULE_TYPE = BASE
- LIBRARY_CLASS = BoardAcpiTableLib
-
-#
-# The following information is for reference only and not required by the build tools.
-#
-# VALID_ARCHITECTURES = IA32 X64 IPF EBC -#
-
-[LibraryClasses]
- BaseLib
- IoLib
- PciLib
- PcdLib
-[Packages]
- MdePkg/MdePkg.dec
- MdeModulePkg/MdeModulePkg.dec
- MinPlatformPkg/MinPlatformPkg.dec
- WhitleyOpenBoardPkg/PlatformPkg.dec
- WhitleySiliconPkg/WhitleySiliconPkg.dec
- WhitleySiliconPkg/SiliconPkg.dec
-
-[Pcd]
- gOemSkuTokenSpaceGuid.PcdAcpiGnvsAddress
-
-[Sources]
- DxeMtOlympusAcpiTableLib.c
- DxeBoardAcpiTableLib.c
-
diff --git a/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/DxeMtOlympusAcpiTableLib.c b/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/DxeMtOlympusAcpiTableLib.c
deleted file mode 100644
index 09b917083c..0000000000
--- a/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/DxeMtOlympusAcpiTableLib.c
+++ /dev/null
@@ -1,54 +0,0 @@
-/** @file
- Platform Hook Library instances
-
- @copyright
- Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
-
- SPDX-License-Identifier: BSD-2-Clause-Patent -**/
-
-#include <Base.h>
-#include <Uefi.h>
-#include <PiDxe.h>
-#include <Library/BaseLib.h>
-#include <Library/IoLib.h>
-#include <Library/BoardAcpiTableLib.h>
-#include <Library/PcdLib.h>
-#include <Library/DebugLib.h>
-#include <Library/UefiBootServicesTableLib.h>
-#include <Protocol/GlobalNvsArea.h>
-
-GLOBAL_REMOVE_IF_UNREFERENCED BIOS_ACPI_PARAM *mGlobalNvsArea;
-
-VOID
-MtOlympusUpdateGlobalNvs (
- VOID
- )
-{
-
- //
- // Allocate and initialize the NVS area for SMM and ASL communication.
- //
- mGlobalNvsArea = (VOID *)(UINTN)PcdGet64 (PcdAcpiGnvsAddress);
-
- //
- // Update global NVS area for ASL and SMM init code to use
- //
-
-
-}
-
-EFI_STATUS
-EFIAPI
-MtOlympusBoardUpdateAcpiTable (
- IN OUT EFI_ACPI_COMMON_HEADER *Table,
- IN OUT EFI_ACPI_TABLE_VERSION *Version
- )
-{
- if (Table->Signature == EFI_ACPI_2_0_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_SIGNATURE) {
- MtOlympusUpdateGlobalNvs ();
- }
-
- return EFI_SUCCESS;
-}
-
diff --git a/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.c b/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.c
index 09a6b00877..0f8a62460a 100644
--- a/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.c
+++ b/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmBoardAc
+++ piEnableLib.c
@@ -15,18 +15,9 @@
#include <Library/BoardAcpiEnableLib.h> #include <Library/PcdLib.h> #include <Library/DebugLib.h>
-
-EFI_STATUS
-EFIAPI
-SiliconEnableAcpi (
- IN BOOLEAN EnableSci
- );
-
-EFI_STATUS
-EFIAPI
-SiliconDisableAcpi (
- IN BOOLEAN DisableSci
- );
+#include <PchAccess.h>
+#include <Protocol/DynamicSiLibrarySmmProtocol.h>
+#include <Library/SmmServicesTableLib.h>

EFI_STATUS
EFIAPI
@@ -34,7 +25,52 @@ BoardEnableAcpi (
IN BOOLEAN EnableSci
)
{
- SiliconEnableAcpi (EnableSci);
+ UINT32 SmiEn;
+ UINT16 Pm1En;
+ UINT16 Pm1Cnt;
+ UINT16 PchPmBase;
+ EFI_STATUS Status;
+ DYNAMIC_SI_LIBARY_SMM_PROTOCOL *DynamicSiLibrarySmmProtocol = NULL;
+
+ Status = gSmst->SmmLocateProtocol (&gDynamicSiLibrarySmmProtocolGuid,
+ NULL, &DynamicSiLibrarySmmProtocol); if (EFI_ERROR (Status)) {
+ ASSERT_EFI_ERROR (Status);
+ return Status;
+ }
+
+ //
+ // Init Power Management I/O Base aka ACPI Base // PchPmBase =
+ DynamicSiLibrarySmmProtocol->PmcGetAcpiBase ();
+
+ SmiEn = IoRead32 (PchPmBase + R_ACPI_IO_SMI_EN);
+
+ //
+ // Disable SW SMI Timer and legacy USB // SmiEn &=
+ ~(B_ACPI_IO_SMI_EN_SWSMI_TMR | B_ACPI_IO_SMI_EN_LEGACY_USB |
+ B_ACPI_IO_SMI_EN_LEGACY_USB2);
+
+ //
+ // And enable SMI on write to B_PCH_ACPI_PM1_CNT_SLP_EN when SLP_TYP
+ is written // SmiEn |= B_ACPI_IO_SMI_EN_ON_SLP_EN ;
+ IoWrite32 (PchPmBase + R_ACPI_IO_SMI_EN, SmiEn);
+
+ //
+ // Disable PM sources except power button //
+ Pm1En = B_ACPI_IO_PM1_EN_PWRBTN;
+ IoWrite16 (PchPmBase + R_ACPI_IO_PM1_EN, Pm1En);
+
+ //
+ // Enable SCI
+ //
+ if (EnableSci) {
+ Pm1Cnt = IoRead16 (PchPmBase + R_ACPI_IO_PM1_CNT);
+ Pm1Cnt |= B_ACPI_IO_PM1_CNT_SCI_EN;
+ IoWrite16 (PchPmBase + R_ACPI_IO_PM1_CNT, Pm1Cnt); }
+
return EFI_SUCCESS;
}

@@ -44,7 +80,32 @@ BoardDisableAcpi (
IN BOOLEAN DisableSci
)
{
- SiliconDisableAcpi (DisableSci);
+ UINT16 Pm1Cnt;
+ UINT16 PchPmBase;
+ EFI_STATUS Status;
+ DYNAMIC_SI_LIBARY_SMM_PROTOCOL *DynamicSiLibrarySmmProtocol = NULL;
+
+ Status = gSmst->SmmLocateProtocol (&gDynamicSiLibrarySmmProtocolGuid,
+ NULL, &DynamicSiLibrarySmmProtocol); if (EFI_ERROR (Status)) {
+ ASSERT_EFI_ERROR (Status);
+ return Status;
+ }
+
+ //
+ // Init Power Management I/O Base aka ACPI Base // PchPmBase =
+ DynamicSiLibrarySmmProtocol->PmcGetAcpiBase ();
+
+
+ //
+ // Disable SCI
+ //
+ if (DisableSci) {
+ Pm1Cnt = IoRead16 (PchPmBase + R_ACPI_IO_PM1_CNT);
+ Pm1Cnt &= ~B_ACPI_IO_PM1_CNT_SCI_EN;
+ IoWrite16 (PchPmBase + R_ACPI_IO_PM1_CNT, Pm1Cnt); }
+
return EFI_SUCCESS;
}

diff --git a/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.inf b/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.inf
index 19d29ed40f..fcb3e23365 100644
--- a/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.inf
+++ b/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmBoardAc
+++ piEnableLib.inf
@@ -38,7 +38,6 @@
WhitleySiliconPkg/CpRcPkg.dec

[Sources]
- SmmSiliconAcpiEnableLib.c
SmmBoardAcpiEnableLib.c

[Protocols]
diff --git a/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmSiliconAcpiEnableLib.c b/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmSiliconAcpiEnableLib.c
deleted file mode 100644
index 484311811b..0000000000
--- a/Platform/Intel/WhitleyOpenBoardPkg/Library/BoardAcpiLib/SmmSiliconAcpiEnableLib.c
+++ /dev/null
@@ -1,138 +0,0 @@
-/** @file
- Platform Hook Library instances
-
- @copyright
- Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
-
- SPDX-License-Identifier: BSD-2-Clause-Patent -**/
-
-#include <Base.h>
-#include <Uefi.h>
-#include <PiDxe.h>
-#include <Library/BaseLib.h>
-#include <Library/IoLib.h>
-#include <Library/BoardAcpiEnableLib.h> -#include <Library/PcdLib.h> -#include <Library/DebugLib.h> -#include <PchAccess.h> -#include <Protocol/DynamicSiLibrarySmmProtocol.h>
-#include <Library/SmmServicesTableLib.h>
-
-/**
- Clear Port 80h
-
- SMI handler to enable ACPI mode
-
- Dispatched on reads from APM port with value EFI_ACPI_ENABLE_SW_SMI
-
- Disables the SW SMI Timer.
- ACPI events are disabled and ACPI event status is cleared.
- SCI mode is then enabled.
-
- Clear SLP SMI status
- Enable SLP SMI
-
- Disable SW SMI Timer
-
- Clear all ACPI event status and disable all ACPI events
-
- Disable PM sources except power button
- Clear status bits
-
- Disable GPE0 sources
- Clear status bits
-
- Disable GPE1 sources
- Clear status bits
-
- Guarantee day-of-month alarm is invalid (ACPI 1.0 section 4.7.2.4)
-
- Enable SCI
-**/
-EFI_STATUS
-EFIAPI
-SiliconEnableAcpi (
- IN BOOLEAN EnableSci
- )
-{
- UINT32 SmiEn;
- UINT16 Pm1En;
- UINT16 Pm1Cnt;
- UINT16 PchPmBase;
- EFI_STATUS Status;
- DYNAMIC_SI_LIBARY_SMM_PROTOCOL *DynamicSiLibrarySmmProtocol = NULL;
-
- Status = gSmst->SmmLocateProtocol (&gDynamicSiLibrarySmmProtocolGuid, NULL, &DynamicSiLibrarySmmProtocol);
- if (EFI_ERROR (Status)) {
- ASSERT_EFI_ERROR (Status);
- return Status;
- }
-
- //
- // Init Power Management I/O Base aka ACPI Base
- //
- PchPmBase = DynamicSiLibrarySmmProtocol->PmcGetAcpiBase ();
-
- SmiEn = IoRead32 (PchPmBase + R_ACPI_IO_SMI_EN);
-
- //
- // Disable SW SMI Timer and legacy USB
- //
- SmiEn &= ~(B_ACPI_IO_SMI_EN_SWSMI_TMR | B_ACPI_IO_SMI_EN_LEGACY_USB | B_ACPI_IO_SMI_EN_LEGACY_USB2);
-
- //
- // And enable SMI on write to B_PCH_ACPI_PM1_CNT_SLP_EN when SLP_TYP is written
- //
- SmiEn |= B_ACPI_IO_SMI_EN_ON_SLP_EN ;
- IoWrite32 (PchPmBase + R_ACPI_IO_SMI_EN, SmiEn);
-
- //
- // Disable PM sources except power button
- //
- Pm1En = B_ACPI_IO_PM1_EN_PWRBTN;
- IoWrite16 (PchPmBase + R_ACPI_IO_PM1_EN, Pm1En);
-
- //
- // Enable SCI
- //
- Pm1Cnt = IoRead16 (PchPmBase + R_ACPI_IO_PM1_CNT);
- Pm1Cnt |= B_ACPI_IO_PM1_CNT_SCI_EN;
- IoWrite16 (PchPmBase + R_ACPI_IO_PM1_CNT, Pm1Cnt);
-
- return EFI_SUCCESS;
-}
-
-EFI_STATUS
-EFIAPI
-SiliconDisableAcpi (
- IN BOOLEAN DisableSci
- )
-{
- UINT16 Pm1Cnt;
- UINT16 PchPmBase;
- EFI_STATUS Status;
- DYNAMIC_SI_LIBARY_SMM_PROTOCOL *DynamicSiLibrarySmmProtocol = NULL;
-
- Status = gSmst->SmmLocateProtocol (&gDynamicSiLibrarySmmProtocolGuid, NULL, &DynamicSiLibrarySmmProtocol);
- if (EFI_ERROR (Status)) {
- ASSERT_EFI_ERROR (Status);
- return Status;
- }
-
- //
- // Init Power Management I/O Base aka ACPI Base
- //
- PchPmBase = DynamicSiLibrarySmmProtocol->PmcGetAcpiBase ();
-
- Pm1Cnt = IoRead16 (PchPmBase + R_ACPI_IO_PM1_CNT);
-
- //
- // Disable SCI
- //
- Pm1Cnt &= ~B_ACPI_IO_PM1_CNT_SCI_EN;
-
- IoWrite16 (PchPmBase + R_ACPI_IO_PM1_CNT, Pm1Cnt);
-
- return EFI_SUCCESS;
-}
diff --git a/Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc b/Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc
index 63cb4dd559..ae3646df7a 100644
--- a/Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc
+++ b/Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc
@@ -627,10 +627,10 @@

[LibraryClasses.Common.DXE_SMM_DRIVER]
SpiFlashCommonLib|$(RP_PKG)/Library/SmmSpiFlashCommonLib/SmmSpiFlashCommonLib.inf
-
TestPointCheckLib|MinPlatformPkg/Test/Library/TestPointCheckLib/SmmTestPointCheckLib.inf
TestPointLib|MinPlatformPkg/Test/Library/TestPointLib/SmmTestPointLib.inf
MmServicesTableLib|MdePkg/Library/MmServicesTableLib/MmServicesTableLib.inf
+
+ BoardAcpiEnableLib|$(RP_PKG)/Library/BoardAcpiLib/SmmBoardAcpiEnableLi
+ b.inf

[LibraryClasses.Common.SMM_CORE]
S3BootScriptLib|MdePkg/Library/BaseS3BootScriptLibNull/BaseS3BootScriptLibNull.inf
@@ -745,10 +745,7 @@

$(RP_PKG)/Features/AcpiVtd/AcpiVtd.inf

- $(PLATFORM_PKG)/Acpi/AcpiSmm/AcpiSmm.inf {
- <LibraryClasses>
- BoardAcpiEnableLib|$(RP_PKG)/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.inf
- }
+ $(PLATFORM_PKG)/Acpi/AcpiSmm/AcpiSmm.inf

$(PLATFORM_PKG)/PlatformInit/PlatformInitDxe/PlatformInitDxe.inf {
<LibraryClasses>
--
2.27.0.windows.1

6561 - 6580 of 90922