On Thu, Feb 12, 2015 at 07:19:09PM +0800, Ard Biesheuvel wrote: > This refactors the Xen hypercall implementation that is part of the > XenBusDxe driver, in preparation of splitting it off entirely into > a XenHypercallLib library. This involves: > - removing the dependency on XENBUS_DEVICE* pointers in the XenHypercall() > prototypes > - moving the discovered hyperpage address to a global variable > - moving XenGetSharedInfoPage() to its only user XenBusDxe.c (the shared info > page is not strictly part of the Xen hypercall interface, and is not used > by other expected users of XenHypercallLib such as the Xen console version > of SerialPortLib > - reimplement XenHypercall2() in C and move the indexing of the hyperpage > there; the existing asm implementations are renamed to __XenHypercall2() and > invoked from the new C implementation. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Acked-by: Laszlo Ersek <ler...@redhat.com> > Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org> > --- > OvmfPkg/XenBusDxe/EventChannel.c | 11 +++-------- > OvmfPkg/XenBusDxe/GrantTable.c | 4 ++-- > OvmfPkg/XenBusDxe/Ia32/hypercall.nasm | 6 +++--- > OvmfPkg/XenBusDxe/X64/hypercall.nasm | 6 +++--- > OvmfPkg/XenBusDxe/XenBusDxe.c | 44 > +++++++++++++++++++++++++++++++++++++++++++- > OvmfPkg/XenBusDxe/XenBusDxe.h | 1 - > OvmfPkg/XenBusDxe/XenHypercall.c | 61 > +++++++++++++++++++++++++------------------------------------ > OvmfPkg/XenBusDxe/XenHypercall.h | 28 +++------------------------- > OvmfPkg/XenBusDxe/XenStore.c | 4 ++-- > 9 files changed, 84 insertions(+), 81 deletions(-) >
> diff --git a/OvmfPkg/XenBusDxe/XenHypercall.c > b/OvmfPkg/XenBusDxe/XenHypercall.c > index 34d92e76b7e3..19c34bdd0cec 100644 > --- a/OvmfPkg/XenBusDxe/XenHypercall.c > +++ b/OvmfPkg/XenBusDxe/XenHypercall.c > @@ -23,9 +23,21 @@ > #include <IndustryStandard/Xen/hvm/params.h> > #include <IndustryStandard/Xen/memory.h> > > +STATIC VOID *HyperPage; > + > +// > +// Interface exposed by the ASM implementation of the core hypercall > +// > +INTN > +EFIAPI > +__XenHypercall2 ( > + IN VOID *HypercallAddr, > + IN OUT INTN Arg1, > + IN OUT INTN Arg2 > + ); > + > EFI_STATUS > XenHyperpageInit ( > - IN OUT XENBUS_DEVICE *Dev > ) > { > EFI_HOB_GUID_TYPE *GuidHob; > @@ -36,24 +48,21 @@ XenHyperpageInit ( > return EFI_NOT_FOUND; > } > XenInfo = (EFI_XEN_INFO *) GET_GUID_HOB_DATA (GuidHob); > - Dev->Hyperpage = XenInfo->HyperPages; > + HyperPage = XenInfo->HyperPages; > return EFI_SUCCESS; > } > > UINT64 > XenHypercallHvmGetParam ( > - IN XENBUS_DEVICE *Dev, > IN UINT32 Index > ) > { > xen_hvm_param_t Parameter; > INTN Error; > > - ASSERT (Dev->Hyperpage != NULL); > - > Parameter.domid = DOMID_SELF; > Parameter.index = Index; > - Error = XenHypercall2 ((UINT8*)Dev->Hyperpage + __HYPERVISOR_hvm_op * 32, > + Error = XenHypercall2 (__HYPERVISOR_hvm_op, > HVMOP_get_param, (INTN) &Parameter); > if (Error != 0) { > DEBUG ((EFI_D_ERROR, > @@ -66,53 +75,33 @@ XenHypercallHvmGetParam ( > > INTN > XenHypercallMemoryOp ( > - IN XENBUS_DEVICE *Dev, > IN UINTN Operation, > IN OUT VOID *Arguments > ) > { > - ASSERT (Dev->Hyperpage != NULL); > - return XenHypercall2 ((UINT8*)Dev->Hyperpage + __HYPERVISOR_memory_op * 32, > + return XenHypercall2 (__HYPERVISOR_memory_op, > Operation, (INTN) Arguments); > } > > INTN > XenHypercallEventChannelOp ( > - IN XENBUS_DEVICE *Dev, > IN INTN Operation, > IN OUT VOID *Arguments > ) > { > - ASSERT (Dev->Hyperpage != NULL); > - return XenHypercall2 ((UINT8*)Dev->Hyperpage + > __HYPERVISOR_event_channel_op * 32, > + return XenHypercall2 (__HYPERVISOR_event_channel_op, > Operation, (INTN) Arguments); > } > > -EFI_STATUS > -XenGetSharedInfoPage ( > - IN OUT XENBUS_DEVICE *Dev > +INTN > +EFIAPI > +XenHypercall2 ( > + IN INTN HypercallID, This could be an unsigned since HypercallID should probably not be negative. Beside this comment, Reviewed-by: Anthony PERARD <anthony.per...@citrix.com> > + IN OUT INTN Arg1, > + IN OUT INTN Arg2 > ) > { > - xen_add_to_physmap_t Parameter; > + ASSERT (HyperPage != NULL); > > - ASSERT (Dev->SharedInfo == NULL); > - > - Parameter.domid = DOMID_SELF; > - Parameter.space = XENMAPSPACE_shared_info; > - Parameter.idx = 0; > - > - // > - // using reserved page because the page is not released when Linux is > - // starting because of the add_to_physmap. QEMU might try to access the > - // page, and fail because it have no right to do so (segv). > - // > - Dev->SharedInfo = AllocateReservedPages (1); > - Parameter.gpfn = (UINTN) Dev->SharedInfo >> EFI_PAGE_SHIFT; > - if (XenHypercallMemoryOp (Dev, XENMEM_add_to_physmap, &Parameter) != 0) { > - FreePages (Dev->SharedInfo, 1); > - Dev->SharedInfo = NULL; > - return EFI_LOAD_ERROR; > - } > - > - return EFI_SUCCESS; > + return __XenHypercall2 ((UINT8*)HyperPage + HypercallID * 32, Arg1, Arg2); > } -- Anthony PERARD _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel