Two cases maybe happen in kvm_mmu_get_page() function:
- one case is, the goal sp is already in cache, if the sp is unsync,
we only need update it to assure this mapping is valid, but not
mark it sync and not write-protect sp->gfn since it not broke unsync
rule(one shadow page for a gfn)
- another case is, the goal sp not existed, we need create a new sp
for gfn, i.e, gfn (may)has another shadow page, to keep unsync rule,
we should sync(mark sync and write-protect) gfn's unsync shadow page.
After enabling multiple unsync shadows, we sync those shadow pages
only when the new sp not allow to become unsync(also for the unsyc
rule, the new rule is: allow all pte page become unsync)
Signed-off-by: Xiao Guangrong <[email protected]>
---
arch/x86/kvm/mmu.c | 14 +++++++++++---
1 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 60a91c6..b946a5f 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -1330,7 +1330,7 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
unsigned index;
unsigned quadrant;
struct hlist_head *bucket;
- struct kvm_mmu_page *sp;
+ struct kvm_mmu_page *sp, *unsync_sp = NULL;
struct hlist_node *node, *tmp;
role = vcpu->arch.mmu.base_role;
@@ -1349,12 +1349,17 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
hlist_for_each_entry_safe(sp, node, tmp, bucket, hash_link)
if (sp->gfn == gfn) {
if (sp->unsync)
- if (kvm_sync_page(vcpu, sp))
- continue;
+ unsync_sp = sp;
if (sp->role.word != role.word)
continue;
+ if (unsync_sp &&
+ kvm_sync_page_transient(vcpu, unsync_sp)) {
+ unsync_sp = NULL;
+ break;
+ }
+
mmu_page_add_parent_pte(vcpu, sp, parent_pte);
if (sp->unsync_children) {
set_bit(KVM_REQ_MMU_SYNC, &vcpu->requests);
@@ -1363,6 +1368,9 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
trace_kvm_mmu_get_page(sp, false);
return sp;
}
+ if (!direct && unsync_sp)
+ kvm_sync_page(vcpu, unsync_sp);
+
++vcpu->kvm->stat.mmu_cache_miss;
sp = kvm_mmu_alloc_page(vcpu, parent_pte);
if (!sp)
--
1.6.1.2
On 04/25/2010 10:00 AM, Xiao Guangrong wrote:
> Two cases maybe happen in kvm_mmu_get_page() function:
>
> - one case is, the goal sp is already in cache, if the sp is unsync,
> we only need update it to assure this mapping is valid, but not
> mark it sync and not write-protect sp->gfn since it not broke unsync
> rule(one shadow page for a gfn)
>
> - another case is, the goal sp not existed, we need create a new sp
> for gfn, i.e, gfn (may)has another shadow page, to keep unsync rule,
> we should sync(mark sync and write-protect) gfn's unsync shadow page.
> After enabling multiple unsync shadows, we sync those shadow pages
> only when the new sp not allow to become unsync(also for the unsyc
> rule, the new rule is: allow all pte page become unsync)
>
Another interesting case is to create new shadow pages in the unsync
state. That can help when the guest starts a short lived process: we
can avoid write protecting its pagetables completely. Even if we do
sync them, we can sync them in a batch instead of one by one, saving IPIs.
--
error compiling committee.c: too many arguments to function
Avi Kivity wrote:
> On 04/25/2010 10:00 AM, Xiao Guangrong wrote:
>> Two cases maybe happen in kvm_mmu_get_page() function:
>>
>> - one case is, the goal sp is already in cache, if the sp is unsync,
>> we only need update it to assure this mapping is valid, but not
>> mark it sync and not write-protect sp->gfn since it not broke unsync
>> rule(one shadow page for a gfn)
>>
>> - another case is, the goal sp not existed, we need create a new sp
>> for gfn, i.e, gfn (may)has another shadow page, to keep unsync rule,
>> we should sync(mark sync and write-protect) gfn's unsync shadow page.
>> After enabling multiple unsync shadows, we sync those shadow pages
>> only when the new sp not allow to become unsync(also for the unsyc
>> rule, the new rule is: allow all pte page become unsync)
>>
>
> Another interesting case is to create new shadow pages in the unsync
> state. That can help when the guest starts a short lived process: we
> can avoid write protecting its pagetables completely. Even if we do
> sync them, we can sync them in a batch instead of one by one, saving IPIs.
IPI is needed when rmap_write_protect() changes mappings form writable to read-only,
so while we sync all gfn's unsync page, only one IPI is needed.
And, another problem is we call ramp_write_protect()/flush-local-tlb many times when sync gfn's
unsync page, the same problem is in mmu_sync_children() function, could you allow me to improve
it after this patchset? :-)
On 04/26/2010 06:58 AM, Xiao Guangrong wrote:
>
> Avi Kivity wrote:
>
>> On 04/25/2010 10:00 AM, Xiao Guangrong wrote:
>>
>>> Two cases maybe happen in kvm_mmu_get_page() function:
>>>
>>> - one case is, the goal sp is already in cache, if the sp is unsync,
>>> we only need update it to assure this mapping is valid, but not
>>> mark it sync and not write-protect sp->gfn since it not broke unsync
>>> rule(one shadow page for a gfn)
>>>
>>> - another case is, the goal sp not existed, we need create a new sp
>>> for gfn, i.e, gfn (may)has another shadow page, to keep unsync rule,
>>> we should sync(mark sync and write-protect) gfn's unsync shadow page.
>>> After enabling multiple unsync shadows, we sync those shadow pages
>>> only when the new sp not allow to become unsync(also for the unsyc
>>> rule, the new rule is: allow all pte page become unsync)
>>>
>>>
>> Another interesting case is to create new shadow pages in the unsync
>> state. That can help when the guest starts a short lived process: we
>> can avoid write protecting its pagetables completely. Even if we do
>> sync them, we can sync them in a batch instead of one by one, saving IPIs.
>>
> IPI is needed when rmap_write_protect() changes mappings form writable to read-only,
> so while we sync all gfn's unsync page, only one IPI is needed.
>
I meant, we can write protect all pages, then use one IPI to drop the
tlbs for all of them.
> And, another problem is we call ramp_write_protect()/flush-local-tlb many times when sync gfn's
> unsync page, the same problem is in mmu_sync_children() function, could you allow me to improve
> it after this patchset? :-)
>
Of course, this is more than enough to chew on. Just suggesting an idea...
--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.