| Message ID | 20240613190103.1054877-1-seanjc@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: nVMX: Update VMCS12_REVISION comment to state it should never change | expand |
On Thu, Jun 13, 2024 at 12:01 PM Sean Christopherson <seanjc@google.com> wrote: > > Rewrite the comment above VMCS12_REVISION to unequivocally state that the > ID must never change. KVM_{G,S}ET_NESTED_STATE have been officially > supported for some time now, i.e. changing VMCS12_REVISION would break > userspace. > > Opportunistically add a blurb to the CHECK_OFFSET() comment to make it > explicitly clear that new fields are allowed, i.e. that the restriction > on the layout is all about backwards compatibility. > > No functional change intended. > > Cc: Jim Mattson <jmattson@google.com> > Signed-off-by: Sean Christopherson <seanjc@google.com> Reviewed-by: Jim Mattson <jmattson@google.com>
On Thu, 13 Jun 2024 12:01:03 -0700, Sean Christopherson wrote: > Rewrite the comment above VMCS12_REVISION to unequivocally state that the > ID must never change. KVM_{G,S}ET_NESTED_STATE have been officially > supported for some time now, i.e. changing VMCS12_REVISION would break > userspace. > > Opportunistically add a blurb to the CHECK_OFFSET() comment to make it > explicitly clear that new fields are allowed, i.e. that the restriction > on the layout is all about backwards compatibility. > > [...] Applied to kvm-x86 vmx, thanks! [1/1] KVM: nVMX: Update VMCS12_REVISION comment to state it should never change https://github.com/kvm-x86/linux/commit/cb9fb5fc12ef -- https://github.com/kvm-x86/linux/tree/next
diff --git a/arch/x86/kvm/vmx/vmcs12.h b/arch/x86/kvm/vmx/vmcs12.h index 01936013428b..56fd150a6f24 100644 --- a/arch/x86/kvm/vmx/vmcs12.h +++ b/arch/x86/kvm/vmx/vmcs12.h @@ -188,12 +188,13 @@ struct __packed vmcs12 { }; /* - * VMCS12_REVISION is an arbitrary id that should be changed if the content or - * layout of struct vmcs12 is changed. MSR_IA32_VMX_BASIC returns this id, and - * VMPTRLD verifies that the VMCS region that L1 is loading contains this id. + * VMCS12_REVISION is KVM's arbitrary ID for the layout of struct vmcs12. KVM + * enumerates this value to L1 via MSR_IA32_VMX_BASIC, and checks the revision + * ID during nested VMPTRLD to verify that L1 is loading a VMCS that adhere's + * to KVM's virtual CPU definition. * - * IMPORTANT: Changing this value will break save/restore compatibility with - * older kvm releases. + * DO NOT change this value, as it will break save/restore compatibility with + * older KVM releases. */ #define VMCS12_REVISION 0x11e57ed0 @@ -206,7 +207,8 @@ struct __packed vmcs12 { #define VMCS12_SIZE KVM_STATE_NESTED_VMX_VMCS_SIZE /* - * For save/restore compatibility, the vmcs12 field offsets must not change. + * For save/restore compatibility, the vmcs12 field offsets must not change, + * although appending fields and/or filling gaps is obviously allowed. */ #define CHECK_OFFSET(field, loc) \ ASSERT_STRUCT_OFFSET(struct vmcs12, field, loc)
Rewrite the comment above VMCS12_REVISION to unequivocally state that the ID must never change. KVM_{G,S}ET_NESTED_STATE have been officially supported for some time now, i.e. changing VMCS12_REVISION would break userspace. Opportunistically add a blurb to the CHECK_OFFSET() comment to make it explicitly clear that new fields are allowed, i.e. that the restriction on the layout is all about backwards compatibility. No functional change intended. Cc: Jim Mattson <jmattson@google.com> Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/kvm/vmx/vmcs12.h | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) base-commit: e4e9e1067138e5620cf0500c3e5f6ebfb9d322c8