[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v1 4/7] x86/vmx: add do_vmtrace_op
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 compilers, using stuff that's not explicit in the C standard needs to be avoided. For example using standard types like "int" or "long" is no good since it's really up to the compiler to decide how much memory those need. The C specification is great like that.. Same stands for padding. Different compilers can decide to align things differently, pack things or not pack things, so we have to manually add the padding to take that choice away from the compiler. As discussed many times on the list, using C struct as the base of the ABI was a bad design decision to start with, but we are now stuck with it. It would now make more sense to use something like JSON to pass information like this between the hypervisor and the toolstack which is what we opted to do in new hypervisors like Bareflank/Boxy. Tamas
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |