From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: redhat.com, ip: 209.132.183.28, mailfrom: lersek@redhat.com) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by groups.io with SMTP; Mon, 08 Apr 2019 08:51:02 -0700 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9C8B730B49D3; Mon, 8 Apr 2019 15:51:01 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-121-29.rdu2.redhat.com [10.10.121.29]) by smtp.corp.redhat.com (Postfix) with ESMTP id 259F960BE2; Mon, 8 Apr 2019 15:50:58 +0000 (UTC) Subject: Re: [PATCH v2 00/31] Specific platform to run OVMF in Xen PVH and HVM guests To: Anthony PERARD , edk2-devel@lists.01.org Cc: Julien Grall , Ard Biesheuvel , Jordan Justen , xen-devel@lists.xenproject.org, Leif Lindholm , edk2-devel-groups-io References: <20190408142408.30419-1-anthony.perard@citrix.com> From: "Laszlo Ersek" Message-ID: <6c00d5f5-e187-5fe2-8b3e-301ec51efb6b@redhat.com> Date: Mon, 8 Apr 2019 17:50:57 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20190408142408.30419-1-anthony.perard@citrix.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.49]); Mon, 08 Apr 2019 15:51:01 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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 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): (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. . 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 >