| Message ID | 1494426678-32639-1-git-send-email-bhupinder.thakur@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, May 10, 2017 at 08:01:18PM +0530, Bhupinder Thakur wrote: > Add two new domctl APIs to initialize and de-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 guest domain's behalf. > > Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org> [...] > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c > index 5d914a5..e7489d9 100644 > --- a/tools/libxl/libxl_dom.c > +++ b/tools/libxl/libxl_dom.c > @@ -688,6 +688,15 @@ static int libxl__build_dom(libxl__gc *gc, uint32_t domid, > goto out; > } > > + if ( info->vuart && > + (ret = xc_dom_vpl011_init(CTX->xch, > + domid, > + state->console_domid, > + dom->vuart_gfn, > + &state->vuart_port)) != 0 ) { > + LOGE(ERROR, "xc_dom_vpl011_init failed"); > + goto out; > + } Please push this to arch-specific function. > out: > return ret != 0 ? ERROR_FAIL : 0; > } > @@ -788,6 +797,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_domain.c b/tools/libxl/libxl_domain.c > index 08eccd0..1d2c65a 100644 > --- a/tools/libxl/libxl_domain.c > +++ b/tools/libxl/libxl_domain.c > @@ -1028,6 +1028,8 @@ void libxl__destroy_domid(libxl__egc *egc, libxl__destroy_domid_state *dis) > goto out; > } > > + xc_dom_vpl011_deinit(ctx->xch, domid); > + Again, arch-specific function please. > if (libxl__device_pci_destroy_all(gc, domid) < 0) > LOGD(ERROR, domid, "Pci shutdown failed"); > rc = xc_domain_pause(ctx->xch, domid); > diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c > index 971caec..11707db 100644 > --- a/xen/arch/arm/domctl.c > +++ b/xen/arch/arm/domctl.c > @@ -10,6 +10,7 @@ > #include <xen/sched.h> > #include <xen/hypercall.h> > #include <xen/iocap.h> > +#include <xen/guest_access.h> Please order the header files alphabetically.
Hi Wei, >> --- a/tools/libxl/libxl_dom.c >> +++ b/tools/libxl/libxl_dom.c >> @@ -688,6 +688,15 @@ static int libxl__build_dom(libxl__gc *gc, uint32_t domid, >> goto out; >> } >> >> + if ( info->vuart && >> + (ret = xc_dom_vpl011_init(CTX->xch, >> + domid, >> + state->console_domid, >> + dom->vuart_gfn, >> + &state->vuart_port)) != 0 ) { >> + LOGE(ERROR, "xc_dom_vpl011_init failed"); >> + goto out; >> + } > > Please push this to arch-specific function. > libxl__arch_domain_create() seems to be a good candidate where I can move this code. But this function does not take libxl__domain_build_state *state as input an input parameter, which carries information such as state->vuart_port for intializing vpl011. Should I modify the function prototype to take that also as input? This will require changes in the x86 code also, which is using this function. Regards, Bhupinder
Hi Bhupinder, On 10/05/17 15:31, Bhupinder Thakur wrote: > Add two new domctl APIs to initialize and de-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 guest domain's behalf. > > Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org> > --- > > 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 | 26 ++++++++++++++++++++++++++ > tools/libxc/xc_domain.c | 38 ++++++++++++++++++++++++++++++++++++++ > tools/libxl/libxl_dom.c | 10 ++++++++++ > tools/libxl/libxl_domain.c | 2 ++ > xen/arch/arm/domctl.c | 27 +++++++++++++++++++++++++++ > xen/include/public/domctl.h | 13 +++++++++++++ > 6 files changed, 116 insertions(+) > > diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h > index 1629f41..13ef215 100644 > --- a/tools/libxc/include/xenctrl.h > +++ b/tools/libxc/include/xenctrl.h > @@ -884,6 +884,32 @@ int xc_vcpu_getcontext(xc_interface *xch, > uint32_t domid, > uint32_t vcpu, > vcpu_guest_context_any_t *ctxt); > +/** > + * This function intializes the vpl011 emulation and returns s/intializes/initialize/ > + * 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 de-intializes the vpl011 emulation. > + * > + * @parm xch a handle to an open hypervisor interface > + * @parm domid the domain to get information from > + */ > +int xc_dom_vpl011_deinit(xc_interface *xch, > + uint32_t domid); I am not convinced of the purpose of this function. A pl011 device should never be removed whilst a domain is running and we should not rely on the toolstack to remove the pl011 at domain destruction for us. > > /** > * 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..ec48b9b 100644 > --- a/tools/libxc/xc_domain.c > +++ b/tools/libxc/xc_domain.c > @@ -343,6 +343,44 @@ 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_dom_vpl011_deinit(xc_interface *xch, > + uint32_t domid) > +{ > + 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_DEINIT_VPL011; > + > + rc = do_domctl(xch, &domctl); > + > + return rc; > +} > + > int xc_domain_getinfo(xc_interface *xch, > uint32_t first_domid, > unsigned int max_doms, > diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c > index 5d914a5..e7489d9 100644 > --- a/tools/libxl/libxl_dom.c > +++ b/tools/libxl/libxl_dom.c > @@ -688,6 +688,15 @@ static int libxl__build_dom(libxl__gc *gc, uint32_t domid, > goto out; > } > > + if ( info->vuart && > + (ret = xc_dom_vpl011_init(CTX->xch, > + domid, > + state->console_domid, > + dom->vuart_gfn, > + &state->vuart_port)) != 0 ) { > + LOGE(ERROR, "xc_dom_vpl011_init failed"); > + goto out; > + } > out: > return ret != 0 ? ERROR_FAIL : 0; > } > @@ -788,6 +797,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_domain.c b/tools/libxl/libxl_domain.c > index 08eccd0..1d2c65a 100644 > --- a/tools/libxl/libxl_domain.c > +++ b/tools/libxl/libxl_domain.c > @@ -1028,6 +1028,8 @@ void libxl__destroy_domid(libxl__egc *egc, libxl__destroy_domid_state *dis) > goto out; > } > > + xc_dom_vpl011_deinit(ctx->xch, domid); See my remark above. > + > if (libxl__device_pci_destroy_all(gc, domid) < 0) > LOGD(ERROR, domid, "Pci shutdown failed"); > rc = xc_domain_pause(ctx->xch, domid); > diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c > index 971caec..11707db 100644 > --- a/xen/arch/arm/domctl.c > +++ b/xen/arch/arm/domctl.c > @@ -10,6 +10,7 @@ > #include <xen/sched.h> > #include <xen/hypercall.h> > #include <xen/iocap.h> > +#include <xen/guest_access.h> > #include <xsm/xsm.h> > #include <public/domctl.h> > > @@ -119,6 +120,32 @@ 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: Technically a UART cannot be added after a domain has started. So please add if we are still building the domain (e.g !domain->creation_finished). > + rc = domain_vpl011_init(d, vuart_op->console_domid, > + vuart_op->gfn, &vuart_op->evtchn); It sounds like to me it would make sense to directly pass xen_domctl_vuart_op to domain_vpl011_init. > + if ( !rc ) > + rc = __copy_to_guest(u_domctl, domctl, 1); > + break; > + > + case XEN_DOMCTL_VUART_OP_DEINIT_VPL011: > + domain_vpl011_deinit(d); I was actually expecting domain_vpl011_deinit to be called from arch_domain_destroy. > + rc = 0; > + 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..bfa56cb 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,16 @@ 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 > +#define XEN_DOMCTL_VUART_OP_DEINIT_VPL011 1 > + 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 +1229,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 +1292,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 Mon, May 22, 2017 at 04:05:44PM +0530, Bhupinder Thakur wrote: > Hi Wei, > > > >> --- a/tools/libxl/libxl_dom.c > >> +++ b/tools/libxl/libxl_dom.c > >> @@ -688,6 +688,15 @@ static int libxl__build_dom(libxl__gc *gc, uint32_t domid, > >> goto out; > >> } > >> > >> + if ( info->vuart && > >> + (ret = xc_dom_vpl011_init(CTX->xch, > >> + domid, > >> + state->console_domid, > >> + dom->vuart_gfn, > >> + &state->vuart_port)) != 0 ) { > >> + LOGE(ERROR, "xc_dom_vpl011_init failed"); > >> + goto out; > >> + } > > > > Please push this to arch-specific function. > > > libxl__arch_domain_create() seems to be a good candidate where I can > move this code. But this function does not take > libxl__domain_build_state *state as input an input parameter, which > carries information such as state->vuart_port for intializing vpl011. > Should I modify the function prototype to take that also as input? > This will require changes in the x86 code also, which is using this > function. That's fine. Please do it in a separate patch. Wei. > > Regards, > Bhupinder
diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h index 1629f41..13ef215 100644 --- a/tools/libxc/include/xenctrl.h +++ b/tools/libxc/include/xenctrl.h @@ -884,6 +884,32 @@ int xc_vcpu_getcontext(xc_interface *xch, uint32_t domid, uint32_t vcpu, vcpu_guest_context_any_t *ctxt); +/** + * This function intializes 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 de-intializes the vpl011 emulation. + * + * @parm xch a handle to an open hypervisor interface + * @parm domid the domain to get information from + */ +int xc_dom_vpl011_deinit(xc_interface *xch, + uint32_t domid); /** * 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..ec48b9b 100644 --- a/tools/libxc/xc_domain.c +++ b/tools/libxc/xc_domain.c @@ -343,6 +343,44 @@ 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_dom_vpl011_deinit(xc_interface *xch, + uint32_t domid) +{ + 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_DEINIT_VPL011; + + rc = do_domctl(xch, &domctl); + + return rc; +} + int xc_domain_getinfo(xc_interface *xch, uint32_t first_domid, unsigned int max_doms, diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c index 5d914a5..e7489d9 100644 --- a/tools/libxl/libxl_dom.c +++ b/tools/libxl/libxl_dom.c @@ -688,6 +688,15 @@ static int libxl__build_dom(libxl__gc *gc, uint32_t domid, goto out; } + if ( info->vuart && + (ret = xc_dom_vpl011_init(CTX->xch, + domid, + state->console_domid, + dom->vuart_gfn, + &state->vuart_port)) != 0 ) { + LOGE(ERROR, "xc_dom_vpl011_init failed"); + goto out; + } out: return ret != 0 ? ERROR_FAIL : 0; } @@ -788,6 +797,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_domain.c b/tools/libxl/libxl_domain.c index 08eccd0..1d2c65a 100644 --- a/tools/libxl/libxl_domain.c +++ b/tools/libxl/libxl_domain.c @@ -1028,6 +1028,8 @@ void libxl__destroy_domid(libxl__egc *egc, libxl__destroy_domid_state *dis) goto out; } + xc_dom_vpl011_deinit(ctx->xch, domid); + if (libxl__device_pci_destroy_all(gc, domid) < 0) LOGD(ERROR, domid, "Pci shutdown failed"); rc = xc_domain_pause(ctx->xch, domid); diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c index 971caec..11707db 100644 --- a/xen/arch/arm/domctl.c +++ b/xen/arch/arm/domctl.c @@ -10,6 +10,7 @@ #include <xen/sched.h> #include <xen/hypercall.h> #include <xen/iocap.h> +#include <xen/guest_access.h> #include <xsm/xsm.h> #include <public/domctl.h> @@ -119,6 +120,32 @@ 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: + rc = domain_vpl011_init(d, vuart_op->console_domid, + vuart_op->gfn, &vuart_op->evtchn); + if ( !rc ) + rc = __copy_to_guest(u_domctl, domctl, 1); + break; + + case XEN_DOMCTL_VUART_OP_DEINIT_VPL011: + domain_vpl011_deinit(d); + rc = 0; + 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..bfa56cb 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,16 @@ 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 +#define XEN_DOMCTL_VUART_OP_DEINIT_VPL011 1 + 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 +1229,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 +1292,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 two new domctl APIs to initialize and de-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 guest domain's behalf. Signed-off-by: Bhupinder Thakur <bhupinder.thakur@linaro.org> --- 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 | 26 ++++++++++++++++++++++++++ tools/libxc/xc_domain.c | 38 ++++++++++++++++++++++++++++++++++++++ tools/libxl/libxl_dom.c | 10 ++++++++++ tools/libxl/libxl_domain.c | 2 ++ xen/arch/arm/domctl.c | 27 +++++++++++++++++++++++++++ xen/include/public/domctl.h | 13 +++++++++++++ 6 files changed, 116 insertions(+)