When virtio vdpa device removed, the abnormal damage of the device cannot be
perceived normally, which will cause problems similar to:
Commit 43bb40c5b926 ("virtio_pci: Support surprise removal of
virtio pci device")
Hence, add the ability to abort the command on surprise removal
Signed-off-by: dongsheng li <[email protected]>
---
drivers/virtio/virtio_vdpa.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
index 4a9ddb44b2a7..fd930409d190 100644
--- a/drivers/virtio/virtio_vdpa.c
+++ b/drivers/virtio/virtio_vdpa.c
@@ -374,6 +374,7 @@ static void virtio_vdpa_remove(struct vdpa_device *vdpa)
{
struct virtio_vdpa_device *vd_dev = vdpa_get_drvdata(vdpa);
+ virtio_break_device(vd_dev->vdev);
unregister_virtio_device(&vd_dev->vdev);
}
--
2.17.1
On Tue, Jan 11, 2022 at 11:38 AM 李东升 <[email protected]> wrote:
>
> When virtio vdpa device removed, the abnormal damage of the device cannot be
> perceived normally, which will cause problems similar to:
>
> 43bb40c5b926
>
> Hence, add the ability to abort the command on surprise removal
>
> Signed-off-by: dongsheng li <[email protected]>
Acked-by: Jason Wang <[email protected]>
> ---
> drivers/virtio/virtio_vdpa.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
> index 4a9ddb44b2a7..fd930409d190 100644
> --- a/drivers/virtio/virtio_vdpa.c
> +++ b/drivers/virtio/virtio_vdpa.c
> @@ -374,6 +374,7 @@ static void virtio_vdpa_remove(struct vdpa_device *vdpa)
> {
> struct virtio_vdpa_device *vd_dev = vdpa_get_drvdata(vdpa);
>
> + virtio_break_device(vd_dev->vdev);
> unregister_virtio_device(&vd_dev->vdev);
> }
>
> --
> 2.17.1
On Tue, Jan 11, 2022 at 11:36:42AM +0800, 李东升 wrote:
> When virtio vdpa device removed, the abnormal damage of the device cannot be
> perceived normally, which will cause problems similar to:
>
> 43bb40c5b926
Should include the subject of the patch too.
> Hence, add the ability to abort the command on surprise removal
>
> Signed-off-by: dongsheng li <[email protected]>
When removing gracefully,
I am not sure we should break device unconditionally like this
before giving drivers a chance to clean up.
Should we just do it for surprise removal?
> ---
> drivers/virtio/virtio_vdpa.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
> index 4a9ddb44b2a7..fd930409d190 100644
> --- a/drivers/virtio/virtio_vdpa.c
> +++ b/drivers/virtio/virtio_vdpa.c
> @@ -374,6 +374,7 @@ static void virtio_vdpa_remove(struct vdpa_device *vdpa)
> {
> struct virtio_vdpa_device *vd_dev = vdpa_get_drvdata(vdpa);
>
> + virtio_break_device(vd_dev->vdev);
> unregister_virtio_device(&vd_dev->vdev);
> }
>
> --
> 2.17.1
On Tue, Jan 11, 2022 at 7:52 PM Michael S. Tsirkin <[email protected]> wrote:
>
> On Tue, Jan 11, 2022 at 11:36:42AM +0800, 李东升 wrote:
> > When virtio vdpa device removed, the abnormal damage of the device cannot be
> > perceived normally, which will cause problems similar to:
> >
> > 43bb40c5b926
>
>
> Should include the subject of the patch too.
>
> > Hence, add the ability to abort the command on surprise removal
> >
> > Signed-off-by: dongsheng li <[email protected]>
>
> When removing gracefully,
> I am not sure we should break device unconditionally like this
> before giving drivers a chance to clean up.
> Should we just do it for surprise removal?
That requires a new method to query whether it's a surprise removal.
Thanks
>
> > ---
> > drivers/virtio/virtio_vdpa.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
> > index 4a9ddb44b2a7..fd930409d190 100644
> > --- a/drivers/virtio/virtio_vdpa.c
> > +++ b/drivers/virtio/virtio_vdpa.c
> > @@ -374,6 +374,7 @@ static void virtio_vdpa_remove(struct vdpa_device *vdpa)
> > {
> > struct virtio_vdpa_device *vd_dev = vdpa_get_drvdata(vdpa);
> >
> > + virtio_break_device(vd_dev->vdev);
> > unregister_virtio_device(&vd_dev->vdev);
> > }
> >
> > --
> > 2.17.1
>
On Wed, Jan 12, 2022 at 10:23:07AM +0800, Jason Wang wrote:
> On Tue, Jan 11, 2022 at 7:52 PM Michael S. Tsirkin <[email protected]> wrote:
> >
> > On Tue, Jan 11, 2022 at 11:36:42AM +0800, 李东升 wrote:
> > > When virtio vdpa device removed, the abnormal damage of the device cannot be
> > > perceived normally, which will cause problems similar to:
> > >
> > > 43bb40c5b926
> >
> >
> > Should include the subject of the patch too.
> >
> > > Hence, add the ability to abort the command on surprise removal
> > >
> > > Signed-off-by: dongsheng li <[email protected]>
> >
> > When removing gracefully,
> > I am not sure we should break device unconditionally like this
> > before giving drivers a chance to clean up.
> > Should we just do it for surprise removal?
>
> That requires a new method to query whether it's a surprise removal.
>
> Thanks
We can check pci_device_is_present like virtio does.
> >
> > > ---
> > > drivers/virtio/virtio_vdpa.c | 1 +
> > > 1 file changed, 1 insertion(+)
> > >
> > > diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
> > > index 4a9ddb44b2a7..fd930409d190 100644
> > > --- a/drivers/virtio/virtio_vdpa.c
> > > +++ b/drivers/virtio/virtio_vdpa.c
> > > @@ -374,6 +374,7 @@ static void virtio_vdpa_remove(struct vdpa_device *vdpa)
> > > {
> > > struct virtio_vdpa_device *vd_dev = vdpa_get_drvdata(vdpa);
> > >
> > > + virtio_break_device(vd_dev->vdev);
> > > unregister_virtio_device(&vd_dev->vdev);
> > > }
> > >
> > > --
> > > 2.17.1
> >
On Wed, Jan 12, 2022 at 2:30 PM Michael S. Tsirkin <[email protected]> wrote:
>
> On Wed, Jan 12, 2022 at 10:23:07AM +0800, Jason Wang wrote:
> > On Tue, Jan 11, 2022 at 7:52 PM Michael S. Tsirkin <[email protected]> wrote:
> > >
> > > On Tue, Jan 11, 2022 at 11:36:42AM +0800, 李东升 wrote:
> > > > When virtio vdpa device removed, the abnormal damage of the device cannot be
> > > > perceived normally, which will cause problems similar to:
> > > >
> > > > 43bb40c5b926
> > >
> > >
> > > Should include the subject of the patch too.
> > >
> > > > Hence, add the ability to abort the command on surprise removal
> > > >
> > > > Signed-off-by: dongsheng li <[email protected]>
> > >
> > > When removing gracefully,
> > > I am not sure we should break device unconditionally like this
> > > before giving drivers a chance to clean up.
> > > Should we just do it for surprise removal?
> >
> > That requires a new method to query whether it's a surprise removal.
> >
> > Thanks
>
> We can check pci_device_is_present like virtio does.
Though the PCI device is the main user, we are vdpa transport that is
unaware of the type of its parent.
So it looks to me we need a new method and PCI parent and do call this
function when needed.
Thanks
>
> > >
> > > > ---
> > > > drivers/virtio/virtio_vdpa.c | 1 +
> > > > 1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
> > > > index 4a9ddb44b2a7..fd930409d190 100644
> > > > --- a/drivers/virtio/virtio_vdpa.c
> > > > +++ b/drivers/virtio/virtio_vdpa.c
> > > > @@ -374,6 +374,7 @@ static void virtio_vdpa_remove(struct vdpa_device *vdpa)
> > > > {
> > > > struct virtio_vdpa_device *vd_dev = vdpa_get_drvdata(vdpa);
> > > >
> > > > + virtio_break_device(vd_dev->vdev);
> > > > unregister_virtio_device(&vd_dev->vdev);
> > > > }
> > > >
> > > > --
> > > > 2.17.1
> > >
>
On Wed, Jan 12, 2022 at 03:52:52PM +0800, Jason Wang wrote:
> On Wed, Jan 12, 2022 at 2:30 PM Michael S. Tsirkin <[email protected]> wrote:
> >
> > On Wed, Jan 12, 2022 at 10:23:07AM +0800, Jason Wang wrote:
> > > On Tue, Jan 11, 2022 at 7:52 PM Michael S. Tsirkin <[email protected]> wrote:
> > > >
> > > > On Tue, Jan 11, 2022 at 11:36:42AM +0800, 李东升 wrote:
> > > > > When virtio vdpa device removed, the abnormal damage of the device cannot be
> > > > > perceived normally, which will cause problems similar to:
> > > > >
> > > > > 43bb40c5b926
> > > >
> > > >
> > > > Should include the subject of the patch too.
> > > >
> > > > > Hence, add the ability to abort the command on surprise removal
> > > > >
> > > > > Signed-off-by: dongsheng li <[email protected]>
> > > >
> > > > When removing gracefully,
> > > > I am not sure we should break device unconditionally like this
> > > > before giving drivers a chance to clean up.
> > > > Should we just do it for surprise removal?
> > >
> > > That requires a new method to query whether it's a surprise removal.
> > >
> > > Thanks
> >
> > We can check pci_device_is_present like virtio does.
>
> Though the PCI device is the main user, we are vdpa transport that is
> unaware of the type of its parent.
>
> So it looks to me we need a new method and PCI parent and do call this
> function when needed.
>
> Thanks
Sure.
> >
> > > >
> > > > > ---
> > > > > drivers/virtio/virtio_vdpa.c | 1 +
> > > > > 1 file changed, 1 insertion(+)
> > > > >
> > > > > diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
> > > > > index 4a9ddb44b2a7..fd930409d190 100644
> > > > > --- a/drivers/virtio/virtio_vdpa.c
> > > > > +++ b/drivers/virtio/virtio_vdpa.c
> > > > > @@ -374,6 +374,7 @@ static void virtio_vdpa_remove(struct vdpa_device *vdpa)
> > > > > {
> > > > > struct virtio_vdpa_device *vd_dev = vdpa_get_drvdata(vdpa);
> > > > >
> > > > > + virtio_break_device(vd_dev->vdev);
> > > > > unregister_virtio_device(&vd_dev->vdev);
> > > > > }
> > > > >
> > > > > --
> > > > > 2.17.1
> > > >
> >
On Wed, Jan 12, 2022 at 04:44:08PM +0800, 李东升 wrote:
> Indeed, there are hidden dangers for storage devices.
> So maybe we can break the device before virtio_net unregister_netdev and before
> virtio_blk reset
>
> Unlike pci device, there is a manufacturer-driven existence between virtio vdpa
> and hardware devices,
> unless the manufacturer provides a state interface, virtio vdpa and
> higher-level drivers cannot obtain the real hardware state.
I think for surprise removal you need to propagate the presence status up.
No way around that imho.
>
> ------------------ Original ------------------
> From: "mst"<[email protected]>;
> Date: Wed, Jan 12, 2022 02:30 PM
> To: "jasowang"<[email protected]>;
> Cc: "李东升"<[email protected]>; "virtualization"
> <[email protected]>; "linux-kernel"
> <[email protected]>;
> Subject: Re: [PATCH] virtio_vdpa: Support surprise removal of virtio vdpa
> device
>
> On Wed, Jan 12, 2022 at 10:23:07AM +0800, Jason Wang wrote:
> > On Tue, Jan 11, 2022 at 7:52 PM Michael S. Tsirkin <[email protected]> wrote:
> > >
> > > On Tue, Jan 11, 2022 at 11:36:42AM +0800, 李东升 wrote:
> > > > When virtio vdpa device removed, the abnormal damage of the device cannot
> be
> > > > perceived normally, which will cause problems similar to:
> > > >
> > > > 43bb40c5b926
> > >
> > >
> > > Should include the subject of the patch too.
> > >
> > > > Hence, add the ability to abort the command on surprise removal
> > > >
> > > > Signed-off-by: dongsheng li <[email protected]>
> > >
> > > When removing gracefully,
> > > I am not sure we should break device unconditionally like this
> > > before giving drivers a chance to clean up.
> > > Should we just do it for surprise removal?
> >
> > That requires a new method to query whether it's a surprise removal.
> >
> > Thanks
>
> We can check pci_device_is_present like virtio does.
>
> > >
> > > > ---
> > > > drivers/virtio/virtio_vdpa.c | 1 +
> > > > 1 file changed, 1 insertion(+)
> > > >
> > > > diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
> > > > index 4a9ddb44b2a7..fd930409d190 100644
> > > > --- a/drivers/virtio/virtio_vdpa.c
> > > > +++ b/drivers/virtio/virtio_vdpa.c
> > > > @@ -374,6 +374,7 @@ static void virtio_vdpa_remove(struct vdpa_device
> *vdpa)
> > > > {
> > > > struct virtio_vdpa_device *vd_dev = vdpa_get_drvdata(vdpa);
> > > >
> > > > + virtio_break_device(vd_dev->vdev);
> > > > unregister_virtio_device(&vd_dev->vdev);
> > > > }
> > > >
> > > > --
> > > > 2.17.1
> > >
>