[PATCH] amd iommu: disable iommu emulation on non-iommu systems

[PATCH] amd iommu: disable iommu emulation on non-iommu systems

[Wei Wang]

[-- Attachment #1: Type: text/plain, Size: 404 bytes --]

Introduce a new flag to disable iommu emulation on old iommu systems.
This patch is taken from my v4 patch queue, which is till pending, to 
make old or non-iommu system to run cleanly without interfered by 
iommuv2 codes. This might be helpful to isolate iommuv2 code in 
debugging unstable regressions. The reset part of v4 will be re-based.

Thanks,
Wei

Signed-off-by: Wei Wang <wei.wang2@amd.com>



[-- Attachment #2: iommuv2_flag.patch --]
[-- Type: text/x-patch, Size: 2264 bytes --]

diff -r a2a8089b1ffb xen/drivers/passthrough/amd/iommu_guest.c
--- a/xen/drivers/passthrough/amd/iommu_guest.c	Tue Jan 24 16:46:17 2012 +0000
+++ b/xen/drivers/passthrough/amd/iommu_guest.c	Thu Jan 26 11:50:02 2012 +0100
@@ -805,6 +805,9 @@ int guest_iommu_set_base(struct domain *
     p2m_type_t t;
     struct guest_iommu *iommu = domain_iommu(d);
 
+    if ( !is_hvm_domain(d) || !iommu_enabled || !iommuv2_enabled )
+        return 0;
+
     if ( !iommu )
         return -EACCES;
 
@@ -867,7 +870,7 @@ int guest_iommu_init(struct domain* d)
     struct guest_iommu *iommu;
     struct hvm_iommu *hd  = domain_hvm_iommu(d);
 
-    if ( !is_hvm_domain(d) )
+    if ( !is_hvm_domain(d) || !iommu_enabled || !iommuv2_enabled )
         return 0;
 
     iommu = xzalloc(struct guest_iommu);
@@ -893,7 +896,7 @@ void guest_iommu_destroy(struct domain *
 {
     struct guest_iommu *iommu;
 
-    if ( !is_hvm_domain(d) )
+    if ( !is_hvm_domain(d) || !iommu_enabled || !iommuv2_enabled )
         return;
 
     iommu = domain_iommu(d);
diff -r a2a8089b1ffb xen/drivers/passthrough/amd/iommu_init.c
--- a/xen/drivers/passthrough/amd/iommu_init.c	Tue Jan 24 16:46:17 2012 +0000
+++ b/xen/drivers/passthrough/amd/iommu_init.c	Thu Jan 26 11:50:02 2012 +0100
@@ -38,6 +38,7 @@ unsigned short ivrs_bdf_entries;
 static struct radix_tree_root ivrs_maps;
 struct list_head amd_iommu_head;
 struct table_struct device_table;
+bool_t iommuv2_enabled = 0;
 
 static int iommu_has_ht_flag(struct amd_iommu *iommu, u8 mask)
 {
@@ -887,6 +888,9 @@ static int __init amd_iommu_init_one(str
 
     get_iommu_features(iommu);
 
+    if ( iommu->features )
+        iommuv2_enabled = 1;
+
     if ( allocate_cmd_buffer(iommu) == NULL )
         goto error_out;
 
@@ -950,6 +954,7 @@ static void __init amd_iommu_init_cleanu
     iommu_enabled = 0;
     iommu_passthrough = 0;
     iommu_intremap = 0;
+    iommuv2_enabled = 0;
 }
 
 /*
diff -r a2a8089b1ffb xen/include/asm-x86/amd-iommu.h
--- a/xen/include/asm-x86/amd-iommu.h	Tue Jan 24 16:46:17 2012 +0000
+++ b/xen/include/asm-x86/amd-iommu.h	Thu Jan 26 11:50:02 2012 +0100
@@ -182,4 +182,6 @@ struct guest_iommu {
     struct guest_iommu_msi  msi;
 };
 
+extern bool_t iommuv2_enabled;
+
 #endif /* _ASM_X86_64_AMD_IOMMU_H */


[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] amd iommu: disable iommu emulation on non-iommu systems

[Jan Beulich]

>>> On 26.01.12 at 11:56, Wei Wang <wei.wang2@amd.com> wrote:
>--- a/xen/drivers/passthrough/amd/iommu_guest.c	Tue Jan 24 16:46:17 2012 +0000
>+++ b/xen/drivers/passthrough/amd/iommu_guest.c	Thu Jan 26 11:50:02 2012 +0100
>@@ -805,6 +805,9 @@ int guest_iommu_set_base(struct domain *
>     p2m_type_t t;
>     struct guest_iommu *iommu = domain_iommu(d);
> 
>+    if ( !is_hvm_domain(d) || !iommu_enabled || !iommuv2_enabled )
>+        return 0;

Is it really appropriate/correct to return 0 here, while ...

>+
>     if ( !iommu )
>         return -EACCES;
> 

... here it is -EACCES? Further, are the extra checks needed at all
(i.e. wouldn't domain_iommu() return NULL in all of these cases
anyway due to the same checks being added to guest_iommu_init())?
If so, the checks you add to guest_iommu_destroy() are pointless
too.

Jan

Re: [PATCH] amd iommu: disable iommu emulation on non-iommu systems

[Wei Wang]

On 02/01/2012 04:58 PM, Jan Beulich wrote:
>>>> On 26.01.12 at 11:56, Wei Wang<wei.wang2@amd.com>  wrote:
>> --- a/xen/drivers/passthrough/amd/iommu_guest.c	Tue Jan 24 16:46:17 2012 +0000
>> +++ b/xen/drivers/passthrough/amd/iommu_guest.c	Thu Jan 26 11:50:02 2012 +0100
>> @@ -805,6 +805,9 @@ int guest_iommu_set_base(struct domain *
>>      p2m_type_t t;
>>      struct guest_iommu *iommu = domain_iommu(d);
>>
>> +    if ( !is_hvm_domain(d) || !iommu_enabled || !iommuv2_enabled )
>> +        return 0;
>
> Is it really appropriate/correct to return 0 here, while ...


good point, will be fixed in the next try. No one use 
guest_iommu_set_base so far until remaining patches got committed.

>> +
>>      if ( !iommu )
>>          return -EACCES;

>
> ... here it is -EACCES? Further, are the extra checks needed at all
> (i.e. wouldn't domain_iommu() return NULL in all of these cases
> anyway due to the same checks being added to guest_iommu_init())?
> If so, the checks you add to guest_iommu_destroy() are pointless
> too.

It is just to make sure those functions are not called by an unexpected 
code path since it is non-static. But I can remove it if you prefer that.


Thanks,
Wei

> Jan
>
>

Re: [PATCH] amd iommu: disable iommu emulation on non-iommu systems

[Jan Beulich]

>>> On 03.02.12 at 15:09, Wei Wang <wei.wang2@amd.com> wrote:
> On 02/01/2012 04:58 PM, Jan Beulich wrote:
>> ... Further, are the extra checks needed at all
>> (i.e. wouldn't domain_iommu() return NULL in all of these cases
>> anyway due to the same checks being added to guest_iommu_init())?
>> If so, the checks you add to guest_iommu_destroy() are pointless
>> too.
> 
> It is just to make sure those functions are not called by an unexpected 
> code path since it is non-static. But I can remove it if you prefer that.

Keir already committed the patch, but converting things like this
(where the impossible is being checked) to assertions is preferred
imo (meaning less redundant, possibly dead code in production
builds), so a follow-up patch would be appreciated.

Jan

Re: [PATCH] amd iommu: disable iommu emulation on non-iommu systems

[Wei Wang]

On 02/03/2012 03:11 PM, Jan Beulich wrote:
>>>> On 03.02.12 at 15:09, Wei Wang<wei.wang2@amd.com>  wrote:
>> On 02/01/2012 04:58 PM, Jan Beulich wrote:
>>> ... Further, are the extra checks needed at all
>>> (i.e. wouldn't domain_iommu() return NULL in all of these cases
>>> anyway due to the same checks being added to guest_iommu_init())?
>>> If so, the checks you add to guest_iommu_destroy() are pointless
>>> too.
>>
>> It is just to make sure those functions are not called by an unexpected
>> code path since it is non-static. But I can remove it if you prefer that.
>
> Keir already committed the patch, but converting things like this
> (where the impossible is being checked) to assertions is preferred
> imo (meaning less redundant, possibly dead code in production
> builds), so a follow-up patch would be appreciated.
No problem. Do it right away.
Thanks,
Wei

> Jan
>
>