Skip to content

sync with the latest downstream viostor and vioscsi related changes #1423

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Sep 26, 2025
+151 −123
Merged

sync with the latest downstream viostor and vioscsi related changes #1423

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
4cc1831
Revert "NO-SDV [vioscsi] Reduce spinlock management complexity"
vrozenfe Jun 18, 2025
7b9c6bd
Revert "Fix returning SUCCESS when really -ENOSPC (regression)"
vrozenfe Apr 2, 2025
939cac4
Revert "Backport vioscsi SendSRB fixes and improvements"
vrozenfe Apr 2, 2025
0686f2c
RHEL-107617 set data transfer length to 0 in case if no data transfered
vrozenfe Aug 12, 2025
410f21f
RHEL-107617 set data transfer length to 0 in case if no data transfered
vrozenfe Aug 12, 2025
798b688
[vioscsi] fix clang format warnings
vrozenfe Sep 23, 2025
70c568d
[viostor] fix clang format warnings
vrozenfe Sep 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 54 additions & 12 deletions vioscsi/helper.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,9 @@ VOID SendSRB(IN PVOID DeviceExtension, IN PSRB_TYPE Srb)
ULONG QueueNumber = VIRTIO_SCSI_REQUEST_QUEUE_0;
BOOLEAN notify = FALSE;
STOR_LOCK_HANDLE LockHandle = {0};
PVOID LockContext;
ULONG status = STOR_STATUS_SUCCESS;
UCHAR ScsiStatus = SCSISTAT_GOOD;
ULONG MessageId;
INT add_buffer_req_status = VQ_ADD_BUFFER_SUCCESS;
PREQUEST_LIST element;
ULONG vq_req_idx;
Expand All @@ -69,6 +69,16 @@ VOID SendSRB(IN PVOID DeviceExtension, IN PSRB_TYPE Srb)
if (adaptExt->bRemoved)
{
SRB_SET_SRB_STATUS(Srb, SRB_STATUS_NO_DEVICE);
SRB_SET_DATA_TRANSFER_LENGTH(Srb, 0);
CompleteRequest(DeviceExtension, Srb);
return;
}

if (adaptExt->reset_in_progress)
{
RhelDbgPrint(TRACE_LEVEL_FATAL, " Reset is in progress, completing SRB 0x%p with SRB_STATUS_BUS_RESET.\n", Srb);
SRB_SET_DATA_TRANSFER_LENGTH(Srb, 0);
SRB_SET_SRB_STATUS(Srb, SRB_STATUS_BUS_RESET);
CompleteRequest(DeviceExtension, Srb);
return;
}
Expand Down Expand Up @@ -106,18 +116,10 @@ VOID SendSRB(IN PVOID DeviceExtension, IN PSRB_TYPE Srb)
return;
}

MessageId = QUEUE_TO_MESSAGE(QueueNumber);
vq_req_idx = QueueNumber - VIRTIO_SCSI_REQUEST_QUEUE_0;
LockContext = &adaptExt->dpc[vq_req_idx];

if (adaptExt->reset_in_progress)
{
RhelDbgPrint(TRACE_LEVEL_FATAL, " Reset is in progress, completing SRB 0x%p with SRB_STATUS_BUS_RESET.\n", Srb);
SRB_SET_SRB_STATUS(Srb, SRB_STATUS_BUS_RESET);
CompleteRequest(DeviceExtension, Srb);
return;
}

StorPortAcquireSpinLock(DeviceExtension, DpcLock, LockContext, &LockHandle);
VioScsiVQLock(DeviceExtension, MessageId, &LockHandle, FALSE);
SET_VA_PA();
add_buffer_req_status = virtqueue_add_buf(adaptExt->vq[QueueNumber],
srbExt->psgl,
Expand Down Expand Up @@ -152,7 +154,7 @@ VOID SendSRB(IN PVOID DeviceExtension, IN PSRB_TYPE Srb)
Srb->TimeOutValue);
CompleteRequest(DeviceExtension, Srb);
}
StorPortReleaseSpinLock(DeviceExtension, &LockHandle);
VioScsiVQUnlock(DeviceExtension, MessageId, &LockHandle, FALSE);
if (notify)
{
virtqueue_notify(adaptExt->vq[QueueNumber]);
Expand Down Expand Up @@ -538,6 +540,45 @@ KickEvent(IN PVOID DeviceExtension, IN PVirtIOSCSIEventNode EventNode)
EXIT_FN();
}

VOID VioScsiVQLock(IN PVOID DeviceExtension, IN ULONG MessageID, IN OUT PSTOR_LOCK_HANDLE LockHandle, IN BOOLEAN isr)
{
PADAPTER_EXTENSION adaptExt = (PADAPTER_EXTENSION)DeviceExtension;
ULONG QueueNumber = MESSAGE_TO_QUEUE(MessageID);
ENTER_FN();

if (!isr)
{
if (adaptExt->msix_enabled)
{
// Queue numbers start at 0, message ids at 1.
NT_ASSERT(MessageID > VIRTIO_SCSI_REQUEST_QUEUE_0);
if (QueueNumber >= (adaptExt->num_queues + VIRTIO_SCSI_REQUEST_QUEUE_0))
{
QueueNumber %= adaptExt->num_queues;
}
StorPortAcquireSpinLock(DeviceExtension,
DpcLock,
&adaptExt->dpc[QueueNumber - VIRTIO_SCSI_REQUEST_QUEUE_0],
LockHandle);
}
else
{
StorPortAcquireSpinLock(DeviceExtension, InterruptLock, NULL, LockHandle);
}
}
EXIT_FN();
}

VOID VioScsiVQUnlock(IN PVOID DeviceExtension, IN ULONG MessageID, IN PSTOR_LOCK_HANDLE LockHandle, IN BOOLEAN isr)
{
Copy link

@gemini-code-assist gemini-code-assist bot Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The MessageID parameter is unused in this function. Consider removing it from the function signature to improve code clarity. This change would also need to be applied to the function declaration in vioscsi/helper.h and all call sites.

VioScsiVQUnlock(IN PVOID DeviceExtension, IN PSTOR_LOCK_HANDLE LockHandle, IN BOOLEAN isr)

ENTER_FN();
if (!isr)
{
StorPortReleaseSpinLock(DeviceExtension, LockHandle);
}
EXIT_FN();
}

VOID FirmwareRequest(IN PVOID DeviceExtension, IN PSRB_TYPE Srb)
{
PADAPTER_EXTENSION adaptExt;
Expand Down Expand Up @@ -662,6 +703,7 @@ VOID FirmwareRequest(IN PVOID DeviceExtension, IN PSRB_TYPE Srb)
break;
default:
RhelDbgPrint(TRACE_LEVEL_INFORMATION, " Unsupported Function %ul\n", firmwareRequest->Function);
SRB_SET_DATA_TRANSFER_LENGTH(Srb, 0);
SRB_SET_SRB_STATUS(Srb, SRB_STATUS_INVALID_REQUEST);
break;
}
Expand Down
10 changes: 7 additions & 3 deletions vioscsi/helper.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,13 @@ VOID VioScsiCompleteDpcRoutine(IN PSTOR_DPC Dpc, IN PVOID Context, IN PVOID Syst

VOID ProcessBuffer(IN PVOID DeviceExtension, IN ULONG MessageId, IN STOR_SPINLOCK LockMode);

VOID
// FORCEINLINE
HandleResponse(IN PVOID DeviceExtension, IN PVirtIOSCSICmd cmd);
VOID ProcessQueue(IN PVOID DeviceExtension, IN ULONG MessageID, IN BOOLEAN isr);

VOID VioScsiVQLock(IN PVOID DeviceExtension, IN ULONG MessageID, IN OUT PSTOR_LOCK_HANDLE LockHandle, IN BOOLEAN isr);

VOID VioScsiVQUnlock(IN PVOID DeviceExtension, IN ULONG MessageID, IN PSTOR_LOCK_HANDLE LockHandle, IN BOOLEAN isr);

VOID HandleResponse(IN PVOID DeviceExtension, IN PVirtIOSCSICmd cmd);

PVOID
VioScsiPoolAlloc(IN PVOID DeviceExtension, IN SIZE_T size);
Expand Down
75 changes: 34 additions & 41 deletions vioscsi/vioscsi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1025,7 +1025,7 @@ VioScsiInterrupt(IN PVOID DeviceExtension)
}
else
{
ProcessBuffer(DeviceExtension, QUEUE_TO_MESSAGE(VIRTIO_SCSI_REQUEST_QUEUE_0), InterruptLock);
ProcessQueue(DeviceExtension, QUEUE_TO_MESSAGE(VIRTIO_SCSI_REQUEST_QUEUE_0), TRUE);
}
}

Expand Down Expand Up @@ -1254,17 +1254,17 @@ VioScsiUnitControl(IN PVOID DeviceExtension, IN SCSI_UNIT_CONTROL_TYPE ControlTy
break;
case ScsiUnitRemove:
case ScsiUnitSurpriseRemoval:
ULONG vq_req_idx;
PREQUEST_LIST element;
ULONG QueuNum;
ULONG MsgId;
Comment on lines +1257 to +1258
Copy link

@gemini-code-assist gemini-code-assist bot Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

There's a typo in QueuNum; it should be QueueNum. Also, for consistency and clarity, consider using the full name MessageId instead of the abbreviation MsgId.

            ULONG QueueNum;
            ULONG MessageId;

Copy link
Collaborator

@YanVugenfirer YanVugenfirer Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 for typo

-1 for renaming MsgId

STOR_LOCK_HANDLE LockHandle = {0};
PVOID LockContext = NULL; // sanity check for LockMode = InterruptLock or StartIoLock
PSTOR_ADDR_BTL8 stor_addr = (PSTOR_ADDR_BTL8)Parameters;

for (vq_req_idx = 0; vq_req_idx < adaptExt->num_queues; vq_req_idx++)
for (index = 0; index < adaptExt->num_queues; index++)
{
element = &adaptExt->processing_srbs[vq_req_idx];
LockContext = &adaptExt->dpc[vq_req_idx];
StorPortAcquireSpinLock(DeviceExtension, DpcLock, LockContext, &LockHandle);
PREQUEST_LIST element = &adaptExt->processing_srbs[index];
QueuNum = index + VIRTIO_SCSI_REQUEST_QUEUE_0;
MsgId = QUEUE_TO_MESSAGE(QueuNum);
VioScsiVQLock(DeviceExtension, MsgId, &LockHandle, FALSE);
if (!IsListEmpty(&element->srb_list))
{
PLIST_ENTRY entry = element->srb_list.Flink;
Expand All @@ -1276,6 +1276,7 @@ VioScsiUnitControl(IN PVOID DeviceExtension, IN SCSI_UNIT_CONTROL_TYPE ControlTy
SRB_LUN(currSrb) == stor_addr->Lun)
{
SRB_SET_SRB_STATUS(currSrb, SRB_STATUS_NO_DEVICE);
SRB_SET_DATA_TRANSFER_LENGTH(currSrb, 0);
CompleteRequest(DeviceExtension, (PSRB_TYPE)currSrb);
RhelDbgPrint(TRACE_LEVEL_INFORMATION,
" Complete pending I/Os on Path %d Target %d Lun %d \n",
Expand All @@ -1287,7 +1288,7 @@ VioScsiUnitControl(IN PVOID DeviceExtension, IN SCSI_UNIT_CONTROL_TYPE ControlTy
entry = entry->Flink;
}
}
StorPortReleaseSpinLock(DeviceExtension, &LockHandle);
VioScsiVQUnlock(DeviceExtension, MsgId, &LockHandle, FALSE);
}
Status = ScsiUnitControlSuccess;
break;
Expand Down Expand Up @@ -1326,6 +1327,7 @@ VioScsiBuildIo(IN PVOID DeviceExtension, IN PSCSI_REQUEST_BLOCK Srb)
(Lun >= adaptExt->scsi_config.max_lun) || adaptExt->bRemoved)
{
SRB_SET_SRB_STATUS(Srb, SRB_STATUS_NO_DEVICE);
SRB_SET_DATA_TRANSFER_LENGTH(Srb, 0);
StorPortNotification(RequestComplete, DeviceExtension, Srb);
return FALSE;
}
Expand Down Expand Up @@ -1432,41 +1434,31 @@ VOID FORCEINLINE DispatchQueue(IN PVOID DeviceExtension, IN ULONG MessageId)
EXIT_FN();
return;
}
ProcessBuffer(DeviceExtension, MessageId, InterruptLock);
ProcessQueue(DeviceExtension, MessageId, TRUE);
EXIT_FN();
}

