Date   

[RESEND PATCH v3 2/4] OvmfPkg/LockBoxLib: use PcdAcpiS3Enable to detect S3 support

Lin, Gary (HPS OE-Linux)
 

To avoid the potential inconsistency between PcdAcpiS3Enable and
QemuFwCfgS3Enabled(), this commit modifies LockBoxLib to detect
S3 support by PcdAcpiS3Enable as modules in MdeModulePkg do.

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

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Jim Fehlig <jfehlig@suse.com>
Cc: Joey Li <jlee@suse.com>
Signed-off-by: Gary Lin <gary.lin@hpe.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
Tested-by: Jim Fehlig <jfehlig@suse.com>
---
v3:
- Add the bugzilla link
---
OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf | 3 +--
OvmfPkg/Library/LockBoxLib/LockBoxDxe.c | 4 +---
2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf b/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
index 38bcc577084a..9140b1ba9de9 100644
--- a/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
+++ b/OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf
@@ -33,8 +33,6 @@ [LibraryClasses]
BaseMemoryLib
DebugLib
UefiBootServicesTableLib
- QemuFwCfgLib
- QemuFwCfgS3Lib

[Protocols]
gEfiLockBoxProtocolGuid ## SOMETIMES_PRODUCES
@@ -42,6 +40,7 @@ [Protocols]
[Pcd]
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageSize
+ gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable

[FeaturePcd]
gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire
diff --git a/OvmfPkg/Library/LockBoxLib/LockBoxDxe.c b/OvmfPkg/Library/LockBoxLib/LockBoxDxe.c
index b28ad4d2dba7..7dc2eea2395a 100644
--- a/OvmfPkg/Library/LockBoxLib/LockBoxDxe.c
+++ b/OvmfPkg/Library/LockBoxLib/LockBoxDxe.c
@@ -12,8 +12,6 @@
#include <Library/UefiBootServicesTableLib.h>
#include <Library/BaseMemoryLib.h>
#include <Library/DebugLib.h>
-#include <Library/QemuFwCfgLib.h>
-#include <Library/QemuFwCfgS3Lib.h>
#include <Protocol/LockBox.h>
#include <LockBoxLib.h>

