Leif,

Sorry. I missed this e-mail last Friday.  

I think this is history reason to name this package to UefiCpuPkg when we add 
it into EDKII code base in 2009. 

However, it is only support IA32/x64 arch only by now. I am not sure if there 
is requirement to add other arch in this package or not for now or for the 
future.

Jordan,

Do you have any information on this package's original goal?

Best Regards,
Jeff

-----Original Message-----
From: Leif Lindholm [mailto:leif.lindh...@linaro.org] 
Sent: Thursday, July 09, 2015 8:38 PM
To: Fan, Jeff
Cc: edk2-devel@lists.sourceforge.net
Subject: Re: [edk2] [Patch] UefiCpuPkg/Library/CpuExceptionHandlerLib: Add 
exception type decoder

Hi Jeff,

On Thu, Jul 09, 2015 at 09:07:48AM +0000, Fan, Jeff wrote:
> Cpu Exception Handler Lib is intended to support IA32/x64 arch only.
> You could see the following description in 
> UefiCpuPkg/CpuExceptionHandlerLib INF file.
> 
> #
> # The following information is for reference only and not required # 
> by the build tools.
> #
> #  VALID_ARCHITECTURES           = IA32 X64
> #
> 
> I think other arch has own implementation of CPU Exception Handler lib 
> instances.
> 
> For IA32/x64 implementation, we abstracted the common code in module 
> root directly.

Ok, so I guess that brings me on to a further question that is not directly 
related to your patch:

If this entire package (UefiCpuPkg) is intended to be for IA32/X64 only, why 
does it have a completely generic-looking name?
If it is a package for Intel CPUs, should it not be called IntelCpuPkg?

Regards,

Leif

