xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/4] Support xen-driven USB3 debug capability
@ 2021-05-14  0:56 Connor Davis
  2021-05-14  0:56 ` [PATCH v2 1/4] usb: early: Avoid using DbC if already enabled Connor Davis
  2021-05-14  6:46 ` [PATCH v2 0/4] Support xen-driven USB3 debug capability Greg Kroah-Hartman
  0 siblings, 2 replies; 13+ messages in thread
From: Connor Davis @ 2021-05-14  0:56 UTC (permalink / raw)
  To: linux-kernel
  Cc: Connor Davis, linux-usb, xen-devel, Greg Kroah-Hartman,
	Lee Jones, Jann Horn, Chunfeng Yun, Boris Ostrovsky,
	Juergen Gross, Stefano Stabellini, Mathias Nyman,
	Douglas Anderson, Eric W. Biederman, Petr Mladek, Sumit Garg

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 third ensures that xen_dbgp_reset_prep/xen_dbgp_external_startup
return consistent values in failure cases. This inconsistency illustrated
another issue: dbgp_reset_prep returned the value of xen_dbgp_reset_prep
if it was nonzero, but callers of dbgp_reset_prep interpret nonzero
as "keep using the debug port" and would eventually (needlessly) call
dbgp_external_startup. Patch three _should_ fix this issue, but
I don't have any EHCI hardware available to test unfortunately.

The last uses the xen_dbgp_* functions to notify xen of unsafe periods
(e.g. reset and D3hot transition).

Thanks,
Connor

--
Changes since v1:
 - Added patch for dbgp return value fixes
 - Return -EPERM when !xen_initial_domain() in xen_dbgp_op
 - Moved #ifdef-ary out of xhci.c into xhci-dbgcap.h

--
Connor Davis (4):
  usb: early: Avoid using DbC if already enabled
  xen: Export dbgp functions when CONFIG_XEN_DOM0 is enabled
  usb: dbgp: Fix return values for reset prep and startup
  usb: xhci: Notify xen when DbC is unsafe to use

 drivers/usb/early/ehci-dbgp.c  |  9 ++++---
 drivers/usb/early/xhci-dbc.c   | 10 ++++++++
 drivers/usb/host/xhci-dbgcap.h | 19 ++++++++++++++
 drivers/usb/host/xhci.c        | 47 ++++++++++++++++++++++++++++++++++
 drivers/usb/host/xhci.h        |  1 +
 drivers/xen/dbgp.c             |  4 +--
 include/linux/usb/ehci-dbgp.h  | 14 ++++++----
 7 files changed, 94 insertions(+), 10 deletions(-)


base-commit: 88b06399c9c766c283e070b022b5ceafa4f63f19
-- 
2.31.1



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 1/4] usb: early: Avoid using DbC if already enabled
  2021-05-14  0:56 [PATCH v2 0/4] Support xen-driven USB3 debug capability Connor Davis
@ 2021-05-14  0:56 ` Connor Davis
  2021-05-14  0:56   ` [PATCH v2 2/4] xen: Export dbgp functions when CONFIG_XEN_DOM0 is enabled Connor Davis
                     ` (2 more replies)
  2021-05-14  6:46 ` [PATCH v2 0/4] Support xen-driven USB3 debug capability Greg Kroah-Hartman
  1 sibling, 3 replies; 13+ messages in thread
From: Connor Davis @ 2021-05-14  0:56 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Connor Davis, Jann Horn, Lee Jones, Chunfeng Yun, linux-usb,
	linux-kernel, xen-devel

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 <connojdavis@gmail.com>
---
 drivers/usb/early/xhci-dbc.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/usb/early/xhci-dbc.c b/drivers/usb/early/xhci-dbc.c
index be4ecbabdd58..ca67fddc2d36 100644
--- a/drivers/usb/early/xhci-dbc.c
+++ b/drivers/usb/early/xhci-dbc.c
@@ -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



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 2/4] xen: Export dbgp functions when CONFIG_XEN_DOM0 is enabled
  2021-05-14  0:56 ` [PATCH v2 1/4] usb: early: Avoid using DbC if already enabled Connor Davis
@ 2021-05-14  0:56   ` Connor Davis
  2021-05-14 14:06     ` Connor Davis
  2021-05-14  0:56   ` [PATCH v2 3/4] usb: dbgp: Fix return values for reset prep and startup Connor Davis
  2021-05-17  9:32   ` [PATCH v2 1/4] usb: early: Avoid using DbC if already enabled Jan Beulich
  2 siblings, 1 reply; 13+ messages in thread
From: Connor Davis @ 2021-05-14  0:56 UTC (permalink / raw)
  To: Boris Ostrovsky, Juergen Gross, Stefano Stabellini
  Cc: Connor Davis, xen-devel, linux-kernel

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 <connojdavis@gmail.com>
Acked-by: Juergen Gross <jgross@suse.com>
---
 drivers/xen/dbgp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/xen/dbgp.c b/drivers/xen/dbgp.c
index cfb5de31d860..fef32dd1a5dc 100644
--- a/drivers/xen/dbgp.c
+++ b/drivers/xen/dbgp.c
@@ -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



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v2 3/4] usb: dbgp: Fix return values for reset prep and startup
  2021-05-14  0:56 ` [PATCH v2 1/4] usb: early: Avoid using DbC if already enabled Connor Davis
  2021-05-14  0:56   ` [PATCH v2 2/4] xen: Export dbgp functions when CONFIG_XEN_DOM0 is enabled Connor Davis
@ 2021-05-14  0:56   ` Connor Davis
  2021-05-14 15:33     ` Boris Ostrovsky
  2021-05-17  9:32   ` [PATCH v2 1/4] usb: early: Avoid using DbC if already enabled Jan Beulich
  2 siblings, 1 reply; 13+ messages in thread
From: Connor Davis @ 2021-05-14  0:56 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Boris Ostrovsky, Juergen Gross, Stefano Stabellini
  Cc: Connor Davis, Douglas Anderson, Eric W. Biederman, Chunfeng Yun,
	Petr Mladek, Sumit Garg, Lee Jones, linux-usb, linux-kernel,
	xen-devel

Callers of dbgp_reset_prep treat a 0 return value as "stop using
the debug port", which means they don't make any subsequent calls to
dbgp_reset_prep or dbgp_external_startup.

To ensure the callers' interpretation is correct, first return -EPERM
from xen_dbgp_op if !xen_initial_domain(). This ensures that
both xen_dbgp_reset_prep and xen_dbgp_external_startup return 0
iff the PHYSDEVOP_DBGP_RESET_{PREPARE,DONE} hypercalls succeed. Also
update xen_dbgp_reset_prep and xen_dbgp_external_startup to return
-EPERM when !CONFIG_XEN_DOM0 for consistency.

Next, return nonzero from dbgp_reset_prep if xen_dbgp_reset_prep returns
0. The nonzero value ensures that callers of dbgp_reset_prep will
subsequently call dbgp_external_startup when it is safe to do so.

Also invert the return values from dbgp_external_startup for
consistency with dbgp_reset_prep (this inversion has no functional
change since no callers actually check the value).

Signed-off-by: Connor Davis <connojdavis@gmail.com>
---
 drivers/usb/early/ehci-dbgp.c |  9 ++++++---
 drivers/xen/dbgp.c            |  2 +-
 include/linux/usb/ehci-dbgp.h | 14 +++++++++-----
 3 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/early/ehci-dbgp.c b/drivers/usb/early/ehci-dbgp.c
index 45b42d8f6453..ff993d330c01 100644
--- a/drivers/usb/early/ehci-dbgp.c
+++ b/drivers/usb/early/ehci-dbgp.c
@@ -970,8 +970,8 @@ int dbgp_reset_prep(struct usb_hcd *hcd)
 	int ret = xen_dbgp_reset_prep(hcd);
 	u32 ctrl;
 
-	if (ret)
-		return ret;
+	if (!ret)
+		return 1;
 
 	dbgp_not_safe = 1;
 	if (!ehci_debug)
@@ -995,7 +995,10 @@ EXPORT_SYMBOL_GPL(dbgp_reset_prep);
 
 int dbgp_external_startup(struct usb_hcd *hcd)
 {
-	return xen_dbgp_external_startup(hcd) ?: _dbgp_external_startup();
+	if (!xen_dbgp_external_startup(hcd))
+		return 1;
+
+	return !_dbgp_external_startup();
 }
 EXPORT_SYMBOL_GPL(dbgp_external_startup);
 #endif /* USB */
