Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Tom Joseph <tjoseph@cadence.com>
To:     Li Chen <lchen@ambarella.com>
CC:     Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>,
        Rob Herring <robh@kernel.org>,
        =?iso-8859-2?Q?Krzysztof_Wilczy=F1ski?= <kw@linux.com>,
        Bjorn Helgaas <bhelgaas@google.com>,
        "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: Why does cdns_pcie_ep_set_bar use sz > SZ_2G for is_64bits in
 pcie-cadence-ep.c?
Thread-Topic: Why does cdns_pcie_ep_set_bar use sz > SZ_2G for is_64bits in
 pcie-cadence-ep.c?
Thread-Index: AdgNFhFdLIAJoUumRauHA5pdsoTIFwA1O41wACEoNOAAHaA/QA==
Date:   Fri, 21 Jan 2022 18:08:47 +0000
Message-ID: <BN8PR07MB6195FAE77802B3B04DE87C71A15B9@BN8PR07MB6195.namprd07.prod.outlook.com>
References: <PH7PR19MB55626CD5D22EABDEC879EF41A0599@PH7PR19MB5562.namprd19.prod.outlook.com>
 <MN2PR07MB62084589DE98AD2611C55FFDA15A9@MN2PR07MB6208.namprd07.prod.outlook.com>
 <PH7PR19MB55625B4733F2F45133AA4EACA05B9@PH7PR19MB5562.namprd19.prod.outlook.com>
