All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel <xen-devel@lists.xenproject.org>
Cc: "Michał Leszczyński" <michal.leszczynski@cert.pl>,
	"Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Jan Beulich" <JBeulich@suse.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>,
	"Anthony PERARD" <anthony.perard@citrix.com>,
	"Tamas K Lengyel" <tamas@tklengyel.com>
Subject: [PATCH v8 08/16] xen/domain: Add vmtrace_size domain creation parameter
Date: Sat, 30 Jan 2021 02:58:44 +0000	[thread overview]
Message-ID: <20210130025852.12430-9-andrew.cooper3@citrix.com> (raw)
In-Reply-To: <20210130025852.12430-1-andrew.cooper3@citrix.com>

From: Michał Leszczyński <michal.leszczynski@cert.pl>

To use vmtrace, buffers of a suitable size need allocating, and different
tasks will want different sizes.

Add a domain creation parameter, and audit it appropriately in the
{arch_,}sanitise_domain_config() functions.

For now, the x86 specific auditing is tuned to Processor Trace running in
Single Output mode, which requires a single contiguous range of memory.

The size is given an arbitrary limit of 64M which is expected to be enough for
anticipated usecases, but not large enough to get into long-running-hypercall
problems.

Signed-off-by: Michał Leszczyński <michal.leszczynski@cert.pl>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Anthony PERARD <anthony.perard@citrix.com>
CC: Michał Leszczyński <michal.leszczynski@cert.pl>
CC: Tamas K Lengyel <tamas@tklengyel.com>

When support for later generations of IPT get added, we can in principle start
to use ToTP which is a scatter list of smaller trace regions to use, if we
need to massively up the buffer size available.

v8:
 * Rename vmtrace_frames to vmtrace_size.  Reposition to fill a hole.
 * Rename vmtrace.buf to vmtrace.pg.
 * Rework the refcounting logic and comment it *very* clearly.

v7:
 * Major chop&change within the series.
 * Use the name 'vmtrace' consistently.
 * Use the (new) common vcpu_teardown() functionality, rather than leaving a
   latent memory leak on ARM.
---
 xen/arch/x86/domain.c       | 23 +++++++++++++
 xen/common/domain.c         | 79 +++++++++++++++++++++++++++++++++++++++++++++
 xen/include/public/domctl.h |  3 ++
 xen/include/xen/sched.h     |  6 ++++
 4 files changed, 111 insertions(+)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index b9ba04633e..6c7ee25f3b 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -660,6 +660,29 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config)
         return -EINVAL;
     }
 
+    if ( config->vmtrace_size )
+    {
+        unsigned int size = config->vmtrace_size;
+
+        ASSERT(vmtrace_available); /* Checked by common code. */
+
+        /*
+         * For now, vmtrace is restricted to HVM guests, and using a
+         * power-of-2 buffer between 4k and 64M in size.
+         */
+        if ( !hvm )
+        {
+            dprintk(XENLOG_INFO, "vmtrace not supported for PV\n");
+            return -EINVAL;
+        }
+
+        if ( size < PAGE_SIZE || size > MB(64) || (size & (size - 1)) )
+        {
+            dprintk(XENLOG_INFO, "Unsupported vmtrace size: %#x\n", size);
+            return -EINVAL;
+        }
+    }
+
     return 0;
 }
 
diff --git a/xen/common/domain.c b/xen/common/domain.c
index d1e94d88cf..491b32812e 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -132,6 +132,71 @@ static void vcpu_info_reset(struct vcpu *v)
     v->vcpu_info_mfn = INVALID_MFN;
 }
 
