Re: [Qemu-devel] [RFC 6/8] hw/arm/virt: Allow GICv3 DT node with multiple redistributor regions

2018-04-13 Thread Auger Eric
Hi Peter,

On 13/04/18 15:36, Peter Maydell wrote:
> On 27 March 2018 at 15:15, Eric Auger  wrote:
>> This patch adds the GICState handle in the virtual machine state and
>> allows to create a GIC device tree node advertising multiple redistributor
>> regions.
>>
>> There is one range per distributor region following the GIC distributor.
>> Please refer to kernel documentation for further details:
>> Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt
>>
>> Signed-off-by: Eric Auger 
> 
>> +
>> +num_reg_values =  4 * (s->nb_redist_regions + 1);
>> +regs = g_new0(uint64_t, num_reg_values);
>> +qemu_fdt_setprop_cell(vms->fdt, "/intc", "#redistributor-regions",
>> +  s->nb_redist_regions);
>> +regs[0] = 2;
>> +regs[1] = vms->memmap[VIRT_GIC_DIST].base;
>> +regs[2] = 2;
>> +regs[3] = vms->memmap[VIRT_GIC_DIST].size;
>> +
>> +for (r = 1; r <= s->nb_redist_regions; r++) {
>> +regs[4 * r] = 2;
>> +regs[4 * r  + 1] = s->redist_region[r - 1].base;
>> +regs[4 * r  + 2] = 2;
>> +/* count redistributors of 2 x 64kB pages */
>> +regs[4 * r  + 3] = (uint64_t)s->redist_region[r - 1].count << 
>> 17;
> 
> Board code shouldn't be reaching into the GIC device state struct like this.

I will add a dedicated API.

Thanks

Eric
> 
>> +}
>> +qemu_fdt_setprop_sized_cells_from_array(vms->fdt, "/intc", "reg",
>> +num_reg_values / 2, regs);
>> +g_free(regs);
>> +
>>  if (vms->virt) {
>>  qemu_fdt_setprop_cells(vms->fdt, "/intc", "interrupts",
> 
> thanks
> -- PMM
> 



Re: [Qemu-devel] [RFC 6/8] hw/arm/virt: Allow GICv3 DT node with multiple redistributor regions

2018-04-13 Thread Peter Maydell
On 27 March 2018 at 15:15, Eric Auger  wrote:
> This patch adds the GICState handle in the virtual machine state and
> allows to create a GIC device tree node advertising multiple redistributor
> regions.
>
> There is one range per distributor region following the GIC distributor.
> Please refer to kernel documentation for further details:
> Documentation/devicetree/bindings/interrupt-controller/arm,gic-v3.txt
>
> Signed-off-by: Eric Auger 

> +
> +num_reg_values =  4 * (s->nb_redist_regions + 1);
> +regs = g_new0(uint64_t, num_reg_values);
> +qemu_fdt_setprop_cell(vms->fdt, "/intc", "#redistributor-regions",
> +  s->nb_redist_regions);
> +regs[0] = 2;
> +regs[1] = vms->memmap[VIRT_GIC_DIST].base;
> +regs[2] = 2;
> +regs[3] = vms->memmap[VIRT_GIC_DIST].size;
> +
> +for (r = 1; r <= s->nb_redist_regions; r++) {
> +regs[4 * r] = 2;
> +regs[4 * r  + 1] = s->redist_region[r - 1].base;
> +regs[4 * r  + 2] = 2;
> +/* count redistributors of 2 x 64kB pages */
> +regs[4 * r  + 3] = (uint64_t)s->redist_region[r - 1].count << 17;

Board code shouldn't be reaching into the GIC device state struct like this.

> +}
> +qemu_fdt_setprop_sized_cells_from_array(vms->fdt, "/intc", "reg",
> +num_reg_values / 2, regs);
> +g_free(regs);
> +
>  if (vms->virt) {
>  qemu_fdt_setprop_cells(vms->fdt, "/intc", "interrupts",

thanks
-- PMM