Re: [PATCH] ArmPkg: GICv2/v3 base address width fix-up

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

Hello Dennis,

On 5 September 2016 at 09:27, Dennis Chen <dennis.chen@arm.com> wrote:
> According to the ACPI 6.0/6.1 spec, the physical base address of
> GICC, GICD, GICR and GIC ITS is 64-bit. This patch is trying to
> fix the original 32-bit width.
>

$ git grep PcdGic |grep -E Pcd.et32

gives me many instances in ArmPlatformPkg and ArmVirtPkg that refer to
these PCDs as 32 bits wide. If you are going to make this change,
please fix up all those occurrences as well, and please don't forget
about OpenPlatformPkg.

Thanks,
Ard.


> 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>
> ---
>  ArmPkg/ArmPkg.dec                         | 7 ++++---
>  ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c | 8 ++++----
>  ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c | 4 ++--
>  3 files changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
> index c189117..cc764d9 100644
> --- a/ArmPkg/ArmPkg.dec
> +++ b/ArmPkg/ArmPkg.dec
> @@ -251,10 +251,11 @@
>    #
>    # ARM Generic Interrupt Controller
>    #
> -  gArmTokenSpaceGuid.PcdGicDistributorBase|0|UINT32|0x0000000C
> +  gArmTokenSpaceGuid.PcdGicDistributorBase|0|UINT64|0x0000000C
>    # Base address for the GIC Redistributor region that contains the boot CPU
> -  gArmTokenSpaceGuid.PcdGicRedistributorsBase|0|UINT32|0x0000000E
> -  gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase|0|UINT32|0x0000000D
> +  gArmTokenSpaceGuid.PcdGicRedistributorsBase|0|UINT64|0x0000000E
> +  gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase|0|UINT64|0x0000000D
> +  gArmTokenSpaceGuid.PcdGicItsBase|0|UINT64|0x0000000F
>    gArmTokenSpaceGuid.PcdGicSgiIntId|0|UINT32|0x00000025
>
>    #
> diff --git a/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c b/ArmPkg/Drivers/ArmGic/GicV2/ArmGicV2Dxe.c
> index 34d4be3..a4ba5cf 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 UINTN mGicInterruptInterfaceBase;
> +STATIC UINTN mGicDistributorBase;
>
>  /**
>    Enable interrupt source Source.
> @@ -259,8 +259,8 @@ GicV2DxeInitialize (
>    // Make sure the Interrupt Controller Protocol is not already installed in the system.
>    ASSERT_PROTOCOL_ALREADY_INSTALLED (NULL, &gHardwareInterruptProtocolGuid);
>
> -  mGicInterruptInterfaceBase = PcdGet32 (PcdGicInterruptInterfaceBase);
> -  mGicDistributorBase = PcdGet32 (PcdGicDistributorBase);
> +  mGicInterruptInterfaceBase = PcdGet64 (PcdGicInterruptInterfaceBase);
> +  mGicDistributorBase = PcdGet64 (PcdGicDistributorBase);
>    mGicNumInterrupts = ArmGicGetMaxNumInterrupts (mGicDistributorBase);
>
>    for (Index = 0; Index < mGicNumInterrupts; Index++) {
> diff --git a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c
> index 983936f..8af97a9 100644
> --- a/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c
> +++ b/ArmPkg/Drivers/ArmGic/GicV3/ArmGicV3Dxe.c
> @@ -245,8 +245,8 @@ GicV3DxeInitialize (
>    // Make sure the Interrupt Controller Protocol is not already installed in the system.
>    ASSERT_PROTOCOL_ALREADY_INSTALLED (NULL, &gHardwareInterruptProtocolGuid);
>
> -  mGicDistributorBase    = PcdGet32 (PcdGicDistributorBase);
> -  mGicRedistributorsBase = PcdGet32 (PcdGicRedistributorsBase);
> +  mGicDistributorBase    = PcdGet64 (PcdGicDistributorBase);
> +  mGicRedistributorsBase = PcdGet64 (PcdGicRedistributorsBase);
>    mGicNumInterrupts      = ArmGicGetMaxNumInterrupts (mGicDistributorBase);
>
>    //
> --
> 2.7.4
>