Re: [PATCH v2 2/2] vhost-vdpa: add trace-events

From: Jason Wang <jasowang@redhat.com>
To: Laurent Vivier <lvivier@redhat.com>, qemu-devel@nongnu.org
Cc: Cindy Lu <lulu@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [PATCH v2 2/2] vhost-vdpa: add trace-events
Date: Thu, 24 Sep 2020 17:13:03 +0800	[thread overview]
Message-ID: <2bf402e0-cdef-9ab8-b09c-d7f5768afaf8@redhat.com> (raw)
In-Reply-To: <0622a2db-98e8-6378-9bc4-6119de7042ae@redhat.com>

On 2020/9/24 下午4:42, Laurent Vivier wrote:
> On 22/09/2020 04:09, Jason Wang wrote:
>> On 2020/9/21 下午9:04, Laurent Vivier wrote:
>>> Add trace functionis in vhost-vdpa.c.
>>>
>>> All traces from this file can be enabled with '-trace vhost_vdpa*'.
>>>
>>> Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
>>> Signed-off-by: Laurent Vivier <lvivier@redhat.com>
>>> ---
>>>    hw/virtio/trace-events | 29 ++++++++++++++
>>>    hw/virtio/vhost-vdpa.c | 86 +++++++++++++++++++++++++++++++++++++++---
>>>    2 files changed, 110 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/hw/virtio/trace-events b/hw/virtio/trace-events
>>> index 845200bf109d..2b453f77a4e3 100644
>>> --- a/hw/virtio/trace-events
>>> +++ b/hw/virtio/trace-events
>>> @@ -22,6 +22,35 @@ vhost_user_postcopy_waker(const char *rb, uint64_t
>>> rb_offset) "%s + 0x%"PRIx64
>>>    vhost_user_postcopy_waker_found(uint64_t client_addr) "0x%"PRIx64
>>>    vhost_user_postcopy_waker_nomatch(const char *rb, uint64_t
>>> rb_offset) "%s + 0x%"PRIx64
>>>    +# vhost-vdpa.c
>>> +vhost_vdpa_listener_region_add(void *vdpa, uint64_t iova, uint64_t
>>> llend, void *vaddr, bool readonly) "vdpa: %p iova 0x%"PRIx64" llend
>>> 0x%"PRIx64" vaddr: %p read-only: %d"
>>> +vhost_vdpa_listener_region_del(void *vdpa, uint64_t iova, uint64_t
>>> llend) "vdpa: %p iova 0x%"PRIx64" llend 0x%"PRIx64
>>> +vhost_vdpa_add_status(void *dev, uint8_t status) "dev: %p status:
>>> 0x%"PRIx8
>>> +vhost_vdpa_init(void *dev, void *vdpa) "dev: %p vdpa: %p"
>>> +vhost_vdpa_cleanup(void *dev, void *vdpa) "dev: %p vdpa: %p"
>>> +vhost_vdpa_memslots_limit(void *dev, int ret) "dev: %p = 0x%x"
>>> +vhost_vdpa_set_mem_table(void *dev, uint32_t nregions, uint32_t
>>> padding) "dev: %p nregions: %"PRIu32" padding: 0x%"PRIx32
>>> +vhost_vdpa_dump_regions(void *dev, int i, uint64_t guest_phys_addr,
>>> uint64_t memory_size, uint64_t userspace_addr, uint64_t flags_padding)
>>> "dev: %p %d: guest_phys_addr: 0x%"PRIx64" memory_size: 0x%"PRIx64"
>>> userspace_addr: 0x%"PRIx64" flags_padding: 0x%"PRIx64
>>> +vhost_vdpa_set_features(void *dev, uint64_t features) "dev: %p
>>> features: 0x%"PRIx64
>>> +vhost_vdpa_get_device_id(void *dev, uint32_t device_id) "dev: %p
>>> device_id %"PRIu32
>>> +vhost_vdpa_reset_device(void *dev, uint8_t status) "dev: %p status:
>>> 0x%"PRIx8
>>> +vhost_vdpa_get_vq_index(void *dev, int idx, int vq_idx) "dev: %p idx:
>>> %d vq idx: %d"
>>> +vhost_vdpa_set_vring_ready(void *dev) "dev: %p"
>>> +vhost_vdpa_dump_config(void *dev, const char *line) "dev: %p %s"
>>> +vhost_vdpa_set_config(void *dev, uint32_t offset, uint32_t size,
>>> uint32_t flags) "dev: %p offset: %"PRIu32" size: %"PRIu32" flags:
>>> 0x%"PRIx32
>>> +vhost_vdpa_get_config(void *dev, void *config, uint32_t config_len)
>>> "dev: %p config: %p config_len: %"PRIu32
>>> +vhost_vdpa_dev_start(void *dev, bool started) "dev: %p started: %d"
>>> +vhost_vdpa_set_log_base(void *dev, uint64_t base, unsigned long long
>>> size, int refcnt, int fd, void *log) "dev: %p base: 0x%"PRIx64" size:
>>> %llu refcnt: %d fd: %d log: %p"
>>> +vhost_vdpa_set_vring_addr(void *dev, unsigned int index, unsigned int
>>> flags, uint64_t desc_user_addr, uint64_t used_user_addr, uint64_t
>>> avail_user_addr, uint64_t log_guest_addr) "dev: %p index: %u flags:
>>> 0x%x desc_user_addr: 0x%"PRIx64" used_user_addr: 0x%"PRIx64"
>>> avail_user_addr: 0x%"PRIx64" log_guest_addr: 0x%"PRIx64
>>> +vhost_vdpa_set_vring_num(void *dev, unsigned int index, unsigned int
>>> num) "dev: %p index: %u num: %u"
>>> +vhost_vdpa_set_vring_base(void *dev, unsigned int index, unsigned int
>>> num) "dev: %p index: %u num: %u"
>>> +vhost_vdpa_get_vring_base(void *dev, unsigned int index, unsigned int
>>> num) "dev: %p index: %u num: %u"
>>> +vhost_vdpa_set_vring_kick(void *dev, unsigned int index, int fd)
>>> "dev: %p index: %u fd: %d"
>>> +vhost_vdpa_set_vring_call(void *dev, unsigned int index, int fd)
>>> "dev: %p index: %u fd: %d"
>>
>> It's better to add set/get_vring_addr() and dma_map()/dma_unmap().
> I'm adding dma_map()/dma_unmap().
>
> set_vring_addr() is already in the list, and get_vring_addr() doesn't
> exist.

Your are right. We don't have get_vring_addr() actually.

> As I don't see the link with vhost_vdpa_set_vring_call() I would
> keep it.
>
> Did I miss something?

Nope, there's no link.

Please keep that.

Thanks

>
> Thanks,
> Laurent
>
>