From: "Laszlo Ersek" <lersek@redhat.com>
To: Anthony PERARD <anthony.perard@citrix.com>
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: Tue, 9 Apr 2019 10:09:28 +0200 [thread overview]
Message-ID: <34ead1d8-12d8-91b5-9767-3d706121a6b0@redhat.com> (raw)
In-Reply-To: <6c00d5f5-e187-5fe2-8b3e-301ec51efb6b@redhat.com>
On 04/08/19 17:50, Laszlo Ersek wrote:
> 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).
(7) I forgot to mention that, due to
<https://bugzilla.tianocore.org/show_bug.cgi?id=1373> being very close
to being fixed / pushed, the the "Contributed-under" lines near your
signoffs will no longer be necessary.
Thanks
Laszlo
>
> 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 prev parent reply other threads:[~2019-04-09 8:09 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 ` [PATCH v2 00/31] Specific platform to run OVMF in Xen PVH and HVM guests Laszlo Ersek
2019-04-09 8:09 ` Laszlo Ersek [this message]
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=34ead1d8-12d8-91b5-9767-3d706121a6b0@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