public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] Proposing v3 of MM communicate buffer
@ 2024-08-07 18:14 Kun Qin via groups.io
  2024-08-07 20:25 ` Kun Qin via groups.io
  2024-08-08  8:14 ` Ni, Ray
  0 siblings, 2 replies; 8+ messages in thread
From: Kun Qin via groups.io @ 2024-08-07 18:14 UTC (permalink / raw)
  To: devel@edk2.groups.io

[-- Attachment #1: Type: text/plain, Size: 1553 bytes --]

Hi all,

I am trying to propose a change into PI spec and would like to gather some feedback in this forum.

Essentially, the current communicate header contains a UINTN field in place, which is causing programing
errors when trying to communicate the message between different operation mode (i.e. PEI in IA32
communicate into MM in x64). There are various implementations at large to compensate for this
size discrepancy through the edk2 codebase, thus fixing the existing communicate buffer definition
will be less feasible. Thus I think proposing a new structure and implement the corresponding header
parser will be a simpler approach, which also allows a bit more flexibility to inject new features/checks
into the communication channel.

The proposed change for the spec is detailed here:
https://github.com/kuqin12/edk2/blob/BZ3398-MmCommunicate-Length-v4/CodeFirst/BZ3430-SpecChange.md

And the code first change is listed here:
https://github.com/kuqin12/edk2/blob/BZ3398-MmCommunicate-Length-v4/

Could you please provide me with any feedback that you think might be helpful for future usage of MM
communicate? Any input is appreciated.

Regards,
Kun



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120286): https://edk2.groups.io/g/devel/message/120286
Mute This Topic: https://groups.io/mt/107775882/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 6539 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] Proposing v3 of MM communicate buffer
  2024-08-07 18:14 [edk2-devel] Proposing v3 of MM communicate buffer Kun Qin via groups.io
@ 2024-08-07 20:25 ` Kun Qin via groups.io
  2024-08-08  8:14 ` Ni, Ray
  1 sibling, 0 replies; 8+ messages in thread
From: Kun Qin via groups.io @ 2024-08-07 20:25 UTC (permalink / raw)
  To: devel@edk2.groups.io
  Cc: Kinney, Michael D, gaoliming, 'Liu, Zhiguang',
	afish@apple.com, leif@nuviainc.com, jiaxin.wu@intel.com, Ni, Ray

[-- Attachment #1: Type: text/plain, Size: 1727 bytes --]

Adding stakeholders.

Regards,
Kun

From: Kun Qin
Sent: Wednesday, August 7, 2024 11:15 AM
To: devel@edk2.groups.io
Subject: Proposing v3 of MM communicate buffer

Hi all,

I am trying to propose a change into PI spec and would like to gather some feedback in this forum.

Essentially, the current communicate header contains a UINTN field in place, which is causing programing
errors when trying to communicate the message between different operation mode (i.e. PEI in IA32
communicate into MM in x64). There are various implementations at large to compensate for this
size discrepancy through the edk2 codebase, thus fixing the existing communicate buffer definition
will be less feasible. Thus I think proposing a new structure and implement the corresponding header
parser will be a simpler approach, which also allows a bit more flexibility to inject new features/checks
into the communication channel.

The proposed change for the spec is detailed here:
https://github.com/kuqin12/edk2/blob/BZ3398-MmCommunicate-Length-v4/CodeFirst/BZ3430-SpecChange.md

And the code first change is listed here:
https://github.com/kuqin12/edk2/blob/BZ3398-MmCommunicate-Length-v4/

Could you please provide me with any feedback that you think might be helpful for future usage of MM
communicate? Any input is appreciated.

Regards,
Kun



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120287): https://edk2.groups.io/g/devel/message/120287
Mute This Topic: https://groups.io/mt/107775882/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 7830 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] Proposing v3 of MM communicate buffer
  2024-08-07 18:14 [edk2-devel] Proposing v3 of MM communicate buffer Kun Qin via groups.io
  2024-08-07 20:25 ` Kun Qin via groups.io
@ 2024-08-08  8:14 ` Ni, Ray
  2024-08-08 17:41   ` Kun Qin via groups.io
  1 sibling, 1 reply; 8+ messages in thread
From: Ni, Ray @ 2024-08-08  8:14 UTC (permalink / raw)
  To: devel@edk2.groups.io, Kun.Qin@microsoft.com
  Cc: Wu, Jiaxin, Tan, Dun, Xu, Wei6, Zhang, Hongbin1, Ni, Ray,
	Kinney, Michael D

[-- Attachment #1: Type: text/plain, Size: 2638 bytes --]

Kun,
I like your proposed solution as it is backward compatible.

But, I think the new PPI/Protocol is only useful when the CPU mode where PPI/Protocol is produced does not match the CPU mode in MM.

In X86, it could be: 32bit PEI + 64bit MM, 32bit DXE + 64bit MM, or vice versa. But I doubt the value of support these combinations in X86. Because that means the IPL (either PEI or DXE module) needs to support invoking MM Core in a different CPU mode.
And the latest X86 platforms are switching to 64bit PEI + 64bit DXE + 64bit MM.

Does the proposal try to solve some ARM problem? Can you explain the necessity? I would like to avoid the complicated interfaces which do not solve a practical problem.

Thanks,
Ray

________________________________
From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Kun Qin via groups.io <Kun.Qin=microsoft.com@groups.io>
Sent: Thursday, August 8, 2024 2:14
To: devel@edk2.groups.io <devel@edk2.groups.io>
Subject: [edk2-devel] Proposing v3 of MM communicate buffer


Hi all,



I am trying to propose a change into PI spec and would like to gather some feedback in this forum.



Essentially, the current communicate header contains a UINTN field in place, which is causing programing

errors when trying to communicate the message between different operation mode (i.e. PEI in IA32

communicate into MM in x64). There are various implementations at large to compensate for this

size discrepancy through the edk2 codebase, thus fixing the existing communicate buffer definition

will be less feasible. Thus I think proposing a new structure and implement the corresponding header

parser will be a simpler approach, which also allows a bit more flexibility to inject new features/checks

into the communication channel.



The proposed change for the spec is detailed here:

https://github.com/kuqin12/edk2/blob/BZ3398-MmCommunicate-Length-v4/CodeFirst/BZ3430-SpecChange.md



And the code first change is listed here:

https://github.com/kuqin12/edk2/blob/BZ3398-MmCommunicate-Length-v4/



Could you please provide me with any feedback that you think might be helpful for future usage of MM

communicate? Any input is appreciated.



Regards,

Kun






-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120291): https://edk2.groups.io/g/devel/message/120291
Mute This Topic: https://groups.io/mt/107775882/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 9929 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] Proposing v3 of MM communicate buffer
  2024-08-08  8:14 ` Ni, Ray
@ 2024-08-08 17:41   ` Kun Qin via groups.io
  2024-08-09  3:47     ` Ni, Ray
  0 siblings, 1 reply; 8+ messages in thread
From: Kun Qin via groups.io @ 2024-08-08 17:41 UTC (permalink / raw)
  To: Ni, Ray, devel@edk2.groups.io
  Cc: Wu, Jiaxin, Tan, Dun, Xu, Wei6, Zhang, Hongbin1, Ni, Ray,
	Kinney, Michael D

[-- Attachment #1: Type: text/plain, Size: 4211 bytes --]

Hi Ray,

Thanks for your feedback. The ARM platforms I was exposed to have consistent operation mode is only AARCH64, so this proposal is not particularly attached to any ARM problem.

I agree that 32bit PEI/DXE communicate into MM will have issue on x86 platforms as of today. But I have only heard Intel processors moving to support x64 PEI/DXE. I think introducing a new MM communicate header will help to prevent the issue to propagate much further as it might take non-Intel x86 platforms years to fully move away from 32bit PEI/DXE. Please let me know if you have any thoughts.

Regards,
Kun

P.S. Project MU have a thunking module that can launch x64 MM core from 32bit environment: mu_feature_mm_supv/MmSupervisorPkg/Drivers/MmPeiLaunchers/MmIplX64Relay.inf at main * microsoft/mu_feature_mm_supv (github.com)<https://github.com/microsoft/mu_feature_mm_supv/blob/main/MmSupervisorPkg/Drivers/MmPeiLaunchers/MmIplX64Relay.inf>. We can upstream it to edk2 if folks think it will help.

From: Ni, Ray <ray.ni@intel.com>
Sent: Thursday, August 8, 2024 1:15 AM
To: devel@edk2.groups.io; Kun Qin <Kun.Qin@microsoft.com>
Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Tan, Dun <dun.tan@intel.com>; Xu, Wei6 <wei6.xu@intel.com>; Zhang, Hongbin1 <hongbin1.zhang@intel.com>; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: [EXTERNAL] Re: Proposing v3 of MM communicate buffer

Kun,
I like your proposed solution as it is backward compatible.

But, I think the new PPI/Protocol is only useful when the CPU mode where PPI/Protocol is produced does not match the CPU mode in MM.

In X86, it could be: 32bit PEI + 64bit MM, 32bit DXE + 64bit MM, or vice versa. But I doubt the value of support these combinations in X86. Because that means the IPL (either PEI or DXE module) needs to support invoking MM Core in a different CPU mode.
And the latest X86 platforms are switching to 64bit PEI + 64bit DXE + 64bit MM.

Does the proposal try to solve some ARM problem? Can you explain the necessity? I would like to avoid the complicated interfaces which do not solve a practical problem.

Thanks,
Ray

________________________________
From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> on behalf of Kun Qin via groups.io <Kun.Qin=microsoft.com@groups.io<mailto:Kun.Qin=microsoft.com@groups.io>>
Sent: Thursday, August 8, 2024 2:14
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>
Subject: [edk2-devel] Proposing v3 of MM communicate buffer


Hi all,



I am trying to propose a change into PI spec and would like to gather some feedback in this forum.



Essentially, the current communicate header contains a UINTN field in place, which is causing programing

errors when trying to communicate the message between different operation mode (i.e. PEI in IA32

communicate into MM in x64). There are various implementations at large to compensate for this

size discrepancy through the edk2 codebase, thus fixing the existing communicate buffer definition

will be less feasible. Thus I think proposing a new structure and implement the corresponding header

parser will be a simpler approach, which also allows a bit more flexibility to inject new features/checks

into the communication channel.



The proposed change for the spec is detailed here:

https://github.com/kuqin12/edk2/blob/BZ3398-MmCommunicate-Length-v4/CodeFirst/BZ3430-SpecChange.md



And the code first change is listed here:

https://github.com/kuqin12/edk2/blob/BZ3398-MmCommunicate-Length-v4/



Could you please provide me with any feedback that you think might be helpful for future usage of MM

communicate? Any input is appreciated.



Regards,

Kun





-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120307): https://edk2.groups.io/g/devel/message/120307
Mute This Topic: https://groups.io/mt/107775882/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 13836 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] Proposing v3 of MM communicate buffer
  2024-08-08 17:41   ` Kun Qin via groups.io
@ 2024-08-09  3:47     ` Ni, Ray
  2024-08-09  7:58       ` Kun Qin via groups.io
  0 siblings, 1 reply; 8+ messages in thread
From: Ni, Ray @ 2024-08-09  3:47 UTC (permalink / raw)
  To: Kun Qin, devel@edk2.groups.io
  Cc: Wu, Jiaxin, Tan, Dun, Xu, Wei6, Zhang, Hongbin1,
	Kinney, Michael D

[-- Attachment #1: Type: text/plain, Size: 5059 bytes --]

Kun, thanks for explaining that ARM does not require this.

IMO, the proposal is only useful in X86 when:
* BIOS uses standalone MM
* BIOS launches standalone MM in PEI
* BIOS PEI runs in 32bit mode

I do not see any such platform in open source as the X86 standalone MM is not available yet in edk2 trunk.

Thanks,
Ray
________________________________
From: Kun Qin <Kun.Qin@microsoft.com>
Sent: Friday, August 9, 2024 1:41
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Tan, Dun <dun.tan@intel.com>; Xu, Wei6 <wei6.xu@intel.com>; Zhang, Hongbin1 <hongbin1.zhang@intel.com>; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: Proposing v3 of MM communicate buffer


Hi Ray,



Thanks for your feedback. The ARM platforms I was exposed to have consistent operation mode is only AARCH64, so this proposal is not particularly attached to any ARM problem.



I agree that 32bit PEI/DXE communicate into MM will have issue on x86 platforms as of today. But I have only heard Intel processors moving to support x64 PEI/DXE. I think introducing a new MM communicate header will help to prevent the issue to propagate much further as it might take non-Intel x86 platforms years to fully move away from 32bit PEI/DXE. Please let me know if you have any thoughts.



Regards,

Kun



P.S. Project MU have a thunking module that can launch x64 MM core from 32bit environment: mu_feature_mm_supv/MmSupervisorPkg/Drivers/MmPeiLaunchers/MmIplX64Relay.inf at main · microsoft/mu_feature_mm_supv (github.com)<https://github.com/microsoft/mu_feature_mm_supv/blob/main/MmSupervisorPkg/Drivers/MmPeiLaunchers/MmIplX64Relay.inf>. We can upstream it to edk2 if folks think it will help.



From: Ni, Ray <ray.ni@intel.com>
Sent: Thursday, August 8, 2024 1:15 AM
To: devel@edk2.groups.io; Kun Qin <Kun.Qin@microsoft.com>
Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Tan, Dun <dun.tan@intel.com>; Xu, Wei6 <wei6.xu@intel.com>; Zhang, Hongbin1 <hongbin1.zhang@intel.com>; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: [EXTERNAL] Re: Proposing v3 of MM communicate buffer



Kun,

I like your proposed solution as it is backward compatible.



But, I think the new PPI/Protocol is only useful when the CPU mode where PPI/Protocol is produced does not match the CPU mode in MM.



In X86, it could be: 32bit PEI + 64bit MM, 32bit DXE + 64bit MM, or vice versa. But I doubt the value of support these combinations in X86. Because that means the IPL (either PEI or DXE module) needs to support invoking MM Core in a different CPU mode.

And the latest X86 platforms are switching to 64bit PEI + 64bit DXE + 64bit MM.



Does the proposal try to solve some ARM problem? Can you explain the necessity? I would like to avoid the complicated interfaces which do not solve a practical problem.



Thanks,

Ray



________________________________

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> on behalf of Kun Qin via groups.io <Kun.Qin=microsoft.com@groups.io<mailto:Kun.Qin=microsoft.com@groups.io>>
Sent: Thursday, August 8, 2024 2:14
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>
Subject: [edk2-devel] Proposing v3 of MM communicate buffer



Hi all,



I am trying to propose a change into PI spec and would like to gather some feedback in this forum.



Essentially, the current communicate header contains a UINTN field in place, which is causing programing

errors when trying to communicate the message between different operation mode (i.e. PEI in IA32

communicate into MM in x64). There are various implementations at large to compensate for this

size discrepancy through the edk2 codebase, thus fixing the existing communicate buffer definition

will be less feasible. Thus I think proposing a new structure and implement the corresponding header

parser will be a simpler approach, which also allows a bit more flexibility to inject new features/checks

into the communication channel.



The proposed change for the spec is detailed here:

https://github.com/kuqin12/edk2/blob/BZ3398-MmCommunicate-Length-v4/CodeFirst/BZ3430-SpecChange.md



And the code first change is listed here:

https://github.com/kuqin12/edk2/blob/BZ3398-MmCommunicate-Length-v4/



Could you please provide me with any feedback that you think might be helpful for future usage of MM

communicate? Any input is appreciated.



Regards,

Kun






-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120304): https://edk2.groups.io/g/devel/message/120304
Mute This Topic: https://groups.io/mt/107775882/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 15439 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] Proposing v3 of MM communicate buffer
  2024-08-09  3:47     ` Ni, Ray
@ 2024-08-09  7:58       ` Kun Qin via groups.io
  2024-08-12  5:50         ` Ni, Ray
  0 siblings, 1 reply; 8+ messages in thread
From: Kun Qin via groups.io @ 2024-08-09  7:58 UTC (permalink / raw)
  To: Ni, Ray, devel@edk2.groups.io
  Cc: Wu, Jiaxin, Tan, Dun, Xu, Wei6, Zhang, Hongbin1,
	Kinney, Michael D

[-- Attachment #1: Type: text/plain, Size: 7063 bytes --]

Hi Ray,

Your description is correct. I believe there are efforts trying to make Standalone MM launch in PEI happen? i.e. Add standalone mm ipl pei driver by hongbin123 · Pull Request #5236 · tianocore/edk2 (github.com)<https://github.com/tianocore/edk2/pull/5236> will bring in support for #1 and #2. Thanks to Hongbin explaining to me that such efforts are mainly targeting 64bit PEI. But I do not think it takes much more work to support 32bit PEI + Standalone MM once this groundwork is done (again, we can help upstream the code that supports #3 from Project MU if so desired 😊).

Moreover, just because there are no platforms in the public have such configuration, I would perceive it as a great opportunity to introduce the new MM communicate header definition to prevent further contamination from the old definition. In as the new platforms can setup MM foundation in PEI (32 or 64 bit, Intel or AMD) and communicate under the new definition of data structure and PPI.

Please let me know your thoughts. Your feedback is really appreciated!

Regards,
Kun

From: Ni, Ray <ray.ni@intel.com>
Sent: Thursday, August 8, 2024 8:48 PM
To: Kun Qin <Kun.Qin@microsoft.com>; devel@edk2.groups.io
Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Tan, Dun <dun.tan@intel.com>; Xu, Wei6 <wei6.xu@intel.com>; Zhang, Hongbin1 <hongbin1.zhang@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: [EXTERNAL] Re: Proposing v3 of MM communicate buffer

Kun, thanks for explaining that ARM does not require this.

IMO, the proposal is only useful in X86 when:
* BIOS uses standalone MM
* BIOS launches standalone MM in PEI
* BIOS PEI runs in 32bit mode

I do not see any such platform in open source as the X86 standalone MM is not available yet in edk2 trunk.

Thanks,
Ray
________________________________
From: Kun Qin <Kun.Qin@microsoft.com<mailto:Kun.Qin@microsoft.com>>
Sent: Friday, August 9, 2024 1:41
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>
Cc: Wu, Jiaxin <jiaxin.wu@intel.com<mailto:jiaxin.wu@intel.com>>; Tan, Dun <dun.tan@intel.com<mailto:dun.tan@intel.com>>; Xu, Wei6 <wei6.xu@intel.com<mailto:wei6.xu@intel.com>>; Zhang, Hongbin1 <hongbin1.zhang@intel.com<mailto:hongbin1.zhang@intel.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: RE: Proposing v3 of MM communicate buffer


Hi Ray,



Thanks for your feedback. The ARM platforms I was exposed to have consistent operation mode is only AARCH64, so this proposal is not particularly attached to any ARM problem.



I agree that 32bit PEI/DXE communicate into MM will have issue on x86 platforms as of today. But I have only heard Intel processors moving to support x64 PEI/DXE. I think introducing a new MM communicate header will help to prevent the issue to propagate much further as it might take non-Intel x86 platforms years to fully move away from 32bit PEI/DXE. Please let me know if you have any thoughts.



Regards,

Kun



P.S. Project MU have a thunking module that can launch x64 MM core from 32bit environment: mu_feature_mm_supv/MmSupervisorPkg/Drivers/MmPeiLaunchers/MmIplX64Relay.inf at main · microsoft/mu_feature_mm_supv (github.com)<https://github.com/microsoft/mu_feature_mm_supv/blob/main/MmSupervisorPkg/Drivers/MmPeiLaunchers/MmIplX64Relay.inf>. We can upstream it to edk2 if folks think it will help.



From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Thursday, August 8, 2024 1:15 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Kun Qin <Kun.Qin@microsoft.com<mailto:Kun.Qin@microsoft.com>>
Cc: Wu, Jiaxin <jiaxin.wu@intel.com<mailto:jiaxin.wu@intel.com>>; Tan, Dun <dun.tan@intel.com<mailto:dun.tan@intel.com>>; Xu, Wei6 <wei6.xu@intel.com<mailto:wei6.xu@intel.com>>; Zhang, Hongbin1 <hongbin1.zhang@intel.com<mailto:hongbin1.zhang@intel.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: [EXTERNAL] Re: Proposing v3 of MM communicate buffer



Kun,

I like your proposed solution as it is backward compatible.



But, I think the new PPI/Protocol is only useful when the CPU mode where PPI/Protocol is produced does not match the CPU mode in MM.



In X86, it could be: 32bit PEI + 64bit MM, 32bit DXE + 64bit MM, or vice versa. But I doubt the value of support these combinations in X86. Because that means the IPL (either PEI or DXE module) needs to support invoking MM Core in a different CPU mode.

And the latest X86 platforms are switching to 64bit PEI + 64bit DXE + 64bit MM.



Does the proposal try to solve some ARM problem? Can you explain the necessity? I would like to avoid the complicated interfaces which do not solve a practical problem.



Thanks,

Ray



________________________________

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> on behalf of Kun Qin via groups.io <Kun.Qin=microsoft.com@groups.io<mailto:Kun.Qin=microsoft.com@groups.io>>
Sent: Thursday, August 8, 2024 2:14
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>
Subject: [edk2-devel] Proposing v3 of MM communicate buffer



Hi all,



I am trying to propose a change into PI spec and would like to gather some feedback in this forum.



Essentially, the current communicate header contains a UINTN field in place, which is causing programing

errors when trying to communicate the message between different operation mode (i.e. PEI in IA32

communicate into MM in x64). There are various implementations at large to compensate for this

size discrepancy through the edk2 codebase, thus fixing the existing communicate buffer definition

will be less feasible. Thus I think proposing a new structure and implement the corresponding header

parser will be a simpler approach, which also allows a bit more flexibility to inject new features/checks

into the communication channel.



The proposed change for the spec is detailed here:

https://github.com/kuqin12/edk2/blob/BZ3398-MmCommunicate-Length-v4/CodeFirst/BZ3430-SpecChange.md



And the code first change is listed here:

https://github.com/kuqin12/edk2/blob/BZ3398-MmCommunicate-Length-v4/



Could you please provide me with any feedback that you think might be helpful for future usage of MM

communicate? Any input is appreciated.



Regards,

Kun






-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120310): https://edk2.groups.io/g/devel/message/120310
Mute This Topic: https://groups.io/mt/107775882/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 19642 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] Proposing v3 of MM communicate buffer
  2024-08-09  7:58       ` Kun Qin via groups.io
@ 2024-08-12  5:50         ` Ni, Ray
  2024-08-12 19:07           ` Kun Qin via groups.io
  0 siblings, 1 reply; 8+ messages in thread
From: Ni, Ray @ 2024-08-12  5:50 UTC (permalink / raw)
  To: Kun Qin, devel@edk2.groups.io
  Cc: Wu, Jiaxin, Tan, Dun, Xu, Wei6, Zhang, Hongbin1,
	Kinney, Michael D, Zimmer, Vincent

[-- Attachment #1: Type: text/plain, Size: 8064 bytes --]

Kun,
This PR (Mm infra by jiaxinwu · Pull Request #5914<https://github.com/tianocore/edk2/pull/5914>) contains all standalone MM infra related code in edk2 repo. Please take a look at if you are interested in it.

Back to your proposal, can we revisit it once there are platforms that want to invoke 64bit MM env from 32bit PEI? I personally do not prefer to make the interfaces complicated to support a platform that only exists in theory.

+ Vincent for comments from PI spec perspective.

Thanks,
Ray
________________________________
From: Kun Qin <Kun.Qin@microsoft.com>
Sent: Friday, August 9, 2024 15:58
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Tan, Dun <dun.tan@intel.com>; Xu, Wei6 <wei6.xu@intel.com>; Zhang, Hongbin1 <hongbin1.zhang@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: Proposing v3 of MM communicate buffer


Hi Ray,



Your description is correct. I believe there are efforts trying to make Standalone MM launch in PEI happen? i.e. Add standalone mm ipl pei driver by hongbin123 · Pull Request #5236 · tianocore/edk2 (github.com)<https://github.com/tianocore/edk2/pull/5236> will bring in support for #1 and #2. Thanks to Hongbin explaining to me that such efforts are mainly targeting 64bit PEI. But I do not think it takes much more work to support 32bit PEI + Standalone MM once this groundwork is done (again, we can help upstream the code that supports #3 from Project MU if so desired ??).



Moreover, just because there are no platforms in the public have such configuration, I would perceive it as a great opportunity to introduce the new MM communicate header definition to prevent further contamination from the old definition. In as the new platforms can setup MM foundation in PEI (32 or 64 bit, Intel or AMD) and communicate under the new definition of data structure and PPI.



Please let me know your thoughts. Your feedback is really appreciated!



Regards,

Kun



From: Ni, Ray <ray.ni@intel.com>
Sent: Thursday, August 8, 2024 8:48 PM
To: Kun Qin <Kun.Qin@microsoft.com>; devel@edk2.groups.io
Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Tan, Dun <dun.tan@intel.com>; Xu, Wei6 <wei6.xu@intel.com>; Zhang, Hongbin1 <hongbin1.zhang@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: [EXTERNAL] Re: Proposing v3 of MM communicate buffer



Kun, thanks for explaining that ARM does not require this.



IMO, the proposal is only useful in X86 when:

* BIOS uses standalone MM

* BIOS launches standalone MM in PEI

* BIOS PEI runs in 32bit mode



I do not see any such platform in open source as the X86 standalone MM is not available yet in edk2 trunk.



Thanks,

Ray

________________________________

From: Kun Qin <Kun.Qin@microsoft.com<mailto:Kun.Qin@microsoft.com>>
Sent: Friday, August 9, 2024 1:41
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>
Cc: Wu, Jiaxin <jiaxin.wu@intel.com<mailto:jiaxin.wu@intel.com>>; Tan, Dun <dun.tan@intel.com<mailto:dun.tan@intel.com>>; Xu, Wei6 <wei6.xu@intel.com<mailto:wei6.xu@intel.com>>; Zhang, Hongbin1 <hongbin1.zhang@intel.com<mailto:hongbin1.zhang@intel.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: RE: Proposing v3 of MM communicate buffer



Hi Ray,



Thanks for your feedback. The ARM platforms I was exposed to have consistent operation mode is only AARCH64, so this proposal is not particularly attached to any ARM problem.



I agree that 32bit PEI/DXE communicate into MM will have issue on x86 platforms as of today. But I have only heard Intel processors moving to support x64 PEI/DXE. I think introducing a new MM communicate header will help to prevent the issue to propagate much further as it might take non-Intel x86 platforms years to fully move away from 32bit PEI/DXE. Please let me know if you have any thoughts.



Regards,

Kun



P.S. Project MU have a thunking module that can launch x64 MM core from 32bit environment: mu_feature_mm_supv/MmSupervisorPkg/Drivers/MmPeiLaunchers/MmIplX64Relay.inf at main · microsoft/mu_feature_mm_supv (github.com)<https://github.com/microsoft/mu_feature_mm_supv/blob/main/MmSupervisorPkg/Drivers/MmPeiLaunchers/MmIplX64Relay.inf>. We can upstream it to edk2 if folks think it will help.



From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Thursday, August 8, 2024 1:15 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Kun Qin <Kun.Qin@microsoft.com<mailto:Kun.Qin@microsoft.com>>
Cc: Wu, Jiaxin <jiaxin.wu@intel.com<mailto:jiaxin.wu@intel.com>>; Tan, Dun <dun.tan@intel.com<mailto:dun.tan@intel.com>>; Xu, Wei6 <wei6.xu@intel.com<mailto:wei6.xu@intel.com>>; Zhang, Hongbin1 <hongbin1.zhang@intel.com<mailto:hongbin1.zhang@intel.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: [EXTERNAL] Re: Proposing v3 of MM communicate buffer



Kun,

I like your proposed solution as it is backward compatible.



But, I think the new PPI/Protocol is only useful when the CPU mode where PPI/Protocol is produced does not match the CPU mode in MM.



In X86, it could be: 32bit PEI + 64bit MM, 32bit DXE + 64bit MM, or vice versa. But I doubt the value of support these combinations in X86. Because that means the IPL (either PEI or DXE module) needs to support invoking MM Core in a different CPU mode.

And the latest X86 platforms are switching to 64bit PEI + 64bit DXE + 64bit MM.



Does the proposal try to solve some ARM problem? Can you explain the necessity? I would like to avoid the complicated interfaces which do not solve a practical problem.



Thanks,

Ray



________________________________

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> on behalf of Kun Qin via groups.io <Kun.Qin=microsoft.com@groups.io<mailto:Kun.Qin=microsoft.com@groups.io>>
Sent: Thursday, August 8, 2024 2:14
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>
Subject: [edk2-devel] Proposing v3 of MM communicate buffer



Hi all,



I am trying to propose a change into PI spec and would like to gather some feedback in this forum.



Essentially, the current communicate header contains a UINTN field in place, which is causing programing

errors when trying to communicate the message between different operation mode (i.e. PEI in IA32

communicate into MM in x64). There are various implementations at large to compensate for this

size discrepancy through the edk2 codebase, thus fixing the existing communicate buffer definition

will be less feasible. Thus I think proposing a new structure and implement the corresponding header

parser will be a simpler approach, which also allows a bit more flexibility to inject new features/checks

into the communication channel.



The proposed change for the spec is detailed here:

https://github.com/kuqin12/edk2/blob/BZ3398-MmCommunicate-Length-v4/CodeFirst/BZ3430-SpecChange.md



And the code first change is listed here:

https://github.com/kuqin12/edk2/blob/BZ3398-MmCommunicate-Length-v4/



Could you please provide me with any feedback that you think might be helpful for future usage of MM

communicate? Any input is appreciated.



Regards,

Kun






-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120317): https://edk2.groups.io/g/devel/message/120317
Mute This Topic: https://groups.io/mt/107775882/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 20930 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [edk2-devel] Proposing v3 of MM communicate buffer
  2024-08-12  5:50         ` Ni, Ray
