linux-kernel - Re: [PATCH v4 3/5] PCI: brcmstb: Set PCIe transaction completion timeout

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+-6iNwRrnWWG+_HeRBryi4=N3+rtH-8rCF3orz13pYxaG_iXA@mail.gmail.com>
Date:   Wed, 3 May 2023 10:06:54 -0400
From:   Jim Quinlan <james.quinlan@...adcom.com>
To:     Lukas Wunner <lukas@...ner.de>
Cc:     Bjorn Helgaas <helgaas@...nel.org>,
        Jim Quinlan <jim2101024@...il.com>, linux-pci@...r.kernel.org,
        Nicolas Saenz Julienne <nsaenz@...nel.org>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Cyril Brulebois <kibi@...ian.org>,
        Phil Elwell <phil@...pberrypi.com>,
        bcm-kernel-feedback-list@...adcom.com,
        Florian Fainelli <f.fainelli@...il.com>,
        Lorenzo Pieralisi <lpieralisi@...nel.org>,
        "Krzysztof Wilczy??ski" <kw@...ux.com>,
        Rob Herring <robh@...nel.org>,
        "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" 
        <linux-rpi-kernel@...ts.infradead.org>,
        "moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" 
        <linux-arm-kernel@...ts.infradead.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 3/5] PCI: brcmstb: Set PCIe transaction completion timeout

On Mon, May 1, 2023 at 4:55 PM Lukas Wunner <lukas@...ner.de> wrote:
>
> On Sun, Apr 30, 2023 at 05:24:26PM -0400, Jim Quinlan wrote:
> > I've been maintaining this driver for over eight years or so and we've
> > done fine with the HW default completion timeout value.
> > Only recently has a major customer requested that this timeout value
> > be changed, and their reason was so they could
> > avoid a CPU abort when using L1SS.
> >
> > Now we could set this value to a big number for all cases and not
> > require "brcm,completion-timeout-us".  I cannot see any
> > downsides, other than another customer coming along asking us to
> > double the default or lessen it.
>
> The Completion Timeout is configurable in the Device Control 2 Register
> (PCIe r2.1 sec 7.8.16).  Your controller does expose that register:
>
>   DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis- LTR+ OBFF Disabled, ARIFwd-
>            AtomicOpsCtl: ReqEn- EgressBlck-
>
> Why does your controller allow configuration of the timeout in a
> proprietary register in addition to DevCtl2?
>
> If you make the Completion Timeout configurable, please do so in
> a spec-compliant way, i.e. via DevCtl2, so that it works for
> other products as well.
>
> If the proprietary register has advantages over DevCtl2 (e.g.
> finer granularity), perhaps you can divert accesses to the
> Completion Timeout Value in DevCtl2 to your proprietary register.

Hello Lukas & Bjorn,

Yes, you are right.  I was under the (mis)understanding that writing
this register is (a) necessary for long L1SS periods and (b) overrides
or updates the value in the CFG register you mentioned.  It turns out
that only (a) is true.

After communicating with the HW engineer, it appears this register is
for an internal bus timeout.  Further, this bus timeout can occur when
there is no PCIe access involved.  However, the error appears as a
completion timeout,  which I also am investigating.

At any rate, I am dropping the "brcm,completion-timeout-usec" property
completely and V5 will just set this internal timeout to a higher
default if  the "brcm,enable-l1ss" is present.

Thanks &  regards,
Jim




>
> Thanks,
>
> Lukas

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4210 bytes)