Re: [PATCH v11 05/10] iommu/vt-d: Add bind guest PASID support
On Fri, 17 Apr 2020 23:46:13 +
"Tian, Kevin" wrote:
> > From: Jacob Pan
> > Sent: Friday, April 17, 2020 11:29 PM
> >
> > On Fri, 17 Apr 2020 09:46:55 +0200
> > Auger Eric wrote:
> >
> > > Hi Kevin,
> > > On 4/17/20 4:45 AM, Tian, Kevin wrote:
> > > >> From: Auger Eric
> > > >> Sent: Thursday, April 16, 2020 6:43 PM
> > > >>
> > > > [...]
> > > > + if (svm) {
> > > > + /*
> > > > +* If we found svm for the PASID, there
> > > > must be at
> > > > +* least one device bond, otherwise svm
> > > > should be freed.
> > > > +*/
> > > > + if (WARN_ON(list_empty(>devs))) {
> > > > + ret = -EINVAL;
> > > > + goto out;
> > > > + }
> > > > +
> > > > + for_each_svm_dev(sdev, 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++;
> > > What if this is not an mdev device. Is it also allowed?
> > > >>> Yes. IOMMU and VT-d driver is not mdev aware. Here mdev is
> > > >>> just an example of normal use case. You can bind the same PCI
> > > >>> device (PF or SRIOV VF) more than once to the same PASID.
> > > >>> Just need to unbind also.
> > > >>
> > > >> I don't get the point of binding a non mdev device several
> > > >> times with the same PASID. Do you intend to allow that at
> > > >> userspace level or prevent this from happening in VFIO?
> > > >
> > > > I feel it's better to prevent this from happening, otherwise
> > > > VFIO also needs to track the bind count and do multiple unbinds
> > > > at mm_exit. But it's not necessary to prevent it in VFIO. We
> > > > can check here upon whether aux_domain is valid, and if not
> > > > return -EBUSY.
> > > Ah OK. So if we can detect the case here it is even better
> > >
> > I don't understand why VFIO cannot track, since it is mdev aware.
> > if we don;t refcount the users, one mdev unbind will result unbind
> > for all mdev under the same pdev. That may not be the right thing
> > to do.
>
> The open here is not for mdev, which refcount is still required.
> Eric's point is for non-mdev endpoints. It's meaningless and not
> intuitive to allow binding a PASID multiple-times to the same device.
>
That seems contradictory. The refcount here is intended/required for sub
devices such as mdev. Since IOMMU driver is not mdev aware, we cannot
treat devices differently.
Perhaps Yi can clarify if this case is handled within VFIO, then I can
drop the refcount here.
> Thanks
> Kevin
[Jacob Pan]
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v11 05/10] iommu/vt-d: Add bind guest PASID support
> From: Jacob Pan
> Sent: Friday, April 17, 2020 11:29 PM
>
> On Fri, 17 Apr 2020 09:46:55 +0200
> Auger Eric wrote:
>
> > Hi Kevin,
> > On 4/17/20 4:45 AM, Tian, Kevin wrote:
> > >> From: Auger Eric
> > >> Sent: Thursday, April 16, 2020 6:43 PM
> > >>
> > > [...]
> > > + if (svm) {
> > > + /*
> > > + * If we found svm for the PASID, there must
> > > be at
> > > + * least one device bond, otherwise svm should
> > > be freed.
> > > + */
> > > + if (WARN_ON(list_empty(>devs))) {
> > > + ret = -EINVAL;
> > > + goto out;
> > > + }
> > > +
> > > + for_each_svm_dev(sdev, 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++;
> > What if this is not an mdev device. Is it also allowed?
> > >>> Yes. IOMMU and VT-d driver is not mdev aware. Here mdev is just an
> > >>> example of normal use case. You can bind the same PCI device (PF
> > >>> or SRIOV VF) more than once to the same PASID. Just need to
> > >>> unbind also.
> > >>
> > >> I don't get the point of binding a non mdev device several times
> > >> with the same PASID. Do you intend to allow that at userspace
> > >> level or prevent this from happening in VFIO?
> > >
> > > I feel it's better to prevent this from happening, otherwise VFIO
> > > also needs to track the bind count and do multiple unbinds at
> > > mm_exit. But it's not necessary to prevent it in VFIO. We can check
> > > here upon whether aux_domain is valid, and if not return -EBUSY.
> > Ah OK. So if we can detect the case here it is even better
> >
> I don't understand why VFIO cannot track, since it is mdev aware. if we
> don;t refcount the users, one mdev unbind will result unbind for all
> mdev under the same pdev. That may not be the right thing to do.
>
The open here is not for mdev, which refcount is still required. Eric's
point is for non-mdev endpoints. It's meaningless and not intuitive
to allow binding a PASID multiple-times to the same device.
Thanks
Kevin
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 05/10] iommu/vt-d: Add bind guest PASID support
On Fri, 17 Apr 2020 09:46:55 +0200
Auger Eric wrote:
> Hi Kevin,
> On 4/17/20 4:45 AM, Tian, Kevin wrote:
> >> From: Auger Eric
> >> Sent: Thursday, April 16, 2020 6:43 PM
> >>
> > [...]
> > + if (svm) {
> > + /*
> > +* If we found svm for the PASID, there must
> > be at
> > +* least one device bond, otherwise svm should
> > be freed.
> > +*/
> > + if (WARN_ON(list_empty(>devs))) {
> > + ret = -EINVAL;
> > + goto out;
> > + }
> > +
> > + for_each_svm_dev(sdev, 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++;
> What if this is not an mdev device. Is it also allowed?
> >>> Yes. IOMMU and VT-d driver is not mdev aware. Here mdev is just an
> >>> example of normal use case. You can bind the same PCI device (PF
> >>> or SRIOV VF) more than once to the same PASID. Just need to
> >>> unbind also.
> >>
> >> I don't get the point of binding a non mdev device several times
> >> with the same PASID. Do you intend to allow that at userspace
> >> level or prevent this from happening in VFIO?
> >
> > I feel it's better to prevent this from happening, otherwise VFIO
> > also needs to track the bind count and do multiple unbinds at
> > mm_exit. But it's not necessary to prevent it in VFIO. We can check
> > here upon whether aux_domain is valid, and if not return -EBUSY.
> Ah OK. So if we can detect the case here it is even better
>
I don't understand why VFIO cannot track, since it is mdev aware. if we
don;t refcount the users, one mdev unbind will result unbind for all
mdev under the same pdev. That may not be the right thing to do.
> Thanks
>
> Eric
> >
> >>
> >> Besides, the comment is a bit misleading as it gives the
> >> impression it is only true for mdev and there is no associated
> >> check.
> >
> > Thanks
> > Kevin
> >
>
[Jacob Pan]
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 05/10] iommu/vt-d: Add bind guest PASID support
Hi Kevin,
On 4/17/20 4:45 AM, Tian, Kevin wrote:
>> From: Auger Eric
>> Sent: Thursday, April 16, 2020 6:43 PM
>>
> [...]
> + if (svm) {
> + /*
> + * If we found svm for the PASID, there must be at
> + * least one device bond, otherwise svm should be
> freed.
> + */
> + if (WARN_ON(list_empty(>devs))) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + for_each_svm_dev(sdev, 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++;
What if this is not an mdev device. Is it also allowed?
>>> Yes. IOMMU and VT-d driver is not mdev aware. Here mdev is just an
>>> example of normal use case. You can bind the same PCI device (PF or
>>> SRIOV VF) more than once to the same PASID. Just need to unbind also.
>>
>> I don't get the point of binding a non mdev device several times with
>> the same PASID. Do you intend to allow that at userspace level or
>> prevent this from happening in VFIO?
>
> I feel it's better to prevent this from happening, otherwise VFIO also
> needs to track the bind count and do multiple unbinds at mm_exit.
> But it's not necessary to prevent it in VFIO. We can check here
> upon whether aux_domain is valid, and if not return -EBUSY.
Ah OK. So if we can detect the case here it is even better
Thanks
Eric
>
>>
>> Besides, the comment is a bit misleading as it gives the impression it
>> is only true for mdev and there is no associated check.
>
> Thanks
> Kevin
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH v11 05/10] iommu/vt-d: Add bind guest PASID support
> From: Auger Eric
> Sent: Thursday, April 16, 2020 6:43 PM
>
[...]
> >>> + if (svm) {
> >>> + /*
> >>> + * If we found svm for the PASID, there must be at
> >>> + * least one device bond, otherwise svm should be
> >>> freed.
> >>> + */
> >>> + if (WARN_ON(list_empty(>devs))) {
> >>> + ret = -EINVAL;
> >>> + goto out;
> >>> + }
> >>> +
> >>> + for_each_svm_dev(sdev, 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++;
> >> What if this is not an mdev device. Is it also allowed?
> > Yes. IOMMU and VT-d driver is not mdev aware. Here mdev is just an
> > example of normal use case. You can bind the same PCI device (PF or
> > SRIOV VF) more than once to the same PASID. Just need to unbind also.
>
> I don't get the point of binding a non mdev device several times with
> the same PASID. Do you intend to allow that at userspace level or
> prevent this from happening in VFIO?
I feel it's better to prevent this from happening, otherwise VFIO also
needs to track the bind count and do multiple unbinds at mm_exit.
But it's not necessary to prevent it in VFIO. We can check here
upon whether aux_domain is valid, and if not return -EBUSY.
>
> Besides, the comment is a bit misleading as it gives the impression it
> is only true for mdev and there is no associated check.
Thanks
Kevin
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 05/10] iommu/vt-d: Add bind guest PASID support
Hi Jacob,
On 4/10/20 11:06 PM, Jacob Pan wrote:
> Hi Eric,
>
> Missed a few things in the last reply.
>
> On Thu, 9 Apr 2020 09:41:32 +0200
> Auger Eric wrote:
>
>>> + intel_pasid_tear_down_entry(iommu, dev,
>>> svm->pasid);
>> intel_svm_unbind_mm() calls intel_flush_svm_range_dev(svm, sdev, 0,
>> -1, 0); Don't we need to flush the (DEV-)IOTLBs as well?
> Right, pasid tear down should always include (DEV-)IOTLB flush, I
> initially thought it is taken care of by intel_pasid_tear_down_entry().
>
>>> + /* TODO: Drain in flight PRQ for the PASID
>>> since it
>>> +* may get reused soon, we don't want to
>>> +* confuse with its previous life.
>>> +* intel_svm_drain_prq(dev, pasid);
>>> +*/
>>> + kfree_rcu(sdev, rcu);
>>> +
>>> + if (list_empty(>devs)) {
>>> + /*
>>> +* We do not free the IOASID here
>>> in that
>>> +* IOMMU driver did not allocate
>>> it.
>> s/in/as?
> I meant to say "in that" as "for the reason that"
ok sorry
>
>>> +* Unlike native SVM, IOASID for
>>> guest use was
>>> +* allocated prior to the bind
>>> call.> + * In any case, if the free
>>> call comes before
>>> +* the unbind, IOMMU driver will
>>> get notified
>>> +* and perform cleanup.
>>> +*/
>>> + ioasid_set_data(pasid, NULL);
>>> + kfree(svm);
>>> + }
>> nit: you may use intel_svm_free_if_empty()
> True, but I meant to insert ioasid_notifier under the list_empty
> condition in the following ioasid patch.
ok
Thanks
Eric
>
>
> Thanks,
>
> Jacob
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 05/10] iommu/vt-d: Add bind guest PASID support
Hi Jacob,
On 4/10/20 9:45 PM, Jacob Pan wrote:
> Hi Eric,
>
> On Thu, 9 Apr 2020 09:41:32 +0200
> Auger Eric wrote:
>
>> Hi Jacob,
>>
>> On 4/3/20 8:42 PM, Jacob Pan wrote:
>>> When supporting guest SVA with emulated IOMMU, the guest PASID
>>> table is shadowed in VMM. Updates to guest vIOMMU PASID table
>>> will result in PASID cache flush which will be passed down to
>>> the host as bind guest PASID calls.
>>>
>>> For the SL page tables, it will be harvested from device's
>>> default domain (request w/o PASID), or aux domain in case of
>>> mediated device.
>>>
>>> .-. .---.
>>> | vIOMMU| | Guest process CR3, FL only|
>>> | | '---'
>>> ./
>>> | PASID Entry |--- PASID cache flush -
>>> '-' |
>>> | | V
>>> | |CR3 in GPA
>>> '-'
>>> Guest
>>> --| Shadow |--|
>>> vv v
>>> Host
>>> .-. .--.
>>> | pIOMMU| | Bind FL for GVA-GPA |
>>> | | '--'
>>> ./ |
>>> | PASID Entry | V (Nested xlate)
>>> '\.--.
>>> | | |SL for GPA-HPA, default domain|
>>> | | '--'
>>> '-'
>>> Where:
>>> - FL = First level/stage one page tables
>>> - SL = Second level/stage two page tables
>>>
>>> ---
>>> v11: Fixed locking, avoid duplicated paging mode check, added
>>> helper to free svm if device list is empty. Use rate limited error
>>> message since the bind gpasid call comes from user space.
>>> ---
>>>
>>> Signed-off-by: Jacob Pan
>>> Signed-off-by: Liu, Yi L
>>> ---
>>> drivers/iommu/intel-iommu.c | 4 +
>>> drivers/iommu/intel-svm.c | 206
>>>
>>> include/linux/intel-iommu.h | 8 +- include/linux/intel-svm.h |
>>> 17 4 files changed, 234 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/iommu/intel-iommu.c
>>> b/drivers/iommu/intel-iommu.c index c0dadec5a6b3..94c7993dac6a
>>> 100644 --- a/drivers/iommu/intel-iommu.c
>>> +++ b/drivers/iommu/intel-iommu.c
>>> @@ -6178,6 +6178,10 @@ const struct iommu_ops intel_iommu_ops = {
>>> .dev_disable_feat = intel_iommu_dev_disable_feat,
>>> .is_attach_deferred =
>>> intel_iommu_is_attach_deferred, .pgsize_bitmap =
>>> INTEL_IOMMU_PGSIZES, +#ifdef CONFIG_INTEL_IOMMU_SVM
>>> + .sva_bind_gpasid= intel_svm_bind_gpasid,
>>> + .sva_unbind_gpasid = intel_svm_unbind_gpasid,
>>> +#endif
>>> };
>>>
>>> static void quirk_iommu_igfx(struct pci_dev *dev)
>>> diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
>>> index d7f2a5358900..7cf711318b87 100644
>>> --- a/drivers/iommu/intel-svm.c
>>> +++ b/drivers/iommu/intel-svm.c
>>> @@ -226,6 +226,212 @@ static LIST_HEAD(global_svm_list);
>>> list_for_each_entry((sdev), &(svm)->devs, list) \
>>> if ((d) != (sdev)->dev) {} else
>>>
>>> +
>>> +static inline void intel_svm_free_if_empty(struct intel_svm *svm,
>>> u64 pasid) +{
>>> + if (list_empty(>devs)) {
>>> + ioasid_set_data(pasid, NULL);
>>> + kfree(svm);
>>> + }
>>> +}
>>> +
>>> +int intel_svm_bind_gpasid(struct iommu_domain *domain,
>>> + struct device *dev,
>>> + struct iommu_gpasid_bind_data *data)
>>> +{
>>> + struct intel_iommu *iommu = intel_svm_device_to_iommu(dev);
>>> + struct dmar_domain *dmar_domain;
>>> + struct intel_svm_dev *sdev;
>>> + struct intel_svm *svm;
>>> + int ret = 0;
>>> +
>>> + if (WARN_ON(!iommu) || !data)
>>> + return -EINVAL;
>>> +
>>> + if (data->version != IOMMU_GPASID_BIND_VERSION_1 ||
>>> + data->format != IOMMU_PASID_FORMAT_INTEL_VTD)
>>> + return -EINVAL;
>>> +
>>> + if (dev_is_pci(dev)) {
>>> + /* VT-d supports devices with full 20 bit PASIDs
>>> only */
>>> + if (pci_max_pasids(to_pci_dev(dev)) != PASID_MAX)
>>> + return -EINVAL;
>>> + } else {
>>> + return -ENOTSUPP;
>>> + }
>>> +
>>> + /*
>>> +* We only check host PASID range, we have no knowledge to
>>> check
>>> +* guest PASID range.
>>> +*/
>>> + if (data->hpasid <= 0 || data->hpasid >= PASID_MAX)
>>> + return -EINVAL;
>>> +
>>> + dmar_domain = to_dmar_domain(domain);
>>> +
>>> + mutex_lock(_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.
>>>
Re: [PATCH v11 05/10] iommu/vt-d: Add bind guest PASID support
Hi Eric,
Missed a few things in the last reply.
On Thu, 9 Apr 2020 09:41:32 +0200
Auger Eric wrote:
> > + intel_pasid_tear_down_entry(iommu, dev,
> > svm->pasid);
> intel_svm_unbind_mm() calls intel_flush_svm_range_dev(svm, sdev, 0,
> -1, 0); Don't we need to flush the (DEV-)IOTLBs as well?
Right, pasid tear down should always include (DEV-)IOTLB flush, I
initially thought it is taken care of by intel_pasid_tear_down_entry().
> > + /* TODO: Drain in flight PRQ for the PASID
> > since it
> > +* may get reused soon, we don't want to
> > +* confuse with its previous life.
> > +* intel_svm_drain_prq(dev, pasid);
> > +*/
> > + kfree_rcu(sdev, rcu);
> > +
> > + if (list_empty(>devs)) {
> > + /*
> > +* We do not free the IOASID here
> > in that
> > +* IOMMU driver did not allocate
> > it.
> s/in/as?
I meant to say "in that" as "for the reason that"
> > +* Unlike native SVM, IOASID for
> > guest use was
> > +* allocated prior to the bind
> > call.> + * In any case, if the free
> > call comes before
> > +* the unbind, IOMMU driver will
> > get notified
> > +* and perform cleanup.
> > +*/
> > + ioasid_set_data(pasid, NULL);
> > + kfree(svm);
> > + }
> nit: you may use intel_svm_free_if_empty()
True, but I meant to insert ioasid_notifier under the list_empty
condition in the following ioasid patch.
Thanks,
Jacob
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v11 05/10] iommu/vt-d: Add bind guest PASID support
Hi Eric,
On Thu, 9 Apr 2020 09:41:32 +0200
Auger Eric wrote:
> Hi Jacob,
>
> On 4/3/20 8:42 PM, Jacob Pan wrote:
> > When supporting guest SVA with emulated IOMMU, the guest PASID
> > table is shadowed in VMM. Updates to guest vIOMMU PASID table
> > will result in PASID cache flush which will be passed down to
> > the host as bind guest PASID calls.
> >
> > For the SL page tables, it will be harvested from device's
> > default domain (request w/o PASID), or aux domain in case of
> > mediated device.
> >
> > .-. .---.
> > | vIOMMU| | Guest process CR3, FL only|
> > | | '---'
> > ./
> > | PASID Entry |--- PASID cache flush -
> > '-' |
> > | | V
> > | |CR3 in GPA
> > '-'
> > Guest
> > --| Shadow |--|
> > vv v
> > Host
> > .-. .--.
> > | pIOMMU| | Bind FL for GVA-GPA |
> > | | '--'
> > ./ |
> > | PASID Entry | V (Nested xlate)
> > '\.--.
> > | | |SL for GPA-HPA, default domain|
> > | | '--'
> > '-'
> > Where:
> > - FL = First level/stage one page tables
> > - SL = Second level/stage two page tables
> >
> > ---
> > v11: Fixed locking, avoid duplicated paging mode check, added
> > helper to free svm if device list is empty. Use rate limited error
> > message since the bind gpasid call comes from user space.
> > ---
> >
> > Signed-off-by: Jacob Pan
> > Signed-off-by: Liu, Yi L
> > ---
> > drivers/iommu/intel-iommu.c | 4 +
> > drivers/iommu/intel-svm.c | 206
> >
> > include/linux/intel-iommu.h | 8 +- include/linux/intel-svm.h |
> > 17 4 files changed, 234 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iommu/intel-iommu.c
> > b/drivers/iommu/intel-iommu.c index c0dadec5a6b3..94c7993dac6a
> > 100644 --- a/drivers/iommu/intel-iommu.c
> > +++ b/drivers/iommu/intel-iommu.c
> > @@ -6178,6 +6178,10 @@ const struct iommu_ops intel_iommu_ops = {
> > .dev_disable_feat = intel_iommu_dev_disable_feat,
> > .is_attach_deferred =
> > intel_iommu_is_attach_deferred, .pgsize_bitmap =
> > INTEL_IOMMU_PGSIZES, +#ifdef CONFIG_INTEL_IOMMU_SVM
> > + .sva_bind_gpasid= intel_svm_bind_gpasid,
> > + .sva_unbind_gpasid = intel_svm_unbind_gpasid,
> > +#endif
> > };
> >
> > static void quirk_iommu_igfx(struct pci_dev *dev)
> > diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
> > index d7f2a5358900..7cf711318b87 100644
> > --- a/drivers/iommu/intel-svm.c
> > +++ b/drivers/iommu/intel-svm.c
> > @@ -226,6 +226,212 @@ static LIST_HEAD(global_svm_list);
> > list_for_each_entry((sdev), &(svm)->devs, list) \
> > if ((d) != (sdev)->dev) {} else
> >
> > +
> > +static inline void intel_svm_free_if_empty(struct intel_svm *svm,
> > u64 pasid) +{
> > + if (list_empty(>devs)) {
> > + ioasid_set_data(pasid, NULL);
> > + kfree(svm);
> > + }
> > +}
> > +
> > +int intel_svm_bind_gpasid(struct iommu_domain *domain,
> > + struct device *dev,
> > + struct iommu_gpasid_bind_data *data)
> > +{
> > + struct intel_iommu *iommu = intel_svm_device_to_iommu(dev);
> > + struct dmar_domain *dmar_domain;
> > + struct intel_svm_dev *sdev;
> > + struct intel_svm *svm;
> > + int ret = 0;
> > +
> > + if (WARN_ON(!iommu) || !data)
> > + return -EINVAL;
> > +
> > + if (data->version != IOMMU_GPASID_BIND_VERSION_1 ||
> > + data->format != IOMMU_PASID_FORMAT_INTEL_VTD)
> > + return -EINVAL;
> > +
> > + if (dev_is_pci(dev)) {
> > + /* VT-d supports devices with full 20 bit PASIDs
> > only */
> > + if (pci_max_pasids(to_pci_dev(dev)) != PASID_MAX)
> > + return -EINVAL;
> > + } else {
> > + return -ENOTSUPP;
> > + }
> > +
> > + /*
> > +* We only check host PASID range, we have no knowledge to
> > check
> > +* guest PASID range.
> > +*/
> > + if (data->hpasid <= 0 || data->hpasid >= PASID_MAX)
> > + return -EINVAL;
> > +
> > + dmar_domain = to_dmar_domain(domain);
> > +
> > + mutex_lock(_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.
> > +*/
> > + if
Re: [PATCH v11 05/10] iommu/vt-d: Add bind guest PASID support
Hi Jacob,
On 4/3/20 8:42 PM, Jacob Pan wrote:
> When supporting guest SVA with emulated IOMMU, the guest PASID
> table is shadowed in VMM. Updates to guest vIOMMU PASID table
> will result in PASID cache flush which will be passed down to
> the host as bind guest PASID calls.
>
> For the SL page tables, it will be harvested from device's
> default domain (request w/o PASID), or aux domain in case of
> mediated device.
>
> .-. .---.
> | vIOMMU| | Guest process CR3, FL only|
> | | '---'
> ./
> | PASID Entry |--- PASID cache flush -
> '-' |
> | | V
> | |CR3 in GPA
> '-'
> Guest
> --| Shadow |--|
> vv v
> Host
> .-. .--.
> | pIOMMU| | Bind FL for GVA-GPA |
> | | '--'
> ./ |
> | PASID Entry | V (Nested xlate)
> '\.--.
> | | |SL for GPA-HPA, default domain|
> | | '--'
> '-'
> Where:
> - FL = First level/stage one page tables
> - SL = Second level/stage two page tables
>
> ---
> v11: Fixed locking, avoid duplicated paging mode check, added helper to
> free svm if device list is empty. Use rate limited error message since
> the bind gpasid call comes from user space.
> ---
>
> Signed-off-by: Jacob Pan
> Signed-off-by: Liu, Yi L
> ---
> drivers/iommu/intel-iommu.c | 4 +
> drivers/iommu/intel-svm.c | 206
>
> include/linux/intel-iommu.h | 8 +-
> include/linux/intel-svm.h | 17
> 4 files changed, 234 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index c0dadec5a6b3..94c7993dac6a 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -6178,6 +6178,10 @@ const struct iommu_ops intel_iommu_ops = {
> .dev_disable_feat = intel_iommu_dev_disable_feat,
> .is_attach_deferred = intel_iommu_is_attach_deferred,
> .pgsize_bitmap = INTEL_IOMMU_PGSIZES,
> +#ifdef CONFIG_INTEL_IOMMU_SVM
> + .sva_bind_gpasid= intel_svm_bind_gpasid,
> + .sva_unbind_gpasid = intel_svm_unbind_gpasid,
> +#endif
> };
>
> static void quirk_iommu_igfx(struct pci_dev *dev)
> diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
> index d7f2a5358900..7cf711318b87 100644
> --- a/drivers/iommu/intel-svm.c
> +++ b/drivers/iommu/intel-svm.c
> @@ -226,6 +226,212 @@ static LIST_HEAD(global_svm_list);
> list_for_each_entry((sdev), &(svm)->devs, list) \
> if ((d) != (sdev)->dev) {} else
>
> +
> +static inline void intel_svm_free_if_empty(struct intel_svm *svm, u64 pasid)
> +{
> + if (list_empty(>devs)) {
> + ioasid_set_data(pasid, NULL);
> + kfree(svm);
> + }
> +}
> +
> +int intel_svm_bind_gpasid(struct iommu_domain *domain,
> + struct device *dev,
> + struct iommu_gpasid_bind_data *data)
> +{
> + struct intel_iommu *iommu = intel_svm_device_to_iommu(dev);
> + struct dmar_domain *dmar_domain;
> + struct intel_svm_dev *sdev;
> + struct intel_svm *svm;
> + int ret = 0;
> +
> + if (WARN_ON(!iommu) || !data)
> + return -EINVAL;
> +
> + if (data->version != IOMMU_GPASID_BIND_VERSION_1 ||
> + data->format != IOMMU_PASID_FORMAT_INTEL_VTD)
> + return -EINVAL;
> +
> + if (dev_is_pci(dev)) {
> + /* VT-d supports devices with full 20 bit PASIDs only */
> + if (pci_max_pasids(to_pci_dev(dev)) != PASID_MAX)
> + return -EINVAL;
> + } else {
> + return -ENOTSUPP;
> + }
> +
> + /*
> + * We only check host PASID range, we have no knowledge to check
> + * guest PASID range.
> + */
> + if (data->hpasid <= 0 || data->hpasid >= PASID_MAX)
> + return -EINVAL;
> +
> + dmar_domain = to_dmar_domain(domain);
> +
> + mutex_lock(_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.
> + */
> + if (WARN_ON(list_empty(>devs))) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + for_each_svm_dev(sdev, svm, dev) {
> + /* In case of multiple sub-devices of the same pdev
>
