linux-kernel - RE: [PATCH v2] PCI: Fix up L1SS capability for Intel Apollolake PCIe bridge

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: <BL0PR11MB34579A5F2BEC634F2F546522FEF09@BL0PR11MB3457.namprd11.prod.outlook.com>
Date:   Fri, 30 Dec 2022 12:13:11 +0000
From:   "Lee, Ron" <ron.lee@...el.com>
To:     Bjorn Helgaas <helgaas@...nel.org>
CC:     "bhelgaas@...gle.com" <bhelgaas@...gle.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        "lmajczak@...gle.com" <lmajczak@...gle.com>,
        "Jain, Rajat" <rajatja@...gle.com>,
        Ron Lee <ron.lee.intel@...il.com>
Subject: RE: [PATCH v2] PCI: Fix up L1SS capability for Intel Apollolake PCIe
 bridge

> The issue being "lspci doesn't show L1SS after suspend/resume"?
> 
> Is the point of this basically to fix lspci output after suspend/resume?  Or is
> there something else this fixes?
> 
> It sounds like ASPM and L1SS works correctly after suspend/resume even
> without this patch?
> 
> Bjorn

Yes, that's my point. 
This quirk is going to recover the display of L1SS in lspci output, but not recover the L1SS contents and change its ASPM behavior.
If the commit title here make misunderstanding, I can change it per your suggestion.

Besides, with power measurement by power meter, I didn't see significant power consumption and battery life difference w/wo this patch. 
With suspend/resume stress testing, the system can pass the testing w/wo this patch and the its child iwlwifi device work fine in both situation. 
So I believe this quirk didn't change the ASPM behavior.

I have a generic implementation for this quirk.
It save the L1SS header and the link offset to L1SS at system booting, and restore them every time after resume.
It also check the L1SS header and its link, only recover them if they are missing and do nothing in case system has BIOS fix.
Do you think it's practical ?

```
#ifdef CONFIG_PCIEASPM
static u16 pos_to_l1ss;
static u32 l1ss_header;
static void chromeos_save_pci_l1ss_capability(struct pci_dev *pdev)
{
	u32 header;
	int pos = PCI_CFG_SPACE_SIZE;

	while (pos) {
		pci_read_config_dword(pdev, pos, &header);
		if (PCI_EXT_CAP_NEXT(header) == pdev->l1ss)
			pos_to_l1ss = pos;
		else if (PCI_EXT_CAP_ID(header) == PCI_EXT_CAP_ID_L1SS)
			l1ss_header = header;

		pos = PCI_EXT_CAP_NEXT(header);
	}
}

static void chromeos_fixup_apl_pci_l1ss_capability(struct pci_dev *pdev)
{
	u32 header;

	if (!pos_to_l1ss || !l1ss_header)
		return;

	pci_info(pdev, "Fixup L1SS Capability\n");
	/* Fixup the header of L1SS Capability if missing */
	pci_read_config_dword(pdev, pdev->l1ss, &header);
	if (PCI_EXT_CAP_ID(header) != PCI_EXT_CAP_ID_L1SS)
		pci_write_config_dword(pdev, pdev->l1ss, l1ss_header);

	/* Fixup the link to L1SS Capability if missing*/
	pci_read_config_dword(pdev, pos_to_l1ss, &header);
	if (PCI_EXT_CAP_NEXT(header) != pdev->l1ss)
		pci_write_config_dword(pdev, pos_to_l1ss, pdev->l1ss << 20);
}
DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_INTEL, 0x5ad6, chromeos_save_pci_l1ss_capability);
DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_INTEL, 0x5ad6, chromeos_fixup_apl_pci_l1ss_capability);
#endif
```