On Thu, 3 May 2018 11:36:35 +0800
Peter Xu <peterx@xxxxxxxxxx> wrote:
> On Tue, May 01, 2018 at 10:43:32AM -0600, Alex Williamson wrote:
>
> [...]
>
> > @@ -743,6 +843,60 @@ static void vfio_nvidia_quirk_mirror_write(void *opaque, hwaddr addr,
> > addr + mirror->offset, data, size);
> > trace_vfio_quirk_nvidia_bar0_msi_ack(vdev->vbasedev.name);
> > }
> > +
> > + /*
> > + * Automatically add an ioeventfd to handle any repeated write with the
> > + * same data and size above the standard PCI config space header. This is
> > + * primarily expected to accelerate the MSI-ACK behavior, such as noted
> > + * above. Current hardware/drivers should trigger an ioeventfd at config
> > + * offset 0x704 (region offset 0x88704), with data 0x0, size 4.
> > + *
> > + * The criteria of 10 successive hits is arbitrary but reliably adds the
> > + * MSI-ACK region. Note that as some writes are bypassed via the ioeventfd,
> > + * the remaining ones have a greater chance of being seen successively.
> > + * To avoid the pathological case of burning up all of QEMU's open file
> > + * handles, arbitrarily limit this algorithm from adding no more than 10
> > + * ioeventfds, print an error if we would have added an 11th, and then
> > + * stop counting.
> > + */
> > + if (!vdev->no_kvm_ioeventfd &&
> > + addr > PCI_STD_HEADER_SIZEOF && last->added < MAX_DYN_IOEVENTFD + 1) {
> > + 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) {
>
> I'm not sure whether the quirks list can be a long one, otherwise not
> sure whether we can just cache the quirk pointer inside the mirror to
> avoid the loop.
The list here only has two entries, but it does seem simple enough to
add a VFIOQuirk* to LastData which we set when it's allocated to avoid
this loop. I'll test and post and update tomorrow.
> > + if (quirk->data == mirror) {
> > + QLIST_INSERT_HEAD(&quirk->ioeventfds,
> > + ioeventfd, next);
> > + break;
> > + }
> > + }
>
> [...]
>
> > +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;
>
> Would it possible in the future that match_data will be false?
> Otherwise I'm not sure whether we can even omit this field.
I don't see how we could, and you're right, it's pretty useless. I'll
drop it in the next version. Thanks!
Alex
