Re: [edk2-devel] [PATCH 1/1] MdeModulePkg/Bus/Pci/EhciDxe: Fix FORWARD_NULL Coverity issues

2023-07-11 Thread Wu, Hao A
Reviewed-by: Hao A Wu 

Best Regards,
Hao Wu

> -Original Message-
> From: Ranbir Singh 
> Sent: Monday, July 3, 2023 7:44 PM
> To: devel@edk2.groups.io; rsi...@ventanamicro.com
> Cc: Wu, Hao A ; Ni, Ray 
> Subject: [PATCH 1/1] MdeModulePkg/Bus/Pci/EhciDxe: Fix FORWARD_NULL
> Coverity issues
> 
> From: Ranbir Singh 
> 
> The function UsbHcGetPciAddressForHostMem has
> 
> ASSERT ((Block != NULL));
> 
> and the UsbHcFreeMem has
> 
> ASSERT (Block != NULL);
> 
> statement after for loop, but these are applicable only in DEBUG mode.
> In RELEASE mode, if for whatever reasons there is no match inside the
> for loop and the loop exits because of Block != NULL; condition, then
> there is no "Block" NULL pointer check afterwards and the code proceeds
> to do dereferencing "Block" which will lead to CRASH.
> 
> Hence, for safety add NULL pointer checks always.
> 
> Cc: Hao A Wu 
> Cc: Ray Ni 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4210
> Signed-off-by: Ranbir Singh 
> Signed-off-by: Ranbir Singh 
> ---
>  MdeModulePkg/Bus/Pci/EhciDxe/UsbHcMem.c | 9 +
>  1 file changed, 9 insertions(+)
> 
> diff --git a/MdeModulePkg/Bus/Pci/EhciDxe/UsbHcMem.c
> b/MdeModulePkg/Bus/Pci/EhciDxe/UsbHcMem.c
> index 0a3ceb9f711a..79575b6f6304 100644
> --- a/MdeModulePkg/Bus/Pci/EhciDxe/UsbHcMem.c
> +++ b/MdeModulePkg/Bus/Pci/EhciDxe/UsbHcMem.c
> @@ -250,6 +250,11 @@ UsbHcGetPciAddressForHostMem (
>}
> 
> 
> 
>ASSERT ((Block != NULL));
> 
> +
> 
> +  if (Block == NULL) {
> 
> +return 0;
> 
> +  }
> 
> +
> 
>//
> 
>// calculate the pci memory address for host memory address.
> 
>//
> 
> @@ -536,6 +541,10 @@ UsbHcFreeMem (
>//
> 
>ASSERT (Block != NULL);
> 
> 
> 
> +  if (Block == NULL) {
> 
> +return;
> 
> +  }
> 
> +
> 
>//
> 
>// Release the current memory block if it is empty and not the head
> 
>//
> 
> --
> 2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106846): https://edk2.groups.io/g/devel/message/106846
Mute This Topic: https://groups.io/mt/99936708/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v2 1/2] MdeModulePkg/Bus/Ata/AtaAtapiPassThru: Fix SIGN_EXTENSION Coverity issue

2023-07-11 Thread Wu, Hao A
Reviewed-by: Hao A Wu 

Best Regards,
Hao Wu

> -Original Message-
> From: Ranbir Singh 
> Sent: Friday, June 9, 2023 8:33 PM
> To: devel@edk2.groups.io; rsi...@ventanamicro.com
> Cc: Wu, Hao A ; Ni, Ray 
> Subject: [PATCH v2 1/2] MdeModulePkg/Bus/Ata/AtaAtapiPassThru: Fix
> SIGN_EXTENSION Coverity issue
> 
> From: Ranbir Singh 
> 
> Line number 1348 does contain a typecast with UINT32, but it is after
> all the operations (16-bit left shift followed by OR'ing) are over.
> To avoid any SIGN_EXTENSION, typecast the intermediate result after
> 16-bit left shift operation immediately with UINT32.
> 
> Cc: Hao A Wu 
> Cc: Ray Ni 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4204
> Signed-off-by: Ranbir Singh 
> Signed-off-by: Ranbir Singh 
> ---
> 
> Notes:
> Retain outer cast
> 
>  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
> b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
> index 50406fe0270d..f39c909d0631 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.c
> @@ -1345,7 +1345,7 @@ AtaPassThruPassThru (
>  // Check logical block size
> 
>  //
> 
>  if ((IdentifyData->AtaData.phy_logic_sector_support & BIT12) != 0) {
> 
> -  BlockSize = (UINT32)(((IdentifyData->AtaData.logic_sector_size_hi << 
> 16)
> | IdentifyData->AtaData.logic_sector_size_lo) * sizeof (UINT16));
> 
> +  BlockSize = (UINT32)(((UINT32)(IdentifyData-
> >AtaData.logic_sector_size_hi << 16) | IdentifyData-
> >AtaData.logic_sector_size_lo) * sizeof (UINT16));
> 
>  }
> 
>}
> 
> 
> 
> --
> 2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106845): https://edk2.groups.io/g/devel/message/106845
Mute This Topic: https://groups.io/mt/99432079/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH v2 2/2] MdeModulePkg/Bus/Ata/AtaAtapiPassThru: Fix UNUSED_VALUE Coverity issue

2023-07-11 Thread Wu, Hao A
Really sorry,

After referring to the Information Technology - AT Attachment with Packet 
Interface (ATA/ATAPI) Specification,
It seems to me that the commands being executed in function 
SetDriveParameters() are not mandatory during device initialization.

1. INITIALIZE DEVICE PARAMETERS command (ID 0x91h):
This command got obsoleted since ATA/ATAPI-6 spec version.
Also, the return status of SetDriveParameters() is irrelevant with the 
execution result of this command.

2. SET MULTIPLE MODE command (ID 0xC6h):
My take is that this command is necessary if there is subsequent usage of 
command READ MULTIPLE, READ MULTIPLE EXT, WRITE MULTIPLE, or WRITE MULTIPLE EXT.
I do not find usage of the above 4 commands within edk2, so I think the 
successful execution of SET MULTIPLE MODE command is not mandatory for 
detecting hard disk device.

Based on the above findings, could you help to update the patch to:
  Status = SetDriveParameters (Instance, IdeChannel, IdeDevice, 
, NULL);

  if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_WARN, "Set Drive Parameters Fail, Status = %r\n", 
Status));
  }

Will doing so still please Coverity?

Best Regards,
Hao Wu

> -Original Message-
> From: Ranbir Singh 
> Sent: Friday, June 9, 2023 8:33 PM
> To: devel@edk2.groups.io; rsi...@ventanamicro.com
> Cc: Wu, Hao A ; Ni, Ray 
> Subject: [PATCH v2 2/2] MdeModulePkg/Bus/Ata/AtaAtapiPassThru: Fix
> UNUSED_VALUE Coverity issue
> 
> From: Ranbir Singh 
> 
> The return value stored in Status after call to SetDriveParameters
> is not made of any use thereafter and hence it remains as UNUSED.
> 
> Add error check as is done after calls to SetDeviceTransferMode.
> 
> Cc: Hao A Wu 
> Cc: Ray Ni 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4204
> Signed-off-by: Ranbir Singh 
> Signed-off-by: Ranbir Singh 
> ---
> 
> Notes:
> Add error check instead of Status storage removal
> 
>  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
> b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
> index 75403886e44a..d04b1d95a7f5 100644
> --- a/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
> +++ b/MdeModulePkg/Bus/Ata/AtaAtapiPassThru/IdeMode.c
> @@ -2549,13 +2549,18 @@ DetectAndConfigIdeDevice (
>  //
> 
>  if (DeviceType == EfiIdeHarddisk) {
> 
>//
> 
> -  // Init driver parameters
> 
> +  // Init drive parameters
> 
>//
> 
>DriveParameters.Sector = (UINT8)((ATA5_IDENTIFY_DATA
> *)())->sectors_per_track;
> 
>DriveParameters.Heads  = (UINT8)(((ATA5_IDENTIFY_DATA
> *)())->heads - 1);
> 
>DriveParameters.MultipleSector = (UINT8)((ATA5_IDENTIFY_DATA
> *)())->multi_sector_cmd_max_sct_cnt;
> 
> 
> 
>Status = SetDriveParameters (Instance, IdeChannel, IdeDevice,
> , NULL);
> 
> +
> 
> +  if (EFI_ERROR (Status)) {
> 
> +DEBUG ((DEBUG_ERROR, "Set Drive Parameters Fail, Status = %r\n",
> Status));
> 
> +continue;
> 
> +  }
> 
>  }
> 
> 
> 
>  //
> 
> --
> 2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106844): https://edk2.groups.io/g/devel/message/106844
Mute This Topic: https://groups.io/mt/99432080/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [edk2-platforms][PATCH v1 1/1] ManageabilityPkg: Ipmi Get/Set Boot Options

2023-07-11 Thread Chang, Abner via groups.io
[AMD Official Use Only - General]

Hi Isaac, thanks for the last minutes review.

Abner

> -Original Message-
> From: Oram, Isaac W 
> Sent: Wednesday, July 12, 2023 10:06 AM
> To: Chang, Abner ; Chesley, Brit
> ; devel@edk2.groups.io
> Cc: Attar, AbdulLateef (Abdul Lateef) ; Nickle
> Wang 
> Subject: RE: [edk2-platforms][PATCH v1 1/1] ManageabilityPkg: Ipmi Get/Set
> Boot Options
>
> [AMD Official Use Only - General]
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Reviewed-by: Isaac Oram 
>
> -Original Message-
> From: Chang, Abner 
> Sent: Monday, July 10, 2023 6:36 PM
> To: Chesley, Brit ; devel@edk2.groups.io
> Cc: Oram, Isaac W ; Attar, AbdulLateef (Abdul
> Lateef) ; Nickle Wang 
> Subject: RE: [edk2-platforms][PATCH v1 1/1] ManageabilityPkg: Ipmi Get/Set
> Boot Options
>
> [AMD Official Use Only - General]
>
> Thank you Brit for the contribution! Let's wait for 1-2 days to see if there 
> is any
> other comments for this change.
>
> Reviewed-by: Abner Chang 
>
> > -Original Message-
> > From: Chesley, Brit 
> > Sent: Tuesday, July 11, 2023 3:23 AM
> > To: devel@edk2.groups.io
> > Cc: Chang, Abner ; Isaac Oram
> > ; Attar, AbdulLateef (Abdul Lateef)
> > ; Nickle Wang 
> > Subject: [edk2-platforms][PATCH v1 1/1] ManageabilityPkg: Ipmi Get/Set
> > Boot Options
> >
> > From: Brit Chesley 
> >
> > BZ #: 4455.
> > Support parameter selectors for IPMI Get/Set boot options. The size of
> > the response data is now dependent on the parameter selector, rather
> > than being fixed.
> >
> > Cc: Abner Chang 
> > Cc: Isaac Oram 
> > Cc: Abdul Lateef Attar 
> > Cc: Nickle Wang 
> > Signed-off-by: Brit Chesley 
> > ---
> >  .../IpmiCommandLibNetFnChassis.c  | 124 ++
> >  1 file changed, 102 insertions(+), 22 deletions(-)
> >
> > diff --git
> >
> a/Features/ManageabilityPkg/Library/IpmiCommandLib/IpmiCommandLibNe
> > tFnChassis.c
> >
> b/Features/ManageabilityPkg/Library/IpmiCommandLib/IpmiCommandLibNe
> > tFnChassis.c
> > index 0c40ad20b98a..01682f55b36d 100644
> > ---
> >
> a/Features/ManageabilityPkg/Library/IpmiCommandLib/IpmiCommandLibNe
> > tFnChassis.c
> > +++
> >
> b/Features/ManageabilityPkg/Library/IpmiCommandLib/IpmiCommandLibNe
> > tFnChassis.c
> > @@ -152,17 +152,58 @@ IpmiSetSystemBootOptions (
> >)
> >  {
> >EFI_STATUS  Status;
> > -  UINT32  DataSize;
> > -
> > -  DataSize = sizeof (*BootOptionsResponse);
> > -  Status   = IpmiSubmitCommand (
> > -   IPMI_NETFN_CHASSIS,
> > -   IPMI_CHASSIS_SET_SYSTEM_BOOT_OPTIONS,
> > -   (VOID *)BootOptionsRequest,
> > -   sizeof (*BootOptionsRequest),
> > -   (VOID *)BootOptionsResponse,
> > -   
> > -   );
> > +  UINT32  RequestDataSize;
> > +  UINT32  ResponseDataSize;
> > +
> > +  ResponseDataSize = sizeof (*BootOptionsResponse);  RequestDataSize
> > + = sizeof (*BootOptionsRequest);
> > +
> > +  switch (BootOptionsRequest->ParameterValid.Bits.ParameterSelector) {
> > +case IPMI_BOOT_OPTIONS_PARAMETER_SELECTOR_SET_IN_PROGRESS:
> > +  RequestDataSize += sizeof
> > (IPMI_BOOT_OPTIONS_RESPONSE_PARAMETER_0);
> > +  break;
> > +
> > +case
> >
> IPMI_BOOT_OPTIONS_PARAMETER_SELECTOR_SERVICE_PARTITION_SELECT
> > OR:
> > +  RequestDataSize += sizeof
> > (IPMI_BOOT_OPTIONS_RESPONSE_PARAMETER_1);
> > +  break;
> > +
> > +case
> > IPMI_BOOT_OPTIONS_PARAMETER_SELECTOR_SERVICE_PARTITION_SCAN:
> > +  RequestDataSize += sizeof
> > (IPMI_BOOT_OPTIONS_RESPONSE_PARAMETER_2);
> > +  break;
> > +
> > +case IPMI_BOOT_OPTIONS_PARAMETER_SELECTOR_BMC_BOOT_FLAG:
> > +  RequestDataSize += sizeof
> > (IPMI_BOOT_OPTIONS_RESPONSE_PARAMETER_3);
> > +  break;
> > +
> > +case IPMI_BOOT_OPTIONS_PARAMETER_BOOT_INFO_ACK:
> > +  RequestDataSize += sizeof
> > (IPMI_BOOT_OPTIONS_RESPONSE_PARAMETER_4);
> > +  break;
> > +
> > +case IPMI_BOOT_OPTIONS_PARAMETER_BOOT_FLAGS:
> > +  RequestDataSize += sizeof
> > (IPMI_BOOT_OPTIONS_RESPONSE_PARAMETER_5);
> > +  break;
> > +
> > +case IPMI_BOOT_OPTIONS_PARAMETER_BOOT_INITIATOR_INFO:
> > +  RequestDataSize += sizeof
> > (IPMI_BOOT_OPTIONS_RESPONSE_PARAMETER_6);
> > +  break;
> > +
> > +case IPMI_BOOT_OPTIONS_PARAMETER_BOOT_INITIATOR_MAILBOX:
> > +  RequestDataSize += sizeof
> > (IPMI_BOOT_OPTIONS_RESPONSE_PARAMETER_7);
> > +  break;
> > +
> > +default:
> > +  return EFI_INVALID_PARAMETER;
> > +  break;
> > +  }
> > +
> > +  Status = IpmiSubmitCommand (
> > + IPMI_NETFN_CHASSIS,
> > + IPMI_CHASSIS_SET_SYSTEM_BOOT_OPTIONS,
> > + (VOID *)BootOptionsRequest,
> > + RequestDataSize,
> > + (VOID *)BootOptionsResponse,
> > + 
> > + );
> >return Status;
> >  }
> >
> > @@ -184,16 +225,55 @@ IpmiGetSystemBootOptions (
> >)

Re: [edk2-devel] [edk2-platforms][PATCH v1 1/1] ManageabilityPkg: Ipmi Get/Set Boot Options

2023-07-11 Thread Isaac Oram
Reviewed-by: Isaac Oram 

-Original Message-
From: Chang, Abner  
Sent: Monday, July 10, 2023 6:36 PM
To: Chesley, Brit ; devel@edk2.groups.io
Cc: Oram, Isaac W ; Attar, AbdulLateef (Abdul Lateef) 
; Nickle Wang 
Subject: RE: [edk2-platforms][PATCH v1 1/1] ManageabilityPkg: Ipmi Get/Set Boot 
Options

[AMD Official Use Only - General]

Thank you Brit for the contribution! Let's wait for 1-2 days to see if there is 
any other comments for this change.

Reviewed-by: Abner Chang 

