diff mbox series

[v9,03/11] tools/[lib]xl: Add vmtrace_buf_size parameter

Message ID 20210201232703.29275-4-andrew.cooper3@citrix.com (mailing list archive)
State New
Headers show
Series acquire_resource size and external IPT monitoring | expand

Commit Message

Andrew Cooper Feb. 1, 2021, 11:26 p.m. UTC
From: Michał Leszczyński <michal.leszczynski@cert.pl>

Allow to specify the size of per-vCPU trace buffer upon
domain creation. This is zero by default (meaning: not enabled).

Signed-off-by: Michał Leszczyński <michal.leszczynski@cert.pl>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
---
CC: Ian Jackson <iwj@xenproject.org>
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>

v8:
 * Rebase over vmtrace_size change.
v7:
 * Use the name 'vmtrace' consistently.
---
 docs/man/xl.cfg.5.pod.in             | 9 +++++++++
 tools/golang/xenlight/helpers.gen.go | 2 ++
 tools/golang/xenlight/types.gen.go   | 1 +
 tools/include/libxl.h                | 7 +++++++
 tools/libs/light/libxl_create.c      | 1 +
 tools/libs/light/libxl_types.idl     | 4 ++++
 tools/xl/xl_parse.c                  | 4 ++++
 7 files changed, 28 insertions(+)

Comments

Ian Jackson Feb. 2, 2021, 12:16 p.m. UTC | #1
Andrew Cooper writes ("[PATCH v9 03/11] tools/[lib]xl: Add vmtrace_buf_size parameter"):
> From: Michał Leszczyński <michal.leszczynski@cert.pl>
> 
> Allow to specify the size of per-vCPU trace buffer upon
> domain creation. This is zero by default (meaning: not enabled).
...

Wearing my maintainer/reviewer hat:

Release risk assessment for this patch:

 * This contains golang changes which might break the build or need
   updates to golang generated files.  This ought to be detected by
   our tests so we can fix it.  At this stage of the release that is
   probably OK.  The risk of actually shipping a broken build is low.

 * The patch introduces a new libxl config parameter.  That has API
   and UI implications.  But it is a very small change and the
   semantics are fairly obvious.  The name likewise is fine.  So I am
   very comfortable with recommending this late addition to these
   APIs.

 * The patch contains buffer size handling code.  In the general case
   that might produce a risk of buffer overruns.  But at least here in
   this patch this is actually just the configured size of a buffer,
   and actual length/use checks are done elsewhere, so this is is not
   a real risk.

Ian.
Ian Jackson Feb. 2, 2021, 12:17 p.m. UTC | #2
Ian Jackson writes ("Re: [PATCH v9 03/11] tools/[lib]xl: Add vmtrace_buf_size parameter"):
> Andrew Cooper writes ("[PATCH v9 03/11] tools/[lib]xl: Add vmtrace_buf_size parameter"):
> > From: Michał Leszczyński <michal.leszczynski@cert.pl>
> > 
> > Allow to specify the size of per-vCPU trace buffer upon
> > domain creation. This is zero by default (meaning: not enabled).
> ...
> 
> Wearing my maintainer/reviewer hat:
> 
> Release risk assessment for this patch:
> 
>  * This contains golang changes which might break the build or need
>    updates to golang generated files.  This ought to be detected by
>    our tests so we can fix it.  At this stage of the release that is
>    probably OK.  The risk of actually shipping a broken build is low.
> 
>  * The patch introduces a new libxl config parameter.  That has API
>    and UI implications.  But it is a very small change and the
>    semantics are fairly obvious.  The name likewise is fine.  So I am
>    very comfortable with recommending this late addition to these
>    APIs.
> 
>  * The patch contains buffer size handling code.  In the general case
>    that might produce a risk of buffer overruns.  But at least here in
>    this patch this is actually just the configured size of a buffer,
>    and actual length/use checks are done elsewhere, so this is is not
>    a real risk.

Consequently, wearing my RM hat, this patch:

Release-Acked-by: Ian Jackson <iwj@xenproject.org>
diff mbox series

Patch

diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index 7cdb8595d3..040374dcd6 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -681,6 +681,15 @@  Windows).
 
 If this option is not specified then it will default to B<false>.
 
+=item B<vmtrace_buf_kb=KBYTES>
+
+Specifies the size of vmtrace buffer that would be allocated for each
+vCPU belonging to this domain.  Disabled (i.e.  B<vmtrace_buf_kb=0>) by
+default.
+
+B<NOTE>: Acceptable values are platform specific.  For Intel Processor
+Trace, this value must be a power of 2 between 4k and 16M.
+
 =back
 
 =head2 Devices
diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go
index 63e2876463..4c60d27a9c 100644
--- a/tools/golang/xenlight/helpers.gen.go
+++ b/tools/golang/xenlight/helpers.gen.go
@@ -1114,6 +1114,7 @@  return fmt.Errorf("invalid union key '%v'", x.Type)}
 x.ArchArm.GicVersion = GicVersion(xc.arch_arm.gic_version)
 x.ArchArm.Vuart = VuartType(xc.arch_arm.vuart)
 x.Altp2M = Altp2MMode(xc.altp2m)
+x.VmtraceBufKb = int(xc.vmtrace_buf_kb)
 
  return nil}
 
@@ -1589,6 +1590,7 @@  return fmt.Errorf("invalid union key '%v'", x.Type)}
 xc.arch_arm.gic_version = C.libxl_gic_version(x.ArchArm.GicVersion)
 xc.arch_arm.vuart = C.libxl_vuart_type(x.ArchArm.Vuart)
 xc.altp2m = C.libxl_altp2m_mode(x.Altp2M)
+xc.vmtrace_buf_kb = C.int(x.VmtraceBufKb)
 
  return nil
  }
diff --git a/tools/golang/xenlight/types.gen.go b/tools/golang/xenlight/types.gen.go
index 5851c38057..cb13002fdb 100644
--- a/tools/golang/xenlight/types.gen.go
+++ b/tools/golang/xenlight/types.gen.go
@@ -514,6 +514,7 @@  GicVersion GicVersion
 Vuart VuartType
 }
 Altp2M Altp2MMode
+VmtraceBufKb int
 }
 
 type domainBuildInfoTypeUnion interface {
diff --git a/tools/include/libxl.h b/tools/include/libxl.h
index f48d0c5e8a..a7b673e89d 100644
--- a/tools/include/libxl.h
+++ b/tools/include/libxl.h
@@ -489,6 +489,13 @@ 
 #define LIBXL_HAVE_PHYSINFO_CAP_VMTRACE 1
 
 /*
+ * LIBXL_HAVE_VMTRACE_BUF_KB indicates that libxl_domain_create_info has a
+ * vmtrace_buf_kb parameter, which allows to enable pre-allocation of
+ * processor tracing buffers of given size.
+ */
+#define LIBXL_HAVE_VMTRACE_BUF_KB 1
+
+/*
  * libxl ABI compatibility
  *
  * The only guarantee which libxl makes regarding ABI compatibility
diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
index 9848d65f36..46f68da697 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -607,6 +607,7 @@  int libxl__domain_make(libxl__gc *gc, libxl_domain_config *d_config,
             .max_evtchn_port = b_info->event_channels,
             .max_grant_frames = b_info->max_grant_frames,
             .max_maptrack_frames = b_info->max_maptrack_frames,
+            .vmtrace_size = ROUNDUP(b_info->vmtrace_buf_kb << 10, XC_PAGE_SHIFT),
         };
 
         if (info->type != LIBXL_DOMAIN_TYPE_PV) {
diff --git a/tools/libs/light/libxl_types.idl b/tools/libs/light/libxl_types.idl
index dacb7df6b7..5b85a7419f 100644
--- a/tools/libs/light/libxl_types.idl
+++ b/tools/libs/light/libxl_types.idl
@@ -648,6 +648,10 @@  libxl_domain_build_info = Struct("domain_build_info",[
     # supported by x86 HVM and ARM support is planned.
     ("altp2m", libxl_altp2m_mode),
 
+    # Size of preallocated vmtrace trace buffers (in KBYTES).
+    # Use zero value to disable this feature.
+    ("vmtrace_buf_kb", integer),
+
     ], dir=DIR_IN,
        copy_deprecated_fn="libxl__domain_build_info_copy_deprecated",
 )
diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
index 867e4d068a..1893cfc086 100644
--- a/tools/xl/xl_parse.c
+++ b/tools/xl/xl_parse.c
@@ -1863,6 +1863,10 @@  void parse_config_data(const char *config_source,
         }
     }
 
+    if (!xlu_cfg_get_long(config, "vmtrace_buf_kb", &l, 1) && l) {
+        b_info->vmtrace_buf_kb = l;
+    }
+
     if (!xlu_cfg_get_list(config, "ioports", &ioports, &num_ioports, 0)) {
         b_info->num_ioports = num_ioports;
         b_info->ioports = calloc(num_ioports, sizeof(*b_info->ioports));