[PATCH 0/6] trivial cleanup for iommu/vt-d

[PATCH 0/6] trivial cleanup for iommu/vt-d

[Yijing Wang]

Some cleanup patches for iommu/vt-d.

Yijing Wang (6):
  iommu/vt-d: Use list_for_each_safe() to simplify code
  iommu/vt-d: move up no_iommu and dmar_disabled check
  iommu/vt-d: clear the redundant assignment in dmar_enable_qi
  iommu/vt-d: clear the redundant assignment for domain->nid
  iommu/vt-d: use inline function dma_pte_superpage instead of macros
  iommu/vt-d: fix reference count in iommu_prepare_isa

 drivers/iommu/dmar.c        |    3 ---
 drivers/iommu/intel-iommu.c |   18 ++++++++----------
 2 files changed, 8 insertions(+), 13 deletions(-)

[PATCH 1/6] iommu/vt-d: Use list_for_each_safe() to simplify code

[Yijing Wang]

Use list_for_each_entry_safe() instead of list_entry()
to simplify code.

Signed-off-by: Yijing Wang <wangyijing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/intel-iommu.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index f256ffc..e020dcf 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2101,13 +2101,11 @@ static inline void unlink_domain_info(struct device_domain_info *info)
 
 static void domain_remove_dev_info(struct dmar_domain *domain)
 {
-	struct device_domain_info *info;
+	struct device_domain_info *info, *tmp;
 	unsigned long flags, flags2;
 
 	spin_lock_irqsave(&device_domain_lock, flags);
-	while (!list_empty(&domain->devices)) {
-		info = list_entry(domain->devices.next,
-			struct device_domain_info, link);
+	list_for_each_entry_safe(info, tmp, &domain->devices, link) {
 		unlink_domain_info(info);
 		spin_unlock_irqrestore(&device_domain_lock, flags);
 
-- 
1.7.1

[PATCH 2/6] iommu/vt-d: move up no_iommu and dmar_disabled check

[Yijing Wang]

Move up the no_iommu and dmar_disabled check, avoid the
useless initialization for dmar.

Signed-off-by: Yijing Wang <wangyijing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/intel-iommu.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index e020dcf..6b71608 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3948,6 +3948,9 @@ int __init intel_iommu_init(void)
 	/* VT-d is required for a TXT/tboot launch, so enforce that */
 	force_on = tboot_force_iommu();
 
+	if (no_iommu || dmar_disabled)
+		return ret;
+
 	if (iommu_init_mempool()) {
 		if (force_on)
 			panic("tboot: Failed to initialize iommu memory\n");
@@ -3974,9 +3977,6 @@ int __init intel_iommu_init(void)
 		goto out_free_dmar;
 	}
 
-	if (no_iommu || dmar_disabled)
-		goto out_free_dmar;
-
 	if (list_empty(&dmar_rmrr_units))
 		printk(KERN_INFO "DMAR: No RMRR found\n");
 
-- 
1.7.1

Re: [PATCH 2/6] iommu/vt-d: move up no_iommu and dmar_disabled check

[Joerg Roedel]

On Tue, May 20, 2014 at 08:37:48PM +0800, Yijing Wang wrote:
> Move up the no_iommu and dmar_disabled check, avoid the
> useless initialization for dmar.
> 
> Signed-off-by: Yijing Wang <wangyijing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/iommu/intel-iommu.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index e020dcf..6b71608 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -3948,6 +3948,9 @@ int __init intel_iommu_init(void)
>  	/* VT-d is required for a TXT/tboot launch, so enforce that */
>  	force_on = tboot_force_iommu();
>  
> +	if (no_iommu || dmar_disabled)
> +		return ret;
> +
>  	if (iommu_init_mempool()) {
>  		if (force_on)
>  			panic("tboot: Failed to initialize iommu memory\n");
> @@ -3974,9 +3977,6 @@ int __init intel_iommu_init(void)
>  		goto out_free_dmar;
>  	}
>  
> -	if (no_iommu || dmar_disabled)
> -		goto out_free_dmar;
> -
>  	if (list_empty(&dmar_rmrr_units))
>  		printk(KERN_INFO "DMAR: No RMRR found\n");

This breaks the kexec case were the old kernel had VT-d enabled and the
new one disabled. In this case the new kernel might need to disable the
IOMMUs.


	Joerg

Re: [PATCH 2/6] iommu/vt-d: move up no_iommu and dmar_disabled check

[Yijing Wang]

>> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
>> index e020dcf..6b71608 100644
>> --- a/drivers/iommu/intel-iommu.c
>> +++ b/drivers/iommu/intel-iommu.c
>> @@ -3948,6 +3948,9 @@ int __init intel_iommu_init(void)
>>  	/* VT-d is required for a TXT/tboot launch, so enforce that */
>>  	force_on = tboot_force_iommu();
>>  
>> +	if (no_iommu || dmar_disabled)
>> +		return ret;
>> +
>>  	if (iommu_init_mempool()) {
>>  		if (force_on)
>>  			panic("tboot: Failed to initialize iommu memory\n");
>> @@ -3974,9 +3977,6 @@ int __init intel_iommu_init(void)
>>  		goto out_free_dmar;
>>  	}
>>  
>> -	if (no_iommu || dmar_disabled)
>> -		goto out_free_dmar;
>> -
>>  	if (list_empty(&dmar_rmrr_units))
>>  		printk(KERN_INFO "DMAR: No RMRR found\n");
> 
> This breaks the kexec case were the old kernel had VT-d enabled and the
> new one disabled. In this case the new kernel might need to disable the
> IOMMUs.

Hi Joerg,
   Thanks for your review and comments! You are right, I didn't consider the kexec case,
I will drop this patch.

Thanks!
Yijing.


> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 
> 


-- 
Thanks!
Yijing

[PATCH 3/6] iommu/vt-d: clear the redundant assignment in dmar_enable_qi

[Yijing Wang]

__dmar_enable_qi() will initialize free_head,free_tail and
free_cnt for q_inval. Remove the redundant initialization
in dmar_enable_qi().

Signed-off-by: Yijing Wang <wangyijing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/dmar.c |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index 39f8b71..1168469 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -1339,9 +1339,6 @@ int dmar_enable_qi(struct intel_iommu *iommu)
 		return -ENOMEM;
 	}
 
-	qi->free_head = qi->free_tail = 0;
-	qi->free_cnt = QI_LENGTH;
-
 	raw_spin_lock_init(&qi->q_lock);
 
 	__dmar_enable_qi(iommu);
-- 
1.7.1

[PATCH 4/6] iommu/vt-d: clear the redundant assignment for domain->nid

[Yijing Wang]

Alloc_domain() will initialize domain->nid to -1. So the
initialization for domain->nid in md_domain_init() is redundant,
clear it.

Signed-off-by: Yijing Wang <wangyijing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/intel-iommu.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 6b71608..d1d6636 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -4134,7 +4134,6 @@ static int md_domain_init(struct dmar_domain *domain, int guest_width)
 	domain->iommu_snooping = 0;
 	domain->iommu_superpage = 0;
 	domain->max_addr = 0;
-	domain->nid = -1;
 
 	/* always allocate the top pgd */
 	domain->pgd = (struct dma_pte *)alloc_pgtable_page(domain->nid);
-- 
1.7.1

[PATCH 5/6] iommu/vt-d: use inline function dma_pte_superpage instead of macros

[Yijing Wang]

Use inline function dma_pte_superpage() instead of macro for
better readability.

Signed-off-by: Yijing Wang <wangyijing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/intel-iommu.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index d1d6636..455896c 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -891,7 +891,7 @@ static struct dma_pte *dma_pfn_level_pte(struct dmar_domain *domain,
 			break;
 		}
 
-		if (pte->val & DMA_PTE_LARGE_PAGE) {
+		if (dma_pte_superpage(pte)) {
 			*large_page = total;
 			return pte;
 		}
-- 
1.7.1

[PATCH 6/6] iommu/vt-d: fix reference count in iommu_prepare_isa

[Yijing Wang]

Decrease the device reference count avoid memory leak.

Signed-off-by: Yijing Wang <wangyijing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/iommu/intel-iommu.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 455896c..a78a824 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -2401,7 +2401,8 @@ static inline void iommu_prepare_isa(void)
 	if (ret)
 		printk(KERN_ERR "IOMMU: Failed to create 0-16MiB identity map; "
 		       "floppy might not work\n");
-
+	
+	pci_dev_put(pdev);
 }
 #else
 static inline void iommu_prepare_isa(void)
-- 
1.7.1

Re: [PATCH 0/6] trivial cleanup for iommu/vt-d

[Yijing Wang]

ping...

On 2014/5/20 20:37, Yijing Wang wrote:
> Some cleanup patches for iommu/vt-d.
> 
> Yijing Wang (6):
>   iommu/vt-d: Use list_for_each_safe() to simplify code
>   iommu/vt-d: move up no_iommu and dmar_disabled check
>   iommu/vt-d: clear the redundant assignment in dmar_enable_qi
>   iommu/vt-d: clear the redundant assignment for domain->nid
>   iommu/vt-d: use inline function dma_pte_superpage instead of macros
>   iommu/vt-d: fix reference count in iommu_prepare_isa
> 
>  drivers/iommu/dmar.c        |    3 ---
>  drivers/iommu/intel-iommu.c |   18 ++++++++----------
>  2 files changed, 8 insertions(+), 13 deletions(-)
> 
> 
> 
> .
> 


-- 
Thanks!
Yijing

Re: [PATCH 0/6] trivial cleanup for iommu/vt-d

[Joerg Roedel]

On Tue, May 20, 2014 at 08:37:46PM +0800, Yijing Wang wrote:
> 
> Yijing Wang (6):
>   iommu/vt-d: Use list_for_each_safe() to simplify code
>   iommu/vt-d: move up no_iommu and dmar_disabled check
>   iommu/vt-d: clear the redundant assignment in dmar_enable_qi
>   iommu/vt-d: clear the redundant assignment for domain->nid
>   iommu/vt-d: use inline function dma_pte_superpage instead of macros
>   iommu/vt-d: fix reference count in iommu_prepare_isa

Applied patches 1, 3, 4, 5 and 6, thanks. Also added another patch
on-top, see below.

>From 77c704bebc5d023ece7af32ea18bc3508cdb0007 Mon Sep 17 00:00:00 2001
From: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
Date: Fri, 4 Jul 2014 11:19:10 +0200
Subject: [PATCH] iommu/vt-d: Don't use magic number in dma_pte_superpage

Use the already defined DMA_PTE_LARGE_PAGE for testing
instead of hardcoding the value again.

Signed-off-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
---
 drivers/iommu/intel-iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 9b9f28e..5d86e93 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -304,7 +304,7 @@ static inline bool dma_pte_present(struct dma_pte *pte)
 
 static inline bool dma_pte_superpage(struct dma_pte *pte)
 {
-	return (pte->val & (1 << 7));
+	return (pte->val & DMA_PTE_LARGE_PAGE);
 }
 
 static inline int first_pte_in_page(struct dma_pte *pte)
-- 
1.8.4.5

Re: [PATCH 0/6] trivial cleanup for iommu/vt-d

[Yijing Wang]

On 2014/7/4 17:22, Joerg Roedel wrote:
> On Tue, May 20, 2014 at 08:37:46PM +0800, Yijing Wang wrote:
>>
>> Yijing Wang (6):
>>   iommu/vt-d: Use list_for_each_safe() to simplify code
>>   iommu/vt-d: move up no_iommu and dmar_disabled check
>>   iommu/vt-d: clear the redundant assignment in dmar_enable_qi
>>   iommu/vt-d: clear the redundant assignment for domain->nid
>>   iommu/vt-d: use inline function dma_pte_superpage instead of macros
>>   iommu/vt-d: fix reference count in iommu_prepare_isa
> 
> Applied patches 1, 3, 4, 5 and 6, thanks. Also added another patch
> on-top, see below.

Thanks a lot!

Yijing.

> 
>>From 77c704bebc5d023ece7af32ea18bc3508cdb0007 Mon Sep 17 00:00:00 2001
> From: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
> Date: Fri, 4 Jul 2014 11:19:10 +0200
> Subject: [PATCH] iommu/vt-d: Don't use magic number in dma_pte_superpage
> 
> Use the already defined DMA_PTE_LARGE_PAGE for testing
> instead of hardcoding the value again.
> 
> Signed-off-by: Joerg Roedel <jroedel-l3A5Bk7waGM@public.gmane.org>
> ---
>  drivers/iommu/intel-iommu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 9b9f28e..5d86e93 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -304,7 +304,7 @@ static inline bool dma_pte_present(struct dma_pte *pte)
>  
>  static inline bool dma_pte_superpage(struct dma_pte *pte)
>  {
> -	return (pte->val & (1 << 7));
> +	return (pte->val & DMA_PTE_LARGE_PAGE);
>  }
>  
>  static inline int first_pte_in_page(struct dma_pte *pte)
> 


-- 
Thanks!
Yijing