On Mon, Nov 19, 2018 at 09:50:52AM -0800, Jim Mattson wrote:
> I vaguely recall that at some time in the distant past, field 440EH of
> the VMCS was referred to as the "VMX instruction-information field."
Very distant past :) Internal documentation has referred to the field
as VM_EXIT_INTSTRUCTION_INFO since at least 2002. Odds are this was a
SDM typo or a KVM quirk.
> However, this field now provides instruction-information for string
> PIO VM-exits, descriptor table VM-exits, RDRAND VM-exits, and RDSEED
> VM-exits. The SDM now refers to it as the "VM-exit
> instruction-information field." Since this field still is not yet
> exposed as part of a userspace API, let's rename it to match the SDM.
>
> Signed-off-by: Jim Mattson <jmattson@xxxxxxxxxx>
> Reviewed-by: Peter Shier <pshier@xxxxxxxxxx>
> ---
...
> @@ -9122,7 +9123,7 @@ static int handle_vmread(struct kvm_vcpu *vcpu)
> unsigned long field;
> u64 field_value;
> unsigned long exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
> - u32 vmx_instruction_info = vmcs_read32(VMX_INSTRUCTION_INFO);
> + u32 vm_exit_instruction_info = vmcs_read32(VM_EXIT_INSTRUCTION_INFO);
What about using "instr_info" when caching the field in a local variable?
It'd eliminate a bit of weird wrapping, and personally I always use
"instr info" when talking/thinking about the field. handle_vmptrst()
already uses the shorthand and I find that to be much more readable.
> gva_t gva = 0;
> struct vmcs12 *vmcs12;
>
> @@ -9145,7 +9146,8 @@ static int handle_vmread(struct kvm_vcpu *vcpu)
> }
>
> /* Decode instruction info and find the field to read */
> - field = kvm_register_readl(vcpu, (((vmx_instruction_info) >> 28) & 0xf));
> + field = kvm_register_readl(vcpu,
> + (((vm_exit_instruction_info) >> 28) & 0xf));
> /* Read the field, zero-extended to a u64 field_value */
> if (vmcs12_read_any(vmcs12, field, &field_value) < 0)
> return nested_vmx_failValid(vcpu,
> @@ -9156,12 +9158,13 @@ static int handle_vmread(struct kvm_vcpu *vcpu)
> * Note that the number of bits actually copied is 32 or 64 depending
> * on the guest's mode (32 or 64 bit), not on the given field's length.
> */
> - if (vmx_instruction_info & (1u << 10)) {
> - kvm_register_writel(vcpu, (((vmx_instruction_info) >> 3) & 0xf),
> - field_value);
> + if (vm_exit_instruction_info & (1u << 10)) {
> + kvm_register_writel(vcpu,
> + (((vm_exit_instruction_info) >> 3) & 0xf),
> + field_value);
> } else {
> if (get_vmx_mem_address(vcpu, exit_qualification,
> - vmx_instruction_info, true, &gva))
> + vm_exit_instruction_info, true, &gva))
> return 1;
> /* _system ok, nested_vmx_check_permission has verified cpl=0 */
> kvm_write_guest_virt_system(vcpu, gva, &field_value,
> @@ -9178,7 +9181,7 @@ static int handle_vmwrite(struct kvm_vcpu *vcpu)
> gva_t gva;
> struct vcpu_vmx *vmx = to_vmx(vcpu);
> unsigned long exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
> - u32 vmx_instruction_info = vmcs_read32(VMX_INSTRUCTION_INFO);
> + u32 vm_exit_instruction_info = vmcs_read32(VM_EXIT_INSTRUCTION_INFO);
>
> /* The value to write might be 32 or 64 bits, depending on L1's long
> * mode, and eventually we need to write that into a field of several
> @@ -9196,12 +9199,12 @@ static int handle_vmwrite(struct kvm_vcpu *vcpu)
> if (vmx->nested.current_vmptr == -1ull)
> return nested_vmx_failInvalid(vcpu);
>
> - if (vmx_instruction_info & (1u << 10))
> + if (vm_exit_instruction_info & (1u << 10))
> field_value = kvm_register_readl(vcpu,
> - (((vmx_instruction_info) >> 3) & 0xf));
> + (((vm_exit_instruction_info) >> 3) & 0xf));
> else {
> if (get_vmx_mem_address(vcpu, exit_qualification,
> - vmx_instruction_info, false, &gva))
> + vm_exit_instruction_info, false, &gva))
> return 1;
> if (kvm_read_guest_virt(vcpu, gva, &field_value,
> (is_64_bit_mode(vcpu) ? 8 : 4), &e)) {
> @@ -9211,7 +9214,8 @@ static int handle_vmwrite(struct kvm_vcpu *vcpu)
> }
>
>
> - field = kvm_register_readl(vcpu, (((vmx_instruction_info) >> 28) & 0xf));
> + field = kvm_register_readl(vcpu,
> + (((vm_exit_instruction_info) >> 28) & 0xf));
> /*
> * If the vCPU supports "VMWRITE to any supported field in the
> * VMCS," then the "read-only" fields are actually read/write.
> @@ -9398,7 +9402,7 @@ static int nested_vmx_handle_enlightened_vmptrld(struct kvm_vcpu *vcpu,
> static int handle_vmptrst(struct kvm_vcpu *vcpu)
> {
> unsigned long exit_qual = vmcs_readl(EXIT_QUALIFICATION);
> - u32 instr_info = vmcs_read32(VMX_INSTRUCTION_INFO);
> + u32 instr_info = vmcs_read32(VM_EXIT_INSTRUCTION_INFO);
> gpa_t current_vmptr = to_vmx(vcpu)->nested.current_vmptr;
> struct x86_exception e;
> gva_t gva;
