A collection of bug fixes for rc2...

The following changes since commit 146aa0f104bb3bf88e43c4082a0bfc4bbda4fbd8:

Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2020-04-03 15:30:11 +0100)

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

for you to fetch changes up to 8893790966d9c964557ad01be4a68ef50696ace8:

dma/xlnx-zdma: Reorg to fix CUR_DSCR (2020-04-06 10:59:56 +0100)

* don't expose "ieee_half" via gdbstub (prevents gdb crashes or errors

with older GDB versions)

* hw/arm/collie: Put StrongARMState* into a CollieMachineState struct

* PSTATE.PAN should not clear exec bits

* hw/gpio/aspeed_gpio.c: Don't directly include assert.h

(fixes compilation on some Windows build scenarios)

* dump: Fix writing of ELF section

* dma/xlnx-zdma: various bug fixes

* target/arm/helperc. delete obsolete TODO comment

Alex Bennée (1):

target/arm: don't expose "ieee_half" via gdbstub

Edgar E. Iglesias (5):

dma/xlnx-zdma: Remove comment

dma/xlnx-zdma: Populate DBG0.CMN_BUF_FREE

dma/xlnx-zdma: Clear DMA_DONE when halting

dma/xlnx-zdma: Advance the descriptor address when stopping

dma/xlnx-zdma: Reorg to fix CUR_DSCR

hw/arm/collie: Put StrongARMState* into a CollieMachineState struct

target/arm: PSTATE.PAN should not clear exec bits

target/arm: Remove obsolete TODO note from get_phys_addr_lpae()

hw/gpio/aspeed_gpio.c: Don't directly include assert.h

dump: Fix writing of ELF section

dump/dump.c | 2 +-

hw/arm/collie.c | 33 +++++++++++++++++++++++++-----

hw/dma/xlnx-zdma.c | 56 ++++++++++++++++++++++++++-------------------------

hw/gpio/aspeed_gpio.c | 2 --

target/arm/gdbstub.c | 7 ++++++-

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

6 files changed, 69 insertions(+), 44 deletions(-)

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Clear DMA_DONE when halting the DMA channel.

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

Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>

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

Message-id: 20200402134721.27863-4-edgar.iglesias@gmail.com

hw/dma/xlnx-zdma.c | 1 +

diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c

--- a/hw/dma/xlnx-zdma.c

+++ b/hw/dma/xlnx-zdma.c

@@ -XXX,XX +XXX,XX @@ static void zdma_process_descr(XlnxZDMA *s)

if (src_cmd == CMD_HALT) {

zdma_set_state(s, PAUSED);

ARRAY_FIELD_DP32(s->regs, ZDMA_CH_ISR, DMA_PAUSE, 1);

+ ARRAY_FIELD_DP32(s->regs, ZDMA_CH_ISR, DMA_DONE, false);

zdma_ch_imr_update_irq(s);

return;

}

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Reorganize the descriptor handling so that CUR_DSCR always

points to the next descriptor to be processed.

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

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

Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>

Message-id: 20200402134721.27863-6-edgar.iglesias@gmail.com

hw/dma/xlnx-zdma.c | 47 ++++++++++++++++++++++------------------------

1 file changed, 22 insertions(+), 25 deletions(-)

diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c

--- a/hw/dma/xlnx-zdma.c

+++ b/hw/dma/xlnx-zdma.c

@@ -XXX,XX +XXX,XX @@ static void zdma_load_src_descriptor(XlnxZDMA *s)

+static void zdma_update_descr_addr(XlnxZDMA *s, bool type,

+ unsigned int basereg)

+{

+ uint64_t addr, next;

+

+ if (type == DTYPE_LINEAR) {

+ addr = zdma_get_regaddr64(s, basereg);

+ next = addr + sizeof(s->dsc_dst);

+ } else {

+ addr = zdma_get_regaddr64(s, basereg);

+ addr += sizeof(s->dsc_dst);

+ address_space_read(s->dma_as, addr, s->attr, (void *) &next, 8);

+ }

+

+ zdma_put_regaddr64(s, basereg, next);

+}

+

static void zdma_load_dst_descriptor(XlnxZDMA *s)

{

uint64_t dst_addr;

unsigned int ptype = ARRAY_FIELD_EX32(s->regs, ZDMA_CH_CTRL0, POINT_TYPE);

+ bool dst_type;

if (ptype == PT_REG) {

memcpy(&s->dsc_dst, &s->regs[R_ZDMA_CH_DST_DSCR_WORD0],

@@ -XXX,XX +XXX,XX @@ static void zdma_load_dst_descriptor(XlnxZDMA *s)

if (!zdma_load_descriptor(s, dst_addr, &s->dsc_dst)) {

ARRAY_FIELD_DP32(s->regs, ZDMA_CH_ISR, AXI_RD_DST_DSCR, true);

static void zdma_write_dst(XlnxZDMA *s, uint8_t *buf, uint32_t len)

@@ -XXX,XX +XXX,XX @@ static void zdma_write_dst(XlnxZDMA *s, uint8_t *buf, uint32_t len)

dst_size = FIELD_EX32(s->dsc_dst.words[2], ZDMA_CH_DST_DSCR_WORD2,

SIZE);

if (dst_size == 0 && ptype == PT_MEM) {

- uint64_t next;

- bool dst_type = FIELD_EX32(s->dsc_dst.words[3],

- ZDMA_CH_DST_DSCR_WORD3,

- TYPE);

-

- next = zdma_update_descr_addr(s, dst_type,

- R_ZDMA_CH_DST_CUR_DSCR_LSB);

- zdma_load_descriptor(s, next, &s->dsc_dst);

+ zdma_load_dst_descriptor(s);

dst_size = FIELD_EX32(s->dsc_dst.words[2], ZDMA_CH_DST_DSCR_WORD2,

SIZE);

}

Remove a direct include of assert.h -- this is already

provided by qemu/osdep.h, and it breaks our rule that the

first include must always be osdep.h.

In particular we must get the assert() macro via osdep.h

to avoid compile failures on mingw (see the comment in

osdep.h where we redefine assert() for that platform).

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

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

hw/gpio/aspeed_gpio.c | 2 --

1 file changed, 2 deletions(-)

diff --git a/hw/gpio/aspeed_gpio.c b/hw/gpio/aspeed_gpio.c

--- a/hw/gpio/aspeed_gpio.c

+++ b/hw/gpio/aspeed_gpio.c

* SPDX-License-Identifier: GPL-2.0-or-later

*/

-#include <assert.h>

#include "qemu/osdep.h"

#include "qemu/host-utils.h"

#include "qemu/log.h"

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Advance the descriptor address when stopping the channel.

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

Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>

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

Message-id: 20200402134721.27863-5-edgar.iglesias@gmail.com

hw/dma/xlnx-zdma.c | 1 -

1 file changed, 1 deletion(-)

diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c

--- a/hw/dma/xlnx-zdma.c

+++ b/hw/dma/xlnx-zdma.c

@@ -XXX,XX +XXX,XX @@ static void zdma_process_descr(XlnxZDMA *s)

if (ptype == PT_REG || src_cmd == CMD_STOP) {

ARRAY_FIELD_DP32(s->regs, ZDMA_CH_CTRL2, EN, 0);

zdma_set_state(s, DISABLED);

- return;

}

if (src_cmd == CMD_HALT) {

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Populate DBG0.CMN_BUF_FREE so that SW can see some free space.

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

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

Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>

Message-id: 20200402134721.27863-3-edgar.iglesias@gmail.com

hw/dma/xlnx-zdma.c | 6 ++++++

1 file changed, 6 insertions(+)

diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c

--- a/hw/dma/xlnx-zdma.c

+++ b/hw/dma/xlnx-zdma.c

@@ -XXX,XX +XXX,XX @@ static RegisterAccessInfo zdma_regs_info[] = {

},{ .name = "ZDMA_CH_DBG0", .addr = A_ZDMA_CH_DBG0,

.rsvd = 0xfffffe00,

.ro = 0x1ff,

+

+ /*

+ * There's SW out there that will check the debug regs for free space.

+ * Claim that we always have 0x100 free.

+ */

+ .reset = 0x100

},{ .name = "ZDMA_CH_DBG1", .addr = A_ZDMA_CH_DBG1,

.rsvd = 0xfffffe00,

.ro = 0x1ff,

From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>

Remove comment.

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

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

Reviewed-by: Francisco Iglesias <frasse.iglesias@gmail.com>

Message-id: 20200402134721.27863-2-edgar.iglesias@gmail.com

hw/dma/xlnx-zdma.c | 1 -

1 file changed, 1 deletion(-)

diff --git a/hw/dma/xlnx-zdma.c b/hw/dma/xlnx-zdma.c

--- a/hw/dma/xlnx-zdma.c

+++ b/hw/dma/xlnx-zdma.c

@@ -XXX,XX +XXX,XX @@ static void zdma_process_descr(XlnxZDMA *s)

zdma_src_done(s);

- /* Load next descriptor. */

if (ptype == PT_REG || src_cmd == CMD_STOP) {

ARRAY_FIELD_DP32(s->regs, ZDMA_CH_CTRL2, EN, 0);

zdma_set_state(s, DISABLED);

An old comment in get_phys_addr_lpae() claims that the code does not

support the different format TCR for VTCR_EL2. This used to be true

but it is not true now (in particular the aa64_va_parameters() and

aa32_va_parameters() functions correctly handle the different

register format by checking whether the mmu_idx is Stage2).

Remove the out of date parts of the comment.

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

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

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

1 file changed, 1 insertion(+), 6 deletions(-)

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

index XXXXXXX..XXXXXXX 100644

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

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

@@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,

bool aarch64 = arm_el_is_aa64(env, el);

bool guarded = false;

- /* TODO:

- * This code does not handle the different format TCR for VTCR_EL2.

- * This code also does not support shareability levels.

- * Attribute and permission bit handling should also be checked when adding

- * support for those page table walks.

- */

+ /* TODO: This code does not support shareability levels. */

if (aarch64) {

param = aa64_va_parameters(env, address, mmu_idx,

access_type != MMU_INST_FETCH);

Coverity complains that the collie_init() function leaks the memory

allocated in sa1110_init(). This is true but not significant since

the function is called only once on machine init and the memory must

remain in existence until QEMU exits anyway.

Still, we can avoid the technical memory leak by keeping the pointer

to the StrongARMState inside the machine state struct. Switch from

the simple DEFINE_MACHINE() style to defining a subclass of

TYPE_MACHINE which extends the MachineState struct, and keep the

pointer there.

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

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

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

hw/arm/collie.c | 33 ++++++++++++++++++++++++++++-----

1 file changed, 28 insertions(+), 5 deletions(-)

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

--- a/hw/arm/collie.c

+++ b/hw/arm/collie.c

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

#include "exec/address-spaces.h"

#include "cpu.h"

+typedef struct {

+ MachineState parent;

+ StrongARMState *sa1110;

+} CollieMachineState;

+

+#define TYPE_COLLIE_MACHINE MACHINE_TYPE_NAME("collie")

+#define COLLIE_MACHINE(obj) \

+ OBJECT_CHECK(CollieMachineState, obj, TYPE_COLLIE_MACHINE)

+

static struct arm_boot_info collie_binfo = {

.loader_start = SA_SDCS0,

.ram_size = 0x20000000,

@@ -XXX,XX +XXX,XX @@ static struct arm_boot_info collie_binfo = {

static void collie_init(MachineState *machine)

{

- StrongARMState *s;

DriveInfo *dinfo;

MachineClass *mc = MACHINE_GET_CLASS(machine);

+ CollieMachineState *cms = COLLIE_MACHINE(machine);

if (machine->ram_size != mc->default_ram_size) {

char *sz = size_to_str(mc->default_ram_size);

@@ -XXX,XX +XXX,XX @@ static void collie_init(MachineState *machine)

exit(EXIT_FAILURE);

}

- s = sa1110_init(machine->cpu_type);

+ cms->sa1110 = sa1110_init(machine->cpu_type);

memory_region_add_subregion(get_system_memory(), SA_SDCS0, machine->ram);

@@ -XXX,XX +XXX,XX @@ static void collie_init(MachineState *machine)

sysbus_create_simple("scoop", 0x40800000, NULL);

collie_binfo.board_id = 0x208;

- arm_load_kernel(s->cpu, machine, &collie_binfo);

+ arm_load_kernel(cms->sa1110->cpu, machine, &collie_binfo);

Our implementation of the PSTATE.PAN bit incorrectly cleared all

access permission bits for privileged access to memory which is

user-accessible. It should only affect the privileged read and write

permissions; execute permission is dealt with via XN/PXN instead.

Fixes: 81636b70c226dc27d7ebc8d

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

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

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

1 file changed, 4 insertions(+), 2 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 int get_S1prot(CPUARMState *env, ARMMMUIdx mmu_idx, bool is_aa64,

prot_rw = user_rw;

if (user_rw && regime_is_pan(env, mmu_idx)) {

- return 0;

+ /* PAN forbids data accesses but doesn't affect insn fetch */

+ prot_rw = 0;

+ prot_rw = simple_ap_to_rw_prot_is_user(ap, false);

}

- prot_rw = simple_ap_to_rw_prot_is_user(ap, false);

if (ns && arm_is_secure(env) && (env->cp15.scr_el3 & SCR_SIF)) {