diff --git a/drivers/xen/dbgp.c b/drivers/xen/dbgp.c
index fef32dd1a5dc..d54f98380e6f 100644
--- a/drivers/xen/dbgp.c
+++ b/drivers/xen/dbgp.c
@@ -15,7 +15,7 @@ static int xen_dbgp_op(struct usb_hcd *hcd, int op)
 	struct physdev_dbgp_op dbgp;
 
 	if (!xen_initial_domain())
-		return 0;
+		return -EPERM;
 
 	dbgp.op = op;
 
diff --git a/include/linux/usb/ehci-dbgp.h b/include/linux/usb/ehci-dbgp.h
index 62ab3805172d..c0e98557efc0 100644
--- a/include/linux/usb/ehci-dbgp.h
+++ b/include/linux/usb/ehci-dbgp.h
@@ -56,28 +56,32 @@ extern int xen_dbgp_external_startup(struct usb_hcd *);
 #else
 static inline int xen_dbgp_reset_prep(struct usb_hcd *hcd)
 {
-	return 1; /* Shouldn't this be 0? */
+	return -EPERM;
 }
 
 static inline int xen_dbgp_external_startup(struct usb_hcd *hcd)
 {
-	return -1;
+	return -EPERM;
 }
 #endif
 
 #ifdef CONFIG_EARLY_PRINTK_DBGP
-/* Call backs from ehci host driver to ehci debug driver */
+/*
+ * Call backs from ehci host driver to ehci debug driver.
+ * Returns 0 if the debug port should stopped being used,
+ * nonzero otherwise.
+ */
 extern int dbgp_external_startup(struct usb_hcd *);
 extern int dbgp_reset_prep(struct usb_hcd *);
 #else
 static inline int dbgp_reset_prep(struct usb_hcd *hcd)
 {
-	return xen_dbgp_reset_prep(hcd);
+	return !xen_dbgp_reset_prep(hcd);
 }
 
 static inline int dbgp_external_startup(struct usb_hcd *hcd)
 {
-	return xen_dbgp_external_startup(hcd);
+	return !xen_dbgp_external_startup(hcd);
 }
 #endif
 
-- 
2.31.1



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 0/4] Support xen-driven USB3 debug capability
  2021-05-14  0:56 [PATCH v2 0/4] Support xen-driven USB3 debug capability Connor Davis
  2021-05-14  0:56 ` [PATCH v2 1/4] usb: early: Avoid using DbC if already enabled Connor Davis
@ 2021-05-14  6:46 ` Greg Kroah-Hartman
  2021-05-14 14:07   ` Connor Davis
  1 sibling, 1 reply; 13+ messages in thread
From: Greg Kroah-Hartman @ 2021-05-14  6:46 UTC (permalink / raw)
  To: Connor Davis
  Cc: linux-kernel, linux-usb, xen-devel, Lee Jones, Jann Horn,
	Chunfeng Yun, Boris Ostrovsky, Juergen Gross, Stefano Stabellini,
	Mathias Nyman, Douglas Anderson, Eric W. Biederman, Petr Mladek,
	Sumit Garg

On Thu, May 13, 2021 at 06:56:47PM -0600, Connor Davis wrote:
> 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.

Patch 2/4 does not seem to be showing up anywhere, did it get lost?

thanks,

greg k-h


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 2/4] xen: Export dbgp functions when CONFIG_XEN_DOM0 is enabled
  2021-05-14  0:56   ` [PATCH v2 2/4] xen: Export dbgp functions when CONFIG_XEN_DOM0 is enabled Connor Davis
@ 2021-05-14 14:06     ` Connor Davis
  0 siblings, 0 replies; 13+ messages in thread
From: Connor Davis @ 2021-05-14 14:06 UTC (permalink / raw)
  To: Boris Ostrovsky, Juergen Gross, Stefano Stabellini, Greg Kroah-Hartman
  Cc: xen-devel, linux-kernel, linux-usb

Adding Greg and linux-usb

On 5/13/21 6:56 PM, Connor Davis wrote:
> 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 <connojdavis@gmail.com>
> Acked-by: Juergen Gross <jgross@suse.com>
> ---
>   drivers/xen/dbgp.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/xen/dbgp.c b/drivers/xen/dbgp.c
> index cfb5de31d860..fef32dd1a5dc 100644
> --- a/drivers/xen/dbgp.c
> +++ b/drivers/xen/dbgp.c
> @@ -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);


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 0/4] Support xen-driven USB3 debug capability
  2021-05-14  6:46 ` [PATCH v2 0/4] Support xen-driven USB3 debug capability Greg Kroah-Hartman
