Re: [PATCH 2/2] KVM: Make CR4 reloads also reload CR3 if paging is enabled

[Avi Kivity <avi@redhat.com>]

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