target-arm queue: this time around is all small fixes

and changes.

The following changes since commit fec105c2abda8567ec15230429c41429b5ee307c:

Merge remote-tracking branch 'remotes/kraxel/tags/audio-20190828-pull-request' into staging (2019-09-03 14:03:15 +0100)

https://git.linaro.org/people/pmaydell/qemu-arm.git tags/pull-target-arm-20190903

for you to fetch changes up to 5e5584c89f36b302c666bc6db535fd3f7ff35ad2:

target/arm: Don't abort on M-profile exception return in linux-user mode (2019-09-03 16:20:35 +0100)

* Revert and correctly fix refactoring of unallocated_encoding()

* Take exceptions on ATS instructions when needed

* aspeed/timer: Provide back-pressure information for short periods

* memory: Remove unused memory_region_iommu_replay_all()

* hw/arm/smmuv3: Log a guest error when decoding an invalid STE

* hw/arm/smmuv3: Remove spurious error messages on IOVA invalidations

* target/arm: Fix SMMLS argument order

* hw/arm: Use ARM_CPU_TYPE_NAME() macro when appropriate

* hw/arm: Correct reference counting for creation of various objects

* includes: remove stale [smp|max]_cpus externs

* tcg/README: fix typo

* atomic_template: fix indentation in GEN_ATOMIC_HELPER

* include/exec/cpu-defs.h: fix typo

* target/arm: Free TCG temps in trans_VMOV_64_sp()

* target/arm: Don't abort on M-profile exception return in linux-user mode

Alex Bennée (2):

includes: remove stale [smp|max]_cpus externs

include/exec/cpu-defs.h: fix typo

Andrew Jeffery (1):

aspeed/timer: Provide back-pressure information for short periods

Emilio G. Cota (2):

tcg/README: fix typo s/afterwise/afterwards/

atomic_template: fix indentation in GEN_ATOMIC_HELPER

Eric Auger (3):

memory: Remove unused memory_region_iommu_replay_all()

hw/arm/smmuv3: Log a guest error when decoding an invalid STE

hw/arm/smmuv3: Remove spurious error messages on IOVA invalidations

Peter Maydell (4):

target/arm: Allow ARMCPRegInfo read/write functions to throw exceptions

target/arm: Take exceptions on ATS instructions when needed

target/arm: Free TCG temps in trans_VMOV_64_sp()

target/arm: Don't abort on M-profile exception return in linux-user mode

Philippe Mathieu-Daudé (6):

hw/arm: Use ARM_CPU_TYPE_NAME() macro when appropriate

hw/arm: Use object_initialize_child for correct reference counting

hw/arm: Use sysbus_init_child_obj for correct reference counting

hw/arm/fsl-imx: Add the cpu as child of the SoC object

hw/dma/xilinx_axi: Use object_initialize_child for correct ref. counting

hw/net/xilinx_axi: Use object_initialize_child for correct ref. counting

Revert "target/arm: Use unallocated_encoding for aarch32"

target/arm: Factor out unallocated_encoding for aarch32

target/arm: Fix SMMLS argument order

accel/tcg/atomic_template.h | 2 +-

hw/arm/smmuv3-internal.h | 1 +

include/exec/cpu-defs.h | 2 +-

include/exec/memory.h | 10 ----

include/sysemu/sysemu.h | 2 -

target/arm/cpu.h | 6 ++-

target/arm/translate-a64.h | 2 +

target/arm/translate.h | 2 -

hw/arm/allwinner-a10.c | 3 +-

hw/arm/cubieboard.c | 3 +-

hw/arm/digic.c | 3 +-

hw/arm/exynos4_boards.c | 4 +-

hw/arm/fsl-imx25.c | 4 +-

hw/arm/fsl-imx31.c | 4 +-

hw/arm/fsl-imx6.c | 3 +-

hw/arm/fsl-imx6ul.c | 2 +-

hw/arm/mcimx7d-sabre.c | 9 ++--

hw/arm/mps2-tz.c | 15 +++---

hw/arm/musca.c | 9 ++--

hw/arm/smmuv3.c | 18 ++++---

hw/arm/xlnx-zynqmp.c | 8 +--

hw/dma/xilinx_axidma.c | 16 +++---

hw/net/xilinx_axienet.c | 17 +++----

hw/timer/aspeed_timer.c | 17 ++++++-

memory.c | 9 ----

target/arm/helper.c | 107 +++++++++++++++++++++++++++++++++++------

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

target/arm/translate-vfp.inc.c | 2 +

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

tcg/README | 2 +-

30 files changed, 244 insertions(+), 101 deletions(-)

From: Eric Auger <eric.auger@redhat.com>

