We incorrectly treat SVC 0xf0002 as a cacheflush request (which is a
NOP for QEMU).  This is the wrong syscall number, because in the
svc-immediate OABI syscall numbers are all offset by the
ARM_SYSCALL_BASE value and so the correct insn is SVC 0x9f0002.
(This is handled further down in the code with the other Arm-specific
syscalls like NR_breakpoint.)

When this code was initially added in commit 6f1f31c069b20611 in
2004, ARM_NR_cacheflush was defined as (ARM_SYSCALL_BASE + 0xf0000 + 2)
so the value in the comparison took account of the extra 0x900000
offset. In commit fbb4a2e371f2fa7 in 2008, the ARM_SYSCALL_BASE
was removed from the definition of ARM_NR_cacheflush and handling
for this group of syscalls was added below the point where we subtract
ARM_SYSCALL_BASE from the SVC immediate value. However that commit
forgot to remove the now-obsolete earlier handling code.

Remove the spurious ARM_NR_cacheflush condition.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 linux-user/arm/cpu_loop.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c
index 82d0dd3c312..025887d6b86 100644
--- a/linux-user/arm/cpu_loop.c
+++ b/linux-user/arm/cpu_loop.c
@@ -308,9 +308,7 @@ void cpu_loop(CPUARMState *env)
                     n = insn & 0xffffff;
                 }
 
-                if (n == ARM_NR_cacheflush) {
-                    /* nop */
-                } else if (n == 0 || n >= ARM_SYSCALL_BASE || env->thumb) {
+                if (n == 0 || n >= ARM_SYSCALL_BASE || env->thumb) {
                     /* linux syscall */
                     if (env->thumb || n == 0) {
                         n = env->regs[7];
-- 
2.20.1

On 4/20/20 11:22 PM, Peter Maydell wrote:
> We incorrectly treat SVC 0xf0002 as a cacheflush request (which is a
> NOP for QEMU).  This is the wrong syscall number, because in the
> svc-immediate OABI syscall numbers are all offset by the
> ARM_SYSCALL_BASE value and so the correct insn is SVC 0x9f0002.
> (This is handled further down in the code with the other Arm-specific
> syscalls like NR_breakpoint.)
> 
> When this code was initially added in commit 6f1f31c069b20611 in
> 2004, ARM_NR_cacheflush was defined as (ARM_SYSCALL_BASE + 0xf0000 + 2)
> so the value in the comparison took account of the extra 0x900000
> offset. In commit fbb4a2e371f2fa7 in 2008, the ARM_SYSCALL_BASE
> was removed from the definition of ARM_NR_cacheflush and handling
> for this group of syscalls was added below the point where we subtract
> ARM_SYSCALL_BASE from the SVC immediate value. However that commit
> forgot to remove the now-obsolete earlier handling code.

I imagine you wrote this patch wearing an archeologist hat =)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> 
> Remove the spurious ARM_NR_cacheflush condition.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  linux-user/arm/cpu_loop.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c
> index 82d0dd3c312..025887d6b86 100644
> --- a/linux-user/arm/cpu_loop.c
> +++ b/linux-user/arm/cpu_loop.c
> @@ -308,9 +308,7 @@ void cpu_loop(CPUARMState *env)
>                      n = insn & 0xffffff;
>                  }
>  
> -                if (n == ARM_NR_cacheflush) {
> -                    /* nop */
> -                } else if (n == 0 || n >= ARM_SYSCALL_BASE || env->thumb) {
> +                if (n == 0 || n >= ARM_SYSCALL_BASE || env->thumb) {
>                      /* linux syscall */
>                      if (env->thumb || n == 0) {
>                          n = env->regs[7];
> 

On Mon, Apr 20, 2020 at 10:22:04PM +0100, Peter Maydell wrote:
> We incorrectly treat SVC 0xf0002 as a cacheflush request (which is a
> NOP for QEMU).  This is the wrong syscall number, because in the
> svc-immediate OABI syscall numbers are all offset by the
> ARM_SYSCALL_BASE value and so the correct insn is SVC 0x9f0002.
> (This is handled further down in the code with the other Arm-specific
> syscalls like NR_breakpoint.)
> 
> When this code was initially added in commit 6f1f31c069b20611 in
> 2004, ARM_NR_cacheflush was defined as (ARM_SYSCALL_BASE + 0xf0000 + 2)
> so the value in the comparison took account of the extra 0x900000
> offset. In commit fbb4a2e371f2fa7 in 2008, the ARM_SYSCALL_BASE
> was removed from the definition of ARM_NR_cacheflush and handling
> for this group of syscalls was added below the point where we subtract
> ARM_SYSCALL_BASE from the SVC immediate value. However that commit
> forgot to remove the now-obsolete earlier handling code.
> 
> Remove the spurious ARM_NR_cacheflush condition.

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>


> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  linux-user/arm/cpu_loop.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/linux-user/arm/cpu_loop.c b/linux-user/arm/cpu_loop.c
> index 82d0dd3c312..025887d6b86 100644
> --- a/linux-user/arm/cpu_loop.c
> +++ b/linux-user/arm/cpu_loop.c
> @@ -308,9 +308,7 @@ void cpu_loop(CPUARMState *env)
>                      n = insn & 0xffffff;
>                  }
>  
> -                if (n == ARM_NR_cacheflush) {
> -                    /* nop */
> -                } else if (n == 0 || n >= ARM_SYSCALL_BASE || env->thumb) {
> +                if (n == 0 || n >= ARM_SYSCALL_BASE || env->thumb) {
>                      /* linux syscall */
>                      if (env->thumb || n == 0) {
>                          n = env->regs[7];
> -- 
> 2.20.1
> 
>