[bug] 'VT-d 1G super page' feature is blocked

[bug] 'VT-d 1G super page' feature is blocked

[Ren, Yongjie]

Hi Tim,
Could you please have a look at this bug? Thanks a lot.
http://bugzilla.xensource.com/bugzilla/show_bug.cgi?id=1774

Best Regards,
     Yongjie Ren  (Jay)

Re: [bug] 'VT-d 1G super page' feature is blocked

[Tim Deegan]

Hi, 

At 14:58 +0800 on 30 Jul (1312037906), Ren, Yongjie wrote:
> Could you please have a look at this bug? Thanks a lot.
> http://bugzilla.xensource.com/bugzilla/show_bug.cgi?id=1774

The command-line option for sharing IOMMU tables has changed: it's now
"iommu=sharept" instead of "sharept=1".  Does changing the commend-line
to "iommu=1,sharept" solve your problem?

Tim.

-- 
Tim Deegan <tim@xen.org>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

RE: [bug] 'VT-d 1G super page' feature is blocked

[Ren, Yongjie]

> -----Original Message-----
> From: Tim Deegan [mailto:tim@xen.org]
> Sent: Saturday, July 30, 2011 4:12 PM
> To: Ren, Yongjie
> Cc: Xen Devel
> Subject: Re: [Xen-devel] [bug] 'VT-d 1G super page' feature is blocked
> 
> Hi,
> 
> At 14:58 +0800 on 30 Jul (1312037906), Ren, Yongjie wrote:
> > Could you please have a look at this bug? Thanks a lot.
> > http://bugzilla.xensource.com/bugzilla/show_bug.cgi?id=1774
> 
> The command-line option for sharing IOMMU tables has changed: it's now
> "iommu=sharept" instead of "sharept=1".  Does changing the commend-line
> to "iommu=1,sharept" solve your problem?
No, it doesn't work.  xl dmesg also shows "(XEN) Intel VT-d Shared EPT tables not enabled."

RE: [bug] 'VT-d 1G super page' feature is blocked

[Ren, Yongjie]

Hi Tim,
Any comment on this issue ?
It seems that cs# 23247 introduced this issue.
CC: Allen, Xin and Jan

Best Regards,
     Yongjie Ren  (Jay)

> -----Original Message-----
> From: xen-devel-bounces@lists.xensource.com
> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Ren, Yongjie
> Sent: Saturday, July 30, 2011 5:01 PM
> To: Tim Deegan
> Cc: Xen Devel
> Subject: RE: [Xen-devel] [bug] 'VT-d 1G super page' feature is blocked
> 
> > -----Original Message-----
> > From: Tim Deegan [mailto:tim@xen.org]
> > Sent: Saturday, July 30, 2011 4:12 PM
> > To: Ren, Yongjie
> > Cc: Xen Devel
> > Subject: Re: [Xen-devel] [bug] 'VT-d 1G super page' feature is blocked
> >
> > Hi,
> >
> > At 14:58 +0800 on 30 Jul (1312037906), Ren, Yongjie wrote:
> > > Could you please have a look at this bug? Thanks a lot.
> > > http://bugzilla.xensource.com/bugzilla/show_bug.cgi?id=1774
> >
> > The command-line option for sharing IOMMU tables has changed: it's now
> > "iommu=sharept" instead of "sharept=1".  Does changing the
> commend-line
> > to "iommu=1,sharept" solve your problem?
> No, it doesn't work.  xl dmesg also shows "(XEN) Intel VT-d Shared EPT tables
> not enabled."
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [bug] 'VT-d 1G super page' feature is blocked

[Tim Deegan]

At 10:08 +0800 on 03 Aug (1312366080), Ren, Yongjie wrote:
> Hi Tim,
> Any comment on this issue ?
> It seems that cs# 23247 introduced this issue.

Hi, 

Allen points out that the EPT capability variable that is checked
doesn't get initialized until VMX is set up, which right now happens
_after_ the IOMMU is configured.

Allen, is there any reason that the call to iommu_enable() shouldn't be
moved to after do_presmp_initcalls()?  If so, then I guess we should
open-code the hardware capability check in the vtD init code. 

Tim.

-- 
Tim Deegan <tim@xen.org>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

RE: [bug] 'VT-d 1G super page' feature is blocked

[Kay, Allen M]

Off hand, I don't see any reason iommu_setup() cannot be moved after do_presmp_initcalls() in __start_xen().

Theoretically, iommu_setup() just needs to be called before any device operation.  We should give it a try.

Allen

-----Original Message-----
From: Tim Deegan [mailto:tim@xen.org] 
Sent: Wednesday, August 03, 2011 10:13 AM
To: Ren, Yongjie
Cc: Xen Devel; Kay, Allen M; Li, Xin
Subject: Re: [Xen-devel] [bug] 'VT-d 1G super page' feature is blocked

At 10:08 +0800 on 03 Aug (1312366080), Ren, Yongjie wrote:
> Hi Tim,
> Any comment on this issue ?
> It seems that cs# 23247 introduced this issue.

Hi, 

Allen points out that the EPT capability variable that is checked
doesn't get initialized until VMX is set up, which right now happens
_after_ the IOMMU is configured.

Allen, is there any reason that the call to iommu_enable() shouldn't be
moved to after do_presmp_initcalls()?  If so, then I guess we should
open-code the hardware capability check in the vtD init code. 

Tim.

-- 
Tim Deegan <tim@xen.org>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

RE: [bug] 'VT-d 1G super page' feature is blocked

[Jan Beulich]

>>> On 03.08.11 at 19:58, "Kay, Allen M" <allen.m.kay@intel.com> wrote:
> Off hand, I don't see any reason iommu_setup() cannot be moved after 
> do_presmp_initcalls() in __start_xen().
> 
> Theoretically, iommu_setup() just needs to be called before any device 
> operation.  We should give it a try.

So who's going to try this out then, and submit a patch?

Jan

> Allen
> 
> -----Original Message-----
> From: Tim Deegan [mailto:tim@xen.org] 
> Sent: Wednesday, August 03, 2011 10:13 AM
> To: Ren, Yongjie
> Cc: Xen Devel; Kay, Allen M; Li, Xin
> Subject: Re: [Xen-devel] [bug] 'VT-d 1G super page' feature is blocked
> 
> At 10:08 +0800 on 03 Aug (1312366080), Ren, Yongjie wrote:
>> Hi Tim,
>> Any comment on this issue ?
>> It seems that cs# 23247 introduced this issue.
> 
> Hi, 
> 
> Allen points out that the EPT capability variable that is checked
> doesn't get initialized until VMX is set up, which right now happens
> _after_ the IOMMU is configured.
> 
> Allen, is there any reason that the call to iommu_enable() shouldn't be
> moved to after do_presmp_initcalls()?  If so, then I guess we should
> open-code the hardware capability check in the vtD init code. 
> 
> Tim.

