Discussion:
[edk2] [Patch] UefiCpuPkg/Library/CpuExceptionHandlerLib: Add exception type decoder
Jeff Fan
2015-07-06 08:02:06 UTC
Permalink
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 <***@intel.com>
Reviewed-by: Feng Tian <***@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
@@ -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";
+CONST CHAR8 *mExceptionNameStr[] = {
+ "#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"
+};
+
+#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/ArchExceptionHandler.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/ArchExceptionHandler.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
Leif Lindholm
2015-07-08 16:03:52 UTC
Permalink
Hi Jeff,

I notice that this patch has already been committed, but some comments
Post by Jeff Fan
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
---
.../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.
Post by Jeff Fan
@@ -1,7 +1,7 @@
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?
Post by Jeff Fan
+CONST CHAR8 *mExceptionNameStr[] = {
STATIC?
Post by Jeff Fan
+ "#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
Post by Jeff Fan
+
+#define EXCEPTION_KNOWN_NAME_NUM (sizeof (mExceptionNameStr) / sizeof (CHAR8 *))
+
+/**
+ Get ASCII format string exception name by exception type.
+
+
+**/
+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 @@
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.
+
+
+**/
+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/ArchExceptionHandler.c
@@ -1,7 +1,7 @@
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/ArchExceptionHandler.c
@@ -1,7 +1,7 @@
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
https://lists.sourceforge.net/lists/listinfo/edk2-devel
Fan, Jeff
2015-07-09 09:07:48 UTC
Permalink
Hi, Leif

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.

For STATIC, I agree with it. I will update it then.

Thanks!
Jeff

-----Original Message-----
From: Leif Lindholm [mailto:***@linaro.org]
Sent: Thursday, July 09, 2015 12:04 AM
To: edk2-***@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
Post by Jeff Fan
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
---
.../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.
Post by Jeff Fan
@@ -1,7 +1,7 @@
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
*mReservedVectors = NULL; // #define MAX_DEBUG_MESSAGE_LENGTH 0x100
+CONST CHAR8 mExceptionReservedStr[] = "Reserved";
STATIC?
Post by Jeff Fan
+CONST CHAR8 *mExceptionNameStr[] = {
STATIC?
Post by Jeff Fan
+ "#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
Post by Jeff Fan
+
+#define EXCEPTION_KNOWN_NAME_NUM (sizeof (mExceptionNameStr) /
+sizeof (CHAR8 *))
+
+/**
+ Get ASCII format string exception name by exception type.
+
+
+**/
+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 @@
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
IN UINTN VectorCount
);
+/**
+ Get ASCII format string exception name by exception type.
+
+
+**/
+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/ArchExceptionHand
+++ ler.c
@@ -1,7 +1,7 @@
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
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/ArchExceptionHandl
+++ er.c
@@ -1,7 +1,7 @@
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
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
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-***@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel
Leif Lindholm
2015-07-09 12:38:25 UTC
Permalink
Hi Jeff,
Post by Fan, Jeff
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
Post by Fan, Jeff
For STATIC, I agree with it. I will update it then.
Thanks!
Jeff
-----Original Message-----
Sent: Thursday, July 09, 2015 12:04 AM
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
Post by Jeff Fan
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
---
.../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.
Post by Jeff Fan
@@ -1,7 +1,7 @@
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
*mReservedVectors = NULL; // #define MAX_DEBUG_MESSAGE_LENGTH 0x100
+CONST CHAR8 mExceptionReservedStr[] = "Reserved";
STATIC?
Post by Jeff Fan
+CONST CHAR8 *mExceptionNameStr[] = {
STATIC?
Post by Jeff Fan
+ "#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
Post by Jeff Fan
+
+#define EXCEPTION_KNOWN_NAME_NUM (sizeof (mExceptionNameStr) /
+sizeof (CHAR8 *))
+
+/**
+ Get ASCII format string exception name by exception type.
+
+
+**/
+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 @@
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
IN UINTN VectorCount
);
+/**
+ Get ASCII format string exception name by exception type.
+
+
+**/
+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/ArchExceptionHand
+++ ler.c
@@ -1,7 +1,7 @@
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
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/ArchExceptionHandl
+++ er.c
@@ -1,7 +1,7 @@
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
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
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
https://lists.sourceforge.net/lists/listinfo/edk2-devel
Fan, Jeff
2015-07-13 06:29:32 UTC
Permalink
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:***@linaro.org]
Sent: Thursday, July 09, 2015 8:38 PM
To: Fan, Jeff
Cc: edk2-***@lists.sourceforge.net
Subject: Re: [edk2] [Patch] UefiCpuPkg/Library/CpuExceptionHandlerLib: Add exception type decoder

Hi Jeff,
Post by Fan, Jeff
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
Post by Fan, Jeff
For STATIC, I agree with it. I will update it then.
Thanks!
Jeff
-----Original Message-----
Sent: Thursday, July 09, 2015 12:04 AM
Add exception type decoder
Hi Jeff,
I notice that this patch has already been committed, but some comments
Post by Jeff Fan
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
---
.../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.
Post by Jeff Fan
@@ -1,7 +1,7 @@
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
*mReservedVectors = NULL; // #define MAX_DEBUG_MESSAGE_LENGTH
0x100
+CONST CHAR8 mExceptionReservedStr[] = "Reserved";
STATIC?
Post by Jeff Fan
+CONST CHAR8 *mExceptionNameStr[] = {
STATIC?
Post by Jeff Fan
+ "#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
Post by Jeff Fan
+
+#define EXCEPTION_KNOWN_NAME_NUM (sizeof (mExceptionNameStr) /
+sizeof (CHAR8 *))
+
+/**
+ Get ASCII format string exception name by exception type.
+
+
+**/
+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 @@
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
IN UINTN VectorCount
);
+/**
+ Get ASCII format string exception name by exception type.
+
+
+**/
+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 @@
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
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 @@
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
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
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
https://lists.sourceforge.net/lists/listinfo/edk2-devel
Loading...