Re: [PATCH v6 09/10] vduse: Introduce VDUSE - vDPA Device in Userspace

2021-04-08 Thread Jason Wang


在 2021/4/8 下午5:36, Yongji Xie 写道:

On Thu, Apr 8, 2021 at 2:57 PM Jason Wang  wrote:


在 2021/3/31 下午4:05, Xie Yongji 写道:

This VDUSE driver enables implementing vDPA devices in userspace.
Both control path and data path of vDPA devices will be able to
be handled in userspace.

In the control path, the VDUSE driver will make use of message
mechnism to forward the config operation from vdpa bus driver
to userspace. Userspace can use read()/write() to receive/reply
those control messages.

In the data path, VDUSE_IOTLB_GET_FD ioctl will be used to get
the file descriptors referring to vDPA device's iova regions. Then
userspace can use mmap() to access those iova regions. Besides,
userspace can use ioctl() to inject interrupt and use the eventfd
mechanism to receive virtqueue kicks.

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 | 1362 

   include/uapi/linux/vduse.h |  175 +++
   6 files changed, 1554 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 a4c75a28c839..71722e6f8f23 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 a245809c99d0..77a1da522c21 100644
--- a/drivers/vdpa/Kconfig
+++ b/drivers/vdpa/Kconfig
@@ -25,6 +25,16 @@ config VDPA_SIM_NET
   help
 vDPA networking device simulator which loops TX traffic back to RX.

+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 ..51ca73464d0d
--- /dev/null
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -0,0 +1,1362 @@
+// 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 "iova_domain.h"
+
+#define DRV_VERSION  "1.0"
+#define DRV_AUTHOR   "Yongji Xie "
+#define DRV_DESC "vDPA Device in Userspace"
+#define DRV_LICENSE  "GPL v2"
+
+#define VDUSE_DEV_MAX (1U << MINORBITS)
+
+struct vduse_virtqueue {
+ u16 index;
+ bool ready;
+ spinlock_t kick_lock;
+ spinlock_t irq_lock;
+ struct eventfd_ctx *kickfd;
+ struct vdpa_callback cb;
+ struct work_struct inject;
+};
+
+struct vduse_dev;
+
+struct vduse_vdpa {
+ struct vdpa_device vdpa;
+ struct vduse_dev *dev;
+};
+
+struct vduse_dev {
+ struct vduse_vdpa *vdev;
+ struct device dev;
+ struct cdev cdev;
+ struct vduse_virtqueue *vqs;
+ struct vduse_iova_domain *domain;
+ struct mutex lock;
+ spinlock_t msg_lock;
+ atomic64_t msg_unique;
+ wait_queue_head_t waitq;
+

Re: [PATCH v2 1/3] virtio: update reset callback to return status

2021-04-08 Thread Jason Wang


在 2021/4/8 下午10:24, Max Gurtovoy 写道:


On 4/8/2021 4:14 PM, Jason Wang wrote:


在 2021/4/8 下午5:56, Max Gurtovoy 写道:


On 4/8/2021 11:58 AM, Jason Wang wrote:


在 2021/4/8 下午4:11, Max Gurtovoy 写道:
The reset device operation, usually is an operation that might 
fail from
various reasons. For example, the controller might be in a bad 
state and

can't answer to any request. Usually, the paravirt SW based virtio
devices always succeed in reset operation but this is not the case 
for

HW based virtio devices.



I would like to know under what condition that the reset operation 
may fail (except for the case of a bugg guest).


The controller might not be ready or stuck. This is a real use case 
for many PCI devices.


For real devices the FW might be in a bad state and it can happen 
also for paravirt device if you have a bug in the controller code or 
if you entered some error flow (Out of memory).


You don't want to be stuck because of one bad device.



So the buggy driver can damage the host through various ways, I'm not 
sure doing such workaround is worthwhile.


do you mean device ?



Yes.




sometimes you need to replace device FW and it will work.

I don't think it's a workaround. Other protocols, such as NVMe, solved 
this in the specification.


PCI config space and PCI controller are sometimes 2 different 
components and sometimes the controller is not active, although the 
device is plugged and seen by the PCI subsystem.



So I think we need patch to spec to see if it works first.







Note that this driver has been used for real hardware PCI devices for 
many years. We don't receive any report of this before.











This commit is also a preparation for adding a timeout mechanism for
resetting virtio devices.

Signed-off-by: Max Gurtovoy 
---

changes from v1:
  - update virtio_ccw.c (Cornelia)
  - update virtio_uml.c
  - update mlxbf-tmfifo.c



Note that virtio driver may call reset, so you probably need to 
convert them.


I'm sure I understand.

Convert to what ?



Convert to deal with the possible rest failure. E.g in 
virtblk_freeze() we had:


static int virtblk_freeze(struct virtio_device *vdev)
{
    struct virtio_blk *vblk = vdev->priv;

    /* Ensure we don't receive any more interrupts */
    vdev->config->reset(vdev);
...

We need fail the freeze here.



Agree.




Another example is the driver remove which is not expected to be 
fail. A lot of virtio drivers tries to call reset there. I'm not sure 
how hard to deal with the failure in the path of remove (e.g 
__device_release_driver tends to ignore the return value of 
bus->remove().)


I think it can stay as-is and ignore the ->reset return value and 
continue free the other resources to avoid leakage.



The problem is that it's unclear that what kind of behaviour would the 
device do, e.g can it still send interrupts?


That's why we need to formalize the bahviour first if necessary.

Thanks







Thanks




Thanks.



Thanks




---
  arch/um/drivers/virtio_uml.c |  4 +++-
  drivers/platform/mellanox/mlxbf-tmfifo.c |  4 +++-
  drivers/remoteproc/remoteproc_virtio.c   |  4 +++-
  drivers/s390/virtio/virtio_ccw.c |  9 ++---
  drivers/virtio/virtio.c  | 22 
+++---

  drivers/virtio/virtio_mmio.c |  3 ++-
  drivers/virtio/virtio_pci_legacy.c   |  4 +++-
  drivers/virtio/virtio_pci_modern.c   |  3 ++-
  drivers/virtio/virtio_vdpa.c |  4 +++-
  include/linux/virtio_config.h    |  5 +++--
  10 files changed, 43 insertions(+), 19 deletions(-)

diff --git a/arch/um/drivers/virtio_uml.c 
b/arch/um/drivers/virtio_uml.c

index 91ddf74ca888..b6e66265ed32 100644
--- a/arch/um/drivers/virtio_uml.c
+++ b/arch/um/drivers/virtio_uml.c
@@ -827,11 +827,13 @@ static void vu_set_status(struct 
virtio_device *vdev, u8 status)

  vu_dev->status = status;
  }
  -static void vu_reset(struct virtio_device *vdev)
+static int vu_reset(struct virtio_device *vdev)
  {
  struct virtio_uml_device *vu_dev = to_virtio_uml_device(vdev);
    vu_dev->status = 0;
+
+    return 0;
  }
    static void vu_del_vq(struct virtqueue *vq)
diff --git a/drivers/platform/mellanox/mlxbf-tmfifo.c 
b/drivers/platform/mellanox/mlxbf-tmfifo.c

index bbc4e71a16ff..c192b8ac5d9e 100644
--- a/drivers/platform/mellanox/mlxbf-tmfifo.c
+++ b/drivers/platform/mellanox/mlxbf-tmfifo.c
@@ -980,11 +980,13 @@ static void 
mlxbf_tmfifo_virtio_set_status(struct virtio_device *vdev,

  }
    /* Reset the device. Not much here for now. */
-static void mlxbf_tmfifo_virtio_reset(struct virtio_device *vdev)
+static int mlxbf_tmfifo_virtio_reset(struct virtio_device *vdev)
  {
  struct mlxbf_tmfifo_vdev *tm_vdev = mlxbf_vdev_to_tmfifo(vdev);
    tm_vdev->status = 0;
+
+    return 0;
  }
    /* Read the value of a configuration field. */
diff --git a/drivers/remoteproc/remoteproc_virtio.c 
b/drivers/remoteproc/remoteproc_virtio.c

index 0cc617f76068..ca9573c62c3d 

Re: [RFC PATCH] vdpa: mandate 1.0 device

2021-04-08 Thread Jason Wang


在 2021/4/8 下午11:59, Michael S. Tsirkin 写道:

On Thu, Apr 08, 2021 at 04:26:48PM +0800, Jason Wang wrote:

This patch mandates 1.0 for vDPA devices. The goal is to have the
semantic of normative statement in the virtio spec and eliminate the
burden of transitional device for both vDPA bus and vDPA parent.

uAPI seems fine since all the vDPA parent mandates
VIRTIO_F_ACCESS_PLATFORM which implies 1.0 devices.

For legacy guests, it can still work since Qemu will mediate when
necessary (e.g doing the endian conversion).

Signed-off-by: Jason Wang 

Hmm. If we do this, don't we still have a problem with
legacy drivers which don't ack 1.0?



Yes, but it's not something that is introduced in this commit. The 
legacy driver never work ...




Note 1.0 affects ring endianness which is not mediated in QEMU
so QEMU can't pretend to device guest is 1.0.



Right, I plan to send patches to do mediation in the Qemu to unbreak 
legacy drivers.


Thanks









---
  include/linux/vdpa.h | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index 0fefeb976877..cfde4ec999b4 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -6,6 +6,7 @@
  #include 
  #include 
  #include 
+#include 
  
  /**

   * vDPA callback definition.
@@ -317,6 +318,11 @@ static inline int vdpa_set_features(struct vdpa_device 
*vdev, u64 features)
  {
  const struct vdpa_config_ops *ops = vdev->config;
  
+/* Mandating 1.0 to have semantics of normative statements in

+ * the spec. */
+if (!(features & BIT_ULL(VIRTIO_F_VERSION_1)))
+   return -EINVAL;
+
vdev->features_valid = true;
  return ops->set_features(vdev, features);
  }
--
2.25.1


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 1/3] virtio: update reset callback to return status

2021-04-08 Thread Michael S. Tsirkin
On Thu, Apr 08, 2021 at 08:11:07AM +, Max Gurtovoy wrote:
> The reset device operation, usually is an operation that might fail from
> various reasons. For example, the controller might be in a bad state and
> can't answer to any request. Usually, the paravirt SW based virtio
> devices always succeed in reset operation but this is not the case for
> HW based virtio devices.

Well it reports the error to callers however
no callers check the return value. So what good is it?
If callers are expected to just proceed then we don't
really need to return a value at all ...

