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

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: <58ca7ae5-2197-4fd5-afe6-73743cd45e8c@linux.intel.com>
Date: Fri, 26 Jul 2024 10:27:31 +0800
From: Ethan Zhao <haifeng.zhao@...ux.intel.com>
To: Zhou Shengqing <zhoushengqing@...info.com>
Cc: helgaas@...nel.org, linux-kernel@...r.kernel.org,
 linux-pci@...r.kernel.org, lkp@...el.com, llvm@...ts.linux.dev,
 oe-kbuild-all@...ts.linux.dev
Subject: Re: [PATCH v4] PCI: Enable io space 1k granularity for intel cpu root
 port


On 7/25/2024 3:44 PM, Zhou Shengqing wrote:
>> 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?

VMD, if enabled, is EP, not RP. and its RPs are mapped into its own space, and
sit at different buses as VT-d, no need to care about them if am correct.

Thanks,
Ethan

>>
>