From: Yi Sun <[email protected]>
Current interface is good enough for SVA virtualization on an assigned
physical PCI device, but when it comes to mediated devices, a physical
device may attached with multiple aux-domains. Also, for guest unbind,
the PASID to be unbind should be allocated to the VM. This check requires
to know the ioasid_set which is associated with the domain.
So this interface needs to pass in domain info. Then the iommu driver is
able to know which domain will be used for the 2nd stage translation of
the nesting mode and also be able to do PASID ownership check. This patch
passes @domain per the above reason. Also, the prototype of &pasid is
changed frnt" to "u32" as the below link.
https://lore.kernel.org/kvm/[email protected]/
Cc: Kevin Tian <[email protected]>
CC: Jacob Pan <[email protected]>
Cc: Alex Williamson <[email protected]>
Cc: Eric Auger <[email protected]>
Cc: Jean-Philippe Brucker <[email protected]>
Cc: Joerg Roedel <[email protected]>
Cc: Lu Baolu <[email protected]>
Reviewed-by: Eric Auger <[email protected]>
Signed-off-by: Yi Sun <[email protected]>
Signed-off-by: Liu Yi L <[email protected]>
---
v5 -> v6:
*) use "u32" prototype for @pasid.
*) add review-by from Eric Auger.
v2 -> v3:
*) pass in domain info only
*) use u32 for pasid instead of int type
v1 -> v2:
*) added in v2.
---
drivers/iommu/intel/svm.c | 3 ++-
drivers/iommu/iommu.c | 2 +-
include/linux/intel-iommu.h | 3 ++-
include/linux/iommu.h | 3 ++-
4 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
index c27d16a..c85b8d5 100644
--- a/drivers/iommu/intel/svm.c
+++ b/drivers/iommu/intel/svm.c
@@ -436,7 +436,8 @@ int intel_svm_bind_gpasid(struct iommu_domain *domain, struct device *dev,
return ret;
}
-int intel_svm_unbind_gpasid(struct device *dev, int pasid)
+int intel_svm_unbind_gpasid(struct iommu_domain *domain,
+ struct device *dev, u32 pasid)
{
struct intel_iommu *iommu = intel_svm_device_to_iommu(dev);
struct intel_svm_dev *sdev;
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 1ce2a61..bee79d7 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2145,7 +2145,7 @@ int iommu_sva_unbind_gpasid(struct iommu_domain *domain, struct device *dev,
if (unlikely(!domain->ops->sva_unbind_gpasid))
return -ENODEV;
- return domain->ops->sva_unbind_gpasid(dev, data->hpasid);
+ return domain->ops->sva_unbind_gpasid(domain, dev, data->hpasid);
}
EXPORT_SYMBOL_GPL(iommu_sva_unbind_gpasid);
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 0d0ab32..f98146b 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -738,7 +738,8 @@ extern int intel_svm_enable_prq(struct intel_iommu *iommu);
extern int intel_svm_finish_prq(struct intel_iommu *iommu);
int intel_svm_bind_gpasid(struct iommu_domain *domain, struct device *dev,
struct iommu_gpasid_bind_data *data);
-int intel_svm_unbind_gpasid(struct device *dev, int pasid);
+int intel_svm_unbind_gpasid(struct iommu_domain *domain,
+ struct device *dev, u32 pasid);
struct iommu_sva *intel_svm_bind(struct device *dev, struct mm_struct *mm,
void *drvdata);
void intel_svm_unbind(struct iommu_sva *handle);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index b1ff702..80467fc 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -303,7 +303,8 @@ struct iommu_ops {
int (*sva_bind_gpasid)(struct iommu_domain *domain,
struct device *dev, struct iommu_gpasid_bind_data *data);
- int (*sva_unbind_gpasid)(struct device *dev, int pasid);
+ int (*sva_unbind_gpasid)(struct iommu_domain *domain,
+ struct device *dev, u32 pasid);
int (*def_domain_type)(struct device *dev);
--
2.7.4
On Mon, 27 Jul 2020 23:27:37 -0700
Liu Yi L <[email protected]> wrote:
> From: Yi Sun <[email protected]>
>
> Current interface is good enough for SVA virtualization on an assigned
> physical PCI device, but when it comes to mediated devices, a physical
> device may attached with multiple aux-domains. Also, for guest unbind,
s/may/may be/
> the PASID to be unbind should be allocated to the VM. This check requires
> to know the ioasid_set which is associated with the domain.
>
> So this interface needs to pass in domain info. Then the iommu driver is
> able to know which domain will be used for the 2nd stage translation of
> the nesting mode and also be able to do PASID ownership check. This patch
> passes @domain per the above reason. Also, the prototype of &pasid is
> changed frnt" to "u32" as the below link.
s/frnt"/from an "int"/
> https://lore.kernel.org/kvm/[email protected]/
This is really confusing, the link is to Eric's comment asking that the
conversion from (at the time) int to ioasid_t be included in the commit
log. The text here implies that it's pointing to some sort of
justification for the change, which it isn't. It just notes that it
happened, not why it happened, with a mostly irrelevant link.
> Cc: Kevin Tian <[email protected]>
> CC: Jacob Pan <[email protected]>
> Cc: Alex Williamson <[email protected]>
> Cc: Eric Auger <[email protected]>
> Cc: Jean-Philippe Brucker <[email protected]>
> Cc: Joerg Roedel <[email protected]>
> Cc: Lu Baolu <[email protected]>
> Reviewed-by: Eric Auger <[email protected]>
> Signed-off-by: Yi Sun <[email protected]>
> Signed-off-by: Liu Yi L <[email protected]>
> ---
> v5 -> v6:
> *) use "u32" prototype for @pasid.
> *) add review-by from Eric Auger.
I'd probably hold off on adding Eric's R-b given the additional change
in this version FWIW. Thanks,
Alex
> v2 -> v3:
> *) pass in domain info only
> *) use u32 for pasid instead of int type
>
> v1 -> v2:
> *) added in v2.
> ---
> drivers/iommu/intel/svm.c | 3 ++-
> drivers/iommu/iommu.c | 2 +-
> include/linux/intel-iommu.h | 3 ++-
> include/linux/iommu.h | 3 ++-
> 4 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
> index c27d16a..c85b8d5 100644
> --- a/drivers/iommu/intel/svm.c
> +++ b/drivers/iommu/intel/svm.c
> @@ -436,7 +436,8 @@ int intel_svm_bind_gpasid(struct iommu_domain *domain, struct device *dev,
> return ret;
> }
>
> -int intel_svm_unbind_gpasid(struct device *dev, int pasid)
> +int intel_svm_unbind_gpasid(struct iommu_domain *domain,
> + struct device *dev, u32 pasid)
> {
> struct intel_iommu *iommu = intel_svm_device_to_iommu(dev);
> struct intel_svm_dev *sdev;
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 1ce2a61..bee79d7 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -2145,7 +2145,7 @@ int iommu_sva_unbind_gpasid(struct iommu_domain *domain, struct device *dev,
> if (unlikely(!domain->ops->sva_unbind_gpasid))
> return -ENODEV;
>
> - return domain->ops->sva_unbind_gpasid(dev, data->hpasid);
> + return domain->ops->sva_unbind_gpasid(domain, dev, data->hpasid);
> }
> EXPORT_SYMBOL_GPL(iommu_sva_unbind_gpasid);
>
> diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
> index 0d0ab32..f98146b 100644
> --- a/include/linux/intel-iommu.h
> +++ b/include/linux/intel-iommu.h
> @@ -738,7 +738,8 @@ extern int intel_svm_enable_prq(struct intel_iommu *iommu);
> extern int intel_svm_finish_prq(struct intel_iommu *iommu);
> int intel_svm_bind_gpasid(struct iommu_domain *domain, struct device *dev,
> struct iommu_gpasid_bind_data *data);
> -int intel_svm_unbind_gpasid(struct device *dev, int pasid);
> +int intel_svm_unbind_gpasid(struct iommu_domain *domain,
> + struct device *dev, u32 pasid);
> struct iommu_sva *intel_svm_bind(struct device *dev, struct mm_struct *mm,
> void *drvdata);
> void intel_svm_unbind(struct iommu_sva *handle);
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index b1ff702..80467fc 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -303,7 +303,8 @@ struct iommu_ops {
> int (*sva_bind_gpasid)(struct iommu_domain *domain,
> struct device *dev, struct iommu_gpasid_bind_data *data);
>
> - int (*sva_unbind_gpasid)(struct device *dev, int pasid);
> + int (*sva_unbind_gpasid)(struct iommu_domain *domain,
> + struct device *dev, u32 pasid);
>
> int (*def_domain_type)(struct device *dev);
>
Hi Alex,
> From: Alex Williamson <[email protected]>
> Sent: Friday, August 21, 2020 5:06 AM
>
> On Mon, 27 Jul 2020 23:27:37 -0700
> Liu Yi L <[email protected]> wrote:
>
> > From: Yi Sun <[email protected]>
> >
> > Current interface is good enough for SVA virtualization on an assigned
> > physical PCI device, but when it comes to mediated devices, a physical
> > device may attached with multiple aux-domains. Also, for guest unbind,
>
> s/may/may be/
got it.
>
> > the PASID to be unbind should be allocated to the VM. This check
> > requires to know the ioasid_set which is associated with the domain.
> >
> > So this interface needs to pass in domain info. Then the iommu driver
> > is able to know which domain will be used for the 2nd stage
> > translation of the nesting mode and also be able to do PASID ownership
> > check. This patch passes @domain per the above reason. Also, the
> > prototype of &pasid is changed frnt" to "u32" as the below link.
>
> s/frnt"/from an "int"/
got it.
> > https://lore.kernel.org/kvm/27ac7880-bdd3-2891-139e-b4a7cd18420b@redha
> > t.com/
>
> This is really confusing, the link is to Eric's comment asking that the conversion from
> (at the time) int to ioasid_t be included in the commit log. The text here implies that
> it's pointing to some sort of justification for the change, which it isn't. It just notes
> that it happened, not why it happened, with a mostly irrelevant link.
really sorry, a mistake from me. it should be the below link.
[PATCH v6 01/12] iommu: Change type of pasid to u32
https://lore.kernel.org/linux-iommu/[email protected]/
> > Cc: Kevin Tian <[email protected]>
> > CC: Jacob Pan <[email protected]>
> > Cc: Alex Williamson <[email protected]>
> > Cc: Eric Auger <[email protected]>
> > Cc: Jean-Philippe Brucker <[email protected]>
> > Cc: Joerg Roedel <[email protected]>
> > Cc: Lu Baolu <[email protected]>
> > Reviewed-by: Eric Auger <[email protected]>
> > Signed-off-by: Yi Sun <[email protected]>
> > Signed-off-by: Liu Yi L <[email protected]>
> > ---
> > v5 -> v6:
> > *) use "u32" prototype for @pasid.
> > *) add review-by from Eric Auger.
>
> I'd probably hold off on adding Eric's R-b given the additional change in this version
> FWIW. Thanks,
ok, will hold on it. :-)
Regards,
Yi Liu
> Alex
>
> > v2 -> v3:
> > *) pass in domain info only
> > *) use u32 for pasid instead of int type
> >
> > v1 -> v2:
> > *) added in v2.
> > ---
> > drivers/iommu/intel/svm.c | 3 ++-
> > drivers/iommu/iommu.c | 2 +-
> > include/linux/intel-iommu.h | 3 ++-
> > include/linux/iommu.h | 3 ++-
> > 4 files changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
> > index c27d16a..c85b8d5 100644
> > --- a/drivers/iommu/intel/svm.c
> > +++ b/drivers/iommu/intel/svm.c
> > @@ -436,7 +436,8 @@ int intel_svm_bind_gpasid(struct iommu_domain *domain,
> struct device *dev,
> > return ret;
> > }
> >
> > -int intel_svm_unbind_gpasid(struct device *dev, int pasid)
> > +int intel_svm_unbind_gpasid(struct iommu_domain *domain,
> > + struct device *dev, u32 pasid)
> > {
> > struct intel_iommu *iommu = intel_svm_device_to_iommu(dev);
> > struct intel_svm_dev *sdev;
> > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index
> > 1ce2a61..bee79d7 100644
> > --- a/drivers/iommu/iommu.c
> > +++ b/drivers/iommu/iommu.c
> > @@ -2145,7 +2145,7 @@ int iommu_sva_unbind_gpasid(struct iommu_domain
> *domain, struct device *dev,
> > if (unlikely(!domain->ops->sva_unbind_gpasid))
> > return -ENODEV;
> >
> > - return domain->ops->sva_unbind_gpasid(dev, data->hpasid);
> > + return domain->ops->sva_unbind_gpasid(domain, dev, data->hpasid);
> > }
> > EXPORT_SYMBOL_GPL(iommu_sva_unbind_gpasid);
> >
> > diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
> > index 0d0ab32..f98146b 100644
> > --- a/include/linux/intel-iommu.h
> > +++ b/include/linux/intel-iommu.h
> > @@ -738,7 +738,8 @@ extern int intel_svm_enable_prq(struct intel_iommu
> > *iommu); extern int intel_svm_finish_prq(struct intel_iommu *iommu);
> > int intel_svm_bind_gpasid(struct iommu_domain *domain, struct device *dev,
> > struct iommu_gpasid_bind_data *data); -int
> > intel_svm_unbind_gpasid(struct device *dev, int pasid);
> > +int intel_svm_unbind_gpasid(struct iommu_domain *domain,
> > + struct device *dev, u32 pasid);
> > struct iommu_sva *intel_svm_bind(struct device *dev, struct mm_struct *mm,
> > void *drvdata);
> > void intel_svm_unbind(struct iommu_sva *handle); diff --git
> > a/include/linux/iommu.h b/include/linux/iommu.h index b1ff702..80467fc
> > 100644
> > --- a/include/linux/iommu.h
> > +++ b/include/linux/iommu.h
> > @@ -303,7 +303,8 @@ struct iommu_ops {
> > int (*sva_bind_gpasid)(struct iommu_domain *domain,
> > struct device *dev, struct iommu_gpasid_bind_data *data);
> >
> > - int (*sva_unbind_gpasid)(struct device *dev, int pasid);
> > + int (*sva_unbind_gpasid)(struct iommu_domain *domain,
> > + struct device *dev, u32 pasid);
> >
> > int (*def_domain_type)(struct device *dev);
> >
Hi,
On 8/20/20 11:06 PM, Alex Williamson wrote:
> On Mon, 27 Jul 2020 23:27:37 -0700
> Liu Yi L <[email protected]> wrote:
>
>> From: Yi Sun <[email protected]>
>>
>> Current interface is good enough for SVA virtualization on an assigned
>> physical PCI device, but when it comes to mediated devices, a physical
>> device may attached with multiple aux-domains. Also, for guest unbind,
>
> s/may/may be/
>
>> the PASID to be unbind should be allocated to the VM. This check requires
>> to know the ioasid_set which is associated with the domain.
>>
>> So this interface needs to pass in domain info. Then the iommu driver is
>> able to know which domain will be used for the 2nd stage translation of
>> the nesting mode and also be able to do PASID ownership check. This patch
>> passes @domain per the above reason. Also, the prototype of &pasid is
>> changed frnt" to "u32" as the below link.
>
> s/frnt"/from an "int"/
>
>> https://lore.kernel.org/kvm/[email protected]/
>
> This is really confusing, the link is to Eric's comment asking that the
> conversion from (at the time) int to ioasid_t be included in the commit
> log. The text here implies that it's pointing to some sort of
> justification for the change, which it isn't. It just notes that it
> happened, not why it happened, with a mostly irrelevant link.
>
>> Cc: Kevin Tian <[email protected]>
>> CC: Jacob Pan <[email protected]>
>> Cc: Alex Williamson <[email protected]>
>> Cc: Eric Auger <[email protected]>
>> Cc: Jean-Philippe Brucker <[email protected]>
>> Cc: Joerg Roedel <[email protected]>
>> Cc: Lu Baolu <[email protected]>
>> Reviewed-by: Eric Auger <[email protected]>
>> Signed-off-by: Yi Sun <[email protected]>
>> Signed-off-by: Liu Yi L <[email protected]>
>> ---
>> v5 -> v6:
>> *) use "u32" prototype for @pasid.
>> *) add review-by from Eric Auger.
>
> I'd probably hold off on adding Eric's R-b given the additional change
> in this version FWIW. Thanks,
Yep I did not notice that change given the R-b was applied ;-)
Thanks
Eric
>
> Alex
>
>> v2 -> v3:
>> *) pass in domain info only
>> *) use u32 for pasid instead of int type
>>
>> v1 -> v2:
>> *) added in v2.
>> ---
>> drivers/iommu/intel/svm.c | 3 ++-
>> drivers/iommu/iommu.c | 2 +-
>> include/linux/intel-iommu.h | 3 ++-
>> include/linux/iommu.h | 3 ++-
>> 4 files changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iommu/intel/svm.c b/drivers/iommu/intel/svm.c
>> index c27d16a..c85b8d5 100644
>> --- a/drivers/iommu/intel/svm.c
>> +++ b/drivers/iommu/intel/svm.c
>> @@ -436,7 +436,8 @@ int intel_svm_bind_gpasid(struct iommu_domain *domain, struct device *dev,
>> return ret;
>> }
>>
>> -int intel_svm_unbind_gpasid(struct device *dev, int pasid)
>> +int intel_svm_unbind_gpasid(struct iommu_domain *domain,
>> + struct device *dev, u32 pasid)
>> {
>> struct intel_iommu *iommu = intel_svm_device_to_iommu(dev);
>> struct intel_svm_dev *sdev;
>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>> index 1ce2a61..bee79d7 100644
>> --- a/drivers/iommu/iommu.c
>> +++ b/drivers/iommu/iommu.c
>> @@ -2145,7 +2145,7 @@ int iommu_sva_unbind_gpasid(struct iommu_domain *domain, struct device *dev,
>> if (unlikely(!domain->ops->sva_unbind_gpasid))
>> return -ENODEV;
>>
>> - return domain->ops->sva_unbind_gpasid(dev, data->hpasid);
>> + return domain->ops->sva_unbind_gpasid(domain, dev, data->hpasid);
>> }
>> EXPORT_SYMBOL_GPL(iommu_sva_unbind_gpasid);
>>
>> diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
>> index 0d0ab32..f98146b 100644
>> --- a/include/linux/intel-iommu.h
>> +++ b/include/linux/intel-iommu.h
>> @@ -738,7 +738,8 @@ extern int intel_svm_enable_prq(struct intel_iommu *iommu);
>> extern int intel_svm_finish_prq(struct intel_iommu *iommu);
>> int intel_svm_bind_gpasid(struct iommu_domain *domain, struct device *dev,
>> struct iommu_gpasid_bind_data *data);
>> -int intel_svm_unbind_gpasid(struct device *dev, int pasid);
>> +int intel_svm_unbind_gpasid(struct iommu_domain *domain,
>> + struct device *dev, u32 pasid);
>> struct iommu_sva *intel_svm_bind(struct device *dev, struct mm_struct *mm,
>> void *drvdata);
>> void intel_svm_unbind(struct iommu_sva *handle);
>> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
>> index b1ff702..80467fc 100644
>> --- a/include/linux/iommu.h
>> +++ b/include/linux/iommu.h
>> @@ -303,7 +303,8 @@ struct iommu_ops {
>> int (*sva_bind_gpasid)(struct iommu_domain *domain,
>> struct device *dev, struct iommu_gpasid_bind_data *data);
>>
>> - int (*sva_unbind_gpasid)(struct device *dev, int pasid);
>> + int (*sva_unbind_gpasid)(struct iommu_domain *domain,
>> + struct device *dev, u32 pasid);
>>
>> int (*def_domain_type)(struct device *dev);
>>
>