@@ -117,7 +115,7 @@ LockBoxDxeLibInitialize (

Status = LockBoxLibInitialize ();
if (!EFI_ERROR (Status)) {
- if (QemuFwCfgS3Enabled ()) {
+ if (PcdGetBool (PcdAcpiS3Enable)) {
//
// When S3 enabled, the first driver run with this library linked will
// have this library constructor to install LockBox protocol on the
--
2.31.1


[RESEND PATCH v3 3/4] OvmfPkg/PlatformBootManagerLib: use PcdAcpiS3Enable to detect S3 support

Lin, Gary (HPS OE-Linux)
 

To avoid the potential inconsistency between PcdAcpiS3Enable and
QemuFwCfgS3Enabled(), this commit modifies PlatformBootManagerLib to
detect S3 support by PcdAcpiS3Enable as modules in MdeModulePkg do.

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

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Jim Fehlig <jfehlig@suse.com>
Cc: Joey Li <jlee@suse.com>
Signed-off-by: Gary Lin <gary.lin@hpe.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
Tested-by: Jim Fehlig <jfehlig@suse.com>
---
v3:
- Add the bugzilla link
---
.../Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 1 +
OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index e470b9a6a3e5..c249a3cf1e35 100644
--- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -61,6 +61,7 @@ [Pcd]
gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable
gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
+ gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable
gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut
gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate ## CONSUMES
gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits ## CONSUMES
diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
index b0e97429372b..71f63b244828 100644
--- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
+++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
@@ -379,7 +379,7 @@ PlatformBootManagerBeforeConsole (
//
EfiEventGroupSignal (&gEfiEndOfDxeEventGroupGuid);

- if (QemuFwCfgS3Enabled ()) {
+ if (PcdGetBool (PcdAcpiS3Enable)) {
//
// Save the boot script too. Note that this will require us to emit the
// DxeSmmReadyToLock event just below, which in turn locks down SMM.
--
2.31.1


[RESEND PATCH v3 0/4] Fix OvmfXen boot failure due to s3 support state

Lin, Gary (HPS OE-Linux)
 

When using HVM Direct kernel boot with OvmfXen, it could fail at the
S3BootScript due to the inconsistency between QemuFwCfgS3Enabled()
and PcdAcpiS3Enable.

This patch series initializes PcdAcpiS3Enable in
. Besides, QemuFwCfgS3Enabled() is
replaced with PcdAcpiS3Enable in several OVMF libraries to avoid the
potential inconsistency.

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

v3:
- Update the description per Anthony's suggestion
- Add the bugzilla links
- Move the QemuKernelLoaderFsDxe patch out of this patch series
and make it an independent patch
v2:
- Amend the description and address "HVM Direct Kernel Boot"
- Add the comment for the conditional test of QemuFwCfgS3Enabled()
- Remove unused QemuFwCfgLib
- Update my email address

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Julien Grall <julien@xen.org>
Cc: Jim Fehlig <jfehlig@suse.com>
Cc: Joey Li <jlee@suse.com>

Gary Lin (4):
OvmfPkg/OvmfXen: set PcdAcpiS3Enable at initialization
OvmfPkg/LockBoxLib: use PcdAcpiS3Enable to detect S3 support
OvmfPkg/PlatformBootManagerLib: use PcdAcpiS3Enable to detect S3
support
OvmfPkg/SmmControl2Dxe: use PcdAcpiS3Enable to detect S3 support

OvmfPkg/Library/LockBoxLib/LockBoxDxeLib.inf | 3 +--
.../PlatformBootManagerLib.inf | 1 +
OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.inf | 2 ++
OvmfPkg/XenPlatformPei/XenPlatformPei.inf | 2 ++
OvmfPkg/Library/LockBoxLib/LockBoxDxe.c | 4 +---
.../Library/PlatformBootManagerLib/BdsPlatform.c | 2 +-
OvmfPkg/SmmControl2Dxe/SmmControl2Dxe.c | 4 +---
OvmfPkg/XenPlatformPei/Platform.c | 13 +++++++++++++
8 files changed, 22 insertions(+), 9 deletions(-)

--
2.31.1


[RESEND PATCH v3 1/4] OvmfPkg/OvmfXen: set PcdAcpiS3Enable at initialization

Lin, Gary (HPS OE-Linux)
 

There are several functions in OvmfPkg/Library using
QemuFwCfgS3Enabled() to detect the S3 support status. However, in
MdeModulePkg, PcdAcpiS3Enable is used to check S3 support. Since
InitializeXenPlatform() didn't set PcdAcpiS3Enable as
InitializePlatform() did, this made the inconsistency between
drivers/functions.

For example, S3SaveStateDxe checked PcdAcpiS3Enable and skipped
S3BootScript because the default value is FALSE. On the other hand,
PlatformBootManagerBeforeConsole() from OvmfPkg/Library called
QemuFwCfgS3Enabled() and found it returned TRUE, so it invoked
SaveS3BootScript(). However, S3SaveStateDxe skipped S3BootScript, so
SaveS3BootScript() asserted due to EFI_NOT_FOUND.

This issue mainly affects "HVM Direct Kernel Boot". When used,
"fw_cfg" is enabled in QEMU and QemuFwCfgS3Enabled() returns true in
that case.

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

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Julien Grall <julien@xen.org>
Cc: Jim Fehlig <jfehlig@suse.com>
Cc: Joey Li <jlee@suse.com>
Signed-off-by: Gary Lin <gary.lin@hpe.com>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
Tested-by: Jim Fehlig <jfehlig@suse.com>
---
v3:
- Update the description per Anthony's suggestion
- Add the bugzilla link
v2:
- Amend the description and address "HVM Direct Kernel Boot"
- Add the comment for the conditional test of QemuFwCfgS3Enabled()
- Remove unused QemuFwCfgLib
---
OvmfPkg/XenPlatformPei/XenPlatformPei.inf | 2 ++
OvmfPkg/XenPlatformPei/Platform.c | 13 +++++++++++++
2 files changed, 15 insertions(+)

diff --git a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
index 597cb6fcd7ff..20c27ff34b6c 100644
--- a/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
+++ b/OvmfPkg/XenPlatformPei/XenPlatformPei.inf
@@ -57,6 +57,7 @@ [LibraryClasses]
ResourcePublicationLib
PeiServicesLib
PeimEntryPoint
+ QemuFwCfgS3Lib
MtrrLib
MemEncryptSevLib
PcdLib
@@ -79,6 +80,7 @@ [Pcd]
gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Base
gUefiOvmfPkgTokenSpaceGuid.PcdPciMmio64Size
gUefiOvmfPkgTokenSpaceGuid.PcdQ35TsegMbytes
+ gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved
gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode
diff --git a/OvmfPkg/XenPlatformPei/Platform.c b/OvmfPkg/XenPlatformPei/Platform.c
index a811e72ee301..e60478fdb493 100644
--- a/OvmfPkg/XenPlatformPei/Platform.c
+++ b/OvmfPkg/XenPlatformPei/Platform.c
@@ -26,6 +26,7 @@
#include <Library/PciLib.h>
#include <Library/PeimEntryPoint.h>
#include <Library/PeiServicesLib.h>
+#include <Library/QemuFwCfgS3Lib.h>
#include <Library/ResourcePublicationLib.h>
#include <Guid/MemoryTypeInformation.h>
#include <Ppi/MasterBootMode.h>
@@ -423,6 +424,8 @@ InitializeXenPlatform (
IN CONST EFI_PEI_SERVICES **PeiServices
)
{
+ EFI_STATUS Status;
+
DEBUG ((DEBUG_INFO, "Platform PEIM Loaded\n"));

DebugDumpCmos ();
@@ -433,6 +436,16 @@ InitializeXenPlatform (
CpuDeadLoop ();
}

+ //
+ // This S3 conditional test is mainly for HVM Direct Kernel Boot since
+ // QEMU fwcfg isn't really supported other than that.
+ //
+ if (QemuFwCfgS3Enabled ()) {
+ DEBUG ((DEBUG_INFO, "S3 support was detected on QEMU\n"));
+ Status = PcdSetBoolS (PcdAcpiS3Enable, TRUE);
+ ASSERT_EFI_ERROR (Status);
+ }
+
XenConnect ();

BootModeInitialization ();
--
2.31.1


[RESEND PATCH v3] OvmfPkg/OvmfXen: add QemuKernelLoaderFsDxe

Lin, Gary (HPS OE-Linux)
 

Without QemuKernelLoaderFsDxe, QemuLoadKernelImage() couldn't download
the kernel, initrd, and kernel command line from QEMU's fw_cfg.

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

Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Jiewen Yao <jiewen.yao@intel.com>
cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Anthony Perard <anthony.perard@citrix.com>
Cc: Julien Grall <julien@xen.org>
Cc: Jim Fehlig <jfehlig@suse.com>
Cc: Joey Li <jlee@suse.com>
Signed-off-by: Gary Lin <gary.lin@hpe.com>
Acked-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
Tested-by: Jim Fehlig <jfehlig@suse.com>
---
v3:
Add the bugzilla link
---
OvmfPkg/OvmfXen.dsc | 1 +
OvmfPkg/OvmfXen.fdf | 1 +
2 files changed, 2 insertions(+)

diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc
index 3c1ca6bfd493..1a9c06c164a8 100644
--- a/OvmfPkg/OvmfXen.dsc
+++ b/OvmfPkg/OvmfXen.dsc
@@ -587,6 +587,7 @@ [Components]
NULL|OvmfPkg/Csm/LegacyBootMaintUiLib/LegacyBootMaintUiLib.inf
!endif
}
+ OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.inf
OvmfPkg/XenIoPvhDxe/XenIoPvhDxe.inf
OvmfPkg/XenIoPciDxe/XenIoPciDxe.inf
OvmfPkg/XenBusDxe/XenBusDxe.inf
diff --git a/OvmfPkg/OvmfXen.fdf b/OvmfPkg/OvmfXen.fdf
index aeb9336fd5b7..8b5823555937 100644
--- a/OvmfPkg/OvmfXen.fdf
+++ b/OvmfPkg/OvmfXen.fdf
@@ -324,6 +324,7 @@ [FV.DXEFV]
INF MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
INF MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
INF MdeModulePkg/Application/UiApp/UiApp.inf
+INF OvmfPkg/QemuKernelLoaderFsDxe/QemuKernelLoaderFsDxe.inf
INF MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
INF MdeModulePkg/Universal/PrintDxe/PrintDxe.inf
INF MdeModulePkg/Universal/Disk/DiskIoDxe/DiskIoDxe.inf
--
2.31.1


Event: TianoCore Bug Triage - APAC / NAMO - 08/31/2021 #cal-reminder

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

Reminder: TianoCore Bug Triage - APAC / NAMO

When:
08/31/2021
6:30pm to 7:30pm
(UTC-07: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


回复: [edk2-devel] [PATCH 2/2] .azurepipelines: Add UefiPayloadPkg in gate-build-job.yml and CISetting.py

gaoliming
 

Dun:
This PR also includes one additional commit to fix the build issue. Have you send the patch for it?

For this patch set, Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>. It will be merged after the build issue has been fixed.

Thanks
Liming

-----邮件原件-----
发件人: Tan, Dun <dun.tan@intel.com>
发送时间: 2021年8月30日 16:31
收件人: gaoliming <gaoliming@byosoft.com.cn>; devel@edk2.groups.io
抄送: 'Sean Brogan' <sean.brogan@microsoft.com>; 'Bret Barkelew'
<Bret.Barkelew@microsoft.com>; Kinney, Michael D
<michael.d.kinney@intel.com>
主题: RE: [edk2-devel] [PATCH 2/2] .azurepipelines: Add UefiPayloadPkg in
gate-build-job.yml and CISetting.py

Hi Liming,

Here is the link of the PR to verify the change in Tiano EDKII. It passed the CI
by GCC and VS both.
https://github.com/tianocore/edk2/pull/1931

Thanks,
Dun
-----Original Message-----
From: gaoliming <gaoliming@byosoft.com.cn>
Sent: Monday, August 30, 2021 11:26 AM
To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>
Cc: 'Sean Brogan' <sean.brogan@microsoft.com>; 'Bret Barkelew'
<Bret.Barkelew@microsoft.com>; Kinney, Michael D
<michael.d.kinney@intel.com>
Subject: 回复: [edk2-devel] [PATCH 2/2] .azurepipelines: Add UefiPayloadPkg
in gate-build-job.yml and CISetting.py

Dun:
I don't see the issues to enable UefiPayloadPkg in CI. Have you created the
private PR to verify this change?

Thanks
Liming
-----邮件原件-----
发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 duntan
发送时间: 2021年8月24日 17:24
收件人: devel@edk2.groups.io
抄送: Sean Brogan <sean.brogan@microsoft.com>; Bret Barkelew
<Bret.Barkelew@microsoft.com>; Kinney, Michael D
<michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>
主题: Re: [edk2-devel] [PATCH 2/2] .azurepipelines: Add UefiPayloadPkg
in gate-build-job.yml and CISetting.py

Hi all,
Since the CI for UefiPayloadPkg is important to our develop
progress, would you please speed up the review process? Thanks a lot!

Thanks,
Dun Tan
-----Original Message-----
From: Tan, Dun <dun.tan@intel.com>
Sent: Friday, August 20, 2021 2:44 PM
To: devel@edk2.groups.io
Cc: Sean Brogan <sean.brogan@microsoft.com>; Bret Barkelew
<Bret.Barkelew@microsoft.com>; Kinney, Michael D
<michael.d.kinney@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>;
Tan, Dun <dun.tan@intel.com>
Subject: [PATCH 2/2] .azurepipelines: Add UefiPayloadPkg in
gate-build-job.yml and CISetting.py

Add UefiPayloadPkg in gate-build-job.yml to enable Core ci for
UefiPayloadPkg.
Add UefiPayloadPkg to supported Packages in CISettings.

Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: DunTan <dun.tan@intel.com>
---
.azurepipelines/templates/pr-gate-build-job.yml | 3 +++
.pytool/CISettings.py | 3 ++-
2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/.azurepipelines/templates/pr-gate-build-job.yml
b/.azurepipelines/templates/pr-gate-build-job.yml
index 207acc7631..d5b16c127f 100644
--- a/.azurepipelines/templates/pr-gate-build-job.yml
+++ b/.azurepipelines/templates/pr-gate-build-job.yml
@@ -48,6 +48,9 @@ jobs:
TARGET_SECURITY:
Build.Pkgs: 'SecurityPkg'
Build.Targets: 'DEBUG,RELEASE,NO-TARGET'
+ TARGET_UEFIPAYLOAD:
+ Build.Pkgs: 'UefiPayloadPkg'
+ Build.Targets: 'DEBUG,RELEASE,NO-TARGET'
TARGET_PLATFORMS:
# For Platforms only check code. Leave it to Platform CI
# to build them.
diff --git a/.pytool/CISettings.py b/.pytool/CISettings.py index
96e6baa519..ce330e2c73 100644
--- a/.pytool/CISettings.py
+++ b/.pytool/CISettings.py
@@ -67,7 +67,8 @@ class Settings(CiBuildSettingsManager,
UpdateSettingsManager, SetupSettingsManag
"CryptoPkg",
"UnitTestFrameworkPkg",
"OvmfPkg",
- "RedfishPkg"
+ "RedfishPkg",
+ "UefiPayloadPkg"
)

def GetArchitecturesSupported(self):
--
2.31.1.windows.1





Re: [PATCH 1/1] MdeModulePkg: Fix typo of "memory" in RamDiskDxe debug message

Wu, Hao A
 

-----Original Message-----
From: Rebecca Cran <rebecca@bsdio.com>
Sent: Tuesday, August 31, 2021 6:37 AM
To: Wang, Jian J <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>
Cc: Rebecca Cran <rebecca@bsdio.com>; devel@edk2.groups.io
Subject: [PATCH 1/1] MdeModulePkg: Fix typo of "memory" in RamDiskDxe
debug message

Fix a typo of "memory" in a debug message in RamDiskProtocol.c.

Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
---
MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c
b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c
index 4333e000539b..a45a55c82306 100644
--- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c
+++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c
@@ -195,7 +195,7 @@ RamDiskPublishNfit (
MemoryFound = TRUE;
DEBUG ((
EFI_D_INFO,
- "RamDiskPublishNfit: RAM disk with reserved meomry type, will publish to
NFIT.\n"
+ "RamDiskPublishNfit: RAM disk with reserved memory type, will publish to
NFIT.\n"

Thanks.
Reviewed-by: Hao A Wu <hao.a.wu@intel.com>

Best Regards,
Hao Wu


));
break;
}
--
2.31.1


[PATCH 1/1] MdeModulePkg: Fix typo of "memory" in RamDiskDxe debug message

Rebecca Cran
 

Fix a typo of "memory" in a debug message in RamDiskProtocol.c.

Signed-off-by: Rebecca Cran <rebecca@bsdio.com>
---
MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c
index 4333e000539b..a45a55c82306 100644
--- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c
+++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskProtocol.c
@@ -195,7 +195,7 @@ RamDiskPublishNfit (
MemoryFound = TRUE;
DEBUG ((
EFI_D_INFO,
- "RamDiskPublishNfit: RAM disk with reserved meomry type, will publish to NFIT.\n"
+ "RamDiskPublishNfit: RAM disk with reserved memory type, will publish to NFIT.\n"
));
break;
}
--
2.31.1


Re: OVMF: NV Variable Store Layout of Larger Build Targets

Andrew Fish
 



On Aug 30, 2021, at 10:52 AM, Devon Bautista <dbautista@...> wrote:

Hi Gerd,

The current maximum image size of an OVMF image is 4MB, which is
insufficient for storing even a minimal and compressed kernel and initramfs.
To get around this, we've been maintaining our own fork of EDK2 that adds
8MiB and 16MiB OVMF build targets that have enough room in the DXE volume to
store a reasonably-sized kernel and initramfs. However, it would be
convenient if upstream EDK2 supported these larger OVMF targets.
I'm wondering whenever it makes sense to have the 8M option.  I think
I'd tend to go straight to 16M (which is the max size we can do on x86).

On the Linuxboot side, we really only need 16MiB. However, I think Laszlo justified an 8MiB target because the QEMU commit he pointed to (referenced in my initial post) increased the absolute firmware size limit to be 16MiB when setting the maximum (`pcms->max_fw_size`) in `pc_machine_set_max_fw_size()`, but the default maximum if not set is 8MiB.

So I understand why an 8MiB target is justified, but, like you, I am not sure if it's really needed.

However, as Laszlo mentioned, introducing a larger volume size is
compatibility breaking, and so seizing the opportunity to come up
with a larger non-volatile variable store layout is necessary.

That said, I would like to use this thread to discuss among hardware
vendors an optimal variable store layout for these larger image sizes.
The 2M -> 4M switch happened because the varstore was too small.  It was
Confirm64KilobytesOfUnauthenticatedVariableStorage test of the the
Microsoft Hardware Certification failing.  I guess Microsoft has good
reasons to test for 64k varstore, probably they expect this is big
enough in practice.

The varstore size of the 4M layout is *way* above that (see 2M -> 4M
commit message):

  Variable store      56 ->   256 ( +200)
  Spare area          64 ->   264 ( +200)

Assuming 256k varstore is more than enough:  Sticking to the 4M variable
store layout for the 16M (and maybe 8M) builds looks like the best
option to me.  I think the varstore would be identical for 4M and 16M
builds then, so it should be possible to switch guests from 4M to 16M
while keeping the varstore.

Keeping the 4MiB varstore layout would be the most compatible and straightforward option and is what I would want to go with.

But I also think that it might make sense when introducing a considerably larger build target to account for any possible increases in variable store size that vendors may expect in the future. I for one dismay any further size increase, but I suppose the more relevant question is, is 256KiB of varstore enough for vendors?

I’m also in the 16 MiB camp and I’m OK with the 256KiB varstore. 

Thanks,

Andrew Fish

-- 
Best,
Devon


Re: OVMF: NV Variable Store Layout of Larger Build Targets

Devon Bautista
 

Hi Gerd,

The current maximum image size of an OVMF image is 4MB, which is
insufficient for storing even a minimal and compressed kernel and initramfs.
To get around this, we've been maintaining our own fork of EDK2 that adds
8MiB and 16MiB OVMF build targets that have enough room in the DXE volume to
store a reasonably-sized kernel and initramfs. However, it would be
convenient if upstream EDK2 supported these larger OVMF targets.
I'm wondering whenever it makes sense to have the 8M option.  I think
I'd tend to go straight to 16M (which is the max size we can do on x86).

On the Linuxboot side, we really only need 16MiB. However, I think Laszlo justified an 8MiB target because the QEMU commit he pointed to (referenced in my initial post) increased the absolute firmware size limit to be 16MiB when setting the maximum (`pcms->max_fw_size`) in `pc_machine_set_max_fw_size()`, but the default maximum if not set is 8MiB.

So I understand why an 8MiB target is justified, but, like you, I am not sure if it's really needed.

However, as Laszlo mentioned, introducing a larger volume size is
compatibility breaking, and so seizing the opportunity to come up
with a larger non-volatile variable store layout is necessary.

That said, I would like to use this thread to discuss among hardware
vendors an optimal variable store layout for these larger image sizes.
The 2M -> 4M switch happened because the varstore was too small.  It was
Confirm64KilobytesOfUnauthenticatedVariableStorage test of the the
Microsoft Hardware Certification failing.  I guess Microsoft has good
reasons to test for 64k varstore, probably they expect this is big
enough in practice.

The varstore size of the 4M layout is *way* above that (see 2M -> 4M
commit message):

  Variable store      56 ->   256 ( +200)
  Spare area          64 ->   264 ( +200)

Assuming 256k varstore is more than enough:  Sticking to the 4M variable
store layout for the 16M (and maybe 8M) builds looks like the best
option to me.  I think the varstore would be identical for 4M and 16M
builds then, so it should be possible to switch guests from 4M to 16M
while keeping the varstore.

Keeping the 4MiB varstore layout would be the most compatible and straightforward option and is what I would want to go with.

But I also think that it might make sense when introducing a considerably larger build target to account for any possible increases in variable store size that vendors may expect in the future. I for one dismay any further size increase, but I suppose the more relevant question is, is 256KiB of varstore enough for vendors?

--
Best,
Devon


Re: [edk2-platforms PATCH v3 1/1] Maintainers.txt: Add maintainer of Ext4Pkg

Michael D Kinney
 

Reviewed-by: Michael D Kinney <michael.d.kinney@intel.com>

Mike

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Pedro Falcato
Sent: Thursday, August 26, 2021 5:54 AM
To: devel@edk2.groups.io
Cc: Leif Lindholm <leif@nuviainc.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: [edk2-devel] [edk2-platforms PATCH v3 1/1] Maintainers.txt: Add maintainer of Ext4Pkg

Add myself as a maintainter for Features/Ext4Pkg.

Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com>
---
Maintainers.txt | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/Maintainers.txt b/Maintainers.txt
index 9b8d6aead923..e5ddaa8d2284 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -141,6 +141,10 @@ M: Leif Lindholm <leif@nuviainc.com>
R: Ard Biesheuvel <ardb+tianocore@kernel.org>

R: Wenyi Xie <xiewenyi2@huawei.com>



+Features/Ext4Pkg

+F: Features/Ext4Pkg/

+M: Pedro Falcato <pedro.falcato@gmail.com>

+

Features/Intel

F: Features/Intel/

M: Sai Chaganty <rangasai.v.chaganty@intel.com>

--
2.33.0



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79844): https://edk2.groups.io/g/devel/message/79844
Mute This Topic: https://groups.io/mt/85160135/1643496
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kinney@intel.com]
-=-=-=-=-=-=


Re: mmio mappings for runtime service

Michael D Kinney
 

Hi Gerd,

The following library provides an example of all the steps to register an MMIO range for RT access.

https://github.com/tianocore/edk2/blob/master/MdePkg/Library/DxeRuntimePciExpressLib/PciExpressLib.c

The example is from PCI Express, but the PCI Config space for PCI Express is an MMIO window. This
library contains a Constructor/Destructor to create/close a Set Virtual Address Map event. It has the
Set Virtual Address Map Event Notification Function and a function to request a specific 4KB
portion of the 256 MB MMIO window to be marked as RT.

DxeRuntimePciExpressLibConstructor()
DxeRuntimePciExpressLibDestructor()
DxeRuntimePciExpressLibVirtualNotify()
PciExpressRegisterForRuntimeAccess()

Best regards,

Mike

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen
Sent: Monday, August 30, 2021 6:02 AM
To: devel@edk2.groups.io; kraxel@redhat.com
Subject: Re: [edk2-devel] mmio mappings for runtime service

The BIOS driver must set EFI_RUNTIME_MEMORY attribute for the EfiGcdMemoryTypeMemoryMappedIo region with GCD service gDS-
SetMemorySpaceAttributes().
Then DXE will report EfiMemoryMappedIO with EFI_RUNTIME_MEMORY attribute in UEFI memory map.
The OS will gBS->GetMemoryMap() and assign virtual address for the MMIO, and gRT->SetVirtualAddressMap() back to the BIOS.

Finally, the BIOS driver can gRT->ConvertPointer() the MMIO physical address to virtual address, then access it at
runtime.

Thank you
Yao Jiewen

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd
Hoffmann
Sent: Monday, August 30, 2021 6:24 PM
To: devel@edk2.groups.io
Subject: [edk2-devel] mmio mappings for runtime service

Hi,

What is the correct way to make sure runtime services can access
mmio registers, i.e. that there is a mapping in the page tables
for the mmio page needed?

Is that the job of the firmware?
Or should the OS calling the runtime service handle that?
In case of the latter: How does the OS figure which pages are needed?

thanks,
Gerd








Re: [PATCH v3] ArmPkg: Enable boot discovery policy for ARM package.

Ard Biesheuvel
 

On Mon, 30 Aug 2021 at 14:08, Grzegorz Bernacki <gjb@semihalf.com> wrote:

This commit adds code which check BootDiscoveryPolicy variable and
calls Boot Policy Manager Protocol to connect device specified by
the variable. To enable that mechanism for platform
EfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy PCD must be
added to DSC file and BootDiscoveryPolicyUiLib should be added to
UiApp libraries.

Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com>
Reviewed-by: Sunny Wang <sunny.wang@arm.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
Merged as #1932

Thanks all,

---

Notes:
v2:
- fix error from CI

v3:
- use local variable DiscoveryPolicy to initialize BootDiscoveryPolicy
- coding style fixes
- run EfiBootManagerRefreshAllBootOption () only if BootDiscoveryPolicy

ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 5 +
ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 118 +++++++++++++++++++-
2 files changed, 122 insertions(+), 1 deletion(-)

diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index 353d7a967b..86751b45f8 100644
--- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -65,11 +65,15 @@

[Pcd]
gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut
+ gEfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy

[Guids]
+ gBootDiscoveryPolicyMgrFormsetGuid
gEdkiiNonDiscoverableEhciDeviceGuid
gEdkiiNonDiscoverableUhciDeviceGuid
gEdkiiNonDiscoverableXhciDeviceGuid
+ gEfiBootManagerPolicyNetworkGuid
+ gEfiBootManagerPolicyConnectAllGuid
gEfiFileInfoGuid
gEfiFileSystemInfoGuid
gEfiFileSystemVolumeLabelInfoIdGuid
@@ -79,6 +83,7 @@

[Protocols]
gEdkiiNonDiscoverableDeviceProtocolGuid
+ gEfiBootManagerPolicyProtocolGuid
gEfiDevicePathProtocolGuid
gEfiGraphicsOutputProtocolGuid
gEfiLoadedImageProtocolGuid
diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
index 5ceb23d822..1e4020487a 100644
--- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
+++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
@@ -2,9 +2,10 @@
Implementation for PlatformBootManagerLib library class interfaces.

Copyright (C) 2015-2016, Red Hat, Inc.
- Copyright (c) 2014 - 2019, ARM Ltd. All rights reserved.<BR>
+ Copyright (c) 2014 - 2021, ARM Ltd. All rights reserved.<BR>
Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.<BR>
Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
+ Copyright (c) 2021, Semihalf All rights reserved.<BR>

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

@@ -19,6 +20,7 @@
#include <Library/UefiBootManagerLib.h>
#include <Library/UefiLib.h>
#include <Library/UefiRuntimeServicesTableLib.h>
+#include <Protocol/BootManagerPolicy.h>
#include <Protocol/DevicePath.h>
#include <Protocol/EsrtManagement.h>
#include <Protocol/GraphicsOutput.h>
@@ -27,6 +29,7 @@
#include <Protocol/PciIo.h>
#include <Protocol/PciRootBridgeIo.h>
#include <Protocol/PlatformBootManager.h>
+#include <Guid/BootDiscoveryPolicy.h>
#include <Guid/EventGroup.h>
#include <Guid/NonDiscoverableDevice.h>
#include <Guid/TtyTerm.h>
@@ -703,6 +706,113 @@ HandleCapsules (

#define VERSION_STRING_PREFIX L"Tianocore/EDK2 firmware version "

+/**
+ This functions checks the value of BootDiscoverPolicy variable and
+ connect devices of class specified by that variable. Then it refreshes
+ Boot order for newly discovered boot device.
+
+ @retval EFI_SUCCESS Devices connected successfully or connection
+ not required.
+ @retval others Return values from GetVariable(), LocateProtocol()
+ and ConnectDeviceClass().
+**/
+STATIC
+EFI_STATUS
+BootDiscoveryPolicyHandler (
+ VOID
+ )
+{
+ EFI_STATUS Status;
+ UINT32 DiscoveryPolicy;
+ UINT32 DiscoveryPolicyOld;
+ UINTN Size;
+ EFI_BOOT_MANAGER_POLICY_PROTOCOL *BMPolicy;
+ EFI_GUID *Class;
+
+ Size = sizeof (DiscoveryPolicy);
+ Status = gRT->GetVariable (
+ BOOT_DISCOVERY_POLICY_VAR,
+ &gBootDiscoveryPolicyMgrFormsetGuid,
+ NULL,
+ &Size,
+ &DiscoveryPolicy
+ );
+ if (Status == EFI_NOT_FOUND) {
+ DiscoveryPolicy = PcdGet32 (PcdBootDiscoveryPolicy);
+ Status = PcdSet32S (PcdBootDiscoveryPolicy, DiscoveryPolicy);
+ if (Status == EFI_NOT_FOUND) {
+ return EFI_SUCCESS;
+ } else if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ } else if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ if (DiscoveryPolicy == BDP_CONNECT_MINIMAL) {
+ return EFI_SUCCESS;
+ }
+
+ switch (DiscoveryPolicy) {
+ case BDP_CONNECT_NET:
+ Class = &gEfiBootManagerPolicyNetworkGuid;
+ break;
+ case BDP_CONNECT_ALL:
+ Class = &gEfiBootManagerPolicyConnectAllGuid;
+ break;
+ default:
+ DEBUG ((
+ DEBUG_INFO,
+ "%a - Unexpected DiscoveryPolicy (0x%x). Run Minimal Discovery Policy\n",
+ __FUNCTION__,
+ DiscoveryPolicy
+ ));
+ return EFI_SUCCESS;
+ }
+
+ Status = gBS->LocateProtocol (
+ &gEfiBootManagerPolicyProtocolGuid,
+ NULL,
+ (VOID **)&BMPolicy
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_INFO, "%a - Failed to locate gEfiBootManagerPolicyProtocolGuid."
+ "Driver connect will be skipped.\n", __FUNCTION__));
+ return Status;
+ }
+
+ Status = BMPolicy->ConnectDeviceClass (BMPolicy, Class);
+ if (EFI_ERROR (Status)){
+ DEBUG ((DEBUG_ERROR, "%a - ConnectDeviceClass returns - %r\n", __FUNCTION__, Status));
+ return Status;
+ }
+
+ //
+ // Refresh Boot Options if Boot Discovery Policy has been changed
+ //
+ Size = sizeof (DiscoveryPolicyOld);
+ Status = gRT->GetVariable (
+ BOOT_DISCOVERY_POLICY_OLD_VAR,
+ &gBootDiscoveryPolicyMgrFormsetGuid,
+ NULL,
+ &Size,
+ &DiscoveryPolicyOld
+ );
+ if ((Status == EFI_NOT_FOUND) || (DiscoveryPolicyOld != DiscoveryPolicy)) {
+ EfiBootManagerRefreshAllBootOption ();
+
+ Status = gRT->SetVariable (
+ BOOT_DISCOVERY_POLICY_OLD_VAR,
+ &gBootDiscoveryPolicyMgrFormsetGuid,
+ EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS,
+ sizeof (DiscoveryPolicyOld),
+ &DiscoveryPolicy
+ );
+ }
+
+ return EFI_SUCCESS;
+}
+
/**
Do the platform specific action after the console is ready
Possible things that can be done in PlatformBootManagerAfterConsole:
@@ -753,6 +863,12 @@ PlatformBootManagerAfterConsole (
}
}

+ //
+ // Connect device specified by BootDiscoverPolicy variable and
+ // refresh Boot order for newly discovered boot devices
+ //
+ BootDiscoveryPolicyHandler ();
+
//
// On ARM, there is currently no reason to use the phased capsule
// update approach where some capsules are dispatched before EndOfDxe
--
2.25.1


Re: [PATCH v2 0/2] BaseTools: Switch to downloading the ARM/AARCH64 compiler from Arm's site

Ard Biesheuvel
 

On Mon, 30 Aug 2021 at 07:15, Rebecca Cran <rebecca@bsdio.com> wrote:

Linaro no longer do gcc releases - Arm creates them now.

Update the gcc_[arm,aarch64]_linux_ext_dep.yaml files in BaseTools/Bin to
switch from Linaro's old release to the latest gcc 10.3-2021.07 release
from Arm and fix LinuxGcc5ToolChain.py with the new gcc prefix.


Rebecca Cran (2):
BaseTools: Switch to downloading the ARM compiler from Arm's site
BaseTools: Switch to downloading the AARCH64 compiler from Arm's site
For the series,


Acked-by: Ard Biesheuvel <ardb@kernel.org>

BaseTools/Bin/gcc_aarch64_linux_ext_dep.yaml | 10 +++++-----
BaseTools/Bin/gcc_arm_linux_ext_dep.yaml | 10 +++++-----
BaseTools/Plugin/LinuxGcc5ToolChain/LinuxGcc5ToolChain.py | 4 ++--
3 files changed, 12 insertions(+), 12 deletions(-)

--
2.31.1


Re: mmio mappings for runtime service

Yao, Jiewen
 

The BIOS driver must set EFI_RUNTIME_MEMORY attribute for the EfiGcdMemoryTypeMemoryMappedIo region with GCD service gDS->SetMemorySpaceAttributes().
Then DXE will report EfiMemoryMappedIO with EFI_RUNTIME_MEMORY attribute in UEFI memory map.
The OS will gBS->GetMemoryMap() and assign virtual address for the MMIO, and gRT->SetVirtualAddressMap() back to the BIOS.

Finally, the BIOS driver can gRT->ConvertPointer() the MMIO physical address to virtual address, then access it at runtime.

Thank you
Yao Jiewen

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd
Hoffmann
Sent: Monday, August 30, 2021 6:24 PM
To: devel@edk2.groups.io
Subject: [edk2-devel] mmio mappings for runtime service

Hi,

What is the correct way to make sure runtime services can access
mmio registers, i.e. that there is a mapping in the page tables
for the mmio page needed?

Is that the job of the firmware?
Or should the OS calling the runtime service handle that?
In case of the latter: How does the OS figure which pages are needed?

thanks,
Gerd





Re: [PATCH v8 02/11] SecurityPkg: Create library for enrolling Secure Boot variables.

Grzegorz Bernacki
 

Hi Patrick,

Current implementation does not allow to use data in
EFI_VARIABLE_AUTHENTICATION_2 format as a source of default data. I
will add the possibility to use that kind of data to initialize secure
boot default data.
thanks,
greg

wt., 24 sie 2021 o 14:26 Grzegorz Bernacki <gjb@semihalf.com> napisał(a):


Hi Patrick,

Yes, I tested the dbx enrollment, but with my own data. Please let me
try that dbx.

thanks,
greg

wt., 24 sie 2021 o 14:22 Patrick Rudolph
<patrick.rudolph@9elements.com> napisał(a):

Hi Grzegorz,
I tried this patch, but I cannot enroll the DBX downloaded from here:
https://uefi.org/revocationlistfile

Is it even possible with current code? Did you test DBX enrollment as well using the revocation list file?

Regards,
Patrick

On Mon, Aug 2, 2021 at 12:47 PM Grzegorz Bernacki <gjb@semihalf.com> wrote:

This commits add library, which consist functions to
enrolll Secure Boot keys and initialize Secure Boot
default variables. Some of the functions was moved
from SecureBootConfigImpl.c file.

Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com>
Reviewed-by: Sunny Wang <sunny.wang@arm.com>
Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
---
SecurityPkg/SecurityPkg.dec | 4 +
SecurityPkg/SecurityPkg.dsc | 1 +
SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.inf | 80 ++++
SecurityPkg/Include/Library/SecureBootVariableProvisionLib.h | 134 ++++++
SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.c | 482 ++++++++++++++++++++
SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.uni | 16 +
6 files changed, 717 insertions(+)
create mode 100644 SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.inf
create mode 100644 SecurityPkg/Include/Library/SecureBootVariableProvisionLib.h
create mode 100644 SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.c
create mode 100644 SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.uni

diff --git a/SecurityPkg/SecurityPkg.dec b/SecurityPkg/SecurityPkg.dec
index 8f3710e59f..e30c39f321 100644
--- a/SecurityPkg/SecurityPkg.dec
+++ b/SecurityPkg/SecurityPkg.dec
@@ -91,6 +91,10 @@
## @libraryclass Provides helper functions related to creation/removal Secure Boot variables.
#
SecureBootVariableLib|Include/Library/SecureBootVariableLib.h
+
+ ## @libraryclass Provides support to enroll Secure Boot keys.
+ #
+ SecureBootVariableProvisionLib|Include/Library/SecureBootVariableProvisionLib.h
[Guids]
## Security package token space guid.
# Include/Guid/SecurityPkgTokenSpace.h
diff --git a/SecurityPkg/SecurityPkg.dsc b/SecurityPkg/SecurityPkg.dsc
index 854f250625..99c227dad2 100644
--- a/SecurityPkg/SecurityPkg.dsc
+++ b/SecurityPkg/SecurityPkg.dsc
@@ -71,6 +71,7 @@
TcgEventLogRecordLib|SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLogRecordLib.inf
MmUnblockMemoryLib|MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf
SecureBootVariableLib|SecurityPkg/Library/SecureBootVariableLib/SecureBootVariableLib.inf
+ SecureBootVariableProvisionLib|SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.inf

[LibraryClasses.ARM]
#
diff --git a/SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.inf b/SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.inf
new file mode 100644
index 0000000000..a09abd29ce
--- /dev/null
+++ b/SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.inf
@@ -0,0 +1,80 @@
+## @file
+# Provides initialization of Secure Boot keys and databases.
+#
+# Copyright (c) 2021, ARM Ltd. All rights reserved.<BR>
+# Copyright (c) 2021, Semihalf All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x00010005
+ BASE_NAME = SecureBootVariableLib
+ MODULE_UNI_FILE = SecureBootVariableLib.uni
+ FILE_GUID = 18192DD0-9430-45F1-80C7-5C52061CD183
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = SecureBootVariableProvisionLib|DXE_DRIVER DXE_RUNTIME_DRIVER UEFI_APPLICATION
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+# VALID_ARCHITECTURES = IA32 X64 AARCH64
+#
+
+[Sources]
+ SecureBootVariableProvisionLib.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ SecurityPkg/SecurityPkg.dec
+ CryptoPkg/CryptoPkg.dec
+
+[LibraryClasses]
+ BaseLib
+ BaseMemoryLib
+ DebugLib
+ MemoryAllocationLib
+ BaseCryptLib
+ DxeServicesLib
+ SecureBootVariableLib
+
+[Guids]
+ ## CONSUMES ## Variable:L"SetupMode"
+ ## PRODUCES ## Variable:L"SetupMode"
+ ## CONSUMES ## Variable:L"SecureBoot"
+ ## PRODUCES ## Variable:L"SecureBoot"
+ ## PRODUCES ## Variable:L"PK"
+ ## PRODUCES ## Variable:L"KEK"
+ ## CONSUMES ## Variable:L"PKDefault"
+ ## CONSUMES ## Variable:L"KEKDefault"
+ ## CONSUMES ## Variable:L"dbDefault"
+ ## CONSUMES ## Variable:L"dbxDefault"
+ ## CONSUMES ## Variable:L"dbtDefault"
+ gEfiGlobalVariableGuid
+
+ ## SOMETIMES_CONSUMES ## Variable:L"DB"
+ ## SOMETIMES_CONSUMES ## Variable:L"DBX"
+ ## SOMETIMES_CONSUMES ## Variable:L"DBT"
+ gEfiImageSecurityDatabaseGuid
+
+ ## CONSUMES ## Variable:L"SecureBootEnable"
+ ## PRODUCES ## Variable:L"SecureBootEnable"
+ gEfiSecureBootEnableDisableGuid
+
+ ## CONSUMES ## Variable:L"CustomMode"
+ ## PRODUCES ## Variable:L"CustomMode"
+ gEfiCustomModeEnableGuid
+
+ gEfiCertTypeRsa2048Sha256Guid ## CONSUMES
+ gEfiCertX509Guid ## CONSUMES
+ gEfiCertPkcs7Guid ## CONSUMES
+
+ gDefaultPKFileGuid
+ gDefaultKEKFileGuid
+ gDefaultdbFileGuid
+ gDefaultdbxFileGuid
+ gDefaultdbtFileGuid
+
diff --git a/SecurityPkg/Include/Library/SecureBootVariableProvisionLib.h b/SecurityPkg/Include/Library/SecureBootVariableProvisionLib.h
new file mode 100644
index 0000000000..ba8009b5cd
--- /dev/null
+++ b/SecurityPkg/Include/Library/SecureBootVariableProvisionLib.h
@@ -0,0 +1,134 @@
+/** @file
+ Provides a functions to enroll keys based on default values.
+
+Copyright (c) 2011 - 2018, Intel Corporation. All rights reserved.<BR>
+(C) Copyright 2018 Hewlett Packard Enterprise Development LP<BR>
+Copyright (c) 2021, ARM Ltd. All rights reserved.<BR>
+Copyright (c) 2021, Semihalf All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef SECURE_BOOT_VARIABLE_PROVISION_LIB_H_
+#define SECURE_BOOT_VARIABLE_PROVISION_LIB_H_
+
+/**
+ Sets the content of the 'db' variable based on 'dbDefault' variable content.
+
+ @retval EFI_OUT_OF_RESOURCES If memory allocation for EFI_VARIABLE_AUTHENTICATION_2 fails
+ while VendorGuid is NULL.
+ @retval other Errors from GetVariable2(), GetTime() and SetVariable()
+--*/
+EFI_STATUS
+EFIAPI
+EnrollDbFromDefault (
+ VOID
+);
+
+/**
+ Sets the content of the 'dbx' variable based on 'dbxDefault' variable content.
+
+ @retval EFI_OUT_OF_RESOURCES If memory allocation for EFI_VARIABLE_AUTHENTICATION_2 fails
+ while VendorGuid is NULL.
+ @retval other Errors from GetVariable2(), GetTime() and SetVariable()
+--*/
+EFI_STATUS
+EFIAPI
+EnrollDbxFromDefault (
+ VOID
+);
+
+/**
+ Sets the content of the 'dbt' variable based on 'dbtDefault' variable content.
+
+ @retval EFI_OUT_OF_RESOURCES If memory allocation for EFI_VARIABLE_AUTHENTICATION_2 fails
+ while VendorGuid is NULL.
+ @retval other Errors from GetVariable2(), GetTime() and SetVariable()
+--*/
+EFI_STATUS
+EFIAPI
+EnrollDbtFromDefault (
+ VOID
+);
+
+/**
+ Sets the content of the 'KEK' variable based on 'KEKDefault' variable content.
+
+ @retval EFI_OUT_OF_RESOURCES If memory allocation for EFI_VARIABLE_AUTHENTICATION_2 fails
+ while VendorGuid is NULL.
+ @retval other Errors from GetVariable2(), GetTime() and SetVariable()
+--*/
+EFI_STATUS
+EFIAPI
+EnrollKEKFromDefault (
+ VOID
+);
+
+/**
+ Sets the content of the 'PK' variable based on 'PKDefault' variable content.
+
+ @retval EFI_OUT_OF_RESOURCES If memory allocation for EFI_VARIABLE_AUTHENTICATION_2 fails
+ while VendorGuid is NULL.
+ @retval other Errors from GetVariable2(), GetTime() and SetVariable()
+--*/
+EFI_STATUS
+EFIAPI
+EnrollPKFromDefault (
+ VOID
+);
+
+/**
+ Initializes PKDefault variable with data from FFS section.
+
+ @retval EFI_SUCCESS Variable was initialized successfully.
+ @retval EFI_UNSUPPORTED Variable already exists.
+--*/
+EFI_STATUS
+SecureBootInitPKDefault (
+ IN VOID
+ );
+
+/**
+ Initializes KEKDefault variable with data from FFS section.
+
+ @retval EFI_SUCCESS Variable was initialized successfully.
+ @retval EFI_UNSUPPORTED Variable already exists.
+--*/
+EFI_STATUS
+SecureBootInitKEKDefault (
+ IN VOID
+ );
+
+/**
+ Initializes dbDefault variable with data from FFS section.
+
+ @retval EFI_SUCCESS Variable was initialized successfully.
+ @retval EFI_UNSUPPORTED Variable already exists.
+--*/
+EFI_STATUS
+SecureBootInitDbDefault (
+ IN VOID
+ );
+
+/**
+ Initializes dbtDefault variable with data from FFS section.
+
+ @retval EFI_SUCCESS Variable was initialized successfully.
+ @retval EFI_UNSUPPORTED Variable already exists.
+--*/
+EFI_STATUS
+SecureBootInitDbtDefault (
+ IN VOID
+ );
+
+/**
+ Initializes dbxDefault variable with data from FFS section.
+
+ @retval EFI_SUCCESS Variable was initialized successfully.
+ @retval EFI_UNSUPPORTED Variable already exists.
+--*/
+EFI_STATUS
+SecureBootInitDbxDefault (
+ IN VOID
+ );
+#endif
diff --git a/SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.c b/SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.c
new file mode 100644
index 0000000000..848f7ce929
--- /dev/null
+++ b/SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.c
@@ -0,0 +1,482 @@
+/** @file
+ This library provides functions to set/clear Secure Boot
+ keys and databases.
+
+ Copyright (c) 2011 - 2018, Intel Corporation. All rights reserved.<BR>
+ (C) Copyright 2018 Hewlett Packard Enterprise Development LP<BR>
+ Copyright (c) 2021, ARM Ltd. All rights reserved.<BR>
+ Copyright (c) 2021, Semihalf All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+#include <Guid/GlobalVariable.h>
+#include <Guid/AuthenticatedVariableFormat.h>
+#include <Guid/ImageAuthentication.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/UefiLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/UefiRuntimeServicesTableLib.h>
+#include <Library/SecureBootVariableLib.h>
+#include <Library/SecureBootVariableProvisionLib.h>
+
+/**
+ Enroll a key/certificate based on a default variable.
+
+ @param[in] VariableName The name of the key/database.
+ @param[in] DefaultName The name of the default variable.
+ @param[in] VendorGuid The namespace (ie. vendor GUID) of the variable
+
+ @retval EFI_OUT_OF_RESOURCES Out of memory while allocating AuthHeader.
+ @retval EFI_SUCCESS Successful enrollment.
+ @return Error codes from GetTime () and SetVariable ().
+**/
+STATIC
+EFI_STATUS
+EnrollFromDefault (
+ IN CHAR16 *VariableName,
+ IN CHAR16 *DefaultName,
+ IN EFI_GUID *VendorGuid
+ )
+{
+ VOID *Data;
+ UINTN DataSize;
+ EFI_STATUS Status;
+
+ Status = EFI_SUCCESS;
+
+ DataSize = 0;
+ Status = GetVariable2 (DefaultName, &gEfiGlobalVariableGuid, &Data, &DataSize);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "error: GetVariable (\"%s): %r\n", DefaultName, Status));
+ return Status;
+ }
+
+ CreateTimeBasedPayload (&DataSize, (UINT8 **)&Data);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Fail to create time-based data payload: %r", Status));
+ return Status;
+ }
+
+ //
+ // Allocate memory for auth variable
+ //
+ Status = gRT->SetVariable (
+ VariableName,
+ VendorGuid,
+ (EFI_VARIABLE_NON_VOLATILE |
+ EFI_VARIABLE_BOOTSERVICE_ACCESS |
+ EFI_VARIABLE_RUNTIME_ACCESS |
+ EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS),
+ DataSize,
+ Data
+ );
+
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "error: %a (\"%s\", %g): %r\n", __FUNCTION__, VariableName,
+ VendorGuid, Status));
+ }
+
+ if (Data != NULL) {
+ FreePool (Data);
+ }
+
+ return Status;
+}
+
+/** Initializes PKDefault variable with data from FFS section.
+
+ @retval EFI_SUCCESS Variable was initialized successfully.
+ @retval EFI_UNSUPPORTED Variable already exists.
+**/
+EFI_STATUS
+SecureBootInitPKDefault (
+ IN VOID
+ )
+{
+ EFI_SIGNATURE_LIST *EfiSig;
+ UINTN SigListsSize;
+ EFI_STATUS Status;
+ UINT8 *Data;
+ UINTN DataSize;
+
+ //
+ // Check if variable exists, if so do not change it
+ //
+ Status = GetVariable2 (EFI_PK_DEFAULT_VARIABLE_NAME, &gEfiGlobalVariableGuid, (VOID **) &Data, &DataSize);
+ if (Status == EFI_SUCCESS) {
+ DEBUG ((DEBUG_INFO, "Variable %s exists. Old value is preserved\n", EFI_PK_DEFAULT_VARIABLE_NAME));
+ FreePool (Data);
+ return EFI_UNSUPPORTED;
+ }
+
+ if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) {
+ return Status;
+ }
+
+ //
+ // Variable does not exist, can be initialized
+ //
+ DEBUG ((DEBUG_INFO, "Variable %s does not exist.\n", EFI_PK_DEFAULT_VARIABLE_NAME));
+
+ Status = SecureBootFetchData (&gDefaultPKFileGuid, &SigListsSize, &EfiSig);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_INFO, "Content for %s not found\n", EFI_PK_DEFAULT_VARIABLE_NAME));
+ return Status;
+ }
+
+ Status = gRT->SetVariable (
+ EFI_PK_DEFAULT_VARIABLE_NAME,
+ &gEfiGlobalVariableGuid,
+ EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS,
+ SigListsSize,
+ (VOID *)EfiSig
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_INFO, "Failed to set %s\n", EFI_PK_DEFAULT_VARIABLE_NAME));
+ }
+
+ FreePool (EfiSig);
+
+ return Status;
+}
+
+/** Initializes KEKDefault variable with data from FFS section.
+
+ @retval EFI_SUCCESS Variable was initialized successfully.
+ @retval EFI_UNSUPPORTED Variable already exists.
+**/
+EFI_STATUS
+SecureBootInitKEKDefault (
+ IN VOID
+ )
+{
+ EFI_SIGNATURE_LIST *EfiSig;
+ UINTN SigListsSize;
+ EFI_STATUS Status;
+ UINT8 *Data;
+ UINTN DataSize;
+
+ //
+ // Check if variable exists, if so do not change it
+ //
+ Status = GetVariable2 (EFI_KEK_DEFAULT_VARIABLE_NAME, &gEfiGlobalVariableGuid, (VOID **) &Data, &DataSize);
+ if (Status == EFI_SUCCESS) {
+ DEBUG ((DEBUG_INFO, "Variable %s exists. Old value is preserved\n", EFI_KEK_DEFAULT_VARIABLE_NAME));
+ FreePool (Data);
+ return EFI_UNSUPPORTED;
+ }
+
+ if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) {
+ return Status;
+ }
+
+ //
+ // Variable does not exist, can be initialized
+ //
+ DEBUG ((DEBUG_INFO, "Variable %s does not exist.\n", EFI_KEK_DEFAULT_VARIABLE_NAME));
+
+ Status = SecureBootFetchData (&gDefaultKEKFileGuid, &SigListsSize, &EfiSig);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_INFO, "Content for %s not found\n", EFI_KEK_DEFAULT_VARIABLE_NAME));
+ return Status;
+ }
+
+
+ Status = gRT->SetVariable (
+ EFI_KEK_DEFAULT_VARIABLE_NAME,
+ &gEfiGlobalVariableGuid,
+ EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS,
+ SigListsSize,
+ (VOID *)EfiSig
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_INFO, "Failed to set %s\n", EFI_KEK_DEFAULT_VARIABLE_NAME));
+ }
+
+ FreePool (EfiSig);
+
+ return Status;
+}
+
+/** Initializes dbDefault variable with data from FFS section.
+
+ @retval EFI_SUCCESS Variable was initialized successfully.
+ @retval EFI_UNSUPPORTED Variable already exists.
+**/
+EFI_STATUS
+SecureBootInitDbDefault (
+ IN VOID
+ )
+{
+ EFI_SIGNATURE_LIST *EfiSig;
+ UINTN SigListsSize;
+ EFI_STATUS Status;
+ UINT8 *Data;
+ UINTN DataSize;
+
+ Status = GetVariable2 (EFI_DB_DEFAULT_VARIABLE_NAME, &gEfiGlobalVariableGuid, (VOID **) &Data, &DataSize);
+ if (Status == EFI_SUCCESS) {
+ DEBUG ((DEBUG_INFO, "Variable %s exists. Old value is preserved\n", EFI_DB_DEFAULT_VARIABLE_NAME));
+ FreePool (Data);
+ return EFI_UNSUPPORTED;
+ }
+
+ if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) {
+ return Status;
+ }
+
+ DEBUG ((DEBUG_INFO, "Variable %s does not exist.\n", EFI_DB_DEFAULT_VARIABLE_NAME));
+
+ Status = SecureBootFetchData (&gDefaultdbFileGuid, &SigListsSize, &EfiSig);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Status = gRT->SetVariable (
+ EFI_DB_DEFAULT_VARIABLE_NAME,
+ &gEfiGlobalVariableGuid,
+ EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS,
+ SigListsSize,
+ (VOID *)EfiSig
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_INFO, "Failed to set %s\n", EFI_DB_DEFAULT_VARIABLE_NAME));
+ }
+
+ FreePool (EfiSig);
+
+ return Status;
+}
+
+/** Initializes dbxDefault variable with data from FFS section.
+
+ @retval EFI_SUCCESS Variable was initialized successfully.
+ @retval EFI_UNSUPPORTED Variable already exists.
+**/
+EFI_STATUS
+SecureBootInitDbxDefault (
+ IN VOID
+ )
+{
+ EFI_SIGNATURE_LIST *EfiSig;
+ UINTN SigListsSize;
+ EFI_STATUS Status;
+ UINT8 *Data;
+ UINTN DataSize;
+
+ //
+ // Check if variable exists, if so do not change it
+ //
+ Status = GetVariable2 (EFI_DBX_DEFAULT_VARIABLE_NAME, &gEfiGlobalVariableGuid, (VOID **) &Data, &DataSize);
+ if (Status == EFI_SUCCESS) {
+ DEBUG ((DEBUG_INFO, "Variable %s exists. Old value is preserved\n", EFI_DBX_DEFAULT_VARIABLE_NAME));
+ FreePool (Data);
+ return EFI_UNSUPPORTED;
+ }
+
+ if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) {
+ return Status;
+ }
+
+ //
+ // Variable does not exist, can be initialized
+ //
+ DEBUG ((DEBUG_INFO, "Variable %s does not exist.\n", EFI_DBX_DEFAULT_VARIABLE_NAME));
+
+ Status = SecureBootFetchData (&gDefaultdbxFileGuid, &SigListsSize, &EfiSig);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_INFO, "Content for %s not found\n", EFI_DBX_DEFAULT_VARIABLE_NAME));
+ return Status;
+ }
+
+ Status = gRT->SetVariable (
+ EFI_DBX_DEFAULT_VARIABLE_NAME,
+ &gEfiGlobalVariableGuid,
+ EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS,
+ SigListsSize,
+ (VOID *)EfiSig
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_INFO, "Failed to set %s\n", EFI_DBX_DEFAULT_VARIABLE_NAME));
+ }
+
+ FreePool (EfiSig);
+
+ return Status;
+}
+
+/** Initializes dbtDefault variable with data from FFS section.
+
+ @retval EFI_SUCCESS Variable was initialized successfully.
+ @retval EFI_UNSUPPORTED Variable already exists.
+**/
+EFI_STATUS
+SecureBootInitDbtDefault (
+ IN VOID
+ )
+{
+ EFI_SIGNATURE_LIST *EfiSig;
+ UINTN SigListsSize;
+ EFI_STATUS Status;
+ UINT8 *Data;
+ UINTN DataSize;
+
+ //
+ // Check if variable exists, if so do not change it
+ //
+ Status = GetVariable2 (EFI_DBT_DEFAULT_VARIABLE_NAME, &gEfiGlobalVariableGuid, (VOID **) &Data, &DataSize);
+ if (Status == EFI_SUCCESS) {
+ DEBUG ((DEBUG_INFO, "Variable %s exists. Old value is preserved\n", EFI_DBT_DEFAULT_VARIABLE_NAME));
+ FreePool (Data);
+ return EFI_UNSUPPORTED;
+ }
+
+ if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) {
+ return Status;
+ }
+
+ //
+ // Variable does not exist, can be initialized
+ //
+ DEBUG ((DEBUG_INFO, "Variable %s does not exist.\n", EFI_DBT_DEFAULT_VARIABLE_NAME));
+
+ Status = SecureBootFetchData (&gDefaultdbtFileGuid, &SigListsSize, &EfiSig);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Status = gRT->SetVariable (
+ EFI_DBT_DEFAULT_VARIABLE_NAME,
+ &gEfiGlobalVariableGuid,
+ EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_BOOTSERVICE_ACCESS,
+ SigListsSize,
+ (VOID *)EfiSig
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_INFO, "Failed to set %s\n", EFI_DBT_DEFAULT_VARIABLE_NAME));
+ }
+
+ FreePool (EfiSig);
+
+ return EFI_SUCCESS;
+}
+
+/**
+ Sets the content of the 'db' variable based on 'dbDefault' variable content.
+
+ @retval EFI_OUT_OF_RESOURCES If memory allocation for EFI_VARIABLE_AUTHENTICATION_2 fails
+ while VendorGuid is NULL.
+ @retval other Errors from GetVariable2 (), GetTime () and SetVariable ()
+**/
+EFI_STATUS
+EFIAPI
+EnrollDbFromDefault (
+ VOID
+)
+{
+ EFI_STATUS Status;
+
+ Status = EnrollFromDefault (
+ EFI_IMAGE_SECURITY_DATABASE,
+ EFI_DB_DEFAULT_VARIABLE_NAME,
+ &gEfiImageSecurityDatabaseGuid
+ );
+
+ return Status;
+}
+
+/**
+ Sets the content of the 'dbx' variable based on 'dbxDefault' variable content.
+
+ @retval EFI_OUT_OF_RESOURCES If memory allocation for EFI_VARIABLE_AUTHENTICATION_2 fails
+ while VendorGuid is NULL.
+ @retval other Errors from GetVariable2 (), GetTime () and SetVariable ()
+**/
+EFI_STATUS
+EFIAPI
+EnrollDbxFromDefault (
+ VOID
+)
+{
+ EFI_STATUS Status;
+
+ Status = EnrollFromDefault (
+ EFI_IMAGE_SECURITY_DATABASE1,
+ EFI_DBX_DEFAULT_VARIABLE_NAME,
+ &gEfiImageSecurityDatabaseGuid
+ );
+
+ return Status;
+}
+
+/**
+ Sets the content of the 'dbt' variable based on 'dbtDefault' variable content.
+
+ @retval EFI_OUT_OF_RESOURCES If memory allocation for EFI_VARIABLE_AUTHENTICATION_2 fails
+ while VendorGuid is NULL.
+ @retval other Errors from GetVariable2 (), GetTime () and SetVariable ()
+**/
+EFI_STATUS
+EFIAPI
+EnrollDbtFromDefault (
+ VOID
+)
+{
+ EFI_STATUS Status;
+
+ Status = EnrollFromDefault (
+ EFI_IMAGE_SECURITY_DATABASE2,
+ EFI_DBT_DEFAULT_VARIABLE_NAME,
+ &gEfiImageSecurityDatabaseGuid);
+
+ return Status;
+}
+
+/**
+ Sets the content of the 'KEK' variable based on 'KEKDefault' variable content.
+
+ @retval EFI_OUT_OF_RESOURCES If memory allocation for EFI_VARIABLE_AUTHENTICATION_2 fails
+ while VendorGuid is NULL.
+ @retval other Errors from GetVariable2 (), GetTime () and SetVariable ()
+**/
+EFI_STATUS
+EFIAPI
+EnrollKEKFromDefault (
+ VOID
+)
+{
+ EFI_STATUS Status;
+
+ Status = EnrollFromDefault (
+ EFI_KEY_EXCHANGE_KEY_NAME,
+ EFI_KEK_DEFAULT_VARIABLE_NAME,
+ &gEfiGlobalVariableGuid
+ );
+
+ return Status;
+}
+
+/**
+ Sets the content of the 'KEK' variable based on 'KEKDefault' variable content.
+
+ @retval EFI_OUT_OF_RESOURCES If memory allocation for EFI_VARIABLE_AUTHENTICATION_2 fails
+ while VendorGuid is NULL.
+ @retval other Errors from GetVariable2 (), GetTime () and SetVariable ()
+**/
+EFI_STATUS
+EFIAPI
+EnrollPKFromDefault (
+ VOID
+)
+{
+ EFI_STATUS Status;
+
+ Status = EnrollFromDefault (
+ EFI_PLATFORM_KEY_NAME,
+ EFI_PK_DEFAULT_VARIABLE_NAME,
+ &gEfiGlobalVariableGuid
+ );
+
+ return Status;
+}
diff --git a/SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.uni b/SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.uni
new file mode 100644
index 0000000000..68d928ef30
--- /dev/null
+++ b/SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.uni
@@ -0,0 +1,16 @@
+// /** @file
+//
+// Provides initialization of Secure Boot keys and databases.
+//
+// Copyright (c) 2021, ARM Ltd. All rights reserved.<BR>
+// Copyright (c) 2021, Semihalf All rights reserved.<BR>
+//
+// SPDX-License-Identifier: BSD-2-Clause-Patent
+//
+// **/
+
+
+#string STR_MODULE_ABSTRACT #language en-US "Provides functions to initialize PK, KEK and databases based on default variables."
+
+#string STR_MODULE_DESCRIPTION #language en-US "Provides functions to initialize PK, KEK and databases based on default variables."
+
--
2.25.1






