Re: [PATCH v5 21/22] xen/arm: ITS: Generate ITS node for Dom0

[Vijay Kilari <vijay.kilari@gmail.com>]

On Tue, Aug 18, 2015 at 1:11 AM, Julien Grall <julien.grall@citrix.com> wrote:
>
>
> On 27/07/2015 04:12, vijay.kilari@gmail.com wrote:
>>
>> From: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
>>
>> Parse host dt and generate ITS node for Dom0.
>> ITS node resides inside GIC node so when GIC node
>> is encountered look for ITS node.
>>
>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@caviumnetworks.com>
>> ---
>> v5: - Moved ITS dt node generation to ITS driver
>> v4: - Generate only one ITS node for Dom0
>>      - Replace msi-parent references to single its phandle
>> ---
>>   xen/arch/arm/domain_build.c   |   17 ++++++++++
>>   xen/arch/arm/gic-v3-its.c     |   74
>> +++++++++++++++++++++++++++++++++++++++++
>>   xen/arch/arm/gic-v3.c         |   29 ++++++++++++++++
>>   xen/arch/arm/gic.c            |   18 ++++++++++
>>   xen/include/asm-arm/gic-its.h |    3 ++
>>   xen/include/asm-arm/gic.h     |    7 ++++
>>   6 files changed, 148 insertions(+)
>>
>> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
>> index 8556afd..6b6f013 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -469,6 +469,19 @@ static int write_properties(struct domain *d, struct
>> kernel_info *kinfo,
>>               continue;
>>           }
>>
>> +        /*
>> +         * Replace all msi-parent phandle references to single ITS node
>> +         * generated for Dom0
>> +         */
>> +        if ( dt_property_name_is_equal(prop, "msi-parent") )
>
>
> I think this need more care than replacing every msi-parent without any
> checking.
>
> You need to make sure that the msi-parent points to an ITS MSI just in case
> there is other possibility of MSI.
>
> Furthermore, I would do this a ITS specific callback (gic_rewrite_node or
> smth similar) to avoid replacing msi-parent when it's not necessary. I have
> in mind GICv2M.
>
>> +        {
>> +            fdt32_t phandle = gic_get_msi_handle();
>> +            DPRINT(" Set msi-parent(ITS) phandle 0x%x\n",phandle);
>> +            fdt_property(kinfo->fdt, prop->name, (void *)&phandle,
>> +                         sizeof(phandle));
>> +            continue;
>> +        }
>> +
>>           res = fdt_property(kinfo->fdt, prop->name, prop_data, prop_len);
>>
>>           xfree(new_data);
>> @@ -875,6 +888,10 @@ static int make_gic_node(const struct domain *d, void
>> *fdt,
>>           return res;
>>
>>       res = fdt_end_node(fdt);
>> +    if ( res )
>> +        return res;
>> +
>> +    res = gic_its_hwdom_dt_node(d, node, fdt);
>
>
> Can you explain why you didn't follow my suggestion to plumb the ITS node
> creation in gic_hwdow_dt_node? IHMO there is no need of new callback.
>
> Furthermore the call is misplaced. You will end up to have a DT looking like
>
> gic {
> }
>
> gic-its {
> }
>
> rather than
>
> gic {
>   gic-its {
>   }
> }

 As discussed, I have tried to generate ITS node inside gic node and call
fdt_end_node() for gic after generating ITS node. But dom0 fails to
find ITS node in the generated device tree. Any clue?