Re: [Xen-devel] [PATCH v2 3/4] sysctl: Add sysctl interface for querying PCI topology

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 01/07/2015 04:21 AM, Jan Beulich wrote:
> > > > On 06.01.15 at 03:18, <boris.ostrovsky@xxxxxxxxxx> wrote:
> > --- a/xen/common/sysctl.c
> > +++ b/xen/common/sysctl.c
> > @@ -365,6 +365,66 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) 
> > u_sysctl)
> >       }
> >       break;
> >   
> #ifdef HAS_PCI
>
> > +    case XEN_SYSCTL_pcitopoinfo:
> > +    {
> > +        xen_sysctl_pcitopoinfo_t *ti = &op->u.pcitopoinfo;
> > +
> > +        if ( guest_handle_is_null(ti->pcitopo) ||
> > +             (ti->first_dev >= ti->num_devs) )
> > +        {
> > +            ret = -EINVAL;
> > +            break;
> > +        }
> > +
> > +        for ( ; ti->first_dev < ti->num_devs; ti->first_dev++ )
> > +        {
> > +            xen_sysctl_pcitopo_t pcitopo;
> > +            struct pci_dev *pdev;
> > +
> > +            if ( copy_from_guest_offset(&pcitopo, ti->pcitopo,
> > +                                        ti->first_dev, 1) )
> > +            {
> > +                ret = -EFAULT;
> > +                break;
> > +            }
> > +
> > +            spin_lock(&pcidevs_lock);
> > +            pdev = pci_get_pdev(pcitopo.pcidev.seg, pcitopo.pcidev.bus,
> > +                                pcitopo.pcidev.devfn);
> > +            if ( !pdev || (pdev->node == NUMA_NO_NODE) )
> > +                pcitopo.node = INVALID_TOPOLOGY_ID;
> > +            else
> > +                pcitopo.node = pdev->node;
> Are hypervisor-internal node numbers really meaningful to the caller?

This is the same information (pxm -> node mapping ) that we provide in 
XEN_SYSCTL_topologyinfo (renamed in this series to
XEN_SYSCTL_cputopoinfo). Given that I expect the two topologies to be 
used together I think the answer is yes.

> > @@ -463,7 +464,7 @@ typedef struct xen_sysctl_lockprof_op 
> > xen_sysctl_lockprof_op_t;
> >   DEFINE_XEN_GUEST_HANDLE(xen_sysctl_lockprof_op_t);
> >   
> >   /* XEN_SYSCTL_cputopoinfo */
> > -#define INVALID_TOPOLOGY_ID  (~0U)
> > +#define INVALID_TOPOLOGY_ID  (~0U) /* Also used by pcitopo */
> Better extend the preceding comment.
I mentioned it to Wei yesterday that the file is structured (or at least 
appears to me to be structured) in such a way that these top comments 
mark sections of definitions for each sysctl. And so I thought that I'd 
be breaking this convention if I were to extend the comment.
-boris

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