On 07/12/2018 03:37 PM, Peter Maydell wrote:
> On 29 June 2018 at 14:29, Luc Michel <luc.mic...@greensocs.com> wrote:
>> Add the read/write functions to handle accesses to the vCPU interface.
>> Those accesses are forwarded to the real CPU interface, with the CPU id
>> being converted to the corresponding vCPU id (vCPU id = CPU id +
>> GIC_NCPU).
>>
>> As for the CPU interface, we create a base region for the vCPU interface
>> that fetches the current vCPU id using the current_cpu global variable, and
>> one mirror region per vCPU which maps to that specific vCPU id. This is
>> required by the GIC architecture specification.
>
>
>> static void arm_gic_realize(DeviceState *dev, Error **errp)
>> {
>> /* Device instance realize function for the GIC sysbus device */
>> @@ -1531,7 +1590,7 @@ static void arm_gic_realize(DeviceState *dev, Error
>> **errp)
>> }
>>
>> /* This creates distributor and main CPU interface (s->cpuiomem[0]) */
>
> Can we also update this comment to indicate what virt-related
> memory regions are being created?
>
>> - gic_init_irqs_and_mmio(s, gic_set_irq, gic_ops, NULL);
>> + gic_init_irqs_and_mmio(s, gic_set_irq, gic_ops, gic_virt_ops);
>>
>> /* Extra core-specific regions for the CPU interfaces. This is
>> * necessary for "franken-GIC" implementations, for example on
>> @@ -1547,6 +1606,16 @@ static void arm_gic_realize(DeviceState *dev, Error
>> **errp)
>> &s->backref[i], "gic_cpu", 0x100);
>> sysbus_init_mmio(sbd, &s->cpuiomem[i+1]);
>> }
>> +
>> + if (s->virt_extn) {
>
> ...and a comment about what these regions are for.
>
> What requires these per-core regions anyway? There's no way to
> specify them in the device tree bindings for Linux, which AFAIK
> only cares about using the "vcpu i/f for this core" registers.
> I don't think the GIC-400 has these. (It does have per-cpu
> aliases of the GICH_* registers, but this patchset doesn't seem
> to implement those.)
My mistake. I misread the specifications. Those aliases should target
the virtual interfaces, and not the virtual CPU interfaces. I'll change
that.
>
>> + for (i = 0; i < s->num_cpu; i++) {
>> + memory_region_init_io(&s->vcpuiomem[i + 1], OBJECT(s),
>> + &gic_vcpu_ops, &s->backref[i],
>> + "gic_vcpu", 0x2000);
>> + sysbus_init_mmio(sbd, &s->vcpuiomem[i + 1]);
>> + }
>> + }
>> +
>> }
>
> thanks
> -- PMM
>
signature.asc
Description: OpenPGP digital signature
