Move its definition to the AMD-Vi driver and use CONFIG_AMD_IOMMU
to guard its usage in common code.

Take the opportunity to replace bool_t with bool and 1 with true.

No functional change intended.

Signed-off-by: Xenia Ragiadakou <burzalodowa@gmail.com>
---
 xen/drivers/passthrough/amd/iommu_init.c | 2 ++
 xen/drivers/passthrough/iommu.c          | 3 ++-
 xen/include/xen/iommu.h                  | 4 +++-
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/xen/drivers/passthrough/amd/iommu_init.c b/xen/drivers/passthrough/amd/iommu_init.c
index 1f14aaf49e..c4a41630e5 100644
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -36,6 +36,8 @@ static struct radix_tree_root ivrs_maps;
 LIST_HEAD_READ_MOSTLY(amd_iommu_head);
 bool_t iommuv2_enabled;
 
+bool __read_mostly amd_iommu_perdev_intremap = true;
+
 static bool iommu_has_ht_flag(struct amd_iommu *iommu, u8 mask)
 {
     return iommu->ht_flags & mask;
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 5e2a720d29..1a02fdc453 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -58,7 +58,6 @@ bool __read_mostly iommu_hap_pt_share = true;
 #endif
 
 bool_t __read_mostly iommu_debug;
-bool_t __read_mostly amd_iommu_perdev_intremap = 1;
 
 DEFINE_PER_CPU(bool_t, iommu_dont_flush_iotlb);
 
@@ -115,8 +114,10 @@ static int __init cf_check parse_iommu_param(const char *s)
             if ( val )
                 iommu_verbose = 1;
         }
+#ifdef CONFIG_AMD_IOMMU
         else if ( (val = parse_boolean("amd-iommu-perdev-intremap", s, ss)) >= 0 )
             amd_iommu_perdev_intremap = val;
+#endif
         else if ( (val = parse_boolean("dom0-passthrough", s, ss)) >= 0 )
             iommu_hwdom_passthrough = val;
         else if ( (val = parse_boolean("dom0-strict", s, ss)) >= 0 )
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index 4f22fc1bed..6b06732792 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -104,7 +104,9 @@ static inline void clear_iommu_hap_pt_share(void)
 }
 
 extern bool_t iommu_debug;
-extern bool_t amd_iommu_perdev_intremap;
+#ifdef CONFIG_AMD_IOMMU
+extern bool amd_iommu_perdev_intremap;
+#endif
 
 extern bool iommu_hwdom_strict, iommu_hwdom_passthrough, iommu_hwdom_inclusive;
 extern int8_t iommu_hwdom_reserved;
-- 
2.37.2

On 19.12.2022 07:34, Xenia Ragiadakou wrote:
> Move its definition to the AMD-Vi driver and use CONFIG_AMD_IOMMU
> to guard its usage in common code.
> 
> Take the opportunity to replace bool_t with bool and 1 with true.

Please further take the opportunity and use ...

> --- a/xen/drivers/passthrough/amd/iommu_init.c
> +++ b/xen/drivers/passthrough/amd/iommu_init.c
> @@ -36,6 +36,8 @@ static struct radix_tree_root ivrs_maps;
>  LIST_HEAD_READ_MOSTLY(amd_iommu_head);
>  bool_t iommuv2_enabled;
>  
> +bool __read_mostly amd_iommu_perdev_intremap = true;

... __ro_after_init here.

> --- a/xen/include/xen/iommu.h
> +++ b/xen/include/xen/iommu.h
> @@ -104,7 +104,9 @@ static inline void clear_iommu_hap_pt_share(void)
>  }
>  
>  extern bool_t iommu_debug;
> -extern bool_t amd_iommu_perdev_intremap;
> +#ifdef CONFIG_AMD_IOMMU
> +extern bool amd_iommu_perdev_intremap;
> +#endif

We try to avoid such #ifdef-ary: There's no real harm from the declaration
being in scope; in the worst case the build will fail not at the compile,
but at the linking stage.

Jan

On 12/20/22 19:04, Jan Beulich wrote:
> On 19.12.2022 07:34, Xenia Ragiadakou wrote:
>> Move its definition to the AMD-Vi driver and use CONFIG_AMD_IOMMU
>> to guard its usage in common code.
>>
>> Take the opportunity to replace bool_t with bool and 1 with true.
> 
> Please further take the opportunity and use ...
> 
>> --- a/xen/drivers/passthrough/amd/iommu_init.c
>> +++ b/xen/drivers/passthrough/amd/iommu_init.c
>> @@ -36,6 +36,8 @@ static struct radix_tree_root ivrs_maps;
>>   LIST_HEAD_READ_MOSTLY(amd_iommu_head);
>>   bool_t iommuv2_enabled;
>>   
>> +bool __read_mostly amd_iommu_perdev_intremap = true;
> 
> ... __ro_after_init here.
> 
>> --- a/xen/include/xen/iommu.h
>> +++ b/xen/include/xen/iommu.h
>> @@ -104,7 +104,9 @@ static inline void clear_iommu_hap_pt_share(void)
>>   }
>>   
>>   extern bool_t iommu_debug;
>> -extern bool_t amd_iommu_perdev_intremap;
>> +#ifdef CONFIG_AMD_IOMMU
>> +extern bool amd_iommu_perdev_intremap;
>> +#endif
> 
> We try to avoid such #ifdef-ary: There's no real harm from the declaration
> being in scope; in the worst case the build will fail not at the compile,
> but at the linking stage.

That's true. I will leave it as it is.

> 
> Jan

-- 
Xenia

On 19/12/2022 6:34 am, Xenia Ragiadakou wrote:
> diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
> index 5e2a720d29..1a02fdc453 100644
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -58,7 +58,6 @@ bool __read_mostly iommu_hap_pt_share = true;
>  #endif
>  
>  bool_t __read_mostly iommu_debug;
> -bool_t __read_mostly amd_iommu_perdev_intremap = 1;
>  
>  DEFINE_PER_CPU(bool_t, iommu_dont_flush_iotlb);
>  
> @@ -115,8 +114,10 @@ static int __init cf_check parse_iommu_param(const char *s)
>              if ( val )
>                  iommu_verbose = 1;
>          }
> +#ifdef CONFIG_AMD_IOMMU
>          else if ( (val = parse_boolean("amd-iommu-perdev-intremap", s, ss)) >= 0 )
>              amd_iommu_perdev_intremap = val;
> +#endif

See parse_cet() and the use of no_config_param() so users get a bit of a
hint as to why the option they specified is getting ignored.

~Andrew

On 12/20/22 12:31, Andrew Cooper wrote:
> On 19/12/2022 6:34 am, Xenia Ragiadakou wrote:
>> diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
>> index 5e2a720d29..1a02fdc453 100644
>> --- a/xen/drivers/passthrough/iommu.c
>> +++ b/xen/drivers/passthrough/iommu.c
>> @@ -58,7 +58,6 @@ bool __read_mostly iommu_hap_pt_share = true;
>>   #endif
>>   
>>   bool_t __read_mostly iommu_debug;
>> -bool_t __read_mostly amd_iommu_perdev_intremap = 1;
>>   
>>   DEFINE_PER_CPU(bool_t, iommu_dont_flush_iotlb);
>>   
>> @@ -115,8 +114,10 @@ static int __init cf_check parse_iommu_param(const char *s)
>>               if ( val )
>>                   iommu_verbose = 1;
>>           }
>> +#ifdef CONFIG_AMD_IOMMU
>>           else if ( (val = parse_boolean("amd-iommu-perdev-intremap", s, ss)) >= 0 )
>>               amd_iommu_perdev_intremap = val;
>> +#endif
> 
> See parse_cet() and the use of no_config_param() so users get a bit of a
> hint as to why the option they specified is getting ignored.

Ah, ok I see. Thx for pointing that out.

> 
> ~Andrew

-- 
Xenia