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

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On Tue, 25 Sep 2018 at 17:41, 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
>
> 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).
>

FYI I am seeing something similar with a Renesas uPD70201.

> 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