diff mbox series

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

Message ID 20180812140148.31669-2-ming.huang@linaro.org
State Accepted
Commit cb5f4f45ce1fca390b99dae5c42b9c4c8b53deea
Headers show
Series Add generic error macros for ARM platform | expand

Commit Message

Ming Huang Aug. 12, 2018, 2:01 p.m. UTC
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(+)

-- 
2.17.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Leif Lindholm Aug. 14, 2018, 4:42 p.m. UTC | #1
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
Kinney, Michael D Aug. 14, 2018, 6:37 p.m. UTC | #2
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

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Laszlo Ersek Aug. 15, 2018, 11:18 a.m. UTC | #3
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox series

Patch

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
 ///@}
 
 ///