Re: [PATCH v4 1/9] KVM: x86: Add helper to provide intercept linear addresses

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The newly added `intercept_linear_addr` field is calculated but never consumed by architecture-specific intercept handlers, rendering the logic dead code.
- [Medium] The calculated linear address in `get_intercept_linear_addr()` is not untagged, violating architectural specifications when LAM/UAI is active.
--

--- Patch [1]: [PATCH v4 1/9] KVM: x86: Add helper to provide intercept linear addresses ---
commit bde7560cfe988f666acaf2bb0b4fbe6285414375
Author: Tina Zhang <[email protected]>

KVM: x86: Add helper to provide intercept linear addresses

This commit adds a helper get_intercept_linear_addr() to compute the linear
address for intercepts like INVLPG, and passes this address through
x86_instruction_info so intercept handlers can use it directly.

> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 8071b372d233e..a0e57b64cadd4 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
[ ... ]
> @@ -410,6 +410,26 @@ static int em_salc(struct x86_emulate_ctxt *ctxt)
[ ... ]
> +static u64 get_intercept_linear_addr(struct x86_emulate_ctxt *ctxt,
> +				     enum x86_intercept intercept)
> +{
> +	u64 la;
> +
> +	if (intercept != x86_intercept_invlpg)
> +		return 0;
> +
> +	la = seg_base(ctxt, ctxt->src.addr.mem.seg) + ctxt->src.addr.mem.ea;
> +	return ctxt->mode == X86EMUL_MODE_PROT64 ? la : (u32)la;

[Severity: Medium]
Should this address be untagged before being returned in 64-bit mode?

According to the Intel SDM and AMD APM, VM exits reporting a linear address
like INVLPG must report the address after masking tag bits if Linear Address
Masking (LAM) or Upper Address Ignore (UAI) is active.

Would it be necessary to pass this through ctxt->ops->get_untagged_addr()
here to ensure the resulting address is architecturally compliant?

> +}
> +
>  static int emulator_check_intercept(struct x86_emulate_ctxt *ctxt,
>  				    enum x86_intercept intercept,
>  				    enum x86_intercept_stage stage)
> @@ -427,6 +447,7 @@ static int emulator_check_intercept(struct x86_emulate_ctxt *ctxt,
>  		.src_type   = ctxt->src.type,
>  		.dst_type   = ctxt->dst.type,
>  		.ad_bytes   = ctxt->ad_bytes,
> +		.intercept_linear_addr = get_intercept_linear_addr(ctxt, intercept),

[Severity: Medium]
Is this new field actually consumed by the architecture-specific handlers?

Looking at svm_check_intercept() and vmx_check_intercept(), it doesn't
appear that either handler reads the intercept_linear_addr field from
x86_instruction_info.

If this field remains unused, does the original issue with nested
hypervisors receiving an incorrect INVLPG address remain unfixed?

>  		.rip	    = ctxt->eip,
>  		.next_rip   = ctxt->_eip,
>  	};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1