Re: [PATCH v9 16/17] vduse: Introduce VDUSE - vDPA Device in Userspace
在 2021/7/15 下午12:03, Yongji Xie 写道: Which ioctl can be used for this? I mean we can introduce a new ioctl for that in the future. Ok, I see. I wonder if it's better to do something similar to ccw: 1) requires the userspace to update the status bit in the response 2) update the dev->status to the status in the response if no timeout Then userspace could then set NEEDS_RESET if necessary. But NEEDS_RESET does not only happen in this case. Yes, so you need an ioctl for userspace to update the device status (NEEDS_RESET) probably. Yes, but I'd like to do that after the initial version is merged since NEEDS_RESET is not supported now. Right. Thanks ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v9 16/17] vduse: Introduce VDUSE - vDPA Device in Userspace
在 2021/7/14 下午2:49, Yongji Xie 写道:
On Wed, Jul 14, 2021 at 1:45 PM Jason Wang wrote:
在 2021/7/13 下午4:46, Xie Yongji 写道:
This VDUSE driver enables implementing software-emulated vDPA
devices in userspace. The vDPA device is created by
ioctl(VDUSE_CREATE_DEV) on /dev/vduse/control. Then a char device
interface (/dev/vduse/$NAME) is exported to userspace for device
emulation.
In order to make the device emulation more secure, the device's
control path is handled in kernel. A message mechnism is introduced
to forward some dataplane related control messages to userspace.
And in the data path, the DMA buffer will be mapped into userspace
address space through different ways depending on the vDPA bus to
which the vDPA device is attached. In virtio-vdpa case, the MMU-based
IOMMU driver is used to achieve that. And in vhost-vdpa case, the
DMA buffer is reside in a userspace memory region which can be shared
to the VDUSE userspace processs via transferring the shmfd.
For more details on VDUSE design and usage, please see the follow-on
Documentation commit.
Signed-off-by: Xie Yongji
---
Documentation/userspace-api/ioctl/ioctl-number.rst |1 +
drivers/vdpa/Kconfig | 10 +
drivers/vdpa/Makefile |1 +
drivers/vdpa/vdpa_user/Makefile|5 +
drivers/vdpa/vdpa_user/vduse_dev.c | 1502
include/uapi/linux/vduse.h | 221 +++
6 files changed, 1740 insertions(+)
create mode 100644 drivers/vdpa/vdpa_user/Makefile
create mode 100644 drivers/vdpa/vdpa_user/vduse_dev.c
create mode 100644 include/uapi/linux/vduse.h
diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst
b/Documentation/userspace-api/ioctl/ioctl-number.rst
index 1409e40e6345..293ca3aef358 100644
--- a/Documentation/userspace-api/ioctl/ioctl-number.rst
+++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
@@ -300,6 +300,7 @@ Code Seq#Include File
Comments
'z' 10-4F drivers/s390/crypto/zcrypt_api.h
conflict!
'|' 00-7F linux/media.h
0x80 00-1F linux/fb.h
+0x81 00-1F linux/vduse.h
0x89 00-06 arch/x86/include/asm/sockios.h
0x89 0B-DF linux/sockios.h
0x89 E0-EF linux/sockios.h
SIOCPROTOPRIVATE range
diff --git a/drivers/vdpa/Kconfig b/drivers/vdpa/Kconfig
index a503c1b2bfd9..6e23bce6433a 100644
--- a/drivers/vdpa/Kconfig
+++ b/drivers/vdpa/Kconfig
@@ -33,6 +33,16 @@ config VDPA_SIM_BLOCK
vDPA block device simulator which terminates IO request in a
memory buffer.
+config VDPA_USER
+ tristate "VDUSE (vDPA Device in Userspace) support"
+ depends on EVENTFD && MMU && HAS_DMA
+ select DMA_OPS
+ select VHOST_IOTLB
+ select IOMMU_IOVA
+ help
+ With VDUSE it is possible to emulate a vDPA Device
+ in a userspace program.
+
config IFCVF
tristate "Intel IFC VF vDPA driver"
depends on PCI_MSI
diff --git a/drivers/vdpa/Makefile b/drivers/vdpa/Makefile
index 67fe7f3d6943..f02ebed33f19 100644
--- a/drivers/vdpa/Makefile
+++ b/drivers/vdpa/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_VDPA) += vdpa.o
obj-$(CONFIG_VDPA_SIM) += vdpa_sim/
+obj-$(CONFIG_VDPA_USER) += vdpa_user/
obj-$(CONFIG_IFCVF)+= ifcvf/
obj-$(CONFIG_MLX5_VDPA) += mlx5/
obj-$(CONFIG_VP_VDPA)+= virtio_pci/
diff --git a/drivers/vdpa/vdpa_user/Makefile b/drivers/vdpa/vdpa_user/Makefile
new file mode 100644
index ..260e0b26af99
--- /dev/null
+++ b/drivers/vdpa/vdpa_user/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+
+vduse-y := vduse_dev.o iova_domain.o
+
+obj-$(CONFIG_VDPA_USER) += vduse.o
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
b/drivers/vdpa/vdpa_user/vduse_dev.c
new file mode 100644
index ..c994a4a4660c
--- /dev/null
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -0,0 +1,1502 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * VDUSE: vDPA Device in Userspace
+ *
+ * Copyright (C) 2020-2021 Bytedance Inc. and/or its affiliates. All rights
reserved.
+ *
+ * Author: Xie Yongji
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "iova_domain.h"
+
+#define DRV_AUTHOR "Yongji Xie "
+#define DRV_DESC "vDPA Device in Userspace"
+#define DRV_LICENSE "GPL v2"
+
+#define VDUSE_DEV_MAX (1U << MINORBITS)
+#define VDUSE_MAX_BOUNCE_SIZE (64 * 1024 * 1024)
+#define VDUSE_IOVA_SIZE (128 * 1024 * 1024)
+#define VDUSE_REQUEST_TIMEOUT 30
+
+struct vduse_virtqueue {
+ u16 index;
+ u16 num_max;
+ u32 num;
+ u64 desc_addr;
+ u64 driver_addr;
+ u64 device_addr;
+ struct vdpa_vq_state state;
+ bool ready;
+ bool
Re: [PATCH v9 16/17] vduse: Introduce VDUSE - vDPA Device in Userspace
在 2021/7/14 下午2:47, Greg KH 写道:
On Wed, Jul 14, 2021 at 02:02:50PM +0800, Jason Wang wrote:
在 2021/7/14 下午1:54, Michael S. Tsirkin 写道:
On Wed, Jul 14, 2021 at 01:45:39PM +0800, Jason Wang wrote:
+static int vduse_dev_msg_sync(struct vduse_dev *dev,
+ struct vduse_dev_msg *msg)
+{
+ int ret;
+
+ init_waitqueue_head(>waitq);
+ spin_lock(>msg_lock);
+ msg->req.request_id = dev->msg_unique++;
+ vduse_enqueue_msg(>send_list, msg);
+ wake_up(>waitq);
+ spin_unlock(>msg_lock);
+
+ wait_event_killable_timeout(msg->waitq, msg->completed,
+ VDUSE_REQUEST_TIMEOUT * HZ);
+ spin_lock(>msg_lock);
+ if (!msg->completed) {
+ list_del(>list);
+ msg->resp.result = VDUSE_REQ_RESULT_FAILED;
+ }
+ ret = (msg->resp.result == VDUSE_REQ_RESULT_OK) ? 0 : -EIO;
I think we should mark the device as malfunction when there is a timeout and
forbid any userspace operations except for the destroy aftwards for safety.
This looks like if one tried to run gdb on the program the behaviour
will change completely because kernel wants it to respond within
specific time. Looks like a receipe for heisenbugs.
Let's not build interfaces with arbitrary timeouts like that.
Interruptible wait exists for this very reason.
The problem is. Do we want userspace program like modprobe to be stuck for
indefinite time and expect the administrator to kill that?
Why would modprobe be stuck for forever?
Is this on the module probe path?
Yes, it is called in the device probing path where the kernel forwards
the device configuration request to userspace and wait for its response.
If it turns out to be tricky, we can implement the whole device inside
the kernel and leave only the datapath in the userspace (as what TUN did).
Thanks
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v9 16/17] vduse: Introduce VDUSE - vDPA Device in Userspace
On Wed, Jul 14, 2021 at 02:02:50PM +0800, Jason Wang wrote:
>
> 在 2021/7/14 下午1:54, Michael S. Tsirkin 写道:
> > On Wed, Jul 14, 2021 at 01:45:39PM +0800, Jason Wang wrote:
> > > > +static int vduse_dev_msg_sync(struct vduse_dev *dev,
> > > > + struct vduse_dev_msg *msg)
> > > > +{
> > > > + int ret;
> > > > +
> > > > + init_waitqueue_head(>waitq);
> > > > + spin_lock(>msg_lock);
> > > > + msg->req.request_id = dev->msg_unique++;
> > > > + vduse_enqueue_msg(>send_list, msg);
> > > > + wake_up(>waitq);
> > > > + spin_unlock(>msg_lock);
> > > > +
> > > > + wait_event_killable_timeout(msg->waitq, msg->completed,
> > > > + VDUSE_REQUEST_TIMEOUT * HZ);
> > > > + spin_lock(>msg_lock);
> > > > + if (!msg->completed) {
> > > > + list_del(>list);
> > > > + msg->resp.result = VDUSE_REQ_RESULT_FAILED;
> > > > + }
> > > > + ret = (msg->resp.result == VDUSE_REQ_RESULT_OK) ? 0 : -EIO;
> > >
> > > I think we should mark the device as malfunction when there is a timeout
> > > and
> > > forbid any userspace operations except for the destroy aftwards for
> > > safety.
> > This looks like if one tried to run gdb on the program the behaviour
> > will change completely because kernel wants it to respond within
> > specific time. Looks like a receipe for heisenbugs.
> >
> > Let's not build interfaces with arbitrary timeouts like that.
> > Interruptible wait exists for this very reason.
>
>
> The problem is. Do we want userspace program like modprobe to be stuck for
> indefinite time and expect the administrator to kill that?
Why would modprobe be stuck for forever?
Is this on the module probe path?
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v9 16/17] vduse: Introduce VDUSE - vDPA Device in Userspace
在 2021/7/14 下午1:54, Michael S. Tsirkin 写道:
On Wed, Jul 14, 2021 at 01:45:39PM +0800, Jason Wang wrote:
+static int vduse_dev_msg_sync(struct vduse_dev *dev,
+ struct vduse_dev_msg *msg)
+{
+ int ret;
+
+ init_waitqueue_head(>waitq);
+ spin_lock(>msg_lock);
+ msg->req.request_id = dev->msg_unique++;
+ vduse_enqueue_msg(>send_list, msg);
+ wake_up(>waitq);
+ spin_unlock(>msg_lock);
+
+ wait_event_killable_timeout(msg->waitq, msg->completed,
+ VDUSE_REQUEST_TIMEOUT * HZ);
+ spin_lock(>msg_lock);
+ if (!msg->completed) {
+ list_del(>list);
+ msg->resp.result = VDUSE_REQ_RESULT_FAILED;
+ }
+ ret = (msg->resp.result == VDUSE_REQ_RESULT_OK) ? 0 : -EIO;
I think we should mark the device as malfunction when there is a timeout and
forbid any userspace operations except for the destroy aftwards for safety.
This looks like if one tried to run gdb on the program the behaviour
will change completely because kernel wants it to respond within
specific time. Looks like a receipe for heisenbugs.
Let's not build interfaces with arbitrary timeouts like that.
Interruptible wait exists for this very reason.
The problem is. Do we want userspace program like modprobe to be stuck
for indefinite time and expect the administrator to kill that?
Thanks
Let userspace have its
own code to set and use timers. This does mean that userspace will
likely have to change a bit to support this driver, such is life.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v9 16/17] vduse: Introduce VDUSE - vDPA Device in Userspace
On Wed, Jul 14, 2021 at 01:45:39PM +0800, Jason Wang wrote:
> > +static int vduse_dev_msg_sync(struct vduse_dev *dev,
> > + struct vduse_dev_msg *msg)
> > +{
> > + int ret;
> > +
> > + init_waitqueue_head(>waitq);
> > + spin_lock(>msg_lock);
> > + msg->req.request_id = dev->msg_unique++;
> > + vduse_enqueue_msg(>send_list, msg);
> > + wake_up(>waitq);
> > + spin_unlock(>msg_lock);
> > +
> > + wait_event_killable_timeout(msg->waitq, msg->completed,
> > + VDUSE_REQUEST_TIMEOUT * HZ);
> > + spin_lock(>msg_lock);
> > + if (!msg->completed) {
> > + list_del(>list);
> > + msg->resp.result = VDUSE_REQ_RESULT_FAILED;
> > + }
> > + ret = (msg->resp.result == VDUSE_REQ_RESULT_OK) ? 0 : -EIO;
>
>
> I think we should mark the device as malfunction when there is a timeout and
> forbid any userspace operations except for the destroy aftwards for safety.
This looks like if one tried to run gdb on the program the behaviour
will change completely because kernel wants it to respond within
specific time. Looks like a receipe for heisenbugs.
Let's not build interfaces with arbitrary timeouts like that.
Interruptible wait exists for this very reason. Let userspace have its
own code to set and use timers. This does mean that userspace will
likely have to change a bit to support this driver, such is life.
--
MST
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v9 16/17] vduse: Introduce VDUSE - vDPA Device in Userspace
在 2021/7/13 下午4:46, Xie Yongji 写道:
This VDUSE driver enables implementing software-emulated vDPA
devices in userspace. The vDPA device is created by
ioctl(VDUSE_CREATE_DEV) on /dev/vduse/control. Then a char device
interface (/dev/vduse/$NAME) is exported to userspace for device
emulation.
In order to make the device emulation more secure, the device's
control path is handled in kernel. A message mechnism is introduced
to forward some dataplane related control messages to userspace.
And in the data path, the DMA buffer will be mapped into userspace
address space through different ways depending on the vDPA bus to
which the vDPA device is attached. In virtio-vdpa case, the MMU-based
IOMMU driver is used to achieve that. And in vhost-vdpa case, the
DMA buffer is reside in a userspace memory region which can be shared
to the VDUSE userspace processs via transferring the shmfd.
For more details on VDUSE design and usage, please see the follow-on
Documentation commit.
Signed-off-by: Xie Yongji
---
Documentation/userspace-api/ioctl/ioctl-number.rst |1 +
drivers/vdpa/Kconfig | 10 +
drivers/vdpa/Makefile |1 +
drivers/vdpa/vdpa_user/Makefile|5 +
drivers/vdpa/vdpa_user/vduse_dev.c | 1502
include/uapi/linux/vduse.h | 221 +++
6 files changed, 1740 insertions(+)
create mode 100644 drivers/vdpa/vdpa_user/Makefile
create mode 100644 drivers/vdpa/vdpa_user/vduse_dev.c
create mode 100644 include/uapi/linux/vduse.h
diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst
b/Documentation/userspace-api/ioctl/ioctl-number.rst
index 1409e40e6345..293ca3aef358 100644
--- a/Documentation/userspace-api/ioctl/ioctl-number.rst
+++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
@@ -300,6 +300,7 @@ Code Seq#Include File
Comments
'z' 10-4F drivers/s390/crypto/zcrypt_api.hconflict!
'|' 00-7F linux/media.h
0x80 00-1F linux/fb.h
+0x81 00-1F linux/vduse.h
0x89 00-06 arch/x86/include/asm/sockios.h
0x89 0B-DF linux/sockios.h
0x89 E0-EF linux/sockios.h
SIOCPROTOPRIVATE range
diff --git a/drivers/vdpa/Kconfig b/drivers/vdpa/Kconfig
index a503c1b2bfd9..6e23bce6433a 100644
--- a/drivers/vdpa/Kconfig
+++ b/drivers/vdpa/Kconfig
@@ -33,6 +33,16 @@ config VDPA_SIM_BLOCK
vDPA block device simulator which terminates IO request in a
memory buffer.
+config VDPA_USER
+ tristate "VDUSE (vDPA Device in Userspace) support"
+ depends on EVENTFD && MMU && HAS_DMA
+ select DMA_OPS
+ select VHOST_IOTLB
+ select IOMMU_IOVA
+ help
+ With VDUSE it is possible to emulate a vDPA Device
+ in a userspace program.
+
config IFCVF
tristate "Intel IFC VF vDPA driver"
depends on PCI_MSI
diff --git a/drivers/vdpa/Makefile b/drivers/vdpa/Makefile
index 67fe7f3d6943..f02ebed33f19 100644
--- a/drivers/vdpa/Makefile
+++ b/drivers/vdpa/Makefile
@@ -1,6 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_VDPA) += vdpa.o
obj-$(CONFIG_VDPA_SIM) += vdpa_sim/
+obj-$(CONFIG_VDPA_USER) += vdpa_user/
obj-$(CONFIG_IFCVF)+= ifcvf/
obj-$(CONFIG_MLX5_VDPA) += mlx5/
obj-$(CONFIG_VP_VDPA)+= virtio_pci/
diff --git a/drivers/vdpa/vdpa_user/Makefile b/drivers/vdpa/vdpa_user/Makefile
new file mode 100644
index ..260e0b26af99
--- /dev/null
+++ b/drivers/vdpa/vdpa_user/Makefile
@@ -0,0 +1,5 @@
+# SPDX-License-Identifier: GPL-2.0
+
+vduse-y := vduse_dev.o iova_domain.o
+
+obj-$(CONFIG_VDPA_USER) += vduse.o
diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c
b/drivers/vdpa/vdpa_user/vduse_dev.c
new file mode 100644
index ..c994a4a4660c
--- /dev/null
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -0,0 +1,1502 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * VDUSE: vDPA Device in Userspace
+ *
+ * Copyright (C) 2020-2021 Bytedance Inc. and/or its affiliates. All rights
reserved.
+ *
+ * Author: Xie Yongji
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "iova_domain.h"
+
+#define DRV_AUTHOR "Yongji Xie "
+#define DRV_DESC "vDPA Device in Userspace"
+#define DRV_LICENSE "GPL v2"
+
+#define VDUSE_DEV_MAX (1U << MINORBITS)
+#define VDUSE_MAX_BOUNCE_SIZE (64 * 1024 * 1024)
+#define VDUSE_IOVA_SIZE (128 * 1024 * 1024)
+#define VDUSE_REQUEST_TIMEOUT 30
+
+struct vduse_virtqueue {
+ u16 index;
+ u16 num_max;
+ u32 num;
+ u64 desc_addr;
+ u64 driver_addr;
+ u64 device_addr;
+ struct vdpa_vq_state state;
+ bool ready;
+ bool kicked;
+ spinlock_t kick_lock;
+ spinlock_t irq_lock;
+
Re: [PATCH v9 16/17] vduse: Introduce VDUSE - vDPA Device in Userspace
在 2021/7/13 下午9:27, Dan Carpenter 写道:
On Tue, Jul 13, 2021 at 04:46:55PM +0800, Xie Yongji wrote:
+static int vduse_dev_init_vdpa(struct vduse_dev *dev, const char *name)
+{
+ struct vduse_vdpa *vdev;
+ int ret;
+
+ if (dev->vdev)
+ return -EEXIST;
+
+ vdev = vdpa_alloc_device(struct vduse_vdpa, vdpa, dev->dev,
+_vdpa_config_ops, name, true);
+ if (!vdev)
+ return -ENOMEM;
This should be an IS_ERR() check instead of a NULL check.
Yes.
The vdpa_alloc_device() macro is doing something very complicated but
I'm not sure what. It calls container_of() and that looks buggy until
you spot the BUILD_BUG_ON_ZERO() compile time assert which ensures that
the container_of() is a no-op.
Only one of the callers checks for error pointers correctly so maybe
it's too complicated or maybe there should be better documentation.
We need better documentation for this macro and fix all the buggy callers.
Yong Ji, want to do that?
Thanks
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: [PATCH v9 16/17] vduse: Introduce VDUSE - vDPA Device in Userspace
On Tue, Jul 13, 2021 at 04:46:55PM +0800, Xie Yongji wrote:
> +static int vduse_dev_init_vdpa(struct vduse_dev *dev, const char *name)
> +{
> + struct vduse_vdpa *vdev;
> + int ret;
> +
> + if (dev->vdev)
> + return -EEXIST;
> +
> + vdev = vdpa_alloc_device(struct vduse_vdpa, vdpa, dev->dev,
> + _vdpa_config_ops, name, true);
> + if (!vdev)
> + return -ENOMEM;
This should be an IS_ERR() check instead of a NULL check.
The vdpa_alloc_device() macro is doing something very complicated but
I'm not sure what. It calls container_of() and that looks buggy until
you spot the BUILD_BUG_ON_ZERO() compile time assert which ensures that
the container_of() is a no-op.
Only one of the callers checks for error pointers correctly so maybe
it's too complicated or maybe there should be better documentation.
regards,
dan carpenter
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
