public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: Dennis Chen <dennis.chen@arm.com>
Cc: edk2-devel-01 <edk2-devel@lists.01.org>,
	nd@arm.com,  Leif Lindholm <leif.lindholm@linaro.org>
Subject: Re: [RESEND PATCH] ARMPkg: Unify the GIC base address variables as 64-bit
Date: Fri, 21 Oct 2016 09:34:32 +0100	[thread overview]
Message-ID: <CAKv+Gu_onqgJtHdKEExejomZx11_4Ek_E1ueRafNFmc6eU1pFg@mail.gmail.com> (raw)
In-Reply-To: <1477029054-31980-1-git-send-email-dennis.chen@arm.com>

Hi Dennis,

On 21 October 2016 at 06:50, Dennis Chen <dennis.chen@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.biesheuvel@linaro.org>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Signed-off-by: Dennis Chen <dennis.chen@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,
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
>


  reply	other threads:[~2016-10-21  8:34 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-10-21  5:50 [RESEND PATCH] ARMPkg: Unify the GIC base address variables as 64-bit Dennis Chen
2016-10-21  8:34 ` Ard Biesheuvel [this message]
2016-10-24  2:56   ` Dennis Chen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAKv+Gu_onqgJtHdKEExejomZx11_4Ek_E1ueRafNFmc6eU1pFg@mail.gmail.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox