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.coque...@redhat.com>
>> Reported-by: Pei Zhang <pezh...@redhat.com>
>>
>> Signed-off-by: Adrian Moreno <amore...@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
>>
