Re: [PATCH v3 1/2] vdpa/mlx5: Extend driver support for new features

2023-03-20 Thread Si-Wei Liu




On 3/20/2023 4:49 AM, Eli Cohen wrote:

Extend the possible list for features that can be supported by firmware.
Note that different versions of firmware may or may not support these
features. The driver is made aware of them by querying the firmware.

While doing this, improve the code so we use enum names instead of hard
coded numerical values.

The new features supported by the driver are the following:

VIRTIO_NET_F_MRG_RXBUF
VIRTIO_NET_F_HOST_ECN
VIRTIO_NET_F_GUEST_ECN
VIRTIO_NET_F_GUEST_TSO6
VIRTIO_NET_F_GUEST_TSO4

Signed-off-by: Eli Cohen 
---
  drivers/vdpa/mlx5/net/mlx5_vnet.c | 56 ++-
  1 file changed, 40 insertions(+), 16 deletions(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 520646ae7fa0..5285dd76c793 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -778,12 +778,28 @@ static bool vq_is_tx(u16 idx)
return idx % 2;
  }
  
-static u16 get_features_12_3(u64 features)

+enum {
+   MLX5_VIRTIO_NET_F_MRG_RXBUF = 2,
+   MLX5_VIRTIO_NET_F_HOST_ECN = 4,
+   MLX5_VIRTIO_NET_F_GUEST_ECN = 6,
+   MLX5_VIRTIO_NET_F_GUEST_TSO6 = 7,
+   MLX5_VIRTIO_NET_F_GUEST_TSO4 = 8,
+   MLX5_VIRTIO_NET_F_GUEST_CSUM = 9,
+   MLX5_VIRTIO_NET_F_CSUM = 10,
+   MLX5_VIRTIO_NET_F_HOST_TSO6 = 11,
+   MLX5_VIRTIO_NET_F_HOST_TSO4 = 12,
+};
+
+static u16 get_features(u64 features)
  {
-   return (!!(features & BIT_ULL(VIRTIO_NET_F_HOST_TSO4)) << 9) |
-  (!!(features & BIT_ULL(VIRTIO_NET_F_HOST_TSO6)) << 8) |
-  (!!(features & BIT_ULL(VIRTIO_NET_F_CSUM)) << 7) |
-  (!!(features & BIT_ULL(VIRTIO_NET_F_GUEST_CSUM)) << 6);
+   return (!!(features & BIT_ULL(VIRTIO_NET_F_MRG_RXBUF)) << 
MLX5_VIRTIO_NET_F_MRG_RXBUF) |
+  (!!(features & BIT_ULL(VIRTIO_NET_F_HOST_ECN)) << 
MLX5_VIRTIO_NET_F_HOST_ECN) |
+  (!!(features & BIT_ULL(VIRTIO_NET_F_GUEST_ECN)) << 
MLX5_VIRTIO_NET_F_GUEST_ECN) |
+  (!!(features & BIT_ULL(VIRTIO_NET_F_GUEST_TSO6)) << 
MLX5_VIRTIO_NET_F_GUEST_TSO6) |
+  (!!(features & BIT_ULL(VIRTIO_NET_F_GUEST_TSO4)) << 
MLX5_VIRTIO_NET_F_GUEST_TSO4) |
+  (!!(features & BIT_ULL(VIRTIO_NET_F_CSUM)) << 
MLX5_VIRTIO_NET_F_CSUM) |
+  (!!(features & BIT_ULL(VIRTIO_NET_F_HOST_TSO6)) << 
MLX5_VIRTIO_NET_F_HOST_TSO6) |
+  (!!(features & BIT_ULL(VIRTIO_NET_F_HOST_TSO4)) << 
MLX5_VIRTIO_NET_F_HOST_TSO4);
  }
  
  static bool counters_supported(const struct mlx5_vdpa_dev *mvdev)

@@ -797,6 +813,7 @@ static int create_virtqueue(struct mlx5_vdpa_net *ndev, 
struct mlx5_vdpa_virtque
int inlen = MLX5_ST_SZ_BYTES(create_virtio_net_q_in);
u32 out[MLX5_ST_SZ_DW(create_virtio_net_q_out)] = {};
void *obj_context;
+   u16 mlx_features;
void *cmd_hdr;
void *vq_ctx;
void *in;
@@ -812,6 +829,7 @@ static int create_virtqueue(struct mlx5_vdpa_net *ndev, 
struct mlx5_vdpa_virtque
goto err_alloc;
}
  
+	mlx_features = get_features(ndev->mvdev.actual_features);

cmd_hdr = MLX5_ADDR_OF(create_virtio_net_q_in, in, 
general_obj_in_cmd_hdr);
  
  	MLX5_SET(general_obj_in_cmd_hdr, cmd_hdr, opcode, MLX5_CMD_OP_CREATE_GENERAL_OBJECT);

@@ -822,7 +840,9 @@ static int create_virtqueue(struct mlx5_vdpa_net *ndev, 
struct mlx5_vdpa_virtque
MLX5_SET(virtio_net_q_object, obj_context, hw_available_index, 
mvq->avail_idx);
MLX5_SET(virtio_net_q_object, obj_context, hw_used_index, 
mvq->used_idx);
MLX5_SET(virtio_net_q_object, obj_context, queue_feature_bit_mask_12_3,
-get_features_12_3(ndev->mvdev.actual_features));
+mlx_features >> 3);
+   MLX5_SET(virtio_net_q_object, obj_context, queue_feature_bit_mask_2_0,
+mlx_features & 7);
vq_ctx = MLX5_ADDR_OF(virtio_net_q_object, obj_context, 
virtio_q_context);
MLX5_SET(virtio_q, vq_ctx, virtio_q_type, get_queue_type(ndev));
  
@@ -2171,23 +2191,27 @@ static u32 mlx5_vdpa_get_vq_group(struct vdpa_device *vdev, u16 idx)

return MLX5_VDPA_DATAVQ_GROUP;
  }
  
-enum { MLX5_VIRTIO_NET_F_GUEST_CSUM = 1 << 9,

-   MLX5_VIRTIO_NET_F_CSUM = 1 << 10,
-   MLX5_VIRTIO_NET_F_HOST_TSO6 = 1 << 11,
-   MLX5_VIRTIO_NET_F_HOST_TSO4 = 1 << 12,
-};
-
  static u64 mlx_to_vritio_features(u16 dev_features)
  {
u64 result = 0;
  
-	if (dev_features & MLX5_VIRTIO_NET_F_GUEST_CSUM)

+   if (dev_features & BIT_ULL(MLX5_VIRTIO_NET_F_MRG_RXBUF))
+   result += BIT_ULL(VIRTIO_NET_F_MRG_RXBUF);
+   if (dev_features & BIT_ULL(MLX5_VIRTIO_NET_F_HOST_ECN))
+   result += BIT_ULL(VIRTIO_NET_F_HOST_ECN);
+   if (dev_features & BIT_ULL(MLX5_VIRTIO_NET_F_GUEST_ECN))
+   result += BIT_ULL(VIRTIO_NET_F_GUEST_ECN);
+   if (dev_features & BIT_ULL(MLX5_VIRTIO_NET_F_GUEST_TSO6))
+   result += BIT_ULL(VIRTIO_NET_F_GUEST_TSO6);

Re: [PATCH v2 1/2] vdpa/mlx5: Extend driver support for new features

2023-03-20 Thread Si-Wei Liu



On 3/19/2023 11:44 PM, Jason Wang wrote:

On Fri, Mar 17, 2023 at 9:58 PM Parav Pandit  wrote:




From: Eli Cohen 
Sent: Wednesday, March 15, 2023 3:28 AM

Extend the possible list for features that can be supported by firmware.
Note that different versions of firmware may or may not support these
features. The driver is made aware of them by querying the firmware.

While doing this, improve the code so we use enum names instead of hard
coded numerical values.

The new features supported by the driver are the following:

VIRTIO_NET_F_MRG_RXBUF
VIRTIO_NET_F_HOST_UFO

UFO is deprecated in Linux kernel, there are no known user either and we do not 
plan to support it.

Note that there's an emulation code for preserving migration
compatibility in the kernel.
I wonder if there's a command line option to prohibit QEMU from saving 
this host capability to the migration stream? If not I think it's a 
nightmare every vendor has to support already-deprecated UFO in their 
h/w device.


Thanks,
-Siwei



Please remove this entry along with below GUEST_UFO.

If there's no plan for supporting migration from existing software
backends, we can remove this.

Thanks


VIRTIO_NET_F_HOST_ECN
VIRTIO_NET_F_GUEST_UFO
VIRTIO_NET_F_GUEST_ECN
VIRTIO_NET_F_GUEST_TSO6
VIRTIO_NET_F_GUEST_TSO4


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

Re: [PATCH] vdpa/mlx5: Remove debugfs file after device unregister

2023-03-20 Thread Jason Wang
On Mon, Mar 20, 2023 at 5:35 PM Eli Cohen  wrote:
>
>
> On 20/03/2023 11:28, Jason Wang wrote:
> > On Mon, Mar 20, 2023 at 5:07 PM Eli Cohen  wrote:
> >> On 17/03/2023 5:32, Jason Wang wrote:
> >>> On Sun, Mar 12, 2023 at 4:41 PM Eli Cohen  wrote:
>  When deleting the vdpa device, the debugfs files need to be removed so
>  need to remove debugfs after the device has been unregistered.
> 
>  This fixes null pointer dereference when someone deletes the device
>  after debugfs has been populated.
> 
>  Fixes: 294221004322 ("vdpa/mlx5: Add debugfs subtree")
>  Signed-off-by: Eli Cohen 
>  ---
> 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 3858ba1e8975..3f6149f2ffd4 100644
>  --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
>  +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
>  @@ -3322,8 +3322,6 @@ static void mlx5_vdpa_dev_del(struct vdpa_mgmt_dev 
>  *v_mdev, struct vdpa_device *
>    struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
>    struct workqueue_struct *wq;
> 
>  -   mlx5_vdpa_remove_debugfs(ndev->debugfs);
>  -   ndev->debugfs = NULL;
>    if (ndev->nb_registered) {
>    ndev->nb_registered = false;
>    mlx5_notifier_unregister(mvdev->mdev, >nb);
>  @@ -3332,6 +3330,8 @@ static void mlx5_vdpa_dev_del(struct vdpa_mgmt_dev 
>  *v_mdev, struct vdpa_device *
>    mvdev->wq = NULL;
>    destroy_workqueue(wq);
>    _vdpa_unregister_device(dev);
> >>> What if the user tries to access debugfs after _vdpa_unregister_device()?
> >> I don't see an issue with that. During the process of deleting a device,
> >> the resources are destroyed and their corresponding debugfs files are
> >> also destroyed just prior to destroying the resource.
> > For example, rx_flow_table_show() requires the structure mlx5_vdpa_net
> > alive, but it seems the structure has been freed after
> > _vdpa_unregister_device().
> But in this case, rx_flow_table_show() gets called first, so the file
> will be removed from the debugfs before
>
> mlx5_vdpa_net gets released.

Just to make sure we are at the same page. I meant:

[CPU 0] _vdpa_unregister_device(dev);
[CPU 1] rx_flow_table_show()
[CPU 0] mlx5_vdpa_remove_debugfs()

So when CPU 1 tries to access the flow table, the ndev has been
released by CPU0 in _vdpa_unregister_device()?

Thanks

>
> >
> > If a user tries to access that file after _vdpa_unregister_device()
> > but before mlx5_vdpa_remove_debugfs(), will we end up with
> > use-after-free?
> >
> > Thanks
> >
> >
> >>> Thanks
> >>>
>  +   mlx5_vdpa_remove_debugfs(ndev->debugfs);
>  +   ndev->debugfs = NULL;
>    mgtdev->ndev = NULL;
> }
> 
>  --
>  2.38.1
> 
>

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

Re: [PATCH v2 0/7] vhost-scsi: Fix crashes and management op hangs

2023-03-20 Thread michael . christie
On 3/20/23 9:06 PM, Mike Christie wrote:
> The following patches were made over Linus tree.

Hi Michael, I see you merged my first version of the patchset in your
vhost branch.

Do you want me to just send a followup patchset?

The major diff between the 2 versions:

1. I added the first 2 patches which fix some bugs in the existing code
I found while doing some code review and testing another LIO patchset
plus v1.

Note: The other day I posted that I thought the 3rd patch in v1 caused
the bugs but they were already in the code.

2. In v2 I made one of the patches not need the vhost device lock when
unmapping/mapping LUNs, so you can add new LUNs even if one LUN on the same
vhost_scsi device was hung.

Since it's not regressions with the existing patches, I can just send those
as a followup patchset if that's preferred.
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v2] virtio: add VIRTIO_F_NOTIFICATION_DATA feature support

2023-03-20 Thread Jason Wang
On Tue, Mar 21, 2023 at 7:21 AM Viktor Prutyanov  wrote:
>
> According to VirtIO spec v1.2, VIRTIO_F_NOTIFICATION_DATA feature
> indicates that the driver passes extra data along with the queue
> notifications.
>
> In a split queue case, the extra data is 16-bit available index. In a
> packed queue case, the extra data is 1-bit wrap counter and 15-bit
> available index.
>
> Add support for this feature for MMIO and PCI transports. Channel I/O
> transport will not accept this feature.
>
> Signed-off-by: Viktor Prutyanov 
> ---
>
>  v2: reject the feature in virtio_ccw, replace __le32 with u32
>
>  drivers/s390/virtio/virtio_ccw.c   |  4 +---
>  drivers/virtio/virtio_mmio.c   | 15 ++-
>  drivers/virtio/virtio_pci_common.c | 10 ++
>  drivers/virtio/virtio_pci_common.h |  4 
>  drivers/virtio/virtio_pci_legacy.c |  2 +-
>  drivers/virtio/virtio_pci_modern.c |  2 +-
>  drivers/virtio/virtio_ring.c   | 17 +
>  include/linux/virtio_ring.h|  2 ++
>  include/uapi/linux/virtio_config.h |  6 ++
>  9 files changed, 56 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/s390/virtio/virtio_ccw.c 
> b/drivers/s390/virtio/virtio_ccw.c
> index a10dbe632ef9..d72a59415527 100644
> --- a/drivers/s390/virtio/virtio_ccw.c
> +++ b/drivers/s390/virtio/virtio_ccw.c
> @@ -789,9 +789,7 @@ static u64 virtio_ccw_get_features(struct virtio_device 
> *vdev)
>
>  static void ccw_transport_features(struct virtio_device *vdev)
>  {
> -   /*
> -* Currently nothing to do here.
> -*/
> +   __virtio_clear_bit(vdev, VIRTIO_F_NOTIFICATION_DATA);

Is there any restriction that prevents us from implementing
VIRTIO_F_NOTIFICATION_DATA? (Spec seems doesn't limit us from this)

>  }
>
>  static int virtio_ccw_finalize_features(struct virtio_device *vdev)
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index 3ff746e3f24a..0e13da17fe0a 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -285,6 +285,19 @@ static bool vm_notify(struct virtqueue *vq)
> return true;
>  }
>
> +static bool vm_notify_with_data(struct virtqueue *vq)
> +{
> +   struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vq->vdev);
> +   u32 data = vring_fill_notification_data(vq);

Can we move this to the initialization?

Thanks

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

Re: [PATCH] virtio: add VIRTIO_F_NOTIFICATION_DATA feature support

2023-03-20 Thread Xuan Zhuo
On Mon, 20 Mar 2023 14:54:51 +0300, Viktor Prutyanov  wrote:
> According to VirtIO spec v1.2, VIRTIO_F_NOTIFICATION_DATA feature
> indicates that the driver passes extra data along with the queue
> notifications.
>
> In a split queue case, the extra data is 16-bit available index. In a
> packed queue case, the extra data is 1-bit wrap counter and 15-bit
> available index.
>
> Add support for this feature for both MMIO and PCI.
>
> Signed-off-by: Viktor Prutyanov 
> ---
>  drivers/virtio/virtio_mmio.c   | 15 ++-
>  drivers/virtio/virtio_pci_common.c | 10 ++
>  drivers/virtio/virtio_pci_common.h |  4 
>  drivers/virtio/virtio_pci_legacy.c |  2 +-
>  drivers/virtio/virtio_pci_modern.c |  2 +-
>  drivers/virtio/virtio_ring.c   | 17 +
>  include/linux/virtio_ring.h|  2 ++
>  include/uapi/linux/virtio_config.h |  6 ++
>  8 files changed, 55 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index 3ff746e3f24a..05da5ad7fc93 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -285,6 +285,19 @@ static bool vm_notify(struct virtqueue *vq)
>   return true;
>  }
>
> +static bool vm_notify_with_data(struct virtqueue *vq)
> +{
> + struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vq->vdev);
> + __le32 data = vring_fill_notification_data(vq);
> +
> + writel(data, vm_dev->base + VIRTIO_MMIO_QUEUE_NOTIFY);
> +
> + return true;
> +}
> +
> +#define VM_NOTIFY(vdev) (__virtio_test_bit((vdev), 
> VIRTIO_F_NOTIFICATION_DATA) \
> + ? vm_notify_with_data : vm_notify)


