Re: [PATCH 08/17 v5] xen/arm: vpl011: Add a new domctl API to initialize vpl011

[Julien Grall <julien.grall@arm.com>]

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,

-- 
Julien Grall

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