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