linux-kernel - Re: [PATCH v7 3/6] PCI: endpoint: Add pci_epf_align

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: <ZzeGa-aWlv6bV3fv@ryzen>
Date: Fri, 15 Nov 2024 18:35:39 +0100
From: Niklas Cassel <cassel@...nel.org>
To: Frank Li <Frank.li@....com>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
	Krzysztof Wilczyński <kw@...ux.com>,
	Kishon Vijay Abraham I <kishon@...nel.org>,
	Bjorn Helgaas <bhelgaas@...gle.com>, Arnd Bergmann <arnd@...db.de>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
	imx@...ts.linux.dev, dlemoal@...nel.org, maz@...nel.org,
	tglx@...utronix.de, jdmason@...zu.us
Subject: Re: [PATCH v7 3/6] PCI: endpoint: Add pci_epf_align_addr() helper
 for address alignment

On Fri, Nov 15, 2024 at 11:43:22AM -0500, Frank Li wrote:
> On Fri, Nov 15, 2024 at 10:53:07AM +0100, Niklas Cassel wrote:
> > On Thu, Nov 14, 2024 at 05:52:39PM -0500, Frank Li wrote:
> > > +static inline int pci_epf_align_addr_lo_hi(struct pci_epf *epf, enum pci_barno bar,
> > > +					   u32 low, u32 high, u64 *base, size_t *off)
> > > +{
> > > +	u64 addr = high;
> > > +
> > > +	addr <<= 32;
> > > +	addr |= low;
> > > +
> > > +	return pci_epf_align_addr(epf, bar, addr, base, off);
> > > +}
> >
> > I'm not sure if this function deserves to live :)
> > Can't the caller just do this before calling pci_epf_align_addr() ?
> 
> Ideally, kernel should have macro to combine 32bit macro to a 64bit, but
> I have not found it.
> 
> It is quite easy to make error or warning by simple
> (high << 32 | low)
> 
> It needs ((u64) high << 32 | low) at least. I just want to avoid everyone
> to struggle simple issue.
> 
> And msi_msg use lo and hi. pci_function_test actually just demostrate how
> to use doorbell. if other function driver use doorbell in future, avoid
> "(u64) high << 32 | low" copy to everywhere.

Keeping the helper is fine with me, but it probably should be renamed
to include _ib_ or _inbound_ in the function name, similar to the comment
for pci_epf_align_addr().


Kind regards,
Niklas