Liu Yu wrote:
-----Original Message-----
From: Christian Ehrhardt [mailto:ehrhardt@xxxxxxxxxxxxxxxxxx]
Sent: Monday, August 11, 2008 7:48 PM
To: Liu Yu
Cc: Hollis Blanchard; kvm-ppc@xxxxxxxxxxxxxxx
Subject: Re: [PATCH 4 of 5] kvm: ppc: Write only modified
shadow entries into theTLB on exit
When you set nearly all tlb entries modified in this loop,
you have to
write all of them back before entering the guest in
booke_interrupts.S.
So, why not "_tlbia()" here outside this loop, and don't
set them modified.
Then, you needn't write them back.
In fact, the shadow tlb entries are still consistent with
the hardware, as all of them are invalidated.
Am I overlooking something ?
As Hollis wrote in his patch comment usually just one tlb
entry is modified which saves a lot of updates.
If we invalidate all we later on would have to satisfy the
misses we cause by doing so.
In this loop, aren't you invalidating them all?
With some free time we could test _tlbia() here, but I expect
it to be slower by causing the need for tlb re-population
compared to what we save here.
I don't understand why it will be slower.
I think _tlbia will take the same effect as what you do here.
That is to say, it will always cause the need for tlb re-population.
And what's more, _tlbia will save some time when it enter the guest.
On 440 which this file 440x_tlb.c is for, the implementation of tlbia is
more or less the same than Hollis code when reentering the guest (there
is no tlbia instruction on 440) with the addition of a check of the
shadow modified flag.
I agree with you that the code in the kvmppc_mmu_priv_switch effectively
releases all tlbe's to the high water mark, marks them modified and
later on pushes them (being invalid) when entering the guest.
That means that the part of revoking all tlbe's to the high water mark
can be done with tlbia() instead of using the kvmppc_tlbe_set_modified
call for all indexes here.
I also agree that this can save us memory accesses from 0 to
tlb_44x_hwater for the setting of the shadow modified flag, but
everything else will be the same. Don't get me wrong I usually embrace
any performance improvement, but when you look in the patched source
file the is a comment just above the patched section:
/* Future optimization: clear only userspace mappings. */
Once we do that (or anything similar if we get other ideas here) keeping
the guest kernel mapping we can't use _tlbia() anymore because it clears
everyting. That as an example would save us a guest exit which should
save more time than some memory accesses.
Alltogether thats just my opinion, maybe we both wait for Hollis come
back from vacation next week and add a third opinion to this discussion
making decisions easier.
P.S. as I stated above this code is from the 440 tlb implementation, so
if e.g. there is hardware support for it on some platforms that might
make that improvement more beneficial we need an abstraction here.
--
Grüsse / regards,
Christian Ehrhardt
IBM Linux Technology Center, Open Virtualization
--
To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
