Re: [PATCH v4] Subject: PCI: Enable io space 1k granularity for intel cpu root port

[Ethan Zhao]

On 7/23/2024 4:04 PM, Zhou Shengqing wrote:
> > I think this has potential.  Can you include a more complete citation
> > for the Intel spec?  Complete name, document number if available,
> > revision, section?  Hopefully it's publically available?
> Most of intel CPU EDS specs are under NDA. But you can refer to
> https://www.intel.com/content/dam/www/public/us/en/documents/datasheets/xeon-e5-v2-datasheet-vol-2.pdf
> keyword:"EN1K".
>
> > + /*
> > > + * Per intel sever CPU EDS vol2(register) spec,
> > > + * Intel Memory Map/Intel VT-d configuration space,
> > > + * IIO MISC Control (IIOMISCCTRL_1_5_0_CFG) — Offset 1C0h
> > > + * bit 2.
> > > + * Enable 1K (EN1K):
> > > + * This bit when set, enables 1K granularity for I/O space decode
> > > + * in each of the virtual P2P bridges
> > > + * corresponding to root ports, and DMI ports.
> > > + */
> > > + while ((d = pci_get_device(PCI_VENDOR_ID_INTEL, 0x09a2, d))) {
> > To be safe, "d" (the [8086:09a2] device) should be on the same bus as
> > "dev" (with VMD, I think we get Root Ports *below* the VMD bridge,
> > which would be a different bus, and they presumably are not influenced
> > by the EN1K bit.
> I modified the code as follows, can you help me review it?
>
> /* Enable 1k I/O space granularity on the intel root port */
> static void quirk_intel_rootport_1k_io(struct pci_dev *dev)
> {
> 	struct pci_dev *d = NULL;
> 	u16 en1k = 0;
>
> 	if (!pci_is_pcie(dev) || pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT)
> 		return;
>
> 	/*
> 	 * Per intel sever CPU (ICX SPR GNR)EDS vol2(register) spec,
> 	 * Intel Memory Map/Intel VT-d configuration space,
> 	 * IIO MISC Control (IIOMISCCTRL_1_5_0_CFG) — Offset 1C0h
> 	 * bit 2.
> 	 * Enable 1K (EN1K):
> 	 * This bit when set, enables 1K granularity for I/O space decode
> 	 * in each of the virtual P2P bridges
> 	 * corresponding to root ports, and DMI ports.
> 	 */
> 	while ((d = pci_get_device(PCI_VENDOR_ID_INTEL, 0x09a2, d))) {
> 		if (pci_domain_nr(d->bus) == pci_domain_nr(dev->bus)) {

Perhaps it is enough to check if the 0x09a2 VT-d and the rootport are on the smae bus
e.g. On my SPR, domain 0000

00:00.0 System peripheral: Intel Corporation Device 09a2 (rev 20)
00:0f.0 PCI bridge: Intel Corporation Device 1bbf (rev 10) (prog-if 00 [Normal decode])

 
15:00.0 System peripheral: Intel Corporation Device 09a2 (rev 20)
15:01.0 PCI bridge: Intel Corporation Device 352a (rev 04) (prog-if 00 [Normal decode])

and if you check domain number only, they might sit on different bus, perhaps that
would make thing complex, could you make sure the VT-d is on the upstream bus of the
bridge ?

Thanks,
Ethan
 

> 			pci_read_config_word(d, 0x1c0, &en1k);
> 			if (en1k & 0x4) {
> 				pci_info(dev, "1K I/O windows enabled per %s EN1K setting\n", pci_name(d));
> 				dev->io_window_1k = 1;
> 			}
> 		}
> 	}
> }
> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_INTEL,	PCI_ANY_ID,	quirk_intel_rootport_1k_io);
>
> If you have a better method, please let me know. If there are no issues,
> I can submit a new patch.