Received-SPF: pass (google.com: domain of linux-kernel+bounces-7293-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Wed, 20 Dec 2023 11:24:18 -0500
From: Frank Li <Frank.li@nxp.com>
To: Manivannan Sadhasivam <mani@kernel.org>
Cc: allenbh@gmail.com, bhelgaas@google.com, dave.jiang@intel.com,
	imx@lists.linux.dev, jdmason@kudzu.us, kishon@kernel.org,
	kw@linux.com, linux-kernel@vger.kernel.org,
	linux-pci@vger.kernel.org, lpieralisi@kernel.org,
	ntb@lists.linux.dev
Subject: Re: [PATCH v2 1/1] PCI: endpoint: pci-epf-vntb: Fix transfer fail
 when BAR1 is fixed size
Message-ID: <ZYMVMo3TVPMiEN/L@lizhi-Precision-Tower-5810>
References: <20231219142403.1223873-1-Frank.Li@nxp.com>
 <20231220142736.GE3544@thinkpad>
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20231220142736.GE3544@thinkpad>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?dmZZRENpcVRSa1c5Sk0wcmcxb3laV3hqdDFscFVRc1hOaFNiRUQ5aXFUV0th?=
 =?utf-8?B?SFRNZzUyMVE4RENqQUZOYUIyTTNEQmd0eHA5UFhwVGZkN3NaZUYwbGdlSjV1?=
 =?utf-8?B?eUY4RURmbng5bUZsSjRXeXgrTUJaNWNKYVllZGtsZ1NHbmZWUTZFYzgySjBC?=
 =?utf-8?B?WEJCUCtRUmJZM0t4dGlZMWJMRExCaVBqQUYxeGIwd1ZLOGhzcksrVHREN0d5?=
 =?utf-8?B?dlVMbVBWbVZBT1VxbDU3dVRGV3FsM2VPckVxcHFJRnFNSWtZbzNReWVjOFJw?=
 =?utf-8?B?V3JySk1XR3BmK1ViL0g2Y2M4Q3dtRmMwQkxZb0YxdVYwNm0vbk5XR0s1WUpN?=
 =?utf-8?B?TVcvSk5uNlNubU9BZVp0V2d5VFJaWWZhVkl0cFpMbHlGSTUyS3hMbVBhTDBu?=
 =?utf-8?B?ZlZUSTJpK0pMMU9qYWU0dTMvT0swaHpYOU9lbHVTVEN4WFJSV1ViTzI0cnZ0?=
 =?utf-8?B?VUF3Z25jVWJGMHhGTzZFSW9QamFCckd5Z3dNU3B3MzltekxrakhSNGV5aUdD?=
 =?utf-8?B?bWc2VjNxS2dBdU13NWpTMWRVRUZ3dDdteklyYVp1RnZaT2xxOC92VWczRFpq?=
 =?utf-8?B?VDJEb255WEp2QWtsNFFqR2J1eTBFaDVtWWlRZDZ4Mm5kcTMzREIrTFFOTWRQ?=
 =?utf-8?B?V3pRVXYyMlcvL0xyL2lyQ1BLM3J4bTBqeWsrQ2dTNTdpblpXTUVwbCs2NTU0?=
 =?utf-8?B?ZWEwL2RWekFBb1VWR2dyb1A3M1Nqby9VY28zb2FwM25oajBzUHpCcW0ydW45?=
 =?utf-8?B?REJTajA2V2l0dWUreFU3NGdPcWhrdFAxcUFJQ1o0K2ZWTjBDc2Z5eFVLVXk3?=
 =?utf-8?B?VGlkT1B6VUdsRE5VaFcyZ1E4Q2pkSVMya2t2eUxDMGdERVp3bEZJZ1ZWVG43?=
 =?utf-8?B?TlkyenBPbUd0SGJncmVEVW9wTy8wdkc4cEhvVmlUeG0vQkpUZFhDYW5HV0FG?=
 =?utf-8?B?eHd3b1kxVHpkTldtZERlcVRzU1hZT1VVQWxlNFQzNm1ZQmdBQmlTZTlSQ0Z3?=
 =?utf-8?B?M21sdlRIb2c0UkhsK3hHbmhqNHZZcldvWmN6U2gzN1BwemVOS1lYdENic1VQ?=
 =?utf-8?B?Q25mK2hELzcyTnlBVXJqT0xDMHRjSzJnQlFYRnZQTGZFUWxucHE0YkZNcWtS?=
 =?utf-8?B?dFFXOGZ5Y0JUZk1lYzJxb0h1ZkY5ckw0c2xMVUdRQlZPelpMSXFwbnJsNk1Z?=
 =?utf-8?B?d3lJTjBoZ2tHTk5SUjRkOXNRYW12SG9OcDExVjBaSmNhL2JaYzkyeTJ6Ynhk?=
 =?utf-8?B?UDJ0VXdtNXdYTGVKdHQySGxUMWJremlzVlZtVTdoQlhrUzRZU2RMcFdVMHVW?=
 =?utf-8?B?VXJwVHVlb3NyQktMS0VXNmJ4OFlWZ3BIUkFBVGNEdmZ0djV3dkdOcmxERDUz?=
 =?utf-8?B?VTNCenphbFdRVVUxeEhPbEdFYWhib0R4VzNOdy8vRkZxUUNzaFl5dlV0NVdw?=
 =?utf-8?B?MU14NVVYUjU0ZHlNUjJ2bVkySFAzQUtmRnpmUnB4Z1diYlh3cStZZktyZCtJ?=
 =?utf-8?B?V2VOMUxVMmpJODFWb2dra2JIdXdPNllMUEpGZ2prTmIzNzN3YmlDYzA3amtM?=
 =?utf-8?B?NGU4VTNiUFdOeDI2ZWhvaEQ2SFpVUTFXMEtHTnNqOGpUaWJRU0l6NTArNEs3?=
 =?utf-8?B?eFVtSmhoSGRLTjJlSlRNUTZjMTFKU3BRS1hZd21vamd4dENqZndOY3lOaVVE?=
 =?utf-8?B?QWdtNVZialA3QVRLYi9leU5ZT2xUaFVqYy9zbVNDcURiV0tPRmdDZ2ttS01z?=
 =?utf-8?B?VzQ1bVpOL1NrS0NDNjl1bEpJMTYxMTNVcEtIZEF6QUh6Z2wya3RzeGtJV0sv?=
 =?utf-8?B?N1hyZjY5QzJqakZQMFVlanl2QjdIMVBNb0piVUk0YzNwVWg3S1EvdWwrbU5y?=
 =?utf-8?B?azJTZ25Qc2J5ZFk3SXZEcy9jeC81TWpGc2R6TVdxODk4ZlEzSzlFQmJoRkgv?=
 =?utf-8?B?OFZZd2EvUmFZa1d5eldHMm1SQnhvTlVvcFBuMzVOSEM0TUxQdFRMU3hDSi95?=
 =?utf-8?B?V2cwUkhwV2h2MVpTNkk2YnR5UjJsL21yWXlvTjlXKzZ0TlZONUdDendEZkFJ?=
 =?utf-8?B?VmsyUy83YXRmdVpxb2s2NTJ1S0NQd0srWjZZMk51bFVEQjA2c3FIN3NDOER1?=
 =?utf-8?Q?6Ikc=3D?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 92c46b32-d9ae-484e-6fee-08dc01782177
X-MS-Exchange-CrossTenant-AuthSource: AM6PR04MB4838.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Dec 2023 16:24:25.1248
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 5dGuqjSEE4FgHc33TdFtlpWrbyNQ6tc5L1fS2RZ4zyZXkHRFUOmXEoFM5hsMkNvNGYnBFeLbWVtlz/B1Pb2Ttg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DU0PR04MB9494

On Wed, Dec 20, 2023 at 07:57:36PM +0530, Manivannan Sadhasivam wrote:
> On Tue, Dec 19, 2023 at 09:24:03AM -0500, Frank Li wrote:
> > ntb_netdev transfer is failing when epc controller's BAR1 is fix size, such
> > as 64K. Certain controller(like dwc) require memory address must be align
> > with the fixed bar size.
> > 
> > For example:
> > 	If BAR1's fix size is 64K, and other size programmable BAR's
> > alignment is 4K.
> > 	vntb call pci_epf_alloc_space() get 4K aligned address, like
> > 0x104E31000. But root complex actually write to address 0x104E30000 when
> > write BAR1.
> > 
> > Adds bar_fixed_size check and sets correct alignment for fixed-size BAR.
> > 
> 
> Change looks fine by me, but I have a hard time understanding this commit
> message.
> 
> The change just checks the size of the doorbell BAR if a fixed size BAR is used
> by the controller and uses the fixed size. In the commit message you are talking
> about alignment and root complex writing to the BAR which are just confusing.
> 
> Please reword this commit message to make it understandable.

Maybe I talk about too much about it. Actually, supposed it should  work if
use fixed-size bar(64K), which actually only mapped 4k, if RC only use 4k
also.

"copy from dwc spec"

But dwc iATU IATU_LWR_TARGET_ADDR_OFF_INBOUND_0
11-0
LWR_TARGET_HW
Forms the LSB's of the Lower Target part of the new address of the translated region.
Forms the LSB's of the Lower Target part of the new address of the translated region. The start address must be aligned to a CX_ATU_MIN_REGION_SIZE kB boundary (in address match mode); and to the Bar size boundary (in BAR match mode) so that these bits are always '0'. If the BAR is smaller than the iATU region size, then the iATU target address must align to the iATU region size; otherwise it must align to the BAR size.
A write to this location is ignored by the PCIe controller.
- Field size depends on log2(CX_ATU_MIN_REGION_SIZE) in address match mode.
- Field size depends on log2(BAR_MASK+1) in BAR match mode.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

For fixed size bar, BAR_MASK is fixed value. If pass down a 4k aligned
address to iATU for 64K fixed sized bar, some address bits will be
truncated. 

Hidden some hardware detail, do you think if it is enough?

"ntb_netdev transfer is failing when epc controller's BAR1 is fix size.
Adds bar_fixed_size check. If require memory bigger than BAR1's fixed size,
return -ENOMEM. If smaller than BAR1's fixed size, use BAR1's fixed size."

Frank

> 
> > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > ---
> > 
> > Notes:
> >     Change from v1 to v2
> >     - Remove unnessary set align when fix_bar_size.
> > 
> >  drivers/pci/endpoint/functions/pci-epf-vntb.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > index 3f60128560ed0..ec3922f404efe 100644
> > --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > @@ -550,6 +550,14 @@ static int epf_ntb_db_bar_init(struct epf_ntb *ntb)
> >  
> >  	barno = ntb->epf_ntb_bar[BAR_DB];
> >  
> > +	if (epc_features->bar_fixed_size[barno]) {
> > +		if (size > epc_features->bar_fixed_size[barno]) {
> > +			dev_err(dev, "Fixed BAR%d is too small for doorbell\n", barno);
> > +			return -EINVAL;
> 
> -ENOMEM?
> 
> - Mani
> 
> > +		}
> > +		size = epc_features->bar_fixed_size[barno];
> > +	}
> > +
> >  	mw_addr = pci_epf_alloc_space(ntb->epf, size, barno, align, 0);
> >  	if (!mw_addr) {
> >  		dev_err(dev, "Failed to allocate OB address\n");
> > -- 
> > 2.34.1
> > 
> 
> -- 
> மணிவண்ணன் சதாசிவம்