Re: [PATCH v3 1/5] RAS: Report all ARM processor CPER information to userspace

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Sep 04, 2024 at 08:07:14AM +0200, Mauro Carvalho Chehab wrote:
> From: Jason Tian <jason@xxxxxxxxxxxxxxxxxxxxxx>
> 
> The ARM processor CPER record was added at UEFI 2.6, and hasn't
> any changes up to UEFI 2.10 on its struct.

Did some fixups to the commit message, please use it in your next
submission.

There was also this typo:

Co-developed-by: Signed-off-by: Shengwei Luo <luoshengwei@xxxxxxxxxx>

From: Jason Tian <jason@xxxxxxxxxxxxxxxxxxxxxx>

    RAS: Report all ARM processor CPER information to userspace
    
    The ARM processor CPER record was added in UEFI v2.6 and remained
    unchanged up to v2.10.
    
    Yet, the original arm_event trace code added by
    
      e9279e83ad1f ("trace, ras: add ARM processor error trace event")
    
    is incomplete, as it only traces some fields of UAPI 2.6 table N.16, not
    exporting any information from tables N.17 to N.29 of the record.
    
    This is not enough for the user to be able to figure out what has
    exactly happened or to take appropriate action.
    
    According to the UEFI v2.9 specification chapter N2.4.4, the ARM
    processor error section includes:
    
    - several (ERR_INFO_NUM) ARM processor error information structures
      (Tables N.17 to N.20);
    - several (CONTEXT_INFO_NUM) ARM processor context information
      structures (Tables N.21 to N.29);
    - several vendor specific error information structures. The
      size is given by Section Length minus the size of the other
      fields.
    
    In addition, it also exports two fields that are parsed by the GHES
    driver when firmware reports it, e.g.:
    
    - error severity
    - CPU logical index
    
    Report all of these information to userspace via a the ARM tracepoint so
    that userspace can properly record the error and take decisions related
    to CPU core isolation according to error severity and other info.
    
    The updated ARM trace event now contains the following fields:
    
    ======================================  =============================
    UEFI field on table N.16                ARM Processor trace fields
    ======================================  =============================
    Validation                              handled when filling data for
                                            affinity MPIDR and running
                                            state.
    ERR_INFO_NUM                            pei_len
    CONTEXT_INFO_NUM                        ctx_len
    Section Length                          indirectly reported by
                                            pei_len, ctx_len and oem_len
    Error affinity level                    affinity
    MPIDR_EL1                               mpidr
    MIDR_EL1                                midr
    Running State                           running_state
    PSCI State                              psci_state
    Processor Error Information Structure   pei_err - count at pei_len
    Processor Context                       ctx_err- count at ctx_len
    Vendor Specific Error Info              oem - count at oem_len
    ======================================  =============================
    
    It should be noted that decoding of tables N.17 to N.29, if needed, will
    be handled in userspace. That gives more flexibility, as there won't be
    any need to flood the kernel with micro-architecture specific error
    decoding.
    
    Also, decoding the other fields require a complex logic, and should be
    done for each of the several values inside the record field.  So, let
    userspace daemons like rasdaemon decode them, parsing such tables and
    having vendor-specific micro-architecture-specific decoders.
    
      [mchehab: modified description, solved merge conflicts and fixed coding style]
    
    Fixes: e9279e83ad1f ("trace, ras: add ARM processor error trace event")
    Co-developed-by: Jason Tian <jason@xxxxxxxxxxxxxxxxxxxxxx>
    Signed-off-by: Jason Tian <jason@xxxxxxxxxxxxxxxxxxxxxx>
    Co-developed-by: Shengwei Luo <luoshengwei@xxxxxxxxxx>
    Signed-off-by: Shengwei Luo <luoshengwei@xxxxxxxxxx>
    Co-developed-by: Daniel Ferguson <danielf@xxxxxxxxxxxxxxxxxxxxxx>
    Signed-off-by: Daniel Ferguson <danielf@xxxxxxxxxxxxxxxxxxxxxx>
    Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@xxxxxxxxxx>
    Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>
    Tested-by: Shiju Jose <shiju.jose@xxxxxxxxxx>
    Link: https://uefi.org/specs/UEFI/2.10/Apx_N_Common_Platform_Error_Record.html#arm-processor-error-section

>  drivers/acpi/apei/ghes.c | 11 ++++-----
>  drivers/ras/ras.c        | 41 ++++++++++++++++++++++++++++++++--
>  include/linux/ras.h      | 16 +++++++++++---
>  include/ras/ras_event.h  | 48 +++++++++++++++++++++++++++++++++++-----
>  4 files changed, 99 insertions(+), 17 deletions(-)

Also, some cleanups ontop:

diff --git a/drivers/ras/ras.c b/drivers/ras/ras.c
index 6a0480c76e19..179b1744df71 100644
--- a/drivers/ras/ras.c
+++ b/drivers/ras/ras.c
@@ -70,14 +70,14 @@ void log_arm_hw_error(struct cper_sec_proc_arm *err, const u8 sev)
 	err_info = (struct cper_arm_err_info *)(err + 1);
 	ctx_info = (struct cper_arm_ctx_info *)(err_info + err->err_info_num);
 	ctx_err = (u8 *)ctx_info;
+
 	for (n = 0; n < err->context_info_num; n++) {
 		sz = sizeof(struct cper_arm_ctx_info) + ctx_info->size;
 		ctx_info = (struct cper_arm_ctx_info *)((long)ctx_info + sz);
 		ctx_len += sz;
 	}
 
-	vsei_len = err->section_length - (sizeof(struct cper_sec_proc_arm) +
-					  pei_len + ctx_len);
+	vsei_len = err->section_length - (sizeof(struct cper_sec_proc_arm) + pei_len + ctx_len);
 	if (vsei_len < 0) {
 		pr_warn(FW_BUG "section length: %d\n", err->section_length);
 		pr_warn(FW_BUG "section length is too small\n");
diff --git a/include/linux/ras.h b/include/linux/ras.h
index df444492b434..468941bfe855 100644
--- a/include/linux/ras.h
+++ b/include/linux/ras.h
@@ -55,8 +55,8 @@ amd_convert_umc_mca_addr_to_sys_addr(struct atl_err *err) { return -EINVAL; }
 #if defined(CONFIG_ARM) || defined(CONFIG_ARM64)
 #include <asm/smp_plat.h>
 /*
- * Include ARM specific SMP header which provides a function mapping mpidr to
- * cpu logical index.
+ * Include ARM-specific SMP header which provides a function mapping mpidr to
+ * CPU logical index.
  */
 #define GET_LOGICAL_INDEX(mpidr) get_logical_index(mpidr & MPIDR_HWID_BITMASK)
 #else


Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette




[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]
  Powered by Linux