Hi Sami, Thanks for reviewing this patch series. Please find my response inline.
Regards, Omkar > Hi Omkar, > > How did you check that the HEST table is populated correctly? > > There is no HEST parser in Acpiview at the moment. Do you plan to add an > HEST parser to Acpiview? > > Regards, > > Sami Mujawar > Yes, we will follow up on the HEST parser patch and post it soon. - Omkar > > On 02/08/2021 01:49 PM, Sami Mujawar wrote: > > Hi Omkar, > > > > Thank you for this patch series and for the clear explaination below. > > > > The explaination below is very useful for anyone who is trying to > > understand the code. > > > > Since the cover letter will not be part of the patch commit messages, > > would it be possible to include this explanation: > > > > 1. as part of a commit message for one of the patches in this series > > (patch 2/4 or 3/4). > > > > Or > > > > 2. in a Readme.md file > > > > Regards, > > > > Sami Mujawar > > Ack. - Omkar > > > > On 10/07/2021 05:18 PM, Omkar Anand Kulkarni wrote: > >> Changes since v1: > >> - Helper added for HEST ACPI table generation. > >> - Rebased to the latest upstream code. > >> > >> Hardware Error Source Table (HEST)[1] and Software Delegated > >> Exception Interface (SDEI)[2] ACPI tables are used to acomplish > >> firmware first error handling.This patch series introduces a > >> framework to build and install the HEST ACPI table dynamically. > >> > >> The following figure illustrates the possible usage of the dyanamic > >> generation of HEST ACPI table. > >> > >> NS | S > >> +--------------------------------------+--------------------------------------+ > >> > >> | | | > >> |+-------------------------------------+---------------------+ | > >> || +---------------------+--------------------+| | > >> || | | || | > >> || +-----------+ |+------------------+ | +-----------------+|| > >> +-------------+| > >> || |HestTable | || HestErrorSource | | | HestErrorSource ||| | > >> DMC-620 || > >> || | DXE | || DXE | | | StandaloneMM ||| > >> |Standalone MM|| > >> || +-----------+ |+------------------+ | +-----------------+|| > >> +-------------+| > >> || |GHESv2 | || | > >> || +---------------------+--------------------+| | > >> || +--------------------+ | | | > >> || |PlatformErrorHandler| | | | > >> || | DXE | | | | > >> || +--------------------+ | | | > >> ||FF FWK | | | > >> |+-------------------------------------+---------------------+ | > >> | | | > >> +--------------------------------------+--------------------------------------+ > >> > >> | > >> Figure: Firmware First Error Handling approach. > >> > >> All the hardware error sources are added to HEST table as GHESv2[3] > >> error source descriptors. The framework comprises of following DXE > >> and MM drivers: > >> > >> - HestTableDxe: > >> Builds HEST table header and allows appending error source > >> descriptors to the > >> HEST table. Also provides protocol interface to install the built > >> HEST table. > >> > >> - HestErrorSourceDxe & HestErrorSourceStandaloneMM: > >> These two drivers together retrieve all possible error source > >> descriptors of > >> type GHESv2 from the MM drivers implementing HEST Error Source > >> Descriptor > >> protocol. Once all the descriptors are collected > >> HestErrorSourceDxe appends > >> it to HEST table using HestTableDxe driver. > >> - PlatformErrorHandlerDxe: > >> Builds and installs SDEI ACPI table. This driver does not > >> initialize(load) > >> until HestErrorSourceDxe driver has finished appending all > >> possible GHESv2 > >> error source descriptors to the HEST table. Once that is complete > >> using the > >> HestTableDxe driver it installs the HEST table. > >> > >> This patch series provides reference implementation for DMC-620 > >> Dynamic Memory Controller[4] that has RAS feature enabled. This is > >> platform code implemented as Standalone MM driver in edk2-platforms. > >> > >> References: > >> [1] : ACPI 6.3, Table 18-382, Hardware Error Source Table [2] : SDEI > >> Platform Design Document, revision b, 10 Appendix C, ACPI table > >> definitions for SDEI > >> [3] : ACPI Reference Specification 6.3, Table 18-393 GHESv2 Structure > >> [4] : DMC620 Dynamic Memory Controller, revision r1p0 [5] : UEFI > >> Reference Specification 2.8, Appendix N - Common Platform Error > >> Record > >> [6] : UEFI Reference Specification 2.8, Section N.2.5 Memory Error > >> Section > >> > >> Link to github branch with the patches in this series - > >> https://github.com/omkkul01/edk2/tree/ras_firmware_first_edk2 > >> > >> Omkar Anand Kulkarni (4): > >> ArmPlatformPkg: Allow dynamic generation of HEST ACPI table > >> ArmPlatformPkg: add definition for > MM_HEST_ERROR_SOURCE_DESC_PROTOCOL > >> ArmPlatformPkg: retreive error source descriptors from MM > >> ArmPlatformPkg: Add helpers for HEST table generation > >> > >> ArmPlatformPkg/ArmPlatformPkg.dec | 12 + > >> .../Drivers/Apei/HestDxe/HestDxe.inf | 49 +++ > >> .../HestMmErrorSources/HestErrorSourceDxe.inf | 44 +++ > >> .../HestErrorSourceStandaloneMm.inf | 51 +++ > >> .../HestMmErrorSourceCommon.h | 37 ++ > >> ArmPlatformPkg/Include/HestAcpiHeader.h | 49 +++ > >> .../Include/Protocol/HestErrorSourceInfo.h | 64 ++++ > >> ArmPlatformPkg/Include/Protocol/HestTable.h | 71 ++++ > >> ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.c | 354 > ++++++++++++++++++ > >> .../HestMmErrorSources/HestErrorSourceDxe.c | 308 +++++++++++++++ > >> .../HestErrorSourceStandaloneMm.c | 312 +++++++++++++++ > >> 11 files changed, 1351 insertions(+) > >> create mode 100644 ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.inf > >> create mode 100644 > >> ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf > >> create mode 100644 > >> > ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandalone > Mm.inf > >> create mode 100644 > >> > ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmErrorSourceCommo > n.h > >> create mode 100644 ArmPlatformPkg/Include/HestAcpiHeader.h > >> create mode 100644 > >> ArmPlatformPkg/Include/Protocol/HestErrorSourceInfo.h > >> create mode 100644 ArmPlatformPkg/Include/Protocol/HestTable.h > >> create mode 100644 ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.c > >> create mode 100644 > >> ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c > >> create mode 100644 > >> > ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandalone > Mm > >> .c > >> > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#79736): https://edk2.groups.io/g/devel/message/79736 Mute This Topic: https://groups.io/mt/84115237/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
