Re: [PATCH v5 09/19] KVM:x86: Make guest supervisor states as non-XSAVE managed

From: Chao Gao <chao.gao@intel.com>
To: Yang Weijiang <weijiang.yang@intel.com>
Cc: <seanjc@google.com>, <pbonzini@redhat.com>,
	<peterz@infradead.org>, <john.allen@amd.com>,
	<kvm@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<rick.p.edgecombe@intel.com>, <binbin.wu@linux.intel.com>
Subject: Re: [PATCH v5 09/19] KVM:x86: Make guest supervisor states as non-XSAVE managed
Date: Thu, 3 Aug 2023 19:15:03 +0800	[thread overview]
Message-ID: <ZMuMN/8Qa1sjJR/n@chao-email> (raw)
In-Reply-To: <20230803042732.88515-10-weijiang.yang@intel.com>

On Thu, Aug 03, 2023 at 12:27:22AM -0400, Yang Weijiang wrote:
>+void save_cet_supervisor_ssp(struct kvm_vcpu *vcpu)
>+{
>+	if (unlikely(guest_can_use(vcpu, X86_FEATURE_SHSTK))) {
>+		rdmsrl(MSR_IA32_PL0_SSP, vcpu->arch.cet_s_ssp[0]);
>+		rdmsrl(MSR_IA32_PL1_SSP, vcpu->arch.cet_s_ssp[1]);
>+		rdmsrl(MSR_IA32_PL2_SSP, vcpu->arch.cet_s_ssp[2]);

>+		/*
>+		 * Omit reset to host PL{1,2}_SSP because Linux will never use
>+		 * these MSRs.
>+		 */
>+		wrmsrl(MSR_IA32_PL0_SSP, 0);

This wrmsrl() can be dropped because host doesn't support SSS yet.

>+	}
>+}
>+EXPORT_SYMBOL_GPL(save_cet_supervisor_ssp);
>+
>+void reload_cet_supervisor_ssp(struct kvm_vcpu *vcpu)
>+{
>+	if (unlikely(guest_can_use(vcpu, X86_FEATURE_SHSTK))) {

ditto

>+		wrmsrl(MSR_IA32_PL0_SSP, vcpu->arch.cet_s_ssp[0]);
>+		wrmsrl(MSR_IA32_PL1_SSP, vcpu->arch.cet_s_ssp[1]);
>+		wrmsrl(MSR_IA32_PL2_SSP, vcpu->arch.cet_s_ssp[2]);
>+	}
>+}
>+EXPORT_SYMBOL_GPL(reload_cet_supervisor_ssp);
>+
> int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu)
> {
> 	struct kvm_queued_exception *ex = &vcpu->arch.exception;
>@@ -12133,6 +12158,7 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
> 
> 	vcpu->arch.cr3 = 0;
> 	kvm_register_mark_dirty(vcpu, VCPU_EXREG_CR3);
>+	memset(vcpu->arch.cet_s_ssp, 0, sizeof(vcpu->arch.cet_s_ssp));
> 
> 	/*
> 	 * CR0.CD/NW are set on RESET, preserved on INIT.  Note, some versions
>@@ -12313,6 +12339,7 @@ void kvm_arch_sched_in(struct kvm_vcpu *vcpu, int cpu)
> 		pmu->need_cleanup = true;
> 		kvm_make_request(KVM_REQ_PMU, vcpu);
> 	}
>+

remove the stray newline.

> 	static_call(kvm_x86_sched_in)(vcpu, cpu);
> }
> 
>diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
>index 6e6292915f8c..c69fc027f5ec 100644
>--- a/arch/x86/kvm/x86.h
>+++ b/arch/x86/kvm/x86.h
>@@ -501,6 +501,9 @@ static inline void kvm_machine_check(void)
> 
> void kvm_load_guest_xsave_state(struct kvm_vcpu *vcpu);
> void kvm_load_host_xsave_state(struct kvm_vcpu *vcpu);
>+void save_cet_supervisor_ssp(struct kvm_vcpu *vcpu);
>+void reload_cet_supervisor_ssp(struct kvm_vcpu *vcpu);

nit: please add kvm_ prefix to the function names because they are exposed to
other modules. "cet" in the names is a little redundant. I slightly prefer
kvm_save/load_guest_supervisor_ssp()

Overall, this patch looks good to me. Hence,

Reviewed-by: Chao Gao <chao.gao@intel.com>

>+
> int kvm_spec_ctrl_test_value(u64 value);
> bool __kvm_is_valid_cr4(struct kvm_vcpu *vcpu, unsigned long cr4);
> int kvm_handle_memory_failure(struct kvm_vcpu *vcpu, int r,
>-- 
>2.27.0
>