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; Tue, 30 Jul 2019 06:01:34 -0700 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5BC82308FEC1; Tue, 30 Jul 2019 13:01:34 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (unknown [10.36.118.39]) by smtp.corp.redhat.com (Postfix) with ESMTP id C2F995D6D0; Tue, 30 Jul 2019 13:01:32 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v4 33/35] OvmfPkg: Introduce XenIoPvhDxe to initialize Grant Tables To: devel@edk2.groups.io, anthony.perard@citrix.com Cc: Julien Grall , xen-devel@lists.xenproject.org, Jordan Justen , Ard Biesheuvel References: <20190729153944.24239-1-anthony.perard@citrix.com> <20190729153944.24239-34-anthony.perard@citrix.com> From: "Laszlo Ersek" Message-ID: <8a5d9e63-12f1-2ff8-7c40-773d15baf333@redhat.com> Date: Tue, 30 Jul 2019 15:01:31 +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: <20190729153944.24239-34-anthony.perard@citrix.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.49]); Tue, 30 Jul 2019 13:01:34 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 07/29/19 17:39, Anthony PERARD wrote: > XenIoPvhDxe use XenIoMmioLib to reserve some space to be use by the > Grant Tables. > > The call is only done if it is necessary, we simply detect if the > guest is PVH, as in this case there is currently no PCI bus, and no > PCI Xen platform device which would start the XenIoPciDxe and allocate > the space for the Grant Tables. > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1689 > Signed-off-by: Anthony PERARD > --- > > Notes: > v4: > - Removed XenIoPvhDxeNotifyExitBoot() which was doing action that should > not be done in an ExitBootServices notification. > (InitializeXenIoPvhDxe() has been cleaned up following this.) > - Use new PcdXenGrantFrames. > - Some coding style fix > - Update Maintainers.txt > > v3: > - downgrade type to DXE_DRIVER > - use SPDX > - rework InitializeXenIoPvhDxe, and handle errors properly. > - Free the reserved allocation in ExitBootServices even if the XenIo > protocol could successfully been uninstalled. > > v2: > - do allocation in EntryPoint like the other user of XenIoMmioLib. > - allocate memory instead of hardcoded addr. > - cleanup, add copyright > - detect if we are running in PVH mode > > OvmfPkg/OvmfXen.dsc | 2 ++ > OvmfPkg/OvmfXen.fdf | 1 + > OvmfPkg/XenIoPvhDxe/XenIoPvhDxe.inf | 35 +++++++++++++++++++ > OvmfPkg/XenIoPvhDxe/XenIoPvhDxe.c | 53 +++++++++++++++++++++++++++++ > Maintainers.txt | 1 + > 5 files changed, 92 insertions(+) > create mode 100644 OvmfPkg/XenIoPvhDxe/XenIoPvhDxe.inf > create mode 100644 OvmfPkg/XenIoPvhDxe/XenIoPvhDxe.c > > diff --git a/OvmfPkg/OvmfXen.dsc b/OvmfPkg/OvmfXen.dsc > index e719a168f8..5e07b37279 100644 > --- a/OvmfPkg/OvmfXen.dsc > +++ b/OvmfPkg/OvmfXen.dsc > @@ -195,6 +195,7 @@ [LibraryClasses] > OrderedCollectionLib|MdePkg/Library/BaseOrderedCollectionRedBlackTreeLib/BaseOrderedCollectionRedBlackTreeLib.inf > XenHypercallLib|OvmfPkg/Library/XenHypercallLib/XenHypercallLib.inf > XenPlatformLib|OvmfPkg/Library/XenPlatformLib/XenPlatformLib.inf > + XenIoMmioLib|OvmfPkg/Library/XenIoMmioLib/XenIoMmioLib.inf > > Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf > TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf > @@ -583,6 +584,7 @@ [Components] > NULL|OvmfPkg/Csm/LegacyBootMaintUiLib/LegacyBootMaintUiLib.inf > !endif > } > + OvmfPkg/XenIoPvhDxe/XenIoPvhDxe.inf > OvmfPkg/XenIoPciDxe/XenIoPciDxe.inf > OvmfPkg/XenBusDxe/XenBusDxe.inf > OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf > diff --git a/OvmfPkg/OvmfXen.fdf b/OvmfPkg/OvmfXen.fdf > index 5c1a925d6a..517a492f14 100644 > --- a/OvmfPkg/OvmfXen.fdf > +++ b/OvmfPkg/OvmfXen.fdf > @@ -309,6 +309,7 @@ [FV.DXEFV] > INF MdeModulePkg/Universal/Metronome/Metronome.inf > INF PcAtChipsetPkg/PcatRealTimeClockRuntimeDxe/PcatRealTimeClockRuntimeDxe.inf > > +INF OvmfPkg/XenIoPvhDxe/XenIoPvhDxe.inf > INF OvmfPkg/XenIoPciDxe/XenIoPciDxe.inf > INF OvmfPkg/XenBusDxe/XenBusDxe.inf > INF OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf > diff --git a/OvmfPkg/XenIoPvhDxe/XenIoPvhDxe.inf b/OvmfPkg/XenIoPvhDxe/XenIoPvhDxe.inf > new file mode 100644 > index 0000000000..5740df6e59 > --- /dev/null > +++ b/OvmfPkg/XenIoPvhDxe/XenIoPvhDxe.inf > @@ -0,0 +1,35 @@ > +## @file > +# Driver for the XenIo protocol > +# > +# Copyright (c) 2019, Citrix Systems, Inc. > +# > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +## > + > +[Defines] > + INF_VERSION = 0x00010005 > + BASE_NAME = XenIoPvhDxe > + FILE_GUID = 7a567cc4-0e75-4d7a-a305-c3db109b53ad > + MODULE_TYPE = DXE_DRIVER > + VERSION_STRING = 1.0 > + ENTRY_POINT = InitializeXenIoPvhDxe > + > +[Packages] > + MdePkg/MdePkg.dec > + OvmfPkg/OvmfPkg.dec > + > +[Sources] > + XenIoPvhDxe.c > + > +[LibraryClasses] > + MemoryAllocationLib > + UefiDriverEntryPoint > + XenIoMmioLib > + XenPlatformLib > + > +[FixedPcd] > + gUefiOvmfPkgTokenSpaceGuid.PcdXenGrantFrames > + > +[Depex] > + TRUE > diff --git a/OvmfPkg/XenIoPvhDxe/XenIoPvhDxe.c b/OvmfPkg/XenIoPvhDxe/XenIoPvhDxe.c > new file mode 100644 > index 0000000000..e5699cdd80 > --- /dev/null > +++ b/OvmfPkg/XenIoPvhDxe/XenIoPvhDxe.c > @@ -0,0 +1,53 @@ > +/** @file > + > + Driver for the XenIo protocol > + > + This driver simply allocate space for the grant tables. > + > + Copyright (c) 2019, Citrix Systems, Inc. > + > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#include > +#include > +#include > + > +EFI_STATUS > +EFIAPI > +InitializeXenIoPvhDxe ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + VOID *Allocation; > + EFI_STATUS Status; > + EFI_HANDLE XenIoHandle; > + > + Allocation = NULL; > + XenIoHandle = NULL; > + > + if (!XenPvhDetected ()) { > + return EFI_UNSUPPORTED; > + } > + > + Allocation = AllocateReservedPages (FixedPcdGet32 (PcdXenGrantFrames)); > + if (Allocation == NULL) { > + Status = EFI_OUT_OF_RESOURCES; > + goto Error; > + } > + > + Status = XenIoMmioInstall (&XenIoHandle, (UINTN) Allocation); > + if (EFI_ERROR (Status)) { > + goto Error; > + } > + > + return EFI_SUCCESS; > + > +Error: > + if (Allocation != NULL) { > + FreePages (Allocation, FixedPcdGet32 (PcdXenGrantFrames)); > + } > + return Status; > +} > diff --git a/Maintainers.txt b/Maintainers.txt > index 78e9f889ab..79defd13bf 100644 > --- a/Maintainers.txt > +++ b/Maintainers.txt > @@ -382,6 +382,7 @@ F: OvmfPkg/PlatformPei/Xen.* > F: OvmfPkg/SmbiosPlatformDxe/*Xen.c > F: OvmfPkg/XenBusDxe/ > F: OvmfPkg/XenIoPciDxe/ > +F: OvmfPkg/XenIoPvhDxe/ > F: OvmfPkg/XenPlatformPei/ > F: OvmfPkg/XenPvBlkDxe/ > F: OvmfPkg/XenResetVector/ > (1) We should #include in the "XenIoPvhDxe.c" file, and list PcdLib in the [LibraryClasses] section of the "XenIoPvhDxe.inf" file. With (1) fixed: Reviewed-by: Laszlo Ersek If v5 doesn't become necessary, I can update this patch for you before pushing the v4 series; if that's what you prefer. Thanks Laszlo