+static void vmtrace_free_buffer(struct vcpu *v)
+{
+    const struct domain *d = v->domain;
+    struct page_info *pg = v->vmtrace.pg;
+    unsigned int i;
+
+    if ( !pg )
+        return;
+
+    v->vmtrace.pg = NULL;
+
+    for ( i = 0; i < (d->vmtrace_size >> PAGE_SHIFT); i++ )
+    {
+        put_page_alloc_ref(&pg[i]);
+        put_page_and_type(&pg[i]);
+    }
+}
+
+static int vmtrace_alloc_buffer(struct vcpu *v)
+{
+    struct domain *d = v->domain;
+    struct page_info *pg;
+    unsigned int i;
+
+    if ( !d->vmtrace_size )
+        return 0;
+
+    pg = alloc_domheap_pages(d, get_order_from_bytes(d->vmtrace_size),
+                             MEMF_no_refcount);
+    if ( !pg )
+        return -ENOMEM;
+
+    /*
+     * Getting the reference counting correct here is hard.
+     *
+     * All pages are now on the domlist.  They, or subranges within, will be
+     * freed when their reference count drops to zero, which may any time
+     * between now and the domain teardown path.
+     */
+
+    for ( i = 0; i < (d->vmtrace_size >> PAGE_SHIFT); i++ )
+        if ( unlikely(!get_page_and_type(&pg[i], d, PGT_writable_page)) )
+            goto refcnt_err;
+
+    /*
+     * We must only let vmtrace_free_buffer() take any action in the success
+     * case when we've taken all the refs it intends to drop.
+     */
+    v->vmtrace.pg = pg;
+
+    return 0;
+
+ refcnt_err:
+    /*
+     * In the failure case, we must drop all the acquired typerefs thus far,
+     * skip vmtrace_free_buffer(), and leave domain_relinquish_resources() to
+     * drop the alloc refs on any remaining pages - some pages could already
+     * have been freed behind our backs.
+     */
+    while ( i-- )
+        put_page_and_type(&pg[i]);
+
+    return -ENODATA;
+}
+
 /*
  * Release resources held by a vcpu.  There may or may not be live references
  * to the vcpu, and it may or may not be fully constructed.
@@ -140,6 +205,8 @@ static void vcpu_info_reset(struct vcpu *v)
  */
 static int vcpu_teardown(struct vcpu *v)
 {
+    vmtrace_free_buffer(v);
+
     return 0;
 }
 
@@ -201,6 +268,9 @@ struct vcpu *vcpu_create(struct domain *d, unsigned int vcpu_id)
     if ( sched_init_vcpu(v) != 0 )
         goto fail_wq;
 
+    if ( vmtrace_alloc_buffer(v) != 0 )
+        goto fail_wq;
+
     if ( arch_vcpu_create(v) != 0 )
         goto fail_sched;
 
@@ -449,6 +519,12 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config)
         }
     }
 
+    if ( config->vmtrace_size && !vmtrace_available )
+    {
+        dprintk(XENLOG_INFO, "vmtrace requested but not available\n");
+        return -EINVAL;
+    }
+
     return arch_sanitise_domain_config(config);
 }
 
@@ -474,7 +550,10 @@ struct domain *domain_create(domid_t domid,
     ASSERT(is_system_domain(d) ? config == NULL : config != NULL);
 
     if ( config )
+    {
         d->options = config->flags;
+        d->vmtrace_size = config->vmtrace_size;
+    }
 
     /* Sort out our idea of is_control_domain(). */
     d->is_privileged = is_priv;
diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h
index 666aeb71bf..88a5b1ef5d 100644
--- a/xen/include/public/domctl.h
+++ b/xen/include/public/domctl.h
@@ -95,6 +95,9 @@ struct xen_domctl_createdomain {
     int32_t max_grant_frames;
     int32_t max_maptrack_frames;
 
+    /* Per-vCPU buffer size in bytes.  0 to disable. */
+    uint32_t vmtrace_size;
+
     struct xen_arch_domainconfig arch;
 };
 
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 06dba1a397..bc78a09a53 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -272,6 +272,10 @@ struct vcpu
     /* vPCI per-vCPU area, used to store data for long running operations. */
     struct vpci_vcpu vpci;
 
+    struct {
+        struct page_info *pg; /* One contiguous allocation of d->vmtrace_size */
+    } vmtrace;
+
     struct arch_vcpu arch;
 
 #ifdef CONFIG_IOREQ_SERVER
@@ -547,6 +551,8 @@ struct domain
         unsigned int guest_request_sync          : 1;
     } monitor;
 
+    unsigned int vmtrace_size; /* Buffer size in bytes, or 0 to disable. */
+
 #ifdef CONFIG_ARGO
     /* Argo interdomain communication support */
     struct argo_domain *argo;
-- 
2.11.0



  parent reply	other threads:[~2021-01-30  2:59 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-30  2:58 [PATCH v8 00/16] acquire_resource size and external IPT monitoring Andrew Cooper
2021-01-30  2:58 ` [PATCH v8 01/16] xen/memory: Reject out-of-range resource 'frame' values Andrew Cooper
2021-01-30  2:58 ` [PATCH v8 02/16] xen/gnttab: Rework resource acquisition Andrew Cooper
2021-01-30  2:58 ` [PATCH v8 03/16] xen/memory: Fix acquire_resource size semantics Andrew Cooper
2021-01-30  2:58 ` [PATCH v8 04/16] xen/memory: Improve compat XENMEM_acquire_resource handling Andrew Cooper
2021-01-30  2:58 ` [PATCH v8 05/16] xen/memory: Indent part of acquire_resource() Andrew Cooper
2021-01-30  2:58 ` [PATCH v8 06/16] xen/memory: Fix mapping grant tables with XENMEM_acquire_resource Andrew Cooper
2021-02-01 10:10   ` Roger Pau Monné
2021-02-01 11:11     ` Andrew Cooper
2021-02-01 12:07       ` Roger Pau Monné
2021-02-01 12:10         ` Andrew Cooper
2021-02-01 13:03       ` Jan Beulich
2021-02-01 14:04         ` Andrew Cooper
2021-02-01 14:32           ` Jan Beulich
2021-01-30  2:58 ` [PATCH v8 07/16] xen+tools: Introduce XEN_SYSCTL_PHYSCAP_vmtrace Andrew Cooper
2021-02-01 10:32   ` Roger Pau Monné
2021-01-30  2:58 ` Andrew Cooper [this message]
2021-02-01 10:51   ` [PATCH v8 08/16] xen/domain: Add vmtrace_size domain creation parameter Roger Pau Monné
2021-02-01 11:19     ` Andrew Cooper
2021-02-01 13:18   ` Jan Beulich
2021-02-01 14:22     ` Andrew Cooper
2021-02-01 14:36       ` Jan Beulich
2021-02-01 22:14         ` Andrew Cooper
2021-01-30  2:58 ` [PATCH v8 09/16] tools/[lib]xl: Add vmtrace_buf_size parameter Andrew Cooper
2021-01-30  2:58 ` [PATCH v8 10/16] xen/memory: Add a vmtrace_buf resource type Andrew Cooper
2021-01-30  2:58 ` [PATCH v8 11/16] x86/vmx: Add Intel Processor Trace support Andrew Cooper
2021-01-30  2:58 ` [PATCH v8 12/16] xen/domctl: Add XEN_DOMCTL_vmtrace_op Andrew Cooper
2021-02-01 12:01   ` Roger Pau Monné
2021-02-01 13:00     ` Andrew Cooper
2021-02-01 14:27       ` Roger Pau Monné
2021-01-30  2:58 ` [PATCH v8 13/16] tools/libxc: Add xc_vmtrace_* functions Andrew Cooper
2021-01-30  2:58 ` [PATCH v8 14/16] tools/misc: Add xen-vmtrace tool Andrew Cooper
2021-01-30  2:58 ` [PATCH v8 15/16] xen/vmtrace: support for VM forks Andrew Cooper
2021-01-30  2:58 ` [PATCH v8 16/16] x86/vm_event: Carry the vmtrace buffer position in vm_event Andrew Cooper
2021-02-01  9:51   ` Jan Beulich
2021-02-01 12:34 ` [PATCH v8 00/16] acquire_resource size and external IPT monitoring Oleksandr
2021-02-01 13:07   ` Andrew Cooper
2021-02-01 13:47     ` Oleksandr
2021-02-01 14:00       ` Andrew Cooper
2021-02-02 12:09         ` Oleksandr

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210130025852.12430-9-andrew.cooper3@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=JBeulich@suse.com \
    --cc=anthony.perard@citrix.com \
    --cc=michal.leszczynski@cert.pl \
    --cc=roger.pau@citrix.com \
    --cc=tamas@tklengyel.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.