diff mbox series

[4/5] iommu: set 'hap_pt_share' and 'need_sync' flags earlier in iommu_domain_init()

Message ID 20201005094905.2929-5-paul@xen.org (mailing list archive)
State New, archived
Headers show
Series iommu page-table memory pool | expand

Commit Message

Paul Durrant Oct. 5, 2020, 9:49 a.m. UTC
From: Paul Durrant <pdurrant@amazon.com>

Set these flags prior to the calls to arch_iommu_domain_init() and the
implementation init() method. There is no reason to hide this information from
those functions and the value of 'hap_pt_share' will be needed by a
modification to arch_iommu_domain_init() made in a subsequent patch.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
---
 xen/drivers/passthrough/iommu.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

Comments

Julien Grall Oct. 16, 2020, 4:07 p.m. UTC | #1
Hi Paul,

On 05/10/2020 10:49, Paul Durrant wrote:
> From: Paul Durrant <pdurrant@amazon.com>
> 
> Set these flags prior to the calls to arch_iommu_domain_init() and the
> implementation init() method. There is no reason to hide this information from
> those functions and the value of 'hap_pt_share' will be needed by a
> modification to arch_iommu_domain_init() made in a subsequent patch.
> 
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>

Reviewed-by: Julien Grall <jgrall@amazon.com>

Cheers,

> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> ---
>   xen/drivers/passthrough/iommu.c | 16 ++++++----------
>   1 file changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
> index 642d5c8331..fd9705b3a9 100644
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -174,15 +174,6 @@ int iommu_domain_init(struct domain *d, unsigned int opts)
>       hd->node = NUMA_NO_NODE;
>   #endif
>   
> -    ret = arch_iommu_domain_init(d);
> -    if ( ret )
> -        return ret;
> -
> -    hd->platform_ops = iommu_get_ops();
> -    ret = hd->platform_ops->init(d);
> -    if ( ret || is_system_domain(d) )
> -        return ret;
> -
>       /*
>        * Use shared page tables for HAP and IOMMU if the global option
>        * is enabled (from which we can infer the h/w is capable) and
> @@ -202,7 +193,12 @@ int iommu_domain_init(struct domain *d, unsigned int opts)
>   
>       ASSERT(!(hd->need_sync && hd->hap_pt_share));
>   
> -    return 0;
> +    ret = arch_iommu_domain_init(d);
> +    if ( ret )
> +        return ret;
> +
> +    hd->platform_ops = iommu_get_ops();
> +    return hd->platform_ops->init(d);
>   }
>   
>   void __hwdom_init iommu_hwdom_init(struct domain *d)
>
Jan Beulich Oct. 30, 2020, 4:11 p.m. UTC | #2
On 05.10.2020 11:49, Paul Durrant wrote:
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -174,15 +174,6 @@ int iommu_domain_init(struct domain *d, unsigned int opts)
>      hd->node = NUMA_NO_NODE;
>  #endif
>  
> -    ret = arch_iommu_domain_init(d);
> -    if ( ret )
> -        return ret;
> -
> -    hd->platform_ops = iommu_get_ops();
> -    ret = hd->platform_ops->init(d);
> -    if ( ret || is_system_domain(d) )
> -        return ret;

Are you suggesting the is_system_domain() here has become
unnecessary? If so, it would be nice if you could say when
or why. Otherwise I would assume it's needed to avoid
setting one or both of the fields.

Jan
diff mbox series

Patch

diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 642d5c8331..fd9705b3a9 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -174,15 +174,6 @@  int iommu_domain_init(struct domain *d, unsigned int opts)
     hd->node = NUMA_NO_NODE;
 #endif
 
-    ret = arch_iommu_domain_init(d);
-    if ( ret )
-        return ret;
-
-    hd->platform_ops = iommu_get_ops();
-    ret = hd->platform_ops->init(d);
-    if ( ret || is_system_domain(d) )
-        return ret;
-
     /*
      * Use shared page tables for HAP and IOMMU if the global option
      * is enabled (from which we can infer the h/w is capable) and
@@ -202,7 +193,12 @@  int iommu_domain_init(struct domain *d, unsigned int opts)
 
     ASSERT(!(hd->need_sync && hd->hap_pt_share));
 
-    return 0;
+    ret = arch_iommu_domain_init(d);
+    if ( ret )
+        return ret;
+
+    hd->platform_ops = iommu_get_ops();
+    return hd->platform_ops->init(d);
 }
 
 void __hwdom_init iommu_hwdom_init(struct domain *d)