Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[Markus Armbruster]

Anthony Liguori anth...@codemonkey.ws writes:

 Avi Kivity wrote:
 On 12/04/2009 06:49 PM, Anthony Liguori wrote:

 I still believe that it is poor practice to pass size==0 to
 *malloc().  I think actively discouraging this in qemu is a good
 thing because it's a broken idiom.

 Why?  Unless we have a separate array allocator (like C++'s new and
 new[]), we need to support zero-element arrays without pushing the
 burden to callers (in the same way that for () supports zero
 iteration loops without a separate if ()).

 If you call malloc(size=0), then it's impossible to differentiate
 between a failed malloc return and a valid result on certain platform
 (like AIX).

 So the only correct way would be to write:

 array = malloc(size);
 if (array == NULL  size != 0) {
return -ENOMEM;
 }

 If you were writing portable code.  But that's not what people write.

Yup.  But

if (n == 0)
p = malloc(n * sizeof(struct foo))
else
p = NULL;

isn't what people write either.

 You can argue that qemu_malloc() can have any semantics we want and
 while that's true, it doesn't change my above statement.  I think the
 main argument for this behavior in qemu is that people are used to
 using this idiom with malloc() but it's a non-portable practice.

We've decided long ago to disable the trap you quoted by not returning
NULL for empty allocations.  That way it doesn't kill us when people
fail to call qemu_malloc() perfectly every time.

 If qemu_malloc() didn't carry the name malloc() then semantics with
 size=0 would be a different discussion.  But so far, all qemu_*
 functions tend to behave almost exactly like their C counterparts.

That's a reason for making qemu_malloc() work for zero arguments,
because its C counterpart does.

 Relying on the result of size=0 with malloc() is broken.

No.  Relying on non-null-ness is broken.  You can write perfectly
portable, working code without doing that.

