>>> On 12.11.14 at 04:05, <tiejun.c...@intel.com> wrote:
> I don't see any feedback to this point, so I think you still prefer we
> should do all check in the callback function.
As a draft this looks reasonable, but there are various bugs to be
dealt with along with cosmetic issues (I'll point out the former, but
I'm tired of pointing out the latter once again - please go back to
earlier reviews of patches to refresh e.g. what types to use for
loop variables).
> I tried to address this but obviously we have to pass each 'pdf' to
> callback functions,
Yes, but at the generic IOMMU layer this shouldn't be named "bdf",
but something more neutral (maybe "id"). And you again lost the
segment there.
> @@ -36,9 +40,24 @@ static int get_reserved_device_memory(xen_pfn_t start,
> if ( rdm.start_pfn != start || rdm.nr_pages != nr )
> return -ERANGE;
>
> - if ( __copy_to_compat_offset(grdm->map.buffer, grdm->used_entries,
> - &rdm, 1) )
> - return -EFAULT;
> + if ( d->arch.hvm_domain.pci_force )
> + {
> + if ( __copy_to_compat_offset(grdm->map.buffer,
> grdm->used_entries,
> + &rdm, 1) )
> + return -EFAULT;
> + }
> + else
> + {
> + for ( i = 0; i < d->arch.hvm_domain.num_pcidevs; i++ )
> + {
> + pt_bdf = PCI_BDF2(d->arch.hvm_domain.pcidevs[i].bus,
> + d->arch.hvm_domain.pcidevs[i].devfn);
> + if ( pt_bdf == bdf )
> + if ( __copy_to_compat_offset(grdm->map.buffer,
> grdm->used_entries,
> + &rdm, 1) )
> + return -EFAULT;
I think d->arch.hvm_domain.pcidevs[] shouldn't contain duplicates,
and hence there's no point continuing the loop if you found a match.
> + }
> + }
> }
>
> ++grdm->used_entries;
This should no longer get incremented unconditionally.
> @@ -314,6 +333,7 @@ int compat_memory_op(unsigned int cmd,
> XEN_GUEST_HANDLE_PARAM(void) compat)
> return -EFAULT;
>
> grdm.used_entries = 0;
> + grdm.domain = current->domain;
> rc =
> iommu_get_reserved_device_memory(get_reserved_device_memory,
> &grdm);
>
Maybe I misguided you with an earlier response, or maybe the
earlier reply was in a different context: There's no point
communicating current->domain to the callback function; there
would be a point communicating the domain if it was _not_
always current->domain.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
