Messages in this thread | | | Subject | Re: [PATCH 05/11] x86, pci, acpi: Move arch-agnostic MMCONFIG (aka ECAM) and ACPI code out of arch/x86/ directory | From | Tomasz Nowicki <> | Date | Mon, 14 Sep 2015 13:34:02 +0200 |
| |
On 14.09.2015 11:37, Lorenzo Pieralisi wrote: > On Fri, Sep 11, 2015 at 01:35:36PM +0100, Tomasz Nowicki wrote: >> On 11.09.2015 13:20, Lorenzo Pieralisi wrote: > > [...] > >>>>> With that in place using raw_pci_write/read or the generic accessors >>>>> becomes almost identical, with code requiring the pci_bus to be >>>>> created using the generic accessors and ACPICA using the raw version. >>>>> >>>>> I might be missing something, so apologies if that's the case. >>>>> >>>> >>>> Actually, I think you showed me the right direction :) Here are some >>>> conclusions/comments/concerns. Please correct me if I am wrong: >>>> >>>> 1. We need raw_pci_write/read accessors (based on ECAM) for ARM64 too >>>> but only up to the point where buses are enumerated. From that point on, >>>> we should reuse generic accessors from access.c file, right? >>> >>> Well, I still have not figured out whether on arm64 the raw accessors >>> required by ACPICA make sense. >>> >>> So either arm64 relies on the generic MCFG based raw read and writes >>> or we define the global raw read and writes as empty (ie x86 overrides >>> them anyway). >>> >>> I will get back to you on this. >>> >>>> 2. For ARM64 ACPI PCI, we can use generic accessors right away, .map_bus >>>> would call common code part (pci_dev_base()). The only thing that worry >>>> me is fact that MCFG regions are RCU list so it needs rcu_read_lock() >>>> for the .map_bus (mcfg lookup) *and* read/write operation. >>> >>> Do you mean the address look-up and the mmio operation should be carried >>> out atomically right ? >> Yes. > > We can wrap the calls pci_generic_read/write() within a function and > add rcu_read_lock()/unlock() around them, eg: > > int pci_generic_config_read_rcu() > { > rcu_read_lock(); > pci_generic_config_read(...); > rcu_read_unlock(); > } It looks good to me, thanks for suggestion.
> > Honestly it seems the RCU API is needed just because config space > can be also accessed by raw_ accessors in ACPICA code, that's the only > reason I see to protect the config structs against config space > removal (basically config entries are removed only when the host > bridge is released if I read the code correctly, and the only way > this can happen concurrently is having ACPICA code reusing the > same config space but accessing it with no pci_bus struct attached > to it, by just using the (segment, bus, dev, fn) tuple). > Right.
Side note: MCFG region can be removed from the pci_mmcfg_list list only if it has been "hot added" there. Which means that PCI host bridge specified configuration base address (_CBA) different than those from MCFG static table e.g.:
DSDT.asl: Device (PCI0) { Name (_HID, EISAID ("PNP0A03")) [...] Name (_CBA, 0xB0000000) [...] }
But pci_mmcfg_list elements coming from static MCFG table cannot be removed, hence they are living there for ever.
Thanks, Tomasz
| |