linux-kernel - RE: [PATCH 1/4] KVM: x86: Add a struct to consolidate host values, e.g. EFER, XCR0, etc...

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <DS0PR11MB6373B95FF222DD6939CFEFC6DC172@DS0PR11MB6373.namprd11.prod.outlook.com>
Date: Thu, 25 Apr 2024 11:29:50 +0000
From: "Wang, Wei W" <wei.w.wang@...el.com>
To: Sean Christopherson <seanjc@...gle.com>, Paolo Bonzini
	<pbonzini@...hat.com>
CC: "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 1/4] KVM: x86: Add a struct to consolidate host values,
 e.g. EFER, XCR0, etc...

On Wednesday, April 24, 2024 6:15 AM, Sean Christopherson wrote:
> @@ -403,7 +403,7 @@ static void vmx_update_fb_clear_dis(struct kvm_vcpu
> *vcpu, struct vcpu_vmx *vmx)
>  	 * and VM-Exit.
>  	 */
>  	vmx->disable_fb_clear
> = !cpu_feature_enabled(X86_FEATURE_CLEAR_CPU_BUF) &&
> -				(host_arch_capabilities &
> ARCH_CAP_FB_CLEAR_CTRL) &&
> +				(kvm_host.arch_capabilities &
> ARCH_CAP_FB_CLEAR_CTRL) &&

The line of code appears to be lengthy. It would be preferable to limit it to under
80 columns per line.

>  				!boot_cpu_has_bug(X86_BUG_MDS) &&
>  				!boot_cpu_has_bug(X86_BUG_TAA);
> 
> @@ -1116,12 +1116,12 @@ static bool update_transition_efer(struct
> vcpu_vmx *vmx)
>  	 * atomically, since it's faster than switching it manually.
>  	 */
>  	if (cpu_has_load_ia32_efer() ||
> -	    (enable_ept && ((vmx->vcpu.arch.efer ^ host_efer) & EFER_NX))) {
> +	    (enable_ept && ((vmx->vcpu.arch.efer ^ kvm_host.efer) & EFER_NX)))
> +{
>  		if (!(guest_efer & EFER_LMA))
>  			guest_efer &= ~EFER_LME;
> -		if (guest_efer != host_efer)
> +		if (guest_efer != kvm_host.efer)
>  			add_atomic_switch_msr(vmx, MSR_EFER,
> -					      guest_efer, host_efer, false);
> +					      guest_efer, kvm_host.efer, false);
>  		else
>  			clear_atomic_switch_msr(vmx, MSR_EFER);
>  		return false;
> diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h index
> d80a4c6b5a38..e69fff7d1f21 100644
> --- a/arch/x86/kvm/x86.h
> +++ b/arch/x86/kvm/x86.h
> @@ -33,6 +33,13 @@ struct kvm_caps {
>  	u64 supported_perf_cap;
>  };
> 
> +struct kvm_host_values {
> +	u64 efer;
> +	u64 xcr0;
> +	u64 xss;
> +	u64 arch_capabilities;
> +};
> +
>  void kvm_spurious_fault(void);
> 
>  #define KVM_NESTED_VMENTER_CONSISTENCY_CHECK(consistency_check)
> 		\
> @@ -325,11 +332,8 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu,
> gpa_t cr2_or_gpa,
>  			    int emulation_type, void *insn, int insn_len);
> fastpath_t handle_fastpath_set_msr_irqoff(struct kvm_vcpu *vcpu);
> 
> -extern u64 host_xcr0;
> -extern u64 host_xss;
> -extern u64 host_arch_capabilities;
> -
>  extern struct kvm_caps kvm_caps;
> +extern struct kvm_host_values kvm_host;

Have you considered merging the kvm_host_values and kvm_caps into one unified
structure?
(If the concern is about naming, we could brainstorm a more encompassing term
for them)