@ 2021-05-14 14:07   ` Connor Davis
  0 siblings, 0 replies; 13+ messages in thread
From: Connor Davis @ 2021-05-14 14:07 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-kernel, linux-usb, xen-devel, Lee Jones, Jann Horn,
	Chunfeng Yun, Boris Ostrovsky, Juergen Gross, Stefano Stabellini,
	Mathias Nyman, Douglas Anderson, Eric W. Biederman, Petr Mladek,
	Sumit Garg


On 5/14/21 12:46 AM, Greg Kroah-Hartman wrote:
> On Thu, May 13, 2021 at 06:56:47PM -0600, Connor Davis wrote:
>> 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.
> Patch 2/4 does not seem to be showing up anywhere, did it get lost?

Yep, just added you, sorry about that


Thanks,

Connor

> thanks,
>
> greg k-h


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 3/4] usb: dbgp: Fix return values for reset prep and startup
  2021-05-14  0:56   ` [PATCH v2 3/4] usb: dbgp: Fix return values for reset prep and startup Connor Davis
@ 2021-05-14 15:33     ` Boris Ostrovsky
  0 siblings, 0 replies; 13+ messages in thread
From: Boris Ostrovsky @ 2021-05-14 15:33 UTC (permalink / raw)
  To: Connor Davis, Greg Kroah-Hartman, Juergen Gross, Stefano Stabellini
  Cc: Douglas Anderson, Eric W. Biederman, Chunfeng Yun, Petr Mladek,
	Sumit Garg, Lee Jones, linux-usb, linux-kernel, xen-devel


On 5/13/21 8:56 PM, Connor Davis wrote:
> Callers of dbgp_reset_prep treat a 0 return value as "stop using
> the debug port", which means they don't make any subsequent calls to
> dbgp_reset_prep or dbgp_external_startup.
>
> To ensure the callers' interpretation is correct, first return -EPERM
> from xen_dbgp_op if !xen_initial_domain(). This ensures that
> both xen_dbgp_reset_prep and xen_dbgp_external_startup return 0
> iff the PHYSDEVOP_DBGP_RESET_{PREPARE,DONE} hypercalls succeed. Also
> update xen_dbgp_reset_prep and xen_dbgp_external_startup to return
> -EPERM when !CONFIG_XEN_DOM0 for consistency.
>
> Next, return nonzero from dbgp_reset_prep if xen_dbgp_reset_prep returns
> 0. The nonzero value ensures that callers of dbgp_reset_prep will
> subsequently call dbgp_external_startup when it is safe to do so.
>
> Also invert the return values from dbgp_external_startup for
> consistency with dbgp_reset_prep (this inversion has no functional
> change since no callers actually check the value).
>
> Signed-off-by: Connor Davis <connojdavis@gmail.com>


For Xen bits:


Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>


For the rest it seems to me that error code passing could be improved: if it's 0 or 1 it should be bool. Or pass actual error code, with zero for no-error case, such as ...


> ---
>  drivers/usb/early/ehci-dbgp.c |  9 ++++++---
>  drivers/xen/dbgp.c            |  2 +-
>  include/linux/usb/ehci-dbgp.h | 14 +++++++++-----
>  3 files changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/usb/early/ehci-dbgp.c b/drivers/usb/early/ehci-dbgp.c
> index 45b42d8f6453..ff993d330c01 100644
> --- a/drivers/usb/early/ehci-dbgp.c
> +++ b/drivers/usb/early/ehci-dbgp.c
> @@ -970,8 +970,8 @@ int dbgp_reset_prep(struct usb_hcd *hcd)
>  	int ret = xen_dbgp_reset_prep(hcd);
>  	u32 ctrl;
>  
> -	if (ret)
> -		return ret;
> +	if (!ret)
> +		return 1;


... here or ...


>  
>  	dbgp_not_safe = 1;
>  	if (!ehci_debug)
> @@ -995,7 +995,10 @@ EXPORT_SYMBOL_GPL(dbgp_reset_prep);
>  
>  int dbgp_external_startup(struct usb_hcd *hcd)
>  {
> -	return xen_dbgp_external_startup(hcd) ?: _dbgp_external_startup();
> +	if (!xen_dbgp_external_startup(hcd))
> +		return 1;


... here.


-boris




^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/4] usb: early: Avoid using DbC if already enabled
  2021-05-14  0:56 ` [PATCH v2 1/4] usb: early: Avoid using DbC if already enabled Connor Davis
  2021-05-14  0:56   ` [PATCH v2 2/4] xen: Export dbgp functions when CONFIG_XEN_DOM0 is enabled Connor Davis
  2021-05-14  0:56   ` [PATCH v2 3/4] usb: dbgp: Fix return values for reset prep and startup Connor Davis
@ 2021-05-17  9:32   ` Jan Beulich
  2021-05-17 13:48     ` Connor Davis
  2 siblings, 1 reply; 13+ messages in thread
From: Jan Beulich @ 2021-05-17  9:32 UTC (permalink / raw)
  To: Connor Davis
  Cc: Jann Horn, Lee Jones, Chunfeng Yun, linux-usb, linux-kernel,
	xen-devel, Greg Kroah-Hartman

On 14.05.2021 02:56, Connor Davis wrote:
> 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.

Doesn't this go too far and prevent use even if firmware (perhaps
mistakenly) left it enabled?

Jan

> Signed-off-by: Connor Davis <connojdavis@gmail.com>
> ---
>  drivers/usb/early/xhci-dbc.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/usb/early/xhci-dbc.c b/drivers/usb/early/xhci-dbc.c
> index be4ecbabdd58..ca67fddc2d36 100644
> --- a/drivers/usb/early/xhci-dbc.c
> +++ b/drivers/usb/early/xhci-dbc.c
> @@ -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;
>  }
>  
> 



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/4] usb: early: Avoid using DbC if already enabled
  2021-05-17  9:32   ` [PATCH v2 1/4] usb: early: Avoid using DbC if already enabled Jan Beulich
@ 2021-05-17 13:48     ` Connor Davis
  2021-05-17 14:13       ` Jan Beulich
  0 siblings, 1 reply; 13+ messages in thread
From: Connor Davis @ 2021-05-17 13:48 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Jann Horn, Lee Jones, Chunfeng Yun, linux-usb, linux-kernel,
	xen-devel, Greg Kroah-Hartman


On 5/17/21 3:32 AM, Jan Beulich wrote:
> On 14.05.2021 02:56, Connor Davis wrote:
>> 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.
> Doesn't this go too far and prevent use even if firmware (perhaps
> mistakenly) left it enabled?
>
> Jan

Yes, but how is one supposed to distinguish the broken firmware and 
non-broken

firmware cases?

>
>> Signed-off-by: Connor Davis <connojdavis@gmail.com>
>> ---
>>   drivers/usb/early/xhci-dbc.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/usb/early/xhci-dbc.c b/drivers/usb/early/xhci-dbc.c
>> index be4ecbabdd58..ca67fddc2d36 100644
>> --- a/drivers/usb/early/xhci-dbc.c
>> +++ b/drivers/usb/early/xhci-dbc.c
>> @@ -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;
>>   }
>>   
>>
Thanks,

Connor



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/4] usb: early: Avoid using DbC if already enabled
  2021-05-17 13:48     ` Connor Davis
@ 2021-05-17 14:13       ` Jan Beulich
  2021-05-17 14:24         ` Connor Davis
  0 siblings, 1 reply; 13+ messages in thread
From: Jan Beulich @ 2021-05-17 14:13 UTC (permalink / raw)
  To: Connor Davis
  Cc: Jann Horn, Lee Jones, Chunfeng Yun, linux-usb, linux-kernel,
	xen-devel, Greg Kroah-Hartman

On 17.05.2021 15:48, Connor Davis wrote:
> 
> On 5/17/21 3:32 AM, Jan Beulich wrote:
>> On 14.05.2021 02:56, Connor Davis wrote:
>>> 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.
>> Doesn't this go too far and prevent use even if firmware (perhaps
>> mistakenly) left it enabled?
> 
> Yes, but how is one supposed to distinguish the broken firmware and 
> non-broken
> 
> firmware cases?

Well, a first step might be to only check if running virtualized.
And then if your running virtualized, there might be a way to
inquire the hypervisor?

Jan


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/4] usb: early: Avoid using DbC if already enabled
  2021-05-17 14:13       ` Jan Beulich
@ 2021-05-17 14:24         ` Connor Davis
  2021-05-18 21:50           ` Mathias Nyman
  0 siblings, 1 reply; 13+ messages in thread
From: Connor Davis @ 2021-05-17 14:24 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Jann Horn, Lee Jones, Chunfeng Yun, linux-usb, linux-kernel,
	xen-devel, Greg Kroah-Hartman


On 5/17/21 8:13 AM, Jan Beulich wrote:
> On 17.05.2021 15:48, Connor Davis wrote:
>> On 5/17/21 3:32 AM, Jan Beulich wrote:
>>> On 14.05.2021 02:56, Connor Davis wrote:
>>>> 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.
>>> Doesn't this go too far and prevent use even if firmware (perhaps
>>> mistakenly) left it enabled?
>> Yes, but how is one supposed to distinguish the broken firmware and
>> non-broken
>>
>> firmware cases?
> Well, a first step might be to only check if running virtualized.
> And then if your running virtualized, there might be a way to
> inquire the hypervisor?

Right, but if it was enabled by something other than a hypervisor,

or you're not running virtualized, how do you distinguish then? IMO

the proper thing to do in any case is to simply not use the DbC in linux.

Thanks,

Connor



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v2 1/4] usb: early: Avoid using DbC if already enabled
  2021-05-17 14:24         ` Connor Davis
@ 2021-05-18 21:50           ` Mathias Nyman
  0 siblings, 0 replies; 13+ messages in thread
From: Mathias Nyman @ 2021-05-18 21:50 UTC (permalink / raw)
  To: Connor Davis, Jan Beulich
  Cc: Jann Horn, Lee Jones, Chunfeng Yun, linux-usb, linux-kernel,
	xen-devel, Greg Kroah-Hartman

On 17.5.2021 17.24, Connor Davis wrote:
> 
> On 5/17/21 8:13 AM, Jan Beulich wrote:
>> On 17.05.2021 15:48, Connor Davis wrote:
>>> On 5/17/21 3:32 AM, Jan Beulich wrote:
>>>> On 14.05.2021 02:56, Connor Davis wrote:
>>>>> 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.
>>>> Doesn't this go too far and prevent use even if firmware (perhaps
>>>> mistakenly) left it enabled?
>>> Yes, but how is one supposed to distinguish the broken firmware and
>>> non-broken
>>>
>>> firmware cases?
>> Well, a first step might be to only check if running virtualized.
>> And then if your running virtualized, there might be a way to
>> inquire the hypervisor?
> 
> Right, but if it was enabled by something other than a hypervisor,
> 
> or you're not running virtualized, how do you distinguish then? IMO
> 
> the proper thing to do in any case is to simply not use the DbC in linux.
> 

Sounds reasonable.

We can address "broken firmware" during the xHC handoff from BIOS to OS. 
Only first OS that loads after BIOS should see the 
"HC BIOS owned semaphore" bit set in xHCI MMIO.

If it's set then OS requests ownership, which clears BIOS owned bit.
If DbC is running here we can stop it.

-Mathias


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2021-05-19  5:11 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-14  0:56 [PATCH v2 0/4] Support xen-driven USB3 debug capability Connor Davis
2021-05-14  0:56 ` [PATCH v2 1/4] usb: early: Avoid using DbC if already enabled Connor Davis
2021-05-14  0:56   ` [PATCH v2 2/4] xen: Export dbgp functions when CONFIG_XEN_DOM0 is enabled Connor Davis
2021-05-14 14:06     ` Connor Davis
2021-05-14  0:56   ` [PATCH v2 3/4] usb: dbgp: Fix return values for reset prep and startup Connor Davis
2021-05-14 15:33     ` Boris Ostrovsky
2021-05-17  9:32   ` [PATCH v2 1/4] usb: early: Avoid using DbC if already enabled Jan Beulich
2021-05-17 13:48     ` Connor Davis
2021-05-17 14:13       ` Jan Beulich
2021-05-17 14:24         ` Connor Davis
2021-05-18 21:50           ` Mathias Nyman
2021-05-14  6:46 ` [PATCH v2 0/4] Support xen-driven USB3 debug capability Greg Kroah-Hartman
2021-05-14 14:07   ` Connor Davis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).