linux-kernel - Re: [PATCH 2/3] PCI: Use resource_set

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]

Date: Thu, 13 Jun 2024 14:41:43 +0100
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
CC: Bjorn Helgaas <bhelgaas@...gle.com>, <linux-pci@...r.kernel.org>, "Lorenzo
 Pieralisi" <lpieralisi@...nel.org>, Rob Herring <robh@...nel.org>, Krzysztof
 Wilczyński <kw@...ux.com>, Thierry Reding
	<thierry.reding@...il.com>, Jonathan Hunter <jonathanh@...dia.com>, "Robert
 Richter" <rric@...nel.org>, <linux-tegra@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 2/3] PCI: Use resource_set_{range,size}() helpers

On Wed, 12 Jun 2024 11:56:28 +0300
Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com> wrote:

> Convert open-coded resource size calculations to use
> resource_set_{range,size}() helpers.
> 
> While at it, use SZ_* for size parameter where appropriate which makes
> the intent of code more obvious.
> 
> Also, cast sizes to resource_size_t, not u64.
> 
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>

LGTM - one comment inline.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@...wei.com>



> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 909e6a7c3cc3..004405edf290 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c

> @@ -1948,15 +1943,15 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,

>  
>  		res = &dev->resource[PCI_BRIDGE_PREF_MEM_WINDOW];
>  		align = pci_resource_alignment(dev, res);
> -		mmio_pref.end = align ? mmio_pref.start +
> -					ALIGN_DOWN(mmio_pref_per_b, align) - 1
> -				      : mmio_pref.start + mmio_pref_per_b - 1;
> +		resource_set_size(&mmio_pref,
> +				  align ? ALIGN_DOWN(mmio_pref_per_b, align)
> +					: mmio_pref_per_b);

I wonder.  Maybe it's worth defining an ALIGN_DOWN_IF_NON_ZERO() as locally
at least this pattern is annoying common and we can't just change
pci_resource_alignment() to return 1 in those cases because we want a
clean way to check it's not set in a lot of places.

Bikeshedding that name might take longer than it's worth though.




>  		mmio_pref.start -= resource_size(res);
>  
>  		pci_bus_distribute_available_resources(b, add_list, io, mmio,