On 19/06/19 00:36, Liran Alon wrote: > > >> On 18 Jun 2019, at 19:24, Paolo Bonzini <pbonzini@xxxxxxxxxx> wrote: >> >> From: Liran Alon <liran.alon@xxxxxxxxxx> >> >> Improve the KVM_{GET,SET}_NESTED_STATE structs by detailing the format >> of VMX nested state data in a struct. >> >> In order to avoid changing the ioctl values of >> KVM_{GET,SET}_NESTED_STATE, there is a need to preserve >> sizeof(struct kvm_nested_state). This is done by defining the data >> struct as "data.vmx[0]". It was the most elegant way I found to >> preserve struct size while still keeping struct readable and easy to >> maintain. It does have a misfortunate side-effect that now it has to be >> accessed as "data.vmx[0]" rather than just "data.vmx". >> >> Because we are already modifying these structs, I also modified the >> following: >> * Define the "format" field values as macros. >> * Rename vmcs_pa to vmcs12_pa for better readability. >> >> Signed-off-by: Liran Alon <liran.alon@xxxxxxxxxx> >> [Remove SVM stubs, add KVM_STATE_NESTED_VMX_VMCS12_SIZE. - Paolo] > > 1) Why should we remove SVM stubs? I think it makes the interface intention more clear. > Do you see any disadvantage of having them? In its current state I think it would not require any state apart from the global flags, because MSRs can be extracted independent of KVM_GET_NESTED_STATE; this may change as things are cleaned up, but if that remains the case there would be no need for SVM structs at all. > 2) What is the advantage of defining a separate KVM_STATE_NESTED_VMX_VMCS12_SIZE > rather than just moving VMCS12_SIZE to userspace header? It's just for namespace cleanliness. I'm keeping VMCS12_SIZE for the arch/x86/kvm/vmx/ code because it's shorter and we're used to it, but userspace headers should use a more specific name. Paolo