Re: [PATCH V3 00/12] Migrate ArmVirtPkg modules to OvmfPkg


Abner Chang
 

Hi Are, I am so sorry about that I just merged it two hours ago. Any process needed if we want to change the commit messages for adding your review tag?

Abner


From: Ard Biesheuvel <ardb@...>
Sent: Thursday, October 14, 2021 5:51:56 PM
To: edk2-devel-groups-io <devel@edk2.groups.io>; Chang, Abner (HPS SW/FW Technologist) <abner.chang@...>
Cc: gaoliming <gaoliming@...>; Ard Biesheuvel <ardb+tianocore@...>; Leif Lindholm <leif@...>; Sami Mujawar <sami.mujawar@...>; Jiewen Yao <jiewen.yao@...>; Jordan Justen <jordan.l.justen@...>; Gerd Hoffmann <kraxel@...>; Schaefer, Daniel <daniel.schaefer@...>; Sunil V L <sunilvl@...>; Zhiguang Liu <zhiguang.liu@...>; Michael D Kinney <michael.d.kinney@...>
Subject: Re: [edk2-devel] [PATCH V3 00/12] Migrate ArmVirtPkg modules to OvmfPkg
 
On Tue, 12 Oct 2021 at 06:17, Abner Chang <abner.chang@...> wrote:
>
> Hi package maintainers,
>
> The review process of this patch set is almost done and please allow me to merge it because the corresponding changes on edk2-platform is also required to merge.
>
>
>
> Ard and Leif, do I need the Reviewed-by or Acked-by from either of you? Or I can just proceed the merge process as Ard has no problem with this patch set?
>
>

Hi Abner,

Where needed

Acked-by: Ard Biesheuvel <ardb@...>

Feel free to merge it whenever convenient for you.


