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] [thread-next>] [day] [month] [year] [list]
Message-ID: <a158c4f5-e9c3-48c2-b440-fa9dc281b276@oss.qualcomm.com>
Date: Mon, 25 Aug 2025 11:05:35 -0700
From: Mayank Rana <mayank.rana@....qualcomm.com>
To: Krishna Chaitanya Chundru <krishna.chundru@....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>,
        Manivannan Sadhasivam <mani@...nel.org>,
        Krzysztof WilczyƄski <kwilczynski@...nel.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_mrana@...cinc.com,
        quic_vpernami@...cinc.com, mmareddy@...cinc.com
Subject: Re: [PATCH v7 4/5] PCI: dwc: Add ECAM support with iATU configuration

Hi Krishna

On 8/22/2025 2:27 AM, Krishna Chaitanya Chundru wrote:
> 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.

Also add 256 MB alignment requirement for using iATU config shift mode here.

> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
> ---
>   drivers/pci/controller/dwc/Kconfig                |   1 +
>   drivers/pci/controller/dwc/pcie-designware-host.c | 131 +++++++++++++++++++---
>   drivers/pci/controller/dwc/pcie-designware.c      |   2 +-
>   drivers/pci/controller/dwc/pcie-designware.h      |   5 +
>   4 files changed, 124 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index ff6b6d9e18ecfa44273e87931551f9e63fbe3cba..a0e7ad3fb5afec63b0f919732a50147229623186 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -20,6 +20,7 @@ config PCIE_DW_HOST
>   	bool
>   	select PCIE_DW
>   	select IRQ_MSI_LIB
> +	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 952f8594b501254d2b2de5d5e056e16d2aa8d4b7..abb93265a19fd62d3fecc64f29f37baf67291b40 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> @@ -413,6 +413,81 @@ 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};
> +	resource_size_t bus_range_max;
> +	struct resource_entry *bus;
> +	int ret;
> +
> +	bus = resource_list_first_type(&pp->bridge->windows, IORESOURCE_BUS);
> +
> +	/*
> +	 * Root bus under the host bridge doesn't require any iATU configuration
> +	 * as DBI region will be used to access root bus config 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.parent_bus_addr = pp->cfg0_base + SZ_1M;
> +	/* 1MiB is to cover 1 (bus) * 32 (devices) * 8 (functions) */
> +	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);
> +
> +	if (bus_range_max < 2)
> +		return 0;
> +
> +	/* Configure remaining buses in type 1 iATU configuration */
> +	atu.index = 1;
> +	atu.type = PCIE_ATU_TYPE_CFG1;
> +	atu.parent_bus_addr = pp->cfg0_base + SZ_2M;
> +	atu.size = (SZ_1M * bus_range_max) - SZ_2M;
> +	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;
> +
> +	return 0;
> +}
> +
> +static bool dw_pcie_ecam_enabled(struct dw_pcie_rp *pp, struct resource *config_res)
> +{
> +	struct resource *bus_range;
> +	u64 nr_buses;

As change is using Synopsis IP based iATU config shift mode 
functionality, it is must that ECAM/DBI base address has to be 256 MB 
aligned. Hence add change to check against alignment.

#define IS_256MB_ALIGNED(x) IS_ALIGNED(x, SZ_256M)

if (!IS_256MB_ALIGNED(config_res->start))
           return false;

> +
> +	bus_range = resource_list_first_type(&pp->bridge->windows, IORESOURCE_BUS)->res;
> +	if (!bus_range)
> +		return false;
> +
> +	nr_buses = resource_size(config_res) >> PCIE_ECAM_BUS_SHIFT;
> +
> +	return !!(nr_buses >= resource_size(bus_range));
> +}
> +
>   static int dw_pcie_host_get_resources(struct dw_pcie_rp *pp)
>   {
>   	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
> @@ -422,10 +497,6 @@ static int dw_pcie_host_get_resources(struct dw_pcie_rp *pp)
>   	struct resource *res;
>   	int ret;
>   
> -	ret = dw_pcie_get_resources(pci);
> -	if (ret)
> -		return ret;
> -
>   	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "config");
>   	if (!res) {
>   		dev_err(dev, "Missing \"config\" reg space\n");
> @@ -435,9 +506,32 @@ static int dw_pcie_host_get_resources(struct dw_pcie_rp *pp)
>   	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);
> +	pp->ecam_enabled = dw_pcie_ecam_enabled(pp, res);
> +	if (pp->ecam_enabled) {
> +		ret = dw_pcie_create_ecam_window(pp, res);
> +		if (ret)
> +			return ret;
> +
> +		pp->bridge->ops = (struct pci_ops *)&pci_generic_ecam_ops.pci_ops;
> +		pp->bridge->sysdata = pp->cfg;
> +		pp->cfg->priv = pp;
> +	} else {
> +		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 */
> +		pp->bridge->ops = &dw_pcie_ops;
> +		pp->bridge->child_ops = &dw_child_pcie_ops;
> +		pp->bridge->sysdata = pp;
> +	}
> +
> +	ret = dw_pcie_get_resources(pci);
> +	if (ret) {
> +		if (pp->cfg)
> +			pci_ecam_free(pp->cfg);
> +		return ret;
> +	}
>   
>   	/* Get the I/O range from DT */
>   	win = resource_list_first_type(&pp->bridge->windows, IORESOURCE_IO);
> @@ -476,14 +570,10 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
>   	if (ret)
>   		return ret;
>   
> -	/* Set default bus ops */
> -	bridge->ops = &dw_pcie_ops;
> -	bridge->child_ops = &dw_child_pcie_ops;
> -
>   	if (pp->ops->init) {
>   		ret = pp->ops->init(pp);
>   		if (ret)
> -			return ret;
> +			goto err_free_ecam;
>   	}
>   
>   	if (pci_msi_enabled()) {
> @@ -525,6 +615,14 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
>   	if (ret)
>   		goto err_free_msi;
>   
> +	if (pp->ecam_enabled) {
> +		ret = dw_pcie_config_ecam_iatu(pp);
> +		if (ret) {
> +			dev_err(dev, "Failed to configure iATU in ECAM mode\n");
> +			goto err_free_msi;
> +		}
> +	}
> +
>   	/*
>   	 * Allocate the resource for MSG TLP before programming the iATU
>   	 * outbound window in dw_pcie_setup_rc(). Since the allocation depends
> @@ -560,8 +658,6 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
>   		/* Ignore errors, the link may come up later */
>   		dw_pcie_wait_for_link(pci);
>   
> -	bridge->sysdata = pp;
> -
>   	ret = pci_host_probe(bridge);
>   	if (ret)
>   		goto err_stop_link;
> @@ -587,6 +683,10 @@ int dw_pcie_host_init(struct dw_pcie_rp *pp)
>   	if (pp->ops->deinit)
>   		pp->ops->deinit(pp);
>   
> +err_free_ecam:
> +	if (pp->cfg)
> +		pci_ecam_free(pp->cfg);
> +
>   	return ret;
>   }
>   EXPORT_SYMBOL_GPL(dw_pcie_host_init);
> @@ -609,6 +709,9 @@ void dw_pcie_host_deinit(struct dw_pcie_rp *pp)
>   
>   	if (pp->ops->deinit)
>   		pp->ops->deinit(pp);
> +
> +	if (pp->cfg)
> +		pci_ecam_free(pp->cfg);
>   }
>   EXPORT_SYMBOL_GPL(dw_pcie_host_deinit);
>   
> diff --git a/drivers/pci/controller/dwc/pcie-designware.c b/drivers/pci/controller/dwc/pcie-designware.c
> index 4684c671a81bee468f686a83cc992433b38af59d..6826ddb9478d41227fa011018cffa8d2242336a9 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.c
> +++ b/drivers/pci/controller/dwc/pcie-designware.c
> @@ -576,7 +576,7 @@ int dw_pcie_prog_outbound_atu(struct dw_pcie *pci,
>   		val = dw_pcie_enable_ecrc(val);
>   	dw_pcie_writel_atu_ob(pci, atu->index, PCIE_ATU_REGION_CTRL1, val);
>   
> -	val = PCIE_ATU_ENABLE;
> +	val = PCIE_ATU_ENABLE | atu->ctrl2;
>   	if (atu->type == PCIE_ATU_TYPE_MSG) {
>   		/* The data-less messages only for now */
>   		val |= PCIE_ATU_INHIBIT_PAYLOAD | atu->code;
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index ceb022506c3191cd8fe580411526e20cc3758fed..f770e160ce7c538e0835e7cf80bae9ed099f906c 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -20,6 +20,7 @@
>   #include <linux/irq.h>
>   #include <linux/msi.h>
>   #include <linux/pci.h>
> +#include <linux/pci-ecam.h>
>   #include <linux/reset.h>
>   
>   #include <linux/pci-epc.h>
> @@ -169,6 +170,7 @@
>   #define PCIE_ATU_REGION_CTRL2		0x004
>   #define PCIE_ATU_ENABLE			BIT(31)
>   #define PCIE_ATU_BAR_MODE_ENABLE	BIT(30)
> +#define PCIE_ATU_CFG_SHIFT_MODE_ENABLE	BIT(28)
>   #define PCIE_ATU_INHIBIT_PAYLOAD	BIT(22)
>   #define PCIE_ATU_FUNC_NUM_MATCH_EN      BIT(19)
>   #define PCIE_ATU_LOWER_BASE		0x008
> @@ -387,6 +389,7 @@ struct dw_pcie_ob_atu_cfg {
>   	u8 func_no;
>   	u8 code;
>   	u8 routing;
> +	u32 ctrl2;
>   	u64 parent_bus_addr;
>   	u64 pci_addr;
>   	u64 size;
> @@ -425,6 +428,8 @@ struct dw_pcie_rp {
>   	struct resource		*msg_res;
>   	bool			use_linkup_irq;
>   	struct pci_eq_presets	presets;
> +	bool			ecam_enabled;
> +	struct pci_config_window *cfg;
>   };
>   
>   struct dw_pcie_ep_ops {
> 
Regards,
Mayank

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