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

[Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>]

On 01/07/2015 10:17 AM, Jan Beulich wrote:
> > > > On 07.01.15 at 15:55, <boris.ostrovsky@xxxxxxxxxx> wrote:
> > On 01/07/2015 04:21 AM, Jan Beulich wrote:
> > > > > > On 06.01.15 at 03:18, <boris.ostrovsky@xxxxxxxxxx> wrote:
> > > > +        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.
> Building your argumentation on potentially mis-designed existing
> interfaces is bogus. The question is - what use is a Xen internal
> node number to a caller of a particular hypercall (other than it
> being purely informational, e.g. for printing human readable
> output)?
Just as with knowing CPU/memory topology --- this will help with placing 
a guest if we know what "proximity domain" both the device and the 
CPUs/memory belong to.
Exposing PXM values to the caller would be as good as those internal 
node IDs. The only (I think) problem is that PXMs are not necessarily 
zero-based and may not be contiguous and so we need to have some sort of 
a common mapping for both CPUs and devices. And hypervisor provides such 
mapping in persistent way.
And if we are going to keep this as a sysctl then we need to be 
consistent with what we do now for CPUs, which is pxm2node[]. Or change 
CPU topology sysctl as well, which I don't think is a good idea.
> In particular if we were to introduce a new non-sysctl interface,
> determining whether the hypervisor internal representation is really
> the right one to expose here should be one of the most important
> design aspects.
Yes.

> I personally think that exposing e.g. the firmware
> determined (and hence hopefully stable across reboots) PXM would
> be more reasonable.
Again, the main argument that I see against using PXM values directly is 
the fact that it's not zero-based/non-contiguous.
-boris


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