> For STATIC, I agree with it. I will update it then. 
> 
> Thanks!
> Jeff
> 
> -----Original Message-----
> From: Leif Lindholm [mailto:leif.lindh...@linaro.org]
> Sent: Thursday, July 09, 2015 12:04 AM
> To: edk2-devel@lists.sourceforge.net
> Subject: Re: [edk2] [Patch] UefiCpuPkg/Library/CpuExceptionHandlerLib: 
> Add exception type decoder
> 
> Hi Jeff,
> 
> I notice that this patch has already been committed, but some comments
> below:
> 
> On Mon, Jul 06, 2015 at 04:02:06PM +0800, Jeff Fan wrote:
> > Add exception type decoder to print exception name string beside 
> > print exception type value. The exception names are from IA32 SDM.
> > 
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Jeff Fan <jeff....@intel.com>
> > Reviewed-by: Feng Tian <feng.t...@intel.com>
> > ---
> >  .../CpuExceptionHandlerLib/CpuExceptionCommon.c    | 48 
> > +++++++++++++++++++++-
> >  .../CpuExceptionHandlerLib/CpuExceptionCommon.h    | 14 ++++++-
> >  .../Ia32/ArchExceptionHandler.c                    |  5 ++-
> >  .../X64/ArchExceptionHandler.c                     |  5 ++-
> >  4 files changed, 66 insertions(+), 6 deletions(-)
> > 
> > diff --git
> > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> > index f8cbcf0..b971754 100644
> > --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> > +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c
> 
> This looks to me like it should be a completely architecture independent 
> source file.
> 
> > @@ -1,7 +1,7 @@
> >  /** @file
> >    CPU Exception Handler Library common functions.
> >  
> > -  Copyright (c) 2012 - 2014, Intel Corporation. All rights 
> > reserved.<BR>
> > +  Copyright (c) 2012 - 2015, Intel Corporation. All rights 
> > + reserved.<BR>
> >    This program and the accompanying materials
> >    are licensed and made available under the terms and conditions of the 
> > BSD License
> >    which accompanies this distribution.  The full text of the 
> > license may be found at @@ -28,6 +28,52 @@ RESERVED_VECTORS_DATA 
> > *mReservedVectors = NULL;  //  #define MAX_DEBUG_MESSAGE_LENGTH  
> > 0x100
> >  
> > +CONST CHAR8 mExceptionReservedStr[] = "Reserved";
> 
> STATIC?
> 
> > +CONST CHAR8 *mExceptionNameStr[] = {
> 
> STATIC?
> 
> > +  "#DE - Divide Error",
> > +  "#DB - Debug",
> > +  "NMI Interrupt",
> > +  "#BP - Breakpoint",
> > +  "#OF - Overflow",
> > +  "#BR - BOUND Range Exceeded",
> > +  "#UD - Invalid Opcode",
> > +  "#NM - Device Not Available",
> > +  "#DF - Double Fault",
> > +  "Coprocessor Segment Overrun",
> > +  "#TS - Invalid TSS",
> > +  "#NP - Segment Not Present",
> > +  "#SS - Stack Fault Fault",
> > +  "#GP - General Protection",
> > +  "#PF - Page-Fault",
> > +  "Reserved",
> > +  "#MF - x87 FPU Floating-Point Error",
> > +  "#AC - Alignment Check",
> > +  "#MC - Machine-Check",
> > +  "#XM - SIMD floating-point",
> > +  "#VE - Virtualization"
> > +};
> 
> ... yet here we are encoding Ia32/X64-specific exception value types, with 
> architecture specific exception numbers.
> 
> Would this functionality not be better kept in a shared source file included 
> for Sources.Ia32/X64 targets in the relevant .inf files?
> 
> /
>     Leif
> 
> > +
> > +#define EXCEPTION_KNOWN_NAME_NUM  (sizeof (mExceptionNameStr) / 
> > +sizeof (CHAR8 *))
> > +
> > +/**
> > +  Get ASCII format string exception name by exception type.
> > +
> > +  @param ExceptionType  Exception type.
> > +
> > +  @return  ASCII format string exception name.
> > +**/
> > +CONST CHAR8 *
> > +GetExceptionNameStr (
> > +  IN EFI_EXCEPTION_TYPE          ExceptionType
> > +  )
> > +{
> > +  if ((UINTN) ExceptionType < EXCEPTION_KNOWN_NAME_NUM) {
> > +    return mExceptionNameStr[ExceptionType];
> > +  } else {
> > +    return mExceptionReservedStr;
> > +  }
> > +}
> > +
> >  /**
> >    Prints a message to the serial port.
> >  
> > diff --git
> > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> > index efe77eb..b28e9c5 100644
> > --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> > +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h
> > @@ -1,7 +1,7 @@
> >  /** @file
> >    Common header file for CPU Exception Handler Library.
> >  
> > -  Copyright (c) 2012 - 2013, Intel Corporation. All rights 
> > reserved.<BR>
> > +  Copyright (c) 2012 - 2015, Intel Corporation. All rights 
> > + reserved.<BR>
> >    This program and the accompanying materials
> >    are licensed and made available under the terms and conditions of the 
> > BSD License
> >    which accompanies this distribution.  The full text of the 
> > license may be found at @@ -237,5 +237,17 @@ ReadAndVerifyVectorInfo (
> >    IN  UINTN                         VectorCount
> >    );
> >  
> > +/**
> > +  Get ASCII format string exception name by exception type.
> > +
> > +  @param ExceptionType  Exception type.
> > +
> > +  @return  ASCII format string exception name.
> > +**/
> > +CONST CHAR8 *
> > +GetExceptionNameStr (
> > +  IN EFI_EXCEPTION_TYPE          ExceptionType
> > +  );
> > +
> >  #endif
> >  
> > diff --git
> > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.
> > c
> > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.
> > c
> > index 40cdedf..e4a26ab 100644
> > ---
> > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.
> > c
> > +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHa
> > +++ nd
> > +++ ler.c
> > @@ -1,7 +1,7 @@
> >  /** @file
> >    IA32 CPU Exception Handler functons.
> >  
> > -  Copyright (c) 2012 - 2013, Intel Corporation. All rights 
> > reserved.<BR>
> > +  Copyright (c) 2012 - 2015, Intel Corporation. All rights 
> > + reserved.<BR>
> >    This program and the accompanying materials
> >    are licensed and made available under the terms and conditions of the 
> > BSD License
> >    which accompanies this distribution.  The full text of the 
> > license may be found at @@ -114,8 +114,9 @@ DumpCpuContent (
> >    UINTN                   EntryPoint;
> >  
> >    InternalPrintMessage (
> > -    "!!!! IA32 Exception Type - %08x    CPU Apic ID - %08x !!!!\n",
> > +    "!!!! IA32 Exception Type - %02x(%a)  CPU Apic ID - %08x 
> > + !!!!\n",
> >      ExceptionType,
> > +    GetExceptionNameStr (ExceptionType),
> >      GetApicId ()
> >      );
> >    InternalPrintMessage (
> > diff --git
> > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler
> > .c 
> > b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler
> > .c
> > index ee16ea8..7de1cc0 100644
> > ---
> > a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler
> > .c
> > +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHan
> > +++ dl
> > +++ er.c
> > @@ -1,7 +1,7 @@
> >  /** @file
> >    x64 CPU Exception Handler.
> >  
> > -  Copyright (c) 2012 - 2013, Intel Corporation. All rights 
> > reserved.<BR>
> > +  Copyright (c) 2012 - 2015, Intel Corporation. All rights 
> > + reserved.<BR>
> >    This program and the accompanying materials
> >    are licensed and made available under the terms and conditions of the 
> > BSD License
> >    which accompanies this distribution.  The full text of the 
> > license may be found at @@ -118,8 +118,9 @@ DumpCpuContent (
> >    UINTN                   EntryPoint;
> >  
> >    InternalPrintMessage (
> > -    "!!!! X64 Exception Type - %016lx     CPU Apic ID - %08x !!!!\n",
> > +    "!!!! X64 Exception Type - %02x(%a)  CPU Apic ID - %08x 
> > + !!!!\n",
> >      ExceptionType,
> > +    GetExceptionNameStr (ExceptionType),
> >      GetApicId ()
> >      );
> >    InternalPrintMessage (
> > --
> > 1.9.5.msysgit.0
> > 
> > 
> > --------------------------------------------------------------------
> > --
> > -------- Don't Limit Your Business. Reach for the Cloud.
> > GigeNET's Cloud Solutions provide you with the tools and support 
> > that you need to offload your IT needs and focus on growing your business.
> > Configured For All Businesses. Start Your Cloud Today.
> > https://www.gigenetcloud.com/
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/edk2-devel
> 
> ----------------------------------------------------------------------
> -------- Don't Limit Your Business. Reach for the Cloud.
> GigeNET's Cloud Solutions provide you with the tools and support that you 
> need to offload your IT needs and focus on growing your business.
> Configured For All Businesses. Start Your Cloud Today.
> https://www.gigenetcloud.com/
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel

------------------------------------------------------------------------------
Don't Limit Your Business. Reach for the Cloud.
GigeNET's Cloud Solutions provide you with the tools and support that
you need to offload your IT needs and focus on growing your business.
Configured For All Businesses. Start Your Cloud Today.
https://www.gigenetcloud.com/
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to