Re: [Xen-devel] [PATCH 2/4] sysctl/libxl: Add interface for returning IO topology data

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 02/12/14 21:34, Boris Ostrovsky wrote:
>  /* XEN_SYSCTL_topologyinfo */
>  #define INVALID_TOPOLOGY_ID  (~0U)
> +
> +struct xen_sysctl_cputopo {
> +    uint32_t core;
> +    uint32_t socket;
> +    uint32_t node;
> +};
> +typedef struct xen_sysctl_cputopo xen_sysctl_cputopo_t;
> +DEFINE_XEN_GUEST_HANDLE(xen_sysctl_cputopo_t);
> +
> +struct xen_sysctl_iotopo {
> +    uint16_t seg;
> +    uint8_t bus;
> +    uint8_t devfn;
> +    uint32_t node;
> +};
> +typedef struct xen_sysctl_iotopo xen_sysctl_iotopo_t;
> +DEFINE_XEN_GUEST_HANDLE(xen_sysctl_iotopo_t);
> +
>  struct xen_sysctl_topologyinfo {
>      /*
>       * IN: maximum addressable entry in the caller-provided arrays.
> -     * OUT: largest cpu identifier in the system.
> +     * OUT: largest cpu identifier or max number of devices in the system.
>       * If OUT is greater than IN then the arrays are truncated!
>       * If OUT is leass than IN then the array tails are not written by 
> sysctl.
>       */
>      uint32_t max_cpu_index;
> +    uint32_t max_devs;
>  
>      /*
>       * If not NULL, these arrays are filled with core/socket/node identifier
> -     * for each cpu.
> -     * If a cpu has no core/socket/node information (e.g., cpu not present) 
> -     * then the sentinel value ~0u is written to each array.
> -     * The number of array elements written by the sysctl is:
> +     * for each cpu and/or node for each PCI device.
> +     * If information for a particular entry is not avalable it is set to
> +     * INVALID_TOPOLOGY_ID.
> +     * The number of array elements for CPU topology written by the sysctl 
> is:
>       *   min(@max_cpu_index_IN,@max_cpu_index_OUT)+1
>       */
> -    XEN_GUEST_HANDLE_64(uint32) cpu_to_core;
> -    XEN_GUEST_HANDLE_64(uint32) cpu_to_socket;
> -    XEN_GUEST_HANDLE_64(uint32) cpu_to_node;
> +    XEN_GUEST_HANDLE_64(xen_sysctl_cputopo_t) cputopo;
> +    XEN_GUEST_HANDLE_64(xen_sysctl_iotopo_t) iotopo;

These are inherently lists with different indicies.  They should not
conglomerated like this.

I would suggest introducing a new hypercall (xen_sysctl_iotopologyinfo
?) and leave this one alone.

~Andrew

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