Date   

Re: [Patch v2 0/2] UefiCpuPkg: Remove debug message.

Laszlo Ersek
 

On 08/07/19 19:11, Laszlo Ersek wrote:
On 08/05/19 08:43, Eric Dong wrote:
This debug message may be called by BSP and APs. It may
caused ASSERT when APs call this debug code.

In order to avoid system boot assert, Remove this debug
message.

Cc: Ray Ni <ray.ni@...>
Cc: Laszlo Ersek <lersek@...>

Eric Dong (2):
UefiCpuPkg/RegisterCpuFeaturesLib: Remove debug message.
UefiCpuPkg/PiSmmCpuDxeSmm: Remove debug message.

.../CpuFeaturesInitialize.c | 22 -------------------
UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 22 +------------------
2 files changed, 1 insertion(+), 43 deletions(-)
It seems to me that, after these patches are applied, no uses of
"ConsoleLogLock" remain, in either module (RegisterCpuFeaturesLib and
PiSmmCpuDxeSmm).

Can we eliminate the field from both modules? Otherwise we'll be left
with a useless, initialized spinlock, in each of these modules.
I can see that this series has been pushed already, so I've now filed a
reminder:

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

Thanks
Laszlo


Re: [tianocore-docs EDK_II_Secure_Coding_Guide PATCH] Add Appendix: Threat Mode for EDK II.

Vincent Zimmer
 

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
Sent: Wednesday, August 7, 2019 10:46 AM
To: Yao, Jiewen <jiewen.yao@...>
Cc: devel@edk2.groups.io; Zimmer, Vincent <vincent.zimmer@...>; Jarlstrom, Laurie <@laurie0131>
Subject: Re: [edk2-devel] [tianocore-docs EDK_II_Secure_Coding_Guide PATCH] Add Appendix: Threat Mode for EDK II.

(+ Laurie)

On 08/05/19 09:47, Yao, Jiewen wrote:
This patch adds "Threat model for EDK II" as the appendix section

of "EDK II secure coding guide" document.


The threat model discussed here is a general guide and serves as the
baseline of

the EDK II firmware. For each specific feature in EDK II firmware,
there might be

additional feature-based threat models in addition to the general threat model.


The full gitbook can be also avaiable at

https://github.com/jyao1/EDK_II_Secure_Coding_Guide/tree/Threat_model.


Cc: Vincent Zimmer <vincent.zimmer@...>
Signed-off-by: Jiewen Yao <jiewen.yao@...>

Reviewed-by: Vincent Zimmer <vincent.zimmer@...>


---
SUMMARY.md | 6 ++
appendix_threat_model_for_edk_ii/README.md | 70 +++++++++++++++++++
.../asset_boot_flow.md | 63 +++++++++++++++++
.../asset_build_tool.md | 39 +++++++++++
.../asset_flash_content.md | 59 ++++++++++++++++
.../asset_management_mode.md | 58 +++++++++++++++
.../asset_s3_resume.md | 61 ++++++++++++++++
7 files changed, 356 insertions(+)
create mode 100644 appendix_threat_model_for_edk_ii/README.md
create mode 100644
appendix_threat_model_for_edk_ii/asset_boot_flow.md
create mode 100644
appendix_threat_model_for_edk_ii/asset_build_tool.md
create mode 100644
appendix_threat_model_for_edk_ii/asset_flash_content.md
create mode 100644
appendix_threat_model_for_edk_ii/asset_management_mode.md
create mode 100644
appendix_threat_model_for_edk_ii/asset_s3_resume.md
It looks like we have at least two kinds of documents under <https://github.com/tianocore-docs/>, namely:
- specs (for edk2)
- guides

I know of the article at
<https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Specifications>,
which presents the specs very nicely.

Do we have a similar wiki article about the guides too?

(If I use the article search box to the right, and enter "guide", nothing relevant comes up.)

Thanks!
Laszlo


Re: [tianocore-docs EDK_II_Secure_Coding_Guide PATCH] Add Appendix: Threat Mode for EDK II.

Laszlo Ersek
 

(+ Laurie)

On 08/05/19 09:47, Yao, Jiewen wrote:
This patch adds "Threat model for EDK II" as the appendix section

of "EDK II secure coding guide" document.


The threat model discussed here is a general guide and serves as the baseline of

the EDK II firmware. For each specific feature in EDK II firmware, there might be

additional feature-based threat models in addition to the general threat model.


The full gitbook can be also avaiable at

https://github.com/jyao1/EDK_II_Secure_Coding_Guide/tree/Threat_model.


Cc: Vincent Zimmer <vincent.zimmer@...>
Signed-off-by: Jiewen Yao <jiewen.yao@...>

Reviewed-by: Vincent Zimmer <vincent.zimmer@...>


---
SUMMARY.md | 6 ++
appendix_threat_model_for_edk_ii/README.md | 70 +++++++++++++++++++
.../asset_boot_flow.md | 63 +++++++++++++++++
.../asset_build_tool.md | 39 +++++++++++
.../asset_flash_content.md | 59 ++++++++++++++++
.../asset_management_mode.md | 58 +++++++++++++++
.../asset_s3_resume.md | 61 ++++++++++++++++
7 files changed, 356 insertions(+)
create mode 100644 appendix_threat_model_for_edk_ii/README.md
create mode 100644 appendix_threat_model_for_edk_ii/asset_boot_flow.md
create mode 100644 appendix_threat_model_for_edk_ii/asset_build_tool.md
create mode 100644 appendix_threat_model_for_edk_ii/asset_flash_content.md
create mode 100644 appendix_threat_model_for_edk_ii/asset_management_mode.md
create mode 100644 appendix_threat_model_for_edk_ii/asset_s3_resume.md
It looks like we have at least two kinds of documents under
<https://github.com/tianocore-docs/>, namely:
- specs (for edk2)
- guides

I know of the article at
<https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Specifications>,
which presents the specs very nicely.

Do we have a similar wiki article about the guides too?

(If I use the article search box to the right, and enter "guide",
nothing relevant comes up.)

Thanks!
Laszlo


Re: static data in dxe_runtime modules

Andrew Fish
 



On Aug 7, 2019, at 10:29 AM, Laszlo Ersek <lersek@...> wrote:

ummm... not sure why, but I never got this email in my inbox. I only see
it in my list folder. I see myself addressed on it as:

 Laszlo Ersek via Groups.Io <lersek@...>

which could be the reason.

Anyway:

On 08/05/19 12:18, Roman Kagan wrote:
On Sat, Aug 03, 2019 at 04:03:04AM +0200, Laszlo Ersek via Groups.Io wrote:
On 08/01/19 21:16, Roman Kagan wrote:
This is a serious bug. Thank you for reporting and analyzing it. Can you
file it in the TianoCore Bugzilla too, please?

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

I wonder how far in OpenSSL history this issue goes back. Is this a
regression from the latest OpenSSL rebase in edk2?

This is certainly not a recent regression.  We've initially caught it
with Virtuozzo OVMF based on the one from RHEL-7.6, based in turn on
EDKII as of May 2018.  However, the infrastructure that causes the
problem is there for much longer.

OK. Thank you for confirming!

What would be the best way to fix it?

One option is to audit OpenSSL and make sure it either doesn't put
pointers into static storage or properly cleans them up (and call the
cleanup function in RuntimeCryptLibAddressChangeEvent); then assume the
rest of EDKII code is safe in this regard.

Another is to assume that no static data in dxe_runtime modules should
survive SetVirtualAddressMap, and thus make
PeCoffLoaderRelocateImageForRuntime reinitialize the modules from
scratch instead of re-applying the relocations only.

I must admit I don't yet quite understand the full consequences of
either option.  Perhaps there are better ones.
Any suggestion is appreciated.

If the runtime driver remembers pointer *values* from before
SetVirtualAddressMap() -- i.e. it saves pointer values into some
storage, for de-referencing at OS runtime --, those stored values have
to be converted in the virtual address change event notification function.
[...]
The "thread_local_storage" array from
"CryptoPkg/Library/OpensslLib/openssl/crypto/threads_none.c" has to be
exposed to RuntimeCryptLibAddressChangeEvent() somehow.

Perhaps OpenSSL should allow edk2 to bring its own CRYPTO_THREAD_* APIs.

I think this would be too awkward, as edk2 has no reason to have any
visibility into it.

Edk2 already implements various sets of APIs that "plug" into OpenSSL.

Not saying that it's optimal, but there is precedence.

I'd rather make use of the observation that in reality there's no data
in OpenSSL that needs to survive SetVirtualAddressMap().  At first I
started to cook up a fix that involved calling OPENSSL_cleanup() from
RuntimeCryptLibAddressChangeEvent(), but it soon turned out it didn't
clean things up to the pristine state so it didn't address the problem.

Moreover I think it's overoptimistic to expect from OpenSSL developers
the mindset that their code should work seamlessly across relocations at
runtime.

Well, they do have a UEFI system ID in "Configurations/10-main.conf".

And I do think OpenSSL developers are interested in robustness over a
number of use cases. After all, the thread-specific key abstraction
exists for this kind of portability in the first place.

I don't see what would stop them from introducing another
pointer variable with global storage further down the road, and nothing
would allow to even timely spot this new problem.

Edk2 could deal with this kind of problem a lot better if we timed our
OpenSSL submodule updates to the *start* of every edk2 development
cycle, not to the *end* of it.

That's why I think the most reliable solution would be to just reload
the module completely.  If this can't be done for all runtime modules
then I'd do it for the one(s) linking to OpenSSL.

I don't think we should special-case how
RuntimeDriverSetVirtualAddressMap()
[MdeModulePkg/Core/RuntimeDxe/Runtime.c] works. UEFI (the specification)
already specifies a general facility for this problem; we should use it.

I'm convinced that OpenSSL needs to expose a new API for this particular
problem.


This feels right to me too. The reality is any pointer assigned in code needs to get "fixed up" for the new virtual mapping. The other restriction is non of the EFI Boot Services are available so you can't allocate memory etc.  The code that runs to fixup pointers runs in the physical mapping, but has access to an API that can convert a physical address to its future virtual mapping. 

If there is an API EFI can call to re-init passing in the old buffers that may be the easiest thing to maintain. The EFI driver would then have a boolean flag and need to call the OpenSSL re-init on the 1st virtual call into the driver. This would let the OpenSSL code function in a more normal fashion. Every thing else I can think of requires the OpenSSL code to keep a list of the pointers that need to be fixed up, and that would need to be maintained by hand. 

I can help answer any detailed questions about how EFI Runtime functions if that would help. 

Thanks,

Andrew Fish

David, can you please offer some thoughts on this?

Thanks!
Laszlo


Roman.







Re: static data in dxe_runtime modules

Laszlo Ersek
 

ummm... not sure why, but I never got this email in my inbox. I only see
it in my list folder. I see myself addressed on it as:

Laszlo Ersek via Groups.Io <lersek=redhat.com@groups.io>

which could be the reason.

Anyway:

On 08/05/19 12:18, Roman Kagan wrote:
On Sat, Aug 03, 2019 at 04:03:04AM +0200, Laszlo Ersek via Groups.Io wrote:
On 08/01/19 21:16, Roman Kagan wrote:
This is a serious bug. Thank you for reporting and analyzing it. Can you
file it in the TianoCore Bugzilla too, please?
https://bugzilla.tianocore.org/show_bug.cgi?id=2053

I wonder how far in OpenSSL history this issue goes back. Is this a
regression from the latest OpenSSL rebase in edk2?
This is certainly not a recent regression. We've initially caught it
with Virtuozzo OVMF based on the one from RHEL-7.6, based in turn on
EDKII as of May 2018. However, the infrastructure that causes the
problem is there for much longer.
OK. Thank you for confirming!

What would be the best way to fix it?

One option is to audit OpenSSL and make sure it either doesn't put
pointers into static storage or properly cleans them up (and call the
cleanup function in RuntimeCryptLibAddressChangeEvent); then assume the
rest of EDKII code is safe in this regard.

Another is to assume that no static data in dxe_runtime modules should
survive SetVirtualAddressMap, and thus make
PeCoffLoaderRelocateImageForRuntime reinitialize the modules from
scratch instead of re-applying the relocations only.

I must admit I don't yet quite understand the full consequences of
either option. Perhaps there are better ones.
Any suggestion is appreciated.
If the runtime driver remembers pointer *values* from before
SetVirtualAddressMap() -- i.e. it saves pointer values into some
storage, for de-referencing at OS runtime --, those stored values have
to be converted in the virtual address change event notification function.
[...]
The "thread_local_storage" array from
"CryptoPkg/Library/OpensslLib/openssl/crypto/threads_none.c" has to be
exposed to RuntimeCryptLibAddressChangeEvent() somehow.

Perhaps OpenSSL should allow edk2 to bring its own CRYPTO_THREAD_* APIs.
I think this would be too awkward, as edk2 has no reason to have any
visibility into it.
Edk2 already implements various sets of APIs that "plug" into OpenSSL.

Not saying that it's optimal, but there is precedence.

I'd rather make use of the observation that in reality there's no data
in OpenSSL that needs to survive SetVirtualAddressMap(). At first I
started to cook up a fix that involved calling OPENSSL_cleanup() from
RuntimeCryptLibAddressChangeEvent(), but it soon turned out it didn't
clean things up to the pristine state so it didn't address the problem.

Moreover I think it's overoptimistic to expect from OpenSSL developers
the mindset that their code should work seamlessly across relocations at
runtime.
Well, they do have a UEFI system ID in "Configurations/10-main.conf".

And I do think OpenSSL developers are interested in robustness over a
number of use cases. After all, the thread-specific key abstraction
exists for this kind of portability in the first place.

I don't see what would stop them from introducing another
pointer variable with global storage further down the road, and nothing
would allow to even timely spot this new problem.
Edk2 could deal with this kind of problem a lot better if we timed our
OpenSSL submodule updates to the *start* of every edk2 development
cycle, not to the *end* of it.

That's why I think the most reliable solution would be to just reload
the module completely. If this can't be done for all runtime modules
then I'd do it for the one(s) linking to OpenSSL.
I don't think we should special-case how
RuntimeDriverSetVirtualAddressMap()
[MdeModulePkg/Core/RuntimeDxe/Runtime.c] works. UEFI (the specification)
already specifies a general facility for this problem; we should use it.

I'm convinced that OpenSSL needs to expose a new API for this particular
problem.

David, can you please offer some thoughts on this?

Thanks!
Laszlo


Roman.



Re: [Patch v2 1/2] UefiCpuPkg/RegisterCpuFeaturesLib: Remove debug message.

rebecca@...
 

On 2019-08-05 00:43, Dong, Eric wrote:
This debug message may be called by BSP and APs. It may
caused ASSERT when APs call this debug code.

In order to avoid system boot assert, Remove this debug
message.

Signed-off-by: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>
Cc: Laszlo Ersek <lersek@...>
I can't find it just now, but I seem to recall there's an associated BZ
ticket that we triaged a couple of weeks ago.


--
Rebecca Cran


Re: [Patch v2 0/2] UefiCpuPkg: Remove debug message.

Laszlo Ersek
 

On 08/05/19 08:43, Eric Dong wrote:
This debug message may be called by BSP and APs. It may
caused ASSERT when APs call this debug code.

In order to avoid system boot assert, Remove this debug
message.

Cc: Ray Ni <ray.ni@...>
Cc: Laszlo Ersek <lersek@...>

Eric Dong (2):
UefiCpuPkg/RegisterCpuFeaturesLib: Remove debug message.
UefiCpuPkg/PiSmmCpuDxeSmm: Remove debug message.

.../CpuFeaturesInitialize.c | 22 -------------------
UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 22 +------------------
2 files changed, 1 insertion(+), 43 deletions(-)
It seems to me that, after these patches are applied, no uses of
"ConsoleLogLock" remain, in either module (RegisterCpuFeaturesLib and
PiSmmCpuDxeSmm).

Can we eliminate the field from both modules? Otherwise we'll be left
with a useless, initialized spinlock, in each of these modules.

Thanks
Laszlo


[PATCH 4/4] MdeModulePkg/UfsPassThruDxe: Implement EDKII_UFS_HC_PLATFORM_PROTOCOL

Albecki, Mateusz
 

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

This commit adds EDKII_UFS_HC_PLATFORM_PROTOCOL implementation
in UfsPassThruDxe driver in version 1. Driver assumes that at
most one instance of the protocol exists in the system. Presence
of the protocol is not mandatory.

