From mboxrd@z Thu Jan  1 00:00:00 1970
From: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Subject: Re: [PATCH v2 3/4] sysctl: Add sysctl interface for
 querying PCI topology
Date: Wed, 07 Jan 2015 09:55:13 -0500
Message-ID: <54AD48D1.5000904@oracle.com>
References: <1420510737-22813-1-git-send-email-boris.ostrovsky@oracle.com>
	<1420510737-22813-4-git-send-email-boris.ostrovsky@oracle.com>
	<54AD08AE02000078000522B7@mail.emea.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <54AD08AE02000078000522B7@mail.emea.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: wei.liu2@citrix.com, ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com, dario.faggioli@citrix.com, ian.jackson@eu.citrix.com, xen-devel@lists.xen.org, ufimtseva@gmail.com, keir@xen.org
List-Id: xen-devel@lists.xenproject.org

On 01/07/2015 04:21 AM, Jan Beulich wrote:
>>>> On 06.01.15 at 03:18, <boris.ostrovsky@oracle.com> 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