[v4] vhost: fix vhost user virtqueue not accessible

Message ID 20191104101322.53017-1-yong.liu@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Maxime Coquelin
Headers
Series [v4] vhost: fix vhost user virtqueue not accessible |

Checks

Context Check Description
ci/travis-robot warning Travis build: failed
ci/checkpatch success coding style OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-compilation success Compile Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK

Commit Message

Marvin Liu Nov. 4, 2019, 10:13 a.m. UTC
  Log feature is disabled in vhost user, so that log address was invalid
when checking. Check whether log address is valid can work around it.
Log address should also be translated in packed ring virtqueue.

Fixes: 04cfc7fdbfca ("vhost: translate incoming log address to gpa")

Signed-off-by: Marvin Liu <yong.liu@intel.com>
---
 lib/librte_vhost/vhost_user.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)
  

Comments

Adrian Moreno Nov. 4, 2019, 8:47 a.m. UTC | #1
On 11/4/19 11:13 AM, Marvin Liu wrote:
> Log feature is disabled in vhost user, so that log address was invalid
> when checking. Check whether log address is valid can work around it.
> Log address should also be translated in packed ring virtqueue.
> 
> Fixes: 04cfc7fdbfca ("vhost: translate incoming log address to gpa")
> 
> Signed-off-by: Marvin Liu <yong.liu@intel.com>
> ---
>  lib/librte_vhost/vhost_user.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
> 
> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> index 61ef699ac..09e241305 100644
> --- a/lib/librte_vhost/vhost_user.c
> +++ b/lib/librte_vhost/vhost_user.c
> @@ -641,11 +641,21 @@ translate_ring_addresses(struct virtio_net *dev, int vq_index)
>  	struct vhost_vring_addr *addr = &vq->ring_addrs;
>  	uint64_t len, expected_len;
>  
> +	if (addr->flags & (1 << VHOST_VRING_F_LOG)) {
> +		vq->log_guest_addr =
> +			translate_log_addr(dev, vq, addr->log_guest_addr);
> +		if (vq->log_guest_addr == 0) {
> +			RTE_LOG(DEBUG, VHOST_CONFIG,
> +				"(%d) failed to map log_guest_addr.\n",
> +				dev->vid);
> +			return dev;
> +		}
> +	}
> +
>  	if (vq_is_packed(dev)) {
>  		len = sizeof(struct vring_packed_desc) * vq->size;
>  		vq->desc_packed = (struct vring_packed_desc *)(uintptr_t)
>  			ring_addr_to_vva(dev, vq, addr->desc_user_addr, &len);
> -		vq->log_guest_addr = 0;
>  		if (vq->desc_packed == NULL ||
>  				len != sizeof(struct vring_packed_desc) *
>  				vq->size) {
> @@ -741,14 +751,6 @@ translate_ring_addresses(struct virtio_net *dev, int vq_index)
>  		vq->last_avail_idx = vq->used->idx;
>  	}
>  
> -	vq->log_guest_addr =
> -		translate_log_addr(dev, vq, addr->log_guest_addr);
> -	if (vq->log_guest_addr == 0) {
> -		RTE_LOG(DEBUG, VHOST_CONFIG,
> -			"(%d) failed to map log_guest_addr .\n",
> -			dev->vid);
> -		return dev;
> -	}
>  	vq->access_ok = 1;
>  
>  	VHOST_LOG_DEBUG(VHOST_CONFIG, "(%d) mapped address desc: %p\n",
> 

Reviewed-by: Adrian Moreno <amorenoz@redhat.com>
  
Maxime Coquelin Nov. 6, 2019, 8:16 p.m. UTC | #2
On 11/4/19 9:47 AM, Adrian Moreno wrote:
> On 11/4/19 11:13 AM, Marvin Liu wrote:
>> Log feature is disabled in vhost user, so that log address was invalid
>> when checking. Check whether log address is valid can work around it.
>> Log address should also be translated in packed ring virtqueue.
>>
>> Fixes: 04cfc7fdbfca ("vhost: translate incoming log address to gpa")
>>
>> Signed-off-by: Marvin Liu <yong.liu@intel.com>
>> ---
>>  lib/librte_vhost/vhost_user.c | 20 +++++++++++---------
>>  1 file changed, 11 insertions(+), 9 deletions(-)
>>
>> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
>> index 61ef699ac..09e241305 100644
>> --- a/lib/librte_vhost/vhost_user.c
>> +++ b/lib/librte_vhost/vhost_user.c
>> @@ -641,11 +641,21 @@ translate_ring_addresses(struct virtio_net *dev, int vq_index)
>>  	struct vhost_vring_addr *addr = &vq->ring_addrs;
>>  	uint64_t len, expected_len;
>>  
>> +	if (addr->flags & (1 << VHOST_VRING_F_LOG)) {
>> +		vq->log_guest_addr =
>> +			translate_log_addr(dev, vq, addr->log_guest_addr);
>> +		if (vq->log_guest_addr == 0) {
>> +			RTE_LOG(DEBUG, VHOST_CONFIG,
>> +				"(%d) failed to map log_guest_addr.\n",
>> +				dev->vid);
>> +			return dev;
>> +		}
>> +	}
>> +
>>  	if (vq_is_packed(dev)) {
>>  		len = sizeof(struct vring_packed_desc) * vq->size;
>>  		vq->desc_packed = (struct vring_packed_desc *)(uintptr_t)
>>  			ring_addr_to_vva(dev, vq, addr->desc_user_addr, &len);
>> -		vq->log_guest_addr = 0;
>>  		if (vq->desc_packed == NULL ||
>>  				len != sizeof(struct vring_packed_desc) *
>>  				vq->size) {
>> @@ -741,14 +751,6 @@ translate_ring_addresses(struct virtio_net *dev, int vq_index)
>>  		vq->last_avail_idx = vq->used->idx;
>>  	}
>>  
>> -	vq->log_guest_addr =
>> -		translate_log_addr(dev, vq, addr->log_guest_addr);
>> -	if (vq->log_guest_addr == 0) {
>> -		RTE_LOG(DEBUG, VHOST_CONFIG,
>> -			"(%d) failed to map log_guest_addr .\n",
>> -			dev->vid);
>> -		return dev;
>> -	}
>>  	vq->access_ok = 1;
>>  
>>  	VHOST_LOG_DEBUG(VHOST_CONFIG, "(%d) mapped address desc: %p\n",
>>
> 
> Reviewed-by: Adrian Moreno <amorenoz@redhat.com>
> 

Thanks Marvin for the fix, and Adrian for the review.

It looks good to me too now:
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Maxime
  
Maxime Coquelin Nov. 6, 2019, 8:59 p.m. UTC | #3
On 11/4/19 11:13 AM, Marvin Liu wrote:
> Log feature is disabled in vhost user, so that log address was invalid
> when checking. Check whether log address is valid can work around it.
> Log address should also be translated in packed ring virtqueue.
> 
> Fixes: 04cfc7fdbfca ("vhost: translate incoming log address to gpa")
> 
> Signed-off-by: Marvin Liu <yong.liu@intel.com>
> ---
>  lib/librte_vhost/vhost_user.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)

Applied to dpdk-next-virtio/master.

Thanks,
Maxime
  

Patch

diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 61ef699ac..09e241305 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -641,11 +641,21 @@  translate_ring_addresses(struct virtio_net *dev, int vq_index)
 	struct vhost_vring_addr *addr = &vq->ring_addrs;
 	uint64_t len, expected_len;
 
+	if (addr->flags & (1 << VHOST_VRING_F_LOG)) {
+		vq->log_guest_addr =
+			translate_log_addr(dev, vq, addr->log_guest_addr);
+		if (vq->log_guest_addr == 0) {
+			RTE_LOG(DEBUG, VHOST_CONFIG,
+				"(%d) failed to map log_guest_addr.\n",
+				dev->vid);
+			return dev;
+		}
+	}
+
 	if (vq_is_packed(dev)) {
 		len = sizeof(struct vring_packed_desc) * vq->size;
 		vq->desc_packed = (struct vring_packed_desc *)(uintptr_t)
 			ring_addr_to_vva(dev, vq, addr->desc_user_addr, &len);
-		vq->log_guest_addr = 0;
 		if (vq->desc_packed == NULL ||
 				len != sizeof(struct vring_packed_desc) *
 				vq->size) {
@@ -741,14 +751,6 @@  translate_ring_addresses(struct virtio_net *dev, int vq_index)
 		vq->last_avail_idx = vq->used->idx;
 	}
 
-	vq->log_guest_addr =
-		translate_log_addr(dev, vq, addr->log_guest_addr);
-	if (vq->log_guest_addr == 0) {
-		RTE_LOG(DEBUG, VHOST_CONFIG,
-			"(%d) failed to map log_guest_addr .\n",
-			dev->vid);
-		return dev;
-	}
 	vq->access_ok = 1;
 
 	VHOST_LOG_DEBUG(VHOST_CONFIG, "(%d) mapped address desc: %p\n",