[edk2] [PATCH v2 2/8] MdeModulePkg/CpuExceptionHandlerLib.h: Add a new API

Jian J Wang posted 8 patches 7 years, 1 month ago
There is a newer version of this series
[edk2] [PATCH v2 2/8] MdeModulePkg/CpuExceptionHandlerLib.h: Add a new API
Posted by Jian J Wang 7 years, 1 month ago
> v2:
>    Add prototype definition of InitializeCpuExceptionStackSwitchHandlers()

A new API InitializeCpuExceptionStackSwitchHandlers() is introduced to support
initializing exception handlers being able to switch stack. StackSwitchData is
arch dependent and required by IA32 processor to convey resources reserved in
advance. This is necessary because the CpuExceptionHandlerLib will be linked
in different phases, in which there's no common way to reserve resources.

EFI_STATUS
EFIAPI
InitializeCpuExceptionStackSwitchHandlers (
  IN VOID       *StackSwitchData  OPTIONAL
  );

Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Suggested-by: Ayellet Wolman <ayellet.wolman@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
 MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
index 6cd8230127..68de4850e1 100644
--- a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
+++ b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
@@ -41,6 +41,24 @@ InitializeCpuExceptionHandlers (
   IN EFI_VECTOR_HANDOFF_INFO       *VectorInfo OPTIONAL
   );
 
