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] [thread-next>] [day] [month] [year] [list]
Message-ID: <aXyMWIHy_gMnIxOX@ryzen>
Date: Fri, 30 Jan 2026 11:47:52 +0100
From: Niklas Cassel <cassel@...nel.org>
To: Aksh Garg <a-garg7@...com>
Cc: Koichiro Den <den@...inux.co.jp>, linux-pci@...r.kernel.org,
	jingoohan1@...il.com, mani@...nel.org, lpieralisi@...nel.org,
	kwilczynski@...nel.org, robh@...nel.org, bhelgaas@...gle.com,
	Zhiqiang.Hou@....com, gustavo.pimentel@...opsys.com,
	linux-kernel@...r.kernel.org, s-vadapalli@...com,
	danishanwar@...com
Subject: Re: [PATCH v4 2/3] PCI: dwc: ep: Add per-PF BAR and inbound ATU
 mapping support

On Fri, Jan 30, 2026 at 04:09:26PM +0530, Aksh Garg wrote:
> On 30/01/26 15:23, Niklas Cassel wrote:
> > On Fri, Jan 30, 2026 at 09:42:43AM +0530, Aksh Garg wrote:
> > > On 29/01/26 19:44, Niklas Cassel wrote:
> > > > On Thu, Jan 29, 2026 at 02:47:52PM +0530, Aksh Garg wrote:
> > > > > -static void dw_pcie_ep_clear_ib_maps(struct dw_pcie_ep *ep, enum pci_barno bar)
> > > > > +static void dw_pcie_ep_clear_ib_maps(struct dw_pcie_ep *ep, u8 func_no, enum pci_barno bar)
> > > > >  {
> > > > > +	struct dw_pcie_ep_func *ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
> > > > >  	struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > > >  	struct device *dev = pci->dev;
> > > > >  	unsigned int i, num;
> > > > > @@ -152,18 +157,18 @@ static void dw_pcie_ep_clear_ib_maps(struct dw_pcie_ep *ep, enum pci_barno bar)
> > > > >  	u32 *indexes;
> > > > > Hello Aksh,
> > > > > Considering that all other functions that you have modified, you
> > > have added a:
> > > > > if (!ep_func)
> > > > 	return;
> > > > > > I think you should do the same to this function.
> > > >
> > > 
> > > I omitted this NULL check here because all the current call sites of
> > > dw_pcie_ep_clear_ib_maps() already perform this validation. I felt
> > > adding it here would add redundancy in the code.
> > 
> > Ok, but with that logic, shouldn't we also remove the NULL checks from
> > dw_pcie_ep_ib_atu_bar() and dw_pcie_ep_ib_atu_addr(), because they are
> > only called from dw_pcie_ep_set_bar(), which already has the ep_func
> > NULL check?
> > 
> 
> Yes, that's correct. Alternatively, we can add the NULL check in
> dw_pcie_ep_clear_ib_maps() as well, making all the functions using
> ep_func self-contained and defensive, removing the dependency on whether
> the callers perform NULL checks. This makes the code more future proof,
> as new callers won't need to be aware of the NULL pointer possibility.

Sounds like a good idea to me.

Neither of these functions is in the hot path, so the performance of having
one less NULL check is not super critical.


Kind regards,
Niklas

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