> This commit is also a preparation for adding a timeout mechanism for
> resetting virtio devices.
> 
> Signed-off-by: Max Gurtovoy 
> ---
> 
> changes from v1:
>  - update virtio_ccw.c (Cornelia)
>  - update virtio_uml.c
>  - update mlxbf-tmfifo.c
> 
> ---
>  arch/um/drivers/virtio_uml.c |  4 +++-
>  drivers/platform/mellanox/mlxbf-tmfifo.c |  4 +++-
>  drivers/remoteproc/remoteproc_virtio.c   |  4 +++-
>  drivers/s390/virtio/virtio_ccw.c |  9 ++---
>  drivers/virtio/virtio.c  | 22 +++---
>  drivers/virtio/virtio_mmio.c |  3 ++-
>  drivers/virtio/virtio_pci_legacy.c   |  4 +++-
>  drivers/virtio/virtio_pci_modern.c   |  3 ++-
>  drivers/virtio/virtio_vdpa.c |  4 +++-
>  include/linux/virtio_config.h|  5 +++--
>  10 files changed, 43 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/um/drivers/virtio_uml.c b/arch/um/drivers/virtio_uml.c
> index 91ddf74ca888..b6e66265ed32 100644
> --- a/arch/um/drivers/virtio_uml.c
> +++ b/arch/um/drivers/virtio_uml.c
> @@ -827,11 +827,13 @@ static void vu_set_status(struct virtio_device *vdev, 
> u8 status)
>   vu_dev->status = status;
>  }
>  
> -static void vu_reset(struct virtio_device *vdev)
> +static int vu_reset(struct virtio_device *vdev)
>  {
>   struct virtio_uml_device *vu_dev = to_virtio_uml_device(vdev);
>  
>   vu_dev->status = 0;
> +
> + return 0;
>  }
>  
>  static void vu_del_vq(struct virtqueue *vq)
> diff --git a/drivers/platform/mellanox/mlxbf-tmfifo.c 
> b/drivers/platform/mellanox/mlxbf-tmfifo.c
> index bbc4e71a16ff..c192b8ac5d9e 100644
> --- a/drivers/platform/mellanox/mlxbf-tmfifo.c
> +++ b/drivers/platform/mellanox/mlxbf-tmfifo.c
> @@ -980,11 +980,13 @@ static void mlxbf_tmfifo_virtio_set_status(struct 
> virtio_device *vdev,
>  }
>  
>  /* Reset the device. Not much here for now. */
> -static void mlxbf_tmfifo_virtio_reset(struct virtio_device *vdev)
> +static int mlxbf_tmfifo_virtio_reset(struct virtio_device *vdev)
>  {
>   struct mlxbf_tmfifo_vdev *tm_vdev = mlxbf_vdev_to_tmfifo(vdev);
>  
>   tm_vdev->status = 0;
> +
> + return 0;
>  }
>  
>  /* Read the value of a configuration field. */
> diff --git a/drivers/remoteproc/remoteproc_virtio.c 
> b/drivers/remoteproc/remoteproc_virtio.c
> index 0cc617f76068..ca9573c62c3d 100644
> --- a/drivers/remoteproc/remoteproc_virtio.c
> +++ b/drivers/remoteproc/remoteproc_virtio.c
> @@ -191,7 +191,7 @@ static void rproc_virtio_set_status(struct virtio_device 
> *vdev, u8 status)
>   dev_dbg(>dev, "status: %d\n", status);
>  }
>  
> -static void rproc_virtio_reset(struct virtio_device *vdev)
> +static int rproc_virtio_reset(struct virtio_device *vdev)
>  {
>   struct rproc_vdev *rvdev = vdev_to_rvdev(vdev);
>   struct fw_rsc_vdev *rsc;
> @@ -200,6 +200,8 @@ static void rproc_virtio_reset(struct virtio_device *vdev)
>  
>   rsc->status = 0;
>   dev_dbg(>dev, "reset !\n");
> +
> + return 0;
>  }
>  
>  /* provide the vdev features as retrieved from the firmware */
> diff --git a/drivers/s390/virtio/virtio_ccw.c 
> b/drivers/s390/virtio/virtio_ccw.c
> index 54e686dca6de..52b32555e746 100644
> --- a/drivers/s390/virtio/virtio_ccw.c
> +++ b/drivers/s390/virtio/virtio_ccw.c
> @@ -732,14 +732,15 @@ static int virtio_ccw_find_vqs(struct virtio_device 
> *vdev, unsigned nvqs,
>   return ret;
>  }
>  
> -static void virtio_ccw_reset(struct virtio_device *vdev)
> +static int virtio_ccw_reset(struct virtio_device *vdev)
>  {
>   struct virtio_ccw_device *vcdev = to_vc_device(vdev);
>   struct ccw1 *ccw;
> + int ret;
>  
>   ccw = ccw_device_dma_zalloc(vcdev->cdev, sizeof(*ccw));
>   if (!ccw)
> - return;
> + return -ENOMEM;
>  
>   /* Zero status bits. */
>   vcdev->dma_area->status = 0;
> @@ -749,8 +750,10 @@ static void virtio_ccw_reset(struct virtio_device *vdev)
>   ccw->flags = 0;
>   ccw->count = 0;
>   ccw->cda = 0;
> - ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_RESET);
> + ret = ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_RESET);
>   ccw_device_dma_free(vcdev->cdev, ccw, sizeof(*ccw));
> +
> + return ret;
>  }
>  
>  static u64 virtio_ccw_get_features(struct virtio_device *vdev)
> diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
> index 4b15c00c0a0a..ddbfd5b5f3bd 100644
> --- 

Re: update of MST's linux-next

2021-04-08 Thread Michael S. Tsirkin
On Thu, Apr 08, 2021 at 09:29:41AM +0300, Eli Cohen wrote:
> Hi Michael,
> 
> can you please update your linux-next branch with bits from other
> subsystems? There are fixes that we need to send and rely on code from
> those trees.
> 
> Moreover, there have been some patches that were reviewed but cannot be
> found in your branch.  Can you please pull those too into your branch?


Working on that. Anything you have in mind
pls feel free to remind me to make sure I don't miss it.

> Thanks,
> Eli

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [RFC PATCH] vdpa: mandate 1.0 device

2021-04-08 Thread Michael S. Tsirkin
On Thu, Apr 08, 2021 at 04:26:48PM +0800, Jason Wang wrote:
> This patch mandates 1.0 for vDPA devices. The goal is to have the
> semantic of normative statement in the virtio spec and eliminate the
> burden of transitional device for both vDPA bus and vDPA parent.
> 
> uAPI seems fine since all the vDPA parent mandates
> VIRTIO_F_ACCESS_PLATFORM which implies 1.0 devices.
> 
> For legacy guests, it can still work since Qemu will mediate when
> necessary (e.g doing the endian conversion).
> 
> Signed-off-by: Jason Wang 

Hmm. If we do this, don't we still have a problem with
legacy drivers which don't ack 1.0?
Note 1.0 affects ring endianness which is not mediated in QEMU
so QEMU can't pretend to device guest is 1.0.





> ---
>  include/linux/vdpa.h | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> index 0fefeb976877..cfde4ec999b4 100644
> --- a/include/linux/vdpa.h
> +++ b/include/linux/vdpa.h
> @@ -6,6 +6,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  /**
>   * vDPA callback definition.
> @@ -317,6 +318,11 @@ static inline int vdpa_set_features(struct vdpa_device 
> *vdev, u64 features)
>  {
>  const struct vdpa_config_ops *ops = vdev->config;
>  
> +/* Mandating 1.0 to have semantics of normative statements in
> + * the spec. */
> +if (!(features & BIT_ULL(VIRTIO_F_VERSION_1)))
> + return -EINVAL;
> +
>   vdev->features_valid = true;
>  return ops->set_features(vdev, features);
>  }
> -- 
> 2.25.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v2 1/3] virtio: update reset callback to return status

2021-04-08 Thread Michael S. Tsirkin
On Thu, Apr 08, 2021 at 12:56:52PM +0300, Max Gurtovoy wrote:
> 
> On 4/8/2021 11:58 AM, Jason Wang wrote:
> > 
> > 在 2021/4/8 下午4:11, Max Gurtovoy 写道:
> > > The reset device operation, usually is an operation that might fail from
> > > various reasons. For example, the controller might be in a bad state and
> > > can't answer to any request. Usually, the paravirt SW based virtio
> > > devices always succeed in reset operation but this is not the case for
> > > HW based virtio devices.
> > 
> > 
> > I would like to know under what condition that the reset operation may
> > fail (except for the case of a bugg guest).
> 
> The controller might not be ready or stuck. This is a real use case for many
> PCI devices.
> 
> For real devices the FW might be in a bad state and it can happen also for
> paravirt device if you have a bug in the controller code or if you entered
> some error flow (Out of memory).
> 
> You don't want to be stuck because of one bad device.

OK so maybe we can do more to detect the bad device.
Won't we get all 1's on a read in this case?


> 
> > 
> > 
> > > 
> > > This commit is also a preparation for adding a timeout mechanism for
> > > resetting virtio devices.
> > > 
> > > Signed-off-by: Max Gurtovoy 
> > > ---
> > > 
> > > changes from v1:
> > >   - update virtio_ccw.c (Cornelia)
> > >   - update virtio_uml.c
> > >   - update mlxbf-tmfifo.c
> > 
> > 
> > Note that virtio driver may call reset, so you probably need to convert
> > them.
> 
> I'm sure I understand.
> 
> Convert to what ?
> 
> Thanks.
> 
> > 
> > Thanks
> > 
> > 
> > > 
> > > ---
> > >   arch/um/drivers/virtio_uml.c |  4 +++-
> > >   drivers/platform/mellanox/mlxbf-tmfifo.c |  4 +++-
> > >   drivers/remoteproc/remoteproc_virtio.c   |  4 +++-
> > >   drivers/s390/virtio/virtio_ccw.c |  9 ++---
> > >   drivers/virtio/virtio.c  | 22 +++---
> > >   drivers/virtio/virtio_mmio.c |  3 ++-
> > >   drivers/virtio/virtio_pci_legacy.c   |  4 +++-
> > >   drivers/virtio/virtio_pci_modern.c   |  3 ++-
> > >   drivers/virtio/virtio_vdpa.c |  4 +++-
> > >   include/linux/virtio_config.h    |  5 +++--
> > >   10 files changed, 43 insertions(+), 19 deletions(-)
> > > 
> > > diff --git a/arch/um/drivers/virtio_uml.c b/arch/um/drivers/virtio_uml.c
> > > index 91ddf74ca888..b6e66265ed32 100644
> > > --- a/arch/um/drivers/virtio_uml.c
> > > +++ b/arch/um/drivers/virtio_uml.c
> > > @@ -827,11 +827,13 @@ static void vu_set_status(struct virtio_device
> > > *vdev, u8 status)
> > >   vu_dev->status = status;
> > >   }
> > >   -static void vu_reset(struct virtio_device *vdev)
> > > +static int vu_reset(struct virtio_device *vdev)
> > >   {
> > >   struct virtio_uml_device *vu_dev = to_virtio_uml_device(vdev);
> > >     vu_dev->status = 0;
> > > +
> > > +    return 0;
> > >   }
> > >     static void vu_del_vq(struct virtqueue *vq)
> > > diff --git a/drivers/platform/mellanox/mlxbf-tmfifo.c
> > > b/drivers/platform/mellanox/mlxbf-tmfifo.c
> > > index bbc4e71a16ff..c192b8ac5d9e 100644
> > > --- a/drivers/platform/mellanox/mlxbf-tmfifo.c
> > > +++ b/drivers/platform/mellanox/mlxbf-tmfifo.c
> > > @@ -980,11 +980,13 @@ static void
> > > mlxbf_tmfifo_virtio_set_status(struct virtio_device *vdev,
> > >   }
> > >     /* Reset the device. Not much here for now. */
> > > -static void mlxbf_tmfifo_virtio_reset(struct virtio_device *vdev)
> > > +static int mlxbf_tmfifo_virtio_reset(struct virtio_device *vdev)
> > >   {
> > >   struct mlxbf_tmfifo_vdev *tm_vdev = mlxbf_vdev_to_tmfifo(vdev);
> > >     tm_vdev->status = 0;
> > > +
> > > +    return 0;
> > >   }
> > >     /* Read the value of a configuration field. */
> > > diff --git a/drivers/remoteproc/remoteproc_virtio.c
> > > b/drivers/remoteproc/remoteproc_virtio.c
> > > index 0cc617f76068..ca9573c62c3d 100644
> > > --- a/drivers/remoteproc/remoteproc_virtio.c
> > > +++ b/drivers/remoteproc/remoteproc_virtio.c
> > > @@ -191,7 +191,7 @@ static void rproc_virtio_set_status(struct
> > > virtio_device *vdev, u8 status)
> > >   dev_dbg(>dev, "status: %d\n", status);
> > >   }
> > >   -static void rproc_virtio_reset(struct virtio_device *vdev)
> > > +static int rproc_virtio_reset(struct virtio_device *vdev)
> > >   {
> > >   struct rproc_vdev *rvdev = vdev_to_rvdev(vdev);
> > >   struct fw_rsc_vdev *rsc;
> > > @@ -200,6 +200,8 @@ static void rproc_virtio_reset(struct
> > > virtio_device *vdev)
> > >     rsc->status = 0;
> > >   dev_dbg(>dev, "reset !\n");
> > > +
> > > +    return 0;
> > >   }
> > >     /* provide the vdev features as retrieved from the firmware */
> > > diff --git a/drivers/s390/virtio/virtio_ccw.c
> > > b/drivers/s390/virtio/virtio_ccw.c
> > > index 54e686dca6de..52b32555e746 100644
> > > --- a/drivers/s390/virtio/virtio_ccw.c
> > > +++ b/drivers/s390/virtio/virtio_ccw.c
> > > @@ -732,14 +732,15 @@ static int virtio_ccw_find_vqs(struct
> > > virtio_device 

[PATCH v2 3/4] drm/nouveau: Use drm_gem_ttm_dumb_map_offset()

2021-04-08 Thread Thomas Zimmermann
Nouveau now uses drm_gem_ttm_dumb_map_offset() to implement
struct drm_driver.dumb_map_offset.

Signed-off-by: Thomas Zimmermann 
Acked-by: Maxime Ripard 
---
 drivers/gpu/drm/nouveau/nouveau_display.c | 18 --
 drivers/gpu/drm/nouveau/nouveau_display.h |  2 --
 drivers/gpu/drm/nouveau/nouveau_drm.c |  3 ++-
 3 files changed, 2 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c 
b/drivers/gpu/drm/nouveau/nouveau_display.c
index dac02c7be54d..14101bd2a0ff 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -838,21 +838,3 @@ nouveau_display_dumb_create(struct drm_file *file_priv, 
struct drm_device *dev,
drm_gem_object_put(>bo.base);
return ret;
 }
-
-int
-nouveau_display_dumb_map_offset(struct drm_file *file_priv,
-   struct drm_device *dev,
-   uint32_t handle, uint64_t *poffset)
-{
-   struct drm_gem_object *gem;
-
-   gem = drm_gem_object_lookup(file_priv, handle);
-   if (gem) {
-   struct nouveau_bo *bo = nouveau_gem_object(gem);
-   *poffset = drm_vma_node_offset_addr(>bo.base.vma_node);
-   drm_gem_object_put(gem);
-   return 0;
-   }
-
-   return -ENOENT;
-}
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h 
b/drivers/gpu/drm/nouveau/nouveau_display.h
index 616c43427059..2ab2ddb1eadf 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.h
+++ b/drivers/gpu/drm/nouveau/nouveau_display.h
@@ -58,8 +58,6 @@ bool nouveau_display_scanoutpos(struct drm_crtc *crtc,
 
 int  nouveau_display_dumb_create(struct drm_file *, struct drm_device *,
 struct drm_mode_create_dumb *args);
-int  nouveau_display_dumb_map_offset(struct drm_file *, struct drm_device *,
-u32 handle, u64 *offset);
 
 void nouveau_hdmi_mode_set(struct drm_encoder *, struct drm_display_mode *);
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c 
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 885815ea917f..9766218a99ca 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -31,6 +31,7 @@
 #include 
 
 #include 
+#include 
 #include 
 #include 
 
@@ -1212,7 +1213,7 @@ driver_stub = {
.gem_prime_import_sg_table = nouveau_gem_prime_import_sg_table,
 
.dumb_create = nouveau_display_dumb_create,
-   .dumb_map_offset = nouveau_display_dumb_map_offset,
+   .dumb_map_offset = drm_gem_ttm_dumb_map_offset,
 
.name = DRIVER_NAME,
.desc = DRIVER_DESC,
-- 
2.30.2

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH v2 2/4] drm/vram-helper: Use drm_gem_ttm_dumb_map_offset()

2021-04-08 Thread Thomas Zimmermann
VRAM helpers now use drm_gem_ttm_dumb_map_offset() to implement
struct drm_driver.dumb_map_offset.

v2:
* update hibmc as well (kernel test robot)

Signed-off-by: Thomas Zimmermann 
Acked-by: Maxime Ripard 
---
 drivers/gpu/drm/drm_gem_vram_helper.c | 48 ---
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   |  2 +-
 include/drm/drm_gem_vram_helper.h |  7 +--
 3 files changed, 3 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c 
b/drivers/gpu/drm/drm_gem_vram_helper.c
index 2b7c3a07956d..797200315854 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -245,22 +245,6 @@ void drm_gem_vram_put(struct drm_gem_vram_object *gbo)
 }
 EXPORT_SYMBOL(drm_gem_vram_put);
 
-/**
- * drm_gem_vram_mmap_offset() - Returns a GEM VRAM object's mmap offset
- * @gbo:   the GEM VRAM object
- *
- * See drm_vma_node_offset_addr() for more information.
- *
- * Returns:
- * The buffer object's offset for userspace mappings on success, or
- * 0 if no offset is allocated.
- */
-u64 drm_gem_vram_mmap_offset(struct drm_gem_vram_object *gbo)
-{
-   return drm_vma_node_offset_addr(>bo.base.vma_node);
-}
-EXPORT_SYMBOL(drm_gem_vram_mmap_offset);
-
 static u64 drm_gem_vram_pg_offset(struct drm_gem_vram_object *gbo)
 {
/* Keep TTM behavior for now, remove when drivers are audited */
@@ -638,38 +622,6 @@ int drm_gem_vram_driver_dumb_create(struct drm_file *file,
 }
 EXPORT_SYMBOL(drm_gem_vram_driver_dumb_create);
 
-/**
- * drm_gem_vram_driver_dumb_mmap_offset() - \
-   Implements  drm_driver.dumb_mmap_offset
- * @file:  DRM file pointer.
- * @dev:   DRM device.
- * @handle:GEM handle
- * @offset:Returns the mapping's memory offset on success
- *
- * Returns:
- * 0 on success, or
- * a negative errno code otherwise.
- */
-int drm_gem_vram_driver_dumb_mmap_offset(struct drm_file *file,
-struct drm_device *dev,
-uint32_t handle, uint64_t *offset)
-{
-   struct drm_gem_object *gem;
-   struct drm_gem_vram_object *gbo;
-
-   gem = drm_gem_object_lookup(file, handle);
-   if (!gem)
-   return -ENOENT;
-
-   gbo = drm_gem_vram_of_gem(gem);
-   *offset = drm_gem_vram_mmap_offset(gbo);
-
-   drm_gem_object_put(gem);
-
-   return 0;
-}
-EXPORT_SYMBOL(drm_gem_vram_driver_dumb_mmap_offset);
-
 /*
  * Helpers for struct drm_plane_helper_funcs
  */
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c 
b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
index abd6250d5a14..89edc796deda 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c
@@ -60,7 +60,7 @@ static const struct drm_driver hibmc_driver = {
.minor  = 0,
.debugfs_init   = drm_vram_mm_debugfs_init,
.dumb_create= hibmc_dumb_create,
-   .dumb_map_offset= drm_gem_vram_driver_dumb_mmap_offset,
+   .dumb_map_offset= drm_gem_ttm_dumb_map_offset,
.gem_prime_mmap = drm_gem_prime_mmap,
.irq_handler= hibmc_drm_interrupt,
 };
diff --git a/include/drm/drm_gem_vram_helper.h 
b/include/drm/drm_gem_vram_helper.h
index 288055d397d9..27ed7e9243b9 100644
--- a/include/drm/drm_gem_vram_helper.h
+++ b/include/drm/drm_gem_vram_helper.h
@@ -5,6 +5,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -93,7 +94,6 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct 
drm_device *dev,
size_t size,
unsigned long pg_align);
 void drm_gem_vram_put(struct drm_gem_vram_object *gbo);
-u64 drm_gem_vram_mmap_offset(struct drm_gem_vram_object *gbo);
 s64 drm_gem_vram_offset(struct drm_gem_vram_object *gbo);
 int drm_gem_vram_pin(struct drm_gem_vram_object *gbo, unsigned long pl_flag);
 int drm_gem_vram_unpin(struct drm_gem_vram_object *gbo);
@@ -113,9 +113,6 @@ int drm_gem_vram_fill_create_dumb(struct drm_file *file,
 int drm_gem_vram_driver_dumb_create(struct drm_file *file,
struct drm_device *dev,
struct drm_mode_create_dumb *args);
-int drm_gem_vram_driver_dumb_mmap_offset(struct drm_file *file,
-struct drm_device *dev,
-uint32_t handle, uint64_t *offset);
 
 /*
  * Helpers for struct drm_plane_helper_funcs
@@ -149,7 +146,7 @@ void drm_gem_vram_simple_display_pipe_cleanup_fb(
 #define DRM_GEM_VRAM_DRIVER \
.debugfs_init = drm_vram_mm_debugfs_init, \
.dumb_create  = drm_gem_vram_driver_dumb_create, \
-   .dumb_map_offset  = drm_gem_vram_driver_dumb_mmap_offset, \
+   .dumb_map_offset  = drm_gem_ttm_dumb_map_offset, \

[PATCH v2 1/4] drm/gem-ttm-helper: Provide helper for struct drm_driver.dumb_map_offset

2021-04-08 Thread Thomas Zimmermann
Provides an implementation of struct drm_driver.dumb_map_offset that
can be used by TTM-based GEM drivers.

Signed-off-by: Thomas Zimmermann 
Acked-by: Maxime Ripard 
---
 drivers/gpu/drm/drm_gem_ttm_helper.c | 33 
 include/drm/drm_gem_ttm_helper.h |  5 -
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_gem_ttm_helper.c 
b/drivers/gpu/drm/drm_gem_ttm_helper.c
index de28720757af..b14bed8be771 100644
--- a/drivers/gpu/drm/drm_gem_ttm_helper.c
+++ b/drivers/gpu/drm/drm_gem_ttm_helper.c
@@ -114,5 +114,38 @@ int drm_gem_ttm_mmap(struct drm_gem_object *gem,
 }
 EXPORT_SYMBOL(drm_gem_ttm_mmap);
 
+/**
+ * drm_gem_ttm_dumb_map_offset() - Implements struct 
_driver.dumb_map_offset
+ * @file:  DRM file pointer.
+ * @dev:   DRM device.
+ * @handle:GEM handle
+ * @offset:Returns the mapping's memory offset on success
+ *
+ * Provides an implementation of struct _driver.dumb_map_offset for
+ * TTM-based GEM drivers. TTM allocates the offset internally and
+ * drm_gem_ttm_dumb_map_offset() returns it for dumb-buffer implementations.
+ *
+ * See struct _driver.dumb_map_offset.
+ *
+ * Returns:
+ * 0 on success, or a negative errno code otherwise.
+ */
+int drm_gem_ttm_dumb_map_offset(struct drm_file *file, struct drm_device *dev,
+   uint32_t handle, uint64_t *offset)
+{
+   struct drm_gem_object *gem;
+
+   gem = drm_gem_object_lookup(file, handle);
+   if (!gem)
+   return -ENOENT;
+
+   *offset = drm_vma_node_offset_addr(>vma_node);
+
+   drm_gem_object_put(gem);
+
+   return 0;
+}
+EXPORT_SYMBOL(drm_gem_ttm_dumb_map_offset);
+
 MODULE_DESCRIPTION("DRM gem ttm helpers");
 MODULE_LICENSE("GPL");
diff --git a/include/drm/drm_gem_ttm_helper.h b/include/drm/drm_gem_ttm_helper.h
index 7c6d874910b8..c1aa02bd4c89 100644
--- a/include/drm/drm_gem_ttm_helper.h
+++ b/include/drm/drm_gem_ttm_helper.h
@@ -5,8 +5,8 @@
 
 #include 
 
-#include 
 #include 
+#include 
 #include 
 #include 
 
@@ -24,4 +24,7 @@ void drm_gem_ttm_vunmap(struct drm_gem_object *gem,
 int drm_gem_ttm_mmap(struct drm_gem_object *gem,
 struct vm_area_struct *vma);
 
+int drm_gem_ttm_dumb_map_offset(struct drm_file *file, struct drm_device *dev,
+   uint32_t handle, uint64_t *offset);
+
 #endif
-- 
2.30.2

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH v2 0/4] drm: Generic dumb_map_offset for TTM-based drivers

2021-04-08 Thread Thomas Zimmermann
The implementation of drm_driver.dumb_map_offset is the same for several
TTM-based drivers. Provide a common function in GEM-TTM helpers.

v2:
* update hibmc as well (kernel test robot)

Thomas Zimmermann (4):
  drm/gem-ttm-helper: Provide helper for struct
drm_driver.dumb_map_offset
  drm/vram-helper: Use drm_gem_ttm_dumb_map_offset()
  drm/nouveau: Use drm_gem_ttm_dumb_map_offset()
  drm/qxl: Use drm_gem_ttm_dumb_map_offset()

 drivers/gpu/drm/drm_gem_ttm_helper.c  | 33 +
 drivers/gpu/drm/drm_gem_vram_helper.c | 48 ---
 .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c   |  2 +-
 drivers/gpu/drm/nouveau/nouveau_display.c | 18 ---
 drivers/gpu/drm/nouveau/nouveau_display.h |  2 -
 drivers/gpu/drm/nouveau/nouveau_drm.c |  3 +-
 drivers/gpu/drm/qxl/qxl_drv.c |  3 +-
 drivers/gpu/drm/qxl/qxl_drv.h |  3 --
 drivers/gpu/drm/qxl/qxl_dumb.c| 17 ---
 drivers/gpu/drm/qxl/qxl_ioctl.c   |  4 +-
 drivers/gpu/drm/qxl/qxl_object.h  |  5 --
 include/drm/drm_gem_ttm_helper.h  |  5 +-
 include/drm/drm_gem_vram_helper.h |  7 +--
 13 files changed, 46 insertions(+), 104 deletions(-)

--
2.30.2

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH v2 4/4] drm/qxl: Use drm_gem_ttm_dumb_map_offset()

2021-04-08 Thread Thomas Zimmermann
Qxl now uses drm_gem_ttm_dumb_map_offset() to implement struct
drm_driver.dumb_map_offset.

Signed-off-by: Thomas Zimmermann 
Acked-by: Maxime Ripard 
---
 drivers/gpu/drm/qxl/qxl_drv.c|  3 ++-
 drivers/gpu/drm/qxl/qxl_drv.h|  3 ---
 drivers/gpu/drm/qxl/qxl_dumb.c   | 17 -
 drivers/gpu/drm/qxl/qxl_ioctl.c  |  4 ++--
 drivers/gpu/drm/qxl/qxl_object.h |  5 -
 5 files changed, 4 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index 1864467f1063..db92eec07d96 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -37,6 +37,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -271,7 +272,7 @@ static struct drm_driver qxl_driver = {
.driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
 
.dumb_create = qxl_mode_dumb_create,
-   .dumb_map_offset = qxl_mode_dumb_mmap,
+   .dumb_map_offset = drm_gem_ttm_dumb_map_offset,
 #if defined(CONFIG_DEBUG_FS)
.debugfs_init = qxl_debugfs_init,
 #endif
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 6dd57cfb2e7c..20a0f3ab84ad 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -330,9 +330,6 @@ void qxl_bo_force_delete(struct qxl_device *qdev);
 int qxl_mode_dumb_create(struct drm_file *file_priv,
 struct drm_device *dev,
 struct drm_mode_create_dumb *args);
-int qxl_mode_dumb_mmap(struct drm_file *filp,
-  struct drm_device *dev,
-  uint32_t handle, uint64_t *offset_p);
 
 /* qxl ttm */
 int qxl_ttm_init(struct qxl_device *qdev);
diff --git a/drivers/gpu/drm/qxl/qxl_dumb.c b/drivers/gpu/drm/qxl/qxl_dumb.c
index 48a58ba1db96..a635d9fdf8ac 100644
--- a/drivers/gpu/drm/qxl/qxl_dumb.c
+++ b/drivers/gpu/drm/qxl/qxl_dumb.c
@@ -69,20 +69,3 @@ int qxl_mode_dumb_create(struct drm_file *file_priv,
args->handle = handle;
return 0;
 }
-
-int qxl_mode_dumb_mmap(struct drm_file *file_priv,
-  struct drm_device *dev,
-  uint32_t handle, uint64_t *offset_p)
-{
-   struct drm_gem_object *gobj;
-   struct qxl_bo *qobj;
-
-   BUG_ON(!offset_p);
-   gobj = drm_gem_object_lookup(file_priv, handle);
-   if (gobj == NULL)
-   return -ENOENT;
-   qobj = gem_to_qxl_bo(gobj);
-   *offset_p = qxl_bo_mmap_offset(qobj);
-   drm_gem_object_put(gobj);
-   return 0;
-}
diff --git a/drivers/gpu/drm/qxl/qxl_ioctl.c b/drivers/gpu/drm/qxl/qxl_ioctl.c
index b6075f452b9e..38aabcbe2238 100644
--- a/drivers/gpu/drm/qxl/qxl_ioctl.c
+++ b/drivers/gpu/drm/qxl/qxl_ioctl.c
@@ -67,8 +67,8 @@ static int qxl_map_ioctl(struct drm_device *dev, void *data,
struct qxl_device *qdev = to_qxl(dev);
struct drm_qxl_map *qxl_map = data;
 
-   return qxl_mode_dumb_mmap(file_priv, >ddev, qxl_map->handle,
- _map->offset);
+   return drm_gem_ttm_dumb_map_offset(file_priv, >ddev, 
qxl_map->handle,
+  _map->offset);
 }
 
 struct qxl_reloc_info {
diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h
index ee9c29de4d3d..cee4b52b75dd 100644
--- a/drivers/gpu/drm/qxl/qxl_object.h
+++ b/drivers/gpu/drm/qxl/qxl_object.h
@@ -53,11 +53,6 @@ static inline unsigned long qxl_bo_size(struct qxl_bo *bo)
return bo->tbo.base.size;
 }
 
-static inline u64 qxl_bo_mmap_offset(struct qxl_bo *bo)
-{
-   return drm_vma_node_offset_addr(>tbo.base.vma_node);
-}
-
 extern int qxl_bo_create(struct qxl_device *qdev,
 unsigned long size,
 bool kernel, bool pinned, u32 domain,
-- 
2.30.2

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v2 2/3] virito_pci: add timeout to reset device operation

2021-04-08 Thread Jason Wang


在 2021/4/8 下午8:57, Max Gurtovoy 写道:


On 4/8/2021 3:45 PM, Jason Wang wrote:


在 2021/4/8 下午5:44, Max Gurtovoy 写道:


On 4/8/2021 12:01 PM, Jason Wang wrote:


在 2021/4/8 下午4:11, Max Gurtovoy 写道:
According to the spec after writing 0 to device_status, the driver 
MUST
wait for a read of device_status to return 0 before reinitializing 
the

device. In case we have a device that won't return 0, the reset
operation will loop forever and cause the host/vm to stuck. Set 
timeout

for 3 minutes before giving up on the device.

Signed-off-by: Max Gurtovoy 
---
  drivers/virtio/virtio_pci_modern.c | 10 +-
  1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_pci_modern.c 
b/drivers/virtio/virtio_pci_modern.c

index cc3412a96a17..dcee616e8d21 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -162,6 +162,7 @@ static int vp_reset(struct virtio_device *vdev)
  {
  struct virtio_pci_device *vp_dev = to_vp_device(vdev);
  struct virtio_pci_modern_device *mdev = _dev->mdev;
+    unsigned long timeout = jiffies + msecs_to_jiffies(18);
    /* 0 status means a reset. */
  vp_modern_set_status(mdev, 0);
@@ -169,9 +170,16 @@ static int vp_reset(struct virtio_device *vdev)
   * device_status to return 0 before reinitializing the device.
   * This will flush out the status write, and flush in device 
writes,

   * including MSI-X interrupts, if any.
+ * Set a timeout before giving up on the device.
   */
-    while (vp_modern_get_status(mdev))
+    while (vp_modern_get_status(mdev)) {
+    if (time_after(jiffies, timeout)) {



What happens if the device finish the rest after the timeout?



The driver will set VIRTIO_CONFIG_S_FAILED and one can re-probe it 
later on (e.g by re-scanning the pci bus).



Ok, so do we need the flush through vp_synchronize_vectors() here?


If the device didn't write 0 to status I guess we don't need that.

The device shouldn't raise any interrupt before negotiation finish 
successfully.



The reset could be triggered in other places like driver removing.

Thanks




MST, is that correct ?



Thanks







Thanks



+ dev_err(>dev, "virtio: device not ready. "
+    "Aborting. Try again later\n");
+    return -EAGAIN;
+    }
  msleep(1);
+    }
  /* Flush pending VQ/configuration callbacks. */
  vp_synchronize_vectors(vdev);
  return 0;










___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 1/3] virtio: update reset callback to return status

2021-04-08 Thread Jason Wang


在 2021/4/8 下午5:56, Max Gurtovoy 写道:


On 4/8/2021 11:58 AM, Jason Wang wrote:


在 2021/4/8 下午4:11, Max Gurtovoy 写道:
The reset device operation, usually is an operation that might fail 
from
various reasons. For example, the controller might be in a bad state 
and

can't answer to any request. Usually, the paravirt SW based virtio
devices always succeed in reset operation but this is not the case for
HW based virtio devices.



I would like to know under what condition that the reset operation 
may fail (except for the case of a bugg guest).


The controller might not be ready or stuck. This is a real use case 
for many PCI devices.


For real devices the FW might be in a bad state and it can happen also 
for paravirt device if you have a bug in the controller code or if you 
entered some error flow (Out of memory).


You don't want to be stuck because of one bad device.



So the buggy driver can damage the host through various ways, I'm not 
sure doing such workaround is worthwhile.


Note that this driver has been used for real hardware PCI devices for 
many years. We don't receive any report of this before.











This commit is also a preparation for adding a timeout mechanism for
resetting virtio devices.

Signed-off-by: Max Gurtovoy 
---

changes from v1:
  - update virtio_ccw.c (Cornelia)
  - update virtio_uml.c
  - update mlxbf-tmfifo.c



Note that virtio driver may call reset, so you probably need to 
convert them.


I'm sure I understand.

Convert to what ?



Convert to deal with the possible rest failure. E.g in virtblk_freeze() 
we had:


static int virtblk_freeze(struct virtio_device *vdev)
{
    struct virtio_blk *vblk = vdev->priv;

    /* Ensure we don't receive any more interrupts */
    vdev->config->reset(vdev);
...

We need fail the freeze here.

Another example is the driver remove which is not expected to be fail. A 
lot of virtio drivers tries to call reset there. I'm not sure how hard 
to deal with the failure in the path of remove (e.g 
__device_release_driver tends to ignore the return value of bus->remove().)


Thanks




Thanks.



Thanks




---
  arch/um/drivers/virtio_uml.c |  4 +++-
  drivers/platform/mellanox/mlxbf-tmfifo.c |  4 +++-
  drivers/remoteproc/remoteproc_virtio.c   |  4 +++-
  drivers/s390/virtio/virtio_ccw.c |  9 ++---
  drivers/virtio/virtio.c  | 22 +++---
  drivers/virtio/virtio_mmio.c |  3 ++-
  drivers/virtio/virtio_pci_legacy.c   |  4 +++-
  drivers/virtio/virtio_pci_modern.c   |  3 ++-
  drivers/virtio/virtio_vdpa.c |  4 +++-
  include/linux/virtio_config.h    |  5 +++--
  10 files changed, 43 insertions(+), 19 deletions(-)

diff --git a/arch/um/drivers/virtio_uml.c 
b/arch/um/drivers/virtio_uml.c

index 91ddf74ca888..b6e66265ed32 100644
--- a/arch/um/drivers/virtio_uml.c
+++ b/arch/um/drivers/virtio_uml.c
@@ -827,11 +827,13 @@ static void vu_set_status(struct virtio_device 
*vdev, u8 status)

  vu_dev->status = status;
  }
  -static void vu_reset(struct virtio_device *vdev)
+static int vu_reset(struct virtio_device *vdev)
  {
  struct virtio_uml_device *vu_dev = to_virtio_uml_device(vdev);
    vu_dev->status = 0;
+
+    return 0;
  }
    static void vu_del_vq(struct virtqueue *vq)
diff --git a/drivers/platform/mellanox/mlxbf-tmfifo.c 
b/drivers/platform/mellanox/mlxbf-tmfifo.c

index bbc4e71a16ff..c192b8ac5d9e 100644
--- a/drivers/platform/mellanox/mlxbf-tmfifo.c
+++ b/drivers/platform/mellanox/mlxbf-tmfifo.c
@@ -980,11 +980,13 @@ static void 
mlxbf_tmfifo_virtio_set_status(struct virtio_device *vdev,

  }
    /* Reset the device. Not much here for now. */
-static void mlxbf_tmfifo_virtio_reset(struct virtio_device *vdev)
+static int mlxbf_tmfifo_virtio_reset(struct virtio_device *vdev)
  {
  struct mlxbf_tmfifo_vdev *tm_vdev = mlxbf_vdev_to_tmfifo(vdev);
    tm_vdev->status = 0;
+
+    return 0;
  }
    /* Read the value of a configuration field. */
diff --git a/drivers/remoteproc/remoteproc_virtio.c 
b/drivers/remoteproc/remoteproc_virtio.c

index 0cc617f76068..ca9573c62c3d 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -191,7 +191,7 @@ static void rproc_virtio_set_status(struct 
virtio_device *vdev, u8 status)

  dev_dbg(>dev, "status: %d\n", status);
  }
  -static void rproc_virtio_reset(struct virtio_device *vdev)
+static int rproc_virtio_reset(struct virtio_device *vdev)
  {
  struct rproc_vdev *rvdev = vdev_to_rvdev(vdev);
  struct fw_rsc_vdev *rsc;
@@ -200,6 +200,8 @@ static void rproc_virtio_reset(struct 
virtio_device *vdev)

    rsc->status = 0;
  dev_dbg(>dev, "reset !\n");
+
+    return 0;
  }
    /* provide the vdev features as retrieved from the firmware */
diff --git a/drivers/s390/virtio/virtio_ccw.c 
b/drivers/s390/virtio/virtio_ccw.c

index 54e686dca6de..52b32555e746 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ 

Re: [PATCH v2 2/3] virito_pci: add timeout to reset device operation

2021-04-08 Thread Jason Wang


在 2021/4/8 下午5:44, Max Gurtovoy 写道:


On 4/8/2021 12:01 PM, Jason Wang wrote:


在 2021/4/8 下午4:11, Max Gurtovoy 写道:

According to the spec after writing 0 to device_status, the driver MUST
wait for a read of device_status to return 0 before reinitializing the
device. In case we have a device that won't return 0, the reset
operation will loop forever and cause the host/vm to stuck. Set timeout
for 3 minutes before giving up on the device.

Signed-off-by: Max Gurtovoy 
---
  drivers/virtio/virtio_pci_modern.c | 10 +-
  1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_pci_modern.c 
b/drivers/virtio/virtio_pci_modern.c

index cc3412a96a17..dcee616e8d21 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -162,6 +162,7 @@ static int vp_reset(struct virtio_device *vdev)
  {
  struct virtio_pci_device *vp_dev = to_vp_device(vdev);
  struct virtio_pci_modern_device *mdev = _dev->mdev;
+    unsigned long timeout = jiffies + msecs_to_jiffies(18);
    /* 0 status means a reset. */
  vp_modern_set_status(mdev, 0);
@@ -169,9 +170,16 @@ static int vp_reset(struct virtio_device *vdev)
   * device_status to return 0 before reinitializing the device.
   * This will flush out the status write, and flush in device 
writes,

   * including MSI-X interrupts, if any.
+ * Set a timeout before giving up on the device.
   */
-    while (vp_modern_get_status(mdev))
+    while (vp_modern_get_status(mdev)) {
+    if (time_after(jiffies, timeout)) {



What happens if the device finish the rest after the timeout?



The driver will set VIRTIO_CONFIG_S_FAILED and one can re-probe it 
later on (e.g by re-scanning the pci bus).



Ok, so do we need the flush through vp_synchronize_vectors() here?

Thanks







Thanks



+    dev_err(>dev, "virtio: device not ready. "
+    "Aborting. Try again later\n");
+    return -EAGAIN;
+    }
  msleep(1);
+    }
  /* Flush pending VQ/configuration callbacks. */
  vp_synchronize_vectors(vdev);
  return 0;






___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 0/4] drm: Generic dumb_map_offset for TTM-based drivers

2021-04-08 Thread Daniel Vetter
On Thu, Apr 08, 2021 at 01:34:03PM +0200, Thomas Zimmermann wrote:
> Hi
> 
> Am 08.04.21 um 13:16 schrieb Daniel Vetter:
> > On Tue, Apr 06, 2021 at 10:29:38AM +0200, Thomas Zimmermann wrote:
> > > The implementation of drm_driver.dumb_map_offset is the same for several
> > > TTM-based drivers. Provide a common function in GEM-TTM helpers.
> > 
> > Out of curiosity, why does this not fit for radeon/amdgpu?
> 
> These drivers perform additional permission checks in their implementations.
> 
> But those checks are also part of the actual mmap code. I want to propose a
> patch to use the generic drm_gem_ttm_map_offset in amdgpu/radeon, and then
> rely on mmap to fail if necessary. It might result in a longer discussion,
> so that's for another patchset.

Ah cool, makes sense.
-Daniel

> 
> Best regards
> Thomas
> 
> > -Daniel
> > 
> > > 
> > > Thomas Zimmermann (4):
> > >drm/gem-ttm-helper: Provide helper for struct
> > >  drm_driver.dumb_map_offset
> > >drm/vram-helper: Use drm_gem_ttm_dumb_map_offset()
> > >drm/nouveau: Use drm_gem_ttm_dumb_map_offset()
> > >drm/qxl: Use drm_gem_ttm_dumb_map_offset()
> > > 
> > >   drivers/gpu/drm/drm_gem_ttm_helper.c  | 33 
> > >   drivers/gpu/drm/drm_gem_vram_helper.c | 48 ---
> > >   drivers/gpu/drm/nouveau/nouveau_display.c | 18 -
> > >   drivers/gpu/drm/nouveau/nouveau_display.h |  2 -
> > >   drivers/gpu/drm/nouveau/nouveau_drm.c |  3 +-
> > >   drivers/gpu/drm/qxl/qxl_drv.c |  3 +-
> > >   drivers/gpu/drm/qxl/qxl_drv.h |  3 --
> > >   drivers/gpu/drm/qxl/qxl_dumb.c| 17 
> > >   drivers/gpu/drm/qxl/qxl_ioctl.c   |  4 +-
> > >   drivers/gpu/drm/qxl/qxl_object.h  |  5 ---
> > >   include/drm/drm_gem_ttm_helper.h  |  5 ++-
> > >   include/drm/drm_gem_vram_helper.h |  7 +---
> > >   12 files changed, 45 insertions(+), 103 deletions(-)
> > > 
> > > --
> > > 2.30.2
> > > 
> > 
> 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Maxfeldstr. 5, 90409 Nürnberg, Germany
> (HRB 36809, AG Nürnberg)
> Geschäftsführer: Felix Imendörffer
> 




-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH 0/4] drm: Generic dumb_map_offset for TTM-based drivers

2021-04-08 Thread Thomas Zimmermann

Hi

Am 08.04.21 um 13:16 schrieb Daniel Vetter:

On Tue, Apr 06, 2021 at 10:29:38AM +0200, Thomas Zimmermann wrote:

The implementation of drm_driver.dumb_map_offset is the same for several
TTM-based drivers. Provide a common function in GEM-TTM helpers.


Out of curiosity, why does this not fit for radeon/amdgpu?


These drivers perform additional permission checks in their implementations.

But those checks are also part of the actual mmap code. I want to 
propose a patch to use the generic drm_gem_ttm_map_offset in 
amdgpu/radeon, and then rely on mmap to fail if necessary. It might 
result in a longer discussion, so that's for another patchset.


Best regards
Thomas


-Daniel



Thomas Zimmermann (4):
   drm/gem-ttm-helper: Provide helper for struct
 drm_driver.dumb_map_offset
   drm/vram-helper: Use drm_gem_ttm_dumb_map_offset()
   drm/nouveau: Use drm_gem_ttm_dumb_map_offset()
   drm/qxl: Use drm_gem_ttm_dumb_map_offset()

  drivers/gpu/drm/drm_gem_ttm_helper.c  | 33 
  drivers/gpu/drm/drm_gem_vram_helper.c | 48 ---
  drivers/gpu/drm/nouveau/nouveau_display.c | 18 -
  drivers/gpu/drm/nouveau/nouveau_display.h |  2 -
  drivers/gpu/drm/nouveau/nouveau_drm.c |  3 +-
  drivers/gpu/drm/qxl/qxl_drv.c |  3 +-
  drivers/gpu/drm/qxl/qxl_drv.h |  3 --
  drivers/gpu/drm/qxl/qxl_dumb.c| 17 
  drivers/gpu/drm/qxl/qxl_ioctl.c   |  4 +-
  drivers/gpu/drm/qxl/qxl_object.h  |  5 ---
  include/drm/drm_gem_ttm_helper.h  |  5 ++-
  include/drm/drm_gem_vram_helper.h |  7 +---
  12 files changed, 45 insertions(+), 103 deletions(-)

--
2.30.2





--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer



OpenPGP_signature
Description: OpenPGP digital signature
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 0/4] drm: Generic dumb_map_offset for TTM-based drivers

2021-04-08 Thread Daniel Vetter
On Tue, Apr 06, 2021 at 10:29:38AM +0200, Thomas Zimmermann wrote:
> The implementation of drm_driver.dumb_map_offset is the same for several
> TTM-based drivers. Provide a common function in GEM-TTM helpers.

Out of curiosity, why does this not fit for radeon/amdgpu?
-Daniel

> 
> Thomas Zimmermann (4):
>   drm/gem-ttm-helper: Provide helper for struct
> drm_driver.dumb_map_offset
>   drm/vram-helper: Use drm_gem_ttm_dumb_map_offset()
>   drm/nouveau: Use drm_gem_ttm_dumb_map_offset()
>   drm/qxl: Use drm_gem_ttm_dumb_map_offset()
> 
>  drivers/gpu/drm/drm_gem_ttm_helper.c  | 33 
>  drivers/gpu/drm/drm_gem_vram_helper.c | 48 ---
>  drivers/gpu/drm/nouveau/nouveau_display.c | 18 -
>  drivers/gpu/drm/nouveau/nouveau_display.h |  2 -
>  drivers/gpu/drm/nouveau/nouveau_drm.c |  3 +-
>  drivers/gpu/drm/qxl/qxl_drv.c |  3 +-
>  drivers/gpu/drm/qxl/qxl_drv.h |  3 --
>  drivers/gpu/drm/qxl/qxl_dumb.c| 17 
>  drivers/gpu/drm/qxl/qxl_ioctl.c   |  4 +-
>  drivers/gpu/drm/qxl/qxl_object.h  |  5 ---
>  include/drm/drm_gem_ttm_helper.h  |  5 ++-
>  include/drm/drm_gem_vram_helper.h |  7 +---
>  12 files changed, 45 insertions(+), 103 deletions(-)
> 
> --
> 2.30.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v2 00/10] drm: Support simple-framebuffer devices and firmware fbs

2021-04-08 Thread Daniel Vetter
On Tue, Mar 30, 2021 at 10:34:12AM +0200, Hans de Goede wrote:
> Hi,
> 
> On 3/30/21 9:09 AM, Thomas Zimmermann wrote:
> > Hi
> > 
> > Am 29.03.21 um 16:50 schrieb Hans de Goede:
> >> Hi,
> >>
> >> On 3/29/21 2:31 PM, Thomas Zimmermann wrote:
> >>> Hi
> >>>
> >>> Am 25.03.21 um 12:29 schrieb Hans de Goede:
>  Hi,
> 
>  On 3/18/21 11:29 AM, Thomas Zimmermann wrote:
> > This patchset adds support for simple-framebuffer platform devices and
> > a handover mechanism for native drivers to take-over control of the
> > hardware.
> >
> > The new driver, called simpledrm, binds to a simple-frambuffer platform
> > device. The kernel's boot code creates such devices for 
> > firmware-provided
> > framebuffers, such as EFI-GOP or VESA. Typically the BIOS, UEFI or boot
> > loader sets up the framebuffers. Description via device tree is also an
> > option.
> >
> > Simpledrm is small enough to be linked into the kernel. The driver's 
> > main
> > purpose is to provide graphical output during the early phases of the 
> > boot
> > process, before the native DRM drivers are available. Native drivers are
> > typically loaded from an initrd ram disk. Occationally simpledrm can 
> > also
> > serve as interim solution on graphics hardware without native DRM 
> > driver.
> >
> > So far distributions rely on fbdev drivers, such as efifb, vesafb or
> > simplefb, for early-boot graphical output. However fbdev is deprecated 
> > and
> > the drivers do not provide DRM interfaces for modern userspace.
> >
> > Patches 1 and 2 prepare the DRM format helpers for simpledrm.
> >
> > Patches 3 and 4 add a hand-over mechanism. Simpledrm acquires it's
> > framebuffer's I/O-memory range and provides a callback function to be
> > removed by a native driver. The native driver will remove simpledrm 
> > before
> > taking over the hardware. The removal is integrated into existing 
> > helpers,
> > so drivers use it automatically.
> >
> > Patches 5 to 10 add the simpledrm driver. It's build on simple DRM 
> > helpers
> > and SHMEM. It supports 16-bit, 24-bit and 32-bit RGB framebuffers. 
> > During
> > pageflips, SHMEM buffers are copied into the framebuffer memory, similar
> > to cirrus or mgag200. The code in patches 8 and 9 handles clocks and
> > regulators. It's based on the simplefb drivers, but has been modified 
> > for
> > DRM.
> 
>  Thank you for your work on this, this is very interesting.
> 
> > I've also been working on fastboot support (i.e., flicker-free booting).
> > This requires state-readout from simpledrm via generic interfaces, as
> > outlined in [1]. I do have some prototype code, but it will take a while
> > to get this ready. Simpledrm will then support it.
> >
> > I've tested simpledrm with x86 EFI and VESA framebuffers, which both 
> > work
> > reliably. The fbdev console and Weston work automatically. Xorg requires
> > manual configuration of the device. Xorgs current modesetting driver 
> > does
> > not work with both, platform and PCI device, for the same physical
> > hardware. Once configured, X11 works. I looked into X11, but couldn't 
> > see
> > an easy way of fixing the problem. With the push towards 
> > Wayland+Xwayland
> > I expect the problem to become a non-issue soon. Additional testing has
> > been reported at [2].
> >
> > One cosmetical issue is that simpledrm's device file is card0 and the
> > native driver's device file is card1. After simpledrm has been kicked 
> > out,
> > only card1 is left. This does not seem to be a practical problem 
> > however.
> >
> > TODO/IDEAS:
> >
> >  * provide deferred takeover
> 
>  I'm not sure what you mean with this ?  Currently deferred-takeover is
>  handled in the fbcon code. Current flickerfree boot works like this
>  (assuming a single LCD panel in a laptop):
> 
>  1. EFI/GOP sets up the framebuffer, draws a vendor logo
>  2. The bootloader runs in silent mode and does not touch anything gfx 
>  related
>  3. kernel boots, with a loglevel of 3 so only CRIT/EMERG messages are 
>  shown
>  2. efifb loads; and tells fbcon that a framebuffer is now available for 
>  it to "bind"
>   to. Since CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER=y fbcon 
>  defers taking over
>   the console and leaves the dummy-console driver in place (unless 
>  there have already
>   been kernel messages logged, which there shouldn't because 
>  loglevel=3)
>  3. i915 loads, reads out the hw state compares this to the 
>  preferred-mode for the
>   panel which it would set, they match, nothing happens. i915 takes 
>  ownership
>   of the scanout-buffer set up by the GOP, 

Re: [PATCH v2 03/10] drm/aperture: Move fbdev conflict helpers into drm_aperture.h

2021-04-08 Thread Daniel Vetter
On Thu, Mar 18, 2021 at 11:29:14AM +0100, Thomas Zimmermann wrote:
> Fbdev's helpers for handling conflicting framebuffers are related to
> framebuffer apertures, not console emulation. Therefore move them into a
> drm_aperture.h, which will contain the interfaces for the new aperture
> helpers.
> 
> Signed-off-by: Thomas Zimmermann 
> Tested-by: nerdopolis 
> ---
>  Documentation/gpu/drm-internals.rst |  6 +++
>  include/drm/drm_aperture.h  | 60 +
>  include/drm/drm_fb_helper.h | 56 ++-
>  3 files changed, 69 insertions(+), 53 deletions(-)
>  create mode 100644 include/drm/drm_aperture.h
> 
> diff --git a/Documentation/gpu/drm-internals.rst 
> b/Documentation/gpu/drm-internals.rst
> index 12272b168580..4c7642d2ca34 100644
> --- a/Documentation/gpu/drm-internals.rst
> +++ b/Documentation/gpu/drm-internals.rst
> @@ -75,6 +75,12 @@ update it, its value is mostly useless. The DRM core 
> prints it to the
>  kernel log at initialization time and passes it to userspace through the
>  DRM_IOCTL_VERSION ioctl.
>  
> +Managing Ownership of the Framebuffer Aperture
> +--
> +
> +.. kernel-doc:: include/drm/drm_aperture.h
> +   :internal:
> +
>  Device Instance and Driver Handling
>  ---
>  
> diff --git a/include/drm/drm_aperture.h b/include/drm/drm_aperture.h
> new file mode 100644
> index ..13766efe9517
> --- /dev/null
> +++ b/include/drm/drm_aperture.h
> @@ -0,0 +1,60 @@
> +/* SPDX-License-Identifier: MIT */
> +
> +#ifndef _DRM_APERTURE_H_
> +#define _DRM_APERTURE_H_
> +
> +#include 
> +#include 
> +
> +/**
> + * drm_fb_helper_remove_conflicting_framebuffers - remove 
> firmware-configured framebuffers

Annoying bikeshed, but I'd give them drm_aperture_ prefixes, for ocd
consistency. Also make them real functions, they're quite big and will
grow more in the next patch.

I'm also not super happy about the naming here but oh well.

Either way: Acked-by: Daniel Vetter 

> + * @a: memory range, users of which are to be removed
> + * @name: requesting driver name
> + * @primary: also kick vga16fb if present
> + *
> + * This function removes framebuffer devices (initialized by 
> firmware/bootloader)
> + * which use memory range described by @a. If @a is NULL all such devices are
> + * removed.
> + */
> +static inline int
> +drm_fb_helper_remove_conflicting_framebuffers(struct apertures_struct *a,
> +   const char *name, bool primary)
> +{
> +#if IS_REACHABLE(CONFIG_FB)
> + return remove_conflicting_framebuffers(a, name, primary);
> +#else
> + return 0;
> +#endif
> +}
> +
> +/**
> + * drm_fb_helper_remove_conflicting_pci_framebuffers - remove 
> firmware-configured
> + * framebuffers for PCI 
> devices
> + * @pdev: PCI device
> + * @name: requesting driver name
> + *
> + * This function removes framebuffer devices (eg. initialized by firmware)
> + * using memory range configured for any of @pdev's memory bars.
> + *
> + * The function assumes that PCI device with shadowed ROM drives a primary
> + * display and so kicks out vga16fb.
> + */
> +static inline int
> +drm_fb_helper_remove_conflicting_pci_framebuffers(struct pci_dev *pdev,
> +   const char *name)
> +{
> + int ret = 0;
> +
> + /*
> +  * WARNING: Apparently we must kick fbdev drivers before vgacon,
> +  * otherwise the vga fbdev driver falls over.
> +  */
> +#if IS_REACHABLE(CONFIG_FB)
> + ret = remove_conflicting_pci_framebuffers(pdev, name);
> +#endif
> + if (ret == 0)
> + ret = vga_remove_vgacon(pdev);
> + return ret;
> +}
> +
> +#endif
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 3b273f9ca39a..d06a3942fddb 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -30,13 +30,13 @@
>  #ifndef DRM_FB_HELPER_H
>  #define DRM_FB_HELPER_H
>  
> -struct drm_fb_helper;
> -
> +#include 
>  #include 
>  #include 
>  #include 
>  #include 
> -#include 
> +
> +struct drm_fb_helper;
>  
>  enum mode_set_atomic {
>   LEAVE_ATOMIC_MODE_SET,
> @@ -451,54 +451,4 @@ drm_fbdev_generic_setup(struct drm_device *dev, unsigned 
> int preferred_bpp)
>  
>  #endif
>  
> -/**
> - * drm_fb_helper_remove_conflicting_framebuffers - remove 
> firmware-configured framebuffers
> - * @a: memory range, users of which are to be removed
> - * @name: requesting driver name
> - * @primary: also kick vga16fb if present
> - *
> - * This function removes framebuffer devices (initialized by 
> firmware/bootloader)
> - * which use memory range described by @a. If @a is NULL all such devices are
> - * removed.
> - */
> -static inline int
> -drm_fb_helper_remove_conflicting_framebuffers(struct apertures_struct *a,
> -   const char *name, bool primary)
> -{
> 

Re: [PATCH v2 04/10] drm/aperture: Add infrastructure for aperture ownership

2021-04-08 Thread Daniel Vetter
On Thu, Mar 18, 2021 at 11:29:15AM +0100, Thomas Zimmermann wrote:
> Platform devices might operate on firmware framebuffers, such as VESA or
> EFI. Before a native driver for the graphics hardware can take over the
> device, it has to remove any platform driver that operates on the firmware
> framebuffer. Aperture helpers provide the infrastructure for platform
> drivers to acquire firmware framebuffers, and for native drivers to remove
> them later on.
> 
> It works similar to the related fbdev mechanism. During initialization, the
> platform driver acquires the firmware framebuffer's I/O memory and provides
> a callback to be removed. The native driver later uses this information to
> remove any platform driver for it's framebuffer I/O memory.
> 
> The aperture removal code is integrated into the existing code for removing
> conflicting framebuffers, so native drivers use it automatically.
> 
> v2:
>   * rename plaform helpers to aperture helpers
>   * tie to device lifetime with devm_ functions
>   * removed unsued remove() callback
>   * rename kickout to detach
>   * make struct drm_aperture private
>   * rebase onto existing drm_aperture.h header file
>   * use MIT license only for simplicity
>   * documentation
> 
> Signed-off-by: Thomas Zimmermann 
> Tested-by: nerdopolis 

Bunch of bikesheds for your considerations below, but overall lgtm.

Acked-by: Daniel Vetter 

Cheers, Daniel

> ---
>  Documentation/gpu/drm-internals.rst |   6 +
>  drivers/gpu/drm/Kconfig |   7 +
>  drivers/gpu/drm/Makefile|   1 +
>  drivers/gpu/drm/drm_aperture.c  | 287 
>  include/drm/drm_aperture.h  |  38 +++-
>  5 files changed, 338 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/drm_aperture.c
> 
> diff --git a/Documentation/gpu/drm-internals.rst 
> b/Documentation/gpu/drm-internals.rst
> index 4c7642d2ca34..06af044c882f 100644
> --- a/Documentation/gpu/drm-internals.rst
> +++ b/Documentation/gpu/drm-internals.rst
> @@ -78,9 +78,15 @@ DRM_IOCTL_VERSION ioctl.
>  Managing Ownership of the Framebuffer Aperture
>  --
>  
> +.. kernel-doc:: drivers/gpu/drm/drm_aperture.c
> +   :doc: overview
> +
>  .. kernel-doc:: include/drm/drm_aperture.h
> :internal:
>  
> +.. kernel-doc:: drivers/gpu/drm/drm_aperture.c
> +   :export:
> +
>  Device Instance and Driver Handling
>  ---
>  
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 1461652921be..b9d3fb91d22d 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -221,6 +221,13 @@ config DRM_SCHED
>   tristate
>   depends on DRM
>  
> +config DRM_APERTURE
> + bool
> + depends on DRM
> + help
> +   Controls ownership of graphics apertures. Required to
> +   synchronize with firmware-based drivers.

Uh I'm not a big fan of Kconfig and .ko modules for every little helper
code. Imo just stuff this into the drm kms helpers and done. Or stuff it
into drm core code, I think either is a good case for this. Everything is
its own module means we need to EXPORT_SYMBOL more stuff, and then drivers
get funny ideas about using these internals ...

> +
>  source "drivers/gpu/drm/i2c/Kconfig"
>  
>  source "drivers/gpu/drm/arm/Kconfig"
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 5eb5bf7c16e3..c9ecb02df0f3 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -32,6 +32,7 @@ drm-$(CONFIG_AGP) += drm_agpsupport.o
>  drm-$(CONFIG_PCI) += drm_pci.o
>  drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o
>  drm-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
> +drm-$(CONFIG_DRM_APERTURE) += drm_aperture.o
>  
>  drm_vram_helper-y := drm_gem_vram_helper.o
>  obj-$(CONFIG_DRM_VRAM_HELPER) += drm_vram_helper.o
> diff --git a/drivers/gpu/drm/drm_aperture.c b/drivers/gpu/drm/drm_aperture.c
> new file mode 100644
> index ..4b02b5fed0a1
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_aperture.c
> @@ -0,0 +1,287 @@
> +// SPDX-License-Identifier: MIT
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include 
> +#include 
> +#include 
> +
> +/**
> + * DOC: overview
> + *
> + * A graphics device might be supported by different drivers, but only one
> + * driver can be active at any given time. Many systems load a generic
> + * graphics drivers, such as EFI-GOP or VESA, early during the boot process.
> + * During later boot stages, they replace the generic driver with a 
> dedicated,
> + * hardware-specific driver. To take over the device the dedicated driver
> + * first has to remove the generic driver. DRM aperture functions manage
> + * ownership of DRM framebuffer memory and hand-over between drivers.
> + *
> + * DRM drivers should call drm_fb_helper_remove_conflicting_framebuffers()
> + * at the top of their probe function. The function removes any 

Re: [PATCH 4/5] vdpa/mlx5: Fix wrong use of bit numbers

2021-04-08 Thread Jason Wang


在 2021/4/8 下午5:10, Eli Cohen 写道:

VIRTIO_F_VERSION_1 is a bit number. Use BIT_ULL() with mask
conditionals.

Also, in mlx5_vdpa_is_little_endian() use BIT_ULL for consistency with
the rest of the code.

Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported mlx5 devices")
Signed-off-by: Eli Cohen 



Acked-by: Jason Wang 



---
  drivers/vdpa/mlx5/net/mlx5_vnet.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index a49ebb250253..6fe61fc57790 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -820,7 +820,7 @@ static int create_virtqueue(struct mlx5_vdpa_net *ndev, 
struct mlx5_vdpa_virtque
MLX5_SET(virtio_q, vq_ctx, event_qpn_or_msix, mvq->fwqp.mqp.qpn);
MLX5_SET(virtio_q, vq_ctx, queue_size, mvq->num_ent);
MLX5_SET(virtio_q, vq_ctx, virtio_version_1_0,
-!!(ndev->mvdev.actual_features & VIRTIO_F_VERSION_1));
+!!(ndev->mvdev.actual_features & BIT_ULL(VIRTIO_F_VERSION_1)));
MLX5_SET64(virtio_q, vq_ctx, desc_addr, mvq->desc_addr);
MLX5_SET64(virtio_q, vq_ctx, used_addr, mvq->device_addr);
MLX5_SET64(virtio_q, vq_ctx, available_addr, mvq->driver_addr);
@@ -1554,7 +1554,7 @@ static void clear_virtqueues(struct mlx5_vdpa_net *ndev)
  static inline bool mlx5_vdpa_is_little_endian(struct mlx5_vdpa_dev *mvdev)
  {
return virtio_legacy_is_little_endian() ||
-   (mvdev->actual_features & (1ULL << VIRTIO_F_VERSION_1));
+   (mvdev->actual_features & BIT_ULL(VIRTIO_F_VERSION_1));
  }
  
  static __virtio16 cpu_to_mlx5vdpa16(struct mlx5_vdpa_dev *mvdev, u16 val)


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 5/5] vdpa/mlx5: Fix suspend/resume index restoration

2021-04-08 Thread Jason Wang


在 2021/4/8 下午5:10, Eli Cohen 写道:

When we suspend the VM, the VDPA interface will be reset. When the VM is
resumed again, clear_virtqueues() will clear the available and used
indices resulting in hardware virqtqueue objects becoming out of sync.
We can avoid this function alltogether since qemu will clear them if
required, e.g. when the VM went through a reboot.

Moreover, since the hw available and used indices should always be
identical on query and should be restored to the same value same value
for virtqueues that complete in order, we set the single value provided
by set_vq_state(). In get_vq_state() we return the value of hardware
used index.

Fixes: b35ccebe3ef7 ("vdpa/mlx5: Restore the hardware used index after change 
map")
Fixes: 1a86b377aa21 ("vdpa/mlx5: Add VDPA driver for supported mlx5 devices")
Signed-off-by: Eli Cohen 
---



Acked-by: Jason Wang 



  drivers/vdpa/mlx5/net/mlx5_vnet.c | 21 -
  1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 6fe61fc57790..4d2809c7d4e3 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -1169,6 +1169,7 @@ static void suspend_vq(struct mlx5_vdpa_net *ndev, struct 
mlx5_vdpa_virtqueue *m
return;
}
mvq->avail_idx = attr.available_index;
+   mvq->used_idx = attr.used_index;
  }
  
  static void suspend_vqs(struct mlx5_vdpa_net *ndev)

@@ -1426,6 +1427,7 @@ static int mlx5_vdpa_set_vq_state(struct vdpa_device 
*vdev, u16 idx,
return -EINVAL;
}
  
+	mvq->used_idx = state->avail_index;

mvq->avail_idx = state->avail_index;
return 0;
  }
@@ -1443,7 +1445,11 @@ static int mlx5_vdpa_get_vq_state(struct vdpa_device 
*vdev, u16 idx, struct vdpa
 * that cares about emulating the index after vq is stopped.
 */
if (!mvq->initialized) {
-   state->avail_index = mvq->avail_idx;
+   /* Firmware returns a wrong value for the available index.
+* Since both values should be identical, we take the value of
+* used_idx which is reported correctly.
+*/
+   state->avail_index = mvq->used_idx;
return 0;
}
  
@@ -1452,7 +1458,7 @@ static int mlx5_vdpa_get_vq_state(struct vdpa_device *vdev, u16 idx, struct vdpa

mlx5_vdpa_warn(mvdev, "failed to query virtqueue\n");
return err;
}
-   state->avail_index = attr.available_index;
+   state->avail_index = attr.used_index;
return 0;
  }
  
@@ -1540,16 +1546,6 @@ static void teardown_virtqueues(struct mlx5_vdpa_net *ndev)

}
  }
  