+/**
+  Setup separate stack for given exceptions. StackSwitchData is optional and its
+  content depends one the specific arch of CPU.
+
+  @param[in] StackSwitchData      Pointer to data required for setuping up
+                                  stack switch.
+
+  @retval EFI_SUCCESS             The exceptions have been successfully
+                                  initialized.
+  @retval EFI_INVALID_PARAMETER   StackSwitchData contains invalid content.
+
+**/
+EFI_STATUS
+EFIAPI
+InitializeCpuExceptionStackSwitchHandlers (
+  IN VOID       *StackSwitchData  OPTIONAL
+  );
+
 /**
   Initializes all CPU interrupt/exceptions entries and provides the default interrupt/exception handlers.
   
-- 
2.14.1.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 2/8] MdeModulePkg/CpuExceptionHandlerLib.h: Add a new API
Posted by Yao, Jiewen 7 years, 1 month ago
Hi
I am a little worried about the way to use VOID * to pass arch dependent data.

Can we define it clearly in each ARCH in the header file, and use a UNION to include all arch?

I think both the caller and the callee need parse it. As such, VOID * is not a good way.

Thank you
Yao Jiewen

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jian J
> Wang
> Sent: Wednesday, November 22, 2017 4:46 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric <eric.dong@intel.com>;
> Zeng, Star <star.zeng@intel.com>
> Subject: [edk2] [PATCH v2 2/8] MdeModulePkg/CpuExceptionHandlerLib.h: Add
> a new API
> 
> > v2:
> >    Add prototype definition of InitializeCpuExceptionStackSwitchHandlers()
> 
> A new API InitializeCpuExceptionStackSwitchHandlers() is introduced to support
> initializing exception handlers being able to switch stack. StackSwitchData is
> arch dependent and required by IA32 processor to convey resources reserved in
> advance. This is necessary because the CpuExceptionHandlerLib will be linked
> in different phases, in which there's no common way to reserve resources.
> 
> EFI_STATUS
> EFIAPI
> InitializeCpuExceptionStackSwitchHandlers (
>   IN VOID       *StackSwitchData  OPTIONAL
>   );
> 
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Suggested-by: Ayellet Wolman <ayellet.wolman@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> ---
>  MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h | 18
> ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
> b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
> index 6cd8230127..68de4850e1 100644
> --- a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
> +++ b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
> @@ -41,6 +41,24 @@ InitializeCpuExceptionHandlers (
>    IN EFI_VECTOR_HANDOFF_INFO       *VectorInfo OPTIONAL
>    );
> 
> +/**
> +  Setup separate stack for given exceptions. StackSwitchData is optional and its
> +  content depends one the specific arch of CPU.
> +
> +  @param[in] StackSwitchData      Pointer to data required for setuping up
> +                                  stack switch.
> +
> +  @retval EFI_SUCCESS             The exceptions have been successfully
> +                                  initialized.
> +  @retval EFI_INVALID_PARAMETER   StackSwitchData contains invalid
> content.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +InitializeCpuExceptionStackSwitchHandlers (
> +  IN VOID       *StackSwitchData  OPTIONAL
> +  );
> +
>  /**
>    Initializes all CPU interrupt/exceptions entries and provides the default
> interrupt/exception handlers.
> 
> --
> 2.14.1.windows.1
> 
> _______________________________________________
> 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
Re: [edk2] [PATCH v2 2/8] MdeModulePkg/CpuExceptionHandlerLib.h: Add a new API
Posted by Wang, Jian J 7 years, 1 month ago
Good idea. I think it should be defined in also in following file besides the new API

MdeModulePkg\Include\Library\CpuExceptionHandlerLib.h

> -----Original Message-----
> From: Yao, Jiewen
> Sent: Thursday, November 23, 2017 12:08 PM
> To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org
> Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: RE: [edk2] [PATCH v2 2/8] MdeModulePkg/CpuExceptionHandlerLib.h:
> Add a new API
> 
> Hi
> I am a little worried about the way to use VOID * to pass arch dependent data.
> 
> Can we define it clearly in each ARCH in the header file, and use a UNION to
> include all arch?
> 
> I think both the caller and the callee need parse it. As such, VOID * is not a good
> way.
> 
> Thank you
> Yao Jiewen
> 
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jian
> J
> > Wang
> > Sent: Wednesday, November 22, 2017 4:46 PM
> > To: edk2-devel@lists.01.org
> > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric <eric.dong@intel.com>;
> > Zeng, Star <star.zeng@intel.com>
> > Subject: [edk2] [PATCH v2 2/8] MdeModulePkg/CpuExceptionHandlerLib.h:
> Add
> > a new API
> >
> > > v2:
> > >    Add prototype definition of InitializeCpuExceptionStackSwitchHandlers()
> >
> > A new API InitializeCpuExceptionStackSwitchHandlers() is introduced to
> support
> > initializing exception handlers being able to switch stack. StackSwitchData is
> > arch dependent and required by IA32 processor to convey resources reserved
> in
> > advance. This is necessary because the CpuExceptionHandlerLib will be linked
> > in different phases, in which there's no common way to reserve resources.
> >
> > EFI_STATUS
> > EFIAPI
> > InitializeCpuExceptionStackSwitchHandlers (
> >   IN VOID       *StackSwitchData  OPTIONAL
> >   );
> >
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Suggested-by: Ayellet Wolman <ayellet.wolman@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> > ---
> >  MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h | 18
> > ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> >
> > diff --git a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
> > b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
> > index 6cd8230127..68de4850e1 100644
> > --- a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
> > +++ b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
> > @@ -41,6 +41,24 @@ InitializeCpuExceptionHandlers (
> >    IN EFI_VECTOR_HANDOFF_INFO       *VectorInfo OPTIONAL
> >    );
> >
> > +/**
> > +  Setup separate stack for given exceptions. StackSwitchData is optional and
> its
> > +  content depends one the specific arch of CPU.
> > +
> > +  @param[in] StackSwitchData      Pointer to data required for setuping up
> > +                                  stack switch.
> > +
> > +  @retval EFI_SUCCESS             The exceptions have been successfully
> > +                                  initialized.
> > +  @retval EFI_INVALID_PARAMETER   StackSwitchData contains invalid
> > content.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +InitializeCpuExceptionStackSwitchHandlers (
> > +  IN VOID       *StackSwitchData  OPTIONAL
> > +  );
> > +
> >  /**
> >    Initializes all CPU interrupt/exceptions entries and provides the default
> > interrupt/exception handlers.
> >
> > --
> > 2.14.1.windows.1
> >
> > _______________________________________________
> > 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
[edk2] 答复: [PATCH v2 2/8] MdeModulePkg/CpuExceptionHandlerLib.h: Add a new API
Posted by Fan Jeff 7 years ago
Hi,



I am not sure if this is good idea to define such arch specific definitions in MdeModulePkg. Moreover, we don’t know how ARM or other processors define this definition, either.



Jeff



________________________________
From: edk2-devel <edk2-devel-bounces@lists.01.org> on behalf of Wang, Jian J <jian.j.wang@intel.com>
Sent: Thursday, November 23, 2017 1:06:53 PM
To: Yao, Jiewen; edk2-devel@lists.01.org
Cc: Dong, Eric; Zeng, Star
Subject: Re: [edk2] [PATCH v2 2/8] MdeModulePkg/CpuExceptionHandlerLib.h: Add a new API

Good idea. I think it should be defined in also in following file besides the new API

MdeModulePkg\Include\Library\CpuExceptionHandlerLib.h

> -----Original Message-----
> From: Yao, Jiewen
> Sent: Thursday, November 23, 2017 12:08 PM
> To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org
> Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: RE: [edk2] [PATCH v2 2/8] MdeModulePkg/CpuExceptionHandlerLib.h:
> Add a new API
>
> Hi
> I am a little worried about the way to use VOID * to pass arch dependent data.
>
> Can we define it clearly in each ARCH in the header file, and use a UNION to
> include all arch?
>
> I think both the caller and the callee need parse it. As such, VOID * is not a good
> way.
>
> Thank you
> Yao Jiewen
>
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jian
> J
> > Wang
> > Sent: Wednesday, November 22, 2017 4:46 PM
> > To: edk2-devel@lists.01.org
> > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric <eric.dong@intel.com>;
> > Zeng, Star <star.zeng@intel.com>
> > Subject: [edk2] [PATCH v2 2/8] MdeModulePkg/CpuExceptionHandlerLib.h:
> Add
> > a new API
> >
> > > v2:
> > >    Add prototype definition of InitializeCpuExceptionStackSwitchHandlers()
> >
> > A new API InitializeCpuExceptionStackSwitchHandlers() is introduced to
> support
> > initializing exception handlers being able to switch stack. StackSwitchData is
> > arch dependent and required by IA32 processor to convey resources reserved
> in
> > advance. This is necessary because the CpuExceptionHandlerLib will be linked
> > in different phases, in which there's no common way to reserve resources.
> >
> > EFI_STATUS
> > EFIAPI
> > InitializeCpuExceptionStackSwitchHandlers (
> >   IN VOID       *StackSwitchData  OPTIONAL
> >   );
> >
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Suggested-by: Ayellet Wolman <ayellet.wolman@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> > ---
> >  MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h | 18
> > ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> >
> > diff --git a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
> > b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
> > index 6cd8230127..68de4850e1 100644
> > --- a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
> > +++ b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
> > @@ -41,6 +41,24 @@ InitializeCpuExceptionHandlers (
> >    IN EFI_VECTOR_HANDOFF_INFO       *VectorInfo OPTIONAL
> >    );
> >
> > +/**
> > +  Setup separate stack for given exceptions. StackSwitchData is optional and
> its
> > +  content depends one the specific arch of CPU.
> > +
> > +  @param[in] StackSwitchData      Pointer to data required for setuping up
> > +                                  stack switch.
> > +
> > +  @retval EFI_SUCCESS             The exceptions have been successfully
> > +                                  initialized.
> > +  @retval EFI_INVALID_PARAMETER   StackSwitchData contains invalid
> > content.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +InitializeCpuExceptionStackSwitchHandlers (
> > +  IN VOID       *StackSwitchData  OPTIONAL
> > +  );
> > +
> >  /**
> >    Initializes all CPU interrupt/exceptions entries and provides the default
> > interrupt/exception handlers.
> >
> > --
> > 2.14.1.windows.1
> >
> > _______________________________________________
> > 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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [PATCH v2 2/8] MdeModulePkg/CpuExceptionHandlerLib.h: Add a new API
Posted by Wang, Jian J 7 years ago
If we use union data, other arch of processors can add their own definitions in it without interfering ours. Is the MdeModulePkg for IA32 only?

This data is used to reserve resources in different boot phases. Unless we can limit its uses in just UefiCpuPkg, maybe MdeModulePkg is the only choice. But at least in my current implementation, we can just put it in UefiCpuPkg. I’m just not sure of any future impact.

From: Fan Jeff [mailto:vanjeff_919@hotmail.com]
Sent: Saturday, November 25, 2017 9:18 PM
To: Wang, Jian J <jian.j.wang@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org
Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: 答复: [edk2] [PATCH v2 2/8] MdeModulePkg/CpuExceptionHandlerLib.h: Add a new API


Hi,



I am not sure if this is good idea to define such arch specific definitions in MdeModulePkg. Moreover, we don’t know how ARM or other processors define this definition, either.



Jeff



________________________________
From: edk2-devel <edk2-devel-bounces@lists.01.org<mailto:edk2-devel-bounces@lists.01.org>> on behalf of Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>
Sent: Thursday, November 23, 2017 1:06:53 PM
To: Yao, Jiewen; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
Cc: Dong, Eric; Zeng, Star
Subject: Re: [edk2] [PATCH v2 2/8] MdeModulePkg/CpuExceptionHandlerLib.h: Add a new API

Good idea. I think it should be defined in also in following file besides the new API

MdeModulePkg\Include\Library\CpuExceptionHandlerLib.h

> -----Original Message-----
> From: Yao, Jiewen
> Sent: Thursday, November 23, 2017 12:08 PM
> To: Wang, Jian J <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> Cc: Dong, Eric <eric.dong@intel.com<mailto:eric.dong@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>
> Subject: RE: [edk2] [PATCH v2 2/8] MdeModulePkg/CpuExceptionHandlerLib.h:
> Add a new API
>
> Hi
> I am a little worried about the way to use VOID * to pass arch dependent data.
>
> Can we define it clearly in each ARCH in the header file, and use a UNION to
> include all arch?
>
> I think both the caller and the callee need parse it. As such, VOID * is not a good
> way.
>
> Thank you
> Yao Jiewen
>
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jian
> J
> > Wang
> > Sent: Wednesday, November 22, 2017 4:46 PM
> > To: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> > Cc: Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>; Dong, Eric <eric.dong@intel.com<mailto:eric.dong@intel.com>>;
> > Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>
> > Subject: [edk2] [PATCH v2 2/8] MdeModulePkg/CpuExceptionHandlerLib.h:
> Add
> > a new API
> >
> > > v2:
> > >    Add prototype definition of InitializeCpuExceptionStackSwitchHandlers()
> >
> > A new API InitializeCpuExceptionStackSwitchHandlers() is introduced to
> support
> > initializing exception handlers being able to switch stack. StackSwitchData is
> > arch dependent and required by IA32 processor to convey resources reserved
> in
> > advance. This is necessary because the CpuExceptionHandlerLib will be linked
> > in different phases, in which there's no common way to reserve resources.
> >
> > EFI_STATUS
> > EFIAPI
> > InitializeCpuExceptionStackSwitchHandlers (
> >   IN VOID       *StackSwitchData  OPTIONAL
> >   );
> >
> > Cc: Star Zeng <star.zeng@intel.com<mailto:star.zeng@intel.com>>
> > Cc: Eric Dong <eric.dong@intel.com<mailto:eric.dong@intel.com>>
> > Cc: Jiewen Yao <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
> > Suggested-by: Ayellet Wolman <ayellet.wolman@intel.com<mailto:ayellet.wolman@intel.com>>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Jian J Wang <jian.j.wang@intel.com<mailto:jian.j.wang@intel.com>>
> > ---
> >  MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h | 18
> > ++++++++++++++++++
> >  1 file changed, 18 insertions(+)
> >
> > diff --git a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
> > b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
> > index 6cd8230127..68de4850e1 100644
> > --- a/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
> > +++ b/MdeModulePkg/Include/Library/CpuExceptionHandlerLib.h
> > @@ -41,6 +41,24 @@ InitializeCpuExceptionHandlers (
> >    IN EFI_VECTOR_HANDOFF_INFO       *VectorInfo OPTIONAL
> >    );
> >
> > +/**
> > +  Setup separate stack for given exceptions. StackSwitchData is optional and
> its
> > +  content depends one the specific arch of CPU.
> > +
> > +  @param[in] StackSwitchData      Pointer to data required for setuping up
> > +                                  stack switch.
> > +
> > +  @retval EFI_SUCCESS             The exceptions have been successfully
> > +                                  initialized.
> > +  @retval EFI_INVALID_PARAMETER   StackSwitchData contains invalid
> > content.
> > +
> > +**/
> > +EFI_STATUS
> > +EFIAPI
> > +InitializeCpuExceptionStackSwitchHandlers (
> > +  IN VOID       *StackSwitchData  OPTIONAL
> > +  );
> > +
> >  /**
> >    Initializes all CPU interrupt/exceptions entries and provides the default
> > interrupt/exception handlers.
> >
> > --
> > 2.14.1.windows.1
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>
> > https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org<mailto: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