linux-kernel - Re: [PATCH 2/3] PCI: dwc: Add ECAM support with iATU configuration

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: <20241209235530.GA3221939@bhelgaas>
Date: Mon, 9 Dec 2024 17:55:30 -0600
From: Bjorn Helgaas <helgaas@...nel.org>
To: Krishna Chaitanya Chundru <quic_krichai@...cinc.com>
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>,
	Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
	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 Mon, Dec 09, 2024 at 10:00:06AM +0530, Krishna Chaitanya Chundru wrote:
> On 12/5/2024 3:47 AM, Bjorn Helgaas wrote:
> > On Wed, Dec 04, 2024 at 07:45:29AM +0530, Krishna Chaitanya Chundru wrote:
> > > On 12/4/2024 12:25 AM, Bjorn Helgaas 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
> > > > > config space, which avoids iATU configuration for every config access.
> > 
> > > > > +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
> > > > 
> > > > I'm confused about the bus numbering; you refer to "bus 1" and "bus
> > > > 2".  Is bus 1 the root bus, i.e., the primary bus of a Root Port?
> > > > 
> > > > The root bus number would typically be 0, not 1, and is sometimes
> > > > programmable.  I don't know how the DesignWare core works, but since
> > > > you have "bus" here, referring to "bus 1" and "bus 2" here seems
> > > > overly specific.
> > > > 
> > > root bus is bus 0 and we don't need any iATU configuration for it as
> > > its config space is accessible from the system memory, for usp port of
> > > the switch or the direct the endpoint i.e bus 1 we need to send
> > > Configuration Type 0 requests and for other buses we need to send
> > > Configuration Type 1 requests this is as per PCIe spec, I will try to
> > > include PCIe spec details in next patch.
> > 
> > I understand the Type 0/Type 1 differences.  The question is whether
> > the root bus number is hard-wired to 0.
> > 
> It is not hard-wired to 0, we can configure it though bus-range property
>
> > I don't think specifying "bus 1" really adds anything.  The point is
> > that we need Type 0 accesses for anything directly below a Root Port
> > (regardless of what the RP's secondary bus number is), and Type 1 for
> > things deeper.
>
> I will update the comment without mentioning the buses as suggested.
>
> > When DWC supports multiple Root Ports in a Root Complex, they will not
> > all have a secondary bus number of 1.
>
> mostly they should be in bus number 0 with different device numbers, but
> it mostly depends upon the design, currently we don't have any multiple
> root ports.

Say "root bus" instead of "bus 0", since you said above that the root
bus number is configurable.

Root Ports should all have a *primary* bus number of the root bus, but
if there are multiple Root Ports, they will all have different
secondary bus numbers.