diff mbox series

[edk2,edk2-platforms] Platform/ARM: remove ReportStatusCodeLib resolutions

Message ID 20171016180347.22930-1-ard.biesheuvel@linaro.org
State New
Headers show
Series [edk2,edk2-platforms] Platform/ARM: remove ReportStatusCodeLib resolutions | expand

Commit Message

Ard Biesheuvel Oct. 16, 2017, 6:03 p.m. UTC 
The generic ResetSystemRuntimeDxe may invoke ReportStatusCodeLib, and
this may happen at runtime. If the chosen resolution is not suitable
for runtime, this will result in a crash.

Given that we don't actually use status codes, let's just switch to
the NULL instance for all modules and be done with it.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

-- 
2.11.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Gao, Liming Oct. 17, 2017, 12:53 a.m. UTC | #1 
Ard:
  MdeModulePkg\Library\RuntimeDxeReportStatusCodeLib\RuntimeDxeReportStatusCodeLib.inf is designed for Runtime driver. If you require StatusCode at runtime, you can use this library instance. 

Thanks
Liming
> -----Original Message-----

> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel

> Sent: Tuesday, October 17, 2017 2:04 AM

> To: edk2-devel@lists.01.org; leif.lindholm@linaro.org

> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; sudeep.holla@arm.com

> Subject: [edk2] [PATCH edk2-platforms] Platform/ARM: remove ReportStatusCodeLib resolutions

> 

> The generic ResetSystemRuntimeDxe may invoke ReportStatusCodeLib, and

> this may happen at runtime. If the chosen resolution is not suitable

> for runtime, this will result in a crash.

> 

> Given that we don't actually use status codes, let's just switch to

> the NULL instance for all modules and be done with it.

> 

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---

>  Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc | 6 +-----

>  1 file changed, 1 insertion(+), 5 deletions(-)

> 

> diff --git a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc

> index 8bcb84869c84..b758c58c9872 100644

> --- a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc

> +++ b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc

> @@ -153,7 +153,7 @@

>    CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf

>    CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf

> 

> -  ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf

> +  ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf

> 

>  [LibraryClasses.common.SEC]

>    ArmPlatformSecExtraActionLib|ArmPlatformPkg/Library/DebugSecExtraActionLib/DebugSecExtraActionLib.inf

> @@ -182,7 +182,6 @@

>    MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf

>    PeiCoreEntryPoint|MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.inf

>    PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf

> -  OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf

>    PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf

>    ExtractGuidedSectionLib|MdePkg/Library/PeiExtractGuidedSectionLib/PeiExtractGuidedSectionLib.inf

> 

> @@ -195,7 +194,6 @@

>    MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf

>    PeimEntryPoint|MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf

>    PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf

> -  OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf

>    PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf

>    PeiResourcePublicationLib|MdePkg/Library/PeiResourcePublicationLib/PeiResourcePublicationLib.inf

>    ExtractGuidedSectionLib|MdePkg/Library/PeiExtractGuidedSectionLib/PeiExtractGuidedSectionLib.inf

> @@ -358,8 +356,6 @@

>    #  DEBUG_ERROR     0x80000000  // Error

>    gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8000000F

> 

> -  gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07

> -

>    gEmbeddedTokenSpaceGuid.PcdEmbeddedAutomaticBootCommand|""

>    gEmbeddedTokenSpaceGuid.PcdEmbeddedDefaultTextColor|0x07

>    gEmbeddedTokenSpaceGuid.PcdEmbeddedMemVariableStoreSize|0x10000

> --

> 2.11.0

> 

> _______________________________________________

> edk2-devel mailing list

> edk2-devel@lists.01.org

> https://lists.01.org/mailman/listinfo/edk2-devel

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Alexei Fedorov Oct. 17, 2017, 8:41 a.m. UTC | #2 
Ard,


BaseReportStatusCodeLibNull.inf was replaced with DxeReportStatusCodeLib.inf to support storing boot performance data required for FPDT ACPI table.

As Liming already mentioned ResetSystemRuntimeDxe.inf should be linked to RuntimeDxeReportStatusCodeLib library:


  MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf {
    <LibraryClasses>
      ReportStatusCodeLib|MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/RuntimeDxeReportStatusCodeLib.inf
  }

Alexei.

________________________________
From: edk2-devel <edk2-devel-bounces@lists.01.org> on behalf of Gao, Liming <liming.gao@intel.com>

Sent: 17 October 2017 01:53:46
To: Ard Biesheuvel; edk2-devel@lists.01.org; leif.lindholm@linaro.org
Cc: Sudeep Holla
Subject: Re: [edk2] [PATCH edk2-platforms] Platform/ARM: remove ReportStatusCodeLib resolutions

Ard:
  MdeModulePkg\Library\RuntimeDxeReportStatusCodeLib\RuntimeDxeReportStatusCodeLib.inf is designed for Runtime driver. If you require StatusCode at runtime, you can use this library instance.

Thanks
Liming
> -----Original Message-----

> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard Biesheuvel

> Sent: Tuesday, October 17, 2017 2:04 AM

> To: edk2-devel@lists.01.org; leif.lindholm@linaro.org

> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; sudeep.holla@arm.com

> Subject: [edk2] [PATCH edk2-platforms] Platform/ARM: remove ReportStatusCodeLib resolutions

>

> The generic ResetSystemRuntimeDxe may invoke ReportStatusCodeLib, and

> this may happen at runtime. If the chosen resolution is not suitable

> for runtime, this will result in a crash.

>

> Given that we don't actually use status codes, let's just switch to

> the NULL instance for all modules and be done with it.

>

> Contributed-under: TianoCore Contribution Agreement 1.1

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---

>  Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc | 6 +-----

>  1 file changed, 1 insertion(+), 5 deletions(-)

>

> diff --git a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc

> index 8bcb84869c84..b758c58c9872 100644

> --- a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc

> +++ b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc

> @@ -153,7 +153,7 @@

>    CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf

>    CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf

>

> -  ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf

> +  ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf

>

>  [LibraryClasses.common.SEC]

>    ArmPlatformSecExtraActionLib|ArmPlatformPkg/Library/DebugSecExtraActionLib/DebugSecExtraActionLib.inf

> @@ -182,7 +182,6 @@

>    MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf

>    PeiCoreEntryPoint|MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.inf

>    PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf

> -  OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf

>    PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf

>    ExtractGuidedSectionLib|MdePkg/Library/PeiExtractGuidedSectionLib/PeiExtractGuidedSectionLib.inf

>

> @@ -195,7 +194,6 @@

>    MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf

>    PeimEntryPoint|MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf

>    PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf

> -  OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf

>    PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf

>    PeiResourcePublicationLib|MdePkg/Library/PeiResourcePublicationLib/PeiResourcePublicationLib.inf

>    ExtractGuidedSectionLib|MdePkg/Library/PeiExtractGuidedSectionLib/PeiExtractGuidedSectionLib.inf

> @@ -358,8 +356,6 @@

>    #  DEBUG_ERROR     0x80000000  // Error

>    gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8000000F

>

> -  gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07

> -

>    gEmbeddedTokenSpaceGuid.PcdEmbeddedAutomaticBootCommand|""

>    gEmbeddedTokenSpaceGuid.PcdEmbeddedDefaultTextColor|0x07

>    gEmbeddedTokenSpaceGuid.PcdEmbeddedMemVariableStoreSize|0x10000

> --

> 2.11.0

>

> _______________________________________________

> edk2-devel mailing list

> edk2-devel@lists.01.org

> https://lists.01.org/mailman/listinfo/edk2-devel

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Oct. 17, 2017, 8:42 a.m. UTC | #3 
On 17 October 2017 at 09:41, Alexei Fedorov <Alexei.Fedorov@arm.com> wrote:
> Ard,

>

>

> BaseReportStatusCodeLibNull.inf was replaced with DxeReportStatusCodeLib.inf

> to support storing boot performance data required for FPDT ACPI table.

>

> As Liming already mentioned ResetSystemRuntimeDxe.inf should be linked to

> RuntimeDxeReportStatusCodeLib library:

>

>

>   MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf {

>     <LibraryClasses>

>

> ReportStatusCodeLib|MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib/RuntimeDxeReportStatusCodeLib.inf

>   }

>


I know how it works. I just was not aware that we used status codes
for anything.

Thanks,
Ard.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Oct. 17, 2017, 8:43 a.m. UTC | #4 
On 17 October 2017 at 01:53, Gao, Liming <liming.gao@intel.com> wrote:
> Ard:

>   MdeModulePkg\Library\RuntimeDxeReportStatusCodeLib\RuntimeDxeReportStatusCodeLib.inf is designed for Runtime driver. If you require StatusCode at runtime, you can use this library instance.

>


Thanks Liming.

I wrongly assumed that these platforms have no use for status code,
but apparently, it is required for the FPDT table?
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Sudeep Holla Oct. 17, 2017, 10:24 a.m. UTC | #5 
On 16/10/17 19:03, Ard Biesheuvel wrote:
> The generic ResetSystemRuntimeDxe may invoke ReportStatusCodeLib, and

> this may happen at runtime. If the chosen resolution is not suitable

> for runtime, this will result in a crash.

> 

> Given that we don't actually use status codes, let's just switch to

> the NULL instance for all modules and be done with it.

> 


I see there's some active discussion yet. But FWIW,

Tested-by: Sudeep Holla <sudeep.holla@arm.com>


-- 
Regards,
Sudeep
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Gao, Liming Oct. 17, 2017, 3:05 p.m. UTC | #6 
Ard:
  MdeModulePkg\Universal\Acpi\FirmwarePerformanceDataTableDxe depends on some status code to fill basic boot FPDT record in FPDT table. Those status codes are reported from DxeCore. For FPDT table, DxeCore must link the real report status code. It links DXE version library instance. 

Thanks
Liming
> -----Original Message-----

> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

> Sent: Tuesday, October 17, 2017 4:44 PM

> To: Gao, Liming <liming.gao@intel.com>

> Cc: edk2-devel@lists.01.org; leif.lindholm@linaro.org; sudeep.holla@arm.com

> Subject: Re: [edk2] [PATCH edk2-platforms] Platform/ARM: remove ReportStatusCodeLib resolutions

> 

> On 17 October 2017 at 01:53, Gao, Liming <liming.gao@intel.com> wrote:

> > Ard:

> >   MdeModulePkg\Library\RuntimeDxeReportStatusCodeLib\RuntimeDxeReportStatusCodeLib.inf is designed for Runtime driver. If

> you require StatusCode at runtime, you can use this library instance.

> >

> 

> Thanks Liming.

> 

> I wrongly assumed that these platforms have no use for status code,

> but apparently, it is required for the FPDT table?

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Oct. 17, 2017, 3:21 p.m. UTC | #7 
On 17 October 2017 at 16:05, Gao, Liming <liming.gao@intel.com> wrote:
> Ard:

>   MdeModulePkg\Universal\Acpi\FirmwarePerformanceDataTableDxe depends on some status code to fill basic boot FPDT record in FPDT table. Those status codes are reported from DxeCore. For FPDT table, DxeCore must link the real report status code. It links DXE version library instance.

>


Thanks for the explanation.

-- 
Ard.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox series

Patch

diff --git a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
index 8bcb84869c84..b758c58c9872 100644
--- a/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
+++ b/Platform/ARM/VExpressPkg/ArmVExpress.dsc.inc
@@ -153,7 +153,7 @@ 
   CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
   CustomizedDisplayLib|MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.inf
 
-  ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
+  ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf
 
 [LibraryClasses.common.SEC]
   ArmPlatformSecExtraActionLib|ArmPlatformPkg/Library/DebugSecExtraActionLib/DebugSecExtraActionLib.inf
@@ -182,7 +182,6 @@ 
   MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf
   PeiCoreEntryPoint|MdePkg/Library/PeiCoreEntryPoint/PeiCoreEntryPoint.inf
   PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf
-  OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf
   ExtractGuidedSectionLib|MdePkg/Library/PeiExtractGuidedSectionLib/PeiExtractGuidedSectionLib.inf
 
@@ -195,7 +194,6 @@ 
   MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf
   PeimEntryPoint|MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
   PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf
-  OemHookStatusCodeLib|MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
   PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf
   PeiResourcePublicationLib|MdePkg/Library/PeiResourcePublicationLib/PeiResourcePublicationLib.inf
   ExtractGuidedSectionLib|MdePkg/Library/PeiExtractGuidedSectionLib/PeiExtractGuidedSectionLib.inf
@@ -358,8 +356,6 @@ 
   #  DEBUG_ERROR     0x80000000  // Error
   gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8000000F
 
-  gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x07
-
   gEmbeddedTokenSpaceGuid.PcdEmbeddedAutomaticBootCommand|""
   gEmbeddedTokenSpaceGuid.PcdEmbeddedDefaultTextColor|0x07
   gEmbeddedTokenSpaceGuid.PcdEmbeddedMemVariableStoreSize|0x10000