|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 06/27 v9] xen/arm: vpl011: Add a new domctl API to initialize vpl011
Found one issue in this patch:
In libxl__arch_build_dom_finish, rc is not getting initialized.
Surprisingly, it was working fine in my testing.
Regards,
Bhupinder
On 18 September 2017 at 16:01, Bhupinder Thakur
<bhupinder.thakur@xxxxxxxxxx> 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@xxxxxxxxxx>
> ---
> CC: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> CC: George Dunlap <George.Dunlap@xxxxxxxxxxxxx>
> CC: Jan Beulich <jbeulich@xxxxxxxx>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Tim Deegan <tim@xxxxxxx>
> CC: Julien Grall <julien.grall@xxxxxxx>
>
> Changes since v8:
> - Added explicit padding in the vuart_op structure
> - Moved vuart_op structure after the PSR structure definition
> - The input fields moved before the output fields in vuart_op structure
> - Checking explicitly that padding fields are initialized to 0
>
> Changes since v6:
> - Renamed the vuart initialization function to a generic name
> xc_dom_vuart_init
> - Used domid_t as a type instead of uint32_t for domid
> - Checking the vuart type explicitly against vpl011 enum value
>
> Changes since v5:
> - xc_dom_vpl011_init() will be compiled for both x86/arm architectures as
> there
> is nothing architecture specific in this function. This function will return
> error when called for x86.
> - Fixed coding style issues in libxl.
>
> 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 | 27 ++++++++++++++++++++++
> tools/libxl/libxl_arch.h | 7 ++++++
> tools/libxl/libxl_arm.c | 27 ++++++++++++++++++++++
> tools/libxl/libxl_dom.c | 4 ++++
> tools/libxl/libxl_x86.c | 8 +++++++
> xen/arch/arm/domain.c | 6 +++++
> xen/arch/arm/domctl.c | 52
> +++++++++++++++++++++++++++++++++++++++++++
> xen/include/public/domctl.h | 22 ++++++++++++++++++
> 9 files changed, 173 insertions(+)
>
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index 43151cb..37beb14 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -886,6 +886,26 @@ int xc_vcpu_getcontext(xc_interface *xch,
> vcpu_guest_context_any_t *ctxt);
>
> /**
> + * This function initializes the vuart 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 type type of vuart
> + * @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_vuart_init(xc_interface *xch,
> + uint32_t type,
> + domid_t domid,
> + domid_t console_domid,
> + xen_pfn_t gfn,
> + evtchn_port_t *evtchn);
> +
> +/**
> * This function returns information about the XSAVE state of a particular
> * vcpu of a domain. If extstate->size and extstate->xfeature_mask are 0,
> * the call is considered a query to retrieve them and the buffer is not
> diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
> index 3bab4e8..43720a2 100644
> --- a/tools/libxc/xc_domain.c
> +++ b/tools/libxc/xc_domain.c
> @@ -343,6 +343,33 @@ int xc_domain_get_guest_width(xc_interface *xch,
> uint32_t domid,
> return 0;
> }
>
> +int xc_dom_vuart_init(xc_interface *xch,
> + uint32_t type,
> + domid_t domid,
> + domid_t console_domid,
> + xen_pfn_t gfn,
> + evtchn_port_t *evtchn)
> +{
> + DECLARE_DOMCTL;
> + int rc = 0;
> +
> + memset(&domctl, 0, sizeof(domctl));
> +
> + domctl.cmd = XEN_DOMCTL_vuart_op;
> + domctl.domain = domid;
> + domctl.u.vuart_op.cmd = XEN_DOMCTL_VUART_OP_INIT;
> + domctl.u.vuart_op.type = type;
> + 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..784ec7f 100644
> --- a/tools/libxl/libxl_arch.h
> +++ b/tools/libxl/libxl_arch.h
> @@ -44,6 +44,13 @@ 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 */
> +_hidden
> +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..2e9f780 100644
> --- a/tools/libxl/libxl_arm.c
> +++ b/tools/libxl/libxl_arm.c
> @@ -1038,6 +1038,33 @@ 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 rc, ret;
> +
> + if (info->arch_arm.vuart != LIBXL_VUART_TYPE_SBSA_UART) {
> + rc = 0;
> + goto out;
> + }
> +
> + ret = xc_dom_vuart_init(CTX->xch,
> + XEN_DOMCTL_VUART_TYPE_VPL011,
> + dom->guest_domid,
> + dom->console_domid,
> + dom->vuart_gfn,
> + &state->vuart_port);
> + if (ret < 0) {
> + rc = ERROR_FAIL;
> + LOG(ERROR, "xc_dom_vuart_init failed\n");
> + }
> +
> +out:
> + return rc;
> +}
> +
> 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 e0f0d78..5f92023 100644
> --- a/tools/libxl/libxl_dom.c
> +++ b/tools/libxl/libxl_dom.c
> @@ -702,6 +702,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 6512f01..788dafa 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -874,6 +874,12 @@ int domain_relinquish_resources(struct domain *d)
> if ( ret )
> return ret;
>
> + /*
> + * Release the resources allocated for vpl011 which were
> + * allocated via a DOMCTL call XEN_DOMCTL_vuart_op.
> + */
> + domain_vpl011_deinit(d);
> +
> d->arch.relmem = RELMEM_xen;
> /* Fallthrough */
>
> diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c
> index db6838d..a2de642 100644
> --- a/xen/arch/arm/domctl.c
> +++ b/xen/arch/arm/domctl.c
> @@ -5,9 +5,11 @@
> */
>
> #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 <xen/types.h>
> #include <xsm/xsm.h>
> @@ -20,6 +22,29 @@ void arch_get_domain_info(const struct domain *d,
> info->flags |= XEN_DOMINF_hap;
> }
>
> +static int handle_vuart_init(struct domain *d,
> + struct xen_domctl_vuart_op *vuart_op)
> +{
> + int rc;
> + struct vpl011_init_info info;
> +
> + info.console_domid = vuart_op->console_domid;
> + info.gfn = _gfn(vuart_op->gfn);
> +
> + if ( d->creation_finished )
> + return -EPERM;
> +
> + if ( vuart_op->type != XEN_DOMCTL_VUART_TYPE_VPL011 )
> + return -EOPNOTSUPP;
> +
> + rc = domain_vpl011_init(d, &info);
> +
> + if ( !rc )
> + vuart_op->evtchn = info.evtchn;
> +
> + return rc;
> +}
> +
> long arch_do_domctl(struct xen_domctl *domctl, struct domain *d,
> XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
> {
> @@ -119,6 +144,33 @@ 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;
> + unsigned int i;
> + struct xen_domctl_vuart_op *vuart_op = &domctl->u.vuart_op;
> +
> + /* check that structure padding must be 0. */
> + for ( i = 0; i < sizeof(vuart_op->pad); i++ )
> + if ( vuart_op->pad[i] )
> + return -EINVAL;
> +
> + switch( vuart_op->cmd )
> + {
> + case XEN_DOMCTL_VUART_OP_INIT:
> + rc = handle_vuart_init(d, vuart_op);
> + break;
> +
> + default:
> + rc = -EINVAL;
> + break;
> + }
> +
> + if ( !rc )
> + rc = __copy_to_guest(u_domctl, domctl, 1);
> +
> + return rc;
> + }
> default:
> {
> int rc;
> diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
> index 50ff58f..17f98eb 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
>
> @@ -1160,9 +1161,28 @@ struct xen_domctl_psr_cat_op {
> uint32_t target; /* IN */
> uint64_t data; /* IN/OUT */
> };
> +
> typedef struct xen_domctl_psr_cat_op xen_domctl_psr_cat_op_t;
> DEFINE_XEN_GUEST_HANDLE(xen_domctl_psr_cat_op_t);
>
> +struct xen_domctl_vuart_op {
> +#define XEN_DOMCTL_VUART_OP_INIT 0
> + uint32_t cmd; /* XEN_DOMCTL_VUART_OP_* */
> +#define XEN_DOMCTL_VUART_TYPE_VPL011 0
> + uint32_t type; /* IN - type of vuart.
> + * Currently only vpl011 supported.
> + */
> + uint64_aligned_t gfn; /* IN - guest gfn to be used as a
> + * ring buffer.
> + */
> + domid_t console_domid; /* IN */
> + uint8_t pad[2];
> + evtchn_port_t evtchn; /* OUT - remote port of the event
> + * channel used for sending
> + * ring buffer events.
> + */
> +};
> +
> struct xen_domctl {
> uint32_t cmd;
> #define XEN_DOMCTL_createdomain 1
> @@ -1240,6 +1260,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
> @@ -1302,6 +1323,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
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |