linux-kernel - Re: [PATCH v8 4/6] PCI: Allow extend_bridge_window() to shrink resource if necessary

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: <SL2P216MB018711E3699EE682FD4437E8806B0@SL2P216MB0187.KORP216.PROD.OUTLOOK.COM>
Date:   Wed, 23 Oct 2019 09:16:10 +0000
From:   Nicholas Johnson <nicholas.johnson-opensource@...look.com.au>
To:     "mika.westerberg@...ux.intel.com" <mika.westerberg@...ux.intel.com>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
        "bhelgaas@...gle.com" <bhelgaas@...gle.com>,
        "corbet@....net" <corbet@....net>,
        "benh@...nel.crashing.org" <benh@...nel.crashing.org>,
        "logang@...tatee.com" <logang@...tatee.com>
Subject: Re: [PATCH v8 4/6] PCI: Allow extend_bridge_window() to shrink
 resource if necessary

On Tue, Oct 08, 2019 at 03:09:07PM +0300, mika.westerberg@...ux.intel.com wrote:
> On Fri, Jul 26, 2019 at 12:54:22PM +0000, Nicholas Johnson wrote:
> > Remove checks for resource size in extend_bridge_window(). This is
> > necessary to allow the pci_bus_distribute_available_resources() to
> > function when the kernel parameter pci=hpmemsize=nn[KMG] is used to
> > allocate resources. Because the kernel parameter sets the size of all
> > hotplug bridges to be the same, there are problems when nested hotplug
> > bridges are encountered. Fitting a downstream hotplug bridge with size X
> > and normal bridges with size Y into parent hotplug bridge with size X is
> > impossible, and hence the downstream hotplug bridge needs to shrink to
> > fit into its parent.
> 
> Maybe you could show the topology here which needs shrinking.
> 
> > Add check for if bridge is extended or shrunken and adjust pci_dbg to
> > reflect this.
> > 
> > Reset the resource if its new size is zero (if we have run out of a
> > bridge window resource). If it is set to zero size and left, it can
> > cause significant problems when it comes to enabling devices.
> 
> Same comment here about explaining the "significant problems".
I have in the past, but because the problems are very hard to describe succinctly, it just turns into a 
nightmare. I can try to do it again.

> > 
> > Signed-off-by: Nicholas Johnson <nicholas.johnson-opensource@...look.com.au>
> > ---
> >  drivers/pci/setup-bus.c | 16 +++++++++++-----
> >  1 file changed, 11 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> > index a072781ab..7e1dc892a 100644
> > --- a/drivers/pci/setup-bus.c
> > +++ b/drivers/pci/setup-bus.c
> > @@ -1823,13 +1823,19 @@ static void extend_bridge_window(struct pci_dev *bridge, struct resource *res,
> 
> Since it is also shrinking now maybe name it adjust_bridge_window() instead?
I am happy to do this.

If we can drop the pci_dbg() calls, then I might be able to drop this 
function entirely. During the development of this patch, that is exactly 
what I did. How important are the pci_dbg calls to you?

Another option is to simply print something with pci_dbg that simply 
says the bridge size has been set to maximum possible while still 
fitting in parent. That will remove the need for logic to detect if it 
shrunk or extended.