From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: citrix.com, ip: 216.71.155.175, mailfrom: anthony.perard@citrix.com) Received: from esa6.hc3370-68.iphmx.com (esa6.hc3370-68.iphmx.com [216.71.155.175]) by groups.io with SMTP; Fri, 19 Jul 2019 03:20:12 -0700 Authentication-Results: esa6.hc3370-68.iphmx.com; dkim=none (message not signed) header.i=none; spf=None smtp.pra=anthony.perard@citrix.com; spf=Pass smtp.mailfrom=anthony.perard@citrix.com; spf=None smtp.helo=postmaster@mail.citrix.com Received-SPF: None (esa6.hc3370-68.iphmx.com: no sender authenticity information available from domain of anthony.perard@citrix.com) identity=pra; client-ip=162.221.158.21; receiver=esa6.hc3370-68.iphmx.com; envelope-from="anthony.perard@citrix.com"; x-sender="anthony.perard@citrix.com"; x-conformance=sidf_compatible Received-SPF: Pass (esa6.hc3370-68.iphmx.com: domain of anthony.perard@citrix.com designates 162.221.158.21 as permitted sender) identity=mailfrom; client-ip=162.221.158.21; receiver=esa6.hc3370-68.iphmx.com; envelope-from="anthony.perard@citrix.com"; x-sender="anthony.perard@citrix.com"; x-conformance=sidf_compatible; x-record-type="v=spf1"; x-record-text="v=spf1 ip4:209.167.231.154 ip4:178.63.86.133 ip4:195.66.111.40/30 ip4:85.115.9.32/28 ip4:199.102.83.4 ip4:192.28.146.160 ip4:192.28.146.107 ip4:216.52.6.88 ip4:216.52.6.188 ip4:162.221.158.21 ip4:162.221.156.83 ~all" Received-SPF: None (esa6.hc3370-68.iphmx.com: no sender authenticity information available from domain of postmaster@mail.citrix.com) identity=helo; client-ip=162.221.158.21; receiver=esa6.hc3370-68.iphmx.com; envelope-from="anthony.perard@citrix.com"; x-sender="postmaster@mail.citrix.com"; x-conformance=sidf_compatible IronPort-SDR: gYZJS3f6gIfbMnVZXmzteW5i/YhvBjZcqY4K2ruLEv8VkJnxOBH+sM3+s9OuJUJH3m7/SRfydD gtBmipG0JhTcDqhYhTT21creU7IIx5pv3Y16GvgdzMyGVZBUyDk3sECA95sZwR3kMemBWijrSg Uwhdyr898h7OJineJGI8ms3uU4aoRxiE+eBpwJnr5rbHGgMdyb9l7higKMldi8dENgJ1bieyYx hoPFYTJBPsNnHBbNNC3ZxEGTar3ycg5HFbbzKu9dyr4Dgv6hMCAaVFAdMjFUIW33ia5SAi8Y12 uoM= X-SBRS: 2.7 X-MesageID: 3264987 X-Ironport-Server: esa6.hc3370-68.iphmx.com X-Remote-IP: 162.221.158.21 X-Policy: $RELAYED X-IronPort-AV: E=Sophos;i="5.64,282,1559534400"; d="scan'208";a="3264987" Date: Fri, 19 Jul 2019 11:20:08 +0100 From: "Anthony PERARD" To: Andrew Cooper CC: , , Ard Biesheuvel , Jordan Justen , Laszlo Ersek , Julien Grall , "Roger Pau =?iso-8859-1?Q?Monn=E9?=" Subject: Re: [PATCH v3 06/35] OvmfPkg/XenResetVector: Add new entry point for Xen PVH Message-ID: <20190719102008.GB1208@perard.uk.xensource.com> References: <20190704144233.27968-1-anthony.perard@citrix.com> <20190704144233.27968-7-anthony.perard@citrix.com> MIME-Version: 1.0 In-Reply-To: User-Agent: Mutt/1.12.1 (2019-06-15) Return-Path: anthony.perard@citrix.com Content-Type: text/plain; charset="iso-8859-1" Content-Disposition: inline Content-Transfer-Encoding: 8bit On Fri, Jul 05, 2019 at 02:57:06PM +0100, Andrew Cooper wrote: > On 04/07/2019 15:42, Anthony PERARD wrote: > > diff --git a/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm b/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm > > new file mode 100644 > > index 0000000000..958195bc5e > > --- /dev/null > > +++ b/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm > > +vtfSignature: > > + DB 'V', 'T', 'F', 0 > > + > > +ALIGN 16 > > + > > +resetVector: > > +; > > +; Reset Vector > > +; > > +; This is where the processor will begin execution > > +; > > + nop > > + nop > > Why two nops? I don't know, this is existing code that I duplicated to allow adding a new entry point. (I wanted to use --find-copies-harder when sending the patch, but forgot this time. This part of the chunk would not be there.) > > + jmp EarlyBspInitReal16 > > + > > +ALIGN 16 > > + > > +fourGigabytes: > > + > > diff --git a/OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm b/OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm > > new file mode 100644 > > index 0000000000..2a17fed52f > > --- /dev/null > > +++ b/OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm > > @@ -0,0 +1,49 @@ > > +;------------------------------------------------------------------------------ > > +; @file > > +; An entry point use by Xen when a guest is started in PVH mode. > > +; > > +; Copyright (c) 2019, Citrix Systems, Inc. > > +; > > +; SPDX-License-Identifier: BSD-2-Clause-Patent > > +; > > +;------------------------------------------------------------------------------ > > + > > +BITS 32 > > + > > +xenPVHMain: > > + ; > > + ; 'BP' to indicate boot-strap processor > > Indicate to what? According to UefiCpuPkg/ResetVector/Vtf0/ReadMe.txt, that's a parameter for the SEC image that this ResetVector locates then run. > > + ; > > + mov di, 'BP' > > + > > + ; > > + ; ESP will be used as initial value of the EAX register > > + ; in Main.asm > > + ; > > + xor esp, esp > > + > > + mov ebx, ADDR_OF(gdtr) > > + lgdt [ebx] > > lgdt [ADDR_OF(gdtr)] > > should work fine, because you're in 32bit mode. Yes, that worked fine, but a subsequent patch is going to want to modify the gdtr address, so I've been lazy and didn't use lgdt [ADDR_OF()] here. See: OvmfPkg/XenResetVector: Allow jumpstart from either hvmloader or PVH https://patchew.org/EDK2/20190704144233.27968-1-anthony.perard@citrix.com/20190704144233.27968-9-anthony.perard@citrix.com/ > More importantly for PVH however, you don't clobber the start_info pointer. I will actually save the start_info pointer before setting the gdt, but that's done in a different patch: OvmfPkg/XenResetVector: Saving start of day pointer for PVH guests https://patchew.org/EDK2/20190704144233.27968-1-anthony.perard@citrix.com/20190704144233.27968-8-anthony.perard@citrix.com/ > > + > > + mov eax, SEC_DEFAULT_CR0 > > + mov cr0, eax > > + > > + jmp LINEAR_CODE_SEL:ADDR_OF(.jmpToNewCodeSeg) > > +.jmpToNewCodeSeg: > > Does 1f (or some equivalent) not work, or is this against the coding style? I didn't find the ${label}f syntax when reading the NASM manual. But using .${label} would be the closest. Those labels starting with a dot are called local labels. The actual full label, if one want to use it from anywhere, would be "XenPVHMain.jmpToNewCodeSeg" here. > > + > > + mov eax, SEC_DEFAULT_CR4 > > + mov cr4, eax > > + > > + mov ax, LINEAR_SEL > > + mov ds, ax > > + mov es, ax > > + mov fs, ax > > + mov gs, ax > > + mov ss, ax > > Use eax rather than ax.  The instruction decode will be much happier > with the result, and it results in shorter assembled code. I look into that. Thanks, -- Anthony PERARD