> -Original Message-
> From: Chesley, Brit 
> Sent: Tuesday, July 11, 2023 3:23 AM
> To: devel@edk2.groups.io
> Cc: Chang, Abner ; Isaac Oram 
> ; Attar, AbdulLateef (Abdul Lateef) 
> ; Nickle Wang 
> Subject: [edk2-platforms][PATCH v1 1/1] ManageabilityPkg: Ipmi Get/Set 
> Boot Options
>
> From: Brit Chesley 
>
> BZ #: 4455.
> Support parameter selectors for IPMI Get/Set boot options. The size of 
> the response data is now dependent on the parameter selector, rather 
> than being fixed.
>
> Cc: Abner Chang 
> Cc: Isaac Oram 
> Cc: Abdul Lateef Attar 
> Cc: Nickle Wang 
> Signed-off-by: Brit Chesley 
> ---
>  .../IpmiCommandLibNetFnChassis.c  | 124 ++
>  1 file changed, 102 insertions(+), 22 deletions(-)
>
> diff --git
> a/Features/ManageabilityPkg/Library/IpmiCommandLib/IpmiCommandLibNe
> tFnChassis.c
> b/Features/ManageabilityPkg/Library/IpmiCommandLib/IpmiCommandLibNe
> tFnChassis.c
> index 0c40ad20b98a..01682f55b36d 100644
> ---
> a/Features/ManageabilityPkg/Library/IpmiCommandLib/IpmiCommandLibNe
> tFnChassis.c
> +++
> b/Features/ManageabilityPkg/Library/IpmiCommandLib/IpmiCommandLibNe
> tFnChassis.c
> @@ -152,17 +152,58 @@ IpmiSetSystemBootOptions (
>)
>  {
>EFI_STATUS  Status;
> -  UINT32  DataSize;
> -
> -  DataSize = sizeof (*BootOptionsResponse);
> -  Status   = IpmiSubmitCommand (
> -   IPMI_NETFN_CHASSIS,
> -   IPMI_CHASSIS_SET_SYSTEM_BOOT_OPTIONS,
> -   (VOID *)BootOptionsRequest,
> -   sizeof (*BootOptionsRequest),
> -   (VOID *)BootOptionsResponse,
> -   
> -   );
> +  UINT32  RequestDataSize;
> +  UINT32  ResponseDataSize;
> +
> +  ResponseDataSize = sizeof (*BootOptionsResponse);  RequestDataSize  
> + = sizeof (*BootOptionsRequest);
> +
> +  switch (BootOptionsRequest->ParameterValid.Bits.ParameterSelector) {
> +case IPMI_BOOT_OPTIONS_PARAMETER_SELECTOR_SET_IN_PROGRESS:
> +  RequestDataSize += sizeof
> (IPMI_BOOT_OPTIONS_RESPONSE_PARAMETER_0);
> +  break;
> +
> +case
> IPMI_BOOT_OPTIONS_PARAMETER_SELECTOR_SERVICE_PARTITION_SELECT
> OR:
> +  RequestDataSize += sizeof
> (IPMI_BOOT_OPTIONS_RESPONSE_PARAMETER_1);
> +  break;
> +
> +case
> IPMI_BOOT_OPTIONS_PARAMETER_SELECTOR_SERVICE_PARTITION_SCAN:
> +  RequestDataSize += sizeof
> (IPMI_BOOT_OPTIONS_RESPONSE_PARAMETER_2);
> +  break;
> +
> +case IPMI_BOOT_OPTIONS_PARAMETER_SELECTOR_BMC_BOOT_FLAG:
> +  RequestDataSize += sizeof
> (IPMI_BOOT_OPTIONS_RESPONSE_PARAMETER_3);
> +  break;
> +
> +case IPMI_BOOT_OPTIONS_PARAMETER_BOOT_INFO_ACK:
> +  RequestDataSize += sizeof
> (IPMI_BOOT_OPTIONS_RESPONSE_PARAMETER_4);
> +  break;
> +
> +case IPMI_BOOT_OPTIONS_PARAMETER_BOOT_FLAGS:
> +  RequestDataSize += sizeof
> (IPMI_BOOT_OPTIONS_RESPONSE_PARAMETER_5);
> +  break;
> +
> +case IPMI_BOOT_OPTIONS_PARAMETER_BOOT_INITIATOR_INFO:
> +  RequestDataSize += sizeof
> (IPMI_BOOT_OPTIONS_RESPONSE_PARAMETER_6);
> +  break;
> +
> +case IPMI_BOOT_OPTIONS_PARAMETER_BOOT_INITIATOR_MAILBOX:
> +  RequestDataSize += sizeof
> (IPMI_BOOT_OPTIONS_RESPONSE_PARAMETER_7);
> +  break;
> +
> +default:
> +  return EFI_INVALID_PARAMETER;
> +  break;
> +  }
> +
> +  Status = IpmiSubmitCommand (
> + IPMI_NETFN_CHASSIS,
> + IPMI_CHASSIS_SET_SYSTEM_BOOT_OPTIONS,
> + (VOID *)BootOptionsRequest,
> + RequestDataSize,
> + (VOID *)BootOptionsResponse,
> + 
> + );
>return Status;
>  }
>
> @@ -184,16 +225,55 @@ IpmiGetSystemBootOptions (
>)
>  {
>EFI_STATUS  Status;
> -  UINT32  DataSize;
> -
> -  DataSize = sizeof (*BootOptionsResponse);
> -  Status   = IpmiSubmitCommand (
> -   IPMI_NETFN_CHASSIS,
> -   IPMI_CHASSIS_GET_SYSTEM_BOOT_OPTIONS,
> -   (VOID *)BootOptionsRequest,
> -   sizeof (*BootOptionsRequest),
> -   (VOID *)BootOptionsResponse,
> -   
> -   );
> +  UINT32  ResponseDataSize;
> +
> +  ResponseDataSize = sizeof (*BootOptionsResponse);
> +
> +  switch (BootOptionsRequest->ParameterSelector.Bits.ParameterSelector) {
> +case IPMI_BOOT_OPTIONS_PARAMETER_SELECTOR_SET_IN_PROGRESS:
> +  ResponseDataSize += sizeof
> (IPMI_BOOT_OPTIONS_RESPONSE_PARAMETER_0);
> +  break;
> +
> +case
> IPMI_BOOT_OPTIONS_PARAMETER_SELECTOR_SERVICE_PARTITION_SELECT
> OR:
> + 

Re: [edk2-devel] [edk2-platforms][PATCH v1 1/1] ManageabilityPkg: Ipmi Get/Set Boot Options

2023-07-11 Thread Chang, Abner via groups.io
[AMD Official Use Only - General]

Hi Brit, I am going to push this change with updating the file header with AMD 
copyrights.

Thanks
Abner

> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Chang,
> Abner via groups.io
> Sent: Tuesday, July 11, 2023 9:36 AM
> To: Chesley, Brit ; devel@edk2.groups.io
> Cc: Isaac Oram ; Attar, AbdulLateef (Abdul Lateef)
> ; Nickle Wang 
> Subject: Re: [edk2-devel] [edk2-platforms][PATCH v1 1/1] ManageabilityPkg:
> Ipmi Get/Set Boot Options
>
> [AMD Official Use Only - General]
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> [AMD Official Use Only - General]
>
> Thank you Brit for the contribution! Let's wait for 1-2 days to see if there 
> is any
> other comments for this change.
>
> Reviewed-by: Abner Chang 
>
> > -Original Message-
> > From: Chesley, Brit 
> > Sent: Tuesday, July 11, 2023 3:23 AM
> > To: devel@edk2.groups.io
> > Cc: Chang, Abner ; Isaac Oram
> > ; Attar, AbdulLateef (Abdul Lateef)
> > ; Nickle Wang 
> > Subject: [edk2-platforms][PATCH v1 1/1] ManageabilityPkg: Ipmi Get/Set
> > Boot Options
> >
> > From: Brit Chesley 
> >
> > BZ #: 4455.
> > Support parameter selectors for IPMI Get/Set boot options. The size of
> > the response data is now dependent on the parameter selector, rather
> > than being fixed.
> >
> > Cc: Abner Chang 
> > Cc: Isaac Oram 
> > Cc: Abdul Lateef Attar 
> > Cc: Nickle Wang 
> > Signed-off-by: Brit Chesley 
> > ---
> >  .../IpmiCommandLibNetFnChassis.c  | 124 ++
> >  1 file changed, 102 insertions(+), 22 deletions(-)
> >
> > diff --git
> >
> a/Features/ManageabilityPkg/Library/IpmiCommandLib/IpmiCommandLibNe
> > tFnChassis.c
> >
> b/Features/ManageabilityPkg/Library/IpmiCommandLib/IpmiCommandLibNe
> > tFnChassis.c
> > index 0c40ad20b98a..01682f55b36d 100644
> > ---
> >
> a/Features/ManageabilityPkg/Library/IpmiCommandLib/IpmiCommandLibNe
> > tFnChassis.c
> > +++
> >
> b/Features/ManageabilityPkg/Library/IpmiCommandLib/IpmiCommandLibNe
> > tFnChassis.c
> > @@ -152,17 +152,58 @@ IpmiSetSystemBootOptions (
> >)
> >  {
> >EFI_STATUS  Status;
> > -  UINT32  DataSize;
> > -
> > -  DataSize = sizeof (*BootOptionsResponse);
> > -  Status   = IpmiSubmitCommand (
> > -   IPMI_NETFN_CHASSIS,
> > -   IPMI_CHASSIS_SET_SYSTEM_BOOT_OPTIONS,
> > -   (VOID *)BootOptionsRequest,
> > -   sizeof (*BootOptionsRequest),
> > -   (VOID *)BootOptionsResponse,
> > -   
> > -   );
> > +  UINT32  RequestDataSize;
> > +  UINT32  ResponseDataSize;
> > +
> > +  ResponseDataSize = sizeof (*BootOptionsResponse);
> > +  RequestDataSize  = sizeof (*BootOptionsRequest);
> > +
> > +  switch (BootOptionsRequest->ParameterValid.Bits.ParameterSelector) {
> > +case IPMI_BOOT_OPTIONS_PARAMETER_SELECTOR_SET_IN_PROGRESS:
> > +  RequestDataSize += sizeof
> > (IPMI_BOOT_OPTIONS_RESPONSE_PARAMETER_0);
> > +  break;
> > +
> > +case
> >
> IPMI_BOOT_OPTIONS_PARAMETER_SELECTOR_SERVICE_PARTITION_SELECT
> > OR:
> > +  RequestDataSize += sizeof
> > (IPMI_BOOT_OPTIONS_RESPONSE_PARAMETER_1);
> > +  break;
> > +
> > +case
> > IPMI_BOOT_OPTIONS_PARAMETER_SELECTOR_SERVICE_PARTITION_SCAN:
> > +  RequestDataSize += sizeof
> > (IPMI_BOOT_OPTIONS_RESPONSE_PARAMETER_2);
> > +  break;
> > +
> > +case IPMI_BOOT_OPTIONS_PARAMETER_SELECTOR_BMC_BOOT_FLAG:
> > +  RequestDataSize += sizeof
> > (IPMI_BOOT_OPTIONS_RESPONSE_PARAMETER_3);
> > +  break;
> > +
> > +case IPMI_BOOT_OPTIONS_PARAMETER_BOOT_INFO_ACK:
> > +  RequestDataSize += sizeof
> > (IPMI_BOOT_OPTIONS_RESPONSE_PARAMETER_4);
> > +  break;
> > +
> > +case IPMI_BOOT_OPTIONS_PARAMETER_BOOT_FLAGS:
> > +  RequestDataSize += sizeof
> > (IPMI_BOOT_OPTIONS_RESPONSE_PARAMETER_5);
> > +  break;
> > +
> > +case IPMI_BOOT_OPTIONS_PARAMETER_BOOT_INITIATOR_INFO:
> > +  RequestDataSize += sizeof
> > (IPMI_BOOT_OPTIONS_RESPONSE_PARAMETER_6);
> > +  break;
> > +
> > +case IPMI_BOOT_OPTIONS_PARAMETER_BOOT_INITIATOR_MAILBOX:
> > +  RequestDataSize += sizeof
> > (IPMI_BOOT_OPTIONS_RESPONSE_PARAMETER_7);
> > +  break;
> > +
> > +default:
> > +  return EFI_INVALID_PARAMETER;
> > +  break;
> > +  }
> > +
> > +  Status = IpmiSubmitCommand (
> > + IPMI_NETFN_CHASSIS,
> > + IPMI_CHASSIS_SET_SYSTEM_BOOT_OPTIONS,
> > + (VOID *)BootOptionsRequest,
> > + RequestDataSize,
> > + (VOID *)BootOptionsResponse,
> > + 
> > + );
> >return Status;
> >  }
> >
> > @@ -184,16 +225,55 @@ IpmiGetSystemBootOptions (
> >)
> >  {
> >EFI_STATUS  Status;
> > -  UINT32  DataSize;
> > -
> > -  DataSize = sizeof (*BootOptionsResponse);
> > -  Status   = IpmiSubmitCommand (
> > -   IPMI_NETFN_CHASSIS,
> > -   

Re: [edk2-devel][edk2-platforms][PATCH V4-1] IpmiFeaturePkg:Provided multiple IPMI interface support in PEI

2023-07-11 Thread Isaac Oram
I think the exception has something to do with gEfiPeiSmbus2PpiGuid.

Regards,
Isaac

-Original Message-
From: devel@edk2.groups.io  On Behalf Of Isaac Oram
Sent: Tuesday, July 11, 2023 6:45 PM
To: Arun K ; devel@edk2.groups.io
Cc: Desimone, Nathaniel L ; Ramkumar 
Krishnamoorthi ; Gao, Liming 
Subject: Re: [edk2-devel][edk2-platforms][PATCH V4-1] IpmiFeaturePkg:Provided 
multiple IPMI interface support in PEI

Arun,

My apologies that my feedback wasn't clear enough.  I would like to avoid the 
preprocessor optimization step of using #if in favor of using C code and using 
link time optimization to remove unused code.  I sent you a patch with examples 
of using the PCD in normal C logic.  I don't think it is 100%, but it should be 
clear on the concept.

Regarding the logic you implemented for 
  gIpmiFeaturePkgTokenSpaceGuid.PcdKcsInterfaceSupport
  gIpmiFeaturePkgTokenSpaceGuid.PcdBtInterfaceSupport
  gIpmiFeaturePkgTokenSpaceGuid.PcdSsifInterfaceSupport
  gIpmiFeaturePkgTokenSpaceGuid.PcdIpmbInterfaceSupport

I think that there might be too much dynamic conditional logic.  Like case 
statements that have #if around cases that should only happen if the interface 
is present and enabled.  I think it is better to simplify the code so that the 
presence and use is enough.  I sent you a patch that converts all existing 
preprocessor (#if) logic to C logic.  But it is better if you can remove some 
of that conditional logic.  The more you can simplify the number of build/test 
combinations, the better.
What I mean specifically is if:
#if (FixedPcdGet8 (PcdKcsInterfaceSupport) == 1)
  if ((InterfaceType == SysInterfaceKcs) && 
(IpmiInstance->IpmiTransport2.Interface.KcsInterfaceState == 
IpmiInterfaceInitialized))...

Could be something like:
  if ((InterfaceType == SysInterfaceKcs) && 
(IpmiInstance->IpmiTransport2.Interface.KcsInterfaceState == 
IpmiInterfaceInitialized))...
Then that would be a little simpler and it doesn't initially look like we lose 
anything particularly valuable.  It doesn't look like enough code to be 
significant in a server use case.


It doesn't seem like everything is working properly with 
PcdSsifInterfaceSupport enabled.  In testing on WilsonCityRvp, I get an 
exception if it is enabled.  It might be a good idea to see if there is some 
more error checking needed there.  It might make sense that it isn't supported, 
but it should be a more graceful failure.

Please run python \edk2\BaseTools\Scripts\PatchCheck.py -3 on the changes and 
correct the issues.

Otherwise, the code looks good.

Regards,
Isaac

-Original Message-
From: Arun K  
Sent: Monday, July 3, 2023 7:43 AM
To: devel@edk2.groups.io; Arun K 
Cc: Oram, Isaac W ; Desimone, Nathaniel L 
; Ramkumar Krishnamoorthi ; 
Gao, Liming 
Subject: [edk2-devel][edk2-platforms][PATCH V4-1] IpmiFeaturePkg:Provided 
multiple IPMI interface support in PEI

Created IpmiTransport2 PPI/Protocol to support multiple IPMI BMC Interface 
support such as KCS/BT/SSIF/IPMB with 2 API's
IpmiSubmitCommand2 & IpmiSubmitCommand2Ex.
IpmiSubmitCommand2 - This API use the default interface
(PcdDefaultSystemInterface) to send IPMI command.
IpmiSubmitCommand2Ex - This API use the specific interface type to send IPMI 
command which is passed as an argument.

Cc: Isaac Oram 
Cc: Nate DeSimone 
Cc: Liming Gao 

Signed-off-by: Arun K 
---
 .../GenericIpmi/Pei/PeiGenericIpmi.c  | 295 ++
 .../GenericIpmi/Pei/PeiGenericIpmi.h  |   3 +
 .../GenericIpmi/Pei/PeiGenericIpmi.inf|  16 +
 .../GenericIpmi/Pei/PeiIpmiBmc.c  | 117 +++---
 .../GenericIpmi/Pei/PeiIpmiBmc.h  |  11 +-
 .../GenericIpmi/Pei/PeiIpmiBmcDef.h   |  86 +++--
 .../GenericIpmi/Pei/PeiIpmiHooks.c| 363 ++
 .../GenericIpmi/Pei/PeiIpmiHooks.h| 218 +++
 8 files changed, 949 insertions(+), 160 deletions(-)  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiIpmiHooks.c
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiIpmiHooks.h

diff --git 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiGenericIpmi.c
 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiGenericIpmi.c
index e8b99b6900..04ebfb6f23 100644
--- 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiGenericIpmi.c
+++ b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/
+++ PeiGenericIpmi.c
@@ -3,6 +3,7 @@


   @copyright

   Copyright 2017 - 2021 Intel Corporation. 

+  Copyright (c) 1985 - 2023, American Megatrends International LLC. 
+ 

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

 **/



@@ -10,10 +11,143 @@
 #include "PeiGenericIpmi.h"

 #include 

 #include 

+#include 



 ///

-// Function Implementations

-//

+

+/**

+Initialize the API and parameters for IPMI 

Re: [edk2-devel][edk2-platforms][PATCH V4-1] IpmiFeaturePkg:Provided multiple IPMI interface support in PEI

2023-07-11 Thread Isaac Oram
Arun,

My apologies that my feedback wasn't clear enough.  I would like to avoid the 
preprocessor optimization step of using #if in favor of using C code and using 
link time optimization to remove unused code.  I sent you a patch with examples 
of using the PCD in normal C logic.  I don't think it is 100%, but it should be 
clear on the concept.

Regarding the logic you implemented for 
  gIpmiFeaturePkgTokenSpaceGuid.PcdKcsInterfaceSupport
  gIpmiFeaturePkgTokenSpaceGuid.PcdBtInterfaceSupport
  gIpmiFeaturePkgTokenSpaceGuid.PcdSsifInterfaceSupport
  gIpmiFeaturePkgTokenSpaceGuid.PcdIpmbInterfaceSupport

I think that there might be too much dynamic conditional logic.  Like case 
statements that have #if around cases that should only happen if the interface 
is present and enabled.  I think it is better to simplify the code so that the 
presence and use is enough.  I sent you a patch that converts all existing 
preprocessor (#if) logic to C logic.  But it is better if you can remove some 
of that conditional logic.  The more you can simplify the number of build/test 
combinations, the better.
What I mean specifically is if:
#if (FixedPcdGet8 (PcdKcsInterfaceSupport) == 1)
  if ((InterfaceType == SysInterfaceKcs) && 
(IpmiInstance->IpmiTransport2.Interface.KcsInterfaceState == 
IpmiInterfaceInitialized))...

Could be something like:
  if ((InterfaceType == SysInterfaceKcs) && 
(IpmiInstance->IpmiTransport2.Interface.KcsInterfaceState == 
IpmiInterfaceInitialized))...
Then that would be a little simpler and it doesn't initially look like we lose 
anything particularly valuable.  It doesn't look like enough code to be 
significant in a server use case.


It doesn't seem like everything is working properly with 
PcdSsifInterfaceSupport enabled.  In testing on WilsonCityRvp, I get an 
exception if it is enabled.  It might be a good idea to see if there is some 
more error checking needed there.  It might make sense that it isn't supported, 
but it should be a more graceful failure.

Please run python \edk2\BaseTools\Scripts\PatchCheck.py -3 on the changes and 
correct the issues.

Otherwise, the code looks good.

Regards,
Isaac

-Original Message-
From: Arun K  
Sent: Monday, July 3, 2023 7:43 AM
To: devel@edk2.groups.io; Arun K 
Cc: Oram, Isaac W ; Desimone, Nathaniel L 
; Ramkumar Krishnamoorthi ; 
Gao, Liming 
Subject: [edk2-devel][edk2-platforms][PATCH V4-1] IpmiFeaturePkg:Provided 
multiple IPMI interface support in PEI

Created IpmiTransport2 PPI/Protocol to support multiple IPMI BMC Interface 
support such as KCS/BT/SSIF/IPMB with 2 API's
IpmiSubmitCommand2 & IpmiSubmitCommand2Ex.
IpmiSubmitCommand2 - This API use the default interface
(PcdDefaultSystemInterface) to send IPMI command.
IpmiSubmitCommand2Ex - This API use the specific interface type to send IPMI 
command which is passed as an argument.

Cc: Isaac Oram 
Cc: Nate DeSimone 
Cc: Liming Gao 

Signed-off-by: Arun K 
---
 .../GenericIpmi/Pei/PeiGenericIpmi.c  | 295 ++
 .../GenericIpmi/Pei/PeiGenericIpmi.h  |   3 +
 .../GenericIpmi/Pei/PeiGenericIpmi.inf|  16 +
 .../GenericIpmi/Pei/PeiIpmiBmc.c  | 117 +++---
 .../GenericIpmi/Pei/PeiIpmiBmc.h  |  11 +-
 .../GenericIpmi/Pei/PeiIpmiBmcDef.h   |  86 +++--
 .../GenericIpmi/Pei/PeiIpmiHooks.c| 363 ++
 .../GenericIpmi/Pei/PeiIpmiHooks.h| 218 +++
 8 files changed, 949 insertions(+), 160 deletions(-)  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiIpmiHooks.c
 create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiIpmiHooks.h

diff --git 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiGenericIpmi.c
 
b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiGenericIpmi.c
index e8b99b6900..04ebfb6f23 100644
--- 
a/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiGenericIpmi.c
+++ b/Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/
+++ PeiGenericIpmi.c
@@ -3,6 +3,7 @@


   @copyright

   Copyright 2017 - 2021 Intel Corporation. 

+  Copyright (c) 1985 - 2023, American Megatrends International LLC. 
+ 

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

 **/



@@ -10,10 +11,143 @@
 #include "PeiGenericIpmi.h"

 #include 

 #include 

+#include 



 ///

-// Function Implementations

-//

+

+/**

+Initialize the API and parameters for IPMI Transport2 Instance

+

+@param[in] IpmiInstance Pointer to IPMI Instance

+

+@return VOID

+

+**/

+VOID

+InitIpmiTransport2 (

+  IN  PEI_IPMI_BMC_INSTANCE_DATA *IpmiInstance

+  )

+{

+  IpmiInstance->IpmiTransport2Ppi.InterfaceType   = FixedPcdGet8 
(PcdDefaultSystemInterface);

+  IpmiInstance->IpmiTransport2Ppi.IpmiTransport2BmcStatus = 
+ BmcStatusOk;

+  

Re: [edk2-devel] [PATCH 14/14] MdeModulePkg: Delete Memory Protection PCDs

2023-07-11 Thread Taylor Beebe

Looks like the title of this patch in the series was mixed up
with the title of the following patch. I'll wait for feedback
before sending out a v2, but the title of this patch
should be:

[PATCH 13/14] ArmVirtPkg: Delete Memory Protection PCDs

On 7/11/2023 4:52 PM, Taylor Beebe via groups.io wrote:

From: Taylor Beebe 

Now that references in the rest of the codebase have been updated
to reference the memory protection HOB, delete the memory protection PCDs.

Signed-off-by: Taylor Beebe 
Cc: Ard Biesheuvel 
Cc: Leif Lindholm 
Cc: Sami Mujawar 
Cc: Gerd Hoffmann 
---
  ArmVirtPkg/ArmVirt.dsc.inc| 15 ---
  ArmVirtPkg/ArmVirtCloudHv.dsc |  5 -
  ArmVirtPkg/ArmVirtQemu.dsc|  5 -
  3 files changed, 25 deletions(-)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 3174b19e51..e1eb189077 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -363,21 +363,6 @@
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderCode|20
gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderData|0
  
-  #

-  # Enable strict image permissions for all images. (This applies
-  # only to images that were built with >= 4 KB section alignment.)
-  #
-  gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x3
-
-  #
-  # Enable NX memory protection for all non-code regions, including OEM and OS
-  # reserved ones, with the exception of LoaderData regions, of which OS 
loaders
-  # (i.e., GRUB) may assume that its contents are executable.
-  #
-  
gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC0007FD5
-
-  gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|TRUE
-
  [Components.common]
#
# Ramdisk support
diff --git a/ArmVirtPkg/ArmVirtCloudHv.dsc b/ArmVirtPkg/ArmVirtCloudHv.dsc
index c975e139a2..c4c3e0da44 100644
--- a/ArmVirtPkg/ArmVirtCloudHv.dsc
+++ b/ArmVirtPkg/ArmVirtCloudHv.dsc
@@ -140,11 +140,6 @@
#
gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
  
-  #

-  # Enable the non-executable DXE stack. (This gets set up by DxeIpl)
-  #
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
-
  !if $(SECURE_BOOT_ENABLE) == TRUE
# override the default values from SecurityPkg to ensure images from all 
sources are verified in secure boot
gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x04
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 1e0225951a..214e08b789 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -212,11 +212,6 @@
#
gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
  
-  #

-  # Enable the non-executable DXE stack. (This gets set up by DxeIpl)
-  #
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
-
  !if $(SECURE_BOOT_ENABLE) == TRUE
# override the default values from SecurityPkg to ensure images from all 
sources are verified in secure boot
gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x04


--
Taylor Beebe
Software Engineer @ Microsoft


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106838): https://edk2.groups.io/g/devel/message/106838
Mute This Topic: https://groups.io/mt/100090808/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH 14/14] MdeModulePkg: Delete Memory Protection PCDs

2023-07-11 Thread Taylor Beebe
From: Taylor Beebe 

Delete the memory protection PCDs

Signed-off-by: Taylor Beebe 
Cc: Jian J Wang 
Cc: Liming Gao 
Cc: Dandan Bi 
---
 MdeModulePkg/MdeModulePkg.dec | 169 --
 MdeModulePkg/MdeModulePkg.uni | 153 --
 2 files changed, 322 deletions(-)

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 2541b2b044..9456e5cdfb 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1006,119 +1006,12 @@
   # @ValidList  0x8006 | 0x03058002
   
gEfiMdeModulePkgTokenSpaceGuid.PcdErrorCodeSetVariable|0x03058002|UINT32|0x30001040
 
-  ## Mask to control the NULL address detection in code for different phases.
-  #  If enabled, accessing NULL address in UEFI or SMM code can be 
caught.
-  #BIT0- Enable NULL pointer detection for UEFI.
-  #BIT1- Enable NULL pointer detection for SMM.
-  #BIT2..5 - Reserved for future uses.
-  #BIT6- Enable non-stop mode.
-  #BIT7- Disable NULL pointer detection just after EndOfDxe. 
-  #  This is a workaround for those unsolvable NULL access issues 
in
-  #  OptionROM, boot loader, etc. It can also help to avoid 
unnecessary
-  #  exception caused by legacy memory (0-4095) access after 
EndOfDxe,
-  #  such as Windows 7 boot on Qemu.
-  # @Prompt Enable NULL address detection.
-  
gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask|0x0|UINT8|0x30001050
-
   ## Init Value in Temp Stack to be shared between SEC and PEI_CORE
   # SEC fills the full temp stack with this values. When switch stack, PeiCore 
can check
   # this value in the temp stack to know how many stack has been used.
   # @Prompt Init Value in Temp Stack
   
gEfiMdeModulePkgTokenSpaceGuid.PcdInitValueInTempStack|0x5AA55AA5|UINT32|0x30001051
 
-  ## Indicates which type allocation need guard page.
-  #
-  # If a bit is set, a head guard page and a tail guard page will be added just
-  # before and after corresponding type of pages allocated if there's enough
-  # free pages for all of them. The page allocation for the type related to
-  # cleared bits keeps the same as ususal.
-  #
-  # This PCD is only valid if BIT0 and/or BIT2 are set in 
PcdHeapGuardPropertyMask.
-  #
-  # Below is bit mask for this PCD: (Order is same as UEFI spec)
-  #  EfiReservedMemoryType 0x0001
-  #  EfiLoaderCode 0x0002
-  #  EfiLoaderData 0x0004
-  #  EfiBootServicesCode   0x0008
-  #  EfiBootServicesData   0x0010
-  #  EfiRuntimeServicesCode0x0020
-  #  EfiRuntimeServicesData0x0040
-  #  EfiConventionalMemory 0x0080
-  #  EfiUnusableMemory 0x0100
-  #  EfiACPIReclaimMemory  0x0200
-  #  EfiACPIMemoryNVS  0x0400
-  #  EfiMemoryMappedIO 0x0800
-  #  EfiMemoryMappedIOPortSpace0x1000
-  #  EfiPalCode0x2000
-  #  EfiPersistentMemory   0x4000
-  #  OEM Reserved  0x4000
-  #  OS Reserved   0x8000
-  # e.g. LoaderCode+LoaderData+BootServicesCode+BootServicesData are needed, 
0x1E should be used.
-  # @Prompt The memory type mask for Page Guard.
-  gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPageType|0x0|UINT64|0x30001052
-
-  ## Indicates which type allocation need guard page.
-  #
-  # If a bit is set, a head guard page and a tail guard page will be added just
-  # before and after corresponding type of pages which the allocated pool 
occupies,
-  # if there's enough free memory for all of them. The pool allocation for the
-  # type related to cleared bits keeps the same as ususal.
-  #
-  # This PCD is only valid if BIT1 and/or BIT3 are set in 
PcdHeapGuardPropertyMask.
-  #
-  # Below is bit mask for this PCD: (Order is same as UEFI spec)
-  #  EfiReservedMemoryType 0x0001
-  #  EfiLoaderCode 0x0002
-  #  EfiLoaderData 0x0004
-  #  EfiBootServicesCode   0x0008
-  #  EfiBootServicesData   0x0010
-  #  EfiRuntimeServicesCode0x0020
-  #  EfiRuntimeServicesData0x0040
-  #  EfiConventionalMemory 0x0080
-  #  EfiUnusableMemory 0x0100
-  #  EfiACPIReclaimMemory  0x0200
-  #  EfiACPIMemoryNVS  0x0400
-  #  EfiMemoryMappedIO 0x0800
-  #  EfiMemoryMappedIOPortSpace0x1000
-  #  EfiPalCode0x2000
-  #  EfiPersistentMemory 

[edk2-devel] [PATCH 11/14] UefiPayloadPkg: Update to use memory protection HOB

2023-07-11 Thread Taylor Beebe
From: Taylor Beebe 

Replace references to the memory protection PCDs with references
to the memory protection HOB.

The X64/DxeLoadFunc.c will check the PcdDxeIplBuildPageTables
PCD to determine if page tables should be built wheras before
they would check both the PcdDxeIplBuildPageTables PCD and the
memory protection settings. If PcdDxeIplBuildPageTables is
FALSE but memory protections are active, ASSERT on X64.

The Ia32/DxeLoadFunc.c was hard-coded to always build the page
tables, and that is unchanged in this patch. However,
this does remove the unused Create4GPageTablesIa32Pae() function
in Ia32/DxeLoadFunc.c.

Signed-off-by: Taylor Beebe 
Cc: Guo Dong 
Cc: Sean Rhodes 
Cc: James Lu 
Cc: Gua Guo 
---
 .../UefiPayloadEntry/Ia32/DxeLoadFunc.c   | 149 +-
 UefiPayloadPkg/UefiPayloadEntry/LoadDxeCore.c |  26 +++
 .../UefiPayloadEntry/UefiPayloadEntry.h   |  15 ++
 .../UefiPayloadEntry/UefiPayloadEntry.inf |   9 +-
 .../UniversalPayloadEntry.inf |   9 +-
 .../UefiPayloadEntry/X64/DxeLoadFunc.c|  25 ++-
 .../UefiPayloadEntry/X64/VirtualMemory.c  |  78 -
 .../UefiPayloadEntry/X64/VirtualMemory.h  |  23 +--
 UefiPayloadPkg/UefiPayloadPkg.dsc |   1 +
 9 files changed, 98 insertions(+), 237 deletions(-)

diff --git a/UefiPayloadPkg/UefiPayloadEntry/Ia32/DxeLoadFunc.c 
b/UefiPayloadPkg/UefiPayloadEntry/Ia32/DxeLoadFunc.c
index 61a9f01ec9..fa1fa53aea 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/Ia32/DxeLoadFunc.c
+++ b/UefiPayloadPkg/UefiPayloadEntry/Ia32/DxeLoadFunc.c
@@ -78,107 +78,6 @@ GLOBAL_REMOVE_IF_UNREFERENCED  IA32_DESCRIPTOR  
gLidtDescriptor = {
   0
 };
 
-/**
-  Allocates and fills in the Page Directory and Page Table Entries to
-  establish a 4G page table.
-
-  @param[in] StackBase  Stack base address.
-  @param[in] StackSize  Stack size.
-
-  @return The address of page table.
-
-**/
-UINTN
-Create4GPageTablesIa32Pae (
-  IN EFI_PHYSICAL_ADDRESS  StackBase,
-  IN UINTN StackSize
-  )
-{
-  UINT8   PhysicalAddressBits;
-  EFI_PHYSICAL_ADDRESSPhysicalAddress;
-  UINTN   IndexOfPdpEntries;
-  UINTN   IndexOfPageDirectoryEntries;
-  UINT32  NumberOfPdpEntriesNeeded;
-  PAGE_MAP_AND_DIRECTORY_POINTER  *PageMap;
-  PAGE_MAP_AND_DIRECTORY_POINTER  *PageDirectoryPointerEntry;
-  PAGE_TABLE_ENTRY*PageDirectoryEntry;
-  UINTN   TotalPagesNum;
-  UINTN   PageAddress;
-  UINT64  AddressEncMask;
-
-  //
-  // Make sure AddressEncMask is contained to smallest supported address field
-  //
-  AddressEncMask = PcdGet64 (PcdPteMemoryEncryptionAddressOrMask) & 
PAGING_1G_ADDRESS_MASK_64;
-
-  PhysicalAddressBits = 32;
-
-  //
-  // Calculate the table entries needed.
-  //
-  NumberOfPdpEntriesNeeded = (UINT32)LShiftU64 (1, (PhysicalAddressBits - 30));
-
-  TotalPagesNum = NumberOfPdpEntriesNeeded + 1;
-  PageAddress   = (UINTN)AllocatePageTableMemory (TotalPagesNum);
-  ASSERT (PageAddress != 0);
-
-  PageMap  = (VOID *)PageAddress;
-  PageAddress += SIZE_4KB;
-
-  PageDirectoryPointerEntry = PageMap;
-  PhysicalAddress   = 0;
-
-  for (IndexOfPdpEntries = 0; IndexOfPdpEntries < NumberOfPdpEntriesNeeded; 
IndexOfPdpEntries++, PageDirectoryPointerEntry++) {
-//
-// Each Directory Pointer entries points to a page of Page Directory 
entires.
-// So allocate space for them and fill them in in the 
IndexOfPageDirectoryEntries loop.
-//
-PageDirectoryEntry = (VOID *)PageAddress;
-PageAddress   += SIZE_4KB;
-
-//
-// Fill in a Page Directory Pointer Entries
-//
-PageDirectoryPointerEntry->Uint64   = 
(UINT64)(UINTN)PageDirectoryEntry | AddressEncMask;
-PageDirectoryPointerEntry->Bits.Present = 1;
-
-for (IndexOfPageDirectoryEntries = 0; IndexOfPageDirectoryEntries < 512; 
IndexOfPageDirectoryEntries++, PageDirectoryEntry++, PhysicalAddress += 
SIZE_2MB) {
-  if (  (IsNullDetectionEnabled () && (PhysicalAddress == 0))
- || (  (PhysicalAddress < StackBase + StackSize)
-&& ((PhysicalAddress + SIZE_2MB) > StackBase)))
-  {
-//
-// Need to split this 2M page that covers stack range.
-//
-Split2MPageTo4K (PhysicalAddress, (UINT64 *)PageDirectoryEntry, 
StackBase, StackSize, 0, 0);
-  } else {
-//
-// Fill in the Page Directory entries
-//
-PageDirectoryEntry->Uint64 = (UINT64)PhysicalAddress | 
AddressEncMask;
-PageDirectoryEntry->Bits.ReadWrite = 1;
-PageDirectoryEntry->Bits.Present   = 1;
-PageDirectoryEntry->Bits.MustBe1   = 1;
-  }
-}
-  }
-
-  for ( ; IndexOfPdpEntries < 512; IndexOfPdpEntries++, 
PageDirectoryPointerEntry++) {
-ZeroMem (
-  PageDirectoryPointerEntry,
-  sizeof (PAGE_MAP_AND_DIRECTORY_POINTER)
-  );
- 

[edk2-devel] [PATCH 12/14] OvmfPkg: Delete Memory Protection PCDs

2023-07-11 Thread Taylor Beebe
From: Taylor Beebe 

Now that references in the rest of the codebase have been updated
to reference the memory protection HOB, delete the memory protection PCDs.

Signed-off-by: Taylor Beebe 
Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc: Gerd Hoffmann 
---
 OvmfPkg/AmdSev/AmdSevX64.dsc | 3 ---
 OvmfPkg/Bhyve/BhyveX64.dsc   | 3 ---
 OvmfPkg/CloudHv/CloudHvX64.dsc   | 3 ---
 OvmfPkg/IntelTdx/IntelTdxX64.dsc | 3 ---
 OvmfPkg/Microvm/MicrovmX64.dsc   | 3 ---
 OvmfPkg/OvmfPkgIa32.dsc  | 3 ---
 OvmfPkg/OvmfPkgIa32X64.dsc   | 3 ---
 OvmfPkg/OvmfPkgX64.dsc   | 3 ---
 OvmfPkg/OvmfXen.dsc  | 3 ---
 9 files changed, 27 deletions(-)

diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index ce028fcb5c..abaaf59df8 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -508,9 +508,6 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE
 
-  # Noexec settings for DXE.
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE
-
   # UefiCpuPkg PCDs related to initial AP bringup and general AP management.
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64
   gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber|0
diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
index 248b6020ed..1ea9ac3bc4 100644
--- a/OvmfPkg/Bhyve/BhyveX64.dsc
+++ b/OvmfPkg/Bhyve/BhyveX64.dsc
@@ -552,9 +552,6 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE
 
-  # Noexec settings for DXE.
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE
-
   # UefiCpuPkg PCDs related to initial AP bringup and general AP management.
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64
   gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds|5
diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc b/OvmfPkg/CloudHv/CloudHvX64.dsc
index 8828e298ca..b852bb5ad0 100644
--- a/OvmfPkg/CloudHv/CloudHvX64.dsc
+++ b/OvmfPkg/CloudHv/CloudHvX64.dsc
@@ -601,9 +601,6 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE
 
-  # Noexec settings for DXE.
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE
-
   # UefiCpuPkg PCDs related to initial AP bringup and general AP management.
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64
   gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber|0
diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
index 27b9d4bf26..f6cc473a06 100644
--- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
+++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
@@ -503,9 +503,6 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE
 
-  # Noexec settings for DXE.
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
-
   # UefiCpuPkg PCDs related to initial AP bringup and general AP management.
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64
   gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber|0
diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc
index 611d64a6a0..d4a612a7e5 100644
--- a/OvmfPkg/Microvm/MicrovmX64.dsc
+++ b/OvmfPkg/Microvm/MicrovmX64.dsc
@@ -615,9 +615,6 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE
 
-  # Noexec settings for DXE.
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE
-
   # UefiCpuPkg PCDs related to initial AP bringup and general AP management.
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64
   gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber|0
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 810d69651c..7dec146841 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -628,9 +628,6 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE
 
-  # Noexec settings for DXE.
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE
-
   # UefiCpuPkg PCDs related to initial AP bringup and general AP management.
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64
   gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber|0
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 13e141a352..b9a35d43b9 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -646,9 +646,6 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE
 
-  # Noexec settings for DXE.
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE
-
   # UefiCpuPkg PCDs related to initial AP bringup and general AP management.
   gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber|64
   gUefiCpuPkgTokenSpaceGuid.PcdCpuBootLogicalProcessorNumber|0
diff --git a/OvmfPkg/OvmfPkgX64.dsc 

[edk2-devel] [PATCH 14/14] MdeModulePkg: Delete Memory Protection PCDs

2023-07-11 Thread Taylor Beebe
From: Taylor Beebe 

Now that references in the rest of the codebase have been updated
to reference the memory protection HOB, delete the memory protection PCDs.

Signed-off-by: Taylor Beebe 
Cc: Ard Biesheuvel 
Cc: Leif Lindholm 
Cc: Sami Mujawar 
Cc: Gerd Hoffmann 
---
 ArmVirtPkg/ArmVirt.dsc.inc| 15 ---
 ArmVirtPkg/ArmVirtCloudHv.dsc |  5 -
 ArmVirtPkg/ArmVirtQemu.dsc|  5 -
 3 files changed, 25 deletions(-)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 3174b19e51..e1eb189077 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -363,21 +363,6 @@
   gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderCode|20
   gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiLoaderData|0
 
-  #
-  # Enable strict image permissions for all images. (This applies
-  # only to images that were built with >= 4 KB section alignment.)
-  #
-  gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x3
-
-  #
-  # Enable NX memory protection for all non-code regions, including OEM and OS
-  # reserved ones, with the exception of LoaderData regions, of which OS 
loaders
-  # (i.e., GRUB) may assume that its contents are executable.
-  #
-  
gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC0007FD5
-
-  gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|TRUE
-
 [Components.common]
   #
   # Ramdisk support
diff --git a/ArmVirtPkg/ArmVirtCloudHv.dsc b/ArmVirtPkg/ArmVirtCloudHv.dsc
index c975e139a2..c4c3e0da44 100644
--- a/ArmVirtPkg/ArmVirtCloudHv.dsc
+++ b/ArmVirtPkg/ArmVirtCloudHv.dsc
@@ -140,11 +140,6 @@
   #
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
 
-  #
-  # Enable the non-executable DXE stack. (This gets set up by DxeIpl)
-  #
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
-
 !if $(SECURE_BOOT_ENABLE) == TRUE
   # override the default values from SecurityPkg to ensure images from all 
sources are verified in secure boot
   gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x04
diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 1e0225951a..214e08b789 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -212,11 +212,6 @@
   #
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuIoSize|16
 
-  #
-  # Enable the non-executable DXE stack. (This gets set up by DxeIpl)
-  #
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
-
 !if $(SECURE_BOOT_ENABLE) == TRUE
   # override the default values from SecurityPkg to ensure images from all 
sources are verified in secure boot
   gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x04
-- 
2.41.0.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106836): https://edk2.groups.io/g/devel/message/106836
Mute This Topic: https://groups.io/mt/100090647/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH 10/14] UefiCpuPkg: Update to use memory protection HOB

2023-07-11 Thread Taylor Beebe
From: Taylor Beebe 

Replace references to the memory protection PCDs with references
to the memory protection HOB.

Stack guard will always be initialized after memory discovery
in PEI, but the memory protection HOB will be checked when
applying stack guard in DxeIpl when the page tables are
rebuilt.

Signed-off-by: Taylor Beebe 
Cc: Eric Dong 
Cc: Ray Ni 
Cc: Rahul Kumar  
Cc: Gerd Hoffmann 
---
 UefiCpuPkg/CpuDxe/CpuDxe.c|  2 +-
 UefiCpuPkg/CpuDxe/CpuDxe.h| 11 +---
 UefiCpuPkg/CpuDxe/CpuDxe.inf  |  4 +--
 UefiCpuPkg/CpuDxe/CpuMp.c |  2 +-
 UefiCpuPkg/CpuDxeRiscV64/CpuDxeRiscV64.inf|  3 ---
 UefiCpuPkg/CpuMpPei/CpuMpPei.c|  8 +++---
 UefiCpuPkg/CpuMpPei/CpuMpPei.h|  3 ++-
 UefiCpuPkg/CpuMpPei/CpuMpPei.inf  |  1 -
 UefiCpuPkg/CpuMpPei/CpuPaging.c   | 14 +-
 .../DxeCpuExceptionHandlerLib.inf |  1 -
 .../PeiCpuExceptionHandlerLib.inf |  1 -
 .../SecPeiCpuExceptionHandlerLib.inf  |  1 -
 .../SmmCpuExceptionHandlerLib.inf |  1 -
 .../UnitTest/CpuExceptionHandlerTest.h|  3 ++-
 .../UnitTest/CpuExceptionHandlerTestCommon.c  | 27 +++
 .../DxeCpuExceptionHandlerLibUnitTest.inf |  2 +-
 .../PeiCpuExceptionHandlerLibUnitTest.inf |  4 ++-
 UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf |  3 ++-
 UefiCpuPkg/Library/MpInitLib/DxeMpLib.c   |  3 ++-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c  |  2 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf  |  3 +--
 .../PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c   | 13 -
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c|  2 +-
 .../PiSmmCpuDxeSmm/SmmProfileInternal.h   | 10 ---
 UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c   |  2 +-
 UefiCpuPkg/UefiCpuPkg.dec |  7 +++--
 UefiCpuPkg/UefiCpuPkg.dsc |  2 ++
 UefiCpuPkg/UefiCpuPkg.uni | 10 +++
 28 files changed, 80 insertions(+), 65 deletions(-)

diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.c b/UefiCpuPkg/CpuDxe/CpuDxe.c
index 804ef5d1fe..b12c43f4c1 100644
--- a/UefiCpuPkg/CpuDxe/CpuDxe.c
+++ b/UefiCpuPkg/CpuDxe/CpuDxe.c
@@ -399,7 +399,7 @@ CpuSetMemoryAttributes (
   // During memory attributes updating, new pages may be allocated to setup
   // smaller granularity of page table. Page allocation action might then cause
   // another calling of CpuSetMemoryAttributes() recursively, due to memory
-  // protection policy configured (such as PcdDxeNxMemoryProtectionPolicy).
+  // protection policy configured (such as the DXE NX Protection Policy).
   // Since this driver will always protect memory used as page table by itself,
   // there's no need to apply protection policy requested from memory service.
   // So it's safe to just return EFI_SUCCESS if this time of calling is caused
diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.h b/UefiCpuPkg/CpuDxe/CpuDxe.h
index 0e7d88dd35..10eabd9b66 100644
--- a/UefiCpuPkg/CpuDxe/CpuDxe.h
+++ b/UefiCpuPkg/CpuDxe/CpuDxe.h
@@ -35,15 +35,18 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
 
-#define HEAP_GUARD_NONSTOP_MODE   \
-((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT6|BIT4|BIT1|BIT0)) > BIT6)
+#define HEAP_GUARD_NONSTOP_MODE  (gDxeMps.HeapGuard.NonstopModeEnabled 
   &&  \
+ (gDxeMps.HeapGuard.PageGuardEnabled   
   ||  \
+  gDxeMps.HeapGuard.PoolGuardEnabled   
   ||  \
+  
gDxeMps.HeapGuard.FreedMemoryGuardEnabled)) \
 
-#define NULL_DETECTION_NONSTOP_MODE   \
-((PcdGet8 (PcdNullPointerDetectionPropertyMask) & (BIT6|BIT0)) > BIT6)
+#define NULL_DETECTION_NONSTOP_MODE  (gDxeMps.NullPointerDetection.Enabled && \
+  
gDxeMps.NullPointerDetection.NonstopModeEnabled)
 
 /**
   Flush CPU data cache. If the instruction cache is fully coherent
diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.inf b/UefiCpuPkg/CpuDxe/CpuDxe.inf
index 1d3e9f8cdb..ab2bd96d97 100644
--- a/UefiCpuPkg/CpuDxe/CpuDxe.inf
+++ b/UefiCpuPkg/CpuDxe/CpuDxe.inf
@@ -40,6 +40,7 @@
   MpInitLib
   TimerLib
   PeCoffGetEntryPointLib
+  DxeMemoryProtectionHobLib
 
 [Sources]
   CpuDxe.c
@@ -74,9 +75,6 @@
 
 [Pcd]
   gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask## 
CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard   ## 
CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask   ## 
CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask## 
CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuStackSwitchExceptionList  ## 
CONSUMES
   gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownGoodStackSize## 
CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdTdxSharedBitMask## 
CONSUMES
diff --git a/UefiCpuPkg/CpuDxe/CpuMp.c 

[edk2-devel] [PATCH 09/14] OvmfPkg: Update to use memory protection HOB

2023-07-11 Thread Taylor Beebe
From: Taylor Beebe 

Replace references to the memory protection PCDs with references
to the memory protection HOB.

The stack NX setting will no longer be fetched from the QEMU
configuration file and will instead be determined via the
HOB published in PlatformPei/Platform.c. PeilessStartup
will check the HOB when creating the page tables
at DXE handoff.

Signed-off-by: Taylor Beebe 
Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc: Gerd Hoffmann 
---
 OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf |   1 -
 OvmfPkg/Fdt/HighMemDxe/HighMemDxe.c   |   5 +-
 OvmfPkg/Fdt/HighMemDxe/HighMemDxe.inf |   4 +-
 OvmfPkg/Include/Library/PlatformInitLib.h |  13 ---
 OvmfPkg/Library/PeilessStartupLib/DxeLoad.c   |  25 ++--
 .../PeilessStartupLib/PeilessStartup.c|   3 -
 .../PeilessStartupLib/PeilessStartupLib.inf   |   5 +-
 .../PeilessStartupLib/X64/PageTables.h|  23 +---
 .../PeilessStartupLib/X64/VirtualMemory.c | 107 ++
 OvmfPkg/Library/PlatformInitLib/Platform.c|  15 ---
 OvmfPkg/PlatformPei/IntelTdx.c|   2 -
 OvmfPkg/PlatformPei/Platform.c|  16 ---
 OvmfPkg/PlatformPei/PlatformPei.inf   |   1 -
 OvmfPkg/QemuVideoDxe/QemuVideoDxe.inf |   2 +-
 OvmfPkg/QemuVideoDxe/VbeShim.c|   5 +-
 OvmfPkg/RiscVVirt/RiscVVirt.dsc.inc   |  13 ---
 OvmfPkg/TdxDxe/TdxDxe.c   |   7 +-
 OvmfPkg/TdxDxe/TdxDxe.inf |   1 -
 18 files changed, 68 insertions(+), 180 deletions(-)

diff --git a/OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf 
b/OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf
index 739d63098b..27b4a595fe 100644
--- a/OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf
+++ b/OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf
@@ -88,7 +88,6 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved
   gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode
   gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable
-  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack
   gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable
   gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask
   gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy
diff --git a/OvmfPkg/Fdt/HighMemDxe/HighMemDxe.c 
b/OvmfPkg/Fdt/HighMemDxe/HighMemDxe.c
index 779bf5c827..93bce776a9 100644
--- a/OvmfPkg/Fdt/HighMemDxe/HighMemDxe.c
+++ b/OvmfPkg/Fdt/HighMemDxe/HighMemDxe.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -148,9 +149,7 @@ InitializeHighMemDxe (
 // on the page table mappings by going through the cpu arch protocol.
 //
 Attributes = EFI_MEMORY_WB;
-if ((PcdGet64 (PcdDxeNxMemoryProtectionPolicy) &
- (1U << (UINT32)EfiConventionalMemory)) != 0)
-{
+if (gDxeMps.ExecutionProtection.EnabledForType[EfiConventionalMemory]) 
{
   Attributes |= EFI_MEMORY_XP;
 }
 
diff --git a/OvmfPkg/Fdt/HighMemDxe/HighMemDxe.inf 
b/OvmfPkg/Fdt/HighMemDxe/HighMemDxe.inf
index c7dde9f455..2d3add492b 100644
--- a/OvmfPkg/Fdt/HighMemDxe/HighMemDxe.inf
+++ b/OvmfPkg/Fdt/HighMemDxe/HighMemDxe.inf
@@ -33,13 +33,11 @@
   PcdLib
   UefiBootServicesTableLib
   UefiDriverEntryPoint
+  DxeMemoryProtectionHobLib
 
 [Protocols]
   gEfiCpuArchProtocolGuid ## CONSUMES
   gFdtClientProtocolGuid  ## CONSUMES
 
-[Pcd]
-  gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy
-
 [Depex]
   gEfiCpuArchProtocolGuid AND gFdtClientProtocolGuid
diff --git a/OvmfPkg/Include/Library/PlatformInitLib.h 
b/OvmfPkg/Include/Library/PlatformInitLib.h
index 57b18b94d9..b2468f2063 100644
--- a/OvmfPkg/Include/Library/PlatformInitLib.h
+++ b/OvmfPkg/Include/Library/PlatformInitLib.h
@@ -32,7 +32,6 @@ typedef struct {
   UINT32   Uc32Base;
   UINT32   Uc32Size;
 
-  BOOLEAN  PcdSetNxForStack;
   UINT64   PcdTdxSharedBitMask;
 
   UINT64   PcdPciMmio64Base;
@@ -182,18 +181,6 @@ PlatformMemMapInitialization (
   IN OUT EFI_HOB_PLATFORM_INFO  *PlatformInfoHob
   );
 
-/**
- * Fetch "opt/ovmf/PcdSetNxForStack" from QEMU
- *
- * @param Setting The pointer to the setting of 
"/opt/ovmf/PcdSetNxForStack".
- * @return EFI_SUCCESS  Successfully fetch the settings.
- */
-EFI_STATUS
-EFIAPI
-PlatformNoexecDxeInitialization (
-  IN OUT EFI_HOB_PLATFORM_INFO  *PlatformInfoHob
-  );
-
 VOID
 EFIAPI
 PlatformMiscInitialization (
diff --git a/OvmfPkg/Library/PeilessStartupLib/DxeLoad.c 
b/OvmfPkg/Library/PeilessStartupLib/DxeLoad.c
index d34690eb8a..169b4931a6 100644
--- a/OvmfPkg/Library/PeilessStartupLib/DxeLoad.c
+++ b/OvmfPkg/Library/PeilessStartupLib/DxeLoad.c
@@ -11,18 +11,21 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include "X64/PageTables.h"
 #include 
 
 #define STACK_SIZE  0x2
-extern EFI_GUID  

[edk2-devel] [PATCH 08/14] MdeModulePkg: Update to use memory protection HOB

2023-07-11 Thread Taylor Beebe
From: Taylor Beebe 

Replace references to the memory protection PCDs with references
to the memory protection HOB.

The memory protection HOB will be ingested during handoff to check
the memory proteciton settings when creating the page tables.

This patch also adjusts the logic for the memory protection callback
to apply protections one at a time (first NX, then stack protections,
then NULL protection etc.) with a debug print before each application
to help narrow down memory integrity related issues if they occur
when applying protections.

Signed-off-by: Taylor Beebe 
Cc: Jian J Wang 
Cc: Liming Gao 
Cc: Dandan Bi 
---
 MdeModulePkg/Core/Dxe/DxeMain.h   |   1 +
 MdeModulePkg/Core/Dxe/DxeMain.inf |   9 +-
 MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c   |   8 +-
 MdeModulePkg/Core/Dxe/Mem/HeapGuard.c |  88 ++---
 MdeModulePkg/Core/Dxe/Mem/HeapGuard.h |  24 +-
 MdeModulePkg/Core/Dxe/Mem/Page.c  |   4 +-
 MdeModulePkg/Core/Dxe/Mem/Pool.c  |   6 +-
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 338 ++
 MdeModulePkg/Core/DxeIplPeim/DxeHandoff.c |   4 +-
 MdeModulePkg/Core/DxeIplPeim/DxeIpl.h |  15 +
 MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf   |  13 +-
 MdeModulePkg/Core/DxeIplPeim/DxeLoad.c|  26 ++
 .../Core/DxeIplPeim/Ia32/DxeLoadFunc.c|  57 +--
 .../Core/DxeIplPeim/X64/DxeLoadFunc.c |  20 +-
 .../Core/DxeIplPeim/X64/VirtualMemory.c   |  87 ++---
 .../Core/DxeIplPeim/X64/VirtualMemory.h   |  23 +-
 MdeModulePkg/Core/PiSmmCore/HeapGuard.c   |  60 +---
 MdeModulePkg/Core/PiSmmCore/HeapGuard.h   |  20 +-
 MdeModulePkg/Core/PiSmmCore/Page.c|   6 +-
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.h   |   1 +
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf |   4 +-
 MdeModulePkg/Core/PiSmmCore/Pool.c|   9 +-
 22 files changed, 367 insertions(+), 456 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h
index 43daa037be..9da87a33a9 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.h
+++ b/MdeModulePkg/Core/Dxe/DxeMain.h
@@ -84,6 +84,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 #include 
 #include 
+#include 
 
 //
 // attributes for reserved memory before it is promoted to system memory
diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf 
b/MdeModulePkg/Core/Dxe/DxeMain.inf
index 35d5bf0dee..3cbd1ae923 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -94,6 +94,7 @@
   DebugAgentLib
   CpuExceptionHandlerLib
   PcdLib
+  DxeMemoryProtectionHobLib
 
 [Guids]
   gEfiEventMemoryMapChangeGuid  ## PRODUCES ## 
Event
@@ -123,6 +124,7 @@
   gEfiMemoryAttributesTableGuid ## SOMETIMES_PRODUCES   ## 
SystemTable
   gEfiEndOfDxeEventGroupGuid## SOMETIMES_CONSUMES   ## 
Event
   gEfiHobMemoryAllocStackGuid   ## SOMETIMES_CONSUMES   ## 
SystemTable
+  gDxeMemoryProtectionSettingsGuid  ## CONSUMES ## HOB
 
 [Ppis]
   gEfiVectorHandoffInfoPpiGuid  ## UNDEFINED # HOB
@@ -179,13 +181,6 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryProfileMemoryType ## 
CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryProfilePropertyMask   ## 
CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdMemoryProfileDriverPath ## 
CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy   ## 
CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy ## 
CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask## 
CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPageType   ## 
CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType   ## 
CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask   ## 
CONSUMES
-  gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard   ## 
CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdFwVolDxeMaxEncapsulationDepth   ## 
CONSUMES
 
 # [Hob]
diff --git a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c 
b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
index 0e0f9769b9..5bcef4f399 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
+++ b/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c
@@ -257,9 +257,13 @@ DxeMain (
   ASSERT_EFI_ERROR (Status);
 
   //
-  // Setup Stack Guard
+  // Get the memory protection HOB entry and setup stack guard
   //
-  if (PcdGetBool (PcdCpuStackGuard)) {
+  GuidHob = GetFirstGuidHob ();
+  if ((GuidHob != NULL) &&
+  DXE_MPS_IS_STRUCT_VALID (GET_GUID_HOB_DATA (GuidHob)) &&
+  ((DXE_MEMORY_PROTECTION_SETTINGS *)GET_GUID_HOB_DATA 
(GuidHob))->CpuStackGuardEnabled)
+  {
 Status = InitializeSeparateExceptionStacks (NULL, NULL);
 ASSERT_EFI_ERROR (Status);
   }
diff --git a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c 

[edk2-devel] [PATCH 06/14] ArmPkg: Update to use memory protection HOB

2023-07-11 Thread Taylor Beebe
From: Taylor Beebe 

Replace references to the memory protection PCDs to instead
reference the memory protection HOB.

Signed-off-by: Taylor Beebe 
Cc: Leif Lindholm 
Cc: Ard Biesheuvel 
Cc: Sami Mujawar 
---
 ArmPkg/ArmPkg.dsc| 1 +
 ArmPkg/Drivers/CpuDxe/CpuDxe.c   | 5 ++---
 ArmPkg/Drivers/CpuDxe/CpuDxe.inf | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/ArmPkg/ArmPkg.dsc b/ArmPkg/ArmPkg.dsc
index 4939b3d59b..3d3605921f 100644
--- a/ArmPkg/ArmPkg.dsc
+++ b/ArmPkg/ArmPkg.dsc
@@ -57,6 +57,7 @@
   PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
   
PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf
   
PeCoffExtraActionLib|MdePkg/Library/BasePeCoffExtraActionLibNull/BasePeCoffExtraActionLibNull.inf
+  
DxeMemoryProtectionHobLib|MdeModulePkg/Library/MemoryProtectionHobLibNull/DxeMemoryProtectionHobLibNull.inf
 
   
UefiHiiServicesLib|MdeModulePkg/Library/UefiHiiServicesLib/UefiHiiServicesLib.inf
   HiiLib|MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf
diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.c b/ArmPkg/Drivers/CpuDxe/CpuDxe.c
index fc63e52784..6518a7130a 100644
--- a/ArmPkg/Drivers/CpuDxe/CpuDxe.c
+++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.c
@@ -12,6 +12,7 @@
 #include 
 
 #include 
+#include 
 
 BOOLEAN  mIsFlushingGCD;
 
@@ -241,7 +242,6 @@ RemapUnusedMemoryNx (
   VOID
   )
 {
-  UINT64 TestBit;
   UINTN  MemoryMapSize;
   UINTN  MapKey;
   UINTN  DescriptorSize;
@@ -251,8 +251,7 @@ RemapUnusedMemoryNx (
   EFI_MEMORY_DESCRIPTOR  *MemoryMapEnd;
   EFI_STATUS Status;
 
-  TestBit = LShiftU64 (1, EfiBootServicesData);
-  if ((PcdGet64 (PcdDxeNxMemoryProtectionPolicy) & TestBit) == 0) {
+  if (!gDxeMps.ExecutionProtection.EnabledForType[EfiBootServicesData]) {
 return;
   }
 
diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf b/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
index 7d8132200e..a0f94bc567 100644
--- a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
+++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
@@ -46,6 +46,7 @@
   CpuExceptionHandlerLib
   DebugLib
   DefaultExceptionHandlerLib
+  DxeMemoryProtectionHobLib
   DxeServicesTableLib
   HobLib
   MemoryAllocationLib
@@ -65,7 +66,6 @@
 
 [Pcd.common]
   gArmTokenSpaceGuid.PcdVFPEnabled
-  gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy
 
 [FeaturePcd.common]
   gArmTokenSpaceGuid.PcdDebuggerExceptionSupport
-- 
2.41.0.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106829): https://edk2.groups.io/g/devel/message/106829
Mute This Topic: https://groups.io/mt/100090637/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH 07/14] EmulatorPkg: Update to use memory protection HOB

2023-07-11 Thread Taylor Beebe
From: Taylor Beebe 

Replace references to the memory protection PCDs with references
to the memory protection HOB.

Signed-off-by: Taylor Beebe 
Cc: Andrew Fish 
Cc: Ray Ni 
---
 EmulatorPkg/EmulatorPkg.dsc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/EmulatorPkg/EmulatorPkg.dsc b/EmulatorPkg/EmulatorPkg.dsc
index b44435d7e6..132ba5e294 100644
--- a/EmulatorPkg/EmulatorPkg.dsc
+++ b/EmulatorPkg/EmulatorPkg.dsc
@@ -126,6 +126,7 @@
   SortLib|MdeModulePkg/Library/BaseSortLib/BaseSortLib.inf
   ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
   FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf
+  
DxeMemoryProtectionHobLib|MdeModulePkg/Library/MemoryProtectionHobLibNull/DxeMemoryProtectionHobLibNull.inf
 
 !if $(SECURE_BOOT_ENABLE) == TRUE
   RngLib|MdePkg/Library/BaseRngLibTimerLib/BaseRngLibTimerLib.inf
@@ -216,7 +217,6 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplBuildPageTables|FALSE
 
 [PcdsFixedAtBuild]
-  gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x
   gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE
   gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8040
   gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x0f
-- 
2.41.0.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106830): https://edk2.groups.io/g/devel/message/106830
Mute This Topic: https://groups.io/mt/100090638/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH 05/14] ArmVirtPkg: Create memory protection settings HOB

2023-07-11 Thread Taylor Beebe
From: Taylor Beebe 

Create the memory protection settings HOBs on Arm virtual platforms.
These platforms will use the DEBUG memory protection profile.

Signed-off-by: Taylor Beebe 
Cc: Ard Biesheuvel 
Cc: Leif Lindholm 
Cc: Sami Mujawar 
Cc: Gerd Hoffmann 
---
 ArmVirtPkg/ArmVirt.dsc.inc  |  9 
 ArmVirtPkg/MemoryInitPei/MemoryInitPeim.c   | 25 +++--
 ArmVirtPkg/MemoryInitPei/MemoryInitPeim.inf |  2 ++
 3 files changed, 34 insertions(+), 2 deletions(-)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 2443e8351c..3174b19e51 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -256,6 +256,15 @@
 [LibraryClasses.ARM]
   ArmSoftFloatLib|ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf
 
+[LibraryClasses.common.DXE_DRIVER, LibraryClasses.common.DXE_CORE, 
LibraryClasses.common.UEFI_APPLICATION, LibraryClasses.common.UEFI_DRIVER]
+  
DxeMemoryProtectionHobLib|MdeModulePkg/Library/MemoryProtectionHobLib/DxeMemoryProtectionHobLib.inf
+
+[LibraryClasses.common.SMM_CORE, LibraryClasses.common.DXE_SMM_DRIVER]
+  
MmMemoryProtectionHobLib|MdeModulePkg/Library/MemoryProtectionHobLib/SmmMemoryProtectionHobLib.inf
+
+[LibraryClasses.common.MM_CORE_STANDALONE, LibraryClasses.common.MM_STANDALONE]
+  
MmMemoryProtectionHobLib|MdeModulePkg/Library/MemoryProtectionHobLib/StandaloneMmMemoryProtectionHobLib.inf
+
 [BuildOptions]
   GCC:RELEASE_*_*_CC_FLAGS  = -DMDEPKG_NDEBUG
 
diff --git a/ArmVirtPkg/MemoryInitPei/MemoryInitPeim.c 
b/ArmVirtPkg/MemoryInitPei/MemoryInitPeim.c
index ef88a9df1d..2033df1a01 100644
--- a/ArmVirtPkg/MemoryInitPei/MemoryInitPeim.c
+++ b/ArmVirtPkg/MemoryInitPei/MemoryInitPeim.c
@@ -14,7 +14,9 @@
 #include 
 #include 
 #include 
+#include 
 #include 
+#include 
 
 EFI_STATUS
 EFIAPI
@@ -76,8 +78,10 @@ InitializeMemory (
   IN CONST EFI_PEI_SERVICES **PeiServices
   )
 {
-  UINTN   UefiMemoryBase;
-  EFI_STATUS  Status;
+  UINTN   UefiMemoryBase;
+  EFI_STATUS  Status;
+  DXE_MEMORY_PROTECTION_SETTINGS  DxeSettings;
+  MM_MEMORY_PROTECTION_SETTINGS   MmSettings;
 
   ASSERT (FixedPcdGet64 (PcdSystemMemoryBase) < (UINT64)MAX_ALLOC_ADDRESS);
 
@@ -100,5 +104,22 @@ InitializeMemory (
  );
   ASSERT_EFI_ERROR (Status);
 
+  DxeSettings = 
(DXE_MEMORY_PROTECTION_SETTINGS)DXE_MEMORY_PROTECTION_SETTINGS_DEBUG;
+  MmSettings  = 
(MM_MEMORY_PROTECTION_SETTINGS)MM_MEMORY_PROTECTION_SETTINGS_DEBUG;
+
+  DxeSettings.NullPointerDetection.DisableEndOfDxe = TRUE;
+
+  BuildGuidDataHob (
+,
+,
+sizeof (DxeSettings)
+);
+
+  BuildGuidDataHob (
+,
+,
+sizeof (MmSettings)
+);
+
   return Status;
 }
diff --git a/ArmVirtPkg/MemoryInitPei/MemoryInitPeim.inf 
b/ArmVirtPkg/MemoryInitPei/MemoryInitPeim.inf
index 2039f71a0e..044542ecb2 100644
--- a/ArmVirtPkg/MemoryInitPei/MemoryInitPeim.inf
+++ b/ArmVirtPkg/MemoryInitPei/MemoryInitPeim.inf
@@ -37,6 +37,8 @@
 
 [Guids]
   gEfiMemoryTypeInformationGuid
+  gDxeMemoryProtectionSettingsGuid
+  gMmMemoryProtectionSettingsGuid
 
 [FeaturePcd]
   gEmbeddedTokenSpaceGuid.PcdPrePiProduceMemoryTypeInformationHob
-- 
2.41.0.windows.2



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106828): https://edk2.groups.io/g/devel/message/106828
Mute This Topic: https://groups.io/mt/100090636/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH 04/14] OvmfPkg: Create the memory protection settings HOB

2023-07-11 Thread Taylor Beebe
From: Taylor Beebe 

Create the memory protection settings HOB on Ovmf platforms with
DEBUG settings.

Signed-off-by: Taylor Beebe 
Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc: Gerd Hoffmann 
---
 OvmfPkg/AmdSev/AmdSevX64.dsc  |  2 ++
 OvmfPkg/Bhyve/BhyveX64.dsc|  2 ++
 OvmfPkg/CloudHv/CloudHvX64.dsc|  2 ++
 .../Dsc/MemoryProtectionLibraries.dsc.inc | 15 +++
 OvmfPkg/IntelTdx/IntelTdxX64.dsc  |  2 ++
 OvmfPkg/Microvm/MicrovmX64.dsc|  2 ++
 OvmfPkg/OvmfPkgIa32.dsc   |  2 ++
 OvmfPkg/OvmfPkgIa32X64.dsc|  2 ++
 OvmfPkg/OvmfPkgX64.dsc|  2 ++
 OvmfPkg/OvmfXen.dsc   |  2 ++
 OvmfPkg/PlatformPei/Platform.c| 27 +--
 OvmfPkg/PlatformPei/PlatformPei.inf   |  2 ++
 OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc   |  1 +
 13 files changed, 61 insertions(+), 2 deletions(-)
 create mode 100644 OvmfPkg/Include/Dsc/MemoryProtectionLibraries.dsc.inc

diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc
index 2c6ed7c974..ce028fcb5c 100644
--- a/OvmfPkg/AmdSev/AmdSevX64.dsc
+++ b/OvmfPkg/AmdSev/AmdSevX64.dsc
@@ -370,6 +370,8 @@
 !endif
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
 
+!include OvmfPkg/Include/Dsc/MemoryProtectionLibraries.dsc.inc
+
 

 #
 # Pcd Section - list of all EDK II PCD Entries defined by this Platform.
diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
index 7fa40998ae..248b6020ed 100644
--- a/OvmfPkg/Bhyve/BhyveX64.dsc
+++ b/OvmfPkg/Bhyve/BhyveX64.dsc
@@ -411,6 +411,8 @@
 !endif
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
 
+!include OvmfPkg/Include/Dsc/MemoryProtectionLibraries.dsc.inc
+
 

 #
 # Pcd Section - list of all EDK II PCD Entries defined by this Platform.
diff --git a/OvmfPkg/CloudHv/CloudHvX64.dsc b/OvmfPkg/CloudHv/CloudHvX64.dsc
index e000deed9e..8828e298ca 100644
--- a/OvmfPkg/CloudHv/CloudHvX64.dsc
+++ b/OvmfPkg/CloudHv/CloudHvX64.dsc
@@ -451,6 +451,8 @@
 !endif
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
 
+!include OvmfPkg/Include/Dsc/MemoryProtectionLibraries.dsc.inc
+
 

 #
 # Pcd Section - list of all EDK II PCD Entries defined by this Platform.
diff --git a/OvmfPkg/Include/Dsc/MemoryProtectionLibraries.dsc.inc 
b/OvmfPkg/Include/Dsc/MemoryProtectionLibraries.dsc.inc
new file mode 100644
index 00..cd8552de0d
--- /dev/null
+++ b/OvmfPkg/Include/Dsc/MemoryProtectionLibraries.dsc.inc
@@ -0,0 +1,15 @@
+##
+#SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+#
+# Memory Protection Libraries
+#
+[LibraryClasses.common.DXE_DRIVER, LibraryClasses.common.DXE_CORE, 
LibraryClasses.common.UEFI_APPLICATION, LibraryClasses.common.UEFI_DRIVER]
+  
DxeMemoryProtectionHobLib|MdeModulePkg/Library/MemoryProtectionHobLib/DxeMemoryProtectionHobLib.inf
+
+[LibraryClasses.common.SMM_CORE, LibraryClasses.common.DXE_SMM_DRIVER]
+  
MmMemoryProtectionHobLib|MdeModulePkg/Library/MemoryProtectionHobLib/SmmMemoryProtectionHobLib.inf
+
+[LibraryClasses.common.MM_CORE_STANDALONE, LibraryClasses.common.MM_STANDALONE]
+  
MmMemoryProtectionHobLib|MdeModulePkg/Library/MemoryProtectionHobLib/StandaloneMmMemoryProtectionHobLib.inf
diff --git a/OvmfPkg/IntelTdx/IntelTdxX64.dsc b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
index 193657ff2d..27b9d4bf26 100644
--- a/OvmfPkg/IntelTdx/IntelTdxX64.dsc
+++ b/OvmfPkg/IntelTdx/IntelTdxX64.dsc
@@ -363,6 +363,8 @@
 !endif
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
 
+!include OvmfPkg/Include/Dsc/MemoryProtectionLibraries.dsc.inc
+
 

 #
 # Pcd Section - list of all EDK II PCD Entries defined by this Platform.
diff --git a/OvmfPkg/Microvm/MicrovmX64.dsc b/OvmfPkg/Microvm/MicrovmX64.dsc
index 2f75856393..611d64a6a0 100644
--- a/OvmfPkg/Microvm/MicrovmX64.dsc
+++ b/OvmfPkg/Microvm/MicrovmX64.dsc
@@ -452,6 +452,8 @@
   PciPcdProducerLib|OvmfPkg/Fdt/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf
   
PciExpressLib|OvmfPkg/Library/BaseCachingPciExpressLib/BaseCachingPciExpressLib.inf
 
+!include OvmfPkg/Include/Dsc/MemoryProtectionLibraries.dsc.inc
+
 

 #
 # Pcd Section - list of all EDK II PCD Entries defined by this Platform.
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index ed36935770..810d69651c 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -458,6 +458,8 @@
 !endif
   PciLib|OvmfPkg/Library/DxePciLibI440FxQ35/DxePciLibI440FxQ35.inf
 
+!include 

[edk2-devel] [PATCH 01/14] MdeModulePkg: Add DXE and MM Memory Protection Settings Definitions

2023-07-11 Thread Taylor Beebe
From: Taylor Beebe 

These headers provide settings definitions for memory protections,
settings profiles for easily enabling memory protections,
and the GUIDs used for producing the memory protection HOB.

The settings options are functionally 1:1 with the existing
PCD bitfield definitions. Instead of setting a fixed at build
PCD, memory protection settings will be created via a HOB
at runtime.

Signed-off-by: Taylor Beebe 
Cc: Jian J Wang 
Cc: Liming Gao 
Cc: Dandan Bi 
---
 .../Guid/DxeMemoryProtectionSettings.h| 413 ++
 .../Include/Guid/MmMemoryProtectionSettings.h | 211 +
 MdeModulePkg/MdeModulePkg.dec |  10 +
 3 files changed, 634 insertions(+)
 create mode 100644 MdeModulePkg/Include/Guid/DxeMemoryProtectionSettings.h
 create mode 100644 MdeModulePkg/Include/Guid/MmMemoryProtectionSettings.h

diff --git a/MdeModulePkg/Include/Guid/DxeMemoryProtectionSettings.h 
b/MdeModulePkg/Include/Guid/DxeMemoryProtectionSettings.h
new file mode 100644
index 00..93144494d5
--- /dev/null
+++ b/MdeModulePkg/Include/Guid/DxeMemoryProtectionSettings.h
@@ -0,0 +1,413 @@
+/** @file
+
+Defines memory protection settings guid and struct for DXE.
+
+Copyright (C) Microsoft Corporation. All rights reserved.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef DXE_MEMORY_PROTECTION_SETTINGS_H_
+#define DXE_MEMORY_PROTECTION_SETTINGS_H_
+
+#include 
+
+// Current iteration of DXE_MEMORY_PROTECTION_SETTINGS
+#define DXE_MEMORY_PROTECTION_SETTINGS_CURRENT_VERSION  1
+
+#define OEM_RESERVED_MPS_MEMORY_TYPE EfiMaxMemoryType
+#define OS_RESERVED_MPS_MEMORY_TYPE  (EfiMaxMemoryType + 1)
+#define MAX_DXE_MPS_MEMORY_TYPE  (EfiMaxMemoryType + 2)
+#define DXE_MPS_MEMORY_TYPE_BUFFER_SIZE  (MAX_DXE_MPS_MEMORY_TYPE * sizeof 
(BOOLEAN))
+
+typedef struct {
+  BOOLEANEnabled: 1;
+  BOOLEANDisableEndOfDxe: 1;
+  BOOLEANNonstopModeEnabled : 1;
+} DXE_NULL_DETECTION_POLICY;
+
+typedef struct {
+  BOOLEANProtectImageFromUnknown : 1;
+  BOOLEANProtectImageFromFv  : 1;
+} DXE_IMAGE_PROTECTION_POLICY;
+
+typedef struct {
+  BOOLEANPageGuardEnabled: 1;
+  BOOLEANPoolGuardEnabled: 1;
+  BOOLEANFreedMemoryGuardEnabled : 1;
+  BOOLEANNonstopModeEnabled  : 1;
+  BOOLEANGuardAlignedToTail  : 1;
+} DXE_HEAP_GUARD_POLICY;
+
+typedef struct {
+  BOOLEANEnabledForType[MAX_DXE_MPS_MEMORY_TYPE];
+} DXE_MPS_MEMORY_TYPES;
+
+typedef UINT8 DXE_MEMORY_PROTECTION_SETTINGS_VERSION;
+
+//
+// Memory Protection Settings struct
+//
+typedef struct {
+  // The current version of the structure definition. This is used to ensure 
there isn't a
+  // definition mismatch if modules have differing iterations of this header. 
When creating
+  // this struct, use the DXE_MEMORY_PROTECTION_SETTINGS_CURRENT_VERSION macro.
+  DXE_MEMORY_PROTECTION_SETTINGS_VERSIONStructVersion;
+
+  // If enabled, the page at the top of the stack will be invalidated to catch 
stack overflow.
+  BOOLEAN   CpuStackGuardEnabled;
+
+  // If enabled, the stack will be marked non-executable.
+  BOOLEAN   StackExecutionProtectionEnabled;
+
+  // If enabled, accessing the NULL address in UEFI will be caught by marking
+  // the NULL page as not present.
+  //   .NullDetectionEnabled: Enable NULL pointer detection.
+  //   .DisableEndOfDxe : Disable NULL pointer detection just after 
EndOfDxe.
+  //  This is a workaround for those unsolvable 
NULL access issues in
+  //  OptionROM, boot loader, etc. It can also 
help to avoid unnecessary
+  //  exception caused by legacy memory (0-4095) 
access after EndOfDxe,
+  //  such as Windows 7 boot on Qemu.
+  //   .NonstopModeEnabled  : If enabled the debug flag will be raised 
when a fault occurs
+  //  to break into debugger.
+  DXE_NULL_DETECTION_POLICYNullPointerDetection;
+
+  // Set image protection policy.
+  //
+  //  .ProtectImageFromUnknown  : If set, images from unknown devices 
will be protected by
+  //  DxeCore if they are aligned. The 
code section becomes
+  //  read-only, and the data section 
becomes non-executable.
+  //  .ProtectImageFromFv   : If set, images from firmware volumes 
will be protected by
+  //  DxeCore if they are aligned. The 
code section becomes
+  //  read-only, and the data section 
becomes non-executable.
+  DXE_IMAGE_PROTECTION_POLICYImageProtection;
+
+  // If a bit is set, memory regions of the associated type will be mapped 
non-executable.
+  //
+  // The execution protection setting for EfiBootServicesData and 
EfiConventionalMemory must
+  // be the same.
+  

[edk2-devel] [PATCH 03/14] MdeModulePkg: Add Phase-Specific MemoryProtectionHobLib Implementations

2023-07-11 Thread Taylor Beebe
From: Taylor Beebe 

Add DXE, SMM, and STANDALONE MM implementations of the
MemoryProtectionHobLib.

Signed-off-by: Taylor Beebe 
Cc: Jian J Wang 
Cc: Liming Gao 
Cc: Dandan Bi 
---
 .../DxeMemoryProtectionHobLib.c   | 132 ++
 .../DxeMemoryProtectionHobLib.inf |  34 +
 .../MmCommonMemoryProtectionHobLib.c  |  89 
 .../SmmMemoryProtectionHobLib.c   |  37 +
 .../SmmMemoryProtectionHobLib.inf |  35 +
 .../StandaloneMmMemoryProtectionHobLib.c  |  37 +
 .../StandaloneMmMemoryProtectionHobLib.inf|  36 +
 MdeModulePkg/MdeModulePkg.dsc |   3 +
 8 files changed, 403 insertions(+)
 create mode 100644 
MdeModulePkg/Library/MemoryProtectionHobLib/DxeMemoryProtectionHobLib.c
 create mode 100644 
MdeModulePkg/Library/MemoryProtectionHobLib/DxeMemoryProtectionHobLib.inf
 create mode 100644 
MdeModulePkg/Library/MemoryProtectionHobLib/MmCommonMemoryProtectionHobLib.c
 create mode 100644 
MdeModulePkg/Library/MemoryProtectionHobLib/SmmMemoryProtectionHobLib.c
 create mode 100644 
MdeModulePkg/Library/MemoryProtectionHobLib/SmmMemoryProtectionHobLib.inf
 create mode 100644 
MdeModulePkg/Library/MemoryProtectionHobLib/StandaloneMmMemoryProtectionHobLib.c
 create mode 100644 
MdeModulePkg/Library/MemoryProtectionHobLib/StandaloneMmMemoryProtectionHobLib.inf

diff --git 
a/MdeModulePkg/Library/MemoryProtectionHobLib/DxeMemoryProtectionHobLib.c 
b/MdeModulePkg/Library/MemoryProtectionHobLib/DxeMemoryProtectionHobLib.c
new file mode 100644
index 00..fde568fcee
--- /dev/null
+++ b/MdeModulePkg/Library/MemoryProtectionHobLib/DxeMemoryProtectionHobLib.c
@@ -0,0 +1,132 @@
+/** @file
+Library fills out gDxeMps global
+
+Copyright (c) Microsoft Corporation.
+SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+DXE_MEMORY_PROTECTION_SETTINGS  gDxeMps;
+
+/**
+  This function checks the memory protection settings and provides warnings of 
settings conflicts.
+  For compatibility, this logic will only ever turn off protections to create 
consistency,
+  never turn others on.
+**/
+VOID
+DxeMemoryProtectionSettingsConsistencyCheck (
+  VOID
+  )
+{
+  if ((gDxeMps.HeapGuard.PoolGuardEnabled || 
gDxeMps.HeapGuard.PageGuardEnabled) &&
+  gDxeMps.HeapGuard.FreedMemoryGuardEnabled)
+  {
+DEBUG ((
+  DEBUG_WARN,
+  "%a: - HeapGuard.FreedMemoryGuardEnabled and "
+  "UEFI HeapGuard.PoolGuardEnabled/HeapGuard.PageGuardEnabled "
+  "cannot be active at the same time. Setting all three to ZERO in "
+  "the memory protection settings global.\n",
+  __func__
+  ));
+ASSERT (
+  !(gDxeMps.HeapGuard.FreedMemoryGuardEnabled &&
+(gDxeMps.HeapGuard.PoolGuardEnabled || 
gDxeMps.HeapGuard.PageGuardEnabled))
+  );
+gDxeMps.HeapGuard.PoolGuardEnabled= FALSE;
+gDxeMps.HeapGuard.PageGuardEnabled= FALSE;
+gDxeMps.HeapGuard.FreedMemoryGuardEnabled = FALSE;
+  }
+
+  if (DXE_MPS_IS_ANY_MEMORY_TYPE_ACTIVE () &&
+  (!(gDxeMps.HeapGuard.PoolGuardEnabled)))
+  {
+DEBUG ((
+  DEBUG_WARN,
+  "%a: - PoolGuard protections are active "
+  "but HeapGuard.PoolGuardEnabled is inactive.\n",
+  __func__
+  ));
+  }
+
+  if (DXE_MPS_IS_ANY_MEMORY_TYPE_ACTIVE () &&
+  (!(gDxeMps.HeapGuard.PageGuardEnabled)))
+  {
+DEBUG ((
+  DEBUG_WARN,
+  "%a: - PageGuard protections are active "
+  "but HeapGuard.PageGuardEnabled is inactive\n",
+  __func__
+  ));
+  }
+
+  if (gDxeMps.ExecutionProtection.EnabledForType[EfiBootServicesData] !=
+  gDxeMps.ExecutionProtection.EnabledForType[EfiConventionalMemory])
+  {
+DEBUG ((
+  DEBUG_WARN,
+  "%a: - EfiBootServicesData and EfiConventionalMemory must have the same "
+  "ExecutionProtection value. Setting both to ZERO in the memory 
protection "
+  "settings global.\n",
+  __func__
+  ));
+ASSERT (
+  gDxeMps.ExecutionProtection.EnabledForType[EfiBootServicesData] ==
+  gDxeMps.ExecutionProtection.EnabledForType[EfiConventionalMemory]
+  );
+gDxeMps.ExecutionProtection.EnabledForType[EfiBootServicesData]   = FALSE;
+gDxeMps.ExecutionProtection.EnabledForType[EfiConventionalMemory] = FALSE;
+  }
+}
+
+/**
+  Populates gDxeMps global with the data present in the HOB. If the HOB entry 
does not exist,
+  this constructor will zero the memory protection settings.
+
+  @param[in]  ImageHandle   The firmware allocated handle for the EFI image.
+  @param[in]  SystemTable   A pointer to the EFI System Table.
+
+  @retval EFI_SUCCESS   The constructor always returns EFI_SUCCESS.
+**/
+EFI_STATUS
+EFIAPI
+DxeMemoryProtectionHobLibConstructor (
+  IN EFI_HANDLEImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  VOID*Ptr;
+  DXE_MEMORY_PROTECTION_SETTINGS  *DxeMps;
+
+  Ptr = GetFirstGuidHob ();
+
+  //
+ 

[edk2-devel] [PATCH 00/14] Implement Dynamic Memory Protections

2023-07-11 Thread Taylor Beebe
In the past, memory protection settings were configured via FixedAtBuild PCDs,
which resulted in a build-time configuration of memory mitigations. This
approach limited the flexibility of applying mitigations to the
system and made it difficult to update or adjust the settings post-build.

In a design, the configuration interface has been revised to allow for dynamic
configuration. This is achieved with HOBs that are published prior to invocation
of the HandoffToDxe() function.

OvmfPkg/PlatformPei/Platform.c contains an example of how to publish the HOB
for DXE and MM.

To check the memory protection settings after PEI, the HOB can be easily 
consumed,
sanity checked, and put into a global for access via the inclusion of the DXE
or MM HOB libraries.

This patch series also increases the memory protection level for OvmfPkg and
ArmVirtPkg.

Reference: https://github.com/tianocore/edk2/pull/4566

Cc: Jian J Wang 
Cc: Liming Gao 
Cc: Dandan Bi 
Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc: Gerd Hoffmann 
Cc: Leif Lindholm 
Cc: Sami Mujawar 
Cc: Andrew Fish 
Cc: Ray Ni 
Cc: Eric Dong 
Cc: Rahul Kumar 
Cc: Guo Dong 
Cc: Sean Rhodes 
Cc: James Lu 
Cc: Gua Guo 

Taylor Beebe (14):
  MdeModulePkg: Add DXE and MM Memory Protection Settings Definitions
  MdeModulePkg: Add MemoryProtectionHobLib Definitions and NULL Libs
  MdeModulePkg: Add Phase-Specific MemoryProtectionHobLib
Implementations
  OvmfPkg: Create the memory protection settings HOB
  ArmVirtPkg: Create memory protection settings HOB
  ArmPkg: Update to use memory protection HOB
  EmulatorPkg: Update to use memory protection HOB
  MdeModulePkg: Update to use memory protection HOB
  OvmfPkg: Update to use memory protection HOB
  UefiCpuPkg: Update to use memory protection HOB
  UefiPayloadPkg: Update to use memory protection HOB
  OvmfPkg: Delete Memory Protection PCDs
  ArmVirtPkg: Delete Memory Protection PCDs
  MdeModulePkg: Delete Memory Protection PCDs

 ArmPkg/ArmPkg.dsc |   1 +
 ArmPkg/Drivers/CpuDxe/CpuDxe.c|   5 +-
 ArmPkg/Drivers/CpuDxe/CpuDxe.inf  |   2 +-
 ArmVirtPkg/ArmVirt.dsc.inc|  24 +-
 ArmVirtPkg/ArmVirtCloudHv.dsc |   5 -
 ArmVirtPkg/ArmVirtQemu.dsc|   5 -
 ArmVirtPkg/MemoryInitPei/MemoryInitPeim.c |  25 +-
 ArmVirtPkg/MemoryInitPei/MemoryInitPeim.inf   |   2 +
 EmulatorPkg/EmulatorPkg.dsc   |   2 +-
 MdeModulePkg/Core/Dxe/DxeMain.h   |   1 +
 MdeModulePkg/Core/Dxe/DxeMain.inf |   9 +-
 MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c   |   8 +-
 MdeModulePkg/Core/Dxe/Mem/HeapGuard.c |  88 ++--
 MdeModulePkg/Core/Dxe/Mem/HeapGuard.h |  24 +-
 MdeModulePkg/Core/Dxe/Mem/Page.c  |   4 +-
 MdeModulePkg/Core/Dxe/Mem/Pool.c  |   6 +-
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 338 +++---
 MdeModulePkg/Core/DxeIplPeim/DxeHandoff.c |   4 +-
 MdeModulePkg/Core/DxeIplPeim/DxeIpl.h |  15 +
 MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf   |  13 +-
 MdeModulePkg/Core/DxeIplPeim/DxeLoad.c|  26 ++
 .../Core/DxeIplPeim/Ia32/DxeLoadFunc.c|  57 +--
 .../Core/DxeIplPeim/X64/DxeLoadFunc.c |  20 +-
 .../Core/DxeIplPeim/X64/VirtualMemory.c   |  87 ++--
 .../Core/DxeIplPeim/X64/VirtualMemory.h   |  23 +-
 MdeModulePkg/Core/PiSmmCore/HeapGuard.c   |  60 +--
 MdeModulePkg/Core/PiSmmCore/HeapGuard.h   |  20 +-
 MdeModulePkg/Core/PiSmmCore/Page.c|   6 +-
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.h   |   1 +
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf |   4 +-
 MdeModulePkg/Core/PiSmmCore/Pool.c|   9 +-
 .../Guid/DxeMemoryProtectionSettings.h| 413 ++
 .../Include/Guid/MmMemoryProtectionSettings.h | 211 +
 .../Library/DxeMemoryProtectionHobLib.h   |  20 +
 .../Library/MmMemoryProtectionHobLib.h|  20 +
 .../DxeMemoryProtectionHobLib.c   | 132 ++
 .../DxeMemoryProtectionHobLib.inf |  34 ++
 .../MmCommonMemoryProtectionHobLib.c  |  89 
 .../SmmMemoryProtectionHobLib.c   |  37 ++
 .../SmmMemoryProtectionHobLib.inf |  35 ++
 .../StandaloneMmMemoryProtectionHobLib.c  |  37 ++
 .../StandaloneMmMemoryProtectionHobLib.inf|  36 ++
 .../DxeMemoryProtectionHobLibNull.c   |  13 +
 .../DxeMemoryProtectionHobLibNull.inf |  28 ++
 .../MmMemoryProtectionHobLibNull.c|  13 +
 .../MmMemoryProtectionHobLibNull.inf  |  28 ++
 MdeModulePkg/MdeModulePkg.dec | 187 +---
 MdeModulePkg/MdeModulePkg.dsc |  11 +
 MdeModulePkg/MdeModulePkg.uni | 153 ---
 OvmfPkg/AmdSev/AmdSevX64.dsc  |   5 +-
 OvmfPkg/Bhyve/BhyveX64.dsc|   5 +-
 OvmfPkg/Bhyve/PlatformPei/PlatformPei.inf |   1 -
 OvmfPkg/CloudHv/CloudHvX64.dsc|   5 +-
 

Re: [edk2-devel] Managing boot order dependencies with ResetNotification Protocol

2023-07-11 Thread Andrew Fish via groups.io
Ajay,

In general the order of dispatch when all the DEPEX evaluate to TRUE and the 
oder of events are not defined by the specification. 

I think you are trying to say stuff that dispatches later is higher up the 
stack and it would make more sense to send the rest to things higher up the 
stack? 

I’m not sure how this would work in your example as the Tcg2Dxe driver has a 
Depex of gEfiVariableArchProtocolGuid and the NVMe driver is a UEFI_DRIVER so 
it has an implied Depex [1] that is more complex?

To really fix we would probably need to add some kind of new protocol tot he 
NVMe driver that lets drivers request the reset get delayed. It would have a 
register, unregister, I’m Done.  
The NVMe driver would do something like:
1) Register for the  reset notification
2) Publish the new protocol
3) When the 1st driver registers the NVMe driver unregisters the reset 
notification, or I guess it could just ignore it. 

When the shutdown happens and the last registered driver calls back in to say 
I’m Done then the NVMe driver can shutdown. 
The other drivers will need to RegisterProtocolNotify so there is not dispatch 
sequence dependency.

I’m not sure that is the best solution, that is just the best I have right now.

[1] UEFI_DRIVER depends on all the protocols required to impelemnt all the UEFI 
Boot and Runtime Services.
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe/DxeMain/DxeProtocolNotify.c#L21
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe/DxeMain/DxeProtocolNotify.c#L80

Thanks,

Andrew FIsh

> On Jun 28, 2023, at 5:23 PM, Ajay Iyengar (QUIC)  
> wrote:
> 
> Hello,
>  
> This is a comment and query on the Reset Notification Protocol defined by the 
> UEFI specification.
>  
> The specification requires clients to be notified for ResetSystem() without 
> imposing any specific ordering on how these notifications are dispatched: 
> https://uefi.org/specs/UEFI/2.10/39_Micellaneous_Protocols.html#reset-notification-protocol.
>  
> For example, this is used for gracefully shutting off NVMe and TPM through 
> their respective spec defined power off notifications: 
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c#L950
> https://github.com/tianocore/edk2/blob/master/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c#L2597
>  
> If we assume a platform implementing fTPM using NVMe-RPMB,  the boot order 
> would require NVMe as a dependency for TPM, but that might result in an issue 
> with the reset notification path. The default EDK2 implementation dispatches 
> these notifications in boot order: 
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c#L78.
>   Here NVMe would power off, before TPM commits to NVMe-RPMB within its reset 
> notification callback. Reversing the order of dispatch (i.e. reverse-boot 
> order) would help with this situation.
>  
> We have several similar situations on our platform -- drivers with a Depex 
> ordering registering for ResetNotify which would benefit from being signaled 
> in reverse-boot order. Are there any provisions that could be made with 
> regards to ensuring the ordering requirement?  Could we mimic the behavior of 
> the ExitBootServices event dispatch for ResetNotify:
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe/Event/Event.c#L492
>  ?
> [The proposal would be to use InsertHeadList() instead of InsertTailList() in 
> RegisterResetNotify()]
>  
> Thanks,
> Ajay



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106822): https://edk2.groups.io/g/devel/message/106822
Mute This Topic: https://groups.io/mt/99845132/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: 
https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH] MdeModulePkg/PciHostBridge: Add support for driver binding

2023-07-11 Thread Jeff Brasen via groups.io
Ray,

  Would you prefer this sort of use would be done by an extra dispatch after 
the wait for everything being completed and the connect controller call in BDS 
as opposed to the driver binding approach?  Basically using a depex on the 
library as we are currently doing. 

-Jeff


> -Original Message-
> From: Jeff Brasen 
> Sent: Friday, June 30, 2023 9:57 AM
> To: Ni, Ray ; devel@edk2.groups.io
> Cc: Wang, Jian J ; Gao, Liming
> ; Wu, Hao A 
> Subject: RE: [PATCH] MdeModulePkg/PciHostBridge: Add support for driver
> binding
> 
> 
> 
> > -Original Message-
> > From: Ni, Ray 
> > Sent: Thursday, June 29, 2023 9:59 PM
> > To: Jeff Brasen ; devel@edk2.groups.io
> > Cc: Wang, Jian J ; Gao, Liming
> > ; Wu, Hao A 
> > Subject: RE: [PATCH] MdeModulePkg/PciHostBridge: Add support for
> > driver binding
> >
> > External email: Use caution opening links or attachments
> >
> >
> > > -Original Message-
> > > From: Jeff Brasen 
> > > Sent: Friday, June 30, 2023 11:21 AM
> > > To: Ni, Ray ; devel@edk2.groups.io
> > > Cc: Wang, Jian J ; Gao, Liming
> > > ; Wu, Hao A 
> > > Subject: RE: [PATCH] MdeModulePkg/PciHostBridge: Add support for
> > > driver binding
> > >
> > > Not sure why the patch failed to apply I'll see if there is
> > > something wrong with my gitconfig tomorrow. The path you suggested
> > > below is exactly what our current implementation does. However, I am
> > > trying to make our pcie controller driver do async initialization so
> > > that using a depex is less ideal as we may have to postpone driver
> > > load to after end of dxe instead of just the connection. It seemed
> > > that a driver binding
> > type approach was a good approach for this.
> >
> > I am curious how the pcie root (root complex) initialization is done
> > in async way?
> > Does it use a timer callback to poll the initialization status every
> > certain interval?
> >
> 
> [JB] That is correct, we use a timer when we expect sleeps or polling loops.
> 
> > On the other hand, even you make PciHostBridge as a UEFI driver-model
> > driver, you still require some code to initiate the
> > "ConnectController()". How is that done?
> >
> > Comparing the two paths (today's = my proposal, new way = your patch),
> > both require some code to explicitly call "ConnectController()".
> >
> 
> [JB] We have a sync point in BDS prior to the ConnectController call that is
> made. I could put that and a dispatch call prior to EndOfDxe signal but was
> hoping to not trigger any of the driver seen but not loaded debug messages
> from the main dispatch loop if possible as that can be a useful message to
> trigger something might be wrong.
> 
> > >
> > > On a less important implementation if the pieces that live under the
> > > library are driver binding we have to reject any stop requests as
> > > there is no driver linkage between the two layers.
> >
> > In x86, root complex (pcie root) is almost the root of all
> > peripherals. I cannot see a value to Stop () the pcie root.
> > If you check the PciBus implementation, it supports Stop() but the
> > Stop() only succeeds when all upper layer drivers succeed to Stop().
> > (usually it's not the
> > case.) And even PciBus.Stop() succeeds, the resource(MMIO/IO/BUS)
> > allocation is not un-done. It's only the PciIo handles that are
> > destroyed in software level.
> >
> 
> [JB] Yeah, not allowing stop to work on this isn't a big deal (and what we
> currently have implemented) but it was something we noticed a while ago is
> that we couldn't implement this even if we wanted to as there was no way to
> stop the host bridge driver.
> 
> > Thanks,
> > Ray
> >
> >
> > >
> > > -Jeff
> > >
> > >
> > > -Original Message-
> > > From: Ni, Ray 
> > > Sent: Thursday, June 29, 2023 8:29 PM
> > > To: Jeff Brasen ; devel@edk2.groups.io
> > > Cc: Wang, Jian J ; Gao, Liming
> > > ; Wu, Hao A 
> > > Subject: RE: [PATCH] MdeModulePkg/PciHostBridge: Add support for
> > > driver binding
> > >
> > > External email: Use caution opening links or attachments
> > >
> > >
> > > I failed to apply the patch in my local tree.
> > >
> > > It seems you invented a new EdkiiRootBridgeIo protocol and a certain
> > > proprietary driver would produce this protocol instance.
> > > Then the open source PciHostBridge driver starts on that.
> > >
> > > Then, why not implement your own PciHostBridgeLib and let it depends
> > > on some "AllRootBridgeIoInformationIsReady" protocol.
> > > So that the PciHostBridge driver could still call PciHostBridgeLib
> > > and all your implementation in this patch can be in that lib.
> > >
> > > Thanks,
> > > Ray
> > >
> > > > -Original Message-
> > > > From: Jeff Brasen 
> > > > Sent: Friday, June 30, 2023 4:54 AM
> > > > To: devel@edk2.groups.io
> > > > Cc: Wang, Jian J ; Gao, Liming
> > > > ; Wu, Hao A ; Ni,
> > > > Ray ; Jeff Brasen 
> > > > Subject: [PATCH] MdeModulePkg/PciHostBridge: Add support for
> > > > driver binding
> > > >
> > > > If the platform does not 

Re: [edk2-devel] [edk2-platforms][PATCH V1 19/20] ArmPkg/MmCommunicationDxe: Use the FF-A transport for MM requests

2023-07-11 Thread Kun Qin

Hi Nishant,

Thank you for sending out the patch. Can you please evaluate how much 
effort it would be
to support the same for MmCommunicatePei? I think it would provide 
better coverage for

the FFA support if we can have that change.

Thanks,
Kun

On 7/11/2023 7:36 AM, Nishant Sharma wrote:

From: Achin Gupta 

This patch packages requests for accessing a Standalone MM driver
through the MM communication protocol as FF-A direct messages.
Corresponding changes in Standalone MM Core ensure that responses are
packaged in the same way.

Signed-off-by: Achin Gupta 
Co-developed-by: Aditya Angadi 
Signed-off-by: Nishant Sharma 
---
  ArmPkg/Include/IndustryStandard/ArmFfaSvc.h |   2 +
  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 141 +---
  2 files changed, 97 insertions(+), 46 deletions(-)

diff --git a/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h 
b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
index 530af8bd3c2e..493997346143 100644
--- a/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
+++ b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
@@ -23,6 +23,7 @@
  #define ARM_SVC_ID_FFA_RXTX_UNMAP_AARCH320x8467
  #define ARM_SVC_ID_FFA_PARTITION_INFO_GET_AARCH320x8468
  #define ARM_SVC_ID_FFA_ID_GET_AARCH320x8469
+#define ARM_SVC_ID_FFA_RUN_AARCH32   0x846D
  #define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH32   0x846F
  #define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP_AARCH32  0x8470
  #define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH64   0xC46F
@@ -31,6 +32,7 @@
  #define ARM_SVC_ID_FFA_SUCCESS_AARCH64   0xC461
  #define ARM_SVC_ID_FFA_MEM_PERM_SET_AARCH32  0x8489
  #define ARM_SVC_ID_FFA_MEM_PERM_GET_AARCH32  0x8488
+#define ARM_SVC_ID_FFA_INTERRUPT_AARCH32 0x8462
  #define ARM_SVC_ID_FFA_ERROR_AARCH32 0x8460
  #define ARM_SVC_ID_FFA_ERROR_AARCH64 0xC460
  #define ARM_SVC_ID_FFA_MSG_WAIT_AARCH32  0x846B
diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c 
b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
index 94a5d96c051d..a70318581bd2 100644
--- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
@@ -100,6 +100,7 @@ MmCommunication2Communicate (
ARM_SMC_ARGS   CommunicateSmcArgs;
EFI_STATUS Status;
UINTN  BufferSize;
+  UINTN  Ret;
  
Status = EFI_ACCESS_DENIED;

BufferSize = 0;
@@ -160,60 +161,108 @@ MmCommunication2Communicate (
  return Status;
}
  
-  // SMC Function ID

-  CommunicateSmcArgs.Arg0 = ARM_SMC_ID_MM_COMMUNICATE_AARCH64;
-
-  // Cookie
-  CommunicateSmcArgs.Arg1 = 0;
-
// Copy Communication Payload
CopyMem ((VOID *)mNsCommBuffMemRegion.VirtualBase, CommBufferVirtual, 
BufferSize);
  
-  // comm_buffer_address (64-bit physical address)

-  CommunicateSmcArgs.Arg2 = (UINTN)mNsCommBuffMemRegion.PhysicalBase;
+  // Use the FF-A interface if enabled.
+  if (FixedPcdGet32 (PcdFfaEnable) != 0) {
+// FF-A Interface ID for direct message communication
+CommunicateSmcArgs.Arg0 = ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH64;
  
-  // comm_size_address (not used, indicated by setting to zero)

-  CommunicateSmcArgs.Arg3 = 0;
+// FF-A Destination EndPoint ID, not used as of now
+CommunicateSmcArgs.Arg1 = mFfaPartId << 16 | mStmmPartInfo.PartId;
  
+// Reserved for future use(MBZ)

+CommunicateSmcArgs.Arg2 = 0x0;
+
+// comm_buffer_address (64-bit physical address)
+CommunicateSmcArgs.Arg3 = (UINTN)mNsCommBuffMemRegion.PhysicalBase;
+
+// Cookie
+CommunicateSmcArgs.Arg4 = 0x0;
+
+// Not Used
+CommunicateSmcArgs.Arg5 = 0;
+
+// comm_size_address (not used, indicated by setting to zero)
+CommunicateSmcArgs.Arg6 = 0;
+  } else {
+// SMC Function ID
+CommunicateSmcArgs.Arg0 = ARM_SMC_ID_MM_COMMUNICATE_AARCH64;
+
+// Cookie
+CommunicateSmcArgs.Arg1 = 0;
+
+// comm_buffer_address (64-bit physical address)
+CommunicateSmcArgs.Arg2 = (UINTN)mNsCommBuffMemRegion.PhysicalBase;
+
+// comm_size_address (not used, indicated by setting to zero)
+CommunicateSmcArgs.Arg3 = 0;
+  }
+
+ffa_intr_loop:
// Call the Standalone MM environment.
ArmCallSmc ();
  
-  switch (CommunicateSmcArgs.Arg0) {

-case ARM_SMC_MM_RET_SUCCESS:
-  ZeroMem (CommBufferVirtual, BufferSize);
-  // On successful return, the size of data being returned is inferred from
-  // MessageLength + Header.
-  CommunicateHeader = (EFI_MM_COMMUNICATE_HEADER 
*)mNsCommBuffMemRegion.VirtualBase;
-  BufferSize= CommunicateHeader->MessageLength +
-  sizeof (CommunicateHeader->HeaderGuid) +
-  sizeof (CommunicateHeader->MessageLength);
-
-  CopyMem (
-CommBufferVirtual,
-(VOID 

Re: [edk2-devel] Use gMmst from MmServiceTableLib in MmSaveStateLib

2023-07-11 Thread Attar, AbdulLateef (Abdul Lateef) via groups.io
[AMD Official Use Only - General]

Thanks Dun for clarifying, I'll submit the patch.

Regards,
AbduL

-Original Message-
From: devel@edk2.groups.io  On Behalf Of duntan via 
groups.io
Sent: Tuesday, July 11, 2023 3:21 PM
To: Attar, AbdulLateef (Abdul Lateef) ; Ni, Ray 
; devel@edk2.groups.io; Chang, Abner 
Subject: Re: [edk2-devel] Use gMmst from MmServiceTableLib in MmSaveStateLib

[AMD Official Use Only - General]

Caution: This message originated from an External Source. Use proper caution 
when opening attachments, clicking links, or responding.


AbduL,

For the part ' preserve the SAVE_STATE pointed by gSmst(Instead of gMmst)', do 
you mean the following code in RestoreSmmConfigurationInS3()?
gSmst->CpuSaveState  = gSmmCpuPrivate->SmmCoreEntryContext.CpuSaveState;

Acctually when PiSmmCpuDxeSmm uses MmServicesTableLib, the gSmst is the same as 
gMmst. You can check the lib constructor MmServicesTableLibConstructor() and 
SmmServicesTableLibConstructor. In the two constructor functions, they both 
locate the same global pointer by gEfiSmmBase2ProtocolGuid/ 
gEfiMmBaseProtocolGuid(same GUID)  and assign the value to gSmst or gMmst. In 
this case the only difference of gMmst and gSmst is the naming. So I think  
PiSmmCpuDxeSmm can still consume the MmSaveStateLib even MmSaveStateLib use 
gMmst.

Thanks,
Dun
-Original Message-
From: Attar, AbdulLateef (Abdul Lateef) 
Sent: Tuesday, July 11, 2023 5:06 PM
To: Ni, Ray ; Tan, Dun ; 
devel@edk2.groups.io; Chang, Abner 
Subject: RE: Use gMmst from MmServiceTableLib in MmSaveStateLib

[AMD Official Use Only - General]

Hi Ray,
I think Michael raised the similar concerned during the patch review.
Its intentionally kept it as gSmst because of the below reason.

2. AmdMmSaveStateLib and IntelMmSaveStateLib depend on SmmServicesTableLib. Can 
they depend on MmServicesTableLib instead?
[Attar, AbdulLateef (Abdul Lateef)] MmSaveStateLib is mainly used by 
PiSmmCpuDxeSmm driver which still uses Smm convention and preserve the 
SAVE_STATE pointed by gSmst(Instead of gMmst).
Hence I don't think we can move to MmServicesTableLib.


Thanks
AbduL

-Original Message-
From: Ni, Ray 
Sent: Tuesday, July 11, 2023 1:40 PM
To: Tan, Dun ; devel@edk2.groups.io; Attar, AbdulLateef 
(Abdul Lateef) ; Chang, Abner 
Subject: RE: Use gMmst from MmServiceTableLib in MmSaveStateLib

Caution: This message originated from an External Source. Use proper caution 
when opening attachments, clicking links, or responding.


Abdul,
Can you please send a patch to fix MmSaveStateLib to use gMmst (instead of 
gSmst)?

Using gSmst forbids the lib instance be linked by standalone MM modules.

Thanks,
Ray

> -Original Message-
> From: Tan, Dun 
> Sent: Wednesday, July 5, 2023 4:42 PM
> To: devel@edk2.groups.io; abdat...@amd.com
> Cc: Ni, Ray 
> Subject: Use gMmst from MmServiceTableLib in MmSaveStateLib
>
> Hi Abdul,
>
> In the new MmSaveStateLib created in this patch set, gSmst from
> SmmServiceTableLib is used. This causes that only DXE_SMM_DRIVER type
> module can consume this lib but MM_STANDALONE type module cannot.
>
> In current edk2, there are different MmServicesTableLib and
> SmmServicesTableLib:
> StadaloneMmServicesTableLib(MmServicesTableLib|MM_STANDALONE):
> initialize gMmst in standalone SMM env.
>   MmServicesTableLib(MmServicesTableLib|DXE_SMM_DRIVER):
> initialize gMmst in legacy SMM env.
> SmmServicesTableLib(SmmServicesTableLib|DXE_SMM_DRIVER): initialize
> gSmst in legacy SMM env.
>
> If MmSaveStateLib uses gMmst from MmServiceTableLib instead of gSmst,
> then MmSaveStateLib can be consumed by both DXE_SMM_DRIVER and
> MM_STANDALONE module.
> Could you pls send patch to fix this?
>
> Thanks,
> Dun
>
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Abdul
> Lateef Attar via groups.io
> Sent: Sunday, July 2, 2023 12:14 PM
> To: devel@edk2.groups.io
> Cc: Abdul Lateef Attar ; Paul Grimes
> ; Abner Chang ; Dong, Eric
> ; Ni, Ray ; Kumar, Rahul R
> ; Gerd Hoffmann ; Kinney,
> Michael D ; Gao, Liming
> ; Liu, Zhiguang ;
> Ard Biesheuvel ; Yao, Jiewen
> ; Justen, Jordan L 
> Subject: [edk2-devel] [PATCH v15 0/8] Adds AmdSmmCpuFeaturesLib and
> MmSaveStateLib
>
> Backward-compatibility changes:
>   This patch series removes the SmmCpuFeaturesReadSaveStateRegister
>   and SmmCpuFeaturesWriteSaveStateRegister interface/function.
>   SmmReadSaveState() and SmmWriteSaveState() now directly invokes
> MmSaveStateLib
>   routines to save/restore registers.
>
> PR: https://github.com/tianocore/edk2/pull/4597
>
> V15: Delta changes
>   Rebase the branch and fix the merge conflicts.
> V14: Delta changes
>   Added @note to the MmSaveStateLib.h.
>   SaveState(Read/Write) of
> EFI_SMM_SAVE_STATE_REGISTER_PROCESSOR_ID/EFI_MM_SAVE_STATE_REGIS
> TER_PROCESSOR_ID
>   is handled by PiSmmCpuDxeSmm driver.
>   Fixed PatchCheck warnings.
> V13: Delta changes
>   Address review comments from Ray Ni
>   Changed the BASE _NAME 

Re: [edk2-devel] Use gMmst from MmServiceTableLib in MmSaveStateLib

2023-07-11 Thread Attar, AbdulLateef (Abdul Lateef) via groups.io
[AMD Official Use Only - General]

Hi Ray,
I think Michael raised the similar concerned during the patch review.
Its intentionally kept it as gSmst because of the below reason.

2. AmdMmSaveStateLib and IntelMmSaveStateLib depend on SmmServicesTableLib. Can 
they depend on MmServicesTableLib instead?
[Attar, AbdulLateef (Abdul Lateef)] MmSaveStateLib is mainly used by 
PiSmmCpuDxeSmm driver which still uses Smm convention and preserve the 
SAVE_STATE pointed by gSmst(Instead of gMmst).
Hence I don't think we can move to MmServicesTableLib.


Thanks
AbduL

-Original Message-
From: Ni, Ray 
Sent: Tuesday, July 11, 2023 1:40 PM
To: Tan, Dun ; devel@edk2.groups.io; Attar, AbdulLateef 
(Abdul Lateef) ; Chang, Abner 
Subject: RE: Use gMmst from MmServiceTableLib in MmSaveStateLib

Caution: This message originated from an External Source. Use proper caution 
when opening attachments, clicking links, or responding.


Abdul,
Can you please send a patch to fix MmSaveStateLib to use gMmst (instead of 
gSmst)?

Using gSmst forbids the lib instance be linked by standalone MM modules.

Thanks,
Ray

> -Original Message-
> From: Tan, Dun 
> Sent: Wednesday, July 5, 2023 4:42 PM
> To: devel@edk2.groups.io; abdat...@amd.com
> Cc: Ni, Ray 
> Subject: Use gMmst from MmServiceTableLib in MmSaveStateLib
>
> Hi Abdul,
>
> In the new MmSaveStateLib created in this patch set, gSmst from
> SmmServiceTableLib is used. This causes that only DXE_SMM_DRIVER type
> module can consume this lib but MM_STANDALONE type module cannot.
>
> In current edk2, there are different MmServicesTableLib and
> SmmServicesTableLib:
> StadaloneMmServicesTableLib(MmServicesTableLib|MM_STANDALONE):
> initialize gMmst in standalone SMM env.
>   MmServicesTableLib(MmServicesTableLib|DXE_SMM_DRIVER):
> initialize gMmst in legacy SMM env.
> SmmServicesTableLib(SmmServicesTableLib|DXE_SMM_DRIVER): initialize
> gSmst in legacy SMM env.
>
> If MmSaveStateLib uses gMmst from MmServiceTableLib instead of gSmst,
> then MmSaveStateLib can be consumed by both DXE_SMM_DRIVER and
> MM_STANDALONE module.
> Could you pls send patch to fix this?
>
> Thanks,
> Dun
>
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Abdul
> Lateef Attar via groups.io
> Sent: Sunday, July 2, 2023 12:14 PM
> To: devel@edk2.groups.io
> Cc: Abdul Lateef Attar ; Paul Grimes
> ; Abner Chang ; Dong, Eric
> ; Ni, Ray ; Kumar, Rahul R
> ; Gerd Hoffmann ; Kinney,
> Michael D ; Gao, Liming
> ; Liu, Zhiguang ;
> Ard Biesheuvel ; Yao, Jiewen
> ; Justen, Jordan L 
> Subject: [edk2-devel] [PATCH v15 0/8] Adds AmdSmmCpuFeaturesLib and
> MmSaveStateLib
>
> Backward-compatibility changes:
>   This patch series removes the SmmCpuFeaturesReadSaveStateRegister
>   and SmmCpuFeaturesWriteSaveStateRegister interface/function.
>   SmmReadSaveState() and SmmWriteSaveState() now directly invokes
> MmSaveStateLib
>   routines to save/restore registers.
>
> PR: https://github.com/tianocore/edk2/pull/4597
>
> V15: Delta changes
>   Rebase the branch and fix the merge conflicts.
> V14: Delta changes
>   Added @note to the MmSaveStateLib.h.
>   SaveState(Read/Write) of
> EFI_SMM_SAVE_STATE_REGISTER_PROCESSOR_ID/EFI_MM_SAVE_STATE_REGIS
> TER_PROCESSOR_ID
>   is handled by PiSmmCpuDxeSmm driver.
>   Fixed PatchCheck warnings.
> V13: Delta changes
>   Address review comments from Ray Ni
>   Changed the BASE _NAME of AmdSmmCpuFeaturesLib.
>   Removed EFIAPI from local function.
>   Removed CpuIndex parameter from MmSaveStateGetRegisterLma
>   Modifed MmSaveStateGetRegisterIndex () to accept RegOffset
> as second parameter.
>   Removed FILE_GUID library instance for intel implemention from
> UefiCpuPkg.dsc.
> V12:
>   Addressed review comments from Michael.
>   Added LibraryClasses to .inf file.
>   removed duplicate MACRO definations.
>   Moved related MACRO defination to respective file.
> V11: Delta changes
>   Drop the OVMF implementation of MmSaveStateLib
> V10: Delta changes:
>   Addressed review comments from Abner.
> V9: Delta changes:
>   Addressed review comments.
>   Rename to MmSaveStateLib.
>   Also rename SMM_ defines to MM_.
>   Implemented OVMF MmSaveStateLib.
>   Removes SmmCpuFeaturesReadSaveStateRegister and
> SmmCpuFeaturesWriteSaveStateRegister
>   function interface.
> V8 delta changes:
>Addressed review comments from Abner,
>Fix the whitespace error.
>Seperate the Ovmf changes to another patch
> V7 delta changes:
>Adds SmmSmramSaveStateLib for Intel processor.
>Integrate SmmSmramSaveStateLib library.
> V6 delta changes:
>Addressed review comments for Ray NI.
>removed unnecessary EFIAPI.
> V5 delta changes:
>rebase to master branch.
>updated Reviewed-by
> V4 delta changes:
>   rebase to master branch.
>   added reviewed-by.
> V3 delta changes:
>   Addressed review comments from Abner chang.
>   Re-arranged patch order.
>
> Cc: Paul Grimes 
> Cc: Abner Chang 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Rahul 

[edk2-devel] [PATCH v2 4/4] OvmfPkg/RiscVVirt: Update README for CLANGDWARF support

2023-07-11 Thread Sunil V L
Update the README with instruction to build using CLANGDWARF
toolchain.

Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc: Gerd Hoffmann 
Cc: Andrei Warkentin 

Signed-off-by: Sunil V L 
Reviewed-by: Heinrich Schuchardt 
Acked-by: Ard Biesheuvel 
---
 OvmfPkg/RiscVVirt/README.md | 28 +---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/OvmfPkg/RiscVVirt/README.md b/OvmfPkg/RiscVVirt/README.md
index 950694419e8b..8c3ac37b802a 100644
--- a/OvmfPkg/RiscVVirt/README.md
+++ b/OvmfPkg/RiscVVirt/README.md
@@ -12,24 +12,46 @@ The minimum QEMU version required is
 
[7efd65423a](https://github.com/qemu/qemu/commit/7efd65423ab22e6f5890ca08ae40c84d6660242f)
 which supports separate pflash devices for EDK2 code and variable storage.
 
+## Get edk2 sources
+
+git clone --recurse-submodule g...@github.com:tianocore/edk2.git
+
 ## Build
+
+### Using GCC toolchain
+**Prerequisite**: RISC-V GNU compiler toolchain should be installed.
+
 export WORKSPACE=`pwd`
 export GCC5_RISCV64_PREFIX=riscv64-linux-gnu-
 export PACKAGES_PATH=$WORKSPACE/edk2
 export EDK_TOOLS_PATH=$WORKSPACE/edk2/BaseTools
-source edk2/edksetup.sh
+source edk2/edksetup.sh --reconfig
 make -C edk2/BaseTools
 source edk2/edksetup.sh BaseTools
 build -a RISCV64 --buildtarget RELEASE -p 
OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc -t GCC5
 
+### Using CLANGDWARF toolchain (clang + lld)
+**Prerequisite**: LLVM toolchain with clang and lld should be installed.
+
+export WORKSPACE=`pwd`
+export CLANGDWARF_BIN=/usr/bin/
+export PACKAGES_PATH=$WORKSPACE/edk2
+export EDK_TOOLS_PATH=$WORKSPACE/edk2/BaseTools
+source edk2/edksetup.sh --reconfig
+make -C edk2/BaseTools
+source edk2/edksetup.sh BaseTools
+build -a RISCV64 --buildtarget RELEASE -p 
OvmfPkg/RiscVVirt/RiscVVirtQemu.dsc -t CLANGDWARF
+
+After a successful build, two files namely **RISCV_VIRT_CODE.fd** and 
**RISCV_VIRT_VARS.fd** are created.
+
 ## Test
 Below example shows how to boot openSUSE Tumbleweed E20.
 
 1) RISC-V QEMU pflash devices should be of of size 32MiB.
 
-`truncate -s 32M Build/RiscVVirtQemu/RELEASE_GCC5/FV/RISCV_VIRT_CODE.fd`
+`truncate -s 32M RISCV_VIRT_CODE.fd`
 
-`truncate -s 32M Build/RiscVVirtQemu/RELEASE_GCC5/FV/RISCV_VIRT_VARS.fd`
+`truncate -s 32M RISCV_VIRT_VARS.fd`
 
 2) Running QEMU
 
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106817): https://edk2.groups.io/g/devel/message/106817
Mute This Topic: https://groups.io/mt/100081397/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH v2 2/4] OvmfPkg/RiscVVirt: SecEntry: Remove unnecessary assembly directives

2023-07-11 Thread Sunil V L
llvm fails to resolve _ModuleEntry when these extra directives are
present. ASM_FUNC already takes care what is required.

Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc: Gerd Hoffmann 
Cc: Andrei Warkentin 

Signed-off-by: Sunil V L 
Acked-by: Ard Biesheuvel 
---
 OvmfPkg/RiscVVirt/Sec/SecEntry.S | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/OvmfPkg/RiscVVirt/Sec/SecEntry.S b/OvmfPkg/RiscVVirt/Sec/SecEntry.S
index e919a3cb0e80..192fff321ce5 100644
--- a/OvmfPkg/RiscVVirt/Sec/SecEntry.S
+++ b/OvmfPkg/RiscVVirt/Sec/SecEntry.S
@@ -7,9 +7,6 @@
 
 #include "SecMain.h"
 
-.text
-.align 3
-
 ASM_FUNC (_ModuleEntryPoint)
   /* Use Temp memory as the stack for calling to C code */
   lia4, FixedPcdGet32 (PcdOvmfSecPeiTempRamBase)
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106815): https://edk2.groups.io/g/devel/message/106815
Mute This Topic: https://groups.io/mt/100081393/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH v2 3/4] BaseTools/tools_def: Add CLANGDWARF support for RISC-V

2023-07-11 Thread Sunil V L
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4478

Add tools_def definitions to support CLANGDWARF toolchain
for RISC-V. This uses clang and the llvm LLD linker. This
helps people by not requiring to install multiple
cross compilers for different architectures.

Cc: Rebecca Cran 
Cc: Liming Gao 
Cc: Bob Feng 
Cc: Yuwei Chen 
Cc: Ard Biesheuvel 

Signed-off-by: Sunil V L 
Acked-by: Ard Biesheuvel 
---
 BaseTools/Conf/tools_def.template | 53 +++
 1 file changed, 53 insertions(+)

diff --git a/BaseTools/Conf/tools_def.template 
b/BaseTools/Conf/tools_def.template
index 90f4105506e5..1bf62362b611 100755
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -745,6 +745,7 @@ DEFINE GCC_LOONGARCH64_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) 
-mabi=lp64d -fno-asyn
 DEFINE GCC_ARM_CC_XIPFLAGS = -mno-unaligned-access
 DEFINE GCC_AARCH64_CC_FLAGS= DEF(GCC_ALL_CC_FLAGS) -mlittle-endian 
-fno-short-enums -fverbose-asm -funsigned-char  -ffunction-sections 
-fdata-sections -Wno-address -fno-asynchronous-unwind-tables -fno-unwind-tables 
-fno-pic -fno-pie -ffixed-x18
 DEFINE GCC_AARCH64_CC_XIPFLAGS = -mstrict-align -mgeneral-regs-only
+DEFINE GCC_RISCV64_CC_XIPFLAGS = -mstrict-align -mgeneral-regs-only
 DEFINE GCC_DLINK_FLAGS_COMMON  = -nostdlib --pie
 DEFINE GCC_DLINK2_FLAGS_COMMON = 
-Wl,--script=$(EDK_TOOLS_PATH)/Scripts/GccBase.lds
 DEFINE GCC_IA32_X64_DLINK_COMMON   = DEF(GCC_DLINK_FLAGS_COMMON) --gc-sections
@@ -2023,6 +2024,58 @@ DEFINE CLANGDWARF_AARCH64_DLINK_FLAGS  = 
DEF(CLANGDWARF_AARCH64_TARGET) DEF(GCC_
 RELEASE_CLANGDWARF_AARCH64_CC_FLAGS= DEF(CLANGDWARF_AARCH64_CC_FLAGS) 
$(PLATFORM_FLAGS) -flto -O3
 RELEASE_CLANGDWARF_AARCH64_DLINK_FLAGS = DEF(CLANGDWARF_AARCH64_DLINK_FLAGS) 
-flto -Wl,-O3 -fuse-ld=lld -L$(WORKSPACE)/ArmPkg/Library/GccLto -llto-aarch64 
-Wl,-plugin-opt=-pass-through=-llto-aarch64 -Wl,--no-pie,--no-relax
 
+##
+# CLANGDWARF RISCV64 definitions
+##
+DEFINE CLANGDWARF_RISCV64_TARGET= -target riscv64-linux-gnu
+DEFINE CLANGDWARF_RISCV64_CC_COMMON = DEF(GCC5_RISCV_ALL_CC_FLAGS) 
DEF(GCC5_RISCV_ALL_CC_FLAGS_WARNING_DISABLE) DEF(GCC5_RISCV_OPENSBI_TYPES) 
-march=DEF(GCC5_RISCV64_ARCH) -fno-builtin -fno-builtin-memcpy 
-fno-stack-protector -Wno-address -fno-asynchronous-unwind-tables 
-fno-unwind-tables -Wno-unused-but-set-variable -fpack-struct=8 -mcmodel=medany 
-mabi=lp64 -mno-relax
+DEFINE CLANGDWARF_RISCV64_CC_FLAGS  = DEF(CLANGDWARF_RISCV64_CC_COMMON) 
DEF(CLANGDWARF_RISCV64_TARGET) DEF(CLANGDWARF_WARNING_OVERRIDES)
+
+# This is similar to GCC flags but without -n
+DEFINE CLANGDWARF_RISCV64_ALL_DLINK_COMMON  = -nostdlib -Wl,-q,--gc-sections 
-z common-page-size=0x40
+DEFINE CLANGDWARF_RISCV64_ALL_DLINK_FLAGS   = 
DEF(CLANGDWARF_RISCV64_ALL_DLINK_COMMON) -Wl,--entry,$(IMAGE_ENTRY_POINT) -u 
$(IMAGE_ENTRY_POINT) -Wl,-Map,$(DEST_DIR_DEBUG)/$(BASE_NAME).map
+DEFINE CLANGDWARF_RISCV64_DLINK_FLAGS   = DEF(CLANGDWARF_RISCV64_TARGET) 
DEF(CLANGDWARF_RISCV64_ALL_DLINK_FLAGS) 
-Wl,-melf64lriscv,--oformat=elf64-littleriscv,--no-relax
+
+*_CLANGDWARF_RISCV64_PP_FLAGS   = DEF(GCC_PP_FLAGS)
+*_CLANGDWARF_RISCV64_ASLCC_FLAGS= DEF(GCC_ASLCC_FLAGS)
+*_CLANGDWARF_RISCV64_APP_FLAGS  =
+*_CLANGDWARF_RISCV64_ASL_FLAGS  = DEF(IASL_FLAGS)
+*_CLANGDWARF_RISCV64_ASL_OUTFLAGS   = DEF(IASL_OUTFLAGS)
+*_CLANGDWARF_RISCV64_DTCPP_FLAGS= DEF(GCC_DTCPP_FLAGS)
+*_CLANGDWARF_RISCV64_DEPS_FLAGS = DEF(GCC_DEPS_FLAGS)
+
+*_CLANGDWARF_RISCV64_CC_PATH= ENV(CLANGDWARF_BIN)clang
+*_CLANGDWARF_RISCV64_ASM_PATH   = ENV(CLANGDWARF_BIN)clang
+*_CLANGDWARF_RISCV64_PP_PATH= ENV(CLANGDWARF_BIN)clang
+*_CLANGDWARF_RISCV64_VFRPP_PATH = ENV(CLANGDWARF_BIN)clang
+*_CLANGDWARF_RISCV64_ASLCC_PATH = ENV(CLANGDWARF_BIN)clang
+*_CLANGDWARF_RISCV64_ASLPP_PATH = ENV(CLANGDWARF_BIN)clang
+*_CLANGDWARF_RISCV64_DLINK_PATH = ENV(CLANGDWARF_BIN)clang
+*_CLANGDWARF_RISCV64_ASLDLINK_PATH  = ENV(CLANGDWARF_BIN)clang
+
+*_CLANGDWARF_RISCV64_SLINK_PATH = ENV(CLANGDWARF_BIN)llvm-ar
+*_CLANGDWARF_RISCV64_RC_PATH= ENV(CLANGDWARF_BIN)llvm-objcopy
+
+*_CLANGDWARF_RISCV64_ASLCC_FLAGS= DEF(GCC_ASLCC_FLAGS) -fno-lto
+*_CLANGDWARF_RISCV64_ASLDLINK_FLAGS = DEF(CLANGDWARF_RISCV64_TARGET) 
DEF(GCC5_RISCV32_RISCV64_ASLDLINK_FLAGS)
+*_CLANGDWARF_RISCV64_ASM_FLAGS  = DEF(GCC_ASM_FLAGS) 
DEF(CLANGDWARF_RISCV64_TARGET) $(PLATFORM_FLAGS) -Qunused-arguments -mabi=lp64 
-mno-relax
+*_CLANGDWARF_RISCV64_DLINK_FLAGS= DEF(CLANGDWARF_RISCV64_TARGET) 
DEF(GCC5_RISCV64_DLINK_FLAGS)
+*_CLANGDWARF_RISCV64_DLINK_XIPFLAGS = -z common-page-size=0x20
+*_CLANGDWARF_RISCV64_DLINK2_FLAGS   = DEF(GCC_DLINK2_FLAGS_COMMON) 
-Wl,--defsym=PECOFF_HEADER_SIZE=0x240
+*_CLANGDWARF_RISCV64_PLATFORM_FLAGS =
+*_CLANGDWARF_RISCV64_PP_FLAGS   = DEF(GCC_PP_FLAGS) 
DEF(CLANGDWARF_RISCV64_TARGET) $(PLATFORM_FLAGS)
+*_CLANGDWARF_RISCV64_RC_FLAGS   = DEF(GCC_RISCV64_RC_FLAGS)

[edk2-devel] [PATCH v2 1/4] OvmfPkg/RiscVVirt: use 'auto' alignment and FIXED for XIP modules

2023-07-11 Thread Sunil V L
Use auto alignment and FIXED FFS attribute for XIP modules similar
to [1]. Without this change, the CLANGDWARF toolchain will fail to
build with below error.

GenFfs: ERROR 1000: Unknown option
  SectionAlign option must be specified with section file.

[1] - 
https://github.com/tianocore/edk2/commit/7669f7349829f0e472ba0d6e600492fd8170

Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc  Gerd Hoffmann 
Cc: Andrei Warkentin 

Signed-off-by: Sunil V L 
Acked-by: Ard Biesheuvel 
---
 OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf | 34 +
 1 file changed, 10 insertions(+), 24 deletions(-)

diff --git a/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf 
b/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf
index 21e4ba67379f..b5aebfae078c 100644
--- a/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf
+++ b/OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf
@@ -240,30 +240,15 @@ [FV.FVMAIN_COMPACT]
  }
 
 [Rule.Common.SEC]
-  FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED {
-PE32 PE32   Align=4K $(INF_OUTPUT)/$(MODULE_NAME).efi
+  FILE SEC = $(NAMED_GUID) RELOCS_STRIPPED FIXED {
+PE32 PE32Align = Auto $(INF_OUTPUT)/$(MODULE_NAME).efi
 UI   STRING ="$(MODULE_NAME)" Optional
 VERSION  STRING ="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
   }
 
-[Rule.Common.PEI_CORE]
-  FILE PEI_CORE = $(NAMED_GUID) {
-PE32 PE32   Align=4K$(INF_OUTPUT)/$(MODULE_NAME).efi
-UI   STRING ="$(MODULE_NAME)" Optional
-VERSION  STRING ="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
-  }
-
-[Rule.Common.PEIM]
-  FILE PEIM = $(NAMED_GUID) {
- PEI_DEPEX PEI_DEPEX Optional$(INF_OUTPUT)/$(MODULE_NAME).depex
- PE32 PE32   Align=4K $(INF_OUTPUT)/$(MODULE_NAME).efi
- UI   STRING="$(MODULE_NAME)" Optional
- VERSION  STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
-  }
-
 [Rule.Common.DXE_CORE]
   FILE DXE_CORE = $(NAMED_GUID) {
-PE32 PE32   Align=4K  $(INF_OUTPUT)/$(MODULE_NAME).efi
+PE32 PE32$(INF_OUTPUT)/$(MODULE_NAME).efi
 UI   STRING="$(MODULE_NAME)" Optional
 VERSION  STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
   }
@@ -271,7 +256,7 @@ [Rule.Common.DXE_CORE]
 [Rule.Common.DXE_DRIVER]
   FILE DRIVER = $(NAMED_GUID) {
 DXE_DEPEXDXE_DEPEX Optional  $(INF_OUTPUT)/$(MODULE_NAME).depex
-PE32 PE32   Align=4K  $(INF_OUTPUT)/$(MODULE_NAME).efi
+PE32 PE32$(INF_OUTPUT)/$(MODULE_NAME).efi
 UI   STRING="$(MODULE_NAME)" Optional
 VERSION  STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
   }
@@ -279,7 +264,7 @@ [Rule.Common.DXE_DRIVER]
 [Rule.Common.DXE_RUNTIME_DRIVER]
   FILE DRIVER = $(NAMED_GUID) {
 DXE_DEPEXDXE_DEPEX Optional  $(INF_OUTPUT)/$(MODULE_NAME).depex
-PE32 PE32   Align = 4K$(INF_OUTPUT)/$(MODULE_NAME).efi
+PE32 PE32$(INF_OUTPUT)/$(MODULE_NAME).efi
 UI   STRING="$(MODULE_NAME)" Optional
 VERSION  STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
   }
@@ -287,7 +272,7 @@ [Rule.Common.DXE_RUNTIME_DRIVER]
 [Rule.Common.UEFI_DRIVER]
   FILE DRIVER = $(NAMED_GUID) {
 DXE_DEPEXDXE_DEPEX Optional  $(INF_OUTPUT)/$(MODULE_NAME).depex
-PE32 PE32  Align=4K  $(INF_OUTPUT)/$(MODULE_NAME).efi
+PE32 PE32   $(INF_OUTPUT)/$(MODULE_NAME).efi
 UI   STRING="$(MODULE_NAME)" Optional
 VERSION  STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
   }
@@ -295,21 +280,21 @@ [Rule.Common.UEFI_DRIVER]
 [Rule.Common.UEFI_DRIVER.BINARY]
   FILE DRIVER = $(NAMED_GUID) {
 DXE_DEPEX DXE_DEPEX Optional  |.depex
-PE32  PE32   Align=4K   |.efi
+PE32  PE32|.efi
 UISTRING="$(MODULE_NAME)" Optional
 VERSION   STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
   }
 
 [Rule.Common.UEFI_APPLICATION]
   FILE APPLICATION = $(NAMED_GUID) {
-PE32 PE32   Align=4K   $(INF_OUTPUT)/$(MODULE_NAME).efi
+PE32 PE32$(INF_OUTPUT)/$(MODULE_NAME).efi
 UI   STRING="$(MODULE_NAME)" Optional
 VERSION  STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
   }
 
 [Rule.Common.UEFI_APPLICATION.BINARY]
   FILE APPLICATION = $(NAMED_GUID) {
-PE32  PE32  Align=4K  |.efi
+PE32  PE32|.efi
 UISTRING="$(MODULE_NAME)" Optional
 VERSION   STRING="$(INF_VERSION)" Optional BUILD_NUM=$(BUILD_NUMBER)
   }
@@ -318,4 +303,5 @@ [Rule.Common.USER_DEFINED.ACPITABLE]
   FILE FREEFORM = $(NAMED_GUID) {
 RAW ACPI   |.acpi
 RAW ASL|.aml
+UISTRING="$(MODULE_NAME)" Optional
   }
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106814): https://edk2.groups.io/g/devel/message/106814
Mute This Topic: https://groups.io/mt/100081391/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]

[edk2-devel] [PATCH v2 0/4] OvmfPkg/RiscVVirt: Add CLANGDWARF toolchain support

2023-07-11 Thread Sunil V L
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4478

This series adds support for building RiscVVirtQemu EDK2 using
CLANGDWARF toolchain. Adding this support helps people to use
the same toolchain to build EDK2 for different architectures.

Cc: Rebecca Cran 
Cc: Liming Gao 
Cc: Bob Feng 
Cc: Yuwei Chen 
Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc  Gerd Hoffmann 
Cc: Andrei Warkentin 
Cc: Heinrich Schuchardt 

Changes since v1:
1) Addressed Liming's comments in PATCH 3.

Sunil V L (4):
  OvmfPkg/RiscVVirt: use 'auto' alignment and FIXED for XIP modules
  OvmfPkg/RiscVVirt: SecEntry: Remove unnecessary assembly directives
  BaseTools/tools_def: Add CLANGDWARF support for RISC-V
  OvmfPkg/RiscVVirt: Update README for CLANGDWARF support

 OvmfPkg/RiscVVirt/RiscVVirtQemu.fdf | 34 ++
 BaseTools/Conf/tools_def.template   | 53 +
 OvmfPkg/RiscVVirt/README.md | 28 +--
 OvmfPkg/RiscVVirt/Sec/SecEntry.S|  3 --
 4 files changed, 88 insertions(+), 30 deletions(-)

-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106813): https://edk2.groups.io/g/devel/message/106813
Mute This Topic: https://groups.io/mt/100081389/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [edk2-platforms][PATCH V1 19/20] ArmPkg/MmCommunicationDxe: Use the FF-A transport for MM requests

2023-07-11 Thread Nishant Sharma
From: Achin Gupta 

This patch packages requests for accessing a Standalone MM driver
through the MM communication protocol as FF-A direct messages.
Corresponding changes in Standalone MM Core ensure that responses are
packaged in the same way.

Signed-off-by: Achin Gupta 
Co-developed-by: Aditya Angadi 
Signed-off-by: Nishant Sharma 
---
 ArmPkg/Include/IndustryStandard/ArmFfaSvc.h |   2 +
 ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 141 +---
 2 files changed, 97 insertions(+), 46 deletions(-)

diff --git a/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h 
b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
index 530af8bd3c2e..493997346143 100644
--- a/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
+++ b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
@@ -23,6 +23,7 @@
 #define ARM_SVC_ID_FFA_RXTX_UNMAP_AARCH320x8467
 #define ARM_SVC_ID_FFA_PARTITION_INFO_GET_AARCH320x8468
 #define ARM_SVC_ID_FFA_ID_GET_AARCH320x8469
+#define ARM_SVC_ID_FFA_RUN_AARCH32   0x846D
 #define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH32   0x846F
 #define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP_AARCH32  0x8470
 #define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH64   0xC46F
@@ -31,6 +32,7 @@
 #define ARM_SVC_ID_FFA_SUCCESS_AARCH64   0xC461
 #define ARM_SVC_ID_FFA_MEM_PERM_SET_AARCH32  0x8489
 #define ARM_SVC_ID_FFA_MEM_PERM_GET_AARCH32  0x8488
+#define ARM_SVC_ID_FFA_INTERRUPT_AARCH32 0x8462
 #define ARM_SVC_ID_FFA_ERROR_AARCH32 0x8460
 #define ARM_SVC_ID_FFA_ERROR_AARCH64 0xC460
 #define ARM_SVC_ID_FFA_MSG_WAIT_AARCH32  0x846B
diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c 
b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
index 94a5d96c051d..a70318581bd2 100644
--- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
@@ -100,6 +100,7 @@ MmCommunication2Communicate (
   ARM_SMC_ARGS   CommunicateSmcArgs;
   EFI_STATUS Status;
   UINTN  BufferSize;
+  UINTN  Ret;
 
   Status = EFI_ACCESS_DENIED;
   BufferSize = 0;
@@ -160,60 +161,108 @@ MmCommunication2Communicate (
 return Status;
   }
 
-  // SMC Function ID
-  CommunicateSmcArgs.Arg0 = ARM_SMC_ID_MM_COMMUNICATE_AARCH64;
-
-  // Cookie
-  CommunicateSmcArgs.Arg1 = 0;
-
   // Copy Communication Payload
   CopyMem ((VOID *)mNsCommBuffMemRegion.VirtualBase, CommBufferVirtual, 
BufferSize);
 
-  // comm_buffer_address (64-bit physical address)
-  CommunicateSmcArgs.Arg2 = (UINTN)mNsCommBuffMemRegion.PhysicalBase;
+  // Use the FF-A interface if enabled.
+  if (FixedPcdGet32 (PcdFfaEnable) != 0) {
+// FF-A Interface ID for direct message communication
+CommunicateSmcArgs.Arg0 = ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH64;
 
-  // comm_size_address (not used, indicated by setting to zero)
-  CommunicateSmcArgs.Arg3 = 0;
+// FF-A Destination EndPoint ID, not used as of now
+CommunicateSmcArgs.Arg1 = mFfaPartId << 16 | mStmmPartInfo.PartId;
 
+// Reserved for future use(MBZ)
+CommunicateSmcArgs.Arg2 = 0x0;
+
+// comm_buffer_address (64-bit physical address)
+CommunicateSmcArgs.Arg3 = (UINTN)mNsCommBuffMemRegion.PhysicalBase;
+
+// Cookie
+CommunicateSmcArgs.Arg4 = 0x0;
+
+// Not Used
+CommunicateSmcArgs.Arg5 = 0;
+
+// comm_size_address (not used, indicated by setting to zero)
+CommunicateSmcArgs.Arg6 = 0;
+  } else {
+// SMC Function ID
+CommunicateSmcArgs.Arg0 = ARM_SMC_ID_MM_COMMUNICATE_AARCH64;
+
+// Cookie
+CommunicateSmcArgs.Arg1 = 0;
+
+// comm_buffer_address (64-bit physical address)
+CommunicateSmcArgs.Arg2 = (UINTN)mNsCommBuffMemRegion.PhysicalBase;
+
+// comm_size_address (not used, indicated by setting to zero)
+CommunicateSmcArgs.Arg3 = 0;
+  }
+
+ffa_intr_loop:
   // Call the Standalone MM environment.
   ArmCallSmc ();
 
-  switch (CommunicateSmcArgs.Arg0) {
-case ARM_SMC_MM_RET_SUCCESS:
-  ZeroMem (CommBufferVirtual, BufferSize);
-  // On successful return, the size of data being returned is inferred from
-  // MessageLength + Header.
-  CommunicateHeader = (EFI_MM_COMMUNICATE_HEADER 
*)mNsCommBuffMemRegion.VirtualBase;
-  BufferSize= CommunicateHeader->MessageLength +
-  sizeof (CommunicateHeader->HeaderGuid) +
-  sizeof (CommunicateHeader->MessageLength);
-
-  CopyMem (
-CommBufferVirtual,
-(VOID *)mNsCommBuffMemRegion.VirtualBase,
-BufferSize
-);
-  Status = EFI_SUCCESS;
-  break;
-
-case ARM_SMC_MM_RET_INVALID_PARAMS:
-  Status = EFI_INVALID_PARAMETER;
-  break;
-
-case ARM_SMC_MM_RET_DENIED:
-  Status = EFI_ACCESS_DENIED;
-  break;
-
-case ARM_SMC_MM_RET_NO_MEMORY:
-  // 

[edk2-devel] [edk2-platforms][PATCH V1 20/20] StandaloneMmPkg: Add support for MM requests as FF-A direct messages

2023-07-11 Thread Nishant Sharma
From: Achin Gupta 

This patch adds support for correctly receiving a request for a
Standalome MM driver service using the MM communication protocol
packaged as an FF-A direct request message.

Signed-off-by: Achin Gupta 
Signed-off-by: Nishant Sharma 
---
 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
 | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
index a763bf8509b2..e0987cba21f9 100644
--- 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
+++ 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
@@ -198,6 +198,8 @@ DelegatedEventLoop (
   BOOLEAN FfaEnabled;
   EFI_STATUS  Status;
   UINTN   SvcStatus;
+  UINT16  SenderPartId;
+  UINT16  ReceiverPartId;
 
   while (TRUE) {
 ArmCallSvc (EventCompleteSvcArgs);
@@ -214,9 +216,12 @@ DelegatedEventLoop (
 
 FfaEnabled = FixedPcdGet32 (PcdFfaEnable != 0);
 if (FfaEnabled) {
+  SenderPartId = EventCompleteSvcArgs->Arg1 >> 16;
+  ReceiverPartId = EventCompleteSvcArgs->Arg1 & 0x;
   Status = CpuDriverEntryPoint (
  EventCompleteSvcArgs->Arg0,
- EventCompleteSvcArgs->Arg6,
+ // Assume CPU number 0
+ 0,
  EventCompleteSvcArgs->Arg3
  );
   if (EFI_ERROR (Status)) {
@@ -266,7 +271,7 @@ DelegatedEventLoop (
 
 if (FfaEnabled) {
   EventCompleteSvcArgs->Arg0 = ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP;
-  EventCompleteSvcArgs->Arg1 = 0;
+  EventCompleteSvcArgs->Arg1 = ReceiverPartId << 16 | SenderPartId;
   EventCompleteSvcArgs->Arg2 = 0;
   EventCompleteSvcArgs->Arg3 = ARM_SVC_ID_SP_EVENT_COMPLETE;
   EventCompleteSvcArgs->Arg4 = SvcStatus;
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106812): https://edk2.groups.io/g/devel/message/106812
Mute This Topic: https://groups.io/mt/100079894/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [edk2-platforms][PATCH V1 10/20] StandaloneMmPkg: Populate Hoblist for SP init from StMM boot information

2023-07-11 Thread Nishant Sharma
From: Achin Gupta 

This patch adds support for creating a hoblist from the reduced boot
information retrieved from the SP manifest.

Signed-off-by: Achin Gupta 
Signed-off-by: Nishant Sharma 
---
 StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h   
 |  16 ++
 StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c 
 | 186 +++-
 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
 |   6 +-
 3 files changed, 206 insertions(+), 2 deletions(-)

diff --git a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h 
b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
index 90d67a2f25b5..9daa76324221 100644
--- a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
+++ b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
@@ -170,6 +170,22 @@ CreateHobListFromBootInfo (
   IN   EFI_SECURE_PARTITION_BOOT_INFO  *PayloadBootInfo
   );
 
+/**
+  Use the boot information passed by the SPMC to populate a HOB list
+  suitable for consumption by the MM Core and drivers.
+
+  @param  [in, out] CpuDriverEntryPoint   Address of MM CPU driver entrypoint
+  @param  [in]  StmmBootInfo  Boot information passed by privileged
+  firmware
+
+**/
+VOID *
+EFIAPI
+CreateHobListFromStmmBootInfo (
+  IN  OUT  PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT *CpuDriverEntryPoint,
+  IN   EFI_STMM_BOOT_INFO *StmmBootInfo
+  );
+
 /**
   The entry point of Standalone MM Foundation.
 
diff --git 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
index 2ac2d354f06a..4592089a6020 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c
@@ -2,7 +2,7 @@
   Creates HOB during Standalone MM Foundation entry point
   on ARM platforms.
 
-Copyright (c) 2017 - 2021, Arm Ltd. All rights reserved.
+Copyright (c) 2017 - 2023, Arm Ltd. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -203,3 +203,187 @@ CreateHobListFromBootInfo (
 
   return HobStart;
 }
+
+STATIC
+VOID
+CreateMmramInformationHobFromImageLayout (
+  IN   EFI_STMM_BOOT_INFO  *StmmBootInfo,
+  IN   EFI_HOB_HANDOFF_INFO_TABLE  *HobStart
+)
+{
+  UINT32  *Idx;
+  UINT32  BufferSize;
+  EFI_MMRAM_HOB_DESCRIPTOR_BLOCK  *MmramRangesHob;
+  EFI_MMRAM_DESCRIPTOR*MmramRanges;
+
+  // Find the size of the GUIDed HOB with SRAM ranges. This excludes any memory
+  // shared with the normal world or the SPMC. It includes the memory allocated
+  // to the SP image, used and unused heap.
+  BufferSize = sizeof (EFI_MMRAM_HOB_DESCRIPTOR_BLOCK);
+  BufferSize += 4 * sizeof (EFI_MMRAM_DESCRIPTOR);
+
+  // Create a GUIDed HOB with SRAM ranges
+  MmramRangesHob = BuildGuidHob (, BufferSize);
+
+  // Initialise the number of MMRAM memory regions
+  MmramRangesHob->NumberOfMmReservedRegions = 0;
+  Idx = >NumberOfMmReservedRegions ;
+
+  // Fill up the MMRAM ranges
+  MmramRanges = >Descriptor[0];
+
+  // Base and size of memory occupied by the Standalone MM image
+  MmramRanges[*Idx].PhysicalStart = StmmBootInfo->SpMemBase;
+  MmramRanges[*Idx].CpuStart  = StmmBootInfo->SpMemBase;
+  MmramRanges[*Idx].PhysicalSize  = StmmBootInfo->SpMemSize;
+  MmramRanges[*Idx].RegionState   = EFI_CACHEABLE | EFI_ALLOCATED;
+  (*Idx)++;
+
+  // Base and size of memory occupied by the Standalone MM image
+  MmramRanges[*Idx].PhysicalStart = StmmBootInfo->SpSharedBufBase;
+  MmramRanges[*Idx].CpuStart  = StmmBootInfo->SpSharedBufBase;
+  MmramRanges[*Idx].PhysicalSize  = StmmBootInfo->SpSharedBufSize;
+  MmramRanges[*Idx].RegionState   = EFI_CACHEABLE | EFI_ALLOCATED;
+  (*Idx)++;
+
+  // Base and size of memory occupied by the hoblist
+  MmramRanges[*Idx].PhysicalStart = (EFI_PHYSICAL_ADDRESS) (UINTN) HobStart;
+  MmramRanges[*Idx].CpuStart  = (EFI_PHYSICAL_ADDRESS) (UINTN) HobStart;
+  MmramRanges[*Idx].PhysicalSize  = HobStart->EfiFreeMemoryBottom - 
(EFI_PHYSICAL_ADDRESS) (UINTN) HobStart;
+  MmramRanges[*Idx].RegionState   = EFI_CACHEABLE | EFI_ALLOCATED;
+  (*Idx)++;
+
+  // Base and size of heap memory shared by all cpus
+  MmramRanges[*Idx].PhysicalStart = HobStart->EfiFreeMemoryBottom;
+  MmramRanges[*Idx].CpuStart  = HobStart->EfiFreeMemoryBottom;
+  MmramRanges[*Idx].PhysicalSize  = HobStart->EfiFreeMemoryTop - 
HobStart->EfiFreeMemoryBottom;
+  MmramRanges[*Idx].RegionState   = EFI_CACHEABLE;
+  (*Idx)++;
+
+  // Sanity check number of MMRAM regions
+  ASSERT (MmramRangesHob->NumberOfMmReservedRegions == 3);
+
+  return;
+}
+
+STATIC
+VOID
+CreateMpInformationHobFromCpuInfo (
+  IN   EFI_SECURE_PARTITION_CPU_INFO *CpuInfo
+)
+{
+  MP_INFORMATION_HOB_DATA 

[edk2-devel] [edk2-platforms][PATCH V1 18/20] ArmPkg/MmCommunicationDxe: Discover the StMM SP

2023-07-11 Thread Nishant Sharma
From: Achin Gupta 

This patch adds support for discovering the presence of the SP using the
EFI_MM_COMMUNICATION_PROTOCOL GUID that implements Standalone MM
drivers. This is done by querying the framework through
FFA_PARTITION_INFO_GET whether any partition that implements the
EFI_MM_COMMUNICATION_PROTOCOL is present or not. The partition ID and
its properties are stashed for use in subsequent communication with the
StMM SP.

Signed-off-by: Achin Gupta 
Signed-off-by: Nishant Sharma 
---
 ArmPkg/Include/IndustryStandard/ArmFfaSvc.h | 24 +
 ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 93 +++-
 2 files changed, 114 insertions(+), 3 deletions(-)

diff --git a/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h 
b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
index f78442a465e1..530af8bd3c2e 100644
--- a/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
+++ b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
@@ -19,6 +19,9 @@
 #define ARM_SVC_ID_FFA_VERSION_AARCH32   0x8463
 #define ARM_SVC_ID_FFA_RXTX_MAP_AARCH32  0x8466
 #define ARM_SVC_ID_FFA_RXTX_MAP_AARCH64  0xC466
+#define ARM_SVC_ID_FFA_RX_RELEASE_AARCH320x8465
+#define ARM_SVC_ID_FFA_RXTX_UNMAP_AARCH320x8467
+#define ARM_SVC_ID_FFA_PARTITION_INFO_GET_AARCH320x8468
 #define ARM_SVC_ID_FFA_ID_GET_AARCH320x8469
 #define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH32   0x846F
 #define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP_AARCH32  0x8470
@@ -154,4 +157,25 @@ typedef struct {
   UINT64 Reserved;
 } EFI_FFA_BOOT_INFO_HEADER;
 
+// FF-A partition information descriptor
+typedef struct {
+  UINT16 PartId;
+  UINT16 EcCnt;
+  UINT32 PartProps;
+  UINT32 PartGuid[4];
+} EFI_FFA_PART_INFO_DESC;
+
+#define PART_INFO_PROP_MASK0x3f
+#define PART_INFO_PROP_SHIFT   0
+#define PART_INFO_PROP_DIR_MSG_RECV_BIT(1u << 0)
+#define PART_INFO_PROP_DIR_MSG_SEND_BIT(1u << 1)
+#define PART_INFO_PROP_INDIR_MSG_BIT   (1u << 2)
+#define PART_INFO_PROP_NOTIFICATIONS_BIT   (1u << 3)
+#define PART_INFO_PROP_EP_TYPE_MASK0x3
+#define PART_INFO_PROP_EP_TYPE_SHIFT   4
+#define PART_INFO_PROP_EP_PE   0
+#define PART_INFO_PROP_EP_SEPID_IND1
+#define PART_INFO_PROP_EP_SEPID_DEP2
+#define PART_INFO_PROP_EP_AUX  3
+
 #endif // ARM_FFA_SVC_H_
diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c 
b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
index 39a1b329b9ea..94a5d96c051d 100644
--- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
@@ -8,6 +8,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -28,6 +29,11 @@
 //
 STATIC UINT16  mFfaPartId;
 
+// Partition information of the StMM SP if FF-A support is enabled
+// TODO: Revisit assumption that there is only a single StMM SP
+//
+STATIC EFI_FFA_PART_INFO_DESC mStmmPartInfo;
+
 //
 // RX/TX pair if FF-A support is enabled
 //
@@ -298,7 +304,9 @@ GetMmCompatibility (
 {
   EFI_STATUSStatus;
   UINT32MmVersion;
+  UINT32SmccUuid[4];
   ARM_SMC_ARGS  SmcArgs = {0};
+  EFI_GUID  MmCommProtGuid = EFI_MM_COMMUNICATION_PROTOCOL_GUID;
 
   if (FixedPcdGet32 (PcdFfaEnable) != 0) {
 SmcArgs.Arg0 = ARM_SVC_ID_FFA_VERSION_AARCH32;
@@ -335,14 +343,21 @@ GetMmCompatibility (
 Status = EFI_UNSUPPORTED;
   }
 
-  // If FF-A is supported then discover our ID and register our RX/TX buffers.
+  // If FF-A is supported then discover the StMM SP's presence, ID, our ID and
+  // register our RX/TX buffers.
   if (FixedPcdGet32 (PcdFfaEnable) != 0) {
+EFI_FFA_PART_INFO_DESC *StmmPartInfo;
+
 // Get our ID
 ZeroMem(, sizeof(SmcArgs));
 SmcArgs.Arg0 = ARM_SVC_ID_FFA_ID_GET_AARCH32;
 ArmCallSmc ();
 if (SmcArgs.Arg0 == ARM_SVC_ID_FFA_ERROR_AARCH32) {
-  DEBUG ((DEBUG_ERROR, "Unable to retrieve FF-A partition ID (%d).\n", 
SmcArgs.Arg2));
+  DEBUG ((
+DEBUG_ERROR,
+"Unable to retrieve FF-A partition ID (%d).\n",
+SmcArgs.Arg2
+));
   return EFI_UNSUPPORTED;
 }
 DEBUG ((DEBUG_INFO, "FF-A partition ID = 0x%lx.\n", SmcArgs.Arg2));
@@ -355,11 +370,83 @@ GetMmCompatibility (
 SmcArgs.Arg3 = EFI_PAGE_SIZE / SIZE_4KB;
 ArmCallSmc ();
 if (SmcArgs.Arg0 == ARM_SVC_ID_FFA_ERROR_AARCH32) {
-  DEBUG ((DEBUG_ERROR, "Unable to register FF-A RX/TX buffers (%d).\n", 
SmcArgs.Arg2));
+  DEBUG ((
+DEBUG_ERROR,
+"Unable to register FF-A RX/TX buffers (%d).\n",
+SmcArgs.Arg2
+));
   return EFI_UNSUPPORTED;
 }
 
+// Discover the StMM SP after converting the EFI_GUID to a format TF-A will
+// understand.
+SmcArgs.Arg0 = ARM_SVC_ID_FFA_PARTITION_INFO_GET_AARCH32;
+MmCommProtGuid.Data2 += MmCommProtGuid.Data3;
+MmCommProtGuid.Data3 = 

[edk2-devel] [edk2-platforms][PATCH V1 17/20] ArmPkg/MmCommunicationDxe: Unmap FF-A RX/TX buffers during ExitBootServices

2023-07-11 Thread Nishant Sharma
From: Achin Gupta 

An FF-A partition can map only a single RX/TX buffer pair with the
framework. The DXE MM communication driver maps its pair before
ExitBootServices is called. The OS cannot re-use this pair once it boots
subsequently and loads its own FF-A driver. This patch ensures that the
DXE MM communication driver unmaps its buffer pair when ExitBootServices
is called so that the OS can register its own pair if required.

Signed-off-by: Achin Gupta 
Signed-off-by: Nishant Sharma 
---
 ArmPkg/Include/IndustryStandard/ArmFfaSvc.h | 10 
 ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 49 
 2 files changed, 59 insertions(+)

diff --git a/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h 
b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
index ebdf29e8d69a..f78442a465e1 100644
--- a/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
+++ b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
@@ -122,6 +122,16 @@
   (((content) & FFA_BOOT_INFO_FLAG_CONTENT_MASK)  \
<< FFA_BOOT_INFO_FLAG_CONTENT_SHIFT)
 
+/* Fromat SP ID info. */
+#define FFA_PARTITION_ID_SHIFT  16
+#define FFA_PARTITION_ID_WIDTH  16
+#define FFA_PARTITION_ID_MASK   \
+  (((1U << FFA_PARTITION_ID_WIDTH) - 1) \
+   << FFA_PARTITION_ID_SHIFT)
+#define FFA_PARTITION_ID(partid)\
+  ((partid << FFA_PARTITION_ID_SHIFT) & \
+   FFA_PARTITION_ID_MASK)
+
 // Descriptor to pass boot information as per the FF-A v1.1 spec.
 typedef struct {
   UINT32 Name[4];
diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c 
b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
index 8a4d46e4f80a..39a1b329b9ea 100644
--- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
@@ -43,6 +43,9 @@ STATIC ARM_MEMORY_REGION_DESCRIPTOR  mNsCommBuffMemRegion;
 // Notification event when virtual address map is set.
 STATIC EFI_EVENT  mSetVirtualAddressMapEvent;
 
+// Notification event when exit boot services is called.
+STATIC EFI_EVENT  mExitBootServicesEvent;
+
 //
 // Handle to install the MM Communication Protocol
 //
@@ -255,6 +258,39 @@ NotifySetVirtualAddressMap (
   }
 }
 
+/**
+  Notification callback on ExitBootServices event.
+
+  This function notifies the MM communication protocol interface on
+  ExitBootServices event and releases the FF-A RX/TX buffer.
+
+  @param  Event  ExitBootServices event.
+  @param  ContextA context when the ExitBootServices triggered.
+
+  @retval EFI_SUCCESSThe function executed successfully.
+  @retval Other  Some error occurred when executing this function.
+
+**/
+STATIC
+VOID
+EFIAPI
+NotifyExitBootServices (
+  IN EFI_EVENT  Event,
+  IN VOID  *Context
+  )
+{
+  ARM_SMC_ARGS SmcArgs = {0};
+
+  SmcArgs.Arg0 = ARM_SVC_ID_FFA_RXTX_UNMAP_AARCH32;
+  SmcArgs.Arg1 = FFA_PARTITION_ID(mFfaPartId);
+  ArmCallSmc ();
+
+  // We do not bother checking the error code of the RXTX_UNMAP invocation
+  // since we did map the buffers and this call must succeed.
+  return;
+
+}
+
 STATIC
 EFI_STATUS
 GetMmCompatibility (
@@ -452,6 +488,19 @@ MmCommunication2Initialize (
 goto CleanAddedMemorySpace;
   }
 
+  // Register notification callback when ExitBootservices is called to
+  // unregister the FF-A RX/TX buffer pair. This allows the OS to register its
+  // own buffer pair.
+  if (FixedPcdGet32 (PcdFfaEnable) != 0) {
+Status = gBS->CreateEvent (
+EVT_SIGNAL_EXIT_BOOT_SERVICES,
+TPL_NOTIFY,
+NotifyExitBootServices,
+NULL,
+
+);
+ASSERT_EFI_ERROR (Status);
+  }
   // Register notification callback when virtual address is associated
   // with the physical address.
   // Create a Set Virtual Address Map event.
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106809): https://edk2.groups.io/g/devel/message/106809
Mute This Topic: https://groups.io/mt/100079891/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [edk2-platforms][PATCH V1 16/20] ArmPkg/MmCommunicationDxe: Register FF-A RX/TX buffers

2023-07-11 Thread Nishant Sharma
From: Achin Gupta 

This patch statically allocates an FF-A RX/TX buffer pair and registers
them with the framework. This enables discovery of the StMM SP in a
subsequent patch.

Signed-off-by: Achin Gupta 
Signed-off-by: Nishant Sharma 
---
 ArmPkg/Include/IndustryStandard/ArmFfaSvc.h |  2 ++
 ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 20 ++--
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h 
b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
index 20eeb822ea56..ebdf29e8d69a 100644
--- a/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
+++ b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
@@ -17,6 +17,8 @@
 #define ARM_FFA_SVC_H_
 
 #define ARM_SVC_ID_FFA_VERSION_AARCH32   0x8463
+#define ARM_SVC_ID_FFA_RXTX_MAP_AARCH32  0x8466
+#define ARM_SVC_ID_FFA_RXTX_MAP_AARCH64  0xC466
 #define ARM_SVC_ID_FFA_ID_GET_AARCH320x8469
 #define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH32   0x846F
 #define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP_AARCH32  0x8470
diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c 
b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
index f907ccf7349f..8a4d46e4f80a 100644
--- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
@@ -28,6 +28,12 @@
 //
 STATIC UINT16  mFfaPartId;
 
+//
+// RX/TX pair if FF-A support is enabled
+//
+STATIC UINT8 FfaRxBuf[EFI_PAGE_SIZE] __attribute__ ((aligned (EFI_PAGE_SIZE)));
+STATIC UINT8 FfaTxBuf[EFI_PAGE_SIZE] __attribute__ ((aligned (EFI_PAGE_SIZE)));
+
 //
 // Address, Length of the pre-allocated buffer for communication with the 
secure
 // world.
@@ -293,9 +299,8 @@ GetMmCompatibility (
 Status = EFI_UNSUPPORTED;
   }
 
-  // If FF-A is supported then discover our ID.
+  // If FF-A is supported then discover our ID and register our RX/TX buffers.
   if (FixedPcdGet32 (PcdFfaEnable) != 0) {
-
 // Get our ID
 ZeroMem(, sizeof(SmcArgs));
 SmcArgs.Arg0 = ARM_SVC_ID_FFA_ID_GET_AARCH32;
@@ -307,6 +312,17 @@ GetMmCompatibility (
 DEBUG ((DEBUG_INFO, "FF-A partition ID = 0x%lx.\n", SmcArgs.Arg2));
 mFfaPartId = SmcArgs.Arg2;
 
+// Register our RX/TX pair
+SmcArgs.Arg0 = ARM_SVC_ID_FFA_RXTX_MAP_AARCH64;
+SmcArgs.Arg1 = (UINTN) FfaTxBuf;
+SmcArgs.Arg2 = (UINTN) FfaRxBuf;
+SmcArgs.Arg3 = EFI_PAGE_SIZE / SIZE_4KB;
+ArmCallSmc ();
+if (SmcArgs.Arg0 == ARM_SVC_ID_FFA_ERROR_AARCH32) {
+  DEBUG ((DEBUG_ERROR, "Unable to register FF-A RX/TX buffers (%d).\n", 
SmcArgs.Arg2));
+  return EFI_UNSUPPORTED;
+}
+
 return EFI_SUCCESS;
   }
 
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106808): https://edk2.groups.io/g/devel/message/106808
Mute This Topic: https://groups.io/mt/100079889/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [edk2-platforms][PATCH V1 14/20] ArmPkg/MmCommunicationDxe: Introduce FF-A version check

2023-07-11 Thread Nishant Sharma
From: Achin Gupta 

This patch adds support for querying whether FF-A v1.1 is supported by the
FF-A impplementation.

Signed-off-by: Achin Gupta 
Signed-off-by: Nishant Sharma 
---
 ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf |  3 +++
 ArmPkg/Drivers/MmCommunicationDxe/MmCommunicate.h |  7 ++-
 ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c   | 17 -
 3 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf 
b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
index 05b6de73ff34..c15b1a7a86ae 100644
--- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
@@ -31,6 +31,9 @@
   ArmPkg/ArmPkg.dec
   MdePkg/MdePkg.dec
 
+[FixedPcd]
+  gArmTokenSpaceGuid.PcdFfaEnable
+
 [LibraryClasses]
   ArmLib
   ArmSmcLib
diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunicate.h 
b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunicate.h
index 5c5fcb576856..71edf7f49174 100644
--- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunicate.h
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunicate.h
@@ -16,7 +16,12 @@
 #define MM_MAJOR_VER(x)  (((x) & MM_MAJOR_VER_MASK) >> MM_MAJOR_VER_SHIFT)
 #define MM_MINOR_VER(x)  ((x) & MM_MINOR_VER_MASK)
 
+#if (FixedPcdGet32 (PcdFfaEnable) == 1)
 #define MM_CALLER_MAJOR_VER  0x1UL
-#define MM_CALLER_MINOR_VER  0x0
+#define MM_CALLER_MINOR_VER  0x1UL
+#else
+#define MM_CALLER_MAJOR_VER  0x1UL
+#define MM_CALLER_MINOR_VER  0x0UL
+#endif
 
 #endif /* MM_COMMUNICATE_H_ */
diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c 
b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
index 85d9034555f0..a6fcd590a65b 100644
--- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
@@ -18,6 +18,7 @@
 
 #include 
 
+#include 
 #include 
 
 #include "MmCommunicate.h"
@@ -250,14 +251,20 @@ GetMmCompatibility (
 {
   EFI_STATUSStatus;
   UINT32MmVersion;
-  ARM_SMC_ARGS  MmVersionArgs;
+  ARM_SMC_ARGS  SmcArgs = {0};
 
-  // MM_VERSION uses SMC32 calling conventions
-  MmVersionArgs.Arg0 = ARM_SMC_ID_MM_VERSION_AARCH32;
+  if (FixedPcdGet32 (PcdFfaEnable) != 0) {
+SmcArgs.Arg0 = ARM_SVC_ID_FFA_VERSION_AARCH32;
+SmcArgs.Arg1 = MM_CALLER_MAJOR_VER << MM_MAJOR_VER_SHIFT;
+SmcArgs.Arg1 |= MM_CALLER_MINOR_VER;
+  } else {
+// MM_VERSION uses SMC32 calling conventions
+SmcArgs.Arg0 = ARM_SMC_ID_MM_VERSION_AARCH32;
+  }
 
-  ArmCallSmc ();
+  ArmCallSmc ();
 
-  MmVersion = MmVersionArgs.Arg0;
+  MmVersion = SmcArgs.Arg0;
 
   if ((MM_MAJOR_VER (MmVersion) == MM_CALLER_MAJOR_VER) &&
   (MM_MINOR_VER (MmVersion) >= MM_CALLER_MINOR_VER))
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106806): https://edk2.groups.io/g/devel/message/106806
Mute This Topic: https://groups.io/mt/100079887/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [edk2-platforms][PATCH V1 15/20] ArmPkg/MmCommunicationDxe: Add support for obtaining FF-A partition ID

2023-07-11 Thread Nishant Sharma
From: Achin Gupta 

This patch invokes the FFA_ID_GET to obtain and stash the ID of the the
FF-A partition that implements the DXE MM communication driver. This ID
is used in subsequent patches for sending and receiving MM communication
protocol requests and responses that are packaged as FF-A messages.

Signed-off-by: Achin Gupta 
Signed-off-by: Nishant Sharma 
---
 ArmPkg/Include/IndustryStandard/ArmFfaSvc.h |  1 +
 ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 22 
 2 files changed, 23 insertions(+)

diff --git a/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h 
b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
index 4a51f9fb56af..20eeb822ea56 100644
--- a/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
+++ b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
@@ -17,6 +17,7 @@
 #define ARM_FFA_SVC_H_
 
 #define ARM_SVC_ID_FFA_VERSION_AARCH32   0x8463
+#define ARM_SVC_ID_FFA_ID_GET_AARCH320x8469
 #define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH32   0x846F
 #define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP_AARCH32  0x8470
 #define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH64   0xC46F
diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c 
b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
index a6fcd590a65b..f907ccf7349f 100644
--- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
@@ -23,6 +23,11 @@
 
 #include "MmCommunicate.h"
 
+//
+// Partition ID if FF-A support is enabled
+//
+STATIC UINT16  mFfaPartId;
+
 //
 // Address, Length of the pre-allocated buffer for communication with the 
secure
 // world.
@@ -288,6 +293,23 @@ GetMmCompatibility (
 Status = EFI_UNSUPPORTED;
   }
 
+  // If FF-A is supported then discover our ID.
+  if (FixedPcdGet32 (PcdFfaEnable) != 0) {
+
+// Get our ID
+ZeroMem(, sizeof(SmcArgs));
+SmcArgs.Arg0 = ARM_SVC_ID_FFA_ID_GET_AARCH32;
+ArmCallSmc ();
+if (SmcArgs.Arg0 == ARM_SVC_ID_FFA_ERROR_AARCH32) {
+  DEBUG ((DEBUG_ERROR, "Unable to retrieve FF-A partition ID (%d).\n", 
SmcArgs.Arg2));
+  return EFI_UNSUPPORTED;
+}
+DEBUG ((DEBUG_INFO, "FF-A partition ID = 0x%lx.\n", SmcArgs.Arg2));
+mFfaPartId = SmcArgs.Arg2;
+
+return EFI_SUCCESS;
+  }
+
   return Status;
 }
 
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106807): https://edk2.groups.io/g/devel/message/106807
Mute This Topic: https://groups.io/mt/100079888/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [edk2-platforms][PATCH V1 13/20] ArmPkg: Bump the StMM SP FF-A minor version to 1

2023-07-11 Thread Nishant Sharma
From: Achin Gupta 

Signed-off-by: Achin Gupta 
Signed-off-by: Nishant Sharma 
---
 ArmPkg/Include/IndustryStandard/ArmFfaSvc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h 
b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
index 7987678c996e..4a51f9fb56af 100644
--- a/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
+++ b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
@@ -40,7 +40,7 @@
 #endif
 
 #define SPM_MAJOR_VERSION_FFA  1
-#define SPM_MINOR_VERSION_FFA  0
+#define SPM_MINOR_VERSION_FFA  1
 
 #define ARM_FFA_SPM_RET_SUCCESS  0
 #define ARM_FFA_SPM_RET_NOT_SUPPORTED   -1
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106805): https://edk2.groups.io/g/devel/message/106805
Mute This Topic: https://groups.io/mt/100079886/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [edk2-platforms][PATCH V1 11/20] StandaloneMmPkg: Skip zero sized sections while tweaking page permissions

2023-07-11 Thread Nishant Sharma
From: Achin Gupta 

This patch skips zero sized sections in the StMM SP image e.g. .reloc since
there is no point in attempting to change their permissions.

Signed-off-by: Achin Gupta 
Signed-off-by: Nishant Sharma 
---
 StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/SetPermissions.c | 18 
+-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/SetPermissions.c 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/SetPermissions.c
index 5c6bd0e1d7d2..891e79b32b6f 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/SetPermissions.c
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/SetPermissions.c
@@ -2,7 +2,7 @@
   Locate, get and update PE/COFF permissions during Standalone MM
   Foundation Entry point on ARM platforms.
 
-Copyright (c) 2017 - 2021, Arm Ltd. All rights reserved.
+Copyright (c) 2017 - 2023, Arm Ltd. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -118,6 +118,22 @@ UpdateMmFoundationPeCoffPermissions (
   ImageContext->ImageAddress,
   SectionHeader.PointerToRawData
   ));
+DEBUG ((
+  DEBUG_INFO,
+  "%a: Section %d of image at 0x%lx has %d bytes size\n",
+   __func__, Index,
+   ImageContext->ImageAddress,
+   SectionHeader.Misc.VirtualSize
+   ));
+
+// Skip sections with a size of 0
+if (SectionHeader.Misc.VirtualSize == 0) {
+  DEBUG ((
+DEBUG_INFO,
+"%a: Skipping section %a \n", __func__, SectionHeader.Name
+));
+  continue;
+}
 
 //
 // If the section is marked as XN then remove the X attribute. Furthermore,
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106803): https://edk2.groups.io/g/devel/message/106803
Mute This Topic: https://groups.io/mt/100079883/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [edk2-platforms][PATCH V1 12/20] StandaloneMmPkg: Add global check for FF-A abis

2023-07-11 Thread Nishant Sharma
From: Achin Gupta 

This patch copies the value of the corresponding stack variable to a global
variable so that it can be used to determine whether FF-A v1.1 or earlier
ABIs should be used for communication with the SPMC.

Signed-off-by: Achin Gupta 
Signed-off-by: Nishant Sharma 
---
 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
 | 16 
 1 file changed, 16 insertions(+)

diff --git 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
index 8131b1984969..a763bf8509b2 100644
--- 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
+++ 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
@@ -42,6 +42,12 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #define FFA_PAGE_16K 1
 #define FFA_PAGE_64K 2
 
+// Local variable to help Standalone MM Core decide whether FF-A ABIs can be
+// used for all communication. This variable is usable only after the StMM 
image
+// has been relocated and all image section permissions have been correctly
+// updated.
+STATIC BOOLEAN mUseOnlyFfaAbis = FALSE;
+
 PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT  CpuDriverEntryPoint = NULL;
 
 /**
@@ -639,6 +645,13 @@ InitArmSvcArgs (
   )
 {
   if (FixedPcdGet32 (PcdFfaEnable) != 0) {
+
+// With FF-A v1.1 invoke FFA_MSG_WAIT to signal completion of SP init.
+if (mUseOnlyFfaAbis) {
+  InitMmFoundationSvcArgs->Arg0 = ARM_SVC_ID_FFA_MSG_WAIT_AARCH32;
+  return;
+}
+
 InitMmFoundationSvcArgs->Arg0 = ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP;
 InitMmFoundationSvcArgs->Arg1 = 0;
 InitMmFoundationSvcArgs->Arg2 = 0;
@@ -820,6 +833,9 @@ ModuleEntryPoint (
 ASSERT_EFI_ERROR (Status);
   }
 
+  // Update the global copy now that the image has been relocated.
+  mUseOnlyFfaAbis = UseOnlyFfaAbis;
+
   //
   // Create Hoblist based upon boot information passed by privileged software
   //
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106804): https://edk2.groups.io/g/devel/message/106804
Mute This Topic: https://groups.io/mt/100079884/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [edk2-platforms][PATCH V1 09/20] StandaloneMmPkg: parse SP manifest and populate new boot information

2023-07-11 Thread Nishant Sharma
From: Achin Gupta 

This patch discovers the SP manifest in DT format passed by the SPMC. It
then parses it to obtain the boot information required to initialise the
SP.

Signed-off-by: Achin Gupta 
Signed-off-by: Sayanta Pattanayak 
Signed-off-by: Nishant Sharma 
---
 StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h   
 |   2 +-
 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
 | 389 +++-
 2 files changed, 381 insertions(+), 10 deletions(-)

diff --git a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h 
b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
index c965192c702e..90d67a2f25b5 100644
--- a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
+++ b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
@@ -2,7 +2,7 @@
   Entry point to the Standalone MM Foundation when initialized during the SEC
   phase on ARM platforms
 
-Copyright (c) 2017 - 2021, Arm Ltd. All rights reserved.
+Copyright (c) 2017 - 2023, Arm Ltd. All rights reserved.
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
diff --git 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
index 9f6af55c86c4..505786aff07c 100644
--- 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
+++ 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
@@ -38,6 +38,10 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #define BOOT_PAYLOAD_VERSION  1
 
+#define FFA_PAGE_4K 0
+#define FFA_PAGE_16K 1
+#define FFA_PAGE_64K 2
+
 PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT  CpuDriverEntryPoint = NULL;
 
 /**
@@ -106,6 +110,7 @@ GetAndPrintBootinformation (
   }
 
   return PayloadBootInfo;
+}
 
 /**
   An StMM SP implements partial support for FF-A v1.0. The FF-A ABIs are used 
to
@@ -266,6 +271,308 @@ DelegatedEventLoop (
   }
 }
 
+STATIC
+BOOLEAN
+CheckDescription (
+IN VOID   * DtbAddress,
+IN INT32Offset,
+OUT CHAR8 * Description,
+OUT UINT32  Size
+)
+{
+  CONST CHAR8 * Property;
+  INT32 LenP;
+
+  Property = fdt_getprop (DtbAddress, Offset, "description", );
+  if (Property == NULL) {
+return FALSE;
+  }
+
+ return CompareMem (Description, Property, MIN(Size, (UINT32)LenP)) == 0;
+
+}
+
+STATIC
+EFI_STATUS
+ReadProperty32 (
+IN  VOID   * DtbAddress,
+IN  INT32Offset,
+IN  CHAR8  * Property,
+OUT UINT32 * Value
+)
+{
+  CONST UINT32 * Property32;
+
+  Property32 =  fdt_getprop (DtbAddress, Offset, Property, NULL);
+  if (Property32 == NULL) {
+DEBUG ((
+  DEBUG_ERROR,
+  "%s: Missing in FF-A boot information manifest\n",
+  Property
+  ));
+return EFI_INVALID_PARAMETER;
+  }
+
+  *Value = fdt32_to_cpu (*Property32);
+
+  return EFI_SUCCESS;
+}
+
+STATIC
+EFI_STATUS
+ReadProperty64 (
+IN  VOID   * DtbAddress,
+IN  INT32Offset,
+IN  CHAR8  * Property,
+OUT UINT64 * Value
+)
+{
+  CONST UINT64 * Property64;
+
+  Property64 =  fdt_getprop (DtbAddress, Offset, Property, NULL);
+  if (Property64 == NULL) {
+DEBUG ((
+  DEBUG_ERROR,
+  "%s: Missing in FF-A boot information manifest\n",
+  Property
+  ));
+return EFI_INVALID_PARAMETER;
+  }
+
+  *Value = fdt64_to_cpu (*Property64);
+
+  return EFI_SUCCESS;
+}
+
+STATIC
+BOOLEAN
+ReadRegionInfo (
+IN VOID  *DtbAddress,
+IN INT32  Node,
+IN CHAR8 *Region,
+IN UINTN  RegionStrSize,
+IN UINT32 PageSize,
+OUT UINT64 *Address,
+OUT UINT64 *Size
+)
+{
+  BOOLEAN FoundBuffer;
+  INTN Status = 0;
+
+  FoundBuffer = CheckDescription (
+  DtbAddress,
+  Node,
+  Region,
+  RegionStrSize
+  );
+  if (!FoundBuffer) {
+return FALSE;
+  }
+
+  DEBUG ((DEBUG_INFO, "Found Node: %a\n", Region));
+  Status = ReadProperty64 (
+  DtbAddress,
+  Node,
+  "base-address",
+  Address
+  );
+  if (Status != EFI_SUCCESS) {
+DEBUG ((DEBUG_ERROR, "base-address missing in DTB"));
+return FALSE;
+  }
+  DEBUG ((
+DEBUG_INFO,
+"base = 0x%llx\n",
+*Address
+));
+
+  Status = ReadProperty32 (
+  DtbAddress,
+  Node,
+  "pages-count",
+  (UINT32*)Size
+  );
+  if (Status != EFI_SUCCESS) {
+DEBUG ((DEBUG_ERROR, "pages-count missing in DTB"));
+return FALSE;
+  }
+
+  DEBUG ((DEBUG_ERROR, "pages-count: 0x%lx\n", *Size));
+
+  *Size = *Size * PageSize;
+  DEBUG ((
+DEBUG_INFO,
+"Size = 0x%llx\n",
+*Size
+));
+
+  return TRUE;
+}
+
+/**
+
+  Populates FF-A boot information structure.
+
+  This function receives the address of a DTB from which boot information 
defind
+  by FF-A and required to initialize the standalone environment is extracted.
+
+  @param [in, out] StmmBootInfo  Pointer to a 

[edk2-devel] [edk2-platforms][PATCH V1 08/20] StandaloneMmPkg: Add backwards compatible support to detect FF-A v1.1

2023-07-11 Thread Nishant Sharma
From: Achin Gupta 

For better or worse, an StMM SP can communicate with the SPM through one
of these interfaces.

1. SPM_MM interface
2. FF-A v1.0 interface
3. FF-A v1.1 interface

2) implements only minimal FF-A support. It reuses the initialisation ABI
defined by 1) and wraps the remaining communicaton in
FFA_MSG_SEND_DIRECT_REQ/RESP ABIs.
3) uses FF-A ABIs from the spec for both initialisation and communication.

Detecting these variations in the GetSpmVersion() function is tedious. This
patch restores the original function that discovered configuration
1). It defines a new function to discover presence of FF-A and
differentiate between v1.0 and v1.1.

Signed-off-by: Achin Gupta 
Signed-off-by: Nishant Sharma 
---
 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
 | 132 +---
 1 file changed, 87 insertions(+), 45 deletions(-)

diff --git 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
index 682b55b5478a..9f6af55c86c4 100644
--- 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
+++ 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
@@ -32,13 +32,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #define SPM_MAJOR_VER_MASK   0x
 #define SPM_MINOR_VER_MASK   0x
 #define SPM_MAJOR_VER_SHIFT  16
-#define FFA_NOT_SUPPORTED-1
 
-STATIC CONST UINT32  mSpmMajorVer = SPM_MAJOR_VERSION;
-STATIC CONST UINT32  mSpmMinorVer = SPM_MINOR_VERSION;
-
-STATIC CONST UINT32  mSpmMajorVerFfa = SPM_MAJOR_VERSION_FFA;
-STATIC CONST UINT32  mSpmMinorVerFfa = SPM_MINOR_VERSION_FFA;
+#define SPM_MAJOR_VER 0
+#define SPM_MINOR_VER 1
 
 #define BOOT_PAYLOAD_VERSION  1
 
@@ -110,6 +106,70 @@ GetAndPrintBootinformation (
   }
 
   return PayloadBootInfo;
+
+/**
+  An StMM SP implements partial support for FF-A v1.0. The FF-A ABIs are used 
to
+  get and set permissions of memory pages in collaboration with the SPMC and
+  signalling completion of initialisation. The original Arm MM communication
+  interface is used for communication with the Normal world. A TF-A specific
+  interface is used for initialising the SP.
+
+  With FF-A v1.1, the StMM SP uses only FF-A ABIs for initialisation and
+  communication. This is subject to support for FF-A v1.1 in the SPMC. If this
+  is not the case, the StMM implementation reverts to the FF-A v1.0
+  behaviour. Any of this is applicable only if the feature flag PcdFfaEnable is
+  TRUE.
+
+  This function helps the caller determine whether FF-A v1.1 or v1.0 are
+  available and if only FF-A ABIs can be used at runtime.
+**/
+STATIC
+EFI_STATUS
+CheckFfaCompatibility (BOOLEAN *UseOnlyFfaAbis)
+{
+  UINT16   SpmcMajorVer;
+  UINT16   SpmcMinorVer;
+  UINT32   SpmcVersion;
+  ARM_SVC_ARGS SpmcVersionArgs = {0};
+
+  // Sanity check in case of a spurious call.
+  if (FixedPcdGet32 (PcdFfaEnable) == 0) {
+return EFI_UNSUPPORTED;
+  }
+
+  // Send the SPMC our version to see whether it supports the same or not.
+  SpmcVersionArgs.Arg0 = ARM_SVC_ID_FFA_VERSION_AARCH32;
+  SpmcVersionArgs.Arg1 = FFA_VERSION_COMPILED;
+
+  ArmCallSvc ();
+  SpmcVersion = SpmcVersionArgs.Arg0;
+
+  // If the SPMC barfs then bail.
+  if (SpmcVersion == ARM_FFA_SPM_RET_NOT_SUPPORTED) {
+return EFI_UNSUPPORTED;
+  }
+
+  // Extract the SPMC version
+  SpmcMajorVer = (SpmcVersion >> FFA_VERSION_MAJOR_SHIFT) & 
FFA_VERSION_MAJOR_MASK;
+  SpmcMinorVer = (SpmcVersion >> FFA_VERSION_MINOR_SHIFT) & 
FFA_VERSION_MINOR_MASK;
+
+  // If the major versions differ then all bets are off.
+  if (SpmcMajorVer != SPM_MAJOR_VERSION_FFA) {
+return EFI_UNSUPPORTED;
+  }
+
+  // We advertised v1.1 as our version. If the SPMC supports it, it must return
+  // the same or a compatible version. If it does not then FF-A ABIs cannot be
+  // used for all communication.
+  if (SpmcMinorVer >= SPM_MINOR_VERSION_FFA) {
+*UseOnlyFfaAbis = TRUE;
+  } else {
+*UseOnlyFfaAbis = FALSE;
+  }
+
+  // We have validated that there is a compatible FF-A
+  // implementation. So. return success.
+  return EFI_SUCCESS;
 }
 
 /**
@@ -219,34 +279,19 @@ GetSpmVersion (
   )
 {
   EFI_STATUSStatus;
-  UINT16CalleeSpmMajorVer;
-  UINT16CallerSpmMajorVer;
-  UINT16CalleeSpmMinorVer;
-  UINT16CallerSpmMinorVer;
+  UINT16SpmMajorVersion;
+  UINT16SpmMinorVersion;
   UINT32SpmVersion;
   ARM_SVC_ARGS  SpmVersionArgs;
 
-  if (FixedPcdGet32 (PcdFfaEnable) != 0) {
-SpmVersionArgs.Arg0  = ARM_SVC_ID_FFA_VERSION_AARCH32;
-SpmVersionArgs.Arg1  = mSpmMajorVerFfa << SPM_MAJOR_VER_SHIFT;
-SpmVersionArgs.Arg1 |= mSpmMinorVerFfa;
-CallerSpmMajorVer= mSpmMajorVerFfa;
-CallerSpmMinorVer= mSpmMinorVerFfa;
-  } else {
-SpmVersionArgs.Arg0 = ARM_SVC_ID_SPM_VERSION_AARCH32;
-

[edk2-devel] [edk2-platforms][PATCH V1 05/20] ArmPkg/ArmFfaSvc: Add helper macros and fids

2023-07-11 Thread Nishant Sharma
From: Achin Gupta 

Add new fid for Success, error and wait. Also add macro to generate FFA
verions.

Signed-off-by: Achin Gupta 
Signed-off-by: Nishant Sharma 
---
 ArmPkg/Include/IndustryStandard/ArmFfaSvc.h | 21 +++-
 1 file changed, 20 insertions(+), 1 deletion(-)

diff --git a/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h 
b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
index 54cc96598032..c80d783fad3f 100644
--- a/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
+++ b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
@@ -21,6 +21,11 @@
 #define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP_AARCH32  0x8470
 #define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_REQ_AARCH64   0xC46F
 #define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP_AARCH64  0xC470
+#define ARM_SVC_ID_FFA_SUCCESS_AARCH32   0x8461
+#define ARM_SVC_ID_FFA_SUCCESS_AARCH64   0xC461
+#define ARM_SVC_ID_FFA_ERROR_AARCH32 0x8460
+#define ARM_SVC_ID_FFA_ERROR_AARCH64 0xC460
+#define ARM_SVC_ID_FFA_MSG_WAIT_AARCH32  0x846B
 
 /* Generic IDs when using AArch32 or AArch64 execution state */
 #ifdef MDE_CPU_AARCH64
@@ -35,7 +40,7 @@
 #define SPM_MAJOR_VERSION_FFA  1
 #define SPM_MINOR_VERSION_FFA  0
 
-#define ARM_FFA_SPM_RET_SUCCESS 0
+#define ARM_FFA_SPM_RET_SUCCESS  0
 #define ARM_FFA_SPM_RET_NOT_SUPPORTED   -1
 #define ARM_FFA_SPM_RET_INVALID_PARAMETERS  -2
 #define ARM_FFA_SPM_RET_NO_MEMORY   -3
@@ -45,6 +50,20 @@
 #define ARM_FFA_SPM_RET_RETRY   -7
 #define ARM_FFA_SPM_RET_ABORTED -8
 
+// FF-A version helper macros
+#define FFA_VERSION_MAJOR_SHIFT 16
+#define FFA_VERSION_MAJOR_MASK  0x7FFF
+#define FFA_VERSION_MINOR_SHIFT 0
+#define FFA_VERSION_MINOR_MASK  0x
+#define FFA_VERSION_BIT31_MASK  (0x1u << 31)
+
+#define MAKE_FFA_VERSION(major, minor)  \
+  major) & FFA_VERSION_MAJOR_MASK) << FFA_VERSION_MAJOR_SHIFT) |  \
+  (((minor) & FFA_VERSION_MINOR_MASK) << FFA_VERSION_MINOR_SHIFT))
+
+#define FFA_VERSION_COMPILED
MAKE_FFA_VERSION(SPM_MAJOR_VERSION_FFA, \
+SPM_MINOR_VERSION_FFA)
+
 // For now, the destination id to be used in the FF-A calls
 // is being hard-coded. Subsequently, support will be added
 // to get the endpoint id's dynamically
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106797): https://edk2.groups.io/g/devel/message/106797
Mute This Topic: https://groups.io/mt/100079876/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [edk2-platforms][PATCH V1 07/20] StandaloneMmPkg: define new data structure to stage FF-A boot information

