Re: [Xen-devel] [PATCH v5 3/4] iommu: elide flushing for higher order map/unmap operations

2018-12-20 Thread Paul Durrant 
> -Original Message-
> From: Woods, Brian [mailto:brian.wo...@amd.com]
> Sent: 20 December 2018 19:27
> To: Paul Durrant ; xen-devel@lists.xenproject.org
> Cc: Stefano Stabellini ; Julien Grall
> ; Andrew Cooper ; George
> Dunlap ; Ian Jackson ;
> Konrad Rzeszutek Wilk ; Tim (Xen.org)
> ; Wei Liu ; Suthikulpanit, Suravee
> ; Roger Pau Monne 
> Subject: Re: [PATCH v5 3/4] iommu: elide flushing for higher order
> map/unmap operations
> 
> From: Paul Durrant 
> Sent: Monday, December 17, 2018 3:22 AM
> To: xen-devel@lists.xenproject.org
> Cc: Paul Durrant; Stefano Stabellini; Julien Grall; Andrew Cooper; George
> Dunlap; Ian Jackson; Konrad Rzeszutek Wilk; Tim Deegan; Wei Liu;
> Suthikulpanit, Suravee; Woods, Brian; Roger Pau Monné
> Subject: [PATCH v5 3/4] iommu: elide flushing for higher order map/unmap
> operations
> 
> This patch removes any implicit flushing that occurs in the implementation
> of map and unmap operations and adds new iommu_map/unmap() wrapper
> functions. To maintain semantics of the iommu_legacy_map/unmap() wrapper
> functions, these are modified to call the new wrapper functions and then
> perform an explicit flush operation.
> 
> Because VT-d currently performs two different types of flush dependent
> upon
> whether a PTE is being modified versus merely added (i.e. replacing a non-
> present PTE) 'iommu flush flags' are defined by this patch and the
> iommu_ops map_page() and unmap_page() methods are modified to OR the type
> of flush necessary for the PTE that has been populated or depopulated into
> an accumulated flags value. The accumulated value can then be passed into
> the explicit flush operation.
> 
> The ARM SMMU implementations of map_page() and unmap_page() currently
> perform no implicit flushing and therefore the modified methods do not
> adjust the flush flags.
> 
> NOTE: The per-cpu 'iommu_dont_flush_iotlb' is respected by the
>   iommu_legacy_map/unmap() wrapper functions and therefore this now
>   applies to all IOMMU implementations rather than just VT-d.
> 
> Signed-off-by: Paul Durrant 
> Reviewed-by: Jan Beulich 
> Reviewed-by: Kevin Tian 
> 
> Acked-by: Brian Woods 

Thanks Brian,

  Paul

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

Re: [Xen-devel] [PATCH v5 3/4] iommu: elide flushing for higher order map/unmap operations

2018-12-20 Thread Woods, Brian 
From: Paul Durrant 
Sent: Monday, December 17, 2018 3:22 AM
To: xen-devel@lists.xenproject.org
Cc: Paul Durrant; Stefano Stabellini; Julien Grall; Andrew Cooper; George 
Dunlap; Ian Jackson; Konrad Rzeszutek Wilk; Tim Deegan; Wei Liu; Suthikulpanit, 
Suravee; Woods, Brian; Roger Pau Monné
Subject: [PATCH v5 3/4] iommu: elide flushing for higher order map/unmap 
operations

This patch removes any implicit flushing that occurs in the implementation
of map and unmap operations and adds new iommu_map/unmap() wrapper
functions. To maintain semantics of the iommu_legacy_map/unmap() wrapper
functions, these are modified to call the new wrapper functions and then
perform an explicit flush operation.

Because VT-d currently performs two different types of flush dependent upon
whether a PTE is being modified versus merely added (i.e. replacing a non-
present PTE) 'iommu flush flags' are defined by this patch and the
iommu_ops map_page() and unmap_page() methods are modified to OR the type
of flush necessary for the PTE that has been populated or depopulated into
an accumulated flags value. The accumulated value can then be passed into
the explicit flush operation.

The ARM SMMU implementations of map_page() and unmap_page() currently
perform no implicit flushing and therefore the modified methods do not
adjust the flush flags.

NOTE: The per-cpu 'iommu_dont_flush_iotlb' is respected by the
  iommu_legacy_map/unmap() wrapper functions and therefore this now
  applies to all IOMMU implementations rather than just VT-d.

Signed-off-by: Paul Durrant 
Reviewed-by: Jan Beulich 
Reviewed-by: Kevin Tian 

Acked-by: Brian Woods 

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

Re: [Xen-devel] [PATCH v5 3/4] iommu: elide flushing for higher order map/unmap operations

2018-12-19 Thread Julien Grall 

Hi Paul,

On 17/12/2018 09:22, Paul Durrant wrote:

This patch removes any implicit flushing that occurs in the implementation
of map and unmap operations and adds new iommu_map/unmap() wrapper
functions. To maintain semantics of the iommu_legacy_map/unmap() wrapper
functions, these are modified to call the new wrapper functions and then
perform an explicit flush operation.

Because VT-d currently performs two different types of flush dependent upon
whether a PTE is being modified versus merely added (i.e. replacing a non-
present PTE) 'iommu flush flags' are defined by this patch and the
iommu_ops map_page() and unmap_page() methods are modified to OR the type
of flush necessary for the PTE that has been populated or depopulated into
an accumulated flags value. The accumulated value can then be passed into
the explicit flush operation.

The ARM SMMU implementations of map_page() and unmap_page() currently
perform no implicit flushing and therefore the modified methods do not
adjust the flush flags.

NOTE: The per-cpu 'iommu_dont_flush_iotlb' is respected by the
   iommu_legacy_map/unmap() wrapper functions and therefore this now
   applies to all IOMMU implementations rather than just VT-d.

Signed-off-by: Paul Durrant 
Reviewed-by: Jan Beulich 
Reviewed-by: Kevin Tian 


Acked-by: Julien Grall 

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [PATCH v5 3/4] iommu: elide flushing for higher order map/unmap operations

2018-12-19 Thread Paul Durrant 
Julien, ping? 

