Re: [PATCH v10 10/17] virtio: Handle device reset failure in register_virtio_device()
在 2021/8/4 下午5:07, Yongji Xie 写道: On Wed, Aug 4, 2021 at 4:54 PM Jason Wang wrote: 在 2021/8/4 下午4:50, Yongji Xie 写道: On Wed, Aug 4, 2021 at 4:32 PM Jason Wang wrote: 在 2021/8/3 下午5:38, Yongji Xie 写道: On Tue, Aug 3, 2021 at 4:09 PM Jason Wang wrote: 在 2021/7/29 下午3:34, Xie Yongji 写道: The device reset may fail in virtio-vdpa case now, so add checks to its return value and fail the register_virtio_device(). So the reset() would be called by the driver during remove as well, or is it sufficient to deal only with the reset during probe? Actually there is no way to handle failure during removal. And it should be safe with the protection of software IOTLB even if the reset() fails. Thanks, Yongji If this is true, does it mean we don't even need to care about reset failure? But we need to handle the failure in the vhost-vdpa case, isn't it? Yes, but: - This patch is for virtio not for vhost, if we don't care virtio, we can avoid the changes - For vhost, there could be two ways probably: 1) let the set_status to report error 2) require userspace to re-read for status It looks to me you want to go with 1) and I'm not sure whether or not it's too late to go with 2). Looks like 2) can't work if reset failure happens in vhost_vdpa_release() and vhost_vdpa_open(). Yes, you're right. Thanks Thanks, Yongji ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v10 10/17] virtio: Handle device reset failure in register_virtio_device()
On Wed, Aug 4, 2021 at 4:54 PM Jason Wang wrote: > > > 在 2021/8/4 下午4:50, Yongji Xie 写道: > > On Wed, Aug 4, 2021 at 4:32 PM Jason Wang wrote: > >> > >> 在 2021/8/3 下午5:38, Yongji Xie 写道: > >>> On Tue, Aug 3, 2021 at 4:09 PM Jason Wang wrote: > 在 2021/7/29 下午3:34, Xie Yongji 写道: > > The device reset may fail in virtio-vdpa case now, so add checks to > > its return value and fail the register_virtio_device(). > So the reset() would be called by the driver during remove as well, or > is it sufficient to deal only with the reset during probe? > > >>> Actually there is no way to handle failure during removal. And it > >>> should be safe with the protection of software IOTLB even if the > >>> reset() fails. > >>> > >>> Thanks, > >>> Yongji > >> > >> If this is true, does it mean we don't even need to care about reset > >> failure? > >> > > But we need to handle the failure in the vhost-vdpa case, isn't it? > > > Yes, but: > > - This patch is for virtio not for vhost, if we don't care virtio, we > can avoid the changes > - For vhost, there could be two ways probably: > > 1) let the set_status to report error > 2) require userspace to re-read for status > > It looks to me you want to go with 1) and I'm not sure whether or not > it's too late to go with 2). > Looks like 2) can't work if reset failure happens in vhost_vdpa_release() and vhost_vdpa_open(). Thanks, Yongji ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v10 10/17] virtio: Handle device reset failure in register_virtio_device()
在 2021/8/4 下午4:50, Yongji Xie 写道: On Wed, Aug 4, 2021 at 4:32 PM Jason Wang wrote: 在 2021/8/3 下午5:38, Yongji Xie 写道: On Tue, Aug 3, 2021 at 4:09 PM Jason Wang wrote: 在 2021/7/29 下午3:34, Xie Yongji 写道: The device reset may fail in virtio-vdpa case now, so add checks to its return value and fail the register_virtio_device(). So the reset() would be called by the driver during remove as well, or is it sufficient to deal only with the reset during probe? Actually there is no way to handle failure during removal. And it should be safe with the protection of software IOTLB even if the reset() fails. Thanks, Yongji If this is true, does it mean we don't even need to care about reset failure? But we need to handle the failure in the vhost-vdpa case, isn't it? Yes, but: - This patch is for virtio not for vhost, if we don't care virtio, we can avoid the changes - For vhost, there could be two ways probably: 1) let the set_status to report error 2) require userspace to re-read for status It looks to me you want to go with 1) and I'm not sure whether or not it's too late to go with 2). Thanks Thanks, Yongji ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v10 10/17] virtio: Handle device reset failure in register_virtio_device()
On Wed, Aug 4, 2021 at 4:32 PM Jason Wang wrote: > > > 在 2021/8/3 下午5:38, Yongji Xie 写道: > > On Tue, Aug 3, 2021 at 4:09 PM Jason Wang wrote: > >> > >> 在 2021/7/29 下午3:34, Xie Yongji 写道: > >>> The device reset may fail in virtio-vdpa case now, so add checks to > >>> its return value and fail the register_virtio_device(). > >> > >> So the reset() would be called by the driver during remove as well, or > >> is it sufficient to deal only with the reset during probe? > >> > > Actually there is no way to handle failure during removal. And it > > should be safe with the protection of software IOTLB even if the > > reset() fails. > > > > Thanks, > > Yongji > > > If this is true, does it mean we don't even need to care about reset > failure? > But we need to handle the failure in the vhost-vdpa case, isn't it? Thanks, Yongji ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v10 10/17] virtio: Handle device reset failure in register_virtio_device()
在 2021/8/3 下午5:38, Yongji Xie 写道: On Tue, Aug 3, 2021 at 4:09 PM Jason Wang wrote: 在 2021/7/29 下午3:34, Xie Yongji 写道: The device reset may fail in virtio-vdpa case now, so add checks to its return value and fail the register_virtio_device(). So the reset() would be called by the driver during remove as well, or is it sufficient to deal only with the reset during probe? Actually there is no way to handle failure during removal. And it should be safe with the protection of software IOTLB even if the reset() fails. Thanks, Yongji If this is true, does it mean we don't even need to care about reset failure? Thanks ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v10 10/17] virtio: Handle device reset failure in register_virtio_device()
On Tue, Aug 3, 2021 at 4:09 PM Jason Wang wrote: > > > 在 2021/7/29 下午3:34, Xie Yongji 写道: > > The device reset may fail in virtio-vdpa case now, so add checks to > > its return value and fail the register_virtio_device(). > > > So the reset() would be called by the driver during remove as well, or > is it sufficient to deal only with the reset during probe? > Actually there is no way to handle failure during removal. And it should be safe with the protection of software IOTLB even if the reset() fails. Thanks, Yongji ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v10 10/17] virtio: Handle device reset failure in register_virtio_device()
在 2021/7/29 下午3:34, Xie Yongji 写道: The device reset may fail in virtio-vdpa case now, so add checks to its return value and fail the register_virtio_device(). So the reset() would be called by the driver during remove as well, or is it sufficient to deal only with the reset during probe? Thanks Signed-off-by: Xie Yongji --- drivers/virtio/virtio.c | 15 ++- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c index a15beb6b593b..8df75425fb43 100644 --- a/drivers/virtio/virtio.c +++ b/drivers/virtio/virtio.c @@ -349,7 +349,9 @@ int register_virtio_device(struct virtio_device *dev) /* We always start by resetting the device, in case a previous * driver messed it up. This also tests that code path a little. */ - dev->config->reset(dev); + err = dev->config->reset(dev); + if (err) + goto err_reset; /* Acknowledge that we've seen the device. */ virtio_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE); @@ -362,10 +364,13 @@ int register_virtio_device(struct virtio_device *dev) */ err = device_add(>dev); if (err) - ida_simple_remove(_index_ida, dev->index); -out: - if (err) - virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED); + goto err_add; + + return 0; +err_add: + virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED); +err_reset: + ida_simple_remove(_index_ida, dev->index); return err; } EXPORT_SYMBOL_GPL(register_virtio_device); ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
