[Qemu-devel] [PATCH v2 0/6] Clang patches

[Qemu-devel] [PATCH v2 0/6] Clang patches

[Blue Swirl]

With this patch set, I'm able to compile AREG0 free softmmu targets.

v1->v2: redo QAPI change (#3), new patches 5 and 6.

Blue Swirl (6):
  sparc: fix floppy TC line setup
  sparc: fix expression with uninitialized initial value
  qapi: avoid reserved keywords
  user: fix accidental AREG0 use
  xilinx_axienet: avoid useless self-assignment
  scsi-bus: remove overlapping entry

 hw/scsi-bus.c       |    1 -
 hw/sun4m.c          |   18 +++++++++++++-----
 hw/xilinx_axienet.c |    1 -
 linux-user/signal.c |    8 +++-----
 net/slirp.c         |    6 +++---
 scripts/qapi.py     |   16 ++++++++++++++++
 user-exec.c         |   17 ++++++++++++-----
 7 files changed, 47 insertions(+), 20 deletions(-)

-- 
1.7.2.5

[Qemu-devel] [PATCH v2 1/6] sparc: fix floppy TC line setup

[Blue Swirl]

The qemu_irq for Terminal Count (TC) line between FDC and Slavio misc
device was created only after use, spotted by Clang compiler. Also,
it was not created if the FDC didn't exist.

Rearrange code to fix order. Always create the TC line.

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 hw/sun4m.c |   18 +++++++++++++-----
 1 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/hw/sun4m.c b/hw/sun4m.c
index a959261..0f909b5 100644
--- a/hw/sun4m.c
+++ b/hw/sun4m.c
@@ -832,6 +832,10 @@ static void cpu_devinit(const char *cpu_model, unsigned int id,
     env->prom_addr = prom_addr;
 }
 
+static void dummy_fdc_tc(void *opaque, int irq, int level)
+{
+}
+
 static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, ram_addr_t RAM_size,
                           const char *boot_device,
                           const char *kernel_filename,
@@ -942,9 +946,6 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, ram_addr_t RAM_size,
               serial_hds[0], serial_hds[1], ESCC_CLOCK, 1);
 
     cpu_halt = qemu_allocate_irqs(cpu_halt_signal, NULL, 1);
-    slavio_misc_init(hwdef->slavio_base, hwdef->aux1_base, hwdef->aux2_base,
-                     slavio_irq[30], fdc_tc);
-
     if (hwdef->apc_base) {
         apc_init(hwdef->apc_base, cpu_halt[0]);
     }
@@ -955,8 +956,13 @@ static void sun4m_hw_init(const struct sun4m_hwdef *hwdef, ram_addr_t RAM_size,
         fd[0] = drive_get(IF_FLOPPY, 0, 0);
         sun4m_fdctrl_init(slavio_irq[22], hwdef->fd_base, fd,
                           &fdc_tc);
+    } else {
+        fdc_tc = *qemu_allocate_irqs(dummy_fdc_tc, NULL, 1);
     }
 
+    slavio_misc_init(hwdef->slavio_base, hwdef->aux1_base, hwdef->aux2_base,
+                     slavio_irq[30], fdc_tc);
+
     if (drive_get_max_bus(IF_SCSI) > 0) {
         fprintf(stderr, "qemu: too many SCSI bus\n");
         exit(1);
@@ -1772,16 +1778,18 @@ static void sun4c_hw_init(const struct sun4c_hwdef *hwdef, ram_addr_t RAM_size,
               slavio_irq[1], serial_hds[0], serial_hds[1],
               ESCC_CLOCK, 1);
 
-    slavio_misc_init(0, hwdef->aux1_base, 0, slavio_irq[1], fdc_tc);
-
     if (hwdef->fd_base != (target_phys_addr_t)-1) {
         /* there is zero or one floppy drive */
         memset(fd, 0, sizeof(fd));
         fd[0] = drive_get(IF_FLOPPY, 0, 0);
         sun4m_fdctrl_init(slavio_irq[1], hwdef->fd_base, fd,
                           &fdc_tc);
+    } else {
+        fdc_tc = *qemu_allocate_irqs(dummy_fdc_tc, NULL, 1);
     }
 
+    slavio_misc_init(0, hwdef->aux1_base, 0, slavio_irq[1], fdc_tc);
+
     if (drive_get_max_bus(IF_SCSI) > 0) {
         fprintf(stderr, "qemu: too many SCSI bus\n");
         exit(1);
-- 
1.7.2.5

[Qemu-devel] [PATCH v2 2/6] sparc: fix expression with uninitialized initial value

[Blue Swirl]

err was uninitialized, it's not OK to use |=. Spotted by Clang
compiler.

Fix by implementing the earlier statement which initializes the variable.

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 linux-user/signal.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/linux-user/signal.c b/linux-user/signal.c
index 9be5ac0..7869147 100644
--- a/linux-user/signal.c
+++ b/linux-user/signal.c
@@ -1844,7 +1844,7 @@ typedef struct {
 } __siginfo_t;
 
 typedef struct {
-        unsigned   long si_float_regs [32];
+        abi_ulong       si_float_regs[32];
         unsigned   long si_fsr;
         unsigned   long si_fpqdepth;
         struct {
@@ -2056,11 +2056,9 @@ restore_fpu_state(CPUSPARCState *env, qemu_siginfo_fpu_t *fpu)
                 return -EFAULT;
 #endif
 
-#if 0
         /* XXX: incorrect */
-        err = __copy_from_user(&env->fpr[0], &fpu->si_float_regs[0],
-	                             (sizeof(unsigned long) * 32));
-#endif
+        err = copy_from_user(&env->fpr[0], fpu->si_float_regs[0],
+                             (sizeof(abi_ulong) * 32));
         err |= __get_user(env->fsr, &fpu->si_fsr);
 #if 0
         err |= __get_user(current->thread.fpqdepth, &fpu->si_fpqdepth);
-- 
1.7.2.5

[Qemu-devel] [PATCH v2 3/6] qapi: avoid reserved keywords

[Blue Swirl]

Clang compiler complained about use of reserved word 'restrict' in SLIRP
and QAPI.

Prefix C keywords with "q_", adjust SLIRP accordingly.

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 net/slirp.c     |    6 +++---
 scripts/qapi.py |   16 ++++++++++++++++
 2 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/net/slirp.c b/net/slirp.c
index 5c2e6b2..b818ea2 100644
--- a/net/slirp.c
+++ b/net/slirp.c
@@ -722,9 +722,9 @@ int net_init_slirp(const NetClientOptions *opts, const char *name,
     net_init_slirp_configs(user->hostfwd, SLIRP_CFG_HOSTFWD);
     net_init_slirp_configs(user->guestfwd, 0);
 
-    ret = net_slirp_init(vlan, "user", name, user->restrict, vnet, user->host,
-                         user->hostname, user->tftp, user->bootfile,
-                         user->dhcpstart, user->dns, user->smb,
+    ret = net_slirp_init(vlan, "user", name, user->q_restrict, vnet,
+                         user->host, user->hostname, user->tftp,
+                         user->bootfile, user->dhcpstart, user->dns, user->smb,
                          user->smbserver);
 
     while (slirp_configs) {
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 8082af3..80f28f8 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -131,6 +131,22 @@ def camel_case(name):
     return new_name
 
 def c_var(name):
+    # ANSI X3J11/88-090, 3.1.1
+    c89_words = set(['auto', 'break', 'case', 'char', 'const', 'continue',
+                     'default', 'do', 'double', 'else', 'enum', 'extern', 'float',
+                     'for', 'goto', 'if', 'int', 'long', 'register', 'return',
+                     'short', 'signed', 'sizeof', 'static', 'struct', 'switch',
+                     'typedef', 'union', 'unsigned', 'void', 'volatile', 'while'])
+    # ISO/IEC 9899:1999, 6.4.1
+    c99_words = set(['inline', 'restrict', '_Bool', '_Complex', '_Imaginary'])
+    # ISO/IEC 9899:2011, 6.4.1
+    c11_words = set(['_Alignas', '_Alignof', '_Atomic', '_Generic', '_Noreturn',
+                     '_Static_assert', '_Thread_local'])
+    # GCC http://gcc.gnu.org/onlinedocs/gcc-4.7.1/gcc/C-Extensions.html
+    # excluding _.*
+    gcc_words = set(['asm', 'typeof'])
+    if name in c89_words | c99_words | c11_words | gcc_words:
+        return "q_" + name
     return name.replace('-', '_').lstrip("*")
 
 def c_fun(name):
-- 
1.7.2.5

Re: [Qemu-devel] [PATCH v2 3/6] qapi: avoid reserved keywords

[Markus Armbruster]

Blue Swirl <blauwirbel@gmail.com> writes:

> Clang compiler complained about use of reserved word 'restrict' in SLIRP
> and QAPI.
>
> Prefix C keywords with "q_", adjust SLIRP accordingly.
>
> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>

I like this solution.

Reviewed-by: Markus Armbruster <armbru@redhat.com>

Re: [Qemu-devel] [PATCH v2 3/6] qapi: avoid reserved keywords

[Luiz Capitulino]

On Thu, 02 Aug 2012 10:32:59 +0200
Markus Armbruster <armbru@redhat.com> wrote:

> Blue Swirl <blauwirbel@gmail.com> writes:
> 
> > Clang compiler complained about use of reserved word 'restrict' in SLIRP
> > and QAPI.
> >
> > Prefix C keywords with "q_", adjust SLIRP accordingly.
> >
> > Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
> 
> I like this solution.

Yeah, looks good.

Would be nice to add a note to docs/writing-qmp-commands.txt (can be in an
incremental patch).

> 
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
> 

[Qemu-devel] [PATCH v2 4/6] user: fix accidental AREG0 use

[Blue Swirl]

Global register AREG0 was always assumed to be usable in user-exec.c,
but this is incorrect for several targets.

Fix with #ifdeffery and by using other variables.

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 user-exec.c |   17 ++++++++++++-----
 1 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/user-exec.c b/user-exec.c
index 1a9c276..b9ea9dd 100644
--- a/user-exec.c
+++ b/user-exec.c
@@ -18,7 +18,9 @@
  */
 #include "config.h"
 #include "cpu.h"
+#ifndef CONFIG_TCG_PASS_AREG0
 #include "dyngen-exec.h"
+#endif
 #include "disas.h"
 #include "tcg.h"
 
@@ -58,9 +60,11 @@ void cpu_resume_from_signal(CPUArchState *env1, void *puc)
     struct sigcontext *uc = puc;
 #endif
 
+#ifndef CONFIG_TCG_PASS_AREG0
     env = env1;
 
     /* XXX: restore cpu registers saved in host registers */
+#endif
 
     if (puc) {
         /* XXX: use siglongjmp ? */
@@ -74,8 +78,8 @@ void cpu_resume_from_signal(CPUArchState *env1, void *puc)
         sigprocmask(SIG_SETMASK, &uc->sc_mask, NULL);
 #endif
     }
-    env->exception_index = -1;
-    longjmp(env->jmp_env, 1);
+    env1->exception_index = -1;
+    longjmp(env1->jmp_env, 1);
 }
 
 /* 'pc' is the host PC at which the exception was raised. 'address' is
@@ -89,9 +93,11 @@ static inline int handle_cpu_signal(uintptr_t pc, unsigned long address,
     TranslationBlock *tb;
     int ret;
 
+#ifndef CONFIG_TCG_PASS_AREG0
     if (cpu_single_env) {
         env = cpu_single_env; /* XXX: find a correct solution for multithread */
     }
+#endif
 #if defined(DEBUG_SIGNAL)
     qemu_printf("qemu: SIGSEGV pc=0x%08lx address=%08lx w=%d oldset=0x%08lx\n",
                 pc, address, is_write, *(unsigned long *)old_set);
@@ -103,7 +109,8 @@ static inline int handle_cpu_signal(uintptr_t pc, unsigned long address,
     }
 
     /* see if it is an MMU fault */
-    ret = cpu_handle_mmu_fault(env, address, is_write, MMU_USER_IDX);
+    ret = cpu_handle_mmu_fault(cpu_single_env, address, is_write,
+                               MMU_USER_IDX);
     if (ret < 0) {
         return 0; /* not an MMU fault */
     }
@@ -115,13 +122,13 @@ static inline int handle_cpu_signal(uintptr_t pc, unsigned long address,
     if (tb) {
         /* the PC is inside the translated code. It means that we have
            a virtual CPU fault */
-        cpu_restore_state(tb, env, pc);
+        cpu_restore_state(tb, cpu_single_env, pc);
     }
 
     /* we restore the process signal mask as the sigreturn should
        do it (XXX: use sigsetjmp) */
     sigprocmask(SIG_SETMASK, old_set, NULL);
-    exception_action(env);
+    exception_action(cpu_single_env);
 
     /* never comes here */
     return 1;
-- 
1.7.2.5

[Qemu-devel] [PATCH v2 5/6] xilinx_axienet: avoid useless self-assignment

[Blue Swirl]

Statement s=s; makes little sense, remove it. Spotted by Clang
compiler.

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 hw/xilinx_axienet.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/hw/xilinx_axienet.c b/hw/xilinx_axienet.c
index e948505..e89a174 100644
--- a/hw/xilinx_axienet.c
+++ b/hw/xilinx_axienet.c
@@ -648,7 +648,6 @@ static ssize_t eth_rx(VLANClientState *nc, const uint8_t *buf, size_t size)
     uint16_t csum16;
     int i;
 
-    s = s;
     DENET(qemu_log("%s: %zd bytes\n", __func__, size));
 
     unicast = ~buf[0] & 0x1;
-- 
1.7.2.5

[Qemu-devel] [PATCH v2 6/6] scsi-bus: remove overlapping entry

[Blue Swirl]

LOAD_UNLOAD and START_STOP have same value, so the table
entry is initialized twice. Spotted by Clang compiler.

Remove LOAD_UNLOAD entry since START_STOP entry already
represents both.

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
 hw/scsi-bus.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
index e4ec19e..a8759bc 100644
--- a/hw/scsi-bus.c
+++ b/hw/scsi-bus.c
@@ -1355,7 +1355,6 @@ static const char *scsi_command_name(uint8_t cmd)
         [ BLANK                    ] = "BLANK",
         [ MOVE_MEDIUM              ] = "MOVE_MEDIUM",
         [ EXCHANGE_MEDIUM          ] = "EXCHANGE MEDIUM",
-        [ LOAD_UNLOAD              ] = "LOAD_UNLOAD",
         [ READ_12                  ] = "READ_12",
         [ WRITE_12                 ] = "WRITE_12",
         [ ERASE_12                 ] = "ERASE_12/GET_PERFORMANCE",
-- 
1.7.2.5

Re: [Qemu-devel] [PATCH v2 6/6] scsi-bus: remove overlapping entry

[Markus Armbruster]

[cc: SCSI maintainer]

Blue Swirl <blauwirbel@gmail.com> writes:

> LOAD_UNLOAD and START_STOP have same value, so the table
> entry is initialized twice. Spotted by Clang compiler.
>
> Remove LOAD_UNLOAD entry since START_STOP entry already
> represents both.
>
> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
> ---
>  hw/scsi-bus.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/hw/scsi-bus.c b/hw/scsi-bus.c
> index e4ec19e..a8759bc 100644
> --- a/hw/scsi-bus.c
> +++ b/hw/scsi-bus.c
> @@ -1355,7 +1355,6 @@ static const char *scsi_command_name(uint8_t cmd)
>          [ BLANK                    ] = "BLANK",
>          [ MOVE_MEDIUM              ] = "MOVE_MEDIUM",
>          [ EXCHANGE_MEDIUM          ] = "EXCHANGE MEDIUM",
> -        [ LOAD_UNLOAD              ] = "LOAD_UNLOAD",
>          [ READ_12                  ] = "READ_12",
>          [ WRITE_12                 ] = "WRITE_12",
>          [ ERASE_12                 ] = "ERASE_12/GET_PERFORMANCE",

Missed in commit 15e58a21.

Reviewed-by: Markus Armbruster <armbru@redhat.com>