hwrng core uses two buffers that can be mixed in the virtio-rng queue.
This series fixes the problem by adding an internal buffer in virtio-rng.
Once the internal buffer is added, we can fix two other problems:
- to be able to release the driver without waiting the device releases the
buffer
- actually returns some data when wait=0 as we can have some already
available data
It also tries to improve the performance by always having a buffer in
the queue of the device.
Laurent Vivier (4):
hwrng: virtio - add an internal buffer
hwrng: virtio - don't wait on cleanup
hwrng: virtio - don't waste entropy
hwrng: virtio - always add a pending request
drivers/char/hw_random/virtio-rng.c | 84 +++++++++++++++++++++--------
1 file changed, 63 insertions(+), 21 deletions(-)
--
2.31.1
When virtio-rng device was dropped by the hwrng core we were forced
to wait the buffer to come back from the device to not have
remaining ongoing operation that could spoil the buffer.
But now, as the buffer is internal to the virtio-rng we can release
the waiting loop immediately, the buffer will be retrieve and use
when the virtio-rng driver will be selected again.
This avoids to hang on an rng_current write command if the virtio-rng
device is blocked by a lack of entropy. This allows to select
another entropy source if the current one is empty.
Signed-off-by: Laurent Vivier <[email protected]>
---
drivers/char/hw_random/virtio-rng.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/char/hw_random/virtio-rng.c b/drivers/char/hw_random/virtio-rng.c
index 208c547dcac1..173aeea835bb 100644
--- a/drivers/char/hw_random/virtio-rng.c
+++ b/drivers/char/hw_random/virtio-rng.c
@@ -82,6 +82,11 @@ static int virtio_read(struct hwrng *rng, void *buf, size_t size, bool wait)
ret = wait_for_completion_killable(&vi->have_data);
if (ret < 0)
return ret;
+ /* if vi->data_avail is 0, we have been interrupted
+ * by a cleanup, but buffer stays in the queue
+ */
+ if (vi->data_avail == 0)
+ return read;
chunk = min_t(unsigned int, size, vi->data_avail);
memcpy(buf + read, vi->data, chunk);
@@ -105,7 +110,7 @@ static void virtio_cleanup(struct hwrng *rng)
struct virtrng_info *vi = (struct virtrng_info *)rng->priv;
if (vi->busy)
- wait_for_completion(&vi->have_data);
+ complete(&vi->have_data);
}
static int probe_common(struct virtio_device *vdev)
--
2.31.1
If we ensure we have already some data available by enqueuing
again the buffer once data are exhausted, we can return what we
have without waiting for the device answer.
Signed-off-by: Laurent Vivier <[email protected]>
---
drivers/char/hw_random/virtio-rng.c | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/drivers/char/hw_random/virtio-rng.c b/drivers/char/hw_random/virtio-rng.c
index 8ba97cf4ca8f..0b3c9b643495 100644
--- a/drivers/char/hw_random/virtio-rng.c
+++ b/drivers/char/hw_random/virtio-rng.c
@@ -20,7 +20,6 @@ struct virtrng_info {
struct virtqueue *vq;
char name[25];
int index;
- bool busy;
bool hwrng_register_done;
bool hwrng_removed;
/* data transfer */
@@ -44,16 +43,16 @@ static void random_recv_done(struct virtqueue *vq)
return;
vi->data_idx = 0;
- vi->busy = false;
complete(&vi->have_data);
}
-/* The host will fill any buffer we give it with sweet, sweet randomness. */
-static void register_buffer(struct virtrng_info *vi)
+static void request_entropy(struct virtrng_info *vi)
{
struct scatterlist sg;
+ reinit_completion(&vi->have_data);
+
sg_init_one(&sg, vi->data, sizeof(vi->data));
/* There should always be room for one buffer. */
@@ -69,6 +68,8 @@ static unsigned int copy_data(struct virtrng_info *vi, void *buf,
memcpy(buf, vi->data + vi->data_idx, size);
vi->data_idx += size;
vi->data_avail -= size;
+ if (vi->data_avail == 0)
+ request_entropy(vi);
return size;
}
@@ -98,13 +99,7 @@ static int virtio_read(struct hwrng *rng, void *buf, size_t size, bool wait)
* so either size is 0 or data_avail is 0
*/
while (size != 0) {
- /* data_avail is 0 */
- if (!vi->busy) {
- /* no pending request, ask for more */
- vi->busy = true;
- reinit_completion(&vi->have_data);
- register_buffer(vi);
- }
+ /* data_avail is 0 but a request is pending */
ret = wait_for_completion_killable(&vi->have_data);
if (ret < 0)
return ret;
@@ -126,8 +121,7 @@ static void virtio_cleanup(struct hwrng *rng)
{
struct virtrng_info *vi = (struct virtrng_info *)rng->priv;
- if (vi->busy)
- complete(&vi->have_data);
+ complete(&vi->have_data);
}
static int probe_common(struct virtio_device *vdev)
@@ -163,6 +157,9 @@ static int probe_common(struct virtio_device *vdev)
goto err_find;
}
+ /* we always have a pending entropy request */
+ request_entropy(vi);
+
return 0;
err_find:
@@ -181,7 +178,6 @@ static void remove_common(struct virtio_device *vdev)
vi->data_idx = 0;
complete(&vi->have_data);
vdev->config->reset(vdev);
- vi->busy = false;
if (vi->hwrng_register_done)
hwrng_unregister(&vi->hwrng);
vdev->config->del_vqs(vdev);
--
2.31.1
On Wed, Sep 22, 2021 at 7:09 PM Laurent Vivier <[email protected]> wrote:
>
> hwrng core uses two buffers that can be mixed in the virtio-rng queue.
>
> This series fixes the problem by adding an internal buffer in virtio-rng.
>
> Once the internal buffer is added, we can fix two other problems:
>
> - to be able to release the driver without waiting the device releases the
> buffer
>
> - actually returns some data when wait=0 as we can have some already
> available data
>
> It also tries to improve the performance by always having a buffer in
> the queue of the device.
Tested-by: Alexander Potapenko <[email protected]>
for the series
With these four patches applied, KMSAN stops reporting boot-time
errors in _mix_pool_bytes reported here:
https://www.spinics.net/lists/linux-virtualization/msg46310.html
> Laurent Vivier (4):
> hwrng: virtio - add an internal buffer
> hwrng: virtio - don't wait on cleanup
> hwrng: virtio - don't waste entropy
> hwrng: virtio - always add a pending request
>
> drivers/char/hw_random/virtio-rng.c | 84 +++++++++++++++++++++--------
> 1 file changed, 63 insertions(+), 21 deletions(-)
>
> --
> 2.31.1
>
>
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
On 22/09/2021 19:08, Laurent Vivier wrote:
> hwrng core uses two buffers that can be mixed in the virtio-rng queue.
>
> This series fixes the problem by adding an internal buffer in virtio-rng.
>
> Once the internal buffer is added, we can fix two other problems:
>
> - to be able to release the driver without waiting the device releases the
> buffer
>
> - actually returns some data when wait=0 as we can have some already
> available data
>
> It also tries to improve the performance by always having a buffer in
> the queue of the device.
>
> Laurent Vivier (4):
> hwrng: virtio - add an internal buffer
> hwrng: virtio - don't wait on cleanup
> hwrng: virtio - don't waste entropy
> hwrng: virtio - always add a pending request
>
> drivers/char/hw_random/virtio-rng.c | 84 +++++++++++++++++++++--------
> 1 file changed, 63 insertions(+), 21 deletions(-)
>
Any comment?
Do we need a v2?
Thanks,
Laurent