XhciDxe: Fail the start of malfunctioning XHCI controllers

Add missing error checking for malfunctioning XHCI controllers.

Signed-off-by: Rebecca Cran <rebecca@os.amperecomputing.com>

3 files changed, 45 insertions, 15 deletions

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
index cf6b32959e..a9d7e3616e 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/Xhci.c
@@ -2077,7 +2077,12 @@ XhcDriverBindingStart (
 

   XhcSetBiosOwnership (Xhc);

 

-  XhcResetHC (Xhc, XHC_RESET_TIMEOUT);

+  Status = XhcResetHC (Xhc, XHC_RESET_TIMEOUT);

+  if (EFI_ERROR (Status)) {

+    DEBUG ((DEBUG_ERROR, "%a: failed to reset HC\n", __func__));

+    goto FREE_POOL;

+  }

+

   ASSERT (XhcIsHalt (Xhc));

 

   //

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
index 525942a167..dc8228b429 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
@@ -29,6 +29,8 @@ XhcReadCapReg8 (
   UINT8       Data;

   EFI_STATUS  Status;

 

+  Data = 0;

+

   Status = Xhc->PciIo->Mem.Read (

                              Xhc->PciIo,

                              EfiPciIoWidthUint8,

diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
index a97ed44dbf..3caa060f35 100644
--- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
+++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciSched.c
@@ -2165,6 +2165,7 @@ XhcInitializeDeviceSlot (
   DEVICE_CONTEXT              *ParentDeviceContext;

   EFI_PHYSICAL_ADDRESS        PhyAddr;

 

+  EvtTrb = NULL;

   ZeroMem (&CmdTrb, sizeof (CMD_TRB_ENABLE_SLOT));

   CmdTrb.CycleBit = 1;

   CmdTrb.Type     = TRB_TYPE_EN_SLOT;

@@ -2175,7 +2176,7 @@ XhcInitializeDeviceSlot (
              XHC_GENERIC_TIMEOUT,

              (TRB_TEMPLATE **)(UINTN)&EvtTrb

              );

-  if (EFI_ERROR (Status)) {

+  if (EFI_ERROR (Status) || (EvtTrb == NULL)) {

     DEBUG ((DEBUG_ERROR, "XhcInitializeDeviceSlot: Enable Slot Failed, Status = %r\n", Status));

     return Status;

   }

@@ -2390,6 +2391,7 @@ XhcInitializeDeviceSlot64 (
   DEVICE_CONTEXT_64           *ParentDeviceContext;

   EFI_PHYSICAL_ADDRESS        PhyAddr;

 

+  EvtTrb = NULL;

   ZeroMem (&CmdTrb, sizeof (CMD_TRB_ENABLE_SLOT));

   CmdTrb.CycleBit = 1;

   CmdTrb.Type     = TRB_TYPE_EN_SLOT;

@@ -2400,7 +2402,7 @@ XhcInitializeDeviceSlot64 (
              XHC_GENERIC_TIMEOUT,

              (TRB_TEMPLATE **)(UINTN)&EvtTrb

              );

-  if (EFI_ERROR (Status)) {

+  if (EFI_ERROR (Status) || (EvtTrb == NULL)) {

     DEBUG ((DEBUG_ERROR, "XhcInitializeDeviceSlot64: Enable Slot Failed, Status = %r\n", Status));

     return Status;

   }

@@ -2602,6 +2604,8 @@ XhcDisableSlotCmd (
   UINT8                 Index;

   VOID                  *RingSeg;

 

+  EvtTrb = NULL;

+

   //

   // Disable the device slots occupied by these devices on its downstream ports.

   // Entry 0 is reserved.

@@ -2637,7 +2641,7 @@ XhcDisableSlotCmd (
                              XHC_GENERIC_TIMEOUT,

                              (TRB_TEMPLATE **)(UINTN)&EvtTrb

                              );

-  if (EFI_ERROR (Status)) {

+  if (EFI_ERROR (Status) || (EvtTrb == NULL)) {

     DEBUG ((DEBUG_ERROR, "XhcDisableSlotCmd: Disable Slot Command Failed, Status = %r\n", Status));

     return Status;

   }

@@ -2713,6 +2717,8 @@ XhcDisableSlotCmd64 (
   UINT8                 Index;

   VOID                  *RingSeg;

 

+  EvtTrb = NULL;

+

   //

   // Disable the device slots occupied by these devices on its downstream ports.

   // Entry 0 is reserved.

@@ -2748,7 +2754,7 @@ XhcDisableSlotCmd64 (
                              XHC_GENERIC_TIMEOUT,

                              (TRB_TEMPLATE **)(UINTN)&EvtTrb

                              );

-  if (EFI_ERROR (Status)) {

+  if (EFI_ERROR (Status) || (EvtTrb == NULL)) {

     DEBUG ((DEBUG_ERROR, "XhcDisableSlotCmd: Disable Slot Command Failed, Status = %r\n", Status));

     return Status;

   }

@@ -3240,6 +3246,8 @@ XhcSetConfigCmd (
   DEVICE_CONTEXT              *OutputContext;

   EVT_TRB_COMMAND_COMPLETION  *EvtTrb;

 

+  EvtTrb = NULL;

+

   //

   // 4.6.6 Configure Endpoint

   //

@@ -3290,7 +3298,7 @@ XhcSetConfigCmd (
              XHC_GENERIC_TIMEOUT,

              (TRB_TEMPLATE **)(UINTN)&EvtTrb

              );

-  if (EFI_ERROR (Status)) {

+  if (EFI_ERROR (Status) || (EvtTrb == NULL)) {

     DEBUG ((DEBUG_ERROR, "XhcSetConfigCmd: Config Endpoint Failed, Status = %r\n", Status));

   } else {

     Xhc->UsbDevContext[SlotId].ActiveConfiguration = ConfigDesc->ConfigurationValue;

@@ -3331,6 +3339,8 @@ XhcSetConfigCmd64 (
   DEVICE_CONTEXT_64           *OutputContext;

   EVT_TRB_COMMAND_COMPLETION  *EvtTrb;

 

+  EvtTrb = NULL;

+

   //

   // 4.6.6 Configure Endpoint

   //

@@ -3381,7 +3391,7 @@ XhcSetConfigCmd64 (
              XHC_GENERIC_TIMEOUT,

              (TRB_TEMPLATE **)(UINTN)&EvtTrb

              );

-  if (EFI_ERROR (Status)) {

+  if (EFI_ERROR (Status) || (EvtTrb == NULL)) {

     DEBUG ((DEBUG_ERROR, "XhcSetConfigCmd64: Config Endpoint Failed, Status = %r\n", Status));

   } else {

     Xhc->UsbDevContext[SlotId].ActiveConfiguration = ConfigDesc->ConfigurationValue;

@@ -3417,6 +3427,8 @@ XhcStopEndpoint (
 

   DEBUG ((DEBUG_VERBOSE, "XhcStopEndpoint: Slot = 0x%x, Dci = 0x%x\n", SlotId, Dci));

 

+  EvtTrb = NULL;

+

   //

   // When XhcCheckUrbResult waits for the Stop_Endpoint completion, it also checks

   // the PendingUrb completion status, because it's possible that the PendingUrb is

@@ -3454,7 +3466,7 @@ XhcStopEndpoint (
                             XHC_GENERIC_TIMEOUT,

                             (TRB_TEMPLATE **)(UINTN)&EvtTrb

                             );

-  if (EFI_ERROR (Status)) {

+  if (EFI_ERROR (Status) || (EvtTrb == NULL)) {

     DEBUG ((DEBUG_ERROR, "XhcStopEndpoint: Stop Endpoint Failed, Status = %r\n", Status));

   }

 

@@ -3488,6 +3500,8 @@ XhcResetEndpoint (
 

   DEBUG ((DEBUG_INFO, "XhcResetEndpoint: Slot = 0x%x, Dci = 0x%x\n", SlotId, Dci));

 

+  EvtTrb = NULL;

+

   //

   // Send stop endpoint command to transit Endpoint from running to stop state

   //

@@ -3502,7 +3516,7 @@ XhcResetEndpoint (
                              XHC_GENERIC_TIMEOUT,

                              (TRB_TEMPLATE **)(UINTN)&EvtTrb

                              );

-  if (EFI_ERROR (Status)) {

+  if (EFI_ERROR (Status) || (EvtTrb == NULL)) {

     DEBUG ((DEBUG_ERROR, "XhcResetEndpoint: Reset Endpoint Failed, Status = %r\n", Status));

   }

 

@@ -3538,6 +3552,8 @@ XhcSetTrDequeuePointer (
 

   DEBUG ((DEBUG_VERBOSE, "XhcSetTrDequeuePointer: Slot = 0x%x, Dci = 0x%x, Urb = 0x%x\n", SlotId, Dci, Urb));

 

+  EvtTrb = NULL;

+

   //

   // Send stop endpoint command to transit Endpoint from running to stop state

   //

@@ -3555,7 +3571,7 @@ XhcSetTrDequeuePointer (
                            XHC_GENERIC_TIMEOUT,

                            (TRB_TEMPLATE **)(UINTN)&EvtTrb

                            );

-  if (EFI_ERROR (Status)) {

+  if (EFI_ERROR (Status) || (EvtTrb == NULL)) {

     DEBUG ((DEBUG_ERROR, "XhcSetTrDequeuePointer: Set TR Dequeue Pointer Failed, Status = %r\n", Status));

   }

 

@@ -3604,6 +3620,7 @@ XhcSetInterface (
   EVT_TRB_COMMAND_COMPLETION  *EvtTrb;

 

   Status = EFI_SUCCESS;

+  EvtTrb = NULL;

 

   InputContext  = Xhc->UsbDevContext[SlotId].InputContext;

   OutputContext = Xhc->UsbDevContext[SlotId].OutputContext;

@@ -3755,7 +3772,7 @@ XhcSetInterface (
                XHC_GENERIC_TIMEOUT,

                (TRB_TEMPLATE **)(UINTN)&EvtTrb

                );

-    if (EFI_ERROR (Status)) {

+    if (EFI_ERROR (Status) || (EvtTrb == NULL)) {

       DEBUG ((DEBUG_ERROR, "SetInterface: Config Endpoint Failed, Status = %r\n", Status));

     } else {

       //

@@ -3810,6 +3827,7 @@ XhcSetInterface64 (
   EVT_TRB_COMMAND_COMPLETION  *EvtTrb;

 

   Status = EFI_SUCCESS;

+  EvtTrb = NULL;

 

   InputContext  = Xhc->UsbDevContext[SlotId].InputContext;

   OutputContext = Xhc->UsbDevContext[SlotId].OutputContext;

@@ -3961,7 +3979,7 @@ XhcSetInterface64 (
                XHC_GENERIC_TIMEOUT,

                (TRB_TEMPLATE **)(UINTN)&EvtTrb

                );

-    if (EFI_ERROR (Status)) {

+    if (EFI_ERROR (Status) || (EvtTrb == NULL)) {

       DEBUG ((DEBUG_ERROR, "SetInterface64: Config Endpoint Failed, Status = %r\n", Status));

     } else {

       //

@@ -4001,6 +4019,8 @@ XhcEvaluateContext (
 

   ASSERT (Xhc->UsbDevContext[SlotId].SlotId != 0);

 

+  EvtTrb = NULL;

+

   //

   // 4.6.7 Evaluate Context

   //

@@ -4028,7 +4048,7 @@ XhcEvaluateContext (
              XHC_GENERIC_TIMEOUT,

              (TRB_TEMPLATE **)(UINTN)&EvtTrb

              );

-  if (EFI_ERROR (Status)) {

+  if (EFI_ERROR (Status) || (EvtTrb == NULL)) {

     DEBUG ((DEBUG_ERROR, "XhcEvaluateContext: Evaluate Context Failed, Status = %r\n", Status));

   }

 

@@ -4062,6 +4082,8 @@ XhcEvaluateContext64 (
 

   ASSERT (Xhc->UsbDevContext[SlotId].SlotId != 0);

 

+  EvtTrb = NULL;

+

   //

   // 4.6.7 Evaluate Context

   //

@@ -4089,7 +4111,7 @@ XhcEvaluateContext64 (
              XHC_GENERIC_TIMEOUT,

              (TRB_TEMPLATE **)(UINTN)&EvtTrb

              );

-  if (EFI_ERROR (Status)) {

+  if (EFI_ERROR (Status) || (EvtTrb == NULL)) {

     DEBUG ((DEBUG_ERROR, "XhcEvaluateContext64: Evaluate Context Failed, Status = %r\n", Status));

   }

 

@@ -4125,6 +4147,7 @@ XhcConfigHubContext (
   EFI_PHYSICAL_ADDRESS        PhyAddr;

 

   ASSERT (Xhc->UsbDevContext[SlotId].SlotId != 0);

+  EvtTrb        = NULL;

   InputContext  = Xhc->UsbDevContext[SlotId].InputContext;

   OutputContext = Xhc->UsbDevContext[SlotId].OutputContext;

 

@@ -4158,7 +4181,7 @@ XhcConfigHubContext (
              XHC_GENERIC_TIMEOUT,

              (TRB_TEMPLATE **)(UINTN)&EvtTrb

              );

-  if (EFI_ERROR (Status)) {

+  if (EFI_ERROR (Status) || (EvtTrb == NULL)) {

     DEBUG ((DEBUG_ERROR, "XhcConfigHubContext: Config Endpoint Failed, Status = %r\n", Status));

   }