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

Zhou Shengqing
2024-07-25T07:44:12Z

> On 7/24/2024 2:35 PM, Zhou Shengqing wrote:
> >>> 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))) {
>
> BTW, don't save letters to use single letter variable 'd', please use 'vtd_dev' or
> something else to express the VT-d device.

Got it!

>
> > ---if (d->bus->number == dev->bus->number) {
> > +++if (d->bus == dev->bus) {
>
> What if their 'bus' are NULL, though it is almost impossible. :)
>
> > 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?
>
> Yes, my understanding, as Bjorn pointed out root port extended from VMD
> bridge not on the same bus as VT-d.

For the root port extended from VMD, should the 1k window be set
when BIOS setup EN1K knob enabled?
In my case, I think EN1K should not apply to the VMD root port.

But what I'm confused about is, how can I reasonably exclude the VMD root port
in the code?

>
>