mbox series

[v2,0/2] vhost-vdpa: add trace functions in vhost-vdpa.c

Message ID 20200921130406.941363-1-lvivier@redhat.com
Headers show
Series vhost-vdpa: add trace functions in vhost-vdpa.c | expand

Message

Laurent Vivier Sept. 21, 2020, 1:04 p.m. UTC 
Add trace events functions in vdpa functions.

Modify qemu_hexdump() to be able to dump the vdpa config
in trace logs in hexadecimal form.

v2: fix problems reported by checkpatch (TAB, missing 0x and 80+ line)
    added Stefan Acked-by

Laurent Vivier (2):
  util/hexdump: introduce qemu_hexdump_line()
  vhost-vdpa: add trace-events

 hw/virtio/trace-events | 29 ++++++++++++++
 hw/virtio/vhost-vdpa.c | 86 +++++++++++++++++++++++++++++++++++++++---
 include/qemu-common.h  |  8 ++++
 util/hexdump.c         | 54 ++++++++++++++++----------
 4 files changed, 152 insertions(+), 25 deletions(-)

-- 
2.26.2

Comments

Philippe Mathieu-Daudé Sept. 22, 2020, 7:37 a.m. UTC | #1 
On 9/21/20 3:04 PM, Laurent Vivier wrote:
> Add trace functionis in vhost-vdpa.c.

Typo "function"?

> 
> 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(-)
Laurent Vivier Sept. 24, 2020, 8:42 a.m. UTC | #2 
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. As I don't see the link with vhost_vdpa_set_vring_call() I would
keep it.

Did I miss something?

Thanks,
Laurent
Jason Wang Sept. 24, 2020, 9:13 a.m. UTC | #3 
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
>
>