From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f41.google.com (mail-pj1-f41.google.com [209.85.216.41]) by mx.groups.io with SMTP id smtpd.web09.1035.1662137505154924234 for ; Fri, 02 Sep 2022 09:51:45 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20210112 header.b=WSOLpzDY; spf=pass (domain: gmail.com, ip: 209.85.216.41, mailfrom: pedro.falcato@gmail.com) Received: by mail-pj1-f41.google.com with SMTP id fa2so2586763pjb.2 for ; Fri, 02 Sep 2022 09:51:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date; bh=RYV3qBPj1llaXQXSJ8NHdKY5JM+KpG8IQ0vPwBRZHAg=; b=WSOLpzDYr1WtZ+gBlkYCOxfzVgbV3C49wEF4bbycwMT6WTlfCNC0hIrEJcMx5NTrFW Aok3seDwcODfQHzpZw6wfiIZ8xR51Dc7FErviP9rfJqlQU+Htr14tgSq2gnLv6pCS1Zq eDPbLrlF8JHLFtGJ9Mlbc5OaeXD9zR5YuKSeQtK4XF0xvyQs8fZUlk/xoF9wRuMFb3Tq oA9zpGEYNVMt4+IloWCxL/lwYOBQOzFzOZelaHhiNvYj+GnLkIZZcN2OZFKz3n9MydEX Yg4G6pT29Eex+a1CqcjkxmdPqRTxKF3ZFZd6HbmOFbIP/RAW3yyY6qkf0YocUYhbt3vA NNvA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date; bh=RYV3qBPj1llaXQXSJ8NHdKY5JM+KpG8IQ0vPwBRZHAg=; b=NZXvBCuUmkMx2GMUW3P0FR9/KVHG2irqjlmnernlsU3YJ7bHXkYqlkJkgMbpKmzBXZ IdGstHS8mjmCyoPjsg/2jw14MuVtjqFOhukyZhaUrRzuQoT+BfWa5HotuDx+XbUkv8qy 2sTb4pto7YOBPfT6LJaKuzXuqFM6XU9E9jrfOvbqt+heInLtcjfSJ0RdVEgtJac5+cLK jRa2OfpxS417QW4m6leJ7MRMdYxUaAmCVVkTC4pEjf6czuAiA2dZtNCjWcw7wrBrdHss ZbFF7XmK3mitID8tZa8TmGyyfeWGUsI0vYV/XdyZJCIPwLBXQJkgu/2yneBF2ibITmrd 1KEg== X-Gm-Message-State: ACgBeo0cm1kbrWXZEOenCmxdJMJwoXeaSoeeDJNFBNVrDOX1WbogTBTG fif0yJvBbpZYkNe5nzw0u+Z+00zdO1oRaGwBD3g= X-Google-Smtp-Source: AA6agR4wgdhT44s67r4kjneKrLA2WfMXyMmW4gfuZr9Tz4gc6cfZTaFraGMgrbB8O2wwZMIpAF18c999UumwUT241GI= X-Received: by 2002:a17:903:2406:b0:174:f1c8:76bc with SMTP id e6-20020a170903240600b00174f1c876bcmr21665077plo.168.1662137504617; Fri, 02 Sep 2022 09:51:44 -0700 (PDT) MIME-Version: 1.0 References: <20220827000201.22235-1-theojehl76@gmail.com> <20220827000201.22235-2-theojehl76@gmail.com> <20220902100245.m5mgcpvub4gmwj54@sirius.home.kraxel.org> In-Reply-To: <20220902100245.m5mgcpvub4gmwj54@sirius.home.kraxel.org> From: "Pedro Falcato" Date: Fri, 2 Sep 2022 17:51:33 +0100 Message-ID: Subject: Re: [edk2-platforms][PATCH v1 01/02] QemuOpenBoardPkg: Add QemuOpenBoardPkg To: Gerd Hoffmann Cc: =?UTF-8?B?VGjDqW8=?= , edk2-devel-groups-io , Leif Lindholm , Michael D Kinney , Isaac Oram , Stefan Hajnoczi Content-Type: multipart/alternative; boundary="00000000000087d94f05e7b48bfe" --00000000000087d94f05e7b48bfe Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, Sep 2, 2022 at 11:02 AM Gerd Hoffmann wrote: > On Sat, Aug 27, 2022 at 02:02:00AM +0200, Th=C3=A9o wrote: > > From: Th=C3=A9o Jehl > > > > QemuOpenBoardPkg adds a MinPlatform port to Qemu x86_64 > > It can boots UEFI Linux and Windows, and works on PIIX4 and Q35 > > This board port provides a simple starting place for investigating edk2 > and > > MinPlatform Arch. > > Currently we implement up to stage 4 of the MinPlatform spec and can bo= ot > > Windows/Linux. > > That is a rather short description for a patch of this size. It > probably makes sense to break that down into smaller pieces and make a > patch series out of it because you can describe the specific pieces much > better then. > > I'm not familiar with MinPlatform, just skimmed the manual. Can't > comment much on those details. > > It seems the goal is to have a MinPlatform board which can easily be > used to learn about and experiment with MinPlatform, is that correct? > Hi Gerd, Yes, for now the goal is to have a MinPlatform board that is simple and easy to use as a learning tool. However, in my opinion the future remains unclear. I think we (as in EDK2) could benefit by moving OVMF itself to MinPlatform, so some merging of QemuOpenBoardPkg and OvmfPkg (in the future, of course) would be a cool idea. I think OVMF has grown a bit hairy in a few places and has a bunch of logic that is already covered in MinPlatform. I believe that we got something close enough to OVMF (of course, while using some OvmfPkg modules and libraries) but drastically simpler. I don't know if you share the same opinion; some feedback from you OvmfPkg folks would be interesting. At the very least, it will serve as a learning tool - it's a great example of "How to create your own firmware", in a relatively straightforward and blobless way. Maybe more valuable as a learning tool than an OVMF replacement, but that of course is open for discussion. We've limited ourselves to targeting "simple learning MinPlatform (and firmware programming in general) tool" as writing a full replacement of OVMF would be very much impossible in 3 months, as you're aware :) > > > > Platform/Qemu/QemuOpenBoardPkg/Library/OpenQemuFwCfgLib/OpenQemuFwCfgLib.= inf > | 23 + > > Platform/Qemu/QemuOpenBoardPkg/Include/Library/OpenQemuFwCfgLib.h > | 102 +++ > > > Platform/Qemu/QemuOpenBoardPkg/Library/OpenQemuFwCfgLib/OpenQemuFwCfgLib.= c > | 130 ++++ > > Why duplicate that lib instead of just using the OvmfPkg version (which > you do elsewhere)? > > > Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Cpu.c > | 56 ++ > > Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c > | 244 ++++++++ > > Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Pci.c > | 59 ++ > > Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Pcie.c > | 91 +++ > > Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.c > | 67 ++ > > Note that OvmfPkg got a PlatformInitLib recently which you might be able > to use to reduce code duplication (didn't check the code though and > maybe MinPlatform init is different enough that this doesn't help much). > > Some context on these two libs: OpenQemuFwCfgLib was created early on as a way to manage initial complexity and as a way to get Th=C3=A9o to code a bit. I'm not too sure we'll want to= keep that library around. PlatformInitPei was again created to manage complexity (and to implement MinPlatform of course) - this ended up getting all the PEI platform initialization code, instead of using OVMF's PlatformInitLib. You'll notice that there's some overlap in functionality between QemuOBP and OVMF. While it's not the most elegant solution from a "UEFI super-duper-modular" POV, I think it was a better end product than getting Th=C3=A9o to look at .dsc and .fdf files for 3 months; we actually went thr= ough x86 PC platform initialization and I'm reasonably confident Th=C3=A9o not o= nly understands how x86 PC works, but that he could write it all on his own again. We also got something with less historical baggage and that resulted in cleaner, simpler code. It also doesn't support every bell and whistle in QEMU and I'm willing to bet it straight up breaks if you go back enough in QEMU versions; but again, at the current stage it's not a replacement for full OVMF functionality, but rather the most common subset of it (no TDX, no SEV, no CPU hotplug, restricts itself to relatively recent versions, SMM and secure boot are not in this patch set but are in progress). Thanks, Pedro --00000000000087d94f05e7b48bfe Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
On Fri, Sep 2, 2022 at 11:02 AM Gerd Hoff= mann <kraxel@redh= at.com> wrote:
On Sat, Aug 27, 2022 at 02:02:00AM +0200, = Th=C3=A9o wrote:
> From: Th=C3=A9o Jehl <theojehl76@gmail.com>
>
> QemuOpenBoardPkg adds a MinPlatform port to Qemu x86_64
> It can boots UEFI Linux and Windows, and works on PIIX4 and Q35
> This board port provides a simple starting place for investigating edk= 2 and
> MinPlatform Arch.
> Currently we implement up to stage 4 of the MinPlatform spec and can b= oot
> Windows/Linux.

That is a rather short description for a patch of this size.=C2=A0 It
probably makes sense to break that down into smaller pieces and make a
patch series out of it because you can describe the specific pieces much better then.

I'm not familiar with MinPlatform, just skimmed the manual.=C2=A0 Can&#= 39;t
comment much on those details.

It seems the goal is to have a MinPlatform board which can easily be
used to learn about and experiment with MinPlatform, is that correct?
Hi Gerd,

Yes, for now the goal is = to have a MinPlatform board that is simple and easy to use as a learning to= ol. However, in my opinion the future remains unclear. I think we (as in ED= K2) could benefit by moving OVMF itself to MinPlatform, so some merging of = QemuOpenBoardPkg and OvmfPkg (in the future, of course) would be a cool ide= a. I think OVMF has grown a bit hairy in a few places and has a bunch of lo= gic that is already covered in MinPlatform. I believe that we got something= close enough to OVMF (of course, while using some OvmfPkg modules and libr= aries) but drastically simpler. I don't know if you share the same opin= ion; some feedback from you OvmfPkg folks would be interesting.
<= br>
At the very least, it will serve as a learning tool - it'= s a great example of "How to create your own firmware", in a rela= tively straightforward and blobless way. Maybe more valuable as a learning = tool than an OVMF replacement, but that of course is open for discussion. W= e've limited ourselves to targeting "simple learning MinPlatform (= and firmware programming in general) tool" as writing a full replaceme= nt of OVMF would be very much impossible in 3 months, as you're aware := )

>=C2=A0 Platform/Qemu/QemuOpenBoardPkg/Library/OpenQemuFwCfgLib/OpenQemu= FwCfgLib.inf=C2=A0 =C2=A0 =C2=A0 =C2=A0|=C2=A0 23 +
>=C2=A0 Platform/Qemu/QemuOpenBoardPkg/Include/Library/OpenQemuFwCfgLib.= h=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 | 102 +++ >=C2=A0 Platform/Qemu/QemuOpenBoardPkg/Library/OpenQemuFwCfgLib/OpenQemu= FwCfgLib.c=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0| 130 ++++

Why duplicate that lib instead of just using the OvmfPkg version (which
you do elsewhere)?

>=C2=A0 Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Cpu.c=C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0|=C2=A0 56 ++
>=C2=A0 Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Memory.c=C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 | 244 ++++++++
>=C2=A0 Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Pci.c=C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0|=C2=A0 59 ++
>=C2=A0 Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/Pcie.c=C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 = =C2=A0 =C2=A0 =C2=A0 |=C2=A0 91 +++
>=C2=A0 Platform/Qemu/QemuOpenBoardPkg/PlatformInitPei/PlatformInit.c=C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 |= =C2=A0 67 ++

Note that OvmfPkg got a PlatformInitLib recently which you might be able to use to reduce code duplication (didn't check the code though and
maybe MinPlatform init is different enough that this doesn't help much)= .

Some context on these two libs:
OpenQemuFwC= fgLib was created early on as a way to manage initial complexity and as a w= ay to get Th=C3=A9o to code a bit. I'm not too sure we'll want to k= eep that library around.
PlatformInitPei was again created to man= age complexity (and to implement MinPlatform of course) - this ended up get= ting all the PEI platform initialization code, instead of using OVMF's = PlatformInitLib.

You'll notice that there'= s some overlap in functionality between QemuOBP and OVMF. While it's no= t the most elegant solution from a "UEFI super-duper-modular" POV= , I think it was a better end product than getting Th=C3=A9o to look at .ds= c and .fdf files for 3 months; we actually went through x86 PC platform ini= tialization and I'm reasonably confident Th=C3=A9o not only understands= how x86 PC works, but that he could write it all on his own again. We also= got something with less historical baggage and that resulted in cleaner, s= impler code. It also doesn't support every bell and whistle in QEMU and= I'm willing to bet it straight up breaks if you go back enough in QEMU= versions; but again, at the current stage it's not a replacement for f= ull OVMF functionality, but rather the most common subset of it (no TDX, no= SEV, no CPU hotplug, restricts itself to relatively recent versions, SMM a= nd secure boot are not in this patch set but are in progress).
Thanks,
Pedro
--00000000000087d94f05e7b48bfe--