[lkml]   [2015]   [Sep]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [PATCH 05/11] x86, pci, acpi: Move arch-agnostic MMCONFIG (aka ECAM) and ACPI code out of arch/x86/ directory
Hi Lorenzo,

On 09/14/2015 04:55 PM, Tomasz Nowicki wrote:
> On 11.09.2015 13:20, Lorenzo Pieralisi wrote:
>> On Wed, Sep 09, 2015 at 02:47:55PM +0100, Tomasz Nowicki wrote:
>> [...]
>>>> I think (but I am happy to be corrected) that the map_bus() hook
>>>> (ie that's why struct pci_bus is required in eg
>>>> pci_generic_config_write)
>>>> is there to ensure that when the generic accessors are called
>>>> a) we have a valid bus b) the host controllers implementing it
>>>> has been initialized.
>>>> I had another look and I noticed you are trying to solve multiple
>>>> things at once:
>>>> 1) ACPICA seems to need PCI config space on bus 0 to be working
>>>> before PCI enumerates (ie before we have a root bus), we need to
>>>> countercheck on that, but you can't use the generic PCI accessors
>>>> for that reasons (ie root bus might not be available, you do not
>>>> have a pci_bus struct)
>>>> 2) the raw_pci_read/write require _generic_ mmio back-ends, since AMD
>>>> can't cope with standard x86 read/write{b,w,l}
>>>> Overall, it seems to me that we can avoid code duplication by
>>>> shuffling your code a bit.
>>>> You could modify the generic accessors in drivers/pci/access.c to
>>>> use your mmio back-end instead of using plain read/write{b,w,l}
>>>> functions (we should check if RobH is ok with that there can be
>>>> reasons that prevent this from happening). This would solve the
>>>> AMD mmio issue.
>>>> By factoring out the code that actually carries out the reads
>>>> and writes in the accessors basically you decouple the functions
>>>> requiring the struct pci_bus from the ones that does not require it
>>>> (ie raw_pci_{read/write}.
>>>> The generic MMIO layer belongs in the drivers/pci/access.c file, it has
>>>> nothing to do with ECAM.
>>>> The mmcfg interface should probably live in pci-acpi.c, I do not think
>>>> you need an extra file in there but that's a detail.
>>>> Basically the generic accessors would become something like eg:
>>>> int pci_generic_config_write(struct pci_bus *bus, unsigned int devfn,
>>>> int where, int size, u32 val)
>>>> {
>>>> void __iomem *addr;
>>>> addr = bus->ops->map_bus(bus, devfn, where);
>>>> if (!addr)
>>>> pci_mmio_write(size, addr + where, value);
>>>> }
>>>> 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).
> My concerns/ideas related to raw accessors for ARM64, please correct me
> at any point.
> ACPI spec - chapter: 19.5.96 OperationRegion (Declare Operation Region)
> defines PCI_Config as one of region types. Every time ASL opcode
> operates on corresponding PCI config space region, ASL interpreter is
> dispatching address space to our raw accessors, please see
> acpi_ex_pci_config_space_handler, acpi_ev_pci_config_region_setup calls.
> What is more important, such operations may happen after (yes after) bus
> enumeration, but always raw accessors are called at the end with the
> {segment, bus, dev, fn} tuple.
> Giving above, here are some ideas:
> 1. We force somehow vendors to avoid operations on PCI config regions in
> ASL code. PCI config region definitions still fall into Hardware Reduced
> profile, so new ACPICA special subset for ARM64 is need. Then raw ACPI
> accessors can be empty (and overridden by x86).
> 2. We provide raw accessors which translate {segment, bus, dev, fn}
> tuple to Linux generic accessors (this can be considered only if PCI
> config accesses happened after bus enumeration for HR profile, thus
> tuple to bus structure map is possible).
> 4. We rely on the generic MCFG based raw read and writes.

I will appreciate your opinion on above ideas.


 \ /
  Last update: 2015-09-25 18:21    [W:0.123 / U:1.408 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site