CPU/PCI addressing in PciIoGetAttributes

CPU/PCI addressing in PciIoGetAttributes

[Bartosz Szczepanek]

Hi guys,

I have some doubt about PciIoGetAttributes behaviour on aarch64 platform
with non-uniform CPU:PCI address space mapping. I'll be grateful if you
verify my understanding.

According to UEFI specification, AddrRangeMin in QWORD Address Space
Descriptor returned by PciIoGetAttributes() should be a CPU address, at
least that's what I conclude from:
> Address Translation Offset. Offset to apply to the Starting address of a
> BAR *to convert it to a PCI address*. This value is zero unless the
> HostAddress and DeviceAddress for the BAR are different.
This mentions "Starting address", which I suppose refers to AddrRangeMin.
It needs to be converted to PCI address, so as-is it should be in CPU
address space.

On the other hand, the AddrRangeMin is assigned BaseAddress value obtained
directly from PciBar:
> Descriptor->AddrRangeMin = PciIoDevice->PciBar[BarIndex].BaseAddress;

PciBar is filled in PciParseBar() using original BAR content (thus
BaseAddress is not translated, PCI address). This way value that refers to
PCI space is returned from PciIoGetBarAttributes as CPU address.

Shouldn't there be translation of it somewhere on the way? Or is my
understanding flawed? Either way, please let me know.

Best regards,
Bartosz

Re: CPU/PCI addressing in PciIoGetAttributes

[Ni, Ruiyu]

Bartosz,

Based on your findings, the AddrRangeMin has to be a device address☺

As far as I can remember, this history is:
In the very beginning, Spec owner assumes the device address equals to CPU address.
The implementation of GetBarAttributes() directly returns the address read from the BAR.
But later we found device address doesn’t equal to CPU address, especially in ARM platform.
So in order to expose the offset of the two addresses, the AddressTranslationOffset field was used.
To avoid breaking the existing consumers, GetBarAttributes() remains to return the BAR address.

Anyone please correct me if I am wrong.

Thanks,
Ray

From: Bartosz Szczepanek [mailto:bsz@semihalf.com]
Sent: Monday, September 4, 2017 7:43 PM
To: edk2-devel@lists.01.org
Cc: Ni, Ruiyu <ruiyu.ni@intel.com>
Subject: [edk2] CPU/PCI addressing in PciIoGetAttributes

Hi guys,

I have some doubt about PciIoGetAttributes behaviour on aarch64 platform with non-uniform CPU:PCI address space mapping. I'll be grateful if you verify my understanding.

According to UEFI specification, AddrRangeMin in QWORD Address Space Descriptor returned by PciIoGetAttributes() should be a CPU address, at least that's what I conclude from:
> Address Translation Offset. Offset to apply to the Starting address of a
> BAR *to convert it to a PCI address*. This value is zero unless the
> HostAddress and DeviceAddress for the BAR are different.
This mentions "Starting address", which I suppose refers to AddrRangeMin. It needs to be converted to PCI address, so as-is it should be in CPU address space.

On the other hand, the AddrRangeMin is assigned BaseAddress value obtained directly from PciBar:
> Descriptor->AddrRangeMin = PciIoDevice->PciBar[BarIndex].BaseAddress;

PciBar is filled in PciParseBar() using original BAR content (thus BaseAddress is not translated, PCI address). This way value that refers to PCI space is returned from PciIoGetBarAttributes as CPU address.

Shouldn't there be translation of it somewhere on the way? Or is my understanding flawed? Either way, please let me know.

Best regards,
Bartosz

Re: CPU/PCI addressing in PciIoGetAttributes

[Bartosz Szczepanek]

Ray, thanks for your answer. This makes sense to me, but I still see some
inconsistency in regard to specification.

UEFI spec says:
> Address Range Minimum. Starting address of a BAR.
and then
> Address Translation Offset. Offset to apply to the Starting address of a
> BAR to convert it to a PCI address.

If it's needed to apply AddrTranslationOffset to AddrRangeMin to convert it
*to a PCI address*, how can it be a PCI (device) address already?

To me it seems like a discrepancy between UEFI specification and what
industry does. Please let me know what is your understanding of the
snippets above, if this is a mistake in spec it would be great to agree on
that and request appropriate correction.

Best regards,
Bartosz

