public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "gaoliming" <gaoliming@byosoft.com.cn>
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
Date: Fri, 8 Oct 2021 11:14:06 +0800	[thread overview]
Message-ID: <008d01d7bbf2$8de53ed0$a9afbc70$@byosoft.com.cn> (raw)
In-Reply-To: <CS1PR8401MB1144D54B9296DB8DA7000E0BFFB09@CS1PR8401MB1144.NAMPRD84.PROD.OUTLOOK.COM>

[-- Attachment #1: Type: text/plain, Size: 11487 bytes --]

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 <mailto:devel@edk2.groups.io>
<devel@edk2.groups.io <mailto:devel@edk2.groups.io> > on behalf of Abner
Chang <abner.chang@hpe.com <mailto:abner.chang@hpe.com> >
Sent: Tuesday, October 5, 2021 11:00 PM
To: edk2-devel-groups-io <devel@edk2.groups.io <mailto:devel@edk2.groups.io>
>; ardb@kernel.org <mailto:ardb@kernel.org>  <ardb@kernel.org
<mailto:ardb@kernel.org> >
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org <mailto:ardb+tianocore@kernel.
org> >; Leif Lindholm <leif@nuviainc.com <mailto:leif@nuviainc.com> >; Sami
Mujawar <sami.mujawar@arm.com <mailto:sami.mujawar@arm.com> >; Jiewen Yao
<jiewen.yao@intel.com <mailto:jiewen.yao@intel.com> >; Jordan Justen
<jordan.l.justen@intel.com <mailto:jordan.l.justen@intel.com> >; Gerd
Hoffmann <kraxel@redhat.com <mailto:kraxel@redhat.com> >; Schaefer, Daniel
<daniel.schaefer@hpe.com <mailto:daniel.schaefer@hpe.com> >; Sunil V L
<sunilvl@ventanamicro.com <mailto:sunilvl@ventanamicro.com> >; Liming Gao
<gaoliming@byosoft.com.cn <mailto:gaoliming@byosoft.com.cn> >; Zhiguang Liu
<zhiguang.liu@intel.com <mailto:zhiguang.liu@intel.com> >; Michael D Kinney
<michael.d.kinney@intel.com <mailto: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 <mailto:devel@edk2.groups.io>
<devel@edk2.groups.io <mailto:devel@edk2.groups.io> > on behalf of Ard
Biesheuvel <ardb@kernel.org <mailto:ardb@kernel.org> >
Sent: Tuesday, October 5, 2021 5:30 PM
To: edk2-devel-groups-io <devel@edk2.groups.io <mailto:devel@edk2.groups.io>
>; Chang, Abner (HPS SW/FW Technologist) <abner.chang@hpe.com <mailto:abner.
chang@hpe.com> >
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org <mailto:ardb+tianocore@kernel.
org> >; Leif Lindholm <leif@nuviainc.com <mailto:leif@nuviainc.com> >; Sami
Mujawar <sami.mujawar@arm.com <mailto:sami.mujawar@arm.com> >; Jiewen Yao
<jiewen.yao@intel.com <mailto:jiewen.yao@intel.com> >; Jordan Justen
<jordan.l.justen@intel.com <mailto:jordan.l.justen@intel.com> >; Gerd
Hoffmann <kraxel@redhat.com <mailto:kraxel@redhat.com> >; Schaefer, Daniel
<daniel.schaefer@hpe.com <mailto:daniel.schaefer@hpe.com> >; Sunil V L
<sunilvl@ventanamicro.com <mailto:sunilvl@ventanamicro.com> >; Liming Gao
<gaoliming@byosoft.com.cn <mailto:gaoliming@byosoft.com.cn> >; Zhiguang Liu
<zhiguang.liu@intel.com <mailto:zhiguang.liu@intel.com> >; Michael D Kinney
<michael.d.kinney@intel.com <mailto: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
<mailto: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
<mailto:abner.chang@hpe.com> >
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org
<mailto:ardb+tianocore@kernel.org> >
> Cc: Leif Lindholm <leif@nuviainc.com <mailto:leif@nuviainc.com> >
> Cc: Sami Mujawar <sami.mujawar@arm.com <mailto:sami.mujawar@arm.com> >
> Cc: Jiewen Yao <jiewen.yao@intel.com <mailto:jiewen.yao@intel.com> >
> Cc: Jordan Justen <jordan.l.justen@intel.com
<mailto:jordan.l.justen@intel.com> >
> Cc: Gerd Hoffmann <kraxel@redhat.com <mailto:kraxel@redhat.com> >
> Cc: Daniel Schaefer <daniel.schaefer@hpe.com
<mailto:daniel.schaefer@hpe.com> >
> Cc: Sunil V L <sunilvl@ventanamicro.com <mailto:sunilvl@ventanamicro.com>
>
> Cc: Liming Gao <gaoliming@byosoft.com.cn <mailto:gaoliming@byosoft.com.cn>
>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com <mailto:zhiguang.liu@intel.com> >
> Cc: Michael D Kinney <michael.d.kinney@intel.com
<mailto: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
>
>
>
> 
>
>









[-- Attachment #2: Type: text/html, Size: 22429 bytes --]

  reply	other threads:[~2021-10-08  3:14 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-30  0:45 [PATCH V3 00/12] Migrate ArmVirtPkg modules to OvmfPkg Abner Chang
2021-09-30  0:45 ` [PATCH V3 01/12] ArmVirtPkg/FdtClintDxe: Move FdtClientDxe to EmbeddedPkg Abner Chang
2021-09-30  0:45 ` [PATCH V3 02/12] MdePkg: Add PcdPciIoTranslation PCD Abner Chang
2021-09-30  0:45 ` [PATCH V3 03/12] ArmPkg: Use PcdPciIoTranslation PCD from MdePkg Abner Chang
2021-09-30  0:45 ` [PATCH V3 04/12] ArmVirtPkg/FdtPciPcdProducerLib: Relocate PciPcdProducerLib to OvmfPkg Abner Chang
2021-09-30  0:45 ` [PATCH V3 05/12] ArmVirtPkg/HighMemDxe: Relocate HighMemDxe " Abner Chang
2021-09-30  0:45 ` [PATCH V3 06/12] OvmfPkg/HighMemDxe: Add RISC-V in the supported arch Abner Chang
2021-09-30  0:45 ` [PATCH V3 07/12] ArmVirtPkg/QemuFwCfgLib: Relocate QemuFwCfgLib to OvmfPkg Abner Chang
2021-09-30  0:45 ` [PATCH V3 08/12] OvmfPkg/QemuFwCfgLibMMIO: Add RISC-V arch support Abner Chang
2021-09-30  0:45 ` [PATCH V3 09/12] MdePkg: Add PcdPciMmio32(64)Translation PCDs Abner Chang
2021-09-30  0:45 ` [PATCH V3 10/12] ArmVirtPkg/FdtPciHostBridgeLib: Relocate FdtPciHostBridgeLib to OvmfPkg/Fdt Abner Chang
2021-09-30  0:45 ` [PATCH V3 11/12] OvmfPkg/FdtPciHostBridgeLib: Add RISC-V in the supported arch Abner Chang
2021-09-30  0:45 ` [PATCH V3 12/12] ArmVirtPkg/VirtioFdtDxe: Relocate VirtioFdtDxe to OvmfPkg/Fdt Abner Chang
2021-09-30  2:39 ` [edk2-devel] [PATCH V3 00/12] Migrate ArmVirtPkg modules to OvmfPkg Yao, Jiewen
2021-09-30  7:12 ` Gerd Hoffmann
2021-10-03  8:11 ` Daniel Schaefer
2021-10-04 10:05 ` Sunil V L
2021-10-05  9:30 ` [edk2-devel] " Ard Biesheuvel
2021-10-05 15:00   ` Abner Chang
     [not found]   ` <16AB2A90260D3DD8.20282@groups.io>
2021-10-06  9:27     ` Abner Chang
2021-10-08  3:14       ` gaoliming [this message]
2021-10-08  3:39         ` Abner Chang
2021-10-11  1:22           ` 回复: " gaoliming
2021-10-12  4:16             ` Abner Chang
2021-10-14  9:51               ` Ard Biesheuvel
2021-10-14 10:13                 ` Abner Chang
2021-10-14 10:57                   ` Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='008d01d7bbf2$8de53ed0$a9afbc70$@byosoft.com.cn' \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox