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

["Ard Biesheuvel" <ardb@kernel.org>]

On Thu, 14 Oct 2021 at 12:14, Chang, Abner (HPS SW/FW Technologist)
<abner.chang@hpe.com> wrote:
>
> 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?
>

Don't worry about it.


> ________________________________
> From: Ard Biesheuvel <ardb@kernel.org>
> 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@hpe.com>
> Cc: gaoliming <gaoliming@byosoft.com.cn>; Ard Biesheuvel <ardb+tianocore@kernel.org>; Leif Lindholm <leif@nuviainc.com>; Sami Mujawar <sami.mujawar@arm.com>; Jiewen Yao <jiewen.yao@intel.com>; Jordan Justen <jordan.l.justen@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Schaefer, Daniel <daniel.schaefer@hpe.com>; Sunil V L <sunilvl@ventanamicro.com>; Zhiguang Liu <zhiguang.liu@intel.com>; Michael D Kinney <michael.d.kinney@intel.com>
> 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@hpe.com> 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@kernel.org>
>
> Feel free to merge it whenever convenient for you.
>
>
> >
> > From: gaoliming [mailto:gaoliming@byosoft.com.cn]
> > Sent: Monday, October 11, 2021 9:22 AM
> > To: devel@edk2.groups.io; Chang, Abner (HPS SW/FW Technologist) <abner.chang@hpe.com>; ardb@kernel.org
> > Cc: 'Ard Biesheuvel' <ardb+tianocore@kernel.org>; 'Leif Lindholm' <leif@nuviainc.com>; 'Sami Mujawar' <sami.mujawar@arm.com>; 'Jiewen Yao' <jiewen.yao@intel.com>; 'Jordan Justen' <jordan.l.justen@intel.com>; 'Gerd Hoffmann' <kraxel@redhat.com>; Schaefer, Daniel <daniel.schaefer@hpe.com>; 'Sunil V L' <sunilvl@ventanamicro.com>; 'Zhiguang Liu' <zhiguang.liu@intel.com>; 'Michael D Kinney' <michael.d.kinney@intel.com>
> > 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@byosoft.com.cn>
> >
> >
> >
> > 发件人: devel@edk2.groups.io <devel@edk2.groups.io> 代表 Abner Chang
> > 发送时间: 2021年10月8日 11:39
> > 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn; ardb@kernel.org
> > 抄送: 'Ard Biesheuvel' <ardb+tianocore@kernel.org>; 'Leif Lindholm' <leif@nuviainc.com>; 'Sami Mujawar' <sami.mujawar@arm.com>; 'Jiewen Yao' <jiewen.yao@intel.com>; 'Jordan Justen' <jordan.l.justen@intel.com>; 'Gerd Hoffmann' <kraxel@redhat.com>; Schaefer, Daniel <daniel.schaefer@hpe.com>; 'Sunil V L' <sunilvl@ventanamicro.com>; 'Zhiguang Liu' <zhiguang.liu@intel.com>; 'Michael D Kinney' <michael.d.kinney@intel.com>
> > 主题: 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@hpe.com>; 'edk2-devel-groups-io' <devel@edk2.groups.io>; ardb@kernel.org
> > Cc: 'Ard Biesheuvel' <ardb+tianocore@kernel.org>; 'Leif Lindholm' <leif@nuviainc.com>; 'Sami Mujawar' <sami.mujawar@arm.com>; 'Jiewen Yao' <jiewen.yao@intel.com>; 'Jordan Justen' <jordan.l.justen@intel.com>; 'Gerd Hoffmann' <kraxel@redhat.com>; Schaefer, Daniel <daniel.schaefer@hpe.com>; 'Sunil V L' <sunilvl@ventanamicro.com>; 'Zhiguang Liu' <zhiguang.liu@intel.com>; 'Michael D Kinney' <michael.d.kinney@intel.com>
> > 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@hpe.com>
> > 发送时间: 2021年10月6日 17:27
> > 收件人: edk2-devel-groups-io <devel@edk2.groups.io>; ardb@kernel.org; Chang, Abner (HPS SW/FW Technologist) <abner.chang@hpe.com>
> > 抄送: Ard Biesheuvel <ardb+tianocore@kernel.org>; Leif Lindholm <leif@nuviainc.com>; Sami Mujawar <sami.mujawar@arm.com>; Jiewen Yao <jiewen.yao@intel.com>; Jordan Justen <jordan.l.justen@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Schaefer, Daniel <daniel.schaefer@hpe.com>; Sunil V L <sunilvl@ventanamicro.com>; Liming Gao <gaoliming@byosoft.com.cn>; Zhiguang Liu <zhiguang.liu@intel.com>; Michael D Kinney <michael.d.kinney@intel.com>
> > 主题: 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@hpe.com>
> > Sent: Tuesday, October 5, 2021 11:00 PM
> > To: edk2-devel-groups-io <devel@edk2.groups.io>; ardb@kernel.org <ardb@kernel.org>
> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Leif Lindholm <leif@nuviainc.com>; Sami Mujawar <sami.mujawar@arm.com>; Jiewen Yao <jiewen.yao@intel.com>; Jordan Justen <jordan.l.justen@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Schaefer, Daniel <daniel.schaefer@hpe.com>; Sunil V L <sunilvl@ventanamicro.com>; Liming Gao <gaoliming@byosoft.com.cn>; Zhiguang Liu <zhiguang.liu@intel.com>; Michael D Kinney <michael.d.kinney@intel.com>
> > 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@kernel.org>
> > 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@hpe.com>
> > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>; Leif Lindholm <leif@nuviainc.com>; Sami Mujawar <sami.mujawar@arm.com>; Jiewen Yao <jiewen.yao@intel.com>; Jordan Justen <jordan.l.justen@intel.com>; Gerd Hoffmann <kraxel@redhat.com>; Schaefer, Daniel <daniel.schaefer@hpe.com>; Sunil V L <sunilvl@ventanamicro.com>; Liming Gao <gaoliming@byosoft.com.cn>; Zhiguang Liu <zhiguang.liu@intel.com>; Michael D Kinney <michael.d.kinney@intel.com>
> > 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@hpe.com> 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@hpe.com>
> > > Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> > > Cc: Leif Lindholm <leif@nuviainc.com>
> > > Cc: Sami Mujawar <sami.mujawar@arm.com>
> > > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > > Cc: Gerd Hoffmann <kraxel@redhat.com>
> > > Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
> > > Cc: Sunil V L <sunilvl@ventanamicro.com>
> > > Cc: Liming Gao <gaoliming@byosoft.com.cn>
> > > Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> > > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > >
> > > 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
> > >
> > >
> > >
> > >
> > >
> > >
> >
> >