Re: [Xen-devel] [PATCH 04/11] xen/arm: vpl011: Enable vpl011 emulation for a domain in Xen

[Julien Grall <julien.grall@xxxxxxx>]

(CC the REST maintainer)

Hi Bhupinder,

On 02/21/2017 11:26 AM, Bhupinder Thakur wrote:
> Based on one of the domain creation flags, the vpl011 emulation will be enabled 
> for
> the domain.
>
> This flag is currently set always but finally it needs to be controlled by the 
> user
> through some configuration option.
We really don't want the pl011 emulation by default. You should provide 
a libxl option to enable pl011 emulation. Also without the next patch, 
this will break guest boot as SPIs are not correctly initialized.
You likely want to re-order the two.

> Signed-off-by: Bhupinder Thakur <bhupinder.thakur@xxxxxxxxxx>
> ---
>  xen/arch/arm/domain.c   | 8 ++++++++
>  xen/common/domctl.c     | 2 ++
>  xen/include/xen/sched.h | 4 ++++
>  3 files changed, 14 insertions(+)
>
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 7e43691..6fee541 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -36,6 +36,9 @@
>  #include <asm/platform.h>
>  #include "vtimer.h"
>  #include "vuart.h"
> +#ifdef CONFIG_VPL011_CONSOLE
> +#include "vpl011.h"
> +#endif
>
>  DEFINE_PER_CPU(struct vcpu *, curr_vcpu);
>
> @@ -626,6 +629,11 @@ int arch_domain_create(struct domain *d, unsigned int 
> domcr_flags,
>      if ( (rc = domain_vtimer_init(d, config)) != 0 )
>          goto fail;
>
> +#ifdef CONFIG_VPL011_CONSOLE
> +    if ( domcr_flags & DOMCRF_vpl011_console )
> +        if ( (rc = domain_vpl011_init(d)) != 0 )
> +            goto fail;
> +#endif
You likely want to return an error if the toolstack request the pl011 
emulation and it is not present in Xen.
So I would add a stub for domain_vpl011_init to return -ENOSYS when the 
pl011 emulation does not exist in Xen.
>      update_domain_wallclock_time(d);
>
>      /*
> diff --git a/xen/common/domctl.c b/xen/common/domctl.c
> index 12cf4a9..fafc506 100644
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -551,6 +551,8 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
> u_domctl)
>          if ( op->u.createdomain.flags & XEN_DOMCTL_CDF_xs_domain )
>              domcr_flags |= DOMCRF_xs_domain;
>
> +        domcr_flags |= DOMCRF_vpl011_console;
> +
Please don't do that. See my suggestion above.


>          d = domain_create(dom, domcr_flags, op->u.createdomain.ssidref,
>                            &op->u.createdomain.config);
>          if ( IS_ERR(d) )
> diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
> index 063efe6..9e03a60 100644
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -555,6 +555,10 @@ struct domain *domain_create(domid_t domid, unsigned int 
> domcr_flags,
>  #define _DOMCRF_xs_domain       6
>  #define DOMCRF_xs_domain        (1U<<_DOMCRF_xs_domain)
>
> + /* DOMCRF_vpl011_console: create domain with pl011 emulation enabled */
> +#define _DOMCRF_vpl011_console  7
> +#define DOMCRF_vpl011_console   (1U<<_DOMCRF_vpl011_console)
> +
>  /*
>   * rcu_lock_domain_by_id() is more efficient than get_domain_by_id().
>   * This is the preferred function if the returned domain reference
Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel