Re: [PATCH 2/6] xen/arm: domain_build: Inherit GIC's interrupt-parent from host device tree

[Julien Grall <julien.grall@arm.com>]

Hello,

On 06/04/2017 20:47, Chris Patterson wrote:
> From: "Chris Patterson" <pattersonc@ainfosec.com>
>
> Currently, the interrupt parent is left undefined during creation in
> make_gic_node().  In cases where a non-GIC interrupt controller is present,
> this can lead to incorrect assignment of interrupt parents.
>
> On the Tegra, the gic's interrupt parent is set to itself:
>
> 	gic: interrupt-controller@0,50041000 {
> 		compatible = "arm,gic-400";
> 		#interrupt-cells = <3>;
> 		interrupt-controller;
> 		reg = <0x0 0x50041000 0x0 0x1000>,
> 		      <0x0 0x50042000 0x0 0x2000>,
> 		      <0x0 0x50044000 0x0 0x2000>,
> 		      <0x0 0x50046000 0x0 0x2000>;
> 		interrupts = <GIC_PPI 9
> 			(GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
> 		interrupt-parent = <&gic>;
> 	};
>
> To prevent the hardware domain from assuming the Legacy Interrupt Controller
> (lic) as the GIC's interrupt-parent, this change explicitly assigns
> the interrupt-parent property from the host device tree.
>
> Authored-by: Kyle Temkin <temkink@ainfosec.com>

We use "From: " for the author and it is different here. So who wrote 
this code?

> Signed-off-by: Kyle Temkin <temkink@ainfosec.com>
> Signed-off-by: Chris Patterson <pattersonc@ainfosec.com>
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
>
> changes from rfc:
> - commit message documentation improvements
>
> ---
>  xen/arch/arm/domain_build.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index de59e5f..cb66304 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -778,8 +778,8 @@ static int make_gic_node(const struct domain *d, void *fdt,
>  {
>      const struct dt_device_node *gic = dt_interrupt_controller;
>      int res = 0;
> -    const void *addrcells, *sizecells;
> -    u32 addrcells_len, sizecells_len;
> +    const void *addrcells, *sizecells, *iparent;
> +    u32 addrcells_len, sizecells_len, iparent_len;
>
>      /*
>       * Xen currently supports only a single GIC. Discard any secondary
> @@ -809,6 +809,19 @@ static int make_gic_node(const struct domain *d, void *fdt,
>              return res;
>      }
>
> +    /*
> +     * If available, explicitly inherit interrupt-parent property from host
> +     * device tree.  This will prevent the risk of incorrect identification
> +     * of the parent on platforms with more than one interrupt controller.
> +     */
> +    iparent = dt_get_property(gic, "interrupt-parent", &iparent_len);
> +    if ( iparent )
> +    {
> +        res = fdt_property(fdt, "interrupt-parent", iparent, iparent_len);
> +        if ( res )
> +          return res;
> +    }
> +
>      addrcells = dt_get_property(gic, "#address-cells", &addrcells_len);
>      if ( addrcells )
>      {
>

Cheers,

-- 
Julien Grall

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