Re: [PATCH] MdeModulePkg XhciDxe: Set HSEE Bit if SERR# Enable Bit is set

[Andrew Fish <afish@apple.com>]

> On Sep 25, 2018, at 8:41 AM, Marcin Wojtas <mw@semihalf.com> wrote:
> 
> Hi Star, Ard
> 
> With this patch, my platforms which use NonDiscoverableDevices layer
> for supporting generic Xhci controller, fail in a strange way:
> "Synchronous Exception at 0x000000003F910AFC
> PC 0x00003F910AFC (0x00003F908000+0x00008AFC) [ 0] DxeCore.dll
> PC 0x00003F910AE0 (0x00003F908000+0x00008AE0) [ 0] DxeCore.dll
> PC 0x00003F91BDF4 (0x00003F908000+0x00013DF4) [ 0] DxeCore.dll
> PC 0x0000BF5BD000 (0x0000BF5AF000+0x0000E000) [ 1] XhciDxe.dll
> PC 0xAFAFAFAFAFAFAFAF
> 

Marcin,

A lot of times 0xAFAFAFAFAFAFAFAF is related to using freed memory. 

See this PCD and its use in the DebugLib:
MdePkg.dec:2168:  gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue|0xAF|UINT8|0x00000008

On an X86 system 0xAFAFAFAFAFAFAFAF is a non canonical address and it will generate a general Protection Fault exception 13 (0xD) on access. 

Thanks,

Andrew Fish


> Recursive exception occurred while dumping the CPU state"
> 
> I've quickly checked and although XhcSetHsee() is eventually called
> from XhcDriverBindingStart() sequence,
> below line is not even executed:
> XhcSetOpRegBit (Xhc, XHC_USBCMD_OFFSET, XHC_USBCMD_HSEE);
> The XhcDriverBindingStart() returns EFI_SUCCESS and we get the sync
> abort right afterwards (haven't found exact place yet).
> 
> What makes the difference is commenting out in XhcSetHsee():
> //  Status = PciIo->Pci.Read (
> //                        PciIo,
> //                       EfiPciIoWidthUint16,
> //                        PCI_COMMAND_OFFSET,
> //                       sizeof (XhciCmd),
> //                        &XhciCmd
> //                        );
> 
> With that everything keeps working as usual. I'd appreciate any hint.
> 
> Best regards.
> Marcin
> 
> wt., 11 wrz 2018 o 04:30 Ni, Ruiyu <ruiyu.ni@intel.com <mailto:ruiyu.ni@intel.com>> napisał(a):
>> 
>> Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
>> 
>> Thanks/Ray
>> 
>>> -----Original Message-----
>>> From: Zeng, Star
>>> Sent: Tuesday, September 11, 2018 10:04 AM
>>> To: edk2-devel@lists.01.org
>>> Cc: Zeng, Star <star.zeng@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Wang,
>>> Jian J <jian.j.wang@intel.com>; Wang, Fei1 <fei1.wang@intel.com>
>>> Subject: [PATCH] MdeModulePkg XhciDxe: Set HSEE Bit if SERR# Enable Bit is
>>> set
>>> 
>>> When the HSEE in the USBCMD bit is a '1' and the HSE bit in the USBSTS
>>> register is a '1', the xHC shall assert out-of-band error signaling to the host
>>> and assert the SERR# pin.
>>> To prevent masking any potential issues with SERR, this patch is to set
>>> USBCMD Host System Error Enable(HSEE) Bit if PCICMD SERR# Enable Bit is
>>> set.
>>> 
>>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>>> Cc: Jian J Wang <jian.j.wang@intel.com>
>>> Cc: Fei1 Wang <fei1.wang@intel.com>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Star Zeng <star.zeng@intel.com>
>>> ---
>>> MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c | 41
>>> ++++++++++++++++++++++++++++++++++
>>> 1 file changed, 41 insertions(+)
>>> 
>>> diff --git a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
>>> b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
>>> index 5f0736a516b6..89f073e1d83f 100644
>>> --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
>>> +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c
>>> @@ -587,6 +587,39 @@ XhcIsSysError (
>>> }
>>> 
>>> /**
>>> +  Set USBCMD Host System Error Enable(HSEE) Bit if PCICMD SERR# Enable
>>> Bit is set.
>>> +
>>> +  The USBCMD HSEE Bit will be reset to default 0 by USBCMD Host Controller
>>> Reset(HCRST).
>>> +  This function is to set USBCMD HSEE Bit if PCICMD SERR# Enable Bit is set.
>>> +
>>> +  @param Xhc            The XHCI Instance.
>>> +
>>> +**/
>>> +VOID
>>> +XhcSetHsee (
>>> +  IN USB_XHCI_INSTANCE  *Xhc
>>> +  )
>>> +{
>>> +  EFI_STATUS            Status;
>>> +  EFI_PCI_IO_PROTOCOL   *PciIo;
>>> +  UINT16                XhciCmd;
>>> +
>>> +  PciIo = Xhc->PciIo;
>>> +  Status = PciIo->Pci.Read (
>>> +                        PciIo,
>>> +                        EfiPciIoWidthUint16,
>>> +                        PCI_COMMAND_OFFSET,
>>> +                        sizeof (XhciCmd),
>>> +                        &XhciCmd
>>> +                        );
>>> +  if (!EFI_ERROR (Status)) {
>>> +    if ((XhciCmd & EFI_PCI_COMMAND_SERR) != 0) {
>>> +      XhcSetOpRegBit (Xhc, XHC_USBCMD_OFFSET, XHC_USBCMD_HSEE);
>>> +    }
>>> +  }
>>> +}
>>> +
>>> +/**
>>>   Reset the XHCI host controller.
>>> 
>>>   @param  Xhc          The XHCI Instance.
>>> @@ -628,6 +661,14 @@ XhcResetHC (
>>>     //
>>>     gBS->Stall (XHC_1_MILLISECOND);
>>>     Status = XhcWaitOpRegBit (Xhc, XHC_USBCMD_OFFSET,
>>> XHC_USBCMD_RESET, FALSE, Timeout);
>>> +
>>> +    if (!EFI_ERROR (Status)) {
>>> +      //
>>> +      // The USBCMD HSEE Bit will be reset to default 0 by USBCMD HCRST.
>>> +      // Set USBCMD HSEE Bit if PCICMD SERR# Enable Bit is set.
>>> +      //
>>> +      XhcSetHsee (Xhc);
>>> +    }
>>>   }
>>> 
>>>   return Status;
>>> --
>>> 2.7.0.windows.1
>> 
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org <mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel <https://lists.01.org/mailman/listinfo/edk2-devel>