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

[Jason Wang]

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