* [V15 PATCH 0/2] pvh dom0 patches... @ 2014-05-22 23:30 Mukesh Rathor 2014-05-22 23:30 ` [V15 PATCH 1/2] pvh dom0: Add and remove foreign pages Mukesh Rathor 2014-05-22 23:30 ` [V15 PATCH 2/2] dom0: add opt_dom0pvh to setup.c Mukesh Rathor 0 siblings, 2 replies; 14+ messages in thread From: Mukesh Rathor @ 2014-05-22 23:30 UTC (permalink / raw) To: xen-devel; +Cc: George.Dunlap, keir.xen, tim, JBeulich Hi, Attached please find v15 of dom0 pvh patch series. Changes from V14: - In ept_set_entry, check for rc after call to atomic_write_ept_entry and clear old_entry if error. Tim, would like to get your ack/nack/comments on this please... thanks, Mukesh ^ permalink raw reply [flat|nested] 14+ messages in thread
* [V15 PATCH 1/2] pvh dom0: Add and remove foreign pages 2014-05-22 23:30 [V15 PATCH 0/2] pvh dom0 patches Mukesh Rathor @ 2014-05-22 23:30 ` Mukesh Rathor 2014-05-23 7:20 ` Jan Beulich 2014-05-23 19:05 ` Tim Deegan 2014-05-22 23:30 ` [V15 PATCH 2/2] dom0: add opt_dom0pvh to setup.c Mukesh Rathor 1 sibling, 2 replies; 14+ messages in thread From: Mukesh Rathor @ 2014-05-22 23:30 UTC (permalink / raw) To: xen-devel Cc: JBeulich, George.Dunlap, tim, eddie.dong, keir.xen, jun.nakajima In this patch, a new function, p2m_add_foreign(), is added to map pages from a foreign guest into dom0 for various purposes like domU creation, running xentrace, etc... Such pages are typed p2m_map_foreign. Note, it is the nature of such pages that a refcnt is held during their stay in the p2m. The refcnt is added and released in the low level ept function atomic_write_ept_entry. That macro is converted to a function to allow for such refcounting, which only applies to leaf entries in the ept. Furthermore, please note that paging/sharing is disabled if the controlling or hardware domain is pvh. Any enabling of those features would need to ensure refcnt are properly maintained for foreign types, or paging/sharing is skipped for foreign types. Also, we change get_pg_owner so it allows foreign mappings for pvh. Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com> --- xen/arch/x86/mm.c | 4 +- xen/arch/x86/mm/mem_event.c | 11 ++++ xen/arch/x86/mm/p2m-ept.c | 104 +++++++++++++++++++++++++++++------- xen/arch/x86/mm/p2m-pt.c | 7 +++ xen/arch/x86/mm/p2m.c | 126 +++++++++++++++++++++++++++++++++++++++++--- xen/include/asm-x86/p2m.h | 7 +++ 6 files changed, 233 insertions(+), 26 deletions(-) diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index d3459f4..2543916 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -2811,7 +2811,7 @@ static struct domain *get_pg_owner(domid_t domid) goto out; } - if ( unlikely(paging_mode_translate(curr)) ) + if ( !is_pvh_domain(curr) && unlikely(paging_mode_translate(curr)) ) { MEM_LOG("Cannot mix foreign mappings with translated domains"); goto out; @@ -4584,6 +4584,8 @@ int xenmem_add_to_physmap_one( page = mfn_to_page(mfn); break; } + case XENMAPSPACE_gmfn_foreign: + return p2m_add_foreign(d, idx, gpfn, foreign_domid); default: break; } diff --git a/xen/arch/x86/mm/mem_event.c b/xen/arch/x86/mm/mem_event.c index f84c383..40ae841 100644 --- a/xen/arch/x86/mm/mem_event.c +++ b/xen/arch/x86/mm/mem_event.c @@ -538,6 +538,12 @@ int mem_event_domctl(struct domain *d, xen_domctl_mem_event_op_t *mec, case XEN_DOMCTL_MEM_EVENT_OP_PAGING_ENABLE: { struct p2m_domain *p2m = p2m_get_hostp2m(d); + + rc = -EOPNOTSUPP; + /* pvh fixme: p2m_is_foreign types need addressing */ + if ( is_pvh_vcpu(current) || is_pvh_domain(hardware_domain) ) + break; + rc = -ENODEV; /* Only HAP is supported */ if ( !hap_enabled(d) ) @@ -620,6 +626,11 @@ int mem_event_domctl(struct domain *d, xen_domctl_mem_event_op_t *mec, { case XEN_DOMCTL_MEM_EVENT_OP_SHARING_ENABLE: { + rc = -EOPNOTSUPP; + /* pvh fixme: p2m_is_foreign types need addressing */ + if ( is_pvh_vcpu(current) || is_pvh_domain(hardware_domain) ) + break; + rc = -ENODEV; /* Only HAP is supported */ if ( !hap_enabled(d) ) diff --git a/xen/arch/x86/mm/p2m-ept.c b/xen/arch/x86/mm/p2m-ept.c index bb98945..12547a3 100644 --- a/xen/arch/x86/mm/p2m-ept.c +++ b/xen/arch/x86/mm/p2m-ept.c @@ -36,8 +36,6 @@ #define atomic_read_ept_entry(__pepte) \ ( (ept_entry_t) { .epte = read_atomic(&(__pepte)->epte) } ) -#define atomic_write_ept_entry(__pepte, __epte) \ - write_atomic(&(__pepte)->epte, (__epte).epte) #define is_epte_present(ept_entry) ((ept_entry)->epte & 0x7) #define is_epte_superpage(ept_entry) ((ept_entry)->sp) @@ -46,6 +44,64 @@ static inline bool_t is_epte_valid(ept_entry_t *e) return (e->epte != 0 && e->sa_p2mt != p2m_invalid); } +/* returns : 0 for success, -errno otherwise */ +static int atomic_write_ept_entry(ept_entry_t *entryptr, ept_entry_t new, + int level) +{ + int rc; + unsigned long oldmfn = INVALID_MFN; + bool_t check_foreign = (new.mfn != entryptr->mfn || + new.sa_p2mt != entryptr->sa_p2mt); + + if ( level ) + { + ASSERT(!is_epte_superpage(&new) || !p2m_is_foreign(new.sa_p2mt)); + write_atomic(&entryptr->epte, new.epte); + return 0; + } + + if ( unlikely(p2m_is_foreign(new.sa_p2mt)) ) + { + rc = -EINVAL; + if ( !is_epte_present(&new) ) + goto out; + + if ( check_foreign ) + { + struct domain *fdom; + + if ( !mfn_valid(new.mfn) ) + goto out; + + rc = -ESRCH; + fdom = page_get_owner(mfn_to_page(new.mfn)); + if ( fdom == NULL ) + goto out; + + /* get refcount on the page */ + rc = -EBUSY; + if ( !get_page(mfn_to_page(new.mfn), fdom) ) + goto out; + } + } + + if ( unlikely(p2m_is_foreign(entryptr->sa_p2mt)) && check_foreign ) + oldmfn = entryptr->mfn; + + write_atomic(&entryptr->epte, new.epte); + + if ( unlikely(oldmfn != INVALID_MFN) ) + put_page(mfn_to_page(oldmfn)); + + rc = 0; + + out: + if ( rc ) + gdprintk(XENLOG_ERR, "epte o:%"PRIx64" n:%"PRIx64" rc:%d\n", + entryptr->epte, new.epte, rc); + return rc; +} + static void ept_p2m_type_to_flags(ept_entry_t *entry, p2m_type_t type, p2m_access_t access) { /* First apply type permissions */ @@ -275,8 +331,9 @@ static int ept_next_level(struct p2m_domain *p2m, bool_t read_only, * present entries in the given page table, optionally marking the entries * also for their subtrees needing P2M type re-calculation. */ -static bool_t ept_invalidate_emt(mfn_t mfn, bool_t recalc) +static bool_t ept_invalidate_emt(mfn_t mfn, bool_t recalc, int level) { + int rc; ept_entry_t *epte = map_domain_page(mfn_x(mfn)); unsigned int i; bool_t changed = 0; @@ -292,7 +349,8 @@ static bool_t ept_invalidate_emt(mfn_t mfn, bool_t recalc) e.emt = MTRR_NUM_TYPES; if ( recalc ) e.recalc = 1; - atomic_write_ept_entry(&epte[i], e); + rc = atomic_write_ept_entry(&epte[i], e, level); + ASSERT(rc == 0); changed = 1; } @@ -316,7 +374,7 @@ static int ept_invalidate_emt_range(struct p2m_domain *p2m, ept_entry_t *table; unsigned long gfn_remainder = first_gfn; unsigned int i, index; - int rc = 0, ret = GUEST_TABLE_MAP_FAILED; + int wrc, rc = 0, ret = GUEST_TABLE_MAP_FAILED; table = map_domain_page(pagetable_get_pfn(p2m_get_pagetable(p2m))); for ( i = ept_get_wl(&p2m->ept); i > target; --i ) @@ -342,7 +400,8 @@ static int ept_invalidate_emt_range(struct p2m_domain *p2m, rc = -ENOMEM; goto out; } - atomic_write_ept_entry(&table[index], split_ept_entry); + wrc = atomic_write_ept_entry(&table[index], split_ept_entry, i); + ASSERT(wrc == 0); for ( ; i > target; --i ) if ( !ept_next_level(p2m, 1, &table, &gfn_remainder, i) ) @@ -361,7 +420,8 @@ static int ept_invalidate_emt_range(struct p2m_domain *p2m, { e.emt = MTRR_NUM_TYPES; e.recalc = 1; - atomic_write_ept_entry(&table[index], e); + wrc = atomic_write_ept_entry(&table[index], e, target); + ASSERT(wrc == 0); rc = 1; } } @@ -390,7 +450,7 @@ static int resolve_misconfig(struct p2m_domain *p2m, unsigned long gfn) unsigned int level = ept_get_wl(ept); unsigned long mfn = ept_get_asr(ept); ept_entry_t *epte; - int rc = 0; + int wrc, rc = 0; if ( !mfn ) return 0; @@ -431,7 +491,8 @@ static int resolve_misconfig(struct p2m_domain *p2m, unsigned long gfn) ept_p2m_type_to_flags(&e, e.sa_p2mt, e.access); } e.recalc = 0; - atomic_write_ept_entry(&epte[i], e); + wrc = atomic_write_ept_entry(&epte[i], e, level); + ASSERT(wrc == 0); } } else @@ -465,7 +526,8 @@ static int resolve_misconfig(struct p2m_domain *p2m, unsigned long gfn) { if ( ept_split_super_page(p2m, &e, level, level - 1) ) { - atomic_write_ept_entry(&epte[i], e); + wrc = atomic_write_ept_entry(&epte[i], e, level); + ASSERT(wrc == 0); unmap_domain_page(epte); mfn = e.mfn; continue; @@ -479,7 +541,8 @@ static int resolve_misconfig(struct p2m_domain *p2m, unsigned long gfn) e.recalc = 0; if ( recalc && p2m_is_changeable(e.sa_p2mt) ) ept_p2m_type_to_flags(&e, e.sa_p2mt, e.access); - atomic_write_ept_entry(&epte[i], e); + wrc = atomic_write_ept_entry(&epte[i], e, level); + ASSERT(wrc == 0); } rc = 1; @@ -489,11 +552,12 @@ static int resolve_misconfig(struct p2m_domain *p2m, unsigned long gfn) if ( e.emt == MTRR_NUM_TYPES ) { ASSERT(is_epte_present(&e)); - ept_invalidate_emt(_mfn(e.mfn), e.recalc); + ept_invalidate_emt(_mfn(e.mfn), e.recalc, level); smp_wmb(); e.emt = 0; e.recalc = 0; - atomic_write_ept_entry(&epte[i], e); + wrc = atomic_write_ept_entry(&epte[i], e, level); + ASSERT(wrc == 0); unmap_domain_page(epte); rc = 1; } @@ -585,6 +649,7 @@ ept_set_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn, ASSERT((target == 2 && hvm_hap_has_1gb()) || (target == 1 && hvm_hap_has_2mb()) || (target == 0)); + ASSERT(!p2m_is_foreign(p2mt) || target == 0); table = map_domain_page(pagetable_get_pfn(p2m_get_pagetable(p2m))); @@ -649,7 +714,8 @@ ept_set_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn, /* now install the newly split ept sub-tree */ /* NB: please make sure domian is paused and no in-fly VT-d DMA. */ - atomic_write_ept_entry(ept_entry, split_ept_entry); + rc = atomic_write_ept_entry(ept_entry, split_ept_entry, i); + ASSERT(rc == 0); /* then move to the level we want to make real changes */ for ( ; i > target; i-- ) @@ -688,10 +754,12 @@ ept_set_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn, ept_p2m_type_to_flags(&new_entry, p2mt, p2ma); } - atomic_write_ept_entry(ept_entry, new_entry); + rc = atomic_write_ept_entry(ept_entry, new_entry, target); + if ( rc ) + old_entry.epte = 0; /* Track the highest gfn for which we have ever had a valid mapping */ - if ( p2mt != p2m_invalid && + if ( rc == 0 && p2mt != p2m_invalid && (gfn + (1UL << order) - 1 > p2m->max_mapped_pfn) ) p2m->max_mapped_pfn = gfn + (1UL << order) - 1; @@ -893,7 +961,7 @@ static void ept_change_entry_type_global(struct p2m_domain *p2m, if ( !mfn ) return; - if ( ept_invalidate_emt(_mfn(mfn), 1) ) + if ( ept_invalidate_emt(_mfn(mfn), 1, ept_get_wl(&p2m->ept)) ) ept_sync_domain(p2m); } @@ -951,7 +1019,7 @@ static void ept_memory_type_changed(struct p2m_domain *p2m) if ( !mfn ) return; - if ( ept_invalidate_emt(_mfn(mfn), 0) ) + if ( ept_invalidate_emt(_mfn(mfn), 0, ept_get_wl(&p2m->ept)) ) ept_sync_domain(p2m); } diff --git a/xen/arch/x86/mm/p2m-pt.c b/xen/arch/x86/mm/p2m-pt.c index cd9867a..a1794d0 100644 --- a/xen/arch/x86/mm/p2m-pt.c +++ b/xen/arch/x86/mm/p2m-pt.c @@ -513,6 +513,13 @@ p2m_pt_set_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn, __trace_var(TRC_MEM_SET_P2M_ENTRY, 0, sizeof(t), &t); } + if ( unlikely(p2m_is_foreign(p2mt)) ) + { + /* pvh fixme: foreign types are only supported on ept at present */ + gdprintk(XENLOG_WARNING, "Unimplemented foreign p2m type.\n"); + return -EINVAL; + } + /* Carry out any eventually pending earlier changes first. */ rc = do_recalc(p2m, gfn); if ( rc < 0 ) diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c index b50747a..642ec28 100644 --- a/xen/arch/x86/mm/p2m.c +++ b/xen/arch/x86/mm/p2m.c @@ -36,6 +36,7 @@ #include <xen/event.h> #include <asm/hvm/nestedhvm.h> #include <asm/hvm/svm/amd-iommu-proto.h> +#include <xsm/xsm.h> #include "mm-locks.h" @@ -311,14 +312,20 @@ struct page_info *get_page_from_gfn_p2m( /* Fast path: look up and get out */ p2m_read_lock(p2m); mfn = __get_gfn_type_access(p2m, gfn, t, a, 0, NULL, 0); - if ( (p2m_is_ram(*t) || p2m_is_grant(*t)) - && mfn_valid(mfn) + if ( p2m_is_any_ram(*t) && mfn_valid(mfn) && !((q & P2M_UNSHARE) && p2m_is_shared(*t)) ) { page = mfn_to_page(mfn); - if ( !get_page(page, d) - /* Page could be shared */ - && !get_page(page, dom_cow) ) + if ( unlikely(p2m_is_foreign(*t)) ) + { + struct domain *fdom = page_get_owner_and_reference(page); + ASSERT(fdom != d); + if ( fdom == NULL ) + page = NULL; + } + else if ( !get_page(page, d) + /* Page could be shared */ + && !get_page(page, dom_cow) ) page = NULL; } p2m_read_unlock(p2m); @@ -468,6 +475,10 @@ int p2m_alloc_table(struct p2m_domain *p2m) return rc; } +/* + * pvh fixme: when adding support for pvh non-hardware domains, this path must + * cleanup any foreign p2m types (release refcnts on them). + */ void p2m_teardown(struct p2m_domain *p2m) /* Return all the p2m pages to Xen. * We know we don't have any extra mappings to these pages */ @@ -836,8 +847,8 @@ static int set_typed_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn, } /* Set foreign mfn in the given guest's p2m table. */ -static int __attribute__((unused)) -set_foreign_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn) +static int set_foreign_p2m_entry(struct domain *d, unsigned long gfn, + mfn_t mfn) { return set_typed_p2m_entry(d, gfn, mfn, p2m_map_foreign); } @@ -1794,6 +1805,107 @@ out_p2m_audit: #endif /* P2M_AUDIT */ /* + * Add frame from foreign domain to target domain's physmap. Similar to + * XENMAPSPACE_gmfn but the frame is foreign being mapped into current, + * and is not removed from foreign domain. + * + * Usage: - libxl on pvh dom0 creating a guest and doing privcmd_ioctl_mmap. + * - xentrace running on dom0 mapping xenheap pages. foreigndom would + * be DOMID_XEN in such a case. + * etc.. + * + * Side Effect: the mfn for fgfn will be refcounted in lower level routines + * so it is not lost while mapped here. The refcnt is released + * via the XENMEM_remove_from_physmap path. + * + * Returns: 0 ==> success + */ +int p2m_add_foreign(struct domain *tdom, unsigned long fgfn, + unsigned long gpfn, domid_t foreigndom) +{ + p2m_type_t p2mt, p2mt_prev; + unsigned long prev_mfn, mfn; + struct page_info *page; + int rc; + struct domain *fdom; + + ASSERT(tdom); + if ( foreigndom == DOMID_SELF || !is_pvh_domain(tdom) ) + return -EINVAL; + /* + * pvh fixme: until support is added to p2m teardown code to cleanup any + * foreign entries, limit this to hardware domain only. + */ + if ( !is_hardware_domain(tdom) ) + return -EPERM; + + if ( foreigndom == DOMID_XEN ) + fdom = rcu_lock_domain(dom_xen); + else + fdom = rcu_lock_domain_by_id(foreigndom); + if ( fdom == NULL ) + return -ESRCH; + + rc = -EINVAL; + if ( tdom == fdom ) + goto out; + + rc = xsm_map_gmfn_foreign(XSM_TARGET, tdom, fdom); + if ( rc ) + goto out; + + /* + * Take a refcnt on the mfn. NB: following supported for foreign mapping: + * ram_rw | ram_logdirty | ram_ro | paging_out. + */ + page = get_page_from_gfn(fdom, fgfn, &p2mt, P2M_ALLOC); + if ( !page || + !p2m_is_ram(p2mt) || p2m_is_shared(p2mt) || p2m_is_hole(p2mt) ) + { + if ( page ) + put_page(page); + rc = -EINVAL; + goto out; + } + mfn = mfn_x(page_to_mfn(page)); + + /* Remove previously mapped page if it is present. */ + prev_mfn = mfn_x(get_gfn(tdom, gpfn, &p2mt_prev)); + if ( mfn_valid(_mfn(prev_mfn)) ) + { + if ( is_xen_heap_mfn(prev_mfn) ) + /* Xen heap frames are simply unhooked from this phys slot */ + guest_physmap_remove_page(tdom, gpfn, prev_mfn, 0); + else + /* Normal domain memory is freed, to avoid leaking memory. */ + guest_remove_page(tdom, gpfn); + } + /* + * Create the new mapping. Can't use guest_physmap_add_page() because it + * will update the m2p table which will result in mfn -> gpfn of dom0 + * and not fgfn of domU. + */ + rc = set_foreign_p2m_entry(tdom, gpfn, _mfn(mfn)); + if ( rc ) + gdprintk(XENLOG_WARNING, "set_foreign_p2m_entry failed. " + "gpfn:%lx mfn:%lx fgfn:%lx td:%d fd:%d\n", + gpfn, mfn, fgfn, tdom->domain_id, fdom->domain_id); + + put_page(page); + + /* + * This put_gfn for the above get_gfn for prev_mfn. We must do this + * after set_foreign_p2m_entry so another cpu doesn't populate the gpfn + * before us. + */ + put_gfn(tdom, gpfn); + +out: + if ( fdom ) + rcu_unlock_domain(fdom); + return rc; +} +/* * Local variables: * mode: C * c-file-style: "BSD" diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h index 027f011..d0cfdac 100644 --- a/xen/include/asm-x86/p2m.h +++ b/xen/include/asm-x86/p2m.h @@ -188,6 +188,10 @@ typedef unsigned int p2m_query_t; #define p2m_is_broken(_t) (p2m_to_mask(_t) & P2M_BROKEN_TYPES) #define p2m_is_foreign(_t) (p2m_to_mask(_t) & p2m_to_mask(p2m_map_foreign)) +#define p2m_is_any_ram(_t) (p2m_to_mask(_t) & \ + (P2M_RAM_TYPES | P2M_GRANT_TYPES | \ + p2m_to_mask(p2m_map_foreign))) + /* Per-p2m-table state */ struct p2m_domain { /* Lock that protects updates to the p2m */ @@ -532,6 +536,9 @@ int p2m_is_logdirty_range(struct p2m_domain *, unsigned long start, int set_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn); int clear_mmio_p2m_entry(struct domain *d, unsigned long gfn); +/* Add foreign mapping to the guest's p2m table. */ +int p2m_add_foreign(struct domain *tdom, unsigned long fgfn, + unsigned long gpfn, domid_t foreign_domid); /* * Populate-on-demand -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [V15 PATCH 1/2] pvh dom0: Add and remove foreign pages 2014-05-22 23:30 ` [V15 PATCH 1/2] pvh dom0: Add and remove foreign pages Mukesh Rathor @ 2014-05-23 7:20 ` Jan Beulich 2014-05-23 19:05 ` Tim Deegan 1 sibling, 0 replies; 14+ messages in thread From: Jan Beulich @ 2014-05-23 7:20 UTC (permalink / raw) To: Mukesh Rathor Cc: George.Dunlap, tim, eddie.dong, keir.xen, jun.nakajima, xen-devel >>> On 23.05.14 at 01:30, <mukesh.rathor@oracle.com> wrote: > @@ -688,10 +754,12 @@ ept_set_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn, > ept_p2m_type_to_flags(&new_entry, p2mt, p2ma); > } > > - atomic_write_ept_entry(ept_entry, new_entry); > + rc = atomic_write_ept_entry(ept_entry, new_entry, target); > + if ( rc ) This should have unlikely() ... > + old_entry.epte = 0; > > /* Track the highest gfn for which we have ever had a valid mapping */ > - if ( p2mt != p2m_invalid && > + if ( rc == 0 && p2mt != p2m_invalid && ... and this would then better be "else if()". Both of which could be adjusted while committing. Beyond that all that's left is to have Tim's approval. Jan ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [V15 PATCH 1/2] pvh dom0: Add and remove foreign pages 2014-05-22 23:30 ` [V15 PATCH 1/2] pvh dom0: Add and remove foreign pages Mukesh Rathor 2014-05-23 7:20 ` Jan Beulich @ 2014-05-23 19:05 ` Tim Deegan 2014-05-23 22:37 ` Mukesh Rathor 1 sibling, 1 reply; 14+ messages in thread From: Tim Deegan @ 2014-05-23 19:05 UTC (permalink / raw) To: Mukesh Rathor Cc: JBeulich, George.Dunlap, eddie.dong, keir.xen, jun.nakajima, xen-devel At 16:30 -0700 on 22 May (1400772630), Mukesh Rathor wrote: > In this patch, a new function, p2m_add_foreign(), is added > to map pages from a foreign guest into dom0 for various purposes > like domU creation, running xentrace, etc... Such pages are > typed p2m_map_foreign. Note, it is the nature of such pages > that a refcnt is held during their stay in the p2m. The > refcnt is added and released in the low level ept function > atomic_write_ept_entry. That macro is converted to a function to allow > for such refcounting, which only applies to leaf entries in the ept. > Furthermore, please note that paging/sharing is disabled if the > controlling or hardware domain is pvh. Any enabling of those features > would need to ensure refcnt are properly maintained for foreign types, > or paging/sharing is skipped for foreign types. > > Also, we change get_pg_owner so it allows foreign mappings for pvh. But you no longer actually call get_pg_owner() for PVH domains, right? So that hunk should go away. With that done, Reviewed-by: Tim Deegan <tim@xen.org> Cheers, Tim. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [V15 PATCH 1/2] pvh dom0: Add and remove foreign pages 2014-05-23 19:05 ` Tim Deegan @ 2014-05-23 22:37 ` Mukesh Rathor 2014-05-23 23:08 ` Tim Deegan 0 siblings, 1 reply; 14+ messages in thread From: Mukesh Rathor @ 2014-05-23 22:37 UTC (permalink / raw) To: Tim Deegan Cc: JBeulich, George.Dunlap, eddie.dong, keir.xen, jun.nakajima, xen-devel On Fri, 23 May 2014 21:05:34 +0200 Tim Deegan <tim@xen.org> wrote: > At 16:30 -0700 on 22 May (1400772630), Mukesh Rathor wrote: > > In this patch, a new function, p2m_add_foreign(), is added > > to map pages from a foreign guest into dom0 for various purposes > > like domU creation, running xentrace, etc... Such pages are > > typed p2m_map_foreign. Note, it is the nature of such pages > > that a refcnt is held during their stay in the p2m. The > > refcnt is added and released in the low level ept function > > atomic_write_ept_entry. That macro is converted to a function to > > allow for such refcounting, which only applies to leaf entries in > > the ept. Furthermore, please note that paging/sharing is disabled > > if the controlling or hardware domain is pvh. Any enabling of those > > features would need to ensure refcnt are properly maintained for > > foreign types, or paging/sharing is skipped for foreign types. > > > > Also, we change get_pg_owner so it allows foreign mappings for pvh. > > But you no longer actually call get_pg_owner() for PVH domains, right? > So that hunk should go away. With that done, Hi Tim, We actually need get_pg_owner for the mmuext call by the toolstack when building a PV domain, doing pinning operations on the guest table. Alternates would be to call rcu directly in do_mmuext_op: if ( pvh ) if ( DOMID_SELF ) rcu_lock_current_domain else if ( DOMID_IO || DOMID_XEN ) error else rcu_lock_domain_by_id else get_pg_owner OR, if you prefer in get_pg_owner(): .. case DOMID_IO: if ( !is_pvh_domain(curr) ) pg_owner = rcu_lock_domain(dom_io); break; ... Please lmk asap: leave as is, first suggestion above, or second suggestion above. thanks a lot, mukesh ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [V15 PATCH 1/2] pvh dom0: Add and remove foreign pages 2014-05-23 22:37 ` Mukesh Rathor @ 2014-05-23 23:08 ` Tim Deegan 2014-05-23 23:50 ` Mukesh Rathor 0 siblings, 1 reply; 14+ messages in thread From: Tim Deegan @ 2014-05-23 23:08 UTC (permalink / raw) To: Mukesh Rathor Cc: JBeulich, George.Dunlap, eddie.dong, keir.xen, jun.nakajima, xen-devel At 15:37 -0700 on 23 May (1400855820), Mukesh Rathor wrote: > On Fri, 23 May 2014 21:05:34 +0200 > Tim Deegan <tim@xen.org> wrote: > > > At 16:30 -0700 on 22 May (1400772630), Mukesh Rathor wrote: > > > In this patch, a new function, p2m_add_foreign(), is added > > > to map pages from a foreign guest into dom0 for various purposes > > > like domU creation, running xentrace, etc... Such pages are > > > typed p2m_map_foreign. Note, it is the nature of such pages > > > that a refcnt is held during their stay in the p2m. The > > > refcnt is added and released in the low level ept function > > > atomic_write_ept_entry. That macro is converted to a function to > > > allow for such refcounting, which only applies to leaf entries in > > > the ept. Furthermore, please note that paging/sharing is disabled > > > if the controlling or hardware domain is pvh. Any enabling of those > > > features would need to ensure refcnt are properly maintained for > > > foreign types, or paging/sharing is skipped for foreign types. > > > > > > Also, we change get_pg_owner so it allows foreign mappings for pvh. > > > > But you no longer actually call get_pg_owner() for PVH domains, right? > > So that hunk should go away. With that done, > > Hi Tim, > > We actually need get_pg_owner for the mmuext call by the toolstack > when building a PV domain, doing pinning operations on the guest table. Ah, I see. Let's handle that in a separate patch then, since it's now unrelated to foreign mappings in PVH any more. Having the change where it is seems fine, but I think the correct test is (is_pv() && paging_mode_translate()) rather than (!is_pvh() && paging_mode_translate()) -- it's a weakness of the PV pagetable ops that's being avoided here, rather than any special treatment for PVH. Cheers, Tim. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [V15 PATCH 1/2] pvh dom0: Add and remove foreign pages 2014-05-23 23:08 ` Tim Deegan @ 2014-05-23 23:50 ` Mukesh Rathor 2014-05-26 9:24 ` Jan Beulich 2014-05-28 10:06 ` Tim Deegan 0 siblings, 2 replies; 14+ messages in thread From: Mukesh Rathor @ 2014-05-23 23:50 UTC (permalink / raw) To: Tim Deegan Cc: JBeulich, George.Dunlap, eddie.dong, keir.xen, jun.nakajima, xen-devel On Sat, 24 May 2014 01:08:49 +0200 Tim Deegan <tim@xen.org> wrote: > At 15:37 -0700 on 23 May (1400855820), Mukesh Rathor wrote: > > On Fri, 23 May 2014 21:05:34 +0200 > > Tim Deegan <tim@xen.org> wrote: > > > > > At 16:30 -0700 on 22 May (1400772630), Mukesh Rathor wrote: > > > > In this patch, a new function, p2m_add_foreign(), is added > > > > to map pages from a foreign guest into dom0 for various purposes > > > > like domU creation, running xentrace, etc... Such pages are > > > > typed p2m_map_foreign. Note, it is the nature of such pages > > > > that a refcnt is held during their stay in the p2m. The > > > > refcnt is added and released in the low level ept function > > > > atomic_write_ept_entry. That macro is converted to a function to > > > > allow for such refcounting, which only applies to leaf entries > > > > in the ept. Furthermore, please note that paging/sharing is > > > > disabled if the controlling or hardware domain is pvh. Any > > > > enabling of those features would need to ensure refcnt are > > > > properly maintained for foreign types, or paging/sharing is > > > > skipped for foreign types. > > > > > > > > Also, we change get_pg_owner so it allows foreign mappings for > > > > pvh. > > > > > > But you no longer actually call get_pg_owner() for PVH domains, > > > right? So that hunk should go away. With that done, > > > > Hi Tim, > > > > We actually need get_pg_owner for the mmuext call by the toolstack > > when building a PV domain, doing pinning operations on the guest > > table. > > Ah, I see. Let's handle that in a separate patch then, since it's > now unrelated to foreign mappings in PVH any more. > > Having the change where it is seems fine, but I think the correct test > is (is_pv() && paging_mode_translate()) rather than (!is_pvh() && > paging_mode_translate()) -- it's a weakness of the PV pagetable ops > that's being avoided here, rather than any special treatment for PVH. Good point, but Jan had a concern on that when I had dropped the if statement completely, that it would allow HVM guests to go thru. Hence !is_pvh to let hvm guest continue to fail. thanks, mukesh ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [V15 PATCH 1/2] pvh dom0: Add and remove foreign pages 2014-05-23 23:50 ` Mukesh Rathor @ 2014-05-26 9:24 ` Jan Beulich 2014-05-28 0:51 ` Mukesh Rathor 2014-05-28 10:06 ` Tim Deegan 1 sibling, 1 reply; 14+ messages in thread From: Jan Beulich @ 2014-05-26 9:24 UTC (permalink / raw) To: Mukesh Rathor, Tim Deegan Cc: George.Dunlap, xen-devel, keir.xen, eddie.dong, jun.nakajima >>> On 24.05.14 at 01:50, <mukesh.rathor@oracle.com> wrote: > On Sat, 24 May 2014 01:08:49 +0200 > Tim Deegan <tim@xen.org> wrote: > >> At 15:37 -0700 on 23 May (1400855820), Mukesh Rathor wrote: >> > On Fri, 23 May 2014 21:05:34 +0200 >> > Tim Deegan <tim@xen.org> wrote: >> > >> > > At 16:30 -0700 on 22 May (1400772630), Mukesh Rathor wrote: >> > > > In this patch, a new function, p2m_add_foreign(), is added >> > > > to map pages from a foreign guest into dom0 for various purposes >> > > > like domU creation, running xentrace, etc... Such pages are >> > > > typed p2m_map_foreign. Note, it is the nature of such pages >> > > > that a refcnt is held during their stay in the p2m. The >> > > > refcnt is added and released in the low level ept function >> > > > atomic_write_ept_entry. That macro is converted to a function to >> > > > allow for such refcounting, which only applies to leaf entries >> > > > in the ept. Furthermore, please note that paging/sharing is >> > > > disabled if the controlling or hardware domain is pvh. Any >> > > > enabling of those features would need to ensure refcnt are >> > > > properly maintained for foreign types, or paging/sharing is >> > > > skipped for foreign types. >> > > > >> > > > Also, we change get_pg_owner so it allows foreign mappings for >> > > > pvh. >> > > >> > > But you no longer actually call get_pg_owner() for PVH domains, >> > > right? So that hunk should go away. With that done, >> > >> > Hi Tim, >> > >> > We actually need get_pg_owner for the mmuext call by the toolstack >> > when building a PV domain, doing pinning operations on the guest >> > table. >> >> Ah, I see. Let's handle that in a separate patch then, since it's >> now unrelated to foreign mappings in PVH any more. >> >> Having the change where it is seems fine, but I think the correct test >> is (is_pv() && paging_mode_translate()) rather than (!is_pvh() && >> paging_mode_translate()) -- it's a weakness of the PV pagetable ops >> that's being avoided here, rather than any special treatment for PVH. > > Good point, but Jan had a concern on that when I had dropped the if > statement completely, that it would allow HVM guests to go thru. > Hence !is_pvh to let hvm guest continue to fail. The same would be achieved by using is_pv as Tim suggested. Jan ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [V15 PATCH 1/2] pvh dom0: Add and remove foreign pages 2014-05-26 9:24 ` Jan Beulich @ 2014-05-28 0:51 ` Mukesh Rathor 2014-05-28 6:50 ` Jan Beulich 0 siblings, 1 reply; 14+ messages in thread From: Mukesh Rathor @ 2014-05-28 0:51 UTC (permalink / raw) To: Jan Beulich Cc: George.Dunlap, Tim Deegan, eddie.dong, keir.xen, jun.nakajima, xen-devel On Mon, 26 May 2014 10:24:01 +0100 "Jan Beulich" <JBeulich@suse.com> wrote: > >>> On 24.05.14 at 01:50, <mukesh.rathor@oracle.com> wrote: > > On Sat, 24 May 2014 01:08:49 +0200 > > Tim Deegan <tim@xen.org> wrote: > > > >> At 15:37 -0700 on 23 May (1400855820), Mukesh Rathor wrote: > >> > On Fri, 23 May 2014 21:05:34 +0200 > >> > Tim Deegan <tim@xen.org> wrote: > >> > > >> > > At 16:30 -0700 on 22 May (1400772630), Mukesh Rathor wrote: > >> > > > In this patch, a new function, p2m_add_foreign(), is added > >> > > > to map pages from a foreign guest into dom0 for various > >> > > > purposes like domU creation, running xentrace, etc... Such > >> > > > pages are typed p2m_map_foreign. Note, it is the nature of > >> > > > such pages that a refcnt is held during their stay in the > >> > > > p2m. The refcnt is added and released in the low level ept > >> > > > function atomic_write_ept_entry. That macro is converted to > >> > > > a function to allow for such refcounting, which only applies > >> > > > to leaf entries in the ept. Furthermore, please note that > >> > > > paging/sharing is disabled if the controlling or hardware > >> > > > domain is pvh. Any enabling of those features would need to > >> > > > ensure refcnt are properly maintained for foreign types, or > >> > > > paging/sharing is skipped for foreign types. > >> > > > > >> > > > Also, we change get_pg_owner so it allows foreign mappings > >> > > > for pvh. > >> > > > >> > > But you no longer actually call get_pg_owner() for PVH domains, > >> > > right? So that hunk should go away. With that done, > >> > > >> > Hi Tim, > >> > > >> > We actually need get_pg_owner for the mmuext call by the > >> > toolstack when building a PV domain, doing pinning operations on > >> > the guest table. > >> > >> Ah, I see. Let's handle that in a separate patch then, since it's > >> now unrelated to foreign mappings in PVH any more. > >> > >> Having the change where it is seems fine, but I think the correct > >> test is (is_pv() && paging_mode_translate()) rather than > >> (!is_pvh() && paging_mode_translate()) -- it's a weakness of the > >> PV pagetable ops that's being avoided here, rather than any > >> special treatment for PVH. > > > > Good point, but Jan had a concern on that when I had dropped the if > > statement completely, that it would allow HVM guests to go thru. > > Hence !is_pvh to let hvm guest continue to fail. > > The same would be achieved by using is_pv as Tim suggested. So sorry, but I don't understand how: if ( is_pv_domain(curr) && unlikely(paging_mode_translate(curr)) ) { MEM_LOG("Cannot mix foreign mappings with translated domains"); goto out; } will cause this error for hvm, which is what happens now without my change, or will continue to with my proposed change: if ( !is_pvh_domain(curr) && unlikely(paging_mode_translate(curr)) ) { MEM_LOG("Cannot mix foreign mappings with translated domains"); .. I understand your suggestion earlier was that hvm should continue to fail. Also, my understanding is that pv domains are never translated? thanks mukesh ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [V15 PATCH 1/2] pvh dom0: Add and remove foreign pages 2014-05-28 0:51 ` Mukesh Rathor @ 2014-05-28 6:50 ` Jan Beulich 2014-05-28 10:40 ` Tim Deegan 0 siblings, 1 reply; 14+ messages in thread From: Jan Beulich @ 2014-05-28 6:50 UTC (permalink / raw) To: Mukesh Rathor Cc: George.Dunlap, Tim Deegan, eddie.dong, keir.xen, jun.nakajima, xen-devel >>> On 28.05.14 at 02:51, <mukesh.rathor@oracle.com> wrote: > On Mon, 26 May 2014 10:24:01 +0100 > "Jan Beulich" <JBeulich@suse.com> wrote: >> >>> On 24.05.14 at 01:50, <mukesh.rathor@oracle.com> wrote: >> > On Sat, 24 May 2014 01:08:49 +0200 >> > Tim Deegan <tim@xen.org> wrote: >> >> Having the change where it is seems fine, but I think the correct >> >> test is (is_pv() && paging_mode_translate()) rather than >> >> (!is_pvh() && paging_mode_translate()) -- it's a weakness of the >> >> PV pagetable ops that's being avoided here, rather than any >> >> special treatment for PVH. >> > >> > Good point, but Jan had a concern on that when I had dropped the if >> > statement completely, that it would allow HVM guests to go thru. >> > Hence !is_pvh to let hvm guest continue to fail. >> >> The same would be achieved by using is_pv as Tim suggested. > > So sorry, but I don't understand how: > > if ( is_pv_domain(curr) && unlikely(paging_mode_translate(curr)) ) > { > MEM_LOG("Cannot mix foreign mappings with translated domains"); > goto out; > } > > will cause this error for hvm, which is what happens now without my > change, or will continue to with my proposed change: > > if ( !is_pvh_domain(curr) && unlikely(paging_mode_translate(curr)) ) > { > MEM_LOG("Cannot mix foreign mappings with translated domains"); > .. > > > I understand your suggestion earlier was that hvm should continue to fail. > > Also, my understanding is that pv domains are never translated? Hmm, indeed, good point. I wonder whether the whole condition shouldn't simply become !is_hvm() then (and perhaps the log message adjusted accordingly). Tim? Jan ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [V15 PATCH 1/2] pvh dom0: Add and remove foreign pages 2014-05-28 6:50 ` Jan Beulich @ 2014-05-28 10:40 ` Tim Deegan 0 siblings, 0 replies; 14+ messages in thread From: Tim Deegan @ 2014-05-28 10:40 UTC (permalink / raw) To: Jan Beulich; +Cc: George.Dunlap, eddie.dong, keir.xen, jun.nakajima, xen-devel At 07:50 +0100 on 28 May (1401259810), Jan Beulich wrote: > >>> On 28.05.14 at 02:51, <mukesh.rathor@oracle.com> wrote: > > On Mon, 26 May 2014 10:24:01 +0100 > > "Jan Beulich" <JBeulich@suse.com> wrote: > >> >>> On 24.05.14 at 01:50, <mukesh.rathor@oracle.com> wrote: > >> > On Sat, 24 May 2014 01:08:49 +0200 > >> > Tim Deegan <tim@xen.org> wrote: > >> >> Having the change where it is seems fine, but I think the correct > >> >> test is (is_pv() && paging_mode_translate()) rather than > >> >> (!is_pvh() && paging_mode_translate()) -- it's a weakness of the > >> >> PV pagetable ops that's being avoided here, rather than any > >> >> special treatment for PVH. > >> > > >> > Good point, but Jan had a concern on that when I had dropped the if > >> > statement completely, that it would allow HVM guests to go thru. > >> > Hence !is_pvh to let hvm guest continue to fail. > >> > >> The same would be achieved by using is_pv as Tim suggested. > > > > So sorry, but I don't understand how: > > > > if ( is_pv_domain(curr) && unlikely(paging_mode_translate(curr)) ) > > { > > MEM_LOG("Cannot mix foreign mappings with translated domains"); > > goto out; > > } > > > > will cause this error for hvm, which is what happens now without my > > change, or will continue to with my proposed change: > > > > if ( !is_pvh_domain(curr) && unlikely(paging_mode_translate(curr)) ) > > { > > MEM_LOG("Cannot mix foreign mappings with translated domains"); > > .. > > > > > > I understand your suggestion earlier was that hvm should continue to fail. > > > > Also, my understanding is that pv domains are never translated? > > Hmm, indeed, good point. I wonder whether the whole condition > shouldn't simply become !is_hvm() then (and perhaps the log > message adjusted accordingly). Tim? I'll have to trawl the history to see where this came from, but AFAIK the worry is for old-style PV+translate, where if we let a guest put foreign mappings directly in its pagetables they would fail p2m lookups in the shadow code when trying to translate them. So we could: - check (is_pv && paging_mode_translate), as I suggested before; or - check (paging_mode_translate && !paging_mode_refcounts), which is a bit more idiomatic and more closely describes the problem; or - not check at all, since PVH has entirely replaced even the partial support for PV+translate that was there before. In any case, I don't think we should be using is_pvh or is_hvm here, since they've got effectively the same memory management. But I could live with it as an intermediate measure to defer checking e.g. shadow pagetable support... For further confusion, get_pg_owner is really for foreign mappings but is reused on the mmuext op path, where the caller is adjusting the foreign domain's pagetables and not its own. There this test is pointless since the state of the _caller's_ pagetables doesn't matter. And _that's_ the case that Mukesh needs to relax for PVH dom0. So it would also be OK (if a bit ugly) to move this check into the other two callers of the function and just not have it for mmuext_op. Tim. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [V15 PATCH 1/2] pvh dom0: Add and remove foreign pages 2014-05-23 23:50 ` Mukesh Rathor 2014-05-26 9:24 ` Jan Beulich @ 2014-05-28 10:06 ` Tim Deegan 2014-05-28 23:41 ` Mukesh Rathor 1 sibling, 1 reply; 14+ messages in thread From: Tim Deegan @ 2014-05-28 10:06 UTC (permalink / raw) To: Mukesh Rathor Cc: JBeulich, George.Dunlap, eddie.dong, keir.xen, jun.nakajima, xen-devel At 16:50 -0700 on 23 May (1400860212), Mukesh Rathor wrote: > On Sat, 24 May 2014 01:08:49 +0200 > Tim Deegan <tim@xen.org> wrote: > > > At 15:37 -0700 on 23 May (1400855820), Mukesh Rathor wrote: > > > On Fri, 23 May 2014 21:05:34 +0200 > > > Tim Deegan <tim@xen.org> wrote: > > > > > > > At 16:30 -0700 on 22 May (1400772630), Mukesh Rathor wrote: > > > > > In this patch, a new function, p2m_add_foreign(), is added > > > > > to map pages from a foreign guest into dom0 for various purposes > > > > > like domU creation, running xentrace, etc... Such pages are > > > > > typed p2m_map_foreign. Note, it is the nature of such pages > > > > > that a refcnt is held during their stay in the p2m. The > > > > > refcnt is added and released in the low level ept function > > > > > atomic_write_ept_entry. That macro is converted to a function to > > > > > allow for such refcounting, which only applies to leaf entries > > > > > in the ept. Furthermore, please note that paging/sharing is > > > > > disabled if the controlling or hardware domain is pvh. Any > > > > > enabling of those features would need to ensure refcnt are > > > > > properly maintained for foreign types, or paging/sharing is > > > > > skipped for foreign types. > > > > > > > > > > Also, we change get_pg_owner so it allows foreign mappings for > > > > > pvh. > > > > > > > > But you no longer actually call get_pg_owner() for PVH domains, > > > > right? So that hunk should go away. With that done, > > > > > > Hi Tim, > > > > > > We actually need get_pg_owner for the mmuext call by the toolstack > > > when building a PV domain, doing pinning operations on the guest > > > table. > > > > Ah, I see. Let's handle that in a separate patch then, since it's > > now unrelated to foreign mappings in PVH any more. > > > > Having the change where it is seems fine, but I think the correct test > > is (is_pv() && paging_mode_translate()) rather than (!is_pvh() && > > paging_mode_translate()) -- it's a weakness of the PV pagetable ops > > that's being avoided here, rather than any special treatment for PVH. > > Good point, but Jan had a concern on that when I had dropped the if > statement completely, that it would allow HVM guests to go thru. > Hence !is_pvh to let hvm guest continue to fail. Well, in that case I don't insist on it. I'll look at it again as part of the PVH->HVM merge. Tim. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [V15 PATCH 1/2] pvh dom0: Add and remove foreign pages 2014-05-28 10:06 ` Tim Deegan @ 2014-05-28 23:41 ` Mukesh Rathor 0 siblings, 0 replies; 14+ messages in thread From: Mukesh Rathor @ 2014-05-28 23:41 UTC (permalink / raw) To: Tim Deegan Cc: JBeulich, George.Dunlap, eddie.dong, keir.xen, jun.nakajima, xen-devel On Wed, 28 May 2014 12:06:35 +0200 Tim Deegan <tim@xen.org> wrote: > At 16:50 -0700 on 23 May (1400860212), Mukesh Rathor wrote: > > On Sat, 24 May 2014 01:08:49 +0200 > > Tim Deegan <tim@xen.org> wrote: > > > > > At 15:37 -0700 on 23 May (1400855820), Mukesh Rathor wrote: > > > > On Fri, 23 May 2014 21:05:34 +0200 > > > > Tim Deegan <tim@xen.org> wrote: > > > > > > > > > At 16:30 -0700 on 22 May (1400772630), Mukesh Rathor wrote: > > > > > > In this patch, a new function, p2m_add_foreign(), is added > > > > > > to map pages from a foreign guest into dom0 for various > > > > > > purposes like domU creation, running xentrace, etc... Such > > > > > > pages are typed p2m_map_foreign. Note, it is the nature of > > > > > > such pages that a refcnt is held during their stay in the > > > > > > p2m. The refcnt is added and released in the low level ept > > > > > > function atomic_write_ept_entry. That macro is converted to > > > > > > a function to allow for such refcounting, which only > > > > > > applies to leaf entries in the ept. Furthermore, please > > > > > > note that paging/sharing is disabled if the controlling or > > > > > > hardware domain is pvh. Any enabling of those features > > > > > > would need to ensure refcnt are properly maintained for > > > > > > foreign types, or paging/sharing is skipped for foreign > > > > > > types. > > > > > > > > > > > > Also, we change get_pg_owner so it allows foreign mappings > > > > > > for pvh. > > > > > > > > > > But you no longer actually call get_pg_owner() for PVH > > > > > domains, right? So that hunk should go away. With that done, > > > > > > > > Hi Tim, > > > > > > > > We actually need get_pg_owner for the mmuext call by the > > > > toolstack when building a PV domain, doing pinning operations > > > > on the guest table. > > > > > > Ah, I see. Let's handle that in a separate patch then, since it's > > > now unrelated to foreign mappings in PVH any more. > > > > > > Having the change where it is seems fine, but I think the correct > > > test is (is_pv() && paging_mode_translate()) rather than > > > (!is_pvh() && paging_mode_translate()) -- it's a weakness of the > > > PV pagetable ops that's being avoided here, rather than any > > > special treatment for PVH. > > > > Good point, but Jan had a concern on that when I had dropped the if > > statement completely, that it would allow HVM guests to go thru. > > Hence !is_pvh to let hvm guest continue to fail. > > Well, in that case I don't insist on it. I'll look at it again as > part of the PVH->HVM merge. I think that would be better, as there are several options you mentioned in the other email, and some of it is cleanup not pvh related. For now I'll just add the !is_pvh check, so that hvm guests will continue to be restricted. Please ack the patch in V16 on its way. If you prefer ( is_pv && paging) instead, I don't mind you just changing it during commit either, if hvm going thru is not an issue. thanks Mukesh ^ permalink raw reply [flat|nested] 14+ messages in thread
* [V15 PATCH 2/2] dom0: add opt_dom0pvh to setup.c 2014-05-22 23:30 [V15 PATCH 0/2] pvh dom0 patches Mukesh Rathor 2014-05-22 23:30 ` [V15 PATCH 1/2] pvh dom0: Add and remove foreign pages Mukesh Rathor @ 2014-05-22 23:30 ` Mukesh Rathor 1 sibling, 0 replies; 14+ messages in thread From: Mukesh Rathor @ 2014-05-22 23:30 UTC (permalink / raw) To: xen-devel; +Cc: George.Dunlap, keir.xen, tim, JBeulich Finally last patch in the series to enable creation of pvh dom0. A pvh dom0 is created by adding dom0pvh to grub xen command line. Signed-off-by: Mukesh Rathor <mukesh.rathor@oracle.com> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> --- docs/misc/pvh-readme.txt | 2 ++ docs/misc/xen-command-line.markdown | 7 +++++++ xen/arch/x86/setup.c | 11 +++++++++-- 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/docs/misc/pvh-readme.txt b/docs/misc/pvh-readme.txt index 9fea137..c5b3de4 100644 --- a/docs/misc/pvh-readme.txt +++ b/docs/misc/pvh-readme.txt @@ -37,6 +37,8 @@ supported. Phase I patches are broken into three parts: - tools changes for creating a PVH guest - boot of 64bit dom0 in PVH mode. +To boot 64bit dom0 in PVH mode, add dom0pvh to grub xen command line. + Following fixme's exist in the code: - arch/x86/time.c: support more tsc modes. diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown index a7ac53d..b45ba7e 100644 --- a/docs/misc/xen-command-line.markdown +++ b/docs/misc/xen-command-line.markdown @@ -494,6 +494,13 @@ Practices](http://wiki.xen.org/wiki/Xen_Best_Practices#Xen_dom0_dedicated_memory Pin dom0 vcpus to their respective pcpus +### dom0pvh +> `= <boolean>` + +> Default: `false` + +Flag that makes a 64bit dom0 boot in PVH mode. No 32bit support at present. + ### e820-mtrr-clip > `= <boolean>` diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index 508649d..d7df750 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -65,6 +65,10 @@ invbool_param("smep", disable_smep); static bool_t __initdata disable_smap; invbool_param("smap", disable_smap); +/* Boot dom0 in pvh mode */ +static bool_t __initdata opt_dom0pvh; +boolean_param("dom0pvh", opt_dom0pvh); + /* **** Linux config option: propagated to domain0. */ /* "acpi=off": Sisables both ACPI table parsing and interpreter. */ /* "acpi=force": Override the disable blacklist. */ @@ -539,7 +543,7 @@ void __init noreturn __start_xen(unsigned long mbi_p) { char *memmap_type = NULL; char *cmdline, *kextra, *loader; - unsigned int initrdidx; + unsigned int initrdidx, domcr_flags = DOMCRF_s3_integrity; multiboot_info_t *mbi = __va(mbi_p); module_t *mod = (module_t *)__va(mbi->mods_addr); unsigned long nr_pages, raw_max_page, modules_headroom, *module_map; @@ -1345,8 +1349,11 @@ void __init noreturn __start_xen(unsigned long mbi_p) if ( !tboot_protect_mem_regions() ) panic("Could not protect TXT memory regions"); + if ( opt_dom0pvh ) + domcr_flags |= DOMCRF_pvh | DOMCRF_hap; + /* Create initial domain 0. */ - dom0 = domain_create(0, DOMCRF_s3_integrity, 0); + dom0 = domain_create(0, domcr_flags, 0); if ( IS_ERR(dom0) || (alloc_dom0_vcpu0(dom0) == NULL) ) panic("Error creating domain 0"); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
end of thread, other threads:[~2014-05-28 23:41 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-05-22 23:30 [V15 PATCH 0/2] pvh dom0 patches Mukesh Rathor 2014-05-22 23:30 ` [V15 PATCH 1/2] pvh dom0: Add and remove foreign pages Mukesh Rathor 2014-05-23 7:20 ` Jan Beulich 2014-05-23 19:05 ` Tim Deegan 2014-05-23 22:37 ` Mukesh Rathor 2014-05-23 23:08 ` Tim Deegan 2014-05-23 23:50 ` Mukesh Rathor 2014-05-26 9:24 ` Jan Beulich 2014-05-28 0:51 ` Mukesh Rathor 2014-05-28 6:50 ` Jan Beulich 2014-05-28 10:40 ` Tim Deegan 2014-05-28 10:06 ` Tim Deegan 2014-05-28 23:41 ` Mukesh Rathor 2014-05-22 23:30 ` [V15 PATCH 2/2] dom0: add opt_dom0pvh to setup.c Mukesh Rathor
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).