From 480c96550c39bf72fcaa181bfdead5ceab7fc6fc Mon Sep 17 00:00:00 2001 From: John Davis Date: Fri, 6 Mar 2026 20:30:31 -0600 Subject: [PATCH] hyperv: Refactor read packet function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently, a packet being read is split into header and data buffers. Change to a single buffer to simplify logic in drivers, especially for those that receive packets with variable sized headers. This change also implements the pending send size functionality for the receive buffer. Change-Id: Ic4a3698fcfe813fbcf4fd9c1941102b5adaeb3b2 Reviewed-on: https://review.haiku-os.org/c/haiku/+/10418 Tested-by: Commit checker robot Reviewed-by: Jérôme Duval --- headers/private/hyperv/hyperv.h | 4 +- .../bus_managers/hyperv/VMBusDevice.cpp | 129 ++++++++---------- .../bus_managers/hyperv/VMBusDeviceModule.cpp | 6 +- .../bus_managers/hyperv/VMBusDevicePrivate.h | 5 +- .../drivers/input/hyperv_hid/HIDDevice.cpp | 14 +- .../drivers/input/hyperv_hid/HIDDevice.h | 2 +- 6 files changed, 75 insertions(+), 85 deletions(-) diff --git a/headers/private/hyperv/hyperv.h b/headers/private/hyperv/hyperv.h index 14610c88dc..4f223e58c4 100644 --- a/headers/private/hyperv/hyperv.h +++ b/headers/private/hyperv/hyperv.h @@ -53,8 +53,8 @@ typedef struct hyperv_device_interface { status_t (*open)(hyperv_device cookie, uint32 txLength, uint32 rxLength, hyperv_device_callback callback, void* callbackData); void (*close)(hyperv_device cookie); - status_t (*read_packet)(hyperv_device cookie, vmbus_pkt_header* _header, - uint32* _headerLength, void* _buffer, uint32* _length); + status_t (*read_packet)(hyperv_device cookie, void* buffer, uint32* bufferLength, + uint32* _headerLength, uint32* _dataLength); status_t (*write_packet)(hyperv_device cookie, uint16 type, const void* buffer, uint32 length, bool responseRequired, uint64 transactionID); status_t (*write_gpa_packet)(hyperv_device cookie, uint32 rangeCount, diff --git a/src/add-ons/kernel/bus_managers/hyperv/VMBusDevice.cpp b/src/add-ons/kernel/bus_managers/hyperv/VMBusDevice.cpp index 78935118f3..355885d05d 100644 --- a/src/add-ons/kernel/bus_managers/hyperv/VMBusDevice.cpp +++ b/src/add-ons/kernel/bus_managers/hyperv/VMBusDevice.cpp @@ -211,90 +211,81 @@ VMBusDevice::WriteGPAPacket(uint32 rangeCount, const vmbus_gpa_range* rangesList status_t -VMBusDevice::PeekPacket(void* _buffer, uint32 length) +VMBusDevice::ReadPacket(void* buffer, uint32* bufferLength, uint32* _headerLength, + uint32* _dataLength) { - InterruptsSpinLocker locker(fRXLock); - - // Ensure at least the requested amount of data is present, plus the shifted packet index - if (_AvailableRX() < length + sizeof(uint64)) - return B_DEV_NOT_READY; - - uint32 readIndex = atomic_get((int32*)&fRXRing->read_index); - TRACE_TX("Channel %u RX peek read idx 0x%X write idx 0x%X\n", fChannelID, readIndex, - atomic_get((int32*)&fRXRing->write_index)); - - _ReadRX(readIndex, _buffer, length); - return B_OK; -} - - -status_t -VMBusDevice::ReadPacket(vmbus_pkt_header* _header, uint32* _headerLength, void* _buffer, - uint32* _length) -{ - vmbus_pkt_header header; - vmbus_pkt_header* headerPtr; - if (_header != NULL) { - if (_headerLength == NULL || *_headerLength < sizeof(vmbus_pkt_header)) - return B_BAD_VALUE; - headerPtr = _header; - } else { - headerPtr = &header; - } - - status_t status = PeekPacket(headerPtr, sizeof(vmbus_pkt_header)); - if (status != B_OK) - return status; - - uint32 headerLength = headerPtr->header_length << VMBUS_PKT_SIZE_SHIFT; - uint32 totalLength = headerPtr->total_length << VMBUS_PKT_SIZE_SHIFT; - if (headerLength < sizeof(vmbus_pkt_header) || totalLength < headerLength) { - ERROR("Channel %u RX invalid pkt hdr len 0x%X tot len 0x%X\n", fChannelID, headerLength, - totalLength); - return B_IO_ERROR; - } - uint32 dataLength = totalLength - headerLength; - - TRACE_RX("Channel %u RX pkt %u hdr len 0x%X tot len 0x%X\n", fChannelID, headerPtr->type, - headerLength, totalLength); - - // Ensure provided buffers are large enough - if (_header != NULL) { - if (*_headerLength < headerLength) { - *_headerLength = headerLength; - return B_NO_MEMORY; - } - *_headerLength = headerLength; - } - - if (*_length < dataLength) { - *_length = dataLength; - return B_NO_MEMORY; - } - *_length = dataLength; + if (*bufferLength < sizeof(vmbus_pkt_header)) + return B_BAD_VALUE; InterruptsSpinLocker locker(fRXLock); - if (_AvailableRX() < totalLength + sizeof(uint64)) + // Should have at least the standard header and the shifted read index present on the ring + if (_AvailableRX() < sizeof(vmbus_pkt_header) + sizeof(uint64)) return B_DEV_NOT_READY; uint32 readIndexNew = atomic_get((int32*)&fRXRing->read_index); - TRACE_TX("Channel %u RX old read idx 0x%X write idx 0x%X\n", fChannelID, readIndexNew, + TRACE_RX("Channel %u RX old read idx 0x%X write idx 0x%X\n", fChannelID, readIndexNew, atomic_get((int32*)&fRXRing->write_index)); - // Read the header, data, and seek past the shifted read index - if (_header != NULL && headerLength > sizeof(vmbus_pkt_header)) - readIndexNew = _ReadRX(readIndexNew, _header, headerLength); - else - readIndexNew = _SeekRX(readIndexNew, headerLength); - readIndexNew = _ReadRX(readIndexNew, _buffer, dataLength); + // Read in the standard header and determine the length of the remainder of the data + vmbus_pkt_header* header = reinterpret_cast(buffer); + readIndexNew = _ReadRX(readIndexNew, header, sizeof(*header)); + + uint32 headerLength = header->header_length << VMBUS_PKT_SIZE_SHIFT; + uint32 totalLength = header->total_length << VMBUS_PKT_SIZE_SHIFT; + if (headerLength < sizeof(*header) || totalLength < headerLength) { + ERROR("Channel %u RX invalid pkt hdr len 0x%X tot len 0x%X\n", fChannelID, headerLength, + totalLength); + return B_BAD_DATA; + } + void* dataBuffer = reinterpret_cast(buffer) + headerLength; + uint32 dataLength = totalLength - headerLength; + + TRACE_RX("Channel %u RX pkt %u hdr len 0x%X data len 0x%X tran %" B_PRIu64 "\n", fChannelID, + header->type, headerLength, dataLength, header->transaction_id); + + // Ensure provided buffer is large enough + if (*bufferLength < totalLength) { + *bufferLength = totalLength; + return B_NO_MEMORY; + } + *bufferLength = totalLength; + + uint32 readLength = totalLength + sizeof(uint64); + if (_AvailableRX() < readLength) + return B_DEV_NOT_READY; + + // Standard header was already read above; read remainder of header and data + // Shifted index is discarded + readIndexNew = _ReadRX(readIndexNew, header + 1, headerLength - sizeof(*header)); + readIndexNew = _ReadRX(readIndexNew, dataBuffer, dataLength); readIndexNew = _SeekRX(readIndexNew, sizeof(uint64)); memory_write_barrier(); atomic_set((int32*)&fRXRing->read_index, (int32)readIndexNew); - TRACE_TX("Channel %u RX new read idx 0x%X write idx 0x%X\n", fChannelID, + TRACE_RX("Channel %u RX new read idx 0x%X write idx 0x%X\n", fChannelID, atomic_get((int32*)&fRXRing->read_index), atomic_get((int32*)&fRXRing->write_index)); + locker.Unlock(); + + *_headerLength = headerLength; + *_dataLength = dataLength; + + // Signal Hyper-V if required; signaling is only needed if the RX ring buffer was previously + // completely full, and there is now enough space to write pending data (if supported) + memory_read_barrier(); + if (fRXRing->features.pending_send_size_supported) { + uint32 pendingSendLength = atomic_get((int32*)&fRXRing->pending_send_size); + if (pendingSendLength > 0) { + uint32 availableLength = _AvailableRX(); + if ((availableLength - readLength) < pendingSendLength + && availableLength > pendingSendLength) { + atomic_add64((int64*)&fRXRing->guest_to_host_interrupt_count, 1); + fVMBus->signal_channel(fVMBusCookie, fChannelID); + } + } + } + return B_OK; } diff --git a/src/add-ons/kernel/bus_managers/hyperv/VMBusDeviceModule.cpp b/src/add-ons/kernel/bus_managers/hyperv/VMBusDeviceModule.cpp index 090dae0d4e..509f4e95da 100644 --- a/src/add-ons/kernel/bus_managers/hyperv/VMBusDeviceModule.cpp +++ b/src/add-ons/kernel/bus_managers/hyperv/VMBusDeviceModule.cpp @@ -76,11 +76,11 @@ vmbus_device_close(hyperv_device cookie) static status_t -vmbus_device_read_packet(hyperv_device cookie, vmbus_pkt_header* _header, uint32* _headerLength, - void* _buffer, uint32* _length) +vmbus_device_read_packet(hyperv_device cookie, void* buffer, uint32* bufferLength, + uint32* _headerLength, uint32* _dataLength) { VMBusDevice* device = reinterpret_cast(cookie); - return device->ReadPacket(_header, _headerLength, _buffer, _length); + return device->ReadPacket(buffer, bufferLength, _headerLength, _dataLength); } diff --git a/src/add-ons/kernel/bus_managers/hyperv/VMBusDevicePrivate.h b/src/add-ons/kernel/bus_managers/hyperv/VMBusDevicePrivate.h index a15835a56e..a6f50b50d1 100644 --- a/src/add-ons/kernel/bus_managers/hyperv/VMBusDevicePrivate.h +++ b/src/add-ons/kernel/bus_managers/hyperv/VMBusDevicePrivate.h @@ -57,9 +57,8 @@ public: const vmbus_gpa_range* rangesList, uint32 rangesLength, const void* buffer, uint32 length, bool responseRequired, uint64 transactionID); - status_t PeekPacket(void* _buffer, uint32 length); - status_t ReadPacket(vmbus_pkt_header* _header, uint32* _headerLength, - void* _buffer, uint32* _length); + status_t ReadPacket(void* buffer, uint32* bufferLength, + uint32* _headerLength, uint32* _dataLength); status_t AllocateGPADL(uint32 length, void** _buffer, uint32* _gpadl); status_t FreeGPADL(uint32 gpadl); diff --git a/src/add-ons/kernel/drivers/input/hyperv_hid/HIDDevice.cpp b/src/add-ons/kernel/drivers/input/hyperv_hid/HIDDevice.cpp index 6e37f472d9..4af5c1daa9 100644 --- a/src/add-ons/kernel/drivers/input/hyperv_hid/HIDDevice.cpp +++ b/src/add-ons/kernel/drivers/input/hyperv_hid/HIDDevice.cpp @@ -36,7 +36,7 @@ HIDDevice::HIDDevice(hyperv_device_interface* hyperv, fProtocolRespEvent.Init(this, "hyper-v hid protoresp"); fDeviceInfoEvent.Init(this, "hyper-v hid devinfo"); - fPacket = malloc(HV_HID_RX_PKT_BUFFER_SIZE); + fPacket = static_cast(malloc(HV_HID_RX_PKT_BUFFER_SIZE)); if (fPacket == NULL) { fStatus = B_NO_MEMORY; return; @@ -137,12 +137,12 @@ void HIDDevice::_Callback() { while (true) { - vmbus_pkt_header header; - uint32 headerLength = sizeof(header); - uint32 packetLength = HV_HID_RX_PKT_BUFFER_SIZE; + uint32 length = HV_HID_RX_PKT_BUFFER_SIZE; + uint32 headerLength; + uint32 messageLength; - status_t status = fHyperV->read_packet(fHyperVCookie, &header, &headerLength, - fPacket, &packetLength); + status_t status = fHyperV->read_packet(fHyperVCookie, fPacket, &length, &headerLength, + &messageLength); if (status == B_DEV_NOT_READY) { break; } else if (status != B_OK) { @@ -151,7 +151,7 @@ HIDDevice::_Callback() } // Check if this is an HID pipe data message - hv_hid_pipe_in_msg* message = reinterpret_cast(fPacket); + hv_hid_pipe_in_msg* message = reinterpret_cast(fPacket + headerLength); if (message->pipe_header.type != HV_HID_PIPE_MSGTYPE_DATA) { ERROR("Non-data HID pipe message type %u received\n", message->pipe_header.type); continue; diff --git a/src/add-ons/kernel/drivers/input/hyperv_hid/HIDDevice.h b/src/add-ons/kernel/drivers/input/hyperv_hid/HIDDevice.h index a4414141b8..a954972d9e 100644 --- a/src/add-ons/kernel/drivers/input/hyperv_hid/HIDDevice.h +++ b/src/add-ons/kernel/drivers/input/hyperv_hid/HIDDevice.h @@ -68,7 +68,7 @@ private: ConditionVariable fProtocolRespEvent; ConditionVariable fDeviceInfoEvent; - void* fPacket; + uint8* fPacket; uint16 fLastX; uint16 fLastY; };