RFC Paging support for AMD NPT V2
PATCH 1/1 RFC Paging support for AMD NPT V2
There has been some progress, but still no joy. Definitely not intended for inclusion at this point.
Tim, Wei, I added a Xen command line toggle to disable IOMMU and P2M table sharing.
Tim, I verified that changes to p2m-pt.c don't break shadow mode (64bit hypervisor and Win 7 guest).
Hongkaixing, I incorporated your suggestion in patch 2, so I should add your Signed-off-by eventually. Please review.
Olaf, I do not see errors when mapping pages prior to eviction. Let me know how it goes for you.
If I unleash xenpaging on a domain for a non-ambitious target (64 MiBs paged out for one minute), we sail by just fine. But if I kick things up a notch, still dying on an VMEXIT_SHUTDOWN (a.k.a. triple fault). Not sure at all what to do next.
xen/drivers/passthrough/iommu.c | 2 + xen/arch/x86/mm/p2m-pt.c | 56 ++++++++++++++++++++++++++++------------ xen/arch/x86/mm/mem_event.c | 7 +++- xen/arch/x86/mm/mem_sharing.c | 7 +++++ 4 files changed, 53 insertions(+), 19 deletions(-)
PATCH 1/1 IOMMU: Add command line param to disable sharing of IOMMU and hap tables
xen/drivers/passthrough/iommu.c | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
The default is 1, and the command line parameter sets to 1. Disabling may be desired if the host will contain VMs that do paging, sharing or mem access, and won't be doing passthrough. These two features are mutually exclusive for AMD processors.
xen/drivers/passthrough/iommu.c +2 -0
@@ -87,6 +87,8 @@ static void __init parse_iommu_param(cha 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 ; s = ss +1 ; }while ( ss );
PATCH 1/1 x86/mm: Teach paging to page table-based p2m
In reply to Andres Lagar-Cavillaxen/arch/x86/mm/p2m-pt.c | 56 +++++++++++++++++++++++++++++++++-------------- 1 files changed, 39 insertions(+), 17 deletions(-)
The p2m-pt.c code, used by both shadow and AMD NPT modes, was not aware of paging types, and the implications those types have on p2m entries. Add support to the page table-based p2m to understand the paging types. This is a necessary step towards enabling memory paging on AMD NPT mode, but not yet the full solution.
Tested not to break neither shadow mode nor "normal" (i.e. no paging) AMD NPT mode.
xen/arch/x86/mm/p2m-pt.c +39 -17
@@ -53,6 +53,20 @@#define P2M_BASE_FLAGS \ (_PAGE_PRESENT | _PAGE_USER | _PAGE_DIRTY | _PAGE_ACCESSED) +#ifdef __x86_64__ +/* l1e_from_pfn is not designed to have INVALID_MFN stored. The 0xff..ff +* value tramples over the higher-order bits used for flags (NX, p2mt, +* etc.) This happens for paging entries. Thus we do this clip/unclip +* juggle for l1 entries only (no paging superpages!) */ +#define EFF_MFN_WIDTH (PADDR_BITS-PAGE_SHIFT)/* 40 bits */ +#define clipped_mfn (mfn) ((mfn) & ((1UL << EFF_MFN_WIDTH) - 1)) +#define unclip_mfn (mfn) (((mfn) == clipped_mfn(INVALID_MFN)) ? \ + INVALID_MFN : (mfn)) +#else +#define clipped_mfn (mfn) (mfn) +#define unclip_mfn (mfn) (mfn) +#endif /* __x86_64__ */ +static unsigned long p2m_type_to_flags (p2m_type_t t,mfn_t mfn) {unsigned long flags;
@@ -77,6 +91,9 @@ static unsigned long p2m_type_to_flags(pcase p2m_invalid:case p2m_mmio_dm:case p2m_populate_on_demand: +case p2m_ram_paging_out: +case p2m_ram_paged: +case p2m_ram_paging_in:default :return flags;case p2m_ram_ro:
@@ -168,7 +185,7 @@ p2m_next_level(struct p2m_domain *p2m, m shift, max)) )return 0 ; -/* PoD: Not present doesn't imply empty. */ +/* PoD/paging: Not present doesn't imply empty. */ if ( !l1e_get_flags (*p2m_entry) ) {struct page_info *pg;
@@ -384,8 +401,8 @@ p2m_set_entry(struct p2m_domain *p2m, un 0, L1_PAGETABLE_ENTRIES);ASSERT (p2m_entry); -if (mfn_valid (mfn) || (p2mt == p2m_mmio_direct) ) - entry_content =l1e_from_pfn (mfn_x (mfn), +if (mfn_valid (mfn) || (p2mt == p2m_mmio_direct) ||p2m_is_paging (p2mt) ) + entry_content =l1e_from_pfn (clipped_mfn (mfn_x (mfn)),p2m_type_to_flags (p2mt, mfn));else entry_content =l1e_empty ();
@@ -393,7 +410,7 @@ p2m_set_entry(struct p2m_domain *p2m, unif ( entry_content.l1 !=0 ) {p2m_add_iommu_flags (&entry_content,0 , iommu_pte_flags); - old_mfn =l1e_get_pfn (*p2m_entry); + old_mfn =unclip_mfn (l1e_get_pfn (*p2m_entry)); }/* level 1 entry */ p2m->write_p2m_entry (p2m, gfn, p2m_entry, table_mfn, entry_content,1 );
@@ -615,11 +632,12 @@ pod_retry_l1: sizeof(l1e));if ( ret ==0 ) { +unsigned long l1e_mfn =unclip_mfn (l1e_get_pfn (l1e)); p2mt =p2m_flags_to_type (l1e_get_flags (l1e)); -ASSERT (l1e_get_pfn (l1e) !=INVALID_MFN || !p2m_is_ram (p2mt)); +ASSERT ( (l1e_mfn !=INVALID_MFN || !p2m_is_ram (p2mt)) || + (l1e_mfn ==INVALID_MFN &&p2m_is_paging (p2mt)) ); -if (p2m_flags_to_type (l1e_get_flags (l1e)) - == p2m_populate_on_demand ) +if ( p2mt == p2m_populate_on_demand ) {/* The read has succeeded, so we know that the mapping * exits at this point. */
@@ -641,7 +659,7 @@ pod_retry_l1: }if (p2m_is_valid (p2mt) ||p2m_is_grant (p2mt) ) - mfn =_mfn (l1e_get_pfn (l1e)); + mfn =_mfn (l1e_mfn);else /* XXX see above */ p2mt = p2m_mmio_dm;
@@ -663,6 +681,8 @@ p2m_gfn_to_mfn(struct p2m_domain *p2m, upaddr_t addr = ((paddr_t )gfn) <<PAGE_SHIFT ;l2_pgentry_t *l2e;l1_pgentry_t *l1e; +unsigned long l1e_flags; +p2m_type_t l1t;ASSERT (paging_mode_translate (p2m->domain));
@@ -781,10 +801,12 @@ pod_retry_l2: l1e =map_domain_page (mfn_x (mfn)); l1e +=l1_table_offset (addr); pod_retry_l1: -if ( (l1e_get_flags (*l1e) & _PAGE_PRESENT) ==0 ) + l1e_flags =l1e_get_flags (*l1e); + l1t =p2m_flags_to_type (l1e_flags); +if ( ((l1e_flags & _PAGE_PRESENT) ==0 ) && (!p2m_is_paging (l1t)) ) {/* PoD: Try to populate */ -if (p2m_flags_to_type (l1e_get_flags (*l1e)) == p2m_populate_on_demand ) +if ( l1t == p2m_populate_on_demand ) {if ( q != p2m_query ) {if ( !p2m_pod_demand_populate (p2m, gfn,PAGE_ORDER_4K , q) )
@@ -792,15 +814,15 @@ pod_retry_l1: }else *t = p2m_populate_on_demand; } - +unmap_domain_page (l1e);return _mfn (INVALID_MFN ); } - mfn =_mfn (l1e_get_pfn (*l1e)); - *t =p2m_flags_to_type (l1e_get_flags (*l1e)); + mfn =_mfn (unclip_mfn (l1e_get_pfn (*l1e))); + *t = l1t;unmap_domain_page (l1e); -ASSERT (mfn_valid (mfn) || !p2m_is_ram (*t)); +ASSERT (mfn_valid (mfn) || !p2m_is_ram (*t) ||p2m_is_paging (*t));if ( page_order ) *page_order =PAGE_ORDER_4K ;return (p2m_is_valid (*t) ||p2m_is_grant (*t)) ? mfn :_mfn (INVALID_MFN );
@@ -914,7 +936,7 @@ static void p2m_change_type_global(struc flags =l1e_get_flags (l1e[i1]);if (p2m_flags_to_type (flags) != ot )continue ; - mfn =l1e_get_pfn (l1e[i1]); + mfn =unclip_mfn (l1e_get_pfn (l1e[i1])); gfn = i1 + (i2 + (i3 #if CONFIG_PAGING_LEVELS >= 4 + (i4 * L3_PAGETABLE_ENTRIES)
@@ -923,7 +945,7 @@ static void p2m_change_type_global(struc * L2_PAGETABLE_ENTRIES) * L1_PAGETABLE_ENTRIES;/* create a new 1le entry with the new type */ flags =p2m_type_to_flags (nt,_mfn (mfn)); - l1e_content =l1e_from_pfn (mfn, flags); + l1e_content =l1e_from_pfn (clipped_mfn (mfn), flags); p2m->write_p2m_entry (p2m, gfn, &l1e[i1], l1mfn, l1e_content,1 ); }
@@ -1073,7 +1095,7 @@ long p2m_pt_audit_p2m(struct p2m_domain entry_count++;continue ; } - mfn =l1e_get_pfn (l1e[i1]); + mfn =unclip_mfn (l1e_get_pfn (l1e[i1]));ASSERT (mfn_valid (_mfn (mfn))); m2pfn =get_gpfn_from_mfn (mfn); if ( m2pfn != gfn &&
PATCH 1/1 x86/mm: Enable paging and sharing in AMD NPT mode
In reply to Andres Lagar-Cavillaxen/arch/x86/mm/mem_event.c | 7 +++++-- xen/arch/x86/mm/mem_sharing.c | 7 +++++++ 2 files changed, 12 insertions(+), 2 deletions(-)
Both features are mutually exclusive with sharing iommu and p2m tables.
xen/arch/x86/mm/mem_event.c +5 -2
@@ -550,8 +550,11 @@ int mem_event_domctl(struct domain *d, xif ( !hap_enabled (d) )break ; -/* Currently only EPT is supported */ -if ( boot_cpu_data.x86_vendor !=X86_VENDOR_INTEL ) +/* Currently EPT or AMD with no iommu/hap page table sharing are +* supported */ +if ( !((boot_cpu_data.x86_vendor ==X86_VENDOR_INTEL ) || + ((boot_cpu_data.x86_vendor ==X86_VENDOR_AMD ) && + !iommu_hap_pt_share)) )break ; rc = -EXDEV ;
xen/arch/x86/mm/mem_sharing.c +7 -0
@@ -1202,6 +1202,13 @@ int mem_sharing_domctl(struct domain *d,if ( !hap_enabled (d) )return -ENODEV ; +/* Currently EPT or AMD with no iommu/hap page table sharing are +* supported */ +if ( !((boot_cpu_data.x86_vendor ==X86_VENDOR_INTEL ) || + ((boot_cpu_data.x86_vendor ==X86_VENDOR_AMD ) && + !iommu_hap_pt_share)) ) +return -ENODEV ; +switch (mec->op) {case XEN_DOMCTL_MEM_SHARING_CONTROL :
Quoted 12 lines
-----Original Message-----
From: Andres Lagar-Cavilla [mailto:[email protected]]
Sent: Thursday, March 01, 2012 11:16 AM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: [PATCH 0 of 3] RFC Paging support for AMD NPT V2There has been some progress, but still no joy. Definitely not intended for inclusion at this point.
Quoted 8 lines
Tim, Wei, I added a Xen command line toggle to disable IOMMU and P2M table sharing.
Tim, I verified that changes to
p2m-pt.cdon't break shadow mode (64bit hypervisor and Win 7 guest).Hongkaixing, I incorporated your suggestion in patch 2, so I should add your Signed-off-by eventually. Please review.
I have checked the code, it looks like OK. But I don't have AMD machine to run it. We used almost the same code in a AMD server months before, and it worked fine. The only difference is we have not changed the clipped invalid mfn to INVALID_MFN, just like EPT.
Quoted 17 lines
Olaf, I do not see errors when mapping pages prior to eviction. Let me know how it goes for you.
If I unleash xenpaging on a domain for a non-ambitious target (64 MiBs paged out for one minute), we sail by just fine. But if I kick things up a notch, still dying on an
VMEXIT_SHUTDOWN(a.k.a. triple fault). Not sure at all what to do next.Signed-off-by Andres Lagar-Cavilla <[email protected]>Signed-off-by Adin Scannell <[email protected]>xen/drivers/passthrough/iommu.c | 2 + xen/arch/x86/mm/p2m-pt.c | 56 ++++++++++++++++++++++++++++------------ xen/arch/x86/mm/mem_event.c | 7 +++- xen/arch/x86/mm/mem_sharing.c | 7 +++++ 4 files changed, 53 insertions(+), 19 deletions(-)
Quoted 25 lines
Quoted 18 lines
-----Original Message-----
From: Andres Lagar-Cavilla [mailto:[email protected]]
Sent: Thursday, March 01, 2012 11:16 AM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: [PATCH 0 of 3] RFC Paging support for AMD NPT V2There has been some progress, but still no joy. Definitely not intended for
inclusion at this point.Tim, Wei, I added a Xen command line toggle to disable IOMMU and P2M table
sharing.Quoted 6 lines
Tim, I verified that changes to
p2m-pt.cdon't break shadow mode (64bit hypervisor and Win 7 guest).Hongkaixing, I incorporated your suggestion in patch 2, so I should add your
Signed-off-by eventually. Please review.
Quoted 6 lines
I have checked the code, it looks like OK. But I don't have AMD machine
to run it.
We used almost the same code in a AMD server months before, and it
worked fine.
The only difference is we have not changed the clipped invalid mfn toINVALID_MFN, just like EPT.
That is most interesting. Can you elaborate? do you have functional paging on AMD since "months before"? If you do, can you please post it to xen-devel for inclusion?
Thanks a lot,
Andres
Quoted 26 lines
Quoted 24 lines
Olaf, I do not see errors when mapping pages prior to eviction. Let me know how
it goes for you.If I unleash xenpaging on a domain for a non-ambitious target (64 MiBs paged
out for one minute), we sail by just fine. But if I kick things up a notch,
still dying on anVMEXIT_SHUTDOWN(a.k.a. triple fault). Not sure at all what
to do next.Signed-off-by Andres Lagar-Cavilla <[email protected]>Signed-off-by Adin Scannell <[email protected]>xen/drivers/passthrough/iommu.c | 2 + xen/arch/x86/mm/p2m-pt.c | 56 ++++++++++++++++++++++++++++------------ xen/arch/x86/mm/mem_event.c | 7 +++- xen/arch/x86/mm/mem_sharing.c | 7 +++++ 4 files changed, 53 insertions(+), 19 deletions(-)
Quoted 42 lines
-----Original Message-----
From: Andres Lagar-Cavilla [mailto:[email protected]]
Sent: Friday, March 02, 2012 12:34 AM
To: Hongkaixing
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: RE: [PATCH 0 of 3] RFC Paging support for AMD NPT V2Quoted 25 lines
Quoted 18 lines
-----Original Message-----
From: Andres Lagar-Cavilla [mailto:[email protected]]
Sent: Thursday, March 01, 2012 11:16 AM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: [PATCH 0 of 3] RFC Paging support for AMD NPT V2There has been some progress, but still no joy. Definitely not intended for
inclusion at this point.Tim, Wei, I added a Xen command line toggle to disable IOMMU and P2M table
sharing.Quoted 6 lines
Tim, I verified that changes to
p2m-pt.cdon't break shadow mode (64bit hypervisor and Win 7 guest).Hongkaixing, I incorporated your suggestion in patch 2, so I should add your
Signed-off-by eventually. Please review.Quoted 6 lines
I have checked the code, it looks like OK. But I don't have AMD machine
to run it.
We used almost the same code in a AMD server months before, and it
worked fine.
The only difference is we have not changed the clipped invalid mfn toINVALID_MFN, just like EPT.
Quoted 6 lines
That is most interesting. Can you elaborate? do you have functional paging on AMD since "months before"? If you do, can you please post it to xen-devel for inclusion?
Thanks a lot,
Andres
All what we have done is shown below.
The code is based on xen 4.0.2, it looks informal, but it works fine when xenpaging target is set at arbitrary number.. Now we do not have AMD platform to validate, so I am not sure it can work under unstable code.
xen-4.0.2/xen/arch/x86/mm/p2m.c +12 -1
@@ -77,6 +77,13 @@#define SUPERPAGE_PAGES (1UL << 9)#define superpage_aligned (_x) (((_x)&(SUPERPAGE_PAGES-1))==0) +#ifdef __x86_64__ +/* there are only 40bits for pfn in PTE */ +#define MASK_MFN (x) ( x & ((1UL << 40) -1 )) +#else +#define MASK_MFN (x) ( x ) +#endif +static unsigned long p2m_type_to_flags (p2m_type_t t) {unsigned long flags;
@@ -1306,6 +1313,8 @@if (mfn_valid (mfn) || (p2mt == p2m_mmio_direct) ) entry_content =l1e_from_pfn (mfn_x (mfn),p2m_type_to_flags (p2mt)); +else if (p2m_is_paging ( p2mt ) ) + entry_content =l1e_from_pfn (MASK_MFN (mfn_x (mfn)),p2m_type_to_flags (p2mt));else entry_content =l1e_empty ();
@@ -1370,6 +1379,7 @@paddr_t addr = ((paddr_t )gfn) <<PAGE_SHIFT ;l2_pgentry_t *l2e;l1_pgentry_t *l1e; +unsigned long flags;ASSERT (paging_mode_translate (d));
@@ -1455,7 +1465,8 @@ l1e =map_domain_page (mfn_x (mfn)); l1e +=l1_table_offset (addr); pod_retry_l1: -if ( (l1e_get_flags (*l1e) & _PAGE_PRESENT) ==0 ) + flags =l1e_get_flags (*l1e); +if ((( flags & _PAGE_PRESENT) ==0 ) && (!p2m_is_paging (p2m_flags_to_type (flags)))) {/* PoD: Try to populate */ if (p2m_flags_to_type (l1e_get_flags (*l1e)) == p2m_populate_on_demand )
xen-4.0.2/xen/arch/x86/mm/mem_event.c +2 -3
@@ -231,10 +231,9 @@if ( d->mem_event.ring_page )break ; -/* Currently only EPT is supported */ +/* Both AMD NPT and intel EPT are supported */ rc = -ENODEV ; -if ( !(is_hvm_domain (d) && d->arch.hvm_domain.hap_enabled && - (boot_cpu_data.x86_vendor ==X86_VENDOR_INTEL )) ) +if ( !(is_hvm_domain (d) && d->arch.hvm_domain.hap_enabled ))break ;/* Get MFN of ring page */
Quoted 27 lines
Quoted 27 lines
Quoted 25 lines
Olaf, I do not see errors when mapping pages prior to eviction. Let me know how
it goes for you.If I unleash xenpaging on a domain for a non-ambitious target (64 MiBs paged
out for one minute), we sail by just fine. But if I kick things up a notch,
still dying on anVMEXIT_SHUTDOWN(a.k.a. triple fault). Not sure at all what
to do next.Signed-off-by Andres Lagar-Cavilla <[email protected]>Signed-off-by Adin Scannell <[email protected]>xen/drivers/passthrough/iommu.c | 2 + xen/arch/x86/mm/p2m-pt.c | 56 ++++++++++++++++++++++++++++------------ xen/arch/x86/mm/mem_event.c | 7 +++- xen/arch/x86/mm/mem_sharing.c | 7 +++++ 4 files changed, 53 insertions(+), 19 deletions(-)
Quoted 53 lines
Quoted 53 lines
-----Original Message-----
From: Andres Lagar-Cavilla [mailto:[email protected]]
Sent: Friday, March 02, 2012 12:34 AM
To: Hongkaixing
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: RE: [PATCH 0 of 3] RFC Paging support for AMD NPT V2Quoted 11 lines
Quoted 11 lines
-----Original Message-----
From: Andres Lagar-Cavilla [mailto:[email protected]]
Sent: Thursday, March 01, 2012 11:16 AM
To: [email protected]
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Subject: [PATCH 0 of 3] RFC Paging support for AMD NPT V2There has been some progress, but still no joy. Definitely not
intended
Quoted 8 lines
Quoted 8 lines
for
inclusion at this point.Tim, Wei, I added a Xen command line toggle to disable IOMMU and P2M table
sharing.Tim, I verified that changes to
p2m-pt.cdon't break shadow mode(64bit
Quoted 3 lines
Quoted 3 lines
hypervisor and Win 7 guest).
Hongkaixing, I incorporated your suggestion in patch 2, so I should
add
Quoted 4 lines
Quoted 2 lines
your
Signed-off-by eventually. Please review.I have checked the code, it looks like OK. But I don't have AMD
machine
Quoted 4 lines
to run it.
We used almost the same code in a AMD server months before, and it
worked fine.
The only difference is we have not changed the clipped invalid mfnto
Quoted 1 line
INVALID_MFN, just like EPT.That is most interesting. Can you elaborate? do you have functional paging
on AMD since "months before"? If you do, can you please post it to xen-devel for inclusion?
Quoted 8 lines
Quoted 2 lines
Thanks a lot,
AndresAll what we have done is shown below.
The code is based on xen 4.0.2, it looks informal, but it works fine when xenpaging target is set at arbitrary number..
Now we do not have AMD platform to validate, so I am not sure it can work under unstable code.
Thanks. That's actually great. The code is roughly the same, and it shows we're on a good lead.
Cheers!
Andres
Quoted 100 lines
Index: E:/xen-4.0.2/xen/arch/x86/mm/p2m.c
===================================================================xen-4.0.2/xen/arch/x86/mm/p2m.c +9
-0@@ -77,6 +77,13 @@#define SUPERPAGE_PAGES (1UL << 9)#define superpage_aligned (_x) (((_x)&(SUPERPAGE_PAGES-1))==0) +#ifdef __x86_64__ +/* there are only 40bits for pfn in PTE */ +#define MASK_MFN (x) ( x & ((1UL << 40) -1 )) +#else +#define MASK_MFN (x) ( x ) +#endif +static unsigned long p2m_type_to_flags (p2m_type_t t) {unsigned long flags;@@ -1306,6 +1313,8 @@if (mfn_valid (mfn) || (p2mt == p2m_mmio_direct) ) entry_content =l1e_from_pfn (mfn_x (mfn),p2m_type_to_flags (p2mt)); +else if (p2m_is_paging ( p2mt ) ) + entry_content =l1e_from_pfn (MASK_MFN (mfn_x (mfn)),p2m_type_to_flags (p2mt));else entry_content = l1e_empty();xen-4.0.2/xen/arch/x86/mm/p2m.c +3
-1@@ -1370,6 +1379,7 @@paddr_t addr = ((paddr_t )gfn) <<PAGE_SHIFT ;l2_pgentry_t *l2e;l1_pgentry_t *l1e; +unsigned long flags;ASSERT (paging_mode_translate (d));@@ -1455,7 +1465,8 @@ l1e =map_domain_page (mfn_x (mfn)); l1e +=l1_table_offset (addr); pod_retry_l1: -if ( (l1e_get_flags (*l1e) & _PAGE_PRESENT) ==0 ) + flags =l1e_get_flags (*l1e); +if ((( flags & _PAGE_PRESENT) ==0 ) && (!p2m_is_paging (p2m_flags_to_type (flags)))) {/* PoD: Try to populate */ if ( p2m_flags_to_type(l1e_get_flags(*l1e)) ==
p2m_populate_on_demand)
Index: E: /xen-4.0.2/xen/arch/x86/mm/mem_event.c
===================================================================xen-4.0.2/xen/arch/x86/mm/mem_event.c +2
-3@@ -231,10 +231,9 @@if ( d->mem_event.ring_page )break ; -/* Currently only EPT is supported */ +/* Both AMD NPT and intel EPT are supported */ rc = -ENODEV ; -if ( !(is_hvm_domain (d) && d->arch.hvm_domain.hap_enabled && - (boot_cpu_data.x86_vendor ==X86_VENDOR_INTEL )) ) +if ( !(is_hvm_domain (d) && d->arch.hvm_domain.hap_enabled ))break ;/* Get MFN of ring page */ Quoted 30 lines
Quoted 5 lines
Quoted 5 lines
Olaf, I do not see errors when mapping pages prior to eviction. Let
me
Quoted 4 lines
Quoted 4 lines
know how
it goes for you.If I unleash xenpaging on a domain for a non-ambitious target (64
MiBs
Quoted 4 lines
Quoted 4 lines
paged
out for one minute), we sail by just fine. But if I kick things up a notch,
still dying on anVMEXIT_SHUTDOWN(a.k.a. triple fault). Not sure atall
Quoted 14 lines
Quoted 12 lines
what
to do next.Signed-off-by Andres Lagar-Cavilla <[email protected]>Signed-off-by Adin Scannell <[email protected]>xen/drivers/passthrough/iommu.c | 2 + xen/arch/x86/mm/p2m-pt.c | 56 ++++++++++++++++++++++++++++------------ xen/arch/x86/mm/mem_event.c | 7 +++- xen/arch/x86/mm/mem_sharing.c | 7 +++++ 4 files changed, 53 insertions(+), 19 deletions(-)
Re: [PATCH 3 of 3] x86/mm: Enable paging and sharing in AMD NPT mode
In reply to Andres Lagar-CavillaQuoted 14 lines · Andres Lagar-Cavilla
xen/arch/x86/mm/mem_event.c | 7 +++++-- xen/arch/x86/mm/mem_sharing.c | 7 +++++++ 2 files changed, 12 insertions(+), 2 deletions(-)Both features are mutually exclusive with sharing iommu and p2m tables.
Signed-off-by Andres Lagar-Cavilla <[email protected]>Signed-off-by Adin Scannell <[email protected]>diff -r
d6c3c77ad749-r4c6bee5a191axen/arch/x86/mm/mem_event.c
xen/arch/x86/mm/mem_event.c int mem_event_domctl(struct domain *d, x · line 557
Quoted 10 lines
xen/arch/x86/mm/mem_event.c +5
-2@@ -550,8 +550,11 @@ int mem_event_domctl(struct domain *d, xif ( !hap_enabled (d) )break ; -/* Currently only EPT is supported */ -if ( boot_cpu_data.x86_vendor !=X86_VENDOR_INTEL ) +/* Currently EPT or AMD with no iommu/hap page table sharing are +* supported */ +if ( !((boot_cpu_data.x86_vendor ==X86_VENDOR_INTEL ) || + ((boot_cpu_data.x86_vendor ==X86_VENDOR_AMD ) && + !iommu_hap_pt_share)) )
AFAICT p2m tricks are not going to work with IOMMU/passthrough at all, since there's no mechanism to fault and retry. So the interlock should really be against need_iommu() or similar. (And vice versa, to stop a device being passed through to a VM that has sharing/paging/events enabled).
Cheers,
Tim
Re: [PATCH 2 of 3] x86/mm: Teach paging to page table-based p2m
In reply to Andres Lagar-CavillaQuoted 9 lines · Andres Lagar-Cavilla
xen/arch/x86/mm/p2m-pt.c +3
-0@@ -53,6 +53,20 @@#define P2M_BASE_FLAGS \ (_PAGE_PRESENT | _PAGE_USER | _PAGE_DIRTY | _PAGE_ACCESSED) +#ifdef __x86_64__ +/* l1e_from_pfn is not designed to have INVALID_MFN stored. The 0xff..ff +* value tramples over the higher-order bits used for flags (NX, p2mt,
xen/arch/x86/mm/p2m-pt.c line 68
Quoted 10 lines
xen/arch/x86/mm/p2m-pt.c +10
-0+* etc.) This happens for paging entries. Thus we do this clip/unclip +* juggle for l1 entries only (no paging superpages!) */ +#define EFF_MFN_WIDTH (PADDR_BITS-PAGE_SHIFT)/* 40 bits */ +#define clipped_mfn (mfn) ((mfn) & ((1UL << EFF_MFN_WIDTH) - 1)) +#define unclip_mfn (mfn) (((mfn) == clipped_mfn(INVALID_MFN)) ? \ + INVALID_MFN : (mfn)) +#else +#define clipped_mfn (mfn) (mfn) +#define unclip_mfn (mfn) (mfn) +#endif /* __x86_64__ */
Hmmm. If we need to have this, we should probably have it in the main l*_from_pfn and l*_get_pfn code rather than needing to scatter it around in the callers. And we need a check in the e820 map to make sure we don't ever use MFN 0xffffffff (once CPUs start supporting it).
The alternative would be to add another type so we don't have to store INVALID_MFN in p2m_ram_paging_in entries.
Cheers,
Tim.
Re: [PATCH 2 of 3] x86/mm: Teach paging to page table-based p2m
Quoted 27 lines
Quoted 12 lines · Andres Lagar-Cavilla
xen/arch/x86/mm/p2m-pt.c +2
-0@@ -53,6 +53,20 @@#define P2M_BASE_FLAGS \ (_PAGE_PRESENT | _PAGE_USER | _PAGE_DIRTY | _PAGE_ACCESSED) +#ifdef __x86_64__ +/* l1e_from_pfn is not designed to have INVALID_MFN stored. The 0xff..ff
+ * value tramples over the higher-order bits used for flags (NX, p2mt,
+ * etc.) This happens for paging entries. Thus we do this clip/unclip
+ * juggle for l1 entries only (no paging superpages!) */xen/arch/x86/mm/p2m-pt.c line 68
Quoted 8 lines
xen/arch/x86/mm/p2m-pt.c +8
-0+#define EFF_MFN_WIDTH (PADDR_BITS-PAGE_SHIFT)/* 40 bits */ +#define clipped_mfn (mfn) ((mfn) & ((1UL << EFF_MFN_WIDTH) - 1)) +#define unclip_mfn (mfn) (((mfn) == clipped_mfn(INVALID_MFN)) ? \ + INVALID_MFN : (mfn)) +#else +#define clipped_mfn (mfn) (mfn) +#define unclip_mfn (mfn) (mfn) +#endif /* __x86_64__ */ Hmmm. If we need to have this, we should probably have it in the main l*_from_pfn and l*_get_pfn code rather than needing to scatter it around in the callers. And we need a check in the e820 map to make sure we don't ever use MFN
0xffffffff(once CPUs start supporting it).
Quoted 2 lines
The alternative would be to add another type so we don't have to store
INVALID_MFNinp2m_ram_paging_inentries.
A lot of callers store INVALID_MFN into p2m entries (clear_mmio_p2m_entry, p2m_remove_page, more). For all of them, as well as for paging eviction, what matters is the type stored, not the mfn.
That is why retrieving the INVALID_MFN is not the problem. The ept code itself clips the INVALID_MFN (typecast to bitfield in ept_entry union) and everything seems to work fine when returning the clipped INVALID_MFN: no one actually cares about that mfn.
Because of that, I believe it's neither necessary to unclip on the extraction path, nor to take any special precautions in the e820.
But for p2m-pt, all the callers that set INVALID_MFN seem to work purely by chance. In all cases INVALID_MFN will trample over the higher order bits that are supposed to store the type. When reading the entry, the p2m-pt code will not understand, default to p2m_mmio_dm type, and that seems to be good enough (but not good enough when the type was supposed to be paged_out).
So, I could submit one patch to clip the INVALID_MFN for p2m-pt in the 4k page case of set_entry. That is the only place where we need it, and avoids subtly changing semantics for a zillion other callers.
Or, we could change the paging code to store _mfn(0). This is the way of PoD. I get the feeling George got lucky, or knew about this all along :) The key, again, is not to trample over the high order bits.
Then, the rest of this patch, adding if's and changing asserts, can be dealt with separately.
Thanks,
Andres
Quoted 5 lines
Cheers,
Tim.
Re: [PATCH 2 of 3] x86/mm: Teach paging to page table-based p2m
Quoted 12 lines · Andres Lagar-Cavilla
Quoted 7 lines
Hmmm. If we need to have this, we should probably have it in the main l*_from_pfn and l*_get_pfn code rather than needing to scatter it around in the callers. And we need a check in the e820 map to make sure we don't ever use MFN
0xffffffff(once CPUs start supporting it).The alternative would be to add another type so we don't have to store
INVALID_MFNinp2m_ram_paging_inentries.A lot of callers store
INVALID_MFNinto p2m entries (clear_mmio_p2m_entry,p2m_remove_page, more). For all of them, as well as for paging eviction, what matters is the type stored, not the mfn.
Quoted 7 lines
That is why retrieving the
INVALID_MFNis not the problem. The ept code itself clips theINVALID_MFN(typecast to bitfield inept_entryunion) and everything seems to work fine when returning the clippedINVALID_MFN: no one actually cares about that mfn.Because of that, I believe it's neither necessary to unclip on the extraction path, nor to take any special precautions in the e820.
Righto. In that case, I'd be happy with just clipping MFNs and not trying to unpack them. But I think it should happen in the main pte-building macros, not scattered around the p2m code. It should just be a matter of using PADDR_MASK in the right place.
Cheers,
Tim.
Re: [PATCH 2 of 3] x86/mm: Teach paging to page table-based p2m
Quoted 21 lines
Quoted 20 lines · Andres Lagar-Cavilla
Quoted 2 lines
Hmmm. If we need to have this, we should probably have it in the main l*_from_pfn and l*_get_pfn code rather than needing to scatter it
around
Quoted 5 lines
in the callers. And we need a check in the e820 map to make sure we don't ever use MFN
0xffffffff(once CPUs start supporting it).The alternative would be to add another type so we don't have to store
INVALID_MFNinp2m_ram_paging_inentries.A lot of callers store
INVALID_MFNinto p2m entries (clear_mmio_p2m_entry,p2m_remove_page, more). For all of them, as well as for paging eviction, what matters is the type stored, not the mfn.That is why retrieving the
INVALID_MFNis not the problem. The ept code itself clips theINVALID_MFN(typecast to bitfield inept_entryunion) and
everything seems to work fine when returning the clippedINVALID_MFN: no one actually cares about that mfn.
Quoted 7 lines
Quoted 2 lines
Because of that, I believe it's neither necessary to unclip on the extraction path, nor to take any special precautions in the e820.
Righto. In that case, I'd be happy with just clipping MFNs and not trying to unpack them. But I think it should happen in the main pte-building macros, not scattered around the p2m code. It should just be a matter of using
PADDR_MASKin the right place.
Something along these lines? (RFC, not tested yet) Andres
# HG changeset patch
# Parent e490b3ca615d0141c1d97a89c1880dc8dc8f0783
Clip mfn to allowable width when building a PTE.
Otherwise, INVALID_MFN tramples over high order bits used for additional flags.
xen/include/asm-x86/page.h +0 -1
@@ -107,13 +107,17 @@/* Construct a pte from a pfn and access flags. */ #define l1e_from_pfn (pfn, flags) \ - ((l1_pgentry_t) { ((intpte_t)(pfn) << PAGE_SHIFT) |
put_pte_flags(flags) })
+ ((l1_pgentry_t) { ((intpte_t)((pfn) & (PADDR_MASK >> PAGE_SHIFT)) <<
\
+ PAGE_SHIFT) | put_pte_flags(flags) })
#define l2e_from_pfn(pfn, flags) \
- ((l2_pgentry_t) { ((intpte_t)(pfn) << PAGE_SHIFT) |put_pte_flags(flags) })
+ ((l2_pgentry_t) { ((intpte_t)((pfn) & (PADDR_MASK >> PAGE_SHIFT)) <<
\
+ PAGE_SHIFT) | put_pte_flags(flags) })
#define l3e_from_pfn(pfn, flags) \
- ((l3_pgentry_t) { ((intpte_t)(pfn) << PAGE_SHIFT) |put_pte_flags(flags) })
+ ((l3_pgentry_t) { ((intpte_t)((pfn) & (PADDR_MASK >> PAGE_SHIFT)) <<
\
+ PAGE_SHIFT) | put_pte_flags(flags) })
#define l4e_from_pfn(pfn, flags) \
- ((l4_pgentry_t) { ((intpte_t)(pfn) << PAGE_SHIFT) |put_pte_flags(flags) })
+ ((l4_pgentry_t) { ((intpte_t)((pfn) & (PADDR_MASK >> PAGE_SHIFT)) <<
\
+ PAGE_SHIFT) | put_pte_flags(flags) })
/* Construct a pte from a physical address and access flags. */ #ifndef __ASSEMBLY__
Quoted 5 lines
Cheers,
Tim.
Re: [PATCH 2 of 3] x86/mm: Teach paging to page table-based p2m
Review
Quoted 13 lines · Andres Lagar-Cavilla
Quoted 4 lines
Righto. In that case, I'd be happy with just clipping MFNs and not trying to unpack them. But I think it should happen in the main pte-building macros, not scattered around the p2m code. It should just be a matter of using
PADDR_MASKin the right place.Something along these lines? (RFC, not tested yet) Andres
/* Construct a pte from a pfn and access flags. */
#definel1e_from_pfn(pfn, flags)\(quoted diff) +2
-1- ((l1_pgentry_t) { ((intpte_t)(pfn) << PAGE_SHIFT) | put_pte_flags(flags) }) + ((l1_pgentry_t) { ((intpte_t)((pfn) & (PADDR_MASK >> PAGE_SHIFT)) << \ + PAGE_SHIFT) | put_pte_flags(flags) })
Yes, that's the idea. I think
Review
Quoted 2 lines
(quoted diff) +2
-0+ ((l1_pgentry_t) { (((intpte_t)(pfn) << PAGE_SHIFT) & PADDR_MASK) \ + | put_pte_flags(flags) })
is a little neater, maybe?
In any case, I'd like Keir's ack on this, since it will affect all the PV pagetable code too (hopefully in a trivial and correct way).
Tim.
Re: [PATCH 2 of 3] x86/mm: Teach paging to page table-based p2m
Quoted 18 lines · Tim Deegan
Quoted 12 lines · Andres Lagar-Cavilla
Quoted 4 lines
Righto. In that case, I'd be happy with just clipping MFNs and not trying to unpack them. But I think it should happen in the main pte-building macros, not scattered around the p2m code. It should just be a matter of using
PADDR_MASKin the right place.Something along these lines? (RFC, not tested yet) Andres
/* Construct a pte from a pfn and access flags. */
#definel1e_from_pfn(pfn, flags)\
- ((l1_pgentry_t) { ((intpte_t)(pfn) <<PAGE_SHIFT) |put_pte_flags(flags)
})Review
Quoted 2 lines
(quoted diff) +2
-0+ ((l1_pgentry_t) { ((intpte_t)((pfn) & (PADDR_MASK >> PAGE_SHIFT)) << \ + PAGE_SHIFT) | put_pte_flags(flags) })Yes, that's the idea. I think
Quoted 7 lines
Review
Quoted 2 lines
(quoted diff) +2
-0+ ((l1_pgentry_t) { (((intpte_t)(pfn) << PAGE_SHIFT) & PADDR_MASK) \ + | put_pte_flags(flags) })is a little neater, maybe?
In any case, I'd like Keir's ack on this, since it will affect all the PV pagetable code too (hopefully in a trivial and correct way).
Fine by me. I don't think any existing users should be intentionally stuffing non-address bits into a pte via the pfn parameter of a pte-constructor. I wonder though whether you should have your own constructor, or wrapper round the generic constructor, for laundering your filthy nasty pfns? ;-)
-- Keir
Quoted 1 line
Tim.