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

[Marcin Wojtas <mw@semihalf.com>]

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

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> 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