Re: [edk2] [RFC] MdeModulePkg/PciHostBridgeDxe: Add support for address translation

2018-02-05 Thread Guo Heyi 
On Mon, Feb 05, 2018 at 10:11:30AM +0100, Ard Biesheuvel wrote:
> On 5 February 2018 at 09:06, Ni, Ruiyu <ruiyu...@intel.com> wrote:
> > On 2/3/2018 2:00 PM, Ni, Ruiyu wrote:
> >>
> >>
> >>
> >>> -Original Message-
> >>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> >>> Sent: Friday, February 2, 2018 4:22 PM
> >>> To: Ni, Ruiyu <ruiyu...@intel.com>
> >>> Cc: Guo Heyi <heyi@linaro.org>,Dong Wei <dong@arm.com>; Dong,
> >>> Eric <eric.d...@intel.com>; edk2-devel@lists.01.org; linaro-uefi  >>> u...@lists.linaro.org>; Kinney, Michael D <michael.d.kin...@intel.com>;
> >>> Zeng,
> >>> Star <star.z...@intel.com>
> >>> Subject: Re: [edk2] [RFC] MdeModulePkg/PciHostBridgeDxe: Add support for
> >>> address translation
> >>>
> >>> On 2 February 2018 at 00:34, Ni, Ruiyu <ruiyu...@intel.com> wrote:
> >>>>
> >>>>
> >>>>
> >>>>> -Original Message-
> >>>>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> >>>>> Sent: Friday, February 2, 2018 1:23 AM
> >>>>> To: Ni, Ruiyu <ruiyu...@intel.com>
> >>>>> Cc: Guo Heyi <heyi@linaro.org>,Dong Wei <dong@arm.com>;
> >>>
> >>> Dong,
> >>>>>
> >>>>> Eric <eric.d...@intel.com>; edk2-devel@lists.01.org; linaro-uefi
> >>>>> ; Kinney, Michael D
> >>>>> <michael.d.kin...@intel.com>; Zeng, Star <star.z...@intel.com>
> >>>>> Subject: Re: [edk2] [RFC] MdeModulePkg/PciHostBridgeDxe: Add support
> >>>>> for address translation
> >>>>>
> >>>>> On 1 February 2018 at 05:03, Ni, Ruiyu <ruiyu...@intel.com> wrote:
> >>>>>>
> >>>>>> On 1/29/2018 4:50 PM, Guo Heyi wrote:
> >>>>>>>
> >>>>>>>
> >>>>>>> Sorry for the late; I caught cold and didn't work for several days
> >>>>>>> last week :( Please see my comments below:
> >>>>>>>
> >>>>>>>
> >>>>>>> On Mon, Jan 22, 2018 at 11:36:14AM +0800, Ni, Ruiyu wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On 1/18/2018 9:26 AM, Guo Heyi wrote:
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> On Wed, Jan 17, 2018 at 02:08:06PM +, Ard Biesheuvel wrote:
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> On 15 January 2018 at 14:46, Heyi Guo <heyi@linaro.org> wrote:
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> This is the draft patch for the discussion posted in
> >>>>>>>>>>> edk2-devel mailing list:
> >>>>>>>>>>> https://lists.01.org/pipermail/edk2-devel/2017-December/019289
> >>>>>>>>>>> .ht
> >>>>>>>>>>> ml
> >>>>>>>>>>>
> >>>>>>>>>>> As discussed in the mailing list, we'd like to add support for
> >>>>>>>>>>> PCI address translation which is necessary for some non-x86
> >>>>>>>>>>> platforms. I also want to minimize the changes to the generic
> >>>>>>>>>>> host bridge driver and platform PciHostBridgeLib
> >>>>>>>>>>> implemetations, so additional two interfaces are added to
> >>>>>>>>>>> expose translation information of the platform. To be generic,
> >>>>>>>>>>> I add translation for each type of IO or memory resources.
> >>>>>>>>>>>
> >>>>>>>>>>> The patch is still a RFC, so I only passed the build for
> >>>>>>>>>>> qemu64 and the function has not been tested yet.
> >>>>>>>>>>>
> >>>>>>>>>>> Please let me know your comments about it.
> >>>>>>>>>>>
> >>>>>>>>>>> Thanks.
> >>>>>>>>

Re: [edk2] [RFC] MdeModulePkg/PciHostBridgeDxe: Add support for address translation

2018-02-05 Thread Ard Biesheuvel 
On 5 February 2018 at 09:06, Ni, Ruiyu <ruiyu...@intel.com> wrote:
> On 2/3/2018 2:00 PM, Ni, Ruiyu wrote:
>>
>>
>>
>>> -Original Message-
>>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>>> Sent: Friday, February 2, 2018 4:22 PM
>>> To: Ni, Ruiyu <ruiyu...@intel.com>
>>> Cc: Guo Heyi <heyi@linaro.org>,Dong Wei <dong@arm.com>; Dong,
>>> Eric <eric.d...@intel.com>; edk2-devel@lists.01.org; linaro-uefi >> u...@lists.linaro.org>; Kinney, Michael D <michael.d.kin...@intel.com>;
>>> Zeng,
>>> Star <star.z...@intel.com>
>>> Subject: Re: [edk2] [RFC] MdeModulePkg/PciHostBridgeDxe: Add support for
>>> address translation
>>>
>>> On 2 February 2018 at 00:34, Ni, Ruiyu <ruiyu...@intel.com> wrote:
>>>>
>>>>
>>>>
>>>>> -Original Message-
>>>>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>>>>> Sent: Friday, February 2, 2018 1:23 AM
>>>>> To: Ni, Ruiyu <ruiyu...@intel.com>
>>>>> Cc: Guo Heyi <heyi@linaro.org>,Dong Wei <dong@arm.com>;
>>>
>>> Dong,
>>>>>
>>>>> Eric <eric.d...@intel.com>; edk2-devel@lists.01.org; linaro-uefi
>>>>> ; Kinney, Michael D
>>>>> <michael.d.kin...@intel.com>; Zeng, Star <star.z...@intel.com>
>>>>> Subject: Re: [edk2] [RFC] MdeModulePkg/PciHostBridgeDxe: Add support
>>>>> for address translation
>>>>>
>>>>> On 1 February 2018 at 05:03, Ni, Ruiyu <ruiyu...@intel.com> wrote:
>>>>>>
>>>>>> On 1/29/2018 4:50 PM, Guo Heyi wrote:
>>>>>>>
>>>>>>>
>>>>>>> Sorry for the late; I caught cold and didn't work for several days
>>>>>>> last week :( Please see my comments below:
>>>>>>>
>>>>>>>
>>>>>>> On Mon, Jan 22, 2018 at 11:36:14AM +0800, Ni, Ruiyu wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 1/18/2018 9:26 AM, Guo Heyi wrote:
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On Wed, Jan 17, 2018 at 02:08:06PM +, Ard Biesheuvel wrote:
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 15 January 2018 at 14:46, Heyi Guo <heyi@linaro.org> wrote:
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> This is the draft patch for the discussion posted in
>>>>>>>>>>> edk2-devel mailing list:
>>>>>>>>>>> https://lists.01.org/pipermail/edk2-devel/2017-December/019289
>>>>>>>>>>> .ht
>>>>>>>>>>> ml
>>>>>>>>>>>
>>>>>>>>>>> As discussed in the mailing list, we'd like to add support for
>>>>>>>>>>> PCI address translation which is necessary for some non-x86
>>>>>>>>>>> platforms. I also want to minimize the changes to the generic
>>>>>>>>>>> host bridge driver and platform PciHostBridgeLib
>>>>>>>>>>> implemetations, so additional two interfaces are added to
>>>>>>>>>>> expose translation information of the platform. To be generic,
>>>>>>>>>>> I add translation for each type of IO or memory resources.
>>>>>>>>>>>
>>>>>>>>>>> The patch is still a RFC, so I only passed the build for
>>>>>>>>>>> qemu64 and the function has not been tested yet.
>>>>>>>>>>>
>>>>>>>>>>> Please let me know your comments about it.
>>>>>>>>>>>
>>>>>>>>>>> Thanks.
>>>>>>>>>>>
>>>>>>>>>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>>>>>>>>>> Signed-off-by: Heyi Guo <heyi@linaro.org>
>>>>>>>>>>> Cc: Ruiyu Ni <ruiyu...@intel.com>
>>>>>>>>>>> Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
>>>>>>>>>>

Re: [edk2] [RFC] MdeModulePkg/PciHostBridgeDxe: Add support for address translation

2018-02-05 Thread Ni, Ruiyu 

On 2/3/2018 2:00 PM, Ni, Ruiyu wrote:




-Original Message-
From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
Sent: Friday, February 2, 2018 4:22 PM
To: Ni, Ruiyu <ruiyu...@intel.com>
Cc: Guo Heyi <heyi@linaro.org>,Dong Wei <dong@arm.com>; Dong,
Eric <eric.d...@intel.com>; edk2-devel@lists.01.org; linaro-uefi ; Kinney, Michael D <michael.d.kin...@intel.com>; Zeng,
Star <star.z...@intel.com>
Subject: Re: [edk2] [RFC] MdeModulePkg/PciHostBridgeDxe: Add support for
address translation

On 2 February 2018 at 00:34, Ni, Ruiyu <ruiyu...@intel.com> wrote:




-Original Message-
From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
Sent: Friday, February 2, 2018 1:23 AM
To: Ni, Ruiyu <ruiyu...@intel.com>
Cc: Guo Heyi <heyi@linaro.org>,Dong Wei <dong@arm.com>;

Dong,

Eric <eric.d...@intel.com>; edk2-devel@lists.01.org; linaro-uefi
; Kinney, Michael D
<michael.d.kin...@intel.com>; Zeng, Star <star.z...@intel.com>
Subject: Re: [edk2] [RFC] MdeModulePkg/PciHostBridgeDxe: Add support
for address translation

On 1 February 2018 at 05:03, Ni, Ruiyu <ruiyu...@intel.com> wrote:

On 1/29/2018 4:50 PM, Guo Heyi wrote:


Sorry for the late; I caught cold and didn't work for several days
last week :( Please see my comments below:


On Mon, Jan 22, 2018 at 11:36:14AM +0800, Ni, Ruiyu wrote:


On 1/18/2018 9:26 AM, Guo Heyi wrote:


On Wed, Jan 17, 2018 at 02:08:06PM +, Ard Biesheuvel wrote:


On 15 January 2018 at 14:46, Heyi Guo <heyi@linaro.org> wrote:


This is the draft patch for the discussion posted in
edk2-devel mailing list:
https://lists.01.org/pipermail/edk2-devel/2017-December/019289
.ht
ml

As discussed in the mailing list, we'd like to add support for
PCI address translation which is necessary for some non-x86
platforms. I also want to minimize the changes to the generic
host bridge driver and platform PciHostBridgeLib
implemetations, so additional two interfaces are added to
expose translation information of the platform. To be generic,
I add translation for each type of IO or memory resources.

The patch is still a RFC, so I only passed the build for
qemu64 and the function has not been tested yet.

Please let me know your comments about it.

Thanks.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Heyi Guo <heyi@linaro.org>
Cc: Ruiyu Ni <ruiyu...@intel.com>
Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
Cc: Star Zeng <star.z...@intel.com>
Cc: Eric Dong <eric.d...@intel.com>
---
   .../FdtPciHostBridgeLib/FdtPciHostBridgeLib.c  |  19 
   .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c   |  53 ---
   .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h   |   8 +-
   .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 101
++---
   MdeModulePkg/Include/Library/PciHostBridgeLib.h|  36



   5 files changed, 192 insertions(+), 25 deletions(-)

diff --git
a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
index 5b9c887..0c8371a 100644
---
a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
+++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.
+++ c
@@ -360,6 +360,16 @@ PciHostBridgeGetRootBridges (
 return 
   }

+PCI_ROOT_BRIDGE_TRANSLATION * EFIAPI
+PciHostBridgeGetTranslations (
+  UINTN *Count
+  )
+{
+  *Count = 0;
+  return NULL;
+}
+
   /**
 Free the root bridge instances array returned from
 PciHostBridgeGetRootBridges().
@@ -377,6 +387,15 @@ PciHostBridgeFreeRootBridges (
 ASSERT (Count == 1);
   }

+VOID
+EFIAPI
+PciHostBridgeFreeTranslations (
+  PCI_ROOT_BRIDGE_TRANSLATION *Translations,
+  UINTN   Count
+  )
+{
+}
+
   /**
 Inform the platform that the resource conflict happens.

diff --git
asame/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
index 1494848..835e411 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
@@ -360,18 +360,38 @@ InitializePciHostBridge (
 PCI_HOST_BRIDGE_INSTANCE*HostBridge;
 PCI_ROOT_BRIDGE_INSTANCE*RootBridge;
 PCI_ROOT_BRIDGE *RootBridges;
+  PCI_ROOT_BRIDGE_TRANSLATION *Translations;
 UINTN   RootBridgeCount;
+  UINTN   TranslationCount;
 UINTN   Index;
 PCI_ROOT_BRIDGE_APERTURE*MemApertures[4];



Wouldn't it be much better to add a 'translation' member to
PCI_ROOT_BRIDGE_APERTURE? That way, existing code just default
to a translation of 0, and all the handling of the separate
array can be dropped.


Actually my first idea was the same, but when I looked at the
implementation of PciHostBridgeLib of Ovmf, I found it a little
tedious to change the exist

Re: [edk2] [RFC] MdeModulePkg/PciHostBridgeDxe: Add support for address translation

2018-02-02 Thread Ni, Ruiyu 


> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Friday, February 2, 2018 4:22 PM
> To: Ni, Ruiyu <ruiyu...@intel.com>
> Cc: Guo Heyi <heyi@linaro.org>,Dong Wei <dong@arm.com>; Dong,
> Eric <eric.d...@intel.com>; edk2-devel@lists.01.org; linaro-uefi  u...@lists.linaro.org>; Kinney, Michael D <michael.d.kin...@intel.com>; Zeng,
> Star <star.z...@intel.com>
> Subject: Re: [edk2] [RFC] MdeModulePkg/PciHostBridgeDxe: Add support for
> address translation
> 
> On 2 February 2018 at 00:34, Ni, Ruiyu <ruiyu...@intel.com> wrote:
> >
> >
> >> -Original Message-
> >> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> >> Sent: Friday, February 2, 2018 1:23 AM
> >> To: Ni, Ruiyu <ruiyu...@intel.com>
> >> Cc: Guo Heyi <heyi@linaro.org>,Dong Wei <dong@arm.com>;
> Dong,
> >> Eric <eric.d...@intel.com>; edk2-devel@lists.01.org; linaro-uefi
> >> ; Kinney, Michael D
> >> <michael.d.kin...@intel.com>; Zeng, Star <star.z...@intel.com>
> >> Subject: Re: [edk2] [RFC] MdeModulePkg/PciHostBridgeDxe: Add support
> >> for address translation
> >>
> >> On 1 February 2018 at 05:03, Ni, Ruiyu <ruiyu...@intel.com> wrote:
> >> > On 1/29/2018 4:50 PM, Guo Heyi wrote:
> >> >>
> >> >> Sorry for the late; I caught cold and didn't work for several days
> >> >> last week :( Please see my comments below:
> >> >>
> >> >>
> >> >> On Mon, Jan 22, 2018 at 11:36:14AM +0800, Ni, Ruiyu wrote:
> >> >>>
> >> >>> On 1/18/2018 9:26 AM, Guo Heyi wrote:
> >> >>>>
> >> >>>> On Wed, Jan 17, 2018 at 02:08:06PM +, Ard Biesheuvel wrote:
> >> >>>>>
> >> >>>>> On 15 January 2018 at 14:46, Heyi Guo <heyi@linaro.org> wrote:
> >> >>>>>>
> >> >>>>>> This is the draft patch for the discussion posted in
> >> >>>>>> edk2-devel mailing list:
> >> >>>>>> https://lists.01.org/pipermail/edk2-devel/2017-December/019289
> >> >>>>>> .ht
> >> >>>>>> ml
> >> >>>>>>
> >> >>>>>> As discussed in the mailing list, we'd like to add support for
> >> >>>>>> PCI address translation which is necessary for some non-x86
> >> >>>>>> platforms. I also want to minimize the changes to the generic
> >> >>>>>> host bridge driver and platform PciHostBridgeLib
> >> >>>>>> implemetations, so additional two interfaces are added to
> >> >>>>>> expose translation information of the platform. To be generic,
> >> >>>>>> I add translation for each type of IO or memory resources.
> >> >>>>>>
> >> >>>>>> The patch is still a RFC, so I only passed the build for
> >> >>>>>> qemu64 and the function has not been tested yet.
> >> >>>>>>
> >> >>>>>> Please let me know your comments about it.
> >> >>>>>>
> >> >>>>>> Thanks.
> >> >>>>>>
> >> >>>>>> Contributed-under: TianoCore Contribution Agreement 1.1
> >> >>>>>> Signed-off-by: Heyi Guo <heyi@linaro.org>
> >> >>>>>> Cc: Ruiyu Ni <ruiyu...@intel.com>
> >> >>>>>> Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
> >> >>>>>> Cc: Star Zeng <star.z...@intel.com>
> >> >>>>>> Cc: Eric Dong <eric.d...@intel.com>
> >> >>>>>> ---
> >> >>>>>>   .../FdtPciHostBridgeLib/FdtPciHostBridgeLib.c  |  19 
> >> >>>>>>   .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c   |  53 
> >> >>>>>> ---
> >> >>>>>>   .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h   |   8 +-
> >> >>>>>>   .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 101
> >> >>>>>> ++---
> >> >>>>>>   MdeModulePkg/Include/Library/PciHostBridgeLib.h|  36
> 
> >> >>>>>>   5 files changed, 192 insertions(

Re: [edk2] [RFC] MdeModulePkg/PciHostBridgeDxe: Add support for address translation

2018-02-02 Thread Ard Biesheuvel 
On 2 February 2018 at 00:34, Ni, Ruiyu <ruiyu...@intel.com> wrote:
>
>
>> -Original Message-
>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>> Sent: Friday, February 2, 2018 1:23 AM
>> To: Ni, Ruiyu <ruiyu...@intel.com>
>> Cc: Guo Heyi <heyi@linaro.org>,Dong Wei <dong@arm.com>; Dong,
>> Eric <eric.d...@intel.com>; edk2-devel@lists.01.org; linaro-uefi > u...@lists.linaro.org>; Kinney, Michael D <michael.d.kin...@intel.com>; Zeng,
>> Star <star.z...@intel.com>
>> Subject: Re: [edk2] [RFC] MdeModulePkg/PciHostBridgeDxe: Add support for
>> address translation
>>
>> On 1 February 2018 at 05:03, Ni, Ruiyu <ruiyu...@intel.com> wrote:
>> > On 1/29/2018 4:50 PM, Guo Heyi wrote:
>> >>
>> >> Sorry for the late; I caught cold and didn't work for several days
>> >> last week :( Please see my comments below:
>> >>
>> >>
>> >> On Mon, Jan 22, 2018 at 11:36:14AM +0800, Ni, Ruiyu wrote:
>> >>>
>> >>> On 1/18/2018 9:26 AM, Guo Heyi wrote:
>> >>>>
>> >>>> On Wed, Jan 17, 2018 at 02:08:06PM +, Ard Biesheuvel wrote:
>> >>>>>
>> >>>>> On 15 January 2018 at 14:46, Heyi Guo <heyi@linaro.org> wrote:
>> >>>>>>
>> >>>>>> This is the draft patch for the discussion posted in edk2-devel
>> >>>>>> mailing list:
>> >>>>>> https://lists.01.org/pipermail/edk2-devel/2017-December/019289.ht
>> >>>>>> ml
>> >>>>>>
>> >>>>>> As discussed in the mailing list, we'd like to add support for
>> >>>>>> PCI address translation which is necessary for some non-x86
>> >>>>>> platforms. I also want to minimize the changes to the generic
>> >>>>>> host bridge driver and platform PciHostBridgeLib implemetations,
>> >>>>>> so additional two interfaces are added to expose translation
>> >>>>>> information of the platform. To be generic, I add translation for
>> >>>>>> each type of IO or memory resources.
>> >>>>>>
>> >>>>>> The patch is still a RFC, so I only passed the build for qemu64
>> >>>>>> and the function has not been tested yet.
>> >>>>>>
>> >>>>>> Please let me know your comments about it.
>> >>>>>>
>> >>>>>> Thanks.
>> >>>>>>
>> >>>>>> Contributed-under: TianoCore Contribution Agreement 1.1
>> >>>>>> Signed-off-by: Heyi Guo <heyi@linaro.org>
>> >>>>>> Cc: Ruiyu Ni <ruiyu...@intel.com>
>> >>>>>> Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
>> >>>>>> Cc: Star Zeng <star.z...@intel.com>
>> >>>>>> Cc: Eric Dong <eric.d...@intel.com>
>> >>>>>> ---
>> >>>>>>   .../FdtPciHostBridgeLib/FdtPciHostBridgeLib.c  |  19 
>> >>>>>>   .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c   |  53 ---
>> >>>>>>   .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h   |   8 +-
>> >>>>>>   .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 101
>> >>>>>> ++---
>> >>>>>>   MdeModulePkg/Include/Library/PciHostBridgeLib.h|  36 
>> >>>>>>   5 files changed, 192 insertions(+), 25 deletions(-)
>> >>>>>>
>> >>>>>> diff --git
>> >>>>>> a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
>> >>>>>> b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
>> >>>>>> index 5b9c887..0c8371a 100644
>> >>>>>> ---
>> >>>>>> a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
>> >>>>>> +++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.
>> >>>>>> +++ c
>> >>>>>> @@ -360,6 +360,16 @@ PciHostBridgeGetRootBridges (
>> >>>>>> return 
>> >>>>>>   }
>> >>>>>>
>> >>>>>> +PCI_ROOT_BRIDGE_TRANSLATION *
>> >>>>>> +EFIAPI
>> >>&g

Re: [edk2] [RFC] MdeModulePkg/PciHostBridgeDxe: Add support for address translation

2018-02-01 Thread Ni, Ruiyu 


> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Friday, February 2, 2018 1:23 AM
> To: Ni, Ruiyu <ruiyu...@intel.com>
> Cc: Guo Heyi <heyi@linaro.org>,Dong Wei <dong@arm.com>; Dong,
> Eric <eric.d...@intel.com>; edk2-devel@lists.01.org; linaro-uefi  u...@lists.linaro.org>; Kinney, Michael D <michael.d.kin...@intel.com>; Zeng,
> Star <star.z...@intel.com>
> Subject: Re: [edk2] [RFC] MdeModulePkg/PciHostBridgeDxe: Add support for
> address translation
> 
> On 1 February 2018 at 05:03, Ni, Ruiyu <ruiyu...@intel.com> wrote:
> > On 1/29/2018 4:50 PM, Guo Heyi wrote:
> >>
> >> Sorry for the late; I caught cold and didn't work for several days
> >> last week :( Please see my comments below:
> >>
> >>
> >> On Mon, Jan 22, 2018 at 11:36:14AM +0800, Ni, Ruiyu wrote:
> >>>
> >>> On 1/18/2018 9:26 AM, Guo Heyi wrote:
> >>>>
> >>>> On Wed, Jan 17, 2018 at 02:08:06PM +, Ard Biesheuvel wrote:
> >>>>>
> >>>>> On 15 January 2018 at 14:46, Heyi Guo <heyi@linaro.org> wrote:
> >>>>>>
> >>>>>> This is the draft patch for the discussion posted in edk2-devel
> >>>>>> mailing list:
> >>>>>> https://lists.01.org/pipermail/edk2-devel/2017-December/019289.ht
> >>>>>> ml
> >>>>>>
> >>>>>> As discussed in the mailing list, we'd like to add support for
> >>>>>> PCI address translation which is necessary for some non-x86
> >>>>>> platforms. I also want to minimize the changes to the generic
> >>>>>> host bridge driver and platform PciHostBridgeLib implemetations,
> >>>>>> so additional two interfaces are added to expose translation
> >>>>>> information of the platform. To be generic, I add translation for
> >>>>>> each type of IO or memory resources.
> >>>>>>
> >>>>>> The patch is still a RFC, so I only passed the build for qemu64
> >>>>>> and the function has not been tested yet.
> >>>>>>
> >>>>>> Please let me know your comments about it.
> >>>>>>
> >>>>>> Thanks.
> >>>>>>
> >>>>>> Contributed-under: TianoCore Contribution Agreement 1.1
> >>>>>> Signed-off-by: Heyi Guo <heyi@linaro.org>
> >>>>>> Cc: Ruiyu Ni <ruiyu...@intel.com>
> >>>>>> Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>
> >>>>>> Cc: Star Zeng <star.z...@intel.com>
> >>>>>> Cc: Eric Dong <eric.d...@intel.com>
> >>>>>> ---
> >>>>>>   .../FdtPciHostBridgeLib/FdtPciHostBridgeLib.c  |  19 
> >>>>>>   .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c   |  53 ---
> >>>>>>   .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h   |   8 +-
> >>>>>>   .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 101
> >>>>>> ++---
> >>>>>>   MdeModulePkg/Include/Library/PciHostBridgeLib.h|  36 
> >>>>>>   5 files changed, 192 insertions(+), 25 deletions(-)
> >>>>>>
> >>>>>> diff --git
> >>>>>> a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> >>>>>> b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> >>>>>> index 5b9c887..0c8371a 100644
> >>>>>> ---
> >>>>>> a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> >>>>>> +++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.
> >>>>>> +++ c
> >>>>>> @@ -360,6 +360,16 @@ PciHostBridgeGetRootBridges (
> >>>>>> return 
> >>>>>>   }
> >>>>>>
> >>>>>> +PCI_ROOT_BRIDGE_TRANSLATION *
> >>>>>> +EFIAPI
> >>>>>> +PciHostBridgeGetTranslations (
> >>>>>> +  UINTN *Count
> >>>>>> +  )
> >>>>>> +{
> >>>>>> +  *Count = 0;
> >>>>>> +  return NULL;
> >>>>>> +}
> >>>>>> +
> >>>>>>   /**
> >>>>&

Re: [edk2] [RFC] MdeModulePkg/PciHostBridgeDxe: Add support for address translation

2018-02-01 Thread Ard Biesheuvel 
On 1 February 2018 at 05:03, Ni, Ruiyu  wrote:
> On 1/29/2018 4:50 PM, Guo Heyi wrote:
>>
>> Sorry for the late; I caught cold and didn't work for several days last
>> week :(
>> Please see my comments below:
>>
>>
>> On Mon, Jan 22, 2018 at 11:36:14AM +0800, Ni, Ruiyu wrote:
>>>
>>> On 1/18/2018 9:26 AM, Guo Heyi wrote:

 On Wed, Jan 17, 2018 at 02:08:06PM +, Ard Biesheuvel wrote:
>
> On 15 January 2018 at 14:46, Heyi Guo  wrote:
>>
>> This is the draft patch for the discussion posted in edk2-devel
>> mailing list:
>> https://lists.01.org/pipermail/edk2-devel/2017-December/019289.html
>>
>> As discussed in the mailing list, we'd like to add support for PCI
>> address translation which is necessary for some non-x86 platforms. I
>> also want to minimize the changes to the generic host bridge driver
>> and platform PciHostBridgeLib implemetations, so additional two
>> interfaces are added to expose translation information of the
>> platform. To be generic, I add translation for each type of IO or
>> memory resources.
>>
>> The patch is still a RFC, so I only passed the build for qemu64 and
>> the function has not been tested yet.
>>
>> Please let me know your comments about it.
>>
>> Thanks.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Heyi Guo 
>> Cc: Ruiyu Ni 
>> Cc: Ard Biesheuvel 
>> Cc: Star Zeng 
>> Cc: Eric Dong 
>> ---
>>   .../FdtPciHostBridgeLib/FdtPciHostBridgeLib.c  |  19 
>>   .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c   |  53 ---
>>   .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h   |   8 +-
>>   .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 101
>> ++---
>>   MdeModulePkg/Include/Library/PciHostBridgeLib.h|  36 
>>   5 files changed, 192 insertions(+), 25 deletions(-)
>>
>> diff --git
>> a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
>> b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
>> index 5b9c887..0c8371a 100644
>> --- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
>> +++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
>> @@ -360,6 +360,16 @@ PciHostBridgeGetRootBridges (
>> return 
>>   }
>>
>> +PCI_ROOT_BRIDGE_TRANSLATION *
>> +EFIAPI
>> +PciHostBridgeGetTranslations (
>> +  UINTN *Count
>> +  )
>> +{
>> +  *Count = 0;
>> +  return NULL;
>> +}
>> +
>>   /**
>> Free the root bridge instances array returned from
>> PciHostBridgeGetRootBridges().
>> @@ -377,6 +387,15 @@ PciHostBridgeFreeRootBridges (
>> ASSERT (Count == 1);
>>   }
>>
>> +VOID
>> +EFIAPI
>> +PciHostBridgeFreeTranslations (
>> +  PCI_ROOT_BRIDGE_TRANSLATION *Translations,
>> +  UINTN   Count
>> +  )
>> +{
>> +}
>> +
>>   /**
>> Inform the platform that the resource conflict happens.
>>
>> diff --git asame/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
>> b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
>> index 1494848..835e411 100644
>> --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
>> +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
>> @@ -360,18 +360,38 @@ InitializePciHostBridge (
>> PCI_HOST_BRIDGE_INSTANCE*HostBridge;
>> PCI_ROOT_BRIDGE_INSTANCE*RootBridge;
>> PCI_ROOT_BRIDGE *RootBridges;
>> +  PCI_ROOT_BRIDGE_TRANSLATION *Translations;
>> UINTN   RootBridgeCount;
>> +  UINTN   TranslationCount;
>> UINTN   Index;
>> PCI_ROOT_BRIDGE_APERTURE*MemApertures[4];
>
>
> Wouldn't it be much better to add a 'translation' member to
> PCI_ROOT_BRIDGE_APERTURE? That way, existing code just default to a
> translation of 0, and all the handling of the separate array can be
> dropped.
>
 Actually my first idea was the same, but when I looked at the
 implementation of
 PciHostBridgeLib of Ovmf, I found it a little tedious to change the
 existing code in this way. We'll need to check everywhere
 PCI_ROOT_BRIDGE_APERTURE or PCI_ROOT_BRIDGE is used, to make sure the
 translation field is initialized to be zero, e.g. line 235~245.

 What I did in this RFC is not so straightforward indeed. So I can change
 the
 code if we all agree to add Translation field into
 PCI_ROOT_BRIDGE_APERTURE
 directly.

 Thanks,

 Gary (Heyi Guo)
>>>
>>>
>>> I also agree to put the translation fields into the
>>>

Re: [edk2] [RFC] MdeModulePkg/PciHostBridgeDxe: Add support for address translation

2018-01-31 Thread Ni, Ruiyu 

On 1/29/2018 4:50 PM, Guo Heyi wrote:

Sorry for the late; I caught cold and didn't work for several days last week :(
Please see my comments below:


On Mon, Jan 22, 2018 at 11:36:14AM +0800, Ni, Ruiyu wrote:

On 1/18/2018 9:26 AM, Guo Heyi wrote:

On Wed, Jan 17, 2018 at 02:08:06PM +, Ard Biesheuvel wrote:

On 15 January 2018 at 14:46, Heyi Guo  wrote:

This is the draft patch for the discussion posted in edk2-devel
mailing list:
https://lists.01.org/pipermail/edk2-devel/2017-December/019289.html

As discussed in the mailing list, we'd like to add support for PCI
address translation which is necessary for some non-x86 platforms. I
also want to minimize the changes to the generic host bridge driver
and platform PciHostBridgeLib implemetations, so additional two
interfaces are added to expose translation information of the
platform. To be generic, I add translation for each type of IO or
memory resources.

The patch is still a RFC, so I only passed the build for qemu64 and
the function has not been tested yet.

Please let me know your comments about it.

Thanks.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Heyi Guo 
Cc: Ruiyu Ni 
Cc: Ard Biesheuvel 
Cc: Star Zeng 
Cc: Eric Dong 
---
  .../FdtPciHostBridgeLib/FdtPciHostBridgeLib.c  |  19 
  .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c   |  53 ---
  .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h   |   8 +-
  .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 101 ++---
  MdeModulePkg/Include/Library/PciHostBridgeLib.h|  36 
  5 files changed, 192 insertions(+), 25 deletions(-)

diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c 
b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
index 5b9c887..0c8371a 100644
--- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
+++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
@@ -360,6 +360,16 @@ PciHostBridgeGetRootBridges (
return 
  }

+PCI_ROOT_BRIDGE_TRANSLATION *
+EFIAPI
+PciHostBridgeGetTranslations (
+  UINTN *Count
+  )
+{
+  *Count = 0;
+  return NULL;
+}
+
  /**
Free the root bridge instances array returned from
PciHostBridgeGetRootBridges().
@@ -377,6 +387,15 @@ PciHostBridgeFreeRootBridges (
ASSERT (Count == 1);
  }

+VOID
+EFIAPI
+PciHostBridgeFreeTranslations (
+  PCI_ROOT_BRIDGE_TRANSLATION *Translations,
+  UINTN   Count
+  )
+{
+}
+
  /**
Inform the platform that the resource conflict happens.

diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c 
b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
index 1494848..835e411 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
@@ -360,18 +360,38 @@ InitializePciHostBridge (
PCI_HOST_BRIDGE_INSTANCE*HostBridge;
PCI_ROOT_BRIDGE_INSTANCE*RootBridge;
PCI_ROOT_BRIDGE *RootBridges;
+  PCI_ROOT_BRIDGE_TRANSLATION *Translations;
UINTN   RootBridgeCount;
+  UINTN   TranslationCount;
UINTN   Index;
PCI_ROOT_BRIDGE_APERTURE*MemApertures[4];


Wouldn't it be much better to add a 'translation' member to
PCI_ROOT_BRIDGE_APERTURE? That way, existing code just default to a
translation of 0, and all the handling of the separate array can be
dropped.


Actually my first idea was the same, but when I looked at the implementation of
PciHostBridgeLib of Ovmf, I found it a little tedious to change the
existing code in this way. We'll need to check everywhere
PCI_ROOT_BRIDGE_APERTURE or PCI_ROOT_BRIDGE is used, to make sure the
translation field is initialized to be zero, e.g. line 235~245.

What I did in this RFC is not so straightforward indeed. So I can change the
code if we all agree to add Translation field into PCI_ROOT_BRIDGE_APERTURE
directly.

Thanks,

Gary (Heyi Guo)


I also agree to put the translation fields into the
PCI_ROOT_BRIDGE_APERTURE.


But I think we may have different understandings to the address
translation.
My understanding to the whole translation:
1. PciHostBridge.GetProposedResources () returns resource information
for a single root bridge. It only carries the address range in PCI
view.
The address range/resource is reported/submitted by PciHostBridgeLib.
Before the change, CPU view equals to PCI view. So PciHostBridgeDxe
driver directly adds the resource to GCD.
In your change, PciHostBridgeDxe assumes the source is in PCI view
and it adds offset to convert to CPU view.
CPU-address = PCI-address + offset. <-- Equation #A


2. PciRootBridgeIo.Configuration() returns the resource information
for a single root bridge. It carries the address range in CPU view,
and the translation offset.
However, per

Re: [edk2] [RFC] MdeModulePkg/PciHostBridgeDxe: Add support for address translation

2018-01-29 Thread Guo Heyi 
Sorry for the late; I caught cold and didn't work for several days last week :(
Please see my comments below:


On Mon, Jan 22, 2018 at 11:36:14AM +0800, Ni, Ruiyu wrote:
> On 1/18/2018 9:26 AM, Guo Heyi wrote:
> >On Wed, Jan 17, 2018 at 02:08:06PM +, Ard Biesheuvel wrote:
> >>On 15 January 2018 at 14:46, Heyi Guo  wrote:
> >>>This is the draft patch for the discussion posted in edk2-devel
> >>>mailing list:
> >>>https://lists.01.org/pipermail/edk2-devel/2017-December/019289.html
> >>>
> >>>As discussed in the mailing list, we'd like to add support for PCI
> >>>address translation which is necessary for some non-x86 platforms. I
> >>>also want to minimize the changes to the generic host bridge driver
> >>>and platform PciHostBridgeLib implemetations, so additional two
> >>>interfaces are added to expose translation information of the
> >>>platform. To be generic, I add translation for each type of IO or
> >>>memory resources.
> >>>
> >>>The patch is still a RFC, so I only passed the build for qemu64 and
> >>>the function has not been tested yet.
> >>>
> >>>Please let me know your comments about it.
> >>>
> >>>Thanks.
> >>>
> >>>Contributed-under: TianoCore Contribution Agreement 1.1
> >>>Signed-off-by: Heyi Guo 
> >>>Cc: Ruiyu Ni 
> >>>Cc: Ard Biesheuvel 
> >>>Cc: Star Zeng 
> >>>Cc: Eric Dong 
> >>>---
> >>>  .../FdtPciHostBridgeLib/FdtPciHostBridgeLib.c  |  19 
> >>>  .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c   |  53 ---
> >>>  .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h   |   8 +-
> >>>  .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 101 
> >>> ++---
> >>>  MdeModulePkg/Include/Library/PciHostBridgeLib.h|  36 
> >>>  5 files changed, 192 insertions(+), 25 deletions(-)
> >>>
> >>>diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c 
> >>>b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> >>>index 5b9c887..0c8371a 100644
> >>>--- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> >>>+++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> >>>@@ -360,6 +360,16 @@ PciHostBridgeGetRootBridges (
> >>>return 
> >>>  }
> >>>
> >>>+PCI_ROOT_BRIDGE_TRANSLATION *
> >>>+EFIAPI
> >>>+PciHostBridgeGetTranslations (
> >>>+  UINTN *Count
> >>>+  )
> >>>+{
> >>>+  *Count = 0;
> >>>+  return NULL;
> >>>+}
> >>>+
> >>>  /**
> >>>Free the root bridge instances array returned from
> >>>PciHostBridgeGetRootBridges().
> >>>@@ -377,6 +387,15 @@ PciHostBridgeFreeRootBridges (
> >>>ASSERT (Count == 1);
> >>>  }
> >>>
> >>>+VOID
> >>>+EFIAPI
> >>>+PciHostBridgeFreeTranslations (
> >>>+  PCI_ROOT_BRIDGE_TRANSLATION *Translations,
> >>>+  UINTN   Count
> >>>+  )
> >>>+{
> >>>+}
> >>>+
> >>>  /**
> >>>Inform the platform that the resource conflict happens.
> >>>
> >>>diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c 
> >>>b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> >>>index 1494848..835e411 100644
> >>>--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> >>>+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> >>>@@ -360,18 +360,38 @@ InitializePciHostBridge (
> >>>PCI_HOST_BRIDGE_INSTANCE*HostBridge;
> >>>PCI_ROOT_BRIDGE_INSTANCE*RootBridge;
> >>>PCI_ROOT_BRIDGE *RootBridges;
> >>>+  PCI_ROOT_BRIDGE_TRANSLATION *Translations;
> >>>UINTN   RootBridgeCount;
> >>>+  UINTN   TranslationCount;
> >>>UINTN   Index;
> >>>PCI_ROOT_BRIDGE_APERTURE*MemApertures[4];
> >>
> >>Wouldn't it be much better to add a 'translation' member to
> >>PCI_ROOT_BRIDGE_APERTURE? That way, existing code just default to a
> >>translation of 0, and all the handling of the separate array can be
> >>dropped.
> >>
> >Actually my first idea was the same, but when I looked at the implementation 
> >of
> >PciHostBridgeLib of Ovmf, I found it a little tedious to change the
> >existing code in this way. We'll need to check everywhere
> >PCI_ROOT_BRIDGE_APERTURE or PCI_ROOT_BRIDGE is used, to make sure the
> >translation field is initialized to be zero, e.g. line 235~245.
> >
> >What I did in this RFC is not so straightforward indeed. So I can change the
> >code if we all agree to add Translation field into PCI_ROOT_BRIDGE_APERTURE
> >directly.
> >
> >Thanks,
> >
> >Gary (Heyi Guo)
> 
> I also agree to put the translation fields into the
> PCI_ROOT_BRIDGE_APERTURE.
> 
> 
> But I think we may have different understandings to the address
> translation.
> My understanding to the whole translation:
> 1. PciHostBridge.GetProposedResources () returns resource information
>for a single root bridge. It only carries the address range in PCI
>view.
>The address range/resource is reported/submitted by PciHostBridgeLib.
>

Re: [edk2] [RFC] MdeModulePkg/PciHostBridgeDxe: Add support for address translation

2018-01-21 Thread Ni, Ruiyu 

On 1/18/2018 9:26 AM, Guo Heyi wrote:

On Wed, Jan 17, 2018 at 02:08:06PM +, Ard Biesheuvel wrote:

On 15 January 2018 at 14:46, Heyi Guo  wrote:

This is the draft patch for the discussion posted in edk2-devel
mailing list:
https://lists.01.org/pipermail/edk2-devel/2017-December/019289.html

As discussed in the mailing list, we'd like to add support for PCI
address translation which is necessary for some non-x86 platforms. I
also want to minimize the changes to the generic host bridge driver
and platform PciHostBridgeLib implemetations, so additional two
interfaces are added to expose translation information of the
platform. To be generic, I add translation for each type of IO or
memory resources.

The patch is still a RFC, so I only passed the build for qemu64 and
the function has not been tested yet.

Please let me know your comments about it.

Thanks.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Heyi Guo 
Cc: Ruiyu Ni 
Cc: Ard Biesheuvel 
Cc: Star Zeng 
Cc: Eric Dong 
---
  .../FdtPciHostBridgeLib/FdtPciHostBridgeLib.c  |  19 
  .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c   |  53 ---
  .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h   |   8 +-
  .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 101 ++---
  MdeModulePkg/Include/Library/PciHostBridgeLib.h|  36 
  5 files changed, 192 insertions(+), 25 deletions(-)

diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c 
b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
index 5b9c887..0c8371a 100644
--- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
+++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
@@ -360,6 +360,16 @@ PciHostBridgeGetRootBridges (
return 
  }

+PCI_ROOT_BRIDGE_TRANSLATION *
+EFIAPI
+PciHostBridgeGetTranslations (
+  UINTN *Count
+  )
+{
+  *Count = 0;
+  return NULL;
+}
+
  /**
Free the root bridge instances array returned from
PciHostBridgeGetRootBridges().
@@ -377,6 +387,15 @@ PciHostBridgeFreeRootBridges (
ASSERT (Count == 1);
  }

+VOID
+EFIAPI
+PciHostBridgeFreeTranslations (
+  PCI_ROOT_BRIDGE_TRANSLATION *Translations,
+  UINTN   Count
+  )
+{
+}
+
  /**
Inform the platform that the resource conflict happens.

diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c 
b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
index 1494848..835e411 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
@@ -360,18 +360,38 @@ InitializePciHostBridge (
PCI_HOST_BRIDGE_INSTANCE*HostBridge;
PCI_ROOT_BRIDGE_INSTANCE*RootBridge;
PCI_ROOT_BRIDGE *RootBridges;
+  PCI_ROOT_BRIDGE_TRANSLATION *Translations;
UINTN   RootBridgeCount;
+  UINTN   TranslationCount;
UINTN   Index;
PCI_ROOT_BRIDGE_APERTURE*MemApertures[4];


Wouldn't it be much better to add a 'translation' member to
PCI_ROOT_BRIDGE_APERTURE? That way, existing code just default to a
translation of 0, and all the handling of the separate array can be
dropped.


Actually my first idea was the same, but when I looked at the implementation of
PciHostBridgeLib of Ovmf, I found it a little tedious to change the
existing code in this way. We'll need to check everywhere
PCI_ROOT_BRIDGE_APERTURE or PCI_ROOT_BRIDGE is used, to make sure the
translation field is initialized to be zero, e.g. line 235~245.

What I did in this RFC is not so straightforward indeed. So I can change the
code if we all agree to add Translation field into PCI_ROOT_BRIDGE_APERTURE
directly.

Thanks,

Gary (Heyi Guo)


I also agree to put the translation fields into the
PCI_ROOT_BRIDGE_APERTURE.


But I think we may have different understandings to the address
translation.
My understanding to the whole translation:
1. PciHostBridge.GetProposedResources () returns resource information
   for a single root bridge. It only carries the address range in PCI
   view.
   The address range/resource is reported/submitted by PciHostBridgeLib.
   Before the change, CPU view equals to PCI view. So PciHostBridgeDxe
   driver directly adds the resource to GCD.
   In your change, PciHostBridgeDxe assumes the source is in PCI view
   and it adds offset to convert to CPU view.
   CPU-address = PCI-address + offset. <-- Equation #A


2. PciRootBridgeIo.Configuration() returns the resource information
   for a single root bridge. It carries the address range in CPU view,
   and the translation offset.
   However, per UEFI spec, "Address Translation Offset. Offset to apply
   to the Starting address of a BAR to convert it to a PCI address. "
   PCI-address = CPU-address + offset. <-- Equation #B

   You can see that equation #A and #B are

Re: [edk2] [RFC] MdeModulePkg/PciHostBridgeDxe: Add support for address translation

2018-01-17 Thread Guo Heyi 
On Wed, Jan 17, 2018 at 02:08:06PM +, Ard Biesheuvel wrote:
> On 15 January 2018 at 14:46, Heyi Guo  wrote:
> > This is the draft patch for the discussion posted in edk2-devel
> > mailing list:
> > https://lists.01.org/pipermail/edk2-devel/2017-December/019289.html
> >
> > As discussed in the mailing list, we'd like to add support for PCI
> > address translation which is necessary for some non-x86 platforms. I
> > also want to minimize the changes to the generic host bridge driver
> > and platform PciHostBridgeLib implemetations, so additional two
> > interfaces are added to expose translation information of the
> > platform. To be generic, I add translation for each type of IO or
> > memory resources.
> >
> > The patch is still a RFC, so I only passed the build for qemu64 and
> > the function has not been tested yet.
> >
> > Please let me know your comments about it.
> >
> > Thanks.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Heyi Guo 
> > Cc: Ruiyu Ni 
> > Cc: Ard Biesheuvel 
> > Cc: Star Zeng 
> > Cc: Eric Dong 
> > ---
> >  .../FdtPciHostBridgeLib/FdtPciHostBridgeLib.c  |  19 
> >  .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c   |  53 ---
> >  .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h   |   8 +-
> >  .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 101 
> > ++---
> >  MdeModulePkg/Include/Library/PciHostBridgeLib.h|  36 
> >  5 files changed, 192 insertions(+), 25 deletions(-)
> >
> > diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c 
> > b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> > index 5b9c887..0c8371a 100644
> > --- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> > +++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> > @@ -360,6 +360,16 @@ PciHostBridgeGetRootBridges (
> >return 
> >  }
> >
> > +PCI_ROOT_BRIDGE_TRANSLATION *
> > +EFIAPI
> > +PciHostBridgeGetTranslations (
> > +  UINTN *Count
> > +  )
> > +{
> > +  *Count = 0;
> > +  return NULL;
> > +}
> > +
> >  /**
> >Free the root bridge instances array returned from
> >PciHostBridgeGetRootBridges().
> > @@ -377,6 +387,15 @@ PciHostBridgeFreeRootBridges (
> >ASSERT (Count == 1);
> >  }
> >
> > +VOID
> > +EFIAPI
> > +PciHostBridgeFreeTranslations (
> > +  PCI_ROOT_BRIDGE_TRANSLATION *Translations,
> > +  UINTN   Count
> > +  )
> > +{
> > +}
> > +
> >  /**
> >Inform the platform that the resource conflict happens.
> >
> > diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c 
> > b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> > index 1494848..835e411 100644
> > --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> > +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> > @@ -360,18 +360,38 @@ InitializePciHostBridge (
> >PCI_HOST_BRIDGE_INSTANCE*HostBridge;
> >PCI_ROOT_BRIDGE_INSTANCE*RootBridge;
> >PCI_ROOT_BRIDGE *RootBridges;
> > +  PCI_ROOT_BRIDGE_TRANSLATION *Translations;
> >UINTN   RootBridgeCount;
> > +  UINTN   TranslationCount;
> >UINTN   Index;
> >PCI_ROOT_BRIDGE_APERTURE*MemApertures[4];
> 
> Wouldn't it be much better to add a 'translation' member to
> PCI_ROOT_BRIDGE_APERTURE? That way, existing code just default to a
> translation of 0, and all the handling of the separate array can be
> dropped.
> 
Actually my first idea was the same, but when I looked at the implementation of
PciHostBridgeLib of Ovmf, I found it a little tedious to change the
existing code in this way. We'll need to check everywhere
PCI_ROOT_BRIDGE_APERTURE or PCI_ROOT_BRIDGE is used, to make sure the
translation field is initialized to be zero, e.g. line 235~245.

What I did in this RFC is not so straightforward indeed. So I can change the
code if we all agree to add Translation field into PCI_ROOT_BRIDGE_APERTURE
directly.

Thanks,

Gary (Heyi Guo)


> > +  UINT64  MemTranslation[4];
> >UINTN   MemApertureIndex;
> >BOOLEAN ResourceAssigned;
> >LIST_ENTRY  *Link;
> > +  UINT64  Trans;
> >
> >RootBridges = PciHostBridgeGetRootBridges ();
> >if ((RootBridges == NULL) || (RootBridgeCount == 0)) {
> >  return EFI_UNSUPPORTED;
> >}
> >
> > +  Translations = PciHostBridgeGetTranslations ();
> > +  if (Translations == NULL || TranslationCount == 0) {
> > +TranslationCount = 0;
> > +Translations = AllocateZeroPool (RootBridgeCount * sizeof 
> > (*Translations));
> > +if (Translations == NULL) {
> > +  PciHostBridgeFreeRootBridges (RootBridges, RootBridgeCount);
> > +  return EFI_OUT_OF_RESOURCES;
> > +}
> > +  }
> > +
> > +  if

Re: [edk2] [RFC] MdeModulePkg/PciHostBridgeDxe: Add support for address translation

2018-01-17 Thread Ard Biesheuvel 
On 15 January 2018 at 14:46, Heyi Guo  wrote:
> This is the draft patch for the discussion posted in edk2-devel
> mailing list:
> https://lists.01.org/pipermail/edk2-devel/2017-December/019289.html
>
> As discussed in the mailing list, we'd like to add support for PCI
> address translation which is necessary for some non-x86 platforms. I
> also want to minimize the changes to the generic host bridge driver
> and platform PciHostBridgeLib implemetations, so additional two
> interfaces are added to expose translation information of the
> platform. To be generic, I add translation for each type of IO or
> memory resources.
>
> The patch is still a RFC, so I only passed the build for qemu64 and
> the function has not been tested yet.
>
> Please let me know your comments about it.
>
> Thanks.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Heyi Guo 
> Cc: Ruiyu Ni 
> Cc: Ard Biesheuvel 
> Cc: Star Zeng 
> Cc: Eric Dong 
> ---
>  .../FdtPciHostBridgeLib/FdtPciHostBridgeLib.c  |  19 
>  .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c   |  53 ---
>  .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h   |   8 +-
>  .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 101 
> ++---
>  MdeModulePkg/Include/Library/PciHostBridgeLib.h|  36 
>  5 files changed, 192 insertions(+), 25 deletions(-)
>
> diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c 
> b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> index 5b9c887..0c8371a 100644
> --- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> +++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
> @@ -360,6 +360,16 @@ PciHostBridgeGetRootBridges (
>return 
>  }
>
> +PCI_ROOT_BRIDGE_TRANSLATION *
> +EFIAPI
> +PciHostBridgeGetTranslations (
> +  UINTN *Count
> +  )
> +{
> +  *Count = 0;
> +  return NULL;
> +}
> +
>  /**
>Free the root bridge instances array returned from
>PciHostBridgeGetRootBridges().
> @@ -377,6 +387,15 @@ PciHostBridgeFreeRootBridges (
>ASSERT (Count == 1);
>  }
>
> +VOID
> +EFIAPI
> +PciHostBridgeFreeTranslations (
> +  PCI_ROOT_BRIDGE_TRANSLATION *Translations,
> +  UINTN   Count
> +  )
> +{
> +}
> +
>  /**
>Inform the platform that the resource conflict happens.
>
> diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c 
> b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> index 1494848..835e411 100644
> --- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> +++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
> @@ -360,18 +360,38 @@ InitializePciHostBridge (
>PCI_HOST_BRIDGE_INSTANCE*HostBridge;
>PCI_ROOT_BRIDGE_INSTANCE*RootBridge;
>PCI_ROOT_BRIDGE *RootBridges;
> +  PCI_ROOT_BRIDGE_TRANSLATION *Translations;
>UINTN   RootBridgeCount;
> +  UINTN   TranslationCount;
>UINTN   Index;
>PCI_ROOT_BRIDGE_APERTURE*MemApertures[4];

Wouldn't it be much better to add a 'translation' member to
PCI_ROOT_BRIDGE_APERTURE? That way, existing code just default to a
translation of 0, and all the handling of the separate array can be
dropped.

> +  UINT64  MemTranslation[4];
>UINTN   MemApertureIndex;
>BOOLEAN ResourceAssigned;
>LIST_ENTRY  *Link;
> +  UINT64  Trans;
>
>RootBridges = PciHostBridgeGetRootBridges ();
>if ((RootBridges == NULL) || (RootBridgeCount == 0)) {
>  return EFI_UNSUPPORTED;
>}
>
> +  Translations = PciHostBridgeGetTranslations ();
> +  if (Translations == NULL || TranslationCount == 0) {
> +TranslationCount = 0;
> +Translations = AllocateZeroPool (RootBridgeCount * sizeof 
> (*Translations));
> +if (Translations == NULL) {
> +  PciHostBridgeFreeRootBridges (RootBridges, RootBridgeCount);
> +  return EFI_OUT_OF_RESOURCES;
> +}
> +  }
> +
> +  if (TranslationCount != 0 && TranslationCount != RootBridgeCount) {
> +DEBUG ((DEBUG_ERROR, "Error: count of root bridges (%d) and translation 
> (%d) are different!\n",
> +  RootBridgeCount, TranslationCount));
> +return EFI_INVALID_PARAMETER;
> +  }
> +
>Status = gBS->LocateProtocol (, NULL, (VOID 
> **) );
>ASSERT_EFI_ERROR (Status);
>Status = gBS->LocateProtocol (, NULL, (VOID **) 
> );
> @@ -395,7 +415,7 @@ InitializePciHostBridge (
>  //
>  // Create Root Bridge Handle Instance
>  //
> -RootBridge = CreateRootBridge ([Index]);
> +RootBridge = CreateRootBridge ([Index], 
> [Index]);
>  ASSERT (RootBridge != NULL);
>  if (RootBridge == NULL) {
>continue;
> @@ -411,8 +431,9 @@ InitializePciHostBridge (
>  }
>
>  if (RootBridges[Index].Io.Base <=

[edk2] [RFC] MdeModulePkg/PciHostBridgeDxe: Add support for address translation

2018-01-15 Thread Heyi Guo 
This is the draft patch for the discussion posted in edk2-devel
mailing list:
https://lists.01.org/pipermail/edk2-devel/2017-December/019289.html

As discussed in the mailing list, we'd like to add support for PCI
address translation which is necessary for some non-x86 platforms. I
also want to minimize the changes to the generic host bridge driver
and platform PciHostBridgeLib implemetations, so additional two
interfaces are added to expose translation information of the
platform. To be generic, I add translation for each type of IO or
memory resources.

The patch is still a RFC, so I only passed the build for qemu64 and
the function has not been tested yet.

Please let me know your comments about it.

Thanks.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Heyi Guo 
Cc: Ruiyu Ni 
Cc: Ard Biesheuvel 
Cc: Star Zeng 
Cc: Eric Dong 
---
 .../FdtPciHostBridgeLib/FdtPciHostBridgeLib.c  |  19 
 .../Bus/Pci/PciHostBridgeDxe/PciHostBridge.c   |  53 ---
 .../Bus/Pci/PciHostBridgeDxe/PciRootBridge.h   |   8 +-
 .../Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c | 101 ++---
 MdeModulePkg/Include/Library/PciHostBridgeLib.h|  36 
 5 files changed, 192 insertions(+), 25 deletions(-)

diff --git a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c 
b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
index 5b9c887..0c8371a 100644
--- a/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
+++ b/ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c
@@ -360,6 +360,16 @@ PciHostBridgeGetRootBridges (
   return 
 }
 
+PCI_ROOT_BRIDGE_TRANSLATION *
+EFIAPI
+PciHostBridgeGetTranslations (
+  UINTN *Count
+  )
+{
+  *Count = 0;
+  return NULL;
+}
+
 /**
   Free the root bridge instances array returned from
   PciHostBridgeGetRootBridges().
@@ -377,6 +387,15 @@ PciHostBridgeFreeRootBridges (
   ASSERT (Count == 1);
 }
 
+VOID
+EFIAPI
+PciHostBridgeFreeTranslations (
+  PCI_ROOT_BRIDGE_TRANSLATION *Translations,
+  UINTN   Count
+  )
+{
+}
+
 /**
   Inform the platform that the resource conflict happens.
 
diff --git a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c 
b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
index 1494848..835e411 100644
--- a/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
+++ b/MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridge.c
@@ -360,18 +360,38 @@ InitializePciHostBridge (
   PCI_HOST_BRIDGE_INSTANCE*HostBridge;
   PCI_ROOT_BRIDGE_INSTANCE*RootBridge;
   PCI_ROOT_BRIDGE *RootBridges;
+  PCI_ROOT_BRIDGE_TRANSLATION *Translations;
   UINTN   RootBridgeCount;
+  UINTN   TranslationCount;
   UINTN   Index;
   PCI_ROOT_BRIDGE_APERTURE*MemApertures[4];
+  UINT64  MemTranslation[4];
   UINTN   MemApertureIndex;
   BOOLEAN ResourceAssigned;
   LIST_ENTRY  *Link;
+  UINT64  Trans;
 
   RootBridges = PciHostBridgeGetRootBridges ();
   if ((RootBridges == NULL) || (RootBridgeCount == 0)) {
 return EFI_UNSUPPORTED;
   }
 
+  Translations = PciHostBridgeGetTranslations ();
+  if (Translations == NULL || TranslationCount == 0) {
+TranslationCount = 0;
+Translations = AllocateZeroPool (RootBridgeCount * sizeof (*Translations));
+if (Translations == NULL) {
+  PciHostBridgeFreeRootBridges (RootBridges, RootBridgeCount);
+  return EFI_OUT_OF_RESOURCES;
+}
+  }
+
+  if (TranslationCount != 0 && TranslationCount != RootBridgeCount) {
+DEBUG ((DEBUG_ERROR, "Error: count of root bridges (%d) and translation 
(%d) are different!\n",
+  RootBridgeCount, TranslationCount));
+return EFI_INVALID_PARAMETER;
+  }
+
   Status = gBS->LocateProtocol (, NULL, (VOID 
**) );
   ASSERT_EFI_ERROR (Status);
   Status = gBS->LocateProtocol (, NULL, (VOID **) 
);
@@ -395,7 +415,7 @@ InitializePciHostBridge (
 //
 // Create Root Bridge Handle Instance
 //
-RootBridge = CreateRootBridge ([Index]);
+RootBridge = CreateRootBridge ([Index], [Index]);
 ASSERT (RootBridge != NULL);
 if (RootBridge == NULL) {
   continue;
@@ -411,8 +431,9 @@ InitializePciHostBridge (
 }
 
 if (RootBridges[Index].Io.Base <= RootBridges[Index].Io.Limit) {
+  Trans = Translations[Index].IoTranslation;
   Status = AddIoSpace (
- RootBridges[Index].Io.Base,
+ RootBridges[Index].Io.Base + Trans,
  RootBridges[Index].Io.Limit - RootBridges[Index].Io.Base + 1
  );
   ASSERT_EFI_ERROR (Status);
@@ -422,7 +443,7 @@ InitializePciHostBridge (
 EfiGcdIoTypeIo,
 0,
 RootBridges[Index].Io.Limit -