RE: [virtio-comment] [RFC PATCH] VIRTIO_F_USED_EVENT_AUTO_DISABLE: add new used buffer notification suppression mechanism

[Lege Wang <lege.wang@jaguarmicro.com>]

hi,


> >
> > > And for that may be current notification can be made 64-bits or as
> > > suggested in your patch to have new location.
> > After some thoughts, I decide to just make current notification extended to
> > 64-bits for simplicity.
> I think having 2 32-bit notification registers is better.
> Existing one for avail desc posting.
> New 32-bit one for used buffer consumption and re-enablement of
> notification.
> This way we get good backward compatibility and also the optional behavior.
> 3rd advantage of keeping them as two is when used buffer notification
> enablement is done, it does not need to touch the device for avail posting.
Yes, indeed this is what I have in mind, I did not clarify it clearly.
These two 32-bits registers are adjacent, and both are 4-byte aligned.

> 
> In some systems, a 64-bit single PCIe TLP write may not be possible, through
> rare, it may be there.
> So having it as separate 32-bit used buffer notification is good.
OK, I see.

> 
> > Yes, I understand your concerns. Initially I intended to add new PCI capability
> > to describe this new notification area, but found that this new capability
> > would be similar to virtio_pci_notify_cap, and will introduce many duplicate
> > descriptions in virtio spec doc. If we choose to extent current notification
> area,
> > there're no needs to touch current pci capability, the corresponding queue
> > notify address to enable used buffer notification within a BAR for a virtqueue
> > would look like below:
> >   cap.offset + queue_notify_off * notify_off_multiplier + 4
> >
> Just to double check,
> 
> If avail buffer notification is at offset = X
> Used buffer notification is at offset X + 4.
> Right?
Yes.

> 
> > > If this multiplier is not useful, maybe it should be added only if needed.
> > > Did you add it because to match the current flexibility of the spec,
> > > or your hw actually needs this multiplier?
> > Yes, just to match the current spec ??
> >
> Ok. that explains.
> Yes lets keep things simple as X, and X+4 offsets?
Yes, I think so.

Regards.
lege.wang
> 
> > Regards,
> > lege.wang