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

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 12/03/2014 10:20 AM, Andrew Cooper wrote:
> 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 don't follow this. These are indeed lists with different indicies but 
why can't they both be part of this struct?
-boris

> 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