Re: [PATCH edk2/MdePkg v1 1/1] MdePkg Cper.h: Add generic error macros for ARM platform

[Leif Lindholm <leif.lindholm@linaro.org>]

On Sun, Aug 12, 2018 at 10:01:48PM +0800, Ming Huang wrote:
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ming Huang <ming.huang@linaro.org>

Mike, Liming.
Could we merge this during the quiet period?
It's basically an omission - these are defined since (at least) UEFI 2.6.

> ---
>  MdePkg/Include/Guid/Cper.h | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/MdePkg/Include/Guid/Cper.h b/MdePkg/Include/Guid/Cper.h
> index 5ddd4c715ebc..525bfe57b21b 100644
> --- a/MdePkg/Include/Guid/Cper.h
> +++ b/MdePkg/Include/Guid/Cper.h
> @@ -256,6 +256,7 @@ typedef struct {
>  ///@{
>  #define EFI_GENERIC_ERROR_PROC_TYPE_IA32_X64         0x00
>  #define EFI_GENERIC_ERROR_PROC_TYPE_IA64             0x01
> +#define EFI_GENERIC_ERROR_PROC_TYPE_ARM              0x02
>  ///@}
>  
>  ///
> @@ -265,6 +266,8 @@ typedef struct {
>  #define EFI_GENERIC_ERROR_PROC_ISA_IA32              0x00
>  #define EFI_GENERIC_ERROR_PROC_ISA_IA64              0x01
>  #define EFI_GENERIC_ERROR_PROC_ISA_X64               0x02
> +#define EFI_GENERIC_ERROR_PROC_ISA_ARM32             0x03
> +#define EFI_GENERIC_ERROR_PROC_ISA_ARM64             0x04

Looking at the spec though, these are called:
ARM A32/T32 and
ARM A64

For architectural correctness, I think the names should be
EFI_GENERIC_ERROR_PROC_ISA_A32_T32
EFI_GENERIC_ERROR_PROC_ISA_A64
or 
EFI_GENERIC_ERROR_PROC_ISA_ARM_A32_T32
EFI_GENERIC_ERROR_PROC_ISA_ARM_A64
which gives some form of glorious symmetry against the
EFI_GENERIC_ERROR_PROC_TYPE_IA32_X64 above :)

With either change, as preferred by the package maintainers:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

>  ///@}
>  
>  ///
> -- 
> 2.17.0
>