Re: qemu-kvm.git now live
Hollis Blanchard wrote: On Thu, 2009-04-23 at 18:40 +0300, Avi Kivity wrote: Still missing: - I have not tested powerpc or ia64. Patches welcome! This repository seems pretty messed up; I'm not even sure what you want from me. Some of the bigger problems: * configure completely ignores --kerneldir and only uses kvm/kernel headers instead. That's intentional. * The headers in kvm/kernel/arch/foo seem to be the important ones, but they have odd ifdefs at the top and I'm not sure how they should be generated. They were generated by the old 'make sync' to remove CONFIG_ dependencies. I guess a better way to generate them is a 'make headers-install' from the kernel tree and grab the results. * make -C kvm/kernel sync doesn't even come close to working. One of the goals of qemu-kvm.git was to have a standalone repository so I could release kvm-kmod and qemu-kvm independently. There are a host of other issues and confusing issues, even with a fresh checkout. In general it seems like the mess that was kvm-userspace has been rearranged, and as a consequence is even worse than before. Can you detail specific grievances? Please exclude temporary brokenness that was the result of the change. How am I supposed to create kvm/kernel/arch/powerpc/include/asm/*? Just cp from a Linux tree, or must I add funny ifdefs somehow? cp from a Linux tree won't compile. The results of the old 'make sync' would do, but a 'make headers-install' is probably better. Here's one patch that I think will be needed: Set kvm_arch=powerpc for PPC builds. The name of the Linux arch directory is powerpc, not ppc. Signed-off-by: Hollis Blanchard holl...@us.ibm.com diff --git a/configure b/configure index fc0fb9b..257cf02 100755 --- a/configure +++ b/configure @@ -816,6 +816,9 @@ case $cpu in i386 | x86_64) kvm_arch=x86 ;; +ppc) + kvm_arch=powerpc + ;; *) kvm_arch=$cpu ;; Applied, thanks. -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain. -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: qemu-kvm.git now live
Hollis Blanchard wrote: Since PPC is now supported in upstream QEMU, does it really matter if it works in qemu-kvm.git? I was going to take that position too, except Avi asked me specifically if some of the code inside TARGET_I386 ifdefs in qemu-kvm.c works for other architectures. In that case it's sufficient to have the build system use the upstream kvm integration (CONFIG_KVM) rather than the qemu-kvm integration (USE_KVM). -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain. -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 4/7] kvm: Add sanity checks to slot management
Liu Yu-B13201 wrote: -Original Message- From: qemu-devel-bounces+yu.liu=freescale@nongnu.org [mailto:qemu-devel-bounces+yu.liu=freescale@nongnu.org] On Behalf Of Jan Kiszka Sent: Sunday, April 12, 2009 1:20 AM To: qemu-de...@nongnu.org Subject: [Qemu-devel] [PATCH 4/7] kvm: Add sanity checks to slot management Fail loudly if we run out of memory slot. Make sure that dirty log start/stop works with consistent memory regions by reporting invalid parameters. This reveals several inconsistencies in the vga code, patch to fix them follows later in this series. And, for simplicity reasons, also catch and report unaligned memory regions passed to kvm_set_phys_mem (KVM works on page basis). Commit d3f8d37fe2d0c24ec8bac9c94d5b0e2dc09c0d2a hurts kvm/powerpc The alignment check in kvm_set_phys_mem prevents pci controller and mpic initializing mmio regions. What is the alignment of those regions then? None? And do regions of different types overlap even on the same page? Maybe the check reveals some deeper conflict /wrt KVM. Can you point me to the involved code files? Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: qemu-kvm.git now live
Avi Kivity wrote: Jan Kiszka wrote: Avi Kivity wrote: After a lengthy testing phase, qemu-kvm.git has replaced kvm-userspace.git as the source repository for kvm userspace development. Differences from kvm-userspace.git are as follows: - everything under qemu/ has been moved to the top-level directory - everything not under qemu/ has been moved under a new top-level directory, kvm/ - all qemu subversion commits have been rewritten to be compatible with what will become the master qemu git repository - all branches and tags have been converted - qemu-kvm.git builds standalone (does not require kvm.git) The repository is compatible with upstream qemu.git; merges and cherry-picking will work fine in either direction Still missing: - I have not tested powerpc or ia64. Patches welcome! - testsuite (kvm/user/ directory) does not build - proper guest memory setup During merge with upstream, the call to qemu_alloc_physram dropped under the table (due to removal of phys_ram_base from qemu). And that means we no longer call kvm_setup_guest_memory (fork will cause troubles, deja vu...) and -mempath is broken. Sorry, I'm not deep enough into all parts to provide a quick-fix. That's unrelated to the repository conversion. Then it's a merge artifact, but it's a problematic regression compared to still-working kvm-userspace. I'm unhappy with both qemu.git and qemu-kvm.git memory slot management; qemu-kvm.git is clumsy, and qemu.git is too simplistic (for example, it ignores the fact that dirty logging is a global resource with multiple users). Don't get your point yet. Can you name a concrete scenario that is problematic in upstream? I'd really like to get slot management right there before we drop the old version of qemu-kvm. Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 4/7] kvm: Add sanity checks to slot management
Liu Yu-B13201 wrote: -Original Message- From: Jan Kiszka [mailto:jan.kis...@siemens.com] Sent: Wednesday, April 29, 2009 6:38 PM To: Liu Yu-B13201 Cc: qemu-de...@nongnu.org; kvm-ppc@vger.kernel.org Subject: Re: [Qemu-devel] [PATCH 4/7] kvm: Add sanity checks to slot management Liu Yu-B13201 wrote: -Original Message- From: qemu-devel-bounces+yu.liu=freescale@nongnu.org [mailto:qemu-devel-bounces+yu.liu=freescale@nongnu.org] On Behalf Of Jan Kiszka Sent: Sunday, April 12, 2009 1:20 AM To: qemu-de...@nongnu.org Subject: [Qemu-devel] [PATCH 4/7] kvm: Add sanity checks to slot management Fail loudly if we run out of memory slot. Make sure that dirty log start/stop works with consistent memory regions by reporting invalid parameters. This reveals several inconsistencies in the vga code, patch to fix them follows later in this series. And, for simplicity reasons, also catch and report unaligned memory regions passed to kvm_set_phys_mem (KVM works on page basis). Commit d3f8d37fe2d0c24ec8bac9c94d5b0e2dc09c0d2a hurts kvm/powerpc The alignment check in kvm_set_phys_mem prevents pci controller and mpic initializing mmio regions. What is the alignment of those regions then? None? And do regions of different types overlap even on the same page? I think so. Maybe the check reveals some deeper conflict /wrt KVM. Can you point me to the involved code files? hw/ppc4xx_pci.c ppc4xx_pci_init() hw/ppce500_pci.c ppce500_pci_init() hw/openpic.c mpic_init() hw/ppce500_mpc8544ds.cserial_mm_init() Hmm, too bad that I have no platform at hand to test this. Could you instrument kvm_set_phys_mem (on an older, working version), dumping its arguments and post this trace? TIA, Jan -- Siemens AG, Corporate Technology, CT SE 2 Corporate Competence Center Embedded Linux -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: qemu-kvm.git now live
Jan Kiszka wrote: If migration disables dirty memory logging, it must keep the vga logging enabled, and vice versa. So we need some notifier callbacks on slot changes so that all users can re-enable dirty logging after the update as required. Simpler to do reference counting. When a user enables logging for a memory range, the refcount for all slots containing that range gets bumped. When a user enables logging for all of memory, a global refcount is bumped. For a given slot, dirty logging is enabled if either the slot logging refcount or the global logging refcount is nonzero. That's sort of what's implemented in qemu-kvm.git. In qemu.git vga logging does not get disabled, which is really broken. It prevents optimizations like disabling logging when the screen is not displayed to a human. Where/how does the migration code disable dirty logging? Should be phase 3 of ram_save_live(). -- I have a truly marvellous patch that fixes the bug which this signature is too narrow to contain. -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/04] qemu-kvm: other archs should maintain memory mapping also.
Avi Kivity wrote:
Hollis, does this work for you?
If now, you can add a new define KVM_WANT_MAPPING or something, and
define it for I386 and IA64.
Hi,
This is the one implementing the KVM_WANT_MAPPING change.
Cheers,
Jes
Introduce KVM_WANT_MAPPING define to switch on mapping code, instead
of relying on TARGET_arch defines.
Signed-off-by: Jes Sorensen j...@sgi.com
---
qemu-kvm.c | 42 ++
target-i386/qemu-kvm-arch.h |2 ++
target-ia64/qemu-kvm-arch.h |2 ++
3 files changed, 30 insertions(+), 16 deletions(-)
Index: qemu-kvm/qemu-kvm.c
===
--- qemu-kvm.orig/qemu-kvm.c
+++ qemu-kvm/qemu-kvm.c
@@ -824,7 +824,7 @@
kvm_finalize(kvm_context);
}
-#ifdef TARGET_I386
+#ifdef KVM_WANT_MAPPING
static struct mapping {
target_phys_addr_t phys;
ram_addr_t ram;
@@ -863,6 +863,26 @@
if (p)
*p = mappings[--nr_mappings];
}
+
+static inline void kvm_cpu_drop_mappings(target_phys_addr_t start_addr,
+ unsigned long size)
+{
+struct mapping *p;
+
+while (size 0) {
+p = find_mapping(start_addr);
+if (p) {
+kvm_unregister_memory_area(kvm_context, p-phys, p-len);
+drop_mapping(p-phys);
+}
+start_addr += TARGET_PAGE_SIZE;
+if (size TARGET_PAGE_SIZE) {
+size -= TARGET_PAGE_SIZE;
+} else {
+size = 0;
+}
+}
+}
#endif
void kvm_cpu_register_physical_memory(target_phys_addr_t start_addr,
@@ -871,7 +891,7 @@
{
int r = 0;
unsigned long area_flags;
-#ifdef TARGET_I386
+#ifdef KVM_WANT_MAPPING
struct mapping *p;
#endif
@@ -890,19 +910,9 @@
if (kvm_arch_must_use_aliases_target(start_addr))
return;
-while (size 0) {
-p = find_mapping(start_addr);
-if (p) {
-kvm_unregister_memory_area(kvm_context, p-phys, p-len);
-drop_mapping(p-phys);
-}
-start_addr += TARGET_PAGE_SIZE;
-if (size TARGET_PAGE_SIZE) {
-size -= TARGET_PAGE_SIZE;
-} else {
-size = 0;
-}
-}
+#ifdef KVM_WANT_MAPPING
+kvm_cpu_drop_mappings(start_addr, size);
+#endif
return;
}
@@ -930,7 +940,7 @@
exit(1);
}
-#ifdef TARGET_I386
+#ifdef KVM_WANT_MAPPING
drop_mapping(start_addr);
p = mappings[nr_mappings++];
p-phys = start_addr;
Index: qemu-kvm/target-i386/qemu-kvm-arch.h
===
--- qemu-kvm.orig/target-i386/qemu-kvm-arch.h
+++ qemu-kvm/target-i386/qemu-kvm-arch.h
@@ -14,4 +14,6 @@
extern int kvm_arch_must_use_aliases_source(target_phys_addr_t addr);
extern int kvm_arch_must_use_aliases_target(target_phys_addr_t addr);
+#define KVM_WANT_MAPPING 1
+
#endif
Index: qemu-kvm/target-ia64/qemu-kvm-arch.h
===
--- qemu-kvm.orig/target-ia64/qemu-kvm-arch.h
+++ qemu-kvm/target-ia64/qemu-kvm-arch.h
@@ -19,4 +19,6 @@
return 0;
}
+#define KVM_WANT_MAPPING 1
+
#endif
Re: [Qemu-devel] [PATCH 4/7] kvm: Add sanity checks to slot management
On Wed, 2009-04-29 at 12:38 +0200, Jan Kiszka wrote: Liu Yu-B13201 wrote: -Original Message- From: qemu-devel-bounces+yu.liu=freescale@nongnu.org [mailto:qemu-devel-bounces+yu.liu=freescale@nongnu.org] On Behalf Of Jan Kiszka Sent: Sunday, April 12, 2009 1:20 AM To: qemu-de...@nongnu.org Subject: [Qemu-devel] [PATCH 4/7] kvm: Add sanity checks to slot management Fail loudly if we run out of memory slot. Make sure that dirty log start/stop works with consistent memory regions by reporting invalid parameters. This reveals several inconsistencies in the vga code, patch to fix them follows later in this series. And, for simplicity reasons, also catch and report unaligned memory regions passed to kvm_set_phys_mem (KVM works on page basis). Commit d3f8d37fe2d0c24ec8bac9c94d5b0e2dc09c0d2a hurts kvm/powerpc The alignment check in kvm_set_phys_mem prevents pci controller and mpic initializing mmio regions. What is the alignment of those regions then? None? And do regions of different types overlap even on the same page? Maybe the check reveals some deeper conflict /wrt KVM. Can you point me to the involved code files? These PCI controllers make separate calls to cpu_register_physical_memory() for separate callbacks. Reading ppce500_pci_init(), for example: 0xe0008000 - CFGADDR (4 bytes) 0xe0008004 - CFGDATA (4 bytes) 0xe0008c00 - other registers The loop in cpu_register_physical_memory_offset() handles subpage registration. However, kvm_set_phys_mem() is called outside that loop, so it gets the non-page-aligned addresses. -- Hollis Blanchard IBM Linux Technology Center -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 4/7] kvm: Add sanity checks to slot management
On Wed, 2009-04-29 at 19:30 +0200, Jan Kiszka wrote:
Hollis Blanchard wrote:
On Wed, 2009-04-29 at 12:38 +0200, Jan Kiszka wrote:
Liu Yu-B13201 wrote:
-Original Message-
From: qemu-devel-bounces+yu.liu=freescale@nongnu.org
[mailto:qemu-devel-bounces+yu.liu=freescale@nongnu.org]
On Behalf Of Jan Kiszka
Sent: Sunday, April 12, 2009 1:20 AM
To: qemu-de...@nongnu.org
Subject: [Qemu-devel] [PATCH 4/7] kvm: Add sanity checks to
slot management
Fail loudly if we run out of memory slot.
Make sure that dirty log start/stop works with consistent
memory regions
by reporting invalid parameters. This reveals several
inconsistencies in
the vga code, patch to fix them follows later in this series.
And, for simplicity reasons, also catch and report unaligned memory
regions passed to kvm_set_phys_mem (KVM works on page basis).
Commit d3f8d37fe2d0c24ec8bac9c94d5b0e2dc09c0d2a hurts kvm/powerpc
The alignment check in kvm_set_phys_mem prevents pci controller and mpic
initializing mmio regions.
What is the alignment of those regions then? None? And do regions of
different types overlap even on the same page? Maybe the check reveals
some deeper conflict /wrt KVM. Can you point me to the involved code files?
These PCI controllers make separate calls to
cpu_register_physical_memory() for separate callbacks. Reading
ppce500_pci_init(), for example:
0xe0008000 - CFGADDR (4 bytes)
0xe0008004 - CFGDATA (4 bytes)
0xe0008c00 - other registers
The loop in cpu_register_physical_memory_offset() handles subpage
registration. However, kvm_set_phys_mem() is called outside that loop,
so it gets the non-page-aligned addresses.
Half-blind shot:
diff --git a/kvm-all.c b/kvm-all.c
index 32cd636..c2c760e 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -583,6 +583,9 @@ void kvm_set_phys_mem(target_phys_addr_t start_addr,
int err;
if (start_addr ~TARGET_PAGE_MASK) {
+if (flags = IO_MEM_UNASSIGNED) {
+return;
+}
fprintf(stderr, Only page-aligned memory slots supported\n);
abort();
}
If it works, it likely needs a cleaner approach to handle all cases.
I don't understand the point. kvm_set_phys_mem() already works without
this new abort() check.
--
Hollis Blanchard
IBM Linux Technology Center
--
To unsubscribe from this list: send the line unsubscribe kvm-ppc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 4/7] kvm: Add sanity checks to slot management
Hollis Blanchard wrote: On Wed, 2009-04-29 at 12:38 +0200, Jan Kiszka wrote: What is the alignment of those regions then? None? And do regions of different types overlap even on the same page? Maybe the check reveals some deeper conflict /wrt KVM. Can you point me to the involved code files? These PCI controllers make separate calls to cpu_register_physical_memory() for separate callbacks. Reading ppce500_pci_init(), for example: 0xe0008000 - CFGADDR (4 bytes) 0xe0008004 - CFGDATA (4 bytes) 0xe0008c00 - other registers That's goofy. If the single device owns the entire region, it should region the entire region instead of relying on subpage functionality. If just requires a switch() on the address to dispatch to the appropriate functions. It should be easy enough to fix. Regards, Anthony Liguori -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH 4/7] kvm: Add sanity checks to slot management
On Wed, 2009-04-29 at 12:38 -0500, Anthony Liguori wrote: Hollis Blanchard wrote: On Wed, 2009-04-29 at 12:38 +0200, Jan Kiszka wrote: What is the alignment of those regions then? None? And do regions of different types overlap even on the same page? Maybe the check reveals some deeper conflict /wrt KVM. Can you point me to the involved code files? These PCI controllers make separate calls to cpu_register_physical_memory() for separate callbacks. Reading ppce500_pci_init(), for example: 0xe0008000 - CFGADDR (4 bytes) 0xe0008004 - CFGDATA (4 bytes) 0xe0008c00 - other registers That's goofy. If the single device owns the entire region, it should region the entire region instead of relying on subpage functionality. If just requires a switch() on the address to dispatch to the appropriate functions. It should be easy enough to fix. There are two cases that share this code path: 1) same driver registers multiple regions in the same page 2) different drivers register regions in the same page This is case 1, and as you say, we could add a switch statement to handle it. I did not look closely to see how many other callers fall into this category. However, are you suggesting that case 2 is also goofy and will never work with KVM? It works in qemu today. As long as case 2 works, case 1 will work too, so why change anything? -- Hollis Blanchard IBM Linux Technology Center -- To unsubscribe from this list: send the line unsubscribe kvm-ppc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
