2018-05-01 15:40-0700, Jim Mattson:
> Enforce the invariant that existing VMCS12 field offsets must not
> change. Experience has shown that without strict enforcement, this
> invariant will not be maintained.
>
> Signed-off-by: Jim Mattson <jmattson@xxxxxxxxxx>
> ---
> arch/x86/kvm/vmx.c | 154 +++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 154 insertions(+)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index dde3cf978c8c..2a29bcfb00c7 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -413,6 +413,160 @@ struct __packed vmcs12 {
> u16 guest_pml_index;
> };
>
> +/*
> + * For save/restore compatibility, the vmcs12 field offsets must not change.
> + */
> +#define CHECK_OFFSET(field, loc) \
> + _Static_assert(offsetof(struct vmcs12, field) == (loc), \
_Static_assert is only supported since GCC 4.6 and I think the push to
raise the minimal requirements went nowhere, so I've changed this to
BUILD_BUG_ON.
This required wrapping the list in a pointless function that is called
from reachable code, please see the result in kvm/queue,
thanks.
---
> u16 guest_pml_index;
> FIELD(GUEST_PML_INDEX, guest_pml_index),
> CHECK_OFFSET(guest_pml_index, 996);
Hm, having the list three times makes me thing of adding a header file
in a format like
FIELD32(REVISION_ID, revision_id, 0)
...
PADDING(u64 padding64[3])
...
FIELD16(GUEST_PML_INDEX, guest_pml_index, 996)
and including it three times with different definitions of FIELD*, e.g.:
#define FIELD16(number, name, offset) \
u16 name;
#define FIELD16(number, name, offset) \
[ROL16(number, 6)] = VMCS12_OFFSET(name),
#define FIELD16(number, name, offset) \
BUILD_BUG_ON_MSG(VMCS12_OFFSET(name) != (offset), "Offset ...");
the advantage is that we make sure that there is always a check in
place, but it never looks great in C ... will post a RFC.
