> On 15 Jun 2019, at 3:42, Paolo Bonzini <pbonz...@redhat.com> wrote: > > This patch improves the KVM_GET/SET_NESTED_STATE structs by detailing > the format of VMX nested state in a struct. The VMX nested state is > accessible through struct kvm_vmx_nested_state though, to avoid > changing the size of the structs, it has to be accessed as "vmx.data[0]" > rather than just "vmx.data". > > Also, the values of the "format" field are defined as macros. This > patch should be sent to Linus very shortly. > > Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> > --- > linux-headers/asm-x86/kvm.h | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/linux-headers/asm-x86/kvm.h b/linux-headers/asm-x86/kvm.h > index 7a0e64ccd6..06b8727a3b 100644 > --- a/linux-headers/asm-x86/kvm.h > +++ b/linux-headers/asm-x86/kvm.h > @@ -383,6 +383,9 @@ struct kvm_sync_regs { > #define KVM_X86_QUIRK_LAPIC_MMIO_HOLE (1 << 2) > #define KVM_X86_QUIRK_OUT_7E_INC_RIP (1 << 3) > > +#define KVM_STATE_NESTED_FORMAT_VMX 0 > +#define KVM_STATE_NESTED_FORMAT_SVM 1 > + > #define KVM_STATE_NESTED_GUEST_MODE 0x00000001 > #define KVM_STATE_NESTED_RUN_PENDING 0x00000002 > #define KVM_STATE_NESTED_EVMCS 0x00000004 > @@ -390,6 +393,11 @@ struct kvm_sync_regs { > #define KVM_STATE_NESTED_SMM_GUEST_MODE 0x00000001 > #define KVM_STATE_NESTED_SMM_VMXON 0x00000002 > > +struct kvm_vmx_nested_state_data { > + __u8 vmcs12[0x1000]; > + __u8 shadow_vmcs12[0x1000]; > +};
Do you think we should replace this 0x1000 with VMCS12_SIZE? > + > struct kvm_vmx_nested_state { > __u64 vmxon_pa; > __u64 vmcs_pa; > @@ -397,6 +405,9 @@ struct kvm_vmx_nested_state { > struct { > __u16 flags; > } smm; > + > + __u8 pad[120 - 18]; > + struct kvm_vmx_nested_state_data data[0]; > }; I don’t like this pad[] thing. It creates weird dependency between the padding in kvm_nested_state and this one. Also, it doesn’t separate nicely between header & data regions. What do you think on the following alternative patch? (Note that it should still preserve kvm_nested_state struct size) -struct kvm_vmx_nested_state { +struct kvm_vmx_nested_state_data { + __u8 vmcs12[0x1000]; + __u8 shadow_vmcs12[0x1000]; +}; + +struct kvm_vmx_nested_state_hdr { __u64 vmxon_pa; - __u64 vmcs_pa; + __u64 vmcs12_pa; struct { __u16 flags; } smm; }; +struct kvm_svm_nested_state_data { + /* TODO: Implement */ +}; + +struct kvm_svm_nested_state_hdr { + /* TODO: Implement */ +}; + /* for KVM_CAP_NESTED_STATE */ struct kvm_nested_state { - /* KVM_STATE_* flags */ __u16 flags; - - /* 0 for VMX, 1 for SVM. */ __u16 format; - - /* 128 for SVM, 128 + VMCS size for VMX. */ __u32 size; union { - /* VMXON, VMCS */ - struct kvm_vmx_nested_state vmx; + struct kvm_vmx_nested_state_hdr vmx; + struct kvm_svm_nested_state_hdr svm; /* Pad the header to 128 bytes. */ __u8 pad[120]; - }; + } hdr; - __u8 data[0]; + /* + * Define data region as 0 bytes to preserve backwards-compatability + * to old definition of kvm_nested_state in order to avoid changing + * KVM_{GET,PUT}_NESTED_STATE ioctl values. + */ + union { + struct kvm_vmx_nested_state_data vmx[0]; + struct kvm_svm_nested_state_data svm[0]; + } data; }; I think this is cleaner. -Liran