Re: [Qemu-devel] [PATCH for-1.1?] slirp: Avoid redefining MAX_TCPOPTLEN

2012-05-27 Thread Paolo Bonzini
Il 27/05/2012 19:02, Andreas Färber ha scritto:
> MAX_TCPOPTLEN is being defined as 32. Darwin has it as 40, causing a
> warning. The value is only used to declare an array, into which currently
> 4 bytes are written at most. It should therefore be acceptable to adopt
> the host's definition.
> 
> Therefore only define MAX_TCPOPTLEN if not already defined.
> 
> Signed-off-by: Andreas Färber 
> ---
>  slirp/tcp_output.c |2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/slirp/tcp_output.c b/slirp/tcp_output.c
> index 779314b..9815123 100644
> --- a/slirp/tcp_output.c
> +++ b/slirp/tcp_output.c
> @@ -47,7 +47,9 @@ static const u_char  tcp_outflags[TCP_NSTATES] = {
>  };
>  
>  
> +#ifndef MAX_TCPOPTLEN
>  #define MAX_TCPOPTLEN32  /* max # bytes that go in options */
> +#endif
>  
>  /*
>   * Tcp output routine: figure out what should be sent and send it.

Reviewed-by: Paolo Bonzini 

Paolo



Re: [Qemu-devel] [PATCH for-1.1] arch_init: Fix AltiVec build on Darwin/ppc

2012-05-27 Thread Paolo Bonzini
Il 27/05/2012 17:37, Andreas Färber ha scritto:
> GCC's altivec.h may in a !__APPLE_ALTIVEC__ code path redefine bool,
> leading to type mismatches. altivec.h recommends to #undef for C++
> compatibility, but doing so on C leads to bool remaining undefined.

Reviewed-by: Paolo Bonzini 

Paolo



Re: [Qemu-devel] [PATCH 4/4] ISCSI: If the device we open is a SMC device, then force the use of sg. We dont have any medium changer emulation so only passthrough via real sg or scsi-generic via iscsi

2012-05-27 Thread Paolo Bonzini
Il 27/05/2012 15:12, Andreas Färber ha scritto:
>> > Modified to also do the same for tapes, applied to scsi-next branch for 
>> > 1.2.
> Paolo, it seems you haven't pushed scsi-next since then.

Yeah, I have a pending push request for scsi-next, so I'm waiting till
Anthony applies it before pushing 1.2-only patches (I wasn't expecting
parallel 1.1/1.2 development for SCSI).

> I hope you've
> also shortened the subject to a humanly bearable length?

Yes. :)

Paolo



Re: [Qemu-devel] [PATCH qom-next] qom: Introduce object_property_is_{child, link}()

2012-05-27 Thread Paolo Bonzini
Il 27/05/2012 01:19, Andreas Färber ha scritto:
> Avoids hardcoding partial string comparisons.
> 
> Signed-off-by: Alexander Barabash 
> Signed-off-by: Andreas Färber 
> ---
>  qom/object.c |   22 --
>  1 files changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/qom/object.c b/qom/object.c
> index 173835b..00bb3b0 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -305,6 +305,16 @@ void object_initialize(void *data, const char *typename)
>  object_initialize_with_type(data, type);
>  }
>  
> +static inline bool object_property_is_child(ObjectProperty *prop)
> +{
> +return strstart(prop->type, "child<", NULL);
> +}
> +
> +static inline bool object_property_is_link(ObjectProperty *prop)
> +{
> +return strstart(prop->type, "link<", NULL);
> +}
> +
>  static void object_property_del_all(Object *obj)
>  {
>  while (!QTAILQ_EMPTY(&obj->properties)) {
> @@ -327,7 +337,7 @@ static void object_property_del_child(Object *obj, Object 
> *child, Error **errp)
>  ObjectProperty *prop;
>  
>  QTAILQ_FOREACH(prop, &obj->properties, node) {
> -if (strstart(prop->type, "child<", NULL) && prop->opaque == child) {
> +if (object_property_is_child(prop) && prop->opaque == child) {
>  object_property_del(obj, prop->name, errp);
>  break;
>  }
> @@ -600,7 +610,7 @@ int object_child_foreach(Object *obj, int (*fn)(Object 
> *child, void *opaque),
>  int ret = 0;
>  
>  QTAILQ_FOREACH(prop, &obj->properties, node) {
> -if (strstart(prop->type, "child<", NULL)) {
> +if (object_property_is_child(prop)) {
>  ret = fn(prop->opaque, opaque);
>  if (ret != 0) {
>  break;
> @@ -1022,7 +1032,7 @@ gchar *object_get_canonical_path(Object *obj)
>  g_assert(obj->parent != NULL);
>  
>  QTAILQ_FOREACH(prop, &obj->parent->properties, node) {
> -if (!strstart(prop->type, "child<", NULL)) {
> +if (!object_property_is_child(prop)) {
>  continue;
>  }
>  
> @@ -1056,9 +1066,9 @@ Object *object_resolve_path_component(Object *parent, 
> gchar *part)
>  return NULL;
>  }
>  
> -if (strstart(prop->type, "link<", NULL)) {
> +if (object_property_is_link(prop)) {
>  return *(Object **)prop->opaque;
> -} else if (strstart(prop->type, "child<", NULL)) {
> +} else if (object_property_is_child(prop)) {
>  return prop->opaque;
>  } else {
>  return NULL;
> @@ -1101,7 +,7 @@ static Object *object_resolve_partial_path(Object 
> *parent,
>  QTAILQ_FOREACH(prop, &parent->properties, node) {
>  Object *found;
>  
> -if (!strstart(prop->type, "child<", NULL)) {
> +if (!object_property_is_child(prop)) {
>  continue;
>  }
>  

Reviewed-by: Paolo Bonzini 

Paolo



Re: [Qemu-devel] [PULL 1.1 0/2] SCSI patches for 1.1.0-rc3

2012-05-27 Thread Paolo Bonzini
Il 26/05/2012 11:18, ronnie sahlberg ha scritto:
> I have compiled your branch and run through some tests.
> 
> It all looks good as long as you apply the patch to #include "hw/scsi-defs.h"

Thanks, I updated the scsi-next branch.

Paolo




Re: [Qemu-devel] [PATCH 1.1] es1370: Fix debug code

2012-05-27 Thread Stefan Weil

Am 28.05.2012 06:11, schrieb Peter Maydell:

On 23 May 2012 22:26, Stefan Weil  wrote:

When DEBUG_ES1370 is defined, the compiler shows these warnings:

hw/es1370.c: In function ‘es1370_update_voices’:
hw/es1370.c:414: warning: format ‘%d’ expects type ‘int’, but argument 3 has 
type ‘size_t’

The unicode quotes in this commit message seem to have got smashed to
question-mark characters at some point between this email and it hitting
git master :-(

-- PMM


Malc, could you please check your git workflow?

Commit 24f50d7ea5896a30b0e78f68884586bb8b40ff97 and
other commits also changed Andreas Färber to Andreas F?rber.

See also a394aed235d6b3f048eeae83289f4d21eca7023c
and lots more.

The author is always correct, but UTF-8 characters in  the
commit message were replaced by ?.

Regards,
Stefan W.




Re: [Qemu-devel] [PATCH qemu 5/6] implement -no-user-config command-line option (v3)

2012-05-27 Thread Paolo Bonzini
Il 27/05/2012 16:02, Andreas Färber ha scritto:
> Any suggestion how to fix?

Does it work to "typedef _Bool __bool" before including altivec.h (and
in the same #ifdef)?

Paolo




Re: [Qemu-devel] [PATCH 1.1] es1370: Fix debug code

2012-05-27 Thread Peter Maydell
On 23 May 2012 22:26, Stefan Weil  wrote:
> When DEBUG_ES1370 is defined, the compiler shows these warnings:
>
> hw/es1370.c: In function ‘es1370_update_voices’:
> hw/es1370.c:414: warning: format ‘%d’ expects type ‘int’, but argument 3 has 
> type ‘size_t’

The unicode quotes in this commit message seem to have got smashed to
question-mark characters at some point between this email and it hitting
git master :-(

-- PMM



Re: [Qemu-devel] [BUG] qemu-ga: failed to fsfreeze-thaw on RHEL 5.8 Guest

2012-05-27 Thread Michael Roth
On Mon, May 28, 2012 at 08:12:12AM +0900, MATSUDA, Daiki wrote:
> I researched the source of the bug.
> 
> qemu-ga calls guest_fsfreeze_build_mount_list from
> qemp_guest_fsreeze_thaw in qga/commands-posix.c. And it tries to read
> /etc/mtab (= MOUNTED) to get mounted filesystems. But when they are
> frozen, getmntent(fp) is not finished in the situation /etc/mtab in
> frozen filesystem.
> 
> I suggest to read the list from not frozen filesystem file or on-memory
> data, e.g. /proc/mounts.

Yikes, this is a scary bug. Thanks for catching this.

I suspect the getmntent() call is causing an access time update to
/etc/mtab, which unfortunately will block while in a frozen state.

RHEL 6 and newer kernels use relatime by default so the issue isn't as
prevalent (though an atime update can still occur if /etc/mtab hasn't been
updated since the last mtime update within the kernel's 24 hour limit.
Unlikely, since guest-fsfreeze-freeze causes an update, so you'd need to
wait that long before guest-fsfreeze-thaw would trigger it, since
/etc/mtab modifications would block within that time, but not still plausible)

If you can reproduce it on RHEL 6 using the "strictatime" mount option
for /etc's filesystem, and I think that should confirm it.

Previously to commit 9e8aded432884477bcd4fa1c7e849a196412bcc4, we stored
the mount list created by guest-fsfreeze-freeze, but that behavior was
changed so that qemu-ga could thaw the system regardless of whether
or not it was the same instance that did the freeze, so that's why
you're only seeing it in 1.1 RCs.

Your suggested fix seems reasonable, but I'm having a hard time figuring
out what the differences are between /etc/mtab and /proc/mounts, and
whether they can be safely ignored for our purposes. One issue seems to
be that it doesnt distinguish --bind mounts from real ones, but that at
least is handled gracefully with the newer code (qemu-ga might freeze
the filesystem multiple times, but the thaw implementation will thaw as
many times as it needs to to unfreeze).

If all seems well I'll send a patch this evening (or feel free to resend
your's with a signed-off-by for due credit)

> 
> Regards
> MATSUDA Daiki
> 
> --- qga/commands-posix.c.orig   2012-05-28 08:10:47.842332018 +0900
> +++ qga/commands-posix.c2012-05-28 08:11:01.598340937 +0900
> @@ -347,7 +347,7 @@ static int guest_fsfreeze_build_mount_li
>  {
>  struct mntent *ment;
>  GuestFsfreezeMount *mount;
> -char const *mtab = MOUNTED;
> +char const *mtab = "/proc/mounts";
>  FILE *fp;
> 
>  fp = setmntent(mtab, "r");
> 
> > I encountered the serious bug on QEMU Guest Agent.
> > 
> > environment
> > Guest OS : RHEL 5.8 / 5.7 (i686)
> > Guest Agent Version : qemu-1.1.0rc2 and rc3
> > 
> > I am trying to take snapshot via virsh snapshot-create-as command. And
> > to freeze guest's filesystem and take snapshot is succeed. But after
> > sending the thaw command to Guest, time error occurs on libvirt qemu
> > agent because of not catch Guest's answer.
> > In addition, its situation is worst because the Guest Filesystem is kept
> > as frozen.
> > 
> > The problem does not occur on RHEL 6.2 Guest OS and in about qemu-1.0 it
> > does not occur.
> > 
> > Regards
> > MATSUDA Daiki
> > 
> > 
> > 
> > 
> 



[Qemu-devel] [PATCH 1.1] target-xtensa: fix CCOUNT for conditional branches

2012-05-27 Thread Max Filippov
Taken conditional branches fail to update CCOUNT register because
accumulated ccount_delta is reset during translation of non-taken
branch. To fix it only update CCOUNT once per conditional branch
instruction translation.

This fixes guest linux freeze on LTP waitpid06 test.

Signed-off-by: Max Filippov 
---
 target-xtensa/translate.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/target-xtensa/translate.c b/target-xtensa/translate.c
index 521c0e6..a542a31 100644
--- a/target-xtensa/translate.c
+++ b/target-xtensa/translate.c
@@ -388,6 +388,7 @@ static bool gen_check_loop_end(DisasContext *dc, int slot)
 dc->next_pc == dc->lend) {
 int label = gen_new_label();
 
+gen_advance_ccount(dc);
 tcg_gen_brcondi_i32(TCG_COND_EQ, cpu_SR[LCOUNT], 0, label);
 tcg_gen_subi_i32(cpu_SR[LCOUNT], cpu_SR[LCOUNT], 1);
 gen_jumpi(dc, dc->lbeg, slot);
@@ -410,6 +411,7 @@ static void gen_brcond(DisasContext *dc, TCGCond cond,
 {
 int label = gen_new_label();
 
+gen_advance_ccount(dc);
 tcg_gen_brcond_i32(cond, t0, t1, label);
 gen_jumpi_check_loop_end(dc, 0);
 gen_set_label(label);
-- 
1.7.7.6




Re: [Qemu-devel] [BUG] qemu-ga: failed to fsfreeze-thaw on RHEL 5.8 Guest

2012-05-27 Thread MATSUDA, Daiki
I researched the source of the bug.

qemu-ga calls guest_fsfreeze_build_mount_list from
qemp_guest_fsreeze_thaw in qga/commands-posix.c. And it tries to read
/etc/mtab (= MOUNTED) to get mounted filesystems. But when they are
frozen, getmntent(fp) is not finished in the situation /etc/mtab in
frozen filesystem.

I suggest to read the list from not frozen filesystem file or on-memory
data, e.g. /proc/mounts.

Regards
MATSUDA Daiki

--- qga/commands-posix.c.orig   2012-05-28 08:10:47.842332018 +0900
+++ qga/commands-posix.c2012-05-28 08:11:01.598340937 +0900
@@ -347,7 +347,7 @@ static int guest_fsfreeze_build_mount_li
 {
 struct mntent *ment;
 GuestFsfreezeMount *mount;
-char const *mtab = MOUNTED;
+char const *mtab = "/proc/mounts";
 FILE *fp;

 fp = setmntent(mtab, "r");

> I encountered the serious bug on QEMU Guest Agent.
> 
> environment
> Guest OS : RHEL 5.8 / 5.7 (i686)
> Guest Agent Version : qemu-1.1.0rc2 and rc3
> 
> I am trying to take snapshot via virsh snapshot-create-as command. And
> to freeze guest's filesystem and take snapshot is succeed. But after
> sending the thaw command to Guest, time error occurs on libvirt qemu
> agent because of not catch Guest's answer.
> In addition, its situation is worst because the Guest Filesystem is kept
> as frozen.
> 
> The problem does not occur on RHEL 6.2 Guest OS and in about qemu-1.0 it
> does not occur.
> 
> Regards
> MATSUDA Daiki
> 
> 
> 
> 




[Qemu-devel] [PATCH for-1.1] libqtest: Fix socket_accept() to pass address_len

2012-05-27 Thread Andreas Färber
accept() expects address_len to point to the length of the sockaddr on
input. Initialize it accordingly.

Resolves an assertion due to EFAULT on illumos.

Signed-off-by: Andreas Färber 
---
 tests/libqtest.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/tests/libqtest.c b/tests/libqtest.c
index 6d333ef..1d73fd1 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -74,6 +74,7 @@ static int socket_accept(int sock)
 socklen_t addrlen;
 int ret;
 
+addrlen = sizeof(addr);
 do {
 ret = accept(sock, (struct sockaddr *)&addr, &addrlen);
 } while (ret == -1 && errno == EINTR);
-- 
1.7.7




Re: [Qemu-devel] [PATCH for-1.1? v2] slirp: Avoid statements without effect on Big Endian host

2012-05-27 Thread Peter Maydell
On 27 May 2012 17:42, Andreas Färber  wrote:
> +# if defined(__APPLE__)
> +#  undef NTOHL
> +#  undef NTOHS
> +#  undef HTONL
> +#  undef HTONS
> +#  define NTOHL(d) do { } while (0)
> +#  define NTOHS(d) do { } while (0)
> +#  define HTONL(d) do { } while (0)
> +#  define HTONS(d) do { } while (0)
> +# else

We could just use this for everything, not just if __APPLE__,
right? For big-endian the semantics we want are always "do
nothing" so it's always OK to undef and redefine...
That would save having a special case.

-- PMM



Re: [Qemu-devel] [PATCH for-1.1 v2] tcg/ppc: Handle _CALL_DARWIN being undefined on Darwin

2012-05-27 Thread malc
On Sun, 27 May 2012, Andreas F?rber wrote:

> powerpc-apple-darwin9-gcc-4.2.1 (GCC) 4.2.1 (Apple Inc. build 5577)
> does not define _CALL_DARWIN, leading to unexpected behavior w.r.t.
> register clobbering and stack frame layout.
> 
> Since _CALL_DARWIN is a reserved identifier, define a custom
> TCG_TARGET_CALL_DARWIN based on either _CALL_DARWIN or __APPLE__.

Applied, thanks.

[..snip..]

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



[Qemu-devel] [PATCH for-1.1 v2] tcg/ppc: Handle _CALL_DARWIN being undefined on Darwin

2012-05-27 Thread Andreas Färber
powerpc-apple-darwin9-gcc-4.2.1 (GCC) 4.2.1 (Apple Inc. build 5577)
does not define _CALL_DARWIN, leading to unexpected behavior w.r.t.
register clobbering and stack frame layout.

Since _CALL_DARWIN is a reserved identifier, define a custom
TCG_TARGET_CALL_DARWIN based on either _CALL_DARWIN or __APPLE__.

Signed-off-by: Andreas Färber 
---
 tcg/ppc/tcg-target.c |   16 ++--
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/tcg/ppc/tcg-target.c b/tcg/ppc/tcg-target.c
index 4cde48d..d265697 100644
--- a/tcg/ppc/tcg-target.c
+++ b/tcg/ppc/tcg-target.c
@@ -24,7 +24,11 @@
 
 static uint8_t *tb_ret_addr;
 
-#ifdef _CALL_DARWIN
+#if defined _CALL_DARWIN || defined __APPLE__
+#define TCG_TARGET_CALL_DARWIN
+#endif
+
+#ifdef TCG_TARGET_CALL_DARWIN
 #define LINKAGE_AREA_SIZE 24
 #define LR_OFFSET 8
 #elif defined _CALL_AIX
@@ -99,7 +103,7 @@ static const int tcg_target_reg_alloc_order[] = {
 TCG_REG_R29,
 TCG_REG_R30,
 TCG_REG_R31,
-#ifdef _CALL_DARWIN
+#ifdef TCG_TARGET_CALL_DARWIN
 TCG_REG_R2,
 #endif
 TCG_REG_R3,
@@ -110,7 +114,7 @@ static const int tcg_target_reg_alloc_order[] = {
 TCG_REG_R8,
 TCG_REG_R9,
 TCG_REG_R10,
-#ifndef _CALL_DARWIN
+#ifndef TCG_TARGET_CALL_DARWIN
 TCG_REG_R11,
 #endif
 TCG_REG_R12,
@@ -140,7 +144,7 @@ static const int tcg_target_call_oarg_regs[2] = {
 };
 
 static const int tcg_target_callee_save_regs[] = {
-#ifdef _CALL_DARWIN
+#ifdef TCG_TARGET_CALL_DARWIN
 TCG_REG_R11,
 TCG_REG_R13,
 #endif
@@ -1965,7 +1969,7 @@ static void tcg_target_init(TCGContext *s)
 tcg_regset_set32(tcg_target_available_regs[TCG_TYPE_I32], 0, 0x);
 tcg_regset_set32(tcg_target_call_clobber_regs, 0,
  (1 << TCG_REG_R0) |
-#ifdef _CALL_DARWIN
+#ifdef TCG_TARGET_CALL_DARWIN
  (1 << TCG_REG_R2) |
 #endif
  (1 << TCG_REG_R3) |
@@ -1983,7 +1987,7 @@ static void tcg_target_init(TCGContext *s)
 tcg_regset_clear(s->reserved_regs);
 tcg_regset_set_reg(s->reserved_regs, TCG_REG_R0);
 tcg_regset_set_reg(s->reserved_regs, TCG_REG_R1);
-#ifndef _CALL_DARWIN
+#ifndef TCG_TARGET_CALL_DARWIN
 tcg_regset_set_reg(s->reserved_regs, TCG_REG_R2);
 #endif
 #ifdef _CALL_SYSV
-- 
1.7.5.3




Re: [Qemu-devel] [PATCH for-1.1] slirp: Untangle TCPOLEN_* from TCPOPT_*

2012-05-27 Thread Andreas Färber
Am 28.04.2012 00:29, schrieb Andreas Färber:
> From: Andreas Färber 
> 
> Commit b72210568ef0c0fb141a01cffb71a09c4efa0364 (slirp: clean up
> conflicts with system headers) enclosed TCPOLEN_MAXSEG with an #ifdef
> TCPOPT_EOL. This broke the build on illumos, which has TCPOPT_*
> but not TCPOLEN_*.
> 
> Move them to their own #ifdef TCPOLEN_MAXSEG section to remedy this.
> 
> Cc: Paolo Bonzini 
> Signed-off-by: Andreas Färber 

Ping! It seems this patch has not been applied yet despite agreement
between Blue and Jan that we should apply this least-invasive version
for 1.1. Please apply / include in a PULL for rc4.

Andreas

> ---
>  slirp/tcp.h |   13 -
>  1 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/slirp/tcp.h b/slirp/tcp.h
> index 8299603..2e2b403 100644
> --- a/slirp/tcp.h
> +++ b/slirp/tcp.h
> @@ -79,20 +79,23 @@ struct tcphdr {
>  #define  TCPOPT_EOL  0
>  #define  TCPOPT_NOP  1
>  #define  TCPOPT_MAXSEG   2
> -#defineTCPOLEN_MAXSEG4
>  #define TCPOPT_WINDOW3
> -#defineTCPOLEN_WINDOW3
>  #define TCPOPT_SACK_PERMITTED4   /* Experimental */
> -#defineTCPOLEN_SACK_PERMITTED2
>  #define TCPOPT_SACK  5   /* Experimental */
>  #define TCPOPT_TIMESTAMP 8
> -#defineTCPOLEN_TIMESTAMP 10
> -#defineTCPOLEN_TSTAMP_APPA   (TCPOLEN_TIMESTAMP+2) /* 
> appendix A */
>  
>  #define TCPOPT_TSTAMP_HDR\
>  (TCPOPT_NOP<<24|TCPOPT_NOP<<16|TCPOPT_TIMESTAMP<<8|TCPOLEN_TIMESTAMP)
>  #endif
>  
> +#ifndef TCPOLEN_MAXSEG
> +#defineTCPOLEN_MAXSEG4
> +#defineTCPOLEN_WINDOW3
> +#defineTCPOLEN_SACK_PERMITTED2
> +#defineTCPOLEN_TIMESTAMP 10
> +#defineTCPOLEN_TSTAMP_APPA   (TCPOLEN_TIMESTAMP+2) /* 
> appendix A */
> +#endif
> +
>  /*
>   * Default maximum segment size for TCP.
>   * With an IP MSS of 576, this is 536,




[Qemu-devel] [PATCH for-1.1?] slirp: Avoid redefining MAX_TCPOPTLEN

2012-05-27 Thread Andreas Färber
MAX_TCPOPTLEN is being defined as 32. Darwin has it as 40, causing a
warning. The value is only used to declare an array, into which currently
4 bytes are written at most. It should therefore be acceptable to adopt
the host's definition.

Therefore only define MAX_TCPOPTLEN if not already defined.

Signed-off-by: Andreas Färber 
---
 slirp/tcp_output.c |2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/slirp/tcp_output.c b/slirp/tcp_output.c
index 779314b..9815123 100644
--- a/slirp/tcp_output.c
+++ b/slirp/tcp_output.c
@@ -47,7 +47,9 @@ static const u_char  tcp_outflags[TCP_NSTATES] = {
 };
 
 
+#ifndef MAX_TCPOPTLEN
 #define MAX_TCPOPTLEN  32  /* max # bytes that go in options */
+#endif
 
 /*
  * Tcp output routine: figure out what should be sent and send it.
-- 
1.7.5.3




[Qemu-devel] [PATCH for-1.1? v2] slirp: Avoid statements without effect on Big Endian host

2012-05-27 Thread Andreas Färber
Darwin has HTON*/NTOH* macros that on BE simply return the argument.
This is incompatible with SLIRP's use of these macros as a statement.

Special-case Darwin in the HOST_WORDS_BIGENDIAN code path to redefine
these macros as no-op statement to avoid tons of compiler warnings.

Also adapt the fallback definitions.

Signed-off-by: Andreas Färber 
---
 slirp/ip.h |   33 ++---
 1 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/slirp/ip.h b/slirp/ip.h
index 88c903f..ddf9e9e 100644
--- a/slirp/ip.h
+++ b/slirp/ip.h
@@ -34,17 +34,28 @@
 #define _IP_H_
 
 #ifdef HOST_WORDS_BIGENDIAN
-# ifndef NTOHL
-#  define NTOHL(d)
-# endif
-# ifndef NTOHS
-#  define NTOHS(d)
-# endif
-# ifndef HTONL
-#  define HTONL(d)
-# endif
-# ifndef HTONS
-#  define HTONS(d)
+# if defined(__APPLE__)
+#  undef NTOHL
+#  undef NTOHS
+#  undef HTONL
+#  undef HTONS
+#  define NTOHL(d) do { } while (0)
+#  define NTOHS(d) do { } while (0)
+#  define HTONL(d) do { } while (0)
+#  define HTONS(d) do { } while (0)
+# else
+#  ifndef NTOHL
+#   define NTOHL(d) do { } while (0)
+#  endif
+#  ifndef NTOHS
+#   define NTOHS(d) do { } while (0)
+#  endif
+#  ifndef HTONL
+#   define HTONL(d) do { } while (0)
+#  endif
+#  ifndef HTONS
+#   define HTONS(d) do { } while (0)
+#  endif
 # endif
 #else
 # ifndef NTOHL
-- 
1.7.5.3




Re: [Qemu-devel] [PATCH for-1.1?] slirp: Avoid statements without effect on Big Endian host

2012-05-27 Thread Andreas Färber
Am 27.05.2012 18:32, schrieb Andreas Färber:
> Darwin has HTON*/NTOH* macros that on BE simply return the argument.
> This is incompatible with SLIRP's use of these macros as a statement.
> 
> Special-case Darwin in the HOST_WORDS_BIGENDIAN code path to redefine
> these macros as no-op statement to avoid tons of compiler warnings.
> 
> Signed-off-by: Andreas Färber 
> ---
>  slirp/ip.h |   33 ++---
>  1 files changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git a/slirp/ip.h b/slirp/ip.h
> index 88c903f..6e98b1d 100644
> --- a/slirp/ip.h
> +++ b/slirp/ip.h
> @@ -34,17 +34,28 @@
>  #define _IP_H_
>  
>  #ifdef HOST_WORDS_BIGENDIAN
> -# ifndef NTOHL
> -#  define NTOHL(d)
> -# endif
> -# ifndef NTOHS
> -#  define NTOHS(d)
> -# endif
> -# ifndef HTONL
> -#  define HTONL(d)
> -# endif
> -# ifndef HTONS
> -#  define HTONS(d)
> +# if defined(__APPLE__)
> +#  undef NTOHL
> +#  undef NTOHS
> +#  undef HTONL
> +#  undef HTONS
> +#  define NTOHL(d) do { } while (0)
> +#  define NTOHS(d) do { } while (0)
> +#  define HTONL(d) do { } while (0)
> +#  define HTONS(d) do { } while (0)
> +# else
> +#  ifndef NTOHL
> +#   define NTOHL(d)
> +#  endif
> +#  ifndef NTOHS
> +#   define NTOHS(d)
> +#  endif
> +#  ifndef HTONL
> +#   define HTONL(d)
> +#  endif
> +#  ifndef HTONS
> +#   define HTONS(d)
> +#  endif

Ahem, these would have the same issue. v2 coming up.

Andreas

>  # endif
>  #else
>  # ifndef NTOHL

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



[Qemu-devel] [PATCH for-1.1?] slirp: Avoid statements without effect on Big Endian host

