* [PATCH] KVM: fix the handling of dirty bitmaps to avoid overflows
@ 2010-04-12 10:35 ` Takuya Yoshikawa
0 siblings, 0 replies; 23+ messages in thread
From: Takuya Yoshikawa @ 2010-04-12 10:35 UTC (permalink / raw)
To: avi, mtosatti; +Cc: kvm, kvm-ia64, kvm-ppc
This patch fixes a bug found by Avi during the review process
of my dirty bitmap related work.
To ppc and ia64 people:
The fix is really simple but touches all architectures using
dirty bitmaps. So please check this will not suffer your part.
===
Int is not long enough to store the size of a dirty bitmap.
This patch fixes this problem with the introduction of a wrapper
function to calculate the sizes of dirty bitmaps.
Note: in mark_page_dirty(), we have to consider the fact that
__set_bit() takes the offset as int, not long.
Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
---
arch/ia64/kvm/kvm-ia64.c | 9 +++++----
arch/powerpc/kvm/book3s.c | 5 +++--
arch/x86/kvm/x86.c | 5 +++--
include/linux/kvm_host.h | 5 +++++
virt/kvm/kvm_main.c | 13 ++++++++-----
5 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
index b0ed80c..566aff6 100644
--- a/arch/ia64/kvm/kvm-ia64.c
+++ b/arch/ia64/kvm/kvm-ia64.c
@@ -1806,7 +1806,8 @@ static int kvm_ia64_sync_dirty_log(struct kvm *kvm,
{
struct kvm_memory_slot *memslot;
int r, i;
- long n, base;
+ long base;
+ unsigned long n;
unsigned long *dirty_bitmap = (unsigned long *)(kvm->arch.vm_base +
offsetof(struct kvm_vm_data, kvm_mem_dirty_log));
@@ -1819,7 +1820,7 @@ static int kvm_ia64_sync_dirty_log(struct kvm *kvm,
if (!memslot->dirty_bitmap)
goto out;
- n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
+ n = kvm_dirty_bitmap_bytes(memslot);
base = memslot->base_gfn / BITS_PER_LONG;
for (i = 0; i < n/sizeof(long); ++i) {
@@ -1835,7 +1836,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
struct kvm_dirty_log *log)
{
int r;
- int n;
+ unsigned long n;
struct kvm_memory_slot *memslot;
int is_dirty = 0;
@@ -1854,7 +1855,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
if (is_dirty) {
kvm_flush_remote_tlbs(kvm);
memslot = &kvm->memslots->memslots[log->slot];
- n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
+ n = kvm_dirty_bitmap_bytes(memslot);
memset(memslot->dirty_bitmap, 0, n);
}
r = 0;
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index a7ab2ea..4ac7b15 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -1118,7 +1118,8 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
struct kvm_vcpu *vcpu;
ulong ga, ga_end;
int is_dirty = 0;
- int r, n;
+ int r;
+ unsigned long n;
mutex_lock(&kvm->slots_lock);
@@ -1136,7 +1137,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
kvm_for_each_vcpu(n, vcpu, kvm)
kvmppc_mmu_pte_pflush(vcpu, ga, ga_end);
- n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
+ n = kvm_dirty_bitmap_bytes(memslot);
memset(memslot->dirty_bitmap, 0, n);
}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index fd5c3d3..bb36c3c 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2648,8 +2648,9 @@ static int kvm_vm_ioctl_reinject(struct kvm *kvm,
int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
struct kvm_dirty_log *log)
{
- int r, n, i;
+ int r, i;
struct kvm_memory_slot *memslot;
+ unsigned long n;
unsigned long is_dirty = 0;
unsigned long *dirty_bitmap = NULL;
@@ -2664,7 +2665,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
if (!memslot->dirty_bitmap)
goto out;
- n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
+ n = kvm_dirty_bitmap_bytes(memslot);
r = -ENOMEM;
dirty_bitmap = vmalloc(n);
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 8e91fa7..169d077 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -119,6 +119,11 @@ struct kvm_memory_slot {
int user_alloc;
};
+static inline unsigned long kvm_dirty_bitmap_bytes(struct kvm_memory_slot *memslot)
+{
+ return ALIGN(memslot->npages, BITS_PER_LONG) / 8;
+}
+
struct kvm_kernel_irq_routing_entry {
u32 gsi;
u32 type;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 9379533..6fe79c4 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -645,7 +645,7 @@ skip_lpage:
/* Allocate page dirty bitmap if needed */
if ((new.flags & KVM_MEM_LOG_DIRTY_PAGES) && !new.dirty_bitmap) {
- unsigned dirty_bytes = ALIGN(npages, BITS_PER_LONG) / 8;
+ unsigned long dirty_bytes = kvm_dirty_bitmap_bytes(&new);
new.dirty_bitmap = vmalloc(dirty_bytes);
if (!new.dirty_bitmap)
@@ -765,7 +765,7 @@ int kvm_get_dirty_log(struct kvm *kvm,
{
struct kvm_memory_slot *memslot;
int r, i;
- int n;
+ unsigned long n;
unsigned long any = 0;
r = -EINVAL;
@@ -777,7 +777,7 @@ int kvm_get_dirty_log(struct kvm *kvm,
if (!memslot->dirty_bitmap)
goto out;
- n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
+ n = kvm_dirty_bitmap_bytes(memslot);
for (i = 0; !any && i < n/sizeof(long); ++i)
any = memslot->dirty_bitmap[i];
@@ -1183,10 +1183,13 @@ void mark_page_dirty(struct kvm *kvm, gfn_t gfn)
memslot = gfn_to_memslot_unaliased(kvm, gfn);
if (memslot && memslot->dirty_bitmap) {
unsigned long rel_gfn = gfn - memslot->base_gfn;
+ unsigned long *p = memslot->dirty_bitmap +
+ rel_gfn / BITS_PER_LONG;
+ int offset = rel_gfn % BITS_PER_LONG;
/* avoid RMW */
- if (!generic_test_le_bit(rel_gfn, memslot->dirty_bitmap))
- generic___set_le_bit(rel_gfn, memslot->dirty_bitmap);
+ if (!generic_test_le_bit(offset, p))
+ generic___set_le_bit(offset, p);
}
}
--
1.6.3.3
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH] KVM: fix the handling of dirty bitmaps to avoid overflows
@ 2010-04-12 10:35 ` Takuya Yoshikawa
0 siblings, 0 replies; 23+ messages in thread
From: Takuya Yoshikawa @ 2010-04-12 10:35 UTC (permalink / raw)
To: avi, mtosatti; +Cc: kvm, kvm-ia64, kvm-ppc
This patch fixes a bug found by Avi during the review process
of my dirty bitmap related work.
To ppc and ia64 people:
The fix is really simple but touches all architectures using
dirty bitmaps. So please check this will not suffer your part.
=
Int is not long enough to store the size of a dirty bitmap.
This patch fixes this problem with the introduction of a wrapper
function to calculate the sizes of dirty bitmaps.
Note: in mark_page_dirty(), we have to consider the fact that
__set_bit() takes the offset as int, not long.
Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
---
arch/ia64/kvm/kvm-ia64.c | 9 +++++----
arch/powerpc/kvm/book3s.c | 5 +++--
arch/x86/kvm/x86.c | 5 +++--
include/linux/kvm_host.h | 5 +++++
virt/kvm/kvm_main.c | 13 ++++++++-----
5 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
index b0ed80c..566aff6 100644
--- a/arch/ia64/kvm/kvm-ia64.c
+++ b/arch/ia64/kvm/kvm-ia64.c
@@ -1806,7 +1806,8 @@ static int kvm_ia64_sync_dirty_log(struct kvm *kvm,
{
struct kvm_memory_slot *memslot;
int r, i;
- long n, base;
+ long base;
+ unsigned long n;
unsigned long *dirty_bitmap = (unsigned long *)(kvm->arch.vm_base +
offsetof(struct kvm_vm_data, kvm_mem_dirty_log));
@@ -1819,7 +1820,7 @@ static int kvm_ia64_sync_dirty_log(struct kvm *kvm,
if (!memslot->dirty_bitmap)
goto out;
- n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
+ n = kvm_dirty_bitmap_bytes(memslot);
base = memslot->base_gfn / BITS_PER_LONG;
for (i = 0; i < n/sizeof(long); ++i) {
@@ -1835,7 +1836,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
struct kvm_dirty_log *log)
{
int r;
- int n;
+ unsigned long n;
struct kvm_memory_slot *memslot;
int is_dirty = 0;
@@ -1854,7 +1855,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
if (is_dirty) {
kvm_flush_remote_tlbs(kvm);
memslot = &kvm->memslots->memslots[log->slot];
- n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
+ n = kvm_dirty_bitmap_bytes(memslot);
memset(memslot->dirty_bitmap, 0, n);
}
r = 0;
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index a7ab2ea..4ac7b15 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -1118,7 +1118,8 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
struct kvm_vcpu *vcpu;
ulong ga, ga_end;
int is_dirty = 0;
- int r, n;
+ int r;
+ unsigned long n;
mutex_lock(&kvm->slots_lock);
@@ -1136,7 +1137,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
kvm_for_each_vcpu(n, vcpu, kvm)
kvmppc_mmu_pte_pflush(vcpu, ga, ga_end);
- n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
+ n = kvm_dirty_bitmap_bytes(memslot);
memset(memslot->dirty_bitmap, 0, n);
}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index fd5c3d3..bb36c3c 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2648,8 +2648,9 @@ static int kvm_vm_ioctl_reinject(struct kvm *kvm,
int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
struct kvm_dirty_log *log)
{
- int r, n, i;
+ int r, i;
struct kvm_memory_slot *memslot;
+ unsigned long n;
unsigned long is_dirty = 0;
unsigned long *dirty_bitmap = NULL;
@@ -2664,7 +2665,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
if (!memslot->dirty_bitmap)
goto out;
- n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
+ n = kvm_dirty_bitmap_bytes(memslot);
r = -ENOMEM;
dirty_bitmap = vmalloc(n);
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 8e91fa7..169d077 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -119,6 +119,11 @@ struct kvm_memory_slot {
int user_alloc;
};
+static inline unsigned long kvm_dirty_bitmap_bytes(struct kvm_memory_slot *memslot)
+{
+ return ALIGN(memslot->npages, BITS_PER_LONG) / 8;
+}
+
struct kvm_kernel_irq_routing_entry {
u32 gsi;
u32 type;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 9379533..6fe79c4 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -645,7 +645,7 @@ skip_lpage:
/* Allocate page dirty bitmap if needed */
if ((new.flags & KVM_MEM_LOG_DIRTY_PAGES) && !new.dirty_bitmap) {
- unsigned dirty_bytes = ALIGN(npages, BITS_PER_LONG) / 8;
+ unsigned long dirty_bytes = kvm_dirty_bitmap_bytes(&new);
new.dirty_bitmap = vmalloc(dirty_bytes);
if (!new.dirty_bitmap)
@@ -765,7 +765,7 @@ int kvm_get_dirty_log(struct kvm *kvm,
{
struct kvm_memory_slot *memslot;
int r, i;
- int n;
+ unsigned long n;
unsigned long any = 0;
r = -EINVAL;
@@ -777,7 +777,7 @@ int kvm_get_dirty_log(struct kvm *kvm,
if (!memslot->dirty_bitmap)
goto out;
- n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
+ n = kvm_dirty_bitmap_bytes(memslot);
for (i = 0; !any && i < n/sizeof(long); ++i)
any = memslot->dirty_bitmap[i];
@@ -1183,10 +1183,13 @@ void mark_page_dirty(struct kvm *kvm, gfn_t gfn)
memslot = gfn_to_memslot_unaliased(kvm, gfn);
if (memslot && memslot->dirty_bitmap) {
unsigned long rel_gfn = gfn - memslot->base_gfn;
+ unsigned long *p = memslot->dirty_bitmap +
+ rel_gfn / BITS_PER_LONG;
+ int offset = rel_gfn % BITS_PER_LONG;
/* avoid RMW */
- if (!generic_test_le_bit(rel_gfn, memslot->dirty_bitmap))
- generic___set_le_bit(rel_gfn, memslot->dirty_bitmap);
+ if (!generic_test_le_bit(offset, p))
+ generic___set_le_bit(offset, p);
}
}
--
1.6.3.3
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH] KVM: fix the handling of dirty bitmaps to avoid overflows
@ 2010-04-12 10:35 ` Takuya Yoshikawa
0 siblings, 0 replies; 23+ messages in thread
From: Takuya Yoshikawa @ 2010-04-12 10:35 UTC (permalink / raw)
To: kvm-ia64
This patch fixes a bug found by Avi during the review process
of my dirty bitmap related work.
To ppc and ia64 people:
The fix is really simple but touches all architectures using
dirty bitmaps. So please check this will not suffer your part.
=
Int is not long enough to store the size of a dirty bitmap.
This patch fixes this problem with the introduction of a wrapper
function to calculate the sizes of dirty bitmaps.
Note: in mark_page_dirty(), we have to consider the fact that
__set_bit() takes the offset as int, not long.
Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
---
arch/ia64/kvm/kvm-ia64.c | 9 +++++----
arch/powerpc/kvm/book3s.c | 5 +++--
arch/x86/kvm/x86.c | 5 +++--
include/linux/kvm_host.h | 5 +++++
virt/kvm/kvm_main.c | 13 ++++++++-----
5 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
index b0ed80c..566aff6 100644
--- a/arch/ia64/kvm/kvm-ia64.c
+++ b/arch/ia64/kvm/kvm-ia64.c
@@ -1806,7 +1806,8 @@ static int kvm_ia64_sync_dirty_log(struct kvm *kvm,
{
struct kvm_memory_slot *memslot;
int r, i;
- long n, base;
+ long base;
+ unsigned long n;
unsigned long *dirty_bitmap = (unsigned long *)(kvm->arch.vm_base +
offsetof(struct kvm_vm_data, kvm_mem_dirty_log));
@@ -1819,7 +1820,7 @@ static int kvm_ia64_sync_dirty_log(struct kvm *kvm,
if (!memslot->dirty_bitmap)
goto out;
- n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
+ n = kvm_dirty_bitmap_bytes(memslot);
base = memslot->base_gfn / BITS_PER_LONG;
for (i = 0; i < n/sizeof(long); ++i) {
@@ -1835,7 +1836,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
struct kvm_dirty_log *log)
{
int r;
- int n;
+ unsigned long n;
struct kvm_memory_slot *memslot;
int is_dirty = 0;
@@ -1854,7 +1855,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
if (is_dirty) {
kvm_flush_remote_tlbs(kvm);
memslot = &kvm->memslots->memslots[log->slot];
- n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
+ n = kvm_dirty_bitmap_bytes(memslot);
memset(memslot->dirty_bitmap, 0, n);
}
r = 0;
diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
index a7ab2ea..4ac7b15 100644
--- a/arch/powerpc/kvm/book3s.c
+++ b/arch/powerpc/kvm/book3s.c
@@ -1118,7 +1118,8 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
struct kvm_vcpu *vcpu;
ulong ga, ga_end;
int is_dirty = 0;
- int r, n;
+ int r;
+ unsigned long n;
mutex_lock(&kvm->slots_lock);
@@ -1136,7 +1137,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
kvm_for_each_vcpu(n, vcpu, kvm)
kvmppc_mmu_pte_pflush(vcpu, ga, ga_end);
- n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
+ n = kvm_dirty_bitmap_bytes(memslot);
memset(memslot->dirty_bitmap, 0, n);
}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index fd5c3d3..bb36c3c 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2648,8 +2648,9 @@ static int kvm_vm_ioctl_reinject(struct kvm *kvm,
int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
struct kvm_dirty_log *log)
{
- int r, n, i;
+ int r, i;
struct kvm_memory_slot *memslot;
+ unsigned long n;
unsigned long is_dirty = 0;
unsigned long *dirty_bitmap = NULL;
@@ -2664,7 +2665,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
if (!memslot->dirty_bitmap)
goto out;
- n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
+ n = kvm_dirty_bitmap_bytes(memslot);
r = -ENOMEM;
dirty_bitmap = vmalloc(n);
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 8e91fa7..169d077 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -119,6 +119,11 @@ struct kvm_memory_slot {
int user_alloc;
};
+static inline unsigned long kvm_dirty_bitmap_bytes(struct kvm_memory_slot *memslot)
+{
+ return ALIGN(memslot->npages, BITS_PER_LONG) / 8;
+}
+
struct kvm_kernel_irq_routing_entry {
u32 gsi;
u32 type;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 9379533..6fe79c4 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -645,7 +645,7 @@ skip_lpage:
/* Allocate page dirty bitmap if needed */
if ((new.flags & KVM_MEM_LOG_DIRTY_PAGES) && !new.dirty_bitmap) {
- unsigned dirty_bytes = ALIGN(npages, BITS_PER_LONG) / 8;
+ unsigned long dirty_bytes = kvm_dirty_bitmap_bytes(&new);
new.dirty_bitmap = vmalloc(dirty_bytes);
if (!new.dirty_bitmap)
@@ -765,7 +765,7 @@ int kvm_get_dirty_log(struct kvm *kvm,
{
struct kvm_memory_slot *memslot;
int r, i;
- int n;
+ unsigned long n;
unsigned long any = 0;
r = -EINVAL;
@@ -777,7 +777,7 @@ int kvm_get_dirty_log(struct kvm *kvm,
if (!memslot->dirty_bitmap)
goto out;
- n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
+ n = kvm_dirty_bitmap_bytes(memslot);
for (i = 0; !any && i < n/sizeof(long); ++i)
any = memslot->dirty_bitmap[i];
@@ -1183,10 +1183,13 @@ void mark_page_dirty(struct kvm *kvm, gfn_t gfn)
memslot = gfn_to_memslot_unaliased(kvm, gfn);
if (memslot && memslot->dirty_bitmap) {
unsigned long rel_gfn = gfn - memslot->base_gfn;
+ unsigned long *p = memslot->dirty_bitmap +
+ rel_gfn / BITS_PER_LONG;
+ int offset = rel_gfn % BITS_PER_LONG;
/* avoid RMW */
- if (!generic_test_le_bit(rel_gfn, memslot->dirty_bitmap))
- generic___set_le_bit(rel_gfn, memslot->dirty_bitmap);
+ if (!generic_test_le_bit(offset, p))
+ generic___set_le_bit(offset, p);
}
}
--
1.6.3.3
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH] KVM: fix the handling of dirty bitmaps to avoid overflows
[not found] ` <20100412193535.6c502695.yoshikawa.takuya-gVGce1chcLdL9jVzuh4AOg@public.gmane.org>
2010-04-12 11:10 ` Alexander Graf
@ 2010-04-12 11:10 ` Alexander Graf
0 siblings, 0 replies; 23+ messages in thread
From: Alexander Graf @ 2010-04-12 11:10 UTC (permalink / raw)
To: Takuya Yoshikawa
Cc: avi-H+wXaHxf7aLQT0dZR+AlfA, mtosatti-H+wXaHxf7aLQT0dZR+AlfA,
kvm-u79uwXL29TY76Z2rM5mHXA, kvm-ia64-u79uwXL29TY76Z2rM5mHXA,
kvm-ppc-u79uwXL29TY76Z2rM5mHXA
On 12.04.2010, at 12:35, Takuya Yoshikawa wrote:
> This patch fixes a bug found by Avi during the review process
> of my dirty bitmap related work.
>
> To ppc and ia64 people:
> The fix is really simple but touches all architectures using
> dirty bitmaps. So please check this will not suffer your part.
>
> ===
>
> Int is not long enough to store the size of a dirty bitmap.
>
> This patch fixes this problem with the introduction of a wrapper
> function to calculate the sizes of dirty bitmaps.
>
> Note: in mark_page_dirty(), we have to consider the fact that
> __set_bit() takes the offset as int, not long.
>
> Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya-gVGce1chcLdL9jVzuh4AOg@public.gmane.org>
> ---
> arch/ia64/kvm/kvm-ia64.c | 9 +++++----
> arch/powerpc/kvm/book3s.c | 5 +++--
> arch/x86/kvm/x86.c | 5 +++--
> include/linux/kvm_host.h | 5 +++++
> virt/kvm/kvm_main.c | 13 ++++++++-----
> 5 files changed, 24 insertions(+), 13 deletions(-)
>
> diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
> index b0ed80c..566aff6 100644
> --- a/arch/ia64/kvm/kvm-ia64.c
> +++ b/arch/ia64/kvm/kvm-ia64.c
> @@ -1806,7 +1806,8 @@ static int kvm_ia64_sync_dirty_log(struct kvm *kvm,
> {
> struct kvm_memory_slot *memslot;
> int r, i;
> - long n, base;
> + long base;
> + unsigned long n;
> unsigned long *dirty_bitmap = (unsigned long *)(kvm->arch.vm_base +
> offsetof(struct kvm_vm_data, kvm_mem_dirty_log));
>
> @@ -1819,7 +1820,7 @@ static int kvm_ia64_sync_dirty_log(struct kvm *kvm,
> if (!memslot->dirty_bitmap)
> goto out;
>
> - n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
> + n = kvm_dirty_bitmap_bytes(memslot);
> base = memslot->base_gfn / BITS_PER_LONG;
>
> for (i = 0; i < n/sizeof(long); ++i) {
> @@ -1835,7 +1836,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
> struct kvm_dirty_log *log)
> {
> int r;
> - int n;
> + unsigned long n;
> struct kvm_memory_slot *memslot;
> int is_dirty = 0;
>
> @@ -1854,7 +1855,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
> if (is_dirty) {
> kvm_flush_remote_tlbs(kvm);
> memslot = &kvm->memslots->memslots[log->slot];
> - n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
> + n = kvm_dirty_bitmap_bytes(memslot);
> memset(memslot->dirty_bitmap, 0, n);
> }
> r = 0;
> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
> index a7ab2ea..4ac7b15 100644
> --- a/arch/powerpc/kvm/book3s.c
> +++ b/arch/powerpc/kvm/book3s.c
> @@ -1118,7 +1118,8 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
> struct kvm_vcpu *vcpu;
> ulong ga, ga_end;
> int is_dirty = 0;
> - int r, n;
> + int r;
> + unsigned long n;
>
> mutex_lock(&kvm->slots_lock);
>
> @@ -1136,7 +1137,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
> kvm_for_each_vcpu(n, vcpu, kvm)
> kvmppc_mmu_pte_pflush(vcpu, ga, ga_end);
>
> - n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
> + n = kvm_dirty_bitmap_bytes(memslot);
Looks like a pretty mechanical change, so no objections from me.
Alex
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] KVM: fix the handling of dirty bitmaps to avoid overflows
@ 2010-04-12 11:10 ` Alexander Graf
0 siblings, 0 replies; 23+ messages in thread
From: Alexander Graf @ 2010-04-12 11:10 UTC (permalink / raw)
To: Takuya Yoshikawa
Cc: avi-H+wXaHxf7aLQT0dZR+AlfA, mtosatti-H+wXaHxf7aLQT0dZR+AlfA,
kvm-u79uwXL29TY76Z2rM5mHXA, kvm-ia64-u79uwXL29TY76Z2rM5mHXA,
kvm-ppc-u79uwXL29TY76Z2rM5mHXA
On 12.04.2010, at 12:35, Takuya Yoshikawa wrote:
> This patch fixes a bug found by Avi during the review process
> of my dirty bitmap related work.
>
> To ppc and ia64 people:
> The fix is really simple but touches all architectures using
> dirty bitmaps. So please check this will not suffer your part.
>
> =>
> Int is not long enough to store the size of a dirty bitmap.
>
> This patch fixes this problem with the introduction of a wrapper
> function to calculate the sizes of dirty bitmaps.
>
> Note: in mark_page_dirty(), we have to consider the fact that
> __set_bit() takes the offset as int, not long.
>
> Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
> ---
> arch/ia64/kvm/kvm-ia64.c | 9 +++++----
> arch/powerpc/kvm/book3s.c | 5 +++--
> arch/x86/kvm/x86.c | 5 +++--
> include/linux/kvm_host.h | 5 +++++
> virt/kvm/kvm_main.c | 13 ++++++++-----
> 5 files changed, 24 insertions(+), 13 deletions(-)
>
> diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
> index b0ed80c..566aff6 100644
> --- a/arch/ia64/kvm/kvm-ia64.c
> +++ b/arch/ia64/kvm/kvm-ia64.c
> @@ -1806,7 +1806,8 @@ static int kvm_ia64_sync_dirty_log(struct kvm *kvm,
> {
> struct kvm_memory_slot *memslot;
> int r, i;
> - long n, base;
> + long base;
> + unsigned long n;
> unsigned long *dirty_bitmap = (unsigned long *)(kvm->arch.vm_base +
> offsetof(struct kvm_vm_data, kvm_mem_dirty_log));
>
> @@ -1819,7 +1820,7 @@ static int kvm_ia64_sync_dirty_log(struct kvm *kvm,
> if (!memslot->dirty_bitmap)
> goto out;
>
> - n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
> + n = kvm_dirty_bitmap_bytes(memslot);
> base = memslot->base_gfn / BITS_PER_LONG;
>
> for (i = 0; i < n/sizeof(long); ++i) {
> @@ -1835,7 +1836,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
> struct kvm_dirty_log *log)
> {
> int r;
> - int n;
> + unsigned long n;
> struct kvm_memory_slot *memslot;
> int is_dirty = 0;
>
> @@ -1854,7 +1855,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
> if (is_dirty) {
> kvm_flush_remote_tlbs(kvm);
> memslot = &kvm->memslots->memslots[log->slot];
> - n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
> + n = kvm_dirty_bitmap_bytes(memslot);
> memset(memslot->dirty_bitmap, 0, n);
> }
> r = 0;
> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
> index a7ab2ea..4ac7b15 100644
> --- a/arch/powerpc/kvm/book3s.c
> +++ b/arch/powerpc/kvm/book3s.c
> @@ -1118,7 +1118,8 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
> struct kvm_vcpu *vcpu;
> ulong ga, ga_end;
> int is_dirty = 0;
> - int r, n;
> + int r;
> + unsigned long n;
>
> mutex_lock(&kvm->slots_lock);
>
> @@ -1136,7 +1137,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
> kvm_for_each_vcpu(n, vcpu, kvm)
> kvmppc_mmu_pte_pflush(vcpu, ga, ga_end);
>
> - n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
> + n = kvm_dirty_bitmap_bytes(memslot);
Looks like a pretty mechanical change, so no objections from me.
Alex
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] KVM: fix the handling of dirty bitmaps to avoid overflows
@ 2010-04-12 11:10 ` Alexander Graf
0 siblings, 0 replies; 23+ messages in thread
From: Alexander Graf @ 2010-04-12 11:10 UTC (permalink / raw)
To: kvm-ia64
On 12.04.2010, at 12:35, Takuya Yoshikawa wrote:
> This patch fixes a bug found by Avi during the review process
> of my dirty bitmap related work.
>
> To ppc and ia64 people:
> The fix is really simple but touches all architectures using
> dirty bitmaps. So please check this will not suffer your part.
>
> =>
> Int is not long enough to store the size of a dirty bitmap.
>
> This patch fixes this problem with the introduction of a wrapper
> function to calculate the sizes of dirty bitmaps.
>
> Note: in mark_page_dirty(), we have to consider the fact that
> __set_bit() takes the offset as int, not long.
>
> Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
> ---
> arch/ia64/kvm/kvm-ia64.c | 9 +++++----
> arch/powerpc/kvm/book3s.c | 5 +++--
> arch/x86/kvm/x86.c | 5 +++--
> include/linux/kvm_host.h | 5 +++++
> virt/kvm/kvm_main.c | 13 ++++++++-----
> 5 files changed, 24 insertions(+), 13 deletions(-)
>
> diff --git a/arch/ia64/kvm/kvm-ia64.c b/arch/ia64/kvm/kvm-ia64.c
> index b0ed80c..566aff6 100644
> --- a/arch/ia64/kvm/kvm-ia64.c
> +++ b/arch/ia64/kvm/kvm-ia64.c
> @@ -1806,7 +1806,8 @@ static int kvm_ia64_sync_dirty_log(struct kvm *kvm,
> {
> struct kvm_memory_slot *memslot;
> int r, i;
> - long n, base;
> + long base;
> + unsigned long n;
> unsigned long *dirty_bitmap = (unsigned long *)(kvm->arch.vm_base +
> offsetof(struct kvm_vm_data, kvm_mem_dirty_log));
>
> @@ -1819,7 +1820,7 @@ static int kvm_ia64_sync_dirty_log(struct kvm *kvm,
> if (!memslot->dirty_bitmap)
> goto out;
>
> - n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
> + n = kvm_dirty_bitmap_bytes(memslot);
> base = memslot->base_gfn / BITS_PER_LONG;
>
> for (i = 0; i < n/sizeof(long); ++i) {
> @@ -1835,7 +1836,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
> struct kvm_dirty_log *log)
> {
> int r;
> - int n;
> + unsigned long n;
> struct kvm_memory_slot *memslot;
> int is_dirty = 0;
>
> @@ -1854,7 +1855,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
> if (is_dirty) {
> kvm_flush_remote_tlbs(kvm);
> memslot = &kvm->memslots->memslots[log->slot];
> - n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
> + n = kvm_dirty_bitmap_bytes(memslot);
> memset(memslot->dirty_bitmap, 0, n);
> }
> r = 0;
> diff --git a/arch/powerpc/kvm/book3s.c b/arch/powerpc/kvm/book3s.c
> index a7ab2ea..4ac7b15 100644
> --- a/arch/powerpc/kvm/book3s.c
> +++ b/arch/powerpc/kvm/book3s.c
> @@ -1118,7 +1118,8 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
> struct kvm_vcpu *vcpu;
> ulong ga, ga_end;
> int is_dirty = 0;
> - int r, n;
> + int r;
> + unsigned long n;
>
> mutex_lock(&kvm->slots_lock);
>
> @@ -1136,7 +1137,7 @@ int kvm_vm_ioctl_get_dirty_log(struct kvm *kvm,
> kvm_for_each_vcpu(n, vcpu, kvm)
> kvmppc_mmu_pte_pflush(vcpu, ga, ga_end);
>
> - n = ALIGN(memslot->npages, BITS_PER_LONG) / 8;
> + n = kvm_dirty_bitmap_bytes(memslot);
Looks like a pretty mechanical change, so no objections from me.
Alex
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] KVM: fix the handling of dirty bitmaps to avoid overflows
2010-04-12 10:35 ` Takuya Yoshikawa
(?)
@ 2010-04-12 17:39 ` Marcelo Tosatti
-1 siblings, 0 replies; 23+ messages in thread
From: Marcelo Tosatti @ 2010-04-12 17:39 UTC (permalink / raw)
To: Takuya Yoshikawa; +Cc: avi, kvm, kvm-ia64, kvm-ppc
On Mon, Apr 12, 2010 at 07:35:35PM +0900, Takuya Yoshikawa wrote:
> This patch fixes a bug found by Avi during the review process
> of my dirty bitmap related work.
>
> To ppc and ia64 people:
> The fix is really simple but touches all architectures using
> dirty bitmaps. So please check this will not suffer your part.
>
> ===
>
> Int is not long enough to store the size of a dirty bitmap.
>
> This patch fixes this problem with the introduction of a wrapper
> function to calculate the sizes of dirty bitmaps.
>
> Note: in mark_page_dirty(), we have to consider the fact that
> __set_bit() takes the offset as int, not long.
>
> Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
Applied, thanks.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] KVM: fix the handling of dirty bitmaps to avoid overflows
@ 2010-04-12 17:39 ` Marcelo Tosatti
0 siblings, 0 replies; 23+ messages in thread
From: Marcelo Tosatti @ 2010-04-12 17:39 UTC (permalink / raw)
To: Takuya Yoshikawa; +Cc: avi, kvm, kvm-ia64, kvm-ppc
On Mon, Apr 12, 2010 at 07:35:35PM +0900, Takuya Yoshikawa wrote:
> This patch fixes a bug found by Avi during the review process
> of my dirty bitmap related work.
>
> To ppc and ia64 people:
> The fix is really simple but touches all architectures using
> dirty bitmaps. So please check this will not suffer your part.
>
> =>
> Int is not long enough to store the size of a dirty bitmap.
>
> This patch fixes this problem with the introduction of a wrapper
> function to calculate the sizes of dirty bitmaps.
>
> Note: in mark_page_dirty(), we have to consider the fact that
> __set_bit() takes the offset as int, not long.
>
> Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
Applied, thanks.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] KVM: fix the handling of dirty bitmaps to avoid overflows
@ 2010-04-12 17:39 ` Marcelo Tosatti
0 siblings, 0 replies; 23+ messages in thread
From: Marcelo Tosatti @ 2010-04-12 17:39 UTC (permalink / raw)
To: kvm-ia64
On Mon, Apr 12, 2010 at 07:35:35PM +0900, Takuya Yoshikawa wrote:
> This patch fixes a bug found by Avi during the review process
> of my dirty bitmap related work.
>
> To ppc and ia64 people:
> The fix is really simple but touches all architectures using
> dirty bitmaps. So please check this will not suffer your part.
>
> =>
> Int is not long enough to store the size of a dirty bitmap.
>
> This patch fixes this problem with the introduction of a wrapper
> function to calculate the sizes of dirty bitmaps.
>
> Note: in mark_page_dirty(), we have to consider the fact that
> __set_bit() takes the offset as int, not long.
>
> Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
Applied, thanks.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] KVM: fix the handling of dirty bitmaps to avoid overflows
[not found] ` <20100412173951.GA5614-I4X2Mt4zSy4@public.gmane.org>
2010-04-13 0:52 ` Takuya Yoshikawa
@ 2010-04-13 0:52 ` Takuya Yoshikawa
0 siblings, 0 replies; 23+ messages in thread
From: Takuya Yoshikawa @ 2010-04-13 0:52 UTC (permalink / raw)
To: Marcelo Tosatti
Cc: avi-H+wXaHxf7aLQT0dZR+AlfA, kvm-u79uwXL29TY76Z2rM5mHXA,
kvm-ia64-u79uwXL29TY76Z2rM5mHXA, kvm-ppc-u79uwXL29TY76Z2rM5mHXA
(2010/04/13 2:39), Marcelo Tosatti wrote:
> On Mon, Apr 12, 2010 at 07:35:35PM +0900, Takuya Yoshikawa wrote:
>> This patch fixes a bug found by Avi during the review process
>> of my dirty bitmap related work.
>>
>> To ppc and ia64 people:
>> The fix is really simple but touches all architectures using
>> dirty bitmaps. So please check this will not suffer your part.
>>
>> ===
>>
>> Int is not long enough to store the size of a dirty bitmap.
>>
>> This patch fixes this problem with the introduction of a wrapper
>> function to calculate the sizes of dirty bitmaps.
>>
>> Note: in mark_page_dirty(), we have to consider the fact that
>> __set_bit() takes the offset as int, not long.
>>
>> Signed-off-by: Takuya Yoshikawa<yoshikawa.takuya-gVGce1chcLdL9jVzuh4AOg@public.gmane.org>
>
> Applied, thanks.
>
Thanks everyone!
BTW, just from my curiosity, are there any cases in which we use such huge
number of pages currently?
ALIGN(memslot->npages, BITS_PER_LONG) / 8;
More than G pages need really big memory!
-- We are assuming some special cases like "short" int size?
If so, we may have to care about a lot of things from now on, because common
functions like __set_bit() don't support such long buffers.
If not, my patch might be over hacking -- especially the following part:
@@ -1183,10 +1183,13 @@ void mark_page_dirty(struct kvm *kvm, gfn_t gfn)
memslot = gfn_to_memslot_unaliased(kvm, gfn);
if (memslot && memslot->dirty_bitmap) {
unsigned long rel_gfn = gfn - memslot->base_gfn;
+ unsigned long *p = memslot->dirty_bitmap +
+ rel_gfn / BITS_PER_LONG;
+ int offset = rel_gfn % BITS_PER_LONG;
/* avoid RMW */
- if (!generic_test_le_bit(rel_gfn, memslot->dirty_bitmap))
- generic___set_le_bit(rel_gfn, memslot->dirty_bitmap);
+ if (!generic_test_le_bit(offset, p))
+ generic___set_le_bit(offset, p);
}
}
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] KVM: fix the handling of dirty bitmaps to avoid overflows
@ 2010-04-13 0:52 ` Takuya Yoshikawa
0 siblings, 0 replies; 23+ messages in thread
From: Takuya Yoshikawa @ 2010-04-13 0:52 UTC (permalink / raw)
To: Marcelo Tosatti
Cc: avi-H+wXaHxf7aLQT0dZR+AlfA, kvm-u79uwXL29TY76Z2rM5mHXA,
kvm-ia64-u79uwXL29TY76Z2rM5mHXA, kvm-ppc-u79uwXL29TY76Z2rM5mHXA
(2010/04/13 2:39), Marcelo Tosatti wrote:
> On Mon, Apr 12, 2010 at 07:35:35PM +0900, Takuya Yoshikawa wrote:
>> This patch fixes a bug found by Avi during the review process
>> of my dirty bitmap related work.
>>
>> To ppc and ia64 people:
>> The fix is really simple but touches all architectures using
>> dirty bitmaps. So please check this will not suffer your part.
>>
>> =>>
>> Int is not long enough to store the size of a dirty bitmap.
>>
>> This patch fixes this problem with the introduction of a wrapper
>> function to calculate the sizes of dirty bitmaps.
>>
>> Note: in mark_page_dirty(), we have to consider the fact that
>> __set_bit() takes the offset as int, not long.
>>
>> Signed-off-by: Takuya Yoshikawa<yoshikawa.takuya@oss.ntt.co.jp>
>
> Applied, thanks.
>
Thanks everyone!
BTW, just from my curiosity, are there any cases in which we use such huge
number of pages currently?
ALIGN(memslot->npages, BITS_PER_LONG) / 8;
More than G pages need really big memory!
-- We are assuming some special cases like "short" int size?
If so, we may have to care about a lot of things from now on, because common
functions like __set_bit() don't support such long buffers.
If not, my patch might be over hacking -- especially the following part:
@@ -1183,10 +1183,13 @@ void mark_page_dirty(struct kvm *kvm, gfn_t gfn)
memslot = gfn_to_memslot_unaliased(kvm, gfn);
if (memslot && memslot->dirty_bitmap) {
unsigned long rel_gfn = gfn - memslot->base_gfn;
+ unsigned long *p = memslot->dirty_bitmap +
+ rel_gfn / BITS_PER_LONG;
+ int offset = rel_gfn % BITS_PER_LONG;
/* avoid RMW */
- if (!generic_test_le_bit(rel_gfn, memslot->dirty_bitmap))
- generic___set_le_bit(rel_gfn, memslot->dirty_bitmap);
+ if (!generic_test_le_bit(offset, p))
+ generic___set_le_bit(offset, p);
}
}
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] KVM: fix the handling of dirty bitmaps to avoid overflows
@ 2010-04-13 0:52 ` Takuya Yoshikawa
0 siblings, 0 replies; 23+ messages in thread
From: Takuya Yoshikawa @ 2010-04-13 0:52 UTC (permalink / raw)
To: kvm-ia64
(2010/04/13 2:39), Marcelo Tosatti wrote:
> On Mon, Apr 12, 2010 at 07:35:35PM +0900, Takuya Yoshikawa wrote:
>> This patch fixes a bug found by Avi during the review process
>> of my dirty bitmap related work.
>>
>> To ppc and ia64 people:
>> The fix is really simple but touches all architectures using
>> dirty bitmaps. So please check this will not suffer your part.
>>
>> =>>
>> Int is not long enough to store the size of a dirty bitmap.
>>
>> This patch fixes this problem with the introduction of a wrapper
>> function to calculate the sizes of dirty bitmaps.
>>
>> Note: in mark_page_dirty(), we have to consider the fact that
>> __set_bit() takes the offset as int, not long.
>>
>> Signed-off-by: Takuya Yoshikawa<yoshikawa.takuya@oss.ntt.co.jp>
>
> Applied, thanks.
>
Thanks everyone!
BTW, just from my curiosity, are there any cases in which we use such huge
number of pages currently?
ALIGN(memslot->npages, BITS_PER_LONG) / 8;
More than G pages need really big memory!
-- We are assuming some special cases like "short" int size?
If so, we may have to care about a lot of things from now on, because common
functions like __set_bit() don't support such long buffers.
If not, my patch might be over hacking -- especially the following part:
@@ -1183,10 +1183,13 @@ void mark_page_dirty(struct kvm *kvm, gfn_t gfn)
memslot = gfn_to_memslot_unaliased(kvm, gfn);
if (memslot && memslot->dirty_bitmap) {
unsigned long rel_gfn = gfn - memslot->base_gfn;
+ unsigned long *p = memslot->dirty_bitmap +
+ rel_gfn / BITS_PER_LONG;
+ int offset = rel_gfn % BITS_PER_LONG;
/* avoid RMW */
- if (!generic_test_le_bit(rel_gfn, memslot->dirty_bitmap))
- generic___set_le_bit(rel_gfn, memslot->dirty_bitmap);
+ if (!generic_test_le_bit(offset, p))
+ generic___set_le_bit(offset, p);
}
}
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] KVM: fix the handling of dirty bitmaps to avoid overflows
[not found] ` <4BC3C048.5030704-gVGce1chcLdL9jVzuh4AOg@public.gmane.org>
2010-04-13 6:50 ` Avi Kivity
@ 2010-04-13 6:50 ` Avi Kivity
0 siblings, 0 replies; 23+ messages in thread
From: Avi Kivity @ 2010-04-13 6:50 UTC (permalink / raw)
To: Takuya Yoshikawa
Cc: Marcelo Tosatti, kvm-u79uwXL29TY76Z2rM5mHXA,
kvm-ia64-u79uwXL29TY76Z2rM5mHXA, kvm-ppc-u79uwXL29TY76Z2rM5mHXA
On 04/13/2010 03:52 AM, Takuya Yoshikawa wrote:
> (2010/04/13 2:39), Marcelo Tosatti wrote:
>> On Mon, Apr 12, 2010 at 07:35:35PM +0900, Takuya Yoshikawa wrote:
>>> This patch fixes a bug found by Avi during the review process
>>> of my dirty bitmap related work.
>>>
>>> To ppc and ia64 people:
>>> The fix is really simple but touches all architectures using
>>> dirty bitmaps. So please check this will not suffer your part.
>>>
>>> ===
>>>
>>> Int is not long enough to store the size of a dirty bitmap.
>>>
>>> This patch fixes this problem with the introduction of a wrapper
>>> function to calculate the sizes of dirty bitmaps.
>>>
>>> Note: in mark_page_dirty(), we have to consider the fact that
>>> __set_bit() takes the offset as int, not long.
>>>
>>> Signed-off-by: Takuya Yoshikawa<yoshikawa.takuya-gVGce1chcLdL9jVzuh4AOg@public.gmane.org>
>>
>> Applied, thanks.
>>
>
> Thanks everyone!
>
> BTW, just from my curiosity, are there any cases in which we use such
> huge
> number of pages currently?
>
> ALIGN(memslot->npages, BITS_PER_LONG) / 8;
>
> More than G pages need really big memory!
> -- We are assuming some special cases like "short" int size?
No, int is 32 bits, but memslot->npages is not our under control.
Note that you don't actually need all those pages to create a large
memory slot.
>
> If so, we may have to care about a lot of things from now on, because
> common
> functions like __set_bit() don't support such long buffers.
It's better to limit memory slots to something that can be handled by
everything, then. 2^31 pages is plenty. Return -EINVAL if the slot is
too large.
--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] KVM: fix the handling of dirty bitmaps to avoid overflows
@ 2010-04-13 6:50 ` Avi Kivity
0 siblings, 0 replies; 23+ messages in thread
From: Avi Kivity @ 2010-04-13 6:50 UTC (permalink / raw)
To: Takuya Yoshikawa
Cc: Marcelo Tosatti, kvm-u79uwXL29TY76Z2rM5mHXA,
kvm-ia64-u79uwXL29TY76Z2rM5mHXA, kvm-ppc-u79uwXL29TY76Z2rM5mHXA
On 04/13/2010 03:52 AM, Takuya Yoshikawa wrote:
> (2010/04/13 2:39), Marcelo Tosatti wrote:
>> On Mon, Apr 12, 2010 at 07:35:35PM +0900, Takuya Yoshikawa wrote:
>>> This patch fixes a bug found by Avi during the review process
>>> of my dirty bitmap related work.
>>>
>>> To ppc and ia64 people:
>>> The fix is really simple but touches all architectures using
>>> dirty bitmaps. So please check this will not suffer your part.
>>>
>>> =>>>
>>> Int is not long enough to store the size of a dirty bitmap.
>>>
>>> This patch fixes this problem with the introduction of a wrapper
>>> function to calculate the sizes of dirty bitmaps.
>>>
>>> Note: in mark_page_dirty(), we have to consider the fact that
>>> __set_bit() takes the offset as int, not long.
>>>
>>> Signed-off-by: Takuya Yoshikawa<yoshikawa.takuya@oss.ntt.co.jp>
>>
>> Applied, thanks.
>>
>
> Thanks everyone!
>
> BTW, just from my curiosity, are there any cases in which we use such
> huge
> number of pages currently?
>
> ALIGN(memslot->npages, BITS_PER_LONG) / 8;
>
> More than G pages need really big memory!
> -- We are assuming some special cases like "short" int size?
No, int is 32 bits, but memslot->npages is not our under control.
Note that you don't actually need all those pages to create a large
memory slot.
>
> If so, we may have to care about a lot of things from now on, because
> common
> functions like __set_bit() don't support such long buffers.
It's better to limit memory slots to something that can be handled by
everything, then. 2^31 pages is plenty. Return -EINVAL if the slot is
too large.
--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] KVM: fix the handling of dirty bitmaps to avoid overflows
@ 2010-04-13 6:50 ` Avi Kivity
0 siblings, 0 replies; 23+ messages in thread
From: Avi Kivity @ 2010-04-13 6:50 UTC (permalink / raw)
To: kvm-ia64
On 04/13/2010 03:52 AM, Takuya Yoshikawa wrote:
> (2010/04/13 2:39), Marcelo Tosatti wrote:
>> On Mon, Apr 12, 2010 at 07:35:35PM +0900, Takuya Yoshikawa wrote:
>>> This patch fixes a bug found by Avi during the review process
>>> of my dirty bitmap related work.
>>>
>>> To ppc and ia64 people:
>>> The fix is really simple but touches all architectures using
>>> dirty bitmaps. So please check this will not suffer your part.
>>>
>>> =>>>
>>> Int is not long enough to store the size of a dirty bitmap.
>>>
>>> This patch fixes this problem with the introduction of a wrapper
>>> function to calculate the sizes of dirty bitmaps.
>>>
>>> Note: in mark_page_dirty(), we have to consider the fact that
>>> __set_bit() takes the offset as int, not long.
>>>
>>> Signed-off-by: Takuya Yoshikawa<yoshikawa.takuya@oss.ntt.co.jp>
>>
>> Applied, thanks.
>>
>
> Thanks everyone!
>
> BTW, just from my curiosity, are there any cases in which we use such
> huge
> number of pages currently?
>
> ALIGN(memslot->npages, BITS_PER_LONG) / 8;
>
> More than G pages need really big memory!
> -- We are assuming some special cases like "short" int size?
No, int is 32 bits, but memslot->npages is not our under control.
Note that you don't actually need all those pages to create a large
memory slot.
>
> If so, we may have to care about a lot of things from now on, because
> common
> functions like __set_bit() don't support such long buffers.
It's better to limit memory slots to something that can be handled by
everything, then. 2^31 pages is plenty. Return -EINVAL if the slot is
too large.
--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] KVM: fix the handling of dirty bitmaps to avoid overflows
2010-04-13 6:50 ` Avi Kivity
(?)
@ 2010-04-13 7:03 ` Takuya Yoshikawa
-1 siblings, 0 replies; 23+ messages in thread
From: Takuya Yoshikawa @ 2010-04-13 7:03 UTC (permalink / raw)
To: Avi Kivity; +Cc: Marcelo Tosatti, kvm, kvm-ia64, kvm-ppc
>>
>> BTW, just from my curiosity, are there any cases in which we use such
>> huge
>> number of pages currently?
>>
>> ALIGN(memslot->npages, BITS_PER_LONG) / 8;
>>
>> More than G pages need really big memory!
>> -- We are assuming some special cases like "short" int size?
>
> No, int is 32 bits, but memslot->npages is not our under control.
>
> Note that you don't actually need all those pages to create a large
> memory slot.
>
>>
>> If so, we may have to care about a lot of things from now on, because
>> common
>> functions like __set_bit() don't support such long buffers.
>
> It's better to limit memory slots to something that can be handled by
> everything, then. 2^31 pages is plenty. Return -EINVAL if the slot is
> too large.
I agree with that, so we make this patch pending to fix like that?
-- or should make a new patch based on this patch?
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] KVM: fix the handling of dirty bitmaps to avoid overflows
@ 2010-04-13 7:03 ` Takuya Yoshikawa
0 siblings, 0 replies; 23+ messages in thread
From: Takuya Yoshikawa @ 2010-04-13 7:03 UTC (permalink / raw)
To: Avi Kivity; +Cc: Marcelo Tosatti, kvm, kvm-ia64, kvm-ppc
>>
>> BTW, just from my curiosity, are there any cases in which we use such
>> huge
>> number of pages currently?
>>
>> ALIGN(memslot->npages, BITS_PER_LONG) / 8;
>>
>> More than G pages need really big memory!
>> -- We are assuming some special cases like "short" int size?
>
> No, int is 32 bits, but memslot->npages is not our under control.
>
> Note that you don't actually need all those pages to create a large
> memory slot.
>
>>
>> If so, we may have to care about a lot of things from now on, because
>> common
>> functions like __set_bit() don't support such long buffers.
>
> It's better to limit memory slots to something that can be handled by
> everything, then. 2^31 pages is plenty. Return -EINVAL if the slot is
> too large.
I agree with that, so we make this patch pending to fix like that?
-- or should make a new patch based on this patch?
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] KVM: fix the handling of dirty bitmaps to avoid overflows
@ 2010-04-13 7:03 ` Takuya Yoshikawa
0 siblings, 0 replies; 23+ messages in thread
From: Takuya Yoshikawa @ 2010-04-13 7:03 UTC (permalink / raw)
To: kvm-ia64
>>
>> BTW, just from my curiosity, are there any cases in which we use such
>> huge
>> number of pages currently?
>>
>> ALIGN(memslot->npages, BITS_PER_LONG) / 8;
>>
>> More than G pages need really big memory!
>> -- We are assuming some special cases like "short" int size?
>
> No, int is 32 bits, but memslot->npages is not our under control.
>
> Note that you don't actually need all those pages to create a large
> memory slot.
>
>>
>> If so, we may have to care about a lot of things from now on, because
>> common
>> functions like __set_bit() don't support such long buffers.
>
> It's better to limit memory slots to something that can be handled by
> everything, then. 2^31 pages is plenty. Return -EINVAL if the slot is
> too large.
I agree with that, so we make this patch pending to fix like that?
-- or should make a new patch based on this patch?
>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] KVM: fix the handling of dirty bitmaps to avoid overflows
2010-04-13 7:03 ` Takuya Yoshikawa
(?)
@ 2010-04-13 7:05 ` Avi Kivity
-1 siblings, 0 replies; 23+ messages in thread
From: Avi Kivity @ 2010-04-13 7:05 UTC (permalink / raw)
To: Takuya Yoshikawa; +Cc: Marcelo Tosatti, kvm, kvm-ia64, kvm-ppc
On 04/13/2010 10:03 AM, Takuya Yoshikawa wrote:
>> It's better to limit memory slots to something that can be handled by
>> everything, then. 2^31 pages is plenty. Return -EINVAL if the slot is
>> too large.
>
>
> I agree with that, so we make this patch pending to fix like that?
> -- or should make a new patch based on this patch?
We need a new patch to block oversize memory slots. The current patch
can come on top (but now page numbers fit inside an int, so it is just a
cleanup, not a bugfix).
--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] KVM: fix the handling of dirty bitmaps to avoid overflows
@ 2010-04-13 7:05 ` Avi Kivity
0 siblings, 0 replies; 23+ messages in thread
From: Avi Kivity @ 2010-04-13 7:05 UTC (permalink / raw)
To: Takuya Yoshikawa; +Cc: Marcelo Tosatti, kvm, kvm-ia64, kvm-ppc
On 04/13/2010 10:03 AM, Takuya Yoshikawa wrote:
>> It's better to limit memory slots to something that can be handled by
>> everything, then. 2^31 pages is plenty. Return -EINVAL if the slot is
>> too large.
>
>
> I agree with that, so we make this patch pending to fix like that?
> -- or should make a new patch based on this patch?
We need a new patch to block oversize memory slots. The current patch
can come on top (but now page numbers fit inside an int, so it is just a
cleanup, not a bugfix).
--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH] KVM: fix the handling of dirty bitmaps to avoid overflows
@ 2010-04-13 7:05 ` Avi Kivity
0 siblings, 0 replies; 23+ messages in thread
From: Avi Kivity @ 2010-04-13 7:05 UTC (permalink / raw)
To: kvm-ia64
On 04/13/2010 10:03 AM, Takuya Yoshikawa wrote:
>> It's better to limit memory slots to something that can be handled by
>> everything, then. 2^31 pages is plenty. Return -EINVAL if the slot is
>> too large.
>
>
> I agree with that, so we make this patch pending to fix like that?
> -- or should make a new patch based on this patch?
We need a new patch to block oversize memory slots. The current patch
can come on top (but now page numbers fit inside an int, so it is just a
cleanup, not a bugfix).
--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH] KVM: cleanup: limit the number of pages per memory slot
2010-04-12 10:35 ` Takuya Yoshikawa
` (3 preceding siblings ...)
(?)
@ 2010-04-13 13:47 ` Takuya Yoshikawa
2010-04-14 16:58 ` Marcelo Tosatti
-1 siblings, 1 reply; 23+ messages in thread
From: Takuya Yoshikawa @ 2010-04-13 13:47 UTC (permalink / raw)
To: avi, mtosatti; +Cc: kvm
This is based on my last patch:
"fix the handling of dirty bitmaps to avoid overflows"
Sorry, this resulted in reverting part of that.
===
This patch limits the number of pages per memory slot to make
us free from extra care about type issues.
Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
---
include/linux/kvm_host.h | 6 ++++++
virt/kvm/kvm_main.c | 11 ++++++-----
2 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 169d077..5583063 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -105,6 +105,12 @@ struct kvm_vcpu {
struct kvm_vcpu_arch arch;
};
+/*
+ * Some of the bitops functions do not support too long bitmaps.
+ * This number must be determined not to exceed such limits.
+ */
+#define KVM_MEM_MAX_NR_PAGES ((1UL << 31) - 1)
+
struct kvm_memory_slot {
gfn_t base_gfn;
unsigned long npages;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 6fe79c4..799ff1f 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -553,6 +553,10 @@ int __kvm_set_memory_region(struct kvm *kvm,
base_gfn = mem->guest_phys_addr >> PAGE_SHIFT;
npages = mem->memory_size >> PAGE_SHIFT;
+ r = -EINVAL;
+ if (npages > KVM_MEM_MAX_NR_PAGES)
+ goto out;
+
if (!npages)
mem->flags &= ~KVM_MEM_LOG_DIRTY_PAGES;
@@ -1183,13 +1187,10 @@ void mark_page_dirty(struct kvm *kvm, gfn_t gfn)
memslot = gfn_to_memslot_unaliased(kvm, gfn);
if (memslot && memslot->dirty_bitmap) {
unsigned long rel_gfn = gfn - memslot->base_gfn;
- unsigned long *p = memslot->dirty_bitmap +
- rel_gfn / BITS_PER_LONG;
- int offset = rel_gfn % BITS_PER_LONG;
/* avoid RMW */
- if (!generic_test_le_bit(offset, p))
- generic___set_le_bit(offset, p);
+ if (!generic_test_le_bit(rel_gfn, memslot->dirty_bitmap))
+ generic___set_le_bit(rel_gfn, memslot->dirty_bitmap);
}
}
--
1.6.3.3
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH] KVM: cleanup: limit the number of pages per memory slot
2010-04-13 13:47 ` [PATCH] KVM: cleanup: limit the number of pages per memory slot Takuya Yoshikawa
@ 2010-04-14 16:58 ` Marcelo Tosatti
0 siblings, 0 replies; 23+ messages in thread
From: Marcelo Tosatti @ 2010-04-14 16:58 UTC (permalink / raw)
To: Takuya Yoshikawa; +Cc: avi, kvm
On Tue, Apr 13, 2010 at 10:47:24PM +0900, Takuya Yoshikawa wrote:
> This is based on my last patch:
> "fix the handling of dirty bitmaps to avoid overflows"
>
> Sorry, this resulted in reverting part of that.
>
> ===
>
> This patch limits the number of pages per memory slot to make
> us free from extra care about type issues.
>
> Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
> ---
> include/linux/kvm_host.h | 6 ++++++
> virt/kvm/kvm_main.c | 11 ++++++-----
> 2 files changed, 12 insertions(+), 5 deletions(-)
Applied, thanks.
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2010-04-14 17:46 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-04-12 10:35 [PATCH] KVM: fix the handling of dirty bitmaps to avoid overflows Takuya Yoshikawa
2010-04-12 10:35 ` Takuya Yoshikawa
2010-04-12 10:35 ` Takuya Yoshikawa
[not found] ` <20100412193535.6c502695.yoshikawa.takuya-gVGce1chcLdL9jVzuh4AOg@public.gmane.org>
2010-04-12 11:10 ` Alexander Graf
2010-04-12 11:10 ` Alexander Graf
2010-04-12 11:10 ` Alexander Graf
2010-04-12 17:39 ` Marcelo Tosatti
2010-04-12 17:39 ` Marcelo Tosatti
2010-04-12 17:39 ` Marcelo Tosatti
[not found] ` <20100412173951.GA5614-I4X2Mt4zSy4@public.gmane.org>
2010-04-13 0:52 ` Takuya Yoshikawa
2010-04-13 0:52 ` Takuya Yoshikawa
2010-04-13 0:52 ` Takuya Yoshikawa
[not found] ` <4BC3C048.5030704-gVGce1chcLdL9jVzuh4AOg@public.gmane.org>
2010-04-13 6:50 ` Avi Kivity
2010-04-13 6:50 ` Avi Kivity
2010-04-13 6:50 ` Avi Kivity
2010-04-13 7:03 ` Takuya Yoshikawa
2010-04-13 7:03 ` Takuya Yoshikawa
2010-04-13 7:03 ` Takuya Yoshikawa
2010-04-13 7:05 ` Avi Kivity
2010-04-13 7:05 ` Avi Kivity
2010-04-13 7:05 ` Avi Kivity
2010-04-13 13:47 ` [PATCH] KVM: cleanup: limit the number of pages per memory slot Takuya Yoshikawa
2010-04-14 16:58 ` Marcelo Tosatti
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.