From: "Philippe Mathieu-Daudé" <philmd@redhat.com>
To: devel@edk2.groups.io, lersek@redhat.com,
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>
Subject: Re: [edk2-devel] [PATCH v2 00/31] Specific platform to run OVMF in Xen PVH and HVM guests
Date: Tue, 9 Apr 2019 13:15:20 +0200 [thread overview]
Message-ID: <bc109e32-6bb7-2836-9e90-1328b554a74b@redhat.com> (raw)
In-Reply-To: <34ead1d8-12d8-91b5-9767-3d706121a6b0@redhat.com>
On 4/9/19 10:09 AM, Laszlo Ersek wrote:
> 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'm happy to read this simplification suggestion :)
>> 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 11:15 UTC|newest]
Thread overview: 4+ 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
2019-04-09 11:15 ` Philippe Mathieu-Daudé [this message]
2019-04-09 10:50 ` 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=bc109e32-6bb7-2836-9e90-1328b554a74b@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