Received-SPF: pass (google.com: domain of linux-kernel+bounces-163619-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Tue, 30 Apr 2024 17:09:22 +0800
From: Chao Gao <chao.gao@intel.com>
To: Xin Li <xin3.li@intel.com>
CC: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>, "linux-doc@vger.kernel.org"
	<linux-doc@vger.kernel.org>, "linux-kselftest@vger.kernel.org"
	<linux-kselftest@vger.kernel.org>, "seanjc@google.com" <seanjc@google.com>,
	"pbonzini@redhat.com" <pbonzini@redhat.com>, "corbet@lwn.net"
	<corbet@lwn.net>, "tglx@linutronix.de" <tglx@linutronix.de>,
	"mingo@redhat.com" <mingo@redhat.com>, "bp@alien8.de" <bp@alien8.de>,
	"dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>, "x86@kernel.org"
	<x86@kernel.org>, "hpa@zytor.com" <hpa@zytor.com>, "shuah@kernel.org"
	<shuah@kernel.org>, "vkuznets@redhat.com" <vkuznets@redhat.com>,
	"peterz@infradead.org" <peterz@infradead.org>, "Shankar, Ravi V"
	<ravi.v.shankar@intel.com>, "xin@zytor.com" <xin@zytor.com>
Subject: Re: [PATCH v2 15/25] KVM: VMX: Dump FRED context in dump_vmcs()
Message-ID: <ZjC1QpnyQA0gHoo4@chao-email>
References: <20240207172646.3981-1-xin3.li@intel.com>
 <20240207172646.3981-16-xin3.li@intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20240207172646.3981-16-xin3.li@intel.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?HVoh91nMo8zKVH/NpxxqREAERcHupfoxusCwuFLbeOmBum6/8BYU5NRtaDyA?=
 =?us-ascii?Q?KSk7zfWr7WMvzoHDI5/djxBrXQFOUyYIzafsqiv3gWfWxpTOYX/AGY8NMVSZ?=
 =?us-ascii?Q?BKQXox+h1Bvt480l4Q+PBmWA9Q3EBt0I02hX+iRutXubAHXoLHIUY+SoPLDV?=
 =?us-ascii?Q?5PaSUEyY3fCFdmVQR0tq0ppFYwTdB276kMfsFCqZ/csaIuxfw+W0zQdSgrAG?=
 =?us-ascii?Q?Z41bGpPPW/hLvSzSV4WlG2b9EWmVjimV1WMF1m7CN7w6gQ1du+1O1hOWkuiu?=
 =?us-ascii?Q?W9t4puGjK2vVMiQnOjxO0sm6TELN85n9itXe9DBzbR3zAXMqPNq1WuUxFWe5?=
 =?us-ascii?Q?8DBWnONsygwm2O7YTPy9wvmU4HO3Q4pvBMrjDan0QDl9+WyhNbG8MYmvNEl0?=
 =?us-ascii?Q?6bwwGDDegF3MMMou830U2VLShGPpQBLNh0soFkalrGLZo08RLUxKhvOCAqzs?=
 =?us-ascii?Q?CvAlbvDbOkmchMhmZw5XYtdEHqUMaSquQIZmJ1aP2LbYqzNFzftY9UYUlu4n?=
 =?us-ascii?Q?SQ5D/O4h0H74v4EnAN1SLdzHSAsmeuoTNG5s2Xa7OeiaSpP//xeDQyWetPEm?=
 =?us-ascii?Q?ECkz1zDl6DRMrejkwBq/SA/9+HR5eHcLAcKkcThDm8T6xHmVGrRDX4Mws+8L?=
 =?us-ascii?Q?SzwVMk5OHjxkcexaqNsoankGxYO9K9ElKV5t7R7hcguE2BlQctYTyGQ68w2r?=
 =?us-ascii?Q?YBzjOByjDJ/WJ927dr60d5a9jlAMggfl74ZRH2EPI4ih7kQnKgqsYLowbnG6?=
 =?us-ascii?Q?ocit57nw16aMRyu7aGaM9VMIURptw23pAWGE/bFcfGR//3no9c3boB6izbVu?=
 =?us-ascii?Q?GJXL+zoui9l4gnPaLaxBu5kc2P+hQAJ4roAssZyUaHgg3riM9dN/wbO/AXdG?=
 =?us-ascii?Q?JhouEORfWvP7UlFrnUX57bir2DRxlbx5dS+kFmllrIcIb1aEVimhTLzyTYo3?=
 =?us-ascii?Q?kQ+qRwQ5uheBNn2c6t6CdAWkPX5N2f1MTWnTrnLhYZjX0XBx1d8bK5LCypWP?=
 =?us-ascii?Q?MFyICuDZ+Zgi+NMha7eGBYEnU7UmpImD4NUfxk/8zzqUU4XprJlnP0oDOdn2?=
 =?us-ascii?Q?nAV7FDo0KO1oNYrwUWA9246Lgygx3//0paIORl7TAozREckS45X/Jt+y294E?=
 =?us-ascii?Q?t12JJmv/xE9NtXF95a3O2Gc8fUIAYHHiIdtJCMJoHRJTnKKPrCJensUqfpW9?=
 =?us-ascii?Q?2B+2U6Tdq4zjpscgK45WAT3vwUvYCxrnoLUwlzJ2o3HNyM37841HJm79gca9?=
 =?us-ascii?Q?e21oAVPksDfQn6OEI/wtdkwMpbGHDmNU9NQMLemvShz54KvwcnI5nQ3NjZcH?=
 =?us-ascii?Q?QPiUyIxSo63qFnU6X/TCzAbDJbXxL6Gtj1/8jkMHvTqCIyY+H1/W+Cm4tKMv?=
 =?us-ascii?Q?KYMiPUmEBfVgqmCoiNBcG8vGSJWrL72EZ5vKB4GIdYal8ghHqm4vdM8z8u7h?=
 =?us-ascii?Q?8XWPkJPX68u9lmL74hnuXCxX3yak2JuOAeiajFhtt6RjElsRLjka+Wdqz2tF?=
 =?us-ascii?Q?HbDSS2ebIwjalRN4Jp37yqvlzJRQxS+TIYcM45hE/UXViSWYeW6VwKavCGjk?=
 =?us-ascii?Q?J+LvzFcgcwHv5I0+KnfqlG7sDmOieBHbeMQFamzm?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 8f2a98ab-d61a-4de3-b22e-08dc68f53f62
X-MS-Exchange-CrossTenant-AuthSource: CH3PR11MB8660.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Apr 2024 09:09:32.2233
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: s+efYIXUM4Blb4tjkKUJwuNC/QFbPU+sd0QxSwmnn5WXLBDBREVZuAxipFEcLAFnJrMlOzXBa8tifobZ9QRWBw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB5927
X-OriginatorOrg: intel.com

On Thu, Feb 08, 2024 at 01:26:35AM +0800, Xin Li wrote:
>Add FRED related VMCS fields to dump_vmcs() to have it dump FRED context.
>
>Signed-off-by: Xin Li <xin3.li@intel.com>
>Tested-by: Shan Kang <shan.kang@intel.com>
>---
>
>Change since v1:
>* Use kvm_cpu_cap_has() instead of cpu_feature_enabled() (Chao Gao).
>* Dump guest FRED states only if guest has FRED enabled (Nikolay Borisov).
>---
> arch/x86/kvm/vmx/vmx.c | 46 +++++++++++++++++++++++++++++++++++-------
> 1 file changed, 39 insertions(+), 7 deletions(-)
>
>diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>index a484b9ac2400..e3409607122d 100644
>--- a/arch/x86/kvm/vmx/vmx.c
>+++ b/arch/x86/kvm/vmx/vmx.c
>@@ -6392,7 +6392,7 @@ void dump_vmcs(struct kvm_vcpu *vcpu)
>        struct vcpu_vmx *vmx = to_vmx(vcpu);
>        u32 vmentry_ctl, vmexit_ctl;
>        u32 cpu_based_exec_ctrl, pin_based_exec_ctrl, secondary_exec_control;
>-       u64 tertiary_exec_control;
>+       u64 tertiary_exec_control, secondary_vmexit_ctl;
>        unsigned long cr4;
>        int efer_slot;
>
>@@ -6403,6 +6403,8 @@ void dump_vmcs(struct kvm_vcpu *vcpu)
>
>        vmentry_ctl = vmcs_read32(VM_ENTRY_CONTROLS);
>        vmexit_ctl = vmcs_read32(VM_EXIT_CONTROLS);
>+       secondary_vmexit_ctl = cpu_has_secondary_vmexit_ctrls() ?
>+                              vmcs_read64(SECONDARY_VM_EXIT_CONTROLS) : 0;
>        cpu_based_exec_ctrl = vmcs_read32(CPU_BASED_VM_EXEC_CONTROL);
>        pin_based_exec_ctrl = vmcs_read32(PIN_BASED_VM_EXEC_CONTROL);
>        cr4 = vmcs_readl(GUEST_CR4);
>@@ -6449,6 +6451,19 @@ 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);
>+#ifdef CONFIG_X86_64
>+       if (kvm_is_fred_enabled(vcpu)) {

FRED MSRs are accessible even if CR4.FRED isn't set and #ifdef is ugly, I think
you can simply do:

	if (vmentry_ctrl & VM_ENTRY_LOAD_IA32_FRED)

just like below handling for EFER/PAT etc.

>+               pr_err("FRED guest: config=0x%016llx, stack levels=0x%016llx\n"
>+                      "RSP0=0x%016lx, RSP1=0x%016llx\n"
>+                      "RSP2=0x%016llx, RSP3=0x%016llx\n",
>+                      vmcs_read64(GUEST_IA32_FRED_CONFIG),
>+                      vmcs_read64(GUEST_IA32_FRED_STKLVLS),
>+                      read_msr(MSR_IA32_FRED_RSP0),
>+                      vmcs_read64(GUEST_IA32_FRED_RSP1),
>+                      vmcs_read64(GUEST_IA32_FRED_RSP2),
>+                      vmcs_read64(GUEST_IA32_FRED_RSP3));
>+       }
>+#endif
>        efer_slot = vmx_find_loadstore_msr_slot(&vmx->msr_autoload.guest, MSR_EFER);
>        if (vmentry_ctl & VM_ENTRY_LOAD_IA32_EFER)
>                pr_err("EFER= 0x%016llx\n", vmcs_read64(GUEST_IA32_EFER));
>@@ -6496,6 +6511,19 @@ 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));
>+#ifdef CONFIG_X86_64
>+       if (kvm_cpu_cap_has(X86_FEATURE_FRED)) {

ditto

>+               pr_err("FRED host: config=0x%016llx, stack levels=0x%016llx\n"
>+                      "RSP0=0x%016llx, RSP1=0x%016llx\n"
>+                      "RSP2=0x%016llx, RSP3=0x%016llx\n",
>+                      vmcs_read64(HOST_IA32_FRED_CONFIG),
>+                      vmcs_read64(HOST_IA32_FRED_STKLVLS),
>+                      vmx->msr_host_fred_rsp0,
>+                      vmcs_read64(HOST_IA32_FRED_RSP1),
>+                      vmcs_read64(HOST_IA32_FRED_RSP2),
>+                      vmcs_read64(HOST_IA32_FRED_RSP3));
>+       }
>+#endif
>        pr_err("CR0=%016lx CR3=%016lx CR4=%016lx\n",
>               vmcs_readl(HOST_CR0), vmcs_readl(HOST_CR3),
>               vmcs_readl(HOST_CR4));
>@@ -6517,25 +6545,29 @@ void dump_vmcs(struct kvm_vcpu *vcpu)
>        pr_err("*** Control State ***\n");
>        pr_err("CPUBased=0x%08x SecondaryExec=0x%08x TertiaryExec=0x%016llx\n",
>               cpu_based_exec_ctrl, secondary_exec_control, tertiary_exec_control);
>-       pr_err("PinBased=0x%08x EntryControls=%08x ExitControls=%08x\n",
>-              pin_based_exec_ctrl, vmentry_ctl, vmexit_ctl);
>+       pr_err("PinBased=0x%08x EntryControls=0x%08x\n",
>+              pin_based_exec_ctrl, vmentry_ctl);
>+       pr_err("ExitControls=0x%08x SecondaryExitControls=0x%016llx\n",
>+              vmexit_ctl, secondary_vmexit_ctl);
>        pr_err("ExceptionBitmap=%08x PFECmask=%08x PFECmatch=%08x\n",
>               vmcs_read32(EXCEPTION_BITMAP),
>               vmcs_read32(PAGE_FAULT_ERROR_CODE_MASK),
>               vmcs_read32(PAGE_FAULT_ERROR_CODE_MATCH));
>-       pr_err("VMEntry: intr_info=%08x errcode=%08x ilen=%08x\n",
>+       pr_err("VMEntry: intr_info=%08x errcode=%08x ilen=%08x event data=%016llx\n",

s/event data/event_data/

>               vmcs_read32(VM_ENTRY_INTR_INFO_FIELD),
>               vmcs_read32(VM_ENTRY_EXCEPTION_ERROR_CODE),
>-              vmcs_read32(VM_ENTRY_INSTRUCTION_LEN));
>+              vmcs_read32(VM_ENTRY_INSTRUCTION_LEN),
>+              kvm_cpu_cap_has(X86_FEATURE_FRED) ? vmcs_read64(INJECTED_EVENT_DATA) : 0);

again, it is better to check some vmexit/vmentry ctrl bit.

>        pr_err("VMExit: intr_info=%08x errcode=%08x ilen=%08x\n",
>               vmcs_read32(VM_EXIT_INTR_INFO),
>               vmcs_read32(VM_EXIT_INTR_ERROR_CODE),
>               vmcs_read32(VM_EXIT_INSTRUCTION_LEN));
>        pr_err("        reason=%08x qualification=%016lx\n",
>               vmcs_read32(VM_EXIT_REASON), vmcs_readl(EXIT_QUALIFICATION));
>-       pr_err("IDTVectoring: info=%08x errcode=%08x\n",
>+       pr_err("IDTVectoring: info=%08x errcode=%08x event data=%016llx\n",

s/event data/event_data/

>               vmcs_read32(IDT_VECTORING_INFO_FIELD),
>-              vmcs_read32(IDT_VECTORING_ERROR_CODE));
>+              vmcs_read32(IDT_VECTORING_ERROR_CODE),
>+              kvm_cpu_cap_has(X86_FEATURE_FRED) ? vmcs_read64(ORIGINAL_EVENT_DATA) : 0);

ditto