| Message ID | 4F4F7C47.6090005@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Mar 1, 2012 at 8:40 AM, Avi Kivity <avi@redhat.com> wrote: > The memory core may generate RAM memory regions that are not page > aligned, but the kvm code is not prepared to handle them well and will > abort under certain conditions. This patch fixes the problem. > > Please pull from: > > git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git memory/urgent > > ---------------------------------------------------------------- > Avi Kivity (1): > kvm: fix unaligned slots > > kvm-all.c | 15 ++++++++++++--- > 1 files changed, 12 insertions(+), 3 deletions(-) > > diff --git a/kvm-all.c b/kvm-all.c > index c4babda..4b7a4ae 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -541,17 +541,26 @@ static void kvm_set_phys_mem(MemoryRegionSection > *section, bool add) > target_phys_addr_t start_addr = section->offset_within_address_space; > ram_addr_t size = section->size; > void *ram = NULL; > + unsigned delta; > > /* kvm works in page size chunks, but the function may be called > with sub-page size and unaligned start address. */ > - size = TARGET_PAGE_ALIGN(size); > - start_addr = TARGET_PAGE_ALIGN(start_addr); > + delta = TARGET_PAGE_ALIGN(size) - size; > + if (delta > size) { > + return; > + } > + start_addr += delta; > + size -= delta; > + size &= TARGET_PAGE_MASK; > + if (!size || (start_addr & ~TARGET_PAGE_MASK)) { > + return; > + } > > if (!memory_region_is_ram(mr)) { > return; > } > > - ram = memory_region_get_ram_ptr(mr) + section->offset_within_region; > + ram = memory_region_get_ram_ptr(mr) + section->offset_within_region > + delta; Am I crazy, or does this look wrong? > > while (1) { > mem = kvm_lookup_overlapping_slot(s, start_addr, start_addr + > size); > > -- > 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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On 03/01/2012 06:51 PM, Bobby Powers wrote: > > /* kvm works in page size chunks, but the function may be called > > with sub-page size and unaligned start address. */ > > - size = TARGET_PAGE_ALIGN(size); > > - start_addr = TARGET_PAGE_ALIGN(start_addr); > > + delta = TARGET_PAGE_ALIGN(size) - size; > > + if (delta > size) { > > + return; > > + } > > + start_addr += delta; > > + size -= delta; > > + size &= TARGET_PAGE_MASK; > > + if (!size || (start_addr & ~TARGET_PAGE_MASK)) { > > + return; > > + } > > > > if (!memory_region_is_ram(mr)) { > > return; > > } > > > > - ram = memory_region_get_ram_ptr(mr) + section->offset_within_region; > > + ram = memory_region_get_ram_ptr(mr) + section->offset_within_region > > + delta; > > Am I crazy, or does this look wrong? Could be both. Why do you thing it is wrong?
On 03/01/2012 10:03 AM, Avi Kivity wrote: >>> >>> - ram = memory_region_get_ram_ptr(mr) + section->offset_within_region; >>> + ram = memory_region_get_ram_ptr(mr) + section->offset_within_region >>> + delta; >> >> Am I crazy, or does this look wrong? > > Could be both. Why do you thing it is wrong? Line wrapping makes it look like we are adding two lines, one line ending in 'section->offset_within_region', and the next line starting with 'delta;', which is a syntax error. But without line wrapping, we are adding just one line with 'offset_within_region + delta;' at the end of that line.
On Thu, Mar 1, 2012 at 12:08 PM, Eric Blake <eblake@redhat.com> wrote: > On 03/01/2012 10:03 AM, Avi Kivity wrote: >>>> >>>> - ram = memory_region_get_ram_ptr(mr) + section->offset_within_region; >>>> + ram = memory_region_get_ram_ptr(mr) + section->offset_within_region >>>> + delta; >>> >>> Am I crazy, or does this look wrong? >> >> Could be both. Why do you thing it is wrong? > > Line wrapping makes it look like we are adding two lines, one line > ending in 'section->offset_within_region', and the next line starting > with 'delta;', which is a syntax error. yea, patch line wrapping was making my eyes see things. okay, sorry for the noise. > > But without line wrapping, we are adding just one line with > 'offset_within_region + delta;' at the end of that line. > > -- > Eric Blake eblake@redhat.com +1-919-301-3266 > Libvirt virtualization library http://libvirt.org >
On 03/01/2012 07:08 PM, Eric Blake wrote: > On 03/01/2012 10:03 AM, Avi Kivity wrote: > >>> > >>> - ram = memory_region_get_ram_ptr(mr) + section->offset_within_region; > >>> + ram = memory_region_get_ram_ptr(mr) + section->offset_within_region > >>> + delta; > >> > >> Am I crazy, or does this look wrong? > > > > Could be both. Why do you thing it is wrong? > > Line wrapping makes it look like we are adding two lines, one line > ending in 'section->offset_within_region', and the next line starting > with 'delta;', which is a syntax error. > > But without line wrapping, we are adding just one line with > 'offset_within_region + delta;' at the end of that line. > Ah, of course. I just copy/pasted this into thunderbird, this is not a proper patch but a pull request. Sorry about the confusion. Bobby: so it does indeed look wrong. This says nothing about your sanity though, for that consult a qualified professional instead of asking on the mailing list.
On 03/01/2012 07:40 AM, Avi Kivity wrote: > The memory core may generate RAM memory regions that are not page > aligned, but the kvm code is not prepared to handle them well and will > abort under certain conditions. This patch fixes the problem. > > Please pull from: > > git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git memory/urgent Pulled. Thanks. Regards, Anthony Liguori > > ---------------------------------------------------------------- > Avi Kivity (1): > kvm: fix unaligned slots > > kvm-all.c | 15 ++++++++++++--- > 1 files changed, 12 insertions(+), 3 deletions(-) > > diff --git a/kvm-all.c b/kvm-all.c > index c4babda..4b7a4ae 100644 > --- a/kvm-all.c > +++ b/kvm-all.c > @@ -541,17 +541,26 @@ static void kvm_set_phys_mem(MemoryRegionSection > *section, bool add) > target_phys_addr_t start_addr = section->offset_within_address_space; > ram_addr_t size = section->size; > void *ram = NULL; > + unsigned delta; > > /* kvm works in page size chunks, but the function may be called > with sub-page size and unaligned start address. */ > - size = TARGET_PAGE_ALIGN(size); > - start_addr = TARGET_PAGE_ALIGN(start_addr); > + delta = TARGET_PAGE_ALIGN(size) - size; > + if (delta> size) { > + return; > + } > + start_addr += delta; > + size -= delta; > + size&= TARGET_PAGE_MASK; > + if (!size || (start_addr& ~TARGET_PAGE_MASK)) { > + return; > + } > > if (!memory_region_is_ram(mr)) { > return; > } > > - ram = memory_region_get_ram_ptr(mr) + section->offset_within_region; > + ram = memory_region_get_ram_ptr(mr) + section->offset_within_region > + delta; > > while (1) { > mem = kvm_lookup_overlapping_slot(s, start_addr, start_addr + > size); >
diff --git a/kvm-all.c b/kvm-all.c index c4babda..4b7a4ae 100644 --- a/kvm-all.c +++ b/kvm-all.c @@ -541,17 +541,26 @@ static void kvm_set_phys_mem(MemoryRegionSection *section, bool add) target_phys_addr_t start_addr = section->offset_within_address_space; ram_addr_t size = section->size; void *ram = NULL; + unsigned delta; /* kvm works in page size chunks, but the function may be called with sub-page size and unaligned start address. */ - size = TARGET_PAGE_ALIGN(size); - start_addr = TARGET_PAGE_ALIGN(start_addr); + delta = TARGET_PAGE_ALIGN(size) - size; + if (delta > size) { + return; + } + start_addr += delta; + size -= delta; + size &= TARGET_PAGE_MASK; + if (!size || (start_addr & ~TARGET_PAGE_MASK)) { + return; + } if (!memory_region_is_ram(mr)) { return; } - ram = memory_region_get_ram_ptr(mr) + section->offset_within_region; + ram = memory_region_get_ram_ptr(mr) + section->offset_within_region + delta; while (1) {
The memory core may generate RAM memory regions that are not page aligned, but the kvm code is not prepared to handle them well and will abort under certain conditions. This patch fixes the problem. Please pull from: git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git memory/urgent ---------------------------------------------------------------- Avi Kivity (1): kvm: fix unaligned slots kvm-all.c | 15 ++++++++++++--- 1 files changed, 12 insertions(+), 3 deletions(-) mem = kvm_lookup_overlapping_slot(s, start_addr, start_addr + size);