Re: [PATCH v10 00/10]vhost-vdpa: add support for configure interrupt

[Stefan Hajnoczi]

On Thu, Nov 11, 2021 at 12:41:02PM +0800, Jason Wang wrote:
> 
> 在 2021/11/8 下午6:53, Stefan Hajnoczi 写道:
> > On Fri, Nov 05, 2021 at 12:48:17AM +0800, Cindy Lu wrote:
> > > these patches add the support for configure interrupt
> > > 
> > > These codes are all tested in vp-vdpa (support configure interrupt)
> > > vdpa_sim (not support configure interrupt), virtio tap device
> > > 
> > > test in virtio-pci bus and virtio-mmio bus
> > Hi,
> > vhost-user has a configuration space change notification but it uses a
> > slave channel message (VHOST_USER_SLAVE_CONFIG_CHANGE_MSG) instead of an
> > eventfd. Ideally the vhost kernel ioctl and vhost-user interfaces would
> > follow the same design.
> > 
> > I'm concerned "common" vhost code is going to end up with lots of
> > callbacks that are not available uniformly across vhost kernel, vdpa,
> > and vhost-user. That makes it hard to understand and debug vhost, plus
> > differences make it harder to to correctly extend these interfaces in
> > the future.
> > 
> > Is the decision to a new eventfd-based interface instead of
> > vhost_chr_read/write() deliberate?
> 
> 
> I think this is a good question. Here're some reasons for using eventfd from
> the kernel perspective:
> 
> 1) the eventfd is used for relaying interrupts for vqs, so we choose to use
> that for the config interrupt
> 2) make it possible to be used for irq bypassing (posted interrupt)

Interesting point. Posted interrupts aren't supported by vhost-user's
slave channel message. Since configuration change notifications are rare
it's probably not a performance problem, but still.

This makes me think vhost-user's approach is sub-optimal, it should have
been an eventfd :(. Maybe the idea was that a slave message is less
complex than adding an additional interface to set a configuration
change notification eventfd.

Let's not worry about it too much. I guess in the long run vhost-user
can be rebased on top of the vDPA kernel interface (I wrote about that
here:
https://blog.vmsplice.net/2020/09/on-unifying-vhost-user-and-virtio.html).

Stefan

signature.asc
Description: PGP signature