Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <CAKf6xpuJgj2TPGZsUw7JXvMkpv-wzgPq13ySzMaz0R2Ti9WQ4A@mail.gmail.com>
 <CAKf6xpuKGp6HSB--xNKqKXMAY2Jj13Zzeg8=8G=-YJOsN2UvAg@mail.gmail.com>
In-Reply-To: <CAKf6xpuKGp6HSB--xNKqKXMAY2Jj13Zzeg8=8G=-YJOsN2UvAg@mail.gmail.com>
From:   Emmanuel Grumbach <egrumbach@gmail.com>
Date:   Sun, 20 Nov 2022 21:49:57 +0200
Message-ID: <CANUX_P0tXpmcia=djC8yzyY0OCS2UNOZjSAuxRjeBfgQqyUrtQ@mail.gmail.com>
Subject: Re: iwlwifi write to PCI_CFG_RETRY_TIMEOUT
To:     Jason Andryuk <jandryuk@gmail.com>
Cc:     linux-wireless@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Nov 18, 2022 at 11:13 PM Jason Andryuk <jandryuk@gmail.com> wrote:
>
> On Thu, Nov 17, 2022 at 2:33 PM Jason Andryuk <jandryuk@gmail.com> wrote:
> >
> > Hi,
> >
> > I was looking at iwlwifi under Xen PCI passthrough and I noticed a
> > curious PCI config space write:
> >
> > https://github.com/torvalds/linux/blob/master/drivers/net/wireless/intel/iwlwifi/pcie/drv.c#L1721
> > /*
> > * We disable the RETRY_TIMEOUT register (0x41) to keep
> > * PCI Tx retries from interfering with C3 CPU state.
> > */
> > pci_write_config_byte(pdev, PCI_CFG_RETRY_TIMEOUT, 0x00);
> >
> > With 00:14.3 Network controller: Intel Corporation Cannon Point-LP
> > CNVi [Wireless-AC] (rev 30)
> > register 0x41 in the PCI config space is the next cap pointer for
> > "Capabilities: [40] Express Root Complex Integrated Endpoint, MSI 00".
> >
> > On baremetal, the write seems to be dropped since `hexdump -C
> > /sys/bus/pci/devices/0000\:00\:14.3/config` still shows 0x80 (in my
> > case).  Though I suppose the device could be acting on it even if the
> > value doesn't change.
> >
> > With Xen PCI passthrough, QEMU seems to honor emulating the write and
> > it breaks lspci traversing the capabilities so MSI-X is no longer
> > shown.
> >
> > Is the write to RETRY_TIMEOUT at 0x41 correct?  It seems to be really
> > old.  Here it references being copied from ipw2100:
>
> It seems like lots of drivers copied the write from ipw2100.  And it
> seems like no one knows exactly why it is there.  But it does do
> something for some devices which is why ath9k kept it.
>
> These are some interesting and relevant emails:
> https://lore.kernel.org/linux-wireless/43e72e890912192251r4de4a3c3idb5e4c3723ef87aa@mail.gmail.com/
>
> https://lore.kernel.org/linux-wireless/20090616151258.GA22849@jm.kir.nu/
>
> >> I seem to remember the provenance of this code was copy-paste from
> >> an intel driver, so while it does "something," the comment may not
> >> match the code, 0x41 being vendor-defined.
> >
> > The exact story behind this has been a bit more than trivial task to
> > figure out ;-). I would assume this comment is referring to a madwifi
> > changeset: http://madwifi-project.org/changeset/584
>
> Unfortunately, that link seems to be dead and archive.org doesn't have it.
>

I asked internally and unfortunately I couldn't get any clear answer.
Looks like this setting
has been there for many many years (~15) and nobody really understands
why. It has been
removed for our newest device in another (close source) driver and I
guess we'll do the
same in Linux. I am not sure what we'll do with legacy devices though...
We could remove it and risk a regression, or just ... leave it..