Re: [PATCH v2 13/21] xen/dts: Add hypercalls to retrieve device node information

["Jan Beulich" <JBeulich@suse.com>]

>>> On 31.07.14 at 17:00, <julien.grall@linaro.org> wrote:
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -315,6 +315,7 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
>      case XEN_DOMCTL_createdomain:
>      case XEN_DOMCTL_getdomaininfo:
>      case XEN_DOMCTL_test_assign_device:
> +    case XEN_DOMCTL_dtdev_op:
>          d = NULL;
>          break;
>      default:
> @@ -1017,6 +1018,15 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
>      }
>      break;
>  
> +#ifdef HAS_DEVICE_TREE
> +    case XEN_DOMCTL_dtdev_op:
> +    {
> +        ret = dt_do_domctl(op);
> +        copyback = 1;
> +    }
> +    break;
> +#endif

No pointless braces please.

> --- a/xen/include/public/domctl.h
> +++ b/xen/include/public/domctl.h
> @@ -946,6 +946,44 @@ typedef struct xen_domctl_vcpu_msrs xen_domctl_vcpu_msrs_t;
>  DEFINE_XEN_GUEST_HANDLE(xen_domctl_vcpu_msrs_t);
>  #endif
>  
> +/* Device Tree: Retrieve informations about a device node */
> +struct xen_domctl_dtdev_op {
> +    /* IN */
> +    uint32_t plen;                  /* Length of the path */
> +    XEN_GUEST_HANDLE(char) path;    /* Path to the device tree node */

XEN_GUEST_HANDLE_64? And padding between the two above
members, or fields re-ordered?

> +#define DOMCTL_DTDEV_GET_INFO        0
> +#define DOMCTL_DTDEV_GET_IRQ         1
> +#define DOMCTL_DTDEV_GET_MMIO        2
> +#define DOMCTL_DTDEV_GET_COMPAT      3
> +    uint8_t op;
> +    uint32_t pad0:24;

uint8_t pad0[3].

> +    uint32_t index;                 /* Index for the IRQ/MMIO to retrieve */
> +    /* OUT */
> +    union {
> +        struct {
> +            uint32_t num_irqs;      /* Number of IRQs */
> +            uint32_t num_mmios;     /* Number of MMIOs */
> +            uint32_t compat_len;    /* Length of the compatible string */
> +        } info;
> +        struct {
> +            /* TODO: Do we need to handle MSI-X? */
> +            uint32_t irq;           /* IRQ number */
> +            /* TODO: Describe with defines the IRQ type */

???

Also, are you planning to address these two TODOs before this gets
ready to be committed?

> +            uint32_t type;          /* IRQ type (i.e edge, level...) */

#define-s to specify what values here mean?

> +        } irq;
> +        struct {
> +            xen_pfn_t mfn;
> +            xen_pfn_t nr_mfn;
> +        } mmio;
> +        struct {
> +            uint32_t len;           /* IN: Size of buffer. OUT: Size copied */
> +            XEN_GUEST_HANDLE_64(char) compat;

Padding again?

Jan