Re: [PATCH 2/3] kvm: svm: reset cr0 properly on vcpu reset
Eduardo Habkost wrote: > On Wed, Mar 17, 2010 at 10:48:23PM +0100, Alexander Graf wrote: > >> On 17.03.2010, at 22:42, Eduardo Habkost wrote: >> >> >>> On Wed, Mar 17, 2010 at 07:17:32PM +0100, Alexander Graf wrote: >>> Eduardo Habkost wrote: > svm_vcpu_reset() was not properly resetting the contents of the > guest-visible > cr0 register, causing the following issue: > https://bugzilla.redhat.com/show_bug.cgi?id=525699 > > Without resetting cr0 properly, the vcpu was running the SIPI bootstrap > routine > with paging enabled, making the vcpu get a pagefault exception while > trying to > run it. > > Instead of setting vmcb->save.cr0 directly, the new code just resets > kvm->arch.cr0 and calls kvm_set_cr0(). The bits that were set/cleared on > vmcb->save.cr0 (PG, WP, !CD, !NW) will be set properly by svm_set_cr0(). > > kvm_set_cr0() is used instead of calling svm_set_cr0() directly to make > sure > kvm_mmu_reset_context() is called to reset the mmu to nonpaging mode. > > Signed-off-by: Eduardo Habkost > > Should this go into -stable? >>> I think so. The patch is from October, was -stable branched before that? >>> >> If I read the diff log correctly 2.6.32 kvm development was branched >> off end of July 2009. The important question is if this patch fixes a >> regression introduced by some speedup magic. >> > > I have just checked git history, and it looks like this is not a > regression. Before this patch, vcpu->cr0 (the guest-visible cr0 value) > was never reset on vcpu reset, but only vcpu->svm->vmcb->save.cr0 (the > actual cr0 value used by the CPU). > Good to know. Thanks for looking into this! Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] kvm: svm: reset cr0 properly on vcpu reset
On Wed, Mar 17, 2010 at 10:48:23PM +0100, Alexander Graf wrote: > > On 17.03.2010, at 22:42, Eduardo Habkost wrote: > > > On Wed, Mar 17, 2010 at 07:17:32PM +0100, Alexander Graf wrote: > >> Eduardo Habkost wrote: > >>> svm_vcpu_reset() was not properly resetting the contents of the > >>> guest-visible > >>> cr0 register, causing the following issue: > >>> https://bugzilla.redhat.com/show_bug.cgi?id=525699 > >>> > >>> Without resetting cr0 properly, the vcpu was running the SIPI bootstrap > >>> routine > >>> with paging enabled, making the vcpu get a pagefault exception while > >>> trying to > >>> run it. > >>> > >>> Instead of setting vmcb->save.cr0 directly, the new code just resets > >>> kvm->arch.cr0 and calls kvm_set_cr0(). The bits that were set/cleared on > >>> vmcb->save.cr0 (PG, WP, !CD, !NW) will be set properly by svm_set_cr0(). > >>> > >>> kvm_set_cr0() is used instead of calling svm_set_cr0() directly to make > >>> sure > >>> kvm_mmu_reset_context() is called to reset the mmu to nonpaging mode. > >>> > >>> Signed-off-by: Eduardo Habkost > >>> > >> > >> Should this go into -stable? > > > > I think so. The patch is from October, was -stable branched before that? > > If I read the diff log correctly 2.6.32 kvm development was branched > off end of July 2009. The important question is if this patch fixes a > regression introduced by some speedup magic. I have just checked git history, and it looks like this is not a regression. Before this patch, vcpu->cr0 (the guest-visible cr0 value) was never reset on vcpu reset, but only vcpu->svm->vmcb->save.cr0 (the actual cr0 value used by the CPU). -- Eduardo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] kvm: svm: reset cr0 properly on vcpu reset
On 17.03.2010, at 22:42, Eduardo Habkost wrote: > On Wed, Mar 17, 2010 at 07:17:32PM +0100, Alexander Graf wrote: >> Eduardo Habkost wrote: >>> svm_vcpu_reset() was not properly resetting the contents of the >>> guest-visible >>> cr0 register, causing the following issue: >>> https://bugzilla.redhat.com/show_bug.cgi?id=525699 >>> >>> Without resetting cr0 properly, the vcpu was running the SIPI bootstrap >>> routine >>> with paging enabled, making the vcpu get a pagefault exception while trying >>> to >>> run it. >>> >>> Instead of setting vmcb->save.cr0 directly, the new code just resets >>> kvm->arch.cr0 and calls kvm_set_cr0(). The bits that were set/cleared on >>> vmcb->save.cr0 (PG, WP, !CD, !NW) will be set properly by svm_set_cr0(). >>> >>> kvm_set_cr0() is used instead of calling svm_set_cr0() directly to make sure >>> kvm_mmu_reset_context() is called to reset the mmu to nonpaging mode. >>> >>> Signed-off-by: Eduardo Habkost >>> >> >> Should this go into -stable? > > I think so. The patch is from October, was -stable branched before that? If I read the diff log correctly 2.6.32 kvm development was branched off end of July 2009. The important question is if this patch fixes a regression introduced by some speedup magic. Alex-- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] kvm: svm: reset cr0 properly on vcpu reset
On Wed, Mar 17, 2010 at 07:17:32PM +0100, Alexander Graf wrote: > Eduardo Habkost wrote: > > svm_vcpu_reset() was not properly resetting the contents of the > > guest-visible > > cr0 register, causing the following issue: > > https://bugzilla.redhat.com/show_bug.cgi?id=525699 > > > > Without resetting cr0 properly, the vcpu was running the SIPI bootstrap > > routine > > with paging enabled, making the vcpu get a pagefault exception while trying > > to > > run it. > > > > Instead of setting vmcb->save.cr0 directly, the new code just resets > > kvm->arch.cr0 and calls kvm_set_cr0(). The bits that were set/cleared on > > vmcb->save.cr0 (PG, WP, !CD, !NW) will be set properly by svm_set_cr0(). > > > > kvm_set_cr0() is used instead of calling svm_set_cr0() directly to make sure > > kvm_mmu_reset_context() is called to reset the mmu to nonpaging mode. > > > > Signed-off-by: Eduardo Habkost > > > > Should this go into -stable? I think so. The patch is from October, was -stable branched before that? -- Eduardo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] kvm: svm: reset cr0 properly on vcpu reset
Eduardo Habkost wrote: > svm_vcpu_reset() was not properly resetting the contents of the guest-visible > cr0 register, causing the following issue: > https://bugzilla.redhat.com/show_bug.cgi?id=525699 > > Without resetting cr0 properly, the vcpu was running the SIPI bootstrap > routine > with paging enabled, making the vcpu get a pagefault exception while trying to > run it. > > Instead of setting vmcb->save.cr0 directly, the new code just resets > kvm->arch.cr0 and calls kvm_set_cr0(). The bits that were set/cleared on > vmcb->save.cr0 (PG, WP, !CD, !NW) will be set properly by svm_set_cr0(). > > kvm_set_cr0() is used instead of calling svm_set_cr0() directly to make sure > kvm_mmu_reset_context() is called to reset the mmu to nonpaging mode. > > Signed-off-by: Eduardo Habkost > Should this go into -stable? Alex -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] kvm: svm: reset cr0 properly on vcpu reset
svm_vcpu_reset() was not properly resetting the contents of the guest-visible cr0 register, causing the following issue: https://bugzilla.redhat.com/show_bug.cgi?id=525699 Without resetting cr0 properly, the vcpu was running the SIPI bootstrap routine with paging enabled, making the vcpu get a pagefault exception while trying to run it. Instead of setting vmcb->save.cr0 directly, the new code just resets kvm->arch.cr0 and calls kvm_set_cr0(). The bits that were set/cleared on vmcb->save.cr0 (PG, WP, !CD, !NW) will be set properly by svm_set_cr0(). kvm_set_cr0() is used instead of calling svm_set_cr0() directly to make sure kvm_mmu_reset_context() is called to reset the mmu to nonpaging mode. Signed-off-by: Eduardo Habkost --- arch/x86/kvm/svm.c |9 + 1 files changed, 5 insertions(+), 4 deletions(-) diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 170b2d9..6b86a11 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -626,11 +626,12 @@ static void init_vmcb(struct vcpu_svm *svm) save->rip = 0xfff0; svm->vcpu.arch.regs[VCPU_REGS_RIP] = save->rip; - /* -* cr0 val on cpu init should be 0x6010, we enable cpu -* cache by default. the orderly way is to enable cache in bios. + /* This is the guest-visible cr0 value. +* svm_set_cr0() sets PG and WP and clears NW and CD on save->cr0. */ - save->cr0 = 0x0010 | X86_CR0_PG | X86_CR0_WP; + svm->vcpu.arch.cr0 = X86_CR0_NW | X86_CR0_CD | X86_CR0_ET; + kvm_set_cr0(&svm->vcpu, svm->vcpu.arch.cr0); + save->cr4 = X86_CR4_PAE; /* rdx = ?? */ -- 1.6.3.rc4.29.g8146 -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
