Re: [Xen-devel] [PATCH 03/10 v2] xen/arm: vpl011: Enable pl011 emulation for a guest domain in Xen

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.04.17 at 18:01, <bhupinder.thakur@xxxxxxxxxx> wrote:
> @@ -631,6 +632,9 @@ int arch_domain_create(struct domain *d, unsigned int 
> domcr_flags,
>      if ( (rc = domain_vtimer_init(d, config)) != 0 )
>          goto fail;
>  
> +    if ( domcr_flags & DOMCRF_vuart )
> +        if ( (rc = domain_vpl011_init(d, config)) != 0 )
> +            goto fail;
>      update_domain_wallclock_time(d);

This is ARM code, so my opinion may not mean much, but why two
if()s instead of one, and why no blank line after your addition?

> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -63,6 +63,8 @@ struct xen_domctl_createdomain {
>   /* Is this a xenstore domain? */
>  #define _XEN_DOMCTL_CDF_xs_domain     4
>  #define XEN_DOMCTL_CDF_xs_domain      (1U<<_XEN_DOMCTL_CDF_xs_domain)
> +#define _XEN_DOMCTL_VUART_enable      6
> +#define XEN_DOMCTL_VUART_enable       (1U<<_XEN_DOMCTL_VUART_enable)

As expressed before, I object to this addition, as doing things this
way does not scale. I don't think I've seen any proper reply to my
previous objection, and the patch description here certainly does
not justify why an exception should be made in this case. As a
side note, why bit 6 instead of the first available one (5)?

Jan


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