| Message ID | 20170706195218.98520-2-jmattson@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/07/2017 21:52, Jim Mattson wrote: > Allow VMWRITE in L1 to modify VM-exit information fields and report > this feature in L1's IA32_VMX_MISC MSR. > > Note that this feature is a prerequisite for kvm in L1 to use VMCS > shadowing, once that feature is available. > > Signed-off-by: Jim Mattson <jmattson@google.com> > --- > arch/x86/kvm/vmx.c | 105 ++++++++++++++++++++--------------------------------- > 1 file changed, 39 insertions(+), 66 deletions(-) > > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index b4cfdcfdc1c1..72f295510f76 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -7467,14 +7447,7 @@ static int handle_vmwrite(struct kvm_vcpu *vcpu) > } > } > > - > field = kvm_register_readl(vcpu, (((vmx_instruction_info) >> 28) & 0xf)); > - if (vmcs_field_readonly(field)) { > - nested_vmx_failValid(vcpu, > - VMXERR_VMWRITE_READ_ONLY_VMCS_COMPONENT); > - return kvm_skip_emulated_instruction(vcpu); > - } > - > if (vmcs12_write_any(vcpu, field, field_value) < 0) { > nested_vmx_failValid(vcpu, VMXERR_UNSUPPORTED_VMCS_COMPONENT); > return kvm_skip_emulated_instruction(vcpu); > vmcs_field_readonly is now unused. With that removed, Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
On 07/07/2017 10:22, Paolo Bonzini wrote: > > > On 06/07/2017 21:52, Jim Mattson wrote: >> Allow VMWRITE in L1 to modify VM-exit information fields and report >> this feature in L1's IA32_VMX_MISC MSR. >> >> Note that this feature is a prerequisite for kvm in L1 to use VMCS >> shadowing, once that feature is available. >> >> Signed-off-by: Jim Mattson <jmattson@google.com> >> --- >> arch/x86/kvm/vmx.c | 105 ++++++++++++++++++++--------------------------------- >> 1 file changed, 39 insertions(+), 66 deletions(-) >> >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >> index b4cfdcfdc1c1..72f295510f76 100644 >> --- a/arch/x86/kvm/vmx.c >> +++ b/arch/x86/kvm/vmx.c >> @@ -7467,14 +7447,7 @@ static int handle_vmwrite(struct kvm_vcpu *vcpu) >> } >> } >> >> - >> field = kvm_register_readl(vcpu, (((vmx_instruction_info) >> 28) & 0xf)); >> - if (vmcs_field_readonly(field)) { >> - nested_vmx_failValid(vcpu, >> - VMXERR_VMWRITE_READ_ONLY_VMCS_COMPONENT); >> - return kvm_skip_emulated_instruction(vcpu); >> - } >> - >> if (vmcs12_write_any(vcpu, field, field_value) < 0) { >> nested_vmx_failValid(vcpu, VMXERR_UNSUPPORTED_VMCS_COMPONENT); >> return kvm_skip_emulated_instruction(vcpu); >> > > vmcs_field_readonly is now unused. With that removed, > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> Actually, no. The error must be kept if the host has disabled the feature with a KVM_SET_MSR ioctl for MSR_IA32_VMX_MISC. The upside is that patch 4 is good as is. :) Also: >> >> + /* >> + * We can emulate "VMWRITE to any supported field," even if >> + * the hardware doesn't support it. >> + */ >> + vmx->nested.nested_vmx_misc_low |= >> + MSR_IA32_VMX_MISC_VMWRITE_SHADOW_RO_FIELDS; >> + There is already a "vmx->nested.nested_vmx_misc_low |= " statement a couple lines above. Please generalize the comment to something like "We can always emulate these features, even if the hardware doesn't support them". Paolo
On Fri, Jul 7, 2017 at 1:34 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > > On 07/07/2017 10:22, Paolo Bonzini wrote: > > > > > > On 06/07/2017 21:52, Jim Mattson wrote: > >> Allow VMWRITE in L1 to modify VM-exit information fields and report > >> this feature in L1's IA32_VMX_MISC MSR. > >> > >> Note that this feature is a prerequisite for kvm in L1 to use VMCS > >> shadowing, once that feature is available. > >> > >> Signed-off-by: Jim Mattson <jmattson@google.com> > >> --- > >> arch/x86/kvm/vmx.c | 105 ++++++++++++++++++++--------------------------------- > >> 1 file changed, 39 insertions(+), 66 deletions(-) > >> > >> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > >> index b4cfdcfdc1c1..72f295510f76 100644 > >> --- a/arch/x86/kvm/vmx.c > >> +++ b/arch/x86/kvm/vmx.c > >> @@ -7467,14 +7447,7 @@ static int handle_vmwrite(struct kvm_vcpu *vcpu) > >> } > >> } > >> > >> - > >> field = kvm_register_readl(vcpu, (((vmx_instruction_info) >> 28) & 0xf)); > >> - if (vmcs_field_readonly(field)) { > >> - nested_vmx_failValid(vcpu, > >> - VMXERR_VMWRITE_READ_ONLY_VMCS_COMPONENT); > >> - return kvm_skip_emulated_instruction(vcpu); > >> - } > >> - > >> if (vmcs12_write_any(vcpu, field, field_value) < 0) { > >> nested_vmx_failValid(vcpu, VMXERR_UNSUPPORTED_VMCS_COMPONENT); > >> return kvm_skip_emulated_instruction(vcpu); > >> > > > > vmcs_field_readonly is now unused. With that removed, > > > > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > > Actually, no. The error must be kept if the host has disabled the > feature with a KVM_SET_MSR ioctl for MSR_IA32_VMX_MISC. Supporting both settings of this feature bit is a bit more complicated. > The upside is that patch 4 is good as is. :) Not quite, but close. > Also: > > >> > >> + /* > >> + * We can emulate "VMWRITE to any supported field," even if > >> + * the hardware doesn't support it. > >> + */ > >> + vmx->nested.nested_vmx_misc_low |= > >> + MSR_IA32_VMX_MISC_VMWRITE_SHADOW_RO_FIELDS; > >> + > > There is already a "vmx->nested.nested_vmx_misc_low |= " statement a > couple lines above. Please generalize the comment to something like "We > can always emulate these features, even if the hardware doesn't support > them". > > Paolo
On 07/07/2017 17:44, Jim Mattson wrote: > On Fri, Jul 7, 2017 at 1:34 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: >> >> >> >> On 07/07/2017 10:22, Paolo Bonzini wrote: >>> >>> >>> On 06/07/2017 21:52, Jim Mattson wrote: >>>> Allow VMWRITE in L1 to modify VM-exit information fields and report >>>> this feature in L1's IA32_VMX_MISC MSR. >>>> >>>> Note that this feature is a prerequisite for kvm in L1 to use VMCS >>>> shadowing, once that feature is available. >>>> >>>> Signed-off-by: Jim Mattson <jmattson@google.com> >>>> --- >>>> arch/x86/kvm/vmx.c | 105 ++++++++++++++++++++--------------------------------- >>>> 1 file changed, 39 insertions(+), 66 deletions(-) >>>> >>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >>>> index b4cfdcfdc1c1..72f295510f76 100644 >>>> --- a/arch/x86/kvm/vmx.c >>>> +++ b/arch/x86/kvm/vmx.c >>>> @@ -7467,14 +7447,7 @@ static int handle_vmwrite(struct kvm_vcpu *vcpu) >>>> } >>>> } >>>> >>>> - >>>> field = kvm_register_readl(vcpu, (((vmx_instruction_info) >> 28) & 0xf)); >>>> - if (vmcs_field_readonly(field)) { >>>> - nested_vmx_failValid(vcpu, >>>> - VMXERR_VMWRITE_READ_ONLY_VMCS_COMPONENT); >>>> - return kvm_skip_emulated_instruction(vcpu); >>>> - } >>>> - >>>> if (vmcs12_write_any(vcpu, field, field_value) < 0) { >>>> nested_vmx_failValid(vcpu, VMXERR_UNSUPPORTED_VMCS_COMPONENT); >>>> return kvm_skip_emulated_instruction(vcpu); >>>> >>> >>> vmcs_field_readonly is now unused. With that removed, >>> >>> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> >> >> Actually, no. The error must be kept if the host has disabled the >> feature with a KVM_SET_MSR ioctl for MSR_IA32_VMX_MISC. > > Supporting both settings of this feature bit is a bit more complicated. Right, you have to set up the vmwrite bitmap correctly. But actually it should be a couple lines of code and your patch would be much simpler, because you keep the RW and RO field lists separate. After your changes here, vmx_vmread_bitmap and vmx_vmwrite_bitmap have always the same content. Instead, it should be possible for the read and write bitmap to point to the same address. KVM_SET_MSR can do: if (enable_shadow_vmcs) { if (/* L1 has vmwrite to all fields */) vmcs_write64(VMWRITE_BITMAP, __pa(vmx_vmread_bitmap)); else vmcs_write64(VMWRITE_BITMAP, __pa(vmx_vmwrite_bitmap)); } And in vmx_vcpu_setup, - vmcs_write64(VMWRITE_BITMAP, __pa(vmx_vmwrite_bitmap)); + vmcs_write64(VMWRITE_BITMAP, __pa(vmx_vmread_bitmap)); so that the default is to expose the feature. Paolo >> The upside is that patch 4 is good as is. :) > > Not quite, but close. > >> Also: >> >>>> >>>> + /* >>>> + * We can emulate "VMWRITE to any supported field," even if >>>> + * the hardware doesn't support it. >>>> + */ >>>> + vmx->nested.nested_vmx_misc_low |= >>>> + MSR_IA32_VMX_MISC_VMWRITE_SHADOW_RO_FIELDS; >>>> + >> >> There is already a "vmx->nested.nested_vmx_misc_low |= " statement a >> couple lines above. Please generalize the comment to something like "We >> can always emulate these features, even if the hardware doesn't support >> them". >> >> Paolo
On Fri, Jul 7, 2017 at 9:03 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > On 07/07/2017 17:44, Jim Mattson wrote: >> On Fri, Jul 7, 2017 at 1:34 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: >>> >>> >>> >>> On 07/07/2017 10:22, Paolo Bonzini wrote: >>>> >>>> >>>> On 06/07/2017 21:52, Jim Mattson wrote: >>>>> Allow VMWRITE in L1 to modify VM-exit information fields and report >>>>> this feature in L1's IA32_VMX_MISC MSR. >>>>> >>>>> Note that this feature is a prerequisite for kvm in L1 to use VMCS >>>>> shadowing, once that feature is available. >>>>> >>>>> Signed-off-by: Jim Mattson <jmattson@google.com> >>>>> --- >>>>> arch/x86/kvm/vmx.c | 105 ++++++++++++++++++++--------------------------------- >>>>> 1 file changed, 39 insertions(+), 66 deletions(-) >>>>> >>>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >>>>> index b4cfdcfdc1c1..72f295510f76 100644 >>>>> --- a/arch/x86/kvm/vmx.c >>>>> +++ b/arch/x86/kvm/vmx.c >>>>> @@ -7467,14 +7447,7 @@ static int handle_vmwrite(struct kvm_vcpu *vcpu) >>>>> } >>>>> } >>>>> >>>>> - >>>>> field = kvm_register_readl(vcpu, (((vmx_instruction_info) >> 28) & 0xf)); >>>>> - if (vmcs_field_readonly(field)) { >>>>> - nested_vmx_failValid(vcpu, >>>>> - VMXERR_VMWRITE_READ_ONLY_VMCS_COMPONENT); >>>>> - return kvm_skip_emulated_instruction(vcpu); >>>>> - } >>>>> - >>>>> if (vmcs12_write_any(vcpu, field, field_value) < 0) { >>>>> nested_vmx_failValid(vcpu, VMXERR_UNSUPPORTED_VMCS_COMPONENT); >>>>> return kvm_skip_emulated_instruction(vcpu); >>>>> >>>> >>>> vmcs_field_readonly is now unused. With that removed, >>>> >>>> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> >>> >>> Actually, no. The error must be kept if the host has disabled the >>> feature with a KVM_SET_MSR ioctl for MSR_IA32_VMX_MISC. >> >> Supporting both settings of this feature bit is a bit more complicated. > > Right, you have to set up the vmwrite bitmap correctly. But actually it should > be a couple lines of code and your patch would be much simpler, because you keep > the RW and RO field lists separate. > > After your changes here, vmx_vmread_bitmap and vmx_vmwrite_bitmap have > always the same content. Instead, it should be possible for the read > and write bitmap to point to the same address. KVM_SET_MSR can do: > > if (enable_shadow_vmcs) { > if (/* L1 has vmwrite to all fields */) > vmcs_write64(VMWRITE_BITMAP, __pa(vmx_vmread_bitmap)); > else > vmcs_write64(VMWRITE_BITMAP, __pa(vmx_vmwrite_bitmap)); > } > Actually, KVM_SET_MSR can only disable the feature. > And in vmx_vcpu_setup, > > - vmcs_write64(VMWRITE_BITMAP, __pa(vmx_vmwrite_bitmap)); > + vmcs_write64(VMWRITE_BITMAP, __pa(vmx_vmread_bitmap)); > > so that the default is to expose the feature. Yes, but copy_shadow_to_vmcs12() also needs to walk the "read-only" fields when they are read/write. Dealing with the two arrays of indices is awkward. Let me return to this later. I'll fix up patch 4 to deal with the code as it stands today. > > Paolo > >>> The upside is that patch 4 is good as is. :) >> >> Not quite, but close. >> >>> Also: >>> >>>>> >>>>> + /* >>>>> + * We can emulate "VMWRITE to any supported field," even if >>>>> + * the hardware doesn't support it. >>>>> + */ >>>>> + vmx->nested.nested_vmx_misc_low |= >>>>> + MSR_IA32_VMX_MISC_VMWRITE_SHADOW_RO_FIELDS; >>>>> + >>> >>> There is already a "vmx->nested.nested_vmx_misc_low |= " statement a >>> couple lines above. Please generalize the comment to something like "We >>> can always emulate these features, even if the hardware doesn't support >>> them". >>> >>> Paolo
> > After your changes here, vmx_vmread_bitmap and vmx_vmwrite_bitmap have > > always the same content. Instead, it should be possible for the read > > and write bitmap to point to the same address. > > Yes, but copy_shadow_to_vmcs12() also needs to walk the "read-only" fields when > they are read/write. Dealing with the two arrays of indices is awkward. It's just the code that's already in copy_vmcs12_to_shadow. If you first introduce vmcs_read/write_any, the two nested for loops aren't bad. Or, make the "for (i = 0; i < num_fields; i++)" loop its own function, like copy_vmcs12_fields_to_shadow and copy_shadow_fields_to_vmcs12, and call it twice. Paolo
On Fri, Jul 7, 2017 at 1:34 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > On 07/07/2017 10:22, Paolo Bonzini wrote: >> >> >> On 06/07/2017 21:52, Jim Mattson wrote: >>> Allow VMWRITE in L1 to modify VM-exit information fields and report >>> this feature in L1's IA32_VMX_MISC MSR. >>> >>> Note that this feature is a prerequisite for kvm in L1 to use VMCS >>> shadowing, once that feature is available. >>> >>> Signed-off-by: Jim Mattson <jmattson@google.com> >>> --- >>> arch/x86/kvm/vmx.c | 105 ++++++++++++++++++++--------------------------------- >>> 1 file changed, 39 insertions(+), 66 deletions(-) >>> >>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c >>> index b4cfdcfdc1c1..72f295510f76 100644 >>> --- a/arch/x86/kvm/vmx.c >>> +++ b/arch/x86/kvm/vmx.c >>> @@ -7467,14 +7447,7 @@ static int handle_vmwrite(struct kvm_vcpu *vcpu) >>> } >>> } >>> >>> - >>> field = kvm_register_readl(vcpu, (((vmx_instruction_info) >> 28) & 0xf)); >>> - if (vmcs_field_readonly(field)) { >>> - nested_vmx_failValid(vcpu, >>> - VMXERR_VMWRITE_READ_ONLY_VMCS_COMPONENT); >>> - return kvm_skip_emulated_instruction(vcpu); >>> - } >>> - >>> if (vmcs12_write_any(vcpu, field, field_value) < 0) { >>> nested_vmx_failValid(vcpu, VMXERR_UNSUPPORTED_VMCS_COMPONENT); >>> return kvm_skip_emulated_instruction(vcpu); >>> >> >> vmcs_field_readonly is now unused. With that removed, >> >> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> > > Actually, no. The error must be kept if the host has disabled the > feature with a KVM_SET_MSR ioctl for MSR_IA32_VMX_MISC. > Coming back to this patch set... Is there a good reason to allow userspace to clear MSR_IA32_VMX_MISC_VMWRITE_SHADOW_RO_FIELDS? It's not yet possible to migrate a VMX-capable VM (though I think that ability is imminent), so there are no compatibility issues. Why not just force this on?
On 02/05/2018 17:52, Jim Mattson wrote: >>> vmcs_field_readonly is now unused. With that removed, >>> >>> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> >> Actually, no. The error must be kept if the host has disabled the >> feature with a KVM_SET_MSR ioctl for MSR_IA32_VMX_MISC. >> > Coming back to this patch set... > > Is there a good reason to allow userspace to clear > MSR_IA32_VMX_MISC_VMWRITE_SHADOW_RO_FIELDS? It's not yet possible to > migrate a VMX-capable VM (though I think that ability is imminent), so > there are no compatibility issues. Why not just force this on? I suppose that can be done, yes. The only reason is that silicon ties that bit to the availability of shadow VMCS, but we can certainly do things differently than actual processors. Technically it would be a small userspace API break, but we can live with it. Paolo
On Wed, May 2, 2018 at 9:02 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: > On 02/05/2018 17:52, Jim Mattson wrote: >>>> vmcs_field_readonly is now unused. With that removed, >>>> >>>> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> >>> Actually, no. The error must be kept if the host has disabled the >>> feature with a KVM_SET_MSR ioctl for MSR_IA32_VMX_MISC. >>> >> Coming back to this patch set... >> >> Is there a good reason to allow userspace to clear >> MSR_IA32_VMX_MISC_VMWRITE_SHADOW_RO_FIELDS? It's not yet possible to >> migrate a VMX-capable VM (though I think that ability is imminent), so >> there are no compatibility issues. Why not just force this on? > > I suppose that can be done, yes. The only reason is that silicon ties > that bit to the availability of shadow VMCS, but we can certainly do > things differently than actual processors. > > Technically it would be a small userspace API break, but we can live > with it. Does qemu explicitly set IA32_VMX_MISC today?
On 02/05/2018 18:56, Jim Mattson wrote: > On Wed, May 2, 2018 at 9:02 AM, Paolo Bonzini <pbonzini@redhat.com> wrote: >> On 02/05/2018 17:52, Jim Mattson wrote: >>>>> vmcs_field_readonly is now unused. With that removed, >>>>> >>>>> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> >>>> Actually, no. The error must be kept if the host has disabled the >>>> feature with a KVM_SET_MSR ioctl for MSR_IA32_VMX_MISC. >>>> >>> Coming back to this patch set... >>> >>> Is there a good reason to allow userspace to clear >>> MSR_IA32_VMX_MISC_VMWRITE_SHADOW_RO_FIELDS? It's not yet possible to >>> migrate a VMX-capable VM (though I think that ability is imminent), so >>> there are no compatibility issues. Why not just force this on? >> >> I suppose that can be done, yes. The only reason is that silicon ties >> that bit to the availability of shadow VMCS, but we can certainly do >> things differently than actual processors. >> >> Technically it would be a small userspace API break, but we can live >> with it. > > Does qemu explicitly set IA32_VMX_MISC today? It doesn't use KVM_SET_MSR for VMX MSRs at all. "We" is KVM, not QEMU. Paolo
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index b4cfdcfdc1c1..72f295510f76 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -674,7 +674,7 @@ static struct pi_desc *vcpu_to_pi_desc(struct kvm_vcpu *vcpu) [number##_HIGH] = VMCS12_OFFSET(name)+4 -static unsigned long shadow_read_only_fields[] = { +static unsigned long shadow_fields[] = { /* * We do NOT shadow fields that are modified when L0 * traps and emulates any vmx instruction (e.g. VMPTRLD, @@ -695,12 +695,7 @@ static unsigned long shadow_read_only_fields[] = { VM_EXIT_INTR_ERROR_CODE, EXIT_QUALIFICATION, GUEST_LINEAR_ADDRESS, - GUEST_PHYSICAL_ADDRESS -}; -static int max_shadow_read_only_fields = - ARRAY_SIZE(shadow_read_only_fields); - -static unsigned long shadow_read_write_fields[] = { + GUEST_PHYSICAL_ADDRESS, TPR_THRESHOLD, GUEST_RIP, GUEST_RSP, @@ -728,10 +723,9 @@ static unsigned long shadow_read_write_fields[] = { HOST_FS_BASE, HOST_GS_BASE, HOST_FS_SELECTOR, - HOST_GS_SELECTOR + HOST_GS_SELECTOR, }; -static int max_shadow_read_write_fields = - ARRAY_SIZE(shadow_read_write_fields); +static int max_shadow_fields = ARRAY_SIZE(shadow_fields); static const unsigned short vmcs_field_to_offset_table[] = { FIELD(VIRTUAL_PROCESSOR_ID, virtual_processor_id), @@ -2803,6 +2797,13 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx) VMX_MISC_ACTIVITY_HLT; vmx->nested.nested_vmx_misc_high = 0; + /* + * We can emulate "VMWRITE to any supported field," even if + * the hardware doesn't support it. + */ + vmx->nested.nested_vmx_misc_low |= + MSR_IA32_VMX_MISC_VMWRITE_SHADOW_RO_FIELDS; + /* * This MSR reports some information about VMX support. We * should return information about the VMX we emulate for the @@ -3765,10 +3766,8 @@ static void init_vmcs_shadow_fields(void) { int i, j; - /* No checks for read only fields yet */ - - for (i = j = 0; i < max_shadow_read_write_fields; i++) { - switch (shadow_read_write_fields[i]) { + for (i = j = 0; i < max_shadow_fields; i++) { + switch (shadow_fields[i]) { case GUEST_BNDCFGS: if (!kvm_mpx_supported()) continue; @@ -3778,22 +3777,16 @@ static void init_vmcs_shadow_fields(void) } if (j < i) - shadow_read_write_fields[j] = - shadow_read_write_fields[i]; + shadow_fields[j] = shadow_fields[i]; j++; } - max_shadow_read_write_fields = j; + max_shadow_fields = j; /* shadowed fields guest access without vmexit */ - for (i = 0; i < max_shadow_read_write_fields; i++) { - clear_bit(shadow_read_write_fields[i], - vmx_vmwrite_bitmap); - clear_bit(shadow_read_write_fields[i], - vmx_vmread_bitmap); + for (i = 0; i < max_shadow_fields; i++) { + clear_bit(shadow_fields[i], vmx_vmwrite_bitmap); + clear_bit(shadow_fields[i], vmx_vmread_bitmap); } - for (i = 0; i < max_shadow_read_only_fields; i++) - clear_bit(shadow_read_only_fields[i], - vmx_vmread_bitmap); } static __init int alloc_kvm_area(void) @@ -7294,14 +7287,13 @@ static void copy_shadow_to_vmcs12(struct vcpu_vmx *vmx) unsigned long field; u64 field_value; struct vmcs *shadow_vmcs = vmx->vmcs01.shadow_vmcs; - const unsigned long *fields = shadow_read_write_fields; - const int num_fields = max_shadow_read_write_fields; + unsigned long *fields = shadow_fields; preempt_disable(); vmcs_load(shadow_vmcs); - for (i = 0; i < num_fields; i++) { + for (i = 0; i < max_shadow_fields; i++) { field = fields[i]; switch (vmcs_field_type(field)) { case VMCS_FIELD_TYPE_U16: @@ -7331,43 +7323,31 @@ static void copy_shadow_to_vmcs12(struct vcpu_vmx *vmx) static void copy_vmcs12_to_shadow(struct vcpu_vmx *vmx) { - const unsigned long *fields[] = { - shadow_read_write_fields, - shadow_read_only_fields - }; - const int max_fields[] = { - max_shadow_read_write_fields, - max_shadow_read_only_fields - }; - int i, q; + int i; unsigned long field; u64 field_value = 0; struct vmcs *shadow_vmcs = vmx->vmcs01.shadow_vmcs; + unsigned long *fields = shadow_fields; vmcs_load(shadow_vmcs); - for (q = 0; q < ARRAY_SIZE(fields); q++) { - for (i = 0; i < max_fields[q]; i++) { - field = fields[q][i]; - vmcs12_read_any(&vmx->vcpu, field, &field_value); - - switch (vmcs_field_type(field)) { - case VMCS_FIELD_TYPE_U16: - vmcs_write16(field, (u16)field_value); - break; - case VMCS_FIELD_TYPE_U32: - vmcs_write32(field, (u32)field_value); - break; - case VMCS_FIELD_TYPE_U64: - vmcs_write64(field, (u64)field_value); - break; - case VMCS_FIELD_TYPE_NATURAL_WIDTH: - vmcs_writel(field, (long)field_value); - break; - default: - WARN_ON(1); - break; - } + for (i = 0; i < max_shadow_fields; i++) { + field = fields[i]; + vmcs12_read_any(&vmx->vcpu, field, &field_value); + + switch (vmcs_field_type(field)) { + case VMCS_FIELD_TYPE_U16: + vmcs_write16(field, (u16)field_value); + break; + case VMCS_FIELD_TYPE_U32: + vmcs_write32(field, (u32)field_value); + break; + case VMCS_FIELD_TYPE_U64: + vmcs_write64(field, (u64)field_value); + break; + case VMCS_FIELD_TYPE_NATURAL_WIDTH: + vmcs_writel(field, (long)field_value); + break; } } @@ -7467,14 +7447,7 @@ static int handle_vmwrite(struct kvm_vcpu *vcpu) } } - field = kvm_register_readl(vcpu, (((vmx_instruction_info) >> 28) & 0xf)); - if (vmcs_field_readonly(field)) { - nested_vmx_failValid(vcpu, - VMXERR_VMWRITE_READ_ONLY_VMCS_COMPONENT); - return kvm_skip_emulated_instruction(vcpu); - } - if (vmcs12_write_any(vcpu, field, field_value) < 0) { nested_vmx_failValid(vcpu, VMXERR_UNSUPPORTED_VMCS_COMPONENT); return kvm_skip_emulated_instruction(vcpu);
Allow VMWRITE in L1 to modify VM-exit information fields and report this feature in L1's IA32_VMX_MISC MSR. Note that this feature is a prerequisite for kvm in L1 to use VMCS shadowing, once that feature is available. Signed-off-by: Jim Mattson <jmattson@google.com> --- arch/x86/kvm/vmx.c | 105 ++++++++++++++++++++--------------------------------- 1 file changed, 39 insertions(+), 66 deletions(-)