I forgot to say your line endings are not correct. You have to use CRLF -
sorry that's the coding convention.

> -----Original Message-----
> From: Olivier Martin [mailto:olivier.mar...@arm.com]
> Sent: 12 November 2014 19:43
> To: 'Ard Biesheuvel'; edk2-devel@lists.sourceforge.net
> Cc: Marc Zyngier; leif.lindh...@linaro.org;
> christoffer.d...@linaro.org; kvm...@lists.cs.columbia.edu;
> ler...@redhat.com
> Subject: RE: [PATCH 1/2] ArmGicLib: cache GIC arch revision in global
> variable
> 
> I also thought using a global variable in my initial implementation.
> But it does not work when the library is called from XIP phases. In
> this case the global variables live in Read-Only memory and it would
> crash as soon as you try to write to it.
> This change is only safe when this library is called from these
> following module types: DXE_DRIVER, DXE_RUNTIME_DRIVER, UEFI_DRIVER, or
> UEFI_APPLICATION.
> 
> The way to fix it is to introduce a new
> ArmPkg/Drivers/ArmGic/ArmGicDxeLib.inf (limited to the above
> MODULE_TYPEs) and to enable your change in this file.
> You would also need to move out the functions from
> ArmPkg/Drivers/ArmGic/ArmGicLib.c that use this change (ie: using
> either the global variable mGicArchRevision or calling
> ArmGicGetSupportedArchRevision()).
> 
> 
> > -----Original Message-----
> > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> > Sent: 12 November 2014 11:47
> > To: Olivier Martin; edk2-devel@lists.sourceforge.net
> > Cc: Marc Zyngier; leif.lindh...@linaro.org;
> > christoffer.d...@linaro.org; kvm...@lists.cs.columbia.edu;
> > ler...@redhat.com; Ard Biesheuvel
> > Subject: [PATCH 1/2] ArmGicLib: cache GIC arch revision in global
> > variable
> >
> > Before extending the GIC arch revision check to support an emulated
> > GICv2 on GICv3 capable hardware in the next patch, add a constructor
> > to ArmGicLib that caches the result of the check so we don't have to
> > go through it on every call into ArmGicLib.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
> > ---
> >  ArmPkg/Drivers/ArmGic/ArmGicLib.c   | 42 +++++++++++++++++++--------
> --
> > --------
> >  ArmPkg/Drivers/ArmGic/ArmGicLib.inf |  1 +
> >  2 files changed, 23 insertions(+), 20 deletions(-)
> >
> > diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.c
> > b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
> > index 1e5924f5a49f..c48b97e3eb4d 100644
> > --- a/ArmPkg/Drivers/ArmGic/ArmGicLib.c
> > +++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
> > @@ -20,6 +20,8 @@
> >  #include "GicV2/ArmGicV2Lib.h"
> >  #include "GicV3/ArmGicV3Lib.h"
> >
> > +STATIC ARM_GIC_ARCH_REVISION    mGicArchRevision;
> > +
> >  UINTN
> >  EFIAPI
> >  ArmGicGetInterfaceIdentification (
> > @@ -72,10 +74,7 @@ ArmGicAcknowledgeInterrupt (
> >    )
> >  {
> >    UINTN Value;
> > -  ARM_GIC_ARCH_REVISION Revision;
> > -
> > -  Revision = ArmGicGetSupportedArchRevision ();
> > -  if (Revision == ARM_GIC_ARCH_REVISION_2) {
> > +  if (mGicArchRevision == ARM_GIC_ARCH_REVISION_2) {
> >      Value = ArmGicV2AcknowledgeInterrupt
> (GicInterruptInterfaceBase);
> >      // InterruptId is required for the caller to know if a valid or
> > spurious
> >      // interrupt has been read
> > @@ -83,7 +82,7 @@ ArmGicAcknowledgeInterrupt (
> >      if (InterruptId != NULL) {
> >        *InterruptId = Value & ARM_GIC_ICCIAR_ACKINTID;
> >      }
> > -  } else if (Revision == ARM_GIC_ARCH_REVISION_3) {
> > +  } else if (mGicArchRevision == ARM_GIC_ARCH_REVISION_3) {
> >      Value = ArmGicV3AcknowledgeInterrupt ();
> >    } else {
> >      ASSERT_EFI_ERROR (EFI_UNSUPPORTED);
> > @@ -102,12 +101,9 @@ ArmGicEndOfInterrupt (
> >    IN UINTN                  Source
> >    )
> >  {
> > -  ARM_GIC_ARCH_REVISION Revision;
> > -
> > -  Revision = ArmGicGetSupportedArchRevision ();
> > -  if (Revision == ARM_GIC_ARCH_REVISION_2) {
> > +  if (mGicArchRevision == ARM_GIC_ARCH_REVISION_2) {
> >      ArmGicV2EndOfInterrupt (GicInterruptInterfaceBase, Source);
> > -  } else if (Revision == ARM_GIC_ARCH_REVISION_3) {
> > +  } else if (mGicArchRevision == ARM_GIC_ARCH_REVISION_3) {
> >      ArmGicV3EndOfInterrupt (Source);
> >    } else {
> >      ASSERT_EFI_ERROR (EFI_UNSUPPORTED);
> > @@ -183,12 +179,9 @@ ArmGicEnableInterruptInterface (
> >    IN  INTN          GicInterruptInterfaceBase
> >    )
> >  {
> > -  ARM_GIC_ARCH_REVISION Revision;
> > -
> > -  Revision = ArmGicGetSupportedArchRevision ();
> > -  if (Revision == ARM_GIC_ARCH_REVISION_2) {
> > +  if (mGicArchRevision == ARM_GIC_ARCH_REVISION_2) {
> >      ArmGicV2EnableInterruptInterface (GicInterruptInterfaceBase);
> > -  } else if (Revision == ARM_GIC_ARCH_REVISION_3) {
> > +  } else if (mGicArchRevision == ARM_GIC_ARCH_REVISION_3) {
> >      ArmGicV3EnableInterruptInterface ();
> >    } else {
> >      ASSERT_EFI_ERROR (EFI_UNSUPPORTED);
> > @@ -201,14 +194,23 @@ ArmGicDisableInterruptInterface (
> >    IN  INTN          GicInterruptInterfaceBase
> >    )
> >  {
> > -  ARM_GIC_ARCH_REVISION Revision;
> > -
> > -  Revision = ArmGicGetSupportedArchRevision ();
> > -  if (Revision == ARM_GIC_ARCH_REVISION_2) {
> > +  if (mGicArchRevision == ARM_GIC_ARCH_REVISION_2) {
> >      ArmGicV2DisableInterruptInterface (GicInterruptInterfaceBase);
> > -  } else if (Revision == ARM_GIC_ARCH_REVISION_3) {
> > +  } else if (mGicArchRevision == ARM_GIC_ARCH_REVISION_3) {
> >      ArmGicV3DisableInterruptInterface ();
> >    } else {
> >      ASSERT_EFI_ERROR (EFI_UNSUPPORTED);
> >    }
> >  }
> > +
> > +
> > +RETURN_STATUS
> > +EFIAPI
> > +ArmGicLibInitialize (
> > +  VOID
> > +  )
> > +{
> > +  mGicArchRevision = ArmGicGetSupportedArchRevision ();
> > +
> > +  return RETURN_SUCCESS;
> > +}
> > diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.inf
> > b/ArmPkg/Drivers/ArmGic/ArmGicLib.inf
> > index 81282b9b8299..ceeb95c53e98 100644
> > --- a/ArmPkg/Drivers/ArmGic/ArmGicLib.inf
> > +++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.inf
> > @@ -18,6 +18,7 @@
> >    MODULE_TYPE                    = SEC
> >    VERSION_STRING                 = 1.0
> >    LIBRARY_CLASS                  = ArmGicLib
> > +  CONSTRUCTOR                    = ArmGicLibInitialize
> >
> >  [Sources]
> >    ArmGicLib.c
> > --
> > 1.8.3.2
> >





------------------------------------------------------------------------------
Comprehensive Server Monitoring with Site24x7.
Monitor 10 servers for $9/Month.
Get alerted through email, SMS, voice calls or mobile push notifications.
Take corrective actions from your mobile device.
http://pubads.g.doubleclick.net/gampad/clk?id=154624111&iu=/4140/ostg.clktrk
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to