| Message ID | 1496769929-23355-7-git-send-email-bhupinder.thakur@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | PL011 emulation support in Xen | expand |
On Tue, 6 Jun 2017, Bhupinder Thakur wrote: > Add a new domctl API to initialize vpl011. It takes the GFN and console > backend domid as input and returns an event channel to be used for > sending and receiving events from Xen. > > Xen will communicate with xenconsole using GFN as the ring buffer and > the event channel to transmit and receive pl011 data on the guest domain's > behalf. > > Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org> > --- > CC: ij > CC: wl > CC: ss > CC: jg > > Changes since v3: > - Added a new arch specific function libxl__arch_domain_create_finish(), which > calls the vpl011 initialization function. For x86 this function does not do > anything. > - domain_vpl011_init() takes a pointer to a structure which contains all the > required information such as console_domid, gfn instead of passing parameters > separately. > - Dropped a DOMCTL API defined for de-initializing vpl011 as that should be > taken care when the domain is destroyed (and not dependent on userspace > libraries/applications). > > Changes since v2: > - Replaced the DOMCTL APIs defined for get/set of event channel and GFN with > a set of DOMCTL APIs for initializing and de-initializing vpl011 emulation. > > tools/libxc/include/xenctrl.h | 17 +++++++++++++++++ > tools/libxc/xc_domain.c | 23 ++++++++++++++++++++++ > tools/libxl/libxl_arch.h | 7 +++++++ > tools/libxl/libxl_arm.c | 19 +++++++++++++++++++ > tools/libxl/libxl_dom.c | 6 +++++- > tools/libxl/libxl_x86.c | 8 ++++++++ > xen/arch/arm/domain.c | 2 ++ > xen/arch/arm/domctl.c | 44 ++++++++++++++++++++++++++++++++++++++++--- > xen/include/public/domctl.h | 12 ++++++++++++ > 9 files changed, 134 insertions(+), 4 deletions(-) > > diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h > index 1629f41..77425dd 100644 > --- a/tools/libxc/include/xenctrl.h > +++ b/tools/libxc/include/xenctrl.h > @@ -884,6 +884,23 @@ int xc_vcpu_getcontext(xc_interface *xch, > uint32_t domid, > uint32_t vcpu, > vcpu_guest_context_any_t *ctxt); > +/** > + * This function initializes the vpl011 emulation and returns > + * the event to be used by the backend for communicating with > + * the emulation code. > + * > + * @parm xch a handle to an open hypervisor interface > + * @parm domid the domain to get information from > + * @parm console_domid the domid of the backend console > + * @parm gfn the guest pfn to be used as the ring buffer > + * @parm evtchn the event channel to be used for events > + * @return 0 on success, negative error on failure > + */ > +int xc_dom_vpl011_init(xc_interface *xch, > + uint32_t domid, > + uint32_t console_domid, > + xen_pfn_t gfn, > + evtchn_port_t *evtchn); > > /** > * This function returns information about the XSAVE state of a particular > diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c > index 00909ad4..a8efd5e 100644 > --- a/tools/libxc/xc_domain.c > +++ b/tools/libxc/xc_domain.c > @@ -343,6 +343,29 @@ int xc_domain_get_guest_width(xc_interface *xch, uint32_t domid, > return 0; > } > > +int xc_dom_vpl011_init(xc_interface *xch, > + uint32_t domid, > + uint32_t console_domid, > + xen_pfn_t gfn, > + evtchn_port_t *evtchn) > +{ > + DECLARE_DOMCTL; > + int rc = 0; > + > + domctl.cmd = XEN_DOMCTL_vuart_op; > + domctl.domain = (domid_t)domid; > + domctl.u.vuart_op.cmd = XEN_DOMCTL_VUART_OP_INIT_VPL011; > + domctl.u.vuart_op.console_domid = console_domid; > + domctl.u.vuart_op.gfn = gfn; > + > + if ( (rc = do_domctl(xch, &domctl)) < 0 ) > + return rc; > + > + *evtchn = domctl.u.vuart_op.evtchn; > + > + return rc; > +} It looks like this function should be in one of the arm specific files, such as xc_dom_arm.c (otherwise it becomes available to x86 too). > int xc_domain_getinfo(xc_interface *xch, > uint32_t first_domid, > unsigned int max_doms, > diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h > index 5e1fc60..d1ca9c6 100644 > --- a/tools/libxl/libxl_arch.h > +++ b/tools/libxl/libxl_arch.h > @@ -32,6 +32,13 @@ _hidden > int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config, > uint32_t domid); > > +/* arch specific internal domain creation finish function */ > +_hidden > +int libxl__arch_domain_create_finish(libxl__gc *gc, > + libxl_domain_build_info *info, > + uint32_t domid, > + libxl__domain_build_state *state); > + > /* setup arch specific hardware description, i.e. DTB on ARM */ > _hidden > int libxl__arch_domain_init_hw_description(libxl__gc *gc, > diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c > index d842d88..b60dfa9 100644 > --- a/tools/libxl/libxl_arm.c > +++ b/tools/libxl/libxl_arm.c > @@ -106,6 +106,25 @@ int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config, > return 0; > } > > +int libxl__arch_domain_create_finish(libxl__gc *gc, > + libxl_domain_build_info *info, > + uint32_t domid, > + libxl__domain_build_state *state) > +{ > + int ret = 0; > + > + if ( info->arch_arm.vuart && > + (ret = xc_dom_vpl011_init(CTX->xch, > + domid, > + state->console_domid, > + xc_get_vuart_gfn(), > + &state->vuart_port)) != 0 ) { > + LOG(ERROR, "xc_dom_vpl011_init failed\n"); > + } > + > + return ret; > +} > + > int libxl__arch_extra_memory(libxl__gc *gc, > const libxl_domain_build_info *info, > uint64_t *out) > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c > index 5d914a5..187c5bd 100644 > --- a/tools/libxl/libxl_dom.c > +++ b/tools/libxl/libxl_dom.c > @@ -587,7 +587,10 @@ retry_transaction: > goto retry_transaction; > xs_introduce_domain(ctx->xsh, domid, state->store_mfn, state->store_port); > free(vm_path); > - return 0; > + > + rc = libxl__arch_domain_create_finish(gc, info, domid, state); > + > + return rc; > } > > static int set_vnuma_info(libxl__gc *gc, uint32_t domid, > @@ -788,6 +791,7 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid, > if (xc_dom_translated(dom)) { > state->console_mfn = dom->console_pfn; > state->store_mfn = dom->xenstore_pfn; > + state->vuart_gfn = dom->vuart_gfn; > } else { > state->console_mfn = xc_dom_p2m(dom, dom->console_pfn); > state->store_mfn = xc_dom_p2m(dom, dom->xenstore_pfn); vuart_gfn was introduced in patch #4, why are we setting it only now? > diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c > index 455f6f0..3544028 100644 > --- a/tools/libxl/libxl_x86.c > +++ b/tools/libxl/libxl_x86.c > @@ -358,6 +358,14 @@ out: > return ret; > } > > +int libxl__arch_domain_create_finish(libxl__gc *gc, > + libxl_domain_build_info *info, > + uint32_t domid, > + libxl__domain_build_state *state) > +{ > + return 0; > +} > + > int libxl__arch_extra_memory(libxl__gc *gc, > const libxl_domain_build_info *info, > uint64_t *out) > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c > index 76310ed..9e150ba 100644 > --- a/xen/arch/arm/domain.c > +++ b/xen/arch/arm/domain.c > @@ -665,6 +665,8 @@ fail: > > void arch_domain_destroy(struct domain *d) > { > + domain_vpl011_deinit(d); > + > /* IOMMU page table is shared with P2M, always call > * iommu_domain_destroy() before p2m_teardown(). > */ I cannot find the definition of domain_vpl011_deinit > diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c > index 971caec..741679b 100644 > --- a/xen/arch/arm/domctl.c > +++ b/xen/arch/arm/domctl.c > @@ -5,13 +5,15 @@ > */ > > #include <xen/types.h> > -#include <xen/lib.h> > +#include <public/domctl.h> > #include <xen/errno.h> > -#include <xen/sched.h> > +#include <xen/guest_access.h> > #include <xen/hypercall.h> > #include <xen/iocap.h> > +#include <xen/lib.h> > +#include <xen/mm.h> > +#include <xen/sched.h> > #include <xsm/xsm.h> > -#include <public/domctl.h> > > void arch_get_domain_info(const struct domain *d, > struct xen_domctl_getdomaininfo *info) > @@ -119,6 +121,42 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d, > d->disable_migrate = domctl->u.disable_migrate.disable; > return 0; > > + case XEN_DOMCTL_vuart_op: > + { > + int rc; > + struct xen_domctl_vuart_op *vuart_op = &domctl->u.vuart_op; > + > + switch(vuart_op->cmd) > + { > + case XEN_DOMCTL_VUART_OP_INIT_VPL011: > + > + if ( !d->creation_finished ) > + { > + struct vpl011_init_info info; > + > + info.console_domid = vuart_op->console_domid; > + info.gfn = _gfn(vuart_op->gfn); > + > + rc = domain_vpl011_init(d, &info); > + if ( !rc ) > + { > + vuart_op->evtchn = info.evtchn; > + rc = __copy_to_guest(u_domctl, domctl, 1); > + } > + } > + else > + { > + rc = - EPERM; > + } > + break; > + > + default: > + rc = -EINVAL; > + break; > + } > + > + return rc; > + } > default: > { > int rc; > diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h > index e6cf211..c6ff458 100644 > --- a/xen/include/public/domctl.h > +++ b/xen/include/public/domctl.h > @@ -36,6 +36,7 @@ > #include "grant_table.h" > #include "hvm/save.h" > #include "memory.h" > +#include "event_channel.h" > > #define XEN_DOMCTL_INTERFACE_VERSION 0x0000000d > > @@ -1138,6 +1139,15 @@ struct xen_domctl_psr_cat_op { > uint32_t target; /* IN */ > uint64_t data; /* IN/OUT */ > }; > + > +struct xen_domctl_vuart_op { > +#define XEN_DOMCTL_VUART_OP_INIT_VPL011 0 > + uint32_t cmd; /* XEN_DOMCTL_VUART_OP_* */ > + uint32_t console_domid; /* IN */ > + xen_pfn_t gfn; /* IN */ > + evtchn_port_t evtchn; /* OUT */ > +}; > + > typedef struct xen_domctl_psr_cat_op xen_domctl_psr_cat_op_t; > DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cat_op_t); > > @@ -1218,6 +1228,7 @@ struct xen_domctl { > #define XEN_DOMCTL_monitor_op 77 > #define XEN_DOMCTL_psr_cat_op 78 > #define XEN_DOMCTL_soft_reset 79 > +#define XEN_DOMCTL_vuart_op 80 > #define XEN_DOMCTL_gdbsx_guestmemio 1000 > #define XEN_DOMCTL_gdbsx_pausevcpu 1001 > #define XEN_DOMCTL_gdbsx_unpausevcpu 1002 > @@ -1280,6 +1291,7 @@ struct xen_domctl { > struct xen_domctl_psr_cmt_op psr_cmt_op; > struct xen_domctl_monitor_op monitor_op; > struct xen_domctl_psr_cat_op psr_cat_op; > + struct xen_domctl_vuart_op vuart_op; > uint8_t pad[128]; > } u; > }; > -- > 2.7.4 >
Hi Stefano, On 07/06/17 00:26, Stefano Stabellini wrote: >> diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c >> index 00909ad4..a8efd5e 100644 >> --- a/tools/libxc/xc_domain.c >> +++ b/tools/libxc/xc_domain.c >> @@ -343,6 +343,29 @@ int xc_domain_get_guest_width(xc_interface *xch, uint32_t domid, >> return 0; >> } >> >> +int xc_dom_vpl011_init(xc_interface *xch, >> + uint32_t domid, >> + uint32_t console_domid, >> + xen_pfn_t gfn, >> + evtchn_port_t *evtchn) >> +{ >> + DECLARE_DOMCTL; >> + int rc = 0; >> + >> + domctl.cmd = XEN_DOMCTL_vuart_op; >> + domctl.domain = (domid_t)domid; >> + domctl.u.vuart_op.cmd = XEN_DOMCTL_VUART_OP_INIT_VPL011; >> + domctl.u.vuart_op.console_domid = console_domid; >> + domctl.u.vuart_op.gfn = gfn; >> + >> + if ( (rc = do_domctl(xch, &domctl)) < 0 ) >> + return rc; >> + >> + *evtchn = domctl.u.vuart_op.evtchn; >> + >> + return rc; >> +} > > It looks like this function should be in one of the arm specific files, > such as xc_dom_arm.c (otherwise it becomes available to x86 too). AFAICT xc_dom_arm.c has a completely different purpose. Looking at other helpers, it seems the usage if too #ifdef helpers (see xc_vcpu_get_extstate or xc_domain_set_memory_map). [...] >> +int libxl__arch_domain_create_finish(libxl__gc *gc, >> + libxl_domain_build_info *info, >> + uint32_t domid, >> + libxl__domain_build_state *state) >> +{ >> + return 0; >> +} >> + >> int libxl__arch_extra_memory(libxl__gc *gc, >> const libxl_domain_build_info *info, >> uint64_t *out) >> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c >> index 76310ed..9e150ba 100644 >> --- a/xen/arch/arm/domain.c >> +++ b/xen/arch/arm/domain.c >> @@ -665,6 +665,8 @@ fail: >> >> void arch_domain_destroy(struct domain *d) >> { >> + domain_vpl011_deinit(d); >> + >> /* IOMMU page table is shared with P2M, always call >> * iommu_domain_destroy() before p2m_teardown(). >> */ > > I cannot find the definition of domain_vpl011_deinit See patch #3. Cheers,
Hi Bhupinder, On 06/06/17 18:25, Bhupinder Thakur wrote: > diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h > index 1629f41..77425dd 100644 > --- a/tools/libxc/include/xenctrl.h > +++ b/tools/libxc/include/xenctrl.h > @@ -884,6 +884,23 @@ int xc_vcpu_getcontext(xc_interface *xch, > uint32_t domid, > uint32_t vcpu, > vcpu_guest_context_any_t *ctxt); Newline here please. [...] > diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h > index 5e1fc60..d1ca9c6 100644 > --- a/tools/libxl/libxl_arch.h > +++ b/tools/libxl/libxl_arch.h > @@ -32,6 +32,13 @@ _hidden > int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config, > uint32_t domid); > > +/* arch specific internal domain creation finish function */ > +_hidden > +int libxl__arch_domain_create_finish(libxl__gc *gc, > + libxl_domain_build_info *info, > + uint32_t domid, > + libxl__domain_build_state *state); Can you explain why you need a new arch helper rather than using the current one? [...] > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c > index 76310ed..9e150ba 100644 > --- a/xen/arch/arm/domain.c > +++ b/xen/arch/arm/domain.c > @@ -665,6 +665,8 @@ fail: > > void arch_domain_destroy(struct domain *d) > { > + domain_vpl011_deinit(d); Please add a comment explain where the initialization has been done (i.e via a DOMCTL). This would make easier to know what's going on. > + > /* IOMMU page table is shared with P2M, always call > * iommu_domain_destroy() before p2m_teardown(). > */ > diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c > index 971caec..741679b 100644 > --- a/xen/arch/arm/domctl.c > +++ b/xen/arch/arm/domctl.c > @@ -5,13 +5,15 @@ > */ > > #include <xen/types.h> > -#include <xen/lib.h> > +#include <public/domctl.h> > #include <xen/errno.h> > -#include <xen/sched.h> > +#include <xen/guest_access.h> > #include <xen/hypercall.h> > #include <xen/iocap.h> > +#include <xen/lib.h> > +#include <xen/mm.h> > +#include <xen/sched.h> > #include <xsm/xsm.h> > -#include <public/domctl.h> Why do you reshuffle the headers? Is it to use the alphabetical order? If so, this should really be done in a separate patch. > > void arch_get_domain_info(const struct domain *d, > struct xen_domctl_getdomaininfo *info) > @@ -119,6 +121,42 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d, > d->disable_migrate = domctl->u.disable_migrate.disable; > return 0; > > + case XEN_DOMCTL_vuart_op: > + { > + int rc; > + struct xen_domctl_vuart_op *vuart_op = &domctl->u.vuart_op; > + > + switch(vuart_op->cmd) > + { > + case XEN_DOMCTL_VUART_OP_INIT_VPL011: > + > + if ( !d->creation_finished ) > + { > + struct vpl011_init_info info; > + > + info.console_domid = vuart_op->console_domid; > + info.gfn = _gfn(vuart_op->gfn); > + > + rc = domain_vpl011_init(d, &info); > + if ( !rc ) > + { > + vuart_op->evtchn = info.evtchn; > + rc = __copy_to_guest(u_domctl, domctl, 1); > + } > + } > + else > + { > + rc = - EPERM; > + } Unecessary {}. > + break; > + > + default: > + rc = -EINVAL; > + break; > + } > + > + return rc; > + } > default: > { > int rc; Cheers,
On Fri, 9 Jun 2017, Julien Grall wrote: > Hi Stefano, > > On 07/06/17 00:26, Stefano Stabellini wrote: > > > diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c > > > index 00909ad4..a8efd5e 100644 > > > --- a/tools/libxc/xc_domain.c > > > +++ b/tools/libxc/xc_domain.c > > > @@ -343,6 +343,29 @@ int xc_domain_get_guest_width(xc_interface *xch, > > > uint32_t domid, > > > return 0; > > > } > > > > > > +int xc_dom_vpl011_init(xc_interface *xch, > > > + uint32_t domid, > > > + uint32_t console_domid, > > > + xen_pfn_t gfn, > > > + evtchn_port_t *evtchn) > > > +{ > > > + DECLARE_DOMCTL; > > > + int rc = 0; > > > + > > > + domctl.cmd = XEN_DOMCTL_vuart_op; > > > + domctl.domain = (domid_t)domid; > > > + domctl.u.vuart_op.cmd = XEN_DOMCTL_VUART_OP_INIT_VPL011; > > > + domctl.u.vuart_op.console_domid = console_domid; > > > + domctl.u.vuart_op.gfn = gfn; > > > + > > > + if ( (rc = do_domctl(xch, &domctl)) < 0 ) > > > + return rc; > > > + > > > + *evtchn = domctl.u.vuart_op.evtchn; > > > + > > > + return rc; > > > +} > > > > It looks like this function should be in one of the arm specific files, > > such as xc_dom_arm.c (otherwise it becomes available to x86 too). > > AFAICT xc_dom_arm.c has a completely different purpose. Looking at other > helpers, it seems the usage if too #ifdef helpers (see xc_vcpu_get_extstate or > xc_domain_set_memory_map). That's true. It's best to continue that pattern and use #ifdefs here. > > > +int libxl__arch_domain_create_finish(libxl__gc *gc, > > > + libxl_domain_build_info *info, > > > + uint32_t domid, > > > + libxl__domain_build_state *state) > > > +{ > > > + return 0; > > > +} > > > + > > > int libxl__arch_extra_memory(libxl__gc *gc, > > > const libxl_domain_build_info *info, > > > uint64_t *out) > > > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c > > > index 76310ed..9e150ba 100644 > > > --- a/xen/arch/arm/domain.c > > > +++ b/xen/arch/arm/domain.c > > > @@ -665,6 +665,8 @@ fail: > > > > > > void arch_domain_destroy(struct domain *d) > > > { > > > + domain_vpl011_deinit(d); > > > + > > > /* IOMMU page table is shared with P2M, always call > > > * iommu_domain_destroy() before p2m_teardown(). > > > */ > > > > I cannot find the definition of domain_vpl011_deinit > > See patch #3. All right, thanks. Initially I thought it was weird for this change to be here, but now I think it makes sense because this patch introduces the call to domain_vpl011_init.
Hi Stefano, On 7 June 2017 at 04:56, Stefano Stabellini <sstabellini@kernel.org> wrote: >> static int set_vnuma_info(libxl__gc *gc, uint32_t domid, >> @@ -788,6 +791,7 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid, >> if (xc_dom_translated(dom)) { >> state->console_mfn = dom->console_pfn; >> state->store_mfn = dom->xenstore_pfn; >> + state->vuart_gfn = dom->vuart_gfn; >> } else { >> state->console_mfn = xc_dom_p2m(dom, dom->console_pfn); >> state->store_mfn = xc_dom_p2m(dom, dom->xenstore_pfn); > > vuart_gfn was introduced in patch #4, why are we setting it only now? I think this change can be moved to patch #5, which initializes dom->vuart_gfn, which is required for this change. Regards, Bhupinder
On 14 June 2017 at 13:05, Bhupinder Thakur <bhupinder.thakur@linaro.org> wrote: > Hi Stefano, > > On 7 June 2017 at 04:56, Stefano Stabellini <sstabellini@kernel.org> wrote: >>> static int set_vnuma_info(libxl__gc *gc, uint32_t domid, >>> @@ -788,6 +791,7 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid, >>> if (xc_dom_translated(dom)) { >>> state->console_mfn = dom->console_pfn; >>> state->store_mfn = dom->xenstore_pfn; >>> + state->vuart_gfn = dom->vuart_gfn; >>> } else { >>> state->console_mfn = xc_dom_p2m(dom, dom->console_pfn); >>> state->store_mfn = xc_dom_p2m(dom, dom->xenstore_pfn); >> >> vuart_gfn was introduced in patch #4, why are we setting it only now? > > I think this change can be moved to patch #5, which initializes > dom->vuart_gfn, which is required for this change. I will move this change to patch #4 only and move patch #4 after patch #5. Regards, Bhupinder
Hi Julien, >> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h >> index 1629f41..77425dd 100644 >> --- a/tools/libxc/include/xenctrl.h >> +++ b/tools/libxc/include/xenctrl.h >> @@ -884,6 +884,23 @@ int xc_vcpu_getcontext(xc_interface *xch, >> uint32_t domid, >> uint32_t vcpu, >> vcpu_guest_context_any_t *ctxt); > > > Newline here please. > ok. > [...] > >> diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h >> index 5e1fc60..d1ca9c6 100644 >> --- a/tools/libxl/libxl_arch.h >> +++ b/tools/libxl/libxl_arch.h >> @@ -32,6 +32,13 @@ _hidden >> int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config >> *d_config, >> uint32_t domid); >> >> +/* arch specific internal domain creation finish function */ >> +_hidden >> +int libxl__arch_domain_create_finish(libxl__gc *gc, >> + libxl_domain_build_info *info, >> + uint32_t domid, >> + libxl__domain_build_state *state); > > > Can you explain why you need a new arch helper rather than using the current > one? libxl__arch_domain_create() is called from libxl__build_pre(). This function is called before libxl__build_pv(). By this time the domain has not be created and I found that if I tried to initialize vpl011 from inside libxl__arch_domain_create() then initialization was failing due to prepare_ring_for_helper() failing. So I had to create another function which will be called from libxl__build_post() after domain has been setup. > > [...] > >> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c >> index 76310ed..9e150ba 100644 >> --- a/xen/arch/arm/domain.c >> +++ b/xen/arch/arm/domain.c >> @@ -665,6 +665,8 @@ fail: >> >> void arch_domain_destroy(struct domain *d) >> { >> + domain_vpl011_deinit(d); > > > Please add a comment explain where the initialization has been done (i.e via > a DOMCTL). This would make easier to know what's going on. ok. > >> + >> /* IOMMU page table is shared with P2M, always call >> * iommu_domain_destroy() before p2m_teardown(). >> */ >> diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c >> index 971caec..741679b 100644 >> --- a/xen/arch/arm/domctl.c >> +++ b/xen/arch/arm/domctl.c >> @@ -5,13 +5,15 @@ >> */ >> >> #include <xen/types.h> >> -#include <xen/lib.h> >> +#include <public/domctl.h> >> #include <xen/errno.h> >> -#include <xen/sched.h> >> +#include <xen/guest_access.h> >> #include <xen/hypercall.h> >> #include <xen/iocap.h> >> +#include <xen/lib.h> >> +#include <xen/mm.h> >> +#include <xen/sched.h> >> #include <xsm/xsm.h> >> -#include <public/domctl.h> > > > Why do you reshuffle the headers? Is it to use the alphabetical order? If > so, this should really be done in a separate patch. > ok. I will introduce a new patch for header files reshuffling. > >> >> void arch_get_domain_info(const struct domain *d, >> struct xen_domctl_getdomaininfo *info) >> @@ -119,6 +121,42 @@ long arch_do_domctl(struct xen_domctl *domctl, struct >> domain *d, >> d->disable_migrate = domctl->u.disable_migrate.disable; >> return 0; >> >> + case XEN_DOMCTL_vuart_op: >> + { >> + int rc; >> + struct xen_domctl_vuart_op *vuart_op = &domctl->u.vuart_op; >> + >> + switch(vuart_op->cmd) >> + { >> + case XEN_DOMCTL_VUART_OP_INIT_VPL011: >> + >> + if ( !d->creation_finished ) >> + { >> + struct vpl011_init_info info; >> + >> + info.console_domid = vuart_op->console_domid; >> + info.gfn = _gfn(vuart_op->gfn); >> + >> + rc = domain_vpl011_init(d, &info); >> + if ( !rc ) >> + { >> + vuart_op->evtchn = info.evtchn; >> + rc = __copy_to_guest(u_domctl, domctl, 1); >> + } >> + } >> + else >> + { >> + rc = - EPERM; >> + } > > > Unecessary {}. > ok. Regards, Bhupinder
On 06/14/2017 10:16 AM, Bhupinder Thakur wrote: > Hi Julien, Hi Bhupinder, > >>> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h >>> index 1629f41..77425dd 100644 >>> --- a/tools/libxc/include/xenctrl.h >>> +++ b/tools/libxc/include/xenctrl.h >>> @@ -884,6 +884,23 @@ int xc_vcpu_getcontext(xc_interface *xch, >>> uint32_t domid, >>> uint32_t vcpu, >>> vcpu_guest_context_any_t *ctxt); >> >> >> Newline here please. >> > ok. >> [...] >> >>> diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h >>> index 5e1fc60..d1ca9c6 100644 >>> --- a/tools/libxl/libxl_arch.h >>> +++ b/tools/libxl/libxl_arch.h >>> @@ -32,6 +32,13 @@ _hidden >>> int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config >>> *d_config, >>> uint32_t domid); >>> >>> +/* arch specific internal domain creation finish function */ >>> +_hidden >>> +int libxl__arch_domain_create_finish(libxl__gc *gc, >>> + libxl_domain_build_info *info, >>> + uint32_t domid, >>> + libxl__domain_build_state *state); >> >> >> Can you explain why you need a new arch helper rather than using the current >> one? > > libxl__arch_domain_create() is called from libxl__build_pre(). This > function is called before libxl__build_pv(). By this time the domain > has not be created and I found that if I tried to initialize vpl011 > from inside libxl__arch_domain_create() then initialization was > failing due to prepare_ring_for_helper() failing. What do you mean by the domain has not been created? The domain has already been created (you have a domid in hand) when you libxl__build_pre. So the problem is different. Looking at the code, I guess the problem is because the vuart pfn will be allocated by xc_dom_build_image called by libxl_build_pv -> libxl__build_dom. > > So I had to create another function which will be called from > libxl__build_post() after domain has been setup. It looks a bit odd to me to create the vpl011 UART that late in the process because when you read the code you would expect all the hardware to be setup after libxl__arch_domain_finalise_hw_descriptions is called. But I understand it is not possible to do it as the ring has not yet been allocated. So is there a way to allocate the ring before? Wei, Ian, do you have any opinions on what should the workflow in libxl? Cheers,
Hi Julien, >>>> diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h >>>> index 5e1fc60..d1ca9c6 100644 >>>> --- a/tools/libxl/libxl_arch.h >>>> +++ b/tools/libxl/libxl_arch.h >>>> @@ -32,6 +32,13 @@ _hidden >>>> int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config >>>> *d_config, >>>> uint32_t domid); >>>> >>>> +/* arch specific internal domain creation finish function */ >>>> +_hidden >>>> +int libxl__arch_domain_create_finish(libxl__gc *gc, >>>> + libxl_domain_build_info *info, >>>> + uint32_t domid, >>>> + libxl__domain_build_state *state); >>> >>> >>> >>> Can you explain why you need a new arch helper rather than using the >>> current >>> one? >> >> >> libxl__arch_domain_create() is called from libxl__build_pre(). This >> function is called before libxl__build_pv(). By this time the domain >> has not be created and I found that if I tried to initialize vpl011 >> from inside libxl__arch_domain_create() then initialization was >> failing due to prepare_ring_for_helper() failing. > > > What do you mean by the domain has not been created? The domain has already > been created (you have a domid in hand) when you libxl__build_pre. So the > problem is different. > > Looking at the code, I guess the problem is because the vuart pfn will be > allocated by xc_dom_build_image called by libxl_build_pv -> > libxl__build_dom. > >> >> So I had to create another function which will be called from >> libxl__build_post() after domain has been setup. > > > It looks a bit odd to me to create the vpl011 UART that late in the process > because when you read the code you would expect all the hardware to be setup > after libxl__arch_domain_finalise_hw_descriptions is called. > > But I understand it is not possible to do it as the ring has not yet been > allocated. So is there a way to allocate the ring before? > > Wei, Ian, do you have any opinions on what should the workflow in libxl? Actually, I had introduced an API xc_get_vuart_gfn() to get the pfn. Since it is a fixed pfn, the API can return it even before xc_build_dom_image() is called. I tried after moving the vpl011_init function to libxl__arch_domain_create() and it is working. Regards, Bhupinder
Hi Julien, I was mistaken in my earlier mail about vpl011 init working if it is moved to libxl__arch_domain_create(). It is failing because as you have mentioned vuart_pfn is allocated later in xc_dom_build_image(). Can we delay mapping of this page in Xen until the ring buffer is actually required by the emulation code for reading/writing data. By that time, the page would have been physically mapped. Regards, Bhupinder On 15 June 2017 at 12:03, Bhupinder Thakur <bhupinder.thakur@linaro.org> wrote: > Hi Julien, > > >>>>> diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h >>>>> index 5e1fc60..d1ca9c6 100644 >>>>> --- a/tools/libxl/libxl_arch.h >>>>> +++ b/tools/libxl/libxl_arch.h >>>>> @@ -32,6 +32,13 @@ _hidden >>>>> int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config >>>>> *d_config, >>>>> uint32_t domid); >>>>> >>>>> +/* arch specific internal domain creation finish function */ >>>>> +_hidden >>>>> +int libxl__arch_domain_create_finish(libxl__gc *gc, >>>>> + libxl_domain_build_info *info, >>>>> + uint32_t domid, >>>>> + libxl__domain_build_state *state); >>>> >>>> >>>> >>>> Can you explain why you need a new arch helper rather than using the >>>> current >>>> one? >>> >>> >>> libxl__arch_domain_create() is called from libxl__build_pre(). This >>> function is called before libxl__build_pv(). By this time the domain >>> has not be created and I found that if I tried to initialize vpl011 >>> from inside libxl__arch_domain_create() then initialization was >>> failing due to prepare_ring_for_helper() failing. >> >> >> What do you mean by the domain has not been created? The domain has already >> been created (you have a domid in hand) when you libxl__build_pre. So the >> problem is different. >> >> Looking at the code, I guess the problem is because the vuart pfn will be >> allocated by xc_dom_build_image called by libxl_build_pv -> >> libxl__build_dom. >> >>> >>> So I had to create another function which will be called from >>> libxl__build_post() after domain has been setup. >> >> >> It looks a bit odd to me to create the vpl011 UART that late in the process >> because when you read the code you would expect all the hardware to be setup >> after libxl__arch_domain_finalise_hw_descriptions is called. >> >> But I understand it is not possible to do it as the ring has not yet been >> allocated. So is there a way to allocate the ring before? > > >> Wei, Ian, do you have any opinions on what should the workflow in libxl? > > Actually, I had introduced an API xc_get_vuart_gfn() to get the pfn. > Since it is a fixed pfn, the API can > return it even before xc_build_dom_image() is called. I tried after > moving the vpl011_init function to libxl__arch_domain_create() and it > is working. > > Regards, > Bhupinder
On 19/06/17 11:59, Bhupinder Thakur wrote: > Hi Julien, > > I was mistaken in my earlier mail about vpl011 init working if it is > moved to libxl__arch_domain_create(). It is failing because as you > have mentioned vuart_pfn is allocated later in xc_dom_build_image(). > > Can we delay mapping of this page in Xen until the ring buffer is > actually required by the emulation code for reading/writing data. By > that time, the page would have been physically mapped. You would not be able to report an error if you fail to map it. But this looks like to me a workaround for a tool problem. Anyway, as I said, I'd like feedback from the tools maintainers to see how we can proceed. Cheers,
On Mon, Jun 19, 2017 at 12:01:32PM +0100, Julien Grall wrote: > > > On 19/06/17 11:59, Bhupinder Thakur wrote: > > Hi Julien, > > > > I was mistaken in my earlier mail about vpl011 init working if it is > > moved to libxl__arch_domain_create(). It is failing because as you > > have mentioned vuart_pfn is allocated later in xc_dom_build_image(). > > > > Can we delay mapping of this page in Xen until the ring buffer is > > actually required by the emulation code for reading/writing data. By > > that time, the page would have been physically mapped. > > You would not be able to report an error if you fail to map it. But this > looks like to me a workaround for a tool problem. > > Anyway, as I said, I'd like feedback from the tools maintainers to see how > we can proceed. > Is there a summary of the problem, is there a particular email in this thread I should look at? Sorry I'm swamped by emails and patches at the moment.
Hi Wei, On 19 June 2017 at 17:17, Wei Liu <wei.liu2@citrix.com> wrote: > On Mon, Jun 19, 2017 at 12:01:32PM +0100, Julien Grall wrote: >> >> >> On 19/06/17 11:59, Bhupinder Thakur wrote: >> > Hi Julien, >> > >> > I was mistaken in my earlier mail about vpl011 init working if it is >> > moved to libxl__arch_domain_create(). It is failing because as you >> > have mentioned vuart_pfn is allocated later in xc_dom_build_image(). >> > >> > Can we delay mapping of this page in Xen until the ring buffer is >> > actually required by the emulation code for reading/writing data. By >> > that time, the page would have been physically mapped. >> >> You would not be able to report an error if you fail to map it. But this >> looks like to me a workaround for a tool problem. >> >> Anyway, as I said, I'd like feedback from the tools maintainers to see how >> we can proceed. >> > > Is there a summary of the problem, is there a particular email in this > thread I should look at? Sorry I'm swamped by emails and patches at the > moment. I will summarize the problem. It was decided to call domain_vpl011_init() from inside libxl__arch_domain_create() to initialize vpl011. However, domain_vpl011_init() fails to map the the vuart GFN because it has not been physically mapped yet by the toolstack. The following call flows highlights the issue. libxl__domain_build() ---> libxl__build_pv ---> libxl__build_dom() ----> xc_dom_build_image() ---> alloc_magic_pages() ----> vuart GFN allocated/mapped here libxl__domain_build() ----> libxl__build_pre() ----> libxl__arch_domain_create() ----> domain_vpl011_init() ---> this call fails as the vuart GFN has not been physically mapped yet as shown in the first call flow. However, libxl__build_pv() is called after libxl__build_pre(). It means that the domain_vpl011_init() is called before alloc_magic_pages() is called and hence the initialization fails. For that reason, I had introduced a new function libxl__arch_domain_create_finish() which will be called from libxl__build_post(). I moved the domain_vpl011_init() there. However, Julien pointed out that vuart should be initialized early in libxl__arch_domain_create() function. So the issue is what is the right place to call domain_vpl011_init()? I hope it clarifies the issue. Regards, Bhupinder
On 06/19/2017 02:11 PM, Bhupinder Thakur wrote: > Hi Wei, Hi Bhupinder, > On 19 June 2017 at 17:17, Wei Liu <wei.liu2@citrix.com> wrote: >> On Mon, Jun 19, 2017 at 12:01:32PM +0100, Julien Grall wrote: >>> >>> >>> On 19/06/17 11:59, Bhupinder Thakur wrote: >>>> Hi Julien, >>>> >>>> I was mistaken in my earlier mail about vpl011 init working if it is >>>> moved to libxl__arch_domain_create(). It is failing because as you >>>> have mentioned vuart_pfn is allocated later in xc_dom_build_image(). >>>> >>>> Can we delay mapping of this page in Xen until the ring buffer is >>>> actually required by the emulation code for reading/writing data. By >>>> that time, the page would have been physically mapped. >>> >>> You would not be able to report an error if you fail to map it. But this >>> looks like to me a workaround for a tool problem. >>> >>> Anyway, as I said, I'd like feedback from the tools maintainers to see how >>> we can proceed. >>> >> >> Is there a summary of the problem, is there a particular email in this >> thread I should look at? Sorry I'm swamped by emails and patches at the >> moment. > > I will summarize the problem. > > It was decided to call domain_vpl011_init() from inside > libxl__arch_domain_create() to initialize vpl011. However, > domain_vpl011_init() fails to map the the vuart GFN because it has not > been physically mapped yet by the toolstack. > > The following call flows highlights the issue. > > libxl__domain_build() ---> libxl__build_pv ---> libxl__build_dom() > ----> xc_dom_build_image() ---> alloc_magic_pages() ----> vuart GFN > allocated/mapped here > > libxl__domain_build() ----> libxl__build_pre() ----> > libxl__arch_domain_create() ----> domain_vpl011_init() ---> this call > fails as the vuart GFN has not been physically mapped yet as shown in > the first call flow. > > However, libxl__build_pv() is called after libxl__build_pre(). It > means that the domain_vpl011_init() is called before > alloc_magic_pages() is called and hence the initialization fails. > > For that reason, I had introduced a new function > libxl__arch_domain_create_finish() which will be called from > libxl__build_post(). I moved the domain_vpl011_init() there. However, > Julien pointed out that vuart should be initialized early in > libxl__arch_domain_create() function. libxl__arch_domain_create could be a place or even libxl__arch_domain_finalise_hw_descriptions. My point is it looks a bit odd to create the vpl011 UART very late in the process as from the code you would expect all the hardware to be setup after libxl__arch_domain_finialise_hw_descriptions is called. > > So the issue is what is the right place to call domain_vpl011_init()? > > I hope it clarifies the issue. Cheers,
On Tue, Jun 20, 2017 at 12:16:52PM +0100, Julien Grall wrote: > On 06/19/2017 02:11 PM, Bhupinder Thakur wrote: > > Hi Wei, > > Hi Bhupinder, > > > On 19 June 2017 at 17:17, Wei Liu <wei.liu2@citrix.com> wrote: > > > On Mon, Jun 19, 2017 at 12:01:32PM +0100, Julien Grall wrote: > > > > > > > > > > > > On 19/06/17 11:59, Bhupinder Thakur wrote: > > > > > Hi Julien, > > > > > > > > > > I was mistaken in my earlier mail about vpl011 init working if it is > > > > > moved to libxl__arch_domain_create(). It is failing because as you > > > > > have mentioned vuart_pfn is allocated later in xc_dom_build_image(). > > > > > > > > > > Can we delay mapping of this page in Xen until the ring buffer is > > > > > actually required by the emulation code for reading/writing data. By > > > > > that time, the page would have been physically mapped. > > > > > > > > You would not be able to report an error if you fail to map it. But this > > > > looks like to me a workaround for a tool problem. > > > > > > > > Anyway, as I said, I'd like feedback from the tools maintainers to see how > > > > we can proceed. > > > > > > > > > > Is there a summary of the problem, is there a particular email in this > > > thread I should look at? Sorry I'm swamped by emails and patches at the > > > moment. > > > > I will summarize the problem. > > > > It was decided to call domain_vpl011_init() from inside > > libxl__arch_domain_create() to initialize vpl011. However, > > domain_vpl011_init() fails to map the the vuart GFN because it has not > > been physically mapped yet by the toolstack. > > > > The following call flows highlights the issue. > > > > libxl__domain_build() ---> libxl__build_pv ---> libxl__build_dom() > > ----> xc_dom_build_image() ---> alloc_magic_pages() ----> vuart GFN > > allocated/mapped here > > > > libxl__domain_build() ----> libxl__build_pre() ----> > > libxl__arch_domain_create() ----> domain_vpl011_init() ---> this call > > fails as the vuart GFN has not been physically mapped yet as shown in > > the first call flow. > > > > However, libxl__build_pv() is called after libxl__build_pre(). It > > means that the domain_vpl011_init() is called before > > alloc_magic_pages() is called and hence the initialization fails. > > > > For that reason, I had introduced a new function > > libxl__arch_domain_create_finish() which will be called from > > libxl__build_post(). I moved the domain_vpl011_init() there. However, > > Julien pointed out that vuart should be initialized early in > > libxl__arch_domain_create() function. > > libxl__arch_domain_create could be a place or even > libxl__arch_domain_finalise_hw_descriptions. > libxl__arch_domain_finialise_hw_descriptions sounds like a good place to me.
Hi Julien, On 20 June 2017 at 16:46, Julien Grall <julien.grall@arm.com> wrote: > On 06/19/2017 02:11 PM, Bhupinder Thakur wrote: >> >> Hi Wei, > > > Hi Bhupinder, > > >> On 19 June 2017 at 17:17, Wei Liu <wei.liu2@citrix.com> wrote: >>> >>> On Mon, Jun 19, 2017 at 12:01:32PM +0100, Julien Grall wrote: >>>> >>>> >>>> >>>> On 19/06/17 11:59, Bhupinder Thakur wrote: >>>>> >>>>> Hi Julien, >>>>> >>>>> I was mistaken in my earlier mail about vpl011 init working if it is >>>>> moved to libxl__arch_domain_create(). It is failing because as you >>>>> have mentioned vuart_pfn is allocated later in xc_dom_build_image(). >>>>> >>>>> Can we delay mapping of this page in Xen until the ring buffer is >>>>> actually required by the emulation code for reading/writing data. By >>>>> that time, the page would have been physically mapped. >>>> >>>> >>>> You would not be able to report an error if you fail to map it. But this >>>> looks like to me a workaround for a tool problem. >>>> >>>> Anyway, as I said, I'd like feedback from the tools maintainers to see >>>> how >>>> we can proceed. >>>> >>> >>> Is there a summary of the problem, is there a particular email in this >>> thread I should look at? Sorry I'm swamped by emails and patches at the >>> moment. >> >> >> I will summarize the problem. >> >> It was decided to call domain_vpl011_init() from inside >> libxl__arch_domain_create() to initialize vpl011. However, >> domain_vpl011_init() fails to map the the vuart GFN because it has not >> been physically mapped yet by the toolstack. >> >> The following call flows highlights the issue. >> >> libxl__domain_build() ---> libxl__build_pv ---> libxl__build_dom() >> ----> xc_dom_build_image() ---> alloc_magic_pages() ----> vuart GFN >> allocated/mapped here >> >> libxl__domain_build() ----> libxl__build_pre() ----> >> libxl__arch_domain_create() ----> domain_vpl011_init() ---> this call >> fails as the vuart GFN has not been physically mapped yet as shown in >> the first call flow. >> >> However, libxl__build_pv() is called after libxl__build_pre(). It >> means that the domain_vpl011_init() is called before >> alloc_magic_pages() is called and hence the initialization fails. >> >> For that reason, I had introduced a new function >> libxl__arch_domain_create_finish() which will be called from >> libxl__build_post(). I moved the domain_vpl011_init() there. However, >> Julien pointed out that vuart should be initialized early in >> libxl__arch_domain_create() function. > > > libxl__arch_domain_create could be a place or even > libxl__arch_domain_finalise_hw_descriptions. > > My point is it looks a bit odd to create the vpl011 UART very late in the > process as from the code you would expect all the hardware to be setup after > libxl__arch_domain_finialise_hw_descriptions is called. > libxl__arch_domain_finalise_hw_descriptions() is called just before xc_dom_build_image() and therefore the vuart gfn is still not allocated. Maybe I can introduce a new arch specific libxl__arch_domain_init_vpl011() function and call it from inside libxl__build_dom() after call to xc_dom_build_image() so that the vuart gfn is allocated. Regards, Bhupinder
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h index 1629f41..77425dd 100644 --- a/tools/libxc/include/xenctrl.h +++ b/tools/libxc/include/xenctrl.h @@ -884,6 +884,23 @@ int xc_vcpu_getcontext(xc_interface *xch, uint32_t domid, uint32_t vcpu, vcpu_guest_context_any_t *ctxt); +/** + * This function initializes the vpl011 emulation and returns + * the event to be used by the backend for communicating with + * the emulation code. + * + * @parm xch a handle to an open hypervisor interface + * @parm domid the domain to get information from + * @parm console_domid the domid of the backend console + * @parm gfn the guest pfn to be used as the ring buffer + * @parm evtchn the event channel to be used for events + * @return 0 on success, negative error on failure + */ +int xc_dom_vpl011_init(xc_interface *xch, + uint32_t domid, + uint32_t console_domid, + xen_pfn_t gfn, + evtchn_port_t *evtchn); /** * This function returns information about the XSAVE state of a particular diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c index 00909ad4..a8efd5e 100644 --- a/tools/libxc/xc_domain.c +++ b/tools/libxc/xc_domain.c @@ -343,6 +343,29 @@ int xc_domain_get_guest_width(xc_interface *xch, uint32_t domid, return 0; } +int xc_dom_vpl011_init(xc_interface *xch, + uint32_t domid, + uint32_t console_domid, + xen_pfn_t gfn, + evtchn_port_t *evtchn) +{ + DECLARE_DOMCTL; + int rc = 0; + + domctl.cmd = XEN_DOMCTL_vuart_op; + domctl.domain = (domid_t)domid; + domctl.u.vuart_op.cmd = XEN_DOMCTL_VUART_OP_INIT_VPL011; + domctl.u.vuart_op.console_domid = console_domid; + domctl.u.vuart_op.gfn = gfn; + + if ( (rc = do_domctl(xch, &domctl)) < 0 ) + return rc; + + *evtchn = domctl.u.vuart_op.evtchn; + + return rc; +} + int xc_domain_getinfo(xc_interface *xch, uint32_t first_domid, unsigned int max_doms, diff --git a/tools/libxl/libxl_arch.h b/tools/libxl/libxl_arch.h index 5e1fc60..d1ca9c6 100644 --- a/tools/libxl/libxl_arch.h +++ b/tools/libxl/libxl_arch.h @@ -32,6 +32,13 @@ _hidden int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config, uint32_t domid); +/* arch specific internal domain creation finish function */ +_hidden +int libxl__arch_domain_create_finish(libxl__gc *gc, + libxl_domain_build_info *info, + uint32_t domid, + libxl__domain_build_state *state); + /* setup arch specific hardware description, i.e. DTB on ARM */ _hidden int libxl__arch_domain_init_hw_description(libxl__gc *gc, diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c index d842d88..b60dfa9 100644 --- a/tools/libxl/libxl_arm.c +++ b/tools/libxl/libxl_arm.c @@ -106,6 +106,25 @@ int libxl__arch_domain_create(libxl__gc *gc, libxl_domain_config *d_config, return 0; } +int libxl__arch_domain_create_finish(libxl__gc *gc, + libxl_domain_build_info *info, + uint32_t domid, + libxl__domain_build_state *state) +{ + int ret = 0; + + if ( info->arch_arm.vuart && + (ret = xc_dom_vpl011_init(CTX->xch, + domid, + state->console_domid, + xc_get_vuart_gfn(), + &state->vuart_port)) != 0 ) { + LOG(ERROR, "xc_dom_vpl011_init failed\n"); + } + + return ret; +} + int libxl__arch_extra_memory(libxl__gc *gc, const libxl_domain_build_info *info, uint64_t *out) diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c index 5d914a5..187c5bd 100644 --- a/tools/libxl/libxl_dom.c +++ b/tools/libxl/libxl_dom.c @@ -587,7 +587,10 @@ retry_transaction: goto retry_transaction; xs_introduce_domain(ctx->xsh, domid, state->store_mfn, state->store_port); free(vm_path); - return 0; + + rc = libxl__arch_domain_create_finish(gc, info, domid, state); + + return rc; } static int set_vnuma_info(libxl__gc *gc, uint32_t domid, @@ -788,6 +791,7 @@ int libxl__build_pv(libxl__gc *gc, uint32_t domid, if (xc_dom_translated(dom)) { state->console_mfn = dom->console_pfn; state->store_mfn = dom->xenstore_pfn; + state->vuart_gfn = dom->vuart_gfn; } else { state->console_mfn = xc_dom_p2m(dom, dom->console_pfn); state->store_mfn = xc_dom_p2m(dom, dom->xenstore_pfn); diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c index 455f6f0..3544028 100644 --- a/tools/libxl/libxl_x86.c +++ b/tools/libxl/libxl_x86.c @@ -358,6 +358,14 @@ out: return ret; } +int libxl__arch_domain_create_finish(libxl__gc *gc, + libxl_domain_build_info *info, + uint32_t domid, + libxl__domain_build_state *state) +{ + return 0; +} + int libxl__arch_extra_memory(libxl__gc *gc, const libxl_domain_build_info *info, uint64_t *out) diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index 76310ed..9e150ba 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -665,6 +665,8 @@ fail: void arch_domain_destroy(struct domain *d) { + domain_vpl011_deinit(d); + /* IOMMU page table is shared with P2M, always call * iommu_domain_destroy() before p2m_teardown(). */ diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c index 971caec..741679b 100644 --- a/xen/arch/arm/domctl.c +++ b/xen/arch/arm/domctl.c @@ -5,13 +5,15 @@ */ #include <xen/types.h> -#include <xen/lib.h> +#include <public/domctl.h> #include <xen/errno.h> -#include <xen/sched.h> +#include <xen/guest_access.h> #include <xen/hypercall.h> #include <xen/iocap.h> +#include <xen/lib.h> +#include <xen/mm.h> +#include <xen/sched.h> #include <xsm/xsm.h> -#include <public/domctl.h> void arch_get_domain_info(const struct domain *d, struct xen_domctl_getdomaininfo *info) @@ -119,6 +121,42 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d, d->disable_migrate = domctl->u.disable_migrate.disable; return 0; + case XEN_DOMCTL_vuart_op: + { + int rc; + struct xen_domctl_vuart_op *vuart_op = &domctl->u.vuart_op; + + switch(vuart_op->cmd) + { + case XEN_DOMCTL_VUART_OP_INIT_VPL011: + + if ( !d->creation_finished ) + { + struct vpl011_init_info info; + + info.console_domid = vuart_op->console_domid; + info.gfn = _gfn(vuart_op->gfn); + + rc = domain_vpl011_init(d, &info); + if ( !rc ) + { + vuart_op->evtchn = info.evtchn; + rc = __copy_to_guest(u_domctl, domctl, 1); + } + } + else + { + rc = - EPERM; + } + break; + + default: + rc = -EINVAL; + break; + } + + return rc; + } default: { int rc; diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h index e6cf211..c6ff458 100644 --- a/xen/include/public/domctl.h +++ b/xen/include/public/domctl.h @@ -36,6 +36,7 @@ #include "grant_table.h" #include "hvm/save.h" #include "memory.h" +#include "event_channel.h" #define XEN_DOMCTL_INTERFACE_VERSION 0x0000000d @@ -1138,6 +1139,15 @@ struct xen_domctl_psr_cat_op { uint32_t target; /* IN */ uint64_t data; /* IN/OUT */ }; + +struct xen_domctl_vuart_op { +#define XEN_DOMCTL_VUART_OP_INIT_VPL011 0 + uint32_t cmd; /* XEN_DOMCTL_VUART_OP_* */ + uint32_t console_domid; /* IN */ + xen_pfn_t gfn; /* IN */ + evtchn_port_t evtchn; /* OUT */ +}; + typedef struct xen_domctl_psr_cat_op xen_domctl_psr_cat_op_t; DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cat_op_t); @@ -1218,6 +1228,7 @@ struct xen_domctl { #define XEN_DOMCTL_monitor_op 77 #define XEN_DOMCTL_psr_cat_op 78 #define XEN_DOMCTL_soft_reset 79 +#define XEN_DOMCTL_vuart_op 80 #define XEN_DOMCTL_gdbsx_guestmemio 1000 #define XEN_DOMCTL_gdbsx_pausevcpu 1001 #define XEN_DOMCTL_gdbsx_unpausevcpu 1002 @@ -1280,6 +1291,7 @@ struct xen_domctl { struct xen_domctl_psr_cmt_op psr_cmt_op; struct xen_domctl_monitor_op monitor_op; struct xen_domctl_psr_cat_op psr_cat_op; + struct xen_domctl_vuart_op vuart_op; uint8_t pad[128]; } u; };
Add a new domctl API to initialize vpl011. It takes the GFN and console backend domid as input and returns an event channel to be used for sending and receiving events from Xen. Xen will communicate with xenconsole using GFN as the ring buffer and the event channel to transmit and receive pl011 data on the guest domain's behalf. Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org> --- CC: ij CC: wl CC: ss CC: jg Changes since v3: - Added a new arch specific function libxl__arch_domain_create_finish(), which calls the vpl011 initialization function. For x86 this function does not do anything. - domain_vpl011_init() takes a pointer to a structure which contains all the required information such as console_domid, gfn instead of passing parameters separately. - Dropped a DOMCTL API defined for de-initializing vpl011 as that should be taken care when the domain is destroyed (and not dependent on userspace libraries/applications). Changes since v2: - Replaced the DOMCTL APIs defined for get/set of event channel and GFN with a set of DOMCTL APIs for initializing and de-initializing vpl011 emulation. tools/libxc/include/xenctrl.h | 17 +++++++++++++++++ tools/libxc/xc_domain.c | 23 ++++++++++++++++++++++ tools/libxl/libxl_arch.h | 7 +++++++ tools/libxl/libxl_arm.c | 19 +++++++++++++++++++ tools/libxl/libxl_dom.c | 6 +++++- tools/libxl/libxl_x86.c | 8 ++++++++ xen/arch/arm/domain.c | 2 ++ xen/arch/arm/domctl.c | 44 ++++++++++++++++++++++++++++++++++++++++--- xen/include/public/domctl.h | 12 ++++++++++++ 9 files changed, 134 insertions(+), 4 deletions(-)