Hello,
Recently, we found that when we plug and remove USB3.0 devices quickly under
UEFI Shell on LoongArch platform,
there will be a lag of about 3~15 minutes (the keyboard and mouse do not
reflect under Setup), which looks like the system has halted.
Here we find two possible areas that can be improved.Please take a look and
comment.
In xhcidxe driver, XhcBulkTransfer will be called and XhcTransfer is called
inside. Its function is to submit a new transaction to a target USB device
If we unplug the USB device at this time, the xhci driver will call
RecoveryStatus = XhcDequeueTrbFromEndpoint(Xhc, Urb); Whose function is Abort
the transfer
by dequeueing of the TD. is the function required? Because the USB device is
no longer on the USB port at this time, and the process is relatively slow,
can we change the code of this part into the following form without using this
function:
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
index 9b26e46a..d877b0f6 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
@@ -716,6 +716,7 @@ ON_EXIT:
return Status;
}
+#if 0
/**
Submits a new transaction to a target USB device.
@@ -784,7 +785,9 @@ XhcTransfer (
//
// The transfer timed out. Abort the transfer by dequeueing of the TD.
//
RecoveryStatus = XhcDequeueTrbFromEndpoint(Xhc, Urb);
if (RecoveryStatus == EFI_ALREADY_STARTED) {
//
// The URB is finished just before stopping endpoint.
@@ -813,6 +816,7 @@ XhcTransfer (
XhcFreeUrb (Xhc, Urb);
return Status;
}
+#endif
/**
Submits control transfer to a target USB device.
@@ -855,6 +859,7 @@ XhcControlTransfer (
)
{
USB_XHCI_INSTANCE *Xhc;
+ URB *Urb;
UINT8 Endpoint;
UINT8 Index;
UINT8 DescriptorType;
@@ -865,6 +870,7 @@ XhcControlTransfer (
EFI_USB_HUB_DESCRIPTOR *HubDesc;
EFI_TPL OldTpl;
EFI_STATUS Status;
+ EFI_STATUS RecoveryStatus;
UINTN MapSize;
EFI_USB_PORT_STATUS PortStatus;
UINT32 State;
@@ -971,6 +977,8 @@ XhcControlTransfer (
// combination of Ep addr and its direction.
//
Endpoint = (UINT8) (0 | ((TransferDirection == EfiUsbDataIn) ? 0x80 : 0));
+
+#if 0
Status = XhcTransfer (
Xhc,
DeviceAddress,
@@ -988,6 +996,49 @@ XhcControlTransfer (
if (EFI_ERROR (Status)) {
goto ON_EXIT;
}
+#endif
+
+#if 1
+ Urb = XhcCreateUrb (
+ Xhc,
+ DeviceAddress,
+ Endpoint,
+ DeviceSpeed,
+ MaximumPacketLength,
+ XHC_CTRL_TRANSFER,
+ Request,
+ Data,
+ *DataLength,
+ NULL,
+ NULL
+ );
+
+ if (Urb == NULL) {
+ DEBUG ((DEBUG_ERROR, "XhcControlTransfer: failed to create URB!\n"));
+ Status = EFI_OUT_OF_RESOURCES;
+ goto ON_EXIT;
+ }
+
+ Status = XhcExecTransfer (Xhc, FALSE, Urb, Timeout);
+
+ *TransferResult = Urb->Result;
+ *DataLength = Urb->Completed;
+
+ if (*TransferResult == EFI_USB_NOERROR) {
+ Status = EFI_SUCCESS;
+ } else if (*TransferResult == EFI_USB_ERR_STALL) {
+ RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
+
+ if (EFI_ERROR (RecoveryStatus)) {
+ DEBUG ((EFI_D_VERBOSE, "XhcControlTransfer: XhcRecoverHaltedEndpoint
failed\n"));
+ }
+ Status = EFI_DEVICE_ERROR;
+ }
+
+ Xhc->PciIo->Flush (Xhc->PciIo);
+ XhcFreeUrb (Xhc, Urb);
+
+#endif
//
// Hook Get_Descriptor request from UsbBus as we need evaluate context and
configure endpoint.
@@ -1223,9 +1274,11 @@ XhcBulkTransfer (
)
{
USB_XHCI_INSTANCE *Xhc;
+ URB *Urb;
UINT8 SlotId;
EFI_STATUS Status;
EFI_TPL OldTpl;
+ EFI_STATUS RecoveryStatus;
//
// Validate the parameters
@@ -1270,6 +1323,7 @@ XhcBulkTransfer (
// Create a new URB, insert it into the asynchronous
// schedule list, then poll the execution status.
//
+#if 0
Status = XhcTransfer (
Xhc,
DeviceAddress,
@@ -1283,6 +1337,46 @@ XhcBulkTransfer (
Timeout,
TransferResult
);
+#endif
+
+ Urb = XhcCreateUrb (
+ Xhc,
+ DeviceAddress,
+ EndPointAddress,
+ DeviceSpeed,
+ MaximumPacketLength,
+ XHC_BULK_TRANSFER,
+ NULL,
+ Data[0],
+ *DataLength,
+ NULL,
+ NULL
+ );
+
+ if (Urb == NULL) {
+ DEBUG ((DEBUG_ERROR, "XhcBulkTransfer: failed to create URB!\n"));
+ Status = EFI_OUT_OF_RESOURCES;
+ goto ON_EXIT;
+ }
+
+ Status = XhcExecTransfer (Xhc, FALSE, Urb, Timeout);
+
+ *TransferResult = Urb->Result;
+ *DataLength = Urb->Completed;
+
+ if (*TransferResult == EFI_USB_NOERROR) {
+ Status = EFI_SUCCESS;
+ } else if (*TransferResult == EFI_USB_ERR_STALL) {
+ RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
+
+ if (EFI_ERROR (RecoveryStatus)) {
+ DEBUG ((EFI_D_VERBOSE, "XhcControlTransfer: XhcRecoverHaltedEndpoint
failed\n"));
+ }
+ Status = EFI_DEVICE_ERROR;
+ }
+
+ Xhc->PciIo->Flush (Xhc->PciIo);
+ XhcFreeUrb (Xhc, Urb);
ON_EXIT:
if (EFI_ERROR (Status)) {
@@ -1498,9 +1592,11 @@ XhcSyncInterruptTransfer (
)
{
USB_XHCI_INSTANCE *Xhc;
+ URB *Urb;
UINT8 SlotId;
EFI_STATUS Status;
EFI_TPL OldTpl;
+ EFI_STATUS RecoveryStatus;
//
// Validates parameters
@@ -1540,6 +1636,7 @@ XhcSyncInterruptTransfer (
goto ON_EXIT;
}
+#if 0
Status = XhcTransfer (
Xhc,
DeviceAddress,
@@ -1553,6 +1650,45 @@ XhcSyncInterruptTransfer (
Timeout,
TransferResult
);
+#endif
+
+ Urb = XhcCreateUrb (
+ Xhc,
+ DeviceAddress,
+ EndPointAddress,
+ DeviceSpeed,
+ MaximumPacketLength,
+ XHC_INT_TRANSFER_SYNC,
+ NULL,
+ Data,
+ *DataLength,
+ NULL,
+ NULL
+ );
+
+ if (Urb == NULL) {
+ DEBUG ((EFI_D_VERBOSE, "XhcSyncInterruptTransfer: failed to create
URB\n"));
+ Status = EFI_OUT_OF_RESOURCES;
+ goto ON_EXIT;
+ }
+
+ Status = XhcExecTransfer (Xhc, FALSE, Urb, Timeout);
+
+ *TransferResult = Urb->Result;
+ *DataLength = Urb->Completed;
+
+ if (*TransferResult == EFI_USB_NOERROR) {
+ Status = EFI_SUCCESS;
+ } else if (*TransferResult == EFI_USB_ERR_STALL) {
+ RecoveryStatus = XhcRecoverHaltedEndpoint(Xhc, Urb);
+ if (EFI_ERROR (RecoveryStatus)) {
+ DEBUG ((EFI_D_VERBOSE, "XhcSyncInterruptTransfer:
XhcRecoverHaltedEndpoint failed\n"));
+ }
+ Status = EFI_DEVICE_ERROR;
+ }
+
+ Xhc->PciIo->Flush (Xhc->PciIo);
+ XhcFreeUrb (Xhc, Urb);
ON_EXIT:
if (EFI_ERROR (Status)) {
--
2.27.0
Another point worth noteing is that in the UsbMassStorageDxe driver,if the USB
device is pulled out during UsbBootExecCmdRetry, the UsbBootExecCmd will
repeatedly return Timeout.So, is it more reasonable to change the code to the
following format:
diff --git a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
index eb8f8e81..30586406 100644
--- a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
+++ b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.c
@@ -52,6 +52,7 @@ UsbBootRequestSense (
SenseCmd.Lun = (UINT8) (USB_BOOT_LUN (UsbMass->Lun));
SenseCmd.AllocLen = (UINT8) sizeof (USB_BOOT_REQUEST_SENSE_DATA);
+
Status = Transport->ExecCommand (
UsbMass->Context,
&SenseCmd,
@@ -198,7 +199,7 @@ UsbBootExecCmd (
if (Status == EFI_TIMEOUT) {
DEBUG ((EFI_D_ERROR, "UsbBootExecCmd: %r to Exec 0x%x Cmd\n", Status,
*(UINT8 *)Cmd));
- return EFI_TIMEOUT;
+ return EFI_DEVICE_ERROR;
}
//
@@ -213,7 +214,8 @@ UsbBootExecCmd (
// If command execution failed, then retrieve error info via sense request.
//
DEBUG ((EFI_D_ERROR, "UsbBootExecCmd: %r to Exec 0x%x Cmd (Result = %x)\n",
Status, *(UINT8 *)Cmd, CmdResult));
- return UsbBootRequestSense (UsbMass);
+ //return UsbBootRequestSense (UsbMass);
+ return EFI_SUCCESS;
}
@@ -251,10 +253,12 @@ UsbBootExecCmdWithRetry (
{
EFI_STATUS Status;
UINTN Retry;
- EFI_EVENT TimeoutEvt;
+ //EFI_EVENT TimeoutEvt;
- Retry = 0;
+ //Retry = 0;
Status = EFI_SUCCESS;
+
+#if 0
Status = gBS->CreateEvent (
EVT_TIMER,
TPL_CALLBACK,
@@ -271,6 +275,7 @@ UsbBootExecCmdWithRetry (
goto EXIT;
}
+
//
// Execute the cmd and retry if it fails.
//
@@ -307,6 +312,24 @@ EXIT:
gBS->CloseEvent (TimeoutEvt);
}
+#endif
+
+ for (Retry = 0; Retry < USB_BOOT_COMMAND_RETRY; Retry++) {
+ Status = UsbBootExecCmd (
+ UsbMass,
+ Cmd,
+ CmdLen,
+ DataDir,
+ Data,
+ DataLen,
+ Timeout
+ );
+
+ if (Status == EFI_SUCCESS || Status == EFI_NO_MEDIA || Status ==
EFI_MEDIA_CHANGED) {
+ break;
+ }
+ }
+
return Status;
}
@@ -540,6 +563,7 @@ UsbBootReadCapacity (
//
// Get sense data
//
+ DbgPrint (EFI_D_INFO,"Sorry,the BlockSize now is 0!!!!\n");
return UsbBootRequestSense (UsbMass);
} else {
Media->BlockSize = BlockSize;
@@ -692,6 +716,7 @@ UsbBootDetectMedia (
EFI_BLOCK_IO_MEDIA *Media;
UINT8 CmdSet;
EFI_STATUS Status;
+ EFI_TPL OldTpl;
Media = &UsbMass->BlockIoMedia;
@@ -754,7 +779,10 @@ UsbBootDetectMedia (
// This function is called from:
// Block I/O Protocol APIs, which run at TPL_CALLBACK.
// DriverBindingStart(), which raises to TPL_CALLBACK.
- ASSERT (EfiGetCurrentTpl () == TPL_CALLBACK);
+ //ASSERT (EfiGetCurrentTpl () == TPL_CALLBACK);
+ OldTpl = EfiGetCurrentTpl ();
+ gBS->RestoreTPL (TPL_CALLBACK);
+
//
// When it is called from DriverBindingStart(), below reinstall fails.
@@ -767,6 +795,9 @@ UsbBootDetectMedia (
&UsbMass->BlockIo
);
+ ASSERT (EfiGetCurrentTpl () == TPL_CALLBACK);
+ gBS->RaiseTPL (OldTpl);
+
//
// Reset MediaId after reinstalling Block I/O Protocol.
//
diff --git a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h
b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h
index 64b2985e..c502a363 100644
--- a/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h
+++ b/MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassBoot.h
@@ -72,7 +72,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
EXPRESS OR IMPLIED.
//
// Retry mass command times, set by experience
//
-#define USB_BOOT_COMMAND_RETRY 5
+#define USB_BOOT_COMMAND_RETRY 1
Thank you
Look forward to your reply.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#86677): https://edk2.groups.io/g/devel/message/86677
Mute This Topic: https://groups.io/mt/89156195/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
