Re: [PATCH v2 3/4] vfio/quirks: ioeventfd quirk acceleration

Alex Williamson
2018-05-03T14:48:16Z

On Thu, 3 May 2018 16:33:25 +0200
Auger Eric <eric.auger@redhat.com> wrote:
> > + if (!vdev->no_kvm_ioeventfd &&
> > + addr > PCI_STD_HEADER_SIZEOF && last->added < MAX_DYN_IOEVENTFD + 1) {
> nit: <= MAX_DYN_IOEVENTFD?

Done, also the addr test should be >=, it doesn't make sense to exclude
the first byte after the standard capabilities, though for this quirk
we know we're looking well above this.

> > + if (addr != last->addr || data != last->data || size != last->size) {
> > + last->addr = addr;
> > + last->data = data;
> > + last->size = size;
> > + last->hits = 1;
> > + } else if (++last->hits >= HITS_FOR_IOEVENTFD) {
> > + if (last->added < MAX_DYN_IOEVENTFD) {
> > + VFIOIOEventFD *ioeventfd;
> > + ioeventfd = vfio_ioeventfd_init(vdev, mirror->mem, addr, size,
> > + data, &vdev->bars[mirror->bar].region,
> > + mirror->offset + addr, true);
> > + if (ioeventfd) {
> > + VFIOQuirk *quirk;
> > +
> > + QLIST_FOREACH(quirk,
> > + &vdev->bars[mirror->bar].quirks, next) {
> > + if (quirk->data == mirror) {
> > + QLIST_INSERT_HEAD(&quirk->ioeventfds,
> > + ioeventfd, next);
> > + break;
> > + }
> > + }
> > +
> > + assert(quirk != NULL); /* Check not found */
> > +
> > + last->added++;
> > + }
> > + } else {
> > + last->added++;
> > +
> > + error_report("NVIDIA ioeventfd queue full for %s, unable to "
> > + "accelerate 0x%"HWADDR_PRIx", data 0x%"PRIx64", "
> > + "size %u", vdev->vbasedev.name, addr, data, size);
> nit: warn_report?

Done

...
> > +typedef struct VFIOIOEventFD {
> > + QLIST_ENTRY(VFIOIOEventFD) next;
> > + MemoryRegion *mr;
> > + hwaddr addr;
> > + unsigned size;
> > + uint64_t data;
> > + EventNotifier e;
> > + VFIORegion *region;
> > + hwaddr region_addr;
> > + bool match_data;
> > + bool dynamic;
> maybe the "dynamic" semantics may be docuemnted in the code and not only
> in the commit message.

Done, added

/* Added runtime, removed on device reset */

Thanks!

Alex