[PATCH v3] ArmPkg: Enable boot discovery policy for ARM package.

Grzegorz Bernacki
 

This commit adds code which check BootDiscoveryPolicy variable and
calls Boot Policy Manager Protocol to connect device specified by
the variable. To enable that mechanism for platform
EfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy PCD must be
added to DSC file and BootDiscoveryPolicyUiLib should be added to
UiApp libraries.

Signed-off-by: Grzegorz Bernacki <gjb@semihalf.com>
Reviewed-by: Sunny Wang <sunny.wang@arm.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
---

Notes:
v2:
- fix error from CI

v3:
- use local variable DiscoveryPolicy to initialize BootDiscoveryPolicy
- coding style fixes
- run EfiBootManagerRefreshAllBootOption () only if BootDiscoveryPolicy

ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 5 +
ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c | 118 +++++++++++++++++++-
2 files changed, 122 insertions(+), 1 deletion(-)

diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index 353d7a967b..86751b45f8 100644
--- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -65,11 +65,15 @@

[Pcd]
gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut
+ gEfiMdeModulePkgTokenSpaceGuid.PcdBootDiscoveryPolicy

[Guids]
+ gBootDiscoveryPolicyMgrFormsetGuid
gEdkiiNonDiscoverableEhciDeviceGuid
gEdkiiNonDiscoverableUhciDeviceGuid
gEdkiiNonDiscoverableXhciDeviceGuid
+ gEfiBootManagerPolicyNetworkGuid
+ gEfiBootManagerPolicyConnectAllGuid
gEfiFileInfoGuid
gEfiFileSystemInfoGuid
gEfiFileSystemVolumeLabelInfoIdGuid
@@ -79,6 +83,7 @@

[Protocols]
gEdkiiNonDiscoverableDeviceProtocolGuid
+ gEfiBootManagerPolicyProtocolGuid
gEfiDevicePathProtocolGuid
gEfiGraphicsOutputProtocolGuid
gEfiLoadedImageProtocolGuid
diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
index 5ceb23d822..1e4020487a 100644
--- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
+++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
@@ -2,9 +2,10 @@
Implementation for PlatformBootManagerLib library class interfaces.

Copyright (C) 2015-2016, Red Hat, Inc.
- Copyright (c) 2014 - 2019, ARM Ltd. All rights reserved.<BR>
+ Copyright (c) 2014 - 2021, ARM Ltd. All rights reserved.<BR>
Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.<BR>
Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
+ Copyright (c) 2021, Semihalf All rights reserved.<BR>

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

@@ -19,6 +20,7 @@
#include <Library/UefiBootManagerLib.h>
#include <Library/UefiLib.h>
#include <Library/UefiRuntimeServicesTableLib.h>
+#include <Protocol/BootManagerPolicy.h>
#include <Protocol/DevicePath.h>
#include <Protocol/EsrtManagement.h>
#include <Protocol/GraphicsOutput.h>
@@ -27,6 +29,7 @@
#include <Protocol/PciIo.h>
#include <Protocol/PciRootBridgeIo.h>
#include <Protocol/PlatformBootManager.h>
+#include <Guid/BootDiscoveryPolicy.h>
#include <Guid/EventGroup.h>
#include <Guid/NonDiscoverableDevice.h>
#include <Guid/TtyTerm.h>
@@ -703,6 +706,113 @@ HandleCapsules (

#define VERSION_STRING_PREFIX L"Tianocore/EDK2 firmware version "

+/**
+ This functions checks the value of BootDiscoverPolicy variable and
+ connect devices of class specified by that variable. Then it refreshes
+ Boot order for newly discovered boot device.
+
+ @retval EFI_SUCCESS Devices connected successfully or connection
+ not required.
+ @retval others Return values from GetVariable(), LocateProtocol()
+ and ConnectDeviceClass().
+**/
+STATIC
+EFI_STATUS
+BootDiscoveryPolicyHandler (
+ VOID
+ )
+{
+ EFI_STATUS Status;
+ UINT32 DiscoveryPolicy;
+ UINT32 DiscoveryPolicyOld;
+ UINTN Size;
+ EFI_BOOT_MANAGER_POLICY_PROTOCOL *BMPolicy;
+ EFI_GUID *Class;
+
+ Size = sizeof (DiscoveryPolicy);
+ Status = gRT->GetVariable (
+ BOOT_DISCOVERY_POLICY_VAR,
+ &gBootDiscoveryPolicyMgrFormsetGuid,
+ NULL,
+ &Size,
+ &DiscoveryPolicy
+ );
+ if (Status == EFI_NOT_FOUND) {
+ DiscoveryPolicy = PcdGet32 (PcdBootDiscoveryPolicy);
+ Status = PcdSet32S (PcdBootDiscoveryPolicy, DiscoveryPolicy);
+ if (Status == EFI_NOT_FOUND) {
+ return EFI_SUCCESS;
+ } else if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ } else if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ if (DiscoveryPolicy == BDP_CONNECT_MINIMAL) {
+ return EFI_SUCCESS;
+ }
+
+ switch (DiscoveryPolicy) {
+ case BDP_CONNECT_NET:
+ Class = &gEfiBootManagerPolicyNetworkGuid;
+ break;
+ case BDP_CONNECT_ALL:
+ Class = &gEfiBootManagerPolicyConnectAllGuid;
+ break;
+ default:
+ DEBUG ((
+ DEBUG_INFO,
+ "%a - Unexpected DiscoveryPolicy (0x%x). Run Minimal Discovery Policy\n",
+ __FUNCTION__,
+ DiscoveryPolicy
+ ));
+ return EFI_SUCCESS;
+ }
+
+ Status = gBS->LocateProtocol (
+ &gEfiBootManagerPolicyProtocolGuid,
+ NULL,
+ (VOID **)&BMPolicy
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_INFO, "%a - Failed to locate gEfiBootManagerPolicyProtocolGuid."
+ "Driver connect will be skipped.\n", __FUNCTION__));
+ return Status;
+ }
+
+ Status = BMPolicy->ConnectDeviceClass (BMPolicy, Class);
+ if (EFI_ERROR (Status)){
+ DEBUG ((DEBUG_ERROR, "%a - ConnectDeviceClass returns - %r\n", __FUNCTION__, Status));
+ return Status;
+ }
+
+ //
+ // Refresh Boot Options if Boot Discovery Policy has been changed
+ //
+ Size = sizeof (DiscoveryPolicyOld);
+ Status = gRT->GetVariable (
+ BOOT_DISCOVERY_POLICY_OLD_VAR,
+ &gBootDiscoveryPolicyMgrFormsetGuid,
+ NULL,
+ &Size,
+ &DiscoveryPolicyOld
+ );
+ if ((Status == EFI_NOT_FOUND) || (DiscoveryPolicyOld != DiscoveryPolicy)) {
+ EfiBootManagerRefreshAllBootOption ();
+
+ Status = gRT->SetVariable (
+ BOOT_DISCOVERY_POLICY_OLD_VAR,
+ &gBootDiscoveryPolicyMgrFormsetGuid,
+ EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS,
+ sizeof (DiscoveryPolicyOld),
+ &DiscoveryPolicy
+ );
+ }
+
+ return EFI_SUCCESS;
+}
+
/**
Do the platform specific action after the console is ready
Possible things that can be done in PlatformBootManagerAfterConsole:
@@ -753,6 +863,12 @@ PlatformBootManagerAfterConsole (
}
}

+ //
+ // Connect device specified by BootDiscoverPolicy variable and
+ // refresh Boot order for newly discovered boot devices
+ //
+ BootDiscoveryPolicyHandler ();
+
//
// On ARM, there is currently no reason to use the phased capsule
// update approach where some capsules are dispatched before EndOfDxe
--
2.25.1


[PATCH EDK2 v1 1/1] MdeModulePkg/XhciDxe: Decreasing stuck time

wenyi,xie
 

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

After entering setup browser, if frequently plug and unplug usb
device, the browser may be stuck for a few minutes, and user
can not do any operation during that time.
The root cause is that usually it needs 0.5s to create connection,
and this process will be possibly interrupted when frequently
plug and unplug the usb device. Then USBMassStorageDxe and
XhciDxe will retry and it made the browser stucked.

To decrease the stuck time, add a new flag DisConnect to
struct UsbDevContext, if the device is not connected, no need to
send XhcBulkTransfer, XhcAsyncInterruptTransfer or
XhcSyncInterruptTransfer.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Signed-off-by: Wenyi Xie <xiewenyi2@huawei.com>
---
MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h | 46 ++++
MdeModulePkg/Bus/Pci/XhciDxe/XhciBus.h | 229 ++++++++++++++++++++
MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c | 130 ++++++++---
MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 111 ++++++++++
4 files changed, 488 insertions(+), 28 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
index 3285eb8798c0..43552cb7d470 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h
@@ -191,6 +191,10 @@ struct _USB_DEV_CONTEXT {
// Every interface has an active AlternateSetting.
//
UINT8 *ActiveAlternateSetting;
+ //
+ // Whether the usb device status is connected or not.
+ //
+ UINT8 DisConnect;
};

struct _USB_XHCI_INSTANCE {
@@ -508,6 +512,48 @@ XhcControlTransfer (
OUT UINT32 *TransferResult
);

+/**
+ Submits control transfer to a target USB device.
+
+ @param This This EFI_USB2_HC_PROTOCOL instance.
+ @param DeviceAddress The target device address.
+ @param DeviceSpeed Target device speed.
+ @param MaximumPacketLength Maximum packet size the default control transfer
+ endpoint is capable of sending or receiving.
+ @param Request USB device request to send.
+ @param TransferDirection Specifies the data direction for the data stage
+ @param Data Data buffer to be transmitted or received from USB
+ device.
+ @param DataLength The size (in bytes) of the data buffer.
+ @param Timeout Indicates the maximum timeout, in millisecond.
+ @param Translator Transaction translator to be used by this device.
+ @param TransferResult Return the result of this control transfer.
+ @param IsClear The flag used to determine whether clear the port or not
+
+ @retval EFI_SUCCESS Transfer was completed successfully.
+ @retval EFI_OUT_OF_RESOURCES The transfer failed due to lack of resources.
+ @retval EFI_INVALID_PARAMETER Some parameters are invalid.
+ @retval EFI_TIMEOUT Transfer failed due to timeout.
+ @retval EFI_DEVICE_ERROR Transfer failed due to host controller or device error.
+
+**/
+EFI_STATUS
+EFIAPI
+InnerXhcControlTransfer (
+ IN EFI_USB2_HC_PROTOCOL *This,
+ IN UINT8 DeviceAddress,
+ IN UINT8 DeviceSpeed,
+ IN UINTN MaximumPacketLength,
+ IN EFI_USB_DEVICE_REQUEST *Request,
+ IN EFI_USB_DATA_DIRECTION TransferDirection,
+ IN OUT VOID *Data,
+ IN OUT UINTN *DataLength,
+ IN UINTN Timeout,
+ IN EFI_USB2_HC_TRANSACTION_TRANSLATOR *Translator,
+ OUT UINT32 *TransferResult,
+ IN UINT8 IsClear
+ );
+
/**
Submits bulk transfer to a bulk endpoint of a USB device.

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciBus.h b/MdeModulePkg/Bus/Pci/XhciDxe/XhciBus.h
new file mode 100644
index 000000000000..8b2591b23bd7
--- /dev/null
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciBus.h
@@ -0,0 +1,229 @@
+/** @file
+ Only used by Xhc controller to avoid U disk plug-in/out delay
+Copyright (c) 2019 - 2021, Huawei Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+#ifndef _EFI_XHCI_BUS_H_
+#define _EFI_XHCI_BUS_H_
+
+#include <Uefi.h>
+#include <Protocol/UsbIo.h>
+#include <Protocol/Usb2HostController.h>
+#include <Protocol/UsbHostController.h>
+
+#define USB_MAX_LANG_ID 16
+#define USB_MAX_INTERFACE 16
+#define USB_MAX_DEVICES 255
+#define USB_MAX_INTERFACE_SETTING 256
+//
+// Hub class control transfer target
+//
+#define USB_HUB_TARGET_PORT 3
+#define USB_HUB_REQ_GET_STATUS 0
+#define USB_GENERAL_DEVICE_REQUEST_TIMEOUT 500
+
+typedef struct _USB_DEVICE USB_DEVICE;
+typedef struct _USB_INTERFACE USB_INTERFACE;
+typedef struct _USB_BUS USB_BUS;
+typedef struct _USB_HUB_API USB_HUB_API;
+typedef struct {
+ EFI_USB_ENDPOINT_DESCRIPTOR Desc;
+ UINT8 Toggle;
+} USB_ENDPOINT_DESC;
+
+typedef struct {
+ EFI_USB_INTERFACE_DESCRIPTOR Desc;
+ USB_ENDPOINT_DESC **Endpoints;
+} USB_INTERFACE_SETTING;
+
+typedef struct {
+ USB_INTERFACE_SETTING* Settings[USB_MAX_INTERFACE_SETTING];
+ UINTN NumOfSetting;
+ UINTN ActiveIndex; // Index of active setting
+} USB_INTERFACE_DESC;
+
+typedef struct {
+ EFI_USB_CONFIG_DESCRIPTOR Desc;
+ USB_INTERFACE_DESC **Interfaces;
+} USB_CONFIG_DESC;
+
+typedef struct {
+ EFI_USB_DEVICE_DESCRIPTOR Desc;
+ USB_CONFIG_DESC **Configs;
+} USB_DEVICE_DESC;
+
+//
+// Stands for the real USB device. Each device may
+// has several seperately working interfaces.
+//
+struct _USB_DEVICE {
+ USB_BUS *Bus;
+ //
+ // Configuration information
+ //
+ UINT8 Speed;
+ UINT8 Address;
+ UINT32 MaxPacket0;
+ //
+ // The device's descriptors and its configuration
+ //
+ USB_DEVICE_DESC *DevDesc;
+ USB_CONFIG_DESC *ActiveConfig;
+ UINT16 LangId [USB_MAX_LANG_ID];
+ UINT16 TotalLangId;
+ UINT8 NumOfInterface;
+ USB_INTERFACE *Interfaces [USB_MAX_INTERFACE];
+ //
+ // Parent child relationship
+ //
+ EFI_USB2_HC_TRANSACTION_TRANSLATOR Translator;
+ UINT8 ParentAddr;
+ USB_INTERFACE *ParentIf;
+ UINT8 ParentPort; // Start at 0
+ UINT8 Tier;
+ BOOLEAN DisconnectFail;
+};
+
+//
+// Stands for different functions of USB device
+//
+struct _USB_INTERFACE {
+ UINTN Signature;
+ USB_DEVICE *Device;
+ USB_INTERFACE_DESC *IfDesc;
+ USB_INTERFACE_SETTING *IfSetting;
+ //
+ // Handles and protocols
+ //
+ EFI_HANDLE Handle;
+ EFI_USB_IO_PROTOCOL UsbIo;
+ EFI_DEVICE_PATH_PROTOCOL *DevicePath;
+ BOOLEAN IsManaged;
+ //
+ // Hub device special data
+ //
+ BOOLEAN IsHub;
+ USB_HUB_API *HubApi;
+ UINT8 NumOfPort;
+ EFI_EVENT HubNotify;
+ //
+ // Data used only by normal hub devices
+ //
+ USB_ENDPOINT_DESC *HubEp;
+ UINT8 *ChangeMap;
+ //
+ // Data used only by root hub to hand over device to
+ // companion UHCI driver if low/full speed devices are
+ // connected to EHCI.
+ //
+ UINT8 MaxSpeed;
+};
+
+typedef struct _EFI_USB_BUS_PROTOCOL {
+ UINT64 Reserved;
+} EFI_USB_BUS_PROTOCOL;
+
+//
+// Stands for the current USB Bus
+//
+struct _USB_BUS {
+ UINTN Signature;
+ EFI_USB_BUS_PROTOCOL BusId;
+ //
+ // Managed USB host controller
+ //
+ EFI_HANDLE HostHandle;
+ EFI_DEVICE_PATH_PROTOCOL *DevicePath;
+ EFI_USB2_HC_PROTOCOL *Usb2Hc;
+ EFI_USB_HC_PROTOCOL *UsbHc;
+ //
+ // Recorded the max supported usb devices.
+ // XHCI can support up to 255 devices.
+ // EHCI/UHCI/OHCI supports up to 127 devices.
+ //
+ UINT32 MaxDevices;
+ //
+ // An array of device that is on the bus. Devices[0] is
+ // for root hub. Device with address i is at Devices[i].
+ //
+ USB_DEVICE *Devices[256];
+ //
+ // USB Bus driver need to control the recursive connect policy of the bus, only those wanted
+ // usb child device will be recursively connected.
+ //
+ // WantedUsbIoDPList tracks the Usb child devices which user want to recursivly fully connecte,
+ // every wanted child device is stored in a item of the WantedUsbIoDPList, whose structrure is
+ // DEVICE_PATH_LIST_ITEM
+ //
+ LIST_ENTRY WantedUsbIoDPList;
+};
+
+typedef
+EFI_STATUS
+(*USB_HUB_INIT) (
+ IN USB_INTERFACE *UsbIf
+ );
+
+//
+// Get the port status. This function is required to
+// ACK the port change bits although it will return
+// the port changes in PortState. Bus enumeration code
+// doesn't need to ACK the port change bits.
+//
+typedef
+EFI_STATUS
+(*USB_HUB_GET_PORT_STATUS) (
+ IN USB_INTERFACE *UsbIf,
+ IN UINT8 Port,
+ OUT EFI_USB_PORT_STATUS *PortState
+ );
+
+typedef
+VOID
+(*USB_HUB_CLEAR_PORT_CHANGE) (
+ IN USB_INTERFACE *HubIf,
+ IN UINT8 Port
+ );
+
+typedef
+EFI_STATUS
+(*USB_HUB_SET_PORT_FEATURE) (
+ IN USB_INTERFACE *UsbIf,
+ IN UINT8 Port,
+ IN EFI_USB_PORT_FEATURE Feature
+ );
+
+typedef
+EFI_STATUS
+(*USB_HUB_CLEAR_PORT_FEATURE) (
+ IN USB_INTERFACE *UsbIf,
+ IN UINT8 Port,
+ IN EFI_USB_PORT_FEATURE Feature
+ );
+
+typedef
+EFI_STATUS
+(*USB_HUB_RESET_PORT) (
+ IN USB_INTERFACE *UsbIf,
+ IN UINT8 Port
+ );
+
+typedef
+EFI_STATUS
+(*USB_HUB_RELEASE) (
+ IN USB_INTERFACE *UsbIf
+ );
+
+//
+// USB Hub Api
+//
+struct _USB_HUB_API{
+ USB_HUB_INIT Init;
+ USB_HUB_GET_PORT_STATUS GetPortStatus;
+ USB_HUB_CLEAR_PORT_CHANGE ClearPortChange;
+ USB_HUB_SET_PORT_FEATURE SetPortFeature;
+ USB_HUB_CLEAR_PORT_FEATURE ClearPortFeature;
+ USB_HUB_RESET_PORT ResetPort;
+ USB_HUB_RELEASE Release;
+};
+#endif
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
index 43c53bad4e4a..59421d13395c 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
@@ -849,6 +849,66 @@ XhcControlTransfer (
IN EFI_USB2_HC_TRANSACTION_TRANSLATOR *Translator,
OUT UINT32 *TransferResult
)
+{
+ EFI_STATUS Status;
+ Status = InnerXhcControlTransfer(
+ This,
+ DeviceAddress,
+ DeviceSpeed,
+ MaximumPacketLength,
+ Request,
+ TransferDirection,
+ Data,
+ DataLength,
+ Timeout,
+ Translator,
+ TransferResult,
+ 1
+ );
+ return Status;
+}
+
+/**
+ Submits control transfer to a target USB device.
+
+ @param This This EFI_USB2_HC_PROTOCOL instance.
+ @param DeviceAddress The target device address.
+ @param DeviceSpeed Target device speed.
+ @param MaximumPacketLength Maximum packet size the default control transfer
+ endpoint is capable of sending or receiving.
+ @param Request USB device request to send.
+ @param TransferDirection Specifies the data direction for the data stage
+ @param Data Data buffer to be transmitted or received from USB
+ device.
+ @param DataLength The size (in bytes) of the data buffer.
+ @param Timeout Indicates the maximum timeout, in millisecond.
+ @param Translator Transaction translator to be used by this device.
+ @param TransferResult Return the result of this control transfer.
+ @param IsClear The flag used to determine whether clear the port or not
+
+ @retval EFI_SUCCESS Transfer was completed successfully.
+ @retval EFI_OUT_OF_RESOURCES The transfer failed due to lack of resources.
+ @retval EFI_INVALID_PARAMETER Some parameters are invalid.
+ @retval EFI_TIMEOUT Transfer failed due to timeout.
+ @retval EFI_DEVICE_ERROR Transfer failed due to host controller or device error.
+
+**/
+EFI_STATUS
+EFIAPI
+InnerXhcControlTransfer (
+ IN EFI_USB2_HC_PROTOCOL *This,
+ IN UINT8 DeviceAddress,
+ IN UINT8 DeviceSpeed,
+ IN UINTN MaximumPacketLength,
+ IN EFI_USB_DEVICE_REQUEST *Request,
+ IN EFI_USB_DATA_DIRECTION TransferDirection,
+ IN OUT VOID *Data,
+ IN OUT UINTN *DataLength,
+ IN UINTN Timeout,
+ IN EFI_USB2_HC_TRANSACTION_TRANSLATOR *Translator,
+ OUT UINT32 *TransferResult,
+ IN UINT8 IsClear
+ )
{
USB_XHCI_INSTANCE *Xhc;
UINT8 Endpoint;
@@ -959,6 +1019,10 @@ XhcControlTransfer (
goto ON_EXIT;
}

+ if (Xhc->UsbDevContext[SlotId].DisConnect == 1) {
+ DEBUG ((EFI_D_ERROR, "XhcControlTransfer: ContextIndex %u disconnect\n", SlotId));
+ goto ON_EXIT;
+ }
//
// Create a new URB, insert it into the asynchronous
// schedule list, then poll the execution status.
@@ -1128,35 +1192,34 @@ XhcControlTransfer (
}
}

