On 04/06/2012 05:57 AM, Xiao Guangrong wrote:
>>>>
>>>> What's the difference between this and
>>>>
>>>>
>>>> if test_and_set_bit(spte.lock)
>>>> return_to_guest
>>>> else
>>>> do checks and cmpxchg
>>>>
>>>> ?
>>>>
>>>
>>>
>>> test_and_set_bit is a atomic operation that is i want to avoid.
>>
>> Right. Can you check what the effect is (with say
>> test_and_set_bit(spte, 0) in the same path).
>>
>> I'm not convinced it's significant.
>>
>
>
> Okay, if you prefer to test_and_set_bit, we may introduce two bits:
> fast_pf and write_protect, do things like this:
>
> on fast page fault path:
>
> old_spte = *spte
>
> if (test_and_set_bit(spte.fast_pf))
> return_to_guest
>
> old_spte |= fast_pf
>
> if (old_spte.wp)
> clear-fast-pf bit and return_to_guest
>
> if (!rmap.PTE_LIST_WRITE_PROTECT)
> cmpxchg(spte, old_spte, old_spte +w - fast_pf)
> else
> clear-fast-pf bit
>
> on page write-protect path:
> lock mmu-lock
> set rmap.PTE_LIST_WRITE_PROTECT
> smp_mb()
> if (spte.w || spte.fast_pf)
> spte -w -fast_pf + write_protect
>
> unlock mmu-lock
>
> I think it works but can not make page fault fast for unsync sp (
> e.g: two sptes point to the gfn, a page fault on one of the spte
> make gfn's sp become unsync.), it is not too bad since we can cache
> access bits in spte and properly make all sptes become writable on
> mmu_need_write_protect path in a new patchset.
>
Foolish me, i should be crazy. Sorry for my mistake. :(
Unfortunately, it can not work, we can not get a stable gfn from gpte or
sp->gfns[]. For example:
beginning:
Gpte = Gfn1
gfn_to_pfn(Gfn1) = Pfn
Spte = Pfn
Gfn1 is write-free
Gfn2 is write-protected
VCPU 0 VCPU 1 VCPU 2
fault on gpte
fast page fault path:
set Spte.fast_pf
get Gfn1 from Gpte/sp->gfns[]
if (Gfn1 is writable)
Pfn is swapped out:
Spte = 0
Gpte is modified to Gfn2,
and Pfn is realloced and remapped
to Gfn2, so:
Spte = Pfn
fast page fault path:
set Spte.fast_pf
cmpxchg Spte+w
OOPS!!!
<we see Spte is not changed and
happily make it writable, so gfn2 can be writable>
It seems only a unique identification can prevent this. :(
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
