Re: [dpdk-dev] [PATCH 5/9] vfio_user: implement interrupt related APIs

["Xia, Chenbo" <chenbo.xia@intel.com>]

Hi Jingjing,

> -----Original Message-----
> From: Wu, Jingjing <jingjing.wu@intel.com>
> Sent: Wednesday, December 30, 2020 9:05 AM
> To: Xia, Chenbo <chenbo.xia@intel.com>; dev@dpdk.org; thomas@monjalon.net;
> david.marchand@redhat.com
> Cc: stephen@networkplumber.org; Liang, Cunming <cunming.liang@intel.com>; Lu,
> Xiuchun <xiuchun.lu@intel.com>; Li, Miao <miao.li@intel.com>
> Subject: RE: [PATCH 5/9] vfio_user: implement interrupt related APIs
> 
> >  	if ((cmd == VFIO_USER_DMA_MAP || cmd == VFIO_USER_DMA_UNMAP
> > ||
> > +		cmd == VFIO_USER_DEVICE_SET_IRQS ||
> >  		cmd == VFIO_USER_DEVICE_RESET)
> >  		&& dev->ops->lock_dp) {
> >  		dev->ops->lock_dp(dev_id, 1);
> 
> About cmd "VFIO_USER_REGION_WRITE", irq setting would cause update_status to
> iavfbe device.
> Where will the lock be?

If emulated device needs some lock in slow-path region rw, IMHO, it should be implemented
in rte_vfio_user_reg_acc_t, which is the region rw callback. As region rw has different
behavior for different registers, it's better to make it device-specific 😊

Thanks!
Chenbo

> 
> > @@ -871,7 +1056,8 @@ static int vfio_user_message_handler(int dev_id, int fd)
> >  		if (vfio_user_is_ready(dev) && dev->ops->new_device)
> >  			dev->ops->new_device(dev_id);
> >  	} else {
> > -		if ((cmd == VFIO_USER_DMA_MAP || cmd ==
> > VFIO_USER_DMA_UNMAP)
> > +		if ((cmd == VFIO_USER_DMA_MAP || cmd ==
> > VFIO_USER_DMA_UNMAP
> > +			|| cmd == VFIO_USER_DEVICE_SET_IRQS)
> >  			&& dev->ops->update_status)
> >  			dev->ops->update_status(dev_id);
> >  	}
> > @@ -898,6 +1084,7 @@ static int vfio_user_sock_read(int fd, void *data)
> >  		if (dev) {
> >  			dev->ops->destroy_device(dev_id);
> >  			vfio_user_destroy_mem_entries(dev->mem);
> > +			vfio_user_clean_irqfd(dev);
> >  			dev->is_ready = 0;
> >  			dev->msg_id = 0;
> >  		}
> > @@ -995,9 +1182,9 @@ vfio_user_start_server(struct vfio_user_server_socket
> > *sk)
> >  	}
> >
> >  	/* All the info must be set before start */
> > -	if (!dev->dev_info || !dev->reg) {
> > +	if (!dev->dev_info || !dev->reg || !dev->irqs.info) {
> >  		VFIO_USER_LOG(ERR, "Failed to start, "
> > -			"dev/reg info must be set before start\n");
> > +			"dev/reg/irq info must be set before start\n");
> >  		return -1;
> >  	}
> >