Re: [bug] 'VT-d 1G super page' feature is blocked

[Tim Deegan]

At 08:40 +0100 on 08 Aug (1312792831), Jan Beulich wrote:
> >>> On 03.08.11 at 19:58, "Kay, Allen M" <allen.m.kay@intel.com> wrote:
> > Off hand, I don't see any reason iommu_setup() cannot be moved after 
> > do_presmp_initcalls() in __start_xen().
> > 
> > Theoretically, iommu_setup() just needs to be called before any device 
> > operation.  We should give it a try.
> 
> So who's going to try this out then, and submit a patch?

I am, once I get back to the office (probably Wednesday)

Tim.

> > Allen
> > 
> > -----Original Message-----
> > From: Tim Deegan [mailto:tim@xen.org] 
> > Sent: Wednesday, August 03, 2011 10:13 AM
> > To: Ren, Yongjie
> > Cc: Xen Devel; Kay, Allen M; Li, Xin
> > Subject: Re: [Xen-devel] [bug] 'VT-d 1G super page' feature is blocked
> > 
> > At 10:08 +0800 on 03 Aug (1312366080), Ren, Yongjie wrote:
> >> Hi Tim,
> >> Any comment on this issue ?
> >> It seems that cs# 23247 introduced this issue.
> > 
> > Hi, 
> > 
> > Allen points out that the EPT capability variable that is checked
> > doesn't get initialized until VMX is set up, which right now happens
> > _after_ the IOMMU is configured.
> > 
> > Allen, is there any reason that the call to iommu_enable() shouldn't be
> > moved to after do_presmp_initcalls()?  If so, then I guess we should
> > open-code the hardware capability check in the vtD init code. 
> > 
> > Tim.
> 
> 
> 
> 

-- 
Tim Deegan <tim@xen.org>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

Re: [bug] 'VT-d 1G super page' feature is blocked

[Tim Deegan]

[-- Attachment #1: Type: text/plain, Size: 791 bytes --]

At 16:11 +0100 on 08 Aug (1312819910), Tim Deegan wrote:
> At 08:40 +0100 on 08 Aug (1312792831), Jan Beulich wrote:
> > >>> On 03.08.11 at 19:58, "Kay, Allen M" <allen.m.kay@intel.com> wrote:
> > > Off hand, I don't see any reason iommu_setup() cannot be moved after 
> > > do_presmp_initcalls() in __start_xen().
> > > 
> > > Theoretically, iommu_setup() just needs to be called before any device 
> > > operation.  We should give it a try.
> > 
> > So who's going to try this out then, and submit a patch?
> 
> I am, once I get back to the office (probably Wednesday)

This turns out not to work.  Allen, how about the attached patch
instead?

Cheers,

Tim.

-- 
Tim Deegan <tim@xen.org>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

[-- Attachment #2: x --]
[-- Type: text/plain, Size: 922 bytes --]

diff -r 1f08b380d438 xen/drivers/passthrough/vtd/iommu.c
--- a/xen/drivers/passthrough/vtd/iommu.c	Wed Aug 10 14:43:34 2011 +0100
+++ b/xen/drivers/passthrough/vtd/iommu.c	Wed Aug 10 17:24:19 2011 +0100
@@ -1731,15 +1731,15 @@ void iommu_pte_flush(struct domain *d, u
 
 static int vtd_ept_page_compatible(struct iommu *iommu)
 {
-    u64 cap = iommu->cap;
+    u64 ept_cap, vtd_cap = iommu->cap;
 
-    if ( ept_has_2mb(cpu_has_vmx_ept_2mb) != cap_sps_2mb(cap) )
+    /* EPT is not initialised yet, so we must check the capability in
+     * the MSR explicitly rather than use cpu_has_vmx_ept_*() */
+    if ( rdmsr_safe(MSR_IA32_VMX_EPT_VPID_CAP, ept_cap) != 0 ) 
         return 0;
 
-    if ( ept_has_1gb(cpu_has_vmx_ept_1gb) != cap_sps_1gb(cap) )
-        return 0;
-
-    return 1;
+    return ( ept_has_2mb(ept_cap) == cap_sps_2mb(vtd_cap) 
+             && ept_has_1gb(ept_cap) == cap_sps_1gb(vtd_cap) );
 }
 
 /*

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [bug] 'VT-d 1G super page' feature is blocked

[Tim Deegan]

At 17:25 +0100 on 10 Aug (1312997134), Tim Deegan wrote:
> This turns out not to work.  Allen, how about the attached patch
> instead?

So this breaks on dom0 boot, but that's a symptom of a deeper illness. 
By chance, the old code (before 23247) only actually enabled EPT-sharing
when the first HVM domain tried to use it.  23247 pulled that enabling
up to boot time, and exposed the bug that PV guests aren't properly
supported.  AFAICS the code just tries to share the P2M table without
making sure there is one there to share. :(

Tim.


> diff -r 1f08b380d438 xen/drivers/passthrough/vtd/iommu.c
> --- a/xen/drivers/passthrough/vtd/iommu.c	Wed Aug 10 14:43:34 2011 +0100
> +++ b/xen/drivers/passthrough/vtd/iommu.c	Wed Aug 10 17:24:19 2011 +0100
> @@ -1731,15 +1731,15 @@ void iommu_pte_flush(struct domain *d, u
>  
>  static int vtd_ept_page_compatible(struct iommu *iommu)
>  {
> -    u64 cap = iommu->cap;
> +    u64 ept_cap, vtd_cap = iommu->cap;
>  
> -    if ( ept_has_2mb(cpu_has_vmx_ept_2mb) != cap_sps_2mb(cap) )
> +    /* EPT is not initialised yet, so we must check the capability in
> +     * the MSR explicitly rather than use cpu_has_vmx_ept_*() */
> +    if ( rdmsr_safe(MSR_IA32_VMX_EPT_VPID_CAP, ept_cap) != 0 ) 
>          return 0;
>  
> -    if ( ept_has_1gb(cpu_has_vmx_ept_1gb) != cap_sps_1gb(cap) )
> -        return 0;
> -
> -    return 1;
> +    return ( ept_has_2mb(ept_cap) == cap_sps_2mb(vtd_cap) 
> +             && ept_has_1gb(ept_cap) == cap_sps_1gb(vtd_cap) );
>  }
>  
>  /*

> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel


-- 
Tim Deegan <tim@xen.org>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

Re: [bug] 'VT-d 1G super page' feature is blocked

[Tim Deegan]

[-- Attachment #1: Type: text/plain, Size: 694 bytes --]

Hi, 

At 11:11 +0100 on 15 Aug (1313406719), Tim Deegan wrote:
> So this breaks on dom0 boot, but that's a symptom of a deeper illness. 
> By chance, the old code (before 23247) only actually enabled EPT-sharing
> when the first HVM domain tried to use it.  23247 pulled that enabling
> up to boot time, and exposed the bug that PV guests aren't properly
> supported.  AFAICS the code just tries to share the P2M table without
> making sure there is one there to share. :(

Please try the attached patch, as well as the one earlier in the
thread. 

Cheers, 

Tim.

-- 
Tim Deegan <tim@xen.org>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

[-- Attachment #2: iommu-share-check --]
[-- Type: text/plain, Size: 3875 bytes --]

IOMMU: only try to share IOMMU and HAP tables for domains with P2M.
This makes the check more precise, and brings VTd in line with AMD code.

Signed-off-by: Tim Deegan <tim@xen.org>

diff -r e856f75d327c xen/include/xen/iommu.h
--- a/xen/include/xen/iommu.h	Mon Aug 15 11:22:52 2011 +0100
+++ b/xen/include/xen/iommu.h	Mon Aug 15 11:24:18 2011 +0100
@@ -33,6 +33,9 @@ extern bool_t iommu_snoop, iommu_qinval,
 extern bool_t iommu_hap_pt_share;
 extern bool_t iommu_debug;
 
+/* Does this domain have a P2M table we can use as its IOMMU pagetable? */
+#define iommu_use_hap_pt(d) (paging_mode_translate(d) && iommu_hap_pt_share)
+
 extern struct rangeset *mmio_ro_ranges;
 
 #define domain_hvm_iommu(d)     (&d->arch.hvm_domain.hvm_iommu)
diff -r e856f75d327c xen/drivers/passthrough/amd/iommu_map.c
--- a/xen/drivers/passthrough/amd/iommu_map.c	Mon Aug 15 11:22:52 2011 +0100
+++ b/xen/drivers/passthrough/amd/iommu_map.c	Mon Aug 15 11:24:18 2011 +0100
@@ -581,7 +581,7 @@ int amd_iommu_map_page(struct domain *d,
 
     BUG_ON( !hd->root_table );
 
-    if ( iommu_hap_pt_share && is_hvm_domain(d) )
+    if ( iommu_use_hap_pt(d) )
         return 0;
 
     spin_lock(&hd->mapping_lock);
@@ -624,7 +624,7 @@ int amd_iommu_unmap_page(struct domain *
 
     BUG_ON( !hd->root_table );
 
-    if ( iommu_hap_pt_share && is_hvm_domain(d) )
+    if ( iommu_use_hap_pt(d) )
         return 0;
 
     spin_lock(&hd->mapping_lock);
@@ -723,7 +723,7 @@ void amd_iommu_share_p2m(struct domain *
 
     ASSERT( is_hvm_domain(d) && d->arch.hvm_domain.hap_enabled );
 
-    if ( !iommu_hap_pt_share )
+    if ( !iommu_use_hap_pt(d) )
         return;
 
     pgd_mfn = pagetable_get_mfn(p2m_get_pagetable(p2m_get_hostp2m(d)));
diff -r e856f75d327c xen/drivers/passthrough/amd/pci_amd_iommu.c
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c	Mon Aug 15 11:22:52 2011 +0100
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c	Mon Aug 15 11:24:18 2011 +0100
@@ -21,6 +21,7 @@
 #include <xen/sched.h>
 #include <xen/pci.h>
 #include <xen/pci_regs.h>
+#include <xen/paging.h>
 #include <asm/hvm/iommu.h>
 #include <asm/amd-iommu.h>
 #include <asm/hvm/svm/amd-iommu-proto.h>
@@ -359,7 +360,7 @@ static void deallocate_iommu_page_tables
 {
     struct hvm_iommu *hd  = domain_hvm_iommu(d);
 
-    if ( iommu_hap_pt_share )
+    if ( iommu_use_hap_pt(d) )
         return;
 
     spin_lock(&hd->mapping_lock);
diff -r e856f75d327c xen/drivers/passthrough/iommu.c
--- a/xen/drivers/passthrough/iommu.c	Mon Aug 15 11:22:52 2011 +0100
+++ b/xen/drivers/passthrough/iommu.c	Mon Aug 15 11:24:18 2011 +0100
@@ -177,7 +177,7 @@ int assign_device(struct domain *d, u8 b
     if ( has_arch_pdevs(d) && !need_iommu(d) )
     {
         d->need_iommu = 1;
-        if ( !iommu_hap_pt_share )
+        if ( !iommu_use_hap_pt(d) )
             rc = iommu_populate_page_table(d);
         goto done;
     }
diff -r e856f75d327c xen/drivers/passthrough/vtd/iommu.c
--- a/xen/drivers/passthrough/vtd/iommu.c	Mon Aug 15 11:22:52 2011 +0100
+++ b/xen/drivers/passthrough/vtd/iommu.c	Mon Aug 15 11:24:18 2011 +0100
@@ -1613,7 +1613,7 @@ void iommu_domain_teardown(struct domain
     if ( list_empty(&acpi_drhd_units) )
         return;
 
-    if ( iommu_hap_pt_share )
+    if ( iommu_use_hap_pt(d) )
         return;
 
     spin_lock(&hd->mapping_lock);
@@ -1635,7 +1635,7 @@ static int intel_iommu_map_page(
     int iommu_domid;
 
     /* Do nothing if VT-d shares EPT page table */
-    if ( iommu_hap_pt_share )
+    if ( iommu_use_hap_pt(d) )
         return 0;
 
     /* do nothing if dom0 and iommu supports pass thru */
@@ -1760,7 +1760,7 @@ void iommu_set_pgd(struct domain *d)
 
     ASSERT( is_hvm_domain(d) && d->arch.hvm_domain.hap_enabled );
 
-    if ( !iommu_hap_pt_share )
+    if ( !iommu_use_hap_pt(d) )
         return;
 
     pgd_mfn = pagetable_get_mfn(p2m_get_pagetable(p2m_get_hostp2m(d)));

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

RE: [bug] 'VT-d 1G super page' feature is blocked

[Kay, Allen M]

> #define iommu_use_hap_pt(d) (paging_mode_translate(d) && iommu_hap_pt_share)

Tim, should the code be checking for paging_mode_hap(d) instead to make sure HAP page table is used?


-----Original Message-----
From: Tim Deegan [mailto:tim@xen.org] 
Sent: Monday, August 15, 2011 3:41 AM
To: Ren, Yongjie; Xen Devel; Jan Beulich; Kay, Allen M; Li, Xin
Subject: Re: [Xen-devel] [bug] 'VT-d 1G super page' feature is blocked

Hi, 

At 11:11 +0100 on 15 Aug (1313406719), Tim Deegan wrote:
> So this breaks on dom0 boot, but that's a symptom of a deeper illness. 
> By chance, the old code (before 23247) only actually enabled 
> EPT-sharing when the first HVM domain tried to use it.  23247 pulled 
> that enabling up to boot time, and exposed the bug that PV guests 
> aren't properly supported.  AFAICS the code just tries to share the 
> P2M table without making sure there is one there to share. :(

Please try the attached patch, as well as the one earlier in the thread. 

Cheers, 

Tim.

--
Tim Deegan <tim@xen.org>
Principal Software Engineer, Xen Platform Team Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

Re: [bug] 'VT-d 1G super page' feature is blocked

[Tim Deegan]

[-- Attachment #1: Type: text/plain, Size: 518 bytes --]

At 15:37 -0700 on 15 Aug (1313422621), Kay, Allen M wrote:
> > #define iommu_use_hap_pt(d) (paging_mode_translate(d) && iommu_hap_pt_share)
> 
> Tim, should the code be checking for paging_mode_hap(d) instead to
> make sure HAP page table is used?

Ah, yes it should; thanks for spotting that.  For AMD _external() is
sufficient but not for VtD.  Updated version attached. 

Cheers,

Tim

-- 
Tim Deegan <tim@xen.org>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

[-- Attachment #2: iommu-share-check --]
[-- Type: text/plain, Size: 3869 bytes --]

IOMMU: only try to share IOMMU and HAP tables for domains with P2M.
This makes the check more precise, and brings VTd in line with AMD code.

Signed-off-by: Tim Deegan <tim@xen.org>

diff -r e856f75d327c xen/include/xen/iommu.h
--- a/xen/include/xen/iommu.h	Mon Aug 15 11:22:52 2011 +0100
+++ b/xen/include/xen/iommu.h	Mon Aug 15 11:24:18 2011 +0100
@@ -33,6 +33,9 @@ extern bool_t iommu_snoop, iommu_qinval,
 extern bool_t iommu_hap_pt_share;
 extern bool_t iommu_debug;
 
+/* Does this domain have a P2M table we can use as its IOMMU pagetable? */
+#define iommu_use_hap_pt(d) (paging_mode_hap(d) && iommu_hap_pt_share)
+
 extern struct rangeset *mmio_ro_ranges;
 
 #define domain_hvm_iommu(d)     (&d->arch.hvm_domain.hvm_iommu)
diff -r e856f75d327c xen/drivers/passthrough/amd/iommu_map.c
--- a/xen/drivers/passthrough/amd/iommu_map.c	Mon Aug 15 11:22:52 2011 +0100
+++ b/xen/drivers/passthrough/amd/iommu_map.c	Mon Aug 15 11:24:18 2011 +0100
@@ -581,7 +581,7 @@ int amd_iommu_map_page(struct domain *d,
 
     BUG_ON( !hd->root_table );
 
-    if ( iommu_hap_pt_share && is_hvm_domain(d) )
+    if ( iommu_use_hap_pt(d) )
         return 0;
 
     spin_lock(&hd->mapping_lock);
@@ -624,7 +624,7 @@ int amd_iommu_unmap_page(struct domain *
 
     BUG_ON( !hd->root_table );
 
-    if ( iommu_hap_pt_share && is_hvm_domain(d) )
+    if ( iommu_use_hap_pt(d) )
         return 0;
 
     spin_lock(&hd->mapping_lock);
@@ -723,7 +723,7 @@ void amd_iommu_share_p2m(struct domain *
 
     ASSERT( is_hvm_domain(d) && d->arch.hvm_domain.hap_enabled );
 
-    if ( !iommu_hap_pt_share )
+    if ( !iommu_use_hap_pt(d) )
         return;
 
     pgd_mfn = pagetable_get_mfn(p2m_get_pagetable(p2m_get_hostp2m(d)));
diff -r e856f75d327c xen/drivers/passthrough/amd/pci_amd_iommu.c
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c	Mon Aug 15 11:22:52 2011 +0100
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c	Mon Aug 15 11:24:18 2011 +0100
@@ -21,6 +21,7 @@
 #include <xen/sched.h>
 #include <xen/pci.h>
 #include <xen/pci_regs.h>
+#include <xen/paging.h>
 #include <asm/hvm/iommu.h>
 #include <asm/amd-iommu.h>
 #include <asm/hvm/svm/amd-iommu-proto.h>
@@ -359,7 +360,7 @@ static void deallocate_iommu_page_tables
 {
     struct hvm_iommu *hd  = domain_hvm_iommu(d);
 
-    if ( iommu_hap_pt_share )
+    if ( iommu_use_hap_pt(d) )
         return;
 
     spin_lock(&hd->mapping_lock);
diff -r e856f75d327c xen/drivers/passthrough/iommu.c
--- a/xen/drivers/passthrough/iommu.c	Mon Aug 15 11:22:52 2011 +0100
+++ b/xen/drivers/passthrough/iommu.c	Mon Aug 15 11:24:18 2011 +0100
@@ -177,7 +177,7 @@ int assign_device(struct domain *d, u8 b
     if ( has_arch_pdevs(d) && !need_iommu(d) )
     {
         d->need_iommu = 1;
-        if ( !iommu_hap_pt_share )
+        if ( !iommu_use_hap_pt(d) )
             rc = iommu_populate_page_table(d);
         goto done;
     }
diff -r e856f75d327c xen/drivers/passthrough/vtd/iommu.c
--- a/xen/drivers/passthrough/vtd/iommu.c	Mon Aug 15 11:22:52 2011 +0100
+++ b/xen/drivers/passthrough/vtd/iommu.c	Mon Aug 15 11:24:18 2011 +0100
@@ -1613,7 +1613,7 @@ void iommu_domain_teardown(struct domain
     if ( list_empty(&acpi_drhd_units) )
         return;
 
-    if ( iommu_hap_pt_share )
+    if ( iommu_use_hap_pt(d) )
         return;
 
     spin_lock(&hd->mapping_lock);
@@ -1635,7 +1635,7 @@ static int intel_iommu_map_page(
     int iommu_domid;
 
     /* Do nothing if VT-d shares EPT page table */
-    if ( iommu_hap_pt_share )
+    if ( iommu_use_hap_pt(d) )
         return 0;
 
     /* do nothing if dom0 and iommu supports pass thru */
@@ -1760,7 +1760,7 @@ void iommu_set_pgd(struct domain *d)
 
     ASSERT( is_hvm_domain(d) && d->arch.hvm_domain.hap_enabled );
 
-    if ( !iommu_hap_pt_share )
+    if ( !iommu_use_hap_pt(d) )
         return;
 
     pgd_mfn = pagetable_get_mfn(p2m_get_pagetable(p2m_get_hostp2m(d)));

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

RE: [bug] 'VT-d 1G super page' feature is blocked

[Kay, Allen M]

Tim,

This patch at least fixed the dom0 boot issue on my jaketown system when sharept is used.  I'm still having some issue in the guest which might not be sharept related.  I need to verify it further tomorrow as I need to leave early today.

When you get a chance, can you also make a similar patch for xen-4.1-testing?  Jan need it for backporting to SLES11.

Allen

-----Original Message-----
From: Tim Deegan [mailto:tim@xen.org] 
Sent: Tuesday, August 16, 2011 2:13 AM
To: Kay, Allen M
Cc: Ren, Yongjie; Xen Devel; Jan Beulich; Li, Xin
Subject: Re: [Xen-devel] [bug] 'VT-d 1G super page' feature is blocked

At 15:37 -0700 on 15 Aug (1313422621), Kay, Allen M wrote:
> > #define iommu_use_hap_pt(d) (paging_mode_translate(d) && 
> > iommu_hap_pt_share)
> 
> Tim, should the code be checking for paging_mode_hap(d) instead to 
> make sure HAP page table is used?

Ah, yes it should; thanks for spotting that.  For AMD _external() is sufficient but not for VtD.  Updated version attached. 

Cheers,

Tim

--
Tim Deegan <tim@xen.org>
Principal Software Engineer, Xen Platform Team Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

[PATCH][VTD] fixing vt-d/ept page table sharing in xen-4.1

[Kay, Allen M]

[-- Attachment #1: Type: text/plain, Size: 792 bytes --]

Tim/Keir,

Attached is a patch for fixing VT-d/EPT sharing issue in xen-4.1-testing.  It is base on Tim's patch with the following changes:

1) moving "d->arch.paging.mode = mode | PG_HAP_enable" before calling p2m_alloc_table().  Otherwise, the call to iommu_use_hap_pt() in iommu_set_pgd() will not return TRUE.

2) added following to passthrough/iommu.c so that now uses "iommu=sharept" to turn on this feature.  I have removed the old "sharpt=1" boot option.

+        else if ( !strcmp(s, "sharept") )
+            iommu_hap_pt_share = 1;

If there is no problem with change in (1), then this patch can be incorporated into the xen 4.1.2 release.

Here is the sign off if needed:

Signed-off-by: Tim Deegan <tim@xen.org>
Signed-off-by: Allen Kay <allen.m.kay@intel.com>

[-- Attachment #2: share0819.patch --]
[-- Type: application/octet-stream, Size: 3808 bytes --]

diff -r 98c98daab56a xen/arch/x86/mm/hap/hap.c
--- a/xen/arch/x86/mm/hap/hap.c	Tue Aug 16 15:21:46 2011 +0100
+++ b/xen/arch/x86/mm/hap/hap.c	Fri Aug 19 18:28:23 2011 -0700
@@ -612,6 +612,9 @@ int hap_enable(struct domain *d, u32 mod
     d->arch.paging.alloc_page = hap_alloc_p2m_page;
     d->arch.paging.free_page = hap_free_p2m_page;
 
+    /* Now let other users see the new mode */
+    d->arch.paging.mode = mode | PG_HAP_enable;
+
     /* allocate P2m table */
     if ( mode & PG_translate )
     {
@@ -620,9 +623,6 @@ int hap_enable(struct domain *d, u32 mod
             goto out;
     }
 
-    /* Now let other users see the new mode */
-    d->arch.paging.mode = mode | PG_HAP_enable;
-
  out:
     domain_unpause(d);
     return rv;
diff -r 98c98daab56a xen/drivers/passthrough/iommu.c
--- a/xen/drivers/passthrough/iommu.c	Tue Aug 16 15:21:46 2011 +0100
+++ b/xen/drivers/passthrough/iommu.c	Fri Aug 19 18:28:23 2011 -0700
@@ -82,6 +82,8 @@ static void __init parse_iommu_param(cha
             iommu_passthrough = 1;
         else if ( !strcmp(s, "dom0-strict") )
             iommu_dom0_strict = 1;
+        else if ( !strcmp(s, "sharept") )
+            iommu_hap_pt_share = 1;
 
         s = ss + 1;
     } while ( ss );
@@ -175,7 +177,7 @@ int assign_device(struct domain *d, u8 b
     if ( has_arch_pdevs(d) && !need_iommu(d) )
     {
         d->need_iommu = 1;
-        if ( !iommu_hap_pt_share )
+        if ( !iommu_use_hap_pt(d) )
             rc = iommu_populate_page_table(d);
         goto done;
     }
diff -r 98c98daab56a xen/drivers/passthrough/vtd/iommu.c
--- a/xen/drivers/passthrough/vtd/iommu.c	Tue Aug 16 15:21:46 2011 +0100
+++ b/xen/drivers/passthrough/vtd/iommu.c	Fri Aug 19 18:28:23 2011 -0700
@@ -1622,7 +1622,7 @@ void iommu_domain_teardown(struct domain
     if ( list_empty(&acpi_drhd_units) )
         return;
 
-    if ( iommu_hap_pt_share )
+    if ( iommu_use_hap_pt(d) )
         return;
 
     spin_lock(&hd->mapping_lock);
@@ -1644,7 +1644,7 @@ static int intel_iommu_map_page(
     int iommu_domid;
 
     /* Do nothing if VT-d shares EPT page table */
-    if ( iommu_hap_pt_share )
+    if ( iommu_use_hap_pt(d) )
         return 0;
 
     /* do nothing if dom0 and iommu supports pass thru */
@@ -1748,15 +1748,15 @@ void iommu_pte_flush(struct domain *d, u
 
 static int vtd_ept_page_compatible(struct iommu *iommu)
 {
-    u64 cap = iommu->cap;
+    u64 ept_cap, vtd_cap = iommu->cap;
 
-    if ( ept_has_2mb(cpu_has_vmx_ept_2mb) != cap_sps_2mb(cap) )
+    /* EPT is not initialised yet, so we must check the capability in
+     * the MSR explicitly rather than use cpu_has_vmx_ept_*() */
+    if ( rdmsr_safe(MSR_IA32_VMX_EPT_VPID_CAP, ept_cap) != 0 ) 
         return 0;
 
-    if ( ept_has_1gb(cpu_has_vmx_ept_1gb) != cap_sps_1gb(cap) )
-        return 0;
-
-    return 1;
+    return ( ept_has_2mb(ept_cap) == cap_sps_2mb(vtd_cap) 
+             && ept_has_1gb(ept_cap) == cap_sps_1gb(vtd_cap) );
 }
 
 /*
@@ -1769,7 +1769,7 @@ void iommu_set_pgd(struct domain *d)
 
     ASSERT( is_hvm_domain(d) && d->arch.hvm_domain.hap_enabled );
 
-    if ( !iommu_hap_pt_share )
+    if ( !iommu_use_hap_pt(d) )
         return;
 
     pgd_mfn = pagetable_get_mfn(p2m_get_pagetable(p2m_get_hostp2m(d)));
diff -r 98c98daab56a xen/include/xen/iommu.h
--- a/xen/include/xen/iommu.h	Tue Aug 16 15:21:46 2011 +0100
+++ b/xen/include/xen/iommu.h	Fri Aug 19 18:28:23 2011 -0700
@@ -34,6 +34,9 @@ extern bool_t iommu_hap_pt_share;
 extern bool_t amd_iommu_debug;
 extern bool_t amd_iommu_perdev_intremap;
 
+/* Does this domain have a P2M table we can use as its IOMMU pagetable? */
+#define iommu_use_hap_pt(d) (paging_mode_hap(d) && iommu_hap_pt_share)
+
 extern struct rangeset *mmio_ro_ranges;
 
 #define domain_hvm_iommu(d)     (&d->arch.hvm_domain.hvm_iommu)

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH][VTD] fixing vt-d/ept page table sharing in xen-4.1

[Jan Beulich]

>>> On 20.08.11 at 03:47, "Kay, Allen M" <allen.m.kay@intel.com> wrote:
> Tim/Keir,
> 
> Attached is a patch for fixing VT-d/EPT sharing issue in xen-4.1-testing.  It 
> is base on Tim's patch with the following changes:
> 
> 1) moving "d->arch.paging.mode = mode | PG_HAP_enable" before calling 
> p2m_alloc_table().  Otherwise, the call to iommu_use_hap_pt() in 
> iommu_set_pgd() will not return TRUE.

Wouldn't you need to revert this change in the error paths that sit
between the new and old places of where this happens/happened?

And is it certain that setting this earlier won't confuse other code?

Jan

RE: [PATCH][VTD] fixing vt-d/ept page table sharing in xen-4.1

[Kay, Allen M]

Probably the right thing to do is set this flag in p2m_alloc_table(), after calling pagetabel_from_mfn() and before calling iommu_share_p2m_table().   However, this will require passing the domain pointer to p2m_alloc_table().

What do you think Tim?

Allen

-----Original Message-----
From: Jan Beulich [mailto:JBeulich@novell.com] 
Sent: Monday, August 22, 2011 12:39 AM
To: Kay, Allen M
Cc: Li, Xin; Hao, Xudong; Ren, Yongjie; Xen Devel; keir@xen.org; Tim Deegan
Subject: Re: [PATCH][VTD] fixing vt-d/ept page table sharing in xen-4.1

>>> On 20.08.11 at 03:47, "Kay, Allen M" <allen.m.kay@intel.com> wrote:
> Tim/Keir,
> 
> Attached is a patch for fixing VT-d/EPT sharing issue in xen-4.1-testing.  It 
> is base on Tim's patch with the following changes:
> 
> 1) moving "d->arch.paging.mode = mode | PG_HAP_enable" before calling 
> p2m_alloc_table().  Otherwise, the call to iommu_use_hap_pt() in 
> iommu_set_pgd() will not return TRUE.

Wouldn't you need to revert this change in the error paths that sit
between the new and old places of where this happens/happened?

And is it certain that setting this earlier won't confuse other code?

Jan

Re: [PATCH][VTD] fixing vt-d/ept page table sharing in xen-4.1

[Tim Deegan]

At 18:47 -0700 on 19 Aug (1313779670), Kay, Allen M wrote:
> 1) moving "d->arch.paging.mode = mode | PG_HAP_enable" before calling
> p2m_alloc_table().  Otherwise, the call to iommu_use_hap_pt() in
> iommu_set_pgd() will not return TRUE.

Ah.  This problem is there in the xen-unstable patches (which I just
pushed) as well, isn't it?  Oops.  The correct answer is to use
hap_enabled(d) in the test (see 22924:86000076dcee).  Third time lucky? :)

--- 

Passthrough: fix iommu_use_hap_pt() to use hap_enabled()

In line with 22924:86000076dcee, paging_mode_hap(d) shouldn't be 
used in HAP internals that are called during HAP setup. 

Signed-off-by: Tim Deegan <tim@xen.org>

diff -r 9d2a8912597d -r e511a9e3c68d xen/include/xen/iommu.h
--- a/xen/include/xen/iommu.h	Tue Aug 23 10:43:25 2011 +0100
+++ b/xen/include/xen/iommu.h	Tue Aug 23 10:52:37 2011 +0100
@@ -35,7 +35,7 @@ extern bool_t iommu_debug;
 extern bool_t amd_iommu_perdev_intremap;
 
 /* Does this domain have a P2M table we can use as its IOMMU pagetable? */
-#define iommu_use_hap_pt(d) (paging_mode_hap(d) && iommu_hap_pt_share)
+#define iommu_use_hap_pt(d) (hap_enabled(d) && iommu_hap_pt_share)
 
 extern struct rangeset *mmio_ro_ranges;

RE: [PATCH][VTD] fixing vt-d/ept page table sharing in xen-4.1

[Kay, Allen M]

[-- Attachment #1: Type: text/plain, Size: 1897 bytes --]

This patch works.  Can you also back port it to xen-4.1-test?  Given this feature is pretty mature now, we should turn it on by default to minimize future regressions.

---

Attached patch turns on EPT/VT-d page table sharing by default.  Iommu parameter "no-sharept" turns off sharing.

Signed-off-by: Allen Kay <allen.m.kay@intel.com>

-----Original Message-----
From: Tim Deegan [mailto:tim@xen.org] 
Sent: Tuesday, August 23, 2011 2:59 AM
To: Kay, Allen M
Cc: keir@xen.org; Ren, Yongjie; xen-devel@lists.xensource.com; Hao, Xudong; Li, Xin; Jan Beulich
Subject: Re: [Xen-devel] [PATCH][VTD] fixing vt-d/ept page table sharing in xen-4.1

At 18:47 -0700 on 19 Aug (1313779670), Kay, Allen M wrote:
> 1) moving "d->arch.paging.mode = mode | PG_HAP_enable" before calling
> p2m_alloc_table().  Otherwise, the call to iommu_use_hap_pt() in
> iommu_set_pgd() will not return TRUE.

Ah.  This problem is there in the xen-unstable patches (which I just
pushed) as well, isn't it?  Oops.  The correct answer is to use
hap_enabled(d) in the test (see 22924:86000076dcee).  Third time lucky? :)

--- 

Passthrough: fix iommu_use_hap_pt() to use hap_enabled()

In line with 22924:86000076dcee, paging_mode_hap(d) shouldn't be 
used in HAP internals that are called during HAP setup. 

Signed-off-by: Tim Deegan <tim@xen.org>

diff -r 9d2a8912597d -r e511a9e3c68d xen/include/xen/iommu.h
--- a/xen/include/xen/iommu.h	Tue Aug 23 10:43:25 2011 +0100
+++ b/xen/include/xen/iommu.h	Tue Aug 23 10:52:37 2011 +0100
@@ -35,7 +35,7 @@ extern bool_t iommu_debug;
 extern bool_t amd_iommu_perdev_intremap;
 
 /* Does this domain have a P2M table we can use as its IOMMU pagetable? */
-#define iommu_use_hap_pt(d) (paging_mode_hap(d) && iommu_hap_pt_share)
+#define iommu_use_hap_pt(d) (hap_enabled(d) && iommu_hap_pt_share)
 
 extern struct rangeset *mmio_ro_ranges;
 

[-- Attachment #2: share0823.patch --]
[-- Type: application/octet-stream, Size: 1042 bytes --]

diff -r 1b77cf8305df xen/drivers/passthrough/iommu.c
--- a/xen/drivers/passthrough/iommu.c	Tue Aug 23 10:54:27 2011 +0100
+++ b/xen/drivers/passthrough/iommu.c	Tue Aug 23 10:45:24 2011 -0700
@@ -47,7 +47,7 @@ bool_t __read_mostly iommu_passthrough;
 bool_t __read_mostly iommu_snoop = 1;
 bool_t __read_mostly iommu_qinval = 1;
 bool_t __read_mostly iommu_intremap = 1;
-bool_t __read_mostly iommu_hap_pt_share;
+bool_t __read_mostly iommu_hap_pt_share = 1;
 bool_t __read_mostly iommu_debug;
 bool_t __read_mostly iommu_amd_perdev_vector_map = 1;
 bool_t __read_mostly amd_iommu_perdev_intremap;
@@ -83,8 +83,8 @@ static void __init parse_iommu_param(cha
             iommu_passthrough = 1;
         else if ( !strcmp(s, "dom0-strict") )
             iommu_dom0_strict = 1;
-        else if ( !strcmp(s, "sharept") )
-            iommu_hap_pt_share = 1;
+        else if ( !strcmp(s, "no-sharept") )
+            iommu_hap_pt_share = 0;
         else if ( !strcmp(s, "no-perdev-vector-map") )
             iommu_amd_perdev_vector_map = 0;
 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH][VTD] fixing vt-d/ept page table sharing in xen-4.1

[Tim Deegan]

[-- Attachment #1: Type: text/plain, Size: 595 bytes --]

Hi, 

At 10:40 -0700 on 23 Aug (1314096056), Kay, Allen M wrote:
> This patch works.  Can you also back port it to xen-4.1-test?

Sure.  Try the attached series of patches on 4.1. 

> Given this feature is pretty mature now,

Eh, given that until last week it broke PV guests, I'm not so sure. :)  

> we should turn it on by default to minimize future regressions.

I'll turn it on by default for xen-unstable but I think 4.1 should stay
as it is. 

Cheers,

Tim.

-- 
Tim Deegan <tim@xen.org>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

[-- Attachment #2: 23787 --]
[-- Type: text/plain, Size: 1386 bytes --]

# HG changeset patch
# User Tim Deegan <tim@xen.org>
# Date 1314092600 -3600
# Node ID 41f00cf6b82205cab03a583356c6a64855539726
# Parent  3a05da2dc7c0a5fc0fcfc40c535d1fcb71203625
VT-d: Explicitly test EPT capabilities during IOMMU init
because the cached version isn't set up until the EPT init happens.

Signed-off-by: Tim Deegan <tim@xen.org>
xen-unstable changeset:   23787:41f00cf6b822
xen-unstable date:        Tue Aug 23 10:43:20 2011 +0100

diff -r 3a05da2dc7c0 -r 41f00cf6b822 xen/drivers/passthrough/vtd/iommu.c
--- a/xen/drivers/passthrough/vtd/iommu.c	Mon Aug 22 16:15:33 2011 +0100
+++ b/xen/drivers/passthrough/vtd/iommu.c	Tue Aug 23 10:43:20 2011 +0100
@@ -1739,15 +1739,15 @@ void iommu_pte_flush(struct domain *d, u
 
 static int vtd_ept_page_compatible(struct iommu *iommu)
 {
-    u64 cap = iommu->cap;
+    u64 ept_cap, vtd_cap = iommu->cap;
 
-    if ( ept_has_2mb(cpu_has_vmx_ept_2mb) != cap_sps_2mb(cap) )
+    /* EPT is not initialised yet, so we must check the capability in
+     * the MSR explicitly rather than use cpu_has_vmx_ept_*() */
+    if ( rdmsr_safe(MSR_IA32_VMX_EPT_VPID_CAP, ept_cap) != 0 ) 
         return 0;
 
-    if ( ept_has_1gb(cpu_has_vmx_ept_1gb) != cap_sps_1gb(cap) )
-        return 0;
-
-    return 1;
+    return ( ept_has_2mb(ept_cap) == cap_sps_2mb(vtd_cap) 
+             && ept_has_1gb(ept_cap) == cap_sps_1gb(vtd_cap) );
 }
 
 /*

[-- Attachment #3: 23788 --]
[-- Type: text/plain, Size: 2503 bytes --]

# HG changeset patch
# User Tim Deegan <tim@xen.org>
# Date 1314092605 -3600
# Node ID 9d2a8912597dba29e57a3e4db0a98f1f6c73168d
# Parent c7702b73f3823512864a39380799bf643f0312bf
IOMMU: only try to share IOMMU and HAP tables for domains with P2M.
This makes the check more precise, and brings VTd in line with AMD code.

Signed-off-by: Tim Deegan <tim@xen.org>
xen-unstable changeset:   23788:9d2a8912597d
xen-unstable date:        Tue Aug 23 10:43:25 2011 +0100

diff -r c7702b73f382 xen/drivers/passthrough/iommu.c
--- a/xen/drivers/passthrough/iommu.c	Wed Aug 24 09:34:54 2011 +0100
+++ b/xen/drivers/passthrough/iommu.c	Thu Aug 25 10:34:16 2011 +0100
@@ -175,7 +175,7 @@ int assign_device(struct domain *d, u8 b
     if ( has_arch_pdevs(d) && !need_iommu(d) )
     {
         d->need_iommu = 1;
-        if ( !iommu_hap_pt_share )
+        if ( !iommu_use_hap_pt(d) )
             rc = iommu_populate_page_table(d);
         goto done;
     }
diff -r c7702b73f382 xen/drivers/passthrough/vtd/iommu.c
--- a/xen/drivers/passthrough/vtd/iommu.c	Wed Aug 24 09:34:54 2011 +0100
+++ b/xen/drivers/passthrough/vtd/iommu.c	Thu Aug 25 10:34:16 2011 +0100
@@ -1622,7 +1622,7 @@ void iommu_domain_teardown(struct domain
     if ( list_empty(&acpi_drhd_units) )
         return;
 
-    if ( iommu_hap_pt_share )
+    if ( iommu_use_hap_pt(d) )
         return;
 
     spin_lock(&hd->mapping_lock);
@@ -1644,7 +1644,7 @@ static int intel_iommu_map_page(
     int iommu_domid;
 
     /* Do nothing if VT-d shares EPT page table */
-    if ( iommu_hap_pt_share )
+    if ( iommu_use_hap_pt(d) )
         return 0;
 
     /* do nothing if dom0 and iommu supports pass thru */
@@ -1769,7 +1769,7 @@ void iommu_set_pgd(struct domain *d)
 
     ASSERT( is_hvm_domain(d) && d->arch.hvm_domain.hap_enabled );
 
-    if ( !iommu_hap_pt_share )
+    if ( !iommu_use_hap_pt(d) )
         return;
 
     pgd_mfn = pagetable_get_mfn(p2m_get_pagetable(p2m_get_hostp2m(d)));
diff -r c7702b73f382 xen/include/xen/iommu.h
--- a/xen/include/xen/iommu.h	Wed Aug 24 09:34:54 2011 +0100
+++ b/xen/include/xen/iommu.h	Thu Aug 25 10:34:16 2011 +0100
@@ -34,6 +34,9 @@ extern bool_t iommu_hap_pt_share;
 extern bool_t amd_iommu_debug;
 extern bool_t amd_iommu_perdev_intremap;
 
+/* Does this domain have a P2M table we can use as its IOMMU pagetable? */
+#define iommu_use_hap_pt(d) (paging_mode_hap(d) && iommu_hap_pt_share)
+
 extern struct rangeset *mmio_ro_ranges;
 
 #define domain_hvm_iommu(d)     (&d->arch.hvm_domain.hvm_iommu)

[-- Attachment #4: 23789 --]
[-- Type: text/plain, Size: 1022 bytes --]

# HG changeset patch
# User Tim Deegan <tim@xen.org>
# Date 1314093267 -3600
# Node ID 1b77cf8305df0fa27acd0701f6dfbf5acbffb238
# Parent 93cc277779d3cdb6ce3fad844296a0f2638d3012
Passthrough: fix iommu_use_hap_pt() to use hap_enabled()

In line with 22924:86000076dcee, paging_mode_hap(d) shouldn't be
used in HAP internals that are called during HAP setup.

Signed-off-by: Tim Deegan <tim@xen.org>
xen-unstable changeset:   23789:1b77cf8305df
xen-unstable date:        Tue Aug 23 10:54:27 2011 +0100

diff -r 93cc277779d3 xen/include/xen/iommu.h
--- a/xen/include/xen/iommu.h	Tue Aug 23 10:43:25 2011 +0100
+++ b/xen/include/xen/iommu.h	Thu Aug 25 10:34:20 2011 +0100
@@ -35,7 +35,7 @@ extern bool_t amd_iommu_debug;
 extern bool_t amd_iommu_perdev_intremap;
 
 /* Does this domain have a P2M table we can use as its IOMMU pagetable? */
-#define iommu_use_hap_pt(d) (paging_mode_hap(d) && iommu_hap_pt_share)
+#define iommu_use_hap_pt(d) (hap_enabled(d) && iommu_hap_pt_share)
 
 extern struct rangeset *mmio_ro_ranges;
 

[-- Attachment #5: sharept --]
[-- Type: text/plain, Size: 615 bytes --]

x86/mm: Add iommu=sharept parameter for compatibility with 4.2

Signed-off-by: Tim Deegan <tim@xen.org>


diff -r 0b7b70e5290a -r abdfa414c5fc xen/drivers/passthrough/iommu.c
--- a/xen/drivers/passthrough/iommu.c	Tue Aug 23 10:54:27 2011 +0100
+++ b/xen/drivers/passthrough/iommu.c	Thu Aug 25 11:00:45 2011 +0100
@@ -82,6 +82,8 @@ static void __init parse_iommu_param(cha
             iommu_passthrough = 1;
         else if ( !strcmp(s, "dom0-strict") )
             iommu_dom0_strict = 1;
+        else if ( !strcmp(s, "sharept") )
+            iommu_hap_pt_share = 1;
 
         s = ss + 1;
     } while ( ss );

[-- Attachment #6: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

RE: [PATCH][VTD] fixing vt-d/ept page table sharing in xen-4.1

[Kay, Allen M]

> I'll turn it on by default for xen-unstable but I think 4.1 should stay as it is.

This is what I meant.  Turning it on in xen-unstable will allow QA to test this regularly.

Allen

-----Original Message-----
From: Tim Deegan [mailto:tim@xen.org] 
Sent: Thursday, August 25, 2011 3:59 AM
To: Kay, Allen M
Cc: Ren, Yongjie; xen-devel@lists.xensource.com; keir@xen.org; Hao, Xudong; Jan Beulich; Li, Xin
Subject: Re: [Xen-devel] [PATCH][VTD] fixing vt-d/ept page table sharing in xen-4.1

Hi, 

At 10:40 -0700 on 23 Aug (1314096056), Kay, Allen M wrote:
> This patch works.  Can you also back port it to xen-4.1-test?

Sure.  Try the attached series of patches on 4.1. 

> Given this feature is pretty mature now,

Eh, given that until last week it broke PV guests, I'm not so sure. :)  

> we should turn it on by default to minimize future regressions.

I'll turn it on by default for xen-unstable but I think 4.1 should stay as it is. 

Cheers,

Tim.

--
Tim Deegan <tim@xen.org>
Principal Software Engineer, Xen Platform Team Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

RE: [PATCH][VTD] fixing vt-d/ept page table sharing in xen-4.1

[Kay, Allen M]

The patches for 4.1.2 looks good.  They work without problems on my Jaketown system.

Allen

-----Original Message-----
From: Tim Deegan [mailto:tim@xen.org] 
Sent: Thursday, August 25, 2011 3:59 AM
To: Kay, Allen M
Cc: Ren, Yongjie; xen-devel@lists.xensource.com; keir@xen.org; Hao, Xudong; Jan Beulich; Li, Xin
Subject: Re: [Xen-devel] [PATCH][VTD] fixing vt-d/ept page table sharing in xen-4.1

Hi, 

At 10:40 -0700 on 23 Aug (1314096056), Kay, Allen M wrote:
> This patch works.  Can you also back port it to xen-4.1-test?

Sure.  Try the attached series of patches on 4.1. 

> Given this feature is pretty mature now,

Eh, given that until last week it broke PV guests, I'm not so sure. :)  

> we should turn it on by default to minimize future regressions.

I'll turn it on by default for xen-unstable but I think 4.1 should stay as it is. 

Cheers,

Tim.

--
Tim Deegan <tim@xen.org>
Principal Software Engineer, Xen Platform Team Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

Re: [PATCH][VTD] fixing vt-d/ept page table sharing in xen-4.1

[Tim Deegan]

At 09:56 -0700 on 25 Aug (1314266178), Kay, Allen M wrote:
> The patches for 4.1.2 looks good.  They work without problems on my Jaketown system.
> 

Thanks.  I've applied them to the 4.1-testing tree.

Tim.

-- 
Tim Deegan <tim@xen.org>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)