On Wed, Jul 20, 2016 at 5:46 PM, Rafael J. Wysocki <rjw@xxxxxxxxxxxxx> wrote:
> On Wednesday, July 20, 2016 02:35:43 PM Bjorn Helgaas wrote:
>> On Tue, Jun 28, 2016 at 09:41:19PM -0700, Aaron Durbin wrote:
>> > On Fri, Jun 24, 2016 at 12:34 PM, Aaron Durbin <adurbin@xxxxxxxxxx> wrote:
>> > > On Tue, Jun 14, 2016 at 12:04 PM, Aaron Durbin <adurbin@xxxxxxxxxx> wrote:
>> > >> On Wed, May 18, 2016 at 3:32 PM, Aaron Durbin <adurbin@xxxxxxxxxx> wrote:
>> > >>> On Wed, May 18, 2016 at 4:25 PM, Rafael J. Wysocki <rafael@xxxxxxxxxx> wrote:
>> > >>>> On Wed, May 18, 2016 at 5:54 PM, Aaron Durbin <adurbin@xxxxxxxxxx> wrote:
>> > >>>>> Hi,
>> > >>>>>
>> > >>>>> We're currently running into a problem of resource conflicts with a
>> > >>>>> PCI device and ACPI devices.
>> > >>>>>
>> > >>>>> [ 0.243534] pci 0000:00:0d.0: can't claim BAR 0 [mem
>> > >>>>> 0xd0000000-0xd0ffffff 64bit]: address conflict with INT3452:03 [mem
>> > >>>>> 0xd0c00000-0xd0c03fff]
>> > >>>>>
>> > >>>>> The PCI BAR covers a large amount mmio resources, however, there are
>> > >>>>> ACPI devices with their own HID (for probing) which uses resources
>> > >>>>> that are a subset of the PCI BAR.
>> > >>>>>
>> > >>>>> Short of re-structuring the linux driver is there anything that can be
>> > >>>>> done with ASL to properly have the ACPI device use a sub-resource of
>> > >>>>> the PCI device during the ACPI/PCI probing?
>> > >>>>
>> > >>>> Do you have an ACPI device object corresponding to the PCI device?
>> > >>>
>> > >>> I've been debugging this by proxy, and I did request that test. The
>> > >>> following is the overall structure:
>> > >>>
>> > >>> scope (\_SB.PCI0) {
>> > >>>
>> > >>> Device (P2S)
>> > >>> {
>> > >>> Name (_ADR, 0x000D0000)
>> > >>> Device (GPO0)
>> > >>> {
>> > >>> Name (_ADR, 0)
>> > >>> Name (_HID, "INT3452")
>> > >>> Name (_CID, "INT3452")
>> > >>> }
>> > >>> }
>> > >>> }
>> > >>>
>> > >>> There are _STA methods in both Devices. The GP0 device has a _CRS
>> > >>> method which just returns a ResourceTemplate which is filled in with
>> > >>> static values. The PCI bar is at a fixed address from the firmware
>> > >>> which allows the fixed calculations. However there is no specific
>> > >>> reference to the P2S device's resources proper -- only the parent
>> > >>> child relationship within the ASL. I'm not sure how to directly say "I
>> > >>> want this sub-region of this other device's resource for my resource."
>> > >>> That seems like the right thing, but it's not clear if that's implied
>> > >>> by hierarchy of the devices.
>> > >>>
>> > >>> Lastly, if it helps, the kernel being used is based on v4.4 (no core
>> > >>> patches on top).
>> > >>>
>> > >>
>> > >> Hi Rafael,
>> > >>
>> > >> I haven't tried a newer kernel yet, but are you of the opinion that
>> > >> having the Devices as parent-child within the ASL should work? I'm
>> > >> wondering if there's already a patch in newer kernels that doesn't
>> > >> report the conflict and works as expected once there are child Devices
>> > >> under the P2S device.
>> > >>
>> > >
>> > > I've been looking at this more closely. A child ACPI device under a
>> > > ACPI PCI device doesn't change the resource conflict even when a _CRS
>> > > method is added to the ACPI PCI device. Below is my sleuthing which
>> > > is probably not a surprise to anyone here, but please correct me where
>> > > I am wrong.
>> > >
>> > > acpi_init() and pci_subsys_init() are both subsys_initcalls during
>> > > boot up. I'm not sure if the ordering is dumb luck or not, but
>> > > acpi_init() is called prior to pci_subsys_init(). The conflict error
>> > > is spit out from pcibios_resource_survey() by way of pci_subsys_init()
>> > > subsys_initcall. However, the PCI device scanning is kicked off prior
>> > > to this through acpi_scan_init() by way of acpi_init()
>> > > subsys_initcall. The conflict error occurs because there's already
>> > > the child ACPI device in the resource tree. I'm not sure when/where
>> > > those ACPI devices' resources are added, but clearly they are sitting
>> > > in there since the conflict was found.
>>
>> I think the acpi_init()/pci_subsys_init() ordering is correct. The
>> ACPI namespace is primary. A PCI hierarchy originates at a PCI host
>> bridge in the ACPI namespace, so we should enumerate the ACPI
>> namespace first, and when we find a PCI host bridge, we should
>> enumerate the PCI devices below it.
>>
>> That said, I think it is correct mostly by accident and it would be
>> nice if it were more explicit.
>
> No, it isn't by accident.
>
> The enumeration of PCI devices under a PCI host bridge discovered via ACPI
> starts in acpi_pci_root_add() which quite explicitly is only called after
> enumerating the ACPI namespace entirely.
>
> acpi_bus_scan() has two passes now, one is to call acpi_bus_check_add() for
> all namespace objects and the other is the acpi_bus_attach() pass where
> all things like acpi_pci_root_add() are called.
>
>> > > Somewhere along the way a PCI device from a scan is linked with the
>> > > ACPI device for that same PCI device in sysfs. This is with me
>> > > putting a _HID and _CID in the PCI ACPI device.
>> > > # readlink -f /sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/INT5A92:00/physical_node
>> > > /sys/devices/pci0000:00/0000:00:0d.0
>> > > # readlink -f /sys/devices/pci0000\:00/0000\:00\:0d.0/firmware_node/
>> > > /sys/devices/LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/INT5A92:00
>> > >
>> > > So the hierarchy is known eventually, but it's clearly not honored
>> > > when adding resources. The current ACPI support doesn't handle
>> > > PciBarTarget which initially sounds (from ACPI spec) like the way to
>> > > go for referencing a resource in a PCI device from an ACPI device.
>>
>> Yes, I think PCIBARTarget looks like the right way to do this. It
>> doesn't *seem* like it'd be that hard to implement; have you looked
>> into that at all?
>>
>> Without PCIBARTarget, the AML contains fixed register addresses, so it
>> will break if Linux reassigns the BAR.
>
> Right.
>
>> > > So
>> > > that's out of the question currently, but maybe someone has a patch
>> > > for that? I don't think reordering the acpi_init() and
>> > > pci_subsys_init() would do anything different except change which
>> > > device discovers the conflict.
>> > >
>> > > Is there a way to honor the ACPI device hierarchy during resource
>> > > addition for the PCI devices? The conflict is found because of the
>> > > presence of a child device claiming resources through _CRS.
>> > > Alternatively, is there a good way to defer the probing of an ACPI
>> > > device until one knows PCI resources have been added?
>> > >
>> > > Any insights would be very helpful. Thank you.
>> >
>> > I stumbled upon the hierarchy connection. That's all handled with the
>> > platform_notify() end of things when device_add() is done on the pci
>> > device. I was thinking we could take advantage of this when adding
>> > resources, but a struct resource has no struct device. It's just a
>> > name description for the resource at hand. However, platform devices
>> > are added when the ACPI tree is parsed along with adding the resources
>> > associated with them (PciBarTarget would be helpful here) so those
>> > resources are sitting in the resource tree when PCI BARs are added.
>> >
>> > The following suggestion is sort of hacky, but it's the best I could
>> > come up with provided the currently supported infrastructure. In
>> > pci_claim_resource() do request_resource_conflict() as before. If it
>> > fails do the following: 1. check if the device has an ACPI companion.
>> > 2. For any children hanging off the ACPI companion device. check if
>> > that device's name matches the conflict resource's name. 3. If so,
>> > insert_resource_conflict() to place the BAR within the tree itself.
>>
>> I think the best solution would be to implement PCIBARTarget, but if
>> that's impossible, this seems like a plausible workaround.
>>
>> I don't know if the conflict would necessarily have to be with the
>> ACPI companion itself. It seems like you could have some hierarchy of
>> ACPI devices where the leaf conflicts with a PCI BAR. Maybe if a
>> resource of *any* ACPI device below a PCI host bridge conflicts with a
>> PCI BAR, we should insert the PCI BAR as a parent?
>>
>> And since moving that BAR would break the AML, we should probably mark
>> the BAR as IORESOURCE_PCI_FIXED.
>
> Isn't the problem a probe ordering issue, though?
No. Not just ordering. There's no parent-child relationship taken into
account when PCI devices are added to the resource tree. Deferring
adding an ACPI device's resources would likely fail similarly because
one doesn't take into account the proper parent. One lives in PCI land
-- the other in ACPI land.
Currently, the issue is manifesting that the ACPI device's resources
are added into the resource tree. PCI devices the follow trying to add
their own resources. Conflicts ensue without taking into account the
proper hierarchy. To make matters worse, struct resource doesn't have
a struct device -- only a name. That means there's no way of knowing
who owns what resource. All that information is unavailable. That's
why the semi-hacky proposal was to see if there is an ACPI companion
device and see if it has children. If it's children's names match the
conflicting resource it's a good bet that the PCI device's resources
should be placed as the parent in the resource tree.
>
> Do we assign BARs for endpoints during enumeration or only when we find a
> matching driver?
BARs look to be assigned during enumeration. There's where conflicts
are found and resources reassigned in the current failing case.
>
> Thanks,
> Rafael
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