2023-07-11 Thread Nishant Sharma
From: Achin Gupta 

With FF-A v1.1, the SPMC sends a reduced amount of boot information as
compared to the original SPM implementation. For example, the stack layout,
MP information etc. This information could be accommodated in the old data
structure but this is too complicated. This patch defines a new structure
to clearly separate FF-A v1.1 and other functionality.

Signed-off-by: Achin Gupta 
Signed-off-by: Nishant Sharma 
---
 StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h | 21 

 1 file changed, 21 insertions(+)

diff --git a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h 
b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
index 41bf0f132b4f..c965192c702e 100644
--- a/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
+++ b/StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h
@@ -47,6 +47,27 @@ typedef struct {
   EFI_SECURE_PARTITION_CPU_INFO*CpuInfo;
 } EFI_SECURE_PARTITION_BOOT_INFO;
 
+
+/**
+  This structure is used to stage boot information required to initialize the
+  standalone MM environment when FF-A is used as the interface between this
+  secure partition and the SPMC. This structure supersedes
+  EFI_SECURE_PARTITION_BOOT_INFO and reduces the amount of information that 
must
+  be passed by the SPMC for SP initialization.
+**/
+typedef struct {
+  UINT64SpMemBase;
+  UINT64SpMemSize;
+  UINT64SpNsCommBufBase;
+  UINT64SpNsCommBufSize;
+  UINT64SpSharedBufBase;
+  UINT64SpSharedBufSize;
+  UINT64SpHeapBase;
+  UINT64SpHeapSize;
+  /* UP migrate-able FF-A SP requires awareness of only 1 cpu */
+  EFI_SECURE_PARTITION_CPU_INFO CpuInfo;
+} EFI_STMM_BOOT_INFO;
+
 typedef
 EFI_STATUS
 (*PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT) (
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106799): https://edk2.groups.io/g/devel/message/106799
Mute This Topic: https://groups.io/mt/100079879/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [edk2-platforms][PATCH V1 06/20] ArmPkg: Add support for FFA_MEM_PERM_GET/SET ABIs

2023-07-11 Thread Nishant Sharma
From: Achin Gupta 

This patch uses the FFA_MEM_PERM_GET/SET ABIs to tweak the permissions of a
set of pages if FF-A v1.1 and above is supported by the SPMC. For FF-A v1.0
the previous method through FFA_MSG_SEND_DIRECT_REQ/RESP is used.

Signed-off-by: Achin Gupta 
Signed-off-by: Nishant Sharma 
---
 ArmPkg/Include/IndustryStandard/ArmFfaSvc.h   |   2 +
 ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.c | 132 
+---
 2 files changed, 120 insertions(+), 14 deletions(-)

diff --git a/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h 
b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
index c80d783fad3f..7987678c996e 100644
--- a/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
+++ b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
@@ -23,6 +23,8 @@
 #define ARM_SVC_ID_FFA_MSG_SEND_DIRECT_RESP_AARCH64  0xC470
 #define ARM_SVC_ID_FFA_SUCCESS_AARCH32   0x8461
 #define ARM_SVC_ID_FFA_SUCCESS_AARCH64   0xC461
+#define ARM_SVC_ID_FFA_MEM_PERM_SET_AARCH32  0x8489
+#define ARM_SVC_ID_FFA_MEM_PERM_GET_AARCH32  0x8488
 #define ARM_SVC_ID_FFA_ERROR_AARCH32 0x8460
 #define ARM_SVC_ID_FFA_ERROR_AARCH64 0xC460
 #define ARM_SVC_ID_FFA_MSG_WAIT_AARCH32  0x846B
diff --git a/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.c 
b/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.c
index 1a41a289ef17..76ef214bcb85 100644
--- a/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.c
+++ b/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.c
@@ -25,6 +25,66 @@
 #include 
 #include 
 
+
+/**
+  Utility function to determine whether ABIs in FF-A v1.1 to set and get
+  memory permissions can be used. Ideally, this should be invoked once in the
+  library constructor and set a flag that can be used at runtime. However, the
+  StMM Core invokes this library before constructors are called and before the
+  StMM image itself is relocated.
+
+  @retval EFI_SUCCESS The availability of ABIs was correctly determined.
+  @retval Other value Software stack is misconfigured.
+
+**/
+STATIC
+BOOLEAN
+UseFfaMemPermAbis (
+  VOID
+  )
+{
+  ARM_SVC_ARGSSvcArgs;
+  UINT32  SpmcFfaVersion;
+  STATIC UINT16   SpmcMajorVer = 0;
+  STATIC UINT16   SpmcMinorVer = 0;
+
+  // Use prefetched version info. if either is not 0, then the version is
+  // already fetched.
+  if ((SpmcMajorVer | SpmcMinorVer) != 0) {
+return (SpmcMajorVer == SPM_MAJOR_VERSION_FFA) && (SpmcMinorVer >= 
SPM_MINOR_VERSION_FFA);
+  }
+
+  // Nothing to do if FF-A has not be enabled
+  if (FixedPcdGet32 (PcdFfaEnable) == 0) {
+return FALSE;
+  }
+
+  // Prepare the message parameters.
+  ZeroMem (, sizeof (ARM_SVC_ARGS));
+  SvcArgs.Arg0 = ARM_SVC_ID_FFA_VERSION_AARCH32;
+  SvcArgs.Arg1 = FFA_VERSION_COMPILED;
+
+  // Invoke the ABI
+  ArmCallSvc ();
+
+  // Check if FF-A is supported and what version
+  SpmcFfaVersion = SvcArgs.Arg0;
+
+  // Damn! FF-A is not supported at all even though we specified v1.0 as our
+  // version. However, the feature flag has been turned on. This is a
+  // misconfigured software stack. So, return an error and assert in a debug 
build.
+  if (SpmcFfaVersion == ARM_FFA_SPM_RET_NOT_SUPPORTED) {
+ASSERT (0);
+return FALSE;
+  }
+
+  SpmcMajorVer = (SpmcFfaVersion >> FFA_VERSION_MAJOR_SHIFT) & 
FFA_VERSION_MAJOR_MASK;
+  SpmcMinorVer = (SpmcFfaVersion >> FFA_VERSION_MINOR_SHIFT) & 
FFA_VERSION_MINOR_MASK;
+
+  return (SpmcMajorVer == SPM_MAJOR_VERSION_FFA) && (SpmcMinorVer >= 
SPM_MINOR_VERSION_FFA);
+}
+
+
 /** Send memory permission request to target.
 
   @param [in, out]  SvcArgs Pointer to SVC arguments to send. On
@@ -55,6 +115,36 @@ SendMemoryPermissionRequest (
 
   ArmCallSvc (SvcArgs);
   if (FixedPcdGet32 (PcdFfaEnable) != 0) {
+
+// Check if FF-A memory permission ABIs were used.
+if (UseFfaMemPermAbis()) {
+  switch (SvcArgs->Arg0) {
+
+case ARM_SVC_ID_FFA_ERROR_AARCH32:
+case ARM_SVC_ID_FFA_ERROR_AARCH64:
+  switch (SvcArgs->Arg2) {
+  case ARM_FFA_SPM_RET_INVALID_PARAMETERS:
+return EFI_INVALID_PARAMETER;
+  case ARM_FFA_SPM_RET_NOT_SUPPORTED:
+return EFI_UNSUPPORTED;
+  default:
+// Undefined error code received.
+ASSERT (0);
+return EFI_INVALID_PARAMETER;
+  }
+
+case ARM_SVC_ID_FFA_SUCCESS_AARCH32:
+case ARM_SVC_ID_FFA_SUCCESS_AARCH64:
+  *RetVal = SvcArgs->Arg2;
+  return EFI_SUCCESS;
+
+default:
+  // Undefined error code received.
+  ASSERT (0);
+  return EFI_INVALID_PARAMETER;
+  }
+}
+
 // Get/Set memory attributes is an atomic call, with
 // StandaloneMm at S-EL0 being the caller and the SPM
 // core being the callee. Thus there won't be a
@@ -164,12 +254,18 @@ GetMemoryPermissions (
   // See [1], Section 13.5.5.1 

[edk2-devel] [edk2-platforms][PATCH V1 03/20] StandaloneMmPkg: Include libfdt in the StMM

2023-07-11 Thread Nishant Sharma
From: Achin Gupta 

The StMM SP will extract boot information from its manifest instead of a C
data structure populated by the SPM. The manifest will be passed by the
SPM. This patch includes support for libfdt to prepare for parsing the
manifest in future patches.

Signed-off-by: Sayanta Pattanayak 
Signed-off-by: Nishant Sharma 
---
 StandaloneMmPkg/StandaloneMmPkg.dsc
 | 3 ++-
 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
   | 3 +++
 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
 | 1 +
 3 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/StandaloneMmPkg/StandaloneMmPkg.dsc 
b/StandaloneMmPkg/StandaloneMmPkg.dsc
index 8012f93b7dcc..0060eddb900c 100644
--- a/StandaloneMmPkg/StandaloneMmPkg.dsc
+++ b/StandaloneMmPkg/StandaloneMmPkg.dsc
@@ -2,7 +2,7 @@
 # Standalone MM Platform.
 #
 # Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.
-# Copyright (c) 2016 - 2021, Arm Limited. All rights reserved.
+# Copyright (c) 2016 - 2023, Arm Limited. All rights reserved.
 # Copyright (C) Microsoft Corporation
 #
 #SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -49,6 +49,7 @@
   HobLib|StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf
   IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
   MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.inf
+  FdtLib|EmbeddedPkg/Library/FdtLib/FdtLib.inf
   
MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmCoreMemoryAllocationLib/StandaloneMmCoreMemoryAllocationLib.inf
   
MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf
   PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
diff --git 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
index 10fafa43ce59..80af62450e22 100644
--- 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
+++ 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
@@ -31,6 +31,7 @@
   X64/StandaloneMmCoreEntryPoint.c
 
 [Packages]
+  EmbeddedPkg/EmbeddedPkg.dec
   MdePkg/MdePkg.dec
   MdeModulePkg/MdeModulePkg.dec
   StandaloneMmPkg/StandaloneMmPkg.dec
@@ -41,10 +42,12 @@
 [LibraryClasses]
   BaseLib
   DebugLib
+  FdtLib
 
 [LibraryClasses.ARM, LibraryClasses.AARCH64]
   StandaloneMmMmuLib
   ArmSvcLib
+  FdtLib
 
 [Guids]
   gMpInformationHobGuid
diff --git 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
index ce867fe85158..682b55b5478a 100644
--- 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
+++ 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
@@ -15,6 +15,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106795): https://edk2.groups.io/g/devel/message/106795
Mute This Topic: https://groups.io/mt/100079873/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [edk2-platforms][PATCH V1 04/20] ArmPkg: Add data structures to receive FF-A boot information

2023-07-11 Thread Nishant Sharma
From: Achin Gupta 

The SPMC will pass the manifest to the StMM SP which contains the boot
information required for SP initialisation. This patch defines the data
structures defined in Section 5.4 of the FF-A v1.1 BETA0 spec to enable
this support. The manifest is identified by the TF-A UUID_TOS_FW_CONFIG.

Signed-off-by: Achin Gupta 
Signed-off-by: Nishant Sharma 
---
 ArmPkg/Include/IndustryStandard/ArmFfaSvc.h | 69 +++-
 1 file changed, 68 insertions(+), 1 deletion(-)

diff --git a/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h 
b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
index 4126a4985bb2..54cc96598032 100644
--- a/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
+++ b/ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
@@ -3,7 +3,7 @@
   communication between S-EL0 and the Secure Partition
   Manager(SPM)
 
-  Copyright (c) 2020, ARM Limited. All rights reserved.
+  Copyright (c) 2020 - 2023, ARM Limited. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -53,4 +53,71 @@
 // 
https://github.com/OP-TEE/optee_os/blob/master/core/arch/arm/kernel/stmm_sp.c#L66
 #define ARM_FFA_DESTINATION_ENDPOINT_ID  3
 
+/**
+ * Boot information protocol as per the FF-A v1.1 spec.
+ */
+#define FFA_INIT_DESC_SIGNATURE 0x0FFA
+
+/* Boot information type. */
+#define FFA_BOOT_INFO_TYPE_STD  0x0U
+#define FFA_BOOT_INFO_TYPE_IMPL 0x1U
+
+#define FFA_BOOT_INFO_TYPE_MASK 0x1U
+#define FFA_BOOT_INFO_TYPE_SHIFT0x7U
+#define FFA_BOOT_INFO_TYPE(type)\
+  (((type) & FFA_BOOT_INFO_TYPE_MASK)   \
+   << FFA_BOOT_INFO_TYPE_SHIFT)
+
+/* Boot information identifier. */
+#define FFA_BOOT_INFO_TYPE_ID_FDT   0x0U
+#define FFA_BOOT_INFO_TYPE_ID_HOB   0x1U
+
+#define FFA_BOOT_INFO_TYPE_ID_MASK0x3FU
+#define FFA_BOOT_INFO_TYPE_ID_SHIFT   0x0U
+#define FFA_BOOT_INFO_TYPE_ID(type)   \
+  (((type) & FFA_BOOT_INFO_TYPE_ID_MASK)  \
+   << FFA_BOOT_INFO_TYPE_ID_SHIFT)
+
+/* Format of Flags Name field. */
+#define FFA_BOOT_INFO_FLAG_NAME_STRING  0x0U
+#define FFA_BOOT_INFO_FLAG_NAME_UUID0x1U
+
+#define FFA_BOOT_INFO_FLAG_NAME_MASK0x3U
+#define FFA_BOOT_INFO_FLAG_NAME_SHIFT   0x0U
+#define FFA_BOOT_INFO_FLAG_NAME(type)   \
+  (((type) & FFA_BOOT_INFO_FLAG_NAME_MASK)  \
+   << FFA_BOOT_INFO_FLAG_NAME_SHIFT)
+
+/* Format of Flags Contents field. */
+#define FFA_BOOT_INFO_FLAG_CONTENT_ADR0x0U
+#define FFA_BOOT_INFO_FLAG_CONTENT_VAL0x1U
+
+#define FFA_BOOT_INFO_FLAG_CONTENT_MASK   0x1U
+#define FFA_BOOT_INFO_FLAG_CONTENT_SHIFT  0x2U
+#define FFA_BOOT_INFO_FLAG_CONTENT(content)   \
+  (((content) & FFA_BOOT_INFO_FLAG_CONTENT_MASK)  \
+   << FFA_BOOT_INFO_FLAG_CONTENT_SHIFT)
+
+// Descriptor to pass boot information as per the FF-A v1.1 spec.
+typedef struct {
+  UINT32 Name[4];
+  UINT8 Type;
+  UINT8 Reserved;
+  UINT16 Flags;
+  UINT32 SizeBotInfo;
+  UINT64 Content;
+} EFI_FFA_BOOT_INFO_DESC;
+
+// Descriptor that contains boot info blobs size, number of desc it cointains
+// size of each descriptor and offset to the first descriptor.
+typedef struct {
+  UINT32 Magic; // 0xFFA^M
+  UINT32 Version;
+  UINT32 SizeBootInfoBlob;
+  UINT32 SizeBootInfoDesc;
+  UINT32 CountBootInfoDesc;
+  UINT32 OffsetBootInfoDesc;
+  UINT64 Reserved;
+} EFI_FFA_BOOT_INFO_HEADER;
+
 #endif // ARM_FFA_SVC_H_
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106796): https://edk2.groups.io/g/devel/message/106796
Mute This Topic: https://groups.io/mt/100079874/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [edk2-platforms][PATCH V1 00/20] Add the support for ARM Firmware First Framework

2023-07-11 Thread Nishant Sharma
V1 :

Following patchset add the support of ARM Firmware Framework protocol to
MM communication.

Following chagnes are made to enable the support
 1> Changed the FFA enable flag(PcdFfaEnable) to integer to enable it
 use in assembly.
 2> Add the support to Reserved SP stack space and program in startup
 code.
 3> Added ABI for enabling FFA protocol
 4> Added conditional support in Standalone MM to parse DTB and
 populated configuration info required for FFA.
 5> Added FFA support in MmCommunicationDxe module.

Patches are pushed to
https://github.com/nissha03/edk2/tree/ArmFirmwareFramework

ARM Firmware Framework Protocol:
https://developer.arm.com/documentation/den0077/latest/

Achin Gupta (19):
  StandaloneMmPkg: Allocate and initialise SP stack from internal memory
  StandaloneMmPkg: Include libfdt in the StMM
  ArmPkg: Add data structures to receive FF-A boot information
  ArmPkg/ArmFfaSvc: Add helper macros and fids
  ArmPkg: Add support for FFA_MEM_PERM_GET/SET ABIs
  StandaloneMmPkg: define new data structure to stage FF-A boot
information
  StandaloneMmPkg: Add backwards compatible support to detect FF-A v1.1
  StandaloneMmPkg: parse SP manifest and populate new boot information
  StandaloneMmPkg: Populate Hoblist for SP init from StMM boot
