| 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: <cc98ec47-ce33-33a2-bbe3-c0da34d85d97@quicinc.com>
Date: Wed, 4 Dec 2024 07:32:09 +0530
From: Krishna Chaitanya Chundru <quic_krichai@...cinc.com>
To: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
CC: <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>, Jingoo Han <jingoohan1@...il.com>,
Lorenzo Pieralisi
<lpieralisi@...nel.org>,
Krzysztof Wilczyński
<kw@...ux.com>,
Bjorn Helgaas <bhelgaas@...gle.com>, <quic_vbadigan@...cinc.com>,
<quic_ramkri@...cinc.com>, <quic_nitegupt@...cinc.com>,
<quic_skananth@...cinc.com>, <quic_vpernami@...cinc.com>,
<quic_mrana@...cinc.com>, <mmareddy@...cinc.com>,
<linux-arm-msm@...r.kernel.org>, <devicetree@...r.kernel.org>,
<linux-kernel@...r.kernel.org>, <linux-pci@...r.kernel.org>
Subject: Re: [PATCH 2/3] PCI: dwc: Add ECAM support with iATU configuration
On 12/2/2024 10:12 PM, Manivannan Sadhasivam wrote:
> On Sun, Nov 17, 2024 at 03:30:19AM +0530, Krishna chaitanya chundru wrote:
>> The current implementation requires iATU for every configuration
>> space access which increases latency & cpu utilization.
>>
>> Configuring iATU in config shift mode enables ECAM feature to access the
>
> Can you please elaborate 'config shift mode'? Quote relevant section in DWC
> databook for reference.
>
>> config space, which avoids iATU configuration for every config access.
>>
>> Add "ctrl2" into struct dw_pcie_ob_atu_cfg to enable config shift mode.
>>
>> 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 acheive this.
>>
>> Introduce new ecam_init() function op for the clients to configure after
>
> We use 'DWC glue drivers' to refer the 'clients' of this driver.
>
>> ecam window creation has been done.
>>
>
> Use 'ECAM' everywhere.
>
>> Signed-off-by: Krishna chaitanya chundru <quic_krichai@...cinc.com>
>> ---
>> drivers/pci/controller/dwc/pcie-designware-host.c | 114 ++++++++++++++++++----
>> drivers/pci/controller/dwc/pcie-designware.c | 2 +-
>> drivers/pci/controller/dwc/pcie-designware.h | 6 ++
>> 3 files changed, 102 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
>> index 3e41865c7290..e98cc841a2a9 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
>> @@ -418,6 +418,62 @@ 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;
>> +
>> + bus = resource_list_first_type(&pp->bridge->windows, IORESOURCE_BUS);
>> +
>> + /*
>> + * Bus 1 config space needs type 0 atu configuration
>> + * Remaining buses need type 1 atu configuration
>> + */
>> + atu.index = 0;
>> + atu.type = PCIE_ATU_TYPE_CFG0;
>> + atu.cpu_addr = pp->cfg0_base + SZ_1M;
>
> You didn't mention what occupies the first 1MB.
>
>> + 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 = bus->res->end - bus->res->start + 1;
>
> resource_size(bus->res)
>
>> +
>> + /* Configure for bus 2 - bus_range_max in type 1 */
>> + 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));
>> + atu.ctrl2 = PCIE_ATU_CFG_SHIFT_MODE_ENABLE;
>> + return dw_pcie_prog_outbound_atu(pci, &atu);
>> +}
>> +
>> +static int dw_pcie_create_ecam_window(struct dw_pcie_rp *pp, struct resource *res)
>> +{
>> + struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> + struct device *dev = pci->dev;
>> + struct resource_entry *bus;
>> +
>> + bus = resource_list_first_type(&pp->bridge->windows, IORESOURCE_BUS);
>> + if (!bus)
>> + return -ENODEV;
>> +
>> + pp->cfg = pci_ecam_create(dev, res, bus->res, &pci_generic_ecam_ops);
>> + if (IS_ERR(pp->cfg))
>> + return PTR_ERR(pp->cfg);
>> +
>> + pci->dbi_base = pp->cfg->win;
>> + pci->dbi_phys_addr = res->start;
>> +
>> + if (pp->ops->ecam_init)
>> + pp->ops->ecam_init(pci, pp->cfg);
>> +
>> + return 0;
>> +}
>> +
>> int dw_pcie_host_init(struct dw_pcie_rp *pp)
>> {
>> struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
>> @@ -431,19 +487,8 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
>>
>> raw_spin_lock_init(&pp->lock);
>>
>> - ret = dw_pcie_get_resources(pci);
>> - if (ret)
>> - return ret;
>> -
>> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
>> - if (res) {
>> - pp->cfg0_size = resource_size(res);
>> - pp->cfg0_base = res->start;
>> -
>> - pp->va_cfg0_base = devm_pci_remap_cfg_resource(dev, res);
>> - if (IS_ERR(pp->va_cfg0_base))
>> - return PTR_ERR(pp->va_cfg0_base);
>> - } else {
>> + if (!res) {
>> dev_err(dev, "Missing *config* reg space\n");
>> return -ENODEV;
>> }
>> @@ -454,6 +499,30 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
>>
>> pp->bridge = bridge;
>>
>> + pp->cfg0_size = resource_size(res);
>> + pp->cfg0_base = res->start;
>> +
>> + if (!pp->enable_ecam) {
>
> Why can't you just use the ECAM mode when there is enough memory defined in DT?
> Using this flag slightly defeats the purpose of the ECAM mode.
>
>> + pp->va_cfg0_base = devm_pci_remap_cfg_resource(dev, res);
>> + if (IS_ERR(pp->va_cfg0_base))
>> + return PTR_ERR(pp->va_cfg0_base);
>> +
>> + /* Set default bus ops */
>> + bridge->ops = &dw_pcie_ops;
>> + bridge->child_ops = &dw_child_pcie_ops;
>> + bridge->sysdata = pp;
>> + } else {
>> + ret = dw_pcie_create_ecam_window(pp, res);
>> + if (ret)
>> + return ret;
>> + bridge->ops = (struct pci_ops *)&pci_generic_ecam_ops.pci_ops;
>> + pp->bridge->sysdata = pp->cfg;
>
> 'bridge->sysdata = pp->cfg'?
>
as we are using pci_generic_ecam_ops.pci_ops for config reads & writes
it expects cfg space as sysdata[1].
[1] https://elixir.bootlin.com/linux/v6.12.1/source/drivers/pci/ecam.c#L170
- Krishna Chaitanya.
> - Mani
>
Powered by blists - more mailing lists