Re: [PATCH v4 06/35] OvmfPkg/XenResetVector: Add new entry point for Xen PVH

Roger Pau Monné

On Mon, Jul 29, 2019 at 04:39:15PM +0100, Anthony PERARD wrote:
Add a new entry point for Xen PVH that enter directly in 32bits.

Information on the expected state of the machine when this entry point
is used can be found at:

Also, compare to the original file [1], the two `nop' of the "resetVector"
entry point are removed. There were introduced by 8332983e2e33
("UefiCpuPkg: Replace the un-necessary WBINVD instruction at the reset
vector with two NOPs in VTF0.", 2011-08-04), but don't seems to be
useful. This is the entry point used by HVM guest (hvmloader).

[1] UefiCpuPkg/ResetVector/Vtf0/Ia16/ResetVectorVtf0.asm

Signed-off-by: Anthony PERARD <anthony.perard@...>
Acked-by: Laszlo Ersek <lersek@...>

- remove the two nop in the HVM entry point

- rebased, SPDX
- remove `cli' as via PVH the interrupts are guaranteed to be off
- rewrite some comments

.../XenResetVector/Ia16/ResetVectorVtf0.asm | 79 +++++++++++++++++++
OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm | 49 ++++++++++++
OvmfPkg/XenResetVector/XenResetVector.nasmb | 1 +
3 files changed, 129 insertions(+)
create mode 100644 OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm
create mode 100644 OvmfPkg/XenResetVector/Ia32/XenPVHMain.asm

diff --git a/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm b/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm
new file mode 100644
index 0000000000..56749bdbc9
--- /dev/null
+++ b/OvmfPkg/XenResetVector/Ia16/ResetVectorVtf0.asm
@@ -0,0 +1,79 @@
+; @file
+; First code executed by processor after resetting.
+; Copyright (c) 2008 - 2014, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2019, Citrix Systems, Inc.
+; SPDX-License-Identifier: BSD-2-Clause-Patent
+BITS 16
+; Pad the image size to 4k when page tables are in VTF0
+; If the VTF0 image has page tables built in, then we need to make
+; sure the end of VTF0 is 4k above where the page tables end.
+; This is required so the page tables will be 4k aligned when VTF0 is
+; located just below 0x100000000 (4GB) in the firmware device.
+ TIMES (0x1000 - ($ - EndOfPageTables) - (fourGigabytes - xenPVHEntryPoint)) DB 0
+BITS 32
I think you likely want an ALIGN 32 here in case
ALIGN_TOP_TO_4K_FOR_PAGING is not defined, so that xenPVHEntryPoint is
aligned to a double word boundary?

(not that it's mandatory for this to work)

The rest LGTM.

Thanks, Roger.

Join to automatically receive all group messages.