information
  StandaloneMmPkg: Skip zero sized sections while tweaking page
permissions
  StandaloneMmPkg: Add global check for FF-A abis
  ArmPkg: Bump the StMM SP FF-A minor version to 1
  ArmPkg/MmCommunicationDxe: Introduce FF-A version check
  ArmPkg/MmCommunicationDxe: Add support for obtaining FF-A partition ID
  ArmPkg/MmCommunicationDxe: Register FF-A RX/TX buffers
  ArmPkg/MmCommunicationDxe: Unmap FF-A RX/TX buffers during
ExitBootServices
  ArmPkg/MmCommunicationDxe: Discover the StMM SP
  ArmPkg/MmCommunicationDxe: Use the FF-A transport for MM requests
  StandaloneMmPkg: Add support for MM requests as FF-A direct messages

Nishant Sharma (1):
  ArmPkg: Change PcdFfaEnable flag datatype

 ArmPkg/ArmPkg.dec  
 |  14 +-
 StandaloneMmPkg/StandaloneMmPkg.dsc
 |   3 +-
 ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf  
 |   3 +
 ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf
 |   4 +-
 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
   |   8 +-
 ArmPkg/Drivers/MmCommunicationDxe/MmCommunicate.h  
 |   7 +-
 ArmPkg/Include/IndustryStandard/ArmFfaSvc.h
 | 133 -
 StandaloneMmPkg/Include/Library/Arm/StandaloneMmCoreEntryPoint.h   
 |  39 +-
 ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
 | 332 ++--
 ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.c  
 | 140 -
 StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/CreateHobList.c 
 | 186 ++-
 StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/SetPermissions.c
 |  18 +-
 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
 | 561 +---
 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/AArch64/ModuleEntryPoint.S
   |  68 +++
 14 files changed, 1367 insertions(+), 149 deletions(-)
 create mode 100644 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/AArch64/ModuleEntryPoint.S

