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: [email protected] <[email protected]> on behalf of Abner Chang <[email protected]> Sent: Tuesday, October 5, 2021 11:00 PM To: edk2-devel-groups-io <[email protected]>; [email protected] <[email protected]> Cc: Ard Biesheuvel <[email protected]>; Leif Lindholm <[email protected]>; Sami Mujawar <[email protected]>; Jiewen Yao <[email protected]>; Jordan Justen <[email protected]>; Gerd Hoffmann <[email protected]>; Schaefer, Daniel <[email protected]>; Sunil V L <[email protected]>; Liming Gao <[email protected]>; Zhiguang Liu <[email protected]>; Michael D Kinney <[email protected]> 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: [email protected] <[email protected]> on behalf of Ard Biesheuvel <[email protected]> Sent: Tuesday, October 5, 2021 5:30 PM To: edk2-devel-groups-io <[email protected]>; Chang, Abner (HPS SW/FW Technologist) <[email protected]> Cc: Ard Biesheuvel <[email protected]>; Leif Lindholm <[email protected]>; Sami Mujawar <[email protected]>; Jiewen Yao <[email protected]>; Jordan Justen <[email protected]>; Gerd Hoffmann <[email protected]>; Schaefer, Daniel <[email protected]>; Sunil V L <[email protected]>; Liming Gao <[email protected]>; Zhiguang Liu <[email protected]>; Michael D Kinney <[email protected]> Subject: Re: [edk2-devel] [PATCH V3 00/12] Migrate ArmVirtPkg modules to OvmfPkg On Thu, 30 Sept 2021 at 03:43, Abner Chang <[email protected]> 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 <[email protected]> > Cc: Ard Biesheuvel <[email protected]> > Cc: Leif Lindholm <[email protected]> > Cc: Sami Mujawar <[email protected]> > Cc: Jiewen Yao <[email protected]> > Cc: Jordan Justen <[email protected]> > Cc: Gerd Hoffmann <[email protected]> > Cc: Daniel Schaefer <[email protected]> > Cc: Sunil V L <[email protected]> > Cc: Liming Gao <[email protected]> > Cc: Zhiguang Liu <[email protected]> > Cc: Michael D Kinney <[email protected]> > > 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 > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#81533): https://edk2.groups.io/g/devel/message/81533 Mute This Topic: https://groups.io/mt/85964669/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
