From nobody Tue Nov 11 08:34:40 2025 Delivered-To: importer@patchew.org Received-SPF: none (zoho.com: 192.237.175.120 is neither permitted nor denied by domain of lists.xenproject.org) client-ip=192.237.175.120; envelope-from=xen-devel-bounces@lists.xenproject.org; helo=lists.xenproject.org; Authentication-Results: mx.zohomail.com; spf=none (zoho.com: 192.237.175.120 is neither permitted nor denied by domain of lists.xenproject.org) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=fail(p=none dis=none) header.from=intel.com ARC-Seal: i=1; a=rsa-sha256; t=1569426662; cv=none; d=zoho.com; s=zohoarc; b=YIRDNGbzwr3fNd9wGbPhnrsOz+XeLuj5X2yagsnLAt7dNBZ5HFdCOIjofHWAIx415m710ohcrV/QkbvHhOYmhvtuWRbN63YGcUnHCy80Ar6Z/uuFmtjln9wUakYA3dGjJeQVSan60Fp8BPCkd+gGodatmjf3hTvT8mHlvKs8tTs= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1569426662; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To:ARC-Authentication-Results; bh=mVCUh11Ow3Wq5XpbA9FpMPhr//xng+BXucIbHV413tk=; b=XPAICpuA2DIC6LRqByLgEY2bZ8O5lRqNEYNJ91BEaZOjKtFqvYtwHHgz8xqk6pqqh47tLduvD0PZqeBzd3AsZfT/T9jsiiHU2S1eWzRt5D8OtlRIexgDLhIN2oYVv4eFJEnow9j/MlIjDeHmvLN+XlGL7W/fyYiYuOjSXzUkZF8= ARC-Authentication-Results: i=1; mx.zoho.com; spf=none (zoho.com: 192.237.175.120 is neither permitted nor denied by domain of lists.xenproject.org) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=fail header.from= (p=none dis=none) header.from= Return-Path: Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) by mx.zohomail.com with SMTPS id 1569426662828556.6907872263521; Wed, 25 Sep 2019 08:51:02 -0700 (PDT) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1iD9Xv-0000pB-H7; Wed, 25 Sep 2019 15:49:39 +0000 Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.89) (envelope-from ) id 1iD9Xt-0000oE-VZ for xen-devel@lists.xenproject.org; Wed, 25 Sep 2019 15:49:38 +0000 Received: from mga12.intel.com (unknown [192.55.52.136]) by localhost (Halon) with ESMTPS id 113e1fba-dfac-11e9-9637-12813bfff9fa; Wed, 25 Sep 2019 15:49:32 +0000 (UTC) Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Sep 2019 08:49:30 -0700 Received: from tlengyel-mobl2.amr.corp.intel.com (HELO localhost.localdomain) ([10.252.129.153]) by orsmga006.jf.intel.com with ESMTP; 25 Sep 2019 08:49:29 -0700 X-Inumbo-ID: 113e1fba-dfac-11e9-9637-12813bfff9fa X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.64,548,1559545200"; d="scan'208";a="193812639" From: Tamas K Lengyel To: xen-devel@lists.xenproject.org Date: Wed, 25 Sep 2019 08:48:42 -0700 Message-Id: X-Mailer: git-send-email 2.20.1 In-Reply-To: References: MIME-Version: 1.0 Subject: [Xen-devel] [RFC PATCH for-next 04/18] x86/mem_sharing: cleanup code in various locations X-BeenThere: xen-devel@lists.xenproject.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: Tamas K Lengyel , Tamas K Lengyel , Wei Liu , George Dunlap , Andrew Cooper , Jan Beulich , =?UTF-8?q?Roger=20Pau=20Monn=C3=A9?= Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Errors-To: xen-devel-bounces@lists.xenproject.org Sender: "Xen-devel" No functional changes. Signed-off-by: Tamas K Lengyel --- xen/arch/x86/hvm/hvm.c | 11 +- xen/arch/x86/mm/mem_sharing.c | 342 +++++++++++++++++------------- xen/arch/x86/mm/p2m.c | 17 +- xen/include/asm-x86/mem_sharing.h | 49 +++-- 4 files changed, 235 insertions(+), 184 deletions(-) diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 667c830db5..d71d2ad5d7 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -1879,12 +1879,11 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned= long gla, if ( npfec.write_access && (p2mt =3D=3D p2m_ram_shared) ) { ASSERT(p2m_is_hostp2m(p2m)); - sharing_enomem =3D=20 - (mem_sharing_unshare_page(currd, gfn, 0) < 0); + sharing_enomem =3D mem_sharing_unshare_page(currd, gfn, 0); rc =3D 1; goto out_put_gfn; } -=20 + /* Spurious fault? PoD and log-dirty also take this path. */ if ( p2m_is_ram(p2mt) ) { @@ -1930,9 +1929,11 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned = long gla, __put_gfn(p2m, gfn); __put_gfn(hostp2m, gfn); out: - /* All of these are delayed until we exit, since we might=20 + /* + * All of these are delayed until we exit, since we might * sleep on event ring wait queues, and we must not hold - * locks in such circumstance */ + * locks in such circumstance. + */ if ( paged ) p2m_mem_paging_populate(currd, gfn); if ( sharing_enomem ) diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c index a5fe89e339..8ad6cf3850 100644 --- a/xen/arch/x86/mm/mem_sharing.c +++ b/xen/arch/x86/mm/mem_sharing.c @@ -59,8 +59,10 @@ static DEFINE_PER_CPU(pg_lock_data_t, __pld); #define RMAP_USES_HASHTAB(page) \ ((page)->sharing->hash_table.flag =3D=3D NULL) #define RMAP_HEAVY_SHARED_PAGE RMAP_HASHTAB_SIZE -/* A bit of hysteresis. We don't want to be mutating between list and hash - * table constantly. */ +/* + * A bit of hysteresis. We don't want to be mutating between list and hash + * table constantly. + */ #define RMAP_LIGHT_SHARED_PAGE (RMAP_HEAVY_SHARED_PAGE >> 2) =20 #if MEM_SHARING_AUDIT @@ -88,7 +90,7 @@ static inline void page_sharing_dispose(struct page_info = *page) { /* Unlikely given our thresholds, but we should be careful. */ if ( unlikely(RMAP_USES_HASHTAB(page)) ) - free_xenheap_pages(page->sharing->hash_table.bucket,=20 + free_xenheap_pages(page->sharing->hash_table.bucket, RMAP_HASHTAB_ORDER); =20 spin_lock(&shr_audit_lock); @@ -105,7 +107,7 @@ static inline void page_sharing_dispose(struct page_inf= o *page) { /* Unlikely given our thresholds, but we should be careful. */ if ( unlikely(RMAP_USES_HASHTAB(page)) ) - free_xenheap_pages(page->sharing->hash_table.bucket,=20 + free_xenheap_pages(page->sharing->hash_table.bucket, RMAP_HASHTAB_ORDER); xfree(page->sharing); } @@ -122,8 +124,8 @@ static inline void page_sharing_dispose(struct page_inf= o *page) * Nesting may happen when sharing (and locking) two pages. * Deadlock is avoided by locking pages in increasing order. * All memory sharing code paths take the p2m lock of the affected gfn bef= ore - * taking the lock for the underlying page. We enforce ordering between pa= ge_lock - * and p2m_lock using an mm-locks.h construct. + * taking the lock for the underlying page. We enforce ordering between + * page_lock and p2m_lock using an mm-locks.h construct. * * TODO: Investigate if PGT_validated is necessary. */ @@ -168,7 +170,7 @@ static inline bool mem_sharing_page_lock(struct page_in= fo *pg) if ( rc ) { preempt_disable(); - page_sharing_mm_post_lock(&pld->mm_unlock_level,=20 + page_sharing_mm_post_lock(&pld->mm_unlock_level, &pld->recurse_count); } return rc; @@ -178,7 +180,7 @@ static inline void mem_sharing_page_unlock(struct page_= info *pg) { pg_lock_data_t *pld =3D &(this_cpu(__pld)); =20 - page_sharing_mm_unlock(pld->mm_unlock_level,=20 + page_sharing_mm_unlock(pld->mm_unlock_level, &pld->recurse_count); preempt_enable(); _page_unlock(pg); @@ -186,7 +188,7 @@ static inline void mem_sharing_page_unlock(struct page_= info *pg) =20 static inline shr_handle_t get_next_handle(void) { - /* Get the next handle get_page style */=20 + /* Get the next handle get_page style */ uint64_t x, y =3D next_handle; do { x =3D y; @@ -198,24 +200,26 @@ static inline shr_handle_t get_next_handle(void) #define mem_sharing_enabled(d) \ (is_hvm_domain(d) && (d)->arch.hvm.mem_sharing_enabled) =20 -static atomic_t nr_saved_mfns =3D ATOMIC_INIT(0);=20 +static atomic_t nr_saved_mfns =3D ATOMIC_INIT(0); static atomic_t nr_shared_mfns =3D ATOMIC_INIT(0); =20 -/** Reverse map **/ -/* Every shared frame keeps a reverse map (rmap) of tuples t= hat +/* + * Reverse map + * + * Every shared frame keeps a reverse map (rmap) of tuples t= hat * this shared frame backs. For pages with a low degree of sharing, a O(n) * search linked list is good enough. For pages with higher degree of shar= ing, - * we use a hash table instead. */ + * we use a hash table instead. + */ =20 typedef struct gfn_info { unsigned long gfn; - domid_t domain;=20 + domid_t domain; struct list_head list; } gfn_info_t; =20 -static inline void -rmap_init(struct page_info *page) +static inline void rmap_init(struct page_info *page) { /* We always start off as a doubly linked list. */ INIT_LIST_HEAD(&page->sharing->gfns); @@ -225,10 +229,11 @@ rmap_init(struct page_info *page) #define HASH(domain, gfn) \ (((gfn) + (domain)) % RMAP_HASHTAB_SIZE) =20 -/* Conversions. Tuned by the thresholds. Should only happen twice=20 - * (once each) during the lifetime of a shared page */ -static inline int -rmap_list_to_hash_table(struct page_info *page) +/* + * Conversions. Tuned by the thresholds. Should only happen twice + * (once each) during the lifetime of a shared page. + */ +static inline int rmap_list_to_hash_table(struct page_info *page) { unsigned int i; struct list_head *pos, *tmp, *b =3D @@ -254,8 +259,7 @@ rmap_list_to_hash_table(struct page_info *page) return 0; } =20 -static inline void -rmap_hash_table_to_list(struct page_info *page) +static inline void rmap_hash_table_to_list(struct page_info *page) { unsigned int i; struct list_head *bucket =3D page->sharing->hash_table.bucket; @@ -276,8 +280,7 @@ rmap_hash_table_to_list(struct page_info *page) } =20 /* Generic accessors to the rmap */ -static inline unsigned long -rmap_count(struct page_info *pg) +static inline unsigned long rmap_count(struct page_info *pg) { unsigned long count; unsigned long t =3D read_atomic(&pg->u.inuse.type_info); @@ -287,11 +290,13 @@ rmap_count(struct page_info *pg) return count; } =20 -/* The page type count is always decreased after removing from the rmap. - * Use a convert flag to avoid mutating the rmap if in the middle of an=20 - * iterator, or if the page will be soon destroyed anyways. */ -static inline void -rmap_del(gfn_info_t *gfn_info, struct page_info *page, int convert) +/* + * The page type count is always decreased after removing from the rmap. + * Use a convert flag to avoid mutating the rmap if in the middle of an + * iterator, or if the page will be soon destroyed anyways. + */ +static inline +void rmap_del(gfn_info_t *gfn_info, struct page_info *page, int convert) { if ( RMAP_USES_HASHTAB(page) && convert && (rmap_count(page) <=3D RMAP_LIGHT_SHARED_PAGE) ) @@ -302,8 +307,7 @@ rmap_del(gfn_info_t *gfn_info, struct page_info *page, = int convert) } =20 /* The page type count is always increased before adding to the rmap. */ -static inline void -rmap_add(gfn_info_t *gfn_info, struct page_info *page) +static inline void rmap_add(gfn_info_t *gfn_info, struct page_info *page) { struct list_head *head; =20 @@ -314,7 +318,7 @@ rmap_add(gfn_info_t *gfn_info, struct page_info *page) (void)rmap_list_to_hash_table(page); =20 head =3D (RMAP_USES_HASHTAB(page)) ? - page->sharing->hash_table.bucket +=20 + page->sharing->hash_table.bucket + HASH(gfn_info->domain, gfn_info->gfn) : &page->sharing->gfns; =20 @@ -322,9 +326,9 @@ rmap_add(gfn_info_t *gfn_info, struct page_info *page) list_add(&gfn_info->list, head); } =20 -static inline gfn_info_t * -rmap_retrieve(uint16_t domain_id, unsigned long gfn,=20 - struct page_info *page) +static inline +gfn_info_t *rmap_retrieve(uint16_t domain_id, unsigned long gfn, + struct page_info *page) { gfn_info_t *gfn_info; struct list_head *le, *head; @@ -364,18 +368,18 @@ struct rmap_iterator { unsigned int bucket; }; =20 -static inline void -rmap_seed_iterator(struct page_info *page, struct rmap_iterator *ri) +static inline +void rmap_seed_iterator(struct page_info *page, struct rmap_iterator *ri) { ri->curr =3D (RMAP_USES_HASHTAB(page)) ? page->sharing->hash_table.bucket : &page->sharing->gfns; - ri->next =3D ri->curr->next;=20 + ri->next =3D ri->curr->next; ri->bucket =3D 0; } =20 -static inline gfn_info_t * -rmap_iterate(struct page_info *page, struct rmap_iterator *ri) +static inline +gfn_info_t *rmap_iterate(struct page_info *page, struct rmap_iterator *ri) { struct list_head *head =3D (RMAP_USES_HASHTAB(page)) ? page->sharing->hash_table.bucket + ri->bucket : @@ -405,14 +409,14 @@ retry: return list_entry(ri->curr, gfn_info_t, list); } =20 -static inline gfn_info_t *mem_sharing_gfn_alloc(struct page_info *page, - struct domain *d, - unsigned long gfn) +static inline +gfn_info_t *mem_sharing_gfn_alloc(struct page_info *page, struct domain *d, + unsigned long gfn) { gfn_info_t *gfn_info =3D xmalloc(gfn_info_t); =20 if ( gfn_info =3D=3D NULL ) - return NULL;=20 + return NULL; =20 gfn_info->gfn =3D gfn; gfn_info->domain =3D d->domain_id; @@ -425,9 +429,9 @@ static inline gfn_info_t *mem_sharing_gfn_alloc(struct = page_info *page, return gfn_info; } =20 -static inline void mem_sharing_gfn_destroy(struct page_info *page, - struct domain *d, - gfn_info_t *gfn_info) +static inline +void mem_sharing_gfn_destroy(struct page_info *page, struct domain *d, + gfn_info_t *gfn_info) { /* Decrement the number of pages. */ atomic_dec(&d->shr_pages); @@ -437,25 +441,29 @@ static inline void mem_sharing_gfn_destroy(struct pag= e_info *page, xfree(gfn_info); } =20 -static struct page_info* mem_sharing_lookup(unsigned long mfn) +static inline struct page_info* mem_sharing_lookup(unsigned long mfn) { - if ( mfn_valid(_mfn(mfn)) ) - { - struct page_info* page =3D mfn_to_page(_mfn(mfn)); - if ( page_get_owner(page) =3D=3D dom_cow ) - { - /* Count has to be at least two, because we're called - * with the mfn locked (1) and this is supposed to be=20 - * a shared page (1). */ - unsigned long t =3D read_atomic(&page->u.inuse.type_info); - ASSERT((t & PGT_type_mask) =3D=3D PGT_shared_page); - ASSERT((t & PGT_count_mask) >=3D 2); - ASSERT(SHARED_M2P(get_gpfn_from_mfn(mfn))); - return page; - } - } + struct page_info* page; + unsigned long t; =20 - return NULL; + if ( !mfn_valid(_mfn(mfn)) ) + return NULL; + + page =3D mfn_to_page(_mfn(mfn)); + if ( page_get_owner(page) !=3D dom_cow ) + return NULL; + + /* + * Count has to be at least two, because we're called + * with the mfn locked (1) and this is supposed to be + * a shared page (1). + */ + t =3D read_atomic(&page->u.inuse.type_info); + ASSERT((t & PGT_type_mask) =3D=3D PGT_shared_page); + ASSERT((t & PGT_count_mask) >=3D 2); + ASSERT(SHARED_M2P(get_gpfn_from_mfn(mfn))); + + return page; } =20 static int audit(void) @@ -492,7 +500,7 @@ static int audit(void) continue; } =20 - /* Check if the MFN has correct type, owner and handle. */=20 + /* Check if the MFN has correct type, owner and handle. */ if ( (pg->u.inuse.type_info & PGT_type_mask) !=3D PGT_shared_page ) { MEM_SHARING_DEBUG("mfn %lx in audit list, but not PGT_shared_pa= ge (%lx)!\n", @@ -545,7 +553,7 @@ static int audit(void) errors++; continue; } - o_mfn =3D get_gfn_query_unlocked(d, g->gfn, &t);=20 + o_mfn =3D get_gfn_query_unlocked(d, g->gfn, &t); if ( !mfn_eq(o_mfn, mfn) ) { MEM_SHARING_DEBUG("Incorrect P2M for d=3D%hu, PFN=3D%lx." @@ -568,7 +576,7 @@ static int audit(void) { MEM_SHARING_DEBUG("Mismatched counts for MFN=3D%lx." "nr_gfns in list %lu, in type_info %lx\n", - mfn_x(mfn), nr_gfns,=20 + mfn_x(mfn), nr_gfns, (pg->u.inuse.type_info & PGT_count_mask)); errors++; } @@ -603,7 +611,7 @@ int mem_sharing_notify_enomem(struct domain *d, unsigne= d long gfn, .u.mem_sharing.p2mt =3D p2m_ram_shared }; =20 - if ( (rc =3D __vm_event_claim_slot(d,=20 + if ( (rc =3D __vm_event_claim_slot(d, d->vm_event_share, allow_sleep)) < 0 ) return rc; =20 @@ -629,9 +637,9 @@ unsigned int mem_sharing_get_nr_shared_mfns(void) } =20 /* Functions that change a page's type and ownership */ -static int page_make_sharable(struct domain *d,=20 - struct page_info *page,=20 - int expected_refcnt) +static int page_make_sharable(struct domain *d, + struct page_info *page, + int expected_refcnt) { bool_t drop_dom_ref; =20 @@ -658,8 +666,10 @@ static int page_make_sharable(struct domain *d, return -EEXIST; } =20 - /* Check if the ref count is 2. The first from PGC_allocated, and - * the second from get_page_and_type at the top of this function */ + /* + * Check if the ref count is 2. The first from PGC_allocated, and + * the second from get_page_and_type at the top of this function. + */ if ( page->count_info !=3D (PGC_allocated | (2 + expected_refcnt)) ) { spin_unlock(&d->page_alloc_lock); @@ -675,6 +685,7 @@ static int page_make_sharable(struct domain *d, =20 if ( drop_dom_ref ) put_domain(d); + return 0; } =20 @@ -684,7 +695,7 @@ static int page_make_private(struct domain *d, struct p= age_info *page) =20 if ( !get_page(page, dom_cow) ) return -EINVAL; - =20 + spin_lock(&d->page_alloc_lock); =20 if ( d->is_dying ) @@ -727,10 +738,13 @@ static inline struct page_info *__grab_shared_page(mf= n_t mfn) =20 if ( !mfn_valid(mfn) ) return NULL; + pg =3D mfn_to_page(mfn); =20 - /* If the page is not validated we can't lock it, and if it's =20 - * not validated it's obviously not shared. */ + /* + * If the page is not validated we can't lock it, and if it's + * not validated it's obviously not shared. + */ if ( !mem_sharing_page_lock(pg) ) return NULL; =20 @@ -754,10 +768,10 @@ static int debug_mfn(mfn_t mfn) return -EINVAL; } =20 - MEM_SHARING_DEBUG(=20 + MEM_SHARING_DEBUG( "Debug page: MFN=3D%lx is ci=3D%lx, ti=3D%lx, owner_id=3D%d\n", - mfn_x(page_to_mfn(page)),=20 - page->count_info,=20 + mfn_x(page_to_mfn(page)), + page->count_info, page->u.inuse.type_info, page_get_owner(page)->domain_id); =20 @@ -775,7 +789,7 @@ static int debug_gfn(struct domain *d, gfn_t gfn) =20 mfn =3D get_gfn_query(d, gfn_x(gfn), &p2mt); =20 - MEM_SHARING_DEBUG("Debug for dom%d, gfn=3D%" PRI_gfn "\n",=20 + MEM_SHARING_DEBUG("Debug for dom%d, gfn=3D%" PRI_gfn "\n", d->domain_id, gfn_x(gfn)); num_refs =3D debug_mfn(mfn); put_gfn(d, gfn_x(gfn)); @@ -796,9 +810,9 @@ static int debug_gref(struct domain *d, grant_ref_t ref) d->domain_id, ref, rc); return rc; } - =20 + MEM_SHARING_DEBUG( - "=3D=3D> Grant [dom=3D%d,ref=3D%d], status=3D%x. ",=20 + "=3D=3D> Grant [dom=3D%d,ref=3D%d], status=3D%x. ", d->domain_id, ref, status); =20 return debug_gfn(d, gfn); @@ -824,15 +838,12 @@ static int nominate_page(struct domain *d, gfn_t gfn, goto out; =20 /* Return the handle if the page is already shared */ - if ( p2m_is_shared(p2mt) ) { + if ( p2m_is_shared(p2mt) ) + { struct page_info *pg =3D __grab_shared_page(mfn); if ( !pg ) - { - gprintk(XENLOG_ERR, - "Shared p2m entry gfn %" PRI_gfn ", but could not grab= mfn %" PRI_mfn " dom%d\n", - gfn_x(gfn), mfn_x(mfn), d->domain_id); BUG(); - } + *phandle =3D pg->sharing->handle; ret =3D 0; mem_sharing_page_unlock(pg); @@ -843,7 +854,6 @@ static int nominate_page(struct domain *d, gfn_t gfn, if ( !p2m_is_sharable(p2mt) ) goto out; =20 -#ifdef CONFIG_HVM /* Check if there are mem_access/remapped altp2m entries for this page= */ if ( altp2m_active(d) ) { @@ -872,42 +882,42 @@ static int nominate_page(struct domain *d, gfn_t gfn, =20 altp2m_list_unlock(d); } -#endif =20 /* Try to convert the mfn to the sharable type */ page =3D mfn_to_page(mfn); - ret =3D page_make_sharable(d, page, expected_refcnt);=20 - if ( ret )=20 + ret =3D page_make_sharable(d, page, expected_refcnt); + if ( ret ) goto out; =20 - /* Now that the page is validated, we can lock it. There is no=20 - * race because we're holding the p2m entry, so no one else=20 - * could be nominating this gfn */ + /* + * Now that the page is validated, we can lock it. There is no + * race because we're holding the p2m entry, so no one else + * could be nominating this gfn. + */ ret =3D -ENOENT; if ( !mem_sharing_page_lock(page) ) goto out; =20 /* Initialize the shared state */ ret =3D -ENOMEM; - if ( (page->sharing =3D=20 - xmalloc(struct page_sharing_info)) =3D=3D NULL ) + if ( !(page->sharing =3D xmalloc(struct page_sharing_info)) ) { /* Making a page private atomically unlocks it */ - BUG_ON(page_make_private(d, page) !=3D 0); + BUG_ON(page_make_private(d, page)); goto out; } page->sharing->pg =3D page; rmap_init(page); =20 /* Create the handle */ - page->sharing->handle =3D get_next_handle(); =20 + page->sharing->handle =3D get_next_handle(); =20 /* Create the local gfn info */ - if ( mem_sharing_gfn_alloc(page, d, gfn_x(gfn)) =3D=3D NULL ) + if ( !mem_sharing_gfn_alloc(page, d, gfn_x(gfn)) ) { xfree(page->sharing); page->sharing =3D NULL; - BUG_ON(page_make_private(d, page) !=3D 0); + BUG_ON(page_make_private(d, page)); goto out; } =20 @@ -946,15 +956,19 @@ static int share_pages(struct domain *sd, gfn_t sgfn,= shr_handle_t sh, get_two_gfns(sd, sgfn, &smfn_type, NULL, &smfn, cd, cgfn, &cmfn_type, NULL, &cmfn, 0, &tg); =20 - /* This tricky business is to avoid two callers deadlocking if=20 - * grabbing pages in opposite client/source order */ + /* + * This tricky business is to avoid two callers deadlocking if + * grabbing pages in opposite client/source order. + */ if ( mfn_eq(smfn, cmfn) ) { - /* The pages are already the same. We could return some + /* + * The pages are already the same. We could return some * kind of error here, but no matter how you look at it, * the pages are already 'shared'. It possibly represents * a big problem somewhere else, but as far as sharing is - * concerned: great success! */ + * concerned: great success! + */ ret =3D 0; goto err_out; } @@ -1010,11 +1024,15 @@ static int share_pages(struct domain *sd, gfn_t sgf= n, shr_handle_t sh, rmap_seed_iterator(cpage, &ri); while ( (gfn =3D rmap_iterate(cpage, &ri)) !=3D NULL) { - /* Get the source page and type, this should never fail:=20 - * we are under shr lock, and got a successful lookup */ + /* + * Get the source page and type, this should never fail: + * we are under shr lock, and got a successful lookup. + */ BUG_ON(!get_page_and_type(spage, dom_cow, PGT_shared_page)); - /* Move the gfn_info from client list to source list. - * Don't change the type of rmap for the client page. */ + /* + * Move the gfn_info from client list to source list. + * Don't change the type of rmap for the client page. + */ rmap_del(gfn, cpage, 0); rmap_add(gfn, spage); put_count++; @@ -1043,14 +1061,14 @@ static int share_pages(struct domain *sd, gfn_t sgf= n, shr_handle_t sh, atomic_dec(&nr_shared_mfns); atomic_inc(&nr_saved_mfns); ret =3D 0; - =20 + err_out: put_two_gfns(&tg); return ret; } =20 int mem_sharing_add_to_physmap(struct domain *sd, unsigned long sgfn, shr_= handle_t sh, - struct domain *cd, unsigned long cgfn)=20 + struct domain *cd, unsigned long cgfn) { struct page_info *spage; int ret =3D -EINVAL; @@ -1069,15 +1087,18 @@ int mem_sharing_add_to_physmap(struct domain *sd, u= nsigned long sgfn, shr_handle spage =3D __grab_shared_page(smfn); if ( spage =3D=3D NULL ) goto err_out; + ASSERT(smfn_type =3D=3D p2m_ram_shared); =20 /* Check that the handles match */ if ( spage->sharing->handle !=3D sh ) goto err_unlock; =20 - /* Make sure the target page is a hole in the physmap. These are typic= ally + /* + * Make sure the target page is a hole in the physmap. These are typic= ally * p2m_mmio_dm, but also accept p2m_invalid and paged out pages. See t= he - * definition of p2m_is_hole in p2m.h. */ + * definition of p2m_is_hole in p2m.h. + */ if ( !p2m_is_hole(cmfn_type) ) { ret =3D XENMEM_SHARING_OP_C_HANDLE_INVALID; @@ -1086,7 +1107,7 @@ int mem_sharing_add_to_physmap(struct domain *sd, uns= igned long sgfn, shr_handle =20 /* This is simpler than regular sharing */ BUG_ON(!get_page_and_type(spage, dom_cow, PGT_shared_page)); - if ( (gfn_info =3D mem_sharing_gfn_alloc(spage, cd, cgfn)) =3D=3D NULL= ) + if ( !(gfn_info =3D mem_sharing_gfn_alloc(spage, cd, cgfn)) ) { put_page_and_type(spage); ret =3D -ENOMEM; @@ -1102,11 +1123,17 @@ int mem_sharing_add_to_physmap(struct domain *sd, u= nsigned long sgfn, shr_handle mem_sharing_gfn_destroy(spage, cd, gfn_info); put_page_and_type(spage); } else { - /* There is a chance we're plugging a hole where a paged out page = was */ + /* + * There is a chance we're plugging a hole where a paged out + * page was. + */ if ( p2m_is_paging(cmfn_type) && (cmfn_type !=3D p2m_ram_paging_ou= t) ) { atomic_dec(&cd->paged_pages); - /* Further, there is a chance this was a valid page. Don't lea= k it. */ + /* + * Further, there is a chance this was a valid page. + * Don't leak it. + */ if ( mfn_valid(cmfn) ) { struct page_info *cpage =3D mfn_to_page(cmfn); @@ -1133,13 +1160,14 @@ err_out: } =20 =20 -/* A note on the rationale for unshare error handling: +/* + * A note on the rationale for unshare error handling: * 1. Unshare can only fail with ENOMEM. Any other error conditions BUG_O= N()'s * 2. We notify a potential dom0 helper through a vm_event ring. But we - * allow the notification to not go to sleep. If the event ring is ful= l=20 + * allow the notification to not go to sleep. If the event ring is full * of ENOMEM warnings, then it's on the ball. * 3. We cannot go to sleep until the unshare is resolved, because we mig= ht - * be buried deep into locks (e.g. something -> copy_to_user -> __hvm_= copy)=20 + * be buried deep into locks (e.g. something -> copy_to_user -> __hvm_= copy) * 4. So, we make sure we: * 4.1. return an error * 4.2. do not corrupt shared memory @@ -1147,19 +1175,20 @@ err_out: * 4.4. let the guest deal with it if the error propagation will reach= it */ int __mem_sharing_unshare_page(struct domain *d, - unsigned long gfn,=20 - uint16_t flags) + unsigned long gfn, + uint16_t flags) { p2m_type_t p2mt; mfn_t mfn; struct page_info *page, *old_page; int last_gfn; gfn_info_t *gfn_info =3D NULL; - =20 + mfn =3D get_gfn(d, gfn, &p2mt); - =20 + /* Has someone already unshared it? */ - if ( !p2m_is_shared(p2mt) ) { + if ( !p2m_is_shared(p2mt) ) + { put_gfn(d, gfn); return 0; } @@ -1167,26 +1196,30 @@ int __mem_sharing_unshare_page(struct domain *d, page =3D __grab_shared_page(mfn); if ( page =3D=3D NULL ) { - gdprintk(XENLOG_ERR, "Domain p2m is shared, but page is not: " - "%lx\n", gfn); + gdprintk(XENLOG_ERR, "Domain p2m is shared, but page is not: %lx\n= ", + gfn); BUG(); } =20 gfn_info =3D rmap_retrieve(d->domain_id, gfn, page); if ( unlikely(gfn_info =3D=3D NULL) ) { - gdprintk(XENLOG_ERR, "Could not find gfn_info for shared gfn: " - "%lx\n", gfn); + gdprintk(XENLOG_ERR, "Could not find gfn_info for shared gfn: %lx\= n", + gfn); BUG(); } =20 - /* Do the accounting first. If anything fails below, we have bigger - * bigger fish to fry. First, remove the gfn from the list. */=20 + /* + * Do the accounting first. If anything fails below, we have bigger + * bigger fish to fry. First, remove the gfn from the list. + */ last_gfn =3D rmap_has_one_entry(page); if ( last_gfn ) { - /* Clean up shared state. Get rid of the tuple - * before destroying the rmap. */ + /* + * Clean up shared state. Get rid of the tuple + * before destroying the rmap. + */ mem_sharing_gfn_destroy(page, d, gfn_info); page_sharing_dispose(page); page->sharing =3D NULL; @@ -1195,8 +1228,10 @@ int __mem_sharing_unshare_page(struct domain *d, else atomic_dec(&nr_saved_mfns); =20 - /* If the GFN is getting destroyed drop the references to MFN=20 - * (possibly freeing the page), and exit early */ + /* + * If the GFN is getting destroyed drop the references to MFN + * (possibly freeing the page), and exit early. + */ if ( flags & MEM_SHARING_DESTROY_GFN ) { if ( !last_gfn ) @@ -1212,7 +1247,7 @@ int __mem_sharing_unshare_page(struct domain *d, =20 return 0; } -=20 + if ( last_gfn ) { /* Making a page private atomically unlocks it */ @@ -1222,14 +1257,16 @@ int __mem_sharing_unshare_page(struct domain *d, =20 old_page =3D page; page =3D alloc_domheap_page(d, 0); - if ( !page )=20 + if ( !page ) { /* Undo dec of nr_saved_mfns, as the retry will decrease again. */ atomic_inc(&nr_saved_mfns); mem_sharing_page_unlock(old_page); put_gfn(d, gfn); - /* Caller is responsible for placing an event - * in the ring */ + /* + * Caller is responsible for placing an event + * in the ring. + */ return -ENOMEM; } =20 @@ -1240,11 +1277,11 @@ int __mem_sharing_unshare_page(struct domain *d, mem_sharing_page_unlock(old_page); put_page_and_type(old_page); =20 -private_page_found: =20 +private_page_found: if ( p2m_change_type_one(d, gfn, p2m_ram_shared, p2m_ram_rw) ) { - gdprintk(XENLOG_ERR, "Could not change p2m type d %hu gfn %lx.\n",=20 - d->domain_id, gfn); + gdprintk(XENLOG_ERR, "Could not change p2m type d %hu gfn %lx.\n", + d->domain_id, gfn); BUG(); } =20 @@ -1277,20 +1314,23 @@ int relinquish_shared_pages(struct domain *d) mfn_t mfn; int set_rc; =20 - if ( atomic_read(&d->shr_pages) =3D=3D 0 ) + if ( !atomic_read(&d->shr_pages) ) break; + mfn =3D p2m->get_entry(p2m, _gfn(gfn), &t, &a, 0, NULL, NULL); - if ( mfn_valid(mfn) && (t =3D=3D p2m_ram_shared) ) + if ( mfn_valid(mfn) && t =3D=3D p2m_ram_shared ) { /* Does not fail with ENOMEM given the DESTROY flag */ - BUG_ON(__mem_sharing_unshare_page(d, gfn,=20 - MEM_SHARING_DESTROY_GFN)); - /* Clear out the p2m entry so no one else may try to + BUG_ON(__mem_sharing_unshare_page(d, gfn, + MEM_SHARING_DESTROY_GFN)); + /* + * Clear out the p2m entry so no one else may try to * unshare. Must succeed: we just read the old entry and - * we hold the p2m lock. */ + * we hold the p2m lock. + */ set_rc =3D p2m->set_entry(p2m, _gfn(gfn), _mfn(0), PAGE_ORDER_= 4K, p2m_invalid, p2m_access_rwx, -1); - ASSERT(set_rc =3D=3D 0); + ASSERT(!set_rc); count +=3D 0x10; } else @@ -1454,7 +1494,7 @@ int mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_= sharing_op_t) arg) =20 if ( XENMEM_SHARING_OP_FIELD_IS_GREF(mso.u.share.source_gfn) ) { - grant_ref_t gref =3D (grant_ref_t)=20 + grant_ref_t gref =3D (grant_ref_t) (XENMEM_SHARING_OP_FIELD_GET_GREF( mso.u.share.source_gfn)); rc =3D mem_sharing_gref_to_gfn(d->grant_table, gref, &sgfn, @@ -1470,7 +1510,7 @@ int mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_= sharing_op_t) arg) =20 if ( XENMEM_SHARING_OP_FIELD_IS_GREF(mso.u.share.client_gfn) ) { - grant_ref_t gref =3D (grant_ref_t)=20 + grant_ref_t gref =3D (grant_ref_t) (XENMEM_SHARING_OP_FIELD_GET_GREF( mso.u.share.client_gfn)); rc =3D mem_sharing_gref_to_gfn(cd->grant_table, gref, &cgf= n, @@ -1534,7 +1574,7 @@ int mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_= sharing_op_t) arg) sh =3D mso.u.share.source_handle; cgfn =3D mso.u.share.client_gfn; =20 - rc =3D mem_sharing_add_to_physmap(d, sgfn, sh, cd, cgfn);=20 + rc =3D mem_sharing_add_to_physmap(d, sgfn, sh, cd, cgfn); =20 rcu_unlock_domain(cd); } diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c index 8a5229ee21..714158d2a6 100644 --- a/xen/arch/x86/mm/p2m.c +++ b/xen/arch/x86/mm/p2m.c @@ -506,8 +506,10 @@ mfn_t __get_gfn_type_access(struct p2m_domain *p2m, un= signed long gfn_l, if ( (q & P2M_UNSHARE) && p2m_is_shared(*t) ) { ASSERT(p2m_is_hostp2m(p2m)); - /* Try to unshare. If we fail, communicate ENOMEM without - * sleeping. */ + /* + * Try to unshare. If we fail, communicate ENOMEM without + * sleeping. + */ if ( mem_sharing_unshare_page(p2m->domain, gfn_l, 0) < 0 ) mem_sharing_notify_enomem(p2m->domain, gfn_l, false); mfn =3D p2m->get_entry(p2m, gfn, t, a, q, page_order, NULL); @@ -887,15 +889,15 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn, = mfn_t mfn, &a, 0, NULL, NULL); if ( p2m_is_shared(ot) ) { - /* Do an unshare to cleanly take care of all corner=20 - * cases. */ + /* Do an unshare to cleanly take care of all corner cases. */ int rc; rc =3D mem_sharing_unshare_page(p2m->domain, gfn_x(gfn_add(gfn, i)), 0); if ( rc ) { p2m_unlock(p2m); - /* NOTE: Should a guest domain bring this upon itself, + /* + * NOTE: Should a guest domain bring this upon itself, * there is not a whole lot we can do. We are buried * deep in locks from most code paths by now. So, fail * the call and don't try to sleep on a wait queue @@ -904,8 +906,9 @@ guest_physmap_add_entry(struct domain *d, gfn_t gfn, mf= n_t mfn, * However, all current (changeset 3432abcf9380) code * paths avoid this unsavoury situation. For now. * - * Foreign domains are okay to place an event as they=20 - * won't go to sleep. */ + * Foreign domains are okay to place an event as they + * won't go to sleep. + */ (void)mem_sharing_notify_enomem(p2m->domain, gfn_x(gfn_add(gfn, i)), fa= lse); return rc; diff --git a/xen/include/asm-x86/mem_sharing.h b/xen/include/asm-x86/mem_sh= aring.h index db22468744..1280830a85 100644 --- a/xen/include/asm-x86/mem_sharing.h +++ b/xen/include/asm-x86/mem_sharing.h @@ -33,12 +33,14 @@ #define MEM_SHARING_AUDIT 0 #endif =20 -typedef uint64_t shr_handle_t;=20 +typedef uint64_t shr_handle_t; =20 typedef struct rmap_hashtab { struct list_head *bucket; - /* Overlaps with prev pointer of list_head in union below. - * Unlike the prev pointer, this can be NULL. */ + /* + * Overlaps with prev pointer of list_head in union below. + * Unlike the prev pointer, this can be NULL. + */ void *flag; } rmap_hashtab_t; =20 @@ -57,34 +59,34 @@ struct page_sharing_info }; }; =20 -#define sharing_supported(_d) \ - (is_hvm_domain(_d) && paging_mode_hap(_d))=20 - unsigned int mem_sharing_get_nr_saved_mfns(void); unsigned int mem_sharing_get_nr_shared_mfns(void); =20 #define MEM_SHARING_DESTROY_GFN (1<<1) /* Only fails with -ENOMEM. Enforce it with a BUG_ON wrapper. */ int __mem_sharing_unshare_page(struct domain *d, - unsigned long gfn,=20 - uint16_t flags); -static inline int mem_sharing_unshare_page(struct domain *d, - unsigned long gfn, - uint16_t flags) + unsigned long gfn, + uint16_t flags); + +static inline +int mem_sharing_unshare_page(struct domain *d, + unsigned long gfn, + uint16_t flags) { int rc =3D __mem_sharing_unshare_page(d, gfn, flags); BUG_ON( rc && (rc !=3D -ENOMEM) ); return rc; } =20 -/* If called by a foreign domain, possible errors are +/* + * If called by a foreign domain, possible errors are * -EBUSY -> ring full * -ENOSYS -> no ring to begin with * and the foreign mapper is responsible for retrying. * - * If called by the guest vcpu itself and allow_sleep is set, may=20 - * sleep on a wait queue, so the caller is responsible for not=20 - * holding locks on entry. It may only fail with ENOSYS=20 + * If called by the guest vcpu itself and allow_sleep is set, may + * sleep on a wait queue, so the caller is responsible for not + * holding locks on entry. It may only fail with ENOSYS * * If called by the guest vcpu itself and allow_sleep is not set, * then it's the same as a foreign domain. @@ -92,10 +94,11 @@ static inline int mem_sharing_unshare_page(struct domai= n *d, int mem_sharing_notify_enomem(struct domain *d, unsigned long gfn, bool allow_sleep); int mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op_t) arg); -int mem_sharing_domctl(struct domain *d,=20 +int mem_sharing_domctl(struct domain *d, struct xen_domctl_mem_sharing_op *mec); =20 -/* Scans the p2m and relinquishes any shared pages, destroying=20 +/* + * Scans the p2m and relinquishes any shared pages, destroying * those for which this domain holds the final reference. * Preemptible. */ @@ -107,18 +110,22 @@ static inline unsigned int mem_sharing_get_nr_saved_m= fns(void) { return 0; } + static inline unsigned int mem_sharing_get_nr_shared_mfns(void) { return 0; } -static inline int mem_sharing_unshare_page(struct domain *d, - unsigned long gfn, - uint16_t flags) + +static inline +int mem_sharing_unshare_page(struct domain *d, unsigned long gfn, + uint16_t flags) { ASSERT_UNREACHABLE(); return -EOPNOTSUPP; } -static inline int mem_sharing_notify_enomem(struct domain *d, unsigned lon= g gfn, + +static inline +int mem_sharing_notify_enomem(struct domain *d, unsigned long gfn, bool allow_sleep) { ASSERT_UNREACHABLE(); --=20 2.20.1 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel