linux-kernel - Re: [PATCH 1/1] PCI: Use the correct bit in Link Training not active check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <c81ecb2a-9365-dbb0-2207-ed692c8c7487@linux.intel.com>
Date: Fri, 15 Mar 2024 11:58:31 +0200 (EET)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: "Maciej W. Rozycki" <macro@...am.me.uk>
cc: Bjorn Helgaas <bhelgaas@...gle.com>, linux-pci@...r.kernel.org, 
    LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/1] PCI: Use the correct bit in Link Training not active
 check

On Thu, 14 Mar 2024, Maciej W. Rozycki wrote:

> On Thu, 14 Mar 2024, Ilpo Järvinen wrote:
> 
> > One more point to add here, I started to wonder today why that use_lt 
> > parameter is needed at all for pcie_retrain_link()?
> > 
> > Once the Target Speed has been changed to 2.5GT/s which is what the quirk 
> > does before calling retraining, LT too should work "normally" after that.
> 
>  We don't know if the link is going to become stable with the TLS update 
> to 2.5GT/s and we want to ensure that the link has reached the active 
> state before claiming victory; LT clear does not mean the link is active, 
> it only means what it means, that is that the link isn't being trained at 
> the moment.

LT clear means retraining has ended which is the condition 
pcie_retrain_link() should terminate at. It tried and finished retraining 
as proven by LT clear.

>  Also we don't want to reset the TLS to the maximum before the link has 
> become active.

I'm not suggesting to change the if DLLLA check that is within the quirk 
so it will remain the same even if pcie_retrain_link() would no longer 
have the use_lt parameter.

If LT clear after retraining does not imply DLLLA set, then this again 
falls into the quirk territory and IMO the quirk itself should be what 
makes the additional call to wait for DLLLA, not pcie_retrain_link().
I suspect though that DL clear does imply DLLLA set (after the target 
speed was lowered) so my expectation is that the extra wait wouldn't be 
necessary at that point.

>  Does this answer your question?

What I'm trying to achieve is having pcie_retrain_link() focus on 
retraining and quirk on steps required by the quirk. Currently they're 
kind of mixed if we assume the assumption that LT clear doesn't imply 
active link is true. That tells quirk would need additional step, 
that is, wait for DLLLA after the retraining has completed which is 
currently hidden into pcie_retrain_link() rather than explicitly 
called by the quirk.

-- 
 i.