2020-04-24 10:09:39

by Zhu, Lingshan

[permalink] [raw]
Subject: [PATCH 0/2] Config interrupt support in VDPA and IFCVF

This series includes two patches, one introduced
config interrupt support in VDPA core, the other
one implemented config interrupt in IFCVF.

Zhu Lingshan (2):
vdpa: Support config interrupt in vhost_vdpa
vdpa: implement config interrupt in IFCVF

drivers/vdpa/ifcvf/ifcvf_base.c | 3 +++
drivers/vdpa/ifcvf/ifcvf_base.h | 2 ++
drivers/vdpa/ifcvf/ifcvf_main.c | 22 ++++++++++++++++-
drivers/vhost/vdpa.c | 53 ++++++++++++++++++++++++++++++++++++++++
include/uapi/linux/vhost.h | 2 ++
include/uapi/linux/vhost_types.h | 2 ++
6 files changed, 83 insertions(+), 1 deletion(-)

--
1.8.3.1


2020-04-24 10:10:12

by Zhu, Lingshan

[permalink] [raw]
Subject: [PATCH 1/2] vdpa: Support config interrupt in vhost_vdpa

This commit implements config interrupt support in
vhost_vdpa layer.

Signed-off-by: Zhu Lingshan <[email protected]>

Signed-off-by: Zhu Lingshan <[email protected]>
---
drivers/vhost/vdpa.c | 53 ++++++++++++++++++++++++++++++++++++++++
include/uapi/linux/vhost.h | 2 ++
include/uapi/linux/vhost_types.h | 2 ++
3 files changed, 57 insertions(+)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 421f02a..f1f69bf 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -21,6 +21,7 @@
#include <linux/nospec.h>
#include <linux/vhost.h>
#include <linux/virtio_net.h>
+#include <linux/kernel.h>

#include "vhost.h"

@@ -70,6 +71,7 @@ struct vhost_vdpa {
int nvqs;
int virtio_id;
int minor;
+ struct eventfd_ctx *config_ctx;
};

static DEFINE_IDA(vhost_vdpa_ida);
@@ -101,6 +103,17 @@ static irqreturn_t vhost_vdpa_virtqueue_cb(void *private)
return IRQ_HANDLED;
}

+static irqreturn_t vhost_vdpa_config_cb(void *private)
+{
+ struct vhost_vdpa *v = private;
+ struct eventfd_ctx *config_ctx = v->config_ctx;
+
+ if (config_ctx)
+ eventfd_signal(config_ctx, 1);
+
+ return IRQ_HANDLED;
+}
+
static void vhost_vdpa_reset(struct vhost_vdpa *v)
{
struct vdpa_device *vdpa = v->vdpa;
@@ -288,6 +301,42 @@ static long vhost_vdpa_get_vring_num(struct vhost_vdpa *v, u16 __user *argp)
return 0;
}

+static void vhost_vdpa_config_put(struct vhost_vdpa *v)
+{
+ if (v->config_ctx)
+ eventfd_ctx_put(v->config_ctx);
+}
+
+static long vhost_vdpa_set_config_call(struct vhost_vdpa *v, u32 __user *argp)
+{
+ struct vdpa_callback cb;
+ vhost_config_file file;
+ struct eventfd_ctx *ctx;
+
+ cb.callback = vhost_vdpa_config_cb;
+ cb.private = v->vdpa;
+ if (copy_from_user(&file, argp, sizeof(file)))
+ return -EFAULT;
+
+ if (file.fd == -1) {
+ vhost_vdpa_config_put(v);
+ v->config_ctx = NULL;
+ return PTR_ERR(v->config_ctx);
+ }
+
+ ctx = eventfd_ctx_fdget(file.fd);
+ swap(ctx, v->config_ctx);
+
+ if (!IS_ERR_OR_NULL(ctx))
+ eventfd_ctx_put(ctx);
+
+ if (IS_ERR(v->config_ctx))
+ return PTR_ERR(v->config_ctx);
+
+ v->vdpa->config->set_config_cb(v->vdpa, &cb);
+
+ return 0;
+}
static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd,
void __user *argp)
{
@@ -398,6 +447,9 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
case VHOST_SET_LOG_FD:
r = -ENOIOCTLCMD;
break;
+ case VHOST_VDPA_SET_CONFIG_CALL:
+ r = vhost_vdpa_set_config_call(v, argp);
+ break;
default:
r = vhost_dev_ioctl(&v->vdev, cmd, argp);
if (r == -ENOIOCTLCMD)
@@ -734,6 +786,7 @@ static int vhost_vdpa_release(struct inode *inode, struct file *filep)
vhost_dev_stop(&v->vdev);
vhost_vdpa_iotlb_free(v);
vhost_vdpa_free_domain(v);
+ vhost_vdpa_config_put(v);
vhost_dev_cleanup(&v->vdev);
kfree(v->vdev.vqs);
mutex_unlock(&d->mutex);
diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
index 9fe72e4..c474a35 100644
--- a/include/uapi/linux/vhost.h
+++ b/include/uapi/linux/vhost.h
@@ -140,4 +140,6 @@
/* Get the max ring size. */
#define VHOST_VDPA_GET_VRING_NUM _IOR(VHOST_VIRTIO, 0x76, __u16)

+/* Set event fd for config interrupt*/
+#define VHOST_VDPA_SET_CONFIG_CALL _IOW(VHOST_VIRTIO, 0x77, vhost_config_file)
#endif
diff --git a/include/uapi/linux/vhost_types.h b/include/uapi/linux/vhost_types.h
index 669457c..6759aefb 100644
--- a/include/uapi/linux/vhost_types.h
+++ b/include/uapi/linux/vhost_types.h
@@ -27,6 +27,8 @@ struct vhost_vring_file {

};

+typedef struct vhost_vring_file vhost_config_file;
+
struct vhost_vring_addr {
unsigned int index;
/* Option flags. */
--
1.8.3.1

2020-04-24 10:10:35

by Zhu, Lingshan

[permalink] [raw]
Subject: [PATCH 2/2] vdpa: implement config interrupt in IFCVF

This commit implements config interrupt support
in IFC VF

Signed-off-by: Zhu Lingshan <[email protected]>
---
drivers/vdpa/ifcvf/ifcvf_base.c | 3 +++
drivers/vdpa/ifcvf/ifcvf_base.h | 2 ++
drivers/vdpa/ifcvf/ifcvf_main.c | 22 +++++++++++++++++++++-
3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c
index b61b06e..c825d99 100644
--- a/drivers/vdpa/ifcvf/ifcvf_base.c
+++ b/drivers/vdpa/ifcvf/ifcvf_base.c
@@ -185,6 +185,9 @@ void ifcvf_set_status(struct ifcvf_hw *hw, u8 status)

void ifcvf_reset(struct ifcvf_hw *hw)
{
+ hw->config_cb.callback = NULL;
+ hw->config_cb.private = NULL;
+
ifcvf_set_status(hw, 0);
/* flush set_status, make sure VF is stopped, reset */
ifcvf_get_status(hw);
diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h
index e803070..76928b0 100644
--- a/drivers/vdpa/ifcvf/ifcvf_base.h
+++ b/drivers/vdpa/ifcvf/ifcvf_base.h
@@ -81,6 +81,8 @@ struct ifcvf_hw {
void __iomem *net_cfg;
struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2];
void __iomem * const *base;
+ char config_msix_name[256];
+ struct vdpa_callback config_cb;
};

struct ifcvf_adapter {
diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
index 8d54dc5..f7baeca 100644
--- a/drivers/vdpa/ifcvf/ifcvf_main.c
+++ b/drivers/vdpa/ifcvf/ifcvf_main.c
@@ -18,6 +18,16 @@
#define DRIVER_AUTHOR "Intel Corporation"
#define IFCVF_DRIVER_NAME "ifcvf"

+static irqreturn_t ifcvf_config_changed(int irq, void *arg)
+{
+ struct ifcvf_hw *vf = arg;
+
+ if (vf->config_cb.callback)
+ return vf->config_cb.callback(vf->config_cb.private);
+
+ return IRQ_HANDLED;
+}
+
static irqreturn_t ifcvf_intr_handler(int irq, void *arg)
{
struct vring_info *vring = arg;
@@ -256,7 +266,10 @@ static void ifcvf_vdpa_set_config(struct vdpa_device *vdpa_dev,
static void ifcvf_vdpa_set_config_cb(struct vdpa_device *vdpa_dev,
struct vdpa_callback *cb)
{
- /* We don't support config interrupt */
+ struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev);
+
+ vf->config_cb.callback = cb->callback;
+ vf->config_cb.private = cb->private;
}

/*
@@ -292,6 +305,13 @@ static int ifcvf_request_irq(struct ifcvf_adapter *adapter)
struct ifcvf_hw *vf = &adapter->vf;
int vector, i, ret, irq;

+ snprintf(vf->config_msix_name, 256, "ifcvf[%s]-config\n",
+ pci_name(pdev));
+ vector = 0;
+ irq = pci_irq_vector(pdev, vector);
+ ret = devm_request_irq(&pdev->dev, irq,
+ ifcvf_config_changed, 0,
+ vf->config_msix_name, vf);

for (i = 0; i < IFCVF_MAX_QUEUE_PAIRS * 2; i++) {
snprintf(vf->vring[i].msix_name, 256, "ifcvf[%s]-%d\n",
--
1.8.3.1

2020-04-26 03:09:13

by Jason Wang

[permalink] [raw]
Subject: Re: [PATCH 1/2] vdpa: Support config interrupt in vhost_vdpa


On 2020/4/24 下午6:04, Zhu Lingshan wrote:
> This commit implements config interrupt support in
> vhost_vdpa layer.
>
> Signed-off-by: Zhu Lingshan <[email protected]>
>
> Signed-off-by: Zhu Lingshan <[email protected]>


One should be sufficient.


> ---
> drivers/vhost/vdpa.c | 53 ++++++++++++++++++++++++++++++++++++++++
> include/uapi/linux/vhost.h | 2 ++
> include/uapi/linux/vhost_types.h | 2 ++
> 3 files changed, 57 insertions(+)
>
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index 421f02a..f1f69bf 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -21,6 +21,7 @@
> #include <linux/nospec.h>
> #include <linux/vhost.h>
> #include <linux/virtio_net.h>
> +#include <linux/kernel.h>
>
> #include "vhost.h"
>
> @@ -70,6 +71,7 @@ struct vhost_vdpa {
> int nvqs;
> int virtio_id;
> int minor;
> + struct eventfd_ctx *config_ctx;
> };
>
> static DEFINE_IDA(vhost_vdpa_ida);
> @@ -101,6 +103,17 @@ static irqreturn_t vhost_vdpa_virtqueue_cb(void *private)
> return IRQ_HANDLED;
> }
>
> +static irqreturn_t vhost_vdpa_config_cb(void *private)
> +{
> + struct vhost_vdpa *v = private;
> + struct eventfd_ctx *config_ctx = v->config_ctx;
> +
> + if (config_ctx)
> + eventfd_signal(config_ctx, 1);
> +
> + return IRQ_HANDLED;
> +}
> +
> static void vhost_vdpa_reset(struct vhost_vdpa *v)
> {
> struct vdpa_device *vdpa = v->vdpa;
> @@ -288,6 +301,42 @@ static long vhost_vdpa_get_vring_num(struct vhost_vdpa *v, u16 __user *argp)
> return 0;
> }
>
> +static void vhost_vdpa_config_put(struct vhost_vdpa *v)
> +{
> + if (v->config_ctx)
> + eventfd_ctx_put(v->config_ctx);
> +}
> +
> +static long vhost_vdpa_set_config_call(struct vhost_vdpa *v, u32 __user *argp)
> +{
> + struct vdpa_callback cb;
> + vhost_config_file file;
> + struct eventfd_ctx *ctx;
> +
> + cb.callback = vhost_vdpa_config_cb;
> + cb.private = v->vdpa;
> + if (copy_from_user(&file, argp, sizeof(file)))
> + return -EFAULT;
> +
> + if (file.fd == -1) {
> + vhost_vdpa_config_put(v);
> + v->config_ctx = NULL;
> + return PTR_ERR(v->config_ctx);
> + }
> +
> + ctx = eventfd_ctx_fdget(file.fd);


You may simply did ctx = f.fd == -1 ? NULL : eventfd_ctx_fdget(f.fd);

Then there's no need for the specialized action for -1 above.


> + swap(ctx, v->config_ctx);
> +
> + if (!IS_ERR_OR_NULL(ctx))
> + eventfd_ctx_put(ctx);
> +
> + if (IS_ERR(v->config_ctx))
> + return PTR_ERR(v->config_ctx);
> +
> + v->vdpa->config->set_config_cb(v->vdpa, &cb);
> +
> + return 0;
> +}
> static long vhost_vdpa_vring_ioctl(struct vhost_vdpa *v, unsigned int cmd,
> void __user *argp)
> {
> @@ -398,6 +447,9 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
> case VHOST_SET_LOG_FD:
> r = -ENOIOCTLCMD;
> break;
> + case VHOST_VDPA_SET_CONFIG_CALL:
> + r = vhost_vdpa_set_config_call(v, argp);
> + break;
> default:
> r = vhost_dev_ioctl(&v->vdev, cmd, argp);
> if (r == -ENOIOCTLCMD)
> @@ -734,6 +786,7 @@ static int vhost_vdpa_release(struct inode *inode, struct file *filep)
> vhost_dev_stop(&v->vdev);
> vhost_vdpa_iotlb_free(v);
> vhost_vdpa_free_domain(v);
> + vhost_vdpa_config_put(v);
> vhost_dev_cleanup(&v->vdev);
> kfree(v->vdev.vqs);
> mutex_unlock(&d->mutex);
> diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> index 9fe72e4..c474a35 100644
> --- a/include/uapi/linux/vhost.h
> +++ b/include/uapi/linux/vhost.h
> @@ -140,4 +140,6 @@
> /* Get the max ring size. */
> #define VHOST_VDPA_GET_VRING_NUM _IOR(VHOST_VIRTIO, 0x76, __u16)
>
> +/* Set event fd for config interrupt*/
> +#define VHOST_VDPA_SET_CONFIG_CALL _IOW(VHOST_VIRTIO, 0x77, vhost_config_file)
> #endif
> diff --git a/include/uapi/linux/vhost_types.h b/include/uapi/linux/vhost_types.h
> index 669457c..6759aefb 100644
> --- a/include/uapi/linux/vhost_types.h
> +++ b/include/uapi/linux/vhost_types.h
> @@ -27,6 +27,8 @@ struct vhost_vring_file {
>
> };
>
> +typedef struct vhost_vring_file vhost_config_file;
> +


I wonder maybe this is the best approach. Maybe it's better to use
vhost_vring_file or just use a int (but need document the -1 action).

Thanks


> struct vhost_vring_addr {
> unsigned int index;
> /* Option flags. */

2020-04-26 03:30:42

by Jason Wang

[permalink] [raw]
Subject: Re: [PATCH 2/2] vdpa: implement config interrupt in IFCVF


On 2020/4/24 下午6:04, Zhu Lingshan wrote:
> This commit implements config interrupt support
> in IFC VF
>
> Signed-off-by: Zhu Lingshan <[email protected]>
> ---
> drivers/vdpa/ifcvf/ifcvf_base.c | 3 +++
> drivers/vdpa/ifcvf/ifcvf_base.h | 2 ++
> drivers/vdpa/ifcvf/ifcvf_main.c | 22 +++++++++++++++++++++-
> 3 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.c b/drivers/vdpa/ifcvf/ifcvf_base.c
> index b61b06e..c825d99 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_base.c
> +++ b/drivers/vdpa/ifcvf/ifcvf_base.c
> @@ -185,6 +185,9 @@ void ifcvf_set_status(struct ifcvf_hw *hw, u8 status)
>
> void ifcvf_reset(struct ifcvf_hw *hw)
> {
> + hw->config_cb.callback = NULL;
> + hw->config_cb.private = NULL;
> +
> ifcvf_set_status(hw, 0);
> /* flush set_status, make sure VF is stopped, reset */
> ifcvf_get_status(hw);
> diff --git a/drivers/vdpa/ifcvf/ifcvf_base.h b/drivers/vdpa/ifcvf/ifcvf_base.h
> index e803070..76928b0 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_base.h
> +++ b/drivers/vdpa/ifcvf/ifcvf_base.h
> @@ -81,6 +81,8 @@ struct ifcvf_hw {
> void __iomem *net_cfg;
> struct vring_info vring[IFCVF_MAX_QUEUE_PAIRS * 2];
> void __iomem * const *base;
> + char config_msix_name[256];
> + struct vdpa_callback config_cb;
> };
>
> struct ifcvf_adapter {
> diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
> index 8d54dc5..f7baeca 100644
> --- a/drivers/vdpa/ifcvf/ifcvf_main.c
> +++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> @@ -18,6 +18,16 @@
> #define DRIVER_AUTHOR "Intel Corporation"
> #define IFCVF_DRIVER_NAME "ifcvf"
>
> +static irqreturn_t ifcvf_config_changed(int irq, void *arg)
> +{
> + struct ifcvf_hw *vf = arg;
> +
> + if (vf->config_cb.callback)
> + return vf->config_cb.callback(vf->config_cb.private);
> +
> + return IRQ_HANDLED;


So it looks to me the current support of VIRTIO_NET_F_STATUS is broken
without this patch.

We probably need to patch to disable it.

Thanks


> +}
> +
> static irqreturn_t ifcvf_intr_handler(int irq, void *arg)
> {
> struct vring_info *vring = arg;
> @@ -256,7 +266,10 @@ static void ifcvf_vdpa_set_config(struct vdpa_device *vdpa_dev,
> static void ifcvf_vdpa_set_config_cb(struct vdpa_device *vdpa_dev,
> struct vdpa_callback *cb)
> {
> - /* We don't support config interrupt */
> + struct ifcvf_hw *vf = vdpa_to_vf(vdpa_dev);
> +
> + vf->config_cb.callback = cb->callback;
> + vf->config_cb.private = cb->private;
> }
>
> /*
> @@ -292,6 +305,13 @@ static int ifcvf_request_irq(struct ifcvf_adapter *adapter)
> struct ifcvf_hw *vf = &adapter->vf;
> int vector, i, ret, irq;
>
> + snprintf(vf->config_msix_name, 256, "ifcvf[%s]-config\n",
> + pci_name(pdev));
> + vector = 0;
> + irq = pci_irq_vector(pdev, vector);
> + ret = devm_request_irq(&pdev->dev, irq,
> + ifcvf_config_changed, 0,
> + vf->config_msix_name, vf);
>
> for (i = 0; i < IFCVF_MAX_QUEUE_PAIRS * 2; i++) {
> snprintf(vf->vring[i].msix_name, 256, "ifcvf[%s]-%d\n",