On Wed, Dec 09, 2015 at 03:55:11PM +0000, Lorenzo Pieralisi wrote:
On Wed, Dec 09, 2015 at 03:31:41PM +0530, Jayachandran C. wrote:
[trimmed the cc list a bit] On Mon, Dec 07, 2015 at 02:31:17PM -0600, Bjorn Helgaas wrote:
On Tue, Oct 27, 2015 at 05:38:31PM +0100, Tomasz Nowicki wrote:
[...]
Tomasz Nowicki (10): x86, pci: Reorder logic of pci_mmconfig_insert() function x86, pci, acpi: Move arch-agnostic MMCONFIG (aka ECAM) and ACPI code out of arch/x86/ directory pci, acpi, mcfg: Provide generic implementation of MCFG code initialization. x86, pci: mmconfig_{32,64}.c code refactoring - remove code duplication. x86, pci, ecam: mmconfig_64.c becomes default implementation for ECAM driver. pci, acpi, mcfg: Provide default RAW ACPI PCI config space accessors. pci, acpi, ecam: Add flag to indicate whether ECAM region was hot added or not. x86, pci: Use previously added ECAM hot_added flag to remove ECAM regions.
[...]
Waiting for an updated version after all the discussion.
Hi Bjorn,
After looking thru the code, I don't think moving the mmconfig code out of x86 and using it in ARM64 is a good idea.
The code maintains a list of mapping of ECAM regions which can modified at run time, and ends doing this sequence for doing a config access:
rcu_read_lock(); addr = pci_dev_base(seg, bus, devfn); list_for_each_entry_rcu(cfg, &pci_mmcfg_list, list) ...find mapping...do pci operation rcu_read_unlock();
A lot of the complexity in mmconfig_*.c is to maintain and validate the pci_mmcfg_list which seems to be unecessary.
That list is there to manage hotplug bridges, what makes you think it is not necessary ? Jiang (in CC) can certainly comment on that and how that list handling can be updated/simplified, if possible.
Looking thru the code, I think moving the MCFG code to common can be done in a simpler way. I have posted a new patchset for this. (And I don't see how the hotplug case needs the list, but that is not relevant now)
The better way would be to keep the mapping of ECAM region in the PCI host bridge sysdata like pci-host-generic and directly access using map_bus/read/write.
I see that early raw_pci_read/raw_pci_write may need to be supported. I am not sure if this is needed in ARM64 - and if it is, we can handle this by taking an early mapping for ECAM regions until the PCI host bridges are setup, and dropping the mapping at that point.
I do not think it is needed (and if it is it can be added later, it should not block this series), this was already debated in previous threads (and in the ASWG, where basically nobody could provide a reason why those raw accessors in ACPICA are meant to exist, they are there for AML to access config space, why that has to be happen before PCI busses are enumerated is still a mystery to me).
The posted patchset adds raw_pci_read/raw_pci_write as well.
JC.