From: Richard Henderson <richard.henderson@linaro.org>

This is, after all, how we implement extract2 in tcg/aarch64.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Message-id: 20190514011129.11330-2-richard.henderson@linaro.org

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---

target/arm/translate-a64.c | 38 ++++++++++++++++++++------------------

1 file changed, 20 insertions(+), 18 deletions(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/translate-a64.c

+++ b/target/arm/translate-a64.c

@@ -XXX,XX +XXX,XX @@ static void disas_extract(DisasContext *s, uint32_t insn)

} else {

tcg_gen_ext32u_i64(tcg_rd, cpu_reg(s, rm));

}

- } else if (rm == rn) { /* ROR */

- tcg_rm = cpu_reg(s, rm);

- if (sf) {

- tcg_gen_rotri_i64(tcg_rd, tcg_rm, imm);

- } else {

- TCGv_i32 tmp = tcg_temp_new_i32();

- tcg_gen_extrl_i64_i32(tmp, tcg_rm);

- tcg_gen_rotri_i32(tmp, tmp, imm);

- tcg_gen_extu_i32_i64(tcg_rd, tmp);

- tcg_temp_free_i32(tmp);

- }

} else {

- tcg_rm = read_cpu_reg(s, rm, sf);

- tcg_rn = read_cpu_reg(s, rn, sf);

- tcg_gen_shri_i64(tcg_rm, tcg_rm, imm);

- tcg_gen_shli_i64(tcg_rn, tcg_rn, bitsize - imm);

- tcg_gen_or_i64(tcg_rd, tcg_rm, tcg_rn);

- if (!sf) {

- tcg_gen_ext32u_i64(tcg_rd, tcg_rd);

+ tcg_rm = cpu_reg(s, rm);

+ tcg_rn = cpu_reg(s, rn);

+

+ if (sf) {

+ /* Specialization to ROR happens in EXTRACT2. */

+ tcg_gen_extract2_i64(tcg_rd, tcg_rm, tcg_rn, imm);

+ } else {

+ TCGv_i32 t0 = tcg_temp_new_i32();

+

+ tcg_gen_extrl_i64_i32(t0, tcg_rm);

+ if (rm == rn) {

+ tcg_gen_rotri_i32(t0, t0, imm);

+ } else {

+ TCGv_i32 t1 = tcg_temp_new_i32();

+ tcg_gen_extrl_i64_i32(t1, tcg_rn);

+ tcg_gen_extract2_i32(t0, t0, t1, imm);

+ tcg_temp_free_i32(t1);

+ }

+ tcg_gen_extu_i32_i64(tcg_rd, t0);

+ tcg_temp_free_i32(t0);

}

}

}

--

2.20.1

From: Richard Henderson <richard.henderson@linaro.org>

The mask implied by the extract is redundant with the one

implied by the deposit. Also, fix spelling of BFXIL.

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

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Message-id: 20190514011129.11330-3-richard.henderson@linaro.org

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---

target/arm/translate-a64.c | 6 +++---

1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/translate-a64.c

+++ b/target/arm/translate-a64.c

@@ -XXX,XX +XXX,XX @@ static void disas_bitfield(DisasContext *s, uint32_t insn)

tcg_gen_extract_i64(tcg_rd, tcg_tmp, ri, len);

return;

}

- /* opc == 1, BXFIL fall through to deposit */

- tcg_gen_extract_i64(tcg_tmp, tcg_tmp, ri, len);

+ /* opc == 1, BFXIL fall through to deposit */

+ tcg_gen_shri_i64(tcg_tmp, tcg_tmp, ri);

pos = 0;

} else {

/* Handle the ri > si case with a deposit

@@ -XXX,XX +XXX,XX @@ static void disas_bitfield(DisasContext *s, uint32_t insn)

len = ri;

}

- if (opc == 1) { /* BFM, BXFIL */

+ if (opc == 1) { /* BFM, BFXIL */

tcg_gen_deposit_i64(tcg_rd, tcg_rd, tcg_tmp, pos, len);

} else {

/* SBFM or UBFM: We start with zero, and we haven't modified

--

2.20.1

From: Alistair Francis <alistair.francis@wdc.com>

Commit 89e68b575 "target/arm: Use vector operations for saturation"

causes this abort() when booting QEMU ARM with a Cortex-A15:

0 0x00007ffff4c2382f in raise () at /usr/lib/libc.so.6

1 0x00007ffff4c0e672 in abort () at /usr/lib/libc.so.6

2 0x00005555559c1839 in disas_neon_data_insn (insn=<optimized out>, s=<optimized out>) at ./target/arm/translate.c:6673

3 0x00005555559c1839 in disas_neon_data_insn (s=<optimized out>, insn=<optimized out>) at ./target/arm/translate.c:6386

4 0x00005555559cd8a4 in disas_arm_insn (insn=4081107068, s=0x7fffe59a9510) at ./target/arm/translate.c:9289

5 0x00005555559cd8a4 in arm_tr_translate_insn (dcbase=0x7fffe59a9510, cpu=<optimized out>) at ./target/arm/translate.c:13612

6 0x00005555558d1d39 in translator_loop (ops=0x5555561cc580 <arm_translator_ops>, db=0x7fffe59a9510, cpu=0x55555686a2f0, tb=<optimized out>, max_insns=<optimized out>) at ./accel/tcg/translator.c:96

7 0x00005555559d10d4 in gen_intermediate_code (cpu=cpu@entry=0x55555686a2f0, tb=tb@entry=0x7fffd7840080 <code_gen_buffer+126091347>, max_insns=max_insns@entry=512) at ./target/arm/translate.c:13901

8 0x00005555558d06b9 in tb_gen_code (cpu=cpu@entry=0x55555686a2f0, pc=3067096216, cs_base=0, flags=192, cflags=-16252928, cflags@entry=524288) at ./accel/tcg/translate-all.c:1736

9 0x00005555558ce467 in tb_find (cf_mask=524288, tb_exit=1, last_tb=0x7fffd783e640 <code_gen_buffer+126084627>, cpu=0x1) at ./accel/tcg/cpu-exec.c:407

10 0x00005555558ce467 in cpu_exec (cpu=cpu@entry=0x55555686a2f0) at ./accel/tcg/cpu-exec.c:728

11 0x000055555588b0cf in tcg_cpu_exec (cpu=0x55555686a2f0) at ./cpus.c:1431

12 0x000055555588d223 in qemu_tcg_cpu_thread_fn (arg=0x55555686a2f0) at ./cpus.c:1735

13 0x000055555588d223 in qemu_tcg_cpu_thread_fn (arg=arg@entry=0x55555686a2f0) at ./cpus.c:1709

14 0x0000555555d2629a in qemu_thread_start (args=<optimized out>) at ./util/qemu-thread-posix.c:502

15 0x00007ffff4db8a92 in start_thread () at /usr/lib/libpthread.

This patch ensures that we don't hit the abort() in the second switch

case in disas_neon_data_insn() as we will return from the first case.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

Tested-by: Alex Bennée <alex.bennee@linaro.org>

Message-id: ad91b397f360b2fc7f4087e476f7df5b04d42ddb.1558021877.git.alistair.francis@wdc.com

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---

target/arm/translate.c | 4 ++--

1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/translate.c b/target/arm/translate.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/translate.c

+++ b/target/arm/translate.c

@@ -XXX,XX +XXX,XX @@ static int disas_neon_data_insn(DisasContext *s, uint32_t insn)

tcg_gen_gvec_4(rd_ofs, offsetof(CPUARMState, vfp.qc),

rn_ofs, rm_ofs, vec_size, vec_size,

(u ? uqadd_op : sqadd_op) + size);

- break;

+ return 0;

case NEON_3R_VQSUB:

tcg_gen_gvec_4(rd_ofs, offsetof(CPUARMState, vfp.qc),

rn_ofs, rm_ofs, vec_size, vec_size,

(u ? uqsub_op : sqsub_op) + size);

- break;

+ return 0;

case NEON_3R_VMUL: /* VMUL */

if (u) {

--

2.20.1

The system_clock_scale global is used only by the armv7m systick

device; move the extern declaration to the armv7m_systick.h header,

and expand the comment to explain what it is and that it should

ideally be replaced with a different approach.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Message-id: 20190516163857.6430-2-peter.maydell@linaro.org

---

include/hw/arm/arm.h | 4 ----

include/hw/timer/armv7m_systick.h | 22 ++++++++++++++++++++++

2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/include/hw/arm/arm.h b/include/hw/arm/arm.h

index XXXXXXX..XXXXXXX 100644

--- a/include/hw/arm/arm.h

+++ b/include/hw/arm/arm.h

@@ -XXX,XX +XXX,XX @@ void arm_write_secure_board_setup_dummy_smc(ARMCPU *cpu,

const struct arm_boot_info *info,

hwaddr mvbar_addr);

-/* Multiplication factor to convert from system clock ticks to qemu timer

- ticks. */

-extern int system_clock_scale;

-

#endif /* HW_ARM_H */

diff --git a/include/hw/timer/armv7m_systick.h b/include/hw/timer/armv7m_systick.h

index XXXXXXX..XXXXXXX 100644

--- a/include/hw/timer/armv7m_systick.h

+++ b/include/hw/timer/armv7m_systick.h

@@ -XXX,XX +XXX,XX @@ typedef struct SysTickState {

qemu_irq irq;

} SysTickState;

+/*

+ * Multiplication factor to convert from system clock ticks to qemu timer

+ * ticks. This should be set (by board code, usually) to a value

+ * equal to NANOSECONDS_PER_SECOND / frq, where frq is the clock frequency

+ * in Hz of the CPU.

+ *

+ * This value is used by the systick device when it is running in

+ * its "use the CPU clock" mode (ie when SYST_CSR.CLKSOURCE == 1) to

+ * set how fast the timer should tick.

+ *

+ * TODO: we should refactor this so that rather than using a global

+ * we use a device property or something similar. This is complicated

+ * because (a) the property would need to be plumbed through from the

+ * board code down through various layers to the systick device

+ * and (b) the property needs to be modifiable after realize, because

+ * the stellaris board uses this to implement the behaviour where the

+ * guest can reprogram the PLL registers to downclock the CPU, and the

+ * systick device needs to react accordingly. Possibly this should

+ * be deferred until we have a good API for modelling clock trees.

+ */

+extern int system_clock_scale;

+

#endif

--

2.20.1

The hw/arm/arm.h header now only includes declarations relating

to boot.c code, so it is only needed by Arm board or SoC code.

Remove some unnecessary inclusions of it from target/arm files

and from hw/intc/armv7m_nvic.c.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Message-id: 20190516163857.6430-3-peter.maydell@linaro.org

---

hw/intc/armv7m_nvic.c | 1 -

target/arm/arm-semi.c | 1 -

target/arm/cpu.c | 1 -

target/arm/cpu64.c | 1 -

target/arm/kvm.c | 1 -

target/arm/kvm32.c | 1 -

target/arm/kvm64.c | 1 -

7 files changed, 7 deletions(-)

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/intc/armv7m_nvic.c

+++ b/hw/intc/armv7m_nvic.c

@@ -XXX,XX +XXX,XX @@

#include "cpu.h"

#include "hw/sysbus.h"

#include "qemu/timer.h"

-#include "hw/arm/arm.h"

#include "hw/intc/armv7m_nvic.h"

#include "target/arm/cpu.h"

#include "exec/exec-all.h"

diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/arm-semi.c

+++ b/target/arm/arm-semi.c

@@ -XXX,XX +XXX,XX @@

#else

#include "qemu-common.h"

#include "exec/gdbstub.h"

-#include "hw/arm/arm.h"

#include "qemu/cutils.h"

#endif

diff --git a/target/arm/cpu.c b/target/arm/cpu.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/cpu.c

+++ b/target/arm/cpu.c

@@ -XXX,XX +XXX,XX @@

#if !defined(CONFIG_USER_ONLY)

#include "hw/loader.h"

#endif

-#include "hw/arm/arm.h"

#include "sysemu/sysemu.h"

#include "sysemu/hw_accel.h"

#include "kvm_arm.h"

diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/cpu64.c

+++ b/target/arm/cpu64.c

@@ -XXX,XX +XXX,XX @@

#if !defined(CONFIG_USER_ONLY)

#include "hw/loader.h"

#endif

-#include "hw/arm/arm.h"

#include "sysemu/sysemu.h"

#include "sysemu/kvm.h"

#include "kvm_arm.h"

diff --git a/target/arm/kvm.c b/target/arm/kvm.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/kvm.c

+++ b/target/arm/kvm.c

@@ -XXX,XX +XXX,XX @@

#include "cpu.h"

#include "trace.h"

#include "internals.h"

-#include "hw/arm/arm.h"

#include "hw/pci/pci.h"

#include "exec/memattrs.h"

#include "exec/address-spaces.h"

diff --git a/target/arm/kvm32.c b/target/arm/kvm32.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/kvm32.c

+++ b/target/arm/kvm32.c

@@ -XXX,XX +XXX,XX @@

#include "sysemu/kvm.h"

#include "kvm_arm.h"

#include "internals.h"

-#include "hw/arm/arm.h"

#include "qemu/log.h"

static inline void set_feature(uint64_t *features, int feature)

diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c

index XXXXXXX..XXXXXXX 100644

--- a/target/arm/kvm64.c

+++ b/target/arm/kvm64.c

@@ -XXX,XX +XXX,XX @@

#include "sysemu/kvm.h"

#include "kvm_arm.h"

#include "internals.h"

-#include "hw/arm/arm.h"

static bool have_guest_debug;

--

2.20.1