>
> From: gaoliming [mailto:gaoliming@...]
> Sent: Monday, October 11, 2021 9:22 AM
> To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist) <abner.chang@...>; ardb@...
> Cc: 'Ard Biesheuvel' <ardb+tianocore@...>; 'Leif Lindholm' <leif@...>; 'Sami Mujawar' <sami.mujawar@...>; 'Jiewen Yao' <jiewen.yao@...>; 'Jordan Justen' <jordan.l.justen@...>; 'Gerd Hoffmann' <kraxel@...>; Schaefer, Daniel <daniel.schaefer@...>; 'Sunil V L' <sunilvl@...>; 'Zhiguang Liu' <zhiguang.liu@...>; 'Michael D Kinney' <michael.d.kinney@...>
> Subject: 回复: [edk2-devel] [PATCH V3 00/12] Migrate ArmVirtPkg modules to OvmfPkg
>
>
>
> The change in MdePkg is good to me. Reviewed-by: Liming Gao <gaoliming@...>
>
>
>
> 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Abner Chang
> 发送时间: 2021年10月8日 11:39
> 收件人: devel@edk2.groups.io; gaoliming@...; ardb@...
> 抄送: 'Ard Biesheuvel' <ardb+tianocore@...>; 'Leif Lindholm' <leif@...>; 'Sami Mujawar' <sami.mujawar@...>; 'Jiewen Yao' <jiewen.yao@...>; 'Jordan Justen' <jordan.l.justen@...>; 'Gerd Hoffmann' <kraxel@...>; Schaefer, Daniel <daniel.schaefer@...>; 'Sunil V L' <sunilvl@...>; 'Zhiguang Liu' <zhiguang.liu@...>; 'Michael D Kinney' <michael.d.kinney@...>
> 主题: Re: [edk2-devel] [PATCH V3 00/12] Migrate ArmVirtPkg modules to OvmfPkg
>
>
>
> Thanks Liming, could you please also give the reviewed-by.
>
>
>
> Hi Ard, also need your reviewed-by for ArmPkg. Then this changes could be upstream and finished.
>
> Thanks
>
> Abner
>
>
>
>
>
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of gaoliming
> Sent: Friday, October 8, 2021 11:14 AM
> To: Chang, Abner (HPS SW/FW Technologist) <abner.chang@...>; 'edk2-devel-groups-io' <devel@edk2.groups.io>; ardb@...
> Cc: 'Ard Biesheuvel' <ardb+tianocore@...>; 'Leif Lindholm' <leif@...>; 'Sami Mujawar' <sami.mujawar@...>; 'Jiewen Yao' <jiewen.yao@...>; 'Jordan Justen' <jordan.l.justen@...>; 'Gerd Hoffmann' <kraxel@...>; Schaefer, Daniel <daniel.schaefer@...>; 'Sunil V L' <sunilvl@...>; 'Zhiguang Liu' <zhiguang.liu@...>; 'Michael D Kinney' <michael.d.kinney@...>
> Subject: 回复: [edk2-devel] [PATCH V3 00/12] Migrate ArmVirtPkg modules to OvmfPkg
>
>
>
> Ard and Abner:
>
>  I am OK to add these three PCDs PcdPciMmio32Translation, PcdPciMmio64Translation, PcdPciIoTranslation to MdePkg.
>
>
>
> Thanks
>
> Liming
>
> 发件人: Chang, Abner (HPS SW/FW Technologist) <abner.chang@...>
> 发送时间: 2021年10月6日 17:27
> 收件人: edk2-devel-groups-io <devel@edk2.groups.io>; ardb@...; Chang, Abner (HPS SW/FW Technologist) <abner.chang@...>
> 抄送: Ard Biesheuvel <ardb+tianocore@...>; Leif Lindholm <leif@...>; Sami Mujawar <sami.mujawar@...>; Jiewen Yao <jiewen.yao@...>; Jordan Justen <jordan.l.justen@...>; Gerd Hoffmann <kraxel@...>; Schaefer, Daniel <daniel.schaefer@...>; Sunil V L <sunilvl@...>; Liming Gao <gaoliming@...>; Zhiguang Liu <zhiguang.liu@...>; Michael D Kinney <michael.d.kinney@...>
> 主题: Re: [edk2-devel] [PATCH V3 00/12] Migrate ArmVirtPkg modules to OvmfPkg
>
>
>
> Hi Ard,
>
> I realized there is a problem if we duplicate ArmPkg defined PCD to under OvmfPkg (e.g. PcdPciIoTranslate PCD) when I was duplicating this PCD to OvmfPkg.
>
> FdtPciProducerLib is relocated to OvmfPkg/Fdt and uses PcdPciIoTranslate PCD declared with OvmfPkg namespace. FdtPciProducerLib is also used by both ArmVirtPkg  and RiscVVirtPkg.
>
> ArmVirtPkg uses ArmPciCpuIoDxe provided by ArmPkg however PcdPciIoTranslate used by ArmPciCpuIoDxe  is declared with ArmPkg namespace.
>
> I think this results in the problem because PcdPciIoTranslate(s) that are referred by ArmPkg and ArmVirtPkg come from two different namespaces, right? Unless ArmPciCpuIoDxe uses the one declared in OvmfPkg, but I don't think we want to do this.
>
> Thought? Otherwise, we should still keep the original patch that relocates these PCDs under MdePkg.
>
>
>
> Thanks
>
> Abner
>
>
>
>
>
> ________________________________
>
> From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Abner Chang <abner.chang@...>
> Sent: Tuesday, October 5, 2021 11:00 PM
> To: edk2-devel-groups-io <devel@edk2.groups.io>; ardb@... <ardb@...>
> Cc: Ard Biesheuvel <ardb+tianocore@...>; Leif Lindholm <leif@...>; Sami Mujawar <sami.mujawar@...>; Jiewen Yao <jiewen.yao@...>; Jordan Justen <jordan.l.justen@...>; Gerd Hoffmann <kraxel@...>; Schaefer, Daniel <daniel.schaefer@...>; Sunil V L <sunilvl@...>; Liming Gao <gaoliming@...>; Zhiguang Liu <zhiguang.liu@...>; Michael D Kinney <michael.d.kinney@...>
> Subject: Re: [edk2-devel] [PATCH V3 00/12] Migrate ArmVirtPkg modules to OvmfPkg
>
>
>
> Hi Ard,
>
> This way reduces the impact of MdePkg. We can try it.
>
>
>
> Thanks
>
> Abner
>
>
>
> ________________________________
>
> From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Ard Biesheuvel <ardb@...>
> Sent: Tuesday, October 5, 2021 5:30 PM
> To: edk2-devel-groups-io <devel@edk2.groups.io>; Chang, Abner (HPS SW/FW Technologist) <abner.chang@...>
> Cc: Ard Biesheuvel <ardb+tianocore@...>; Leif Lindholm <leif@...>; Sami Mujawar <sami.mujawar@...>; Jiewen Yao <jiewen.yao@...>; Jordan Justen <jordan.l.justen@...>; Gerd Hoffmann <kraxel@...>; Schaefer, Daniel <daniel.schaefer@...>; Sunil V L <sunilvl@...>; Liming Gao <gaoliming@...>; Zhiguang Liu <zhiguang.liu@...>; Michael D Kinney <michael.d.kinney@...>
> Subject: Re: [edk2-devel] [PATCH V3 00/12] Migrate ArmVirtPkg modules to OvmfPkg
>
>
>
> On Thu, 30 Sept 2021 at 03:43, Abner Chang <abner.chang@...> wrote:
> >
> > In V3: Address comments on V2.
> > In V2: Remove HPE license on the files that just moved around or
> >        the changes in the file are just code removal.
> >
> > edk2 BZ #: 3665
> > edk2 platform corresponding changes will be submitted after
> > this pactch set is reviewed.
> >
> > This pacthes set is to migrate some modules from ArmVirtPkg
> > to under OvmfPkg for the upcoming RiscVVirtPkg that can leverage
> > those modules without the dependency with Arm*Pkg.
> >
> > The modules moved from ArmVirtPkg to OvmfPkg are,
> > - FdtClientDxe
> > - PciPcdProducerLib
> > - HighMemDxe
> > - QemuFwCfgLib
> > - FdtPciHostBridgeLib
> > - VirtioFdtDxe
> >
> > Below PCDs are moved to under MdePkg and leverage by RiscVVirtPkg.
> > This change also remove the dependency on ArmPkg of OvmfPkg.
> > - PcdPciIoTranslation
> > - PcdPciIoTranslation
> > - PcdPciMmio32(64)Translation
> >
> > Signed-off-by: Abner Chang <abner.chang@...>
> > Cc: Ard Biesheuvel <ardb+tianocore@...>
> > Cc: Leif Lindholm <leif@...>
> > Cc: Sami Mujawar <sami.mujawar@...>
> > Cc: Jiewen Yao <jiewen.yao@...>
> > Cc: Jordan Justen <jordan.l.justen@...>
> > Cc: Gerd Hoffmann <kraxel@...>
> > Cc: Daniel Schaefer <daniel.schaefer@...>
> > Cc: Sunil V L <sunilvl@...>
> > Cc: Liming Gao <gaoliming@...>
> > Cc: Zhiguang Liu <zhiguang.liu@...>
> > Cc: Michael D Kinney <michael.d.kinney@...>
> >
> > Abner Chang (12):
> >   ArmVirtPkg/FdtClintDxe: Move FdtClientDxe to EmbeddedPkg
> >   MdePkg: Add PcdPciIoTranslation PCD
> >   ArmPkg: Use PcdPciIoTranslation PCD from MdePkg
> >   ArmVirtPkg/FdtPciPcdProducerLib: Relocate PciPcdProducerLib to OvmfPkg
> >   ArmVirtPkg/HighMemDxe: Relocate HighMemDxe to OvmfPkg
> >   OvmfPkg/HighMemDxe: Add RISC-V in the supported arch.
> >   ArmVirtPkg/QemuFwCfgLib: Relocate QemuFwCfgLib to OvmfPkg
> >   OvmfPkg/QemuFwCfgLibMMIO: Add RISC-V arch support
> >   MdePkg: Add PcdPciMmio32(64)Translation PCDs
> >   ArmVirtPkg/FdtPciHostBridgeLib: Relocate FdtPciHostBridgeLib to
> >     OvmfPkg/Fdt
> >   OvmfPkg/FdtPciHostBridgeLib: Add RISC-V in the supported arch.
> >   ArmVirtPkg/VirtioFdtDxe: Relocate VirtioFdtDxe to OvmfPkg/Fdt
> >
>
> Hello all,
>
> These patches look ok to me, but I wonder if the MdePkg maintainers
> are happy taking these PCD declaration changes. Translations for PCIe
> are typically defined per host bridge, and I would rather move away
> from using PCDs for this entirely than 'promote' them by carrying them
> in MdePkg.
>
> As this issue is somewhat orthogonal to what Abner is trying to fix,
> perhaps it is better to avoid MdePkg changes for now, and just
> duplicate these PCDs into OvmfPkg. This is reasonable, given that we
> know that QEMU only exposes a single host bridge.
>
> The one in ArmPkg can hopefully be removed and replaced with something
> that is more appropriate.
>
>
> >  ArmPkg/ArmPkg.dec                             | 15 ++++++--------
> >  ArmVirtPkg/ArmVirtPkg.dec                     |  3 ---
> >  EmbeddedPkg/EmbeddedPkg.dec                   |  1 +
> >  MdePkg/MdePkg.dec                             | 12 +++++++++++
> >  ArmVirtPkg/ArmVirtCloudHv.dsc                 | 18 ++++++++---------
> >  ArmVirtPkg/ArmVirtKvmTool.dsc                 | 18 ++++++++---------
> >  ArmVirtPkg/ArmVirtQemu.dsc                    | 20 +++++++++----------
> >  ArmVirtPkg/ArmVirtQemuKernel.dsc              | 20 +++++++++----------
> >  ArmVirtPkg/ArmVirtXen.dsc                     |  2 +-
> >  EmbeddedPkg/EmbeddedPkg.dsc                   |  1 +
> >  ArmVirtPkg/ArmVirtCloudHv.fdf                 |  6 +++---
> >  ArmVirtPkg/ArmVirtKvmTool.fdf                 |  6 +++---
> >  ArmVirtPkg/ArmVirtXen.fdf                     |  2 +-
> >  ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc          |  6 +++---
> >  .../ArmPciCpuIo2Dxe/ArmPciCpuIo2Dxe.inf       |  2 +-
> >  .../ArmVirtGicArchLib/ArmVirtGicArchLib.inf   |  1 +
> >  .../ArmVirtPL031FdtClientLib.inf              |  1 +
> >  .../ArmVirtPsciResetSystemLib.inf             |  1 +
> >  .../ArmVirtTimerFdtClientLib.inf              |  1 +
> >  .../KvmtoolRtcFdtClientLib.inf                |  1 +
> >  .../NorFlashKvmtoolLib/NorFlashKvmtoolLib.inf |  1 +
> >  .../NorFlashQemuLib/NorFlashQemuLib.inf       |  1 +
> >  .../XenAcpiPlatformDxe/XenAcpiPlatformDxe.inf |  1 +
> >  ArmVirtPkg/XenioFdtDxe/XenioFdtDxe.inf        |  1 +
> >  .../Drivers}/FdtClientDxe/FdtClientDxe.inf    |  1 -
> >  .../FdtPciHostBridgeLib.inf                   | 11 +++++-----
> >  .../FdtPciPcdProducerLib.inf                  |  5 ++---
> >  .../Fdt}/HighMemDxe/HighMemDxe.inf            |  7 ++++---
> >  .../Fdt}/VirtioFdtDxe/VirtioFdtDxe.inf        |  2 +-
> >  .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf |  6 +++---
> >  .../Include/Protocol/FdtClient.h              |  0
> >  .../Drivers}/FdtClientDxe/FdtClientDxe.c      |  0
> >  .../FdtPciHostBridgeLib/FdtPciHostBridgeLib.c |  0
> >  .../FdtPciPcdProducerLib.c                    |  0
> >  .../Fdt}/HighMemDxe/HighMemDxe.c              |  3 ++-
> >  .../Fdt}/VirtioFdtDxe/VirtioFdtDxe.c          |  0
> >  .../Library/QemuFwCfgLib/QemuFwCfgLibMmio.c   |  7 ++++---
> >  Maintainers.txt                               |  6 ++++++
> >  38 files changed, 106 insertions(+), 83 deletions(-)
> >  rename {ArmVirtPkg => EmbeddedPkg/Drivers}/FdtClientDxe/FdtClientDxe.inf (92%)
> >  rename {ArmVirtPkg/Library => OvmfPkg/Fdt}/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf (77%)
> >  rename {ArmVirtPkg/Library => OvmfPkg/Fdt}/FdtPciPcdProducerLib/FdtPciPcdProducerLib.inf (87%)
> >  rename {ArmVirtPkg => OvmfPkg/Fdt}/HighMemDxe/HighMemDxe.inf (83%)
> >  rename {ArmVirtPkg => OvmfPkg/Fdt}/VirtioFdtDxe/VirtioFdtDxe.inf (92%)
> >  rename ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.inf => OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.inf (86%)
> >  rename {ArmVirtPkg => EmbeddedPkg}/Include/Protocol/FdtClient.h (100%)
> >  rename {ArmVirtPkg => EmbeddedPkg/Drivers}/FdtClientDxe/FdtClientDxe.c (100%)
> >  rename {ArmVirtPkg/Library => OvmfPkg/Fdt}/FdtPciHostBridgeLib/FdtPciHostBridgeLib.c (100%)
> >  rename {ArmVirtPkg/Library => OvmfPkg/Fdt}/FdtPciPcdProducerLib/FdtPciPcdProducerLib.c (100%)
> >  rename {ArmVirtPkg => OvmfPkg/Fdt}/HighMemDxe/HighMemDxe.c (95%)
> >  rename {ArmVirtPkg => OvmfPkg/Fdt}/VirtioFdtDxe/VirtioFdtDxe.c (100%)
> >  rename ArmVirtPkg/Library/QemuFwCfgLib/QemuFwCfgLib.c => OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgLibMmio.c (93%)
> >
> > --
> > 2.17.1
> >
> >
> >
> >
> >
> >
>

Join devel@edk2.groups.io to automatically receive all group messages.