[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <840a6943-87ab-938f-5f8d-3a2c21e08549@linux.intel.com>
Date: Fri, 5 Jul 2019 10:21:27 +0800
From: Lu Baolu <baolu.lu@...ux.intel.com>
To: Jacob Pan <jacob.jun.pan@...ux.intel.com>
Cc: baolu.lu@...ux.intel.com, iommu@...ts.linux-foundation.org,
LKML <linux-kernel@...r.kernel.org>,
Joerg Roedel <joro@...tes.org>,
David Woodhouse <dwmw2@...radead.org>,
Eric Auger <eric.auger@...hat.com>,
Alex Williamson <alex.williamson@...hat.com>,
Jean-Philippe Brucker <jean-philippe.brucker@....com>,
Yi Liu <yi.l.liu@...el.com>,
"Tian, Kevin" <kevin.tian@...el.com>,
Raj Ashok <ashok.raj@...el.com>,
Christoph Hellwig <hch@...radead.org>,
Andriy Shevchenko <andriy.shevchenko@...ux.intel.com>
Subject: Re: [PATCH v4 20/22] iommu/vt-d: Add bind guest PASID support
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.
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
Powered by blists - more mailing lists