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 <pbonzini@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];
+};
+
 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];
 };
 
 /* for KVM_CAP_NESTED_STATE */
-- 
2.21.0

> On 15 Jun 2019, at 3:42, Paolo Bonzini <pbonzini@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 <pbonzini@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

On 16/06/19 10:29, Liran Alon wrote:
> 
> I think this is cleaner.
> 
> -Liran

Yes, it is.  I'll post it to kvm@vger.kernel.org.  Are you going to send
v2 of this series or shall I?

Paolo

> On 17 Jun 2019, at 20:32, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> On 16/06/19 10:29, Liran Alon wrote:
>> 
>> I think this is cleaner.
>> 
>> -Liran
> 
> Yes, it is.  I'll post it to kvm@vger.kernel.org.  Are you going to send
> v2 of this series or shall I?
> 
> Paolo

The KVM patch is already submitted and I think you saw it.
I’m just about to send today a new v2 for the QEMU patches. It has additional changes.

-Liran