On Fri, 5 Jul 2019 10:21:27 +0800
Lu Baolu <baolu...@linux.intel.com> wrote:
> Hi Jacob,
>
> On 6/28/19 4:22 AM, Jacob Pan wrote:
> >>> + }
> >>> + refcount_set(&svm->refs, 0);
> >>> + ioasid_set_data(data->hpasid, svm);
> >>> + INIT_LIST_HEAD_RCU(&svm->devs);
> >>> + INIT_LIST_HEAD(&svm->list);
> >>> +
> >>> + mmput(svm->mm);
> >>> + }
> >>> + sdev = kzalloc(sizeof(*sdev), GFP_KERNEL);
> >>> + if (!sdev) {
> >>> + ret = -ENOMEM;
> >>> + goto out;
> >> I think you need to clean up svm if its device list is empty here,
> >> as you said above:
> >>
> > No, we come here only if the device list is not empty and the new
> > device to bind is different than any existing device in the list.
> > If we cannot allocate memory for the new device, should not free
> > the existing SVM, right?
> >
>
> I'm sorry, but the code doesn't show this. We come here even an svm
> data structure was newly created with an empty device list. I post
> the code below to ensure that we are reading a same piece of code.
>
Sorry for the delay. You are right, I need to clean up svm if device
list is empty.
Thanks!
> mutex_lock(&pasid_mutex);
> svm = ioasid_find(NULL, data->hpasid, NULL);
> if (IS_ERR(svm)) {
> ret = PTR_ERR(svm);
> goto out;
> }
> if (svm) {
> /*
> * If we found svm for the PASID, there must be at
> * least one device bond, otherwise svm should be
> freed. */
> BUG_ON(list_empty(&svm->devs));
>
> for_each_svm_dev() {
> /* In case of multiple sub-devices of the
> same pdev assigned, we should
> * allow multiple bind calls with the same
> PASID and pdev.
> */
> sdev->users++;
> goto out;
> }
> } else {
> /* We come here when PASID has never been bond to a
> device. */
> svm = kzalloc(sizeof(*svm), GFP_KERNEL);
> if (!svm) {
> ret = -ENOMEM;
> goto out;
> }
> /* REVISIT: upper layer/VFIO can track host process
> that bind the PASID.
> * ioasid_set = mm might be sufficient for vfio to
> check pasid VMM
> * ownership.
> */
> svm->mm = get_task_mm(current);
> svm->pasid = data->hpasid;
> if (data->flags & IOMMU_SVA_GPASID_VAL) {
> svm->gpasid = data->gpasid;
> svm->flags &= SVM_FLAG_GUEST_PASID;
> }
> refcount_set(&svm->refs, 0);
> ioasid_set_data(data->hpasid, svm);
> INIT_LIST_HEAD_RCU(&svm->devs);
> INIT_LIST_HEAD(&svm->list);
>
> mmput(svm->mm);
> }
> sdev = kzalloc(sizeof(*sdev), GFP_KERNEL);
> if (!sdev) {
> ret = -ENOMEM;
> goto out;
> }
> sdev->dev = dev;
> sdev->users = 1;
>
> Best regards,
> Baolu
[Jacob Pan]
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
