Re: [PATCH 3/8] kvm/mmu: rename is_largepage_backed to mapping_level

Joerg Roedel
2009-06-23T17:00:47Z

On Tue, Jun 23, 2009 at 12:59:33PM -0300, Marcelo Tosatti wrote:
> Hi Joerg,
>
> On Fri, Jun 19, 2009 at 03:16:24PM +0200, Joerg Roedel wrote:
> > gfn = unalias_gfn(kvm, gfn);
> > - write_count = slot_largepage_idx(gfn,
> > - gfn_to_memslot_unaliased(kvm, gfn));
> > - *write_count += 1;
> > +
> > + for (i = PT_DIRECTORY_LEVEL;
> > + i < PT_PAGE_TABLE_LEVEL + KVM_NR_PAGE_SIZES; ++i) {
> > + slot = gfn_to_memslot_unaliased(kvm, gfn);
>
> Can't you move this call out of the loop?

True. Will do this.

> > @@ -1704,7 +1739,7 @@ static int set_spte(struct kvm_vcpu *vcpu, u64 *sptep,
> > if ((pte_access & ACC_WRITE_MASK)
> > || (write_fault && !is_write_protection(vcpu) && !user_fault)) {
> >
> > - if (largepage && has_wrprotected_page(vcpu->kvm, gfn)) {
> > + if (largepage && has_wrprotected_page(vcpu->kvm, gfn, 1)) {
>
> It seems direct_map is missing the large pte overwrite check that
> fetch() contains:
>
> if (is_large_pte(*sptep)) {
> rmap_remove(vcpu->kvm, sptep);
> __set_spte(sptep, shadow_trap_nonpresent_pte);
> kvm_flush_remote_tlbs(vcpu->kvm);
> }
>
> (perhaps its not a possible scenario at the moment, but...).

This function is only called from mmu_set_spte which takes care of this.

Thanks,

Joerg

--
| Advanced Micro Devices GmbH
Operating | Karl-Hammerschmidt-Str. 34, 85609 Dornach bei München
System |
Research | Geschäftsführer: Thomas M. McCoy, Giuliano Meroni
Center | Sitz: Dornach, Gemeinde Aschheim, Landkreis München
| Registergericht München, HRB Nr. 43632