On 31 July 2015 at 10:32, Pavel Fedin <p.fe...@samsung.com> wrote:
> Hello!
>
>> On 24 July 2015 at 10:55, Pavel Fedin <p.fe...@samsung.com> 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
