[Qemu-devel] [PULL] Urgent memory fix for kvm with unaligned memory slots

[Qemu-devel] [PULL] Urgent memory fix for kvm with unaligned memory slots

[Avi Kivity]

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;
 
     while (1) {
         mem = kvm_lookup_overlapping_slot(s, start_addr, start_addr +
size);

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PULL] Urgent memory fix for kvm with unaligned memory slots

[Bobby Powers]

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

Re: [Qemu-devel] [PULL] Urgent memory fix for kvm with unaligned memory slots

[Avi Kivity]

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?

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PULL] Urgent memory fix for kvm with unaligned memory slots

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 731 bytes --]

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.

-- 
Eric Blake   eblake@redhat.com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 620 bytes --]

Re: [Qemu-devel] [PULL] Urgent memory fix for kvm with unaligned memory slots

[Bobby Powers]

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
>

Re: [Qemu-devel] [PULL] Urgent memory fix for kvm with unaligned memory slots

[Avi Kivity]

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.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] [PULL] Urgent memory fix for kvm with unaligned memory slots

[Anthony Liguori]

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);
>