Re: [PATCH v4 9/9] iommu/vt-d: Remove the obsolete per iommu pasid tables
Hi, On 07/13/2018 01:00 PM, Peter Xu wrote: > On Fri, Jul 13, 2018 at 09:34:30AM +0800, Lu Baolu wrote: >> Hi Peter, >> >> Do you have further comments for this series? I will work out a >> new version if there are no ones. > Not for now. Though since you mentioned about the work to remove the > pasid state table, not sure whether it'll be nice to put that into > this series, or even put more patches that are directly related > (e.g. including the two-level pasid table work and anything related) > so the series could be more complete. > >> Thank you for reviewing my patches. Do you allow me to add your >> reviewed-by to the patches which you've reviewed? > I'm glad to read your work. I didn't leave r-bs because I'm not that > familiar with the codes so I'm not confident to leave any, just to > raise the questions. Please feel free to post a new version as you > wish, I'll try to leave r-b there if possible. > > Thanks, > Okay! Thank you! Best regards, Lu Baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 9/9] iommu/vt-d: Remove the obsolete per iommu pasid tables
On Fri, Jul 13, 2018 at 09:34:30AM +0800, Lu Baolu wrote: > Hi Peter, > > Do you have further comments for this series? I will work out a > new version if there are no ones. Not for now. Though since you mentioned about the work to remove the pasid state table, not sure whether it'll be nice to put that into this series, or even put more patches that are directly related (e.g. including the two-level pasid table work and anything related) so the series could be more complete. > > Thank you for reviewing my patches. Do you allow me to add your > reviewed-by to the patches which you've reviewed? I'm glad to read your work. I didn't leave r-bs because I'm not that familiar with the codes so I'm not confident to leave any, just to raise the questions. Please feel free to post a new version as you wish, I'll try to leave r-b there if possible. Thanks, -- Peter Xu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 9/9] iommu/vt-d: Remove the obsolete per iommu pasid tables
Hi Peter,
On 07/11/2018 10:45 AM, Peter Xu wrote:
> On Mon, Jul 09, 2018 at 01:22:58PM +0800, Lu Baolu wrote:
>> The obsolete per iommu pasid tables are no longer used. Hence,
>> clean up them.
>>
>> Cc: Ashok Raj
>> Cc: Jacob Pan
>> Cc: Kevin Tian
>> Cc: Liu Yi L
>> Signed-off-by: Lu Baolu
>> Reviewed-by: Liu Yi L
>> ---
>> drivers/iommu/intel-iommu.c | 6 +++---
>> drivers/iommu/intel-svm.c | 17 ++---
>> include/linux/intel-iommu.h | 5 ++---
>> 3 files changed, 7 insertions(+), 21 deletions(-)
>>
>> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
>> index f8609b5..9a88081 100644
>> --- a/drivers/iommu/intel-iommu.c
>> +++ b/drivers/iommu/intel-iommu.c
>> @@ -1755,7 +1755,7 @@ static void free_dmar_iommu(struct intel_iommu *iommu)
>> if (pasid_enabled(iommu)) {
>> if (ecap_prs(iommu->ecap))
>> intel_svm_finish_prq(iommu);
>> -intel_svm_free_pasid_tables(iommu);
>> +intel_svm_exit(iommu);
>> }
>> #endif
>> }
>> @@ -3336,7 +3336,7 @@ static int __init init_dmars(void)
>> hw_pass_through = 0;
>> #ifdef CONFIG_INTEL_IOMMU_SVM
>> if (pasid_enabled(iommu))
>> -intel_svm_alloc_pasid_tables(iommu);
>> +intel_svm_init(iommu);
>> #endif
>> }
>>
>> @@ -4330,7 +4330,7 @@ static int intel_iommu_add(struct dmar_drhd_unit
>> *dmaru)
>>
>> #ifdef CONFIG_INTEL_IOMMU_SVM
>> if (pasid_enabled(iommu))
>> -intel_svm_alloc_pasid_tables(iommu);
>> +intel_svm_init(iommu);
>> #endif
>>
>> if (dmaru->ignored) {
>> diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
>> index a253cde..eb30836 100644
>> --- a/drivers/iommu/intel-svm.c
>> +++ b/drivers/iommu/intel-svm.c
>> @@ -38,7 +38,7 @@ struct pasid_state_entry {
>> u64 val;
>> };
>>
>> -int intel_svm_alloc_pasid_tables(struct intel_iommu *iommu)
>> +int intel_svm_init(struct intel_iommu *iommu)
>> {
>> struct page *pages;
>> int order;
>> @@ -63,15 +63,6 @@ int intel_svm_alloc_pasid_tables(struct intel_iommu
>> *iommu)
>> iommu->pasid_max = 0x2;
>>
>> order = get_order(sizeof(struct pasid_entry) * iommu->pasid_max);
>> -pages = alloc_pages(GFP_KERNEL | __GFP_ZERO, order);
>> -if (!pages) {
>> -pr_warn("IOMMU: %s: Failed to allocate PASID table\n",
>> -iommu->name);
>> -return -ENOMEM;
>> -}
>> -iommu->pasid_table = page_address(pages);
>> -pr_info("%s: Allocated order %d PASID table.\n", iommu->name, order);
>> -
>> if (ecap_dis(iommu->ecap)) {
>> /* Just making it explicit... */
>> BUILD_BUG_ON(sizeof(struct pasid_entry) != sizeof(struct
>> pasid_state_entry));
> Then do we still need the so-called pasid state table? I tried to
> find out what it did before but I failed since I see that the bit 27
> of the extended cap is marked as reserved now in the latest vt-d spec
> (then ecap_dis could be meaningless too).
>
> Asked since if we don't need both the per-iommu pasid table and the
> pasid state table, then maybe we don't need intel_svm_init() at all?
>
>> @@ -86,14 +77,10 @@ int intel_svm_alloc_pasid_tables(struct intel_iommu
>> *iommu)
>> return 0;
>> }
>>
>> -int intel_svm_free_pasid_tables(struct intel_iommu *iommu)
>> +int intel_svm_exit(struct intel_iommu *iommu)
> Same thing applies to this exit() function - after we removed the
> per-iommu idr, per-iommu pasid table, and (possibly obsolete)
> per-iommu pasid state table, do we need that at all?
>
> Thanks,
>
Good eyes!
Yes, we will cleanup this with a following patch series which enables
features defined in the vt-d v3.0.
Best regards,
Lu Baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v4 9/9] iommu/vt-d: Remove the obsolete per iommu pasid tables
On Mon, Jul 09, 2018 at 01:22:58PM +0800, Lu Baolu wrote:
> The obsolete per iommu pasid tables are no longer used. Hence,
> clean up them.
>
> Cc: Ashok Raj
> Cc: Jacob Pan
> Cc: Kevin Tian
> Cc: Liu Yi L
> Signed-off-by: Lu Baolu
> Reviewed-by: Liu Yi L
> ---
> drivers/iommu/intel-iommu.c | 6 +++---
> drivers/iommu/intel-svm.c | 17 ++---
> include/linux/intel-iommu.h | 5 ++---
> 3 files changed, 7 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index f8609b5..9a88081 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -1755,7 +1755,7 @@ static void free_dmar_iommu(struct intel_iommu *iommu)
> if (pasid_enabled(iommu)) {
> if (ecap_prs(iommu->ecap))
> intel_svm_finish_prq(iommu);
> - intel_svm_free_pasid_tables(iommu);
> + intel_svm_exit(iommu);
> }
> #endif
> }
> @@ -3336,7 +3336,7 @@ static int __init init_dmars(void)
> hw_pass_through = 0;
> #ifdef CONFIG_INTEL_IOMMU_SVM
> if (pasid_enabled(iommu))
> - intel_svm_alloc_pasid_tables(iommu);
> + intel_svm_init(iommu);
> #endif
> }
>
> @@ -4330,7 +4330,7 @@ static int intel_iommu_add(struct dmar_drhd_unit *dmaru)
>
> #ifdef CONFIG_INTEL_IOMMU_SVM
> if (pasid_enabled(iommu))
> - intel_svm_alloc_pasid_tables(iommu);
> + intel_svm_init(iommu);
> #endif
>
> if (dmaru->ignored) {
> diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
> index a253cde..eb30836 100644
> --- a/drivers/iommu/intel-svm.c
> +++ b/drivers/iommu/intel-svm.c
> @@ -38,7 +38,7 @@ struct pasid_state_entry {
> u64 val;
> };
>
> -int intel_svm_alloc_pasid_tables(struct intel_iommu *iommu)
> +int intel_svm_init(struct intel_iommu *iommu)
> {
> struct page *pages;
> int order;
> @@ -63,15 +63,6 @@ int intel_svm_alloc_pasid_tables(struct intel_iommu *iommu)
> iommu->pasid_max = 0x2;
>
> order = get_order(sizeof(struct pasid_entry) * iommu->pasid_max);
> - pages = alloc_pages(GFP_KERNEL | __GFP_ZERO, order);
> - if (!pages) {
> - pr_warn("IOMMU: %s: Failed to allocate PASID table\n",
> - iommu->name);
> - return -ENOMEM;
> - }
> - iommu->pasid_table = page_address(pages);
> - pr_info("%s: Allocated order %d PASID table.\n", iommu->name, order);
> -
> if (ecap_dis(iommu->ecap)) {
> /* Just making it explicit... */
> BUILD_BUG_ON(sizeof(struct pasid_entry) != sizeof(struct
> pasid_state_entry));
Then do we still need the so-called pasid state table? I tried to
find out what it did before but I failed since I see that the bit 27
of the extended cap is marked as reserved now in the latest vt-d spec
(then ecap_dis could be meaningless too).
Asked since if we don't need both the per-iommu pasid table and the
pasid state table, then maybe we don't need intel_svm_init() at all?
> @@ -86,14 +77,10 @@ int intel_svm_alloc_pasid_tables(struct intel_iommu
> *iommu)
> return 0;
> }
>
> -int intel_svm_free_pasid_tables(struct intel_iommu *iommu)
> +int intel_svm_exit(struct intel_iommu *iommu)
Same thing applies to this exit() function - after we removed the
per-iommu idr, per-iommu pasid table, and (possibly obsolete)
per-iommu pasid state table, do we need that at all?
Thanks,
> {
> int order = get_order(sizeof(struct pasid_entry) * iommu->pasid_max);
>
> - if (iommu->pasid_table) {
> - free_pages((unsigned long)iommu->pasid_table, order);
> - iommu->pasid_table = NULL;
> - }
> if (iommu->pasid_state_table) {
> free_pages((unsigned long)iommu->pasid_state_table, order);
> iommu->pasid_state_table = NULL;
> diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
> index e7901d4..3c43882 100644
> --- a/include/linux/intel-iommu.h
> +++ b/include/linux/intel-iommu.h
> @@ -453,7 +453,6 @@ struct intel_iommu {
>* devices away to userspace processes (e.g. for DPDK) and don't
>* want to trust that userspace will use *only* the PASID it was
>* told to. But while it's all driver-arbitrated, we're fine. */
> - struct pasid_entry *pasid_table;
> struct pasid_state_entry *pasid_state_table;
> struct page_req_dsc *prq;
> unsigned char prq_name[16];/* Name for PRQ interrupt */
> @@ -526,8 +525,8 @@ int for_each_device_domain(int (*fn)(struct
> device_domain_info *info,
>void *data), void *data);
>
> #ifdef CONFIG_INTEL_IOMMU_SVM
> -extern int intel_svm_alloc_pasid_tables(struct intel_iommu *iommu);
> -extern int intel_svm_free_pasid_tables(struct intel_iommu *iommu);
> +int intel_svm_init(struct intel_iommu *iommu);
> +int intel_svm_exit(struct
[PATCH v4 9/9] iommu/vt-d: Remove the obsolete per iommu pasid tables
The obsolete per iommu pasid tables are no longer used. Hence,
clean up them.
Cc: Ashok Raj
Cc: Jacob Pan
Cc: Kevin Tian
Cc: Liu Yi L
Signed-off-by: Lu Baolu
Reviewed-by: Liu Yi L
---
drivers/iommu/intel-iommu.c | 6 +++---
drivers/iommu/intel-svm.c | 17 ++---
include/linux/intel-iommu.h | 5 ++---
3 files changed, 7 insertions(+), 21 deletions(-)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index f8609b5..9a88081 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -1755,7 +1755,7 @@ static void free_dmar_iommu(struct intel_iommu *iommu)
if (pasid_enabled(iommu)) {
if (ecap_prs(iommu->ecap))
intel_svm_finish_prq(iommu);
- intel_svm_free_pasid_tables(iommu);
+ intel_svm_exit(iommu);
}
#endif
}
@@ -3336,7 +3336,7 @@ static int __init init_dmars(void)
hw_pass_through = 0;
#ifdef CONFIG_INTEL_IOMMU_SVM
if (pasid_enabled(iommu))
- intel_svm_alloc_pasid_tables(iommu);
+ intel_svm_init(iommu);
#endif
}
@@ -4330,7 +4330,7 @@ static int intel_iommu_add(struct dmar_drhd_unit *dmaru)
#ifdef CONFIG_INTEL_IOMMU_SVM
if (pasid_enabled(iommu))
- intel_svm_alloc_pasid_tables(iommu);
+ intel_svm_init(iommu);
#endif
if (dmaru->ignored) {
diff --git a/drivers/iommu/intel-svm.c b/drivers/iommu/intel-svm.c
index a253cde..eb30836 100644
--- a/drivers/iommu/intel-svm.c
+++ b/drivers/iommu/intel-svm.c
@@ -38,7 +38,7 @@ struct pasid_state_entry {
u64 val;
};
-int intel_svm_alloc_pasid_tables(struct intel_iommu *iommu)
+int intel_svm_init(struct intel_iommu *iommu)
{
struct page *pages;
int order;
@@ -63,15 +63,6 @@ int intel_svm_alloc_pasid_tables(struct intel_iommu *iommu)
iommu->pasid_max = 0x2;
order = get_order(sizeof(struct pasid_entry) * iommu->pasid_max);
- pages = alloc_pages(GFP_KERNEL | __GFP_ZERO, order);
- if (!pages) {
- pr_warn("IOMMU: %s: Failed to allocate PASID table\n",
- iommu->name);
- return -ENOMEM;
- }
- iommu->pasid_table = page_address(pages);
- pr_info("%s: Allocated order %d PASID table.\n", iommu->name, order);
-
if (ecap_dis(iommu->ecap)) {
/* Just making it explicit... */
BUILD_BUG_ON(sizeof(struct pasid_entry) != sizeof(struct
pasid_state_entry));
@@ -86,14 +77,10 @@ int intel_svm_alloc_pasid_tables(struct intel_iommu *iommu)
return 0;
}
-int intel_svm_free_pasid_tables(struct intel_iommu *iommu)
+int intel_svm_exit(struct intel_iommu *iommu)
{
int order = get_order(sizeof(struct pasid_entry) * iommu->pasid_max);
- if (iommu->pasid_table) {
- free_pages((unsigned long)iommu->pasid_table, order);
- iommu->pasid_table = NULL;
- }
if (iommu->pasid_state_table) {
free_pages((unsigned long)iommu->pasid_state_table, order);
iommu->pasid_state_table = NULL;
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index e7901d4..3c43882 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -453,7 +453,6 @@ struct intel_iommu {
* devices away to userspace processes (e.g. for DPDK) and don't
* want to trust that userspace will use *only* the PASID it was
* told to. But while it's all driver-arbitrated, we're fine. */
- struct pasid_entry *pasid_table;
struct pasid_state_entry *pasid_state_table;
struct page_req_dsc *prq;
unsigned char prq_name[16];/* Name for PRQ interrupt */
@@ -526,8 +525,8 @@ int for_each_device_domain(int (*fn)(struct
device_domain_info *info,
void *data), void *data);
#ifdef CONFIG_INTEL_IOMMU_SVM
-extern int intel_svm_alloc_pasid_tables(struct intel_iommu *iommu);
-extern int intel_svm_free_pasid_tables(struct intel_iommu *iommu);
+int intel_svm_init(struct intel_iommu *iommu);
+int intel_svm_exit(struct intel_iommu *iommu);
extern int intel_svm_enable_prq(struct intel_iommu *iommu);
extern int intel_svm_finish_prq(struct intel_iommu *iommu);
--
2.7.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
