Re: [Xen-devel] [PATCH v3 04/27] ArmVirtualizationPkg: add GICv3 detection to VirtFdtDxe

[olimar01 <olivier.martin@xxxxxxx>]

The main comment I will make on this patch is to rename "redistributor
interface" into "redistributor region".
The Device Tree binding actually defines the entry as 'GIC Redistributor
region' or 'GIC Redistributors':
http://lxr.free-electrons.com/diff/Documentation/devicetree/bindings/arm/gic-v3.txt?diffvar=v;diffval=3.4
The GIC redistributor region contains the GIC redistributors for all the
cores.


On 03/02/15 19:19, Ard Biesheuvel wrote:
> This adds support for detecting the presence of a GICv3 interrupt
> controller from the device tree, and recording its distributor and
> redistributor base addresses in their respective PCDs.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Acked-by: Laszlo Ersek <lersek@xxxxxxxxxx>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
> ---
>
> Removed Olivier's R-b due to the fact that this patch was changed to
> accomodate pending but not yet finalized changes to the GICv3 driver.
> I think that recording the distributor and redistributor base addresses
> should be sufficient in any case, but we may have to revisit this still
> when the GIC dust settles.
>
>   .../ArmVirtualizationPkg/ArmVirtualizationQemu.dsc |  1 +
>   .../ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.c   | 31 +++++++++++++++++++++-
>   .../ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.inf |  1 +
>   3 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc 
> b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc
> index 32c8deb3b1d6..f38ffd034a59 100644
> --- a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc
> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc
> @@ -172,6 +172,7 @@
>     # ARM General Interrupt Controller
>     #
>     gArmTokenSpaceGuid.PcdGicDistributorBase|0x0
> +  gArmTokenSpaceGuid.PcdGicRedistributorBase|0x0
>     gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase|0x0
>
>     ## PL031 RealTimeClock
> diff --git a/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.c 
> b/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.c
> index 1d44f9ba02b3..e8bbea0847c0 100644
> --- a/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.c
> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.c
> @@ -46,6 +46,7 @@ typedef enum {
>     PropertyTypeTimer,
>     PropertyTypePsci,
>     PropertyTypeFwCfg,
> +  PropertyTypeGicV3,
>   } PROPERTY_TYPE;
>
>   typedef struct {
> @@ -62,6 +63,7 @@ STATIC CONST PROPERTY CompatibleProperties[] = {
>     { PropertyTypeTimer,   "arm,armv8-timer"     },
>     { PropertyTypePsci,    "arm,psci-0.2"        },
>     { PropertyTypeFwCfg,   "qemu,fw-cfg-mmio"    },
> +  { PropertyTypeGicV3,   "arm,gic-v3"          },
>     { PropertyTypeUnknown, ""                    }
>   };
>
> @@ -114,7 +116,7 @@ InitializeVirtFdtDxe (
>     VIRTIO_TRANSPORT_DEVICE_PATH   *DevicePath;
>     EFI_HANDLE                     Handle;
>     UINT64                         RegBase;
> -  UINT64                         DistBase, CpuBase;
> +  UINT64                         DistBase, CpuBase, RedistBase;
>     CONST INTERRUPT_PROPERTY       *InterruptProp;
>     INT32                          SecIntrNum, IntrNum, VirtIntrNum, HypIntrNum;
>     CONST CHAR8                    *PsciMethod;
> @@ -256,6 +258,33 @@ InitializeVirtFdtDxe (
>         DEBUG ((EFI_D_INFO, "Found GIC @ 0x%Lx/0x%Lx\n", DistBase, CpuBase));
>         break;
>
> +    case PropertyTypeGicV3:
> +      //
> +      // The GIC v3 DT binding describes a series of at least 3 physical base
> +      // addresses: the distributor interface (GICD), at least one 
> redistributor
> +      // interface (GICR) and the CPU interface (GICC).
> +      // Under virtualization, we assume that the first redistributor interface
> +      // listed covers the boot CPU. Also, our GICv3 driver only supports the
> +      // system register CPU interface, so we can safely ignore the MMIO 
> version
> +      // which is listed after the sequence of redistributor interfaces.
> +      // This means we are only interested in the first two memory regions
> +      // supplied, and ignore everything else.
> +      //
> +      ASSERT (Len >= 32);
> +
> +      DistBase = fdt64_to_cpu (((UINT64 *)RegProp)[0]);
> +      ASSERT (DistBase < MAX_UINT32);
> +
> +      RedistBase = fdt64_to_cpu (((UINT64 *)RegProp)[2]);
Add a comment about [2] to understand where it is come from
> +      ASSERT (RedistBase < MAX_UINT32);
> +
> +      PcdSet32 (PcdGicDistributorBase, (UINT32)DistBase);
> +      PcdSet32 (PcdGicRedistributorBase, (UINT32)RedistBase);
> +
> +      DEBUG ((EFI_D_INFO, "Found GIC v3 (re)distributor @ 0x%Lx (0x%Lx)\n",
> +        DistBase, RedistBase));
> +      break;
> +
>       case PropertyTypeRtc:
>         ASSERT (Len == 16);
>
> diff --git a/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.inf 
> b/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.inf
> index 514ce2fdf658..dbf0f8eb54bc 100644
> --- a/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.inf
> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.inf
> @@ -51,6 +51,7 @@
>     gArmVirtualizationTokenSpaceGuid.PcdFwCfgSelectorAddress
>     gArmVirtualizationTokenSpaceGuid.PcdFwCfgDataAddress
>     gArmTokenSpaceGuid.PcdGicDistributorBase
> +  gArmTokenSpaceGuid.PcdGicRedistributorBase
>     gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase
>     gArmTokenSpaceGuid.PcdArmArchTimerSecIntrNum
>     gArmTokenSpaceGuid.PcdArmArchTimerIntrNum

-- IMPORTANT NOTICE: The contents of this email and any attachments are 
confidential and may also be privileged. If you are not the intended recipient, 
please notify the sender immediately and do not disclose the contents to any 
other person, use it for any purpose, or store or copy the information in any 
medium.  Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in 
England & Wales, Company No:  2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, 
Registered in England & Wales, Company No:  2548782


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel