Skip to contentSkip to conversation

Support xen-driven USB3 debug capability

PATCH Superseded Connor Davis 4 participants 11 messages May 12, 2021

Subsystems (3)
USB Xen hypervisor interface USB xHCI driver

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Connor Davis

author
#0 lore raw

cover Support xen-driven USB3 debug capability

Hi all,

This goal of this series is to allow the USB3 debug capability (DbC) to be safely used by xen while linux runs as dom0. The first patch prevents the early DbC driver from using an already-running DbC. The second exports xen_dbgp_reset_prep and xen_dbgp_external_startup functions when CONFIG_XEN_DOM0 is enabled so they may be used by the xHCI driver. The last uses those functions to notify xen of unsafe periods (e.g. reset and D3hot transition).

Thanks,
Connor

--
Connor Davis (3):
usb: early: Avoid using DbC if already enabled
xen: Export dbgp functions when CONFIG_XEN_DOM0 is enabled
usb: xhci: Notify xen when DbC is unsafe to use

 drivers/usb/early/xhci-dbc.c   | 10 ++++++
 drivers/usb/host/xhci-dbgcap.h |  6 ++++
 drivers/usb/host/xhci.c        | 57 ++++++++++++++++++++++++++++++++++
 drivers/usb/host/xhci.h        |  1 +
 drivers/xen/dbgp.c             |  2 +-
 5 files changed, 75 insertions(+), 1 deletion(-)

base-commit: 88b06399c9c766c283e070b022b5ceafa4f63f19 --
2.31.1

author
#1 lore raw

PATCH 1/3 usb: early: Avoid using DbC if already enabled

Check if the debug capability is enabled in early_xdbc_parse_parameter, and if it is, return with an error. This avoids collisions with whatever enabled the DbC prior to linux starting.

Signed-off-by Connor Davis <[email protected]>
---
 drivers/usb/early/xhci-dbc.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
drivers/usb/early/xhci-dbc.c +10 -0
index be4ecbabdd58..ca67fddc2d36 100644
@@ -642,6 +642,16 @@ int __init early_xdbc_parse_parameter(char *s)
 	}
 	xdbc.xdbc_reg = (struct xdbc_regs __iomem *)(xdbc.xhci_base + offset);
 
+	if (readl(&xdbc.xdbc_reg->control) & CTRL_DBC_ENABLE) {
+		pr_notice("xhci debug capability already in use\n");
+		early_iounmap(xdbc.xhci_base, xdbc.xhci_length);
+		xdbc.xdbc_reg = NULL;
+		xdbc.xhci_base = NULL;
+		xdbc.xhci_length = 0;
+
+		return -ENODEV;
+	}
+
 	return 0;
 }

--
2.31.1

author
#2 lore raw

PATCH 2/3 xen: Export dbgp functions when CONFIG_XEN_DOM0 is enabled

In reply to Connor Davis

Export xen_dbgp_reset_prep and xen_dbgp_external_startup when CONFIG_XEN_DOM0 is defined. This allows use of these symbols even if CONFIG_EARLY_PRINK_DBGP is defined.

Signed-off-by Connor Davis <[email protected]>
---
 drivers/xen/dbgp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/xen/dbgp.c +1 -1
index cfb5de31d860..fef32dd1a5dc 100644
@@ -44,7 +44,7 @@ int xen_dbgp_external_startup(struct usb_hcd *hcd)
 	return xen_dbgp_op(hcd, PHYSDEVOP_DBGP_RESET_DONE);
 }
 
-#ifndef CONFIG_EARLY_PRINTK_DBGP
+#if defined(CONFIG_XEN_DOM0) || !defined(CONFIG_EARLY_PRINTK_DBGP)
 #include <linux/export.h>
 EXPORT_SYMBOL_GPL(xen_dbgp_reset_prep);
 EXPORT_SYMBOL_GPL(xen_dbgp_external_startup);

--
2.31.1

maintainer Acked-by on v1 2/3
#3 lore raw

Re: [PATCH 2/3] xen: Export dbgp functions when CONFIG_XEN_DOM0 is enabled

Quoted 5 lines · Connor Davis

Export xen_dbgp_reset_prep and xen_dbgp_external_startup when CONFIG_XEN_DOM0 is defined. This allows use of these symbols even if CONFIG_EARLY_PRINK_DBGP is defined.

Signed-off-by Connor Davis <[email protected]>
Acked-by Juergen Gross <[email protected]>

Juergen

author
#4 lore raw

Re: [PATCH 2/3] xen: Export dbgp functions when CONFIG_XEN_DOM0 is enabled

Quoted 8 lines · Juergen Gross
Quoted 5 lines · Connor Davis

Export xen_dbgp_reset_prep and xen_dbgp_external_startup when CONFIG_XEN_DOM0 is defined. This allows use of these symbols even if CONFIG_EARLY_PRINK_DBGP is defined.

Signed-off-by Connor Davis <[email protected]>
Acked-by Juergen Gross <[email protected]>

Thank you.

- Connor

#5 lore raw

Re: [PATCH 2/3] xen: Export dbgp functions when CONFIG_XEN_DOM0 is enabled

In reply to Connor Davis
Quoted 7 lines · Connor Davis

Export xen_dbgp_reset_prep and xen_dbgp_external_startup when CONFIG_XEN_DOM0 is defined. This allows use of these symbols even if CONFIG_EARLY_PRINK_DBGP is defined.

Signed-off-by Connor Davis <[email protected]>
---
 drivers/xen/dbgp.c | 2 +-

Unrelated to your patch but since you are fixing things around that file --- should we return -EPERM in xen_dbgp_op() when !xen_initial_domain()?

-boris

author
#6 lore raw

Re: [PATCH 2/3] xen: Export dbgp functions when CONFIG_XEN_DOM0 is enabled

Quoted 6 lines · Boris Ostrovsky
Quoted 4 lines · Connor Davis

Export xen_dbgp_reset_prep and xen_dbgp_external_startup when CONFIG_XEN_DOM0 is defined. This allows use of these symbols even if CONFIG_EARLY_PRINK_DBGP is defined.

Quoted 6 lines
Quoted 3 lines
Signed-off-by Connor Davis <[email protected]>
---
 drivers/xen/dbgp.c | 2 +-

Unrelated to your patch but since you are fixing things around that file --- should we return -EPERM in xen_dbgp_op() when !xen_initial_domain()?

Yeah looks like it. That would make patch 3 simpler too. Do you want me to add a patch that fixes that up?

Quoted 3 lines

-boris

Thanks,
Connor

#7 lore raw

Re: [PATCH 2/3] xen: Export dbgp functions when CONFIG_XEN_DOM0 is enabled

Quoted 4 lines · Connor Davis
Quoted 1 line · Boris Ostrovsky

Unrelated to your patch but since you are fixing things around that file --- should we return -EPERM in xen_dbgp_op() when !xen_initial_domain()?

Yeah looks like it. That would make patch 3 simpler too. Do you want me to add a patch that fixes that up?

Yes, please.

-boris

author
#8 lore raw

PATCH 3/3 usb: xhci: Notify xen when DbC is unsafe to use

In reply to Connor Davis

When running as a dom0 guest on Xen, check if the USB3 debug capability is enabled before xHCI reset, suspend, and resume. If it is, call xen_dbgp_reset_prep() to notify Xen that it is unsafe to touch MMIO registers until the next xen_dbgp_external_startup().

This notification allows Xen to avoid undefined behavior resulting from MMIO access when the host controller's CNR bit is set or when the device transitions to D3hot.

Signed-off-by Connor Davis <[email protected]>
---
 drivers/usb/host/xhci-dbgcap.h |  6 ++++
 drivers/usb/host/xhci.c        | 57 ++++++++++++++++++++++++++++++++++
 drivers/usb/host/xhci.h        |  1 +
 3 files changed, 64 insertions(+)
drivers/usb/host/xhci-dbgcap.h +6 -0
index c70b78d504eb..24784b82a840 100644
@@ -227,4 +227,10 @@ static inline int xhci_dbc_resume(struct xhci_hcd *xhci)
 	return 0;
 }
 #endif /* CONFIG_USB_XHCI_DBGCAP */
+
+#ifdef CONFIG_XEN_DOM0
+int xen_dbgp_reset_prep(struct usb_hcd *hcd);
+int xen_dbgp_external_startup(struct usb_hcd *hcd);
+#endif /* CONFIG_XEN_DOM0 */
+
 #endif /* __LINUX_XHCI_DBGCAP_H */
drivers/usb/host/xhci.c +57 -0

87 lines not on this page · raw message

drivers/usb/host/xhci.h +1 -0
index 2595a8f057c4..61d8efc9eef2 100644
@@ -1920,6 +1920,7 @@ struct xhci_hcd {
 	struct list_head	regset_list;
 
 	void			*dbc;
+	int			external_dbc;
 	/* platform-specific data -- must come last */
 	unsigned long		priv[] __aligned(sizeof(s64));
 };

--
2.31.1

maintainer
#9 lore raw

Re: [PATCH 3/3] usb: xhci: Notify xen when DbC is unsafe to use

Quoted 36 lines · Connor Davis

When running as a dom0 guest on Xen, check if the USB3 debug capability is enabled before xHCI reset, suspend, and resume. If it is, call xen_dbgp_reset_prep() to notify Xen that it is unsafe to touch MMIO registers until the next xen_dbgp_external_startup().

This notification allows Xen to avoid undefined behavior resulting from MMIO access when the host controller's CNR bit is set or when the device transitions to D3hot.

Signed-off-by Connor Davis <[email protected]>
---
 drivers/usb/host/xhci-dbgcap.h |  6 ++++
 drivers/usb/host/xhci.c        | 57 ++++++++++++++++++++++++++++++++++
 drivers/usb/host/xhci.h        |  1 +
 3 files changed, 64 insertions(+)
drivers/usb/host/xhci-dbgcap.h +6 -0
index c70b78d504eb..24784b82a840 100644
@@ -227,4 +227,10 @@ static inline int xhci_dbc_resume(struct xhci_hcd *xhci)
 	return 0;
 }
 #endif /* CONFIG_USB_XHCI_DBGCAP */
+
+#ifdef CONFIG_XEN_DOM0
+int xen_dbgp_reset_prep(struct usb_hcd *hcd);
+int xen_dbgp_external_startup(struct usb_hcd *hcd);
+#endif /* CONFIG_XEN_DOM0 */
+
 #endif /* __LINUX_XHCI_DBGCAP_H */
drivers/usb/host/xhci.c +0 -0
index ca9385d22f68..afe44169183f 100644

drivers/usb/host/xhci.c static unsigned long long quirks; · line 41

Quoted 5 lines
drivers/usb/host/xhci.c +2 -0
@@ -37,6 +37,57 @@ static unsigned long long quirks;
 module_param(quirks, ullong, S_IRUGO);
 MODULE_PARM_DESC(quirks, "Bit flags for quirks to be enabled as default");
 
+#ifdef CONFIG_XEN_DOM0
+#include <xen/xen.h>

<snip>

Can't this #ifdef stuff go into a .h file?

thanks,

greg k-h

author
#10 lore raw

Re: [PATCH 3/3] usb: xhci: Notify xen when DbC is unsafe to use

Quoted 40 lines · Greg Kroah-Hartman
Quoted 39 lines · Connor Davis

When running as a dom0 guest on Xen, check if the USB3 debug capability is enabled before xHCI reset, suspend, and resume. If it is, call xen_dbgp_reset_prep() to notify Xen that it is unsafe to touch MMIO registers until the next xen_dbgp_external_startup().

This notification allows Xen to avoid undefined behavior resulting from MMIO access when the host controller's CNR bit is set or when the device transitions to D3hot.

Signed-off-by Connor Davis <[email protected]>
---
 drivers/usb/host/xhci-dbgcap.h |  6 ++++
 drivers/usb/host/xhci.c        | 57 ++++++++++++++++++++++++++++++++++
 drivers/usb/host/xhci.h        |  1 +
 3 files changed, 64 insertions(+)
drivers/usb/host/xhci-dbgcap.h +6 -0
index c70b78d504eb..24784b82a840 100644
@@ -227,4 +227,10 @@ static inline int xhci_dbc_resume(struct xhci_hcd *xhci)
 	return 0;
 }
 #endif /* CONFIG_USB_XHCI_DBGCAP */
+
+#ifdef CONFIG_XEN_DOM0
+int xen_dbgp_reset_prep(struct usb_hcd *hcd);
+int xen_dbgp_external_startup(struct usb_hcd *hcd);
+#endif /* CONFIG_XEN_DOM0 */
+
 #endif /* __LINUX_XHCI_DBGCAP_H */
drivers/usb/host/xhci.c +0 -0
index ca9385d22f68..afe44169183f 100644
@@ -37,6 +37,57 @@ static unsigned long long quirks;
 module_param(quirks, ullong, S_IRUGO);
 MODULE_PARM_DESC(quirks, "Bit flags for quirks to be enabled as default");
Quoted 7 lines

drivers/usb/host/xhci.c static unsigned long long quirks; · line 41

Quoted 2 lines
drivers/usb/host/xhci.c +2 -0
+#ifdef CONFIG_XEN_DOM0
+#include <xen/xen.h>

<snip>

Can't this #ifdef stuff go into a .h file?

Yep, will clean that up in v2.

Quoted 3 lines

thanks,

greg k-h

Thanks,
Connor