2020-04-26 06:47:47

by Zhu, Lingshan

[permalink] [raw]
Subject: [PATCH V2 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.

changes from V1:
vdpa: more efficient code to handle eventfd unbind.
ifcvf: add VIRTIO_NET_F_STATUS feature bit.

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 | 3 +++
drivers/vdpa/ifcvf/ifcvf_main.c | 22 ++++++++++++++++++-
drivers/vhost/vdpa.c | 47 +++++++++++++++++++++++++++++++++++++++++
drivers/vhost/vhost.c | 2 +-
drivers/vhost/vhost.h | 2 ++
include/uapi/linux/vhost.h | 2 ++
7 files changed, 79 insertions(+), 2 deletions(-)

--
1.8.3.1


2020-04-26 06:59:43

by Zhu, Lingshan

[permalink] [raw]
Subject: [PATCH V2 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 | 3 +++
drivers/vdpa/ifcvf/ifcvf_main.c | 22 +++++++++++++++++++++-
3 files changed, 27 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..23ac47d 100644
--- a/drivers/vdpa/ifcvf/ifcvf_base.h
+++ b/drivers/vdpa/ifcvf/ifcvf_base.h
@@ -27,6 +27,7 @@
((1ULL << VIRTIO_NET_F_MAC) | \
(1ULL << VIRTIO_F_ANY_LAYOUT) | \
(1ULL << VIRTIO_F_VERSION_1) | \
+ (1ULL << VIRTIO_NET_F_STATUS) | \
(1ULL << VIRTIO_F_ORDER_PLATFORM) | \
(1ULL << VIRTIO_F_IOMMU_PLATFORM) | \
(1ULL << VIRTIO_NET_F_MRG_RXBUF))
@@ -81,6 +82,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 07:01:50

by Zhu, Lingshan

[permalink] [raw]
Subject: [PATCH V2 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]>
---
drivers/vhost/vdpa.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++
drivers/vhost/vhost.c | 2 +-
drivers/vhost/vhost.h | 2 ++
include/uapi/linux/vhost.h | 2 ++
4 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 421f02a..b94e349 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,36 @@ 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;
+ u32 fd;
+ struct eventfd_ctx *ctx;
+
+ cb.callback = vhost_vdpa_config_cb;
+ cb.private = v->vdpa;
+ if (copy_from_user(&fd, argp, sizeof(fd)))
+ return -EFAULT;
+
+ ctx = fd == VHOST_FILE_UNBIND ? NULL : eventfd_ctx_fdget(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 +441,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 +780,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/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index d450e16..e8f5b20 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1590,7 +1590,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *arg
r = -EFAULT;
break;
}
- ctx = f.fd == -1 ? NULL : eventfd_ctx_fdget(f.fd);
+ ctx = f.fd == VHOST_FILE_UNBIND ? NULL : eventfd_ctx_fdget(f.fd);
if (IS_ERR(ctx)) {
r = PTR_ERR(ctx);
break;
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 1813821..8663139 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -18,6 +18,8 @@
typedef void (*vhost_work_fn_t)(struct vhost_work *work);

#define VHOST_WORK_QUEUED 1
+#define VHOST_FILE_UNBIND -1
+
struct vhost_work {
struct llist_node node;
vhost_work_fn_t fn;
diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
index 9fe72e4..345acb3 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, u32)
#endif
--
1.8.3.1

2020-04-26 07:27:35

by Jason Wang

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


On 2020/4/26 下午2:58, Jason Wang wrote:
>>
>> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
>> index 1813821..8663139 100644
>> --- a/drivers/vhost/vhost.h
>> +++ b/drivers/vhost/vhost.h
>> @@ -18,6 +18,8 @@
>>   typedef void (*vhost_work_fn_t)(struct vhost_work *work);
>>     #define VHOST_WORK_QUEUED 1
>> +#define VHOST_FILE_UNBIND -1
>
>
> I think it's better to document this in uapi.


I meant e.g in vhost_vring_file, we had a comment of unbinding:

struct vhost_vring_file {
    unsigned int index;
    int fd; /* Pass -1 to unbind from file. */

};

Thanks

2020-04-26 07:28:01

by Jason Wang

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


On 2020/4/26 下午2:09, Zhu Lingshan wrote:
> This commit implements config interrupt support in
> vhost_vdpa layer.
>
> Signed-off-by: Zhu Lingshan <[email protected]>
> ---
> drivers/vhost/vdpa.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++
> drivers/vhost/vhost.c | 2 +-
> drivers/vhost/vhost.h | 2 ++
> include/uapi/linux/vhost.h | 2 ++
> 4 files changed, 52 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index 421f02a..b94e349 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,36 @@ 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;
> + u32 fd;
> + struct eventfd_ctx *ctx;
> +
> + cb.callback = vhost_vdpa_config_cb;
> + cb.private = v->vdpa;
> + if (copy_from_user(&fd, argp, sizeof(fd)))
> + return -EFAULT;
> +
> + ctx = fd == VHOST_FILE_UNBIND ? NULL : eventfd_ctx_fdget(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 +441,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 +780,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/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index d450e16..e8f5b20 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -1590,7 +1590,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *arg
> r = -EFAULT;
> break;
> }
> - ctx = f.fd == -1 ? NULL : eventfd_ctx_fdget(f.fd);
> + ctx = f.fd == VHOST_FILE_UNBIND ? NULL : eventfd_ctx_fdget(f.fd);
> if (IS_ERR(ctx)) {
> r = PTR_ERR(ctx);
> break;
> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
> index 1813821..8663139 100644
> --- a/drivers/vhost/vhost.h
> +++ b/drivers/vhost/vhost.h
> @@ -18,6 +18,8 @@
> typedef void (*vhost_work_fn_t)(struct vhost_work *work);
>
> #define VHOST_WORK_QUEUED 1
> +#define VHOST_FILE_UNBIND -1


I think it's better to document this in uapi.


> +
> struct vhost_work {
> struct llist_node node;
> vhost_work_fn_t fn;
> diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> index 9fe72e4..345acb3 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, u32)
> #endif


Should be "int" instead of "u32".

Thanks

2020-04-26 07:42:05

by Jason Wang

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


On 2020/4/26 下午3:24, Zhu Lingshan wrote:
>
>
> On 4/26/2020 3:03 PM, Jason Wang wrote:
>>
>> On 2020/4/26 下午2:58, Jason Wang wrote:
>>>>
>>>> diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
>>>> index 1813821..8663139 100644
>>>> --- a/drivers/vhost/vhost.h
>>>> +++ b/drivers/vhost/vhost.h
>>>> @@ -18,6 +18,8 @@
>>>>   typedef void (*vhost_work_fn_t)(struct vhost_work *work);
>>>>     #define VHOST_WORK_QUEUED 1
>>>> +#define VHOST_FILE_UNBIND -1
>>>
>>>
>>> I think it's better to document this in uapi.
>>
>>
>> I meant e.g in vhost_vring_file, we had a comment of unbinding:
>>
>> struct vhost_vring_file {
>>     unsigned int index;
>>     int fd; /* Pass -1 to unbind from file. */
>>
>> };
> I think it is better to use an int fd than vhost_vring_file, to avoid the confusions,
> so if we add#define VHOST_FILE_UNBIND -1 in the uapi header, can it document itself?
> Thanks


I think so.

Thanks


>>
>> Thanks
>>