An IOVA/ASID invalidation is notified to all IOMMU Memory Regions

through smmuv3_inv_notifiers_iova/smmuv3_notify_iova.

When the notification occurs it is possible that some of the

PCIe devices associated to the notified regions do not have a

valid stream table entry. In that case we output a LOG_GUEST_ERROR

message, for example:

invalid sid=<SID> (L1STD span=0)

"smmuv3_notify_iova error decoding the configuration for iommu mr=<MR>

This is unfortunate as the user gets the impression that there

are some translation decoding errors whereas there are not.

This patch adds a new field in SMMUEventInfo that tells whether

the detection of an invalid STE must lead to an error report.

invalid_ste_allowed is set before doing the invalidations and

kept unset on actual translation.

The other configuration decoding error messages are kept since if the

STE is valid then the rest of the config must be correct.

Signed-off-by: Eric Auger <eric.auger@redhat.com>

Message-id: 20190822172350.12008-6-eric.auger@redhat.com

hw/arm/smmuv3-internal.h | 1 +

hw/arm/smmuv3.c | 19 +++++++++++--------

2 files changed, 12 insertions(+), 8 deletions(-)

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

uint32_t sid;

bool recorded;

bool record_trans_faults;

+ bool inval_ste_allowed;

union {

struct {

uint32_t ssid;

diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/arm/smmuv3.c

+++ b/hw/arm/smmuv3.c

@@ -XXX,XX +XXX,XX @@ static int decode_ste(SMMUv3State *s, SMMUTransCfg *cfg,

uint32_t config;

if (!STE_VALID(ste)) {

- qemu_log_mask(LOG_GUEST_ERROR, "invalid STE\n");

+ if (!event->inval_ste_allowed) {

+ qemu_log_mask(LOG_GUEST_ERROR, "invalid STE\n");

+ }

goto bad_ste;

}

@@ -XXX,XX +XXX,XX @@ static int smmu_find_ste(SMMUv3State *s, uint32_t sid, STE *ste,

if (!span) {

/* l2ptr is not valid */

- qemu_log_mask(LOG_GUEST_ERROR,

- "invalid sid=%d (L1STD span=0)\n", sid);

+ if (!event->inval_ste_allowed) {

+ qemu_log_mask(LOG_GUEST_ERROR,

+ "invalid sid=%d (L1STD span=0)\n", sid);

+ }

event->type = SMMU_EVT_C_BAD_STREAMID;

return -EINVAL;

}

@@ -XXX,XX +XXX,XX @@ static IOMMUTLBEntry smmuv3_translate(IOMMUMemoryRegion *mr, hwaddr addr,

SMMUDevice *sdev = container_of(mr, SMMUDevice, iommu);

SMMUv3State *s = sdev->smmu;

uint32_t sid = smmu_get_sid(sdev);

- SMMUEventInfo event = {.type = SMMU_EVT_NONE, .sid = sid};

+ SMMUEventInfo event = {.type = SMMU_EVT_NONE,

+ .sid = sid,

+ .inval_ste_allowed = false};

SMMUPTWEventInfo ptw_info = {};

SMMUTranslationStatus status;

SMMUState *bs = ARM_SMMU(s);

@@ -XXX,XX +XXX,XX @@ static void smmuv3_notify_iova(IOMMUMemoryRegion *mr,

dma_addr_t iova)

{

SMMUDevice *sdev = container_of(mr, SMMUDevice, iommu);

- SMMUEventInfo event = {};

+ SMMUEventInfo event = {.inval_ste_allowed = true};

SMMUTransTableInfo *tt;

SMMUTransCfg *cfg;

IOMMUTLBEntry entry;

cfg = smmuv3_get_config(sdev, &event);

if (!cfg) {

- qemu_log_mask(LOG_GUEST_ERROR,

- "%s error decoding the configuration for iommu mr=%s\n",

- __func__, mr->parent_obj.name);

return;

}

An attempt to do an exception-return (branch to one of the magic

addresses) in linux-user mode for M-profile should behave like

a normal branch, because linux-user mode is always going to be

in 'handler' mode. This used to work, but we broke it when we added

support for the M-profile security extension in commit d02a8698d7ae2bfed.

In that commit we allowed even handler-mode calls to magic return

values to be checked for and dealt with by causing an

EXCP_EXCEPTION_EXIT exception to be taken, because this is

needed for the FNC_RETURN return-from-non-secure-function-call

handling. For system mode we added a check in do_v7m_exception_exit()

to make any spurious calls from Handler mode behave correctly, but

forgot that linux-user mode would also be affected.

How an attempted return-from-non-secure-function-call in linux-user

mode should be handled is not clear -- on real hardware it would

result in return to secure code (not to the Linux kernel) which

could then handle the error in any way it chose. For QEMU we take

the simple approach of treating this erroneous return the same way

it would be handled on a CPU without the security extensions --

treat it as a normal branch.

The upshot of all this is that for linux-user mode we should never

do any of the bx_excret magic, so the code change is simple.

This ought to be a weird corner case that only affects broken guest

code (because Linux user processes should never be attempting to do

exception returns or NS function returns), except that the code that

assigns addresses in RAM for the process and stack in our linux-user

code does not attempt to avoid this magic address range, so

legitimate code attempting to return to a trampoline routine on the

stack can fall into this case. This change fixes those programs,

but we should also look at restricting the range of memory we

use for M-profile linux-user guests to the area that would be

real RAM in hardware.

Cc: qemu-stable@nongnu.org

Reported-by: Christophe Lyon <christophe.lyon@linaro.org>

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

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

Message-id: 20190822131534.16602-1-peter.maydell@linaro.org

Fixes: https://bugs.launchpad.net/qemu/+bug/1840922

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

---

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

1 file changed, 20 insertions(+), 1 deletion(-)

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

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

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

@@ -XXX,XX +XXX,XX @@ static inline void gen_bx(DisasContext *s, TCGv_i32 var)

store_cpu_field(var, thumb);

}

-/* Set PC and Thumb state from var. var is marked as dead.

+/*

+ * Set PC and Thumb state from var. var is marked as dead.

* For M-profile CPUs, include logic to detect exception-return

* branches and handle them. This is needed for Thumb POP/LDM to PC, LDR to PC,

* and BX reg, and no others, and happens only for code in Handler mode.

+ * The Security Extension also requires us to check for the FNC_RETURN

+ * which signals a function return from non-secure state; this can happen

+ * in both Handler and Thread mode.

+ * To avoid having to do multiple comparisons in inline generated code,

+ * we make the check we do here loose, so it will match for EXC_RETURN

+ * in Thread mode. For system emulation do_v7m_exception_exit() checks

+ * for these spurious cases and returns without doing anything (giving

+ * the same behaviour as for a branch to a non-magic address).

+ *

+ * In linux-user mode it is unclear what the right behaviour for an

+ * attempted FNC_RETURN should be, because in real hardware this will go

+ * directly to Secure code (ie not the Linux kernel) which will then treat

+ * the error in any way it chooses. For QEMU we opt to make the FNC_RETURN

+ * attempt behave the way it would on a CPU without the security extension,

+ * which is to say "like a normal branch". That means we can simply treat

+ * all branches as normal with no magic address behaviour.

*/

static inline void gen_bx_excret(DisasContext *s, TCGv_i32 var)

{

@@ -XXX,XX +XXX,XX @@ static inline void gen_bx_excret(DisasContext *s, TCGv_i32 var)

* s->base.is_jmp that we need to do the rest of the work later.

*/

gen_bx(s, var);

+#ifndef CONFIG_USER_ONLY

if (arm_dc_feature(s, ARM_FEATURE_M_SECURITY) ||

(s->v7m_handler_mode && arm_dc_feature(s, ARM_FEATURE_M))) {

s->base.is_jmp = DISAS_BX_EXCRET;

}

+#endif

}

static inline void gen_bx_excret_final_code(DisasContext *s)

Currently the only part of an ARMCPRegInfo which is allowed to cause

a CPU exception is the access function, which returns a value indicating

that some flavour of UNDEF should be generated.

For the ATS system instructions, we would like to conditionally

generate exceptions as part of the writefn, because some faults

during the page table walk (like external aborts) should cause

an exception to be raised rather than returning a value.

There are several ways we could do this:

* plumb the GETPC() value from the top level set_cp_reg/get_cp_reg

helper functions through into the readfn and writefn hooks

* add extra readfn_with_ra/writefn_with_ra hooks that take the GETPC()

value

* require the ATS instructions to provide a dummy accessfn,

which serves no purpose except to cause the code generation

to emit TCG ops to sync the CPU state

* add an ARM_CP_ flag to mark the ARMCPRegInfo as possibly

throwing an exception in its read/write hooks, and make the

codegen sync the CPU state before calling the hooks if the

flag is set

This patch opts for the last of these, as it is fairly simple

to implement and doesn't require invasive changes like updating

the readfn/writefn hook function prototype signature.

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

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

target/arm/cpu.h | 6 +++++-

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

target/arm/translate.c | 7 +++++++

3 files changed, 18 insertions(+), 1 deletion(-)

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

--- a/target/arm/cpu.h

+++ b/target/arm/cpu.h

@@ -XXX,XX +XXX,XX @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid)

* IO indicates that this register does I/O and therefore its accesses

* need to be surrounded by gen_io_start()/gen_io_end(). In particular,

* registers which implement clocks or timers require this.

+ * RAISES_EXC is for when the read or write hook might raise an exception;

+ * the generated code will synchronize the CPU state before calling the hook

+ * so that it is safe for the hook to call raise_exception().

*/

#define ARM_CP_SPECIAL 0x0001

#define ARM_CP_CONST 0x0002

@@ -XXX,XX +XXX,XX @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid)

#define ARM_CP_FPU 0x1000

#define ARM_CP_SVE 0x2000

#define ARM_CP_NO_GDB 0x4000

+#define ARM_CP_RAISES_EXC 0x8000

/* Used only as a terminator for ARMCPRegInfo lists */

#define ARM_CP_SENTINEL 0xffff

/* Mask of only the flag bits in a type field */

-#define ARM_CP_FLAG_MASK 0x70ff

+#define ARM_CP_FLAG_MASK 0xf0ff

/* Valid values for ARMCPRegInfo state field, indicating which of

* the AArch32 and AArch64 execution states this register is visible in.

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

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

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

@@ -XXX,XX +XXX,XX @@ static void handle_sys(DisasContext *s, uint32_t insn, bool isread,

tcg_temp_free_ptr(tmpptr);

tcg_temp_free_i32(tcg_syn);

tcg_temp_free_i32(tcg_isread);

+ } else if (ri->type & ARM_CP_RAISES_EXC) {

+ /*

+ * The readfn or writefn might raise an exception;

+ * synchronize the CPU state in case it does.

+ */

+ gen_a64_set_pc_im(s->pc_curr);

}

/* Handle special cases first */

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

tcg_temp_free_ptr(tmpptr);

tcg_temp_free_i32(tcg_syn);

tcg_temp_free_i32(tcg_isread);

+ } else if (ri->type & ARM_CP_RAISES_EXC) {

+ /*

+ * The readfn or writefn might raise an exception;

+ * synchronize the CPU state in case it does.

+ */

+ gen_set_condexec(s);

+ gen_set_pc_im(s, s->pc_curr);

}

/* Handle special cases first */

The function neon_store_reg32() doesn't free the TCG temp that it

is passed, so the caller must do that. We got this right in most

places but forgot to free the TCG temps in trans_VMOV_64_sp().

Cc: qemu-stable@nongnu.org

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

Message-id: 20190827121931.26836-1-peter.maydell@linaro.org

target/arm/translate-vfp.inc.c | 2 ++

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

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

+++ b/target/arm/translate-vfp.inc.c

@@ -XXX,XX +XXX,XX @@ static bool trans_VMOV_64_sp(DisasContext *s, arg_VMOV_64_sp *a)

/* gpreg to fpreg */

tmp = load_reg(s, a->rt);

neon_store_reg32(tmp, a->vm);

+ tcg_temp_free_i32(tmp);

tmp = load_reg(s, a->rt2);

neon_store_reg32(tmp, a->vm + 1);

+ tcg_temp_free_i32(tmp);

}

