linux-kernel - Re: [PATCH 08/28] KVM: nSVM: Use dedicated array of MSRPM offsets to merge L0 and L1 bitmaps

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: <688ba3c4-bf8a-47f1-ad14-85a23399582c@redhat.com>
Date: Wed, 4 Jun 2025 17:35:36 +0200
From: Paolo Bonzini <pbonzini@...hat.com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
 Borislav Petkov <bp@...en8.de>, Xin Li <xin@...or.com>,
 Chao Gao <chao.gao@...el.com>, Dapeng Mi <dapeng1.mi@...ux.intel.com>
Subject: Re: [PATCH 08/28] KVM: nSVM: Use dedicated array of MSRPM offsets to
 merge L0 and L1 bitmaps

On 5/30/25 01:39, Sean Christopherson wrote:
> Use a dedicated array of MSRPM offsets to merge L0 and L1 bitmaps, i.e. to
> merge KVM's vmcb01 bitmap with L1's vmcb12 bitmap.  This will eventually
> allow for the removal of direct_access_msrs, as the only path where
> tracking the offsets is truly justified is the merge for nested SVM, where
> merging in chunks is an easy way to batch uaccess reads/writes.
> 
> Opportunistically omit the x2APIC MSRs from the merge-specific array
> instead of filtering them out at runtime.
> 
> Note, disabling interception of XSS, EFER, PAT, GHCB, and TSC_AUX is
> mutually exclusive with nested virtualization, as KVM passes through the
> MSRs only for SEV-ES guests, and KVM doesn't support nested virtualization
> for SEV+ guests.  Defer removing those MSRs to a future cleanup in order
> to make this refactoring as benign as possible.
> 
> Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> ---
>   arch/x86/kvm/svm/nested.c | 72 +++++++++++++++++++++++++++++++++------
>   arch/x86/kvm/svm/svm.c    |  4 +++
>   arch/x86/kvm/svm/svm.h    |  2 ++
>   3 files changed, 67 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 89a77f0f1cc8..e53020939e60 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -184,6 +184,64 @@ void recalc_intercepts(struct vcpu_svm *svm)
>   	}
>   }
>   
> +static int nested_svm_msrpm_merge_offsets[9] __ro_after_init;
> +static int nested_svm_nr_msrpm_merge_offsets __ro_after_init;
> +
> +int __init nested_svm_init_msrpm_merge_offsets(void)
> +{
> +	const u32 merge_msrs[] = {

"static const", please.

Paolo

> +		MSR_STAR,
> +		MSR_IA32_SYSENTER_CS,
> +		MSR_IA32_SYSENTER_EIP,
> +		MSR_IA32_SYSENTER_ESP,
> +	#ifdef CONFIG_X86_64
> +		MSR_GS_BASE,
> +		MSR_FS_BASE,
> +		MSR_KERNEL_GS_BASE,
> +		MSR_LSTAR,
> +		MSR_CSTAR,
> +		MSR_SYSCALL_MASK,
> +	#endif
> +		MSR_IA32_SPEC_CTRL,
> +		MSR_IA32_PRED_CMD,
> +		MSR_IA32_FLUSH_CMD,
> +		MSR_IA32_LASTBRANCHFROMIP,
> +		MSR_IA32_LASTBRANCHTOIP,
> +		MSR_IA32_LASTINTFROMIP,
> +		MSR_IA32_LASTINTTOIP,
> +
> +		MSR_IA32_XSS,
> +		MSR_EFER,
> +		MSR_IA32_CR_PAT,
> +		MSR_AMD64_SEV_ES_GHCB,
> +		MSR_TSC_AUX,
> +	};
> +	int i, j;
> +
> +	for (i = 0; i < ARRAY_SIZE(merge_msrs); i++) {
> +		u32 offset = svm_msrpm_offset(merge_msrs[i]);
> +
> +		if (WARN_ON(offset == MSR_INVALID))
> +			return -EIO;
> +
> +		for (j = 0; j < nested_svm_nr_msrpm_merge_offsets; j++) {
> +			if (nested_svm_msrpm_merge_offsets[j] == offset)
> +				break;
> +		}
> +
> +		if (j < nested_svm_nr_msrpm_merge_offsets)
> +			continue;
> +
> +		if (WARN_ON(j >= ARRAY_SIZE(nested_svm_msrpm_merge_offsets)))
> +			return -EIO;
> +
> +		nested_svm_msrpm_merge_offsets[j] = offset;
> +		nested_svm_nr_msrpm_merge_offsets++;
> +	}
> +
> +	return 0;
> +}
> +
>   /*
>    * Merge L0's (KVM) and L1's (Nested VMCB) MSR permission bitmaps. The function
>    * is optimized in that it only merges the parts where KVM MSR permission bitmap
> @@ -216,19 +274,11 @@ static bool nested_svm_merge_msrpm(struct kvm_vcpu *vcpu)
>   	if (!(vmcb12_is_intercept(&svm->nested.ctl, INTERCEPT_MSR_PROT)))
>   		return true;
>   
> -	for (i = 0; i < MSRPM_OFFSETS; i++) {
> -		u32 value, p;
> +	for (i = 0; i < nested_svm_nr_msrpm_merge_offsets; i++) {
> +		const int p = nested_svm_msrpm_merge_offsets[i];
> +		u32 value;
>   		u64 offset;
>   
> -		if (msrpm_offsets[i] == 0xffffffff)
> -			break;
> -
> -		p      = msrpm_offsets[i];
> -
> -		/* x2apic msrs are intercepted always for the nested guest */
> -		if (is_x2apic_msrpm_offset(p))
> -			continue;
> -
>   		offset = svm->nested.ctl.msrpm_base_pa + (p * 4);
>   
>   		if (kvm_vcpu_read_guest(vcpu, offset, &value, 4))
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 1c70293400bc..84dd1f220986 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -5689,6 +5689,10 @@ static int __init svm_init(void)
>   	if (!kvm_is_svm_supported())
>   		return -EOPNOTSUPP;
>   
> +	r = nested_svm_init_msrpm_merge_offsets();
> +	if (r)
> +		return r;
> +
>   	r = kvm_x86_vendor_init(&svm_init_ops);
>   	if (r)
>   		return r;
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 909b9af6b3c1..0a8041d70994 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -686,6 +686,8 @@ static inline bool nested_exit_on_nmi(struct vcpu_svm *svm)
>   	return vmcb12_is_intercept(&svm->nested.ctl, INTERCEPT_NMI);
>   }
>   
> +int __init nested_svm_init_msrpm_merge_offsets(void);
> +
>   int enter_svm_guest_mode(struct kvm_vcpu *vcpu,
>   			 u64 vmcb_gpa, struct vmcb *vmcb12, bool from_vmrun);
>   void svm_leave_nested(struct kvm_vcpu *vcpu);