diff mbox series

[RFC,1/2] vhost: add packed ring support to vring base requests

Message ID 20180808152323.9660-2-maxime.coquelin@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series vhost: packed ring support completion |

Checks

Context Check Description
ci/Intel-compilation success Compilation OK

Commit Message

Maxime Coquelin Aug. 8, 2018, 3:23 p.m. UTC 
  For consistency with Vhost kernel backend, save the wrap
counter value into bit 31 of num's vring state field.

Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/vhost_user.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

Comments

Jens Freimann Aug. 10, 2018, 7:09 a.m. UTC | #1 
On Wed, Aug 08, 2018 at 05:23:22PM +0200, Maxime Coquelin wrote:
>For consistency with Vhost kernel backend, save the wrap
>counter value into bit 31 of num's vring state field.
>
>Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>---
> lib/librte_vhost/vhost_user.c | 16 ++++++++++++----
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
>diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
>index a2d4c9ffc..31cfdf163 100644
>--- a/lib/librte_vhost/vhost_user.c
>+++ b/lib/librte_vhost/vhost_user.c
>@@ -648,10 +648,15 @@ static int
> vhost_user_set_vring_base(struct virtio_net *dev,
> 			  VhostUserMsg *msg)
> {
>-	dev->virtqueue[msg->payload.state.index]->last_used_idx  =
>-			msg->payload.state.num;
>-	dev->virtqueue[msg->payload.state.index]->last_avail_idx =
>-			msg->payload.state.num;
>+	struct vhost_virtqueue *vq = dev->virtqueue[msg->payload.state.index];
>+	if (dev->features & (1ULL << VIRTIO_F_RING_PACKED)) {
>+		vq->avail_wrap_counter = msg->payload.state.num >> 31;
>+		vq->used_wrap_counter = vq->avail_wrap_counter;

Maybe I don't understand when this is called, but the wrap counters
could have different values, no?

Apart from that:

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

regards,
Jens
Maxime Coquelin Aug. 14, 2018, 12:09 p.m. UTC | #2 
Hi Jens,

On 08/10/2018 09:09 AM, Jens Freimann wrote:
> On Wed, Aug 08, 2018 at 05:23:22PM +0200, Maxime Coquelin wrote:
>> For consistency with Vhost kernel backend, save the wrap
>> counter value into bit 31 of num's vring state field.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>> lib/librte_vhost/vhost_user.c | 16 ++++++++++++----
>> 1 file changed, 12 insertions(+), 4 deletions(-)
>>
>> diff --git a/lib/librte_vhost/vhost_user.c 
>> b/lib/librte_vhost/vhost_user.c
>> index a2d4c9ffc..31cfdf163 100644
>> --- a/lib/librte_vhost/vhost_user.c
>> +++ b/lib/librte_vhost/vhost_user.c
>> @@ -648,10 +648,15 @@ static int
>> vhost_user_set_vring_base(struct virtio_net *dev,
>>               VhostUserMsg *msg)
>> {
>> -    dev->virtqueue[msg->payload.state.index]->last_used_idx  =
>> -            msg->payload.state.num;
>> -    dev->virtqueue[msg->payload.state.index]->last_avail_idx =
>> -            msg->payload.state.num;
>> +    struct vhost_virtqueue *vq = 
>> dev->virtqueue[msg->payload.state.index];
>> +    if (dev->features & (1ULL << VIRTIO_F_RING_PACKED)) {
>> +        vq->avail_wrap_counter = msg->payload.state.num >> 31;
>> +        vq->used_wrap_counter = vq->avail_wrap_counter;
> 
> Maybe I don't understand when this is called, but the wrap counters
> could have different values, no?

No, because we set the used index to the saved avail index value, so
wrap counters must be the same.

> Apart from that:
> 
> Reviewed-by: Jens Freimann <jfreimann@redhat.com>
> regards,
> Jens
diff mbox series

Patch

diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index a2d4c9ffc..31cfdf163 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -648,10 +648,15 @@  static int
 vhost_user_set_vring_base(struct virtio_net *dev,
 			  VhostUserMsg *msg)
 {
-	dev->virtqueue[msg->payload.state.index]->last_used_idx  =
-			msg->payload.state.num;
-	dev->virtqueue[msg->payload.state.index]->last_avail_idx =
-			msg->payload.state.num;
+	struct vhost_virtqueue *vq = dev->virtqueue[msg->payload.state.index];
+	if (dev->features & (1ULL << VIRTIO_F_RING_PACKED)) {
+		vq->avail_wrap_counter = msg->payload.state.num >> 31;
+		vq->used_wrap_counter = vq->avail_wrap_counter;
+		msg->payload.state.num &= ~(1 << 31);
+	}
+
+	vq->last_used_idx = msg->payload.state.num;
+	vq->last_avail_idx = msg->payload.state.num;
 
 	return 0;
 }
@@ -1083,6 +1088,9 @@  vhost_user_get_vring_base(struct virtio_net *dev,
 	/* Here we are safe to get the last avail index */
 	msg->payload.state.num = vq->last_avail_idx;
 
+	if (dev->features & (1ULL << VIRTIO_F_RING_PACKED))
+		msg->payload.state.num |= vq->avail_wrap_counter << 31;
+
 	RTE_LOG(INFO, VHOST_CONFIG,
 		"vring base idx:%d file:%d\n", msg->payload.state.index,
 		msg->payload.state.num);