Re: [PATCH] Enable using device address when programming BARs

[Laszlo Ersek <lersek@redhat.com>]

Hi Roman,

On 05/14/18 19:35, Ard Biesheuvel wrote:
> On 14 May 2018 at 19:28, Roman Bacik <roman.bacik@broadcom.com> wrote:
>> Ard,
>>
>> Thank you very much for your comment.
>>
>>> -----Original Message-----
>>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
>>> Sent: Sunday, May 13, 2018 3:25 AM
>>> To: Roman Bacik
>>> Cc: edk2-devel@lists.01.org; Ruiyu Ni; Vladimir Olovyannikov
>>> Subject: Re: [edk2] [PATCH] Enable using device address when
>>> programming BARs
>>>
>>> On 9 May 2018 at 22:17, Roman Bacik <roman.bacik@broadcom.com>
>>> wrote:
>>>> I will upload v2 with the corrected subject - add package name
>>>> MdeModulePkg/Bus .
>>>>
>>>
>>> I don't think this is the correct approach. Please use the address
>>> translation
>>> support that has been added recently to PciHostBridgeDxe and
>>> PciHostBridgeLib.
>>>
>>
>> Would you like to see this change:
>>
>>   Address = Base + Node->Offset;
>> + if (UseDeviceAddress)
>> +  Address = TO_DEVICE_ADDRESS(Address, -Base);
>>
>> Instead of:
>>
>> -  Address = Base + Node->Offset;
>> +  Address = UseDeviceAddress? Node->Offset: Base + Node->Offset;
>>
>
> No.
>
> Programming BARs should always involve device addresses, never CPU
> addresses. If you wire up the MMIO translation support correctly, the
> existing code will already do what you want.

To clarify a bit, please look at the following commits:

  1  5bb1866e5383 MdeModulePkg/PciHostBridgeLib.h: add address Translation
  2  74d0a339b832 MdeModulePkg/PciHostBridgeDxe: Add support for address translation
  3  c03860d05233 MdeModulePkg/PciBus: convert host address to device address
  4  dc080d3b61e5 MdeModulePkg/PciBus: return CPU address for GetBarAttributes

in particular, as Ard's first response suggests, the new field
introduced in commit 5bb1866e5383. The PciHostBridgeLib instance that
your platform plugs into MdeModulePkg/PciHostBridgeDxe should populate
this field correctly. Then PciHostBridgeDxe and PciBusDxe will do the
right thing for you.

Thanks
Laszlo