Re: [PATCH edk2-platforms 1/2] QemuOpenBoardPkg: Redo PCI bus initialization

["Pedro Falcato" <pedro.falcato@gmail.com>]

On Fri, Jan 13, 2023 at 6:34 AM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> > diff --git a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c
> > index 21705256191b..4f312c36016e 100644
> > --- a/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c
> > +++ b/Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c
>
> There is OvmfPkg/Library/PlatformInitLib which you could use instead of
> reinventing the wheel ...
>
> If there are changes needed to make PlatformInitLib work for you feel
> free to propose patches (same goes for eventually moving code from
> OvmfPkg/PlatformPei to the Library).

Gerd,

Thank you for the comments.

That is an interesting idea, however I believe
OvmfPkg/Library/PlatformInitLib is ATM a bit too tightly coupled with
the rest of OVMF itself.
For instance, this work was partially inspired by your recent-ish work
in similar OvmfPkg code and also drove me to remove the dependency on
PciHostBridgeLib, et al (replacing it with the "standard"
MinPlatformPkg PCI libs). My solution is based on dynamic PCD
war-crimes but it Just Works(tm).
Changing it in OvmfPkg would be too invasive, I feel; it's not like my
solution is good or anything.

And because of the way the repos are split, there is no way to
currently dedup OVMF's PciHostBridgeLib by also making OVMF depend on
MinPlatformPkg's, because why would there be, right?

Anyway, doing deep changes to OVMF for QemuOpenBoardPkg is
uncomfortable for me because its place in Tiano is not defined. OVMF
and Intel folks have said very few things about QOBP.
At the moment, it's the typical "GSoC project stagnating in
edk2-platforms/-staging" that Tianocore seems to love so much. I don't
see much point in it being this way.
QemuOBP has been a nice fun exercise in reducing code duplication,
which shows a lot of promise, but it also doesn't support some of the
things OvmfPkg does, like PEI-less booting (and TDX/SEV but those
could certainly be worked in).

If this kicks off discussion on the future of QemuOpenBoardPkg, I'll be happy.

Until that happens, I am reluctant to change OVMF internals for an
obscure QEMU PlatformPkg hidden in the best worst repo, edk2-platforms
:)

> > +  // It's worth noting that QEMU also grew an option to change lowmem based on the
> > +  // user's preferences. Because of all of this, it's near impossible to hardcode
> > +  // a range, so we grab TOLUD (not in a literal way, since QEMU does not implement
> > +  // that register ;)) and calculate our PCI MMIO based on that. This also makes it so
> > +  // the DSDT built by QEMU will have correct _CRS ranges.
> > +  // hw/pci-host/q35.c explicitly says our PCI hole ranges from [TOLUD, IO APIC].
> > +  // As far as I can tell, we seem to be allowed to add a 64-bit resource range anywhere.
>
> There is a etc/reserved-memory-end fw_cfg FwCfg file.  If present it
> the 64-bit resource range should be placed above the address specified
> there (qemu uses that to reserve address space if needed, happens for
> example when you enable memory hotplug).

ACK, will change on v2 (together with Isaac's comments).

> The patch should be splitted up into smaller pieces to make it easier
> to review the changes.

Agreed, will try to devise a strategy to break it up in smaller, logical chunks.
Again, thank you so much for your time looking at something you're not
even a maintainer for :))

-- 
Pedro