Re: [Qemu-devel] [PATCH v7 5/6] Initial implementation of vGICv3

[Peter Maydell]

On 31 July 2015 at 10:32, Pavel Fedin <address@hidden> wrote:
>  Hello!
>
>> On 24 July 2015 at 10:55, Pavel Fedin <address@hidden> wrote:
>> > +static void kvm_arm_gicv3_reset(DeviceState *dev)
>> > +{
>> > +    GICv3State *s = ARM_GICV3_COMMON(dev);
>> > +    KVMARMGICv3Class *kgc = KVM_ARM_GICV3_GET_CLASS(s);
>> > +
>> > +    DPRINTF("Reset\n");
>> > +
>> > +    kgc->parent_reset(dev);
>> > +    kvm_arm_gicv3_put(s);
>> > +}
>>
>> If we don't currently do anything in reset then does the GIC just
>> go wrong on a VM reset?
>
>  No it doesn't, reset works.

So who resets the KVM internal GIC state and how?

>> > +        error_setg_errno(errp, -ret, "error creating in-kernel VGIC");
>> > +        return;
>> > +    }
>> > +
>> > +    if (kvm_gic_supports_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_NR_IRQS, 
>> > 0)) {
>>
>> Is there any kernel which supports GICv3 but does not support
>> this attribute? I would hope not, in which case we can skip the
>> conditional check for support.
>>
>> > +        uint32_t numirqs = s->num_irq;
>> > +        DPRINTF("KVM_DEV_ARM_VGIC_GRP_NR_IRQS = %u\n", numirqs);
>> > +        kvm_gic_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_NR_IRQS,
>> > +                       0, 0, &numirqs, 1);
>> > +    }
>> > +
>> > +    /* Tell the kernel to complete VGIC initialization now */
>> > +    if (kvm_gic_supports_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,
>> > +                              KVM_DEV_ARM_VGIC_CTRL_INIT)) {
>>
>> Ditto.
>
>  I intentionally put some tracing to these conditions. On my
> system KVM_DEV_ARM_VGIC_GRP_NR_IRQS is supported and
> KVM_DEV_ARM_VGIC_CTRL_INIT is not. So will it always be this way?

That is weird. I thought that the kernel devs had agreed that all
new VGIC code should require explicit initialization (ie that
lazy init-on-first-time-it's-needed was only there for legacy
guests and the old GICv2). The lazy-init stuff was a big source
of bugs where things weren't inited at the point where you
expected, which is why it should have gone away...

-- PMM