On Fri, Apr 27, 2018 at 10:01:10AM -0700, Jim Mattson wrote:
> Changing the VMCS12 layout breaks save/restore compatibility with
> older kvm releases (and should induce a change of the VMCS12 revision
> identifier).
>
> The new VMCS12 fields added in 2017 should be relocated so that the
> offsets of previously existing fields do not change.
>
> Fixes: c5f983f6e8455 ("nVMX: Implement emulated Page Modification Logging")
> Fixes: 27c42a1bb8674 ("KVM: nVMX: Enable VMFUNC for the L1 hypervisor")
> Fixes: 41ab93727467c ("KVM: nVMX: Emulate EPTP switching for the L1 hypervisor")
> Signed-off-by: Jim Mattson <jmattson@xxxxxxxxxx>
So how does this work with vendors who have piggy backed on those patches
and are already releasing this? Should this go to stable@xxxxxxxxxxxxxxx ?
> ---
> arch/x86/kvm/vmx.c | 27 +++++++++++++++++----------
> 1 file changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index cd273f7926ef..41f9f932de54 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -242,7 +242,11 @@ struct shared_msr_entry {
> * underlying hardware which will be used to run L2.
> * This structure is packed to ensure that its layout is identical across
> * machines (necessary for live migration).
> - * If there are changes in this struct, VMCS12_REVISION must be changed.
> + *
> + * IMPORTANT: Changing the layout of existing fields in this structure
> + * will break save/restore compatibility with older kvm releases. When
> + * adding new fields, either use space in the reserved padding* arrays
> + * or add the new fields to the end of the structure.
> */
> typedef u64 natural_width;
> struct __packed vmcs12 {
> @@ -265,17 +269,14 @@ struct __packed vmcs12 {
> u64 virtual_apic_page_addr;
> u64 apic_access_addr;
> u64 posted_intr_desc_addr;
> - u64 vm_function_control;
> u64 ept_pointer;
> u64 eoi_exit_bitmap0;
> u64 eoi_exit_bitmap1;
> u64 eoi_exit_bitmap2;
> u64 eoi_exit_bitmap3;
> - u64 eptp_list_address;
> u64 xss_exit_bitmap;
> u64 guest_physical_address;
> u64 vmcs_link_pointer;
> - u64 pml_address;
> u64 guest_ia32_debugctl;
> u64 guest_ia32_pat;
> u64 guest_ia32_efer;
> @@ -288,7 +289,10 @@ struct __packed vmcs12 {
> u64 host_ia32_pat;
> u64 host_ia32_efer;
> u64 host_ia32_perf_global_ctrl;
> - u64 padding64[8]; /* room for future expansion */
> + u64 vm_function_control;
> + u64 eptp_list_address;
> + u64 pml_address;
> + u64 padding64[5]; /* room for future expansion */
> /*
> * To allow migration of L1 (complete with its L2 guests) between
> * machines of different natural widths (32 or 64 bit), we cannot have
> @@ -397,7 +401,6 @@ struct __packed vmcs12 {
> u16 guest_ldtr_selector;
> u16 guest_tr_selector;
> u16 guest_intr_status;
> - u16 guest_pml_index;
> u16 host_es_selector;
> u16 host_cs_selector;
> u16 host_ss_selector;
> @@ -405,12 +408,16 @@ struct __packed vmcs12 {
> u16 host_fs_selector;
> u16 host_gs_selector;
> u16 host_tr_selector;
> + u16 guest_pml_index;
> };
>
> /*
> * 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.
> + *
> + * IMPORTANT: Changing this value will break save/restore compatibility with
> + * older kvm releases.
> */
> #define VMCS12_REVISION 0x11e57ed0
>
> @@ -747,7 +754,6 @@ static const unsigned short vmcs_field_to_offset_table[] = {
> FIELD(GUEST_LDTR_SELECTOR, guest_ldtr_selector),
> FIELD(GUEST_TR_SELECTOR, guest_tr_selector),
> FIELD(GUEST_INTR_STATUS, guest_intr_status),
> - FIELD(GUEST_PML_INDEX, guest_pml_index),
> FIELD(HOST_ES_SELECTOR, host_es_selector),
> FIELD(HOST_CS_SELECTOR, host_cs_selector),
> FIELD(HOST_SS_SELECTOR, host_ss_selector),
> @@ -755,6 +761,7 @@ static const unsigned short vmcs_field_to_offset_table[] = {
> FIELD(HOST_FS_SELECTOR, host_fs_selector),
> FIELD(HOST_GS_SELECTOR, host_gs_selector),
> FIELD(HOST_TR_SELECTOR, host_tr_selector),
> + FIELD(GUEST_PML_INDEX, guest_pml_index),
> FIELD64(IO_BITMAP_A, io_bitmap_a),
> FIELD64(IO_BITMAP_B, io_bitmap_b),
> FIELD64(MSR_BITMAP, msr_bitmap),
> @@ -765,17 +772,14 @@ static const unsigned short vmcs_field_to_offset_table[] = {
> FIELD64(VIRTUAL_APIC_PAGE_ADDR, virtual_apic_page_addr),
> FIELD64(APIC_ACCESS_ADDR, apic_access_addr),
> FIELD64(POSTED_INTR_DESC_ADDR, posted_intr_desc_addr),
> - FIELD64(VM_FUNCTION_CONTROL, vm_function_control),
> FIELD64(EPT_POINTER, ept_pointer),
> FIELD64(EOI_EXIT_BITMAP0, eoi_exit_bitmap0),
> FIELD64(EOI_EXIT_BITMAP1, eoi_exit_bitmap1),
> FIELD64(EOI_EXIT_BITMAP2, eoi_exit_bitmap2),
> FIELD64(EOI_EXIT_BITMAP3, eoi_exit_bitmap3),
> - FIELD64(EPTP_LIST_ADDRESS, eptp_list_address),
> FIELD64(XSS_EXIT_BITMAP, xss_exit_bitmap),
> FIELD64(GUEST_PHYSICAL_ADDRESS, guest_physical_address),
> FIELD64(VMCS_LINK_POINTER, vmcs_link_pointer),
> - FIELD64(PML_ADDRESS, pml_address),
> FIELD64(GUEST_IA32_DEBUGCTL, guest_ia32_debugctl),
> FIELD64(GUEST_IA32_PAT, guest_ia32_pat),
> FIELD64(GUEST_IA32_EFER, guest_ia32_efer),
> @@ -788,6 +792,9 @@ static const unsigned short vmcs_field_to_offset_table[] = {
> FIELD64(HOST_IA32_PAT, host_ia32_pat),
> FIELD64(HOST_IA32_EFER, host_ia32_efer),
> FIELD64(HOST_IA32_PERF_GLOBAL_CTRL, host_ia32_perf_global_ctrl),
> + FIELD64(VM_FUNCTION_CONTROL, vm_function_control),
> + FIELD64(EPTP_LIST_ADDRESS, eptp_list_address),
> + FIELD64(PML_ADDRESS, pml_address),
> FIELD(PIN_BASED_VM_EXEC_CONTROL, pin_based_vm_exec_control),
> FIELD(CPU_BASED_VM_EXEC_CONTROL, cpu_based_vm_exec_control),
> FIELD(EXCEPTION_BITMAP, exception_bitmap),
> --
> 2.17.0.441.gb46fe60e1d-goog
>
