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

[Tamas K Lengyel <tamas.k.lengyel@xxxxxxxxx>]

On Thu, Jun 18, 2020 at 9:47 AM Tamas K Lengyel
<tamas.k.lengyel@xxxxxxxxx> wrote:
>
> On Thu, Jun 18, 2020 at 9:41 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
> >
> > On 18.06.2020 17:25, Michał Leszczyński wrote:
> > > ----- 16 cze 2020 o 19:23, Roger Pau Monné roger.pau@xxxxxxxxxx 
> > > napisał(a):
> > >> On Tue, Jun 16, 2020 at 05:22:06PM +0200, Michał Leszczyński wrote:
> > >>> --- a/xen/include/public/hvm/hvm_op.h
> > >>> +++ b/xen/include/public/hvm/hvm_op.h
> > >>> @@ -382,6 +382,33 @@ struct xen_hvm_altp2m_op {
> > >>>  typedef struct xen_hvm_altp2m_op xen_hvm_altp2m_op_t;
> > >>>  DEFINE_XEN_GUEST_HANDLE(xen_hvm_altp2m_op_t);
> > >>>
> > >>> +/* HVMOP_vmtrace: Perform VM tracing related operation */
> > >>> +#define HVMOP_vmtrace 26
> > >>> +
> > >>> +#define HVMOP_VMTRACE_INTERFACE_VERSION 0x00000001
> > >>> +
> > >>> +struct xen_hvm_vmtrace_op {
> > >>> +    /* IN variable */
> > >>> +    uint32_t version;   /* HVMOP_VMTRACE_INTERFACE_VERSION */
> > >>> +    uint32_t cmd;
> > >>> +/* Enable/disable external vmtrace for given domain */
> > >>> +#define HVMOP_vmtrace_ipt_enable      1
> > >>> +#define HVMOP_vmtrace_ipt_disable     2
> > >>> +#define HVMOP_vmtrace_ipt_get_buf     3
> > >>> +#define HVMOP_vmtrace_ipt_get_offset  4
> > >>> +    domid_t domain;
> > >>
> > >> You are missing a padding field here AFAICT.
> > >
> > >
> > > Could you point out what is the purpose of this padding field and what 
> > > should be the size in this case? It's pretty unclear to me.
> >
> > In the public interface we aim at making all padding explicit.
>
> Just to expand a bit on this: this is an ABI meaning the hypervisor
> and the tool sending this structure communicate via memory only. Since
> the hypervisor and the compiler can be compiled using different

   ^ meant to write "hypervisor and the toolstack" above

Tamas