In-Reply-To: <PH7PR19MB55625B4733F2F45133AA4EACA05B9@PH7PR19MB5562.namprd19.prod.outlook.com>
Accept-Language: en-GB, en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?iso-8859-2?Q?5OQoxwG1Xnks4+dzjC5iO/rkkH6Z1XO5ZcGJMiER8kPjzdSVE81WjgAGPe?=
 =?iso-8859-2?Q?y94WQ7DgogUy1AWt91VCfEzPls6xhfYgP0v9AK7JanApfgvAY4P8bQKYJt?=
 =?iso-8859-2?Q?yPRdWQCcTWRTFGbnH+7TFSiyc3Vr2T6pmYPlViVwMbZdwlR6J6eJlwFl04?=
 =?iso-8859-2?Q?oe2v7HBSnLwSTrUNYOmSGj7+tfyoaQ96H3+H/quJkxFQwwF6YdzYhM6Dn1?=
 =?iso-8859-2?Q?fLUX0i+RBvCQW8Ohb8sTdbQML9/JBBA55PXshRSRFPd/mkhPpmSDpnxGUz?=
 =?iso-8859-2?Q?Tu2Ru2rdqJT0STn9UUWnVFc263mHFMmxi2G/ZcfHtw4Eiz9z50mGdYMHLX?=
 =?iso-8859-2?Q?kGz1wUdbDjgjG5HBNfCUM1Zmhg6r/s82tafWW5cFE2cF93DbF62Pl+T9yb?=
 =?iso-8859-2?Q?taLSf+AJNow7MhStAUb1tMz3n1+5Qw2DFryMxjXTEfl8KpfVNQupWrtl6v?=
 =?iso-8859-2?Q?wYE0VUlHedDUtEuhX9MPyieVeaNyg+hDdcI3UAm550wTAy82ImMlt4jCSW?=
 =?iso-8859-2?Q?w4b3ERvIi6a9joXifEH19MsVMTYAaSKqDYaoVcxzjm3hQIvmyVhgy+zdMM?=
 =?iso-8859-2?Q?sb6YZ1DASP2VcQIso7TsPJvwQuhnE0HiFxW6QDDR9YdhkFXMCUIZD1Stx3?=
 =?iso-8859-2?Q?awudLrDSENtt0ShqgxePYRnWRTyFbiX+p0XRMYF4eE8W5tBuuoUwj33e7H?=
 =?iso-8859-2?Q?SeokTbD4tMUGl8lTBHa9SfPvvvd9Uh2nubYWpixw0jy2nDunaZbIk6gbPy?=
 =?iso-8859-2?Q?muAZ2DJiCRXm6shkAnVlBKRApW6sdDCdEReMZKzWO9WerKSxA0+klGrIVM?=
 =?iso-8859-2?Q?zs+uvD7pmE7/dnDwScIsgAK/jTFZ4S/kVRER98DSurSFfN9rtBYssXBqFb?=
 =?iso-8859-2?Q?0gw02tyd2IlpE2zRFt8x+rwOdZ9CWPK/ivelRK/6OfN7PFQbr3QSCZyTDK?=
 =?iso-8859-2?Q?z3/XoQJn/0QBte4TEpZg685I26Z0vXkfN0pyZgV+AMaKe92PjwbmeGbc2C?=
 =?iso-8859-2?Q?5Fdim/RXY000riMSc8vjsXqmlDa/0F+cAsXtL5BOz8lUsAw32/nA4sleqx?=
 =?iso-8859-2?Q?2PioGYSkJx4aaIgG2q3mt41AxK0pf0JJjGMeeovBb/BfGfvToFRFhulukI?=
 =?iso-8859-2?Q?YMdFtHhLt6s4JzBicra2MqSX2Sn+Kz5oVPajdRJZOQPf9YkhwhcYXgbl8t?=
 =?iso-8859-2?Q?2Ay6IjQP0cBY9gCi8vFuXjApewnTmXHH4b3BRW6pUKX+uKLcwnNwaK3IKq?=
 =?iso-8859-2?Q?nXCWJp80k1OPYluEjHrrPtursZOysqGe5ExqaijgG9Q8JYInDw2QGzlBDW?=
 =?iso-8859-2?Q?xfc94EwzjX8/w/2m3TdFkS1BUbesdavi5NcnNs80qZ6eMDpXQC/ttQ+snF?=
 =?iso-8859-2?Q?j3IhNhKlOnjSeTh/PLcrbTVk7dU/GeoFPewo6Ld3Xt5WFQ8Fre5wfrxOv2?=
 =?iso-8859-2?Q?7jGq/Aq4vAeBzNukhyT143Ebc1C7XOKckGT5LKlXrU3S8vVXW/u+RPv4+M?=
 =?iso-8859-2?Q?4Sc3bWhdBRoqZwdDGBPjamdJETNDFrVkEWR/vOc2wb9+aAHkpdcZmodc8e?=
 =?iso-8859-2?Q?+8C9C0nLEPcIJJaqHQFlb7HGFgUISinGYhnVYh2M+NkUiEabQkcBMxcsVH?=
 =?iso-8859-2?Q?PtN3bbarSAXHqmN3l7m5C+GOkMySf5OjhaHu80zdAfD0gyYtbQFfit/Jlu?=
 =?iso-8859-2?Q?fb1DdJnSOLK2PPOh3NSUsjcsCoYCCchvBJC8Rg/WkCcTkUJs0Eqv5ohCid?=
 =?iso-8859-2?Q?onsQ=3D=3D?=
Content-Type: text/plain; charset="iso-8859-2"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: cadence.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN8PR07MB6195.namprd07.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: dd39d173-75df-4039-3da2-08d9dd0911ef
X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Jan 2022 18:08:47.4190
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: d36035c5-6ce6-4662-a3dc-e762e61ae4c9
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: fMzR1wqt6+84O2dbpV6Qjtbfu+s1eDPNb0jHMv32kkmacu9vzpEx0D4W4t6IR7qxbMwCYtiw/BQk1eKN+yO7S8WtsO6OnJ82oGz7/abd4q0=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN8PR07MB6338
X-Proofpoint-GUID: hBIXrqFvzCo_NCUS5yvFsyNeF49ehwJa
X-Proofpoint-ORIG-GUID: hBIXrqFvzCo_NCUS5yvFsyNeF49ehwJa
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.816,Hydra:6.0.425,FMLib:17.11.62.513
 definitions=2022-01-21_09,2022-01-21_01,2021-12-02_01
X-Proofpoint-Spam-Details: rule=outbound_check_notspam policy=outbound_check score=0
 priorityscore=1501 mlxlogscore=569 clxscore=1015 bulkscore=0
 malwarescore=0 impostorscore=0 spamscore=0 suspectscore=0 adultscore=0
 phishscore=0 lowpriorityscore=0 mlxscore=0 classifier=spam adjust=0
 reason=mlx scancount=1 engine=8.12.0-2201110000
 definitions=main-2201210119
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Li,

> -----Original Message-----
> From: Li Chen <lchen@ambarella.com>
> Sent: 21 January 2022 02:56
> To: Tom Joseph <tjoseph@cadence.com>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>; Rob Herring
> <robh@kernel.org>; Krzysztof Wilczy=F1ski <kw@linux.com>; Bjorn Helgaas
> <bhelgaas@google.com>; linux-pci@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: RE: Why does cdns_pcie_ep_set_bar use sz > SZ_2G for is_64bits i=
n
> pcie-cadence-ep.c?
>=20
> EXTERNAL MAIL
>=20
>=20
> Hi, Tom
>=20
> > -----Original Message-----
> > From: Tom Joseph [mailto:tjoseph@cadence.com]
> > Sent: Thursday, January 20, 2022 9:11 PM
> > To: Li Chen
> > Cc: Lorenzo Pieralisi; Rob Herring; Krzysztof Wilczy=F1ski; Bjorn Helga=
as; linux-
> > pci@vger.kernel.org; linux-kernel@vger.kernel.org
> > Subject: [EXT] RE: Why does cdns_pcie_ep_set_bar use sz > SZ_2G for
> is_64bits
> > in pcie-cadence-ep.c?
> >
> > Hi Li,
> >
> >  For 64_bits ,  all the odd bars (BAR1, 3 ,5) will be disabled ( so as =
to use as
> upper
> > bits).
>=20
> Yes, I get it.
> > I see that the code is assuming 32_bits if size < 2G , so all bars coul=
d be
> enabled.
> >
>=20
> Ok, but I still wonder why 2G instead of other sizes like 1G or 512M? IMO=
 if
> there is no obvious limitation, 64 or 32 bit should be left to the user t=
o
> decide(like bar_fixed_64bit and bar_fixed_size in pci_epc_features), inst=
ead
> of hardcode 2G here.

Check for 2G is important as this is the max valid aperture size encoding (=
0x11000)
allowed by the controller for 32 bit BARs.

>=20
> > As I understand, you have a use case where you want to set the bar as 6=
4
> bit,
> > actually use small size.
> > Is it possible to describe bit more about this use case (just curious)?
>=20
> It's because our SoC use 0-64G AMBA address space for our dram(system
> memory), so if I want to use 32 bit bar like 16M bus address, I must rese=
rve
> this 16M area with kernel's reserve-memory, otherwise endpoints like nvme
> will report unsupported request when it do dma and the dma address is als=
o
> located under this 16M area. More details have been put in this thread:
> https://urldefense.com/v3/__https://lore.kernel.org/lkml/CH2PR19MB4024
> 5BF88CF2F7210DCB1AA4A0669@CH2PR19MB4024.namprd19.prod.outlook.c
> om/T/*m0dd09b7e6f868b9692185ec57c1986b3c786e8d3__;Iw!!EHscmS1ygiU
> 1lA!SVgmPO0MrmUUnZzlmc-fCPsSBddkfUgT-
> Y7EmlLAgz9AoQSVZXa_18TIdT6O7kY$
>=20
>=20
> So, if I don't want to reserve much memory for BAR, I have to use 64-bit =
bar,
> and it must be prefetch 64 bit, not non-prefetch in my case, because my
> virtual p2p bridge has three windows: io, mem(32bit), prefetch mem(64 bit=
,
> because CDNS_PCIE_LM_RC_BAR_CFG_PREFETCH_MEM_64BITS is set), and
> if the controller running under ep-mode use 64 non-prefetch, this region =
will
> fallback to bridge's 32-bit mem window but I don't(and cannot) reserve so
> much 32bit memory for it).
>=20
Got your point. Does a change in the code as below will be good enough ?

- bool is_64bits =3D sz > SZ_2G;=20
+bool is_64bits =3D (sz > SZ_2G) ||(!!( flags &  PCI_BASE_ADDRESS_MEM_TYPE_=
64)) ;


Thanks,
Tom