p = malloc(n * sizeof(struct foo);
if (n  !p)
exit_no_mem();
for (i = 0; i  n; i++)
compute_one(p, i);

With qemu_malloc(), the error handling moves into qemu_malloc():

p = qemu_malloc(n * sizeof(struct foo);
for (i = 0; i  n; i++)
compute_one(p, i);

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[Markus Armbruster]

Avi Kivity a...@redhat.com writes:

 A NEW(type) and ARRAY_NEW(type, count) marcros would improve type
 safety and plug a dormant buffer overflow due to multiplication
 overflow, yes.  Even qemu_calloc() would be an improvement.  But
 having qemu_malloc() not fix the zero length array case which we know
 we have is irresponsible, IMO.

Agree on all counts.

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[malc]

On Sun, 6 Dec 2009, Markus Armbruster wrote:

 malc av1...@comtv.ru writes:

  On Sat, 5 Dec 2009, Markus Armbruster wrote:
 
  Anthony Liguori anth...@codemonkey.ws writes:
 
   Markus Armbruster wrote:
   Commit a7d27b53 made zero-sized allocations a fatal error, deviating
   from ISO C's malloc()  friends.  Revert that, but take care never to
   return a null pointer, like malloc()  friends may do (it's
   implementation defined), because that's another source of bugs.
  
   Rationale: while zero-sized allocations might occasionally be a sign of
   something going wrong, they can also be perfectly legitimate.  The
   change broke such legitimate uses.  We've found and fixed at least one
   of them already (commit eb0b64f7, also reverted by this patch), and
   another one just popped up: the change broke qcow2 images with virtual
   disk size zero, i.e. images that don't hold real data but only VM state
   of snapshots.
  
  

[..snip..]


 
  P.S. It would be interesting to know how this code behaves under OpenBSD, 
  with
   p = malloc (0);
 
  [1] As does, in essence, 
  http://www.opengroup.org/onlinepubs/7990989775/xsh/read.html

 Replace p = (void *)-1 by p = NULL and it works just fine.


That's why i asked for somone to run it on OpenBSD:

Quoting 
http://www.openbsd.org/cgi-bin/man.cgi?query=mallocapropos=0sektion=3manpath=OpenBSD+Currentarch=i386format=html

  Allocation of a zero size object returns a pointer to a zero size object.
  This zero size object is access protected, so any access to it will gen-
  erate an exception (SIGSEGV).  Many zero-sized objects can be placed con-
  secutively in shared protected pages.  The minimum size of the protection
  on each object is suitably aligned and sized as previously stated, but
  the protection may extend further depending on where in a protected zone
  the object lands.

 malloc() either returns a valid pointer or NULL, so what read() does for
 other pointers doesn't matter.

Replace returns with should and this still won't match the wording
of the standard, besides as read behaviour on Linux shows following
those are not high on the agenda.

  7.20.3.3  The malloc function

   Synopsis

   [#1]

   #include stdlib.h
   void *malloc(size_t size);

   Description

   [#2] The malloc function allocates space for an object whose
   size  is specified by size and whose value is indeterminate.

   Returns

   [#3] The malloc function returns either a null pointer or  a
   pointer to the allocated space.

I don't know what a valid pointer in this context represents.

 qemu_malloc() always returns a valid pointer, but that's not even needed
 in this case.


--
mailto:av1...@comtv.ru

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[Markus Armbruster]

malc av1...@comtv.ru writes:

 On Sun, 6 Dec 2009, Markus Armbruster wrote:

 malc av1...@comtv.ru writes:

  On Sat, 5 Dec 2009, Markus Armbruster wrote:
 
  Anthony Liguori anth...@codemonkey.ws writes:
 
   Markus Armbruster wrote:
   Commit a7d27b53 made zero-sized allocations a fatal error, deviating
   from ISO C's malloc()  friends.  Revert that, but take care never to
   return a null pointer, like malloc()  friends may do (it's
   implementation defined), because that's another source of bugs.
  
   Rationale: while zero-sized allocations might occasionally be a sign of
   something going wrong, they can also be perfectly legitimate.  The
   change broke such legitimate uses.  We've found and fixed at least 
   one
   of them already (commit eb0b64f7, also reverted by this patch), and
   another one just popped up: the change broke qcow2 images with virtual
   disk size zero, i.e. images that don't hold real data but only VM state
   of snapshots.
  
  

 [..snip..]


 
  P.S. It would be interesting to know how this code behaves under OpenBSD, 
  with
   p = malloc (0);
[...]

 Replace p = (void *)-1 by p = NULL and it works just fine.


 That's why i asked for somone to run it on OpenBSD:

 Quoting 
 http://www.openbsd.org/cgi-bin/man.cgi?query=mallocapropos=0sektion=3manpath=OpenBSD+Currentarch=i386format=html

   Allocation of a zero size object returns a pointer to a zero size object.
   This zero size object is access protected, so any access to it will gen-
   erate an exception (SIGSEGV).  Many zero-sized objects can be placed con-
   secutively in shared protected pages.  The minimum size of the protection
   on each object is suitably aligned and sized as previously stated, but
   the protection may extend further depending on where in a protected zone
   the object lands.

read(fd, malloc(0), 0) is just fine, because read() doesn't touch the
buffer when the size is zero.

 malloc() either returns a valid pointer or NULL, so what read() does for
 other pointers doesn't matter.

 Replace returns with should and this still won't match the wording
 of the standard, besides as read behaviour on Linux shows following
 those are not high on the agenda.

   7.20.3.3  The malloc function

Synopsis

[#1]

#include stdlib.h
void *malloc(size_t size);

Description

[#2] The malloc function allocates space for an object whose
size  is specified by size and whose value is indeterminate.

Returns

[#3] The malloc function returns either a null pointer or  a
pointer to the allocated space.

 I don't know what a valid pointer in this context represents.

I can talk standardese, if you prefer :)

malloc() either returns either a null pointer or a pointer to the
allocated space.  In either case, you must not dereference the pointer.

OpenBSD chooses to return a pointer to the allocated space.  It chooses
to catch common ways to dereference the pointer.

Your p = (void *)-1 is neither a null pointer nor can it point to
allocated space on your particular system.  Hence, it cannot be a value
of malloc() for any argument, and therefore what read() does with it on
that particular system doesn't matter.

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[Blue Swirl]

On Sun, Dec 6, 2009 at 10:39 AM, malc av1...@comtv.ru wrote:
 On Sun, 6 Dec 2009, Markus Armbruster wrote:

 malc av1...@comtv.ru writes:

  On Sat, 5 Dec 2009, Markus Armbruster wrote:
 
  Anthony Liguori anth...@codemonkey.ws writes:
 
   Markus Armbruster wrote:
   Commit a7d27b53 made zero-sized allocations a fatal error, deviating
   from ISO C's malloc()  friends.  Revert that, but take care never to
   return a null pointer, like malloc()  friends may do (it's
   implementation defined), because that's another source of bugs.
  
   Rationale: while zero-sized allocations might occasionally be a sign of
   something going wrong, they can also be perfectly legitimate.  The
   change broke such legitimate uses.  We've found and fixed at least 
   one
   of them already (commit eb0b64f7, also reverted by this patch), and
   another one just popped up: the change broke qcow2 images with virtual
   disk size zero, i.e. images that don't hold real data but only VM state
   of snapshots.
  
  

 [..snip..]


 
  P.S. It would be interesting to know how this code behaves under OpenBSD, 
  with
       p = malloc (0);
 
  [1] As does, in essence, 
  http://www.opengroup.org/onlinepubs/7990989775/xsh/read.html

 Replace p = (void *)-1 by p = NULL and it works just fine.


 That's why i asked for somone to run it on OpenBSD:

$ cat mall.c
#define _GNU_SOURCE
#include err.h
#include unistd.h
#include stdlib.h
#include fcntl.h
#include stdio.h

int main (void)
{
int fd = open (/dev/zero, 0);
int ret;
#if 0
void *p = (void *) -1;
#else
void *p = malloc(0);
#endif

fprintf(stderr, ptr %p\n, p);
if (fd == -1) err (1, open);
ret = read (fd, p, 0);
if (ret != 0) err (1, read);
return 0;
}
$ gcc mall.c
$ ./a.out
ptr 0x46974060
$

Changing read count to 1:
$ ./a.out
ptr 0x41ce0070
a.out: read: Bad address

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[malc]

On Sun, 6 Dec 2009, Blue Swirl wrote:

 On Sun, Dec 6, 2009 at 10:39 AM, malc av1...@comtv.ru wrote:
  On Sun, 6 Dec 2009, Markus Armbruster wrote:

[..snip..]

 $ gcc mall.c
 $ ./a.out
 ptr 0x46974060
 $
 
 Changing read count to 1:
 $ ./a.out
 ptr 0x41ce0070
 a.out: read: Bad address
 

Thanks.

-- 
mailto:av1...@comtv.ru

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[malc]

On Sun, 6 Dec 2009, Markus Armbruster wrote:

 malc av1...@comtv.ru writes:
 

[..snip..]

 
 read(fd, malloc(0), 0) is just fine, because read() doesn't touch the
 buffer when the size is zero.
 

[..snip..]

Yet under linux the address is checked even for zero case.

 
  I don't know what a valid pointer in this context represents.
 
 I can talk standardese, if you prefer :)
 
 malloc() either returns either a null pointer or a pointer to the
 allocated space.  In either case, you must not dereference the pointer.
 
 OpenBSD chooses to return a pointer to the allocated space.  It chooses
 to catch common ways to dereference the pointer.
 
 Your p = (void *)-1 is neither a null pointer nor can it point to
 allocated space on your particular system.  Hence, it cannot be a value
 of malloc() for any argument, and therefore what read() does with it on
 that particular system doesn't matter.
 

Here, i believe, you are inventing artificial restrictions on how
malloc behaves, i don't see anything that prevents the implementor
from setting aside a range of addresses with 31st bit set as an
indicator of zero allocations, and then happily giving it to the
user of malloc and consumming it in free.

-- 
mailto:av1...@comtv.ru

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[Avi Kivity]

On 12/06/2009 12:22 PM, malc wrote:

Here, i believe, you are inventing artificial restrictions on how
malloc behaves, i don't see anything that prevents the implementor
from setting aside a range of addresses with 31st bit set as an
indicator of zero allocations, and then happily giving it to the
user of malloc and consumming it in free.
   


The implementation needs to track which addresses it handed out, since 
it is required that malloc(0) != malloc(0) (unless both are NULL).


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

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[Markus Armbruster]

malc av1...@comtv.ru writes:

 On Sun, 6 Dec 2009, Markus Armbruster wrote:

 malc av1...@comtv.ru writes:
 

 [..snip..]

 
 read(fd, malloc(0), 0) is just fine, because read() doesn't touch the
 buffer when the size is zero.
 

 [..snip..]

 Yet under linux the address is checked even for zero case.

Any value you can obtain from malloc() passes that check.

Why does the fact that you can construct pointers that don't pass this
check matter for our discussion of malloc()?

  I don't know what a valid pointer in this context represents.
 
 I can talk standardese, if you prefer :)
 
 malloc() either returns either a null pointer or a pointer to the
 allocated space.  In either case, you must not dereference the pointer.
 
 OpenBSD chooses to return a pointer to the allocated space.  It chooses
 to catch common ways to dereference the pointer.
 
 Your p = (void *)-1 is neither a null pointer nor can it point to
 allocated space on your particular system.  Hence, it cannot be a value
 of malloc() for any argument, and therefore what read() does with it on
 that particular system doesn't matter.
 

 Here, i believe, you are inventing artificial restrictions on how
 malloc behaves, i don't see anything that prevents the implementor
 from setting aside a range of addresses with 31st bit set as an
 indicator of zero allocations, and then happily giving it to the
 user of malloc and consumming it in free.

Misunderstanding?  Such behavior is indeed permissible, and I can't see
where I restricted it away.  An implementation that behaves as you
describe returns pointer to allocated space.  That the pointer has
some funny bit set doesn't matter.  That it can't be dereferenced is
just fine.

I'm not sure what your point is.  If it is that malloc(0) can return a
value that cannot be passed to a zero-sized read(), then I fear you have
not made your point.

[Qemu-devel] Re: [PATCH] Permit zero-sized qemu_malloc() friends

[Paolo Bonzini]

On 12/06/2009 11:22 AM, malc wrote:

Here, i believe, you are inventing artificial restrictions on how
malloc behaves, i don't see anything that prevents the implementor
from setting aside a range of addresses with 31st bit set as an
indicator of zero allocations, and then happily giving it to the
user of malloc and consumming it in free.


But it has to make it a valid address anyway.  If a zero-sized read 
treats it as invalid (SIGSEGV, EFAULT, whatever), malloc has failed to 
return a valid address and is not obeying its specification.


Paolo

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[malc]

On Sun, 6 Dec 2009, Avi Kivity wrote:

 On 12/06/2009 12:22 PM, malc wrote:
  Here, i believe, you are inventing artificial restrictions on how
  malloc behaves, i don't see anything that prevents the implementor
  from setting aside a range of addresses with 31st bit set as an
  indicator of zero allocations, and then happily giving it to the
  user of malloc and consumming it in free.
 
 
 The implementation needs to track which addresses it handed out, since it is
 required that malloc(0) != malloc(0) (unless both are NULL).

You haven't read carefully, i said range.

-- 
mailto:av1...@comtv.ru

Re: [Qemu-devel] Staging update (0.12 pending freeze)

[Aurelien Jarno]

On Sat, Dec 05, 2009 at 08:07:13PM +, Blue Swirl wrote:
 On Sat, Dec 5, 2009 at 8:05 PM, Aurelien Jarno aurel...@aurel32.net wrote:
  On Thu, Dec 03, 2009 at 10:03:18PM +0200, Blue Swirl wrote:
  On Thu, Dec 3, 2009 at 9:26 PM, Aurelien Jarno aurel...@aurel32.net 
  wrote:
   On Wed, Dec 02, 2009 at 10:46:11AM -0600, Anthony Liguori wrote:
   I've got all of the patches I'm considering for 0.12 currently in
   staging.  I'm going to work through and test/commit these in a few
   chunks over the next few days before freezing the tree.
  
  
   What are the plans on the OpenBIOS side? The version currently included
   in QEMU is old compared to the SVN. Is it plan to sync a release of
   OpenBIOS with QEMU?
 
  At least the images should be updated.
 
 
  Now that version 0.12.0-rca has been tagged, we should probably do that
  asap. Should I do it?
 
 Please do.
 

I have seen you have been faster than me, thanks.

Anyway I am not able to fully build the powerpc images here,
openbios-unix fails to build with:

| libmodules.a(elf-loader.o): In function `elf_loader_init_program':
| /home/aurel32/openbios-devel/obj-ppc/../modules/elf-loader.c:77: undefined 
reference to `flush_icache_range'
| libmodules.a(xcoff-loader.o): In function `xcoff_loader_init_program':
| /home/aurel32/openbios-devel/obj-ppc/../modules/xcoff-loader.c:110: undefined 
reference to `flush_icache_range'
| libmodules.a(ofmem_common.o): In function `ofmem_translate':
| /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:680: undefined 
reference to `ofmem_arch_get_private'
| libmodules.a(ofmem_common.o): In function `ofmem_free':
| /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:129: undefined 
reference to `ofmem_arch_get_private'
| libmodules.a(ofmem_common.o): In function `split_trans':
| /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:505: undefined 
reference to `ofmem_arch_get_private'
| libmodules.a(ofmem_common.o): In function `ofmem_claim_virt_':
| /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:422: undefined 
reference to `ofmem_arch_get_private'
| libmodules.a(ofmem_common.o): In function `ofmem_update_translations':
| /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:256: undefined 
reference to `ofmem_arch_get_private'
| 
libmodules.a(ofmem_common.o):/home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:35:
 more undefined references to `ofmem_arch_get_private' follow
| libmodules.a(ofmem_common.o): In function `unmap_page_range':
| /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:609: undefined 
reference to `ofmem_arch_unmap_pages'
| libmodules.a(ofmem_common.o): In function `ofmem_map_page_range':
| /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:524: undefined 
reference to `ofmem_arch_get_private'
| /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:551: undefined 
reference to `ofmem_arch_unmap_pages'
| libmodules.a(ofmem_common.o): In function `ofmem_map':
| /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:647: undefined 
reference to `ofmem_arch_default_translation_mode'
| /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:654: undefined 
reference to `ofmem_arch_early_map_pages'
| libmodules.a(ofmem_common.o): In function `ofmem_claim':
| /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:456: undefined 
reference to `ofmem_arch_get_private'
| libmodules.a(ofmem_common.o): In function `get_ram_size':
| /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:35: undefined 
reference to `ofmem_arch_get_private'
| /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:35: undefined 
reference to `ofmem_arch_get_private'
| /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:35: undefined 
reference to `ofmem_arch_get_private'
| libmodules.a(ofmem_common.o): In function `ofmem_claim_virt':
| /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:448: undefined 
reference to `ofmem_arch_get_virt_top'
| libmodules.a(ofmem_common.o): In function `ofmem_malloc':
| /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:83: undefined 
reference to `ofmem_arch_get_private'
| /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:92: undefined 
reference to `ofmem_arch_get_malloc_base'
| /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:108: undefined 
reference to `ofmem_arch_get_heap_top'
| collect2: ld returned 1 exit status
| make[1]: *** [openbios-unix] Error 1
| make[1]: Leaving directory `/home/aurel32/openbios-devel/obj-ppc'

This is something that needs to be fixed before an OpenBIOS release,
though I don't know if its planned to release OpenBIOS in sync with QEMU.

-- 
Aurelien Jarno  GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[malc]

On Sun, 6 Dec 2009, Markus Armbruster wrote:

 malc av1...@comtv.ru writes:
 
  On Sun, 6 Dec 2009, Markus Armbruster wrote:
 
  malc av1...@comtv.ru writes:
  
 
  [..snip..]
 
  
  read(fd, malloc(0), 0) is just fine, because read() doesn't touch the
  buffer when the size is zero.
  
 
  [..snip..]
 
  Yet under linux the address is checked even for zero case.
 
 Any value you can obtain from malloc() passes that check.
 
 Why does the fact that you can construct pointers that don't pass this
 check matter for our discussion of malloc()?
 
   I don't know what a valid pointer in this context represents.
  
  I can talk standardese, if you prefer :)
  
  malloc() either returns either a null pointer or a pointer to the
  allocated space.  In either case, you must not dereference the pointer.
  
  OpenBSD chooses to return a pointer to the allocated space.  It chooses
  to catch common ways to dereference the pointer.
  
  Your p = (void *)-1 is neither a null pointer nor can it point to
  allocated space on your particular system.  Hence, it cannot be a value
  of malloc() for any argument, and therefore what read() does with it on
  that particular system doesn't matter.
  
 
  Here, i believe, you are inventing artificial restrictions on how
  malloc behaves, i don't see anything that prevents the implementor
  from setting aside a range of addresses with 31st bit set as an
  indicator of zero allocations, and then happily giving it to the
  user of malloc and consumming it in free.
 
 Misunderstanding?  Such behavior is indeed permissible, and I can't see
 where I restricted it away.  An implementation that behaves as you
 describe returns pointer to allocated space.  That the pointer has
 some funny bit set doesn't matter.  That it can't be dereferenced is
 just fine.
 
 I'm not sure what your point is.  If it is that malloc(0) can return a
 value that cannot be passed to a zero-sized read(), then I fear you have
 not made your point.

One more attempt to make it clearer. If you agree that this behaviour
is permissible then the game is lost as things stand now under Linux,
since replacing [1]:

void *p = (void *) -1 
with:
void *p = (void *) 0x8000

or anything else with said bit set will yield EFAULT. Consequently the
code you cited as a well behaving malloc(0) call site will bomb.

[1] Under 32bit Linux that is, with the usual split.

-- 
mailto:av1...@comtv.ru

Re: [Qemu-devel] Re: [PATCH] Permit zero-sized qemu_malloc() friends

[malc]

On Sun, 6 Dec 2009, Paolo Bonzini wrote:

 On 12/06/2009 11:22 AM, malc wrote:
  Here, i believe, you are inventing artificial restrictions on how
  malloc behaves, i don't see anything that prevents the implementor
  from setting aside a range of addresses with 31st bit set as an
  indicator of zero allocations, and then happily giving it to the
  user of malloc and consumming it in free.
 
 But it has to make it a valid address anyway.  If a zero-sized read treats it
 as invalid (SIGSEGV, EFAULT, whatever), malloc has failed to return a valid
 address and is not obeying its specification.

Once again - standard doesn't speak about valid addresses.

-- 
mailto:av1...@comtv.ru

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[Avi Kivity]

On 12/06/2009 01:53 PM, malc wrote:

On Sun, 6 Dec 2009, Avi Kivity wrote:

   

On 12/06/2009 12:22 PM, malc wrote:
 

Here, i believe, you are inventing artificial restrictions on how
malloc behaves, i don't see anything that prevents the implementor
from setting aside a range of addresses with 31st bit set as an
indicator of zero allocations, and then happily giving it to the
user of malloc and consumming it in free.

   

The implementation needs to track which addresses it handed out, since it is
required that malloc(0) != malloc(0) (unless both are NULL).
 

You haven't read carefully, i said range.
   


I did in fact.  Consider a loop

  malloc(0);
  p = malloc(0);
  while (1) {
  n = malloc(0);
  free(p);
  p = n;
  }

without some form of tracking, you won't be able to return unique 
addresses eventually.


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

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[malc]

On Sun, 6 Dec 2009, Avi Kivity wrote:

 On 12/06/2009 01:53 PM, malc wrote:
  On Sun, 6 Dec 2009, Avi Kivity wrote:
  
 
   On 12/06/2009 12:22 PM, malc wrote:

Here, i believe, you are inventing artificial restrictions on how
malloc behaves, i don't see anything that prevents the implementor
from setting aside a range of addresses with 31st bit set as an
indicator of zero allocations, and then happily giving it to the
user of malloc and consumming it in free.

   
   The implementation needs to track which addresses it handed out, since it
   is
   required that malloc(0) != malloc(0) (unless both are NULL).

  You haven't read carefully, i said range.
 
 
 I did in fact.  Consider a loop
 
   malloc(0);
   p = malloc(0);
   while (1) {
   n = malloc(0);
   free(p);
   p = n;
   }
 
 without some form of tracking, you won't be able to return unique addresses
 eventually.

So? There will be tracking, it's the same as with OpenBSD where
allocations of zero and greater than zero are handled differently.

-- 
mailto:av1...@comtv.ru

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[Avi Kivity]

On 12/06/2009 02:11 PM, malc wrote:

The implementation needs to track which addresses it handed out, since it
is
required that malloc(0) != malloc(0) (unless both are NULL).

 

You haven't read carefully, i said range.

   



So? There will be tracking, it's the same as with OpenBSD where
allocations of zero and greater than zero are handled differently.
   



That's exactly what I said.  Some tracking is needed.

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

[Qemu-devel] [PATCH 1/2] multiboot: Support arbitrary number of modules.

[Adam Lackorzynski]

On Thu Dec 03, 2009 at 17:24:59 -0600, Anthony Liguori wrote:
 +struct MultibootState {
 +void *mb_buf; /* buffer holding kernel, cmdlines and mb_infos */
 +uint32_t mb_buf_phys; /* address in target */
 +int mb_buf_size;  /* size of mb_buf in bytes */
 +int mb_mods_avail;/* available slots for mb modules infos */
 +int mb_mods_count;/* currently used slots of mb modules */
 +int offset_mbinfo;/* offset of mb-info's in bytes */
 +int offset_cmdlines;  /* offset in buffer for cmdlines */
 +int offset_mods;  /* offset of modules in bytes */
 +};
 
 Should be typedef struct MultibootState.  See CODING_STYLE.  Also,
 shouldn't these offsets be something other than int?  Does multiboot
 always load in 32-bit mode (never 64-bit mode?).
 
 mb_buf_phys should not be a u32.  It should be a target address type.

The spec defines all values as u32 but since the offsets are relative to
mb_buf_phys I've made them of type target_phys_addr_t too.


Signed-off-by: Adam Lackorzynski a...@os.inf.tu-dresden.de
---
 hw/pc.c |  268 +++
 1 files changed, 167 insertions(+), 101 deletions(-)

diff --git a/hw/pc.c b/hw/pc.c
index 8c1b7ea..44f3193 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -51,6 +51,12 @@
 /* Show multiboot debug output */
 //#define DEBUG_MULTIBOOT
 
+#ifdef DEBUG_MULTIBOOT
+#define mb_debug(a...) fprintf(stderr, ## a)
+#else
+#define mb_debug(a...)
+#endif
+
 #define BIOS_FILENAME bios.bin
 
 #define PC_MAX_BIOS_SIZE (4 * 1024 * 1024)
@@ -512,6 +518,85 @@ static long get_file_size(FILE *f)
 #error multiboot struct needs to fit in 16 bit real mode
 #endif
 
+enum {
+/* Multiboot info */
+MBI_FLAGS   = 0,
+MBI_MEM_LOWER   = 4,
+MBI_MEM_UPPER   = 8,
+MBI_BOOT_DEVICE = 12,
+MBI_CMDLINE = 16,
+MBI_MODS_COUNT  = 20,
+MBI_MODS_ADDR   = 24,
+MBI_MMAP_ADDR   = 48,
+
+MBI_SIZE= 88,
+
+/* Multiboot modules */
+MB_MOD_START= 0,
+MB_MOD_END  = 4,
+MB_MOD_CMDLINE  = 8,
+
+MB_MOD_SIZE = 16,
+
+/* Region offsets */
+ADDR_E820_MAP = MULTIBOOT_STRUCT_ADDR + 0,
+ADDR_MBI  = ADDR_E820_MAP + 0x500,
+
+/* Multiboot flags */
+MULTIBOOT_FLAGS_MEMORY  = 1  0,
+MULTIBOOT_FLAGS_BOOT_DEVICE = 1  1,
+MULTIBOOT_FLAGS_CMDLINE = 1  2,
+MULTIBOOT_FLAGS_MODULES = 1  3,
+MULTIBOOT_FLAGS_MMAP= 1  6,
+};
+
+typedef struct {
+/* buffer holding kernel, cmdlines and mb_infos */
+void *mb_buf;
+/* address in target */
+target_phys_addr_t mb_buf_phys;
+/* size of mb_buf in bytes */
+unsigned mb_buf_size;
+/* offset of mb-info's in bytes */
+target_phys_addr_t offset_mbinfo;
+/* offset in buffer for cmdlines in bytes */
+target_phys_addr_t offset_cmdlines;
+/* offset of modules in bytes */
+target_phys_addr_t offset_mods;
+/* available slots for mb modules infos */
+int mb_mods_avail;
+/* currently used slots of mb modules */
+int mb_mods_count;
+} MultibootState;
+
+static uint32_t mb_add_cmdline(MultibootState *s, const char *cmdline)
+{
+int len = strlen(cmdline) + 1;
+target_phys_addr_t p = s-offset_cmdlines;
+
+pstrcpy((char *)s-mb_buf + p, len, cmdline);
+s-offset_cmdlines += len;
+return s-mb_buf_phys + p;
+}
+
+static void mb_add_mod(MultibootState *s,
+   target_phys_addr_t start, target_phys_addr_t end,
+   target_phys_addr_t cmdline_phys)
+{
+char *p;
+assert(s-mb_mods_count  s-mb_mods_avail);
+
+p = (char *)s-mb_buf + s-offset_mbinfo + MB_MOD_SIZE * s-mb_mods_count;
+
+stl_p(p + MB_MOD_START,   start);
+stl_p(p + MB_MOD_END, end);
+stl_p(p + MB_MOD_CMDLINE, cmdline_phys);
+
+mb_debug(mod%02d: %08x - %08x\n, s-mb_mods_count, start, end);
+
+s-mb_mods_count++;
+}
+
 static int load_multiboot(void *fw_cfg,
   FILE *f,
   const char *kernel_filename,
@@ -524,12 +609,8 @@ static int load_multiboot(void *fw_cfg,
 uint32_t mh_entry_addr;
 uint32_t mh_load_addr;
 uint32_t mb_kernel_size;
-uint32_t mmap_addr = MULTIBOOT_STRUCT_ADDR;
-uint32_t mb_bootinfo = MULTIBOOT_STRUCT_ADDR + 0x500;
-uint32_t mb_mod_end;
-uint8_t bootinfo[0x500];
-uint32_t cmdline = 0x200;
-uint8_t *mb_kernel_data;
+MultibootState mbs;
+uint8_t bootinfo[MBI_SIZE];
 uint8_t *mb_bootinfo_data;
 
 /* Ok, let's see if it is a multiboot image.
@@ -550,10 +631,9 @@ static int load_multiboot(void *fw_cfg,
 if (!is_multiboot)
 return 0; /* no multiboot */
 
-#ifdef DEBUG_MULTIBOOT
-fprintf(stderr, qemu: I believe we found a multiboot image!\n);
-#endif
+mb_debug(qemu: I believe we found a multiboot image!\n);
 memset(bootinfo, 0, sizeof(bootinfo));
+memset(mbs, 0, sizeof(mbs));
 
 if (flags  0x0004) { /* MULTIBOOT_HEADER_HAS_VBE */

[Qemu-devel] [PATCH 2/2] multiboot: Separate multiboot loading into separate file

[Adam Lackorzynski]

Move multiboot loading code into a separate files as suggested by Alex Graf.

Signed-off-by: Adam Lackorzynski a...@os.inf.tu-dresden.de
---
 Makefile.target |2 +-
 hw/multiboot.c  |  326 +++
 hw/multiboot.h  |   12 ++
 hw/pc.c |  300 +--
 4 files changed, 342 insertions(+), 298 deletions(-)
 create mode 100644 hw/multiboot.c
 create mode 100644 hw/multiboot.h

diff --git a/Makefile.target b/Makefile.target
index 7c1f30c..7f3e497 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -194,7 +194,7 @@ obj-i386-y += fdc.o mc146818rtc.o serial.o i8259.o i8254.o 
pcspk.o pc.o
 obj-i386-y += cirrus_vga.o apic.o ioapic.o parallel.o acpi.o piix_pci.o
 obj-i386-y += usb-uhci.o vmmouse.o vmport.o vmware_vga.o hpet.o
 obj-i386-y += device-hotplug.o pci-hotplug.o smbios.o wdt_ib700.o
-obj-i386-y += ne2000-isa.o
+obj-i386-y += ne2000-isa.o multiboot.o
 
 # shared objects
 obj-ppc-y = ppc.o ide/core.o ide/qdev.o ide/isa.o ide/pci.o ide/macio.o
diff --git a/hw/multiboot.c b/hw/multiboot.c
new file mode 100644
index 000..04d224e
--- /dev/null
+++ b/hw/multiboot.c
@@ -0,0 +1,326 @@
+/*
+ * QEMU PC System Emulator
+ *
+ * Copyright (c) 2003-2004 Fabrice Bellard
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the Software), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include hw.h
+#include fw_cfg.h
+#include multiboot.h
+#include loader.h
+#include elf.h
+#include sysemu.h
+
+/* Show multiboot debug output */
+//#define DEBUG_MULTIBOOT
+
+#ifdef DEBUG_MULTIBOOT
+#define mb_debug(a...) fprintf(stderr, ## a)
+#else
+#define mb_debug(a...)
+#endif
+
+#define MULTIBOOT_STRUCT_ADDR 0x9000
+
+#if MULTIBOOT_STRUCT_ADDR  0xf
+#error multiboot struct needs to fit in 16 bit real mode
+#endif
+
+enum {
+/* Multiboot info */
+MBI_FLAGS   = 0,
+MBI_MEM_LOWER   = 4,
+MBI_MEM_UPPER   = 8,
+MBI_BOOT_DEVICE = 12,
+MBI_CMDLINE = 16,
+MBI_MODS_COUNT  = 20,
+MBI_MODS_ADDR   = 24,
+MBI_MMAP_ADDR   = 48,
+
+MBI_SIZE= 88,
+
+/* Multiboot modules */
+MB_MOD_START= 0,
+MB_MOD_END  = 4,
+MB_MOD_CMDLINE  = 8,
+
+MB_MOD_SIZE = 16,
+
+/* Region offsets */
+ADDR_E820_MAP = MULTIBOOT_STRUCT_ADDR + 0,
+ADDR_MBI  = ADDR_E820_MAP + 0x500,
+
+/* Multiboot flags */
+MULTIBOOT_FLAGS_MEMORY  = 1  0,
+MULTIBOOT_FLAGS_BOOT_DEVICE = 1  1,
+MULTIBOOT_FLAGS_CMDLINE = 1  2,
+MULTIBOOT_FLAGS_MODULES = 1  3,
+MULTIBOOT_FLAGS_MMAP= 1  6,
+};
+
+typedef struct {
+/* buffer holding kernel, cmdlines and mb_infos */
+void *mb_buf;
+/* address in target */
+target_phys_addr_t mb_buf_phys;
+/* size of mb_buf in bytes */
+unsigned mb_buf_size;
+/* offset of mb-info's in bytes */
+target_phys_addr_t offset_mbinfo;
+/* offset in buffer for cmdlines in bytes */
+target_phys_addr_t offset_cmdlines;
+/* offset of modules in bytes */
+target_phys_addr_t offset_mods;
+/* available slots for mb modules infos */
+int mb_mods_avail;
+/* currently used slots of mb modules */
+int mb_mods_count;
+} MultibootState;
+
+static uint32_t mb_add_cmdline(MultibootState *s, const char *cmdline)
+{
+int len = strlen(cmdline) + 1;
+target_phys_addr_t p = s-offset_cmdlines;
+
+pstrcpy((char *)s-mb_buf + p, len, cmdline);
+s-offset_cmdlines += len;
+return s-mb_buf_phys + p;
+}
+
+static void mb_add_mod(MultibootState *s,
+   target_phys_addr_t start, target_phys_addr_t end,
+   target_phys_addr_t cmdline_phys)
+{
+char *p;
+assert(s-mb_mods_count  s-mb_mods_avail);
+
+p = (char *)s-mb_buf + s-offset_mbinfo + MB_MOD_SIZE * s-mb_mods_count;
+
+stl_p(p + MB_MOD_START,   start);
+stl_p(p + MB_MOD_END, end);
+stl_p(p + MB_MOD_CMDLINE, cmdline_phys);
+
+mb_debug(mod%02d: %08x - %08x\n, s-mb_mods_count, start, end);

[Qemu-devel] [PATCH v2 2/3] Cocoa: ppc64 host support

[Andreas Faerber]

Fix integer usage in the Cocoa backend: NSInteger is long on LP64.

http://developer.apple.com/mac/library/documentation/Cocoa/Reference/ApplicationKit/Classes/NSView_Class/Reference/NSView.html#//apple_ref/doc/uid/2014-BBCFHHCD

This makes the graphical display show up on a ppc64 host.

Signed-off-by: Andreas Faerber andreas.faer...@web.de
Cc: Alexander Graf a...@csgraf.de
Cc: Mike Kronenberg mike.kronenb...@kronenberg.org
---
 cocoa.m |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/cocoa.m b/cocoa.m
index 55ff2b4..7062571 100644
--- a/cocoa.m
+++ b/cocoa.m
@@ -337,7 +337,7 @@ int cocoa_keycode_to_qemu(int keycode)
 } else {
 // selective drawing code (draws only dirty rectangles) (OS X = 
10.4)
 const NSRect *rectList;
-int rectCount;
+NSInteger rectCount;
 int i;
 CGImageRef clipImageRef;
 CGRect clipRect;
-- 
1.6.5.3

[Qemu-devel] [PATCH v2 3/3] Cocoa: Silence warnings

[Andreas Faerber]

Missing static for cocoa_keycode_to_qemu.
Missing const for character constant.

__LITTLE_ENDIAN__ is undefined on Big Endian host.

Signed-off-by: Andreas Faerber andreas.faer...@web.de
Cc: John Arbuckle programmingk...@gmail.com
---
 cocoa.m |6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/cocoa.m b/cocoa.m
index 7062571..05f507d 100644
--- a/cocoa.m
+++ b/cocoa.m
@@ -229,7 +229,7 @@ int keymap[] =
 */
 };
 
-int cocoa_keycode_to_qemu(int keycode)
+static int cocoa_keycode_to_qemu(int keycode)
 {
 if((sizeof(keymap)/sizeof(int)) = keycode)
 {
@@ -315,7 +315,7 @@ int cocoa_keycode_to_qemu(int keycode)
 screen.bitsPerComponent, //bitsPerComponent
 screen.bitsPerPixel, //bitsPerPixel
 (screen.width * (screen.bitsPerComponent/2)), //bytesPerRow
-#if __LITTLE_ENDIAN__
+#ifdef __LITTLE_ENDIAN__
 CGColorSpaceCreateWithName(kCGColorSpaceGenericRGB), //colorspace 
for OS X = 10.4
 kCGBitmapByteOrder32Little | kCGImageAlphaNoneSkipFirst,
 #else
@@ -783,7 +783,7 @@ int cocoa_keycode_to_qemu(int keycode)
 if(returnCode == NSCancelButton) {
 exit(0);
 } else if(returnCode == NSOKButton) {
-char *bin = qemu;
+const char *bin = qemu;
 char *img = (char*)[ [ sheet filename ] 
cStringUsingEncoding:NSASCIIStringEncoding];
 
 char **argv = (char**)malloc( sizeof(char*)*3 );
-- 
1.6.5.3

[Qemu-devel] [PATCH v2 1/3] TCG: Mac OS X support for ppc64 target

[Andreas Faerber]

Darwin/ppc64 does not use function descriptors,
adapt prologue and tcg_out_call accordingly.
GPR2 is available for general use, so let's use it.

http://developer.apple.com/mac/library/documentation/DeveloperTools/Conceptual/LowLevelABI/110-64-bit_PowerPC_Function_Calling_Conventions/64bitPowerPC.html

v2:
- Don't mark reserved GPR13 as callee-save.
- Move tcg_out_b up.
- Fix unused variable warning in prologue.

Signed-off-by: Andreas Faerber andreas.faer...@web.de
Cc: malc av1...@comtv.ru
---
 tcg/ppc64/tcg-target.c |   55 +++
 1 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/tcg/ppc64/tcg-target.c b/tcg/ppc64/tcg-target.c
index a612e10..0c11917 100644
--- a/tcg/ppc64/tcg-target.c
+++ b/tcg/ppc64/tcg-target.c
@@ -104,6 +104,9 @@ static const int tcg_target_reg_alloc_order[] = {
 TCG_REG_R29,
 TCG_REG_R30,
 TCG_REG_R31,
+#ifdef __APPLE__
+TCG_REG_R2,
+#endif
 TCG_REG_R3,
 TCG_REG_R4,
 TCG_REG_R5,
@@ -112,7 +115,9 @@ static const int tcg_target_reg_alloc_order[] = {
 TCG_REG_R8,
 TCG_REG_R9,
 TCG_REG_R10,
+#ifndef __APPLE__
 TCG_REG_R11,
+#endif
 TCG_REG_R12,
 TCG_REG_R24,
 TCG_REG_R25,
@@ -136,6 +141,9 @@ static const int tcg_target_call_oarg_regs[2] = {
 };
 
 static const int tcg_target_callee_save_regs[] = {
+#ifdef __APPLE__
+TCG_REG_R11,
+#endif
 TCG_REG_R14,
 TCG_REG_R15,
 TCG_REG_R16,
@@ -477,8 +485,31 @@ static void tcg_out_movi (TCGContext *s, TCGType type,
 }
 }
 
+static void tcg_out_b (TCGContext *s, int mask, tcg_target_long target)
+{
+tcg_target_long disp;
+
+disp = target - (tcg_target_long) s-code_ptr;
+if ((disp  38)  38 == disp)
+tcg_out32 (s, B | (disp  0x3fc) | mask);
+else {
+tcg_out_movi (s, TCG_TYPE_I64, 0, (tcg_target_long) target);
+tcg_out32 (s, MTSPR | RS (0) | CTR);
+tcg_out32 (s, BCCTR | BO_ALWAYS | mask);
+}
+}
+
 static void tcg_out_call (TCGContext *s, tcg_target_long arg, int const_arg)
 {
+#ifdef __APPLE__
+if (const_arg) {
+tcg_out_b (s, LK, arg);
+}
+else {
+tcg_out32 (s, MTSPR | RS (arg) | LR);
+tcg_out32 (s, BCLR | BO_ALWAYS | LK);
+}
+#else
 int reg;
 
 if (const_arg) {
@@ -492,6 +523,7 @@ static void tcg_out_call (TCGContext *s, tcg_target_long 
arg, int const_arg)
 tcg_out32 (s, LD | RT (11) | RA (reg) | 16);
 tcg_out32 (s, LD | RT (2) | RA (reg) | 8);
 tcg_out32 (s, BCCTR | BO_ALWAYS | LK);
+#endif
 }
 
 static void tcg_out_ldst (TCGContext *s, int ret, int addr,
@@ -516,20 +548,6 @@ static void tcg_out_ldsta (TCGContext *s, int ret, int 
addr,
 }
 }
 
-static void tcg_out_b (TCGContext *s, int mask, tcg_target_long target)
-{
-tcg_target_long disp;
-
-disp = target - (tcg_target_long) s-code_ptr;
-if ((disp  38)  38 == disp)
-tcg_out32 (s, B | (disp  0x3fc) | mask);
-else {
-tcg_out_movi (s, TCG_TYPE_I64, 0, (tcg_target_long) target);
-tcg_out32 (s, MTSPR | RS (0) | CTR);
-tcg_out32 (s, BCCTR | BO_ALWAYS | mask);
-}
-}
-
 #if defined (CONFIG_SOFTMMU)
 
 #include ../../softmmu_defs.h
@@ -845,7 +863,9 @@ static void tcg_out_qemu_st (TCGContext *s, const TCGArg 
*args, int opc)
 void tcg_target_qemu_prologue (TCGContext *s)
 {
 int i, frame_size;
+#ifndef __APPLE__
 uint64_t addr;
+#endif
 
 frame_size = 0
 + 8 /* back chain */
@@ -859,10 +879,12 @@ void tcg_target_qemu_prologue (TCGContext *s)
 ;
 frame_size = (frame_size + 15)  ~15;
 
+#ifndef __APPLE__
 /* First emit adhoc function descriptor */
 addr = (uint64_t) s-code_ptr + 24;
 tcg_out32 (s, addr  32); tcg_out32 (s, addr); /* entry point */
 s-code_ptr += 16;  /* skip TOC and environment pointer */
+#endif
 
 /* Prologue */
 tcg_out32 (s, MFSPR | RT (0) | LR);
@@ -1516,6 +1538,9 @@ void tcg_target_init (TCGContext *s)
 tcg_regset_set32 (tcg_target_available_regs[TCG_TYPE_I64], 0, 0x);
 tcg_regset_set32 (tcg_target_call_clobber_regs, 0,
  (1  TCG_REG_R0) |
+#ifdef __APPLE__
+ (1  TCG_REG_R2) |
+#endif
  (1  TCG_REG_R3) |
  (1  TCG_REG_R4) |
  (1  TCG_REG_R5) |
@@ -1531,7 +1556,9 @@ void tcg_target_init (TCGContext *s)
 tcg_regset_clear (s-reserved_regs);
 tcg_regset_set_reg (s-reserved_regs, TCG_REG_R0);
 tcg_regset_set_reg (s-reserved_regs, TCG_REG_R1);
+#ifndef __APPLE__
 tcg_regset_set_reg (s-reserved_regs, TCG_REG_R2);
+#endif
 tcg_regset_set_reg (s-reserved_regs, TCG_REG_R13);
 
 #ifdef CONFIG_USE_GUEST_BASE
-- 
1.6.5.3

[Qemu-devel] Re: [PATCH v2 2/3] Cocoa: ppc64 host support

[Alexander Graf]

On 06.12.2009, at 14:00, Andreas Faerber wrote:

 Fix integer usage in the Cocoa backend: NSInteger is long on LP64.
 
 http://developer.apple.com/mac/library/documentation/Cocoa/Reference/ApplicationKit/Classes/NSView_Class/Reference/NSView.html#//apple_ref/doc/uid/2014-BBCFHHCD
 
 This makes the graphical display show up on a ppc64 host.

Interesting! Unfortunately I don't have a PPC64 Apple machine handy, so I can't 
test it.

Alex

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[Avi Kivity]

On 12/06/2009 01:25 AM, Ian Molton wrote:

Avi Kivity wrote:

   

It's not that it doesn't have a way to report failure, it's that it
doesn't fail.  Do you prefer functions that fail and report it to
functions that don't fail?
 

You have a way of allocating memory that will _never_ fail?

   


Sort of.  Did you look at the code?


Seriously, who does that anyway? why call malloc when you dont want the
space? so you can use realloc? 99.99% of the time realloc() is the Wrong
Solution(tm).

   

Read the beginning of the thread.  Basically it's for arrays, malloc(n *
sizeof(x)).
 

well, make sure n is not 0. Its not that hard. I dont think I've *ever*
had a situation where I wanted to pass 0 to malloc.
   


There are multiple such cases in the code.


stick to what people know, and LART them for misuse of it if necessary.
   

The LART is a crash, great.
 

No, the LART would be a 'your patch does this wrong, try this:'
   


What about existing usage?  Will you audit all the existing calls?

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

[Qemu-devel] Re: [PATCH 1/3] TCG: Mac OS X support for ppc64 target

[Andreas Färber]

Am 06.12.2009 um 07:28 schrieb malc:


On Sun, 6 Dec 2009, Andreas F?rber wrote:



Am 06.12.2009 um 06:13 schrieb malc:


On Sun, 6 Dec 2009, Andreas Faerber wrote:


Darwin/ppc64 does not use function descriptors,
adapt prologue and tcg_out_call accordingly.
GPR2 is available for general use, so let's use it.

http://developer.apple.com/mac/library/documentation/DeveloperTools/Conceptual/LowLevelABI/110-64-bit_PowerPC_Function_Calling_Conventions/64bitPowerPC.html

Signed-off-by: Andreas Faerber andreas.faer...@web.de
Cc: malc av1...@comtv.ru
---
tcg/ppc64/tcg-target.c |   30 ++
1 files changed, 30 insertions(+), 0 deletions(-)

diff --git a/tcg/ppc64/tcg-target.c b/tcg/ppc64/tcg-target.c
index a612e10..bf9b7d9 100644
--- a/tcg/ppc64/tcg-target.c
+++ b/tcg/ppc64/tcg-target.c



Otherwise looks good.. Should i commit it with R13 fixed?


About the callee-save stuff I was less certain. Feel free to make
modifications (e.g., moving tcg_out_call up?) or have me resubmit.


Sorry, i don't get this part, i was just thinking of removing R13 from
the list. Moving tcg_out_call?


Sorry, I meant tcg_out_b. Just declaring it was the least intrusive  
patch.


I've sent a v2 series, doing the above plus silencing a warning.
They are also available for pulling from git://repo.or.cz/qemu/ 
afaerber.git, branch tcg-osx-ppc64.


Andreas

[Qemu-devel] Re: [PATCH v2 2/3] Cocoa: ppc64 host support

[Andreas Färber]

Hi Alex,

Am 06.12.2009 um 14:02 schrieb Alexander Graf:



On 06.12.2009, at 14:00, Andreas Faerber wrote:


Fix integer usage in the Cocoa backend: NSInteger is long on LP64.

http://developer.apple.com/mac/library/documentation/Cocoa/Reference/ApplicationKit/Classes/NSView_Class/Reference/NSView.html#/ 
/apple_ref/doc/uid/2014-BBCFHHCD


This makes the graphical display show up on a ppc64 host.


Interesting! Unfortunately I don't have a PPC64 Apple machine handy,  
so I can't test it.


I cc'ed you so that you could check your earlier x86_64 Mac patch  
series, if you care. I don't have an x86_64 Mac at hands myself.


Btw I'll be looking into bringing the Cocoa frontend on par with SDL,  
namely closing the app when closing the window and not creating the  
window in -nographic mode. That's awful when debugging!


Andreas

Re: [Qemu-devel] [PATCH] Adds missing function prototype int cocoa_keycode_to_qemu(int keycode) to cocoa.m

[Andreas Färber]

Am 13.07.2009 um 22:37 schrieb G 3:

This patch adds the missing function prototype cocoa_keycode_to_qemu  
to cocoa.m.


Signed-off-by: John Arbuckle programmingk...@gmail.com

---
cocoa.m |1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/cocoa.m b/cocoa.m
index 3bf1a87..1e5fbe8 100644
--- a/cocoa.m
+++ b/cocoa.m
@@ -62,6 +62,7 @@ typedef struct {
} QEMUScreen;

int qemu_main(int argc, char **argv); // main defined in qemu/vl.c
+int cocoa_keycode_to_qemu(int keycode);
NSWindow *normalWindow;
id cocoaView;
static DisplayChangeListener *dcl;


I've fixed that warning slightly different in my recent patch: We  
don't appear to need a declaration, adding static to the definition is  
sufficient.


Andreas

[Qemu-devel] Re: [PATCH v2 2/3] Cocoa: ppc64 host support

[Alexander Graf]

On 06.12.2009, at 14:23, Andreas Färber wrote:

 Hi Alex,
 
 Am 06.12.2009 um 14:02 schrieb Alexander Graf:
 
 
 On 06.12.2009, at 14:00, Andreas Faerber wrote:
 
 Fix integer usage in the Cocoa backend: NSInteger is long on LP64.
 
 http://developer.apple.com/mac/library/documentation/Cocoa/Reference/ApplicationKit/Classes/NSView_Class/Reference/NSView.html#//apple_ref/doc/uid/2014-BBCFHHCD
 
 This makes the graphical display show up on a ppc64 host.
 
 Interesting! Unfortunately I don't have a PPC64 Apple machine handy, so I 
 can't test it.
 
 I cc'ed you so that you could check your earlier x86_64 Mac patch series, if 
 you care. I don't have an x86_64 Mac at hands myself.

Oh I see, I'll test it later today or tomorrow.

 Btw I'll be looking into bringing the Cocoa frontend on par with SDL, namely 
 closing the app when closing the window and not creating the window in 
 -nographic mode. That's awful when debugging!

I agree. I did a patch for that once - might be worth searching the ML for 
that. Basically it searched the args for a -nographic parameter. Pretty 
hacky, but at least it worked.

Alex

[Qemu-devel] Re: [PATCH v2 2/3] Cocoa: ppc64 host support

[Mike Kronenberg]

On 06.12.2009, at 14:00, Andreas Faerber wrote:

 -int rectCount;
 +NSInteger rectCount;

I know that this is endorsed by apple since 10.5 but NSInteger will break 
compiling on Tiger and older. Int on the other hand is only throwing a warning 
on Leopard if I'm not mistaken.

Especially with qemu, one has to have an eye on the type... NSInteger can be an 
int or a long, depending on the host...

#if __LP64__ || NS_BUILD_32_LIKE_64
typedef long NSInteger;
typedef unsigned long NSUInteger;
#else
typedef int NSInteger;
typedef unsigned int NSUInteger;
#endif

I have no G5 at hand to test either.

Mike

smime.p7s
Description: S/MIME cryptographic signature

[Qemu-devel] Re: [PATCH v2 2/3] Cocoa: ppc64 host support

[Andreas Färber]

Am 06.12.2009 um 14:41 schrieb Mike Kronenberg:


On 06.12.2009, at 14:00, Andreas Faerber wrote:


-int rectCount;
+NSInteger rectCount;


I know that this is endorsed by apple since 10.5 but NSInteger will  
break compiling on Tiger and older.


You appear to be right there... no trace of it in 10.4u SDK.

Int on the other hand is only throwing a warning on Leopard if I'm  
not mistaken.


On Leopard ppc it just warns and works because NSInteger == int.
On Leopard ppc64 it warns and fails at runtime because NSInteger != int.

Especially with qemu, one has to have an eye on the type...  
NSInteger can be an int or a long, depending on the host...


#if __LP64__ || NS_BUILD_32_LIKE_64
typedef long NSInteger;
typedef unsigned long NSUInteger;
#else
typedef int NSInteger;
typedef unsigned int NSUInteger;
#endif


Right, so we do need NSInteger - I'll add a version check. Thanks for  
the feedback!


Andreas

Re: [Qemu-devel] Socket reconnection.

[Jamie Lokier]

Ian Molton wrote:
 Jamie Lokier wrote:
 
  I'm a bit puzzled.
  
  Why isn't virtio-rng getting entropy from /dev/random on the host?
 
 /dev/random may not be available.

Understood on a non-Linux host.

 Besides, not all entropy comes from /dev/random.

On a Linux host, why isn't rngd simply injecting it's entropy into
/dev/random where it would be more convenient to access?  (No need for
socket reconnection code, for example).

-- Jamie

[Qemu-devel] [PATCH] kvm: x86: Fix initial kvm_has_msr_star

[Jan Kiszka]

KVM_GET_MSR_INDEX_LIST returns -E2BIG when the provided space is too
small for all MSRs. But this is precisely the error we trigger with the
initial request in order to obtain that size. Do not fail in that case.

This caused a subtle corruption of the guest state as MSR_STAR was not
properly saved/restored. The corruption became visible with latest kvm
optimizing the MSR updates.

Signed-off-by: Jan Kiszka jan.kis...@siemens.com
---

 target-i386/kvm.c |4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 3b61a7f..88b504c 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -244,9 +244,9 @@ static int kvm_has_msr_star(CPUState *env)
  * save/restore */
 msr_list.nmsrs = 0;
 ret = kvm_ioctl(env-kvm_state, KVM_GET_MSR_INDEX_LIST, msr_list);
-if (ret  0)
+if (ret  0  ret != -E2BIG) {
 return 0;
-
+}
 /* Old kernel modules had a bug and could write beyond the provided
memory. Allocate at least a safe amount of 1K. */
 kvm_msr_list = qemu_mallocz(MAX(1024, sizeof(msr_list) +



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [FOR 0.12][FOR 0.11][PATCH] kvm: x86: Fix initial kvm_has_msr_star

[Jan Kiszka]

Sorry, this is of course a critical fix for all branches that have KVM
support.

Jan

Jan Kiszka wrote:
 KVM_GET_MSR_INDEX_LIST returns -E2BIG when the provided space is too
 small for all MSRs. But this is precisely the error we trigger with the
 initial request in order to obtain that size. Do not fail in that case.
 
 This caused a subtle corruption of the guest state as MSR_STAR was not
 properly saved/restored. The corruption became visible with latest kvm
 optimizing the MSR updates.
 
 Signed-off-by: Jan Kiszka jan.kis...@siemens.com
 ---
 
  target-i386/kvm.c |4 ++--
  1 files changed, 2 insertions(+), 2 deletions(-)
 
 diff --git a/target-i386/kvm.c b/target-i386/kvm.c
 index 3b61a7f..88b504c 100644
 --- a/target-i386/kvm.c
 +++ b/target-i386/kvm.c
 @@ -244,9 +244,9 @@ static int kvm_has_msr_star(CPUState *env)
   * save/restore */
  msr_list.nmsrs = 0;
  ret = kvm_ioctl(env-kvm_state, KVM_GET_MSR_INDEX_LIST, msr_list);
 -if (ret  0)
 +if (ret  0  ret != -E2BIG) {
  return 0;
 -
 +}
  /* Old kernel modules had a bug and could write beyond the provided
 memory. Allocate at least a safe amount of 1K. */
  kvm_msr_list = qemu_mallocz(MAX(1024, sizeof(msr_list) +
 




signature.asc
Description: OpenPGP digital signature

[Qemu-devel] Re: Bugs in kvm guest migration

[Jan Kiszka]

Jan Kiszka wrote:
 Avi Kivity wrote:
 On 12/03/2009 09:23 PM, Jan Kiszka wrote:
 Jan Kiszka wrote:

 Problem 2
 -
 Setup: qemu head with vmstate fixes. kvm-kmod master, 64-bit host  guest.
 Effect: The migration target either locks up or reboots immediately.
 I've nailed this down to 84d0b66c778d881eafca2a5d0d66678211c4e861. Every
 kvm module build before that works, everything including and after
 26ede77f536d1bb369527a96c7fe7fdc8ba2f890 shows the effect (everything in
 between crashes the host for known reasons). It's still unclear if this
 is an kvm-kmod wrapping issue of the user-space return notifiers. Will
 check once problem #1 is understood.
  
 It is kernel-related, using kvm.git as host kernel makes no difference.

 This may now mean that kvm is buggy or that it triggers some msr
 save/restore related issues in qemu. Digging even deeper...

 Does calling drop_user_return_notifiers() (static in x86.c, will need 
 exporting) in vmx_load_host_state() within the preempt-disable region help?

 
 Nope.
 

It was a qemu bug, resolved by KVM: x86: Fix initial kvm_has_msr_star.

Jan



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] kvm: x86: Fix initial kvm_has_msr_star

[Avi Kivity]

On 12/06/2009 04:51 PM, Jan Kiszka wrote:

KVM_GET_MSR_INDEX_LIST returns -E2BIG when the provided space is too
small for all MSRs. But this is precisely the error we trigger with the
initial request in order to obtain that size. Do not fail in that case.

This caused a subtle corruption of the guest state as MSR_STAR was not
properly saved/restored. The corruption became visible with latest kvm
optimizing the MSR updates.
   


Strong ack.  Anthony, please apply ASAP.  This is worthy of a 0.11.2, IMO.

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

[Qemu-devel] [PATCH v3 2/3] Cocoa: ppc64 host support

[Andreas Faerber]

Fix integer usage in the Cocoa backend: NSInteger is long on LP64.

http://developer.apple.com/mac/library/documentation/Cocoa/Reference/ApplicationKit/Classes/NSView_Class/Reference/NSView.html#//apple_ref/doc/uid/2014-BBCFHHCD

This makes the graphical display show up on a ppc64 host.

v3:
- Confine NSInteger to Mac OS X v10.5 and later

Signed-off-by: Andreas Faerber andreas.faer...@web.de
Cc: Alexander Graf a...@csgraf.de
Cc: Mike Kronenberg mike.kronenb...@kronenberg.org
Cc: John Arbuckle programmingk...@gmail.com
---
 cocoa.m |8 
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/cocoa.m b/cocoa.m
index 55ff2b4..989efd5 100644
--- a/cocoa.m
+++ b/cocoa.m
@@ -28,6 +28,10 @@
 #include console.h
 #include sysemu.h
 
+#ifndef MAC_OS_X_VERSION_10_5
+#define MAC_OS_X_VERSION_10_5 1050
+#endif
+
 
 //#define DEBUG
 
@@ -337,7 +341,11 @@ int cocoa_keycode_to_qemu(int keycode)
 } else {
 // selective drawing code (draws only dirty rectangles) (OS X = 
10.4)
 const NSRect *rectList;
+#if (MAC_OS_X_VERSION_MAX_ALLOWED = MAC_OS_X_VERSION_10_5)
+NSInteger rectCount;
+#else
 int rectCount;
+#endif
 int i;
 CGImageRef clipImageRef;
 CGRect clipRect;
-- 
1.6.5.3

[Qemu-devel] [PATCH v3 3/3] Cocoa: Silence warnings

[Andreas Faerber]

Missing static for cocoa_keycode_to_qemu.
Missing const for character constant.

__LITTLE_ENDIAN__ is undefined on Big Endian host.
MAC_OS_X_VERSION_10_4 is undefined on v10.3 and earlier.

v3:
- Silence warnings reported from Mac OS X v10.3.9

Signed-off-by: Andreas Faerber andreas.faer...@web.de
Cc: John Arbuckle programmingk...@gmail.com
Cc: Mike Kronenberg mike.kronenb...@kronenberg.org
---
 cocoa.m |9 ++---
 1 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/cocoa.m b/cocoa.m
index 989efd5..ae2fd86 100644
--- a/cocoa.m
+++ b/cocoa.m
@@ -28,6 +28,9 @@
 #include console.h
 #include sysemu.h
 
+#ifndef MAC_OS_X_VERSION_10_4
+#define MAC_OS_X_VERSION_10_4 1040
+#endif
 #ifndef MAC_OS_X_VERSION_10_5
 #define MAC_OS_X_VERSION_10_5 1050
 #endif
@@ -233,7 +236,7 @@ int keymap[] =
 */
 };
 
-int cocoa_keycode_to_qemu(int keycode)
+static int cocoa_keycode_to_qemu(int keycode)
 {
 if((sizeof(keymap)/sizeof(int)) = keycode)
 {
@@ -319,7 +322,7 @@ int cocoa_keycode_to_qemu(int keycode)
 screen.bitsPerComponent, //bitsPerComponent
 screen.bitsPerPixel, //bitsPerPixel
 (screen.width * (screen.bitsPerComponent/2)), //bytesPerRow
-#if __LITTLE_ENDIAN__
+#ifdef __LITTLE_ENDIAN__
 CGColorSpaceCreateWithName(kCGColorSpaceGenericRGB), //colorspace 
for OS X = 10.4
 kCGBitmapByteOrder32Little | kCGImageAlphaNoneSkipFirst,
 #else
@@ -791,7 +794,7 @@ int cocoa_keycode_to_qemu(int keycode)
 if(returnCode == NSCancelButton) {
 exit(0);
 } else if(returnCode == NSOKButton) {
-char *bin = qemu;
+const char *bin = qemu;
 char *img = (char*)[ [ sheet filename ] 
cStringUsingEncoding:NSASCIIStringEncoding];
 
 char **argv = (char**)malloc( sizeof(char*)*3 );
-- 
1.6.5.3

[Qemu-devel] [PATCH v3 1/3] TCG: Mac OS X support for ppc64 target

[Andreas Faerber]

Darwin/ppc64 does not use function descriptors,
adapt prologue and tcg_out_call accordingly.
GPR2 is available for general use, so let's use it.

http://developer.apple.com/mac/library/documentation/DeveloperTools/Conceptual/LowLevelABI/110-64-bit_PowerPC_Function_Calling_Conventions/64bitPowerPC.html

v2:
- Don't mark reserved GPR13 as callee-save.
- Move tcg_out_b up.
- Fix unused variable warning in prologue.

Signed-off-by: Andreas Faerber andreas.faer...@web.de
Cc: malc av1...@comtv.ru
---
 tcg/ppc64/tcg-target.c |   55 +++
 1 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/tcg/ppc64/tcg-target.c b/tcg/ppc64/tcg-target.c
index a612e10..0c11917 100644
--- a/tcg/ppc64/tcg-target.c
+++ b/tcg/ppc64/tcg-target.c
@@ -104,6 +104,9 @@ static const int tcg_target_reg_alloc_order[] = {
 TCG_REG_R29,
 TCG_REG_R30,
 TCG_REG_R31,
+#ifdef __APPLE__
+TCG_REG_R2,
+#endif
 TCG_REG_R3,
 TCG_REG_R4,
 TCG_REG_R5,
@@ -112,7 +115,9 @@ static const int tcg_target_reg_alloc_order[] = {
 TCG_REG_R8,
 TCG_REG_R9,
 TCG_REG_R10,
+#ifndef __APPLE__
 TCG_REG_R11,
+#endif
 TCG_REG_R12,
 TCG_REG_R24,
 TCG_REG_R25,
@@ -136,6 +141,9 @@ static const int tcg_target_call_oarg_regs[2] = {
 };
 
 static const int tcg_target_callee_save_regs[] = {
+#ifdef __APPLE__
+TCG_REG_R11,
+#endif
 TCG_REG_R14,
 TCG_REG_R15,
 TCG_REG_R16,
@@ -477,8 +485,31 @@ static void tcg_out_movi (TCGContext *s, TCGType type,
 }
 }
 
+static void tcg_out_b (TCGContext *s, int mask, tcg_target_long target)
+{
+tcg_target_long disp;
+
+disp = target - (tcg_target_long) s-code_ptr;
+if ((disp  38)  38 == disp)
+tcg_out32 (s, B | (disp  0x3fc) | mask);
+else {
+tcg_out_movi (s, TCG_TYPE_I64, 0, (tcg_target_long) target);
+tcg_out32 (s, MTSPR | RS (0) | CTR);
+tcg_out32 (s, BCCTR | BO_ALWAYS | mask);
+}
+}
+
 static void tcg_out_call (TCGContext *s, tcg_target_long arg, int const_arg)
 {
+#ifdef __APPLE__
+if (const_arg) {
+tcg_out_b (s, LK, arg);
+}
+else {
+tcg_out32 (s, MTSPR | RS (arg) | LR);
+tcg_out32 (s, BCLR | BO_ALWAYS | LK);
+}
+#else
 int reg;
 
 if (const_arg) {
@@ -492,6 +523,7 @@ static void tcg_out_call (TCGContext *s, tcg_target_long 
arg, int const_arg)
 tcg_out32 (s, LD | RT (11) | RA (reg) | 16);
 tcg_out32 (s, LD | RT (2) | RA (reg) | 8);
 tcg_out32 (s, BCCTR | BO_ALWAYS | LK);
+#endif
 }
 
 static void tcg_out_ldst (TCGContext *s, int ret, int addr,
@@ -516,20 +548,6 @@ static void tcg_out_ldsta (TCGContext *s, int ret, int 
addr,
 }
 }
 
-static void tcg_out_b (TCGContext *s, int mask, tcg_target_long target)
-{
-tcg_target_long disp;
-
-disp = target - (tcg_target_long) s-code_ptr;
-if ((disp  38)  38 == disp)
-tcg_out32 (s, B | (disp  0x3fc) | mask);
-else {
-tcg_out_movi (s, TCG_TYPE_I64, 0, (tcg_target_long) target);
-tcg_out32 (s, MTSPR | RS (0) | CTR);
-tcg_out32 (s, BCCTR | BO_ALWAYS | mask);
-}
-}
-
 #if defined (CONFIG_SOFTMMU)
 
 #include ../../softmmu_defs.h
@@ -845,7 +863,9 @@ static void tcg_out_qemu_st (TCGContext *s, const TCGArg 
*args, int opc)
 void tcg_target_qemu_prologue (TCGContext *s)
 {
 int i, frame_size;
+#ifndef __APPLE__
 uint64_t addr;
+#endif
 
 frame_size = 0
 + 8 /* back chain */
@@ -859,10 +879,12 @@ void tcg_target_qemu_prologue (TCGContext *s)
 ;
 frame_size = (frame_size + 15)  ~15;
 
+#ifndef __APPLE__
 /* First emit adhoc function descriptor */
 addr = (uint64_t) s-code_ptr + 24;
 tcg_out32 (s, addr  32); tcg_out32 (s, addr); /* entry point */
 s-code_ptr += 16;  /* skip TOC and environment pointer */
+#endif
 
 /* Prologue */
 tcg_out32 (s, MFSPR | RT (0) | LR);
@@ -1516,6 +1538,9 @@ void tcg_target_init (TCGContext *s)
 tcg_regset_set32 (tcg_target_available_regs[TCG_TYPE_I64], 0, 0x);
 tcg_regset_set32 (tcg_target_call_clobber_regs, 0,
  (1  TCG_REG_R0) |
+#ifdef __APPLE__
+ (1  TCG_REG_R2) |
+#endif
  (1  TCG_REG_R3) |
  (1  TCG_REG_R4) |
  (1  TCG_REG_R5) |
@@ -1531,7 +1556,9 @@ void tcg_target_init (TCGContext *s)
 tcg_regset_clear (s-reserved_regs);
 tcg_regset_set_reg (s-reserved_regs, TCG_REG_R0);
 tcg_regset_set_reg (s-reserved_regs, TCG_REG_R1);
+#ifndef __APPLE__
 tcg_regset_set_reg (s-reserved_regs, TCG_REG_R2);
+#endif
 tcg_regset_set_reg (s-reserved_regs, TCG_REG_R13);
 
 #ifdef CONFIG_USE_GUEST_BASE
-- 
1.6.5.3

Re: [Qemu-devel] Staging update (0.12 pending freeze)

[Blue Swirl]

On Sun, Dec 6, 2009 at 11:59 AM, Aurelien Jarno aurel...@aurel32.net wrote:
 On Sat, Dec 05, 2009 at 08:07:13PM +, Blue Swirl wrote:
 On Sat, Dec 5, 2009 at 8:05 PM, Aurelien Jarno aurel...@aurel32.net wrote:
  On Thu, Dec 03, 2009 at 10:03:18PM +0200, Blue Swirl wrote:
  On Thu, Dec 3, 2009 at 9:26 PM, Aurelien Jarno aurel...@aurel32.net 
  wrote:
   On Wed, Dec 02, 2009 at 10:46:11AM -0600, Anthony Liguori wrote:
   I've got all of the patches I'm considering for 0.12 currently in
   staging.  I'm going to work through and test/commit these in a few
   chunks over the next few days before freezing the tree.
  
  
   What are the plans on the OpenBIOS side? The version currently included
   in QEMU is old compared to the SVN. Is it plan to sync a release of
   OpenBIOS with QEMU?
 
  At least the images should be updated.
 
 
  Now that version 0.12.0-rca has been tagged, we should probably do that
  asap. Should I do it?

 Please do.


 I have seen you have been faster than me, thanks.

 Anyway I am not able to fully build the powerpc images here,
 openbios-unix fails to build with:

 | libmodules.a(elf-loader.o): In function `elf_loader_init_program':
 | /home/aurel32/openbios-devel/obj-ppc/../modules/elf-loader.c:77: undefined 
 reference to `flush_icache_range'
 | libmodules.a(xcoff-loader.o): In function `xcoff_loader_init_program':
 | /home/aurel32/openbios-devel/obj-ppc/../modules/xcoff-loader.c:110: 
 undefined reference to `flush_icache_range'
 | libmodules.a(ofmem_common.o): In function `ofmem_translate':
 | /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:680: 
 undefined reference to `ofmem_arch_get_private'
 | libmodules.a(ofmem_common.o): In function `ofmem_free':
 | /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:129: 
 undefined reference to `ofmem_arch_get_private'
 | libmodules.a(ofmem_common.o): In function `split_trans':
 | /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:505: 
 undefined reference to `ofmem_arch_get_private'
 | libmodules.a(ofmem_common.o): In function `ofmem_claim_virt_':
 | /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:422: 
 undefined reference to `ofmem_arch_get_private'
 | libmodules.a(ofmem_common.o): In function `ofmem_update_translations':
 | /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:256: 
 undefined reference to `ofmem_arch_get_private'
 | 
 libmodules.a(ofmem_common.o):/home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:35:
  more undefined references to `ofmem_arch_get_private' follow
 | libmodules.a(ofmem_common.o): In function `unmap_page_range':
 | /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:609: 
 undefined reference to `ofmem_arch_unmap_pages'
 | libmodules.a(ofmem_common.o): In function `ofmem_map_page_range':
 | /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:524: 
 undefined reference to `ofmem_arch_get_private'
 | /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:551: 
 undefined reference to `ofmem_arch_unmap_pages'
 | libmodules.a(ofmem_common.o): In function `ofmem_map':
 | /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:647: 
 undefined reference to `ofmem_arch_default_translation_mode'
 | /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:654: 
 undefined reference to `ofmem_arch_early_map_pages'
 | libmodules.a(ofmem_common.o): In function `ofmem_claim':
 | /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:456: 
 undefined reference to `ofmem_arch_get_private'
 | libmodules.a(ofmem_common.o): In function `get_ram_size':
 | /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:35: 
 undefined reference to `ofmem_arch_get_private'
 | /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:35: 
 undefined reference to `ofmem_arch_get_private'
 | /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:35: 
 undefined reference to `ofmem_arch_get_private'
 | libmodules.a(ofmem_common.o): In function `ofmem_claim_virt':
 | /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:448: 
 undefined reference to `ofmem_arch_get_virt_top'
 | libmodules.a(ofmem_common.o): In function `ofmem_malloc':
 | /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:83: 
 undefined reference to `ofmem_arch_get_private'
 | /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:92: 
 undefined reference to `ofmem_arch_get_malloc_base'
 | /home/aurel32/openbios-devel/obj-ppc/../modules/ofmem_common.c:108: 
 undefined reference to `ofmem_arch_get_heap_top'
 | collect2: ld returned 1 exit status
 | make[1]: *** [openbios-unix] Error 1
 | make[1]: Leaving directory `/home/aurel32/openbios-devel/obj-ppc'

 This is something that needs to be fixed before an OpenBIOS release,
 though I don't know if its planned to release OpenBIOS in sync with QEMU.

Does this patch fix the problem?


0001-Fix-openbios-unix-compile-on-PPC.patch
Description: 

Re: [Qemu-devel] [PATCH] Fixes compile problems on Mac OS 10.4 and under.

[Andreas Färber]

Am 18.09.2009 um 18:18 schrieb G 3:

This patch eliminates all the warnings and errors that appear when  
compiling cocoa.m on Mac OS 10.4 and under.


Signed-off-by: John Arbuckle programmingk...@gmail.com
---
cocoa.m |   25 -
1 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/cocoa.m b/cocoa.m
index 55ff2b4..cb75e3e 100644
--- a/cocoa.m
+++ b/cocoa.m
@@ -23,11 +23,17 @@
 */

#import Cocoa/Cocoa.h
-
#include qemu-common.h
#include console.h
#include sysemu.h

+#ifndef MAC_OS_X_VERSION_10_4
+#define MAC_OS_X_VERSION_10_4 1040
+#endif


I've picked this part up...


@@ -421,7 +429,7 @@ int cocoa_keycode_to_qemu(int keycode)
[self ungrabMouse];
[self setContentDimensions];
// test if host support enterFullScreenMode:withOptions at  
compiletime

-#if (MAC_OS_X_VERSION_MAX_ALLOWED = MAC_OS_X_VERSION_10_4)
+#if (MAC_OS_X_VERSION_MAX_ALLOWED  MAC_OS_X_VERSION_10_4)
if ([NSView  
respondsToSelector:@selector(exitFullScreenModeWithOptions:)]) { //  
test if exitFullScreenModeWithOptions is supported on host at  
runtime

[self exitFullScreenModeWithOptions:nil];
} else {


... but why do you do these changes? On v10.4 respondsToSelector:  
should simply return NO at runtime and not call the method. Are there  
warnings?


Andreas

[Qemu-devel] Re: [PATCH] Permit zero-sized qemu_malloc() friends

[Paolo Bonzini]

On 12/06/2009 01:02 PM, malc wrote:

On Sun, 6 Dec 2009, Paolo Bonzini wrote:


On 12/06/2009 11:22 AM, malc wrote:

Here, i believe, you are inventing artificial restrictions on how
malloc behaves, i don't see anything that prevents the implementor
from setting aside a range of addresses with 31st bit set as an
indicator of zero allocations, and then happily giving it to the
user of malloc and consumming it in free.


But it has to make it a valid address anyway.  If a zero-sized read treats it
as invalid (SIGSEGV, EFAULT, whatever), malloc has failed to return a valid
address and is not obeying its specification.


Once again - standard doesn't speak about valid addresses.


For that matter, POSIX doesn't mention EFAULT at all, and doesn't 
include detecting valid addresses among the things that read can do 
before returning 0.  So if an OS extends POSIX with EFAULT, it had 
better provide a malloc that is consistent with whatever definition of 
valid address EFAULT uses.  While if it doesn't provide EFAULT, read 
should return 0 for the OS to be conforming to POSIX, and the whole 
discussion is moot.


Paolo

[Qemu-devel] Re: [PATCH] Permit zero-sized qemu_malloc() friends

[Paolo Bonzini]

On 12/06/2009 01:00 PM, malc wrote:

or anything else with said bit set will yield EFAULT. Consequently the
code you cited as a well behaving malloc(0) call site will bomb.


It is not well-behaving unless the definition of EFAULT is changed 
consistently.


Paolo

Re: [Qemu-devel] [PATCH] This patch adds a Priority and a Machine menu to the menu bar.

[Andreas Färber]

Hello,

Am 12.11.2009 um 19:15 schrieb G 3:

The Priority menu controls how much cpu time qemu receives, and the  
Machine menu has the Restart menu item for restarting the emulator.


Signed-off-by: John Arbuckle programmingk...@gmail.com
---
cocoa.m |   91 + 
+

1 files changed, 79 insertions(+), 12 deletions(-)


Please do not combine unrelated changes into one large patch.

I'm not sure if adding more menus to QEMU is the way to go,  
considering that this has traditionally been the job of frontends like  
Q. I haven't noticed such options in the SDL frontend either.
What I am missing though is the system-provided Services menu item in  
the application menu.



diff --git a/cocoa.m b/cocoa.m
index 55ff2b4..14a3004 100644
--- a/cocoa.m
+++ b/cocoa.m



@@ -947,13 +1004,23 @@ static void cocoa_refresh(DisplayState *ds)

NSDate *distantPast;
NSEvent *event;
+   int count;
+   
+   count = 0;
distantPast = [NSDate distantPast];
-do {
+do {   
+   
+   if(qemuAtHighPriority == NO) {
+   sleep(1000);
+   }
+   
event = [NSApp nextEventMatchingMask:NSAnyEventMask  
untilDate:distantPast

inMode: NSDefaultRunLoopMode dequeue:YES];
if (event != nil) {
[cocoaView handleEvent:event];
-}


What exactly are you trying to accomplish with this patch?

This is most certainly the wrong way, you are blocking the event loop  
during a pending update for one second.
If you want to throttle events like screen updates or reduce the QEMU- 
induced CPU load, this likely needs to be done in another way, putting  
less events into the queue in the first place, no?



+   }
+
+   
} while(event != nil);
vga_hw_update();
}


Andreas

Re: [Qemu-devel] Socket reconnection.

[Ian Molton]

Jamie Lokier wrote:
 Ian Molton wrote:
 Jamie Lokier wrote:

 I'm a bit puzzled.

 Why isn't virtio-rng getting entropy from /dev/random on the host?
 /dev/random may not be available.
 
 Understood on a non-Linux host.

Or a linux host with a user with insufficient privs...

 Besides, not all entropy comes from /dev/random.
 
 On a Linux host, why isn't rngd simply injecting it's entropy into
 /dev/random where it would be more convenient to access?  (No need for
 socket reconnection code, for example).

Who knows? lack of privs, an admin who only uses egd, a machine which is
being fed entropy by egd via a tunnel. User doesnt trust /dev/random,
/dev/random known to be failing FIPS tests on a shared machine - there
could be any number of reasons. In our case, entropy is comming from
hardware via egd, to be used in the guest VMs. why feed it into RNGD,
then the hosts entropy pool, THEN the guests - just feed them directly.
the egd daemon in this case also offers load balancing to all consumers
of entropy.

Since we need this on hosts without /dev/random anyway, I dont see why
we would need to deliberately cripple qemu on linux hosts...

-Ian

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[Ian Molton]

Markus Armbruster wrote:

 p = malloc(n * sizeof(struct foo);
 if (n  !p)
 exit_no_mem();
 for (i = 0; i  n; i++)
 compute_one(p, i);
 
 With qemu_malloc(), the error handling moves into qemu_malloc():
 
 p = qemu_malloc(n * sizeof(struct foo);
 for (i = 0; i  n; i++)
 compute_one(p, i);

And you lose the ability to fail gracefully...

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[Ian Molton]

Avi Kivity wrote:
 On 12/06/2009 01:25 AM, Ian Molton wrote:
 Avi Kivity wrote:

   
 It's not that it doesn't have a way to report failure, it's that it
 doesn't fail.  Do you prefer functions that fail and report it to
 functions that don't fail?
  
 You have a way of allocating memory that will _never_ fail?

 Sort of. 

'sort of' never ?

 Did you look at the code?

Yes. Its hardly infallible.

 well, make sure n is not 0. Its not that hard. I dont think I've *ever*
 had a situation where I wanted to pass 0 to malloc.
 
 There are multiple such cases in the code.
 
 stick to what people know, and LART them for misuse of it if necessary.

 The LART is a crash, great.
  
 No, the LART would be a 'your patch does this wrong, try this:'
 
 What about existing usage?  Will you audit all the existing calls?

mark qemu_malloc as deprecated. don't include new patches that use it.
Plenty of time to fix the broken uses...

-Ian

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[Avi Kivity]

On 12/06/2009 06:58 PM, Ian Molton wrote:

Avi Kivity wrote:
   

On 12/06/2009 01:25 AM, Ian Molton wrote:
 

Avi Kivity wrote:


   

It's not that it doesn't have a way to report failure, it's that it
doesn't fail.  Do you prefer functions that fail and report it to
functions that don't fail?

 

You have a way of allocating memory that will _never_ fail?
   

Sort of.
 

'sort of' never ?

   

Did you look at the code?
 

Yes. Its hardly infallible.
   


It will never fail on Linux.  On other hosts it prevents a broken oom 
handler from taking the guest down a death spiral.



What about existing usage?  Will you audit all the existing calls?
 

mark qemu_malloc as deprecated. don't include new patches that use it.
Plenty of time to fix the broken uses...
   


Send patches.  I don't think it's realistic to handle OOM in qemu 
(handling n=0 is easy, but a lot of work for no real gain).


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

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[Avi Kivity]

On 12/06/2009 06:52 PM, Ian Molton wrote:

Markus Armbruster wrote:

   

 p = malloc(n * sizeof(struct foo);
 if (n  !p)
 exit_no_mem();
 for (i = 0; i  n; i++)
 compute_one(p, i);

With qemu_malloc(), the error handling moves into qemu_malloc():

 p = qemu_malloc(n * sizeof(struct foo);
 for (i = 0; i  n; i++)
 compute_one(p, i);
 

And you lose the ability to fail gracefully...
   


We never had it.  Suppose p is allocated in response to an SCSI register 
write, and we allocate a scatter-gather list.  What do you do if you OOM?


1) Introduce an error path that works synchronously off the stack and so 
does not need to allocate memory
2) Don't allocate in the first place, always read guest memory and 
transform it for sglists, preallocate everything else
3) Have a per-thread emergency pool, stall the vcpu until all memory is 
returned to the emergency pool


all of these options are pretty horrible, either to the code or to the 
guest, for something that never happens.


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

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[malc]

On Sun, 6 Dec 2009, Avi Kivity wrote:

 On 12/06/2009 06:58 PM, Ian Molton wrote:
  Avi Kivity wrote:
 
   On 12/06/2009 01:25 AM, Ian Molton wrote:

Avi Kivity wrote:


   
 It's not that it doesn't have a way to report failure, it's that it
 doesn't fail.  Do you prefer functions that fail and report it to
 functions that don't fail?
 
  
You have a way of allocating memory that will _never_ fail?
   
   Sort of.

  'sort of' never ?
  
 
   Did you look at the code?

  Yes. Its hardly infallible.
 
 
 It will never fail on Linux.  On other hosts it prevents a broken oom handler
 from taking the guest down a death spiral.

It fails here all the time i'm sorry to say, i have overcommit disabled
(mostly because kpdf when doing a text search tends to overwhelm the VM
subsystem and Linux happily picks X11 as it's OOM kill target)

 
   What about existing usage?  Will you audit all the existing calls?

  mark qemu_malloc as deprecated. don't include new patches that use it.
  Plenty of time to fix the broken uses...
 
 
 Send patches.  I don't think it's realistic to handle OOM in qemu (handling
 n=0 is easy, but a lot of work for no real gain).


-- 
mailto:av1...@comtv.ru

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[Avi Kivity]

On 12/06/2009 07:47 PM, malc wrote:



It will never fail on Linux.  On other hosts it prevents a broken oom handler
from taking the guest down a death spiral.
 

It fails here all the time i'm sorry to say, i have overcommit disabled
(mostly because kpdf when doing a text search tends to overwhelm the VM
subsystem and Linux happily picks X11 as it's OOM kill target)
   


Right, I meant under the default configuration.  Unfortunately there is 
no good configuration for Linux - without strict overcommit you'll 
invoke the oom killer, with strict overcommit you'll need ridiculous 
amounts of swap because fork() and MAP_PRIVATE require tons of backing 
store.


On my home machine I have

  $ grep Commit /proc/meminfo
  CommitLimit: 7235160 kB
  Committed_AS:4386172 kB

So, 4GB of virtual memory needed just to run a normal desktop with 
thunderbird+firefox.  Actual anonymous memory is less than 2GB, so you 
could easily run this workload on a 2GB machine without swap, but with 
strict overcommit 2GB RAM + 2GB swap will see failures even though you 
have free RAM.


--
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[Avi Kivity]

On 12/06/2009 07:45 PM, malc wrote:


   

And you lose the ability to fail gracefully...

   

We never had it.  Suppose p is allocated in response to an SCSI register
write, and we allocate a scatter-gather list.  What do you do if you OOM?
 

Uh, please do not generalize.

   


Sorry.


See for instance 29ddf27b72960d6e6b115cd69812c9c57b2a7b13 the incident was
logged and debugged, no OOM, no abort. Not all scenarios admit this, but
claiming that there are none that do is incorrect.
   


Init is pretty easy to handle.  I'm worried about runtime where you 
can't report an error to the guest.  Real hardware doesn't oom.


--
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[malc]

On Sun, 6 Dec 2009, Avi Kivity wrote:

 On 12/06/2009 07:47 PM, malc wrote:
  
   It will never fail on Linux.  On other hosts it prevents a broken oom
   handler
   from taking the guest down a death spiral.

  It fails here all the time i'm sorry to say, i have overcommit disabled
  (mostly because kpdf when doing a text search tends to overwhelm the VM
  subsystem and Linux happily picks X11 as it's OOM kill target)
 
 
 Right, I meant under the default configuration.  Unfortunately there is no
 good configuration for Linux - without strict overcommit you'll invoke the oom
 killer, with strict overcommit you'll need ridiculous amounts of swap because
 fork() and MAP_PRIVATE require tons of backing store.
 
 On my home machine I have
 
   $ grep Commit /proc/meminfo
   CommitLimit: 7235160 kB
   Committed_AS:4386172 kB
 
 So, 4GB of virtual memory needed just to run a normal desktop with
 thunderbird+firefox.  Actual anonymous memory is less than 2GB, so you could
 easily run this workload on a 2GB machine without swap, but with strict
 overcommit 2GB RAM + 2GB swap will see failures even though you have free RAM.
 

Well, i don't have a swap...

~$ cat /proc/meminfo 
MemTotal:   515708 kB
MemFree:  3932 kB
Buffers: 10120 kB
Cached: 365320 kB
SwapCached:  0 kB
Active: 238120 kB
Inactive:   222396 kB
SwapTotal:   0 kB
SwapFree:0 kB
...
CommitLimit:464136 kB
Committed_AS:   178920 kB
...

-- 
mailto:av1...@comtv.ru

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[malc]

On Sun, 6 Dec 2009, Avi Kivity wrote:

 On 12/06/2009 07:45 PM, malc wrote:
  
 
And you lose the ability to fail gracefully...

   
   We never had it.  Suppose p is allocated in response to an SCSI register
   write, and we allocate a scatter-gather list.  What do you do if you OOM?

  Uh, please do not generalize.
  
 
 
 Sorry.
 
  See for instance 29ddf27b72960d6e6b115cd69812c9c57b2a7b13 the incident was
  logged and debugged, no OOM, no abort. Not all scenarios admit this, but
  claiming that there are none that do is incorrect.
 
 
 Init is pretty easy to handle.  I'm worried about runtime where you can't
 report an error to the guest.  Real hardware doesn't oom.

Here, i do agree, but mostly because most of the users of allocation
functions just themselves returned NULL or -1 or whatever and never
really bothered to report anything, so the addition of OOM check that
you've added made the code less cluttered.

-- 
mailto:av1...@comtv.ru

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[Avi Kivity]

On 12/06/2009 08:09 PM, malc wrote:


Well, i don't have a swap...

~$ cat /proc/meminfo
MemTotal:   515708 kB
MemFree:  3932 kB
Buffers: 10120 kB
Cached: 365320 kB
SwapCached:  0 kB
Active: 238120 kB
Inactive:   222396 kB
SwapTotal:   0 kB
SwapFree:0 kB
...
CommitLimit:464136 kB
Committed_AS:   178920 kB
...
   


Out of curiosity, what desktop and apps are you running?  Here firefox 
takes 1.3GB virt size and 377MB RSS, that alone could overwhelm your system.


[...@firebolt qemu-kvm (master)]$ cat /proc/2698/status
Name:firefox
State:S (sleeping)
Tgid:2698
Pid:2698
PPid:2686
TracerPid:0
Uid:500500500500
Gid:500500500500
Utrace:0
FDSize:256
Groups:500 502 512
VmPeak: 1478804 kB
VmSize: 1288100 kB
VmLck:   0 kB
VmHWM:  434788 kB
VmRSS:  386296 kB
VmData:  676384 kB
VmStk: 248 kB
VmExe:  88 kB
VmLib:   62504 kB
VmPTE:2312 kB


--
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[Avi Kivity]

On 12/06/2009 08:12 PM, malc wrote:



Init is pretty easy to handle.  I'm worried about runtime where you can't
report an error to the guest.  Real hardware doesn't oom.
 

Here, i do agree, but mostly because most of the users of allocation
functions just themselves returned NULL or -1 or whatever and never
really bothered to report anything, so the addition of OOM check that
you've added made the code less cluttered.
   


My point is that it would take a major rework, and would probably 
involve removing the allocations instead of handling any errors.  For 
example, a scsi device would tell the block device the upper bound of 
aiocbs it could possibly issue, and the maximum number of sg elements in 
a request, and qcow2 (or any other backing format driver) would 
preallocate enough resources to satisfy the worst case.  And we still 
can't handle a syscall returning ENOMEM.


--
Do not meddle in the internals of kernels, for they are subtle and quick to 
panic.

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[malc]

On Sun, 6 Dec 2009, Avi Kivity wrote:

 On 12/06/2009 08:09 PM, malc wrote:
  
  Well, i don't have a swap...
  
  ~$ cat /proc/meminfo
  MemTotal:   515708 kB
  MemFree:  3932 kB
  Buffers: 10120 kB
  Cached: 365320 kB
  SwapCached:  0 kB
  Active: 238120 kB
  Inactive:   222396 kB
  SwapTotal:   0 kB
  SwapFree:0 kB
  ...
  CommitLimit:464136 kB
  Committed_AS:   178920 kB
  ...
 
 
 Out of curiosity, what desktop and apps are you running?  Here firefox takes
 1.3GB virt size and 377MB RSS, that alone could overwhelm your system.

Most of the time my setup is:

IceWM - 4 virtual desktops
XEmacs
Seamonkey 1.1.16
IceDock
A bunch of rxvts

 
 [...@firebolt qemu-kvm (master)]$ cat /proc/2698/status
 Name:firefox
 State:S (sleeping)
 Tgid:2698
 Pid:2698
 PPid:2686
 TracerPid:0
 Uid:500500500500
 Gid:500500500500
 Utrace:0
 FDSize:256
 Groups:500 502 512
 VmPeak: 1478804 kB
 VmSize: 1288100 kB
 VmLck:   0 kB
 VmHWM:  434788 kB
 VmRSS:  386296 kB
 VmData:  676384 kB
 VmStk: 248 kB
 VmExe:  88 kB
 VmLib:   62504 kB
 VmPTE:2312 kB
 

~$ cat /proc/$(pidof seamonkey-bin)/status
Name:   seamonkey-bin
State:  S (sleeping)
Tgid:   2332
Pid:2332
PPid:   1
TracerPid:  0
Uid:1000100010001000
Gid:100 100 100 100
FDSize: 256
Groups: 10 11 17 18 19 100 
VmPeak:   151848 kB
VmSize:   150976 kB
VmLck: 0 kB
VmHWM: 57032 kB
VmRSS: 56260 kB
VmData:   105740 kB
VmStk:84 kB
VmExe:   296 kB
VmLib: 34472 kB
VmPTE:   136 kB
Threads:5
[...]

-- 
mailto:av1...@comtv.ru

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[Jamie Lokier]

Ian Molton wrote:
  Read the beginning of the thread.  Basically it's for arrays, malloc(n *
  sizeof(x)).
 
 well, make sure n is not 0. Its not that hard. I dont think I've *ever*
 had a situation where I wanted to pass 0 to malloc.

I would like to remind everyone that sizeof(x) can be 0 too.  For
example, on Linux sizeof(spinlock_t) == 0 on UP.  Anything where you
have a bunch of structure fields which depend on compile time
configuration, or where a type might be replaced by a stub empty
structure, is a possible sizeof(x) == 0.

 Its not that hard.

The fact is there are a number of bugs in qemu where n == 0 is not
checked prior to calling qemu_malloc() at the moment.  None of them
are hard to fix - they are rare cases that nobody noticed when
writing them.

Until we have code analysis tools checking for that, bugs of that kind
will probably keep arising.

-- Jamie

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[Jamie Lokier]

Avi Kivity wrote:
 A NEW(type) and ARRAY_NEW(type, count) marcros would improve type safety
 and plug a dormant buffer overflow due to multiplication overflow, yes.  
 Even qemu_calloc() would be an improvement.

In my code I regularly use type_alloc(type) and type_free(type, ptr),
giving type safety at both ends (and possibility to optimise
allocations, but that's separate).

If you have ARRAY_NEW(type, count) which permits count to be zero and
returns a non-NULL result, I wonder, why is it ok to convert zero
count to a guaranteed non-NULL unique result, but not do that for
sizeof(type) (or just size)?

-- Jamie

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[malc]

On Sun, 6 Dec 2009, Avi Kivity wrote:

 On 12/06/2009 08:12 PM, malc wrote:
  
   Init is pretty easy to handle.  I'm worried about runtime where you can't
   report an error to the guest.  Real hardware doesn't oom.

  Here, i do agree, but mostly because most of the users of allocation
  functions just themselves returned NULL or -1 or whatever and never
  really bothered to report anything, so the addition of OOM check that
  you've added made the code less cluttered.
 
 
 My point is that it would take a major rework, and would probably involve
 removing the allocations instead of handling any errors.  For example, a scsi
 device would tell the block device the upper bound of aiocbs it could possibly
 issue, and the maximum number of sg elements in a request, and qcow2 (or any
 other backing format driver) would preallocate enough resources to satisfy the
 worst case.  And we still can't handle a syscall returning ENOMEM.
 

Sure. My point is that sometimes failure to allocate is due to bugs,
invalid input etc, and conversion to OOM aborts en masse, might have
not been the best possible route to take, but most likely it was better
than doing nothing.

-- 
mailto:av1...@comtv.ru

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[Ian Molton]

Avi Kivity wrote:

 Init is pretty easy to handle.  I'm worried about runtime where you
 can't report an error to the guest.  Real hardware doesn't oom.

In the case of the socket reconnect code I posted recently, if the
allocation failed, it would give up trying to reconnect and inform the
user of that chardev that it had closed. Ok, this doesnt help the guest,
but it allows other code to clean up nicely, and we can report the
failure to the host. IMHO thats better than leaving a sysadmin
scratching their head wondering why it suddenly just stopped feeding the
guest entropy and isnt trying to reconnect anymore...

(IMO)

-Ian

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[Ian Molton]

malc wrote:

 Well, i don't have a swap...

Indeed, nor do I (machine has an NFS root. swap on NFS is... not good.)

[Qemu-devel] [ANNOUNCE][Call-For-Testing] Release 0.12.0-rc1 of QEMU

[Anthony Liguori]

The QEMU team is pleased to announce the availability of the 0.12.0-rc1
release.  This is the first release candidate for the 0.12.0 release. 
This release is not intended for production use.


Testing release candidates is a great way to contribute to QEMU and 
improve the quality of the 0.12.0 release.


The current plan is to release a 0.12.0-rc2 on Friday, Dec. 11th 
followed by the final 0.12.0 release on Wednesday, Dec 15th.


For this release candidate, we would appreciate as much testing and 
feedback as possible, particularly the following features which have 
seen a lot of activity since the 0.11.0 release:


 - live migration; especially with non-x86 and complex machine 
configurations

 - block device migration
 - exotic x86 guests that may interact with the BIOS in special ways
 - PXE booting
 - SCSI disk support
 - any tools that depend on parsing monitor output

It can be downloaded from Savannah at:

http://download.savannah.gnu.org/releases/qemu/qemu-0.12.0-rc1.tar.gz

Please send testing feedback (positive or negative) to qemu-devel and 
file bugs against the release candidate at:


https://bugs.launchpad.net/qemu

On behalf of the QEMU team, I'd like to thank everyone who contributed
to make this release happen!

--
Regards,

Anthony Liguori

Re: [Qemu-devel] Re: [Qemu-commits] [COMMIT c34ebfd] Bring pcbios, seabios, and vgabios into the tree as git submodules. Right now,

[Isaku Yamahata]

On Wed, Sep 30, 2009 at 08:02:08AM -0500, Anthony Liguori wrote:
 Hi Isaku,

 Isaku Yamahata wrote:
 Is there any documentation available about where those git repo are and 
 how to compile?

 Fortunately I was able to find the repo
 in git.qemu.org by digging out the ML archive.
 However there should be some kind of description
 in pc-bios/README or somewhere else.
   

 Good point, I'll work on that.

What happened to this?
At this moment, there is no document which describes the official
procedure to compile BIOSes.
I suppose this kind of document is suitable for 0.12.0 release.

-- 
yamahata

Re: [Qemu-devel] Socket reconnection.

[Jamie Lokier]

Ian Molton wrote:
  Besides, not all entropy comes from /dev/random.
  
  On a Linux host, why isn't rngd simply injecting it's entropy into
  /dev/random where it would be more convenient to access?  (No need for
  socket reconnection code, for example).
 
 Who knows? lack of privs, an admin who only uses egd, a machine which is
 being fed entropy by egd via a tunnel. User doesnt trust /dev/random,
 /dev/random known to be failing FIPS tests on a shared machine - there
 could be any number of reasons. In our case, entropy is comming from
 hardware via egd, to be used in the guest VMs. why feed it into RNGD,
 then the hosts entropy pool, THEN the guests - just feed them directly.
 the egd daemon in this case also offers load balancing to all consumers
 of entropy.

Those arguments are weak because they also apply the other way: an
admin who only uses /dev/random (got lots of those), a machine which
is being fed entropy into /dev/random via a tunnel (been there), user
doesn't trust egd (why should I trust it more than the kernel?
userspace is more easily corrupted - and with a socket, it might not
even be egd running), etc.

I grant you there are advantages sometimes, but also disadvantages.
Why would I run egd on a guest VM running an embedded system with 16MB
RAM, where every process is precious, for example?  But those systems
need entropy too.

As for why feed it directly via hardware - egd - guest, the
reason surely is because most clients read /dev/random or
/dev/urandom, so entropy that isn't injected into /dev/random is wasted.

Anyway, I've just read the rngd manual, and it does inject it's
entropy into /dev/random so that's the end of that discussion :-)

But the main issue is below.  On all host systems I have access to,
there is _no_ entropy available from egd/rngd...

 Since we need this on hosts without /dev/random anyway, I dont see why
 we would need to deliberately cripple qemu on linux hosts...

Since nobody has asked you to cripple anything, that is irrelevant.
Options don't cripple.

The lack of option is crippling qemu on linux hosts which don't run egd.

Which hosts are those?  Well, I've just checked five live systems,
four servers and one laptop, running:

Ubuntu 9.10 (Karmic)  - no egd running, no rngd running
Ubuntu 9.04 (Jaunty)  - no egd running, no rngd running
Debian 4.0 (Etch) - no egd running, no rngd running
CentOS 4.0- no egd running, no rngd running
RHEL 4.0  - no egd running, no rngd running

So if I understand right, the virtio-rng host driver won't work on any
host system I have access to?  Is that correct?

Btw, /dev/random is available on virtually every host - Linux,
Solaris, FreeBSD, Mac OS X, NetBSD, OpenBSD, Tru64, HP-UX, AIX, and
there's even a similar device for Windows :-)

I grant you the egd/rngd entropy is preferable on a system that's
running it.  It is much more careful.  (Let's ignore this in rngd's
man page: Do not do that unless you trust rngd's source of random
data).  So a good default would be to use the egd/rngd socket when
available, and fall back to /dev/random or even /dev/urandom (at user
request) when not.

My tests show that egd/rngd are not running on any host system I have
access to - and that's quite a diverse range of Linuxes.  It would
have to be run specially, which sounds problematic as qemu would be
the only process using it.

-- Jamie

Re: [Qemu-devel] [PATCH] Permit zero-sized qemu_malloc() friends

[Jamie Lokier]

Ian Molton wrote:
 Avi Kivity wrote:
 
  Init is pretty easy to handle.  I'm worried about runtime where you
  can't report an error to the guest.  Real hardware doesn't oom.
 
 In the case of the socket reconnect code I posted recently, if the
 allocation failed, it would give up trying to reconnect and inform the
 user of that chardev that it had closed. Ok, this doesnt help the guest,
 but it allows other code to clean up nicely, and we can report the
 failure to the host. IMHO thats better than leaving a sysadmin
 scratching their head wondering why it suddenly just stopped feeding the
 guest entropy and isnt trying to reconnect anymore...

If the system as a whole runs out of memory so that no-overcommit
malloc() fails on a small alloc, there's a good chance that you won't
be able to send a message to the host (how do you format the QMP
message without malloc?), and if you do manage that, there's a good
chance the host won't be able to receive it (it can't malloc either),
and if it does manage to receive the message, you can be almost
certain that it won't be able to run any GUI operations, send mail,
etc. to inform the admin.

The chances of the path qemu small alloc - chardev error - send QMP
message - receive QMP message - parse QMG message - do something
useful (log/email/UI) having fully preallocated buffers for every
step, including a preallocated emergency pool for the buffers used by
QMG formatting and parsing, so that it gets all the way past the last
step are very slim indeed.

There's no point writing the code for the first steps, if it's
intractable to make the later steps do something useful.

Btw, as an admin I would really rather the socket reconnection code
keeps trying in that circumstance, if qemu does not simply fall over
due to alloc failing for something else soon after.  The most likely
scenario, imho in a server like that, is to notice it is running out
of memory and kill the real cause (e.g. another runaway process), then
restart all daemons which have died.  I'm not going to notice a
non-fatal message (in the unlikely event it is propagated all the way
up) because there are plenty of other non-fatal messages in normal
use, multiplied by hundreds of guests (across a cluster).  Or, if you
mean the chardev closing causes qemu to terminate - what's the
difference from the current qemu_malloc() behaviour?

I'd rather it behaves like a broken HWRNG if it can't get host
entropy: Don't provide data, and let the guest decide what to do, just
like it does for a broken HWRNG.  Except virtio-rng can report
unavailability rather than simply being broken :-)

-- Jamie

Re: [Qemu-devel] Re: [Qemu-commits] [COMMIT c34ebfd] Bring pcbios, seabios, and vgabios into the tree as git submodules. Right now,

[Anthony Liguori]

Isaku Yamahata wrote:

On Wed, Sep 30, 2009 at 08:02:08AM -0500, Anthony Liguori wrote:
  

Hi Isaku,

Isaku Yamahata wrote:

Is there any documentation available about where those git repo are and 
how to compile?


Fortunately I was able to find the repo
in git.qemu.org by digging out the ML archive.
However there should be some kind of description
in pc-bios/README or somewhere else.
  
  

Good point, I'll work on that.



What happened to this?
At this moment, there is no document which describes the official
procedure to compile BIOSes.
I suppose this kind of document is suitable for 0.12.0 release.
  


There's really nothing that clever about it.

$ git clone git://git.qemu.org/qemu.git
$ cd qemu
$ git submodule update --init
$ cd roms/seabios
$ make
$ cp out/bios.bin ../../pc-bios/bios.bin

The last three steps could be totally automated by the build system.  
That's what I'd like to see.


Regards,

Anthony Liguori

Re: [Qemu-devel] Re: [Qemu-commits] [COMMIT c34ebfd] Bring pcbios, seabios, and vgabios into the tree as git submodules. Right now,

[Isaku Yamahata]

On Sun, Dec 06, 2009 at 08:52:11PM -0600, Anthony Liguori wrote:
 Isaku Yamahata wrote:
 On Wed, Sep 30, 2009 at 08:02:08AM -0500, Anthony Liguori wrote:
   
 Hi Isaku,

 Isaku Yamahata wrote:
 
 Is there any documentation available about where those git repo are 
 and how to compile?

 Fortunately I was able to find the repo
 in git.qemu.org by digging out the ML archive.
 However there should be some kind of description
 in pc-bios/README or somewhere else.
 
 Good point, I'll work on that.
 

 What happened to this?
 At this moment, there is no document which describes the official
 procedure to compile BIOSes.
 I suppose this kind of document is suitable for 0.12.0 release.
   

 There's really nothing that clever about it.

 $ git clone git://git.qemu.org/qemu.git
 $ cd qemu
 $ git submodule update --init
 $ cd roms/seabios
 $ make
 $ cp out/bios.bin ../../pc-bios/bios.bin

 The last three steps could be totally automated by the build system.   
 That's what I'd like to see.

So the repository will be moved from nongnu to git.qemu.org
even for qemu itself?
Anyway here's the patch.

From 60c5325a84776beb0dd20581396126a0e068e707 Mon Sep 17 00:00:00 2001
From: Isaku Yamahata yamah...@valinux.co.jp
Date: Mon, 7 Dec 2009 12:15:25 +0900
Subject: [PATCH] pc-bios: update README to describe compile BIOSes.

Add the procedure to compile BIOSes.

Signed-off-by: Isaku Yamahata yamah...@valinux.co.jp
---
 pc-bios/README |   13 +
 1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/pc-bios/README b/pc-bios/README
index 3f2c978..9bf2532 100644
--- a/pc-bios/README
+++ b/pc-bios/README
@@ -26,3 +26,16 @@
   virtio 1af4:1000
 
   http://rom-o-matic.net/
+
+- To compile PC related BIOSes
+  git.qemu.org holds the repositories for BIOSes sources.
+  So you can compile BIOSes by following steps.
+
+  $ git clone git://git.qemu.org/qemu.git
+  $ cd qemu
+  $ git submodule update --init
+  $ cd roms/seabios
+  $ make
+  $ cp out/bios.bin ../../pc-bios/bios.bin
+
+  The last three steps could be totally automated by the build system.
-- 
1.6.5.4