linux-kernel - Re: [kvm-devel] [PATCH 8/8] SVM: add support for Nested Paging

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: <479C464E.2060009@qumranet.com>
Date:	Sun, 27 Jan 2008 10:52:30 +0200
From:	Avi Kivity <avi@...ranet.com>
To:	Joerg Roedel <joerg.roedel@....com>
CC:	kvm-devel@...ts.sourceforge.net, linux-kernel@...r.kernel.org
Subject: Re: [kvm-devel] [PATCH 8/8] SVM: add support for Nested Paging

Joerg Roedel wrote:
> This patch contains the SVM architecture dependent changes for KVM to enable
> support for the Nested Paging feature of AMD Barcelona and Phenom processors.
>  
> +#ifdef CONFIG_X86_64
> +static bool npt_enabled = true;
> +#else
>  static bool npt_enabled = false;
> +#endif
>   

I think that i386 + pae can also support npt, with no changes, no?

So we should check CONFIG_X86_PAE, not X86_64.

> +
> +	if (npt_enabled) {
> +		/* Setup VMCB for Nested Paging */
> +		control->nested_ctl = 1;
> +		control->intercept_exceptions &= ~(1 << PF_VECTOR);
> +		control->intercept_cr_read &= ~(INTERCEPT_CR0_MASK|
> +						INTERCEPT_CR3_MASK|
> +						INTERCEPT_CR4_MASK);
> +		control->intercept_cr_write &= ~(INTERCEPT_CR0_MASK|
> +						 INTERCEPT_CR3_MASK|
> +						 INTERCEPT_CR4_MASK);
>   

What happens to lazy fpu if we don't trap cr0 changes?

Perhaps it's worth disabling lazy fpu with npt.

>  static int svm_vcpu_reset(struct kvm_vcpu *vcpu)
> @@ -789,6 +812,15 @@ static void svm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0)
>  {
>  	struct vcpu_svm *svm = to_svm(vcpu);
>  
> +	if (npt_enabled) {
> +		/*
> +		 * re-enable caching here because the QEMU bios
> +		 * does not do it - this results in some delay at
> +		 * reboot
> +		 */
> +		cr0 &= ~(X86_CR0_CD | X86_CR0_NW);
> +		goto set;
> +	}
>  #ifdef CONFIG_X86_64
>  	if (vcpu->arch.shadow_efer & EFER_LME) {
>  		if (!is_paging(vcpu) && (cr0 & X86_CR0_PG)) {
> @@ -812,13 +844,16 @@ static void svm_set_cr0(struct kvm_vcpu *vcpu, unsigned long cr0)
>  	cr0 &= ~(X86_CR0_CD | X86_CR0_NW);
>  	if (!vcpu->fpu_active)
>  		cr0 |= X86_CR0_TS;
> +set:
>  	svm->vmcb->save.cr0 = cr0;
>  }
>   

You could move 'cr0 &= ~(X86_CR0_CD | X86_CR0_NW);' from three lines 
above the 'set' label downwards, and avoid it in the if () above.
>  static int handle_exit(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu)
>  {
>  	struct vcpu_svm *svm = to_svm(vcpu);
>  	u32 exit_code = svm->vmcb->control.exit_code;
>  
> +	if (npt_enabled) {
> +		int mmu_reload = 0;
>   

blank line

> +		if (((vcpu->arch.cr0 ^ svm->vmcb->save.cr0) & X86_CR0_PG)
> +		    || ((vcpu->arch.cr4 ^ svm->vmcb->save.cr4) &
> +			(X86_CR4_PGE|X86_CR4_PAE)))
> +			mmu_reload = 1;
> +		vcpu->arch.cr0 = svm->vmcb->save.cr0;
> +		vcpu->arch.cr4 = svm->vmcb->save.cr4;
> +		vcpu->arch.cr3 = svm->vmcb->save.cr3;
> +		if (mmu_reload) {
> +			kvm_mmu_reset_context(vcpu);
> +			kvm_mmu_load(vcpu);
> +		}
> +		if (is_pae(vcpu) && !is_long_mode(vcpu))
> +			load_pdptrs(vcpu, vcpu->arch.cr3);
>   

load_pdptrs() may fail; you need to check that.


-- 
error compiling committee.c: too many arguments to function

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/