Re: [Xen-devel] [PATCH v2 02/13] iommu: Add extra order argument to the IOMMU APIs and platform callbacks

2017-08-22 Thread Oleksandr Tyshchenko 
Hi, Jan

On Tue, Aug 22, 2017 at 10:21 AM, Jan Beulich  wrote:
 On 21.08.17 at 18:20,  wrote:
>> Hi, all.
>>
>> Any comments?
>
> Excuse me, but comments on what? The quoted text below just has
> two "will add" comments of yours. I don't think you expect any
> further comments on those. As to the series as a whole - I still have
> it on my to-be-reviewed list, but there's no way I can predict when
> I would get to it.
I got it. No problem, will wait.

>
> Jan
>
>> On Thu, Aug 3, 2017 at 3:32 PM, Oleksandr Tyshchenko
>>  wrote:
>>> Hi, Julien
>>>
>>> On Thu, Aug 3, 2017 at 2:21 PM, Julien Grall  wrote:
 Hi Oleksandr,

 On 25/07/17 18:26, Oleksandr Tyshchenko wrote:
>
> diff --git a/xen/drivers/passthrough/arm/smmu.c
> b/xen/drivers/passthrough/arm/smmu.c
> index 74c09b0..7c313c0 100644
> --- a/xen/drivers/passthrough/arm/smmu.c
> +++ b/xen/drivers/passthrough/arm/smmu.c


 [...]

> +static int __must_check arm_smmu_unmap_pages(struct domain *d,
> +   unsigned long gfn, unsigned int order)
> +{
> +   unsigned long i;
> +   int rc = 0;
> +
> +   for (i = 0; i < (1UL << order); i++) {
> +   int ret = arm_smmu_unmap_page(d, gfn + i);



 Missing blank line between declaration(s) and statement(s).
>>> Will add.
>>>

> +   if (!rc)
> +   rc = ret;
> +   }
> +
> +   return rc;
> +}
> +
>  static const struct iommu_ops arm_smmu_iommu_ops = {
>  .init = arm_smmu_iommu_domain_init,
>  .hwdom_init = arm_smmu_iommu_hwdom_init,
> @@ -2786,8 +2823,8 @@ static const struct iommu_ops arm_smmu_iommu_ops = {
>  .iotlb_flush_all = arm_smmu_iotlb_flush_all,
>  .assign_device = arm_smmu_assign_dev,
>  .reassign_device = arm_smmu_reassign_dev,
> -.map_page = arm_smmu_map_page,
> -.unmap_page = arm_smmu_unmap_page,
> +.map_pages = arm_smmu_map_pages,
> +.unmap_pages = arm_smmu_unmap_pages,
>  };
>
>  static __init const struct arm_smmu_device *find_smmu(const struct device
> *dev)


 [...]

> diff --git a/xen/drivers/passthrough/vtd/iommu.c
> b/xen/drivers/passthrough/vtd/iommu.c
> index 19328f6..b4e8c89 100644
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c


 [...]

> +static int __must_check intel_iommu_unmap_pages(struct domain *d,
> +unsigned long gfn,
> +unsigned int order)
> +{
> +unsigned long i;
> +int rc = 0;
> +
> +for ( i = 0; i < (1UL << order); i++ )
> +{
> +int ret = intel_iommu_unmap_page(d, gfn + i);


 Missing blank line between declaration(s) and statement(s).
>>> Will add.
>>>

> +if ( !rc )
> +rc = ret;
> +}
> +
> +return rc;
> +}
> +


 Cheers,


 --
 Julien Grall
>>>
>>>
>>>
>>> --
>>> Regards,
>>>
>>> Oleksandr Tyshchenko
>>
>>
>>
>> --
>> Regards,
>>
>> Oleksandr Tyshchenko
>>
>> ___
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> https://lists.xen.org/xen-devel
>
>
>



-- 
Regards,

Oleksandr Tyshchenko

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 02/13] iommu: Add extra order argument to the IOMMU APIs and platform callbacks

2017-08-22 Thread Jan Beulich 
>>> On 21.08.17 at 18:20,  wrote:
> Hi, all.
> 
> Any comments?

Excuse me, but comments on what? The quoted text below just has
two "will add" comments of yours. I don't think you expect any
further comments on those. As to the series as a whole - I still have
it on my to-be-reviewed list, but there's no way I can predict when
I would get to it.

