[Qemu-devel] [PATCH] introduce environment variables for all qemu-user options

[Johannes Schauer]

Rework option parsing code for linux-user in a table-driven manner to allow
environment variables for all commandline options.

Also generate usage() output from option table.

Fix complains from checkpatch.pl, also have envlist global


Signed-off-by: Johannes Schauer 
---
 linux-user/main.c |  530 ++---
 1 files changed, 342 insertions(+), 188 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index dbba8be..74c5198 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -40,6 +40,11 @@
 char *exec_path;
 
 int singlestep;
+const char *filename;
+const char *argv0;
+int gdbstub_port;
+envlist_t *envlist;
+const char *cpu_model;
 unsigned long mmap_min_addr;
 #if defined(CONFIG_USE_GUEST_BASE)
 unsigned long guest_base;
@@ -47,6 +52,8 @@ int have_guest_base;
 unsigned long reserved_va;
 #endif
 
+static void usage(void);
+
 static const char *interp_prefix = CONFIG_QEMU_INTERP_PREFIX;
 const char *qemu_uname_release = CONFIG_UNAME_RELEASE;
 
@@ -2624,50 +2631,6 @@ void cpu_loop (CPUState *env)
 }
 #endif /* TARGET_ALPHA */
 
-static void usage(void)
-{
-printf("qemu-" TARGET_ARCH " version " QEMU_VERSION QEMU_PKGVERSION ", 
Copyright (c) 2003-2008 Fabrice Bellard\n"
-   "usage: qemu-" TARGET_ARCH " [options] program [arguments...]\n"
-   "Linux CPU emulator (compiled for %s emulation)\n"
-   "\n"
-   "Standard options:\n"
-   "-hprint this help\n"
-   "-g port   wait gdb connection to port\n"
-   "-L path   set the elf interpreter prefix (default=%s)\n"
-   "-s size   set the stack size in bytes (default=%ld)\n"
-   "-cpu modelselect CPU (-cpu ? for list)\n"
-   "-drop-ld-preload  drop LD_PRELOAD for target process\n"
-   "-E var=value  sets/modifies targets environment variable(s)\n"
-   "-U varunsets targets environment variable(s)\n"
-   "-0 argv0  forces target process argv[0] to be argv0\n"
-#if defined(CONFIG_USE_GUEST_BASE)
-   "-B addressset guest_base address to address\n"
-   "-R size   reserve size bytes for guest virtual address 
space\n"
-#endif
-   "\n"
-   "Debug options:\n"
-   "-d options   activate log (logfile=%s)\n"
-   "-p pagesize  set the host page size to 'pagesize'\n"
-   "-singlestep  always run in singlestep mode\n"
-   "-strace  log system calls\n"
-   "\n"
-   "Environment variables:\n"
-   "QEMU_STRACE   Print system calls and arguments similar to 
the\n"
-   "  'strace' program.  Enable by setting to any 
value.\n"
-   "You can use -E and -U options to set/unset environment variables\n"
-   "for target process.  It is possible to provide several variables\n"
-   "by repeating the option.  For example:\n"
-   "-E var1=val2 -E var2=val2 -U LD_PRELOAD -U LD_DEBUG\n"
-   "Note that if you provide several changes to single variable\n"
-   "last change will stay in effect.\n"
-   ,
-   TARGET_ARCH,
-   interp_prefix,
-   guest_stack_size,
-   DEBUG_LOGFILE);
-exit(1);
-}
-
 THREAD CPUState *thread_env;
 
 void task_settid(TaskState *ts)
@@ -2703,24 +2666,351 @@ void init_task_state(TaskState *ts)
 }
 ts->sigqueue_table[i].next = NULL;
 }
- 
+
+static void handle_arg_help(const char *arg)
+{
+usage();
+}
+
+static void handle_arg_log(const char *arg)
+{
+int mask;
+const CPULogItem *item;
+
+mask = cpu_str_to_log_mask(arg);
+if (!mask) {
+printf("Log items (comma separated):\n");
+for (item = cpu_log_items; item->mask != 0; item++) {
+printf("%-10s %s\n", item->name, item->help);
+}
+exit(1);
+}
+cpu_set_log(mask);
+}
+
+static void handle_arg_set_env(const char *arg)
+{
+char *r, *p, *token;
+r = p = strdup(arg);
+while ((token = strsep(&p, ",")) != NULL) {
+if (envlist_setenv(envlist, token) != 0) {
+usage();
+}
+}
+free(r);
+}
+
+static void handle_arg_unset_env(const char *arg)
+{
+char *r, *p, *token;
+r = p = strdup(arg);
+while ((token = strsep(&p, ",")) != NULL) {
+if (envlist_unsetenv(envlist, token) != 0) {
+usage();
+}
+}
+free(r);
+}
+
+static void handle_arg_argv0(const char *arg)
+{
+argv0 = strdup(arg);
+}
+
+static void handle_arg_stack_size(const char *arg)
+{
+char *p;
+guest_stack_size = strtoul(arg, &p, 0);
+if (guest_stack_size == 0) {
+usage();
+}
+
+if (*p == 'M') {
+guest_stack_size *= 1024 * 1024;
+} else if (*p == 'k' || *p == 'K') {
+guest_stack_size *= 1024;
+}
+}
+
+static void handle_arg_ld_prefix(const char *arg)
+{
+interp_prefix = strdup(arg);
+}
+
+s

Re: [Qemu-devel] Build broken

[Stefan Hajnoczi]

On Fri, Aug 5, 2011 at 5:49 PM, malc  wrote:
> On Fri, 5 Aug 2011, Stefan Hajnoczi wrote:
>
>> On Fri, Aug 5, 2011 at 7:22 AM, malc  wrote:
>> >
>> > /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c: In function 
>> > 'coroutine_new':
>> > /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c:160:16: error: 
>> > 'arg.i[1]' may be used uninitialized in this function
>> > /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c:136:18: note: 'arg.i[1]' 
>> > was declared here
>> >
>> > diff --git a/coroutine-ucontext.c b/coroutine-ucontext.c
>> > index 41c2379..42dc3e2 100644
>> > --- a/coroutine-ucontext.c
>> > +++ b/coroutine-ucontext.c
>> > @@ -133,7 +133,7 @@ static Coroutine *coroutine_new(void)
>> >     CoroutineUContext *co;
>> >     ucontext_t old_uc, uc;
>> >     jmp_buf old_env;
>> > -    union cc_arg arg;
>> > +    union cc_arg arg = {0};
>> >
>> >     /* The ucontext functions preserve signal masks which incurs a system 
>> > call
>> >      * overhead.  setjmp()/longjmp() does not preserve signal masks but 
>> > only
>> >
>> > I guess gcc should yell not only here on ppc32 but on any machine where
>> > pointer size is less than the size of two ints.
>>
>> Makes sense.  Are you going to commit a fix or send a signed-off-by patch?
>>
>
> If the author(s)(you and Kevin? just you?) agree with the above i can just
> push it.

The change makes sense to me.

Kevin?

Stefan

[Qemu-devel] [PATCH v2 2/6] VMDK: add twoGbMaxExtentSparse support

[Fam Zheng]

Add twoGbMaxExtentSparse support. Introduce vmdk_free_last_extent.

Signed-off-by: Fam Zheng 
---
 block/vmdk.c |  133 --
 1 files changed, 83 insertions(+), 50 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index e22a893..ab15840 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -174,6 +174,17 @@ static void vmdk_free_extents(BlockDriverState *bs)
 qemu_free(s->extents);
 }
 
+static void vmdk_free_last_extent(BlockDriverState *bs)
+{
+BDRVVmdkState *s = bs->opaque;
+
+if (s->num_extents == 0) {
+return;
+}
+s->num_extents--;
+s->extents = qemu_realloc(s->extents, s->num_extents * sizeof(VmdkExtent));
+}
+
 static uint32_t vmdk_read_cid(BlockDriverState *bs, int parent)
 {
 char desc[DESC_SIZE];
@@ -357,18 +368,18 @@ static int vmdk_init_tables(BlockDriverState *bs, 
VmdkExtent *extent)
 return ret;
 }
 
-static int vmdk_open_vmdk3(BlockDriverState *bs, int flags)
+static int vmdk_open_vmdk3(BlockDriverState *bs,
+   BlockDriverState *file,
+   int flags)
 {
 int ret;
 uint32_t magic;
 VMDK3Header header;
-BDRVVmdkState *s = bs->opaque;
 VmdkExtent *extent;
 
-s->desc_offset = 0x200;
-ret = bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header));
+ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header));
 if (ret < 0) {
-goto fail;
+return ret;
 }
 extent = vmdk_add_extent(bs,
  bs->file, false,
@@ -378,58 +389,45 @@ static int vmdk_open_vmdk3(BlockDriverState *bs, int 
flags)
  le32_to_cpu(header.granularity));
 ret = vmdk_init_tables(bs, extent);
 if (ret) {
-/* vmdk_init_tables cleans up on fail, so only free allocation of
- * vmdk_add_extent here. */
-goto fail;
+/* free extent allocated by vmdk_add_extent */
+vmdk_free_last_extent(bs);
 }
-return 0;
- fail:
-vmdk_free_extents(bs);
 return ret;
 }
 
-static int vmdk_open_vmdk4(BlockDriverState *bs, int flags)
+static int vmdk_open_vmdk4(BlockDriverState *bs,
+   BlockDriverState *file,
+   int flags)
 {
 int ret;
 uint32_t magic;
 uint32_t l1_size, l1_entry_sectors;
 VMDK4Header header;
-BDRVVmdkState *s = bs->opaque;
 VmdkExtent *extent;
 
-s->desc_offset = 0x200;
-ret = bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header));
+ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header));
 if (ret < 0) {
-goto fail;
+return ret;
 }
 l1_entry_sectors = le32_to_cpu(header.num_gtes_per_gte)
 * le64_to_cpu(header.granularity);
+if (l1_entry_sectors <= 0) {
+return -EINVAL;
+}
 l1_size = (le64_to_cpu(header.capacity) + l1_entry_sectors - 1)
 / l1_entry_sectors;
-extent = vmdk_add_extent(bs, bs->file, false,
+extent = vmdk_add_extent(bs, file, false,
   le64_to_cpu(header.capacity),
   le64_to_cpu(header.gd_offset) << 9,
   le64_to_cpu(header.rgd_offset) << 9,
   l1_size,
   le32_to_cpu(header.num_gtes_per_gte),
   le64_to_cpu(header.granularity));
-if (extent->l1_entry_sectors <= 0) {
-ret = -EINVAL;
-goto fail;
-}
-/* try to open parent images, if exist */
-ret = vmdk_parent_open(bs);
-if (ret) {
-goto fail;
-}
-s->parent_cid = vmdk_read_cid(bs, 1);
 ret = vmdk_init_tables(bs, extent);
 if (ret) {
-goto fail;
+/* free extent allocated by vmdk_add_extent */
+vmdk_free_last_extent(bs);
 }
-return 0;
- fail:
-vmdk_free_extents(bs);
 return ret;
 }
 
@@ -460,6 +458,31 @@ static int vmdk_parse_description(const char *desc, const 
char *opt_name,
 return 0;
 }
 
