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

[Jan Beulich <jbeulich@xxxxxxxx>]

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