Jan

> On Thu, Aug 3, 2017 at 3:32 PM, Oleksandr Tyshchenko
>  wrote:
>> Hi, Julien
>>
>> On Thu, Aug 3, 2017 at 2:21 PM, Julien Grall  wrote:
>>> Hi Oleksandr,
>>>
>>> On 25/07/17 18:26, Oleksandr Tyshchenko wrote:

 diff --git a/xen/drivers/passthrough/arm/smmu.c
 b/xen/drivers/passthrough/arm/smmu.c
 index 74c09b0..7c313c0 100644
 --- a/xen/drivers/passthrough/arm/smmu.c
 +++ b/xen/drivers/passthrough/arm/smmu.c
>>>
>>>
>>> [...]
>>>
 +static int __must_check arm_smmu_unmap_pages(struct domain *d,
 +   unsigned long gfn, unsigned int order)
 +{
 +   unsigned long i;
 +   int rc = 0;
 +
 +   for (i = 0; i < (1UL << order); i++) {
 +   int ret = arm_smmu_unmap_page(d, gfn + i);
>>>
>>>
>>>
>>> Missing blank line between declaration(s) and statement(s).
>> Will add.
>>
>>>
 +   if (!rc)
 +   rc = ret;
 +   }
 +
 +   return rc;
 +}
 +
  static const struct iommu_ops arm_smmu_iommu_ops = {
  .init = arm_smmu_iommu_domain_init,
  .hwdom_init = arm_smmu_iommu_hwdom_init,
 @@ -2786,8 +2823,8 @@ static const struct iommu_ops arm_smmu_iommu_ops = {
  .iotlb_flush_all = arm_smmu_iotlb_flush_all,
  .assign_device = arm_smmu_assign_dev,
  .reassign_device = arm_smmu_reassign_dev,
 -.map_page = arm_smmu_map_page,
 -.unmap_page = arm_smmu_unmap_page,
 +.map_pages = arm_smmu_map_pages,
 +.unmap_pages = arm_smmu_unmap_pages,
  };

  static __init const struct arm_smmu_device *find_smmu(const struct device
 *dev)
>>>
>>>
>>> [...]
>>>
 diff --git a/xen/drivers/passthrough/vtd/iommu.c
 b/xen/drivers/passthrough/vtd/iommu.c
 index 19328f6..b4e8c89 100644
 --- a/xen/drivers/passthrough/vtd/iommu.c
 +++ b/xen/drivers/passthrough/vtd/iommu.c
>>>
>>>
>>> [...]
>>>
 +static int __must_check intel_iommu_unmap_pages(struct domain *d,
 +unsigned long gfn,
 +unsigned int order)
 +{
 +unsigned long i;
 +int rc = 0;
 +
 +for ( i = 0; i < (1UL << order); i++ )
 +{
 +int ret = intel_iommu_unmap_page(d, gfn + i);
>>>
>>>
>>> Missing blank line between declaration(s) and statement(s).
>> Will add.
>>
>>>
 +if ( !rc )
 +rc = ret;
 +}
 +
 +return rc;
 +}
 +
>>>
>>>
>>> Cheers,
>>>
>>>
>>> --
>>> Julien Grall
>>
>>
>>
>> --
>> Regards,
>>
>> Oleksandr Tyshchenko
> 
> 
> 
> -- 
> Regards,
> 
> Oleksandr Tyshchenko
> 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org 
> https://lists.xen.org/xen-devel 




___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 02/13] iommu: Add extra order argument to the IOMMU APIs and platform callbacks

2017-08-21 Thread Oleksandr Tyshchenko 
Hi, all.

Any comments?

On Thu, Aug 3, 2017 at 3:32 PM, Oleksandr Tyshchenko
 wrote:
> Hi, Julien
>
> On Thu, Aug 3, 2017 at 2:21 PM, Julien Grall  wrote:
>> Hi Oleksandr,
>>
>> On 25/07/17 18:26, Oleksandr Tyshchenko wrote:
>>>
>>> diff --git a/xen/drivers/passthrough/arm/smmu.c
>>> b/xen/drivers/passthrough/arm/smmu.c
>>> index 74c09b0..7c313c0 100644
>>> --- a/xen/drivers/passthrough/arm/smmu.c
>>> +++ b/xen/drivers/passthrough/arm/smmu.c
>>
>>
>> [...]
>>
>>> +static int __must_check arm_smmu_unmap_pages(struct domain *d,
>>> +   unsigned long gfn, unsigned int order)
>>> +{
>>> +   unsigned long i;
>>> +   int rc = 0;
>>> +
>>> +   for (i = 0; i < (1UL << order); i++) {
>>> +   int ret = arm_smmu_unmap_page(d, gfn + i);
>>
>>
>>
>> Missing blank line between declaration(s) and statement(s).
> Will add.
>
>>
>>> +   if (!rc)
>>> +   rc = ret;
>>> +   }
>>> +
>>> +   return rc;
>>> +}
>>> +
>>>  static const struct iommu_ops arm_smmu_iommu_ops = {
>>>  .init = arm_smmu_iommu_domain_init,
>>>  .hwdom_init = arm_smmu_iommu_hwdom_init,
>>> @@ -2786,8 +2823,8 @@ static const struct iommu_ops arm_smmu_iommu_ops = {
>>>  .iotlb_flush_all = arm_smmu_iotlb_flush_all,
>>>  .assign_device = arm_smmu_assign_dev,
>>>  .reassign_device = arm_smmu_reassign_dev,
>>> -.map_page = arm_smmu_map_page,
>>> -.unmap_page = arm_smmu_unmap_page,
>>> +.map_pages = arm_smmu_map_pages,
>>> +.unmap_pages = arm_smmu_unmap_pages,
>>>  };
>>>
>>>  static __init const struct arm_smmu_device *find_smmu(const struct device
>>> *dev)
>>
>>
>> [...]
>>
>>> diff --git a/xen/drivers/passthrough/vtd/iommu.c
>>> b/xen/drivers/passthrough/vtd/iommu.c
>>> index 19328f6..b4e8c89 100644
>>> --- a/xen/drivers/passthrough/vtd/iommu.c
>>> +++ b/xen/drivers/passthrough/vtd/iommu.c
>>
>>
>> [...]
>>
>>> +static int __must_check intel_iommu_unmap_pages(struct domain *d,
>>> +unsigned long gfn,
>>> +unsigned int order)
>>> +{
>>> +unsigned long i;
>>> +int rc = 0;
>>> +
>>> +for ( i = 0; i < (1UL << order); i++ )
>>> +{
>>> +int ret = intel_iommu_unmap_page(d, gfn + i);
>>
>>
>> Missing blank line between declaration(s) and statement(s).
> Will add.
>
>>
>>> +if ( !rc )
>>> +rc = ret;
>>> +}
>>> +
>>> +return rc;
>>> +}
>>> +
>>
>>
>> Cheers,
>>
>>
>> --
>> Julien Grall
>
>
>
> --
> Regards,
>
> Oleksandr Tyshchenko



-- 
Regards,

Oleksandr Tyshchenko

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 02/13] iommu: Add extra order argument to the IOMMU APIs and platform callbacks

2017-08-03 Thread Oleksandr Tyshchenko 
Hi, Julien

