* [PATCH v3 01/12] virtio_ring: Merge identical variables
2022-05-16 10:41 [PATCH v3 00/12] virtio: Harden and test vring Andrew Scull
@ 2022-05-16 10:41 ` Andrew Scull
2022-06-08 17:25 ` Tom Rini
2022-05-16 10:41 ` [PATCH v3 02/12] virtio_ring: Add helper to attach vring descriptor Andrew Scull
` (10 subsequent siblings)
11 siblings, 1 reply; 14+ messages in thread
From: Andrew Scull @ 2022-05-16 10:41 UTC (permalink / raw)
To: u-boot; +Cc: sjg, trini, bmeng.cn, Andrew Scull
The variables `total_sg` and `descs_used` have the same value. Replace
the few uses of `total_sg` with `descs_used` to simplify the situation.
Signed-off-by: Andrew Scull <ascull@google.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
---
drivers/virtio/virtio_ring.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 7f1cbc5932..a6922ce1b8 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -20,17 +20,16 @@ int virtqueue_add(struct virtqueue *vq, struct virtio_sg *sgs[],
unsigned int out_sgs, unsigned int in_sgs)
{
struct vring_desc *desc;
- unsigned int total_sg = out_sgs + in_sgs;
- unsigned int i, n, avail, descs_used, uninitialized_var(prev);
+ unsigned int descs_used = out_sgs + in_sgs;
+ unsigned int i, n, avail, uninitialized_var(prev);
int head;
- WARN_ON(total_sg == 0);
+ WARN_ON(descs_used == 0);
head = vq->free_head;
desc = vq->vring.desc;
i = head;
- descs_used = total_sg;
if (vq->num_free < descs_used) {
debug("Can't add buf len %i - avail = %i\n",
--
2.36.0.550.gb090851708-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH v3 01/12] virtio_ring: Merge identical variables
2022-05-16 10:41 ` [PATCH v3 01/12] virtio_ring: Merge identical variables Andrew Scull
@ 2022-06-08 17:25 ` Tom Rini
0 siblings, 0 replies; 14+ messages in thread
From: Tom Rini @ 2022-06-08 17:25 UTC (permalink / raw)
To: Andrew Scull; +Cc: u-boot, sjg, bmeng.cn
[-- Attachment #1: Type: text/plain, Size: 425 bytes --]
On Mon, May 16, 2022 at 10:41:29AM +0000, Andrew Scull wrote:
> The variables `total_sg` and `descs_used` have the same value. Replace
> the few uses of `total_sg` with `descs_used` to simplify the situation.
>
> Signed-off-by: Andrew Scull <ascull@google.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
For the series, applied to u-boot/next, thanks!
--
Tom
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 02/12] virtio_ring: Add helper to attach vring descriptor
2022-05-16 10:41 [PATCH v3 00/12] virtio: Harden and test vring Andrew Scull
2022-05-16 10:41 ` [PATCH v3 01/12] virtio_ring: Merge identical variables Andrew Scull
@ 2022-05-16 10:41 ` Andrew Scull
2022-05-16 10:41 ` [PATCH v3 03/12] virtio_ring: Maintain a shadow copy of descriptors Andrew Scull
` (9 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Andrew Scull @ 2022-05-16 10:41 UTC (permalink / raw)
To: u-boot; +Cc: sjg, trini, bmeng.cn, Andrew Scull
Move the logic for attaching a descriptor to its own function.
Signed-off-by: Andrew Scull <ascull@google.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
drivers/virtio/virtio_ring.c | 36 +++++++++++++++++-------------------
1 file changed, 17 insertions(+), 19 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index a6922ce1b8..d3fc842f30 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -16,6 +16,18 @@
#include <linux/bug.h>
#include <linux/compat.h>
+static unsigned int virtqueue_attach_desc(struct virtqueue *vq, unsigned int i,
+ struct virtio_sg *sg, u16 flags)
+{
+ struct vring_desc *desc = &vq->vring.desc[i];
+
+ desc->addr = cpu_to_virtio64(vq->vdev, (u64)(uintptr_t)sg->addr);
+ desc->len = cpu_to_virtio32(vq->vdev, sg->length);
+ desc->flags = cpu_to_virtio16(vq->vdev, flags);
+
+ return virtio16_to_cpu(vq->vdev, desc->next);
+}
+
int virtqueue_add(struct virtqueue *vq, struct virtio_sg *sgs[],
unsigned int out_sgs, unsigned int in_sgs)
{
@@ -44,27 +56,13 @@ int virtqueue_add(struct virtqueue *vq, struct virtio_sg *sgs[],
return -ENOSPC;
}
- for (n = 0; n < out_sgs; n++) {
- struct virtio_sg *sg = sgs[n];
-
- desc[i].flags = cpu_to_virtio16(vq->vdev, VRING_DESC_F_NEXT);
- desc[i].addr = cpu_to_virtio64(vq->vdev, (u64)(size_t)sg->addr);
- desc[i].len = cpu_to_virtio32(vq->vdev, sg->length);
-
- prev = i;
- i = virtio16_to_cpu(vq->vdev, desc[i].next);
- }
- for (; n < (out_sgs + in_sgs); n++) {
- struct virtio_sg *sg = sgs[n];
-
- desc[i].flags = cpu_to_virtio16(vq->vdev, VRING_DESC_F_NEXT |
- VRING_DESC_F_WRITE);
- desc[i].addr = cpu_to_virtio64(vq->vdev,
- (u64)(uintptr_t)sg->addr);
- desc[i].len = cpu_to_virtio32(vq->vdev, sg->length);
+ for (n = 0; n < descs_used; n++) {
+ u16 flags = VRING_DESC_F_NEXT;
+ if (n >= out_sgs)
+ flags |= VRING_DESC_F_WRITE;
prev = i;
- i = virtio16_to_cpu(vq->vdev, desc[i].next);
+ i = virtqueue_attach_desc(vq, i, sgs[n], flags);
}
/* Last one doesn't continue */
desc[prev].flags &= cpu_to_virtio16(vq->vdev, ~VRING_DESC_F_NEXT);
--
2.36.0.550.gb090851708-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 03/12] virtio_ring: Maintain a shadow copy of descriptors
2022-05-16 10:41 [PATCH v3 00/12] virtio: Harden and test vring Andrew Scull
2022-05-16 10:41 ` [PATCH v3 01/12] virtio_ring: Merge identical variables Andrew Scull
2022-05-16 10:41 ` [PATCH v3 02/12] virtio_ring: Add helper to attach vring descriptor Andrew Scull
@ 2022-05-16 10:41 ` Andrew Scull
2022-05-16 10:41 ` [PATCH v3 04/12] virtio_ring: Check used descriptors are chain heads Andrew Scull
` (8 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Andrew Scull @ 2022-05-16 10:41 UTC (permalink / raw)
To: u-boot; +Cc: sjg, trini, bmeng.cn, Andrew Scull
The shared descriptors should only be written by the guest driver,
however, the device is still able to overwrite and corrupt them.
Maintain a private shadow copy of the descriptors for the driver to
use for state tracking, removing the need to read from the shared
descriptors.
Signed-off-by: Andrew Scull <ascull@google.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
drivers/virtio/virtio_ring.c | 49 ++++++++++++++++++++++++------------
include/virtio_ring.h | 10 ++++++++
2 files changed, 43 insertions(+), 16 deletions(-)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index d3fc842f30..73671d79da 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -19,13 +19,21 @@
static unsigned int virtqueue_attach_desc(struct virtqueue *vq, unsigned int i,
struct virtio_sg *sg, u16 flags)
{
+ struct vring_desc_shadow *desc_shadow = &vq->vring_desc_shadow[i];
struct vring_desc *desc = &vq->vring.desc[i];
- desc->addr = cpu_to_virtio64(vq->vdev, (u64)(uintptr_t)sg->addr);
- desc->len = cpu_to_virtio32(vq->vdev, sg->length);
- desc->flags = cpu_to_virtio16(vq->vdev, flags);
+ /* Update the shadow descriptor. */
+ desc_shadow->addr = (u64)(uintptr_t)sg->addr;
+ desc_shadow->len = sg->length;
+ desc_shadow->flags = flags;
- return virtio16_to_cpu(vq->vdev, desc->next);
+ /* Update the shared descriptor to match the shadow. */
+ desc->addr = cpu_to_virtio64(vq->vdev, desc_shadow->addr);
+ desc->len = cpu_to_virtio32(vq->vdev, desc_shadow->len);
+ desc->flags = cpu_to_virtio16(vq->vdev, desc_shadow->flags);
+ desc->next = cpu_to_virtio16(vq->vdev, desc_shadow->next);
+
+ return desc_shadow->next;
}
int virtqueue_add(struct virtqueue *vq, struct virtio_sg *sgs[],
@@ -65,7 +73,8 @@ int virtqueue_add(struct virtqueue *vq, struct virtio_sg *sgs[],
i = virtqueue_attach_desc(vq, i, sgs[n], flags);
}
/* Last one doesn't continue */
- desc[prev].flags &= cpu_to_virtio16(vq->vdev, ~VRING_DESC_F_NEXT);
+ vq->vring_desc_shadow[prev].flags &= ~VRING_DESC_F_NEXT;
+ desc[prev].flags = cpu_to_virtio16(vq->vdev, vq->vring_desc_shadow[prev].flags);
/* We're using some buffers from the free list. */
vq->num_free -= descs_used;
@@ -134,17 +143,16 @@ void virtqueue_kick(struct virtqueue *vq)
static void detach_buf(struct virtqueue *vq, unsigned int head)
{
unsigned int i;
- __virtio16 nextflag = cpu_to_virtio16(vq->vdev, VRING_DESC_F_NEXT);
/* Put back on free list: unmap first-level descriptors and find end */
i = head;
- while (vq->vring.desc[i].flags & nextflag) {
- i = virtio16_to_cpu(vq->vdev, vq->vring.desc[i].next);
+ while (vq->vring_desc_shadow[i].flags & VRING_DESC_F_NEXT) {
+ i = vq->vring_desc_shadow[i].next;
vq->num_free++;
}
- vq->vring.desc[i].next = cpu_to_virtio16(vq->vdev, vq->free_head);
+ vq->vring_desc_shadow[i].next = vq->free_head;
vq->free_head = head;
/* Plus final descriptor */
@@ -197,8 +205,7 @@ void *virtqueue_get_buf(struct virtqueue *vq, unsigned int *len)
virtio_store_mb(&vring_used_event(&vq->vring),
cpu_to_virtio16(vq->vdev, vq->last_used_idx));
- return (void *)(uintptr_t)virtio64_to_cpu(vq->vdev,
- vq->vring.desc[i].addr);
+ return (void *)(uintptr_t)vq->vring_desc_shadow[i].addr;
}
static struct virtqueue *__vring_new_virtqueue(unsigned int index,
@@ -207,6 +214,7 @@ static struct virtqueue *__vring_new_virtqueue(unsigned int index,
{
unsigned int i;
struct virtqueue *vq;
+ struct vring_desc_shadow *vring_desc_shadow;
struct virtio_dev_priv *uc_priv = dev_get_uclass_priv(udev);
struct udevice *vdev = uc_priv->vdev;
@@ -214,10 +222,17 @@ static struct virtqueue *__vring_new_virtqueue(unsigned int index,
if (!vq)
return NULL;
+ vring_desc_shadow = calloc(vring.num, sizeof(struct vring_desc_shadow));
+ if (!vring_desc_shadow) {
+ free(vq);
+ return NULL;
+ }
+
vq->vdev = vdev;
vq->index = index;
vq->num_free = vring.num;
vq->vring = vring;
+ vq->vring_desc_shadow = vring_desc_shadow;
vq->last_used_idx = 0;
vq->avail_flags_shadow = 0;
vq->avail_idx_shadow = 0;
@@ -235,7 +250,7 @@ static struct virtqueue *__vring_new_virtqueue(unsigned int index,
/* Put everything in free lists */
vq->free_head = 0;
for (i = 0; i < vring.num - 1; i++)
- vq->vring.desc[i].next = cpu_to_virtio16(vdev, i + 1);
+ vq->vring_desc_shadow[i].next = i + 1;
return vq;
}
@@ -288,6 +303,7 @@ struct virtqueue *vring_create_virtqueue(unsigned int index, unsigned int num,
void vring_del_virtqueue(struct virtqueue *vq)
{
free(vq->vring.desc);
+ free(vq->vring_desc_shadow);
list_del(&vq->list);
free(vq);
}
@@ -333,11 +349,12 @@ void virtqueue_dump(struct virtqueue *vq)
printf("\tlast_used_idx %u, avail_flags_shadow %u, avail_idx_shadow %u\n",
vq->last_used_idx, vq->avail_flags_shadow, vq->avail_idx_shadow);
- printf("Descriptor dump:\n");
+ printf("Shadow descriptor dump:\n");
for (i = 0; i < vq->vring.num; i++) {
- printf("\tdesc[%u] = { 0x%llx, len %u, flags %u, next %u }\n",
- i, vq->vring.desc[i].addr, vq->vring.desc[i].len,
- vq->vring.desc[i].flags, vq->vring.desc[i].next);
+ struct vring_desc_shadow *desc = &vq->vring_desc_shadow[i];
+
+ printf("\tdesc_shadow[%u] = { 0x%llx, len %u, flags %u, next %u }\n",
+ i, desc->addr, desc->len, desc->flags, desc->next);
}
printf("Avail ring dump:\n");
diff --git a/include/virtio_ring.h b/include/virtio_ring.h
index 6fc0593b14..52cbe77c0a 100644
--- a/include/virtio_ring.h
+++ b/include/virtio_ring.h
@@ -55,6 +55,14 @@ struct vring_desc {
__virtio16 next;
};
+/* Shadow of struct vring_desc in guest byte order. */
+struct vring_desc_shadow {
+ u64 addr;
+ u32 len;
+ u16 flags;
+ u16 next;
+};
+
struct vring_avail {
__virtio16 flags;
__virtio16 idx;
@@ -89,6 +97,7 @@ struct vring {
* @index: the zero-based ordinal number for this queue
* @num_free: number of elements we expect to be able to fit
* @vring: actual memory layout for this queue
+ * @vring_desc_shadow: guest-only copy of descriptors
* @event: host publishes avail event idx
* @free_head: head of free buffer list
* @num_added: number we've added since last sync
@@ -102,6 +111,7 @@ struct virtqueue {
unsigned int index;
unsigned int num_free;
struct vring vring;
+ struct vring_desc_shadow *vring_desc_shadow;
bool event;
unsigned int free_head;
unsigned int num_added;
--
2.36.0.550.gb090851708-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 04/12] virtio_ring: Check used descriptors are chain heads
2022-05-16 10:41 [PATCH v3 00/12] virtio: Harden and test vring Andrew Scull
` (2 preceding siblings ...)
2022-05-16 10:41 ` [PATCH v3 03/12] virtio_ring: Maintain a shadow copy of descriptors Andrew Scull
@ 2022-05-16 10:41 ` Andrew Scull
2022-05-16 10:41 ` [PATCH v3 05/12] dm: test: virtio: Test the virtio ring Andrew Scull
` (7 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Andrew Scull @ 2022-05-16 10:41 UTC (permalink / raw)
To: u-boot; +Cc: sjg, trini, bmeng.cn, Andrew Scull
When the device returns used buffers, it should refer to the descriptor
that is the head of the descriptor chain for that buffer. Confirm this
to be the case by tracking the head of descriptor chains that have been
made available to the device.
Signed-off-by: Andrew Scull <ascull@google.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
drivers/virtio/virtio_ring.c | 12 ++++++++++++
include/virtio_ring.h | 2 ++
2 files changed, 14 insertions(+)
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 73671d79da..f71bab7847 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -82,6 +82,9 @@ int virtqueue_add(struct virtqueue *vq, struct virtio_sg *sgs[],
/* Update free pointer */
vq->free_head = i;
+ /* Mark the descriptor as the head of a chain. */
+ vq->vring_desc_shadow[head].chain_head = true;
+
/*
* Put entry in available array (but don't update avail->idx
* until they do sync).
@@ -144,6 +147,9 @@ static void detach_buf(struct virtqueue *vq, unsigned int head)
{
unsigned int i;
+ /* Unmark the descriptor as the head of a chain. */
+ vq->vring_desc_shadow[head].chain_head = false;
+
/* Put back on free list: unmap first-level descriptors and find end */
i = head;
@@ -194,6 +200,12 @@ void *virtqueue_get_buf(struct virtqueue *vq, unsigned int *len)
return NULL;
}
+ if (unlikely(!vq->vring_desc_shadow[i].chain_head)) {
+ printf("(%s.%d): id %u is not a head\n",
+ vq->vdev->name, vq->index, i);
+ return NULL;
+ }
+
detach_buf(vq, i);
vq->last_used_idx++;
/*
diff --git a/include/virtio_ring.h b/include/virtio_ring.h
index 52cbe77c0a..c77c212cff 100644
--- a/include/virtio_ring.h
+++ b/include/virtio_ring.h
@@ -61,6 +61,8 @@ struct vring_desc_shadow {
u32 len;
u16 flags;
u16 next;
+ /* Metadata about the descriptor. */
+ bool chain_head;
};
struct vring_avail {
--
2.36.0.550.gb090851708-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 05/12] dm: test: virtio: Test the virtio ring
2022-05-16 10:41 [PATCH v3 00/12] virtio: Harden and test vring Andrew Scull
` (3 preceding siblings ...)
2022-05-16 10:41 ` [PATCH v3 04/12] virtio_ring: Check used descriptors are chain heads Andrew Scull
@ 2022-05-16 10:41 ` Andrew Scull
2022-05-16 10:41 ` [PATCH v3 06/12] virtio: sandbox: Fix device features bitfield Andrew Scull
` (6 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Andrew Scull @ 2022-05-16 10:41 UTC (permalink / raw)
To: u-boot; +Cc: sjg, trini, bmeng.cn, Andrew Scull
The virtio ring is the basis of virtio communication. Test its basic
functionality and its resilience against corruption from the device.
Signed-off-by: Andrew Scull <ascull@google.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
test/dm/virtio.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 72 insertions(+)
diff --git a/test/dm/virtio.c b/test/dm/virtio.c
index 9a7e658cce..adef10592c 100644
--- a/test/dm/virtio.c
+++ b/test/dm/virtio.c
@@ -130,3 +130,75 @@ static int dm_test_virtio_remove(struct unit_test_state *uts)
return 0;
}
DM_TEST(dm_test_virtio_remove, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+
+/* Test all of the virtio ring */
+static int dm_test_virtio_ring(struct unit_test_state *uts)
+{
+ struct udevice *bus, *dev;
+ struct virtio_dev_priv *uc_priv;
+ struct virtqueue *vq;
+ struct virtio_sg sg[2];
+ struct virtio_sg *sgs[2];
+ unsigned int len;
+ u8 buffer[2][32];
+
+ /* check probe success */
+ ut_assertok(uclass_first_device(UCLASS_VIRTIO, &bus));
+ ut_assertnonnull(bus);
+
+ /* check the child virtio-blk device is bound */
+ ut_assertok(device_find_first_child(bus, &dev));
+ ut_assertnonnull(dev);
+
+ /*
+ * fake the virtio device probe by filling in uc_priv->vdev
+ * which is used by virtio_find_vqs/virtio_del_vqs.
+ */
+ uc_priv = dev_get_uclass_priv(bus);
+ ut_assertnonnull(uc_priv);
+ uc_priv->vdev = dev;
+
+ /* prepare the scatter-gather buffer */
+ sg[0].addr = buffer[0];
+ sg[0].length = sizeof(buffer[0]);
+ sg[1].addr = buffer[1];
+ sg[1].length = sizeof(buffer[1]);
+ sgs[0] = &sg[0];
+ sgs[1] = &sg[1];
+
+ /* read a buffer and report written size from device */
+ ut_assertok(virtio_find_vqs(dev, 1, &vq));
+ ut_assertok(virtqueue_add(vq, sgs, 0, 1));
+ vq->vring.used->idx = 1;
+ vq->vring.used->ring[0].id = 0;
+ vq->vring.used->ring[0].len = 0x53355885;
+ ut_asserteq_ptr(buffer, virtqueue_get_buf(vq, &len));
+ ut_asserteq(0x53355885, len);
+ ut_assertok(virtio_del_vqs(dev));
+
+ /* rejects used descriptors that aren't a chain head */
+ ut_assertok(virtio_find_vqs(dev, 1, &vq));
+ ut_assertok(virtqueue_add(vq, sgs, 0, 2));
+ vq->vring.used->idx = 1;
+ vq->vring.used->ring[0].id = 1;
+ vq->vring.used->ring[0].len = 0x53355885;
+ ut_assertnull(virtqueue_get_buf(vq, &len));
+ ut_assertok(virtio_del_vqs(dev));
+
+ /* device changes to descriptor are ignored */
+ ut_assertok(virtio_find_vqs(dev, 1, &vq));
+ ut_assertok(virtqueue_add(vq, sgs, 0, 1));
+ vq->vring.desc[0].addr = cpu_to_virtio64(dev, 0xbadbad11);
+ vq->vring.desc[0].len = cpu_to_virtio32(dev, 0x11badbad);
+ vq->vring.desc[0].flags = cpu_to_virtio16(dev, VRING_DESC_F_NEXT);
+ vq->vring.desc[0].next = cpu_to_virtio16(dev, U16_MAX);
+ vq->vring.used->idx = 1;
+ vq->vring.used->ring[0].id = 0;
+ vq->vring.used->ring[0].len = 6;
+ ut_asserteq_ptr(buffer, virtqueue_get_buf(vq, &len));
+ ut_asserteq(6, len);
+ ut_assertok(virtio_del_vqs(dev));
+
+ return 0;
+}
+DM_TEST(dm_test_virtio_ring, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
--
2.36.0.550.gb090851708-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 06/12] virtio: sandbox: Fix device features bitfield
2022-05-16 10:41 [PATCH v3 00/12] virtio: Harden and test vring Andrew Scull
` (4 preceding siblings ...)
2022-05-16 10:41 ` [PATCH v3 05/12] dm: test: virtio: Test the virtio ring Andrew Scull
@ 2022-05-16 10:41 ` Andrew Scull
2022-05-16 10:41 ` [PATCH v3 07/12] test: dm: virtio: Test notify before del_vqs Andrew Scull
` (5 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Andrew Scull @ 2022-05-16 10:41 UTC (permalink / raw)
To: u-boot; +Cc: sjg, trini, bmeng.cn, Andrew Scull
The virtio sandbox transport was setting the device features value to
the bit index rather than shifting a bit to the right index. Fix this
using the bit manipulation macros.
Signed-off-by: Andrew Scull <ascull@google.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
drivers/virtio/virtio_sandbox.c | 2 +-
test/dm/virtio.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/virtio/virtio_sandbox.c b/drivers/virtio/virtio_sandbox.c
index aafb7beb94..a73b123454 100644
--- a/drivers/virtio/virtio_sandbox.c
+++ b/drivers/virtio/virtio_sandbox.c
@@ -160,7 +160,7 @@ static int virtio_sandbox_probe(struct udevice *udev)
struct virtio_dev_priv *uc_priv = dev_get_uclass_priv(udev);
/* fake some information for testing */
- priv->device_features = VIRTIO_F_VERSION_1;
+ priv->device_features = BIT_ULL(VIRTIO_F_VERSION_1);
uc_priv->device = VIRTIO_ID_BLOCK;
uc_priv->vendor = ('u' << 24) | ('b' << 16) | ('o' << 8) | 't';
diff --git a/test/dm/virtio.c b/test/dm/virtio.c
index adef10592c..aa4e3d778e 100644
--- a/test/dm/virtio.c
+++ b/test/dm/virtio.c
@@ -77,7 +77,7 @@ static int dm_test_virtio_all_ops(struct unit_test_state *uts)
ut_assertok(virtio_get_status(dev, &status));
ut_asserteq(0, status);
ut_assertok(virtio_get_features(dev, &features));
- ut_asserteq(VIRTIO_F_VERSION_1, features);
+ ut_asserteq_64(BIT_ULL(VIRTIO_F_VERSION_1), features);
ut_assertok(virtio_set_features(dev));
ut_assertok(virtio_find_vqs(dev, nvqs, vqs));
ut_assertok(virtio_del_vqs(dev));
--
2.36.0.550.gb090851708-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 07/12] test: dm: virtio: Test notify before del_vqs
2022-05-16 10:41 [PATCH v3 00/12] virtio: Harden and test vring Andrew Scull
` (5 preceding siblings ...)
2022-05-16 10:41 ` [PATCH v3 06/12] virtio: sandbox: Fix device features bitfield Andrew Scull
@ 2022-05-16 10:41 ` Andrew Scull
2022-05-16 10:41 ` [PATCH v3 08/12] test: dm: virtio: Split out virtio device tests Andrew Scull
` (4 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Andrew Scull @ 2022-05-16 10:41 UTC (permalink / raw)
To: u-boot; +Cc: sjg, trini, bmeng.cn, Andrew Scull
The virtqueue is passed to virtio_notify() so move the virtqueue
deletion to the end of the test when it's no longer needed. This wasn't
causing any problems because the sandbox virtio transport driver doesn't
do anything for notifications, but it could cause problems if things
change and it was a bad example.
Signed-off-by: Andrew Scull <ascull@google.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
test/dm/virtio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/test/dm/virtio.c b/test/dm/virtio.c
index aa4e3d778e..ff1dea323c 100644
--- a/test/dm/virtio.c
+++ b/test/dm/virtio.c
@@ -80,8 +80,8 @@ static int dm_test_virtio_all_ops(struct unit_test_state *uts)
ut_asserteq_64(BIT_ULL(VIRTIO_F_VERSION_1), features);
ut_assertok(virtio_set_features(dev));
ut_assertok(virtio_find_vqs(dev, nvqs, vqs));
- ut_assertok(virtio_del_vqs(dev));
ut_assertok(virtio_notify(dev, vqs[0]));
+ ut_assertok(virtio_del_vqs(dev));
return 0;
}
--
2.36.0.550.gb090851708-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 08/12] test: dm: virtio: Split out virtio device tests
2022-05-16 10:41 [PATCH v3 00/12] virtio: Harden and test vring Andrew Scull
` (6 preceding siblings ...)
2022-05-16 10:41 ` [PATCH v3 07/12] test: dm: virtio: Test notify before del_vqs Andrew Scull
@ 2022-05-16 10:41 ` Andrew Scull
2022-05-16 10:41 ` [PATCH v3 09/12] virtio: sandbox: Bind RNG rather than block device Andrew Scull
` (3 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Andrew Scull @ 2022-05-16 10:41 UTC (permalink / raw)
To: u-boot; +Cc: sjg, trini, bmeng.cn, Andrew Scull
Virtio tests that find a child device require the virtio device driver
to be included in the build so it can probe. The sandbox virtio
transport driver currently reports a virtio-blk device so make sure the
corresponding driver is built before running tests that need it.
Signed-off-by: Andrew Scull <ascull@google.com>
---
test/dm/Makefile | 5 +-
test/dm/virtio.c | 171 ------------------------------------
test/dm/virtio_device.c | 186 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 190 insertions(+), 172 deletions(-)
create mode 100644 test/dm/virtio_device.c
diff --git a/test/dm/Makefile b/test/dm/Makefile
index f0a7c97e3d..29dd143517 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -107,7 +107,10 @@ obj-$(CONFIG_TEE) += tee.o
obj-$(CONFIG_TIMER) += timer.o
obj-$(CONFIG_DM_USB) += usb.o
obj-$(CONFIG_DM_VIDEO) += video.o
-obj-$(CONFIG_VIRTIO_SANDBOX) += virtio.o
+ifeq ($(CONFIG_VIRTIO_SANDBOX),y)
+obj-y += virtio.o
+obj-$(CONFIG_VIRTIO_BLK) += virtio_device.o
+endif
ifeq ($(CONFIG_WDT_GPIO)$(CONFIG_WDT_SANDBOX),yy)
obj-y += wdt.o
endif
diff --git a/test/dm/virtio.c b/test/dm/virtio.c
index ff1dea323c..3e108cdc35 100644
--- a/test/dm/virtio.c
+++ b/test/dm/virtio.c
@@ -7,7 +7,6 @@
#include <dm.h>
#include <virtio_types.h>
#include <virtio.h>
-#include <virtio_ring.h>
#include <dm/device-internal.h>
#include <dm/root.h>
#include <dm/test.h>
@@ -15,78 +14,6 @@
#include <test/test.h>
#include <test/ut.h>
-/* Basic test of the virtio uclass */
-static int dm_test_virtio_base(struct unit_test_state *uts)
-{
- struct udevice *bus, *dev;
- u8 status;
-
- /* check probe success */
- ut_assertok(uclass_first_device(UCLASS_VIRTIO, &bus));
- ut_assertnonnull(bus);
-
- /* check the child virtio-blk device is bound */
- ut_assertok(device_find_first_child(bus, &dev));
- ut_assertnonnull(dev);
- ut_assertok(strcmp(dev->name, "virtio-blk#0"));
-
- /* check driver status */
- ut_assertok(virtio_get_status(dev, &status));
- ut_asserteq(VIRTIO_CONFIG_S_ACKNOWLEDGE, status);
-
- return 0;
-}
-DM_TEST(dm_test_virtio_base, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
-
-/* Test all of the virtio uclass ops */
-static int dm_test_virtio_all_ops(struct unit_test_state *uts)
-{
- struct udevice *bus, *dev;
- struct virtio_dev_priv *uc_priv;
- uint offset = 0, len = 0, nvqs = 1;
- void *buffer = NULL;
- u8 status;
- u32 counter;
- u64 features;
- struct virtqueue *vqs[2];
-
- /* check probe success */
- ut_assertok(uclass_first_device(UCLASS_VIRTIO, &bus));
- ut_assertnonnull(bus);
-
- /* check the child virtio-blk device is bound */
- ut_assertok(device_find_first_child(bus, &dev));
- ut_assertnonnull(dev);
-
- /*
- * fake the virtio device probe by filling in uc_priv->vdev
- * which is used by virtio_find_vqs/virtio_del_vqs.
- */
- uc_priv = dev_get_uclass_priv(bus);
- ut_assertnonnull(uc_priv);
- uc_priv->vdev = dev;
-
- /* test virtio_xxx APIs */
- ut_assertok(virtio_get_config(dev, offset, buffer, len));
- ut_assertok(virtio_set_config(dev, offset, buffer, len));
- ut_asserteq(-ENOSYS, virtio_generation(dev, &counter));
- ut_assertok(virtio_set_status(dev, VIRTIO_CONFIG_S_DRIVER_OK));
- ut_assertok(virtio_get_status(dev, &status));
- ut_asserteq(VIRTIO_CONFIG_S_DRIVER_OK, status);
- ut_assertok(virtio_reset(dev));
- ut_assertok(virtio_get_status(dev, &status));
- ut_asserteq(0, status);
- ut_assertok(virtio_get_features(dev, &features));
- ut_asserteq_64(BIT_ULL(VIRTIO_F_VERSION_1), features);
- ut_assertok(virtio_set_features(dev));
- ut_assertok(virtio_find_vqs(dev, nvqs, vqs));
- ut_assertok(virtio_notify(dev, vqs[0]));
- ut_assertok(virtio_del_vqs(dev));
-
- return 0;
-}
-DM_TEST(dm_test_virtio_all_ops, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
-
/* Test of the virtio driver that does not have required driver ops */
static int dm_test_virtio_missing_ops(struct unit_test_state *uts)
{
@@ -104,101 +31,3 @@ static int dm_test_virtio_missing_ops(struct unit_test_state *uts)
return 0;
}
DM_TEST(dm_test_virtio_missing_ops, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
-
-/* Test removal of virtio device driver */
-static int dm_test_virtio_remove(struct unit_test_state *uts)
-{
- struct udevice *bus, *dev;
-
- /* check probe success */
- ut_assertok(uclass_first_device(UCLASS_VIRTIO, &bus));
- ut_assertnonnull(bus);
-
- /* check the child virtio-blk device is bound */
- ut_assertok(device_find_first_child(bus, &dev));
- ut_assertnonnull(dev);
-
- /* set driver status to VIRTIO_CONFIG_S_DRIVER_OK */
- ut_assertok(virtio_set_status(dev, VIRTIO_CONFIG_S_DRIVER_OK));
-
- /* check the device can be successfully removed */
- dev_or_flags(dev, DM_FLAG_ACTIVATED);
- ut_asserteq(-EKEYREJECTED, device_remove(bus, DM_REMOVE_ACTIVE_ALL));
-
- ut_asserteq(false, device_active(dev));
-
- return 0;
-}
-DM_TEST(dm_test_virtio_remove, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
-
-/* Test all of the virtio ring */
-static int dm_test_virtio_ring(struct unit_test_state *uts)
-{
- struct udevice *bus, *dev;
- struct virtio_dev_priv *uc_priv;
- struct virtqueue *vq;
- struct virtio_sg sg[2];
- struct virtio_sg *sgs[2];
- unsigned int len;
- u8 buffer[2][32];
-
- /* check probe success */
- ut_assertok(uclass_first_device(UCLASS_VIRTIO, &bus));
- ut_assertnonnull(bus);
-
- /* check the child virtio-blk device is bound */
- ut_assertok(device_find_first_child(bus, &dev));
- ut_assertnonnull(dev);
-
- /*
- * fake the virtio device probe by filling in uc_priv->vdev
- * which is used by virtio_find_vqs/virtio_del_vqs.
- */
- uc_priv = dev_get_uclass_priv(bus);
- ut_assertnonnull(uc_priv);
- uc_priv->vdev = dev;
-
- /* prepare the scatter-gather buffer */
- sg[0].addr = buffer[0];
- sg[0].length = sizeof(buffer[0]);
- sg[1].addr = buffer[1];
- sg[1].length = sizeof(buffer[1]);
- sgs[0] = &sg[0];
- sgs[1] = &sg[1];
-
- /* read a buffer and report written size from device */
- ut_assertok(virtio_find_vqs(dev, 1, &vq));
- ut_assertok(virtqueue_add(vq, sgs, 0, 1));
- vq->vring.used->idx = 1;
- vq->vring.used->ring[0].id = 0;
- vq->vring.used->ring[0].len = 0x53355885;
- ut_asserteq_ptr(buffer, virtqueue_get_buf(vq, &len));
- ut_asserteq(0x53355885, len);
- ut_assertok(virtio_del_vqs(dev));
-
- /* rejects used descriptors that aren't a chain head */
- ut_assertok(virtio_find_vqs(dev, 1, &vq));
- ut_assertok(virtqueue_add(vq, sgs, 0, 2));
- vq->vring.used->idx = 1;
- vq->vring.used->ring[0].id = 1;
- vq->vring.used->ring[0].len = 0x53355885;
- ut_assertnull(virtqueue_get_buf(vq, &len));
- ut_assertok(virtio_del_vqs(dev));
-
- /* device changes to descriptor are ignored */
- ut_assertok(virtio_find_vqs(dev, 1, &vq));
- ut_assertok(virtqueue_add(vq, sgs, 0, 1));
- vq->vring.desc[0].addr = cpu_to_virtio64(dev, 0xbadbad11);
- vq->vring.desc[0].len = cpu_to_virtio32(dev, 0x11badbad);
- vq->vring.desc[0].flags = cpu_to_virtio16(dev, VRING_DESC_F_NEXT);
- vq->vring.desc[0].next = cpu_to_virtio16(dev, U16_MAX);
- vq->vring.used->idx = 1;
- vq->vring.used->ring[0].id = 0;
- vq->vring.used->ring[0].len = 6;
- ut_asserteq_ptr(buffer, virtqueue_get_buf(vq, &len));
- ut_asserteq(6, len);
- ut_assertok(virtio_del_vqs(dev));
-
- return 0;
-}
-DM_TEST(dm_test_virtio_ring, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
diff --git a/test/dm/virtio_device.c b/test/dm/virtio_device.c
new file mode 100644
index 0000000000..46f4798fc2
--- /dev/null
+++ b/test/dm/virtio_device.c
@@ -0,0 +1,186 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2018, Bin Meng <bmeng.cn@gmail.com>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <virtio_types.h>
+#include <virtio.h>
+#include <virtio_ring.h>
+#include <dm/device-internal.h>
+#include <dm/root.h>
+#include <dm/test.h>
+#include <dm/uclass-internal.h>
+#include <test/test.h>
+#include <test/ut.h>
+
+/* Basic test of the virtio uclass */
+static int dm_test_virtio_base(struct unit_test_state *uts)
+{
+ struct udevice *bus, *dev;
+ u8 status;
+
+ /* check probe success */
+ ut_assertok(uclass_first_device(UCLASS_VIRTIO, &bus));
+ ut_assertnonnull(bus);
+
+ /* check the child virtio-blk device is bound */
+ ut_assertok(device_find_first_child(bus, &dev));
+ ut_assertnonnull(dev);
+ ut_assertok(strcmp(dev->name, "virtio-blk#0"));
+
+ /* check driver status */
+ ut_assertok(virtio_get_status(dev, &status));
+ ut_asserteq(VIRTIO_CONFIG_S_ACKNOWLEDGE, status);
+
+ return 0;
+}
+DM_TEST(dm_test_virtio_base, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+
+/* Test all of the virtio uclass ops */
+static int dm_test_virtio_all_ops(struct unit_test_state *uts)
+{
+ struct udevice *bus, *dev;
+ struct virtio_dev_priv *uc_priv;
+ uint offset = 0, len = 0, nvqs = 1;
+ void *buffer = NULL;
+ u8 status;
+ u32 counter;
+ u64 features;
+ struct virtqueue *vqs[2];
+
+ /* check probe success */
+ ut_assertok(uclass_first_device(UCLASS_VIRTIO, &bus));
+ ut_assertnonnull(bus);
+
+ /* check the child virtio-rng device is bound */
+ ut_assertok(device_find_first_child(bus, &dev));
+ ut_assertnonnull(dev);
+
+ /*
+ * fake the virtio device probe by filling in uc_priv->vdev
+ * which is used by virtio_find_vqs/virtio_del_vqs.
+ */
+ uc_priv = dev_get_uclass_priv(bus);
+ ut_assertnonnull(uc_priv);
+ uc_priv->vdev = dev;
+
+ /* test virtio_xxx APIs */
+ ut_assertok(virtio_get_config(dev, offset, buffer, len));
+ ut_assertok(virtio_set_config(dev, offset, buffer, len));
+ ut_asserteq(-ENOSYS, virtio_generation(dev, &counter));
+ ut_assertok(virtio_set_status(dev, VIRTIO_CONFIG_S_DRIVER_OK));
+ ut_assertok(virtio_get_status(dev, &status));
+ ut_asserteq(VIRTIO_CONFIG_S_DRIVER_OK, status);
+ ut_assertok(virtio_reset(dev));
+ ut_assertok(virtio_get_status(dev, &status));
+ ut_asserteq(0, status);
+ ut_assertok(virtio_get_features(dev, &features));
+ ut_asserteq_64(BIT_ULL(VIRTIO_F_VERSION_1), features);
+ ut_assertok(virtio_set_features(dev));
+ ut_assertok(virtio_find_vqs(dev, nvqs, vqs));
+ ut_assertok(virtio_notify(dev, vqs[0]));
+ ut_assertok(virtio_del_vqs(dev));
+
+ return 0;
+}
+DM_TEST(dm_test_virtio_all_ops, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+
+/* Test removal of virtio device driver */
+static int dm_test_virtio_remove(struct unit_test_state *uts)
+{
+ struct udevice *bus, *dev;
+
+ /* check probe success */
+ ut_assertok(uclass_first_device(UCLASS_VIRTIO, &bus));
+ ut_assertnonnull(bus);
+
+ /* check the child virtio-rng device is bound */
+ ut_assertok(device_find_first_child(bus, &dev));
+ ut_assertnonnull(dev);
+
+ /* set driver status to VIRTIO_CONFIG_S_DRIVER_OK */
+ ut_assertok(virtio_set_status(dev, VIRTIO_CONFIG_S_DRIVER_OK));
+
+ /* check the device can be successfully removed */
+ dev_or_flags(dev, DM_FLAG_ACTIVATED);
+ ut_asserteq(-EKEYREJECTED, device_remove(bus, DM_REMOVE_ACTIVE_ALL));
+
+ ut_asserteq(false, device_active(dev));
+
+ return 0;
+}
+DM_TEST(dm_test_virtio_remove, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
+
+/* Test all of the virtio ring */
+static int dm_test_virtio_ring(struct unit_test_state *uts)
+{
+ struct udevice *bus, *dev;
+ struct virtio_dev_priv *uc_priv;
+ struct virtqueue *vq;
+ struct virtio_sg sg[2];
+ struct virtio_sg *sgs[2];
+ unsigned int len;
+ u8 buffer[2][32];
+
+ /* check probe success */
+ ut_assertok(uclass_first_device(UCLASS_VIRTIO, &bus));
+ ut_assertnonnull(bus);
+
+ /* check the child virtio-blk device is bound */
+ ut_assertok(device_find_first_child(bus, &dev));
+ ut_assertnonnull(dev);
+
+ /*
+ * fake the virtio device probe by filling in uc_priv->vdev
+ * which is used by virtio_find_vqs/virtio_del_vqs.
+ */
+ uc_priv = dev_get_uclass_priv(bus);
+ ut_assertnonnull(uc_priv);
+ uc_priv->vdev = dev;
+
+ /* prepare the scatter-gather buffer */
+ sg[0].addr = buffer[0];
+ sg[0].length = sizeof(buffer[0]);
+ sg[1].addr = buffer[1];
+ sg[1].length = sizeof(buffer[1]);
+ sgs[0] = &sg[0];
+ sgs[1] = &sg[1];
+
+ /* read a buffer and report written size from device */
+ ut_assertok(virtio_find_vqs(dev, 1, &vq));
+ ut_assertok(virtqueue_add(vq, sgs, 0, 1));
+ vq->vring.used->idx = 1;
+ vq->vring.used->ring[0].id = 0;
+ vq->vring.used->ring[0].len = 0x53355885;
+ ut_asserteq_ptr(buffer, virtqueue_get_buf(vq, &len));
+ ut_asserteq(0x53355885, len);
+ ut_assertok(virtio_del_vqs(dev));
+
+ /* rejects used descriptors that aren't a chain head */
+ ut_assertok(virtio_find_vqs(dev, 1, &vq));
+ ut_assertok(virtqueue_add(vq, sgs, 0, 2));
+ vq->vring.used->idx = 1;
+ vq->vring.used->ring[0].id = 1;
+ vq->vring.used->ring[0].len = 0x53355885;
+ ut_assertnull(virtqueue_get_buf(vq, &len));
+ ut_assertok(virtio_del_vqs(dev));
+
+ /* device changes to descriptor are ignored */
+ ut_assertok(virtio_find_vqs(dev, 1, &vq));
+ ut_assertok(virtqueue_add(vq, sgs, 0, 1));
+ vq->vring.desc[0].addr = cpu_to_virtio64(dev, 0xbadbad11);
+ vq->vring.desc[0].len = cpu_to_virtio32(dev, 0x11badbad);
+ vq->vring.desc[0].flags = cpu_to_virtio16(dev, VRING_DESC_F_NEXT);
+ vq->vring.desc[0].next = cpu_to_virtio16(dev, U16_MAX);
+ vq->vring.used->idx = 1;
+ vq->vring.used->ring[0].id = 0;
+ vq->vring.used->ring[0].len = 6;
+ ut_asserteq_ptr(buffer, virtqueue_get_buf(vq, &len));
+ ut_asserteq(6, len);
+ ut_assertok(virtio_del_vqs(dev));
+
+ return 0;
+}
+DM_TEST(dm_test_virtio_ring, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
--
2.36.0.550.gb090851708-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 09/12] virtio: sandbox: Bind RNG rather than block device
2022-05-16 10:41 [PATCH v3 00/12] virtio: Harden and test vring Andrew Scull
` (7 preceding siblings ...)
2022-05-16 10:41 ` [PATCH v3 08/12] test: dm: virtio: Split out virtio device tests Andrew Scull
@ 2022-05-16 10:41 ` Andrew Scull
2022-05-16 10:41 ` [PATCH v3 10/12] test: dm: virtio: Test virtio device driver probing Andrew Scull
` (2 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Andrew Scull @ 2022-05-16 10:41 UTC (permalink / raw)
To: u-boot; +Cc: sjg, trini, bmeng.cn, Andrew Scull
The virtio-rng driver is extremely simple, making it suitable for
testing more of the virtio uclass logic. Have the sandbox driver bind
the virtio-rng driver rather than the virtio-blk driver so it can be
used in tests.
Signed-off-by: Andrew Scull <ascull@google.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
drivers/virtio/virtio_sandbox.c | 2 +-
test/dm/Makefile | 2 +-
test/dm/virtio_device.c | 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/virtio/virtio_sandbox.c b/drivers/virtio/virtio_sandbox.c
index a73b123454..5484ae3a1a 100644
--- a/drivers/virtio/virtio_sandbox.c
+++ b/drivers/virtio/virtio_sandbox.c
@@ -161,7 +161,7 @@ static int virtio_sandbox_probe(struct udevice *udev)
/* fake some information for testing */
priv->device_features = BIT_ULL(VIRTIO_F_VERSION_1);
- uc_priv->device = VIRTIO_ID_BLOCK;
+ uc_priv->device = VIRTIO_ID_RNG;
uc_priv->vendor = ('u' << 24) | ('b' << 16) | ('o' << 8) | 't';
return 0;
diff --git a/test/dm/Makefile b/test/dm/Makefile
index 29dd143517..809f0f239f 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -109,7 +109,7 @@ obj-$(CONFIG_DM_USB) += usb.o
obj-$(CONFIG_DM_VIDEO) += video.o
ifeq ($(CONFIG_VIRTIO_SANDBOX),y)
obj-y += virtio.o
-obj-$(CONFIG_VIRTIO_BLK) += virtio_device.o
+obj-$(CONFIG_VIRTIO_RNG) += virtio_device.o
endif
ifeq ($(CONFIG_WDT_GPIO)$(CONFIG_WDT_SANDBOX),yy)
obj-y += wdt.o
diff --git a/test/dm/virtio_device.c b/test/dm/virtio_device.c
index 46f4798fc2..f5f2349750 100644
--- a/test/dm/virtio_device.c
+++ b/test/dm/virtio_device.c
@@ -25,10 +25,10 @@ static int dm_test_virtio_base(struct unit_test_state *uts)
ut_assertok(uclass_first_device(UCLASS_VIRTIO, &bus));
ut_assertnonnull(bus);
- /* check the child virtio-blk device is bound */
+ /* check the child virtio-rng device is bound */
ut_assertok(device_find_first_child(bus, &dev));
ut_assertnonnull(dev);
- ut_assertok(strcmp(dev->name, "virtio-blk#0"));
+ ut_asserteq_str("virtio-rng#0", dev->name);
/* check driver status */
ut_assertok(virtio_get_status(dev, &status));
--
2.36.0.550.gb090851708-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 10/12] test: dm: virtio: Test virtio device driver probing
2022-05-16 10:41 [PATCH v3 00/12] virtio: Harden and test vring Andrew Scull
` (8 preceding siblings ...)
2022-05-16 10:41 ` [PATCH v3 09/12] virtio: sandbox: Bind RNG rather than block device Andrew Scull
@ 2022-05-16 10:41 ` Andrew Scull
2022-05-16 10:41 ` [PATCH v3 11/12] virtio: rng: Check length before copying Andrew Scull
2022-05-16 10:41 ` [PATCH v3 12/12] test: dm: virtio_rng: Test virtio-rng with faked device Andrew Scull
11 siblings, 0 replies; 14+ messages in thread
From: Andrew Scull @ 2022-05-16 10:41 UTC (permalink / raw)
To: u-boot; +Cc: sjg, trini, bmeng.cn, Andrew Scull
Once the virtio-rng driver has been bound, probe it to trigger the pre
and post child probe hooks of the virtio uclass driver. Check the status
of the virtio device to confirm it reached the expected state.
Signed-off-by: Andrew Scull <ascull@google.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
test/dm/virtio_device.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/test/dm/virtio_device.c b/test/dm/virtio_device.c
index f5f2349750..d0195e6bf0 100644
--- a/test/dm/virtio_device.c
+++ b/test/dm/virtio_device.c
@@ -34,6 +34,15 @@ static int dm_test_virtio_base(struct unit_test_state *uts)
ut_assertok(virtio_get_status(dev, &status));
ut_asserteq(VIRTIO_CONFIG_S_ACKNOWLEDGE, status);
+ /* probe the virtio-rng driver */
+ ut_assertok(device_probe(dev));
+
+ /* check the device was reset and the driver picked up the device */
+ ut_assertok(virtio_get_status(dev, &status));
+ ut_asserteq(VIRTIO_CONFIG_S_DRIVER |
+ VIRTIO_CONFIG_S_DRIVER_OK |
+ VIRTIO_CONFIG_S_FEATURES_OK, status);
+
return 0;
}
DM_TEST(dm_test_virtio_base, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
--
2.36.0.550.gb090851708-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 11/12] virtio: rng: Check length before copying
2022-05-16 10:41 [PATCH v3 00/12] virtio: Harden and test vring Andrew Scull
` (9 preceding siblings ...)
2022-05-16 10:41 ` [PATCH v3 10/12] test: dm: virtio: Test virtio device driver probing Andrew Scull
@ 2022-05-16 10:41 ` Andrew Scull
2022-05-16 10:41 ` [PATCH v3 12/12] test: dm: virtio_rng: Test virtio-rng with faked device Andrew Scull
11 siblings, 0 replies; 14+ messages in thread
From: Andrew Scull @ 2022-05-16 10:41 UTC (permalink / raw)
To: u-boot; +Cc: sjg, trini, bmeng.cn, Andrew Scull, Sughosh Ganu
Check the length of data written by the device is consistent with the
size of the buffers to avoid out-of-bounds memory accesses in case
values aren't consistent.
Signed-off-by: Andrew Scull <ascull@google.com>
Cc: Sughosh Ganu <sughosh.ganu@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
drivers/virtio/virtio_rng.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/virtio/virtio_rng.c b/drivers/virtio/virtio_rng.c
index 9314c0a03e..b85545c2ee 100644
--- a/drivers/virtio/virtio_rng.c
+++ b/drivers/virtio/virtio_rng.c
@@ -41,6 +41,9 @@ static int virtio_rng_read(struct udevice *dev, void *data, size_t len)
while (!virtqueue_get_buf(priv->rng_vq, &rsize))
;
+ if (rsize > sg.length)
+ return -EIO;
+
memcpy(ptr, buf, rsize);
len -= rsize;
ptr += rsize;
--
2.36.0.550.gb090851708-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH v3 12/12] test: dm: virtio_rng: Test virtio-rng with faked device
2022-05-16 10:41 [PATCH v3 00/12] virtio: Harden and test vring Andrew Scull
` (10 preceding siblings ...)
2022-05-16 10:41 ` [PATCH v3 11/12] virtio: rng: Check length before copying Andrew Scull
@ 2022-05-16 10:41 ` Andrew Scull
11 siblings, 0 replies; 14+ messages in thread
From: Andrew Scull @ 2022-05-16 10:41 UTC (permalink / raw)
To: u-boot; +Cc: sjg, trini, bmeng.cn, Andrew Scull
Add a regression test for virtio-rng reading beyond the end of its
buffer if the virtio device provides an invalid length.
Signed-off-by: Andrew Scull <ascull@google.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
test/dm/Makefile | 1 +
test/dm/virtio_rng.c | 52 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 53 insertions(+)
create mode 100644 test/dm/virtio_rng.c
diff --git a/test/dm/Makefile b/test/dm/Makefile
index 809f0f239f..caea52f4e2 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -110,6 +110,7 @@ obj-$(CONFIG_DM_VIDEO) += video.o
ifeq ($(CONFIG_VIRTIO_SANDBOX),y)
obj-y += virtio.o
obj-$(CONFIG_VIRTIO_RNG) += virtio_device.o
+obj-$(CONFIG_VIRTIO_RNG) += virtio_rng.o
endif
ifeq ($(CONFIG_WDT_GPIO)$(CONFIG_WDT_SANDBOX),yy)
obj-y += wdt.o
diff --git a/test/dm/virtio_rng.c b/test/dm/virtio_rng.c
new file mode 100644
index 0000000000..ff5646b4e1
--- /dev/null
+++ b/test/dm/virtio_rng.c
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2022 Google, Inc.
+ * Written by Andrew Scull <ascull@google.com>
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <virtio_types.h>
+#include <virtio.h>
+#include <virtio_ring.h>
+#include <dm/device-internal.h>
+#include <dm/test.h>
+#include <rng.h>
+#include <test/test.h>
+#include <test/ut.h>
+
+/* This is a brittle means of getting access to the virtqueue */
+struct virtio_rng_priv {
+ struct virtqueue *rng_vq;
+};
+
+/* Test the virtio-rng driver validates the used size */
+static int dm_test_virtio_rng_check_len(struct unit_test_state *uts)
+{
+ struct udevice *bus, *dev;
+ struct virtio_rng_priv *priv;
+ u8 buffer[16];
+
+ /* check probe success */
+ ut_assertok(uclass_first_device(UCLASS_VIRTIO, &bus));
+ ut_assertnonnull(bus);
+
+ /* check the child virtio-rng device is bound */
+ ut_assertok(device_find_first_child(bus, &dev));
+ ut_assertnonnull(dev);
+
+ /* probe the virtio-rng driver */
+ ut_assertok(device_probe(dev));
+
+ /* simulate the device returning the buffer with too much data */
+ priv = dev_get_priv(dev);
+ priv->rng_vq->vring.used->idx = 1;
+ priv->rng_vq->vring.used->ring[0].id = 0;
+ priv->rng_vq->vring.used->ring[0].len = U32_MAX;
+
+ /* check the driver gracefully handles the error */
+ ut_asserteq(-EIO, dm_rng_read(dev, buffer, sizeof(buffer)));
+
+ return 0;
+}
+DM_TEST(dm_test_virtio_rng_check_len, UT_TESTF_SCAN_PDATA | UT_TESTF_SCAN_FDT);
--
2.36.0.550.gb090851708-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread