| Message ID | f7e3c91789a7763b997918b6ebb987be670f9ce5.1593974333.git.michal.leszczynski@cert.pl (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Implement support for external IPT monitoring | expand |
----- 5 lip 2020 o 20:54, Michał Leszczyński michal.leszczynski@cert.pl napisał(a): > From: Michal Leszczynski <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: Michal Leszczynski <michal.leszczynski@cert.pl> > --- > docs/man/xl.cfg.5.pod.in | 11 +++++++++++ > tools/golang/xenlight/helpers.gen.go | 2 ++ > tools/golang/xenlight/types.gen.go | 1 + > tools/libxl/libxl.h | 8 ++++++++ > tools/libxl/libxl_create.c | 1 + > tools/libxl/libxl_types.idl | 2 ++ > tools/xl/xl_parse.c | 22 ++++++++++++++++++++++ > 7 files changed, 47 insertions(+) > > diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in > index 0532739c1f..670759f6bd 100644 > --- a/docs/man/xl.cfg.5.pod.in > +++ b/docs/man/xl.cfg.5.pod.in > @@ -278,6 +278,17 @@ memory=8096 will report significantly less memory available > for use > than a system with maxmem=8096 memory=8096 due to the memory overhead > of having to track the unused pages. > > +=item B<processor_trace_buffer_size=BYTES> > + > +Specifies the size of processor trace buffer that would be allocated > +for each vCPU belonging to this domain. Disabled (i.e. > +B<processor_trace_buffer_size=0> by default. This must be set to > +non-zero value in order to be able to use processor tracing features > +with this domain. > + > +B<NOTE>: The size value must be between 4 kB and 4 GB and it must > +be also a power of 2. > + > =back > > =head3 Guest Virtual NUMA Configuration > diff --git a/tools/golang/xenlight/helpers.gen.go > b/tools/golang/xenlight/helpers.gen.go > index 152c7e8e6b..bfc37b69c8 100644 > --- a/tools/golang/xenlight/helpers.gen.go > +++ b/tools/golang/xenlight/helpers.gen.go > @@ -1117,6 +1117,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.VmtracePtOrder = int(xc.vmtrace_pt_order) > > return nil} > > @@ -1592,6 +1593,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_pt_order = C.int(x.VmtracePtOrder) > > return nil > } > diff --git a/tools/golang/xenlight/types.gen.go > b/tools/golang/xenlight/types.gen.go > index 663c1e86b4..f9b07ac862 100644 > --- a/tools/golang/xenlight/types.gen.go > +++ b/tools/golang/xenlight/types.gen.go > @@ -516,6 +516,7 @@ GicVersion GicVersion > Vuart VuartType > } > Altp2M Altp2MMode > +VmtracePtOrder int > } > > type domainBuildInfoTypeUnion interface { > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h > index 1cd6c38e83..4abb521756 100644 > --- a/tools/libxl/libxl.h > +++ b/tools/libxl/libxl.h > @@ -438,6 +438,14 @@ > */ > #define LIBXL_HAVE_CREATEINFO_PASSTHROUGH 1 > > +/* > + * LIBXL_HAVE_VMTRACE_PT_ORDER indicates that > + * libxl_domain_create_info has a vmtrace_pt_order parameter, which > + * allows to enable pre-allocation of processor tracing buffers > + * with the given order of size. > + */ > +#define LIBXL_HAVE_VMTRACE_PT_ORDER 1 > + > /* > * libxl ABI compatibility > * > diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c > index 2814818e34..82b595161a 100644 > --- a/tools/libxl/libxl_create.c > +++ b/tools/libxl/libxl_create.c > @@ -608,6 +608,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_pt_order = b_info->vmtrace_pt_order, > }; > > if (info->type != LIBXL_DOMAIN_TYPE_PV) { > diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl > index 9d3f05f399..1c5dd43e4d 100644 > --- a/tools/libxl/libxl_types.idl > +++ b/tools/libxl/libxl_types.idl > @@ -645,6 +645,8 @@ libxl_domain_build_info = Struct("domain_build_info",[ > # supported by x86 HVM and ARM support is planned. > ("altp2m", libxl_altp2m_mode), > > + ("vmtrace_pt_order", 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 61b4ef7b7e..279f7c14d3 100644 > --- a/tools/xl/xl_parse.c > +++ b/tools/xl/xl_parse.c > @@ -1861,6 +1861,28 @@ void parse_config_data(const char *config_source, > } > } > > + if (!xlu_cfg_get_long(config, "processor_trace_buffer_size", &l, 1) && l) { > + int32_t shift = 0; > + > + if (l & (l - 1)) > + { > + fprintf(stderr, "ERROR: processor_trace_buffer_size " > + "- must be a power of 2\n"); > + exit(1); > + } > + > + while (l >>= 1) ++shift; > + > + if (shift <= XEN_PAGE_SHIFT) > + { > + fprintf(stderr, "ERROR: processor_trace_buffer_size " > + "- value is too small\n"); > + exit(1); > + } > + > + b_info->vmtrace_pt_order = shift - XEN_PAGE_SHIFT; > + } > + > 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)); > -- > 2.17.1 As there were many different ideas about how the naming scheme should be and what kinds of values should be passed where, I would like to discuss this particular topic. Right now we have it pretty confusing: * user sets "processor_trace_buffer_size" option in xl.cfg * domain creation hypercall uses "vmtrace_pt_order" (derived from above) * hypervisor side stores "vmtrace_pt_size" (converted back to bytes) Best regards, Michał Leszczyński CERT Polska
Hi, On 05/07/2020 20:02, Michał Leszczyński wrote: > ----- 5 lip 2020 o 20:54, Michał Leszczyński michal.leszczynski@cert.pl napisał(a): > >> From: Michal Leszczynski <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: Michal Leszczynski <michal.leszczynski@cert.pl> >> --- >> docs/man/xl.cfg.5.pod.in | 11 +++++++++++ >> tools/golang/xenlight/helpers.gen.go | 2 ++ >> tools/golang/xenlight/types.gen.go | 1 + >> tools/libxl/libxl.h | 8 ++++++++ >> tools/libxl/libxl_create.c | 1 + >> tools/libxl/libxl_types.idl | 2 ++ >> tools/xl/xl_parse.c | 22 ++++++++++++++++++++++ >> 7 files changed, 47 insertions(+) >> >> diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in >> index 0532739c1f..670759f6bd 100644 >> --- a/docs/man/xl.cfg.5.pod.in >> +++ b/docs/man/xl.cfg.5.pod.in >> @@ -278,6 +278,17 @@ memory=8096 will report significantly less memory available >> for use >> than a system with maxmem=8096 memory=8096 due to the memory overhead >> of having to track the unused pages. >> >> +=item B<processor_trace_buffer_size=BYTES> >> + >> +Specifies the size of processor trace buffer that would be allocated >> +for each vCPU belonging to this domain. Disabled (i.e. >> +B<processor_trace_buffer_size=0> by default. This must be set to >> +non-zero value in order to be able to use processor tracing features >> +with this domain. >> + >> +B<NOTE>: The size value must be between 4 kB and 4 GB and it must >> +be also a power of 2. This seems to suggest that 4 kB is allowed. But looking at the code below, you are forbidding the value. [...] > As there were many different ideas about how the naming scheme should be > and what kinds of values should be passed where, I would like to discuss > this particular topic. Right now we have it pretty confusing: > > * user sets "processor_trace_buffer_size" option in xl.cfg > * domain creation hypercall uses "vmtrace_pt_order" (derived from above) You don't only use the order in the hypercall but also the public interface of libxl. > * hypervisor side stores "vmtrace_pt_size" (converted back to bytes) My preference would be to use the size everywhere, but if one still prefer to use the order in the hypercall then the libxl interface should use the size. See my comment in v4 for the rationale. Cheers,
diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in index 0532739c1f..670759f6bd 100644 --- a/docs/man/xl.cfg.5.pod.in +++ b/docs/man/xl.cfg.5.pod.in @@ -278,6 +278,17 @@ memory=8096 will report significantly less memory available for use than a system with maxmem=8096 memory=8096 due to the memory overhead of having to track the unused pages. +=item B<processor_trace_buffer_size=BYTES> + +Specifies the size of processor trace buffer that would be allocated +for each vCPU belonging to this domain. Disabled (i.e. +B<processor_trace_buffer_size=0> by default. This must be set to +non-zero value in order to be able to use processor tracing features +with this domain. + +B<NOTE>: The size value must be between 4 kB and 4 GB and it must +be also a power of 2. + =back =head3 Guest Virtual NUMA Configuration diff --git a/tools/golang/xenlight/helpers.gen.go b/tools/golang/xenlight/helpers.gen.go index 152c7e8e6b..bfc37b69c8 100644 --- a/tools/golang/xenlight/helpers.gen.go +++ b/tools/golang/xenlight/helpers.gen.go @@ -1117,6 +1117,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.VmtracePtOrder = int(xc.vmtrace_pt_order) return nil} @@ -1592,6 +1593,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_pt_order = C.int(x.VmtracePtOrder) return nil } diff --git a/tools/golang/xenlight/types.gen.go b/tools/golang/xenlight/types.gen.go index 663c1e86b4..f9b07ac862 100644 --- a/tools/golang/xenlight/types.gen.go +++ b/tools/golang/xenlight/types.gen.go @@ -516,6 +516,7 @@ GicVersion GicVersion Vuart VuartType } Altp2M Altp2MMode +VmtracePtOrder int } type domainBuildInfoTypeUnion interface { diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h index 1cd6c38e83..4abb521756 100644 --- a/tools/libxl/libxl.h +++ b/tools/libxl/libxl.h @@ -438,6 +438,14 @@ */ #define LIBXL_HAVE_CREATEINFO_PASSTHROUGH 1 +/* + * LIBXL_HAVE_VMTRACE_PT_ORDER indicates that + * libxl_domain_create_info has a vmtrace_pt_order parameter, which + * allows to enable pre-allocation of processor tracing buffers + * with the given order of size. + */ +#define LIBXL_HAVE_VMTRACE_PT_ORDER 1 + /* * libxl ABI compatibility * diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index 2814818e34..82b595161a 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -608,6 +608,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_pt_order = b_info->vmtrace_pt_order, }; if (info->type != LIBXL_DOMAIN_TYPE_PV) { diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index 9d3f05f399..1c5dd43e4d 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -645,6 +645,8 @@ libxl_domain_build_info = Struct("domain_build_info",[ # supported by x86 HVM and ARM support is planned. ("altp2m", libxl_altp2m_mode), + ("vmtrace_pt_order", 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 61b4ef7b7e..279f7c14d3 100644 --- a/tools/xl/xl_parse.c +++ b/tools/xl/xl_parse.c @@ -1861,6 +1861,28 @@ void parse_config_data(const char *config_source, } } + if (!xlu_cfg_get_long(config, "processor_trace_buffer_size", &l, 1) && l) { + int32_t shift = 0; + + if (l & (l - 1)) + { + fprintf(stderr, "ERROR: processor_trace_buffer_size " + "- must be a power of 2\n"); + exit(1); + } + + while (l >>= 1) ++shift; + + if (shift <= XEN_PAGE_SHIFT) + { + fprintf(stderr, "ERROR: processor_trace_buffer_size " + "- value is too small\n"); + exit(1); + } + + b_info->vmtrace_pt_order = shift - XEN_PAGE_SHIFT; + } + 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));