[PATCH edk2/MdePkg v1 0/1] Add generic error macros for ARM platform

[PATCH edk2/MdePkg v1 0/1] Add generic error macros for ARM platform

[Ming Huang]

Ming Huang (1):
  MdePkg Cper.h: Add generic error macros for ARM platform

 MdePkg/Include/Guid/Cper.h | 3 +++
 1 file changed, 3 insertions(+)

-- 
2.17.0

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

[Ming Huang]

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ming Huang <ming.huang@linaro.org>
---
 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
 ///@}
 
 ///
-- 
2.17.0

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

[Leif Lindholm]

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
> 

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

[Kinney, Michael D]

Leif,

I have no objections to this commit being made before
the edk2 stable tag.

I see a Bugzilla for this topic as well:

https://bugzilla.tianocore.org/show_bug.cgi?id=1087

I have updated the Bugzilla approving this change
for edk2-stable201808.

Thanks,

Mike


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-
> bounces@lists.01.org] On Behalf Of Leif Lindholm
> Sent: Tuesday, August 14, 2018 9:42 AM
> To: Ming Huang <ming.huang@linaro.org>
> Cc: huangming23@huawei.com; edk2-devel@lists.01.org;
> Gao, Liming <liming.gao@intel.com>;
> mengfanrong@huawei.com; guoheyi@huawei.com;
> zhangjinsong2@huawei.com; linaro-uefi@lists.linaro.org;
> Kinney, Michael D <michael.d.kinney@intel.com>;
> wanghuiqiang@huawei.com; huangdaode@hisilicon.com
> Subject: Re: [edk2] [PATCH edk2/MdePkg v1 1/1] MdePkg
> Cper.h: Add generic error macros for ARM platform
> 
> 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
> >
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

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

[Laszlo Ersek]

On 08/14/18 20:37, Kinney, Michael D wrote:
> Leif,
> 
> I have no objections to this commit being made before
> the edk2 stable tag.
> 
> I see a Bugzilla for this topic as well:
> 
> https://bugzilla.tianocore.org/show_bug.cgi?id=1087
> 
> I have updated the Bugzilla approving this change
> for edk2-stable201808.

I agree these macros should go into the tagged tree, they will likely be
necessary for platform development that is based upon the tag.

Thanks
Laszlo