Re: [Xen-devel] [PATCH] ARM: GICv3: copy Dom0 GICv3 reg property from host DT

[André Przywara <andre.przywara@xxxxxxx>]

On 30/01/18 19:04, Stefano Stabellini wrote:
> On Tue, 30 Jan 2018, Andre Przywara wrote:
>> At the moment we re-generate the Dom0 GICv3 DT node, by creating the
>> "reg" property from scratch using our previously parsed and
>> translated(!) host addresses. However we then write the *absolute*
>> addresses into the new node, not considering possible "range" mappings
>> in any of the GIC's parent nodes. So whenever one of the parents has a
>> non-empty ranges property, Dom0 will wrongly translate the addresses.
>> Properly incorporating the ranges properties sounds tedious, so let's
>> just copy the first part of the reg property instead (as we do for GICv2),
>> since the addresses for Dom0 are identical to those from the hardware.
>>
>> The mainline kernel DT for the Espressobin board with an Marvell 3720 SoC
>> has the GIC in such an translated bus, so this patch allows this board
>> to boot properly (after adding support for the SoC's UART).
>>
>> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxx>
> 
> There is one code style issue below, but I'll fix it on commit.

Argh, indeed, Julien found this as well.
Thanks for the smooth handling!

Cheers,
Andre.

> Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> 
>> ---
>>  xen/arch/arm/gic-v3.c | 29 +++++++++++------------------
>>  1 file changed, 11 insertions(+), 18 deletions(-)
>>
>> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
>> index a0d290b55c..6b17abd0a1 100644
>> --- a/xen/arch/arm/gic-v3.c
>> +++ b/xen/arch/arm/gic-v3.c
>> @@ -1147,10 +1147,9 @@ static int gicv3_make_hwdom_dt_node(const struct 
>> domain *d,
>>                                      const struct dt_device_node *gic,
>>                                      void *fdt)
>>  {
>> -    const void *compatible = NULL;
>> -    uint32_t len;
>> -    __be32 *new_cells, *tmp;
>> -    int i, res = 0;
>> +    const void *compatible, *hw_reg;
>> +    uint32_t len, new_len;
>> +    int res;
>>  
>>      compatible = dt_get_property(gic, "compatible", &len);
>>      if ( !compatible )
>> @@ -1173,27 +1172,21 @@ static int gicv3_make_hwdom_dt_node(const struct 
>> domain *d,
>>      if ( res )
>>          return res;
>>  
>> -    len = dt_cells_to_size(dt_n_addr_cells(gic) + dt_n_size_cells(gic));
>> +    new_len = dt_cells_to_size(dt_n_addr_cells(gic) + dt_n_size_cells(gic));
>>      /*
>>       * GIC has two memory regions: Distributor + rdist regions
>>       * CPU interface and virtual cpu interfaces accessesed as System 
>> registers
>>       * So cells are created only for Distributor and rdist regions
>>       */
>> -    len = len * (d->arch.vgic.nr_regions + 1);
>> -    new_cells = xzalloc_bytes(len);
>> -    if ( new_cells == NULL )
>> -        return -FDT_ERR_XEN(ENOMEM);
>> -
>> -    tmp = new_cells;
>> -
>> -    dt_set_range(&tmp, gic, d->arch.vgic.dbase, SZ_64K);
>> +    new_len = new_len * (d->arch.vgic.nr_regions + 1);
>>  
>> -    for ( i = 0; i < d->arch.vgic.nr_regions; i++ )
>> -        dt_set_range(&tmp, gic, d->arch.vgic.rdist_regions[i].base,
>> -                     d->arch.vgic.rdist_regions[i].size);
>> +    hw_reg = dt_get_property(gic, "reg", &len);
>> +    if ( !hw_reg )
>> +        return -FDT_ERR_XEN(ENOENT);
>> +    if ( new_len > len )
>> +    return -FDT_ERR_XEN(ERANGE);
>>  
>> -    res = fdt_property(fdt, "reg", new_cells, len);
>> -    xfree(new_cells);
>> +    res = fdt_property(fdt, "reg", hw_reg, new_len);
>>      if ( res )
>>          return res;
>>  
>> -- 
>> 2.14.1
>>


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel