Re: [PATCH 4/9] substitute is_allocated_mem with more general is_containing_region
Glauber Costa wrote: On Sat, Sep 20, 2008 at 11:33:56AM -0700, Avi Kivity wrote: Glauber Costa wrote: is_allocated_mem is a function that checks if every relevant aspect of the memory slot match (start and size). Replace it with a more generic function that checks if a memory region is totally contained into another. The former case is also covered. I think enabling dirty page tracking requires the slot to match exactly. The registering function was the only caller for that. As dirty tracking was happening _inside_ it, so we're not really losing anything here. That said, I believe in the future, it will. (would be a logical next step, and start addressing the problem that Jan raised). Maybe we can keep the function that checks for exact matching, but another alternative is to pass a phys_addr, and turn dirty tracking on/off for whatever slot that contains that phys_addr. What do you think? Exact matching would avoid surprises (catch errors early). -- error compiling committee.c: too many arguments to function -- To unsubscribe from this list: send the line unsubscribe kvm in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/9] substitute is_allocated_mem with more general is_containing_region
On Sat, Sep 20, 2008 at 11:33:56AM -0700, Avi Kivity wrote: Glauber Costa wrote: is_allocated_mem is a function that checks if every relevant aspect of the memory slot match (start and size). Replace it with a more generic function that checks if a memory region is totally contained into another. The former case is also covered. I think enabling dirty page tracking requires the slot to match exactly. The registering function was the only caller for that. As dirty tracking was happening _inside_ it, so we're not really losing anything here. That said, I believe in the future, it will. (would be a logical next step, and start addressing the problem that Jan raised). Maybe we can keep the function that checks for exact matching, but another alternative is to pass a phys_addr, and turn dirty tracking on/off for whatever slot that contains that phys_addr. What do you think? -- 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 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 4/9] substitute is_allocated_mem with more general is_containing_region
Glauber Costa wrote: is_allocated_mem is a function that checks if every relevant aspect of the memory slot match (start and size). Replace it with a more generic function that checks if a memory region is totally contained into another. The former case is also covered. I think enabling dirty page tracking requires the slot to match exactly. -- 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 in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/9] substitute is_allocated_mem with more general is_containing_region
is_allocated_mem is a function that checks if every relevant aspect of the
memory slot
match (start and size). Replace it with a more generic function that checks if
a memory
region is totally contained into another. The former case is also covered.
Signed-off-by: Glauber Costa [EMAIL PROTECTED]
---
libkvm/libkvm.c | 34 +-
libkvm/libkvm.h |2 +-
qemu/qemu-kvm.c |4 ++--
3 files changed, 24 insertions(+), 16 deletions(-)
diff --git a/libkvm/libkvm.c b/libkvm/libkvm.c
index fa65c30..a5e20bb 100644
--- a/libkvm/libkvm.c
+++ b/libkvm/libkvm.c
@@ -136,6 +136,27 @@ int get_intersecting_slot(unsigned long phys_addr)
return -1;
}
+/* Returns -1 if this slot is not totally contained on any other,
+ * and the number of the slot otherwise */
+int get_container_slot(uint64_t phys_addr, unsigned long size)
+{
+ int i;
+
+ for (i = 0; i KVM_MAX_NUM_MEM_REGIONS ; ++i)
+ if (slots[i].len slots[i].phys_addr = phys_addr
+ (slots[i].phys_addr + slots[i].len) = phys_addr + size)
+ return i;
+ return -1;
+}
+
+int kvm_is_containing_region(kvm_context_t kvm, unsigned long phys_addr,
unsigned long size)
+{
+ int slot = get_container_slot(phys_addr, size);
+ if (slot == -1)
+ return 0;
+ return 1;
+}
+
/*
* dirty pages logging control
*/
@@ -423,19 +444,6 @@ int kvm_is_intersecting_mem(kvm_context_t kvm, unsigned
long phys_start)
return get_intersecting_slot(phys_start) != -1;
}
-int kvm_is_allocated_mem(kvm_context_t kvm, unsigned long phys_start,
-unsigned long len)
-{
- int slot;
-
- slot = get_slot(phys_start);
- if (slot == -1)
- return 0;
- if (slots[slot].len == len)
- return 1;
- return 0;
-}
-
int kvm_register_phys_mem(kvm_context_t kvm,
unsigned long phys_start, void *userspace_addr,
unsigned long len, int log)
diff --git a/libkvm/libkvm.h b/libkvm/libkvm.h
index 79dd769..1e89993 100644
--- a/libkvm/libkvm.h
+++ b/libkvm/libkvm.h
@@ -454,7 +454,7 @@ void *kvm_create_phys_mem(kvm_context_t, unsigned long
phys_start,
unsigned long len, int log, int writable);
void kvm_destroy_phys_mem(kvm_context_t, unsigned long phys_start,
unsigned long len);
-int kvm_is_intersecting_mem(kvm_context_t kvm, unsigned long phys_start);
+int kvm_is_containing_region(kvm_context_t kvm, unsigned long phys_start,
unsigned long size);
int kvm_is_allocated_mem(kvm_context_t kvm, unsigned long phys_start,
unsigned long len);
int kvm_create_mem_hole(kvm_context_t kvm, unsigned long phys_start,
diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
index cff04c5..e0b114a 100644
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -778,10 +778,10 @@ void kvm_cpu_register_physical_memory(target_phys_addr_t
start_addr,
int r = 0;
phys_offset = ~IO_MEM_ROM;
-r = kvm_is_allocated_mem(kvm_context, start_addr, size);
+r = kvm_is_containing_region(kvm_context, start_addr, size);
if (r)
return;
-r = kvm_is_intersecting_mem(kvm_context, start_addr);
+r = kvm_is_intersecting_mem(kvm_context, start_addr);
if (r) {
printf(Ignoring intersecting memory %llx (%lx)\n, start_addr, size);
} else
--
1.5.5.1
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/9] substitute is_allocated_mem with more general is_containing_region
is_allocated_mem is a function that checks if every relevant aspect of the
memory slot
match (start and size). Replace it with a more generic function that checks if
a memory
region is totally contained into another. The former case is also covered.
Signed-off-by: Glauber Costa [EMAIL PROTECTED]
---
libkvm/libkvm.c | 34 +-
libkvm/libkvm.h |2 +-
qemu/qemu-kvm.c |4 ++--
3 files changed, 24 insertions(+), 16 deletions(-)
diff --git a/libkvm/libkvm.c b/libkvm/libkvm.c
index fa65c30..a5e20bb 100644
--- a/libkvm/libkvm.c
+++ b/libkvm/libkvm.c
@@ -136,6 +136,27 @@ int get_intersecting_slot(unsigned long phys_addr)
return -1;
}
+/* Returns -1 if this slot is not totally contained on any other,
+ * and the number of the slot otherwise */
+int get_container_slot(uint64_t phys_addr, unsigned long size)
+{
+ int i;
+
+ for (i = 0; i KVM_MAX_NUM_MEM_REGIONS ; ++i)
+ if (slots[i].len slots[i].phys_addr = phys_addr
+ (slots[i].phys_addr + slots[i].len) = phys_addr + size)
+ return i;
+ return -1;
+}
+
+int kvm_is_containing_region(kvm_context_t kvm, unsigned long phys_addr,
unsigned long size)
+{
+ int slot = get_container_slot(phys_addr, size);
+ if (slot == -1)
+ return 0;
+ return 1;
+}
+
/*
* dirty pages logging control
*/
@@ -423,19 +444,6 @@ int kvm_is_intersecting_mem(kvm_context_t kvm, unsigned
long phys_start)
return get_intersecting_slot(phys_start) != -1;
}
-int kvm_is_allocated_mem(kvm_context_t kvm, unsigned long phys_start,
-unsigned long len)
-{
- int slot;
-
- slot = get_slot(phys_start);
- if (slot == -1)
- return 0;
- if (slots[slot].len == len)
- return 1;
- return 0;
-}
-
int kvm_register_phys_mem(kvm_context_t kvm,
unsigned long phys_start, void *userspace_addr,
unsigned long len, int log)
diff --git a/libkvm/libkvm.h b/libkvm/libkvm.h
index 79dd769..1e89993 100644
--- a/libkvm/libkvm.h
+++ b/libkvm/libkvm.h
@@ -454,7 +454,7 @@ void *kvm_create_phys_mem(kvm_context_t, unsigned long
phys_start,
unsigned long len, int log, int writable);
void kvm_destroy_phys_mem(kvm_context_t, unsigned long phys_start,
unsigned long len);
-int kvm_is_intersecting_mem(kvm_context_t kvm, unsigned long phys_start);
+int kvm_is_containing_region(kvm_context_t kvm, unsigned long phys_start,
unsigned long size);
int kvm_is_allocated_mem(kvm_context_t kvm, unsigned long phys_start,
unsigned long len);
int kvm_create_mem_hole(kvm_context_t kvm, unsigned long phys_start,
diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
index cff04c5..e0b114a 100644
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -778,10 +778,10 @@ void kvm_cpu_register_physical_memory(target_phys_addr_t
start_addr,
int r = 0;
phys_offset = ~IO_MEM_ROM;
-r = kvm_is_allocated_mem(kvm_context, start_addr, size);
+r = kvm_is_containing_region(kvm_context, start_addr, size);
if (r)
return;
-r = kvm_is_intersecting_mem(kvm_context, start_addr);
+r = kvm_is_intersecting_mem(kvm_context, start_addr);
if (r) {
printf(Ignoring intersecting memory %llx (%lx)\n, start_addr, size);
} else
--
1.5.5.1
--
To unsubscribe from this list: send the line unsubscribe kvm in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
