| Message ID | 20190409110844.14746-22-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: > .. when hvmloader haven't runned before OVMF. The only way left to get > an E820 table is to ask Xen via an hypercall, the call can only be made > once so keep the result cached for later. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> > --- > OvmfPkg/XenPlatformPei/Xen.c | 46 +++++++++++++++++++- > 1 file changed, 45 insertions(+), 1 deletion(-) (1) Please change the subject to OvmfPkg/XenPlatformPei: no hvmloader: get the E820 table via hypercall (70 characters); and pls write a self-contained commit message body. (2) s/haven't runned/hasn't run/ > > diff --git a/OvmfPkg/XenPlatformPei/Xen.c b/OvmfPkg/XenPlatformPei/Xen.c > index 23ff3102b5..f8cee671d8 100644 > --- a/OvmfPkg/XenPlatformPei/Xen.c > +++ b/OvmfPkg/XenPlatformPei/Xen.c > @@ -33,6 +33,7 @@ > #include <Library/MtrrLib.h> > #include <IndustryStandard/Xen/arch-x86/hvm/start_info.h> > #include <Library/XenHypercallLib.h> > +#include <IndustryStandard/Xen/memory.h> > > #include "Platform.h" > #include "Xen.h" > @@ -46,6 +47,8 @@ EFI_XEN_INFO mXenInfo; > // Only the E820 table is used by OVMF. > // > EFI_XEN_OVMF_INFO *mXenHvmloaderInfo; > +EFI_E820_ENTRY64 E820Entries[128]; > +UINT32 E820EntriesCount; (3) Please prefix both of these global variables with "m". (I'd also recommend STATIC.) > > /** > Returns E820 map provided by Xen > @@ -61,6 +64,12 @@ XenGetE820Map ( > UINT32 *Count > ) > { > + INTN ReturnCode; > + xen_memory_map_t Parameters; > + UINTN LoopIndex; > + UINTN Index; > + EFI_E820_ENTRY64 TmpEntry; > + > // > // Get E820 produced by hvmloader > // > @@ -72,7 +81,42 @@ XenGetE820Map ( > return EFI_SUCCESS; > } > > - return EFI_NOT_FOUND; > + // > + // Otherwise, get the E820 table from the Xen hypervisor > + // > + > + if (E820EntriesCount > 0) { > + *Entries = E820Entries; > + *Count = E820EntriesCount; > + return EFI_SUCCESS; > + } > + > + Parameters.nr_entries = 128; > + set_xen_guest_handle (Parameters.buffer, E820Entries); > + > + // Returns a errno > + ReturnCode = XenHypercallMemoryOp (XENMEM_memory_map, &Parameters); > + ASSERT (ReturnCode == 0); > + > + E820EntriesCount = Parameters.nr_entries; > + > + // > + // Sort E820 entries > + // > + for (LoopIndex = 1; LoopIndex < E820EntriesCount; LoopIndex++) { > + for (Index = LoopIndex; Index < E820EntriesCount; Index++) { > + if (E820Entries[Index - 1].BaseAddr > E820Entries[Index].BaseAddr) { > + TmpEntry = E820Entries[Index]; > + E820Entries[Index] = E820Entries[Index - 1]; > + E820Entries[Index - 1] = TmpEntry; > + } > + } > + } > + > + *Count = E820EntriesCount; > + *Entries = E820Entries; > + > + return EFI_SUCCESS; > } > > /** > "xen_memory_map_t" and "set_xen_guest_handle" look really foreign in edk2, but I guess we'll have to live with them. With (1) through (3) updated: Acked-by: Laszlo Ersek <lersek@redhat.com> Thanks Laszlo
On 12/04/2019 10:33, Laszlo Ersek wrote: > On 04/09/19 13:08, Anthony PERARD wrote: >> @@ -72,7 +81,42 @@ XenGetE820Map ( >> return EFI_SUCCESS; >> } >> >> - return EFI_NOT_FOUND; >> + // >> + // Otherwise, get the E820 table from the Xen hypervisor >> + // >> + >> + if (E820EntriesCount > 0) { >> + *Entries = E820Entries; >> + *Count = E820EntriesCount; >> + return EFI_SUCCESS; >> + } >> + >> + Parameters.nr_entries = 128; >> + set_xen_guest_handle (Parameters.buffer, E820Entries); >> + >> + // Returns a errno >> + ReturnCode = XenHypercallMemoryOp (XENMEM_memory_map, &Parameters); >> + ASSERT (ReturnCode == 0); >> + >> + E820EntriesCount = Parameters.nr_entries; >> + >> + // >> + // Sort E820 entries >> + // >> + for (LoopIndex = 1; LoopIndex < E820EntriesCount; LoopIndex++) { >> + for (Index = LoopIndex; Index < E820EntriesCount; Index++) { >> + if (E820Entries[Index - 1].BaseAddr > E820Entries[Index].BaseAddr) { >> + TmpEntry = E820Entries[Index]; >> + E820Entries[Index] = E820Entries[Index - 1]; >> + E820Entries[Index - 1] = TmpEntry; >> + } >> + } >> + } >> + >> + *Count = E820EntriesCount; >> + *Entries = E820Entries; >> + >> + return EFI_SUCCESS; >> } >> >> /** >> > "xen_memory_map_t" and "set_xen_guest_handle" look really foreign in edk2, but I guess we'll have to live with them. For historical reasons, the ABI was described using C structures (and in hindsight, a very poor idea, seeing as it all went to pot when "unsigned long" changed size for 64bit), and the expectation was that every downstream project wanting to make hypercalls would copy the "canonical copy" of the ABI, which turns it into a defacto API as well. However, there is absolutely nothing stopping a downstream project deciding not to take this route, and having Xen's ABI expressed in their local style. Ultimately, that is down to the maintainers of the project to decide which approach they would prefer. ~Andrew
diff --git a/OvmfPkg/XenPlatformPei/Xen.c b/OvmfPkg/XenPlatformPei/Xen.c index 23ff3102b5..f8cee671d8 100644 --- a/OvmfPkg/XenPlatformPei/Xen.c +++ b/OvmfPkg/XenPlatformPei/Xen.c @@ -33,6 +33,7 @@ #include <Library/MtrrLib.h> #include <IndustryStandard/Xen/arch-x86/hvm/start_info.h> #include <Library/XenHypercallLib.h> +#include <IndustryStandard/Xen/memory.h> #include "Platform.h" #include "Xen.h" @@ -46,6 +47,8 @@ EFI_XEN_INFO mXenInfo; // Only the E820 table is used by OVMF. // EFI_XEN_OVMF_INFO *mXenHvmloaderInfo; +EFI_E820_ENTRY64 E820Entries[128]; +UINT32 E820EntriesCount; /** Returns E820 map provided by Xen @@ -61,6 +64,12 @@ XenGetE820Map ( UINT32 *Count ) { + INTN ReturnCode; + xen_memory_map_t Parameters; + UINTN LoopIndex; + UINTN Index; + EFI_E820_ENTRY64 TmpEntry; + // // Get E820 produced by hvmloader // @@ -72,7 +81,42 @@ XenGetE820Map ( return EFI_SUCCESS; } - return EFI_NOT_FOUND; + // + // Otherwise, get the E820 table from the Xen hypervisor + // + + if (E820EntriesCount > 0) { + *Entries = E820Entries; + *Count = E820EntriesCount; + return EFI_SUCCESS; + } + + Parameters.nr_entries = 128; + set_xen_guest_handle (Parameters.buffer, E820Entries); + + // Returns a errno + ReturnCode = XenHypercallMemoryOp (XENMEM_memory_map, &Parameters); + ASSERT (ReturnCode == 0); + + E820EntriesCount = Parameters.nr_entries; + + // + // Sort E820 entries + // + for (LoopIndex = 1; LoopIndex < E820EntriesCount; LoopIndex++) { + for (Index = LoopIndex; Index < E820EntriesCount; Index++) { + if (E820Entries[Index - 1].BaseAddr > E820Entries[Index].BaseAddr) { + TmpEntry = E820Entries[Index]; + E820Entries[Index] = E820Entries[Index - 1]; + E820Entries[Index - 1] = TmpEntry; + } + } + } + + *Count = E820EntriesCount; + *Entries = E820Entries; + + return EFI_SUCCESS; } /**
.. when hvmloader haven't runned before OVMF. The only way left to get an E820 table is to ask Xen via an hypercall, the call can only be made once so keep the result cached for later. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> --- OvmfPkg/XenPlatformPei/Xen.c | 46 +++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-)