On Fri, 2019-06-14 at 10:57 +0100, Lorenzo Pieralisi wrote: > > Also using "probe_only" for _DSM #5 = 0 isn't a good idea, at least as > > implemented today in the rest of the kernel, probe_only also means we > > shouldn't assign what was left unassigned. However _DSM #5 allows this. > > I am not sure about this. PCI_PROBE_ONLY cannot stop an OS from > reassigning BARs that are clearly misconfigured, it does not make > any sense. PCI_PROBE_ONLY is a linux thing which, as implemented today, implies no assignment at all. I believe it originates as a merge of variants of the same thing, at least one of them being one I created for powerpc back in the days due to our proprietary hypervisor not letting you touch any of the PCI config space. If a device looks broken, disable it, don't use it, but don't reassign either. At least that the semantics we have today. And as such they don't match _DSM #5 = 0. > It can't stop an OS from writing those BARs anyway, > since they must be sized, why firmware would prevent an OS from > reassigning BARs that are programmed with values that can be > deemed 100% bogus ? Or put it differently, why must an OS preserve > those values willy-nilly ? Don't ask me ... IBM firmware :-) At least that was the idea back then. That said I suppose some platforms may also have set that flag to indicate that they aren't sure what other "ghost" things might be in the address space, ie Linux doesnt have a clear view of what's free to allocate devices to for example. > For me, PCI_PROBE_ONLY and _DSM == 0 on a host bridge must be considered > equivalent. Well, that's not what PCI_PROBE_ONLY is today in Linux. It might be what you would like it to be but it's not what it is :-) And I'd like to avoid making arm64 different than everybody else here because I want to consolidate things. Fundamentally, is what _DSM #5 == 0 does any different from our standard (not PROBE_ONLY) mode of operation on server platforms anyway ? Ie, we read what's there, and we leave it alone unless it's broken or unassigned ? This is precisely the definition of _DSM #5 == 0 no ? PROBE_ONLY is .. something else. > I agree with Bjorn on his reading of _DSM #5 and I think that > the original patch that claims on _DSM #5 == 0 is a good > starting point. The original patch is a good starting point, we agree. The only point of disagreement with Bjorn at this stage is what the "default" is in absence of _DSM #5. The spec says it should be the same as _DSM #5 == 0, but we know today it will introduce a much wider ranging change to arm64 to treat it that way. At the very least, changing the default should be a different patch. > I would like to make it a default even without > _DSM #5 == 0 so that claim and reassign on claim failure works > irrespective of _DSM #5, it is now or never, I think we can give > it a shot, with an incremental patch. We should. In fact, I was thinking about it on the way home tonight and was going to ask you and Ard to try this out and send me the debug level log output of anything that looks wrong on any platform: diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c index bb85e2f4603f..0af1f1b4e4d8 100644 --- a/arch/arm64/kernel/pci.c +++ b/arch/arm64/kernel/pci.c @@ -193,8 +193,8 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_pci_root *root) if (!bus) return NULL; - pci_bus_size_bridges(bus); - pci_bus_assign_resources(bus); + pci_bus_claim_resources(bus); + pci_assign_unassigned_root_bus_resources(bus); list_for_each_entry(child, &bus->children, node) pcie_bus_configure_settings(child); Cheers, Ben.
