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.martin@xxxxxxx] > Sent: 12 November 2014 19:43 > To: 'Ard Biesheuvel'; edk2-devel@xxxxxxxxxxxxxxxxxxxxx > Cc: Marc Zyngier; leif.lindholm@xxxxxxxxxx; > christoffer.dall@xxxxxxxxxx; kvmarm@xxxxxxxxxxxxxxxxxxxxx; > lersek@xxxxxxxxxx > 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.biesheuvel@xxxxxxxxxx] > > Sent: 12 November 2014 11:47 > > To: Olivier Martin; edk2-devel@xxxxxxxxxxxxxxxxxxxxx > > Cc: Marc Zyngier; leif.lindholm@xxxxxxxxxx; > > christoffer.dall@xxxxxxxxxx; kvmarm@xxxxxxxxxxxxxxxxxxxxx; > > lersek@xxxxxxxxxx; 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.biesheuvel@xxxxxxxxxx> > > --- > > 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 > > _______________________________________________ kvmarm mailing list kvmarm@xxxxxxxxxxxxxxxxxxxxx https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
