Re: [edk2-devel] [PATCH 1/1] MdeModulePkg/Bus/Pci/EhciDxe: Fix FORWARD_NULL Coverity issues
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
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
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
[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
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
[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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
[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
[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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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"
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"
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
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
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
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