From b90dc7a4f3bc54e560dc944af0d4c62e849c52f7 Mon Sep 17 00:00:00 2001 From: Michael Lotz Date: Tue, 3 Sep 2024 21:35:25 +0200 Subject: [PATCH] virtio: Explicitly request queue sizes where needed. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ensure that allocated queues can hold the amount of descriptors that were previously communicated to DMAResources in virtio_block and virtio_scsi. The queue allocations will now fail with B_BUFFER_OVERFLOW if the requested size cannot be provided. When requestedSizes are set to 0, no requirement is placed and the queue is sized to its advertised maximum. The requestedSizes argument can be NULL which implies all 0. Change-Id: Ifb1e032d48f8c07aedfe2bf941f32783842c8c12 Reviewed-on: https://review.haiku-os.org/c/haiku/+/8220 Reviewed-by: Jérôme Duval --- headers/private/virtio/virtio.h | 2 +- .../virtio/VirtioBalloonDevice.cpp | 2 +- .../bus_managers/virtio/VirtioDevice.cpp | 26 ++++++++++++++----- .../bus_managers/virtio/VirtioModule.cpp | 5 ++-- .../bus_managers/virtio/VirtioPrivate.h | 3 ++- .../busses/random/virtio/VirtioRNGDevice.cpp | 2 +- .../scsi/virtio/VirtioSCSIController.cpp | 7 ++++- .../virtio/virtio_mmio/VirtioDevice.cpp | 5 +++- .../busses/virtio/virtio_mmio/VirtioDevice.h | 2 +- .../busses/virtio/virtio_mmio/virtio_mmio.cpp | 5 ++-- .../virtual/virtio_block/virtio_block.cpp | 8 +++++- .../drivers/graphics/virtio/virtio_gpu.cpp | 2 +- .../input/virtio_input/virtio_input.cpp | 2 +- .../network/ether/virtio/virtio_net.cpp | 2 +- 14 files changed, 52 insertions(+), 21 deletions(-) diff --git a/headers/private/virtio/virtio.h b/headers/private/virtio/virtio.h index 4d665edc27..29a13a0af5 100644 --- a/headers/private/virtio/virtio.h +++ b/headers/private/virtio/virtio.h @@ -117,7 +117,7 @@ typedef struct { const void* buffer, size_t bufferSize); status_t (*alloc_queues)(virtio_device cookie, size_t count, - virtio_queue* queues); + virtio_queue* queues, uint16* requestedSizes); void (*free_queues)(virtio_device cookie); diff --git a/src/add-ons/kernel/bus_managers/virtio/VirtioBalloonDevice.cpp b/src/add-ons/kernel/bus_managers/virtio/VirtioBalloonDevice.cpp index d041e37298..f5de01e49e 100644 --- a/src/add-ons/kernel/bus_managers/virtio/VirtioBalloonDevice.cpp +++ b/src/add-ons/kernel/bus_managers/virtio/VirtioBalloonDevice.cpp @@ -58,7 +58,7 @@ VirtioBalloonDevice::VirtioBalloonDevice(device_node* node) fVirtio->negotiate_features(fVirtioDevice, 0, &fFeatures, &get_feature_name); - fStatus = fVirtio->alloc_queues(fVirtioDevice, 2, fVirtioQueues); + fStatus = fVirtio->alloc_queues(fVirtioDevice, 2, fVirtioQueues, NULL); if (fStatus != B_OK) { ERROR("queue allocation failed (%s)\n", strerror(fStatus)); return; diff --git a/src/add-ons/kernel/bus_managers/virtio/VirtioDevice.cpp b/src/add-ons/kernel/bus_managers/virtio/VirtioDevice.cpp index fad7f01ce4..4ef2b07d94 100644 --- a/src/add-ons/kernel/bus_managers/virtio/VirtioDevice.cpp +++ b/src/add-ons/kernel/bus_managers/virtio/VirtioDevice.cpp @@ -179,7 +179,8 @@ VirtioDevice::WriteDeviceConfig(uint8 offset, const void* buffer, status_t -VirtioDevice::AllocateQueues(size_t count, virtio_queue *queues) +VirtioDevice::AllocateQueues(size_t count, virtio_queue *queues, + uint16 *requestedSizes) { if (count > VIRTIO_VIRTQUEUES_MAX_COUNT || queues == NULL) return B_BAD_VALUE; @@ -192,11 +193,24 @@ VirtioDevice::AllocateQueues(size_t count, virtio_queue *queues) fQueueCount = count; for (size_t index = 0; index < count; index++) { uint16 size = fController->get_queue_ring_size(fCookie, index); - fQueues[index] = new(std::nothrow) VirtioQueue(this, index, size); - queues[index] = fQueues[index]; - status = B_NO_MEMORY; - if (fQueues[index] != NULL) - status = fQueues[index]->InitCheck(); + + uint16 requestedSize + = requestedSizes != NULL ? requestedSizes[index] : 0; + if (requestedSize != 0) { + if (requestedSize > size) + status = B_BUFFER_OVERFLOW; + else + size = requestedSize; + } + + if (status == B_OK) { + fQueues[index] = new(std::nothrow) VirtioQueue(this, index, size); + queues[index] = fQueues[index]; + status = B_NO_MEMORY; + if (fQueues[index] != NULL) + status = fQueues[index]->InitCheck(); + } + if (status != B_OK) { _DestroyQueues(index + 1); return status; diff --git a/src/add-ons/kernel/bus_managers/virtio/VirtioModule.cpp b/src/add-ons/kernel/bus_managers/virtio/VirtioModule.cpp index efd7e8f7b4..310e85ea71 100644 --- a/src/add-ons/kernel/bus_managers/virtio/VirtioModule.cpp +++ b/src/add-ons/kernel/bus_managers/virtio/VirtioModule.cpp @@ -97,11 +97,12 @@ virtio_write_device_config(void* _device, uint8 offset, status_t -virtio_alloc_queues(virtio_device _device, size_t count, virtio_queue *queues) +virtio_alloc_queues(virtio_device _device, size_t count, virtio_queue *queues, + uint16 *requestedSizes) { CALLED(); VirtioDevice *device = (VirtioDevice *)_device; - return device->AllocateQueues(count, queues); + return device->AllocateQueues(count, queues, requestedSizes); } diff --git a/src/add-ons/kernel/bus_managers/virtio/VirtioPrivate.h b/src/add-ons/kernel/bus_managers/virtio/VirtioPrivate.h index 0d08f9ebca..1e8327518d 100644 --- a/src/add-ons/kernel/bus_managers/virtio/VirtioPrivate.h +++ b/src/add-ons/kernel/bus_managers/virtio/VirtioPrivate.h @@ -56,7 +56,8 @@ public: const void* buffer, size_t bufferSize); status_t AllocateQueues(size_t count, - virtio_queue *queues); + virtio_queue *queues, + uint16 *requestedSizes); void FreeQueues(); status_t SetupInterrupt(virtio_intr_func config_handler, void *driverCookie); diff --git a/src/add-ons/kernel/busses/random/virtio/VirtioRNGDevice.cpp b/src/add-ons/kernel/busses/random/virtio/VirtioRNGDevice.cpp index 03a093a00a..0d7910b02a 100644 --- a/src/add-ons/kernel/busses/random/virtio/VirtioRNGDevice.cpp +++ b/src/add-ons/kernel/busses/random/virtio/VirtioRNGDevice.cpp @@ -45,7 +45,7 @@ VirtioRNGDevice::VirtioRNGDevice(device_node *node) fVirtio->negotiate_features(fVirtioDevice, 0, &fFeatures, &get_feature_name); - fStatus = fVirtio->alloc_queues(fVirtioDevice, 1, &fVirtioQueue); + fStatus = fVirtio->alloc_queues(fVirtioDevice, 1, &fVirtioQueue, NULL); if (fStatus != B_OK) { ERROR("queue allocation failed (%s)\n", strerror(fStatus)); return; diff --git a/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIController.cpp b/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIController.cpp index d2350f117d..d854ed8860 100644 --- a/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIController.cpp +++ b/src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIController.cpp @@ -82,7 +82,12 @@ VirtioSCSIController::VirtioSCSIController(device_node *node) } ::virtio_queue virtioQueues[3]; - fStatus = fVirtio->alloc_queues(fVirtioDevice, 3, virtioQueues); + uint16 requestedSizes[3] = { 0, 0, 0 }; + requestedSizes[2] = fConfig.seg_max + 2; + // two entries are taken up by the header and result + + fStatus = fVirtio->alloc_queues(fVirtioDevice, 3, virtioQueues, + requestedSizes); if (fStatus != B_OK) { ERROR("queue allocation failed (%s)\n", strerror(fStatus)); return; diff --git a/src/add-ons/kernel/busses/virtio/virtio_mmio/VirtioDevice.cpp b/src/add-ons/kernel/busses/virtio/virtio_mmio/VirtioDevice.cpp index be9c290a23..f6527a1eff 100644 --- a/src/add-ons/kernel/busses/virtio/virtio_mmio/VirtioDevice.cpp +++ b/src/add-ons/kernel/busses/virtio/virtio_mmio/VirtioDevice.cpp @@ -43,11 +43,14 @@ VirtioQueue::~VirtioQueue() status_t -VirtioQueue::Init() +VirtioQueue::Init(uint16 requestedSize) { fDev->fRegs->queueSel = fId; TRACE("queueNumMax: %d\n", fDev->fRegs->queueNumMax); fQueueLen = fDev->fRegs->queueNumMax; + if (requestedSize != 0 && requestedSize > fQueueLen) + return B_BUFFER_OVERFLOW; + fDescCount = fQueueLen; fDev->fRegs->queueNum = fQueueLen; fLastUsed = 0; diff --git a/src/add-ons/kernel/busses/virtio/virtio_mmio/VirtioDevice.h b/src/add-ons/kernel/busses/virtio/virtio_mmio/VirtioDevice.h index 9edc05c7fc..22c896fd6a 100644 --- a/src/add-ons/kernel/busses/virtio/virtio_mmio/VirtioDevice.h +++ b/src/add-ons/kernel/busses/virtio/virtio_mmio/VirtioDevice.h @@ -48,7 +48,7 @@ struct VirtioQueue { VirtioQueue(VirtioDevice *dev, int32 id); ~VirtioQueue(); - status_t Init(); + status_t Init(uint16 requestedSize); int32 AllocDesc(); void FreeDesc(int32 idx); diff --git a/src/add-ons/kernel/busses/virtio/virtio_mmio/virtio_mmio.cpp b/src/add-ons/kernel/busses/virtio/virtio_mmio/virtio_mmio.cpp index 10d0c5b74d..5c86759d89 100644 --- a/src/add-ons/kernel/busses/virtio/virtio_mmio/virtio_mmio.cpp +++ b/src/add-ons/kernel/busses/virtio/virtio_mmio/virtio_mmio.cpp @@ -466,7 +466,7 @@ virtio_device_write_device_config(virtio_device cookie, uint8 offset, static status_t virtio_device_alloc_queues(virtio_device cookie, size_t count, - virtio_queue* queues) + virtio_queue* queues, uint16* requestedSizes) { TRACE("virtio_device_alloc_queues(%p, %" B_PRIuSIZE ")\n", cookie, count); VirtioDevice* dev = (VirtioDevice*)cookie; @@ -483,7 +483,8 @@ virtio_device_alloc_queues(virtio_device cookie, size_t count, if (!newQueues[i].IsSet()) return B_NO_MEMORY; - status_t res = newQueues[i]->Init(); + status_t res = newQueues[i]->Init( + requestedSizes != NULL ? requestedSizes[i] : 0); if (res < B_OK) return res; } diff --git a/src/add-ons/kernel/drivers/disk/virtual/virtio_block/virtio_block.cpp b/src/add-ons/kernel/drivers/disk/virtual/virtio_block/virtio_block.cpp index 7fdf904553..65ff2ead00 100644 --- a/src/add-ons/kernel/drivers/disk/virtual/virtio_block/virtio_block.cpp +++ b/src/add-ons/kernel/drivers/disk/virtual/virtio_block/virtio_block.cpp @@ -286,12 +286,18 @@ virtio_block_init_device(void* _info, void** _cookie) TRACE("virtio_block: capacity: %" B_PRIu64 ", block_size %" B_PRIu32 "\n", info->capacity, info->block_size); + uint16 requestedSize = 0; + if ((info->features & VIRTIO_BLK_F_SEG_MAX) != 0) + requestedSize = info->config.seg_max + 2; + // two entries are taken up by the header and result + status = info->virtio->alloc_queues(info->virtio_device, 1, - &info->virtio_queue); + &info->virtio_queue, &requestedSize); if (status != B_OK) { ERROR("queue allocation failed (%s)\n", strerror(status)); return status; } + status = info->virtio->setup_interrupt(info->virtio_device, virtio_block_config_callback, info); diff --git a/src/add-ons/kernel/drivers/graphics/virtio/virtio_gpu.cpp b/src/add-ons/kernel/drivers/graphics/virtio/virtio_gpu.cpp index 37c50ac1fb..51345933eb 100644 --- a/src/add-ons/kernel/drivers/graphics/virtio/virtio_gpu.cpp +++ b/src/add-ons/kernel/drivers/graphics/virtio/virtio_gpu.cpp @@ -485,7 +485,7 @@ virtio_gpu_init_device(void* _info, void** _cookie) // Setup queues ::virtio_queue virtioQueues[2]; status_t status = info->virtio->alloc_queues(info->virtio_device, 2, - virtioQueues); + virtioQueues, NULL); if (status != B_OK) { ERROR("queue allocation failed (%s)\n", strerror(status)); return status; diff --git a/src/add-ons/kernel/drivers/input/virtio_input/virtio_input.cpp b/src/add-ons/kernel/drivers/input/virtio_input/virtio_input.cpp index 0d9d1d4aff..635bca5009 100644 --- a/src/add-ons/kernel/drivers/input/virtio_input/virtio_input.cpp +++ b/src/add-ons/kernel/drivers/input/virtio_input/virtio_input.cpp @@ -283,7 +283,7 @@ virtio_input_init_device(void* _info, void** _cookie) InitPackets(info, 8); status = info->virtio->alloc_queues(info->virtio_device, 1, - &info->virtio_queue); + &info->virtio_queue, NULL); if (status != B_OK) { ERROR("queue allocation failed (%s)\n", strerror(status)); return status; diff --git a/src/add-ons/kernel/drivers/network/ether/virtio/virtio_net.cpp b/src/add-ons/kernel/drivers/network/ether/virtio/virtio_net.cpp index cb34094b24..71ed5ff5ca 100644 --- a/src/add-ons/kernel/drivers/network/ether/virtio/virtio_net.cpp +++ b/src/add-ons/kernel/drivers/network/ether/virtio/virtio_net.cpp @@ -321,7 +321,7 @@ virtio_net_init_device(void* _info, void** _cookie) queueCount++; ::virtio_queue virtioQueues[queueCount]; status_t status = info->virtio->alloc_queues(info->virtio_device, queueCount, - virtioQueues); + virtioQueues, NULL); if (status != B_OK) { ERROR("queue allocation failed (%s)\n", strerror(status)); return status;