| 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
| ||
|
Message-ID: <60a41564-43ed-41ea-af5c-eb8409b47ad1@cixtech.com>
Date: Mon, 21 Apr 2025 23:59:48 +0800
From: Hans Zhang <hans.zhang@...tech.com>
To: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
Hans Zhang <18255117159@....com>
Cc: Niklas Cassel <cassel@...nel.org>, Bjorn Helgaas <helgaas@...nel.org>,
Shawn Lin <shawn.lin@...k-chips.com>, lpieralisi@...nel.org, kw@...ux.com,
bhelgaas@...gle.com, heiko@...ech.de, robh@...nel.org, jingoohan1@...il.com,
thomas.richard@...tlin.com, linux-pci@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-rockchip@...ts.infradead.org
Subject: Re: [PATCH] PCI: dw-rockchip: Configure max payload size on host init
On 2025/4/21 22:53, Manivannan Sadhasivam wrote:
> EXTERNAL EMAIL
>
> On Sat, Apr 19, 2025 at 01:21:54AM +0800, Hans Zhang wrote:
>>
>>
>> On 2025/4/18 22:55, Niklas Cassel wrote:
>>>
>>>
>>> On 18 April 2025 14:33:08 CEST, Hans Zhang <18255117159@....com> wrote:
>>>>
>>>> Dear Bjorn,
>>>>
>>>> Thanks your for reply. Niklas and I attempted to modify the relevant logic in drivers/pci/probe.c and found that there was a lot of code judging the global variable pcie_bus_config. At present, there is no good method. I will keep trying.
>>>>
>>>> I wonder if you have any good suggestions? It seems that the code logic regarding pcie_bus_config is a little complicated and cannot be modified for the time being?
>>>
>>>
>>> Hans,
>>>
>>> If:
>>>
>>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>>> index 364fa2a514f8..2e1c92fdd577 100644
>>> --- a/drivers/pci/probe.c
>>> +++ b/drivers/pci/probe.c
>>> @@ -2983,6 +2983,13 @@ void pcie_bus_configure_settings(struct pci_bus *bus)
>>> if (!pci_is_pcie(bus->self))
>>> return;
>>> + /*
>>> + * Start off with DevCtl.MPS == DevCap.MPS, unless PCIE_BUS_TUNE_OFF.
>>> + * This might get overriden by a MPS strategy below.
>>> + */
>>> + if (pcie_bus_config != PCIE_BUS_TUNE_OFF)
>>> + smpss = pcie_get_mps(bus->self);
>>> +
>>
>> Dear Niklas,
>>
>> Thank you very much for your reply and thoughts.
>>
>> pcie_get_mps: Returns maximum payload size in bytes
>>
>> I guess you want to obtain the DevCap MPS. But the purpose of the smpss
>> variable is to save the DevCtl MPS.
>>
>>> /*
>>> * FIXME - Peer to peer DMA is possible, though the endpoint would need
>>> * to be aware of the MPS of the destination. To work around this,
>>>
>>>
>>>
>>> does not work, can't you modify the code slightly so that it works?
>>>
>>> I haven't tried myself, but considering that it works when walking the bus, it seems that it should be possible to get something working.
>>>
>>
>>
>> After making the following modifications, my test shows that it is normal.
>> If the consideration is not comprehensive. Could Bjorn and Niklas please
>> review my revisions?
>>
>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
>> index 364fa2a514f8..5b54f1b0a91d 100644
>> --- a/drivers/pci/probe.c
>> +++ b/drivers/pci/probe.c
>> @@ -2951,8 +2951,7 @@ static int pcie_bus_configure_set(struct pci_dev *dev,
>> void *data)
>> if (!pci_is_pcie(dev))
>> return 0;
>>
>> - if (pcie_bus_config == PCIE_BUS_TUNE_OFF ||
>> - pcie_bus_config == PCIE_BUS_DEFAULT)
>> + if (pcie_bus_config == PCIE_BUS_TUNE_OFF)
>> return 0;
>>
>> mps = 128 << *(u8 *)data;
>> @@ -2991,7 +2990,8 @@ void pcie_bus_configure_settings(struct pci_bus *bus)
>> if (pcie_bus_config == PCIE_BUS_PEER2PEER)
>> smpss = 0;
>>
>> - if (pcie_bus_config == PCIE_BUS_SAFE) {
>> + if ((pcie_bus_config == PCIE_BUS_SAFE) ||
>> + (pcie_bus_config != PCIE_BUS_TUNE_OFF)) {
>> smpss = bus->self->pcie_mpss;
>>
>> pcie_find_smpss(bus->self, &smpss);
>>
>>
>
> As I replied to Niklas, I'd like to limit the changes to platforms having
> controller drivers. So here is my proposal:
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 364fa2a514f8..d32a0f90beb1 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -3228,6 +3228,17 @@ int pci_host_probe(struct pci_host_bridge *bridge)
> */
> pci_assign_unassigned_root_bus_resources(bus);
>
> + if (pcie_bus_config != PCIE_BUS_TUNE_OFF) {
> + /* Configure root ports MPS to be MPSS by default */
> + for_each_pci_bridge(dev, bus) {
> + if (pci_pcie_type(dev) != PCI_EXP_TYPE_ROOT_PORT)
> + continue;
> +
> + pcie_write_mps(dev, 128 << dev->pcie_mpss);
> + pcie_write_mrrs(dev);
> + }
> + }
> +
Sorry. Since the reply just now was made by logging into the email
through the browser, there are some errors. I reply again as follows:
[My personal computer is not at hand. I reply using the company email.]
Hi Mani,
Thank you very much for your reply and suggestions. The following is the
test on our platform and it works properly. I wonder if others have any
other opinions? If not, I will send the next version and then see
everyone's opinions.
root@...-localhost:~# cat /proc/version
Linux version 6.15.0-rc2-00015-gced1536aaf04-dirty (hans@...s) ......
root@...-localhost:~# lspci
0000:c0:00.0 PCI bridge: Device 1f6c:0001
0000:c1:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd
NVMe SSD Controller S4LV008[Pascal]
0001:90:00.0 PCI bridge: Device 1f6c:0001
0001:91:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd
NVMe SSD Controller PM9A1/PM9A3/980PRO
0003:30:00.0 PCI bridge: Device 1f6c:0001
0003:31:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd.
RTL8125 2.5GbE Controller (rev 05)root@...-localhost:~# lspci -vvv
0000:c0:00.0 PCI bridge: Device 1f6c:0001 (prog-if 00 [Normal decode])
......
Capabilities: [c0] Express (v2) Root Port (Slot-), MSI 00
DevCap: MaxPayload 512 bytes, PhantFunc 0
ExtTag+ RBE+
DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
RlxdOrd+ ExtTag+ PhantFunc- AuxPwr- NoSnoop+
MaxPayload 512 bytes, MaxReadReq 1024 bytes
......
0000:c1:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd
NVMe SSD Controller S4LV008[Pascal] (prog-if 02 [NVM Express])
......
Capabilities: [70] Express (v2) Endpoint, MSI 00
DevCap: MaxPayload 512 bytes, PhantFunc 0, Latency L0s
unlimited, L1 unlimited
ExtTag+ AttnBtn- AttnInd- PwrInd- RBE+ FLReset+
SlotPowerLimit 0W
DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
RlxdOrd+ ExtTag+ PhantFunc- AuxPwr- NoSnoop+
FLReset-
MaxPayload 512 bytes, MaxReadReq 512 bytes
......
0001:90:00.0 PCI bridge: Device 1f6c:0001 (prog-if 00 [Normal decode])
......
Capabilities: [c0] Express (v2) Root Port (Slot-), MSI 00
DevCap: MaxPayload 512 bytes, PhantFunc 0
ExtTag- RBE+
DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
MaxPayload 512 bytes, MaxReadReq 1024 bytes
......
0001:91:00.0 Non-Volatile memory controller: Samsung Electronics Co Ltd
NVMe SSD Controller PM9A1/PM9A3/980PRO (prog-if 02 [NVM Express])
......
Capabilities: [70] Express (v2) Endpoint, MSI 00
DevCap: MaxPayload 256 bytes, PhantFunc 0, Latency L0s
unlimited, L1 unlimited
ExtTag+ AttnBtn- AttnInd- PwrInd- RBE+ FLReset+
SlotPowerLimit 0W
DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
RlxdOrd+ ExtTag+ PhantFunc- AuxPwr- NoSnoop+
FLReset-
MaxPayload 256 bytes, MaxReadReq 512 bytes
......
0003:30:00.0 PCI bridge: Device 1f6c:0001 (prog-if 00 [Normal decode])
......
Capabilities: [c0] Express (v2) Root Port (Slot-), MSI 00
DevCap: MaxPayload 512 bytes, PhantFunc 0
ExtTag- RBE+
DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop+
MaxPayload 512 bytes, MaxReadReq 1024 bytes
......
0003:31:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd.
RTL8125 2.5GbE Controller (rev 05)
......
Capabilities: [70] Express (v2) Endpoint, MSI 01
DevCap: MaxPayload 256 bytes, PhantFunc 0, Latency L0s
<512ns, L1 <64us
ExtTag- AttnBtn- AttnInd- PwrInd- RBE+ FLReset-
SlotPowerLimit 0W
DevCtl: CorrErr+ NonFatalErr+ FatalErr+ UnsupReq+
RlxdOrd+ ExtTag- PhantFunc- AuxPwr- NoSnoop-
MaxPayload 256 bytes, MaxReadReq 4096 bytes
......
Best regards,
Hans
> list_for_each_entry(child, &bus->children, node)
> pcie_bus_configure_settings(child);
>
> - Mani
>
> --
> மணிவண்ணன் சதாசிவம்
>
Powered by blists - more mailing lists