Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1526463

RE: [PATCH] PCI: Add information about describing PCI in ACPI

Show all headers | View raw

Hi Bjorn

> -----Original Message-----
> From: Bjorn Helgaas [mailto:helgaas@kernel.org]
> Sent: 18 November 2016 17:54
> To: Gabriele Paoloni
> Cc: Bjorn Helgaas; linux-pci@vger.kernel.org; linux-
> acpi@vger.kernel.org; linux-kernel@vger.kernel.org; linux-arm-
> kernel@lists.infradead.org; linaro-acpi@lists.linaro.org
> Subject: Re: [PATCH] PCI: Add information about describing PCI in ACPI
> 
> On Fri, Nov 18, 2016 at 05:17:34PM +0000, Gabriele Paoloni wrote:
> > > -----Original Message-----
> > > From: linux-kernel-owner@vger.kernel.org [mailto:linux-kernel-
> > > owner@vger.kernel.org] On Behalf Of Bjorn Helgaas
> > > Sent: 17 November 2016 18:00
> 
> > > +Static tables like MCFG, HPET, ECDT, etc., are *not* mechanisms
> for
> > > +reserving address space!  The static tables are for things the OS
> > > +needs to know early in boot, before it can parse the ACPI
> namespace.
> > > +If a new table is defined, an old OS needs to operate correctly
> even
> > > +though it ignores the table.  _CRS allows that because it is
> generic
> > > +and understood by the old OS; a static table does not.
> >
> > Right so if my understanding is correct you are saying that resources
> > described in the MCFG table should also be declared in PNP0C02
> devices
> > so that the PNP driver can reserve these resources.
> 
> Yes.
> 
> > On the other side the PCI Root bridge driver should not reserve such
> > resources.
> >
> > Well if my understanding is correct I think we have a problem here:
> > http://lxr.free-electrons.com/source/drivers/pci/ecam.c#L74
> >
> > As you can see pci_ecam_create() will conflict with the pnp driver
> > as it will try to reserve the resources from the MCFG table...
> >
> > Maybe we need to rework pci_ecam_create() ?
> 
> I think it's OK as it is.
> 
> The pnp/system.c driver does try to reserve PNP0C02 resources, and it
> marks them as "not busy".  That way they appear in /proc/iomem and
> won't be allocated for anything else, but they can still be requested
> by drivers, e.g., pci/ecam.c, which will mark them "busy".
> 
> This is analogous to what the PCI core does in pci_claim_resource().
> This is really a function of the ACPI/PNP *core*, which should reserve
> all _CRS resources for all devices (not just PNP0C02 devices).  But
> it's done by pnp/system.c, and only for PNP0C02, because there's a
> bunch of historical baggage there.
> 
> You'll also notice that in this case, things are out of order:
> logically the pnp/system.c reservation should happen first, but in
> fact the pci/ecam.c request happens *before* the pnp/system.c one.
> That means the pnp/system.c one might fail and complain "[mem ...]
> could not be reserved".

Correct me if I am wrong...

So currently we are relying on the fact that pci_ecam_create() is called
before the pnp driver.
If the pnp driver came first we would end up in pci_ecam_create() failing
here:
http://lxr.free-electrons.com/source/drivers/pci/ecam.c#L76

I am not sure but it seems to me like a bit weak condition to rely on...
what about removing the error condition in pci_ecam_create() and logging
just a dev_info()?

Thanks

Gab


> 
> Bjorn

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH] PCI: Add information about describing PCI in ACPI Bjorn Helgaas <bhelgaas@google.com> - 2016-11-17 19:00 +0100
  RE: [PATCH] PCI: Add information about describing PCI in ACPI Gabriele Paoloni <gabriele.paoloni@huawei.com> - 2016-11-18 18:20 +0100
    Re: [PATCH] PCI: Add information about describing PCI in ACPI Bjorn Helgaas <helgaas@kernel.org> - 2016-11-18 19:00 +0100
      RE: [PATCH] PCI: Add information about describing PCI in ACPI Gabriele Paoloni <gabriele.paoloni@huawei.com> - 2016-11-21 10:00 +0100
        Re: [PATCH] PCI: Add information about describing PCI in ACPI Bjorn Helgaas <helgaas@kernel.org> - 2016-11-21 17:50 +0100
          RE: [PATCH] PCI: Add information about describing PCI in ACPI Gabriele Paoloni <gabriele.paoloni@huawei.com> - 2016-11-21 18:30 +0100
            Re: [PATCH] PCI: Add information about describing PCI in ACPI Bjorn Helgaas <helgaas@kernel.org> - 2016-11-21 21:20 +0100
              RE: [PATCH] PCI: Add information about describing PCI in ACPI Gabriele Paoloni <gabriele.paoloni@huawei.com> - 2016-11-22 14:20 +0100
  Re: [PATCH] PCI: Add information about describing PCI in ACPI "Rafael J. Wysocki" <rafael@kernel.org> - 2016-11-19 00:10 +0100
    Re: [PATCH] PCI: Add information about describing PCI in ACPI Bjorn Helgaas <helgaas@kernel.org> - 2016-11-21 15:00 +0100
  Re: [PATCH] PCI: Add information about describing PCI in ACPI Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2016-11-22 11:20 +0100
    Re: [PATCH] PCI: Add information about describing PCI in ACPI Bjorn Helgaas <helgaas@kernel.org> - 2016-11-23 02:10 +0100
      Re: [PATCH] PCI: Add information about describing PCI in ACPI Ard Biesheuvel <ard.biesheuvel@linaro.org> - 2016-11-23 08:30 +0100
        Re: [PATCH] PCI: Add information about describing PCI in ACPI Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> - 2016-11-23 13:30 +0100
          Re: [Linaro-acpi] [PATCH] PCI: Add information about describing PCI  in ACPI Duc Dang <dhdang@apm.com> - 2016-11-23 22:00 +0100
        Re: [PATCH] PCI: Add information about describing PCI in ACPI Bjorn Helgaas <helgaas@kernel.org> - 2016-11-23 16:10 +0100
  RE: [PATCH] PCI: Add information about describing PCI in ACPI "Zheng, Lv" <lv.zheng@intel.com> - 2016-11-23 04:30 +0100

csiph-web