CV-Bowen commented on code in PR #21408:
URL: https://github.com/apache/nuttx/pull/21408#discussion_r4468359958
##########
drivers/rptun/rptun.c:
##########
@@ -308,6 +318,137 @@ static void rptun_free_buf(FAR struct virtio_device
*vdev, FAR void *buf)
mm_free(carveout->heap, buf);
}
+/****************************************************************************
+ * Name: rptun_map_align
+ *
+ * Description:
+ * Return the alignment used for bounce buffers. Use the dcache line size
+ * so a bounce buffer never shares a cache line with a neighbour (avoids
+ * clobbering adjacent data on invalidate). Fall back to MM_ALIGN when the
+ * arch has no dcache, so the value is never zero.
+ *
+ ****************************************************************************/
+
+static inline_function size_t rptun_map_align(void)
+{
+ size_t linesize = up_get_dcache_linesize();
+
+ return linesize > MM_ALIGN ? linesize : MM_ALIGN;
+}
+
+/****************************************************************************
+ * Name: rptun_free_largest
+ *
+ * Description:
+ * Return the largest buffer a single rptun_map_buf call can satisfy.
+ * mm_heapfree_largest gives the largest mm_malloc allocation; subtract the
+ * extra space mm_memalign consumes for alignment so a map of the returned
+ * size is guaranteed to succeed.
+ *
+ ****************************************************************************/
+
+static size_t rptun_free_largest(FAR struct virtio_device *vdev)
+{
+ FAR struct rptun_carveout_s *carveout =
+ (FAR struct rptun_carveout_s *)vdev->mm_priv;
+ size_t align = rptun_map_align();
+ size_t largest = mm_heapfree_largest(carveout->heap);
+
+ /* mm_memalign over-allocates by up to 2 * align to find an aligned point,
+ * and map rounds len up to align. Reserve 3 * align to stay safe.
+ */
+
+ size_t overhead = 3 * align;
+
+ return largest > overhead ? largest - overhead : 0;
+}
+
+/****************************************************************************
+ * Name: rptun_map_buf
+ *
+ * Description:
+ * Map a buffer so the remote side can access it. The remote can only
+ * touch the carveout shared memory, so a buffer outside the carveout is
+ * bounced through a buffer allocated from the carveout heap. Returns NULL
+ * when the carveout heap cannot satisfy the allocation; the caller may
+ * query rptun_free_largest and retry with a smaller length.
+ *
+ ****************************************************************************/
+
+static FAR void *rptun_map_buf(FAR struct virtio_device *vdev,
+ FAR void *buf, size_t len,
+ enum virtio_dma_map dir)
+{
+ FAR struct rptun_carveout_s *carveout =
+ (FAR struct rptun_carveout_s *)vdev->mm_priv;
+ size_t align = rptun_map_align();
+ FAR void *bounce;
+ size_t maplen;
+
+ /* Already inside the carveout heap: the remote can access it directly. */
+
+ if (mm_heapmember(carveout->heap, buf))
+ {
+ return buf;
+ }
+
+ maplen = ALIGN_UP(len, align);
+ bounce = mm_memalign(carveout->heap, align, maplen);
+ if (bounce == NULL)
+ {
+ /* Out of shared memory: the caller decides whether to query
+ * rptun_free_largest and retry with a smaller length.
+ */
+
+ return NULL;
+ }
+
+ if (dir != VIRTIO_DMA_FROM_DEVICE)
Review Comment:
Fixed: rptun_map_buf() now flushes the bounce for every direction and copies
in only for `dir != VIRTIO_DMA_FROM_DEVICE`.
##########
drivers/virtio/virtio-blk.c:
##########
@@ -212,19 +277,79 @@ static ssize_t virtio_blk_rdwr(FAR struct
virtio_blk_priv_s *priv,
virtio_blk_wait_complete(vq, &respsem);
- if (resp.status != VIRTIO_BLK_S_OK)
+ if (up_interrupt_context())
+ {
+ virtqueue_enable_cb_lock(vq, &priv->lock);
+ }
+
+ ret = chunk;
+
+out_resp:
+
+ /* unmap copies the device-written status back into resp; only after that
+ * is resp.status valid (on a bouncing transport resp is a bounce buffer).
+ */
+
+ virtio_unmap_buf(vdev, &resp, sizeof(resp), VIRTIO_DMA_FROM_DEVICE,
+ respmap);
+out_req:
+ virtio_unmap_buf(vdev, &req, sizeof(req), VIRTIO_DMA_TO_DEVICE, reqmap);
+out_data:
+ virtio_unmap_buf(vdev, buf, datalen, datadir, datamap);
+
+ if (ret >= 0 && resp.status != VIRTIO_BLK_S_OK)
{
vrterr("%s Error\n", write ? "Write" : "Read");
ret = -EIO;
}
-err:
- if (up_interrupt_context())
+ return ret;
+}
+
+/****************************************************************************
+ * Name: virtio_blk_rdwr
+ *
+ * Description:
+ * Common function for read and write
+ *
+ ****************************************************************************/
+
+static ssize_t virtio_blk_rdwr(FAR struct virtio_blk_priv_s *priv,
+ FAR void *buffer, blkcnt_t startsector,
+ unsigned int nsectors, bool write)
+{
+ blkcnt_t sector = startsector;
+ FAR uint8_t *buf = buffer;
+ ssize_t ret = 0;
+
+ /* The data payload may exceed the shared memory a bounce transport can
+ * map at once, so transfer it in chunks. On MMIO/PCI virtio_free_largest
+ * returns SIZE_MAX and map is a no-op, so this runs as a single pass.
+ */
+
+ while (nsectors > 0)
{
- virtqueue_enable_cb_lock(vq, &priv->lock);
+ ret = virtio_blk_rdwr_one(priv, buf, sector, nsectors, write);
+ if (ret < 0)
+ {
+ break;
+ }
+
+ buf += ret * priv->block_size;
+ sector += ret;
+ nsectors -= ret;
+ }
+
+ /* Report the number of sectors transferred, or the failure code when
+ * nothing completed.
+ */
+
+ if (sector > startsector)
+ {
+ ret = sector - startsector;
Review Comment:
Fixed: virtio_blk_rdwr() returns the error of a failed chunk even after
earlier chunks completed.
##########
drivers/virtio/virtio-rng.c:
##########
@@ -160,23 +161,34 @@ static ssize_t virtio_rng_read(FAR struct file *filep,
FAR char *buffer,
cookie.len = 0;
nxsem_init(&cookie.sem, 0, 0);
+ /* Map the user buffer so the device side can access it. On MMIO/PCI the
+ * peer reaches arbitrary memory, so map returns the buffer itself and the
+ * device writes the entropy straight into it (zero copy). On rptun the
+ * buffer is bounced through shared memory and unmap copies the result
+ * back below.
+ */
+
+ ret = virtio_map_buf(priv->vdev, buffer, buflen, VIRTIO_DMA_FROM_DEVICE,
Review Comment:
Fixed in the transports rather than in virtio-rng: with CONFIG_ARCH_ADDRENV,
virtio-mmio and virtio-pci now map through virtio_addrenv_map(), which bounces
a user address through the kernel heap and passes a kernel address through.
This also covers virtio-blk, which hands bchlib's user buffer to the device the
same way. Tested on rv-virt:knetnsh64 (rng into .bss and heap buffers, blk
write/read compare on /dev/virtblk0).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
