Re: [Patch v7 4/7] PCI/ACPI: Add interface acpi_pci_root_create()

From: Jiang Liu
Date: Tue Nov 10 2015 - 00:51:36 EST

This is a multi-part message in MIME format.On 2015/11/10 4:09, Arnd Bergmann wrote:
> On Monday 09 November 2015 17:10:43 Lorenzo Pieralisi wrote:
>> On Mon, Nov 09, 2015 at 03:07:38PM +0100, Tomasz Nowicki wrote:
>>> On 06.11.2015 14:22, Jiang Liu wrote:
>>>> On 2015/11/6 20:40, Tomasz Nowicki wrote:
>>>>> On 06.11.2015 12:46, Jiang Liu wrote:
>>>>>> On 2015/11/6 18:37, Tomasz Nowicki wrote:
>>>>>>> On 06.11.2015 09:52, Jiang Liu wrote:
>>>>>>> Sure, ARM64 (0-16M IO space) QEMU example:
>>>>>>> DWordIO (ResourceProducer, MinFixed, MaxFixed, PosDecode, EntireRange,
>>>>>>> 0x00000000, // Granularity
>>>>>>> 0x00000000, // Range Minimum
>>>>>>> 0x0000FFFF, // Range Maximum
>>>>>>> 0x3EFF0000, // Translation Offset
>>>>>>> 0x00010000, // Length
>>>>>>> ,, , TypeStatic)
>>>>>> The above DWordIO resource descriptor doesn't confirm to the ACPI spec.
>>>>>> According to my understanding, ARM/ARM64 has no concept of IO port
>>>>>> address space, so the PCI host bridge will map IO port on PCI side
>>>>>> onto MMIO on host side. In other words, PCI host bridge on ARM64
>>>>>> implement a IO Port->MMIO translation instead of a IO Port->IO Port
>>>>>> translation. If that's true, it should use 'TypeTranslation' instead
>>>>>> of 'TypeStatic'. And kernel ACPI resource parsing interface doesn't
>>>>>> support 'TypeTranslation' yet, so we need to find a solution for it.
>>>>> I think you are right, we need TypeTranslation flag for ARM64 DWordIO
>>>>> descriptors and an extra kernel patch to support it.
>>>> How about the attached to patch to support TypeTranslation?
>>>> It only passes compilation:)
>>> Based on the further discussion, your draft patch looks good to me.
>>> Lorenzo, do you agree?
>> No, because I still do not understand the difference between ia64 and
>> arm64 (they both drive IO ports cycles through MMIO so the resource
>> descriptors content must be the same or better they must mean the same
>> thing). On top of that, this is something that was heavily debated for DT:
>> and I would like to get Arnd and Bjorn opinion on this because we
>> should not "interpret" ACPI specifications, we should understand
>> what they are supposed to describe and write kernel code accordingly.
>> In particular, I would like to understand, for an eg DWordIO descriptor,
>> what Range Minimum, Range Maximum and Translation Offset represent,
>> they can't mean different things depending on the SW parsing them,
>> this totally defeats the purpose.
> I have no clue about what those mean in ACPI though.
> Generally speaking, each PCI domain is expected to have a (normally 64KB)
> range of CPU addresses that gets translated into PCI I/O space the same
> way that config space and memory space are handled.
> This is true for almost every architecture except for x86, which uses
> different CPU instructions for I/O space compared to the other spaces.
>> By the way, ia64 ioremaps the translation_offset (ie new_space()), so
>> basically that's the CPU physical address at which the PCI host bridge
>> map the IO space transactions), I do not think ia64 is any different from
>> arm64 in this respect, if it is please provide an HW description here from
>> the PCI bus perspective here (also an example of ia64 ACPI PCI host bridge
>> tables would help).
> The main difference between ia64 and a lot of the other architectures (e.g.
> sparc is different again) is that ia64 defines a logical address range
> in terms of having a small number for each I/O space followed by the
> offset within that space as a 'port number' and uses a mapping function
> that is defined as
> static inline void *__ia64_mk_io_addr (unsigned long port)
> {
> struct io_space *space = &io_space[IO_SPACE_NR(port)];
> return (space->mmio_base | IO_SPACE_PORT(port););
> }
> static inline unsigned int inl(unsigned long port)
> {
> return *__ia64_mk_io_addr(port);
> }
> Most architectures allow only one I/O port range and put it at a fixed
> virtual address so that inl() simply becomes
> static inline u32 inl(unsigned long addr)
> {
> return readl(PCI_IOBASE + addr);
> }
> which noticeably reduces code size.
> On some architectures (powerpc, arm, arm64), we then get the same simplified
> definition with a fixed virtual address, and use pci_ioremap_io() or
> something like that to to map a physical address range into this virtual
> address window at the correct io_offset;
Hi all,
Thanks for explanation, I found a way to make the ACPI resource
parsing interface arch neutral, it should help to address Lorenzo's
concern. Please refer to the attached patch. (It's still RFC, not tested