[PATCH] KVM: MMU: initialize sptes early

[PATCH] KVM: MMU: initialize sptes early

[Zhao Jin]

Otherwise, the following kvm_sync_pages() will see invalid sptes in a new
shadow page.

Signed-off-by: Zhao Jin <cronozhj@gmail.com>
---
 arch/x86/kvm/mmu.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 8e8da79..d7e1694 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -1692,6 +1692,7 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
 		return sp;
 	sp->gfn = gfn;
 	sp->role = role;
+	init_shadow_page_table(sp);
 	hlist_add_head(&sp->hash_link,
 		&vcpu->kvm->arch.mmu_page_hash[kvm_page_table_hashfn(gfn)]);
 	if (!direct) {
@@ -1702,7 +1703,6 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
 
 		account_shadowed(vcpu->kvm, gfn);
 	}
-	init_shadow_page_table(sp);
 	trace_kvm_mmu_get_page(sp, true);
 	return sp;
 }
-- 
1.7.5.4

[PATCH] KVM: MMU: fix the condition of syncing a new shadow page

[Zhao Jin]

Should be "or" since a new shadow page is synced if either it is
not leaf or there already exists another unsync shadow page with 
the same gfn.

Signed-off-by: Zhao Jin <cronozhj@gmail.com>
---
 arch/x86/kvm/mmu.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index d7e1694..f36de41 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -1698,7 +1698,7 @@ static struct kvm_mmu_page *kvm_mmu_get_page(struct kvm_vcpu *vcpu,
 	if (!direct) {
 		if (rmap_write_protect(vcpu->kvm, gfn))
 			kvm_flush_remote_tlbs(vcpu->kvm);
-		if (level > PT_PAGE_TABLE_LEVEL && need_sync)
+		if (level > PT_PAGE_TABLE_LEVEL || need_sync)
 			kvm_sync_pages(vcpu, gfn);
 
 		account_shadowed(vcpu->kvm, gfn);
-- 
1.7.5.4

Re: [PATCH] KVM: MMU: fix the condition of syncing a new shadow page

[Xiao Guangrong]

On 2011/10/24 15:21, Zhao Jin wrote:
> Should be "or" since a new shadow page is synced if either it is
> not leaf or there already exists another unsync shadow page with 
> the same gfn.
> 

It is obviously wrong, we need to sync pages only if it has unsync page
*and* the new shadow page breaks the unsync rule(only the level 1 sp can
became unsync).

Re: [PATCH] KVM: MMU: fix the condition of syncing a new shadow page

[Zhao Jin]

2011/10/24 Xiao Guangrong <xiao.guangrong@qq.com>:
> On 2011/10/24 15:21, Zhao Jin wrote:
>> Should be "or" since a new shadow page is synced if either it is
>> not leaf or there already exists another unsync shadow page with
>> the same gfn.
>>
>
> It is obviously wrong, we need to sync pages only if it has unsync page
> *and* the new shadow page breaks the unsync rule(only the level 1 sp can
> became unsync).
>

Please ignore this patch as I had taken an incorrect assumption.
Thanks very much for the correction.

[PATCH] KVM: VMX: fix incorrect operand

[Zhao Jin]

Should test save->ar for access rights.

Signed-off-by: Zhao Jin <cronozhj@gmail.com>
---
 arch/x86/kvm/vmx.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index e65a158..62086da 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -2544,7 +2544,7 @@ static void fix_pmode_dataseg(int seg, struct kvm_save_segment *save)
 {
 	struct kvm_vmx_segment_field *sf = &kvm_vmx_segment_fields[seg];
 
-	if (vmcs_readl(sf->base) == save->base && (save->base & AR_S_MASK)) {
+	if (vmcs_readl(sf->base) == save->base && (save->ar & AR_S_MASK)) {
 		vmcs_write16(sf->selector, save->selector);
 		vmcs_writel(sf->base, save->base);
 		vmcs_write32(sf->limit, save->limit);
-- 
1.7.5.4

Re: [PATCH] KVM: MMU: initialize sptes early

[Xiao Guangrong]

On 2011/10/24 15:21, Zhao Jin wrote:
> Otherwise, the following kvm_sync_pages() will see invalid sptes in a new
> shadow page.
> 

No, kvm_sync_pages just handle the unsync page, but the new sp is the sync page.

Re: [PATCH] KVM: MMU: initialize sptes early

[Zhao Jin]

2011/10/24 Xiao Guangrong <xiao.guangrong@qq.com>:
> On 2011/10/24 15:21, Zhao Jin wrote:
>> Otherwise, the following kvm_sync_pages() will see invalid sptes in a new
>> shadow page.
>>
>
> No, kvm_sync_pages just handle the unsync page, but the new sp is the sync page.
>

Sorry, I didn't notice the sp itself was zero-ed when allocated hence
was considered as synced. Please ignore this patch.
Thanks for the remainder.