| Message ID | 1498117132-27139-9-git-send-email-bhupinder.thakur@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | SBSA UART emulation support in Xen | expand |
On Thu, 22 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: Ian Jackson <ian.jackson@eu.citrix.com> > CC: Wei Liu <wei.liu2@citrix.com> > CC: Stefano Stabellini <sstabellini@kernel.org> > CC: Julien Grall <julien.grall@arm.com> > > Changes since v4: > - Removed libxl__arch_domain_create_finish(). > - Added a new function libxl__arch_build_dom_finish(), which is called at the last > in libxl__build_dom(). This function calls the vpl011 initialization function now. > > 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 | 20 ++++++++++++++++++++ > tools/libxc/xc_domain.c | 25 +++++++++++++++++++++++++ > tools/libxl/libxl_arch.h | 6 ++++++ > tools/libxl/libxl_arm.c | 22 ++++++++++++++++++++++ > tools/libxl/libxl_dom.c | 4 ++++ > tools/libxl/libxl_x86.c | 8 ++++++++ > xen/arch/arm/domain.c | 5 +++++ > xen/arch/arm/domctl.c | 37 +++++++++++++++++++++++++++++++++++++ > xen/include/public/domctl.h | 12 ++++++++++++ > 9 files changed, 139 insertions(+) > > diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h > index 1629f41..26f3d1e 100644 > --- a/tools/libxc/include/xenctrl.h > +++ b/tools/libxc/include/xenctrl.h > @@ -885,6 +885,26 @@ int xc_vcpu_getcontext(xc_interface *xch, > uint32_t vcpu, > vcpu_guest_context_any_t *ctxt); > > +#if defined (__arm__) || defined(__aarch64__) > +/** > + * 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); > +#endif Actually, the pattern is to define the xc_ function on all architecture but only return ENOSYS where it's not implemented, see xc_vcpu_get_extstate. > /** > * This function returns information about the XSAVE state of a particular > * vcpu of a domain. If extstate->size and extstate->xfeature_mask are 0, > diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c > index 5d192ea..55de408 100644 > --- a/tools/libxc/xc_domain.c > +++ b/tools/libxc/xc_domain.c > @@ -343,6 +343,31 @@ int xc_domain_get_guest_width(xc_interface *xch, uint32_t domid, > return 0; > } > > +#if defined (__arm__) || defined(__aarch64__) > +int xc_dom_vpl011_init(xc_interface *xch, > + uint32_t domid, > + uint32_t console_domid, > + xen_pfn_t gfn, > + evtchn_port_t *evtchn) > +{ See other comment. > + 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; > +} > +#endif > + > 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..118b92c 100644 > --- a/tools/libxl/libxl_arch.h > +++ b/tools/libxl/libxl_arch.h > @@ -44,6 +44,12 @@ int libxl__arch_domain_finalise_hw_description(libxl__gc *gc, > libxl_domain_build_info *info, > struct xc_dom_image *dom); > > +/* perform any pending hardware initialization */ > +int libxl__arch_build_dom_finish(libxl__gc *gc, > + libxl_domain_build_info *info, > + struct xc_dom_image *dom, > + libxl__domain_build_state *state); > + > /* build vNUMA vmemrange with arch specific information */ > _hidden > int libxl__arch_vnuma_build_vmemrange(libxl__gc *gc, > diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c > index d842d88..9d6448e 100644 > --- a/tools/libxl/libxl_arm.c > +++ b/tools/libxl/libxl_arm.c > @@ -1038,6 +1038,28 @@ int libxl__arch_domain_finalise_hw_description(libxl__gc *gc, > return 0; > } > > +int libxl__arch_build_dom_finish(libxl__gc *gc, > + libxl_domain_build_info *info, > + struct xc_dom_image *dom, > + libxl__domain_build_state *state) > +{ > + int ret = 0; > + > + if ( info->arch_arm.vuart ) { > + ret = xc_dom_vpl011_init(CTX->xch, > + dom->guest_domid, > + dom->console_domid, > + dom->vuart_gfn, > + &state->vuart_port); > + if ( ret < 0 ) > + { > + LOG(ERROR, "xc_dom_vpl011_init failed\n"); > + } > + } > + > + return ret; > +} > + > int libxl__arch_vnuma_build_vmemrange(libxl__gc *gc, > uint32_t domid, > libxl_domain_build_info *info, > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c > index c98af60..9b42205 100644 > --- a/tools/libxl/libxl_dom.c > +++ b/tools/libxl/libxl_dom.c > @@ -687,6 +687,10 @@ static int libxl__build_dom(libxl__gc *gc, uint32_t domid, > LOGE(ERROR, "xc_dom_gnttab_init failed"); > goto out; > } > + if ( (ret = libxl__arch_build_dom_finish(gc, info, dom, state)) != 0 ) { > + LOGE(ERROR, "libxl__arch_build_dom_finish failed"); > + goto out; > + } > > out: > return ret != 0 ? ERROR_FAIL : 0; > diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c > index 455f6f0..0aaeded 100644 > --- a/tools/libxl/libxl_x86.c > +++ b/tools/libxl/libxl_x86.c > @@ -391,6 +391,14 @@ int libxl__arch_domain_finalise_hw_description(libxl__gc *gc, > return rc; > } > > +int libxl__arch_build_dom_finish(libxl__gc *gc, > + libxl_domain_build_info *info, > + struct xc_dom_image *dom, > + libxl__domain_build_state *state) > +{ > + return 0; > +} > + > /* Return 0 on success, ERROR_* on failure. */ > int libxl__arch_vnuma_build_vmemrange(libxl__gc *gc, > uint32_t domid, > diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c > index 76310ed..f1b24cc 100644 > --- a/xen/arch/arm/domain.c > +++ b/xen/arch/arm/domain.c > @@ -665,6 +665,11 @@ fail: > > void arch_domain_destroy(struct domain *d) > { > + /* > + * vpl011 is initialized via a DOMCTL call XEN_DOMCTL_vuart_op. > + */ > + 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 86fa102..280c412 100644 > --- a/xen/arch/arm/domctl.c > +++ b/xen/arch/arm/domctl.c > @@ -6,9 +6,11 @@ > > #include <xen/types.h> > #include <xen/errno.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> > @@ -119,6 +121,41 @@ 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 f7cbc0a..0da8aa6 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 0x0000000e > > @@ -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 >
On 23/06/17 00:04, Stefano Stabellini wrote: >> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h >> index 1629f41..26f3d1e 100644 >> --- a/tools/libxc/include/xenctrl.h >> +++ b/tools/libxc/include/xenctrl.h >> @@ -885,6 +885,26 @@ int xc_vcpu_getcontext(xc_interface *xch, >> uint32_t vcpu, >> vcpu_guest_context_any_t *ctxt); >> >> +#if defined (__arm__) || defined(__aarch64__) >> +/** >> + * 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); >> +#endif > > Actually, the pattern is to define the xc_ function on all architecture > but only return ENOSYS where it's not implemented, see > xc_vcpu_get_extstate. Well, I think the main reason behind if to avoid dummy call to the hypervisor. But effectively the hypervisor will return a proper error. As the call is not made in common code, I would make this function compile on all the platform (there are nothing arch specific in it). Cheers,
On 23/06/17 14:17, Julien Grall wrote: > > > On 23/06/17 00:04, Stefano Stabellini wrote: >>> diff --git a/tools/libxc/include/xenctrl.h >>> b/tools/libxc/include/xenctrl.h >>> index 1629f41..26f3d1e 100644 >>> --- a/tools/libxc/include/xenctrl.h >>> +++ b/tools/libxc/include/xenctrl.h >>> @@ -885,6 +885,26 @@ int xc_vcpu_getcontext(xc_interface *xch, >>> uint32_t vcpu, >>> vcpu_guest_context_any_t *ctxt); >>> >>> +#if defined (__arm__) || defined(__aarch64__) >>> +/** >>> + * 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); >>> +#endif >> >> Actually, the pattern is to define the xc_ function on all architecture >> but only return ENOSYS where it's not implemented, see >> xc_vcpu_get_extstate. > > Well, I think the main reason behind if to avoid dummy call to the > hypervisor. But effectively the hypervisor will return a proper error. Actually, looking at the public header. This is because vcpu_get_extstate structure is only available on x86. Whereas vpl011_init is available for all the architecture even though only ARM effectively implementing it. But my point stands below, there is no harm to implement it for x86 as it would compile on any platform. > > As the call is not made in common code, I would make this function > compile on all the platform (there are nothing arch specific in it). Cheers,
Hi Bhupinder, On 22/06/17 08:38, Bhupinder Thakur wrote: > diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h > index f7cbc0a..0da8aa6 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 0x0000000e > > @@ -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 Sorry, it just occurred to me that you are using VPL011 in the command name. I think requiring a command per new uart is a bit too much. If we were to support new UART, I would expect a field to tell the type of the UART. Otherwise every time we add a new command, we would have one per UART. DOMCTL can be modified later one. But I would like to get the interface right as we did in other place. > + uint32_t cmd; /* XEN_DOMCTL_VUART_OP_* */ > + uint32_t console_domid; /* IN */ > + xen_pfn_t gfn; /* IN */ > + evtchn_port_t evtchn; /* OUT */ I would be useful if you document the structure. The first two are pretty much straightforward, but gfn and evtchn are less. > +}; > + > 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; > }; > Cheers,
On Fri, 23 Jun 2017, Julien Grall wrote: > On 23/06/17 14:17, Julien Grall wrote: > > > > > > On 23/06/17 00:04, Stefano Stabellini wrote: > > > > diff --git a/tools/libxc/include/xenctrl.h > > > > b/tools/libxc/include/xenctrl.h > > > > index 1629f41..26f3d1e 100644 > > > > --- a/tools/libxc/include/xenctrl.h > > > > +++ b/tools/libxc/include/xenctrl.h > > > > @@ -885,6 +885,26 @@ int xc_vcpu_getcontext(xc_interface *xch, > > > > uint32_t vcpu, > > > > vcpu_guest_context_any_t *ctxt); > > > > > > > > +#if defined (__arm__) || defined(__aarch64__) > > > > +/** > > > > + * 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); > > > > +#endif > > > > > > Actually, the pattern is to define the xc_ function on all architecture > > > but only return ENOSYS where it's not implemented, see > > > xc_vcpu_get_extstate. > > > > Well, I think the main reason behind if to avoid dummy call to the > > hypervisor. But effectively the hypervisor will return a proper error. > > Actually, looking at the public header. This is because vcpu_get_extstate > structure is only available on x86. Whereas vpl011_init is available for all > the architecture even though only ARM effectively implementing it. > > But my point stands below, there is no harm to implement it for x86 as it > would compile on any platform. Sounds good to me > > > > As the call is not made in common code, I would make this function > > compile on all the platform (there are nothing arch specific in it).
Hi Julien, >>>> +#if defined (__arm__) || defined(__aarch64__) >>>> +/** >>>> + * 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); >>>> +#endif >>> >>> >>> Actually, the pattern is to define the xc_ function on all architecture >>> but only return ENOSYS where it's not implemented, see >>> xc_vcpu_get_extstate. >> >> >> Well, I think the main reason behind if to avoid dummy call to the >> hypervisor. But effectively the hypervisor will return a proper error. > > > Actually, looking at the public header. This is because vcpu_get_extstate > structure is only available on x86. Whereas vpl011_init is available for all > the architecture even though only ARM effectively implementing it. > > But my point stands below, there is no harm to implement it for x86 as it > would compile on any platform. > > >> >> As the call is not made in common code, I would make this function >> compile on all the platform (there are nothing arch specific in it). Currently, xc_dom_vpl011_init() is called only if info->arch_arm.vuart flag is true. This check may look a bit odd in the common code. Regards, Bhupinder
On 27/06/17 14:43, Bhupinder Thakur wrote: > Hi Julien, Hi Bhupinder, > >>>>> +#if defined (__arm__) || defined(__aarch64__) >>>>> +/** >>>>> + * 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); >>>>> +#endif >>>> >>>> >>>> Actually, the pattern is to define the xc_ function on all architecture >>>> but only return ENOSYS where it's not implemented, see >>>> xc_vcpu_get_extstate. >>> >>> >>> Well, I think the main reason behind if to avoid dummy call to the >>> hypervisor. But effectively the hypervisor will return a proper error. >> >> >> Actually, looking at the public header. This is because vcpu_get_extstate >> structure is only available on x86. Whereas vpl011_init is available for all >> the architecture even though only ARM effectively implementing it. >> >> But my point stands below, there is no harm to implement it for x86 as it >> would compile on any platform. >> >> >>> >>> As the call is not made in common code, I would make this function >>> compile on all the platform (there are nothing arch specific in it). > > Currently, xc_dom_vpl011_init() is called only if info->arch_arm.vuart > flag is true. This check may look a bit odd in the > common code. I am not asking to call xc_dom_vpl011_init from common code. I am only asking to make this function available for anyone. Cheers,
On Thu, Jun 22, 2017 at 01:08:43PM +0530, Bhupinder Thakur wrote: [...] > +#if defined (__arm__) || defined(__aarch64__) > +/** > + * 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, Use domid_t please. > + xen_pfn_t gfn, > + evtchn_port_t *evtchn); > +#endif > + [...] > } > > +int libxl__arch_build_dom_finish(libxl__gc *gc, > + libxl_domain_build_info *info, > + struct xc_dom_image *dom, > + libxl__domain_build_state *state) > +{ > + int ret = 0; > + > + if ( info->arch_arm.vuart ) { Coding style issues here and in a lot of other places in libxl. > + ret = xc_dom_vpl011_init(CTX->xch, > + dom->guest_domid, > + dom->console_domid, > + dom->vuart_gfn, > + &state->vuart_port); > + if ( ret < 0 ) > + { > + LOG(ERROR, "xc_dom_vpl011_init failed\n"); > + } It is just one line so you can omit {}.
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h index 1629f41..26f3d1e 100644 --- a/tools/libxc/include/xenctrl.h +++ b/tools/libxc/include/xenctrl.h @@ -885,6 +885,26 @@ int xc_vcpu_getcontext(xc_interface *xch, uint32_t vcpu, vcpu_guest_context_any_t *ctxt); +#if defined (__arm__) || defined(__aarch64__) +/** + * 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); +#endif + /** * This function returns information about the XSAVE state of a particular * vcpu of a domain. If extstate->size and extstate->xfeature_mask are 0, diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c index 5d192ea..55de408 100644 --- a/tools/libxc/xc_domain.c +++ b/tools/libxc/xc_domain.c @@ -343,6 +343,31 @@ int xc_domain_get_guest_width(xc_interface *xch, uint32_t domid, return 0; } +#if defined (__arm__) || defined(__aarch64__) +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; +} +#endif + 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..118b92c 100644 --- a/tools/libxl/libxl_arch.h +++ b/tools/libxl/libxl_arch.h @@ -44,6 +44,12 @@ int libxl__arch_domain_finalise_hw_description(libxl__gc *gc, libxl_domain_build_info *info, struct xc_dom_image *dom); +/* perform any pending hardware initialization */ +int libxl__arch_build_dom_finish(libxl__gc *gc, + libxl_domain_build_info *info, + struct xc_dom_image *dom, + libxl__domain_build_state *state); + /* build vNUMA vmemrange with arch specific information */ _hidden int libxl__arch_vnuma_build_vmemrange(libxl__gc *gc, diff --git a/tools/libxl/libxl_arm.c b/tools/libxl/libxl_arm.c index d842d88..9d6448e 100644 --- a/tools/libxl/libxl_arm.c +++ b/tools/libxl/libxl_arm.c @@ -1038,6 +1038,28 @@ int libxl__arch_domain_finalise_hw_description(libxl__gc *gc, return 0; } +int libxl__arch_build_dom_finish(libxl__gc *gc, + libxl_domain_build_info *info, + struct xc_dom_image *dom, + libxl__domain_build_state *state) +{ + int ret = 0; + + if ( info->arch_arm.vuart ) { + ret = xc_dom_vpl011_init(CTX->xch, + dom->guest_domid, + dom->console_domid, + dom->vuart_gfn, + &state->vuart_port); + if ( ret < 0 ) + { + LOG(ERROR, "xc_dom_vpl011_init failed\n"); + } + } + + return ret; +} + int libxl__arch_vnuma_build_vmemrange(libxl__gc *gc, uint32_t domid, libxl_domain_build_info *info, diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c index c98af60..9b42205 100644 --- a/tools/libxl/libxl_dom.c +++ b/tools/libxl/libxl_dom.c @@ -687,6 +687,10 @@ static int libxl__build_dom(libxl__gc *gc, uint32_t domid, LOGE(ERROR, "xc_dom_gnttab_init failed"); goto out; } + if ( (ret = libxl__arch_build_dom_finish(gc, info, dom, state)) != 0 ) { + LOGE(ERROR, "libxl__arch_build_dom_finish failed"); + goto out; + } out: return ret != 0 ? ERROR_FAIL : 0; diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c index 455f6f0..0aaeded 100644 --- a/tools/libxl/libxl_x86.c +++ b/tools/libxl/libxl_x86.c @@ -391,6 +391,14 @@ int libxl__arch_domain_finalise_hw_description(libxl__gc *gc, return rc; } +int libxl__arch_build_dom_finish(libxl__gc *gc, + libxl_domain_build_info *info, + struct xc_dom_image *dom, + libxl__domain_build_state *state) +{ + return 0; +} + /* Return 0 on success, ERROR_* on failure. */ int libxl__arch_vnuma_build_vmemrange(libxl__gc *gc, uint32_t domid, diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index 76310ed..f1b24cc 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -665,6 +665,11 @@ fail: void arch_domain_destroy(struct domain *d) { + /* + * vpl011 is initialized via a DOMCTL call XEN_DOMCTL_vuart_op. + */ + 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 86fa102..280c412 100644 --- a/xen/arch/arm/domctl.c +++ b/xen/arch/arm/domctl.c @@ -6,9 +6,11 @@ #include <xen/types.h> #include <xen/errno.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> @@ -119,6 +121,41 @@ 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 f7cbc0a..0da8aa6 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 0x0000000e @@ -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: Ian Jackson <ian.jackson@eu.citrix.com> CC: Wei Liu <wei.liu2@citrix.com> CC: Stefano Stabellini <sstabellini@kernel.org> CC: Julien Grall <julien.grall@arm.com> Changes since v4: - Removed libxl__arch_domain_create_finish(). - Added a new function libxl__arch_build_dom_finish(), which is called at the last in libxl__build_dom(). This function calls the vpl011 initialization function now. 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 | 20 ++++++++++++++++++++ tools/libxc/xc_domain.c | 25 +++++++++++++++++++++++++ tools/libxl/libxl_arch.h | 6 ++++++ tools/libxl/libxl_arm.c | 22 ++++++++++++++++++++++ tools/libxl/libxl_dom.c | 4 ++++ tools/libxl/libxl_x86.c | 8 ++++++++ xen/arch/arm/domain.c | 5 +++++ xen/arch/arm/domctl.c | 37 +++++++++++++++++++++++++++++++++++++ xen/include/public/domctl.h | 12 ++++++++++++ 9 files changed, 139 insertions(+)