return true;

From: Alex Bennée <alex.bennee@linaro.org>

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

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

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

Message-id: 20190828165307.18321-10-alex.bennee@linaro.org

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

include/exec/cpu-defs.h | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/exec/cpu-defs.h b/include/exec/cpu-defs.h

--- a/include/exec/cpu-defs.h

+++ b/include/exec/cpu-defs.h

@@ -XXX,XX +XXX,XX @@ typedef struct CPUTLB { } CPUTLB;

#endif /* !CONFIG_USER_ONLY && CONFIG_TCG */

/*

- * This structure must be placed in ArchCPU immedately

+ * This structure must be placed in ArchCPU immediately

* before CPUArchState, as a field named "neg".

*/

typedef struct CPUNegativeOffsetState {

The translation table walk for an ATS instruction can result in

various faults. In general these are just reported back via the

PAR_EL1 fault status fields, but in some cases the architecture

requires that the fault is turned into an exception:

* synchronous stage 2 faults of any kind during AT S1E0* and

AT S1E1* instructions executed from NS EL1 fault to EL2 or EL3

* synchronous external aborts are taken as Data Abort exceptions

(This is documented in the v8A Arm ARM DDI0487A.e D5.2.11 and

G5.13.4.)

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

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

target/arm/helper.c | 107 +++++++++++++++++++++++++++++++++++++-------

1 file changed, 92 insertions(+), 15 deletions(-)

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

--- a/target/arm/helper.c

+++ b/target/arm/helper.c

@@ -XXX,XX +XXX,XX @@ static uint64_t do_ats_write(CPUARMState *env, uint64_t value,

ret = get_phys_addr(env, value, access_type, mmu_idx, &phys_addr, &attrs,

&prot, &page_size, &fi, &cacheattrs);

+ if (ret) {

+ /*

+ * Some kinds of translation fault must cause exceptions rather

+ * than being reported in the PAR.

+ */

+ int current_el = arm_current_el(env);

+ int target_el;

+ uint32_t syn, fsr, fsc;

+ bool take_exc = false;

+

+ if (fi.s1ptw && current_el == 1 && !arm_is_secure(env)

+ && (mmu_idx == ARMMMUIdx_S1NSE1 || mmu_idx == ARMMMUIdx_S1NSE0)) {

+ /*

+ * Synchronous stage 2 fault on an access made as part of the

+ * translation table walk for AT S1E0* or AT S1E1* insn

+ * executed from NS EL1. If this is a synchronous external abort

+ * and SCR_EL3.EA == 1, then we take a synchronous external abort

+ * to EL3. Otherwise the fault is taken as an exception to EL2,

+ * and HPFAR_EL2 holds the faulting IPA.

+ */

+ if (fi.type == ARMFault_SyncExternalOnWalk &&

+ (env->cp15.scr_el3 & SCR_EA)) {

+ target_el = 3;

+ } else {

+ env->cp15.hpfar_el2 = extract64(fi.s2addr, 12, 47) << 4;

+ target_el = 2;

+ }

+ take_exc = true;

+ } else if (fi.type == ARMFault_SyncExternalOnWalk) {

+ /*

+ * Synchronous external aborts during a translation table walk

+ * are taken as Data Abort exceptions.

+ */

+ if (fi.stage2) {

+ if (current_el == 3) {

+ target_el = 3;

+ } else {

+ target_el = 2;

+ }

+ } else {

+ target_el = exception_target_el(env);

+ }

+ take_exc = true;

+ }

+

+ if (take_exc) {

+ /* Construct FSR and FSC using same logic as arm_deliver_fault() */

+ if (target_el == 2 || arm_el_is_aa64(env, target_el) ||

+ arm_s1_regime_using_lpae_format(env, mmu_idx)) {

+ fsr = arm_fi_to_lfsc(&fi);

+ fsc = extract32(fsr, 0, 6);

+ } else {

+ fsr = arm_fi_to_sfsc(&fi);

+ fsc = 0x3f;

+ }

+ /*

+ * Report exception with ESR indicating a fault due to a

+ * translation table walk for a cache maintenance instruction.

+ */

+ syn = syn_data_abort_no_iss(current_el == target_el,

+ fi.ea, 1, fi.s1ptw, 1, fsc);

+ env->exception.vaddress = value;

+ env->exception.fsr = fsr;

+ raise_exception(env, EXCP_DATA_ABORT, syn, target_el);

+ }

+ }

+

if (is_a64(env)) {

format64 = true;

} else if (arm_feature(env, ARM_FEATURE_LPAE)) {

@@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo vapa_cp_reginfo[] = {

/* This underdecoding is safe because the reginfo is NO_RAW. */

{ .name = "ATS", .cp = 15, .crn = 7, .crm = 8, .opc1 = 0, .opc2 = CP_ANY,

.access = PL1_W, .accessfn = ats_access,

- .writefn = ats_write, .type = ARM_CP_NO_RAW },

+ .writefn = ats_write, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC },

#endif

REGINFO_SENTINEL

};

@@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo v8_cp_reginfo[] = {

/* 64 bit address translation operations */

{ .name = "AT_S1E1R", .state = ARM_CP_STATE_AA64,

.opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 0,

- .access = PL1_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 },

+ .access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC,

+ .writefn = ats_write64 },

{ .name = "AT_S1E1W", .state = ARM_CP_STATE_AA64,

.opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 1,

- .access = PL1_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 },

+ .access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC,

+ .writefn = ats_write64 },

{ .name = "AT_S1E0R", .state = ARM_CP_STATE_AA64,

.opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 2,

- .access = PL1_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 },

+ .access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC,

+ .writefn = ats_write64 },

