On Sun, Nov 20, 2022 at 2:50 PM Emmanuel Grumbach <egrumbach@xxxxxxxxx> wrote: > > On Fri, Nov 18, 2022 at 11:13 PM Jason Andryuk <jandryuk@xxxxxxxxx> wrote: > > > > On Thu, Nov 17, 2022 at 2:33 PM Jason Andryuk <jandryuk@xxxxxxxxx> 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@xxxxxxxxxxxxxx/ > > > > https://lore.kernel.org/linux-wireless/20090616151258.GA22849@xxxxxxxxx/ > > > > >> 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.. Thanks, for checking, Emmanuel. Yes, it seems safest to just leave the write and avoid the risk of regression. It is curious how this "mystery" write made its way into so many drivers. Regards, Jason
