Re: [dpdk-dev] [PATCH v2 14/14] vhost: split vDPA header file

[Adrian Moreno <amorenoz@redhat.com>]

On 6/24/20 2:27 PM, Maxime Coquelin wrote:
> This patch split the vDPA header file in two, making
> rte_vdpa_device structure opaque to the application.
> 
> Applications should only include rte_vdpa.h, while drivers
> should include both rte_vdpa.h and rte_vdpa_dev.h.
> 
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  drivers/vdpa/ifc/ifcvf_vdpa.c   |   1 +
>  drivers/vdpa/mlx5/mlx5_vdpa.h   |   1 +
>  lib/librte_vhost/Makefile       |   3 +-
>  lib/librte_vhost/meson.build    |   3 +-
>  lib/librte_vhost/rte_vdpa.h     | 142 ------------------------------
>  lib/librte_vhost/rte_vdpa_dev.h | 147 ++++++++++++++++++++++++++++++++
>  lib/librte_vhost/vdpa.c         |   1 +
>  lib/librte_vhost/vhost.h        |   1 +
>  8 files changed, 155 insertions(+), 144 deletions(-)
>  create mode 100644 lib/librte_vhost/rte_vdpa_dev.h
> 
> diff --git a/drivers/vdpa/ifc/ifcvf_vdpa.c b/drivers/vdpa/ifc/ifcvf_vdpa.c
> index 4163047744..f81d13a2c8 100644
> --- a/drivers/vdpa/ifc/ifcvf_vdpa.c
> +++ b/drivers/vdpa/ifc/ifcvf_vdpa.c
> @@ -16,6 +16,7 @@
>  #include <rte_bus_pci.h>
>  #include <rte_vhost.h>
>  #include <rte_vdpa.h>
> +#include <rte_vdpa_dev.h>
>  #include <rte_vfio.h>
>  #include <rte_spinlock.h>
>  #include <rte_log.h>
> diff --git a/drivers/vdpa/mlx5/mlx5_vdpa.h b/drivers/vdpa/mlx5/mlx5_vdpa.h
> index ea09576c5a..de4154b183 100644
> --- a/drivers/vdpa/mlx5/mlx5_vdpa.h
> +++ b/drivers/vdpa/mlx5/mlx5_vdpa.h
> @@ -12,6 +12,7 @@
>  #pragma GCC diagnostic ignored "-Wpedantic"
>  #endif
>  #include <rte_vdpa.h>
> +#include <rte_vdpa_dev.h>
>  #include <rte_vhost.h>
>  #ifdef PEDANTIC
>  #pragma GCC diagnostic error "-Wpedantic"
> diff --git a/lib/librte_vhost/Makefile b/lib/librte_vhost/Makefile
> index e592795f22..b7ff7dc4b3 100644
> --- a/lib/librte_vhost/Makefile
> +++ b/lib/librte_vhost/Makefile
> @@ -41,7 +41,8 @@ SRCS-$(CONFIG_RTE_LIBRTE_VHOST) := fd_man.c iotlb.c socket.c vhost.c \
>  					vhost_user.c virtio_net.c vdpa.c
>  
>  # install includes
> -SYMLINK-$(CONFIG_RTE_LIBRTE_VHOST)-include += rte_vhost.h rte_vdpa.h
> +SYMLINK-$(CONFIG_RTE_LIBRTE_VHOST)-include += rte_vhost.h rte_vdpa.h \
> +						rte_vdpa_dev.h
>  
>  # only compile vhost crypto when cryptodev is enabled
>  ifeq ($(CONFIG_RTE_LIBRTE_CRYPTODEV),y)
> diff --git a/lib/librte_vhost/meson.build b/lib/librte_vhost/meson.build
> index f80632848c..882a0eaf41 100644
> --- a/lib/librte_vhost/meson.build
> +++ b/lib/librte_vhost/meson.build
> @@ -21,5 +21,6 @@ cflags += '-fno-strict-aliasing'
>  sources = files('fd_man.c', 'iotlb.c', 'socket.c', 'vdpa.c',
>  		'vhost.c', 'vhost_user.c',
>  		'virtio_net.c', 'vhost_crypto.c')
> -headers = files('rte_vhost.h', 'rte_vdpa.h', 'rte_vhost_crypto.h')
> +headers = files('rte_vhost.h', 'rte_vdpa.h', 'rte_vdpa_dev.h',
> +		'rte_vhost_crypto.h')
>  deps += ['ethdev', 'cryptodev', 'hash', 'pci']
> diff --git a/lib/librte_vhost/rte_vdpa.h b/lib/librte_vhost/rte_vdpa.h
> index 776ed67c16..5065e2130e 100644
> --- a/lib/librte_vhost/rte_vdpa.h
> +++ b/lib/librte_vhost/rte_vdpa.h
> @@ -11,13 +11,6 @@
>   * Device specific vhost lib
>   */
>  
> -#include <stdbool.h>
> -
> -#include <rte_pci.h>
> -#include "rte_vhost.h"
> -
> -#define MAX_VDPA_NAME_LEN 128
> -
>  /** Maximum name length for statistics counters */
>  #define RTE_VDPA_STATS_NAME_SIZE 64
>  
> @@ -48,103 +41,6 @@ struct rte_vdpa_stat_name {
>  	char name[RTE_VDPA_STATS_NAME_SIZE]; /**< The statistic name */
>  };
>  
> -/**
> - * vdpa device operations
> - */
> -struct rte_vdpa_dev_ops {
> -	/** Get capabilities of this device */
> -	int (*get_queue_num)(struct rte_vdpa_device *dev, uint32_t *queue_num);
> -
> -	/** Get supported features of this device */
> -	int (*get_features)(struct rte_vdpa_device *dev, uint64_t *features);
> -
> -	/** Get supported protocol features of this device */
> -	int (*get_protocol_features)(struct rte_vdpa_device *dev,
> -			uint64_t *protocol_features);
> -
> -	/** Driver configure/close the device */
> -	int (*dev_conf)(int vid);
> -	int (*dev_close)(int vid);
> -
> -	/** Enable/disable this vring */
> -	int (*set_vring_state)(int vid, int vring, int state);
> -
> -	/** Set features when changed */
> -	int (*set_features)(int vid);
> -
> -	/** Destination operations when migration done */
> -	int (*migration_done)(int vid);
> -
> -	/** Get the vfio group fd */
> -	int (*get_vfio_group_fd)(int vid);
> -
> -	/** Get the vfio device fd */
> -	int (*get_vfio_device_fd)(int vid);
> -
> -	/** Get the notify area info of the queue */
> -	int (*get_notify_area)(int vid, int qid,
> -			uint64_t *offset, uint64_t *size);
> -
> -	/** Get statistics name */
> -	int (*get_stats_names)(struct rte_vdpa_device *dev,
> -			struct rte_vdpa_stat_name *stats_names,
> -			unsigned int size);
> -
> -	/** Get statistics of the queue */
> -	int (*get_stats)(struct rte_vdpa_device *dev, int qid,
> -			struct rte_vdpa_stat *stats, unsigned int n);
> -
> -	/** Reset statistics of the queue */
> -	int (*reset_stats)(struct rte_vdpa_device *dev, int qid);
> -
> -	/** Reserved for future extension */
> -	void *reserved[2];
> -};
> -
> -/**
> - * vdpa device structure includes device address and device operations.
> - */
> -struct rte_vdpa_device {
> -	TAILQ_ENTRY(rte_vdpa_device) next;
> -	/** Generic device information */
> -	struct rte_device *device;
> -	/** vdpa device operations */
> -	struct rte_vdpa_dev_ops *ops;
> -} __rte_cache_aligned;
> -
> -/**
> - * @warning
> - * @b EXPERIMENTAL: this API may change without prior notice
> - *
> - * Register a vdpa device
> - *
> - * @param addr
> - *  the vdpa device address
> - * @param ops
> - *  the vdpa device operations
> - * @return
> - *  vDPA device pointer on success, NULL on failure
> - */
> -__rte_experimental
> -struct rte_vdpa_device *
> -rte_vdpa_register_device(struct rte_device *rte_dev,
> -		struct rte_vdpa_dev_ops *ops);
> -
> -/**
> - * @warning
> - * @b EXPERIMENTAL: this API may change without prior notice
> - *
> - * Unregister a vdpa device
> - *
> - * @param did
> - *  vDPA device pointer
> - * @return
> - *  device id on success, -1 on failure
> - */
> -__rte_experimental
> -int
> -rte_vdpa_unregister_device(struct rte_vdpa_device *);
> -
>  /**
>   * @warning
>   * @b EXPERIMENTAL: this API may change without prior notice
> @@ -175,44 +71,6 @@ __rte_experimental
>  struct rte_device *
>  rte_vdpa_get_rte_device(struct rte_vdpa_device *vdpa_dev);
>  
> -/**
> - * @warning
> - * @b EXPERIMENTAL: this API may change without prior notice
> - *
> - * Enable/Disable host notifier mapping for a vdpa port.
> - *
> - * @param vid
> - *  vhost device id
> - * @param enable
> - *  true for host notifier map, false for host notifier unmap
> - * @return
> - *  0 on success, -1 on failure
> - */
> -__rte_experimental
> -int
> -rte_vhost_host_notifier_ctrl(int vid, bool enable);
> -
> -/**
> - * @warning
> - * @b EXPERIMENTAL: this API may change without prior notice
> - *
> - * Synchronize the used ring from mediated ring to guest, log dirty
> - * page for each writeable buffer, caller should handle the used
> - * ring logging before device stop.
> - *
> - * @param vid
> - *  vhost device id
> - * @param qid
> - *  vhost queue id
> - * @param vring_m
> - *  mediated virtio ring pointer
> - * @return
> - *  number of synced used entries on success, -1 on failure
> - */
> -__rte_experimental
> -int
> -rte_vdpa_relay_vring_used(int vid, uint16_t qid, void *vring_m);
> -
>  /**
>   * @warning
>   * @b EXPERIMENTAL: this API may change without prior notice
> diff --git a/lib/librte_vhost/rte_vdpa_dev.h b/lib/librte_vhost/rte_vdpa_dev.h
> new file mode 100644
> index 0000000000..fecaa8e081
> --- /dev/null
> +++ b/lib/librte_vhost/rte_vdpa_dev.h
> @@ -0,0 +1,147 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2018 Intel Corporation
> + */
> +
> +#ifndef _RTE_VDPA_H_DEV_
> +#define _RTE_VDPA_H_DEV_
> +
> +#include <stdbool.h>
> +
> +#include "rte_vhost.h"
> +
> +/**
> + * vdpa device operations
> + */
> +struct rte_vdpa_dev_ops {
> +	/** Get capabilities of this device */
> +	int (*get_queue_num)(struct rte_vdpa_device *dev, uint32_t *queue_num);
> +
> +	/** Get supported features of this device */
> +	int (*get_features)(struct rte_vdpa_device *dev, uint64_t *features);
> +
> +	/** Get supported protocol features of this device */
> +	int (*get_protocol_features)(struct rte_vdpa_device *dev,
> +			uint64_t *protocol_features);
> +
> +	/** Driver configure/close the device */
> +	int (*dev_conf)(int vid);
> +	int (*dev_close)(int vid);
> +
> +	/** Enable/disable this vring */
> +	int (*set_vring_state)(int vid, int vring, int state);
> +
> +	/** Set features when changed */
> +	int (*set_features)(int vid);
> +
> +	/** Destination operations when migration done */
> +	int (*migration_done)(int vid);
> +
> +	/** Get the vfio group fd */
> +	int (*get_vfio_group_fd)(int vid);
> +
> +	/** Get the vfio device fd */
> +	int (*get_vfio_device_fd)(int vid);
> +
> +	/** Get the notify area info of the queue */
> +	int (*get_notify_area)(int vid, int qid,
> +			uint64_t *offset, uint64_t *size);
> +
> +	/** Get statistics name */
> +	int (*get_stats_names)(struct rte_vdpa_device *dev,
> +			struct rte_vdpa_stat_name *stats_names,
> +			unsigned int size);
> +
> +	/** Get statistics of the queue */
> +	int (*get_stats)(struct rte_vdpa_device *dev, int qid,
> +			struct rte_vdpa_stat *stats, unsigned int n);
> +
> +	/** Reset statistics of the queue */
> +	int (*reset_stats)(struct rte_vdpa_device *dev, int qid);
> +
> +	/** Reserved for future extension */
> +	void *reserved[2];
> +};
> +
> +/**
> + * vdpa device structure includes device address and device operations.
> + */
> +struct rte_vdpa_device {
> +	TAILQ_ENTRY(rte_vdpa_device) next;
> +	/** Generic device information */
> +	struct rte_device *device;
> +	/** vdpa device operations */
> +	struct rte_vdpa_dev_ops *ops;
> +};
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Register a vdpa device
> + *
> + * @param rte_dev
> + *  the generic device pointer
> + * @param ops
> + *  the vdpa device operations
> + * @return
> + *  vDPA device pointer on success, NULL on failure
> + */
> +__rte_experimental
> +struct rte_vdpa_device *
> +rte_vdpa_register_device(struct rte_device *rte_dev,
> +		struct rte_vdpa_dev_ops *ops);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Unregister a vdpa device
> + *
> + * @param dev
> + *  vDPA device pointer
> + * @return
> + *  device id on success, -1 on failure
> + */
> +__rte_experimental
> +int
> +rte_vdpa_unregister_device(struct rte_vdpa_device *dev);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Enable/Disable host notifier mapping for a vdpa port.
> + *
> + * @param vid
> + *  vhost device id
> + * @param enable
> + *  true for host notifier map, false for host notifier unmap
> + * @return
> + *  0 on success, -1 on failure
> + */
> +__rte_experimental
> +int
> +rte_vhost_host_notifier_ctrl(int vid, bool enable);
> +
> +/**
> + * @warning
> + * @b EXPERIMENTAL: this API may change without prior notice
> + *
> + * Synchronize the used ring from mediated ring to guest, log dirty
> + * page for each writeable buffer, caller should handle the used
> + * ring logging before device stop.
> + *
> + * @param vid
> + *  vhost device id
> + * @param qid
> + *  vhost queue id
> + * @param vring_m
> + *  mediated virtio ring pointer
> + * @return
> + *  number of synced used entries on success, -1 on failure
> + */
> +__rte_experimental
> +int
> +rte_vdpa_relay_vring_used(int vid, uint16_t qid, void *vring_m);
> +
> +#endif /* _RTE_VDPA_DEV_H_ */
> diff --git a/lib/librte_vhost/vdpa.c b/lib/librte_vhost/vdpa.c
> index 864770d506..04c4b25473 100644
> --- a/lib/librte_vhost/vdpa.c
> +++ b/lib/librte_vhost/vdpa.c
> @@ -17,6 +17,7 @@
>  #include <rte_tailq.h>
>  
>  #include "rte_vdpa.h"
> +#include "rte_vdpa_dev.h"
>  #include "vhost.h"
>  
>  /** Double linked list of vDPA devices. */
> diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
> index 819857a65a..941a42637e 100644
> --- a/lib/librte_vhost/vhost.h
> +++ b/lib/librte_vhost/vhost.h
> @@ -22,6 +22,7 @@
>  
>  #include "rte_vhost.h"
>  #include "rte_vdpa.h"
> +#include "rte_vdpa_dev.h"
>  
>  /* Used to indicate that the device is running on a data core */
>  #define VIRTIO_DEV_RUNNING 1
> 

Acked-by: Adrián Moreno <amorenoz@redhat.com>
-- 
Adrián Moreno