diff mbox

[dpdk-dev,RFC,1/3] vhost: invalidate vring addresses in cleanup_vq()

Message ID 20180222181910.23134-2-maxime.coquelin@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Maxime Coquelin Feb. 22, 2018, 6:19 p.m. UTC 
  When cleaning-up the virtqueue, we also need to invalidate its
addresses to be sure outdated addresses won't be used later.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost.c      | 6 ++++--
 lib/librte_vhost/vhost.h      | 4 +++-
 lib/librte_vhost/vhost_user.c | 2 +-
 3 files changed, 8 insertions(+), 4 deletions(-)

Comments

Jens Freimann Feb. 27, 2018, 11:22 a.m. UTC | #1 
On Thu, Feb 22, 2018 at 07:19:08PM +0100, Maxime Coquelin wrote:
>When cleaning-up the virtqueue, we also need to invalidate its
>addresses to be sure outdated addresses won't be used later.
>
>Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>---
> lib/librte_vhost/vhost.c      | 6 ++++--
> lib/librte_vhost/vhost.h      | 4 +++-
> lib/librte_vhost/vhost_user.c | 2 +-
> 3 files changed, 8 insertions(+), 4 deletions(-)
>
>diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
>index f6f12a03b..e4281cf67 100644
>--- a/lib/librte_vhost/vhost.c
>+++ b/lib/librte_vhost/vhost.c
>@@ -69,12 +69,14 @@ __vhost_iova_to_vva(struct virtio_net *dev, struct vhost_virtqueue *vq,
> }
>
> void
>-cleanup_vq(struct vhost_virtqueue *vq, int destroy)
>+cleanup_vq(struct virtio_net *dev, struct vhost_virtqueue *vq, int destroy)
> {
> 	if ((vq->callfd >= 0) && (destroy != 0))
> 		close(vq->callfd);
> 	if (vq->kickfd >= 0)
> 		close(vq->kickfd);
>+
>+	vring_invalidate(dev, vq);
> }
>
> /*
>@@ -89,7 +91,7 @@ cleanup_device(struct virtio_net *dev, int destroy)
> 	vhost_backend_cleanup(dev);
>
> 	for (i = 0; i < dev->nr_vring; i++)
>-		cleanup_vq(dev->virtqueue[i], destroy);
>+		cleanup_vq(dev, dev->virtqueue[i], destroy);
> }
>
> void
>diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
>index 58aec2e0d..4ebf84bec 100644
>--- a/lib/librte_vhost/vhost.h
>+++ b/lib/librte_vhost/vhost.h
>@@ -241,6 +241,7 @@ struct virtio_net {
> 	struct guest_page       *guest_pages;
>
> 	int			slave_req_fd;
>+	uint8_t		virtio_status;

Belongs into other patch?

Apart from that 
Reviewed-by: Jens Freimann <jfreimann@redhat.com> 

regards,
Jens
Maxime Coquelin Feb. 27, 2018, 11:44 a.m. UTC | #2 
Hi Jens,

On 02/27/2018 12:22 PM, Jens Freimann wrote:
> On Thu, Feb 22, 2018 at 07:19:08PM +0100, Maxime Coquelin wrote:
>> When cleaning-up the virtqueue, we also need to invalidate its
>> addresses to be sure outdated addresses won't be used later.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>> lib/librte_vhost/vhost.c      | 6 ++++--
>> lib/librte_vhost/vhost.h      | 4 +++-
>> lib/librte_vhost/vhost_user.c | 2 +-
>> 3 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
>> index f6f12a03b..e4281cf67 100644
>> --- a/lib/librte_vhost/vhost.c
>> +++ b/lib/librte_vhost/vhost.c
>> @@ -69,12 +69,14 @@ __vhost_iova_to_vva(struct virtio_net *dev, struct 
>> vhost_virtqueue *vq,
>> }
>>
>> void
>> -cleanup_vq(struct vhost_virtqueue *vq, int destroy)
>> +cleanup_vq(struct virtio_net *dev, struct vhost_virtqueue *vq, int 
>> destroy)
>> {
>>     if ((vq->callfd >= 0) && (destroy != 0))
>>         close(vq->callfd);
>>     if (vq->kickfd >= 0)
>>         close(vq->kickfd);
>> +
>> +    vring_invalidate(dev, vq);
>> }
>>
>> /*
>> @@ -89,7 +91,7 @@ cleanup_device(struct virtio_net *dev, int destroy)
>>     vhost_backend_cleanup(dev);
>>
>>     for (i = 0; i < dev->nr_vring; i++)
>> -        cleanup_vq(dev->virtqueue[i], destroy);
>> +        cleanup_vq(dev, dev->virtqueue[i], destroy);
>> }
>>
>> void
>> diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
>> index 58aec2e0d..4ebf84bec 100644
>> --- a/lib/librte_vhost/vhost.h
>> +++ b/lib/librte_vhost/vhost.h
>> @@ -241,6 +241,7 @@ struct virtio_net {
>>     struct guest_page       *guest_pages;
>>
>>     int            slave_req_fd;
>> +    uint8_t        virtio_status;
> 
> Belongs into other patch?

Oh, right! I squashed in wrong commit.

> Apart from that Reviewed-by: Jens Freimann <jfreimann@redhat.com>

Is your r-b for the full series or this single patch?

Thanks!
Maxime
> regards,
> Jens
Jens Freimann Feb. 27, 2018, 11:54 a.m. UTC | #3 
On Tue, Feb 27, 2018 at 12:44:08PM +0100, Maxime Coquelin wrote:
>Hi Jens,
>
>On 02/27/2018 12:22 PM, Jens Freimann wrote:
>>On Thu, Feb 22, 2018 at 07:19:08PM +0100, Maxime Coquelin wrote:
[...]
>>>????????int?????????????????????? slave_req_fd;
>>>+?????? uint8_t?????????????? virtio_status;
>>
>>Belongs into other patch?
>
>Oh, right! I squashed in wrong commit.
>
>>Apart from that Reviewed-by: Jens Freimann <jfreimann@redhat.com>
>
>Is your r-b for the full series or this single patch?

For this one, but I'll review the other patches today as well. 

regards,
Jens 

>Thanks!
>Maxime
>>regards,
>>Jens
diff mbox

Patch

diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
index f6f12a03b..e4281cf67 100644
--- a/lib/librte_vhost/vhost.c
+++ b/lib/librte_vhost/vhost.c
@@ -69,12 +69,14 @@  __vhost_iova_to_vva(struct virtio_net *dev, struct vhost_virtqueue *vq,
 }
 
 void
-cleanup_vq(struct vhost_virtqueue *vq, int destroy)
+cleanup_vq(struct virtio_net *dev, struct vhost_virtqueue *vq, int destroy)
 {
 	if ((vq->callfd >= 0) && (destroy != 0))
 		close(vq->callfd);
 	if (vq->kickfd >= 0)
 		close(vq->kickfd);
+
+	vring_invalidate(dev, vq);
 }
 
 /*
@@ -89,7 +91,7 @@  cleanup_device(struct virtio_net *dev, int destroy)
 	vhost_backend_cleanup(dev);
 
 	for (i = 0; i < dev->nr_vring; i++)
-		cleanup_vq(dev->virtqueue[i], destroy);
+		cleanup_vq(dev, dev->virtqueue[i], destroy);
 }
 
 void
diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
index 58aec2e0d..4ebf84bec 100644
--- a/lib/librte_vhost/vhost.h
+++ b/lib/librte_vhost/vhost.h
@@ -241,6 +241,7 @@  struct virtio_net {
 	struct guest_page       *guest_pages;
 
 	int			slave_req_fd;
+	uint8_t		virtio_status;
 } __rte_cache_aligned;
 
 
@@ -362,7 +363,8 @@  void cleanup_device(struct virtio_net *dev, int destroy);
 void reset_device(struct virtio_net *dev);
 void vhost_destroy_device(int);
 
-void cleanup_vq(struct vhost_virtqueue *vq, int destroy);
+void cleanup_vq(struct virtio_net *dev, struct vhost_virtqueue *vq,
+				int destroy);
 void free_vq(struct vhost_virtqueue *vq);
 
 int alloc_vring_queue(struct virtio_net *dev, uint32_t vring_idx);
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 5c5361066..c256ebb06 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -219,7 +219,7 @@  vhost_user_set_features(struct virtio_net *dev, uint64_t features)
 				continue;
 
 			dev->virtqueue[dev->nr_vring] = NULL;
-			cleanup_vq(vq, 1);
+			cleanup_vq(dev, vq, 1);
 			free_vq(vq);
 		}
 	}