virtio: Rework queue_dequeue to return a boolean.

It previously returned the cookie directly, which made it impossible
to distinguish between a NULL cookie and the function not having
anything to dequeue. This lead to some code setting a cookie that was
not actually used.

Return the dequeue status as a boolean and provide the cookie with an
optionally handed in pointer instead and adjust all users.

Change-Id: Iaac1726ac4bc7ae42bb96b8f0915852b6def5822
Reviewed-on: https://review.haiku-os.org/c/haiku/+/1814
Reviewed-by: Jérôme Duval <[email protected]>
Reviewed-by: waddlesplash <[email protected]>

headers/private/virtio/virtio.h

@@ -134,7 +134,8 @@ typedef struct {
 
 	uint16 (*queue_size)(virtio_queue queue);
 
-	void* (*queue_dequeue)(virtio_queue queue, uint32* _usedLength);
+	bool (*queue_dequeue)(virtio_queue queue, void** _cookie,
+		uint32* _usedLength);
 
 } virtio_device_interface;
 

src/add-ons/kernel/bus_managers/virtio/VirtioBalloonDevice.cpp

@@ -188,14 +188,13 @@ VirtioBalloonDevice::_Thread()
 
 		// alloc or release
 		TRACE("queue request\n");
-		status_t result = fVirtio->queue_request(queue, &fEntry, NULL, 
+		status_t result = fVirtio->queue_request(queue, &fEntry, NULL, NULL);
-			queue);
 		if (result != B_OK) {
 			ERROR("queueing failed (%s)\n", strerror(result));
 			return result;
 		}
 
-		while (fVirtio->queue_dequeue(queue, NULL) == NULL) {
+		while (!fVirtio->queue_dequeue(queue, NULL, NULL)) {
 			TRACE("wait for response\n");
 			queueConditionEntry.Wait(B_CAN_INTERRUPT);
 		}

src/add-ons/kernel/bus_managers/virtio/VirtioModule.cpp

@@ -184,11 +184,11 @@ virtio_queue_size(virtio_queue _queue)
 }
 
 
-void*
+bool
-virtio_queue_dequeue(virtio_queue _queue, uint32* _usedLength)
+virtio_queue_dequeue(virtio_queue _queue, void** _cookie, uint32* _usedLength)
 {
 	VirtioQueue *queue = (VirtioQueue *)_queue;
-	return queue->Dequeue(_usedLength);
+	return queue->Dequeue(_cookie, _usedLength);
 }
 
 

src/add-ons/kernel/bus_managers/virtio/VirtioPrivate.h

@@ -131,7 +131,8 @@ public:
 			void				EnableInterrupt();
 			void				DisableInterrupt();
 
-			void*				Dequeue(uint32* _usedLength = NULL);
+			bool				Dequeue(void** _cookie = NULL,
+									uint32* _usedLength = NULL);
 
 private:
 			void				UpdateAvailable(uint16 index);

src/add-ons/kernel/bus_managers/virtio/VirtioQueue.cpp

@@ -245,13 +245,13 @@ VirtioQueue::Interrupt()
 }
 
 
-void*
+bool
-VirtioQueue::Dequeue(uint32* _usedLength)
+VirtioQueue::Dequeue(void** _cookie, uint32* _usedLength)
 {
 	TRACE("Dequeue() fRingUsedIndex: %u\n", fRingUsedIndex);
 
 	if (fRingUsedIndex == fRing.used->idx)
-		return NULL;
+		return false;
 
 	uint16 usedIndex = fRingUsedIndex++ & (fRingSize - 1);
 	TRACE("Dequeue() usedIndex: %u\n", usedIndex);
@@ -261,6 +261,9 @@ VirtioQueue::Dequeue(uint32* _usedLength)
 		*_usedLength = element->len;
 
 	void* cookie = fDescriptors[descriptorIndex]->Cookie();
+	if (_cookie != NULL)
+		*_cookie = cookie;
+
 	uint16 size = fDescriptors[descriptorIndex]->Size();
 	if (size == 0)
 		panic("VirtioQueue::Dequeue() size is zero\n");
@@ -283,7 +286,7 @@ VirtioQueue::Dequeue(uint32* _usedLength)
 	fRingHeadIndex = descriptorIndex;
 	TRACE("Dequeue() fRingHeadIndex: %u\n", fRingHeadIndex);
 
-	return cookie;
+	return true;
 }
 
 

src/add-ons/kernel/busses/random/VirtioRNGDevice.cpp

@@ -95,7 +95,7 @@ VirtioRNGDevice::Read(void* _buffer, size_t* _numBytes)
 			fInterruptCondition.Add(&fInterruptConditionEntry);
 		}
 		status_t result = fVirtio->queue_request(fVirtioQueue, NULL, &fEntry,
-			this);
+			NULL);
 		if (result != B_OK) {
 			ERROR("queueing failed (%s)\n", strerror(result));
 			return result;
@@ -131,7 +131,7 @@ VirtioRNGDevice::_RequestCallback(void* driverCookie, void* cookie)
 {
 	VirtioRNGDevice* device = (VirtioRNGDevice*)driverCookie;
 
-	while (device->fVirtio->queue_dequeue(device->fVirtioQueue, NULL) != NULL)
+	while (device->fVirtio->queue_dequeue(device->fVirtioQueue, NULL, NULL))
 		;
 
 	device->_RequestInterrupt();

src/add-ons/kernel/busses/scsi/virtio/VirtioSCSIController.cpp

@@ -275,7 +275,7 @@ VirtioSCSIController::_RequestCallback(void* driverCookie, void* cookie)
 	VirtioSCSIController* controller = (VirtioSCSIController*)driverCookie;
 
 	while (controller->fVirtio->queue_dequeue(
-			controller->fRequestVirtioQueue, NULL) != NULL) {
+			controller->fRequestVirtioQueue, NULL, NULL)) {
 	}
 
 	controller->_RequestInterrupt();
@@ -296,12 +296,9 @@ VirtioSCSIController::_EventCallback(void* driverCookie, void* cookie)
 	CALLED();
 	VirtioSCSIController* controller = (VirtioSCSIController*)driverCookie;
 
-	while (true) {
+	virtio_scsi_event* event = NULL;
-		virtio_scsi_event* event = (virtio_scsi_event*)
+	while (controller->fVirtio->queue_dequeue(controller->fEventVirtioQueue,
-			controller->fVirtio->queue_dequeue(controller->fEventVirtioQueue,
+			(void**)&event, NULL)) {
-				NULL);
-		if (event == NULL)
-			break;
 		controller->_EventInterrupt(event);
 	}
 }

src/add-ons/kernel/drivers/disk/virtual/virtio_block/virtio_block.cpp

@@ -189,7 +189,7 @@ virtio_block_callback(void* driverCookie, void* cookie)
 	virtio_block_driver_info* info = (virtio_block_driver_info*)cookie;
 
 	// consume all queued elements
-	while (info->virtio->queue_dequeue(info->virtio_queue, NULL) != NULL)
+	while (info->virtio->queue_dequeue(info->virtio_queue, NULL, NULL))
 		;
 
 	release_sem_etc(info->sem_cb, 1, B_DO_NOT_RESCHEDULE);

src/add-ons/kernel/drivers/network/virtio/virtio_net.cpp

@@ -171,26 +171,15 @@ get_feature_name(uint32 feature)
 static status_t
 virtio_net_drain_queues(virtio_net_driver_info* info)
 {
-	while (true) {
+	BufInfo* buf = NULL;
-		BufInfo* buf = (BufInfo*)info->virtio->queue_dequeue(
+	while (info->virtio->queue_dequeue(info->txQueues[0], (void**)&buf, NULL))
-			info->txQueues[0], NULL);
-		if (buf == NULL)
-			break;
 		info->txFreeList.Add(buf);
-	}
 
-	while (true) {
+	while (info->virtio->queue_dequeue(info->rxQueues[0], NULL, NULL))
-		BufInfo* buf = (BufInfo*)info->virtio->queue_dequeue(
+		;
-			info->rxQueues[0], NULL);
-		if (buf == NULL)
-			break;
-	}
 
-	while (true) {
+	while (info->rxFullList.RemoveHead() != NULL)
-		BufInfo* buf = info->rxFullList.RemoveHead();
+		;
-		if (buf == NULL)
-			break;
-	}
 
 	return B_OK;
 }
@@ -565,10 +554,11 @@ virtio_net_read(void* cookie, off_t pos, void* buffer, size_t* _length)
 		mutex_lock(&info->rxLock);
 		while (info->rxDone != -1) {
 			uint32 usedLength = 0;
-			BufInfo* buf = (BufInfo*)info->virtio->queue_dequeue(
+			BufInfo* buf = NULL;
-				info->rxQueues[0], &usedLength);
+			if (!info->virtio->queue_dequeue(info->rxQueues[0], (void**)&buf,
-			if (buf == NULL)
+					&usedLength) || buf == NULL) {
 				break;
+			}
 
 			buf->rxUsedLength = usedLength;
 			info->rxFullList.Add(buf);
@@ -622,10 +612,12 @@ virtio_net_write(void* cookie, off_t pos, const void* buffer,
 
 		mutex_lock(&info->txLock);
 		while (info->txDone != -1) {
-			BufInfo* buf = (BufInfo*)info->virtio->queue_dequeue(
+			BufInfo* buf = NULL;
-				info->txQueues[0], NULL);
+			if (!info->virtio->queue_dequeue(info->txQueues[0], (void**)&buf,
-			if (buf == NULL)
+					NULL) || buf == NULL) {
 				break;
+			}
+
 			info->txFreeList.Add(buf);
 		}
 	}