{ .name = "AT_S1E0W", .state = ARM_CP_STATE_AA64,

.opc0 = 1, .opc1 = 0, .crn = 7, .crm = 8, .opc2 = 3,

- .access = PL1_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 },

+ .access = PL1_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC,

+ .writefn = ats_write64 },

{ .name = "AT_S12E1R", .state = ARM_CP_STATE_AA64,

.opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 4,

- .access = PL2_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 },

+ .access = PL2_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC,

+ .writefn = ats_write64 },

{ .name = "AT_S12E1W", .state = ARM_CP_STATE_AA64,

.opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 5,

- .access = PL2_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 },

+ .access = PL2_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC,

+ .writefn = ats_write64 },

{ .name = "AT_S12E0R", .state = ARM_CP_STATE_AA64,

.opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 6,

- .access = PL2_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 },

+ .access = PL2_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC,

+ .writefn = ats_write64 },

{ .name = "AT_S12E0W", .state = ARM_CP_STATE_AA64,

.opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 7,

- .access = PL2_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 },

+ .access = PL2_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC,

+ .writefn = ats_write64 },

/* AT S1E2* are elsewhere as they UNDEF from EL3 if EL2 is not present */

{ .name = "AT_S1E3R", .state = ARM_CP_STATE_AA64,

.opc0 = 1, .opc1 = 6, .crn = 7, .crm = 8, .opc2 = 0,

- .access = PL3_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 },

+ .access = PL3_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC,

+ .writefn = ats_write64 },

{ .name = "AT_S1E3W", .state = ARM_CP_STATE_AA64,

.opc0 = 1, .opc1 = 6, .crn = 7, .crm = 8, .opc2 = 1,

- .access = PL3_W, .type = ARM_CP_NO_RAW, .writefn = ats_write64 },

+ .access = PL3_W, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC,

+ .writefn = ats_write64 },

{ .name = "PAR_EL1", .state = ARM_CP_STATE_AA64,

.type = ARM_CP_ALIAS,

.opc0 = 3, .opc1 = 0, .crn = 7, .crm = 4, .opc2 = 0,

@@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo el2_cp_reginfo[] = {

{ .name = "AT_S1E2R", .state = ARM_CP_STATE_AA64,

.opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 0,

.access = PL2_W, .accessfn = at_s1e2_access,

- .type = ARM_CP_NO_RAW, .writefn = ats_write64 },

+ .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, .writefn = ats_write64 },

{ .name = "AT_S1E2W", .state = ARM_CP_STATE_AA64,

.opc0 = 1, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 1,

.access = PL2_W, .accessfn = at_s1e2_access,

- .type = ARM_CP_NO_RAW, .writefn = ats_write64 },

+ .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC, .writefn = ats_write64 },

