On 11/6/2015 10:44 AM, Jiang Liu wrote: > On 2015/11/6 23:32, Jiang Liu wrote: >> On 2015/11/6 22:45, Lorenzo Pieralisi wrote: >>> On Fri, Nov 06, 2015 at 09:22:46PM +0800, 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:) >>> >>> Eh, hopefully there are not any ACPI tables out there with that bit >>> set that work _today_ and would not work with the patch attached :) >>> >>> My question is still there: do we want to handle the same problem >>> as ia64 has in a different manner ? Certainly we won't be able >>> to update ia64 platforms ACPI tables, so we would end up with >>> two platforms handling IO resources in different ways unless I am >>> missing something here. >> There are some difference between IA64 and ARM64. >> On IA64, it supports 16M IO address space per PCI domain and 256 PCI >> domains at max. So the system IO address space is 16M * 256 = 4G. >> So it does two level translations to support IO port >> 1) translate PCI bus local IO port address into system global IO port >> address by adding acpi_des->translation_offset. >> 2) translate the 4G system IO port address space into MMIO address. >> IA64 has reserved a 4G space for IO port mapping. This translation >> is done by arch specific method. >> In other word, IA64 needs two level translation, but ACPI only provides >> on (trans_type, trans_offset) pair for encoding, so it's used for step 1). >> >> For ARM64, I think currently it only needs step 2). >> >>> >>> BTW, why would we add offset to res->start only if TypeTranslation is >>> clear ? Is not that something we would do just to make things "work" ? >>> That flag has no bearing on the offset, only on the resource type AFAIK. >> It's not a hack, but a way to interpret ACPI spec:) >> >> With current linux resource management framework, we need to allocate >> both MMIO and IO port address space range for an ACPI resource of type >> 'TypeTranslation'. And struct resource could be either IO port or MMIO, >> not both. So the choice is to keep the resource as IO port, and let >> arch code to build the special MMIO mapping for it. Otherwise it will >> break too many things if we convert the resource as MMIO. >> >> That said, we need to add translation_offset to convert bus local >> IO port address into system global IO port address if it's type of >> TypeStatic, because ioresource_ioport uses system global IO port >> address. >> >> For an ACPI resource of type TypeTranslation, system global IO port >> address equals bus local IO port address, and the translation_offset >> is used to translate IO port address into MMIO address, so we shouldn't >> add translation_offset to the IO port resource descriptor. > One note for the TypeTranslation case, the arch code needs to reset > resource_win->offset to zero after setting up the MMIO map. Sample > code as below: > va = ioremap(resource_win->offset + res->start, resource_size(res)); > resource_win->offset = 0; > > Otherwise it will break pcibios_resource_to_bus() etc. > >> >> Thanks, >> Gerry >> >>> >>> This without taking into account ARM64 systems shipping with ACPI >>> tables that does not set the TypeTranslation at present. >>> >>> On top of that, I noticed that core ACPI code handles Sparse >>> Translation (ie _TRS), that should be considered meaningful only if _TTP >>> is set (and that's not checked). >> Yes, that's a flaw:( >> >>> >>> Thoughts ? >>> >>> Thanks, >>> Lorenzo >>> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >> the body of a message to majordomo@xxxxxxxxxxxxxxx >> More majordomo info at http://vger.kernel.org/majordomo-info.html >> Please read the FAQ at http://www.tux.org/lkml/ >> > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > Tomasz, Lorenzo; I have an ARMv8 platform with IO address support where I can test this implementation. Let me know if there is any patch that I can try. Sinan -- Sinan Kaya Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
