Re: [PATCH v4 12/35] OvmfPkg/XenPlatformPei: Grab RSDP from PVH guest start of day struct

Roger Pau Monné
 

On Mon, Jul 29, 2019 at 04:39:21PM +0100, Anthony PERARD wrote:
Check if there's a start of the day struct provided to PVH guest, save
the ACPI RSDP address for later.

This patch import import arch-x86/hvm/start_info.h from xen.git.
You seem to change the types when importing start_info.h, is that
absolutely necessary?

From my experience working with different projects when importing such
headers it's better to keep them verbatim, this makes importing future
versions from upstream easier.

I have a comment below, but it's more like a question.

diff --git a/OvmfPkg/XenPlatformPei/Xen.c b/OvmfPkg/XenPlatformPei/Xen.c
index 5c7d7ddc1c..b366139a0a 100644
--- a/OvmfPkg/XenPlatformPei/Xen.c
+++ b/OvmfPkg/XenPlatformPei/Xen.c
@@ -25,6 +25,7 @@
#include <IndustryStandard/E820.h>
#include <Library/ResourcePublicationLib.h>
#include <Library/MtrrLib.h>
+#include <IndustryStandard/Xen/arch-x86/hvm/start_info.h>

#include "Platform.h"
#include "Xen.h"
@@ -86,6 +87,7 @@ XenConnect (
UINT32 XenVersion;
EFI_XEN_OVMF_INFO *Info;
CHAR8 Sig[sizeof (Info->Signature) + 1];
+ UINT32 *PVHResetVectorData;

AsmCpuid (XenLeaf + 2, &TransferPages, &TransferReg, NULL, NULL);
mXenInfo.HyperPages = AllocatePages (TransferPages);
@@ -121,6 +123,29 @@ XenConnect (
mXenHvmloaderInfo = NULL;
}

+ mXenInfo.RsdpPvh = NULL;
+
+ //
+ // Locate and use information from the start of day structure if we have
+ // booted via the PVH entry point.
+ //
+
+ PVHResetVectorData = (VOID *)(UINTN) PcdGet32 (PcdXenPvhStartOfDayStructPtr);
+ //
+ // That magic value is written in XenResetVector/Ia32/XenPVHMain.asm
+ //
+ if (PVHResetVectorData[1] == SIGNATURE_32 ('X', 'P', 'V', 'H')) {
+ struct hvm_start_info *HVMStartInfo;
+
+ HVMStartInfo = (VOID *)(UINTN) PVHResetVectorData[0];
+ if (HVMStartInfo->magic == XEN_HVM_START_MAGIC_VALUE) {
+ ASSERT (HVMStartInfo->rsdp_paddr != 0);
+ if (HVMStartInfo->rsdp_paddr != 0) {
+ mXenInfo.RsdpPvh = (VOID *)(UINTN)HVMStartInfo->rsdp_paddr;
I guess you can do this because OVMF has an identity map of virtual
addresses to physical addresses?

I wonder the size of such identity map, and whether you need to check
that the rsdp address is indeed inside of that region before
converting it to a pointer. The same would apply to
PcdXenPvhStartOfDayStructPtr, but that's likely safe because it's
always < 4GB, which I assume OVMF always has identity mapped?

Thanks, Roger.

Join devel@edk2.groups.io to automatically receive all group messages.