Hi Miao, > -----Original Message----- > From: Li, Miao <miao...@intel.com> > Sent: Friday, September 10, 2021 9:06 PM > To: dev@dpdk.org > Cc: Xia, Chenbo <chenbo....@intel.com>; maxime.coque...@redhat.com; Li, Miao > <miao...@intel.com> > Subject: [PATCH 2/5] lib/vhost: implement rte_power_monitor API
Should be 'vhost: implement rte_power_monitor API' > > This patch defines rte_vhost_power_monitor_cond which is used to pass > some information to vhost driver. The information is including the address > to monitor, the expected value, the mask to extract value read from 'addr', > the flag used to distinguish packed ring or split ring. Vhost driver can > use these information to fill rte_power_monitor_cond. > > Signed-off-by: Miao Li <miao...@intel.com> > --- > lib/vhost/rte_vhost.h | 33 +++++++++++++++++++++++++++++++++ > lib/vhost/version.map | 3 +++ > lib/vhost/vhost.c | 30 ++++++++++++++++++++++++++++++ > 3 files changed, 66 insertions(+) > > diff --git a/lib/vhost/rte_vhost.h b/lib/vhost/rte_vhost.h > index 8d875e9322..f58643b0a3 100644 > --- a/lib/vhost/rte_vhost.h > +++ b/lib/vhost/rte_vhost.h > @@ -38,6 +38,8 @@ extern "C" { > #define RTE_VHOST_USER_ASYNC_COPY (1ULL << 7) > #define RTE_VHOST_USER_NET_COMPLIANT_OL_FLAGS (1ULL << 8) > > +#define VHOST_POWER_MONITOR_RING_PACKED (1ULL << 0) I'd say I don't quite like introducing this flag so that vhost lib app needs to know the vring is split or packed. I have another suggestion to do the same thing, please check below comment. > + > /* Features. */ > #ifndef VIRTIO_NET_F_GUEST_ANNOUNCE > #define VIRTIO_NET_F_GUEST_ANNOUNCE 21 > @@ -292,6 +294,20 @@ struct vhost_device_ops { > void *reserved[1]; /**< Reserved for future extension */ > }; > > +/** > + * Power monitor condition. > + */ > +struct rte_vhost_power_monitor_cond { > + volatile void *addr; /**< Address to monitor for changes */ > + /**< If the `mask` is non-zero, location pointed > + * to by `addr` will be read and compared > + * against this value. > + */ > + uint64_t val; > + uint64_t mask; /**< 64-bit mask to extract value read from `addr` */ > + uint8_t flag; /**< if 1, vhost packed ring, otherwise split ring */ What about define two values instead of the flag. One value for '(value & m) == v ?' is True, another for False. > +}; > + > /** > * Convert guest physical address to host virtual address > * > @@ -914,6 +930,23 @@ int rte_vhost_vring_call(int vid, uint16_t vring_idx); > */ > uint32_t rte_vhost_rx_queue_count(int vid, uint16_t qid); > > +/** > + * Get power monitor address of the vhost device > + * > + * @param vid > + * vhost device ID > + * @param queue_id > + * vhost queue ID > + * @param pmc > + * power monitor condition > + * @return > + * 0 on success, -1 on failure > + */ > +__rte_experimental > +int > +rte_vhost_get_monitor_addr(int vid, uint16_t queue_id, > + struct rte_vhost_power_monitor_cond *pmc); > + > /** > * Get log base and log size of the vhost device > * > diff --git a/lib/vhost/version.map b/lib/vhost/version.map > index c92a9d4962..0a9667ef1e 100644 > --- a/lib/vhost/version.map > +++ b/lib/vhost/version.map > @@ -85,4 +85,7 @@ EXPERIMENTAL { > rte_vhost_async_channel_register_thread_unsafe; > rte_vhost_async_channel_unregister_thread_unsafe; > rte_vhost_clear_queue_thread_unsafe; > + > + # added in 21.11 > + rte_vhost_get_monitor_addr; > }; > diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c > index 355ff37651..f7374d3f94 100644 > --- a/lib/vhost/vhost.c > +++ b/lib/vhost/vhost.c > @@ -1886,5 +1886,35 @@ int rte_vhost_async_get_inflight(int vid, uint16_t > queue_id) > return ret; > } > > +int > +rte_vhost_get_monitor_addr(int vid, uint16_t queue_id, > + struct rte_vhost_power_monitor_cond *pmc) > +{ > + struct virtio_net *dev = get_device(vid); Check dev is not NULL before accessing its member. > + struct vhost_virtqueue *vq = dev->virtqueue[queue_id]; > + if (vq == NULL) > + return -1; > + if (vq_is_packed(dev)) { > + struct vring_packed_desc *desc; > + desc = vq->desc_packed; > + pmc->addr = &desc[vq->last_avail_idx].flags; > + if (vq->avail_wrap_counter) > + pmc->val = VRING_DESC_F_AVAIL; > + else > + pmc->val = VRING_DESC_F_USED; > + pmc->mask = VRING_DESC_F_AVAIL | VRING_DESC_F_USED; > + pmc->flag = VHOST_POWER_MONITOR_RING_PACKED; > + } else { > + pmc->addr = &vq->avail->idx; > + pmc->val = vq->last_avail_idx & (vq->size - 1); > + pmc->mask = vq->size - 1; > + pmc->flag = 0; > + } > + if (pmc->addr == NULL) > + return -1; Is it possible that addr == NULL? Thanks, Chenbo > + > + return 0; > +} > + > RTE_LOG_REGISTER_SUFFIX(vhost_config_log_level, config, INFO); > RTE_LOG_REGISTER_SUFFIX(vhost_data_log_level, data, WARNING); > -- > 2.25.1