| Message ID | 20210201232703.29275-4-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | acquire_resource size and external IPT monitoring | expand |
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 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 --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));