linux-kernel - Re: [PATCH v4 2/3] PCI: dwc: ep: Add per-PF BAR and inbound ATU mapping support

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: <aXtrW7viGZfMNZur@ryzen>
Date: Thu, 29 Jan 2026 15:14:51 +0100
From: Niklas Cassel <cassel@...nel.org>
To: Aksh Garg <a-garg7@...com>, Koichiro Den <den@...inux.co.jp>
Cc: 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 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.


>  
>  	/* Tear down the BAR Match Mode mapping, if any. */
> -	if (ep->bar_to_atu[bar]) {
> -		atu_index = ep->bar_to_atu[bar] - 1;
> +	if (ep_func->bar_to_atu[bar]) {
> +		atu_index = ep_func->bar_to_atu[bar] - 1;
>  		dw_pcie_disable_atu(pci, PCIE_ATU_REGION_DIR_IB, atu_index);
>  		clear_bit(atu_index, ep->ib_window_map);
> -		ep->bar_to_atu[bar] = 0;
> +		ep_func->bar_to_atu[bar] = 0;

Not related to your patch,

Koichiro (he is on To:),

don't you think that it would be clearer if we had a:
		return;

here...

I mean, a BAR can either have a BAR match mode mapping or a subrange mapping,
but not both... So continuing executing code beyond this point seems pointless,
possibly even confusing.


>  	}
>  
>  	/* Tear down all Address Match Mode mappings, if any. */
> -	indexes = ep->ib_atu_indexes[bar];
> -	num = ep->num_ib_atu_indexes[bar];
> -	ep->ib_atu_indexes[bar] = NULL;
> -	ep->num_ib_atu_indexes[bar] = 0;
> +	indexes = ep_func->ib_atu_indexes[bar];
> +	num = ep_func->num_ib_atu_indexes[bar];
> +	ep_func->ib_atu_indexes[bar] = NULL;
> +	ep_func->num_ib_atu_indexes[bar] = 0;
>  	if (!indexes)
>  		return;

Sure, I see that this code will do a return here...

So there will be no harm done, but, having a simple return above
would make it extra clear to the reader that is is always one or
the other... you cannot have both.

If you agree, perhaps you could send a one liner patch?


>  	for (i = 0; i < num; i++) {