On Fri, Jul 7, 2017 at 1:34 AM, Paolo Bonzini <pbonzini@xxxxxxxxxx> 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@xxxxxxxxxx>
> >> ---
> >> 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@xxxxxxxxxx>
>
> 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
