| Message ID | 20190409110844.14746-11-anthony.perard@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Specific platform to run OVMF in Xen PVH and HVM guests | expand |
On 04/09/19 13:08, Anthony PERARD wrote: > This struct is only useful to retrieve the E820 table. The (1) please replace "this struct" with the name of the struct. > mXenHvmloaderInfo isn't used yet, but will be use in a further patch to > retrieve the E820 table. > > Also remove the unused pointer from the XenInfo HOB as that information > is only useful in the XenPlatformPei. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > --- > OvmfPkg/Include/Guid/XenInfo.h | 4 ---- > OvmfPkg/PlatformPei/Xen.c | 3 --- > OvmfPkg/XenPlatformPei/Xen.c | 23 ++++++++++++++++++-- > 3 files changed, 21 insertions(+), 9 deletions(-) > > diff --git a/OvmfPkg/Include/Guid/XenInfo.h b/OvmfPkg/Include/Guid/XenInfo.h > index d512b0b63f..5d4579f244 100644 > --- a/OvmfPkg/Include/Guid/XenInfo.h > +++ b/OvmfPkg/Include/Guid/XenInfo.h > @@ -24,10 +24,6 @@ typedef struct { > /// > VOID *HyperPages; > /// > - /// Location of the hvm_info page. > - /// > - VOID *HvmInfo; > - /// > /// Hypervisor major version. > /// > UINT16 VersionMajor; > diff --git a/OvmfPkg/PlatformPei/Xen.c b/OvmfPkg/PlatformPei/Xen.c > index ab38f97a67..75181be2fd 100644 > --- a/OvmfPkg/PlatformPei/Xen.c > +++ b/OvmfPkg/PlatformPei/Xen.c > @@ -104,9 +104,6 @@ XenConnect ( > mXenInfo.VersionMajor = (UINT16)(XenVersion >> 16); > mXenInfo.VersionMinor = (UINT16)(XenVersion & 0xFFFF); > > - /* TBD: Locate hvm_info and reserve it away. */ > - mXenInfo.HvmInfo = NULL; > - > BuildGuidDataHob ( > &gEfiXenInfoGuid, > &mXenInfo, > diff --git a/OvmfPkg/XenPlatformPei/Xen.c b/OvmfPkg/XenPlatformPei/Xen.c > index 7c82e5b69b..a866641b67 100644 > --- a/OvmfPkg/XenPlatformPei/Xen.c > +++ b/OvmfPkg/XenPlatformPei/Xen.c > @@ -39,6 +39,12 @@ STATIC UINT32 mXenLeaf = 0; > > EFI_XEN_INFO mXenInfo; > > +// > +// Location of the firmware info struct setup by hvmloader. > +// Only the E820 table is used by OVMF. > +// > +EFI_XEN_OVMF_INFO *mXenHvmloaderInfo; > + > /** > Returns E820 map provided by Xen > > @@ -84,6 +90,8 @@ XenConnect ( > UINT32 TransferReg; > UINT32 TransferPages; > UINT32 XenVersion; > + EFI_XEN_OVMF_INFO *Info; > + CHAR8 Sig[sizeof (Info->Signature) + 1]; > > AsmCpuid (XenLeaf + 2, &TransferPages, &TransferReg, NULL, NULL); > mXenInfo.HyperPages = AllocatePages (TransferPages); > @@ -103,8 +111,19 @@ XenConnect ( > mXenInfo.VersionMajor = (UINT16)(XenVersion >> 16); > mXenInfo.VersionMinor = (UINT16)(XenVersion & 0xFFFF); > > - /* TBD: Locate hvm_info and reserve it away. */ > - mXenInfo.HvmInfo = NULL; > + // > + // Check if there are information left by hvmloader > + // > + > + Info = (EFI_XEN_OVMF_INFO *)(UINTN) OVMF_INFO_PHYSICAL_ADDRESS; > + // Copy the signature, and make it null-terminated. (2) Please use the following style: // // Copy the signature, and make it null-terminated. // > + AsciiStrnCpyS(Sig, sizeof (Sig), > + (CHAR8 *) &Info->Signature, sizeof (Info->Signature)); (3) This should be indented as: AsciiStrnCpyS ( Sig, sizeof (Sig), (CHAR8 *) &Info->Signature, sizeof (Info->Signature) ); or: AsciiStrnCpyS (Sig, sizeof (Sig), (CHAR8 *) &Info->Signature, sizeof (Info->Signature)); (note the whitespace before the opening paren, after the function designator) I'm not checking the correctness of the parameters, for now. With (1) and (3) addressed: Acked-by: Laszlo Ersek <lersek@redhat.com> If you can, please verify the comment style and the indentation style in the other patches too. Thanks Laszlo > + if (AsciiStrCmp (Sig, "XenHVMOVMF") == 0) { > + mXenHvmloaderInfo = Info; > + } else { > + mXenHvmloaderInfo = NULL; > + } > > BuildGuidDataHob ( > &gEfiXenInfoGuid, >
On 04/11/19 13:47, Laszlo Ersek wrote: > On 04/09/19 13:08, Anthony PERARD wrote: >> This struct is only useful to retrieve the E820 table. The > > (1) please replace "this struct" with the name of the struct. > >> mXenHvmloaderInfo isn't used yet, but will be use in a further patch to >> retrieve the E820 table. >> >> Also remove the unused pointer from the XenInfo HOB as that information >> is only useful in the XenPlatformPei. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> >> --- >> OvmfPkg/Include/Guid/XenInfo.h | 4 ---- >> OvmfPkg/PlatformPei/Xen.c | 3 --- >> OvmfPkg/XenPlatformPei/Xen.c | 23 ++++++++++++++++++-- >> 3 files changed, 21 insertions(+), 9 deletions(-) >> >> diff --git a/OvmfPkg/Include/Guid/XenInfo.h b/OvmfPkg/Include/Guid/XenInfo.h >> index d512b0b63f..5d4579f244 100644 >> --- a/OvmfPkg/Include/Guid/XenInfo.h >> +++ b/OvmfPkg/Include/Guid/XenInfo.h >> @@ -24,10 +24,6 @@ typedef struct { >> /// >> VOID *HyperPages; >> /// >> - /// Location of the hvm_info page. >> - /// >> - VOID *HvmInfo; >> - /// >> /// Hypervisor major version. >> /// >> UINT16 VersionMajor; >> diff --git a/OvmfPkg/PlatformPei/Xen.c b/OvmfPkg/PlatformPei/Xen.c >> index ab38f97a67..75181be2fd 100644 >> --- a/OvmfPkg/PlatformPei/Xen.c >> +++ b/OvmfPkg/PlatformPei/Xen.c >> @@ -104,9 +104,6 @@ XenConnect ( >> mXenInfo.VersionMajor = (UINT16)(XenVersion >> 16); >> mXenInfo.VersionMinor = (UINT16)(XenVersion & 0xFFFF); >> >> - /* TBD: Locate hvm_info and reserve it away. */ >> - mXenInfo.HvmInfo = NULL; >> - >> BuildGuidDataHob ( >> &gEfiXenInfoGuid, >> &mXenInfo, >> diff --git a/OvmfPkg/XenPlatformPei/Xen.c b/OvmfPkg/XenPlatformPei/Xen.c >> index 7c82e5b69b..a866641b67 100644 >> --- a/OvmfPkg/XenPlatformPei/Xen.c >> +++ b/OvmfPkg/XenPlatformPei/Xen.c >> @@ -39,6 +39,12 @@ STATIC UINT32 mXenLeaf = 0; >> >> EFI_XEN_INFO mXenInfo; >> >> +// >> +// Location of the firmware info struct setup by hvmloader. >> +// Only the E820 table is used by OVMF. >> +// >> +EFI_XEN_OVMF_INFO *mXenHvmloaderInfo; >> + >> /** >> Returns E820 map provided by Xen >> >> @@ -84,6 +90,8 @@ XenConnect ( >> UINT32 TransferReg; >> UINT32 TransferPages; >> UINT32 XenVersion; >> + EFI_XEN_OVMF_INFO *Info; >> + CHAR8 Sig[sizeof (Info->Signature) + 1]; >> >> AsmCpuid (XenLeaf + 2, &TransferPages, &TransferReg, NULL, NULL); >> mXenInfo.HyperPages = AllocatePages (TransferPages); >> @@ -103,8 +111,19 @@ XenConnect ( >> mXenInfo.VersionMajor = (UINT16)(XenVersion >> 16); >> mXenInfo.VersionMinor = (UINT16)(XenVersion & 0xFFFF); >> >> - /* TBD: Locate hvm_info and reserve it away. */ >> - mXenInfo.HvmInfo = NULL; >> + // >> + // Check if there are information left by hvmloader >> + // >> + >> + Info = (EFI_XEN_OVMF_INFO *)(UINTN) OVMF_INFO_PHYSICAL_ADDRESS; >> + // Copy the signature, and make it null-terminated. > > (2) Please use the following style: > > // > // Copy the signature, and make it null-terminated. > // > >> + AsciiStrnCpyS(Sig, sizeof (Sig), >> + (CHAR8 *) &Info->Signature, sizeof (Info->Signature)); > > (3) This should be indented as: > > AsciiStrnCpyS ( > Sig, > sizeof (Sig), > (CHAR8 *) &Info->Signature, > sizeof (Info->Signature) > ); > > or: > > AsciiStrnCpyS (Sig, sizeof (Sig), (CHAR8 *) &Info->Signature, > sizeof (Info->Signature)); > > (note the whitespace before the opening paren, after the function > designator) > > > I'm not checking the correctness of the parameters, for now. > > With (1) and (3) addressed: sigh, I meant "(1) through (3)". > Acked-by: Laszlo Ersek <lersek@redhat.com> > > If you can, please verify the comment style and the indentation style in > the other patches too. > > Thanks > Laszlo > >> + if (AsciiStrCmp (Sig, "XenHVMOVMF") == 0) { >> + mXenHvmloaderInfo = Info; >> + } else { >> + mXenHvmloaderInfo = NULL; >> + } >> >> BuildGuidDataHob ( >> &gEfiXenInfoGuid, >> >
diff --git a/OvmfPkg/Include/Guid/XenInfo.h b/OvmfPkg/Include/Guid/XenInfo.h index d512b0b63f..5d4579f244 100644 --- a/OvmfPkg/Include/Guid/XenInfo.h +++ b/OvmfPkg/Include/Guid/XenInfo.h @@ -24,10 +24,6 @@ typedef struct { /// VOID *HyperPages; /// - /// Location of the hvm_info page. - /// - VOID *HvmInfo; - /// /// Hypervisor major version. /// UINT16 VersionMajor; diff --git a/OvmfPkg/PlatformPei/Xen.c b/OvmfPkg/PlatformPei/Xen.c index ab38f97a67..75181be2fd 100644 --- a/OvmfPkg/PlatformPei/Xen.c +++ b/OvmfPkg/PlatformPei/Xen.c @@ -104,9 +104,6 @@ XenConnect ( mXenInfo.VersionMajor = (UINT16)(XenVersion >> 16); mXenInfo.VersionMinor = (UINT16)(XenVersion & 0xFFFF); - /* TBD: Locate hvm_info and reserve it away. */ - mXenInfo.HvmInfo = NULL; - BuildGuidDataHob ( &gEfiXenInfoGuid, &mXenInfo, diff --git a/OvmfPkg/XenPlatformPei/Xen.c b/OvmfPkg/XenPlatformPei/Xen.c index 7c82e5b69b..a866641b67 100644 --- a/OvmfPkg/XenPlatformPei/Xen.c +++ b/OvmfPkg/XenPlatformPei/Xen.c @@ -39,6 +39,12 @@ STATIC UINT32 mXenLeaf = 0; EFI_XEN_INFO mXenInfo; +// +// Location of the firmware info struct setup by hvmloader. +// Only the E820 table is used by OVMF. +// +EFI_XEN_OVMF_INFO *mXenHvmloaderInfo; + /** Returns E820 map provided by Xen @@ -84,6 +90,8 @@ XenConnect ( UINT32 TransferReg; UINT32 TransferPages; UINT32 XenVersion; + EFI_XEN_OVMF_INFO *Info; + CHAR8 Sig[sizeof (Info->Signature) + 1]; AsmCpuid (XenLeaf + 2, &TransferPages, &TransferReg, NULL, NULL); mXenInfo.HyperPages = AllocatePages (TransferPages); @@ -103,8 +111,19 @@ XenConnect ( mXenInfo.VersionMajor = (UINT16)(XenVersion >> 16); mXenInfo.VersionMinor = (UINT16)(XenVersion & 0xFFFF); - /* TBD: Locate hvm_info and reserve it away. */ - mXenInfo.HvmInfo = NULL; + // + // Check if there are information left by hvmloader + // + + Info = (EFI_XEN_OVMF_INFO *)(UINTN) OVMF_INFO_PHYSICAL_ADDRESS; + // Copy the signature, and make it null-terminated. + AsciiStrnCpyS(Sig, sizeof (Sig), + (CHAR8 *) &Info->Signature, sizeof (Info->Signature)); + if (AsciiStrCmp (Sig, "XenHVMOVMF") == 0) { + mXenHvmloaderInfo = Info; + } else { + mXenHvmloaderInfo = NULL; + } BuildGuidDataHob ( &gEfiXenInfoGuid,
This struct is only useful to retrieve the E820 table. The mXenHvmloaderInfo isn't used yet, but will be use in a further patch to retrieve the E820 table. Also remove the unused pointer from the XenInfo HOB as that information is only useful in the XenPlatformPei. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- OvmfPkg/Include/Guid/XenInfo.h | 4 ---- OvmfPkg/PlatformPei/Xen.c | 3 --- OvmfPkg/XenPlatformPei/Xen.c | 23 ++++++++++++++++++-- 3 files changed, 21 insertions(+), 9 deletions(-)