- MapSize = sizeof (mUsbHubClearPortChangeMap) / sizeof (USB_CLEAR_PORT_MAP);
-
- for (Index = 0; Index < MapSize; Index++) {
- if (XHC_BIT_IS_SET (State, mUsbHubClearPortChangeMap[Index].HwState)) {
- ZeroMem (&ClearPortRequest, sizeof (EFI_USB_DEVICE_REQUEST));
- ClearPortRequest.RequestType = USB_REQUEST_TYPE (EfiUsbNoData, USB_REQ_TYPE_CLASS, USB_TARGET_OTHER);
- ClearPortRequest.Request = (UINT8) USB_REQ_CLEAR_FEATURE;
- ClearPortRequest.Value = mUsbHubClearPortChangeMap[Index].Selector;
- ClearPortRequest.Index = Request->Index;
- ClearPortRequest.Length = 0;
-
- XhcControlTransfer (
- This,
- DeviceAddress,
- DeviceSpeed,
- MaximumPacketLength,
- &ClearPortRequest,
- EfiUsbNoData,
- NULL,
- &Len,
- Timeout,
- Translator,
- TransferResult
- );
+ if (IsClear) {
+ MapSize = sizeof (mUsbHubClearPortChangeMap) / sizeof (USB_CLEAR_PORT_MAP);
+ for (Index = 0; Index < MapSize; Index++) {
+ if (XHC_BIT_IS_SET (State, mUsbHubClearPortChangeMap[Index].HwState)) {
+ ZeroMem (&ClearPortRequest, sizeof (EFI_USB_DEVICE_REQUEST));
+ ClearPortRequest.RequestType = USB_REQUEST_TYPE (EfiUsbNoData, USB_REQ_TYPE_CLASS, USB_TARGET_OTHER);
+ ClearPortRequest.Request = (UINT8) USB_REQ_CLEAR_FEATURE;
+ ClearPortRequest.Value = mUsbHubClearPortChangeMap[Index].Selector;
+ ClearPortRequest.Index = Request->Index;
+ ClearPortRequest.Length = 0;
+ XhcControlTransfer (
+ This,
+ DeviceAddress,
+ DeviceSpeed,
+ MaximumPacketLength,
+ &ClearPortRequest,
+ EfiUsbNoData,
+ NULL,
+ &Len,
+ Timeout,
+ Translator,
+ TransferResult
+ );
+ }
}
+
+ XhcPollPortStatusChange (Xhc, Xhc->UsbDevContext[SlotId].RouteString, (UINT8)Request->Index, &PortStatus);
}
-
- XhcPollPortStatusChange (Xhc, Xhc->UsbDevContext[SlotId].RouteString, (UINT8)Request->Index, &PortStatus);
-
*(UINT32 *)Data = *(UINT32*)&PortStatus;
}

@@ -1261,7 +1324,10 @@ XhcBulkTransfer (
if (SlotId == 0) {
goto ON_EXIT;
}
-
+ if (Xhc->UsbDevContext[SlotId].DisConnect) {
+ DEBUG ((EFI_D_ERROR, "XhcBulkTransfer: UsbDevContext %u disconnect\n", SlotId));
+ goto ON_EXIT;
+ }
//
// Create a new URB, insert it into the asynchronous
// schedule list, then poll the execution status.
@@ -1405,6 +1471,10 @@ XhcAsyncInterruptTransfer (
if (SlotId == 0) {
goto ON_EXIT;
}
+ if (Xhc->UsbDevContext[SlotId].DisConnect) {
+ DEBUG ((EFI_D_ERROR, "XhcAsyncInterruptTransfer: UsbDevContext %u disconnect\n", SlotId));
+ goto ON_EXIT;
+ }

Urb = XhciInsertAsyncIntTransfer (
Xhc,
@@ -1519,6 +1589,10 @@ XhcSyncInterruptTransfer (
if (SlotId == 0) {
goto ON_EXIT;
}
+ if (Xhc->UsbDevContext[SlotId].DisConnect) {
+ DEBUG ((EFI_D_ERROR, "XhcSyncInterruptTransfer: UsbDevContext %u disconnect\n", SlotId));
+ goto ON_EXIT;
+ }

Status = XhcTransfer (
Xhc,
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
index 7cbc9a8502ea..6d7d67d00c18 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
@@ -9,6 +9,114 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
**/

#include "Xhci.h"
+#include "XhciBus.h"
+#define MAX_DEVICE_NUM 255
+EFI_STATUS
+EFIAPI
+XhcGetHubPortStatus (
+ IN USB_INTERFACE *HubIf,
+ IN EFI_USB_PORT_STATUS *PortState,
+ IN UINT8 Port
+ )
+{
+ USB_DEVICE *Device;
+ UINT8 PortNum;
+ EFI_STATUS Status;
+ UINT32 Result;
+ EFI_USB_DEVICE_REQUEST DevReq;
+ UINTN Len;
+ Device = HubIf->Device;
+ PortNum = HubIf->NumOfPort;
+ Len = 4;
+ // DevReq
+ DevReq.RequestType = USB_REQUEST_TYPE (EfiUsbDataIn,
+ USB_REQ_TYPE_CLASS, USB_HUB_TARGET_PORT);
+ DevReq.Request = (UINT8) USB_HUB_REQ_GET_STATUS;
+ DevReq.Value = 0;
+ DevReq.Index = Port;
+ DevReq.Length = 4;
+ //
+ //controlTransfer to get hub port status
+ //
+ Status = InnerXhcControlTransfer (
+ Device->Bus->Usb2Hc,
+ Device->Address,
+ Device->Speed,
+ Device->MaxPacket0,
+ &DevReq,
+ EfiUsbDataIn,
+ (void *) PortState,
+ &Len,
+ USB_GENERAL_DEVICE_REQUEST_TIMEOUT,
+ &Device->Translator,
+ &Result,
+ 0
+ );
+ if (EFI_ERROR(Status)) {
+ DEBUG ((EFI_D_ERROR, "StoreHubPortStatus:ControlTransfer status = %r\n", Status));
+ return Status;
+ }
+ return EFI_SUCCESS;
+}
+VOID
+XhcGetHubDevice (
+ IN USB_XHCI_INSTANCE *Xhc,
+ IN URB *Urb
+ )
+{
+ USB_INTERFACE *HubIf;
+ EFI_USB_PORT_STATUS PortState;
+ USB_ENDPOINT Ep;
+ UINT8 Index;
+ UINT32 HubIndex;
+ UINT32 PortIndex = 0;
+ UINT8 Port[MAX_DEVICE_NUM];
+ UINT32 PortDevice[MAX_DEVICE_NUM];
+ EFI_STATUS Status;
+ Ep = Urb->Ep;
+ //
+ //Find hub device Context Index
+ //
+ for (Index = 0; Index < MAX_DEVICE_NUM; Index++) {
+ if (Xhc->UsbDevContext[Index + 1].BusDevAddr == Ep.BusAddr) {
+ HubIndex = Index + 1;
+ break;
+ }
+ }
+ if (Index == USB_MAX_DEVICES) {
+ return;
+ }
+ //Find port device Index and Port
+ for (Index = 0; Index < MAX_DEVICE_NUM; Index++) {
+ if (CompareMem(&Xhc->UsbDevContext[Index + 1].ParentRouteString,
+ &Xhc->UsbDevContext[HubIndex].RouteString,
+ sizeof(Xhc->UsbDevContext[HubIndex].RouteString)) == 0) {
+ // Device index in Xhc Devcontext
+ PortDevice[PortIndex] = Index + 1;
+ // Device port in hub
+ Port[PortIndex] = Xhc->UsbDevContext[Index + 1].RouteString.Route.RouteString >>
+ (4 * (Xhc->UsbDevContext[Index + 1].ParentRouteString.Route.TierNum - 1));
+ PortIndex ++;
+ }
+ }
+ if (PortIndex == 0) {
+ return;
+ }
+ //
+ // If Device has child device, the device is hud
+ //
+ HubIf = (USB_INTERFACE *) Urb->Context;
+ for (Index = 0; Index < PortIndex; Index ++) {
+ Status = XhcGetHubPortStatus(HubIf, &PortState, Port[Index]);
+ if (EFI_ERROR(Status)) {
+ continue;
+ }
+ if ((PortState.PortStatus & USB_PORT_STAT_CONNECTION) == 0 &&
+ (PortState.PortChangeStatus & USB_PORT_STAT_C_CONNECTION) == 1) {
+ Xhc->UsbDevContext[PortDevice[Index]].DisConnect = 1;
+ }
+ }
+}

/**
Create a command transfer TRB to support XHCI command interfaces.
@@ -1686,6 +1794,7 @@ XhcMonitorAsyncRequests (
//
gBS->RestoreTPL (OldTpl);
(Urb->Callback) (ProcBuf, Urb->Completed, Urb->Context, Urb->Result);
+ XhcGetHubDevice (Xhc, Urb);
OldTpl = gBS->RaiseTPL (XHC_TPL);
}

@@ -2152,6 +2261,7 @@ XhcInitializeDeviceSlot (
Xhc->UsbDevContext[SlotId].SlotId = SlotId;
Xhc->UsbDevContext[SlotId].RouteString.Dword = RouteChart.Dword;
Xhc->UsbDevContext[SlotId].ParentRouteString.Dword = ParentRouteChart.Dword;
+ Xhc->UsbDevContext[SlotId].DisConnect = 0;

//
// 4.3.3 Device Slot Initialization
@@ -2365,6 +2475,7 @@ XhcInitializeDeviceSlot64 (
Xhc->UsbDevContext[SlotId].SlotId = SlotId;
Xhc->UsbDevContext[SlotId].RouteString.Dword = RouteChart.Dword;
Xhc->UsbDevContext[SlotId].ParentRouteString.Dword = ParentRouteChart.Dword;
+ Xhc->UsbDevContext[SlotId].DisConnect = 0;

//
// 4.3.3 Device Slot Initialization
--
2.20.1.windows.1


[PATCH EDK2 v1 0/1] MdeModulePkg/XhciDxe: Decreasing stuck time

wenyi,xie
 

Main Changes :
1.add a new flag DisConnect to struct UsbDevContext to determine whether the device is connected or not
2.add new function XhcGetHubDevice to check port status

Wenyi Xie (1):
MdeModulePkg/XhciDxe: Decreasing stuck time

MdeModulePkg/Bus/Pci/XhciDxe/Xhci.h | 46 ++++
MdeModulePkg/Bus/Pci/XhciDxe/XhciBus.h | 229 ++++++++++++++++++++
MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c | 130 ++++++++---
MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c | 111 ++++++++++
4 files changed, 488 insertions(+), 28 deletions(-)
create mode 100644 MdeModulePkg/Bus/Pci/XhciDxe/XhciBus.h

--
2.20.1.windows.1

6221 - 6240 of 86113