qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Pavel Fedin
Subject: Re: [Qemu-devel] [PATCH v7 5/6] Initial implementation of vGICv3
Date: Fri, 31 Jul 2015 12:32:59 +0300

 Hello!

> On 24 July 2015 at 10:55, Pavel Fedin <address@hidden> wrote:
> > Get/put routines are missing, live migration is not possible.
> 
> This commit message could do with being made a bit less terse.

 Ok. Subject was self-explanatory, so i didn't have much to add.

> > +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.

> > +    /* Try to create the device via the device control API */
> > +    s->dev_fd = -1;
> > +    ret = kvm_create_device(kvm_state, KVM_DEV_TYPE_ARM_VGIC_V3, false);
> > +    if (ret >= 0) {
> > +        s->dev_fd = ret;
> > +    } else if (ret != -ENODEV && ret != -ENOTSUP) {
> 
> Why aren't ENODEV and ENOTSUP fatal errors like other errnos?

 Stupid leftover from vGICv2 code, copypasted. I have already removed it in my 
repo, just decided to delay the respin until i know the fate of this: 
https://lists.gnu.org/archive/html/qemu-devel/2015-07/msg05778.html

> 
> > +        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?

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia





reply via email to

[Prev in Thread] Current Thread [Next in Thread]