@ 2024-08-12 19:07           ` Kun Qin via groups.io
  0 siblings, 0 replies; 8+ messages in thread
From: Kun Qin via groups.io @ 2024-08-12 19:07 UTC (permalink / raw)
  To: Ni, Ray, devel@edk2.groups.io
  Cc: Wu, Jiaxin, Tan, Dun, Xu, Wei6, Zhang, Hongbin1,
	Kinney, Michael D, Zimmer, Vincent

[-- Attachment #1: Type: text/plain, Size: 9143 bytes --]

Hi Ray,

Thanks for pointing me to the MM infra PR. Will definitely take a look.

We have had our first party device launching x64 Standalone MM from IA32 PEI for a few year, which was also why this original proposal was brought up back then (obviously it got delayed for various reasons..). But I think it would be less ideal to rely on the system operating with 64 bit mode to match the prevailing code implementation at large to cover the specification defect.

Please let me know if you have any other thoughts. Other stakeholders are welcome to chime in as well.

Thanks,
Kun

________________________________
From: Ni, Ray <ray.ni@intel.com>
Sent: Sunday, August 11, 2024 10:50 PM
To: Kun Qin <Kun.Qin@microsoft.com>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Tan, Dun <dun.tan@intel.com>; Xu, Wei6 <wei6.xu@intel.com>; Zhang, Hongbin1 <hongbin1.zhang@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Zimmer, Vincent <vincent.zimmer@intel.com>
Subject: [EXTERNAL] Re: Proposing v3 of MM communicate buffer

Kun,
This PR (Mm infra by jiaxinwu · Pull Request #5914<https://github.com/tianocore/edk2/pull/5914>) contains all standalone MM infra related code in edk2 repo. Please take a look at if you are interested in it.

Back to your proposal, can we revisit it once there are platforms that want to invoke 64bit MM env from 32bit PEI? I personally do not prefer to make the interfaces complicated to support a platform that only exists in theory.

+ Vincent for comments from PI spec perspective.

Thanks,
Ray
________________________________
From: Kun Qin <Kun.Qin@microsoft.com>
Sent: Friday, August 9, 2024 15:58
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Tan, Dun <dun.tan@intel.com>; Xu, Wei6 <wei6.xu@intel.com>; Zhang, Hongbin1 <hongbin1.zhang@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: Proposing v3 of MM communicate buffer


Hi Ray,



Your description is correct. I believe there are efforts trying to make Standalone MM launch in PEI happen? i.e. Add standalone mm ipl pei driver by hongbin123 · Pull Request #5236 · tianocore/edk2 (github.com)<https://github.com/tianocore/edk2/pull/5236> will bring in support for #1 and #2. Thanks to Hongbin explaining to me that such efforts are mainly targeting 64bit PEI. But I do not think it takes much more work to support 32bit PEI + Standalone MM once this groundwork is done (again, we can help upstream the code that supports #3 from Project MU if so desired 😊).



Moreover, just because there are no platforms in the public have such configuration, I would perceive it as a great opportunity to introduce the new MM communicate header definition to prevent further contamination from the old definition. In as the new platforms can setup MM foundation in PEI (32 or 64 bit, Intel or AMD) and communicate under the new definition of data structure and PPI.



Please let me know your thoughts. Your feedback is really appreciated!



Regards,

Kun



From: Ni, Ray <ray.ni@intel.com>
Sent: Thursday, August 8, 2024 8:48 PM
To: Kun Qin <Kun.Qin@microsoft.com>; devel@edk2.groups.io
Cc: Wu, Jiaxin <jiaxin.wu@intel.com>; Tan, Dun <dun.tan@intel.com>; Xu, Wei6 <wei6.xu@intel.com>; Zhang, Hongbin1 <hongbin1.zhang@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: [EXTERNAL] Re: Proposing v3 of MM communicate buffer



Kun, thanks for explaining that ARM does not require this.



IMO, the proposal is only useful in X86 when:

* BIOS uses standalone MM

* BIOS launches standalone MM in PEI

* BIOS PEI runs in 32bit mode



I do not see any such platform in open source as the X86 standalone MM is not available yet in edk2 trunk.



Thanks,

Ray

________________________________

From: Kun Qin <Kun.Qin@microsoft.com<mailto:Kun.Qin@microsoft.com>>
Sent: Friday, August 9, 2024 1:41
To: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>
Cc: Wu, Jiaxin <jiaxin.wu@intel.com<mailto:jiaxin.wu@intel.com>>; Tan, Dun <dun.tan@intel.com<mailto:dun.tan@intel.com>>; Xu, Wei6 <wei6.xu@intel.com<mailto:wei6.xu@intel.com>>; Zhang, Hongbin1 <hongbin1.zhang@intel.com<mailto:hongbin1.zhang@intel.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: RE: Proposing v3 of MM communicate buffer



Hi Ray,



Thanks for your feedback. The ARM platforms I was exposed to have consistent operation mode is only AARCH64, so this proposal is not particularly attached to any ARM problem.



I agree that 32bit PEI/DXE communicate into MM will have issue on x86 platforms as of today. But I have only heard Intel processors moving to support x64 PEI/DXE. I think introducing a new MM communicate header will help to prevent the issue to propagate much further as it might take non-Intel x86 platforms years to fully move away from 32bit PEI/DXE. Please let me know if you have any thoughts.



Regards,

Kun



P.S. Project MU have a thunking module that can launch x64 MM core from 32bit environment: mu_feature_mm_supv/MmSupervisorPkg/Drivers/MmPeiLaunchers/MmIplX64Relay.inf at main · microsoft/mu_feature_mm_supv (github.com)<https://github.com/microsoft/mu_feature_mm_supv/blob/main/MmSupervisorPkg/Drivers/MmPeiLaunchers/MmIplX64Relay.inf>. We can upstream it to edk2 if folks think it will help.



From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Thursday, August 8, 2024 1:15 AM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Kun Qin <Kun.Qin@microsoft.com<mailto:Kun.Qin@microsoft.com>>
Cc: Wu, Jiaxin <jiaxin.wu@intel.com<mailto:jiaxin.wu@intel.com>>; Tan, Dun <dun.tan@intel.com<mailto:dun.tan@intel.com>>; Xu, Wei6 <wei6.xu@intel.com<mailto:wei6.xu@intel.com>>; Zhang, Hongbin1 <hongbin1.zhang@intel.com<mailto:hongbin1.zhang@intel.com>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
Subject: [EXTERNAL] Re: Proposing v3 of MM communicate buffer



Kun,

I like your proposed solution as it is backward compatible.



But, I think the new PPI/Protocol is only useful when the CPU mode where PPI/Protocol is produced does not match the CPU mode in MM.



In X86, it could be: 32bit PEI + 64bit MM, 32bit DXE + 64bit MM, or vice versa. But I doubt the value of support these combinations in X86. Because that means the IPL (either PEI or DXE module) needs to support invoking MM Core in a different CPU mode.

And the latest X86 platforms are switching to 64bit PEI + 64bit DXE + 64bit MM.



Does the proposal try to solve some ARM problem? Can you explain the necessity? I would like to avoid the complicated interfaces which do not solve a practical problem.



Thanks,

Ray



________________________________

From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> on behalf of Kun Qin via groups.io <Kun.Qin=microsoft.com@groups.io<mailto:Kun.Qin=microsoft.com@groups.io>>
Sent: Thursday, August 8, 2024 2:14
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>
Subject: [edk2-devel] Proposing v3 of MM communicate buffer



Hi all,



I am trying to propose a change into PI spec and would like to gather some feedback in this forum.



Essentially, the current communicate header contains a UINTN field in place, which is causing programing

errors when trying to communicate the message between different operation mode (i.e. PEI in IA32

communicate into MM in x64). There are various implementations at large to compensate for this

size discrepancy through the edk2 codebase, thus fixing the existing communicate buffer definition

will be less feasible. Thus I think proposing a new structure and implement the corresponding header

parser will be a simpler approach, which also allows a bit more flexibility to inject new features/checks

into the communication channel.



The proposed change for the spec is detailed here:

https://github.com/kuqin12/edk2/blob/BZ3398-MmCommunicate-Length-v4/CodeFirst/BZ3430-SpecChange.md



And the code first change is listed here:

https://github.com/kuqin12/edk2/blob/BZ3398-MmCommunicate-Length-v4/



Could you please provide me with any feedback that you think might be helpful for future usage of MM

communicate? Any input is appreciated.



Regards,

Kun






-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#120319): https://edk2.groups.io/g/devel/message/120319
Mute This Topic: https://groups.io/mt/107775882/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



[-- Attachment #2: Type: text/html, Size: 35729 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2024-08-12 21:34 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-07 18:14 [edk2-devel] Proposing v3 of MM communicate buffer Kun Qin via groups.io
2024-08-07 20:25 ` Kun Qin via groups.io
2024-08-08  8:14 ` Ni, Ray
2024-08-08 17:41   ` Kun Qin via groups.io
2024-08-09  3:47     ` Ni, Ray
2024-08-09  7:58       ` Kun Qin via groups.io
2024-08-12  5:50         ` Ni, Ray
2024-08-12 19:07           ` Kun Qin via groups.io

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox