[Qemu-devel] [PULL 1.1 0/6] Missing patches for QEMU 1.1

[Qemu-devel] [PULL 1.1 0/6] Missing patches for QEMU 1.1

[Stefan Weil]

Hi Anthony,

here are some of my patches which were not committed up to now,
but which should be included in QEMU 1.1.

Only the last one (qemu-timer, which did not get any review up to now)
is w32 related. The first one was discussed a lot, but then forgotten.

All other patches were reviewed and acked. 

Regards,

Stefan Weil

The following changes since commit 58a331bab950e9a12da8b17b57e11927893eaf5e:

  Merge remote-tracking branch 'kwolf/for-anthony' into staging (2012-05-02 14:49:53 -0500)

are available in the git repository at:


  git://qemu.weilnetz.de/qemu.git fixes

for you to fetch changes up to 40f08e87f613273f9dcc8df7d6a0f574b7d28d05:

  qemu-timer: Fix limits for w32 mmtimer (2012-05-03 07:04:48 +0200)

----------------------------------------------------------------
Stefan Weil (6):
      target-mips: Remove unused inline function
      arm-semi: Rename SYS_XXX macros to TARGET_SYS_XXX (fixes compiler warning)
      qdev: Fix memory leak in function set_pci_devfn
      hw/pc_sysfw: Fix memory leak
      qom: Fix memory leak in function container_get
      qemu-timer: Fix limits for w32 mmtimer

 arm-semi.c              |   86 +++++++++++++++++++++++-----------------------
 hw/pc_sysfw.c           |    3 ++
 hw/qdev-properties.c    |    1 +
 qemu-timer.c            |   34 ++++++++-----------
 qom/container.c         |    2 +
 target-mips/op_helper.c |    6 ---
 6 files changed, 63 insertions(+), 69 deletions(-)

[Qemu-devel] [PATCH 1/6] target-mips: Remove unused inline function

[Stefan Weil]

Function set_HILO is not needed anywhere.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 target-mips/op_helper.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index 5627447..66037ac 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -192,12 +192,6 @@ static inline uint64_t get_HILO (void)
     return ((uint64_t)(env->active_tc.HI[0]) << 32) | (uint32_t)env->active_tc.LO[0];
 }
 
-static inline void set_HILO (uint64_t HILO)
-{
-    env->active_tc.LO[0] = (int32_t)HILO;
-    env->active_tc.HI[0] = (int32_t)(HILO >> 32);
-}
-
 static inline void set_HIT0_LO (target_ulong arg1, uint64_t HILO)
 {
     env->active_tc.LO[0] = (int32_t)(HILO & 0xFFFFFFFF);
-- 
1.7.9

[Qemu-devel] [PATCH 2/6] arm-semi: Rename SYS_XXX macros to TARGET_SYS_XXX (fixes compiler warning)

[Stefan Weil]

SYS_OPEN is already defined in stdio.h of MinGW-w64,
therefore the compiler complains when building for w64.

Adding the prefix TARGET_ avoids that macro redefinition.
xtensa-semi.c also uses the same prefix (but mixed case macros
TARGET_SYS_xxx instead of TARGET_SYS_XXX).

Signed-off-by: Stefan Weil <sw@weilnetz.de>
Acked-by: Peter Maydell <peter.maydell@linaro.org>
---
 arm-semi.c |   86 ++++++++++++++++++++++++++++++------------------------------
 1 files changed, 43 insertions(+), 43 deletions(-)

diff --git a/arm-semi.c b/arm-semi.c
index 8debd19..88ca9bb 100644
--- a/arm-semi.c
+++ b/arm-semi.c
@@ -37,26 +37,26 @@
 #include "hw/arm-misc.h"
 #endif
 
-#define SYS_OPEN        0x01
-#define SYS_CLOSE       0x02
-#define SYS_WRITEC      0x03
-#define SYS_WRITE0      0x04
-#define SYS_WRITE       0x05
-#define SYS_READ        0x06
-#define SYS_READC       0x07
-#define SYS_ISTTY       0x09
-#define SYS_SEEK        0x0a
-#define SYS_FLEN        0x0c
-#define SYS_TMPNAM      0x0d
-#define SYS_REMOVE      0x0e
-#define SYS_RENAME      0x0f
-#define SYS_CLOCK       0x10
-#define SYS_TIME        0x11
-#define SYS_SYSTEM      0x12
-#define SYS_ERRNO       0x13
-#define SYS_GET_CMDLINE 0x15
-#define SYS_HEAPINFO    0x16
-#define SYS_EXIT        0x18
+#define TARGET_SYS_OPEN        0x01
+#define TARGET_SYS_CLOSE       0x02
+#define TARGET_SYS_WRITEC      0x03
+#define TARGET_SYS_WRITE0      0x04
+#define TARGET_SYS_WRITE       0x05
+#define TARGET_SYS_READ        0x06
+#define TARGET_SYS_READC       0x07
+#define TARGET_SYS_ISTTY       0x09
+#define TARGET_SYS_SEEK        0x0a
+#define TARGET_SYS_FLEN        0x0c
+#define TARGET_SYS_TMPNAM      0x0d
+#define TARGET_SYS_REMOVE      0x0e
+#define TARGET_SYS_RENAME      0x0f
+#define TARGET_SYS_CLOCK       0x10
+#define TARGET_SYS_TIME        0x11
+#define TARGET_SYS_SYSTEM      0x12
+#define TARGET_SYS_ERRNO       0x13
+#define TARGET_SYS_GET_CMDLINE 0x15
+#define TARGET_SYS_HEAPINFO    0x16
+#define TARGET_SYS_EXIT        0x18
 
 #ifndef O_BINARY
 #define O_BINARY 0
@@ -138,11 +138,11 @@ static void arm_semi_cb(CPUARMState *env, target_ulong ret, target_ulong err)
     } else {
         /* Fixup syscalls that use nonstardard return conventions.  */
         switch (env->regs[0]) {
-        case SYS_WRITE:
-        case SYS_READ:
+        case TARGET_SYS_WRITE:
+        case TARGET_SYS_READ:
             env->regs[0] = arm_semi_syscall_len - ret;
             break;
-        case SYS_SEEK:
+        case TARGET_SYS_SEEK:
             env->regs[0] = 0;
             break;
         default:
@@ -190,7 +190,7 @@ uint32_t do_arm_semihosting(CPUARMState *env)
     nr = env->regs[0];
     args = env->regs[1];
     switch (nr) {
-    case SYS_OPEN:
+    case TARGET_SYS_OPEN:
         if (!(s = lock_user_string(ARG(0))))
             /* FIXME - should this error code be -TARGET_EFAULT ? */
             return (uint32_t)-1;
@@ -211,14 +211,14 @@ uint32_t do_arm_semihosting(CPUARMState *env)
         }
         unlock_user(s, ARG(0), 0);
         return ret;
-    case SYS_CLOSE:
+    case TARGET_SYS_CLOSE:
         if (use_gdb_syscalls()) {
             gdb_do_syscall(arm_semi_cb, "close,%x", ARG(0));
             return env->regs[0];
         } else {
             return set_swi_errno(ts, close(ARG(0)));
         }
-    case SYS_WRITEC:
+    case TARGET_SYS_WRITEC:
         {
           char c;
 
@@ -233,7 +233,7 @@ uint32_t do_arm_semihosting(CPUARMState *env)
                 return write(STDERR_FILENO, &c, 1);
           }
         }
-    case SYS_WRITE0:
+    case TARGET_SYS_WRITE0:
         if (!(s = lock_user_string(args)))
             /* FIXME - should this error code be -TARGET_EFAULT ? */
             return (uint32_t)-1;
@@ -246,7 +246,7 @@ uint32_t do_arm_semihosting(CPUARMState *env)
         }
         unlock_user(s, args, 0);
         return ret;
-    case SYS_WRITE:
+    case TARGET_SYS_WRITE:
         len = ARG(2);
         if (use_gdb_syscalls()) {
             arm_semi_syscall_len = len;
@@ -262,7 +262,7 @@ uint32_t do_arm_semihosting(CPUARMState *env)
                 return -1;
             return len - ret;
         }
-    case SYS_READ:
+    case TARGET_SYS_READ:
         len = ARG(2);
         if (use_gdb_syscalls()) {
             arm_semi_syscall_len = len;
@@ -280,17 +280,17 @@ uint32_t do_arm_semihosting(CPUARMState *env)
                 return -1;
             return len - ret;
         }
-    case SYS_READC:
+    case TARGET_SYS_READC:
        /* XXX: Read from debug cosole. Not implemented.  */
         return 0;
-    case SYS_ISTTY:
+    case TARGET_SYS_ISTTY:
         if (use_gdb_syscalls()) {
             gdb_do_syscall(arm_semi_cb, "isatty,%x", ARG(0));
             return env->regs[0];
         } else {
             return isatty(ARG(0));
         }
-    case SYS_SEEK:
+    case TARGET_SYS_SEEK:
         if (use_gdb_syscalls()) {
             gdb_do_syscall(arm_semi_cb, "lseek,%x,%x,0", ARG(0), ARG(1));
             return env->regs[0];
@@ -300,7 +300,7 @@ uint32_t do_arm_semihosting(CPUARMState *env)
               return -1;
             return 0;
         }
-    case SYS_FLEN:
+    case TARGET_SYS_FLEN:
         if (use_gdb_syscalls()) {
             gdb_do_syscall(arm_semi_flen_cb, "fstat,%x,%x",
 			   ARG(0), env->regs[13]-64);
@@ -312,10 +312,10 @@ uint32_t do_arm_semihosting(CPUARMState *env)
                 return -1;
             return buf.st_size;
         }
-    case SYS_TMPNAM:
+    case TARGET_SYS_TMPNAM:
         /* XXX: Not implemented.  */
         return -1;
-    case SYS_REMOVE:
+    case TARGET_SYS_REMOVE:
         if (use_gdb_syscalls()) {
             gdb_do_syscall(arm_semi_cb, "unlink,%s", ARG(0), (int)ARG(1)+1);
             ret = env->regs[0];
@@ -327,7 +327,7 @@ uint32_t do_arm_semihosting(CPUARMState *env)
             unlock_user(s, ARG(0), 0);
         }
         return ret;
-    case SYS_RENAME:
+    case TARGET_SYS_RENAME:
         if (use_gdb_syscalls()) {
             gdb_do_syscall(arm_semi_cb, "rename,%s,%s",
                            ARG(0), (int)ARG(1)+1, ARG(2), (int)ARG(3)+1);
@@ -347,11 +347,11 @@ uint32_t do_arm_semihosting(CPUARMState *env)
                 unlock_user(s, ARG(0), 0);
             return ret;
         }
-    case SYS_CLOCK:
+    case TARGET_SYS_CLOCK:
         return clock() / (CLOCKS_PER_SEC / 100);
-    case SYS_TIME:
+    case TARGET_SYS_TIME:
         return set_swi_errno(ts, time(NULL));
-    case SYS_SYSTEM:
+    case TARGET_SYS_SYSTEM:
         if (use_gdb_syscalls()) {
             gdb_do_syscall(arm_semi_cb, "system,%s", ARG(0), (int)ARG(1)+1);
             return env->regs[0];
@@ -363,13 +363,13 @@ uint32_t do_arm_semihosting(CPUARMState *env)
             unlock_user(s, ARG(0), 0);
             return ret;
         }
-    case SYS_ERRNO:
+    case TARGET_SYS_ERRNO:
 #ifdef CONFIG_USER_ONLY
         return ts->swi_errno;
 #else
         return syscall_err;
 #endif
-    case SYS_GET_CMDLINE:
+    case TARGET_SYS_GET_CMDLINE:
         {
             /* Build a command-line from the original argv.
              *
@@ -452,7 +452,7 @@ uint32_t do_arm_semihosting(CPUARMState *env)
 
             return status;
         }
-    case SYS_HEAPINFO:
+    case TARGET_SYS_HEAPINFO:
         {
             uint32_t *ptr;
             uint32_t limit;
@@ -498,7 +498,7 @@ uint32_t do_arm_semihosting(CPUARMState *env)
 #endif
             return 0;
         }
-    case SYS_EXIT:
+    case TARGET_SYS_EXIT:
         gdb_exit(env, 0);
         exit(0);
     default:
-- 
1.7.9

[Qemu-devel] [PATCH 3/6] qdev: Fix memory leak in function set_pci_devfn

[Stefan Weil]

Valgrind reported this memory leak which occured very often.

Test scenario:

qemu-system-i386 (no arguments), only BIOS started, terminate with
monitor command (quit).

v2:
Use error_free instead of g_free (hint from Andreas Färber, thanks).

Signed-off-by: Stefan Weil <sw@weilnetz.de>
Acked-by: Andreas Färber <afaerber@suse.de>
---
 hw/qdev-properties.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c
index 98dd06a..c5545dc 100644
--- a/hw/qdev-properties.c
+++ b/hw/qdev-properties.c
@@ -834,6 +834,7 @@ static void set_pci_devfn(Object *obj, Visitor *v, void *opaque,
 
     visit_type_str(v, &str, name, &local_err);
     if (local_err) {
+        error_free(local_err);
         return set_int32(obj, v, opaque, name, errp);
     }
 
-- 
1.7.9

[Qemu-devel] [PATCH 4/6] hw/pc_sysfw: Fix memory leak

[Stefan Weil]

Valgrind reported this memory leak which occured a few times.

Test scenario:

qemu-system-i386 (no arguments), only BIOS started, terminate with
monitor command (quit).

Signed-off-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: Andreas Färber <afaerber@suse.de>
---
 hw/pc_sysfw.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/hw/pc_sysfw.c b/hw/pc_sysfw.c
index fafdf9b..f0d7c21 100644
--- a/hw/pc_sysfw.c
+++ b/hw/pc_sysfw.c
@@ -85,6 +85,9 @@ static void pc_fw_add_pflash_drv(void)
     filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
 
     opts = drive_add(IF_PFLASH, -1, filename, "readonly=on");
+
+    g_free(filename);
+
     if (opts == NULL) {
       return;
     }
-- 
1.7.9

[Qemu-devel] [PATCH 5/6] qom: Fix memory leak in function container_get

[Stefan Weil]

Valgrind reported this memory leak which occured very often.

Test scenario:

qemu-system-i386 (no arguments), only BIOS started, terminate with
monitor command (quit).

Signed-off-by: Stefan Weil <sw@weilnetz.de>
Reviewed-by: Andreas Färber <afaerber@suse.de>
---
 qom/container.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/qom/container.c b/qom/container.c
index c9940ab..4ca8b5c 100644
--- a/qom/container.c
+++ b/qom/container.c
@@ -43,6 +43,8 @@ Object *container_get(Object *root, const char *path)
         }
     }
 
+    g_strfreev(parts);
+
     return obj;
 }
 
-- 
1.7.9

[Qemu-devel] [PATCH 6/6] qemu-timer: Fix limits for w32 mmtimer

[Stefan Weil]

timeSetEvent only accepts delays in the range which is returned by
timeGetDevCaps.

The lower limit is typically 1 (= 1 ms), so the constant value of 1
in the old code usually worked.

The upper limit can be as low as 10000 ms, so the latest changes in
QEMU's timer handling which introduced timeout values above that limit
could result in failures of timeSetEvent when the timer was re-armed.

Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 qemu-timer.c |   34 ++++++++++++++--------------------
 1 files changed, 14 insertions(+), 20 deletions(-)

diff --git a/qemu-timer.c b/qemu-timer.c
index 8eadd16..b9fd75d 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -611,7 +611,7 @@ static void unix_stop_timer(struct qemu_alarm_timer *t)
 #ifdef _WIN32
 
 static MMRESULT mm_timer;
-static unsigned mm_period;
+static TIMECAPS mm_tc;
 
 static void CALLBACK mm_alarm_handler(UINT uTimerID, UINT uMsg,
                                       DWORD_PTR dwUser, DWORD_PTR dw1,
@@ -628,16 +628,12 @@ static void CALLBACK mm_alarm_handler(UINT uTimerID, UINT uMsg,
 
 static int mm_start_timer(struct qemu_alarm_timer *t)
 {
-    TIMECAPS tc;
+    timeGetDevCaps(&mm_tc, sizeof(mm_tc));
 
-    memset(&tc, 0, sizeof(tc));
-    timeGetDevCaps(&tc, sizeof(tc));
+    timeBeginPeriod(mm_tc.wPeriodMin);
 
-    mm_period = tc.wPeriodMin;
-    timeBeginPeriod(mm_period);
-
-    mm_timer = timeSetEvent(1,                  /* interval (ms) */
-                            mm_period,          /* resolution */
+    mm_timer = timeSetEvent(mm_tc.wPeriodMin,   /* interval (ms) */
+                            mm_tc.wPeriodMin,   /* resolution */
                             mm_alarm_handler,   /* function */
                             (DWORD_PTR)t,       /* parameter */
                             TIME_ONESHOT | TIME_CALLBACK_FUNCTION);
@@ -645,7 +641,7 @@ static int mm_start_timer(struct qemu_alarm_timer *t)
     if (!mm_timer) {
         fprintf(stderr, "Failed to initialize win32 alarm timer: %ld\n",
                 GetLastError());
-        timeEndPeriod(mm_period);
+        timeEndPeriod(mm_tc.wPeriodMin);
         return -1;
     }
 
@@ -655,23 +651,21 @@ static int mm_start_timer(struct qemu_alarm_timer *t)
 static void mm_stop_timer(struct qemu_alarm_timer *t)
 {
     timeKillEvent(mm_timer);
-    timeEndPeriod(mm_period);
+    timeEndPeriod(mm_tc.wPeriodMin);
 }
 
 static void mm_rearm_timer(struct qemu_alarm_timer *t, int64_t delta)
 {
     int64_t nearest_delta_ms = delta / 1000000;
-    if (nearest_delta_ms < 1) {
-        nearest_delta_ms = 1;
-    }
-    /* UINT_MAX can be 32 bit */
-    if (nearest_delta_ms > UINT_MAX) {
-        nearest_delta_ms = UINT_MAX;
+    if (nearest_delta_ms < mm_tc.wPeriodMin) {
+        nearest_delta_ms = mm_tc.wPeriodMin;
+    } else if (nearest_delta_ms > mm_tc.wPeriodMax) {
+        nearest_delta_ms = mm_tc.wPeriodMax;
     }
 
     timeKillEvent(mm_timer);
-    mm_timer = timeSetEvent((unsigned int) nearest_delta_ms,
-                            mm_period,
+    mm_timer = timeSetEvent((UINT)nearest_delta_ms,
+                            mm_tc.wPeriodMin,
                             mm_alarm_handler,
                             (DWORD_PTR)t,
                             TIME_ONESHOT | TIME_CALLBACK_FUNCTION);
@@ -680,7 +674,7 @@ static void mm_rearm_timer(struct qemu_alarm_timer *t, int64_t delta)
         fprintf(stderr, "Failed to re-arm win32 alarm timer %ld\n",
                 GetLastError());
 
-        timeEndPeriod(mm_period);
+        timeEndPeriod(mm_tc.wPeriodMin);
         exit(1);
     }
 }
-- 
1.7.9

Re: [Qemu-devel] [PULL 1.1 0/6] Missing patches for QEMU 1.1

[Anthony Liguori]

On 05/03/2012 12:14 AM, Stefan Weil wrote:
> Hi Anthony,
>
> here are some of my patches which were not committed up to now,
> but which should be included in QEMU 1.1.
>
> Only the last one (qemu-timer, which did not get any review up to now)
> is w32 related. The first one was discussed a lot, but then forgotten.
>
> All other patches were reviewed and acked.

Pulled.  Thanks.

Regards,

Anthony Liguori

>
> Regards,
>
> Stefan Weil
>
> The following changes since commit 58a331bab950e9a12da8b17b57e11927893eaf5e:
>
>    Merge remote-tracking branch 'kwolf/for-anthony' into staging (2012-05-02 14:49:53 -0500)
>
> are available in the git repository at:
>
>
>    git://qemu.weilnetz.de/qemu.git fixes
>
> for you to fetch changes up to 40f08e87f613273f9dcc8df7d6a0f574b7d28d05:
>
>    qemu-timer: Fix limits for w32 mmtimer (2012-05-03 07:04:48 +0200)
>
> ----------------------------------------------------------------
> Stefan Weil (6):
>        target-mips: Remove unused inline function
>        arm-semi: Rename SYS_XXX macros to TARGET_SYS_XXX (fixes compiler warning)
>        qdev: Fix memory leak in function set_pci_devfn
>        hw/pc_sysfw: Fix memory leak
>        qom: Fix memory leak in function container_get
>        qemu-timer: Fix limits for w32 mmtimer
>
>   arm-semi.c              |   86 +++++++++++++++++++++++-----------------------
>   hw/pc_sysfw.c           |    3 ++
>   hw/qdev-properties.c    |    1 +
>   qemu-timer.c            |   34 ++++++++-----------
>   qom/container.c         |    2 +
>   target-mips/op_helper.c |    6 ---
>   6 files changed, 63 insertions(+), 69 deletions(-)
>
>
>