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

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]

Date:   Thu, 3 Aug 2023 19:15:03 +0800
From:   Chao Gao <chao.gao@...el.com>
To:     Yang Weijiang <weijiang.yang@...el.com>
CC:     <seanjc@...gle.com>, <pbonzini@...hat.com>, <peterz@...radead.org>,
        <john.allen@....com>, <kvm@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <rick.p.edgecombe@...el.com>,
        <binbin.wu@...ux.intel.com>
Subject: Re: [PATCH v5 09/19] KVM:x86: Make guest supervisor states as
 non-XSAVE managed

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@...el.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
>