| 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: <2684f4b3-b79d-14a2-c547-8b1f7000737c@oss.qualcomm.com>
Date: Tue, 31 Dec 2024 10:17:46 +0530
From: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
To: Konrad Dybcio <konrad.dybcio@....qualcomm.com>,
cros-qcom-dts-watchers@...omium.org,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konradybcio@...nel.org>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley
<conor+dt@...nel.org>,
Lorenzo Pieralisi <lpieralisi@...nel.org>,
Krzysztof Wilczyński <kw@...ux.com>,
Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
Bjorn Helgaas <bhelgaas@...gle.com>, Jingoo Han <jingoohan1@...il.com>
Cc: linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
quic_vbadigan@...cinc.com, quic_vpernami@...cinc.com,
quic_mrana@...cinc.com, mmareddy@...cinc.com,
Krishna chaitanya chundru <quic_krichai@...cinc.com>
Subject: Re: [PATCH v2 2/4] PCI: dwc: Add ECAM support with iATU configuration
On 12/30/2024 8:34 PM, Konrad Dybcio wrote:
> On 24.12.2024 3:10 PM, Krishna Chaitanya Chundru wrote:
>> From: Krishna chaitanya chundru <quic_krichai@...cinc.com>
>>
>> The current implementation requires iATU for every configuration
>> space access which increases latency & cpu utilization.
>>
>> Designware databook 5.20a, section 3.10.10.3 says about CFG Shift Feature,
>> which shifts/maps the BDF (bits [31:16] of the third header DWORD, which
>> would be matched against the Base and Limit addresses) of the incoming
>> CfgRd0/CfgWr0 down to bits[27:12]of the translated address.
>>
>> Configuring iATU in config shift feature enables ECAM feature to access the
>> config space, which avoids iATU configuration for every config access.
>>
>> Add "ctrl2" into struct dw_pcie_ob_atu_cfg to enable config shift feature.
>>
>> As DBI comes under config space, this avoids remapping of DBI space
>> separately. Instead, it uses the mapped config space address returned from
>> ECAM initialization. Change the order of dw_pcie_get_resources() execution
>> to achieve this.
>>
>> Enable the ECAM feature if the config space size is equal to size required
>> to represent number of buses in the bus range property, add a function
>> which checks this. The DWC glue drivers uses this function and decide to
>> enable ECAM mode or not.
>>
>> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
>> ---
>> drivers/pci/controller/dwc/Kconfig | 1 +
>> drivers/pci/controller/dwc/pcie-designware-host.c | 136 +++++++++++++++++++---
>> drivers/pci/controller/dwc/pcie-designware.c | 2 +-
>> drivers/pci/controller/dwc/pcie-designware.h | 11 ++
>> 4 files changed, 130 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
>> index b6d6778b0698..73c3aed6b60a 100644
>> --- a/drivers/pci/controller/dwc/Kconfig
>> +++ b/drivers/pci/controller/dwc/Kconfig
>> @@ -9,6 +9,7 @@ config PCIE_DW
>> config PCIE_DW_HOST
>> bool
>> select PCIE_DW
>> + select PCI_HOST_COMMON
>>
>> config PCIE_DW_EP
>> bool
>> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
>> index d2291c3ceb8b..4e07fefe12e1 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
>> @@ -418,6 +418,61 @@ static void dw_pcie_host_request_msg_tlp_res(struct dw_pcie_rp *pp)
>> }
>> }
>>
>> +static int dw_pcie_config_ecam_iatu(struct dw_pcie_rp *pp)
>> +{
>> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> + struct dw_pcie_ob_atu_cfg atu = {0};
>> + struct resource_entry *bus;
>> + int ret, bus_range_max;
> resource_size_t for bus_range_max since you feed it the ouput of
> resource_size()
>
>> +
>> + bus = resource_list_first_type(&pp->bridge->windows, IORESOURCE_BUS);
>> +
>> + /*
>> + * Root bus under the root port doesn't require any iATU configuration
>> + * as DBI space will represent Root bus configuration space.
>> + * Immediate bus under Root Bus, needs type 0 iATU configuration and
>> + * remaining buses need type 1 iATU configuration.
>> + */
>> + atu.index = 0;
>> + atu.type = PCIE_ATU_TYPE_CFG0;
>> + atu.cpu_addr = pp->cfg0_base + SZ_1M;
>> + atu.size = SZ_1M;
>> + atu.ctrl2 = PCIE_ATU_CFG_SHIFT_MODE_ENABLE;
>> + ret = dw_pcie_prog_outbound_atu(pci, &atu);
>> + if (ret)
>> + return ret;
>> +
>> + bus_range_max = resource_size(bus->res);
>> +
>> + /* Configure remaining buses in type 1 iATU configuration */
>> + atu.index = 1;
>> + atu.type = PCIE_ATU_TYPE_CFG1;
>> + atu.cpu_addr = pp->cfg0_base + SZ_2M;
>> + atu.size = (SZ_1M * (bus_range_max - 2));
> This explodes badly with:
>
> bus-range = <0 0>;
The bus range = <0 0> is not a valid configuration but with bus-range =
<0 1> it will
be a issue I will update the logic next series, thanks for pointing it out.
- Krishna Chaitanya.
>
>> + atu.ctrl2 = PCIE_ATU_CFG_SHIFT_MODE_ENABLE;
>> + return dw_pcie_prog_outbound_atu(pci, &atu);
> A newline before the return statement would make it prettier
>
> [...]
>
>> +bool dw_pcie_ecam_supported(struct dw_pcie_rp *pp)
>> +{
>> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> + struct platform_device *pdev = to_platform_device(pci->dev);
>> + struct resource *config_res, *bus_range;
>> + u64 bus_config_space_count;
>> +
>> + bus_range = resource_list_first_type(&pp->bridge->windows, IORESOURCE_BUS)->res;
>> + if (!bus_range)
>> + return false;
>> +
>> + config_res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
>> + if (!config_res)
>> + return false;
>> +
>> + bus_config_space_count = resource_size(config_res) >> PCIE_ECAM_BUS_SHIFT;
>> + if (resource_size(bus_range) > bus_config_space_count)
>> + return false;
>> +
>> + return true;
> return bus_config_space_count <= resource_size(bus_range);
>
> Konrad
Powered by blists - more mailing lists