| Message ID | d160cee9b61c0ec41c2cd5ff9b4e107011d39d8c.1620952511.git.connojdavis@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Support xen-driven USB3 debug capability | expand |
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);
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; > } > >
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
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
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
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
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; }
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(+)