Re: [Xen-devel] [PATCH v3 16/24] xen/passthrough: Introduce iommu_construct
Hi Ian, On 23/02/15 15:31, Ian Campbell wrote: > On Fri, 2015-02-20 at 17:45 +, Julien Grall wrote: >> On 20/02/15 16:58, Ian Campbell wrote: >>> On Tue, 2015-01-13 at 14:25 +, Julien Grall wrote: This new function will correctly initialize the IOMMU page table for the current domain. Also use it in iommu_assign_dt_device even though the current IOMMU implementation on ARM shares P2M with the processor. Signed-off-by: Julien Grall Cc: Jan Beulich >>> >>> Acked-by: Ian Campbell >>> >>> Although I'm not actually sure what it is going to be used for. >> >> The caller are in this patch too :) (see pci.c and device_tree.c changes). > > Right, I meant "what non-trivial functionality is going to land here" > i..e what the grandparent callers etc will eventually expect from it. Oh ok. The function is used to the prepare the IOMMU structure for the domain. Currently, the steps are: - build the page table on platforms which don't share the P2M - set need_iommu to 1. Within this series, this is the only patch who touch this function. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 16/24] xen/passthrough: Introduce iommu_construct
On Fri, 2015-02-20 at 17:45 +, Julien Grall wrote: > On 20/02/15 16:58, Ian Campbell wrote: > > On Tue, 2015-01-13 at 14:25 +, Julien Grall wrote: > >> This new function will correctly initialize the IOMMU page table for the > >> current domain. > >> > >> Also use it in iommu_assign_dt_device even though the current IOMMU > >> implementation on ARM shares P2M with the processor. > >> > >> Signed-off-by: Julien Grall > >> Cc: Jan Beulich > > > > Acked-by: Ian Campbell > > > > Although I'm not actually sure what it is going to be used for. > > The caller are in this patch too :) (see pci.c and device_tree.c changes). Right, I meant "what non-trivial functionality is going to land here" i..e what the grandparent callers etc will eventually expect from it. Ian ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 16/24] xen/passthrough: Introduce iommu_construct
On 20/02/15 16:58, Ian Campbell wrote: > On Tue, 2015-01-13 at 14:25 +, Julien Grall wrote: >> This new function will correctly initialize the IOMMU page table for the >> current domain. >> >> Also use it in iommu_assign_dt_device even though the current IOMMU >> implementation on ARM shares P2M with the processor. >> >> Signed-off-by: Julien Grall >> Cc: Jan Beulich > > Acked-by: Ian Campbell > > Although I'm not actually sure what it is going to be used for. The caller are in this patch too :) (see pci.c and device_tree.c changes). > Also, needs Jan to ack I think. I have some comments from Jan which I need to address in the next series. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 16/24] xen/passthrough: Introduce iommu_construct
On Tue, 2015-01-13 at 14:25 +, Julien Grall wrote: > This new function will correctly initialize the IOMMU page table for the > current domain. > > Also use it in iommu_assign_dt_device even though the current IOMMU > implementation on ARM shares P2M with the processor. > > Signed-off-by: Julien Grall > Cc: Jan Beulich Acked-by: Ian Campbell Although I'm not actually sure what it is going to be used for. Also, needs Jan to ack I think. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 16/24] xen/passthrough: Introduce iommu_construct
>>> On 21.01.15 at 15:22, wrote: > On 21/01/15 14:13, Jan Beulich wrote: > On 21.01.15 at 13:13, wrote: >>> iommu_dom0_strict is always set to 1 when IOMMU is used on ARM (see >>> check_hwdom_reqs). >>> >>> Futhermore, we always share the page table with the processor, so we >>> never need to populate the page table. >> >> That's all far from obvious looking at the patch at hand. And you >> can then only hope that these two "always" will always remain to >> be that way. > > Hence the suggested ASSERT on a previous mail. I could also add a > comment in the code explaining the ASSERT(). Yes please, for both parts. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 16/24] xen/passthrough: Introduce iommu_construct
On 21/01/15 14:13, Jan Beulich wrote: On 21.01.15 at 13:13, wrote: >> Hi Jan, >> >> On 21/01/15 10:48, Jan Beulich wrote: >> On 21.01.15 at 11:37, wrote: On 21/01/2015 10:23, Jan Beulich wrote: On 20.01.15 at 18:11, wrote: >> While this function is currently only used for DOM0, this will be used >> in a later patch for guest non-PCI passthrough. > > Okay, but you shouldn't break (or alter in [seemingly] benign ways) the > Dom0 case imo. As iommu_hwdom_init is initialized correctly the IOMMU for DOM0, iommu_construct is a no-op. Would an if ( need_iommu(d) ) will be more clear? Maybe we an assert (!is_hardware_domain(d)). >>> >>> Just think this through properly: iommu_hwdom_init() may leave >>> Dom0's ->need_iommu at 0 or 1 (depending on iommu_dom0_strict). >>> And iommu_construct() specifically is a nop only when ->need_iommu >>> is positive (x86's arch_iommu_populate_page_table() sets it to a >>> negative value to indicate "being set up", and I wonder how ARM >>> gets away without doing so). >> >> iommu_dom0_strict is always set to 1 when IOMMU is used on ARM (see >> check_hwdom_reqs). >> >> Futhermore, we always share the page table with the processor, so we >> never need to populate the page table. > > That's all far from obvious looking at the patch at hand. And you > can then only hope that these two "always" will always remain to > be that way. Hence the suggested ASSERT on a previous mail. I could also add a comment in the code explaining the ASSERT(). Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 16/24] xen/passthrough: Introduce iommu_construct
>>> On 21.01.15 at 13:13, wrote: > Hi Jan, > > On 21/01/15 10:48, Jan Beulich wrote: > On 21.01.15 at 11:37, wrote: >>> On 21/01/2015 10:23, Jan Beulich wrote: >>> On 20.01.15 at 18:11, wrote: > While this function is currently only used for DOM0, this will be used > in a later patch for guest non-PCI passthrough. Okay, but you shouldn't break (or alter in [seemingly] benign ways) the Dom0 case imo. >>> >>> As iommu_hwdom_init is initialized correctly the IOMMU for DOM0, >>> iommu_construct is a no-op. >>> >>> Would an if ( need_iommu(d) ) will be more clear? Maybe we an assert >>> (!is_hardware_domain(d)). >> >> Just think this through properly: iommu_hwdom_init() may leave >> Dom0's ->need_iommu at 0 or 1 (depending on iommu_dom0_strict). >> And iommu_construct() specifically is a nop only when ->need_iommu >> is positive (x86's arch_iommu_populate_page_table() sets it to a >> negative value to indicate "being set up", and I wonder how ARM >> gets away without doing so). > > iommu_dom0_strict is always set to 1 when IOMMU is used on ARM (see > check_hwdom_reqs). > > Futhermore, we always share the page table with the processor, so we > never need to populate the page table. That's all far from obvious looking at the patch at hand. And you can then only hope that these two "always" will always remain to be that way. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 16/24] xen/passthrough: Introduce iommu_construct
Hi Jan, On 21/01/15 10:48, Jan Beulich wrote: On 21.01.15 at 11:37, wrote: >> On 21/01/2015 10:23, Jan Beulich wrote: >> On 20.01.15 at 18:11, wrote: While this function is currently only used for DOM0, this will be used in a later patch for guest non-PCI passthrough. >>> >>> Okay, but you shouldn't break (or alter in [seemingly] benign ways) the >>> Dom0 case imo. >> >> As iommu_hwdom_init is initialized correctly the IOMMU for DOM0, >> iommu_construct is a no-op. >> >> Would an if ( need_iommu(d) ) will be more clear? Maybe we an assert >> (!is_hardware_domain(d)). > > Just think this through properly: iommu_hwdom_init() may leave > Dom0's ->need_iommu at 0 or 1 (depending on iommu_dom0_strict). > And iommu_construct() specifically is a nop only when ->need_iommu > is positive (x86's arch_iommu_populate_page_table() sets it to a > negative value to indicate "being set up", and I wonder how ARM > gets away without doing so). iommu_dom0_strict is always set to 1 when IOMMU is used on ARM (see check_hwdom_reqs). Futhermore, we always share the page table with the processor, so we never need to populate the page table. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 16/24] xen/passthrough: Introduce iommu_construct
>>> On 21.01.15 at 11:37, wrote: > On 21/01/2015 10:23, Jan Beulich wrote: > On 20.01.15 at 18:11, wrote: >>> While this function is currently only used for DOM0, this will be used >>> in a later patch for guest non-PCI passthrough. >> >> Okay, but you shouldn't break (or alter in [seemingly] benign ways) the >> Dom0 case imo. > > As iommu_hwdom_init is initialized correctly the IOMMU for DOM0, > iommu_construct is a no-op. > > Would an if ( need_iommu(d) ) will be more clear? Maybe we an assert > (!is_hardware_domain(d)). Just think this through properly: iommu_hwdom_init() may leave Dom0's ->need_iommu at 0 or 1 (depending on iommu_dom0_strict). And iommu_construct() specifically is a nop only when ->need_iommu is positive (x86's arch_iommu_populate_page_table() sets it to a negative value to indicate "being set up", and I wonder how ARM gets away without doing so). Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 16/24] xen/passthrough: Introduce iommu_construct
On 21/01/2015 10:23, Jan Beulich wrote:
On 20.01.15 at 18:11, wrote:
On 20/01/15 16:40, Jan Beulich wrote:
On 20.01.15 at 15:28, wrote:
On 19/01/15 17:02, Jan Beulich wrote:
On 13.01.15 at 15:25, wrote:
--- a/xen/drivers/passthrough/device_tree.c
+++ b/xen/drivers/passthrough/device_tree.c
@@ -41,6 +41,10 @@ int iommu_assign_dt_device(struct domain *d, struct
dt_device_node *dev)
if ( !list_empty(&dev->domain_list) )
goto fail;
+rc = iommu_construct(d);
+if ( rc )
+goto fail;
Considering that the only (current) caller of this it domain_build.c I'm
afraid you're going to get into trouble if you get back -ERESTART
here. Note that on x86 Dom0 setup works via iommu_hwdom_init(),
which deals with the preemption needs (at that point in time) by
calling process_pending_softirqs() every once in a while.
iommu_hwdom_init is also called for ARM (it's part of the common domain
creation code). So, I don't see any issue here as we match the same
behavior as PCI.
FWIW, on the previous version you asked to check the need_iommu(d) in
iommu_construct. For DOM0 it will return 0 and therefore never return
-ERESTART.
Quoting the function:
+int iommu_construct(struct domain *d)
+{
+int rc = 0;
+
+if ( need_iommu(d) > 0 )
+return 0;
+
+if ( !iommu_use_hap_pt(d) )
+{
+rc = arch_iommu_populate_page_table(d);
+if ( rc )
+return rc;
+}
+
+d->need_iommu = 1;
+
+return rc;
+}
If need_iommu() returns 0 for Dom0, then the early return won't get
used. Hence I don't follow your comment above. And if what you say
there was correct, then I don't understand why you add the call
quoted at the very top in the first place (again taking into consideration
that - afaict - the only [current] caller is in domain_build.c).
I don't understand what is the issue in the device tree use case. As I
said, assign_device in the pci do exactly the same things.
Sure, but it's not being called for Dom0, but only out of the domctl
handler.
Hmmm right. It's the main difference between non-PCI and PCI passthrough.
While this function is currently only used for DOM0, this will be used
in a later patch for guest non-PCI passthrough.
Okay, but you shouldn't break (or alter in [seemingly] benign ways) the
Dom0 case imo.
As iommu_hwdom_init is initialized correctly the IOMMU for DOM0,
iommu_construct is a no-op.
Would an if ( need_iommu(d) ) will be more clear? Maybe we an assert
(!is_hardware_domain(d)).
Regards,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 16/24] xen/passthrough: Introduce iommu_construct
>>> On 20.01.15 at 18:11, wrote:
> On 20/01/15 16:40, Jan Beulich wrote:
> On 20.01.15 at 15:28, wrote:
>>> On 19/01/15 17:02, Jan Beulich wrote:
>>> On 13.01.15 at 15:25, wrote:
> --- a/xen/drivers/passthrough/device_tree.c
> +++ b/xen/drivers/passthrough/device_tree.c
> @@ -41,6 +41,10 @@ int iommu_assign_dt_device(struct domain *d, struct
> dt_device_node *dev)
> if ( !list_empty(&dev->domain_list) )
> goto fail;
>
> +rc = iommu_construct(d);
> +if ( rc )
> +goto fail;
Considering that the only (current) caller of this it domain_build.c I'm
afraid you're going to get into trouble if you get back -ERESTART
here. Note that on x86 Dom0 setup works via iommu_hwdom_init(),
which deals with the preemption needs (at that point in time) by
calling process_pending_softirqs() every once in a while.
>>>
>>> iommu_hwdom_init is also called for ARM (it's part of the common domain
>>> creation code). So, I don't see any issue here as we match the same
>>> behavior as PCI.
>>>
>>> FWIW, on the previous version you asked to check the need_iommu(d) in
>>> iommu_construct. For DOM0 it will return 0 and therefore never return
>>> -ERESTART.
>>
>> Quoting the function:
>>
>> +int iommu_construct(struct domain *d)
>> +{
>> +int rc = 0;
>> +
>> +if ( need_iommu(d) > 0 )
>> +return 0;
>> +
>> +if ( !iommu_use_hap_pt(d) )
>> +{
>> +rc = arch_iommu_populate_page_table(d);
>> +if ( rc )
>> +return rc;
>> +}
>> +
>> +d->need_iommu = 1;
>> +
>> +return rc;
>> +}
>
>> If need_iommu() returns 0 for Dom0, then the early return won't get
>> used. Hence I don't follow your comment above. And if what you say
>> there was correct, then I don't understand why you add the call
>> quoted at the very top in the first place (again taking into consideration
>> that - afaict - the only [current] caller is in domain_build.c).
>
> I don't understand what is the issue in the device tree use case. As I
> said, assign_device in the pci do exactly the same things.
Sure, but it's not being called for Dom0, but only out of the domctl
handler.
> While this function is currently only used for DOM0, this will be used
> in a later patch for guest non-PCI passthrough.
Okay, but you shouldn't break (or alter in [seemingly] benign ways) the
Dom0 case imo.
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 16/24] xen/passthrough: Introduce iommu_construct
On 20/01/15 16:40, Jan Beulich wrote:
On 20.01.15 at 15:28, wrote:
>> On 19/01/15 17:02, Jan Beulich wrote:
>> On 13.01.15 at 15:25, wrote:
--- a/xen/drivers/passthrough/device_tree.c
+++ b/xen/drivers/passthrough/device_tree.c
@@ -41,6 +41,10 @@ int iommu_assign_dt_device(struct domain *d, struct
dt_device_node *dev)
if ( !list_empty(&dev->domain_list) )
goto fail;
+rc = iommu_construct(d);
+if ( rc )
+goto fail;
>>>
>>> Considering that the only (current) caller of this it domain_build.c I'm
>>> afraid you're going to get into trouble if you get back -ERESTART
>>> here. Note that on x86 Dom0 setup works via iommu_hwdom_init(),
>>> which deals with the preemption needs (at that point in time) by
>>> calling process_pending_softirqs() every once in a while.
>>
>> iommu_hwdom_init is also called for ARM (it's part of the common domain
>> creation code). So, I don't see any issue here as we match the same
>> behavior as PCI.
>>
>> FWIW, on the previous version you asked to check the need_iommu(d) in
>> iommu_construct. For DOM0 it will return 0 and therefore never return
>> -ERESTART.
>
> Quoting the function:
>
> +int iommu_construct(struct domain *d)
> +{
> +int rc = 0;
> +
> +if ( need_iommu(d) > 0 )
> +return 0;
> +
> +if ( !iommu_use_hap_pt(d) )
> +{
> +rc = arch_iommu_populate_page_table(d);
> +if ( rc )
> +return rc;
> +}
> +
> +d->need_iommu = 1;
> +
> +return rc;
> +}
> If need_iommu() returns 0 for Dom0, then the early return won't get
> used. Hence I don't follow your comment above. And if what you say
> there was correct, then I don't understand why you add the call
> quoted at the very top in the first place (again taking into consideration
> that - afaict - the only [current] caller is in domain_build.c).
I don't understand what is the issue in the device tree use case. As I
said, assign_device in the pci do exactly the same things.
While this function is currently only used for DOM0, this will be used
in a later patch for guest non-PCI passthrough.
Regards,
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 16/24] xen/passthrough: Introduce iommu_construct
>>> On 20.01.15 at 15:28, wrote:
> On 19/01/15 17:02, Jan Beulich wrote:
> On 13.01.15 at 15:25, wrote:
>>> --- a/xen/drivers/passthrough/device_tree.c
>>> +++ b/xen/drivers/passthrough/device_tree.c
>>> @@ -41,6 +41,10 @@ int iommu_assign_dt_device(struct domain *d, struct
>>> dt_device_node *dev)
>>> if ( !list_empty(&dev->domain_list) )
>>> goto fail;
>>>
>>> +rc = iommu_construct(d);
>>> +if ( rc )
>>> +goto fail;
>>
>> Considering that the only (current) caller of this it domain_build.c I'm
>> afraid you're going to get into trouble if you get back -ERESTART
>> here. Note that on x86 Dom0 setup works via iommu_hwdom_init(),
>> which deals with the preemption needs (at that point in time) by
>> calling process_pending_softirqs() every once in a while.
>
> iommu_hwdom_init is also called for ARM (it's part of the common domain
> creation code). So, I don't see any issue here as we match the same
> behavior as PCI.
>
> FWIW, on the previous version you asked to check the need_iommu(d) in
> iommu_construct. For DOM0 it will return 0 and therefore never return
> -ERESTART.
Quoting the function:
+int iommu_construct(struct domain *d)
+{
+int rc = 0;
+
+if ( need_iommu(d) > 0 )
+return 0;
+
+if ( !iommu_use_hap_pt(d) )
+{
+rc = arch_iommu_populate_page_table(d);
+if ( rc )
+return rc;
+}
+
+d->need_iommu = 1;
+
+return rc;
+}
If need_iommu() returns 0 for Dom0, then the early return won't get
used. Hence I don't follow your comment above. And if what you say
there was correct, then I don't understand why you add the call
quoted at the very top in the first place (again taking into consideration
that - afaict - the only [current] caller is in domain_build.c).
Jan
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 16/24] xen/passthrough: Introduce iommu_construct
Hi Jan, On 19/01/15 17:02, Jan Beulich wrote: On 13.01.15 at 15:25, wrote: >> --- a/xen/drivers/passthrough/device_tree.c >> +++ b/xen/drivers/passthrough/device_tree.c >> @@ -41,6 +41,10 @@ int iommu_assign_dt_device(struct domain *d, struct >> dt_device_node *dev) >> if ( !list_empty(&dev->domain_list) ) >> goto fail; >> >> +rc = iommu_construct(d); >> +if ( rc ) >> +goto fail; > > Considering that the only (current) caller of this it domain_build.c I'm > afraid you're going to get into trouble if you get back -ERESTART > here. Note that on x86 Dom0 setup works via iommu_hwdom_init(), > which deals with the preemption needs (at that point in time) by > calling process_pending_softirqs() every once in a while. iommu_hwdom_init is also called for ARM (it's part of the common domain creation code). So, I don't see any issue here as we match the same behavior as PCI. FWIW, on the previous version you asked to check the need_iommu(d) in iommu_construct. For DOM0 it will return 0 and therefore never return -ERESTART. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 16/24] xen/passthrough: Introduce iommu_construct
>>> On 13.01.15 at 15:25, wrote: > --- a/xen/drivers/passthrough/device_tree.c > +++ b/xen/drivers/passthrough/device_tree.c > @@ -41,6 +41,10 @@ int iommu_assign_dt_device(struct domain *d, struct > dt_device_node *dev) > if ( !list_empty(&dev->domain_list) ) > goto fail; > > +rc = iommu_construct(d); > +if ( rc ) > +goto fail; Considering that the only (current) caller of this it domain_build.c I'm afraid you're going to get into trouble if you get back -ERESTART here. Note that on x86 Dom0 setup works via iommu_hwdom_init(), which deals with the preemption needs (at that point in time) by calling process_pending_softirqs() every once in a while. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
[Xen-devel] [PATCH v3 16/24] xen/passthrough: Introduce iommu_construct
This new function will correctly initialize the IOMMU page table for the
current domain.
Also use it in iommu_assign_dt_device even though the current IOMMU
implementation on ARM shares P2M with the processor.
Signed-off-by: Julien Grall
Cc: Jan Beulich
---
Changes in v3:
- The ASSERT in iommu_construct was redundant with the if ()
- Remove d->need_iommu = 1 in assign_device has it's already
done by iommu_construct.
- Simplify the code in the caller of iommu_construct
Changes in v2:
- Add missing Signed-off-by
- Rename iommu_buildup to iommu_construct
---
xen/drivers/passthrough/arm/iommu.c | 6 ++
xen/drivers/passthrough/device_tree.c | 4
xen/drivers/passthrough/iommu.c | 19 +++
xen/drivers/passthrough/pci.c | 15 ---
xen/include/xen/iommu.h | 2 ++
5 files changed, 35 insertions(+), 11 deletions(-)
diff --git a/xen/drivers/passthrough/arm/iommu.c
b/xen/drivers/passthrough/arm/iommu.c
index 3e9303a..5870aef 100644
--- a/xen/drivers/passthrough/arm/iommu.c
+++ b/xen/drivers/passthrough/arm/iommu.c
@@ -68,3 +68,9 @@ void arch_iommu_domain_destroy(struct domain *d)
{
iommu_dt_domain_destroy(d);
}
+
+int arch_iommu_populate_page_table(struct domain *d)
+{
+/* The IOMMU shares the p2m with the CPU */
+return -ENOSYS;
+}
diff --git a/xen/drivers/passthrough/device_tree.c
b/xen/drivers/passthrough/device_tree.c
index 377d41d..88e496e 100644
--- a/xen/drivers/passthrough/device_tree.c
+++ b/xen/drivers/passthrough/device_tree.c
@@ -41,6 +41,10 @@ int iommu_assign_dt_device(struct domain *d, struct
dt_device_node *dev)
if ( !list_empty(&dev->domain_list) )
goto fail;
+rc = iommu_construct(d);
+if ( rc )
+goto fail;
+
rc = hd->platform_ops->assign_device(d, 0, dt_to_dev(dev));
if ( rc )
diff --git a/xen/drivers/passthrough/iommu.c b/xen/drivers/passthrough/iommu.c
index cc12735..8915244 100644
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -187,6 +187,25 @@ void iommu_teardown(struct domain *d)
tasklet_schedule(&iommu_pt_cleanup_tasklet);
}
+int iommu_construct(struct domain *d)
+{
+int rc = 0;
+
+if ( need_iommu(d) > 0 )
+return 0;
+
+if ( !iommu_use_hap_pt(d) )
+{
+rc = arch_iommu_populate_page_table(d);
+if ( rc )
+return rc;
+}
+
+d->need_iommu = 1;
+
+return rc;
+}
+
void iommu_domain_destroy(struct domain *d)
{
struct hvm_iommu *hd = domain_hvm_iommu(d);
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index 43ce5dc..9a47a37 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -1355,18 +1355,11 @@ static int assign_device(struct domain *d, u16 seg, u8
bus, u8 devfn)
if ( !spin_trylock(&pcidevs_lock) )
return -ERESTART;
-if ( need_iommu(d) <= 0 )
+rc = iommu_construct(d);
+if ( rc )
{
-if ( !iommu_use_hap_pt(d) )
-{
-rc = arch_iommu_populate_page_table(d);
-if ( rc )
-{
-spin_unlock(&pcidevs_lock);
-return rc;
-}
-}
-d->need_iommu = 1;
+spin_unlock(&pcidevs_lock);
+return rc;
}
pdev = pci_get_pdev_by_domain(hardware_domain, seg, bus, devfn);
diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
index d0f99ef..c146ee4 100644
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -65,6 +65,8 @@ int arch_iommu_domain_init(struct domain *d);
int arch_iommu_populate_page_table(struct domain *d);
void arch_iommu_check_autotranslated_hwdom(struct domain *d);
+int iommu_construct(struct domain *d);
+
/* Function used internally, use iommu_domain_destroy */
void iommu_teardown(struct domain *d);
--
2.1.4
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