/* The AArch32 ATS1H* operations are CONSTRAINED UNPREDICTABLE

* if EL2 is not implemented; we choose to UNDEF. Behaviour at EL3

* with SCR.NS == 0 outside Monitor mode is UNPREDICTABLE; we choose

@@ -XXX,XX +XXX,XX @@ static const ARMCPRegInfo el2_cp_reginfo[] = {

{ .name = "ATS1HR", .cp = 15, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 0,

.access = PL2_W,

- .writefn = ats1h_write, .type = ARM_CP_NO_RAW },

+ .writefn = ats1h_write, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC },

{ .name = "ATS1HW", .cp = 15, .opc1 = 4, .crn = 7, .crm = 8, .opc2 = 1,

.access = PL2_W,

- .writefn = ats1h_write, .type = ARM_CP_NO_RAW },

+ .writefn = ats1h_write, .type = ARM_CP_NO_RAW | ARM_CP_RAISES_EXC },

{ .name = "CNTHCTL_EL2", .state = ARM_CP_STATE_BOTH,

.opc0 = 3, .opc1 = 4, .crn = 14, .crm = 1, .opc2 = 0,

/* ARMv7 requires bit 0 and 1 to reset to 1. ARMv8 defines the

From: "Emilio G. Cota" <cota@braap.org>

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

Signed-off-by: Emilio G. Cota <cota@braap.org>

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

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

Message-id: 20190828165307.18321-8-alex.bennee@linaro.org

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

accel/tcg/atomic_template.h | 2 +-

diff --git a/accel/tcg/atomic_template.h b/accel/tcg/atomic_template.h

--- a/accel/tcg/atomic_template.h

+++ b/accel/tcg/atomic_template.h

@@ -XXX,XX +XXX,XX @@ ABI_TYPE ATOMIC_NAME(xchg)(CPUArchState *env, target_ulong addr,

#define GEN_ATOMIC_HELPER(X) \

ABI_TYPE ATOMIC_NAME(X)(CPUArchState *env, target_ulong addr, \

- ABI_TYPE val EXTRA_ARGS) \

+ ABI_TYPE val EXTRA_ARGS) \

{ \

ATOMIC_MMU_DECLS; \

DATA_TYPE *haddr = ATOMIC_MMU_LOOKUP; \

From: Andrew Jeffery <andrew@aj.id.au>

First up: This is not the way the hardware behaves.

However, it helps resolve real-world problems with short periods being

used under Linux. Commit 4451d3f59f2a ("clocksource/drivers/fttmr010:

Fix set_next_event handler") in Linux fixed the timer driver to

correctly schedule the next event for the Aspeed controller, and in

combination with 5daa8212c08e ("ARM: dts: aspeed: Describe random number

device") Linux will now set a timer with a period as low as 1us.

Configuring a qemu timer with such a short period results in spending

time handling the interrupt in the model rather than executing guest

code, leading to noticeable "sticky" behaviour in the guest.

The behaviour of Linux is correct with respect to the hardware, so we

need to improve our handling under emulation. The approach chosen is to

provide back-pressure information by calculating an acceptable minimum

number of ticks to be set on the model. Under Linux an additional read

is added in the timer configuration path to detect back-pressure, which

will never occur on hardware. However if back-pressure is observed, the

driver alerts the clock event subsystem, which then performs its own

next event dilation via a config option - d1748302f70b ("clockevents:

Make minimum delay adjustments configurable")

A minimum period of 5us was experimentally determined on a Lenovo

T480s, which I've increased to 20us for "safety".

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

Reviewed-by: Joel Stanley <joel@jms.id.au>

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

Tested-by: Joel Stanley <joel@jms.id.au>

Signed-off-by: Cédric Le Goater <clg@kaod.org>

Message-id: 20190704055150.4899-1-clg@kaod.org

[clg: - changed the computation of min_ticks to be done each time the

timer value is reloaded. It removes the ordering issue of the

timer and scu reset handlers but is slightly slower ]

- introduced TIMER_MIN_NS

- introduced calculate_min_ticks() ]

Signed-off-by: Cédric Le Goater <clg@kaod.org>

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

---

hw/timer/aspeed_timer.c | 17 ++++++++++++++++-

1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/hw/timer/aspeed_timer.c b/hw/timer/aspeed_timer.c

--- a/hw/timer/aspeed_timer.c

+++ b/hw/timer/aspeed_timer.c

@@ -XXX,XX +XXX,XX @@ enum timer_ctrl_op {

op_pulse_enable

};

+/*

+ * Minimum value of the reload register to filter out short period

+ * timers which have a noticeable impact in emulation. 5us should be

+ * enough, use 20us for "safety".

+ */

