Re: [PATCH v10 18/28] KVM: VMX: Dump FRED context in dump_vmcs()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The FRED context dump in dump_vmcs() omits the FRED Shadow Stack Pointer (SSP) VMCS fields for both guest and host states.
--

commit 823b37b6644ed01e20ab9d85f3f985b6c7d1c23f
Author: Xin Li (Intel) <xin@zytor.com>

KVM: VMX: Dump FRED context in dump_vmcs()

This commit adds FRED related VMCS fields to dump_vmcs() to dump FRED
context for both guest and host states.

> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index cd041381b322c..3a3315a7fc112 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
[ ... ]
> @@ -6784,6 +6789,16 @@ void dump_vmcs(struct kvm_vcpu *vcpu)
>  	vmx_dump_sel("LDTR:", GUEST_LDTR_SELECTOR);
>  	vmx_dump_dtsel("IDTR:", GUEST_IDTR_LIMIT);
>  	vmx_dump_sel("TR:  ", GUEST_TR_SELECTOR);
> +	if (vmentry_ctl & VM_ENTRY_LOAD_IA32_FRED)
> +		pr_err("FRED guest: config=0x%016llx, stack_levels=0x%016llx\n"
> +		       "RSP0=0x%016llx, RSP1=0x%016llx\n"
> +		       "RSP2=0x%016llx, RSP3=0x%016llx\n",
> +		       vmcs_read64(GUEST_IA32_FRED_CONFIG),
> +		       vmcs_read64(GUEST_IA32_FRED_STKLVLS),
> +		       vmx_read_guest_fred_rsp0(vmx),
> +		       vmcs_read64(GUEST_IA32_FRED_RSP1),
> +		       vmcs_read64(GUEST_IA32_FRED_RSP2),
> +		       vmcs_read64(GUEST_IA32_FRED_RSP3));

[Severity: Low]
Does this guest context dump in dump_vmcs() miss the FRED Shadow Stack
Pointer fields?

It looks like GUEST_IA32_FRED_SSP1, GUEST_IA32_FRED_SSP2, and
GUEST_IA32_FRED_SSP3 are omitted from the diagnostic output. Since this is
dumping the FRED state when kvm_intel.dump_invalid_vmcs=1 is set, should
the SSP fields be included to ensure the diagnostic information is complete?

[ ... ]
> @@ -6835,6 +6850,16 @@ void dump_vmcs(struct kvm_vcpu *vcpu)
>  	       vmcs_readl(HOST_TR_BASE));
>  	pr_err("GDTBase=%016lx IDTBase=%016lx\n",
>  	       vmcs_readl(HOST_GDTR_BASE), vmcs_readl(HOST_IDTR_BASE));
> +	if (secondary_vmexit_ctl & SECONDARY_VM_EXIT_LOAD_IA32_FRED)
> +		pr_err("FRED host: config=0x%016llx, stack_levels=0x%016llx\n"
> +		       "RSP0=0x%016lx, RSP1=0x%016llx\n"
> +		       "RSP2=0x%016llx, RSP3=0x%016llx\n",
> +		       vmcs_read64(HOST_IA32_FRED_CONFIG),
> +		       vmcs_read64(HOST_IA32_FRED_STKLVLS),
> +		       (unsigned long)task_stack_page(current) + THREAD_SIZE,
> +		       vmcs_read64(HOST_IA32_FRED_RSP1),
> +		       vmcs_read64(HOST_IA32_FRED_RSP2),
> +		       vmcs_read64(HOST_IA32_FRED_RSP3));

[Severity: Low]
Similarly, does the host context dump also miss the equivalent
HOST_IA32_FRED_SSP1, HOST_IA32_FRED_SSP2, and HOST_IA32_FRED_SSP3 fields?

While this omission doesn't affect runtime correctness, including them
alongside the RSP fields might be helpful for full diagnostic visibility.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911213659.2025974-1-sohil.mehta@intel.com?part=18