Re: [PATCH v5 9/9] drivers/char: fix handling cable re-plug in XHCI console driver

[Jan Beulich]

On 26.08.2022 17:44, Andrew Cooper wrote:
> On 26/08/2022 15:50, Jan Beulich wrote:
>> On 22.08.2022 17:27, Marek Marczykowski-Górecki wrote:
>>> @@ -1023,6 +1032,8 @@ static bool dbc_ensure_running(struct dbc *dbc)
>>>          writel(ctrl | (1U << DBC_CTRL_DRC), &reg->ctrl);
>>>          writel(readl(&reg->portsc) | (1U << DBC_PSC_PED), &reg->portsc);
>>>          wmb();
>>> +        dbc_ring_doorbell(dbc, dbc->dbc_iring.db);
>>> +        dbc_ring_doorbell(dbc, dbc->dbc_oring.db);
>>>      }
>> You retain the wmb() here, but ...
>>
>>> @@ -1066,8 +1073,7 @@ static void dbc_flush(struct dbc *dbc, struct 
>>> xhci_trb_ring *trb,
>>>          }
>>>      }
>>>  
>>> -    wmb();
>>> -    writel(db, &reg->db);
>>> +    dbc_ring_doorbell(dbc, trb->db);
>>>  }
>> ... you drop it here. Why the difference?
> 
> As a tangent, every single barrier in this file is buggy.  Should be
> smp_*() variants, not mandatory variants.
> 
> All (interesting) data is in plain WB cached memory, and the few BAR
> registers which are configured have a UC mapping which orders properly
> WRT other writes on x86.

But such drivers shouldn't be x86-specific when it comes to their use
of barriers. For this reason I specifically did not complain about any
of the barrier uses throughout the series (with the further thinking
of "better one too many than one too few").

Jan