Re: [PATCH v8 2/4] xen/arm: Check for interrupt controller directly

[Julien Grall <julien.grall@linaro.org>]

On 03/03/15 15:07, Frediano Ziglio wrote:
> 2015-03-03 14:45 GMT+00:00 Julien Grall <julien.grall@linaro.org>:
>> Hello Frediano,
>>
>> On 03/03/15 11:19, Frediano Ziglio wrote:
>>> This check allow to detect mail interrupt controller even if it does
>>
>> main
>>
>>> not match one of the standard ones.
>>> This allow boards with non standard controllers to be handled correctly
>>> without having to manually edit the global list every time.
>>>
>>> Signed-off-by: Frediano Ziglio <frediano.ziglio@huawei.com>
>>> ---
>>>  xen/arch/arm/domain_build.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>>> index 9f1f59f..83951a3 100644
>>> --- a/xen/arch/arm/domain_build.c
>>> +++ b/xen/arch/arm/domain_build.c
>>> @@ -1069,7 +1069,7 @@ static int handle_node(struct domain *d, struct kernel_info *kinfo,
>>>
>>>      /* Replace these nodes with our own. Note that the original may be
>>>       * used_by DOMID_XEN so this check comes first. */
>>> -    if ( dt_match_node(gic_matches, node) )
>>> +    if ( node == dt_interrupt_controller || dt_match_node(gic_matches, node) )
>>>          return make_gic_node(d, kinfo->fdt, node);
>>
>> What about if the device tree exposes multiple GICs? By mistake we will
>> expose the secondaries GIC if they are not standard.
>>
>> As I suggested on a previous mail, I would prefer to introduce a new
>> callback to check if the node is a GIC.
>>
> 
> I think something like this would make all happy (I hope, I tested and
> works correctly on my board):

This looks good to me.

Regards,

> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 9f1f59f..aaa3b97 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1035,12 +1035,6 @@ static int handle_node(struct domain *d, struct
> kernel_info *kinfo,
>          DT_MATCH_COMPATIBLE("arm,armv7-timer-mem"),
>          { /* sentinel */ },
>      };
> -    static const struct dt_device_match gic_matches[] __initconst =
> -    {
> -        DT_MATCH_GIC_V2,
> -        DT_MATCH_GIC_V3,
> -        { /* sentinel */ },
> -    };
>      static const struct dt_device_match timer_matches[] __initconst =
>      {
>          DT_MATCH_TIMER,
> @@ -1069,7 +1063,7 @@ static int handle_node(struct domain *d, struct
> kernel_info *kinfo,
> 
>      /* Replace these nodes with our own. Note that the original may be
>       * used_by DOMID_XEN so this check comes first. */
> -    if ( dt_match_node(gic_matches, node) )
> +    if ( device_get_class(node) == DEVICE_GIC )
>          return make_gic_node(d, kinfo->fdt, node);
>      if ( dt_match_node(timer_matches, node) )
>          return make_timer_node(d, kinfo->fdt, node);
> 


-- 
Julien Grall