+#define TIMER_MIN_NS (20 * SCALE_US)

+

/**

* Avoid mutual references between AspeedTimerCtrlState and AspeedTimer

* structs, as it's a waste of memory. The ptimer BH callback needs to know

@@ -XXX,XX +XXX,XX @@ static inline uint32_t calculate_ticks(struct AspeedTimer *t, uint64_t now_ns)

return t->reload - MIN(t->reload, ticks);

+static uint32_t calculate_min_ticks(AspeedTimer *t, uint32_t value)

+ uint32_t rate = calculate_rate(t);

+ uint32_t min_ticks = muldiv64(TIMER_MIN_NS, rate, NANOSECONDS_PER_SECOND);

+

+ return value < min_ticks ? min_ticks : value;

static inline uint64_t calculate_time(struct AspeedTimer *t, uint32_t ticks)

uint64_t delta_ns;

@@ -XXX,XX +XXX,XX @@ static void aspeed_timer_set_value(AspeedTimerCtrlState *s, int timer, int reg,

switch (reg) {

case TIMER_REG_RELOAD:

old_reload = t->reload;

- t->reload = value;

+ t->reload = calculate_min_ticks(t, value);

/* If the reload value was not previously set, or zero, and

* the current value is valid, try to start the timer if it is

From: "Emilio G. Cota" <cota@braap.org>

Afterwise is "wise after the fact", as in "hindsight".

Here we meant "afterwards" (as in "subsequently"). Fix it.

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

Signed-off-by: Emilio G. Cota <cota@braap.org>

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

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

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

Message-id: 20190828165307.18321-7-alex.bennee@linaro.org

tcg/README | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tcg/README b/tcg/README

--- a/tcg/README

+++ b/tcg/README

@@ -XXX,XX +XXX,XX @@ This can be overridden using the following function modifiers:

canonical locations before calling the helper.

- TCG_CALL_NO_WRITE_GLOBALS means that the helper does not modify any globals.

They will only be saved to their canonical location before calling helpers,

- but they won't be reloaded afterwise.

+ but they won't be reloaded afterwards.

- TCG_CALL_NO_SIDE_EFFECTS means that the call to the function is removed if

the return value is not used.

The previous simplification got the order of operands to the

subtraction wrong. Since the 64-bit product is the subtrahend,

we must use a 64-bit subtract to properly compute the borrow

from the low-part of the product.

Fixes: 5f8cd06ebcf5 ("target/arm: Simplify SMMLA, SMMLAR, SMMLS, SMMLSR")

Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>

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

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

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

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

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

@@ -XXX,XX +XXX,XX @@ static void disas_arm_insn(DisasContext *s, unsigned int insn)

if (rd != 15) {

tmp3 = load_reg(s, rd);

if (insn & (1 << 6)) {

- tcg_gen_sub_i32(tmp, tmp, tmp3);

+ /*

+ * For SMMLS, we need a 64-bit subtract.

+ * Borrow caused by a non-zero multiplicand

+ * lowpart, and the correct result lowpart

+ * for rounding.

+ */

+ TCGv_i32 zero = tcg_const_i32(0);

+ tcg_gen_sub2_i32(tmp2, tmp, zero, tmp3,

+ tmp2, tmp);

+ tcg_temp_free_i32(zero);

} else {

tcg_gen_add_i32(tmp, tmp, tmp3);

}

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

if (insn & (1 << 20)) {

tcg_gen_add_i32(tmp, tmp, tmp3);

} else {

- tcg_gen_sub_i32(tmp, tmp, tmp3);

+ /*

+ * For SMMLS, we need a 64-bit subtract.

+ * Borrow caused by a non-zero multiplicand lowpart,

+ * and the correct result lowpart for rounding.

+ */

+ TCGv_i32 zero = tcg_const_i32(0);

+ tcg_gen_sub2_i32(tmp2, tmp, zero, tmp3, tmp2, tmp);

+ tcg_temp_free_i32(zero);

}

tcg_temp_free_i32(tmp3);

}

Make this a static function private to translate.c.

Thus we can use the same idiom between aarch64 and aarch32

without actually sharing function implementations.

Reviewed-by: Laurent Desnogues <laurent.desnogues@gmail.com>

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

target/arm/translate-vfp.inc.c | 3 +--

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

2 files changed, 13 insertions(+), 12 deletions(-)

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

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

+++ b/target/arm/translate-vfp.inc.c

@@ -XXX,XX +XXX,XX @@ static bool full_vfp_access_check(DisasContext *s, bool ignore_vfp_enabled)

if (!s->vfp_enabled && !ignore_vfp_enabled) {

assert(!arm_dc_feature(s, ARM_FEATURE_M));

- gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),

- default_exception_el(s));

+ unallocated_encoding(s);

return false;

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 void gen_exception_bkpt_insn(DisasContext *s, uint32_t syn)

s->base.is_jmp = DISAS_NORETURN;

+static void unallocated_encoding(DisasContext *s)

+{

+ /* Unallocated and reserved encodings are uncategorized */

+ gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),

+ default_exception_el(s));

+}

+

/* Force a TB lookup after an instruction that changes the CPU state. */

static inline void gen_lookup_tb(DisasContext *s)

{

@@ -XXX,XX +XXX,XX @@ static inline void gen_hlt(DisasContext *s, int imm)

return;

}

- gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),

- default_exception_el(s));

+ unallocated_encoding(s);

}

static inline void gen_add_data_offset(DisasContext *s, unsigned int insn,

@@ -XXX,XX +XXX,XX @@ static void gen_srs(DisasContext *s,

}

if (undef) {

- gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),

- default_exception_el(s));

+ unallocated_encoding(s);

return;

}

@@ -XXX,XX +XXX,XX @@ static void disas_arm_insn(DisasContext *s, unsigned int insn)

break;

default:

illegal_op:

- gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),

- default_exception_el(s));

+ unallocated_encoding(s);

break;

}

}

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

}

return;

illegal_op:

- gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),

- default_exception_el(s));

+ unallocated_encoding(s);

}

static void disas_thumb_insn(DisasContext *s, uint32_t insn)

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

return;

illegal_op:

undef:

- gen_exception_insn(s, s->pc_curr, EXCP_UDEF, syn_uncategorized(),

- default_exception_el(s));

+ unallocated_encoding(s);

}

static bool insn_crosses_page(CPUARMState *env, DisasContext *s)

This reverts commit 3cb36637157088892e9e33ddb1034bffd1251d3b.

Despite the fact that the text for the call to gen_exception_insn

is identical for aarch64 and aarch32, the implementation inside

gen_exception_insn is totally different.

This fixes exceptions raised from aarch64.

Reported-by: Laurent Desnogues <laurent.desnogues@gmail.com>

Reviewed-by: Laurent Desnogues <laurent.desnogues@gmail.com>

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

target/arm/translate-a64.h | 2 ++

target/arm/translate.h | 2 --

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

target/arm/translate-vfp.inc.c | 3 ++-

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

5 files changed, 21 insertions(+), 15 deletions(-)

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

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

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

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

#ifndef TARGET_ARM_TRANSLATE_A64_H

#define TARGET_ARM_TRANSLATE_A64_H

+void unallocated_encoding(DisasContext *s);

#define unsupported_encoding(s, insn) \

do { \

qemu_log_mask(LOG_UNIMP, \

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