Re: [PATCH v6 04/20] KVM: VMX: Refactor VMX instruction information access

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incomplete refactoring of VMX instruction information access leaves brittle hardcoded bit shifts.
--

--- Patch [4]: [PATCH v6 04/20] KVM: VMX: Refactor VMX instruction information access ---
commit b0d7d66df897ae19fd14659a411d846da0d89653
Author: Chang S. Bae <chang.seok.bae@intel.com>

KVM: VMX: Refactor VMX instruction information access

Introduce a helper that returns the instruction information as a 64-bit
value and adjust existing sites to prepare for a wider field.

> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index 0635e92471c89..92b530baa0ee9 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -5234,7 +5234,7 @@ static void nested_vmx_triple_fault(struct kvm_vcpu *vcpu)
>   * #UD, #GP, or #SS.
>   */
>  int get_vmx_mem_address(struct kvm_vcpu *vcpu, unsigned long exit_qualification,
> -			u32 vmx_instruction_info, bool wr, int len, gva_t *ret)
> +			u64 vmx_instruction_info, bool wr, int len, gva_t *ret)
>  {

[Severity: Medium]
This is a pre-existing issue, but while we are transitioning this function
to take a u64, should we also update the memory operand extraction logic
inside it?

It currently contains open-coded bit shifts that might not handle the wider
format correctly when extended register IDs are introduced:

    int  index_reg = (vmx_instruction_info >> 18) & 0xf;
    bool index_is_valid = !(vmx_instruction_info & (1u << 22));
    int  base_reg       = (vmx_instruction_info >> 23) & 0xf;

[ ... ]

> @@ -6428,7 +6429,7 @@ static bool nested_vmx_exit_handled_encls(struct kvm_vcpu *vcpu,
>  static bool nested_vmx_exit_handled_vmcs_access(struct kvm_vcpu *vcpu,
>  	struct vmcs12 *vmcs12, gpa_t bitmap)
>  {
> -	u32 vmx_instruction_info;
> +	u64 vmx_instruction_info;
>  	unsigned long field;
>  	u8 b;
>  
> @@ -6436,7 +6437,7 @@ static bool nested_vmx_exit_handled_vmcs_access(struct kvm_vcpu *vcpu,
>  		return true;
>  
>  	/* Decode instruction info and find the field to access */
> -	vmx_instruction_info = vmcs_read32(VMX_INSTRUCTION_INFO);
> +	vmx_instruction_info = vmx_get_instr_info();
>  	field = kvm_register_read(vcpu, (((vmx_instruction_info) >> 28) & 0xf));

[Severity: Medium]
Since the goal is to prepare existing sites for a wider field, should this
open-coded bit shift be converted to use the vmx_get_instr_info_reg2() helper?

If the 64-bit extension changes register bit offsets in the future, this
hardcoded shift could read incorrect bits.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729191656.598771-1-chang.seok.bae@intel.com?part=4