* [PATCH] MdeModulePkg XhciDxe: Set HSEE Bit if SERR# Enable Bit is set @ 2018-09-11 2:03 Star Zeng 2018-09-11 2:23 ` Ni, Ruiyu 0 siblings, 1 reply; 7+ messages in thread From: Star Zeng @ 2018-09-11 2:03 UTC (permalink / raw) To: edk2-devel; +Cc: Star Zeng, Ruiyu Ni, Jian J Wang, Fei1 Wang 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] MdeModulePkg XhciDxe: Set HSEE Bit if SERR# Enable Bit is set 2018-09-11 2:03 [PATCH] MdeModulePkg XhciDxe: Set HSEE Bit if SERR# Enable Bit is set Star Zeng @ 2018-09-11 2:23 ` Ni, Ruiyu 2018-09-25 15:41 ` Marcin Wojtas 0 siblings, 1 reply; 7+ messages in thread From: Ni, Ruiyu @ 2018-09-11 2:23 UTC (permalink / raw) To: Zeng, Star, edk2-devel@lists.01.org; +Cc: Wang, Jian J, Wang, Fei1 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] MdeModulePkg XhciDxe: Set HSEE Bit if SERR# Enable Bit is set 2018-09-11 2:23 ` Ni, Ruiyu @ 2018-09-25 15:41 ` Marcin Wojtas 2018-09-25 15:51 ` Ard Biesheuvel 2018-09-25 16:25 ` Andrew Fish 0 siblings, 2 replies; 7+ messages in thread From: Marcin Wojtas @ 2018-09-25 15:41 UTC (permalink / raw) To: Zeng, Star, Ard Biesheuvel Cc: edk2-devel-01, Ni, Ruiyu, fei1.wang, Grzegorz Jaszczyk, nadavh 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] MdeModulePkg XhciDxe: Set HSEE Bit if SERR# Enable Bit is set 2018-09-25 15:41 ` Marcin Wojtas @ 2018-09-25 15:51 ` Ard Biesheuvel 2018-09-25 16:19 ` Ard Biesheuvel 2018-09-25 16:25 ` Andrew Fish 1 sibling, 1 reply; 7+ messages in thread From: Ard Biesheuvel @ 2018-09-25 15:51 UTC (permalink / raw) To: Marcin Wojtas Cc: Zeng, Star, edk2-devel@lists.01.org, Ruiyu Ni, fei1.wang, Grzegorz Jaszczyk, Nadav Haklai 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] MdeModulePkg XhciDxe: Set HSEE Bit if SERR# Enable Bit is set 2018-09-25 15:51 ` Ard Biesheuvel @ 2018-09-25 16:19 ` Ard Biesheuvel 0 siblings, 0 replies; 7+ messages in thread From: Ard Biesheuvel @ 2018-09-25 16:19 UTC (permalink / raw) To: Marcin Wojtas Cc: Zeng, Star, edk2-devel@lists.01.org, Ruiyu Ni, fei1.wang, Grzegorz Jaszczyk, Nadav Haklai 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] MdeModulePkg XhciDxe: Set HSEE Bit if SERR# Enable Bit is set 2018-09-25 15:41 ` Marcin Wojtas 2018-09-25 15:51 ` Ard Biesheuvel @ 2018-09-25 16:25 ` Andrew Fish 2018-09-25 17:29 ` Marcin Wojtas 1 sibling, 1 reply; 7+ messages in thread From: Andrew Fish @ 2018-09-25 16:25 UTC (permalink / raw) To: Marcin Wojtas Cc: Zeng, Star, Ard Biesheuvel, Ni, Ruiyu, Grzegorz Jaszczyk, edk2-devel-01, nadavh, fei1.wang > 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> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] MdeModulePkg XhciDxe: Set HSEE Bit if SERR# Enable Bit is set 2018-09-25 16:25 ` Andrew Fish @ 2018-09-25 17:29 ` Marcin Wojtas 0 siblings, 0 replies; 7+ messages in thread From: Marcin Wojtas @ 2018-09-25 17:29 UTC (permalink / raw) To: Andrew Fish Cc: Zeng, Star, Ard Biesheuvel, Ni, Ruiyu, Grzegorz Jaszczyk, edk2-devel-01, nadavh, fei1.wang Hi Andrew, wt., 25 wrz 2018 o 18:25 Andrew Fish <afish@apple.com> napisał(a): > > > > 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 for the hint. I found a root cause of this behavior. With EfiPciIoWidthUint16 and sizeof (XhciCmd), we read in fact 4 bytes from &XhciCmd, which is UINT16... We can fix it in a couple of ways, but I'll create bugzilla entry and submit below fix later today: --- a/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c +++ b/MdeModulePkg/Bus/Pci/XhciDxe/XhciReg.c @@ -607,7 +607,7 @@ XhcSetHsee ( PciIo = Xhc->PciIo; Status = PciIo->Pci.Read ( PciIo, - EfiPciIoWidthUint16, + EfiPciIoWidthUint8, PCI_COMMAND_OFFSET, sizeof (XhciCmd), &XhciCmd Best regards, Marcin > > > 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 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel > > ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-09-25 17:29 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-09-11 2:03 [PATCH] MdeModulePkg XhciDxe: Set HSEE Bit if SERR# Enable Bit is set Star Zeng 2018-09-11 2:23 ` Ni, Ruiyu 2018-09-25 15:41 ` Marcin Wojtas 2018-09-25 15:51 ` Ard Biesheuvel 2018-09-25 16:19 ` Ard Biesheuvel 2018-09-25 16:25 ` Andrew Fish 2018-09-25 17:29 ` Marcin Wojtas
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox