| Message ID | 20170706195218.98520-5-jmattson@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/07/2017 21:52, Jim Mattson wrote: > Inconsistencies result from shadowing only accesses to the full > 64-bits of a 64-bit VMCS field, but not shadowing accesses to the high > 32-bits of the field. The "high" part of a 64-bit field should be > shadowed whenever the full 64-bit field is shadowed. > > Signed-off-by: Jim Mattson <jmattson@google.com> > --- > arch/x86/kvm/vmx.c | 70 +++++++++++++++++++++++++++++++++--------------------- > 1 file changed, 43 insertions(+), 27 deletions(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 2d0fe813fa34..c20d5f093af8 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -3762,6 +3762,43 @@ static void free_kvm_area(void) > } > } > > +enum vmcs_field_width { > + VMCS_FIELD_WIDTH_U16 = 0, > + VMCS_FIELD_WIDTH_U64 = 1, > + VMCS_FIELD_WIDTH_U32 = 2, > + VMCS_FIELD_WIDTH_NATURAL = 3, > + VMCS_FIELD_WIDTHS = 4 > +}; > + > +enum vmcs_field_type { > + VMCS_FIELD_TYPE_CONTROL = 0, > + VMCS_FIELD_TYPE_DATA = 1, > + VMCS_FIELD_TYPE_GUEST = 2, > + VMCS_FIELD_TYPE_HOST = 3, > + VMCS_FIELD_TYPES = 4 > +}; > + > +static inline short encoding(enum vmcs_field_width width, > + enum vmcs_field_type type, > + unsigned index, bool high) > +{ > + WARN_ON(index >= 1u << 9); > + index &= (1u << 9) - 1; > + return (width << 13) | (type << 10) | (index << 1) | (high ? 1 : 0); > +} > + > +static inline int vmcs_field_width(unsigned long field) > +{ > + if (0x1 & field) /* the *_HIGH fields are all 32 bit */ > + return VMCS_FIELD_WIDTH_U32; > + return (field >> 13) & 0x3; > +} > + > +static inline int vmcs_field_readonly(unsigned long field) > +{ > + return (((field >> 10) & 0x3) == 1); > +} See review to patch 1; with vmcs_field_readonly removed, Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Paolo > static void init_vmcs_shadow_fields(void) > { > int i, j; > @@ -3784,8 +3821,14 @@ static void init_vmcs_shadow_fields(void) > > /* shadowed fields guest access without vmexit */ > for (i = 0; i < max_shadow_fields; i++) { > + int width = vmcs_field_width(shadow_fields[i]); > + > clear_bit(shadow_fields[i], vmx_vmwrite_bitmap); > clear_bit(shadow_fields[i], vmx_vmread_bitmap); > + if (width == VMCS_FIELD_WIDTH_U64) { > + clear_bit(shadow_fields[i] + 1, vmx_vmwrite_bitmap); > + clear_bit(shadow_fields[i] + 1, vmx_vmread_bitmap); > + } > } > } > > @@ -7197,33 +7240,6 @@ static int handle_vmresume(struct kvm_vcpu *vcpu) > return nested_vmx_run(vcpu, false); > } > > -enum vmcs_field_width { > - VMCS_FIELD_WIDTH_U16 = 0, > - VMCS_FIELD_WIDTH_U64 = 1, > - VMCS_FIELD_WIDTH_U32 = 2, > - VMCS_FIELD_WIDTH_NATURAL = 3, > - VMCS_FIELD_WIDTHS = 4 > -}; > - > -enum vmcs_field_type { > - VMCS_FIELD_TYPE_CONTROL = 0, > - VMCS_FIELD_TYPE_DATA = 1, > - VMCS_FIELD_TYPE_GUEST = 2, > - VMCS_FIELD_TYPE_HOST = 3, > - VMCS_FIELD_TYPES = 4 > -}; > - > -static inline int vmcs_field_width(unsigned long field) > -{ > - if (0x1 & field) /* the *_HIGH fields are all 32 bit */ > - return VMCS_FIELD_WIDTH_U32; > - return (field >> 13) & 0x3; > -} > - > -static inline int vmcs_field_readonly(unsigned long field) > -{ > - return (((field >> 10) & 0x3) == 1); > -} > > /* > * Read a vmcs12 field. Since these can have varying lengths and we return >
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 2d0fe813fa34..c20d5f093af8 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -3762,6 +3762,43 @@ static void free_kvm_area(void) } } +enum vmcs_field_width { + VMCS_FIELD_WIDTH_U16 = 0, + VMCS_FIELD_WIDTH_U64 = 1, + VMCS_FIELD_WIDTH_U32 = 2, + VMCS_FIELD_WIDTH_NATURAL = 3, + VMCS_FIELD_WIDTHS = 4 +}; + +enum vmcs_field_type { + VMCS_FIELD_TYPE_CONTROL = 0, + VMCS_FIELD_TYPE_DATA = 1, + VMCS_FIELD_TYPE_GUEST = 2, + VMCS_FIELD_TYPE_HOST = 3, + VMCS_FIELD_TYPES = 4 +}; + +static inline short encoding(enum vmcs_field_width width, + enum vmcs_field_type type, + unsigned index, bool high) +{ + WARN_ON(index >= 1u << 9); + index &= (1u << 9) - 1; + return (width << 13) | (type << 10) | (index << 1) | (high ? 1 : 0); +} + +static inline int vmcs_field_width(unsigned long field) +{ + if (0x1 & field) /* the *_HIGH fields are all 32 bit */ + return VMCS_FIELD_WIDTH_U32; + return (field >> 13) & 0x3; +} + +static inline int vmcs_field_readonly(unsigned long field) +{ + return (((field >> 10) & 0x3) == 1); +} + static void init_vmcs_shadow_fields(void) { int i, j; @@ -3784,8 +3821,14 @@ static void init_vmcs_shadow_fields(void) /* shadowed fields guest access without vmexit */ for (i = 0; i < max_shadow_fields; i++) { + int width = vmcs_field_width(shadow_fields[i]); + clear_bit(shadow_fields[i], vmx_vmwrite_bitmap); clear_bit(shadow_fields[i], vmx_vmread_bitmap); + if (width == VMCS_FIELD_WIDTH_U64) { + clear_bit(shadow_fields[i] + 1, vmx_vmwrite_bitmap); + clear_bit(shadow_fields[i] + 1, vmx_vmread_bitmap); + } } } @@ -7197,33 +7240,6 @@ static int handle_vmresume(struct kvm_vcpu *vcpu) return nested_vmx_run(vcpu, false); } -enum vmcs_field_width { - VMCS_FIELD_WIDTH_U16 = 0, - VMCS_FIELD_WIDTH_U64 = 1, - VMCS_FIELD_WIDTH_U32 = 2, - VMCS_FIELD_WIDTH_NATURAL = 3, - VMCS_FIELD_WIDTHS = 4 -}; - -enum vmcs_field_type { - VMCS_FIELD_TYPE_CONTROL = 0, - VMCS_FIELD_TYPE_DATA = 1, - VMCS_FIELD_TYPE_GUEST = 2, - VMCS_FIELD_TYPE_HOST = 3, - VMCS_FIELD_TYPES = 4 -}; - -static inline int vmcs_field_width(unsigned long field) -{ - if (0x1 & field) /* the *_HIGH fields are all 32 bit */ - return VMCS_FIELD_WIDTH_U32; - return (field >> 13) & 0x3; -} - -static inline int vmcs_field_readonly(unsigned long field) -{ - return (((field >> 10) & 0x3) == 1); -} /* * Read a vmcs12 field. Since these can have varying lengths and we return
Inconsistencies result from shadowing only accesses to the full 64-bits of a 64-bit VMCS field, but not shadowing accesses to the high 32-bits of the field. The "high" part of a 64-bit field should be shadowed whenever the full 64-bit field is shadowed. Signed-off-by: Jim Mattson <jmattson@google.com> --- arch/x86/kvm/vmx.c | 70 +++++++++++++++++++++++++++++++++--------------------- 1 file changed, 43 insertions(+), 27 deletions(-)