-static void clear_virtqueues(struct mlx5_vdpa_net *ndev)

-{
-   int i;
-
-   for (i = ndev->mvdev.max_vqs - 1; i >= 0; i--) {
-   ndev->vqs[i].avail_idx = 0;
-   ndev->vqs[i].used_idx = 0;
-   }
-}
-
  /* TODO: cross-endian support */
  static inline bool mlx5_vdpa_is_little_endian(struct mlx5_vdpa_dev *mvdev)
  {
@@ -1785,7 +1781,6 @@ static void mlx5_vdpa_set_status(struct vdpa_device 
*vdev, u8 status)
if (!status) {
mlx5_vdpa_info(mvdev, "performing device reset\n");
teardown_driver(ndev);
-   clear_virtqueues(ndev);
mlx5_vdpa_destroy_mr(>mvdev);
ndev->mvdev.status = 0;
ndev->mvdev.mlx_features = 0;


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 3/5] vdpa/mlx5: Retrieve BAR address suitable any function

2021-04-08 Thread Jason Wang


在 2021/4/8 下午5:10, Eli Cohen 写道:

struct mlx5_core_dev has a bar_addr field that contains the correct bar
address for the function regardless of whether it is pci function or sub
function. Use it.

Fixes: 1958fc2f0712 ("net/mlx5: SF, Add auxiliary device driver")
Signed-off-by: Eli Cohen 
Reviewed-by: Parav Pandit 



Acked-by: Jason Wang 



---
  drivers/vdpa/mlx5/core/resources.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/vdpa/mlx5/core/resources.c 
b/drivers/vdpa/mlx5/core/resources.c
index 96e6421c5d1c..6521cbd0f5c2 100644
--- a/drivers/vdpa/mlx5/core/resources.c
+++ b/drivers/vdpa/mlx5/core/resources.c
@@ -246,7 +246,8 @@ int mlx5_vdpa_alloc_resources(struct mlx5_vdpa_dev *mvdev)
if (err)
goto err_key;
  
-	kick_addr = pci_resource_start(mdev->pdev, 0) + offset;

+   kick_addr = mdev->bar_addr + offset;
+
res->kick_addr = ioremap(kick_addr, PAGE_SIZE);
if (!res->kick_addr) {
err = -ENOMEM;


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 2/5] vdpa/mlx5: Use the correct dma device when registering memory

2021-04-08 Thread Jason Wang


在 2021/4/8 下午5:10, Eli Cohen 写道:

In cases where the vdpa instance uses a SF (sub function), the DMA
device is the parent device. Use a function to retrieve the correct DMA
device.

Fixes: 1958fc2f0712 ("net/mlx5: SF, Add auxiliary device driver")
Signed-off-by: Eli Cohen 
Reviewed-by: Parav Pandit 



Acked-by: Jason Wang 



---
  drivers/vdpa/mlx5/core/mr.c | 9 +++--
  1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/vdpa/mlx5/core/mr.c b/drivers/vdpa/mlx5/core/mr.c
index d300f799efcd..3908ff28eec0 100644
--- a/drivers/vdpa/mlx5/core/mr.c
+++ b/drivers/vdpa/mlx5/core/mr.c
@@ -219,6 +219,11 @@ static void destroy_indirect_key(struct mlx5_vdpa_dev 
*mvdev, struct mlx5_vdpa_m
mlx5_vdpa_destroy_mkey(mvdev, >mkey);
  }
  
+static struct device *get_dma_device(struct mlx5_vdpa_dev *mvdev)

+{
+   return >mdev->pdev->dev;
+}
+
  static int map_direct_mr(struct mlx5_vdpa_dev *mvdev, struct 
mlx5_vdpa_direct_mr *mr,
 struct vhost_iotlb *iotlb)
  {
@@ -234,7 +239,7 @@ static int map_direct_mr(struct mlx5_vdpa_dev *mvdev, 
struct mlx5_vdpa_direct_mr
u64 pa;
u64 paend;
struct scatterlist *sg;
-   struct device *dma = mvdev->mdev->device;
+   struct device *dma = get_dma_device(mvdev);
  
  	for (map = vhost_iotlb_itree_first(iotlb, mr->start, mr->end - 1);

 map; map = vhost_iotlb_itree_next(map, start, mr->end - 1)) {
@@ -291,7 +296,7 @@ static int map_direct_mr(struct mlx5_vdpa_dev *mvdev, 
struct mlx5_vdpa_direct_mr
  
  static void unmap_direct_mr(struct mlx5_vdpa_dev *mvdev, struct mlx5_vdpa_direct_mr *mr)

  {
-   struct device *dma = mvdev->mdev->device;
+   struct device *dma = get_dma_device(mvdev);
  
  	destroy_direct_mr(mvdev, mr);

dma_unmap_sg_attrs(dma, mr->sg_head.sgl, mr->nsg, DMA_BIDIRECTIONAL, 0);


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 0/5] VDPA mlx5 fixes

2021-04-08 Thread Greg KH
On Thu, Apr 08, 2021 at 12:10:42PM +0300, Eli Cohen wrote:
> Hi Michael,
> 
> The following series contains fixes to mlx5 vdpa driver.  Included first
> is Siwei's fix to queried MTU was already reviewed a while ago and is
> not in your tree.
> 
> Patches 2 and 3 are required to allow mlx5_vdpa run on sub functions.
> 
> This series contains patches that were included in Parav's series
> http://lists.infradead.org/pipermail/linux-mtd/2016-January/064878.html
> but that series will be sent again at a later time.
> 
> Eli Cohen (4):
>   vdpa/mlx5: Use the correct dma device when registering memory
>   vdpa/mlx5: Retrieve BAR address suitable any function
>   vdpa/mlx5: Fix wrong use of bit numbers
>   vdpa/mlx5: Fix suspend/resume index restoration
> 
> Si-Wei Liu (1):
>   vdpa/mlx5: should exclude header length and fcs from mtu
> 
>  drivers/vdpa/mlx5/core/mlx5_vdpa.h |  4 +++
>  drivers/vdpa/mlx5/core/mr.c|  9 +--
>  drivers/vdpa/mlx5/core/resources.c |  3 ++-
>  drivers/vdpa/mlx5/net/mlx5_vnet.c  | 40 ++
>  4 files changed, 37 insertions(+), 19 deletions(-)
> 
> -- 
> 2.30.1
> 




This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v2 2/3] virito_pci: add timeout to reset device operation

2021-04-08 Thread Jason Wang


在 2021/4/8 下午4:11, Max Gurtovoy 写道:

According to the spec after writing 0 to device_status, the driver MUST
wait for a read of device_status to return 0 before reinitializing the
device. In case we have a device that won't return 0, the reset
operation will loop forever and cause the host/vm to stuck. Set timeout
for 3 minutes before giving up on the device.

Signed-off-by: Max Gurtovoy 
---
  drivers/virtio/virtio_pci_modern.c | 10 +-
  1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_pci_modern.c 
b/drivers/virtio/virtio_pci_modern.c
index cc3412a96a17..dcee616e8d21 100644
--- a/drivers/virtio/virtio_pci_modern.c
+++ b/drivers/virtio/virtio_pci_modern.c
@@ -162,6 +162,7 @@ static int vp_reset(struct virtio_device *vdev)
  {
struct virtio_pci_device *vp_dev = to_vp_device(vdev);
struct virtio_pci_modern_device *mdev = _dev->mdev;
+   unsigned long timeout = jiffies + msecs_to_jiffies(18);
  
  	/* 0 status means a reset. */

vp_modern_set_status(mdev, 0);
@@ -169,9 +170,16 @@ static int vp_reset(struct virtio_device *vdev)
 * device_status to return 0 before reinitializing the device.
 * This will flush out the status write, and flush in device writes,
 * including MSI-X interrupts, if any.
+* Set a timeout before giving up on the device.
 */
-   while (vp_modern_get_status(mdev))
+   while (vp_modern_get_status(mdev)) {
+   if (time_after(jiffies, timeout)) {



What happens if the device finish the rest after the timeout?

Thanks



+   dev_err(>dev, "virtio: device not ready. "
+   "Aborting. Try again later\n");
+   return -EAGAIN;
+   }
msleep(1);
+   }
/* Flush pending VQ/configuration callbacks. */
vp_synchronize_vectors(vdev);
return 0;


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH v2 1/3] virtio: update reset callback to return status

2021-04-08 Thread Jason Wang


在 2021/4/8 下午4:11, Max Gurtovoy 写道:

The reset device operation, usually is an operation that might fail from
various reasons. For example, the controller might be in a bad state and
can't answer to any request. Usually, the paravirt SW based virtio
devices always succeed in reset operation but this is not the case for
HW based virtio devices.



I would like to know under what condition that the reset operation may 
fail (except for the case of a bugg guest).





This commit is also a preparation for adding a timeout mechanism for
resetting virtio devices.

Signed-off-by: Max Gurtovoy 
---

changes from v1:
  - update virtio_ccw.c (Cornelia)
  - update virtio_uml.c
  - update mlxbf-tmfifo.c



Note that virtio driver may call reset, so you probably need to convert 
them.


Thanks




---
  arch/um/drivers/virtio_uml.c |  4 +++-
  drivers/platform/mellanox/mlxbf-tmfifo.c |  4 +++-
  drivers/remoteproc/remoteproc_virtio.c   |  4 +++-
  drivers/s390/virtio/virtio_ccw.c |  9 ++---
  drivers/virtio/virtio.c  | 22 +++---
  drivers/virtio/virtio_mmio.c |  3 ++-
  drivers/virtio/virtio_pci_legacy.c   |  4 +++-
  drivers/virtio/virtio_pci_modern.c   |  3 ++-
  drivers/virtio/virtio_vdpa.c |  4 +++-
  include/linux/virtio_config.h|  5 +++--
  10 files changed, 43 insertions(+), 19 deletions(-)

diff --git a/arch/um/drivers/virtio_uml.c b/arch/um/drivers/virtio_uml.c
index 91ddf74ca888..b6e66265ed32 100644
--- a/arch/um/drivers/virtio_uml.c
+++ b/arch/um/drivers/virtio_uml.c
@@ -827,11 +827,13 @@ static void vu_set_status(struct virtio_device *vdev, u8 
status)
vu_dev->status = status;
  }
  
-static void vu_reset(struct virtio_device *vdev)

+static int vu_reset(struct virtio_device *vdev)
  {
struct virtio_uml_device *vu_dev = to_virtio_uml_device(vdev);
  
  	vu_dev->status = 0;

+
+   return 0;
  }
  
  static void vu_del_vq(struct virtqueue *vq)

diff --git a/drivers/platform/mellanox/mlxbf-tmfifo.c 
b/drivers/platform/mellanox/mlxbf-tmfifo.c
index bbc4e71a16ff..c192b8ac5d9e 100644
--- a/drivers/platform/mellanox/mlxbf-tmfifo.c
+++ b/drivers/platform/mellanox/mlxbf-tmfifo.c
@@ -980,11 +980,13 @@ static void mlxbf_tmfifo_virtio_set_status(struct 
virtio_device *vdev,
  }
  
  /* Reset the device. Not much here for now. */

-static void mlxbf_tmfifo_virtio_reset(struct virtio_device *vdev)
+static int mlxbf_tmfifo_virtio_reset(struct virtio_device *vdev)
  {
struct mlxbf_tmfifo_vdev *tm_vdev = mlxbf_vdev_to_tmfifo(vdev);
  
  	tm_vdev->status = 0;

+
+   return 0;
  }
  
  /* Read the value of a configuration field. */

diff --git a/drivers/remoteproc/remoteproc_virtio.c 
b/drivers/remoteproc/remoteproc_virtio.c
index 0cc617f76068..ca9573c62c3d 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -191,7 +191,7 @@ static void rproc_virtio_set_status(struct virtio_device 
*vdev, u8 status)
dev_dbg(>dev, "status: %d\n", status);
  }
  
-static void rproc_virtio_reset(struct virtio_device *vdev)

+static int rproc_virtio_reset(struct virtio_device *vdev)
  {
struct rproc_vdev *rvdev = vdev_to_rvdev(vdev);
struct fw_rsc_vdev *rsc;
@@ -200,6 +200,8 @@ static void rproc_virtio_reset(struct virtio_device *vdev)
  
  	rsc->status = 0;

dev_dbg(>dev, "reset !\n");
+
+   return 0;
  }
  
  /* provide the vdev features as retrieved from the firmware */

diff --git a/drivers/s390/virtio/virtio_ccw.c b/drivers/s390/virtio/virtio_ccw.c
index 54e686dca6de..52b32555e746 100644
--- a/drivers/s390/virtio/virtio_ccw.c
+++ b/drivers/s390/virtio/virtio_ccw.c
@@ -732,14 +732,15 @@ static int virtio_ccw_find_vqs(struct virtio_device 
*vdev, unsigned nvqs,
return ret;
  }
  
-static void virtio_ccw_reset(struct virtio_device *vdev)

+static int virtio_ccw_reset(struct virtio_device *vdev)
  {
struct virtio_ccw_device *vcdev = to_vc_device(vdev);
struct ccw1 *ccw;
+   int ret;
  
  	ccw = ccw_device_dma_zalloc(vcdev->cdev, sizeof(*ccw));

if (!ccw)
-   return;
+   return -ENOMEM;
  
  	/* Zero status bits. */

vcdev->dma_area->status = 0;
@@ -749,8 +750,10 @@ static void virtio_ccw_reset(struct virtio_device *vdev)
ccw->flags = 0;
ccw->count = 0;
ccw->cda = 0;
-   ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_RESET);
+   ret = ccw_io_helper(vcdev, ccw, VIRTIO_CCW_DOING_RESET);
ccw_device_dma_free(vcdev->cdev, ccw, sizeof(*ccw));
+
+   return ret;
  }
  
  static u64 virtio_ccw_get_features(struct virtio_device *vdev)

diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index 4b15c00c0a0a..ddbfd5b5f3bd 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -338,7 +338,7 @@ int register_virtio_device(struct virtio_device *dev)
/* Assign a unique device index and hence name. */

[RFC PATCH] vdpa: mandate 1.0 device

2021-04-08 Thread Jason Wang
This patch mandates 1.0 for vDPA devices. The goal is to have the
semantic of normative statement in the virtio spec and eliminate the
burden of transitional device for both vDPA bus and vDPA parent.

uAPI seems fine since all the vDPA parent mandates
VIRTIO_F_ACCESS_PLATFORM which implies 1.0 devices.

For legacy guests, it can still work since Qemu will mediate when
necessary (e.g doing the endian conversion).

Signed-off-by: Jason Wang 
---
 include/linux/vdpa.h | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index 0fefeb976877..cfde4ec999b4 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -6,6 +6,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /**
  * vDPA callback definition.
@@ -317,6 +318,11 @@ static inline int vdpa_set_features(struct vdpa_device 
*vdev, u64 features)
 {
 const struct vdpa_config_ops *ops = vdev->config;
 
+/* Mandating 1.0 to have semantics of normative statements in
+ * the spec. */
+if (!(features & BIT_ULL(VIRTIO_F_VERSION_1)))
+   return -EINVAL;
+
vdev->features_valid = true;
 return ops->set_features(vdev, features);
 }