2012-05-27 Thread Andreas Färber
Darwin has HTON*/NTOH* macros that on BE simply return the argument.
This is incompatible with SLIRP's use of these macros as a statement.

Special-case Darwin in the HOST_WORDS_BIGENDIAN code path to redefine
these macros as no-op statement to avoid tons of compiler warnings.

Signed-off-by: Andreas Färber 
---
 slirp/ip.h |   33 ++---
 1 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/slirp/ip.h b/slirp/ip.h
index 88c903f..6e98b1d 100644
--- a/slirp/ip.h
+++ b/slirp/ip.h
@@ -34,17 +34,28 @@
 #define _IP_H_
 
 #ifdef HOST_WORDS_BIGENDIAN
-# ifndef NTOHL
-#  define NTOHL(d)
-# endif
-# ifndef NTOHS
-#  define NTOHS(d)
-# endif
-# ifndef HTONL
-#  define HTONL(d)
-# endif
-# ifndef HTONS
-#  define HTONS(d)
+# if defined(__APPLE__)
+#  undef NTOHL
+#  undef NTOHS
+#  undef HTONL
+#  undef HTONS
+#  define NTOHL(d) do { } while (0)
+#  define NTOHS(d) do { } while (0)
+#  define HTONL(d) do { } while (0)
+#  define HTONS(d) do { } while (0)
+# else
+#  ifndef NTOHL
+#   define NTOHL(d)
+#  endif
+#  ifndef NTOHS
+#   define NTOHS(d)
+#  endif
+#  ifndef HTONL
+#   define HTONL(d)
+#  endif
+#  ifndef HTONS
+#   define HTONS(d)
+#  endif
 # endif
 #else
 # ifndef NTOHL
-- 
1.7.5.3




[Qemu-devel] [PATCH 1.1] exec: fix TB invalidation after breakpoint insertion/deletion

2012-05-27 Thread Max Filippov
tb_invalidate_phys_addr has to be called with the exact physical address of
the breakpoint we add/remove, not just the page's base address.
Otherwise we easily fail to flush the right TB.

This breakage was introduced by the commit f3705d5329 "memory: make
phys_page_find() return an unadjusted".

This appeared to work for some guest architectures because their
cpu_get_phys_page_debug implementation returns full translated physical
address, not just the base of the TARGET_PAGE_SIZE-sized page.

Reported-by: TeLeMan 
Signed-off-by: Jan Kiszka 
Signed-off-by: Max Filippov 
---
 exec.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/exec.c b/exec.c
index a0494c7..0a67f07 100644
--- a/exec.c
+++ b/exec.c
@@ -1492,7 +1492,8 @@ void tb_invalidate_phys_addr(target_phys_addr_t addr)
 
 static void breakpoint_invalidate(CPUArchState *env, target_ulong pc)
 {
-tb_invalidate_phys_addr(cpu_get_phys_page_debug(env, pc));
+tb_invalidate_phys_addr(cpu_get_phys_page_debug(env, pc) |
+(pc & ~TARGET_PAGE_MASK));
 }
 #endif
 #endif /* TARGET_HAS_ICE */
-- 
1.7.7.6




Re: [Qemu-devel] [PATCH for-1.1] arch_init: Fix AltiVec build on Darwin/ppc

2012-05-27 Thread Andreas Färber
Am 27.05.2012 17:37, schrieb Andreas Färber:
> Commit f29a56147b66845914d0a645bf9b4c5bb9a6af57 (implement
> -no-user-config command-line option (v3)) introduced uses of bool
> in arch_init.c. Shortly before that usage is support code for
> AltiVec (conditional to __ALTIVEC__).
> 
> GCC's altivec.h may in a !__APPLE_ALTIVEC__ code path redefine bool,
> leading to type mismatches. altivec.h recommends to #undef for C++
> compatibility, but doing so on C leads to bool remaining undefined.

"in C" - please fix when applying.

Thanks,
Andreas

> Fix by redefining bool to _Bool as mandated for stdbool.h by POSIX.
> 
> Signed-off-by: Andreas Färber 
> Cc: Eduardo Habkost 
> Cc: Paolo Bonzini 
> Cc: Alexander Graf 
> Cc: qemu-ppc 

P.S. No such errors with gcc (SUSE Linux) 4.6.3, even with
--extra-cflags="-maltivec", so I'm guessing __bool differs.

> ---
>  arch_init.c |4 
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/arch_init.c b/arch_init.c
> index 988adca..a9e8b74 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -100,6 +100,10 @@ const uint32_t arch_type = QEMU_ARCH;
>  #define VECTYPEvector unsigned char
>  #define SPLAT(p)   vec_splat(vec_ld(0, p), 0)
>  #define ALL_EQ(v1, v2) vec_all_eq(v1, v2)
> +/* altivec.h may redefine the bool macro as vector type.
> + * Reset it to POSIX semantics. */
> +#undef bool
> +#define bool _Bool
>  #elif defined __SSE2__
>  #include 
>  #define VECTYPE__m128i

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



[Qemu-devel] [PATCH for-1.1] arch_init: Fix AltiVec build on Darwin/ppc

2012-05-27 Thread Andreas Färber
Commit f29a56147b66845914d0a645bf9b4c5bb9a6af57 (implement
-no-user-config command-line option (v3)) introduced uses of bool
in arch_init.c. Shortly before that usage is support code for
AltiVec (conditional to __ALTIVEC__).

GCC's altivec.h may in a !__APPLE_ALTIVEC__ code path redefine bool,
leading to type mismatches. altivec.h recommends to #undef for C++
compatibility, but doing so on C leads to bool remaining undefined.

Fix by redefining bool to _Bool as mandated for stdbool.h by POSIX.

Signed-off-by: Andreas Färber 
Cc: Eduardo Habkost 
Cc: Paolo Bonzini 
Cc: Alexander Graf 
Cc: qemu-ppc 
---
 arch_init.c |4 
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch_init.c b/arch_init.c
index 988adca..a9e8b74 100644
--- a/arch_init.c
+++ b/arch_init.c
@@ -100,6 +100,10 @@ const uint32_t arch_type = QEMU_ARCH;
 #define VECTYPEvector unsigned char
 #define SPLAT(p)   vec_splat(vec_ld(0, p), 0)
 #define ALL_EQ(v1, v2) vec_all_eq(v1, v2)
+/* altivec.h may redefine the bool macro as vector type.
+ * Reset it to POSIX semantics. */
+#undef bool
+#define bool _Bool
 #elif defined __SSE2__
 #include 
 #define VECTYPE__m128i
-- 
1.7.5.3




Re: [Qemu-devel] [PULL 1.1 0/6] target-xtensa MMU fixes

2012-05-27 Thread Max Filippov
On Sun, May 27, 2012 at 6:54 PM, Andreas Färber  wrote:
> Am 27.05.2012 16:34, schrieb Max Filippov:
>> Hello.
>>
>> This series fixes subtle bugs in the xtensa hardware pagewalking 
>> implementation
>> and adds more MMU test cases.
>>
>> The following changes since commit aeb29b6459cb9496b38c820f3faff64cf2369d0d:
>>
>>   audio: Always call fini on exit (2012-05-24 19:35:27 +0400)
>>
>> are available in the git repository at:
>>   git://jcmvbkbc.spb.ru/dumb/qemu-xtensa.git xtensa
>>
>> Max Filippov (6):
>>   target-xtensa: flush TLB page for new MMU mapping
>
> At least this patch seems to have never reached my inbox before. The

My bad, actually I send it for the first time, should have been mere
patch series. Please review.

> expected procedure is to first patches on the list, wait for review and
> only after acks have been received or sufficient time passed and a ping
> remained without acks, post a PULL request that can then be applied
> without review of contents, only making sure it doesn't break the build
> or significantly regresses the target(s).
>
> This patch itself looks okay, but I'm pointing it out since the list
> seems to be lagging once again and because you're sending it for the
> final RC.

-- 
Thanks.
-- Max



[Qemu-devel] [PATCH 1.1 5/6] target-xtensa: control page table lookup explicitly

2012-05-27 Thread Max Filippov
Hardware pagetable walking may not be nested. Stop guessing and pass
explicit flag to the get_physical_addr_mmu function that controls page
table lookup.

Signed-off-by: Max Filippov 
---
 target-xtensa/helper.c |   10 +-
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/target-xtensa/helper.c b/target-xtensa/helper.c
index 86c33d2..8ebef72 100644
--- a/target-xtensa/helper.c
+++ b/target-xtensa/helper.c
@@ -452,7 +452,8 @@ static int get_pte(CPUXtensaState *env, uint32_t vaddr, 
uint32_t *pte);
 
 static int get_physical_addr_mmu(CPUXtensaState *env, bool update_tlb,
 uint32_t vaddr, int is_write, int mmu_idx,
-uint32_t *paddr, uint32_t *page_size, unsigned *access)
+uint32_t *paddr, uint32_t *page_size, unsigned *access,
+bool may_lookup_pt)
 {
 bool dtlb = is_write != 2;
 uint32_t wi;
@@ -465,8 +466,7 @@ static int get_physical_addr_mmu(CPUXtensaState *env, bool 
update_tlb,
 int ret = xtensa_tlb_lookup(env, vaddr, dtlb, &wi, &ei, &ring);
 
 if ((ret == INST_TLB_MISS_CAUSE || ret == LOAD_STORE_TLB_MISS_CAUSE) &&
-(mmu_idx != 0 || ((vaddr ^ env->sregs[PTEVADDR]) & 0xffc0)) &&
-get_pte(env, vaddr, &pte) == 0) {
+may_lookup_pt && get_pte(env, vaddr, &pte) == 0) {
 ring = (pte >> 4) & 0x3;
 wi = 0;
 split_tlb_entry_spec_way(env, vaddr, dtlb, &vpn, wi, &ei);
@@ -520,7 +520,7 @@ static int get_pte(CPUXtensaState *env, uint32_t vaddr, 
uint32_t *pte)
 uint32_t pt_vaddr =
 (env->sregs[PTEVADDR] | (vaddr >> 10)) & 0xfffc;
 int ret = get_physical_addr_mmu(env, false, pt_vaddr, 0, 0,
-&paddr, &page_size, &access);
+&paddr, &page_size, &access, false);
 
 qemu_log("%s: trying autorefill(%08x) -> %08x\n", __func__,
 vaddr, ret ? ~0 : paddr);
@@ -568,7 +568,7 @@ int xtensa_get_physical_addr(CPUXtensaState *env, bool 
update_tlb,
 {
 if (xtensa_option_enabled(env->config, XTENSA_OPTION_MMU)) {
 return get_physical_addr_mmu(env, update_tlb,
-vaddr, is_write, mmu_idx, paddr, page_size, access);
+vaddr, is_write, mmu_idx, paddr, page_size, access, true);
 } else if (xtensa_option_bits_enabled(env->config,
 XTENSA_OPTION_BIT(XTENSA_OPTION_REGION_PROTECTION) |
 XTENSA_OPTION_BIT(XTENSA_OPTION_REGION_TRANSLATION))) {
-- 
1.7.7.6




Re: [Qemu-devel] [PATCH for-1.1] virtio-blk: Fix unused variables in virtio_blk_handle_scsi()

2012-05-27 Thread Andreas Färber
Am 27.05.2012 16:41, schrieb Andreas Färber:
> Commit f34e73cd69bdbdb9b1d56b288c5e14d6fff58165 (virtio-blk: report
> non-zero status when failing SG_IO requests) exposed the function
> to non-Linux guests. Move all Linux-only variable declarations into

"hosts", obviously. Please fix when applying.

Thanks,
Andreas

> an #ifdef in the variable declaration block.
> 
> Signed-off-by: Andreas Färber 
> Cc: Paolo Bonzini 
> ---
>  hw/virtio-blk.c |6 --
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
> index f9e1896..a1b64cb 100644
> --- a/hw/virtio-blk.c
> +++ b/hw/virtio-blk.c
> @@ -147,9 +147,12 @@ static VirtIOBlockReq 
> *virtio_blk_get_request(VirtIOBlock *s)
>  
>  static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
>  {
> -int ret;
>  int status = VIRTIO_BLK_S_OK;
> +#ifdef __linux__
> +struct sg_io_hdr hdr;
> +int ret;
>  int i;
> +#endif
>  
>  /*
>   * We require at least one output segment each for the virtio_blk_outhdr
> @@ -184,7 +187,6 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
>  }
>  
>  #ifdef __linux__
> -struct sg_io_hdr hdr;
>  memset(&hdr, 0, sizeof(struct sg_io_hdr));
>  hdr.interface_id = 'S';
>  hdr.cmd_len = req->elem.out_sg[1].iov_len;

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] [PATCH for-1.1] qemu-ga: Fix use of environ on Darwin

2012-05-27 Thread Andreas Färber
Am 27.05.2012 17:02, schrieb Andreas Färber:
> Use _NSGetEnviron() helper to access the environment.
> 
> Signed-off-by: Andreas Färber 
> Cc: Charlie Somerville 
> ---
>  Michael, can you please append this to your qemu-ga PULL?
>  
>  qga/commands-posix.c |5 +
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index dab3bf9..4a71c27 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -22,8 +22,13 @@
>  #include "host-utils.h"
>  
>  #ifndef CONFIG_HAS_ENVIRON
> +#ifdef __APPLE__
> +#include 
> +#define environ (*_NSGetEnviron())
> +#else
>  extern char **environ;
>  #endif
> +#endif
>  
>  #if defined(__linux__)
>  #include 

For 1.2 it might also be a good idea to move this block to osdep.h, so
that it doesn't get duplicated when needed somewhere else.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



[Qemu-devel] [PATCH for-1.1] qemu-ga: Fix use of environ on Darwin

2012-05-27 Thread Andreas Färber
Use _NSGetEnviron() helper to access the environment.

Signed-off-by: Andreas Färber 
Cc: Charlie Somerville 
---
 Michael, can you please append this to your qemu-ga PULL?
 
 qga/commands-posix.c |5 +
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/qga/commands-posix.c b/qga/commands-posix.c
index dab3bf9..4a71c27 100644
--- a/qga/commands-posix.c
+++ b/qga/commands-posix.c
@@ -22,8 +22,13 @@
 #include "host-utils.h"
 
 #ifndef CONFIG_HAS_ENVIRON
+#ifdef __APPLE__
+#include 
+#define environ (*_NSGetEnviron())
+#else
 extern char **environ;
 #endif
+#endif
 
 #if defined(__linux__)
 #include 
-- 
1.7.5.3




[Qemu-devel] [PATCH 1.1 4/6] target-xtensa: update autorefill TLB entries conditionally

2012-05-27 Thread Max Filippov
This is to avoid interference of internal QEMU helpers
(cpu_get_phys_page_debug, tb_invalidate_virtual_addr) with guest-visible
TLB state.

Signed-off-by: Max Filippov 
---
 target-xtensa/cpu.h   |2 +-
 target-xtensa/helper.c|   56 +---
 target-xtensa/op_helper.c |4 +-
 3 files changed, 35 insertions(+), 27 deletions(-)

diff --git a/target-xtensa/cpu.h b/target-xtensa/cpu.h
index 6c590fe..d5b50d1 100644
--- a/target-xtensa/cpu.h
+++ b/target-xtensa/cpu.h
@@ -375,7 +375,7 @@ void xtensa_tlb_set_entry_mmu(const CPUXtensaState *env,
 unsigned wi, unsigned ei, uint32_t vpn, uint32_t pte);
 void xtensa_tlb_set_entry(CPUXtensaState *env, bool dtlb,
 unsigned wi, unsigned ei, uint32_t vpn, uint32_t pte);
-int xtensa_get_physical_addr(CPUXtensaState *env,
+int xtensa_get_physical_addr(CPUXtensaState *env, bool update_tlb,
 uint32_t vaddr, int is_write, int mmu_idx,
 uint32_t *paddr, uint32_t *page_size, unsigned *access);
 void reset_mmu(CPUXtensaState *env);
diff --git a/target-xtensa/helper.c b/target-xtensa/helper.c
index 43a6611..86c33d2 100644
--- a/target-xtensa/helper.c
+++ b/target-xtensa/helper.c
@@ -135,11 +135,11 @@ target_phys_addr_t cpu_get_phys_page_debug(CPUXtensaState 
*env, target_ulong add
 uint32_t page_size;
 unsigned access;
 
-if (xtensa_get_physical_addr(env, addr, 0, 0,
+if (xtensa_get_physical_addr(env, false, addr, 0, 0,
 &paddr, &page_size, &access) == 0) {
 return paddr;
 }
-if (xtensa_get_physical_addr(env, addr, 2, 0,
+if (xtensa_get_physical_addr(env, false, addr, 2, 0,
 &paddr, &page_size, &access) == 0) {
 return paddr;
 }
@@ -448,10 +448,9 @@ static bool is_access_granted(unsigned access, int 
is_write)
 }
 }
 
-static int autorefill_mmu(CPUXtensaState *env, uint32_t vaddr, bool dtlb,
-uint32_t *wi, uint32_t *ei, uint8_t *ring);
+static int get_pte(CPUXtensaState *env, uint32_t vaddr, uint32_t *pte);
 
-static int get_physical_addr_mmu(CPUXtensaState *env,
+static int get_physical_addr_mmu(CPUXtensaState *env, bool update_tlb,
 uint32_t vaddr, int is_write, int mmu_idx,
 uint32_t *paddr, uint32_t *page_size, unsigned *access)
 {
@@ -459,19 +458,38 @@ static int get_physical_addr_mmu(CPUXtensaState *env,
 uint32_t wi;
 uint32_t ei;
 uint8_t ring;
+uint32_t vpn;
+uint32_t pte;
+const xtensa_tlb_entry *entry = NULL;
+xtensa_tlb_entry tmp_entry;
 int ret = xtensa_tlb_lookup(env, vaddr, dtlb, &wi, &ei, &ring);
 
 if ((ret == INST_TLB_MISS_CAUSE || ret == LOAD_STORE_TLB_MISS_CAUSE) &&
 (mmu_idx != 0 || ((vaddr ^ env->sregs[PTEVADDR]) & 0xffc0)) &&
-autorefill_mmu(env, vaddr, dtlb, &wi, &ei, &ring) == 0) {
+get_pte(env, vaddr, &pte) == 0) {
+ring = (pte >> 4) & 0x3;
+wi = 0;
+split_tlb_entry_spec_way(env, vaddr, dtlb, &vpn, wi, &ei);
+
+if (update_tlb) {
+wi = ++env->autorefill_idx & 0x3;
+xtensa_tlb_set_entry(env, dtlb, wi, ei, vpn, pte);
+env->sregs[EXCVADDR] = vaddr;
+qemu_log("%s: autorefill(%08x): %08x -> %08x\n",
+__func__, vaddr, vpn, pte);
+} else {
+xtensa_tlb_set_entry_mmu(env, &tmp_entry, dtlb, wi, ei, vpn, pte);
+entry = &tmp_entry;
+}
 ret = 0;
 }
 if (ret != 0) {
 return ret;
 }
 
-const xtensa_tlb_entry *entry =
-xtensa_tlb_get_entry(env, dtlb, wi, ei);
+if (entry == NULL) {
+entry = xtensa_tlb_get_entry(env, dtlb, wi, ei);
+}
 
 if (ring < mmu_idx) {
 return dtlb ?
@@ -494,31 +512,21 @@ static int get_physical_addr_mmu(CPUXtensaState *env,
 return 0;
 }
 
-static int autorefill_mmu(CPUXtensaState *env, uint32_t vaddr, bool dtlb,
-uint32_t *wi, uint32_t *ei, uint8_t *ring)
+static int get_pte(CPUXtensaState *env, uint32_t vaddr, uint32_t *pte)
 {
 uint32_t paddr;
 uint32_t page_size;
 unsigned access;
 uint32_t pt_vaddr =
 (env->sregs[PTEVADDR] | (vaddr >> 10)) & 0xfffc;
-int ret = get_physical_addr_mmu(env, pt_vaddr, 0, 0,
+int ret = get_physical_addr_mmu(env, false, pt_vaddr, 0, 0,
 &paddr, &page_size, &access);
 
 qemu_log("%s: trying autorefill(%08x) -> %08x\n", __func__,
 vaddr, ret ? ~0 : paddr);
 
 if (ret == 0) {
-uint32_t vpn;
-uint32_t pte = ldl_phys(paddr);
-
-*ring = (pte >> 4) & 0x3;
-*wi = (++env->autorefill_idx) & 0x3;
-split_tlb_entry_spec_way(env, vaddr, dtlb, &vpn, *wi, ei);
-xtensa_tlb_set_entry(env, dtlb, *wi, *ei, vpn, pte);
-env->sregs[EXCVADDR] = vaddr;
-qemu_log("%s: autorefill(%08x): %08x -> %08x\n",
-__func__, vaddr, vpn, pte);
+*pte = ldl_phys(paddr);
 }
 return ret;
 }
@@ -554,13 +562,13 @@ static int

Re: [Qemu-devel] [PULL 1.1 0/6] target-xtensa MMU fixes

2012-05-27 Thread Andreas Färber
Am 27.05.2012 16:34, schrieb Max Filippov:
> Hello.
> 
> This series fixes subtle bugs in the xtensa hardware pagewalking 
> implementation
> and adds more MMU test cases.
> 
> The following changes since commit aeb29b6459cb9496b38c820f3faff64cf2369d0d:
> 
>   audio: Always call fini on exit (2012-05-24 19:35:27 +0400)
> 
> are available in the git repository at:
>   git://jcmvbkbc.spb.ru/dumb/qemu-xtensa.git xtensa
> 
> Max Filippov (6):
>   target-xtensa: flush TLB page for new MMU mapping

At least this patch seems to have never reached my inbox before. The
expected procedure is to first patches on the list, wait for review and
only after acks have been received or sufficient time passed and a ping
remained without acks, post a PULL request that can then be applied
without review of contents, only making sure it doesn't break the build
or significantly regresses the target(s).

This patch itself looks okay, but I'm pointing it out since the list
seems to be lagging once again and because you're sending it for the
final RC.

Regards,
Andreas

>   target-xtensa: update EXCVADDR in case of page table lookup
>   target-xtensa: extract TLB entry setting method
>   target-xtensa: update autorefill TLB entries conditionally
>   target-xtensa: control page table lookup explicitly
>   target-xtensa: add MMU pagewalking tests
> 
>  target-xtensa/cpu.h |5 +-
>  target-xtensa/helper.c  |   61 +++-
>  target-xtensa/op_helper.c   |   20 +++-
>  tests/tcg/xtensa/test_mmu.S |  221 
> ---
>  4 files changed, 260 insertions(+), 47 deletions(-)

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



[Qemu-devel] [PATCH 1.1 6/6] target-xtensa: add MMU pagewalking tests

2012-05-27 Thread Max Filippov
Signed-off-by: Max Filippov 
---
 tests/tcg/xtensa/test_mmu.S |  221 ---
 1 files changed, 207 insertions(+), 14 deletions(-)

diff --git a/tests/tcg/xtensa/test_mmu.S b/tests/tcg/xtensa/test_mmu.S
index 52d5774..5d87fbb 100644
--- a/tests/tcg/xtensa/test_mmu.S
+++ b/tests/tcg/xtensa/test_mmu.S
@@ -293,26 +293,219 @@ test store_prohibited
 assert  eq, a2, a3
 test_end
 
-test dtlb_autoload
-set_vector kernel, 0
-
-movia2, 0xd400
+/* Set up page table entry vaddr->paddr, ring=pte_ring, attr=pte_attr
+ * and DTLB way 7 to cover this PTE, ring=pt_ring, attr=pt_attr
+ */
+.macro pt_setup pt_ring, pt_attr, pte_ring, vaddr, paddr, pte_attr
+movia2, 0x8000
 wsr a2, ptevaddr
-movia3, 0x1013
-s32ia3, a2, 4
+
+movia3, 0x8007 | (((\vaddr) >> 10) & 0xf000) /* way 7 */
+movia4, 0x0403 | ((\pt_ring) << 4) /* PADDR 64M */
+wdtlb   a4, a3
+isync
+
+movia3, ((\paddr) & 0xf000) | ((\pte_ring) << 4) | (\pte_attr)
+movia1, ((\vaddr) >> 12) << 2
+add a2, a1, a2
+s32ia3, a2, 0
+
+movia3, 0x8007 | (((\vaddr) >> 10) & 0xf000) /* way 7 */
+movia4, 0x0400 | ((\pt_ring) << 4) | (\pt_attr) /* PADDR 64M */
+wdtlb   a4, a3
+isync
+
+movia3, (\vaddr)
+.endm
+
+/* out: PS.RING=ring, PS.EXCM=excm, a3=vaddr */
+.macro go_ring ring, excm, vaddr
+movia3, 10f
+pitlb   a3, a3
+ritlb1  a2, a3
+movia1, 0x10
+or  a2, a2, a1
+movia1, 0x000ff000
+and a3, a3, a1
+movia1, 4
+or  a3, a3, a1
+witlb   a2, a3
+movia3, 10f
+movia1, 0x000f
+and a1, a3, a1
+
+movia2, 0
+wsr a2, excvaddr
+
+movia3, \vaddr
+movia2, 0x4000f | ((\ring) << 6) | ((\excm) << 4)
+jx  a1
+10:
+wsr a2, ps
+isync
+.endm
+
+/* in: a3 -- virtual address to test */
+.macro assert_auto_tlb
+movia2, 0x4000f
+wsr a2, ps
+isync
+pdtlb   a2, a3
+movia1, 0xf01f
+and a2, a2, a1
+movia1, 0xf000
+and a1, a1, a3
+xor a1, a1, a2
+assert  gei, a1, 0x10
+movia2, 0x14
+assert  lt, a1, a2
+.endm
+
+/* in: a3 -- virtual address to test */
+.macro assert_no_auto_tlb
+movia2, 0x4000f
+wsr a2, ps
+isync
 pdtlb   a2, a3
 movia1, 0x10
 and a1, a1, a2
 assert  eqi, a1, 0
-l8uia1, a3, 0
-pdtlb   a2, a3
-movia1, 0xf010
-and a1, a1, a2
-movia3, 0x1010
-assert  eq, a1, a3
-movia1, 0xf
+.endm
+
+.macro assert_sr sr, v
+rsr a2, \sr
+movia1, (\v)
+assert  eq, a1, a2
+.endm
+
+.macro assert_epc1_1m vaddr
+movia2, (\vaddr)
+movia1, 0xf
 and a1, a1, a2
-assert  lti, a1, 4
+rsr a2, epc1
+assert  eq, a1, a2
+.endm
+
+test dtlb_autoload
+set_vector kernel, 0
+
+pt_setup0, 3, 1, 0x1000, 0x1000, 3
+assert_no_auto_tlb
+
+l8uia1, a3, 0
+
+rsr a2, excvaddr
+assert  eq, a2, a3
+
+assert_auto_tlb
+test_end
+
+test autoload_load_store_privilege
+set_vector kernel, 0
+set_vector double, 2f
+
+pt_setup0, 3, 0, 0x2000, 0x2000, 3
+movia3, 0x2004
+assert_no_auto_tlb
+
+movia2, 0x4005f/* ring 1 + excm => cring == 0 */
+wsr a2, ps
+isync
+1:
+l32ea2, a3, -4 /* ring used */
+test_fail
+2:
+rsr a2, excvaddr
+addia1, a3, -4
+assert  eq, a1, a2
+
+assert_auto_tlb
+assert_sr depc, 1b
+assert_sr exccause, 26
+test_end
+
+test autoload_pte_load_prohibited
+set_vector kernel, 2f
+
+pt_setup0, 3, 0, 0x3000, 0, 0xc
+assert_no_auto_tlb
+1:
+l32ia2, a3, 0
+test_fail
+2:
+rsr a2, excvaddr
+assert  eq, a2, a3
+
+assert_auto_tlb
+assert_sr epc1, 1b
+assert_sr exccause, 28
+test_end
+
+test autoload_pt_load_prohibited
+set_vector kernel, 2f
+
+pt_setup0, 0xc, 0, 0x4000, 0x4000, 3
+assert_no_auto_tlb
+1:
+l32ia2, a3, 0
+test_fail
+2:
+rsr a2, excvaddr
+assert  eq, a2, a3
+
+assert_no_auto_tlb
+assert_sr epc1, 1b
+assert_sr exccause, 24
+test_end
+
+test autoload_pt_privilege
+set_vector  kernel, 2f
+pt_setup0, 3, 1, 0x5000, 0, 3
+go_ring 1, 0, 0x5001
+
+l8uia2, a3, 0
+1:
+syscall
+2:
+rsr a2, excvaddr
+assert  eq, a2, a3
+
+assert_auto_tlb
+assert_epc1_1m 1b
+assert_sr exccause, 1
+test_end
+
+test autoload_pte_privilege
+set_vector  kernel, 2f
+pt_setup0, 3, 0, 0x6000, 0, 3
+go_ring 1, 0, 0x6001
+1:
+l8uia2, a3, 0
+syscall
+2:
+rsr a2, excvaddr
+assert  eq, a2, a3
+
+assert_auto_tlb
+assert_epc1_1m 1b
+assert_sr exccause, 26
+test_end
+
+test autoload_3_level_pt
+set_vector  kernel, 2f
+pt_s

[Qemu-devel] [PATCH 1.1 2/6] target-xtensa: update EXCVADDR in case of page table lookup

2012-05-27 Thread Max Filippov
According to ISA, 4.4.2.6, EXCVADDR may be changed by any TLB miss, even
if the miss is handled entirely by processor hardware.

Signed-off-by: Max Filippov 
---
 target-xtensa/helper.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/target-xtensa/helper.c b/target-xtensa/helper.c
index 2094227..43a6611 100644
--- a/target-xtensa/helper.c
+++ b/target-xtensa/helper.c
@@ -516,6 +516,7 @@ static int autorefill_mmu(CPUXtensaState *env, uint32_t 
vaddr, bool dtlb,
 *wi = (++env->autorefill_idx) & 0x3;
 split_tlb_entry_spec_way(env, vaddr, dtlb, &vpn, *wi, ei);
 xtensa_tlb_set_entry(env, dtlb, *wi, *ei, vpn, pte);
+env->sregs[EXCVADDR] = vaddr;
 qemu_log("%s: autorefill(%08x): %08x -> %08x\n",
 __func__, vaddr, vpn, pte);
 }
-- 
1.7.7.6




[Qemu-devel] [PATCH for-1.1] virtio-blk: Fix unused variables in virtio_blk_handle_scsi()

2012-05-27 Thread Andreas Färber
Commit f34e73cd69bdbdb9b1d56b288c5e14d6fff58165 (virtio-blk: report
non-zero status when failing SG_IO requests) exposed the function
to non-Linux guests. Move all Linux-only variable declarations into
an #ifdef in the variable declaration block.

Signed-off-by: Andreas Färber 
Cc: Paolo Bonzini 
---
 hw/virtio-blk.c |6 --
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/virtio-blk.c b/hw/virtio-blk.c
index f9e1896..a1b64cb 100644
--- a/hw/virtio-blk.c
+++ b/hw/virtio-blk.c
@@ -147,9 +147,12 @@ static VirtIOBlockReq *virtio_blk_get_request(VirtIOBlock 
*s)
 
 static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
 {
-int ret;
 int status = VIRTIO_BLK_S_OK;
+#ifdef __linux__
+struct sg_io_hdr hdr;
+int ret;
 int i;
+#endif
 
 /*
  * We require at least one output segment each for the virtio_blk_outhdr
@@ -184,7 +187,6 @@ static void virtio_blk_handle_scsi(VirtIOBlockReq *req)
 }
 
 #ifdef __linux__
-struct sg_io_hdr hdr;
 memset(&hdr, 0, sizeof(struct sg_io_hdr));
 hdr.interface_id = 'S';
 hdr.cmd_len = req->elem.out_sg[1].iov_len;
-- 
1.7.5.3




[Qemu-devel] [PATCH 1.1 3/6] target-xtensa: extract TLB entry setting method

2012-05-27 Thread Max Filippov
Signed-off-by: Max Filippov 
---
 target-xtensa/cpu.h   |3 +++
 target-xtensa/op_helper.c |   15 +++
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/target-xtensa/cpu.h b/target-xtensa/cpu.h
index 6d0ea7c..6c590fe 100644
--- a/target-xtensa/cpu.h
+++ b/target-xtensa/cpu.h
@@ -370,6 +370,9 @@ void split_tlb_entry_spec_way(const CPUXtensaState *env, 
uint32_t v, bool dtlb,
 uint32_t *vpn, uint32_t wi, uint32_t *ei);
 int xtensa_tlb_lookup(const CPUXtensaState *env, uint32_t addr, bool dtlb,
 uint32_t *pwi, uint32_t *pei, uint8_t *pring);
+void xtensa_tlb_set_entry_mmu(const CPUXtensaState *env,
+xtensa_tlb_entry *entry, bool dtlb,
+unsigned wi, unsigned ei, uint32_t vpn, uint32_t pte);
 void xtensa_tlb_set_entry(CPUXtensaState *env, bool dtlb,
 unsigned wi, unsigned ei, uint32_t vpn, uint32_t pte);
 int xtensa_get_physical_addr(CPUXtensaState *env,
diff --git a/target-xtensa/op_helper.c b/target-xtensa/op_helper.c
index ce61157..663bb6d 100644
--- a/target-xtensa/op_helper.c
+++ b/target-xtensa/op_helper.c
@@ -655,6 +655,16 @@ uint32_t HELPER(ptlb)(uint32_t v, uint32_t dtlb)
 }
 }
 
+void xtensa_tlb_set_entry_mmu(const CPUXtensaState *env,
+xtensa_tlb_entry *entry, bool dtlb,
+unsigned wi, unsigned ei, uint32_t vpn, uint32_t pte)
+{
+entry->vaddr = vpn;
+entry->paddr = pte & xtensa_tlb_get_addr_mask(env, dtlb, wi);
+entry->asid = (env->sregs[RASID] >> ((pte >> 1) & 0x18)) & 0xff;
+entry->attr = pte & 0xf;
+}
+
 void xtensa_tlb_set_entry(CPUXtensaState *env, bool dtlb,
 unsigned wi, unsigned ei, uint32_t vpn, uint32_t pte)
 {
@@ -665,10 +675,7 @@ void xtensa_tlb_set_entry(CPUXtensaState *env, bool dtlb,
 if (entry->asid) {
 tlb_flush_page(env, entry->vaddr);
 }
-entry->vaddr = vpn;
-entry->paddr = pte & xtensa_tlb_get_addr_mask(env, dtlb, wi);
-entry->asid = (env->sregs[RASID] >> ((pte >> 1) & 0x18)) & 0xff;
-entry->attr = pte & 0xf;
+xtensa_tlb_set_entry_mmu(env, entry, dtlb, wi, ei, vpn, pte);
 tlb_flush_page(env, entry->vaddr);
 } else {
 qemu_log("%s %d, %d, %d trying to set immutable entry\n",
-- 
1.7.7.6




[Qemu-devel] [PATCH 1.1 1/6] target-xtensa: flush TLB page for new MMU mapping

2012-05-27 Thread Max Filippov
Both old and new mappings need flushing because their VPN may be
different in MMU case.

Signed-off-by: Max Filippov 
---
 target-xtensa/op_helper.c |1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/target-xtensa/op_helper.c b/target-xtensa/op_helper.c
index 364dc19..ce61157 100644
--- a/target-xtensa/op_helper.c
+++ b/target-xtensa/op_helper.c
@@ -669,6 +669,7 @@ void xtensa_tlb_set_entry(CPUXtensaState *env, bool dtlb,
 entry->paddr = pte & xtensa_tlb_get_addr_mask(env, dtlb, wi);
 entry->asid = (env->sregs[RASID] >> ((pte >> 1) & 0x18)) & 0xff;
 entry->attr = pte & 0xf;
+tlb_flush_page(env, entry->vaddr);
 } else {
 qemu_log("%s %d, %d, %d trying to set immutable entry\n",
 __func__, dtlb, wi, ei);
-- 
1.7.7.6




[Qemu-devel] [PULL 1.1 0/6] target-xtensa MMU fixes

2012-05-27 Thread Max Filippov
Hello.

This series fixes subtle bugs in the xtensa hardware pagewalking implementation
and adds more MMU test cases.

The following changes since commit aeb29b6459cb9496b38c820f3faff64cf2369d0d:

  audio: Always call fini on exit (2012-05-24 19:35:27 +0400)

are available in the git repository at:
  git://jcmvbkbc.spb.ru/dumb/qemu-xtensa.git xtensa

Max Filippov (6):
  target-xtensa: flush TLB page for new MMU mapping
  target-xtensa: update EXCVADDR in case of page table lookup
  target-xtensa: extract TLB entry setting method
  target-xtensa: update autorefill TLB entries conditionally
  target-xtensa: control page table lookup explicitly
  target-xtensa: add MMU pagewalking tests

 target-xtensa/cpu.h |5 +-
 target-xtensa/helper.c  |   61 +++-
 target-xtensa/op_helper.c   |   20 +++-
 tests/tcg/xtensa/test_mmu.S |  221 ---
 4 files changed, 260 insertions(+), 47 deletions(-)

-- 
1.7.7.6



Re: [Qemu-devel] [PATCH qemu 5/6] implement -no-user-config command-line option (v3)

2012-05-27 Thread Andreas Färber
Am 02.05.2012 18:07, schrieb Eduardo Habkost:
> Changes v2 -> v3:
>  - Rebase against latest qemu.git
> 
> Changes v1 -> v2:
>  - Change 'userconfig' field/variables to bool instead of int
>  - Coding style change
> 
> Signed-off-by: Eduardo Habkost 
> ---
>  arch_init.c |   11 ---
>  qemu-config.h   |2 +-
>  qemu-options.hx |   16 +---
>  vl.c|6 +-
>  4 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/arch_init.c b/arch_init.c
> index 62332e9..996baba 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -114,19 +114,24 @@ const uint32_t arch_type = QEMU_ARCH;
>  
>  static struct defconfig_file {
>  const char *filename;
> +/* Indicates it is an user config file (disabled by -no-user-config) */
> +bool userconfig;
>  } default_config_files[] = {
> -{ CONFIG_QEMU_CONFDIR "/qemu.conf" },
> -{ CONFIG_QEMU_CONFDIR "/target-" TARGET_ARCH ".conf" },
> +{ CONFIG_QEMU_CONFDIR "/qemu.conf",   true },
> +{ CONFIG_QEMU_CONFDIR "/target-" TARGET_ARCH ".conf", true },
>  { NULL }, /* end of list */
>  };
>  
>  
> -int qemu_read_default_config_files(void)
> +int qemu_read_default_config_files(bool userconfig)

These changes broke the build on Darwin/ppc(64).

Just before this block there's an inclusion of altivec.h, which does
#define bool __bool
for powerpc-apple-darwin9-gcc-4.0.1 (GCC) 4.0.1 (Apple Inc. build 5493).

  CCi386-softmmu/arch_init.o
/Users/andreas/QEMU/qemu/arch_init.c:120: error: incompatible types in
initialization
/Users/andreas/QEMU/qemu/arch_init.c:121: error: incompatible types in
initialization
/Users/andreas/QEMU/qemu/arch_init.c:122: error: incompatible types in
initialization
/Users/andreas/QEMU/qemu/arch_init.c:128: error: conflicting types for
'qemu_read_default_config_files'
/Users/andreas/QEMU/qemu/qemu-config.h:21: error: previous declaration
of 'qemu_read_default_config_files' was here
/Users/andreas/QEMU/qemu/arch_init.c: In function
'qemu_read_default_config_files':
/Users/andreas/QEMU/qemu/arch_init.c:133: error: wrong type argument to
unary exclamation mark
/Users/andreas/QEMU/qemu/arch_init.c:133: error: invalid operands to
binary && (have 'int' and '__vector __bool int')
make[1]: *** [arch_init.o] Error 1
make: *** [subdir-i386-softmmu] Error 2

Any suggestion how to fix?

Andreas



Re: [Qemu-devel] [PATCH 4/4] ISCSI: If the device we open is a SMC device, then force the use of sg. We dont have any medium changer emulation so only passthrough via real sg or scsi-generic via iscsi

2012-05-27 Thread Andreas Färber
Am 26.05.2012 09:36, schrieb Paolo Bonzini:
> Il 26/05/2012 06:56, Ronnie Sahlberg ha scritto:
>> Forcing sg also makes qemu skip trying to read from the device to guess the 
>> image format by reading from the device (find_image_format()).
>> SMC devices do not implement READ6/10/12/16  so it is noit possible to read 
>> from them.
>>
>> With this patch I can successfully manage a SMC device wiht iscsi in 
>> passthrough mode.
>>
>> Signed-off-by: Ronnie Sahlberg 
[...]
> Modified to also do the same for tapes, applied to scsi-next branch for 1.2.

Paolo, it seems you haven't pushed scsi-next since then. I hope you've
also shortened the subject to a humanly bearable length?

Ronnie, please restrict commit message lines to at most 76 characters.
You can check by running `git log` in an 80-char-wide terminal. The
subject line is especially sensitive since this email subject is simply
unreadable. And Patchwork currently looks really ugly in WebKit due to
this patch subject.

https://live.gnome.org/Git/CommitMessages

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg



Re: [Qemu-devel] Build fails on OS X Lion

2012-05-27 Thread Andreas Färber
Am 27.05.2012 07:58, schrieb dunrong huang:
> 2012/5/27 Charlie Somerville :
>> Hi qemu-devel,
>>
>> I tried to build the 1.1.0-rc3 on Mac OS X Lion, and I get this compile
>> error:
>>
>> qga/commands-posix.c: In function ‘qmp_guest_shutdown’:
>> qga/commands-posix.c:65: error: ‘environ’ undeclared (first use in this
>> function)
>> qga/commands-posix.c:65: error: (Each undeclared identifier is reported only
>> once
>> qga/commands-posix.c:65: error: for each function it appears in.)
>>
>> If I hack in an 'extern char** environ;' to the top of that file, the build
>> proceeds without error.
> 
> Luiz Capitulino has fixed this bug, but it haven't be merged into master 
> branch.
> 
> More details, see:
> http://lists.nongnu.org/archive/html/qemu-devel/2012-05/msg03578.html

Hm, I'm less certain that fix is going to work for Darwin...

I remember using a code snippet #define environ *_NSGetEnviron() or so
on Darwin.

Will check.

Andreas



Re: [Qemu-devel] [PATCH v2 01/17] Openrisc: add target stubs

2012-05-27 Thread Andreas Färber
Am 27.05.2012 07:32, schrieb Jia Liu:
> add openrisc target stubs.
> 
> Signed-off-by: Jia Liu 

Minor nitpick: I'd recommend to stick to the typographic conventions
outlined here:
https://live.gnome.org/Git/CommitMessages

In particular please start the sentence with a capital A. GNOME
recommend a lowercase topic (we usually use the file/directory mainly
affected) and uppercase beginning of the actual description, e.g.

target-or32: Add target stubs

Add OpenRISC target stubs.

Signed-off-by: ...

Writing it that way is not mandatory but when you're reposting and
fixing the English grammar you can just as well make it perfect. ;)

As Stefan pointed out, www.opencores.org writes it as OpenRISC, not
Openrisc. I saw no prominent notice whether OpenRISC may be a trademark
but better to respect their naming, seeing all the misspellings of QEMU.

[...]
> diff --git a/target-openrisc/cpu.c b/target-openrisc/cpu.c
> new file mode 100644
> index 000..ef3ffb1
> --- /dev/null
> +++ b/target-openrisc/cpu.c
> @@ -0,0 +1,24 @@
> +/*
> + *  QEMU Openrisc CPU
> + *
> + *  Copyright (c) 2012 Jia Liu 
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library 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
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see 
> .
> + */
> +
> +#include "cpu.h"
> +#include "qemu-common.h"
> +#if !defined(CONFIG_USER_ONLY)
> +#include "hw/loader.h"
> +#endif

Missing TypeInfo, missing class_init, missing initfn (where you might
want to move the openrisc_translate_init() call btw, following Igor's
example), missing reset function. This cannot all be deferred to a later
patch.

> diff --git a/target-openrisc/cpu.h b/target-openrisc/cpu.h
> new file mode 100644
> index 000..80018df
> --- /dev/null
> +++ b/target-openrisc/cpu.h
> @@ -0,0 +1,184 @@
> +/*
> + *  Openrisc virtual CPU header.
> + *
> + *  Copyright (c) 2011-2012 Jia Liu 
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library 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
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see 
> .
> + */
> +
> +#ifndef CPU_OPENRISC_H
> +#define CPU_OPENRISC_H
> +
> +#define TARGET_HAS_ICE 1
> +
> +#define ELF_MACHINE EM_OPENRISC
> +
> +#define CPUArchState struct CPUOpenriscState
> +
> +#define TARGET_LONG_BITS 32
> +
> +#include "config.h"
> +#include "qemu-common.h"
> +#include "cpu-defs.h"
> +#include "softfloat.h"
> +#include "qemu/cpu.h"
> +
> +struct CPUOpenriscState;
> +
> +#define NB_MMU_MODES 3
> +#define MMU_NOMMU_IDX   0
> +#define MMU_SUPERVISOR_IDX  1
> +#define MMU_USER_IDX2

Maybe make these three an enum?

> +
> +#define TARGET_PAGE_BITS 13
> +
> +#define TARGET_PHYS_ADDR_SPACE_BITS 32
> +#define TARGET_VIRT_ADDR_SPACE_BITS 32
> +
> +/* Verison Register */
> +#define SPR_VR   0x1201
> +#define SPR_CPUCFGR  0x1201
> +
> +/* Registers */
> +enum {
> +R0 = 0, R1, R2, R3, R4, R5, R6, R7, R8, R9, R10,
> +R11, R12, R13, R14, R15, R16, R17, R18, R19, R20,
> +R21, R22, R23, R24, R25, R26, R27, R28, R29, R30,
> +R31
> +};
> +
> +/* Register aliases */
> +enum {
> +R_ZERO = R0,
> +R_SP = R1,
> +R_FP = R2,
> +R_LR = R9,
> +R_RV = R11,
> +R_RVH = R12
> +};
> +
> +typedef struct CPUOpenriscState CPUOpenriscState;
> +struct CPUOpenriscState {
> +target_ulong gpr[32];   /* General registers */
> +
> +CPU_COMMON
> +
> +target_ulong pc;/* Program counter */
> +target_ulong npc;   /* Next PC */
> +target_ulong ppc;   /* Prev PC */
> +target_ulong jmp_pc;/* Jump PC */
> +uint32_t flags;
> +/* Branch. */
> +uint32_t btaken;/* the SR_F bit */
> +};

Why are pc, etc. placed after CPU_COMMON? Are they not supposed to be reset?

> +
> +#define TYPE_OPENRISC_CPU "or32-cpu"
> +
> +#define OPENRISC_CPU_CLASS(klass) \
> +OBJECT_CLASS_CHECK(OpenriscCPUClass, (klass), TYPE_OPENRISC_CPU)
> +#define OPENRISC_CPU(