More arm patches (mostly the SDHCI ones from Philippe)

The following changes since commit f521eeee3bd060b460c99e605472b7e03967db43:

Merge remote-tracking branch 'remotes/juanquintela/tags/migration/20180115' into staging (2018-01-15 13:17:47 +0000)

are available in the git repository at:

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

for you to fetch changes up to 60765b6ceeb4998a0d4220b3a53f1f185061da77:

sdhci: add a 'dma' property to the sysbus devices (2018-01-16 13:28:21 +0000)

* SDHCI: cleanups and minor bug fixes

* target/arm: minor refactor preparatory to fp16 support

* omap_ssd, ssi-sd, pl181, milkymist-memcard: reset the SD

card on controller reset (fixes migration failures)

* target/arm: Handle page table walk load failures correctly

* hw/arm/virt: Add virt-2.12 machine type

* get_phys_addr_pmsav7: Support AP=0b111 for v7M

* hw/intc/armv7m: Support byte and halfword accesses to CFSR

Andrey Smirnov (1):

sdhci: Implement write method of ACMD12ERRSTS register

Peter Maydell (8):

hw/intc/armv7m: Support byte and halfword accesses to CFSR

get_phys_addr_pmsav7: Support AP=0b111 for v7M

hw/arm/virt: Add virt-2.12 machine type

target/arm: Handle page table walk load failures correctly

hw/sd/pl181: Reset SD card on controller reset

hw/sd/milkymist-memcard: Reset SD card on controller reset

hw/sd/ssi-sd: Reset SD card on controller reset

hw/sd/omap_mmc: Reset SD card on controller reset

Philippe Mathieu-Daudé (13):

sdhci: clean up includes

sdhci: remove dead code

sdhci: use DEFINE_SDHCI_COMMON_PROPERTIES() for common sysbus/pci properties

sdhci: refactor common sysbus/pci class_init() into sdhci_common_class_init()

sdhci: refactor common sysbus/pci realize() into sdhci_common_realize()

sdhci: refactor common sysbus/pci unrealize() into sdhci_common_unrealize()

sdhci: use qemu_log_mask(UNIMP) instead of fprintf()

sdhci: convert the DPRINT() calls into trace events

sdhci: move MASK_TRNMOD with other SDHC_TRN* defines in "sd-internal.h"

sdhci: rename the SDHC_CAPAB register

sdhci: fix CAPAB/MAXCURR registers, both are 64bit and read-only

sdhci: fix the PCI device, using the PCI address space for DMA

sdhci: add a 'dma' property to the sysbus devices

Richard Henderson (2):

target/arm: Split out vfp_expand_imm

target/arm: Add fp16 support to vfp_expand_imm

hw/sd/sdhci-internal.h | 7 +-

include/hw/sd/sdhci.h | 19 +++-

target/arm/internals.h | 10 ++

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

hw/intc/armv7m_nvic.c | 38 ++++---

hw/sd/milkymist-memcard.c | 4 +

hw/sd/omap_mmc.c | 14 ++-

hw/sd/pl181.c | 4 +

hw/sd/sdhci.c | 266 +++++++++++++++++++++++++++------------------

hw/sd/ssi-sd.c | 25 ++++-

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

target/arm/op_helper.c | 7 +-

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

hw/sd/trace-events | 14 +++

14 files changed, 362 insertions(+), 167 deletions(-)

From: Andrey Smirnov <andrew.smirnov@gmail.com>

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>

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

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

Message-id: 20180115182436.2066-13-f4bug@amsat.org

hw/sd/sdhci.c | 3 +++

1 file changed, 3 insertions(+)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c

--- a/hw/sd/sdhci.c

+++ b/hw/sd/sdhci.c

@@ -XXX,XX +XXX,XX @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)

}

sdhci_update_irq(s);

break;

+ case SDHC_ACMD12ERRSTS:

+ MASKED_WRITE(s->acmd12errsts, mask, value);

+ break;

case SDHC_CAPAB:

case SDHC_CAPAB + 4:

2.7.4

Since omap_mmc is still using the legacy SD card API, the SD

card created by sd_init() is not plugged into any bus. This

means that the controller has to reset it manually.

Failing to do this mostly didn't affect the guest since the