Is this macro necessary, it is only used once. And I don't recognize that this
logic is necessary to use macro.

> +
>  /* Notify all virtqueues on an interrupt. */
>  static irqreturn_t vm_interrupt(int irq, void *opaque)
>  {
> @@ -397,7 +410,7 @@ static struct virtqueue *vm_setup_vq(struct virtio_device 
> *vdev, unsigned int in
>
>   /* Create the vring */
>   vq = vring_create_virtqueue(index, num, VIRTIO_MMIO_VRING_ALIGN, vdev,
> -  true, true, ctx, vm_notify, callback, name);
> + true, true, ctx, VM_NOTIFY(vdev), callback, name);
>   if (!vq) {
>   err = -ENOMEM;
>   goto error_new_virtqueue;
> diff --git a/drivers/virtio/virtio_pci_common.c 
> b/drivers/virtio/virtio_pci_common.c
> index a6c86f916dbd..bf7daad9ce65 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -43,6 +43,16 @@ bool vp_notify(struct virtqueue *vq)
>   /* we write the queue's selector into the notification register to
>* signal the other end */
>   iowrite16(vq->index, (void __iomem *)vq->priv);
> +
> + return true;
> +}
> +
> +bool vp_notify_with_data(struct virtqueue *vq)
> +{
> + __le32 data = vring_fill_notification_data(vq);
> +
> + iowrite32(data, (void __iomem *)vq->priv);
> +
>   return true;
>  }
>
> diff --git a/drivers/virtio/virtio_pci_common.h 
> b/drivers/virtio/virtio_pci_common.h
> index 23112d84218f..9a7212dcbb32 100644
> --- a/drivers/virtio/virtio_pci_common.h
> +++ b/drivers/virtio/virtio_pci_common.h
> @@ -105,6 +105,7 @@ static struct virtio_pci_device *to_vp_device(struct 
> virtio_device *vdev)
>  void vp_synchronize_vectors(struct virtio_device *vdev);
>  /* the notify function used when creating a virt queue */
>  bool vp_notify(struct virtqueue *vq);
> +bool vp_notify_with_data(struct virtqueue *vq);
>  /* the config->del_vqs() implementation */
>  void vp_del_vqs(struct virtio_device *vdev);
>  /* the config->find_vqs() implementation */
> @@ -114,6 +115,9 @@ int vp_find_vqs(struct virtio_device *vdev, unsigned int 
> nvqs,
>   struct irq_affinity *desc);
>  const char *vp_bus_name(struct virtio_device *vdev);
>
> +#define VP_NOTIFY(vdev) (__virtio_test_bit((vdev), 
> VIRTIO_F_NOTIFICATION_DATA) \
> + ? vp_notify : vp_notify_with_data)
> +

I also think that this is not necessary, although it has been used twice.


>  /* Setup the affinity for a virtqueue:
>   * - force the affinity for per vq vector
>   * - OR over all affinities for shared MSI
> diff --git a/drivers/virtio/virtio_pci_legacy.c 
> b/drivers/virtio/virtio_pci_legacy.c
> index 2257f1b3d8ae..b98e994cae48 100644
> --- a/drivers/virtio/virtio_pci_legacy.c
> +++ b/drivers/virtio/virtio_pci_legacy.c
> @@ -131,7 +131,7 @@ static struct virtqueue *setup_vq(struct 
> virtio_pci_device *vp_dev,
>   vq = vring_create_virtqueue(index, num,
>   VIRTIO_PCI_VRING_ALIGN, _dev->vdev,
>   true, false, ctx,
> - vp_notify, callback, name);
> + VP_NOTIFY(_dev->vdev), callback, name);
>   if (!vq)
>   return ERR_PTR(-ENOMEM);
>
> diff --git a/drivers/virtio/virtio_pci_modern.c 
> b/drivers/virtio/virtio_pci_modern.c

[PATCH v2 6/7] vhost-scsi: Drop vhost_scsi_mutex use in port callouts

2023-03-20 Thread Mike Christie
We are using the vhost_scsi_mutex to make sure vhost_scsi_port_link and
vhost_scsi_port_unlink see if vhost_scsi_clear_endpoint has cleared
tpg->vhost_scsi and it can't be freed while they are using.
 
However, we currently set the tpg->vhost_scsi pointer while holding
tv_tpg_mutex. So, we can just hold that while calling
vhost_scsi_hotplug/hotunplug. We then don't need to hold the
vhost_scsi_mutex while vhost_scsi_clear_endpoint is holding it and doing
a flush which could cause the LUN map/unmap to have to wait on another
device's flush.

Signed-off-by: Mike Christie 
---
 drivers/vhost/scsi.c | 14 ++
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index ba8097fcea43..d4372a4aff49 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -2040,15 +2040,10 @@ static int vhost_scsi_port_link(struct se_portal_group 
*se_tpg,
struct vhost_scsi_tpg *tpg = container_of(se_tpg,
struct vhost_scsi_tpg, se_tpg);
 
-   mutex_lock(_scsi_mutex);
-
mutex_lock(>tv_tpg_mutex);
tpg->tv_tpg_port_count++;
-   mutex_unlock(>tv_tpg_mutex);
-
vhost_scsi_hotplug(tpg, lun);
-
-   mutex_unlock(_scsi_mutex);
+   mutex_unlock(>tv_tpg_mutex);
 
return 0;
 }
@@ -2059,15 +2054,10 @@ static void vhost_scsi_port_unlink(struct 
se_portal_group *se_tpg,
struct vhost_scsi_tpg *tpg = container_of(se_tpg,
struct vhost_scsi_tpg, se_tpg);
 
-   mutex_lock(_scsi_mutex);
-
mutex_lock(>tv_tpg_mutex);
tpg->tv_tpg_port_count--;
-   mutex_unlock(>tv_tpg_mutex);
-
vhost_scsi_hotunplug(tpg, lun);
-
-   mutex_unlock(_scsi_mutex);
+   mutex_unlock(>tv_tpg_mutex);
 }
 
 static ssize_t vhost_scsi_tpg_attrib_fabric_prot_type_store(
-- 
2.25.1

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


[PATCH v2 7/7] vhost-scsi: Reduce vhost_scsi_mutex use

2023-03-20 Thread Mike Christie
We on longer need to hold the vhost_scsi_mutex the entire time we
set/clear the endpoint. The tv_tpg_mutex handles tpg accesses not related
to the tpg list, the port link/unlink functions use the tv_tpg_mutex while
accessing the tpg->vhost_scsi pointer, vhost_scsi_do_plug will no longer
queue events after the virtqueue's backend has been cleared and flushed,
and we don't drop our refcount to the tpg until after we have stopped
cmds and wait for outstanding cmds to complete.

This moves the vhost_scsi_mutex use to it's documented use of being used
to access the tpg list. We then don't need to hold it while a flush is
being performed causing other device's vhost_scsi_set_endpoint
and vhost_scsi_make_tpg/vhost_scsi_drop_tpg calls to have to wait on a
flakey device.

Signed-off-by: Mike Christie 
---
 drivers/vhost/scsi.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index d4372a4aff49..3b0b556c57ef 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -229,7 +229,10 @@ struct vhost_scsi_ctx {
struct iov_iter out_iter;
 };
 
-/* Global spinlock to protect vhost_scsi TPG list for vhost IOCTL access */
+/*
+ * Global mutex to protect vhost_scsi TPG list for vhost IOCTLs and LIO
+ * configfs management operations.
+ */
 static DEFINE_MUTEX(vhost_scsi_mutex);
 static LIST_HEAD(vhost_scsi_list);
 
@@ -1526,7 +1529,7 @@ static int vhost_scsi_setup_vq_cmds(struct 
vhost_virtqueue *vq, int max_cmds)
  * vhost_scsi_tpg with an active struct vhost_scsi_nexus
  *
  *  The lock nesting rule is:
