date:20211026

Re: [PATCH v6 8/8] eni_vdpa: add vDPA driver for Alibaba ENI

2021-10-26 Thread Jason Wang

On Wed, Oct 27, 2021 at 10:47 AM Wu Zongyong
 wrote:
>
> On Mon, Oct 25, 2021 at 12:40:41PM +0800, Jason Wang wrote:
> >
> > 在 2021/10/25 上午11:21, Wu Zongyong 写道:
> > > On Mon, Oct 25, 2021 at 10:27:31AM +0800, Jason Wang wrote:
> > > > On Fri, Oct 22, 2021 at 10:44 AM Wu Zongyong
> > > >  wrote:
> > > > > This patch adds a new vDPA driver for Alibaba ENI(Elastic Network
> > > > > Interface) which is build upon virtio 0.9.5 specification.
> > > > > And this driver doesn't support to run on BE host.
> > > > >
> > > > > Signed-off-by: Wu Zongyong 
> > > > > ---
> > > > >   drivers/vdpa/Kconfig|   8 +
> > > > >   drivers/vdpa/Makefile   |   1 +
> > > > >   drivers/vdpa/alibaba/Makefile   |   3 +
> > > > >   drivers/vdpa/alibaba/eni_vdpa.c | 553 
> > > > > 
> > > > >   4 files changed, 565 insertions(+)
> > > > >   create mode 100644 drivers/vdpa/alibaba/Makefile
> > > > >   create mode 100644 drivers/vdpa/alibaba/eni_vdpa.c
> > > > >
> > > > > diff --git a/drivers/vdpa/Kconfig b/drivers/vdpa/Kconfig
> > > > > index 3d91982d8371..c0232a2148a7 100644
> > > > > --- a/drivers/vdpa/Kconfig
> > > > > +++ b/drivers/vdpa/Kconfig
> > > > > @@ -78,4 +78,12 @@ config VP_VDPA
> > > > >  help
> > > > >This kernel module bridges virtio PCI device to vDPA bus.
> > > > >
> > > > > +config ALIBABA_ENI_VDPA
> > > > > +   tristate "vDPA driver for Alibaba ENI"
> > > > > +   select VIRTIO_PCI_LEGACY_LIB
> > > > > +   depends on PCI_MSI && !CPU_BIG_ENDIAN
> > > > > +   help
> > > > > + VDPA driver for Alibaba ENI(Elastic Network Interface) 
> > > > > which is build upon
> > > > > + virtio 0.9.5 specification.
> > > > > +
> > > > >   endif # VDPA
> > > > > diff --git a/drivers/vdpa/Makefile b/drivers/vdpa/Makefile
> > > > > index f02ebed33f19..15665563a7f4 100644
> > > > > --- a/drivers/vdpa/Makefile
> > > > > +++ b/drivers/vdpa/Makefile
> > > > > @@ -5,3 +5,4 @@ obj-$(CONFIG_VDPA_USER) += vdpa_user/
> > > > >   obj-$(CONFIG_IFCVF)+= ifcvf/
> > > > >   obj-$(CONFIG_MLX5_VDPA) += mlx5/
> > > > >   obj-$(CONFIG_VP_VDPA)+= virtio_pci/
> > > > > +obj-$(CONFIG_ALIBABA_ENI_VDPA) += alibaba/
> > > > > diff --git a/drivers/vdpa/alibaba/Makefile 
> > > > > b/drivers/vdpa/alibaba/Makefile
> > > > > new file mode 100644
> > > > > index ..ef4aae69f87a
> > > > > --- /dev/null
> > > > > +++ b/drivers/vdpa/alibaba/Makefile
> > > > > @@ -0,0 +1,3 @@
> > > > > +# SPDX-License-Identifier: GPL-2.0
> > > > > +obj-$(CONFIG_ALIBABA_ENI_VDPA) += eni_vdpa.o
> > > > > +
> > > > > diff --git a/drivers/vdpa/alibaba/eni_vdpa.c 
> > > > > b/drivers/vdpa/alibaba/eni_vdpa.c
> > > > > new file mode 100644
> > > > > index ..6a09f157d810
> > > > > --- /dev/null
> > > > > +++ b/drivers/vdpa/alibaba/eni_vdpa.c
> > > > > @@ -0,0 +1,553 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > > +/*
> > > > > + * vDPA bridge driver for Alibaba ENI(Elastic Network Interface)
> > > > > + *
> > > > > + * Copyright (c) 2021, Alibaba Inc. All rights reserved.
> > > > > + * Author: Wu Zongyong 
> > > > > + *
> > > > > + */
> > > > > +
> > > > > +#include "linux/bits.h"
> > > > > +#include 
> > > > > +#include 
> > > > > +#include 
> > > > > +#include 
> > > > > +#include 
> > > > > +#include 
> > > > > +#include 
> > > > > +#include 
> > > > > +#include 
> > > > > +#include 
> > > > > +
> > > > > +#define ENI_MSIX_NAME_SIZE 256
> > > > > +
> > > > > +#define ENI_ERR(pdev, fmt, ...)\
> > > > > +   dev_err(>dev, "%s"fmt, "eni_vdpa: ", ##__VA_ARGS__)
> > > > > +#define ENI_DBG(pdev, fmt, ...)\
> > > > > +   dev_dbg(>dev, "%s"fmt, "eni_vdpa: ", ##__VA_ARGS__)
> > > > > +#define ENI_INFO(pdev, fmt, ...) \
> > > > > +   dev_info(>dev, "%s"fmt, "eni_vdpa: ", ##__VA_ARGS__)
> > > > > +
> > > > > +struct eni_vring {
> > > > > +   void __iomem *notify;
> > > > > +   char msix_name[ENI_MSIX_NAME_SIZE];
> > > > > +   struct vdpa_callback cb;
> > > > > +   int irq;
> > > > > +};
> > > > > +
> > > > > +struct eni_vdpa {
> > > > > +   struct vdpa_device vdpa;
> > > > > +   struct virtio_pci_legacy_device ldev;
> > > > > +   struct eni_vring *vring;
> > > > > +   struct vdpa_callback config_cb;
> > > > > +   char msix_name[ENI_MSIX_NAME_SIZE];
> > > > > +   int config_irq;
> > > > > +   int queues;
> > > > > +   int vectors;
> > > > > +};
> > > > > +
> > > > > +static struct eni_vdpa *vdpa_to_eni(struct vdpa_device *vdpa)
> > > > > +{
> > > > > +   return container_of(vdpa, struct eni_vdpa, vdpa);
> > > > > +}
> > > > > +
> > > > > +static struct virtio_pci_legacy_device *vdpa_to_ldev(struct 
> > > > > vdpa_device *vdpa)
> > > > > +{
> > > > > +   struct eni_vdpa *eni_vdpa = vdpa_to_eni(vdpa);
> > > > > +
> > > > > +   return _vdpa->ldev;
> > > > > +}
> > > > > +
> > > > > +static u64 eni_vdpa_get_features(struct vdpa_device *vdpa)
> > > > > +{
> > > > > +

Re: [PATCH V3 11/11] vhost: allow userspace to create workers

2021-10-26 Thread Jason Wang

On Tue, Oct 26, 2021 at 11:45 PM Stefan Hajnoczi  wrote:
>
> On Tue, Oct 26, 2021 at 01:37:14PM +0800, Jason Wang wrote:
> >
> > 在 2021/10/22 下午1:19, Mike Christie 写道:
> > > This patch allows userspace to create workers and bind them to vqs. You
> > > can have N workers per dev and also share N workers with M vqs.
> > >
> > > Signed-off-by: Mike Christie 
> >
> >
> > A question, who is the best one to determine the binding? Is it the VMM
> > (Qemu etc) or the management stack? If the latter, it looks to me it's
> > better to expose this via sysfs?
>
> A few options that let the management stack control vhost worker CPU
> affinity:
>
> 1. The management tool opens the vhost device node, calls
>ioctl(VHOST_SET_VRING_WORKER), sets up CPU affinity, and then passes
>the fd to the VMM. In this case the VMM is still able to call the
>ioctl, which may be undesirable from an attack surface perspective.

Yes, and we can't do post or dynamic configuration afterwards after
the VM is launched?

>
> 2. The VMM calls ioctl(VHOST_SET_VRING_WORKER) itself and the management
>tool queries the vq:worker details from the VMM (e.g. a new QEMU QMP
>query-vhost-workers command similar to query-iothreads). The
>management tool can then control CPU affinity on the vhost worker
>threads.
>
>(This is how CPU affinity works in QEMU and libvirt today.)

Then we also need a "bind-vhost-workers" command.

>
> 3. The sysfs approach you suggested. Does sysfs export vq-0/, vq-1/, etc
>directories with a "worker" attribute?

Something like this.

> Do we need to define a point
>when the VMM has set up vqs and the management stack is able to query
>them?

It could be the point that the vhost fd is opened.

>  Vhost devices currently pre-allocate the maximum number of vqs
>and I'm not sure how to determine the number of vqs that will
>actually be used?

It requires more information to be exposed. But before this, we should
allow the dynamic binding of between vq and worker.

>
>One advantage of this is that access to the vq:worker mapping can be
>limited to the management stack and the VMM cannot access it. But it
>seems a little tricky because the vhost model today doesn't use sysfs
>or define a lifecycle where the management stack can configure
>devices.

Yes.

Thanks

>
> Stefan

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

Re: [PATCH net-next] net: virtio: use eth_hw_addr_set()

2021-10-26 Thread Jason Wang

On Wed, Oct 27, 2021 at 1:56 AM Jakub Kicinski  wrote:
>
> Commit 406f42fa0d3c ("net-next: When a bond have a massive amount
> of VLANs...") introduced a rbtree for faster Ethernet address look
> up. To maintain netdev->dev_addr in this tree we need to make all
> the writes to it go through appropriate helpers.

I think the title should be "net: virtio: use eth_hw_addr_set()"

>
> Signed-off-by: Jakub Kicinski 
> ---
> CC: m...@redhat.com
> CC: jasow...@redhat.com
> CC: virtualization@lists.linux-foundation.org
> ---
>  drivers/net/virtio_net.c | 10 +++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index c501b5974aee..b7f35aff8e82 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -3177,12 +3177,16 @@ static int virtnet_probe(struct virtio_device *vdev)
> dev->max_mtu = MAX_MTU;
>
> /* Configuration may specify what MAC to use.  Otherwise random. */
> -   if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC))
> +   if (virtio_has_feature(vdev, VIRTIO_NET_F_MAC)) {
> +   u8 addr[MAX_ADDR_LEN];
> +
> virtio_cread_bytes(vdev,
>offsetof(struct virtio_net_config, mac),
> -  dev->dev_addr, dev->addr_len);
> -   else
> +  addr, dev->addr_len);
> +   dev_addr_set(dev, addr);
> +   } else {
> eth_hw_addr_random(dev);
> +   }

Do we need to change virtnet_set_mac_address() as well?

Thanks

>
> /* Set up our device-specific information */
> vi = netdev_priv(dev);
> --
> 2.31.1
>

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

Re: [PATCH -next] virtio-pci: fix error return code in vp_legacy_probe()

2021-10-26 Thread Jason Wang

On Tue, Oct 26, 2021 at 9:39 PM Yang Yingliang  wrote:
>
> Return error code if pci_iomap() fails in vp_legacy_probe()
>
> Reported-by: Hulk Robot 
> Fixes: c3ca8a3eeb54 ("virtio-pci: introduce legacy device module")
> Signed-off-by: Yang Yingliang 
> ---

Acked-by: Jason Wang 

>  drivers/virtio/virtio_pci_legacy_dev.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_pci_legacy_dev.c 
> b/drivers/virtio/virtio_pci_legacy_dev.c
> index 9b97680dd02b..4ca3ef38d3bf 100644
> --- a/drivers/virtio/virtio_pci_legacy_dev.c
> +++ b/drivers/virtio/virtio_pci_legacy_dev.c
> @@ -45,8 +45,10 @@ int vp_legacy_probe(struct virtio_pci_legacy_device *ldev)
> return rc;
>
> ldev->ioaddr = pci_iomap(pci_dev, 0, 0);
> -   if (!ldev->ioaddr)
> +   if (!ldev->ioaddr) {
> +   rc = -ENOMEM;
> goto err_iomap;
> +   }
>
> ldev->isr = ldev->ioaddr + VIRTIO_PCI_ISR;
>
> --
> 2.25.1
>

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

Re: [PATCH] virtio-ring: fix DMA metadata flags

2021-10-26 Thread Jason Wang

On Tue, Oct 26, 2021 at 9:31 PM Vincent Whitchurch
 wrote:
>
> The flags are currently overwritten, leading to the wrong direction
> being passed to the DMA unmap functions.
>
> Fixes: 72b5e8958738aaa4 ("virtio-ring: store DMA metadata in desc_extra for 
> split virtqueue")
> Signed-off-by: Vincent Whitchurch 

Acked-by: Jason Wang 

Thanks

> ---
>  drivers/virtio/virtio_ring.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index dd95dfd85e98..3035bb6f5458 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -576,7 +576,7 @@ static inline int virtqueue_add_split(struct virtqueue 
> *_vq,
> /* Last one doesn't continue. */
> desc[prev].flags &= cpu_to_virtio16(_vq->vdev, ~VRING_DESC_F_NEXT);
> if (!indirect && vq->use_dma_api)
> -   vq->split.desc_extra[prev & (vq->split.vring.num - 1)].flags =
> +   vq->split.desc_extra[prev & (vq->split.vring.num - 1)].flags 
> &=
> ~VRING_DESC_F_NEXT;
>
> if (indirect) {
> --
> 2.28.0
>

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

[PATCH V5 4/4] virtio-scsi: don't let virtio core to validate used buffer length

2021-10-26 Thread Jason Wang

We never tries to use used length, so the patch prevents the virtio
core from validating used length.

Signed-off-by: Jason Wang 
---
 drivers/scsi/virtio_scsi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 07d0250f17c3..03b09ecea42d 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -977,6 +977,7 @@ static unsigned int features[] = {
 static struct virtio_driver virtio_scsi_driver = {
.feature_table = features,
.feature_table_size = ARRAY_SIZE(features),
+   .suppress_used_validation = true,
.driver.name = KBUILD_MODNAME,
.driver.owner = THIS_MODULE,
.id_table = id_table,
-- 
2.25.1

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

[PATCH V5 3/4] virtio-blk: don't let virtio core to validate used length

2021-10-26 Thread Jason Wang

We never tries to use used length, so the patch prevents the virtio
core from validating used length.

Signed-off-by: Jason Wang 
---
 drivers/block/virtio_blk.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index c7d05ff24084..36d645ec5002 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -1041,6 +1041,7 @@ static struct virtio_driver virtio_blk = {
.feature_table_size = ARRAY_SIZE(features),
.feature_table_legacy   = features_legacy,
.feature_table_size_legacy  = ARRAY_SIZE(features_legacy),
+   .suppress_used_validation   = true,
.driver.name= KBUILD_MODNAME,
.driver.owner   = THIS_MODULE,
.id_table   = id_table,
-- 
2.25.1

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

[PATCH V5 2/4] virtio-net: don't let virtio core to validate used length

2021-10-26 Thread Jason Wang

For RX virtuqueue, the used length is validated in all the three paths
(big, small and mergeable). For control vq, we never tries to use used
length. So this patch forbids the core to validate the used length.

Signed-off-by: Jason Wang 
---
 drivers/net/virtio_net.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 6d8c8745bf24..7c43bfc1ce44 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -3385,6 +3385,7 @@ static struct virtio_driver virtio_net_driver = {
.feature_table_size = ARRAY_SIZE(features),
.feature_table_legacy = features_legacy,
.feature_table_size_legacy = ARRAY_SIZE(features_legacy),
+   .suppress_used_validation = true,
.driver.name =  KBUILD_MODNAME,
.driver.owner = THIS_MODULE,
.id_table = id_table,
-- 
2.25.1

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

[PATCH V5 1/4] virtio_ring: validate used buffer length

2021-10-26 Thread Jason Wang

This patch validate the used buffer length provided by the device
before trying to use it. This is done by record the in buffer length
in a new field in desc_state structure during virtqueue_add(), then we
can fail the virtqueue_get_buf() when we find the device is trying to
give us a used buffer length which is greater than the in buffer
length.

Since some drivers have already done the validation by themselves,
this patch tries to makes the core validation optional. For the driver
that doesn't want the validation, it can set the
suppress_used_validation to be true (which could be overridden by
force_used_validation module parameter). To be more efficient, a
dedicate array is used for storing the validate used length, this
helps to eliminate the cache stress if validation is done by the
driver.

Signed-off-by: Jason Wang 
---
 drivers/virtio/virtio_ring.c | 60 
 include/linux/virtio.h   |  2 ++
 2 files changed, 62 insertions(+)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 4c0ec82cef56..a6e5a3b94337 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -14,6 +14,9 @@
 #include 
 #include 
 
+static bool force_used_validation = false;
+module_param(force_used_validation, bool, 0444);
+
 #ifdef DEBUG
 /* For development, we want to crash whenever the ring is screwed. */
 #define BAD_RING(_vq, fmt, args...)\
@@ -182,6 +185,9 @@ struct vring_virtqueue {
} packed;
};
 
+   /* Per-descriptor in buffer length */
+   u32 *buflen;
+
/* How to notify other side. FIXME: commonalize hcalls! */
bool (*notify)(struct virtqueue *vq);
 
@@ -490,6 +496,7 @@ static inline int virtqueue_add_split(struct virtqueue *_vq,
unsigned int i, n, avail, descs_used, prev, err_idx;
int head;
bool indirect;
+   u32 buflen = 0;
 
START_USE(vq);
 
@@ -571,6 +578,7 @@ static inline int virtqueue_add_split(struct virtqueue *_vq,
 VRING_DESC_F_NEXT |
 VRING_DESC_F_WRITE,
 indirect);
+   buflen += sg->length;
}
}
/* Last one doesn't continue. */
@@ -610,6 +618,10 @@ static inline int virtqueue_add_split(struct virtqueue 
*_vq,
else
vq->split.desc_state[head].indir_desc = ctx;
 
+   /* Store in buffer length if necessary */
+   if (vq->buflen)
+   vq->buflen[head] = buflen;
+
/* Put entry in available array (but don't update avail->idx until they
 * do sync). */
avail = vq->split.avail_idx_shadow & (vq->split.vring.num - 1);
@@ -784,6 +796,11 @@ static void *virtqueue_get_buf_ctx_split(struct virtqueue 
*_vq,
BAD_RING(vq, "id %u is not a head!\n", i);
return NULL;
}
+   if (vq->buflen && unlikely(*len > vq->buflen[i])) {
+   BAD_RING(vq, "used len %d is larger than in buflen %u\n",
+   *len, vq->buflen[i]);
+   return NULL;
+   }
 
/* detach_buf_split clears data, so grab it now. */
ret = vq->split.desc_state[i].data;
@@ -1062,6 +1079,7 @@ static int virtqueue_add_indirect_packed(struct 
vring_virtqueue *vq,
unsigned int i, n, err_idx;
u16 head, id;
dma_addr_t addr;
+   u32 buflen = 0;
 
head = vq->packed.next_avail_idx;
desc = alloc_indirect_packed(total_sg, gfp);
@@ -1091,6 +1109,8 @@ static int virtqueue_add_indirect_packed(struct 
vring_virtqueue *vq,
desc[i].addr = cpu_to_le64(addr);
desc[i].len = cpu_to_le32(sg->length);
i++;
+   if (n >= out_sgs)
+   buflen += sg->length;
}
}
 
@@ -1144,6 +1164,10 @@ static int virtqueue_add_indirect_packed(struct 
vring_virtqueue *vq,
vq->packed.desc_state[id].indir_desc = desc;
vq->packed.desc_state[id].last = id;
 
+   /* Store in buffer length if necessary */
+   if (vq->buflen)
+   vq->buflen[id] = buflen;
+
vq->num_added += 1;
 
pr_debug("Added buffer head %i to %p\n", head, vq);
@@ -1179,6 +1203,7 @@ static inline int virtqueue_add_packed(struct virtqueue 
*_vq,
__le16 head_flags, flags;
u16 head, id, prev, curr, avail_used_flags;
int err;
+   u32 buflen = 0;
 
START_USE(vq);
 
@@ -1258,6 +1283,8 @@ static inline int virtqueue_add_packed(struct virtqueue 
*_vq,
1 << VRING_PACKED_DESC_F_AVAIL |
1 << VRING_PACKED_DESC_F_USED;
}
+   if (n >= out_sgs)
+   buflen += sg->length;

[PATCH V5 0/4] Validate used buffer length

2021-10-26 Thread Jason Wang

Hi All:

This patch tries to validate the used buffer length in the virtio
core. This help to eliminate the unexpected result caused by buggy or
mailicous devices. For the drivers that can do the validation itself,
they can ask the virtio core to suppress the check.

Changes since V4:

- Fix the out of date description in the commit log

Changes since V3:

- Initialize the buflen to zero when the validation is done by the
  driver.

Jason Wang (4):
  virtio_ring: validate used buffer length
  virtio-net: don't let virtio core to validate used length
  virtio-blk: don't let virtio core to validate used length
  virtio-scsi: don't let virtio core to validate used buffer length

 drivers/block/virtio_blk.c   |  1 +
 drivers/net/virtio_net.c |  1 +
 drivers/scsi/virtio_scsi.c   |  1 +
 drivers/virtio/virtio_ring.c | 60 
 include/linux/virtio.h   |  2 ++
 5 files changed, 65 insertions(+)

-- 
2.25.1

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

[PATCH linux-next v7 8/8] vdpa/mlx5: Forward only packets with allowed MAC address

2021-10-26 Thread Parav Pandit via Virtualization

From: Eli Cohen 

Add rules to forward packets to the net device's TIR only if the
destination MAC is equal to the configured MAC. This is required to
prevent the netdevice from receiving traffic not destined to its
configured MAC.

Signed-off-by: Eli Cohen 
Reviewed-by: Parav Pandit 
---
 drivers/vdpa/mlx5/net/mlx5_vnet.c | 76 +++
 1 file changed, 58 insertions(+), 18 deletions(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index cc027499c4d3..9b7d8c721354 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -158,7 +158,8 @@ struct mlx5_vdpa_net {
struct mutex reslock;
struct mlx5_flow_table *rxft;
struct mlx5_fc *rx_counter;
-   struct mlx5_flow_handle *rx_rule;
+   struct mlx5_flow_handle *rx_rule_ucast;
+   struct mlx5_flow_handle *rx_rule_mcast;
bool setup;
u32 cur_num_vqs;
struct notifier_block nb;
@@ -1383,21 +1384,33 @@ static int add_fwd_to_tir(struct mlx5_vdpa_net *ndev)
struct mlx5_flow_table_attr ft_attr = {};
struct mlx5_flow_act flow_act = {};
struct mlx5_flow_namespace *ns;
+   struct mlx5_flow_spec *spec;
+   void *headers_c;
+   void *headers_v;
+   u8 *dmac_c;
+   u8 *dmac_v;
int err;
 
-   /* for now, one entry, match all, forward to tir */
-   ft_attr.max_fte = 1;
-   ft_attr.autogroup.max_num_groups = 1;
+   spec = kvzalloc(sizeof(*spec), GFP_KERNEL);
+   if (!spec)
+   return -ENOMEM;
+
+   spec->match_criteria_enable = MLX5_MATCH_OUTER_HEADERS;
+   ft_attr.max_fte = 2;
+   ft_attr.autogroup.max_num_groups = 2;
 
ns = mlx5_get_flow_namespace(ndev->mvdev.mdev, 
MLX5_FLOW_NAMESPACE_BYPASS);
if (!ns) {
-   mlx5_vdpa_warn(>mvdev, "get flow namespace\n");
-   return -EOPNOTSUPP;
+   mlx5_vdpa_warn(>mvdev, "failed to get flow namespace\n");
+   err = -EOPNOTSUPP;
+   goto err_ns;
}
 
ndev->rxft = mlx5_create_auto_grouped_flow_table(ns, _attr);
-   if (IS_ERR(ndev->rxft))
-   return PTR_ERR(ndev->rxft);
+   if (IS_ERR(ndev->rxft)) {
+   err = PTR_ERR(ndev->rxft);
+   goto err_ns;
+   }
 
ndev->rx_counter = mlx5_fc_create(ndev->mvdev.mdev, false);
if (IS_ERR(ndev->rx_counter)) {
@@ -1405,37 +1418,64 @@ static int add_fwd_to_tir(struct mlx5_vdpa_net *ndev)
goto err_fc;
}
 
+   headers_c = MLX5_ADDR_OF(fte_match_param, spec->match_criteria, 
outer_headers);
+   dmac_c = MLX5_ADDR_OF(fte_match_param, headers_c, 
outer_headers.dmac_47_16);
+   memset(dmac_c, 0xff, ETH_ALEN);
+   headers_v = MLX5_ADDR_OF(fte_match_param, spec->match_value, 
outer_headers);
+   dmac_v = MLX5_ADDR_OF(fte_match_param, headers_v, 
outer_headers.dmac_47_16);
+   ether_addr_copy(dmac_v, ndev->config.mac);
+
flow_act.action = MLX5_FLOW_CONTEXT_ACTION_FWD_DEST | 
MLX5_FLOW_CONTEXT_ACTION_COUNT;
dest[0].type = MLX5_FLOW_DESTINATION_TYPE_TIR;
dest[0].tir_num = ndev->res.tirn;
dest[1].type = MLX5_FLOW_DESTINATION_TYPE_COUNTER;
dest[1].counter_id = mlx5_fc_id(ndev->rx_counter);
-   ndev->rx_rule = mlx5_add_flow_rules(ndev->rxft, NULL, _act, dest, 
2);
-   if (IS_ERR(ndev->rx_rule)) {
-   err = PTR_ERR(ndev->rx_rule);
-   ndev->rx_rule = NULL;
-   goto err_rule;
+   ndev->rx_rule_ucast = mlx5_add_flow_rules(ndev->rxft, spec, _act, 
dest, 2);
+
+   if (IS_ERR(ndev->rx_rule_ucast)) {
+   err = PTR_ERR(ndev->rx_rule_ucast);
+   ndev->rx_rule_ucast = NULL;
+   goto err_rule_ucast;
+   }
+
+   memset(dmac_c, 0, ETH_ALEN);
+   memset(dmac_v, 0, ETH_ALEN);
+   dmac_c[0] = 1;
+   dmac_v[0] = 1;
+   flow_act.action = MLX5_FLOW_CONTEXT_ACTION_FWD_DEST;
+   ndev->rx_rule_mcast = mlx5_add_flow_rules(ndev->rxft, spec, _act, 
dest, 1);
+   if (IS_ERR(ndev->rx_rule_mcast)) {
+   err = PTR_ERR(ndev->rx_rule_mcast);
+   ndev->rx_rule_mcast = NULL;
+   goto err_rule_mcast;
}
 
+   kvfree(spec);
return 0;
 
-err_rule:
+err_rule_mcast:
+   mlx5_del_flow_rules(ndev->rx_rule_ucast);
+   ndev->rx_rule_ucast = NULL;
+err_rule_ucast:
mlx5_fc_destroy(ndev->mvdev.mdev, ndev->rx_counter);
 err_fc:
mlx5_destroy_flow_table(ndev->rxft);
+err_ns:
+   kvfree(spec);
return err;
 }
 
 static void remove_fwd_to_tir(struct mlx5_vdpa_net *ndev)
 {
-   if (!ndev->rx_rule)
+   if (!ndev->rx_rule_ucast)
return;
 
-   mlx5_del_flow_rules(ndev->rx_rule);
+   mlx5_del_flow_rules(ndev->rx_rule_mcast);
+   ndev->rx_rule_mcast = NULL;
+   mlx5_del_flow_rules(ndev->rx_rule_ucast);
+   ndev->rx_rule_ucast = NULL;

[PATCH linux-next v7 7/8] vdpa/mlx5: Support configuration of MAC

2021-10-26 Thread Parav Pandit via Virtualization

From: Eli Cohen 

Add code to accept MAC configuration through vdpa tool. The MAC is
written into the config struct and later can be retrieved through
get_config().

Examples:
1. Configure MAC while adding a device:
$ vdpa dev add mgmtdev pci/:06:00.2 name vdpa0 mac 00:11:22:33:44:55

2. Show configured params:
$ vdpa dev config show
vdpa0: mac 00:11:22:33:44:55 link down link_announce false max_vq_pairs 8 mtu 
1500

Signed-off-by: Eli Cohen 
Reviewed-by: Parav Pandit 
Acked-by: Jason Wang 
---
 drivers/vdpa/mlx5/net/mlx5_vnet.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 8d1539728a59..cc027499c4d3 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -6,6 +6,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -2524,17 +2525,20 @@ static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev 
*v_mdev, const char *name,
goto err_mtu;
 
ndev->config.mtu = cpu_to_mlx5vdpa16(mvdev, mtu);
-   ndev->config.status |= cpu_to_mlx5vdpa16(mvdev, VIRTIO_NET_S_LINK_UP);
-
-   err = mlx5_query_nic_vport_mac_address(mdev, 0, 0, config->mac);
-   if (err)
-   goto err_mtu;
 
if (get_link_state(mvdev))
ndev->config.status |= cpu_to_mlx5vdpa16(mvdev, 
VIRTIO_NET_S_LINK_UP);
else
ndev->config.status &= cpu_to_mlx5vdpa16(mvdev, 
~VIRTIO_NET_S_LINK_UP);
 
+   if (add_config->mask & (1 << VDPA_ATTR_DEV_NET_CFG_MACADDR)) {
+   memcpy(ndev->config.mac, add_config->net.mac, ETH_ALEN);
+   } else {
+   err = mlx5_query_nic_vport_mac_address(mdev, 0, 0, config->mac);
+   if (err)
+   goto err_mtu;
+   }
+
if (!is_zero_ether_addr(config->mac)) {
pfmdev = pci_get_drvdata(pci_physfn(mdev->pdev));
err = mlx5_mpfs_add_mac(pfmdev, config->mac);
@@ -2632,6 +2636,7 @@ static int mlx5v_probe(struct auxiliary_device *adev,
mgtdev->mgtdev.ops = _ops;
mgtdev->mgtdev.device = mdev->device;
mgtdev->mgtdev.id_table = id_table;
+   mgtdev->mgtdev.config_attr_mask = (1 << VDPA_ATTR_DEV_NET_CFG_MACADDR);
mgtdev->madev = madev;
 
err = vdpa_mgmtdev_register(>mgtdev);
-- 
2.25.4

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

[PATCH linux-next v7 6/8] vdpa/mlx5: Fix clearing of VIRTIO_NET_F_MAC feature bit

2021-10-26 Thread Parav Pandit via Virtualization

Cited patch in the fixes tag clears the features bit during reset.
mlx5 vdpa device feature bits are static decided by device capabilities.
These feature bits (including VIRTIO_NET_F_MAC) are initialized during
device addition time.

Clearing features bit in reset callback cleared the VIRTIO_NET_F_MAC. Due
to this, MAC address provided by the device is not honored.

Fix it by not clearing the static feature bits during reset.

Fixes: 0686082dbf7a ("vdpa: Add reset callback in vdpa_config_ops")
Signed-off-by: Parav Pandit 
Reviewed-by: Eli Cohen 
---
 drivers/vdpa/mlx5/net/mlx5_vnet.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index 6bbdc0ece707..8d1539728a59 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -2194,7 +2194,6 @@ static int mlx5_vdpa_reset(struct vdpa_device *vdev)
clear_vqs_ready(ndev);
mlx5_vdpa_destroy_mr(>mvdev);
ndev->mvdev.status = 0;
-   ndev->mvdev.mlx_features = 0;
memset(ndev->event_cbs, 0, sizeof(ndev->event_cbs));
ndev->mvdev.actual_features = 0;
++mvdev->generation;
-- 
2.25.4

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

[PATCH linux-next v7 5/8] vdpa_sim_net: Enable user to set mac address and mtu

2021-10-26 Thread Parav Pandit via Virtualization

Enable user to set the mac address and mtu so that each vdpa device
can have its own user specified mac address and mtu.

Now that user is enabled to set the mac address, remove the module
parameter for same.

And example of setting mac addr and mtu and view the configuration:
$ vdpa mgmtdev show
vdpasim_net:
  supported_classes net

$ vdpa dev add name bar mgmtdev vdpasim_net mac 00:11:22:33:44:55 mtu 9000

$ vdpa dev config show
bar: mac 00:11:22:33:44:55 link up link_announce false mtu 9000

Signed-off-by: Parav Pandit 
Reviewed-by: Eli Cohen 
Reviewed-by: Stefano Garzarella 
---
changelog:
v4->v5:
 - updated commit log example for add command
---
 drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 35 +++-
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c 
b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
index d681e423e64f..76dd24abc791 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "vdpa_sim.h"
 
@@ -29,12 +30,6 @@
 
 #define VDPASIM_NET_VQ_NUM 2
 
-static char *macaddr;
-module_param(macaddr, charp, 0);
-MODULE_PARM_DESC(macaddr, "Ethernet MAC address");
-
-static u8 macaddr_buf[ETH_ALEN];
-
 static void vdpasim_net_work(struct work_struct *work)
 {
struct vdpasim *vdpasim = container_of(work, struct vdpasim, work);
@@ -112,9 +107,21 @@ static void vdpasim_net_get_config(struct vdpasim 
*vdpasim, void *config)
 {
struct virtio_net_config *net_config = config;
 
-   net_config->mtu = cpu_to_vdpasim16(vdpasim, 1500);
net_config->status = cpu_to_vdpasim16(vdpasim, VIRTIO_NET_S_LINK_UP);
-   memcpy(net_config->mac, macaddr_buf, ETH_ALEN);
+}
+
+static void vdpasim_net_setup_config(struct vdpasim *vdpasim,
+const struct vdpa_dev_set_config *config)
+{
+   struct virtio_net_config *vio_config = vdpasim->config;
+
+   if (config->mask & (1 << VDPA_ATTR_DEV_NET_CFG_MACADDR))
+   memcpy(vio_config->mac, config->net.mac, ETH_ALEN);
+   if (config->mask & (1 << VDPA_ATTR_DEV_NET_CFG_MTU))
+   vio_config->mtu = cpu_to_vdpasim16(vdpasim, config->net.mtu);
+   else
+   /* Setup default MTU to be 1500 */
+   vio_config->mtu = cpu_to_vdpasim16(vdpasim, 1500);
 }
 
 static void vdpasim_net_mgmtdev_release(struct device *dev)
@@ -147,6 +154,8 @@ static int vdpasim_net_dev_add(struct vdpa_mgmt_dev *mdev, 
const char *name,
if (IS_ERR(simdev))
return PTR_ERR(simdev);
 
+   vdpasim_net_setup_config(simdev, config);
+
ret = _vdpa_register_device(>vdpa, VDPASIM_NET_VQ_NUM);
if (ret)
goto reg_err;
@@ -180,20 +189,14 @@ static struct vdpa_mgmt_dev mgmt_dev = {
.device = _net_mgmtdev,
.id_table = id_table,
.ops = _net_mgmtdev_ops,
+   .config_attr_mask = (1 << VDPA_ATTR_DEV_NET_CFG_MACADDR |
+1 << VDPA_ATTR_DEV_NET_CFG_MTU),
 };
 
 static int __init vdpasim_net_init(void)
 {
int ret;
 
-   if (macaddr) {
-   mac_pton(macaddr, macaddr_buf);
-   if (!is_valid_ether_addr(macaddr_buf))
-   return -EADDRNOTAVAIL;
-   } else {
-   eth_random_addr(macaddr_buf);
-   }
-
ret = device_register(_net_mgmtdev);
if (ret)
return ret;
-- 
2.25.4

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

[PATCH linux-next v7 4/8] vdpa: Enable user to set mac and mtu of vdpa device

2021-10-26 Thread Parav Pandit via Virtualization

$ vdpa dev add name bar mgmtdev vdpasim_net mac 00:11:22:33:44:55 mtu 9000

$ vdpa dev config show
bar: mac 00:11:22:33:44:55 link up link_announce false mtu 9000

$ vdpa dev config show -jp
{
"config": {
"bar": {
"mac": "00:11:22:33:44:55",
"link ": "up",
"link_announce ": false,
"mtu": 9000,
}
}
}

Signed-off-by: Parav Pandit 
Reviewed-by: Eli Cohen 
Acked-by: Jason Wang 

---
changelog:
v5->v6:
 - fixed typo of device
v4->v5:
 - added comment for checking device capabilities
v3->v4:
 - provide config attributes during device addition time
---
 drivers/vdpa/ifcvf/ifcvf_main.c  |  3 ++-
 drivers/vdpa/mlx5/net/mlx5_vnet.c|  3 ++-
 drivers/vdpa/vdpa.c  | 38 ++--
 drivers/vdpa/vdpa_sim/vdpa_sim_blk.c |  3 ++-
 drivers/vdpa/vdpa_sim/vdpa_sim_net.c |  3 ++-
 drivers/vdpa/vdpa_user/vduse_dev.c   |  3 ++-
 include/linux/vdpa.h | 17 -
 7 files changed, 62 insertions(+), 8 deletions(-)

diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
index dcd648e1f7e7..6dc75ca70b37 100644
--- a/drivers/vdpa/ifcvf/ifcvf_main.c
+++ b/drivers/vdpa/ifcvf/ifcvf_main.c
@@ -499,7 +499,8 @@ static u32 get_dev_type(struct pci_dev *pdev)
return dev_type;
 }
 
-static int ifcvf_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name)
+static int ifcvf_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
+ const struct vdpa_dev_set_config *config)
 {
struct ifcvf_vdpa_mgmt_dev *ifcvf_mgmt_dev;
struct ifcvf_adapter *adapter;
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index b5bd1a553256..6bbdc0ece707 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -2482,7 +2482,8 @@ static int event_handler(struct notifier_block *nb, 
unsigned long event, void *p
return ret;
 }
 
-static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name)
+static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,
+const struct vdpa_dev_set_config *add_config)
 {
struct mlx5_vdpa_mgmtdev *mgtdev = container_of(v_mdev, struct 
mlx5_vdpa_mgmtdev, mgtdev);
struct virtio_net_config *config;
diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index 973c56fb60b9..a1168a7fa8b8 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -14,7 +14,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 
 static LIST_HEAD(mdev_head);
@@ -480,9 +479,15 @@ vdpa_nl_cmd_mgmtdev_get_dumpit(struct sk_buff *msg, struct 
netlink_callback *cb)
return msg->len;
 }
 
+#define VDPA_DEV_NET_ATTRS_MASK ((1 << VDPA_ATTR_DEV_NET_CFG_MACADDR) | \
+(1 << VDPA_ATTR_DEV_NET_CFG_MTU))
+
 static int vdpa_nl_cmd_dev_add_set_doit(struct sk_buff *skb, struct genl_info 
*info)
 {
+   struct vdpa_dev_set_config config = {};
+   struct nlattr **nl_attrs = info->attrs;
struct vdpa_mgmt_dev *mdev;
+   const u8 *macaddr;
const char *name;
int err = 0;
 
@@ -491,6 +496,26 @@ static int vdpa_nl_cmd_dev_add_set_doit(struct sk_buff 
*skb, struct genl_info *i
 
name = nla_data(info->attrs[VDPA_ATTR_DEV_NAME]);
 
+   if (nl_attrs[VDPA_ATTR_DEV_NET_CFG_MACADDR]) {
+   macaddr = nla_data(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MACADDR]);
+   memcpy(config.net.mac, macaddr, sizeof(config.net.mac));
+   config.mask |= (1 << VDPA_ATTR_DEV_NET_CFG_MACADDR);
+   }
+   if (nl_attrs[VDPA_ATTR_DEV_NET_CFG_MTU]) {
+   config.net.mtu =
+   nla_get_u16(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MTU]);
+   config.mask |= (1 << VDPA_ATTR_DEV_NET_CFG_MTU);
+   }
+
+   /* Skip checking capability if user didn't prefer to configure any
+* device networking attributes. It is likely that user might have used
+* a device specific method to configure such attributes or using device
+* default attributes.
+*/
+   if ((config.mask & VDPA_DEV_NET_ATTRS_MASK) &&
+   !netlink_capable(skb, CAP_NET_ADMIN))
+   return -EPERM;
+
mutex_lock(_dev_mutex);
mdev = vdpa_mgmtdev_get_from_attr(info->attrs);
if (IS_ERR(mdev)) {
@@ -498,8 +523,14 @@ static int vdpa_nl_cmd_dev_add_set_doit(struct sk_buff 
*skb, struct genl_info *i
err = PTR_ERR(mdev);
goto err;
}
+   if ((config.mask & mdev->config_attr_mask) != config.mask) {
+   NL_SET_ERR_MSG_MOD(info->extack,
+  "All provided attributes are not supported");
+   err = -EOPNOTSUPP;
+   goto err;
+   }
 
-   err = mdev->ops->dev_add(mdev, name);
+   err = mdev->ops->dev_add(mdev, name, );
 err:
mutex_unlock(_dev_mutex);

[PATCH linux-next v7 3/8] vdpa: Use kernel coding style for structure comments

2021-10-26 Thread Parav Pandit via Virtualization

As subsequent patch adds new structure field with comment, move the
structure comment to follow kernel coding style.

Signed-off-by: Parav Pandit 
Reviewed-by: Eli Cohen 
Acked-by: Jason Wang 
Reviewed-by: Stefano Garzarella 
---
 include/linux/vdpa.h | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index 5cc5e501397f..fafb7202482c 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -411,10 +411,17 @@ struct vdpa_mgmtdev_ops {
void (*dev_del)(struct vdpa_mgmt_dev *mdev, struct vdpa_device *dev);
 };
 
+/**
+ * struct vdpa_mgmt_dev - vdpa management device
+ * @device: Management parent device
+ * @ops: operations supported by management device
+ * @id_table: Pointer to device id table of supported ids
+ * @list: list entry
+ */
 struct vdpa_mgmt_dev {
struct device *device;
const struct vdpa_mgmtdev_ops *ops;
-   const struct virtio_device_id *id_table; /* supported ids */
+   const struct virtio_device_id *id_table;
struct list_head list;
 };
 
-- 
2.25.4

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

[PATCH linux-next v7 2/8] vdpa: Introduce query of device config layout

2021-10-26 Thread Parav Pandit via Virtualization

Introduce a command to query a device config layout.

An example query of network vdpa device:

$ vdpa dev add name bar mgmtdev vdpasim_net

$ vdpa dev config show
bar: mac 00:35:09:19:48:05 link up link_announce false mtu 1500

$ vdpa dev config show -jp
{
"config": {
"bar": {
"mac": "00:35:09:19:48:05",
"link ": "up",
"link_announce ": false,
"mtu": 1500,
}
}
}

Signed-off-by: Parav Pandit 
Signed-off-by: Eli Cohen 
Acked-by: Jason Wang 
---
changelog:
v3->v4:
 - removed config space fields reporting which are not used by mlx5
   and sim drivers
---
 drivers/vdpa/vdpa.c   | 176 ++
 include/linux/vdpa.h  |   2 +
 include/uapi/linux/vdpa.h |   6 ++
 3 files changed, 184 insertions(+)

diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index d42697699366..973c56fb60b9 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -14,6 +14,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 static LIST_HEAD(mdev_head);
 /* A global mutex that protects vdpa management device and device level 
operations. */
@@ -66,6 +68,7 @@ static void vdpa_release_dev(struct device *d)
ops->free(vdev);
 
ida_simple_remove(_index_ida, vdev->index);
+   mutex_destroy(>cf_mutex);
kfree(vdev);
 }
 
@@ -127,6 +130,7 @@ struct vdpa_device *__vdpa_alloc_device(struct device 
*parent,
if (err)
goto err_name;
 
+   mutex_init(>cf_mutex);
device_initialize(>dev);
 
return vdev;
@@ -309,6 +313,7 @@ void vdpa_get_config(struct vdpa_device *vdev, unsigned int 
offset,
 {
const struct vdpa_config_ops *ops = vdev->config;
 
+   mutex_lock(>cf_mutex);
/*
 * Config accesses aren't supposed to trigger before features are set.
 * If it does happen we assume a legacy guest.
@@ -316,6 +321,7 @@ void vdpa_get_config(struct vdpa_device *vdev, unsigned int 
offset,
if (!vdev->features_valid)
vdpa_set_features(vdev, 0);
ops->get_config(vdev, offset, buf, len);
+   mutex_unlock(>cf_mutex);
 }
 EXPORT_SYMBOL_GPL(vdpa_get_config);
 
@@ -329,7 +335,9 @@ EXPORT_SYMBOL_GPL(vdpa_get_config);
 void vdpa_set_config(struct vdpa_device *vdev, unsigned int offset,
 const void *buf, unsigned int length)
 {
+   mutex_lock(>cf_mutex);
vdev->config->set_config(vdev, offset, buf, length);
+   mutex_unlock(>cf_mutex);
 }
 EXPORT_SYMBOL_GPL(vdpa_set_config);
 
@@ -661,6 +669,168 @@ static int vdpa_nl_cmd_dev_get_dumpit(struct sk_buff 
*msg, struct netlink_callba
return msg->len;
 }
 
+static int vdpa_dev_net_mq_config_fill(struct vdpa_device *vdev,
+  struct sk_buff *msg, u64 features,
+  const struct virtio_net_config *config)
+{
+   u16 val_u16;
+
+   if ((features & (1ULL << VIRTIO_NET_F_MQ)) == 0)
+   return 0;
+
+   val_u16 = le16_to_cpu(config->max_virtqueue_pairs);
+   return nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MAX_VQP, val_u16);
+}
+
+static int vdpa_dev_net_config_fill(struct vdpa_device *vdev, struct sk_buff 
*msg)
+{
+   struct virtio_net_config config = {};
+   u64 features;
+   u16 val_u16;
+
+   vdpa_get_config(vdev, 0, , sizeof(config));
+
+   if (nla_put(msg, VDPA_ATTR_DEV_NET_CFG_MACADDR, sizeof(config.mac),
+   config.mac))
+   return -EMSGSIZE;
+
+   val_u16 = le16_to_cpu(config.status);
+   if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_STATUS, val_u16))
+   return -EMSGSIZE;
+
+   val_u16 = le16_to_cpu(config.mtu);
+   if (nla_put_u16(msg, VDPA_ATTR_DEV_NET_CFG_MTU, val_u16))
+   return -EMSGSIZE;
+
+   features = vdev->config->get_features(vdev);
+
+   return vdpa_dev_net_mq_config_fill(vdev, msg, features, );
+}
+
+static int
+vdpa_dev_config_fill(struct vdpa_device *vdev, struct sk_buff *msg, u32 
portid, u32 seq,
+int flags, struct netlink_ext_ack *extack)
+{
+   u32 device_id;
+   void *hdr;
+   int err;
+
+   hdr = genlmsg_put(msg, portid, seq, _nl_family, flags,
+ VDPA_CMD_DEV_CONFIG_GET);
+   if (!hdr)
+   return -EMSGSIZE;
+
+   if (nla_put_string(msg, VDPA_ATTR_DEV_NAME, dev_name(>dev))) {
+   err = -EMSGSIZE;
+   goto msg_err;
+   }
+
+   device_id = vdev->config->get_device_id(vdev);
+   if (nla_put_u32(msg, VDPA_ATTR_DEV_ID, device_id)) {
+   err = -EMSGSIZE;
+   goto msg_err;
+   }
+
+   switch (device_id) {
+   case VIRTIO_ID_NET:
+   err = vdpa_dev_net_config_fill(vdev, msg);
+   break;
+   default:
+   err = -EOPNOTSUPP;
+   break;
+   }
+   if (err)
+   goto msg_err;
+
+   genlmsg_end(msg, hdr);

[PATCH linux-next v7 1/8] vdpa: Introduce and use vdpa device get, set config helpers

2021-10-26 Thread Parav Pandit via Virtualization

Subsequent patches enable get and set configuration either
via management device or via vdpa device' config ops.

This requires synchronization between multiple callers to get and set
config callbacks. Features setting also influence the layout of the
configuration fields endianness.

To avoid exposing synchronization primitives to callers, introduce
helper for setting the configuration and use it.

Signed-off-by: Parav Pandit 
Reviewed-by: Eli Cohen 
Acked-by: Jason Wang 
Reviewed-by: Stefano Garzarella 
---
changelog:
v4->v5:
 - use vdpa_set_config() API in virtio_vdpa driver
---
 drivers/vdpa/vdpa.c  | 36 
 drivers/vhost/vdpa.c |  3 +--
 drivers/virtio/virtio_vdpa.c |  3 +--
 include/linux/vdpa.h | 19 ---
 4 files changed, 42 insertions(+), 19 deletions(-)

diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index 4aeb1458b924..d42697699366 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -297,6 +297,42 @@ void vdpa_mgmtdev_unregister(struct vdpa_mgmt_dev *mdev)
 }
 EXPORT_SYMBOL_GPL(vdpa_mgmtdev_unregister);
 
+/**
+ * vdpa_get_config - Get one or more device configuration fields.
+ * @vdev: vdpa device to operate on
+ * @offset: starting byte offset of the field
+ * @buf: buffer pointer to read to
+ * @len: length of the configuration fields in bytes
+ */
+void vdpa_get_config(struct vdpa_device *vdev, unsigned int offset,
+void *buf, unsigned int len)
+{
+   const struct vdpa_config_ops *ops = vdev->config;
+
+   /*
+* Config accesses aren't supposed to trigger before features are set.
+* If it does happen we assume a legacy guest.
+*/
+   if (!vdev->features_valid)
+   vdpa_set_features(vdev, 0);
+   ops->get_config(vdev, offset, buf, len);
+}
+EXPORT_SYMBOL_GPL(vdpa_get_config);
+
+/**
+ * vdpa_set_config - Set one or more device configuration fields.
+ * @vdev: vdpa device to operate on
+ * @offset: starting byte offset of the field
+ * @buf: buffer pointer to read from
+ * @length: length of the configuration fields in bytes
+ */
+void vdpa_set_config(struct vdpa_device *vdev, unsigned int offset,
+const void *buf, unsigned int length)
+{
+   vdev->config->set_config(vdev, offset, buf, length);
+}
+EXPORT_SYMBOL_GPL(vdpa_set_config);
+
 static bool mgmtdev_handle_match(const struct vdpa_mgmt_dev *mdev,
 const char *busname, const char *devname)
 {
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 39039e046117..01c59ce7e250 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -237,7 +237,6 @@ static long vhost_vdpa_set_config(struct vhost_vdpa *v,
  struct vhost_vdpa_config __user *c)
 {
struct vdpa_device *vdpa = v->vdpa;
-   const struct vdpa_config_ops *ops = vdpa->config;
struct vhost_vdpa_config config;
unsigned long size = offsetof(struct vhost_vdpa_config, buf);
u8 *buf;
@@ -251,7 +250,7 @@ static long vhost_vdpa_set_config(struct vhost_vdpa *v,
if (IS_ERR(buf))
return PTR_ERR(buf);
 
-   ops->set_config(vdpa, config.off, buf, config.len);
+   vdpa_set_config(vdpa, config.off, buf, config.len);
 
kvfree(buf);
return 0;
diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
index e42ace29daa1..4bb148d4b4e8 100644
--- a/drivers/virtio/virtio_vdpa.c
+++ b/drivers/virtio/virtio_vdpa.c
@@ -65,9 +65,8 @@ static void virtio_vdpa_set(struct virtio_device *vdev, 
unsigned offset,
const void *buf, unsigned len)
 {
struct vdpa_device *vdpa = vd_get_vdpa(vdev);
-   const struct vdpa_config_ops *ops = vdpa->config;
 
-   ops->set_config(vdpa, offset, buf, len);
+   vdpa_set_config(vdpa, offset, buf, len);
 }
 
 static u32 virtio_vdpa_generation(struct virtio_device *vdev)
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h
index 30864848950b..267236aab34c 100644
--- a/include/linux/vdpa.h
+++ b/include/linux/vdpa.h
@@ -386,21 +386,10 @@ static inline int vdpa_set_features(struct vdpa_device 
*vdev, u64 features)
return ops->set_features(vdev, features);
 }
 
-static inline void vdpa_get_config(struct vdpa_device *vdev,
-  unsigned int offset, void *buf,
-  unsigned int len)
-{
-   const struct vdpa_config_ops *ops = vdev->config;
-
-   /*
-* Config accesses aren't supposed to trigger before features are set.
-* If it does happen we assume a legacy guest.
-*/
-   if (!vdev->features_valid)
-   vdpa_set_features(vdev, 0);
-   ops->get_config(vdev, offset, buf, len);
-}
-
+void vdpa_get_config(struct vdpa_device *vdev, unsigned int offset,
+void *buf, unsigned int len);
+void vdpa_set_config(struct vdpa_device *dev, unsigned int offset,
+

[PATCH linux-next v7 0/8] vdpa: enable user to set mac, mtu

2021-10-26 Thread Parav Pandit via Virtualization

Currently user cannot view the vdpa device config space. Also user
cannot set the mac address and mtu of the vdpa device.
This patchset enables users to set the mac address and mtu of the vdpa
device once the device is created.
If a vendor driver supports such configuration user can set it otherwise
user gets unsupported error.

vdpa mac address and mtu are device configuration layout fields.
To keep interface generic enough for multiple types of vdpa devices, mac
address and mtu setting is implemented as configuration layout fields.

An example of query & set of config layout fields for vdpa_sim_net
driver:

Configuration layout fields are set when a vdpa device is created.

$ vdpa mgmtdev show
vdpasim_net:
  supported_classes net
pci/:08:00.2:
  supported_classes net

Add the device with MAC and MTU:
$ vdpa dev add name bar mgmtdev vdpasim_net mac 00:11:22:33:44:66 mtu 1500

In above command only mac address or only mtu can also be set.

View the config after setting:
$ vdpa dev config show
bar: mac 00:11:22:33:44:66 link up link_announce false mtu 1500

Patch summary:
Patch-1 introduced and use helpers for get/set config area
Patch-2 implement query device config layout
Patch-3 use kernel coding style for structure comment
Patch-4 enanble user to set mac and mtu in config space
Patch-5 vdpa_sim_net implements get and set of config layout
Patch-6 mlx vdpa driver fix to avoid clearing VIRTIO_NET_F_MAC during
reset callback
Patch-7 mlx5 vdpa driver supports user provided mac config
Patch-8 mlx5 vdpa driver uses user provided mac during rx flow steering

changelog:
v6->v7:
 - fixed typo in device
v5->v6:
 - fixed mlx5 vdpa device mtu initialization during add
 - corrected example for vdpasim_net in commit log
 - expanded commit log description that fixes mlx5 _MAC feature bit
v4->v5:
 - use vdpa_set_config() API in virtio_vdpa driver
 - make vdpa_set_config() buffer argument const
 - added comment for checking process capability
 - updated commit log example for add command
v3->v4:
 - enable setting mac and mtu of the vdpa device using creation time
 - introduced a patch to fix mlx5 driver to avoid clearing
   VIRTIO_NET_F_MAC
 - introduced a patch to use kernel coding style for structure comment
 - removed config attributes not used by sim and mlx5 net drivers
v2->v3:
 - dropped patches which are merged
 - simplified code to handle non transitional devices
v1->v2:
 - new patches to fix kdoc comment to add new kdoc section
 - new patch to have synchronized access to features and config space
 - read whole net config layout instead of individual fields
 - added error extack for unmanaged vdpa device
 - fixed several endianness issues
 - introduced vdpa device ops for get config which is synchronized
   with other get/set features ops and config ops
 - fixed mtu range checking for max
 - using NLA_POLICY_ETH_ADDR
 - set config moved to device ops instead of mgmtdev ops
 - merged build and set to single routine
 - ensuring that user has NET_ADMIN capability for configuring network
   attributes
 - using updated interface and callbacks for get/set config
 - following new api for config get/set for mgmt tool in mlx5 vdpa
   driver
 - fixes for accessing right SF dma device and bar address
 - fix for mtu calculation
 - fix for bit access in features
 - fix for index restore with suspend/resume operation

Eli Cohen (2):
  vdpa/mlx5: Support configuration of MAC
  vdpa/mlx5: Forward only packets with allowed MAC address

Parav Pandit (6):
  vdpa: Introduce and use vdpa device get, set config helpers
  vdpa: Introduce query of device config layout
  vdpa: Use kernel coding style for structure comments
  vdpa: Enable user to set mac and mtu of vdpa device
  vdpa_sim_net: Enable user to set mac address and mtu
  vdpa/mlx5: Fix clearing of VIRTIO_NET_F_MAC feature bit

 drivers/vdpa/ifcvf/ifcvf_main.c  |   3 +-
 drivers/vdpa/mlx5/net/mlx5_vnet.c|  95 +++---
 drivers/vdpa/vdpa.c  | 248 ++-
 drivers/vdpa/vdpa_sim/vdpa_sim_blk.c |   3 +-
 drivers/vdpa/vdpa_sim/vdpa_sim_net.c |  38 ++--
 drivers/vdpa/vdpa_user/vduse_dev.c   |   3 +-
 drivers/vhost/vdpa.c |   3 +-
 drivers/virtio/virtio_vdpa.c |   3 +-
 include/linux/vdpa.h |  47 +++--
 include/uapi/linux/vdpa.h|   6 +
 10 files changed, 382 insertions(+), 67 deletions(-)

-- 
2.25.4

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

Re: [PATCH V3 11/11] vhost: allow userspace to create workers

2021-10-26 Thread michael . christie

On 10/26/21 12:37 AM, Jason Wang wrote:
> 
> 在 2021/10/22 下午1:19, Mike Christie 写道:
>> This patch allows userspace to create workers and bind them to vqs. You
>> can have N workers per dev and also share N workers with M vqs.
>>
>> Signed-off-by: Mike Christie 
> 
> 
> A question, who is the best one to determine the binding? Is it the VMM (Qemu 
> etc) or the management stack? If the latter, it looks to me it's better to 
> expose this via sysfs?

I thought it would be where you have management app settings, then the
management app talks to the qemu control interface like it does when it
adds new devices on the fly.

A problem with the management app doing it is to handle the RLIMIT_NPROC
review comment, this patchset:

https://lore.kernel.org/all/20211007214448.6282-1-michael.chris...@oracle.com/

basically has the kernel do a clone() from the caller's context. So adding
a worker is like doing the VHOST_SET_OWNER ioctl where it still has to be done
from a process you can inherit values like the mm, cgroups, and now RLIMITs.

>> diff --git a/include/uapi/linux/vhost_types.h 
>> b/include/uapi/linux/vhost_types.h
>> index f7f6a3a28977..af654e3cef0e 100644
>> --- a/include/uapi/linux/vhost_types.h
>> +++ b/include/uapi/linux/vhost_types.h
>> @@ -47,6 +47,18 @@ struct vhost_vring_addr {
>>   __u64 log_guest_addr;
>>   };
>>   +#define VHOST_VRING_NEW_WORKER -1
> 
> 
> Do we need VHOST_VRING_FREE_WORKER? And I wonder if using dedicated ioctls 
> are better:
> 
> VHOST_VRING_NEW/FREE_WORKER
> VHOST_VRING_ATTACH_WORKER

We didn't need a free worker, because the kernel handles it for userspace. I
tried to make it easy for userspace because in some cases it may not be able
to do syscalls like close on the device. For example if qemu crashes or for
vhost-scsi we don't do an explicit close during VM shutdown.

So we start off with the default worker thread that's used by all vqs like we do
today. Userspace can then override it by creating a new worker. That also 
unbinds/
detaches the existing worker and does a put on the workers refcount. We also do 
a
put on the worker when we stop using it during device shutdown/closure/release.
When the worker's refcount goes to zero the kernel deletes it.

I think separating the calls could be helpful though.

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

Re: [PATCH V3 11/11] vhost: allow userspace to create workers

2021-10-26 Thread Stefan Hajnoczi

On Tue, Oct 26, 2021 at 09:09:52AM -0400, Michael S. Tsirkin wrote:
> On Tue, Oct 26, 2021 at 01:37:14PM +0800, Jason Wang wrote:
> > 
> > 在 2021/10/22 下午1:19, Mike Christie 写道:
> > > This patch allows userspace to create workers and bind them to vqs. You
> > > can have N workers per dev and also share N workers with M vqs.
> > > 
> > > Signed-off-by: Mike Christie 
> > 
> > 
> > A question, who is the best one to determine the binding? Is it the VMM
> > (Qemu etc) or the management stack? If the latter, it looks to me it's
> > better to expose this via sysfs?
> 
> I think it's a bit much to expect this from management.

The management stack controls the number of vqs used as well as the vCPU
and IOThread CPU affinity. It seems natural for it to also control the
vhost worker CPU affinity. Where else should that be controlled?

Stefan


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

Re: [PATCH V3 11/11] vhost: allow userspace to create workers

2021-10-26 Thread Stefan Hajnoczi

On Tue, Oct 26, 2021 at 01:37:14PM +0800, Jason Wang wrote:
> 
> 在 2021/10/22 下午1:19, Mike Christie 写道:
> > This patch allows userspace to create workers and bind them to vqs. You
> > can have N workers per dev and also share N workers with M vqs.
> > 
> > Signed-off-by: Mike Christie 
> 
> 
> A question, who is the best one to determine the binding? Is it the VMM
> (Qemu etc) or the management stack? If the latter, it looks to me it's
> better to expose this via sysfs?

A few options that let the management stack control vhost worker CPU
affinity:

1. The management tool opens the vhost device node, calls
   ioctl(VHOST_SET_VRING_WORKER), sets up CPU affinity, and then passes
   the fd to the VMM. In this case the VMM is still able to call the
   ioctl, which may be undesirable from an attack surface perspective.

2. The VMM calls ioctl(VHOST_SET_VRING_WORKER) itself and the management
   tool queries the vq:worker details from the VMM (e.g. a new QEMU QMP
   query-vhost-workers command similar to query-iothreads). The
   management tool can then control CPU affinity on the vhost worker
   threads.

   (This is how CPU affinity works in QEMU and libvirt today.)

3. The sysfs approach you suggested. Does sysfs export vq-0/, vq-1/, etc
   directories with a "worker" attribute? Do we need to define a point
   when the VMM has set up vqs and the management stack is able to query
   them? Vhost devices currently pre-allocate the maximum number of vqs
   and I'm not sure how to determine the number of vqs that will
   actually be used?

   One advantage of this is that access to the vq:worker mapping can be
   limited to the management stack and the VMM cannot access it. But it
   seems a little tricky because the vhost model today doesn't use sysfs
   or define a lifecycle where the management stack can configure
   devices.

Stefan

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

Re: [PATCH V3 11/11] vhost: allow userspace to create workers

2021-10-26 Thread Stefan Hajnoczi

On Fri, Oct 22, 2021 at 12:19:11AM -0500, Mike Christie wrote:
> diff --git a/include/uapi/linux/vhost.h b/include/uapi/linux/vhost.h
> index c998860d7bbc..e5c0669430e5 100644
> --- a/include/uapi/linux/vhost.h
> +++ b/include/uapi/linux/vhost.h
> @@ -70,6 +70,17 @@
>  #define VHOST_VRING_BIG_ENDIAN 1
>  #define VHOST_SET_VRING_ENDIAN _IOW(VHOST_VIRTIO, 0x13, struct 
> vhost_vring_state)
>  #define VHOST_GET_VRING_ENDIAN _IOW(VHOST_VIRTIO, 0x14, struct 
> vhost_vring_state)
> +/* By default, a device gets one vhost_worker created during VHOST_SET_OWNER
> + * that its virtqueues share. This allows userspace to create a vhost_worker
> + * and map a virtqueue to it or map a virtqueue to an existing worker.
> + *
> + * If pid > 0 and it matches an existing vhost_worker thread it will be bound
> + * to the vq. If pid is VHOST_VRING_NEW_WORKER, then a new worker will be
> + * created and bound to the vq.
> + *
> + * This must be called after VHOST_SET_OWNER and before the vq is active.
> + */
> +#define VHOST_SET_VRING_WORKER _IOWR(VHOST_VIRTIO, 0x15, struct 
> vhost_vring_worker)

Please clarify whether or not multiple devices can attach vqs to the same 
worker.

Stefan


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

Re: [PATCH V3 11/11] vhost: allow userspace to create workers

2021-10-26 Thread Stefan Hajnoczi

On Fri, Oct 22, 2021 at 12:19:11AM -0500, Mike Christie wrote:
> +/* Caller must have device mutex */
> +static int vhost_vq_setup_worker(struct vhost_virtqueue *vq,
> +  struct vhost_vring_worker *info)

It's clearer if the function name matches the ioctl name
(VHOST_SET_VRING_WORKER).


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

[PATCH] virtio-ring: fix DMA metadata flags

2021-10-26 Thread Vincent Whitchurch

The flags are currently overwritten, leading to the wrong direction
being passed to the DMA unmap functions.

Fixes: 72b5e8958738aaa4 ("virtio-ring: store DMA metadata in desc_extra for 
split virtqueue")
Signed-off-by: Vincent Whitchurch 
---
 drivers/virtio/virtio_ring.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index dd95dfd85e98..3035bb6f5458 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -576,7 +576,7 @@ static inline int virtqueue_add_split(struct virtqueue *_vq,
/* Last one doesn't continue. */
desc[prev].flags &= cpu_to_virtio16(_vq->vdev, ~VRING_DESC_F_NEXT);
if (!indirect && vq->use_dma_api)
-   vq->split.desc_extra[prev & (vq->split.vring.num - 1)].flags =
+   vq->split.desc_extra[prev & (vq->split.vring.num - 1)].flags &=
~VRING_DESC_F_NEXT;
 
if (indirect) {
-- 
2.28.0

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

Re: [PATCH linux-next v6 4/8] vdpa: Enable user to set mac and mtu of vdpa device

2021-10-26 Thread Stefano Garzarella


On Tue, Oct 26, 2021 at 01:11:51PM +, Parav Pandit wrote:




From: Michael S. Tsirkin 
Sent: Tuesday, October 26, 2021 6:38 PM

On Tue, Oct 26, 2021 at 01:03:41PM +, Parav Pandit wrote:
>
>
> > From: Stefano Garzarella 
> > Sent: Tuesday, October 26, 2021 6:31 PM
> >
> > On Tue, Oct 26, 2021 at 07:02:39AM +0300, Parav Pandit via
> > Virtualization
> > wrote:
> > >$ vdpa dev add name bar mgmtdev vdpasim_net mac 00:11:22:33:44:55
> > >mtu
> > >9000
> > >
> > >$ vdpa dev config show
> > >bar: mac 00:11:22:33:44:55 link up link_announce false mtu 9000
> > >
> > >$ vdpa dev config show -jp
> > >{
> > >"config": {
> > >"bar": {
> > >"mac": "00:11:22:33:44:55",
> > >"link ": "up",
> > >"link_announce ": false,
> > >"mtu": 9000,
> > >}
> > >}
> > >}
> > >
> > >Signed-off-by: Parav Pandit 
> > >Reviewed-by: Eli Cohen 
> > >Acked-by: Jason Wang 
> > >
> > >---
> > >changelog:
> > >v4->v5:
> > > - added comment for checking device capabilities
> > >v3->v4:
> > > - provide config attributes during device addition time
> > >---
> > > drivers/vdpa/ifcvf/ifcvf_main.c  |  3 ++-
> > > drivers/vdpa/mlx5/net/mlx5_vnet.c|  3 ++-
> > > drivers/vdpa/vdpa.c  | 38 ++--
> > > drivers/vdpa/vdpa_sim/vdpa_sim_blk.c |  3 ++-
> > >drivers/vdpa/vdpa_sim/vdpa_sim_net.c |  3 ++-
> > > drivers/vdpa/vdpa_user/vduse_dev.c   |  3 ++-
> > > include/linux/vdpa.h | 17 -
> > > 7 files changed, 62 insertions(+), 8 deletions(-)
> > >
> > >diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c
> > >b/drivers/vdpa/ifcvf/ifcvf_main.c index dcd648e1f7e7..6dc75ca70b37
> > >100644
> > >--- a/drivers/vdpa/ifcvf/ifcvf_main.c
> > >+++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> > >@@ -499,7 +499,8 @@ static u32 get_dev_type(struct pci_dev *pdev)
> > >  return dev_type;
> > > }
> > >
> > >-static int ifcvf_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const
> > >char
> > >*name)
> > >+static int ifcvf_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const
> > >+char
> > *name,
> > >+   const struct vdpa_dev_set_config *config)
> > > {
> > >  struct ifcvf_vdpa_mgmt_dev *ifcvf_mgmt_dev;
> > >  struct ifcvf_adapter *adapter;
> > >diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > >b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > >index b5bd1a553256..6bbdc0ece707 100644
> > >--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > >+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > >@@ -2482,7 +2482,8 @@ static int event_handler(struct
> > >notifier_block *nb,
> > unsigned long event, void *p
> > >  return ret;
> > > }
> > >
> > >-static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const
> > >char
> > >*name)
> > >+static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const
> > >+char
> > *name,
> > >+  const struct vdpa_dev_set_config
> > >*add_config)
> > > {
> > >  struct mlx5_vdpa_mgmtdev *mgtdev = container_of(v_mdev, struct
> > >  mlx5_vdpa_mgmtdev, mgtdev);
> > >  struct virtio_net_config *config; diff --git
> > >a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index
> > >973c56fb60b9..a1168a7fa8b8 100644
> > >--- a/drivers/vdpa/vdpa.c
> > >+++ b/drivers/vdpa/vdpa.c
> > >@@ -14,7 +14,6 @@
> > > #include 
> > > #include 
> > > #include  -#include 
> > >#include 
> > >
> > > static LIST_HEAD(mdev_head);
> > >@@ -480,9 +479,15 @@ vdpa_nl_cmd_mgmtdev_get_dumpit(struct
sk_buff
> > >*msg, struct netlink_callback *cb)
> > >  return msg->len;
> > > }
> > >
> > >+#define VDPA_DEV_NET_ATTRS_MASK ((1 <<
> > VDPA_ATTR_DEV_NET_CFG_MACADDR) | \
> > >+  (1 << VDPA_ATTR_DEV_NET_CFG_MTU))
> > >+
> > > static int vdpa_nl_cmd_dev_add_set_doit(struct sk_buff *skb,
> > > struct genl_info *info) {
> > >+ struct vdpa_dev_set_config config = {};
> > >+ struct nlattr **nl_attrs = info->attrs;
> > >  struct vdpa_mgmt_dev *mdev;
> > >+ const u8 *macaddr;
> > >  const char *name;
> > >  int err = 0;
> > >
> > >@@ -491,6 +496,26 @@ static int vdpa_nl_cmd_dev_add_set_doit(struct
> > >sk_buff *skb, struct genl_info *i
> > >
> > >  name = nla_data(info->attrs[VDPA_ATTR_DEV_NAME]);
> > >
> > >+ if (nl_attrs[VDPA_ATTR_DEV_NET_CFG_MACADDR]) {
> > >+ macaddr =
> > nla_data(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MACADDR]);
> > >+ memcpy(config.net.mac, macaddr, sizeof(config.net.mac));
> > >+ config.mask |= (1 << VDPA_ATTR_DEV_NET_CFG_MACADDR);
> > >+ }
> > >+ if (nl_attrs[VDPA_ATTR_DEV_NET_CFG_MTU]) {
> > >+ config.net.mtu =
> > >+
> >   nla_get_u16(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MTU]);
> > >+ config.mask |= (1 << VDPA_ATTR_DEV_NET_CFG_MTU);
> > >+ }
> > >+
> > >+ /* Skip checking capability if user didn't prefer to configure any
> > >+  * device networking attributes. It is likely that user might have used
> > >+  * a device specific method to configure such attributes or using device
> > >+  * default attributes.
> > >+  */
> > >+ if ((config.mask & VDPA_DEV_NET_ATTRS_MASK) &&
> > >+

Re: [PATCH linux-next v6 5/8] vdpa_sim_net: Enable user to set mac address and mtu

2021-10-26 Thread Stefano Garzarella


On Tue, Oct 26, 2021 at 07:02:40AM +0300, Parav Pandit via Virtualization wrote:

Enable user to set the mac address and mtu so that each vdpa device
can have its own user specified mac address and mtu.

Now that user is enabled to set the mac address, remove the module
parameter for same.

And example of setting mac addr and mtu and view the configuration:
$ vdpa mgmtdev show
vdpasim_net:
 supported_classes net

$ vdpa dev add name bar mgmtdev vdpasim_net mac 00:11:22:33:44:55 mtu 9000

$ vdpa dev config show
bar: mac 00:11:22:33:44:55 link up link_announce false mtu 9000

Signed-off-by: Parav Pandit 
Reviewed-by: Eli Cohen 
---
changelog:
v4->v5:
- updated commit log example for add command
---
drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 35 +++-
1 file changed, 19 insertions(+), 16 deletions(-)


Reviewed-by: Stefano Garzarella 

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

RE: [PATCH linux-next v6 4/8] vdpa: Enable user to set mac and mtu of vdpa device

2021-10-26 Thread Parav Pandit via Virtualization




> From: Michael S. Tsirkin 
> Sent: Tuesday, October 26, 2021 6:38 PM
> 
> On Tue, Oct 26, 2021 at 01:03:41PM +, Parav Pandit wrote:
> >
> >
> > > From: Stefano Garzarella 
> > > Sent: Tuesday, October 26, 2021 6:31 PM
> > >
> > > On Tue, Oct 26, 2021 at 07:02:39AM +0300, Parav Pandit via
> > > Virtualization
> > > wrote:
> > > >$ vdpa dev add name bar mgmtdev vdpasim_net mac 00:11:22:33:44:55
> > > >mtu
> > > >9000
> > > >
> > > >$ vdpa dev config show
> > > >bar: mac 00:11:22:33:44:55 link up link_announce false mtu 9000
> > > >
> > > >$ vdpa dev config show -jp
> > > >{
> > > >"config": {
> > > >"bar": {
> > > >"mac": "00:11:22:33:44:55",
> > > >"link ": "up",
> > > >"link_announce ": false,
> > > >"mtu": 9000,
> > > >}
> > > >}
> > > >}
> > > >
> > > >Signed-off-by: Parav Pandit 
> > > >Reviewed-by: Eli Cohen 
> > > >Acked-by: Jason Wang 
> > > >
> > > >---
> > > >changelog:
> > > >v4->v5:
> > > > - added comment for checking device capabilities
> > > >v3->v4:
> > > > - provide config attributes during device addition time
> > > >---
> > > > drivers/vdpa/ifcvf/ifcvf_main.c  |  3 ++-
> > > > drivers/vdpa/mlx5/net/mlx5_vnet.c|  3 ++-
> > > > drivers/vdpa/vdpa.c  | 38 ++--
> > > > drivers/vdpa/vdpa_sim/vdpa_sim_blk.c |  3 ++-
> > > >drivers/vdpa/vdpa_sim/vdpa_sim_net.c |  3 ++-
> > > > drivers/vdpa/vdpa_user/vduse_dev.c   |  3 ++-
> > > > include/linux/vdpa.h | 17 -
> > > > 7 files changed, 62 insertions(+), 8 deletions(-)
> > > >
> > > >diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c
> > > >b/drivers/vdpa/ifcvf/ifcvf_main.c index dcd648e1f7e7..6dc75ca70b37
> > > >100644
> > > >--- a/drivers/vdpa/ifcvf/ifcvf_main.c
> > > >+++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> > > >@@ -499,7 +499,8 @@ static u32 get_dev_type(struct pci_dev *pdev)
> > > > return dev_type;
> > > > }
> > > >
> > > >-static int ifcvf_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const
> > > >char
> > > >*name)
> > > >+static int ifcvf_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const
> > > >+char
> > > *name,
> > > >+  const struct vdpa_dev_set_config *config)
> > > > {
> > > > struct ifcvf_vdpa_mgmt_dev *ifcvf_mgmt_dev;
> > > > struct ifcvf_adapter *adapter;
> > > >diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > >b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > >index b5bd1a553256..6bbdc0ece707 100644
> > > >--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > >+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > > >@@ -2482,7 +2482,8 @@ static int event_handler(struct
> > > >notifier_block *nb,
> > > unsigned long event, void *p
> > > > return ret;
> > > > }
> > > >
> > > >-static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const
> > > >char
> > > >*name)
> > > >+static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const
> > > >+char
> > > *name,
> > > >+ const struct vdpa_dev_set_config
> > > >*add_config)
> > > > {
> > > > struct mlx5_vdpa_mgmtdev *mgtdev = container_of(v_mdev, struct
> > > > mlx5_vdpa_mgmtdev, mgtdev);
> > > > struct virtio_net_config *config; diff --git
> > > >a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index
> > > >973c56fb60b9..a1168a7fa8b8 100644
> > > >--- a/drivers/vdpa/vdpa.c
> > > >+++ b/drivers/vdpa/vdpa.c
> > > >@@ -14,7 +14,6 @@
> > > > #include 
> > > > #include 
> > > > #include  -#include 
> > > >#include 
> > > >
> > > > static LIST_HEAD(mdev_head);
> > > >@@ -480,9 +479,15 @@ vdpa_nl_cmd_mgmtdev_get_dumpit(struct
> sk_buff
> > > >*msg, struct netlink_callback *cb)
> > > > return msg->len;
> > > > }
> > > >
> > > >+#define VDPA_DEV_NET_ATTRS_MASK ((1 <<
> > > VDPA_ATTR_DEV_NET_CFG_MACADDR) | \
> > > >+ (1 << VDPA_ATTR_DEV_NET_CFG_MTU))
> > > >+
> > > > static int vdpa_nl_cmd_dev_add_set_doit(struct sk_buff *skb,
> > > > struct genl_info *info) {
> > > >+struct vdpa_dev_set_config config = {};
> > > >+struct nlattr **nl_attrs = info->attrs;
> > > > struct vdpa_mgmt_dev *mdev;
> > > >+const u8 *macaddr;
> > > > const char *name;
> > > > int err = 0;
> > > >
> > > >@@ -491,6 +496,26 @@ static int vdpa_nl_cmd_dev_add_set_doit(struct
> > > >sk_buff *skb, struct genl_info *i
> > > >
> > > > name = nla_data(info->attrs[VDPA_ATTR_DEV_NAME]);
> > > >
> > > >+if (nl_attrs[VDPA_ATTR_DEV_NET_CFG_MACADDR]) {
> > > >+macaddr =
> > > nla_data(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MACADDR]);
> > > >+memcpy(config.net.mac, macaddr, sizeof(config.net.mac));
> > > >+config.mask |= (1 << VDPA_ATTR_DEV_NET_CFG_MACADDR);
> > > >+}
> > > >+if (nl_attrs[VDPA_ATTR_DEV_NET_CFG_MTU]) {
> > > >+config.net.mtu =
> > > >+
> > >   nla_get_u16(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MTU]);
> > > >+

Re: [PATCH V3 11/11] vhost: allow userspace to create workers

2021-10-26 Thread Michael S. Tsirkin

On Tue, Oct 26, 2021 at 01:37:14PM +0800, Jason Wang wrote:
> 
> 在 2021/10/22 下午1:19, Mike Christie 写道:
> > This patch allows userspace to create workers and bind them to vqs. You
> > can have N workers per dev and also share N workers with M vqs.
> > 
> > Signed-off-by: Mike Christie 
> 
> 
> A question, who is the best one to determine the binding? Is it the VMM
> (Qemu etc) or the management stack? If the latter, it looks to me it's
> better to expose this via sysfs?

I think it's a bit much to expect this from management.

> 
> > ---
> >   drivers/vhost/vhost.c| 99 
> >   drivers/vhost/vhost.h|  2 +-
> >   include/uapi/linux/vhost.h   | 11 
> >   include/uapi/linux/vhost_types.h | 12 
> >   4 files changed, 112 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > index 04f43a6445e1..c86e88d7f35c 100644
> > --- a/drivers/vhost/vhost.c
> > +++ b/drivers/vhost/vhost.c
> > @@ -493,7 +493,6 @@ void vhost_dev_init(struct vhost_dev *dev,
> > dev->umem = NULL;
> > dev->iotlb = NULL;
> > dev->mm = NULL;
> > -   dev->worker = NULL;
> > dev->iov_limit = iov_limit;
> > dev->weight = weight;
> > dev->byte_weight = byte_weight;
> > @@ -576,20 +575,40 @@ static void vhost_worker_stop(struct vhost_worker 
> > *worker)
> > wait_for_completion(worker->exit_done);
> >   }
> > -static void vhost_worker_free(struct vhost_dev *dev)
> > -{
> > -   struct vhost_worker *worker = dev->worker;
> > +static void vhost_worker_put(struct vhost_worker *worker)
> > +{
> > if (!worker)
> > return;
> > -   dev->worker = NULL;
> > +   if (!refcount_dec_and_test(>refcount))
> > +   return;
> > +
> > WARN_ON(!llist_empty(>work_list));
> > vhost_worker_stop(worker);
> > kfree(worker);
> >   }
> > -static struct vhost_worker *vhost_worker_create(struct vhost_dev *dev)
> > +static void vhost_vq_clear_worker(struct vhost_virtqueue *vq)
> > +{
> > +   if (vq->worker)
> > +   vhost_worker_put(vq->worker);
> > +   vq->worker = NULL;
> > +}
> > +
> > +static void vhost_workers_free(struct vhost_dev *dev)
> > +{
> > +   int i;
> > +
> > +   if (!dev->use_worker)
> > +   return;
> > +
> > +   for (i = 0; i < dev->nvqs; i++)
> > +   vhost_vq_clear_worker(dev->vqs[i]);
> > +}
> > +
> > +static struct vhost_worker *vhost_worker_create(struct vhost_dev *dev,
> > +   int init_vq_map_count)
> >   {
> > struct vhost_worker *worker;
> > struct task_struct *task;
> > @@ -598,9 +617,9 @@ static struct vhost_worker *vhost_worker_create(struct 
> > vhost_dev *dev)
> > if (!worker)
> > return NULL;
> > -   dev->worker = worker;
> > worker->kcov_handle = kcov_common_handle();
> > init_llist_head(>work_list);
> > +   refcount_set(>refcount, init_vq_map_count);
> > /*
> >  * vhost used to use the kthread API which ignores all signals by
> > @@ -617,10 +636,58 @@ static struct vhost_worker 
> > *vhost_worker_create(struct vhost_dev *dev)
> >   free_worker:
> > kfree(worker);
> > -   dev->worker = NULL;
> > return NULL;
> >   }
> > +static struct vhost_worker *vhost_worker_find(struct vhost_dev *dev, pid_t 
> > pid)
> > +{
> > +   struct vhost_worker *worker = NULL;
> > +   int i;
> > +
> > +   for (i = 0; i < dev->nvqs; i++) {
> > +   if (dev->vqs[i]->worker->task->pid != pid)
> > +   continue;
> > +
> > +   worker = dev->vqs[i]->worker;
> > +   break;
> > +   }
> > +
> > +   return worker;
> > +}
> > +
> > +/* Caller must have device mutex */
> > +static int vhost_vq_setup_worker(struct vhost_virtqueue *vq,
> > +struct vhost_vring_worker *info)
> > +{
> > +   struct vhost_dev *dev = vq->dev;
> > +   struct vhost_worker *worker;
> > +
> > +   if (!dev->use_worker)
> > +   return -EINVAL;
> > +
> > +   /* We don't support setting a worker on an active vq */
> > +   if (vq->private_data)
> > +   return -EBUSY;
> 
> 
> Is it valuable to allow the worker switching on active vq?
> 
> 
> > +
> > +   if (info->pid == VHOST_VRING_NEW_WORKER) {
> > +   worker = vhost_worker_create(dev, 1);
> > +   if (!worker)
> > +   return -ENOMEM;
> > +
> > +   info->pid = worker->task->pid;
> > +   } else {
> > +   worker = vhost_worker_find(dev, info->pid);
> > +   if (!worker)
> > +   return -ENODEV;
> > +
> > +   refcount_inc(>refcount);
> > +   }
> > +
> > +   vhost_vq_clear_worker(vq);
> > +   vq->worker = worker;
> > +   return 0;
> > +}
> > +
> >   /* Caller should have device mutex */
> >   long vhost_dev_set_owner(struct vhost_dev *dev)
> >   {
> > @@ -636,7 +703,7 @@ long vhost_dev_set_owner(struct vhost_dev *dev)
> > vhost_attach_mm(dev);
> > if (dev->use_worker) {
> > -   worker = vhost_worker_create(dev);
> >

Re: [PATCH linux-next v6 4/8] vdpa: Enable user to set mac and mtu of vdpa device

2021-10-26 Thread Michael S. Tsirkin

On Tue, Oct 26, 2021 at 01:03:41PM +, Parav Pandit wrote:
> 
> 
> > From: Stefano Garzarella 
> > Sent: Tuesday, October 26, 2021 6:31 PM
> > 
> > On Tue, Oct 26, 2021 at 07:02:39AM +0300, Parav Pandit via Virtualization
> > wrote:
> > >$ vdpa dev add name bar mgmtdev vdpasim_net mac 00:11:22:33:44:55 mtu
> > >9000
> > >
> > >$ vdpa dev config show
> > >bar: mac 00:11:22:33:44:55 link up link_announce false mtu 9000
> > >
> > >$ vdpa dev config show -jp
> > >{
> > >"config": {
> > >"bar": {
> > >"mac": "00:11:22:33:44:55",
> > >"link ": "up",
> > >"link_announce ": false,
> > >"mtu": 9000,
> > >}
> > >}
> > >}
> > >
> > >Signed-off-by: Parav Pandit 
> > >Reviewed-by: Eli Cohen 
> > >Acked-by: Jason Wang 
> > >
> > >---
> > >changelog:
> > >v4->v5:
> > > - added comment for checking device capabilities
> > >v3->v4:
> > > - provide config attributes during device addition time
> > >---
> > > drivers/vdpa/ifcvf/ifcvf_main.c  |  3 ++-
> > > drivers/vdpa/mlx5/net/mlx5_vnet.c|  3 ++-
> > > drivers/vdpa/vdpa.c  | 38 ++--
> > > drivers/vdpa/vdpa_sim/vdpa_sim_blk.c |  3 ++-
> > >drivers/vdpa/vdpa_sim/vdpa_sim_net.c |  3 ++-
> > > drivers/vdpa/vdpa_user/vduse_dev.c   |  3 ++-
> > > include/linux/vdpa.h | 17 -
> > > 7 files changed, 62 insertions(+), 8 deletions(-)
> > >
> > >diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c
> > >b/drivers/vdpa/ifcvf/ifcvf_main.c index dcd648e1f7e7..6dc75ca70b37
> > >100644
> > >--- a/drivers/vdpa/ifcvf/ifcvf_main.c
> > >+++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> > >@@ -499,7 +499,8 @@ static u32 get_dev_type(struct pci_dev *pdev)
> > >   return dev_type;
> > > }
> > >
> > >-static int ifcvf_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char
> > >*name)
> > >+static int ifcvf_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char
> > *name,
> > >+const struct vdpa_dev_set_config *config)
> > > {
> > >   struct ifcvf_vdpa_mgmt_dev *ifcvf_mgmt_dev;
> > >   struct ifcvf_adapter *adapter;
> > >diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > >b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > >index b5bd1a553256..6bbdc0ece707 100644
> > >--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > >+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> > >@@ -2482,7 +2482,8 @@ static int event_handler(struct notifier_block *nb,
> > unsigned long event, void *p
> > >   return ret;
> > > }
> > >
> > >-static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char
> > >*name)
> > >+static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char
> > *name,
> > >+   const struct vdpa_dev_set_config
> > >*add_config)
> > > {
> > >   struct mlx5_vdpa_mgmtdev *mgtdev = container_of(v_mdev, struct
> > >   mlx5_vdpa_mgmtdev, mgtdev);
> > >   struct virtio_net_config *config;
> > >diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index
> > >973c56fb60b9..a1168a7fa8b8 100644
> > >--- a/drivers/vdpa/vdpa.c
> > >+++ b/drivers/vdpa/vdpa.c
> > >@@ -14,7 +14,6 @@
> > > #include 
> > > #include 
> > > #include 
> > >-#include 
> > > #include 
> > >
> > > static LIST_HEAD(mdev_head);
> > >@@ -480,9 +479,15 @@ vdpa_nl_cmd_mgmtdev_get_dumpit(struct sk_buff
> > >*msg, struct netlink_callback *cb)
> > >   return msg->len;
> > > }
> > >
> > >+#define VDPA_DEV_NET_ATTRS_MASK ((1 <<
> > VDPA_ATTR_DEV_NET_CFG_MACADDR) | \
> > >+   (1 << VDPA_ATTR_DEV_NET_CFG_MTU))
> > >+
> > > static int vdpa_nl_cmd_dev_add_set_doit(struct sk_buff *skb, struct
> > > genl_info *info) {
> > >+  struct vdpa_dev_set_config config = {};
> > >+  struct nlattr **nl_attrs = info->attrs;
> > >   struct vdpa_mgmt_dev *mdev;
> > >+  const u8 *macaddr;
> > >   const char *name;
> > >   int err = 0;
> > >
> > >@@ -491,6 +496,26 @@ static int vdpa_nl_cmd_dev_add_set_doit(struct
> > >sk_buff *skb, struct genl_info *i
> > >
> > >   name = nla_data(info->attrs[VDPA_ATTR_DEV_NAME]);
> > >
> > >+  if (nl_attrs[VDPA_ATTR_DEV_NET_CFG_MACADDR]) {
> > >+  macaddr =
> > nla_data(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MACADDR]);
> > >+  memcpy(config.net.mac, macaddr, sizeof(config.net.mac));
> > >+  config.mask |= (1 << VDPA_ATTR_DEV_NET_CFG_MACADDR);
> > >+  }
> > >+  if (nl_attrs[VDPA_ATTR_DEV_NET_CFG_MTU]) {
> > >+  config.net.mtu =
> > >+
> > nla_get_u16(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MTU]);
> > >+  config.mask |= (1 << VDPA_ATTR_DEV_NET_CFG_MTU);
> > >+  }
> > >+
> > >+  /* Skip checking capability if user didn't prefer to configure any
> > >+   * device networking attributes. It is likely that user might have used
> > >+   * a device specific method to configure such attributes or using device
> > >+   * default attributes.
> > >+   */
> > >+  if ((config.mask & VDPA_DEV_NET_ATTRS_MASK) &&
> > >+  !netlink_capable(skb, CAP_NET_ADMIN))
> > >+  return -EPERM;
> > >+
> > >   mutex_lock(_dev_mutex);
> > >   mdev =

RE: [PATCH linux-next v6 4/8] vdpa: Enable user to set mac and mtu of vdpa device

2021-10-26 Thread Parav Pandit via Virtualization




> From: Stefano Garzarella 
> Sent: Tuesday, October 26, 2021 6:31 PM
> 
> On Tue, Oct 26, 2021 at 07:02:39AM +0300, Parav Pandit via Virtualization
> wrote:
> >$ vdpa dev add name bar mgmtdev vdpasim_net mac 00:11:22:33:44:55 mtu
> >9000
> >
> >$ vdpa dev config show
> >bar: mac 00:11:22:33:44:55 link up link_announce false mtu 9000
> >
> >$ vdpa dev config show -jp
> >{
> >"config": {
> >"bar": {
> >"mac": "00:11:22:33:44:55",
> >"link ": "up",
> >"link_announce ": false,
> >"mtu": 9000,
> >}
> >}
> >}
> >
> >Signed-off-by: Parav Pandit 
> >Reviewed-by: Eli Cohen 
> >Acked-by: Jason Wang 
> >
> >---
> >changelog:
> >v4->v5:
> > - added comment for checking device capabilities
> >v3->v4:
> > - provide config attributes during device addition time
> >---
> > drivers/vdpa/ifcvf/ifcvf_main.c  |  3 ++-
> > drivers/vdpa/mlx5/net/mlx5_vnet.c|  3 ++-
> > drivers/vdpa/vdpa.c  | 38 ++--
> > drivers/vdpa/vdpa_sim/vdpa_sim_blk.c |  3 ++-
> >drivers/vdpa/vdpa_sim/vdpa_sim_net.c |  3 ++-
> > drivers/vdpa/vdpa_user/vduse_dev.c   |  3 ++-
> > include/linux/vdpa.h | 17 -
> > 7 files changed, 62 insertions(+), 8 deletions(-)
> >
> >diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c
> >b/drivers/vdpa/ifcvf/ifcvf_main.c index dcd648e1f7e7..6dc75ca70b37
> >100644
> >--- a/drivers/vdpa/ifcvf/ifcvf_main.c
> >+++ b/drivers/vdpa/ifcvf/ifcvf_main.c
> >@@ -499,7 +499,8 @@ static u32 get_dev_type(struct pci_dev *pdev)
> > return dev_type;
> > }
> >
> >-static int ifcvf_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char
> >*name)
> >+static int ifcvf_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char
> *name,
> >+  const struct vdpa_dev_set_config *config)
> > {
> > struct ifcvf_vdpa_mgmt_dev *ifcvf_mgmt_dev;
> > struct ifcvf_adapter *adapter;
> >diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> >b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> >index b5bd1a553256..6bbdc0ece707 100644
> >--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
> >+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
> >@@ -2482,7 +2482,8 @@ static int event_handler(struct notifier_block *nb,
> unsigned long event, void *p
> > return ret;
> > }
> >
> >-static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char
> >*name)
> >+static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char
> *name,
> >+ const struct vdpa_dev_set_config
> >*add_config)
> > {
> > struct mlx5_vdpa_mgmtdev *mgtdev = container_of(v_mdev, struct
> > mlx5_vdpa_mgmtdev, mgtdev);
> > struct virtio_net_config *config;
> >diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c index
> >973c56fb60b9..a1168a7fa8b8 100644
> >--- a/drivers/vdpa/vdpa.c
> >+++ b/drivers/vdpa/vdpa.c
> >@@ -14,7 +14,6 @@
> > #include 
> > #include 
> > #include 
> >-#include 
> > #include 
> >
> > static LIST_HEAD(mdev_head);
> >@@ -480,9 +479,15 @@ vdpa_nl_cmd_mgmtdev_get_dumpit(struct sk_buff
> >*msg, struct netlink_callback *cb)
> > return msg->len;
> > }
> >
> >+#define VDPA_DEV_NET_ATTRS_MASK ((1 <<
> VDPA_ATTR_DEV_NET_CFG_MACADDR) | \
> >+ (1 << VDPA_ATTR_DEV_NET_CFG_MTU))
> >+
> > static int vdpa_nl_cmd_dev_add_set_doit(struct sk_buff *skb, struct
> > genl_info *info) {
> >+struct vdpa_dev_set_config config = {};
> >+struct nlattr **nl_attrs = info->attrs;
> > struct vdpa_mgmt_dev *mdev;
> >+const u8 *macaddr;
> > const char *name;
> > int err = 0;
> >
> >@@ -491,6 +496,26 @@ static int vdpa_nl_cmd_dev_add_set_doit(struct
> >sk_buff *skb, struct genl_info *i
> >
> > name = nla_data(info->attrs[VDPA_ATTR_DEV_NAME]);
> >
> >+if (nl_attrs[VDPA_ATTR_DEV_NET_CFG_MACADDR]) {
> >+macaddr =
> nla_data(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MACADDR]);
> >+memcpy(config.net.mac, macaddr, sizeof(config.net.mac));
> >+config.mask |= (1 << VDPA_ATTR_DEV_NET_CFG_MACADDR);
> >+}
> >+if (nl_attrs[VDPA_ATTR_DEV_NET_CFG_MTU]) {
> >+config.net.mtu =
> >+
>   nla_get_u16(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MTU]);
> >+config.mask |= (1 << VDPA_ATTR_DEV_NET_CFG_MTU);
> >+}
> >+
> >+/* Skip checking capability if user didn't prefer to configure any
> >+ * device networking attributes. It is likely that user might have used
> >+ * a device specific method to configure such attributes or using device
> >+ * default attributes.
> >+ */
> >+if ((config.mask & VDPA_DEV_NET_ATTRS_MASK) &&
> >+!netlink_capable(skb, CAP_NET_ADMIN))
> >+return -EPERM;
> >+
> > mutex_lock(_dev_mutex);
> > mdev = vdpa_mgmtdev_get_from_attr(info->attrs);
> > if (IS_ERR(mdev)) {
> >@@ -498,8 +523,14 @@ static int vdpa_nl_cmd_dev_add_set_doit(struct
> sk_buff *skb, struct genl_info *i
> > err = PTR_ERR(mdev);
> > goto err;
> > }
> >+if

Re: [PATCH linux-next v6 4/8] vdpa: Enable user to set mac and mtu of vdpa device

2021-10-26 Thread Stefano Garzarella


On Tue, Oct 26, 2021 at 07:02:39AM +0300, Parav Pandit via Virtualization wrote:

$ vdpa dev add name bar mgmtdev vdpasim_net mac 00:11:22:33:44:55 mtu 9000

$ vdpa dev config show
bar: mac 00:11:22:33:44:55 link up link_announce false mtu 9000

$ vdpa dev config show -jp
{
   "config": {
   "bar": {
   "mac": "00:11:22:33:44:55",
   "link ": "up",
   "link_announce ": false,
   "mtu": 9000,
   }
   }
}

Signed-off-by: Parav Pandit 
Reviewed-by: Eli Cohen 
Acked-by: Jason Wang 

---
changelog:
v4->v5:
- added comment for checking device capabilities
v3->v4:
- provide config attributes during device addition time
---
drivers/vdpa/ifcvf/ifcvf_main.c  |  3 ++-
drivers/vdpa/mlx5/net/mlx5_vnet.c|  3 ++-
drivers/vdpa/vdpa.c  | 38 ++--
drivers/vdpa/vdpa_sim/vdpa_sim_blk.c |  3 ++-
drivers/vdpa/vdpa_sim/vdpa_sim_net.c |  3 ++-
drivers/vdpa/vdpa_user/vduse_dev.c   |  3 ++-
include/linux/vdpa.h | 17 -
7 files changed, 62 insertions(+), 8 deletions(-)

diff --git a/drivers/vdpa/ifcvf/ifcvf_main.c b/drivers/vdpa/ifcvf/ifcvf_main.c
index dcd648e1f7e7..6dc75ca70b37 100644
--- a/drivers/vdpa/ifcvf/ifcvf_main.c
+++ b/drivers/vdpa/ifcvf/ifcvf_main.c
@@ -499,7 +499,8 @@ static u32 get_dev_type(struct pci_dev *pdev)
return dev_type;
}

-static int ifcvf_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name)
+static int ifcvf_vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
+ const struct vdpa_dev_set_config *config)
{
struct ifcvf_vdpa_mgmt_dev *ifcvf_mgmt_dev;
struct ifcvf_adapter *adapter;
diff --git a/drivers/vdpa/mlx5/net/mlx5_vnet.c 
b/drivers/vdpa/mlx5/net/mlx5_vnet.c
index b5bd1a553256..6bbdc0ece707 100644
--- a/drivers/vdpa/mlx5/net/mlx5_vnet.c
+++ b/drivers/vdpa/mlx5/net/mlx5_vnet.c
@@ -2482,7 +2482,8 @@ static int event_handler(struct notifier_block *nb, 
unsigned long event, void *p
return ret;
}

-static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name)
+static int mlx5_vdpa_dev_add(struct vdpa_mgmt_dev *v_mdev, const char *name,
+			 const struct vdpa_dev_set_config 
*add_config)

