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