From: "Laszlo Ersek" <lersek@redhat.com>
To: Anthony PERARD <anthony.perard@citrix.com>, edk2-devel@lists.01.org
Cc: Julien Grall <julien.grall@arm.com>,
Ard Biesheuvel <ard.biesheuvel@linaro.org>,
Jordan Justen <jordan.l.justen@intel.com>,
xen-devel@lists.xenproject.org,
Leif Lindholm <leif.lindholm@linaro.org>,
edk2-devel-groups-io <devel@edk2.groups.io>
Subject: Re: [PATCH v2 00/31] Specific platform to run OVMF in Xen PVH and HVM guests
Date: Mon, 8 Apr 2019 17:50:57 +0200 [thread overview]
Message-ID: <6c00d5f5-e187-5fe2-8b3e-301ec51efb6b@redhat.com> (raw)
In-Reply-To: <20190408142408.30419-1-anthony.perard@citrix.com>
On 04/08/19 16:23, Anthony PERARD wrote:
> Patch series available in this git branch:
> https://xenbits.xen.org/git-http/people/aperard/ovmf.git br.platform-xen-pvh-v2
>
> Hi,
>
> I've started to create a Xen specific platform, in OvmfPkg/XenOvmf.dsc
> with the goal to make it work on both Xen HVM and Xen PVH.
The previous version of this feature dates back to Dec 2016 / Jan 2017;
I haven't forgotten about it, and I'm happy that you are adding the
feature as a separate platform DSC, as I requested.
> The first few patches only create the platform and duplicate some code
> from OvmfPkg and the later patches makes OVMF boot in a Xen PVH guest
> and can boot a Linux guest.
>
> After this patch series, I'd like to wait a bit before removing Xen
> support from the OvmfPkg*.dsc, to allow time to switch to the new Xen
> only platform, maybe 1 year.
I welcome this proposal very much. This will eliminate a significant
amount of complexity in the current modules, allow for
easier-to-understand maintenance assignments/responsibilities, and pave
the way for features that are unique to either Xen or QEMU (in
particular in the variable storage area, for which Xen has just received
a dedicated userspace daemon IIRC, and for which -- on QEMU -- I had
attempted to make pflash a conditional hard requirement, in the past,
but failed due to OvmfPkg*.dsc targeting Xen too).
> Question:
>
> Should we start moving these to a different *Pkg? Like it's done for
> ArmPkg and ArmVirtPkg? Maybe XenPkg.
I'm pretty happy with the current package structure. ArmPkg is for both
physical and virtual hardware. ArmVirtPkg is virt-only, and we already
have separate platform DSCs/FDFs between Xen (ArmVirtXen) and QEMU/KVM
(ArmVirtQemu[Kernel]). Xen- and QEMU/KVM-specific drivers and library
instances peacefully coexist under ArmVirtPkg; the DSCs/FDFs include
them as appropriate.
The same should map nicely to OvmfPkg. x86 stuff that targets both
physical and virtual hardware belongs under PcAtChipsetPkg and
UefiCpuPkg. Virt-only stuff should go under OvmfPkg; Xen-specific and
QEMU/KVM-specific modules can coexist under OvmfPkg. It's sufficient if
the platform DSCs cherry-pick them as appropriate.
And, if there are Xen-specific (but not arch-specific) PEIMs / drivers /
libraries that work alike on ARM and x86, they should go under OvmfPkg,
as ArmVirtPkg can (and already does) pull Xen-only modules from OvmfPkg.
See for example, in ArmVirtXen.dsc:
SerialPortLib|OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.inf
XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf
OvmfPkg/XenBusDxe/XenBusDxe.inf
OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
> To build and boot:
>
> To build, simply run OvmfPkg/build.sh -p OvmfPkg/XenOvmf.dsc
(1) To stick with the ArmVirt pattern, we should initially call this
platform OvmfXen.
(2) And, once you remove the Xen bits from the current
OvmfPkg*.dsc files, we should likely rename those to OvmfQemu*dsc.
I'll try to process this series in a timely manner. My main focus will
be possible regressions on QEMU/KVM, and formalities (license blocks,
signoffs, edk2 coding style, build issues that I might notice in
review).
I can't provide testing feedback, but xen-devel subscribers (and any
other Xen users too, obviously) are super welcome to test & report
results!
(3) Please file a BZ at <https://bugzilla.tianocore.org/> about this
feature, and assign it to yourself. The BZ should keep track of all
versions of the patch series (from the mailing list archive).
(4) Ideally, the release notes for the edk2 stable release in which the
feature will appear, should mention the feature (via linking the BZ):
<https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning#proposed-features>
(5) The BZ should be referenced in all the commit messages.
(6) The new edk2 development mailing list is at:
https://edk2.groups.io/g/devel
Please subscribe there, and resend this series to that address, i.e.
<devel@edk2.groups.io>. I'm CC'ing the new address myself, for this
initial response, but I'd prefer the rest of my comments to go only to
the new list (without manually updating the CC list on every response).
Thanks!
Laszlo
> Then use OVMF.fd as a kernel of a pvh guest config file (with
> xl/libxl).
>
> Patch series available in this git branch:
> https://xenbits.xen.org/git-http/people/aperard/ovmf.git br.platform-xen-pvh-v2
>
> Anthony PERARD (31):
> OvmfPkg/ResetSystemLib: Add missing dependency on PciLib
> OvmfPkg: Create platform XenOvmf
> OvmfPkg: Introduce XenResetVector
> OvmfPkg: Introduce XenPlatformPei
> OvmfPkg/XenOvmf: Creating an ELF header
> OvmfPkg/XenResetVector: Add new entry point for Xen PVH
> OvmfPkg/XenResetVector: Saving start of day pointer for PVH guests
> OvmfPkg/XenResetVector: Allow to jumpstart from either hvmloader or
> PVH
> OvmfPkg/XenOvmf: use a TimerLib instance that depends only on the CPU
> OvmfPkg/XenPlatformPei: Detect OVMF_INFO from hvmloader
> OvmfPkg/XenPlatformPei: Use mXenHvmloaderInfo to get E820
> OvmfPkg/XenPlatformPei: Grab RSDP from PVH guest start of day struct
> OvmfPkg/Library/XenPlatformLib: New library
> OvmfPkg/AcpiPlatformDxe: Use PVH RSDP if exist
> OvmfPkg/XenHypercallLib: Enable it in PEIM
> OvmfPkg/XenPlatformPei: Introduce XenHvmloaderDetected
> OvmfPkg/XenPlatformPei: Reserve hvmloader's memory only when it as
> runned
> OvmfPkg/XenPlatformPei: Setup HyperPages earlier
> OvmfPkg/XenPlatformPei: Introduce XenPvhDetected
> OvmfPkg: Import XENMEM_memory_map hypercall to Xen/memory.h
> OvmfPkg/XenPlatformPei: Get E820 table via hypercall ...
> OvmfPkg/XenPlatformPei: Rework memory detection
> OvmfPkg/XenPlatformPei: Reserve VGA memory region, to boot Linux
> OvmfPkg/XenPlatformPei: Ignore missing PCI Host Bridge on Xen PVH
> OvmfPkg/PlatformBootManagerLib: Handle the absence of PCI bus on Xen
> PVH
> OvmfPkg/XenOvmf: Override PcdFSBClock to Xen vLAPIC timer frequency
> OvmfPkg/XenOvmf: Introduce XenTimerDxe
> OvmfPkg/PlatformBootManagerLib: Use a Xen console for ConOut/ConIn
> OvmfPkg: Introduce XenIoPvhDxe to initialize Grant Tables
> OvmfPkg: Move XenRealTimeClockLib from ArmVirtPkg
> OvmfPkg/XenOvmf: use RealTimeClockRuntimeDxe from EmbeddedPkg
>
> OvmfPkg/OvmfPkg.dec | 4 +
> ArmVirtPkg/ArmVirtXen.dsc | 2 +-
> OvmfPkg/OvmfPkgIa32.dsc | 1 +
> OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
> OvmfPkg/OvmfPkgX64.dsc | 1 +
> OvmfPkg/{OvmfPkgX64.dsc => XenOvmf.dsc} | 236 ++------
> OvmfPkg/XenOvmf.fdf | 561 ++++++++++++++++++++
> OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf | 2 +-
> OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 4 +
> OvmfPkg/Library/ResetSystemLib/ResetSystemLib.inf | 1 +
> OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf | 2 +-
> ArmVirtPkg/Library/XenRealTimeClockLib/XenRealTimeClockLib.inf => OvmfPkg/Library/XenPlatformLib/XenPlatformLib.inf | 30 +-
> {ArmVirtPkg => OvmfPkg}/Library/XenRealTimeClockLib/XenRealTimeClockLib.inf | 0
> OvmfPkg/XenIoPvhDxe/XenIoPvhDxe.inf | 35 ++
> OvmfPkg/XenPlatformPei/XenPlatformPei.inf | 107 ++++
> OvmfPkg/XenResetVector/XenResetVector.inf | 46 ++
> OvmfPkg/XenTimerDxe/XenTimerDxe.inf | 49 ++
> OvmfPkg/AcpiPlatformDxe/AcpiPlatform.h | 6 +-
> OvmfPkg/Include/Guid/XenInfo.h | 8 +-
> OvmfPkg/Include/IndustryStandard/Xen/arch-x86/hvm/start_info.h | 159 ++++++
> OvmfPkg/Include/IndustryStandard/Xen/memory.h | 23 +
> OvmfPkg/Include/Library/XenHypercallLib.h | 12 +
> OvmfPkg/Include/Library/XenPlatformLib.h | 59 ++
> OvmfPkg/Include/OvmfPlatforms.h | 6 +
> OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.h | 1 +
> OvmfPkg/XenPlatformPei/Cmos.h | 58 ++
> OvmfPkg/XenPlatformPei/Platform.h | 135 +++++
> OvmfPkg/{PlatformPei => XenPlatformPei}/Xen.h | 0
> OvmfPkg/XenTimerDxe/XenTimerDxe.h | 183 +++++++
> OvmfPkg/AcpiPlatformDxe/Xen.c | 41 +-
> OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c | 15 +-
> OvmfPkg/Library/PlatformBootManagerLib/PlatformData.c | 59 ++
> OvmfPkg/Library/ResetSystemLib/ResetSystemLib.c | 3 +-
> OvmfPkg/Library/XenHypercallLib/X86XenHypercall.c | 11 +
> OvmfPkg/Library/XenPlatformLib/XenPlatformLib.c | 75 +++
> {ArmVirtPkg => OvmfPkg}/Library/XenRealTimeClockLib/XenRealTimeClockLib.c | 0
> OvmfPkg/PlatformPei/Xen.c | 3 -
> OvmfPkg/XenIoPvhDxe/XenIoPvhDxe.c | 38 ++
> OvmfPkg/XenPlatformPei/AmdSev.c | 70 +++
> OvmfPkg/XenPlatformPei/ClearCache.c | 118 ++++
> OvmfPkg/XenPlatformPei/Cmos.c | 66 +++
> OvmfPkg/XenPlatformPei/Fv.c | 82 +++
> OvmfPkg/XenPlatformPei/MemDetect.c | 498 +++++++++++++++++
> OvmfPkg/XenPlatformPei/Platform.c | 458 ++++++++++++++++
> OvmfPkg/XenPlatformPei/Xen.c | 381 +++++++++++++
> OvmfPkg/XenTimerDxe/XenTimerDxe.c | 361 +++++++++++++
> generate_elf_header.c | 78 +++
> OvmfPkg/XenResetVector/Ia16/Real16ToFlat32.asm | 144 +++++
> OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm | 87 +++
> OvmfPkg/XenResetVector/Ia32/Flat32ToFlat64.asm | 66 +++
> OvmfPkg/XenResetVector/Ia32/PageTables64.asm | 156 ++++++
> OvmfPkg/XenResetVector/Ia32/SearchForBfvBase.asm | 93 ++++
> OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm | 75 +++
> OvmfPkg/XenResetVector/XenResetVector.nasmb | 78 +++
> 54 files changed, 4526 insertions(+), 262 deletions(-)
> copy OvmfPkg/{OvmfPkgX64.dsc => XenOvmf.dsc} (78%)
> create mode 100644 OvmfPkg/XenOvmf.fdf
> copy ArmVirtPkg/Library/XenRealTimeClockLib/XenRealTimeClockLib.inf => OvmfPkg/Library/XenPlatformLib/XenPlatformLib.inf (50%)
> rename {ArmVirtPkg => OvmfPkg}/Library/XenRealTimeClockLib/XenRealTimeClockLib.inf (100%)
> create mode 100644 OvmfPkg/XenIoPvhDxe/XenIoPvhDxe.inf
> create mode 100644 OvmfPkg/XenPlatformPei/XenPlatformPei.inf
> create mode 100644 OvmfPkg/XenResetVector/XenResetVector.inf
> create mode 100644 OvmfPkg/XenTimerDxe/XenTimerDxe.inf
> create mode 100644 OvmfPkg/Include/IndustryStandard/Xen/arch-x86/hvm/start_info.h
> create mode 100644 OvmfPkg/Include/Library/XenPlatformLib.h
> create mode 100644 OvmfPkg/XenPlatformPei/Cmos.h
> create mode 100644 OvmfPkg/XenPlatformPei/Platform.h
> copy OvmfPkg/{PlatformPei => XenPlatformPei}/Xen.h (100%)
> create mode 100644 OvmfPkg/XenTimerDxe/XenTimerDxe.h
> create mode 100644 OvmfPkg/Library/XenPlatformLib/XenPlatformLib.c
> rename {ArmVirtPkg => OvmfPkg}/Library/XenRealTimeClockLib/XenRealTimeClockLib.c (100%)
> create mode 100644 OvmfPkg/XenIoPvhDxe/XenIoPvhDxe.c
> create mode 100644 OvmfPkg/XenPlatformPei/AmdSev.c
> create mode 100644 OvmfPkg/XenPlatformPei/ClearCache.c
> create mode 100644 OvmfPkg/XenPlatformPei/Cmos.c
> create mode 100644 OvmfPkg/XenPlatformPei/Fv.c
> create mode 100644 OvmfPkg/XenPlatformPei/MemDetect.c
> create mode 100644 OvmfPkg/XenPlatformPei/Platform.c
> create mode 100644 OvmfPkg/XenPlatformPei/Xen.c
> create mode 100644 OvmfPkg/XenTimerDxe/XenTimerDxe.c
> create mode 100644 generate_elf_header.c
> create mode 100644 OvmfPkg/XenResetVector/Ia16/Real16ToFlat32.asm
> create mode 100644 OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm
> create mode 100644 OvmfPkg/XenResetVector/Ia32/Flat32ToFlat64.asm
> create mode 100644 OvmfPkg/XenResetVector/Ia32/PageTables64.asm
> create mode 100644 OvmfPkg/XenResetVector/Ia32/SearchForBfvBase.asm
> create mode 100644 OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm
> create mode 100644 OvmfPkg/XenResetVector/XenResetVector.nasmb
>
next parent reply other threads:[~2019-04-08 15:51 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20190408142408.30419-1-anthony.perard@citrix.com>
2019-04-08 15:50 ` Laszlo Ersek [this message]
2019-04-09 8:09 ` [PATCH v2 00/31] Specific platform to run OVMF in Xen PVH and HVM guests Laszlo Ersek
2019-04-09 11:15 ` [edk2-devel] " Philippe Mathieu-Daudé
2019-04-09 10:50 ` Anthony PERARD
2019-04-09 11:08 Anthony PERARD
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=6c00d5f5-e187-5fe2-8b3e-301ec51efb6b@redhat.com \
--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