Hello Ard,

On Fri, Oct 21, 2016 at 09:34:32AM +0100, Ard Biesheuvel wrote:
> Hi Dennis,
> 
> On 21 October 2016 at 06:50, Dennis Chen <dennis.c...@arm.com> wrote:
> > Since ACPI spec defines the GIC base addresses (CPU interface,
> > Distributor and Redistributor*GICv3 only*) as 64-bit, so we should
> > define these corresponding base address variables as 64-bit instead of
> > 32-bit. This patch redefines them according to the ACPI spec.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
> > Cc: Leif Lindholm <leif.lindh...@linaro.org>
> > Signed-off-by: Dennis Chen <dennis.c...@arm.com>
> 
> Even if this is GICv3 only, you are breaking the build for 32-bit
> because of some changes that also affect GICv2
> 
> /home/ard/build/edk2/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Lib.c:20:1:
> error: conflicting types for ‘ArmGicV2AcknowledgeInterrupt’
>  ArmGicV2AcknowledgeInterrupt (
>  ^
> In file included from
> /home/ard/build/edk2/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Lib.c:15:0:
> /home/ard/build/edk2/ArmPkg/Include/Library/ArmGicLib.h:252:1: note:
> previous declaration of ‘ArmGicV2AcknowledgeInterrupt’ was here
>  ArmGicV2AcknowledgeInterrupt (
>  ^
> /home/ard/build/edk2/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Lib.c:30:1:
> error: conflicting types for ‘ArmGicV2EndOfInterrupt’
>  ArmGicV2EndOfInterrupt (
>  ^
> In file included from
> /home/ard/build/edk2/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Lib.c:15:0:
> /home/ard/build/edk2/ArmPkg/Include/Library/ArmGicLib.h:258:1: note:
> previous declaration of ‘ArmGicV2EndOfInterrupt’ was here
>  ArmGicV2EndOfInterrupt (
>  ^
> 
> When you propose changes like this one, please make sure if works on
> 64-bit as well as 32-bit
>
Thanks for spotting that! My fault not to try the 32-bit in my always 64-bit
environment, but that's not to seek some kind of excuse. In order to make you
guys feel a little bit comfortable ;-), I will give a quick fix about that.
So fingers crossed, that's the last issue with this patch blatted...

Thanks,
Dennis

> 
> Thanks,
> Ard.
> 
> 
> > ---
> >  ArmPkg/Drivers/ArmGic/ArmGicLib.c               | 64 ++++++++++----------
> >  ArmPkg/Drivers/ArmGic/ArmGicNonSecLib.c         |  2 +-
> >  ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c       |  4 +-
> >  ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2NonSecLib.c |  4 +-
> >  ArmPkg/Include/Library/ArmGicLib.h              | 78 
> > ++++++++++++-------------
> >  5 files changed, 76 insertions(+), 76 deletions(-)
> >
> > diff --git a/ArmPkg/Drivers/ArmGic/ArmGicLib.c 
> > b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
> > index e658e9b..733488c 100644
> > --- a/ArmPkg/Drivers/ArmGic/ArmGicLib.c
> > +++ b/ArmPkg/Drivers/ArmGic/ArmGicLib.c
> > @@ -41,18 +41,18 @@ SourceIsSpi (
> >   * @retval Base address of the associated GIC Redistributor
> >   */
> >  STATIC
> > -UINTN
> > +UINT64
> >  GicGetCpuRedistributorBase (
> > -  IN UINTN                 GicRedistributorBase,
> > -  IN ARM_GIC_ARCH_REVISION Revision
> > +  IN UINT64                 GicRedistributorBase,
> > +  IN ARM_GIC_ARCH_REVISION  Revision
> >    )
> >  {
> > -  UINTN Index;
> > -  UINTN MpId;
> > -  UINTN CpuAffinity;
> > -  UINTN Affinity;
> > -  UINTN GicRedistributorGranularity;
> > -  UINTN GicCpuRedistributorBase;
> > +  UINTN  Index;
> > +  UINTN  MpId;
> > +  UINTN  CpuAffinity;
> > +  UINTN  Affinity;
> > +  UINTN  GicRedistributorGranularity;
> > +  UINT64 GicCpuRedistributorBase;
> >
> >    MpId = ArmReadMpidr ();
> >    // Define CPU affinity as Affinity0[0:8], Affinity1[9:15], 
> > Affinity2[16:23], Affinity3[24:32]
> > @@ -87,7 +87,7 @@ GicGetCpuRedistributorBase (
> >  UINTN
> >  EFIAPI
> >  ArmGicGetInterfaceIdentification (
> > -  IN  INTN          GicInterruptInterfaceBase
> > +  IN  UINT64         GicInterruptInterfaceBase
> >    )
> >  {
> >    // Read the GIC Identification Register
> > @@ -97,7 +97,7 @@ ArmGicGetInterfaceIdentification (
> >  UINTN
> >  EFIAPI
> >  ArmGicGetMaxNumInterrupts (
> > -  IN  INTN          GicDistributorBase
> > +  IN  UINT64         GicDistributorBase
> >    )
> >  {
> >    return 32 * ((MmioRead32 (GicDistributorBase + ARM_GIC_ICDICTR) & 0x1F) 
> > + 1);
> > @@ -106,10 +106,10 @@ ArmGicGetMaxNumInterrupts (
> >  VOID
> >  EFIAPI
> >  ArmGicSendSgiTo (
> > -  IN  INTN          GicDistributorBase,
> > -  IN  INTN          TargetListFilter,
> > -  IN  INTN          CPUTargetList,
> > -  IN  INTN          SgiId
> > +  IN  UINT64          GicDistributorBase,
> > +  IN  UINTN           TargetListFilter,
> > +  IN  UINTN           CPUTargetList,
> > +  IN  UINTN           SgiId
> >    )
> >  {
> >    MmioWrite32 (GicDistributorBase + ARM_GIC_ICDSGIR, ((TargetListFilter & 
> > 0x3) << 24) | ((CPUTargetList & 0xFF) << 16) | SgiId);
> > @@ -131,8 +131,8 @@ ArmGicSendSgiTo (
> >  UINTN
> >  EFIAPI
> >  ArmGicAcknowledgeInterrupt (
> > -  IN  UINTN          GicInterruptInterfaceBase,
> > -  OUT UINTN          *InterruptId
> > +  IN  UINT64          GicInterruptInterfaceBase,
> > +  OUT UINTN           *InterruptId
> >    )
> >  {
> >    UINTN Value;
> > @@ -162,8 +162,8 @@ ArmGicAcknowledgeInterrupt (
> >  VOID
> >  EFIAPI
> >  ArmGicEndOfInterrupt (
> > -  IN  UINTN                 GicInterruptInterfaceBase,
> > -  IN UINTN                  Source
> > +  IN  UINT64                 GicInterruptInterfaceBase,
> > +  IN  UINTN                  Source
> >    )
> >  {
> >    ARM_GIC_ARCH_REVISION Revision;
> > @@ -181,9 +181,9 @@ ArmGicEndOfInterrupt (
> >  VOID
> >  EFIAPI
> >  ArmGicEnableInterrupt (
> > -  IN UINTN                  GicDistributorBase,
> > -  IN UINTN                  GicRedistributorBase,
> > -  IN UINTN                  Source
> > +  IN UINT64                  GicDistributorBase,
> > +  IN UINT64                  GicRedistributorBase,
> > +  IN UINTN                   Source
> >    )
> >  {
> >    UINT32                RegOffset;
> > @@ -216,9 +216,9 @@ ArmGicEnableInterrupt (
> >  VOID
> >  EFIAPI
> >  ArmGicDisableInterrupt (
> > -  IN UINTN                  GicDistributorBase,
> > -  IN UINTN                  GicRedistributorBase,
> > -  IN UINTN                  Source
> > +  IN UINT64                  GicDistributorBase,
> > +  IN UINT64                  GicRedistributorBase,
> > +  IN UINTN                   Source
> >    )
> >  {
> >    UINT32                RegOffset;
> > @@ -250,15 +250,15 @@ ArmGicDisableInterrupt (
> >  BOOLEAN
> >  EFIAPI
> >  ArmGicIsInterruptEnabled (
> > -  IN UINTN                  GicDistributorBase,
> > -  IN UINTN                  GicRedistributorBase,
> > -  IN UINTN                  Source
> > +  IN UINT64                  GicDistributorBase,
> > +  IN UINT64                  GicRedistributorBase,
> > +  IN UINTN                   Source
> >    )
> >  {
> >    UINT32                RegOffset;
> >    UINTN                 RegShift;
> >    ARM_GIC_ARCH_REVISION Revision;
> > -  UINTN                 GicCpuRedistributorBase;
> > +  UINT64                GicCpuRedistributorBase;
> >    UINT32                Interrupts;
> >
> >    // Calculate enable register offset and bit position
> > @@ -286,7 +286,7 @@ ArmGicIsInterruptEnabled (
> >  VOID
> >  EFIAPI
> >  ArmGicDisableDistributor (
> > -  IN  INTN          GicDistributorBase
> > +  IN  UINT64          GicDistributorBase
> >    )
> >  {
> >    // Disable Gic Distributor
> > @@ -296,7 +296,7 @@ ArmGicDisableDistributor (
> >  VOID
> >  EFIAPI
> >  ArmGicEnableInterruptInterface (
> > -  IN  INTN          GicInterruptInterfaceBase
> > +  IN  UINT64          GicInterruptInterfaceBase
> >    )
> >  {
> >    ARM_GIC_ARCH_REVISION Revision;
> > @@ -314,7 +314,7 @@ ArmGicEnableInterruptInterface (
> >  VOID
> >  EFIAPI
> >  ArmGicDisableInterruptInterface (
> > -  IN  INTN          GicInterruptInterfaceBase
> > +  IN  UINT64          GicInterruptInterfaceBase
> >    )
> >  {
> >    ARM_GIC_ARCH_REVISION Revision;
> > diff --git a/ArmPkg/Drivers/ArmGic/ArmGicNonSecLib.c 
> > b/ArmPkg/Drivers/ArmGic/ArmGicNonSecLib.c
> > index f90391b..bc01db9 100644
> > --- a/ArmPkg/Drivers/ArmGic/ArmGicNonSecLib.c
> > +++ b/ArmPkg/Drivers/ArmGic/ArmGicNonSecLib.c
> > @@ -19,7 +19,7 @@
> >  VOID
> >  EFIAPI
> >  ArmGicEnableDistributor (
> > -  IN  INTN          GicDistributorBase
> > +  IN  UINT64          GicDistributorBase
> >    )
> >  {
> >    ARM_GIC_ARCH_REVISION Revision;
> > diff --git a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c 
> > b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
> > index b9ecd55..c7c5af1 100644
> > --- a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
> > +++ b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
> > @@ -30,8 +30,8 @@ Abstract:
> >
> >  extern EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptV2Protocol;
> >
> > -STATIC UINT32 mGicInterruptInterfaceBase;
> > -STATIC UINT32 mGicDistributorBase;
> > +STATIC UINT64 mGicInterruptInterfaceBase;
> > +STATIC UINT64 mGicDistributorBase;
> >
> >  /**
> >    Enable interrupt source Source.
> > diff --git a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2NonSecLib.c 
> > b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2NonSecLib.c
> > index 92b764f..a7adbaf 100644
> > --- a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2NonSecLib.c
> > +++ b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2NonSecLib.c
> > @@ -20,7 +20,7 @@
> >  VOID
> >  EFIAPI
> >  ArmGicV2EnableInterruptInterface (
> > -  IN  INTN          GicInterruptInterfaceBase
> > +  IN  UINT64          GicInterruptInterfaceBase
> >    )
> >  {
> >    /*
> > @@ -33,7 +33,7 @@ ArmGicV2EnableInterruptInterface (
> >  VOID
> >  EFIAPI
> >  ArmGicV2DisableInterruptInterface (
> > -  IN  INTN          GicInterruptInterfaceBase
> > +  IN  UINT64          GicInterruptInterfaceBase
> >    )
> >  {
> >    // Disable Gic Interface
> > diff --git a/ArmPkg/Include/Library/ArmGicLib.h 
> > b/ArmPkg/Include/Library/ArmGicLib.h
> > index 4364f3f..bf6405c 100644
> > --- a/ArmPkg/Include/Library/ArmGicLib.h
> > +++ b/ArmPkg/Include/Library/ArmGicLib.h
> > @@ -101,7 +101,7 @@
> >  UINTN
> >  EFIAPI
> >  ArmGicGetInterfaceIdentification (
> > -  IN  INTN          GicInterruptInterfaceBase
> > +  IN  UINT64          GicInterruptInterfaceBase
> >    );
> >
> >  //
> > @@ -110,56 +110,56 @@ ArmGicGetInterfaceIdentification (
> >  VOID
> >  EFIAPI
> >  ArmGicSetupNonSecure (
> > -  IN  UINTN         MpId,
> > -  IN  INTN          GicDistributorBase,
> > -  IN  INTN          GicInterruptInterfaceBase
> > +  IN  UINTN           MpId,
> > +  IN  UINT64          GicDistributorBase,
> > +  IN  UINT64          GicInterruptInterfaceBase
> >    );
> >
> >  VOID
> >  EFIAPI
> >  ArmGicSetSecureInterrupts (
> > -  IN  UINTN         GicDistributorBase,
> > -  IN  UINTN*        GicSecureInterruptMask,
> > -  IN  UINTN         GicSecureInterruptMaskSize
> > +  IN  UINT64          GicDistributorBase,
> > +  IN  UINTN*          GicSecureInterruptMask,
> > +  IN  UINTN           GicSecureInterruptMaskSize
> >    );
> >
> >  VOID
> >  EFIAPI
> >  ArmGicEnableInterruptInterface (
> > -  IN  INTN          GicInterruptInterfaceBase
> > +  IN  UINT64          GicInterruptInterfaceBase
> >    );
> >
> >  VOID
> >  EFIAPI
> >  ArmGicDisableInterruptInterface (
> > -  IN  INTN          GicInterruptInterfaceBase
> > +  IN  UINT64          GicInterruptInterfaceBase
> >    );
> >
> >  VOID
> >  EFIAPI
> >  ArmGicEnableDistributor (
> > -  IN  INTN          GicDistributorBase
> > +  IN  UINT64          GicDistributorBase
> >    );
> >
> >  VOID
> >  EFIAPI
> >  ArmGicDisableDistributor (
> > -  IN  INTN          GicDistributorBase
> > +  IN  UINT64          GicDistributorBase
> >    );
> >
> >  UINTN
> >  EFIAPI
> >  ArmGicGetMaxNumInterrupts (
> > -  IN  INTN          GicDistributorBase
> > +  IN  UINT64          GicDistributorBase
> >    );
> >
> >  VOID
> >  EFIAPI
> >  ArmGicSendSgiTo (
> > -  IN  INTN          GicDistributorBase,
> > -  IN  INTN          TargetListFilter,
> > -  IN  INTN          CPUTargetList,
> > -  IN  INTN          SgiId
> > +  IN  UINT64         GicDistributorBase,
> > +  IN  UINTN          TargetListFilter,
> > +  IN  UINTN          CPUTargetList,
> > +  IN  UINTN          SgiId
> >    );
> >
> >  /*
> > @@ -178,46 +178,46 @@ ArmGicSendSgiTo (
> >  UINTN
> >  EFIAPI
> >  ArmGicAcknowledgeInterrupt (
> > -  IN  UINTN          GicInterruptInterfaceBase,
> > -  OUT UINTN          *InterruptId
> > +  IN  UINT64          GicInterruptInterfaceBase,
> > +  OUT UINTN           *InterruptId
> >    );
> >
> >  VOID
> >  EFIAPI
> >  ArmGicEndOfInterrupt (
> > -  IN  UINTN                 GicInterruptInterfaceBase,
> > -  IN UINTN                  Source
> > +  IN  UINT64                 GicInterruptInterfaceBase,
> > +  IN  UINTN                  Source
> >    );
> >
> >  UINTN
> >  EFIAPI
> >  ArmGicSetPriorityMask (
> > -  IN  INTN          GicInterruptInterfaceBase,
> > -  IN  INTN          PriorityMask
> > +  IN  UINT64          GicInterruptInterfaceBase,
> > +  IN  UINTN           PriorityMask
> >    );
> >
> >  VOID
> >  EFIAPI
> >  ArmGicEnableInterrupt (
> > -  IN UINTN                  GicDistributorBase,
> > -  IN UINTN                  GicRedistributorBase,
> > -  IN UINTN                  Source
> > +  IN UINT64                  GicDistributorBase,
> > +  IN UINT64                  GicRedistributorBase,
> > +  IN UINTN                   Source
> >    );
> >
> >  VOID
> >  EFIAPI
> >  ArmGicDisableInterrupt (
> > -  IN UINTN                  GicDistributorBase,
> > -  IN UINTN                  GicRedistributorBase,
> > -  IN UINTN                  Source
> > +  IN UINT64                  GicDistributorBase,
> > +  IN UINT64                  GicRedistributorBase,
> > +  IN UINTN                   Source
> >    );
> >
> >  BOOLEAN
> >  EFIAPI
> >  ArmGicIsInterruptEnabled (
> > -  IN UINTN                  GicDistributorBase,
> > -  IN UINTN                  GicRedistributorBase,
> > -  IN UINTN                  Source
> > +  IN UINT64                  GicDistributorBase,
> > +  IN UINT64                  GicRedistributorBase,
> > +  IN UINTN                   Source
> >    );
> >
> >  //
> > @@ -230,34 +230,34 @@ ArmGicIsInterruptEnabled (
> >  VOID
> >  EFIAPI
> >  ArmGicV2SetupNonSecure (
> > -  IN  UINTN         MpId,
> > -  IN  INTN          GicDistributorBase,
> > -  IN  INTN          GicInterruptInterfaceBase
> > +  IN  UINTN           MpId,
> > +  IN  UINT64          GicDistributorBase,
> > +  IN  UINT64          GicInterruptInterfaceBase
> >    );
> >
> >  VOID
> >  EFIAPI
> >  ArmGicV2EnableInterruptInterface (
> > -  IN  INTN          GicInterruptInterfaceBase
> > +  IN  UINT64          GicInterruptInterfaceBase
> >    );
> >
> >  VOID
> >  EFIAPI
> >  ArmGicV2DisableInterruptInterface (
> > -  IN  INTN          GicInterruptInterfaceBase
> > +  IN  UINT64          GicInterruptInterfaceBase
> >    );
> >
> >  UINTN
> >  EFIAPI
> >  ArmGicV2AcknowledgeInterrupt (
> > -  IN  UINTN          GicInterruptInterfaceBase
> > +  IN  UINT64          GicInterruptInterfaceBase
> >    );
> >
> >  VOID
> >  EFIAPI
> >  ArmGicV2EndOfInterrupt (
> > -  IN UINTN                  GicInterruptInterfaceBase,
> > -  IN UINTN                  Source
> > +  IN  UINT64          GicInterruptInterfaceBase,
> > +  IN  UINTN           Source
> >    );
> >
> >  //
> > --
> > 2.7.4
> >
> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to