Re: [PATCH 04/04] qemu-kvm: other archs should maintain memory mappingalso.
On Mon, 2009-05-04 at 11:25 +0200, Jes Sorensen wrote: > Avi Kivity wrote: > > Jes Sorensen wrote: > >> +int destroy_region_works = 0; > > > > Global name, prefix with kvm_. Does it actually need to be global? > > Gone, now local to qemu-kvm-x86.c. I moved the initializer into > kvm_arch_create_context() instead. > > > The header depends on target_phys_addr_t, so it must include whatever > > defines it. > > Added an #include "cpu-all.h" which defines it. > > > Missing other archs... > > > > Instead of duplicating this for every arch, you can have a #define that > > tells you if you want non-trivial arch definitions, and supply the > > trivial definitions in qemu-kvm.h. > > Done, I also added a PPC header file - which may or may not be wanted > at this point. You can just cut it out if you don't think it should be > added. I don't understand the code being moved, but I guess I don't want it, so your patch is fine with me. (Wtf are those magic addresses? And not a single comment?? Aren't we better than this?) -- 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-kvm.git now live
On Sat, 2009-05-02 at 10:52 +0300, Avi Kivity wrote: > Hollis Blanchard wrote: > >> 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). > >> > > > > OK, I give up... how is this supposed to work? Nobody ever sets > > CONFIG_KVM or KVM_UPSTREAM, but there are a couple tests for it. Glauber > > once sent a patch related to that, but I don't see how it helps. > > > > KVM_UPSTREAM is just a marker to let us know which parts of upstream > qemu/kvm integration conflict with qemu-kvm.git. OK, so where do I define KVM_UPSTREAM? Also, where do I define CONFIG_KVM? I would expect the configure script to do that, but apparently it does not. -- 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: [PATCH 04/04] qemu-kvm: other archs should maintain memory mapping also.
Avi Kivity wrote:
Jes Sorensen wrote:
+int destroy_region_works = 0;
Global name, prefix with kvm_. Does it actually need to be global?
Gone, now local to qemu-kvm-x86.c. I moved the initializer into
kvm_arch_create_context() instead.
The header depends on target_phys_addr_t, so it must include whatever
defines it.
Added an #include "cpu-all.h" which defines it.
Missing other archs...
Instead of duplicating this for every arch, you can have a #define that
tells you if you want non-trivial arch definitions, and supply the
trivial definitions in qemu-kvm.h.
Done, I also added a PPC header file - which may or may not be wanted
at this point. You can just cut it out if you don't think it should be
added.
What do you think of this version then?
Cheers,
Jes
Move must_use_aliases_{source,target} functions to qemu-kvm- and
introduce qemu-kvm-arch.h header files, making it possible to inline
functions that a noops on a given architecture.
This removes a lot of ugly #ifdef TARGET_I386 from qemu-kvm.c
Signed-off-by: Jes Sorensen
---
qemu-kvm-x86.c | 25 +++-
qemu-kvm.c | 53 +++-
qemu-kvm.h | 13 ++
target-i386/qemu-kvm-arch.h | 17 ++
target-ia64/qemu-kvm-arch.h | 14 +++
target-ppc/qemu-kvm-arch.h | 14 +++
6 files changed, 91 insertions(+), 45 deletions(-)
Index: qemu-kvm/qemu-kvm-x86.c
===
--- qemu-kvm.orig/qemu-kvm-x86.c
+++ qemu-kvm/qemu-kvm-x86.c
@@ -28,6 +28,8 @@
static int lm_capable_kernel;
+static int destroy_region_works = 0;
+
int kvm_qemu_create_memory_alias(uint64_t phys_start,
uint64_t len,
uint64_t target_phys)
@@ -57,7 +59,10 @@
for (i = 0; i < kvm_msr_list->nmsrs; ++i)
if (kvm_msr_list->indices[i] == MSR_STAR)
kvm_has_msr_star = 1;
- return 0;
+
+destroy_region_works = kvm_destroy_memory_region_works(kvm_context);
+
+return 0;
}
static void set_msr_entry(struct kvm_msr_entry *entry, uint32_t index,
@@ -856,3 +861,21 @@
struct ioperm_data *data = _data;
ioperm(data->start_port, data->num, data->turn_on);
}
+
+int kvm_arch_must_use_aliases_source(target_phys_addr_t addr)
+{
+if (destroy_region_works)
+return false;
+if (addr == 0xa || addr == 0xa8000)
+return true;
+return false;
+}
+
+int kvm_arch_must_use_aliases_target(target_phys_addr_t addr)
+{
+if (destroy_region_works)
+return false;
+if (addr >= 0xe000 && addr < 0x1ull)
+return true;
+return false;
+}
Index: qemu-kvm/qemu-kvm.c
===
--- qemu-kvm.orig/qemu-kvm.c
+++ qemu-kvm/qemu-kvm.c
@@ -767,10 +767,6 @@
return 0;
}
-#ifdef TARGET_I386
-static int destroy_region_works = 0;
-#endif
-
int kvm_qemu_create_context(void)
{
int r;
@@ -795,9 +791,6 @@
return -1;
}
}
-#ifdef TARGET_I386
-destroy_region_works = kvm_destroy_memory_region_works(kvm_context);
-#endif
if (kvm_irqchip && kvm_has_gsi_routing(kvm_context)) {
kvm_clear_gsi_routes(kvm_context);
@@ -829,24 +822,6 @@
}
#ifdef TARGET_I386
-static int must_use_aliases_source(target_phys_addr_t addr)
-{
-if (destroy_region_works)
-return false;
-if (addr == 0xa || addr == 0xa8000)
-return true;
-return false;
-}
-
-static int must_use_aliases_target(target_phys_addr_t addr)
-{
-if (destroy_region_works)
-return false;
-if (addr >= 0xe000 && addr < 0x1ull)
-return true;
-return false;
-}
-
static struct mapping {
target_phys_addr_t phys;
ram_addr_t ram;
@@ -905,14 +880,13 @@
area_flags = phys_offset & ~TARGET_PAGE_MASK;
if (area_flags != IO_MEM_RAM) {
-#ifdef TARGET_I386
-if (must_use_aliases_source(start_addr)) {
+if (kvm_arch_must_use_aliases_source(start_addr)) {
kvm_destroy_memory_alias(kvm_context, start_addr);
return;
}
-if (must_use_aliases_target(start_addr))
+if (kvm_arch_must_use_aliases_target(start_addr))
return;
-#endif
+
while (size > 0) {
p = find_mapping(start_addr);
if (p) {
@@ -936,8 +910,7 @@
if (area_flags >= TLB_MMIO)
return;
-#ifdef TARGET_I386
-if (must_use_aliases_source(start_addr)) {
+if (kvm_arch_must_use_aliases_source(start_addr)) {
p = find_ram_mapping(phys_offset);
if (p) {
kvm_create_memory_alias(kvm_context, start_addr, size,
@@ -945,7 +918,6 @@
}
return;
}
-#endif
r = kvm_register_phys_mem(kvm_context, start_addr,
qemu_get_ram_ptr(phys_offset),
@@ -1256,10 +1228,9 @@
if (log)
kvm_dirty_pages_lo
Re: [PATCH 04/04] qemu-kvm: other archs should maintain memory mapping also.
Avi Kivity wrote: Avi Kivity wrote: This is the one implementing the KVM_WANT_MAPPING change. There is in fact a call to drop_mapping() outside any #ifdef (in kvm_cpu_register_physical_memory()). I'm confused... maybe we should make this code unconditional. Hi Avi, I don't follow this - if you apply my patch 0005-qemu-kvm-arch-mapping.patch this is no longer the case. Cheers, Jes -- 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: Jes Sorensen wrote: 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 There is in fact a call to drop_mapping() outside any #ifdef (in kvm_cpu_register_physical_memory()). I'm confused... maybe we should make this code unconditional. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. -- 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.
Jes Sorensen wrote:
Avi Kivity wrote:
Currently, use TARGET_I386 to comment out the mapping machanism
for other archs, but mapping machanism should be useful for other archs
to maintain guest's memory mapping.
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 must_use_alias patch mentioned in my previous email.
+int destroy_region_works = 0;
+
Global name, prefix with kvm_. Does it actually need to be global?
Index: qemu-kvm/target-i386/qemu-kvm-arch.h
===
--- /dev/null
+++ qemu-kvm/target-i386/qemu-kvm-arch.h
@@ -0,0 +1,17 @@
+/*
+ * qemu/kvm x86 integration
+ *
+ * Copyright (C) 2006-2008 Qumranet Technologies
+ * Copyright (C) 2009 Silicon Graphics Inc.
+ *
+ * Licensed under the terms of the GNU GPL version 2 or higher.
+ */
+#ifndef QEMU_KVM_ARCH_H
+#define QEMU_KVM_ARCH_H
+
+extern int destroy_region_works;
+
+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);
The header depends on target_phys_addr_t, so it must include whatever
defines it.
--- /dev/null
+++ qemu-kvm/target-ia64/qemu-kvm-arch.h
@@ -0,0 +1,22 @@
+/*
+ * qemu/kvm ia64 integration
+ *
+ * Copyright (C) 2006-2008 Qumranet Technologies
+ * Copyright (C) 2009 Silicon Graphics Inc.
+ *
+ * Licensed under the terms of the GNU GPL version 2 or higher.
+ */
+#ifndef QEMU_KVM_ARCH_H
+#define QEMU_KVM_ARCH_H
+
+static inline int kvm_arch_must_use_aliases_source(target_phys_addr_t addr)
+{
+return 0;
+}
+
+static inline int kvm_arch_must_use_aliases_target(target_phys_addr_t addr)
+{
+return 0;
+}
+
+#endif
Missing other archs...
Instead of duplicating this for every arch, you can have a #define that
tells you if you want non-trivial arch definitions, and supply the
trivial definitions in qemu-kvm.h.
--
Do not meddle in the internals of kernels, for they are subtle and quick to
panic.
--
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