Cc: Hao A Wu <hao.a.wu@...>
Signed-off-by: Mateusz Albecki <mateusz.albecki@...>
---
MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c | 17 ++++++
MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h | 26 +++++++++
.../Bus/Ufs/UfsPassThruDxe/UfsPassThruDxe.inf | 3 +-
.../Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 68 ++++++++++++++++++++++
4 files changed, 113 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
index 09333c51d6..1559efe191 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
@@ -36,6 +36,7 @@ UFS_PASS_THRU_PRIVATE_DATA gUfsPassThruTemplate = {
0, // UfsHostController
0, // UfsHcBase
{0, 0}, // UfsHcInfo
+ {NULL, NULL}, // UfsHcDriverInterface
0, // TaskTag
0, // UtpTrlBase
0, // Nutrs
@@ -92,6 +93,8 @@ UFS_DEVICE_PATH mUfsDevicePathTemplate = {

UINT8 mUfsTargetId[TARGET_MAX_BYTES];

+GLOBAL_REMOVE_IF_UNREFERENCED EDKII_UFS_HC_PLATFORM_PROTOCOL *mUfsHcPlatform;
+
/**
Sends a SCSI Request Packet to a SCSI device that is attached to the SCSI channel. This function
supports both blocking I/O and nonblocking I/O. The blocking I/O functionality is required, and the
@@ -864,7 +867,21 @@ UfsPassThruDriverBindingStart (
Private->ExtScsiPassThru.Mode = &Private->ExtScsiPassThruMode;
Private->UfsHostController = UfsHc;
Private->UfsHcBase = UfsHcBase;
+ Private->Handle = Controller;
+ Private->UfsHcDriverInterface.UfsHcProtocol = UfsHc;
+ Private->UfsHcDriverInterface.UfsExecUicCommand = UfsHcDriverInterfaceExecUicCommand;
InitializeListHead (&Private->Queue);
+
+ //
+ // This has to be done before initializing UfsHcInfo or calling the UfsControllerInit
+ //
+ if (mUfsHcPlatform == NULL) {
+ Status = gBS->LocateProtocol (&gEdkiiUfsHcPlatformProtocolGuid, NULL, (VOID**)&mUfsHcPlatform);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_INFO, "No UfsHcPlatformProtocol present\n"));
+ }
+ }
+
Status = GetUfsHcInfo (Private);
if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_ERROR, "Failed to initialize UfsHcInfo\n"));
diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
index c511aa8c7a..cbc0c2126e 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
@@ -63,6 +63,7 @@ typedef struct _UFS_PASS_THRU_PRIVATE_DATA {
EDKII_UFS_HOST_CONTROLLER_PROTOCOL *UfsHostController;
UINTN UfsHcBase;
EDKII_UFS_HC_INFO UfsHcInfo;
+ EDKII_UFS_HC_DRIVER_INTERFACE UfsHcDriverInterface;

UINT8 TaskTag;

@@ -126,6 +127,13 @@ typedef struct {
UFS_PASS_THRU_SIG \
)

+#define UFS_PASS_THRU_PRIVATE_DATA_FROM_DRIVER_INTF(a) \
+ CR (a, \
+ UFS_PASS_THRU_PRIVATE_DATA, \
+ UfsHcDriverInterface, \
+ UFS_PASS_THRU_SIG \
+ )
+
typedef struct _UFS_DEVICE_MANAGEMENT_REQUEST_PACKET {
UINT64 Timeout;
VOID *DataBuffer;
@@ -959,6 +967,23 @@ UfsRwUfsAttribute (
IN OUT UINT32 *AttrSize
);

+/**
+ Execute UIC command.
+
+ @param[in] This Pointer to driver interface produced by the UFS controller.
+ @param[in, out] UicCommand Descriptor of the command that will be executed.
+
+ @retval EFI_SUCCESS Command executed successfully.
+ @retval EFI_INVALID_PARAMETER This or UicCommand is NULL.
+ @retval Others Command failed to execute.
+**/
+EFI_STATUS
+EFIAPI
+UfsHcDriverInterfaceExecUicCommand (
+ IN EDKII_UFS_HC_DRIVER_INTERFACE *This,
+ IN OUT EDKII_UIC_COMMAND *UicCommand
+ );
+
/**
Initializes UfsHcInfo field in private data.

@@ -975,5 +1000,6 @@ GetUfsHcInfo (
extern EFI_COMPONENT_NAME_PROTOCOL gUfsPassThruComponentName;
extern EFI_COMPONENT_NAME2_PROTOCOL gUfsPassThruComponentName2;
extern EFI_DRIVER_BINDING_PROTOCOL gUfsPassThruDriverBinding;
+extern EDKII_UFS_HC_PLATFORM_PROTOCOL *mUfsHcPlatform;

#endif
diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruDxe.inf b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruDxe.inf
index 24f5ea3a8f..92dc25714b 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruDxe.inf
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruDxe.inf
@@ -1,7 +1,7 @@
## @file
# Description file for the Universal Flash Storage (UFS) Pass Thru driver.
#
-# Copyright (c) 2014 - 2018, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2014 - 2019, Intel Corporation. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -54,6 +54,7 @@
gEfiExtScsiPassThruProtocolGuid ## BY_START
gEfiUfsDeviceConfigProtocolGuid ## BY_START
gEdkiiUfsHostControllerProtocolGuid ## TO_START
+ gEdkiiUfsHcPlatformProtocolGuid ## SOMETIMES_CONSUMES

[UserExtensions.TianoCore."ExtraFiles"]
UfsPassThruExtra.uni
diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
index 74be3efc41..e2c104b103 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
@@ -1835,6 +1835,14 @@ UfsEnableHostController (
EFI_STATUS Status;
UINT32 Data;

+ if (mUfsHcPlatform != NULL && mUfsHcPlatform->Callback != NULL) {
+ Status = mUfsHcPlatform->Callback (Private->Handle, EdkiiUfsHcPreHce, &Private->UfsHcDriverInterface);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Failure from platform driver during EdkiiUfsHcPreHce, Status = %r\n", Status));
+ return Status;
+ }
+ }
+
//
// UFS 2.0 spec section 7.1.1 - Host Controller Initialization
//
@@ -1878,6 +1886,14 @@ UfsEnableHostController (
return EFI_DEVICE_ERROR;
}

+ if (mUfsHcPlatform != NULL && mUfsHcPlatform->Callback != NULL) {
+ Status = mUfsHcPlatform->Callback (Private->Handle, EdkiiUfsHcPostHce, &Private->UfsHcDriverInterface);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Failure from platform driver during EdkiiUfsHcPostHce, Status = %r\n", Status));
+ return Status;
+ }
+ }
+
return EFI_SUCCESS;
}

@@ -1901,6 +1917,14 @@ UfsDeviceDetection (
UINT32 Data;
EDKII_UIC_COMMAND LinkStartupCommand;

+ if (mUfsHcPlatform != NULL && mUfsHcPlatform->Callback != NULL) {
+ Status = mUfsHcPlatform->Callback (Private->Handle, EdkiiUfsHcPreLinkStartup, &Private->UfsHcDriverInterface);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Failure from platform driver during EdkiiUfsHcPreLinkStartup, Status = %r\n", Status));
+ return Status;
+ }
+ }
+
//
// Start UFS device detection.
// Try up to 3 times for establishing data link with device.
@@ -1930,6 +1954,14 @@ UfsDeviceDetection (
}
}

+ if (mUfsHcPlatform != NULL && mUfsHcPlatform->Callback != NULL) {
+ Status = mUfsHcPlatform->Callback (Private->Handle, EdkiiUfsHcPostLinkStartup, &Private->UfsHcDriverInterface);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Failure from platform driver during EdkiiUfsHcPostLinkStartup, Status = %r\n", Status));
+ return Status;
+ }
+ }
+
return EFI_NOT_FOUND;
}

@@ -2350,6 +2382,34 @@ ProcessAsyncTaskList (
}
}

+/**
+ Execute UIC command.
+
+ @param[in] This Pointer to driver interface produced by the UFS controller.
+ @param[in, out] UicCommand Descriptor of the command that will be executed.
+
+ @retval EFI_SUCCESS Command executed successfully.
+ @retval EFI_INVALID_PARAMETER This or UicCommand is NULL.
+ @retval Others Command failed to execute.
+**/
+EFI_STATUS
+EFIAPI
+UfsHcDriverInterfaceExecUicCommand (
+ IN EDKII_UFS_HC_DRIVER_INTERFACE *This,
+ IN OUT EDKII_UIC_COMMAND *UicCommand
+ )
+{
+ UFS_PASS_THRU_PRIVATE_DATA *Private;
+
+ if (This == NULL || UicCommand == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Private = UFS_PASS_THRU_PRIVATE_DATA_FROM_DRIVER_INTF (This);
+
+ return UfsExecUicCommands (Private, UicCommand);
+}
+
/**
Initializes UfsHcInfo field in private data.

@@ -2380,6 +2440,14 @@ GetUfsHcInfo (

Private->UfsHcInfo.Capabilities = Data;

+ if (mUfsHcPlatform != NULL && mUfsHcPlatform->OverrideHcInfo != NULL) {
+ Status = mUfsHcPlatform->OverrideHcInfo (Private->Handle, &Private->UfsHcInfo);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Failure from platform on OverrideHcInfo, Status = %r\n", Status));
+ return Status;
+ }
+ }
+
return EFI_SUCCESS;
}

--
2.14.1.windows.1

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

Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.


[PATCH 3/4] MdeModulePkg/UfsPassThruDxe: Refactor private data to use EDKII_UFS_HC_INFO

Albecki, Mateusz
 

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

Private data has been refactored to use EDKII_UFS_HC_INFO structure
to store host controller capabilities and version
information. Getting host controller data has been moved
into single place and is done before host controller enable.

Cc: Hao A Wu <hao.a.wu@...>
Signed-off-by: Mateusz Albecki <mateusz.albecki@...>
---
MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c | 8 ++-
MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h | 15 +++++-
.../Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 57 ++++++++++++++--------
3 files changed, 57 insertions(+), 23 deletions(-)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
index 1518b251d8..09333c51d6 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c
@@ -1,6 +1,6 @@
/** @file

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

**/
@@ -35,7 +35,7 @@ UFS_PASS_THRU_PRIVATE_DATA gUfsPassThruTemplate = {
},
0, // UfsHostController
0, // UfsHcBase
- 0, // Capabilities
+ {0, 0}, // UfsHcInfo
0, // TaskTag
0, // UtpTrlBase
0, // Nutrs
@@ -865,6 +865,10 @@ UfsPassThruDriverBindingStart (
Private->UfsHostController = UfsHc;
Private->UfsHcBase = UfsHcBase;
InitializeListHead (&Private->Queue);
+ Status = GetUfsHcInfo (Private);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "Failed to initialize UfsHcInfo\n"));
+ }

//
// Initialize UFS Host Controller H/W.
diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
index b79be77709..c511aa8c7a 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
@@ -62,7 +62,7 @@ typedef struct _UFS_PASS_THRU_PRIVATE_DATA {
EFI_UFS_DEVICE_CONFIG_PROTOCOL UfsDevConfig;
EDKII_UFS_HOST_CONTROLLER_PROTOCOL *UfsHostController;
UINTN UfsHcBase;
- UINT32 Capabilities;
+ EDKII_UFS_HC_INFO UfsHcInfo;

UINT8 TaskTag;

@@ -959,6 +959,19 @@ UfsRwUfsAttribute (
IN OUT UINT32 *AttrSize
);

+/**
+ Initializes UfsHcInfo field in private data.
+
+ @param[in] Private Pointer to host controller private data.
+
+ @retval EFI_SUCCESS UfsHcInfo initialized successfully.
+ @retval Others Failed to initalize UfsHcInfo.
+**/
+EFI_STATUS
+GetUfsHcInfo (
+ IN UFS_PASS_THRU_PRIVATE_DATA *Private
+ );
+
extern EFI_COMPONENT_NAME_PROTOCOL gUfsPassThruComponentName;
extern EFI_COMPONENT_NAME2_PROTOCOL gUfsPassThruComponentName2;
extern EFI_DRIVER_BINDING_PROTOCOL gUfsPassThruDriverBinding;
diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
index 6ea27e473c..74be3efc41 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
@@ -731,7 +731,7 @@ UfsFindAvailableSlotInTrl (
return Status;
}

- Nutrs = (UINT8)((Private->Capabilities & UFS_HC_CAP_NUTRS) + 1);
+ Nutrs = (UINT8)((Private->UfsHcInfo.Capabilities & UFS_HC_CAP_NUTRS) + 1);

for (Index = 0; Index < Nutrs; Index++) {
if ((Data & (BIT0 << Index)) == 0) {
@@ -1754,7 +1754,7 @@ UfsAllocateAlignCommonBuffer (
BOOLEAN Is32BitAddr;
EDKII_UFS_HOST_CONTROLLER_PROTOCOL *UfsHc;

- if ((Private->Capabilities & UFS_HC_CAP_64ADDR) == UFS_HC_CAP_64ADDR) {
+ if ((Private->UfsHcInfo.Capabilities & UFS_HC_CAP_64ADDR) == UFS_HC_CAP_64ADDR) {
Is32BitAddr = FALSE;
} else {
Is32BitAddr = TRUE;
@@ -1947,7 +1947,6 @@ UfsInitTaskManagementRequestList (
IN UFS_PASS_THRU_PRIVATE_DATA *Private
)
{
- UINT32 Data;
UINT8 Nutmrs;
VOID *CmdDescHost;
EFI_PHYSICAL_ADDRESS CmdDescPhyAddr;
@@ -1961,17 +1960,10 @@ UfsInitTaskManagementRequestList (
CmdDescMapping = NULL;
CmdDescPhyAddr = 0;

- Status = UfsMmioRead32 (Private, UFS_HC_CAP_OFFSET, &Data);
- if (EFI_ERROR (Status)) {
- return Status;
- }
-
- Private->Capabilities = Data;
-
//
// Allocate and initialize UTP Task Management Request List.
//
- Nutmrs = (UINT8) (RShiftU64 ((Private->Capabilities & UFS_HC_CAP_NUTMRS), 16) + 1);
+ Nutmrs = (UINT8) (RShiftU64 ((Private->UfsHcInfo.Capabilities & UFS_HC_CAP_NUTMRS), 16) + 1);
Status = UfsAllocateAlignCommonBuffer (Private, Nutmrs * sizeof (UTP_TMRD), &CmdDescHost, &CmdDescPhyAddr, &CmdDescMapping);
if (EFI_ERROR (Status)) {
return Status;
@@ -2020,7 +2012,6 @@ UfsInitTransferRequestList (
IN UFS_PASS_THRU_PRIVATE_DATA *Private
)
{
- UINT32 Data;
UINT8 Nutrs;
VOID *CmdDescHost;
EFI_PHYSICAL_ADDRESS CmdDescPhyAddr;
@@ -2034,17 +2025,10 @@ UfsInitTransferRequestList (
CmdDescMapping = NULL;
CmdDescPhyAddr = 0;

- Status = UfsMmioRead32 (Private, UFS_HC_CAP_OFFSET, &Data);
- if (EFI_ERROR (Status)) {
- return Status;
- }
-
- Private->Capabilities = Data;
-
//
// Allocate and initialize UTP Transfer Request List.
//
- Nutrs = (UINT8)((Private->Capabilities & UFS_HC_CAP_NUTRS) + 1);
+ Nutrs = (UINT8)((Private->UfsHcInfo.Capabilities & UFS_HC_CAP_NUTRS) + 1);
Status = UfsAllocateAlignCommonBuffer (Private, Nutrs * sizeof (UTP_TRD), &CmdDescHost, &CmdDescPhyAddr, &CmdDescMapping);
if (EFI_ERROR (Status)) {
return Status;
@@ -2366,3 +2350,36 @@ ProcessAsyncTaskList (
}
}

+/**
+ Initializes UfsHcInfo field in private data.
+
+ @param[in] Private Pointer to host controller private data.
+
+ @retval EFI_SUCCESS UfsHcInfo initialized successfully.
+ @retval Others Failed to initalize UfsHcInfo.
+**/
+EFI_STATUS
+GetUfsHcInfo (
+ IN UFS_PASS_THRU_PRIVATE_DATA *Private
+ )
+{
+ UINT32 Data;
+ EFI_STATUS Status;
+
+ Status = UfsMmioRead32 (Private, UFS_HC_VER_OFFSET, &Data);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Private->UfsHcInfo.Version = Data;
+
+ Status = UfsMmioRead32 (Private, UFS_HC_CAP_OFFSET, &Data);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Private->UfsHcInfo.Capabilities = Data;
+
+ return EFI_SUCCESS;
+}
+
--
2.14.1.windows.1

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

Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.


[PATCH 1/4] MdeModulePkg: Add definition of the EDKII_UFS_HC_PLATFORM_PROTOCOL

Albecki, Mateusz
 

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

EDKII_UFS_HC_PLATFORM_PROTOCOL will allow the platform to inject
platform specific logic into standard UFS flows. Right now we
support callbacks pre and post host controller enable and pre
and post link startup. Provided callbacks allow the platform
driver to inject UIC programming after HCE is set which is
a standard initialization step covered by UFS specification as
well as cover some additional use cases during other calllbacks.
For instance platform driver may switch to fast mode after link
startup.

We also allow the platform to override host controller capabilities
and version which might be usefull to manage silicon bugs or
allow testign experimental features from new versions of the
specification.

Cc: Hao A Wu <hao.a.wu@...>
Signed-off-by: Mateusz Albecki <mateusz.albecki@...>
---
.../Include/Protocol/UfsHostControllerPlatform.h | 124 +++++++++++++++++++++
MdeModulePkg/MdeModulePkg.dec | 3 +
2 files changed, 127 insertions(+)
create mode 100644 MdeModulePkg/Include/Protocol/UfsHostControllerPlatform.h

diff --git a/MdeModulePkg/Include/Protocol/UfsHostControllerPlatform.h b/MdeModulePkg/Include/Protocol/UfsHostControllerPlatform.h
new file mode 100644
index 0000000000..0f6732a1f8
--- /dev/null
+++ b/MdeModulePkg/Include/Protocol/UfsHostControllerPlatform.h
@@ -0,0 +1,124 @@
+/** @file
+ EDKII_UFS_HC_PLATFORM_PROTOCOL definition.
+
+Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef __EDKII_UFS_HC_PLATFORM_PROTOCOL_H__
+#define __EDKII_UFS_HC_PLATFORM_PROTOCOL_H__
+
+#include <Protocol/UfsHostController.h>
+
+#define EDKII_UFS_HC_PLATFORM_PROTOCOL_VERSION 1
+
+extern EFI_GUID gEdkiiUfsHcPlatformProtocolGuid;
+
+typedef struct _EDKII_UFS_HC_PLATFORM_PROTOCOL EDKII_UFS_HC_PLATFORM_PROTOCOL;
+
+typedef struct _EDKII_UFS_HC_DRIVER_INTERFACE EDKII_UFS_HC_DRIVER_INTERFACE;
+
+typedef struct {
+ UINT32 Opcode;
+ UINT32 Arg1;
+ UINT32 Arg2;
+ UINT32 Arg3;
+} EDKII_UIC_COMMAND;
+
+/**
+ Execute UIC command
+
+ @param[in] This Pointer to driver interface produced by the UFS controller.
+ @param[in, out] UicCommand Descriptor of the command that will be executed.
+
+ @retval EFI_SUCCESS Command executed successfully.
+ @retval EFI_INVALID_PARAMETER This or UicCommand is NULL.
+ @retval Others Command failed to execute.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_UFS_EXEC_UIC_COMMAND) (
+ IN EDKII_UFS_HC_DRIVER_INTERFACE *This,
+ IN OUT EDKII_UIC_COMMAND *UicCommand
+);
+
+struct _EDKII_UFS_HC_DRIVER_INTERFACE {
+ ///
+ /// Protocol to accesss host controller MMIO and PCI registers.
+ ///
+ EDKII_UFS_HOST_CONTROLLER_PROTOCOL *UfsHcProtocol;
+ ///
+ /// Function implementing UIC command execution.
+ ///
+ EDKII_UFS_EXEC_UIC_COMMAND UfsExecUicCommand;
+};
+
+typedef struct {
+ UINT32 Capabilities;
+ UINT32 Version;
+} EDKII_UFS_HC_INFO;
+
+/**
+ Allows platform protocol to override host controller information
+
+ @param[in] ControllerHandle Handle of the UFS controller.
+ @param[in, out] HcInfo Pointer EDKII_UFS_HC_INFO associated with host controller.
+
+ @retval EFI_SUCCESS Function completed successfully.
+ @retval EFI_INVALID_PARAMETER HcInfo is NULL.
+ @retval Others Function failed to complete.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_UFS_HC_PLATFORM_OVERRIDE_HC_INFO) (
+ IN EFI_HANDLE ControllerHandle,
+ IN OUT EDKII_UFS_HC_INFO *HcInfo
+);
+
+typedef enum {
+ EdkiiUfsHcPreHce,
+ EdkiiUfsHcPostHce,
+ EdkiiUfsHcPreLinkStartup,
+ EdkiiUfsHcPostLinkStartup
+} EDKII_UFS_HC_PLATFORM_CALLBACK_PHASE;
+
+/**
+ Callback function for platform driver.
+
+ @param[in] ControllerHandle Handle of the UFS controller.
+ @param[in] CallbackPhase Specifies when the platform protocol is called
+ @param[in, out] CallbackData Data specific to the callback phase.
+ For PreHce and PostHce - EDKII_UFS_HC_DRIVER_INTERFACE.
+ For PreLinkStartup and PostLinkStartup - EDKII_UFS_HC_DRIVER_INTERFACE.
+
+ @retval EFI_SUCCESS Override function completed successfully.
+ @retval EFI_INVALID_PARAMETER CallbackPhase is invalid or CallbackData is NULL when phase expects valid data.
+ @retval Others Function failed to complete.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EDKII_UFS_HC_PLATFORM_CALLBACK) (
+ IN EFI_HANDLE ControllerHandle,
+ IN EDKII_UFS_HC_PLATFORM_CALLBACK_PHASE CallbackPhase,
+ IN OUT VOID *CallbackData
+);
+
+struct _EDKII_UFS_HC_PLATFORM_PROTOCOL {
+ ///
+ /// Version of the protocol.
+ ///
+ UINT32 Version;
+ ///
+ /// Allows platform driver to override host controller information.
+ ///
+ EDKII_UFS_HC_PLATFORM_OVERRIDE_HC_INFO OverrideHcInfo;
+ ///
+ /// Allows platform driver to implement platform specific flows
+ /// for host controller.
+ ///
+ EDKII_UFS_HC_PLATFORM_CALLBACK Callback;
+};
+
+#endif
+
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index c4139753d3..b663453c8b 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -558,6 +558,9 @@
## Include/Protocol/UfsHostController.h
gEdkiiUfsHostControllerProtocolGuid = { 0xebc01af5, 0x7a9, 0x489e, { 0xb7, 0xce, 0xdc, 0x8, 0x9e, 0x45, 0x9b, 0x2f } }

+ ## Include/Protocol/UfsHostControllerPlatform.h
+ gEdkiiUfsHcPlatformProtocolGuid = { 0x3d18ba13, 0xd9b1, 0x4dd4, {0xb9, 0x16, 0xd3, 0x07, 0x96, 0x53, 0x9e, 0xd8}}
+
## Include/Protocol/EsrtManagement.h
gEsrtManagementProtocolGuid = { 0xa340c064, 0x723c, 0x4a9c, { 0xa4, 0xdd, 0xd5, 0xb4, 0x7a, 0x26, 0xfb, 0xb0 }}

--
2.14.1.windows.1

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

Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.


[PATCH 2/4] MdeModulePkg/UfsPassThruDxe: Refactor UfsExecUicCommand function

Albecki, Mateusz
 

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

UfsExecUicCommand function has been refactored to allow
the caller to check the command results which is important
for commands such as UIC read.

Cc: Hao A Wu <hao.a.wu@...>
Signed-off-by: Mateusz Albecki <mateusz.albecki@...>
---
MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h | 3 +-
.../Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 47 ++++++++++++----------
2 files changed, 27 insertions(+), 23 deletions(-)

diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
index 9b68db5ffe..b79be77709 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h
@@ -1,6 +1,6 @@
/** @file

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

**/
@@ -13,6 +13,7 @@
#include <Protocol/ScsiPassThruExt.h>
#include <Protocol/UfsDeviceConfig.h>
#include <Protocol/UfsHostController.h>
+#include <Protocol/UfsHostControllerPlatform.h>

#include <Library/DebugLib.h>
#include <Library/UefiDriverEntryPoint.h>
diff --git a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
index 912d6f8202..6ea27e473c 100644
--- a/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
+++ b/MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c
@@ -2,7 +2,7 @@
UfsPassThruDxe driver is used to produce EFI_EXT_SCSI_PASS_THRU protocol interface
for upper layer application to execute UFS-supported SCSI cmds.

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

**/
@@ -1633,11 +1633,8 @@ Exit1:
/**
Send UIC command.

- @param[in] Private The pointer to the UFS_PASS_THRU_PRIVATE_DATA data structure.
- @param[in] UicOpcode The opcode of the UIC command.
- @param[in] Arg1 The value for 1st argument of the UIC command.
- @param[in] Arg2 The value for 2nd argument of the UIC command.
- @param[in] Arg3 The value for 3rd argument of the UIC command.
+ @param[in] Private The pointer to the UFS_PASS_THRU_PRIVATE_DATA data structure.
+ @param[in, out] UicCommand UIC command descriptor. On exit contains UIC command results.

@return EFI_SUCCESS Successfully execute this UIC command and detect attached UFS device.
@return EFI_DEVICE_ERROR Fail to execute this UIC command and detect attached UFS device.
@@ -1646,10 +1643,7 @@ Exit1:
EFI_STATUS
UfsExecUicCommands (
IN UFS_PASS_THRU_PRIVATE_DATA *Private,
- IN UINT8 UicOpcode,
- IN UINT32 Arg1,
- IN UINT32 Arg2,
- IN UINT32 Arg3
+ IN OUT EDKII_UIC_COMMAND *UicCommand
)
{
EFI_STATUS Status;
@@ -1675,17 +1669,17 @@ UfsExecUicCommands (
// only after all the UIC command argument registers (UICCMDARG1, UICCMDARG2 and UICCMDARG3)
// are set.
//
- Status = UfsMmioWrite32 (Private, UFS_HC_UCMD_ARG1_OFFSET, Arg1);
+ Status = UfsMmioWrite32 (Private, UFS_HC_UCMD_ARG1_OFFSET, UicCommand->Arg1);
if (EFI_ERROR (Status)) {
return Status;
}

- Status = UfsMmioWrite32 (Private, UFS_HC_UCMD_ARG2_OFFSET, Arg2);
+ Status = UfsMmioWrite32 (Private, UFS_HC_UCMD_ARG2_OFFSET, UicCommand->Arg2);
if (EFI_ERROR (Status)) {
return Status;
}

- Status = UfsMmioWrite32 (Private, UFS_HC_UCMD_ARG3_OFFSET, Arg3);
+ Status = UfsMmioWrite32 (Private, UFS_HC_UCMD_ARG3_OFFSET, UicCommand->Arg3);
if (EFI_ERROR (Status)) {
return Status;
}
@@ -1698,7 +1692,7 @@ UfsExecUicCommands (
return Status;
}

- Status = UfsMmioWrite32 (Private, UFS_HC_UIC_CMD_OFFSET, (UINT32)UicOpcode);
+ Status = UfsMmioWrite32 (Private, UFS_HC_UIC_CMD_OFFSET, UicCommand->Opcode);
if (EFI_ERROR (Status)) {
return Status;
}
@@ -1712,14 +1706,18 @@ UfsExecUicCommands (
return Status;
}

- if (UicOpcode != UfsUicDmeReset) {
- Status = UfsMmioRead32 (Private, UFS_HC_UCMD_ARG2_OFFSET, &Data);
+ if (UicCommand->Opcode != UfsUicDmeReset) {
+ Status = UfsMmioRead32 (Private, UFS_HC_UCMD_ARG2_OFFSET, &UicCommand->Arg2);
if (EFI_ERROR (Status)) {
return Status;
}
- if ((Data & 0xFF) != 0) {
+ Status = UfsMmioRead32 (Private, UFS_HC_UCMD_ARG3_OFFSET, &UicCommand->Arg3);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+ if ((UicCommand->Arg2 & 0xFF) != 0) {
DEBUG_CODE_BEGIN();
- DumpUicCmdExecResult (UicOpcode, (UINT8)(Data & 0xFF));
+ DumpUicCmdExecResult ((UINT8)UicCommand->Opcode, (UINT8)(UicCommand->Arg2 & 0xFF));
DEBUG_CODE_END();
return EFI_DEVICE_ERROR;
}
@@ -1898,16 +1896,21 @@ UfsDeviceDetection (
IN UFS_PASS_THRU_PRIVATE_DATA *Private
)
{
- UINTN Retry;
- EFI_STATUS Status;
- UINT32 Data;
+ UINTN Retry;
+ EFI_STATUS Status;
+ UINT32 Data;
+ EDKII_UIC_COMMAND LinkStartupCommand;

//
// Start UFS device detection.
// Try up to 3 times for establishing data link with device.
//
for (Retry = 0; Retry < 3; Retry++) {
- Status = UfsExecUicCommands (Private, UfsUicDmeLinkStartup, 0, 0, 0);
+ LinkStartupCommand.Opcode = UfsUicDmeLinkStartup;
+ LinkStartupCommand.Arg1 = 0;
+ LinkStartupCommand.Arg2 = 0;
+ LinkStartupCommand.Arg3 = 0;
+ Status = UfsExecUicCommands (Private, &LinkStartupCommand);
if (EFI_ERROR (Status)) {
return EFI_DEVICE_ERROR;
}
--
2.14.1.windows.1

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

Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.


[PATCH 0/4] Add EDKII_UFS_HC_PLATFORM_PROTOCOL to support platform specific programming of UFS host controllers

Albecki, Mateusz
 

To cover additional host controller programming mentioned in the UFS specification
we have added an additional protocol that allows the UEFI driver to give control
to platform driver. This allows the platform to perform any additional steps
needed for the stable operation.

Test coverage:
Tested on platform with UFS 2.1 host controller with Samsung UFS2.0 part with 3 LUs enabled
All LUs have been enumerated in boot manager.
Tested that enumeration works without platform protocol installed(on host controller that can support it)
Tested that enumeration works with platform protocol installed and with additional programming steps after
link startup(power mode change to GEAR2).

Cc: Hao A Wu <hao.a.wu@...>

Mateusz Albecki (4):
MdeModulePkg: Add definition of the EDKII_UFS_HC_PLATFORM_PROTOCOL
MdeModulePkg/UfsPassThruDxe: Refactor UfsExecUicCommand function
MdeModulePkg/UfsPassThruDxe: Refactor private data to use
EDKII_UFS_HC_INFO
MdeModulePkg/UfsPassThruDxe: Implement EDKII_UFS_HC_PLATFORM_PROTOCOL

MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.c | 25 ++-
MdeModulePkg/Bus/Ufs/UfsPassThruDxe/UfsPassThru.h | 44 +++++-
.../Bus/Ufs/UfsPassThruDxe/UfsPassThruDxe.inf | 3 +-
.../Bus/Ufs/UfsPassThruDxe/UfsPassThruHci.c | 172 ++++++++++++++++-----
.../Include/Protocol/UfsHostControllerPlatform.h | 124 +++++++++++++++
MdeModulePkg/MdeModulePkg.dec | 3 +
6 files changed, 324 insertions(+), 47 deletions(-)
create mode 100644 MdeModulePkg/Include/Protocol/UfsHostControllerPlatform.h

--
2.14.1.windows.1

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

Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.


Re: UefiCpuPkg/PiSmmCpuDxeSmm: don't free page table pages that are required to handle current page fault

Laszlo Ersek
 

On 08/05/19 23:57, Krzysztof Rusocki wrote:
From: Damian Nikodem <damian.nikodem@...>

Reclaim may free page table pages that are required to handle current page
fault. This causes a page leak, and, after sufficent number of specific
page fault+reclaim pairs, we run out of reclaimable pages and hit:

ASSERT (MinAcc != (UINT64)-1);

To remedy, prevent pages essential to handling current page fault:
(1) from being considered as reclaim candidates (first reclaim phase)
(2) from being freed as part of "branch cleanup" (second reclaim phase)

Signed-off-by: Damian Nikodem <damian.nikodem@...>
Cc: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Laszlo Ersek <lersek@...>
Cc: Krzysztof Rusocki <krzysztof.rusocki@...>

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
index a3b62f7787..f11323f439 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
@@ -543,6 +543,11 @@ ReclaimPages (
UINT64 *ReleasePageAddress;
IA32_CR4 Cr4;
BOOLEAN Enable5LevelPaging;
+ UINT64 PFAddress;
+ UINT64 PFAddressPml5Index;
+ UINT64 PFAddressPml4Index;
+ UINT64 PFAddressPdptIndex;
+ UINT64 PFAddressPdtIndex;

Pml4 = NULL;
Pdpt = NULL;
@@ -554,6 +559,11 @@ ReclaimPages (
MinPdt = (UINTN)-1;
Acc = 0;
ReleasePageAddress = 0;
+ PFAddress = AsmReadCr2 ();
+ PFAddressPml5Index = BitFieldRead64 (PFAddress, 48, 48 + 8);
+ PFAddressPml4Index = BitFieldRead64 (PFAddress, 39, 39 + 8);
+ PFAddressPdptIndex = BitFieldRead64 (PFAddress, 30, 30 + 8);
+ PFAddressPdtIndex = BitFieldRead64 (PFAddress, 21, 21 + 8);

Cr4.UintN = AsmReadCr4 ();
Enable5LevelPaging = (BOOLEAN) (Cr4.Bits.LA57 == 1);
@@ -628,40 +638,46 @@ ReclaimPages (
// we will find the entry has the smallest access record value
//
PDPTEIgnore = TRUE;
- Acc = GetAndUpdateAccNum (Pdt + PdtIndex);
+ if (PdtIndex != PFAddressPdtIndex || PdptIndex != PFAddressPdptIndex ||
+ Pml4Index != PFAddressPml4Index || Pml5Index != PFAddressPml5Index) {
+ Acc = GetAndUpdateAccNum (Pdt + PdtIndex);
+ if (Acc < MinAcc) {
+ //
+ // If the PD entry has the smallest access record value,
+ // save the Page address to be released
+ //
+ MinAcc = Acc;
+ MinPml5 = Pml5Index;
+ MinPml4 = Pml4Index;
+ MinPdpt = PdptIndex;
+ MinPdt = PdtIndex;
+ ReleasePageAddress = Pdt + PdtIndex;
+ }
+ }
+ }
+ }
+ if (!PDPTEIgnore) {
+ //
+ // If this PDPT entry has no PDT entries pointer to 4 KByte pages,
+ // it should only has the entries point to 2 MByte Pages
+ //
+ if (PdptIndex != PFAddressPdptIndex || Pml4Index != PFAddressPml4Index ||
+ Pml5Index != PFAddressPml5Index) {
+ Acc = GetAndUpdateAccNum (Pdpt + PdptIndex);
if (Acc < MinAcc) {
//
- // If the PD entry has the smallest access record value,
+ // If the PDPT entry has the smallest access record value,
// save the Page address to be released
//
MinAcc = Acc;
MinPml5 = Pml5Index;
MinPml4 = Pml4Index;
MinPdpt = PdptIndex;
- MinPdt = PdtIndex;
- ReleasePageAddress = Pdt + PdtIndex;
+ MinPdt = (UINTN)-1;
+ ReleasePageAddress = Pdpt + PdptIndex;
}
}
}
- if (!PDPTEIgnore) {
- //
- // If this PDPT entry has no PDT entries pointer to 4 KByte pages,
- // it should only has the entries point to 2 MByte Pages
- //
- Acc = GetAndUpdateAccNum (Pdpt + PdptIndex);
- if (Acc < MinAcc) {
- //
- // If the PDPT entry has the smallest access record value,
- // save the Page address to be released
- //
- MinAcc = Acc;
- MinPml5 = Pml5Index;
- MinPml4 = Pml4Index;
- MinPdpt = PdptIndex;
- MinPdt = (UINTN)-1;
- ReleasePageAddress = Pdpt + PdptIndex;
- }
- }
}
}
if (!PML4EIgnore) {
@@ -669,18 +685,20 @@ ReclaimPages (
// If PML4 entry has no the PDPT entry pointer to 2 MByte pages,
// it should only has the entries point to 1 GByte Pages
//
- Acc = GetAndUpdateAccNum (Pml4 + Pml4Index);
- if (Acc < MinAcc) {
- //
- // If the PML4 entry has the smallest access record value,
- // save the Page address to be released
- //
- MinAcc = Acc;
- MinPml5 = Pml5Index;
- MinPml4 = Pml4Index;
- MinPdpt = (UINTN)-1;
- MinPdt = (UINTN)-1;
- ReleasePageAddress = Pml4 + Pml4Index;
+ if (Pml4Index != PFAddressPml4Index || Pml5Index != PFAddressPml5Index) {
+ Acc = GetAndUpdateAccNum (Pml4 + Pml4Index);
+ if (Acc < MinAcc) {
+ //
+ // If the PML4 entry has the smallest access record value,
+ // save the Page address to be released
+ //
+ MinAcc = Acc;
+ MinPml5 = Pml5Index;
+ MinPml4 = Pml4Index;
+ MinPdpt = (UINTN)-1;
+ MinPdt = (UINTN)-1;
+ ReleasePageAddress = Pml4 + Pml4Index;
+ }
}
}
}
@@ -708,7 +726,8 @@ ReclaimPages (
Pml4 = (UINT64 *) (UINTN) (Pml5[MinPml5] & gPhyMask);
Pdpt = (UINT64*)(UINTN)(Pml4[MinPml4] & ~mAddressEncMask & gPhyMask);
SubEntriesNum = GetSubEntriesNum(Pdpt + MinPdpt);
- if (SubEntriesNum == 0) {
+ if (SubEntriesNum == 0 &&
+ (MinPdpt != PFAddressPdptIndex || MinPml4 != PFAddressPml4Index || MinPml5 != PFAddressPml5Index)) {
//
// Release the empty Page Directory table if there was no more 4 KByte Page Table entry
// clear the Page directory entry
@@ -724,7 +743,7 @@ ReclaimPages (
//
// Update the sub-entries filed in PDPT entry and exit
//
- SetSubEntriesNum (Pdpt + MinPdpt, SubEntriesNum - 1);
+ SetSubEntriesNum (Pdpt + MinPdpt, (SubEntriesNum - 1) & 0x1FF);
break;
}
if (MinPdpt != (UINTN)-1) {
@@ -732,7 +751,7 @@ ReclaimPages (
// One 2MB Page Table is released or Page Directory table is released, check the PML4 entry
//
SubEntriesNum = GetSubEntriesNum (Pml4 + MinPml4);
- if (SubEntriesNum == 0) {
+ if (SubEntriesNum == 0 && (MinPml4 != PFAddressPml4Index || MinPml5 != PFAddressPml5Index)) {
//
// Release the empty PML4 table if there was no more 1G KByte Page Table entry
// clear the Page directory entry
@@ -745,7 +764,7 @@ ReclaimPages (
//
// Update the sub-entries filed in PML4 entry and exit
//
- SetSubEntriesNum (Pml4 + MinPml4, SubEntriesNum - 1);
+ SetSubEntriesNum (Pml4 + MinPml4, (SubEntriesNum - 1) & 0x1FF);
break;
}
//
@@ -918,7 +937,7 @@ SmiDefaultPFHandler (
PageTable[PTIndex] = ((PFAddress | mAddressEncMask) & gPhyMask & ~((1ull << EndBit) - 1)) |
PageAttribute | IA32_PG_A | PAGE_ATTRIBUTE_BITS;
if (UpperEntry != NULL) {
- SetSubEntriesNum (UpperEntry, GetSubEntriesNum (UpperEntry) + 1);
+ SetSubEntriesNum (UpperEntry, (GetSubEntriesNum (UpperEntry) + 1) & 0x1FF);
}
//
// Get the next page address if we need to create more page tables
--------------------------------------------------------------------

Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i moze zawierac informacje poufne. W razie przypadkowego otrzymania tej wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). If you are not the intended recipient, please contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.
got no capacity left for this; please go ahead with the reviews you
already got

thanks
Laszlo


Re: [PATCH] MdePkg: Add MmAccess and MmControl definition.

Laszlo Ersek
 

On 08/06/19 12:08, Ni, Ray wrote:
Laszlo,
Do you want to avoid touching the OvmfPkg due the file removal in MdeModulePkg?
My main problem with this *specific* update proposal is that the
identifiers (type names and such) that would be subject to removal were
publicly specified in earlier PI spec releases.

I don't suggest that we maintain two parallel sets of typedefs and such
in edk2. It's fine to keep the new "MM" nomenclature the main one. But
we should keep the "SMM" set of terms too, as a thin wrapper, if that's
technically feasible.

If we remove "SMM", that will not only affect OvmfPkg unpleasantly, but
a bunch of other, out-of-tree code. And the reason I suddenly care about
out-of-tree code is that such code was written against a public industry
spec (= earlier versions of the PI spec).

I don't think that the simplicity that would come from removing "SMM"
altogether, is well-balanced against the pain that it would cause for
platforms.

Earlier this was solved in the following commits:
- 07c6a47e70ba ("MdePkg: Add new definitions for Management Mode.",
2017-08-29)
- 2f208e59e4b9 ("MdePkg: Reference new definitions for Management
Mode.", 2017-08-29)

Thanks
Laszlo

I prefer to remove the files in MdeModulePkg to avoid future confusion.

Thanks,
Ray

-----Original Message-----
From: Chen, Marc W
Sent: Tuesday, August 6, 2019 4:57 PM
To: devel@edk2.groups.io; lersek@...; Ni, Ray <ray.ni@...>;
Gao, Liming <liming.gao@...>
Cc: Kinney, Michael D <michael.d.kinney@...>
Subject: RE: [edk2-devel] [PATCH] MdePkg: Add MmAccess and MmControl
definition.

Hi Liming and Ray

Do you also agree?

Thanks,
Marc

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo
Ersek
Sent: Friday, August 2, 2019 10:14 AM
To: devel@edk2.groups.io; Chen, Marc W <marc.w.chen@...>; Ni,
Ray <ray.ni@...>; Gao, Liming <liming.gao@...>
Cc: Kinney, Michael D <michael.d.kinney@...>
Subject: Re: [edk2-devel] [PATCH] MdePkg: Add MmAccess and
MmControl
definition.

On 08/01/19 12:15, Marc W Chen wrote:
Yes, my purpose is to avoid platform code update if the package is
allowed
to use MdeModulePkg like OvmfPkg.
For those packages that cannot depend on MdeModulePkg must change
to
use new definition.

Agreed.

Thanks!
Laszlo

-----Original Message-----
From: Ni, Ray
Sent: Thursday, August 1, 2019 6:04 PM
To: Chen, Marc W <marc.w.chen@...>; Gao, Liming
<liming.gao@...>; devel@edk2.groups.io
Cc: Kinney, Michael D <michael.d.kinney@...>
Subject: RE: [edk2-devel] [PATCH] MdePkg: Add MmAccess and
MmControl
definition.

Marc,
Is the purpose to avoid platform code update with the wrapper
header
file in
MdeModulePkg?
Certain platforms they may require to not depend on MdeModulePkg
but just depend on MdePkg.
Having SmmAccess.h in MdeModulePkg doesn't help.

It also bring confusing.

Thanks,
Ray

-----Original Message-----
From: Chen, Marc W
Sent: Thursday, August 1, 2019 4:53 PM
To: Gao, Liming <liming.gao@...>; devel@edk2.groups.io
Cc: Kinney, Michael D <michael.d.kinney@...>; Ni, Ray
<ray.ni@...>
Subject: RE: [edk2-devel] [PATCH] MdePkg: Add MmAccess and
MmControl
definition.

Hi Liming

Another thought, do you think it is ok to keep SmmAccess.h in
MdeModulePkg? We can use #define and typedef to convert
MmAccess
to
SmmAccess, just like current SmmAccess Protocol in MdePkg.

Thanks,
Marc

-----Original Message-----
From: Chen, Marc W
Sent: Thursday, August 1, 2019 4:48 PM
To: Gao, Liming <liming.gao@...>; devel@edk2.groups.io
Cc: Kinney, Michael D <michael.d.kinney@...>; Ni, Ray
<ray.ni@...>
Subject: RE: [edk2-devel] [PATCH] MdePkg: Add MmAccess and
MmControl
definition.

Hi Liming

Since there are multiple packages are still using SmmAccess.h, we
need to change all packages to use MmAccess.h instead of
SmmAccess.h first, then clean up SmmAccess.h from MdeModulePkg
will be the last step.
Below are the packages that has "include <Ppi/SmmAccess.h>" for
your reference.
* Edk2 repo:
o MdeModulePkg
o OvmfPkg
o UefiCpuPkg
* Edk2Platform repo:
o MinPlatformPkg
o QuarkSocPkg

Thanks,
Marc

-----Original Message-----
From: Gao, Liming
Sent: Thursday, August 1, 2019 4:42 PM
To: devel@edk2.groups.io; Chen, Marc W <marc.w.chen@...>
Cc: Kinney, Michael D <michael.d.kinney@...>; Ni, Ray
<ray.ni@...>
Subject: RE: [edk2-devel] [PATCH] MdePkg: Add MmAccess and
MmControl
definition.

Marc:
The change is good. Reviewed-by: Liming Gao
<liming.gao@...>

Besides, I see BZ also mention to remove the one in
MdeModulePkg.
Have you the following patches for the change in MdeModulePkg?

Thanks
Liming
-----Original Message-----
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On
Behalf
Of Marc W Chen
Sent: Monday, July 29, 2019 12:26 PM
To: devel@edk2.groups.io
Cc: Kinney, Michael D <michael.d.kinney@...>; Gao, Liming
<liming.gao@...>; Ni, Ray <ray.ni@...>
Subject: [edk2-devel] [PATCH] MdePkg: Add MmAccess and
MmControl
definition.

EFI MmAccess and MmControl PPIs are defined in the PI 1.5
specification.

Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <liming.gao@...>
Cc: Ray Ni <ray.ni@...>
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2023
Signed-off-by: Marc W Chen <marc.w.chen@...>
---
MdePkg/Include/Ppi/MmAccess.h | 155
+++++++++++++++++++++++++++++++++++++++++
MdePkg/Include/Ppi/MmControl.h | 90
++++++++++++++++++++++++
MdePkg/MdePkg.dec | 6 ++
3 files changed, 251 insertions(+) create mode 100644
MdePkg/Include/Ppi/MmAccess.h create mode 100644
MdePkg/Include/Ppi/MmControl.h

diff --git a/MdePkg/Include/Ppi/MmAccess.h
b/MdePkg/Include/Ppi/MmAccess.h new file mode 100644 index
0000000000..636e7288a0
--- /dev/null
+++ b/MdePkg/Include/Ppi/MmAccess.h
@@ -0,0 +1,155 @@
+/** @file
+ EFI MM Access PPI definition.
+
+ This PPI is used to control the visibility of the MMRAM on
+ the
platform.
+ The EFI_PEI_MM_ACCESS_PPI abstracts the location and
+ characteristics
of
MMRAM. The
+ principal functionality found in the memory controller
+ includes the
following:
+ - Exposing the MMRAM to all non-MM agents, or the "open"
+ state
+ - Shrouding the MMRAM to all but the MM agents, or the
"closed"
+ state
+ - Preserving the system integrity, or "locking" the MMRAM,
+ such that
the
settings cannot be
+ perturbed by either boot service or runtime agents
+
+ Copyright (c) 2019, Intel Corporation. All rights
+ reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Revision Reference:
+ This PPI is introduced in PI Version 1.5.
+
+**/
+
+#ifndef _MM_ACCESS_PPI_H_
+#define _MM_ACCESS_PPI_H_
+
+#define EFI_PEI_MM_ACCESS_PPI_GUID \
+ { 0x268f33a9, 0xcccd, 0x48be, { 0x88, 0x17, 0x86, 0x5, 0x3a,
+0xc3, 0x2e,
0xd6 }}
+
+typedef struct _EFI_PEI_MM_ACCESS_PPI
EFI_PEI_MM_ACCESS_PPI;
+
+/**
+ Opens the MMRAM area to be accessible by a PEIM.
+
+ This function "opens" MMRAM so that it is visible while not
+ inside of
MM.
The function should
+ return EFI_UNSUPPORTED if the hardware does not support
+ hiding of
MMRAM. The function
+ should return EFI_DEVICE_ERROR if the MMRAM configuration
is
locked.
+
+ @param PeiServices An indirect pointer to the PEI Services
Table
published by the PEI Foundation.
+ @param This The EFI_PEI_MM_ACCESS_PPI instance.
+ @param DescriptorIndex The region of MMRAM to Open.
+
+ @retval EFI_SUCCESS The operation was successful.
+ @retval EFI_UNSUPPORTED The system does not support
opening
and
closing of MMRAM.
+ @retval EFI_DEVICE_ERROR MMRAM cannot be opened,
perhaps
because it is locked.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EFI_PEI_MM_OPEN)(
+ IN EFI_PEI_SERVICES **PeiServices,
+ IN EFI_PEI_MM_ACCESS_PPI *This,
+ IN UINTN DescriptorIndex
+ );
+
+/**
+ Inhibits access to the MMRAM.
+
+ This function "closes" MMRAM so that it is not visible while
+ outside of
MM.
The function should
+ return EFI_UNSUPPORTED if the hardware does not support
+ hiding of
MMRAM.
+
+ @param PeiServices An indirect pointer to the PEI Services
Table
published by the PEI Foundation.
+ @param This The EFI_PEI_MM_ACCESS_PPI instance.
+ @param DescriptorIndex The region of MMRAM to Close.
+
+ @retval EFI_SUCCESS The operation was successful.
+ @retval EFI_UNSUPPORTED The system does not support
opening
and
closing of MMRAM.
+ @retval EFI_DEVICE_ERROR MMRAM cannot be closed.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EFI_PEI_MM_CLOSE)(
+ IN EFI_PEI_SERVICES **PeiServices,
+ IN EFI_PEI_MM_ACCESS_PPI *This,
+ IN UINTN DescriptorIndex
+ );
+
+/**
+ This function prohibits access to the MMRAM region. This
+function is
usually
implemented such
+ that it is a write-once operation.
+
+ @param PeiServices An indirect pointer to the PEI Services
Table
published by the PEI Foundation.
+ @param This The EFI_PEI_MM_ACCESS_PPI instance.
+ @param DescriptorIndex The region of MMRAM to Lock.
+
+ @retval EFI_SUCCESS The operation was successful.
+ @retval EFI_UNSUPPORTED The system does not support
opening
and
closing of MMRAM.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EFI_PEI_MM_LOCK)(
+ IN EFI_PEI_SERVICES **PeiServices,
+ IN EFI_PEI_MM_ACCESS_PPI *This,
+ IN UINTN DescriptorIndex
+ );
+
+/**
+ Queries the memory controller for the possible regions that
+will support
MMRAM.
+
+ This function describes the MMRAM regions.
+ This data structure forms the contract between the
MM_ACCESS
and
MM_IPL drivers. There is an
+ ambiguity when any MMRAM region is remapped. For example,
on
some
chipsets, some MMRAM
+ regions can be initialized at one physical address but is
+ later accessed at
another processor address.
+ There is currently no way for the MM IPL driver to know that
+ it must use
two different addresses
+ depending on what it is trying to do. As a result, initial
+ configuration and
loading can use the
+ physical address PhysicalStart while MMRAM is open. However,
+ once
the
region has been
+ closed and needs to be accessed by agents in MM, the
+ CpuStart address
must be used.
+ This PPI publishes the available memory that the chipset can
+ shroud for
the
use of installing code.
+ These regions serve the dual purpose of describing which
+ regions have
been open, closed, or locked.
+ In addition, these regions may include overlapping memory
+ ranges,
depending on the chipset
+ implementation. The latter might include a chipset that
+ supports T-SEG,
where memory near the top
+ of the physical DRAM can be allocated for MMRAM too.
+ The key thing to note is that the regions that are described
+ by the PPI
are
a
subset of the capabilities
+ of the hardware.
+
+ @param PeiServices An indirect pointer to the PEI Services
Table
published by the PEI Foundation.
+ @param This The EFI_PEI_MM_ACCESS_PPI instance.
+ @param MmramMapSize A pointer to the size, in bytes, of
the
MmramMemoryMap buffer. On input, this value is
+ the size of the buffer that
+ is allocated by the caller. On
output, it is the size of the
+ buffer that was returned by
+ the firmware if the buffer
was
large enough, or, if the
+ buffer was too small, the
+ size of the buffer that is
needed to
contain the map.
+ @param MmramMap A pointer to the buffer in which
firmware
places the current memory map. The map is
+ an array of
+ EFI_MMRAM_DESCRIPTORs
+
+ @retval EFI_SUCCESS The chipset supported the given
resource.
+ @retval EFI_BUFFER_TOO_SMALL The MmramMap parameter
was
too
small. The current
+ buffer size needed to hold
+ the memory map is
returned
in
+ MmramMapSize.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EFI_PEI_MM_CAPABILITIES)(
+ IN EFI_PEI_SERVICES **PeiServices,
+ IN EFI_PEI_MM_ACCESS_PPI *This,
+ IN OUT UINTN *MmramMapSize,
+ IN OUT EFI_MMRAM_DESCRIPTOR *MmramMap
+ );
+
+///
+/// EFI MM Access PPI is used to control the visibility of
+the MMRAM on
the
platform.
+/// It abstracts the location and characteristics of MMRAM.
+The platform
should report
+/// all MMRAM via EFI_PEI_MM_ACCESS_PPI. The expectation is
that
+the
north bridge or
+/// memory controller would publish this PPI.
+///
+struct _EFI_PEI_MM_ACCESS_PPI {
+ EFI_PEI_MM_OPEN Open;
+ EFI_PEI_MM_CLOSE Close;
+ EFI_PEI_MM_LOCK Lock;
+ EFI_PEI_MM_CAPABILITIES GetCapabilities;
+ BOOLEAN LockState;
+ BOOLEAN OpenState;
+};
+
+extern EFI_GUID gEfiPeiMmAccessPpiGuid;
+
+#endif
diff --git a/MdePkg/Include/Ppi/MmControl.h
b/MdePkg/Include/Ppi/MmControl.h new file mode 100644 index
0000000000..983ed95cd5
--- /dev/null
+++ b/MdePkg/Include/Ppi/MmControl.h
@@ -0,0 +1,90 @@
+/** @file
+ EFI MM Control PPI definition.
+
+ This PPI is used initiate synchronous MMI activations. This
+ PPI could be
published by a processor
+ driver to abstract the MMI IPI or a driver which abstracts
+ the ASIC that
is
supporting the APM port.
+ Because of the possibility of performing MMI IPI
+ transactions, the ability
to
generate this event
+ from a platform chipset agent is an optional capability for
+ both
+ IA-32
and
x64-based systems.
+
+ Copyright (c) 2019, Intel Corporation. All rights
+ reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Revision Reference:
+ This PPI is introduced in PI Version 1.5.
+
+**/
+
+
+#ifndef _MM_CONTROL_PPI_H_
+#define _MM_CONTROL_PPI_H_
+
+#define EFI_PEI_MM_CONTROL_PPI_GUID \
+ { 0x61c68702, 0x4d7e, 0x4f43, 0x8d, 0xef, 0xa7, 0x43, 0x5,
+0xce, 0x74,
0xc5 }
+
+typedef struct _EFI_PEI_MM_CONTROL_PPI
EFI_PEI_MM_CONTROL_PPI;
+
+/**
+ Invokes PPI activation from the PI PEI environment.
+
+ @param PeiServices An indirect pointer to the PEI Services
Table
published by the PEI Foundation.
+ @param This The PEI_MM_CONTROL_PPI instance.
+ @param ArgumentBuffer The value passed to the MMI
handler.
This
value corresponds to the
+ SwMmiInputValue in the
+ RegisterContext parameter
for
the
Register()
+ function in the
+ EFI_MM_SW_DISPATCH_PROTOCOL
and
in
the Context parameter
+ in the call to the DispatchFunction
+ @param ArgumentBufferSize The size of the data passed in
ArgumentBuffer or NULL if ArgumentBuffer is NULL.
+ @param Periodic An optional mechanism to periodically
repeat
activation.
+ @param ActivationInterval An optional parameter to repeat at
this
period
one
+ time or, if the Periodic Boolean is set, periodically.
+
+ @retval EFI_SUCCESS The MMI has been engendered.
+ @retval EFI_DEVICE_ERROR The timing is unsupported.
+ @retval EFI_INVALID_PARAMETER The activation period is
unsupported.
+ @retval EFI_NOT_STARTED The MM base service has not
been
initialized.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EFI_PEI_MM_ACTIVATE) (
+ IN EFI_PEI_SERVICES **PeiServices,
+ IN EFI_PEI_MM_CONTROL_PPI * This,
+ IN OUT INT8 *ArgumentBuffer OPTIONAL,
+ IN OUT UINTN *ArgumentBufferSize
OPTIONAL,
+ IN BOOLEAN Periodic OPTIONAL,
+ IN UINTN ActivationInterval OPTIONAL
+ );
+
+/**
+ Clears any system state that was created in response to the
Trigger()
call.
+
+ @param PeiServices General purpose services available to
every
PEIM.
+ @param This The PEI_MM_CONTROL_PPI instance.
+ @param Periodic Optional parameter to repeat at this
period
one
+ time or, if the Periodic Boolean is set, periodically.
+
+ @retval EFI_SUCCESS The MMI has been engendered.
+ @retval EFI_DEVICE_ERROR The source could not be cleared.
+ @retval EFI_INVALID_PARAMETER The service did not support
+ the
Periodic
input argument.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *PEI_MM_DEACTIVATE) (
+ IN EFI_PEI_SERVICES **PeiServices,
+ IN EFI_PEI_MM_CONTROL_PPI * This,
+ IN BOOLEAN Periodic OPTIONAL
+ );
+
+///
+/// The EFI_PEI_MM_CONTROL_PPI is produced by a PEIM. It
provides
an
abstraction of the
+/// platform hardware that generates an MMI. There are often
+I/O ports
that, when accessed, will
+/// generate the MMI. Also, the hardware optionally supports
+the
periodic
generation of these signals.
+///
+struct _PEI_MM_CONTROL_PPI {
+ PEI_MM_ACTIVATE Trigger;
+ PEI_MM_DEACTIVATE Clear;
+};
+
+extern EFI_GUID gEfiPeiMmControlPpiGuid;
+
+#endif
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index
b382efd578..34e0f39395 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -925,6 +925,12 @@
## Include/Ppi/SecHobData.h
gEfiSecHobDataPpiGuid = { 0x3ebdaf20, 0x6667, 0x40d8, {0xb4,
0xee,
0xf5,
0x99, 0x9a, 0xc1, 0xb7, 0x1f } }

+ ## Include/Ppi/MmAccess.h
+ gEfiPeiMmAccessPpiGuid = { 0x268f33a9, 0xcccd, 0x48be,
{ 0x88,
0x17,
0x86, 0x5, 0x3a, 0xc3, 0x2e, 0xd6 }}
+
+ ## Include/Ppi/MmControl.h
+ gEfiPeiMmControlPpiGuid = { 0x61c68702, 0x4d7e, 0x4f43,
{ 0x8d,
0xef,
0xa7, 0x43, 0x5, 0xce, 0x74, 0xc5 }}
+
#
# PPIs defined in PI 1.7.
#
--
2.16.2.windows.1






Re: [PATCH v4 35/35] OvmfPkg/OvmfXen: use RealTimeClockRuntimeDxe from EmbeddedPkg

Roger Pau Monné
 

On Mon, Jul 29, 2019 at 04:39:44PM +0100, Anthony PERARD wrote:
A Xen PVH guest doesn't have a RTC that OVMF would expect, so
PcatRealTimeClockRuntimeDxe fails to initialize and prevent the
firmware from finish to boot. To prevent that, we will use
XenRealTimeClockLib which simply always return the same time.
This will work on both Xen PVH and HVM guests.
Not that this is not correct, but what's the point in requiring a
clock if it can be faked by always returning the same value?

It seems like it's usage is not really required, and could indeed be
dropped altogether?

Alternatively, there's the PV clock which is available to PVH guests
and will return a proper time.

Thanks, Roger.


Re: [PATCH v4 33/35] OvmfPkg: Introduce XenIoPvhDxe to initialize Grant Tables

Roger Pau Monné
 

On Mon, Jul 29, 2019 at 04:39:42PM +0100, Anthony PERARD wrote:
XenIoPvhDxe use XenIoMmioLib to reserve some space to be use by the
Grant Tables.

The call is only done if it is necessary, we simply detect if the
guest is PVH, as in this case there is currently no PCI bus, and no
PCI Xen platform device which would start the XenIoPciDxe and allocate
the space for the Grant Tables.
Since I'm not familiar with OVMF code, where is the grant table
physical memory coming from then, is it allocated from a hole in the
memory map?

Thanks, Roger.


Re: [PATCH v4 31/35] OvmfPkg/PlatformBootManagerLib: Use a Xen console for ConOut/ConIn

Roger Pau Monné
 

On Mon, Jul 29, 2019 at 04:39:40PM +0100, Anthony PERARD wrote:
On a Xen PVH guest, none of the existing serial or console interface
works, so we add a new one, based on XenConsoleSerialPortLib, and
implemented via SerialDxe.

That is a simple console implementation that can works on both PVH
^ work
guest and HVM guests, even if it rarely going to be use on HVM.
^ is ^ used


Re: [PATCH 1/1] UefiCpuPkg/PiSmmCpuDxeSmm: Fix coding style

Laszlo Ersek
 

On 08/07/19 04:09, Shenglei Zhang wrote:
1. Update CPUStatus to CpuStatus in comments to align comments
with code.
2. Add "OUT" attribute for "ProcedureArguments" parameter in function
header.

Cc: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>
Cc: Laszlo Ersek <lersek@...>
Signed-off-by: Shenglei Zhang <shenglei.zhang@...>
---
UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index a0e59f20886b..7b3fbc8b55ec 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -1311,7 +1311,7 @@ RestoreCr2 (
Note that timeout support is optional. Whether an implementation
supports this feature can be determined via the Attributes data
member.
- @param[in,out] CPUStatus This optional pointer may be used to get the status code returned
+ @param[in,out] CpuStatus This optional pointer may be used to get the status code returned
by Procedure when it completes execution on the target AP, or with
EFI_TIMEOUT if the Procedure fails to complete within the optional
timeout. The implementation will update this variable with
@@ -1437,8 +1437,8 @@ InternalSmmStartupAllAPs (
**/
EFI_STATUS
RegisterStartupProcedure (
- IN EFI_AP_PROCEDURE Procedure,
- IN VOID *ProcedureArguments OPTIONAL
+ IN EFI_AP_PROCEDURE Procedure,
+ IN OUT VOID *ProcedureArguments OPTIONAL
);

/**
scrambling to wade through my email backlog after being away for a few
days, so I'm skipping this

thanks
Laszlo


Re: [PATCH v4 29/35] OvmfPkg/OvmfXen: Override PcdFSBClock to Xen vLAPIC timer frequency

Roger Pau Monné
 

On Mon, Jul 29, 2019 at 04:39:38PM +0100, Anthony PERARD wrote:
PcdFSBClock is used by SecPeiDxeTimerLibCpu, the TimerLib
implementation. It will also be used by XenTimerDxe. Override
PcdFSBClock to match Xen vLAPIC timer frequency.
I'm kind of surprised that his is not automatically detected?

Is it a bug in Xen or just always hardcoded on OVMF?

Thanks, Roger.


Re: [Patch 1/3] EmulatorPkg: Fix VS20xx IA32 boot failure

Michael D Kinney
 

Hao Wu,

I agree that the exception is not an expected result
at all. I will debug a bit more to see why the mapping
was allowed to pass. The expected result should be
an error message with normal app exit if the address
can not be mapped.

Mike

-----Original Message-----
From: Wu, Hao A
Sent: Wednesday, August 7, 2019 1:15 AM
To: Kinney, Michael D <michael.d.kinney@...>;
devel@edk2.groups.io
Cc: Justen, Jordan L <jordan.l.justen@...>;
Andrew Fish <afish@...>; Ni, Ray
<ray.ni@...>
Subject: RE: [edk2-devel] [Patch 1/3] EmulatorPkg: Fix
VS20xx IA32 boot failure

-----Original Message-----
From: Kinney, Michael D
Sent: Wednesday, August 07, 2019 3:43 PM
To: Wu, Hao A; devel@edk2.groups.io; Kinney, Michael
D
Cc: Justen, Jordan L; Andrew Fish; Ni, Ray
Subject: RE: [edk2-devel] [Patch 1/3] EmulatorPkg:
Fix VS20xx IA32
boot failure

I observed 2 failure modes

1) The range could not be mapped at the request
address
And an error message was displayed and the app
exited.
2) The mapping passed, but when it was accessed by
the
Sec module, the app generated an exception.

Thanks for the additional information.

For case 2), even though it might not be directly
related with this issue.
I think it would be the best to ensure WinNtOpenFile()
returns with success only when the map operation takes
effect.

For this patch:
Reviewed-by: Hao A Wu <hao.a.wu@...>

Best Regards,
Hao Wu



Mike

-----Original Message-----
From: Wu, Hao A
Sent: Tuesday, August 6, 2019 11:19 PM
To: devel@edk2.groups.io; Kinney, Michael D
<michael.d.kinney@...>
Cc: Justen, Jordan L <jordan.l.justen@...>;
Andrew Fish
<afish@...>; Ni, Ray <ray.ni@...>
Subject: RE: [edk2-devel] [Patch 1/3] EmulatorPkg:
Fix VS20xx IA32
boot failure

-----Original Message-----
From: devel@edk2.groups.io
[mailto:devel@edk2.groups.io] On Behalf Of
Michael D Kinney
Sent: Wednesday, August 07, 2019 12:20 PM
To: devel@edk2.groups.io
Cc: Justen, Jordan L; Andrew Fish; Ni, Ray
Subject: [edk2-devel] [Patch 1/3] EmulatorPkg:
Fix
VS20xx IA32 boot
failure

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

The IA32 build of the EmulatorPkg for VS20xx does
not
boot because the
default value of PCD PcdPeiServicesTablePage is
set
for X64 Windows
Host environments. If the EmulatorPkg is build
for
an IA32 Windows
Host environment, then set this PCD to 0x13000000
that can be mapped
into a 32-bit user process.

Hello Mike,

I have a question for this patch.

For the main() function within file
EmulatorPkg/Win/Host/WinHost.c, I saw the below
codes:

EmuMagicPage = (VOID *)(UINTN)(FixedPcdGet64
(PcdPeiServicesTablePage) & MAX_UINTN);
if (EmuMagicPage != NULL) {
UINT64 Size;
Status = WinNtOpenFile (
NULL,
SIZE_4KB,
0,
&EmuMagicPage,
&Size
);
if (EFI_ERROR (Status)) {
SecPrint ("ERROR : Could not allocate
PeiServicesTablePage @
%p\n", EmuMagicPage);
return EFI_DEVICE_ERROR;
}
}

My understanding is that the WinNtOpenFile()
function call is
attempting to map the address specified by
'EmuMagicPage'. For IA32
case, the value in 'EmuMagicPage' here has already
been truncated
(0x03000000). If the
WinNtOpenFile() call returned successfully, the
address should be
mapped.

Is it the case that even if WinNtOpenFile() returns
with no error,
the specified address (0x03000000) is not actually
being mapped?

Best Regards,
Hao Wu



Cc: Jordan Justen <jordan.l.justen@...>
Cc: Andrew Fish <afish@...>
Cc: Ray Ni <ray.ni@...>
Signed-off-by: Michael D Kinney
<michael.d.kinney@...>
---
EmulatorPkg/EmulatorPkg.dsc | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/EmulatorPkg/EmulatorPkg.dsc
b/EmulatorPkg/EmulatorPkg.dsc
index ea8b6ce76e..c4ec10d1d8 100644
--- a/EmulatorPkg/EmulatorPkg.dsc
+++ b/EmulatorPkg/EmulatorPkg.dsc
@@ -4,7 +4,7 @@
# The Emulation Platform can be used to debug
individual modules,
prior to creating # a real platform. This also
provides an example
for how an DSC is created.
#
-# Copyright (c) 2006 - 2018, Intel Corporation.
All
rights
reserved.<BR>
+# Copyright (c) 2006 - 2019, Intel Corporation.
All
rights
+reserved.<BR>
# Portions copyright (c) 2010 - 2011, Apple Inc.
All
rights
reserved.<BR> # # SPDX-License-Identifier: BSD-
2-
Clause-Patent @@
-225,6 +225,10 @@ [PcdsFixedAtBuild]
# 0-PCANSI, 1-VT100, 2-VT00+, 3-UTF8, 4-
TTYTERM
gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|1

+!if "IA32" in $(ARCH) && "MSFT" in $(FAMILY)
+
gEmulatorPkgTokenSpaceGuid.PcdPeiServicesTablePage|0x13
000000
+!endif
+
[PcdsDynamicDefault.common.DEFAULT]
gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpar
eBase64|0

gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWork
ingBase64|
0
--
2.21.0.windows.1