From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH 2/2] KVM: Make CR4 reloads also reload CR3 if paging is enabled Date: Mon, 25 May 2009 18:08:33 +0300 Message-ID: <4A1AB471.5070901@redhat.com> References: <1243241244-25953-1-git-send-email-avi@redhat.com> <1243241244-25953-3-git-send-email-avi@redhat.com> <20090525115842.GA3417@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org To: Marcelo Tosatti Return-path: Received: from mx2.redhat.com ([66.187.237.31]:45510 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751582AbZEYPId (ORCPT ); Mon, 25 May 2009 11:08:33 -0400 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n4PF8Zdw005705 for ; Mon, 25 May 2009 11:08:35 -0400 In-Reply-To: <20090525115842.GA3417@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: Marcelo Tosatti wrote: > On Mon, May 25, 2009 at 11:47:24AM +0300, Avi Kivity wrote: > >> The processor is documented to reload the PDPTRs while in PAE mode if any >> of the CR4 bits PSE, PGE, or PAE change. Linux relies on this >> behaviour when zapping the low mappings of PAE kernels during boot. >> >> The code already handled changes to CR4.PAE; augment it to also notice changes >> to PSE and PGE. >> >> This triggered while booting an F11 PAE kernel; the futex initialization code >> runs before any CR3 reloads and writes to a NULL pointer; the futex subsystem >> ended up uninitialized, killing PI futexes and pulseaudio which uses them. >> > > One comment regarding set_cr0. Section 8.1 of the TLB doc says: > > * The processor does not maintain a PDP cache as described in > Section 4. The processor always caches information from the four > page-directory-pointer-table entries. These entries are not cached at > the time of address translation. Instead, they are always cached as part > of the execution of the following instructions: > > * A MOV to CR0 that modifies CR0.PG and that occurs with IA32_EFER.LMA = 0 > and CR4.PAE = 1. > > However kvm_set_cr0 only caches the PDPTRs if CR0.PG changed from 0->1. > Can't see a problem there though. > Yes, if cr0.pg == 0, then the pdptrs are meaningless. > Also, the checks in kvm_arch_vcpu_ioctl_set_sregs should probably be unified > with kvm_set_crX, to avoid future mistakes. > Yes please. But it needs to be done very carefully, since the order of the checks matters here. > Otherwise, ACK. > Thanks for the review. -- error compiling committee.c: too many arguments to function