{
	struct mlx5_vdpa_mgmtdev *mgtdev = container_of(v_mdev, struct 
	mlx5_vdpa_mgmtdev, mgtdev);

struct virtio_net_config *config;
diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
index 973c56fb60b9..a1168a7fa8b8 100644
--- a/drivers/vdpa/vdpa.c
+++ b/drivers/vdpa/vdpa.c
@@ -14,7 +14,6 @@
#include 
#include 
#include 
-#include 
#include 

static LIST_HEAD(mdev_head);
@@ -480,9 +479,15 @@ vdpa_nl_cmd_mgmtdev_get_dumpit(struct sk_buff 
*msg, struct netlink_callback *cb)

return msg->len;
}

+#define VDPA_DEV_NET_ATTRS_MASK ((1 << VDPA_ATTR_DEV_NET_CFG_MACADDR) | \
+(1 << VDPA_ATTR_DEV_NET_CFG_MTU))
+
static int vdpa_nl_cmd_dev_add_set_doit(struct sk_buff *skb, struct genl_info 
*info)
{
+   struct vdpa_dev_set_config config = {};
+   struct nlattr **nl_attrs = info->attrs;
struct vdpa_mgmt_dev *mdev;
+   const u8 *macaddr;
const char *name;
int err = 0;

@@ -491,6 +496,26 @@ static int vdpa_nl_cmd_dev_add_set_doit(struct sk_buff 
*skb, struct genl_info *i

name = nla_data(info->attrs[VDPA_ATTR_DEV_NAME]);

+   if (nl_attrs[VDPA_ATTR_DEV_NET_CFG_MACADDR]) {
+   macaddr = nla_data(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MACADDR]);
+   memcpy(config.net.mac, macaddr, sizeof(config.net.mac));
+   config.mask |= (1 << VDPA_ATTR_DEV_NET_CFG_MACADDR);
+   }
+   if (nl_attrs[VDPA_ATTR_DEV_NET_CFG_MTU]) {
+   config.net.mtu =
+   nla_get_u16(nl_attrs[VDPA_ATTR_DEV_NET_CFG_MTU]);
+   config.mask |= (1 << VDPA_ATTR_DEV_NET_CFG_MTU);
+   }
+
+   /* Skip checking capability if user didn't prefer to configure any
+* device networking attributes. It is likely that user might have used
+* a device specific method to configure such attributes or using device
+* default attributes.
+*/
+   if ((config.mask & VDPA_DEV_NET_ATTRS_MASK) &&
+   !netlink_capable(skb, CAP_NET_ADMIN))
+   return -EPERM;
+
mutex_lock(_dev_mutex);
mdev = vdpa_mgmtdev_get_from_attr(info->attrs);
if (IS_ERR(mdev)) {
@@ -498,8 +523,14 @@ static int vdpa_nl_cmd_dev_add_set_doit(struct sk_buff 
*skb, struct genl_info *i
err = PTR_ERR(mdev);
goto err;
}
+   if ((config.mask & mdev->config_attr_mask) != config.mask) {
+   NL_SET_ERR_MSG_MOD(info->extack,
+  "All provided attributes are not supported");
+   err = -EOPNOTSUPP;
+   goto err;
+   }

-   err = mdev->ops->dev_add(mdev, name);
+   err = mdev->ops->dev_add(mdev, name, );
err:
mutex_unlock(_dev_mutex);
return err;
@@

Re: [PATCH linux-next v6 3/8] vdpa: Use kernel coding style for structure comments

2021-10-26 Thread Stefano Garzarella


On Tue, Oct 26, 2021 at 07:02:38AM +0300, Parav Pandit via Virtualization wrote:

As subsequent patch adds new structure field with comment, move the
structure comment to follow kernel coding style.

Signed-off-by: Parav Pandit 
Reviewed-by: Eli Cohen 
Acked-by: Jason Wang 
---
include/linux/vdpa.h | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)


Reviewed-by: Stefano Garzarella 

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

Re: [PATCH linux-next v6 1/8] vdpa: Introduce and use vdpa device get, set config helpers

2021-10-26 Thread Stefano Garzarella


On Tue, Oct 26, 2021 at 07:02:36AM +0300, Parav Pandit via Virtualization wrote:

Subsequent patches enable get and set configuration either
via management device or via vdpa device' config ops.

This requires synchronization between multiple callers to get and set
config callbacks. Features setting also influence the layout of the
configuration fields endianness.

To avoid exposing synchronization primitives to callers, introduce
helper for setting the configuration and use it.

Signed-off-by: Parav Pandit 
Reviewed-by: Eli Cohen 
Acked-by: Jason Wang 
---
changelog:
v4->v5:
- use vdpa_set_config() API in virtio_vdpa driver
---
drivers/vdpa/vdpa.c  | 36 
drivers/vhost/vdpa.c |  3 +--
drivers/virtio/virtio_vdpa.c |  3 +--
include/linux/vdpa.h | 19 ---
4 files changed, 42 insertions(+), 19 deletions(-)


Reviewed-by: Stefano Garzarella 

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

Re: [PATCH] i2c: virtio: Add support for zero-length requests

2021-10-26 Thread Viresh Kumar

On 21-10-21, 15:17, Viresh Kumar wrote:
> The virtio specification received a new mandatory feature
> (VIRTIO_I2C_F_ZERO_LENGTH_REQUEST) for zero length requests. Fail if the
> feature isn't offered by the device.
> 
> For each read-request, set the VIRTIO_I2C_FLAGS_M_RD flag, as required
> by the VIRTIO_I2C_F_ZERO_LENGTH_REQUEST feature.
> 
> This allows us to support zero length requests, like SMBUS Quick, where
> the buffer need not be sent anymore.
> 
> Signed-off-by: Viresh Kumar 
> ---
> Hi Wolfram,
> 
> Please do not apply this until the spec changes [1] are merged, sending it 
> early
> to get review done. I will ping you later once the spec is merged.
> 
> [1] https://lists.oasis-open.org/archives/virtio-dev/202110/msg00109.html

Michael,

Can this be merged as well based on the current voting at the ballot ?

https://www.oasis-open.org/committees/ballot.php?id=3659

Wolfram,

I am asking as this patch should be considered as a fix, which needs to be
applied to the 5.15 kernel itself if possible (now or via stable), as we are
implementing a new mandatory feature, which will make the currently merged
version of the driver unusable going forward (since this won't be backwards
compatible).

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

Re: [PATCH V4 1/4] virtio_ring: validate used buffer length

2021-10-26 Thread Michael S. Tsirkin

On Tue, Oct 26, 2021 at 06:21:46PM +0800, Jason Wang wrote:
> On Tue, Oct 26, 2021 at 5:44 PM Michael S. Tsirkin  wrote:
> >
> > On Tue, Oct 26, 2021 at 03:19:57PM +0800, Jason Wang wrote:
> > > This patch validate the used buffer length provided by the device
> > > before trying to use it. This is done by record the in buffer length
> > > in a new field in desc_state structure during virtqueue_add(), then we
> > > can fail the virtqueue_get_buf() when we find the device is trying to
> > > give us a used buffer length which is greater than the in buffer
> > > length.
> > >
> > > Since some drivers have already done the validation by themselves,
> > > this patch tries to makes the core validation optional. For the driver
> > > that doesn't want the validation, it can set the validate_used to be
> > > true (which could be overridden by force_used_validation). To be more
> >
> > This description is now out of date. it's suppress_used_validation.
> 
> Yes, do you want me to post a new version or do you want to fix it for me?
> 
> Thanks

repost pls

> >
> > > efficient, a dedicate array is used for storing the validate used
> > > length, this helps to eliminate the cache stress if validation is done
> > > by the driver.
> > >
> > > Signed-off-by: Jason Wang 
> > > ---
> > >  drivers/virtio/virtio_ring.c | 60 
> > >  include/linux/virtio.h   |  2 ++
> > >  2 files changed, 62 insertions(+)
> > >
> > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > index 4c0ec82cef56..a6e5a3b94337 100644
> > > --- a/drivers/virtio/virtio_ring.c
> > > +++ b/drivers/virtio/virtio_ring.c
> > > @@ -14,6 +14,9 @@
> > >  #include 
> > >  #include 
> > >
> > > +static bool force_used_validation = false;
> > > +module_param(force_used_validation, bool, 0444);
> > > +
> > >  #ifdef DEBUG
> > >  /* For development, we want to crash whenever the ring is screwed. */
> > >  #define BAD_RING(_vq, fmt, args...)  \
> > > @@ -182,6 +185,9 @@ struct vring_virtqueue {
> > >   } packed;
> > >   };
> > >
> > > + /* Per-descriptor in buffer length */
> > > + u32 *buflen;
> > > +
> > >   /* How to notify other side. FIXME: commonalize hcalls! */
> > >   bool (*notify)(struct virtqueue *vq);
> > >
> > > @@ -490,6 +496,7 @@ static inline int virtqueue_add_split(struct 
> > > virtqueue *_vq,
> > >   unsigned int i, n, avail, descs_used, prev, err_idx;
> > >   int head;
> > >   bool indirect;
> > > + u32 buflen = 0;
> > >
> > >   START_USE(vq);
> > >
> > > @@ -571,6 +578,7 @@ static inline int virtqueue_add_split(struct 
> > > virtqueue *_vq,
> > >VRING_DESC_F_NEXT |
> > >VRING_DESC_F_WRITE,
> > >indirect);
> > > + buflen += sg->length;
> > >   }
> > >   }
> > >   /* Last one doesn't continue. */
> > > @@ -610,6 +618,10 @@ static inline int virtqueue_add_split(struct 
> > > virtqueue *_vq,
> > >   else
> > >   vq->split.desc_state[head].indir_desc = ctx;
> > >
> > > + /* Store in buffer length if necessary */
> > > + if (vq->buflen)
> > > + vq->buflen[head] = buflen;
> > > +
> > >   /* Put entry in available array (but don't update avail->idx until 
> > > they
> > >* do sync). */
> > >   avail = vq->split.avail_idx_shadow & (vq->split.vring.num - 1);
> > > @@ -784,6 +796,11 @@ static void *virtqueue_get_buf_ctx_split(struct 
> > > virtqueue *_vq,
> > >   BAD_RING(vq, "id %u is not a head!\n", i);
> > >   return NULL;
> > >   }
> > > + if (vq->buflen && unlikely(*len > vq->buflen[i])) {
> > > + BAD_RING(vq, "used len %d is larger than in buflen %u\n",
> > > + *len, vq->buflen[i]);
> > > + return NULL;
> > > + }
> > >
> > >   /* detach_buf_split clears data, so grab it now. */
> > >   ret = vq->split.desc_state[i].data;
> > > @@ -1062,6 +1079,7 @@ static int virtqueue_add_indirect_packed(struct 
> > > vring_virtqueue *vq,
> > >   unsigned int i, n, err_idx;
> > >   u16 head, id;
> > >   dma_addr_t addr;
> > > + u32 buflen = 0;
> > >
> > >   head = vq->packed.next_avail_idx;
> > >   desc = alloc_indirect_packed(total_sg, gfp);
> > > @@ -1091,6 +1109,8 @@ static int virtqueue_add_indirect_packed(struct 
> > > vring_virtqueue *vq,
> > >   desc[i].addr = cpu_to_le64(addr);
> > >   desc[i].len = cpu_to_le32(sg->length);
> > >   i++;
> > > + if (n >= out_sgs)
> > > + buflen += sg->length;
> > >   }
> > >   }
> > >
> > > @@ -1144,6 +1164,10 @@ static int virtqueue_add_indirect_packed(struct 
> > > vring_virtqueue *vq,
> > >

Re: [PATCH V4 1/4] virtio_ring: validate used buffer length

2021-10-26 Thread Jason Wang

On Tue, Oct 26, 2021 at 5:44 PM Michael S. Tsirkin  wrote:
>
> On Tue, Oct 26, 2021 at 03:19:57PM +0800, Jason Wang wrote:
> > This patch validate the used buffer length provided by the device
> > before trying to use it. This is done by record the in buffer length
> > in a new field in desc_state structure during virtqueue_add(), then we
> > can fail the virtqueue_get_buf() when we find the device is trying to
> > give us a used buffer length which is greater than the in buffer
> > length.
> >
> > Since some drivers have already done the validation by themselves,
> > this patch tries to makes the core validation optional. For the driver
> > that doesn't want the validation, it can set the validate_used to be
> > true (which could be overridden by force_used_validation). To be more
>
> This description is now out of date. it's suppress_used_validation.

Yes, do you want me to post a new version or do you want to fix it for me?

Thanks

>
> > efficient, a dedicate array is used for storing the validate used
> > length, this helps to eliminate the cache stress if validation is done
> > by the driver.
> >
> > Signed-off-by: Jason Wang 
> > ---
> >  drivers/virtio/virtio_ring.c | 60 
> >  include/linux/virtio.h   |  2 ++
> >  2 files changed, 62 insertions(+)
> >
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index 4c0ec82cef56..a6e5a3b94337 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -14,6 +14,9 @@
> >  #include 
> >  #include 
> >
> > +static bool force_used_validation = false;
> > +module_param(force_used_validation, bool, 0444);
> > +
> >  #ifdef DEBUG
> >  /* For development, we want to crash whenever the ring is screwed. */
> >  #define BAD_RING(_vq, fmt, args...)  \
> > @@ -182,6 +185,9 @@ struct vring_virtqueue {
> >   } packed;
> >   };
> >
> > + /* Per-descriptor in buffer length */
> > + u32 *buflen;
> > +
> >   /* How to notify other side. FIXME: commonalize hcalls! */
> >   bool (*notify)(struct virtqueue *vq);
> >
> > @@ -490,6 +496,7 @@ static inline int virtqueue_add_split(struct virtqueue 
> > *_vq,
> >   unsigned int i, n, avail, descs_used, prev, err_idx;
> >   int head;
> >   bool indirect;
> > + u32 buflen = 0;
> >
> >   START_USE(vq);
> >
> > @@ -571,6 +578,7 @@ static inline int virtqueue_add_split(struct virtqueue 
> > *_vq,
> >VRING_DESC_F_NEXT |
> >VRING_DESC_F_WRITE,
> >indirect);
> > + buflen += sg->length;
> >   }
> >   }
> >   /* Last one doesn't continue. */
> > @@ -610,6 +618,10 @@ static inline int virtqueue_add_split(struct virtqueue 
> > *_vq,
> >   else
> >   vq->split.desc_state[head].indir_desc = ctx;
> >
> > + /* Store in buffer length if necessary */
> > + if (vq->buflen)
> > + vq->buflen[head] = buflen;
> > +
> >   /* Put entry in available array (but don't update avail->idx until 
> > they
> >* do sync). */
> >   avail = vq->split.avail_idx_shadow & (vq->split.vring.num - 1);
> > @@ -784,6 +796,11 @@ static void *virtqueue_get_buf_ctx_split(struct 
> > virtqueue *_vq,
> >   BAD_RING(vq, "id %u is not a head!\n", i);
> >   return NULL;
> >   }
> > + if (vq->buflen && unlikely(*len > vq->buflen[i])) {
> > + BAD_RING(vq, "used len %d is larger than in buflen %u\n",
> > + *len, vq->buflen[i]);
> > + return NULL;
> > + }
> >
> >   /* detach_buf_split clears data, so grab it now. */
> >   ret = vq->split.desc_state[i].data;
> > @@ -1062,6 +1079,7 @@ static int virtqueue_add_indirect_packed(struct 
> > vring_virtqueue *vq,
> >   unsigned int i, n, err_idx;
> >   u16 head, id;
> >   dma_addr_t addr;
> > + u32 buflen = 0;
> >
> >   head = vq->packed.next_avail_idx;
> >   desc = alloc_indirect_packed(total_sg, gfp);
> > @@ -1091,6 +1109,8 @@ static int virtqueue_add_indirect_packed(struct 
> > vring_virtqueue *vq,
> >   desc[i].addr = cpu_to_le64(addr);
> >   desc[i].len = cpu_to_le32(sg->length);
> >   i++;
> > + if (n >= out_sgs)
> > + buflen += sg->length;
> >   }
> >   }
> >
> > @@ -1144,6 +1164,10 @@ static int virtqueue_add_indirect_packed(struct 
> > vring_virtqueue *vq,
> >   vq->packed.desc_state[id].indir_desc = desc;
> >   vq->packed.desc_state[id].last = id;
> >
> > + /* Store in buffer length if necessary */
> > + if (vq->buflen)
> > + vq->buflen[id] = buflen;
> > +
> >   vq->num_added += 1;
> >
> >   pr_debug("Added buffer head %i to %p\n", head, vq);
> > @@

Re: [PATCH] eni_vdpa: alibaba: fix Kconfig typo

2021-10-26 Thread Stefano Garzarella


On Tue, Oct 26, 2021 at 10:31:59AM +0200, Arnd Bergmann wrote:

From: Arnd Bergmann 

The Kconfig symbol was misspelled, which leads to randconfig link
failures:

ld.lld: error: undefined symbol: vp_legacy_probe

referenced by eni_vdpa.c
  vdpa/alibaba/eni_vdpa.o:(eni_vdpa_probe) in archive 
drivers/built-in.a


Fixes: 52e437b2b222 ("eni_vdpa: add vDPA driver for Alibaba ENI")
Signed-off-by: Arnd Bergmann 
---
drivers/vdpa/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Stefano Garzarella 

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

Re: [PATCH V4 1/4] virtio_ring: validate used buffer length

2021-10-26 Thread Michael S. Tsirkin

On Tue, Oct 26, 2021 at 03:19:57PM +0800, Jason Wang wrote:
> This patch validate the used buffer length provided by the device
> before trying to use it. This is done by record the in buffer length
> in a new field in desc_state structure during virtqueue_add(), then we
> can fail the virtqueue_get_buf() when we find the device is trying to
> give us a used buffer length which is greater than the in buffer
> length.
> 
> Since some drivers have already done the validation by themselves,
> this patch tries to makes the core validation optional. For the driver
> that doesn't want the validation, it can set the validate_used to be
> true (which could be overridden by force_used_validation). To be more

This description is now out of date. it's suppress_used_validation.

> efficient, a dedicate array is used for storing the validate used
> length, this helps to eliminate the cache stress if validation is done
> by the driver.
> 
> Signed-off-by: Jason Wang 
> ---
>  drivers/virtio/virtio_ring.c | 60 
>  include/linux/virtio.h   |  2 ++
>  2 files changed, 62 insertions(+)
> 
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index 4c0ec82cef56..a6e5a3b94337 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -14,6 +14,9 @@
>  #include 
>  #include 
>  
> +static bool force_used_validation = false;
> +module_param(force_used_validation, bool, 0444);
> +
>  #ifdef DEBUG
>  /* For development, we want to crash whenever the ring is screwed. */
>  #define BAD_RING(_vq, fmt, args...)  \
> @@ -182,6 +185,9 @@ struct vring_virtqueue {
>   } packed;
>   };
>  
> + /* Per-descriptor in buffer length */
> + u32 *buflen;
> +
>   /* How to notify other side. FIXME: commonalize hcalls! */
>   bool (*notify)(struct virtqueue *vq);
>  
> @@ -490,6 +496,7 @@ static inline int virtqueue_add_split(struct virtqueue 
> *_vq,
>   unsigned int i, n, avail, descs_used, prev, err_idx;
>   int head;
>   bool indirect;
> + u32 buflen = 0;
>  
>   START_USE(vq);
>  
> @@ -571,6 +578,7 @@ static inline int virtqueue_add_split(struct virtqueue 
> *_vq,
>VRING_DESC_F_NEXT |
>VRING_DESC_F_WRITE,
>indirect);
> + buflen += sg->length;
>   }
>   }
>   /* Last one doesn't continue. */
> @@ -610,6 +618,10 @@ static inline int virtqueue_add_split(struct virtqueue 
> *_vq,
>   else
>   vq->split.desc_state[head].indir_desc = ctx;
>  
> + /* Store in buffer length if necessary */
> + if (vq->buflen)
> + vq->buflen[head] = buflen;
> +
>   /* Put entry in available array (but don't update avail->idx until they
>* do sync). */
>   avail = vq->split.avail_idx_shadow & (vq->split.vring.num - 1);
> @@ -784,6 +796,11 @@ static void *virtqueue_get_buf_ctx_split(struct 
> virtqueue *_vq,
>   BAD_RING(vq, "id %u is not a head!\n", i);
>   return NULL;
>   }
> + if (vq->buflen && unlikely(*len > vq->buflen[i])) {
> + BAD_RING(vq, "used len %d is larger than in buflen %u\n",
> + *len, vq->buflen[i]);
> + return NULL;
> + }
>  
>   /* detach_buf_split clears data, so grab it now. */
>   ret = vq->split.desc_state[i].data;
> @@ -1062,6 +1079,7 @@ static int virtqueue_add_indirect_packed(struct 
> vring_virtqueue *vq,
>   unsigned int i, n, err_idx;
>   u16 head, id;
>   dma_addr_t addr;
> + u32 buflen = 0;
>  
>   head = vq->packed.next_avail_idx;
>   desc = alloc_indirect_packed(total_sg, gfp);
> @@ -1091,6 +1109,8 @@ static int virtqueue_add_indirect_packed(struct 
> vring_virtqueue *vq,
>   desc[i].addr = cpu_to_le64(addr);
>   desc[i].len = cpu_to_le32(sg->length);
>   i++;
> + if (n >= out_sgs)
> + buflen += sg->length;
>   }
>   }
>  
> @@ -1144,6 +1164,10 @@ static int virtqueue_add_indirect_packed(struct 
> vring_virtqueue *vq,
>   vq->packed.desc_state[id].indir_desc = desc;
>   vq->packed.desc_state[id].last = id;
>  
> + /* Store in buffer length if necessary */
> + if (vq->buflen)
> + vq->buflen[id] = buflen;
> +
>   vq->num_added += 1;
>  
>   pr_debug("Added buffer head %i to %p\n", head, vq);
> @@ -1179,6 +1203,7 @@ static inline int virtqueue_add_packed(struct virtqueue 
> *_vq,
>   __le16 head_flags, flags;
>   u16 head, id, prev, curr, avail_used_flags;
>   int err;
> + u32 buflen = 0;
>  
>   START_USE(vq);
>  
> @@ -1258,6 +1283,8 @@ static inline int virtqueue_add_packed(struct virtqueue 
> *_vq,
>   1

[PATCH V4 3/4] virtio-blk: don't let virtio core to validate used length

2021-10-26 Thread Jason Wang

We never tries to use used length, so the patch prevents the virtio
core from validating used length.

Signed-off-by: Jason Wang 
---
 drivers/block/virtio_blk.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
index c7d05ff24084..36d645ec5002 100644
--- a/drivers/block/virtio_blk.c
+++ b/drivers/block/virtio_blk.c
@@ -1041,6 +1041,7 @@ static struct virtio_driver virtio_blk = {
.feature_table_size = ARRAY_SIZE(features),
.feature_table_legacy   = features_legacy,
.feature_table_size_legacy  = ARRAY_SIZE(features_legacy),
+   .suppress_used_validation   = true,
.driver.name= KBUILD_MODNAME,
.driver.owner   = THIS_MODULE,
.id_table   = id_table,
-- 
2.25.1

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

[PATCH V4 4/4] virtio-scsi: don't let virtio core to validate used buffer length

2021-10-26 Thread Jason Wang

We never tries to use used length, so the patch prevents the virtio
core from validating used length.

Signed-off-by: Jason Wang 
---
 drivers/scsi/virtio_scsi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/virtio_scsi.c b/drivers/scsi/virtio_scsi.c
index 07d0250f17c3..03b09ecea42d 100644
--- a/drivers/scsi/virtio_scsi.c
+++ b/drivers/scsi/virtio_scsi.c
@@ -977,6 +977,7 @@ static unsigned int features[] = {
 static struct virtio_driver virtio_scsi_driver = {
.feature_table = features,
.feature_table_size = ARRAY_SIZE(features),
+   .suppress_used_validation = true,
.driver.name = KBUILD_MODNAME,
.driver.owner = THIS_MODULE,
.id_table = id_table,
-- 
2.25.1

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

[PATCH V4 2/4] virtio-net: don't let virtio core to validate used length

2021-10-26 Thread Jason Wang

For RX virtuqueue, the used length is validated in all the three paths
(big, small and mergeable). For control vq, we never tries to use used
length. So this patch forbids the core to validate the used length.

Signed-off-by: Jason Wang 
---
 drivers/net/virtio_net.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 6d8c8745bf24..7c43bfc1ce44 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -3385,6 +3385,7 @@ static struct virtio_driver virtio_net_driver = {
.feature_table_size = ARRAY_SIZE(features),
.feature_table_legacy = features_legacy,
.feature_table_size_legacy = ARRAY_SIZE(features_legacy),
+   .suppress_used_validation = true,
.driver.name =  KBUILD_MODNAME,
.driver.owner = THIS_MODULE,
.id_table = id_table,
-- 
2.25.1

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

[PATCH V4 1/4] virtio_ring: validate used buffer length

2021-10-26 Thread Jason Wang

This patch validate the used buffer length provided by the device
before trying to use it. This is done by record the in buffer length
in a new field in desc_state structure during virtqueue_add(), then we
can fail the virtqueue_get_buf() when we find the device is trying to
give us a used buffer length which is greater than the in buffer
length.

Since some drivers have already done the validation by themselves,
this patch tries to makes the core validation optional. For the driver
that doesn't want the validation, it can set the validate_used to be
true (which could be overridden by force_used_validation). To be more
efficient, a dedicate array is used for storing the validate used
length, this helps to eliminate the cache stress if validation is done
by the driver.

Signed-off-by: Jason Wang 
---
 drivers/virtio/virtio_ring.c | 60 
 include/linux/virtio.h   |  2 ++
 2 files changed, 62 insertions(+)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 4c0ec82cef56..a6e5a3b94337 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -14,6 +14,9 @@
 #include 
 #include 
 
+static bool force_used_validation = false;
+module_param(force_used_validation, bool, 0444);
+
 #ifdef DEBUG
 /* For development, we want to crash whenever the ring is screwed. */
 #define BAD_RING(_vq, fmt, args...)\
@@ -182,6 +185,9 @@ struct vring_virtqueue {
} packed;
};
 
+   /* Per-descriptor in buffer length */
+   u32 *buflen;
+
/* How to notify other side. FIXME: commonalize hcalls! */
bool (*notify)(struct virtqueue *vq);
 
@@ -490,6 +496,7 @@ static inline int virtqueue_add_split(struct virtqueue *_vq,
unsigned int i, n, avail, descs_used, prev, err_idx;
int head;
bool indirect;
+   u32 buflen = 0;
 
START_USE(vq);
 
@@ -571,6 +578,7 @@ static inline int virtqueue_add_split(struct virtqueue *_vq,
 VRING_DESC_F_NEXT |
 VRING_DESC_F_WRITE,
 indirect);
+   buflen += sg->length;
}
}
/* Last one doesn't continue. */
@@ -610,6 +618,10 @@ static inline int virtqueue_add_split(struct virtqueue 
*_vq,
else
vq->split.desc_state[head].indir_desc = ctx;
 
+   /* Store in buffer length if necessary */
+   if (vq->buflen)
+   vq->buflen[head] = buflen;
+
/* Put entry in available array (but don't update avail->idx until they
 * do sync). */
avail = vq->split.avail_idx_shadow & (vq->split.vring.num - 1);
@@ -784,6 +796,11 @@ static void *virtqueue_get_buf_ctx_split(struct virtqueue 
*_vq,
BAD_RING(vq, "id %u is not a head!\n", i);
return NULL;
}
+   if (vq->buflen && unlikely(*len > vq->buflen[i])) {
+   BAD_RING(vq, "used len %d is larger than in buflen %u\n",
+   *len, vq->buflen[i]);
+   return NULL;
+   }
 
/* detach_buf_split clears data, so grab it now. */
ret = vq->split.desc_state[i].data;
@@ -1062,6 +1079,7 @@ static int virtqueue_add_indirect_packed(struct 
vring_virtqueue *vq,
unsigned int i, n, err_idx;
u16 head, id;
dma_addr_t addr;
+   u32 buflen = 0;
 
head = vq->packed.next_avail_idx;
desc = alloc_indirect_packed(total_sg, gfp);
@@ -1091,6 +1109,8 @@ static int virtqueue_add_indirect_packed(struct 
vring_virtqueue *vq,
desc[i].addr = cpu_to_le64(addr);
desc[i].len = cpu_to_le32(sg->length);
i++;
+   if (n >= out_sgs)
+   buflen += sg->length;
}
}
 
@@ -1144,6 +1164,10 @@ static int virtqueue_add_indirect_packed(struct 
vring_virtqueue *vq,
vq->packed.desc_state[id].indir_desc = desc;
vq->packed.desc_state[id].last = id;
 
+   /* Store in buffer length if necessary */
+   if (vq->buflen)
+   vq->buflen[id] = buflen;
+
vq->num_added += 1;
 
pr_debug("Added buffer head %i to %p\n", head, vq);
@@ -1179,6 +1203,7 @@ static inline int virtqueue_add_packed(struct virtqueue 
*_vq,
__le16 head_flags, flags;
u16 head, id, prev, curr, avail_used_flags;
int err;
+   u32 buflen = 0;
 
START_USE(vq);
 
@@ -1258,6 +1283,8 @@ static inline int virtqueue_add_packed(struct virtqueue 
*_vq,
1 << VRING_PACKED_DESC_F_AVAIL |
1 << VRING_PACKED_DESC_F_USED;
}
+   if (n >= out_sgs)
+   buflen += sg->length;
}
}
 
@@ -1277,6

[PATCH V4 0/4] Validate used buffer length

2021-10-26 Thread Jason Wang

Hi All:

This patch tries to validate the used buffer length in the virtio
core. This help to eliminate the unexpected result caused by buggy or
mailicous devices. For the drivers that can do the validation itself,
they can ask the virtio core to suppress the check.

Changes since V3:

- Initialize the buflen to zero when the validation is done by the driver.

Jason Wang (4):
  virtio_ring: validate used buffer length
  virtio-net: don't let virtio core to validate used length
  virtio-blk: don't let virtio core to validate used length
  virtio-scsi: don't let virtio core to validate used buffer length

 drivers/block/virtio_blk.c   |  1 +
 drivers/net/virtio_net.c |  1 +
 drivers/scsi/virtio_scsi.c   |  1 +
 drivers/virtio/virtio_ring.c | 60 
 include/linux/virtio.h   |  2 ++
 5 files changed, 65 insertions(+)

-- 
2.25.1

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

Re: [PATCH v11 2/3] tty: hvc: pass DMA capable memory to put_chars()

2021-10-26 Thread Greg KH

On Tue, Oct 26, 2021 at 02:11:51PM +0800, Xianting Tian wrote:
> 
> 在 2021/10/26 下午2:10, Greg KH 写道:
> > On Tue, Oct 26, 2021 at 02:02:21PM +0800, Xianting Tian wrote:
> > > 在 2021/10/26 下午1:10, Jiri Slaby 写道:
> > > > On 15. 10. 21, 4:46, Xianting Tian wrote:
> > > > > @@ -151,9 +142,11 @@ static uint32_t vtermnos[MAX_NR_HVC_CONSOLES] =
> > > > >    static void hvc_console_print(struct console *co, const char *b,
> > > > >      unsigned count)
> > > > >    {
> > > > > -    char c[N_OUTBUF] __ALIGNED__;
> > > > > +    char *c;
> > > > >    unsigned i = 0, n = 0;
> > > > >    int r, donecr = 0, index = co->index;
> > > > > +    unsigned long flags;
> > > > > +    struct hvc_struct *hp;
> > > > >      /* Console access attempt outside of acceptable console
> > > > > range. */
> > > > >    if (index >= MAX_NR_HVC_CONSOLES)
> > > > > @@ -163,6 +156,13 @@ static void hvc_console_print(struct console
> > > > > *co, const char *b,
> > > > >    if (vtermnos[index] == -1)
> > > > >    return;
> > > > >    +    hp = cons_hvcs[index];
> > > > > +    if (!hp)
> > > > > +    return;
> > > > You effectively make the console unusable until someone calls
> > > > hvc_alloc() for this device, correct? This doesn't look right. Neither
> > > > you describe this change of behaviour in the commit log.
> > > I mentioned such info in the commit log:
> > > 'Introduce another array(cons_hvcs[]) for hvc pointers next to the
> > > cons_ops[] and vtermnos[] arrays. With the array, we can easily find
> > > hvc's cons_outbuf and its lock.'
> > > 
> > > After you pointed it out, I just found what you said make sense, I 
> > > checked the code hvc_console_print() can support print before hvc_alloc() 
> > > is called when someone use hvc_instantiate() for an early console 
> > > discovery method.
> > > I send a patch to fix the issue?  or these serial pathches reverted 
> > > fisrtly then I resend new version patches? thanks
> > Let me revert these now and you can send an updated version.
> OK, thanks.

I have now reverted patches 2/3 and 3/3 in this series from my tree.
The first patch was just fine.

thanks,

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

Re: [PATCH v11 2/3] tty: hvc: pass DMA capable memory to put_chars()

2021-10-26 Thread Greg KH

On Tue, Oct 26, 2021 at 02:02:21PM +0800, Xianting Tian wrote:
> 在 2021/10/26 下午1:10, Jiri Slaby 写道:
> > On 15. 10. 21, 4:46, Xianting Tian wrote:
> > > @@ -151,9 +142,11 @@ static uint32_t vtermnos[MAX_NR_HVC_CONSOLES] =
> > >   static void hvc_console_print(struct console *co, const char *b,
> > >     unsigned count)
> > >   {
> > > -    char c[N_OUTBUF] __ALIGNED__;
> > > +    char *c;
> > >   unsigned i = 0, n = 0;
> > >   int r, donecr = 0, index = co->index;
> > > +    unsigned long flags;
> > > +    struct hvc_struct *hp;
> > >     /* Console access attempt outside of acceptable console
> > > range. */
> > >   if (index >= MAX_NR_HVC_CONSOLES)
> > > @@ -163,6 +156,13 @@ static void hvc_console_print(struct console
> > > *co, const char *b,
> > >   if (vtermnos[index] == -1)
> > >   return;
> > >   +    hp = cons_hvcs[index];
> > > +    if (!hp)
> > > +    return;
> > 
> > You effectively make the console unusable until someone calls
> > hvc_alloc() for this device, correct? This doesn't look right. Neither
> > you describe this change of behaviour in the commit log.
> 
> I mentioned such info in the commit log:
> 'Introduce another array(cons_hvcs[]) for hvc pointers next to the
> cons_ops[] and vtermnos[] arrays. With the array, we can easily find
> hvc's cons_outbuf and its lock.'
> 
> After you pointed it out, I just found what you said make sense, I checked 
> the code hvc_console_print() can support print before hvc_alloc() is called 
> when someone use hvc_instantiate() for an early console discovery method.
> I send a patch to fix the issue?  or these serial pathches reverted fisrtly 
> then I resend new version patches? thanks

Let me revert these now and you can send an updated version.

thanks,

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

46 matches

Mail list logo