On Thu, Aug 3, 2017 at 2:21 PM, Julien Grall  wrote:
> Hi Oleksandr,
>
> On 25/07/17 18:26, Oleksandr Tyshchenko wrote:
>>
>> diff --git a/xen/drivers/passthrough/arm/smmu.c
>> b/xen/drivers/passthrough/arm/smmu.c
>> index 74c09b0..7c313c0 100644
>> --- a/xen/drivers/passthrough/arm/smmu.c
>> +++ b/xen/drivers/passthrough/arm/smmu.c
>
>
> [...]
>
>> +static int __must_check arm_smmu_unmap_pages(struct domain *d,
>> +   unsigned long gfn, unsigned int order)
>> +{
>> +   unsigned long i;
>> +   int rc = 0;
>> +
>> +   for (i = 0; i < (1UL << order); i++) {
>> +   int ret = arm_smmu_unmap_page(d, gfn + i);
>
>
>
> Missing blank line between declaration(s) and statement(s).
Will add.

>
>> +   if (!rc)
>> +   rc = ret;
>> +   }
>> +
>> +   return rc;
>> +}
>> +
>>  static const struct iommu_ops arm_smmu_iommu_ops = {
>>  .init = arm_smmu_iommu_domain_init,
>>  .hwdom_init = arm_smmu_iommu_hwdom_init,
>> @@ -2786,8 +2823,8 @@ static const struct iommu_ops arm_smmu_iommu_ops = {
>>  .iotlb_flush_all = arm_smmu_iotlb_flush_all,
>>  .assign_device = arm_smmu_assign_dev,
>>  .reassign_device = arm_smmu_reassign_dev,
>> -.map_page = arm_smmu_map_page,
>> -.unmap_page = arm_smmu_unmap_page,
>> +.map_pages = arm_smmu_map_pages,
>> +.unmap_pages = arm_smmu_unmap_pages,
>>  };
>>
>>  static __init const struct arm_smmu_device *find_smmu(const struct device
>> *dev)
>
>
> [...]
>
>> diff --git a/xen/drivers/passthrough/vtd/iommu.c
>> b/xen/drivers/passthrough/vtd/iommu.c
>> index 19328f6..b4e8c89 100644
>> --- a/xen/drivers/passthrough/vtd/iommu.c
>> +++ b/xen/drivers/passthrough/vtd/iommu.c
>
>
> [...]
>
>> +static int __must_check intel_iommu_unmap_pages(struct domain *d,
>> +unsigned long gfn,
>> +unsigned int order)
>> +{
>> +unsigned long i;
>> +int rc = 0;
>> +
>> +for ( i = 0; i < (1UL << order); i++ )
>> +{
>> +int ret = intel_iommu_unmap_page(d, gfn + i);
>
>
> Missing blank line between declaration(s) and statement(s).
Will add.

>
>> +if ( !rc )
>> +rc = ret;
>> +}
>> +
>> +return rc;
>> +}
>> +
>
>
> Cheers,
>
>
> --
> Julien Grall



-- 
Regards,

Oleksandr Tyshchenko

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH v2 02/13] iommu: Add extra order argument to the IOMMU APIs and platform callbacks

2017-08-03 Thread Julien Grall 

Hi Oleksandr,

On 25/07/17 18:26, Oleksandr Tyshchenko wrote:

diff --git a/xen/drivers/passthrough/arm/smmu.c 
b/xen/drivers/passthrough/arm/smmu.c
index 74c09b0..7c313c0 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c


[...]


+static int __must_check arm_smmu_unmap_pages(struct domain *d,
+   unsigned long gfn, unsigned int order)
+{
+   unsigned long i;
+   int rc = 0;
+
+   for (i = 0; i < (1UL << order); i++) {
+   int ret = arm_smmu_unmap_page(d, gfn + i);



Missing blank line between declaration(s) and statement(s).


+   if (!rc)
+   rc = ret;
+   }
+
+   return rc;
+}
+
 static const struct iommu_ops arm_smmu_iommu_ops = {
 .init = arm_smmu_iommu_domain_init,
 .hwdom_init = arm_smmu_iommu_hwdom_init,
@@ -2786,8 +2823,8 @@ static const struct iommu_ops arm_smmu_iommu_ops = {
 .iotlb_flush_all = arm_smmu_iotlb_flush_all,
 .assign_device = arm_smmu_assign_dev,
 .reassign_device = arm_smmu_reassign_dev,
-.map_page = arm_smmu_map_page,
-.unmap_page = arm_smmu_unmap_page,
+.map_pages = arm_smmu_map_pages,
+.unmap_pages = arm_smmu_unmap_pages,
 };

 static __init const struct arm_smmu_device *find_smmu(const struct device *dev)


[...]


diff --git a/xen/drivers/passthrough/vtd/iommu.c 
b/xen/drivers/passthrough/vtd/iommu.c
index 19328f6..b4e8c89 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c


[...]


+static int __must_check intel_iommu_unmap_pages(struct domain *d,
+unsigned long gfn,
+unsigned int order)
+{
+unsigned long i;
+int rc = 0;
+
+for ( i = 0; i < (1UL << order); i++ )
+{
+int ret = intel_iommu_unmap_page(d, gfn + i);


Missing blank line between declaration(s) and statement(s).


+if ( !rc )
+rc = ret;
+}
+
+return rc;
+}
+


Cheers,


--
Julien Grall

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH v2 02/13] iommu: Add extra order argument to the IOMMU APIs and platform callbacks

2017-07-25 Thread Oleksandr Tyshchenko 
From: Oleksandr Tyshchenko 

Replace existing single-page stuff (IOMMU APIs and platform callbacks)
with the multi-page one followed by modifications of all related parts.

These new map_pages/unmap_pages APIs do almost the same thing
as old map_page/unmap_page ones except the formers have extra
order argument and as the result can handle the number of pages.
So have new platform callbacks.

Although the current behavior was retained in all places (I hope),
it should be noted that the rollback logic was moved from the common code
to the IOMMU drivers. Now the IOMMU drivers are responsible for unmapping
already mapped pages if something went wrong during mapping the number
of pages (order > 0).

Signed-off-by: Oleksandr Tyshchenko 
(only for x86 and generic parts)
Reviewed-by/CC: Jan Beulich 
CC: Julien Grall 
CC: Kevin Tian 
CC: Suravee Suthikulpanit 
CC: Andrew Cooper 
CC: George Dunlap 
CC: Ian Jackson 
CC: Konrad Rzeszutek Wilk 
CC: Stefano Stabellini 
CC: Tim Deegan 
CC: Wei Liu 

---
   Changes in v1:
  - Replace existing single-page IOMMU APIs/platform callbacks with
multi-page ones instead of just keeping both variants of them.
  - Use order argument instead of page_count.
  - Clarify patch subject/description.

   Changes in v2:
  - Add maintainers in CC
---
 xen/arch/x86/mm.c | 11 +++---
 xen/arch/x86/mm/p2m-ept.c | 21 ++-
 xen/arch/x86/mm/p2m-pt.c  | 26 +++---
 xen/arch/x86/mm/p2m.c | 38 
 xen/arch/x86/x86_64/mm.c  |  5 +--
 xen/common/grant_table.c  | 10 +++---
 xen/drivers/passthrough/amd/iommu_map.c   | 50 +--
 xen/drivers/passthrough/amd/pci_amd_iommu.c   |  8 ++---
 xen/drivers/passthrough/arm/smmu.c| 41 --
 xen/drivers/passthrough/iommu.c   | 21 +--
 xen/drivers/passthrough/vtd/iommu.c   | 48 +++--
 xen/drivers/passthrough/vtd/x86/vtd.c |  4 +--
 xen/drivers/passthrough/x86/iommu.c   |  6 ++--
 xen/include/asm-x86/hvm/svm/amd-iommu-proto.h |  8 +++--
 xen/include/xen/iommu.h   | 20 ++-
 15 files changed, 196 insertions(+), 121 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 2dc7db9..33fcffe 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -2623,11 +2623,14 @@ static int __get_page_type(struct page_info *page, 
unsigned long type,
 if ( d && is_pv_domain(d) && unlikely(need_iommu(d)) )
 {
 if ( (x & PGT_type_mask) == PGT_writable_page )
-iommu_ret = iommu_unmap_page(d, mfn_to_gmfn(d, 
page_to_mfn(page)));
+iommu_ret = iommu_unmap_pages(d,
+  mfn_to_gmfn(d, 
page_to_mfn(page)),
+  0);
 else if ( type == PGT_writable_page )
-iommu_ret = iommu_map_page(d, mfn_to_gmfn(d, 
page_to_mfn(page)),
-   page_to_mfn(page),
-   IOMMUF_readable|IOMMUF_writable);
+iommu_ret = iommu_map_pages(d,
+mfn_to_gmfn(d, page_to_mfn(page)),
+page_to_mfn(page), 0,
+IOMMUF_readable|IOMMUF_writable);
 }
 }
 
diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c
index ecab56f..0ccf451 100644
--- a/xen/arch/x86/mm/p2m-ept.c
+++ b/xen/arch/x86/mm/p2m-ept.c
@@ -870,26 +870,9 @@ out:
 else
 {
 if ( iommu_flags )
-for ( i = 0; i < (1 << order); i++ )
-{
-rc = iommu_map_page(d, gfn + i, mfn_x(mfn) + i, 
iommu_flags);
-if ( unlikely(rc) )
-{
-while ( i-- )
-/* If statement to satisfy __must_check. */
-if ( iommu_unmap_page(p2m->domain, gfn + i) )
-continue;
-
-break;
-}
-}
+rc = iommu_map_pages(d, gfn, mfn_x(mfn), order, iommu_flags);
 else
-for ( i = 0; i < (1 << order); i++ )
-{
-ret = iommu_unmap_page(d, gfn + i);
-if ( !rc )
-rc = ret;
-}
+rc = iommu_unmap_pages(d, gfn, order);