DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] vhost: translate incoming log address to gpa
@ 2019-09-17 14:49 Adrian Moreno
  2019-09-23  7:25 ` Tiwei Bie
  0 siblings, 1 reply; 4+ messages in thread
From: Adrian Moreno @ 2019-09-17 14:49 UTC (permalink / raw)
  To: dev; +Cc: tiwei.bie, zhihong.wang, Adrian Moreno, Maxime Coquelin, Pei Zhang

When IOMMU is enabled the incoming log address is in IOVA space. In that
case, look in IOTLB table and translate the resulting HVA to GPA.

If IOMMU is not enabled, the incoming log address is already a GPA so no
transformation is needed.

This change makes page logging work when IOVA_VA is selected in the guest.
Further information: https://bugs.dpdk.org/show_bug.cgi?id=337

Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
Reported-by: Pei Zhang <pezhang@redhat.com>

Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
---
 lib/librte_vhost/vhost.c      |  1 +
 lib/librte_vhost/vhost_user.c | 78 ++++++++++++++++++++++++++++++++++-
 2 files changed, 78 insertions(+), 1 deletion(-)

diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
index 981837b5d..e57dda22f 100644
--- a/lib/librte_vhost/vhost.c
+++ b/lib/librte_vhost/vhost.c
@@ -383,6 +383,7 @@ vring_invalidate(struct virtio_net *dev, struct vhost_virtqueue *vq)
 	vq->desc = NULL;
 	vq->avail = NULL;
 	vq->used = NULL;
+	vq->log_guest_addr = 0;
 
 	if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM))
 		vhost_user_iotlb_wr_unlock(vq);
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index 0b72648a5..35fca00fe 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -570,6 +570,74 @@ ring_addr_to_vva(struct virtio_net *dev, struct vhost_virtqueue *vq,
 	return qva_to_vva(dev, ra, size);
 }
 
+/*
+ * Converts Vhost Virtual Address to Guest Physical Address
+ */
+static uint64_t
+vva_to_gpa(struct virtio_net *dev, uint64_t vva, uint64_t *len)
+{
+	struct rte_vhost_mem_region *r;
+	uint32_t i;
+
+	if (unlikely(!dev || !dev->mem))
+		goto out_error;
+
+	/* Find the region where the address lives. */
+	for (i = 0; i < dev->mem->nregions; i++) {
+		r = &dev->mem->regions[i];
+
+		if (vva >= r->host_user_addr &&
+		    vva <  r->host_user_addr + r->size) {
+
+			if (unlikely(vva + *len > r->host_user_addr + r->size))
+				*len = r->guest_user_addr + r->size - vva;
+
+			return r->guest_phys_addr + vva - r->host_user_addr;
+		}
+	}
+out_error:
+	*len = 0;
+
+	return 0;
+}
+
+/*
+ * Converts vring log address to GPA
+ * If IOMMU is enabled, the log address is IOVA
+ * If IOMMU not enabled, the log address is already GPA
+ */
+static uint64_t
+translate_log_addr(struct virtio_net *dev, struct vhost_virtqueue *vq,
+		uint64_t log_addr)
+{
+	if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) {
+		const uint64_t exp_size = sizeof(struct vring_used) +
+			sizeof(struct vring_used_elem) * vq->size;
+		uint64_t vva, gpa;
+		uint64_t size = exp_size;
+
+		vva = vhost_user_iotlb_cache_find(vq, log_addr,
+					&size, VHOST_ACCESS_RW);
+		if (size != exp_size) {
+			vhost_user_iotlb_miss(dev, log_addr + size,
+					      VHOST_ACCESS_RW);
+			return 0;
+		}
+
+		gpa = vva_to_gpa(dev, vva, &size);
+		if (size != exp_size) {
+			RTE_LOG(ERR, VHOST_CONFIG,
+				"VQ: Failed to find GPA mapping for log_addr."
+				"log_addr: 0x%0lx vva: 0x%0lx\n",
+				log_addr, vva);
+			return 0;
+		}
+		return gpa;
+
+	} else
+		return log_addr;
+}
+
 static struct virtio_net *
 translate_ring_addresses(struct virtio_net *dev, int vq_index)
 {
@@ -676,7 +744,15 @@ translate_ring_addresses(struct virtio_net *dev, int vq_index)
 		vq->last_avail_idx = vq->used->idx;
 	}
 
-	vq->log_guest_addr = addr->log_guest_addr;
+	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;
+	}
+
 
 	VHOST_LOG_DEBUG(VHOST_CONFIG, "(%d) mapped address desc: %p\n",
 			dev->vid, vq->desc);
-- 
2.21.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] vhost: translate incoming log address to gpa
  2019-09-17 14:49 [dpdk-dev] [PATCH] vhost: translate incoming log address to gpa Adrian Moreno
@ 2019-09-23  7:25 ` Tiwei Bie
  2019-09-23 10:28   ` Adrian Moreno
  0 siblings, 1 reply; 4+ messages in thread
From: Tiwei Bie @ 2019-09-23  7:25 UTC (permalink / raw)
  To: Adrian Moreno; +Cc: dev, zhihong.wang, Maxime Coquelin, Pei Zhang

On Tue, Sep 17, 2019 at 04:49:00PM +0200, Adrian Moreno wrote:
> When IOMMU is enabled the incoming log address is in IOVA space. In that
> case, look in IOTLB table and translate the resulting HVA to GPA.
> 
> If IOMMU is not enabled, the incoming log address is already a GPA so no
> transformation is needed.
> 
> This change makes page logging work when IOVA_VA is selected in the guest.

Besides the log address of the ring, when IOMMU is enabled,
the addresses in descriptors are also IOVAs and should be
translated to GPAs before doing the dirty page logging.

> Further information: https://bugs.dpdk.org/show_bug.cgi?id=337

As this is fixing a real bug, we also need a "Fixes: " line and Cc stable.

Thanks!
Tiwei

> 
> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
> Reported-by: Pei Zhang <pezhang@redhat.com>
> 
> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
> ---
>  lib/librte_vhost/vhost.c      |  1 +
>  lib/librte_vhost/vhost_user.c | 78 ++++++++++++++++++++++++++++++++++-
>  2 files changed, 78 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
> index 981837b5d..e57dda22f 100644
> --- a/lib/librte_vhost/vhost.c
> +++ b/lib/librte_vhost/vhost.c
> @@ -383,6 +383,7 @@ vring_invalidate(struct virtio_net *dev, struct vhost_virtqueue *vq)
>  	vq->desc = NULL;
>  	vq->avail = NULL;
>  	vq->used = NULL;
> +	vq->log_guest_addr = 0;
>  
>  	if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM))
>  		vhost_user_iotlb_wr_unlock(vq);
> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
> index 0b72648a5..35fca00fe 100644
> --- a/lib/librte_vhost/vhost_user.c
> +++ b/lib/librte_vhost/vhost_user.c
> @@ -570,6 +570,74 @@ ring_addr_to_vva(struct virtio_net *dev, struct vhost_virtqueue *vq,
>  	return qva_to_vva(dev, ra, size);
>  }
>  
> +/*
> + * Converts Vhost Virtual Address to Guest Physical Address
> + */
> +static uint64_t
> +vva_to_gpa(struct virtio_net *dev, uint64_t vva, uint64_t *len)
> +{
> +	struct rte_vhost_mem_region *r;
> +	uint32_t i;
> +
> +	if (unlikely(!dev || !dev->mem))
> +		goto out_error;
> +
> +	/* Find the region where the address lives. */
> +	for (i = 0; i < dev->mem->nregions; i++) {
> +		r = &dev->mem->regions[i];
> +
> +		if (vva >= r->host_user_addr &&
> +		    vva <  r->host_user_addr + r->size) {
> +
> +			if (unlikely(vva + *len > r->host_user_addr + r->size))
> +				*len = r->guest_user_addr + r->size - vva;
> +
> +			return r->guest_phys_addr + vva - r->host_user_addr;
> +		}
> +	}
> +out_error:
> +	*len = 0;
> +
> +	return 0;
> +}
> +
> +/*
> + * Converts vring log address to GPA
> + * If IOMMU is enabled, the log address is IOVA
> + * If IOMMU not enabled, the log address is already GPA
> + */
> +static uint64_t
> +translate_log_addr(struct virtio_net *dev, struct vhost_virtqueue *vq,
> +		uint64_t log_addr)
> +{
> +	if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) {
> +		const uint64_t exp_size = sizeof(struct vring_used) +
> +			sizeof(struct vring_used_elem) * vq->size;
> +		uint64_t vva, gpa;
> +		uint64_t size = exp_size;
> +
> +		vva = vhost_user_iotlb_cache_find(vq, log_addr,
> +					&size, VHOST_ACCESS_RW);
> +		if (size != exp_size) {
> +			vhost_user_iotlb_miss(dev, log_addr + size,
> +					      VHOST_ACCESS_RW);
> +			return 0;
> +		}
> +
> +		gpa = vva_to_gpa(dev, vva, &size);
> +		if (size != exp_size) {
> +			RTE_LOG(ERR, VHOST_CONFIG,
> +				"VQ: Failed to find GPA mapping for log_addr."
> +				"log_addr: 0x%0lx vva: 0x%0lx\n",
> +				log_addr, vva);
> +			return 0;
> +		}
> +		return gpa;
> +
> +	} else
> +		return log_addr;
> +}
> +
>  static struct virtio_net *
>  translate_ring_addresses(struct virtio_net *dev, int vq_index)
>  {
> @@ -676,7 +744,15 @@ translate_ring_addresses(struct virtio_net *dev, int vq_index)
>  		vq->last_avail_idx = vq->used->idx;
>  	}
>  
> -	vq->log_guest_addr = addr->log_guest_addr;
> +	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;
> +	}
> +
>  
>  	VHOST_LOG_DEBUG(VHOST_CONFIG, "(%d) mapped address desc: %p\n",
>  			dev->vid, vq->desc);
> -- 
> 2.21.0
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] vhost: translate incoming log address to gpa
  2019-09-23  7:25 ` Tiwei Bie
