Re: [PATCH v7 20/26] PNP: Don't reserve BARs for PCI when enabled movable BARs

From: Sergei Miroshnichenko <s.miroshnichenko@yadro.com>
To: "helgaas@kernel.org" <helgaas@kernel.org>
Cc: "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	"linux@yadro.com" <linux@yadro.com>,
	"rafael.j.wysocki@intel.com" <rafael.j.wysocki@intel.com>,
	"sr@denx.de" <sr@denx.de>
Subject: Re: [PATCH v7 20/26] PNP: Don't reserve BARs for PCI when enabled movable BARs
Date: Fri, 31 Jan 2020 15:48:48 +0000	[thread overview]
Message-ID: <d744d743a8a2512ebcd31818cdc76400cfd0086e.camel@yadro.com> (raw)
In-Reply-To: <20200130211453.GA127287@google.com>

Hello Bjorn,

On Thu, 2020-01-30 at 15:14 -0600, Bjorn Helgaas wrote:
> On Wed, Jan 29, 2020 at 06:29:31PM +0300, Sergei Miroshnichenko
> wrote:
> > When the Movable BARs feature is supported, the PCI subsystem is
> > able to
> > distribute existing BARs and allocate the new ones itself, without
> > need to
> > reserve gaps by BIOS.
> > 
> > CC: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Signed-off-by: Sergei Miroshnichenko <s.miroshnichenko@yadro.com>
> > ---
> >  drivers/pnp/system.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/pnp/system.c b/drivers/pnp/system.c
> > index 6950503741eb..16cd260a609d 100644
> > --- a/drivers/pnp/system.c
> > +++ b/drivers/pnp/system.c
> > @@ -12,6 +12,7 @@
> >  #include <linux/device.h>
> >  #include <linux/init.h>
> >  #include <linux/slab.h>
> > +#include <linux/pci.h>
> >  #include <linux/kernel.h>
> >  #include <linux/ioport.h>
> >  
> > @@ -58,6 +59,11 @@ static void reserve_resources_of_dev(struct
> > pnp_dev *dev)
> >  	struct resource *res;
> >  	int i;
> >  
> > +#ifdef CONFIG_PCI
> > +	if (pci_can_move_bars)
> > +		return;
> > +#endif
> 
> I don't understand this.  The reason this function exists is so we
> keep track of the resources consumed by PNP devices and we can keep
> from assigning those resources to other things like PCI devices.
> 
> Admittedly we currently only do this for PNP0C01 and PNP0C02 devices,
> but we really should do it for all PNP devices.
> 
> Why does Movable BARs mean that we no longer need this information?
> The whole point is that this information is needed *during* PCI
> resource allocation, so I don't understand the idea that "because the
> PCI subsystem is able to distribute existing BARs and allocate the
> new
> ones itself", we don't need to know about PNP resources to avoid.
> 

Oh. I've made this patch in assumption that non-PCI PNP devices should
not reside in the PCI address space, and PCI PNP devices behave like
usual PCI devices - with BARs handled by the common PCI subsystem.

Do I understand correctly after digging a bit into drivers/pnp, that
some of these resources are some kind of "invisible" BARs, which are
used by drivers, but the PCI subsystem can't "see" them, so that's why
the PNP reserves them?

In this case I need just to discard this patch and to modify the
pci_bus_release_root_bridge_resources() added in patch 06/26 - remove
the pci_bus_for_each_resource(root_bus, r, i) block there, which
releases such non-BAR resourses. I've just checked that it works, so
the next version - v8 - of this patchset will be a bit lighter. Thank
you for pointing that out!

Best regards,
Serge

> >  	for (i = 0; (res = pnp_get_resource(dev, IORESOURCE_IO, i));
> > i++) {
> >  		if (res->flags & IORESOURCE_DISABLED)
> >  			continue;
> > -- 
> > 2.24.1
> > 