[+cc David, Nirmal, linux-pci] On Thu, Dec 12, 2024 at 03:04:53PM -0800, Kenneth Crudup wrote: > On 12/12/24 12:56, Bjorn Helgaas wrote: > > On Wed, Dec 11, 2024 at 03:26:37PM -0800, Kenneth Crudup wrote: > > > So we're on 6.13-rc2 and the patches are getting closer and > > > closer, but they still need to be manually added. > > > > > > The good news is now only (variants of) "PCI/ASPM: Enable LTR > > > for endpoints behind VMD" and "PCI/ASPM: Enable ASPM for links > > > under VMD domain" are needed. > ... > https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/lunar/commit/?id=47c7bfd31514e7b54a1f830f7707297aebbb8679 This is "UBUNTU: SAUCE: PCI/ASPM: Enable LTR for endpoints behind VMD", which writes PCI_LTR_MAX_SNOOP_LAT and PCI_LTR_MAX_NOSNOOP_LAT for PCI_CLASS_STORAGE_EXPRESS devices directly below VMDs with Device IDs 0x9a09, 0xa0b0, or 0xa0bc. This looks equivalent in spirit to upstream https://git.kernel.org/linus/f492edb40b54 ("PCI: vmd: Add quirk to configure PCIe ASPM and LTR"), which writes PCI_LTR_MAX_SNOOP_LAT and PCI_LTR_MAX_NOSNOOP_LAT for any kind of device below VMDs tagged with VMD_FEATS_CLIENT, which includes 0x467f, 0x4c3d, 0x7d0b, 0x9a0b, 0xa77f, 0xad0b, 0xb06f, 0xb60b. However, IDs 0x9a09, 0xa0b0, and 0xa0bc are NOT tagged with VMD_FEATS_CLIENT. In fact, they're not included in vmd_ids[] at all, so I'd be surprised if VMD worked for those devices unless BIOS set up the VMD itself. Maybe David or Nirmal can comment on this? > https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/lunar/commit/?id=1a0102a08f206149d9abd56c2b28877c878b5526 This is "UBUNTU: SAUCE: PCI/ASPM: Enable ASPM for links under VMD domain", which adds "link->aspm_default = ASPM_STATE_ALL" for device IDs 0x9a09 and 0xa0b0. This looks like it should also be handled by upstream f492edb40b54 ("PCI: vmd: Add quirk to configure PCIe ASPM and LTR") [1], which adds "pci_enable_link_state(pdev, PCIE_LINK_STATE_ALL)". But again, the Device IDs mentioned in the Ubuntu commit are NOT included in the upstream VMD_FEATS_CLIENT list. > https://git.launchpad.net/~ubuntu-kernel/ubuntu/+source/linux/+git/lunar/commit/?id=154d48da2c57514e4b5dadc7b8c70a4edb550981 This is "UBUNTU: SAUCE: vmd: fixup bridge ASPM by driver name instead", which applies the quirk that writes PCI_LTR_MAX_SNOOP_LAT and PCI_LTR_MAX_NOSNOOP_LAT for PCI_CLASS_STORAGE_EXPRESS devices below any VMD claimed by the "vmd" driver, not just VMDs with Device IDs 0x9a09, 0xa0b0, or 0xa0bc. I think the only thing that's missing is that the upstream vmd_ids[] needs to be updated with some new VMD Device IDs that are tagged with VMD_FEATS_CLIENT. I don't know what the vmd_ids[] strategy is, but Kenneth, you might try an upstream patch like the one below. If that resolves the standby/low-power issues, maybe David or Nirmal can figure out the "right" way to do this. Bjorn diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c index 9d9596947350..4de7ff3bbf23 100644 --- a/drivers/pci/controller/vmd.c +++ b/drivers/pci/controller/vmd.c @@ -1109,6 +1109,12 @@ static const struct pci_device_id vmd_ids[] = { .driver_data = VMD_FEATS_CLIENT,}, {PCI_VDEVICE(INTEL, 0xb06f), .driver_data = VMD_FEATS_CLIENT,}, + {PCI_VDEVICE(INTEL, 0x9a09), + .driver_data = VMD_FEATS_CLIENT,}, + {PCI_VDEVICE(INTEL, 0xa0b0), + .driver_data = VMD_FEATS_CLIENT,}, + {PCI_VDEVICE(INTEL, 0xa0bc), + .driver_data = VMD_FEATS_CLIENT,}, {0,} }; MODULE_DEVICE_TABLE(pci, vmd_ids);
