On 03/13/2015 12:03 PM, Jan Beulich wrote:
On 10.03.15 at 03:27, <boris.ostrov...@oracle.com> wrote:+ case XEN_SYSCTL_pcitopoinfo: + { + xen_sysctl_pcitopoinfo_t *ti = &op->u.pcitopoinfo; + + if ( guest_handle_is_null(ti->devs) || + guest_handle_is_null(ti->nodes) || + (ti->first_dev > ti->num_devs) ) + { + ret = -EINVAL; + break; + } + + for ( ; ti->first_dev < ti->num_devs; ti->first_dev++ ) + { + physdev_pci_device_t dev; + uint8_t node; + struct pci_dev *pdev; + + if ( copy_from_guest_offset(&dev, ti->devs, ti->first_dev, 1) ) + { + ret = -EFAULT; + break; + } + + spin_lock(&pcidevs_lock); + pdev = pci_get_pdev(dev.seg, dev.bus, dev.devfn); + if ( !pdev || (pdev->node == NUMA_NO_NODE) ) + node = XEN_INVALID_NODE_ID; + else + node = pdev->node; + spin_unlock(&pcidevs_lock); + + if ( copy_to_guest_offset(ti->nodes, ti->first_dev, &node, 1) ) + { + ret = -EFAULT; + break; + }+ if ( hypercall_preempt_check() )+ break; + } + + if ( !ret ) + { + ti->first_dev++;This is correct in the preempt case, but it seems wrong to me in the completely-done one. Perhaps it would be better to put the increment right before the preempt check?
Callers are really not supposed to use this field as it is useless as output.
But for consistency (and to make this look more natural) --- yes, moving it to the loop above (which will become a 'while') would be better. -boris _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