- *vhost_scsi_mutex -> vs->dev.mutex -> tpg->tv_tpg_mutex -> vq->mutex
+ *vs->dev.mutex -> vhost_scsi_mutex -> tpg->tv_tpg_mutex -> vq->mutex
  */
 static int
 vhost_scsi_set_endpoint(struct vhost_scsi *vs,
@@ -1540,7 +1543,6 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs,
int index, ret, i, len;
bool match = false;
 
-   mutex_lock(_scsi_mutex);
mutex_lock(>dev.mutex);
 
/* Verify that ring has been setup correctly. */
@@ -1561,6 +1563,7 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs,
if (vs->vs_tpg)
memcpy(vs_tpg, vs->vs_tpg, len);
 
+   mutex_lock(_scsi_mutex);
list_for_each_entry(tpg, _scsi_list, tv_tpg_list) {
mutex_lock(>tv_tpg_mutex);
if (!tpg->tpg_nexus) {
@@ -1576,6 +1579,7 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs,
if (!strcmp(tv_tport->tport_name, t->vhost_wwpn)) {
if (vs->vs_tpg && vs->vs_tpg[tpg->tport_tpgt]) {
mutex_unlock(>tv_tpg_mutex);
+   mutex_unlock(_scsi_mutex);
ret = -EEXIST;
goto undepend;
}
@@ -1590,6 +1594,7 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs,
if (ret) {
pr_warn("target_depend_item() failed: %d\n", 
ret);
mutex_unlock(>tv_tpg_mutex);
+   mutex_unlock(_scsi_mutex);
goto undepend;
}
tpg->tv_tpg_vhost_count++;
@@ -1599,6 +1604,7 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs,
}
mutex_unlock(>tv_tpg_mutex);
}
+   mutex_unlock(_scsi_mutex);
 
if (match) {
memcpy(vs->vs_vhost_wwpn, t->vhost_wwpn,
@@ -1654,7 +1660,6 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs,
kfree(vs_tpg);
 out:
mutex_unlock(>dev.mutex);
-   mutex_unlock(_scsi_mutex);
return ret;
 }
 
@@ -1670,7 +1675,6 @@ vhost_scsi_clear_endpoint(struct vhost_scsi *vs,
int index, ret, i;
u8 target;
 
-   mutex_lock(_scsi_mutex);
mutex_lock(>dev.mutex);
/* Verify that ring has been setup correctly. */
for (index = 0; index < vs->dev.nvqs; ++index) {
@@ -1757,12 +1761,10 @@ vhost_scsi_clear_endpoint(struct vhost_scsi *vs,
vs->vs_tpg = NULL;
WARN_ON(vs->vs_events_nr);
mutex_unlock(>dev.mutex);
-   mutex_unlock(_scsi_mutex);
return 0;
 
 err_dev:
mutex_unlock(>dev.mutex);
-   mutex_unlock(_scsi_mutex);
return ret;
 }
 
-- 
2.25.1

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


[PATCH v2 5/7] vhost-scsi: Check for a cleared backend before queueing an event

2023-03-20 Thread Mike Christie
We currenly hold the vhost_scsi_mutex while clearing the endpoint and
while performing vhost_scsi_do_plug, so tpg->vhost_scsi can't be freed
from uder us, and to make sure anything queued is handled by the
full call in vhost_scsi_clear_endpoint.

This patch removes the need for the vhost_scsi_mutex for the latter
case. In the next patches, we won't hold the vhost_scsi_mutex while
flushing so this patch adds a check for the clearing of the virtqueue
from vhost_scsi_clear_endpoint. We then know that once
vhost_scsi_clear_endpoint has cleared the backend that no new events
will be queued, and the flush after the vhost_vq_set_backend(vq, NULL)
call will see everything that's been queued to that point. So the flush
will then handle all events without the need for the vhost_scsi_mutex.

Signed-off-by: Mike Christie 
---
 drivers/vhost/scsi.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index c945136ecf18..ba8097fcea43 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -2010,9 +2010,17 @@ vhost_scsi_do_plug(struct vhost_scsi_tpg *tpg,
 
vq = >vqs[VHOST_SCSI_VQ_EVT].vq;
mutex_lock(>mutex);
+   /*
+* We can't queue events if the backend has been cleared, because
+* we could end up queueing an event after the flush.
+*/
+   if (!vhost_vq_get_backend(vq))
+   goto unlock;
+
if (vhost_has_feature(vq, VIRTIO_SCSI_F_HOTPLUG))
vhost_scsi_send_evt(vs, tpg, lun,
   VIRTIO_SCSI_T_TRANSPORT_RESET, reason);
+unlock:
mutex_unlock(>mutex);
 }
 
-- 
2.25.1

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


[PATCH v2 4/7] vhost-scsi: Drop device mutex use in vhost_scsi_do_plug

2023-03-20 Thread Mike Christie
We don't need the device mutex in vhost_scsi_do_plug because:
1. we have the vhost_scsi_mutex so the tpg->vhost_scsi pointer will not
change on us and the vhost_scsi can't be freed from under us if it was
set.
2. vhost_scsi_clear_endpoint will stop the virtqueues and flush them while
holding the vhost_scsi_mutex so we know once vhost_scsi_clear_endpoint
has completed that vhost_scsi_do_plug can't send new events and any
queued ones have completed.

So this patch drops the device mutex use in vhost_scsi_do_plug.

Signed-off-by: Mike Christie 
---
 drivers/vhost/scsi.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 502d6803df0b..c945136ecf18 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -2003,8 +2003,6 @@ vhost_scsi_do_plug(struct vhost_scsi_tpg *tpg,
if (!vs)
return;
 
-   mutex_lock(>dev.mutex);
-
if (plug)
reason = VIRTIO_SCSI_EVT_RESET_RESCAN;
else
@@ -2016,7 +2014,6 @@ vhost_scsi_do_plug(struct vhost_scsi_tpg *tpg,
vhost_scsi_send_evt(vs, tpg, lun,
   VIRTIO_SCSI_T_TRANSPORT_RESET, reason);
mutex_unlock(>mutex);
-   mutex_unlock(>dev.mutex);
 }
 
 static void vhost_scsi_hotplug(struct vhost_scsi_tpg *tpg, struct se_lun *lun)
-- 
2.25.1

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


[PATCH v2 2/7] vhost-scsi: Fix crash during LUN unmapping

2023-03-20 Thread Mike Christie
We normally clear the endpoint then unmap LUNs so the devices are fully
shutdown when the LUN is unmapped, but it's legal to unmap before
clearing. If the user does that while TMFs are running then we can end
up crashing.

vhost_scsi_port_unlink assumes that the LUN's tmf struct will always be on
the tmf_queue list. However, if a TMF is running then it will have been
removed while it's executing. If we do a LUN unmap at this time, then
we assume the entry is on the list and just start accessing it and free
it.

This fixes the bug by just allocating the vhost_scsi_tmf struct when it's
needed like is done with the se_tmr struct that's needed when we submit
the TMF. In this path perf is not an issue and we can use GFP_KERNEL
since it won't swing directly back on us, so we don't need to preallocate
the struct.

Signed-off-by: Mike Christie 
---
 drivers/vhost/scsi.c | 36 
 1 file changed, 4 insertions(+), 32 deletions(-)

diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 5875241e1654..32d0be968103 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -125,7 +125,6 @@ struct vhost_scsi_tpg {
struct se_portal_group se_tpg;
/* Pointer back to vhost_scsi, protected by tv_tpg_mutex */
struct vhost_scsi *vhost_scsi;
-   struct list_head tmf_queue;
 };
 
 struct vhost_scsi_tport {
@@ -206,10 +205,8 @@ struct vhost_scsi {
 
 struct vhost_scsi_tmf {
struct vhost_work vwork;
-   struct vhost_scsi_tpg *tpg;
struct vhost_scsi *vhost;
struct vhost_scsi_virtqueue *svq;
-   struct list_head queue_entry;
 
struct se_cmd se_cmd;
u8 scsi_resp;
@@ -352,12 +349,9 @@ static void vhost_scsi_release_cmd_res(struct se_cmd 
*se_cmd)
 
 static void vhost_scsi_release_tmf_res(struct vhost_scsi_tmf *tmf)
 {
-   struct vhost_scsi_tpg *tpg = tmf->tpg;
struct vhost_scsi_inflight *inflight = tmf->inflight;
 
-   mutex_lock(>tv_tpg_mutex);
-   list_add_tail(>tmf_queue, >queue_entry);
-   mutex_unlock(>tv_tpg_mutex);
+   kfree(tmf);
vhost_scsi_put_inflight(inflight);
 }
 
@@ -1194,19 +1188,11 @@ vhost_scsi_handle_tmf(struct vhost_scsi *vs, struct 
vhost_scsi_tpg *tpg,
goto send_reject;
}
 
-   mutex_lock(>tv_tpg_mutex);
-   if (list_empty(>tmf_queue)) {
-   pr_err("Missing reserve TMF. Could not handle LUN RESET.\n");
-   mutex_unlock(>tv_tpg_mutex);
+   tmf = kzalloc(sizeof(*tmf), GFP_KERNEL);
+   if (!tmf)
goto send_reject;
-   }
-
-   tmf = list_first_entry(>tmf_queue, struct vhost_scsi_tmf,
-  queue_entry);
-   list_del_init(>queue_entry);
-   mutex_unlock(>tv_tpg_mutex);
 
-   tmf->tpg = tpg;
+   vhost_work_init(>vwork, vhost_scsi_tmf_resp_work);
tmf->vhost = vs;
tmf->svq = svq;
tmf->resp_iov = vq->iov[vc->out];
@@ -2035,19 +2021,11 @@ static int vhost_scsi_port_link(struct se_portal_group 
*se_tpg,
 {
struct vhost_scsi_tpg *tpg = container_of(se_tpg,
struct vhost_scsi_tpg, se_tpg);
-   struct vhost_scsi_tmf *tmf;
-
-   tmf = kzalloc(sizeof(*tmf), GFP_KERNEL);
-   if (!tmf)
-   return -ENOMEM;
-   INIT_LIST_HEAD(>queue_entry);
-   vhost_work_init(>vwork, vhost_scsi_tmf_resp_work);
 
mutex_lock(_scsi_mutex);
 
mutex_lock(>tv_tpg_mutex);
tpg->tv_tpg_port_count++;
-   list_add_tail(>queue_entry, >tmf_queue);
mutex_unlock(>tv_tpg_mutex);
 
vhost_scsi_hotplug(tpg, lun);
@@ -2062,16 +2040,11 @@ static void vhost_scsi_port_unlink(struct 
se_portal_group *se_tpg,
 {
struct vhost_scsi_tpg *tpg = container_of(se_tpg,
struct vhost_scsi_tpg, se_tpg);
-   struct vhost_scsi_tmf *tmf;
 
mutex_lock(_scsi_mutex);
 
mutex_lock(>tv_tpg_mutex);
tpg->tv_tpg_port_count--;
-   tmf = list_first_entry(>tmf_queue, struct vhost_scsi_tmf,
-  queue_entry);
-   list_del(>queue_entry);
-   kfree(tmf);
mutex_unlock(>tv_tpg_mutex);
 
vhost_scsi_hotunplug(tpg, lun);
@@ -2332,7 +2305,6 @@ vhost_scsi_make_tpg(struct se_wwn *wwn, const char *name)
}
mutex_init(>tv_tpg_mutex);
INIT_LIST_HEAD(>tv_tpg_list);
-   INIT_LIST_HEAD(>tmf_queue);
tpg->tport = tport;
tpg->tport_tpgt = tpgt;
 
-- 
2.25.1

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


[PATCH v2 0/7] vhost-scsi: Fix crashes and management op hangs

2023-03-20 Thread Mike Christie
The following patches were made over Linus tree. The patches fix 3
issues:

1. If a user performs LIO LUN unmapping before the endpoint has been
cleared then we can end up trying to free a bogus tmf struct if the
TMF is still exucuting when we do the unmap.
2. If vhost_scsi_setup_vq_cmds fails we can leave the tpg->vhost_scsi
pointer set and we can end up trying to access a freed struct.
3. Management operations like LUN mapping/unmapping and device addition
hang for 30 seconds or up to N minutes depending on the device.

The problem is that we use a global mutex to protect the list of tpgs
and for accessing the tpg, and to make sure they are flushed. We then
hold that mutex during a lot of management operations. So if you
are just trying to add another device, it will have to wait on another
device if we are in the middle of clearing it's endpoint and it's
waiting on hung IO.

This patchset fixes up the ordering of how we flush IO and release
refcounts and how often the global mutex is used so we don't need to
always hold it

v2:
1. Added fix for possible use after free and merge with a locking
cleanup patch.
2. Added fix for LIO LUN unmap during TMF execution bug.
3. Fixed bug where we needed to hold the tpg mutex instead of the
vhost_scsi_mutex when calling vhost_scsi_do_plug.


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


[PATCH v2 1/7] vhost-scsi: Fix vhost_scsi struct use after free

2023-03-20 Thread Mike Christie
If vhost_scsi_setup_vq_cmds fails we leave the tpg->vhost_scsi pointer
set. If the device is freed and then the user unmaps the LUN, the call to
vhost_scsi_port_unlink -> vhost_scsi_hotunplug will see the that
tpg->vhost_scsi is still set and try to use it.

This has us clear the vhost_scsi pointer in the failure path. It also
has us take tv_tpg_mutex in this failure path, because tv_tpg_vhost_count
is accessed under this mutex in vhost_scsi_drop_nexus and in the future
we will want to serialize access to tpg->vhost_scsi with that mutex
instead of the vhost_scsi_mutex.

Signed-off-by: Mike Christie 
---
 drivers/vhost/scsi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index b244e7c0f514..5875241e1654 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -1658,7 +1658,10 @@ vhost_scsi_set_endpoint(struct vhost_scsi *vs,
for (i = 0; i < VHOST_SCSI_MAX_TARGET; i++) {
tpg = vs_tpg[i];
if (tpg) {
+   mutex_lock(>tv_tpg_mutex);
+   tpg->vhost_scsi = NULL;
tpg->tv_tpg_vhost_count--;
+   mutex_unlock(>tv_tpg_mutex);
target_undepend_item(>se_tpg.tpg_group.cg_item);
}
}
-- 
2.25.1

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


[PATCH v2 3/7] vhost-scsi: Delay releasing our refcount on the tpg

2023-03-20 Thread Mike Christie
We currently hold the vhost_scsi_mutex the entire time we are running
vhost_scsi_clear_endpoint. One of the reasons for this is that it prevents
userspace from being able to free the se_tpg from under us after we have
called target_undepend_item. However, it forces management operations for
for other devices to have to wait on a flakey device's:

vhost_scsi_clear_endpoint -> vhost_scsi_flush()

call which can which can take a long time.

This moves the target_undepend_item call and the tpg unsetup code to after
we have stopped new IO from starting up and after we have waited on
running IO. We can then release our refcount on the tpg and session
knowing our device is no longer accessing them. We can then drop the
vhost_scsi_mutex use during thee flush call in later patches in this set,
when we have removed other reasons for holding it.

Signed-off-by: Mike Christie 
---
 drivers/vhost/scsi.c | 61 +++-
 1 file changed, 37 insertions(+), 24 deletions(-)

diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 32d0be968103..502d6803df0b 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -1691,11 +1691,10 @@ vhost_scsi_clear_endpoint(struct vhost_scsi *vs,
if (!tpg)
continue;
 
-   mutex_lock(>tv_tpg_mutex);
tv_tport = tpg->tport;
if (!tv_tport) {
ret = -ENODEV;
-   goto err_tpg;
+   goto err_dev;
}
 
if (strcmp(tv_tport->tport_name, t->vhost_wwpn)) {
@@ -1704,35 +1703,51 @@ vhost_scsi_clear_endpoint(struct vhost_scsi *vs,
tv_tport->tport_name, tpg->tport_tpgt,
t->vhost_wwpn, t->vhost_tpgt);
ret = -EINVAL;
-   goto err_tpg;
+   goto err_dev;
}
+   match = true;
+   }
+   if (!match)
+   goto free_vs_tpg;
+
+   /* Prevent new cmds from starting and accessing the tpgs/sessions */
+   for (i = 0; i < vs->dev.nvqs; i++) {
+   vq = >vqs[i].vq;
+   mutex_lock(>mutex);
+   vhost_vq_set_backend(vq, NULL);
+   mutex_unlock(>mutex);
+   }
+   /* Make sure cmds are not running before tearing them down. */
+   vhost_scsi_flush(vs);
+
+   for (i = 0; i < vs->dev.nvqs; i++) {
+   vq = >vqs[i].vq;
+   vhost_scsi_destroy_vq_cmds(vq);
+   }
+
+   /*
+* We can now release our hold on the tpg and sessions and userspace
+* can free them after this point.
+*/
+   for (i = 0; i < VHOST_SCSI_MAX_TARGET; i++) {
+   target = i;
+   tpg = vs->vs_tpg[target];
+   if (!tpg)
+   continue;
+
+   mutex_lock(>tv_tpg_mutex);
+
tpg->tv_tpg_vhost_count--;
tpg->vhost_scsi = NULL;
vs->vs_tpg[target] = NULL;
-   match = true;
+
mutex_unlock(>tv_tpg_mutex);
-   /*
-* Release se_tpg->tpg_group.cg_item configfs dependency now
-* to allow vhost-scsi WWPN se_tpg->tpg_group shutdown to occur.
-*/
+
se_tpg = >se_tpg;
target_undepend_item(_tpg->tpg_group.cg_item);
}
-   if (match) {
-   for (i = 0; i < vs->dev.nvqs; i++) {
-   vq = >vqs[i].vq;
-   mutex_lock(>mutex);
-   vhost_vq_set_backend(vq, NULL);
-   mutex_unlock(>mutex);
-   }
-   /* Make sure cmds are not running before tearing them down. */
-   vhost_scsi_flush(vs);
 
-   for (i = 0; i < vs->dev.nvqs; i++) {
-   vq = >vqs[i].vq;
-   vhost_scsi_destroy_vq_cmds(vq);
-   }
-   }
+free_vs_tpg:
/*
 * Act as synchronize_rcu to make sure access to
 * old vs->vs_tpg is finished.
@@ -1745,8 +1760,6 @@ vhost_scsi_clear_endpoint(struct vhost_scsi *vs,
mutex_unlock(_scsi_mutex);
return 0;
 
-err_tpg:
-   mutex_unlock(>tv_tpg_mutex);
 err_dev:
mutex_unlock(>dev.mutex);
mutex_unlock(_scsi_mutex);
-- 
2.25.1

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


Re: [PATCH] virtio: add VIRTIO_F_NOTIFICATION_DATA feature support

2023-03-20 Thread kernel test robot
Hi Viktor,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mst-vhost/linux-next]
[also build test WARNING on linus/master v6.3-rc3 next-20230320]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Viktor-Prutyanov/virtio-add-VIRTIO_F_NOTIFICATION_DATA-feature-support/20230320-195725
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
patch link:
https://lore.kernel.org/r/20230320115451.1232171-1-viktor%40daynix.com
patch subject: [PATCH] virtio: add VIRTIO_F_NOTIFICATION_DATA feature support
config: sparc64-randconfig-s031-20230319 
(https://download.01.org/0day-ci/archive/20230321/202303210740.w7riuizb-...@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 12.1.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# 
https://github.com/intel-lab-lkp/linux/commit/b6212a12ca1691dc346e5de046ec46bd3ce11247
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review 
Viktor-Prutyanov/virtio-add-VIRTIO_F_NOTIFICATION_DATA-feature-support/20230320-195725
git checkout b6212a12ca1691dc346e5de046ec46bd3ce11247
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 
CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=sparc64 
olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 
CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=sparc64 
SHELL=/bin/bash drivers/virtio/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot 
| Link: 
https://lore.kernel.org/oe-kbuild-all/202303210740.w7riuizb-...@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/virtio/virtio_pci_common.c:54:19: sparse: sparse: incorrect type in 
>> argument 1 (different base types) @@ expected unsigned int [usertype] l 
>> @@ got restricted __le32 [usertype] data @@
   drivers/virtio/virtio_pci_common.c:54:19: sparse: expected unsigned int 
[usertype] l
   drivers/virtio/virtio_pci_common.c:54:19: sparse: got restricted __le32 
[usertype] data

vim +54 drivers/virtio/virtio_pci_common.c

49  
50  bool vp_notify_with_data(struct virtqueue *vq)
51  {
52  __le32 data = vring_fill_notification_data(vq);
53  
  > 54  iowrite32(data, (void __iomem *)vq->priv);
55  
56  return true;
57  }
58  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] virtio: add VIRTIO_F_NOTIFICATION_DATA feature support

2023-03-20 Thread kernel test robot
Hi Viktor,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mst-vhost/linux-next]
[also build test WARNING on linus/master v6.3-rc3 next-20230320]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Viktor-Prutyanov/virtio-add-VIRTIO_F_NOTIFICATION_DATA-feature-support/20230320-195725
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
patch link:
https://lore.kernel.org/r/20230320115451.1232171-1-viktor%40daynix.com
patch subject: [PATCH] virtio: add VIRTIO_F_NOTIFICATION_DATA feature support
config: alpha-randconfig-s043-20230319 
(https://download.01.org/0day-ci/archive/20230321/202303210759.krnnnzb4-...@intel.com/config)
compiler: alpha-linux-gcc (GCC) 12.1.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# 
https://github.com/intel-lab-lkp/linux/commit/b6212a12ca1691dc346e5de046ec46bd3ce11247
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review 
Viktor-Prutyanov/virtio-add-VIRTIO_F_NOTIFICATION_DATA-feature-support/20230320-195725
git checkout b6212a12ca1691dc346e5de046ec46bd3ce11247
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 
CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=alpha olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 
CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=alpha 
SHELL=/bin/bash drivers/virtio/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot 
| Link: 
https://lore.kernel.org/oe-kbuild-all/202303210759.krnnnzb4-...@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/virtio/virtio_mmio.c:293:16: sparse: sparse: incorrect type in 
>> argument 1 (different base types) @@ expected unsigned int [usertype] b 
>> @@ got restricted __le32 [usertype] data @@
   drivers/virtio/virtio_mmio.c:293:16: sparse: expected unsigned int 
[usertype] b
   drivers/virtio/virtio_mmio.c:293:16: sparse: got restricted __le32 
[usertype] data

vim +293 drivers/virtio/virtio_mmio.c

   287  
   288  static bool vm_notify_with_data(struct virtqueue *vq)
   289  {
   290  struct virtio_mmio_device *vm_dev = 
to_virtio_mmio_device(vq->vdev);
   291  __le32 data = vring_fill_notification_data(vq);
   292  
 > 293  writel(data, vm_dev->base + VIRTIO_MMIO_QUEUE_NOTIFY);
   294  
   295  return true;
   296  }
   297  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] virtio: add VIRTIO_F_NOTIFICATION_DATA feature support

2023-03-20 Thread kernel test robot
Hi Viktor,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mst-vhost/linux-next]
[also build test WARNING on linus/master v6.3-rc3 next-20230320]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Viktor-Prutyanov/virtio-add-VIRTIO_F_NOTIFICATION_DATA-feature-support/20230320-195725
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
patch link:
https://lore.kernel.org/r/20230320115451.1232171-1-viktor%40daynix.com
patch subject: [PATCH] virtio: add VIRTIO_F_NOTIFICATION_DATA feature support
config: i386-randconfig-s002 
(https://download.01.org/0day-ci/archive/20230321/202303210515.zhhe1nmc-...@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# 
https://github.com/intel-lab-lkp/linux/commit/b6212a12ca1691dc346e5de046ec46bd3ce11247
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review 
Viktor-Prutyanov/virtio-add-VIRTIO_F_NOTIFICATION_DATA-feature-support/20230320-195725
git checkout b6212a12ca1691dc346e5de046ec46bd3ce11247
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir 
ARCH=i386 olddefconfig
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir 
ARCH=i386 SHELL=/bin/bash drivers/virtio/ net/bluetooth/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot 
| Link: 
https://lore.kernel.org/oe-kbuild-all/202303210515.zhhe1nmc-...@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/virtio/virtio_pci_common.c:54:19: sparse: sparse: incorrect type in 
>> argument 1 (different base types) @@ expected unsigned int [usertype] @@ 
>> got restricted __le32 [usertype] data @@
   drivers/virtio/virtio_pci_common.c:54:19: sparse: expected unsigned int 
[usertype]
   drivers/virtio/virtio_pci_common.c:54:19: sparse: got restricted __le32 
[usertype] data

vim +54 drivers/virtio/virtio_pci_common.c

49  
50  bool vp_notify_with_data(struct virtqueue *vq)
51  {
52  __le32 data = vring_fill_notification_data(vq);
53  
  > 54  iowrite32(data, (void __iomem *)vq->priv);
55  
56  return true;
57  }
58  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] virtio: add VIRTIO_F_NOTIFICATION_DATA feature support

2023-03-20 Thread kernel test robot
Hi Viktor,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mst-vhost/linux-next]
[also build test WARNING on linus/master v6.3-rc3 next-20230320]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Viktor-Prutyanov/virtio-add-VIRTIO_F_NOTIFICATION_DATA-feature-support/20230320-195725
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
patch link:
https://lore.kernel.org/r/20230320115451.1232171-1-viktor%40daynix.com
patch subject: [PATCH] virtio: add VIRTIO_F_NOTIFICATION_DATA feature support
config: loongarch-randconfig-s032-20230319 
(https://download.01.org/0day-ci/archive/20230321/202303210441.xoa05pbs-...@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 12.1.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# 
https://github.com/intel-lab-lkp/linux/commit/b6212a12ca1691dc346e5de046ec46bd3ce11247
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review 
Viktor-Prutyanov/virtio-add-VIRTIO_F_NOTIFICATION_DATA-feature-support/20230320-195725
git checkout b6212a12ca1691dc346e5de046ec46bd3ce11247
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 
CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=loongarch 
olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 
CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=loongarch 
SHELL=/bin/bash drivers/virtio/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot 
| Link: 
https://lore.kernel.org/oe-kbuild-all/202303210441.xoa05pbs-...@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/virtio/virtio_pci_common.c:54:19: sparse: sparse: incorrect type in 
>> argument 1 (different base types) @@ expected unsigned int [usertype] 
>> value @@ got restricted __le32 [usertype] data @@
   drivers/virtio/virtio_pci_common.c:54:19: sparse: expected unsigned int 
[usertype] value
   drivers/virtio/virtio_pci_common.c:54:19: sparse: got restricted __le32 
[usertype] data

vim +54 drivers/virtio/virtio_pci_common.c

49  
50  bool vp_notify_with_data(struct virtqueue *vq)
51  {
52  __le32 data = vring_fill_notification_data(vq);
53  
  > 54  iowrite32(data, (void __iomem *)vq->priv);
55  
56  return true;
57  }
58  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] virtio: add VIRTIO_F_NOTIFICATION_DATA feature support

2023-03-20 Thread kernel test robot
Hi Viktor,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mst-vhost/linux-next]
[also build test WARNING on linus/master v6.3-rc3 next-20230320]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Viktor-Prutyanov/virtio-add-VIRTIO_F_NOTIFICATION_DATA-feature-support/20230320-195725
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
patch link:
https://lore.kernel.org/r/20230320115451.1232171-1-viktor%40daynix.com
patch subject: [PATCH] virtio: add VIRTIO_F_NOTIFICATION_DATA feature support
config: sparc64-randconfig-s052-20230319 
(https://download.01.org/0day-ci/archive/20230321/202303210405.8gkuvbfx-...@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 12.1.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# 
https://github.com/intel-lab-lkp/linux/commit/b6212a12ca1691dc346e5de046ec46bd3ce11247
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review 
Viktor-Prutyanov/virtio-add-VIRTIO_F_NOTIFICATION_DATA-feature-support/20230320-195725
git checkout b6212a12ca1691dc346e5de046ec46bd3ce11247
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 
CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=sparc64 
olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 
CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=sparc64 
SHELL=/bin/bash drivers/virtio/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot 
| Link: 
https://lore.kernel.org/oe-kbuild-all/202303210405.8gkuvbfx-...@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/virtio/virtio_mmio.c:293:16: sparse: sparse: incorrect type in 
>> argument 1 (different base types) @@ expected unsigned int [usertype] l 
>> @@ got restricted __le32 [usertype] data @@
   drivers/virtio/virtio_mmio.c:293:16: sparse: expected unsigned int 
[usertype] l
   drivers/virtio/virtio_mmio.c:293:16: sparse: got restricted __le32 
[usertype] data

vim +293 drivers/virtio/virtio_mmio.c

   287  
   288  static bool vm_notify_with_data(struct virtqueue *vq)
   289  {
   290  struct virtio_mmio_device *vm_dev = 
to_virtio_mmio_device(vq->vdev);
   291  __le32 data = vring_fill_notification_data(vq);
   292  
 > 293  writel(data, vm_dev->base + VIRTIO_MMIO_QUEUE_NOTIFY);
   294  
   295  return true;
   296  }
   297  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] virtio: add VIRTIO_F_NOTIFICATION_DATA feature support

2023-03-20 Thread kernel test robot
Hi Viktor,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mst-vhost/linux-next]
[also build test WARNING on linus/master v6.3-rc3 next-20230320]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Viktor-Prutyanov/virtio-add-VIRTIO_F_NOTIFICATION_DATA-feature-support/20230320-195725
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
patch link:
https://lore.kernel.org/r/20230320115451.1232171-1-viktor%40daynix.com
patch subject: [PATCH] virtio: add VIRTIO_F_NOTIFICATION_DATA feature support
config: i386-randconfig-s003 
(https://download.01.org/0day-ci/archive/20230321/202303210403.lsrg8goq-...@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-8) 11.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# 
https://github.com/intel-lab-lkp/linux/commit/b6212a12ca1691dc346e5de046ec46bd3ce11247
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review 
Viktor-Prutyanov/virtio-add-VIRTIO_F_NOTIFICATION_DATA-feature-support/20230320-195725
git checkout b6212a12ca1691dc346e5de046ec46bd3ce11247
# save the config file
mkdir build_dir && cp config build_dir/.config
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir 
ARCH=i386 olddefconfig
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir 
ARCH=i386 SHELL=/bin/bash drivers/virtio/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot 
| Link: 
https://lore.kernel.org/oe-kbuild-all/202303210403.lsrg8goq-...@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/virtio/virtio_mmio.c:293:16: sparse: sparse: incorrect type in 
>> argument 1 (different base types) @@ expected unsigned int val @@ 
>> got restricted __le32 [usertype] data @@
   drivers/virtio/virtio_mmio.c:293:16: sparse: expected unsigned int val
   drivers/virtio/virtio_mmio.c:293:16: sparse: got restricted __le32 
[usertype] data

vim +293 drivers/virtio/virtio_mmio.c

   287  
   288  static bool vm_notify_with_data(struct virtqueue *vq)
   289  {
   290  struct virtio_mmio_device *vm_dev = 
to_virtio_mmio_device(vq->vdev);
   291  __le32 data = vring_fill_notification_data(vq);
   292  
 > 293  writel(data, vm_dev->base + VIRTIO_MMIO_QUEUE_NOTIFY);
   294  
   295  return true;
   296  }
   297  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH] virtio: add VIRTIO_F_NOTIFICATION_DATA feature support

2023-03-20 Thread kernel test robot
Hi Viktor,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mst-vhost/linux-next]
[also build test WARNING on linus/master v6.3-rc3 next-20230320]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Viktor-Prutyanov/virtio-add-VIRTIO_F_NOTIFICATION_DATA-feature-support/20230320-195725
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
patch link:
https://lore.kernel.org/r/20230320115451.1232171-1-viktor%40daynix.com
patch subject: [PATCH] virtio: add VIRTIO_F_NOTIFICATION_DATA feature support
config: nios2-randconfig-s051-20230319 
(https://download.01.org/0day-ci/archive/20230321/202303210449.m2erqjxb-...@intel.com/config)
compiler: nios2-linux-gcc (GCC) 12.1.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# apt-get install sparse
# sparse version: v0.6.4-39-gce1a6720-dirty
# 
https://github.com/intel-lab-lkp/linux/commit/b6212a12ca1691dc346e5de046ec46bd3ce11247
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review 
Viktor-Prutyanov/virtio-add-VIRTIO_F_NOTIFICATION_DATA-feature-support/20230320-195725
git checkout b6212a12ca1691dc346e5de046ec46bd3ce11247
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 
CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=nios2 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross C=1 
CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=nios2 
SHELL=/bin/bash drivers/virtio/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot 
| Link: 
https://lore.kernel.org/oe-kbuild-all/202303210449.m2erqjxb-...@intel.com/

sparse warnings: (new ones prefixed by >>)
>> drivers/virtio/virtio_mmio.c:293:16: sparse: sparse: incorrect type in 
>> argument 1 (different base types) @@ expected unsigned int [usertype] 
>> value @@ got restricted __le32 [usertype] data @@
   drivers/virtio/virtio_mmio.c:293:16: sparse: expected unsigned int 
[usertype] value
   drivers/virtio/virtio_mmio.c:293:16: sparse: got restricted __le32 
[usertype] data

vim +293 drivers/virtio/virtio_mmio.c

   287  
   288  static bool vm_notify_with_data(struct virtqueue *vq)
   289  {
   290  struct virtio_mmio_device *vm_dev = 
to_virtio_mmio_device(vq->vdev);
   291  __le32 data = vring_fill_notification_data(vq);
   292  
 > 293  writel(data, vm_dev->base + VIRTIO_MMIO_QUEUE_NOTIFY);
   294  
   295  return true;
   296  }
   297  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v3 2/2] vdpa/mlx5: Make VIRTIO_NET_F_MRG_RXBUF off by default

2023-03-20 Thread Michael S. Tsirkin
On Mon, Mar 20, 2023 at 01:49:30PM +0200, Eli Cohen wrote:
> One can still enable it when creating the vdpa device using vdpa tool by
> providing features that include it.
> 
> For example:
> $ vdpa dev add name vdpa0 mgmtdev pci/:86:00.2 device_features 0x300cb982b
> 
> Please be aware that this feature was not supported before the previous
> patch in this list was introduced so we don't change user experience.

so I would  say the patches have to be reordered to avoid a regression?

> Current firmware versions show degradation in packet rate when using
> MRG_RXBUF. Users who favor memory saving over packet rate could enable
> this feature but we want to keep it off by default.
> 
> Signed-off-by: Eli Cohen 

OK and when future firmware will (maybe) fix this up how
will you know it's ok to enable by default?
Some version check I guess?
It would be better if firmware specified flags to enable
by default ...


> ---
>  drivers/vdpa/mlx5/net/mlx5_vnet.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
> b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> index 5285dd76c793..24397a71d6f3 100644
> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> @@ -3146,6 +3146,8 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev 
> *v_mdev, const char *name,
>   return -EINVAL;
>   }
>   device_features &= add_config->device_features;
> + } else {
> + device_features &= ~BIT_ULL(VIRTIO_NET_F_MRG_RXBUF);
>   }
>   if (!(device_features & BIT_ULL(VIRTIO_F_VERSION_1) &&
> device_features & BIT_ULL(VIRTIO_F_ACCESS_PLATFORM))) {
> -- 
> 2.38.1

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


Re: [PATCH] virtio: add VIRTIO_F_NOTIFICATION_DATA feature support

2023-03-20 Thread Michael S. Tsirkin
On Mon, Mar 20, 2023 at 02:54:51PM +0300, Viktor Prutyanov wrote:
> According to VirtIO spec v1.2, VIRTIO_F_NOTIFICATION_DATA feature
> indicates that the driver passes extra data along with the queue
> notifications.
> 
> In a split queue case, the extra data is 16-bit available index. In a
> packed queue case, the extra data is 1-bit wrap counter and 15-bit
> available index.
> 
> Add support for this feature for both MMIO and PCI.
> 
> Signed-off-by: Viktor Prutyanov 
> ---
>  drivers/virtio/virtio_mmio.c   | 15 ++-
>  drivers/virtio/virtio_pci_common.c | 10 ++
>  drivers/virtio/virtio_pci_common.h |  4 
>  drivers/virtio/virtio_pci_legacy.c |  2 +-
>  drivers/virtio/virtio_pci_modern.c |  2 +-
>  drivers/virtio/virtio_ring.c   | 17 +
>  include/linux/virtio_ring.h|  2 ++
>  include/uapi/linux/virtio_config.h |  6 ++
>  8 files changed, 55 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> index 3ff746e3f24a..05da5ad7fc93 100644
> --- a/drivers/virtio/virtio_mmio.c
> +++ b/drivers/virtio/virtio_mmio.c
> @@ -285,6 +285,19 @@ static bool vm_notify(struct virtqueue *vq)
>   return true;
>  }
>  
> +static bool vm_notify_with_data(struct virtqueue *vq)
> +{
> + struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vq->vdev);
> + __le32 data = vring_fill_notification_data(vq);
> +
> + writel(data, vm_dev->base + VIRTIO_MMIO_QUEUE_NOTIFY);
> +
> + return true;
> +}
> +
> +#define VM_NOTIFY(vdev) (__virtio_test_bit((vdev), 
> VIRTIO_F_NOTIFICATION_DATA) \
> + ? vm_notify_with_data : vm_notify)
> +
>  /* Notify all virtqueues on an interrupt. */
>  static irqreturn_t vm_interrupt(int irq, void *opaque)
>  {
> @@ -397,7 +410,7 @@ static struct virtqueue *vm_setup_vq(struct virtio_device 
> *vdev, unsigned int in
>  
>   /* Create the vring */
>   vq = vring_create_virtqueue(index, num, VIRTIO_MMIO_VRING_ALIGN, vdev,
> -  true, true, ctx, vm_notify, callback, name);
> + true, true, ctx, VM_NOTIFY(vdev), callback, name);
>   if (!vq) {
>   err = -ENOMEM;
>   goto error_new_virtqueue;
> diff --git a/drivers/virtio/virtio_pci_common.c 
> b/drivers/virtio/virtio_pci_common.c
> index a6c86f916dbd..bf7daad9ce65 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -43,6 +43,16 @@ bool vp_notify(struct virtqueue *vq)
>   /* we write the queue's selector into the notification register to
>* signal the other end */
>   iowrite16(vq->index, (void __iomem *)vq->priv);
> +
> + return true;
> +}
> +
> +bool vp_notify_with_data(struct virtqueue *vq)
> +{
> + __le32 data = vring_fill_notification_data(vq);
> +
> + iowrite32(data, (void __iomem *)vq->priv);
> +
>   return true;
>  }
>  
> diff --git a/drivers/virtio/virtio_pci_common.h 
> b/drivers/virtio/virtio_pci_common.h
> index 23112d84218f..9a7212dcbb32 100644
> --- a/drivers/virtio/virtio_pci_common.h
> +++ b/drivers/virtio/virtio_pci_common.h
> @@ -105,6 +105,7 @@ static struct virtio_pci_device *to_vp_device(struct 
> virtio_device *vdev)
>  void vp_synchronize_vectors(struct virtio_device *vdev);
>  /* the notify function used when creating a virt queue */
>  bool vp_notify(struct virtqueue *vq);
> +bool vp_notify_with_data(struct virtqueue *vq);
>  /* the config->del_vqs() implementation */
>  void vp_del_vqs(struct virtio_device *vdev);
>  /* the config->find_vqs() implementation */
> @@ -114,6 +115,9 @@ int vp_find_vqs(struct virtio_device *vdev, unsigned int 
> nvqs,
>   struct irq_affinity *desc);
>  const char *vp_bus_name(struct virtio_device *vdev);
>  
> +#define VP_NOTIFY(vdev) (__virtio_test_bit((vdev), 
> VIRTIO_F_NOTIFICATION_DATA) \
> + ? vp_notify : vp_notify_with_data)
> +
>  /* Setup the affinity for a virtqueue:
>   * - force the affinity for per vq vector
>   * - OR over all affinities for shared MSI
> diff --git a/drivers/virtio/virtio_pci_legacy.c 
> b/drivers/virtio/virtio_pci_legacy.c
> index 2257f1b3d8ae..b98e994cae48 100644
> --- a/drivers/virtio/virtio_pci_legacy.c
> +++ b/drivers/virtio/virtio_pci_legacy.c
> @@ -131,7 +131,7 @@ static struct virtqueue *setup_vq(struct 
> virtio_pci_device *vp_dev,
>   vq = vring_create_virtqueue(index, num,
>   VIRTIO_PCI_VRING_ALIGN, _dev->vdev,
>   true, false, ctx,
> - vp_notify, callback, name);
> + VP_NOTIFY(_dev->vdev), callback, name);
>   if (!vq)
>   return ERR_PTR(-ENOMEM);
>  
> diff --git a/drivers/virtio/virtio_pci_modern.c 
> b/drivers/virtio/virtio_pci_modern.c
> index 9e496e288cfa..7fcd8af5af7e 100644
> --- a/drivers/virtio/virtio_pci_modern.c
> +++ b/drivers/virtio/virtio_pci_modern.c
> @@ -321,7 +321,7 @@ static struct virtqueue 

Re: [RFC PATCH v1 3/3] test/vsock: skbuff merging test

2023-03-20 Thread Stefano Garzarella

On Sun, Mar 19, 2023 at 09:53:54PM +0300, Arseniy Krasnov wrote:

This adds test which checks case when data of newly received skbuff is
appended to the last skbuff in the socket's queue.

This test is actual only for virtio transport.

Signed-off-by: Arseniy Krasnov 
---
tools/testing/vsock/vsock_test.c | 81 
1 file changed, 81 insertions(+)

diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index 3de10dbb50f5..00216c52d8b6 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -968,6 +968,82 @@ static void test_seqpacket_inv_buf_server(const struct 
test_opts *opts)
test_inv_buf_server(opts, false);
}

+static void test_stream_virtio_skb_merge_client(const struct test_opts *opts)
+{
+   ssize_t res;
+   int fd;
+
+   fd = vsock_stream_connect(opts->peer_cid, 1234);
+   if (fd < 0) {
+   perror("connect");
+   exit(EXIT_FAILURE);
+   }
+


Please use a macro for "HELLO" or a variabile, e.g.

char *buf;
...

buf = "HELLO";
res = send(fd, buf, strlen(buf), 0);
...


+   res = send(fd, "HELLO", strlen("HELLO"), 0);
+   if (res != strlen("HELLO")) {
+   fprintf(stderr, "unexpected send(2) result %zi\n", res);
+   exit(EXIT_FAILURE);
+   }
+
+   control_writeln("SEND0");
+   /* Peer reads part of first packet. */
+   control_expectln("REPLY0");
+
+   /* Send second skbuff, it will be merged. */
+   res = send(fd, "WORLD", strlen("WORLD"), 0);


Ditto.


+   if (res != strlen("WORLD")) {
+   fprintf(stderr, "unexpected send(2) result %zi\n", res);
+   exit(EXIT_FAILURE);
+   }
+
+   control_writeln("SEND1");
+   /* Peer reads merged skbuff packet. */
+   control_expectln("REPLY1");
+
+   close(fd);
+}
+
+static void test_stream_virtio_skb_merge_server(const struct test_opts *opts)
+{
+   unsigned char buf[64];
+   ssize_t res;
+   int fd;
+
+   fd = vsock_stream_accept(VMADDR_CID_ANY, 1234, NULL);
+   if (fd < 0) {
+   perror("accept");
+   exit(EXIT_FAILURE);
+   }
+
+   control_expectln("SEND0");
+
+   /* Read skbuff partially. */
+   res = recv(fd, buf, 2, 0);
+   if (res != 2) {
+   fprintf(stderr, "expected recv(2) failure, got %zi\n", res);


We don't expect a failure, so please update the error message and make
it easy to figure out which recv() is failing. For example by saying
how many bytes you expected and how many you received.


+   exit(EXIT_FAILURE);
+   }
+
+   control_writeln("REPLY0");
+   control_expectln("SEND1");
+
+
+   res = recv(fd, buf, sizeof(buf), 0);


Perhaps a comment here to explain why we expect only 8 bytes.


+   if (res != 8) {
+   fprintf(stderr, "expected recv(2) failure, got %zi\n", res);


Ditto.


+   exit(EXIT_FAILURE);
+   }
+
+   res = recv(fd, buf, sizeof(buf), MSG_DONTWAIT);
+   if (res != -1) {
+   fprintf(stderr, "expected recv(2) success, got %zi\n", res);


It's the other way around, isn't it?
Here you expect it to fail instead it is not failing.


+   exit(EXIT_FAILURE);
+   }


Moving the pointer correctly, I would also check that there is
HELLOWORLD in the buffer.

Thanks for adding tests in this suite!
Stefano


+
+   control_writeln("REPLY1");
+
+   close(fd);
+}
+
static struct test_case test_cases[] = {
{
.name = "SOCK_STREAM connection reset",
@@ -1038,6 +1114,11 @@ static struct test_case test_cases[] = {
.run_client = test_seqpacket_inv_buf_client,
.run_server = test_seqpacket_inv_buf_server,
},
+   {
+   .name = "SOCK_STREAM virtio skb merge",
+   .run_client = test_stream_virtio_skb_merge_client,
+   .run_server = test_stream_virtio_skb_merge_server,
+   },
{},
};

--
2.25.1



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


Re: [RFC PATCH v1 2/3] virtio/vsock: add WARN() for invalid state of socket

2023-03-20 Thread Stefano Garzarella

On Sun, Mar 19, 2023 at 09:52:19PM +0300, Arseniy Krasnov wrote:

This prints WARN() and returns from stream dequeue callback when socket's
queue is empty, but 'rx_bytes' still non-zero.

Signed-off-by: Arseniy Krasnov 
---
net/vmw_vsock/virtio_transport_common.c | 7 +++
1 file changed, 7 insertions(+)

diff --git a/net/vmw_vsock/virtio_transport_common.c 
b/net/vmw_vsock/virtio_transport_common.c
index 3c75986e16c2..c35b03adad8d 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -388,6 +388,13 @@ virtio_transport_stream_do_dequeue(struct vsock_sock *vsk,
u32 free_space;

spin_lock_bh(>rx_lock);
+
+   if (skb_queue_empty(>rx_queue) && vvs->rx_bytes) {
+   WARN(1, "No skbuffs with non-zero 'rx_bytes'\n");


I would use WARN_ONCE, since we can't recover so we will flood the log.

And you can put the condition in the first argument, I mean something
like this:
if (WARN_ONCE(skb_queue_empty(>rx_queue) && vvs->rx_bytes,
  "rx_queue is empty, but rx_bytes is non-zero\n")) {

Thanks,
Stefano


+   spin_unlock_bh(>rx_lock);
+   return err;
+   }
+
while (total < len && !skb_queue_empty(>rx_queue)) {
skb = skb_peek(>rx_queue);

--
2.25.1



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


[PATCH v2 3/8] drm/fb-helper: Export drm_fb_helper_release_info()

2023-03-20 Thread Thomas Zimmermann
Export the fb_info release code as drm_fb_helper_release_info(). Will
help with cleaning up failed fbdev probing.

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Javier Martinez Canillas 
Acked-by: Zack Rusin 
---
 drivers/gpu/drm/drm_fb_helper.c | 33 -
 include/drm/drm_fb_helper.h |  5 +
 2 files changed, 29 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index a39998047f8a..7e96ed9efdb5 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -538,6 +538,29 @@ struct fb_info *drm_fb_helper_alloc_info(struct 
drm_fb_helper *fb_helper)
 }
 EXPORT_SYMBOL(drm_fb_helper_alloc_info);
 
+/**
+ * drm_fb_helper_release_info - release fb_info and its members
+ * @fb_helper: driver-allocated fbdev helper
+ *
+ * A helper to release fb_info and the member cmap.  Drivers do not
+ * need to release the allocated fb_info structure themselves, this is
+ * automatically done when calling drm_fb_helper_fini().
+ */
+void drm_fb_helper_release_info(struct drm_fb_helper *fb_helper)
+{
+   struct fb_info *info = fb_helper->info;
+
+   if (!info)
+   return;
+
+   fb_helper->info = NULL;
+
+   if (info->cmap.len)
+   fb_dealloc_cmap(>cmap);
+   framebuffer_release(info);
+}
+EXPORT_SYMBOL(drm_fb_helper_release_info);
+
 /**
  * drm_fb_helper_unregister_info - unregister fb_info framebuffer device
  * @fb_helper: driver-allocated fbdev helper, can be NULL
@@ -561,8 +584,6 @@ EXPORT_SYMBOL(drm_fb_helper_unregister_info);
  */
 void drm_fb_helper_fini(struct drm_fb_helper *fb_helper)
 {
-   struct fb_info *info;
-
if (!fb_helper)
return;
 
@@ -574,13 +595,7 @@ void drm_fb_helper_fini(struct drm_fb_helper *fb_helper)
cancel_work_sync(_helper->resume_work);
cancel_work_sync(_helper->damage_work);
 
-   info = fb_helper->info;
-   if (info) {
-   if (info->cmap.len)
-   fb_dealloc_cmap(>cmap);
-   framebuffer_release(info);
-   }
-   fb_helper->info = NULL;
+   drm_fb_helper_release_info(fb_helper);
 
mutex_lock(_fb_helper_lock);
if (!list_empty(_helper->kernel_fb_list)) {
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index 013654de3fc5..c5822ec2fdd1 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -256,6 +256,7 @@ int drm_fb_helper_check_var(struct fb_var_screeninfo *var,
 int drm_fb_helper_restore_fbdev_mode_unlocked(struct drm_fb_helper *fb_helper);
 
 struct fb_info *drm_fb_helper_alloc_info(struct drm_fb_helper *fb_helper);
+void drm_fb_helper_release_info(struct drm_fb_helper *fb_helper);
 void drm_fb_helper_unregister_info(struct drm_fb_helper *fb_helper);
 void drm_fb_helper_fill_info(struct fb_info *info,
 struct drm_fb_helper *fb_helper,
@@ -365,6 +366,10 @@ drm_fb_helper_alloc_info(struct drm_fb_helper *fb_helper)
return NULL;
 }
 
+static inline void drm_fb_helper_release_info(struct drm_fb_helper *fb_helper)
+{
+}
+
 static inline void drm_fb_helper_unregister_info(struct drm_fb_helper 
*fb_helper)
 {
 }
-- 
2.40.0

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


[PATCH v2 6/8] drm/fbdev-generic: Clean up after failed probing

2023-03-20 Thread Thomas Zimmermann
Clean up fbdev and client state if the probe function fails. It
used to leak allocated resources. Also reorder the individual steps
to simplify cleanup.

v2:
* move screen_size update into separate patches

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Javier Martinez Canillas 
Acked-by: Zack Rusin 
---
 drivers/gpu/drm/drm_fbdev_generic.c | 40 -
 1 file changed, 28 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/drm_fbdev_generic.c 
b/drivers/gpu/drm/drm_fbdev_generic.c
index 73834a3cc6b0..e7eeba0c44b4 100644
--- a/drivers/gpu/drm/drm_fbdev_generic.c
+++ b/drivers/gpu/drm/drm_fbdev_generic.c
@@ -77,6 +77,7 @@ static int drm_fbdev_fb_probe(struct drm_fb_helper *fb_helper,
struct drm_client_buffer *buffer;
struct fb_info *info;
size_t screen_size;
+   void *screen_buffer;
u32 format;
int ret;
 
@@ -92,36 +93,51 @@ static int drm_fbdev_fb_probe(struct drm_fb_helper 
*fb_helper,
 
fb_helper->buffer = buffer;
fb_helper->fb = buffer->fb;
+
screen_size = buffer->gem->size;
+   screen_buffer = vzalloc(screen_size);
+   if (!screen_buffer) {
+   ret = -ENOMEM;
+   goto err_drm_client_framebuffer_delete;
+   }
 
info = drm_fb_helper_alloc_info(fb_helper);
-   if (IS_ERR(info))
-   return PTR_ERR(info);
+   if (IS_ERR(info)) {
+   ret = PTR_ERR(info);
+   goto err_vfree;
+   }
+
+   drm_fb_helper_fill_info(info, fb_helper, sizes);
 
info->fbops = _fbdev_fb_ops;
-   info->screen_size = screen_size;
-   info->fix.smem_len = screen_size;
info->flags = FBINFO_DEFAULT;
 
-   drm_fb_helper_fill_info(info, fb_helper, sizes);
-
-   info->screen_buffer = vzalloc(screen_size);
-   if (!info->screen_buffer)
-   return -ENOMEM;
+   /* screen */
info->flags |= FBINFO_VIRTFB | FBINFO_READS_FAST;
-
+   info->screen_buffer = screen_buffer;
info->fix.smem_start = 
page_to_phys(vmalloc_to_page(info->screen_buffer));
+   info->fix.smem_len = screen_size;
 
-   /* Set a default deferred I/O handler */
+   /* deferred I/O */
fb_helper->fbdefio.delay = HZ / 20;
fb_helper->fbdefio.deferred_io = drm_fb_helper_deferred_io;
 
info->fbdefio = _helper->fbdefio;
ret = fb_deferred_io_init(info);
if (ret)
-   return ret;
+   goto err_drm_fb_helper_release_info;
 
return 0;
+
+err_drm_fb_helper_release_info:
+   drm_fb_helper_release_info(fb_helper);
+err_vfree:
+   vfree(screen_buffer);
+err_drm_client_framebuffer_delete:
+   fb_helper->fb = NULL;
+   fb_helper->buffer = NULL;
+   drm_client_framebuffer_delete(buffer);
+   return ret;
 }
 
 static void drm_fbdev_damage_blit_real(struct drm_fb_helper *fb_helper,
-- 
2.40.0

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


[PATCH v2 1/8] drm/fbdev-generic: Always use shadow buffering

2023-03-20 Thread Thomas Zimmermann
Remove all codepaths that implement fbdev output directly on GEM
buffers. Always allocate a shadow buffer in system memory and set
up deferred I/O for mmap.

The fbdev code that operated directly on GEM buffers was used by
drivers based on GEM DMA helpers. Those drivers have been migrated
to use fbdev-dma, a dedicated fbdev emulation for DMA memory. All
remaining users of fbdev-generic require shadow buffering.

Memory management of the remaining callers uses TTM, GEM SHMEM
helpers or a variant of GEM DMA helpers that is incompatible with
fbdev-dma. Therefore remove the unused codepaths from fbdev-generic
and simplify the code.

Using a shadow buffer with deferred I/O is probably the best case
for most remaining callers. Some of the TTM-based drivers might
benefit from a dedicated fbdev emulation that operates directly on
the driver's video memory.

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Javier Martinez Canillas 
Acked-by: Zack Rusin 
---
 drivers/gpu/drm/drm_fbdev_generic.c | 184 +---
 1 file changed, 30 insertions(+), 154 deletions(-)

diff --git a/drivers/gpu/drm/drm_fbdev_generic.c 
b/drivers/gpu/drm/drm_fbdev_generic.c
index 4d6325e91565..e48a8e82378d 100644
--- a/drivers/gpu/drm/drm_fbdev_generic.c
+++ b/drivers/gpu/drm/drm_fbdev_generic.c
@@ -11,16 +11,6 @@
 
 #include 
 
-static bool drm_fbdev_use_shadow_fb(struct drm_fb_helper *fb_helper)
-{
-   struct drm_device *dev = fb_helper->dev;
-   struct drm_framebuffer *fb = fb_helper->fb;
-
-   return dev->mode_config.prefer_shadow_fbdev ||
-  dev->mode_config.prefer_shadow ||
-  fb->funcs->dirty;
-}
-
 /* @user: 1=userspace, 0=fbcon */
 static int drm_fbdev_fb_open(struct fb_info *info, int user)
 {
@@ -46,115 +36,33 @@ static int drm_fbdev_fb_release(struct fb_info *info, int 
user)
 static void drm_fbdev_fb_destroy(struct fb_info *info)
 {
struct drm_fb_helper *fb_helper = info->par;
-   void *shadow = NULL;
+   void *shadow = info->screen_buffer;
 
if (!fb_helper->dev)
return;
 
-   if (info->fbdefio)
-   fb_deferred_io_cleanup(info);
-   if (drm_fbdev_use_shadow_fb(fb_helper))
-   shadow = info->screen_buffer;
-
+   fb_deferred_io_cleanup(info);
drm_fb_helper_fini(fb_helper);
-
-   if (shadow)
-   vfree(shadow);
-   else if (fb_helper->buffer)
-   drm_client_buffer_vunmap(fb_helper->buffer);
-
+   vfree(shadow);
drm_client_framebuffer_delete(fb_helper->buffer);
-   drm_client_release(_helper->client);
 
+   drm_client_release(_helper->client);
drm_fb_helper_unprepare(fb_helper);
kfree(fb_helper);
 }
 
-static int drm_fbdev_fb_mmap(struct fb_info *info, struct vm_area_struct *vma)
-{
-   struct drm_fb_helper *fb_helper = info->par;
-
-   if (drm_fbdev_use_shadow_fb(fb_helper))
-   return fb_deferred_io_mmap(info, vma);
-   else if (fb_helper->dev->driver->gem_prime_mmap)
-   return 
fb_helper->dev->driver->gem_prime_mmap(fb_helper->buffer->gem, vma);
-   else
-   return -ENODEV;
-}
-
-static bool drm_fbdev_use_iomem(struct fb_info *info)
-{
-   struct drm_fb_helper *fb_helper = info->par;
-   struct drm_client_buffer *buffer = fb_helper->buffer;
-
-   return !drm_fbdev_use_shadow_fb(fb_helper) && buffer->map.is_iomem;
-}
-
-static ssize_t drm_fbdev_fb_read(struct fb_info *info, char __user *buf,
-size_t count, loff_t *ppos)
-{
-   ssize_t ret;
-
-   if (drm_fbdev_use_iomem(info))
-   ret = drm_fb_helper_cfb_read(info, buf, count, ppos);
-   else
-   ret = drm_fb_helper_sys_read(info, buf, count, ppos);
-
-   return ret;
-}
-
-static ssize_t drm_fbdev_fb_write(struct fb_info *info, const char __user *buf,
- size_t count, loff_t *ppos)
-{
-   ssize_t ret;
-
-   if (drm_fbdev_use_iomem(info))
-   ret = drm_fb_helper_cfb_write(info, buf, count, ppos);
-   else
-   ret = drm_fb_helper_sys_write(info, buf, count, ppos);
-
-   return ret;
-}
-
-static void drm_fbdev_fb_fillrect(struct fb_info *info,
- const struct fb_fillrect *rect)
-{
-   if (drm_fbdev_use_iomem(info))
-   drm_fb_helper_cfb_fillrect(info, rect);
-   else
-   drm_fb_helper_sys_fillrect(info, rect);
-}
-
-static void drm_fbdev_fb_copyarea(struct fb_info *info,
- const struct fb_copyarea *area)
-{
-   if (drm_fbdev_use_iomem(info))
-   drm_fb_helper_cfb_copyarea(info, area);
-   else
-   drm_fb_helper_sys_copyarea(info, area);
-}
-
-static void drm_fbdev_fb_imageblit(struct fb_info *info,
-  const struct fb_image *image)
-{
-   if (drm_fbdev_use_iomem(info))
-   drm_fb_helper_cfb_imageblit(info, 

[PATCH v2 5/8] drm/fbdev-generic: Set screen size to size of GEM buffer

2023-03-20 Thread Thomas Zimmermann
The size of the screen memory should be equivalent to the size of
the screen's GEM buffer. Don't recalculate the value.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/drm_fbdev_generic.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_fbdev_generic.c 
b/drivers/gpu/drm/drm_fbdev_generic.c
index e48a8e82378d..73834a3cc6b0 100644
--- a/drivers/gpu/drm/drm_fbdev_generic.c
+++ b/drivers/gpu/drm/drm_fbdev_generic.c
@@ -7,6 +7,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -74,8 +75,8 @@ static int drm_fbdev_fb_probe(struct drm_fb_helper *fb_helper,
struct drm_client_dev *client = _helper->client;
struct drm_device *dev = fb_helper->dev;
struct drm_client_buffer *buffer;
-   struct drm_framebuffer *fb;
struct fb_info *info;
+   size_t screen_size;
u32 format;
int ret;
 
@@ -91,20 +92,20 @@ static int drm_fbdev_fb_probe(struct drm_fb_helper 
*fb_helper,
 
fb_helper->buffer = buffer;
fb_helper->fb = buffer->fb;
-   fb = buffer->fb;
+   screen_size = buffer->gem->size;
 
info = drm_fb_helper_alloc_info(fb_helper);
if (IS_ERR(info))
return PTR_ERR(info);
 
info->fbops = _fbdev_fb_ops;
-   info->screen_size = sizes->surface_height * fb->pitches[0];
-   info->fix.smem_len = info->screen_size;
+   info->screen_size = screen_size;
+   info->fix.smem_len = screen_size;
info->flags = FBINFO_DEFAULT;
 
drm_fb_helper_fill_info(info, fb_helper, sizes);
 
-   info->screen_buffer = vzalloc(info->screen_size);
+   info->screen_buffer = vzalloc(screen_size);
if (!info->screen_buffer)
return -ENOMEM;
info->flags |= FBINFO_VIRTFB | FBINFO_READS_FAST;
-- 
2.40.0

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


[PATCH v2 2/8] drm/fbdev-generic: Remove unused prefer_shadow_fbdev flag

2023-03-20 Thread Thomas Zimmermann
Remove the flag prefer_shadow_fbdev from struct drm_mode_config.
Drivers set this flag to enable shadow buffering in the generic
fbdev emulation. Such shadow buffering is now mandatory, so the
flag is unused.

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Javier Martinez Canillas 
Reviewed-by: Zack Rusin 
---
 drivers/gpu/drm/tiny/bochs.c| 1 -
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 1 -
 include/drm/drm_mode_config.h   | 7 ---
 3 files changed, 9 deletions(-)

diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index 024346054c70..d254679a136e 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -545,7 +545,6 @@ static int bochs_kms_init(struct bochs_device *bochs)
 
bochs->dev->mode_config.preferred_depth = 24;
bochs->dev->mode_config.prefer_shadow = 0;
-   bochs->dev->mode_config.prefer_shadow_fbdev = 1;
bochs->dev->mode_config.quirk_addfb_prefer_host_byte_order = true;
 
bochs->dev->mode_config.funcs = _mode_funcs;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c 
b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 84d6380b9895..5162a7a12792 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -2112,7 +2112,6 @@ int vmw_kms_init(struct vmw_private *dev_priv)
dev->mode_config.max_width = dev_priv->texture_max_width;
dev->mode_config.max_height = dev_priv->texture_max_height;
dev->mode_config.preferred_depth = dev_priv->assume_16bpp ? 16 : 32;
-   dev->mode_config.prefer_shadow_fbdev = !dev_priv->has_mob;
 
drm_mode_create_suggested_offset_properties(dev);
vmw_kms_create_hotplug_mode_update_property(dev_priv);
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index e5b053001d22..973119a9176b 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -890,13 +890,6 @@ struct drm_mode_config {
/* dumb ioctl parameters */
uint32_t preferred_depth, prefer_shadow;
 
-   /**
-* @prefer_shadow_fbdev:
-*
-* Hint to framebuffer emulation to prefer shadow-fb rendering.
-*/
-   bool prefer_shadow_fbdev;
-
/**
 * @quirk_addfb_prefer_xbgr_30bpp:
 *
-- 
2.40.0

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


[PATCH v2 8/8] drm/fbdev-generic: Rename symbols

2023-03-20 Thread Thomas Zimmermann
Rename symbols to match the style of other fbdev-emulation source
code. No functional changes.

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Javier Martinez Canillas 
Acked-by: Zack Rusin 
---
 drivers/gpu/drm/drm_fbdev_generic.c | 66 ++---
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/drm_fbdev_generic.c 
b/drivers/gpu/drm/drm_fbdev_generic.c
index e7eeba0c44b4..8e5148bf40bb 100644
--- a/drivers/gpu/drm/drm_fbdev_generic.c
+++ b/drivers/gpu/drm/drm_fbdev_generic.c
@@ -13,7 +13,7 @@
 #include 
 
 /* @user: 1=userspace, 0=fbcon */
-static int drm_fbdev_fb_open(struct fb_info *info, int user)
+static int drm_fbdev_generic_fb_open(struct fb_info *info, int user)
 {
struct drm_fb_helper *fb_helper = info->par;
 
@@ -24,7 +24,7 @@ static int drm_fbdev_fb_open(struct fb_info *info, int user)
return 0;
 }
 
-static int drm_fbdev_fb_release(struct fb_info *info, int user)
+static int drm_fbdev_generic_fb_release(struct fb_info *info, int user)
 {
struct drm_fb_helper *fb_helper = info->par;
 
@@ -34,7 +34,7 @@ static int drm_fbdev_fb_release(struct fb_info *info, int 
user)
return 0;
 }
 
-static void drm_fbdev_fb_destroy(struct fb_info *info)
+static void drm_fbdev_generic_fb_destroy(struct fb_info *info)
 {
struct drm_fb_helper *fb_helper = info->par;
void *shadow = info->screen_buffer;
@@ -52,10 +52,10 @@ static void drm_fbdev_fb_destroy(struct fb_info *info)
kfree(fb_helper);
 }
 
-static const struct fb_ops drm_fbdev_fb_ops = {
+static const struct fb_ops drm_fbdev_generic_fb_ops = {
.owner  = THIS_MODULE,
-   .fb_open= drm_fbdev_fb_open,
-   .fb_release = drm_fbdev_fb_release,
+   .fb_open= drm_fbdev_generic_fb_open,
+   .fb_release = drm_fbdev_generic_fb_release,
.fb_read= drm_fb_helper_sys_read,
.fb_write   = drm_fb_helper_sys_write,
DRM_FB_HELPER_DEFAULT_OPS,
@@ -63,14 +63,14 @@ static const struct fb_ops drm_fbdev_fb_ops = {
.fb_copyarea= drm_fb_helper_sys_copyarea,
.fb_imageblit   = drm_fb_helper_sys_imageblit,
.fb_mmap= fb_deferred_io_mmap,
-   .fb_destroy = drm_fbdev_fb_destroy,
+   .fb_destroy = drm_fbdev_generic_fb_destroy,
 };
 
 /*
  * This function uses the client API to create a framebuffer backed by a dumb 
buffer.
  */
-static int drm_fbdev_fb_probe(struct drm_fb_helper *fb_helper,
- struct drm_fb_helper_surface_size *sizes)
+static int drm_fbdev_generic_helper_fb_probe(struct drm_fb_helper *fb_helper,
+struct drm_fb_helper_surface_size 
*sizes)
 {
struct drm_client_dev *client = _helper->client;
struct drm_device *dev = fb_helper->dev;
@@ -109,7 +109,7 @@ static int drm_fbdev_fb_probe(struct drm_fb_helper 
*fb_helper,
 
drm_fb_helper_fill_info(info, fb_helper, sizes);
 
-   info->fbops = _fbdev_fb_ops;
+   info->fbops = _fbdev_generic_fb_ops;
info->flags = FBINFO_DEFAULT;
 
/* screen */
@@ -140,9 +140,9 @@ static int drm_fbdev_fb_probe(struct drm_fb_helper 
*fb_helper,
return ret;
 }
 
-static void drm_fbdev_damage_blit_real(struct drm_fb_helper *fb_helper,
-  struct drm_clip_rect *clip,
-  struct iosys_map *dst)
+static void drm_fbdev_generic_damage_blit_real(struct drm_fb_helper *fb_helper,
+  struct drm_clip_rect *clip,
+  struct iosys_map *dst)
 {
struct drm_framebuffer *fb = fb_helper->fb;
size_t offset = clip->y1 * fb->pitches[0];
@@ -179,8 +179,8 @@ static void drm_fbdev_damage_blit_real(struct drm_fb_helper 
*fb_helper,
}
 }
 
-static int drm_fbdev_damage_blit(struct drm_fb_helper *fb_helper,
-struct drm_clip_rect *clip)
+static int drm_fbdev_generic_damage_blit(struct drm_fb_helper *fb_helper,
+struct drm_clip_rect *clip)
 {
struct drm_client_buffer *buffer = fb_helper->buffer;
struct iosys_map map, dst;
@@ -204,7 +204,7 @@ static int drm_fbdev_damage_blit(struct drm_fb_helper 
*fb_helper,
goto out;
 
dst = map;
-   drm_fbdev_damage_blit_real(fb_helper, clip, );
+   drm_fbdev_generic_damage_blit_real(fb_helper, clip, );
 
drm_client_buffer_vunmap(buffer);
 
@@ -214,7 +214,8 @@ static int drm_fbdev_damage_blit(struct drm_fb_helper 
*fb_helper,
return ret;
 }
 
-static int drm_fbdev_fb_dirty(struct drm_fb_helper *helper, struct 
drm_clip_rect *clip)
+static int drm_fbdev_generic_helper_fb_dirty(struct drm_fb_helper *helper,
+struct drm_clip_rect *clip)
 {
struct drm_device *dev = helper->dev;
int ret;
@@ -223,7 +224,7 @@ static 

[PATCH v2 7/8] drm/fb-helper: Consolidate CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM

2023-03-20 Thread Thomas Zimmermann
Consolidate all handling of CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM by
making the module parameter optional in drm_fb_helper.c.

Without the config option, modules can set smem_start in struct
fb_info for internal usage, but not export if to userspace. The
address can only be exported by enabling the option and setting
the module parameter. Also update the comment.

Signed-off-by: Thomas Zimmermann 
Reviewed-by: Javier Martinez Canillas 
Acked-by: Zack Rusin 
---
 drivers/gpu/drm/drm_fb_helper.c | 22 --
 drivers/gpu/drm/drm_fbdev_dma.c |  9 +
 include/drm/drm_fb_helper.h |  9 -
 3 files changed, 9 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index bb0b25209b3e..63ec95e86d0e 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -60,16 +60,17 @@ MODULE_PARM_DESC(drm_fbdev_overalloc,
  * In order to keep user-space compatibility, we want in certain use-cases
  * to keep leaking the fbdev physical address to the user-space program
  * handling the fbdev buffer.
- * This is a bad habit essentially kept into closed source opengl driver
- * that should really be moved into open-source upstream projects instead
- * of using legacy physical addresses in user space to communicate with
- * other out-of-tree kernel modules.
+ *
+ * This is a bad habit, essentially kept to support closed-source OpenGL
+ * drivers that should really be moved into open-source upstream projects
+ * instead of using legacy physical addresses in user space to communicate
+ * with other out-of-tree kernel modules.
  *
  * This module_param *should* be removed as soon as possible and be
  * considered as a broken and legacy behaviour from a modern fbdev device.
  */
-#if IS_ENABLED(CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM)
 static bool drm_leak_fbdev_smem;
+#if IS_ENABLED(CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM)
 module_param_unsafe(drm_leak_fbdev_smem, bool, 0600);
 MODULE_PARM_DESC(drm_leak_fbdev_smem,
 "Allow unsafe leaking fbdev physical smem address 
[default=false]");
@@ -1983,10 +1984,6 @@ static int drm_fb_helper_single_fb_probe(struct 
drm_fb_helper *fb_helper)
return ret;
}
 
-#if IS_ENABLED(CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM)
-   fb_helper->hint_leak_smem_start = drm_leak_fbdev_smem;
-#endif
-
/* push down into drivers */
ret = (*fb_helper->funcs->fb_probe)(fb_helper, );
if (ret < 0)
@@ -2185,11 +2182,8 @@ __drm_fb_helper_initial_config_and_unlock(struct 
drm_fb_helper *fb_helper)
 
info = fb_helper->info;
info->var.pixclock = 0;
-   /* Shamelessly allow physical address leaking to userspace */
-#if IS_ENABLED(CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM)
-   if (!fb_helper->hint_leak_smem_start)
-#endif
-   /* don't leak any physical addresses to userspace */
+
+   if (!drm_leak_fbdev_smem)
info->flags |= FBINFO_HIDE_SMEM_START;
 
/* Need to drop locks to avoid recursive deadlock in
diff --git a/drivers/gpu/drm/drm_fbdev_dma.c b/drivers/gpu/drm/drm_fbdev_dma.c
index cf553ac12a0f..728deffcc0d9 100644
--- a/drivers/gpu/drm/drm_fbdev_dma.c
+++ b/drivers/gpu/drm/drm_fbdev_dma.c
@@ -136,16 +136,9 @@ static int drm_fbdev_dma_helper_fb_probe(struct 
drm_fb_helper *fb_helper,
info->flags |= FBINFO_READS_FAST; /* signal caching */
info->screen_size = sizes->surface_height * fb->pitches[0];
info->screen_buffer = map.vaddr;
+   info->fix.smem_start = page_to_phys(virt_to_page(info->screen_buffer));
info->fix.smem_len = info->screen_size;
 
-#if IS_ENABLED(CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM)
-   /*
-* Shamelessly leak the physical address to user-space.
-*/
-   if (fb_helper->hint_leak_smem_start && !info->fix.smem_start)
-   info->fix.smem_start = 
page_to_phys(virt_to_page(info->screen_buffer));
-#endif
-
return 0;
 
 err_drm_client_buffer_vunmap:
diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
index c5822ec2fdd1..72032c354a30 100644
--- a/include/drm/drm_fb_helper.h
+++ b/include/drm/drm_fb_helper.h
@@ -195,15 +195,6 @@ struct drm_fb_helper {
 */
int preferred_bpp;
 
-   /**
-* @hint_leak_smem_start:
-*
-* Hint to the fbdev emulation to store the framebuffer's physical
-* address in struct _info.fix.smem_start. If the hint is unset,
-* the smem_start field should always be cleared to zero.
-*/
-   bool hint_leak_smem_start;
-
 #ifdef CONFIG_FB_DEFERRED_IO
/**
 * @fbdefio:
-- 
2.40.0

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


[PATCH v2 4/8] drm/fb-helper: Support smem_len in deferred I/O

2023-03-20 Thread Thomas Zimmermann
The size of the framebuffer can either be stored in screen_info or
smem_len. Take both into account in the deferred I/O code.

Signed-off-by: Thomas Zimmermann 
---
 drivers/gpu/drm/drm_fb_helper.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 7e96ed9efdb5..bb0b25209b3e 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -672,7 +672,7 @@ static void drm_fb_helper_memory_range_to_clip(struct 
fb_info *info, off_t off,
 void drm_fb_helper_deferred_io(struct fb_info *info, struct list_head 
*pagereflist)
 {
struct drm_fb_helper *helper = info->par;
-   unsigned long start, end, min_off, max_off;
+   unsigned long start, end, min_off, max_off, total_size;
struct fb_deferred_io_pageref *pageref;
struct drm_rect damage_area;
 
@@ -690,7 +690,11 @@ void drm_fb_helper_deferred_io(struct fb_info *info, 
struct list_head *pagerefli
 * of the screen and account for non-existing scanlines. Hence,
 * keep the covered memory area within the screen buffer.
 */
-   max_off = min(max_off, info->screen_size);
+   if (info->screen_size)
+   total_size = info->screen_size;
+   else
+   total_size = info->fix.smem_len;
+   max_off = min(max_off, total_size);
 
if (min_off < max_off) {
drm_fb_helper_memory_range_to_clip(info, min_off, max_off - 
min_off, _area);
-- 
2.40.0

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


[PATCH v2 0/8] drm/fbdev-generic: Mandatory shadow buffering

2023-03-20 Thread Thomas Zimmermann
After adding fbdev-dma and converting drivers, all users of
fbdev-generic require shadow buffering. Make it mandatory and
remove all other codepaths. This change greatly simplifies the
code for generic fbdev emulation. It will work with any driver
that supports GEM's vmap and vunmap.

The change further allows for a number of cleanups and fixes. The
flag prefer_shadow_fbdev is unused and to be removed. Probing in
fbdev-generic is now simple enough to roll back if it fails. Further
simplify the code for exporting the framebuffer's physical address.
Finally rename the symbols to follow other fbdev emulation.

v2:
* handle screen_size in separate patches (Javier)

Thomas Zimmermann (8):
  drm/fbdev-generic: Always use shadow buffering
  drm/fbdev-generic: Remove unused prefer_shadow_fbdev flag
  drm/fb-helper: Export drm_fb_helper_release_info()
  drm/fb-helper: Support smem_len in deferred I/O
  drm/fbdev-generic: Set screen size to size of GEM buffer
  drm/fbdev-generic: Clean up after failed probing
  drm/fb-helper: Consolidate CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM
  drm/fbdev-generic: Rename symbols

 drivers/gpu/drm/drm_fb_helper.c |  63 ---
 drivers/gpu/drm/drm_fbdev_dma.c |   9 +-
 drivers/gpu/drm/drm_fbdev_generic.c | 279 +---
 drivers/gpu/drm/tiny/bochs.c|   1 -
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c |   1 -
 include/drm/drm_fb_helper.h |  14 +-
 include/drm/drm_mode_config.h   |   7 -
 7 files changed, 130 insertions(+), 244 deletions(-)


base-commit: 280906872bb8c2dff02666c7a8e46f085b24
-- 
2.40.0

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


Re: [RFC PATCH v1 1/3] virtio/vsock: fix header length on skb merging

2023-03-20 Thread Stefano Garzarella

On Sun, Mar 19, 2023 at 09:51:06PM +0300, Arseniy Krasnov wrote:

This fixes header length calculation of skbuff during data appending to
it. When such skbuff is processed in dequeue callbacks, e.g. 'skb_pull()'
is called on it, 'skb->len' is dynamic value, so it is impossible to use
it in header, because value from header must be permanent for valid
credit calculation ('rx_bytes'/'fwd_cnt').

Fixes: 077706165717 ("virtio/vsock: don't use skbuff state to account credit")


I don't understand how this commit introduced this problem, can you
explain it better?

Is it related more to the credit than to the size in the header itself?

Anyway, the patch LGTM, but we should explain better the issue.

Thanks,
Stefano


Signed-off-by: Arseniy Krasnov 
---
net/vmw_vsock/virtio_transport_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/vmw_vsock/virtio_transport_common.c 
b/net/vmw_vsock/virtio_transport_common.c
index 6d15cd4d090a..3c75986e16c2 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -1091,7 +1091,7 @@ virtio_transport_recv_enqueue(struct vsock_sock *vsk,
memcpy(skb_put(last_skb, skb->len), skb->data, 
skb->len);
free_pkt = true;
last_hdr->flags |= hdr->flags;
-   last_hdr->len = cpu_to_le32(last_skb->len);
+   le32_add_cpu(_hdr->len, len);
goto out;
}
}
--
2.25.1



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


Re: [RFC PATCH v2] virtio/vsock: allocate multiple skbuffs on tx

2023-03-20 Thread Stefano Garzarella

On Sun, Mar 19, 2023 at 09:46:10PM +0300, Arseniy Krasnov wrote:

This adds small optimization for tx path: instead of allocating single
skbuff on every call to transport, allocate multiple skbuff's until
credit space allows, thus trying to send as much as possible data without
return to af_vsock.c.

Signed-off-by: Arseniy Krasnov 
---
Link to v1:
https://lore.kernel.org/netdev/2c52aa26-8181-d37a-bccd-a86bd3cbc...@sberdevices.ru/

Changelog:
v1 -> v2:
- If sent something, return number of bytes sent (even in
  case of error). Return error only if failed to sent first
  skbuff.

net/vmw_vsock/virtio_transport_common.c | 53 ++---
1 file changed, 39 insertions(+), 14 deletions(-)

diff --git a/net/vmw_vsock/virtio_transport_common.c 
b/net/vmw_vsock/virtio_transport_common.c
index 6564192e7f20..3fdf1433ec28 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -196,7 +196,8 @@ static int virtio_transport_send_pkt_info(struct vsock_sock 
*vsk,
const struct virtio_transport *t_ops;
struct virtio_vsock_sock *vvs;
u32 pkt_len = info->pkt_len;
-   struct sk_buff *skb;
+   u32 rest_len;
+   int ret;

info->type = virtio_transport_get_type(sk_vsock(vsk));

@@ -216,10 +217,6 @@ static int virtio_transport_send_pkt_info(struct 
vsock_sock *vsk,

vvs = vsk->trans;

-   /* we can send less than pkt_len bytes */
-   if (pkt_len > VIRTIO_VSOCK_MAX_PKT_BUF_SIZE)
-   pkt_len = VIRTIO_VSOCK_MAX_PKT_BUF_SIZE;
-
/* virtio_transport_get_credit might return less than pkt_len credit */
pkt_len = virtio_transport_get_credit(vvs, pkt_len);

@@ -227,17 +224,45 @@ static int virtio_transport_send_pkt_info(struct 
vsock_sock *vsk,
if (pkt_len == 0 && info->op == VIRTIO_VSOCK_OP_RW)
return pkt_len;

-   skb = virtio_transport_alloc_skb(info, pkt_len,
-src_cid, src_port,
-dst_cid, dst_port);
-   if (!skb) {
-   virtio_transport_put_credit(vvs, pkt_len);
-   return -ENOMEM;
-   }
+   ret = 0;
+   rest_len = pkt_len;
+
+   do {
+   struct sk_buff *skb;
+   size_t skb_len;
+
+   skb_len = min_t(u32, VIRTIO_VSOCK_MAX_PKT_BUF_SIZE, rest_len);
+
+   skb = virtio_transport_alloc_skb(info, skb_len,
+src_cid, src_port,
+dst_cid, dst_port);
+   if (!skb) {
+   ret = -ENOMEM;
+   break;
+   }
+
+   virtio_transport_inc_tx_pkt(vvs, skb);
+
+   ret = t_ops->send_pkt(skb);
+
+   if (ret < 0)
+   break;

-   virtio_transport_inc_tx_pkt(vvs, skb);
+   rest_len -= skb_len;


t_ops->send_pkt() is returning the number of bytes sent. Current
implementations always return `skb_len`, so there should be no problem,
but it would be better to put a comment here, or we should handle the
case where ret != skb_len to avoid future issues.


+   } while (rest_len);

-   return t_ops->send_pkt(skb);
+   /* Don't call this function with zero as argument:
+* it tries to acquire spinlock and such argument
+* makes this call useless.


Good point, can we do the same also for virtio_transport_get_credit()?
(Maybe in a separate patch)

I'm thinking if may be better to do it directly inside the functions,
but I don't have a strong opinion on that since we only call them here.

Thanks,
Stefano


+*/
+   if (rest_len)
+   virtio_transport_put_credit(vvs, rest_len);
+
+   /* Return number of bytes, if any data has been sent. */
+   if (rest_len != pkt_len)
+   ret = pkt_len - rest_len;
+
+   return ret;
}

static bool virtio_transport_inc_rx_pkt(struct virtio_vsock_sock *vvs,
--
2.25.1



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


Re: [RFC PATCH v1] virtio/vsock: check transport before skb allocation

2023-03-20 Thread Stefano Garzarella

On Fri, Mar 17, 2023 at 01:37:10PM +0300, Arseniy Krasnov wrote:

Pointer to transport could be checked before allocation of skbuff, thus
there is no need to free skbuff when this pointer is NULL.

Signed-off-by: Arseniy Krasnov 
---
net/vmw_vsock/virtio_transport_common.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)


LGTM, I think net-next is fine for this.

Reviewed-by: Stefano Garzarella 

Thanks,
Stefano



diff --git a/net/vmw_vsock/virtio_transport_common.c 
b/net/vmw_vsock/virtio_transport_common.c
index cda587196475..607149259e8b 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -867,6 +867,9 @@ static int virtio_transport_reset_no_sock(const struct 
virtio_transport *t,
if (le16_to_cpu(hdr->op) == VIRTIO_VSOCK_OP_RST)
return 0;

+   if (!t)
+   return -ENOTCONN;
+
reply = virtio_transport_alloc_skb(, 0,
   le64_to_cpu(hdr->dst_cid),
   le32_to_cpu(hdr->dst_port),
@@ -875,11 +878,6 @@ static int virtio_transport_reset_no_sock(const struct 
virtio_transport *t,
if (!reply)
return -ENOMEM;

-   if (!t) {
-   kfree_skb(reply);
-   return -ENOTCONN;
-   }
-
return t->send_pkt(reply);
}

--
2.25.1



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


Re: [PATCH 4/6] drm/fbdev-generic: Clean up after failed probing

2023-03-20 Thread Thomas Zimmermann

Hi Javier

Am 17.03.23 um 13:24 schrieb Javier Martinez Canillas:

Thomas Zimmermann  writes:

[...]


@@ -91,36 +93,52 @@ static int drm_fbdev_fb_probe(struct drm_fb_helper 
*fb_helper,
  
  	fb_helper->buffer = buffer;

fb_helper->fb = buffer->fb;
-   fb = buffer->fb;
+
+   screen_size = buffer->gem->size;


[...]


-   info->screen_size = sizes->surface_height * fb->pitches[0];


[...]

I wonder if this change should be a separate patch? I know that it should
be the same size but still feels like an unrelated change that deserves a
different patch and description.


This comment made me look up the exact meaning if screen_size, which is 
"Amount of ioremapped VRAM or 0". [1] Other drivers with shadow buffers 
(udlfb, metronomefb) apparently don't set this field. So the generic 
fbdev probably shouldn't either. The size of the video memory (physical 
or shadowed) in in fix.smem_len. [2] From grep'ing through the source 
code, it's not clear to me why screen_size exists in the first place.


Best regards
Thomas

[1] https://elixir.bootlin.com/linux/latest/source/include/linux/fb.h#L494
[2] 
https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/fb.h#L161




[...]
   

-   /* Set a default deferred I/O handler */
+   /* deferred I/O */


I would either have it as /* Generic fbdev deferred I/O handler */ or just
remove the comment. I understand why you are removing the "default", since
implies that drivers can change the handler and that's not the case here.

But I think that just leaving the "deferred I/O" comment there doesn't say
that much.

Reviewed-by: Javier Martinez Canillas 



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


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

Re: [PATCH v2 0/2] [RFC] virtio-rng entropy leak reporting feature

2023-03-20 Thread Amit Shah
On Mon, 2023-03-13 at 19:05 +0100, Amit Shah wrote:
> Hey Herbert / Jason / crypto maintainers,

[...]

> Let's wait a couple more days for responses, otherwise I suggest you
> resubmit to kickstart a new discussion, with the note that Jason had
> something else in mind - so that it doesn't appear as though we're
> trying to override that.

I reached out to Jason on IRC, and he mentioned he will follow up with
a patch that incorporates ideas from your patch plus hooking into
random.c.  Sounds promising!

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


[PATCH 2/2] vdpa/snet: support the suspend vDPA callback

2023-03-20 Thread Alvaro Karsz
When suspend is called, the driver sends a suspend command to the DPU
through the control mechanism.

Signed-off-by: Alvaro Karsz 
---
 drivers/vdpa/solidrun/snet_ctrl.c |  6 ++
 drivers/vdpa/solidrun/snet_main.c | 15 +++
 drivers/vdpa/solidrun/snet_vdpa.h |  1 +
 3 files changed, 22 insertions(+)

diff --git a/drivers/vdpa/solidrun/snet_ctrl.c 
b/drivers/vdpa/solidrun/snet_ctrl.c
index 54909549a00..7d08e6f 100644
--- a/drivers/vdpa/solidrun/snet_ctrl.c
+++ b/drivers/vdpa/solidrun/snet_ctrl.c
@@ -15,6 +15,7 @@
 enum snet_ctrl_opcodes {
SNET_CTRL_OP_DESTROY = 1,
SNET_CTRL_OP_READ_VQ_STATE,
+   SNET_CTRL_OP_SUSPEND,
 };
 
 #define SNET_CTRL_TIMEOUT  200
@@ -316,3 +317,8 @@ int snet_read_vq_state(struct snet *snet, u16 idx, struct 
vdpa_vq_state *state)
return snet_ctrl_read_from_dpu(snet, SNET_CTRL_OP_READ_VQ_STATE, idx, 
state,
   sizeof(*state));
 }
+
+int snet_suspend_dev(struct snet *snet)
+{
+   return snet_send_ctrl_msg(snet, SNET_CTRL_OP_SUSPEND, 0);
+}
diff --git a/drivers/vdpa/solidrun/snet_main.c 
b/drivers/vdpa/solidrun/snet_main.c
index daeb69d00ed..96830e58211 100644
--- a/drivers/vdpa/solidrun/snet_main.c
+++ b/drivers/vdpa/solidrun/snet_main.c
@@ -483,6 +483,20 @@ static void snet_set_config(struct vdpa_device *vdev, 
unsigned int offset,
iowrite8(*buf_ptr++, cfg_ptr + i);
 }
 
+static int snet_suspend(struct vdpa_device *vdev)
+{
+   struct snet *snet = vdpa_to_snet(vdev);
+   int ret;
+
+   ret = snet_suspend_dev(snet);
+   if (ret)
+   SNET_ERR(snet->pdev, "SNET[%u] suspend failed, err: %d\n", 
snet->sid, ret);
+   else
+   SNET_DBG(snet->pdev, "Suspend SNET[%u] device\n", snet->sid);
+
+   return ret;
+}
+
 static const struct vdpa_config_ops snet_config_ops = {
.set_vq_address = snet_set_vq_address,
.set_vq_num = snet_set_vq_num,
@@ -508,6 +522,7 @@ static const struct vdpa_config_ops snet_config_ops = {
.set_status = snet_set_status,
.get_config = snet_get_config,
.set_config = snet_set_config,
+   .suspend= snet_suspend,
 };
 
 static int psnet_open_pf_bar(struct pci_dev *pdev, struct psnet *psnet)
diff --git a/drivers/vdpa/solidrun/snet_vdpa.h 
b/drivers/vdpa/solidrun/snet_vdpa.h
index a4213e97cfc..6dbb95fe2b7 100644
--- a/drivers/vdpa/solidrun/snet_vdpa.h
+++ b/drivers/vdpa/solidrun/snet_vdpa.h
@@ -201,5 +201,6 @@ void psnet_create_hwmon(struct pci_dev *pdev);
 void snet_ctrl_clear(struct snet *snet);
 int snet_destroy_dev(struct snet *snet);
 int snet_read_vq_state(struct snet *snet, u16 idx, struct vdpa_vq_state 
*state);
+int snet_suspend_dev(struct snet *snet);
 
 #endif //_SNET_VDPA_H_
-- 
2.34.1

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


[PATCH 1/2] vdpa/snet: support getting and setting VQ state

2023-03-20 Thread Alvaro Karsz
This patch adds the get_vq_state and set_vq_state vDPA callbacks.

In order to get the VQ state, the state needs to be read from the DPU.
In order to allow that, the old messaging mechanism is replaced with a new,
flexible control mechanism.
This mechanism allows to read data from the DPU.

The mechanism can be used if the negotiated config version is 2 or
higher.

If the new mechanism is used when the config version is 1, it will call
snet_send_ctrl_msg_old, which is config 1 compatible.

Signed-off-by: Alvaro Karsz 
---
 drivers/vdpa/solidrun/Makefile |   1 +
 drivers/vdpa/solidrun/snet_ctrl.c  | 318 +
 drivers/vdpa/solidrun/snet_hwmon.c |   2 +-
 drivers/vdpa/solidrun/snet_main.c  | 111 --
 drivers/vdpa/solidrun/snet_vdpa.h  |  17 +-
 5 files changed, 378 insertions(+), 71 deletions(-)
 create mode 100644 drivers/vdpa/solidrun/snet_ctrl.c

diff --git a/drivers/vdpa/solidrun/Makefile b/drivers/vdpa/solidrun/Makefile
index c0aa3415bf7..9116252cd5f 100644
--- a/drivers/vdpa/solidrun/Makefile
+++ b/drivers/vdpa/solidrun/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_SNET_VDPA) += snet_vdpa.o
 snet_vdpa-$(CONFIG_SNET_VDPA) += snet_main.o
+snet_vdpa-$(CONFIG_SNET_VDPA) += snet_ctrl.o
 ifdef CONFIG_HWMON
 snet_vdpa-$(CONFIG_SNET_VDPA) += snet_hwmon.o
 endif
diff --git a/drivers/vdpa/solidrun/snet_ctrl.c 
b/drivers/vdpa/solidrun/snet_ctrl.c
new file mode 100644
index 000..54909549a00
--- /dev/null
+++ b/drivers/vdpa/solidrun/snet_ctrl.c
@@ -0,0 +1,318 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * SolidRun DPU driver for control plane
+ *
+ * Copyright (C) 2022-2023 SolidRun
+ *
+ * Author: Alvaro Karsz 
+ *
+ */
+
+#include 
+
+#include "snet_vdpa.h"
+
+enum snet_ctrl_opcodes {
+   SNET_CTRL_OP_DESTROY = 1,
+   SNET_CTRL_OP_READ_VQ_STATE,
+};
+
+#define SNET_CTRL_TIMEOUT  200
+
+#define SNET_CTRL_DATA_SIZE_MASK   0x
+#define SNET_CTRL_IN_PROCESS_MASK  0x0001
+#define SNET_CTRL_CHUNK_RDY_MASK   0x0002
+#define SNET_CTRL_ERROR_MASK   0x0FFC
+
+#define SNET_VAL_TO_ERR(val)   (-(((val) & SNET_CTRL_ERROR_MASK) >> 
18))
+#define SNET_EMPTY_CTRL(val)   (((val) & SNET_CTRL_ERROR_MASK) || \
+   !((val) & 
SNET_CTRL_IN_PROCESS_MASK))
+#define SNET_DATA_READY(val)   ((val) & (SNET_CTRL_ERROR_MASK | 
SNET_CTRL_CHUNK_RDY_MASK))
+
+/* Control register used to read data from the DPU */
+struct snet_ctrl_reg_ctrl {
+   /* Chunk size in 4B words */
+   u16 data_size;
+   /* We are in the middle of a command */
+   u16 in_process:1;
+   /* A data chunk is ready and can be consumed */
+   u16 chunk_ready:1;
+   /* Error code */
+   u16 error:10;
+   /* Saved for future usage */
+   u16 rsvd:4;
+};
+
+/* Opcode register */
+struct snet_ctrl_reg_op {
+   u16 opcode;
+   /* Only if VQ index is relevant for the command */
+   u16 vq_idx;
+};
+
+struct snet_ctrl_regs {
+   struct snet_ctrl_reg_op op;
+   struct snet_ctrl_reg_ctrl ctrl;
+   u32 rsvd;
+   u32 data[];
+};
+
+static struct snet_ctrl_regs __iomem *snet_get_ctrl(struct snet *snet)
+{
+   return snet->bar + snet->psnet->cfg.ctrl_off;
+}
+
+static int snet_wait_for_empty_ctrl(struct snet_ctrl_regs __iomem *regs)
+{
+   u32 val;
+
+   return readx_poll_timeout(ioread32, >ctrl, val, 
SNET_EMPTY_CTRL(val), 10,
+ SNET_CTRL_TIMEOUT);
+}
+
+static int snet_wait_for_empty_op(struct snet_ctrl_regs __iomem *regs)
+{
+   u32 val;
+
+   return readx_poll_timeout(ioread32, >op, val, !val, 10, 
SNET_CTRL_TIMEOUT);
+}
+
+static int snet_wait_for_data(struct snet_ctrl_regs __iomem *regs)
+{
+   u32 val;
+
+   return readx_poll_timeout(ioread32, >ctrl, val, 
SNET_DATA_READY(val), 10,
+ SNET_CTRL_TIMEOUT);
+}
+
+static u32 snet_read32_word(struct snet_ctrl_regs __iomem *ctrl_regs, u16 
word_idx)
+{
+   return ioread32(_regs->data[word_idx]);
+}
+
+static u32 snet_read_ctrl(struct snet_ctrl_regs __iomem *ctrl_regs)
+{
+   return ioread32(_regs->ctrl);
+}
+
+static void snet_write_ctrl(struct snet_ctrl_regs __iomem *ctrl_regs, u32 val)
+{
+   iowrite32(val, _regs->ctrl);
+}
+
+static void snet_write_op(struct snet_ctrl_regs __iomem *ctrl_regs, u32 val)
+{
+   iowrite32(val, _regs->op);
+}
+
+static int snet_wait_for_dpu_completion(struct snet_ctrl_regs __iomem 
*ctrl_regs)
+{
+   /* Wait until the DPU finishes completely.
+* It will clear the opcode register.
+*/
+   return snet_wait_for_empty_op(ctrl_regs);
+}
+
+/* Reading ctrl from the DPU:
+ * buf_size must be 4B aligned
+ *
+ * Steps:
+ *
+ * (1) Verify that the DPU is not in the middle of another operation by
+ * reading the in_process and error bits in the control register.
+ * (2) Write the request opcode and the VQ idx in 

[PATCH 0/2] vdpa/snet: support [s/g]et_vq_state and suspend

2023-03-20 Thread Alvaro Karsz
Add more vDPA callbacks.

[s/g]et_vq_state is added in patch 1, including a new control mechanism
to read data from the DPU.

suspend is added in patch 2.

Alvaro Karsz (2):
  vdpa/snet: support getting and setting VQ state
  vdpa/snet: support the suspend vDPA callback

 drivers/vdpa/solidrun/Makefile |   1 +
 drivers/vdpa/solidrun/snet_ctrl.c  | 324 +
 drivers/vdpa/solidrun/snet_hwmon.c |   2 +-
 drivers/vdpa/solidrun/snet_main.c  | 126 ++-
 drivers/vdpa/solidrun/snet_vdpa.h  |  18 +-
 5 files changed, 400 insertions(+), 71 deletions(-)
 create mode 100644 drivers/vdpa/solidrun/snet_ctrl.c

-- 
2.34.1

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


Re: [PATCH v3 03/11] vdpa: Add set_irq_affinity callback in vdpa_config_ops

2023-03-20 Thread Jason Wang
On Fri, Mar 17, 2023 at 3:45 PM Yongji Xie  wrote:
>
> On Thu, Mar 16, 2023 at 12:03 PM Jason Wang  wrote:
> >
> > On Tue, Feb 28, 2023 at 5:42 PM Xie Yongji  wrote:
> > >
> > > This introduces set_irq_affinity callback in
> > > vdpa_config_ops so that vdpa device driver can
> > > get the interrupt affinity hint from the virtio
> > > device driver. The interrupt affinity hint would
> > > be needed by the interrupt affinity spreading
> > > mechanism.
> > >
> > > Signed-off-by: Xie Yongji 
> > > ---
> > >  drivers/virtio/virtio_vdpa.c | 4 
> > >  include/linux/vdpa.h | 9 +
> > >  2 files changed, 13 insertions(+)
> > >
> > > diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
> > > index f72696b4c1c2..9eee8afabda8 100644
> > > --- a/drivers/virtio/virtio_vdpa.c
> > > +++ b/drivers/virtio/virtio_vdpa.c
> > > @@ -282,9 +282,13 @@ static int virtio_vdpa_find_vqs(struct virtio_device 
> > > *vdev, unsigned int nvqs,
> > > struct virtio_vdpa_device *vd_dev = to_virtio_vdpa_device(vdev);
> > > struct vdpa_device *vdpa = vd_get_vdpa(vdev);
> > > const struct vdpa_config_ops *ops = vdpa->config;
> > > +   struct irq_affinity default_affd = { 0 };
> > > struct vdpa_callback cb;
> > > int i, err, queue_idx = 0;
> > >
> > > +   if (ops->set_irq_affinity)
> > > +   ops->set_irq_affinity(vdpa, desc ? desc : _affd);
> > > +
> > > for (i = 0; i < nvqs; ++i) {
> > > if (!names[i]) {
> > > vqs[i] = NULL;
> > > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
> > > index d61f369f9cd6..10bd22387276 100644
> > > --- a/include/linux/vdpa.h
> > > +++ b/include/linux/vdpa.h
> > > @@ -259,6 +259,13 @@ struct vdpa_map_file {
> > >   * @vdev: vdpa device
> > >   * @idx: virtqueue index
> > >   * Returns the irq affinity mask
> > > + * @set_irq_affinity:  Pass the irq affinity hint (best effort)
> >
> > Note that this could easily confuse the users. I wonder if we can
> > unify it with set_irq_affinity. Looking at vduse's implementation, it
> > should be possible.
> >
>
> Do you mean unify set_irq_affinity() with set_vq_affinity()? Actually
> I didn't get how to achieve that. The set_vq_affinity() callback is
> called by virtio_config_ops.set_vq_affinity() but the set_irq_affinity
> is called by virtio_config_ops.find_vqs(), I don't know where to call
> the unified callback.

I meant, can we stick a single per vq affinity config ops then use
that in virtio-vpda's find_vqs() by something like:

masks = create_affinity_masks(dev->vq_num, desc);
for (i = 0; i < dev->vq_num; i++)
config->set_vq_affinity()
...

?

Thanks

>
> Thanks,
> Yongji
>

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

Re: [PATCH] vdpa/mlx5: Remove debugfs file after device unregister

2023-03-20 Thread Jason Wang
On Mon, Mar 20, 2023 at 5:07 PM Eli Cohen  wrote:
>
> On 17/03/2023 5:32, Jason Wang wrote:
> > On Sun, Mar 12, 2023 at 4:41 PM Eli Cohen  wrote:
> >> When deleting the vdpa device, the debugfs files need to be removed so
> >> need to remove debugfs after the device has been unregistered.
> >>
> >> This fixes null pointer dereference when someone deletes the device
> >> after debugfs has been populated.
> >>
> >> Fixes: 294221004322 ("vdpa/mlx5: Add debugfs subtree")
> >> Signed-off-by: Eli Cohen 
> >> ---
> >>   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 3858ba1e8975..3f6149f2ffd4 100644
> >> --- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> >> +++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> >> @@ -3322,8 +3322,6 @@ static void mlx5_vdpa_dev_del(struct vdpa_mgmt_dev 
> >> *v_mdev, struct vdpa_device *
> >>  struct mlx5_vdpa_net *ndev = to_mlx5_vdpa_ndev(mvdev);
> >>  struct workqueue_struct *wq;
> >>
> >> -   mlx5_vdpa_remove_debugfs(ndev->debugfs);
> >> -   ndev->debugfs = NULL;
> >>  if (ndev->nb_registered) {
> >>  ndev->nb_registered = false;
> >>  mlx5_notifier_unregister(mvdev->mdev, >nb);
> >> @@ -3332,6 +3330,8 @@ static void mlx5_vdpa_dev_del(struct vdpa_mgmt_dev 
> >> *v_mdev, struct vdpa_device *
> >>  mvdev->wq = NULL;
> >>  destroy_workqueue(wq);
> >>  _vdpa_unregister_device(dev);
> > What if the user tries to access debugfs after _vdpa_unregister_device()?
> I don't see an issue with that. During the process of deleting a device,
> the resources are destroyed and their corresponding debugfs files are
> also destroyed just prior to destroying the resource.

For example, rx_flow_table_show() requires the structure mlx5_vdpa_net
alive, but it seems the structure has been freed after
_vdpa_unregister_device().

If a user tries to access that file after _vdpa_unregister_device()
but before mlx5_vdpa_remove_debugfs(), will we end up with
use-after-free?

Thanks


> >
> > Thanks
> >
> >> +   mlx5_vdpa_remove_debugfs(ndev->debugfs);
> >> +   ndev->debugfs = NULL;
> >>  mgtdev->ndev = NULL;
> >>   }
> >>
> >> --
> >> 2.38.1
> >>
>

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

Re: [RFC net-next 0/8] virtio_net: refactor xdp codes

2023-03-20 Thread Xuan Zhuo
On Wed, 15 Mar 2023 12:10:34 +0800, Xuan Zhuo  
wrote:

Ping

Thanks.


> Due to historical reasons, the implementation of XDP in virtio-net is 
> relatively
> chaotic. For example, the processing of XDP actions has two copies of similar
> code. Such as page, xdp_page processing, etc.
>
> The purpose of this patch set is to refactor these code. Reduce the difficulty
> of subsequent maintenance. Subsequent developers will not introduce new bugs
> because of some complex logical relationships.
>
> In addition, the supporting to AF_XDP that I want to submit later will also 
> need
> to reuse the logic of XDP, such as the processing of actions, I don't want to
> introduce a new similar code. In this way, I can reuse these codes in the
> future.
>
> This patches are developed on the top of another patch set[1]. I may have to
> wait to merge this. So this patch set is a RFC.
>
> Please review.
>
> Thanks.
>
> [1]. 
> https://lore.kernel.org/netdev/20230315015223.89137-1-xuanz...@linux.alibaba.com/
>
>
> Xuan Zhuo (8):
>   virtio_net: mergeable xdp: put old page immediately
>   virtio_net: mergeable xdp: introduce mergeable_xdp_prepare
>   virtio_net: introduce virtnet_xdp_handler() to seprate the logic of
> run xdp
>   virtio_net: separate the logic of freeing xdp shinfo
>   virtio_net: separate the logic of freeing the rest mergeable buf
>   virtio_net: auto release xdp shinfo
>   virtio_net: introduce receive_mergeable_xdp()
>   virtio_net: introduce receive_small_xdp()
>
>  drivers/net/virtio_net.c | 615 +++
>  1 file changed, 357 insertions(+), 258 deletions(-)
>
> --
> 2.32.0.3.g01195cf9f
>
> ___
> Virtualization mailing list
> Virtualization@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/virtualization
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH v2 1/2] vdpa/mlx5: Extend driver support for new features

2023-03-20 Thread Jason Wang
On Fri, Mar 17, 2023 at 9:58 PM Parav Pandit  wrote:
>
>
>
> > From: Eli Cohen 
> > Sent: Wednesday, March 15, 2023 3:28 AM
> >
> > Extend the possible list for features that can be supported by firmware.
> > Note that different versions of firmware may or may not support these
> > features. The driver is made aware of them by querying the firmware.
> >
> > While doing this, improve the code so we use enum names instead of hard
> > coded numerical values.
> >
> > The new features supported by the driver are the following:
> >
> > VIRTIO_NET_F_MRG_RXBUF
> > VIRTIO_NET_F_HOST_UFO
> UFO is deprecated in Linux kernel, there are no known user either and we do 
> not plan to support it.

Note that there's an emulation code for preserving migration
compatibility in the kernel.

> Please remove this entry along with below GUEST_UFO.

If there's no plan for supporting migration from existing software
backends, we can remove this.

Thanks

>
> > VIRTIO_NET_F_HOST_ECN
> > VIRTIO_NET_F_GUEST_UFO
> > VIRTIO_NET_F_GUEST_ECN
> > VIRTIO_NET_F_GUEST_TSO6
> > VIRTIO_NET_F_GUEST_TSO4
>

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