-- 
2.25.1

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v6 10/10] Documentation: Add documentation for VDUSE

2021-04-08 Thread Jason Wang


在 2021/3/31 下午4:05, Xie Yongji 写道:

VDUSE (vDPA Device in Userspace) is a framework to support
implementing software-emulated vDPA devices in userspace. This
document is intended to clarify the VDUSE design and usage.

Signed-off-by: Xie Yongji 
---
  Documentation/userspace-api/index.rst |   1 +
  Documentation/userspace-api/vduse.rst | 212 ++
  2 files changed, 213 insertions(+)
  create mode 100644 Documentation/userspace-api/vduse.rst

diff --git a/Documentation/userspace-api/index.rst 
b/Documentation/userspace-api/index.rst
index acd2cc2a538d..f63119130898 100644
--- a/Documentation/userspace-api/index.rst
+++ b/Documentation/userspace-api/index.rst
@@ -24,6 +24,7 @@ place where this information is gathered.
 ioctl/index
 iommu
 media/index
+   vduse
  
  .. only::  subproject and html
  
diff --git a/Documentation/userspace-api/vduse.rst b/Documentation/userspace-api/vduse.rst

new file mode 100644
index ..8c4e2b2df8bb
--- /dev/null
+++ b/Documentation/userspace-api/vduse.rst
@@ -0,0 +1,212 @@
+==
+VDUSE - "vDPA Device in Userspace"
+==
+
+vDPA (virtio data path acceleration) device is a device that uses a
+datapath which complies with the virtio specifications with vendor
+specific control path. vDPA devices can be both physically located on
+the hardware or emulated by software. VDUSE is a framework that makes it
+possible to implement software-emulated vDPA devices in userspace.
+
+How VDUSE works
+
+Each userspace vDPA device is created by the VDUSE_CREATE_DEV ioctl on
+the character device (/dev/vduse/control). Then a device file with the
+specified name (/dev/vduse/$NAME) will appear, which can be used to
+implement the userspace vDPA device's control path and data path.
+
+To implement control path, a message-based communication protocol and some
+types of control messages are introduced in the VDUSE framework:
+
+- VDUSE_SET_VQ_ADDR: Set the vring address of virtqueue.
+
+- VDUSE_SET_VQ_NUM: Set the size of virtqueue
+
+- VDUSE_SET_VQ_READY: Set ready status of virtqueue
+
+- VDUSE_GET_VQ_READY: Get ready status of virtqueue
+
+- VDUSE_SET_VQ_STATE: Set the state for virtqueue
+
+- VDUSE_GET_VQ_STATE: Get the state for virtqueue
+
+- VDUSE_SET_FEATURES: Set virtio features supported by the driver
+
+- VDUSE_GET_FEATURES: Get virtio features supported by the device
+
+- VDUSE_SET_STATUS: Set the device status
+
+- VDUSE_GET_STATUS: Get the device status
+
+- VDUSE_SET_CONFIG: Write to device specific configuration space
+
+- VDUSE_GET_CONFIG: Read from device specific configuration space
+
+- VDUSE_UPDATE_IOTLB: Notify userspace to update the memory mapping in device 
IOTLB
+
+Those control messages are mostly based on the vdpa_config_ops in
+include/linux/vdpa.h which defines a unified interface to control
+different types of vdpa device. Userspace needs to read()/write()
+on the VDUSE device file to receive/reply those control messages
+from/to VDUSE kernel module as follows:
+
+.. code-block:: c
+
+   static int vduse_message_handler(int dev_fd)
+   {
+   int len;
+   struct vduse_dev_request req;
+   struct vduse_dev_response resp;
+
+   len = read(dev_fd, , sizeof(req));
+   if (len != sizeof(req))
+   return -1;
+
+   resp.request_id = req.request_id;
+
+   switch (req.type) {
+
+   /* handle different types of message */
+
+   }
+
+   len = write(dev_fd, , sizeof(resp));
+   if (len != sizeof(resp))
+   return -1;
+
+   return 0;
+   }
+
+In the data path, vDPA device's iova regions will be mapped into userspace
+with the help of VDUSE_IOTLB_GET_FD ioctl on the VDUSE device file:
+
+- VDUSE_IOTLB_GET_FD: get the file descriptor to the first overlapped iova 
region.
+  Userspace can access this iova region by passing fd and corresponding size, 
offset,
+  perm to mmap(). For example:
+
+.. code-block:: c
+
+   static int perm_to_prot(uint8_t perm)
+   {
+   int prot = 0;
+
+   switch (perm) {
+   case VDUSE_ACCESS_WO:
+   prot |= PROT_WRITE;
+   break;
+   case VDUSE_ACCESS_RO:
+   prot |= PROT_READ;
+   break;
+   case VDUSE_ACCESS_RW:
+   prot |= PROT_READ | PROT_WRITE;
+   break;
+   }
+
+   return prot;
+   }
+
+   static void *iova_to_va(int dev_fd, uint64_t iova, uint64_t *len)
+   {
+   int fd;
+   void *addr;
+   size_t size;
+   struct vduse_iotlb_entry entry;
+
+   entry.start = iova;
+   entry.last = iova + 1;
+   fd = ioctl(dev_fd, 

Re: [PATCH v6 09/10] vduse: Introduce VDUSE - vDPA Device in Userspace

2021-04-08 Thread Jason Wang


在 2021/3/31 下午4:05, Xie Yongji 写道:

This VDUSE driver enables implementing vDPA devices in userspace.
Both control path and data path of vDPA devices will be able to
be handled in userspace.

In the control path, the VDUSE driver will make use of message
mechnism to forward the config operation from vdpa bus driver
to userspace. Userspace can use read()/write() to receive/reply
those control messages.

In the data path, VDUSE_IOTLB_GET_FD ioctl will be used to get
the file descriptors referring to vDPA device's iova regions. Then
userspace can use mmap() to access those iova regions. Besides,
userspace can use ioctl() to inject interrupt and use the eventfd
mechanism to receive virtqueue kicks.

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 | 1362 
  include/uapi/linux/vduse.h |  175 +++
  6 files changed, 1554 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 a4c75a28c839..71722e6f8f23 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 a245809c99d0..77a1da522c21 100644
--- a/drivers/vdpa/Kconfig
+++ b/drivers/vdpa/Kconfig
@@ -25,6 +25,16 @@ config VDPA_SIM_NET
help
  vDPA networking device simulator which loops TX traffic back to RX.
  
+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 ..51ca73464d0d
--- /dev/null
+++ b/drivers/vdpa/vdpa_user/vduse_dev.c
@@ -0,0 +1,1362 @@
+// 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 "iova_domain.h"
+
+#define DRV_VERSION  "1.0"
+#define DRV_AUTHOR   "Yongji Xie "
+#define DRV_DESC "vDPA Device in Userspace"
+#define DRV_LICENSE  "GPL v2"
+
+#define VDUSE_DEV_MAX (1U << MINORBITS)
+
+struct vduse_virtqueue {
+   u16 index;
+   bool ready;
+   spinlock_t kick_lock;
+   spinlock_t irq_lock;
+   struct eventfd_ctx *kickfd;
+   struct vdpa_callback cb;
+   struct work_struct inject;
+};
+
+struct vduse_dev;
+
+struct vduse_vdpa {
+   struct vdpa_device vdpa;
+   struct vduse_dev *dev;
+};
+
+struct vduse_dev {
+   struct vduse_vdpa *vdev;
+   struct device dev;
+   struct cdev cdev;
+   struct vduse_virtqueue *vqs;
+   struct vduse_iova_domain *domain;
+   struct mutex lock;
+   spinlock_t msg_lock;
+   atomic64_t msg_unique;
+   wait_queue_head_t waitq;
+   struct list_head send_list;
+   struct 

RE: update of MST's linux-next

2021-04-08 Thread Parav Pandit
> From: Eli Cohen 
> Sent: Thursday, April 8, 2021 12:00 PM
> 
> Hi Michael,
> 
> can you please update your linux-next branch with bits from other
> subsystems? There are fixes that we need to send and rely on code from
> those trees.
Having vhost's linux-next upto date with [1] is best.

We at least need till commit [2].

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
[2] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=7c1ef1959b6fefe616ef3e7df832bf63dfbab9cf
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


RE: [PATCH linux-next v2 04/14] vdpa: Introduce query of device config layout

2021-04-08 Thread Parav Pandit



> From: Jason Wang 
> Sent: Wednesday, April 7, 2021 12:42 PM
> 
> 在 2021/4/7 下午1:10, Parav Pandit 写道:
> >
> >> From: Jason Wang 
> >> Sent: Wednesday, April 7, 2021 9:26 AM
> > [..]
> >>>/**
> >>> * struct vdpa_config_ops - operations for configuring a vDPA device.
> >>> * Note: vDPA device drivers are required to implement all of the
> >>> @@
> >>> -164,6 +200,10 @@ struct vdpa_iova_range {
> >>> * @buf: buffer used to write from
> >>> * @len: the length to write to
> >>> * configuration space
> >>> + * @get_ce_config:   Read device specific configuration in
> >>> + *   cpu endianness.
> >>> + *   @vdev: vdpa device
> >>> + *   @config: pointer to config buffer used 
> >>> to
> >> read to
> >>
> >>
> >> So I wonder what's the reason for having this? If it's only the
> >> reason of endian, we can just use get_config.
> >>
> > Didn't follow your suggestion.
> > How does in kernel management tool caller get_config  know how to do
> endianenss conversion?
> 
> 
> LE should be used, so it's the responsibility of the vDPA parent
> (driver) to do the endian conversion.
> 
> 
> > Or you are proposing to send this whole virtio_config structure as binary
> data to user space via netlink?
> > If so, it is not a practice in netlink to return struct.
> 
> 
> I don't get here, it should work as mac address I think.
> 
> 
> >
> > Also making netlink user space to understand __virtio16 doesn't sound
> correct.
> > Often orchestration is not written in C. I cannot think of returning
> virtio_net_config via netlink socket if it is your thought.
> 
> 
> I'm not sure I get here. __virtio16 is part of uapi which is defined
> virtio_types.h so did the virtio_net_config. Duplicating those through
> dedicated netlink attr looks like burden. E.g you need to introduce new
> attrs for each field of the config for every virtio devices and keep it
> up-to-date with the virtio uapis.
> 
> 
Given that only few fields are valid from the config struct, if we pass the 
whole struct, we additionally need to pass the bitmask to indicate what is 
valid.
And so I think passing individual fields via the currently defined netlink is 
better.
Lets continue using individual fields.
> So I think device should maintain a stable features that will is
> returned by get_features(), otherwise a lot of things will be broken.


> I've had some disucssion with Michael, the conclusion is that we should
> only advertise (or mandate) a modern device to be exposed userspace.
> Otherwise it could be a lot of burden. Qemu can still present a
> transtitonal device by doing the endian conversion when necessary in the
> middle. I'm working on the Qemu patches to do that.

Ok. enforcing this as first thing so that netlink config callbacks can assume 
them LE would be nice.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization