[Xen-devel] [PATCH v8 0/4] purge free_shared_domheap_page()

[Xen-devel] [PATCH v8 0/4] purge free_shared_domheap_page()

[Paul Durrant]

Paul Durrant (4):
  x86 / vmx: move teardown from domain_destroy()...
  add a domain_tot_pages() helper function
  mm: make pages allocated with MEMF_no_refcount safe to assign
  x86 / vmx: use a MEMF_no_refcount domheap page for
    APIC_DEFAULT_PHYS_BASE

 xen/arch/x86/domain.c           |  2 +-
 xen/arch/x86/hvm/vmx/vmx.c      | 25 ++++++++---
 xen/arch/x86/mm.c               | 19 +++-----
 xen/arch/x86/mm/p2m-pod.c       | 10 ++---
 xen/arch/x86/mm/shadow/common.c |  2 +-
 xen/arch/x86/msi.c              |  2 +-
 xen/arch/x86/numa.c             |  2 +-
 xen/arch/x86/pv/dom0_build.c    | 25 ++++++-----
 xen/arch/x86/pv/domain.c        |  2 +-
 xen/common/domctl.c             |  2 +-
 xen/common/grant_table.c        |  4 +-
 xen/common/keyhandler.c         |  2 +-
 xen/common/memory.c             |  4 +-
 xen/common/page_alloc.c         | 78 ++++++++++++++++++++++++---------
 xen/include/asm-arm/mm.h        |  5 ++-
 xen/include/asm-x86/mm.h        |  9 ++--
 xen/include/public/memory.h     |  4 +-
 xen/include/xen/sched.h         | 27 +++++++++---
 18 files changed, 143 insertions(+), 81 deletions(-)

-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v8 1/4] x86 / vmx: move teardown from domain_destroy()...

[Paul Durrant]

... to domain_relinquish_resources().

The teardown code frees the APICv page. This does not need to be done late
so do it in domain_relinquish_resources() rather than domain_destroy().

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wl@xen.org>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: George Dunlap <george.dunlap@citrix.com>

v4:
  - New in v4 (disaggregated from v3 patch #3)
---
 xen/arch/x86/hvm/vmx/vmx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index b262d38a7c..606f3dc2eb 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -419,7 +419,7 @@ static int vmx_domain_initialise(struct domain *d)
     return 0;
 }
 
-static void vmx_domain_destroy(struct domain *d)
+static void vmx_domain_relinquish_resources(struct domain *d)
 {
     if ( !has_vlapic(d) )
         return;
@@ -2240,7 +2240,7 @@ static struct hvm_function_table __initdata vmx_function_table = {
     .cpu_up_prepare       = vmx_cpu_up_prepare,
     .cpu_dead             = vmx_cpu_dead,
     .domain_initialise    = vmx_domain_initialise,
-    .domain_destroy       = vmx_domain_destroy,
+    .domain_relinquish_resources = vmx_domain_relinquish_resources,
     .vcpu_initialise      = vmx_vcpu_initialise,
     .vcpu_destroy         = vmx_vcpu_destroy,
     .save_cpu_ctxt        = vmx_save_vmcs_ctxt,
-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v8 2/4] add a domain_tot_pages() helper function

[Paul Durrant]

This patch adds a new domain_tot_pages() inline helper function into
sched.h, which will be needed by a subsequent patch.

No functional change.

NOTE: While modifying the comment for 'tot_pages' in sched.h this patch
      makes some cosmetic fixes to surrounding comments.

Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wl@xen.org>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Julien Grall <julien@xen.org>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: George Dunlap <george.dunlap@eu.citrix.com>
Cc: Tim Deegan <tim@xen.org>

v8:
 - New in v8
---
 xen/arch/x86/domain.c           |  2 +-
 xen/arch/x86/mm.c               |  6 +++---
 xen/arch/x86/mm/p2m-pod.c       | 10 +++++-----
 xen/arch/x86/mm/shadow/common.c |  2 +-
 xen/arch/x86/msi.c              |  2 +-
 xen/arch/x86/numa.c             |  2 +-
 xen/arch/x86/pv/dom0_build.c    | 25 +++++++++++++------------
 xen/arch/x86/pv/domain.c        |  2 +-
 xen/common/domctl.c             |  2 +-
 xen/common/grant_table.c        |  4 ++--
 xen/common/keyhandler.c         |  2 +-
 xen/common/memory.c             |  4 ++--
 xen/common/page_alloc.c         | 15 ++++++++-------
 xen/include/public/memory.h     |  4 ++--
 xen/include/xen/sched.h         | 24 ++++++++++++++++++------
 15 files changed, 60 insertions(+), 46 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 28fefa1f81..643c23ffb0 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -218,7 +218,7 @@ void dump_pageframe_info(struct domain *d)
 
     printk("Memory pages belonging to domain %u:\n", d->domain_id);
 
-    if ( d->tot_pages >= 10 && d->is_dying < DOMDYING_dead )
+    if ( domain_tot_pages(d) >= 10 && d->is_dying < DOMDYING_dead )
     {
         printk("    DomPage list too long to display\n");
     }
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index f50c065af3..8bb66cf30c 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4194,8 +4194,8 @@ long do_mmu_update(
  * - page caching attributes cleaned up
  * - removed from the domain's page_list
  *
- * If MEMF_no_refcount is not set, the domain's tot_pages will be
- * adjusted.  If this results in the page count falling to 0,
+ * If MEMF_no_refcount is not set, the domain_adjust_tot_pages() will
+ * be called.  If this results in the page count falling to 0,
  * put_domain() will be called.
  *
  * The caller should either call free_domheap_page() to free the
@@ -4870,7 +4870,7 @@ long arch_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
         else if ( rc >= 0 )
         {
             p2m = p2m_get_hostp2m(d);
-            target.tot_pages       = d->tot_pages;
+            target.tot_pages       = domain_tot_pages(d);
             target.pod_cache_pages = p2m->pod.count;
             target.pod_entries     = p2m->pod.entry_count;
 
diff --git a/xen/arch/x86/mm/p2m-pod.c b/xen/arch/x86/mm/p2m-pod.c
index 096e2773fb..f2c9409568 100644
--- a/xen/arch/x86/mm/p2m-pod.c
+++ b/xen/arch/x86/mm/p2m-pod.c
@@ -302,7 +302,7 @@ out:
  * The following equations should hold:
  *  0 <= P <= T <= B <= M
  *  d->arch.p2m->pod.entry_count == B - P
- *  d->tot_pages == P + d->arch.p2m->pod.count
+ *  domain_tot_pages(d) == P + d->arch.p2m->pod.count
  *
  * Now we have the following potential cases to cover:
  *     B <T': Set the PoD cache size equal to the number of outstanding PoD
@@ -336,7 +336,7 @@ p2m_pod_set_mem_target(struct domain *d, unsigned long target)
     pod_lock(p2m);
 
     /* P == B: Nothing to do (unless the guest is being created). */
-    populated = d->tot_pages - p2m->pod.count;
+    populated = domain_tot_pages(d) - p2m->pod.count;
     if ( populated > 0 && p2m->pod.entry_count == 0 )
         goto out;
 
@@ -348,7 +348,7 @@ p2m_pod_set_mem_target(struct domain *d, unsigned long target)
      * T' < B: Don't reduce the cache size; let the balloon driver
      * take care of it.
      */
-    if ( target < d->tot_pages )
+    if ( target < domain_tot_pages(d) )
         goto out;
 
     pod_target = target - populated;
@@ -1231,8 +1231,8 @@ out_of_memory:
     pod_unlock(p2m);
 
     printk("%s: Dom%d out of PoD memory! (tot=%"PRIu32" ents=%ld dom%d)\n",
-           __func__, d->domain_id, d->tot_pages, p2m->pod.entry_count,
-           current->domain->domain_id);
+           __func__, d->domain_id, domain_tot_pages(d),
+           p2m->pod.entry_count, current->domain->domain_id);
     domain_crash(d);
     return false;
 out_fail:
diff --git a/xen/arch/x86/mm/shadow/common.c b/xen/arch/x86/mm/shadow/common.c
index 6212ec2c4a..cba3ab1eba 100644
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -1256,7 +1256,7 @@ static unsigned int sh_min_allocation(const struct domain *d)
      * up of slot zero and an LAPIC page), plus one for HVM's 1-to-1 pagetable.
      */
     return shadow_min_acceptable_pages(d) +
-           max(max(d->tot_pages / 256,
+           max(max(domain_tot_pages(d) / 256,
                    is_hvm_domain(d) ? CONFIG_PAGING_LEVELS + 2 : 0U) +
                is_hvm_domain(d),
                d->arch.paging.shadow.p2m_pages);
diff --git a/xen/arch/x86/msi.c b/xen/arch/x86/msi.c
index df97ce0c72..2fabaaa155 100644
--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -991,7 +991,7 @@ static int msix_capability_init(struct pci_dev *dev,
                        seg, bus, slot, func, d->domain_id);
             if ( !is_hardware_domain(d) &&
                  /* Assume a domain without memory has no mappings yet. */
-                 (!is_hardware_domain(currd) || d->tot_pages) )
+                 (!is_hardware_domain(currd) || domain_tot_pages(d)) )
                 domain_crash(d);
             /* XXX How to deal with existing mappings? */
         }
diff --git a/xen/arch/x86/numa.c b/xen/arch/x86/numa.c
index 7e1f563012..7f0d27c153 100644
--- a/xen/arch/x86/numa.c
+++ b/xen/arch/x86/numa.c
@@ -419,7 +419,7 @@ static void dump_numa(unsigned char key)
     {
         process_pending_softirqs();
 
-        printk("Domain %u (total: %u):\n", d->domain_id, d->tot_pages);
+        printk("Domain %u (total: %u):\n", d->domain_id, domain_tot_pages(d));
 
         for_each_online_node ( i )
             page_num_node[i] = 0;
diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
index 9a97cf4abf..5678da782d 100644
--- a/xen/arch/x86/pv/dom0_build.c
+++ b/xen/arch/x86/pv/dom0_build.c
@@ -110,8 +110,9 @@ static __init void setup_pv_physmap(struct domain *d, unsigned long pgtbl_pfn,
 
     while ( vphysmap_start < vphysmap_end )
     {
-        if ( d->tot_pages + ((round_pgup(vphysmap_end) - vphysmap_start)
-                             >> PAGE_SHIFT) + 3 > nr_pages )
+        if ( domain_tot_pages(d) +
+             ((round_pgup(vphysmap_end) - vphysmap_start) >> PAGE_SHIFT) +
+             3 > nr_pages )
             panic("Dom0 allocation too small for initial P->M table\n");
 
         if ( pl1e )
@@ -264,7 +265,7 @@ static struct page_info * __init alloc_chunk(struct domain *d,
     {
         struct page_info *pg2;
 
-        if ( d->tot_pages + (1 << order) > d->max_pages )
+        if ( domain_tot_pages(d) + (1 << order) > d->max_pages )
             continue;
         pg2 = alloc_domheap_pages(d, order, MEMF_exact_node | MEMF_no_scrub);
         if ( pg2 > page )
@@ -500,13 +501,13 @@ int __init dom0_construct_pv(struct domain *d,
     if ( page == NULL )
         panic("Not enough RAM for domain 0 allocation\n");
     alloc_spfn = mfn_x(page_to_mfn(page));
-    alloc_epfn = alloc_spfn + d->tot_pages;
+    alloc_epfn = alloc_spfn + domain_tot_pages(d);
 
     if ( initrd_len )
     {
         initrd_pfn = vinitrd_start ?
                      (vinitrd_start - v_start) >> PAGE_SHIFT :
-                     d->tot_pages;
+                     domain_tot_pages(d);
         initrd_mfn = mfn = initrd->mod_start;
         count = PFN_UP(initrd_len);
         if ( d->arch.physaddr_bitsize &&
@@ -541,9 +542,9 @@ int __init dom0_construct_pv(struct domain *d,
     printk("PHYSICAL MEMORY ARRANGEMENT:\n"
            " Dom0 alloc.:   %"PRIpaddr"->%"PRIpaddr,
            pfn_to_paddr(alloc_spfn), pfn_to_paddr(alloc_epfn));
-    if ( d->tot_pages < nr_pages )
+    if ( domain_tot_pages(d) < nr_pages )
         printk(" (%lu pages to be allocated)",
-               nr_pages - d->tot_pages);
+               nr_pages - domain_tot_pages(d));
     if ( initrd )
     {
         mpt_alloc = (paddr_t)initrd->mod_start << PAGE_SHIFT;
@@ -755,7 +756,7 @@ int __init dom0_construct_pv(struct domain *d,
     snprintf(si->magic, sizeof(si->magic), "xen-3.0-x86_%d%s",
              elf_64bit(&elf) ? 64 : 32, parms.pae ? "p" : "");
 
-    count = d->tot_pages;
+    count = domain_tot_pages(d);
 
     /* Set up the phys->machine table if not part of the initial mapping. */
     if ( parms.p2m_base != UNSET_ADDR )
@@ -786,7 +787,7 @@ int __init dom0_construct_pv(struct domain *d,
             process_pending_softirqs();
     }
     si->first_p2m_pfn = pfn;
-    si->nr_p2m_frames = d->tot_pages - count;
+    si->nr_p2m_frames = domain_tot_pages(d) - count;
     page_list_for_each ( page, &d->page_list )
     {
         mfn = mfn_x(page_to_mfn(page));
@@ -804,15 +805,15 @@ int __init dom0_construct_pv(struct domain *d,
                 process_pending_softirqs();
         }
     }
-    BUG_ON(pfn != d->tot_pages);
+    BUG_ON(pfn != domain_tot_pages(d));
 #ifndef NDEBUG
     alloc_epfn += PFN_UP(initrd_len) + si->nr_p2m_frames;
 #endif
     while ( pfn < nr_pages )
     {
-        if ( (page = alloc_chunk(d, nr_pages - d->tot_pages)) == NULL )
+        if ( (page = alloc_chunk(d, nr_pages - domain_tot_pages(d))) == NULL )
             panic("Not enough RAM for DOM0 reservation\n");
-        while ( pfn < d->tot_pages )
+        while ( pfn < domain_tot_pages(d) )
         {
             mfn = mfn_x(page_to_mfn(page));
 #ifndef NDEBUG
diff --git a/xen/arch/x86/pv/domain.c b/xen/arch/x86/pv/domain.c
index 4da0b2afff..c95652d1b8 100644
--- a/xen/arch/x86/pv/domain.c
+++ b/xen/arch/x86/pv/domain.c
@@ -173,7 +173,7 @@ int switch_compat(struct domain *d)
 
     BUILD_BUG_ON(offsetof(struct shared_info, vcpu_info) != 0);
 
-    if ( is_hvm_domain(d) || d->tot_pages != 0 )
+    if ( is_hvm_domain(d) || domain_tot_pages(d) != 0 )
         return -EACCES;
     if ( is_pv_32bit_domain(d) )
         return 0;
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 8b819f56e5..bdc24bbd7c 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -191,7 +191,7 @@ void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info)
 
     xsm_security_domaininfo(d, info);
 
-    info->tot_pages         = d->tot_pages;
+    info->tot_pages         = domain_tot_pages(d);
     info->max_pages         = d->max_pages;
     info->outstanding_pages = d->outstanding_pages;
     info->shr_pages         = atomic_read(&d->shr_pages);
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 5536d282b9..8bee6b3b66 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -2261,7 +2261,7 @@ gnttab_transfer(
          * pages when it is dying.
          */
         if ( unlikely(e->is_dying) ||
-             unlikely(e->tot_pages >= e->max_pages) )
+             unlikely(domain_tot_pages(e) >= e->max_pages) )
         {
             spin_unlock(&e->page_alloc_lock);
 
@@ -2271,7 +2271,7 @@ gnttab_transfer(
             else
                 gdprintk(XENLOG_INFO,
                          "Transferee d%d has no headroom (tot %u, max %u)\n",
-                         e->domain_id, e->tot_pages, e->max_pages);
+                         e->domain_id, domain_tot_pages(e), e->max_pages);
 
             gop.status = GNTST_general_error;
             goto unlock_and_copyback;
diff --git a/xen/common/keyhandler.c b/xen/common/keyhandler.c
index f50490d0f3..87bd145374 100644
--- a/xen/common/keyhandler.c
+++ b/xen/common/keyhandler.c
@@ -271,7 +271,7 @@ static void dump_domains(unsigned char key)
                atomic_read(&d->pause_count));
         printk("    nr_pages=%d xenheap_pages=%d shared_pages=%u paged_pages=%u "
                "dirty_cpus={%*pbl} max_pages=%u\n",
-               d->tot_pages, d->xenheap_pages, atomic_read(&d->shr_pages),
+               domain_tot_pages(d), d->xenheap_pages, atomic_read(&d->shr_pages),
                atomic_read(&d->paged_pages), CPUMASK_PR(d->dirty_cpumask),
                d->max_pages);
         printk("    handle=%02x%02x%02x%02x-%02x%02x-%02x%02x-"
diff --git a/xen/common/memory.c b/xen/common/memory.c
index c7d2bac452..bf464e8799 100644
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -717,7 +717,7 @@ static long memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
 
                 /*
                  * Pages in in_chunk_list is stolen without
-                 * decreasing the tot_pages. If the domain is dying when
+                 * decreasing domain_tot_pages(). If the domain is dying when
                  * assign pages, we need decrease the count. For those pages
                  * that has been assigned, it should be covered by
                  * domain_relinquish_resources().
@@ -1267,7 +1267,7 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
         switch ( op )
         {
         case XENMEM_current_reservation:
-            rc = d->tot_pages;
+            rc = domain_tot_pages(d);
             break;
         case XENMEM_maximum_reservation:
             rc = d->max_pages;
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index 919a270587..bbd3163909 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -518,8 +518,8 @@ int domain_set_outstanding_pages(struct domain *d, unsigned long pages)
         goto out;
     }
 
-    /* disallow a claim not exceeding current tot_pages or above max_pages */
-    if ( (pages <= d->tot_pages) || (pages > d->max_pages) )
+    /* disallow a claim not exceeding domain_tot_pages() or above max_pages */
+    if ( (pages <= domain_tot_pages(d)) || (pages > d->max_pages) )
     {
         ret = -EINVAL;
         goto out;
@@ -532,9 +532,9 @@ int domain_set_outstanding_pages(struct domain *d, unsigned long pages)
 
     /*
      * Note, if domain has already allocated memory before making a claim
-     * then the claim must take tot_pages into account
+     * then the claim must take domain_tot_pages() into account
      */
-    claim = pages - d->tot_pages;
+    claim = pages - domain_tot_pages(d);
     if ( claim > avail_pages )
         goto out;
 
@@ -2269,11 +2269,12 @@ int assign_pages(
 
     if ( !(memflags & MEMF_no_refcount) )
     {
-        if ( unlikely((d->tot_pages + (1 << order)) > d->max_pages) )
+        unsigned int tot_pages = domain_tot_pages(d) + (1 << order);
+
+        if ( unlikely(tot_pages > d->max_pages) )
         {
             gprintk(XENLOG_INFO, "Over-allocation for domain %u: "
-                    "%u > %u\n", d->domain_id,
-                    d->tot_pages + (1 << order), d->max_pages);
+                    "%u > %u\n", d->domain_id, tot_pages, d->max_pages);
             rc = -E2BIG;
             goto out;
         }
diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
index cfdda6e2a8..126d0ff06e 100644
--- a/xen/include/public/memory.h
+++ b/xen/include/public/memory.h
@@ -553,8 +553,8 @@ DEFINE_XEN_GUEST_HANDLE(xen_mem_sharing_op_t);
  *
  * Note that a valid claim may be staked even after memory has been
  * allocated for a domain.  In this case, the claim is not incremental,
- * i.e. if the domain's tot_pages is 3, and a claim is staked for 10,
- * only 7 additional pages are claimed.
+ * i.e. if the domain's total page count is 3, and a claim is staked
+ * for 10, only 7 additional pages are claimed.
  *
  * Caller must be privileged or the hypercall fails.
  */
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 7c5c437247..1b6d7b941f 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -364,12 +364,18 @@ struct domain
     spinlock_t       page_alloc_lock; /* protects all the following fields  */
     struct page_list_head page_list;  /* linked list */
     struct page_list_head xenpage_list; /* linked list (size xenheap_pages) */
-    unsigned int     tot_pages;       /* number of pages currently possesed */
-    unsigned int     xenheap_pages;   /* # pages allocated from Xen heap    */
-    unsigned int     outstanding_pages; /* pages claimed but not possessed  */
-    unsigned int     max_pages;       /* maximum value for tot_pages        */
-    atomic_t         shr_pages;       /* number of shared pages             */
-    atomic_t         paged_pages;     /* number of paged-out pages          */
+
+    /*
+     * This field should only be directly accessed by domain_adjust_tot_pages()
+     * and the domain_tot_pages() helper function defined below.
+     */
+    unsigned int     tot_pages;
+
+    unsigned int     xenheap_pages;     /* pages allocated from Xen heap */
+    unsigned int     outstanding_pages; /* pages claimed but not possessed */
+    unsigned int     max_pages;         /* maximum value for domain_tot_pages() */
+    atomic_t         shr_pages;         /* shared pages */
+    atomic_t         paged_pages;       /* paged-out pages */
 
     /* Scheduling. */
     void            *sched_priv;    /* scheduler-specific data */
@@ -539,6 +545,12 @@ struct domain
 #endif
 };
 
+/* Return number of pages currently posessed by the domain */
+static inline unsigned int domain_tot_pages(const struct domain *d)
+{
+    return d->tot_pages;
+}
+
 /* Protect updates/reads (resp.) of domain_list and domain_hash. */
 extern spinlock_t domlist_update_lock;
 extern rcu_read_lock_t domlist_read_lock;
-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v8 3/4] mm: make pages allocated with MEMF_no_refcount safe to assign

[Paul Durrant]

Currently it is unsafe to assign a domheap page allocated with
MEMF_no_refcount to a domain because the domain't 'tot_pages' will not
be incremented, but will be decrement when the page is freed (since
free_domheap_pages() has no way of telling that the increment was skipped).

This patch allocates a new 'count_info' bit for a PGC_extra flag
which is then used to mark pages when alloc_domheap_pages() is called
with MEMF_no_refcount. assign_pages() because it still needs to call
domain_adjust_tot_pages() to make sure the domain is appropriately
referenced. Hence it is modified to do that for PGC_extra pages even if it
is passed MEMF_no_refount.

The number of PGC_extra pages assigned to a domain is tracked in a new
'extra_pages' counter, which is then subtracted from 'total_pages' in
the domain_tot_pages() helper. Thus 'normal' page assignments will still
be appropriately checked against 'max_pages'.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Julien Grall <julien@xen.org>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Wei Liu <wl@xen.org>
Cc: Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>

v8:
 - Drop the idea of post-allocation assignment adding an error path to
   steal_page() if it encounters a PGC_extra page
 - Tighten up the ASSERTs in assign_pages()

v7:
 - s/PGC_no_refcount/PGC_extra/g
 - Re-work allocation to account for 'extra' pages, also making it
   safe to assign PGC_extra pages post-allocation

v6:
 - Add an extra ASSERT into assign_pages() that PGC_no_refcount is not
   set if MEMF_no_refcount is clear
 - ASSERT that count_info is 0 in alloc_domheap_pages() and set to
   PGC_no_refcount rather than ORing

v5:
 - Make sure PGC_no_refcount is set before assign_pages() is called
 - Don't bother to clear PGC_no_refcount in free_domheap_pages() and
   drop ASSERT in free_heap_pages()
 - Don't latch count_info in free_heap_pages()

v4:
 - New in v4
---
 xen/arch/x86/mm.c        |  3 +-
 xen/common/page_alloc.c  | 63 +++++++++++++++++++++++++++++++---------
 xen/include/asm-arm/mm.h |  5 +++-
 xen/include/asm-x86/mm.h |  7 +++--
 xen/include/xen/sched.h  |  5 +++-
 5 files changed, 64 insertions(+), 19 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 8bb66cf30c..2796161c1f 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -4217,7 +4217,8 @@ int steal_page(
     if ( !(owner = page_get_owner_and_reference(page)) )
         goto fail;
 
-    if ( owner != d || is_xen_heap_page(page) )
+    if ( owner != d || is_xen_heap_page(page) ||
+         (page->count_info & PGC_extra) )
         goto fail_put;
 
     /*
diff --git a/xen/common/page_alloc.c b/xen/common/page_alloc.c
index bbd3163909..1ac9d9c719 100644
--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -2267,7 +2267,29 @@ int assign_pages(
         goto out;
     }
 
-    if ( !(memflags & MEMF_no_refcount) )
+#ifndef NDEBUG
+    {
+        unsigned int extra_pages = 0;
+
+        for ( i = 0; i < (1ul << order); i++ )
+        {
+            ASSERT(!(pg[i].count_info & ~PGC_extra));
+            if ( pg[i].count_info & PGC_extra )
+                extra_pages++;
+        }
+
+        ASSERT(!extra_pages ||
+               ((memflags & MEMF_no_refcount) &&
+                extra_pages == 1u << order));
+    }
+#endif
+
+    if ( pg[0].count_info & PGC_extra )
+    {
+        d->extra_pages += 1u << order;
+        memflags &= ~MEMF_no_refcount;
+    }
+    else if ( !(memflags & MEMF_no_refcount) )
     {
         unsigned int tot_pages = domain_tot_pages(d) + (1 << order);
 
@@ -2278,18 +2300,19 @@ int assign_pages(
             rc = -E2BIG;
             goto out;
         }
+    }
 
-        if ( unlikely(domain_adjust_tot_pages(d, 1 << order) == (1 << order)) )
+    if ( !(memflags & MEMF_no_refcount) &&
+         unlikely(domain_adjust_tot_pages(d, 1 << order) == (1 << order)) )
             get_knownalive_domain(d);
-    }
 
     for ( i = 0; i < (1 << order); i++ )
     {
         ASSERT(page_get_owner(&pg[i]) == NULL);
-        ASSERT(!pg[i].count_info);
         page_set_owner(&pg[i], d);
         smp_wmb(); /* Domain pointer must be visible before updating refcnt. */
-        pg[i].count_info = PGC_allocated | 1;
+        pg[i].count_info =
+            (pg[i].count_info & PGC_extra) | PGC_allocated | 1;
         page_list_add_tail(&pg[i], &d->page_list);
     }
 
@@ -2315,11 +2338,6 @@ struct page_info *alloc_domheap_pages(
 
     if ( memflags & MEMF_no_owner )
         memflags |= MEMF_no_refcount;
-    else if ( (memflags & MEMF_no_refcount) && d )
-    {
-        ASSERT(!(memflags & MEMF_no_refcount));
-        return NULL;
-    }
 
     if ( !dma_bitsize )
         memflags &= ~MEMF_no_dma;
@@ -2332,11 +2350,23 @@ struct page_info *alloc_domheap_pages(
                                   memflags, d)) == NULL)) )
          return NULL;
 
-    if ( d && !(memflags & MEMF_no_owner) &&
-         assign_pages(d, pg, order, memflags) )
+    if ( d && !(memflags & MEMF_no_owner) )
     {
-        free_heap_pages(pg, order, memflags & MEMF_no_scrub);
-        return NULL;
+        if ( memflags & MEMF_no_refcount )
+        {
+            unsigned long i;
+
+            for ( i = 0; i < (1ul << order); i++ )
+            {
+                ASSERT(!pg[i].count_info);
+                pg[i].count_info = PGC_extra;
+            }
+        }
+        if ( assign_pages(d, pg, order, memflags) )
+        {
+            free_heap_pages(pg, order, memflags & MEMF_no_scrub);
+            return NULL;
+        }
     }
 
     return pg;
@@ -2384,6 +2414,11 @@ void free_domheap_pages(struct page_info *pg, unsigned int order)
                     BUG();
                 }
                 arch_free_heap_page(d, &pg[i]);
+                if ( pg[i].count_info & PGC_extra )
+                {
+                    ASSERT(d->extra_pages);
+                    d->extra_pages--;
+                }
             }
 
             drop_dom_ref = !domain_adjust_tot_pages(d, -(1 << order));
diff --git a/xen/include/asm-arm/mm.h b/xen/include/asm-arm/mm.h
index 333efd3a60..7df91280bc 100644
--- a/xen/include/asm-arm/mm.h
+++ b/xen/include/asm-arm/mm.h
@@ -119,9 +119,12 @@ struct page_info
 #define PGC_state_offlined PG_mask(2, 9)
 #define PGC_state_free    PG_mask(3, 9)
 #define page_state_is(pg, st) (((pg)->count_info&PGC_state) == PGC_state_##st)
+/* Page is not reference counted */
+#define _PGC_extra        PG_shift(10)
+#define PGC_extra         PG_mask(1, 10)
 
 /* Count of references to this frame. */
-#define PGC_count_width   PG_shift(9)
+#define PGC_count_width   PG_shift(10)
 #define PGC_count_mask    ((1UL<<PGC_count_width)-1)
 
 /*
diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h
index 2ca8882ad0..06d64d494d 100644
--- a/xen/include/asm-x86/mm.h
+++ b/xen/include/asm-x86/mm.h
@@ -77,9 +77,12 @@
 #define PGC_state_offlined PG_mask(2, 9)
 #define PGC_state_free    PG_mask(3, 9)
 #define page_state_is(pg, st) (((pg)->count_info&PGC_state) == PGC_state_##st)
+/* Page is not reference counted */
+#define _PGC_extra        PG_shift(10)
+#define PGC_extra         PG_mask(1, 10)
 
- /* Count of references to this frame. */
-#define PGC_count_width   PG_shift(9)
+/* Count of references to this frame. */
+#define PGC_count_width   PG_shift(10)
 #define PGC_count_mask    ((1UL<<PGC_count_width)-1)
 
 /*
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 1b6d7b941f..21b5f4cebd 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -374,6 +374,7 @@ struct domain
     unsigned int     xenheap_pages;     /* pages allocated from Xen heap */
     unsigned int     outstanding_pages; /* pages claimed but not possessed */
     unsigned int     max_pages;         /* maximum value for domain_tot_pages() */
+    unsigned int     extra_pages;       /* pages not included in domain_tot_pages() */
     atomic_t         shr_pages;         /* shared pages */
     atomic_t         paged_pages;       /* paged-out pages */
 
@@ -548,7 +549,9 @@ struct domain
 /* Return number of pages currently posessed by the domain */
 static inline unsigned int domain_tot_pages(const struct domain *d)
 {
-    return d->tot_pages;
+    ASSERT(d->extra_pages <= d->tot_pages);
+
+    return d->tot_pages - d->extra_pages;
 }
 
 /* Protect updates/reads (resp.) of domain_list and domain_hash. */
-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[Xen-devel] [PATCH v8 4/4] x86 / vmx: use a MEMF_no_refcount domheap page for APIC_DEFAULT_PHYS_BASE

[Paul Durrant]

vmx_alloc_vlapic_mapping() currently contains some very odd looking code
that allocates a MEMF_no_owner domheap page and then shares with the guest
as if it were a xenheap page. This then requires vmx_free_vlapic_mapping()
to call a special function in the mm code: free_shared_domheap_page().

By using a MEMF_no_refcount domheap page instead, the odd looking code in
vmx_alloc_vlapic_mapping() can simply use get_page_and_type() to set up a
writable mapping before insertion in the P2M and vmx_free_vlapic_mapping()
can simply release the page using put_page_alloc_ref() followed by
put_page_and_type(). This then allows free_shared_domheap_page() to be
purged.

Signed-off-by: Paul Durrant <pdurrant@amazon.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wl@xen.org>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>

v4:
 - Use a MEMF_no_refcount page rather than a 'normal' page

v2:
 - Set an initial value for max_pages rather than avoiding the check in
   assign_pages()
 - Make domain_destroy() optional
---
 xen/arch/x86/hvm/vmx/vmx.c | 21 ++++++++++++++++++---
 xen/arch/x86/mm.c          | 10 ----------
 xen/include/asm-x86/mm.h   |  2 --
 3 files changed, 18 insertions(+), 15 deletions(-)

diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 606f3dc2eb..7423d2421b 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3028,12 +3028,22 @@ static int vmx_alloc_vlapic_mapping(struct domain *d)
     if ( !cpu_has_vmx_virtualize_apic_accesses )
         return 0;
 
-    pg = alloc_domheap_page(d, MEMF_no_owner);
+    pg = alloc_domheap_page(d, MEMF_no_refcount);
     if ( !pg )
         return -ENOMEM;
+
+    if ( !get_page_and_type(pg, d, PGT_writable_page) )
+    {
+        /*
+         * The domain can't possibly know about this page yet, so failure
+         * here is a clear indication of something fishy going on.
+         */
+        domain_crash(d);
+        return -ENODATA;
+    }
+
     mfn = page_to_mfn(pg);
     clear_domain_page(mfn);
-    share_xen_page_with_guest(pg, d, SHARE_rw);
     d->arch.hvm.vmx.apic_access_mfn = mfn;
 
     return set_mmio_p2m_entry(d, paddr_to_pfn(APIC_DEFAULT_PHYS_BASE), mfn,
@@ -3047,7 +3057,12 @@ static void vmx_free_vlapic_mapping(struct domain *d)
 
     d->arch.hvm.vmx.apic_access_mfn = _mfn(0);
     if ( !mfn_eq(mfn, _mfn(0)) )
-        free_shared_domheap_page(mfn_to_page(mfn));
+    {
+        struct page_info *pg = mfn_to_page(mfn);
+
+        put_page_alloc_ref(pg);
+        put_page_and_type(pg);
+    }
 }
 
 static void vmx_install_vlapic_mapping(struct vcpu *v)
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 2796161c1f..461eed839d 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -496,16 +496,6 @@ void share_xen_page_with_guest(struct page_info *page, struct domain *d,
     spin_unlock(&d->page_alloc_lock);
 }
 
-void free_shared_domheap_page(struct page_info *page)
-{
-    put_page_alloc_ref(page);
-    if ( !test_and_clear_bit(_PGC_xen_heap, &page->count_info) )
-        ASSERT_UNREACHABLE();
-    page->u.inuse.type_info = 0;
-    page_set_owner(page, NULL);
-    free_domheap_page(page);
-}
-
 void make_cr3(struct vcpu *v, mfn_t mfn)
 {
     struct domain *d = v->domain;
diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h
index 06d64d494d..fafb3af46d 100644
--- a/xen/include/asm-x86/mm.h
+++ b/xen/include/asm-x86/mm.h
@@ -320,8 +320,6 @@ struct page_info
 
 #define maddr_get_owner(ma)   (page_get_owner(maddr_to_page((ma))))
 
-extern void free_shared_domheap_page(struct page_info *page);
-
 #define frame_table ((struct page_info *)FRAMETABLE_VIRT_START)
 extern unsigned long max_page;
 extern unsigned long total_pages;
-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v8 3/4] mm: make pages allocated with MEMF_no_refcount safe to assign

[Jan Beulich]

On 30.01.2020 15:57, Paul Durrant wrote:
> Currently it is unsafe to assign a domheap page allocated with
> MEMF_no_refcount to a domain because the domain't 'tot_pages' will not
> be incremented, but will be decrement when the page is freed (since
> free_domheap_pages() has no way of telling that the increment was skipped).
> 
> This patch allocates a new 'count_info' bit for a PGC_extra flag
> which is then used to mark pages when alloc_domheap_pages() is called
> with MEMF_no_refcount. assign_pages() because it still needs to call
> domain_adjust_tot_pages() to make sure the domain is appropriately
> referenced. Hence it is modified to do that for PGC_extra pages even if it
> is passed MEMF_no_refount.
> 
> The number of PGC_extra pages assigned to a domain is tracked in a new
> 'extra_pages' counter, which is then subtracted from 'total_pages' in
> the domain_tot_pages() helper. Thus 'normal' page assignments will still
> be appropriately checked against 'max_pages'.
> 
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v8 2/4] add a domain_tot_pages() helper function

[Jan Beulich]

On 30.01.2020 15:57, Paul Durrant wrote:
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -218,7 +218,7 @@ void dump_pageframe_info(struct domain *d)
>  
>      printk("Memory pages belonging to domain %u:\n", d->domain_id);
>  
> -    if ( d->tot_pages >= 10 && d->is_dying < DOMDYING_dead )
> +    if ( domain_tot_pages(d) >= 10 && d->is_dying < DOMDYING_dead )

Before I go any further - are you simply replacing _all_
->tot_pages uses by the new helper? In the case here, for
example, I don't think this is what we want.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v8 2/4] add a domain_tot_pages() helper function

[Durrant, Paul]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 30 January 2020 16:29
> To: Durrant, Paul <pdurrant@amazon.co.uk>
> Cc: xen-devel@lists.xenproject.org; Andrew Cooper
> <andrew.cooper3@citrix.com>; Wei Liu <wl@xen.org>; Roger Pau Monné
> <roger.pau@citrix.com>; George Dunlap <George.Dunlap@eu.citrix.com>; Ian
> Jackson <ian.jackson@eu.citrix.com>; Julien Grall <julien@xen.org>; Konrad
> Rzeszutek Wilk <konrad.wilk@oracle.com>; Stefano Stabellini
> <sstabellini@kernel.org>; Tim Deegan <tim@xen.org>
> Subject: Re: [PATCH v8 2/4] add a domain_tot_pages() helper function
> 
> On 30.01.2020 15:57, Paul Durrant wrote:
> > --- a/xen/arch/x86/domain.c
> > +++ b/xen/arch/x86/domain.c
> > @@ -218,7 +218,7 @@ void dump_pageframe_info(struct domain *d)
> >
> >      printk("Memory pages belonging to domain %u:\n", d->domain_id);
> >
> > -    if ( d->tot_pages >= 10 && d->is_dying < DOMDYING_dead )
> > +    if ( domain_tot_pages(d) >= 10 && d->is_dying < DOMDYING_dead )
> 
> Before I go any further - are you simply replacing _all_
> ->tot_pages uses by the new helper?

Basically, apart from domain_adjust_tot_pages(), yes.

> In the case here, for
> example, I don't think this is what we want.
> 

Why not? I would have thought any 'extra' pages would always be of interest.

  Paul
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v8 2/4] add a domain_tot_pages() helper function

[Jan Beulich]

On 30.01.2020 17:32, Durrant, Paul wrote:
>> -----Original Message-----
>> From: Jan Beulich <jbeulich@suse.com>
>> Sent: 30 January 2020 16:29
>> To: Durrant, Paul <pdurrant@amazon.co.uk>
>> Cc: xen-devel@lists.xenproject.org; Andrew Cooper
>> <andrew.cooper3@citrix.com>; Wei Liu <wl@xen.org>; Roger Pau Monné
>> <roger.pau@citrix.com>; George Dunlap <George.Dunlap@eu.citrix.com>; Ian
>> Jackson <ian.jackson@eu.citrix.com>; Julien Grall <julien@xen.org>; Konrad
>> Rzeszutek Wilk <konrad.wilk@oracle.com>; Stefano Stabellini
>> <sstabellini@kernel.org>; Tim Deegan <tim@xen.org>
>> Subject: Re: [PATCH v8 2/4] add a domain_tot_pages() helper function
>>
>> On 30.01.2020 15:57, Paul Durrant wrote:
>>> --- a/xen/arch/x86/domain.c
>>> +++ b/xen/arch/x86/domain.c
>>> @@ -218,7 +218,7 @@ void dump_pageframe_info(struct domain *d)
>>>
>>>      printk("Memory pages belonging to domain %u:\n", d->domain_id);
>>>
>>> -    if ( d->tot_pages >= 10 && d->is_dying < DOMDYING_dead )
>>> +    if ( domain_tot_pages(d) >= 10 && d->is_dying < DOMDYING_dead )
>>
>> Before I go any further - are you simply replacing _all_
>> ->tot_pages uses by the new helper?
> 
> Basically, apart from domain_adjust_tot_pages(), yes.
> 
>> In the case here, for
>> example, I don't think this is what we want.
>>
> 
> Why not? I would have thought any 'extra' pages would always be of interest.

Could be hundreds or thousands in the future. As long as it's in
reality just one, perhaps it indeed doesn't matter much. I'll
take a closer look tomorrow, to see if there are other places
where the adjusted count would better not be used. From my
partial audit in the morning I seem to recall that there were
both kinds of situations...

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v8 2/4] add a domain_tot_pages() helper function

[Jan Beulich]

On 30.01.2020 15:57, Paul Durrant wrote:
> v8:
>  - New in v8
> ---
>  xen/arch/x86/domain.c           |  2 +-
>  xen/arch/x86/mm.c               |  6 +++---
>  xen/arch/x86/mm/p2m-pod.c       | 10 +++++-----
>  xen/arch/x86/mm/shadow/common.c |  2 +-
>  xen/arch/x86/msi.c              |  2 +-
>  xen/arch/x86/numa.c             |  2 +-
>  xen/arch/x86/pv/dom0_build.c    | 25 +++++++++++++------------
>  xen/arch/x86/pv/domain.c        |  2 +-
>  xen/common/domctl.c             |  2 +-
>  xen/common/grant_table.c        |  4 ++--
>  xen/common/keyhandler.c         |  2 +-
>  xen/common/memory.c             |  4 ++--
>  xen/common/page_alloc.c         | 15 ++++++++-------
>  xen/include/public/memory.h     |  4 ++--
>  xen/include/xen/sched.h         | 24 ++++++++++++++++++------
>  15 files changed, 60 insertions(+), 46 deletions(-)

From this, with the comment you add next to the struct field, and
with your reply yesterday, what about the uses in
- arch/arm/arm64/domctl.c:switch_mode(),
- arch/x86/pv/shim.c:pv_shim_setup_dom(),
- arch/x86/pv/shim.c:write_start_info()?

> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -4194,8 +4194,8 @@ long do_mmu_update(
>   * - page caching attributes cleaned up
>   * - removed from the domain's page_list
>   *
> - * If MEMF_no_refcount is not set, the domain's tot_pages will be
> - * adjusted.  If this results in the page count falling to 0,
> + * If MEMF_no_refcount is not set, the domain_adjust_tot_pages() will
> + * be called.  If this results in the page count falling to 0,
>   * put_domain() will be called.

If you fiddle with this comment, please also drop the "the" ahead
of the function name. Unless you as a native speaker would confirm
it's appropriate there (it doesn't seem so to me). Of course I
also wouldn't mind leaving this untouched altogether.

> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -717,7 +717,7 @@ static long memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
>  
>                  /*
>                   * Pages in in_chunk_list is stolen without
> -                 * decreasing the tot_pages. If the domain is dying when
> +                 * decreasing domain_tot_pages(). If the domain is dying when

I'd leave this comment alone, or at least not use the function
name. Maybe do as you did in the public header?

> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -364,12 +364,18 @@ struct domain
>      spinlock_t       page_alloc_lock; /* protects all the following fields  */
>      struct page_list_head page_list;  /* linked list */
>      struct page_list_head xenpage_list; /* linked list (size xenheap_pages) */
> -    unsigned int     tot_pages;       /* number of pages currently possesed */
> -    unsigned int     xenheap_pages;   /* # pages allocated from Xen heap    */
> -    unsigned int     outstanding_pages; /* pages claimed but not possessed  */
> -    unsigned int     max_pages;       /* maximum value for tot_pages        */
> -    atomic_t         shr_pages;       /* number of shared pages             */
> -    atomic_t         paged_pages;     /* number of paged-out pages          */
> +
> +    /*
> +     * This field should only be directly accessed by domain_adjust_tot_pages()
> +     * and the domain_tot_pages() helper function defined below.
> +     */
> +    unsigned int     tot_pages;

If the three missing ones got taken care of, with there being arguments
both pro and con your change to dump_pageframe_info(), I'd be okay with
it getting changed as you do, to not render this comment partially
wrong.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v8 1/4] x86 / vmx: move teardown from domain_destroy()...

[Jan Beulich]

On 30.01.2020 15:57, Paul Durrant wrote:
> ... to domain_relinquish_resources().
> 
> The teardown code frees the APICv page. This does not need to be done late
> so do it in domain_relinquish_resources() rather than domain_destroy().
> 
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>

Btw., this can have my
Reviewed-by: Jan Beulich <jbeulich@suse.com>
as soon as "x86/HVM: relinquish resources also from hvm_domain_destroy()"
has gone in. But that's still pending an ack or otherwise by you.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v8 1/4] x86 / vmx: move teardown from domain_destroy()...

[Durrant, Paul]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 31 January 2020 13:32
> To: Durrant, Paul <pdurrant@amazon.co.uk>
> Cc: xen-devel@lists.xenproject.org; Jun Nakajima <jun.nakajima@intel.com>;
> Kevin Tian <kevin.tian@intel.com>; Andrew Cooper
> <andrew.cooper3@citrix.com>; Wei Liu <wl@xen.org>; Roger Pau Monné
> <roger.pau@citrix.com>; George Dunlap <george.dunlap@citrix.com>
> Subject: Re: [PATCH v8 1/4] x86 / vmx: move teardown from
> domain_destroy()...
> 
> On 30.01.2020 15:57, Paul Durrant wrote:
> > ... to domain_relinquish_resources().
> >
> > The teardown code frees the APICv page. This does not need to be done
> late
> > so do it in domain_relinquish_resources() rather than domain_destroy().
> >
> > Signed-off-by: Paul Durrant <pdurrant@amazon.com>
> 
> Btw., this can have my
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> as soon as "x86/HVM: relinquish resources also from hvm_domain_destroy()"
> has gone in. But that's still pending an ack or otherwise by you.
> 

Ok, thanks, I'll get on and review that.

  Paul
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v8 2/4] add a domain_tot_pages() helper function

[Durrant, Paul]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: 31 January 2020 12:53
> To: Durrant, Paul <pdurrant@amazon.co.uk>
> Cc: xen-devel@lists.xenproject.org; Andrew Cooper
> <andrew.cooper3@citrix.com>; Wei Liu <wl@xen.org>; Roger Pau Monné
> <roger.pau@citrix.com>; George Dunlap <George.Dunlap@eu.citrix.com>; Ian
> Jackson <ian.jackson@eu.citrix.com>; Julien Grall <julien@xen.org>; Konrad
> Rzeszutek Wilk <konrad.wilk@oracle.com>; Stefano Stabellini
> <sstabellini@kernel.org>; Tim Deegan <tim@xen.org>
> Subject: Re: [PATCH v8 2/4] add a domain_tot_pages() helper function
> 
> On 30.01.2020 15:57, Paul Durrant wrote:
> > v8:
> >  - New in v8
> > ---
> >  xen/arch/x86/domain.c           |  2 +-
> >  xen/arch/x86/mm.c               |  6 +++---
> >  xen/arch/x86/mm/p2m-pod.c       | 10 +++++-----
> >  xen/arch/x86/mm/shadow/common.c |  2 +-
> >  xen/arch/x86/msi.c              |  2 +-
> >  xen/arch/x86/numa.c             |  2 +-
> >  xen/arch/x86/pv/dom0_build.c    | 25 +++++++++++++------------
> >  xen/arch/x86/pv/domain.c        |  2 +-
> >  xen/common/domctl.c             |  2 +-
> >  xen/common/grant_table.c        |  4 ++--
> >  xen/common/keyhandler.c         |  2 +-
> >  xen/common/memory.c             |  4 ++--
> >  xen/common/page_alloc.c         | 15 ++++++++-------
> >  xen/include/public/memory.h     |  4 ++--
> >  xen/include/xen/sched.h         | 24 ++++++++++++++++++------
> >  15 files changed, 60 insertions(+), 46 deletions(-)
> 
> From this, with the comment you add next to the struct field, and
> with your reply yesterday, what about the uses in
> - arch/arm/arm64/domctl.c:switch_mode(),

TBH I'm not sure with that one. It looks to me like it needs to check whether the domain has *any* memory assigned. Perhaps checking page_list would be more appropriate. Perhaps Julien can comment?

> - arch/x86/pv/shim.c:pv_shim_setup_dom(),
> - arch/x86/pv/shim.c:write_start_info()?

It looks like both of those should be changed.

> 
> > --- a/xen/arch/x86/mm.c
> > +++ b/xen/arch/x86/mm.c
> > @@ -4194,8 +4194,8 @@ long do_mmu_update(
> >   * - page caching attributes cleaned up
> >   * - removed from the domain's page_list
> >   *
> > - * If MEMF_no_refcount is not set, the domain's tot_pages will be
> > - * adjusted.  If this results in the page count falling to 0,
> > + * If MEMF_no_refcount is not set, the domain_adjust_tot_pages() will
> > + * be called.  If this results in the page count falling to 0,
> >   * put_domain() will be called.
> 
> If you fiddle with this comment, please also drop the "the" ahead
> of the function name. Unless you as a native speaker would confirm
> it's appropriate there (it doesn't seem so to me). Of course I
> also wouldn't mind leaving this untouched altogether.
> 

Ok, I'll drop that hunk.

> > --- a/xen/common/memory.c
> > +++ b/xen/common/memory.c
> > @@ -717,7 +717,7 @@ static long
> memory_exchange(XEN_GUEST_HANDLE_PARAM(xen_memory_exchange_t) arg)
> >
> >                  /*
> >                   * Pages in in_chunk_list is stolen without
> > -                 * decreasing the tot_pages. If the domain is dying
> when
> > +                 * decreasing domain_tot_pages(). If the domain is
> dying when
> 
> I'd leave this comment alone, or at least not use the function
> name. Maybe do as you did in the public header?
> 

OK I'll leave this alone too.

> > --- a/xen/include/xen/sched.h
> > +++ b/xen/include/xen/sched.h
> > @@ -364,12 +364,18 @@ struct domain
> >      spinlock_t       page_alloc_lock; /* protects all the following
> fields  */
> >      struct page_list_head page_list;  /* linked list */
> >      struct page_list_head xenpage_list; /* linked list (size
> xenheap_pages) */
> > -    unsigned int     tot_pages;       /* number of pages currently
> possesed */
> > -    unsigned int     xenheap_pages;   /* # pages allocated from Xen
> heap    */
> > -    unsigned int     outstanding_pages; /* pages claimed but not
> possessed  */
> > -    unsigned int     max_pages;       /* maximum value for tot_pages
> */
> > -    atomic_t         shr_pages;       /* number of shared pages
> */
> > -    atomic_t         paged_pages;     /* number of paged-out pages
> */
> > +
> > +    /*
> > +     * This field should only be directly accessed by
> domain_adjust_tot_pages()
> > +     * and the domain_tot_pages() helper function defined below.
> > +     */
> > +    unsigned int     tot_pages;
> 
> If the three missing ones got taken care of, with there being arguments
> both pro and con your change to dump_pageframe_info(), I'd be okay with
> it getting changed as you do, to not render this comment partially
> wrong.

Ok.

  Paul

> 
> Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v8 2/4] add a domain_tot_pages() helper function

[Durrant, Paul]

> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of
> Durrant, Paul
> Sent: 31 January 2020 15:19
> To: Jan Beulich <jbeulich@suse.com>; Julien Grall <julien@xen.org>
> Cc: Stefano Stabellini <sstabellini@kernel.org>; Wei Liu <wl@xen.org>;
> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>; George Dunlap
> <George.Dunlap@eu.citrix.com>; Andrew Cooper <andrew.cooper3@citrix.com>;
> Ian Jackson <ian.jackson@eu.citrix.com>; Tim Deegan <tim@xen.org>; xen-
> devel@lists.xenproject.org; Roger Pau Monné <roger.pau@citrix.com>
> Subject: Re: [Xen-devel] [PATCH v8 2/4] add a domain_tot_pages() helper
> function
> 
> > -----Original Message-----
> > From: Jan Beulich <jbeulich@suse.com>
> > Sent: 31 January 2020 12:53
> > To: Durrant, Paul <pdurrant@amazon.co.uk>
> > Cc: xen-devel@lists.xenproject.org; Andrew Cooper
> > <andrew.cooper3@citrix.com>; Wei Liu <wl@xen.org>; Roger Pau Monné
> > <roger.pau@citrix.com>; George Dunlap <George.Dunlap@eu.citrix.com>; Ian
> > Jackson <ian.jackson@eu.citrix.com>; Julien Grall <julien@xen.org>;
> Konrad
> > Rzeszutek Wilk <konrad.wilk@oracle.com>; Stefano Stabellini
> > <sstabellini@kernel.org>; Tim Deegan <tim@xen.org>
> > Subject: Re: [PATCH v8 2/4] add a domain_tot_pages() helper function
> >
> > On 30.01.2020 15:57, Paul Durrant wrote:
> > > v8:
> > >  - New in v8
> > > ---
> > >  xen/arch/x86/domain.c           |  2 +-
> > >  xen/arch/x86/mm.c               |  6 +++---
> > >  xen/arch/x86/mm/p2m-pod.c       | 10 +++++-----
> > >  xen/arch/x86/mm/shadow/common.c |  2 +-
> > >  xen/arch/x86/msi.c              |  2 +-
> > >  xen/arch/x86/numa.c             |  2 +-
> > >  xen/arch/x86/pv/dom0_build.c    | 25 +++++++++++++------------
> > >  xen/arch/x86/pv/domain.c        |  2 +-
> > >  xen/common/domctl.c             |  2 +-
> > >  xen/common/grant_table.c        |  4 ++--
> > >  xen/common/keyhandler.c         |  2 +-
> > >  xen/common/memory.c             |  4 ++--
> > >  xen/common/page_alloc.c         | 15 ++++++++-------
> > >  xen/include/public/memory.h     |  4 ++--
> > >  xen/include/xen/sched.h         | 24 ++++++++++++++++++------
> > >  15 files changed, 60 insertions(+), 46 deletions(-)
> >
> > From this, with the comment you add next to the struct field, and
> > with your reply yesterday, what about the uses in
> > - arch/arm/arm64/domctl.c:switch_mode(),
> 
> TBH I'm not sure with that one. It looks to me like it needs to check
> whether the domain has *any* memory assigned. Perhaps checking page_list
> would be more appropriate. Perhaps Julien can comment?
> 

Having chatted to Julien, the aim of checking tot_pages here is to test whether the domain is newly created, in which case using domain_tot_pages() is the appropriate thing to do.

  Paul

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [Xen-devel] [PATCH v8 4/4] x86 / vmx: use a MEMF_no_refcount domheap page for APIC_DEFAULT_PHYS_BASE

[Tian, Kevin]

> From: Paul Durrant <pdurrant@amazon.com>
> Sent: Thursday, January 30, 2020 10:58 PM
> 
> vmx_alloc_vlapic_mapping() currently contains some very odd looking code
> that allocates a MEMF_no_owner domheap page and then shares with the
> guest
> as if it were a xenheap page. This then requires vmx_free_vlapic_mapping()
> to call a special function in the mm code: free_shared_domheap_page().
> 
> By using a MEMF_no_refcount domheap page instead, the odd looking code
> in
> vmx_alloc_vlapic_mapping() can simply use get_page_and_type() to set up a
> writable mapping before insertion in the P2M and
> vmx_free_vlapic_mapping()
> can simply release the page using put_page_alloc_ref() followed by
> put_page_and_type(). This then allows free_shared_domheap_page() to be
> purged.
> 
> Signed-off-by: Paul Durrant <pdurrant@amazon.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel