[PATCH] drivers/char: add timeout for XHCI console cable

[PATCH] drivers/char: add timeout for XHCI console cable

[Marek Marczykowski-Górecki]

Do not wait indefinitely for XHCI to configure console connection. If
the cable is not plugged in, timeout after about 1s and continue boot.
If the cable is plugged in later, the console will be activated then
(see final part of dbc_ensure_running()). When timeout occurs, print a
warning message - it will be saved in the console ring buffer, to be
later retrieved from dom0, or possibly later printed when the cable is
finally connected. But, do not print the warning when waiting after
controller reset, as that may have been called from from console
dbc_putc() and printing here would result in (possibly infinite)
recursion.

Try the timeout to be about 1s, which is way above time needed for
configuring already connected cable. But, since this part runs very
early in the Xen startup, time is not calibrated yet, and functions like
NOW() do not work yet. Use raw TSC and estimate needed time to be at
least 1s on a fast CPU (5GHz), and possibly longer on a slower one. This
is not very accurate method, but fortunately it doesn't need to be.

This also helps with similar situation on resuming from suspend - if
cable got unplugged in the meantime, system will not hang waiting for
the console anymore. And finally, on some systems, when dom0 is also
allowed to access the USB controller, it may put the device into low
power mode that in some older controllers (for example Kaby Lake)
disables also debug capability. In such a case, adding a timeout allows
the system to resume, then dom0 can wake up the controller and Xen will
resume XHCI console.

Signed-off-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
---
 xen/drivers/char/xhci-dbc.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/xen/drivers/char/xhci-dbc.c b/xen/drivers/char/xhci-dbc.c
index d85c8cd6c36a..3d02bda97999 100644
--- a/xen/drivers/char/xhci-dbc.c
+++ b/xen/drivers/char/xhci-dbc.c
@@ -851,9 +851,15 @@ static void dbc_reset_debug_port(struct dbc *dbc)
     }
 }
 
-static void dbc_enable_dbc(struct dbc *dbc)
+static bool dbc_enable_dbc(struct dbc *dbc)
 {
     struct dbc_reg *reg = dbc->dbc_reg;
+    s_time_t start;
+    /*
+     * Too early for proper time calibration, assume a 5GHz CPU and let it wait
+     * about 1s, with still reasonable timeout on a slower CPU.
+     */
+    unsigned long timeout_ticks = 5UL*1000*1000*1000;
 
     wmb();
     writel(readl(&reg->ctrl) | (1U << DBC_CTRL_DCE), &reg->ctrl);
@@ -870,8 +876,12 @@ static void dbc_enable_dbc(struct dbc *dbc)
     writel(readl(&reg->portsc) | (1U << DBC_PSC_PED), &reg->portsc);
     wmb();
 
-    while ( (readl(&reg->ctrl) & (1U << DBC_CTRL_DCR)) == 0 )
+    start = rdtsc_ordered();
+    while ( (readl(&reg->ctrl) & (1U << DBC_CTRL_DCR)) == 0 &&
+            rdtsc_ordered() - start < timeout_ticks )
         cpu_relax();
+
+    return readl(&reg->ctrl) & (1U << DBC_CTRL_DCR);
 }
 
 static void dbc_disable_dbc(struct dbc *dbc)
@@ -970,7 +980,8 @@ static bool __init dbc_open(struct dbc *dbc)
 
     dbc_init_work_ring(dbc, &dbc->dbc_owork);
     dbc_init_work_ring(dbc, &dbc->dbc_iwork);
-    dbc_enable_dbc(dbc);
+    if ( !dbc_enable_dbc(dbc) )
+        printk(XENLOG_WARNING "Timeout waiting for XHCI console cable\n");
     dbc->open = true;
 
     return true;
@@ -1307,7 +1318,8 @@ static void cf_check dbc_uart_resume(struct serial_port *port)
         return;
     }
 
-    dbc_enable_dbc(dbc);
+    if ( !dbc_enable_dbc(dbc) )
+        printk(XENLOG_WARNING "Timeout waiting for XHCI console cable\n");
     dbc->suspended = false;
     dbc_flush(dbc, &dbc->dbc_oring, &dbc->dbc_owork);
     set_timer(&uart->timer, NOW() + MICROSECS(DBC_POLL_INTERVAL));
-- 
2.54.0

Re: [PATCH] drivers/char: add timeout for XHCI console cable

[Jan Beulich]

On 08.10.2026 17:05, Marek Marczykowski-Górecki wrote:
> Do not wait indefinitely for XHCI to configure console connection. If
> the cable is not plugged in, timeout after about 1s and continue boot.
> If the cable is plugged in later, the console will be activated then
> (see final part of dbc_ensure_running()). When timeout occurs, print a
> warning message - it will be saved in the console ring buffer, to be
> later retrieved from dom0, or possibly later printed when the cable is
> finally connected. But, do not print the warning when waiting after
> controller reset, as that may have been called from from console
> dbc_putc() and printing here would result in (possibly infinite)
> recursion.
> 
> Try the timeout to be about 1s, which is way above time needed for
> configuring already connected cable. But, since this part runs very
> early in the Xen startup, time is not calibrated yet, and functions like
> NOW() do not work yet. Use raw TSC and estimate needed time to be at
> least 1s on a fast CPU (5GHz), and possibly longer on a slower one. This
> is not very accurate method, but fortunately it doesn't need to be.

A little fragile, but well...

> --- a/xen/drivers/char/xhci-dbc.c
> +++ b/xen/drivers/char/xhci-dbc.c
> @@ -851,9 +851,15 @@ static void dbc_reset_debug_port(struct dbc *dbc)
>      }
>  }
>  
> -static void dbc_enable_dbc(struct dbc *dbc)
> +static bool dbc_enable_dbc(struct dbc *dbc)
>  {
>      struct dbc_reg *reg = dbc->dbc_reg;
> +    s_time_t start;
> +    /*
> +     * Too early for proper time calibration, assume a 5GHz CPU and let it wait
> +     * about 1s, with still reasonable timeout on a slower CPU.
> +     */
> +    unsigned long timeout_ticks = 5UL*1000*1000*1000;

Even if right now we don't use the driver in any 32-bit env, I think we
better wouldn't deliberately break such a possible use. IOW unsigned
long isn't a suitable type to use here.

Also, nit (style): Blanks please around *.

> @@ -870,8 +876,12 @@ static void dbc_enable_dbc(struct dbc *dbc)
>      writel(readl(&reg->portsc) | (1U << DBC_PSC_PED), &reg->portsc);
>      wmb();
>  
> -    while ( (readl(&reg->ctrl) & (1U << DBC_CTRL_DCR)) == 0 )
> +    start = rdtsc_ordered();
> +    while ( (readl(&reg->ctrl) & (1U << DBC_CTRL_DCR)) == 0 &&
> +            rdtsc_ordered() - start < timeout_ticks )

Urgh - now the driver's truly becoming x86-specific. Is there a reason
this cannot be get_cycles()?

Jan

Re: [PATCH] drivers/char: add timeout for XHCI console cable

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 2896 bytes --]

On Thu, Oct 08, 2026 at 05:54:53PM +0200, Jan Beulich wrote:
> On 08.10.2026 17:05, Marek Marczykowski-Górecki wrote:
> > Do not wait indefinitely for XHCI to configure console connection. If
> > the cable is not plugged in, timeout after about 1s and continue boot.
> > If the cable is plugged in later, the console will be activated then
> > (see final part of dbc_ensure_running()). When timeout occurs, print a
> > warning message - it will be saved in the console ring buffer, to be
> > later retrieved from dom0, or possibly later printed when the cable is
> > finally connected. But, do not print the warning when waiting after
> > controller reset, as that may have been called from from console
> > dbc_putc() and printing here would result in (possibly infinite)
> > recursion.
> > 
> > Try the timeout to be about 1s, which is way above time needed for
> > configuring already connected cable. But, since this part runs very
> > early in the Xen startup, time is not calibrated yet, and functions like
> > NOW() do not work yet. Use raw TSC and estimate needed time to be at
> > least 1s on a fast CPU (5GHz), and possibly longer on a slower one. This
> > is not very accurate method, but fortunately it doesn't need to be.
> 
> A little fragile, but well...
> 
> > --- a/xen/drivers/char/xhci-dbc.c
> > +++ b/xen/drivers/char/xhci-dbc.c
> > @@ -851,9 +851,15 @@ static void dbc_reset_debug_port(struct dbc *dbc)
> >      }
> >  }
> >  
> > -static void dbc_enable_dbc(struct dbc *dbc)
> > +static bool dbc_enable_dbc(struct dbc *dbc)
> >  {
> >      struct dbc_reg *reg = dbc->dbc_reg;
> > +    s_time_t start;
> > +    /*
> > +     * Too early for proper time calibration, assume a 5GHz CPU and let it wait
> > +     * about 1s, with still reasonable timeout on a slower CPU.
> > +     */
> > +    unsigned long timeout_ticks = 5UL*1000*1000*1000;
> 
> Even if right now we don't use the driver in any 32-bit env, I think we
> better wouldn't deliberately break such a possible use. IOW unsigned
> long isn't a suitable type to use here.

So, uint64_t?

> Also, nit (style): Blanks please around *.
> 
> > @@ -870,8 +876,12 @@ static void dbc_enable_dbc(struct dbc *dbc)
> >      writel(readl(&reg->portsc) | (1U << DBC_PSC_PED), &reg->portsc);
> >      wmb();
> >  
> > -    while ( (readl(&reg->ctrl) & (1U << DBC_CTRL_DCR)) == 0 )
> > +    start = rdtsc_ordered();
> > +    while ( (readl(&reg->ctrl) & (1U << DBC_CTRL_DCR)) == 0 &&
> > +            rdtsc_ordered() - start < timeout_ticks )
> 
> Urgh - now the driver's truly becoming x86-specific. Is there a reason
> this cannot be get_cycles()?

Sure, it can be get_cycles(). I just need it to not try to apply scaling
that works only after calibration, but indeed get_cycles() should work.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] drivers/char: add timeout for XHCI console cable

[Jan Beulich]

On 08.10.2026 18:08, Marek Marczykowski-Górecki wrote:
> On Thu, Oct 08, 2026 at 05:54:53PM +0200, Jan Beulich wrote:
>> On 08.10.2026 17:05, Marek Marczykowski-Górecki wrote:
>>> Do not wait indefinitely for XHCI to configure console connection. If
>>> the cable is not plugged in, timeout after about 1s and continue boot.
>>> If the cable is plugged in later, the console will be activated then
>>> (see final part of dbc_ensure_running()). When timeout occurs, print a
>>> warning message - it will be saved in the console ring buffer, to be
>>> later retrieved from dom0, or possibly later printed when the cable is
>>> finally connected. But, do not print the warning when waiting after
>>> controller reset, as that may have been called from from console
>>> dbc_putc() and printing here would result in (possibly infinite)
>>> recursion.
>>>
>>> Try the timeout to be about 1s, which is way above time needed for
>>> configuring already connected cable. But, since this part runs very
>>> early in the Xen startup, time is not calibrated yet, and functions like
>>> NOW() do not work yet. Use raw TSC and estimate needed time to be at
>>> least 1s on a fast CPU (5GHz), and possibly longer on a slower one. This
>>> is not very accurate method, but fortunately it doesn't need to be.
>>
>> A little fragile, but well...
>>
>>> --- a/xen/drivers/char/xhci-dbc.c
>>> +++ b/xen/drivers/char/xhci-dbc.c
>>> @@ -851,9 +851,15 @@ static void dbc_reset_debug_port(struct dbc *dbc)
>>>      }
>>>  }
>>>  
>>> -static void dbc_enable_dbc(struct dbc *dbc)
>>> +static bool dbc_enable_dbc(struct dbc *dbc)
>>>  {
>>>      struct dbc_reg *reg = dbc->dbc_reg;
>>> +    s_time_t start;
>>> +    /*
>>> +     * Too early for proper time calibration, assume a 5GHz CPU and let it wait
>>> +     * about 1s, with still reasonable timeout on a slower CPU.
>>> +     */
>>> +    unsigned long timeout_ticks = 5UL*1000*1000*1000;
>>
>> Even if right now we don't use the driver in any 32-bit env, I think we
>> better wouldn't deliberately break such a possible use. IOW unsigned
>> long isn't a suitable type to use here.
> 
> So, uint64_t?

No, as per ...

>> Also, nit (style): Blanks please around *.
>>
>>> @@ -870,8 +876,12 @@ static void dbc_enable_dbc(struct dbc *dbc)
>>>      writel(readl(&reg->portsc) | (1U << DBC_PSC_PED), &reg->portsc);
>>>      wmb();
>>>  
>>> -    while ( (readl(&reg->ctrl) & (1U << DBC_CTRL_DCR)) == 0 )
>>> +    start = rdtsc_ordered();
>>> +    while ( (readl(&reg->ctrl) & (1U << DBC_CTRL_DCR)) == 0 &&
>>> +            rdtsc_ordered() - start < timeout_ticks )
>>
>> Urgh - now the driver's truly becoming x86-specific. Is there a reason
>> this cannot be get_cycles()?
> 
> Sure, it can be get_cycles(). I just need it to not try to apply scaling
> that works only after calibration, but indeed get_cycles() should work.

... this it would want to be cycles_t.

Jan