+/* Open an extent file and append to bs array */
+static int vmdk_open_sparse(BlockDriverState *bs,
+BlockDriverState *file,
+int flags)
+{
+uint32_t magic;
+
+if (bdrv_pread(file, 0, &magic, sizeof(magic)) != sizeof(magic)) {
+return -EIO;
+}
+
+magic = be32_to_cpu(magic);
+switch (magic) {
+case VMDK3_MAGIC:
+return vmdk_open_vmdk3(bs, file, flags);
+break;
+case VMDK4_MAGIC:
+return vmdk_open_vmdk4(bs, file, flags);
+break;
+default:
+return -EINVAL;
+break;
+}
+}
+
 static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
 const char *desc_file_path)
 {
@@ -470,6 +493,8 @@ static int vmdk_parse_extents(const char *desc, 
BlockDriverState *bs,
 const char *p = desc;
 int64_t sectors = 0;
 int64_t flat_offset;
+char exten

Re: [Qemu-devel] 0.15.0-rc2 (any version past 0.14.1) having issues with SLIRP on Windows XP host

[TeLeMan]

On Sat, Aug 6, 2011 at 04:46, Blue Swirl  wrote:
> On Fri, Aug 5, 2011 at 8:09 PM, Kenneth Salerno
>  wrote:
>> Hi,
>>
>> I'm not sure if any defaults (build or runtime) have changed since 0.14.1, 
>> but I can no longer get the following to work anymore for QEMU versions 
>> 0.15.0-rc2 or recent development builds:
>>
>>  -device e1000,netdev=mynet0 -netdev type=user,id=mynet0 ...
>>
>> Works great in 0.14.1 however.
>>
>> From the QEMU console, "info networking" shows the NIC e1000 and the VLAN 
>> correctly setup, the guest (RHEL 6.1 x86_64) has its NIC recognized and 
>> networking setup, just can't seem to communicate with the gateway 
>> (10.0.2.2). The only difference I see in the console is cosmetic 
>> (restricted=off rather than restricted=n).
>>
>> Host OS: Windows XP
>> Build env: i686-pc-mingw32-gcc 4.5.2, binutils 2.21.53.20110731 i386pe
>> Runtime env: Cygwin 1.7.9 2011-03-29, SDL 1.2.14, mingw32-glib 2.28.1-1,
>>             mingw32-gettext 0.18.1-2
>> Guest OS: RHEL 6.1
>>
>> Is it just me?
>
> No, this is fallout from glib use:
> http://lists.nongnu.org/archive/html/qemu-devel/2011-08/msg00134.html
>
> The fix is to rewrite structures without using GCC bit fields.

-mms-bitfields affects all byte-alignments in a structure. For example,
struct s
{
   uint8_t a;
   uint32_t b;
} __attribute__((packed));

sizeof(s) is 5 without -mms-bitfields but sizeof(s) is 8 with -mms-bitfields.

[Qemu-devel] Solde Hébergement Web !

[NovaHoster en Tunisie]

Solde d'hébergement Web chez NovaHoster.com



Du 01 au 20 Août 2011, pour tout achat d’un pack PRO 


Vous bénéficiez de deux noms de domaine gratuits à vie.



Commander maintenant:

http://ptf.tweensa.com/link.php?M=4129515&N=121&L=39&F=T

Re: [Qemu-devel] [SeaBIOS] Time to cut a new release?

[Jonathan A. Kollasch]

On Fri, Aug 05, 2011 at 04:59:44PM -0600, Marc Jones wrote:
> On Thu, Aug 4, 2011 at 7:19 PM, Kevin O'Connor  wrote:
> > On Thu, Aug 04, 2011 at 02:10:10PM -0500, Anthony Liguori wrote:
> >> On 08/04/2011 01:49 PM, Gerd Hoffmann wrote:
> >> >Hi,
> >> >
> >> >A bunch of new stuff has been added to master since the last version,
> >> >time to release a new one? Especially I'd like to see qemu get a seabios
> >> >update, and anthony prefers a release instead of a git snapshot for that.
> >>
> >> What's left in order to enable CONFIG_AHCI by default too?  I'd
> >> really prefer not to carry a custom config for QEMU if it's
> >> possible.
> >
> > I'm okay with defaulting AHCI on.  The 32bit trampolining could
> > theoretically be a problem, but there have been no trouble reports to
> > date.
> >
> >> Maybe this is a good time to coordinate a release around the next
> >> QEMU release too.  I'm guessing that October would be a pretty good
> >> time to freeze on a new SeaBIOS so if there was an October release,
> >> that would work very well for us.
> >
> > I'm okay with that.  We could probably arrange a release even sooner.
> 
> I think that AHCI on would be good. What about ATA DMA and ATA 32bit PIO?

ATA DMA caused me a bit of pain this morning, it didn't want to work.

32-bit PIO is probably mostly-safe these days.

Jonathan Kollasch

Re: [Qemu-devel] 0.15.0-rc2 (any version past 0.14.1) having issues with SLIRP on Windows XP host

[Kenneth Salerno]

--- On Fri, 8/5/11, Jan Kiszka  wrote:

> From: Jan Kiszka 
> Subject: Re: 0.15.0-rc2 (any version past 0.14.1) having issues with SLIRP on 
> Windows XP host
> To: "Blue Swirl" , "Kenneth Salerno" 
> 
> Cc: qemu-devel@nongnu.org
> Date: Friday, August 5, 2011, 5:43 PM
> On 2011-08-05 22:46, Blue Swirl
> wrote:
> > On Fri, Aug 5, 2011 at 8:09 PM, Kenneth Salerno
> > 
> wrote:
> >> Hi,
> >>
> >> I'm not sure if any defaults (build or runtime)
> have changed since 0.14.1, but I can no longer get the
> following to work anymore for QEMU versions 0.15.0-rc2 or
> recent development builds:
> >>
> >>  -device e1000,netdev=mynet0 -netdev
> type=user,id=mynet0 ...
> >>
> >> Works great in 0.14.1 however.
> >>
> >> From the QEMU console, "info networking" shows the
> NIC e1000 and the VLAN correctly setup, the guest (RHEL 6.1
> x86_64) has its NIC recognized and networking setup, just
> can't seem to communicate with the gateway (10.0.2.2). The
> only difference I see in the console is cosmetic
> (restricted=off rather than restricted=n).
> >>
> >> Host OS: Windows XP
> >> Build env: i686-pc-mingw32-gcc 4.5.2, binutils
> 2.21.53.20110731 i386pe
> >> Runtime env: Cygwin 1.7.9 2011-03-29, SDL 1.2.14,
> mingw32-glib 2.28.1-1,
> >>         
>    mingw32-gettext 0.18.1-2
> >> Guest OS: RHEL 6.1
> >>
> >> Is it just me?
> > 
> > No, this is fallout from glib use:
> > http://lists.nongnu.org/archive/html/qemu-devel/2011-08/msg00134.html
> > 
> > The fix is to rewrite structures without using GCC bit
> fields.
> 
> Does this help?
> 
> diff --git a/slirp/ip.h b/slirp/ip.h
> index 48ea38e..72dbe9a 100644
> --- a/slirp/ip.h
> +++ b/slirp/ip.h
> @@ -74,10 +74,10 @@ typedef uint32_t n_long;   
>              /* long
> as received from the net */
>   */
>  struct ip {
>  #ifdef HOST_WORDS_BIGENDIAN
> -    u_int ip_v:4,   
>         /* version */
> +    uint8_t ip_v:4,   
>         /* version */
>         
> ip_hl:4;        /* header
> length */
>  #else
> -    u_int ip_hl:4,   
>     /* header length */
> +    uint8_t ip_hl:4,   
>     /* header length */
>         
> ip_v:4;       
>     /* version */
>  #endif
>      uint8_t   
>     ip_tos;   
>         /* type of service */
> @@ -140,10 +140,10 @@ struct    ip_timestamp
> {
>      uint8_t   
> ipt_len;        /* size of
> structure (variable) */
>      uint8_t   
> ipt_ptr;        /* index of
> current entry */
>  #ifdef HOST_WORDS_BIGENDIAN
> -    u_int   
> ipt_oflw:4,        /* overflow
> counter */
> +    uint8_t   
> ipt_oflw:4,        /* overflow
> counter */
>         
> ipt_flg:4;        /* flags,
> see below */
>  #else
> -    u_int   
> ipt_flg:4,        /* flags,
> see below */
> +    uint8_t   
> ipt_flg:4,        /* flags,
> see below */
>         
> ipt_oflw:4;        /* overflow
> counter */
>  #endif
>      union ipt_timestamp {
> diff --git a/slirp/tcp.h b/slirp/tcp.h
> index 9d06836..b3817cb 100644
> --- a/slirp/tcp.h
> +++ b/slirp/tcp.h
> @@ -51,10 +51,10 @@ struct tcphdr {
>      tcp_seq   
> th_seq;       
>     /* sequence number */
>      tcp_seq   
> th_ack;       
>     /* acknowledgement number */
>  #ifdef HOST_WORDS_BIGENDIAN
> -    u_int   
> th_off:4,        /* data
> offset */
> +    uint8_t   
> th_off:4,        /* data
> offset */
>         
> th_x2:4;        /* (unused)
> */
>  #else
> -    u_int   
> th_x2:4,        /* (unused)
> */
> +    uint8_t   
> th_x2:4,        /* (unused)
> */
>         
> th_off:4;        /* data
> offset */
>  #endif
>      uint8_t th_flags;
> 
> Jan
> 
> 


With this patch it gets caught up in tcg/tcg.c line 1646:
  
if (ts->val_type == TEMP_VAL_REG)
...
else if (ts->val_type == TEMP_VAL_MEM)
...
else if (ts->val_type == TEMP_VAL_CONST)

} else {  <--- we get here by changing unsigned int to unsigned 
character
tcg_abort();
}



Output from QEMU:
  
/home/kens/cross-compile/qemu/testing/qemu/tcg/tcg.c:1646: tcg fatal error

This application has requested the Runtime to terminate it in an unusual way.
Please contact the application's support team for more information.

Thanks,
Ken

Re: [Qemu-devel] [SeaBIOS] Time to cut a new release?

[Marc Jones]

On Thu, Aug 4, 2011 at 7:19 PM, Kevin O'Connor  wrote:
> On Thu, Aug 04, 2011 at 02:10:10PM -0500, Anthony Liguori wrote:
>> On 08/04/2011 01:49 PM, Gerd Hoffmann wrote:
>> >Hi,
>> >
>> >A bunch of new stuff has been added to master since the last version,
>> >time to release a new one? Especially I'd like to see qemu get a seabios
>> >update, and anthony prefers a release instead of a git snapshot for that.
>>
>> What's left in order to enable CONFIG_AHCI by default too?  I'd
>> really prefer not to carry a custom config for QEMU if it's
>> possible.
>
> I'm okay with defaulting AHCI on.  The 32bit trampolining could
> theoretically be a problem, but there have been no trouble reports to
> date.
>
>> Maybe this is a good time to coordinate a release around the next
>> QEMU release too.  I'm guessing that October would be a pretty good
>> time to freeze on a new SeaBIOS so if there was an October release,
>> that would work very well for us.
>
> I'm okay with that.  We could probably arrange a release even sooner.

I think that AHCI on would be good. What about ATA DMA and ATA 32bit PIO?

Marc


-- 
http://se-eng.com

Re: [Qemu-devel] another TCG branch weirdness

[Artyom Tarasenko]

On Fri, Aug 5, 2011 at 10:32 PM, Blue Swirl  wrote:
> On Fri, Aug 5, 2011 at 4:36 PM, Artyom Tarasenko  wrote:
>> Host x86_64, guest sparc64. Found a case where a branch instruction
>> (brz,pn   %o0) unexpectedly jumps to an unexpected address. I.e.
>> branch shouldn't be taken at all, but even if it were it should have
>> been to 0x13e26e4 and not to 0x5.
>>
>> Was about to write that the generated OP for brz,pn usually looks
>> different, when realized that in fact it was even generated for this
>> very address just before, but with another branch in the delay slot.
>> The bug looks familiar, Blue, isn't it? :)
>
> Sorry, does not ring a bell.

I meant c27e275 where you fixed unconditional branch in a delay slot.
(One of my first bug reports).
Now it looks pretty similar for the conditional branches.

>> IN:
>> 0x013e26c0:  brz,pn   %o0, 0x13e26e4
>> 0x013e26c4:  brlez,pn   %o1, 0x13e26e4
>>
>> OP:
>>   0x13e26c0
>>  ld_i64 tmp6,regwptr,$0x0
>>  movi_i64 cond,$0x0
>>  movi_i64 tmp8,$0x0
>>  brcond_i64 tmp6,tmp8,ne,$0x0
>>  movi_i64 cond,$0x1
>>  set_label $0x0
>>
>> ^^^ Ok, that's how brz,pn  usually looks like
>>
>>   0x13e26c4
>>  ld_i64 tmp7,regwptr,$0x8
>>  movi_i64 tmp8,$0x0
>>  brcond_i64 cond,tmp8,eq,$0x1
>>  movi_i64 npc,$0x13e26e4
>>  br $0x2
>>  set_label $0x1
>>  movi_i64 npc,$0x13e26c8
>>  set_label $0x2
>>  movi_i64 cond,$0x0
>>  movi_i64 tmp8,$0x0
>>  brcond_i64 tmp7,tmp8,gt,$0x3
>>  movi_i64 cond,$0x1
>>  set_label $0x3
>>  movi_i64 tmp0,$0x0
>>  brcond_i64 cond,tmp0,eq,$0x4
>>  movi_i64 npc,$0x13e26e4
>>  br $0x5
>>  set_label $0x4
>>  movi_i64 npc,$0x5
>>  set_label $0x5
>>  exit_tb $0x0
>> --
>> IN:
>> 0x013e26c0:  brz,pn   %o0, 0x13e26e4
>>
>> OP:
>>   0x13e26c0
>>  ld_i64 tmp6,regwptr,$0x0
>>  movi_i64 cond,$0x0
>>  movi_i64 tmp8,$0x0
>>  brcond_i64 tmp6,tmp8,ne,$0x0
>>  movi_i64 cond,$0x1
>>  set_label $0x0
>>  movi_i64 pc,$0x5
>>
>> ^^^ What's that?
>
> Probably DYNAMIC_PC + 4. I guess we are hitting this ancient comment
> in target-sparc/translate.c:1372:
> /* XXX: potentially incorrect if dynamic npc */

Yes, I think this too. The following patch passes my tests. Do you
think it's correct? If yes, I'll make it for the other branches too.

@@ -1384,8 +1399,14 @@ static void do_branch_reg(DisasContext *dc,
int32_t offset, uint32_t insn,
 } else {
 dc->pc = dc->npc;
 dc->jump_pc[0] = target;
-dc->jump_pc[1] = dc->npc + 4;
-dc->npc = JUMP_PC;
+if (unlikely(dc->npc == DYNAMIC_PC)) {
+dc->jump_pc[1] = DYNAMIC_PC;
+tcg_gen_addi_tl(cpu_pc, cpu_npc, 4);
+
+} else {
+dc->jump_pc[1] = dc->npc + 4;
+dc->npc = JUMP_PC;
+}


Regards,
Artyom Tarasenko

solaris/sparc under qemu blog: http://tyom.blogspot.com/

Re: [Qemu-devel] 0.15.0-rc2 (any version past 0.14.1) having issues with SLIRP on Windows XP host

[Jan Kiszka]

On 2011-08-05 22:46, Blue Swirl wrote:
> On Fri, Aug 5, 2011 at 8:09 PM, Kenneth Salerno
>  wrote:
>> Hi,
>>
>> I'm not sure if any defaults (build or runtime) have changed since 0.14.1, 
>> but I can no longer get the following to work anymore for QEMU versions 
>> 0.15.0-rc2 or recent development builds:
>>
>>  -device e1000,netdev=mynet0 -netdev type=user,id=mynet0 ...
>>
>> Works great in 0.14.1 however.
>>
>> From the QEMU console, "info networking" shows the NIC e1000 and the VLAN 
>> correctly setup, the guest (RHEL 6.1 x86_64) has its NIC recognized and 
>> networking setup, just can't seem to communicate with the gateway 
>> (10.0.2.2). The only difference I see in the console is cosmetic 
>> (restricted=off rather than restricted=n).
>>
>> Host OS: Windows XP
>> Build env: i686-pc-mingw32-gcc 4.5.2, binutils 2.21.53.20110731 i386pe
>> Runtime env: Cygwin 1.7.9 2011-03-29, SDL 1.2.14, mingw32-glib 2.28.1-1,
>> mingw32-gettext 0.18.1-2
>> Guest OS: RHEL 6.1
>>
>> Is it just me?
> 
> No, this is fallout from glib use:
> http://lists.nongnu.org/archive/html/qemu-devel/2011-08/msg00134.html
> 
> The fix is to rewrite structures without using GCC bit fields.

Does this help?

diff --git a/slirp/ip.h b/slirp/ip.h
index 48ea38e..72dbe9a 100644
--- a/slirp/ip.h
+++ b/slirp/ip.h
@@ -74,10 +74,10 @@ typedef uint32_t n_long; /* long as 
received from the net */
  */
 struct ip {
 #ifdef HOST_WORDS_BIGENDIAN
-   u_int ip_v:4,   /* version */
+   uint8_t ip_v:4, /* version */
ip_hl:4;/* header length */
 #else
-   u_int ip_hl:4,  /* header length */
+   uint8_t ip_hl:4,/* header length */
ip_v:4; /* version */
 #endif
uint8_t ip_tos; /* type of service */
@@ -140,10 +140,10 @@ structip_timestamp {
uint8_t ipt_len;/* size of structure (variable) */
uint8_t ipt_ptr;/* index of current entry */
 #ifdef HOST_WORDS_BIGENDIAN
-   u_int   ipt_oflw:4, /* overflow counter */
+   uint8_t ipt_oflw:4, /* overflow counter */
ipt_flg:4;  /* flags, see below */
 #else
-   u_int   ipt_flg:4,  /* flags, see below */
+   uint8_t ipt_flg:4,  /* flags, see below */
ipt_oflw:4; /* overflow counter */
 #endif
union ipt_timestamp {
diff --git a/slirp/tcp.h b/slirp/tcp.h
index 9d06836..b3817cb 100644
--- a/slirp/tcp.h
+++ b/slirp/tcp.h
@@ -51,10 +51,10 @@ struct tcphdr {
tcp_seq th_seq; /* sequence number */
tcp_seq th_ack; /* acknowledgement number */
 #ifdef HOST_WORDS_BIGENDIAN
-   u_int   th_off:4,   /* data offset */
+   uint8_t th_off:4,   /* data offset */
th_x2:4;/* (unused) */
 #else
-   u_int   th_x2:4,/* (unused) */
+   uint8_t th_x2:4,/* (unused) */
th_off:4;   /* data offset */
 #endif
uint8_t th_flags;

Jan



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH 5/6] qemu-x86: Set tsc_khz in kvm when supported

[Jan Kiszka]

On 2011-08-05 22:55, Marcelo Tosatti wrote:
> From: Joerg Roedel 
> 
> Make use of the KVM_TSC_CONTROL feature if available.
> 
> Signed-off-by: Joerg Roedel 
> Signed-off-by: Marcelo Tosatti 
> ---
>  target-i386/kvm.c |   18 +-
>  1 files changed, 17 insertions(+), 1 deletions(-)
> 
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 10fb2c4..923d2d5 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -354,6 +354,7 @@ int kvm_arch_init_vcpu(CPUState *env)
>  uint32_t unused;
>  struct kvm_cpuid_entry2 *c;
>  uint32_t signature[3];
> +int r;
>  
>  env->cpuid_features &= kvm_arch_get_supported_cpuid(s, 1, 0, R_EDX);
>  
> @@ -499,7 +500,22 @@ int kvm_arch_init_vcpu(CPUState *env)
>  
>  qemu_add_vm_change_state_handler(cpu_update_state, env);
>  
> -return kvm_vcpu_ioctl(env, KVM_SET_CPUID2, &cpuid_data);
> +r = kvm_vcpu_ioctl(env, KVM_SET_CPUID2, &cpuid_data);
> +if (r)
> + return r;

Coding style...

Jan



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH] monitor: HMP: fix consecutive integer expression parsing

[Blue Swirl]

On Fri, Aug 5, 2011 at 9:08 PM, Anthony Liguori  wrote:
> On 08/05/2011 03:39 PM, Blue Swirl wrote:
>>
>> On Fri, Aug 5, 2011 at 4:51 PM, Anthony Liguori
>>  wrote:
>>>
>>> On 08/03/2011 06:57 AM, Alon Levy wrote:

 Currently a command that takes two consecutive integer operations, like
 client_migrate_info, will be incorrectly parsed by the human monitor if
 the second expression begins with a minus ('-') or plus ('+') sign:

 client_migrate_info            
 client_migrate_info spice localhost 5900 -1
 =>    port = 5899 = 5900 - 1
    tls-port = -1
 But expected by the user to be:
    port = 5900
    tls-port = -1

 The fix is that for any required integer (ilM) expression followed by
 another
 integer expression (ilM) the first expression will be parsed by
 expr_unary
 instead of expr_sum. So you can still use arithmetic, but you have to
 enclose
 it in parenthesis:

 Command line | Old parsed result | With patch result
 (1+1) 2      | 2, 2              | 2, 2
 1 -1         | 0, -1             | 1, -1
 The rest are bizarre but not any worse then before
 1+2+3        | 6, 5              | 1, 5
 (1+2)+3      | 3, 3              | 3, 3
>>>
>>> I vote for just removing the expression parsing entirely.  It's
>>> incredibly
>>> non-intuitive and I don't think anyone really uses it.
>>>
>>> Does anyone strongly object?
>>
>> I think the expressions would be useful with memory addresses, like
>> "xp/i $pc-4", but I usually start GDB in these cases. Can we disable
>> the expressions only for ports?
>
> Not sure what you mean by ports.  You mean for anything but vc?  My goal in
> disabling the expressions would be to simplify the parsing by removing all
> that messy code.

Retain the parsing for only memory addresses, remove from other areas.
Another way would be to require any expressions to be enclosed in
parentheses for all cases.

But I don't object to removing the code very much, as I said I use
GDB. Also the setjmp stuff is buggy.

[Qemu-devel] [PATCH 0/6] [PULL] qemu-kvm.git uq/master queue

[Marcelo Tosatti]

The following changes since commit 35d7ace74bd07e3d6983c1fd7cbfab4e11175689:

  qcow2: Fix L1 table size after bdrv_snapshot_goto (2011-08-05 07:15:47 -0500)

are available in the git repository at:
  git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git uq/master

Jan Kiszka (2):
  kvmclock: Fix feature detection
  kvm: Drop obsolete KVM_IOEVENTFD #ifdefs

Joerg Roedel (3):
  qemu: Add strtosz_suffix_unit function
  qemu-x86: Add tsc_freq option to -cpu
  qemu-x86: Set tsc_khz in kvm when supported

Marcelo Tosatti (1):
  Remove #ifdef KVM_CAP_TSC_CONTROL

 cutils.c|   16 +++-
 hw/kvmclock.c   |7 ++-
 kvm-all.c   |8 
 qemu-common.h   |2 ++
 target-i386/cpu.h   |1 +
 target-i386/cpuid.c |   13 +
 target-i386/kvm.c   |   16 +++-
 7 files changed, 44 insertions(+), 19 deletions(-)

Re: [Qemu-devel] [PATCH] monitor: HMP: fix consecutive integer expression parsing

[Anthony Liguori]

On 08/05/2011 03:39 PM, Blue Swirl wrote:

On Fri, Aug 5, 2011 at 4:51 PM, Anthony Liguori  wrote:

On 08/03/2011 06:57 AM, Alon Levy wrote:


Currently a command that takes two consecutive integer operations, like
client_migrate_info, will be incorrectly parsed by the human monitor if
the second expression begins with a minus ('-') or plus ('+') sign:

client_migrate_info
client_migrate_info spice localhost 5900 -1
=>port = 5899 = 5900 - 1
tls-port = -1
But expected by the user to be:
port = 5900
tls-port = -1

The fix is that for any required integer (ilM) expression followed by
another
integer expression (ilM) the first expression will be parsed by expr_unary
instead of expr_sum. So you can still use arithmetic, but you have to
enclose
it in parenthesis:

Command line | Old parsed result | With patch result
(1+1) 2  | 2, 2  | 2, 2
1 -1 | 0, -1 | 1, -1
The rest are bizarre but not any worse then before
1+2+3| 6, 5  | 1, 5
(1+2)+3  | 3, 3  | 3, 3


I vote for just removing the expression parsing entirely.  It's incredibly
non-intuitive and I don't think anyone really uses it.

Does anyone strongly object?


I think the expressions would be useful with memory addresses, like
"xp/i $pc-4", but I usually start GDB in these cases. Can we disable
the expressions only for ports?


Not sure what you mean by ports.  You mean for anything but vc?  My goal 
in disabling the expressions would be to simplify the parsing by 
removing all that messy code.


Regards,

Anthony Liguori

[Qemu-devel] [PATCH 3/6] qemu: Add strtosz_suffix_unit function

[Marcelo Tosatti]

From: Joerg Roedel 

This function does the same as the strtosz_suffix function
except that it allows to specify the unit to which the
k/M/B/T suffixes apply. This function will be used later to
parse the tsc-frequency from the command-line.

Signed-off-by: Joerg Roedel 
Signed-off-by: Marcelo Tosatti 
---
 cutils.c  |   16 +++-
 qemu-common.h |2 ++
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/cutils.c b/cutils.c
index f9a7e36..28049e0 100644
--- a/cutils.c
+++ b/cutils.c
@@ -322,7 +322,8 @@ int fcntl_setfl(int fd, int flag)
  * value must be terminated by whitespace, ',' or '\0'. Return -1 on
  * error.
  */
-int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
+int64_t strtosz_suffix_unit(const char *nptr, char **end,
+const char default_suffix, int64_t unit)
 {
 int64_t retval = -1;
 char *endptr;
@@ -362,20 +363,20 @@ int64_t strtosz_suffix(const char *nptr, char **end, 
const char default_suffix)
 }
 break;
 case STRTOSZ_DEFSUFFIX_KB:
-mul = 1 << 10;
+mul = unit;
 break;
 case 0:
 if (mul_required) {
 goto fail;
 }
 case STRTOSZ_DEFSUFFIX_MB:
-mul = 1ULL << 20;
+mul = unit * unit;
 break;
 case STRTOSZ_DEFSUFFIX_GB:
-mul = 1ULL << 30;
+mul = unit * unit * unit;
 break;
 case STRTOSZ_DEFSUFFIX_TB:
-mul = 1ULL << 40;
+mul = unit * unit * unit * unit;
 break;
 default:
 goto fail;
@@ -405,6 +406,11 @@ fail:
 return retval;
 }
 
+int64_t strtosz_suffix(const char *nptr, char **end, const char default_suffix)
+{
+return strtosz_suffix_unit(nptr, end, default_suffix, 1024);
+}
+
 int64_t strtosz(const char *nptr, char **end)
 {
 return strtosz_suffix(nptr, end, STRTOSZ_DEFSUFFIX_MB);
diff --git a/qemu-common.h b/qemu-common.h
index afbd04d..389f4d2 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -157,6 +157,8 @@ int fcntl_setfl(int fd, int flag);
 #define STRTOSZ_DEFSUFFIX_B'B'
 int64_t strtosz(const char *nptr, char **end);
 int64_t strtosz_suffix(const char *nptr, char **end, const char 
default_suffix);
+int64_t strtosz_suffix_unit(const char *nptr, char **end,
+const char default_suffix, int64_t unit);
 
 /* path.c */
 void init_paths(const char *prefix);
-- 
1.7.5.4

[Qemu-devel] [PATCH 2/6] kvm: Drop obsolete KVM_IOEVENTFD #ifdefs

[Marcelo Tosatti]

From: Jan Kiszka 

Signed-off-by: Jan Kiszka 
Signed-off-by: Marcelo Tosatti 
---
 kvm-all.c |8 
 1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index cbc2532..b9c172b 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1328,7 +1328,6 @@ int kvm_set_signal_mask(CPUState *env, const sigset_t 
*sigset)
 
 int kvm_set_ioeventfd_mmio_long(int fd, uint32_t addr, uint32_t val, bool 
assign)
 {
-#ifdef KVM_IOEVENTFD
 int ret;
 struct kvm_ioeventfd iofd;
 
@@ -1353,14 +1352,10 @@ int kvm_set_ioeventfd_mmio_long(int fd, uint32_t addr, 
uint32_t val, bool assign
 }
 
 return 0;
-#else
-return -ENOSYS;
-#endif
 }
 
 int kvm_set_ioeventfd_pio_word(int fd, uint16_t addr, uint16_t val, bool 
assign)
 {
-#ifdef KVM_IOEVENTFD
 struct kvm_ioeventfd kick = {
 .datamatch = val,
 .addr = addr,
@@ -1380,9 +1375,6 @@ int kvm_set_ioeventfd_pio_word(int fd, uint16_t addr, 
uint16_t val, bool assign)
 return r;
 }
 return 0;
-#else
-return -ENOSYS;
-#endif
 }
 
 int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr)
-- 
1.7.5.4

[Qemu-devel] [PATCH 5/6] qemu-x86: Set tsc_khz in kvm when supported

[Marcelo Tosatti]

From: Joerg Roedel 

Make use of the KVM_TSC_CONTROL feature if available.

Signed-off-by: Joerg Roedel 
Signed-off-by: Marcelo Tosatti 
---
 target-i386/kvm.c |   18 +-
 1 files changed, 17 insertions(+), 1 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 10fb2c4..923d2d5 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -354,6 +354,7 @@ int kvm_arch_init_vcpu(CPUState *env)
 uint32_t unused;
 struct kvm_cpuid_entry2 *c;
 uint32_t signature[3];
+int r;
 
 env->cpuid_features &= kvm_arch_get_supported_cpuid(s, 1, 0, R_EDX);
 
@@ -499,7 +500,22 @@ int kvm_arch_init_vcpu(CPUState *env)
 
 qemu_add_vm_change_state_handler(cpu_update_state, env);
 
-return kvm_vcpu_ioctl(env, KVM_SET_CPUID2, &cpuid_data);
+r = kvm_vcpu_ioctl(env, KVM_SET_CPUID2, &cpuid_data);
+if (r)
+   return r;
+
+#ifdef KVM_CAP_TSC_CONTROL
+r = kvm_check_extension(env->kvm_state, KVM_CAP_TSC_CONTROL);
+if (r && env->tsc_khz) {
+r = kvm_vcpu_ioctl(env, KVM_SET_TSC_KHZ, env->tsc_khz);
+if (r < 0) {
+fprintf(stderr, "KVM_SET_TSC_KHZ failed\n");
+return r;
+}
+}
+#endif
+
+return 0;
 }
 
 void kvm_arch_reset_vcpu(CPUState *env)
-- 
1.7.5.4

[Qemu-devel] [PATCH 4/6] qemu-x86: Add tsc_freq option to -cpu

[Marcelo Tosatti]

From: Joerg Roedel 

To let the user configure the desired tsc frequency for the
guest if running in KVM.

Signed-off-by: Joerg Roedel 
Signed-off-by: Marcelo Tosatti 
---
 target-i386/cpu.h   |1 +
 target-i386/cpuid.c |   13 +
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index dd6c5fa..dcdd95f 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -743,6 +743,7 @@ typedef struct CPUX86State {
 uint32_t cpuid_kvm_features;
 uint32_t cpuid_svm_features;
 bool tsc_valid;
+int tsc_khz;
 
 /* in order to simplify APIC support, we leave this pointer to the
user */
diff --git a/target-i386/cpuid.c b/target-i386/cpuid.c
index e1ae3af..89e9623 100644
--- a/target-i386/cpuid.c
+++ b/target-i386/cpuid.c
@@ -224,6 +224,7 @@ typedef struct x86_def_t {
 int family;
 int model;
 int stepping;
+int tsc_khz;
 uint32_t features, ext_features, ext2_features, ext3_features;
 uint32_t kvm_features, svm_features;
 uint32_t xlevel;
@@ -704,6 +705,17 @@ static int cpu_x86_find_by_name(x86_def_t *x86_cpu_def, 
const char *cpu_model)
 } else if (!strcmp(featurestr, "model_id")) {
 pstrcpy(x86_cpu_def->model_id, sizeof(x86_cpu_def->model_id),
 val);
+} else if (!strcmp(featurestr, "tsc_freq")) {
+int64_t tsc_freq;
+char *err;
+
+tsc_freq = strtosz_suffix_unit(val, &err,
+   STRTOSZ_DEFSUFFIX_B, 1000);
+if (!*val || *err) {
+fprintf(stderr, "bad numerical value %s\n", val);
+goto error;
+}
+x86_cpu_def->tsc_khz = tsc_freq / 1000;
 } else {
 fprintf(stderr, "unrecognized feature %s\n", featurestr);
 goto error;
@@ -872,6 +884,7 @@ int cpu_x86_register (CPUX86State *env, const char 
*cpu_model)
 env->cpuid_svm_features = def->svm_features;
 env->cpuid_ext4_features = def->ext4_features;
 env->cpuid_xlevel2 = def->xlevel2;
+env->tsc_khz = def->tsc_khz;
 if (!kvm_enabled()) {
 env->cpuid_features &= TCG_FEATURES;
 env->cpuid_ext_features &= TCG_EXT_FEATURES;
-- 
1.7.5.4

[Qemu-devel] [PATCH 1/6] kvmclock: Fix feature detection

[Marcelo Tosatti]

From: Jan Kiszka 

Bit-wise or the feature flags and drop the obsolete #ifdef.

Signed-off-by: Jan Kiszka 
Signed-off-by: Marcelo Tosatti 
---
 hw/kvmclock.c |7 ++-
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/hw/kvmclock.c b/hw/kvmclock.c
index 692ad18..b73aec4 100644
--- a/hw/kvmclock.c
+++ b/hw/kvmclock.c
@@ -101,11 +101,8 @@ static SysBusDeviceInfo kvmclock_info = {
 void kvmclock_create(void)
 {
 if (kvm_enabled() &&
-first_cpu->cpuid_kvm_features & ((1ULL << KVM_FEATURE_CLOCKSOURCE)
-#ifdef KVM_FEATURE_CLOCKSOURCE2
-|| (1ULL << KVM_FEATURE_CLOCKSOURCE2)
-#endif
-)) {
+first_cpu->cpuid_kvm_features & ((1ULL << KVM_FEATURE_CLOCKSOURCE) |
+ (1ULL << KVM_FEATURE_CLOCKSOURCE2))) {
 sysbus_create_simple("kvmclock", -1, NULL);
 }
 }
-- 
1.7.5.4

[Qemu-devel] [PATCH 6/6] Remove #ifdef KVM_CAP_TSC_CONTROL

[Marcelo Tosatti]

Signed-off-by: Marcelo Tosatti 
---
 target-i386/kvm.c |2 --
 1 files changed, 0 insertions(+), 2 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 923d2d5..31b88b7 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -504,7 +504,6 @@ int kvm_arch_init_vcpu(CPUState *env)
 if (r)
return r;
 
-#ifdef KVM_CAP_TSC_CONTROL
 r = kvm_check_extension(env->kvm_state, KVM_CAP_TSC_CONTROL);
 if (r && env->tsc_khz) {
 r = kvm_vcpu_ioctl(env, KVM_SET_TSC_KHZ, env->tsc_khz);
@@ -513,7 +512,6 @@ int kvm_arch_init_vcpu(CPUState *env)
 return r;
 }
 }
-#endif
 
 return 0;
 }
-- 
1.7.5.4

Re: [Qemu-devel] 0.15.0-rc2 (any version past 0.14.1) having issues with SLIRP on Windows XP host

[Blue Swirl]

On Fri, Aug 5, 2011 at 8:09 PM, Kenneth Salerno
 wrote:
> Hi,
>
> I'm not sure if any defaults (build or runtime) have changed since 0.14.1, 
> but I can no longer get the following to work anymore for QEMU versions 
> 0.15.0-rc2 or recent development builds:
>
>  -device e1000,netdev=mynet0 -netdev type=user,id=mynet0 ...
>
> Works great in 0.14.1 however.
>
> From the QEMU console, "info networking" shows the NIC e1000 and the VLAN 
> correctly setup, the guest (RHEL 6.1 x86_64) has its NIC recognized and 
> networking setup, just can't seem to communicate with the gateway (10.0.2.2). 
> The only difference I see in the console is cosmetic (restricted=off rather 
> than restricted=n).
>
> Host OS: Windows XP
> Build env: i686-pc-mingw32-gcc 4.5.2, binutils 2.21.53.20110731 i386pe
> Runtime env: Cygwin 1.7.9 2011-03-29, SDL 1.2.14, mingw32-glib 2.28.1-1,
>             mingw32-gettext 0.18.1-2
> Guest OS: RHEL 6.1
>
> Is it just me?

No, this is fallout from glib use:
http://lists.nongnu.org/archive/html/qemu-devel/2011-08/msg00134.html

The fix is to rewrite structures without using GCC bit fields.

Re: [Qemu-devel] [PATCH] monitor: HMP: fix consecutive integer expression parsing

[Blue Swirl]

On Fri, Aug 5, 2011 at 4:51 PM, Anthony Liguori  wrote:
> On 08/03/2011 06:57 AM, Alon Levy wrote:
>>
>> Currently a command that takes two consecutive integer operations, like
>> client_migrate_info, will be incorrectly parsed by the human monitor if
>> the second expression begins with a minus ('-') or plus ('+') sign:
>>
>> client_migrate_info      
>> client_migrate_info spice localhost 5900 -1
>> =>  port = 5899 = 5900 - 1
>>    tls-port = -1
>> But expected by the user to be:
>>    port = 5900
>>    tls-port = -1
>>
>> The fix is that for any required integer (ilM) expression followed by
>> another
>> integer expression (ilM) the first expression will be parsed by expr_unary
>> instead of expr_sum. So you can still use arithmetic, but you have to
>> enclose
>> it in parenthesis:
>>
>> Command line | Old parsed result | With patch result
>> (1+1) 2      | 2, 2              | 2, 2
>> 1 -1         | 0, -1             | 1, -1
>> The rest are bizarre but not any worse then before
>> 1+2+3        | 6, 5              | 1, 5
>> (1+2)+3      | 3, 3              | 3, 3
>
> I vote for just removing the expression parsing entirely.  It's incredibly
> non-intuitive and I don't think anyone really uses it.
>
> Does anyone strongly object?

I think the expressions would be useful with memory addresses, like
"xp/i $pc-4", but I usually start GDB in these cases. Can we disable
the expressions only for ports?

Re: [Qemu-devel] another TCG branch weirdness

[Blue Swirl]

On Fri, Aug 5, 2011 at 4:36 PM, Artyom Tarasenko  wrote:
> Host x86_64, guest sparc64. Found a case where a branch instruction
> (brz,pn   %o0) unexpectedly jumps to an unexpected address. I.e.
> branch shouldn't be taken at all, but even if it were it should have
> been to 0x13e26e4 and not to 0x5.
>
> Was about to write that the generated OP for brz,pn usually looks
> different, when realized that in fact it was even generated for this
> very address just before, but with another branch in the delay slot.
> The bug looks familiar, Blue, isn't it? :)

Sorry, does not ring a bell.

> IN:
> 0x013e26c0:  brz,pn   %o0, 0x13e26e4
> 0x013e26c4:  brlez,pn   %o1, 0x13e26e4
>
> OP:
>   0x13e26c0
>  ld_i64 tmp6,regwptr,$0x0
>  movi_i64 cond,$0x0
>  movi_i64 tmp8,$0x0
>  brcond_i64 tmp6,tmp8,ne,$0x0
>  movi_i64 cond,$0x1
>  set_label $0x0
>
> ^^^ Ok, that's how brz,pn  usually looks like
>
>   0x13e26c4
>  ld_i64 tmp7,regwptr,$0x8
>  movi_i64 tmp8,$0x0
>  brcond_i64 cond,tmp8,eq,$0x1
>  movi_i64 npc,$0x13e26e4
>  br $0x2
>  set_label $0x1
>  movi_i64 npc,$0x13e26c8
>  set_label $0x2
>  movi_i64 cond,$0x0
>  movi_i64 tmp8,$0x0
>  brcond_i64 tmp7,tmp8,gt,$0x3
>  movi_i64 cond,$0x1
>  set_label $0x3
>  movi_i64 tmp0,$0x0
>  brcond_i64 cond,tmp0,eq,$0x4
>  movi_i64 npc,$0x13e26e4
>  br $0x5
>  set_label $0x4
>  movi_i64 npc,$0x5
>  set_label $0x5
>  exit_tb $0x0
> --
> IN:
> 0x013e26c0:  brz,pn   %o0, 0x13e26e4
>
> OP:
>   0x13e26c0
>  ld_i64 tmp6,regwptr,$0x0
>  movi_i64 cond,$0x0
>  movi_i64 tmp8,$0x0
>  brcond_i64 tmp6,tmp8,ne,$0x0
>  movi_i64 cond,$0x1
>  set_label $0x0
>  movi_i64 pc,$0x5
>
> ^^^ What's that?

Probably DYNAMIC_PC + 4. I guess we are hitting this ancient comment
in target-sparc/translate.c:1372:
/* XXX: potentially incorrect if dynamic npc */

>  movi_i64 tmp0,$0x0
>  brcond_i64 cond,tmp0,eq,$0x1
>  movi_i64 npc,$0x13e26e4
>  br $0x2
>  set_label $0x1
>  movi_i64 npc,$0x9
>  set_label $0x2
>  exit_tb $0x0
>
>
>  33062: Instruction Access MMU Miss (v=0064) pc=0005
> npc=0009 SP=0c3d2d81
> ...
> Current Register Window:
> %o0-3: 02483d00 0018 0028 000232bd
>            ^^ not zero
>
>
> --
> Regards,
> Artyom Tarasenko
>
> solaris/sparc under qemu blog: http://tyom.blogspot.com/
>

Re: [Qemu-devel] Safely reopening image files by stashing fds

[Blue Swirl]

On Fri, Aug 5, 2011 at 8:40 AM, Stefan Hajnoczi  wrote:
> We've discussed safe methods for reopening image files (e.g. useful for
> changing the hostcache parameter).  The problem is that closing the file first
> and then opening it again exposes us to the error case where the open fails.
> At that point we cannot get to the file anymore and our options are to
> terminate QEMU, pause the VM, or offline the block device.
>
> This window of vulnerability can be eliminated by keeping the file descriptor
> around and falling back to it should the open fail.
>
> The challenge for the file descriptor approach is that image formats, like
> VMDK, can span multiple files.  Therefore the solution is not as simple as
> stashing a single file descriptor and reopening from it.
>
> Here is the outline for an fd stashing mechanism that can handle reopening
> multi-file images and could also solve the file descriptor passing problem for
> libvirt:
>
> 1. Extract monitor getfd/closefd functionality
>
> The monitor already supports fd stashing with getfd/closefd commands.  But the
> fd stash code is part of Monitor and we need to extract it into its own 
> object.
>
> /* A stashed file descriptor */
> typedef FDEntry {
>        const char *name;
>        int fd;
>        QLIST_ENTRY(FDEntry) next;
> } FDEntry;
>
> /* A container for stashing file descriptors */
> typedef struct FDStash {
>        QLIST_HEAD(, FDEntry) fds;
> } FDStash;
>
> void fdstash_init(FDStash *stash);
>
> /**
>  * Clear stashed file descriptors and close them
>  */
> void fdstash_cleanup(FDStash *stash);
>
> /**
>  * Stash a file descriptor and give up ownership
>  *
>  * If a file descriptor is already present with the same name the old fd is
>  * closed and replaced by the new one.
>  */
> void fdstash_give(FDStash *stash, const char *name, int fd);
>
> /**
>  * Find and take ownership of a stashed file descriptor
>  *
>  * Return the file descriptor or -ENOENT if not found.
>  */
> int fdstash_take(FDStash *stash, const char *name);
>
> The monitor is refactored to use this code instead of open coding fd stashing.
>
> 2. Introduce a function to extract open file descriptors from an block device
>
> Add a new .bdrv_extract_fds(BlockDriverState *bs, FDStash *stash) interface,
> which defaults to calling bdrv_extract_fds(bs->file, stash).
>
> VMDK and protocols can implement this function to support extracting open fds
> from a block device.  Note that they need to dup(2) fds before giving them to
> the fdstash, otherwise the fd will be closed when the block device is
> closed/deleted.
>
> 3. Rework bdrv_open() to take a FDStash
>
> Check the FDStash before opening an image file on the host file system.  This
> makes it possible to open an image file and use existing stashed fds.
>
> 4. Implement bdrv_reopen()
>
> First call bdrv_extract_fds() to stash the file descriptors, then close the
> block device.  Try opening the new image but if that fails, reopen using the
> stashed file descriptors.
>
> Thoughts?

I was previously thinking that the fd store should be tightly coupled
with block layer, but maybe it would be better to make this totally
generic like Avi(?) proposed, then all files (logs etc) could benefit
from this functionality.

I'd then handle fd stashing in qemu_open() etc. Stashing would not
need major changes anywhere, except reopen should be handled with
qemu_reopen() instead of qemu_close() + qemu_open() sequence. From the
other side (monitor), the interface could be like you present in 1.

[Qemu-devel] 0.15.0-rc2 (any version past 0.14.1) having issues with SLIRP on Windows XP host

[Kenneth Salerno]

Hi,

I'm not sure if any defaults (build or runtime) have changed since 0.14.1, but 
I can no longer get the following to work anymore for QEMU versions 0.15.0-rc2 
or recent development builds:

  -device e1000,netdev=mynet0 -netdev type=user,id=mynet0 ...

Works great in 0.14.1 however.

>From the QEMU console, "info networking" shows the NIC e1000 and the VLAN 
>correctly setup, the guest (RHEL 6.1 x86_64) has its NIC recognized and 
>networking setup, just can't seem to communicate with the gateway (10.0.2.2). 
>The only difference I see in the console is cosmetic (restricted=off rather 
>than restricted=n).

Host OS: Windows XP
Build env: i686-pc-mingw32-gcc 4.5.2, binutils 2.21.53.20110731 i386pe
Runtime env: Cygwin 1.7.9 2011-03-29, SDL 1.2.14, mingw32-glib 2.28.1-1,
 mingw32-gettext 0.18.1-2
Guest OS: RHEL 6.1

Is it just me?

Thanks,
Ken

Re: [Qemu-devel] [RFC PATCH 2/4] add pc-0.14 machine

[Anthony Liguori]

On 08/05/2011 02:26 PM, Bruce Rogers wrote:

So, do we not need this change, then, to go along with the 0.15 release?


Strictly speaking, the 0.15 machine type is identical to the 0.14 
machine type.  It wouldn't hurt to have a 0.15 machine alias but I don't 
think it's a strict requirement.


Regards,

Anthony Liguori


Bruce

  >>>  On 6/30/2011 at 09:46 AM, Paolo Bonzini  wrote:

The new pc-0.15 machine will have a different migration format, so
define the compatibility one right now.

Signed-off-by: Paolo Bonzini
---
  hw/pc_piix.c |   10 +-
  1 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/hw/pc_piix.c b/hw/pc_piix.c
index c5c16b4..18cc942 100644
--- a/hw/pc_piix.c
+++ b/hw/pc_piix.c
@@ -258,7 +258,7 @@ static void pc_xen_hvm_init(ram_addr_t ram_size,
  #endif

  static QEMUMachine pc_machine = {
-.name = "pc-0.14",
+.name = "pc-0.15",
  .alias = "pc",
  .desc = "Standard PC",
  .init = pc_init_pci,
@@ -266,6 +266,13 @@ static QEMUMachine pc_machine = {
  .is_default = 1,
  };

+static QEMUMachine pc_machine_v0_14 = {
+.name = "pc-0.14",
+.desc = "Standard PC",
+.init = pc_init_pci,
+.max_cpus = 255,
+};
+
  static QEMUMachine pc_machine_v0_13 = {
  .name = "pc-0.13",
  .desc = "Standard PC",
@@ -482,6 +489,7 @@ static QEMUMachine xenfv_machine = {
  static void pc_machine_init(void)
  {
  qemu_register_machine(&pc_machine);
+qemu_register_machine(&pc_machine_v0_14);
  qemu_register_machine(&pc_machine_v0_13);
  qemu_register_machine(&pc_machine_v0_12);
  qemu_register_machine(&pc_machine_v0_11);

Re: [Qemu-devel] [RFC PATCH 2/4] add pc-0.14 machine

[Bruce Rogers]

So, do we not need this change, then, to go along with the 0.15 release?

Bruce

 >>> On 6/30/2011 at 09:46 AM, Paolo Bonzini  wrote: 
> The new pc-0.15 machine will have a different migration format, so
> define the compatibility one right now.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  hw/pc_piix.c |   10 +-
>  1 files changed, 9 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/pc_piix.c b/hw/pc_piix.c
> index c5c16b4..18cc942 100644
> --- a/hw/pc_piix.c
> +++ b/hw/pc_piix.c
> @@ -258,7 +258,7 @@ static void pc_xen_hvm_init(ram_addr_t ram_size,
>  #endif
>  
>  static QEMUMachine pc_machine = {
> -.name = "pc-0.14",
> +.name = "pc-0.15",
>  .alias = "pc",
>  .desc = "Standard PC",
>  .init = pc_init_pci,
> @@ -266,6 +266,13 @@ static QEMUMachine pc_machine = {
>  .is_default = 1,
>  };
>  
> +static QEMUMachine pc_machine_v0_14 = {
> +.name = "pc-0.14",
> +.desc = "Standard PC",
> +.init = pc_init_pci,
> +.max_cpus = 255,
> +};
> +
>  static QEMUMachine pc_machine_v0_13 = {
>  .name = "pc-0.13",
>  .desc = "Standard PC",
> @@ -482,6 +489,7 @@ static QEMUMachine xenfv_machine = {
>  static void pc_machine_init(void)
>  {
>  qemu_register_machine(&pc_machine);
> +qemu_register_machine(&pc_machine_v0_14);
>  qemu_register_machine(&pc_machine_v0_13);
>  qemu_register_machine(&pc_machine_v0_12);
>  qemu_register_machine(&pc_machine_v0_11);

Re: [Qemu-devel] [PATCH] e1000: Do reset when E1000_CTRL_RST bit is set.

[Peter Maydell]

On 5 August 2011 17:53, Anthony Liguori  wrote:
> You'll break some GCCs with -Wall -Werror with this.  Please do:
>
> if ((val & E1000_CTRL_RST)) {

Hmm? There's lots of examples of that in the codebase:
$ git grep 'if ([a-zA-Z]* & ' | wc -l
1558

'=' (assignment) needs those extra braces, but logical
ops don't AFAIK.

-- PMM

Re: [Qemu-devel] [PATCH] e1000: Do reset when E1000_CTRL_RST bit is set.

[Richard Henderson]

On 08/05/2011 09:53 AM, Anthony Liguori wrote:
>> +if (val&  E1000_CTRL_RST) {
> 
> You'll break some GCCs with -Wall -Werror with this.  Please do:
> 
> if ((val & E1000_CTRL_RST)) {

Err, really?  What versions?

I don't recall that ever being true.


r~

[Qemu-devel] FW: Qemu instruction count in user mode

[Jain Diviya-B12553]

Hi,

I am currently using qemu in user mode. target platform is ppc , while host 
machine is x86. I am a newbie and want to understand how to count instructions 
of the target code. I see a number of posts etc , but they all seem to be for 
system mode.

I am also not able to generate execution trace.
using following command line, I am able to see input ams, output asm etc in 
/tmp/qemu.log
./bin/qemu-ppc -cpu e500v2 -d in_asm,out_asm my_bin

However when I give -d exec, I do not see any execution trace. How do I see 
execution trace. my only intent is to count number of target instructions 
executed. I am ok with getting a trace out and then count number of 
instructions. This is because I do not want to spend much time in writing 
complex functions just to count number of instructions in user mode.

Thank you,
Diviya

Re: [Qemu-devel] [PATCH 2/2] linux-user: Fix indirect syscall handling for MIPS

[An-Cheng Huang]

On Fri, Aug 05, 2011 at 10:27:21AM +0100, Peter Maydell wrote:
> On 5 August 2011 01:05, An-Cheng Huang  wrote:
> > Ok the following patch changes the number of arguments for sys_syscall
> > to 8 in mips_syscall_args and also skips the do_syscall() call if any
> > of the get_user() calls fails. Do you think combining these makes sense
> > or should they be two separate patches? Thanks.
> 
> The code in this patch looks good, but yes, I think they should
> be two separate patches.

And the second patch:

This patch verifies that MIPS syscall arguments are successfully taken from the 
stack before proceeding to do_syscall().

Signed-off-by: An-Cheng Huang 
---
 linux-user/main.c |   22 +-
 1 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 9e67b24..701d96e 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -2090,11 +2090,22 @@ void cpu_loop(CPUMIPSState *env)
 sp_reg = env->active_tc.gpr[29];
 switch (nb_args) {
 /* these arguments are taken from the stack */
-/* FIXME - what to do if get_user() fails? */
-case 8: get_user_ual(arg8, sp_reg + 28);
-case 7: get_user_ual(arg7, sp_reg + 24);
-case 6: get_user_ual(arg6, sp_reg + 20);
-case 5: get_user_ual(arg5, sp_reg + 16);
+case 8:
+if ((ret = get_user_ual(arg8, sp_reg + 28)) != 0) {
+goto done_syscall;
+}
+case 7:
+if ((ret = get_user_ual(arg7, sp_reg + 24)) != 0) {
+goto done_syscall;
+}
+case 6:
+if ((ret = get_user_ual(arg6, sp_reg + 20)) != 0) {
+goto done_syscall;
+}
+case 5:
+if ((ret = get_user_ual(arg5, sp_reg + 16)) != 0) {
+goto done_syscall;
+}
 default:
 break;
 }
@@ -2105,6 +2116,7 @@ void cpu_loop(CPUMIPSState *env)
  env->active_tc.gpr[7],
  arg5, arg6, arg7, arg8);
 }
+done_syscall:
 if (ret == -TARGET_QEMU_ESIGRETURN) {
 /* Returning from a successful sigreturn syscall.
Avoid clobbering register state.  */

Re: [Qemu-devel] [PATCH 1/2] linux-user: Fix indirect syscall handling for MIPS

[An-Cheng Huang]

On Fri, Aug 05, 2011 at 10:27:21AM +0100, Peter Maydell wrote:
> On 5 August 2011 01:05, An-Cheng Huang  wrote:
> > Ok the following patch changes the number of arguments for sys_syscall
> > to 8 in mips_syscall_args and also skips the do_syscall() call if any
> > of the get_user() calls fails. Do you think combining these makes sense
> > or should they be two separate patches? Thanks.
> 
> The code in this patch looks good, but yes, I think they should
> be two separate patches.

Here's the first patch for the indirect syscall:

This patch changes the number of arguments for sys_syscall on MIPS to 8, which 
allows the arguments for the actual syscall to be handled correctly.

Signed-off-by: An-Cheng Huang 
---
 linux-user/main.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 6a8f4bd..9e67b24 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -1669,7 +1669,7 @@ void cpu_loop(CPUPPCState *env)
 #define MIPS_SYS(name, args) args,
 
 static const uint8_t mips_syscall_args[] = {
-   MIPS_SYS(sys_syscall, 0)/* 4000 */
+   MIPS_SYS(sys_syscall, 8)/* 4000 */
MIPS_SYS(sys_exit   , 1)
MIPS_SYS(sys_fork   , 0)
MIPS_SYS(sys_read   , 3)

Re: [Qemu-devel] [PATCH] monitor: HMP: fix consecutive integer expression parsing

[Markus Armbruster]

Anthony Liguori  writes:

> On 08/03/2011 06:57 AM, Alon Levy wrote:
>> Currently a command that takes two consecutive integer operations, like
>> client_migrate_info, will be incorrectly parsed by the human monitor if
>> the second expression begins with a minus ('-') or plus ('+') sign:
>>
>> client_migrate_info  
>> client_migrate_info spice localhost 5900 -1
>> =>  port = 5899 = 5900 - 1
>> tls-port = -1
>> But expected by the user to be:
>> port = 5900
>> tls-port = -1
>>
>> The fix is that for any required integer (ilM) expression followed by another
>> integer expression (ilM) the first expression will be parsed by expr_unary
>> instead of expr_sum. So you can still use arithmetic, but you have to enclose
>> it in parenthesis:
>>
>> Command line | Old parsed result | With patch result
>> (1+1) 2  | 2, 2  | 2, 2
>> 1 -1 | 0, -1 | 1, -1
>> The rest are bizarre but not any worse then before
>> 1+2+3| 6, 5  | 1, 5
>> (1+2)+3  | 3, 3  | 3, 3
>
> I vote for just removing the expression parsing entirely.  It's
> incredibly non-intuitive and I don't think anyone really uses it.

Yes, please.

> Does anyone strongly object?

Re: [Qemu-devel] [PATCH 0/3] [PULL] slirp: ARP table related fixes

[Anthony Liguori]

On 08/05/2011 08:32 AM, Jan Kiszka wrote:

The following changes since commit 81e34a2401f7ffd519bb7f093e833cb48734169f:

   Merge remote-tracking branch 'mst/for_anthony' into staging (2011-08-04 
17:15:22 -0500)

are available in the git repository at:

   git://git.kiszka.org/qemu.git queues/slirp

Fixes some fall-outs from the latest merge, ie. a build breakage on
mingw and a regression that broke at least ping.


CC: Gerd Hoffmann


Pulled.  Thanks.

Regards,

Anthony Liguori



Jan Kiszka (3):
   slirp: Fix types of IP address parameters
   slirp: Read current time only once per if_start call
   slirp: Only start packet expiration for delayed ones

  slirp/arp_table.c |   16 
  slirp/if.c|8 +---
  slirp/slirp.c |3 +++
  slirp/slirp.h |4 ++--
  4 files changed, 14 insertions(+), 17 deletions(-)

Re: [Qemu-devel] [PATCH] when overriding default tool names don't add cross-prefix

[Anthony Liguori]

On 08/04/2011 05:10 PM, Stuart yoder wrote:

From: Stuart Yoder

When overriding a tool name via a shell variable, don't
tack on the cross-prefix.  This specifically allows the
pkg-config command to be overridden and work where it
does not exist in some cross build environments.

Signed-off-by: Stuart Yoder


Applied.  Thanks.

Regards,

Anthony Liguori


---
  configure |   16 
  1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/configure b/configure
index 38e3724..ad522f2 100755
--- a/configure
+++ b/configure
@@ -217,14 +217,14 @@ done
  # Using uname is really, really broken.  Once we have the right set of checks
  # we can eliminate it's usage altogether

-cc="${cross_prefix}${CC-gcc}"
-ar="${cross_prefix}${AR-ar}"
-objcopy="${cross_prefix}${OBJCOPY-objcopy}"
-ld="${cross_prefix}${LD-ld}"
-strip="${cross_prefix}${STRIP-strip}"
-windres="${cross_prefix}${WINDRES-windres}"
-pkg_config="${cross_prefix}${PKG_CONFIG-pkg-config}"
-sdl_config="${cross_prefix}${SDL_CONFIG-sdl-config}"
+cc="${CC-${cross_prefix}gcc}"
+ar="${AR-${cross_prefix}ar}"
+objcopy="${OBJCOPY-${cross_prefix}objcopy}"
+ld="${LD-${cross_prefix}ld}"
+strip="${STRIP-${cross_prefix}strip}"
+windres="${WINDRES-${cross_prefix}windres}"
+pkg_config="${PKG_CONFIG-${cross_prefix}pkg-config}"
+sdl_config="${SDL_CONFIG-${cross_prefix}sdl-config}"

  # default flags for all hosts
  QEMU_CFLAGS="-fno-strict-aliasing $QEMU_CFLAGS"

Re: [Qemu-devel] [PATCH] Reorganize and fix monitor resume after migration

[Anthony Liguori]

On 08/05/2011 02:11 AM, Jan Kiszka wrote:

From: Jan Kiszka

If migration failed in migrate_fd_put_buffer, the monitor may have been
resumed not only in the error path of that function but also once again
in migrate_fd_put_ready which is called unconditionally by
migrate_fd_connect.

Fix this by establishing a cleaner policy: the monitor shall be resumed
when the migration file is closed, either via callback
(migrate_fd_close) or in migrate_fd_cleanup if no file is open (i.e. no
callback invoked).

Signed-off-by: Jan Kiszka


Applied.  Thanks.

Regards,

Anthony Liguori


---
  migration.c |   19 +--
  1 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/migration.c b/migration.c
index 2a15b98..756fa62 100644
--- a/migration.c
+++ b/migration.c
@@ -292,18 +292,17 @@ int migrate_fd_cleanup(FdMigrationState *s)
  ret = -1;
  }
  s->file = NULL;
+} else {
+if (s->mon) {
+monitor_resume(s->mon);
+}
  }

-if (s->fd != -1)
+if (s->fd != -1) {
  close(s->fd);
-
-/* Don't resume monitor until we've flushed all of the buffers */
-if (s->mon) {
-monitor_resume(s->mon);
+s->fd = -1;
  }

-s->fd = -1;
-
  return ret;
  }

@@ -330,9 +329,6 @@ ssize_t migrate_fd_put_buffer(void *opaque, const void 
*data, size_t size)
  if (ret == -EAGAIN) {
  qemu_set_fd_handler2(s->fd, NULL, NULL, migrate_fd_put_notify, s);
  } else if (ret<  0) {
-if (s->mon) {
-monitor_resume(s->mon);
-}
  s->state = MIG_STATE_ERROR;
  notifier_list_notify(&migration_state_notifiers, NULL);
  }
@@ -458,6 +454,9 @@ int migrate_fd_close(void *opaque)
  {
  FdMigrationState *s = opaque;

+if (s->mon) {
+monitor_resume(s->mon);
+}
  qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
  return s->close(s);
  }

Re: [Qemu-devel] [PATCH] e1000: Do reset when E1000_CTRL_RST bit is set.

[Anthony Liguori]

On 08/05/2011 09:36 AM, Anthony PERARD wrote:

Signed-off-by: Anthony PERARD
---
  hw/e1000.c |   10 --
  1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/e1000.c b/hw/e1000.c
index 96d84f9..a1388e9 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -150,6 +150,8 @@ static const char phy_regcap[0x20] = {
  [PHY_ID2] = PHY_R,[M88E1000_PHY_SPEC_STATUS] = PHY_R
  };

+static void e1000_reset(void *opaque);
+
  static void
  ioport_map(PCIDevice *pci_dev, int region_num, pcibus_t addr,
 pcibus_t size, int type)
@@ -202,8 +204,12 @@ rxbufsize(uint32_t v)
  static void
  set_ctrl(E1000State *s, int index, uint32_t val)
  {
-/* RST is self clearing */
-s->mac_reg[CTRL] = val&  ~E1000_CTRL_RST;
+DBGOUT(IO, "set ctrl = %08x\n", val);
+if (val&  E1000_CTRL_RST) {


You'll break some GCCs with -Wall -Werror with this.  Please do:

if ((val & E1000_CTRL_RST)) {

Regards,

Anthony Liguori


+e1000_reset(s);
+return;
+}
+s->mac_reg[CTRL] = val;
  }

  static void

Re: [Qemu-devel] [PATCH] monitor: HMP: fix consecutive integer expression parsing

[Anthony Liguori]

On 08/03/2011 06:57 AM, Alon Levy wrote:

Currently a command that takes two consecutive integer operations, like
client_migrate_info, will be incorrectly parsed by the human monitor if
the second expression begins with a minus ('-') or plus ('+') sign:

client_migrate_info  
client_migrate_info spice localhost 5900 -1
=>  port = 5899 = 5900 - 1
tls-port = -1
But expected by the user to be:
port = 5900
tls-port = -1

The fix is that for any required integer (ilM) expression followed by another
integer expression (ilM) the first expression will be parsed by expr_unary
instead of expr_sum. So you can still use arithmetic, but you have to enclose
it in parenthesis:

Command line | Old parsed result | With patch result
(1+1) 2  | 2, 2  | 2, 2
1 -1 | 0, -1 | 1, -1
The rest are bizarre but not any worse then before
1+2+3| 6, 5  | 1, 5
(1+2)+3  | 3, 3  | 3, 3


I vote for just removing the expression parsing entirely.  It's 
incredibly non-intuitive and I don't think anyone really uses it.


Does anyone strongly object?

Regards,

Anthony Liguori

Re: [Qemu-devel] Build broken

[malc]

On Fri, 5 Aug 2011, Stefan Hajnoczi wrote:

> On Fri, Aug 5, 2011 at 7:22 AM, malc  wrote:
> >
> > /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c: In function 
> > 'coroutine_new':
> > /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c:160:16: error: 'arg.i[1]' 
> > may be used uninitialized in this function
> > /home/malc/x/rcs/git/qemuorg/coroutine-ucontext.c:136:18: note: 'arg.i[1]' 
> > was declared here
> >
> > diff --git a/coroutine-ucontext.c b/coroutine-ucontext.c
> > index 41c2379..42dc3e2 100644
> > --- a/coroutine-ucontext.c
> > +++ b/coroutine-ucontext.c
> > @@ -133,7 +133,7 @@ static Coroutine *coroutine_new(void)
> >     CoroutineUContext *co;
> >     ucontext_t old_uc, uc;
> >     jmp_buf old_env;
> > -    union cc_arg arg;
> > +    union cc_arg arg = {0};
> >
> >     /* The ucontext functions preserve signal masks which incurs a system 
> > call
> >      * overhead.  setjmp()/longjmp() does not preserve signal masks but only
> >
> > I guess gcc should yell not only here on ppc32 but on any machine where
> > pointer size is less than the size of two ints.
> 
> Makes sense.  Are you going to commit a fix or send a signed-off-by patch?
> 

If the author(s)(you and Kevin? just you?) agree with the above i can just 
push it.

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

Re: [Qemu-devel] [PATCH] MAINTAINERS: add entry for Xen

[Anthony Liguori]

On 07/29/2011 10:05 AM, stefano.stabell...@eu.citrix.com wrote:

From: Stefano Stabellini

Signed-off-by: Stefano Stabellini


Applied.  Thanks.

Regards,

Anthony Liguori


---
  MAINTAINERS |   10 ++
  1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 6115e4e..273a6a7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -143,6 +143,16 @@ L: k...@vger.kernel.org
  S: Supported
  F: target-i386/kvm.c

+Guest CPU Cores (Xen):
+--
+
+X86
+M: Stefano Stabellini
+L: xen-de...@lists.xensource.com
+S: Supported
+F: xen-*
+F: */xen*
+
  ARM Machines
  
  Gumstix

Re: [Qemu-devel] [PATCH v3 14/39] ac97: convert to memory API

[malc]

On Fri, 5 Aug 2011, Anthony Liguori wrote:

> On 08/04/2011 08:06 AM, Avi Kivity wrote:
> > fixes BAR sizing as well.
> > 
> > Reviewed-by: Richard Henderson
> > Signed-off-by: Avi Kivity
> 
> Reviewed-by: Anthony Liguori 
> 
> Malc, please Ack
>

Ok, please notify me when this is pushed so i can correct the
formatting consistency issues.

[..snip..]

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

Re: [Qemu-devel] [PATCH] memory: synchronize dirty bitmap before unmapping a range

[Anthony Liguori]

On 07/31/2011 02:47 PM, Avi Kivity wrote:

When a range is being unmapped, ask accelerators (e.g. kvm) to synchronize the
dirty bitmap to avoid losing information forever.

Fixes grub2 screen update.

Signed-off-by: Avi Kivity


Applied.  Thanks.

Regards,

Anthony Liguori


---

Please apply before the PCI batch to avoid bisectability issues (and don't
pull, since that removes ordering).

  memory.c |4 
  1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/memory.c b/memory.c
index 5c6e63d..5f20320 100644
--- a/memory.c
+++ b/memory.c
@@ -245,6 +245,10 @@ static void as_memory_range_add(AddressSpace *as, 
FlatRange *fr)

  static void as_memory_range_del(AddressSpace *as, FlatRange *fr)
  {
+if (fr->dirty_log_mask) {
+cpu_physical_sync_dirty_bitmap(fr->addr.start,
+   fr->addr.start + fr->addr.size);
+}
  cpu_register_physical_memory(fr->addr.start, fr->addr.size,
   IO_MEM_UNASSIGNED);
  }

Re: [Qemu-devel] [PATCH 00/15] sdl: Usability improvements

[Anthony Liguori]

On 07/30/2011 04:39 AM, Jan Kiszka wrote:

As SDL is my preferred way of working ad-hoc with guests, I had a closer
look at oddities and shortcomings that this GUI exposed, at least here
on Linux hosts. The result is a series of patches I've now finally
polished and completed. Highlights:
  - fix termination in -no-shutdown mode
  - fix various issues when switching to/from full screen mode
  - polish mouse grabbing in full screen mode, under text console and
when in absolute mouse mode
  - dynamically grab keyboard input in absolute mouse mode, enabling
e.g. ALT+TAB in the guest
  - add zoom hot keys to make window scaling more attractive
  - refactor some ugly functions

Please review/merge.

CC: Stefano Stabellini


Applied all.  Thanks.

Regards,

Anthony Liguori



Jan Kiszka (15):
   sdl: Fix termination in -no-shutdown mode
   sdl: Do not make full screen mode resizable
   sdl: Avoid redundant scaling deactivation
   sdl: Properly mark modifier+u as hotkey
   sdl: Fix full screen toggling from scaled mode
   sdl: Restore scaling mode on return from full screen
   sdl: Drop bogus gui_fullscreen_initial_grab
   sdl: Initialize gui_fullscreen earlier during setup
   sdl: Consistently avoid grabbing input for text consoles
   sdl: Never release input while in full screen mode
   sdl: Fix cursor handling when switching consoles in absolute mouse
 mode
   sdl: Dynamically grab input in absolute mouse mode
   sdl: Add zoom hot keys
   sdl: Factor out event handlers from sdl_refresh
   sdl: Refactor sdl_send_mouse_event

  qemu-doc.texi |8 +
  ui/sdl.c  |  547 -
  2 files changed, 355 insertions(+), 200 deletions(-)

Re: [Qemu-devel] [PATCH] Avoid allocating TCG resources in non-TCG mode

[Anthony Liguori]

On 08/02/2011 09:10 AM, Jan Kiszka wrote:

Do not allocate TCG-only resources like the translation buffer when
running over KVM or XEN. Saves a "few" bytes in the qemu address space
and is also conceptually cleaner.

Signed-off-by: Jan Kiszka


Applied.  Thanks.

Regards,

Anthony Liguori


---

Note: Only tested on x86.

  bsd-user/main.c   |3 ++-
  darwin-user/main.c|4 ++--
  exec.c|   19 ++-
  linux-user/main.c |3 ++-
  qemu-common.h |5 -
  target-i386/helper.c  |4 ++--
  target-ppc/helper.c   |4 +++-
  target-s390x/helper.c |2 +-
  vl.c  |   14 +++---
  9 files changed, 37 insertions(+), 21 deletions(-)

diff --git a/bsd-user/main.c b/bsd-user/main.c
index a63b877..cc7d4a3 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -905,7 +905,8 @@ int main(int argc, char **argv)
  cpu_model = "any";
  #endif
  }
-cpu_exec_init_all(0);
+tcg_exec_init(0);
+cpu_exec_init_all();
  /* NOTE: we need to init the CPU at this stage to get
 qemu_host_page_size */
  env = cpu_init(cpu_model);
diff --git a/darwin-user/main.c b/darwin-user/main.c
index 72307ad..1a881a0 100644
--- a/darwin-user/main.c
+++ b/darwin-user/main.c
@@ -852,8 +852,8 @@ int main(int argc, char **argv)
  #error unsupported CPU
  #endif
  }
-
-cpu_exec_init_all(0);
+tcg_exec_init(0);
+cpu_exec_init_all();
  /* NOTE: we need to init the CPU at this stage to get
 qemu_host_page_size */
  env = cpu_init(cpu_model);
diff --git a/exec.c b/exec.c
index 476b507..2658cb2 100644
--- a/exec.c
+++ b/exec.c
@@ -570,16 +570,12 @@ static void code_gen_alloc(unsigned long tb_size)
  /* Must be called before using the QEMU cpus. 'tb_size' is the size
 (in bytes) allocated to the translation buffer. Zero means default
 size. */
-void cpu_exec_init_all(unsigned long tb_size)
+void tcg_exec_init(unsigned long tb_size)
  {
  cpu_gen_init();
  code_gen_alloc(tb_size);
  code_gen_ptr = code_gen_buffer;
  page_init();
-#if !defined(CONFIG_USER_ONLY)
-memory_map_init();
-io_mem_init();
-#endif
  #if !defined(CONFIG_USER_ONLY) || !defined(CONFIG_USE_GUEST_BASE)
  /* There's no guest base to take into account, so go ahead and
 initialize the prologue now.  */
@@ -587,6 +583,19 @@ void cpu_exec_init_all(unsigned long tb_size)
  #endif
  }

+bool tcg_enabled(void)
+{
+return code_gen_buffer != NULL;
+}
+
+void cpu_exec_init_all(void)
+{
+#if !defined(CONFIG_USER_ONLY)
+memory_map_init();
+io_mem_init();
+#endif
+}
+
  #if defined(CPU_SAVE_VERSION)&&  !defined(CONFIG_USER_ONLY)

  static int cpu_common_post_load(void *opaque, int version_id)
diff --git a/linux-user/main.c b/linux-user/main.c
index 6a8f4bd..8e15474 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -3117,7 +3117,8 @@ int main(int argc, char **argv, char **envp)
  cpu_model = "any";
  #endif
  }
-cpu_exec_init_all(0);
+tcg_exec_init(0);
+cpu_exec_init_all();
  /* NOTE: we need to init the CPU at this stage to get
 qemu_host_page_size */
  env = cpu_init(cpu_model);
diff --git a/qemu-common.h b/qemu-common.h
index 1e3c665..3440986 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -273,7 +273,10 @@ typedef struct VirtIODevice VirtIODevice;

  typedef uint64_t pcibus_t;

-void cpu_exec_init_all(unsigned long tb_size);
+void tcg_exec_init(unsigned long tb_size);
+bool tcg_enabled(void);
+
+void cpu_exec_init_all(void);

  /* CPU save/load.  */
  void cpu_save(QEMUFile *f, void *opaque);
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 182009a..3332195 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1243,8 +1243,8 @@ CPUX86State *cpu_x86_init(const char *cpu_model)
  cpu_exec_init(env);
  env->cpu_model_str = cpu_model;

-/* init various static tables */
-if (!inited) {
+/* init various static tables used in TCG mode */
+if (tcg_enabled()&&  !inited) {
  inited = 1;
  optimize_flags_init();
  #ifndef CONFIG_USER_ONLY
diff --git a/target-ppc/helper.c b/target-ppc/helper.c
index 176128a..e00b3e6 100644
--- a/target-ppc/helper.c
+++ b/target-ppc/helper.c
@@ -3091,7 +3091,9 @@ CPUPPCState *cpu_ppc_init (const char *cpu_model)

  env = qemu_mallocz(sizeof(CPUPPCState));
  cpu_exec_init(env);
-ppc_translate_init();
+if (tcg_enabled()) {
+ppc_translate_init();
+}
  env->cpu_model_str = cpu_model;
  cpu_ppc_register_internal(env, def);

diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index 1ce7079..443bb1d 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -81,7 +81,7 @@ CPUS390XState *cpu_s390x_init(const char *cpu_model)

  env = qemu_mallocz(sizeof(CPUS390XState));
  cpu_exec_init(env);
-if (!inited) {
+if (tcg_enabled()&&  !inited) {
  inited = 1;
  s390x_translate_init();
  

Re: [Qemu-devel] [PATCH] qdev: Eliminate duplicate reset

[Anthony Liguori]

On 08/01/2011 08:59 PM, Isaku Yamahata wrote:

qbus_reset_all_fn was registered twice, so a lot of device reset
functions were also called twice when QEMU started.
Which was introduced by 80376c3fc2c38fdd45354e4b0eb45031f35587ed
This patch fixes it by making the main_system_bus creation not register
reset handler.

Cc: Stefan Weil
Signed-off-by: Isaku Yamahata
Tested-by: Stefan Weil


Applied.  Thanks.

Regards,

Anthony Liguori


---
  hw/qdev.c |   14 --
  1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/hw/qdev.c b/hw/qdev.c
index b4ea8e1..6819537 100644
--- a/hw/qdev.c
+++ b/hw/qdev.c
@@ -36,6 +36,7 @@ static bool qdev_hot_removed = false;

  /* This is a nasty hack to allow passing a NULL bus to qdev_create.  */
  static BusState *main_system_bus;
+static void main_system_bus_create(void);

  DeviceInfo *device_info_list;

@@ -328,8 +329,7 @@ static int qdev_reset_one(DeviceState *dev, void *opaque)
  BusState *sysbus_get_default(void)
  {
  if (!main_system_bus) {
-main_system_bus = qbus_create(&system_bus_info, NULL,
-  "main-system-bus");
+main_system_bus_create();
  }
  return main_system_bus;
  }
@@ -784,6 +784,16 @@ BusState *qbus_create(BusInfo *info, DeviceState *parent, 
const char *name)
  return bus;
  }

+static void main_system_bus_create(void)
+{
+/* assign main_system_bus before qbus_create_inplace()
+ * in order to make "if (bus != main_system_bus)" work */
+main_system_bus = qemu_mallocz(system_bus_info.size);
+main_system_bus->qdev_allocated = 1;
+qbus_create_inplace(main_system_bus,&system_bus_info, NULL,
+"main-system-bus");
+}
+
  void qbus_free(BusState *bus)
  {
  DeviceState *dev;

Re: [Qemu-devel] [PATCH] Check fread() results to avoid gcc 4.6 warnings

[Anthony Liguori]

On 08/01/2011 01:49 AM, David Gibson wrote:

When compiling with gcc 4.6, some code in fw_cfg.c complains that fop_ret
is assigned but not used (which is true).  However, it looks like the
meaningless assignments to fop_ret were done to suppress other gcc warnings
due to the fact that fread() is labelled as warn_unused_result in glibc.

This patch avoids both errors, by actually checking the fread() result code
and dropping out with an error message if it fails.

Signed-off-by: David Gibson


Applied.  Thanks.

Regards,

Anthony Liguori


---
  hw/fw_cfg.c |   13 +
  1 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/hw/fw_cfg.c b/hw/fw_cfg.c
index a29db90..e4847b7 100644
--- a/hw/fw_cfg.c
+++ b/hw/fw_cfg.c
@@ -87,6 +87,13 @@ static FILE *probe_splashfile(char *filename, int 
*file_sizep, int *file_typep)
  /* check magic ID */
  fseek(fp, 0L, SEEK_SET);
  fop_ret = fread(buf, 1, 2, fp);
+if (fop_ret != 2) {
+error_report("Could not read header from '%s': %s",
+ filename, strerror(errno));
+fclose(fp);
+fp = NULL;
+return fp;
+}
  filehead_value = (buf[0] + (buf[1]<<  8))&  0x;
  if (filehead_value == 0xd8ff) {
  file_type = JPG_FILE;
@@ -181,6 +188,12 @@ static void fw_cfg_bootsplash(FWCfgState *s)
  boot_splash_filedata_size = file_size;
  fseek(fp, 0L, SEEK_SET);
  fop_ret = fread(boot_splash_filedata, 1, file_size, fp);
+if (fop_ret != file_size) {
+error_report("failed to read data from '%s'.",
+ boot_splash_filename);
+fclose(fp);
+return;
+}
  fclose(fp);
  /* insert data */
  if (file_type == JPG_FILE) {

Re: [Qemu-devel] [PATCH v2] memory: use signed arithmetic

[Anthony Liguori]

On 08/03/2011 03:56 AM, Avi Kivity wrote:

When trying to map an alias of a ram region, where the alias starts at
address A and we map it into address B, and A>  B, we had an arithmetic
underflow.  Because we use unsigned arithmetic, the underflow converted
into a large number which failed addrrange_intersects() tests.

The concrete example which triggered this was cirrus vga mapping
the framebuffer at offsets 0xc-0xc7fff (relative to the start of
the framebuffer) into offsets 0xa (relative to system addres space
start).

With our favorite analogy of a windowing system, this is equivalent to
dragging a subwindow off the left edge of the screen, and failing to clip
it into its parent window which is on screen.

Fix by switching to signed arithmetic.

Signed-off-by: Avi Kivity


Applied.  Thanks.

Regards,

Anthony Liguori


---

v2: add comment about physical address width limitation to 63 bits

  exec.c   |2 +-
  memory.c |   23 ++-
  2 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/exec.c b/exec.c
index 476b507..751fd89 100644
--- a/exec.c
+++ b/exec.c
@@ -3818,7 +3818,7 @@ static void io_mem_init(void)
  static void memory_map_init(void)
  {
  system_memory = qemu_malloc(sizeof(*system_memory));
-memory_region_init(system_memory, "system", UINT64_MAX);
+memory_region_init(system_memory, "system", INT64_MAX);
  set_system_memory_map(system_memory);
  }

diff --git a/memory.c b/memory.c
index 5f20320..be891c6 100644
--- a/memory.c
+++ b/memory.c
@@ -22,12 +22,17 @@ unsigned memory_region_transaction_depth = 0;

  typedef struct AddrRange AddrRange;

+/*
+ * Note using signed integers limits us to physical addresses at most
+ * 63 bits wide.  They are needed for negative offsetting in aliases
+ * (large MemoryRegion::alias_offset).
+ */
  struct AddrRange {
-uint64_t start;
-uint64_t size;
+int64_t start;
+int64_t size;
  };

-static AddrRange addrrange_make(uint64_t start, uint64_t size)
+static AddrRange addrrange_make(int64_t start, int64_t size)
  {
  return (AddrRange) { start, size };
  }
@@ -37,7 +42,7 @@ static bool addrrange_equal(AddrRange r1, AddrRange r2)
  return r1.start == r2.start&&  r1.size == r2.size;
  }

-static uint64_t addrrange_end(AddrRange r)
+static int64_t addrrange_end(AddrRange r)
  {
  return r.start + r.size;
  }
@@ -56,9 +61,9 @@ static bool addrrange_intersects(AddrRange r1, AddrRange r2)

  static AddrRange addrrange_intersection(AddrRange r1, AddrRange r2)
  {
-uint64_t start = MAX(r1.start, r2.start);
+int64_t start = MAX(r1.start, r2.start);
  /* off-by-one arithmetic to prevent overflow */
-uint64_t end = MIN(addrrange_end(r1) - 1, addrrange_end(r2) - 1);
+int64_t end = MIN(addrrange_end(r1) - 1, addrrange_end(r2) - 1);
  return addrrange_make(start, end - start + 1);
  }

@@ -411,8 +416,8 @@ static void render_memory_region(FlatView *view,
  MemoryRegion *subregion;
  unsigned i;
  target_phys_addr_t offset_in_region;
-uint64_t remain;
-uint64_t now;
+int64_t remain;
+int64_t now;
  FlatRange fr;
  AddrRange tmp;

@@ -486,7 +491,7 @@ static FlatView generate_memory_topology(MemoryRegion *mr)

  flatview_init(&view);

-render_memory_region(&view, mr, 0, addrrange_make(0, UINT64_MAX));
+render_memory_region(&view, mr, 0, addrrange_make(0, INT64_MAX));
  flatview_simplify(&view);

  return view;

[Qemu-devel] another TCG branch weirdness

[Artyom Tarasenko]

Host x86_64, guest sparc64. Found a case where a branch instruction
(brz,pn   %o0) unexpectedly jumps to an unexpected address. I.e.
branch shouldn't be taken at all, but even if it were it should have
been to 0x13e26e4 and not to 0x5.

Was about to write that the generated OP for brz,pn usually looks
different, when realized that in fact it was even generated for this
very address just before, but with another branch in the delay slot.
The bug looks familiar, Blue, isn't it? :)

IN:
0x013e26c0:  brz,pn   %o0, 0x13e26e4
0x013e26c4:  brlez,pn   %o1, 0x13e26e4

OP:
  0x13e26c0
 ld_i64 tmp6,regwptr,$0x0
 movi_i64 cond,$0x0
 movi_i64 tmp8,$0x0
 brcond_i64 tmp6,tmp8,ne,$0x0
 movi_i64 cond,$0x1
 set_label $0x0

^^^ Ok, that's how brz,pn  usually looks like

  0x13e26c4
 ld_i64 tmp7,regwptr,$0x8
 movi_i64 tmp8,$0x0
 brcond_i64 cond,tmp8,eq,$0x1
 movi_i64 npc,$0x13e26e4
 br $0x2
 set_label $0x1
 movi_i64 npc,$0x13e26c8
 set_label $0x2
 movi_i64 cond,$0x0
 movi_i64 tmp8,$0x0
 brcond_i64 tmp7,tmp8,gt,$0x3
 movi_i64 cond,$0x1
 set_label $0x3
 movi_i64 tmp0,$0x0
 brcond_i64 cond,tmp0,eq,$0x4
 movi_i64 npc,$0x13e26e4
 br $0x5
 set_label $0x4
 movi_i64 npc,$0x5
 set_label $0x5
 exit_tb $0x0
--
IN:
0x013e26c0:  brz,pn   %o0, 0x13e26e4

OP:
  0x13e26c0
 ld_i64 tmp6,regwptr,$0x0
 movi_i64 cond,$0x0
 movi_i64 tmp8,$0x0
 brcond_i64 tmp6,tmp8,ne,$0x0
 movi_i64 cond,$0x1
 set_label $0x0
 movi_i64 pc,$0x5

^^^ What's that?

 movi_i64 tmp0,$0x0
 brcond_i64 cond,tmp0,eq,$0x1
 movi_i64 npc,$0x13e26e4
 br $0x2
 set_label $0x1
 movi_i64 npc,$0x9
 set_label $0x2
 exit_tb $0x0


 33062: Instruction Access MMU Miss (v=0064) pc=0005
npc=0009 SP=0c3d2d81
...
Current Register Window:
%o0-3: 02483d00 0018 0028 000232bd
^^ not zero


-- 
Regards,
Artyom Tarasenko

solaris/sparc under qemu blog: http://tyom.blogspot.com/

[Qemu-devel] [Bug 818673] Re: virtio: trying to map MMIO memory

[Rick Vernam]

Vadim,

Have you been able to reproduce this?
Do you require any additional information?

Thanks,
-Rick

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/818673

Title:
  virtio: trying to map MMIO memory

Status in QEMU:
  New

Bug description:
  Qemu host is Core i7, running Linux.  Guest is Windows XP sp3.
  Often, qemu will crash shortly after starting (1-5 minutes) with a statement 
"qemu-system-x86_64: virtio: trying to map MMIO memory"
  This has occured with qemu-kvm 0.14, qemu-kvm 0.14.1, qemu-0.15.0-rc0 and 
qemu 0.15.0-rc1.
  Qemu is started as such:
  qemu-system-x86_64 -cpu host -enable-kvm -pidfile /home/rick/qemu/hds/wxp.pid 
-drive file=/home/rick/qemu/hds/wxp.raw,if=virtio -m 768 -name WinXP -net 
nic,model=virtio -net user -localtime -usb -vga qxl -device virtio-serial 
-chardev spicevmc,name=vdagent,id=vdagent -device 
virtserialport,chardev=vdagent,name=com.redhat.spice.0 -spice 
port=1234,disable-ticketing -daemonize -monitor 
telnet:localhost:12341,server,nowait
  The WXP guest has virtio 1.1.16 drivers for net and scsi, and the most 
current spice binaries from spice-space.org.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/818673/+subscriptions

Re: [Qemu-devel] [PATCH v2 00/31] target-xtensa: new target architecture

[Richard Henderson]

On 07/24/2011 10:10 AM, Max Filippov wrote:
> This series adds support for Tensilica Xtensa target.
> Port status: Linux for DC232B works in the qemu.
>   Not implemented xtensa options: MAC16, floating point coprocessor,
>   boolean option, cache option, debug option.
> 
> v1 -> v2 changes:
> - extract PS register and access control into separate patch;
> - implement all memory protection options;
> - GDB support.
> 
> Max Filippov (31):
>   target-xtensa: add target stubs
>   target-xtensa: add target to the configure script
>   target-xtensa: implement disas_xtensa_insn
>   target-xtensa: implement narrow instructions
>   target-xtensa: implement RT0 group
>   target-xtensa: add sample board
>   target-xtensa: implement conditional jumps
>   target-xtensa: implement JX/RET0/CALLX
>   target-xtensa: add special and user registers
>   target-xtensa: implement RST3 group
>   target-xtensa: implement shifts (ST1 and RST1 groups)
>   target-xtensa: implement LSAI group
>   target-xtensa: mark reserved and TBD opcodes
>   target-xtensa: implement SYNC group
>   target-xtensa: implement CACHE group
>   target-xtensa: add PS register and access control
>   target-xtensa: implement exceptions
>   target-xtensa: implement RST2 group (32 bit mul/div/rem)
>   target-xtensa: implement windowed registers
>   target-xtensa: implement loop option
>   target-xtensa: implement extended L32R
>   target-xtensa: implement unaligned exception option
>   target-xtensa: implement SIMCALL
>   target-xtensa: implement interrupt option
>   target-xtensa: implement accurate window check
>   target-xtensa: implement CPENABLE and PRID SRs
>   target-xtensa: implement relocatable vectors
>   target-xtensa: add gdb support
>   target-xtensa: implement memory protection options
>   target-xtensa: add dc232b core and board
>   MAINTAINERS: add xtensa maintainer

All parts:
Reviewed-by: Richard Henderson 

I guess everything except the last couple of patches I'd
already commented on in the last go-round.

There's of course the conflict with the new Memory API
that'll need to be resolved, but those are trivial changes.



r~

Re: [Qemu-devel] [PATCH 1/2] ptimer: move declarations to ptimer.h

[Anthony Liguori]

On 08/02/2011 06:47 AM, Paolo Bonzini wrote:

Since the next patch will move VMState declarations for ptimers out
of hw/hw.h, prepare a place for them.

Signed-off-by: Paolo Bonzini
---
  hw/arm_timer.c|1 +
  hw/etraxfs_timer.c|1 +
  hw/grlib_apbuart.c|1 +
  hw/grlib_gptimer.c|1 +
  hw/lan9118.c  |1 +
  hw/leon3.c|1 +
  hw/lm32_timer.c   |1 +
  hw/mcf5206.c  |1 +
  hw/mcf5208.c  |1 +
  hw/milkymist-sysctl.c |1 +
  hw/musicpal.c |1 +
  hw/ptimer.c   |1 +
  hw/ptimer.h   |   27 +++
  hw/sh_timer.c |1 +
  hw/slavio_timer.c |1 +


With this series applied, I get:

In file included from /home/anthony/git/qemu/hw/slavio_timer.c:27:0:
/home/anthony/git/qemu/hw/ptimer.h:27:33: error: expected ‘=’, ‘,’, ‘;’, 
‘asm’ or ‘__attribute__’ before ‘vmstate_ptimer’
/home/anthony/git/qemu/hw/slavio_timer.c:338:9: error: ‘vmstate_ptimer’ 
undeclared here (not in a function)

make[1]: *** [slavio_timer.o] Error 1
make: *** [subdir-sparc-softmmu] Error 2

Regards,

Anthony Liguori


  hw/syborg_timer.c |1 +
  hw/xilinx_axidma.c|1 +
  hw/xilinx_timer.c |1 +
  qemu-timer.h  |   13 -
  19 files changed, 44 insertions(+), 13 deletions(-)
  create mode 100644 hw/ptimer.h

diff --git a/hw/arm_timer.c b/hw/arm_timer.c
index fd9448f..ade3125 100644
--- a/hw/arm_timer.c
+++ b/hw/arm_timer.c
@@ -9,6 +9,7 @@

  #include "sysbus.h"
  #include "qemu-timer.h"
+#include "ptimer.h"

  /* Common timer implementation.  */

diff --git a/hw/etraxfs_timer.c b/hw/etraxfs_timer.c
index b08e574..0a28c4c 100644
--- a/hw/etraxfs_timer.c
+++ b/hw/etraxfs_timer.c
@@ -24,6 +24,7 @@
  #include "sysbus.h"
  #include "sysemu.h"
  #include "qemu-timer.h"
+#include "ptimer.h"

  #define D(x)

diff --git a/hw/grlib_apbuart.c b/hw/grlib_apbuart.c
index 169a56e..860c874 100644
--- a/hw/grlib_apbuart.c
+++ b/hw/grlib_apbuart.c
@@ -24,6 +24,7 @@

  #include "sysbus.h"
  #include "qemu-char.h"
+#include "ptimer.h"

  #include "trace.h"

diff --git a/hw/grlib_gptimer.c b/hw/grlib_gptimer.c
index 99e9033..ccb7e06 100644
--- a/hw/grlib_gptimer.c
+++ b/hw/grlib_gptimer.c
@@ -24,6 +24,7 @@

  #include "sysbus.h"
  #include "qemu-timer.h"
+#include "ptimer.h"

  #include "trace.h"

diff --git a/hw/lan9118.c b/hw/lan9118.c
index 73a8661..a562206 100644
--- a/hw/lan9118.c
+++ b/hw/lan9118.c
@@ -11,6 +11,7 @@
  #include "net.h"
  #include "devices.h"
  #include "sysemu.h"
+#include "ptimer.h"
  /* For crc32 */
  #include

diff --git a/hw/leon3.c b/hw/leon3.c
index 919f49f..59f9349 100644
--- a/hw/leon3.c
+++ b/hw/leon3.c
@@ -23,6 +23,7 @@
   */
  #include "hw.h"
  #include "qemu-timer.h"
+#include "ptimer.h"
  #include "qemu-char.h"
  #include "sysemu.h"
  #include "boards.h"
diff --git a/hw/lm32_timer.c b/hw/lm32_timer.c
index 49cbb22..e355d80 100644
--- a/hw/lm32_timer.c
+++ b/hw/lm32_timer.c
@@ -25,6 +25,7 @@
  #include "sysbus.h"
  #include "trace.h"
  #include "qemu-timer.h"
+#include "ptimer.h"
  #include "qemu-error.h"

  #define DEFAULT_FREQUENCY (50*100)
diff --git a/hw/mcf5206.c b/hw/mcf5206.c
index fce282d..3bf4b5f 100644
--- a/hw/mcf5206.c
+++ b/hw/mcf5206.c
@@ -8,6 +8,7 @@
  #include "hw.h"
  #include "mcf.h"
  #include "qemu-timer.h"
+#include "ptimer.h"
  #include "sysemu.h"

  /* General purpose timer module.  */
diff --git a/hw/mcf5208.c b/hw/mcf5208.c
index 78fbc5f..f92f489 100644
--- a/hw/mcf5208.c
+++ b/hw/mcf5208.c
@@ -8,6 +8,7 @@
  #include "hw.h"
  #include "mcf.h"
  #include "qemu-timer.h"
+#include "ptimer.h"
  #include "sysemu.h"
  #include "net.h"
  #include "boards.h"
diff --git a/hw/milkymist-sysctl.c b/hw/milkymist-sysctl.c
index 7b2d544..5f8d4ad 100644
--- a/hw/milkymist-sysctl.c
+++ b/hw/milkymist-sysctl.c
@@ -26,6 +26,7 @@
  #include "sysemu.h"
  #include "trace.h"
  #include "qemu-timer.h"
+#include "ptimer.h"
  #include "qemu-error.h"

  enum {
diff --git a/hw/musicpal.c b/hw/musicpal.c
index 63dd391..1bac24b 100644
--- a/hw/musicpal.c
+++ b/hw/musicpal.c
@@ -14,6 +14,7 @@
  #include "boards.h"
  #include "pc.h"
  #include "qemu-timer.h"
+#include "ptimer.h"
  #include "block.h"
  #include "flash.h"
  #include "console.h"
diff --git a/hw/ptimer.c b/hw/ptimer.c
index 6f13ce9..d416631 100644
--- a/hw/ptimer.c
+++ b/hw/ptimer.c
@@ -7,6 +7,7 @@
   */
  #include "hw.h"
  #include "qemu-timer.h"
+#include "ptimer.h"
  #include "host-utils.h"

  struct ptimer_state
diff --git a/hw/ptimer.h b/hw/ptimer.h
new file mode 100644
index 000..69cdddc
--- /dev/null
+++ b/hw/ptimer.h
@@ -0,0 +1,27 @@
+/*
+ * General purpose implementation of a simple periodic countdown timer.
+ *
+ * Copyright (c) 2007 CodeSourcery.
+ *
+ * This code is licensed under the GNU LGPL.
+ */
+#ifndef PTIMER_H
+#define PTIMER_H
+
+#include "qemu-common.h"
+#include "qemu-timer.h"
+
+/* ptimer.c */
+typedef struct ptimer_state ptimer_state;
+typedef void (

Re: [Qemu-devel] Safely reopening image files by stashing fds

[Anthony Liguori]

On 08/05/2011 10:43 AM, Kevin Wolf wrote:

Am 05.08.2011 17:24, schrieb Stefan Hajnoczi:

On Fri, Aug 5, 2011 at 3:28 PM, Christoph Hellwig  wrote:

On Fri, Aug 05, 2011 at 02:12:48PM +0100, Daniel P. Berrange wrote:

Because you cannot change O_DIRECT on an open fd :(.  This is why
we're going through this pain.


Hmm, I remember hearing that before, but looking at the current fcntl()
manpage, it claims you *can* change O_DIRECT using SET_FL. Perhaps this
is a newish feature, but it'd be nicer to use it if possible ?


It's been there since day 1 of O_DIRECT support.


Sorry, my bad.  So for Linux we could just use fcntl for
block_set_hostcache and not bother with reopening.  However, we will
need to reopen should we wish to support changing O_DSYNC.


We do wish to support that.

Anthony thinks that allowing the guest to toggle WCE is a prerequisite
for making cache=writeback the default. And this is something that I
definitely want to do for 1.0.


Indeed.

Regards,

Anthony Liguori


Kevin

Re: [Qemu-devel] [PATCH] Remove unused is_softmmu parameter from cpu_handle_mmu_fault

[Richard Henderson]

On 08/03/2011 11:00 AM, Blue Swirl wrote:
> Parameter is_softmmu (and its evil mutant twin brother is_softmuu)
> is not used in cpu_*_handle_mmu_fault() functions, remove them
> and adjust callers.
> 
> Signed-off-by: Blue Swirl 

Acked-by: Richard Henderson 


r~

> ---
>  target-alpha/cpu.h|2 +-
>  target-alpha/helper.c |4 ++--
>  target-alpha/op_helper.c  |2 +-
>  target-arm/cpu.h  |2 +-
>  target-arm/helper.c   |4 ++--
>  target-arm/op_helper.c|2 +-
>  target-cris/cpu.h |2 +-
>  target-cris/helper.c  |   11 +--
>  target-cris/op_helper.c   |2 +-
>  target-i386/cpu.h |2 +-
>  target-i386/helper.c  |4 ++--
>  target-i386/op_helper.c   |2 +-
>  target-lm32/cpu.h |2 +-
>  target-lm32/helper.c  |2 +-
>  target-lm32/op_helper.c   |2 +-
>  target-m68k/cpu.h |2 +-
>  target-m68k/helper.c  |4 ++--
>  target-m68k/op_helper.c   |2 +-
>  target-microblaze/cpu.h   |2 +-
>  target-microblaze/helper.c|4 ++--
>  target-microblaze/op_helper.c |2 +-
>  target-mips/cpu.h |2 +-
>  target-mips/helper.c  |6 +++---
>  target-mips/op_helper.c   |2 +-
>  target-ppc/cpu.h  |2 +-
>  target-ppc/helper.c   |4 ++--
>  target-ppc/op_helper.c|2 +-
>  target-s390x/cpu.h|2 +-
>  target-s390x/helper.c |   12 ++--
>  target-s390x/op_helper.c  |2 +-
>  target-sh4/cpu.h  |2 +-
>  target-sh4/helper.c   |4 ++--
>  target-sh4/op_helper.c|2 +-
>  target-sparc/cpu.h|2 +-
>  target-sparc/helper.c |6 +++---
>  target-sparc/op_helper.c  |2 +-
>  target-unicore32/cpu.h|2 +-
>  target-unicore32/helper.c |2 +-
>  user-exec.c   |2 +-
>  39 files changed, 59 insertions(+), 60 deletions(-)

Re: [Qemu-devel] Safely reopening image files by stashing fds

[Kevin Wolf]

Am 05.08.2011 17:24, schrieb Stefan Hajnoczi:
> On Fri, Aug 5, 2011 at 3:28 PM, Christoph Hellwig  wrote:
>> On Fri, Aug 05, 2011 at 02:12:48PM +0100, Daniel P. Berrange wrote:
 Because you cannot change O_DIRECT on an open fd :(.  This is why
 we're going through this pain.
>>>
>>> Hmm, I remember hearing that before, but looking at the current fcntl()
>>> manpage, it claims you *can* change O_DIRECT using SET_FL. Perhaps this
>>> is a newish feature, but it'd be nicer to use it if possible ?
>>
>> It's been there since day 1 of O_DIRECT support.
> 
> Sorry, my bad.  So for Linux we could just use fcntl for
> block_set_hostcache and not bother with reopening.  However, we will
> need to reopen should we wish to support changing O_DSYNC.

We do wish to support that.

Anthony thinks that allowing the guest to toggle WCE is a prerequisite
for making cache=writeback the default. And this is something that I
definitely want to do for 1.0.

Kevin

Re: [Qemu-devel] [PATCH 1/7] Export the unassigned_mem read/write functions.

[Richard Henderson]

On 08/04/2011 04:58 PM, Edgar E. Iglesias wrote:
> QEMU only models the data lanes so I think Richards patch is quite OK
> because it brings the decission back to the CPU model.
> 
> I would prefer though, if the new Memory API would let devices pass
> ctrl data back to the CPU in addition to the data lanes. That would
> probably involve quite a bit of work though. CC:ing Avi, you might
> have more input on this.

Amusingly, after having updated my system to Avi's tree, I find
I don't need the unassigned_mem_read[bwl] helpers anymore, but
only the cpu_unassigned_access function, which is already exported.

It would be nice if the new api allowed signaling of errors, but
I can't think of a really nice way of doing that.


r~

Re: [Qemu-devel] [PATCH v3 00/39] Memory API, batch 2: PCI devices

[Anthony Liguori]

On 08/04/2011 08:05 AM, Avi Kivity wrote:

This is a mostly mindless conversion of all QEMU PCI devices to the memory API.
After this patchset is applied, it is no longer possible to create a PCI device
using the old API.

An immediate benefit is that PCI BARs that overlap each other are now handled
correctly: currently, the sequence

   map BAR 0
   map BAR 1 at an overlapping address
   unmap either BAR 0 or BAR 1

will leave a hole where the overlap exists.  With the patchset, the memory map
is restored correctly.

Note that overlaps of PCI BARs with memory or non-PCI resources are still not
resolved correctly; this will be fixed later on.

The vga patches have ugly intermediate states; however the result is fairly 
clean.

This patchset should be merged after

   memory: synchronize dirty bitmap before unmapping a range
   memory: use signed arithmetic


I'll merge these two patches ASAP btw.

The whole series is awesome.  The mechanical patches are simple enough 
that it was taking longer to write the response than to review them so 
for anything I haven't explicitly responded to,


Reviewed-by: Anthony Liguori 

Once you update the few cosmetic issues and correct the virtio patch, 
I'll apply this series.


Regards,

Anthony Liguori



though there's no hard dependency.

Changes from v2:
  - added patch from Michael simplifying virtio-pci config setup

Changes from v1:
  - cmd646 type fix
  - folded a fixlet into its parent

Avi Kivity (38):
   pci: add API to get a BAR's mapped address
   vmsvga: don't remember pci BAR address in callback any more
   vga: convert vga and its derivatives to the memory API
   cirrus: simplify mmio BAR access functions
   cirrus: simplify bitblt BAR access functions
   cirrus: simplify vga window mmio access functions
   vga: simplify vga window mmio access functions
   cirrus: simplify linear framebuffer access functions
   Integrate I/O memory regions into qemu
   pci: pass I/O address space to new PCI bus
   pci: allow I/O BARs to be registered with pci_register_bar_region()
   rtl8139: convert to memory API
   ac97: convert to memory API
   e1000: convert to memory API
   eepro100: convert to memory API
   es1370: convert to memory API
   ide: convert to memory API
   ivshmem: convert to memory API
   virtio-pci: convert to memory API
   ahci: convert to memory API
   intel-hda: convert to memory API
   lsi53c895a: convert to memory API
   ppc: convert to memory API
   ne2000: convert to memory API
   pcnet: convert to memory API
   i6300esb: convert to memory API
   isa-mmio: concert to memory API
   sun4u: convert to memory API
   ehci: convert to memory API
   uhci: convert to memory API
   xen-platform: convert to memory API
   msix: convert to memory API
   pci: remove pci_register_bar_simple()
   pci: convert pci rom to memory API
   pci: remove pci_register_bar()
   pci: fold BAR mapping function into its caller
   pci: rename pci_register_bar_region() to pci_register_bar()
   pci: remove support for pre memory API BARs

Michael S. Tsirkin (1):
   virtio-pci: get config on init

  exec-memory.h  |2 +
  exec.c |   10 ++
  hw/ac97.c  |   88 ++-
  hw/apb_pci.c   |1 +
  hw/bonito.c|1 +
  hw/cirrus_vga.c|  458 ---
  hw/cuda.c  |6 +-
  hw/e1000.c |  113 ++
  hw/eepro100.c  |  181 -
  hw/es1370.c|   43 +++--
  hw/escc.c  |   42 +++---
  hw/escc.h  |2 +-
  hw/grackle_pci.c   |8 +-
  hw/gt64xxx.c   |4 +-
  hw/heathrow_pic.c  |   29 ++--
  hw/ide.h   |2 +-
  hw/ide/ahci.c  |   31 ++--
  hw/ide/ahci.h  |2 +-
  hw/ide/cmd646.c|  204 +++-
  hw/ide/ich.c   |3 +-
  hw/ide/macio.c |   36 +++--
  hw/ide/pci.c   |   25 ++--
  hw/ide/pci.h   |   19 ++-
  hw/ide/piix.c  |   63 ++--
  hw/ide/via.c   |   64 ++--
  hw/intel-hda.c |   35 +++--
  hw/isa.h   |2 +
  hw/isa_mmio.c  |   30 ++--
  hw/ivshmem.c   |  158 +++
  hw/lance.c |   31 ++--
  hw/lsi53c895a.c|  257 +++---
  hw/mac_dbdma.c |   32 ++--
  hw/mac_dbdma.h |4 +-
  hw/mac_nvram.c |   39 ++---
  hw/macio.c |   73 -
  hw/msix.c  |   64 +++-
  hw/msix.h  |6 +-
  hw/ne2000-isa.c|   14 +--
  hw/ne2000.c|   77 ++---
  hw/ne2000.h|8 +-
  hw/openpic.c   |   81 +-
  hw/openpic.h   |2 +-
  hw/pc.h|4 +-
  hw/pc_piix.c   |6 +-
  hw/pci.c   |  133 +---
  hw/pci.h   |   26 ++--
  hw/pci_internals.h |3 +-
  hw/pcnet-pci.c |   74 +
  hw/pcnet.h |4 +-
  hw/piix_pci.c  |   14 +-
  hw/ppc4xx_pci.c|1 +
  hw/ppc_mac.h   |   27 ++--
  hw/ppc_newworld.c  |   34 ++--
  hw/ppc_oldworld.c  |  

Re: [Qemu-devel] [PATCH] Permit -mem-path without sync mmu

[Marcelo Tosatti]

On Fri, Aug 05, 2011 at 08:16:42AM +0200, Jan Kiszka wrote:
> On 2011-08-05 06:02, David Gibson wrote:
> > At present, an explicit test disallows use of -mem-path when kvm is enabled
> > but KVM_CAP_SYNC_MMU is not set.  In particular, this prevents the user
> > from using hugetlbfs to back the guest memory.
> > 
> > I can see no reason for this check, and when I asked about it previously,
> > the only theory offered was that this was a limitation of the very early
> > days of kvm which only happened to match the SYNC_MMU flag by accident.
> > 
> > This patch, therefore, removes the check.  This is of particular use to
> > us on POWER, where we haven't yet implement SYNC_MMU, but where backing
> > the guest with hugepages is possible, and in fact mandatory (for now).
> > 
> > Signed-off-by: David Gibson 
> > ---
> >  exec.c |5 -
> >  1 files changed, 0 insertions(+), 5 deletions(-)
> > 
> > diff --git a/exec.c b/exec.c
> > index 476b507..041637c 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -2818,11 +2818,6 @@ static void *file_ram_alloc(RAMBlock *block,
> >  return NULL;
> >  }
> >  
> > -if (kvm_enabled() && !kvm_has_sync_mmu()) {
> > -fprintf(stderr, "host lacks kvm mmu notifiers, -mem-path 
> > unsupported\n");
> > -return NULL;
> > -}
> > -
> >  if (asprintf(&filename, "%s/qemu_back_mem.XX", path) == -1) {
> >  return NULL;
> >  }
> 
> This is nothing trivial, see ce9a92411d in qemu-kvm or
> http://thread.gmane.org/gmane.comp.emulators.kvm.devel/27380. And it
> should rather target uq/master. CCing Avi, Marcelo, and the kvm list.
> 
> Jan

Yes, the check cannot be removed because there is the possibility of
corruption using hugepages without mmu notifiers (described in the 
archived message above).

Why are mmu notifiers not implemented for PPC again?

Re: [Qemu-devel] [PATCH v3 29/39] sun4u: convert to memory API

[Anthony Liguori]

On 08/04/2011 08:06 AM, Avi Kivity wrote:

fixes memory leak on repeated BAR map/unmap

Reviewed-by: Richard Henderson
Signed-off-by: Avi Kivity
---
  hw/sun4u.c |   55 +--
  1 files changed, 25 insertions(+), 30 deletions(-)

diff --git a/hw/sun4u.c b/hw/sun4u.c
index d7dcaf0..74a06a8 100644
--- a/hw/sun4u.c
+++ b/hw/sun4u.c
@@ -91,6 +91,12 @@ struct hwdef {
  uint64_t console_serial_base;
  };

+typedef struct EbusState {
+PCIDevice pci_dev;
+MemoryRegion bar0;
+MemoryRegion bar1;
+} EbusState;
+
  int DMA_get_channel_mode (int nchan)
  {
  return 0;
@@ -518,21 +524,6 @@ void cpu_tick_set_limit(CPUTimer *timer, uint64_t limit)
  }
  }

-static void ebus_mmio_mapfunc(PCIDevice *pci_dev, int region_num,
-  pcibus_t addr, pcibus_t size, int type)
-{
-EBUS_DPRINTF("Mapping region %d registers at %" FMT_PCIBUS "\n",
- region_num, addr);
-switch (region_num) {
-case 0:
-isa_mmio_init(addr, 0x100);
-break;
-case 1:
-isa_mmio_init(addr, 0x80);
-break;
-}
-}
-
  static void dummy_isa_irq_handler(void *opaque, int n, int level)
  {
  }
@@ -549,27 +540,31 @@ pci_ebus_init(PCIBus *bus, int devfn)
  }

  static int
-pci_ebus_init1(PCIDevice *s)
+pci_ebus_init1(PCIDevice *pci_dev)
  {
-isa_bus_new(&s->qdev);
+EbusState *s = container_of(pci_dev, EbusState, pci_dev);


DO_UPCAST() is the qdev macro for this.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH v3 28/39] isa-mmio: concert to memory API

[Anthony Liguori]

On 08/04/2011 08:06 AM, Avi Kivity wrote:

Reviewed-by: Richard Henderson
Signed-off-by: Avi Kivity


For the subject, s:concert:convert:g

Otherwise,

Reviewed-by: Anthony Liguori 


---
  hw/isa.h  |2 ++
  hw/isa_mmio.c |   30 +++---
  2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/hw/isa.h b/hw/isa.h
index d2b6126..f1f2181 100644
--- a/hw/isa.h
+++ b/hw/isa.h
@@ -4,6 +4,7 @@
  /* ISA bus */

  #include "ioport.h"
+#include "memory.h"
  #include "qdev.h"

  typedef struct ISABus ISABus;
@@ -37,6 +38,7 @@ ISADevice *isa_create_simple(const char *name);

  extern target_phys_addr_t isa_mem_base;

+void isa_mmio_setup(MemoryRegion *mr, target_phys_addr_t size);
  void isa_mmio_init(target_phys_addr_t base, target_phys_addr_t size);

  /* dma.c */
diff --git a/hw/isa_mmio.c b/hw/isa_mmio.c
index ca957fb..600225f 100644
--- a/hw/isa_mmio.c
+++ b/hw/isa_mmio.c
@@ -58,25 +58,25 @@ static uint32_t isa_mmio_readl(void *opaque, 
target_phys_addr_t addr)
  return cpu_inl(addr&  IOPORTS_MASK);
  }

-static CPUWriteMemoryFunc * const isa_mmio_write[] = {
-&isa_mmio_writeb,
-&isa_mmio_writew,
-&isa_mmio_writel,
+static const MemoryRegionOps isa_mmio_ops = {
+.old_mmio = {
+.write = { isa_mmio_writeb, isa_mmio_writew, isa_mmio_writel },
+.read = { isa_mmio_readb, isa_mmio_readw, isa_mmio_readl, },
+},
+.endianness = DEVICE_LITTLE_ENDIAN,
  };

-static CPUReadMemoryFunc * const isa_mmio_read[] = {
-&isa_mmio_readb,
-&isa_mmio_readw,
-&isa_mmio_readl,
-};
+void isa_mmio_setup(MemoryRegion *mr, target_phys_addr_t size)
+{
+memory_region_init_io(mr,&isa_mmio_ops, NULL, "isa-mmio", size);
+}
+
+#include "exec-memory.h"

  void isa_mmio_init(target_phys_addr_t base, target_phys_addr_t size)
  {
-int isa_mmio_iomemtype;
+MemoryRegion *mr = qemu_malloc(sizeof(*mr));

-isa_mmio_iomemtype = cpu_register_io_memory(isa_mmio_read,
-isa_mmio_write,
-NULL,
-DEVICE_LITTLE_ENDIAN);
-cpu_register_physical_memory(base, size, isa_mmio_iomemtype);
+isa_mmio_setup(mr, size);
+memory_region_add_subregion(get_system_memory(), base, mr);
  }

Re: [Qemu-devel] [PATCH v3 25/39] ne2000: convert to memory API

[Anthony Liguori]

On 08/04/2011 08:06 AM, Avi Kivity wrote:

Reviewed-by: Richard Henderson
Signed-off-by: Avi Kivity
---
  hw/ne2000-isa.c |   14 +++---
  hw/ne2000.c |   77 +-
  hw/ne2000.h |8 +
  3 files changed, 59 insertions(+), 40 deletions(-)

diff --git a/hw/ne2000-isa.c b/hw/ne2000-isa.c
index e41dbba..ce7b365 100644
--- a/hw/ne2000-isa.c
+++ b/hw/ne2000-isa.c
@@ -61,24 +61,18 @@ static const VMStateDescription vmstate_isa_ne2000 = {
  }
  };

+#include "exec-memory.h"
+


Should be at the top of the file.

Regards,

Anthony Liguori

Re: [Qemu-devel] Safely reopening image files by stashing fds

[Stefan Hajnoczi]

On Fri, Aug 5, 2011 at 3:28 PM, Christoph Hellwig  wrote:
> On Fri, Aug 05, 2011 at 02:12:48PM +0100, Daniel P. Berrange wrote:
>> > Because you cannot change O_DIRECT on an open fd :(.  This is why
>> > we're going through this pain.
>>
>> Hmm, I remember hearing that before, but looking at the current fcntl()
>> manpage, it claims you *can* change O_DIRECT using SET_FL. Perhaps this
>> is a newish feature, but it'd be nicer to use it if possible ?
>
> It's been there since day 1 of O_DIRECT support.

Sorry, my bad.  So for Linux we could just use fcntl for
block_set_hostcache and not bother with reopening.  However, we will
need to reopen should we wish to support changing O_DSYNC.

Stefan

Re: [Qemu-devel] [PATCH v3 17/39] es1370: convert to memory API

[Anthony Liguori]

On 08/04/2011 08:06 AM, Avi Kivity wrote:

Reviewed-by: Richard Henderson
Signed-off-by: Avi Kivity


Reviewed-by: Anthony Liguori 

Regards,

Anthony Liguori


---
  hw/es1370.c |   43 +--
  1 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/hw/es1370.c b/hw/es1370.c
index 1ed62b7..6a01797 100644
--- a/hw/es1370.c
+++ b/hw/es1370.c
@@ -268,6 +268,7 @@ struct chan {
  typedef struct ES1370State {
  PCIDevice dev;
  QEMUSoundCard card;
+MemoryRegion io;
  struct chan chan[NB_CHANNELS];
  SWVoiceOut *dac_voice[2];
  SWVoiceIn *adc_voice;
@@ -775,7 +776,6 @@ IO_READ_PROTO (es1370_readl)
  return val;
  }

-
  static void es1370_transfer_audio (ES1370State *s, struct chan *d, int 
loop_sel,
 int max, int *irq)
  {
@@ -906,23 +906,20 @@ static void es1370_adc_callback (void *opaque, int avail)
  es1370_run_channel (s, ADC_CHANNEL, avail);
  }

-static void es1370_map (PCIDevice *pci_dev, int region_num,
-pcibus_t addr, pcibus_t size, int type)
-{
-ES1370State *s = DO_UPCAST (ES1370State, dev, pci_dev);
-
-(void) region_num;
-(void) size;
-(void) type;
-
-register_ioport_write (addr, 0x40 * 4, 1, es1370_writeb, s);
-register_ioport_write (addr, 0x40 * 2, 2, es1370_writew, s);
-register_ioport_write (addr, 0x40, 4, es1370_writel, s);
+static const MemoryRegionPortio es1370_portio[] = {
+{ 0, 0x40 * 4, 1, .write = es1370_writeb, },
+{ 0, 0x40 * 2, 2, .write = es1370_writew, },
+{ 0, 0x40, 4, .write = es1370_writel, },
+{ 0, 0x40 * 4, 1, .read = es1370_readb, },
+{ 0, 0x40 * 2, 2, .read = es1370_readw, },
+{ 0, 0x40, 4, .read = es1370_readl, },
+PORTIO_END
+};

-register_ioport_read (addr, 0x40 * 4, 1, es1370_readb, s);
-register_ioport_read (addr, 0x40 * 2, 2, es1370_readw, s);
-register_ioport_read (addr, 0x40, 4, es1370_readl, s);
-}
+static const MemoryRegionOps es1370_io_ops = {
+.old_portio = es1370_portio,
+.endianness = DEVICE_LITTLE_ENDIAN,
+};

  static const VMStateDescription vmstate_es1370_channel = {
  .name = "es1370_channel",
@@ -1011,7 +1008,8 @@ static int es1370_initfn (PCIDevice *dev)
  c[PCI_MIN_GNT] = 0x0c;
  c[PCI_MAX_LAT] = 0x80;

-pci_register_bar (&s->dev, 0, 256, PCI_BASE_ADDRESS_SPACE_IO, es1370_map);
+memory_region_init_io(&s->io,&es1370_io_ops, s, "es1370", 256);
+pci_register_bar_region(&s->dev, 0, PCI_BASE_ADDRESS_SPACE_IO,&s->io);
  qemu_register_reset (es1370_on_reset, s);

  AUD_register_card ("es1370",&s->card);
@@ -1019,6 +1017,14 @@ static int es1370_initfn (PCIDevice *dev)
  return 0;
  }

+static int es1370_exitfn(PCIDevice *dev)
+{
+ES1370State *s = DO_UPCAST (ES1370State, dev, dev);
+
+memory_region_destroy(&s->io);
+return 0;
+}
+
  int es1370_init (PCIBus *bus)
  {
  pci_create_simple (bus, -1, "ES1370");
@@ -1031,6 +1037,7 @@ static PCIDeviceInfo es1370_info = {
  .qdev.size= sizeof (ES1370State),
  .qdev.vmsd=&vmstate_es1370,
  .init = es1370_initfn,
+.exit = es1370_exitfn,
  .vendor_id= PCI_VENDOR_ID_ENSONIQ,
  .device_id= PCI_DEVICE_ID_ENSONIQ_ES1370,
  .class_id = PCI_CLASS_MULTIMEDIA_AUDIO,

Re: [Qemu-devel] [PATCH v3 16/39] eepro100: convert to memory API

[Anthony Liguori]

On 08/04/2011 08:06 AM, Avi Kivity wrote:

Note: the existing code aliases the flash BAR into the MMIO bar.  This is
probably a bug.  This patch does not correct the problem.

Reviewed-by: Richard Henderson
Signed-off-by: Avi Kivity


Reviewed-by: Anthony Liguori 

Regards,

Anthony Liguori


---
  hw/eepro100.c |  182 -
  1 files changed, 37 insertions(+), 145 deletions(-)

diff --git a/hw/eepro100.c b/hw/eepro100.c
index 9b6f4a5..04723f3 100644
--- a/hw/eepro100.c
+++ b/hw/eepro100.c
@@ -228,13 +228,14 @@ typedef struct {
  PCIDevice dev;
  /* Hash register (multicast mask array, multiple individual addresses). */
  uint8_t mult[8];
-int mmio_index;
+MemoryRegion mmio_bar;
+MemoryRegion io_bar;
+MemoryRegion flash_bar;
  NICState *nic;
  NICConf conf;
  uint8_t scb_stat;   /* SCB stat/ack byte */
  uint8_t int_stat;   /* PCI interrupt status */
  /* region must not be saved by nic_save. */
-uint32_t region1;   /* PCI region 1 address */
  uint16_t mdimem[32];
  eeprom_t *eeprom;
  uint32_t device;/* device variant */
@@ -1584,147 +1585,36 @@ static void eepro100_write4(EEPRO100State * s, 
uint32_t addr, uint32_t val)
  }
  }

-/*
- *
- * Port mapped I/O.
- *
- /
-
-static uint32_t ioport_read1(void *opaque, uint32_t addr)
-{
-EEPRO100State *s = opaque;
-#if 0
-logout("addr=%s\n", regname(addr));
-#endif
-return eepro100_read1(s, addr - s->region1);
-}
-
-static uint32_t ioport_read2(void *opaque, uint32_t addr)
-{
-EEPRO100State *s = opaque;
-return eepro100_read2(s, addr - s->region1);
-}
-
-static uint32_t ioport_read4(void *opaque, uint32_t addr)
-{
-EEPRO100State *s = opaque;
-return eepro100_read4(s, addr - s->region1);
-}
-
-static void ioport_write1(void *opaque, uint32_t addr, uint32_t val)
-{
-EEPRO100State *s = opaque;
-#if 0
-logout("addr=%s val=0x%02x\n", regname(addr), val);
-#endif
-eepro100_write1(s, addr - s->region1, val);
-}
-
-static void ioport_write2(void *opaque, uint32_t addr, uint32_t val)
-{
-EEPRO100State *s = opaque;
-eepro100_write2(s, addr - s->region1, val);
-}
-
-static void ioport_write4(void *opaque, uint32_t addr, uint32_t val)
-{
-EEPRO100State *s = opaque;
-eepro100_write4(s, addr - s->region1, val);
-}
-
-/***/
-/* PCI EEPRO100 definitions */
-
-static void pci_map(PCIDevice * pci_dev, int region_num,
-pcibus_t addr, pcibus_t size, int type)
-{
-EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev);
-
-TRACE(OTHER, logout("region %d, addr=0x%08"FMT_PCIBUS", "
-  "size=0x%08"FMT_PCIBUS", type=%d\n",
-  region_num, addr, size, type));
-
-assert(region_num == 1);
-register_ioport_write(addr, size, 1, ioport_write1, s);
-register_ioport_read(addr, size, 1, ioport_read1, s);
-register_ioport_write(addr, size, 2, ioport_write2, s);
-register_ioport_read(addr, size, 2, ioport_read2, s);
-register_ioport_write(addr, size, 4, ioport_write4, s);
-register_ioport_read(addr, size, 4, ioport_read4, s);
-
-s->region1 = addr;
-}
-
-/*
- *
- * Memory mapped I/O.
- *
- /
-
-static void pci_mmio_writeb(void *opaque, target_phys_addr_t addr, uint32_t 
val)
-{
-EEPRO100State *s = opaque;
-#if 0
-logout("addr=%s val=0x%02x\n", regname(addr), val);
-#endif
-eepro100_write1(s, addr, val);
-}
-
-static void pci_mmio_writew(void *opaque, target_phys_addr_t addr, uint32_t 
val)
+static uint64_t eepro100_read(void *opaque, target_phys_addr_t addr,
+  unsigned size)
  {
  EEPRO100State *s = opaque;
-#if 0
-logout("addr=%s val=0x%02x\n", regname(addr), val);
-#endif
-eepro100_write2(s, addr, val);
-}

-static void pci_mmio_writel(void *opaque, target_phys_addr_t addr, uint32_t 
val)
-{
-EEPRO100State *s = opaque;
-#if 0
-logout("addr=%s val=0x%02x\n", regname(addr), val);
-#endif
-eepro100_write4(s, addr, val);
-}
-
-static uint32_t pci_mmio_readb(void *opaque, target_phys_addr_t addr)
-{
-EEPRO100State *s = opaque;
-#if 0
-logout("addr=%s\n", regname(addr));
-#endif
-return eepro100_read1(s, addr);
+switch (size) {
+case 1: return eepro100_read1(s, addr);
+case 2: return eepro100_read2(s, addr);
+case 4: return eepro100_read4(s, addr);
+default: abort();
+}
  }

-static uint32_t pci_mmio_readw(void *opaque, target_phys_addr_t addr)
+static void eepro100_write(void *opaque, target_phys_addr_t addr,
+   uint64_t data, unsigned size

Re: [Qemu-devel] [PATCH v3 15/39] e1000: convert to memory API

[Anthony Liguori]

On 08/04/2011 08:06 AM, Avi Kivity wrote:

Reviewed-by: Richard Henderson
Signed-off-by: Avi Kivity


Reviewed-by: Anthony Liguori 

Regards,

Anthony Liguori


---
  hw/e1000.c |  114 +--
  1 files changed, 48 insertions(+), 66 deletions(-)

diff --git a/hw/e1000.c b/hw/e1000.c
index 96d84f9..dfc082b 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -82,7 +82,8 @@ typedef struct E1000State_st {
  PCIDevice dev;
  NICState *nic;
  NICConf conf;
-int mmio_index;
+MemoryRegion mmio;
+MemoryRegion io;

  uint32_t mac_reg[0x8000];
  uint16_t phy_reg[0x20];
@@ -151,14 +152,6 @@ static const char phy_regcap[0x20] = {
  };

  static void
-ioport_map(PCIDevice *pci_dev, int region_num, pcibus_t addr,
-   pcibus_t size, int type)
-{
-DBGOUT(IO, "e1000_ioport_map addr=0x%04"FMT_PCIBUS
-   " size=0x%08"FMT_PCIBUS"\n", addr, size);
-}
-
-static void
  set_interrupt_cause(E1000State *s, int index, uint32_t val)
  {
  if (val)
@@ -905,7 +898,8 @@ static void (*macreg_writeops[])(E1000State *, int, 
uint32_t) = {
  enum { NWRITEOPS = ARRAY_SIZE(macreg_writeops) };

  static void
-e1000_mmio_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
+e1000_mmio_write(void *opaque, target_phys_addr_t addr, uint64_t val,
+ unsigned size)
  {
  E1000State *s = opaque;
  unsigned int index = (addr&  0x1)>>  2;
@@ -913,31 +907,15 @@ e1000_mmio_writel(void *opaque, target_phys_addr_t addr, 
uint32_t val)
  if (index<  NWRITEOPS&&  macreg_writeops[index]) {
  macreg_writeops[index](s, index, val);
  } else if (index<  NREADOPS&&  macreg_readops[index]) {
-DBGOUT(MMIO, "e1000_mmio_writel RO %x: 0x%04x\n", index<<2, val);
+DBGOUT(MMIO, "e1000_mmio_writel RO %x: 0x%04"PRIx64"\n", index<<2, 
val);
  } else {
-DBGOUT(UNKNOWN, "MMIO unknown write addr=0x%08x,val=0x%08x\n",
+DBGOUT(UNKNOWN, "MMIO unknown write addr=0x%08x,val=0x%08"PRIx64"\n",
 index<<2, val);
  }
  }

-static void
-e1000_mmio_writew(void *opaque, target_phys_addr_t addr, uint32_t val)
-{
-// emulate hw without byte enables: no RMW
-e1000_mmio_writel(opaque, addr&  ~3,
-  (val&  0x)<<  (8*(addr&  3)));
-}
-
-static void
-e1000_mmio_writeb(void *opaque, target_phys_addr_t addr, uint32_t val)
-{
-// emulate hw without byte enables: no RMW
-e1000_mmio_writel(opaque, addr&  ~3,
-  (val&  0xff)<<  (8*(addr&  3)));
-}
-
-static uint32_t
-e1000_mmio_readl(void *opaque, target_phys_addr_t addr)
+static uint64_t
+e1000_mmio_read(void *opaque, target_phys_addr_t addr, unsigned size)
  {
  E1000State *s = opaque;
  unsigned int index = (addr&  0x1)>>  2;
@@ -950,20 +928,39 @@ e1000_mmio_readl(void *opaque, target_phys_addr_t addr)
  return 0;
  }

-static uint32_t
-e1000_mmio_readb(void *opaque, target_phys_addr_t addr)
+static const MemoryRegionOps e1000_mmio_ops = {
+.read = e1000_mmio_read,
+.write = e1000_mmio_write,
+.endianness = DEVICE_LITTLE_ENDIAN,
+.impl = {
+.min_access_size = 4,
+.max_access_size = 4,
+},
+};
+
+static uint64_t e1000_io_read(void *opaque, target_phys_addr_t addr,
+  unsigned size)
  {
-return ((e1000_mmio_readl(opaque, addr&  ~3))>>
-(8 * (addr&  3)))&  0xff;
+E1000State *s = opaque;
+
+(void)s;
+return 0;
  }

-static uint32_t
-e1000_mmio_readw(void *opaque, target_phys_addr_t addr)
+static void e1000_io_write(void *opaque, target_phys_addr_t addr,
+   uint64_t val, unsigned size)
  {
-return ((e1000_mmio_readl(opaque, addr&  ~3))>>
-(8 * (addr&  3)))&  0x;
+E1000State *s = opaque;
+
+(void)s;
  }

+static const MemoryRegionOps e1000_io_ops = {
+.read = e1000_io_read,
+.write = e1000_io_write,
+.endianness = DEVICE_LITTLE_ENDIAN,
+};
+
  static bool is_version_1(void *opaque, int version_id)
  {
  return version_id == 1;
@@ -1083,36 +1080,22 @@ static const uint32_t mac_reg_init[] = {

  /* PCI interface */

-static CPUWriteMemoryFunc * const e1000_mmio_write[] = {
-e1000_mmio_writeb, e1000_mmio_writew,  e1000_mmio_writel
-};
-
-static CPUReadMemoryFunc * const e1000_mmio_read[] = {
-e1000_mmio_readb,  e1000_mmio_readw,   e1000_mmio_readl
-};
-
  static void
-e1000_mmio_map(PCIDevice *pci_dev, int region_num,
-pcibus_t addr, pcibus_t size, int type)
+e1000_mmio_setup(E1000State *d)
  {
-E1000State *d = DO_UPCAST(E1000State, dev, pci_dev);
  int i;
  const uint32_t excluded_regs[] = {
  E1000_MDIC, E1000_ICR, E1000_ICS, E1000_IMS,
  E1000_IMC, E1000_TCTL, E1000_TDT, PNPMMIO_SIZE
  };

-
-DBGOUT(MMIO, "e1000_mmio_map addr=0x%08"FMT_PCIBUS" 0x%08"FMT_PCIBUS"\n",
-   addr, size);
-
-cpu_register_physical_memory(addr, PNPMMIO_SIZE, d->mmio_i

[Qemu-devel] [PATCH V4] e1000: Handle IO Port.

[Anthony PERARD]

This patch introduces the two IOPorts on e1000, IOADDR and IODATA. The
IOADDR is used to specify which register we want to access when we read
or write on IODATA.

This patch fixes some weird behavior that I see when I use e1000 with
QEMU/Xen, the guest memory can be corrupted by this NIC because it will
write on memory that it doesn't own anymore after a reset. It's because
the kernel Linux use the IOPort to reset the network card instead of the
MMIO.

Signed-off-by: Anthony PERARD 
---

change since v3:
  - the e1000_ioport_readb & readw function should return the right bits now.
  - the reset call with the set of E1000_CTRL_RST bit has been in a separate 
patch.

other comment:
The VMState version_id is still increment in this version of the patch because
I am not sure of how works a subsection and if I have to use it here, for the
ioport_addr register value.


 hw/e1000.c |   95 +--
 1 files changed, 85 insertions(+), 10 deletions(-)

diff --git a/hw/e1000.c b/hw/e1000.c
index a1388e9..26aafd5 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -58,6 +58,9 @@ static int debugflags = DBGBIT(TXERR) | DBGBIT(GENERAL);
 #define PNPMMIO_SIZE  0x2
 #define MIN_BUF_SIZE  60 /* Min. octets in an ethernet frame sans FCS */
 
+#define REG_IOADDR 0x0
+#define REG_IODATA 0x4
+
 /*
  * HW models:
  *  E1000_DEV_ID_82540EM works with Windows and Linux
@@ -83,6 +86,8 @@ typedef struct E1000State_st {
 NICState *nic;
 NICConf conf;
 int mmio_index;
+int ioport_base;
+uint32_t ioport_addr;
 
 uint32_t mac_reg[0x8000];
 uint16_t phy_reg[0x20];
@@ -153,14 +158,6 @@ static const char phy_regcap[0x20] = {
 static void e1000_reset(void *opaque);
 
 static void
-ioport_map(PCIDevice *pci_dev, int region_num, pcibus_t addr,
-   pcibus_t size, int type)
-{
-DBGOUT(IO, "e1000_ioport_map addr=0x%04"FMT_PCIBUS
-   " size=0x%08"FMT_PCIBUS"\n", addr, size);
-}
-
-static void
 set_interrupt_cause(E1000State *s, int index, uint32_t val)
 {
 if (val)
@@ -970,6 +967,64 @@ e1000_mmio_readw(void *opaque, target_phys_addr_t addr)
 (8 * (addr & 3))) & 0x;
 }
 
+static void e1000_ioport_writel(void *opaque, uint32_t addr, uint32_t val)
+{
+E1000State *s = opaque;
+
+if (addr == s->ioport_base + REG_IOADDR) {
+DBGOUT(IO, "e1000_ioport_writel write base: 0x%04x\n", val);
+s->ioport_addr = val & 0xf;
+} else if (addr == (s->ioport_base + REG_IODATA)) {
+DBGOUT(IO, "e1000_ioport_writel %x: 0x%04x\n", s->ioport_addr, val);
+e1000_mmio_writel(s, s->ioport_addr, val);
+} else {
+DBGOUT(UNKNOWN, "IO unknown write addr=0x%08x,val=0x%08x\n",
+   addr, val);
+}
+}
+
+static uint32_t e1000_ioport_readb(void *opaque, uint32_t addr)
+{
+E1000State *s = opaque;
+
+if (addr & ~3 == s->ioport_base + REG_IOADDR) {
+return (s->ioport_addr >> (8 * (addr & 3))) & 0xff;
+} else if (addr & ~3 == (s->ioport_base + REG_IODATA)) {
+return e1000_mmio_readb(s, s->ioport_addr);
+} else {
+DBGOUT(UNKNOWN, "IO unknown read addr=0x%08x\n", addr);
+}
+return 0;
+}
+
+static uint32_t e1000_ioport_readw(void *opaque, uint32_t addr)
+{
+E1000State *s = opaque;
+
+if (addr & ~3 == s->ioport_base + REG_IOADDR) {
+return (s->ioport_addr >> (8 * (addr & 3))) & 0x;
+} else if (addr & ~3 == (s->ioport_base + REG_IODATA)) {
+return e1000_mmio_readw(s, s->ioport_addr);
+} else {
+DBGOUT(UNKNOWN, "IO unknown read addr=0x%08x\n", addr);
+}
+return 0;
+}
+
+static uint32_t e1000_ioport_readl(void *opaque, uint32_t addr)
+{
+E1000State *s = opaque;
+
+if (addr == s->ioport_base + REG_IOADDR) {
+return s->ioport_addr & 0xf;
+} else if (addr == (s->ioport_base + REG_IODATA)) {
+return e1000_mmio_readl(s, s->ioport_addr);
+} else {
+DBGOUT(UNKNOWN, "IO unknown read addr=0x%08x\n", addr);
+}
+return 0;
+}
+
 static bool is_version_1(void *opaque, int version_id)
 {
 return version_id == 1;
@@ -977,7 +1032,7 @@ static bool is_version_1(void *opaque, int version_id)
 
 static const VMStateDescription vmstate_e1000 = {
 .name = "e1000",
-.version_id = 2,
+.version_id = 3,
 .minimum_version_id = 1,
 .minimum_version_id_old = 1,
 .fields  = (VMStateField []) {
@@ -1049,6 +1104,7 @@ static const VMStateDescription vmstate_e1000 = {
 VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, RA, 32),
 VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, MTA, 128),
 VMSTATE_UINT32_SUB_ARRAY(mac_reg, E1000State, VFTA, 128),
+VMSTATE_UINT32_V(ioport_addr, E1000State, 3),
 VMSTATE_END_OF_LIST()
 }
 };
@@ -1089,6 +1145,24 @@ static const uint32_t mac_reg_init[] = {
 
 /* PCI interface */
 
+static void
+e1000_ioport_map(PCIDevice *pci_dev, int region_num, pcibus_t addr,
+ pcibus_t si

[Qemu-devel] [PATCH] e1000: Do reset when E1000_CTRL_RST bit is set.

[Anthony PERARD]

Signed-off-by: Anthony PERARD 
---
 hw/e1000.c |   10 --
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/e1000.c b/hw/e1000.c
index 96d84f9..a1388e9 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -150,6 +150,8 @@ static const char phy_regcap[0x20] = {
 [PHY_ID2] = PHY_R, [M88E1000_PHY_SPEC_STATUS] = PHY_R
 };
 
+static void e1000_reset(void *opaque);
+
 static void
 ioport_map(PCIDevice *pci_dev, int region_num, pcibus_t addr,
pcibus_t size, int type)
@@ -202,8 +204,12 @@ rxbufsize(uint32_t v)
 static void
 set_ctrl(E1000State *s, int index, uint32_t val)
 {
-/* RST is self clearing */
-s->mac_reg[CTRL] = val & ~E1000_CTRL_RST;
+DBGOUT(IO, "set ctrl = %08x\n", val);
+if (val & E1000_CTRL_RST) {
+e1000_reset(s);
+return;
+}
+s->mac_reg[CTRL] = val;
 }
 
 static void
-- 
Anthony PERARD

[Qemu-devel] [PATCH][qemu-iotests] Test loading internal snapshots

[Kevin Wolf]

Test loading internal snapshots where the L1 table of the snapshot
is smaller than the current L1 table.

Signed-off-by: Kevin Wolf 
---
 029 |   65 +++
 029.out |   10 +
 group   |1 +
 3 files changed, 76 insertions(+), 0 deletions(-)
 create mode 100755 029
 create mode 100644 029.out

diff --git a/029 b/029
new file mode 100755
index 000..0ad5e45
--- /dev/null
+++ b/029
@@ -0,0 +1,65 @@
+#!/bin/bash
+#
+# Test loading internal snapshots where the L1 table of the snapshot
+# is smaller than the current L1 table.
+#
+# Copyright (C) 2011 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+# creator
+owner=kw...@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1   # failure is the default!
+
+_cleanup()
+{
+   _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+. ./common.pattern
+
+# Any format supporting intenal snapshots
+_supported_fmt qcow2
+_supported_proto generic
+_supported_os Linux
+
+CLUSTER_SIZE=65536
+_make_test_img 64M
+$QEMU_IMG snapshot -c foo $TEST_IMG
+$QEMU_IO -c 'write -b 0 4k' $TEST_IMG | _filter_qemu_io
+$QEMU_IMG snapshot -a foo $TEST_IMG
+_check_test_img
+
+CLUSTER_SIZE=1024
+_make_test_img 16M
+$QEMU_IMG snapshot -c foo $TEST_IMG
+$QEMU_IO -c 'write -b 0 4M' $TEST_IMG | _filter_qemu_io
+$QEMU_IMG snapshot -a foo $TEST_IMG
+_check_test_img
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/029.out b/029.out
new file mode 100644
index 000..c2ad30c
--- /dev/null
+++ b/029.out
@@ -0,0 +1,10 @@
+QA output created by 029
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 cluster_size=65536 
+wrote 4096/4096 bytes at offset 0
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=16777216 cluster_size=1024 
+wrote 4194304/4194304 bytes at offset 0
+4 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
+*** done
diff --git a/group b/group
index b9e90f5..0a5c866 100644
--- a/group
+++ b/group
@@ -35,3 +35,4 @@
 026 rw blkdbg auto
 027 rw auto
 028 rw backing auto
+029 rw auto
-- 
1.7.6

Re: [Qemu-devel] Safely reopening image files by stashing fds

[Christoph Hellwig]

On Fri, Aug 05, 2011 at 02:12:48PM +0100, Daniel P. Berrange wrote:
> > Because you cannot change O_DIRECT on an open fd :(.  This is why
> > we're going through this pain.
> 
> Hmm, I remember hearing that before, but looking at the current fcntl()
> manpage, it claims you *can* change O_DIRECT using SET_FL. Perhaps this
> is a newish feature, but it'd be nicer to use it if possible ?

It's been there since day 1 of O_DIRECT support.

Re: [Qemu-devel] Safely reopening image files by stashing fds

[Christoph Hellwig]

On Fri, Aug 05, 2011 at 10:27:00AM +0100, Stefan Hajnoczi wrote:
> > Why not do the latter unconditionally?
> 
> Because you cannot change O_DIRECT on an open fd :(.  This is why
> we're going through this pain.

You can.  What you can't right now is O_SYNC, but we're going to change
that.

Re: [Qemu-devel] [PATCH v3 14/39] ac97: convert to memory API

[Anthony Liguori]

On 08/04/2011 08:06 AM, Avi Kivity wrote:

fixes BAR sizing as well.

Reviewed-by: Richard Henderson
Signed-off-by: Avi Kivity


Reviewed-by: Anthony Liguori 

Malc, please Ack

Regards,

Anthony Liguori



---
  hw/ac97.c |   88 +++-
  1 files changed, 51 insertions(+), 37 deletions(-)

diff --git a/hw/ac97.c b/hw/ac97.c
index 0b59896..bcddaa6 100644
--- a/hw/ac97.c
+++ b/hw/ac97.c
@@ -160,8 +160,9 @@ typedef struct AC97LinkState {
  SWVoiceIn *voice_mc;
  int invalid_freq[3];
  uint8_t silence[128];
-uint32_t base[2];
  int bup_flag;
+MemoryRegion io_nam;
+MemoryRegion io_nabm;
  } AC97LinkState;

  enum {
@@ -583,7 +584,7 @@ static uint32_t nam_readw (void *opaque, uint32_t addr)
  {
  AC97LinkState *s = opaque;
  uint32_t val = ~0U;
-uint32_t index = addr - s->base[0];
+uint32_t index = addr;
  s->cas = 0;
  val = mixer_load (s, index);
  return val;
@@ -611,7 +612,7 @@ static void nam_writeb (void *opaque, uint32_t addr, 
uint32_t val)
  static void nam_writew (void *opaque, uint32_t addr, uint32_t val)
  {
  AC97LinkState *s = opaque;
-uint32_t index = addr - s->base[0];
+uint32_t index = addr;
  s->cas = 0;
  switch (index) {
  case AC97_Reset:
@@ -714,7 +715,7 @@ static uint32_t nabm_readb (void *opaque, uint32_t addr)
  {
  AC97LinkState *s = opaque;
  AC97BusMasterRegs *r = NULL;
-uint32_t index = addr - s->base[1];
+uint32_t index = addr;
  uint32_t val = ~0U;

  switch (index) {
@@ -769,7 +770,7 @@ static uint32_t nabm_readw (void *opaque, uint32_t addr)
  {
  AC97LinkState *s = opaque;
  AC97BusMasterRegs *r = NULL;
-uint32_t index = addr - s->base[1];
+uint32_t index = addr;
  uint32_t val = ~0U;

  switch (index) {
@@ -798,7 +799,7 @@ static uint32_t nabm_readl (void *opaque, uint32_t addr)
  {
  AC97LinkState *s = opaque;
  AC97BusMasterRegs *r = NULL;
-uint32_t index = addr - s->base[1];
+uint32_t index = addr;
  uint32_t val = ~0U;

  switch (index) {
@@ -848,7 +849,7 @@ static void nabm_writeb (void *opaque, uint32_t addr, 
uint32_t val)
  {
  AC97LinkState *s = opaque;
  AC97BusMasterRegs *r = NULL;
-uint32_t index = addr - s->base[1];
+uint32_t index = addr;
  switch (index) {
  case PI_LVI:
  case PO_LVI:
@@ -904,7 +905,7 @@ static void nabm_writew (void *opaque, uint32_t addr, 
uint32_t val)
  {
  AC97LinkState *s = opaque;
  AC97BusMasterRegs *r = NULL;
-uint32_t index = addr - s->base[1];
+uint32_t index = addr;
  switch (index) {
  case PI_SR:
  case PO_SR:
@@ -924,7 +925,7 @@ static void nabm_writel (void *opaque, uint32_t addr, 
uint32_t val)
  {
  AC97LinkState *s = opaque;
  AC97BusMasterRegs *r = NULL;
-uint32_t index = addr - s->base[1];
+uint32_t index = addr;
  switch (index) {
  case PI_BDBAR:
  case PO_BDBAR:
@@ -1230,31 +1231,33 @@ static const VMStateDescription vmstate_ac97 = {
  }
  };

-static void ac97_map (PCIDevice *pci_dev, int region_num,
-  pcibus_t addr, pcibus_t size, int type)
-{
-AC97LinkState *s = DO_UPCAST (AC97LinkState, dev, pci_dev);
-PCIDevice *d =&s->dev;
-
-if (!region_num) {
-s->base[0] = addr;
-register_ioport_read (addr, 256 * 1, 1, nam_readb, d);
-register_ioport_read (addr, 256 * 2, 2, nam_readw, d);
-register_ioport_read (addr, 256 * 4, 4, nam_readl, d);
-register_ioport_write (addr, 256 * 1, 1, nam_writeb, d);
-register_ioport_write (addr, 256 * 2, 2, nam_writew, d);
-register_ioport_write (addr, 256 * 4, 4, nam_writel, d);
-}
-else {
-s->base[1] = addr;
-register_ioport_read (addr, 64 * 1, 1, nabm_readb, d);
-register_ioport_read (addr, 64 * 2, 2, nabm_readw, d);
-register_ioport_read (addr, 64 * 4, 4, nabm_readl, d);
-register_ioport_write (addr, 64 * 1, 1, nabm_writeb, d);
-register_ioport_write (addr, 64 * 2, 2, nabm_writew, d);
-register_ioport_write (addr, 64 * 4, 4, nabm_writel, d);
-}
-}
+static const MemoryRegionPortio nam_portio[] = {
+{ 0, 256 * 1, 1, .read = nam_readb, },
+{ 0, 256 * 2, 2, .read = nam_readw, },
+{ 0, 256 * 4, 4, .read = nam_readl, },
+{ 0, 256 * 1, 1, .write = nam_writeb, },
+{ 0, 256 * 2, 2, .write = nam_writew, },
+{ 0, 256 * 4, 4, .write = nam_writel, },
+PORTIO_END,
+};
+
+static const MemoryRegionOps ac97_io_nam_ops = {
+.old_portio = nam_portio,
+};
+
+static const MemoryRegionPortio nabm_portio[] = {
+{ 0, 64 * 1, 1, .read = nabm_readb, },
+{ 0, 64 * 2, 2, .read = nabm_readw, },
+{ 0, 64 * 4, 4, .read = nabm_readl, },
+{ 0, 64 * 1, 1, .write = nabm_writeb, },
+{ 0, 64 * 2, 2, .write = nabm_writew, },
+{ 0, 64 * 4, 4, .write = nabm_writel, },
+PORTIO_END
+};
+
+static const MemoryRegionOps ac97_io_nabm_o

Re: [Qemu-devel] [PATCH v3 13/39] rtl8139: convert to memory API

[Anthony Liguori]

On 08/04/2011 08:06 AM, Avi Kivity wrote:

Reviewed-by: Richard Henderson
Signed-off-by: Avi Kivity
---
  hw/rtl8139.c |   72 ++---
  1 files changed, 38 insertions(+), 34 deletions(-)

diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index 5214b8c..dfbab90 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -474,7 +474,6 @@ typedef struct RTL8139State {

  NICState *nic;
  NICConf conf;
-int rtl8139_mmio_io_addr;

  /* C ring mode */
  uint32_t   currTxDesc;
@@ -506,6 +505,9 @@ typedef struct RTL8139State {
  QEMUTimer *timer;
  int64_t TimerExpire;

+MemoryRegion bar_io;
+MemoryRegion bar_mem;
+
  /* Support migration to/from old versions */
  int rtl8139_mmio_io_addr_dummy;
  } RTL8139State;
@@ -3283,7 +3285,7 @@ static void rtl8139_pre_save(void *opaque)
  rtl8139_set_next_tctr_time(s, current_time);
  s->TCTR = muldiv64(current_time - s->TCTR_base, PCI_FREQUENCY,
 get_ticks_per_sec());
-s->rtl8139_mmio_io_addr_dummy = s->rtl8139_mmio_io_addr;
+s->rtl8139_mmio_io_addr_dummy = 0;
  }

  static const VMStateDescription vmstate_rtl8139 = {
@@ -3379,31 +3381,35 @@ static const VMStateDescription vmstate_rtl8139 = {
  /***/
  /* PCI RTL8139 definitions */

-static void rtl8139_ioport_map(PCIDevice *pci_dev, int region_num,
-   pcibus_t addr, pcibus_t size, int type)
-{
-RTL8139State *s = DO_UPCAST(RTL8139State, dev, pci_dev);
-
-register_ioport_write(addr, 0x100, 1, rtl8139_ioport_writeb, s);
-register_ioport_read( addr, 0x100, 1, rtl8139_ioport_readb,  s);
-
-register_ioport_write(addr, 0x100, 2, rtl8139_ioport_writew, s);
-register_ioport_read( addr, 0x100, 2, rtl8139_ioport_readw,  s);
-
-register_ioport_write(addr, 0x100, 4, rtl8139_ioport_writel, s);
-register_ioport_read( addr, 0x100, 4, rtl8139_ioport_readl,  s);
-}
+static const MemoryRegionPortio rtl8139_portio[] = {
+{ 0, 0x100, 1, .read = rtl8139_ioport_readb, },
+{ 0, 0x100, 1, .write = rtl8139_ioport_writeb, },
+{ 0, 0x100, 2, .read = rtl8139_ioport_readw, },
+{ 0, 0x100, 2, .write = rtl8139_ioport_writew, },
+{ 0, 0x100, 4, .read = rtl8139_ioport_readl, },
+{ 0, 0x100, 4, .write = rtl8139_ioport_writel, },
+PORTIO_END


Hrm, I missed this #define when it was introduced.

Elsewhere we use:

VMSTATE_END_OF_LIST()
DEFINE_PROP_END_OF_LIST()

For consistency, we ought to use PORTIO_END_OF_LIST()

Otherwise:

Reviewed-by: Anthony Liguori 

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 03/16] Add iov_clear()

[Gerd Hoffmann]

On 08/05/11 13:30, Kevin Wolf wrote:

Am 04.08.2011 17:10, schrieb Gerd Hoffmann:

Fill the spefified area with zeros.

Signed-off-by: Gerd Hoffmann


Looks like we're starting to duplicate everything in qemu_iovec_* and
iov_*...

Any reason not to use QEMUIOVector?


I *do* use QEMUIOVector, but for the actual copy I'm using 
iov_{from,to}_buf() instead of qemu_iovec_{from,to}_buffer because the 
former allows to specify an offset.


But, yea, we have some duplication here, qemu_iovec_* can just call 
iov_* instead of reimplementing stuff ...


cheers,
  Gerd

diff --git a/cutils.c b/cutils.c
index f9a7e36..b8ca4c2 100644
--- a/cutils.c
+++ b/cutils.c
@@ -24,6 +24,7 @@
 #include "qemu-common.h"
 #include "host-utils.h"
 #include 
+#include "iov.h"
 
 void pstrcpy(char *buf, int buf_size, const char *str)
 {
@@ -230,29 +231,12 @@ void qemu_iovec_reset(QEMUIOVector *qiov)
 
 void qemu_iovec_to_buffer(QEMUIOVector *qiov, void *buf)
 {
-uint8_t *p = (uint8_t *)buf;
-int i;
-
-for (i = 0; i < qiov->niov; ++i) {
-memcpy(p, qiov->iov[i].iov_base, qiov->iov[i].iov_len);
-p += qiov->iov[i].iov_len;
-}
+iov_to_buf(qiov->iov, qiov->niov, buf, 0, qiov->size);
 }
 
 void qemu_iovec_from_buffer(QEMUIOVector *qiov, const void *buf, size_t count)
 {
-const uint8_t *p = (const uint8_t *)buf;
-size_t copy;
-int i;
-
-for (i = 0; i < qiov->niov && count; ++i) {
-copy = count;
-if (copy > qiov->iov[i].iov_len)
-copy = qiov->iov[i].iov_len;
-memcpy(qiov->iov[i].iov_base, p, copy);
-p += copy;
-count -= copy;
-}
+iov_from_buf(qiov->iov, qiov->niov, buf, 0, count);
 }
 
 void qemu_iovec_memset(QEMUIOVector *qiov, int c, size_t count)

Re: [Qemu-devel] [PATCH v3 12/39] pci: allow I/O BARs to be registered with pci_register_bar_region()

[Anthony Liguori]

On 08/04/2011 08:06 AM, Avi Kivity wrote:

Reviewed-by: Richard Henderson
Signed-off-by: Avi Kivity


Reviewed-by: Anthony Liguori 

Regards,

Anthony Liguori


---
  hw/pci.c   |   43 +++
  hw/pci.h   |1 +
  hw/pci_internals.h |3 ++-
  3 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index 2659d96..980840f 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -271,7 +271,8 @@ void pci_bus_new_inplace(PCIBus *bus, DeviceState *parent,
  qbus_create_inplace(&bus->qbus,&pci_bus_info, parent, name);
  assert(PCI_FUNC(devfn_min) == 0);
  bus->devfn_min = devfn_min;
-bus->address_space = address_space_mem;
+bus->address_space_mem = address_space_mem;
+bus->address_space_io = address_space_io;

  /* host bridge */
  QLIST_INIT(&bus->child);
@@ -847,12 +848,11 @@ static void pci_unregister_io_regions(PCIDevice *pci_dev)
  r =&pci_dev->io_regions[i];
  if (!r->size || r->addr == PCI_BAR_UNMAPPED)
  continue;
-if (r->type == PCI_BASE_ADDRESS_SPACE_IO) {
-isa_unassign_ioport(r->addr, r->filtered_size);
+if (r->memory) {
+memory_region_del_subregion(r->address_space, r->memory);
  } else {
-if (r->memory) {
-memory_region_del_subregion(pci_dev->bus->address_space,
-r->memory);
+if (r->type == PCI_BASE_ADDRESS_SPACE_IO) {
+isa_unassign_ioport(r->addr, r->filtered_size);
  } else {
  cpu_register_physical_memory(pci_to_cpu_addr(pci_dev->bus,
   r->addr),
@@ -934,9 +934,11 @@ static void pci_simple_bar_mapfunc_region(PCIDevice 
*pci_dev, int region_num,
pcibus_t addr, pcibus_t size,
int type)
  {
-memory_region_add_subregion_overlap(pci_dev->bus->address_space,
+PCIIORegion *r =&pci_dev->io_regions[region_num];
+
+memory_region_add_subregion_overlap(r->address_space,
  addr,
-pci_dev->io_regions[region_num].memory,
+r->memory,
  1);
  }

@@ -953,9 +955,13 @@ void pci_register_bar_region(PCIDevice *pci_dev, int 
region_num,
   uint8_t attr, MemoryRegion *memory)
  {
  pci_register_bar(pci_dev, region_num, memory_region_size(memory),
- PCI_BASE_ADDRESS_SPACE_MEMORY | attr,
+ attr,
   pci_simple_bar_mapfunc_region);
  pci_dev->io_regions[region_num].memory = memory;
+pci_dev->io_regions[region_num].address_space
+= attr&  PCI_BASE_ADDRESS_SPACE_IO
+? pci_dev->bus->address_space_io
+: pci_dev->bus->address_space_mem;
  }

  pcibus_t pci_get_bar_addr(PCIDevice *pci_dev, int region_num)
@@ -1090,7 +1096,9 @@ static void pci_update_mappings(PCIDevice *d)

  /* now do the real mapping */
  if (r->addr != PCI_BAR_UNMAPPED) {
-if (r->type&  PCI_BASE_ADDRESS_SPACE_IO) {
+if (r->memory) {
+memory_region_del_subregion(r->address_space, r->memory);
+} else if (r->type&  PCI_BASE_ADDRESS_SPACE_IO) {
  int class;
  /* NOTE: specific hack for IDE in PC case:
 only one byte must be mapped. */
@@ -1101,16 +1109,11 @@ static void pci_update_mappings(PCIDevice *d)
  isa_unassign_ioport(r->addr, r->filtered_size);
  }
  } else {
-if (r->memory) {
-memory_region_del_subregion(d->bus->address_space,
-r->memory);
-} else {
-cpu_register_physical_memory(pci_to_cpu_addr(d->bus,
- r->addr),
- r->filtered_size,
- IO_MEM_UNASSIGNED);
-qemu_unregister_coalesced_mmio(r->addr, r->filtered_size);
-}
+cpu_register_physical_memory(pci_to_cpu_addr(d->bus,
+ r->addr),
+ r->filtered_size,
+ IO_MEM_UNASSIGNED);
+qemu_unregister_coalesced_mmio(r->addr, r->filtered_size);
  }
  }
  r->addr = new_addr;
diff --git a/hw/pci.h b/hw/pci.h
index 45b30fa..928e96c 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -95,6 +95,7 @@ typedef struct PCIIORegion {
  PCIMapIORegionFunc *map_func;
  ram_addr_t ram_addr;
  MemoryRegion *memory;
+ 

Re: [Qemu-devel] [PATCH v3 11/39] pci: pass I/O address space to new PCI bus

[Anthony Liguori]

On 08/04/2011 08:06 AM, Avi Kivity wrote:

This lets us register BARs in the I/O address space.

Reviewed-by: Richard Henderson
Signed-off-by: Avi Kivity


Reviewed-by: Anthony Liguori 

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH v3 10/39] Integrate I/O memory regions into qemu

[Anthony Liguori]

On 08/04/2011 08:06 AM, Avi Kivity wrote:

get_system_io() returns the root I/O memory region.

Reviewed-by: Richard Henderson
Signed-off-by: Avi Kivity
---
  exec-memory.h |2 ++
  exec.c|   10 ++
  2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/exec-memory.h b/exec-memory.h
index c439aba..999fd69 100644
--- a/exec-memory.h
+++ b/exec-memory.h
@@ -28,6 +28,8 @@
   */
  MemoryRegion *get_system_memory(void);

+MemoryRegion *get_system_io(void);
+


Could you add a doc comment for this function?

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH v3 09/39] cirrus: simplify linear framebuffer access functions

[Anthony Liguori]

On 08/04/2011 08:06 AM, Avi Kivity wrote:

Make use of the memory API's ability to satisfy multi-byte accesses via
multiple single-byte accesses.

Reviewed-by: Richard Henderson
Signed-off-by: Avi Kivity


Reviewed-by: Anthony Liguori 

Regards,

Anthony Liguori


---
  hw/cirrus_vga.c |   74 ++-
  1 files changed, 8 insertions(+), 66 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index 3db15bf..15ccf4a 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -2249,7 +2249,8 @@ static void cirrus_cursor_draw_line(VGACommonState *s1, 
uint8_t *d1, int scr_y)
   *
   ***/

-static uint32_t cirrus_linear_readb(void *opaque, target_phys_addr_t addr)
+static uint64_t cirrus_linear_read(void *opaque, target_phys_addr_t addr,
+   unsigned size)
  {
  CirrusVGAState *s = opaque;
  uint32_t ret;
@@ -2277,28 +2278,8 @@ static uint32_t cirrus_linear_readb(void *opaque, 
target_phys_addr_t addr)
  return ret;
  }

-static uint32_t cirrus_linear_readw(void *opaque, target_phys_addr_t addr)
-{
-uint32_t v;
-
-v = cirrus_linear_readb(opaque, addr);
-v |= cirrus_linear_readb(opaque, addr + 1)<<  8;
-return v;
-}
-
-static uint32_t cirrus_linear_readl(void *opaque, target_phys_addr_t addr)
-{
-uint32_t v;
-
-v = cirrus_linear_readb(opaque, addr);
-v |= cirrus_linear_readb(opaque, addr + 1)<<  8;
-v |= cirrus_linear_readb(opaque, addr + 2)<<  16;
-v |= cirrus_linear_readb(opaque, addr + 3)<<  24;
-return v;
-}
-
-static void cirrus_linear_writeb(void *opaque, target_phys_addr_t addr,
-uint32_t val)
+static void cirrus_linear_write(void *opaque, target_phys_addr_t addr,
+uint64_t val, unsigned size)
  {
  CirrusVGAState *s = opaque;
  unsigned mode;
@@ -2338,49 +2319,6 @@ static void cirrus_linear_writeb(void *opaque, 
target_phys_addr_t addr,
  }
  }

-static void cirrus_linear_writew(void *opaque, target_phys_addr_t addr,
-uint32_t val)
-{
-cirrus_linear_writeb(opaque, addr, val&  0xff);
-cirrus_linear_writeb(opaque, addr + 1, (val>>  8)&  0xff);
-}
-
-static void cirrus_linear_writel(void *opaque, target_phys_addr_t addr,
-uint32_t val)
-{
-cirrus_linear_writeb(opaque, addr, val&  0xff);
-cirrus_linear_writeb(opaque, addr + 1, (val>>  8)&  0xff);
-cirrus_linear_writeb(opaque, addr + 2, (val>>  16)&  0xff);
-cirrus_linear_writeb(opaque, addr + 3, (val>>  24)&  0xff);
-}
-
-
-static uint64_t cirrus_linear_read(void *opaque, target_phys_addr_t addr,
-   unsigned size)
-{
-CirrusVGAState *s = opaque;
-
-switch (size) {
-case 1: return cirrus_linear_readb(s, addr);
-case 2: return cirrus_linear_readw(s, addr);
-case 4: return cirrus_linear_readl(s, addr);
-default: abort();
-}
-}
-
-static void cirrus_linear_write(void *opaque, target_phys_addr_t addr,
-uint64_t data, unsigned size)
-{
-CirrusVGAState *s = opaque;
-
-switch (size) {
-case 1: return cirrus_linear_writeb(s, addr, data);
-case 2: return cirrus_linear_writew(s, addr, data);
-case 4: return cirrus_linear_writel(s, addr, data);
-default: abort();
-}
-}
-
  /***
   *
   *  system to screen memory access
@@ -2860,6 +2798,10 @@ static const MemoryRegionOps cirrus_linear_io_ops = {
  .read = cirrus_linear_read,
  .write = cirrus_linear_write,
  .endianness = DEVICE_LITTLE_ENDIAN,
+.impl = {
+.min_access_size = 1,
+.max_access_size = 1,
+},
  };

  static void cirrus_init_common(CirrusVGAState * s, int device_id, int is_pci)

Re: [Qemu-devel] [PATCH v3 08/39] vga: simplify vga window mmio access functions

[Anthony Liguori]

On 08/04/2011 08:06 AM, Avi Kivity wrote:

Make use of the memory API's ability to satisfy multi-byte accesses via
multiple single-byte accesses.

We have to keep vga_mem_{read,write}b() since they're used by cirrus.

Reviewed-by: Richard Henderson
Signed-off-by: Avi Kivity


Reviewed-by: Anthony Liguori 

Regards,

Anthony Liguori


---
  hw/cirrus_vga.c |4 +-
  hw/vga.c|   56 +++---
  hw/vga_int.h|4 +-
  3 files changed, 12 insertions(+), 52 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index 92696d9..3db15bf 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -1965,7 +1965,7 @@ static uint64_t cirrus_vga_mem_read(void *opaque,
  uint32_t val;

  if ((s->vga.sr[0x07]&  0x01) == 0) {
-   return vga_mem_readb(s, addr);
+return vga_mem_readb(&s->vga, addr);
  }

  if (addr<  0x1) {
@@ -2010,7 +2010,7 @@ static void cirrus_vga_mem_write(void *opaque,
  unsigned mode;

  if ((s->vga.sr[0x07]&  0x01) == 0) {
-   vga_mem_writeb(s, addr, mem_value);
+vga_mem_writeb(&s->vga, addr, mem_value);
  return;
  }

diff --git a/hw/vga.c b/hw/vga.c
index cdd8255..f5dd519 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -707,9 +707,8 @@ static void vbe_ioport_write_data(void *opaque, uint32_t 
addr, uint32_t val)
  #endif

  /* called for accesses between 0xa and 0xc */
-uint32_t vga_mem_readb(void *opaque, target_phys_addr_t addr)
+uint32_t vga_mem_readb(VGACommonState *s, target_phys_addr_t addr)
  {
-VGACommonState *s = opaque;
  int memory_map_mode, plane;
  uint32_t ret;

@@ -763,28 +762,9 @@ uint32_t vga_mem_readb(void *opaque, target_phys_addr_t 
addr)
  return ret;
  }

-static uint32_t vga_mem_readw(void *opaque, target_phys_addr_t addr)
-{
-uint32_t v;
-v = vga_mem_readb(opaque, addr);
-v |= vga_mem_readb(opaque, addr + 1)<<  8;
-return v;
-}
-
-static uint32_t vga_mem_readl(void *opaque, target_phys_addr_t addr)
-{
-uint32_t v;
-v = vga_mem_readb(opaque, addr);
-v |= vga_mem_readb(opaque, addr + 1)<<  8;
-v |= vga_mem_readb(opaque, addr + 2)<<  16;
-v |= vga_mem_readb(opaque, addr + 3)<<  24;
-return v;
-}
-
  /* called for accesses between 0xa and 0xc */
-void vga_mem_writeb(void *opaque, target_phys_addr_t addr, uint32_t val)
+void vga_mem_writeb(VGACommonState *s, target_phys_addr_t addr, uint32_t val)
  {
-VGACommonState *s = opaque;
  int memory_map_mode, plane, write_mode, b, func_select, mask;
  uint32_t write_mask, bit_mask, set_mask;

@@ -916,20 +896,6 @@ void vga_mem_writeb(void *opaque, target_phys_addr_t addr, 
uint32_t val)
  }
  }

-static void vga_mem_writew(void *opaque, target_phys_addr_t addr, uint32_t val)
-{
-vga_mem_writeb(opaque, addr, val&  0xff);
-vga_mem_writeb(opaque, addr + 1, (val>>  8)&  0xff);
-}
-
-static void vga_mem_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
-{
-vga_mem_writeb(opaque, addr, val&  0xff);
-vga_mem_writeb(opaque, addr + 1, (val>>  8)&  0xff);
-vga_mem_writeb(opaque, addr + 2, (val>>  16)&  0xff);
-vga_mem_writeb(opaque, addr + 3, (val>>  24)&  0xff);
-}
-
  typedef void vga_draw_glyph8_func(uint8_t *d, int linesize,
   const uint8_t *font_ptr, int h,
   uint32_t fgcol, uint32_t bgcol);
@@ -2104,12 +2070,7 @@ static uint64_t vga_mem_read(void *opaque, 
target_phys_addr_t addr,
  {
  VGACommonState *s = opaque;

-switch (size) {
-case 1: return vga_mem_readb(s, addr);
-case 2: return vga_mem_readw(s, addr);
-case 4: return vga_mem_readl(s, addr);
-default: abort();
-}
+return vga_mem_readb(s, addr);
  }

  static void vga_mem_write(void *opaque, target_phys_addr_t addr,
@@ -2117,18 +2078,17 @@ static void vga_mem_write(void *opaque, 
target_phys_addr_t addr,
  {
  VGACommonState *s = opaque;

-switch (size) {
-case 1: return vga_mem_writeb(s, addr, data);
-case 2: return vga_mem_writew(s, addr, data);
-case 4: return vga_mem_writel(s, addr, data);
-default: abort();
-}
+return vga_mem_writeb(s, addr, data);
  }

  const MemoryRegionOps vga_mem_ops = {
  .read = vga_mem_read,
  .write = vga_mem_write,
  .endianness = DEVICE_LITTLE_ENDIAN,
+.impl = {
+.min_access_size = 1,
+.max_access_size = 1,
+},
  };

  static int vga_common_post_load(void *opaque, int version_id)
diff --git a/hw/vga_int.h b/hw/vga_int.h
index 4592d2c..100d98c 100644
--- a/hw/vga_int.h
+++ b/hw/vga_int.h
@@ -198,8 +198,8 @@ void vga_dirty_log_restart(VGACommonState *s);
  extern const VMStateDescription vmstate_vga_common;
  uint32_t vga_ioport_read(void *opaque, uint32_t addr);
  void vga_ioport_write(void *opaque, uint32_t addr, uint32_t val);
-uint32_t vga_mem_readb(void *opaque, target_phys_addr_t addr);
-void vga_mem_writeb(void *opaque, target_phys_addr_t addr, uint32_t val);

Re: [Qemu-devel] [PATCH v3 07/39] cirrus: simplify vga window mmio access functions

[Anthony Liguori]

On 08/04/2011 08:06 AM, Avi Kivity wrote:

Make use of the memory API's ability to satisfy multi-byte accesses via
multiple single-byte accesses.

Reviewed-by: Richard Henderson
Signed-off-by: Avi Kivity


Reviewed-by: Anthony Liguori 

Regards,

Anthony Liguori


---
  hw/cirrus_vga.c |   79 +++---
  1 files changed, 11 insertions(+), 68 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index b8a51b4..92696d9 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -1955,7 +1955,9 @@ static void 
cirrus_mem_writeb_mode4and5_16bpp(CirrusVGAState * s,
   *
   ***/

-static uint32_t cirrus_vga_mem_readb(void *opaque, target_phys_addr_t addr)
+static uint64_t cirrus_vga_mem_read(void *opaque,
+target_phys_addr_t addr,
+uint32_t size)
  {
  CirrusVGAState *s = opaque;
  unsigned bank_index;
@@ -1966,8 +1968,6 @@ static uint32_t cirrus_vga_mem_readb(void *opaque, 
target_phys_addr_t addr)
return vga_mem_readb(s, addr);
  }

-addr&= 0x1;
-
  if (addr<  0x1) {
/* XXX handle bitblt */
/* video memory */
@@ -1999,28 +1999,10 @@ static uint32_t cirrus_vga_mem_readb(void *opaque, 
target_phys_addr_t addr)
  return val;
  }

-static uint32_t cirrus_vga_mem_readw(void *opaque, target_phys_addr_t addr)
-{
-uint32_t v;
-
-v = cirrus_vga_mem_readb(opaque, addr);
-v |= cirrus_vga_mem_readb(opaque, addr + 1)<<  8;
-return v;
-}
-
-static uint32_t cirrus_vga_mem_readl(void *opaque, target_phys_addr_t addr)
-{
-uint32_t v;
-
-v = cirrus_vga_mem_readb(opaque, addr);
-v |= cirrus_vga_mem_readb(opaque, addr + 1)<<  8;
-v |= cirrus_vga_mem_readb(opaque, addr + 2)<<  16;
-v |= cirrus_vga_mem_readb(opaque, addr + 3)<<  24;
-return v;
-}
-
-static void cirrus_vga_mem_writeb(void *opaque, target_phys_addr_t addr,
-  uint32_t mem_value)
+static void cirrus_vga_mem_write(void *opaque,
+ target_phys_addr_t addr,
+ uint64_t mem_value,
+ uint32_t size)
  {
  CirrusVGAState *s = opaque;
  unsigned bank_index;
@@ -2032,8 +2014,6 @@ static void cirrus_vga_mem_writeb(void *opaque, 
target_phys_addr_t addr,
  return;
  }

-addr&= 0x1;
-
  if (addr<  0x1) {
if (s->cirrus_srcptr != s->cirrus_srcptr_end) {
/* bitblt */
@@ -2083,51 +2063,14 @@ static void cirrus_vga_mem_writeb(void *opaque, 
target_phys_addr_t addr,
  }
  }

-static void cirrus_vga_mem_writew(void *opaque, target_phys_addr_t addr, 
uint32_t val)
-{
-cirrus_vga_mem_writeb(opaque, addr, val&  0xff);
-cirrus_vga_mem_writeb(opaque, addr + 1, (val>>  8)&  0xff);
-}
-
-static void cirrus_vga_mem_writel(void *opaque, target_phys_addr_t addr, 
uint32_t val)
-{
-cirrus_vga_mem_writeb(opaque, addr, val&  0xff);
-cirrus_vga_mem_writeb(opaque, addr + 1, (val>>  8)&  0xff);
-cirrus_vga_mem_writeb(opaque, addr + 2, (val>>  16)&  0xff);
-cirrus_vga_mem_writeb(opaque, addr + 3, (val>>  24)&  0xff);
-}
-
-static uint64_t cirrus_vga_mem_read(void *opaque,
-target_phys_addr_t addr,
-uint32_t size)
-{
-CirrusVGAState *s = opaque;
-
-switch (size) {
-case 1: return cirrus_vga_mem_readb(s, addr);
-case 2: return cirrus_vga_mem_readw(s, addr);
-case 4: return cirrus_vga_mem_readl(s, addr);
-default: abort();
-}
-}
-
-static void cirrus_vga_mem_write(void *opaque, target_phys_addr_t addr,
- uint64_t data, unsigned size)
-{
-CirrusVGAState *s = opaque;
-
-switch (size) {
-case 1: return cirrus_vga_mem_writeb(s, addr, data);
-case 2: return cirrus_vga_mem_writew(s, addr, data);
-case 4: return cirrus_vga_mem_writel(s, addr, data);
-default: abort();
-}
-};
-
  static const MemoryRegionOps cirrus_vga_mem_ops = {
  .read = cirrus_vga_mem_read,
  .write = cirrus_vga_mem_write,
  .endianness = DEVICE_LITTLE_ENDIAN,
+.impl = {
+.min_access_size = 1,
+.max_access_size = 1,
+},
  };

  /***

Re: [Qemu-devel] [PATCH v3 06/39] cirrus: simplify bitblt BAR access functions

[Anthony Liguori]

On 08/04/2011 08:06 AM, Avi Kivity wrote:

Make use of the memory API's ability to satisfy multi-byte accesses via
multiple single-byte accesses.

Reviewed-by: Richard Henderson
Signed-off-by: Avi Kivity


Reviewed-by: Anthony Liguori 

Regards,

Anthony Liguori


---
  hw/cirrus_vga.c |   81 +--
  1 files changed, 13 insertions(+), 68 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index 6e1aa75..b8a51b4 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -2445,37 +2445,23 @@ static void cirrus_linear_write(void *opaque, 
target_phys_addr_t addr,
   ***/


-static uint32_t cirrus_linear_bitblt_readb(void *opaque, target_phys_addr_t 
addr)
+static uint64_t cirrus_linear_bitblt_read(void *opaque,
+  target_phys_addr_t addr,
+  unsigned size)
  {
+CirrusVGAState *s = opaque;
  uint32_t ret;

  /* XXX handle bitblt */
+(void)s;
  ret = 0xff;
  return ret;
  }

-static uint32_t cirrus_linear_bitblt_readw(void *opaque, target_phys_addr_t 
addr)
-{
-uint32_t v;
-
-v = cirrus_linear_bitblt_readb(opaque, addr);
-v |= cirrus_linear_bitblt_readb(opaque, addr + 1)<<  8;
-return v;
-}
-
-static uint32_t cirrus_linear_bitblt_readl(void *opaque, target_phys_addr_t 
addr)
-{
-uint32_t v;
-
-v = cirrus_linear_bitblt_readb(opaque, addr);
-v |= cirrus_linear_bitblt_readb(opaque, addr + 1)<<  8;
-v |= cirrus_linear_bitblt_readb(opaque, addr + 2)<<  16;
-v |= cirrus_linear_bitblt_readb(opaque, addr + 3)<<  24;
-return v;
-}
-
-static void cirrus_linear_bitblt_writeb(void *opaque, target_phys_addr_t addr,
-uint32_t val)
+static void cirrus_linear_bitblt_write(void *opaque,
+   target_phys_addr_t addr,
+   uint64_t val,
+   unsigned size)
  {
  CirrusVGAState *s = opaque;

@@ -2488,55 +2474,14 @@ static void cirrus_linear_bitblt_writeb(void *opaque, 
target_phys_addr_t addr,
  }
  }

-static void cirrus_linear_bitblt_writew(void *opaque, target_phys_addr_t addr,
-uint32_t val)
-{
-cirrus_linear_bitblt_writeb(opaque, addr, val&  0xff);
-cirrus_linear_bitblt_writeb(opaque, addr + 1, (val>>  8)&  0xff);
-}
-
-static void cirrus_linear_bitblt_writel(void *opaque, target_phys_addr_t addr,
-uint32_t val)
-{
-cirrus_linear_bitblt_writeb(opaque, addr, val&  0xff);
-cirrus_linear_bitblt_writeb(opaque, addr + 1, (val>>  8)&  0xff);
-cirrus_linear_bitblt_writeb(opaque, addr + 2, (val>>  16)&  0xff);
-cirrus_linear_bitblt_writeb(opaque, addr + 3, (val>>  24)&  0xff);
-}
-
-static uint64_t cirrus_linear_bitblt_read(void *opaque,
-  target_phys_addr_t addr,
-  unsigned size)
-{
-CirrusVGAState *s = opaque;
-
-switch (size) {
-case 1: return cirrus_linear_bitblt_readb(s, addr);
-case 2: return cirrus_linear_bitblt_readw(s, addr);
-case 4: return cirrus_linear_bitblt_readl(s, addr);
-default: abort();
-}
-};
-
-static void cirrus_linear_bitblt_write(void *opaque,
-   target_phys_addr_t addr,
-   uint64_t data,
-   unsigned size)
-{
-CirrusVGAState *s = opaque;
-
-switch (size) {
-case 1: return cirrus_linear_bitblt_writeb(s, addr, data);
-case 2: return cirrus_linear_bitblt_writew(s, addr, data);
-case 4: return cirrus_linear_bitblt_writel(s, addr, data);
-default: abort();
-}
-};
-
  static const MemoryRegionOps cirrus_linear_bitblt_io_ops = {
  .read = cirrus_linear_bitblt_read,
  .write = cirrus_linear_bitblt_write,
  .endianness = DEVICE_LITTLE_ENDIAN,
+.impl = {
+.min_access_size = 1,
+.max_access_size = 1,
+},
  };

  #include "exec-memory.h"

Re: [Qemu-devel] [PATCH v3 05/39] cirrus: simplify mmio BAR access functions

[Anthony Liguori]

On 08/04/2011 08:06 AM, Avi Kivity wrote:

Make use of the memory API's ability to satisfy multi-byte accesses via
multiple single-byte accesses.

Reviewed-by: Richard Henderson
Signed-off-by: Avi Kivity



-static uint64_t cirrus_mmio_read(void *opaque, target_phys_addr_t addr,
- unsigned size)
-{
-CirrusVGAState *s = opaque;
-
-switch (size) {
-case 1: return cirrus_mmio_readb(s, addr);
-case 2: return cirrus_mmio_readw(s, addr);
-case 4: return cirrus_mmio_readl(s, addr);
-default: abort();
-}
-};
-
-static void cirrus_mmio_write(void *opaque, target_phys_addr_t addr,
-  uint64_t data, unsigned size)
-{
-CirrusVGAState *s = opaque;
-
-switch (size) {
-case 1: return cirrus_mmio_writeb(s, addr, data);
-case 2: return cirrus_mmio_writew(s, addr, data);
-case 4: return cirrus_mmio_writel(s, addr, data);
-default: abort();
-}
-};
-
  static const MemoryRegionOps cirrus_mmio_io_ops = {
  .read = cirrus_mmio_read,
  .write = cirrus_mmio_write,
  .endianness = DEVICE_LITTLE_ENDIAN,
+.impl = {
+.min_access_size = 1,
+.max_access_size = 1,
+},
  };


Nevermind, I get the lack of old_mmio usage now.

Regards,

Anthony Liguori


  /* load/save state */

Re: [Qemu-devel] [PATCH v3 05/39] cirrus: simplify mmio BAR access functions

[Anthony Liguori]

On 08/04/2011 08:06 AM, Avi Kivity wrote:

Make use of the memory API's ability to satisfy multi-byte accesses via
multiple single-byte accesses.

Reviewed-by: Richard Henderson
Signed-off-by: Avi Kivity
---
  hw/cirrus_vga.c |   78 +-
  1 files changed, 8 insertions(+), 70 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index d1475dd..6e1aa75 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -2828,12 +2828,11 @@ static void cirrus_vga_ioport_write(void *opaque, 
uint32_t addr, uint32_t val)
   *
   ***/

-static uint32_t cirrus_mmio_readb(void *opaque, target_phys_addr_t addr)
+static uint64_t cirrus_mmio_read(void *opaque, target_phys_addr_t addr,
+ unsigned size)
  {
  CirrusVGAState *s = opaque;

-addr&= CIRRUS_PNPMMIO_SIZE - 1;
-
  if (addr>= 0x100) {
  return cirrus_mmio_blt_read(s, addr - 0x100);
  } else {
@@ -2841,33 +2840,11 @@ static uint32_t cirrus_mmio_readb(void *opaque, 
target_phys_addr_t addr)
  }
  }

-static uint32_t cirrus_mmio_readw(void *opaque, target_phys_addr_t addr)
-{
-uint32_t v;
-
-v = cirrus_mmio_readb(opaque, addr);
-v |= cirrus_mmio_readb(opaque, addr + 1)<<  8;
-return v;
-}
-
-static uint32_t cirrus_mmio_readl(void *opaque, target_phys_addr_t addr)
-{
-uint32_t v;
-
-v = cirrus_mmio_readb(opaque, addr);
-v |= cirrus_mmio_readb(opaque, addr + 1)<<  8;
-v |= cirrus_mmio_readb(opaque, addr + 2)<<  16;
-v |= cirrus_mmio_readb(opaque, addr + 3)<<  24;
-return v;
-}
-
-static void cirrus_mmio_writeb(void *opaque, target_phys_addr_t addr,
-  uint32_t val)
+static void cirrus_mmio_write(void *opaque, target_phys_addr_t addr,
+  uint64_t val, unsigned size)
  {
  CirrusVGAState *s = opaque;

-addr&= CIRRUS_PNPMMIO_SIZE - 1;
-
  if (addr>= 0x100) {
cirrus_mmio_blt_write(s, addr - 0x100, val);
  } else {
@@ -2875,53 +2852,14 @@ static void cirrus_mmio_writeb(void *opaque, 
target_phys_addr_t addr,
  }
  }

-static void cirrus_mmio_writew(void *opaque, target_phys_addr_t addr,
-  uint32_t val)
-{
-cirrus_mmio_writeb(opaque, addr, val&  0xff);
-cirrus_mmio_writeb(opaque, addr + 1, (val>>  8)&  0xff);
-}
-
-static void cirrus_mmio_writel(void *opaque, target_phys_addr_t addr,
-  uint32_t val)
-{
-cirrus_mmio_writeb(opaque, addr, val&  0xff);
-cirrus_mmio_writeb(opaque, addr + 1, (val>>  8)&  0xff);
-cirrus_mmio_writeb(opaque, addr + 2, (val>>  16)&  0xff);
-cirrus_mmio_writeb(opaque, addr + 3, (val>>  24)&  0xff);
-}
-
-
-static uint64_t cirrus_mmio_read(void *opaque, target_phys_addr_t addr,
- unsigned size)
-{
-CirrusVGAState *s = opaque;
-
-switch (size) {
-case 1: return cirrus_mmio_readb(s, addr);
-case 2: return cirrus_mmio_readw(s, addr);
-case 4: return cirrus_mmio_readl(s, addr);
-default: abort();
-}
-};
-
-static void cirrus_mmio_write(void *opaque, target_phys_addr_t addr,
-  uint64_t data, unsigned size)
-{
-CirrusVGAState *s = opaque;
-
-switch (size) {
-case 1: return cirrus_mmio_writeb(s, addr, data);
-case 2: return cirrus_mmio_writew(s, addr, data);
-case 4: return cirrus_mmio_writel(s, addr, data);
-default: abort();
-}
-};
-
  static const MemoryRegionOps cirrus_mmio_io_ops = {
  .read = cirrus_mmio_read,
  .write = cirrus_mmio_write,
  .endianness = DEVICE_LITTLE_ENDIAN,
+.impl = {
+.min_access_size = 1,
+.max_access_size = 1,
+},
  };


old_mmio?

Regards,

Anthony Liguori


  /* load/save state */

Re: [Qemu-devel] [PATCH v3 04/39] vga: convert vga and its derivatives to the memory API

[Anthony Liguori]

On 08/04/2011 08:06 AM, Avi Kivity wrote:

Convert all vga memory to the memory API.  Note we need to fall back to
get_system_memory(), since the various buses don't pass the vga window
as a memory region.

We no longer need to sync the dirty bitmap of the cirrus mapped memory
banks, since the memory API takes care of that for us.

[jan: fix vga-pci logging]

Reviewed-by: Richard Henderson
Signed-off-by: Avi Kivity
---
  hw/cirrus_vga.c |  343 --
  hw/qxl-render.c |2 +-
  hw/qxl.c|  135 --
  hw/qxl.h|6 +-
  hw/vga-isa-mm.c |   45 +---
  hw/vga-isa.c|   11 +-
  hw/vga-pci.c|   28 +
  hw/vga.c|  147 +++-
  hw/vga_int.h|   14 +--
  hw/vmware_vga.c |  143 ---
  10 files changed, 440 insertions(+), 434 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index f39d1f8..d1475dd 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -200,9 +200,14 @@ typedef void (*cirrus_fill_t)(struct CirrusVGAState *s,
  typedef struct CirrusVGAState {
  VGACommonState vga;

-int cirrus_linear_io_addr;
-int cirrus_linear_bitblt_io_addr;
-int cirrus_mmio_io_addr;
+MemoryRegion cirrus_linear_io;
+MemoryRegion cirrus_linear_bitblt_io;
+MemoryRegion cirrus_mmio_io;
+MemoryRegion pci_bar;
+bool linear_vram;  /* vga.vram mapped over cirrus_linear_io */
+MemoryRegion low_mem_container; /* container for 0xa-0xc */
+MemoryRegion low_mem;   /* always mapped, overridden by: */
+MemoryRegion *cirrus_bank[2];   /*   aliases at 0xa-0xb  */
  uint32_t cirrus_addr_mask;
  uint32_t linear_mmio_mask;
  uint8_t cirrus_shadow_gr0;
@@ -612,7 +617,7 @@ static void cirrus_invalidate_region(CirrusVGAState * s, 
int off_begin,
off_cur_end = (off_cur + bytesperline)&  s->cirrus_addr_mask;
off_cur&= TARGET_PAGE_MASK;
while (off_cur<  off_cur_end) {
-   cpu_physical_memory_set_dirty(s->vga.vram_offset + off_cur);
+   memory_region_set_dirty(&s->vga.vram, off_cur);
off_cur += TARGET_PAGE_SIZE;
}
off_begin += off_pitch;
@@ -1177,12 +1182,6 @@ static void cirrus_update_bank_ptr(CirrusVGAState * s, 
unsigned bank_index)
  }

  if (limit>  0) {
-/* Thinking about changing bank base? First, drop the dirty bitmap 
information
- * on the current location, otherwise we lose this pointer forever */
-if (s->vga.lfb_vram_mapped) {
-target_phys_addr_t base_addr = isa_mem_base + 0xa + bank_index 
* 0x8000;
-cpu_physical_sync_dirty_bitmap(base_addr, base_addr + 0x8000);
-}
s->cirrus_bank_base[bank_index] = offset;
s->cirrus_bank_limit[bank_index] = limit;
  } else {
@@ -1921,8 +1920,8 @@ static void 
cirrus_mem_writeb_mode4and5_8bpp(CirrusVGAState * s,
val<<= 1;
dst++;
  }
-cpu_physical_memory_set_dirty(s->vga.vram_offset + offset);
-cpu_physical_memory_set_dirty(s->vga.vram_offset + offset + 7);
+memory_region_set_dirty(&s->vga.vram, offset);
+memory_region_set_dirty(&s->vga.vram, offset + 7);
  }

  static void cirrus_mem_writeb_mode4and5_16bpp(CirrusVGAState * s,
@@ -1946,8 +1945,8 @@ static void 
cirrus_mem_writeb_mode4and5_16bpp(CirrusVGAState * s,
val<<= 1;
dst += 2;
  }
-cpu_physical_memory_set_dirty(s->vga.vram_offset + offset);
-cpu_physical_memory_set_dirty(s->vga.vram_offset + offset + 15);
+memory_region_set_dirty(&s->vga.vram, offset);
+memory_region_set_dirty(&s->vga.vram, offset + 15);
  }

  /***
@@ -2057,8 +2056,7 @@ static void cirrus_vga_mem_writeb(void *opaque, 
target_phys_addr_t addr,
mode = s->vga.gr[0x05]&  0x7;
if (mode<  4 || mode>  5 || ((s->vga.gr[0x0B]&  0x4) == 0)) {
*(s->vga.vram_ptr + bank_offset) = mem_value;
-   cpu_physical_memory_set_dirty(s->vga.vram_offset +
- bank_offset);
+   memory_region_set_dirty(&s->vga.vram, bank_offset);
} else {
if ((s->vga.gr[0x0B]&  0x14) != 0x14) {
cirrus_mem_writeb_mode4and5_8bpp(s, mode,
@@ -2099,16 +2097,37 @@ static void cirrus_vga_mem_writel(void *opaque, 
target_phys_addr_t addr, uint32_
  cirrus_vga_mem_writeb(opaque, addr + 3, (val>>  24)&  0xff);
  }

-static CPUReadMemoryFunc * const cirrus_vga_mem_read[3] = {
-cirrus_vga_mem_readb,
-cirrus_vga_mem_readw,
-cirrus_vga_mem_readl,
+static uint64_t cirrus_vga_mem_read(void *opaque,
+target_phys_addr_t addr,
+uint32_t size)
+{
+CirrusVGAState *s = opaque;
+
+switch (size) {
+case 1: return cirrus_vga_mem_readb(s, addr);
+case 2: return cirrus_vga_m

Re: [Qemu-devel] [PATCH v3 03/39] vmsvga: don't remember pci BAR address in callback any more

[Anthony Liguori]

On 08/04/2011 08:05 AM, Avi Kivity wrote:

We're going to remove the callback, so we can't use it to save the
address.  Use the pci API instead.

Reviewed-by: Richard Henderson
Signed-off-by: Avi Kivity


Reviewed-by: Anthony Liguori 

Regards,

Anthony Liguori


---
  hw/vmware_vga.c |   12 ++--
  1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index 354c221..190b005 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -52,8 +52,6 @@ struct vmsvga_state_s {
  int on;
  } cursor;

-target_phys_addr_t vram_base;
-
  int index;
  int scratch_size;
  uint32_t *scratch;
@@ -761,8 +759,11 @@ static uint32_t vmsvga_value_read(void *opaque, uint32_t 
address)
  case SVGA_REG_BYTES_PER_LINE:
  return ((s->depth + 7)>>  3) * s->new_width;

-case SVGA_REG_FB_START:
-return s->vram_base;
+case SVGA_REG_FB_START: {
+struct pci_vmsvga_state_s *pci_vmsvga
+= container_of(s, struct pci_vmsvga_state_s, chip);
+return pci_get_bar_addr(&pci_vmsvga->card, 1);
+}

  case SVGA_REG_FB_OFFSET:
  return 0x0;
@@ -1247,14 +1248,13 @@ static void pci_vmsvga_map_mem(PCIDevice *pci_dev, int 
region_num,
  struct vmsvga_state_s *s =&d->chip;
  ram_addr_t iomemtype;

-s->vram_base = addr;
  #ifdef DIRECT_VRAM
  iomemtype = cpu_register_io_memory(vmsvga_vram_read,
  vmsvga_vram_write, s, DEVICE_NATIVE_ENDIAN);
  #else
  iomemtype = s->vga.vram_offset | IO_MEM_RAM;
  #endif
-cpu_register_physical_memory(s->vram_base, s->vga.vram_size,
+cpu_register_physical_memory(addr, s->vga.vram_size,
  iomemtype);

  s->vga.map_addr = addr;

Re: [Qemu-devel] [PATCH v3 02/39] pci: add API to get a BAR's mapped address

[Anthony Liguori]

On 08/04/2011 08:05 AM, Avi Kivity wrote:

This is a hack, for devices that have a back-channel to read this
address back outside the normal configuration mechanisms, such
as VMware svga.

Reviewed-by: Richard Henderson
Signed-off-by: Avi Kivity


Can we add a comment to the header file to this effect?

Michael, if you ever have some free time and are feeling generous, 
adding docs to the pci functions would be quite helpful.


Regards,

Anthony Liguori


---
  hw/pci.c |5 +
  hw/pci.h |1 +
  2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/hw/pci.c b/hw/pci.c
index 36db58b..912f849 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -952,6 +952,11 @@ void pci_register_bar_region(PCIDevice *pci_dev, int 
region_num,
  pci_dev->io_regions[region_num].memory = memory;
  }

+pcibus_t pci_get_bar_addr(PCIDevice *pci_dev, int region_num)
+{
+return pci_dev->io_regions[region_num].addr;
+}
+
  static void pci_bridge_filter(PCIDevice *d, pcibus_t *addr, pcibus_t *size,
uint8_t type)
  {
diff --git a/hw/pci.h b/hw/pci.h
index c51156d..64282ad 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -207,6 +207,7 @@ void pci_register_bar_simple(PCIDevice *pci_dev, int 
region_num,
   pcibus_t size, uint8_t attr, ram_addr_t 
ram_addr);
  void pci_register_bar_region(PCIDevice *pci_dev, int region_num,
   uint8_t attr, MemoryRegion *memory);
+pcibus_t pci_get_bar_addr(PCIDevice *pci_dev, int region_num);

  int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
 uint8_t offset, uint8_t size);

Re: [Qemu-devel] [PATCH v3 01/39] virtio-pci: get config on init

[Anthony Liguori]

On 08/04/2011 08:05 AM, Avi Kivity wrote:

From: "Michael S. Tsirkin"

We originally did get config on map, so that
following write accesses are done on an updated config.
New memory API doesn't give us a callback
on map, and arguably, devices don't know when
cpu really can access there. So updating on
init seems cleaner.

Signed-off-by: Michael S. Tsirkin
Signed-off-by: Avi Kivity
---
  hw/virtio-pci.c |7 ---
  1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index d685243..ca1f12f 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -506,9 +506,6 @@ static void virtio_map(PCIDevice *pci_dev, int region_num,
  register_ioport_read(addr, config_len, 1, virtio_pci_config_readb, proxy);
  register_ioport_read(addr, config_len, 2, virtio_pci_config_readw, proxy);
  register_ioport_read(addr, config_len, 4, virtio_pci_config_readl, proxy);
-
-if (vdev->config_len)
-vdev->get_config(vdev, vdev->config);
  }

  static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
@@ -689,6 +686,10 @@ void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice 
*vdev)
  proxy->host_features |= 0x1<<  VIRTIO_F_NOTIFY_ON_EMPTY;
  proxy->host_features |= 0x1<<  VIRTIO_F_BAD_FEATURE;
  proxy->host_features = vdev->get_features(vdev, proxy->host_features);
+
+if (vdev->config_len) {
+vdev->get_config(vdev, vdev->config);
+}


Thinking more closely, I don't think this right.

Updating on map ensured that the config was refreshed after each time 
the bar was mapped.  In the very least, the config needs to be refreshed 
during reset because the guest may write to the guest space which should 
get cleared after reset.


Regards,

Anthony Liguori


  }

  static int virtio_blk_init_pci(PCIDevice *pci_dev)

Re: [Qemu-devel] [PATCH] target-arm: upgrade for secondary cpu bootloader

[Evgeny Voevodin]

From 4c843ce18a7534e722670ecb892a2dedd3eae5b3 Mon Sep 17 00:00:00 2001
From: Evgeny Voevodin 
Date: Fri, 5 Aug 2011 17:24:48 +0400
Subject: [PATCH 1/1] target-arm: Add new secondary CPU bootloader

Secondary CPU bootloader enables interrupt and issues wfi until start 
address

is written to system controller. The position where to find this start
address is hardcoded to 0x1030. This commit adds new bootloader for
secondary CPU which allows a target board to cpecify a position where
to find start address.

Signed-off-by: Evgeny Voevodin 
Signed-off-by: Kyungmin Park 
---
 hw/arm-misc.h |1 +
 hw/arm_boot.c |   18 +++---
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/hw/arm-misc.h b/hw/arm-misc.h
index 010acb4..ef1d61d 100644
--- a/hw/arm-misc.h
+++ b/hw/arm-misc.h
@@ -28,6 +28,7 @@ struct arm_boot_info {
 const char *initrd_filename;
 target_phys_addr_t loader_start;
 target_phys_addr_t smp_loader_start;
+target_phys_addr_t smp_startaddr;
 target_phys_addr_t smp_priv_base;
 int nb_cpus;
 int board_id;
diff --git a/hw/arm_boot.c b/hw/arm_boot.c
index 620550b..4557bbd 100644
--- a/hw/arm_boot.c
+++ b/hw/arm_boot.c
@@ -31,17 +31,17 @@ static uint32_t bootloader[] = {
 /* Entry point for secondary CPUs.  Enable interrupt controller and
Issue WFI until start address is written to system controller.  */
 static uint32_t smpboot[] = {
-  0xe59f0020, /* ldr r0, privbase */
-  0xe3a01001, /* mov r1, #1 */
-  0xe5801100, /* str r1, [r0, #0x100] */
-  0xe3a00201, /* mov r0, #0x1000 */
-  0xe3800030, /* orr r0, #0x30 */
+  0xe59f201c, /* ldr r2, privbase */
+  0xe59f001c, /* ldr r0, startaddr */
+  0xe3a01001, /* mov r1, #1 */
+  0xe5821100, /* str r1, [r2, #256] */
   0xe320f003, /* wfi */
   0xe5901000, /* ldr r1, [r0] */
   0xe1110001, /* tst r1, r1 */
   0x0afb, /* beq  */
   0xe12fff11, /* bx  r1 */
-  0 /* privbase: Private memory region base address.  */
+  0,  /* privbase: Private memory region base address.  */
+  0   /* startaddr: Where to find start address */
 };

 #define WRITE_WORD(p, value) do { \
@@ -269,7 +269,11 @@ void arm_load_kernel(CPUState *env, struct 
arm_boot_info *info)

 rom_add_blob_fixed("bootloader", bootloader, sizeof(bootloader),
info->loader_start);
 if (info->nb_cpus > 1) {
-smpboot[10] = info->smp_priv_base;
+if (!info->smp_startaddr) {
+info->smp_startaddr = 0x1030;
+}
+smpboot[(sizeof(smpboot) - 8)/4] = info->smp_priv_base;
+smpboot[(sizeof(smpboot) - 4)/4] = info->smp_startaddr;
 for (n = 0; n < sizeof(smpboot) / 4; n++) {
 smpboot[n] = tswap32(smpboot[n]);
 }
--
1.7.4.1

[Qemu-devel] [PATCH 1/3] slirp: Fix types of IP address parameters

[Jan Kiszka]

Should be uint32_t for IPv4, not int. Also avoid in_addr_t without
proper includes. Fixes build regression on mingw32.

Signed-off-by: Jan Kiszka 
---
 slirp/arp_table.c |   16 
 slirp/slirp.h |4 ++--
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/slirp/arp_table.c b/slirp/arp_table.c
index 820dee2..5d7b8ac 100644
--- a/slirp/arp_table.c
+++ b/slirp/arp_table.c
@@ -24,9 +24,9 @@
 
 #include "slirp.h"
 
-void arp_table_add(Slirp *slirp, int ip_addr, uint8_t ethaddr[ETH_ALEN])
+void arp_table_add(Slirp *slirp, uint32_t ip_addr, uint8_t ethaddr[ETH_ALEN])
 {
-const in_addr_t broadcast_addr =
+const uint32_t broadcast_addr =
 ~slirp->vnetwork_mask.s_addr | slirp->vnetwork_addr.s_addr;
 ArpTable *arptbl = &slirp->arp_table;
 int i;
@@ -60,29 +60,29 @@ void arp_table_add(Slirp *slirp, int ip_addr, uint8_t 
ethaddr[ETH_ALEN])
 arptbl->next_victim = (arptbl->next_victim + 1) % ARP_TABLE_SIZE;
 }
 
-bool arp_table_search(Slirp *slirp, int in_ip_addr,
+bool arp_table_search(Slirp *slirp, uint32_t ip_addr,
   uint8_t out_ethaddr[ETH_ALEN])
 {
-const in_addr_t broadcast_addr =
+const uint32_t broadcast_addr =
 ~slirp->vnetwork_mask.s_addr | slirp->vnetwork_addr.s_addr;
 ArpTable *arptbl = &slirp->arp_table;
 int i;
 
 DEBUG_CALL("arp_table_search");
-DEBUG_ARG("ip = 0x%x", in_ip_addr);
+DEBUG_ARG("ip = 0x%x", ip_addr);
 
 /* Check 0.0.0.0/8 invalid source-only addresses */
-assert((in_ip_addr & htonl(~(0xf << 28))) != 0);
+assert((ip_addr & htonl(~(0xf << 28))) != 0);
 
 /* If broadcast address */
-if (in_ip_addr == 0x || in_ip_addr == broadcast_addr) {
+if (ip_addr == 0x || ip_addr == broadcast_addr) {
 /* return Ethernet broadcast address */
 memset(out_ethaddr, 0xff, ETH_ALEN);
 return 1;
 }
 
 for (i = 0; i < ARP_TABLE_SIZE; i++) {
-if (arptbl->table[i].ar_sip == in_ip_addr) {
+if (arptbl->table[i].ar_sip == ip_addr) {
 memcpy(out_ethaddr, arptbl->table[i].ar_sha,  ETH_ALEN);
 DEBUG_ARGS((dfd, " found hw addr = 
%02x:%02x:%02x:%02x:%02x:%02x\n",
 out_ethaddr[0], out_ethaddr[1], out_ethaddr[2],
diff --git a/slirp/slirp.h b/slirp/slirp.h
index 2a070e6..dcf99d5 100644
--- a/slirp/slirp.h
+++ b/slirp/slirp.h
@@ -208,9 +208,9 @@ typedef struct ArpTable {
 int next_victim;
 } ArpTable;
 
-void arp_table_add(Slirp *slirp, int ip_addr, uint8_t ethaddr[ETH_ALEN]);
+void arp_table_add(Slirp *slirp, uint32_t ip_addr, uint8_t ethaddr[ETH_ALEN]);
 
-bool arp_table_search(Slirp *slirp, int in_ip_addr,
+bool arp_table_search(Slirp *slirp, uint32_t ip_addr,
   uint8_t out_ethaddr[ETH_ALEN]);
 
 struct Slirp {
-- 
1.7.3.4

[Qemu-devel] [PATCH 0/3] [PULL] slirp: ARP table related fixes

[Jan Kiszka]

The following changes since commit 81e34a2401f7ffd519bb7f093e833cb48734169f:

  Merge remote-tracking branch 'mst/for_anthony' into staging (2011-08-04 
17:15:22 -0500)

are available in the git repository at:

  git://git.kiszka.org/qemu.git queues/slirp

Fixes some fall-outs from the latest merge, ie. a build breakage on
mingw and a regression that broke at least ping.


CC: Gerd Hoffmann 

Jan Kiszka (3):
  slirp: Fix types of IP address parameters
  slirp: Read current time only once per if_start call
  slirp: Only start packet expiration for delayed ones

 slirp/arp_table.c |   16 
 slirp/if.c|8 +---
 slirp/slirp.c |3 +++
 slirp/slirp.h |4 ++--
 4 files changed, 14 insertions(+), 17 deletions(-)

-- 
1.7.3.4

[Qemu-devel] [PATCH 2/3] slirp: Read current time only once per if_start call

[Jan Kiszka]

No need to update the current time for each packet we send from the
queue. Processing time is comparably short.

Signed-off-by: Jan Kiszka 
---
 slirp/if.c |5 +
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/slirp/if.c b/slirp/if.c
index 2d79e45..47bebe4 100644
--- a/slirp/if.c
+++ b/slirp/if.c
@@ -157,9 +157,8 @@ diddit:
 void
 if_start(Slirp *slirp)
 {
+uint64_t now = qemu_get_clock_ns(rt_clock);
 int requeued = 0;
-uint64_t now;
-
struct mbuf *ifm, *ifqt;
 
DEBUG_CALL("if_start");
@@ -172,8 +171,6 @@ if_start(Slirp *slirp)
 if (!slirp_can_output(slirp->opaque))
 return;
 
-now = qemu_get_clock_ns(rt_clock);
-
/*
 * See which queue to get next packet from
 * If there's something in the fastq, select it immediately
-- 
1.7.3.4

[Qemu-devel] [PATCH 3/3] slirp: Only start packet expiration for delayed ones

[Jan Kiszka]

The expiration timeout must only affect packets that are queued due to
pending ARP resolutions. The old version broke ping e.g.

Signed-off-by: Jan Kiszka 
---
 slirp/if.c|3 ---
 slirp/slirp.c |3 +++
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/slirp/if.c b/slirp/if.c
index 47bebe4..2852396 100644
--- a/slirp/if.c
+++ b/slirp/if.c
@@ -106,9 +106,6 @@ if_output(struct socket *so, struct mbuf *ifm)
ifs_init(ifm);
insque(ifm, ifq);
 
-/* Expiration date = Now + 1 second */
-ifm->expiration_date = qemu_get_clock_ns(rt_clock) + 10ULL;
-
 diddit:
slirp->if_queued++;
 
diff --git a/slirp/slirp.c b/slirp/slirp.c
index a86cc6e..2c242ef 100644
--- a/slirp/slirp.c
+++ b/slirp/slirp.c
@@ -738,6 +738,9 @@ int if_encap(Slirp *slirp, struct mbuf *ifm)
 slirp->client_ipaddr = iph->ip_dst;
 slirp_output(slirp->opaque, arp_req, sizeof(arp_req));
 ifm->arp_requested = true;
+
+/* Expire request and drop outgoing packet after 1 second */
+ifm->expiration_date = qemu_get_clock_ns(rt_clock) + 10ULL;
 }
 return 0;
 } else {
-- 
1.7.3.4

Re: [Qemu-devel] [PATCH V2] Add "tee" option to qemu char device

[Anthony Liguori]

On 08/04/2011 09:26 PM, Chun Yan Liu wrote:

Alex&  Anthony,

About this issue, how should we proceed?


Do the logging somewhere else in the Xen toolchain.

Regards,

Anthony Liguori

Re: [Qemu-devel] qcow2x

[Kevin Wolf]

Am 05.08.2011 15:16, schrieb Frediano Ziglio:
> 2011/8/5 Kevin Wolf :
>> Am 03.08.2011 15:39, schrieb Frediano Ziglio:
>>> 2011/8/2 Kevin Wolf :
 Am 02.08.2011 17:29, schrieb Frediano Ziglio:
>>> - L2 allocation can be done with relative data (this is not easy to do
>>> with current code)
>>
>> What do you mean by that?
>>
>
> Let's take an example. By allocation I mean give a position to
> data/l2/refcount_table. Usually you cannot update/write L2 before data
> is allocated and written if you don't want to get a L2 entry pointing
> to garbage or unwritten data (if physically you write to a sector you
> get new data or old one on fail, not data changing to anything else).
> The exception to this is when even L2 table is not allocated. In this
> case you can write L2 table with data cause in case of failure this
> new L2 table is just not attached to anything (cause L1 still point to
> old L2 or is not allocated). My patch can collapse these two cluster
> writes in a single one. The key point of the patch is mainly
> collapsing all writes as you can not blocking other writes if not
> needed.

 Ok, I see.

 Not sure if it makes any measurable difference. With 64k clusters, an L2
 allocation happens only every 512 MB. But if it's not much code to
 support it, it's probably a good thing.

>>>
>>> Let's see :)
>>>
>>> I published a repo at gitorious. https://gitorious.org/qemu (yes, qemu
>>> was free for me :) )
>>
>> Pretty radical changes. :-)
>>
>> I didn't really read most of the code (yet), but I gave it a try and ran
>> an iozone benchmark with it. It seems to degrade cluster allocation
>> performance to about the level of qcow2 in 0.12.4.
>>
>> The numbers say that you get about 40% of git master throughput on large
>> block sizes (256k), and 65% of git master throughput on small ones (8k).
>> So you may really have triggered some special cases that only apply for
>> qemu-io/qemu-img.
>>
>> Kevin
>>
> 
> I'll try iozone too. I have still to try real tests. It seems that
> many performance boost I can get with qcow2x can be implemented
> without changing so much code in qcow2. Perhaps I posted my code too
> much earlier. Which iozone tests are you running?

The script I use creates a new file system for /mnt and runs this:

./iozone -f /mnt/test_8k -w -e -I -i0 -i1 -i2 -K -r 8k -s 64M
./iozone -f /mnt/test_256k -w -e -I -i0 -i1 -i2 -K -r 256k -s 1G

This is actually more than you really need. The interesting number for
measuring cluster allocation performance is sequential writes.

Kevin

Re: [Qemu-devel] qcow2x

[Frediano Ziglio]

2011/8/5 Kevin Wolf :
> Am 03.08.2011 15:39, schrieb Frediano Ziglio:
>> 2011/8/2 Kevin Wolf :
>>> Am 02.08.2011 17:29, schrieb Frediano Ziglio:
>> - L2 allocation can be done with relative data (this is not easy to do
>> with current code)
>
> What do you mean by that?
>

 Let's take an example. By allocation I mean give a position to
 data/l2/refcount_table. Usually you cannot update/write L2 before data
 is allocated and written if you don't want to get a L2 entry pointing
 to garbage or unwritten data (if physically you write to a sector you
 get new data or old one on fail, not data changing to anything else).
 The exception to this is when even L2 table is not allocated. In this
 case you can write L2 table with data cause in case of failure this
 new L2 table is just not attached to anything (cause L1 still point to
 old L2 or is not allocated). My patch can collapse these two cluster
 writes in a single one. The key point of the patch is mainly
 collapsing all writes as you can not blocking other writes if not
 needed.
>>>
>>> Ok, I see.
>>>
>>> Not sure if it makes any measurable difference. With 64k clusters, an L2
>>> allocation happens only every 512 MB. But if it's not much code to
>>> support it, it's probably a good thing.
>>>
>>
>> Let's see :)
>>
>> I published a repo at gitorious. https://gitorious.org/qemu (yes, qemu
>> was free for me :) )
>
> Pretty radical changes. :-)
>
> I didn't really read most of the code (yet), but I gave it a try and ran
> an iozone benchmark with it. It seems to degrade cluster allocation
> performance to about the level of qcow2 in 0.12.4.
>
> The numbers say that you get about 40% of git master throughput on large
> block sizes (256k), and 65% of git master throughput on small ones (8k).
> So you may really have triggered some special cases that only apply for
> qemu-io/qemu-img.
>
> Kevin
>

I'll try iozone too. I have still to try real tests. It seems that
many performance boost I can get with qcow2x can be implemented
without changing so much code in qcow2. Perhaps I posted my code too
much earlier. Which iozone tests are you running?

Frediano

Re: [Qemu-devel] Safely reopening image files by stashing fds

[Daniel P. Berrange]

On Fri, Aug 05, 2011 at 10:27:00AM +0100, Stefan Hajnoczi wrote:
> On Fri, Aug 5, 2011 at 10:04 AM, Paolo Bonzini  wrote:
> > On 08/05/2011 10:40 AM, Stefan Hajnoczi wrote:
> >>
> >> 4. Implement bdrv_reopen()
> >>
> >> First call bdrv_extract_fds() to stash the file descriptors, then close
> >> the
> >> block device.  Try opening the new image but if that fails, reopen using
> >> the
> >> stashed file descriptors.
> >
> > Why not do the latter unconditionally?
> 
> Because you cannot change O_DIRECT on an open fd :(.  This is why
> we're going through this pain.

Hmm, I remember hearing that before, but looking at the current fcntl()
manpage, it claims you *can* change O_DIRECT using SET_FL. Perhaps this
is a newish feature, but it'd be nicer to use it if possible ?

  [man 2 fcntl]
   F_SETFL (long)
  Set  the  file status flags to the value specified by arg.  File
  access mode (O_RDONLY, O_WRONLY, O_RDWR) and file creation flags
  (i.e.,  O_CREAT,  O_EXCL, O_NOCTTY, O_TRUNC) in arg are ignored.
  On Linux this command can only  change  the  O_APPEND,  O_ASYNC,
  O_DIRECT, O_NOATIME, and O_NONBLOCK flags.
  [/man]

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

Re: [Qemu-devel] [PATCH] target-arm: upgrade for secondary cpu bootloader

[Peter Maydell]

On 5 August 2011 13:10, Evgeny Voevodin  wrote:
> On 08/05/2011 03:35 PM, Peter Maydell wrote:
>> What's the motivation for this? (ie what target board needs this?)
>
> The motivation is that Samsung s5pc210 board which is under development
> needs this. Also, I believe that more boards will need it in future.

Cool. Are you planning to submit this board model upstream? (on
what timescale?)

thanks
-- PMM

Re: [Qemu-devel] Safely reopening image files by stashing fds

[Stefan Hajnoczi]

On Fri, Aug 5, 2011 at 10:55 AM, Paolo Bonzini  wrote:
> On 08/05/2011 11:27 AM, Stefan Hajnoczi wrote:

 First call bdrv_extract_fds() to stash the file descriptors, then close
 the block device.  Try opening the new image but if that fails, reopen
 using
 the stashed file descriptors.
>>>
>>> Why not do the latter unconditionally?
>>
>> Because you cannot change O_DIRECT on an open fd:(.  This is why
>> we're going through this pain.
>>
>> The only method I've found that works is to open("/proc/self/fd/X",
>> new_flags) but that's non-portable.
>
> Maybe I'm missing something obvious, but so is O_DIRECT, no? :)

http://www.freebsd.org/cgi/man.cgi?query=open&apropos=0&sektion=0&manpath=FreeBSD+8.1-RELEASE&format=html

> So for Linux you can dup the stashed file descriptors using /proc/self/fd
> and change flags directly, and for other OSes you can dup them using dup2
> and change flags with F_SETFL.  In any case, reopening can always be done
> using the stashed descriptors (or BlockDriverStates).

F_SETFL doesn't let you change arbitrary flags.

Stefan

Re: [Qemu-devel] [PATCH 2/2] i_generation / st_gen support for handle based fs driver

[Stefan Hajnoczi]

On Fri, Aug 5, 2011 at 12:32 PM, Aneesh Kumar K.V
 wrote:
> On Fri, 5 Aug 2011 10:24:42 +0100, Stefan Hajnoczi  wrote:
>> On Fri, Aug 5, 2011 at 7:40 AM, Aneesh Kumar K.V
>>  wrote:
>> > On Thu, 4 Aug 2011 22:57:34 +0100, Stefan Hajnoczi  
>> > wrote:
>> >> On Thu, Aug 4, 2011 at 7:45 PM, Aneesh Kumar K.V
>> >>  wrote:
>> >> > On Thu, 4 Aug 2011 15:31:08 +0100, Stefan Hajnoczi  
>> >> > wrote:
>> >> >> On Thu, Aug 4, 2011 at 1:03 PM, Aneesh Kumar K.V
>> >> >>  wrote:
>> >> >> > On Thu, 4 Aug 2011 12:47:42 +0100, Stefan Hajnoczi 
>> >> >> >  wrote:
>> >> >> >> On Thu, Aug 4, 2011 at 12:20 PM, Aneesh Kumar K.V
>> >> >> >>  wrote:
>> >> >> >> > On Thu, 4 Aug 2011 11:21:05 +0100, Stefan Hajnoczi 
>> >> >> >> >  wrote:
>> >> >> >> >> On Thu, Aug 4, 2011 at 11:06 AM, Harsh Prateek Bora
>> >> >> >> >>  wrote:
>> >> >> >> >> > This patch provides support for st_gen for handle based fs 
>> >> >> >> >> > type server.
>> >> >> >> >> > Currently the support is provided for ext4, btrfs, reiserfs 
>> >> >> >> >> > and xfs.
>> >> >> >> >> >
>> >> >> >> >> > Signed-off-by: Harsh Prateek Bora 
>> >> >> >> >> > ---
>> >> >> >> >> >  hw/9pfs/virtio-9p-handle.c |   30 
>> >> >> >> >> > ++
>> >> >> >> >> >  1 files changed, 30 insertions(+), 0 deletions(-)
>> >> >> >> >>
>> >> >> >> >> Does handle-based file I/O really need to duplicate all this 
>> >> >> >> >> code?  Is
>> >> >> >> >> it possible to use either regular open or handle-based open from 
>> >> >> >> >> a
>> >> >> >> >> single local fs codebase?
>> >> >> >> >
>> >> >> >> > The only details common between handle based and local based 
>> >> >> >> > getversion
>> >> >> >> > callback is the ioctl. Moving that into a helper may not really 
>> >> >> >> > help in
>> >> >> >> > this case ?.
>> >> >> >>
>> >> >> >> Aneesh, do you have a public virtfs tree that I can look at?  In
>> >> >> >> qemu.git we don't have virtio-9p-handle.c yet, so I can't give any
>> >> >> >> specific feedback.
>> >> >> >
>> >> >> > http://repo.or.cz/w/qemu/v9fs.git for-upstream
>> >> >> >
>> >> >> > I should send the patchset to qemu list soon. Was waiting for the
>> >> >> > co-routine patches to go upstream.
>> >> >>
>> >> >> The handle code looks like a copy of the local backend minus security
>> >> >> models.  It just needs to use handle syscalls instead of using paths.
>> >> >>
>> >> >> If you treat the path as the "handle" and use regular openat(2), then
>> >> >> the handle code could do what the local backend does today.  Except
>> >> >> compared to the local backend it would not have security models and be
>> >> >> a bit slower due to extra syscalls.
>> >> >>
>> >> >> Is the plan to add security models to the handle backend?  If so, then
>> >> >> handle and local will be equivalent and duplicate code.
>> >> >>
>> >> >
>> >> > handle require root user privileges to run. So security model with
>> >> > handle fs driver doesn't make sense. We added mapped security model to
>> >> > avoid requiring user to run as root.
>> >>
>> >> Does it really require root or is a specific set of capabilities
>> >> enough?
>> >
>> > CAP_DAC_READ_SEARCH  is needed.
>> >
>> >>
>> >> A feature that requires QEMU to run as root has really limited value.
>> >> Unprivileged users cannot use the feature, so ad-hoc QEMU users are
>> >> left behind.  People don't want to deploy production guests as root,
>> >> may not be allowed to, or might find that their management tool
>> >> doesn't support that.  So who will be able to use this feature?
>> >>
>> >
>> > One of the main issue that handle based backend fix is the complexity
>> > involved in handling renames, both on the guest and on the host. I am
>> > also not sure how effective it would be to run the qemu as non root user
>> > when exporting a directory with VirtFS. In the mapped security model the
>> > user credentials with which the files are created are stored in xattr
>> > and that mostly implies host cannot look at the files the same way.
>> >
>> > My understanding is passthrough security model (which require qemu to
>> > run as root) will be used if somebody wants to export a directory on the
>> > host to guest. In my case I use none security model, simply because i
>> > don't want new xattr on the file created and I am ok even the files
>> > get created on the host with the credentials on qemu.
>>
>> With xattrs you have to mount the directory on the host in order to
>> see the same view as the guest.
>
> How will that help ? There is nothing on the host that maps those xattr
> to mode/ownership bits currently. We will have to do something similar to 
> fuse to
> make that work ?

Sorry, what I suggested is not actually possible today.  We only have
a virtio-9p transport in the QEMU 9pfs code, not a TCP transport.  I
meant mount -t 9p on the host - don't access the backing directory
directly, instead mount it using 9p on localhost.

> My understanding was passthrough will be preferred
> option. But i may be mistaken.

If passthrou

Re: [Qemu-devel] qcow2x

[Kevin Wolf]

Am 03.08.2011 15:39, schrieb Frediano Ziglio:
> 2011/8/2 Kevin Wolf :
>> Am 02.08.2011 17:29, schrieb Frediano Ziglio:
> - L2 allocation can be done with relative data (this is not easy to do
> with current code)

 What do you mean by that?

>>>
>>> Let's take an example. By allocation I mean give a position to
>>> data/l2/refcount_table. Usually you cannot update/write L2 before data
>>> is allocated and written if you don't want to get a L2 entry pointing
>>> to garbage or unwritten data (if physically you write to a sector you
>>> get new data or old one on fail, not data changing to anything else).
>>> The exception to this is when even L2 table is not allocated. In this
>>> case you can write L2 table with data cause in case of failure this
>>> new L2 table is just not attached to anything (cause L1 still point to
>>> old L2 or is not allocated). My patch can collapse these two cluster
>>> writes in a single one. The key point of the patch is mainly
>>> collapsing all writes as you can not blocking other writes if not
>>> needed.
>>
>> Ok, I see.
>>
>> Not sure if it makes any measurable difference. With 64k clusters, an L2
>> allocation happens only every 512 MB. But if it's not much code to
>> support it, it's probably a good thing.
>>
> 
> Let's see :)
> 
> I published a repo at gitorious. https://gitorious.org/qemu (yes, qemu
> was free for me :) )

Pretty radical changes. :-)

I didn't really read most of the code (yet), but I gave it a try and ran
an iozone benchmark with it. It seems to degrade cluster allocation
performance to about the level of qcow2 in 0.12.4.

The numbers say that you get about 40% of git master throughput on large
block sizes (256k), and 65% of git master throughput on small ones (8k).
So you may really have triggered some special cases that only apply for
qemu-io/qemu-img.

Kevin

Re: [Qemu-devel] [PATCH v3] qcow2: Fix L1 table size after bdrv_snapshot_goto

[Anthony Liguori]

On 08/05/2011 05:06 AM, Kevin Wolf wrote:

When loading an internal snapshot whose L1 table is smaller than the current L1
table, the size of the current L1 would be shrunk to the snapshot's L1 size in
memory, but not on disk. This lead to incorrect refcount updates and eventuelly
to image corruption.

Instead of writing the new L1 size to disk, this simply retains the bigger L1
size that is currently in use and makes sure that the unused part is zeroed.

Signed-off-by: Kevin Wolf
Tested-by: Philipp Hahn


Applied to master and stable-0.15.  Thanks.

Regards,

Anthony Liguori


---
  block/qcow2-snapshot.c |   16 +++-
  1 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 74823a5..e32bcf0 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -317,7 +317,8 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char 
*snapshot_id)
  {
  BDRVQcowState *s = bs->opaque;
  QCowSnapshot *sn;
-int i, snapshot_index, l1_size2;
+int i, snapshot_index;
+int cur_l1_bytes, sn_l1_bytes;

  snapshot_index = find_snapshot_by_id_or_name(bs, snapshot_id);
  if (snapshot_index<  0)
@@ -330,14 +331,19 @@ int qcow2_snapshot_goto(BlockDriverState *bs, const char 
*snapshot_id)
  if (qcow2_grow_l1_table(bs, sn->l1_size, true)<  0)
  goto fail;

-s->l1_size = sn->l1_size;
-l1_size2 = s->l1_size * sizeof(uint64_t);
+cur_l1_bytes = s->l1_size * sizeof(uint64_t);
+sn_l1_bytes = sn->l1_size * sizeof(uint64_t);
+
+if (cur_l1_bytes>  sn_l1_bytes) {
+memset(s->l1_table + sn->l1_size, 0, cur_l1_bytes - sn_l1_bytes);
+}
+
  /* copy the snapshot l1 table to the current l1 table */
  if (bdrv_pread(bs->file, sn->l1_table_offset,
-   s->l1_table, l1_size2) != l1_size2)
+   s->l1_table, sn_l1_bytes)<  0)
  goto fail;
  if (bdrv_pwrite_sync(bs->file, s->l1_table_offset,
-s->l1_table, l1_size2)<  0)
+s->l1_table, cur_l1_bytes)<  0)
  goto fail;
  for(i = 0;i<  s->l1_size; i++) {
  be64_to_cpus(&s->l1_table[i]);

Re: [Qemu-devel] Update to 0.15.0 release schedule

[Anthony Liguori]

On 08/05/2011 06:38 AM, Kevin Wolf wrote:

Am 04.08.2011 23:38, schrieb Anthony Liguori:

Hi,

It took a little longer than I had hoped to converge on a work around
for the migration issue.  -rc2 is out but I don't think 24 hours is
enough time to get proper testing for the final release.

So I'm pushing 0.15.0 to Monday.  Please test and provide feedback.
Unless there's a brown paper bag bug, -rc2 will be the 0.15.0 release.


You know, there was a reason why I CCed you on the qcow2 corruption fix
with [PATCH 0.15.0] in its subject line...


In the future, if you have a patch like this, the right thing to do is 
send a PULL request against stable (with a [STABLE] tag).


Patches should carry a [STABLE] in the subject too.



Though Frediano found another bug in the same function, so the patch
that needs to go into 0.15.0 is now:

[PATCH v3] qcow2: Fix L1 table size after bdrv_snapshot_goto


I'll apply it to master and stable.

Regards,

Anthony Liguori


Kevin