* [PATCH 1/9] KVM: x86: remvoe unnecessary mark_page_dirty
@ 2012-07-17 13:50 Xiao Guangrong
2012-07-17 13:51 ` [PATCH 2/9] KVM: x86: simplify read_emulated Xiao Guangrong
` (8 more replies)
0 siblings, 9 replies; 25+ messages in thread
From: Xiao Guangrong @ 2012-07-17 13:50 UTC (permalink / raw)
To: Avi Kivity; +Cc: Marcelo Tosatti, LKML, KVM
fix:
[ 132.474633] 3.5.0-rc1+ #50 Not tainted
[ 132.474634] -------------------------------
[ 132.474635] include/linux/kvm_host.h:369 suspicious rcu_dereference_check() usage!
[ 132.474636]
[ 132.474636] other info that might help us debug this:
[ 132.474636]
[ 132.474638]
[ 132.474638] rcu_scheduler_active = 1, debug_locks = 1
[ 132.474640] 1 lock held by qemu-kvm/2832:
[ 132.474657] #0: (&vcpu->mutex){+.+.+.}, at: [<ffffffffa01e1636>] vcpu_load+0x1e/0x91 [kvm]
[ 132.474658]
[ 132.474658] stack backtrace:
[ 132.474660] Pid: 2832, comm: qemu-kvm Not tainted 3.5.0-rc1+ #50
[ 132.474661] Call Trace:
[ 132.474665] [<ffffffff81092f40>] lockdep_rcu_suspicious+0xfc/0x105
[ 132.474675] [<ffffffffa01e0c85>] kvm_memslots+0x6d/0x75 [kvm]
[ 132.474683] [<ffffffffa01e0ca1>] gfn_to_memslot+0x14/0x4c [kvm]
[ 132.474693] [<ffffffffa01e3575>] mark_page_dirty+0x17/0x2a [kvm]
[ 132.474706] [<ffffffffa01f21ea>] kvm_arch_vcpu_ioctl+0xbcf/0xc07 [kvm]
Actually, we do not write vcpu->arch.time at this time, mark_page_dirty should
be removed
Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
arch/x86/kvm/x86.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ff0b487..875b8d8 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2615,7 +2615,6 @@ static int kvm_set_guest_paused(struct kvm_vcpu *vcpu)
if (!vcpu->arch.time_page)
return -EINVAL;
src->flags |= PVCLOCK_GUEST_STOPPED;
- mark_page_dirty(vcpu->kvm, vcpu->arch.time >> PAGE_SHIFT);
kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
return 0;
}
--
1.7.7.6
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 2/9] KVM: x86: simplify read_emulated
2012-07-17 13:50 [PATCH 1/9] KVM: x86: remvoe unnecessary mark_page_dirty Xiao Guangrong
@ 2012-07-17 13:51 ` Xiao Guangrong
2012-07-19 23:58 ` Marcelo Tosatti
2012-07-17 13:52 ` [PATCH 3/9] KVM: x86: introduce set_mmio_exit_info Xiao Guangrong
` (7 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Xiao Guangrong @ 2012-07-17 13:51 UTC (permalink / raw)
To: Xiao Guangrong; +Cc: Avi Kivity, Marcelo Tosatti, LKML, KVM
No need split mmio read region into 8-bits pieces since we do it in
emulator_read_write_onepage
Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
arch/x86/kvm/emulate.c | 29 ++++++++++++-----------------
1 files changed, 12 insertions(+), 17 deletions(-)
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
index 97d9a99..2d1916b 100644
--- a/arch/x86/kvm/emulate.c
+++ b/arch/x86/kvm/emulate.c
@@ -1166,24 +1166,19 @@ static int read_emulated(struct x86_emulate_ctxt *ctxt,
int rc;
struct read_cache *mc = &ctxt->mem_read;
- while (size) {
- int n = min(size, 8u);
- size -= n;
- if (mc->pos < mc->end)
- goto read_cached;
-
- rc = ctxt->ops->read_emulated(ctxt, addr, mc->data + mc->end, n,
- &ctxt->exception);
- if (rc != X86EMUL_CONTINUE)
- return rc;
- mc->end += n;
+ if (mc->pos < mc->end)
+ goto read_cached;
- read_cached:
- memcpy(dest, mc->data + mc->pos, n);
- mc->pos += n;
- dest += n;
- addr += n;
- }
+ rc = ctxt->ops->read_emulated(ctxt, addr, mc->data + mc->end, size,
+ &ctxt->exception);
+ if (rc != X86EMUL_CONTINUE)
+ return rc;
+
+ mc->end += size;
+
+read_cached:
+ memcpy(dest, mc->data + mc->pos, size);
+ mc->pos += size;
return X86EMUL_CONTINUE;
}
--
1.7.7.6
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 3/9] KVM: x86: introduce set_mmio_exit_info
2012-07-17 13:50 [PATCH 1/9] KVM: x86: remvoe unnecessary mark_page_dirty Xiao Guangrong
2012-07-17 13:51 ` [PATCH 2/9] KVM: x86: simplify read_emulated Xiao Guangrong
@ 2012-07-17 13:52 ` Xiao Guangrong
2012-07-20 0:03 ` Marcelo Tosatti
2012-07-17 13:52 ` [PATCH 4/9] KVM: MMU: track the refcount when unmap the page Xiao Guangrong
` (6 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Xiao Guangrong @ 2012-07-17 13:52 UTC (permalink / raw)
To: Xiao Guangrong; +Cc: Avi Kivity, Marcelo Tosatti, LKML, KVM
Introduce set_mmio_exit_info to cleanup the common code
Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
arch/x86/kvm/x86.c | 33 +++++++++++++++++----------------
1 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 875b8d8..8171836 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3759,9 +3759,6 @@ static int read_exit_mmio(struct kvm_vcpu *vcpu, gpa_t gpa,
static int write_exit_mmio(struct kvm_vcpu *vcpu, gpa_t gpa,
void *val, int bytes)
{
- struct kvm_mmio_fragment *frag = &vcpu->mmio_fragments[0];
-
- memcpy(vcpu->run->mmio.data, frag->data, frag->len);
return X86EMUL_CONTINUE;
}
@@ -3829,6 +3826,20 @@ mmio:
return X86EMUL_CONTINUE;
}
+static void set_mmio_exit_info(struct kvm_vcpu *vcpu,
+ struct kvm_mmio_fragment *frag, bool write)
+{
+ struct kvm_run *run = vcpu->run;
+
+ run->exit_reason = KVM_EXIT_MMIO;
+ run->mmio.phys_addr = frag->gpa;
+ run->mmio.len = frag->len;
+ run->mmio.is_write = vcpu->mmio_is_write = write;
+
+ if (write)
+ memcpy(run->mmio.data, frag->data, frag->len);
+}
+
int emulator_read_write(struct x86_emulate_ctxt *ctxt, unsigned long addr,
void *val, unsigned int bytes,
struct x86_exception *exception,
@@ -3868,14 +3879,10 @@ int emulator_read_write(struct x86_emulate_ctxt *ctxt, unsigned long addr,
return rc;
gpa = vcpu->mmio_fragments[0].gpa;
-
vcpu->mmio_needed = 1;
vcpu->mmio_cur_fragment = 0;
- vcpu->run->mmio.len = vcpu->mmio_fragments[0].len;
- vcpu->run->mmio.is_write = vcpu->mmio_is_write = ops->write;
- vcpu->run->exit_reason = KVM_EXIT_MMIO;
- vcpu->run->mmio.phys_addr = gpa;
+ set_mmio_exit_info(vcpu, &vcpu->mmio_fragments[0], ops->write);
return ops->read_write_exit_mmio(vcpu, gpa, val, bytes);
}
@@ -5485,7 +5492,6 @@ static int __vcpu_run(struct kvm_vcpu *vcpu)
*/
static int complete_mmio(struct kvm_vcpu *vcpu)
{
- struct kvm_run *run = vcpu->run;
struct kvm_mmio_fragment *frag;
int r;
@@ -5496,7 +5502,7 @@ static int complete_mmio(struct kvm_vcpu *vcpu)
/* Complete previous fragment */
frag = &vcpu->mmio_fragments[vcpu->mmio_cur_fragment++];
if (!vcpu->mmio_is_write)
- memcpy(frag->data, run->mmio.data, frag->len);
+ memcpy(frag->data, vcpu->run->mmio.data, frag->len);
if (vcpu->mmio_cur_fragment == vcpu->mmio_nr_fragments) {
vcpu->mmio_needed = 0;
if (vcpu->mmio_is_write)
@@ -5506,12 +5512,7 @@ static int complete_mmio(struct kvm_vcpu *vcpu)
}
/* Initiate next fragment */
++frag;
- run->exit_reason = KVM_EXIT_MMIO;
- run->mmio.phys_addr = frag->gpa;
- if (vcpu->mmio_is_write)
- memcpy(run->mmio.data, frag->data, frag->len);
- run->mmio.len = frag->len;
- run->mmio.is_write = vcpu->mmio_is_write;
+ set_mmio_exit_info(vcpu, frag, vcpu->mmio_is_write);
return 0;
}
--
1.7.7.6
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 4/9] KVM: MMU: track the refcount when unmap the page
2012-07-17 13:50 [PATCH 1/9] KVM: x86: remvoe unnecessary mark_page_dirty Xiao Guangrong
2012-07-17 13:51 ` [PATCH 2/9] KVM: x86: simplify read_emulated Xiao Guangrong
2012-07-17 13:52 ` [PATCH 3/9] KVM: x86: introduce set_mmio_exit_info Xiao Guangrong
@ 2012-07-17 13:52 ` Xiao Guangrong
2012-07-20 0:09 ` Marcelo Tosatti
2012-07-17 13:53 ` [PATCH 5/9] KVM: MMU: fask check write-protect for direct mmu Xiao Guangrong
` (5 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Xiao Guangrong @ 2012-07-17 13:52 UTC (permalink / raw)
To: Xiao Guangrong; +Cc: Avi Kivity, Marcelo Tosatti, LKML, KVM
It will trigger a WARN_ON if the page has been freed but it is still
used in mmu, it can help us to detect mm bug early
Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
arch/x86/kvm/mmu.c | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 28c8fbc..28b12e2 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -556,6 +556,14 @@ static int mmu_spte_clear_track_bits(u64 *sptep)
return 0;
pfn = spte_to_pfn(old_spte);
+
+ /*
+ * KVM does not hold the refcount of the page used by
+ * kvm mmu, before reclaiming the page, we should
+ * unmap it from mmu first.
+ */
+ WARN_ON(!kvm_is_mmio_pfn(pfn) && !page_count(pfn_to_page(pfn)));
+
if (!shadow_accessed_mask || old_spte & shadow_accessed_mask)
kvm_set_pfn_accessed(pfn);
if (!shadow_dirty_mask || (old_spte & shadow_dirty_mask))
--
1.7.7.6
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 5/9] KVM: MMU: fask check write-protect for direct mmu
2012-07-17 13:50 [PATCH 1/9] KVM: x86: remvoe unnecessary mark_page_dirty Xiao Guangrong
` (2 preceding siblings ...)
2012-07-17 13:52 ` [PATCH 4/9] KVM: MMU: track the refcount when unmap the page Xiao Guangrong
@ 2012-07-17 13:53 ` Xiao Guangrong
2012-07-20 0:39 ` Marcelo Tosatti
2012-07-17 13:54 ` [PATCH 6/9] KVM: using get_fault_pfn to get the fault pfn Xiao Guangrong
` (4 subsequent siblings)
8 siblings, 1 reply; 25+ messages in thread
From: Xiao Guangrong @ 2012-07-17 13:53 UTC (permalink / raw)
To: Xiao Guangrong; +Cc: Avi Kivity, Marcelo Tosatti, LKML, KVM
If it have no indirect shadow pages we need not protect any gfn,
this is always true for direct mmu without nested
Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
arch/x86/kvm/mmu.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 28b12e2..a846a9c 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -2294,6 +2294,9 @@ static int mmu_need_write_protect(struct kvm_vcpu *vcpu, gfn_t gfn,
struct hlist_node *node;
bool need_unsync = false;
+ if (!vcpu->kvm->arch.indirect_shadow_pages)
+ return 0;
+
for_each_gfn_indirect_valid_sp(vcpu->kvm, s, gfn, node) {
if (!can_unsync)
return 1;
--
1.7.7.6
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 6/9] KVM: using get_fault_pfn to get the fault pfn
2012-07-17 13:50 [PATCH 1/9] KVM: x86: remvoe unnecessary mark_page_dirty Xiao Guangrong
` (3 preceding siblings ...)
2012-07-17 13:53 ` [PATCH 5/9] KVM: MMU: fask check write-protect for direct mmu Xiao Guangrong
@ 2012-07-17 13:54 ` Xiao Guangrong
2012-07-17 13:54 ` [PATCH 7/9] KVM: mark do not extern bad_pfn Xiao Guangrong
` (3 subsequent siblings)
8 siblings, 0 replies; 25+ messages in thread
From: Xiao Guangrong @ 2012-07-17 13:54 UTC (permalink / raw)
To: Xiao Guangrong; +Cc: Avi Kivity, Marcelo Tosatti, LKML, KVM
Using get_fault_pfn to cleanup the code
Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
arch/x86/kvm/mmu.c | 6 ++----
include/linux/kvm_host.h | 5 +----
virt/kvm/kvm_main.c | 13 ++++---------
3 files changed, 7 insertions(+), 17 deletions(-)
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index a846a9c..e23cdae 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -2483,10 +2483,8 @@ static pfn_t pte_prefetch_gfn_to_pfn(struct kvm_vcpu *vcpu, gfn_t gfn,
unsigned long hva;
slot = gfn_to_memslot_dirty_bitmap(vcpu, gfn, no_dirty_log);
- if (!slot) {
- get_page(fault_page);
- return page_to_pfn(fault_page);
- }
+ if (!slot)
+ return get_fault_pfn();
hva = gfn_to_hva_memslot(slot, gfn);
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index e3c86f8..258b35a 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -383,15 +383,11 @@ id_to_memslot(struct kvm_memslots *slots, int id)
static inline int is_error_hpa(hpa_t hpa) { return hpa >> HPA_MSB; }
extern struct page *bad_page;
-extern struct page *fault_page;
-
extern pfn_t bad_pfn;
-extern pfn_t fault_pfn;
int is_error_page(struct page *page);
int is_error_pfn(pfn_t pfn);
int is_hwpoison_pfn(pfn_t pfn);
-int is_fault_pfn(pfn_t pfn);
int is_noslot_pfn(pfn_t pfn);
int is_invalid_pfn(pfn_t pfn);
int kvm_is_error_hva(unsigned long addr);
@@ -441,6 +437,7 @@ void kvm_release_pfn_clean(pfn_t pfn);
void kvm_set_pfn_dirty(pfn_t pfn);
void kvm_set_pfn_accessed(pfn_t pfn);
void kvm_get_pfn(pfn_t pfn);
+pfn_t get_fault_pfn(void);
int kvm_read_guest_page(struct kvm *kvm, gfn_t gfn, void *data, int offset,
int len);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 60af030..feef72e 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -103,8 +103,8 @@ static bool largepages_enabled = true;
static struct page *hwpoison_page;
static pfn_t hwpoison_pfn;
-struct page *fault_page;
-pfn_t fault_pfn;
+static struct page *fault_page;
+static pfn_t fault_pfn;
inline int kvm_is_mmio_pfn(pfn_t pfn)
{
@@ -950,12 +950,6 @@ int is_hwpoison_pfn(pfn_t pfn)
}
EXPORT_SYMBOL_GPL(is_hwpoison_pfn);
-int is_fault_pfn(pfn_t pfn)
-{
- return pfn == fault_pfn;
-}
-EXPORT_SYMBOL_GPL(is_fault_pfn);
-
int is_noslot_pfn(pfn_t pfn)
{
return pfn == bad_pfn;
@@ -1039,11 +1033,12 @@ unsigned long gfn_to_hva(struct kvm *kvm, gfn_t gfn)
}
EXPORT_SYMBOL_GPL(gfn_to_hva);
-static pfn_t get_fault_pfn(void)
+pfn_t get_fault_pfn(void)
{
get_page(fault_page);
return fault_pfn;
}
+EXPORT_SYMBOL_GPL(get_fault_pfn);
int get_user_page_nowait(struct task_struct *tsk, struct mm_struct *mm,
unsigned long start, int write, struct page **page)
--
1.7.7.6
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 7/9] KVM: mark do not extern bad_pfn
2012-07-17 13:50 [PATCH 1/9] KVM: x86: remvoe unnecessary mark_page_dirty Xiao Guangrong
` (4 preceding siblings ...)
2012-07-17 13:54 ` [PATCH 6/9] KVM: using get_fault_pfn to get the fault pfn Xiao Guangrong
@ 2012-07-17 13:54 ` Xiao Guangrong
2012-07-17 13:55 ` [PATCH 8/9] KVM: remove is_error_hpa Xiao Guangrong
` (2 subsequent siblings)
8 siblings, 0 replies; 25+ messages in thread
From: Xiao Guangrong @ 2012-07-17 13:54 UTC (permalink / raw)
To: Xiao Guangrong; +Cc: Avi Kivity, Marcelo Tosatti, LKML, KVM
bad_pfn is not used out of kvm_main.c, so mark it static, also move it near
hwpoison_pfn and fault_pfn
Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
include/linux/kvm_host.h | 1 -
virt/kvm/kvm_main.c | 6 +++---
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 258b35a..bafcc56 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -383,7 +383,6 @@ id_to_memslot(struct kvm_memslots *slots, int id)
static inline int is_error_hpa(hpa_t hpa) { return hpa >> HPA_MSB; }
extern struct page *bad_page;
-extern pfn_t bad_pfn;
int is_error_page(struct page *page);
int is_error_pfn(pfn_t pfn);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index feef72e..7082e33 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -100,6 +100,9 @@ EXPORT_SYMBOL_GPL(kvm_rebooting);
static bool largepages_enabled = true;
+struct page *bad_page;
+static pfn_t bad_pfn;
+
static struct page *hwpoison_page;
static pfn_t hwpoison_pfn;
@@ -2692,9 +2695,6 @@ static struct syscore_ops kvm_syscore_ops = {
.resume = kvm_resume,
};
-struct page *bad_page;
-pfn_t bad_pfn;
-
static inline
struct kvm_vcpu *preempt_notifier_to_vcpu(struct preempt_notifier *pn)
{
--
1.7.7.6
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 8/9] KVM: remove is_error_hpa
2012-07-17 13:50 [PATCH 1/9] KVM: x86: remvoe unnecessary mark_page_dirty Xiao Guangrong
` (5 preceding siblings ...)
2012-07-17 13:54 ` [PATCH 7/9] KVM: mark do not extern bad_pfn Xiao Guangrong
@ 2012-07-17 13:55 ` Xiao Guangrong
2012-07-17 13:56 ` [PATCH 9/9] KVM: remove the unused parameter of gfn_to_pfn_memslot Xiao Guangrong
2012-07-20 0:40 ` [PATCH 1/9] KVM: x86: remvoe unnecessary mark_page_dirty Marcelo Tosatti
8 siblings, 0 replies; 25+ messages in thread
From: Xiao Guangrong @ 2012-07-17 13:55 UTC (permalink / raw)
To: Xiao Guangrong; +Cc: Avi Kivity, Marcelo Tosatti, LKML, KVM
Remove them since they are not used anymore
Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
include/linux/kvm_host.h | 4 ----
1 files changed, 0 insertions(+), 4 deletions(-)
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index bafcc56..f1ef5fc 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -378,10 +378,6 @@ id_to_memslot(struct kvm_memslots *slots, int id)
return slot;
}
-#define HPA_MSB ((sizeof(hpa_t) * 8) - 1)
-#define HPA_ERR_MASK ((hpa_t)1 << HPA_MSB)
-static inline int is_error_hpa(hpa_t hpa) { return hpa >> HPA_MSB; }
-
extern struct page *bad_page;
int is_error_page(struct page *page);
--
1.7.7.6
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH 9/9] KVM: remove the unused parameter of gfn_to_pfn_memslot
2012-07-17 13:50 [PATCH 1/9] KVM: x86: remvoe unnecessary mark_page_dirty Xiao Guangrong
` (6 preceding siblings ...)
2012-07-17 13:55 ` [PATCH 8/9] KVM: remove is_error_hpa Xiao Guangrong
@ 2012-07-17 13:56 ` Xiao Guangrong
2012-07-20 0:40 ` [PATCH 1/9] KVM: x86: remvoe unnecessary mark_page_dirty Marcelo Tosatti
8 siblings, 0 replies; 25+ messages in thread
From: Xiao Guangrong @ 2012-07-17 13:56 UTC (permalink / raw)
To: Xiao Guangrong; +Cc: Avi Kivity, Marcelo Tosatti, LKML, KVM
The parameter, 'kvm', is not used in gfn_to_pfn_memslot, we can happily remove
it
Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
---
arch/powerpc/kvm/e500_tlb.c | 2 +-
arch/x86/kvm/mmu.c | 2 +-
include/linux/kvm_host.h | 5 ++---
virt/kvm/iommu.c | 10 +++++-----
virt/kvm/kvm_main.c | 15 +++++++--------
5 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/arch/powerpc/kvm/e500_tlb.c b/arch/powerpc/kvm/e500_tlb.c
index c510fc9..c8f6c58 100644
--- a/arch/powerpc/kvm/e500_tlb.c
+++ b/arch/powerpc/kvm/e500_tlb.c
@@ -520,7 +520,7 @@ static inline void kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
if (likely(!pfnmap)) {
unsigned long tsize_pages = 1 << (tsize + 10 - PAGE_SHIFT);
- pfn = gfn_to_pfn_memslot(vcpu_e500->vcpu.kvm, slot, gfn);
+ pfn = gfn_to_pfn_memslot(slot, gfn);
if (is_error_pfn(pfn)) {
printk(KERN_ERR "Couldn't get real page for gfn %lx!\n",
(long)gfn);
diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index e23cdae..4ed543a 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -2488,7 +2488,7 @@ static pfn_t pte_prefetch_gfn_to_pfn(struct kvm_vcpu *vcpu, gfn_t gfn,
hva = gfn_to_hva_memslot(slot, gfn);
- return hva_to_pfn_atomic(vcpu->kvm, hva);
+ return hva_to_pfn_atomic(hva);
}
static int direct_pte_prefetch_many(struct kvm_vcpu *vcpu,
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index f1ef5fc..14bfde4 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -418,15 +418,14 @@ void kvm_release_page_dirty(struct page *page);
void kvm_set_page_dirty(struct page *page);
void kvm_set_page_accessed(struct page *page);
-pfn_t hva_to_pfn_atomic(struct kvm *kvm, unsigned long addr);
+pfn_t hva_to_pfn_atomic(unsigned long addr);
pfn_t gfn_to_pfn_atomic(struct kvm *kvm, gfn_t gfn);
pfn_t gfn_to_pfn_async(struct kvm *kvm, gfn_t gfn, bool *async,
bool write_fault, bool *writable);
pfn_t gfn_to_pfn(struct kvm *kvm, gfn_t gfn);
pfn_t gfn_to_pfn_prot(struct kvm *kvm, gfn_t gfn, bool write_fault,
bool *writable);
-pfn_t gfn_to_pfn_memslot(struct kvm *kvm,
- struct kvm_memory_slot *slot, gfn_t gfn);
+pfn_t gfn_to_pfn_memslot(struct kvm_memory_slot *slot, gfn_t gfn);
void kvm_release_pfn_dirty(pfn_t);
void kvm_release_pfn_clean(pfn_t pfn);
void kvm_set_pfn_dirty(pfn_t pfn);
diff --git a/virt/kvm/iommu.c b/virt/kvm/iommu.c
index e9fff98..c03f1fb 100644
--- a/virt/kvm/iommu.c
+++ b/virt/kvm/iommu.c
@@ -42,13 +42,13 @@ static int kvm_iommu_unmap_memslots(struct kvm *kvm);
static void kvm_iommu_put_pages(struct kvm *kvm,
gfn_t base_gfn, unsigned long npages);
-static pfn_t kvm_pin_pages(struct kvm *kvm, struct kvm_memory_slot *slot,
- gfn_t gfn, unsigned long size)
+static pfn_t kvm_pin_pages(struct kvm_memory_slot *slot, gfn_t gfn,
+ unsigned long size)
{
gfn_t end_gfn;
pfn_t pfn;
- pfn = gfn_to_pfn_memslot(kvm, slot, gfn);
+ pfn = gfn_to_pfn_memslot(slot, gfn);
end_gfn = gfn + (size >> PAGE_SHIFT);
gfn += 1;
@@ -56,7 +56,7 @@ static pfn_t kvm_pin_pages(struct kvm *kvm, struct kvm_memory_slot *slot,
return pfn;
while (gfn < end_gfn)
- gfn_to_pfn_memslot(kvm, slot, gfn++);
+ gfn_to_pfn_memslot(slot, gfn++);
return pfn;
}
@@ -105,7 +105,7 @@ int kvm_iommu_map_pages(struct kvm *kvm, struct kvm_memory_slot *slot)
* Pin all pages we are about to map in memory. This is
* important because we unmap and unpin in 4kb steps later.
*/
- pfn = kvm_pin_pages(kvm, slot, gfn, page_size);
+ pfn = kvm_pin_pages(slot, gfn, page_size);
if (is_error_pfn(pfn)) {
gfn += 1;
continue;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 7082e33..78cf42f 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1063,8 +1063,8 @@ static inline int check_user_page_hwpoison(unsigned long addr)
return rc == -EHWPOISON;
}
-static pfn_t hva_to_pfn(struct kvm *kvm, unsigned long addr, bool atomic,
- bool *async, bool write_fault, bool *writable)
+static pfn_t hva_to_pfn(unsigned long addr, bool atomic, bool *async,
+ bool write_fault, bool *writable)
{
struct page *page[1];
int npages = 0;
@@ -1144,9 +1144,9 @@ static pfn_t hva_to_pfn(struct kvm *kvm, unsigned long addr, bool atomic,
return pfn;
}
-pfn_t hva_to_pfn_atomic(struct kvm *kvm, unsigned long addr)
+pfn_t hva_to_pfn_atomic(unsigned long addr)
{
- return hva_to_pfn(kvm, addr, true, NULL, true, NULL);
+ return hva_to_pfn(addr, true, NULL, true, NULL);
}
EXPORT_SYMBOL_GPL(hva_to_pfn_atomic);
@@ -1164,7 +1164,7 @@ static pfn_t __gfn_to_pfn(struct kvm *kvm, gfn_t gfn, bool atomic, bool *async,
return page_to_pfn(bad_page);
}
- return hva_to_pfn(kvm, addr, atomic, async, write_fault, writable);
+ return hva_to_pfn(addr, atomic, async, write_fault, writable);
}
pfn_t gfn_to_pfn_atomic(struct kvm *kvm, gfn_t gfn)
@@ -1193,11 +1193,10 @@ pfn_t gfn_to_pfn_prot(struct kvm *kvm, gfn_t gfn, bool write_fault,
}
EXPORT_SYMBOL_GPL(gfn_to_pfn_prot);
-pfn_t gfn_to_pfn_memslot(struct kvm *kvm,
- struct kvm_memory_slot *slot, gfn_t gfn)
+pfn_t gfn_to_pfn_memslot(struct kvm_memory_slot *slot, gfn_t gfn)
{
unsigned long addr = gfn_to_hva_memslot(slot, gfn);
- return hva_to_pfn(kvm, addr, false, NULL, true, NULL);
+ return hva_to_pfn(addr, false, NULL, true, NULL);
}
int gfn_to_page_many_atomic(struct kvm *kvm, gfn_t gfn, struct page **pages,
--
1.7.7.6
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH 2/9] KVM: x86: simplify read_emulated
2012-07-17 13:51 ` [PATCH 2/9] KVM: x86: simplify read_emulated Xiao Guangrong
@ 2012-07-19 23:58 ` Marcelo Tosatti
2012-07-20 2:17 ` Xiao Guangrong
0 siblings, 1 reply; 25+ messages in thread
From: Marcelo Tosatti @ 2012-07-19 23:58 UTC (permalink / raw)
To: Xiao Guangrong; +Cc: Avi Kivity, LKML, KVM
On Tue, Jul 17, 2012 at 09:51:34PM +0800, Xiao Guangrong wrote:
> No need split mmio read region into 8-bits pieces since we do it in
> emulator_read_write_onepage
>
> Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
> ---
> arch/x86/kvm/emulate.c | 29 ++++++++++++-----------------
> 1 files changed, 12 insertions(+), 17 deletions(-)
>
> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index 97d9a99..2d1916b 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -1166,24 +1166,19 @@ static int read_emulated(struct x86_emulate_ctxt *ctxt,
> int rc;
> struct read_cache *mc = &ctxt->mem_read;
>
> - while (size) {
> - int n = min(size, 8u);
> - size -= n;
> - if (mc->pos < mc->end)
> - goto read_cached;
> -
> - rc = ctxt->ops->read_emulated(ctxt, addr, mc->data + mc->end, n,
> - &ctxt->exception);
> - if (rc != X86EMUL_CONTINUE)
> - return rc;
> - mc->end += n;
> + if (mc->pos < mc->end)
> + goto read_cached;
>
> - read_cached:
> - memcpy(dest, mc->data + mc->pos, n);
> - mc->pos += n;
> - dest += n;
> - addr += n;
> - }
> + rc = ctxt->ops->read_emulated(ctxt, addr, mc->data + mc->end, size,
> + &ctxt->exception);
> + if (rc != X86EMUL_CONTINUE)
> + return rc;
> +
> + mc->end += size;
> +
> +read_cached:
> + memcpy(dest, mc->data + mc->pos, size);
What prevents read_emulated(size > 8) call, with
mc->pos == (mc->end - 8) now?
> + mc->pos += size;
> return X86EMUL_CONTINUE;
> }
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/9] KVM: x86: introduce set_mmio_exit_info
2012-07-17 13:52 ` [PATCH 3/9] KVM: x86: introduce set_mmio_exit_info Xiao Guangrong
@ 2012-07-20 0:03 ` Marcelo Tosatti
2012-07-20 2:24 ` Xiao Guangrong
0 siblings, 1 reply; 25+ messages in thread
From: Marcelo Tosatti @ 2012-07-20 0:03 UTC (permalink / raw)
To: Xiao Guangrong; +Cc: Avi Kivity, LKML, KVM
On Tue, Jul 17, 2012 at 09:52:13PM +0800, Xiao Guangrong wrote:
> Introduce set_mmio_exit_info to cleanup the common code
>
> Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
> ---
> arch/x86/kvm/x86.c | 33 +++++++++++++++++----------------
> 1 files changed, 17 insertions(+), 16 deletions(-)
This makes the code less readable IMO. The fragments are small enough
already.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 4/9] KVM: MMU: track the refcount when unmap the page
2012-07-17 13:52 ` [PATCH 4/9] KVM: MMU: track the refcount when unmap the page Xiao Guangrong
@ 2012-07-20 0:09 ` Marcelo Tosatti
0 siblings, 0 replies; 25+ messages in thread
From: Marcelo Tosatti @ 2012-07-20 0:09 UTC (permalink / raw)
To: Xiao Guangrong; +Cc: Avi Kivity, LKML, KVM
On Tue, Jul 17, 2012 at 09:52:52PM +0800, Xiao Guangrong wrote:
> It will trigger a WARN_ON if the page has been freed but it is still
> used in mmu, it can help us to detect mm bug early
>
> Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
> ---
> arch/x86/kvm/mmu.c | 8 ++++++++
> 1 files changed, 8 insertions(+), 0 deletions(-)
Applied, thanks.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 5/9] KVM: MMU: fask check write-protect for direct mmu
2012-07-17 13:53 ` [PATCH 5/9] KVM: MMU: fask check write-protect for direct mmu Xiao Guangrong
@ 2012-07-20 0:39 ` Marcelo Tosatti
2012-07-20 2:34 ` Xiao Guangrong
2012-07-20 3:45 ` Xiao Guangrong
0 siblings, 2 replies; 25+ messages in thread
From: Marcelo Tosatti @ 2012-07-20 0:39 UTC (permalink / raw)
To: Xiao Guangrong; +Cc: Avi Kivity, LKML, KVM
On Tue, Jul 17, 2012 at 09:53:29PM +0800, Xiao Guangrong wrote:
> If it have no indirect shadow pages we need not protect any gfn,
> this is always true for direct mmu without nested
>
> Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
Xiao,
What is the motivation? Numbers please.
In fact, what case was the original indirect_shadow_pages conditional in
kvm_mmu_pte_write optimizing again?
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 1/9] KVM: x86: remvoe unnecessary mark_page_dirty
2012-07-17 13:50 [PATCH 1/9] KVM: x86: remvoe unnecessary mark_page_dirty Xiao Guangrong
` (7 preceding siblings ...)
2012-07-17 13:56 ` [PATCH 9/9] KVM: remove the unused parameter of gfn_to_pfn_memslot Xiao Guangrong
@ 2012-07-20 0:40 ` Marcelo Tosatti
8 siblings, 0 replies; 25+ messages in thread
From: Marcelo Tosatti @ 2012-07-20 0:40 UTC (permalink / raw)
To: Xiao Guangrong; +Cc: Avi Kivity, LKML, KVM
Applied all patches except 2, 3 and 5, thanks.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/9] KVM: x86: simplify read_emulated
2012-07-19 23:58 ` Marcelo Tosatti
@ 2012-07-20 2:17 ` Xiao Guangrong
2012-07-20 10:58 ` Marcelo Tosatti
0 siblings, 1 reply; 25+ messages in thread
From: Xiao Guangrong @ 2012-07-20 2:17 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: Avi Kivity, LKML, KVM
On 07/20/2012 07:58 AM, Marcelo Tosatti wrote:
>> - }
>> + rc = ctxt->ops->read_emulated(ctxt, addr, mc->data + mc->end, size,
>> + &ctxt->exception);
>> + if (rc != X86EMUL_CONTINUE)
>> + return rc;
>> +
>> + mc->end += size;
>> +
>> +read_cached:
>> + memcpy(dest, mc->data + mc->pos, size);
>
> What prevents read_emulated(size > 8) call, with
> mc->pos == (mc->end - 8) now?
Marcelo,
The splitting has been done in emulator_read_write_onepage:
while (bytes) {
unsigned now = min(bytes, 8U);
frag = &vcpu->mmio_fragments[vcpu->mmio_nr_fragments++];
frag->gpa = gpa;
frag->data = val;
frag->len = now;
frag->write_readonly_mem = (ret == -EPERM);
gpa += now;
val += now;
bytes -= now;
}
So i think it is safe to remove the splitting in read_emulated.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 3/9] KVM: x86: introduce set_mmio_exit_info
2012-07-20 0:03 ` Marcelo Tosatti
@ 2012-07-20 2:24 ` Xiao Guangrong
0 siblings, 0 replies; 25+ messages in thread
From: Xiao Guangrong @ 2012-07-20 2:24 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: Avi Kivity, LKML, KVM
On 07/20/2012 08:03 AM, Marcelo Tosatti wrote:
> On Tue, Jul 17, 2012 at 09:52:13PM +0800, Xiao Guangrong wrote:
>> Introduce set_mmio_exit_info to cleanup the common code
>>
>> Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
>> ---
>> arch/x86/kvm/x86.c | 33 +++++++++++++++++----------------
>> 1 files changed, 17 insertions(+), 16 deletions(-)
>
> This makes the code less readable IMO. The fragments are small enough
> already.
>
I think the function name set_mmio_exit_info has the better
explanation - it generates a mmio-exit from the specified fragment.
Actually, the later patchset, readonly memeslot will set a new
member in the mmio-exit-info:
run->mmio.write_readonly_mem = frag->write_readonly_mem;
If we wrap this operation up, i think the code should be neater. :)
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 5/9] KVM: MMU: fask check write-protect for direct mmu
2012-07-20 0:39 ` Marcelo Tosatti
@ 2012-07-20 2:34 ` Xiao Guangrong
2012-07-20 11:09 ` Marcelo Tosatti
2012-07-20 3:45 ` Xiao Guangrong
1 sibling, 1 reply; 25+ messages in thread
From: Xiao Guangrong @ 2012-07-20 2:34 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: Avi Kivity, LKML, KVM
On 07/20/2012 08:39 AM, Marcelo Tosatti wrote:
> On Tue, Jul 17, 2012 at 09:53:29PM +0800, Xiao Guangrong wrote:
>> If it have no indirect shadow pages we need not protect any gfn,
>> this is always true for direct mmu without nested
>>
>> Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
>
> Xiao,
>
> What is the motivation? Numbers please.
>
mmu_need_write_protect is the common path for both soft-mmu and
hard-mmu, checking indirect_shadow_pages can skip hash-table walking
for the case which is tdp is enabled without nested guest.
I will post the Number after I do the performance test.
> In fact, what case was the original indirect_shadow_pages conditional in
> kvm_mmu_pte_write optimizing again?
>
They are the different paths, mmu_need_write_protect is the real
page fault path, and kvm_mmu_pte_write is caused by mmio emulation.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 5/9] KVM: MMU: fask check write-protect for direct mmu
2012-07-20 0:39 ` Marcelo Tosatti
2012-07-20 2:34 ` Xiao Guangrong
@ 2012-07-20 3:45 ` Xiao Guangrong
2012-07-20 11:45 ` Marcelo Tosatti
1 sibling, 1 reply; 25+ messages in thread
From: Xiao Guangrong @ 2012-07-20 3:45 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: Avi Kivity, LKML, KVM
BTW, they are some bug fix patches on -master branch, but
it is not existed on -next branch:
commit: f411930442e01f9cf1bf4df41ff7e89476575c4d
commit: 85b7059169e128c57a3a8a3e588fb89cb2031da1
It causes code conflict if we do the development on -next.
On 07/20/2012 08:39 AM, Marcelo Tosatti wrote:can
> On Tue, Jul 17, 2012 at 09:53:29PM +0800, Xiao Guangrong wrote:
>> If it have no indirect shadow pages we need not protect any gfn,
>> this is always true for direct mmu without nested
>>
>> Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
>
> Xiao,
>
> What is the motivation? Numbers please.
>
> In fact, what case was the original indirect_shadow_pages conditional in
> kvm_mmu_pte_write optimizing again?
>
>
>
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/9] KVM: x86: simplify read_emulated
2012-07-20 2:17 ` Xiao Guangrong
@ 2012-07-20 10:58 ` Marcelo Tosatti
2012-07-20 13:15 ` Xiao Guangrong
0 siblings, 1 reply; 25+ messages in thread
From: Marcelo Tosatti @ 2012-07-20 10:58 UTC (permalink / raw)
To: Xiao Guangrong; +Cc: Avi Kivity, LKML, KVM
On Fri, Jul 20, 2012 at 10:17:36AM +0800, Xiao Guangrong wrote:
> On 07/20/2012 07:58 AM, Marcelo Tosatti wrote:
>
> >> - }
> >> + rc = ctxt->ops->read_emulated(ctxt, addr, mc->data + mc->end, size,
> >> + &ctxt->exception);
> >> + if (rc != X86EMUL_CONTINUE)
> >> + return rc;
> >> +
> >> + mc->end += size;
> >> +
> >> +read_cached:
> >> + memcpy(dest, mc->data + mc->pos, size);
> >
> > What prevents read_emulated(size > 8) call, with
> > mc->pos == (mc->end - 8) now?
>
> Marcelo,
>
> The splitting has been done in emulator_read_write_onepage:
>
> while (bytes) {
> unsigned now = min(bytes, 8U);
>
> frag = &vcpu->mmio_fragments[vcpu->mmio_nr_fragments++];
> frag->gpa = gpa;
> frag->data = val;
> frag->len = now;
> frag->write_readonly_mem = (ret == -EPERM);
>
> gpa += now;
> val += now;
> bytes -= now;
> }
>
> So i think it is safe to remove the splitting in read_emulated.
Yes, it is fine to remove it.
But splitting in emulate.c prevented the case of _cache read_ with size
> 8 beyond end of mc->data. Must handle that case in read_emulated.
"What prevents read_emulated(size > 8) call, with mc->pos == (mc->end - 8) now?"
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 5/9] KVM: MMU: fask check write-protect for direct mmu
2012-07-20 2:34 ` Xiao Guangrong
@ 2012-07-20 11:09 ` Marcelo Tosatti
2012-07-20 13:33 ` Xiao Guangrong
0 siblings, 1 reply; 25+ messages in thread
From: Marcelo Tosatti @ 2012-07-20 11:09 UTC (permalink / raw)
To: Xiao Guangrong; +Cc: Avi Kivity, LKML, KVM
On Fri, Jul 20, 2012 at 10:34:28AM +0800, Xiao Guangrong wrote:
> On 07/20/2012 08:39 AM, Marcelo Tosatti wrote:
> > On Tue, Jul 17, 2012 at 09:53:29PM +0800, Xiao Guangrong wrote:
> >> If it have no indirect shadow pages we need not protect any gfn,
> >> this is always true for direct mmu without nested
> >>
> >> Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
> >
> > Xiao,
> >
> > What is the motivation? Numbers please.
> >
>
> mmu_need_write_protect is the common path for both soft-mmu and
> hard-mmu, checking indirect_shadow_pages can skip hash-table walking
> for the case which is tdp is enabled without nested guest.
I mean motivation as observation that it is a bottleneck.
> I will post the Number after I do the performance test.
>
> > In fact, what case was the original indirect_shadow_pages conditional in
> > kvm_mmu_pte_write optimizing again?
> >
>
> They are the different paths, mmu_need_write_protect is the real
> page fault path, and kvm_mmu_pte_write is caused by mmio emulation.
Sure. What i am asking is, what use case is the indirect_shadow_pages
optimizing? What scenario, what workload?
See the "When to optimize" section of
http://en.wikipedia.org/wiki/Program_optimization.
Can't remember why indirect_shadow_pages was introduced in
kvm_mmu_pte_write.
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 5/9] KVM: MMU: fask check write-protect for direct mmu
2012-07-20 3:45 ` Xiao Guangrong
@ 2012-07-20 11:45 ` Marcelo Tosatti
0 siblings, 0 replies; 25+ messages in thread
From: Marcelo Tosatti @ 2012-07-20 11:45 UTC (permalink / raw)
To: Xiao Guangrong; +Cc: Avi Kivity, LKML, KVM
On Fri, Jul 20, 2012 at 11:45:59AM +0800, Xiao Guangrong wrote:
> BTW, they are some bug fix patches on -master branch, but
> it is not existed on -next branch:
> commit: f411930442e01f9cf1bf4df41ff7e89476575c4d
> commit: 85b7059169e128c57a3a8a3e588fb89cb2031da1
>
> It causes code conflict if we do the development on -next.
See auto-next branch.
http://www.linux-kvm.org/page/Kvm-Git-Workflow
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/9] KVM: x86: simplify read_emulated
2012-07-20 10:58 ` Marcelo Tosatti
@ 2012-07-20 13:15 ` Xiao Guangrong
2012-07-20 19:52 ` Marcelo Tosatti
0 siblings, 1 reply; 25+ messages in thread
From: Xiao Guangrong @ 2012-07-20 13:15 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: Avi Kivity, LKML, KVM
On 07/20/2012 06:58 PM, Marcelo Tosatti wrote:
> On Fri, Jul 20, 2012 at 10:17:36AM +0800, Xiao Guangrong wrote:
>> On 07/20/2012 07:58 AM, Marcelo Tosatti wrote:
>>
>>>> - }
>>>> + rc = ctxt->ops->read_emulated(ctxt, addr, mc->data + mc->end, size,
>>>> + &ctxt->exception);
>>>> + if (rc != X86EMUL_CONTINUE)
>>>> + return rc;
>>>> +
>>>> + mc->end += size;
>>>> +
>>>> +read_cached:
>>>> + memcpy(dest, mc->data + mc->pos, size);
>>>
>>> What prevents read_emulated(size > 8) call, with
>>> mc->pos == (mc->end - 8) now?
>>
>> Marcelo,
>>
>> The splitting has been done in emulator_read_write_onepage:
>>
>> while (bytes) {
>> unsigned now = min(bytes, 8U);
>>
>> frag = &vcpu->mmio_fragments[vcpu->mmio_nr_fragments++];
>> frag->gpa = gpa;
>> frag->data = val;
>> frag->len = now;
>> frag->write_readonly_mem = (ret == -EPERM);
>>
>> gpa += now;
>> val += now;
>> bytes -= now;
>> }
>>
>> So i think it is safe to remove the splitting in read_emulated.
>
> Yes, it is fine to remove it.
>
> But splitting in emulate.c prevented the case of _cache read_ with size
>> 8 beyond end of mc->data. Must handle that case in read_emulated.
>
> "What prevents read_emulated(size > 8) call, with mc->pos == (mc->end - 8) now?"
You mean the mmio region is partly cached?
I think it can not happen. Now, we pass the whole size to emulator_read_write_onepage(),
after it is finished, it saves the whole data into mc->data[], so, the cache-read
can always get the whole data from mc->data[].
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 5/9] KVM: MMU: fask check write-protect for direct mmu
2012-07-20 11:09 ` Marcelo Tosatti
@ 2012-07-20 13:33 ` Xiao Guangrong
0 siblings, 0 replies; 25+ messages in thread
From: Xiao Guangrong @ 2012-07-20 13:33 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: Avi Kivity, LKML, KVM
On 07/20/2012 07:09 PM, Marcelo Tosatti wrote:
> On Fri, Jul 20, 2012 at 10:34:28AM +0800, Xiao Guangrong wrote:
>> On 07/20/2012 08:39 AM, Marcelo Tosatti wrote:
>>> On Tue, Jul 17, 2012 at 09:53:29PM +0800, Xiao Guangrong wrote:
>>>> If it have no indirect shadow pages we need not protect any gfn,
>>>> this is always true for direct mmu without nested
>>>>
>>>> Signed-off-by: Xiao Guangrong <xiaoguangrong@linux.vnet.ibm.com>
>>>
>>> Xiao,
>>>
>>> What is the motivation? Numbers please.
>>>
>>
>> mmu_need_write_protect is the common path for both soft-mmu and
>> hard-mmu, checking indirect_shadow_pages can skip hash-table walking
>> for the case which is tdp is enabled without nested guest.
>
> I mean motivation as observation that it is a bottleneck.
>
>> I will post the Number after I do the performance test.
>>
>>> In fact, what case was the original indirect_shadow_pages conditional in
>>> kvm_mmu_pte_write optimizing again?
>>>
>>
>> They are the different paths, mmu_need_write_protect is the real
>> page fault path, and kvm_mmu_pte_write is caused by mmio emulation.
>
> Sure. What i am asking is, what use case is the indirect_shadow_pages
> optimizing? What scenario, what workload?
>
Sorry, Marcelo, i do know why i completely misunderstood your mail. :(
I am not sure whether this is a bottleneck, i just got it from
code review, i will measure it to see if we can get benefit from
it. :p
> See the "When to optimize" section of
> http://en.wikipedia.org/wiki/Program_optimization.
>
> Can't remember why indirect_shadow_pages was introduced in
> kvm_mmu_pte_write.
>
Please refer to:
https://lkml.org/lkml/2011/5/18/174
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/9] KVM: x86: simplify read_emulated
2012-07-20 13:15 ` Xiao Guangrong
@ 2012-07-20 19:52 ` Marcelo Tosatti
2012-07-23 4:23 ` Xiao Guangrong
0 siblings, 1 reply; 25+ messages in thread
From: Marcelo Tosatti @ 2012-07-20 19:52 UTC (permalink / raw)
To: Xiao Guangrong; +Cc: Avi Kivity, LKML, KVM
On Fri, Jul 20, 2012 at 09:15:44PM +0800, Xiao Guangrong wrote:
> On 07/20/2012 06:58 PM, Marcelo Tosatti wrote:
> > On Fri, Jul 20, 2012 at 10:17:36AM +0800, Xiao Guangrong wrote:
> >> On 07/20/2012 07:58 AM, Marcelo Tosatti wrote:
> >>
> >>>> - }
> >>>> + rc = ctxt->ops->read_emulated(ctxt, addr, mc->data + mc->end, size,
> >>>> + &ctxt->exception);
> >>>> + if (rc != X86EMUL_CONTINUE)
> >>>> + return rc;
> >>>> +
> >>>> + mc->end += size;
> >>>> +
> >>>> +read_cached:
> >>>> + memcpy(dest, mc->data + mc->pos, size);
> >>>
> >>> What prevents read_emulated(size > 8) call, with
> >>> mc->pos == (mc->end - 8) now?
> >>
> >> Marcelo,
> >>
> >> The splitting has been done in emulator_read_write_onepage:
> >>
> >> while (bytes) {
> >> unsigned now = min(bytes, 8U);
> >>
> >> frag = &vcpu->mmio_fragments[vcpu->mmio_nr_fragments++];
> >> frag->gpa = gpa;
> >> frag->data = val;
> >> frag->len = now;
> >> frag->write_readonly_mem = (ret == -EPERM);
> >>
> >> gpa += now;
> >> val += now;
> >> bytes -= now;
> >> }
> >>
> >> So i think it is safe to remove the splitting in read_emulated.
> >
> > Yes, it is fine to remove it.
> >
> > But splitting in emulate.c prevented the case of _cache read_ with size
> >> 8 beyond end of mc->data. Must handle that case in read_emulated.
> >
> > "What prevents read_emulated(size > 8) call, with mc->pos == (mc->end - 8) now?"
>
> You mean the mmio region is partly cached?
>
> I think it can not happen. Now, we pass the whole size to emulator_read_write_onepage(),
> after it is finished, it saves the whole data into mc->data[], so, the cache-read
> can always get the whole data from mc->data[].
I mean that nothing prevents a caller from reading beyond the end of
mc->data array (but then again this was the previous behavior).
ACK
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH 2/9] KVM: x86: simplify read_emulated
2012-07-20 19:52 ` Marcelo Tosatti
@ 2012-07-23 4:23 ` Xiao Guangrong
0 siblings, 0 replies; 25+ messages in thread
From: Xiao Guangrong @ 2012-07-23 4:23 UTC (permalink / raw)
To: Marcelo Tosatti; +Cc: Avi Kivity, LKML, KVM
On 07/21/2012 03:52 AM, Marcelo Tosatti wrote:
> On Fri, Jul 20, 2012 at 09:15:44PM +0800, Xiao Guangrong wrote:
>> On 07/20/2012 06:58 PM, Marcelo Tosatti wrote:
>>> On Fri, Jul 20, 2012 at 10:17:36AM +0800, Xiao Guangrong wrote:
>>>> On 07/20/2012 07:58 AM, Marcelo Tosatti wrote:
>>>>
>>>>>> - }
>>>>>> + rc = ctxt->ops->read_emulated(ctxt, addr, mc->data + mc->end, size,
>>>>>> + &ctxt->exception);
>>>>>> + if (rc != X86EMUL_CONTINUE)
>>>>>> + return rc;
>>>>>> +
>>>>>> + mc->end += size;
>>>>>> +
>>>>>> +read_cached:
>>>>>> + memcpy(dest, mc->data + mc->pos, size);
>>>>>
>>>>> What prevents read_emulated(size > 8) call, with
>>>>> mc->pos == (mc->end - 8) now?
>>>>
>>>> Marcelo,
>>>>
>>>> The splitting has been done in emulator_read_write_onepage:
>>>>
>>>> while (bytes) {
>>>> unsigned now = min(bytes, 8U);
>>>>
>>>> frag = &vcpu->mmio_fragments[vcpu->mmio_nr_fragments++];
>>>> frag->gpa = gpa;
>>>> frag->data = val;
>>>> frag->len = now;
>>>> frag->write_readonly_mem = (ret == -EPERM);
>>>>
>>>> gpa += now;
>>>> val += now;
>>>> bytes -= now;
>>>> }
>>>>
>>>> So i think it is safe to remove the splitting in read_emulated.
>>>
>>> Yes, it is fine to remove it.
>>>
>>> But splitting in emulate.c prevented the case of _cache read_ with size
>>>> 8 beyond end of mc->data. Must handle that case in read_emulated.
>>>
>>> "What prevents read_emulated(size > 8) call, with mc->pos == (mc->end - 8) now?"
>>
>> You mean the mmio region is partly cached?
>>
>> I think it can not happen. Now, we pass the whole size to emulator_read_write_onepage(),
>> after it is finished, it saves the whole data into mc->data[], so, the cache-read
>> can always get the whole data from mc->data[].
>
> I mean that nothing prevents a caller from reading beyond the end of
> mc->data array (but then again this was the previous behavior).
1024 bytes should be enough for instructions, may be we can add a WARN_ON
to check buffer-overflow.
>
> ACK
>
Thank you, Marcelo!
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2012-07-23 4:23 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-17 13:50 [PATCH 1/9] KVM: x86: remvoe unnecessary mark_page_dirty Xiao Guangrong
2012-07-17 13:51 ` [PATCH 2/9] KVM: x86: simplify read_emulated Xiao Guangrong
2012-07-19 23:58 ` Marcelo Tosatti
2012-07-20 2:17 ` Xiao Guangrong
2012-07-20 10:58 ` Marcelo Tosatti
2012-07-20 13:15 ` Xiao Guangrong
2012-07-20 19:52 ` Marcelo Tosatti
2012-07-23 4:23 ` Xiao Guangrong
2012-07-17 13:52 ` [PATCH 3/9] KVM: x86: introduce set_mmio_exit_info Xiao Guangrong
2012-07-20 0:03 ` Marcelo Tosatti
2012-07-20 2:24 ` Xiao Guangrong
2012-07-17 13:52 ` [PATCH 4/9] KVM: MMU: track the refcount when unmap the page Xiao Guangrong
2012-07-20 0:09 ` Marcelo Tosatti
2012-07-17 13:53 ` [PATCH 5/9] KVM: MMU: fask check write-protect for direct mmu Xiao Guangrong
2012-07-20 0:39 ` Marcelo Tosatti
2012-07-20 2:34 ` Xiao Guangrong
2012-07-20 11:09 ` Marcelo Tosatti
2012-07-20 13:33 ` Xiao Guangrong
2012-07-20 3:45 ` Xiao Guangrong
2012-07-20 11:45 ` Marcelo Tosatti
2012-07-17 13:54 ` [PATCH 6/9] KVM: using get_fault_pfn to get the fault pfn Xiao Guangrong
2012-07-17 13:54 ` [PATCH 7/9] KVM: mark do not extern bad_pfn Xiao Guangrong
2012-07-17 13:55 ` [PATCH 8/9] KVM: remove is_error_hpa Xiao Guangrong
2012-07-17 13:56 ` [PATCH 9/9] KVM: remove the unused parameter of gfn_to_pfn_memslot Xiao Guangrong
2012-07-20 0:40 ` [PATCH 1/9] KVM: x86: remvoe unnecessary mark_page_dirty Marcelo Tosatti
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).