Re: [Xen-devel] [PATCH for 4.6] VT-d: Create IOMMU mappings for RMRR regions if shared EPT is not being used

2015-08-27 Thread Tian, Kevin 
 From: Chen, Tiejun
 Sent: Thursday, August 27, 2015 5:05 PM
 
 On 8/27/2015 4:40 PM, Malcolm Crossley wrote:
  On 27/08/15 03:59, Chen, Tiejun wrote:
  This kind of issue is already gone.
 
  https://www.mail-archive.com/xen-devel@lists.xen.org/msg32464.html
 
  There is a bug in the code you refer to above which results in no IOMMU 
  page table
  mappings being created if the guest domain is not sharing it's EPT page 
  tables with
  the IOMMU.
 
  set_identity_p2m_entry only configures the EPT page tables and does not 
  configure
  the IOMMU page tables.
 
 Okay, I got what you mean.
 
 Instead, could you insert iommu_{map,unmap_page() into
 {set,clear}_identity_p2m_entry()? I think this can make
 {set,clear}_identity_p2m_entry approachable in all circumstances.
 
 Kevin and Jan,
 
 Is this fine?
 

Yes that is the right thing to do. It's a bit surprise to me that it's
not there yet. :-)

Thanks
Kevin

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

Re: [Xen-devel] [PATCH for 4.6] VT-d: Create IOMMU mappings for RMRR regions if shared EPT is not being used

2015-08-27 Thread Chen, Tiejun 

On 8/27/2015 4:40 PM, Malcolm Crossley wrote:

On 27/08/15 03:59, Chen, Tiejun wrote:

This kind of issue is already gone.

https://www.mail-archive.com/xen-devel@lists.xen.org/msg32464.html


There is a bug in the code you refer to above which results in no IOMMU page 
table
mappings being created if the guest domain is not sharing it's EPT page tables 
with
the IOMMU.

set_identity_p2m_entry only configures the EPT page tables and does not 
configure
the IOMMU page tables.


Okay, I got what you mean.

Instead, could you insert iommu_{map,unmap_page() into 
{set,clear}_identity_p2m_entry()? I think this can make 
{set,clear}_identity_p2m_entry approachable in all circumstances.


Kevin and Jan,

Is this fine?

Thanks
Tiejun



We had a real world regression (with xen 4.6-rc1) on a Intel Haswell system with
integrated graphics.

The patch below resolves the regression.

Malcolm



Thanks
Tiejun

On 8/26/2015 11:49 PM, Malcolm Crossley wrote:

Add RMRR 1:1 IOMMU mappings to IOMMU page tables if EPT page table are not being
shared with the IOMMU.

This is a regression in behaviour versus Xen 4.5.

Signed-off-by: Malcolm Crossley malcolm.cross...@citrix.com
---
  xen/drivers/passthrough/vtd/iommu.c | 23 ---
  1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/xen/drivers/passthrough/vtd/iommu.c 
b/xen/drivers/passthrough/vtd/iommu.c
index 836aed5..89de741 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1839,8 +1839,16 @@ static int rmrr_identity_mapping(struct domain *d, 
bool_t map,

  while ( base_pfn  end_pfn )
  {
-if (   (d, base_pfn) )
-ret = -ENXIO;
+if ( iommu_use_hap_pt(d) )
+{
+if ( clear_identity_p2m_entry(d, base_pfn) )
+ret = -ENXIO;
+}
+else
+{
+if ( intel_iommu_unmap_page(d, base_pfn) )
+ret = -ENXIO;
+}
  base_pfn++;
  }

@@ -1855,7 +1863,16 @@ static int rmrr_identity_mapping(struct domain *d, 
bool_t map,

  while ( base_pfn  end_pfn )
  {
-int err = set_identity_p2m_entry(d, base_pfn, p2m_access_rw, flag);
+int err;
+if ( iommu_use_hap_pt(d) )
+{
+err = set_identity_p2m_entry(d, base_pfn, p2m_access_rw, flag);
+}
+else
+{
+err = intel_iommu_map_page(d, base_pfn, base_pfn,
+   IOMMUF_readable|IOMMUF_writable);
+}

  if ( err )
  return err;






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

Re: [Xen-devel] [PATCH for 4.6] VT-d: Create IOMMU mappings for RMRR regions if shared EPT is not being used

2015-08-27 Thread Jan Beulich 
 On 26.08.15 at 17:49, malcolm.cross...@citrix.com wrote:
 --- a/xen/drivers/passthrough/vtd/iommu.c
 +++ b/xen/drivers/passthrough/vtd/iommu.c
 @@ -1839,8 +1839,16 @@ static int rmrr_identity_mapping(struct domain *d, 
 bool_t map,
  
  while ( base_pfn  end_pfn )
  {
 -if ( clear_identity_p2m_entry(d, base_pfn) )
 -ret = -ENXIO;
 +if ( iommu_use_hap_pt(d) )
 +{
 +if ( clear_identity_p2m_entry(d, base_pfn) )
 +ret = -ENXIO;
 +}
 +else
 +{
 +if ( intel_iommu_unmap_page(d, base_pfn) )
 +ret = -ENXIO;
 +}
  base_pfn++;
  }
  
 @@ -1855,7 +1863,16 @@ static int rmrr_identity_mapping(struct domain *d, 
 bool_t map,
  
  while ( base_pfn  end_pfn )
  {
 -int err = set_identity_p2m_entry(d, base_pfn, p2m_access_rw, flag);
 +int err;
 +if ( iommu_use_hap_pt(d) )
 +{
 +err = set_identity_p2m_entry(d, base_pfn, p2m_access_rw, flag);
 +}
 +else
 +{
 +err = intel_iommu_map_page(d, base_pfn, base_pfn,
 +   IOMMUF_readable|IOMMUF_writable);
 +}
  
  if ( err )
  return err;

Considering that {set,clear}_identity_p2m_entry are really clones of
guest_physmap_{add,remove}_page(), I think it would be more
logical to follow their model of invoking iommu_{,un}map_page()
instead of having the current (and possible future) callers take care
of this for themselves. Which of course then raises the question of
the right predicate: The guest_physmap_* functions check
!paging_mode_translate(), and the !shared-ept case for HVM/PVH
is being covered by {p2m_pt,ept}_set_entry(). Surely asymmetry
here would be at least suspicious.

And which raises a second question (to the VT-d maintainers): Why
is it that intel_iommu_unmap_page() doesn't use
iommu_use_hap_pt() just like intel_iommu_map_page() as well as
amd_iommu_unmap_page() do?

Jan


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

Re: [Xen-devel] [PATCH for 4.6] VT-d: Create IOMMU mappings for RMRR regions if shared EPT is not being used

2015-08-27 Thread Jan Beulich 
 On 27.08.15 at 11:05, tiejun.c...@intel.com wrote:
 On 8/27/2015 4:40 PM, Malcolm Crossley wrote:
 On 27/08/15 03:59, Chen, Tiejun wrote:
 This kind of issue is already gone.

 https://www.mail-archive.com/xen-devel@lists.xen.org/msg32464.html 

 There is a bug in the code you refer to above which results in no IOMMU page 
 table
 mappings being created if the guest domain is not sharing it's EPT page 
 tables with
 the IOMMU.

 set_identity_p2m_entry only configures the EPT page tables and does not 
 configure
 the IOMMU page tables.
 
 Okay, I got what you mean.
 
 Instead, could you insert iommu_{map,unmap_page() into 
 {set,clear}_identity_p2m_entry()? I think this can make 
 {set,clear}_identity_p2m_entry approachable in all circumstances.
 
 Kevin and Jan,
 
 Is this fine?

This matches what I've also just suggested in another reply.

Jan


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

Re: [Xen-devel] [PATCH for 4.6] VT-d: Create IOMMU mappings for RMRR regions if shared EPT is not being used

2015-08-27 Thread Malcolm Crossley 
On 27/08/15 03:59, Chen, Tiejun wrote:
 This kind of issue is already gone.
 
 https://www.mail-archive.com/xen-devel@lists.xen.org/msg32464.html

There is a bug in the code you refer to above which results in no IOMMU page 
table
mappings being created if the guest domain is not sharing it's EPT page tables 
with
the IOMMU.

set_identity_p2m_entry only configures the EPT page tables and does not 
configure
the IOMMU page tables.

We had a real world regression (with xen 4.6-rc1) on a Intel Haswell system with
integrated graphics.

The patch below resolves the regression.

Malcolm

 
 Thanks
 Tiejun
 
 On 8/26/2015 11:49 PM, Malcolm Crossley wrote:
 Add RMRR 1:1 IOMMU mappings to IOMMU page tables if EPT page table are not 
 being
 shared with the IOMMU.

 This is a regression in behaviour versus Xen 4.5.

 Signed-off-by: Malcolm Crossley malcolm.cross...@citrix.com
 ---
   xen/drivers/passthrough/vtd/iommu.c | 23 ---
   1 file changed, 20 insertions(+), 3 deletions(-)

 diff --git a/xen/drivers/passthrough/vtd/iommu.c 
 b/xen/drivers/passthrough/vtd/iommu.c
 index 836aed5..89de741 100644
 --- a/xen/drivers/passthrough/vtd/iommu.c
 +++ b/xen/drivers/passthrough/vtd/iommu.c
 @@ -1839,8 +1839,16 @@ static int rmrr_identity_mapping(struct domain *d, 
 bool_t map,

   while ( base_pfn  end_pfn )
   {
 -if ( clear_identity_p2m_entry(d, base_pfn) )
 -ret = -ENXIO;
 +if ( iommu_use_hap_pt(d) )
 +{
 +if ( clear_identity_p2m_entry(d, base_pfn) )
 +ret = -ENXIO;
 +}
 +else
 +{
 +if ( intel_iommu_unmap_page(d, base_pfn) )
 +ret = -ENXIO;
 +}
   base_pfn++;
   }

 @@ -1855,7 +1863,16 @@ static int rmrr_identity_mapping(struct domain *d, 
 bool_t map,

   while ( base_pfn  end_pfn )
   {
 -int err = set_identity_p2m_entry(d, base_pfn, p2m_access_rw, flag);
 +int err;
 +if ( iommu_use_hap_pt(d) )
 +{
 +err = set_identity_p2m_entry(d, base_pfn, p2m_access_rw, flag);
 +}
 +else
 +{
 +err = intel_iommu_map_page(d, base_pfn, base_pfn,
 +   IOMMUF_readable|IOMMUF_writable);
 +}

   if ( err )
   return err;



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

[Xen-devel] [PATCH for 4.6] VT-d: Create IOMMU mappings for RMRR regions if shared EPT is not being used

2015-08-26 Thread Malcolm Crossley 
Add RMRR 1:1 IOMMU mappings to IOMMU page tables if EPT page table are not being
shared with the IOMMU.

This is a regression in behaviour versus Xen 4.5.

Signed-off-by: Malcolm Crossley malcolm.cross...@citrix.com
---
 xen/drivers/passthrough/vtd/iommu.c | 23 ---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/xen/drivers/passthrough/vtd/iommu.c 
b/xen/drivers/passthrough/vtd/iommu.c
index 836aed5..89de741 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1839,8 +1839,16 @@ static int rmrr_identity_mapping(struct domain *d, 
bool_t map,
 
 while ( base_pfn  end_pfn )
 {
-if ( clear_identity_p2m_entry(d, base_pfn) )
-ret = -ENXIO;
+if ( iommu_use_hap_pt(d) )
+{
+if ( clear_identity_p2m_entry(d, base_pfn) )
+ret = -ENXIO;
+}
+else
+{
+if ( intel_iommu_unmap_page(d, base_pfn) )
+ret = -ENXIO;
+}
 base_pfn++;
 }
 
@@ -1855,7 +1863,16 @@ static int rmrr_identity_mapping(struct domain *d, 
bool_t map,
 
 while ( base_pfn  end_pfn )
 {
-int err = set_identity_p2m_entry(d, base_pfn, p2m_access_rw, flag);
+int err;
+if ( iommu_use_hap_pt(d) )
+{
+err = set_identity_p2m_entry(d, base_pfn, p2m_access_rw, flag);
+}
+else
+{
+err = intel_iommu_map_page(d, base_pfn, base_pfn,
+   IOMMUF_readable|IOMMUF_writable);
+}
 
 if ( err )
 return err;
-- 
1.7.12.4


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

Re: [Xen-devel] [PATCH for 4.6] VT-d: Create IOMMU mappings for RMRR regions if shared EPT is not being used

2015-08-26 Thread Chen, Tiejun 

This kind of issue is already gone.

https://www.mail-archive.com/xen-devel@lists.xen.org/msg32464.html

Thanks
Tiejun

On 8/26/2015 11:49 PM, Malcolm Crossley wrote:

Add RMRR 1:1 IOMMU mappings to IOMMU page tables if EPT page table are not being
shared with the IOMMU.

This is a regression in behaviour versus Xen 4.5.

Signed-off-by: Malcolm Crossley malcolm.cross...@citrix.com
---
  xen/drivers/passthrough/vtd/iommu.c | 23 ---
  1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/xen/drivers/passthrough/vtd/iommu.c 
b/xen/drivers/passthrough/vtd/iommu.c
index 836aed5..89de741 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1839,8 +1839,16 @@ static int rmrr_identity_mapping(struct domain *d, 
bool_t map,

  while ( base_pfn  end_pfn )
  {
-if ( clear_identity_p2m_entry(d, base_pfn) )
-ret = -ENXIO;
+if ( iommu_use_hap_pt(d) )
+{
+if ( clear_identity_p2m_entry(d, base_pfn) )
+ret = -ENXIO;
+}
+else
+{
+if ( intel_iommu_unmap_page(d, base_pfn) )
+ret = -ENXIO;
+}
  base_pfn++;
  }

@@ -1855,7 +1863,16 @@ static int rmrr_identity_mapping(struct domain *d, 
bool_t map,

  while ( base_pfn  end_pfn )
  {
-int err = set_identity_p2m_entry(d, base_pfn, p2m_access_rw, flag);
+int err;
+if ( iommu_use_hap_pt(d) )
+{
+err = set_identity_p2m_entry(d, base_pfn, p2m_access_rw, flag);
+}
+else
+{
+err = intel_iommu_map_page(d, base_pfn, base_pfn,
+   IOMMUF_readable|IOMMUF_writable);
+}

  if ( err )
  return err;



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