| Message ID | 036bc768bfb074269d9bd4530304a11170b7142d.1594150543.git.michal.leszczynski@cert.pl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Implement support for external IPT monitoring | expand |
On Tue, Jul 07, 2020 at 09:39:43PM +0200, Michał Leszczyński wrote: > From: Michal Leszczynski <michal.leszczynski@cert.pl> > > Add vmtrace_pt_size domain parameter in live domain and > vmtrace_pt_order parameter in xen_domctl_createdomain. > > Signed-off-by: Michal Leszczynski <michal.leszczynski@cert.pl> > --- > xen/arch/x86/domain.c | 6 ++++++ > xen/common/domain.c | 9 +++++++++ > xen/include/public/domctl.h | 1 + > xen/include/xen/sched.h | 3 +++ > 4 files changed, 19 insertions(+) > > diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c > index fee6c3931a..b75017b28b 100644 > --- a/xen/arch/x86/domain.c > +++ b/xen/arch/x86/domain.c > @@ -499,6 +499,12 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config) > */ > config->flags |= XEN_DOMCTL_CDF_oos_off; > > + if ( !hvm && config->processor_trace_buf_kb ) > + { > + dprintk(XENLOG_INFO, "Processor trace is not supported on non-HVM\n"); > + return -EINVAL; > + } > + > return 0; > } > > diff --git a/xen/common/domain.c b/xen/common/domain.c > index a45cf023f7..e6e8f88da1 100644 > --- a/xen/common/domain.c > +++ b/xen/common/domain.c > @@ -338,6 +338,12 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config) > return -EINVAL; > } > > + if ( config->processor_trace_buf_kb && !vmtrace_supported ) > + { > + dprintk(XENLOG_INFO, "Processor tracing is not supported\n"); > + return -EINVAL; > + } > + > return arch_sanitise_domain_config(config); > } > > @@ -443,6 +449,9 @@ struct domain *domain_create(domid_t domid, > d->nr_pirqs = min(d->nr_pirqs, nr_irqs); > > radix_tree_init(&d->pirq_tree); > + > + if ( config->processor_trace_buf_kb ) > + d->processor_trace_buf_kb = config->processor_trace_buf_kb; > } > > if ( (err = arch_domain_create(d, config)) != 0 ) > diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h > index 59bdc28c89..7681675a94 100644 > --- a/xen/include/public/domctl.h > +++ b/xen/include/public/domctl.h > @@ -92,6 +92,7 @@ struct xen_domctl_createdomain { > uint32_t max_evtchn_port; > int32_t max_grant_frames; > int32_t max_maptrack_frames; > + uint32_t processor_trace_buf_kb; > > struct xen_arch_domainconfig arch; > }; > diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h > index ac53519d7f..c046e59886 100644 > --- a/xen/include/xen/sched.h > +++ b/xen/include/xen/sched.h > @@ -457,6 +457,9 @@ struct domain > unsigned pbuf_idx; > spinlock_t pbuf_lock; > > + /* Used by vmtrace features */ > + uint32_t processor_trace_buf_kb; Any reason for adding it here instead of doing it at the end of the struct? Also I think this should be unsigned int, there's no reason to use a specific width type here. IMO it would be nice to have a Kconfig option for this, so that it can be build time disabled, and for example disabled by default on Arm (or on x86 if HVM is not built). Most recently added features have followed this model for example Argo, where a Kconfig option is added in order to build time disable the added feature. Let me know if you need some tips about it, I'm happy to help in order to try to get this in Kconfig. Thanks, Roger.
On 07.07.2020 21:39, Michał Leszczyński wrote: > @@ -443,6 +449,9 @@ struct domain *domain_create(domid_t domid, > d->nr_pirqs = min(d->nr_pirqs, nr_irqs); > > radix_tree_init(&d->pirq_tree); > + > + if ( config->processor_trace_buf_kb ) > + d->processor_trace_buf_kb = config->processor_trace_buf_kb; There's no real need for the if, is there? > --- a/xen/include/public/domctl.h > +++ b/xen/include/public/domctl.h > @@ -92,6 +92,7 @@ struct xen_domctl_createdomain { > uint32_t max_evtchn_port; > int32_t max_grant_frames; > int32_t max_maptrack_frames; > + uint32_t processor_trace_buf_kb; Adding a new field to an existing interface struct requires bumping of the interface version, unless that has already happened during a release cycle. Here I agree with the choice of uint32_t, but ... > --- a/xen/include/xen/sched.h > +++ b/xen/include/xen/sched.h > @@ -457,6 +457,9 @@ struct domain > unsigned pbuf_idx; > spinlock_t pbuf_lock; > > + /* Used by vmtrace features */ > + uint32_t processor_trace_buf_kb; ... why a fixed size type here? unsigned int will do fine, won't it? Jan
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index fee6c3931a..b75017b28b 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -499,6 +499,12 @@ int arch_sanitise_domain_config(struct xen_domctl_createdomain *config) */ config->flags |= XEN_DOMCTL_CDF_oos_off; + if ( !hvm && config->processor_trace_buf_kb ) + { + dprintk(XENLOG_INFO, "Processor trace is not supported on non-HVM\n"); + return -EINVAL; + } + return 0; } diff --git a/xen/common/domain.c b/xen/common/domain.c index a45cf023f7..e6e8f88da1 100644 --- a/xen/common/domain.c +++ b/xen/common/domain.c @@ -338,6 +338,12 @@ static int sanitise_domain_config(struct xen_domctl_createdomain *config) return -EINVAL; } + if ( config->processor_trace_buf_kb && !vmtrace_supported ) + { + dprintk(XENLOG_INFO, "Processor tracing is not supported\n"); + return -EINVAL; + } + return arch_sanitise_domain_config(config); } @@ -443,6 +449,9 @@ struct domain *domain_create(domid_t domid, d->nr_pirqs = min(d->nr_pirqs, nr_irqs); radix_tree_init(&d->pirq_tree); + + if ( config->processor_trace_buf_kb ) + d->processor_trace_buf_kb = config->processor_trace_buf_kb; } if ( (err = arch_domain_create(d, config)) != 0 ) diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h index 59bdc28c89..7681675a94 100644 --- a/xen/include/public/domctl.h +++ b/xen/include/public/domctl.h @@ -92,6 +92,7 @@ struct xen_domctl_createdomain { uint32_t max_evtchn_port; int32_t max_grant_frames; int32_t max_maptrack_frames; + uint32_t processor_trace_buf_kb; struct xen_arch_domainconfig arch; }; diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index ac53519d7f..c046e59886 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -457,6 +457,9 @@ struct domain unsigned pbuf_idx; spinlock_t pbuf_lock; + /* Used by vmtrace features */ + uint32_t processor_trace_buf_kb; + /* OProfile support. */ struct xenoprof *xenoprof;