PS – I wrote PciIoGetAttributes in last mail instead of
PciIoGetBarAttributes, sorry for the mistake.

2017-09-05 9:07 GMT+02:00 Ni, Ruiyu <ruiyu.ni@intel.com>:

> Bartosz,
>
>
>
> Based on your findings, the AddrRangeMin has to be a device addressJ
>
>
>
> As far as I can remember, this history is:
>
> In the very beginning, Spec owner assumes the device address equals to CPU
> address.
>
> The implementation of GetBarAttributes() directly returns the address read
> from the BAR.
>
> But later we found device address doesn’t equal to CPU address, especially
> in ARM platform.
>
> So in order to expose the offset of the two addresses, the
> AddressTranslationOffset field was used.
>
> To avoid breaking the existing consumers, GetBarAttributes() remains to
> return the BAR address.
>
>
>
> Anyone please correct me if I am wrong.
>
>
>
> Thanks,
>
> Ray
>
>
>
> *From:* Bartosz Szczepanek [mailto:bsz@semihalf.com]
> *Sent:* Monday, September 4, 2017 7:43 PM
> *To:* edk2-devel@lists.01.org
> *Cc:* Ni, Ruiyu <ruiyu.ni@intel.com>
> *Subject:* [edk2] CPU/PCI addressing in PciIoGetAttributes
>
>
>
> Hi guys,
>
>
>
> I have some doubt about PciIoGetAttributes behaviour on aarch64 platform
> with non-uniform CPU:PCI address space mapping. I'll be grateful if you
> verify my understanding.
>
>
>
> According to UEFI specification, AddrRangeMin in QWORD Address Space
> Descriptor returned by PciIoGetAttributes() should be a CPU address, at
> least that's what I conclude from:
>
> > Address Translation Offset. Offset to apply to the Starting address of a
>
> > BAR *to convert it to a PCI address*. This value is zero unless the
>
> > HostAddress and DeviceAddress for the BAR are different.
>
> This mentions "Starting address", which I suppose refers to AddrRangeMin.
> It needs to be converted to PCI address, so as-is it should be in CPU
> address space.
>
>
>
> On the other hand, the AddrRangeMin is assigned BaseAddress value obtained
> directly from PciBar:
>
> > Descriptor->AddrRangeMin = PciIoDevice->PciBar[BarIndex].BaseAddress;
>
>
>
> PciBar is filled in PciParseBar() using original BAR content (thus
> BaseAddress is not translated, PCI address). This way value that refers to
> PCI space is returned from PciIoGetBarAttributes as CPU address.
>
>
>
> Shouldn't there be translation of it somewhere on the way? Or is my
> understanding flawed? Either way, please let me know.
>
>
>
> Best regards,
>
> Bartosz
>

Re: CPU/PCI addressing in PciIoGetAttributes

[Ard Biesheuvel]

On 5 September 2017 at 08:07, Ni, Ruiyu <ruiyu.ni@intel.com> wrote:
> Bartosz,
>
> Based on your findings, the AddrRangeMin has to be a device address☺
>
> As far as I can remember, this history is:
> In the very beginning, Spec owner assumes the device address equals to CPU address.
> The implementation of GetBarAttributes() directly returns the address read from the BAR.
> But later we found device address doesn’t equal to CPU address, especially in ARM platform.
> So in order to expose the offset of the two addresses, the AddressTranslationOffset field was used.
> To avoid breaking the existing consumers, GetBarAttributes() remains to return the BAR address.
>
> Anyone please correct me if I am wrong.
>

The spec is quite clear: the address descriptor should *not* contain
the raw BAR value, but the CPU's translated view. So I think we should
fix the implementation to comply with that.

The risk for breakage should be low, given that the translation is
zero in 99% of the cases anyway, especially on traditional UEFI (PC)
platforms.

Re: CPU/PCI addressing in PciIoGetAttributes

[Bartosz Szczepanek]

> The spec is quite clear: the address descriptor should *not* contain
> the raw BAR value, but the CPU's translated view. So I think we should
> fix the implementation to comply with that.

To complicate it a little further, here's what ACPI specification says
about QWORD Address Space Descriptor:
> Address range minimum [...]. For bridges that translate addresses, this is the address space on
> the secondary side of the bridge.

I don't think "secondary" means CPU side here... perhaps it is
something to be discussed in ASWG/USWG?