[v3,2/5] vhost: implement rte_power_monitor API

Message ID 20210924102309.231304-3-miao.li@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series Implement rte_power_monitor API in virtio/vhost PMD |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Li, Miao Sept. 24, 2021, 10:23 a.m. UTC
  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 value size of monitor address, the match flag used to distinguish the
value used to match something or not match something. Vhost driver can use
these information to fill rte_power_monitor_cond.

Signed-off-by: Miao Li <miao.li@intel.com>
---
 lib/vhost/rte_vhost.h | 41 +++++++++++++++++++++++++++++++++++++++++
 lib/vhost/version.map |  3 +++
 lib/vhost/vhost.c     | 38 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+)
  

Comments

Chenbo Xia Sept. 29, 2021, 3:01 a.m. UTC | #1
Hi Miao,

> -----Original Message-----
> From: Li, Miao <miao.li@intel.com>
> Sent: Friday, September 24, 2021 6:23 PM
> To: dev@dpdk.org
> Cc: Xia, Chenbo <chenbo.xia@intel.com>; maxime.coquelin@redhat.com; Li, Miao
> <miao.li@intel.com>
> Subject: [PATCH v3 2/5] 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 value size of monitor address, the match flag used to distinguish the
> value used to match something or not match something. Vhost driver can use
> these information to fill rte_power_monitor_cond.
> 
> Signed-off-by: Miao Li <miao.li@intel.com>
> ---
>  lib/vhost/rte_vhost.h | 41 +++++++++++++++++++++++++++++++++++++++++
>  lib/vhost/version.map |  3 +++
>  lib/vhost/vhost.c     | 38 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 82 insertions(+)
> 
> diff --git a/lib/vhost/rte_vhost.h b/lib/vhost/rte_vhost.h
> index 8d875e9322..4e1f2de12f 100644
> --- a/lib/vhost/rte_vhost.h
> +++ b/lib/vhost/rte_vhost.h
> @@ -292,6 +292,30 @@ 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;

Will be read and masked, then compared with this value?

> +	uint64_t mask; /**< 64-bit mask to extract value read from `addr` */
> +	/**< Data size (in bytes) that will be read from the
> +	 *   monitored memory location (`addr`). Can be 1, 2,
> +	 *   4, or 8. Supplying any other value will result in
> +	 *   an error.
> +	 */
> +	uint8_t size;
> +	/**< If 1, checking if `val` matches something.
> +	 *  If 0, checking if `val` *doesn't* match a
> +	 *  particular value.
> +	 */

If 1, and masked value that read from `addr` equals `val`, drivers can exit the
power-saving state.

If 0, ....

The overall comment can't make me understand how this struct is used if I read
the next patch.

> +	uint8_t match;

The comment style is a bit messy here. You can make every comment above variable
Definition (like val/size/match ) or make the first line the same as the variable
line (like addr/mask). 

And please update the release note(release_20_11.rst), it's a new feature.

Thanks,
Chenbo
  
Li, Miao Oct. 11, 2021, 5:16 a.m. UTC | #2
Hi Chenbo,

> -----Original Message-----
> From: Xia, Chenbo <chenbo.xia@intel.com>
> Sent: Wednesday, September 29, 2021 11:01 AM
> To: Li, Miao <miao.li@intel.com>; dev@dpdk.org
> Cc: maxime.coquelin@redhat.com
> Subject: RE: [PATCH v3 2/5] vhost: implement rte_power_monitor API
> 
> Hi Miao,
> 
> > -----Original Message-----
> > From: Li, Miao <miao.li@intel.com>
> > Sent: Friday, September 24, 2021 6:23 PM
> > To: dev@dpdk.org
> > Cc: Xia, Chenbo <chenbo.xia@intel.com>; maxime.coquelin@redhat.com; Li,
> Miao
> > <miao.li@intel.com>
> > Subject: [PATCH v3 2/5] 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 value size of monitor address, the match flag used to distinguish the
> > value used to match something or not match something. Vhost driver can use
> > these information to fill rte_power_monitor_cond.
> >
> > Signed-off-by: Miao Li <miao.li@intel.com>
> > ---
> >  lib/vhost/rte_vhost.h | 41 +++++++++++++++++++++++++++++++++++++++++
> >  lib/vhost/version.map |  3 +++
> >  lib/vhost/vhost.c     | 38 ++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 82 insertions(+)
> >
> > diff --git a/lib/vhost/rte_vhost.h b/lib/vhost/rte_vhost.h
> > index 8d875e9322..4e1f2de12f 100644
> > --- a/lib/vhost/rte_vhost.h
> > +++ b/lib/vhost/rte_vhost.h
> > @@ -292,6 +292,30 @@ 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;
> 
> Will be read and masked, then compared with this value?
> 
> > +	uint64_t mask; /**< 64-bit mask to extract value read from `addr` */
> > +	/**< Data size (in bytes) that will be read from the
> > +	 *   monitored memory location (`addr`). Can be 1, 2,
> > +	 *   4, or 8. Supplying any other value will result in
> > +	 *   an error.
> > +	 */
> > +	uint8_t size;
> > +	/**< If 1, checking if `val` matches something.
> > +	 *  If 0, checking if `val` *doesn't* match a
> > +	 *  particular value.
> > +	 */
> 
> If 1, and masked value that read from `addr` equals `val`, drivers can exit the
> power-saving state.
> 
> If 0, ....
> 
> The overall comment can't make me understand how this struct is used if I read
> the next patch.
> 
> > +	uint8_t match;
> 
> The comment style is a bit messy here. You can make every comment above
> variable
> Definition (like val/size/match ) or make the first line the same as the variable
> line (like addr/mask).

I will modify them in the next version.

> 
> And please update the release note(release_20_11.rst), it's a new feature.

I will update it in the next version.

Thanks,
Miao

> 
> Thanks,
> Chenbo
  

Patch

diff --git a/lib/vhost/rte_vhost.h b/lib/vhost/rte_vhost.h
index 8d875e9322..4e1f2de12f 100644
--- a/lib/vhost/rte_vhost.h
+++ b/lib/vhost/rte_vhost.h
@@ -292,6 +292,30 @@  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` */
+	/**< Data size (in bytes) that will be read from the
+	 *   monitored memory location (`addr`). Can be 1, 2,
+	 *   4, or 8. Supplying any other value will result in
+	 *   an error.
+	 */
+	uint8_t size;
+	/**< If 1, checking if `val` matches something.
+	 *  If 0, checking if `val` *doesn't* match a
+	 *  particular value.
+	 */
+	uint8_t match;
+};
+
 /**
  * Convert guest physical address to host virtual address
  *
@@ -914,6 +938,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..a2b8133d50 100644
--- a/lib/vhost/vhost.c
+++ b/lib/vhost/vhost.c
@@ -1886,5 +1886,43 @@  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);
+	struct vhost_virtqueue *vq;
+
+	if (dev == NULL)
+		return -1;
+	if (queue_id >= VHOST_MAX_VRING)
+		return -1;
+
+	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->size = sizeof(desc[vq->last_avail_idx].flags);
+		pmc->match = 1;
+	} else {
+		pmc->addr = &vq->avail->idx;
+		pmc->val = vq->last_avail_idx & (vq->size - 1);
+		pmc->mask = vq->size - 1;
+		pmc->size = sizeof(vq->avail->idx);
+		pmc->match = 0;
+	}
+
+	return 0;
+}
+
 RTE_LOG_REGISTER_SUFFIX(vhost_config_log_level, config, INFO);
 RTE_LOG_REGISTER_SUFFIX(vhost_data_log_level, data, WARNING);