On Tue, Jun 30, 2020 at 02:33:51PM +0200, Michał Leszczyński wrote:
> From: Michal Leszczynski <michal.leszczyn...@cert.pl>
>
> Implement domctl to manage the runtime state of
> processor trace feature.
>
> Signed-off-by: Michal Leszczynski <michal.leszczyn...@cert.pl>
> ---
> xen/arch/x86/domctl.c | 48 +++++++++++++++++++++++++++++++++++++
> xen/include/public/domctl.h | 26 ++++++++++++++++++++
> 2 files changed, 74 insertions(+)
>
> diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
> index 6f2c69788d..a041b724d8 100644
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -322,6 +322,48 @@ void arch_get_domain_info(const struct domain *d,
> info->arch_config.emulation_flags = d->arch.emulation_flags;
> }
>
> +static int do_vmtrace_op(struct domain *d, struct xen_domctl_vmtrace_op *op,
> + XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
> +{
> + int rc;
> + struct vcpu *v;
> +
> + if ( !vmtrace_supported )
> + return -EOPNOTSUPP;
> +
> + if ( !is_hvm_domain(d) )
> + return -EOPNOTSUPP;
You can join both checks.
> +
> + if ( op->vcpu >= d->max_vcpus )
> + return -EINVAL;
> +
> + v = domain_vcpu(d, op->vcpu);
> + rc = 0;
No need to init rc to zero, after the switch below it will always be
initialized.
> +
> + switch ( op->cmd )
> + {
> + case XEN_DOMCTL_vmtrace_pt_enable:
> + case XEN_DOMCTL_vmtrace_pt_disable:
> + vcpu_pause(v);
> + spin_lock(&d->vmtrace_lock);
> +
> + rc = vmtrace_control_pt(v, op->cmd == XEN_DOMCTL_vmtrace_pt_enable);
> +
> + spin_unlock(&d->vmtrace_lock);
> + vcpu_unpause(v);
> + break;
> +
> + case XEN_DOMCTL_vmtrace_pt_get_offset:
> + rc = vmtrace_get_pt_offset(v, &op->offset);
Since you don't pause the vcpu here, I think you want to use atomic
operations to update v->arch.hvm.vmx.pt_state->output_mask.raw, or
else you could see inconsistent results if a vmexit is updating it in
parallel? (since you don't pause the target vcpu).
Thanks, Roger.
