Date   

[PATCH v2 05/13] StandaloneMmPkg: Add macros for SPM version

Sughosh Ganu
 

Declare module wide variables for SPM major and minor versions to be
used in checking the SPM version compatibility. Declare the the values
of SPM major and minor versions as macros.

Signed-off-by: Sughosh Ganu <sughosh.ganu@...>
---
ArmPkg/Include/IndustryStandard/ArmMmSvc.h | 3 +++
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c | 10 +++++-----
2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/ArmPkg/Include/IndustryStandard/ArmMmSvc.h b/ArmPkg/Include/IndustryStandard/ArmMmSvc.h
index ee29c2fecc..71a5398558 100644
--- a/ArmPkg/Include/IndustryStandard/ArmMmSvc.h
+++ b/ArmPkg/Include/IndustryStandard/ArmMmSvc.h
@@ -41,4 +41,7 @@
#define ARM_SVC_SPM_RET_DENIED -3
#define ARM_SVC_SPM_RET_NO_MEMORY -5

+#define SPM_MAJOR_VERSION 0
+#define SPM_MINOR_VERSION 1
+
#endif
diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
index c4132b0d78..f2a8feacec 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
@@ -32,8 +32,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#define SPM_MINOR_VER_MASK 0x0000FFFF
#define SPM_MAJOR_VER_SHIFT 16

-CONST UINT32 SPM_MAJOR_VER = 0;
-CONST UINT32 SPM_MINOR_VER = 1;
+CONST UINT32 mSpmMajorVer = SPM_MAJOR_VERSION;
+CONST UINT32 mSpmMinorVer = SPM_MINOR_VERSION;

CONST UINT8 BOOT_PAYLOAD_VERSION = 1;

@@ -183,8 +183,8 @@ GetSpmVersion (VOID)
// revision A must work in a compatible way with revision B.
// However, it is possible for revision B to have a higher
// function count than revision A.
- if ((SpmMajorVersion == SPM_MAJOR_VER) &&
- (SpmMinorVersion >= SPM_MINOR_VER))
+ if ((SpmMajorVersion == mSpmMajorVer) &&
+ (SpmMinorVersion >= mSpmMinorVer))
{
DEBUG ((DEBUG_INFO, "SPM Version: Major=0x%x, Minor=0x%x\n",
SpmMajorVersion, SpmMinorVersion));
@@ -193,7 +193,7 @@ GetSpmVersion (VOID)
else
{
DEBUG ((DEBUG_INFO, "Incompatible SPM Versions.\n Current Version: Major=0x%x, Minor=0x%x.\n Expected: Major=0x%x, Minor>=0x%x.\n",
- SpmMajorVersion, SpmMinorVersion, SPM_MAJOR_VER, SPM_MINOR_VER));
+ SpmMajorVersion, SpmMinorVersion, mSpmMajorVer, mSpmMinorVer));
Status = EFI_UNSUPPORTED;
}

--
2.17.1


[PATCH v2 04/13] ArmPkg: Introduce support for PcdFfaEnable

Sughosh Ganu
 

The Secure Partition(SP) can request services from the Secure
Partition Manager Core(SPMC) either through FF-A calls or through the
existing SVC calls. Add a feature flag Pcd for enabling the FF-A
method -- when this is set to FALSE, the SP uses the existing SVC
calls for making the requests.

Signed-off-by: Sughosh Ganu <sughosh.ganu@...>
---
ArmPkg/ArmPkg.dec | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
index eaf1072d9e..507e16844c 100644
--- a/ArmPkg/ArmPkg.dec
+++ b/ArmPkg/ArmPkg.dec
@@ -78,6 +78,12 @@
# Define if the GICv3 controller should use the GICv2 legacy
gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy|FALSE|BOOLEAN|0x00000042

+ ## Used to select method for requesting services from S-EL1.<BR><BR
+ # TRUE - Selects FF-A calls for communication between S-EL0 and SPMC.<BR>
+ # FALSE - Selects SVC calls for communication between S-EL0 and SPMC.<BR>
+ # @Prompt Enable FF-A support.
+ gArmTokenSpaceGuid.PcdFfaEnable|FALSE|BOOLEAN|0x0000005B
+
[PcdsFeatureFlag.ARM]
# Whether to map normal memory as non-shareable. FALSE is the safe choice, but
# TRUE may be appropriate to fix performance problems if you don't care about
--
2.17.1


[PATCH v2 03/13] StandaloneMmPkg: Use FF-A header file in Standalone MM Core entry point

Sughosh Ganu
 

From: Achin Gupta <achin.gupta@...>

Add the Firmware Framework(FF-A) header in the StandaloneMm entry
point driver. Support for invoking the functions through FF-A will be
added in a subsequent patch.

Signed-off-by: Achin Gupta <achin.gupta@...>
---
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
index 9cecfa667b..c4132b0d78 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
@@ -26,6 +26,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent

#include <IndustryStandard/ArmStdSmc.h>
#include <IndustryStandard/ArmMmSvc.h>
+#include <IndustryStandard/ArmFfaSvc.h>

#define SPM_MAJOR_VER_MASK 0xFFFF0000
#define SPM_MINOR_VER_MASK 0x0000FFFF
--
2.17.1


[PATCH v2 02/13] ArmPkg/ArmSvcLib: Return x4-x7 in output parameters

Sughosh Ganu
 

From: Achin Gupta <achin.gupta@...>

The Arm SMC calling convention standard v1.2 allows 8 input and output
parameter registers. The FF-A specification relies on this
communication. This patch extends the number of output registers
returned by ArmCallSvc() to match this convention.

Signed-off-by: Achin Gupta <achin.gupta@...>
---
ArmPkg/Include/Library/ArmSvcLib.h | 10 ++++++++--
ArmPkg/Library/ArmSvcLib/AArch64/ArmSvc.S | 4 +++-
2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/ArmPkg/Include/Library/ArmSvcLib.h b/ArmPkg/Include/Library/ArmSvcLib.h
index a94ead1965..a4414270f3 100644
--- a/ArmPkg/Include/Library/ArmSvcLib.h
+++ b/ArmPkg/Include/Library/ArmSvcLib.h
@@ -27,10 +27,16 @@ typedef struct {
/**
Trigger an SVC call

- SVC calls can take up to 7 arguments and return up to 4 return values.
- Therefore, the 4 first fields in the ARM_SVC_ARGS structure are used
+ SVC calls can take up to 8 arguments and return up to 8 return values.
+ Therefore, the 8 first fields in the ARM_SVC_ARGS structure are used
for both input and output values.

+ @param[in, out] Args Arguments to be passed as part of the SVC call
+ The return values of the SVC call are also placed
+ in the same structure
+
+ @retval None
+
**/
VOID
ArmCallSvc (
diff --git a/ArmPkg/Library/ArmSvcLib/AArch64/ArmSvc.S b/ArmPkg/Library/ArmSvcLib/AArch64/ArmSvc.S
index ee265f94b9..1a7c10cb79 100644
--- a/ArmPkg/Library/ArmSvcLib/AArch64/ArmSvc.S
+++ b/ArmPkg/Library/ArmSvcLib/AArch64/ArmSvc.S
@@ -33,9 +33,11 @@ ASM_PFX(ArmCallSvc):
ldr x9, [sp, #16]

// Store the SVC returned values into the ARM_SVC_ARGS structure.
- // A SVC call can return up to 4 values - we do not need to store back x4-x7.
+ // A SVC call can return up to 8 values
stp x0, x1, [x9, #0]
stp x2, x3, [x9, #16]
+ stp x4, x5, [x9, #32]
+ stp x6, x7, [x9, #48]

mov x0, x9

--
2.17.1


[PATCH v2 01/13] ArmPkg/IndustryStandard: Add barebones FF-A header

Sughosh Ganu
 

From: Achin Gupta <achin.gupta@...>

This patch adds a rudimentary header file with defines for FF-A ABIs
that will be used as the transport between S-EL0 and the SPM

Signed-off-by: Achin Gupta <achin.gupta@...>
---
ArmPkg/Include/IndustryStandard/ArmFfaSvc.h | 23 ++++++++++++++++++++
1 file changed, 23 insertions(+)

diff --git a/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
new file mode 100644
index 0000000000..1eadf48ab5
--- /dev/null
+++ b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
@@ -0,0 +1,23 @@
+/** @file
+ Header file for FF-A ABI's that will be used for
+ communication between S-EL0 and the Secure Partition
+ Manager(SPM)
+
+ Copyright (c) 2020, ARM Limited. All rights reserved.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Revision Reference:
+ - FF-A Version 1.0
+
+
+**/
+
+#ifndef ARM_FFA_SVC_H_
+#define ARM_FFA_SVC_H_
+
+#define ARM_SVC_ID_FFA_VERSION_AARCH32 0x84000063
+#define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH64 0xC400006F
+#define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP_AARCH64 0xC4000070
+
+#endif // ARM_FFA_SVC_H_
--
2.17.1


[PATCH v2 00/13] Add support for using FF-A calls

Sughosh Ganu
 

The following patch series adds support for using the Firmware
Framework(FF-A) as a transport mechanism for requesting services from
the Secure Partition Manager(SPM). This is done through a Pcd which
can be used to enable the FF-A mechanism or to use the earlier used
SVC calls.


Changes since V1:
Handled review comments from Sami Mujawar


Achin Gupta (7):
ArmPkg/IndustryStandard: Add barebones FF-A header
ArmPkg/ArmSvcLib: Return x4-x7 in output parameters
StandaloneMmPkg: Use FF-A header file in Standalone MM Core entry
point
StandaloneMmPkg: Add option to use FF-A calls for communication with
SPM
StandaloneMmPkg: Use FF-A header file in Standalone MM Arm MMU library
StandaloneMmMmuLib: Add option to use FF-A calls to get memory
region's permissions
StandaloneMmMmuLib: Add option to use FF-A calls to set memory
region's permissions

Ilias Apalodimas (2):
MdeModulePkg/VariableStandaloneMm: Set PcdFlashNvStorageVariableBase
to Pcd
StandaloneMmPkg: Allow sending FFA Direct Request message to
StandaloneMm

Sughosh Ganu (4):
ArmPkg: Introduce support for PcdFfaEnable
StandaloneMmPkg: Add macros for SPM version
StandaloneMmPkg: Add the SPM version for FF-A
StandaloneMmPkg: Add option to use FF-A calls for getting SPM version

ArmPkg/ArmPkg.dec | 6 +
.../ArmMmuStandaloneMmLib.inf | 3 +
.../RuntimeDxe/VariableStandaloneMm.inf | 6 +-
.../StandaloneMmCoreEntryPoint.inf | 3 +
ArmPkg/Include/IndustryStandard/ArmFfaSvc.h | 44 ++++++
ArmPkg/Include/IndustryStandard/ArmMmSvc.h | 3 +
ArmPkg/Include/Library/ArmSvcLib.h | 10 +-
.../AArch64/ArmMmuStandaloneMmLib.c | 145 +++++++++++++-----
.../StandaloneMmCpu/AArch64/EventHandle.c | 4 +-
.../AArch64/StandaloneMmCoreEntryPoint.c | 130 ++++++++++++----
ArmPkg/Library/ArmSvcLib/AArch64/ArmSvc.S | 4 +-
11 files changed, 291 insertions(+), 67 deletions(-)
create mode 100644 ArmPkg/Include/IndustryStandard/ArmFfaSvc.h

--
2.17.1


Re: [PATCH 1/1] Adding the MAC in the HTTP Boot Title

Maciej Rabeda
 

Nainar,

This is not the first time you are contributing a patch, yet you fail to do it in accordance to the process.
I cannot see *at minimum*: commit message, proper CC tags, bugzilla reference.

Please follow the process to have your patches reviewed.

Thanks,
Maciej

On 09-Dec-20 17:20, INDIA\sivaramann wrote:
---
NetworkPkg/HttpBootDxe/HttpBootConfig.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/NetworkPkg/HttpBootDxe/HttpBootConfig.c b/NetworkPkg/HttpBootDxe/HttpBootConfig.c
index 646c907b12..677d913a09 100644
--- a/NetworkPkg/HttpBootDxe/HttpBootConfig.c
+++ b/NetworkPkg/HttpBootDxe/HttpBootConfig.c
@@ -629,6 +629,18 @@ HttpBootConfigFormInit (
NULL

);


+ OldMenuString = HiiGetString (

+ CallbackInfo->RegisteredHandle,

+ STRING_TOKEN (STR_HTTP_BOOT_CONFIG_FORM_TITLE),

+ NULL

+ );

+ UnicodeSPrint (MenuString, 128, L"MAC:%s-%s", MacString,OldMenuString);

+ HiiSetString (

+ CallbackInfo->RegisteredHandle,

+ STRING_TOKEN (STR_HTTP_BOOT_CONFIG_FORM_TITLE),

+ MenuString,

+ NULL

+ );

FreePool (MacString);

FreePool (OldMenuString);


Re: [PATCH v1 1/1] Silicon/Qemu/Sbsa: sbsa-wdt interrupt id update

Leif Lindholm
 

This is a repost of as far as I can tell the same code as the previous
submission, also called v1.

On Fri, Dec 11, 2020 at 12:03:10 -0500, Shashi Mallela wrote:
The previous value of interrupt id used was causing
conflict with a different device of sbsa-ref platform.
This was preventing the watchdog interrupt from getting
identified.Updated SBSA-wdt interrupt id in Gtdt table
to rectify the issue.
This reads like changing this value in GTDT affects the interrupt
routing in the sbsa-ref mode. That is not the case.

This patch rectifies that the interrupt used in the first submission
of the QEMU patch was changed due to a conflict before merging, but
the EDK2 patch was not updated in sync, and this was missed on review.

That is what this message should say.

"preventing the watchdog timer from being identified" lacks an agent.
Please add one.

/
Leif

Cc: Leif Lindholm <leif@...>
Cc: Ard Biesheuvel <ard.biesheuvel@...>
Cc: Graeme Gregory <graeme@...>
Signed-off-by: Shashi Mallela <shashi.mallela@...>
---
Silicon/Qemu/SbsaQemu/AcpiTables/Gtdt.aslc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Silicon/Qemu/SbsaQemu/AcpiTables/Gtdt.aslc b/Silicon/Qemu/SbsaQemu/AcpiTables/Gtdt.aslc
index a010b908c434..14733a37183d 100644
--- a/Silicon/Qemu/SbsaQemu/AcpiTables/Gtdt.aslc
+++ b/Silicon/Qemu/SbsaQemu/AcpiTables/Gtdt.aslc
@@ -36,7 +36,7 @@

#define SBSAQEMU_WDT_REFRESH_FRAME_BASE 0x50010000
#define SBSAQEMU_WDT_CONTROL_FRAME_BASE 0x50011000
-#define SBSAQEMU_WDT_IRQ 44
+#define SBSAQEMU_WDT_IRQ 48

#define GTDT_WDTIMER_EDGE_TRIGGERED EFI_ACPI_6_3_GTDT_SBSA_GENERIC_WATCHDOG_FLAG_TIMER_INTERRUPT_MODE
#define GTDT_WDTIMER_LEVEL_TRIGGERED 0
--
2.27.0


Re: [PATCH 1/2] MdeModulePkg/SataControllerDxe: Add support for drives in RAID mode

Vitaly Cheptsov
 

Hello Hao,

No, it is not possible to change that, since there is no firmware preference for that. The firmware does not support UEFI, and we are running through DuetPkg.

I believe it is not quite a workaround as the fact that an actual RAID array is installed and the fact that a RAID array is supported are separate matters and may not be distinguishable via IS_RAID. At least this is how Intel controllers work to date. A clear name for the PCD should not cause any confusion. If you think TreatRaidAsSata is not great, we could choose ForceRaidAsSingleDrive.

Best regards,
Vitaly

On 14 Dec 2020, at 10:56, Wu, Hao A <hao.a.wu@...> wrote:

Hello Vitaly,

It sounds to me that the controller driver should properly reflect the mode
according to the actual configuration. Is it possible to update the behavior of
the controller driver?

In my opinion, it seems weird to add WA in these general drivers for platform
driver issue. It might also cause confusion for users of AtaAtapiPassThru to
think it has RAID support.

Best Regards,
Hao Wu

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Vitaly
Cheptsov
Sent: Monday, December 14, 2020 3:34 PM
To: Wu, Hao A <hao.a.wu@...>
Cc: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@...>; Albecki,
Mateusz <mateusz.albecki@...>; Laszlo Ersek <lersek@...>
Subject: Re: [edk2-devel] [PATCH 1/2] MdeModulePkg/SataControllerDxe: Add
support for drives in RAID mode

Hello Hao,

This is for the case when the drives are not used as a RAID, but the controller is
initialised in RAID mode. However, you are right that if a dedicated RAID driver is
present, it is best to use it instead. To support both cases can we introduce an
off-by-default PCD (e.g. TreatRaidAsSata) to workaround this?

Best regards,
Vitaly

On 14 Dec 2020, at 09:22, Wu, Hao A <hao.a.wu@...> wrote:


-----Original Message-----
From: Vitaly Cheptsov <cheptsov@...>
Sent: Friday, December 11, 2020 5:25 PM
To: devel@edk2.groups.io
Cc: Vitaly Cheptsov <cheptsov@...>; Wang, Jian J
<jian.j.wang@...>; Wu, Hao A <hao.a.wu@...>; Albecki,
Mateusz <mateusz.albecki@...>; Laszlo Ersek <lersek@...>
Subject: [PATCH 1/2] MdeModulePkg/SataControllerDxe: Add support for
drives in RAID mode

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

This resolves the problem of using drivers connected to Intel G33
builtin SATA controller when run from DuetPkg when it can only be
configured in RAID mode through the firmware settings.

Hello Vitaly,

If my understanding is correct, this driver (SataControllerDxe) and
the AtaAtapiPassThru driver are written for non-RAID case only.

Both drivers (especially AtaAtapiPassThru) do not distinguish
logic/physical SCSI channels, which I think only works for the
non-RAID case. I am not sure if this patch series will have an impact to existing
RAID drivers.

Best Regards,
Hao Wu



Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Mateusz Albecki <mateusz.albecki@...>
Cc: Laszlo Ersek <lersek@...>
Signed-off-by: Vitaly Cheptsov <cheptsov@...>
---
MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
b/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
index ab06e2833c..301335c967 100644
--- a/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
+++ b/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
@@ -324,7 +324,7 @@ SataControllerSupported (
return EFI_UNSUPPORTED;
}

- if (IS_PCI_IDE (&PciData) || IS_PCI_SATADPA (&PciData)) {
+ if (IS_PCI_IDE (&PciData) || IS_PCI_SATADPA (&PciData) ||
+ IS_PCI_RAID
+ (&PciData)) {
return EFI_SUCCESS;
}

@@ -465,7 +465,7 @@ SataControllerStart (
if (IS_PCI_IDE (&PciData)) {
Private->IdeInit.ChannelCount = IDE_MAX_CHANNEL;
Private->DeviceCount = IDE_MAX_DEVICES;
- } else if (IS_PCI_SATADPA (&PciData)) {
+ } else if (IS_PCI_SATADPA (&PciData) || IS_PCI_RAID (&PciData)) {
//
// Read Ports Implemented(PI) to calculate max port number (0 based).
//
--
2.24.3 (Apple Git-128)



Re: [PATCH 1/2] MdeModulePkg/SataControllerDxe: Add support for drives in RAID mode

Wu, Hao A
 

Hello Vitaly,

It sounds to me that the controller driver should properly reflect the mode
according to the actual configuration. Is it possible to update the behavior of
the controller driver?

In my opinion, it seems weird to add WA in these general drivers for platform
driver issue. It might also cause confusion for users of AtaAtapiPassThru to
think it has RAID support.

Best Regards,
Hao Wu

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Vitaly
Cheptsov
Sent: Monday, December 14, 2020 3:34 PM
To: Wu, Hao A <hao.a.wu@...>
Cc: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@...>; Albecki,
Mateusz <mateusz.albecki@...>; Laszlo Ersek <lersek@...>
Subject: Re: [edk2-devel] [PATCH 1/2] MdeModulePkg/SataControllerDxe: Add
support for drives in RAID mode

Hello Hao,

This is for the case when the drives are not used as a RAID, but the controller is
initialised in RAID mode. However, you are right that if a dedicated RAID driver is
present, it is best to use it instead. To support both cases can we introduce an
off-by-default PCD (e.g. TreatRaidAsSata) to workaround this?

Best regards,
Vitaly

On 14 Dec 2020, at 09:22, Wu, Hao A <hao.a.wu@...> wrote:



-----Original Message-----
From: Vitaly Cheptsov <cheptsov@...>
Sent: Friday, December 11, 2020 5:25 PM
To: devel@edk2.groups.io
Cc: Vitaly Cheptsov <cheptsov@...>; Wang, Jian J
<jian.j.wang@...>; Wu, Hao A <hao.a.wu@...>; Albecki,
Mateusz <mateusz.albecki@...>; Laszlo Ersek <lersek@...>
Subject: [PATCH 1/2] MdeModulePkg/SataControllerDxe: Add support for
drives in RAID mode

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

This resolves the problem of using drivers connected to Intel G33
builtin SATA controller when run from DuetPkg when it can only be
configured in RAID mode through the firmware settings.

Hello Vitaly,

If my understanding is correct, this driver (SataControllerDxe) and
the AtaAtapiPassThru driver are written for non-RAID case only.

Both drivers (especially AtaAtapiPassThru) do not distinguish
logic/physical SCSI channels, which I think only works for the
non-RAID case. I am not sure if this patch series will have an impact to existing
RAID drivers.

Best Regards,
Hao Wu



Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Mateusz Albecki <mateusz.albecki@...>
Cc: Laszlo Ersek <lersek@...>
Signed-off-by: Vitaly Cheptsov <cheptsov@...>
---
MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
b/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
index ab06e2833c..301335c967 100644
--- a/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
+++ b/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
@@ -324,7 +324,7 @@ SataControllerSupported (
return EFI_UNSUPPORTED;
}

- if (IS_PCI_IDE (&PciData) || IS_PCI_SATADPA (&PciData)) {
+ if (IS_PCI_IDE (&PciData) || IS_PCI_SATADPA (&PciData) ||
+ IS_PCI_RAID
+ (&PciData)) {
return EFI_SUCCESS;
}

@@ -465,7 +465,7 @@ SataControllerStart (
if (IS_PCI_IDE (&PciData)) {
Private->IdeInit.ChannelCount = IDE_MAX_CHANNEL;
Private->DeviceCount = IDE_MAX_DEVICES;
- } else if (IS_PCI_SATADPA (&PciData)) {
+ } else if (IS_PCI_SATADPA (&PciData) || IS_PCI_RAID (&PciData)) {
//
// Read Ports Implemented(PI) to calculate max port number (0 based).
//
--
2.24.3 (Apple Git-128)



Re: [PATCH 1/2] MdeModulePkg/SataControllerDxe: Add support for drives in RAID mode

Vitaly Cheptsov
 

Hello Hao,

This is for the case when the drives are not used as a RAID, but the controller is initialised in RAID mode. However, you are right that if a dedicated RAID driver is present, it is best to use it instead. To support both cases can we introduce an off-by-default PCD (e.g. TreatRaidAsSata) to workaround this?

Best regards,
Vitaly

On 14 Dec 2020, at 09:22, Wu, Hao A <hao.a.wu@...> wrote:



-----Original Message-----
From: Vitaly Cheptsov <cheptsov@...>
Sent: Friday, December 11, 2020 5:25 PM
To: devel@edk2.groups.io
Cc: Vitaly Cheptsov <cheptsov@...>; Wang, Jian J
<jian.j.wang@...>; Wu, Hao A <hao.a.wu@...>; Albecki,
Mateusz <mateusz.albecki@...>; Laszlo Ersek <lersek@...>
Subject: [PATCH 1/2] MdeModulePkg/SataControllerDxe: Add support for
drives in RAID mode

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

This resolves the problem of using drivers connected to Intel G33 builtin
SATA controller when run from DuetPkg when it can only be configured in
RAID mode through the firmware settings.

Hello Vitaly,

If my understanding is correct, this driver (SataControllerDxe) and the
AtaAtapiPassThru driver are written for non-RAID case only.

Both drivers (especially AtaAtapiPassThru) do not distinguish logic/physical
SCSI channels, which I think only works for the non-RAID case. I am not sure if
this patch series will have an impact to existing RAID drivers.

Best Regards,
Hao Wu



Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Mateusz Albecki <mateusz.albecki@...>
Cc: Laszlo Ersek <lersek@...>
Signed-off-by: Vitaly Cheptsov <cheptsov@...>
---
MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
b/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
index ab06e2833c..301335c967 100644
--- a/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
+++ b/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
@@ -324,7 +324,7 @@ SataControllerSupported (
return EFI_UNSUPPORTED;
}

- if (IS_PCI_IDE (&PciData) || IS_PCI_SATADPA (&PciData)) {
+ if (IS_PCI_IDE (&PciData) || IS_PCI_SATADPA (&PciData) || IS_PCI_RAID
+ (&PciData)) {
return EFI_SUCCESS;
}

@@ -465,7 +465,7 @@ SataControllerStart (
if (IS_PCI_IDE (&PciData)) {
Private->IdeInit.ChannelCount = IDE_MAX_CHANNEL;
Private->DeviceCount = IDE_MAX_DEVICES;
- } else if (IS_PCI_SATADPA (&PciData)) {
+ } else if (IS_PCI_SATADPA (&PciData) || IS_PCI_RAID (&PciData)) {
//
// Read Ports Implemented(PI) to calculate max port number (0 based).
//
--
2.24.3 (Apple Git-128)


Re: [PATCH 1/2] MdeModulePkg/SataControllerDxe: Add support for drives in RAID mode

Wu, Hao A
 

-----Original Message-----
From: Vitaly Cheptsov <cheptsov@...>
Sent: Friday, December 11, 2020 5:25 PM
To: devel@edk2.groups.io
Cc: Vitaly Cheptsov <cheptsov@...>; Wang, Jian J
<jian.j.wang@...>; Wu, Hao A <hao.a.wu@...>; Albecki,
Mateusz <mateusz.albecki@...>; Laszlo Ersek <lersek@...>
Subject: [PATCH 1/2] MdeModulePkg/SataControllerDxe: Add support for
drives in RAID mode

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

This resolves the problem of using drivers connected to Intel G33 builtin
SATA controller when run from DuetPkg when it can only be configured in
RAID mode through the firmware settings.

Hello Vitaly,

If my understanding is correct, this driver (SataControllerDxe) and the
AtaAtapiPassThru driver are written for non-RAID case only.

Both drivers (especially AtaAtapiPassThru) do not distinguish logic/physical
SCSI channels, which I think only works for the non-RAID case. I am not sure if
this patch series will have an impact to existing RAID drivers.

Best Regards,
Hao Wu



Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Mateusz Albecki <mateusz.albecki@...>
Cc: Laszlo Ersek <lersek@...>
Signed-off-by: Vitaly Cheptsov <cheptsov@...>
---
MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
b/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
index ab06e2833c..301335c967 100644
--- a/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
+++ b/MdeModulePkg/Bus/Pci/SataControllerDxe/SataController.c
@@ -324,7 +324,7 @@ SataControllerSupported (
return EFI_UNSUPPORTED;
}

- if (IS_PCI_IDE (&PciData) || IS_PCI_SATADPA (&PciData)) {
+ if (IS_PCI_IDE (&PciData) || IS_PCI_SATADPA (&PciData) || IS_PCI_RAID
+ (&PciData)) {
return EFI_SUCCESS;
}

@@ -465,7 +465,7 @@ SataControllerStart (
if (IS_PCI_IDE (&PciData)) {
Private->IdeInit.ChannelCount = IDE_MAX_CHANNEL;
Private->DeviceCount = IDE_MAX_DEVICES;
- } else if (IS_PCI_SATADPA (&PciData)) {
+ } else if (IS_PCI_SATADPA (&PciData) || IS_PCI_RAID (&PciData)) {
//
// Read Ports Implemented(PI) to calculate max port number (0 based).
//
--
2.24.3 (Apple Git-128)


Re: VariablePolicy support in StandaloneMM

Masahisa Kojima
 

Hi Kun,

I agree with your proposal and implemented the changes you proposed here: kuqin12/mu_basecore at personal/kuqin/var_check (github.com). Could you please take a look?
I have applied your patch to edk2(slightly modified from mu_basecode
to edk2) and works also fine in my environment.
Is it OK to send these patches to edk2-devel with your "Signed-off-by"?

Best Regards,
Masahisa


On Fri, 4 Dec 2020 at 06:58, Kun Qin <kun.q@...> wrote:

Hi Jiewen,

I agree with your proposal and implemented the changes you proposed here: kuqin12/mu_basecore at personal/kuqin/var_check (github.com). Could you please take a look?

Both traditional and standalone instances are tested on our platforms internally, which rendered no issues on compilability or functionality.

Regards,
Kun


Re: [PATCH v5] IntelSiliconPkg/VTd: Add iommu 5 level paging support

Ni, Ray
 

Acked-by: Ray Ni <ray.ni@...>

-----Original Message-----
From: Huang, Jenny <jenny.huang@...>
Sent: Friday, December 4, 2020 11:33 AM
To: Sheng, W <w.sheng@...>; Yao, Jiewen <jiewen.yao@...>;
devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@...>; Chaganty, Rangasai V
<rangasai.v.chaganty@...>; Feng, Roger <roger.feng@...>
Subject: RE: [PATCH v5] IntelSiliconPkg/VTd: Add iommu 5 level paging support

Reviewed-by: Jenny Huang < jenny.huang@...>

-----Original Message-----
From: Sheng, W <w.sheng@...>
Sent: Tuesday, December 1, 2020 7:03 PM
To: Yao, Jiewen <jiewen.yao@...>; devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@...>; Chaganty, Rangasai V
<rangasai.v.chaganty@...>; Huang, Jenny <jenny.huang@...>;
Feng, Roger <roger.feng@...>
Subject: [PATCH v5] IntelSiliconPkg/VTd: Add iommu 5 level paging support

Hi Jiewen,
About the patch of support 5 level paging iommu.
Thank you for giving the review comments.
I have done all the update.
Could you give "review by" on this patch ?
Thank you.
BR
Sheng Wei


-----Original Message-----
From: Sheng, W
Sent: 2020年11月24日 13:44
To: Yao, Jiewen <jiewen.yao@...>; devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@...>; Chaganty, Rangasai V
<rangasai.v.chaganty@...>; Huang, Jenny <jenny.huang@...>
Subject: RE: [PATCH v4] IntelSiliconPkg/VTd: Add iommu 5 level paging
support

Hi Jiewen, All,
Thank you for the review. I just check and update the patch.
https://edk2.groups.io/g/devel/message/67865?p=,,,20,0,0,0::relevance,
,posteri
d%3A2558558,20,2,0,78471874
Could we continue the patch review ?
BR
Sheng Wei

-----Original Message-----
From: Yao, Jiewen <jiewen.yao@...>
Sent: 2020年11月23日 16:44
To: Sheng, W <w.sheng@...>; devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@...>; Chaganty, Rangasai V
<rangasai.v.chaganty@...>; Huang, Jenny
<jenny.huang@...>
Subject: RE: [PATCH v4] IntelSiliconPkg/VTd: Add iommu 5 level
paging support

Thanks.

I only reviewed the policy part. Comment below:

1) I recommend you can merge below 2 if into one - if
((mAcpiDmarTable-
HostAddressWidth <= 48) &&
(mVtdUnitInformation[VtdIndex].CapReg.Bits.SAGAW & BIT2) != 0)) {
You can use 2 lines, but there is no need to use 2 if.

+ if (mAcpiDmarTable->HostAddressWidth <= 48) {
+ if ((mVtdUnitInformation[VtdIndex].CapReg.Bits.SAGAW & BIT2) != 0)
{
+ mVtdUnitInformation[VtdIndex].Is5LevelPaging = FALSE;
+ }
+ }

2) I think below code has typo.
The DEBUG message about 4-level and 5-level should be reversed.
Also we should use DEBUG_INFO instead of DEBUG_ERROR.

+ if (mVtdUnitInformation[VtdIndex].Is5LevelPaging) {
+ ContextEntry->Bits.AddressWidth = 0x3;
+ DEBUG((DEBUG_ERROR, "Using 4-level page-table on VTD %d\n",
VtdIndex));
+ } else {
+ ContextEntry->Bits.AddressWidth = 0x2;
+ DEBUG((DEBUG_ERROR, "Using 5-level page-table on VTD %d\n",
VtdIndex));
+ }




-----Original Message-----
From: Sheng, W <w.sheng@...>
Sent: Monday, November 23, 2020 4:04 PM
To: devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@...>; Chaganty, Rangasai V
<rangasai.v.chaganty@...>; Yao, Jiewen
<jiewen.yao@...>; Huang, Jenny <jenny.huang@...>
Subject: [PATCH v4] IntelSiliconPkg/VTd: Add iommu 5 level paging
support

Support iommu 5 level paging for translation table.

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

Signed-off-by: Sheng Wei <w.sheng@...>
Cc: Ray Ni <ray.ni@...>
Cc: Rangasai V Chaganty <rangasai.v.chaganty@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Jenny Huang <jenny.huang@...>
---
.../Feature/VTd/IntelVTdDxe/DmaProtection.c | 4 +-
.../Feature/VTd/IntelVTdDxe/DmaProtection.h | 19 +-
.../Feature/VTd/IntelVTdDxe/TranslationTable.c | 281
+++++++++++++++--
----
.../Feature/VTd/IntelVTdDxe/TranslationTableEx.c | 31 ++-
.../Feature/VTd/IntelVTdDxe/VtdReg.c | 10 +-
5 files changed, 245 insertions(+), 100 deletions(-)

diff --git
a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.
c
b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.
c
index 9b6135ef..628565ee 100644
---
a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.
c
+++
b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.
c
@@ -523,10 +523,10 @@ SetupVtd (
for (Index = 0; Index < mVtdUnitNumber; Index++) {
DEBUG ((DEBUG_INFO,"VTD Unit %d (Segment: %04x)\n", Index,
mVtdUnitInformation[Index].Segment));
if (mVtdUnitInformation[Index].ExtRootEntryTable != NULL) {
- DumpDmarExtContextEntryTable
(mVtdUnitInformation[Index].ExtRootEntryTable);
+ DumpDmarExtContextEntryTable
(mVtdUnitInformation[Index].ExtRootEntryTable,
mVtdUnitInformation[Index].Is5LevelPaging);
}
if (mVtdUnitInformation[Index].RootEntryTable != NULL) {
- DumpDmarContextEntryTable
(mVtdUnitInformation[Index].RootEntryTable);
+ DumpDmarContextEntryTable
(mVtdUnitInformation[Index].RootEntryTable,
mVtdUnitInformation[Index].Is5LevelPaging);
}
}

diff --git
a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.
h
b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.
h
index a3331db8..f641cea0 100644
---
a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.
h
+++
b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/DmaProtection.
h
@@ -77,6 +77,7 @@ typedef struct {
BOOLEAN HasDirtyContext;
BOOLEAN HasDirtyPages;
PCI_DEVICE_INFORMATION PciDeviceInfo;
+ BOOLEAN Is5LevelPaging;
} VTD_UNIT_INFORMATION;

//
@@ -375,31 +376,37 @@ ParseDmarAcpiTableRmrr (
/**
Dump DMAR context entry table.

- @param[in] RootEntry DMAR root entry.
+ @param[in] RootEntry DMAR root entry.
+ @param[in] Is5LevelPaging If it is the 5 level paging.
**/
VOID
DumpDmarContextEntryTable (
- IN VTD_ROOT_ENTRY *RootEntry
+ IN VTD_ROOT_ENTRY *RootEntry,
+ IN BOOLEAN Is5LevelPaging
);

/**
Dump DMAR extended context entry table.

- @param[in] ExtRootEntry DMAR extended root entry.
+ @param[in] ExtRootEntry DMAR extended root entry.
+ @param[in] Is5LevelPaging If it is the 5 level paging.
**/
VOID
DumpDmarExtContextEntryTable (
- IN VTD_EXT_ROOT_ENTRY *ExtRootEntry
+ IN VTD_EXT_ROOT_ENTRY *ExtRootEntry, IN BOOLEAN Is5LevelPaging
);

/**
Dump DMAR second level paging entry.

- @param[in] SecondLevelPagingEntry The second level paging entry.
+ @param[in] SecondLevelPagingEntry The second level paging entry.
+ @param[in] Is5LevelPaging If it is the 5 level paging.
**/
VOID
DumpSecondLevelPagingEntry (
- IN VOID *SecondLevelPagingEntry
+ IN VOID *SecondLevelPagingEntry, IN BOOLEAN Is5LevelPaging
);

/**
diff --git
a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/Translatio
nT
ab
le.c
b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/Translatio
nT
ab
le.c
index 201d663d..6c786b40 100644
---
a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/Translatio
nT
ab
le.c
+++
b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/Translatio
nT ab le.c @@ -128,11 +128,26 @@ CreateContextEntry (

DEBUG ((DEBUG_INFO,"Source: S%04x B%02x D%02x F%02x\n",
mVtdUnitInformation[VtdIndex].Segment, SourceId.Bits.Bus,
SourceId.Bits.Device, SourceId.Bits.Function));

- if ((mVtdUnitInformation[VtdIndex].CapReg.Bits.SAGAW & BIT2) == 0)
{
- DEBUG((DEBUG_ERROR, "!!!! 4-level page-table is not supported on
VTD %d !!!!\n", VtdIndex));
+ mVtdUnitInformation[VtdIndex].Is5LevelPaging = FALSE;
+ if ((mVtdUnitInformation[VtdIndex].CapReg.Bits.SAGAW & BIT3) != 0)
{
+ mVtdUnitInformation[VtdIndex].Is5LevelPaging = TRUE;
+ if (mAcpiDmarTable->HostAddressWidth <= 48) {
+ if ((mVtdUnitInformation[VtdIndex].CapReg.Bits.SAGAW &
+ BIT2) != 0)
{
+ mVtdUnitInformation[VtdIndex].Is5LevelPaging = FALSE;
+ }
+ }
+ } else if ((mVtdUnitInformation[VtdIndex].CapReg.Bits.SAGAW &
+ BIT2) ==
0) {
+ DEBUG((DEBUG_ERROR, "!!!! Page-table type is not supported
+ on
VTD %d !!!!\n", VtdIndex));
return EFI_UNSUPPORTED;
}
- ContextEntry->Bits.AddressWidth = 0x2;
+
+ if (mVtdUnitInformation[VtdIndex].Is5LevelPaging) {
+ ContextEntry->Bits.AddressWidth = 0x3;
+ DEBUG((DEBUG_ERROR, "Using 4-level page-table on VTD %d\n",
VtdIndex));
+ } else {
+ ContextEntry->Bits.AddressWidth = 0x2;
+ DEBUG((DEBUG_ERROR, "Using 5-level page-table on VTD %d\n",
VtdIndex));
+ }
}

FlushPageTableMemory (VtdIndex,
(UINTN)mVtdUnitInformation[VtdIndex].RootEntryTable,
EFI_PAGES_TO_SIZE(EntryTablePages));
@@ -148,6 +163,7 @@ CreateContextEntry (
@param[in] MemoryBase The base of the memory.
@param[in] MemoryLimit The limit of the memory.
@param[in] IoMmuAccess The IOMMU access.
+ @param[in] Is5LevelPaging If it is the 5 level paging.

@return The second level paging entry.
**/
@@ -157,16 +173,23 @@ CreateSecondLevelPagingEntryTable (
IN VTD_SECOND_LEVEL_PAGING_ENTRY *SecondLevelPagingEntry,
IN UINT64 MemoryBase,
IN UINT64 MemoryLimit,
- IN UINT64 IoMmuAccess
+ IN UINT64 IoMmuAccess,
+ IN BOOLEAN Is5LevelPaging
)
{
+ UINTN Index5;
UINTN Index4;
UINTN Index3;
UINTN Index2;
+ UINTN Lvl5Start;
+ UINTN Lvl5End;
+ UINTN Lvl4PagesStart;
+ UINTN Lvl4PagesEnd;
UINTN Lvl4Start;
UINTN Lvl4End;
UINTN Lvl3Start;
UINTN Lvl3End;
+ VTD_SECOND_LEVEL_PAGING_ENTRY *Lvl5PtEntry;
VTD_SECOND_LEVEL_PAGING_ENTRY *Lvl4PtEntry;
VTD_SECOND_LEVEL_PAGING_ENTRY *Lvl3PtEntry;
VTD_SECOND_LEVEL_PAGING_ENTRY *Lvl2PtEntry; @@ -184,7 +207,7
@@
CreateSecondLevelPagingEntryTable (
if (SecondLevelPagingEntry == NULL) {
SecondLevelPagingEntry = AllocateZeroPages (1);
if (SecondLevelPagingEntry == NULL) {
- DEBUG ((DEBUG_ERROR,"Could not Alloc LVL4 PT. \n"));
+ DEBUG ((DEBUG_ERROR,"Could not Alloc LVL4 or LVL5 PT.
+ \n"));
return NULL;
}
FlushPageTableMemory (VtdIndex,
(UINTN)SecondLevelPagingEntry, EFI_PAGES_TO_SIZE(1)); @@ -197,66
+220,109 @@ CreateSecondLevelPagingEntryTable (
return SecondLevelPagingEntry;
}

- Lvl4Start = RShiftU64 (BaseAddress, 39) & 0x1FF;
- Lvl4End = RShiftU64 (EndAddress - 1, 39) & 0x1FF;
+ if (Is5LevelPaging) {
+ Lvl5Start = RShiftU64 (BaseAddress, 48) & 0x1FF;
+ Lvl5End = RShiftU64 (EndAddress - 1, 48) & 0x1FF;
+ DEBUG ((DEBUG_INFO," Lvl5Start - 0x%x, Lvl5End - 0x%x\n",
+ Lvl5Start,
Lvl5End));

- DEBUG ((DEBUG_INFO," Lvl4Start - 0x%x, Lvl4End - 0x%x\n",
Lvl4Start, Lvl4End));
+ Lvl4Start = RShiftU64 (BaseAddress, 39) & 0x1FF;
+ Lvl4End = RShiftU64 (EndAddress - 1, 39) & 0x1FF;

- Lvl4PtEntry = (VTD_SECOND_LEVEL_PAGING_ENTRY
*)SecondLevelPagingEntry;
- for (Index4 = Lvl4Start; Index4 <= Lvl4End; Index4++) {
- if (Lvl4PtEntry[Index4].Uint64 == 0) {
- Lvl4PtEntry[Index4].Uint64 = (UINT64)(UINTN)AllocateZeroPages (1);
- if (Lvl4PtEntry[Index4].Uint64 == 0) {
- DEBUG ((DEBUG_ERROR,"!!!!!! ALLOCATE LVL4 PAGE FAIL
(0x%x)!!!!!!\n", Index4));
- ASSERT(FALSE);
- return NULL;
- }
- FlushPageTableMemory (VtdIndex,
(UINTN)Lvl4PtEntry[Index4].Uint64,
SIZE_4KB);
- SetSecondLevelPagingEntryAttribute (&Lvl4PtEntry[Index4],
EDKII_IOMMU_ACCESS_READ | EDKII_IOMMU_ACCESS_WRITE);
- }
+ Lvl4PagesStart = (Lvl5Start<<9) | Lvl4Start;
+ Lvl4PagesEnd = (Lvl5End<<9) | Lvl4End;
+ DEBUG ((DEBUG_INFO," Lvl4PagesStart - 0x%x, Lvl4PagesEnd -
+ 0x%x\n",
Lvl4PagesStart, Lvl4PagesEnd));

- Lvl3Start = RShiftU64 (BaseAddress, 30) & 0x1FF;
- if (ALIGN_VALUE_LOW(BaseAddress + SIZE_1GB, SIZE_1GB) <=
EndAddress) {
- Lvl3End = SIZE_4KB/sizeof(VTD_SECOND_LEVEL_PAGING_ENTRY) - 1;
- } else {
- Lvl3End = RShiftU64 (EndAddress - 1, 30) & 0x1FF;
+ Lvl5PtEntry = (VTD_SECOND_LEVEL_PAGING_ENTRY
*)SecondLevelPagingEntry;
+ } else {
+ Lvl5Start = RShiftU64 (BaseAddress, 48) & 0x1FF;
+ Lvl5End = Lvl5Start;
+
+ Lvl4Start = RShiftU64 (BaseAddress, 39) & 0x1FF;
+ Lvl4End = RShiftU64 (EndAddress - 1, 39) & 0x1FF;
+ DEBUG ((DEBUG_INFO," Lvl4Start - 0x%x, Lvl4End - 0x%x\n",
+ Lvl4Start,
Lvl4End));
+
+ Lvl4PtEntry = (VTD_SECOND_LEVEL_PAGING_ENTRY
*)SecondLevelPagingEntry;
+ }
+
+ for (Index5 = Lvl5Start; Index5 <= Lvl5End; Index5++) {
+ if (Is5LevelPaging) {
+ if (Lvl5PtEntry[Index5].Uint64 == 0) {
+ Lvl5PtEntry[Index5].Uint64 = (UINT64)(UINTN)AllocateZeroPages (1);
+ if (Lvl5PtEntry[Index5].Uint64 == 0) {
+ DEBUG ((DEBUG_ERROR,"!!!!!! ALLOCATE LVL4 PAGE FAIL
(0x%x)!!!!!!\n", Index5));
+ ASSERT(FALSE);
+ return NULL;
+ }
+ FlushPageTableMemory (VtdIndex,
+ (UINTN)Lvl5PtEntry[Index5].Uint64,
SIZE_4KB);
+ SetSecondLevelPagingEntryAttribute (&Lvl5PtEntry[Index5],
EDKII_IOMMU_ACCESS_READ | EDKII_IOMMU_ACCESS_WRITE);
+ }
+ Lvl4Start = Lvl4PagesStart & 0x1FF;
+ if (((Index5+1)<<9) > Lvl4PagesEnd) {
+ Lvl4End = SIZE_4KB/sizeof(VTD_SECOND_LEVEL_PAGING_ENTRY) -
1;;
+ Lvl4PagesStart = (Index5+1)<<9;
+ } else {
+ Lvl4End = Lvl4PagesEnd & 0x1FF;
+ }
+ DEBUG ((DEBUG_INFO," Lvl5(0x%x): Lvl4Start - 0x%x, Lvl4End
+ - 0x%x\n",
Index5, Lvl4Start, Lvl4End));
+ Lvl4PtEntry = (VTD_SECOND_LEVEL_PAGING_ENTRY
*)(UINTN)VTD_64BITS_ADDRESS(Lvl5PtEntry[Index5].Bits.AddressLo,
Lvl5PtEntry[Index5].Bits.AddressHi);
}
- DEBUG ((DEBUG_INFO," Lvl4(0x%x): Lvl3Start - 0x%x, Lvl3End -
0x%x\n",
Index4, Lvl3Start, Lvl3End));

- Lvl3PtEntry = (VTD_SECOND_LEVEL_PAGING_ENTRY
*)(UINTN)VTD_64BITS_ADDRESS(Lvl4PtEntry[Index4].Bits.AddressLo,
Lvl4PtEntry[Index4].Bits.AddressHi);
- for (Index3 = Lvl3Start; Index3 <= Lvl3End; Index3++) {
- if (Lvl3PtEntry[Index3].Uint64 == 0) {
- Lvl3PtEntry[Index3].Uint64 = (UINT64)(UINTN)AllocateZeroPages (1);
- if (Lvl3PtEntry[Index3].Uint64 == 0) {
- DEBUG ((DEBUG_ERROR,"!!!!!! ALLOCATE LVL3 PAGE FAIL (0x%x,
0x%x)!!!!!!\n", Index4, Index3));
+ for (Index4 = Lvl4Start; Index4 <= Lvl4End; Index4++) {
+ if (Lvl4PtEntry[Index4].Uint64 == 0) {
+ Lvl4PtEntry[Index4].Uint64 = (UINT64)(UINTN)AllocateZeroPages (1);
+ if (Lvl4PtEntry[Index4].Uint64 == 0) {
+ DEBUG ((DEBUG_ERROR,"!!!!!! ALLOCATE LVL4 PAGE FAIL
(0x%x)!!!!!!\n", Index4));
ASSERT(FALSE);
return NULL;
}
- FlushPageTableMemory (VtdIndex,
(UINTN)Lvl3PtEntry[Index3].Uint64,
SIZE_4KB);
- SetSecondLevelPagingEntryAttribute (&Lvl3PtEntry[Index3],
EDKII_IOMMU_ACCESS_READ | EDKII_IOMMU_ACCESS_WRITE);
+ FlushPageTableMemory (VtdIndex,
+ (UINTN)Lvl4PtEntry[Index4].Uint64,
SIZE_4KB);
+ SetSecondLevelPagingEntryAttribute (&Lvl4PtEntry[Index4],
EDKII_IOMMU_ACCESS_READ | EDKII_IOMMU_ACCESS_WRITE);
+ }
+
+ Lvl3Start = RShiftU64 (BaseAddress, 30) & 0x1FF;
+ if (ALIGN_VALUE_LOW(BaseAddress + SIZE_1GB, SIZE_1GB) <=
EndAddress) {
+ Lvl3End = SIZE_4KB/sizeof(VTD_SECOND_LEVEL_PAGING_ENTRY) -
1;
+ } else {
+ Lvl3End = RShiftU64 (EndAddress - 1, 30) & 0x1FF;
}
+ DEBUG ((DEBUG_INFO," Lvl4(0x%x): Lvl3Start - 0x%x, Lvl3End
+ - 0x%x\n",
Index4, Lvl3Start, Lvl3End));

- Lvl2PtEntry = (VTD_SECOND_LEVEL_PAGING_ENTRY
*)(UINTN)VTD_64BITS_ADDRESS(Lvl3PtEntry[Index3].Bits.AddressLo,
Lvl3PtEntry[Index3].Bits.AddressHi);
- for (Index2 = 0; Index2 <
SIZE_4KB/sizeof(VTD_SECOND_LEVEL_PAGING_ENTRY); Index2++) {
- Lvl2PtEntry[Index2].Uint64 = BaseAddress;
- SetSecondLevelPagingEntryAttribute (&Lvl2PtEntry[Index2],
IoMmuAccess);
- Lvl2PtEntry[Index2].Bits.PageSize = 1;
- BaseAddress += SIZE_2MB;
+ Lvl3PtEntry = (VTD_SECOND_LEVEL_PAGING_ENTRY
*)(UINTN)VTD_64BITS_ADDRESS(Lvl4PtEntry[Index4].Bits.AddressLo,
Lvl4PtEntry[Index4].Bits.AddressHi);
+ for (Index3 = Lvl3Start; Index3 <= Lvl3End; Index3++) {
+ if (Lvl3PtEntry[Index3].Uint64 == 0) {
+ Lvl3PtEntry[Index3].Uint64 = (UINT64)(UINTN)AllocateZeroPages
(1);
+ if (Lvl3PtEntry[Index3].Uint64 == 0) {
+ DEBUG ((DEBUG_ERROR,"!!!!!! ALLOCATE LVL3 PAGE FAIL
+ (0x%x,
0x%x)!!!!!!\n", Index4, Index3));
+ ASSERT(FALSE);
+ return NULL;
+ }
+ FlushPageTableMemory (VtdIndex,
+ (UINTN)Lvl3PtEntry[Index3].Uint64,
SIZE_4KB);
+ SetSecondLevelPagingEntryAttribute
+ (&Lvl3PtEntry[Index3],
EDKII_IOMMU_ACCESS_READ | EDKII_IOMMU_ACCESS_WRITE);
+ }
+
+ Lvl2PtEntry = (VTD_SECOND_LEVEL_PAGING_ENTRY
*)(UINTN)VTD_64BITS_ADDRESS(Lvl3PtEntry[Index3].Bits.AddressLo,
Lvl3PtEntry[Index3].Bits.AddressHi);
+ for (Index2 = 0; Index2 <
SIZE_4KB/sizeof(VTD_SECOND_LEVEL_PAGING_ENTRY); Index2++) {
+ Lvl2PtEntry[Index2].Uint64 = BaseAddress;
+ SetSecondLevelPagingEntryAttribute
+ (&Lvl2PtEntry[Index2],
IoMmuAccess);
+ Lvl2PtEntry[Index2].Bits.PageSize = 1;
+ BaseAddress += SIZE_2MB;
+ if (BaseAddress >= MemoryLimit) {
+ break;
+ }
+ }
+ FlushPageTableMemory (VtdIndex, (UINTN)Lvl2PtEntry,
+ SIZE_4KB);
if (BaseAddress >= MemoryLimit) {
break;
}
}
- FlushPageTableMemory (VtdIndex, (UINTN)Lvl2PtEntry, SIZE_4KB);
+ FlushPageTableMemory (VtdIndex,
+ (UINTN)&Lvl3PtEntry[Lvl3Start],
(UINTN)&Lvl3PtEntry[Lvl3End + 1] - (UINTN)&Lvl3PtEntry[Lvl3Start]);
if (BaseAddress >= MemoryLimit) {
break;
}
}
- FlushPageTableMemory (VtdIndex, (UINTN)&Lvl3PtEntry[Lvl3Start],
(UINTN)&Lvl3PtEntry[Lvl3End + 1] - (UINTN)&Lvl3PtEntry[Lvl3Start]);
- if (BaseAddress >= MemoryLimit) {
- break;
- }
+ FlushPageTableMemory (VtdIndex,
+ (UINTN)&Lvl4PtEntry[Lvl4Start],
(UINTN)&Lvl4PtEntry[Lvl4End + 1] - (UINTN)&Lvl4PtEntry[Lvl4Start]);
}
- FlushPageTableMemory (VtdIndex, (UINTN)&Lvl4PtEntry[Lvl4Start],
(UINTN)&Lvl4PtEntry[Lvl4End + 1] -
(UINTN)&Lvl4PtEntry[Lvl4Start]);
+ FlushPageTableMemory (VtdIndex, (UINTN)&Lvl5PtEntry[Lvl5Start],
(UINTN)&Lvl5PtEntry[Lvl5End + 1] -
(UINTN)&Lvl5PtEntry[Lvl5Start]);

return SecondLevelPagingEntry;
}
@@ -266,26 +332,28 @@ CreateSecondLevelPagingEntryTable (

@param[in] VtdIndex The index of the VTd engine.
@param[in] IoMmuAccess The IOMMU access.
+ @param[in] Is5LevelPaging If it is the 5 level paging.

@return The second level paging entry.
**/
VTD_SECOND_LEVEL_PAGING_ENTRY *
CreateSecondLevelPagingEntry (
IN UINTN VtdIndex,
- IN UINT64 IoMmuAccess
+ IN UINT64 IoMmuAccess,
+ IN BOOLEAN Is5LevelPaging
)
{
VTD_SECOND_LEVEL_PAGING_ENTRY *SecondLevelPagingEntry;

SecondLevelPagingEntry = NULL;
- SecondLevelPagingEntry = CreateSecondLevelPagingEntryTable
(VtdIndex, SecondLevelPagingEntry, 0, mBelow4GMemoryLimit,
IoMmuAccess);
+ SecondLevelPagingEntry = CreateSecondLevelPagingEntryTable
+ (VtdIndex,
SecondLevelPagingEntry, 0, mBelow4GMemoryLimit, IoMmuAccess,
Is5LevelPaging);
if (SecondLevelPagingEntry == NULL) {
return NULL;
}

if (mAbove4GMemoryLimit != 0) {
ASSERT (mAbove4GMemoryLimit > BASE_4GB);
- SecondLevelPagingEntry = CreateSecondLevelPagingEntryTable
(VtdIndex,
SecondLevelPagingEntry, SIZE_4GB, mAbove4GMemoryLimit,
IoMmuAccess);
+ SecondLevelPagingEntry = CreateSecondLevelPagingEntryTable
+ (VtdIndex,
SecondLevelPagingEntry, SIZE_4GB, mAbove4GMemoryLimit,
IoMmuAccess, Is5LevelPaging);
if (SecondLevelPagingEntry == NULL) {
return NULL;
}
@@ -326,11 +394,13 @@ SetupTranslationTable (
/**
Dump DMAR context entry table.

- @param[in] RootEntry DMAR root entry.
+ @param[in] RootEntry DMAR root entry.
+ @param[in] Is5LevelPaging If it is the 5 level paging.
**/
VOID
DumpDmarContextEntryTable (
- IN VTD_ROOT_ENTRY *RootEntry
+ IN VTD_ROOT_ENTRY *RootEntry,
+ IN BOOLEAN Is5LevelPaging
)
{
UINTN Index;
@@ -359,7 +429,7 @@ DumpDmarContextEntryTable (
if (ContextEntry[Index2].Bits.Present == 0) {
continue;
}
- DumpSecondLevelPagingEntry ((VOID
*)(UINTN)VTD_64BITS_ADDRESS(ContextEntry[Index2].Bits.SecondLevelP
ag
e
TranslationPointerLo,
ContextEntry[Index2].Bits.SecondLevelPageTranslationPointerHi));
+ DumpSecondLevelPagingEntry ((VOID
*)(UINTN)VTD_64BITS_ADDRESS(ContextEntry[Index2].Bits.SecondLevelP
ag
e
TranslationPointerLo,
ContextEntry[Index2].Bits.SecondLevelPageTranslationPointerHi),
Is5LevelPaging);
}
}
DEBUG ((DEBUG_INFO,"=========================\n"));
@@ -368,17 +438,22 @@ DumpDmarContextEntryTable (
/**
Dump DMAR second level paging entry.

- @param[in] SecondLevelPagingEntry The second level paging entry.
+ @param[in] SecondLevelPagingEntry The second level paging entry.
+ @param[in] Is5LevelPaging If it is the 5 level paging.
**/
VOID
DumpSecondLevelPagingEntry (
- IN VOID *SecondLevelPagingEntry
+ IN VOID *SecondLevelPagingEntry, IN BOOLEAN Is5LevelPaging
)
{
+ UINTN Index5;
UINTN Index4;
UINTN Index3;
UINTN Index2;
UINTN Index1;
+ UINTN Lvl5IndexEnd;
+ VTD_SECOND_LEVEL_PAGING_ENTRY *Lvl5PtEntry;
VTD_SECOND_LEVEL_PAGING_ENTRY *Lvl4PtEntry;
VTD_SECOND_LEVEL_PAGING_ENTRY *Lvl3PtEntry;
VTD_SECOND_LEVEL_PAGING_ENTRY *Lvl2PtEntry; @@ -386,38
+461,53
@@
DumpSecondLevelPagingEntry (

DEBUG ((DEBUG_VERBOSE,"================\n"));
DEBUG ((DEBUG_VERBOSE,"DMAR Second Level Page Table:\n"));
+ DEBUG ((DEBUG_VERBOSE,"SecondLevelPagingEntry Base - 0x%x,
Is5LevelPaging - %d\n", SecondLevelPagingEntry, Is5LevelPaging));

- DEBUG ((DEBUG_VERBOSE,"SecondLevelPagingEntry Base - 0x%x\n",
SecondLevelPagingEntry));
+ Lvl5IndexEnd = Is5LevelPaging ?
SIZE_4KB/sizeof(VTD_SECOND_LEVEL_PAGING_ENTRY) : 1;
Lvl4PtEntry = (VTD_SECOND_LEVEL_PAGING_ENTRY
*)SecondLevelPagingEntry;
- for (Index4 = 0; Index4 <
SIZE_4KB/sizeof(VTD_SECOND_LEVEL_PAGING_ENTRY); Index4++) {
- if (Lvl4PtEntry[Index4].Uint64 != 0) {
- DEBUG ((DEBUG_VERBOSE," Lvl4Pt Entry(0x%03x) - 0x%016lx\n",
Index4,
Lvl4PtEntry[Index4].Uint64));
- }
- if (Lvl4PtEntry[Index4].Uint64 == 0) {
- continue;
- }
- Lvl3PtEntry = (VTD_SECOND_LEVEL_PAGING_ENTRY
*)(UINTN)VTD_64BITS_ADDRESS(Lvl4PtEntry[Index4].Bits.AddressLo,
Lvl4PtEntry[Index4].Bits.AddressHi);
- for (Index3 = 0; Index3 <
SIZE_4KB/sizeof(VTD_SECOND_LEVEL_PAGING_ENTRY); Index3++) {
- if (Lvl3PtEntry[Index3].Uint64 != 0) {
- DEBUG ((DEBUG_VERBOSE," Lvl3Pt Entry(0x%03x) - 0x%016lx\n",
Index3, Lvl3PtEntry[Index3].Uint64));
+ Lvl5PtEntry = (VTD_SECOND_LEVEL_PAGING_ENTRY
*)SecondLevelPagingEntry;
+
+ for (Index5 = 0; Index5 < Lvl5IndexEnd; Index5++) {
+ if (Is5LevelPaging) {
+ if (Lvl5PtEntry[Index5].Uint64 != 0) {
+ DEBUG ((DEBUG_VERBOSE," Lvl5Pt Entry(0x%03x) -
+ 0x%016lx\n",
Index5, Lvl5PtEntry[Index5].Uint64));
}
- if (Lvl3PtEntry[Index3].Uint64 == 0) {
+ if (Lvl5PtEntry[Index5].Uint64 == 0) {
continue;
}
+ Lvl4PtEntry = (VTD_SECOND_LEVEL_PAGING_ENTRY
*)(UINTN)VTD_64BITS_ADDRESS(Lvl5PtEntry[Index5].Bits.AddressLo,
Lvl5PtEntry[Index5].Bits.AddressHi);
+ }

- Lvl2PtEntry = (VTD_SECOND_LEVEL_PAGING_ENTRY
*)(UINTN)VTD_64BITS_ADDRESS(Lvl3PtEntry[Index3].Bits.AddressLo,
Lvl3PtEntry[Index3].Bits.AddressHi);
- for (Index2 = 0; Index2 <
SIZE_4KB/sizeof(VTD_SECOND_LEVEL_PAGING_ENTRY); Index2++) {
- if (Lvl2PtEntry[Index2].Uint64 != 0) {
- DEBUG ((DEBUG_VERBOSE," Lvl2Pt Entry(0x%03x) -
0x%016lx\n",
Index2, Lvl2PtEntry[Index2].Uint64));
+ for (Index4 = 0; Index4 <
SIZE_4KB/sizeof(VTD_SECOND_LEVEL_PAGING_ENTRY); Index4++) {
+ if (Lvl4PtEntry[Index4].Uint64 != 0) {
+ DEBUG ((DEBUG_VERBOSE," Lvl4Pt Entry(0x%03x) -
+ 0x%016lx\n",
Index4, Lvl4PtEntry[Index4].Uint64));
+ }
+ if (Lvl4PtEntry[Index4].Uint64 == 0) {
+ continue;
+ }
+ Lvl3PtEntry = (VTD_SECOND_LEVEL_PAGING_ENTRY
*)(UINTN)VTD_64BITS_ADDRESS(Lvl4PtEntry[Index4].Bits.AddressLo,
Lvl4PtEntry[Index4].Bits.AddressHi);
+ for (Index3 = 0; Index3 <
SIZE_4KB/sizeof(VTD_SECOND_LEVEL_PAGING_ENTRY); Index3++) {
+ if (Lvl3PtEntry[Index3].Uint64 != 0) {
+ DEBUG ((DEBUG_VERBOSE," Lvl3Pt Entry(0x%03x) - 0x%016lx\n",
Index3, Lvl3PtEntry[Index3].Uint64));
}
- if (Lvl2PtEntry[Index2].Uint64 == 0) {
+ if (Lvl3PtEntry[Index3].Uint64 == 0) {
continue;
}
- if (Lvl2PtEntry[Index2].Bits.PageSize == 0) {
- Lvl1PtEntry = (VTD_SECOND_LEVEL_PAGING_ENTRY
*)(UINTN)VTD_64BITS_ADDRESS(Lvl2PtEntry[Index2].Bits.AddressLo,
Lvl2PtEntry[Index2].Bits.AddressHi);
- for (Index1 = 0; Index1 <
SIZE_4KB/sizeof(VTD_SECOND_LEVEL_PAGING_ENTRY); Index1++) {
- if (Lvl1PtEntry[Index1].Uint64 != 0) {
- DEBUG ((DEBUG_VERBOSE," Lvl1Pt Entry(0x%03x) -
0x%016lx\n",
Index1, Lvl1PtEntry[Index1].Uint64));
+
+ Lvl2PtEntry = (VTD_SECOND_LEVEL_PAGING_ENTRY
*)(UINTN)VTD_64BITS_ADDRESS(Lvl3PtEntry[Index3].Bits.AddressLo,
Lvl3PtEntry[Index3].Bits.AddressHi);
+ for (Index2 = 0; Index2 <
SIZE_4KB/sizeof(VTD_SECOND_LEVEL_PAGING_ENTRY); Index2++) {
+ if (Lvl2PtEntry[Index2].Uint64 != 0) {
+ DEBUG ((DEBUG_VERBOSE," Lvl2Pt Entry(0x%03x) -
0x%016lx\n",
Index2, Lvl2PtEntry[Index2].Uint64));
+ }
+ if (Lvl2PtEntry[Index2].Uint64 == 0) {
+ continue;
+ }
+ if (Lvl2PtEntry[Index2].Bits.PageSize == 0) {
+ Lvl1PtEntry = (VTD_SECOND_LEVEL_PAGING_ENTRY
*)(UINTN)VTD_64BITS_ADDRESS(Lvl2PtEntry[Index2].Bits.AddressLo,
Lvl2PtEntry[Index2].Bits.AddressHi);
+ for (Index1 = 0; Index1 <
SIZE_4KB/sizeof(VTD_SECOND_LEVEL_PAGING_ENTRY); Index1++) {
+ if (Lvl1PtEntry[Index1].Uint64 != 0) {
+ DEBUG ((DEBUG_VERBOSE," Lvl1Pt Entry(0x%03x) -
0x%016lx\n",
Index1, Lvl1PtEntry[Index1].Uint64));
+ }
}
}
}
@@ -510,6 +600,7 @@ PageAttributeToLength (
@param[in] VtdIndex The index used to identify a VTd
engine.
@param[in] SecondLevelPagingEntry The second level paging entry in
VTd table for the device.
@param[in] Address The address to be checked.
+ @param[in] Is5LevelPaging If it is the 5 level paging.
@param[out] PageAttributes The page attribute of the page
entry.

@return The page entry.
@@ -519,6 +610,7 @@ GetSecondLevelPageTableEntry (
IN UINTN VtdIndex,
IN VTD_SECOND_LEVEL_PAGING_ENTRY *SecondLevelPagingEntry,
IN PHYSICAL_ADDRESS Address,
+ IN BOOLEAN Is5LevelPaging,
OUT PAGE_ATTRIBUTE *PageAttribute
)
{
@@ -526,17 +618,38 @@ GetSecondLevelPageTableEntry (
UINTN Index2;
UINTN Index3;
UINTN Index4;
+ UINTN Index5;
UINT64 *L1PageTable;
UINT64 *L2PageTable;
UINT64 *L3PageTable;
UINT64 *L4PageTable;
+ UINT64 *L5PageTable;

+ Index5 = ((UINTN)RShiftU64 (Address, 48)) &
+ PAGING_VTD_INDEX_MASK;
Index4 = ((UINTN)RShiftU64 (Address, 39)) &
PAGING_VTD_INDEX_MASK;
Index3 = ((UINTN)Address >> 30) & PAGING_VTD_INDEX_MASK;
Index2 = ((UINTN)Address >> 21) & PAGING_VTD_INDEX_MASK;
Index1 = ((UINTN)Address >> 12) & PAGING_VTD_INDEX_MASK;

- L4PageTable = (UINT64 *)SecondLevelPagingEntry;
+ if (Is5LevelPaging) {
+ L5PageTable = (UINT64 *)SecondLevelPagingEntry;
+ if (L5PageTable[Index5] == 0) {
+ L5PageTable[Index5] = (UINT64)(UINTN)AllocateZeroPages (1);
+ if (L5PageTable[Index5] == 0) {
+ DEBUG ((DEBUG_ERROR,"!!!!!! ALLOCATE LVL5 PAGE FAIL
(0x%x)!!!!!!\n", Index4));
+ ASSERT(FALSE);
+ *PageAttribute = PageNone;
+ return NULL;
+ }
+ FlushPageTableMemory (VtdIndex, (UINTN)L5PageTable[Index5],
SIZE_4KB);
+ SetSecondLevelPagingEntryAttribute
((VTD_SECOND_LEVEL_PAGING_ENTRY *)&L5PageTable[Index5],
EDKII_IOMMU_ACCESS_READ | EDKII_IOMMU_ACCESS_WRITE);
+ FlushPageTableMemory (VtdIndex,
+ (UINTN)&L5PageTable[Index5],
sizeof(L5PageTable[Index5]));
+ }
+ L4PageTable = (UINT64 *)(UINTN)(L5PageTable[Index5] &
PAGING_4K_ADDRESS_MASK_64);
+ } else {
+ L4PageTable = (UINT64 *)SecondLevelPagingEntry; }
+
if (L4PageTable[Index4] == 0) {
L4PageTable[Index4] = (UINT64)(UINTN)AllocateZeroPages (1);
if (L4PageTable[Index4] == 0) { @@ -785,7 +898,7 @@
SetSecondLevelPagingAttribute (
}

while (Length != 0) {
- PageEntry = GetSecondLevelPageTableEntry (VtdIndex,
SecondLevelPagingEntry, BaseAddress, &PageAttribute);
+ PageEntry = GetSecondLevelPageTableEntry (VtdIndex,
SecondLevelPagingEntry, BaseAddress,
mVtdUnitInformation[VtdIndex].Is5LevelPaging, &PageAttribute);
if (PageEntry == NULL) {
DEBUG ((DEBUG_ERROR, "PageEntry - NULL\n"));
return RETURN_UNSUPPORTED;
@@ -913,7 +1026,7 @@ SetAccessAttribute (

if (ExtContextEntry != NULL) {
if (ExtContextEntry->Bits.Present == 0) {
- SecondLevelPagingEntry = CreateSecondLevelPagingEntry (VtdIndex,
0);
+ SecondLevelPagingEntry = CreateSecondLevelPagingEntry
+ (VtdIndex, 0,
mVtdUnitInformation[VtdIndex].Is5LevelPaging);
DEBUG ((DEBUG_VERBOSE,"SecondLevelPagingEntry - 0x%x (S%04x
B%02x D%02x F%02x) New\n", SecondLevelPagingEntry, Segment,
SourceId.Bits.Bus, SourceId.Bits.Device, SourceId.Bits.Function));
Pt = (UINT64)RShiftU64
((UINT64)(UINTN)SecondLevelPagingEntry,
12);

@@ -922,7 +1035,7 @@ SetAccessAttribute (
ExtContextEntry->Bits.DomainIdentifier = DomainIdentifier;
ExtContextEntry->Bits.Present = 1;
FlushPageTableMemory (VtdIndex, (UINTN)ExtContextEntry,
sizeof(*ExtContextEntry));
- DumpDmarExtContextEntryTable
(mVtdUnitInformation[VtdIndex].ExtRootEntryTable);
+ DumpDmarExtContextEntryTable
(mVtdUnitInformation[VtdIndex].ExtRootEntryTable,
mVtdUnitInformation[VtdIndex].Is5LevelPaging);
mVtdUnitInformation[VtdIndex].HasDirtyContext = TRUE;
} else {
SecondLevelPagingEntry = (VOID
*)(UINTN)VTD_64BITS_ADDRESS(ExtContextEntry-
Bits.SecondLevelPageTranslationPointerLo, ExtContextEntry-
Bits.SecondLevelPageTranslationPointerHi);
@@ -930,7 +1043,7 @@ SetAccessAttribute (
}
} else if (ContextEntry != NULL) {
if (ContextEntry->Bits.Present == 0) {
- SecondLevelPagingEntry = CreateSecondLevelPagingEntry (VtdIndex,
0);
+ SecondLevelPagingEntry = CreateSecondLevelPagingEntry
+ (VtdIndex, 0,
mVtdUnitInformation[VtdIndex].Is5LevelPaging);
DEBUG ((DEBUG_VERBOSE,"SecondLevelPagingEntry - 0x%x (S%04x
B%02x D%02x F%02x) New\n", SecondLevelPagingEntry, Segment,
SourceId.Bits.Bus, SourceId.Bits.Device, SourceId.Bits.Function));
Pt = (UINT64)RShiftU64
((UINT64)(UINTN)SecondLevelPagingEntry,
12);

@@ -939,7 +1052,7 @@ SetAccessAttribute (
ContextEntry->Bits.DomainIdentifier = DomainIdentifier;
ContextEntry->Bits.Present = 1;
FlushPageTableMemory (VtdIndex, (UINTN)ContextEntry,
sizeof(*ContextEntry));
- DumpDmarContextEntryTable
(mVtdUnitInformation[VtdIndex].RootEntryTable);
+ DumpDmarContextEntryTable
(mVtdUnitInformation[VtdIndex].RootEntryTable,
mVtdUnitInformation[VtdIndex].Is5LevelPaging);
mVtdUnitInformation[VtdIndex].HasDirtyContext = TRUE;
} else {
SecondLevelPagingEntry = (VOID
*)(UINTN)VTD_64BITS_ADDRESS(ContextEntry-
Bits.SecondLevelPageTranslationPointerLo, ContextEntry-
Bits.SecondLevelPageTranslationPointerHi);
@@ -1000,7 +1113,7 @@ AlwaysEnablePageAttribute (

if (mVtdUnitInformation[VtdIndex].FixedSecondLevelPagingEntry == 0) {
DEBUG((DEBUG_INFO, "CreateSecondLevelPagingEntry - %d\n",
VtdIndex));
- mVtdUnitInformation[VtdIndex].FixedSecondLevelPagingEntry =
CreateSecondLevelPagingEntry (VtdIndex, EDKII_IOMMU_ACCESS_READ |
EDKII_IOMMU_ACCESS_WRITE);
+ mVtdUnitInformation[VtdIndex].FixedSecondLevelPagingEntry =
CreateSecondLevelPagingEntry (VtdIndex, EDKII_IOMMU_ACCESS_READ |
EDKII_IOMMU_ACCESS_WRITE,
mVtdUnitInformation[VtdIndex].Is5LevelPaging);
}

SecondLevelPagingEntry =
mVtdUnitInformation[VtdIndex].FixedSecondLevelPagingEntry;
diff --git
a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTableEx.
c
b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTableEx.
c
index 0ed9e3ca..a4466891 100644
---
a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTableEx.
c
+++
b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/TranslationTableEx.
c
@@ -78,11 +78,28 @@ CreateExtContextEntry (

DEBUG ((DEBUG_INFO,"DOMAIN: S%04x, B%02x D%02x F%02x\n",
mVtdUnitInformation[VtdIndex].Segment, SourceId.Bits.Bus,
SourceId.Bits.Device, SourceId.Bits.Function));

- if ((mVtdUnitInformation[VtdIndex].CapReg.Bits.SAGAW & BIT2) == 0)
{
- DEBUG((DEBUG_ERROR, "!!!! 4-level page-table is not supported on
VTD %d !!!!\n", VtdIndex));
+ mVtdUnitInformation[VtdIndex].Is5LevelPaging = FALSE;
+ if ((mVtdUnitInformation[VtdIndex].CapReg.Bits.SAGAW & BIT3) != 0)
{
+ mVtdUnitInformation[VtdIndex].Is5LevelPaging = TRUE;
+ if (mAcpiDmarTable->HostAddressWidth <= 48) {
+ if ((mVtdUnitInformation[VtdIndex].CapReg.Bits.SAGAW &
+ BIT2) != 0)
{
+ mVtdUnitInformation[VtdIndex].Is5LevelPaging = FALSE;
+ }
+ }
+ } else if ((mVtdUnitInformation[VtdIndex].CapReg.Bits.SAGAW &
+ BIT2) ==
0) {
+ DEBUG((DEBUG_ERROR, "!!!! Page-table type is not supported
+ on
VTD %d !!!!\n", VtdIndex));
return EFI_UNSUPPORTED;
}
- ExtContextEntry->Bits.AddressWidth = 0x2;
+
+ if (mVtdUnitInformation[VtdIndex].Is5LevelPaging) {
+ ExtContextEntry->Bits.AddressWidth = 0x3;
+ DEBUG((DEBUG_ERROR, "Using 4-level page-table on VTD %d\n",
VtdIndex));
+ } else {
+ ExtContextEntry->Bits.AddressWidth = 0x2;
+ DEBUG((DEBUG_ERROR, "Using 5-level page-table on VTD %d\n",
VtdIndex));
+ }
+
+
}

FlushPageTableMemory (VtdIndex,
(UINTN)mVtdUnitInformation[VtdIndex].ExtRootEntryTable,
EFI_PAGES_TO_SIZE(EntryTablePages));
@@ -93,11 +110,13 @@ CreateExtContextEntry (
/**
Dump DMAR extended context entry table.

- @param[in] ExtRootEntry DMAR extended root entry.
+ @param[in] ExtRootEntry DMAR extended root entry.
+ @param[in] Is5LevelPaging If it is the 5 level paging.
**/
VOID
DumpDmarExtContextEntryTable (
- IN VTD_EXT_ROOT_ENTRY *ExtRootEntry
+ IN VTD_EXT_ROOT_ENTRY *ExtRootEntry, IN BOOLEAN Is5LevelPaging
)
{
UINTN Index;
@@ -127,7 +146,7 @@ DumpDmarExtContextEntryTable (
if (ExtContextEntry[Index2].Bits.Present == 0) {
continue;
}
- DumpSecondLevelPagingEntry ((VOID
*)(UINTN)VTD_64BITS_ADDRESS(ExtContextEntry[Index2].Bits.SecondLev
el
Pa
geTranslationPointerLo,
ExtContextEntry[Index2].Bits.SecondLevelPageTranslationPointerHi))
;
+ DumpSecondLevelPagingEntry ((VOID
*)(UINTN)VTD_64BITS_ADDRESS(ExtContextEntry[Index2].Bits.SecondLev
el
Pa
geTranslationPointerLo,
ExtContextEntry[Index2].Bits.SecondLevelPageTranslationPointerHi),
Is5LevelPaging);
}

if (ExtRootEntry[Index].Bits.UpperPresent == 0) { diff --git
a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c
b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c
index 699639ba..686d235f 100644
---
a/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg.c
+++ b/Silicon/Intel/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/VtdReg
+++ .c
@@ -174,8 +174,14 @@ PrepareVtdConfig (
if ((mVtdUnitInformation[Index].CapReg.Bits.SLLPS & BIT0) == 0) {
DEBUG((DEBUG_WARN, "!!!! 2MB super page is not supported on
VTD %d !!!!\n", Index));
}
- if ((mVtdUnitInformation[Index].CapReg.Bits.SAGAW & BIT2) == 0) {
- DEBUG((DEBUG_ERROR, "!!!! 4-level page-table is not supported on
VTD %d !!!!\n", Index));
+ if ((mVtdUnitInformation[Index].CapReg.Bits.SAGAW & BIT3) != 0) {
+ DEBUG((DEBUG_INFO, "Support 5-level page-table on VTD
+ %d\n",
Index));
+ }
+ if ((mVtdUnitInformation[Index].CapReg.Bits.SAGAW & BIT2) != 0) {
+ DEBUG((DEBUG_INFO, "Support 4-level page-table on VTD
+ %d\n",
Index));
+ }
+ if ((mVtdUnitInformation[Index].CapReg.Bits.SAGAW & (BIT3 |
+ BIT2)) == 0)
{
+ DEBUG((DEBUG_ERROR, "!!!! Page-table type 0x%X is not
+ supported on
VTD %d !!!!\n", Index, mVtdUnitInformation[Index].CapReg.Bits.SAGAW));
return ;
}

--
2.16.2.windows.1


Re: [PATCH V2] UefiCpuPkg/CpuFeature: reduce time complexty to calc CpuInfo.First

Zeng, Star
 

Reviewed-by: Star Zeng <star.zeng@...>

-----Original Message-----
From: Dong, Eric <eric.dong@...>
Sent: Monday, December 14, 2020 8:41 AM
To: Ni, Ray <ray.ni@...>; devel@edk2.groups.io
Cc: Zeng, Star <star.zeng@...>; Lou, Yun <yun.lou@...>; Laszlo Ersek <lersek@...>
Subject: RE: [PATCH V2] UefiCpuPkg/CpuFeature: reduce time complexty to calc CpuInfo.First

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

-----Original Message-----
From: Ni, Ray <ray.ni@...>
Sent: Friday, December 11, 2020 6:48 PM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@...>; Zeng, Star <star.zeng@...>; Lou, Yun <yun.lou@...>; Laszlo Ersek <lersek@...>
Subject: [PATCH V2] UefiCpuPkg/CpuFeature: reduce time complexty to calc CpuInfo.First

CpuInfo.First stores whether the current thread belongs to the first package in the platform, first core in a package, first thread in a core.

But the time complexity of original algorithm to calculate the CpuInfo.First is O (n) * O (p) * O (c).
n: number of processors
p: number of packages
c: number of cores per package

The patch trades time with space by storing the first package, first core per package, first thread per core in an array.
The time complexity becomes O (n).

Signed-off-by: Ray Ni <ray.ni@...>
Cc: Eric Dong <eric.dong@...>
Cc: Star Zeng <star.zeng@...>
Cc: Yun Lou <yun.lou@...>
Cc: Laszlo Ersek <lersek@...>
---
.../CpuFeaturesInitialize.c | 96 +++++++++----------
1 file changed, 47 insertions(+), 49 deletions(-)

diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
index d4a576385f..a1e972b1a2 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
@@ -105,7 +105,10 @@ CpuInitDataInitialize (
EFI_CPU_PHYSICAL_LOCATION *Location; UINT32 PackageIndex; UINT32 CoreIndex;- UINT32 First;+ UINTN Pages;+ UINT32 FirstPackage;+ UINT32 *FirstCore;+ UINT32 *FirstThread; ACPI_CPU_DATA *AcpiCpuData; CPU_STATUS_INFORMATION *CpuStatus; UINT32 *ThreadCountPerPackage;@@ -236,74 +239,69 @@ CpuInitDataInitialize (
// // Initialize CpuFeaturesData->InitOrder[].CpuInfo.First+ // Use AllocatePages () instead of AllocatePool () because pool cannot be freed in PEI phase but page can. //+ Pages = EFI_SIZE_TO_PAGES (CpuStatus->PackageCount * sizeof (UINT32) + CpuStatus->PackageCount * CpuStatus->MaxCoreCount * sizeof (UINT32));+ FirstCore = AllocatePages (Pages);+ ASSERT (FirstCore != NULL);+ FirstThread = FirstCore + CpuStatus->PackageCount; //- // Set First.Package for each thread belonging to the first package.+ // Set FirstPackage, FirstCore[], FirstThread[] to maximum package ID, core ID, thread ID. //- First = MAX_UINT32;+ FirstPackage = MAX_UINT32;+ SetMem32 (FirstCore, CpuStatus->PackageCount * sizeof (UINT32), MAX_UINT32);+ SetMem32 (FirstThread, CpuStatus->PackageCount * CpuStatus->MaxCoreCount * sizeof (UINT32), MAX_UINT32);+ for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) { Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;- First = MIN (Location->Package, First);++ //+ // Save the minimum package ID in the platform.+ //+ FirstPackage = MIN (Location->Package, FirstPackage);+ + //+ // Save the minimum core ID per package.+ //+ FirstCore[Location->Package] = MIN (Location->Core, FirstCore[Location->Package]);+ + //+ // Save the minimum thread ID per core.+ //+ FirstThread[Location->Package * CpuStatus->MaxCoreCount + Location->Core] = MIN (+ Location->Thread,+ FirstThread[Location->Package * CpuStatus->MaxCoreCount + Location->Core]+ ); }++ //+ // Update the First field.+ // for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) { Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;- if (Location->Package == First) {++ if (Location->Package == FirstPackage) { CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Package = 1; }- } - //- // Set First.Die/Tile/Module for each thread assuming:- // single Die under each package, single Tile under each Die, single Module under each Tile- //- for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {+ //+ // Set First.Die/Tile/Module for each thread assuming:+ // single Die under each package, single Tile under each Die, single Module under each Tile+ // CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Die = 1; CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Tile = 1; CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Module = 1;- } - for (PackageIndex = 0; PackageIndex < CpuStatus->PackageCount; PackageIndex++) {- //- // Set First.Core for each thread in the first core of each package.- //- First = MAX_UINT32;- for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {- Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;- if (Location->Package == PackageIndex) {- First = MIN (Location->Core, First);- }+ if (Location->Core == FirstCore[Location->Package]) {+ CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Core = 1; }-- for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {- Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;- if (Location->Package == PackageIndex && Location->Core == First) {- CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Core = 1;- }+ if (Location->Thread == FirstThread[Location->Package * CpuStatus->MaxCoreCount + Location->Core]) {+ CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Thread = 1; } } - for (PackageIndex = 0; PackageIndex < CpuStatus->PackageCount; PackageIndex++) {- for (CoreIndex = 0; CoreIndex < CpuStatus->MaxCoreCount; CoreIndex++) {- //- // Set First.Thread for the first thread of each core.- //- First = MAX_UINT32;- for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {- Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;- if (Location->Package == PackageIndex && Location->Core == CoreIndex) {- First = MIN (Location->Thread, First);- }- }-- for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus; ProcessorNumber++) {- Location = &CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.ProcessorInfo.Location;- if (Location->Package == PackageIndex && Location->Core == CoreIndex && Location->Thread == First) {- CpuFeaturesData->InitOrder[ProcessorNumber].CpuInfo.First.Thread = 1;- }- }- }- }+ FreePages (FirstCore, Pages); } /**--
2.27.0.windows.1


[PATCH EDK2 v1 1/1] BaseTools/GenFfs: Optimazing else if statement

wenyi,xie
 

When Alignment < 0x400 is false, the expression of Alignment >= 0x400 is
always true. So extract the expression from the else if statement.

Cc: Bob Feng <bob.c.feng@...>
Cc: Liming Gao <gaoliming@...>
Cc: Yuwei Chen <yuwei.chen@...>
Signed-off-by: Wenyi Xie <xiewenyi2@...>
---
BaseTools/Source/C/GenFfs/GenFfs.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/BaseTools/Source/C/GenFfs/GenFfs.c b/BaseTools/Source/C/GenFfs/GenFfs.c
index fcb911f4fc34..70538b138f33 100644
--- a/BaseTools/Source/C/GenFfs/GenFfs.c
+++ b/BaseTools/Source/C/GenFfs/GenFfs.c
@@ -821,12 +821,11 @@ Returns:
if (Alignment < 0x400){
sprintf (AlignmentBuffer, "%d", Alignment);
}
- else if (Alignment >= 0x400) {
- if (Alignment >= 0x100000) {
+ else if (Alignment >= 0x100000) {
sprintf (AlignmentBuffer, "%dM", Alignment/0x100000);
- } else {
+ }
+ else {
sprintf (AlignmentBuffer, "%dK", Alignment/0x400);
- }
}
Status = StringtoAlignment (AlignmentBuffer, &(InputFileAlign[InputFileNum]));
}
--
2.20.1.windows.1


[PATCH EDK2 v1 0/1] BaseTools/GenFfs: Optimazing else if statement

wenyi,xie
 

Main Changes :
When Alignment < 0x400 is false, the expression of Alignment >= 0x400 is
always true. So extract the expression from the
else if (Alignment >= 0x400statement).

Wenyi Xie (1):
BaseTools/GenFfs: Optimazing else if statement

BaseTools/Source/C/GenFfs/GenFfs.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

--
2.20.1.windows.1


回复: [PATCH] MdePkg/include: Add DMAR SATC Table Definition

gaoliming
 

Reviewed-by: Liming Gao <gaoliming@...>

-----邮件原件-----
发件人: Liu, Zhiguang <zhiguang.liu@...>
发送时间: 2020年12月11日 10:51
收件人: Sheng, W <w.sheng@...>; devel@edk2.groups.io
抄送: Kinney, Michael D <michael.d.kinney@...>; Liming Gao
<gaoliming@...>; Yao, Jiewen <jiewen.yao@...>; Huang,
Jenny <jenny.huang@...>; Kowalewski, Robert
<robert.kowalewski@...>; Feng, Roger <roger.feng@...>
主题: RE: [PATCH] MdePkg/include: Add DMAR SATC Table Definition

Reviewed-by: Zhiguang Liu <zhiguang.liu@...>

-----Original Message-----
From: Sheng, W <w.sheng@...>
Sent: Friday, December 11, 2020 9:37 AM
To: devel@edk2.groups.io
Cc: Kinney, Michael D <michael.d.kinney@...>; Liming Gao
<gaoliming@...>; Liu, Zhiguang <zhiguang.liu@...>; Yao,
Jiewen <jiewen.yao@...>; Huang, Jenny <jenny.huang@...>;
Kowalewski, Robert <robert.kowalewski@...>; Feng, Roger
<roger.feng@...>
Subject: [PATCH] MdePkg/include: Add DMAR SATC Table Definition

SoC Integrated Address Translation Cache (SATC) reporting structure is
one
of the Remapping Structure, which is imported since Intel(R)
Virtualization
Technology for Directed I/O (VT-D) Architecture Specification v3.2.

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

Signed-off-by: Sheng Wei <w.sheng@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Jenny Huang <jenny.huang@...>
Cc: Kowalewski Robert <robert.kowalewski@...>
Cc: Feng Roger <roger.feng@...>
---
.../IndustryStandard/DmaRemappingReportingTable.h | 34
++++++++++++++++++++--
1 file changed, 31 insertions(+), 3 deletions(-)

diff --git
a/MdePkg/Include/IndustryStandard/DmaRemappingReportingTable.h
b/MdePkg/Include/IndustryStandard/DmaRemappingReportingTable.h
index 7c50dc972e..48f6959fec 100644
--- a/MdePkg/Include/IndustryStandard/DmaRemappingReportingTable.h
+++ b/MdePkg/Include/IndustryStandard/DmaRemappingReportingTable.h
@@ -2,13 +2,13 @@
DMA Remapping Reporting (DMAR) ACPI table definition from Intel(R)
Virtualization Technology for Directed I/O (VT-D) Architecture
Specification.

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

@par Revision Reference:
- Intel(R) Virtualization Technology for Directed I/O (VT-D)
Architecture
- Specification v2.5, Dated November 2017.
-
http://www.intel.com/content/dam/www/public/us/en/documents/product-
specifications/vt-directed-io-spec.pdf
+ Specification v3.2, Dated October 2020.
+
https://software.intel.com/content/dam/develop/external/us/en/documents
/vt-
directed-io-spec.pdf

@par Glossary:
- HPET - High Precision Event Timer
@@ -39,6 +39,7 @@
#define EFI_ACPI_DMAR_TYPE_ATSR 0x02
#define EFI_ACPI_DMAR_TYPE_RHSA 0x03
#define EFI_ACPI_DMAR_TYPE_ANDD 0x04
+#define EFI_ACPI_DMAR_TYPE_SATC 0x05
///@}

///
@@ -216,6 +217,32 @@ typedef struct {
UINT8 AcpiDeviceNumber;
} EFI_ACPI_DMAR_ANDD_HEADER;

+/**
+ An SoC Integrated Address Translation Cache (SATC) reporting
structure
is
+ defined in section 8.8.
+**/
+typedef struct {
+ EFI_ACPI_DMAR_STRUCTURE_HEADER Header;
+ /**
+ - Bit[0]: ATC_REQUIRED:
+ - If Set, indicates that every SoC integrated device
enumerated
+ in this table has a functional requirement to enable
its
ATC
+ (via the ATS capability) for device operation.
+ - If Clear, any device enumerated in this table can
operate when
+ its respective ATC is not enabled (albeit with reduced
+ performance or functionality).
+ - Bits[7:1] Reserved.
+ **/
+ UINT8 Flags;
+ UINT8 Reserved;
+ ///
+ /// The PCI Segment associated with this SATC structure. All SoC
integrated
+ /// devices within a PCI segment with same value for Flags field must
be
+ /// enumerated in the same SATC structure.
+ ///
+ UINT16 SegmentNumber;
+} EFI_ACPI_DMAR_SATC_HEADER;
+
/**
DMA Remapping Reporting Structure Header as defined in section 8.1
This header will be followed by list of Remapping Structures listed
below
@@ -224,6 +251,7 @@ typedef struct {
- Root Port ATS Capability Reporting (ATSR)
- Remapping Hardware Static Affinity (RHSA)
- ACPI Name-space Device Declaration (ANDD)
+ - SoC Integrated Address Translation Cache reporting (SATC)
These structure types must by reported in numerical order.
i.e., All remapping structures of type 0 (DRHD) enumerated before
remapping
structures of type 1 (RMRR), and so forth.
--
2.16.2.windows.1


回复: [edk2-devel] MdeModulePkg: Fix SetMem parameter in OnigurumaUefiPort

gaoliming
 

Create PR https://github.com/tianocore/edk2/pull/1219

 

Thanks

Liming

发件人: bounce+27952+68612+4905953+8761045@groups.io <bounce+27952+68612+4905953+8761045@groups.io> 代表 gaoliming
发送时间: 20201210 9:28
收件人: devel@edk2.groups.io; anbazhagan@...
抄送: jian.j.wang@...; hao.a.wu@...
主题: 回复: [edk2-devel] MdeModulePkg: Fix SetMem parameter in OnigurumaUefiPort

 

Reviewed-by: Liming Gao <gaoliming@...>

 

发件人: bounce+27952+68609+4905953+8761045@groups.io <bounce+27952+68609+4905953+8761045@groups.io> 代表 Anbazhagan, Baraneedharan via groups.io
发送时间: 20201210 8:21
收件人: devel@edk2.groups.io
抄送: gaoliming@...; jian.j.wang@...; hao.a.wu@...
主题: [edk2-devel] MdeModulePkg: Fix SetMem parameter in OnigurumaUefiPort

 

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

 

Coding error in converting memset call to SetMem - Length and Value

is not swapped on calling SetMem

 

Signed-off-by: Baraneedharan Anbazhagan <anbazhagan@...>

---

MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)

 

diff --git a/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.c b/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.c

index 2b2b0d420d..9aa7b0a68e 100644

--- a/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.c

+++ b/MdeModulePkg/Universal/RegularExpressionDxe/OnigurumaUefiPort.c

@@ -93,6 +93,6 @@ void* memcpy (void *dest, const void *src, unsigned int count)

 

 void* memset (void *dest, char ch, unsigned int count)

{

-  return SetMem (dest, ch, count);

+  return SetMem (dest, count, ch);

}

 

--

2.29.2.windows.3

 


Re: [PATCH v5 0/6] jansson edk2 port

Abner Chang
 

-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
gaoliming
Sent: Monday, December 14, 2020 9:30 AM
To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist)
<abner.chang@...>; michael.d.kinney@...
Cc: 'Sean Brogan' <sean.brogan@...>; 'Bret Barkelew'
<Bret.Barkelew@...>; 'Andrew Fish' <afish@...>; 'Laszlo
Ersek' <lersek@...>; 'Leif Lindholm' <leif@...>; Wang,
Nickle (HPS SW) <nickle.wang@...>; O'Hanley, Peter (EXL)
<peter.ohanley@...>
Subject: 回复: [edk2-devel] [PATCH v5 0/6] jansson edk2 port

Abner:

-----邮件原件-----
发件人: bounce+27952+68741+4905953+8761045@groups.io
<bounce+27952+68741+4905953+8761045@groups.io> 代表 Abner Chang
发送时间: 2020年12月13日 12:02
收件人: devel@edk2.groups.io; michael.d.kinney@...;
gaoliming@...
抄送: 'Sean Brogan' <sean.brogan@...>; 'Bret Barkelew'
<Bret.Barkelew@...>; 'Andrew Fish' <afish@...>;
'Laszlo Ersek' <lersek@...>; 'Leif Lindholm'
<leif@...>; Wang, Nickle (HPS SW) <nickle.wang@...>;
O'Hanley, Peter (EXL) <peter.ohanley@...>
主题: Re: [edk2-devel] [PATCH v5 0/6] jansson edk2 port



-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf
Of Michael D Kinney
Sent: Saturday, December 12, 2020 3:23 AM
To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist)
<abner.chang@...>; gaoliming@...; Kinney, Michael D
<michael.d.kinney@...>
Cc: 'Sean Brogan' <sean.brogan@...>; 'Bret Barkelew'
<Bret.Barkelew@...>; 'Andrew Fish' <afish@...>;
'Laszlo
Ersek' <lersek@...>; 'Leif Lindholm' <leif@...>;
Wang, Nickle (HPS SW) <nickle.wang@...>; O'Hanley, Peter (EXL)
<peter.ohanley@...>
Subject: Re: [edk2-devel] [PATCH v5 0/6] jansson edk2 port

Abner,

Feedback below.

Mike

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
Abner Chang
Sent: Wednesday, December 9, 2020 8:02 PM
To: Kinney, Michael D <michael.d.kinney@...>;
devel@edk2.groups.io; gaoliming@...
Cc: 'Sean Brogan' <sean.brogan@...>; 'Bret Barkelew'
<Bret.Barkelew@...>; 'Andrew Fish'
<afish@...>; 'Laszlo Ersek' <lersek@...>; 'Leif
Lindholm'
<leif@...>; Wang, Nickle (HPS SW) <nickle.wang@...>;
O'Hanley, Peter (EXL) <peter.ohanley@...>
Subject: Re: [edk2-devel] [PATCH v5 0/6] jansson edk2 port



-----Original Message-----
From: Kinney, Michael D [mailto:michael.d.kinney@...]
Sent: Thursday, December 10, 2020 10:33 AM
To: Chang, Abner (HPS SW/FW Technologist)
<abner.chang@...>;
devel@edk2.groups.io; gaoliming@...; Kinney, Michael
D <michael.d.kinney@...>
Cc: 'Sean Brogan' <sean.brogan@...>; 'Bret Barkelew'
<Bret.Barkelew@...>; 'Andrew Fish' <afish@...>;
'Laszlo Ersek' <lersek@...>; 'Leif Lindholm'
<leif@...>; Wang, Nickle (HPS SW)
<nickle.wang@...>; O'Hanley, Peter (EXL)
<peter.ohanley@...>
Subject: RE: [edk2-devel] [PATCH v5 0/6] jansson edk2 port

Abner,

Some questions included below.

Mike


-----Original Message-----
From: Chang, Abner (HPS SW/FW Technologist)
<abner.chang@...>
Sent: Wednesday, December 9, 2020 6:14 PM
To: devel@edk2.groups.io; gaoliming@...
Cc: 'Sean Brogan' <sean.brogan@...>; 'Bret Barkelew'
<Bret.Barkelew@...>; 'Andrew Fish'
<afish@...>; 'Laszlo Ersek' <lersek@...>; 'Leif
Lindholm'
<leif@...>; Kinney, Michael D
<michael.d.kinney@...>; Wang, Nickle (HPS SW)
<nickle.wang@...>; O'Hanley, Peter (EXL)
<peter.ohanley@...>
Subject: RE: [edk2-devel] [PATCH v5 0/6] jansson edk2 port



-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On
Behalf Of gaoliming
Sent: Tuesday, December 8, 2020 2:40 PM
To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW
Technologist) <abner.chang@...>
Cc: 'Sean Brogan' <sean.brogan@...>; 'Bret Barkelew'
<Bret.Barkelew@...>; 'Andrew Fish'
<afish@...>;
'Laszlo Ersek' <lersek@...>; 'Leif Lindholm'
<leif@...>; 'Michael D Kinney'
<michael.d.kinney@...>; Wang, Nickle (HPS SW)
<nickle.wang@...>; O'Hanley, Peter (EXL)
<peter.ohanley@...>
Subject: 回复: [edk2-devel] [PATCH v5 0/6] jansson edk2 port

Abner:
I have minor comments on the library header file.

1. JasonLib.h & BaseUcs2Utf8Lib.h. They don't need to
include the additional header files, such as Uefi.h and
BaseLib.h, because the library header file doesn't depend on
the definitions
from BaseLib.
2. CrtLib.inf needs to list the required library class:
BaseMemoryLib &
PrintLib.

OK, I will wait couple days for other comments and address
that all
together.

Thanks

Thanks
Liming
-----邮件原件-----
发件人: bounce+27952+68426+4905953+8761045@groups.io
<bounce+27952+68426+4905953+8761045@groups.io> 代表
Abner
Chang
发送时间: 2020年12月8日 10:11
收件人: devel@edk2.groups.io
抄送: Sean Brogan <sean.brogan@...>; Bret
Barkelew
<Bret.Barkelew@...>; Andrew Fish
<afish@...>;
Laszlo Ersek <lersek@...>; Leif Lindholm
<leif@...>; Michael D Kinney
<michael.d.kinney@...>; Liming Gao
<gaoliming@...>; Nickle Wang
<nickle.wang@...>;
Peter
O'Hanley <peter.ohanley@...>
主题: [edk2-devel] [PATCH v5 0/6] jansson edk2 port

In v5, move BaseUcs2Utf8Lib to under RedfishPkg.
In v4,
- Address review comments
- Seperate CRT functions to a individule library
CrtLib
under
RedfishPkg.
- Seperate UCS2-UTF8 functions to a individule library
BaseUcs2Utf8Lib under MdeModulePkg.

In v3, Add jansson library as the required submoudle in
CiSettings.py for CI test.
In v2, JsonLib is moved to under RedfishPkg.

edk2 JSON library is based on jansson open source
(https://github.com/akheron/jansson) and wrapped as an
edk2
library.
edk2 JsonLib will be used by edk2 Redfish feature drivers
(not contributed yet) and the edk2 port of libredfish
library (not contributed yet) based on DMTF GitHub
(https://github.com/DMTF/libredfish).

Jansson is licensed under the MIT license(refer to
ReadMe.rst under
edk2).
It is used in production and its API is stable. In
UEFI/EDKII environment, Redfish project consumes jansson
to achieve JSON
operations.

* Jansson version on edk2: 2.13.1

* EDKII jansson library wrapper:
- JsonLib.h:
This is the denifitions of EDKII JSON APIs which are
mapped
to
jannson funcitons accordingly.

- JanssonJsonLibMapping.h:
This is the wrapper file to map funcitons and
definitions
used in
native jannson applications to edk2 JsonLib. This avoids the
modifications on native jannson applications to be
built
under
edk2 environment.
Can you explain the use case for this in more detail?
What are these native jannson applications and why do we need to
build them in edk2?
If we have the jannson submodule, why can't these apps just use
the standard jannson services?

I ask because this is a new concept for edk2 and I want to make
sure it is really required.
The users of jannson library are LibRedfish open source
(https://github.com/DMTF/libredfish) and the Edk2 Redfish features
drivers (WIP) to communicate with Redfish service. EDK2 port of
Libredfish will be sent for review after this one.
Libredfish use "jansson.h" in its source code and uses the native
jansson functions directly. I think most of open source projects
use the
same way to leverage jansson open source project.
However, "jansson.h" is defined in the source code directory that
edk2
module can't refer to it in edk2 metafiles.

Why not? The package DEC file can list include paths.
Mike, I thought we are recommended to not referring to the header file
in the source code or the private header files in the module. My
memory may out of date but I remember that the package's "Include/"
directory should contains all public headers files that are exposed to
other packages or this package. Is this still valid in the edk2 driver
writer's guide? I can't find any similar sentence which restrict to
use the header files out of "Include/" directory in UEFI driver writer's guide.
But any way, that's why we use a wrapper header file and put it under
Include/.
This is true for the normal module. But, submodule code is the different.
Submodule code is in module directory. To access its header file, its directory
is Required to be specified in [Includes] section of Package.dec file.
Liming, do we mention this special usage in edk2 module writer's guide? I still can see in 2.1.2, it says
"Each package has a unified directory structure that separate the different source files.
The root directories in each package are: Include, Library, Application and Drivers.
The include directory contains all public header files that are exposed by this package and are used by this package and other packages..."

However, this spec is kept at v0.7 since 2010 and was converted to GitBook in 2018.
That is convenient if we are allowed to use header files from other source code for the open source project.

Thanks


Thanks
Liming

For example, the RedFishPkg DEC file currently has the following
[Includes]
section:

[Includes]
Include

I could be updated to:

[Includes]
Include
Library\JsonLib\jansson\src

This would allow libs/modules that want to directly use the jansson
services to use

#include <janson.h>

If you wanted to limit the use of <janson.h> to only libs/modules in
the RedfishPkg, then you could use the private include feature:

[Includes]
Include

[Includes.Common.Private]
Library\JsonLib\jansson\src

In fact, this exact pattern is used in the UnitTestFrameworkPkg to
allow comonponents to use the standard includes from the cmocka
submodule:

[Includes]
Library/CmockaLib/cmocka/include

[Includes.Common.Private]
PrivateInclude
Library/CmockaLib/cmocka/include/cmockery


Please evaluate this approach and see of the JassonJsoonMapping.h
file can be removed.

Thus we need an wrapper for these jansson applications. That
JanssonJsonMapping.h defines the EDK2 style API for mapping native
jansson functions. For those edk2 based JSON applications, they
can just
use edk2 style APIs. Such as the edk2 Redfish feature drivers, those
can use the EDK2 coding style compliant API to invoke jansson
functions by using JsonLib.h.
For those native jansson applications, we can just use
JanssonJsonMapping.h to map the native jansson API to EDK2 style
API provided by JsonLib.lib. JanssonJsonMapping.h is not just map
the functions, it also gives edk2 style prototype for jansson
variables such as
json_t. These edk2 style prototypes are used in EDK2 Redfish drivers
or
other
edk2 based application as well.



*Known issue:
Build fail with jansson/src/load.c, overrride and add
code in
load.c
to conditionally use stdin according to HAVE_UNISTD_H macro.
The PR is submitted to jansson open source community.
https://github.com/akheron/jansson/pull/558

Signed-off-by: Abner Chang <abner.chang@...>

Cc: Sean Brogan <sean.brogan@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Andrew Fish <afish@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Leif Lindholm <leif@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Nickle Wang <nickle.wang@...>
Cc: Peter O'Hanley <peter.ohanley@...>

Abner Chang (6):
RedfishPkg/Ucs2Utf8lib: UCS2 to UFT8 manipulation library
edk2: jansson submodule for edk2 JSON library
RedfishPkg/CrtLib: C runtime library
RedfishPkg/library: EDK2 port of jansson library
RedfishPkg: Add EDK2 port of jansson library to build
.pytool: Add required submodule for JsonLib

.gitmodules | 3 +
.pytool/CISettings.py | 2 +
ReadMe.rst | 1
+
RedfishPkg/Include/JanssonJsonMapping.h | 63 +
RedfishPkg/Include/Library/BaseUcs2Utf8Lib.h | 69 +
RedfishPkg/Include/Library/CrtLib.h | 195 +++
RedfishPkg/Include/Library/JsonLib.h | 768
++++++++++++
.../Library/BaseUcs2Utf8Lib/BaseUcs2Utf8Lib.c | 417
+++++++
.../BaseUcs2Utf8Lib/BaseUcs2Utf8Lib.inf | 31 +
RedfishPkg/Library/CrtLib/CrtLib.c | 705
+++++++++++
RedfishPkg/Library/CrtLib/CrtLib.inf | 37 +
RedfishPkg/Library/JsonLib/JsonLib.c | 961
++++++++++++++
RedfishPkg/Library/JsonLib/JsonLib.inf | 101 ++
RedfishPkg/Library/JsonLib/Readme.rst | 40 +
RedfishPkg/Library/JsonLib/assert.h | 16 +
RedfishPkg/Library/JsonLib/errno.h | 16 +
RedfishPkg/Library/JsonLib/jansson | 1 +
RedfishPkg/Library/JsonLib/jansson_config.h | 46 +
.../Library/JsonLib/jansson_private_config.h | 19 +
RedfishPkg/Library/JsonLib/limits.h | 16 +
RedfishPkg/Library/JsonLib/load.c | 1111
+++++++++++++++++
RedfishPkg/Library/JsonLib/math.h | 16 +
RedfishPkg/Library/JsonLib/stdarg.h | 15 +
RedfishPkg/Library/JsonLib/stddef.h | 16 +
RedfishPkg/Library/JsonLib/stdio.h | 15 +
RedfishPkg/Library/JsonLib/stdlib.h | 16 +
RedfishPkg/Library/JsonLib/string.h | 16 +
RedfishPkg/Library/JsonLib/sys/time.h | 15 +
RedfishPkg/Library/JsonLib/sys/types.h | 15 +
RedfishPkg/Library/JsonLib/time.h | 15 +
RedfishPkg/RedfishLibs.dsc.inc | 3 +
RedfishPkg/RedfishPkg.ci.yaml | 33 +
RedfishPkg/RedfishPkg.dec | 15 +
RedfishPkg/RedfishPkg.dsc | 3 +
34 files changed, 4811 insertions(+) create mode 100644
RedfishPkg/Include/JanssonJsonMapping.h
create mode 100644
RedfishPkg/Include/Library/BaseUcs2Utf8Lib.h
create mode 100644 RedfishPkg/Include/Library/CrtLib.h
create mode 100644 RedfishPkg/Include/Library/JsonLib.h
create mode 100644
RedfishPkg/Library/BaseUcs2Utf8Lib/BaseUcs2Utf8Lib.c
create mode 100644
RedfishPkg/Library/BaseUcs2Utf8Lib/BaseUcs2Utf8Lib.inf
create mode 100644 RedfishPkg/Library/CrtLib/CrtLib.c
create mode 100644 RedfishPkg/Library/CrtLib/CrtLib.inf
create mode 100644 RedfishPkg/Library/JsonLib/JsonLib.c
create mode 100644 RedfishPkg/Library/JsonLib/JsonLib.inf
create mode 100644 RedfishPkg/Library/JsonLib/Readme.rst
create mode 100644 RedfishPkg/Library/JsonLib/assert.h
create mode 100644 RedfishPkg/Library/JsonLib/errno.h
create mode 160000 RedfishPkg/Library/JsonLib/jansson
create mode 100644
RedfishPkg/Library/JsonLib/jansson_config.h
create mode 100644
RedfishPkg/Library/JsonLib/jansson_private_config.h
create mode 100644 RedfishPkg/Library/JsonLib/limits.h
create mode 100644 RedfishPkg/Library/JsonLib/load.c
create mode
100644 RedfishPkg/Library/JsonLib/math.h create mode
100644 RedfishPkg/Library/JsonLib/stdarg.h
create mode 100644 RedfishPkg/Library/JsonLib/stddef.h
create mode 100644 RedfishPkg/Library/JsonLib/stdio.h
create mode 100644 RedfishPkg/Library/JsonLib/stdlib.h
create mode 100644 RedfishPkg/Library/JsonLib/string.h
create mode 100644 RedfishPkg/Library/JsonLib/sys/time.h
create mode 100644 RedfishPkg/Library/JsonLib/sys/types.h
create mode 100644 RedfishPkg/Library/JsonLib/time.h

--
2.17.1