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

Zhou Shengqing
2024-07-24T06:36:08Z

> > Do you mean it shoud be like this?
> >
> > while ((d = pci_get_device(PCI_VENDOR_ID_INTEL, 0x09a2, d))) {
> > if (d->bus->number == dev->bus->number) {
> > 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;
> > }
> > }
> > }
> >
> >> 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 ?
> > I checked it on ICX SPR EMR GNR, VT-d is always on the same bus with root port,
> > and VT-d device and function number is always 0.
>
> Yes, every VT-d instance in the root complex and the root port integrated are
> on the same bus. and VT-d is the first device of that bus.
>
> The EDS doesn't say if there is exception one of the VT-d instances in an
> system its EN1K wasn't set while others were set, vice vesa. so I suggest
> just check the VT-d and then set the root port's io_windows_1k of the same bus.

But as Bjorn mentioned at July 12, 2024, 6:48 p.m.,

"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."

When VMD enabled, just check bus number identical may lead to enable
1k io windows for VMD domain root port. E.g. 0000:80:00.0 is a
VT-d(09a2). If VMD enabled, there might be a root port 10000:80:01.0 present.
this code may lead to enable 10000:80:01.0 io_window_1k = 1.
This is probably not expected.

If I modify it like this,

while ((d = pci_get_device(PCI_VENDOR_ID_INTEL, 0x09a2, d))) {
---if (d->bus->number == dev->bus->number) {
+++if (d->bus == dev->bus) {
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;
}
}
}

Can the situation mentioned above be avoided?

Hope for your suggestion.

>
> Hope that works for your case.
>