Re: [dpdk-dev] [PATCH v4] vhost: fix vhost user virtqueue not accessible

[Maxime Coquelin <maxime.coquelin@redhat.com>]

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