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:51, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> 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.
>

... which notably is a PCI device. I need to dig into this further,
I'll try to add more context tomorrow.


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