@ 2019-09-23 10:28   ` Adrian Moreno
  2019-09-24  5:23     ` Tiwei Bie
  0 siblings, 1 reply; 4+ messages in thread
From: Adrian Moreno @ 2019-09-23 10:28 UTC (permalink / raw)
  To: Tiwei Bie; +Cc: dev, zhihong.wang, Maxime Coquelin, Pei Zhang

On 9/23/19 9:25 AM, Tiwei Bie wrote:
> On Tue, Sep 17, 2019 at 04:49:00PM +0200, Adrian Moreno wrote:
>> When IOMMU is enabled the incoming log address is in IOVA space. In that
>> case, look in IOTLB table and translate the resulting HVA to GPA.
>>
>> If IOMMU is not enabled, the incoming log address is already a GPA so no
>> transformation is needed.
>>
>> This change makes page logging work when IOVA_VA is selected in the guest.
> 
> Besides the log address of the ring, when IOMMU is enabled,
> the addresses in descriptors are also IOVAs and should be
> translated to GPAs before doing the dirty page logging.
> 
Thanks Tiwei. You're right. In fact, it's not the only place where IOVAs are
assumed to be GPAs, for example in vhost.h:gpa_to_hpa:

/* Convert guest physical address to host physical address */
static __rte_always_inline rte_iova_t
gpa_to_hpa(struct virtio_net *dev, uint64_t gpa, uint64_t size)
{
	uint32_t i;
	struct guest_page *page;

	for (i = 0; i < dev->nr_guest_pages; i++) {
		page = &dev->guest_pages[i];

		if (gpa >= page->guest_phys_addr &&
		    gpa + size < page->guest_phys_addr + page->size) {
			return gpa - page->guest_phys_addr +
			       page->host_phys_addr;
		}
	}

	return 0;
}

used in ZERO-COPY mode to check if the IOVA range is continuous in host physical
memory and called buffer IOVAs. If I'm not mistaken this should also be
translated as:

IOVA --> HVA (iotlb lookup)
HVA --> GPA (mem_region lookup)
GPA --> HIOVA (guest_page lookup)

Right?

Thanks.
Adrián

>> Further information: https://bugs.dpdk.org/show_bug.cgi?id=337
> 
> As this is fixing a real bug, we also need a "Fixes: " line and Cc stable.
> 
> Thanks!
> Tiwei
> 
>>
>> Cc: Maxime Coquelin <maxime.coquelin@redhat.com>
>> Reported-by: Pei Zhang <pezhang@redhat.com>
>>
>> Signed-off-by: Adrian Moreno <amorenoz@redhat.com>
>> ---
>>  lib/librte_vhost/vhost.c      |  1 +
>>  lib/librte_vhost/vhost_user.c | 78 ++++++++++++++++++++++++++++++++++-
>>  2 files changed, 78 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/librte_vhost/vhost.c b/lib/librte_vhost/vhost.c
>> index 981837b5d..e57dda22f 100644
>> --- a/lib/librte_vhost/vhost.c
>> +++ b/lib/librte_vhost/vhost.c
>> @@ -383,6 +383,7 @@ vring_invalidate(struct virtio_net *dev, struct vhost_virtqueue *vq)
>>  	vq->desc = NULL;
>>  	vq->avail = NULL;
>>  	vq->used = NULL;
>> +	vq->log_guest_addr = 0;
>>  
>>  	if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM))
>>  		vhost_user_iotlb_wr_unlock(vq);
>> diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
>> index 0b72648a5..35fca00fe 100644
>> --- a/lib/librte_vhost/vhost_user.c
>> +++ b/lib/librte_vhost/vhost_user.c
>> @@ -570,6 +570,74 @@ ring_addr_to_vva(struct virtio_net *dev, struct vhost_virtqueue *vq,
>>  	return qva_to_vva(dev, ra, size);
>>  }
>>  
>> +/*
>> + * Converts Vhost Virtual Address to Guest Physical Address
>> + */
>> +static uint64_t
>> +vva_to_gpa(struct virtio_net *dev, uint64_t vva, uint64_t *len)
>> +{
>> +	struct rte_vhost_mem_region *r;
>> +	uint32_t i;
>> +
>> +	if (unlikely(!dev || !dev->mem))
>> +		goto out_error;
>> +
>> +	/* Find the region where the address lives. */
>> +	for (i = 0; i < dev->mem->nregions; i++) {
>> +		r = &dev->mem->regions[i];
>> +
>> +		if (vva >= r->host_user_addr &&
>> +		    vva <  r->host_user_addr + r->size) {
>> +
>> +			if (unlikely(vva + *len > r->host_user_addr + r->size))
>> +				*len = r->guest_user_addr + r->size - vva;
>> +
>> +			return r->guest_phys_addr + vva - r->host_user_addr;
>> +		}
>> +	}
>> +out_error:
>> +	*len = 0;
>> +
>> +	return 0;
>> +}
>> +
>> +/*
>> + * Converts vring log address to GPA
>> + * If IOMMU is enabled, the log address is IOVA
>> + * If IOMMU not enabled, the log address is already GPA
>> + */
>> +static uint64_t
>> +translate_log_addr(struct virtio_net *dev, struct vhost_virtqueue *vq,
>> +		uint64_t log_addr)
>> +{
>> +	if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM)) {
>> +		const uint64_t exp_size = sizeof(struct vring_used) +
>> +			sizeof(struct vring_used_elem) * vq->size;
>> +		uint64_t vva, gpa;
>> +		uint64_t size = exp_size;
>> +
>> +		vva = vhost_user_iotlb_cache_find(vq, log_addr,
>> +					&size, VHOST_ACCESS_RW);
>> +		if (size != exp_size) {
>> +			vhost_user_iotlb_miss(dev, log_addr + size,
>> +					      VHOST_ACCESS_RW);
>> +			return 0;
>> +		}
>> +
>> +		gpa = vva_to_gpa(dev, vva, &size);
>> +		if (size != exp_size) {
>> +			RTE_LOG(ERR, VHOST_CONFIG,
>> +				"VQ: Failed to find GPA mapping for log_addr."
>> +				"log_addr: 0x%0lx vva: 0x%0lx\n",
>> +				log_addr, vva);
>> +			return 0;
>> +		}
>> +		return gpa;
>> +
>> +	} else
>> +		return log_addr;
>> +}
>> +
>>  static struct virtio_net *
>>  translate_ring_addresses(struct virtio_net *dev, int vq_index)
>>  {
>> @@ -676,7 +744,15 @@ translate_ring_addresses(struct virtio_net *dev, int vq_index)
>>  		vq->last_avail_idx = vq->used->idx;
>>  	}
>>  
>> -	vq->log_guest_addr = addr->log_guest_addr;
>> +	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;
>> +	}
>> +
>>  
>>  	VHOST_LOG_DEBUG(VHOST_CONFIG, "(%d) mapped address desc: %p\n",
>>  			dev->vid, vq->desc);
>> -- 
>> 2.21.0
>>


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [dpdk-dev] [PATCH] vhost: translate incoming log address to gpa
  2019-09-23 10:28   ` Adrian Moreno
@ 2019-09-24  5:23     ` Tiwei Bie
  0 siblings, 0 replies; 4+ messages in thread
From: Tiwei Bie @ 2019-09-24  5:23 UTC (permalink / raw)
  To: Adrian Moreno; +Cc: dev, zhihong.wang, Maxime Coquelin, Pei Zhang

On Mon, Sep 23, 2019 at 12:28:35PM +0200, Adrian Moreno wrote:
> On 9/23/19 9:25 AM, Tiwei Bie wrote:
> > On Tue, Sep 17, 2019 at 04:49:00PM +0200, Adrian Moreno wrote:
> >> When IOMMU is enabled the incoming log address is in IOVA space. In that
> >> case, look in IOTLB table and translate the resulting HVA to GPA.
> >>
> >> If IOMMU is not enabled, the incoming log address is already a GPA so no
> >> transformation is needed.
> >>
> >> This change makes page logging work when IOVA_VA is selected in the guest.
> > 
> > Besides the log address of the ring, when IOMMU is enabled,
> > the addresses in descriptors are also IOVAs and should be
> > translated to GPAs before doing the dirty page logging.
> > 
> Thanks Tiwei. You're right. In fact, it's not the only place where IOVAs are
> assumed to be GPAs, for example in vhost.h:gpa_to_hpa:
> 
> /* Convert guest physical address to host physical address */
> static __rte_always_inline rte_iova_t
> gpa_to_hpa(struct virtio_net *dev, uint64_t gpa, uint64_t size)
> {
> 	uint32_t i;
> 	struct guest_page *page;
> 
> 	for (i = 0; i < dev->nr_guest_pages; i++) {
> 		page = &dev->guest_pages[i];
> 
> 		if (gpa >= page->guest_phys_addr &&
> 		    gpa + size < page->guest_phys_addr + page->size) {
> 			return gpa - page->guest_phys_addr +
> 			       page->host_phys_addr;
> 		}
> 	}
> 
> 	return 0;
> }
> 
> used in ZERO-COPY mode to check if the IOVA range is continuous in host physical
> memory and called buffer IOVAs. If I'm not mistaken this should also be
> translated as:
> 
> IOVA --> HVA (iotlb lookup)
> HVA --> GPA (mem_region lookup)
> GPA --> HIOVA (guest_page lookup)
> 
> Right?

Yeah, these IOVAs also shouldn't be assumed as GPAs.

Thanks,
Tiwei

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-09-24  5:26 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-17 14:49 [dpdk-dev] [PATCH] vhost: translate incoming log address to gpa Adrian Moreno
2019-09-23  7:25 ` Tiwei Bie
2019-09-23 10:28   ` Adrian Moreno
2019-09-24  5:23     ` Tiwei Bie

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).