linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH for 5.10 v2 0/5] vdpa_sim: fix param validation in vdpasim_get_config()
@ 2021-02-16 14:24 Stefano Garzarella
  2021-02-16 14:24 ` [PATCH for 5.10 v2 1/5] vdpa_sim: remove hard-coded virtq count Stefano Garzarella
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: Stefano Garzarella @ 2021-02-16 14:24 UTC (permalink / raw)
  To: Greg KH
  Cc: Michael S. Tsirkin, stable, Jason Wang, virtualization, linux-kernel

v1: https://lore.kernel.org/stable/20210211162519.215418-1-sgarzare@redhat.com/

v2:
- backport the upstream patch and related patches needed

Commit 65b709586e22 ("vdpa_sim: add get_config callback in
vdpasim_dev_attr") unintentionally solved an issue in vdpasim_get_config()
upstream while refactoring vdpa_sim.c to support multiple devices.

Before that patch, if 'offset + len' was equal to
sizeof(struct virtio_net_config), the entire buffer wasn't filled,
returning incorrect values to the caller.

Since 'vdpasim->config' type is 'struct virtio_net_config', we can
safely copy its content under this condition.

The minimum set of patches to backport the patch that fixes the issue, is the
following:

   423248d60d2b vdpa_sim: remove hard-coded virtq count
   6c6e28fe4579 vdpa_sim: add struct vdpasim_dev_attr for device attributes
   cf1a3b35382c vdpa_sim: store parsed MAC address in a buffer
   f37cbbc65178 vdpa_sim: make 'config' generic and usable for any device type
   65b709586e22 vdpa_sim: add get_config callback in vdpasim_dev_attr

The patches apply fairly cleanly. There are a few contextual differences
due to the lack of the other patches:

   $ git backport-diff -u master -r linux-5.10.y..HEAD
   Key:
   [----] : patches are identical
   [####] : number of functional differences between upstream/downstream patch
   [down] : patch is downstream-only
   The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

   001/5:[----] [--] 'vdpa_sim: remove hard-coded virtq count'
   002/5:[----] [-C] 'vdpa_sim: add struct vdpasim_dev_attr for device attributes'
   003/5:[----] [--] 'vdpa_sim: store parsed MAC address in a buffer'
   004/5:[----] [-C] 'vdpa_sim: make 'config' generic and usable for any device type'
   005/5:[----] [-C] 'vdpa_sim: add get_config callback in vdpasim_dev_attr'

Thanks,
Stefano

Max Gurtovoy (1):
  vdpa_sim: remove hard-coded virtq count

Stefano Garzarella (4):
  vdpa_sim: add struct vdpasim_dev_attr for device attributes
  vdpa_sim: store parsed MAC address in a buffer
  vdpa_sim: make 'config' generic and usable for any device type
  vdpa_sim: add get_config callback in vdpasim_dev_attr

 drivers/vdpa/vdpa_sim/vdpa_sim.c | 83 +++++++++++++++++++++++---------
 1 file changed, 60 insertions(+), 23 deletions(-)

-- 
2.29.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH for 5.10 v2 1/5] vdpa_sim: remove hard-coded virtq count
  2021-02-16 14:24 [PATCH for 5.10 v2 0/5] vdpa_sim: fix param validation in vdpasim_get_config() Stefano Garzarella
@ 2021-02-16 14:24 ` Stefano Garzarella
  2021-02-16 14:24 ` [PATCH for 5.10 v2 2/5] vdpa_sim: add struct vdpasim_dev_attr for device attributes Stefano Garzarella
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Stefano Garzarella @ 2021-02-16 14:24 UTC (permalink / raw)
  To: Greg KH
  Cc: Michael S. Tsirkin, stable, Jason Wang, virtualization, linux-kernel

From: Max Gurtovoy <mgurtovoy@nvidia.com>

commit 423248d60d2b655321fc49eca1545f95a1bc9d6c upstream.

Add a new attribute that will define the number of virt queues to be
created for the vdpasim device.

Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
[sgarzare: replace kmalloc_array() with kcalloc()]
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Link: https://lore.kernel.org/r/20201215144256.155342-4-sgarzare@redhat.com
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Cc: <stable@vger.kernel.org> # 5.10.x
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 18 +++++++++++++-----
 1 file changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 6a90fdb9cbfc..ee8f24a4643b 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -70,7 +70,7 @@ static u64 vdpasim_features = (1ULL << VIRTIO_F_ANY_LAYOUT) |
 /* State of each vdpasim device */
 struct vdpasim {
 	struct vdpa_device vdpa;
-	struct vdpasim_virtqueue vqs[VDPASIM_VQ_NUM];
+	struct vdpasim_virtqueue *vqs;
 	struct work_struct work;
 	/* spinlock to synchronize virtqueue state */
 	spinlock_t lock;
@@ -80,6 +80,7 @@ struct vdpasim {
 	u32 status;
 	u32 generation;
 	u64 features;
+	int nvqs;
 	/* spinlock to synchronize iommu table */
 	spinlock_t iommu_lock;
 };
@@ -144,7 +145,7 @@ static void vdpasim_reset(struct vdpasim *vdpasim)
 {
 	int i;
 
-	for (i = 0; i < VDPASIM_VQ_NUM; i++)
+	for (i = 0; i < vdpasim->nvqs; i++)
 		vdpasim_vq_reset(&vdpasim->vqs[i]);
 
 	spin_lock(&vdpasim->iommu_lock);
@@ -350,7 +351,7 @@ static struct vdpasim *vdpasim_create(void)
 	const struct vdpa_config_ops *ops;
 	struct vdpasim *vdpasim;
 	struct device *dev;
-	int ret = -ENOMEM;
+	int i, ret = -ENOMEM;
 
 	if (batch_mapping)
 		ops = &vdpasim_net_batch_config_ops;
@@ -361,6 +362,7 @@ static struct vdpasim *vdpasim_create(void)
 	if (!vdpasim)
 		goto err_alloc;
 
+	vdpasim->nvqs = VDPASIM_VQ_NUM;
 	INIT_WORK(&vdpasim->work, vdpasim_work);
 	spin_lock_init(&vdpasim->lock);
 	spin_lock_init(&vdpasim->iommu_lock);
@@ -371,6 +373,11 @@ static struct vdpasim *vdpasim_create(void)
 		goto err_iommu;
 	set_dma_ops(dev, &vdpasim_dma_ops);
 
+	vdpasim->vqs = kcalloc(vdpasim->nvqs, sizeof(struct vdpasim_virtqueue),
+			       GFP_KERNEL);
+	if (!vdpasim->vqs)
+		goto err_iommu;
+
 	vdpasim->iommu = vhost_iotlb_alloc(2048, 0);
 	if (!vdpasim->iommu)
 		goto err_iommu;
@@ -389,8 +396,8 @@ static struct vdpasim *vdpasim_create(void)
 		eth_random_addr(vdpasim->config.mac);
 	}
 
-	vringh_set_iotlb(&vdpasim->vqs[0].vring, vdpasim->iommu);
-	vringh_set_iotlb(&vdpasim->vqs[1].vring, vdpasim->iommu);
+	for (i = 0; i < vdpasim->nvqs; i++)
+		vringh_set_iotlb(&vdpasim->vqs[i].vring, vdpasim->iommu);
 
 	vdpasim->vdpa.dma_dev = dev;
 	ret = vdpa_register_device(&vdpasim->vdpa);
@@ -659,6 +666,7 @@ static void vdpasim_free(struct vdpa_device *vdpa)
 	kfree(vdpasim->buffer);
 	if (vdpasim->iommu)
 		vhost_iotlb_free(vdpasim->iommu);
+	kfree(vdpasim->vqs);
 }
 
 static const struct vdpa_config_ops vdpasim_net_config_ops = {
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH for 5.10 v2 2/5] vdpa_sim: add struct vdpasim_dev_attr for device attributes
  2021-02-16 14:24 [PATCH for 5.10 v2 0/5] vdpa_sim: fix param validation in vdpasim_get_config() Stefano Garzarella
  2021-02-16 14:24 ` [PATCH for 5.10 v2 1/5] vdpa_sim: remove hard-coded virtq count Stefano Garzarella
@ 2021-02-16 14:24 ` Stefano Garzarella
  2021-02-16 14:24 ` [PATCH for 5.10 v2 3/5] vdpa_sim: store parsed MAC address in a buffer Stefano Garzarella
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Stefano Garzarella @ 2021-02-16 14:24 UTC (permalink / raw)
  To: Greg KH
  Cc: Michael S. Tsirkin, stable, Jason Wang, virtualization, linux-kernel

commit 6c6e28fe45794054410ad8cd2770af69fbe0338d upstream.

vdpasim_dev_attr will contain device specific attributes. We starting
moving the number of virtqueues (i.e. nvqs) to vdpasim_dev_attr.

vdpasim_create() creates a new vDPA simulator following the device
attributes defined in the vdpasim_dev_attr parameter.

Co-developed-by: Max Gurtovoy <mgurtovoy@nvidia.com>
Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Link: https://lore.kernel.org/r/20201215144256.155342-7-sgarzare@redhat.com
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Cc: <stable@vger.kernel.org> # 5.10.x
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index ee8f24a4643b..6a5603f9d24c 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -67,11 +67,16 @@ static u64 vdpasim_features = (1ULL << VIRTIO_F_ANY_LAYOUT) |
 			      (1ULL << VIRTIO_F_ACCESS_PLATFORM) |
 			      (1ULL << VIRTIO_NET_F_MAC);
 
+struct vdpasim_dev_attr {
+	int nvqs;
+};
+
 /* State of each vdpasim device */
 struct vdpasim {
 	struct vdpa_device vdpa;
 	struct vdpasim_virtqueue *vqs;
 	struct work_struct work;
+	struct vdpasim_dev_attr dev_attr;
 	/* spinlock to synchronize virtqueue state */
 	spinlock_t lock;
 	struct virtio_net_config config;
@@ -80,7 +85,6 @@ struct vdpasim {
 	u32 status;
 	u32 generation;
 	u64 features;
-	int nvqs;
 	/* spinlock to synchronize iommu table */
 	spinlock_t iommu_lock;
 };
@@ -145,7 +149,7 @@ static void vdpasim_reset(struct vdpasim *vdpasim)
 {
 	int i;
 
-	for (i = 0; i < vdpasim->nvqs; i++)
+	for (i = 0; i < vdpasim->dev_attr.nvqs; i++)
 		vdpasim_vq_reset(&vdpasim->vqs[i]);
 
 	spin_lock(&vdpasim->iommu_lock);
@@ -346,7 +350,7 @@ static const struct dma_map_ops vdpasim_dma_ops = {
 static const struct vdpa_config_ops vdpasim_net_config_ops;
 static const struct vdpa_config_ops vdpasim_net_batch_config_ops;
 
-static struct vdpasim *vdpasim_create(void)
+static struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
 {
 	const struct vdpa_config_ops *ops;
 	struct vdpasim *vdpasim;
@@ -358,11 +362,12 @@ static struct vdpasim *vdpasim_create(void)
 	else
 		ops = &vdpasim_net_config_ops;
 
-	vdpasim = vdpa_alloc_device(struct vdpasim, vdpa, NULL, ops, VDPASIM_VQ_NUM);
+	vdpasim = vdpa_alloc_device(struct vdpasim, vdpa, NULL, ops,
+				    dev_attr->nvqs);
 	if (!vdpasim)
 		goto err_alloc;
 
-	vdpasim->nvqs = VDPASIM_VQ_NUM;
+	vdpasim->dev_attr = *dev_attr;
 	INIT_WORK(&vdpasim->work, vdpasim_work);
 	spin_lock_init(&vdpasim->lock);
 	spin_lock_init(&vdpasim->iommu_lock);
@@ -373,7 +378,7 @@ static struct vdpasim *vdpasim_create(void)
 		goto err_iommu;
 	set_dma_ops(dev, &vdpasim_dma_ops);
 
-	vdpasim->vqs = kcalloc(vdpasim->nvqs, sizeof(struct vdpasim_virtqueue),
+	vdpasim->vqs = kcalloc(dev_attr->nvqs, sizeof(struct vdpasim_virtqueue),
 			       GFP_KERNEL);
 	if (!vdpasim->vqs)
 		goto err_iommu;
@@ -396,7 +401,7 @@ static struct vdpasim *vdpasim_create(void)
 		eth_random_addr(vdpasim->config.mac);
 	}
 
-	for (i = 0; i < vdpasim->nvqs; i++)
+	for (i = 0; i < dev_attr->nvqs; i++)
 		vringh_set_iotlb(&vdpasim->vqs[i].vring, vdpasim->iommu);
 
 	vdpasim->vdpa.dma_dev = dev;
@@ -724,7 +729,11 @@ static const struct vdpa_config_ops vdpasim_net_batch_config_ops = {
 
 static int __init vdpasim_dev_init(void)
 {
-	vdpasim_dev = vdpasim_create();
+	struct vdpasim_dev_attr dev_attr = {};
+
+	dev_attr.nvqs = VDPASIM_VQ_NUM;
+
+	vdpasim_dev = vdpasim_create(&dev_attr);
 
 	if (!IS_ERR(vdpasim_dev))
 		return 0;
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH for 5.10 v2 3/5] vdpa_sim: store parsed MAC address in a buffer
  2021-02-16 14:24 [PATCH for 5.10 v2 0/5] vdpa_sim: fix param validation in vdpasim_get_config() Stefano Garzarella
  2021-02-16 14:24 ` [PATCH for 5.10 v2 1/5] vdpa_sim: remove hard-coded virtq count Stefano Garzarella
  2021-02-16 14:24 ` [PATCH for 5.10 v2 2/5] vdpa_sim: add struct vdpasim_dev_attr for device attributes Stefano Garzarella
@ 2021-02-16 14:24 ` Stefano Garzarella
  2021-02-16 14:24 ` [PATCH for 5.10 v2 4/5] vdpa_sim: make 'config' generic and usable for any device type Stefano Garzarella
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Stefano Garzarella @ 2021-02-16 14:24 UTC (permalink / raw)
  To: Greg KH
  Cc: Michael S. Tsirkin, stable, Jason Wang, virtualization, linux-kernel

commit cf1a3b35382c10ce315c32bd2b3d7789897fbe13 upstream.

As preparation for the next patches, we store the MAC address,
parsed during the vdpasim_create(), in a buffer that will be used
to fill 'config' together with other configurations.

Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Link: https://lore.kernel.org/r/20201215144256.155342-11-sgarzare@redhat.com
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Cc: <stable@vger.kernel.org> # 5.10.x
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index 6a5603f9d24c..bc27fa485c3e 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -42,6 +42,8 @@ static char *macaddr;
 module_param(macaddr, charp, 0);
 MODULE_PARM_DESC(macaddr, "Ethernet MAC address");
 
+u8 macaddr_buf[ETH_ALEN];
+
 struct vdpasim_virtqueue {
 	struct vringh vring;
 	struct vringh_kiov iov;
@@ -392,13 +394,13 @@ static struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
 		goto err_iommu;
 
 	if (macaddr) {
-		mac_pton(macaddr, vdpasim->config.mac);
-		if (!is_valid_ether_addr(vdpasim->config.mac)) {
+		mac_pton(macaddr, macaddr_buf);
+		if (!is_valid_ether_addr(macaddr_buf)) {
 			ret = -EADDRNOTAVAIL;
 			goto err_iommu;
 		}
 	} else {
-		eth_random_addr(vdpasim->config.mac);
+		eth_random_addr(macaddr_buf);
 	}
 
 	for (i = 0; i < dev_attr->nvqs; i++)
@@ -532,6 +534,8 @@ static int vdpasim_set_features(struct vdpa_device *vdpa, u64 features)
 
 	config->mtu = cpu_to_vdpasim16(vdpasim, 1500);
 	config->status = cpu_to_vdpasim16(vdpasim, VIRTIO_NET_S_LINK_UP);
+	memcpy(config->mac, macaddr_buf, ETH_ALEN);
+
 	return 0;
 }
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH for 5.10 v2 4/5] vdpa_sim: make 'config' generic and usable for any device type
  2021-02-16 14:24 [PATCH for 5.10 v2 0/5] vdpa_sim: fix param validation in vdpasim_get_config() Stefano Garzarella
                   ` (2 preceding siblings ...)
  2021-02-16 14:24 ` [PATCH for 5.10 v2 3/5] vdpa_sim: store parsed MAC address in a buffer Stefano Garzarella
@ 2021-02-16 14:24 ` Stefano Garzarella
  2021-02-16 14:24 ` [PATCH for 5.10 v2 5/5] vdpa_sim: add get_config callback in vdpasim_dev_attr Stefano Garzarella
  2021-02-17 10:58 ` [PATCH for 5.10 v2 0/5] vdpa_sim: fix param validation in vdpasim_get_config() Greg KH
  5 siblings, 0 replies; 8+ messages in thread
From: Stefano Garzarella @ 2021-02-16 14:24 UTC (permalink / raw)
  To: Greg KH
  Cc: Michael S. Tsirkin, stable, Jason Wang, virtualization, linux-kernel

commit f37cbbc65178e0a45823d281d290c4c02da9631c upstream.

Add new 'config_size' attribute in 'vdpasim_dev_attr' and allocates
'config' dynamically to support any device types.

Acked-by: Jason Wang <jasowang@redhat.com>
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Link: https://lore.kernel.org/r/20201215144256.155342-12-sgarzare@redhat.com
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Cc: <stable@vger.kernel.org> # 5.10.x
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index bc27fa485c3e..d9c494455156 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -70,6 +70,7 @@ static u64 vdpasim_features = (1ULL << VIRTIO_F_ANY_LAYOUT) |
 			      (1ULL << VIRTIO_NET_F_MAC);
 
 struct vdpasim_dev_attr {
+	size_t config_size;
 	int nvqs;
 };
 
@@ -81,7 +82,8 @@ struct vdpasim {
 	struct vdpasim_dev_attr dev_attr;
 	/* spinlock to synchronize virtqueue state */
 	spinlock_t lock;
-	struct virtio_net_config config;
+	/* virtio config according to device type */
+	void *config;
 	struct vhost_iotlb *iommu;
 	void *buffer;
 	u32 status;
@@ -380,6 +382,10 @@ static struct vdpasim *vdpasim_create(struct vdpasim_dev_attr *dev_attr)
 		goto err_iommu;
 	set_dma_ops(dev, &vdpasim_dma_ops);
 
+	vdpasim->config = kzalloc(dev_attr->config_size, GFP_KERNEL);
+	if (!vdpasim->config)
+		goto err_iommu;
+
 	vdpasim->vqs = kcalloc(dev_attr->nvqs, sizeof(struct vdpasim_virtqueue),
 			       GFP_KERNEL);
 	if (!vdpasim->vqs)
@@ -518,7 +524,8 @@ static u64 vdpasim_get_features(struct vdpa_device *vdpa)
 static int vdpasim_set_features(struct vdpa_device *vdpa, u64 features)
 {
 	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
-	struct virtio_net_config *config = &vdpasim->config;
+	struct virtio_net_config *config =
+		(struct virtio_net_config *)vdpasim->config;
 
 	/* DMA mapping must be done by driver */
 	if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)))
@@ -588,8 +595,8 @@ static void vdpasim_get_config(struct vdpa_device *vdpa, unsigned int offset,
 {
 	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
 
-	if (offset + len < sizeof(struct virtio_net_config))
-		memcpy(buf, (u8 *)&vdpasim->config + offset, len);
+	if (offset + len < vdpasim->dev_attr.config_size)
+		memcpy(buf, vdpasim->config + offset, len);
 }
 
 static void vdpasim_set_config(struct vdpa_device *vdpa, unsigned int offset,
@@ -676,6 +683,7 @@ static void vdpasim_free(struct vdpa_device *vdpa)
 	if (vdpasim->iommu)
 		vhost_iotlb_free(vdpasim->iommu);
 	kfree(vdpasim->vqs);
+	kfree(vdpasim->config);
 }
 
 static const struct vdpa_config_ops vdpasim_net_config_ops = {
@@ -736,6 +744,7 @@ static int __init vdpasim_dev_init(void)
 	struct vdpasim_dev_attr dev_attr = {};
 
 	dev_attr.nvqs = VDPASIM_VQ_NUM;
+	dev_attr.config_size = sizeof(struct virtio_net_config);
 
 	vdpasim_dev = vdpasim_create(&dev_attr);
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH for 5.10 v2 5/5] vdpa_sim: add get_config callback in vdpasim_dev_attr
  2021-02-16 14:24 [PATCH for 5.10 v2 0/5] vdpa_sim: fix param validation in vdpasim_get_config() Stefano Garzarella
                   ` (3 preceding siblings ...)
  2021-02-16 14:24 ` [PATCH for 5.10 v2 4/5] vdpa_sim: make 'config' generic and usable for any device type Stefano Garzarella
@ 2021-02-16 14:24 ` Stefano Garzarella
  2021-02-17 10:58 ` [PATCH for 5.10 v2 0/5] vdpa_sim: fix param validation in vdpasim_get_config() Greg KH
  5 siblings, 0 replies; 8+ messages in thread
From: Stefano Garzarella @ 2021-02-16 14:24 UTC (permalink / raw)
  To: Greg KH
  Cc: Michael S. Tsirkin, stable, Jason Wang, virtualization, linux-kernel

commit 65b709586e222fa6ffd4166ac7fdb5d5dad113ee upstream.

The get_config callback can be used by the device to fill the
config structure.
The callback will be invoked in vdpasim_get_config() before copying
bytes into caller buffer.

Move vDPA-net config updates from vdpasim_set_features() in the
new vdpasim_net_get_config() callback.
This is safe since in vdpa_get_config() we already check that
.set_features() callback is called before .get_config().

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Link: https://lore.kernel.org/r/20201215144256.155342-13-sgarzare@redhat.com
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Cc: <stable@vger.kernel.org> # 5.10.x
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 drivers/vdpa/vdpa_sim/vdpa_sim.c | 35 +++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim.c b/drivers/vdpa/vdpa_sim/vdpa_sim.c
index d9c494455156..f2ad450db547 100644
--- a/drivers/vdpa/vdpa_sim/vdpa_sim.c
+++ b/drivers/vdpa/vdpa_sim/vdpa_sim.c
@@ -69,9 +69,12 @@ static u64 vdpasim_features = (1ULL << VIRTIO_F_ANY_LAYOUT) |
 			      (1ULL << VIRTIO_F_ACCESS_PLATFORM) |
 			      (1ULL << VIRTIO_NET_F_MAC);
 
+struct vdpasim;
+
 struct vdpasim_dev_attr {
 	size_t config_size;
 	int nvqs;
+	void (*get_config)(struct vdpasim *vdpasim, void *config);
 };
 
 /* State of each vdpasim device */
@@ -524,8 +527,6 @@ static u64 vdpasim_get_features(struct vdpa_device *vdpa)
 static int vdpasim_set_features(struct vdpa_device *vdpa, u64 features)
 {
 	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
-	struct virtio_net_config *config =
-		(struct virtio_net_config *)vdpasim->config;
 
 	/* DMA mapping must be done by driver */
 	if (!(features & (1ULL << VIRTIO_F_ACCESS_PLATFORM)))
@@ -533,16 +534,6 @@ static int vdpasim_set_features(struct vdpa_device *vdpa, u64 features)
 
 	vdpasim->features = features & vdpasim_features;
 
-	/* We generally only know whether guest is using the legacy interface
-	 * here, so generally that's the earliest we can set config fields.
-	 * Note: We actually require VIRTIO_F_ACCESS_PLATFORM above which
-	 * implies VIRTIO_F_VERSION_1, but let's not try to be clever here.
-	 */
-
-	config->mtu = cpu_to_vdpasim16(vdpasim, 1500);
-	config->status = cpu_to_vdpasim16(vdpasim, VIRTIO_NET_S_LINK_UP);
-	memcpy(config->mac, macaddr_buf, ETH_ALEN);
-
 	return 0;
 }
 
@@ -595,8 +586,13 @@ static void vdpasim_get_config(struct vdpa_device *vdpa, unsigned int offset,
 {
 	struct vdpasim *vdpasim = vdpa_to_sim(vdpa);
 
-	if (offset + len < vdpasim->dev_attr.config_size)
-		memcpy(buf, vdpasim->config + offset, len);
+	if (offset + len > vdpasim->dev_attr.config_size)
+		return;
+
+	if (vdpasim->dev_attr.get_config)
+		vdpasim->dev_attr.get_config(vdpasim, vdpasim->config);
+
+	memcpy(buf, vdpasim->config + offset, len);
 }
 
 static void vdpasim_set_config(struct vdpa_device *vdpa, unsigned int offset,
@@ -739,12 +735,23 @@ static const struct vdpa_config_ops vdpasim_net_batch_config_ops = {
 	.free                   = vdpasim_free,
 };
 
+static void vdpasim_net_get_config(struct vdpasim *vdpasim, void *config)
+{
+	struct virtio_net_config *net_config =
+		(struct virtio_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 int __init vdpasim_dev_init(void)
 {
 	struct vdpasim_dev_attr dev_attr = {};
 
 	dev_attr.nvqs = VDPASIM_VQ_NUM;
 	dev_attr.config_size = sizeof(struct virtio_net_config);
+	dev_attr.get_config = vdpasim_net_get_config;
 
 	vdpasim_dev = vdpasim_create(&dev_attr);
 
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH for 5.10 v2 0/5] vdpa_sim: fix param validation in vdpasim_get_config()
  2021-02-16 14:24 [PATCH for 5.10 v2 0/5] vdpa_sim: fix param validation in vdpasim_get_config() Stefano Garzarella
                   ` (4 preceding siblings ...)
  2021-02-16 14:24 ` [PATCH for 5.10 v2 5/5] vdpa_sim: add get_config callback in vdpasim_dev_attr Stefano Garzarella
@ 2021-02-17 10:58 ` Greg KH
  2021-02-17 11:10   ` Stefano Garzarella
  5 siblings, 1 reply; 8+ messages in thread
From: Greg KH @ 2021-02-17 10:58 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: Michael S. Tsirkin, stable, Jason Wang, virtualization, linux-kernel

On Tue, Feb 16, 2021 at 03:24:34PM +0100, Stefano Garzarella wrote:
> v1: https://lore.kernel.org/stable/20210211162519.215418-1-sgarzare@redhat.com/
> 
> v2:
> - backport the upstream patch and related patches needed
> 
> Commit 65b709586e22 ("vdpa_sim: add get_config callback in
> vdpasim_dev_attr") unintentionally solved an issue in vdpasim_get_config()
> upstream while refactoring vdpa_sim.c to support multiple devices.
> 
> Before that patch, if 'offset + len' was equal to
> sizeof(struct virtio_net_config), the entire buffer wasn't filled,
> returning incorrect values to the caller.
> 
> Since 'vdpasim->config' type is 'struct virtio_net_config', we can
> safely copy its content under this condition.
> 
> The minimum set of patches to backport the patch that fixes the issue, is the
> following:
> 
>    423248d60d2b vdpa_sim: remove hard-coded virtq count
>    6c6e28fe4579 vdpa_sim: add struct vdpasim_dev_attr for device attributes
>    cf1a3b35382c vdpa_sim: store parsed MAC address in a buffer
>    f37cbbc65178 vdpa_sim: make 'config' generic and usable for any device type
>    65b709586e22 vdpa_sim: add get_config callback in vdpasim_dev_attr
> 
> The patches apply fairly cleanly. There are a few contextual differences
> due to the lack of the other patches:
> 
>    $ git backport-diff -u master -r linux-5.10.y..HEAD

Cool, where is 'backport-diff' from?

>    Key:
>    [----] : patches are identical
>    [####] : number of functional differences between upstream/downstream patch
>    [down] : patch is downstream-only
>    The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
> 
>    001/5:[----] [--] 'vdpa_sim: remove hard-coded virtq count'
>    002/5:[----] [-C] 'vdpa_sim: add struct vdpasim_dev_attr for device attributes'
>    003/5:[----] [--] 'vdpa_sim: store parsed MAC address in a buffer'
>    004/5:[----] [-C] 'vdpa_sim: make 'config' generic and usable for any device type'
>    005/5:[----] [-C] 'vdpa_sim: add get_config callback in vdpasim_dev_attr'

Now all applied, thanks.

greg k-h

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH for 5.10 v2 0/5] vdpa_sim: fix param validation in vdpasim_get_config()
  2021-02-17 10:58 ` [PATCH for 5.10 v2 0/5] vdpa_sim: fix param validation in vdpasim_get_config() Greg KH
@ 2021-02-17 11:10   ` Stefano Garzarella
  0 siblings, 0 replies; 8+ messages in thread
From: Stefano Garzarella @ 2021-02-17 11:10 UTC (permalink / raw)
  To: Greg KH
  Cc: Michael S. Tsirkin, stable, Jason Wang, virtualization, linux-kernel

On Wed, Feb 17, 2021 at 11:58:53AM +0100, Greg KH wrote:
>On Tue, Feb 16, 2021 at 03:24:34PM +0100, Stefano Garzarella wrote:
>> v1: https://lore.kernel.org/stable/20210211162519.215418-1-sgarzare@redhat.com/
>>
>> v2:
>> - backport the upstream patch and related patches needed
>>
>> Commit 65b709586e22 ("vdpa_sim: add get_config callback in
>> vdpasim_dev_attr") unintentionally solved an issue in vdpasim_get_config()
>> upstream while refactoring vdpa_sim.c to support multiple devices.
>>
>> Before that patch, if 'offset + len' was equal to
>> sizeof(struct virtio_net_config), the entire buffer wasn't filled,
>> returning incorrect values to the caller.
>>
>> Since 'vdpasim->config' type is 'struct virtio_net_config', we can
>> safely copy its content under this condition.
>>
>> The minimum set of patches to backport the patch that fixes the issue, is the
>> following:
>>
>>    423248d60d2b vdpa_sim: remove hard-coded virtq count
>>    6c6e28fe4579 vdpa_sim: add struct vdpasim_dev_attr for device attributes
>>    cf1a3b35382c vdpa_sim: store parsed MAC address in a buffer
>>    f37cbbc65178 vdpa_sim: make 'config' generic and usable for any device type
>>    65b709586e22 vdpa_sim: add get_config callback in vdpasim_dev_attr
>>
>> The patches apply fairly cleanly. There are a few contextual differences
>> due to the lack of the other patches:
>>
>>    $ git backport-diff -u master -r linux-5.10.y..HEAD
>
>Cool, where is 'backport-diff' from?

It was developed by Jeff Cody and I find it very useful when doing or 
reviewing backports :-)

It's available here:
https://github.com/codyprime/git-scripts/blob/master/git-backport-diff

>
>>    Key:
>>    [----] : patches are identical
>>    [####] : number of functional differences between upstream/downstream patch
>>    [down] : patch is downstream-only
>>    The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively
>>
>>    001/5:[----] [--] 'vdpa_sim: remove hard-coded virtq count'
>>    002/5:[----] [-C] 'vdpa_sim: add struct vdpasim_dev_attr for device attributes'
>>    003/5:[----] [--] 'vdpa_sim: store parsed MAC address in a buffer'
>>    004/5:[----] [-C] 'vdpa_sim: make 'config' generic and usable for any device type'
>>    005/5:[----] [-C] 'vdpa_sim: add get_config callback in vdpasim_dev_attr'
>
>Now all applied, thanks.

Thanks,
Stefano


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-02-17 11:12 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-16 14:24 [PATCH for 5.10 v2 0/5] vdpa_sim: fix param validation in vdpasim_get_config() Stefano Garzarella
2021-02-16 14:24 ` [PATCH for 5.10 v2 1/5] vdpa_sim: remove hard-coded virtq count Stefano Garzarella
2021-02-16 14:24 ` [PATCH for 5.10 v2 2/5] vdpa_sim: add struct vdpasim_dev_attr for device attributes Stefano Garzarella
2021-02-16 14:24 ` [PATCH for 5.10 v2 3/5] vdpa_sim: store parsed MAC address in a buffer Stefano Garzarella
2021-02-16 14:24 ` [PATCH for 5.10 v2 4/5] vdpa_sim: make 'config' generic and usable for any device type Stefano Garzarella
2021-02-16 14:24 ` [PATCH for 5.10 v2 5/5] vdpa_sim: add get_config callback in vdpasim_dev_attr Stefano Garzarella
2021-02-17 10:58 ` [PATCH for 5.10 v2 0/5] vdpa_sim: fix param validation in vdpasim_get_config() Greg KH
2021-02-17 11:10   ` Stefano Garzarella

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).