[RFC,for-4.5,06/12] xen/passthrough: rework dom0_pvh_reqs to use it also on ARM

Message ID 1391794991-5919-7-git-send-email-julien.grall@linaro.org
State RFC
Headers show

Commit Message

Julien Grall Feb. 7, 2014, 5:43 p.m.
DOM0 on ARM will have the same requirements as DOM0 PVH when iommu is enabled.
Both PVH and ARM guest has paging mode translate enabled, so Xen can use it
to know if it needs to check the requirements.

Rename the function and remove "pvh" word in the commit message.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
Cc: Xiantao Zhang <xiantao.zhang@intel.com>
Cc: Jan Beulich <jbeulich@suse.com>
---
 xen/drivers/passthrough/iommu.c |   14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

Comments

Julien Grall Feb. 10, 2014, 11:42 a.m. | #1
Hello Jan,

On 10/02/14 08:07, Jan Beulich wrote:
>>>> On 07.02.14 at 18:43, Julien Grall <julien.grall@linaro.org> wrote:
>> DOM0 on ARM will have the same requirements as DOM0 PVH when iommu is
>> enabled.
>> Both PVH and ARM guest has paging mode translate enabled, so Xen can use it
>> to know if it needs to check the requirements.
>>
>> Rename the function and remove "pvh" word in the commit message.
>
> s/commit/panic/ ?

It's an error when I wrote the commit message. I will replace it on the 
next version.


>>
>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>> Cc: Xiantao Zhang <xiantao.zhang@intel.com>
>
> Other than the above,
> Acked-by: Jan Beulich <jbeulich@suse.com>

Thanks.

Regards,
Julien Grall Feb. 10, 2014, 4:10 p.m. | #2
On 02/07/2014 05:43 PM, Julien Grall wrote:
> DOM0 on ARM will have the same requirements as DOM0 PVH when iommu is enabled.
> Both PVH and ARM guest has paging mode translate enabled, so Xen can use it
> to know if it needs to check the requirements.
> 
> Rename the function and remove "pvh" word in the commit message.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> Cc: Xiantao Zhang <xiantao.zhang@intel.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> ---
>  xen/drivers/passthrough/iommu.c |   14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
> index 19b0e23..26a5d91 100644
> --- a/xen/drivers/passthrough/iommu.c
> +++ b/xen/drivers/passthrough/iommu.c
> @@ -130,13 +130,18 @@ int iommu_domain_init(struct domain *d)
>      return hd->platform_ops->init(d);
>  }
>  
> -static __init void check_dom0_pvh_reqs(struct domain *d)
> +static __init void check_dom0_reqs(struct domain *d)
>  {
> +    if ( !paging_mode_translate(d) )
> +        return;
> +
>      if ( !iommu_enabled )
> -        panic("Presently, iommu must be enabled for pvh dom0\n");
> +        panic("Presently, iommu must be enabled to use dom0 with translate "
> +              "paging mode\n");

Hmmm... this change is wrong. I forgot that iommu doesn't exist on some
ARM platform (for instance Arndale).

Do we really this check for PVH? If yes, I will add replace the check
by: is_pvh_domain(d) && !iommu_enabled.
Julien Grall Feb. 10, 2014, 5:42 p.m. | #3
On 02/10/2014 04:35 PM, Jan Beulich wrote:
>>>> On 10.02.14 at 17:10, Julien Grall <julien.grall@linaro.org> wrote:
>> On 02/07/2014 05:43 PM, Julien Grall wrote:
>>> DOM0 on ARM will have the same requirements as DOM0 PVH when iommu is 
>> enabled.
>>> Both PVH and ARM guest has paging mode translate enabled, so Xen can use it
>>> to know if it needs to check the requirements.
>>>
>>> Rename the function and remove "pvh" word in the commit message.
>>>
>>> Signed-off-by: Julien Grall <julien.grall@linaro.org>
>>> Cc: Xiantao Zhang <xiantao.zhang@intel.com>
>>> Cc: Jan Beulich <jbeulich@suse.com>
>>> ---
>>>  xen/drivers/passthrough/iommu.c |   14 +++++++++-----
>>>  1 file changed, 9 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
>>> index 19b0e23..26a5d91 100644
>>> --- a/xen/drivers/passthrough/iommu.c
>>> +++ b/xen/drivers/passthrough/iommu.c
>>> @@ -130,13 +130,18 @@ int iommu_domain_init(struct domain *d)
>>>      return hd->platform_ops->init(d);
>>>  }
>>>  
>>> -static __init void check_dom0_pvh_reqs(struct domain *d)
>>> +static __init void check_dom0_reqs(struct domain *d)
>>>  {
>>> +    if ( !paging_mode_translate(d) )
>>> +        return;
>>> +
>>>      if ( !iommu_enabled )
>>> -        panic("Presently, iommu must be enabled for pvh dom0\n");
>>> +        panic("Presently, iommu must be enabled to use dom0 with translate 
>> "
>>> +              "paging mode\n");
>>
>> Hmmm... this change is wrong. I forgot that iommu doesn't exist on some
>> ARM platform (for instance Arndale).
>>
>> Do we really this check for PVH? If yes, I will add replace the check
>> by: is_pvh_domain(d) && !iommu_enabled.
> 
> Of course we need it: How would PVH Dom0 be able to do any kind
> of DMA without an IOMMU?

Right, on ARM we have the 1:1 memory mapping to avoid this issue.

I will fix it. Can I keep your ack on this patch?

> Jan
>

Patch

diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index 19b0e23..26a5d91 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -130,13 +130,18 @@  int iommu_domain_init(struct domain *d)
     return hd->platform_ops->init(d);
 }
 
-static __init void check_dom0_pvh_reqs(struct domain *d)
+static __init void check_dom0_reqs(struct domain *d)
 {
+    if ( !paging_mode_translate(d) )
+        return;
+
     if ( !iommu_enabled )
-        panic("Presently, iommu must be enabled for pvh dom0\n");
+        panic("Presently, iommu must be enabled to use dom0 with translate "
+              "paging mode\n");
 
     if ( iommu_passthrough )
-        panic("For pvh dom0, dom0-passthrough must not be enabled\n");
+        panic("Dom0 uses translate paging mode, dom0-passthrough must not be "
+              "enabled\n");
 
     iommu_dom0_strict = 1;
 }
@@ -145,8 +150,7 @@  void __init iommu_dom0_init(struct domain *d)
 {
     struct hvm_iommu *hd = domain_hvm_iommu(d);
 
-    if ( is_pvh_domain(d) )
-        check_dom0_pvh_reqs(d);
+    check_dom0_reqs(d);
 
     if ( !iommu_enabled )
         return;