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

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

[Ard Biesheuvel]

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

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

[Gao, Liming]

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

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

[Alexei Fedorov]

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.

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

[Ard Biesheuvel]

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.

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

[Ard Biesheuvel]

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?

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

[Sudeep Holla]

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

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

[Gao, Liming]

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?

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

[Ard Biesheuvel]

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.