Re: [Intel-wired-lan] [PATCH] igc: fix Tx timestamp timeout caused by unlocked TIMINCA write in adj fine]

[Vinicius Costa Gomes <vinicius.gomes@intel.com>]

Hi,

Bob Van Valzah <bob@vanvalzah.com> writes:

> Vadim,
>
> Thanks for the feedback on our first patch. We've spent more time in
> the lab studying the igc TX timestamp behavior under stress. We
> understand the failure modes much better now.
>
> You were right that ptp_tx_lock was the wrong lock — it guards the TX
> queue, not the timing registers. You suggested tmreg_lock instead. We
> tested tmreg_lock alone (v2) and found it doesn't fix the bug: it appears
> that the race is between the software TIMINCA write and the hardware’s
> asynchronous TX timestamp capture pipeline, not between two software threads.
> tmreg_lock serializes software register accesses but can't prevent the hardware
> from reading TIMINCA at the instant software writes it.
>
> Our v3 patch (attached) takes tmreg_lock as you suggested, and
> additionally disables TX timestamping in hardware via TSYNCTXCTL around
> the TIMINCA write. This prevents the hardware from starting new
> timestamp captures during the rate change:
>
>     spin_lock_irqsave(&igc->tmreg_lock, flags);
>     txctl = rd32(IGC_TSYNCTXCTL);
>     wr32(IGC_TSYNCTXCTL, txctl & ~IGC_TSYNCTXCTL_ENABLED);
>     wr32(IGC_TIMINCA, inca);
>     wr32(IGC_TSYNCTXCTL, txctl);
>     spin_unlock_irqrestore(&igc->tmreg_lock, flags);
>

I sent, a couple of days ago, the link to your report to our hardware
folks, waiting for them to take a look.

I think that this workaround, even if incomplete, will be interesting to
them as well. Again, thanks for the detailed report.


Cheers,
-- 
Vinicius