* [edk2-devel] [PATCH 1/3] UefiCpuPkg: Create MpInformation.h in UefiCpuPkg
2023-11-09 2:51 [edk2-devel] [PATCH 0/3] Move gMpInformationHobGuid from StandaloneMmPkg to UefiCpuPkg duntan
@ 2023-11-09 2:51 ` duntan
2023-11-09 2:51 ` [edk2-devel] [PATCH 2/3] StandaloneMmPkg:Add UefiCpuPkg.dec in DependencyCheck duntan
` (3 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: duntan @ 2023-11-09 2:51 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar, Gerd Hoffmann
Copy MpInformation.h and gMpInformationHobGuid to
UefiCpuPkg.
Previously, the HOB is defined, created and consumed
only in StandaloneMmPkg. The HOB contains the number
of processors and EFI_PROCESSOR_INFORMATION structure.
This is the same as the information that PiSmmCpuDxeSmm
uses EfiMpServiceProtocolGuid to get.
The incoming plan is to create gMpInformationHobGuid
for both StandaloneMm and legacy DXE_SMM in early
phase. Then PiSmmCpuDxeSmm can consume the hob, which can
simplified code logic about consuming MpService Protocol.
So move this HOB definition to UefiCpuPkg.
Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
---
UefiCpuPkg/Include/Guid/MpInformation.h | 35 +++++++++++++++++++++++++++++++++++
UefiCpuPkg/UefiCpuPkg.dec | 3 +++
2 files changed, 38 insertions(+)
diff --git a/UefiCpuPkg/Include/Guid/MpInformation.h b/UefiCpuPkg/Include/Guid/MpInformation.h
new file mode 100644
index 0000000000..f0a6fdb24e
--- /dev/null
+++ b/UefiCpuPkg/Include/Guid/MpInformation.h
@@ -0,0 +1,35 @@
+/** @file
+ EFI MP information protocol provides a lightweight MP_SERVICES_PROTOCOL.
+
+ MP information protocol only provides static information of MP processor.
+
+ Copyright (c) 2009 - 2023, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.<BR>
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef _MP_INFORMATION_H_
+#define _MP_INFORMATION_H_
+
+#include <Protocol/MpService.h>
+#include <PiPei.h>
+#include <Ppi/SecPlatformInformation.h>
+
+#define MP_INFORMATION_GUID \
+ { \
+ 0xba33f15d, 0x4000, 0x45c1, {0x8e, 0x88, 0xf9, 0x16, 0x92, 0xd4, 0x57, 0xe3} \
+ }
+
+#pragma pack(1)
+typedef struct {
+ UINT64 NumberOfProcessors;
+ UINT64 NumberOfEnabledProcessors;
+ EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer[];
+} MP_INFORMATION_HOB_DATA;
+#pragma pack()
+
+extern EFI_GUID gMpInformationHobGuid;
+
+#endif
diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index 0b5431dbf7..92860b4c6e 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -85,6 +85,9 @@
## Include/Guid/SmmBaseHob.h
gSmmBaseHobGuid = { 0xc2217ba7, 0x03bb, 0x4f63, {0xa6, 0x47, 0x7c, 0x25, 0xc5, 0xfc, 0x9d, 0x73 }}
+ ## Include/Guid/MpInformation.h
+ gMpInformationHobGuid = { 0xba33f15d, 0x4000, 0x45c1, { 0x8e, 0x88, 0xf9, 0x16, 0x92, 0xd4, 0x57, 0xe3 }}
+
[Protocols]
## Include/Protocol/SmmCpuService.h
gEfiSmmCpuServiceProtocolGuid = { 0x1d202cab, 0xc8ab, 0x4d5c, { 0x94, 0xf7, 0x3c, 0xfc, 0xc0, 0xd3, 0xd3, 0x35 }}
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110942): https://edk2.groups.io/g/devel/message/110942
Mute This Topic: https://groups.io/mt/102479009/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 12+ messages in thread* [edk2-devel] [PATCH 2/3] StandaloneMmPkg:Add UefiCpuPkg.dec in DependencyCheck
2023-11-09 2:51 [edk2-devel] [PATCH 0/3] Move gMpInformationHobGuid from StandaloneMmPkg to UefiCpuPkg duntan
2023-11-09 2:51 ` [edk2-devel] [PATCH 1/3] UefiCpuPkg: Create MpInformation.h in UefiCpuPkg duntan
@ 2023-11-09 2:51 ` duntan
2023-11-09 2:51 ` [edk2-devel] [PATCH 3/3] StandaloneMmPkg:Remove MpInformation.h duntan
` (2 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: duntan @ 2023-11-09 2:51 UTC (permalink / raw)
To: devel; +Cc: Ard Biesheuvel, Sami Mujawar, Ray Ni
Add UefiCpuPkg.dec in DependencyCheck section of
StandaloneMmPkg.ci.yaml to allow StandaloneMmPkg
depend on UefiCpuPkg.
Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Ray Ni <ray.ni@intel.com>
---
StandaloneMmPkg/StandaloneMmPkg.ci.yaml | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/StandaloneMmPkg/StandaloneMmPkg.ci.yaml b/StandaloneMmPkg/StandaloneMmPkg.ci.yaml
index 4777532a7e..ebd35f515e 100644
--- a/StandaloneMmPkg/StandaloneMmPkg.ci.yaml
+++ b/StandaloneMmPkg/StandaloneMmPkg.ci.yaml
@@ -39,7 +39,8 @@
"EmbeddedPkg/EmbeddedPkg.dec",
"StandaloneMmPkg/StandaloneMmPkg.dec",
"MdeModulePkg/MdeModulePkg.dec",
- "MdePkg/MdePkg.dec"
+ "MdePkg/MdePkg.dec",
+ "UefiCpuPkg/UefiCpuPkg.dec"
],
# For host based unit tests
"AcceptableDependencies-HOST_APPLICATION":[
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110943): https://edk2.groups.io/g/devel/message/110943
Mute This Topic: https://groups.io/mt/102479010/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 12+ messages in thread* [edk2-devel] [PATCH 3/3] StandaloneMmPkg:Remove MpInformation.h
2023-11-09 2:51 [edk2-devel] [PATCH 0/3] Move gMpInformationHobGuid from StandaloneMmPkg to UefiCpuPkg duntan
2023-11-09 2:51 ` [edk2-devel] [PATCH 1/3] UefiCpuPkg: Create MpInformation.h in UefiCpuPkg duntan
2023-11-09 2:51 ` [edk2-devel] [PATCH 2/3] StandaloneMmPkg:Add UefiCpuPkg.dec in DependencyCheck duntan
@ 2023-11-09 2:51 ` duntan
2024-01-03 15:11 ` Ard Biesheuvel
2023-11-13 1:47 ` [edk2-devel] [PATCH 0/3] Move gMpInformationHobGuid from StandaloneMmPkg to UefiCpuPkg Ni, Ray
2023-11-13 14:33 ` Laszlo Ersek
4 siblings, 1 reply; 12+ messages in thread
From: duntan @ 2023-11-09 2:51 UTC (permalink / raw)
To: devel; +Cc: Ard Biesheuvel, Sami Mujawar, Ray Ni
Remove MpInformation.h in StandaloneMmPkg since
it has been moved to UefiCpuPkg
Signed-off-by: Dun Tan <dun.tan@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Cc: Ray Ni <ray.ni@intel.com>
---
StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf | 1 +
StandaloneMmPkg/Include/Guid/MpInformation.h | 35 -----------------------------------
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf | 1 +
StandaloneMmPkg/StandaloneMmPkg.dec | 1 -
4 files changed, 2 insertions(+), 36 deletions(-)
diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
index 1fcb17d89d..4ed0e395c8 100644
--- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
+++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
@@ -27,6 +27,7 @@
MdePkg/MdePkg.dec
MdeModulePkg/MdeModulePkg.dec
StandaloneMmPkg/StandaloneMmPkg.dec
+ UefiCpuPkg/UefiCpuPkg.dec
[LibraryClasses]
ArmLib
diff --git a/StandaloneMmPkg/Include/Guid/MpInformation.h b/StandaloneMmPkg/Include/Guid/MpInformation.h
deleted file mode 100644
index dbf88d12de..0000000000
--- a/StandaloneMmPkg/Include/Guid/MpInformation.h
+++ /dev/null
@@ -1,35 +0,0 @@
-/** @file
- EFI MP information protocol provides a lightweight MP_SERVICES_PROTOCOL.
-
- MP information protocol only provides static information of MP processor.
-
- Copyright (c) 2009, Intel Corporation. All rights reserved.<BR>
- Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.<BR>
-
- SPDX-License-Identifier: BSD-2-Clause-Patent
-
-**/
-
-#ifndef _MP_INFORMATION_H_
-#define _MP_INFORMATION_H_
-
-#include <Protocol/MpService.h>
-#include <PiPei.h>
-#include <Ppi/SecPlatformInformation.h>
-
-#define MP_INFORMATION_GUID \
- { \
- 0xba33f15d, 0x4000, 0x45c1, {0x8e, 0x88, 0xf9, 0x16, 0x92, 0xd4, 0x57, 0xe3} \
- }
-
-#pragma pack(1)
-typedef struct {
- UINT64 NumberOfProcessors;
- UINT64 NumberOfEnabledProcessors;
- EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer[];
-} MP_INFORMATION_HOB_DATA;
-#pragma pack()
-
-extern EFI_GUID gMpInformationHobGuid;
-
-#endif
diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
index 75cfb98c0e..1fc31360ce 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
@@ -33,6 +33,7 @@
MdePkg/MdePkg.dec
MdeModulePkg/MdeModulePkg.dec
StandaloneMmPkg/StandaloneMmPkg.dec
+ UefiCpuPkg/UefiCpuPkg.dec
[Packages.ARM, Packages.AARCH64]
ArmPkg/ArmPkg.dec
diff --git a/StandaloneMmPkg/StandaloneMmPkg.dec b/StandaloneMmPkg/StandaloneMmPkg.dec
index 46784d94e4..01f37deebb 100644
--- a/StandaloneMmPkg/StandaloneMmPkg.dec
+++ b/StandaloneMmPkg/StandaloneMmPkg.dec
@@ -36,7 +36,6 @@
[Guids]
gStandaloneMmPkgTokenSpaceGuid = { 0x18fe7632, 0xf5c8, 0x4e63, { 0x8d, 0xe8, 0x17, 0xa5, 0x5c, 0x59, 0x13, 0xbd }}
- gMpInformationHobGuid = { 0xba33f15d, 0x4000, 0x45c1, { 0x8e, 0x88, 0xf9, 0x16, 0x92, 0xd4, 0x57, 0xe3 }}
gMmFvDispatchGuid = { 0xb65694cc, 0x09e3, 0x4c3b, { 0xb5, 0xcd, 0x05, 0xf4, 0x4d, 0x3c, 0xdb, 0xff }}
## Include/Guid/MmCoreData.h
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110944): https://edk2.groups.io/g/devel/message/110944
Mute This Topic: https://groups.io/mt/102479012/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [edk2-devel] [PATCH 3/3] StandaloneMmPkg:Remove MpInformation.h
2023-11-09 2:51 ` [edk2-devel] [PATCH 3/3] StandaloneMmPkg:Remove MpInformation.h duntan
@ 2024-01-03 15:11 ` Ard Biesheuvel
2024-01-03 17:42 ` Oliver Smith-Denny
2024-01-04 2:21 ` duntan
0 siblings, 2 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2024-01-03 15:11 UTC (permalink / raw)
To: devel, dun.tan; +Cc: Ard Biesheuvel, Sami Mujawar, Ray Ni
On Thu, 9 Nov 2023 at 03:51, duntan <dun.tan@intel.com> wrote:
>
> Remove MpInformation.h in StandaloneMmPkg since
> it has been moved to UefiCpuPkg
>
> Signed-off-by: Dun Tan <dun.tan@intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Ray Ni <ray.ni@intel.com>
Doesn't this break the ARM build?
> ---
> StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf | 1 +
> StandaloneMmPkg/Include/Guid/MpInformation.h | 35 -----------------------------------
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf | 1 +
> StandaloneMmPkg/StandaloneMmPkg.dec | 1 -
> 4 files changed, 2 insertions(+), 36 deletions(-)
>
> diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
> index 1fcb17d89d..4ed0e395c8 100644
> --- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
> +++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
> @@ -27,6 +27,7 @@
> MdePkg/MdePkg.dec
> MdeModulePkg/MdeModulePkg.dec
> StandaloneMmPkg/StandaloneMmPkg.dec
> + UefiCpuPkg/UefiCpuPkg.dec
>
> [LibraryClasses]
> ArmLib
> diff --git a/StandaloneMmPkg/Include/Guid/MpInformation.h b/StandaloneMmPkg/Include/Guid/MpInformation.h
> deleted file mode 100644
> index dbf88d12de..0000000000
> --- a/StandaloneMmPkg/Include/Guid/MpInformation.h
> +++ /dev/null
> @@ -1,35 +0,0 @@
> -/** @file
> - EFI MP information protocol provides a lightweight MP_SERVICES_PROTOCOL.
> -
> - MP information protocol only provides static information of MP processor.
> -
> - Copyright (c) 2009, Intel Corporation. All rights reserved.<BR>
> - Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.<BR>
> -
> - SPDX-License-Identifier: BSD-2-Clause-Patent
> -
> -**/
> -
> -#ifndef _MP_INFORMATION_H_
> -#define _MP_INFORMATION_H_
> -
> -#include <Protocol/MpService.h>
> -#include <PiPei.h>
> -#include <Ppi/SecPlatformInformation.h>
> -
> -#define MP_INFORMATION_GUID \
> - { \
> - 0xba33f15d, 0x4000, 0x45c1, {0x8e, 0x88, 0xf9, 0x16, 0x92, 0xd4, 0x57, 0xe3} \
> - }
> -
> -#pragma pack(1)
> -typedef struct {
> - UINT64 NumberOfProcessors;
> - UINT64 NumberOfEnabledProcessors;
> - EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer[];
> -} MP_INFORMATION_HOB_DATA;
> -#pragma pack()
> -
> -extern EFI_GUID gMpInformationHobGuid;
> -
> -#endif
> diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
> index 75cfb98c0e..1fc31360ce 100644
> --- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
> +++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
> @@ -33,6 +33,7 @@
> MdePkg/MdePkg.dec
> MdeModulePkg/MdeModulePkg.dec
> StandaloneMmPkg/StandaloneMmPkg.dec
> + UefiCpuPkg/UefiCpuPkg.dec
>
> [Packages.ARM, Packages.AARCH64]
> ArmPkg/ArmPkg.dec
> diff --git a/StandaloneMmPkg/StandaloneMmPkg.dec b/StandaloneMmPkg/StandaloneMmPkg.dec
> index 46784d94e4..01f37deebb 100644
> --- a/StandaloneMmPkg/StandaloneMmPkg.dec
> +++ b/StandaloneMmPkg/StandaloneMmPkg.dec
> @@ -36,7 +36,6 @@
>
> [Guids]
> gStandaloneMmPkgTokenSpaceGuid = { 0x18fe7632, 0xf5c8, 0x4e63, { 0x8d, 0xe8, 0x17, 0xa5, 0x5c, 0x59, 0x13, 0xbd }}
> - gMpInformationHobGuid = { 0xba33f15d, 0x4000, 0x45c1, { 0x8e, 0x88, 0xf9, 0x16, 0x92, 0xd4, 0x57, 0xe3 }}
> gMmFvDispatchGuid = { 0xb65694cc, 0x09e3, 0x4c3b, { 0xb5, 0xcd, 0x05, 0xf4, 0x4d, 0x3c, 0xdb, 0xff }}
>
> ## Include/Guid/MmCoreData.h
> --
> 2.31.1.windows.1
>
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113101): https://edk2.groups.io/g/devel/message/113101
Mute This Topic: https://groups.io/mt/102479012/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [edk2-devel] [PATCH 3/3] StandaloneMmPkg:Remove MpInformation.h
2024-01-03 15:11 ` Ard Biesheuvel
@ 2024-01-03 17:42 ` Oliver Smith-Denny
2024-01-04 2:21 ` duntan
1 sibling, 0 replies; 12+ messages in thread
From: Oliver Smith-Denny @ 2024-01-03 17:42 UTC (permalink / raw)
To: devel, ardb, dun.tan; +Cc: Ard Biesheuvel, Sami Mujawar, Ray Ni
On 1/3/2024 7:11 AM, Ard Biesheuvel wrote:
> On Thu, 9 Nov 2023 at 03:51, duntan <dun.tan@intel.com> wrote:
>>
>> Remove MpInformation.h in StandaloneMmPkg since
>> it has been moved to UefiCpuPkg
>>
>> Signed-off-by: Dun Tan <dun.tan@intel.com>
>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>> Cc: Sami Mujawar <sami.mujawar@arm.com>
>> Cc: Ray Ni <ray.ni@intel.com>
>
> Doesn't this break the ARM build?
>
Right, this would add a dependency on UefiCpuPkg in ArmPkg (via
StandaloneMmPkg, which I don't believe is there today?). Don't we
strongly not want that dependency since UefiCpuPkg is x86 specific?
Thanks,
Oliver
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113103): https://edk2.groups.io/g/devel/message/113103
Mute This Topic: https://groups.io/mt/102479012/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH 3/3] StandaloneMmPkg:Remove MpInformation.h
2024-01-03 15:11 ` Ard Biesheuvel
2024-01-03 17:42 ` Oliver Smith-Denny
@ 2024-01-04 2:21 ` duntan
2024-01-04 8:32 ` Ard Biesheuvel
1 sibling, 1 reply; 12+ messages in thread
From: duntan @ 2024-01-04 2:21 UTC (permalink / raw)
To: Ard Biesheuvel, devel@edk2.groups.io
Cc: Ard Biesheuvel, Sami Mujawar, Ni, Ray
Hi Ard,
This patch set has been dropped. Another patch set "Create and consume a new gMpInformationHobGuid2 in UefiCpuPkg." is adopted.
When the Maximum length 64KB is not enough, there might be more than 1 HOB instance of this gMpInformationHobGuid2.
There is no change in ARM related code in the new patch set.
Thanks,
Dun
-----Original Message-----
From: Ard Biesheuvel <ardb@kernel.org>
Sent: Wednesday, January 3, 2024 11:12 PM
To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Sami Mujawar <sami.mujawar@arm.com>; Ni, Ray <ray.ni@intel.com>
Subject: Re: [edk2-devel] [PATCH 3/3] StandaloneMmPkg:Remove MpInformation.h
On Thu, 9 Nov 2023 at 03:51, duntan <dun.tan@intel.com> wrote:
>
> Remove MpInformation.h in StandaloneMmPkg since it has been moved to
> UefiCpuPkg
>
> Signed-off-by: Dun Tan <dun.tan@intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Sami Mujawar <sami.mujawar@arm.com>
> Cc: Ray Ni <ray.ni@intel.com>
Doesn't this break the ARM build?
> ---
> StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf | 1 +
> StandaloneMmPkg/Include/Guid/MpInformation.h | 35 -----------------------------------
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf | 1 +
> StandaloneMmPkg/StandaloneMmPkg.dec | 1 -
> 4 files changed, 2 insertions(+), 36 deletions(-)
>
> diff --git
> a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
> b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
> index 1fcb17d89d..4ed0e395c8 100644
> --- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
> +++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
> @@ -27,6 +27,7 @@
> MdePkg/MdePkg.dec
> MdeModulePkg/MdeModulePkg.dec
> StandaloneMmPkg/StandaloneMmPkg.dec
> + UefiCpuPkg/UefiCpuPkg.dec
>
> [LibraryClasses]
> ArmLib
> diff --git a/StandaloneMmPkg/Include/Guid/MpInformation.h
> b/StandaloneMmPkg/Include/Guid/MpInformation.h
> deleted file mode 100644
> index dbf88d12de..0000000000
> --- a/StandaloneMmPkg/Include/Guid/MpInformation.h
> +++ /dev/null
> @@ -1,35 +0,0 @@
> -/** @file
> - EFI MP information protocol provides a lightweight MP_SERVICES_PROTOCOL.
> -
> - MP information protocol only provides static information of MP processor.
> -
> - Copyright (c) 2009, Intel Corporation. All rights reserved.<BR>
> - Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.<BR>
> -
> - SPDX-License-Identifier: BSD-2-Clause-Patent
> -
> -**/
> -
> -#ifndef _MP_INFORMATION_H_
> -#define _MP_INFORMATION_H_
> -
> -#include <Protocol/MpService.h>
> -#include <PiPei.h>
> -#include <Ppi/SecPlatformInformation.h>
> -
> -#define MP_INFORMATION_GUID \
> - { \
> - 0xba33f15d, 0x4000, 0x45c1, {0x8e, 0x88, 0xf9, 0x16, 0x92, 0xd4, 0x57, 0xe3} \
> - }
> -
> -#pragma pack(1)
> -typedef struct {
> - UINT64 NumberOfProcessors;
> - UINT64 NumberOfEnabledProcessors;
> - EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer[];
> -} MP_INFORMATION_HOB_DATA;
> -#pragma pack()
> -
> -extern EFI_GUID gMpInformationHobGuid;
> -
> -#endif
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreE
> ntryPoint.inf
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreE
> ntryPoint.inf
> index 75cfb98c0e..1fc31360ce 100644
> ---
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreE
> ntryPoint.inf
> +++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmC
> +++ oreEntryPoint.inf
> @@ -33,6 +33,7 @@
> MdePkg/MdePkg.dec
> MdeModulePkg/MdeModulePkg.dec
> StandaloneMmPkg/StandaloneMmPkg.dec
> + UefiCpuPkg/UefiCpuPkg.dec
>
> [Packages.ARM, Packages.AARCH64]
> ArmPkg/ArmPkg.dec
> diff --git a/StandaloneMmPkg/StandaloneMmPkg.dec
> b/StandaloneMmPkg/StandaloneMmPkg.dec
> index 46784d94e4..01f37deebb 100644
> --- a/StandaloneMmPkg/StandaloneMmPkg.dec
> +++ b/StandaloneMmPkg/StandaloneMmPkg.dec
> @@ -36,7 +36,6 @@
>
> [Guids]
> gStandaloneMmPkgTokenSpaceGuid = { 0x18fe7632, 0xf5c8, 0x4e63, { 0x8d, 0xe8, 0x17, 0xa5, 0x5c, 0x59, 0x13, 0xbd }}
> - gMpInformationHobGuid = { 0xba33f15d, 0x4000, 0x45c1, { 0x8e, 0x88, 0xf9, 0x16, 0x92, 0xd4, 0x57, 0xe3 }}
> gMmFvDispatchGuid = { 0xb65694cc, 0x09e3, 0x4c3b, { 0xb5, 0xcd, 0x05, 0xf4, 0x4d, 0x3c, 0xdb, 0xff }}
>
> ## Include/Guid/MmCoreData.h
> --
> 2.31.1.windows.1
>
>
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113112): https://edk2.groups.io/g/devel/message/113112
Mute This Topic: https://groups.io/mt/102479012/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [edk2-devel] [PATCH 3/3] StandaloneMmPkg:Remove MpInformation.h
2024-01-04 2:21 ` duntan
@ 2024-01-04 8:32 ` Ard Biesheuvel
0 siblings, 0 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2024-01-04 8:32 UTC (permalink / raw)
To: Tan, Dun; +Cc: devel@edk2.groups.io, Ard Biesheuvel, Sami Mujawar, Ni, Ray
Thanks for the clarification.
On Thu, 4 Jan 2024 at 03:21, Tan, Dun <dun.tan@intel.com> wrote:
>
> Hi Ard,
>
> This patch set has been dropped. Another patch set "Create and consume a new gMpInformationHobGuid2 in UefiCpuPkg." is adopted.
> When the Maximum length 64KB is not enough, there might be more than 1 HOB instance of this gMpInformationHobGuid2.
> There is no change in ARM related code in the new patch set.
>
> Thanks,
> Dun
>
> -----Original Message-----
> From: Ard Biesheuvel <ardb@kernel.org>
> Sent: Wednesday, January 3, 2024 11:12 PM
> To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Sami Mujawar <sami.mujawar@arm.com>; Ni, Ray <ray.ni@intel.com>
> Subject: Re: [edk2-devel] [PATCH 3/3] StandaloneMmPkg:Remove MpInformation.h
>
> On Thu, 9 Nov 2023 at 03:51, duntan <dun.tan@intel.com> wrote:
> >
> > Remove MpInformation.h in StandaloneMmPkg since it has been moved to
> > UefiCpuPkg
> >
> > Signed-off-by: Dun Tan <dun.tan@intel.com>
> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > Cc: Sami Mujawar <sami.mujawar@arm.com>
> > Cc: Ray Ni <ray.ni@intel.com>
>
> Doesn't this break the ARM build?
>
>
> > ---
> > StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf | 1 +
> > StandaloneMmPkg/Include/Guid/MpInformation.h | 35 -----------------------------------
> > StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf | 1 +
> > StandaloneMmPkg/StandaloneMmPkg.dec | 1 -
> > 4 files changed, 2 insertions(+), 36 deletions(-)
> >
> > diff --git
> > a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
> > b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
> > index 1fcb17d89d..4ed0e395c8 100644
> > --- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
> > +++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf
> > @@ -27,6 +27,7 @@
> > MdePkg/MdePkg.dec
> > MdeModulePkg/MdeModulePkg.dec
> > StandaloneMmPkg/StandaloneMmPkg.dec
> > + UefiCpuPkg/UefiCpuPkg.dec
> >
> > [LibraryClasses]
> > ArmLib
> > diff --git a/StandaloneMmPkg/Include/Guid/MpInformation.h
> > b/StandaloneMmPkg/Include/Guid/MpInformation.h
> > deleted file mode 100644
> > index dbf88d12de..0000000000
> > --- a/StandaloneMmPkg/Include/Guid/MpInformation.h
> > +++ /dev/null
> > @@ -1,35 +0,0 @@
> > -/** @file
> > - EFI MP information protocol provides a lightweight MP_SERVICES_PROTOCOL.
> > -
> > - MP information protocol only provides static information of MP processor.
> > -
> > - Copyright (c) 2009, Intel Corporation. All rights reserved.<BR>
> > - Copyright (c) 2016 - 2018, ARM Limited. All rights reserved.<BR>
> > -
> > - SPDX-License-Identifier: BSD-2-Clause-Patent
> > -
> > -**/
> > -
> > -#ifndef _MP_INFORMATION_H_
> > -#define _MP_INFORMATION_H_
> > -
> > -#include <Protocol/MpService.h>
> > -#include <PiPei.h>
> > -#include <Ppi/SecPlatformInformation.h>
> > -
> > -#define MP_INFORMATION_GUID \
> > - { \
> > - 0xba33f15d, 0x4000, 0x45c1, {0x8e, 0x88, 0xf9, 0x16, 0x92, 0xd4, 0x57, 0xe3} \
> > - }
> > -
> > -#pragma pack(1)
> > -typedef struct {
> > - UINT64 NumberOfProcessors;
> > - UINT64 NumberOfEnabledProcessors;
> > - EFI_PROCESSOR_INFORMATION ProcessorInfoBuffer[];
> > -} MP_INFORMATION_HOB_DATA;
> > -#pragma pack()
> > -
> > -extern EFI_GUID gMpInformationHobGuid;
> > -
> > -#endif
> > diff --git
> > a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreE
> > ntryPoint.inf
> > b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreE
> > ntryPoint.inf
> > index 75cfb98c0e..1fc31360ce 100644
> > ---
> > a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreE
> > ntryPoint.inf
> > +++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmC
> > +++ oreEntryPoint.inf
> > @@ -33,6 +33,7 @@
> > MdePkg/MdePkg.dec
> > MdeModulePkg/MdeModulePkg.dec
> > StandaloneMmPkg/StandaloneMmPkg.dec
> > + UefiCpuPkg/UefiCpuPkg.dec
> >
> > [Packages.ARM, Packages.AARCH64]
> > ArmPkg/ArmPkg.dec
> > diff --git a/StandaloneMmPkg/StandaloneMmPkg.dec
> > b/StandaloneMmPkg/StandaloneMmPkg.dec
> > index 46784d94e4..01f37deebb 100644
> > --- a/StandaloneMmPkg/StandaloneMmPkg.dec
> > +++ b/StandaloneMmPkg/StandaloneMmPkg.dec
> > @@ -36,7 +36,6 @@
> >
> > [Guids]
> > gStandaloneMmPkgTokenSpaceGuid = { 0x18fe7632, 0xf5c8, 0x4e63, { 0x8d, 0xe8, 0x17, 0xa5, 0x5c, 0x59, 0x13, 0xbd }}
> > - gMpInformationHobGuid = { 0xba33f15d, 0x4000, 0x45c1, { 0x8e, 0x88, 0xf9, 0x16, 0x92, 0xd4, 0x57, 0xe3 }}
> > gMmFvDispatchGuid = { 0xb65694cc, 0x09e3, 0x4c3b, { 0xb5, 0xcd, 0x05, 0xf4, 0x4d, 0x3c, 0xdb, 0xff }}
> >
> > ## Include/Guid/MmCoreData.h
> > --
> > 2.31.1.windows.1
> >
> >
> >
> >
> >
> >
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113149): https://edk2.groups.io/g/devel/message/113149
Mute This Topic: https://groups.io/mt/102479012/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH 0/3] Move gMpInformationHobGuid from StandaloneMmPkg to UefiCpuPkg.
2023-11-09 2:51 [edk2-devel] [PATCH 0/3] Move gMpInformationHobGuid from StandaloneMmPkg to UefiCpuPkg duntan
` (2 preceding siblings ...)
2023-11-09 2:51 ` [edk2-devel] [PATCH 3/3] StandaloneMmPkg:Remove MpInformation.h duntan
@ 2023-11-13 1:47 ` Ni, Ray
2023-11-13 14:33 ` Laszlo Ersek
4 siblings, 0 replies; 12+ messages in thread
From: Ni, Ray @ 2023-11-13 1:47 UTC (permalink / raw)
To: devel@edk2.groups.io, Tan, Dun
[-- Attachment #1: Type: text/plain, Size: 2278 bytes --]
Reviewed-by: Ray Ni <ray.ni@intel.com>
Thanks,
Ray
________________________________
From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of duntan <dun.tan@intel.com>
Sent: Thursday, November 9, 2023 10:51 AM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Subject: [edk2-devel] [PATCH 0/3] Move gMpInformationHobGuid from StandaloneMmPkg to UefiCpuPkg.
Move gMpInformationHobGuid from StandaloneMmPkg to UefiCpuPkg.
Previously, the HOB is defined, created and consumed only in StandaloneMmPkg. The HOB contains the number
of processors and EFI_PROCESSOR_INFORMATION structure. This is the same as the information that PiSmmCpuDxeSmm
uses EfiMpServiceProtocolGuid to get.
The incoming plan is to create gMpInformationHobGuid for both StandaloneMm and legacy DXE_SMM in early
phase(for example in CpuMpPei). Then PiSmmCpuDxeSmm can consume the hob, which can simplify code logic
in PiSmmCpuDxeSmm driver.
So move this HOB definition to UefiCpuPkg in this patch series.
Dun Tan (3):
UefiCpuPkg: Create MpInformation.h in UefiCpuPkg
StandaloneMmPkg:Add UefiCpuPkg.dec in DependencyCheck
StandaloneMmPkg:Remove MpInformation.h
StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf | 1 +
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf | 1 +
StandaloneMmPkg/StandaloneMmPkg.ci.yaml | 3 ++-
StandaloneMmPkg/StandaloneMmPkg.dec | 1 -
{StandaloneMmPkg => UefiCpuPkg}/Include/Guid/MpInformation.h | 2 +-
UefiCpuPkg/UefiCpuPkg.dec | 3 +++
6 files changed, 8 insertions(+), 3 deletions(-)
rename {StandaloneMmPkg => UefiCpuPkg}/Include/Guid/MpInformation.h (88%)
--
2.31.1.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111117): https://edk2.groups.io/g/devel/message/111117
Mute This Topic: https://groups.io/mt/102479007/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
[-- Attachment #2: Type: text/html, Size: 5120 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [edk2-devel] [PATCH 0/3] Move gMpInformationHobGuid from StandaloneMmPkg to UefiCpuPkg.
2023-11-09 2:51 [edk2-devel] [PATCH 0/3] Move gMpInformationHobGuid from StandaloneMmPkg to UefiCpuPkg duntan
` (3 preceding siblings ...)
2023-11-13 1:47 ` [edk2-devel] [PATCH 0/3] Move gMpInformationHobGuid from StandaloneMmPkg to UefiCpuPkg Ni, Ray
@ 2023-11-13 14:33 ` Laszlo Ersek
2023-11-14 10:11 ` duntan
2023-11-15 0:30 ` duntan
4 siblings, 2 replies; 12+ messages in thread
From: Laszlo Ersek @ 2023-11-13 14:33 UTC (permalink / raw)
To: devel, dun.tan
On 11/9/23 03:51, duntan wrote:
> Move gMpInformationHobGuid from StandaloneMmPkg to UefiCpuPkg.
>
> Previously, the HOB is defined, created and consumed only in StandaloneMmPkg. The HOB contains the number
> of processors and EFI_PROCESSOR_INFORMATION structure. This is the same as the information that PiSmmCpuDxeSmm
> uses EfiMpServiceProtocolGuid to get.
>
> The incoming plan is to create gMpInformationHobGuid for both StandaloneMm and legacy DXE_SMM in early
> phase(for example in CpuMpPei). Then PiSmmCpuDxeSmm can consume the hob, which can simplify code logic
> in PiSmmCpuDxeSmm driver.
>
> So move this HOB definition to UefiCpuPkg in this patch series.
>
> Dun Tan (3):
> UefiCpuPkg: Create MpInformation.h in UefiCpuPkg
> StandaloneMmPkg:Add UefiCpuPkg.dec in DependencyCheck
> StandaloneMmPkg:Remove MpInformation.h
>
> StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf | 1 +
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf | 1 +
> StandaloneMmPkg/StandaloneMmPkg.ci.yaml | 3 ++-
> StandaloneMmPkg/StandaloneMmPkg.dec | 1 -
> {StandaloneMmPkg => UefiCpuPkg}/Include/Guid/MpInformation.h | 2 +-
> UefiCpuPkg/UefiCpuPkg.dec | 3 +++
> 6 files changed, 8 insertions(+), 3 deletions(-)
> rename {StandaloneMmPkg => UefiCpuPkg}/Include/Guid/MpInformation.h (88%)
>
From a quick skim, the series looks OK to me, and I also agree that the
above two MP services calls (GetNumberOfProcessors, GetProcessorInfo)
seem to be the only ones in PiSmmCpuDxeSmm.
However, what about the following scenario:
- in the PI phase (or HOB producer phase), the HOB is produced
- in the DXE phase, a platform DXE driver uses EFI_MP_SERVICES_PROTOCOL
to change some aspect of the processors in the system. For example, it
calls SwitchBSP, or calls EnableDisableAP.
- Then the information in the HOB will be stale, once PiSmmCpuDxeSmm
consumes it. The EFI_PROCESSOR_INFORMATION.StatusFlag field carries
information both about the CPU in question being BSP vs. AP, and about
the CPU being enabled or disabled. So either of SwitchBSP() and
EnableDisableAP() can invalidate the StatusFlag field for a given
ProcessorId.
I don't know how StandaloneMmPkg currently deals with this scenario, and
I also don't know whether StatusFlag actually matters to PiSmmCpuSmmDxe.
Apparently, it doesn't. So technically, the replacement of the protocol
with the HOB should be fine, but for the general case, we should
document somehow that specific fields of the HOB may be invalidated
between HOB production and HOB consumption. If platform code is required
to prevent that (i.e., the platform is responsible for not calling
SwitchBSP() or EnableDisableAP()), then that requirement should also be
documented.
Acked-by: Laszlo Ersek <lersek@redhat.com>
Thanks
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111155): https://edk2.groups.io/g/devel/message/111155
Mute This Topic: https://groups.io/mt/102479007/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [edk2-devel] [PATCH 0/3] Move gMpInformationHobGuid from StandaloneMmPkg to UefiCpuPkg.
2023-11-13 14:33 ` Laszlo Ersek
@ 2023-11-14 10:11 ` duntan
2023-11-15 0:30 ` duntan
1 sibling, 0 replies; 12+ messages in thread
From: duntan @ 2023-11-14 10:11 UTC (permalink / raw)
To: Laszlo Ersek, devel@edk2.groups.io; +Cc: Ni, Ray
Laszlo,
Thanks for your comments.
Yes the StatusFlag field of a given ProcessorId does change in the scenario you mentioned. I think it's ok to call SwitchBSP() and Enable/DisableAP() after creating the hob, since smm elects its own BSP and all CPU will enter smm after receiving smi regardless of the StatusFlag.
So the NumberOfProcessors, the ProcessorId and Location fields remain unchanged, the StatusFlag and NumberOfEnabledProcessors may be invalidated. I agree that we should document specific fields of the HOB may be invalidated between HOB production and HOB consumption. Will send V2 patch set to document this in the HOB definition head file.
Thanks,
Dun
-----Original Message-----
From: Laszlo Ersek <lersek@redhat.com>
Sent: Monday, November 13, 2023 10:33 PM
To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>
Subject: Re: [edk2-devel] [PATCH 0/3] Move gMpInformationHobGuid from StandaloneMmPkg to UefiCpuPkg.
On 11/9/23 03:51, duntan wrote:
> Move gMpInformationHobGuid from StandaloneMmPkg to UefiCpuPkg.
>
> Previously, the HOB is defined, created and consumed only in
> StandaloneMmPkg. The HOB contains the number of processors and
> EFI_PROCESSOR_INFORMATION structure. This is the same as the information that PiSmmCpuDxeSmm uses EfiMpServiceProtocolGuid to get.
>
> The incoming plan is to create gMpInformationHobGuid for both
> StandaloneMm and legacy DXE_SMM in early phase(for example in
> CpuMpPei). Then PiSmmCpuDxeSmm can consume the hob, which can simplify code logic in PiSmmCpuDxeSmm driver.
>
> So move this HOB definition to UefiCpuPkg in this patch series.
>
> Dun Tan (3):
> UefiCpuPkg: Create MpInformation.h in UefiCpuPkg
> StandaloneMmPkg:Add UefiCpuPkg.dec in DependencyCheck
> StandaloneMmPkg:Remove MpInformation.h
>
> StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf | 1 +
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf | 1 +
> StandaloneMmPkg/StandaloneMmPkg.ci.yaml | 3 ++-
> StandaloneMmPkg/StandaloneMmPkg.dec | 1 -
> {StandaloneMmPkg => UefiCpuPkg}/Include/Guid/MpInformation.h | 2 +-
> UefiCpuPkg/UefiCpuPkg.dec | 3 +++
> 6 files changed, 8 insertions(+), 3 deletions(-) rename
> {StandaloneMmPkg => UefiCpuPkg}/Include/Guid/MpInformation.h (88%)
>
From a quick skim, the series looks OK to me, and I also agree that the above two MP services calls (GetNumberOfProcessors, GetProcessorInfo) seem to be the only ones in PiSmmCpuDxeSmm.
However, what about the following scenario:
- in the PI phase (or HOB producer phase), the HOB is produced
- in the DXE phase, a platform DXE driver uses EFI_MP_SERVICES_PROTOCOL to change some aspect of the processors in the system. For example, it calls SwitchBSP, or calls EnableDisableAP.
- Then the information in the HOB will be stale, once PiSmmCpuDxeSmm consumes it. The EFI_PROCESSOR_INFORMATION.StatusFlag field carries information both about the CPU in question being BSP vs. AP, and about the CPU being enabled or disabled. So either of SwitchBSP() and
EnableDisableAP() can invalidate the StatusFlag field for a given ProcessorId.
I don't know how StandaloneMmPkg currently deals with this scenario, and I also don't know whether StatusFlag actually matters to PiSmmCpuSmmDxe.
Apparently, it doesn't. So technically, the replacement of the protocol with the HOB should be fine, but for the general case, we should document somehow that specific fields of the HOB may be invalidated between HOB production and HOB consumption. If platform code is required to prevent that (i.e., the platform is responsible for not calling
SwitchBSP() or EnableDisableAP()), then that requirement should also be documented.
Acked-by: Laszlo Ersek <lersek@redhat.com>
Thanks
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111194): https://edk2.groups.io/g/devel/message/111194
Mute This Topic: https://groups.io/mt/102479007/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [edk2-devel] [PATCH 0/3] Move gMpInformationHobGuid from StandaloneMmPkg to UefiCpuPkg.
2023-11-13 14:33 ` Laszlo Ersek
2023-11-14 10:11 ` duntan
@ 2023-11-15 0:30 ` duntan
1 sibling, 0 replies; 12+ messages in thread
From: duntan @ 2023-11-15 0:30 UTC (permalink / raw)
To: Laszlo Ersek, devel@edk2.groups.io; +Cc: Ni, Ray
Laszlo,
Thanks for your comments.
Yes the StatusFlag field of a given ProcessorId does change in the scenario you mentioned. I think it's ok to call SwitchBSP() and Enable/DisableAP() after creating the hob, since smm elects its own BSP and all CPU will enter smm after receiving smi regardless of the StatusFlag.
So the NumberOfProcessors, the ProcessorId and Location fields remain unchanged, the StatusFlag and NumberOfEnabledProcessors may be invalidated. I agree that we should document specific fields of the HOB may be invalidated between HOB production and HOB consumption. Will send V2 patch set to document this in the HOB definition head file.
Thanks,
Dun
-----Original Message-----
From: Laszlo Ersek <lersek@redhat.com>
Sent: Monday, November 13, 2023 10:33 PM
To: devel@edk2.groups.io; Tan, Dun <dun.tan@intel.com>
Subject: Re: [edk2-devel] [PATCH 0/3] Move gMpInformationHobGuid from StandaloneMmPkg to UefiCpuPkg.
On 11/9/23 03:51, duntan wrote:
> Move gMpInformationHobGuid from StandaloneMmPkg to UefiCpuPkg.
>
> Previously, the HOB is defined, created and consumed only in
> StandaloneMmPkg. The HOB contains the number of processors and
> EFI_PROCESSOR_INFORMATION structure. This is the same as the information that PiSmmCpuDxeSmm uses EfiMpServiceProtocolGuid to get.
>
> The incoming plan is to create gMpInformationHobGuid for both
> StandaloneMm and legacy DXE_SMM in early phase(for example in
> CpuMpPei). Then PiSmmCpuDxeSmm can consume the hob, which can simplify code logic in PiSmmCpuDxeSmm driver.
>
> So move this HOB definition to UefiCpuPkg in this patch series.
>
> Dun Tan (3):
> UefiCpuPkg: Create MpInformation.h in UefiCpuPkg
> StandaloneMmPkg:Add UefiCpuPkg.dec in DependencyCheck
> StandaloneMmPkg:Remove MpInformation.h
>
> StandaloneMmPkg/Drivers/StandaloneMmCpu/StandaloneMmCpu.inf | 1 +
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf | 1 +
> StandaloneMmPkg/StandaloneMmPkg.ci.yaml | 3 ++-
> StandaloneMmPkg/StandaloneMmPkg.dec | 1 -
> {StandaloneMmPkg => UefiCpuPkg}/Include/Guid/MpInformation.h | 2 +-
> UefiCpuPkg/UefiCpuPkg.dec | 3 +++
> 6 files changed, 8 insertions(+), 3 deletions(-) rename
> {StandaloneMmPkg => UefiCpuPkg}/Include/Guid/MpInformation.h (88%)
>
From a quick skim, the series looks OK to me, and I also agree that the above two MP services calls (GetNumberOfProcessors, GetProcessorInfo) seem to be the only ones in PiSmmCpuDxeSmm.
However, what about the following scenario:
- in the PI phase (or HOB producer phase), the HOB is produced
- in the DXE phase, a platform DXE driver uses EFI_MP_SERVICES_PROTOCOL to change some aspect of the processors in the system. For example, it calls SwitchBSP, or calls EnableDisableAP.
- Then the information in the HOB will be stale, once PiSmmCpuDxeSmm consumes it. The EFI_PROCESSOR_INFORMATION.StatusFlag field carries information both about the CPU in question being BSP vs. AP, and about the CPU being enabled or disabled. So either of SwitchBSP() and
EnableDisableAP() can invalidate the StatusFlag field for a given ProcessorId.
I don't know how StandaloneMmPkg currently deals with this scenario, and I also don't know whether StatusFlag actually matters to PiSmmCpuSmmDxe.
Apparently, it doesn't. So technically, the replacement of the protocol with the HOB should be fine, but for the general case, we should document somehow that specific fields of the HOB may be invalidated between HOB production and HOB consumption. If platform code is required to prevent that (i.e., the platform is responsible for not calling
SwitchBSP() or EnableDisableAP()), then that requirement should also be documented.
Acked-by: Laszlo Ersek <lersek@redhat.com>
Thanks
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#111222): https://edk2.groups.io/g/devel/message/111222
Mute This Topic: https://groups.io/mt/102479007/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
^ permalink raw reply [flat|nested] 12+ messages in thread