VOID ProcessBuffer(IN PVOID DeviceExtension, IN ULONG MessageId, IN STOR_SPINLOCK LockMode)
VOID ProcessQueue(IN PVOID DeviceExtension, IN ULONG MessageID, IN BOOLEAN isr)
{
ULONG_PTR srbId;
unsigned int len;
PADAPTER_EXTENSION adaptExt = (PADAPTER_EXTENSION)DeviceExtension;
ULONG QueueNumber = MESSAGE_TO_QUEUE(MessageId);
STOR_LOCK_HANDLE LockHandle = {0};
ULONG index = MESSAGE_TO_QUEUE(MessageID);
Copy link

@gemini-code-assist gemini-code-assist bot Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The variable index is a bit generic. Since it represents a queue index, renaming it to queueIndex would make the code more self-documenting and easier to understand. This change should be applied to all uses of index in this function.

    ULONG queueIndex = MESSAGE_TO_QUEUE(MessageID);

STOR_LOCK_HANDLE queueLock = {0};
struct virtqueue *vq;
PSRB_EXTENSION srbExt = NULL;
PREQUEST_LIST element;
ULONG vq_req_idx;
PVOID LockContext = NULL; // sanity check for LockMode = InterruptLock or StartIoLock

ENTER_FN();

if (QueueNumber >= (adaptExt->num_queues + VIRTIO_SCSI_REQUEST_QUEUE_0))
if (index >= (adaptExt->num_queues + VIRTIO_SCSI_REQUEST_QUEUE_0))
{
RhelDbgPrint(TRACE_LEVEL_VERBOSE,
" Modulo assignment required for QueueNumber as it exceeds the number of virtqueues available.\n");
QueueNumber %= adaptExt->num_queues;
index %= adaptExt->num_queues;
}
vq_req_idx = QueueNumber - VIRTIO_SCSI_REQUEST_QUEUE_0;
element = &adaptExt->processing_srbs[vq_req_idx];

vq = adaptExt->vq[QueueNumber];
PREQUEST_LIST element = &adaptExt->processing_srbs[index - VIRTIO_SCSI_REQUEST_QUEUE_0];
vq = adaptExt->vq[index];

if (LockMode == DpcLock)
{
LockContext = &adaptExt->dpc[vq_req_idx];
}
StorPortAcquireSpinLock(DeviceExtension, LockMode, LockContext, &LockHandle);
VioScsiVQLock(DeviceExtension, MessageID, &queueLock, isr);

do
{
Expand Down Expand Up @@ -1500,7 +1492,7 @@ VOID ProcessBuffer(IN PVOID DeviceExtension, IN ULONG MessageId, IN STOR_SPINLOC
}
} while (!virtqueue_enable_cb(vq));

StorPortReleaseSpinLock(DeviceExtension, &LockHandle);
VioScsiVQUnlock(DeviceExtension, MessageID, &queueLock, isr);

EXIT_FN();
}
Expand All @@ -1511,18 +1503,15 @@ VOID VioScsiCompleteDpcRoutine(IN PSTOR_DPC Dpc, IN PVOID Context, IN PVOID Syst

ENTER_FN();
MessageId = PtrToUlong(SystemArgument1);
ProcessBuffer(Context, MessageId, DpcLock);
ProcessQueue(Context, MessageId, FALSE);
EXIT_FN();
}

VOID CompletePendingRequestsOnReset(IN PVOID DeviceExtension)
{
PADAPTER_EXTENSION adaptExt;
ULONG QueueNumber;
ULONG vq_req_idx;
PREQUEST_LIST element;
STOR_LOCK_HANDLE LockHandle = {0};
PVOID LockContext = NULL; // sanity check for LockMode = InterruptLock or StartIoLock
ULONG QueueNum;
ULONG MsgId;
Comment on lines +1513 to +1514
Copy link

@gemini-code-assist gemini-code-assist bot Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For consistency and clarity, consider using QueueNum and MessageId instead of QueuNum and MsgId. This fixes a typo and avoids abbreviation.

    ULONG QueueNum;
    ULONG MessageId;


adaptExt = (PADAPTER_EXTENSION)DeviceExtension;

Expand All @@ -1532,12 +1521,14 @@ VOID CompletePendingRequestsOnReset(IN PVOID DeviceExtension)
StorPortPause(DeviceExtension, 10);
DeviceReset(DeviceExtension);

for (vq_req_idx = 0; vq_req_idx < adaptExt->num_queues; vq_req_idx++)
for (ULONG index = 0; index < adaptExt->num_queues; index++)
{
element = &adaptExt->processing_srbs[vq_req_idx];
RhelDbgPrint(TRACE_LEVEL_FATAL, " queue %d cnt %d\n", vq_req_idx, element->srb_cnt);
LockContext = &adaptExt->dpc[vq_req_idx];
StorPortAcquireSpinLock(DeviceExtension, DpcLock, LockContext, &LockHandle);
PREQUEST_LIST element = &adaptExt->processing_srbs[index];
STOR_LOCK_HANDLE LockHandle = {0};
RhelDbgPrint(TRACE_LEVEL_FATAL, " queue %d cnt %d\n", index, element->srb_cnt);
QueueNum = index + VIRTIO_SCSI_REQUEST_QUEUE_0;
MsgId = QUEUE_TO_MESSAGE(QueueNum);
VioScsiVQLock(DeviceExtension, MsgId, &LockHandle, FALSE);
while (!IsListEmpty(&element->srb_list))
{
PLIST_ENTRY entry = RemoveHeadList(&element->srb_list);
Expand All @@ -1548,6 +1539,7 @@ VOID CompletePendingRequestsOnReset(IN PVOID DeviceExtension)
if (currSrb)
{
SRB_SET_SRB_STATUS(currSrb, SRB_STATUS_BUS_RESET);
SRB_SET_DATA_TRANSFER_LENGTH(currSrb, 0);
CompleteRequest(DeviceExtension, (PSRB_TYPE)currSrb);
element->srb_cnt--;
}
Expand All @@ -1557,7 +1549,7 @@ VOID CompletePendingRequestsOnReset(IN PVOID DeviceExtension)
{
element->srb_cnt = 0;
}
StorPortReleaseSpinLock(DeviceExtension, &LockHandle);
VioScsiVQUnlock(DeviceExtension, MsgId, &LockHandle, FALSE);
}
StorPortResume(DeviceExtension);
}
Expand Down Expand Up @@ -1935,6 +1927,7 @@ VOID VioScsiIoControl(IN PVOID DeviceExtension, IN OUT PSRB_TYPE Srb)
break;
default:
SRB_SET_SRB_STATUS(Srb, SRB_STATUS_INVALID_REQUEST);
SRB_SET_DATA_TRANSFER_LENGTH(Srb, 0);
RhelDbgPrint(TRACE_LEVEL_INFORMATION, " <--> Unsupport control code 0x%x\n", srbControl->ControlCode);
break;
}
Expand Down
5 changes: 4 additions & 1 deletion viostor/virtio_stor.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -869,6 +869,7 @@ VOID CompletePendingRequests(IN PVOID DeviceExtension)
PSRB_EXTENSION currSrbExt = SRB_EXTENSION(currSrb);
if (currSrb)
{
SRB_SET_DATA_TRANSFER_LENGTH(currSrb, 0);
CompleteRequestWithStatus(DeviceExtension, (PSRB_TYPE)currSrb, SRB_STATUS_BUS_RESET);
element->srb_cnt--;
}
Expand Down Expand Up @@ -1012,6 +1013,7 @@ VirtIoStartIo(IN PVOID DeviceExtension, IN PSCSI_REQUEST_BLOCK Srb)

default:
{
SRB_SET_DATA_TRANSFER_LENGTH(Srb, 0);
CompleteRequestWithStatus(DeviceExtension, (PSRB_TYPE)Srb, SRB_STATUS_INVALID_REQUEST);
return TRUE;
}
Expand Down Expand Up @@ -1149,7 +1151,7 @@ VirtIoStartIo(IN PVOID DeviceExtension, IN PSCSI_REQUEST_BLOCK Srb)
Srb,
SRB_FUNCTION(Srb),
cdb->CDB6GENERIC.OperationCode);

SRB_SET_DATA_TRANSFER_LENGTH(Srb, 0);
CompleteRequestWithStatus(DeviceExtension, (PSRB_TYPE)Srb, SRB_STATUS_INVALID_REQUEST);
return TRUE;
}
Expand Down Expand Up @@ -1335,6 +1337,7 @@ VirtIoBuildIo(IN PVOID DeviceExtension, IN PSCSI_REQUEST_BLOCK Srb)
#endif
if (SRB_PATH_ID(Srb) || SRB_TARGET_ID(Srb) || SRB_LUN(Srb) || ((adaptExt->removed == TRUE)))
{
SRB_SET_DATA_TRANSFER_LENGTH(Srb, 0);
CompleteRequestWithStatus(DeviceExtension, (PSRB_TYPE)Srb, SRB_STATUS_NO_DEVICE);
return FALSE;
}
Expand Down
Loading