linux-kernel - Re: [PATCH 2/4] PCI: endpoint: pci-epf-vntb: align mw naming with config names

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aC3/PL3jdGMVHX3n@lizhi-Precision-Tower-5810>
Date: Wed, 21 May 2025 12:28:44 -0400
From: Frank Li <Frank.li@....com>
To: Jerome Brunet <jbrunet@...libre.com>
Cc: Jon Mason <jdmason@...zu.us>, Dave Jiang <dave.jiang@...el.com>,
	Allen Hubbe <allenbh@...il.com>,
	Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
	Krzysztof Wilczyński <kw@...ux.com>,
	Kishon Vijay Abraham I <kishon@...nel.org>,
	Bjorn Helgaas <bhelgaas@...gle.com>, ntb@...ts.linux.dev,
	linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/4] PCI: endpoint: pci-epf-vntb: align mw naming with
 config names

On Tue, May 20, 2025 at 09:47:14AM +0200, Jerome Brunet wrote:
> On Mon 19 May 2025 at 17:21, Frank Li <Frank.li@....com> wrote:
>
> > On Mon, May 05, 2025 at 07:41:48PM +0200, Jerome Brunet wrote:
> >
> > PCI tree require keep consistent at subject
> > git log --oneline drivers/pci/endpoint/functions/pci-epf-vntb.c
> >
> > require first char is UP case.
>
> Noted
>
> >
> > Align memory window naming with configfs names.
> >
> >> The config file related to the memory windows start the numbering of
> >
> >                                  memory windows (MW)
> >  then you can use MW later.
>
> Sure
>
> >
> >> the MW from 1. The other NTB function does the same, yet the enumeration
> >> defining the BARs of the vNTB function starts numbering the MW from 0.
> >>
> >> Both numbering are fine I suppose but mixing the two is a bit confusing.
> >> The configfs file being the interface with userspace, lets keep that stable
> >> and consistently start the numbering of the MW from 1.
> >>
> >> Signed-off-by: Jerome Brunet <jbrunet@...libre.com>
> >> ---
> >>  drivers/pci/endpoint/functions/pci-epf-vntb.c | 11 ++++++-----
> >>  1 file changed, 6 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> >> index 35fa0a21fc91100a5539bff775e7ebc25e1fb9c1..f9f4a8bb65f364962dbf1e9011ab0e4479c61034 100644
> >> --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> >> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> >> @@ -70,9 +70,10 @@ static struct workqueue_struct *kpcintb_workqueue;
> >>  enum epf_ntb_bar {
> >>  	BAR_CONFIG,
> >>  	BAR_DB,
> >> -	BAR_MW0,
> >>  	BAR_MW1,
> >>  	BAR_MW2,
> >> +	BAR_MW3,
> >> +	BAR_MW4,
> >
> > where use BAR_MW3 and BAR_MW4?
>
> This is aligned with the file available in configfs and what is possible
> in theory with the function, same as the NTB function and NTB host driver.
>
> Stopping at MW1 because it is only one used in the driver would be weird
> and the number later introduced would be wrong.

Yes, but BAR_MW3 and BAR_MW4 should be added only when both was used in code
actaully.

Frank
>
>
> >
> > Frank
> >>  };
> >>
> >>  /*
> >> @@ -576,7 +577,7 @@ static int epf_ntb_mw_bar_init(struct epf_ntb *ntb)
> >>
> >>  	for (i = 0; i < ntb->num_mws; i++) {
> >>  		size = ntb->mws_size[i];
> >> -		barno = ntb->epf_ntb_bar[BAR_MW0 + i];
> >> +		barno = ntb->epf_ntb_bar[BAR_MW1 + i];
> >>
> >>  		ntb->epf->bar[barno].barno = barno;
> >>  		ntb->epf->bar[barno].size = size;
> >> @@ -629,7 +630,7 @@ static void epf_ntb_mw_bar_clear(struct epf_ntb *ntb, int num_mws)
> >>  	int i;
> >>
> >>  	for (i = 0; i < num_mws; i++) {
> >> -		barno = ntb->epf_ntb_bar[BAR_MW0 + i];
> >> +		barno = ntb->epf_ntb_bar[BAR_MW1 + i];
> >>  		pci_epc_clear_bar(ntb->epf->epc,
> >>  				  ntb->epf->func_no,
> >>  				  ntb->epf->vfunc_no,
> >> @@ -676,7 +677,7 @@ static int epf_ntb_init_epc_bar(struct epf_ntb *ntb)
> >>  	epc_features = pci_epc_get_features(ntb->epf->epc, ntb->epf->func_no, ntb->epf->vfunc_no);
> >>
> >>  	/* These are required BARs which are mandatory for NTB functionality */
> >> -	for (bar = BAR_CONFIG; bar <= BAR_MW0; bar++, barno++) {
> >> +	for (bar = BAR_CONFIG; bar <= BAR_MW1; bar++, barno++) {
> >>  		barno = pci_epc_get_next_free_bar(epc_features, barno);
> >>  		if (barno < 0) {
> >>  			dev_err(dev, "Fail to get NTB function BAR\n");
> >> @@ -1048,7 +1049,7 @@ static int vntb_epf_mw_set_trans(struct ntb_dev *ndev, int pidx, int idx,
> >>  	struct device *dev;
> >>
> >>  	dev = &ntb->ntb.dev;
> >> -	barno = ntb->epf_ntb_bar[BAR_MW0 + idx];
> >> +	barno = ntb->epf_ntb_bar[BAR_MW1 + idx];
> >>  	epf_bar = &ntb->epf->bar[barno];
> >>  	epf_bar->phys_addr = addr;
> >>  	epf_bar->barno = barno;
> >>
> >> --
> >> 2.47.2
> >>
>
> --
> Jerome