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; Thu, 11 Apr 2019 04:09:43 -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 8CB73301EA8E; Thu, 11 Apr 2019 11:09:42 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-225.rdu2.redhat.com [10.10.120.225]) by smtp.corp.redhat.com (Postfix) with ESMTP id B56F760BF7; Thu, 11 Apr 2019 11:09:40 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v2 07/31] OvmfPkg/XenResetVector: Saving start of day pointer for PVH guests To: devel@edk2.groups.io, anthony.perard@citrix.com Cc: Jordan Justen , Ard Biesheuvel , Julien Grall , xen-devel@lists.xenproject.org References: <20190409110844.14746-1-anthony.perard@citrix.com> <20190409110844.14746-8-anthony.perard@citrix.com> From: "Laszlo Ersek" Message-ID: Date: Thu, 11 Apr 2019 13:09:39 +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: <20190409110844.14746-8-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.47]); Thu, 11 Apr 2019 11:09:42 +0000 (UTC) Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 04/09/19 13:08, Anthony PERARD wrote: > As described in the Xen PVH documentation [1], "ebx: contains the > physical memory address where the loader has placed the boot start info > structure". To have this pointer saved to be able to use it later in the > PEI phase, we allocate some space in the MEMFD for it. We use 'XPVH' as > a signature (for "Xen PVH"). > > [1] https://xenbits.xenproject.org/docs/unstable/misc/pvh.html > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Anthony PERARD > --- > OvmfPkg/OvmfPkg.dec | 4 ++++ > OvmfPkg/XenOvmf.fdf | 4 ++++ > OvmfPkg/XenResetVector/XenResetVector.inf | 3 +++ > OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm | 4 ++++ > OvmfPkg/XenResetVector/XenResetVector.nasmb | 2 ++ > 5 files changed, 17 insertions(+) > > diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec > index e50c6179a2..1cbbc49a6b 100644 > --- a/OvmfPkg/OvmfPkg.dec > +++ b/OvmfPkg/OvmfPkg.dec > @@ -127,6 +127,10 @@ [PcdsFixedAtBuild] > gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize|0x0|UINT32|0x1a > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDecompressionScratchEnd|0x0|UINT32|0x1f > > + # Used by XenOvmf (1) I suggest dropping this comment (it'd have to be updated to OvmfXen anyway), and... > + gUefiOvmfPkgTokenSpaceGuid.PcdXenStartOfDayStructPtr|0x0|UINT32|0x30 > + gUefiOvmfPkgTokenSpaceGuid.PcdXenStartOfDayStructPtrSize|0x0|UINT32|0x31 > + (2) calling these PcdXenPvh* rather than just PcdXen* (my understanding is that they are specific to PVH) (3) Please use token values 0x17 and 0x28. That should decrease the token space fragmentation. With those addressed: Acked-by: Laszlo Ersek Thanks Laszlo > [PcdsDynamic, PcdsDynamicEx] > gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent|0|UINT64|2 > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfFlashVariablesEnable|FALSE|BOOLEAN|0x10 > diff --git a/OvmfPkg/XenOvmf.fdf b/OvmfPkg/XenOvmf.fdf > index 20ebacd673..f9e58befd6 100644 > --- a/OvmfPkg/XenOvmf.fdf > +++ b/OvmfPkg/XenOvmf.fdf > @@ -159,6 +159,10 @@ [FD.MEMFD] > 0x007000|0x001000 > gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize > > +0x008000|0x001000 > +# Used by XenResetVector to communicate with XenPlatformPei > +gUefiOvmfPkgTokenSpaceGuid.PcdXenStartOfDayStructPtr|gUefiOvmfPkgTokenSpaceGuid.PcdXenStartOfDayStructPtrSize > + > 0x010000|0x010000 > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize > > diff --git a/OvmfPkg/XenResetVector/XenResetVector.inf b/OvmfPkg/XenResetVector/XenResetVector.inf > index 5c05f02285..ec98c17876 100644 > --- a/OvmfPkg/XenResetVector/XenResetVector.inf > +++ b/OvmfPkg/XenResetVector/XenResetVector.inf > @@ -41,3 +41,6 @@ [BuildOptions] > [Pcd] > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesBase > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPageTablesSize > + > + gUefiOvmfPkgTokenSpaceGuid.PcdXenStartOfDayStructPtr > + gUefiOvmfPkgTokenSpaceGuid.PcdXenStartOfDayStructPtrSize > diff --git a/OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm b/OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm > index c4802bf4d1..4e55b0ac1f 100644 > --- a/OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm > +++ b/OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm > @@ -22,6 +22,10 @@ xenPVHMain: > ; ESP - Initial value of the EAX register (BIST: Built-in Self Test) > mov esp, eax > > + ;; Store "Start of day" struct pointer for later use > + mov dword[PVH_SPACE (0)], ebx > + mov dword[PVH_SPACE (4)], 'XPVH' > + > cli > > mov ebx, ADDR_OF(gdtr) > diff --git a/OvmfPkg/XenResetVector/XenResetVector.nasmb b/OvmfPkg/XenResetVector/XenResetVector.nasmb > index d5a791c139..50cb81fcd1 100644 > --- a/OvmfPkg/XenResetVector/XenResetVector.nasmb > +++ b/OvmfPkg/XenResetVector/XenResetVector.nasmb > @@ -41,6 +41,8 @@ > > %include "CommonMacros.inc" > > +%define PVH_SPACE(Offset) (FixedPcdGet32 (PcdXenStartOfDayStructPtr) + (Offset)) > + > %include "PostCodes.inc" > > %ifdef DEBUG_PORT80 >