guest typically does a programmed SD card reset as part of

its SD controller driver initialization, but would mean that

migration fails because it's only in sd_reset() that we

set up the wpgrps_size field.

hw/sd/omap_mmc.c | 14 ++++++++++----

1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/hw/sd/omap_mmc.c b/hw/sd/omap_mmc.c

--- a/hw/sd/omap_mmc.c

+++ b/hw/sd/omap_mmc.c

@@ -XXX,XX +XXX,XX @@ void omap_mmc_reset(struct omap_mmc_s *host)

host->cdet_enable = 0;

qemu_set_irq(host->coverswitch, host->cdet_state);

host->clkdiv = 0;

+

+ /* Since we're still using the legacy SD API the card is not plugged

+ * into any bus, and we must reset it manually. When omap_mmc is

+ * QOMified this must move into the QOM reset function.

+ */

+ device_reset(DEVICE(host->card));

static uint64_t omap_mmc_read(void *opaque, hwaddr offset,

@@ -XXX,XX +XXX,XX @@ struct omap_mmc_s *omap_mmc_init(hwaddr base,

s->lines = 1;    /* TODO: needs to be settable per-board */

s->rev = 1;

- omap_mmc_reset(s);

-

memory_region_init_io(&s->iomem, NULL, &omap_mmc_ops, s, "omap.mmc", 0x800);

memory_region_add_subregion(sysmem, base, &s->iomem);

@@ -XXX,XX +XXX,XX @@ struct omap_mmc_s *omap_mmc_init(hwaddr base,

+ omap_mmc_reset(s);

+

return s;

}

@@ -XXX,XX +XXX,XX @@ struct omap_mmc_s *omap2_mmc_init(struct omap_target_agent_s *ta,

s->lines = 4;

s->rev = 2;

- omap_mmc_reset(s);

memory_region_init_io(&s->iomem, NULL, &omap_mmc_ops, s, "omap.mmc",

omap_l4_region_size(ta, 0));

omap_l4_attach(ta, 0, &s->iomem);

@@ -XXX,XX +XXX,XX @@ struct omap_mmc_s *omap2_mmc_init(struct omap_target_agent_s *ta,

s->cdet = qemu_allocate_irq(omap_mmc_cover_cb, s, 0);

sd_set_cb(s->card, NULL, s->cdet);

+ omap_mmc_reset(s);

+

return s;

}

2.7.4

For PMSAv7, the v7A/R Arm ARM defines that setting AP to 0b111

is an UNPREDICTABLE reserved combination. However, for v7M

this value is documented as having the same behaviour as 0b110:

read-only for both privileged and unprivileged. Accept this

value on an M profile core rather than treating it as a guest

error and a no-access page.

Reported-by: Andy Gross <andy.gross@linaro.org>

target/arm/helper.c | 14 ++++++++++++++

1 file changed, 14 insertions(+)

@@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,

case 6:

*prot |= PAGE_READ | PAGE_EXEC;

break;

+ case 7:

+ /* for v7M, same as 6; for R profile a reserved value */

+ if (arm_feature(env, ARM_FEATURE_M)) {

+ *prot |= PAGE_READ | PAGE_EXEC;

+ break;

+ }

+ /* fall through */

default:

qemu_log_mask(LOG_GUEST_ERROR,

"DRACR[%d]: Bad value for AP bits: 0x%"

@@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_pmsav7(CPUARMState *env, uint32_t address,

case 6:

*prot |= PAGE_READ | PAGE_EXEC;

break;

+ case 7:

+ /* for v7M, same as 6; for R profile a reserved value */

+ if (arm_feature(env, ARM_FEATURE_M)) {

+ *prot |= PAGE_READ | PAGE_EXEC;

+ break;

+ }

+ /* fall through */

default:

qemu_log_mask(LOG_GUEST_ERROR,

"DRACR[%d]: Bad value for AP bits: 0x%"

2.7.4

The Configurable Fault Status Register for ARMv7M and v8M is

supposed to be byte and halfword accessible, but we were only

implementing word accesses. Add support for the other access

sizes, which are used by the Zephyr RTOS.

Reported-by: Andy Gross <andy.gross@linaro.org>

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

Message-id: 1512742372-31517-1-git-send-email-peter.maydell@linaro.org

hw/intc/armv7m_nvic.c | 38 ++++++++++++++++++++++----------------

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

val |= (1 << 8);

return val;

- case 0xd28: /* Configurable Fault Status. */

- /* The BFSR bits [15:8] are shared between security states

- * and we store them in the NS copy

- */

- val = cpu->env.v7m.cfsr[attrs.secure];

- val |= cpu->env.v7m.cfsr[M_REG_NS] & R_V7M_CFSR_BFSR_MASK;

- return val;

case 0xd2c: /* Hard Fault Status. */

return cpu->env.v7m.hfsr;

case 0xd30: /* Debug Fault Status. */

@@ -XXX,XX +XXX,XX @@ static void nvic_writel(NVICState *s, uint32_t offset, uint32_t value,

s->vectors[ARMV7M_EXCP_DEBUG].active = (value & (1 << 8)) != 0;

nvic_irq_update(s);

break;

- case 0xd28: /* Configurable Fault Status. */

- cpu->env.v7m.cfsr[attrs.secure] &= ~value; /* W1C */

- if (attrs.secure) {

- /* The BFSR bits [15:8] are shared between security states

- * and we store them in the NS copy.

- */

- cpu->env.v7m.cfsr[M_REG_NS] &= ~(value & R_V7M_CFSR_BFSR_MASK);

- }

- break;

case 0xd2c: /* Hard Fault Status. */

cpu->env.v7m.hfsr &= ~value; /* W1C */

break;

@@ -XXX,XX +XXX,XX @@ static MemTxResult nvic_sysreg_read(void *opaque, hwaddr addr,

val = deposit32(val, i * 8, 8, get_prio(s, hdlidx, sbank));

}

break;

+ case 0xd28 ... 0xd2b: /* Configurable Fault Status (CFSR) */

+ /* The BFSR bits [15:8] are shared between security states

+ * and we store them in the NS copy

+ */

+ val = s->cpu->env.v7m.cfsr[attrs.secure];

+ val |= s->cpu->env.v7m.cfsr[M_REG_NS] & R_V7M_CFSR_BFSR_MASK;

+ val = extract32(val, (offset - 0xd28) * 8, size * 8);

+ break;

case 0xfe0 ... 0xfff: /* ID. */

if (offset & 3) {

val = 0;

@@ -XXX,XX +XXX,XX @@ static MemTxResult nvic_sysreg_write(void *opaque, hwaddr addr,

}

nvic_irq_update(s);

return MEMTX_OK;

+ case 0xd28 ... 0xd2b: /* Configurable Fault Status (CFSR) */

+ /* All bits are W1C, so construct 32 bit value with 0s in

+ * the parts not written by the access size

+ */

+ value <<= ((offset - 0xd28) * 8);

+

+ s->cpu->env.v7m.cfsr[attrs.secure] &= ~value;

+ if (attrs.secure) {

+ /* The BFSR bits [15:8] are shared between security states

+ * and we store them in the NS copy.

+ */

+ s->cpu->env.v7m.cfsr[M_REG_NS] &= ~(value & R_V7M_CFSR_BFSR_MASK);

+ }

+ return MEMTX_OK;

}

if (size == 4) {

nvic_writel(s, offset, value, attrs);

2.7.4

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

running qtests:

$ make check-qtest-arm

GTESTER check-qtest-arm

SDHC rd_4b @0x44 not implemented

SDHC wr_4b @0x40 <- 0x89abcdef not implemented

SDHC wr_4b @0x44 <- 0x01234567 not implemented

include/hw/sd/sdhci.h | 4 ++--

hw/sd/sdhci.c | 23 +++++++++++++++++++----

2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h

--- a/include/hw/sd/sdhci.h

+++ b/include/hw/sd/sdhci.h

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

uint64_t admasysaddr; /* ADMA System Address Register */

/* Read-only registers */

- uint32_t capareg; /* Capabilities Register */

- uint32_t maxcurr; /* Maximum Current Capabilities Register */

+ uint64_t capareg; /* Capabilities Register */

+ uint64_t maxcurr; /* Maximum Current Capabilities Register */

uint8_t *fifo_buffer; /* SD host i/o FIFO buffer */

uint32_t buf_maxsz;

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c

index XXXXXXX..XXXXXXX 100644

--- a/hw/sd/sdhci.c

+++ b/hw/sd/sdhci.c

@@ -XXX,XX +XXX,XX @@ static uint64_t sdhci_read(void *opaque, hwaddr offset, unsigned size)

ret = s->acmd12errsts;

2.7.4

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

zero-initialize ADMADescr 'dscr' in sdhci_do_adma() to avoid:

hw/sd/sdhci.c: In function ‘sdhci_do_adma’:

hw/sd/sdhci.c:714:29: error: ‘dscr.addr’ may be used uninitialized in this function [-Werror=maybe-uninitialized]

trace_sdhci_adma("link", s->admasysaddr);

^

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

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

Message-id: 20180115182436.2066-9-f4bug@amsat.org

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

---

hw/sd/sdhci.c | 89 ++++++++++++++++++------------------------------------

hw/sd/trace-events | 14 +++++++++

2 files changed, 44 insertions(+), 59 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c

--- a/hw/sd/sdhci.c

+++ b/hw/sd/sdhci.c

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

#include "sdhci-internal.h"

#include "qapi/error.h"

#include "qemu/log.h"

-

-/* host controller debug messages */

-#ifndef SDHC_DEBUG

-#define SDHC_DEBUG 0

-#endif

-

-#define DPRINT_L1(fmt, args...) \

- do { \

- if (SDHC_DEBUG) { \

- fprintf(stderr, "QEMU SDHC: " fmt, ## args); \

- } \

- } while (0)

-#define DPRINT_L2(fmt, args...) \

- do { \

- if (SDHC_DEBUG > 1) { \

- fprintf(stderr, "QEMU SDHC: " fmt, ## args); \

- } \

- } while (0)

-#define ERRPRINT(fmt, args...) \

- do { \

- if (SDHC_DEBUG) { \

- fprintf(stderr, "QEMU SDHC ERROR: " fmt, ## args); \

- } \

- } while (0)

+#include "trace.h"

#define TYPE_SDHCI_BUS "sdhci-bus"

#define SDHCI_BUS(obj) OBJECT_CHECK(SDBus, (obj), TYPE_SDHCI_BUS)

@@ -XXX,XX +XXX,XX @@ static void sdhci_raise_insertion_irq(void *opaque)

static void sdhci_set_inserted(DeviceState *dev, bool level)

{

SDHCIState *s = (SDHCIState *)dev;

- DPRINT_L1("Card state changed: %s!\n", level ? "insert" : "eject");

+ trace_sdhci_set_inserted(level ? "insert" : "eject");

if ((s->norintsts & SDHC_NIS_REMOVE) && level) {

/* Give target some time to notice card ejection */

timer_mod(s->insert_timer,

@@ -XXX,XX +XXX,XX @@ static void sdhci_send_command(SDHCIState *s)

s->acmd12errsts = 0;

request.cmd = s->cmdreg >> 8;

request.arg = s->argument;

- DPRINT_L1("sending CMD%u ARG[0x%08x]\n", request.cmd, request.arg);

+

+ trace_sdhci_send_command(request.cmd, request.arg);

rlen = sdbus_do_command(&s->sdbus, &request, response);

if (s->cmdreg & SDHC_CMD_RESPONSE) {

@@ -XXX,XX +XXX,XX @@ static void sdhci_send_command(SDHCIState *s)

s->rspreg[0] = (response[0] << 24) | (response[1] << 16) |

(response[2] << 8) | response[3];

s->rspreg[1] = s->rspreg[2] = s->rspreg[3] = 0;

- DPRINT_L1("Response: RSPREG[31..0]=0x%08x\n", s->rspreg[0]);

+ trace_sdhci_response4(s->rspreg[0]);

} else if (rlen == 16) {

s->rspreg[0] = (response[11] << 24) | (response[12] << 16) |

(response[13] << 8) | response[14];

@@ -XXX,XX +XXX,XX @@ static void sdhci_send_command(SDHCIState *s)

(response[5] << 8) | response[6];

s->rspreg[3] = (response[0] << 16) | (response[1] << 8) |

response[2];

- DPRINT_L1("Response received:\n RSPREG[127..96]=0x%08x, RSPREG[95.."

- "64]=0x%08x,\n RSPREG[63..32]=0x%08x, RSPREG[31..0]=0x%08x\n",

- s->rspreg[3], s->rspreg[2], s->rspreg[1], s->rspreg[0]);

+ trace_sdhci_response16(s->rspreg[3], s->rspreg[2],

+ s->rspreg[1], s->rspreg[0]);

} else {

- ERRPRINT("Timeout waiting for command response\n");

+ trace_sdhci_error("timeout waiting for command response");

if (s->errintstsen & SDHC_EISEN_CMDTIMEOUT) {

s->errintsts |= SDHC_EIS_CMDTIMEOUT;

s->norintsts |= SDHC_NIS_ERR;

@@ -XXX,XX +XXX,XX @@ static void sdhci_end_transfer(SDHCIState *s)

request.cmd = 0x0C;

request.arg = 0;

- DPRINT_L1("Automatically issue CMD%d %08x\n", request.cmd, request.arg);

+ trace_sdhci_end_transfer(request.cmd, request.arg);

sdbus_do_command(&s->sdbus, &request, response);

/* Auto CMD12 response goes to the upper Response register */

s->rspreg[3] = (response[0] << 24) | (response[1] << 16) |

@@ -XXX,XX +XXX,XX @@ static uint32_t sdhci_read_dataport(SDHCIState *s, unsigned size)

/* first check that a valid data exists in host controller input buffer */

if ((s->prnsts & SDHC_DATA_AVAILABLE) == 0) {

- ERRPRINT("Trying to read from empty buffer\n");

+ trace_sdhci_error("read from empty buffer");

return 0;

}

@@ -XXX,XX +XXX,XX @@ static uint32_t sdhci_read_dataport(SDHCIState *s, unsigned size)

s->data_count++;

/* check if we've read all valid data (blksize bytes) from buffer */

if ((s->data_count) >= (s->blksize & 0x0fff)) {

- DPRINT_L2("All %u bytes of data have been read from input buffer\n",

- s->data_count);

+ trace_sdhci_read_dataport(s->data_count);

s->prnsts &= ~SDHC_DATA_AVAILABLE; /* no more data in a buffer */

s->data_count = 0; /* next buff read must start at position [0] */

@@ -XXX,XX +XXX,XX @@ static void sdhci_write_dataport(SDHCIState *s, uint32_t value, unsigned size)

/* Check that there is free space left in a buffer */

if (!(s->prnsts & SDHC_SPACE_AVAILABLE)) {

- ERRPRINT("Can't write to data buffer: buffer full\n");

+ trace_sdhci_error("Can't write to data buffer: buffer full");

return;

}

@@ -XXX,XX +XXX,XX @@ static void sdhci_write_dataport(SDHCIState *s, uint32_t value, unsigned size)

s->data_count++;

value >>= 8;

if (s->data_count >= (s->blksize & 0x0fff)) {

- DPRINT_L2("write buffer filled with %u bytes of data\n",

- s->data_count);

+ trace_sdhci_write_dataport(s->data_count);

s->data_count = 0;

s->prnsts &= ~SDHC_SPACE_AVAILABLE;

if (s->prnsts & SDHC_DOING_WRITE) {

@@ -XXX,XX +XXX,XX @@ static void sdhci_do_adma(SDHCIState *s)

{

unsigned int n, begin, length;

const uint16_t block_size = s->blksize & 0x0fff;

- ADMADescr dscr;

+ ADMADescr dscr = {};

int i;

for (i = 0; i < SDHC_ADMA_DESCS_PER_DELAY; ++i) {

s->admaerr &= ~SDHC_ADMAERR_LENGTH_MISMATCH;

get_adma_description(s, &dscr);

- DPRINT_L2("ADMA loop: addr=" TARGET_FMT_plx ", len=%d, attr=%x\n",

- dscr.addr, dscr.length, dscr.attr);

+ trace_sdhci_adma_loop(dscr.addr, dscr.length, dscr.attr);

if ((dscr.attr & SDHC_ADMA_ATTR_VALID) == 0) {

/* Indicate that error occurred in ST_FDS state */

@@ -XXX,XX +XXX,XX @@ static void sdhci_do_adma(SDHCIState *s)

break;

case SDHC_ADMA_ATTR_ACT_LINK: /* link to next descriptor table */

s->admasysaddr = dscr.addr;

- DPRINT_L1("ADMA link: admasysaddr=0x%" PRIx64 "\n",

- s->admasysaddr);

+ trace_sdhci_adma("link", s->admasysaddr);

break;

default:

s->admasysaddr += dscr.incr;

@@ -XXX,XX +XXX,XX @@ static void sdhci_do_adma(SDHCIState *s)

}

if (dscr.attr & SDHC_ADMA_ATTR_INT) {

- DPRINT_L1("ADMA interrupt: admasysaddr=0x%" PRIx64 "\n",

- s->admasysaddr);

+ trace_sdhci_adma("interrupt", s->admasysaddr);

if (s->norintstsen & SDHC_NISEN_DMA) {

s->norintsts |= SDHC_NIS_DMA;

}

@@ -XXX,XX +XXX,XX @@ static void sdhci_do_adma(SDHCIState *s)

/* ADMA transfer terminates if blkcnt == 0 or by END attribute */

if (((s->trnmod & SDHC_TRNS_BLK_CNT_EN) &&

(s->blkcnt == 0)) || (dscr.attr & SDHC_ADMA_ATTR_END)) {

- DPRINT_L2("ADMA transfer completed\n");

+ trace_sdhci_adma_transfer_completed();

if (length || ((dscr.attr & SDHC_ADMA_ATTR_END) &&

(s->trnmod & SDHC_TRNS_BLK_CNT_EN) &&

s->blkcnt != 0)) {

- ERRPRINT("SD/MMC host ADMA length mismatch\n");

+ trace_sdhci_error("SD/MMC host ADMA length mismatch");

s->admaerr |= SDHC_ADMAERR_LENGTH_MISMATCH |

SDHC_ADMAERR_STATE_ST_TFR;

if (s->errintstsen & SDHC_EISEN_ADMAERR) {

- ERRPRINT("Set ADMA error flag\n");

+ trace_sdhci_error("Set ADMA error flag");

s->errintsts |= SDHC_EIS_ADMAERR;

s->norintsts |= SDHC_NIS_ERR;

}

@@ -XXX,XX +XXX,XX @@ static void sdhci_data_transfer(void *opaque)

break;

case SDHC_CTRL_ADMA1_32:

if (!(s->capareg & SDHC_CAN_DO_ADMA1)) {

- ERRPRINT("ADMA1 not supported\n");

+ trace_sdhci_error("ADMA1 not supported");

break;

}

@@ -XXX,XX +XXX,XX @@ static void sdhci_data_transfer(void *opaque)

break;

case SDHC_CTRL_ADMA2_32:

if (!(s->capareg & SDHC_CAN_DO_ADMA2)) {

- ERRPRINT("ADMA2 not supported\n");

+ trace_sdhci_error("ADMA2 not supported");

break;

}

@@ -XXX,XX +XXX,XX @@ static void sdhci_data_transfer(void *opaque)

case SDHC_CTRL_ADMA2_64:

if (!(s->capareg & SDHC_CAN_DO_ADMA2) ||

!(s->capareg & SDHC_64_BIT_BUS_SUPPORT)) {

- ERRPRINT("64 bit ADMA not supported\n");

+ trace_sdhci_error("64 bit ADMA not supported");

break;

}

sdhci_do_adma(s);

break;

default:

- ERRPRINT("Unsupported DMA type\n");

+ trace_sdhci_error("Unsupported DMA type");

break;

}

} else {

@@ -XXX,XX +XXX,XX @@ static inline bool

sdhci_buff_access_is_sequential(SDHCIState *s, unsigned byte_num)

{

if ((s->data_count & 0x3) != byte_num) {

- ERRPRINT("Non-sequential access to Buffer Data Port register"

- "is prohibited\n");

+ trace_sdhci_error("Non-sequential access to Buffer Data Port register"

+ "is prohibited\n");

return false;

}

return true;

@@ -XXX,XX +XXX,XX @@ static uint64_t sdhci_read(void *opaque, hwaddr offset, unsigned size)

case SDHC_BDATA:

if (sdhci_buff_access_is_sequential(s, offset - SDHC_BDATA)) {

ret = sdhci_read_dataport(s, size);

- DPRINT_L2("read %ub: addr[0x%04x] -> %u(0x%x)\n", size, (int)offset,

- ret, ret);

+ trace_sdhci_access("rd", size << 3, offset, "->", ret, ret);

return ret;

@@ -XXX,XX +XXX,XX @@ static uint64_t sdhci_read(void *opaque, hwaddr offset, unsigned size)

ret >>= (offset & 0x3) * 8;

ret &= (1ULL << (size * 8)) - 1;

- DPRINT_L2("read %ub: addr[0x%04x] -> %u(0x%x)\n", size, (int)offset, ret, ret);

+ trace_sdhci_access("rd", size << 3, offset, "->", ret, ret);

return ret;

@@ -XXX,XX +XXX,XX @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)

"not implemented\n", size, offset, value >> shift);

- DPRINT_L2("write %ub: addr[0x%04x] <- %u(0x%x)\n",

- size, (int)offset, value >> shift, value >> shift);

+ trace_sdhci_access("wr", size << 3, offset, "<-",

+ value >> shift, value >> shift);

static const MemoryRegionOps sdhci_mmio_ops = {

diff --git a/hw/sd/trace-events b/hw/sd/trace-events

index XXXXXXX..XXXXXXX 100644

--- a/hw/sd/trace-events

+++ b/hw/sd/trace-events

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

# See docs/devel/tracing.txt for syntax documentation.

+# hw/sd/sdhci.c

+sdhci_set_inserted(const char *level) "card state changed: %s"

+sdhci_send_command(uint8_t cmd, uint32_t arg) "CMD%02u ARG[0x%08x]"

+sdhci_error(const char *msg) "%s"

+sdhci_response4(uint32_t r0) "RSPREG[31..0]=0x%08x"

+sdhci_response16(uint32_t r3, uint32_t r2, uint32_t r1, uint32_t r0) "RSPREG[127..96]=0x%08x, RSPREG[95..64]=0x%08x, RSPREG[63..32]=0x%08x, RSPREG[31..0]=0x%08x"

+sdhci_end_transfer(uint8_t cmd, uint32_t arg) "Automatically issue CMD%02u 0x%08x"

+sdhci_adma(const char *desc, uint32_t sysad) "%s: admasysaddr=0x%" PRIx32

+sdhci_adma_loop(uint64_t addr, uint16_t length, uint8_t attr) "addr=0x%08" PRIx64 ", len=%d, attr=0x%x"

+sdhci_adma_transfer_completed(void) ""

+sdhci_access(const char *access, unsigned int size, uint64_t offset, const char *dir, uint64_t val, uint64_t val2) "%s%u: addr[0x%04" PRIx64 "] %s 0x%08" PRIx64 " (%" PRIu64 ")"

+sdhci_read_dataport(uint16_t data_count) "all %u bytes of data have been read from input buffer"

+sdhci_write_dataport(uint16_t data_count) "write buffer filled with %u bytes of data"

# hw/sd/milkymist-memcard.c

milkymist_memcard_memory_read(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x"

milkymist_memcard_memory_write(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x"

2.7.4

Instead of ignoring the response from address_space_ld*()

(indicating an attempt to read a page table descriptor from

an invalid physical address), use it to report the failure

correctly.

Since this is another couple of locations where we need to

decide the value of the ARMMMUFaultInfo ea bit based on a

MemTxResult, we factor out that operation into a helper

function.

target/arm/internals.h | 10 ++++++++++

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

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

3 files changed, 45 insertions(+), 11 deletions(-)

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

--- a/target/arm/internals.h

+++ b/target/arm/internals.h

@@ -XXX,XX +XXX,XX @@ static inline uint32_t arm_fi_to_lfsc(ARMMMUFaultInfo *fi)

return fsc;

+static inline bool arm_extabort_type(MemTxResult result)

+ /* The EA bit in syndromes and fault status registers is an

+ * IMPDEF classification of external aborts. ARM implementations

+ * usually use this to indicate AXI bus Decode error (0) or

+ * Slave error (1); in QEMU we follow that.

+ return result != MEMTX_DECODE_ERROR;

/* Do a page table walk and add page to TLB if possible */

bool arm_tlb_fill(CPUState *cpu, vaddr address,

MMUAccessType access_type, int mmu_idx,

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 hwaddr S1_ptw_translate(CPUARMState *env, ARMMMUIdx mmu_idx,

ret = get_phys_addr_lpae(env, addr, 0, ARMMMUIdx_S2NS, &s2pa,

&txattrs, &s2prot, &s2size, fi, NULL);

if (ret) {

+ assert(fi->type != ARMFault_None);

fi->s2addr = addr;

fi->stage2 = true;

fi->s1ptw = true;

@@ -XXX,XX +XXX,XX @@ static uint32_t arm_ldl_ptw(CPUState *cs, hwaddr addr, bool is_secure,

ARMCPU *cpu = ARM_CPU(cs);

CPUARMState *env = &cpu->env;

MemTxAttrs attrs = {};

+ MemTxResult result = MEMTX_OK;

AddressSpace *as;

+ uint32_t data;

attrs.secure = is_secure;

as = arm_addressspace(cs, attrs);

@@ -XXX,XX +XXX,XX @@ static uint32_t arm_ldl_ptw(CPUState *cs, hwaddr addr, bool is_secure,

return 0;

}

if (regime_translation_big_endian(env, mmu_idx)) {

- return address_space_ldl_be(as, addr, attrs, NULL);

+ data = address_space_ldl_be(as, addr, attrs, &result);

} else {

- return address_space_ldl_le(as, addr, attrs, NULL);

+ data = address_space_ldl_le(as, addr, attrs, &result);

}

+ if (result == MEMTX_OK) {

+ return data;

+ }

+ fi->type = ARMFault_SyncExternalOnWalk;

+ fi->ea = arm_extabort_type(result);

+ return 0;

}

static uint64_t arm_ldq_ptw(CPUState *cs, hwaddr addr, bool is_secure,

@@ -XXX,XX +XXX,XX @@ static uint64_t arm_ldq_ptw(CPUState *cs, hwaddr addr, bool is_secure,

ARMCPU *cpu = ARM_CPU(cs);

CPUARMState *env = &cpu->env;

MemTxAttrs attrs = {};

+ MemTxResult result = MEMTX_OK;

AddressSpace *as;

+ uint32_t data;

attrs.secure = is_secure;

as = arm_addressspace(cs, attrs);

@@ -XXX,XX +XXX,XX @@ static uint64_t arm_ldq_ptw(CPUState *cs, hwaddr addr, bool is_secure,

return 0;

}

if (regime_translation_big_endian(env, mmu_idx)) {

- return address_space_ldq_be(as, addr, attrs, NULL);

+ data = address_space_ldq_be(as, addr, attrs, &result);

} else {

- return address_space_ldq_le(as, addr, attrs, NULL);

+ data = address_space_ldq_le(as, addr, attrs, &result);

+ }

+ if (result == MEMTX_OK) {

+ return data;

}

+ fi->type = ARMFault_SyncExternalOnWalk;

+ fi->ea = arm_extabort_type(result);

+ return 0;

}

static bool get_phys_addr_v5(CPUARMState *env, uint32_t address,

@@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v5(CPUARMState *env, uint32_t address,

}

desc = arm_ldl_ptw(cs, table, regime_is_secure(env, mmu_idx),

mmu_idx, fi);

+ if (fi->type != ARMFault_None) {

+ goto do_fault;

+ }

type = (desc & 3);

domain = (desc >> 5) & 0x0f;

if (regime_el(env, mmu_idx) == 1) {

@@ -XXX,XX +XXX,XX @@ static bool get_phys_addr_v5(CPUARMState *env, uint32_t address,

}

desc = arm_ldl_ptw(cs, table, regime_is_secure(env, mmu_idx),

mmu_idx, fi);

+ if (fi->type != ARMFault_None) {

+ goto do_fault;

+ }

switch (desc & 3) {

case 0: /* Page translation fault. */

fi->type = ARMFault_Translation;