-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106792): https://edk2.groups.io/g/devel/message/106792
Mute This Topic: https://groups.io/mt/100079869/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [edk2-platforms][PATCH V1 02/20] StandaloneMmPkg: Allocate and initialise SP stack from internal memory

2023-07-11 Thread Nishant Sharma
From: Achin Gupta 

This patch removes the dependency on the SPM to allocate and initialise
stack memory for the StMM SP. This is done by reserving 8K worth of memory
in the StMM image at a page aligned address in the data section. Then,
instead of jumping directly to the C entrypoint, an assembler entrypoint is
invoked. This entrypoint locates the stack memory, changes its permissions
using FF-A ABIs, sets the stack pointer and invokes the C entrypoint.

Signed-off-by: Achin Gupta 
Signed-off-by: Nishant Sharma 
---
 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
   |  1 +
 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
 |  2 +-
 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/AArch64/ModuleEntryPoint.S
   | 68 
 3 files changed, 70 insertions(+), 1 deletion(-)

diff --git 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
index dc6d3d859911..10fafa43ce59 100644
--- 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
+++ 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
@@ -25,6 +25,7 @@
   Arm/StandaloneMmCoreEntryPoint.c
   Arm/SetPermissions.c
   Arm/CreateHobList.c
+  Arm/AArch64/ModuleEntryPoint.S
 
 [Sources.X64]
   X64/StandaloneMmCoreEntryPoint.c
diff --git 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
index 5dd1d9747995..ce867fe85158 100644
--- 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
+++ 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
@@ -316,7 +316,7 @@ InitArmSvcArgs (
 **/
 VOID
 EFIAPI
-_ModuleEntryPoint (
+ModuleEntryPoint (
   IN VOID*SharedBufAddress,
   IN UINT64  SharedBufSize,
   IN UINT64  cookie1,
diff --git 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/AArch64/ModuleEntryPoint.S
 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/AArch64/ModuleEntryPoint.S
new file mode 100644
index ..174bc83ebd64
--- /dev/null
+++ 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/AArch64/ModuleEntryPoint.S
@@ -0,0 +1,68 @@
+//
+//  Copyright (c) 2023, ARM Limited. All rights reserved.
+//
+//  SPDX-License-Identifier: BSD-2-Clause-Patent
+//
+//
+
+#include 
+
+#define FFA_DATA_RW 0x1
+
+.align 12
+StackBase:
+.space 8192
+StackEnd:
+
+.macro FfaMemPermSet start:req end:req perm:req
+adrp x29, \start
+add x29, x29, :lo12: \start
+
+adrp x30, \end
+add x30, x30, :lo12:\end
+
+/* x30 = end - begin */
+sub x30, x30, x29
+/* x28 = x30 >> 12 (number of pages) */
+mov x28, #12
+lsrv x28, x30, x28
+
+mov w0, #0x89
+movk w0, #0x8400, lsl #16
+mov x1, x29
+mov x2, x28
+mov w3, #\perm
+
+svc #0
+
+mov w1, #0x61
+movk w1, #0x8400, lsl #16
+cmp w1, w0
+b.ne .
+.endm
+
+  ASM_FUNC(_ModuleEntryPoint)
+MOV32 (w8, FixedPcdGet32(PcdFfaEnable))
+  cbz w8, FfaNotEnabled
+  // Stash boot information registers from the SPMC
+  mov x8, x0
+  mov x9, x1
+  mov x10, x2
+  mov x11, x3
+
+  // Set the correct permissions on stack memory
+  FfaMemPermSet StackBase StackEnd FFA_DATA_RW
+
+  // Initialise SP
+  adr x0, StackEnd
+  mov   sp, x0
+
+  // Restore boot information registers from the SPMC
+  mov x0, x8
+  mov x1, x9
+  mov x2, x10
+  mov x3, x11
+
+  // Invoke the C entrypoint
+  FfaNotEnabled:
+  b ModuleEntryPoint
-- 
2.34.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106794): https://edk2.groups.io/g/devel/message/106794
Mute This Topic: https://groups.io/mt/100079871/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [edk2-platforms][PATCH V1 01/20] ArmPkg: Change PcdFfaEnable flag datatype

2023-07-11 Thread Nishant Sharma
FeatureFlag type PCD flags are declared by typecasting an integer value
to BOOLEAN. These flags cannot be use in assembly code as assembler does
not recognise C primitive types. Change the flag data type from BOOLEAN
to UINT32.

Signed-off-by: Nishant Sharma 
---
 ArmPkg/ArmPkg.dec  
 | 14 +++---
 ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf
 |  4 ++--
 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
   |  4 ++--
 ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.c  
 |  8 
 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/Arm/StandaloneMmCoreEntryPoint.c
 |  8 
 5 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
index 1a16d044c94b..c36c23e2e059 100644
--- a/ArmPkg/ArmPkg.dec
+++ b/ArmPkg/ArmPkg.dec
@@ -155,13 +155,6 @@
   # hardware coherency (i.e., no virtualization or cache coherent DMA)
   
gArmTokenSpaceGuid.PcdNormalMemoryNonshareableOverride|FALSE|BOOLEAN|0x0043
 
-[PcdsFeatureFlag.AARCH64, PcdsFeatureFlag.ARM]
-  ## Used to select method for requesting services from S-EL1.
-  #   TRUE  - Selects FF-A calls for communication between S-EL0 and SPMC.
-  #   FALSE - Selects SVC calls for communication between S-EL0 and SPMC.
-  # @Prompt Enable FF-A support.
-  gArmTokenSpaceGuid.PcdFfaEnable|FALSE|BOOLEAN|0x005B
-
 [PcdsFixedAtBuild.common]
   gArmTokenSpaceGuid.PcdTrustzoneSupport|FALSE|BOOLEAN|0x0006
 
@@ -298,6 +291,13 @@
   # not currently supported.
   gArmTokenSpaceGuid.PcdArmNonSecModeTransition|0x3c9|UINT32|0x003E
 
+  ## Used to select method for requesting services from S-EL1.
+  #   1 - Selects FF-A calls for communication between S-EL0 and SPMC.
+  #   0 - Selects SVC calls for communication between S-EL0 and SPMC.
+  #   Using unsigned integer as boolean does not work on assembler.
+  # @Prompt Enable FF-A support.
+  gArmTokenSpaceGuid.PcdFfaEnable|0|UINT32|0x005B
+
 
 #
 # These PCDs are also defined as 'PcdsDynamic' or 'PcdsPatchableInModule' to be
diff --git a/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf 
b/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf
index ff20e5898051..3c733585f573 100644
--- a/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf
+++ b/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.inf
@@ -1,6 +1,6 @@
 #/** @file
 #
-#  Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.
+#  Copyright (c) 2017 - 2023, Arm Limited. All rights reserved.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -23,7 +23,7 @@
   ArmPkg/ArmPkg.dec
   MdePkg/MdePkg.dec
 
-[FeaturePcd.ARM, FeaturePcd.AARCH64]
+[FixedPcd]
   gArmTokenSpaceGuid.PcdFfaEnable
 
 [LibraryClasses]
diff --git 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
index 75cfb98c0e75..dc6d3d859911 100644
--- 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
+++ 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
@@ -1,7 +1,7 @@
 ## @file
 # Module entry point library for DXE core.
 #
-# Copyright (c) 2017 - 2021, Arm Ltd. All rights reserved.
+# Copyright (c) 2017 - 2023, Arm Ltd. All rights reserved.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -51,7 +51,7 @@
   gEfiStandaloneMmNonSecureBufferGuid
   gEfiArmTfCpuDriverEpDescriptorGuid
 
-[FeaturePcd.ARM, FeaturePcd.AARCH64]
+[FixedPcd]
   gArmTokenSpaceGuid.PcdFfaEnable
 
 #
diff --git a/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.c 
b/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.c
index d55aff76201e..1a41a289ef17 100644
--- a/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.c
+++ b/ArmPkg/Library/StandaloneMmMmuLib/ArmMmuStandaloneMmLib.c
@@ -1,7 +1,7 @@
 /** @file
   File managing the MMU for ARMv8 architecture in S-EL0
 
-  Copyright (c) 2017 - 2021, Arm Limited. All rights reserved.
+  Copyright (c) 2017 - 2023, Arm Limited. All rights reserved.
   Copyright (c) 2021, Linaro Limited
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -54,7 +54,7 @@ SendMemoryPermissionRequest (
   }
 
   ArmCallSvc (SvcArgs);
-  if (FeaturePcdGet (PcdFfaEnable)) {
+  if (FixedPcdGet32 (PcdFfaEnable) != 0) {
 // Get/Set memory attributes is an atomic call, with
 // StandaloneMm at S-EL0 being the caller and the SPM
 // core being the callee. Thus there won't be a
@@ -163,7 +163,7 @@ GetMemoryPermissions (
   // Prepare the message parameters.
   // See [1], Section 13.5.5.1 MM_SP_MEMORY_ATTRIBUTES_GET_AARCH64.
   ZeroMem (, sizeof (ARM_SVC_ARGS));
-  if (FeaturePcdGet (PcdFfaEnable)) {
+  if (FixedPcdGet32 (PcdFfaEnable) != 0) {
 // See [2], Section 10.2 FFA_MSG_SEND_DIRECT_REQ.
 SvcArgs.Arg0 = 

Re: [edk2-devel] Use gMmst from MmServiceTableLib in MmSaveStateLib

2023-07-11 Thread Ni, Ray
Abdul,
Can you please send a patch to fix MmSaveStateLib to use gMmst (instead of 
gSmst)?

Using gSmst forbids the lib instance be linked by standalone MM modules.

Thanks,
Ray

> -Original Message-
> From: Tan, Dun 
> Sent: Wednesday, July 5, 2023 4:42 PM
> To: devel@edk2.groups.io; abdat...@amd.com
> Cc: Ni, Ray 
> Subject: Use gMmst from MmServiceTableLib in MmSaveStateLib
> 
> Hi Abdul,
> 
> In the new MmSaveStateLib created in this patch set, gSmst from
> SmmServiceTableLib is used. This causes that only DXE_SMM_DRIVER type
> module can consume this lib but MM_STANDALONE type module cannot.
> 
> In current edk2, there are different MmServicesTableLib and
> SmmServicesTableLib:
> StadaloneMmServicesTableLib(MmServicesTableLib|MM_STANDALONE):
> initialize gMmst in standalone SMM env.
>   MmServicesTableLib(MmServicesTableLib|DXE_SMM_DRIVER):
> initialize gMmst in legacy SMM env.
> SmmServicesTableLib(SmmServicesTableLib|DXE_SMM_DRIVER): initialize gSmst
> in legacy SMM env.
> 
> If MmSaveStateLib uses gMmst from MmServiceTableLib instead of gSmst, then
> MmSaveStateLib can be consumed by both DXE_SMM_DRIVER and
> MM_STANDALONE module.
> Could you pls send patch to fix this?
> 
> Thanks,
> Dun
> 
> -Original Message-
> From: devel@edk2.groups.io  On Behalf Of Abdul Lateef
> Attar via groups.io
> Sent: Sunday, July 2, 2023 12:14 PM
> To: devel@edk2.groups.io
> Cc: Abdul Lateef Attar ; Paul Grimes
> ; Abner Chang ; Dong, Eric
> ; Ni, Ray ; Kumar, Rahul R
> ; Gerd Hoffmann ; Kinney,
> Michael D ; Gao, Liming
> ; Liu, Zhiguang ; Ard
> Biesheuvel ; Yao, Jiewen ;
> Justen, Jordan L 
> Subject: [edk2-devel] [PATCH v15 0/8] Adds AmdSmmCpuFeaturesLib and
> MmSaveStateLib
> 
> Backward-compatibility changes:
>   This patch series removes the SmmCpuFeaturesReadSaveStateRegister
>   and SmmCpuFeaturesWriteSaveStateRegister interface/function.
>   SmmReadSaveState() and SmmWriteSaveState() now directly invokes
> MmSaveStateLib
>   routines to save/restore registers.
> 
> PR: https://github.com/tianocore/edk2/pull/4597
> 
> V15: Delta changes
>   Rebase the branch and fix the merge conflicts.
> V14: Delta changes
>   Added @note to the MmSaveStateLib.h.
>   SaveState(Read/Write) of
> EFI_SMM_SAVE_STATE_REGISTER_PROCESSOR_ID/EFI_MM_SAVE_STATE_REGIS
> TER_PROCESSOR_ID
>   is handled by PiSmmCpuDxeSmm driver.
>   Fixed PatchCheck warnings.
> V13: Delta changes
>   Address review comments from Ray Ni
>   Changed the BASE _NAME of AmdSmmCpuFeaturesLib.
>   Removed EFIAPI from local function.
>   Removed CpuIndex parameter from MmSaveStateGetRegisterLma
>   Modifed MmSaveStateGetRegisterIndex () to accept RegOffset
> as second parameter.
>   Removed FILE_GUID library instance for intel implemention from
> UefiCpuPkg.dsc.
> V12:
>   Addressed review comments from Michael.
>   Added LibraryClasses to .inf file.
>   removed duplicate MACRO definations.
>   Moved related MACRO defination to respective file.
> V11: Delta changes
>   Drop the OVMF implementation of MmSaveStateLib
> V10: Delta changes:
>   Addressed review comments from Abner.
> V9: Delta changes:
>   Addressed review comments.
>   Rename to MmSaveStateLib.
>   Also rename SMM_ defines to MM_.
>   Implemented OVMF MmSaveStateLib.
>   Removes SmmCpuFeaturesReadSaveStateRegister and
> SmmCpuFeaturesWriteSaveStateRegister
>   function interface.
> V8 delta changes:
>Addressed review comments from Abner,
>Fix the whitespace error.
>Seperate the Ovmf changes to another patch
> V7 delta changes:
>Adds SmmSmramSaveStateLib for Intel processor.
>Integrate SmmSmramSaveStateLib library.
> V6 delta changes:
>Addressed review comments for Ray NI.
>removed unnecessary EFIAPI.
> V5 delta changes:
>rebase to master branch.
>updated Reviewed-by
> V4 delta changes:
>   rebase to master branch.
>   added reviewed-by.
> V3 delta changes:
>   Addressed review comments from Abner chang.
>   Re-arranged patch order.
> 
> Cc: Paul Grimes 
> Cc: Abner Chang 
> Cc: Eric Dong 
> Cc: Ray Ni 
> Cc: Rahul Kumar 
> Cc: Gerd Hoffmann 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Zhiguang Liu 
> Cc: Ard Biesheuvel 
> Cc: Jiewen Yao 
> Cc: Jordan Justen 
> Cc: Abdul Lateef Attar 
> 
> Abdul Lateef Attar (8):
>   MdePkg: Adds AMD SMRAM save state map
>   UefiCpuPkg: Adds MmSaveStateLib library class
>   UefiCpuPkg: Implements MmSaveStateLib library instance
>   UefiCpuPkg/SmmCpuFeaturesLib: Restructure arch-dependent code
>   UefiCpuPkg: Implements SmmCpuFeaturesLib for AMD Family
>   UefiCpuPkg: Implements MmSaveStateLib for Intel
>   UefiCpuPkg: Removes SmmCpuFeaturesReadSaveStateRegister
>   OvmfPkg: Uses MmSaveStateLib library
> 
>  UefiCpuPkg/UefiCpuPkg.dec |   4 +
>  OvmfPkg/OvmfPkgIa32.dsc   |   1 +
>  OvmfPkg/OvmfPkgIa32X64.dsc|   3 +
>  OvmfPkg/OvmfPkgX64.dsc|   1 +
>  UefiCpuPkg/UefiCpuPkg.dsc |  12 +
>  

[edk2-devel] [PATCH v2 3/3] OvmfPkg/BhyvePkg: enable bus enumeration

2023-07-11 Thread Corvin Köhne
bhyve supports adding a ROM to PCI devices. It was added to support GPU
passthrough of dedicated AMD GPUs. At the moment, this ROM file is
mostly useless as it's not shadowed and executed by firmware. Change
that by enabling bus enumeration.

Signed-off-by: Corvin Köhne 
Acked-by: Peter Grehan 
Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc: Gerd Hoffmann 
Cc: Rebecca Cran 
---
 OvmfPkg/Bhyve/BhyveX64.dsc | 1 -
 1 file changed, 1 deletion(-)

diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
index 873eec60906e..82c60ace1bbd 100644
--- a/OvmfPkg/Bhyve/BhyveX64.dsc
+++ b/OvmfPkg/Bhyve/BhyveX64.dsc
@@ -428,7 +428,6 @@ [PcdsFeatureFlag]
 !endif
 
 [PcdsFixedAtBuild]
-  gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration|TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|FALSE
   gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseMemory|TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1
-- 
2.41.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106788): https://edk2.groups.io/g/devel/message/106788
Mute This Topic: https://groups.io/mt/100074289/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH v2 1/3] Revert "OvmfPkg/Bhyve: consume PciHostBridgeLibScan"

2023-07-11 Thread Corvin Köhne
We like to enable bus enumartion for bhyve. Therefore, this patch needs
to be reverted.

This reverts commit c2f24ba3218ae91a8d5a1a31c31dad3417850d0c.

Signed-off-by: Corvin Köhne 
Acked-by: Peter Grehan 
Cc: Ard Biesheuvel 
Cc: Gerd Hoffmann 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc: Rebecca Cran 
---
 OvmfPkg/Bhyve/BhyveX64.dsc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
index 7fa40998ae80..fd3f8470f2c6 100644
--- a/OvmfPkg/Bhyve/BhyveX64.dsc
+++ b/OvmfPkg/Bhyve/BhyveX64.dsc
@@ -650,7 +650,7 @@ [Components]
   OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
   MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf {
 
-  
PciHostBridgeLib|OvmfPkg/Library/PciHostBridgeLibScan/PciHostBridgeLibScan.inf
+  PciHostBridgeLib|OvmfPkg/Library/PciHostBridgeLib/PciHostBridgeLib.inf
   
PciHostBridgeUtilityLib|OvmfPkg/Library/PciHostBridgeUtilityLib/PciHostBridgeUtilityLib.inf
   NULL|OvmfPkg/Library/PlatformHasIoMmuLib/PlatformHasIoMmuLib.inf
   }
-- 
2.41.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106786): https://edk2.groups.io/g/devel/message/106786
Mute This Topic: https://groups.io/mt/100074283/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH v2 2/3] Revert "OvmfPkg/Bhyve: remove IncompatiblePciDeviceSupport DXE driver"

2023-07-11 Thread Corvin Köhne
We like to enable bus enumeration for bhyve. Therefore, this patch needs
to be reverted.

This reverts commit 8c8f886f27556f2fb6e8b502d32aa9ccee930acc.

Signed-off-by: Corvin Köhne 
Acked-by: Peter Grehan 
Cc: Ard Biesheuvel 
Cc: Jiewen Yao 
Cc: Jordan Justen 
Cc: Gerd Hoffmann 
Cc: Rebecca Cran 
---
 OvmfPkg/Bhyve/BhyveX64.dsc | 1 +
 OvmfPkg/Bhyve/BhyveX64.fdf | 1 +
 2 files changed, 2 insertions(+)

diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc
index fd3f8470f2c6..873eec60906e 100644
--- a/OvmfPkg/Bhyve/BhyveX64.dsc
+++ b/OvmfPkg/Bhyve/BhyveX64.dsc
@@ -647,6 +647,7 @@ [Components]
   UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
   UefiCpuPkg/CpuDxe/CpuDxe.inf
   PcAtChipsetPkg/HpetTimerDxe/HpetTimerDxe.inf
+  OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
   OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
   MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf {
 
diff --git a/OvmfPkg/Bhyve/BhyveX64.fdf b/OvmfPkg/Bhyve/BhyveX64.fdf
index c62d5757092e..282586fa81ec 100644
--- a/OvmfPkg/Bhyve/BhyveX64.fdf
+++ b/OvmfPkg/Bhyve/BhyveX64.fdf
@@ -207,6 +207,7 @@ [FV.DXEFV]
 INF  UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.inf
 INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
 INF  PcAtChipsetPkg/HpetTimerDxe/HpetTimerDxe.inf
+INF  OvmfPkg/IncompatiblePciDeviceSupportDxe/IncompatiblePciDeviceSupport.inf
 INF  OvmfPkg/PciHotPlugInitDxe/PciHotPlugInit.inf
 INF  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
 INF  MdeModulePkg/Bus/Pci/PciBusDxe/PciBusDxe.inf
-- 
2.41.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106787): https://edk2.groups.io/g/devel/message/106787
Mute This Topic: https://groups.io/mt/100074284/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




[edk2-devel] [PATCH v2 0/3] OvmfPkg/Bhyve: enable bus enumeration

2023-07-11 Thread Corvin Köhne
CI: https://github.com/tianocore/edk2/pull/4543

Corvin Köhne (3):
  Revert "OvmfPkg/Bhyve: consume PciHostBridgeLibScan"
  Revert "OvmfPkg/Bhyve: remove IncompatiblePciDeviceSupport DXE driver"
  OvmfPkg/BhyvePkg: enable bus enumeration

 OvmfPkg/Bhyve/BhyveX64.dsc | 4 ++--
 OvmfPkg/Bhyve/BhyveX64.fdf | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

-- 
2.41.0



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106785): https://edk2.groups.io/g/devel/message/106785
Mute This Topic: https://groups.io/mt/100074281/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] ArmVirtPkg: non-executable EFI_LOADER_DATA breaks GRUB on Ubuntu 22.04

2023-07-11 Thread Gerd Hoffmann
On Mon, Jul 10, 2023 at 04:58:15PM +0100, Pedro Falcato wrote:
> On Mon, Jul 10, 2023 at 2:28 PM  wrote:
> >
> > I have an existing install of Ubuntu 22.04 on a QEMU virtual machine which 
> > I've decided to update the UEFI firmware. After doing so, GRUB no longer 
> > boots ("Synchronous Exception" message seen). After a git bisect session, I 
> > found the problematic 2997ae38739756ecba9b0de19e86032ebc689ef9. The comment 
> > says GRUB should have been fixed in 2017, but for one reason or another, my 
> > VM which was built in 2022 still had the issue. Regardless, I don't think 
> > it's a good idea to break GRUB, even if it's fixed in 2017. In the very 
> > least, a better error message would be preferable to crashing with an 
> > "Synchronous Exception." Googling this error message shows that other 
> > people may be hitting this issue as well but the vague error symptom means 
> > its impossible to know if it's the same issue or not.
> 
> +CC Some of the folks involved in the original discussion
> 
> In the original thread, people discussed some alternative behavior to
> just crashing on a NX fault. Is this still an alternative?

The idea is: Improve page fault handler to (a) print a big'n'fat
warning, and (b) loosening up memory permissions for the faulting
page address.

No patch for that emerged (yet?).

> I'm kind of thinking this should be addressed by distros anyway
> How is $CURRENT_YEAR Ubuntu still shipping bad GRUBs? I know the
> situation around GRUB and distro patching is complicated but...
> Do we have any idea of how many distros/GRUBs are affected by this?

Too many :(

> Personally, I would like to avoid loosening up memory permissions.

Well, you can't have both.  You have to pick between strict nx handling
and grub bug compatibility ...

take care,
  Gerd



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#106784): https://edk2.groups.io/g/devel/message/106784
Mute This Topic: https://groups.io/mt/100057351/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH] UefiPayloadPkg: Integrate UiApp and BootManagerMenuApp into MultiFv

2023-07-11 Thread Lu, James
Reviewed-by: James Lu 


Thanks,
James

-Original Message-
From: Lin, MarsX  
Sent: Monday, July 10, 2023 5:02 PM
To: devel@edk2.groups.io
Cc: Lin, MarsX ; Dong, Guo ; Ni, Ray 
; Rhodes, Sean ; Lu, James 
; Guo, Gua 
Subject: [PATCH] UefiPayloadPkg: Integrate UiApp and BootManagerMenuApp into 
MultiFv

From: MarsX Lin 

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

To put UiApp.inf and BootManagerMenuApp.inf to proper FV(BDSFV)

Cc: Guo Dong 
Cc: Ray Ni 
Cc: Sean Rhodes 
Cc: James Lu 
Cc: Gua Guo 

Signed-off-by: MarsX Lin 
---
 .../PlatformBootManager.c | 55 ---
 .../PlatformBootManagerLib.inf|  4 --
 UefiPayloadPkg/UefiPayloadEntry/PrintHob.c| 25 +
 .../UniversalPayloadEntry.inf |  1 -
 UefiPayloadPkg/UefiPayloadPkg.dec |  2 -
 UefiPayloadPkg/UefiPayloadPkg.dsc |  8 +--
 UefiPayloadPkg/UefiPayloadPkg.fdf |  9 ++-
 7 files changed, 11 insertions(+), 93 deletions(-)

diff --git 
a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.c 
b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
index 4a0219624d..a4a49da0e9 100644
--- a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
+++ b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManager.c
@@ -9,8 +9,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 

 #include "PlatformBootManager.h"

 #include "PlatformConsole.h"

-#include 

-#include 

 #include 

 

 /**

@@ -364,56 +362,3 @@ PlatformBootManagerUnableToBoot (
 {

   return;

 }

-

-/**

-  Get/update PcdBootManagerMenuFile from GUID HOB which will be assigned in 
bootloader.

-

-  @param  ImageHandle   The firmware allocated handle for the EFI image.

-  @param  SystemTable   A pointer to the EFI System Table.

-

-  @retval EFI_SUCCESS   The entry point is executed successfully.

-  @retval other Some error occurs.

-

-**/

-EFI_STATUS

-EFIAPI

-PlatformBootManagerLibConstructor (

-  IN EFI_HANDLEImageHandle,

-  IN EFI_SYSTEM_TABLE  *SystemTable

-  )

-{

-  EFI_STATUS   Status;

-  UINTNSize;

-  VOID *GuidHob;

-  UNIVERSAL_PAYLOAD_GENERIC_HEADER *GenericHeader;

-  UNIVERSAL_PAYLOAD_BOOT_MANAGER_MENU  *BootManagerMenuFile;

-

-  GuidHob = GetFirstGuidHob ();

-

-  if (GuidHob == NULL) {

-//

-// If the HOB is not create, the default value of PcdBootManagerMenuFile 
will be used.

-//

-return EFI_SUCCESS;

-  }

-

-  GenericHeader = (UNIVERSAL_PAYLOAD_GENERIC_HEADER *)GET_GUID_HOB_DATA 
(GuidHob);

-  if ((sizeof (UNIVERSAL_PAYLOAD_GENERIC_HEADER) > GET_GUID_HOB_DATA_SIZE 
(GuidHob)) || (GenericHeader->Length > GET_GUID_HOB_DATA_SIZE (GuidHob))) {

-return EFI_NOT_FOUND;

-  }

-

-  if (GenericHeader->Revision == UNIVERSAL_PAYLOAD_BOOT_MANAGER_MENU_REVISION) 
{

-BootManagerMenuFile = (UNIVERSAL_PAYLOAD_BOOT_MANAGER_MENU 
*)GET_GUID_HOB_DATA (GuidHob);

-if (BootManagerMenuFile->Header.Length < 
UNIVERSAL_PAYLOAD_SIZEOF_THROUGH_FIELD (UNIVERSAL_PAYLOAD_BOOT_MANAGER_MENU, 
FileName)) {

-  return EFI_NOT_FOUND;

-}

-

-Size   = sizeof (BootManagerMenuFile->FileName);

-Status = PcdSetPtrS (PcdBootManagerMenuFile, , 
>FileName);

-ASSERT_EFI_ERROR (Status);

-  } else {

-return EFI_NOT_FOUND;

-  }

-

-  return EFI_SUCCESS;

-}

diff --git 
a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf 
b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index a3951b7a7e..ff92c95227 100644
--- a/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/UefiPayloadPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -13,7 +13,6 @@
   MODULE_TYPE= DXE_DRIVER

   VERSION_STRING = 1.0

   LIBRARY_CLASS  = PlatformBootManagerLib|DXE_DRIVER

-  CONSTRUCTOR= PlatformBootManagerLibConstructor

 

 #

 # The following information is for reference only and not required by the 
build tools.

@@ -48,11 +47,9 @@
   HiiLib

   PrintLib

   PlatformHookLib

-  HobLib

 

 [Guids]

   gEfiEndOfDxeEventGroupGuid

-  gEdkiiBootManagerMenuFileGuid

   gUefiShellFileGuid

 

 [Protocols]

@@ -75,5 +72,4 @@
   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits

   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity

   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits

-  gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile

   gUefiPayloadPkgTokenSpaceGuid.PcdBootManagerEscape

diff --git a/UefiPayloadPkg/UefiPayloadEntry/PrintHob.c 
b/UefiPayloadPkg/UefiPayloadEntry/PrintHob.c
index e959be5d95..3b2b7788d4 100644
--- a/UefiPayloadPkg/UefiPayloadEntry/PrintHob.c
+++ b/UefiPayloadPkg/UefiPayloadEntry/PrintHob.c
@@ -10,7 +10,6 @@
 #include 

 #include 

 #include 

-#include 

 

 #define ROW_LIMITER  16

 

@@ -437,27