> -Original Message-
> From: Paul Durrant [mailto:paul.durr...@citrix.com]
> Sent: 17 December 2018 09:23
> To: xen-devel@lists.xenproject.org
> Cc: Paul Durrant ; Stefano Stabellini
> ; Julien Grall ; Andrew
> Cooper ; George Dunlap
> ; Ian Jackson ; Konrad
> Rzeszutek Wilk ; Tim (Xen.org) ; Wei
> Liu ; Suravee Suthikulpanit
> ; Brian Woods ; Roger
> Pau Monne 
> Subject: [PATCH v5 3/4] iommu: elide flushing for higher order map/unmap
> operations
> 
> This patch removes any implicit flushing that occurs in the implementation
> of map and unmap operations and adds new iommu_map/unmap() wrapper
> functions. To maintain semantics of the iommu_legacy_map/unmap() wrapper
> functions, these are modified to call the new wrapper functions and then
> perform an explicit flush operation.
> 
> Because VT-d currently performs two different types of flush dependent
> upon
> whether a PTE is being modified versus merely added (i.e. replacing a non-
> present PTE) 'iommu flush flags' are defined by this patch and the
> iommu_ops map_page() and unmap_page() methods are modified to OR the type
> of flush necessary for the PTE that has been populated or depopulated into
> an accumulated flags value. The accumulated value can then be passed into
> the explicit flush operation.
> 
> The ARM SMMU implementations of map_page() and unmap_page() currently
> perform no implicit flushing and therefore the modified methods do not
> adjust the flush flags.
> 
> NOTE: The per-cpu 'iommu_dont_flush_iotlb' is respected by the
>   iommu_legacy_map/unmap() wrapper functions and therefore this now
>   applies to all IOMMU implementations rather than just VT-d.
> 
> Signed-off-by: Paul Durrant 
> Reviewed-by: Jan Beulich 
> Reviewed-by: Kevin Tian 
> ---
> Cc: Stefano Stabellini 
> Cc: Julien Grall 
> Cc: Andrew Cooper 
> Cc: George Dunlap 
> Cc: Ian Jackson 
> Cc: Konrad Rzeszutek Wilk 
> Cc: Tim Deegan 
> Cc: Wei Liu 
> Cc: Suravee Suthikulpanit 
> Cc: Brian Woods 
> Cc: "Roger Pau Monné" 
> 
> v5:
>  - Fix style issues and typo picked up by Julien.
> 
> v4:
>  - Formatting fixes.
>  - Respect flush flags even on a failed map or unmap.
> 
> v3:
>  - Make AMD IOMMU and Intel VT-d map/unmap operations pass back accurate
>flush_flags.
>  - Respect 'iommu_dont_flush_iotlb' in legacy unmap wrapper.
>  - Pass flush_flags into iommu_iotlb_flush_all().
>  - Improve comments and fix style issues.
> 
> v2:
>  - Add the new iommu_map/unmap() and don't proliferate use of
>iommu_dont_flush_iotlb.
>  - Use 'flush flags' instead of a 'iommu_flush_type'
>  - Add a 'flush_flags' argument to iommu_flush() and modify the call-
> sites.
> 
> This code has only been compile tested for ARM.
> ---
>  xen/arch/arm/p2m.c| 11 +++-
>  xen/common/memory.c   |  6 +-
>  xen/drivers/passthrough/amd/iommu_map.c   | 87 ++
> -
>  xen/drivers/passthrough/arm/smmu.c| 11 +++-
>  xen/drivers/passthrough/iommu.c   | 84 --
> 
>  xen/drivers/passthrough/vtd/iommu.c   | 32 +-
>  xen/drivers/passthrough/x86/iommu.c   | 27 ++---
>  xen/include/asm-x86/hvm/svm/amd-iommu-proto.h |  9 ++-
>  xen/include/xen/iommu.h   | 44 +++---
>  9 files changed, 226 insertions(+), 85 deletions(-)
> 
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 17e2523fc1..1389515199 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -986,8 +986,17 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
> 
>  if ( need_iommu_pt_sync(p2m->domain) &&
>   (lpae_is_valid(orig_pte) || lpae_is_valid(*entry)) )
> +{
> +unsigned int flush_flags = 0;
> +
> +if ( lpae_is_valid(orig_pte) )
> +flush_flags |= IOMMU_FLUSHF_modified;
> +if ( lpae_is_valid(*entry) )
> +flush_flags |= IOMMU_FLUSHF_added;
> +
>  rc = iommu_iotlb_flush(p2m->domain, _dfn(gfn_x(sgfn)),
> -   1UL << page_order);
> +   1UL << page_order, flush_flags);
> +}
>  else
>  rc = 0;
> 
> diff --git a/xen/common/memory.c b/xen/common/memory.c
> index f37eb288d4..b6cf09585c 100644
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -853,11 +853,13 @@ int xenmem_add_to_physmap(struct domain *d, struct
> xen_add_to_physmap *xatp,
> 
>  this_cpu(iommu_dont_flush_iotlb) = 0;
> 
> -ret = iommu_flush(d, _dfn(xatp->idx - done), done);
> +ret = iommu_iotlb_flush(d, _dfn(xatp->idx - done), done,
> +IOMMU_FLUSHF_added |
> IOMMU_FLUSHF_modified);
>  if ( unlikely(ret) && rc >= 0 )
>  rc = ret;
> 
> -ret = iommu_flush(d, _dfn(xatp->gpfn - done), done);
> +ret = iommu_iotlb_flush(d, _dfn(xatp->gpfn - done), done,
> +IOMMU_FLUSHF_added |
>

[Xen-devel] [PATCH v5 3/4] iommu: elide flushing for higher order map/unmap operations

2018-12-17 Thread Paul Durrant 
This patch removes any implicit flushing that occurs in the implementation
of map and unmap operations and adds new iommu_map/unmap() wrapper
functions. To maintain semantics of the iommu_legacy_map/unmap() wrapper
functions, these are modified to call the new wrapper functions and then
perform an explicit flush operation.

Because VT-d currently performs two different types of flush dependent upon
whether a PTE is being modified versus merely added (i.e. replacing a non-
present PTE) 'iommu flush flags' are defined by this patch and the
iommu_ops map_page() and unmap_page() methods are modified to OR the type
of flush necessary for the PTE that has been populated or depopulated into
an accumulated flags value. The accumulated value can then be passed into
the explicit flush operation.

The ARM SMMU implementations of map_page() and unmap_page() currently
perform no implicit flushing and therefore the modified methods do not
adjust the flush flags.

NOTE: The per-cpu 'iommu_dont_flush_iotlb' is respected by the
  iommu_legacy_map/unmap() wrapper functions and therefore this now
  applies to all IOMMU implementations rather than just VT-d.

Signed-off-by: Paul Durrant 
Reviewed-by: Jan Beulich 
Reviewed-by: Kevin Tian 
---
Cc: Stefano Stabellini 
Cc: Julien Grall 
Cc: Andrew Cooper 
Cc: George Dunlap 
Cc: Ian Jackson 
Cc: Konrad Rzeszutek Wilk 
Cc: Tim Deegan 
Cc: Wei Liu 
Cc: Suravee Suthikulpanit 
Cc: Brian Woods 
Cc: "Roger Pau Monné" 

v5:
 - Fix style issues and typo picked up by Julien.

v4:
 - Formatting fixes.
 - Respect flush flags even on a failed map or unmap.

v3:
 - Make AMD IOMMU and Intel VT-d map/unmap operations pass back accurate
   flush_flags.
 - Respect 'iommu_dont_flush_iotlb' in legacy unmap wrapper.
 - Pass flush_flags into iommu_iotlb_flush_all().
 - Improve comments and fix style issues.

v2:
 - Add the new iommu_map/unmap() and don't proliferate use of
   iommu_dont_flush_iotlb.
 - Use 'flush flags' instead of a 'iommu_flush_type'
 - Add a 'flush_flags' argument to iommu_flush() and modify the call-sites.

This code has only been compile tested for ARM.
---
 xen/arch/arm/p2m.c| 11 +++-
 xen/common/memory.c   |  6 +-
 xen/drivers/passthrough/amd/iommu_map.c   | 87 ++-
 xen/drivers/passthrough/arm/smmu.c| 11 +++-
 xen/drivers/passthrough/iommu.c   | 84 --
 xen/drivers/passthrough/vtd/iommu.c   | 32 +-
 xen/drivers/passthrough/x86/iommu.c   | 27 ++---
 xen/include/asm-x86/hvm/svm/amd-iommu-proto.h |  9 ++-
 xen/include/xen/iommu.h   | 44 +++---
 9 files changed, 226 insertions(+), 85 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 17e2523fc1..1389515199 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -986,8 +986,17 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
 
 if ( need_iommu_pt_sync(p2m->domain) &&
  (lpae_is_valid(orig_pte) || lpae_is_valid(*entry)) )
+{
+unsigned int flush_flags = 0;
+
+if ( lpae_is_valid(orig_pte) )
+flush_flags |= IOMMU_FLUSHF_modified;
+if ( lpae_is_valid(*entry) )
+flush_flags |= IOMMU_FLUSHF_added;
+
 rc = iommu_iotlb_flush(p2m->domain, _dfn(gfn_x(sgfn)),
-   1UL << page_order);
+   1UL << page_order, flush_flags);
+}
 else
 rc = 0;
 
diff --git a/xen/common/memory.c b/xen/common/memory.c
index f37eb288d4..b6cf09585c 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -853,11 +853,13 @@ int xenmem_add_to_physmap(struct domain *d, struct 
xen_add_to_physmap *xatp,
 
 this_cpu(iommu_dont_flush_iotlb) = 0;
 
-ret = iommu_flush(d, _dfn(xatp->idx - done), done);
+ret = iommu_iotlb_flush(d, _dfn(xatp->idx - done), done,
+IOMMU_FLUSHF_added | IOMMU_FLUSHF_modified);
 if ( unlikely(ret) && rc >= 0 )
 rc = ret;
 
-ret = iommu_flush(d, _dfn(xatp->gpfn - done), done);
+ret = iommu_iotlb_flush(d, _dfn(xatp->gpfn - done), done,
+IOMMU_FLUSHF_added | IOMMU_FLUSHF_modified);
 if ( unlikely(ret) && rc >= 0 )
 rc = ret;
 }
diff --git a/xen/drivers/passthrough/amd/iommu_map.c 
b/xen/drivers/passthrough/amd/iommu_map.c
index de5a880070..21d147411e 100644
--- a/xen/drivers/passthrough/amd/iommu_map.c
+++ b/xen/drivers/passthrough/amd/iommu_map.c
@@ -35,23 +35,37 @@ static unsigned int pfn_to_pde_idx(unsigned long pfn, 
unsigned int level)
 return idx;
 }
 
-static void clear_iommu_pte_present(unsigned long l1_mfn, unsigned long dfn)
+static unsigned int clear_iommu_pte_present(unsigned long l1_mfn,
+unsigned long dfn)
 {
 uint64_t *table, *pte;
+uint32_t entry;
+unsigned int flush_flags;