Messages in this thread Patch in this message | | | Date | Fri, 21 Aug 2015 11:29:46 +0200 | From | Cornelia Huck <> | Subject | Re: [PATCH 2/3] kvm: don't register wildcard MMIO EVENTFD on two buses |
| |
On Fri, 21 Aug 2015 16:03:52 +0800 Jason Wang <jasowang@redhat.com> wrote:
> diff --git a/virt/kvm/eventfd.c b/virt/kvm/eventfd.c > index 9ff4193..834a409 100644 > --- a/virt/kvm/eventfd.c > +++ b/virt/kvm/eventfd.c > @@ -838,11 +838,6 @@ kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args) > > kvm_iodevice_init(&p->dev, &ioeventfd_ops); > > - ret = kvm_io_bus_register_dev(kvm, bus_idx, p->addr, p->length, > - &p->dev); > - if (ret < 0) > - goto unlock_fail; > - > /* When length is ignored, MMIO is also put on a separate bus, for > * faster lookups.
You probably want to change this comment as well?
> */ > @@ -850,9 +845,15 @@ kvm_assign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args)
Unfortunately snipped by diff, but the check here is on !len && !PIO, which only does the desired thing as VIRTIO_CCW always uses len == 8. Should the check be for !len && MMIO instead?
> ret = kvm_io_bus_register_dev(kvm, KVM_FAST_MMIO_BUS, > p->addr, 0, &p->dev); > if (ret < 0) > - goto register_fail; > + goto unlock_fail; > + } else { > + ret = kvm_io_bus_register_dev(kvm, bus_idx, p->addr, p->length, > + &p->dev); > + if (ret < 0) > + goto unlock_fail; > }
Hm... maybe the following would be more obvious:
my_bus = (p->length == 0) && (bus_idx == KVM_MMIO_BUS) ? KVM_FAST_MMIO_BUS : bus_idx; ret = kvm_io_bus_register_dev(kvm, my_bus, p->addr, p->length, &pdev->dev);
> > + > kvm->buses[bus_idx]->ioeventfd_count++; > list_add_tail(&p->list, &kvm->ioeventfds);
(...)
> @@ -900,10 +899,11 @@ kvm_deassign_ioeventfd(struct kvm *kvm, struct kvm_ioeventfd *args) > if (!p->wildcard && p->datamatch != args->datamatch) > continue; > > - kvm_io_bus_unregister_dev(kvm, bus_idx, &p->dev); > if (!p->length) { > kvm_io_bus_unregister_dev(kvm, KVM_FAST_MMIO_BUS, > &p->dev); > + } else { > + kvm_io_bus_unregister_dev(kvm, bus_idx, &p->dev); > }
Similar comments here... do you want to check for bus_idx == KVM_MMIO_BUS as well?
> kvm->buses[bus_idx]->ioeventfd_count--; > ioeventfd_release(p);
| |