[edk2-devel] [PATCH v3 1/1] MdeModulePkg/UsbBusDxe: Rebuild the description table after Reset Device
Srikumar Subramanian
srikumar-subramanian at hpe.com
Wed Dec 9 12:38:36 UTC 2020
Hi Jiang,
Is it the responsibility of the reset routine to know what is wrong with the device? And on this topic I would like to know your thoughts on another alternative which is saving the config in XhcPollPortStatusChange before disabling the slot and restoring it after initializing the device slot again. My rationale for doing this is that, the reset device state diagram according to the specs moves the device from the configured to the default state and does not completely remove device information.
In this regard is it safe to assume that meta-data such as descriptors should be restored. Since in this case a port reset is triggering a device reset but because we disable and later initialize a new slot for an existing device we lose its configuration which we assume does not change.
Let me know your thoughts and inputs on this. And if I needed an immediate solution I would like to know the better option between the one I am proposing right now and the existing suggested solution (which is making a request to the device again for the descriptors.)
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
index 1754a97..d330f21 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
@@ -1,5 +1,5 @@
/**
**/
/** @file
@@ -1587,6 +1587,65 @@ XhcMonitorAsyncRequests (
gBS->RestoreTPL (OldTpl);
}
+/**
+ Save the current configuration for the Slot in Host Controller
+
+ @param Xhc The XHCI instance
+ @param SlotId The Slot ID
+ @param ActiveAlternateSetting The Alternate interface setting that is part of the device context
+ @param ConfDesc The Configuration Descriptors of the device present in the slot
+ @param DevDesc The Device Descriptor of the device present in the slot
+**/
+VOID
+XhcSaveConfig(
+ IN USB_XHCI_INSTANCE *Xhc,
+ IN UINT8 SlotId,
+ OUT UINT8 **ActiveAlternateSetting,
+ OUT EFI_USB_CONFIG_DESCRIPTOR ***ConfDesc,
+ OUT EFI_USB_DEVICE_DESCRIPTOR *DevDesc
+)
+{
+ UINT8 Index;
+ UINT8 IfIndex;
+ UINT8 NumInterfaces;
+ UINT64 ConfSize;
+ EFI_USB_INTERFACE_DESCRIPTOR *IfDesc;
+
+ *DevDesc = Xhc->UsbDevContext[SlotId].DevDesc;
+
+ if(DevDesc->NumConfigurations==0){
+ return;
+ }
+
+ *ConfDesc = AllocateZeroPool(DevDesc->NumConfigurations * sizeof(EFI_USB_CONFIG_DESCRIPTOR*));
+
+ for(Index = 0; Index < DevDesc->NumConfigurations; Index++){
+ NumInterfaces = Xhc->UsbDevContext[SlotId].ConfDesc[Index]->NumInterfaces;
+ IfDesc = (EFI_USB_INTERFACE_DESCRIPTOR *)(Xhc->UsbDevContext[SlotId].ConfDesc[Index] + 1);
+
+ // Calculating the size of the configuration descriptor.
+ // This is needed since the interface and enpoint descriptors are nested within this.
+ // See USB_CONFIG_DESCRIPTOR, USB_INTERFACE_DESCRIPTOR, USB_ENDPOINT_DESCRIPTOR in MdePkg/Include/IndustryStandard/Usb.h
+ // See USB_CONFIG_DESC, USB_INTERFACE_DESC, USB_ENDPOINT_DESC in MdeModulePkg/Bus/Usb/UsbBusDxe/UsbDesc.h
+ ConfSize = sizeof(Xhc->UsbDevContext[SlotId].ConfDesc[Index]);
+ ConfSize += (sizeof(EFI_USB_INTERFACE_DESCRIPTOR) * (NumInterfaces));
+ for(IfIndex = 0; IfIndex < NumInterfaces; IfIndex++){
+ ConfSize += IfDesc->NumEndpoints * sizeof(EFI_USB_ENDPOINT_DESCRIPTOR);
+ IfDesc = (USB_INTERFACE_DESCRIPTOR *)((UINTN)IfDesc + IfDesc->Length);
+ }
+
+ *ConfDesc[Index] = AllocateZeroPool(ConfSize);
+ CopyMem (*ConfDesc[Index] ,Xhc->UsbDevContext[SlotId].ConfDesc[Index], ConfSize);
+ }
+
+ *ActiveAlternateSetting = AllocateZeroPool(Xhc->UsbDevContext[SlotId].ConfDesc[0]->NumInterfaces * sizeof(UINT8));
+
+ CopyMem(*ActiveAlternateSetting, Xhc->UsbDevContext[SlotId].ActiveAlternateSetting, Xhc->UsbDevContext[SlotId].ConfDesc[0]->NumInterfaces * sizeof(UINT8));
+
+}
+
/**
Monitor the port status change. Enable/Disable device slot if there is a device attached/detached.
@@ -1613,6 +1672,15 @@ XhcPollPortStatusChange (
UINT8 SlotId;
USB_DEV_ROUTE RouteChart;
+ EFI_USB_CONFIG_DESCRIPTOR **ConfDescBuffer;
+ EFI_USB_DEVICE_DESCRIPTOR DevDescBuffer;
+ UINT8 *ActiveAlternateSettingBuffer;
+
+ ConfDescBuffer = NULL;
+ ActiveAlternateSettingBuffer = NULL;
+
Status = EFI_SUCCESS;
if ((PortState->PortChangeStatus & (USB_PORT_STAT_C_CONNECTION | USB_PORT_STAT_C_ENABLE | USB_PORT_STAT_C_OVERCURRENT | USB_PORT_STAT_C_RESET)) == 0) {
@@ -1635,6 +1703,14 @@ XhcPollPortStatusChange (
SlotId = XhcRouteStringToSlotId (Xhc, RouteChart);
if (SlotId != 0) {
+
+ // Saving the current host controller configuration for reuse after device reset
+ if((PortState->PortChangeStatus & USB_PORT_STAT_C_RESET) != 0){
+ XhcSaveConfig(Xhc, SlotId, &ActiveAlternateSettingBuffer, &ConfDescBuffer, &DevDescBuffer);
+ }
+
if (Xhc->HcCParams.Data.Csz == 0) {
Status = XhcDisableSlotCmd (Xhc, SlotId);
} else {
@@ -1660,11 +1736,23 @@ XhcPollPortStatusChange (
//
SlotId = XhcRouteStringToSlotId (Xhc, RouteChart);
if ((SlotId == 0) && ((PortState->PortChangeStatus & USB_PORT_STAT_C_RESET) != 0)) {
+
+
+ // Introducing the SlotId as a new parameter in XhcInitializeDeviceSlot in order to get the
+ // new slotId assigned.
if (Xhc->HcCParams.Data.Csz == 0) {
- Status = XhcInitializeDeviceSlot (Xhc, ParentRouteChart, Port, RouteChart, Speed);
+ Status = XhcInitializeDeviceSlot (Xhc, ParentRouteChart, Port, RouteChart, Speed, &SlotId);
} else {
- Status = XhcInitializeDeviceSlot64 (Xhc, ParentRouteChart, Port, RouteChart, Speed);
+ Status = XhcInitializeDeviceSlot64 (Xhc, ParentRouteChart, Port, RouteChart, Speed, &SlotId);
}
+
+ if(!EFI_ERROR(Status) && ConfDescBuffer!=NULL && SlotId!=0){
+ Xhc->UsbDevContext[SlotId].DevDesc = DevDescBuffer;
+ Xhc->UsbDevContext[SlotId].ConfDesc = ConfDescBuffer;
+ Xhc->UsbDevContext[SlotId].ActiveAlternateSetting = ActiveAlternateSettingBuffer;
+ }
+
}
}
@@ -1987,7 +2075,11 @@ XhcInitializeDeviceSlot (
IN USB_DEV_ROUTE ParentRouteChart,
IN UINT16 ParentPort,
IN USB_DEV_ROUTE RouteChart,
- IN UINT8 DeviceSpeed
+ IN UINT8 DeviceSpeed,
+ // OPTIONAL as this need not be used
+ OPTIONAL OUT UINT8 *SlotIdStore
)
{
EFI_STATUS Status;
@@ -2175,6 +2267,12 @@ XhcInitializeDeviceSlot (
Xhc->UsbDevContext[SlotId].XhciDevAddr = DeviceAddress;
}
+ if(SlotIdStore != NULL){
+ *SlotIdStore = SlotId;
+ }
+
return Status;
}
@@ -2197,7 +2295,11 @@ XhcInitializeDeviceSlot64 (
IN USB_DEV_ROUTE ParentRouteChart,
IN UINT16 ParentPort,
IN USB_DEV_ROUTE RouteChart,
- IN UINT8 DeviceSpeed
+ IN UINT8 DeviceSpeed,
+ // OPTIONAL as this need not be used
+ OPTIONAL OUT UINT8 *SlotIdStore
)
{
EFI_STATUS Status;
@@ -2384,6 +2486,13 @@ XhcInitializeDeviceSlot64 (
DEBUG ((EFI_D_INFO, " Address %d assigned successfully\n", DeviceAddress));
Xhc->UsbDevContext[SlotId].XhciDevAddr = DeviceAddress;
}
+
+ if(SlotIdStore != NULL){
+ *SlotIdStore = SlotId;
+ }
+
return Status;
}
diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h
index 4da9669..64ef878 100644
--- a/PurleyPlatPkg/Override/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h
+++ b/PurleyPlatPkg/Override/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.h
@@ -1,3 +1,6 @@
+/**
+* CONFIDENTIAL (C) Copyright 2020 Hewlett Packard Enterprise Development LP<BR>
+**/
/** @file
This file contains the definition for XHCI host controller schedule routines.
@@ -1120,7 +1123,11 @@ XhcInitializeDeviceSlot (
IN USB_DEV_ROUTE ParentRouteChart,
IN UINT16 ParentPort,
IN USB_DEV_ROUTE RouteChart,
- IN UINT8 DeviceSpeed
+ IN UINT8 DeviceSpeed,
+ // HPE_EXL_SS_QXCR1001775642
+ // OPTIONAL as this need not be used
+ OPTIONAL OUT UINT8 *SlotIdStore
+ // HPE_EXL_SS_QXCR1001775642
);
/**
@@ -1142,7 +1149,11 @@ XhcInitializeDeviceSlot64 (
IN USB_DEV_ROUTE ParentRouteChart,
IN UINT16 ParentPort,
IN USB_DEV_ROUTE RouteChart,
- IN UINT8 DeviceSpeed
+ IN UINT8 DeviceSpeed,
+ // OPTIONAL as this need not be used
+ OPTIONAL OUT UINT8 *SlotIdStore
);
/**
@@ -1458,4 +1469,24 @@ XhcCreateTransferTrb (
IN URB *Urb
);
+/**
+ Save the current configuration for the Slot in Host Controller
+
+ @param Xhc The XHCI instance
+ @param SlotId The Slot ID
+ @param ActiveAlternateSetting The Alternate interface setting that is part of the device context
+ @param ConfDesc The Configuration Descriptors of the device present in the slot
+ @param DevDesc The Device Descriptor of the device present in the slot
+**/
+VOID
+XhcSaveConfig(
+ IN USB_XHCI_INSTANCE *Xhc,
+ IN UINT8 SlotId,
+ OUT UINT8 **ActiveAlternateSetting,
+ OUT EFI_USB_CONFIG_DESCRIPTOR ***ConfDesc,
+ OUT EFI_USB_DEVICE_DESCRIPTOR *DevDesc
+);
+
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#68653): https://edk2.groups.io/g/devel/message/68653
Mute This Topic: https://groups.io/mt/74091912/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/edk2-devel-archive/attachments/20201209/edc0b2cc/attachment.htm>
More information about the edk2-devel-archive
mailing list