[Patch v7 4/7] PCI/ACPI add interface to acpi_pci

[Patch v7 4/7] PCI/ACPI add interface to acpi_pci

[Jeremy Linton]

Hello,

I just took a quick look at the discussion about the DwordIO ACPI 
translation, and I have a couple comments (I don't have the whole thread 
available for some reason..).

First, the "secondary bus" referenced in the ACPI specification is the 
processor side, rather than the PCI device side. So the minimum/max base 
address are the MMIO mapped addresses. To get the actual IO address the 
translation must be subtracted. This means AFAIK that the qemu table is 
wrong in two ways for ARM64.

So that said, I will reference the juno DwordIo resource which I 
recently added to the EDK.

DWordIo ( // IO window
	ResourceProducer,
	MinFixed,
	MaxFixed,
	PosDecode,
	EntireRange,
	0x00000000, 	// Granularity
	0x5f800000, 	// Min Base Address
	0x5fffffff,	// Max Base Address
	0x5f800000, 	// Translate
	0x00800000  	// Length
)

Which after reading the specification, I realized should be setting the 
translation type to TypeTranslation. But that really doesn't matter for 
ARM64 because ARM64 doesn't have a processor accessible IO address 
range. AKA the TranslationType controls whether the processor is doing 
an IN/OUT/etc to access the range or actual MMIO read/writes. Since on 
ARM64 the only choice is MMIO, this field should be ignored, the only 
behavior difference should possibly be a warning if someone leaves it 
set to static.

Put another way, the setting of the TranslationType doesn't change the 
way the "translation" is computed, only how its accessed. AKA it doesn't 
affect the math..

Re: [Patch v7 4/7] PCI/ACPI add interface to acpi_pci

[Lorenzo Pieralisi]

Hi Jeremy,

On Fri, Nov 06, 2015 at 11:50:40AM -0600, Jeremy Linton wrote:
> Hello,
> 
> I just took a quick look at the discussion about the DwordIO ACPI
> translation, and I have a couple comments (I don't have the whole
> thread available for some reason..).
> 
> First, the "secondary bus" referenced in the ACPI specification is
> the processor side, rather than the PCI device side. So the
> minimum/max base address are the MMIO mapped addresses. To get the
> actual IO address the translation must be subtracted. This means
> AFAIK that the qemu table is wrong in two ways for ARM64.

As I said, this is certainly confusing. AFAICT, I read "secondary bus"
as secondary bus side of the PCI bridge, I agree there is a mismatch
in the ACPI specs between the WordIo specification and the actual
Resource descriptors, reading page 366, for an IO resource descriptor,
_TTP:

Bit [4] I/O to Memory Translation, _TTP
1 TypeTranslation: This resource, which is I/O on the secondary
		   side of the bridge, is memory on the primary side
		   of the bridge.
0 TypeStatic: This resource, which is I/O on the secondary side of the
	      bridge, is also I/O on the primary side of the bridge.

Then (19.6.33):

"TranslationType is an optional argument that specifies whether the
resource type on the secondary side of the bus is different (TypeTranslation)
from that on the primary side of the bus or the same
(TypeStatic). If TypeTranslation is specified, then the secondary side
of the bus is Memory. If TypeStatic is specified, then the secondary side
of the bus is I/O. If nothing is specified, then TypeStatic is assumed.
The 1-bit field DescriptorName._TTP is automatically created to refer to
this portion of the resource descriptor, where ‘1’ is TypeTranslation and ‘0’
is TypeStatic. See _TTP (page 365) for more information."

That reads the other way around :), which one is correct ?

> So that said, I will reference the juno DwordIo resource which I
> recently added to the EDK.
> 
> DWordIo ( // IO window
> 	ResourceProducer,
> 	MinFixed,
> 	MaxFixed,
> 	PosDecode,
> 	EntireRange,
> 	0x00000000, 	// Granularity
> 	0x5f800000, 	// Min Base Address
> 	0x5fffffff,	// Max Base Address
> 	0x5f800000, 	// Translate
> 	0x00800000  	// Length
> )
> 
> Which after reading the specification, I realized should be setting
> the translation type to TypeTranslation. But that really doesn't
> matter for ARM64 because ARM64 doesn't have a processor accessible
> IO address range. AKA the TranslationType controls whether the
> processor is doing an IN/OUT/etc to access the range or actual MMIO
> read/writes. Since on ARM64 the only choice is MMIO, this field
> should be ignored, the only behavior difference should possibly be a
> warning if someone leaves it set to static.
> 
> Put another way, the setting of the TranslationType doesn't change
> the way the "translation" is computed, only how its accessed. AKA it
> doesn't affect the math..

It seems to me that is down to interpretation, and that's not good at
all.

We need to raise the point within the ASWG and get this clarified.

Thanks,
Lorenzo
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Patch v7 4/7] PCI/ACPI add interface to acpi_pci

[Jeremy Linton]

On 11/06/2015 12:55 PM, Lorenzo Pieralisi wrote:
> Hi Jeremy,

> As I said, this is certainly confusing. AFAICT, I read "secondary bus"
> as secondary bus side of the PCI bridge, I agree there is a mismatch
> in the ACPI specs between the WordIo specification and the actual
> Resource descriptors, reading page 366, for an IO resource descriptor,
> _TTP:
>
> Bit [4] I/O to Memory Translation, _TTP
> 1 TypeTranslation: This resource, which is I/O on the secondary
> 		   side of the bridge, is memory on the primary side
> 		   of the bridge.
> 0 TypeStatic: This resource, which is I/O on the secondary side of the
> 	      bridge, is also I/O on the primary side of the bridge.
>
> Then (19.6.33):

> That reads the other way around :), which one is correct ?

	Well, I just read it for the two hundredth time today, and I'm back to 
my original position this morning before posting that BS above, that 
"secondary" refers to the device directed side, and primary is the 
processor directed side.

	For sure its made confusing by the link to the table 6-213, which says 
basically the opposite of what is said in table 6-214. The difference 
being that 6-214 is for IO regions (and what we are discussing, and the 
link in the 6.0 docs is wrong, probably because both tables were on the 
same page in 5.x).

	So your probably right, and the juno is incorrect. That said, I suspect 
we aren't the only ones, as i tested the juno against the RHEL release 
which has PCI/ACPI in it (running against all the ARM64 servers, AFAIK), 
and it was doing the right thing given the juno tables. <sigh> Which 
back when I was doing them, initially I created the resource with the 
base address=0, len=7FFFFF and the translate = 5f800000, but it didn't 
work.

	Can someone give us a dwordio example from an IA64 machine?

	But, after reading it over and over, I still don't see how the _TTP bit 
changes the way the translation is done... The TranslationDensity, yes, 
but that is set to dense for the two examples so far. Its really in my 
mind a question of whether the translate is added or subtracted, which 
comes down to which direction primary and secondary refer to.

Re: [Patch v7 4/7] PCI/ACPI add interface to acpi_pci

[Jiang Liu]

On 2015/11/7 4:44, Jeremy Linton wrote:
> On 11/06/2015 12:55 PM, Lorenzo Pieralisi wrote:
>> Hi Jeremy,
> 
>> As I said, this is certainly confusing. AFAICT, I read "secondary bus"
>> as secondary bus side of the PCI bridge, I agree there is a mismatch
>> in the ACPI specs between the WordIo specification and the actual
>> Resource descriptors, reading page 366, for an IO resource descriptor,
>> _TTP:
>>
>> Bit [4] I/O to Memory Translation, _TTP
>> 1 TypeTranslation: This resource, which is I/O on the secondary
>>            side of the bridge, is memory on the primary side
>>            of the bridge.
>> 0 TypeStatic: This resource, which is I/O on the secondary side of the
>>           bridge, is also I/O on the primary side of the bridge.
>>
>> Then (19.6.33):
> 
>> That reads the other way around :), which one is correct ?
> 
>     Well, I just read it for the two hundredth time today, and I'm back
> to my original position this morning before posting that BS above, that
> "secondary" refers to the device directed side, and primary is the
> processor directed side.
> 
>     For sure its made confusing by the link to the table 6-213, which
> says basically the opposite of what is said in table 6-214. The
> difference being that 6-214 is for IO regions (and what we are
> discussing, and the link in the 6.0 docs is wrong, probably because both
> tables were on the same page in 5.x).
> 
>     So your probably right, and the juno is incorrect. That said, I
> suspect we aren't the only ones, as i tested the juno against the RHEL
> release which has PCI/ACPI in it (running against all the ARM64 servers,
> AFAIK), and it was doing the right thing given the juno tables. <sigh>
> Which back when I was doing them, initially I created the resource with
> the base address=0, len=7FFFFF and the translate = 5f800000, but it
> didn't work.
> 
>     Can someone give us a dwordio example from an IA64 machine?
> 
>     But, after reading it over and over, I still don't see how the _TTP
> bit changes the way the translation is done... The TranslationDensity,
> yes, but that is set to dense for the two examples so far. Its really in
> my mind a question of whether the translate is added or subtracted,
> which comes down to which direction primary and secondary refer to.
Hi Jeremy and Lorenzo,
	The ACPI spec is really confusion. I think the issue comes from
the truth that TypeTranslation may be used for both IO port -> MMIO
translation and MMIO -> IO Port translation, but the explanation of
TypeTranslation only covers one direction. If we look at the Extended
Address Space Descriptor, it may give some hints about how to interpret
TypeTranslation.
	About the relationship between _TTP and translation_offset,
we need to first define the context. From hardware point of view,
_TTP doesn't affect the way, primary_side_address always equals to
secondary_side_address + translation_offset.
	From software(linux kernel) point of view, it becomes
complicated. There's only one ACPI DWordIO resource descriptor for
each IO port range, but Linux kernel needs two resource type of
resource objects to support IO port on IA64 and ARM64:
1) An IO port resource object, so PCI devices could allocate IO ports
   from this resource object.
2) An MMIO resource object, which is used to map IO port address into
   MMIO address space.
	So we need make a choice when parsing a DWordIO resource
descriptor with _TTP set as:
a) Translate it into an Linux IO port resource object
b) Translate it into an Linux MMIO resource object
	My recommended choice is a) becomes that will make thing
simpler. That is, the arch independent code will always translate an
ACPI DWordIO resource descriptor into an Linux IO port resource object
no matter _TTP is set or not. And arch specific code will create
an Linux MMIO resource object for b) iff _TTP is set. If we adopt
above design, translation_offset will be used as:
if (_TTP is set) {
   linux_mmio_res->start = acpi_desc->start + acpi_desc->translation_offset;
    linux_ioport_res->start = acpi_desc->start;
} else {
   linux_ioport_res->start = acpi_desc->start +
acpi_desc->translation_offset;
   //linux_mmio_res is not needed
}

To summarize, _TTP doesn't affect address translation from hardware
point of view; but it does matter from software point of view.
Thanks,
Gerry