Re: [PATCH v1 4/7] x86/vmx: add do_vmtrace_op

From: "Roger Pau Monné" <roger.pau@citrix.com>
To: "Michał Leszczyński" <michal.leszczynski@cert.pl>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Julien Grall <julien@xen.org>, Wei Liu <wl@xen.org>,
	Tamas K Lengyel <tamas.k.lengyel@gmail.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Jan Beulich <jbeulich@suse.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Xen-devel <xen-devel@lists.xenproject.org>
Subject: Re: [PATCH v1 4/7] x86/vmx: add do_vmtrace_op
Date: Thu, 18 Jun 2020 13:55:19 +0200	[thread overview]
Message-ID: <20200618115519.GL735@Air-de-Roger> (raw)
In-Reply-To: <832789003.9534743.1592478099057.JavaMail.zimbra@cert.pl>

On Thu, Jun 18, 2020 at 01:01:39PM +0200, Michał Leszczyński wrote:
> ----- 18 cze 2020 o 5:20, Tamas K Lengyel tamas.k.lengyel@gmail.com napisał(a):
> 
> >> >> +
> >> >> +        a.mfn = v->arch.hvm.vmx.ipt_state->output_base >> PAGE_SHIFT;
> >> >
> >> > This will not work for translated domains, ie: a PVH or HVM domain
> >> > won't be able to use this interface since it has no way to request the
> >> > mapping of a specific mfn into it's physmap. I think we need to take
> >> > this into account when deciding how the interface should be, so that
> >> > we don't corner ourselves with a PV only interface.
> >>
> >> Please be aware that this is only going to be used by Dom0. Is is well-supported
> >> case that somebody is using PVH/HVM Dom0?
> >>
> >> I think that all Virtual Machine Introspection stuff currently requires to have
> >> Dom0 PV. Our main goal is to have this working well in combo with VMI.
> > 
> > FYI the VMI interface doesn't require a PV domain. It works fine from
> > PVH dom0 or even from a secondary privileged HVM DomU as well,
> > provided you have the right XSM policy to allow that.
> > 
> > Tamas
> 
> 
> It was previously stated that:
> 
> > PVH or HVM domain
> > won't be able to use this interface since it has no way to request the
> > mapping of a specific mfn into it's physmap.
> 
> but however, taking LibVMI as an example:
> 
> https://github.com/libvmi/libvmi/blob/c461e20ae88bc62c08c27f50fcead23c27a30f9e/libvmi/driver/xen/xen.c#L51
> 
> An essential abstraction xen_get_memory() relies on xc_map_foreign_range(). Doesn't this mean that it's not usable from PVH or HVM domains, or did I got it all wrong?

That was my fault, so the buffer mfns are assigned to Xen, and then
the Xen domain ID is used to map those, which should work on both PV
and HVM (or PVH).

I still think using XENMEM_acquire_resource might be better, but I
would let others comment.

Roger.