[Qemu-devel] [PATCH v4 08/20] sdcard: use the registerfields API for the CARD_STATUS register masks

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Acked-by: Alistair Francis 
---
 hw/sd/sd.c | 48 +---
 1 file changed, 45 insertions(+), 3 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 3970e590e6..b567c44da8 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -420,14 +420,56 @@ static void sd_set_rca(SDState *sd)
 sd->rca += 0x4567;
 }
 
+FIELD(CSR, AKE_SEQ_ERROR,   3,  1)
+FIELD(CSR, APP_CMD, 5,  1)
+FIELD(CSR, FX_EVENT,6,  1)
+FIELD(CSR, READY_FOR_DATA,  8,  1)
+FIELD(CSR, CURRENT_STATE,   9,  4)
+FIELD(CSR, ERASE_RESET,13,  1)
+FIELD(CSR, CARD_ECC_DISABLED,  14,  1)
+FIELD(CSR, WP_ERASE_SKIP,  15,  1)
+FIELD(CSR, CSD_OVERWRITE,  16,  1)
+FIELD(CSR, DEFERRED_RESPONSE,  17,  1)
+FIELD(CSR, ERROR,  19,  1)
+FIELD(CSR, CC_ERROR,   20,  1)
+FIELD(CSR, CARD_ECC_FAILED,21,  1)
+FIELD(CSR, ILLEGAL_COMMAND,22,  1)
+FIELD(CSR, COM_CRC_ERROR,  23,  1)
+FIELD(CSR, LOCK_UNLOCK_FAILED, 24,  1)
+FIELD(CSR, CARD_IS_LOCKED, 25,  1)
+FIELD(CSR, WP_VIOLATION,   26,  1)
+FIELD(CSR, ERASE_PARAM,27,  1)
+FIELD(CSR, ERASE_SEQ_ERROR,28,  1)
+FIELD(CSR, BLOCK_LEN_ERROR,29,  1)
+FIELD(CSR, ADDRESS_ERROR,  30,  1)
+FIELD(CSR, OUT_OF_RANGE,   31,  1)
+
 /* Card status bits, split by clear condition:
  * A : According to the card current state
  * B : Always related to the previous command
  * C : Cleared by read
  */
-#define CARD_STATUS_A  0x02004100
-#define CARD_STATUS_B  0x00c01e00
-#define CARD_STATUS_C  0xfd39a028
+#define CARD_STATUS_A   (R_CSR_READY_FOR_DATA_MASK \
+   | R_CSR_CARD_ECC_DISABLED_MASK \
+   | R_CSR_CARD_IS_LOCKED_MASK)
+#define CARD_STATUS_B   (R_CSR_CURRENT_STATE_MASK \
+   | R_CSR_ILLEGAL_COMMAND_MASK \
+   | R_CSR_COM_CRC_ERROR_MASK)
+#define CARD_STATUS_C   (R_CSR_AKE_SEQ_ERROR_MASK \
+   | R_CSR_APP_CMD_MASK \
+   | R_CSR_ERASE_RESET_MASK \
+   | R_CSR_WP_ERASE_SKIP_MASK \
+   | R_CSR_CSD_OVERWRITE_MASK \
+   | R_CSR_ERROR_MASK \
+   | R_CSR_CC_ERROR_MASK \
+   | R_CSR_CARD_ECC_FAILED_MASK \
+   | R_CSR_LOCK_UNLOCK_FAILED_MASK \
+   | R_CSR_WP_VIOLATION_MASK \
+   | R_CSR_ERASE_PARAM_MASK \
+   | R_CSR_ERASE_SEQ_ERROR_MASK \
+   | R_CSR_BLOCK_LEN_ERROR_MASK \
+   | R_CSR_ADDRESS_ERROR_MASK \
+   | R_CSR_OUT_OF_RANGE_MASK)
 
 static void sd_set_cardstatus(SDState *sd)
 {
-- 
2.16.1

[Qemu-devel] [PATCH v4 04/20] sdcard: clean the SCR register and add few comments

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
---
 hw/sd/sd.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 14c1cb1332..41fac9a4aa 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -299,10 +299,13 @@ static void sd_ocr_powerup(void *opaque)
 
 static void sd_set_scr(SDState *sd)
 {
-sd->scr[0] = 0x00; /* SCR Structure */
-sd->scr[1] = 0x2f; /* SD Security Support */
-sd->scr[2] = 0x00;
+sd->scr[0] = (0 << 4)   /* SCR version 1.0 */
+ | 0;   /* Spec Versions 1.0 and 1.01 */
+sd->scr[1] = (2 << 4)   /* SDSC Card (Security Version 1.01) */
+ | 0b0101;  /* 1-bit or 4-bit width bus modes */
+sd->scr[2] = 0x00;  /* Extended Security is not supported. */
 sd->scr[3] = 0x00;
+/* reserved for manufacturer usage */
 sd->scr[4] = 0x00;
 sd->scr[5] = 0x00;
 sd->scr[6] = 0x00;
-- 
2.16.1

[Qemu-devel] [PATCH v4 07/11] sdcard: define SDMMC_CMD_MAX instead of using the magic '64'

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
---
Since v3: add "sdmmc-internal.h"

 hw/sd/sdmmc-internal.h | 15 +++
 hw/sd/sd.c | 22 --
 2 files changed, 31 insertions(+), 6 deletions(-)
 create mode 100644 hw/sd/sdmmc-internal.h

diff --git a/hw/sd/sdmmc-internal.h b/hw/sd/sdmmc-internal.h
new file mode 100644
index 00..0e96cb0081
--- /dev/null
+++ b/hw/sd/sdmmc-internal.h
@@ -0,0 +1,15 @@
+/*
+ * SD/MMC cards common
+ *
+ * Copyright (c) 2018  Philippe Mathieu-Daudé 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#ifndef SD_INTERNAL_H
+#define SD_INTERNAL_H
+
+#define SDMMC_CMD_MAX 64
+
+#endif
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index af4df2b104..6acd6b3c5c 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -40,6 +40,7 @@
 #include "qemu/error-report.h"
 #include "qemu/timer.h"
 #include "qemu/log.h"
+#include "sdmmc-internal.h"
 #include "trace.h"
 
 //#define DEBUG_SD 1
@@ -215,18 +216,21 @@ static void sd_set_mode(SDState *sd)
 }
 }
 
-static const sd_cmd_type_t sd_cmd_type[64] = {
+static const sd_cmd_type_t sd_cmd_type[SDMMC_CMD_MAX] = {
 sd_bc,   sd_none, sd_bcr,  sd_bcr,  sd_none, sd_none, sd_none, sd_ac,
 sd_bcr,  sd_ac,   sd_ac,   sd_adtc, sd_ac,   sd_ac,   sd_none, sd_ac,
+/* 16 */
 sd_ac,   sd_adtc, sd_adtc, sd_none, sd_none, sd_none, sd_none, sd_none,
 sd_adtc, sd_adtc, sd_adtc, sd_adtc, sd_ac,   sd_ac,   sd_adtc, sd_none,
+/* 32 */
 sd_ac,   sd_ac,   sd_none, sd_none, sd_none, sd_none, sd_ac,   sd_none,
 sd_none, sd_none, sd_bc,   sd_none, sd_none, sd_none, sd_none, sd_none,
+/* 48 */
 sd_none, sd_none, sd_none, sd_none, sd_none, sd_none, sd_none, sd_ac,
 sd_adtc, sd_none, sd_none, sd_none, sd_none, sd_none, sd_none, sd_none,
 };
 
-static const int sd_cmd_class[64] = {
+static const int sd_cmd_class[SDMMC_CMD_MAX] = {
 0,  0,  0,  0,  0,  9, 10,  0,  0,  0,  0,  1,  0,  0,  0,  0,
 2,  2,  2,  2,  3,  3,  3,  3,  4,  4,  4,  4,  6,  6,  6,  6,
 5,  5, 10, 10, 10, 10,  5,  9,  9,  9,  7,  7,  7,  7,  7,  7,
@@ -831,8 +835,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, 
SDRequest req)
 /* Not interpreting this as an app command */
 sd->card_status &= ~APP_CMD;
 
-if (sd_cmd_type[req.cmd & 0x3F] == sd_ac
-|| sd_cmd_type[req.cmd & 0x3F] == sd_adtc) {
+if (sd_cmd_type[req.cmd] == sd_ac
+|| sd_cmd_type[req.cmd] == sd_adtc) {
 rca = req.arg >> 16;
 }
 
@@ -1544,8 +1548,8 @@ static int cmd_valid_while_locked(SDState *sd, SDRequest 
*req)
 if (req->cmd == 16 || req->cmd == 55) {
 return 1;
 }
-return sd_cmd_class[req->cmd & 0x3F] == 0
-|| sd_cmd_class[req->cmd & 0x3F] == 7;
+return sd_cmd_class[req->cmd] == 0
+|| sd_cmd_class[req->cmd] == 7;
 }
 
 int sd_do_command(SDState *sd, SDRequest *req,
@@ -1564,6 +1568,12 @@ int sd_do_command(SDState *sd, SDRequest *req,
 goto send_response;
 }
 
+if (req->cmd >= SDMMC_CMD_MAX) {
+qemu_log_mask(LOG_GUEST_ERROR, "SD: incorrect command 0x%02x\n",
+  req->cmd);
+req->cmd &= 0x3f;
+}
+
 if (sd->card_status & CARD_IS_LOCKED) {
 if (!cmd_valid_while_locked(sd, req)) {
 sd->card_status |= ILLEGAL_COMMAND;
-- 
2.16.1

[Qemu-devel] [PATCH v4 10/11] sdcard: use G_BYTE from cutils

[Philippe Mathieu-Daudé]

code is now easier to read.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
---
 hw/sd/sd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 6760815045..28837768d4 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -36,6 +36,7 @@
 #include "hw/sd/sd.h"
 #include "qapi/error.h"
 #include "qemu/bitmap.h"
+#include "qemu/cutils.h"
 #include "hw/qdev-properties.h"
 #include "qemu/error-report.h"
 #include "qemu/timer.h"
@@ -344,7 +345,7 @@ static void sd_set_csd(SDState *sd, uint64_t size)
 uint32_t sectsize = (1 << (SECTOR_SHIFT + 1)) - 1;
 uint32_t wpsize = (1 << (WPGROUP_SHIFT + 1)) - 1;
 
-if (size <= 0x4000) {  /* Standard Capacity SD */
+if (size <= 1 * G_BYTE) { /* Standard Capacity SD */
 sd->csd[0] = 0x00; /* CSD structure */
 sd->csd[1] = 0x26; /* Data read access-time-1 */
 sd->csd[2] = 0x00; /* Data read access-time-2 */
-- 
2.16.1

[Qemu-devel] [PATCH v4 02/20] sdcard: update the CSD CRC register regardless the CSD structure version

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
---
 hw/sd/sd.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 9dfbd65ac8..c1ba098d86 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -379,7 +379,6 @@ static void sd_set_csd(SDState *sd, uint64_t size)
 sd->csd[13] = 0x20 |   /* Max. write data block length */
 ((HWBLOCK_SHIFT << 6) & 0xc0);
 sd->csd[14] = 0x00;/* File format group */
-sd->csd[15] = (sd_crc7(sd->csd, 15) << 1) | 1;
 } else {   /* SDHC */
 size /= 512 * 1024;
 size -= 1;
@@ -398,8 +397,8 @@ static void sd_set_csd(SDState *sd, uint64_t size)
 sd->csd[12] = 0x0a;
 sd->csd[13] = 0x40;
 sd->csd[14] = 0x00;
-sd->csd[15] = 0x00;
 }
+sd->csd[15] = (sd_crc7(sd->csd, 15) << 1) | 1;
 }
 
 static void sd_set_rca(SDState *sd)
-- 
2.16.1

[Qemu-devel] [PATCH v4 05/11] sdcard: add more trace events

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Acked-by: Alistair Francis 
---
 hw/sd/sd.c | 32 ++--
 hw/sd/trace-events | 13 +
 2 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index ceab263970..564f7a9bfd 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -179,6 +179,8 @@ static bool sd_get_cmd_line(SDState *sd)
 
 static void sd_set_voltage(SDState *sd, uint16_t millivolts)
 {
+trace_sdcard_set_voltage(millivolts);
+
 switch (millivolts) {
 case 3001 ... 3600: /* SD_VOLTAGE_3_3V */
 case 2001 ... 3000: /* SD_VOLTAGE_3_0V */
@@ -274,6 +276,7 @@ static void sd_ocr_powerup(void *opaque)
 {
 SDState *sd = opaque;
 
+trace_sdcard_powerup();
 /* Set powered up bit in OCR */
 assert(!(sd->ocr & OCR_POWER_UP));
 sd->ocr |= OCR_POWER_UP;
@@ -477,6 +480,7 @@ static void sd_reset(DeviceState *dev)
 uint64_t size;
 uint64_t sect;
 
+trace_sdcard_reset();
 if (sd->blk) {
 blk_get_geometry(sd->blk, );
 } else {
@@ -530,7 +534,10 @@ static void sd_cardchange(void *opaque, bool load, Error 
**errp)
 bool readonly = sd_get_readonly(sd);
 
 if (inserted) {
+trace_sdcard_inserted(readonly);
 sd_reset(dev);
+} else {
+trace_sdcard_ejected();
 }
 
 /* The IRQ notification is for legacy non-QOM SD controller devices;
@@ -662,6 +669,7 @@ static void sd_erase(SDState *sd)
 uint64_t erase_start = sd->erase_start;
 uint64_t erase_end = sd->erase_end;
 
+trace_sdcard_erase();
 if (!sd->erase_start || !sd->erase_end) {
 sd->card_status |= ERASE_SEQ_ERROR;
 return;
@@ -751,6 +759,11 @@ static void sd_lock_command(SDState *sd)
 else
 pwd_len = 0;
 
+if (lock) {
+trace_sdcard_lock();
+} else {
+trace_sdcard_unlock();
+}
 if (erase) {
 if (!(sd->card_status & CARD_IS_LOCKED) || sd->blk_len > 1 ||
 set_pwd || clr_pwd || lock || sd->wp_switch ||
@@ -1077,10 +1090,12 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
 case 16:   /* CMD16:  SET_BLOCKLEN */
 switch (sd->state) {
 case sd_transfer_state:
-if (req.arg > (1 << HWBLOCK_SHIFT))
+if (req.arg > (1 << HWBLOCK_SHIFT)) {
 sd->card_status |= BLOCK_LEN_ERROR;
-else
+} else {
+trace_sdcard_set_blocklen(req.arg);
 sd->blk_len = req.arg;
+}
 
 return sd_r1;
 
@@ -1452,10 +1467,13 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
 if ((req.arg & ACMD41_ENQUIRY_MASK) != 0) {
 timer_del(sd->ocr_power_timer);
 sd_ocr_powerup(sd);
-} else if (!timer_pending(sd->ocr_power_timer)) {
-timer_mod_ns(sd->ocr_power_timer,
- (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)
-  + OCR_POWER_DELAY_NS));
+} else {
+trace_sdcard_inquiry_cmd41();
+if (!timer_pending(sd->ocr_power_timer)) {
+timer_mod_ns(sd->ocr_power_timer,
+ (qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL)
+  + OCR_POWER_DELAY_NS));
+}
 }
 }
 
@@ -1668,6 +1686,7 @@ void sd_write_data(SDState *sd, uint8_t value)
 if (sd->card_status & (ADDRESS_ERROR | WP_VIOLATION))
 return;
 
+trace_sdcard_write_data(sd->current_cmd, value);
 switch (sd->current_cmd) {
 case 24:   /* CMD24:  WRITE_SINGLE_BLOCK */
 sd->data[sd->data_offset ++] = value;
@@ -1805,6 +1824,7 @@ uint8_t sd_read_data(SDState *sd)
 
 io_len = (sd->ocr & (1 << 30)) ? 512 : sd->blk_len;
 
+trace_sdcard_read_data(sd->current_cmd, io_len);
 switch (sd->current_cmd) {
 case 6:/* CMD6:   SWITCH_FUNCTION */
 ret = sd->data[sd->data_offset ++];
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index b2aa19ec0d..3040d32560 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -27,8 +27,21 @@ sdhci_capareg(const char *desc, uint16_t val) "%s: %u"
 sdcard_normal_command(uint8_t cmd, uint32_t arg, const char *state) "CMD%d arg 
0x%08x (state %s)"
 sdcard_app_command(uint8_t acmd, uint32_t arg) "ACMD%d arg 0x%08x"
 sdcard_response(const char *rspdesc, int rsplen) "%s (sz:%d)"
+sdcard_powerup(void) ""
+sdcard_inquiry_cmd41(void) ""
+sdcard_set_enable(bool current_state, bool new_state) "%u -> %u"
+sdcard_reset(void) ""
+sdcard_set_blocklen(uint16_t length) "0x%04x"
+sdcard_inserted(bool readonly) "read_only: %u"
+sdcard_ejected(void) ""
+sdcard_erase(void) ""
+sdcard_lock(void) ""
+sdcard_unlock(void) ""
 sdcard_read_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size 0x%x"
 sdcard_write_block(uint64_t addr, uint32_t 

[Qemu-devel] [PATCH v2 3/3] ppc: Add aCube Sam460ex board

[BALATON Zoltan]

Add emulation of aCube Sam460ex board based on AMCC 460EX embedded SoC.
This is not a complete implementation yet with a lot of components
still missing but enough for the U-Boot firmware to start and to boot
a Linux kernel or AROS.

Signed-off-by: François Revol 
Signed-off-by: BALATON Zoltan 
---

v2:
- Rebased to latest changes on master
- Replaced printfs with error_report

 default-configs/ppc-softmmu.mak|   2 +
 default-configs/ppcemb-softmmu.mak |   1 +
 hw/ppc/Makefile.objs   |   3 +-
 hw/ppc/sam460ex.c  | 603 +
 4 files changed, 608 insertions(+), 1 deletion(-)
 create mode 100644 hw/ppc/sam460ex.c

diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
index 76e29cf..4d7be45 100644
--- a/default-configs/ppc-softmmu.mak
+++ b/default-configs/ppc-softmmu.mak
@@ -21,6 +21,8 @@ CONFIG_E500=y
 CONFIG_OPENPIC_KVM=$(call land,$(CONFIG_E500),$(CONFIG_KVM))
 CONFIG_PLATFORM_BUS=y
 CONFIG_ETSEC=y
+# For Sam460ex
+CONFIG_USB_EHCI_SYSBUS=y
 CONFIG_SM501=y
 CONFIG_IDE_SII3112=y
 CONFIG_I2C=y
diff --git a/default-configs/ppcemb-softmmu.mak 
b/default-configs/ppcemb-softmmu.mak
index bc5e1b3..67d18b2 100644
--- a/default-configs/ppcemb-softmmu.mak
+++ b/default-configs/ppcemb-softmmu.mak
@@ -15,6 +15,7 @@ CONFIG_PTIMER=y
 CONFIG_I8259=y
 CONFIG_XILINX=y
 CONFIG_XILINX_ETHLITE=y
+CONFIG_USB_EHCI_SYSBUS=y
 CONFIG_SM501=y
 CONFIG_IDE_SII3112=y
 CONFIG_I2C=y
diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
index bddc742..86d82a6 100644
--- a/hw/ppc/Makefile.objs
+++ b/hw/ppc/Makefile.objs
@@ -13,7 +13,8 @@ endif
 obj-$(CONFIG_PSERIES) += spapr_rtas_ddw.o
 # PowerPC 4xx boards
 obj-y += ppc4xx_devs.o ppc405_uc.o
-obj-$(CONFIG_PPC4XX) += ppc4xx_pci.o ppc405_boards.o ppc440_bamboo.o 
ppc440_pcix.o
+obj-$(CONFIG_PPC4XX) += ppc4xx_pci.o ppc405_boards.o
+obj-$(CONFIG_PPC4XX) += ppc440_bamboo.o ppc440_pcix.o ppc440_uc.o sam460ex.o
 # PReP
 obj-$(CONFIG_PREP) += prep.o
 obj-$(CONFIG_PREP) += prep_systemio.o
diff --git a/hw/ppc/sam460ex.c b/hw/ppc/sam460ex.c
new file mode 100644
index 000..ff99cf9
--- /dev/null
+++ b/hw/ppc/sam460ex.c
@@ -0,0 +1,603 @@
+/*
+ * QEMU aCube Sam460ex board emulation
+ *
+ * Copyright (c) 2012 François Revol
+ * Copyright (c) 2016-2018 BALATON Zoltan
+ *
+ * This file is derived from hw/ppc440_bamboo.c,
+ * the copyright for that material belongs to the original owners.
+ *
+ * This work is licensed under the GNU GPL license version 2 or later.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "qemu/cutils.h"
+#include "qemu/error-report.h"
+#include "qapi/error.h"
+#include "hw/hw.h"
+#include "sysemu/blockdev.h"
+#include "hw/boards.h"
+#include "sysemu/kvm.h"
+#include "kvm_ppc.h"
+#include "sysemu/device_tree.h"
+#include "sysemu/block-backend.h"
+#include "hw/loader.h"
+#include "elf.h"
+#include "exec/address-spaces.h"
+#include "exec/memory.h"
+#include "hw/ppc/ppc440.h"
+#include "hw/ppc/ppc405.h"
+#include "hw/block/flash.h"
+#include "sysemu/sysemu.h"
+#include "sysemu/qtest.h"
+#include "hw/sysbus.h"
+#include "hw/char/serial.h"
+#include "hw/i2c/ppc4xx_i2c.h"
+#include "hw/i2c/smbus.h"
+#include "hw/usb/hcd-ehci.h"
+
+#define BINARY_DEVICE_TREE_FILE "sam460ex.dtb"
+#define UBOOT_FILENAME "u-boot-sam460-20100605.bin"
+/* to extract the official U-Boot bin from the updater: */
+/* dd bs=1 skip=$(($(stat -c '%s' updater/updater-460) - 0x8)) \
+ if=updater/updater-460 of=u-boot-sam460-20100605.bin */
+
+/* from Sam460 U-Boot include/configs/Sam460ex.h */
+#define FLASH_BASE 0xfff0
+#define FLASH_BASE_H   0x4
+#define FLASH_SIZE (1 << 20)
+#define UBOOT_LOAD_BASE0xfff8
+#define UBOOT_SIZE 0x0008
+#define UBOOT_ENTRY0xfffc
+
+/* from U-Boot */
+#define EPAPR_MAGIC   (0x45504150)
+#define KERNEL_ADDR   0x100
+#define FDT_ADDR  0x180
+#define RAMDISK_ADDR  0x190
+
+/* Sam460ex IRQ MAP:
+   IRQ0  = ETH_INT
+   IRQ1  = FPGA_INT
+   IRQ2  = PCI_INT (PCIA, PCIB, PCIC, PCIB)
+   IRQ3  = FPGA_INT2
+   IRQ11 = RTC_INT
+   IRQ12 = SM502_INT
+*/
+
+#define SDRAM_NR_BANKS 4
+
+/* FIXME: See u-boot.git 8ac41e, also fix in ppc440_uc.c */
+static const unsigned int ppc460ex_sdram_bank_sizes[] = {
+1024 << 20, 512 << 20, 256 << 20, 128 << 20, 64 << 20, 32 << 20, 0
+};
+
+struct boot_info {
+uint32_t dt_base;
+uint32_t dt_size;
+uint32_t entry;
+};
+
+/*/
+/* SPD eeprom content from mips_malta.c */
+
+struct _eeprom24c0x_t {
+  uint8_t tick;
+  uint8_t address;
+  uint8_t command;
+  uint8_t ack;
+  uint8_t scl;
+  uint8_t sda;
+  uint8_t data;
+  uint8_t contents[256];
+};
+
+typedef struct _eeprom24c0x_t eeprom24c0x_t;
+
+static eeprom24c0x_t spd_eeprom = {
+.contents = {
+/* : */ 0x80, 0x08, 0xFF, 

[Qemu-devel] [PATCH v4 06/11] sdcard: do not trace CMD55 when expecting ACMD

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Acked-by: Alistair Francis 
---
 hw/sd/sd.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 564f7a9bfd..af4df2b104 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -818,13 +818,15 @@ static void sd_lock_command(SDState *sd)
 sd->card_status &= ~CARD_IS_LOCKED;
 }
 
-static sd_rsp_type_t sd_normal_command(SDState *sd,
-   SDRequest req)
+static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 {
 uint32_t rca = 0x;
 uint64_t addr = (sd->ocr & (1 << 30)) ? (uint64_t) req.arg << 9 : req.arg;
 
-trace_sdcard_normal_command(req.cmd, req.arg, sd_state_name(sd->state));
+if (req.cmd != 55 || sd->expecting_acmd) {
+trace_sdcard_normal_command(req.cmd, req.arg,
+sd_state_name(sd->state));
+}
 
 /* Not interpreting this as an app command */
 sd->card_status &= ~APP_CMD;
-- 
2.16.1

[Qemu-devel] [PATCH v2 2/3] ppc440: Add emulation of plb-pcix controller found in some 440 SoCs

[BALATON Zoltan]

This is the PCIX controller found in newer 440 core SoCs e.g. the
AMMC 460EX. The device tree refers to this as plb-pcix compared to
the plb-pci controller in older 440 SoCs.

Signed-off-by: BALATON Zoltan 
---

v2:
- Replace debug printfs with trace functions
- Fix access of low address registers

 hw/ppc/Makefile.objs |   2 +-
 hw/ppc/ppc440_pcix.c | 528 +++
 hw/ppc/trace-events  |   8 +
 3 files changed, 537 insertions(+), 1 deletion(-)
 create mode 100644 hw/ppc/ppc440_pcix.c

diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
index ad1928c..bddc742 100644
--- a/hw/ppc/Makefile.objs
+++ b/hw/ppc/Makefile.objs
@@ -13,7 +13,7 @@ endif
 obj-$(CONFIG_PSERIES) += spapr_rtas_ddw.o
 # PowerPC 4xx boards
 obj-y += ppc4xx_devs.o ppc405_uc.o
-obj-$(CONFIG_PPC4XX) += ppc4xx_pci.o ppc405_boards.o ppc440_bamboo.o
+obj-$(CONFIG_PPC4XX) += ppc4xx_pci.o ppc405_boards.o ppc440_bamboo.o 
ppc440_pcix.o
 # PReP
 obj-$(CONFIG_PREP) += prep.o
 obj-$(CONFIG_PREP) += prep_systemio.o
diff --git a/hw/ppc/ppc440_pcix.c b/hw/ppc/ppc440_pcix.c
new file mode 100644
index 000..ab2626a
--- /dev/null
+++ b/hw/ppc/ppc440_pcix.c
@@ -0,0 +1,528 @@
+/*
+ * Emulation of the ibm,plb-pcix PCI controller
+ * This is found in some 440 SoCs e.g. the 460EX.
+ *
+ * Copyright (c) 2016-2018 BALATON Zoltan
+ *
+ * Derived from ppc4xx_pci.c and pci-host/ppce500.c
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License, version 2, as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, see .
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/error-report.h"
+#include "hw/hw.h"
+#include "hw/ppc/ppc.h"
+#include "hw/ppc/ppc4xx.h"
+#include "hw/pci/pci.h"
+#include "hw/pci/pci_host.h"
+#include "exec/address-spaces.h"
+#include "trace.h"
+
+struct PLBOutMap {
+uint64_t la;
+uint64_t pcia;
+uint32_t sa;
+MemoryRegion mr;
+};
+
+struct PLBInMap {
+uint64_t sa;
+uint64_t la;
+MemoryRegion mr;
+};
+
+#define TYPE_PPC440_PCIX_HOST_BRIDGE "ppc440-pcix-host"
+#define PPC440_PCIX_HOST_BRIDGE(obj) \
+OBJECT_CHECK(PPC440PCIXState, (obj), TYPE_PPC440_PCIX_HOST_BRIDGE)
+
+#define PPC440_PCIX_NR_POMS 3
+#define PPC440_PCIX_NR_PIMS 3
+
+typedef struct PPC440PCIXState {
+PCIHostState parent_obj;
+
+PCIDevice *dev;
+struct PLBOutMap pom[PPC440_PCIX_NR_POMS];
+struct PLBInMap pim[PPC440_PCIX_NR_PIMS];
+uint32_t sts;
+qemu_irq irq[PCI_NUM_PINS];
+AddressSpace bm_as;
+MemoryRegion bm;
+
+MemoryRegion container;
+MemoryRegion iomem;
+MemoryRegion busmem;
+} PPC440PCIXState;
+
+#define PPC440_REG_BASE 0x8
+#define PPC440_REG_SIZE 0xff
+
+#define PCIC0_CFGADDR   0x0
+#define PCIC0_CFGDATA   0x4
+
+#define PCIX0_POM0LAL   0x68
+#define PCIX0_POM0LAH   0x6c
+#define PCIX0_POM0SA0x70
+#define PCIX0_POM0PCIAL 0x74
+#define PCIX0_POM0PCIAH 0x78
+#define PCIX0_POM1LAL   0x7c
+#define PCIX0_POM1LAH   0x80
+#define PCIX0_POM1SA0x84
+#define PCIX0_POM1PCIAL 0x88
+#define PCIX0_POM1PCIAH 0x8c
+#define PCIX0_POM2SA0x90
+
+#define PCIX0_PIM0SAL   0x98
+#define PCIX0_PIM0LAL   0x9c
+#define PCIX0_PIM0LAH   0xa0
+#define PCIX0_PIM1SA0xa4
+#define PCIX0_PIM1LAL   0xa8
+#define PCIX0_PIM1LAH   0xac
+#define PCIX0_PIM2SAL   0xb0
+#define PCIX0_PIM2LAL   0xb4
+#define PCIX0_PIM2LAH   0xb8
+#define PCIX0_PIM0SAH   0xf8
+#define PCIX0_PIM2SAH   0xfc
+
+#define PCIX0_STS   0xe0
+
+#define PCI_ALL_SIZE(PPC440_REG_BASE + PPC440_REG_SIZE)
+
+static void ppc440_pcix_clear_region(MemoryRegion *parent,
+ MemoryRegion *mem)
+{
+if (memory_region_is_mapped(mem)) {
+memory_region_del_subregion(parent, mem);
+object_unparent(OBJECT(mem));
+}
+}
+
+/* DMA mapping */
+static void ppc440_pcix_update_pim(PPC440PCIXState *s, int idx)
+{
+MemoryRegion *mem = >pim[idx].mr;
+char *name;
+uint64_t size;
+
+/* Before we modify anything, unmap and destroy the region */
+ppc440_pcix_clear_region(>bm, mem);
+
+if (!(s->pim[idx].sa & 1)) {
+/* Not enabled, nothing to do */
+return;
+}
+
+name = g_strdup_printf("PCI Inbound Window %d", idx);
+size = ~(s->pim[idx].sa & ~7ULL) + 1;
+memory_region_init_alias(mem, OBJECT(s), name, get_system_memory(),
+ s->pim[idx].la, size);
+memory_region_add_subregion_overlap(>bm, 0, mem, -1);
+   

[Qemu-devel] [PATCH v4 02/11] sdcard: replace DPRINTF() by trace events

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/sd/sd.c | 32 ++--
 hw/sd/trace-events |  6 ++
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index ce1f2fdf76..72e9b47e34 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -40,6 +40,7 @@
 #include "qemu/error-report.h"
 #include "qemu/timer.h"
 #include "qemu/log.h"
+#include "trace.h"
 
 //#define DEBUG_SD 1
 
@@ -132,6 +133,26 @@ struct SDState {
 bool cmd_line;
 };
 
+static const char *sd_state_name(enum SDCardStates state)
+{
+static const char *state_name[] = {
+[sd_idle_state] = "idle",
+[sd_ready_state]= "ready",
+[sd_identification_state]   = "identification",
+[sd_standby_state]  = "standby",
+[sd_transfer_state] = "transfer",
+[sd_sendingdata_state]  = "sendingdata",
+[sd_receivingdata_state]= "receivingdata",
+[sd_programming_state]  = "programming",
+[sd_disconnect_state]   = "disconnect",
+};
+if (state == sd_inactive_state) {
+return "inactive";
+}
+assert(state <= ARRAY_SIZE(state_name));
+return state_name[state];
+}
+
 static uint8_t sd_get_dat_lines(SDState *sd)
 {
 return sd->enable ? sd->dat_lines : 0;
@@ -776,6 +797,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
 uint32_t rca = 0x;
 uint64_t addr = (sd->ocr & (1 << 30)) ? (uint64_t) req.arg << 9 : req.arg;
 
+trace_sdcard_normal_command(req.cmd, req.arg, sd_state_name(sd->state));
+
 /* Not interpreting this as an app command */
 sd->card_status &= ~APP_CMD;
 
@@ -790,7 +813,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
 sd->multi_blk_cnt = 0;
 }
 
-DPRINTF("CMD%d 0x%08x state %d\n", req.cmd, req.arg, sd->state);
 switch (req.cmd) {
 /* Basic commands (Class 0 and Class 1) */
 case 0:/* CMD0:   GO_IDLE_STATE */
@@ -1310,8 +1332,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
 return sd_r1;
 
 case 56:   /* CMD56:  GEN_CMD */
-fprintf(stderr, "SD: GEN_CMD 0x%08x\n", req.arg);
-
 switch (sd->state) {
 case sd_transfer_state:
 sd->data_offset = 0;
@@ -1345,7 +1365,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
 static sd_rsp_type_t sd_app_command(SDState *sd,
 SDRequest req)
 {
-DPRINTF("ACMD%d 0x%08x\n", req.cmd, req.arg);
+trace_sdcard_app_command(req.cmd, req.arg);
 sd->card_status |= APP_CMD;
 switch (req.cmd) {
 case 6:/* ACMD6:  SET_BUS_WIDTH */
@@ -1606,8 +1626,7 @@ send_response:
 
 static void sd_blk_read(SDState *sd, uint64_t addr, uint32_t len)
 {
-DPRINTF("sd_blk_read: addr = 0x%08llx, len = %d\n",
-(unsigned long long) addr, len);
+trace_sdcard_read_block(addr, len);
 if (!sd->blk || blk_pread(sd->blk, addr, sd->data, len) < 0) {
 fprintf(stderr, "sd_blk_read: read error on host side\n");
 }
@@ -1615,6 +1634,7 @@ static void sd_blk_read(SDState *sd, uint64_t addr, 
uint32_t len)
 
 static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len)
 {
+trace_sdcard_write_block(addr, len);
 if (!sd->blk || blk_pwrite(sd->blk, addr, sd->data, len, 0) < 0) {
 fprintf(stderr, "sd_blk_write: write error on host side\n");
 }
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 0f8536db32..75dac5a2cd 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -23,6 +23,12 @@ sdhci_read_dataport(uint16_t data_count) "all %u bytes of 
data have been read fr
 sdhci_write_dataport(uint16_t data_count) "write buffer filled with %u bytes 
of data"
 sdhci_capareg(const char *desc, uint16_t val) "%s: %u"
 
+# hw/sd/sd.c
+sdcard_normal_command(uint8_t cmd, uint32_t arg, const char *state) "CMD%d arg 
0x%08x (state %s)"
+sdcard_app_command(uint8_t acmd, uint32_t arg) "ACMD%d arg 0x%08x"
+sdcard_read_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size 0x%x"
+sdcard_write_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size 0x%x"
+
 # 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.16.1

[Qemu-devel] [PATCH v4 03/11] sdcard: add a trace event for command responses

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
---
 hw/sd/sd.c | 27 ---
 hw/sd/trace-events |  1 +
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 72e9b47e34..8f72cde534 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -153,6 +153,27 @@ static const char *sd_state_name(enum SDCardStates state)
 return state_name[state];
 }
 
+static const char *sd_response_name(sd_rsp_type_t rsp)
+{
+static const char *response_name[] = {
+[sd_r0] = "RESP#0 (no response)",
+[sd_r1] = "RESP#1 (normal cmd)",
+[sd_r2_i]   = "RESP#2 (CID reg)",
+[sd_r2_s]   = "RESP#2 (CSD reg)",
+[sd_r3] = "RESP#3 (OCR reg)",
+[sd_r6] = "RESP#6 (RCA)",
+[sd_r7] = "RESP#7 (operating voltage)",
+};
+if (rsp == sd_illegal) {
+return "ILLEGAL RESP";
+}
+if (rsp == sd_r1b) {
+rsp = sd_r1;
+}
+assert(rsp <= ARRAY_SIZE(response_name));
+return response_name[rsp];
+}
+
 static uint8_t sd_get_dat_lines(SDState *sd)
 {
 return sd->enable ? sd->dat_lines : 0;
@@ -1596,10 +1617,12 @@ send_response:
 
 case sd_r0:
 case sd_illegal:
-default:
 rsplen = 0;
 break;
+default:
+g_assert_not_reached();
 }
+trace_sdcard_response(sd_response_name(rtype), rsplen);
 
 if (rtype != sd_illegal) {
 /* Clear the "clear on valid command" status bits now we've
@@ -1616,8 +1639,6 @@ send_response:
 DPRINTF(" %02x", response[i]);
 }
 DPRINTF(" state %d\n", sd->state);
-} else {
-DPRINTF("No response %d\n", sd->state);
 }
 #endif
 
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 75dac5a2cd..b2aa19ec0d 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -26,6 +26,7 @@ sdhci_capareg(const char *desc, uint16_t val) "%s: %u"
 # hw/sd/sd.c
 sdcard_normal_command(uint8_t cmd, uint32_t arg, const char *state) "CMD%d arg 
0x%08x (state %s)"
 sdcard_app_command(uint8_t acmd, uint32_t arg) "ACMD%d arg 0x%08x"
+sdcard_response(const char *rspdesc, int rsplen) "%s (sz:%d)"
 sdcard_read_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size 0x%x"
 sdcard_write_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size 0x%x"
 
-- 
2.16.1

[Qemu-devel] [PATCH v4 11/11] sdcard: use the registerfields API to access the OCR register

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
---
 include/hw/sd/sd.h |  1 -
 hw/sd/sd.c | 21 +
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index bf1eb0713c..9bdb3c9285 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -53,7 +53,6 @@
 #define READY_FOR_DATA (1 << 8)
 #define APP_CMD(1 << 5)
 #define AKE_SEQ_ERROR  (1 << 3)
-#define OCR_CCS_BITN30
 
 typedef enum {
 SD_VOLTAGE_0_4V = 400,  /* currently not supported */
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 28837768d4..fbee87afef 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -32,6 +32,7 @@
 #include "qemu/osdep.h"
 #include "hw/qdev.h"
 #include "hw/hw.h"
+#include "hw/registerfields.h"
 #include "sysemu/block-backend.h"
 #include "hw/sd/sd.h"
 #include "qapi/error.h"
@@ -47,8 +48,6 @@
 //#define DEBUG_SD 1
 
 #define ACMD41_ENQUIRY_MASK 0x00ff
-#define OCR_POWER_UP0x8000
-#define OCR_POWER_DELAY_NS  50 /* 0.5ms */
 
 typedef enum {
 sd_r0 = 0,/* no response */
@@ -272,6 +271,11 @@ static uint16_t sd_crc16(void *message, size_t width)
 return shift_reg;
 }
 
+#define OCR_POWER_DELAY_NS  50 /* 0.5ms */
+
+FIELD(OCR, CARD_CAPACITY,  30,  1) /* 0:SDSC, 1:SDHC/SDXC */
+FIELD(OCR, CARD_POWER_UP,  31,  1)
+
 static void sd_set_ocr(SDState *sd)
 {
 /* All voltages OK, Standard Capacity SD Memory Card, not yet powered up */
@@ -283,9 +287,10 @@ static void sd_ocr_powerup(void *opaque)
 SDState *sd = opaque;
 
 trace_sdcard_powerup();
-/* Set powered up bit in OCR */
-assert(!(sd->ocr & OCR_POWER_UP));
-sd->ocr |= OCR_POWER_UP;
+assert(!FIELD_EX32(sd->ocr, OCR, CARD_POWER_UP));
+
+/* card power-up OK */
+sd->ocr = FIELD_DP32(sd->ocr, OCR, CARD_POWER_UP, 1);
 }
 
 static void sd_set_scr(SDState *sd)
@@ -571,7 +576,7 @@ static bool sd_ocr_vmstate_needed(void *opaque)
 SDState *sd = opaque;
 
 /* Include the OCR state (and timer) if it is not yet powered up */
-return !(sd->ocr & OCR_POWER_UP);
+return !FIELD_EX32(sd->ocr, OCR, CARD_POWER_UP);
 }
 
 static const VMStateDescription sd_ocr_vmstate = {
@@ -681,7 +686,7 @@ static void sd_erase(SDState *sd)
 return;
 }
 
-if (extract32(sd->ocr, OCR_CCS_BITN, 1)) {
+if (FIELD_EX32(sd->ocr, OCR, CARD_CAPACITY)) {
 /* High capacity memory card: erase units are 512 byte blocks */
 erase_start *= 512;
 erase_end *= 512;
@@ -1473,7 +1478,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
  * UEFI, which sends an initial enquiry ACMD41, but
  * assumes that the card is in ready state as soon as it
  * sees the power up bit set. */
-if (!(sd->ocr & OCR_POWER_UP)) {
+if (!FIELD_EX32(sd->ocr, OCR, CARD_POWER_UP)) {
 if ((req.arg & ACMD41_ENQUIRY_MASK) != 0) {
 timer_del(sd->ocr_power_timer);
 sd_ocr_powerup(sd);
-- 
2.16.1

[Qemu-devel] [PATCH v4 00/11] SDCard: housekeeping, add tracing (part 4)

[Philippe Mathieu-Daudé]

Since v3:
- use assert() in sd_state_name() and sd_response_name() (Alistair review)
- added sdmmc-internal.h & sdmmc-common.c to reuse helpers with hw/sd/core.c

Since v2:
- split again in 2... this part is cleanup/tracing
- add more tracepoints
- move some code reusable by sdbus in sdmmc-internal.h

Since v1:
- rewrote mostly all patches to keep it simpler.

$ git backport-diff
001/11:[] [--] 'sdcard: reorder SDState struct members'
002/11:[0003] [FC] 'sdcard: replace DPRINTF() by trace events'
003/11:[0001] [FC] 'sdcard: add a trace event for command responses'
004/11:[0007] [FC] 'sdcard: replace fprintf() by qemu_hexdump()'
005/11:[] [--] 'sdcard: add more trace events'
006/11:[] [--] 'sdcard: do not trace CMD55 when expecting ACMD'
007/11:[0014] [FC] 'sdcard: define SDMMC_CMD_MAX instead of using the magic 
'64''
008/11:[0020] [FC] 'sdcard: display command name when tracing CMD/ACMD'
009/11:[] [--] 'sdcard: display protocol used when tracing'
010/11:[] [-C] 'sdcard: use G_BYTE from cutils'
011/11:[] [-C] 'sdcard: use the registerfields API to access the OCR 
register'

Philippe Mathieu-Daudé (11):
  sdcard: reorder SDState struct members
  sdcard: replace DPRINTF() by trace events
  sdcard: add a trace event for command responses
  sdcard: replace fprintf() by qemu_hexdump()
  sdcard: add more trace events
  sdcard: do not trace CMD55 when expecting ACMD
  sdcard: define SDMMC_CMD_MAX instead of using the magic '64'
  sdcard: display command name when tracing CMD/ACMD
  sdcard: display protocol used when tracing
  sdcard: use G_BYTE from cutils
  sdcard: use the registerfields API to access the OCR register

 hw/sd/sdmmc-internal.h |  18 +
 include/hw/sd/sd.h |   1 -
 hw/sd/sd.c | 184 ++---
 hw/sd/sdmmc-common.c   |  72 +++
 hw/sd/Makefile.objs|   2 +-
 hw/sd/trace-events |  20 ++
 6 files changed, 241 insertions(+), 56 deletions(-)
 create mode 100644 hw/sd/sdmmc-internal.h
 create mode 100644 hw/sd/sdmmc-common.c

-- 
2.16.1

[Qemu-devel] [PATCH v4 01/11] sdcard: reorder SDState struct members

[Philippe Mathieu-Daudé]

place card registers first, this will ease further code movements.

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
---
 hw/sd/sd.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 9ac9b63ff8..ce1f2fdf76 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -88,16 +88,21 @@ enum SDCardStates {
 struct SDState {
 DeviceState parent_obj;
 
-uint32_t mode;/* current card mode, one of SDCardModes */
-int32_t state;/* current card state, one of SDCardStates */
+/* SD Memory Card Registers */
 uint32_t ocr;
-QEMUTimer *ocr_power_timer;
 uint8_t scr[8];
 uint8_t cid[16];
 uint8_t csd[16];
 uint16_t rca;
 uint32_t card_status;
 uint8_t sd_status[64];
+
+/* Configurable properties */
+BlockBackend *blk;
+bool spi;
+
+uint32_t mode;/* current card mode, one of SDCardModes */
+int32_t state;/* current card state, one of SDCardStates */
 uint32_t vhs;
 bool wp_switch;
 unsigned long *wp_groups;
@@ -110,8 +115,6 @@ struct SDState {
 uint8_t pwd[16];
 uint32_t pwd_len;
 uint8_t function_group[6];
-
-bool spi;
 uint8_t current_cmd;
 /* True if we will handle the next command as an ACMD. Note that this does
  * *not* track the APP_CMD status bit!
@@ -123,8 +126,7 @@ struct SDState {
 uint8_t data[512];
 qemu_irq readonly_cb;
 qemu_irq inserted_cb;
-BlockBackend *blk;
-
+QEMUTimer *ocr_power_timer;
 bool enable;
 uint8_t dat_lines;
 bool cmd_line;
-- 
2.16.1

[Qemu-devel] [PATCH v4 04/11] sdcard: replace fprintf() by qemu_hexdump()

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Alistair Francis 
---
 hw/sd/sd.c | 16 +---
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 8f72cde534..ceab263970 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -44,13 +44,6 @@
 
 //#define DEBUG_SD 1
 
-#ifdef DEBUG_SD
-#define DPRINTF(fmt, ...) \
-do { fprintf(stderr, "SD: " fmt , ## __VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) do {} while(0)
-#endif
-
 #define ACMD41_ENQUIRY_MASK 0x00ff
 #define OCR_POWER_UP0x8000
 #define OCR_POWER_DELAY_NS  50 /* 0.5ms */
@@ -1632,14 +1625,7 @@ send_response:
 }
 
 #ifdef DEBUG_SD
-if (rsplen) {
-int i;
-DPRINTF("Response:");
-for (i = 0; i < rsplen; i++) {
-DPRINTF(" %02x", response[i]);
-}
-DPRINTF(" state %d\n", sd->state);
-}
+qemu_hexdump((const char *)response, stderr, "Response", rsplen);
 #endif
 
 return rsplen;
-- 
2.16.1

Re: [Qemu-devel] [PATCH 2/9] nbd: change indenting in nbd.h

[Eric Blake]

On 02/15/2018 07:51 AM, Vladimir Sementsov-Ogievskiy wrote:

Prepared indenting for the following patch.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  include/block/nbd.h | 22 +++---
  1 file changed, 11 insertions(+), 11 deletions(-)




-#define NBD_OPT_INFO (6)
-#define NBD_OPT_GO   (7)
-#define NBD_OPT_STRUCTURED_REPLY (8)
+#define NBD_OPT_EXPORT_NAME   (1)
+#define NBD_OPT_ABORT (2)


Just one space added? Could go with a bit more, in case some later 
addition is also long.  But that's trivial, so either way,


Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 1/9] nbd/server: add nbd_opt_invalid helper

[Eric Blake]

On 02/15/2018 07:51 AM, Vladimir Sementsov-Ogievskiy wrote:

NBD_REP_ERR_INVALID is often parameter to nbd_opt_drop and it would
be used more in following patches. So, let's add a helper.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  nbd/server.c | 50 --
  1 file changed, 36 insertions(+), 14 deletions(-)


More than twice the lines added compared to what was removed, so it's a 
tough call whether this refactoring makes a common pattern easier or 
just adds burden in tracing what gets executed, just to remove a 
parameter.  I'm not opposed to the patch, but want to see how it helps 
the rest of the series.


At any rate, the conversion itself is done correctly, so if we keep the 
patch, it has earned:


Reviewed-by: Eric Blake 


  
+static int GCC_FMT_ATTR(4, 5)

+nbd_opt_drop(NBDClient *client, uint32_t type, Error **errp,
+ const char *fmt, ...)
+{
+int ret;
+va_list va;
+
+va_start(va, fmt);
+ret = nbd_opt_vdrop(client, type, errp, fmt, va);
+va_end(va);
+
+return ret;
+}
+
+static int GCC_FMT_ATTR(3, 4)
+nbd_opt_invalid(NBDClient *client, Error **errp, const char *fmt, ...)


No documentation comments?

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 6/6] sdhci-test: fix leaks

[Philippe Mathieu-Daudé]

On 02/15/2018 06:25 PM, Marc-André Lureau wrote:
> Fix the following ASAN reports:
> 
> ==20125==ERROR: LeakSanitizer: detected memory leaks
> 
> Direct leak of 24 byte(s) in 1 object(s) allocated from:
> #0 0x7f0faea03a38 in __interceptor_calloc (/lib64/libasan.so.4+0xdea38)
> #1 0x7f0fae450f75 in g_malloc0 ../glib/gmem.c:124
> #2 0x562fffd526fc in machine_start 
> /home/elmarco/src/qemu/tests/sdhci-test.c:180
> 
> Indirect leak of 152 byte(s) in 1 object(s) allocated from:
> #0 0x7f0faea03850 in malloc (/lib64/libasan.so.4+0xde850)
> #1 0x7f0fae450f0c in g_malloc ../glib/gmem.c:94
> #2 0x562fffd5d21d in qpci_init_pc 
> /home/elmarco/src/qemu/tests/libqos/pci-pc.c:122

Oops my bad, thanks!

> Signed-off-by: Marc-André Lureau 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  tests/sdhci-test.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tests/sdhci-test.c b/tests/sdhci-test.c
> index 493023fd0c..8a7099398c 100644
> --- a/tests/sdhci-test.c
> +++ b/tests/sdhci-test.c
> @@ -209,8 +209,10 @@ static QSDHCI *machine_start(const struct sdhci_t *test)
>  
>  static void machine_stop(QSDHCI *s)
>  {
> +qpci_free_pc(s->pci.bus);
>  g_free(s->pci.dev);
>  qtest_quit(global_qtest);
> +g_free(s);
>  }
>  
>  static void test_machine(const void *data)
> 

[Qemu-devel] [PATCH 6/6] sdhci-test: fix leaks

[Marc-André Lureau]

Fix the following ASAN reports:

==20125==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 24 byte(s) in 1 object(s) allocated from:
#0 0x7f0faea03a38 in __interceptor_calloc (/lib64/libasan.so.4+0xdea38)
#1 0x7f0fae450f75 in g_malloc0 ../glib/gmem.c:124
#2 0x562fffd526fc in machine_start 
/home/elmarco/src/qemu/tests/sdhci-test.c:180

Indirect leak of 152 byte(s) in 1 object(s) allocated from:
#0 0x7f0faea03850 in malloc (/lib64/libasan.so.4+0xde850)
#1 0x7f0fae450f0c in g_malloc ../glib/gmem.c:94
#2 0x562fffd5d21d in qpci_init_pc 
/home/elmarco/src/qemu/tests/libqos/pci-pc.c:122

Signed-off-by: Marc-André Lureau 
---
 tests/sdhci-test.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/sdhci-test.c b/tests/sdhci-test.c
index 493023fd0c..8a7099398c 100644
--- a/tests/sdhci-test.c
+++ b/tests/sdhci-test.c
@@ -209,8 +209,10 @@ static QSDHCI *machine_start(const struct sdhci_t *test)
 
 static void machine_stop(QSDHCI *s)
 {
+qpci_free_pc(s->pci.bus);
 g_free(s->pci.dev);
 qtest_quit(global_qtest);
+g_free(s);
 }
 
 static void test_machine(const void *data)
-- 
2.16.1.73.g5832b7e9f2

[Qemu-devel] [PATCH 3/6] vhost-user-test: add back memfd check

[Marc-André Lureau]

This revert commit fb68096da3d35e64c88cd610c1fa42766c58e92a, and
modify test_read_guest_mem() to use different chardev names, when
using memfd (_test_server_free(), where the chardev is removed, runs
in idle).

Signed-off-by: Marc-André Lureau 
---
 tests/vhost-user-test.c | 93 +++--
 1 file changed, 66 insertions(+), 27 deletions(-)

diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index a217353e2c..67e5f7f858 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -18,6 +18,7 @@
 #include "qemu/range.h"
 #include "qemu/sockets.h"
 #include "chardev/char-fe.h"
+#include "qemu/memfd.h"
 #include "sysemu/sysemu.h"
 #include "libqos/libqos.h"
 #include "libqos/pci-pc.h"
@@ -40,23 +41,14 @@
 #define HAVE_MONOTONIC_TIME
 #endif
 
-#define QEMU_CMD_MEM" -m %d -object memory-backend-file,id=mem,size=%dM,"\
+#define QEMU_CMD_MEM" -m %d -object memory-backend-file,id=mem,size=%dM," \
 "mem-path=%s,share=on -numa node,memdev=mem"
+#define QEMU_CMD_MEMFD  " -m %d -object memory-backend-memfd,id=mem,size=%dM," 
\
+" -numa node,memdev=mem"
 #define QEMU_CMD_CHR" -chardev socket,id=%s,path=%s%s"
 #define QEMU_CMD_NETDEV " -netdev vhost-user,id=net0,chardev=%s,vhostforce"
 #define QEMU_CMD_NET" -device virtio-net-pci,netdev=net0"
 
-#define QEMU_CMDQEMU_CMD_MEM QEMU_CMD_CHR \
-QEMU_CMD_NETDEV QEMU_CMD_NET
-
-#define GET_QEMU_CMD(s) \
-g_strdup_printf(QEMU_CMD, 512, 512, (root), (s)->chr_name,  \
-(s)->socket_path, "", (s)->chr_name)
-
-#define GET_QEMU_CMDE(s, mem, chr_opts, extra, ...) \
-g_strdup_printf(QEMU_CMD extra, (mem), (mem), (root), (s)->chr_name, \
-(s)->socket_path, (chr_opts), (s)->chr_name, ##__VA_ARGS__)
-
 #define HUGETLBFS_MAGIC   0x958458f6
 
 /*** FROM hw/virtio/vhost-user.c */
@@ -175,6 +167,33 @@ static void test_server_listen(TestServer *server);
 static const char *tmpfs;
 static const char *root;
 
+enum test_memfd {
+TEST_MEMFD_AUTO,
+TEST_MEMFD_YES,
+TEST_MEMFD_NO,
+};
+
+static char *get_qemu_cmd(TestServer *s,
+  int mem, enum test_memfd memfd, const char *mem_path,
+  const char *chr_opts, const char *extra)
+{
+if (memfd == TEST_MEMFD_AUTO && qemu_memfd_check()) {
+memfd = TEST_MEMFD_YES;
+}
+
+if (memfd == TEST_MEMFD_YES) {
+return g_strdup_printf(QEMU_CMD_MEMFD QEMU_CMD_CHR
+   QEMU_CMD_NETDEV QEMU_CMD_NET "%s", mem, mem,
+   s->chr_name, s->socket_path,
+   chr_opts, s->chr_name, extra);
+} else {
+return g_strdup_printf(QEMU_CMD_MEM QEMU_CMD_CHR
+   QEMU_CMD_NETDEV QEMU_CMD_NET "%s", mem, mem,
+   mem_path, s->chr_name, s->socket_path,
+   chr_opts, s->chr_name, extra);
+}
+}
+
 static void init_virtio_dev(TestServer *s, uint32_t features_mask)
 {
 uint32_t features;
@@ -640,16 +659,18 @@ GSourceFuncs test_migrate_source_funcs = {
 .check = test_migrate_source_check,
 };
 
-static void test_read_guest_mem(void)
+static void test_read_guest_mem(const void *arg)
 {
+enum test_memfd memfd = GPOINTER_TO_INT(arg);
 TestServer *server = NULL;
 char *qemu_cmd = NULL;
 QTestState *s = NULL;
 
-server = test_server_new("test");
+server = test_server_new(memfd == TEST_MEMFD_YES ?
+ "read-guest-memfd" : "read-guest-mem");
 test_server_listen(server);
 
-qemu_cmd = GET_QEMU_CMD(server);
+qemu_cmd = get_qemu_cmd(server, 512, memfd, root, "", "");
 
 s = qtest_start(qemu_cmd);
 g_free(qemu_cmd);
@@ -671,7 +692,7 @@ static void test_migrate(void)
 char *uri = g_strdup_printf("%s%s", "unix:", dest->mig_path);
 QTestState *global = global_qtest, *from, *to;
 GSource *source;
-gchar *cmd;
+gchar *cmd, *tmp;
 QDict *rsp;
 guint8 *log;
 guint64 size;
@@ -679,7 +700,7 @@ static void test_migrate(void)
 test_server_listen(s);
 test_server_listen(dest);
 
-cmd = GET_QEMU_CMDE(s, 2, "", "");
+cmd = get_qemu_cmd(s, 2, TEST_MEMFD_AUTO, root, "", "");
 from = qtest_start(cmd);
 g_free(cmd);
 
@@ -688,7 +709,9 @@ static void test_migrate(void)
 size = get_log_size(s);
 g_assert_cmpint(size, ==, (2 * 1024 * 1024) / (VHOST_LOG_PAGE * 8));
 
-cmd = GET_QEMU_CMDE(dest, 2, "", " -incoming %s", uri);
+tmp = g_strdup_printf(" -incoming %s", uri);
+cmd = get_qemu_cmd(dest, 2, TEST_MEMFD_AUTO, root, "", tmp);
+g_free(tmp);
 to = qtest_init(cmd);
 g_free(cmd);
 
@@ -801,7 +824,7 @@ static void test_reconnect_subprocess(void)
 char *cmd;
 
   

[Qemu-devel] [PATCH 1/6] build-sys: fix -fsanitize=address check

[Marc-André Lureau]

Since 218bb57dd79d6843e0592c30a82ea8c1fddc74a5, the -fsanitize=address
check fails with:
config-temp/qemu-conf.c:3:20: error: integer overflow in expression 
[-Werror=overflow]
   return INT32_MIN / -1;

Interestingly, UBSAN check doesn't produce a compile time warning.
Use a test that doesn't have compile time warnings, and make it
specific to UBSAN check.

Signed-off-by: Marc-André Lureau 
---
 configure | 22 --
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/configure b/configure
index 913e14839d..cc610823e1 100755
--- a/configure
+++ b/configure
@@ -5306,25 +5306,27 @@ fi
 ##
 # checks for sanitizers
 
-# we could use a simple skeleton for flags checks, but this also
-# detect the static linking issue of ubsan, see also:
-# https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84285
-cat > $TMPC << EOF
-#include 
-int main(void) {
-  return INT32_MIN / -1;
-}
-EOF
-
 have_asan=no
 have_ubsan=no
 have_asan_iface_h=no
 have_asan_iface_fiber=no
 
 if test "$sanitizers" = "yes" ; then
+  write_c_skeleton
   if compile_prog "$CPU_CFLAGS -Werror -fsanitize=address" ""; then
   have_asan=yes
   fi
+
+  # we could use a simple skeleton for flags checks, but this also
+  # detect the static linking issue of ubsan, see also:
+  # https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84285
+  cat > $TMPC << EOF
+#include 
+int main(void) {
+void *tmp = malloc(10);
+return *(int *)(tmp + 2);
+}
+EOF
   if compile_prog "$CPU_CFLAGS -Werror -fsanitize=undefined" ""; then
   have_ubsan=yes
   fi
-- 
2.16.1.73.g5832b7e9f2

[Qemu-devel] [PATCH 5/6] ahci-test: fix opts leak of skip tests

[Marc-André Lureau]

Fixes the following ASAN report:

Direct leak of 128 byte(s) in 8 object(s) allocated from:
#0 0x7fefce311850 in malloc (/lib64/libasan.so.4+0xde850)
#1 0x7fefcdd5ef0c in g_malloc ../glib/gmem.c:94
#2 0x559b976faff0 in create_ahci_io_test 
/home/elmarco/src/qemu/tests/ahci-test.c:1810

Signed-off-by: Marc-André Lureau 
---
 tests/ahci-test.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/ahci-test.c b/tests/ahci-test.c
index 7aa5af428c..1bd3cc7ca8 100644
--- a/tests/ahci-test.c
+++ b/tests/ahci-test.c
@@ -1822,6 +1822,7 @@ static void create_ahci_io_test(enum IOMode type, enum 
AddrMode addr,
 if ((addr == ADDR_MODE_LBA48) && (offset == OFFSET_HIGH) &&
 (mb_to_sectors(test_image_size_mb) <= 0xFFF)) {
 g_test_message("%s: skipped; test image too small", name);
+g_free(opts);
 g_free(name);
 return;
 }
-- 
2.16.1.73.g5832b7e9f2

[Qemu-devel] [PATCH 0/6] vhost-user-test and leak fixes

[Marc-André Lureau]

Hi,

The following patches fix a regression introduced in commit
218bb57dd79d that prevent ASAN from being detected & used.  There is
also a works around for a GCC ASAN optimization bug. A few test leaks
are fixed, and a few patches reenable vhost-user memfd test fixing the
recent race bug that was identified in the previous version.

Marc-André Lureau (6):
  build-sys: fix -fsanitize=address check
  lockable: workaround GCC link issue with ASAN
  vhost-user-test: add back memfd check
  vhost-user-test: do not hang if chardev creation failed
  ahci-test: fix opts leak of skip tests
  sdhci-test: fix leaks

 include/qemu/lockable.h |  2 +-
 tests/ahci-test.c   |  1 +
 tests/sdhci-test.c  |  2 ++
 tests/vhost-user-test.c | 94 +++--
 configure   | 22 ++--
 5 files changed, 83 insertions(+), 38 deletions(-)

-- 
2.16.1.73.g5832b7e9f2

[Qemu-devel] [PATCH 4/6] vhost-user-test: do not hang if chardev creation failed

[Marc-André Lureau]

Before the chardev name fix, the following error may happen: "attempt
to add duplicate property 'chr-test' to object (type 'container')",
due to races.

Sadly, error_vprintf() uses g_test_message(), so you have to use
read the cryptic --debug-log to see it. Later, it would make sense to
use g_critical() instead, and catch errors with
g_test_expect_message() (in glib 2.34).

Signed-off-by: Marc-André Lureau 
---
 tests/vhost-user-test.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/vhost-user-test.c b/tests/vhost-user-test.c
index 67e5f7f858..efd28411d3 100644
--- a/tests/vhost-user-test.c
+++ b/tests/vhost-user-test.c
@@ -513,6 +513,7 @@ static void test_server_create_chr(TestServer *server, 
const gchar *opt)
 chr = qemu_chr_new(server->chr_name, chr_path);
 g_free(chr_path);
 
+g_assert_nonnull(chr);
 qemu_chr_fe_init(>chr, chr, _abort);
 qemu_chr_fe_set_handlers(>chr, chr_can_read, chr_read,
  chr_event, NULL, server, NULL, true);
-- 
2.16.1.73.g5832b7e9f2

[Qemu-devel] [PATCH 2/6] lockable: workaround GCC link issue with ASAN

[Marc-André Lureau]

Current GCC has an optimization bug when compiling with ASAN.

See also GCC bug:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84307

Signed-off-by: Marc-André Lureau 
---
 include/qemu/lockable.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/qemu/lockable.h b/include/qemu/lockable.h
index b6ed6c89ec..84ea794bcf 100644
--- a/include/qemu/lockable.h
+++ b/include/qemu/lockable.h
@@ -28,7 +28,7 @@ struct QemuLockable {
  * to QEMU_MAKE_LOCKABLE.  For optimized builds, we can rely on dead-code 
elimination
  * from the compiler, and give the errors already at link time.
  */
-#ifdef __OPTIMIZE__
+#if defined(__OPTIMIZE__) && !defined(__SANITIZE_ADDRESS__)
 void unknown_lock_type(void *);
 #else
 static inline void unknown_lock_type(void *unused)
-- 
2.16.1.73.g5832b7e9f2

Re: [Qemu-devel] [PATCH v4 1/2] qmp: adding 'wakeup-suspend-support' in query-target

[Michael Roth]

Quoting Daniel Henrique Barboza (2018-01-05 07:03:13)
> When issuing the qmp/hmp 'system_wakeup' command, what happens in a
> nutshell is:
> 
> - qmp_system_wakeup_request set runstate to RUNNING, sets a wakeup_reason
> and notify the event
> - in the main_loop, all vcpus are paused, a system reset is issued, all
> subscribers of wakeup_notifiers receives a notification, vcpus are then
> resumed and the wake up QAPI event is fired
> 
> Note that this procedure alone doesn't ensure that the guest will awake
> from SUSPENDED state - the subscribers of the wake up event must take
> action to resume the guest, otherwise the guest will simply reboot.
> 
> At this moment there are only two subscribers of the wake up event: one
> in hw/acpi/core.c and another one in hw/i386/xen/xen-hvm.c. This means
> that system_wakeup does not work as intended with other architectures.
> 
> However, only the presence of 'system_wakeup' is required for QGA to
> support 'guest-suspend-ram' and 'guest-suspend-hybrid' at this moment.
> This means that the user/management will expect to suspend the guest using
> one of those suspend commands and then resume execution using system_wakeup,
> regardless of the support offered in system_wakeup in the first place.
> 
> This patch adds a new flag called 'wakeup-suspend-support' in TargetInfo
> that allows the caller to query if the guest supports wake up from
> suspend via system_wakeup. It goes over the subscribers of the wake up
> event and, if it's empty, it assumes that the guest does not support
> wake up from suspend (and thus, pm-suspend itself).
> 
> This is the expected output of query-target when running a x86 guest:
> 
> {"execute" : "query-target"}
> {"return": {"arch": "x86_64", "wakeup-suspend-support": true}}
> 
> This is the output when running a pseries guest:
> 
> {"execute" : "query-target"}
> {"return": {"arch": "ppc64", "wakeup-suspend-support": false}}
> 
> Given that the TargetInfo structure is read-only, adding a new flag to
> it is backwards compatible. There is no need to deprecate the old
> TargetInfo format.
> 
> With this extra tool, management can avoid situations where a guest
> that does not have proper suspend/wake capabilities ends up in
> inconsistent state (e.g.
> https://github.com/open-power-host-os/qemu/issues/31).
> 
> Signed-off-by: Daniel Henrique Barboza 
> ---
>  arch_init.c |  1 +
>  include/sysemu/sysemu.h |  1 +
>  qapi-schema.json|  4 +++-
>  vl.c| 29 +
>  4 files changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/arch_init.c b/arch_init.c
> index a0b8ed6167..9c5c519d9d 100644
> --- a/arch_init.c
> +++ b/arch_init.c
> @@ -109,6 +109,7 @@ TargetInfo *qmp_query_target(Error **errp)
>  TargetInfo *info = g_malloc0(sizeof(*info));
> 
>  info->arch = g_strdup(TARGET_NAME);
> +info->wakeup_suspend_support = !qemu_wakeup_notifier_is_empty();
> 
>  return info;
>  }
> diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
> index 31612caf10..b15374e0b8 100644
> --- a/include/sysemu/sysemu.h
> +++ b/include/sysemu/sysemu.h
> @@ -69,6 +69,7 @@ typedef enum WakeupReason {
>  void qemu_system_reset_request(ShutdownCause reason);
>  void qemu_system_suspend_request(void);
>  void qemu_register_suspend_notifier(Notifier *notifier);
> +bool qemu_wakeup_notifier_is_empty(void);
>  void qemu_system_wakeup_request(WakeupReason reason);
>  void qemu_system_wakeup_enable(WakeupReason reason, bool enabled);
>  void qemu_register_wakeup_notifier(Notifier *notifier);
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 5c06745c79..2f1c08fde7 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -2388,11 +2388,13 @@
>  # Information describing the QEMU target.
>  #
>  # @arch: the target architecture (eg "x86_64", "i386", etc)
> +# @wakeup-suspend-support: true if the target supports wake up from
> +#  suspend (since 2.12)
>  #
>  # Since: 1.2.0
>  ##
>  { 'struct': 'TargetInfo',
> -  'data': { 'arch': 'str' } }
> +  'data': { 'arch': 'str', 'wakeup-suspend-support': 'bool' } }
> 
>  ##
>  # @query-target:
> diff --git a/vl.c b/vl.c
> index d3a5c5d021..56d6a1ce5b 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -1839,11 +1839,40 @@ void qemu_system_wakeup_enable(WakeupReason reason, 
> bool enabled)
>  }
>  }
> 
> +/* The existence of a wake-up notifier is being checked in the function
> + * qemu_wakeup_notifier_is_empty and it's used in the logic of the
> + * wakeup-suspend-support flag of QMP 'query-target' command. The idea
> + * of this flag is to indicate whether the guest supports wake-up from
> + * suspend (via system_wakeup QMP/HMP call for example), warning the user
> + * that the guest can't handle both wake-up from suspend and the suspend
> + * itself via QGA guest-suspend-ram and guest-suspend-hybrid (if it
> + * can't wake up, it can't be suspended safely).
> + *
> + * The motivation of this 

Re: [Qemu-devel] [PATCH 01/02] fix issue where a branch to pc+4 confuses GDB because pc and npc are set to the same value

[Steven Seeger]

Oh, let me also add that this assertion fails in gdb: 

Thread 1 hit Breakpoint 3, 0x40f102d8 in ?? ()
(gdb) si
./../../src/gdb-8.1/gdb/sparc-tdep.c:1737: internal-error: std::vector sparc_software_single_step(regcache*): Assertion
`nnpc != npc || orig_npc == 0' failed.

I had this assertion commented out so when I unrolled my patch to repeat the 
test in order to write my previous reply to you, I saw it be stuck on the same 
instruction rather than fail this assertion.

I guess this could be a gdb bug, too.

Steven

Re: [Qemu-devel] [PATCH 01/02] fix issue where a branch to pc+4 confuses GDB because pc and npc are set to the same value

[Steven Seeger]

On Thursday, February 15, 2018 2:39:34 PM EST Peter Maydell wrote:
> On 15 February 2018 at 19:15, Steven Seeger
> These changes look rather odd -- are you sure they're right?
> This is the code for unconditional taken branch, not annulled, and
> my copy of the sparc architecture manual says that in that case
> the new PC value should be the old nPC value, and the new
> nPC value should be the effective address of the branch target.
> There's nothing in there about branches into your own delay
> slot being a special case. Adding 4 to target like this will
> make the new nPC value be 4 further forward, which would mean
> we'd only execute the branch target instruction once, rather
> than twice (once for it being in the branch delay slot and
> once as the instruction target).
> 
> It's a weird thing to do so I wouldn't be surprised if gdb
> mishandled it. Have you tested against real sparc hardware?
> 
> thanks
> -- PMM

Hi Peter. Thank you for the thoughtful reply. I did not consider the 
possibility that in this case the instruction should execute twice. The issue 
here is that when stepping through code in gdb, after the branch to the delay 
slot, pc==npc. So subsequent steps actually go nowhere. This could maybe be a 
problem with GDB, but I don't think so.

I will have access to real hardware Tuesday and will test this case.

If the instruction should execute twice, then we may need to kick npc in qemu 
in order to alleviate this.

Steven

Re: [Qemu-devel] [PULL 00/10] migration queue

[Dr. David Alan Gilbert]

* Peter Maydell (peter.mayd...@linaro.org) wrote:
> On 14 February 2018 at 15:39, Dr. David Alan Gilbert (git)
>  wrote:
> > From: "Dr. David Alan Gilbert" 
> >
> > The following changes since commit bec9c64ef7be8063f1192608b83877bc5c9ea217:
> >
> >   Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into 
> > staging (2018-02-13 18:24:08 +)
> >
> > are available in the Git repository at:
> >
> >   git://github.com/dagrh/qemu.git tags/pull-migration-20180214a
> >
> > for you to fetch changes up to 3e0c8050ebba3f55dc2d92b3790a3cfb80786d07:
> >
> >   migration: pass MigrationState to migrate_init() (2018-02-14 10:37:09 
> > +)
> >
> > 
> > Migration pull 20180214
> >
> > Note that the 'Add test for migration to bad destination' displays
> > a 'Connection refused' during running, but still gives the correct exit
> > code and OK (It's checking that the source doesn't fail when
> > it can't connect, so that's the right error).
> > If it's particularly disliked that patch can be skipped individually.
> >
> > 
> 
> Hi. This fails 'make check' on aarch64 host:
> QTEST_QEMU_BINARY=hppa-softmmu/qemu-system-hppa
> QTEST_QEMU_IMG=qemu-img MALLOC_PERTURB_=${MALLOC_PERTURB_:-$((
> ${RANDOM
> :-0} % 255 + 1))} gtester -k --verbose -m=quick tests/boot-serial-test
> tests/qmp-test tests/device-introspect-test test
> s/qom-test tests/test-hmp
> TEST: tests/boot-serial-test... (pid=2306)
>   /hppa/boot-serial/hppa:  **
> ERROR:/home/pm215/qemu/tests/boot-serial-test.c:137:check_guest_output:
> assertion failed: (output_ok)
> FAIL
> GTester: last random seed: R02S43359b02a322915508b897be44a0e9c6
> (pid=7220)
> FAIL: tests/boot-serial-test

I can't obviously see a reason why it should in any way affect that
test; however I'll try and grab an aarch64 box tomorrow.

Dave

> 
> 
> thanks
> -- PMM
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [Qemu-devel] [PATCH 11/27] block: x-blockdev-create QMP command

[Eric Blake]

On 02/08/2018 01:23 PM, Kevin Wolf wrote:

This adds a synchronous x-blockdev-create QMP command that can create
qcow2 images on a given node name.

We don't want to block while creating an image, so this is not the final
interface in all aspects, but BlockdevCreateOptionsQcow2 and
.bdrv_co_create() are what they actually might look like in the end. In
any case, this should be good enough to test whether we interpret
BlockdevCreateOptions as we should.

Signed-off-by: Kevin Wolf 
---



@@ -3442,6 +3442,18 @@
} }
  
  ##

+# @x-blockdev-create:
+#
+# Create an image format on a given node.
+# TODO Replace with something asynchronous (block job?)


We've learned our lesson - don't commit to the final name on an 
interface that we have not yet experimented with.



+#
+# Since: 2.12
+##
+{ 'command': 'x-blockdev-create',
+  'data': 'BlockdevCreateOptions',
+  'boxed': true }
+


Lots of code packed in that little description ;)



+++ b/include/block/block_int.h
@@ -130,6 +130,8 @@ struct BlockDriver {
  int (*bdrv_file_open)(BlockDriverState *bs, QDict *options, int flags,
Error **errp);
  void (*bdrv_close)(BlockDriverState *bs);
+int coroutine_fn (*bdrv_co_create)(BlockdevCreateOptions *opts,
+   Error **errp);


I know we haven't been very good in the past, but can you add a comment 
here on the contract that drivers are supposed to obey when implementing 
this callback?



+++ b/block.c
@@ -369,7 +369,7 @@ BlockDriver *bdrv_find_format(const char *format_name)
  return bdrv_do_find_format(format_name);
  }
  
-static int bdrv_is_whitelisted(BlockDriver *drv, bool read_only)

+int bdrv_is_whitelisted(BlockDriver *drv, bool read_only)


Worth mentioning that bdrv_is_whitelisted had to be exported as part of 
the commit message?  (Or even promoting it to public in a separate commit?)



+++ b/block/create.c
@@ -0,0 +1,75 @@
+/*
+ * Block layer code related to image creation
+ *
+ * Copyright (c) 2018 Kevin Wolf 


The question came up in another thread, but I didn't hear your answer 
there; I know Red Hat permits you to claim personal copyright while 
still using a redhat.com address for code written on personal time, but 
should this claim belong to Red Hat instead of you?



+/* Call callback if it exists */
+if (!drv->bdrv_co_create) {
+error_setg(errp, "Driver does not support blockdev-create");


Should this error message refer to 'x-blockdev-create' in the short term?

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[Qemu-devel] [PATCH 2/2] target/ppc: convert to TranslatorOps

[Emilio G. Cota]

A few changes worth noting:

- Didn't migrate ctx->exception to DISAS_* since the exception field is
  in many cases architecturally relevant.

- Moved the cross-page check from the end of translate_insn to tb_start.

- Removed the exit(1) after a TCG temp leak; changed the fprintf there to
  qemu_log.

Reviewed-by: Richard Henderson 
Signed-off-by: Emilio G. Cota 
---
 target/ppc/translate.c | 329 +
 1 file changed, 167 insertions(+), 162 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 6e35daa..0a0c090 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -7207,217 +7207,222 @@ void ppc_cpu_dump_statistics(CPUState *cs, FILE*f,
 #endif
 }
 
-/*/
-void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
+static int ppc_tr_init_disas_context(DisasContextBase *dcbase,
+ CPUState *cs, int max_insns)
 {
+DisasContext *ctx = container_of(dcbase, DisasContext, base);
 CPUPPCState *env = cs->env_ptr;
-DisasContext ctx, *ctxp = 
-opc_handler_t **table, *handler;
-int max_insns;
-
-ctx.base.singlestep_enabled = cs->singlestep_enabled;
-ctx.base.tb = tb;
-ctx.base.pc_first = tb->pc;
-ctx.base.pc_next = tb->pc; /* nip */
-ctx.base.num_insns = 0;
-
-ctx.exception = POWERPC_EXCP_NONE;
-ctx.spr_cb = env->spr_cb;
-ctx.pr = msr_pr;
-ctx.mem_idx = env->dmmu_idx;
-ctx.dr = msr_dr;
+int bound;
+
+ctx->exception = POWERPC_EXCP_NONE;
+ctx->spr_cb = env->spr_cb;
+ctx->pr = msr_pr;
+ctx->mem_idx = env->dmmu_idx;
+ctx->dr = msr_dr;
 #if !defined(CONFIG_USER_ONLY)
-ctx.hv = msr_hv || !env->has_hv_mode;
+ctx->hv = msr_hv || !env->has_hv_mode;
 #endif
-ctx.insns_flags = env->insns_flags;
-ctx.insns_flags2 = env->insns_flags2;
-ctx.access_type = -1;
-ctx.need_access_type = !(env->mmu_model & POWERPC_MMU_64B);
-ctx.le_mode = !!(env->hflags & (1 << MSR_LE));
-ctx.default_tcg_memop_mask = ctx.le_mode ? MO_LE : MO_BE;
+ctx->insns_flags = env->insns_flags;
+ctx->insns_flags2 = env->insns_flags2;
+ctx->access_type = -1;
+ctx->need_access_type = !(env->mmu_model & POWERPC_MMU_64B);
+ctx->le_mode = !!(env->hflags & (1 << MSR_LE));
+ctx->default_tcg_memop_mask = ctx->le_mode ? MO_LE : MO_BE;
 #if defined(TARGET_PPC64)
-ctx.sf_mode = msr_is_64bit(env, env->msr);
-ctx.has_cfar = !!(env->flags & POWERPC_FLAG_CFAR);
+ctx->sf_mode = msr_is_64bit(env, env->msr);
+ctx->has_cfar = !!(env->flags & POWERPC_FLAG_CFAR);
 #endif
 if (env->mmu_model == POWERPC_MMU_32B ||
 env->mmu_model == POWERPC_MMU_601 ||
 (env->mmu_model & POWERPC_MMU_64B))
-ctx.lazy_tlb_flush = true;
+ctx->lazy_tlb_flush = true;
 
-ctx.fpu_enabled = !!msr_fp;
+ctx->fpu_enabled = !!msr_fp;
 if ((env->flags & POWERPC_FLAG_SPE) && msr_spe)
-ctx.spe_enabled = !!msr_spe;
+ctx->spe_enabled = !!msr_spe;
 else
-ctx.spe_enabled = false;
+ctx->spe_enabled = false;
 if ((env->flags & POWERPC_FLAG_VRE) && msr_vr)
-ctx.altivec_enabled = !!msr_vr;
+ctx->altivec_enabled = !!msr_vr;
 else
-ctx.altivec_enabled = false;
+ctx->altivec_enabled = false;
 if ((env->flags & POWERPC_FLAG_VSX) && msr_vsx) {
-ctx.vsx_enabled = !!msr_vsx;
+ctx->vsx_enabled = !!msr_vsx;
 } else {
-ctx.vsx_enabled = false;
+ctx->vsx_enabled = false;
 }
 #if defined(TARGET_PPC64)
 if ((env->flags & POWERPC_FLAG_TM) && msr_tm) {
-ctx.tm_enabled = !!msr_tm;
+ctx->tm_enabled = !!msr_tm;
 } else {
-ctx.tm_enabled = false;
+ctx->tm_enabled = false;
 }
 #endif
-ctx.gtse = !!(env->spr[SPR_LPCR] & LPCR_GTSE);
+ctx->gtse = !!(env->spr[SPR_LPCR] & LPCR_GTSE);
 if ((env->flags & POWERPC_FLAG_SE) && msr_se)
-ctx.singlestep_enabled = CPU_SINGLE_STEP;
+ctx->singlestep_enabled = CPU_SINGLE_STEP;
 else
-ctx.singlestep_enabled = 0;
+ctx->singlestep_enabled = 0;
 if ((env->flags & POWERPC_FLAG_BE) && msr_be)
-ctx.singlestep_enabled |= CPU_BRANCH_STEP;
-if (unlikely(ctx.base.singlestep_enabled)) {
-ctx.singlestep_enabled |= GDBSTUB_SINGLE_STEP;
+ctx->singlestep_enabled |= CPU_BRANCH_STEP;
+if (unlikely(ctx->base.singlestep_enabled)) {
+ctx->singlestep_enabled |= GDBSTUB_SINGLE_STEP;
 }
 #if defined (DO_SINGLE_STEP) && 0
 /* Single step trace mode */
 msr_se = 1;
 #endif
-ctx.base.num_insns = 0;
-max_insns = tb_cflags(tb) & CF_COUNT_MASK;
-if (max_insns == 0) {
-max_insns = CF_COUNT_MASK;
-}
-if (max_insns > TCG_MAX_INSNS) {
-max_insns = TCG_MAX_INSNS;
-}
-
-

Re: [Qemu-devel] [PATCH 10/27] qcow2: Use visitor for options in qcow2_create()

[Eric Blake]

On 02/08/2018 01:23 PM, Kevin Wolf wrote:

Signed-off-by: Kevin Wolf 
---
  block/qcow2.c  | 219 -
  tests/qemu-iotests/049.out |   8 +-
  tests/qemu-iotests/112.out |   4 +-
  3 files changed, 84 insertions(+), 147 deletions(-)




  BlockDriverState *bs = NULL;
-Error *local_err = NULL;
+const char *val;
  int ret;
+Error *local_err = NULL;
  


Worth the churn on the local_err declaration position?


-/* Read out options */
-size = ROUND_UP(qemu_opt_get_size_del(opts, BLOCK_OPT_SIZE, 0),
-BDRV_SECTOR_SIZE);
-backing_file = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FILE);
-backing_fmt = qemu_opt_get_del(opts, BLOCK_OPT_BACKING_FMT);
-backing_drv = qapi_enum_parse(_lookup, backing_fmt,
-  0, _err);
-if (local_err) {
-error_propagate(errp, local_err);
+/* Only the keyval visitor supports the dotted syntax needed for
+ * encryption, so go through a QDict before getting a QAPI type. Ignore
+ * options meant for the protocol layer so that the visitor doesn't
+ * complain. */
+qdict = qemu_opts_to_qdict_filtered(opts, NULL, bdrv_qcow2.create_opts,
+true);


Glue code at its finest ;)


+/* Convert compat=0.10/1.1 into compat=v2/v3, to be renamed into
+ * version=v2/v3 below. */
+val = qdict_get_try_str(qdict, BLOCK_OPT_COMPAT_LEVEL);
+if (val && !strcmp(val, "0.10")) {
+qdict_put_str(qdict, BLOCK_OPT_COMPAT_LEVEL, "v2");
+} else if (val && !strcmp(val, "1.1")) {
+qdict_put_str(qdict, BLOCK_OPT_COMPAT_LEVEL, "v3");
+}


Not only does this map the old 'qemu-img create -o' spellings into the 
QMP form, but it means that we now also accept the new spelling via 
qemu-img command line.  Might be worth mentioning in the commit message 
as an intentional enhancement.



+
+/* Change legacy command line options into QMP ones */
+static const QDictRenames opt_renames[] = {
+{ BLOCK_OPT_BACKING_FILE,   "backing-file" },
+{ BLOCK_OPT_BACKING_FMT,"backing-fmt" },
+{ BLOCK_OPT_CLUSTER_SIZE,   "cluster-size" },
+{ BLOCK_OPT_LAZY_REFCOUNTS, "lazy-refcounts" },
+{ BLOCK_OPT_REFCOUNT_BITS,  "refcount-bits" },
+{ BLOCK_OPT_ENCRYPT,BLOCK_OPT_ENCRYPT_FORMAT },
+{ BLOCK_OPT_COMPAT_LEVEL,   "version" },
+{ NULL, NULL },
+};


Looks reasonable to me.


-
-cluster_size = qcow2_opt_get_cluster_size_del(opts, _err);
-if (local_err) {
-error_propagate(errp, local_err);
-ret = -EINVAL;
+/* Create and open the file (protocol layer) */
+ret = bdrv_create_file(filename, opts, errp);
+if (ret < 0) {
  goto finish;


Git got lost on producing a sane diff.  Oh well.


-version = qcow2_opt_get_version_del(opts, _err);
-if (local_err) {
-error_propagate(errp, local_err);
+/* Set 'driver' and 'node' options */
+qdict_put_str(qdict, "driver", "qcow2");
+qdict_put_str(qdict, "file", bs->node_name);
+
+/* Now get the QAPI type BlockdevCreateOptions */
+qobj = qdict_crumple(qdict, errp);
+QDECREF(qdict);
+qdict = qobject_to_qdict(qobj);
+if (qdict == NULL) {


Fun with round trips.  Maybe someday we can improve things, but for now, 
I'm glad that it at least works.



  ret = -EINVAL;
  goto finish;
  }
  
-if (qemu_opt_get_bool_del(opts, BLOCK_OPT_LAZY_REFCOUNTS, false)) {

-flags |= BLOCK_FLAG_LAZY_REFCOUNTS;
-}
+v = qobject_input_visitor_new_keyval(QOBJECT(qdict));
+visit_type_BlockdevCreateOptions(v, NULL, _options, _err);
+visit_free(v);


But this part is definitely the payout of what we wanted to get to!



  /* Create the qcow2 image (format layer) */
-create_options = (BlockdevCreateOptions) {
-.driver = BLOCKDEV_DRIVER_QCOW2,
-.u.qcow2= {
-.file   = &(BlockdevRef) {


And using the visitor is a lot nicer than populating the struct by hand.


+++ b/tests/qemu-iotests/049.out
@@ -166,11 +166,11 @@ qemu-img create -f qcow2 -o compat=1.1 TEST_DIR/t.qcow2 
64M
  Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat=1.1 
cluster_size=65536 lazy_refcounts=off refcount_bits=16
  
  qemu-img create -f qcow2 -o compat=0.42 TEST_DIR/t.qcow2 64M

-qemu-img: TEST_DIR/t.qcow2: Invalid compatibility level: '0.42'
+qemu-img: TEST_DIR/t.qcow2: Invalid parameter '0.42'
  Formatting 'TEST_DIR/t.qcow2', fmt=qcow2 size=67108864 compat=0.42 
cluster_size=65536 lazy_refcounts=off refcount_bits=16


Yep, the visitor has slightly different messages, but I'm fine with the 
fallout.


Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[Qemu-devel] [PATCH 1/2] target/ppc: convert to DisasContextBase

[Emilio G. Cota]

A couple of notes:

- removed ctx->nip in favour of base->pc_next. Yes, it is annoying,
  but didn't want to waste its 4 bytes.

- ctx->singlestep_enabled does a lot more than
  base.singlestep_enabled; this confused me at first.

Reviewed-by: Richard Henderson 
Signed-off-by: Emilio G. Cota 
---
 target/ppc/translate.c  | 129 +++-
 target/ppc/translate/dfp-impl.inc.c |  16 ++---
 target/ppc/translate_init.c |  32 -
 3 files changed, 91 insertions(+), 86 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 4132f67..6e35daa 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -31,6 +31,7 @@
 #include "exec/helper-gen.h"
 
 #include "trace-tcg.h"
+#include "exec/translator.h"
 #include "exec/log.h"
 
 
@@ -187,8 +188,7 @@ void ppc_translate_init(void)
 
 /* internal defines */
 struct DisasContext {
-struct TranslationBlock *tb;
-target_ulong nip;
+DisasContextBase base;
 uint32_t opcode;
 uint32_t exception;
 /* Routine used to access memory */
@@ -275,7 +275,7 @@ static void gen_exception_err(DisasContext *ctx, uint32_t 
excp, uint32_t error)
  * the faulting instruction
  */
 if (ctx->exception == POWERPC_EXCP_NONE) {
-gen_update_nip(ctx, ctx->nip - 4);
+gen_update_nip(ctx, ctx->base.pc_next - 4);
 }
 t0 = tcg_const_i32(excp);
 t1 = tcg_const_i32(error);
@@ -293,7 +293,7 @@ static void gen_exception(DisasContext *ctx, uint32_t excp)
  * the faulting instruction
  */
 if (ctx->exception == POWERPC_EXCP_NONE) {
-gen_update_nip(ctx, ctx->nip - 4);
+gen_update_nip(ctx, ctx->base.pc_next - 4);
 }
 t0 = tcg_const_i32(excp);
 gen_helper_raise_exception(cpu_env, t0);
@@ -322,7 +322,7 @@ static void gen_debug_exception(DisasContext *ctx)
  */
 if ((ctx->exception != POWERPC_EXCP_BRANCH) &&
 (ctx->exception != POWERPC_EXCP_SYNC)) {
-gen_update_nip(ctx, ctx->nip);
+gen_update_nip(ctx, ctx->base.pc_next);
 }
 t0 = tcg_const_i32(EXCP_DEBUG);
 gen_helper_raise_exception(cpu_env, t0);
@@ -349,7 +349,7 @@ static inline void gen_hvpriv_exception(DisasContext *ctx, 
uint32_t error)
 /* Stop translation */
 static inline void gen_stop_exception(DisasContext *ctx)
 {
-gen_update_nip(ctx, ctx->nip);
+gen_update_nip(ctx, ctx->base.pc_next);
 ctx->exception = POWERPC_EXCP_STOP;
 }
 
@@ -978,7 +978,7 @@ static void gen_addpcis(DisasContext *ctx)
 {
 target_long d = DX(ctx->opcode);
 
-tcg_gen_movi_tl(cpu_gpr[rD(ctx->opcode)], ctx->nip + (d << 16));
+tcg_gen_movi_tl(cpu_gpr[rD(ctx->opcode)], ctx->base.pc_next + (d << 16));
 }
 
 static inline void gen_op_arith_divw(DisasContext *ctx, TCGv ret, TCGv arg1,
@@ -1580,7 +1580,7 @@ static void gen_pause(DisasContext *ctx)
 tcg_temp_free_i32(t0);
 
 /* Stop translation, this gives other CPUs a chance to run */
-gen_exception_nip(ctx, EXCP_HLT, ctx->nip);
+gen_exception_nip(ctx, EXCP_HLT, ctx->base.pc_next);
 }
 #endif /* defined(TARGET_PPC64) */
 
@@ -2397,7 +2397,7 @@ static inline void gen_check_align(DisasContext *ctx, 
TCGv EA, int mask)
 tcg_gen_brcondi_tl(TCG_COND_EQ, t0, 0, l1);
 t1 = tcg_const_i32(POWERPC_EXCP_ALIGN);
 t2 = tcg_const_i32(ctx->opcode & 0x03FF);
-gen_update_nip(ctx, ctx->nip - 4);
+gen_update_nip(ctx, ctx->base.pc_next - 4);
 gen_helper_raise_exception_err(cpu_env, t1, t2);
 tcg_temp_free_i32(t1);
 tcg_temp_free_i32(t2);
@@ -3322,7 +3322,7 @@ static void gen_wait(DisasContext *ctx)
-offsetof(PowerPCCPU, env) + offsetof(CPUState, halted));
 tcg_temp_free_i32(t0);
 /* Stop translation, as the CPU is supposed to sleep from now */
-gen_exception_nip(ctx, EXCP_HLT, ctx->nip);
+gen_exception_nip(ctx, EXCP_HLT, ctx->base.pc_next);
 }
 
 #if defined(TARGET_PPC64)
@@ -3407,7 +3407,7 @@ static inline bool use_goto_tb(DisasContext *ctx, 
target_ulong dest)
 }
 
 #ifndef CONFIG_USER_ONLY
-return (ctx->tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK);
+return (ctx->base.tb->pc & TARGET_PAGE_MASK) == (dest & TARGET_PAGE_MASK);
 #else
 return true;
 #endif
@@ -3422,7 +3422,7 @@ static void gen_goto_tb(DisasContext *ctx, int n, 
target_ulong dest)
 if (use_goto_tb(ctx, dest)) {
 tcg_gen_goto_tb(n);
 tcg_gen_movi_tl(cpu_nip, dest & ~3);
-tcg_gen_exit_tb((uintptr_t)ctx->tb + n);
+tcg_gen_exit_tb((uintptr_t)ctx->base.tb + n);
 } else {
 tcg_gen_movi_tl(cpu_nip, dest & ~3);
 if (unlikely(ctx->singlestep_enabled)) {
@@ -3458,14 +3458,14 @@ static void gen_b(DisasContext *ctx)
 li = LI(ctx->opcode);
 li = (li ^ 0x0200) - 0x0200;
 if (likely(AA(ctx->opcode) == 0)) {
-target = ctx->nip + li - 4;
+target = ctx->base.pc_next + li - 4;
 } else {
 target = li;
 

[Qemu-devel] [PATCHv2 0/2] target/ppc: convert to generic translation loop

[Emilio G. Cota]

Changes from v1:
- Removed use of translator_loop_temp_check; call tcg_check_temp_count
  directly.
- Add R-b's.

Thanks,

Emilio

Re: [Qemu-devel] [PULL 0/7] Qio next patches

[Peter Maydell]

On 15 February 2018 at 17:50, Daniel P. Berrangé  wrote:
> The following changes since commit 8c5e7bddc22dac9d4dc3526996babce4c7242d9d:
>
>   Merge remote-tracking branch 'remotes/huth/tags/pull-request-2018-02-14' 
> into staging (2018-02-15 13:00:44 +)
>
> are available in the Git repository at:
>
>   ssh://g...@github.com/berrange/qemu tags/qio-next-pull-request

This isn't a publicly usable git reference; did you mean
   git://github.com/berrange/qemu.git tags/qio-next-pull-request

? (I'm testing the pullreq on the assumption that you did.)

thanks
-- PMM

Re: [Qemu-devel] [PULL 00/20] target-arm queue

[Peter Maydell]

On 15 February 2018 at 18:36, Peter Maydell <peter.mayd...@linaro.org> wrote:
> Changes v1->v2: it turns out that the raspi3 support exposes a
> preexisting bug in our register definitions for VMPIDR/VMIDR:
> https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg04181.html
>
> So I've dropped the final "enable raspi3 board" patch for the
> moment. When that VMIDR/VMPIDR patch gets reviewed we can
> put the raspi3 patch in with it.
>
>
> thanks
> -- PMM
>
> The following changes since commit f003d07337a6d4d02c43429b26a4270459afb51a:
>
>   Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' 
> into staging (2018-02-15 15:45:33 +)
>
> are available in the Git repository at:
>
>   git://git.linaro.org/people/pmaydell/qemu-arm.git 
> tags/pull-target-arm-20180215-1
>
> for you to fetch changes up to bade58166f4466546600d824a2695a00269d10eb:
>
>   raspi: Raspberry Pi 3 support (2018-02-15 18:33:46 +)
>
> 
> target-arm queue:
>  * aspeed: code cleanup to use unimplemented_device
>  * preparatory work for 'raspi3' RaspberryPi 3 machine model
>  * more SVE prep work
>  * v8M: add minor missing registers
>  * v7M: fix bug where we weren't migrating v7m.other_sp
>  * v7M: fix bugs in handling of interrupt registers for
>external interrupts beyond 32
>

Applied this version, thanks.

-- PMM

Re: [Qemu-devel] [PATCH 09/27] qdict: Introduce qdict_rename_keys()

[Eric Blake]

On 02/08/2018 01:23 PM, Kevin Wolf wrote:

A few block drivers will need to rename .bdrv_create options for their
QAPIfication, so let's have a helper function for that.

Signed-off-by: Kevin Wolf 
---
  include/qapi/qmp/qdict.h |  6 ++
  qobject/qdict.c  | 30 ++
  2 files changed, 36 insertions(+)


Again, unit test coverage?


+/**
+ * qdict_rename_keys(): Rename keys in qdict according to the replacements
+ * specified in the array renames. The array must be terminated by an entry
+ * with from = NULL.
+ *
+ * Returns true for success, false in error cases.
+ */
+bool qdict_rename_keys(QDict *qdict, const QDictRenames *renames, Error **errp)
+{
+QObject *qobj;
+
+while (renames->from) {
+if (qdict_haskey(qdict, renames->from)) {
+if (qdict_haskey(qdict, renames->to)) {


Depending on how efficient qdict_haskey() is, this is a lot of looping. 
Good thing our lists aren't so large that we'd notice the effects of 
cubic scaling (I count O(m*n*n), where m is renames length, and n is 
worst-case performance of qdict_haskey).  Definitely not worth the 
effort of more code to try and have a more efficient algorithm in 
large-term scaling but which hurts performance with increased overhead 
on small lists.


Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH 01/02] fix issue where a branch to pc+4 confuses GDB because pc and npc are set to the same value

[Peter Maydell]

On 15 February 2018 at 19:15, Steven Seeger
 wrote:
> From ef3183a1648f45c705b55704de3755f84e9bcf80 Mon Sep 17 00:00:00 2001
> From: Steven Seeger 
> Date: Thu, 15 Feb 2018 13:20:04 -0500
> Subject: [PATCH 1/2] fix issue where a branch to pc+4 confuses GDB because pc
>  and npc are set to the same value
>
> Signed-off-by: Steven Seeger 
> ---
>  target/sparc/translate.c | 6 ++
>  1 file changed, 6 insertions(+)
>
> diff --git a/target/sparc/translate.c b/target/sparc/translate.c
> index 71e0853e43..4c2272003d 100644
> --- a/target/sparc/translate.c
> +++ b/target/sparc/translate.c
> @@ -1464,6 +1464,9 @@ static void do_branch(DisasContext *dc, int32_t offset,
> uint32_t insn, int cc)
>  dc->npc = dc->pc + 4;
>  } else {
>  dc->pc = dc->npc;
> +if (target == dc->pc) {
> +target += 4;
> +}
>  dc->npc = target;
>  tcg_gen_mov_tl(cpu_pc, cpu_npc);
>  }
> @@ -1504,6 +1507,9 @@ static void do_fbranch(DisasContext *dc, int32_t offset,
> uint32_t insn, int cc)
>  dc->npc = dc->pc + 4;
>  } else {
>  dc->pc = dc->npc;
> +if (target == dc->pc) {
> +target += 4;
> +}
>  dc->npc = target;
>  tcg_gen_mov_tl(cpu_pc, cpu_npc);

These changes look rather odd -- are you sure they're right?
This is the code for unconditional taken branch, not annulled, and
my copy of the sparc architecture manual says that in that case
the new PC value should be the old nPC value, and the new
nPC value should be the effective address of the branch target.
There's nothing in there about branches into your own delay
slot being a special case. Adding 4 to target like this will
make the new nPC value be 4 further forward, which would mean
we'd only execute the branch target instruction once, rather
than twice (once for it being in the branch delay slot and
once as the instruction target).

It's a weird thing to do so I wouldn't be surprised if gdb
mishandled it. Have you tested against real sparc hardware?

thanks
-- PMM

[Qemu-devel] [PATCH 1/6] block: Support byte-based aio callbacks

[Eric Blake]

We are gradually moving away from sector-based interfaces, towards
byte-based.  Add new sector-based aio callbacks for read and write,
to match the fact that bdrv_aio_pdiscard is already byte-based.

Ideally, drivers should be converted to use coroutine callbacks
rather than aio; but that is not quite as trivial (if we do that
conversion, the null-aio driver will disappear), so for the
short term, converting the signature but keeping things with aio
is easier.  Once all drivers are converted, the sector-based aio
callbacks will be removed.

Signed-off-by: Eric Blake 
---
 include/block/block_int.h |  6 ++
 block/io.c| 37 +++--
 2 files changed, 33 insertions(+), 10 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index 5ae7738cf8d..c882dc4232d 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -137,9 +137,15 @@ struct BlockDriver {
 BlockAIOCB *(*bdrv_aio_readv)(BlockDriverState *bs,
 int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
 BlockCompletionFunc *cb, void *opaque);
+BlockAIOCB *(*bdrv_aio_preadv)(BlockDriverState *bs,
+uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags,
+BlockCompletionFunc *cb, void *opaque);
 BlockAIOCB *(*bdrv_aio_writev)(BlockDriverState *bs,
 int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
 BlockCompletionFunc *cb, void *opaque);
+BlockAIOCB *(*bdrv_aio_pwritev)(BlockDriverState *bs,
+uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags,
+BlockCompletionFunc *cb, void *opaque);
 BlockAIOCB *(*bdrv_aio_flush)(BlockDriverState *bs,
 BlockCompletionFunc *cb, void *opaque);
 BlockAIOCB *(*bdrv_aio_pdiscard)(BlockDriverState *bs,
diff --git a/block/io.c b/block/io.c
index 4d3d1f640a3..84a4caa72b7 100644
--- a/block/io.c
+++ b/block/io.c
@@ -934,9 +934,11 @@ static int coroutine_fn 
bdrv_driver_preadv(BlockDriverState *bs,
 sector_num = offset >> BDRV_SECTOR_BITS;
 nb_sectors = bytes >> BDRV_SECTOR_BITS;

-assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
-assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
-assert((bytes >> BDRV_SECTOR_BITS) <= BDRV_REQUEST_MAX_SECTORS);
+if (!drv->bdrv_aio_preadv) {
+assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
+assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
+assert((bytes >> BDRV_SECTOR_BITS) <= BDRV_REQUEST_MAX_SECTORS);
+}

 if (drv->bdrv_co_readv) {
 return drv->bdrv_co_readv(bs, sector_num, nb_sectors, qiov);
@@ -946,8 +948,13 @@ static int coroutine_fn 
bdrv_driver_preadv(BlockDriverState *bs,
 .coroutine = qemu_coroutine_self(),
 };

-acb = bs->drv->bdrv_aio_readv(bs, sector_num, qiov, nb_sectors,
-  bdrv_co_io_em_complete, );
+if (drv->bdrv_aio_preadv) {
+acb = bs->drv->bdrv_aio_preadv(bs, offset, bytes, qiov, flags,
+   bdrv_co_io_em_complete, );
+} else {
+acb = bs->drv->bdrv_aio_readv(bs, sector_num, qiov, nb_sectors,
+  bdrv_co_io_em_complete, );
+}
 if (acb == NULL) {
 return -EIO;
 } else {
@@ -982,9 +989,11 @@ static int coroutine_fn 
bdrv_driver_pwritev(BlockDriverState *bs,
 sector_num = offset >> BDRV_SECTOR_BITS;
 nb_sectors = bytes >> BDRV_SECTOR_BITS;

-assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
-assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
-assert((bytes >> BDRV_SECTOR_BITS) <= BDRV_REQUEST_MAX_SECTORS);
+if (!drv->bdrv_aio_pwritev) {
+assert((offset & (BDRV_SECTOR_SIZE - 1)) == 0);
+assert((bytes & (BDRV_SECTOR_SIZE - 1)) == 0);
+assert((bytes >> BDRV_SECTOR_BITS) <= BDRV_REQUEST_MAX_SECTORS);
+}

 if (drv->bdrv_co_writev_flags) {
 ret = drv->bdrv_co_writev_flags(bs, sector_num, nb_sectors, qiov,
@@ -999,8 +1008,16 @@ static int coroutine_fn 
bdrv_driver_pwritev(BlockDriverState *bs,
 .coroutine = qemu_coroutine_self(),
 };

-acb = bs->drv->bdrv_aio_writev(bs, sector_num, qiov, nb_sectors,
-   bdrv_co_io_em_complete, );
+if (drv->bdrv_aio_pwritev) {
+acb = bs->drv->bdrv_aio_pwritev(bs, offset, bytes, qiov,
+flags & bs->supported_write_flags,
+bdrv_co_io_em_complete, );
+flags &= ~bs->supported_write_flags;
+} else {
+assert(!bs->supported_write_flags);
+acb = bs->drv->bdrv_aio_writev(bs, sector_num, qiov, nb_sectors,
+   bdrv_co_io_em_complete, );
+}
 if (acb == NULL) {
 ret = -EIO;
 } else {
-- 
2.14.3

Re: [Qemu-devel] [PATCH 08/27] util: Add qemu_opts_to_qdict_filtered()

[Eric Blake]

On 02/08/2018 01:23 PM, Kevin Wolf wrote:

This allows, given a QemuOpts for a QemuOptsList that was merged from
multiple QemuOptsList, to only consider those options that exist in one
specific list. Block drivers need this to separate format-layer create
options from protocol-level options.

Signed-off-by: Kevin Wolf 
---
  include/qemu/option.h |  2 ++
  util/qemu-option.c| 37 -
  2 files changed, 34 insertions(+), 5 deletions(-)


Is there any unit test coverage we should be adding, beyond the default 
we get when the list parameter is NULL?


But what you have looks sane;
Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[Qemu-devel] [PATCH 3/6] null: Switch to byte-based read/write

[Eric Blake]

We are gradually moving away from sector-based interfaces, towards
byte-based.  Make the change for the last few sector-based callbacks
in the null-co and null-aio drivers.

Note that since the null driver does nothing on writes, it trivially
supports the BDRV_REQ_FUA flag (all writes have already landed to
the same bit-bucket without needing an extra flush call).  Furthermore,
bdrv_refresh_limits() defaults the block size to 512 for any driver
that does not support coroutines; while this is still correct for the
other aio-based drivers, the null driver does just as well with
byte-based requests, and being explicit means we can avoid cycles
wasted on read-modify-write.

Signed-off-by: Eric Blake 
---
 block/null.c | 66 ++--
 1 file changed, 37 insertions(+), 29 deletions(-)

diff --git a/block/null.c b/block/null.c
index 806a8631e4d..2381090dfcf 100644
--- a/block/null.c
+++ b/block/null.c
@@ -93,6 +93,7 @@ static int null_file_open(BlockDriverState *bs, QDict 
*options, int flags,
 }
 s->read_zeroes = qemu_opt_get_bool(opts, NULL_OPT_ZEROES, false);
 qemu_opts_del(opts);
+bs->supported_write_flags = BDRV_REQ_FUA;
 return ret;
 }

@@ -116,22 +117,22 @@ static coroutine_fn int null_co_common(BlockDriverState 
*bs)
 return 0;
 }

-static coroutine_fn int null_co_readv(BlockDriverState *bs,
-  int64_t sector_num, int nb_sectors,
-  QEMUIOVector *qiov)
+static coroutine_fn int null_co_preadv(BlockDriverState *bs,
+   uint64_t offset, uint64_t bytes,
+   QEMUIOVector *qiov, int flags)
 {
 BDRVNullState *s = bs->opaque;

 if (s->read_zeroes) {
-qemu_iovec_memset(qiov, 0, 0, nb_sectors * BDRV_SECTOR_SIZE);
+qemu_iovec_memset(qiov, 0, 0, bytes);
 }

 return null_co_common(bs);
 }

-static coroutine_fn int null_co_writev(BlockDriverState *bs,
-   int64_t sector_num, int nb_sectors,
-   QEMUIOVector *qiov)
+static coroutine_fn int null_co_pwritev(BlockDriverState *bs,
+uint64_t offset, uint64_t bytes,
+QEMUIOVector *qiov, int flags)
 {
 return null_co_common(bs);
 }
@@ -186,26 +187,26 @@ static inline BlockAIOCB 
*null_aio_common(BlockDriverState *bs,
 return >common;
 }

-static BlockAIOCB *null_aio_readv(BlockDriverState *bs,
-  int64_t sector_num, QEMUIOVector *qiov,
-  int nb_sectors,
-  BlockCompletionFunc *cb,
-  void *opaque)
-{
-BDRVNullState *s = bs->opaque;
-
-if (s->read_zeroes) {
-qemu_iovec_memset(qiov, 0, 0, nb_sectors * BDRV_SECTOR_SIZE);
-}
-
-return null_aio_common(bs, cb, opaque);
-}
-
-static BlockAIOCB *null_aio_writev(BlockDriverState *bs,
-   int64_t sector_num, QEMUIOVector *qiov,
-   int nb_sectors,
+static BlockAIOCB *null_aio_preadv(BlockDriverState *bs,
+   uint64_t offset, uint64_t bytes,
+   QEMUIOVector *qiov, int flags,
BlockCompletionFunc *cb,
void *opaque)
+{
+BDRVNullState *s = bs->opaque;
+
+if (s->read_zeroes) {
+qemu_iovec_memset(qiov, 0, 0, bytes);
+}
+
+return null_aio_common(bs, cb, opaque);
+}
+
+static BlockAIOCB *null_aio_pwritev(BlockDriverState *bs,
+uint64_t offset, uint64_t bytes,
+QEMUIOVector *qiov, int flags,
+BlockCompletionFunc *cb,
+void *opaque)
 {
 return null_aio_common(bs, cb, opaque);
 }
@@ -256,6 +257,11 @@ static void null_refresh_filename(BlockDriverState *bs, 
QDict *opts)
 bs->full_open_options = opts;
 }

+static void null_refresh_limits(BlockDriverState *bs, Error **errp)
+{
+bs->bl.request_alignment = 1;
+}
+
 static BlockDriver bdrv_null_co = {
 .format_name= "null-co",
 .protocol_name  = "null-co",
@@ -266,14 +272,15 @@ static BlockDriver bdrv_null_co = {
 .bdrv_close = null_close,
 .bdrv_getlength = null_getlength,

-.bdrv_co_readv  = null_co_readv,
-.bdrv_co_writev = null_co_writev,
+.bdrv_co_preadv = null_co_preadv,
+.bdrv_co_pwritev= null_co_pwritev,
 .bdrv_co_flush_to_disk  = null_co_flush,
 .bdrv_reopen_prepare= null_reopen_prepare,

 .bdrv_co_block_status   = null_co_block_status,

 .bdrv_refresh_filename  = null_refresh_filename,
+.bdrv_refresh_limits= 

[Qemu-devel] [PATCH 5/6] vxhs: Switch to byte-based callbacks

[Eric Blake]

We are gradually moving away from sector-based interfaces, towards
byte-based.  Make the change for the last few sector-based callbacks
in the vxhs driver.

Note that the driver was already using byte-based calls for
performing actual I/O, so this just gets rid of a round trip
of scaling; however, the block layer defaults this driver to
use request_alignment of 512, and I did not change that.

Signed-off-by: Eric Blake 

---
I was unable to get this to compile, as I do not have the vxhs
devel headers installed.  By inspection, it should work, but
a careful review is appreciated.
---
 block/vxhs.c | 36 +++-
 1 file changed, 15 insertions(+), 21 deletions(-)

diff --git a/block/vxhs.c b/block/vxhs.c
index 75cc6c8672c..c478ae26035 100644
--- a/block/vxhs.c
+++ b/block/vxhs.c
@@ -424,21 +424,17 @@ static const AIOCBInfo vxhs_aiocb_info = {
  * and is passed to QNIO. When QNIO completes the work,
  * it will be passed back through the callback.
  */
-static BlockAIOCB *vxhs_aio_rw(BlockDriverState *bs, int64_t sector_num,
-   QEMUIOVector *qiov, int nb_sectors,
+static BlockAIOCB *vxhs_aio_rw(BlockDriverState *bs, uint64_t offset,
+   QEMUIOVector *qiov, uint64_t size,
BlockCompletionFunc *cb, void *opaque,
VDISKAIOCmd iodir)
 {
 VXHSAIOCB *acb = NULL;
 BDRVVXHSState *s = bs->opaque;
-size_t size;
-uint64_t offset;
 int iio_flags = 0;
 int ret = 0;
 void *dev_handle = s->vdisk_hostinfo.dev_handle;

-offset = sector_num * BDRV_SECTOR_SIZE;
-size = nb_sectors * BDRV_SECTOR_SIZE;
 acb = qemu_aio_get(_aiocb_info, bs, cb, opaque);

 /*
@@ -451,11 +447,11 @@ static BlockAIOCB *vxhs_aio_rw(BlockDriverState *bs, 
int64_t sector_num,
 switch (iodir) {
 case VDISK_AIO_WRITE:
 ret = iio_writev(dev_handle, acb, qiov->iov, qiov->niov,
- offset, (uint64_t)size, iio_flags);
+ offset, size, iio_flags);
 break;
 case VDISK_AIO_READ:
 ret = iio_readv(dev_handle, acb, qiov->iov, qiov->niov,
-offset, (uint64_t)size, iio_flags);
+offset, size, iio_flags);
 break;
 default:
 trace_vxhs_aio_rw_invalid(iodir);
@@ -474,22 +470,20 @@ errout:
 return NULL;
 }

-static BlockAIOCB *vxhs_aio_readv(BlockDriverState *bs,
-   int64_t sector_num, QEMUIOVector *qiov,
-   int nb_sectors,
+static BlockAIOCB *vxhs_aio_preadv(BlockDriverState *bs,
+   uint64_t offset, uint64_t bytes,
+   QEMUIOVector *qiov, int flags,
BlockCompletionFunc *cb, void *opaque)
 {
-return vxhs_aio_rw(bs, sector_num, qiov, nb_sectors, cb,
-   opaque, VDISK_AIO_READ);
+return vxhs_aio_rw(bs, offset, qiov, bytes, cb, opaque, VDISK_AIO_READ);
 }

-static BlockAIOCB *vxhs_aio_writev(BlockDriverState *bs,
-   int64_t sector_num, QEMUIOVector *qiov,
-   int nb_sectors,
-   BlockCompletionFunc *cb, void *opaque)
+static BlockAIOCB *vxhs_aio_pwritev(BlockDriverState *bs,
+uint64_t offset, uint64_t bytes,
+QEMUIOVector *qiov, int flags,
+BlockCompletionFunc *cb, void *opaque)
 {
-return vxhs_aio_rw(bs, sector_num, qiov, nb_sectors,
-   cb, opaque, VDISK_AIO_WRITE);
+return vxhs_aio_rw(bs, offset, qiov, bytes, cb, opaque, VDISK_AIO_WRITE);
 }

 static void vxhs_close(BlockDriverState *bs)
@@ -563,8 +557,8 @@ static BlockDriver bdrv_vxhs = {
 .bdrv_parse_filename  = vxhs_parse_filename,
 .bdrv_close   = vxhs_close,
 .bdrv_getlength   = vxhs_getlength,
-.bdrv_aio_readv   = vxhs_aio_readv,
-.bdrv_aio_writev  = vxhs_aio_writev,
+.bdrv_aio_preadv  = vxhs_aio_preadv,
+.bdrv_aio_pwritev = vxhs_aio_pwritev,
 };

 static void bdrv_vxhs_init(void)
-- 
2.14.3

[Qemu-devel] [PATCH 6/6] block: Drop last of the sector-based aio callbacks

[Eric Blake]

We are gradually moving away from sector-based interfaces, towards
byte-based.  Now that all drivers with aio callbacks are using the
byte-based interfaces, we can remove the sector-based versions.

Signed-off-by: Eric Blake 
---
 include/block/block_int.h |  6 --
 block/io.c| 23 ++-
 2 files changed, 6 insertions(+), 23 deletions(-)

diff --git a/include/block/block_int.h b/include/block/block_int.h
index c882dc4232d..0f5b7accfa8 100644
--- a/include/block/block_int.h
+++ b/include/block/block_int.h
@@ -134,15 +134,9 @@ struct BlockDriver {
 void (*bdrv_refresh_filename)(BlockDriverState *bs, QDict *options);

 /* aio */
-BlockAIOCB *(*bdrv_aio_readv)(BlockDriverState *bs,
-int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
-BlockCompletionFunc *cb, void *opaque);
 BlockAIOCB *(*bdrv_aio_preadv)(BlockDriverState *bs,
 uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags,
 BlockCompletionFunc *cb, void *opaque);
-BlockAIOCB *(*bdrv_aio_writev)(BlockDriverState *bs,
-int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
-BlockCompletionFunc *cb, void *opaque);
 BlockAIOCB *(*bdrv_aio_pwritev)(BlockDriverState *bs,
 uint64_t offset, uint64_t bytes, QEMUIOVector *qiov, int flags,
 BlockCompletionFunc *cb, void *opaque);
diff --git a/block/io.c b/block/io.c
index 84a4caa72b7..04239f01eef 100644
--- a/block/io.c
+++ b/block/io.c
@@ -948,13 +948,8 @@ static int coroutine_fn 
bdrv_driver_preadv(BlockDriverState *bs,
 .coroutine = qemu_coroutine_self(),
 };

-if (drv->bdrv_aio_preadv) {
-acb = bs->drv->bdrv_aio_preadv(bs, offset, bytes, qiov, flags,
-   bdrv_co_io_em_complete, );
-} else {
-acb = bs->drv->bdrv_aio_readv(bs, sector_num, qiov, nb_sectors,
-  bdrv_co_io_em_complete, );
-}
+acb = bs->drv->bdrv_aio_preadv(bs, offset, bytes, qiov, flags,
+   bdrv_co_io_em_complete, );
 if (acb == NULL) {
 return -EIO;
 } else {
@@ -1008,16 +1003,10 @@ static int coroutine_fn 
bdrv_driver_pwritev(BlockDriverState *bs,
 .coroutine = qemu_coroutine_self(),
 };

-if (drv->bdrv_aio_pwritev) {
-acb = bs->drv->bdrv_aio_pwritev(bs, offset, bytes, qiov,
-flags & bs->supported_write_flags,
-bdrv_co_io_em_complete, );
-flags &= ~bs->supported_write_flags;
-} else {
-assert(!bs->supported_write_flags);
-acb = bs->drv->bdrv_aio_writev(bs, sector_num, qiov, nb_sectors,
-   bdrv_co_io_em_complete, );
-}
+acb = bs->drv->bdrv_aio_pwritev(bs, offset, bytes, qiov,
+flags & bs->supported_write_flags,
+bdrv_co_io_em_complete, );
+flags &= ~bs->supported_write_flags;
 if (acb == NULL) {
 ret = -EIO;
 } else {
-- 
2.14.3

[Qemu-devel] [PATCH 0/6] block: byte-based AIO read/write

[Eric Blake]

While we would prefer that block drivers use coroutines instead
of aio callbacks, it is a fairly easy exercise to prove that
all existing drivers with aio callbacks are merely scaling
from bytes into sectors and back to bytes.  So, even though I
am not set up to completely run (or even compile-test) this
full series, it seems pretty straightforward to change the
signature to quit playing games with pointless scaling.

Note that except for the null-aio driver, I intentionally did
NOT try and change the request_alignment from the block layer's
default of 512 (it defaults to 1 only for drivers that use
coroutines).

(And along the way, I got my docker-test-mingw@fedora working;
thanks to the help I got on IRC)

Eric Blake (6):
  block: Support byte-based aio callbacks
  file-win32: Switch to byte-based callbacks
  null: Switch to byte-based read/write
  rbd: Switch to byte-based callbacks
  vxhs: Switch to byte-based callbacks
  block: Drop last of the sector-based aio callbacks

 include/block/block_int.h |  8 +++---
 include/block/raw-aio.h   |  2 +-
 block/io.c| 26 ---
 block/file-win32.c| 36 +-
 block/null.c  | 66 ++-
 block/rbd.c   | 36 --
 block/vxhs.c  | 36 +++---
 block/win32-aio.c |  5 ++--
 8 files changed, 109 insertions(+), 106 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH 4/6] rbd: Switch to byte-based callbacks

[Eric Blake]

We are gradually moving away from sector-based interfaces, towards
byte-based.  Make the change for the last few sector-based callbacks
in the rbd driver.

Note that the driver was already using byte-based calls for
performing actual I/O, so this just gets rid of a round trip
of scaling; however, the block layer defaults this driver to
use request_alignment of 512, and I did not change that.

Signed-off-by: Eric Blake 
---
 block/rbd.c | 36 
 1 file changed, 16 insertions(+), 20 deletions(-)

diff --git a/block/rbd.c b/block/rbd.c
index 8474b0ba117..b3a613023c1 100644
--- a/block/rbd.c
+++ b/block/rbd.c
@@ -870,27 +870,23 @@ failed:
 return NULL;
 }

-static BlockAIOCB *qemu_rbd_aio_readv(BlockDriverState *bs,
-  int64_t sector_num,
-  QEMUIOVector *qiov,
-  int nb_sectors,
-  BlockCompletionFunc *cb,
-  void *opaque)
-{
-return rbd_start_aio(bs, sector_num << BDRV_SECTOR_BITS, qiov,
- (int64_t) nb_sectors << BDRV_SECTOR_BITS, cb, opaque,
- RBD_AIO_READ);
-}
-
-static BlockAIOCB *qemu_rbd_aio_writev(BlockDriverState *bs,
-   int64_t sector_num,
-   QEMUIOVector *qiov,
-   int nb_sectors,
+static BlockAIOCB *qemu_rbd_aio_preadv(BlockDriverState *bs,
+   uint64_t offset, uint64_t bytes,
+   QEMUIOVector *qiov, int flags,
BlockCompletionFunc *cb,
void *opaque)
 {
-return rbd_start_aio(bs, sector_num << BDRV_SECTOR_BITS, qiov,
- (int64_t) nb_sectors << BDRV_SECTOR_BITS, cb, opaque,
+return rbd_start_aio(bs, offset, qiov, bytes, cb, opaque,
+ RBD_AIO_READ);
+}
+
+static BlockAIOCB *qemu_rbd_aio_pwritev(BlockDriverState *bs,
+uint64_t offset, uint64_t bytes,
+QEMUIOVector *qiov, int flags,
+BlockCompletionFunc *cb,
+void *opaque)
+{
+return rbd_start_aio(bs, offset, qiov, bytes, cb, opaque,
  RBD_AIO_WRITE);
 }

@@ -1140,8 +1136,8 @@ static BlockDriver bdrv_rbd = {
 .bdrv_truncate  = qemu_rbd_truncate,
 .protocol_name  = "rbd",

-.bdrv_aio_readv = qemu_rbd_aio_readv,
-.bdrv_aio_writev= qemu_rbd_aio_writev,
+.bdrv_aio_preadv= qemu_rbd_aio_preadv,
+.bdrv_aio_pwritev   = qemu_rbd_aio_pwritev,

 #ifdef LIBRBD_SUPPORTS_AIO_FLUSH
 .bdrv_aio_flush = qemu_rbd_aio_flush,
-- 
2.14.3

[Qemu-devel] [PATCH 2/6] file-win32: Switch to byte-based callbacks

[Eric Blake]

We are gradually moving away from sector-based interfaces, towards
byte-based.  Make the change for the last few sector-based callbacks
in the file-win32 driver.

Note that the driver was already using byte-based calls for
performing actual I/O, so this just gets rid of a round trip
of scaling; however, the block layer defaults this driver to
use request_alignment of 512, and I did not change that,
because I don't know if Windows is tolerant of non-sector AIO
operations.

Signed-off-by: Eric Blake 

---
Compile-tested via 'make docker-test-mingw@fedora', but I don't
have a sane way to test whether it actually works.
---
 include/block/raw-aio.h |  2 +-
 block/file-win32.c  | 36 ++--
 block/win32-aio.c   |  5 ++---
 3 files changed, 21 insertions(+), 22 deletions(-)

diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h
index a4cdbbf1b7a..9e47b8a629d 100644
--- a/include/block/raw-aio.h
+++ b/include/block/raw-aio.h
@@ -57,7 +57,7 @@ void win32_aio_cleanup(QEMUWin32AIOState *aio);
 int win32_aio_attach(QEMUWin32AIOState *aio, HANDLE hfile);
 BlockAIOCB *win32_aio_submit(BlockDriverState *bs,
 QEMUWin32AIOState *aio, HANDLE hfile,
-int64_t sector_num, QEMUIOVector *qiov, int nb_sectors,
+uint64_t offset, uint64_t bytes, QEMUIOVector *qiov,
 BlockCompletionFunc *cb, void *opaque, int type);
 void win32_aio_detach_aio_context(QEMUWin32AIOState *aio,
   AioContext *old_context);
diff --git a/block/file-win32.c b/block/file-win32.c
index f24c7bb92c6..7a2e3367946 100644
--- a/block/file-win32.c
+++ b/block/file-win32.c
@@ -410,32 +410,32 @@ fail:
 return ret;
 }

-static BlockAIOCB *raw_aio_readv(BlockDriverState *bs,
- int64_t sector_num, QEMUIOVector *qiov, int 
nb_sectors,
- BlockCompletionFunc *cb, void *opaque)
+static BlockAIOCB *raw_aio_preadv(BlockDriverState *bs,
+  uint64_t offset, uint64_t bytes,
+  QEMUIOVector *qiov, int flags,
+  BlockCompletionFunc *cb, void *opaque)
 {
 BDRVRawState *s = bs->opaque;
 if (s->aio) {
-return win32_aio_submit(bs, s->aio, s->hfile, sector_num, qiov,
-nb_sectors, cb, opaque, QEMU_AIO_READ);
+return win32_aio_submit(bs, s->aio, s->hfile, offset, bytes, qiov,
+cb, opaque, QEMU_AIO_READ);
 } else {
-return paio_submit(bs, s->hfile, sector_num << BDRV_SECTOR_BITS, qiov,
-   nb_sectors << BDRV_SECTOR_BITS,
+return paio_submit(bs, s->hfile, offset, qiov, bytes,
cb, opaque, QEMU_AIO_READ);
 }
 }

-static BlockAIOCB *raw_aio_writev(BlockDriverState *bs,
-  int64_t sector_num, QEMUIOVector *qiov, int 
nb_sectors,
-  BlockCompletionFunc *cb, void *opaque)
+static BlockAIOCB *raw_aio_pwritev(BlockDriverState *bs,
+   uint64_t offset, uint64_t bytes,
+   QEMUIOVector *qiov, int flags,
+   BlockCompletionFunc *cb, void *opaque)
 {
 BDRVRawState *s = bs->opaque;
 if (s->aio) {
-return win32_aio_submit(bs, s->aio, s->hfile, sector_num, qiov,
-nb_sectors, cb, opaque, QEMU_AIO_WRITE);
+return win32_aio_submit(bs, s->aio, s->hfile, offset, bytes, qiov,
+cb, opaque, QEMU_AIO_WRITE);
 } else {
-return paio_submit(bs, s->hfile, sector_num << BDRV_SECTOR_BITS, qiov,
-   nb_sectors << BDRV_SECTOR_BITS,
+return paio_submit(bs, s->hfile, offset, qiov, bytes,
cb, opaque, QEMU_AIO_WRITE);
 }
 }
@@ -602,8 +602,8 @@ BlockDriver bdrv_file = {
 .bdrv_create= raw_create,
 .bdrv_has_zero_init = bdrv_has_zero_init_1,

-.bdrv_aio_readv = raw_aio_readv,
-.bdrv_aio_writev= raw_aio_writev,
+.bdrv_aio_preadv= raw_aio_preadv,
+.bdrv_aio_pwritev   = raw_aio_pwritev,
 .bdrv_aio_flush = raw_aio_flush,

 .bdrv_truncate = raw_truncate,
@@ -764,8 +764,8 @@ static BlockDriver bdrv_host_device = {
 .bdrv_file_open= hdev_open,
 .bdrv_close= raw_close,

-.bdrv_aio_readv = raw_aio_readv,
-.bdrv_aio_writev= raw_aio_writev,
+.bdrv_aio_preadv= raw_aio_preadv,
+.bdrv_aio_pwritev   = raw_aio_pwritev,
 .bdrv_aio_flush = raw_aio_flush,

 .bdrv_detach_aio_context = raw_detach_aio_context,
diff --git a/block/win32-aio.c b/block/win32-aio.c
index 3be8f458fab..9cd355d42f8 100644
--- a/block/win32-aio.c
+++ b/block/win32-aio.c
@@ -112,15 +112,14 @@ static const AIOCBInfo win32_aiocb_info = {

 BlockAIOCB *win32_aio_submit(BlockDriverState *bs,
 

[Qemu-devel] [RFC PATCH 02/02] handle case of delayed control transfer couple with leading branch being conditional. need to correctly take the new branch if conditional is untaken

[Steven Seeger]

This patch attempts to address a crash caused by a branch in a delay slot of 
antoher branch causing qemu to generate an unaligned exception due to loading 
the instruction at 2 (aka JUMP_PC).

This is RFC because I am not sure if I'm handling this correctly. The goal 
here is to have a delayed control transfer couple, which on sparc v8 is 
unpredictable, behave in a predictable way.

In my test case, I have a conditional branch followed by an unconditional 
branch. My desire was to have QEMU follow the first branch if the condition 
matched, otherwise follow the second branch if it did not.

If the instruction following a branch delay instruction causes npc to become 
DYNAMIC_PC I'm not really sure what will happen here, either.

Also translate.c has a function for SPARC64 called do_branch_reg() which I'm 
not sure if I need any special handling for in this case.

>From 113357ed067ec64348a51bf40f97c909f85f5d95 Mon Sep 17 00:00:00 2001
From: Steven Seeger 
Date: Thu, 15 Feb 2018 13:47:42 -0500
Subject: [PATCH 2/2] handle case of delayed control transfer couple with
 leading branch being conditional. need to correctly take the new branch if
 conditional is untaken

Signed-off-by: Steven Seeger 
---
 target/sparc/cpu.h   |  3 +-
 target/sparc/translate.c | 84 +
+--
 2 files changed, 76 insertions(+), 11 deletions(-)

diff --git a/target/sparc/cpu.h b/target/sparc/cpu.h
index 3eaffb354e..af56c49439 100644
--- a/target/sparc/cpu.h
+++ b/target/sparc/cpu.h
@@ -439,7 +439,8 @@ struct CPUSPARCState {
 
 target_ulong cond; /* conditional branch result (XXX: save it in a
   temporary register when possible) */
-
+target_ulong bcc_delayed; /* if 1, indicates the current instruction is
+  executing in a delay slot of a conditional branch 
*/
 uint32_t psr;  /* processor state register */
 target_ulong fsr;  /* FPU state register */
 CPU_DoubleU fpr[TARGET_DPREGS];  /* floating point registers */
diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index 4c2272003d..2223bff640 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -19,6 +19,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/error-report.h"
 
 #include "cpu.h"
 #include "disas/disas.h"
@@ -45,6 +46,7 @@ static TCGv_ptr cpu_regwptr;
 static TCGv cpu_cc_src, cpu_cc_src2, cpu_cc_dst;
 static TCGv_i32 cpu_cc_op;
 static TCGv_i32 cpu_psr;
+static TCGv_i32 bcc_delayed;
 static TCGv cpu_fsr, cpu_pc, cpu_npc;
 static TCGv cpu_regs[32];
 static TCGv cpu_y;
@@ -69,6 +71,7 @@ typedef struct DisasContext {
 target_ulong pc;/* current Program Counter: integer or DYNAMIC_PC */
 target_ulong npc;   /* next PC: integer or DYNAMIC_PC or JUMP_PC */
 target_ulong jump_pc[2]; /* used when JUMP_PC pc value is used */
+target_ulong bcc_delayed;
 int is_br;
 int mem_idx;
 bool fpu_enabled;
@@ -1007,6 +1010,8 @@ static void gen_branch_n(DisasContext *dc, target_ulong 
pc1)
 dc->jump_pc[0] = pc1;
 dc->jump_pc[1] = npc + 4;
 dc->npc = JUMP_PC;
+tcg_gen_movi_tl(bcc_delayed, 1);
+dc->bcc_delayed = 1;
 } else {
 TCGv t, z;
 
@@ -1438,16 +1443,37 @@ static inline void gen_cond_reg(TCGv r_dst, int cond, 
TCGv r_src)
 }
 #endif
 
-static void do_branch(DisasContext *dc, int32_t offset, uint32_t insn, int 
cc)
+/* returns 0 if new branch, or non-zero if branch is in delay slot
+ * of another branch */
+static int do_branch(DisasContext *dc, int32_t offset, uint32_t insn, int cc)
 {
 unsigned int cond = GET_FIELD(insn, 3, 6), a = (insn & (1 << 29));
 target_ulong target = dc->pc + offset;
 
+assert(!(dc->npc & JUMP_PC) || ((dc->npc & JUMP_PC) && dc->bcc_delayed));
+
 #ifdef TARGET_SPARC64
 if (unlikely(AM_CHECK(dc))) {
 target &= 0xULL;
 }
 #endif
+if (dc->bcc_delayed) {
+if (cond == 0x0) {
+/* unconditional not taken */
+/* already in delay slot, so do not take another delay */
+dc->jump_pc[1] += 4;
+} else if (cond == 0x8) {
+/* unconditional taken */
+dc->jump_pc[1] = target;
+} else {
+error_report("conditional branch in delay slot of conditional "
+"branch at pc=0x" TARGET_FMT_lx, dc->pc);
+goto handle_cc;
+}
+
+return -1;
+}
+
 if (cond == 0x0) {
 /* unconditional not taken */
 if (a) {
@@ -1471,6 +1497,7 @@ static void do_branch(DisasContext *dc, int32_t offset, 
uint32_t insn, int cc)
 tcg_gen_mov_tl(cpu_pc, cpu_npc);
 }
 } else {
+handle_cc:
 flush_cond(dc);
 gen_cond(cpu_cond, cc, cond, dc);
 if (a) {
@@ -1479,9 +1506,13 @@ static void do_branch(DisasContext *dc, int32_t offset, 
uint32_t insn, int cc)
 

[Qemu-devel] [PATCH 01/02] fix issue where a branch to pc+4 confuses GDB because pc and npc are set to the same value

[Steven Seeger]

>From ef3183a1648f45c705b55704de3755f84e9bcf80 Mon Sep 17 00:00:00 2001
From: Steven Seeger 
Date: Thu, 15 Feb 2018 13:20:04 -0500
Subject: [PATCH 1/2] fix issue where a branch to pc+4 confuses GDB because pc
 and npc are set to the same value

Signed-off-by: Steven Seeger 
---
 target/sparc/translate.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/target/sparc/translate.c b/target/sparc/translate.c
index 71e0853e43..4c2272003d 100644
--- a/target/sparc/translate.c
+++ b/target/sparc/translate.c
@@ -1464,6 +1464,9 @@ static void do_branch(DisasContext *dc, int32_t offset, 
uint32_t insn, int cc)
 dc->npc = dc->pc + 4;
 } else {
 dc->pc = dc->npc;
+if (target == dc->pc) {
+target += 4;
+}
 dc->npc = target;
 tcg_gen_mov_tl(cpu_pc, cpu_npc);
 }
@@ -1504,6 +1507,9 @@ static void do_fbranch(DisasContext *dc, int32_t offset, 
uint32_t insn, int cc)
 dc->npc = dc->pc + 4;
 } else {
 dc->pc = dc->npc;
+if (target == dc->pc) {
+target += 4;
+}
 dc->npc = target;
 tcg_gen_mov_tl(cpu_pc, cpu_npc);
 }
-- 
2.13.6

Re: [Qemu-devel] [PATCH v3 02/12] sdcard: replace DPRINTF() by trace events

[Philippe Mathieu-Daudé]

Hi Alistair,

On 01/31/2018 01:12 PM, Alistair Francis wrote:
> On Mon, Jan 22, 2018 at 7:21 PM, Philippe Mathieu-Daudé  
> wrote:
>> Signed-off-by: Philippe Mathieu-Daudé 
>> ---
>>  hw/sd/sd.c | 33 ++---
>>  hw/sd/trace-events |  6 ++
>>  2 files changed, 32 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>> index 55d2ba2dd7..f876973a2b 100644
>> --- a/hw/sd/sd.c
>> +++ b/hw/sd/sd.c
>> @@ -40,6 +40,7 @@
>>  #include "qemu/error-report.h"
>>  #include "qemu/timer.h"
>>  #include "qemu/log.h"
>> +#include "trace.h"
>>
>>  //#define DEBUG_SD 1
>>
>> @@ -132,6 +133,25 @@ struct SDState {
>>  bool cmd_line;
>>  };
>>
>> +static const char *sd_state_name(enum SDCardStates state)
>> +{
>> +static const char *state_name[] = {
>> +[sd_idle_state] = "idle",
>> +[sd_ready_state]= "ready",
>> +[sd_identification_state]   = "identification",
>> +[sd_standby_state]  = "standby",
>> +[sd_transfer_state] = "transfer",
>> +[sd_sendingdata_state]  = "sendingdata",
>> +[sd_receivingdata_state]= "receivingdata",
>> +[sd_programming_state]  = "programming",
>> +[sd_disconnect_state]   = "disconnect",
>> +};
>> +if (state == sd_inactive_state) {
>> +return "inactive";
>> +}
> 
> There should be an assert here to make sure we never go off the end of
> this array.

This shouldn't happen since this function is local (static), but I'll
add a check.

> 
> Is this used in future? It seems like a lot of work for one caller.

This resulted useful to me when tracing (in particular when using the
MMC mode). Also note this code is only called when tracing is enabled.

http://lists.nongnu.org/archive/html/qemu-devel/2017-12/msg02387.html

> 
>> +return state_name[state];
>> +}
>> +
>>  static uint8_t sd_get_dat_lines(SDState *sd)
>>  {
>>  return sd->enable ? sd->dat_lines : 0;
>> @@ -776,6 +796,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
>>  uint32_t rca = 0x;
>>  uint64_t addr = (sd->ocr & (1 << 30)) ? (uint64_t) req.arg << 9 : 
>> req.arg;
>>
>> +trace_sdcard_normal_command(req.cmd, req.arg, sd_state_name(sd->state));
>> +
>>  /* Not interpreting this as an app command */
>>  sd->card_status &= ~APP_CMD;
>>
>> @@ -790,7 +812,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
>>  sd->multi_blk_cnt = 0;
>>  }
>>
>> -DPRINTF("CMD%d 0x%08x state %d\n", req.cmd, req.arg, sd->state);
>>  switch (req.cmd) {
>>  /* Basic commands (Class 0 and Class 1) */
>>  case 0:/* CMD0:   GO_IDLE_STATE */
>> @@ -1310,8 +1331,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
>>  return sd_r1;
>>
>>  case 56:   /* CMD56:  GEN_CMD */
>> -fprintf(stderr, "SD: GEN_CMD 0x%08x\n", req.arg);
>> -
>>  switch (sd->state) {
>>  case sd_transfer_state:
>>  sd->data_offset = 0;
>> @@ -1345,7 +1364,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
>>  static sd_rsp_type_t sd_app_command(SDState *sd,
>>  SDRequest req)
>>  {
>> -DPRINTF("ACMD%d 0x%08x\n", req.cmd, req.arg);
>> +trace_sdcard_app_command(req.cmd, req.arg);
>>  sd->card_status |= APP_CMD;
>>  switch (req.cmd) {
>>  case 6:/* ACMD6:  SET_BUS_WIDTH */
>> @@ -1594,7 +1613,7 @@ send_response:
>>  DPRINTF("Response:");
>>  for (i = 0; i < rsplen; i++)
>>  fprintf(stderr, " %02x", response[i]);
>> -fprintf(stderr, " state %d\n", sd->state);
>> +fputc('\n', stderr);
> 
> Why change this?

Probably leftover when rebasing, since trace_sdcard_normal_command() now
displays the state, I was not sure the best way to trace hexdump before
removing this debug code.

> 
> Alistair
> 
>>  } else {
>>  DPRINTF("No response %d\n", sd->state);
>>  }
>> @@ -1605,8 +1624,7 @@ send_response:
>>
>>  static void sd_blk_read(SDState *sd, uint64_t addr, uint32_t len)
>>  {
>> -DPRINTF("sd_blk_read: addr = 0x%08llx, len = %d\n",
>> -(unsigned long long) addr, len);
>> +trace_sdcard_read_block(addr, len);
>>  if (!sd->blk || blk_pread(sd->blk, addr, sd->data, len) < 0) {
>>  fprintf(stderr, "sd_blk_read: read error on host side\n");
>>  }
>> @@ -1614,6 +1632,7 @@ static void sd_blk_read(SDState *sd, uint64_t addr, 
>> uint32_t len)
>>
>>  static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len)
>>  {
>> +trace_sdcard_write_block(addr, len);
>>  if (!sd->blk || blk_pwrite(sd->blk, addr, sd->data, len, 0) < 0) {
>>  fprintf(stderr, "sd_blk_write: write error on host side\n");
>>  }
>> diff --git a/hw/sd/trace-events b/hw/sd/trace-events
>> index 0f8536db32..75dac5a2cd 100644
>> --- a/hw/sd/trace-events
>> +++ b/hw/sd/trace-events
>> @@ -23,6 +23,12 @@ 

Re: [Qemu-devel] [PATCH 2/3] qapi: Rename QMP and QGA schema files

[Michael Roth]

Quoting Eric Blake (2018-02-12 10:12:11)
> On 02/11/2018 03:49 AM, Markus Armbruster wrote:
> > Eric Blake  writes:
> > 
> >> Having two files in the tree both named qapi-schema.json just adds
> >> confusion.  Rename these files, and relocate them into the common
> >> qapi/ subdirectory.  Update all build rules that refer to the file
> >> names, and adjust other documentation and comment references to
> >> either track the new name or be rewritten so as to not mention
> >> the file name.
> >>
> >> Maintainer-wise, this means that qapi/qga-schema.json continues
> >> to belong to Michael as QGA maintainer, but now also notifies
> >> Markus and Eric as QAPI maintainers, alongside all the other
> >> QMP QAPI files, matching how other .json QAPI modules belong
> >> to multiple maintainer blurbs.
> >>
> >> Signed-off-by: Eric Blake 
> >> ---
> >>   docs/devel/writing-qmp-commands.txt  | 13 ++-
> >>   docs/interop/qmp-intro.txt   |  3 ++-
> >>   Makefile | 10 
> >>   qga/qapi-schema.json => qapi/qga-schema.json |  0
> > 
> > This move is up to the QGA maintainer.  My usual argument for keeping
> > the schema in one place is weak for the QGA schema: it's maintained
> > separately, and ususuall grepped separately, too.
> 
> Michael, your thoughts?

No strong feelings either way, but I can see some utility to giving it
proximity to the other schema files. Some values in qapi/common.json might
be applicable to the qga schema in the future for instance, so it might be
a good idea to manage them in a central location as opposed to having
qga-schema hanging out by itself, so I'd be okay with the patch as is.

> 
> > 
> >>   qapi-schema.json => qapi/qmp-schema.json | 34 
> >> ++--
> > 
> > While it was certainly created for QMP, it's now used for non-QMP stuff,
> > too.  Do we mind?
> > 
> 
> Well, patch 3 renames it further; having qapi/qapi-schema.qapi sounds 
> redundant, and the fact that the file DOES drive our QMP decisions (even 
> if it has some other types for internal use only) seems reasonable 
> enough.  If anyone has a better name, I'm all ears.
> 
> >>   tpm.c|  2 +-
> >>   MAINTAINERS  |  2 +-
> >>   7 files changed, 33 insertions(+), 31 deletions(-)
> >>   rename qga/qapi-schema.json => qapi/qga-schema.json (100%)
> >>   rename qapi-schema.json => qapi/qmp-schema.json (99%)
> > 
> 
> -- 
> Eric Blake, Principal Software Engineer
> Red Hat, Inc.   +1-919-301-3266
> Virtualization:  qemu.org | libvirt.org
> 

Re: [Qemu-devel] [Resend][PATCH] qga: unset frozen state if no mount points are frozen

[Michael Roth]

Quoting Chen Hanxiao (2018-02-08 19:35:42)
> From: Chen Hanxiao 
> 
> If we set mountpoints to qmp_guest_fsfreeze_freeze_list,
> we may got nothing to freeze as all mountpoints are
> not valid.
> Call ga_unset_frozen in this senario.
> 
> Cc: Michael Roth 
> Signed-off-by: Chen Hanxiao 
> ---
> Rebase on master
> 
>  qga/commands-posix.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
> index e809e382eb..9fd51f1d7a 100644
> --- a/qga/commands-posix.c
> +++ b/qga/commands-posix.c
> @@ -1273,6 +1273,12 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool 
> has_mountpoints,
>  }
> 
>  free_fs_mount_list();
> +/* We may not issue any FIFREEZE here when had mountpoints.
> + * Just unset ga_state here and ready for the next call.
> + */
> +if (has_mountpoints && i == 0) {
> +ga_unset_frozen(ga_state);
> +}

It seems odd to special-case has_mountpoints. Wouldn't:

  if (i == 0) {
...
  }

be more consistent? Then management could infer i==0 leaves qga in
unfrozen state. Otherwise I'd rather just stick with expecting a
gratuitous unfreeze() since it requires less special-casing on the
management/ side.

And if we do change this, we'd probably want to update
qga/qapi-schema.json to reflect the behavior. I.e.:

##
# @guest-fsfreeze-freeze:
#
# Sync and freeze all freezable, local guest filesystems. If this
# command succeeded, you may call @guest-fsfreeze-thaw later to
# unfreeze.
...
# Returns: Number of file systems currently frozen. On error, all filesystems
-# will be thawed.
+# will be thawed. If no filesystems are frozen as a result of this call,
+# then @guest-fsfreeze-status will remain "thawed" and calling
+# @guest-fsfreeze-thaw is not necessary.


>  return i;
> 
>  error:
> -- 
> 2.14.3
> 

[Qemu-devel] [PULL 19/20] bcm2836: Make CPU type configurable

[Peter Maydell]

From: Pekka Enberg 

This patch adds a "cpu-type" property to BCM2836 SoC in preparation for
reusing the code for the Raspberry Pi 3, which has a different processor
model.

Signed-off-by: Pekka Enberg 
Reviewed-by: Peter Maydell 
Signed-off-by: Peter Maydell 
---
 include/hw/arm/bcm2836.h |  1 +
 hw/arm/bcm2836.c | 17 +
 hw/arm/raspi.c   |  3 +++
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/include/hw/arm/bcm2836.h b/include/hw/arm/bcm2836.h
index 76de1996af..4758b4ae54 100644
--- a/include/hw/arm/bcm2836.h
+++ b/include/hw/arm/bcm2836.h
@@ -25,6 +25,7 @@ typedef struct BCM2836State {
 DeviceState parent_obj;
 /*< public >*/
 
+char *cpu_type;
 uint32_t enabled_cpus;
 
 ARMCPU cpus[BCM2836_NCPUS];
diff --git a/hw/arm/bcm2836.c b/hw/arm/bcm2836.c
index 8c43291112..40e8b25a46 100644
--- a/hw/arm/bcm2836.c
+++ b/hw/arm/bcm2836.c
@@ -26,14 +26,6 @@
 static void bcm2836_init(Object *obj)
 {
 BCM2836State *s = BCM2836(obj);
-int n;
-
-for (n = 0; n < BCM2836_NCPUS; n++) {
-object_initialize(>cpus[n], sizeof(s->cpus[n]),
-  "cortex-a15-" TYPE_ARM_CPU);
-object_property_add_child(obj, "cpu[*]", OBJECT(>cpus[n]),
-  _abort);
-}
 
 object_initialize(>control, sizeof(s->control), TYPE_BCM2836_CONTROL);
 object_property_add_child(obj, "control", OBJECT(>control), NULL);
@@ -59,6 +51,14 @@ static void bcm2836_realize(DeviceState *dev, Error **errp)
 
 /* common peripherals from bcm2835 */
 
+obj = OBJECT(dev);
+for (n = 0; n < BCM2836_NCPUS; n++) {
+object_initialize(>cpus[n], sizeof(s->cpus[n]),
+  s->cpu_type);
+object_property_add_child(obj, "cpu[*]", OBJECT(>cpus[n]),
+  _abort);
+}
+
 obj = object_property_get_link(OBJECT(dev), "ram", );
 if (obj == NULL) {
 error_setg(errp, "%s: required ram link not found: %s",
@@ -150,6 +150,7 @@ static void bcm2836_realize(DeviceState *dev, Error **errp)
 }
 
 static Property bcm2836_props[] = {
+DEFINE_PROP_STRING("cpu-type", BCM2836State, cpu_type),
 DEFINE_PROP_UINT32("enabled-cpus", BCM2836State, enabled_cpus, 
BCM2836_NCPUS),
 DEFINE_PROP_END_OF_LIST()
 };
diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index cd5fa8c3dc..c24a4a1b14 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -135,6 +135,8 @@ static void raspi2_init(MachineState *machine)
 /* Setup the SOC */
 object_property_add_const_link(OBJECT(>soc), "ram", OBJECT(>ram),
_abort);
+object_property_set_str(OBJECT(>soc), machine->cpu_type, "cpu-type",
+_abort);
 object_property_set_int(OBJECT(>soc), smp_cpus, "enabled-cpus",
 _abort);
 object_property_set_int(OBJECT(>soc), 0xa21041, "board-rev",
@@ -166,6 +168,7 @@ static void raspi2_machine_init(MachineClass *mc)
 mc->no_parallel = 1;
 mc->no_floppy = 1;
 mc->no_cdrom = 1;
+mc->default_cpu_type = ARM_CPU_TYPE_NAME("cortex-a15");
 mc->max_cpus = BCM2836_NCPUS;
 mc->min_cpus = BCM2836_NCPUS;
 mc->default_cpus = BCM2836_NCPUS;
-- 
2.16.1

[Qemu-devel] [PULL 12/20] hw/intc/armv7m_nvic: Implement cache ID registers

[Peter Maydell]

M profile cores have a similar setup for cache ID registers
to A profile:
 * Cache Level ID Register (CLIDR) is a fixed value
 * Cache Type Register (CTR) is a fixed value
 * Cache Size ID Registers (CCSIDR) are a bank of registers;
   which one you see is selected by the Cache Size Selection
   Register (CSSELR)

The only difference is that they're in the NVIC memory mapped
register space rather than being coprocessor registers.
Implement the M profile view of them.

Since neither Cortex-M3 nor Cortex-M4 implement caches,
we don't need to update their init functions and can leave
the ctr/clidr/ccsidr[] fields in their ARMCPU structs at zero.
Newer cores (like the Cortex-M33) will want to be able to
set these ID registers to non-zero values, though.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Message-id: 20180209165810.6668-6-peter.mayd...@linaro.org
---
 target/arm/cpu.h  | 26 ++
 hw/intc/armv7m_nvic.c | 16 
 target/arm/machine.c  | 36 
 3 files changed, 78 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 51a3e16275..8938a7c953 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -496,6 +496,7 @@ typedef struct CPUARMState {
 uint32_t faultmask[M_REG_NUM_BANKS];
 uint32_t aircr; /* only holds r/w state if security extn implemented */
 uint32_t secure; /* Is CPU in Secure state? (not guest visible) */
+uint32_t csselr[M_REG_NUM_BANKS];
 } v7m;
 
 /* Information associated with an exception about to be taken:
@@ -1325,6 +1326,23 @@ FIELD(V7M_MPU_CTRL, ENABLE, 0, 1)
 FIELD(V7M_MPU_CTRL, HFNMIENA, 1, 1)
 FIELD(V7M_MPU_CTRL, PRIVDEFENA, 2, 1)
 
+/* v7M CLIDR bits */
+FIELD(V7M_CLIDR, CTYPE_ALL, 0, 21)
+FIELD(V7M_CLIDR, LOUIS, 21, 3)
+FIELD(V7M_CLIDR, LOC, 24, 3)
+FIELD(V7M_CLIDR, LOUU, 27, 3)
+FIELD(V7M_CLIDR, ICB, 30, 2)
+
+FIELD(V7M_CSSELR, IND, 0, 1)
+FIELD(V7M_CSSELR, LEVEL, 1, 3)
+/* We use the combination of InD and Level to index into cpu->ccsidr[];
+ * define a mask for this and check that it doesn't permit running off
+ * the end of the array.
+ */
+FIELD(V7M_CSSELR, INDEX, 0, 4)
+
+QEMU_BUILD_BUG_ON(ARRAY_SIZE(((ARMCPU *)0)->ccsidr) <= 
R_V7M_CSSELR_INDEX_MASK);
+
 /* If adding a feature bit which corresponds to a Linux ELF
  * HWCAP bit, remember to update the feature-bit-to-hwcap
  * mapping in linux-user/elfload.c:get_elf_hwcap().
@@ -2487,6 +2505,14 @@ static inline int arm_debug_target_el(CPUARMState *env)
 }
 }
 
+static inline bool arm_v7m_csselr_razwi(ARMCPU *cpu)
+{
+/* If all the CLIDR.Ctypem bits are 0 there are no caches, and
+ * CSSELR is RAZ/WI.
+ */
+return (cpu->clidr & R_V7M_CLIDR_CTYPE_ALL_MASK) != 0;
+}
+
 static inline bool aa64_generate_debug_exceptions(CPUARMState *env)
 {
 if (arm_is_secure(env)) {
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index eb49fd77c7..040f3380ec 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -1025,6 +1025,17 @@ static uint32_t nvic_readl(NVICState *s, uint32_t 
offset, MemTxAttrs attrs)
 return cpu->id_isar4;
 case 0xd74: /* ISAR5.  */
 return cpu->id_isar5;
+case 0xd78: /* CLIDR */
+return cpu->clidr;
+case 0xd7c: /* CTR */
+return cpu->ctr;
+case 0xd80: /* CSSIDR */
+{
+int idx = cpu->env.v7m.csselr[attrs.secure] & R_V7M_CSSELR_INDEX_MASK;
+return cpu->ccsidr[idx];
+}
+case 0xd84: /* CSSELR */
+return cpu->env.v7m.csselr[attrs.secure];
 /* TODO: Implement debug registers.  */
 case 0xd90: /* MPU_TYPE */
 /* Unified MPU; if the MPU is not present this value is zero */
@@ -1385,6 +1396,11 @@ static void nvic_writel(NVICState *s, uint32_t offset, 
uint32_t value,
 qemu_log_mask(LOG_UNIMP,
   "NVIC: Aux fault status registers unimplemented\n");
 break;
+case 0xd84: /* CSSELR */
+if (!arm_v7m_csselr_razwi(cpu)) {
+cpu->env.v7m.csselr[attrs.secure] = value & 
R_V7M_CSSELR_INDEX_MASK;
+}
+break;
 case 0xd90: /* MPU_TYPE */
 return; /* RO */
 case 0xd94: /* MPU_CTRL */
diff --git a/target/arm/machine.c b/target/arm/machine.c
index 2c8b43062f..cae63c2f98 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -191,6 +191,41 @@ static const VMStateDescription 
vmstate_m_faultmask_primask = {
 }
 };
 
+/* CSSELR is in a subsection because we didn't implement it previously.
+ * Migration from an old implementation will leave it at zero, which
+ * is OK since the only CPUs in the old implementation make the
+ * register RAZ/WI.
+ * Since there was no version of QEMU which implemented the CSSELR for
+ * just non-secure, we transfer both banks here rather than putting
+ * the secure banked version in the m-security subsection.
+ */
+static bool csselr_vmstate_validate(void *opaque, int 

[Qemu-devel] [PULL 11/20] hw/intc/armv7m_nvic: Implement v8M CPPWR register

[Peter Maydell]

The Coprocessor Power Control Register (CPPWR) is new in v8M.
It allows software to control whether coprocessors are allowed
to power down and lose their state. QEMU doesn't have any
notion of power control, so we choose the IMPDEF option of
making the whole register RAZ/WI (indicating that no coprocessors
can ever power down and lose state).

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Message-id: 20180209165810.6668-5-peter.mayd...@linaro.org
---
 hw/intc/armv7m_nvic.c | 14 ++
 1 file changed, 14 insertions(+)

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 74b25ce92c..eb49fd77c7 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -776,6 +776,14 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, 
MemTxAttrs attrs)
 switch (offset) {
 case 4: /* Interrupt Control Type.  */
 return ((s->num_irq - NVIC_FIRST_IRQ) / 32) - 1;
+case 0xc: /* CPPWR */
+if (!arm_feature(>env, ARM_FEATURE_V8)) {
+goto bad_offset;
+}
+/* We make the IMPDEF choice that nothing can ever go into a
+ * non-retentive power state, which allows us to RAZ/WI this.
+ */
+return 0;
 case 0x380 ... 0x3bf: /* NVIC_ITNS */
 {
 int startvec = 8 * (offset - 0x380) + NVIC_FIRST_IRQ;
@@ -1175,6 +1183,12 @@ static void nvic_writel(NVICState *s, uint32_t offset, 
uint32_t value,
 ARMCPU *cpu = s->cpu;
 
 switch (offset) {
+case 0xc: /* CPPWR */
+if (!arm_feature(>env, ARM_FEATURE_V8)) {
+goto bad_offset;
+}
+/* Make the IMPDEF choice to RAZ/WI this. */
+break;
 case 0x380 ... 0x3bf: /* NVIC_ITNS */
 {
 int startvec = 8 * (offset - 0x380) + NVIC_FIRST_IRQ;
-- 
2.16.1

[Qemu-devel] [PULL 07/20] target/arm: Handle SVE registers when using clear_vec_high

[Peter Maydell]

From: Richard Henderson 

When storing to an AdvSIMD FP register, all of the high
bits of the SVE register are zeroed.  Therefore, call it
more often with is_q as a parameter.

Signed-off-by: Richard Henderson 
Message-id: 20180211205848.4568-6-richard.hender...@linaro.org
Reviewed-by: Peter Maydell 
Signed-off-by: Peter Maydell 
---
 target/arm/translate-a64.c | 162 +
 1 file changed, 62 insertions(+), 100 deletions(-)

diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index e3881d4999..1c88539d62 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -602,13 +602,30 @@ static TCGv_i32 read_fp_sreg(DisasContext *s, int reg)
 return v;
 }
 
+/* Clear the bits above an N-bit vector, for N = (is_q ? 128 : 64).
+ * If SVE is not enabled, then there are only 128 bits in the vector.
+ */
+static void clear_vec_high(DisasContext *s, bool is_q, int rd)
+{
+unsigned ofs = fp_reg_offset(s, rd, MO_64);
+unsigned vsz = vec_full_reg_size(s);
+
+if (!is_q) {
+TCGv_i64 tcg_zero = tcg_const_i64(0);
+tcg_gen_st_i64(tcg_zero, cpu_env, ofs + 8);
+tcg_temp_free_i64(tcg_zero);
+}
+if (vsz > 16) {
+tcg_gen_gvec_dup8i(ofs + 16, vsz - 16, vsz - 16, 0);
+}
+}
+
 static void write_fp_dreg(DisasContext *s, int reg, TCGv_i64 v)
 {
-TCGv_i64 tcg_zero = tcg_const_i64(0);
+unsigned ofs = fp_reg_offset(s, reg, MO_64);
 
-tcg_gen_st_i64(v, cpu_env, fp_reg_offset(s, reg, MO_64));
-tcg_gen_st_i64(tcg_zero, cpu_env, fp_reg_hi_offset(s, reg));
-tcg_temp_free_i64(tcg_zero);
+tcg_gen_st_i64(v, cpu_env, ofs);
+clear_vec_high(s, false, reg);
 }
 
 static void write_fp_sreg(DisasContext *s, int reg, TCGv_i32 v)
@@ -1009,6 +1026,8 @@ static void do_fp_ld(DisasContext *s, int destidx, 
TCGv_i64 tcg_addr, int size)
 
 tcg_temp_free_i64(tmplo);
 tcg_temp_free_i64(tmphi);
+
+clear_vec_high(s, true, destidx);
 }
 
 /*
@@ -1124,17 +1143,6 @@ static void write_vec_element_i32(DisasContext *s, 
TCGv_i32 tcg_src,
 }
 }
 
-/* Clear the high 64 bits of a 128 bit vector (in general non-quad
- * vector ops all need to do this).
- */
-static void clear_vec_high(DisasContext *s, int rd)
-{
-TCGv_i64 tcg_zero = tcg_const_i64(0);
-
-write_vec_element(s, tcg_zero, rd, 1, MO_64);
-tcg_temp_free_i64(tcg_zero);
-}
-
 /* Store from vector register to memory */
 static void do_vec_st(DisasContext *s, int srcidx, int element,
   TCGv_i64 tcg_addr, int size)
@@ -2794,12 +2802,13 @@ static void disas_ldst_multiple_struct(DisasContext *s, 
uint32_t insn)
 /* For non-quad operations, setting a slice of the low
  * 64 bits of the register clears the high 64 bits (in
  * the ARM ARM pseudocode this is implicit in the fact
- * that 'rval' is a 64 bit wide variable). We optimize
- * by noticing that we only need to do this the first
- * time we touch a register.
+ * that 'rval' is a 64 bit wide variable).
+ * For quad operations, we might still need to zero the
+ * high bits of SVE.  We optimize by noticing that we only
+ * need to do this the first time we touch a register.
  */
-if (!is_q && e == 0 && (r == 0 || xs == selem - 1)) {
-clear_vec_high(s, tt);
+if (e == 0 && (r == 0 || xs == selem - 1)) {
+clear_vec_high(s, is_q, tt);
 }
 }
 tcg_gen_addi_i64(tcg_addr, tcg_addr, ebytes);
@@ -2942,10 +2951,9 @@ static void disas_ldst_single_struct(DisasContext *s, 
uint32_t insn)
 write_vec_element(s, tcg_tmp, rt, 0, MO_64);
 if (is_q) {
 write_vec_element(s, tcg_tmp, rt, 1, MO_64);
-} else {
-clear_vec_high(s, rt);
 }
 tcg_temp_free_i64(tcg_tmp);
+clear_vec_high(s, is_q, rt);
 } else {
 /* Load/store one element per register */
 if (is_load) {
@@ -6718,7 +6726,6 @@ static void handle_vec_simd_sqshrn(DisasContext *s, bool 
is_scalar, bool is_q,
 }
 
 if (!is_q) {
-clear_vec_high(s, rd);
 write_vec_element(s, tcg_final, rd, 0, MO_64);
 } else {
 write_vec_element(s, tcg_final, rd, 1, MO_64);
@@ -6731,7 +6738,8 @@ static void handle_vec_simd_sqshrn(DisasContext *s, bool 
is_scalar, bool is_q,
 tcg_temp_free_i64(tcg_rd);
 tcg_temp_free_i32(tcg_rd_narrowed);
 tcg_temp_free_i64(tcg_final);
-return;
+
+clear_vec_high(s, is_q, rd);
 }
 
 /* SQSHLU, UQSHL, SQSHL: saturating left shifts */
@@ -6795,10 +6803,7 @@ static void 

Re: [Qemu-devel] [PATCH v3] hw/char: remove legacy interface escc_init()

[Mark Cave-Ayland]

On 14/02/18 04:58, David Gibson wrote:


David, would you be willing to take this via ppc-for-2.12 since it covers
the ESCC used in the PPC Mac machines?


Short term: Yes, I can stage this.  I've lost track of this thread
though.  Laurent, can you please resend with the various R-bs collated
together.

Long term: I've been thinking for a while that I'm not greatly
qualified to review or test patches for the Macintosh platforms.  And
I've also been thinking for a while that you (Mark) seem the obvious
choice to be sub-maintainer for the ppc based Macintosh machine types
and related device models.  Would you be willing to take that on?


Sure, I'm happy to do that. Not that I know a great deal about PPC Macs, 
but I've got a good set of test images that get booted regularly when 
testing OpenBIOS, so it's fairly easy for me to pick up regressions.



ATB,

Mark.

[Qemu-devel] [PULL 10/20] hw/intc/armv7m_nvic: Implement M profile cache maintenance ops

[Peter Maydell]

For M profile cores, cache maintenance operations are done by
writing to special registers in the system register space.
For QEMU, cache operations are always NOPs, since we don't
implement the cache. Implementing these explicitly avoids
a spurious LOG_GUEST_ERROR when the guest uses them.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Message-id: 20180209165810.6668-4-peter.mayd...@linaro.org
---
 hw/intc/armv7m_nvic.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 06b9598fbe..74b25ce92c 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -1594,6 +1594,18 @@ static void nvic_writel(NVICState *s, uint32_t offset, 
uint32_t value,
 }
 break;
 }
+case 0xf50: /* ICIALLU */
+case 0xf58: /* ICIMVAU */
+case 0xf5c: /* DCIMVAC */
+case 0xf60: /* DCISW */
+case 0xf64: /* DCCMVAU */
+case 0xf68: /* DCCMVAC */
+case 0xf6c: /* DCCSW */
+case 0xf70: /* DCCIMVAC */
+case 0xf74: /* DCCISW */
+case 0xf78: /* BPIALL */
+/* Cache and branch predictor maintenance: for QEMU these always NOP */
+break;
 default:
 bad_offset:
 qemu_log_mask(LOG_GUEST_ERROR,
-- 
2.16.1

Re: [Qemu-devel] [RFC PATCH] capstone: fix building using system package

[Philippe Mathieu-Daudé]

Hi Sergei,

On 02/15/2018 03:21 PM, Sergei Trofimovich wrote:
> On Thu, 15 Feb 2018 14:35:39 -0300
> Philippe Mathieu-Daudé  wrote:
> 
>>  #else
>> +#include 
> 
> I think it's incorrect. 'pkg-config' already reports 'capstone/' path:
> $ pkg-config --cflags capstone
> -I/usr/include/capstone

Glad to hear feedback from a Gentoo developer!

Ok so the problem Haiku only, which we don't support anymore.

> 
> $ ls /usr/include/capstone/capstone.h
> /usr/include/capstone/capstone.h
> 
> Thus I would guess
> #include 
> is still correct for system include path as well (contradicts the example).

My guess is the example is probabilisticly safer for people compiling
without using 'pkg-config'.

> qemu just needs to use 'pkg-config' to discover the include path and
> libs. Maybe new capstone release has different pkgconfig setup?

I think it is safer to drop this patch.

Thanks for your review!

Phil.

[Qemu-devel] [PULL 09/20] hw/intc/armv7m_nvic: Fix ICSR PENDNMISET/CLR handling

[Peter Maydell]

The PENDNMISET/CLR bits in the ICSR should be RAZ/WI from
NonSecure state if the AIRCR.BFHFNMINS bit is zero. We had
misimplemented this as making the bits RAZ/WI from both
Secure and NonSecure states. Fix this bug by checking
attrs.secure so that Secure code can pend and unpend NMIs.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Message-id: 20180209165810.6668-3-peter.mayd...@linaro.org
---
 hw/intc/armv7m_nvic.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 63da0fee34..06b9598fbe 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -830,8 +830,8 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, 
MemTxAttrs attrs)
 }
 }
 /* NMIPENDSET */
-if ((cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK) &&
-s->vectors[ARMV7M_EXCP_NMI].pending) {
+if ((attrs.secure || (cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK))
+&& s->vectors[ARMV7M_EXCP_NMI].pending) {
 val |= (1 << 31);
 }
 /* ISRPREEMPT: RES0 when halting debug not implemented */
@@ -1193,7 +1193,7 @@ static void nvic_writel(NVICState *s, uint32_t offset, 
uint32_t value,
 break;
 }
 case 0xd04: /* Interrupt Control State (ICSR) */
-if (cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK) {
+if (attrs.secure || cpu->env.v7m.aircr & R_V7M_AIRCR_BFHFNMINS_MASK) {
 if (value & (1 << 31)) {
 armv7m_nvic_set_pending(s, ARMV7M_EXCP_NMI, false);
 } else if (value & (1 << 30) &&
-- 
2.16.1

[Qemu-devel] [PULL 16/20] target/arm: Add AIRCR to vmstate struct

[Peter Maydell]

In commit commit 3b2e934463121 we added support for the AIRCR
register holding state, but forgot to add it to the vmstate
structs. Since it only holds r/w state if the security extension
is implemented, we can just add it to vmstate_m_security.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Message-id: 20180209165810.6668-10-peter.mayd...@linaro.org
---
 target/arm/machine.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/target/arm/machine.c b/target/arm/machine.c
index 30fb1454a6..25cdf4d581 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -423,6 +423,10 @@ static const VMStateDescription vmstate_m_security = {
 VMSTATE_VALIDATE("SAU_RNR is valid", sau_rnr_vmstate_validate),
 VMSTATE_UINT32(env.sau.ctrl, ARMCPU),
 VMSTATE_UINT32(env.v7m.scr[M_REG_S], ARMCPU),
+/* AIRCR is not secure-only, but our implementation is R/O if the
+ * security extension is unimplemented, so we migrate it here.
+ */
+VMSTATE_UINT32(env.v7m.aircr, ARMCPU),
 VMSTATE_END_OF_LIST()
 }
 };
-- 
2.16.1

[Qemu-devel] [PULL 08/20] hw/intc/armv7m_nvic: Don't hardcode M profile ID registers in NVIC

[Peter Maydell]

Instead of hardcoding the values of M profile ID registers in the
NVIC, use the fields in the CPU struct. This will allow us to
give different M profile CPU types different ID register values.

This commit includes the addition of the missing ID_ISAR5,
which exists as RES0 in both v7M and v8M.

(The values of the ID registers might be wrong for the M4 --
this commit leaves the behaviour there unchanged.)

Signed-off-by: Peter Maydell 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Richard Henderson 
Message-id: 20180209165810.6668-2-peter.mayd...@linaro.org
---
 hw/intc/armv7m_nvic.c | 30 --
 target/arm/cpu.c  | 28 
 2 files changed, 44 insertions(+), 14 deletions(-)

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 360889d30b..63da0fee34 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -990,31 +990,33 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, 
MemTxAttrs attrs)
   "Aux Fault status registers unimplemented\n");
 return 0;
 case 0xd40: /* PFR0.  */
-return 0x0030;
-case 0xd44: /* PRF1.  */
-return 0x0200;
+return cpu->id_pfr0;
+case 0xd44: /* PFR1.  */
+return cpu->id_pfr1;
 case 0xd48: /* DFR0.  */
-return 0x0010;
+return cpu->id_dfr0;
 case 0xd4c: /* AFR0.  */
-return 0x;
+return cpu->id_afr0;
 case 0xd50: /* MMFR0.  */
-return 0x0030;
+return cpu->id_mmfr0;
 case 0xd54: /* MMFR1.  */
-return 0x;
+return cpu->id_mmfr1;
 case 0xd58: /* MMFR2.  */
-return 0x;
+return cpu->id_mmfr2;
 case 0xd5c: /* MMFR3.  */
-return 0x;
+return cpu->id_mmfr3;
 case 0xd60: /* ISAR0.  */
-return 0x01141110;
+return cpu->id_isar0;
 case 0xd64: /* ISAR1.  */
-return 0x02111000;
+return cpu->id_isar1;
 case 0xd68: /* ISAR2.  */
-return 0x21112231;
+return cpu->id_isar2;
 case 0xd6c: /* ISAR3.  */
-return 0x0110;
+return cpu->id_isar3;
 case 0xd70: /* ISAR4.  */
-return 0x01310102;
+return cpu->id_isar4;
+case 0xd74: /* ISAR5.  */
+return cpu->id_isar5;
 /* TODO: Implement debug registers.  */
 case 0xd90: /* MPU_TYPE */
 /* Unified MPU; if the MPU is not present this value is zero */
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 89ccdeae12..d796085be9 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1146,6 +1146,20 @@ static void cortex_m3_initfn(Object *obj)
 set_feature(>env, ARM_FEATURE_M);
 cpu->midr = 0x410fc231;
 cpu->pmsav7_dregion = 8;
+cpu->id_pfr0 = 0x0030;
+cpu->id_pfr1 = 0x0200;
+cpu->id_dfr0 = 0x0010;
+cpu->id_afr0 = 0x;
+cpu->id_mmfr0 = 0x0030;
+cpu->id_mmfr1 = 0x;
+cpu->id_mmfr2 = 0x;
+cpu->id_mmfr3 = 0x;
+cpu->id_isar0 = 0x01141110;
+cpu->id_isar1 = 0x02111000;
+cpu->id_isar2 = 0x21112231;
+cpu->id_isar3 = 0x0110;
+cpu->id_isar4 = 0x01310102;
+cpu->id_isar5 = 0x;
 }
 
 static void cortex_m4_initfn(Object *obj)
@@ -1157,6 +1171,20 @@ static void cortex_m4_initfn(Object *obj)
 set_feature(>env, ARM_FEATURE_THUMB_DSP);
 cpu->midr = 0x410fc240; /* r0p0 */
 cpu->pmsav7_dregion = 8;
+cpu->id_pfr0 = 0x0030;
+cpu->id_pfr1 = 0x0200;
+cpu->id_dfr0 = 0x0010;
+cpu->id_afr0 = 0x;
+cpu->id_mmfr0 = 0x0030;
+cpu->id_mmfr1 = 0x;
+cpu->id_mmfr2 = 0x;
+cpu->id_mmfr3 = 0x;
+cpu->id_isar0 = 0x01141110;
+cpu->id_isar1 = 0x02111000;
+cpu->id_isar2 = 0x21112231;
+cpu->id_isar3 = 0x0110;
+cpu->id_isar4 = 0x01310102;
+cpu->id_isar5 = 0x;
 }
 
 static void arm_v7m_class_init(ObjectClass *oc, void *data)
-- 
2.16.1

[Qemu-devel] [PULL 13/20] hw/intc/armv7m_nvic: Implement SCR

[Peter Maydell]

We were previously making the system control register (SCR)
just RAZ/WI. Although we don't implement the functionality
this register controls, we should at least provide the state,
including the banked state for v8M.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Message-id: 20180209165810.6668-7-peter.mayd...@linaro.org
---
 target/arm/cpu.h  |  7 +++
 hw/intc/armv7m_nvic.c | 12 
 target/arm/machine.c  | 12 
 3 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 8938a7c953..bc0638d3fa 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -497,6 +497,7 @@ typedef struct CPUARMState {
 uint32_t aircr; /* only holds r/w state if security extn implemented */
 uint32_t secure; /* Is CPU in Secure state? (not guest visible) */
 uint32_t csselr[M_REG_NUM_BANKS];
+uint32_t scr[M_REG_NUM_BANKS];
 } v7m;
 
 /* Information associated with an exception about to be taken:
@@ -1258,6 +1259,12 @@ FIELD(V7M_CCR, STKALIGN, 9, 1)
 FIELD(V7M_CCR, DC, 16, 1)
 FIELD(V7M_CCR, IC, 17, 1)
 
+/* V7M SCR bits */
+FIELD(V7M_SCR, SLEEPONEXIT, 1, 1)
+FIELD(V7M_SCR, SLEEPDEEP, 2, 1)
+FIELD(V7M_SCR, SLEEPDEEPS, 3, 1)
+FIELD(V7M_SCR, SEVONPEND, 4, 1)
+
 /* V7M AIRCR bits */
 FIELD(V7M_AIRCR, VECTRESET, 0, 1)
 FIELD(V7M_AIRCR, VECTCLRACTIVE, 1, 1)
diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 040f3380ec..ea3b7cce14 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -863,8 +863,7 @@ static uint32_t nvic_readl(NVICState *s, uint32_t offset, 
MemTxAttrs attrs)
 }
 return val;
 case 0xd10: /* System Control.  */
-/* TODO: Implement SLEEPONEXIT.  */
-return 0;
+return cpu->env.v7m.scr[attrs.secure];
 case 0xd14: /* Configuration Control.  */
 /* The BFHFNMIGN bit is the only non-banked bit; we
  * keep it in the non-secure copy of the register.
@@ -1285,8 +1284,13 @@ static void nvic_writel(NVICState *s, uint32_t offset, 
uint32_t value,
 }
 break;
 case 0xd10: /* System Control.  */
-/* TODO: Implement control registers.  */
-qemu_log_mask(LOG_UNIMP, "NVIC: SCR unimplemented\n");
+/* We don't implement deep-sleep so these bits are RAZ/WI.
+ * The other bits in the register are banked.
+ * QEMU's implementation ignores SEVONPEND and SLEEPONEXIT, which
+ * is architecturally permitted.
+ */
+value &= ~(R_V7M_SCR_SLEEPDEEP_MASK | R_V7M_SCR_SLEEPDEEPS_MASK);
+cpu->env.v7m.scr[attrs.secure] = value;
 break;
 case 0xd14: /* Configuration Control.  */
 /* Enforce RAZ/WI on reserved and must-RAZ/WI bits */
diff --git a/target/arm/machine.c b/target/arm/machine.c
index cae63c2f98..30fb1454a6 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -226,6 +226,16 @@ static const VMStateDescription vmstate_m_csselr = {
 }
 };
 
+static const VMStateDescription vmstate_m_scr = {
+.name = "cpu/m/scr",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_UINT32(env.v7m.scr[M_REG_NS], ARMCPU),
+VMSTATE_END_OF_LIST()
+}
+};
+
 static const VMStateDescription vmstate_m = {
 .name = "cpu/m",
 .version_id = 4,
@@ -248,6 +258,7 @@ static const VMStateDescription vmstate_m = {
 .subsections = (const VMStateDescription*[]) {
 _m_faultmask_primask,
 _m_csselr,
+_m_scr,
 NULL
 }
 };
@@ -411,6 +422,7 @@ static const VMStateDescription vmstate_m_security = {
 VMSTATE_UINT32(env.sau.rnr, ARMCPU),
 VMSTATE_VALIDATE("SAU_RNR is valid", sau_rnr_vmstate_validate),
 VMSTATE_UINT32(env.sau.ctrl, ARMCPU),
+VMSTATE_UINT32(env.v7m.scr[M_REG_S], ARMCPU),
 VMSTATE_END_OF_LIST()
 }
 };
-- 
2.16.1

[Qemu-devel] [PULL 01/20] hw/arm/aspeed: directly map the serial device to the system address space

[Peter Maydell]

From: Philippe Mathieu-Daudé 

(qemu) info mtree
 address-space: cpu-memory-0
   - (prio 0, i/o): system
 -07ff (prio 0, rom): aspeed.boot_rom
 1e60-1e7f (prio -1, i/o): aspeed_soc.io
-  1e784000-1e78401f (prio 0, i/o): serial
 1e62-1e6200ff (prio 0, i/o): aspeed.smc.ast2500-fmc
 1e63-1e6300ff (prio 0, i/o): aspeed.smc.ast2500-spi1
 [...]
 1e72-1e728fff (prio 0, ram): aspeed.sram
 1e782000-1e782fff (prio 0, i/o): aspeed.timer
+1e784000-1e78401f (prio 0, i/o): serial
 1e785000-1e78501f (prio 0, i/o): aspeed.wdt
 1e785020-1e78503f (prio 0, i/o): aspeed.wdt

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Cédric Le Goater 
Reviewed-by: Andrew Jeffery 
Message-id: 20180209085755.30414-2-f4...@amsat.org
Signed-off-by: Peter Maydell 
---
 hw/arm/aspeed_soc.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index c83b7e207b..2a5d041b3b 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -257,7 +257,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error 
**errp)
 /* UART - attach an 8250 to the IO space as our UART5 */
 if (serial_hds[0]) {
 qemu_irq uart5 = qdev_get_gpio_in(DEVICE(>vic), uart_irqs[4]);
-serial_mm_init(>iomem, ASPEED_SOC_UART_5_BASE, 2,
+serial_mm_init(get_system_memory(),
+   ASPEED_SOC_IOMEM_BASE + ASPEED_SOC_UART_5_BASE, 2,
uart5, 38400, serial_hds[0], DEVICE_LITTLE_ENDIAN);
 }
 
-- 
2.16.1

[Qemu-devel] [PULL 18/20] target/arm: Implement v8M MSPLIM and PSPLIM registers

[Peter Maydell]

The v8M architecture includes hardware support for enforcing
stack pointer limits. We don't implement this behaviour yet,
but provide the MSPLIM and PSPLIM stack pointer limit registers
as reads-as-written, so that when we do implement the checks
in future this won't break guest migration.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Message-id: 20180209165810.6668-12-peter.mayd...@linaro.org
---
 target/arm/cpu.h |  2 ++
 target/arm/helper.c  | 46 ++
 target/arm/machine.c | 21 +
 3 files changed, 69 insertions(+)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index bc0638d3fa..de62df091c 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -498,6 +498,8 @@ typedef struct CPUARMState {
 uint32_t secure; /* Is CPU in Secure state? (not guest visible) */
 uint32_t csselr[M_REG_NUM_BANKS];
 uint32_t scr[M_REG_NUM_BANKS];
+uint32_t msplim[M_REG_NUM_BANKS];
+uint32_t psplim[M_REG_NUM_BANKS];
 } v7m;
 
 /* Information associated with an exception about to be taken:
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 1ae11997fb..e7586fcf6c 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -10403,6 +10403,16 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t 
reg)
 return 0;
 }
 return env->v7m.other_ss_psp;
+case 0x8a: /* MSPLIM_NS */
+if (!env->v7m.secure) {
+return 0;
+}
+return env->v7m.msplim[M_REG_NS];
+case 0x8b: /* PSPLIM_NS */
+if (!env->v7m.secure) {
+return 0;
+}
+return env->v7m.psplim[M_REG_NS];
 case 0x90: /* PRIMASK_NS */
 if (!env->v7m.secure) {
 return 0;
@@ -10444,6 +10454,16 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t 
reg)
 return v7m_using_psp(env) ? env->v7m.other_sp : env->regs[13];
 case 9: /* PSP */
 return v7m_using_psp(env) ? env->regs[13] : env->v7m.other_sp;
+case 10: /* MSPLIM */
+if (!arm_feature(env, ARM_FEATURE_V8)) {
+goto bad_reg;
+}
+return env->v7m.msplim[env->v7m.secure];
+case 11: /* PSPLIM */
+if (!arm_feature(env, ARM_FEATURE_V8)) {
+goto bad_reg;
+}
+return env->v7m.psplim[env->v7m.secure];
 case 16: /* PRIMASK */
 return env->v7m.primask[env->v7m.secure];
 case 17: /* BASEPRI */
@@ -10452,6 +10472,7 @@ uint32_t HELPER(v7m_mrs)(CPUARMState *env, uint32_t reg)
 case 19: /* FAULTMASK */
 return env->v7m.faultmask[env->v7m.secure];
 default:
+bad_reg:
 qemu_log_mask(LOG_GUEST_ERROR, "Attempt to read unknown special"
" register %d\n", reg);
 return 0;
@@ -10489,6 +10510,18 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t 
maskreg, uint32_t val)
 }
 env->v7m.other_ss_psp = val;
 return;
+case 0x8a: /* MSPLIM_NS */
+if (!env->v7m.secure) {
+return;
+}
+env->v7m.msplim[M_REG_NS] = val & ~7;
+return;
+case 0x8b: /* PSPLIM_NS */
+if (!env->v7m.secure) {
+return;
+}
+env->v7m.psplim[M_REG_NS] = val & ~7;
+return;
 case 0x90: /* PRIMASK_NS */
 if (!env->v7m.secure) {
 return;
@@ -10568,6 +10601,18 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t 
maskreg, uint32_t val)
 env->v7m.other_sp = val;
 }
 break;
+case 10: /* MSPLIM */
+if (!arm_feature(env, ARM_FEATURE_V8)) {
+goto bad_reg;
+}
+env->v7m.msplim[env->v7m.secure] = val & ~7;
+break;
+case 11: /* PSPLIM */
+if (!arm_feature(env, ARM_FEATURE_V8)) {
+goto bad_reg;
+}
+env->v7m.psplim[env->v7m.secure] = val & ~7;
+break;
 case 16: /* PRIMASK */
 env->v7m.primask[env->v7m.secure] = val & 1;
 break;
@@ -10600,6 +10645,7 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t 
maskreg, uint32_t val)
 env->v7m.control[env->v7m.secure] |= val & R_V7M_CONTROL_NPRIV_MASK;
 break;
 default:
+bad_reg:
 qemu_log_mask(LOG_GUEST_ERROR, "Attempt to write unknown special"
" register %d\n", reg);
 return;
diff --git a/target/arm/machine.c b/target/arm/machine.c
index 1a20d6c36c..2e28d086bd 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -246,6 +246,26 @@ static const VMStateDescription vmstate_m_other_sp = {
 }
 };
 
+static bool m_v8m_needed(void *opaque)
+{
+ARMCPU *cpu = opaque;
+CPUARMState *env = >env;
+
+return arm_feature(env, ARM_FEATURE_M) && arm_feature(env, 

Re: [Qemu-devel] [PATCH] docker: dump 'config.log' if ./configure fails

[Philippe Mathieu-Daudé]

On 02/15/2018 03:26 PM, Eric Blake wrote:
> On 02/15/2018 11:41 AM, Philippe Mathieu-Daudé wrote:
>> On 02/15/2018 02:34 PM, Daniel P. Berrangé wrote:
>>> On Thu, Feb 15, 2018 at 02:23:06PM -0300, Philippe Mathieu-Daudé wrote:
 Suggested-by: Eric Blake 
 Signed-off-by: Philippe Mathieu-Daudé 
 ---
   tests/docker/common.rc | 4 +++-
   1 file changed, 3 insertions(+), 1 deletion(-)

 diff --git a/tests/docker/common.rc b/tests/docker/common.rc
 index 7951555e3f..dd011cdf1e 100755
 --- a/tests/docker/common.rc
 +++ b/tests/docker/common.rc
 @@ -30,7 +30,9 @@ build_qemu()
    $@"
   echo "Configure options:"
   echo $config_opts
 -    $QEMU_SRC/configure $config_opts && make $MAKEFLAGS
 +    $QEMU_SRC/configure $config_opts || \
 +    (cat config.log && prep_fail "Failed to run 'configure'")
> 
> Is a subshell necessary; or can you get by with the simpler
>  { cat && prep_fail "..."; }

You are right, since the subshell is not necessary, better to use { ;}

TIL :)

> 
 +    make $MAKEFLAGS
   }
>>>
>>> Reviewed-by: Daniel P. Berrangé 
>>>
>>> Slightly nicer than my patch thanks to the prep_fail addition.
>>
>> Haha this was a funny experience of parallel programming :)
>>
>> However your patch has a slightly nicer description!
> 
> 

[Qemu-devel] [PULL 04/20] target/arm: Enforce FP access to FPCR/FPSR

[Peter Maydell]

From: Richard Henderson 

Signed-off-by: Richard Henderson 
Message-id: 20180211205848.4568-3-richard.hender...@linaro.org
Reviewed-by: Peter Maydell 
Signed-off-by: Peter Maydell 
---
 target/arm/cpu.h   | 35 ++-
 target/arm/helper.c|  6 --
 target/arm/translate-a64.c |  3 +++
 3 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 521444a5a1..e966a57f8a 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1714,7 +1714,7 @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid)
 }
 
 /* ARMCPRegInfo type field bits. If the SPECIAL bit is set this is a
- * special-behaviour cp reg and bits [15..8] indicate what behaviour
+ * special-behaviour cp reg and bits [11..8] indicate what behaviour
  * it has. Otherwise it is a simple cp reg, where CONST indicates that
  * TCG can assume the value to be constant (ie load at translate time)
  * and 64BIT indicates a 64 bit wide coprocessor register. SUPPRESS_TB_END
@@ -1735,24 +1735,25 @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid)
  * need to be surrounded by gen_io_start()/gen_io_end(). In particular,
  * registers which implement clocks or timers require this.
  */
-#define ARM_CP_SPECIAL 1
-#define ARM_CP_CONST 2
-#define ARM_CP_64BIT 4
-#define ARM_CP_SUPPRESS_TB_END 8
-#define ARM_CP_OVERRIDE 16
-#define ARM_CP_ALIAS 32
-#define ARM_CP_IO 64
-#define ARM_CP_NO_RAW 128
-#define ARM_CP_NOP (ARM_CP_SPECIAL | (1 << 8))
-#define ARM_CP_WFI (ARM_CP_SPECIAL | (2 << 8))
-#define ARM_CP_NZCV (ARM_CP_SPECIAL | (3 << 8))
-#define ARM_CP_CURRENTEL (ARM_CP_SPECIAL | (4 << 8))
-#define ARM_CP_DC_ZVA (ARM_CP_SPECIAL | (5 << 8))
-#define ARM_LAST_SPECIAL ARM_CP_DC_ZVA
+#define ARM_CP_SPECIAL   0x0001
+#define ARM_CP_CONST 0x0002
+#define ARM_CP_64BIT 0x0004
+#define ARM_CP_SUPPRESS_TB_END   0x0008
+#define ARM_CP_OVERRIDE  0x0010
+#define ARM_CP_ALIAS 0x0020
+#define ARM_CP_IO0x0040
+#define ARM_CP_NO_RAW0x0080
+#define ARM_CP_NOP   (ARM_CP_SPECIAL | 0x0100)
+#define ARM_CP_WFI   (ARM_CP_SPECIAL | 0x0200)
+#define ARM_CP_NZCV  (ARM_CP_SPECIAL | 0x0300)
+#define ARM_CP_CURRENTEL (ARM_CP_SPECIAL | 0x0400)
+#define ARM_CP_DC_ZVA(ARM_CP_SPECIAL | 0x0500)
+#define ARM_LAST_SPECIAL ARM_CP_DC_ZVA
+#define ARM_CP_FPU   0x1000
 /* Used only as a terminator for ARMCPRegInfo lists */
-#define ARM_CP_SENTINEL 0x
+#define ARM_CP_SENTINEL  0x
 /* Mask of only the flag bits in a type field */
-#define ARM_CP_FLAG_MASK 0xff
+#define ARM_CP_FLAG_MASK 0x10ff
 
 /* 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/helper.c b/target/arm/helper.c
index 4b102ec356..d41fb8371f 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -3356,10 +3356,12 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
   .writefn = aa64_daif_write, .resetfn = arm_cp_reset_ignore },
 { .name = "FPCR", .state = ARM_CP_STATE_AA64,
   .opc0 = 3, .opc1 = 3, .opc2 = 0, .crn = 4, .crm = 4,
-  .access = PL0_RW, .readfn = aa64_fpcr_read, .writefn = aa64_fpcr_write },
+  .access = PL0_RW, .type = ARM_CP_FPU,
+  .readfn = aa64_fpcr_read, .writefn = aa64_fpcr_write },
 { .name = "FPSR", .state = ARM_CP_STATE_AA64,
   .opc0 = 3, .opc1 = 3, .opc2 = 1, .crn = 4, .crm = 4,
-  .access = PL0_RW, .readfn = aa64_fpsr_read, .writefn = aa64_fpsr_write },
+  .access = PL0_RW, .type = ARM_CP_FPU,
+  .readfn = aa64_fpsr_read, .writefn = aa64_fpsr_write },
 { .name = "DCZID_EL0", .state = ARM_CP_STATE_AA64,
   .opc0 = 3, .opc1 = 3, .opc2 = 7, .crn = 0, .crm = 0,
   .access = PL0_R, .type = ARM_CP_NO_RAW,
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index fb1a4cb532..89f50558a7 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -1631,6 +1631,9 @@ static void handle_sys(DisasContext *s, uint32_t insn, 
bool isread,
 default:
 break;
 }
+if ((ri->type & ARM_CP_FPU) && !fp_access_check(s)) {
+return;
+}
 
 if ((tb_cflags(s->base.tb) & CF_USE_ICOUNT) && (ri->type & ARM_CP_IO)) {
 gen_io_start();
-- 
2.16.1

[Qemu-devel] [PULL 06/20] target/arm: Enforce access to ZCR_EL at translation

[Peter Maydell]

From: Richard Henderson 

This also makes sure that we get the correct ordering of
SVE vs FP exceptions.

Signed-off-by: Richard Henderson 
Message-id: 20180211205848.4568-5-richard.hender...@linaro.org
Reviewed-by: Peter Maydell 
Signed-off-by: Peter Maydell 
---
 target/arm/cpu.h   |  3 ++-
 target/arm/internals.h |  6 ++
 target/arm/helper.c| 22 --
 target/arm/translate-a64.c | 16 
 4 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index e966a57f8a..51a3e16275 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -1750,10 +1750,11 @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid)
 #define ARM_CP_DC_ZVA(ARM_CP_SPECIAL | 0x0500)
 #define ARM_LAST_SPECIAL ARM_CP_DC_ZVA
 #define ARM_CP_FPU   0x1000
+#define ARM_CP_SVE   0x2000
 /* Used only as a terminator for ARMCPRegInfo lists */
 #define ARM_CP_SENTINEL  0x
 /* Mask of only the flag bits in a type field */
-#define ARM_CP_FLAG_MASK 0x10ff
+#define ARM_CP_FLAG_MASK 0x30ff
 
 /* 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/internals.h b/target/arm/internals.h
index 89f5d2fe12..47cc224a46 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -243,6 +243,7 @@ enum arm_exception_class {
 EC_AA64_HVC   = 0x16,
 EC_AA64_SMC   = 0x17,
 EC_SYSTEMREGISTERTRAP = 0x18,
+EC_SVEACCESSTRAP  = 0x19,
 EC_INSNABORT  = 0x20,
 EC_INSNABORT_SAME_EL  = 0x21,
 EC_PCALIGNMENT= 0x22,
@@ -381,6 +382,11 @@ static inline uint32_t syn_fp_access_trap(int cv, int 
cond, bool is_16bit)
 | (cv << 24) | (cond << 20);
 }
 
+static inline uint32_t syn_sve_access_trap(void)
+{
+return EC_SVEACCESSTRAP << ARM_EL_EC_SHIFT;
+}
+
 static inline uint32_t syn_insn_abort(int same_el, int ea, int s1ptw, int fsc)
 {
 return (EC_INSNABORT << ARM_EL_EC_SHIFT) | (same_el << ARM_EL_EC_SHIFT)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index e0184c7162..550dc3d290 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -4335,20 +4335,6 @@ static int sve_exception_el(CPUARMState *env)
 return 0;
 }
 
-static CPAccessResult zcr_access(CPUARMState *env, const ARMCPRegInfo *ri,
- bool isread)
-{
-switch (sve_exception_el(env)) {
-case 3:
-return CP_ACCESS_TRAP_EL3;
-case 2:
-return CP_ACCESS_TRAP_EL2;
-case 1:
-return CP_ACCESS_TRAP;
-}
-return CP_ACCESS_OK;
-}
-
 static void zcr_write(CPUARMState *env, const ARMCPRegInfo *ri,
   uint64_t value)
 {
@@ -4359,7 +4345,7 @@ static void zcr_write(CPUARMState *env, const 
ARMCPRegInfo *ri,
 static const ARMCPRegInfo zcr_el1_reginfo = {
 .name = "ZCR_EL1", .state = ARM_CP_STATE_AA64,
 .opc0 = 3, .opc1 = 0, .crn = 1, .crm = 2, .opc2 = 0,
-.access = PL1_RW, .accessfn = zcr_access,
+.access = PL1_RW, .type = ARM_CP_SVE | ARM_CP_FPU,
 .fieldoffset = offsetof(CPUARMState, vfp.zcr_el[1]),
 .writefn = zcr_write, .raw_writefn = raw_write
 };
@@ -4367,7 +4353,7 @@ static const ARMCPRegInfo zcr_el1_reginfo = {
 static const ARMCPRegInfo zcr_el2_reginfo = {
 .name = "ZCR_EL2", .state = ARM_CP_STATE_AA64,
 .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 2, .opc2 = 0,
-.access = PL2_RW, .accessfn = zcr_access,
+.access = PL2_RW, .type = ARM_CP_SVE | ARM_CP_FPU,
 .fieldoffset = offsetof(CPUARMState, vfp.zcr_el[2]),
 .writefn = zcr_write, .raw_writefn = raw_write
 };
@@ -4375,14 +4361,14 @@ static const ARMCPRegInfo zcr_el2_reginfo = {
 static const ARMCPRegInfo zcr_no_el2_reginfo = {
 .name = "ZCR_EL2", .state = ARM_CP_STATE_AA64,
 .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 2, .opc2 = 0,
-.access = PL2_RW,
+.access = PL2_RW, .type = ARM_CP_SVE | ARM_CP_FPU,
 .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore
 };
 
 static const ARMCPRegInfo zcr_el3_reginfo = {
 .name = "ZCR_EL3", .state = ARM_CP_STATE_AA64,
 .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 2, .opc2 = 0,
-.access = PL3_RW, .accessfn = zcr_access,
+.access = PL3_RW, .type = ARM_CP_SVE | ARM_CP_FPU,
 .fieldoffset = offsetof(CPUARMState, vfp.zcr_el[3]),
 .writefn = zcr_write, .raw_writefn = raw_write
 };
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 89f50558a7..e3881d4999 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -1182,6 +1182,19 @@ static inline bool fp_access_check(DisasContext *s)
 return false;
 }
 
+/* Check that SVE access is enabled.  If it is, return true.
+ * If not, emit code to generate an appropriate 

[Qemu-devel] [PULL 17/20] target/arm: Migrate v7m.other_sp

[Peter Maydell]

In commit abc24d86cc0364f we accidentally broke migration of
the stack pointer value for the mode (process, handler) the CPU
is not currently running as. (The commit correctly removed the
no-longer-used v7m.current_sp flag from the VMState but also
deleted the still very much in use v7m.other_sp SP value field.)

Add a subsection to migrate it again. (We don't need to care
about trying to retain compatibility with pre-abc24d86cc0364f
versions of QEMU, because that commit bumped the version_id
and we've since bumped it again a couple of times.)

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Message-id: 20180209165810.6668-11-peter.mayd...@linaro.org
---
 target/arm/machine.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/target/arm/machine.c b/target/arm/machine.c
index 25cdf4d581..1a20d6c36c 100644
--- a/target/arm/machine.c
+++ b/target/arm/machine.c
@@ -236,6 +236,16 @@ static const VMStateDescription vmstate_m_scr = {
 }
 };
 
+static const VMStateDescription vmstate_m_other_sp = {
+.name = "cpu/m/other-sp",
+.version_id = 1,
+.minimum_version_id = 1,
+.fields = (VMStateField[]) {
+VMSTATE_UINT32(env.v7m.other_sp, ARMCPU),
+VMSTATE_END_OF_LIST()
+}
+};
+
 static const VMStateDescription vmstate_m = {
 .name = "cpu/m",
 .version_id = 4,
@@ -259,6 +269,7 @@ static const VMStateDescription vmstate_m = {
 _m_faultmask_primask,
 _m_csselr,
 _m_scr,
+_m_other_sp,
 NULL
 }
 };
-- 
2.16.1

[Qemu-devel] [PULL 20/20] raspi: Raspberry Pi 3 support

[Peter Maydell]

From: Pekka Enberg 

This patch adds Raspberry Pi 3 support to hw/arm/raspi.c. The
differences to Pi 2 are:

 - Firmware address
 - Board ID
 - Board revision

The CPU is different too, but that's going to be configured as part of
the machine default CPU when we introduce a new machine type.

The patch was written from scratch by me but the logic is similar to
Zoltán Baldaszti's previous work, which I used as a reference (with
permission from the author):

  https://github.com/bztsrc/qemu-raspi3

Signed-off-by: Pekka Enberg 
[PMM: fixed trailing whitespace on one line]
Reviewed-by: Peter Maydell 
Signed-off-by: Peter Maydell 
---
 hw/arm/raspi.c | 31 +--
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c
index c24a4a1b14..93121c56bf 100644
--- a/hw/arm/raspi.c
+++ b/hw/arm/raspi.c
@@ -5,6 +5,9 @@
  * Rasperry Pi 2 emulation Copyright (c) 2015, Microsoft
  * Written by Andrew Baumann
  *
+ * Raspberry Pi 3 emulation Copyright (c) 2018 Zoltán Baldaszti
+ * Upstream code cleanup (c) 2018 Pekka Enberg
+ *
  * This code is licensed under the GNU GPLv2 and later.
  */
 
@@ -22,10 +25,11 @@
 #define SMPBOOT_ADDR0x300 /* this should leave enough space for ATAGS */
 #define MVBAR_ADDR  0x400 /* secure vectors */
 #define BOARDSETUP_ADDR (MVBAR_ADDR + 0x20) /* board setup code */
-#define FIRMWARE_ADDR   0x8000 /* Pi loads kernel.img here by default */
+#define FIRMWARE_ADDR_2 0x8000 /* Pi 2 loads kernel.img here by default */
+#define FIRMWARE_ADDR_3 0x8 /* Pi 3 loads kernel.img here by default */
 
 /* Table of Linux board IDs for different Pi versions */
-static const int raspi_boardid[] = {[1] = 0xc42, [2] = 0xc43};
+static const int raspi_boardid[] = {[1] = 0xc42, [2] = 0xc43, [3] = 0xc44};
 
 typedef struct RasPiState {
 BCM2836State soc;
@@ -83,8 +87,8 @@ static void setup_boot(MachineState *machine, int version, 
size_t ram_size)
 binfo.secure_board_setup = true;
 binfo.secure_boot = true;
 
-/* Pi2 requires SMP setup */
-if (version == 2) {
+/* Pi2 and Pi3 requires SMP setup */
+if (version >= 2) {
 binfo.smp_loader_start = SMPBOOT_ADDR;
 binfo.write_secondary_boot = write_smpboot;
 binfo.secondary_cpu_reset_hook = reset_secondary;
@@ -94,15 +98,16 @@ static void setup_boot(MachineState *machine, int version, 
size_t ram_size)
  * the normal Linux boot process
  */
 if (machine->firmware) {
+hwaddr firmware_addr = version == 3 ? FIRMWARE_ADDR_3 : 
FIRMWARE_ADDR_2;
 /* load the firmware image (typically kernel.img) */
-r = load_image_targphys(machine->firmware, FIRMWARE_ADDR,
-ram_size - FIRMWARE_ADDR);
+r = load_image_targphys(machine->firmware, firmware_addr,
+ram_size - firmware_addr);
 if (r < 0) {
 error_report("Failed to load firmware from %s", machine->firmware);
 exit(1);
 }
 
-binfo.entry = FIRMWARE_ADDR;
+binfo.entry = firmware_addr;
 binfo.firmware_loaded = true;
 } else {
 binfo.kernel_filename = machine->kernel_filename;
@@ -113,7 +118,7 @@ static void setup_boot(MachineState *machine, int version, 
size_t ram_size)
 arm_load_kernel(ARM_CPU(first_cpu), );
 }
 
-static void raspi2_init(MachineState *machine)
+static void raspi_init(MachineState *machine, int version)
 {
 RasPiState *s = g_new0(RasPiState, 1);
 uint32_t vcram_size;
@@ -139,7 +144,8 @@ static void raspi2_init(MachineState *machine)
 _abort);
 object_property_set_int(OBJECT(>soc), smp_cpus, "enabled-cpus",
 _abort);
-object_property_set_int(OBJECT(>soc), 0xa21041, "board-rev",
+int board_rev = version == 3 ? 0xa02082 : 0xa21041;
+object_property_set_int(OBJECT(>soc), board_rev, "board-rev",
 _abort);
 object_property_set_bool(OBJECT(>soc), true, "realized", _abort);
 
@@ -157,7 +163,12 @@ static void raspi2_init(MachineState *machine)
 
 vcram_size = object_property_get_uint(OBJECT(>soc), "vcram-size",
   _abort);
-setup_boot(machine, 2, machine->ram_size - vcram_size);
+setup_boot(machine, version, machine->ram_size - vcram_size);
+}
+
+static void raspi2_init(MachineState *machine)
+{
+raspi_init(machine, 2);
 }
 
 static void raspi2_machine_init(MachineClass *mc)
-- 
2.16.1

[Qemu-devel] [PULL 03/20] target/arm: Remove ARM_CP_64BIT from ZCR_EL registers

[Peter Maydell]

From: Richard Henderson 

Because they are ARM_CP_STATE_AA64, ARM_CP_64BIT is implied.

Signed-off-by: Richard Henderson 
Message-id: 20180211205848.4568-2-richard.hender...@linaro.org
Reviewed-by: Peter Maydell 
Signed-off-by: Peter Maydell 
---
 target/arm/helper.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 180ab75458..4b102ec356 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -4357,7 +4357,7 @@ static void zcr_write(CPUARMState *env, const 
ARMCPRegInfo *ri,
 static const ARMCPRegInfo zcr_el1_reginfo = {
 .name = "ZCR_EL1", .state = ARM_CP_STATE_AA64,
 .opc0 = 3, .opc1 = 0, .crn = 1, .crm = 2, .opc2 = 0,
-.access = PL1_RW, .accessfn = zcr_access, .type = ARM_CP_64BIT,
+.access = PL1_RW, .accessfn = zcr_access,
 .fieldoffset = offsetof(CPUARMState, vfp.zcr_el[1]),
 .writefn = zcr_write, .raw_writefn = raw_write
 };
@@ -4365,7 +4365,7 @@ static const ARMCPRegInfo zcr_el1_reginfo = {
 static const ARMCPRegInfo zcr_el2_reginfo = {
 .name = "ZCR_EL2", .state = ARM_CP_STATE_AA64,
 .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 2, .opc2 = 0,
-.access = PL2_RW, .accessfn = zcr_access, .type = ARM_CP_64BIT,
+.access = PL2_RW, .accessfn = zcr_access,
 .fieldoffset = offsetof(CPUARMState, vfp.zcr_el[2]),
 .writefn = zcr_write, .raw_writefn = raw_write
 };
@@ -4373,14 +4373,14 @@ static const ARMCPRegInfo zcr_el2_reginfo = {
 static const ARMCPRegInfo zcr_no_el2_reginfo = {
 .name = "ZCR_EL2", .state = ARM_CP_STATE_AA64,
 .opc0 = 3, .opc1 = 4, .crn = 1, .crm = 2, .opc2 = 0,
-.access = PL2_RW, .type = ARM_CP_64BIT,
+.access = PL2_RW,
 .readfn = arm_cp_read_zero, .writefn = arm_cp_write_ignore
 };
 
 static const ARMCPRegInfo zcr_el3_reginfo = {
 .name = "ZCR_EL3", .state = ARM_CP_STATE_AA64,
 .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 2, .opc2 = 0,
-.access = PL3_RW, .accessfn = zcr_access, .type = ARM_CP_64BIT,
+.access = PL3_RW, .accessfn = zcr_access,
 .fieldoffset = offsetof(CPUARMState, vfp.zcr_el[3]),
 .writefn = zcr_write, .raw_writefn = raw_write
 };
-- 
2.16.1

[Qemu-devel] [PULL 02/20] hw/arm/aspeed: simplify using the 'unimplemented device' for aspeed_soc.io

[Peter Maydell]

From: Philippe Mathieu-Daudé 

(qemu) info mtree
 address-space: cpu-memory-0
   - (prio 0, i/o): system
 -07ff (prio 0, rom): aspeed.boot_rom
-1e60-1e7f (prio -1, i/o): aspeed_soc.io
+1e60-1e7f (prio -1000, i/o): aspeed_soc.io
 1e62-1e6200ff (prio 0, i/o): aspeed.smc.ast2500-fmc
 1e63-1e6300ff (prio 0, i/o): aspeed.smc.ast2500-spi1
 1e631000-1e6310ff (prio 0, i/o): aspeed.smc.ast2500-spi2

Signed-off-by: Philippe Mathieu-Daudé 
Reviewed-by: Cédric Le Goater 
Reviewed-by: Andrew Jeffery 
Message-id: 20180209085755.30414-3-f4...@amsat.org
Signed-off-by: Peter Maydell 
---
 include/hw/arm/aspeed_soc.h |  1 -
 hw/arm/aspeed_soc.c | 32 +++-
 2 files changed, 3 insertions(+), 30 deletions(-)

diff --git a/include/hw/arm/aspeed_soc.h b/include/hw/arm/aspeed_soc.h
index f26914a2b9..11ec0179db 100644
--- a/include/hw/arm/aspeed_soc.h
+++ b/include/hw/arm/aspeed_soc.h
@@ -31,7 +31,6 @@ typedef struct AspeedSoCState {
 
 /*< public >*/
 ARMCPU cpu;
-MemoryRegion iomem;
 MemoryRegion sram;
 AspeedVICState vic;
 AspeedTimerCtrlState timerctrl;
diff --git a/hw/arm/aspeed_soc.c b/hw/arm/aspeed_soc.c
index 2a5d041b3b..30d25f8b06 100644
--- a/hw/arm/aspeed_soc.c
+++ b/hw/arm/aspeed_soc.c
@@ -15,6 +15,7 @@
 #include "qemu-common.h"
 #include "cpu.h"
 #include "exec/address-spaces.h"
+#include "hw/misc/unimp.h"
 #include "hw/arm/aspeed_soc.h"
 #include "hw/char/serial.h"
 #include "qemu/log.h"
@@ -99,31 +100,6 @@ static const AspeedSoCInfo aspeed_socs[] = {
 },
 };
 
-/*
- * IO handlers: simply catch any reads/writes to IO addresses that aren't
- * handled by a device mapping.
- */
-
-static uint64_t aspeed_soc_io_read(void *p, hwaddr offset, unsigned size)
-{
-qemu_log_mask(LOG_UNIMP, "%s: 0x%" HWADDR_PRIx " [%u]\n",
-  __func__, offset, size);
-return 0;
-}
-
-static void aspeed_soc_io_write(void *opaque, hwaddr offset, uint64_t value,
-unsigned size)
-{
-qemu_log_mask(LOG_UNIMP, "%s: 0x%" HWADDR_PRIx " <- 0x%" PRIx64 " [%u]\n",
-  __func__, offset, value, size);
-}
-
-static const MemoryRegionOps aspeed_soc_io_ops = {
-.read = aspeed_soc_io_read,
-.write = aspeed_soc_io_write,
-.endianness = DEVICE_LITTLE_ENDIAN,
-};
-
 static void aspeed_soc_init(Object *obj)
 {
 AspeedSoCState *s = ASPEED_SOC(obj);
@@ -199,10 +175,8 @@ static void aspeed_soc_realize(DeviceState *dev, Error 
**errp)
 Error *err = NULL, *local_err = NULL;
 
 /* IO space */
-memory_region_init_io(>iomem, NULL, _soc_io_ops, NULL,
-"aspeed_soc.io", ASPEED_SOC_IOMEM_SIZE);
-memory_region_add_subregion_overlap(get_system_memory(),
-ASPEED_SOC_IOMEM_BASE, >iomem, -1);
+create_unimplemented_device("aspeed_soc.io",
+ASPEED_SOC_IOMEM_BASE, ASPEED_SOC_IOMEM_SIZE);
 
 /* CPU */
 object_property_set_bool(OBJECT(>cpu), true, "realized", );
-- 
2.16.1

[Qemu-devel] [PULL 15/20] hw/intc/armv7m_nvic: Fix byte-to-interrupt number conversions

[Peter Maydell]

In many of the NVIC registers relating to interrupts, we
have to convert from a byte offset within a register set
into the number of the first interrupt which is affected.
We were getting this wrong for:
 * reads of NVIC_ISPR, NVIC_ISER, NVIC_ICPR, NVIC_ICER,
   NVIC_IABR -- in all these cases we were missing the "* 8"
   needed to convert from the byte offset to the interrupt number
   (since all these registers use one bit per interrupt)
 * writes of NVIC_IPR had the opposite problem of a spurious
   "* 8" (since these registers use one byte per interrupt)

Signed-off-by: Peter Maydell 
Reviewed-by: Philippe Mathieu-Daudé 
Message-id: 20180209165810.6668-9-peter.mayd...@linaro.org
---
 hw/intc/armv7m_nvic.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index ea3b7cce14..c51151fa8a 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -1724,7 +1724,7 @@ static MemTxResult nvic_sysreg_read(void *opaque, hwaddr 
addr,
 /* fall through */
 case 0x180 ... 0x1bf: /* NVIC Clear enable */
 val = 0;
-startvec = offset - 0x180 + NVIC_FIRST_IRQ; /* vector # */
+startvec = 8 * (offset - 0x180) + NVIC_FIRST_IRQ; /* vector # */
 
 for (i = 0, end = size * 8; i < end && startvec + i < s->num_irq; i++) 
{
 if (s->vectors[startvec + i].enabled &&
@@ -1738,7 +1738,7 @@ static MemTxResult nvic_sysreg_read(void *opaque, hwaddr 
addr,
 /* fall through */
 case 0x280 ... 0x2bf: /* NVIC Clear pend */
 val = 0;
-startvec = offset - 0x280 + NVIC_FIRST_IRQ; /* vector # */
+startvec = 8 * (offset - 0x280) + NVIC_FIRST_IRQ; /* vector # */
 for (i = 0, end = size * 8; i < end && startvec + i < s->num_irq; i++) 
{
 if (s->vectors[startvec + i].pending &&
 (attrs.secure || s->itns[startvec + i])) {
@@ -1748,7 +1748,7 @@ static MemTxResult nvic_sysreg_read(void *opaque, hwaddr 
addr,
 break;
 case 0x300 ... 0x33f: /* NVIC Active */
 val = 0;
-startvec = offset - 0x300 + NVIC_FIRST_IRQ; /* vector # */
+startvec = 8 * (offset - 0x300) + NVIC_FIRST_IRQ; /* vector # */
 
 for (i = 0, end = size * 8; i < end && startvec + i < s->num_irq; i++) 
{
 if (s->vectors[startvec + i].active &&
@@ -1863,7 +1863,7 @@ static MemTxResult nvic_sysreg_write(void *opaque, hwaddr 
addr,
 case 0x300 ... 0x33f: /* NVIC Active */
 return MEMTX_OK; /* R/O */
 case 0x400 ... 0x5ef: /* NVIC Priority */
-startvec = 8 * (offset - 0x400) + NVIC_FIRST_IRQ; /* vector # */
+startvec = (offset - 0x400) + NVIC_FIRST_IRQ; /* vector # */
 
 for (i = 0; i < size && startvec + i < s->num_irq; i++) {
 if (attrs.secure || s->itns[startvec + i]) {
-- 
2.16.1

[Qemu-devel] [PULL 14/20] target/arm: Implement writing to CONTROL_NS for v8M

[Peter Maydell]

In commit 50f11062d4c896 we added support for MSR/MRS access
to the NS banked special registers, but we forgot to implement
the support for writing to CONTROL_NS. Correct the omission.

Signed-off-by: Peter Maydell 
Reviewed-by: Richard Henderson 
Message-id: 20180209165810.6668-8-peter.mayd...@linaro.org
---
 target/arm/helper.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 550dc3d290..1ae11997fb 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -10507,6 +10507,16 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t 
maskreg, uint32_t val)
 }
 env->v7m.faultmask[M_REG_NS] = val & 1;
 return;
+case 0x94: /* CONTROL_NS */
+if (!env->v7m.secure) {
+return;
+}
+write_v7m_control_spsel_for_secstate(env,
+ val & 
R_V7M_CONTROL_SPSEL_MASK,
+ M_REG_NS);
+env->v7m.control[M_REG_NS] &= ~R_V7M_CONTROL_NPRIV_MASK;
+env->v7m.control[M_REG_NS] |= val & R_V7M_CONTROL_NPRIV_MASK;
+return;
 case 0x98: /* SP_NS */
 {
 /* This gives the non-secure SP selected based on whether we're
-- 
2.16.1

[Qemu-devel] [PULL 05/20] target/arm: Suppress TB end for FPCR/FPSR

[Peter Maydell]

From: Richard Henderson 

Nothing in either register affects the TB.

Signed-off-by: Richard Henderson 
Message-id: 20180211205848.4568-4-richard.hender...@linaro.org
Reviewed-by: Peter Maydell 
Signed-off-by: Peter Maydell 
---
 target/arm/helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index d41fb8371f..e0184c7162 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -3356,11 +3356,11 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
   .writefn = aa64_daif_write, .resetfn = arm_cp_reset_ignore },
 { .name = "FPCR", .state = ARM_CP_STATE_AA64,
   .opc0 = 3, .opc1 = 3, .opc2 = 0, .crn = 4, .crm = 4,
-  .access = PL0_RW, .type = ARM_CP_FPU,
+  .access = PL0_RW, .type = ARM_CP_FPU | ARM_CP_SUPPRESS_TB_END,
   .readfn = aa64_fpcr_read, .writefn = aa64_fpcr_write },
 { .name = "FPSR", .state = ARM_CP_STATE_AA64,
   .opc0 = 3, .opc1 = 3, .opc2 = 1, .crn = 4, .crm = 4,
-  .access = PL0_RW, .type = ARM_CP_FPU,
+  .access = PL0_RW, .type = ARM_CP_FPU | ARM_CP_SUPPRESS_TB_END,
   .readfn = aa64_fpsr_read, .writefn = aa64_fpsr_write },
 { .name = "DCZID_EL0", .state = ARM_CP_STATE_AA64,
   .opc0 = 3, .opc1 = 3, .opc2 = 7, .crn = 0, .crm = 0,
-- 
2.16.1

[Qemu-devel] [PULL 00/20] target-arm queue

[Peter Maydell]

Changes v1->v2: it turns out that the raspi3 support exposes a
preexisting bug in our register definitions for VMPIDR/VMIDR:
https://lists.gnu.org/archive/html/qemu-devel/2018-02/msg04181.html

So I've dropped the final "enable raspi3 board" patch for the
moment. When that VMIDR/VMPIDR patch gets reviewed we can
put the raspi3 patch in with it.


thanks
-- PMM

The following changes since commit f003d07337a6d4d02c43429b26a4270459afb51a:

  Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into 
staging (2018-02-15 15:45:33 +)

are available in the Git repository at:

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

for you to fetch changes up to bade58166f4466546600d824a2695a00269d10eb:

  raspi: Raspberry Pi 3 support (2018-02-15 18:33:46 +)


target-arm queue:
 * aspeed: code cleanup to use unimplemented_device
 * preparatory work for 'raspi3' RaspberryPi 3 machine model
 * more SVE prep work
 * v8M: add minor missing registers
 * v7M: fix bug where we weren't migrating v7m.other_sp
 * v7M: fix bugs in handling of interrupt registers for
   external interrupts beyond 32


Pekka Enberg (2):
  bcm2836: Make CPU type configurable
  raspi: Raspberry Pi 3 support

Peter Maydell (11):
  hw/intc/armv7m_nvic: Don't hardcode M profile ID registers in NVIC
  hw/intc/armv7m_nvic: Fix ICSR PENDNMISET/CLR handling
  hw/intc/armv7m_nvic: Implement M profile cache maintenance ops
  hw/intc/armv7m_nvic: Implement v8M CPPWR register
  hw/intc/armv7m_nvic: Implement cache ID registers
  hw/intc/armv7m_nvic: Implement SCR
  target/arm: Implement writing to CONTROL_NS for v8M
  hw/intc/armv7m_nvic: Fix byte-to-interrupt number conversions
  target/arm: Add AIRCR to vmstate struct
  target/arm: Migrate v7m.other_sp
  target/arm: Implement v8M MSPLIM and PSPLIM registers

Philippe Mathieu-Daudé (2):
  hw/arm/aspeed: directly map the serial device to the system address space
  hw/arm/aspeed: simplify using the 'unimplemented device' for aspeed_soc.io

Richard Henderson (5):
  target/arm: Remove ARM_CP_64BIT from ZCR_EL registers
  target/arm: Enforce FP access to FPCR/FPSR
  target/arm: Suppress TB end for FPCR/FPSR
  target/arm: Enforce access to ZCR_EL at translation
  target/arm: Handle SVE registers when using clear_vec_high

 include/hw/arm/aspeed_soc.h |   1 -
 include/hw/arm/bcm2836.h|   1 +
 target/arm/cpu.h|  71 -
 target/arm/internals.h  |   6 ++
 hw/arm/aspeed_soc.c |  35 ++---
 hw/arm/bcm2836.c|  17 +++--
 hw/arm/raspi.c  |  34 ++---
 hw/intc/armv7m_nvic.c   |  98 ++--
 target/arm/cpu.c|  28 +++
 target/arm/helper.c |  84 +++-
 target/arm/machine.c|  84 
 target/arm/translate-a64.c  | 181 
 12 files changed, 429 insertions(+), 211 deletions(-)

Re: [Qemu-devel] [PATCH v2] docs: document our stable process

[Peter Maydell]

On 12 February 2018 at 12:40, Cornelia Huck  wrote:
> Some pointers on how to get a patch into stable.
>
> [contains some suggestions by mdroth]
> Signed-off-by: Cornelia Huck 
> ---
> RFC/D->v2: added mdroth's suggestions
> ---
>  docs/stable-process.rst | 67 
> +
>  1 file changed, 67 insertions(+)
>  create mode 100644 docs/stable-process.rst
>

Drive-by bikeshed comment: should this be in docs/devel ?
It's aimed at QEMU developers rather than users...

thanks
-- PMM

Re: [Qemu-devel] [PATCH] docker: dump 'config.log' if ./configure fails

[Eric Blake]

On 02/15/2018 11:41 AM, Philippe Mathieu-Daudé wrote:

On 02/15/2018 02:34 PM, Daniel P. Berrangé wrote:

On Thu, Feb 15, 2018 at 02:23:06PM -0300, Philippe Mathieu-Daudé wrote:

Suggested-by: Eric Blake 
Signed-off-by: Philippe Mathieu-Daudé 
---
  tests/docker/common.rc | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/docker/common.rc b/tests/docker/common.rc
index 7951555e3f..dd011cdf1e 100755
--- a/tests/docker/common.rc
+++ b/tests/docker/common.rc
@@ -30,7 +30,9 @@ build_qemu()
   $@"
  echo "Configure options:"
  echo $config_opts
-$QEMU_SRC/configure $config_opts && make $MAKEFLAGS
+$QEMU_SRC/configure $config_opts || \
+(cat config.log && prep_fail "Failed to run 'configure'")


Is a subshell necessary; or can you get by with the simpler
 { cat && prep_fail "..."; }


+make $MAKEFLAGS
  }


Reviewed-by: Daniel P. Berrangé 

Slightly nicer than my patch thanks to the prep_fail addition.


Haha this was a funny experience of parallel programming :)

However your patch has a slightly nicer description!



--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH] vnc: remove bogus object_unref on client socket

[Bandan Das]

Daniel P. Berrangé  writes:

> vnc_listen_io() does not own the reference on the 'cioc' parameter is it
> passed, so should not be unref'ing it.
>
> Reported-by: Bandan Das 
> Signed-off-by: Daniel P. Berrangé 

Daniel, wouldn't a Fixes: 13e1d0e71e78a925848258391a6e616b6b5ae219
be helpful here ?

> ---
>  ui/vnc.c | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/ui/vnc.c b/ui/vnc.c
> index c715bae1cf..b97769aa9e 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -3152,7 +3152,6 @@ static void vnc_listen_io(QIONetListener *listener,
>   isWebsock ? "vnc-ws-server" : "vnc-server");
>  qio_channel_set_delay(QIO_CHANNEL(cioc), false);
>  vnc_connect(vd, cioc, false, isWebsock);
> -object_unref(OBJECT(cioc));
>  }
>  
>  static const DisplayChangeListenerOps dcl_ops = {

Re: [Qemu-devel] [PATCH] tests: dump config.log when configure fails in docker job

[Eric Blake]

On 02/15/2018 11:16 AM, Daniel P. Berrangé wrote:

When configure fails inside a docker job it is not easy to get access to
the config.log file. It is nicer for developers if we just splatter the
contents of config.log to stdout upon failure

Suggested-by: Eric Blake 
Signed-off-by: Daniel P. Berrangé 
---
  tests/docker/common.rc | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/docker/common.rc b/tests/docker/common.rc
index 7951555e3f..ecbea13443 100755
--- a/tests/docker/common.rc
+++ b/tests/docker/common.rc
@@ -30,7 +30,8 @@ build_qemu()
   $@"
  echo "Configure options:"
  echo $config_opts
-$QEMU_SRC/configure $config_opts && make $MAKEFLAGS


The old code dies if either configure or make has non-zero exit status.


+$QEMU_SRC/configure $config_opts || cat config.log
+make $MAKEFLAGS


But this turns a failure of configure into a successful exit status 
(presuming that cat doesn't also fail), which means we try the make no 
matter whether configure succeeded.


Better might be:

$QEMU_SRC/configure $config_opts || { cat config.log; exit 1; }


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[Qemu-devel] [PATCH] target/arm: Fix register definitions for VMIDR and VMPIDR

[Peter Maydell]

The register definitions for VMIDR and VMPIDR have separate
reginfo structs for the AArch32 and AArch64 registers. However
the 32-bit versions are wrong:
 * they use offsetof instead of offsetoflow32 to mark where
   the 32-bit value lives in the uint64_t CPU state field
 * they don't mark themselves as ARM_CP_ALIAS

In particular this means that if you try to use an Arm guest CPU
which enables EL2 on a big-endian host it will assert at reset:
 target/arm/cpu.c:114: cp_reg_check_reset: Assertion `oldvalue == newvalue' 
failed.

because the reset of the 32-bit register writes to the top
half of the uint64_t.

Correct the errors in the structures.

Signed-off-by: Peter Maydell 
---
This is necessary for 'make check' to pass on big endian
systems with the 'raspi3' board enabled, which is the
first board which has an EL2-enabled-by-default CPU.
---
 target/arm/helper.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index e7586fcf6c..e27957df38 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -5068,8 +5068,8 @@ void register_cp_regs_for_features(ARMCPU *cpu)
 { .name = "VPIDR", .state = ARM_CP_STATE_AA32,
   .cp = 15, .opc1 = 4, .crn = 0, .crm = 0, .opc2 = 0,
   .access = PL2_RW, .accessfn = access_el3_aa32ns,
-  .resetvalue = cpu->midr,
-  .fieldoffset = offsetof(CPUARMState, cp15.vpidr_el2) },
+  .resetvalue = cpu->midr, .type = ARM_CP_ALIAS,
+  .fieldoffset = offsetoflow32(CPUARMState, cp15.vpidr_el2) },
 { .name = "VPIDR_EL2", .state = ARM_CP_STATE_AA64,
   .opc0 = 3, .opc1 = 4, .crn = 0, .crm = 0, .opc2 = 0,
   .access = PL2_RW, .resetvalue = cpu->midr,
@@ -5077,8 +5077,8 @@ void register_cp_regs_for_features(ARMCPU *cpu)
 { .name = "VMPIDR", .state = ARM_CP_STATE_AA32,
   .cp = 15, .opc1 = 4, .crn = 0, .crm = 0, .opc2 = 5,
   .access = PL2_RW, .accessfn = access_el3_aa32ns,
-  .resetvalue = vmpidr_def,
-  .fieldoffset = offsetof(CPUARMState, cp15.vmpidr_el2) },
+  .resetvalue = vmpidr_def, .type = ARM_CP_ALIAS,
+  .fieldoffset = offsetoflow32(CPUARMState, cp15.vmpidr_el2) },
 { .name = "VMPIDR_EL2", .state = ARM_CP_STATE_AA64,
   .opc0 = 3, .opc1 = 4, .crn = 0, .crm = 0, .opc2 = 5,
   .access = PL2_RW,
-- 
2.16.1

Re: [Qemu-devel] [RFC PATCH] capstone: fix building using system package

[Sergei Trofimovich]

On Thu, 15 Feb 2018 14:35:39 -0300
Philippe Mathieu-Daudé  wrote:

>  #else
> +#include 

I think it's incorrect. 'pkg-config' already reports 'capstone/' path:
$ pkg-config --cflags capstone
-I/usr/include/capstone

$ ls /usr/include/capstone/capstone.h
/usr/include/capstone/capstone.h

Thus I would guess
#include 
is still correct for system include path as well (contradicts the example).

qemu just needs to use 'pkg-config' to discover the include path and
libs. Maybe new capstone release has different pkgconfig setup?

-- 

  Sergei

Re: [Qemu-devel] [Qemu-arm] [PATCH 2/3] hw/arm/boot: Honour CPU's address space for image loads

[Philippe Mathieu-Daudé]

On 02/15/2018 03:06 PM, Peter Maydell wrote:
> On 15 February 2018 at 17:57, Peter Maydell  wrote:
>> Instead of loading kernels, device trees, and the like to
>> the system address space, use the CPU's address space. This
>> is important if we're trying to load the file to memory or
>> via an alias memory region that is provided by an SoC
>> object and thus not mapped into the system address space.
>>
>> Signed-off-by: Peter Maydell 
>> ---
>>  hw/arm/boot.c | 119 
>> +-
>>  1 file changed, 76 insertions(+), 43 deletions(-)
>>
>> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
>> index 9b174b982c..0eac655c98 100644
>> --- a/hw/arm/boot.c
>> +++ b/hw/arm/boot.c
>> @@ -35,6 +35,25 @@
>>  #define ARM64_TEXT_OFFSET_OFFSET8
>>  #define ARM64_MAGIC_OFFSET  56
>>
>> +static AddressSpace *arm_boot_addressspace(ARMCPU *cpu,
>> +   const struct arm_boot_info *info)
>> +{
>> +/* Return the address space to use for bootloader reads and writes.
>> + * We prefer the secure address space if the CPU has it and we're
>> + * going to boot the guest into it.
>> + */
>> +int asidx;
>> +CPUState *cs = CPU(cpu);
>> +
>> +if (arm_feature(>env, ARM_FEATURE_EL3) && info->secure_boot) {
>> +asidx = ARMASIdx_S;
>> +} else {
>> +asidx = ARMASIdx_NS;
>> +}
>> +
>> +return cpu_get_address_space(cs, asidx);
>> +}
>> +
>>  typedef enum {
>>  FIXUP_NONE = 0, /* do nothing */
>>  FIXUP_TERMINATOR,   /* end of insns */
>> @@ -124,7 +143,8 @@ static const ARMInsnFixup smpboot[] = {
>>  };
>>
>>  static void write_bootloader(const char *name, hwaddr addr,
>> - const ARMInsnFixup *insns, uint32_t 
>> *fixupcontext)
>> + const ARMInsnFixup *insns, uint32_t 
>> *fixupcontext,
>> + AddressSpace *as)
>>  {
>>  /* Fix up the specified bootloader fragment and write it into
>>   * guest memory using rom_add_blob_fixed(). fixupcontext is
>> @@ -163,7 +183,7 @@ static void write_bootloader(const char *name, hwaddr 
>> addr,
>>  code[i] = tswap32(insn);
>>  }
>>
>> -rom_add_blob_fixed(name, code, len * sizeof(uint32_t), addr);
>> +rom_add_blob_fixed_as(name, code, len * sizeof(uint32_t), addr, as);
>>
>>  g_free(code);
>>  }
>> @@ -172,6 +192,7 @@ static void default_write_secondary(ARMCPU *cpu,
>>  const struct arm_boot_info *info)
>>  {
>>  uint32_t fixupcontext[FIXUP_MAX];
>> +AddressSpace *as = CPU(cpu)->as;
> 
> Oops. This should be
> AddressSpace *as = arm_boot_addressspace(cpu, info);

I was stuck there wondering why this one were using the CPU AS :)

> 
>>  fixupcontext[FIXUP_GIC_CPU_IF] = info->gic_cpu_if_addr;
>>  fixupcontext[FIXUP_BOOTREG] = info->smp_bootreg_addr;
>> @@ -182,13 +203,14 @@ static void default_write_secondary(ARMCPU *cpu,
>>  }
>>
>>  write_bootloader("smpboot", info->smp_loader_start,
>> - smpboot, fixupcontext);
>> + smpboot, fixupcontext, as);
>>  }
>>
>>  void arm_write_secure_board_setup_dummy_smc(ARMCPU *cpu,
>>  const struct arm_boot_info 
>> *info,
>>  hwaddr mvbar_addr)
>>  {
>> +AddressSpace *as = CPU(cpu)->as;
> 
> ...and so should this.

Reviewed-by: Philippe Mathieu-Daudé 

> 
> thanks
> -- PMM
> 

Re: [Qemu-devel] [PATCH v2] docs: document our stable process

[Michael Roth]

Quoting Cornelia Huck (2018-02-12 06:40:08)
> Some pointers on how to get a patch into stable.
> 
> [contains some suggestions by mdroth]
> Signed-off-by: Cornelia Huck 
> ---
> RFC/D->v2: added mdroth's suggestions
> ---
>  docs/stable-process.rst | 67 
> +
>  1 file changed, 67 insertions(+)
>  create mode 100644 docs/stable-process.rst
> 
> diff --git a/docs/stable-process.rst b/docs/stable-process.rst
> new file mode 100644
> index 00..c571204c9f
> --- /dev/null
> +++ b/docs/stable-process.rst
> @@ -0,0 +1,67 @@
> +QEMU and the stable process
> +===
> +
> +QEMU stable releases
> +
> +
> +QEMU stable releases are based upon the last released QEMU version
> +and marked by an additional version number, e.g. 2.10.1. Occasionally,
> +a four-number version is released, if a single urgent fix needs to go
> +on top.
> +
> +Usually, stable releases are only provided for the last major QEMU
> +release. For example, when QEMU 2.11.0 is released, 2.11.x or 2.11.x.y
> +stable releases are produced only until QEMU 2.12.0 is released, at
> +which the stable process moves to producing 2.12.x/2.12.x.y releases.

"at which point"

Sorry, looks like one of my errors slipped through :)

With that and Eric's suggestions addressed:

Reviewed-by: Michael Roth 

> +
> +What should go into a stable release?
> +-
> +
> +Generally, the following patches are considered stable material:
> +- Patches that fix severe issues, like fixes for CVEs
> +- Patches that fix regressions
> +
> +If you think the patch would be important for users of the current release
> +(or for a distribution picking fixes), it is usually a good candidate
> +for stable.
> +
> +
> +How to get a patch into QEMU stable
> +---
> +
> +There are various ways to get a patch into stable:
> +
> +* Preferred: Make sure that the stable maintainers are on copy when you send
> +  the patch by adding
> +
> +  .. code::
> +
> + Cc: qemu-sta...@nongnu.org
> +
> +  to the patch description. By default, this will send a copy of the  patch
> +  to ``qemu-sta...@nongnu.org`` if you use git send-email, which is where
> +  patches that are stable candidates are tracked by the maintainers.
> +
> +* You can also reply to a patch and put ``qemu-sta...@nongnu.org`` on copy
> +  directly in your mail client if you think a previously submitted patch
> +  should be considered for a stable release.
> +
> +* If a maintainer judges the patch appropriate for stable later on (or you
> +  notify them), they will add the same line to the patch, meaning that
> +  the stable maintainers will be on copy on the maintainer's pull request.
> +
> +* If you judge an already merged patch suitable for stable, send a mail
> +  (preferably as a reply to the most recent patch submission) to
> +  ``qemu-sta...@nongnu.org`` along with ``qemu-devel@nongnu.org`` and
> +  appropriate other people (like the patch author or the relevant maintainer)
> +  on copy.
> +
> +Stable release process
> +--
> +
> +When the stable maintainers prepare a new stable release, they will prepare
> +a git branch with a release candidate and send the patches out to
> +``qemu-devel@nongnu.org`` for review. If any of your patches are included,
> +please verify that they look fine. You may also nominate other patches that
> +you think are suitable for inclusion. After review is complete (may involve
> +more release candidates), a new stable release is made available.
> -- 
> 2.13.6
> 

Re: [Qemu-devel] [Qemu-arm] [PATCH 1/3] loader: Add new load_ramdisk_as()

[Philippe Mathieu-Daudé]

On 02/15/2018 02:57 PM, Peter Maydell wrote:
> Add a function load_ramdisk_as() which behaves like the existing
> load_ramdisk() but allows the caller to specify the AddressSpace
> to use. This matches the pattern we have already for various
> other loader functions.
> 
> Signed-off-by: Peter Maydell 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  include/hw/loader.h | 12 +++-
>  hw/core/loader.c|  8 +++-
>  2 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/loader.h b/include/hw/loader.h
> index 355fe0f5a2..1fd4256782 100644
> --- a/include/hw/loader.h
> +++ b/include/hw/loader.h
> @@ -163,16 +163,26 @@ int load_uimage(const char *filename, hwaddr *ep,
>  void *translate_opaque);
>  
>  /**
> - * load_ramdisk:
> + * load_ramdisk_as:
>   * @filename: Path to the ramdisk image
>   * @addr: Memory address to load the ramdisk to
>   * @max_sz: Maximum allowed ramdisk size (for non-u-boot ramdisks)
> + * @as: The AddressSpace to load the ELF to. The value of 
> address_space_memory
> + *  is used if nothing is supplied here.
>   *
>   * Load a ramdisk image with U-Boot header to the specified memory
>   * address.
>   *
>   * Returns the size of the loaded image on success, -1 otherwise.
>   */
> +int load_ramdisk_as(const char *filename, hwaddr addr, uint64_t max_sz,
> +AddressSpace *as);
> +
> +/**
> + * load_ramdisk:
> + * Same as load_ramdisk_as(), but doesn't allow the caller to specify
> + * an AddressSpace.
> + */
>  int load_ramdisk(const char *filename, hwaddr addr, uint64_t max_sz);
>  
>  ssize_t gunzip(void *dst, size_t dstlen, uint8_t *src, size_t srclen);
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index 91669d65aa..2b9e7394a1 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -729,9 +729,15 @@ int load_uimage_as(const char *filename, hwaddr *ep, 
> hwaddr *loadaddr,
>  
>  /* Load a ramdisk.  */
>  int load_ramdisk(const char *filename, hwaddr addr, uint64_t max_sz)
> +{
> +return load_ramdisk_as(filename, addr, max_sz, NULL);
> +}
> +
> +int load_ramdisk_as(const char *filename, hwaddr addr, uint64_t max_sz,
> +AddressSpace *as)
>  {
>  return load_uboot_image(filename, NULL, , NULL, IH_TYPE_RAMDISK,
> -NULL, NULL, NULL);
> +NULL, NULL, as);
>  }
>  
>  /* Load a gzip-compressed kernel to a dynamically allocated buffer. */
> 

Re: [Qemu-devel] [PATCH 2/3] hw/arm/boot: Honour CPU's address space for image loads

[Peter Maydell]

On 15 February 2018 at 17:57, Peter Maydell  wrote:
> Instead of loading kernels, device trees, and the like to
> the system address space, use the CPU's address space. This
> is important if we're trying to load the file to memory or
> via an alias memory region that is provided by an SoC
> object and thus not mapped into the system address space.
>
> Signed-off-by: Peter Maydell 
> ---
>  hw/arm/boot.c | 119 
> +-
>  1 file changed, 76 insertions(+), 43 deletions(-)
>
> diff --git a/hw/arm/boot.c b/hw/arm/boot.c
> index 9b174b982c..0eac655c98 100644
> --- a/hw/arm/boot.c
> +++ b/hw/arm/boot.c
> @@ -35,6 +35,25 @@
>  #define ARM64_TEXT_OFFSET_OFFSET8
>  #define ARM64_MAGIC_OFFSET  56
>
> +static AddressSpace *arm_boot_addressspace(ARMCPU *cpu,
> +   const struct arm_boot_info *info)
> +{
> +/* Return the address space to use for bootloader reads and writes.
> + * We prefer the secure address space if the CPU has it and we're
> + * going to boot the guest into it.
> + */
> +int asidx;
> +CPUState *cs = CPU(cpu);
> +
> +if (arm_feature(>env, ARM_FEATURE_EL3) && info->secure_boot) {
> +asidx = ARMASIdx_S;
> +} else {
> +asidx = ARMASIdx_NS;
> +}
> +
> +return cpu_get_address_space(cs, asidx);
> +}
> +
>  typedef enum {
>  FIXUP_NONE = 0, /* do nothing */
>  FIXUP_TERMINATOR,   /* end of insns */
> @@ -124,7 +143,8 @@ static const ARMInsnFixup smpboot[] = {
>  };
>
>  static void write_bootloader(const char *name, hwaddr addr,
> - const ARMInsnFixup *insns, uint32_t 
> *fixupcontext)
> + const ARMInsnFixup *insns, uint32_t 
> *fixupcontext,
> + AddressSpace *as)
>  {
>  /* Fix up the specified bootloader fragment and write it into
>   * guest memory using rom_add_blob_fixed(). fixupcontext is
> @@ -163,7 +183,7 @@ static void write_bootloader(const char *name, hwaddr 
> addr,
>  code[i] = tswap32(insn);
>  }
>
> -rom_add_blob_fixed(name, code, len * sizeof(uint32_t), addr);
> +rom_add_blob_fixed_as(name, code, len * sizeof(uint32_t), addr, as);
>
>  g_free(code);
>  }
> @@ -172,6 +192,7 @@ static void default_write_secondary(ARMCPU *cpu,
>  const struct arm_boot_info *info)
>  {
>  uint32_t fixupcontext[FIXUP_MAX];
> +AddressSpace *as = CPU(cpu)->as;

Oops. This should be
AddressSpace *as = arm_boot_addressspace(cpu, info);

>  fixupcontext[FIXUP_GIC_CPU_IF] = info->gic_cpu_if_addr;
>  fixupcontext[FIXUP_BOOTREG] = info->smp_bootreg_addr;
> @@ -182,13 +203,14 @@ static void default_write_secondary(ARMCPU *cpu,
>  }
>
>  write_bootloader("smpboot", info->smp_loader_start,
> - smpboot, fixupcontext);
> + smpboot, fixupcontext, as);
>  }
>
>  void arm_write_secure_board_setup_dummy_smc(ARMCPU *cpu,
>  const struct arm_boot_info *info,
>  hwaddr mvbar_addr)
>  {
> +AddressSpace *as = CPU(cpu)->as;

...and so should this.

thanks
-- PMM

Re: [Qemu-devel] [Qemu-arm] [PATCH 3/3] hw/arm/armv7m: Honour CPU's address space for image loads

[Philippe Mathieu-Daudé]

On 02/15/2018 02:57 PM, Peter Maydell wrote:
> Instead of loading guest images to the system address space, use the
> CPU's address space.  This is important if we're trying to load the
> file to memory or via an alias memory region that is provided by an
> SoC object and thus not mapped into the system address space.
> 
> Signed-off-by: Peter Maydell 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  hw/arm/armv7m.c | 17 ++---
>  1 file changed, 14 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
> index 56770a7048..facc536b07 100644
> --- a/hw/arm/armv7m.c
> +++ b/hw/arm/armv7m.c
> @@ -270,6 +270,9 @@ void armv7m_load_kernel(ARMCPU *cpu, const char 
> *kernel_filename, int mem_size)
>  uint64_t entry;
>  uint64_t lowaddr;
>  int big_endian;
> +AddressSpace *as;
> +int asidx;
> +CPUState *cs = CPU(cpu);
>  
>  #ifdef TARGET_WORDS_BIGENDIAN
>  big_endian = 1;
> @@ -282,11 +285,19 @@ void armv7m_load_kernel(ARMCPU *cpu, const char 
> *kernel_filename, int mem_size)
>  exit(1);
>  }
>  
> +if (arm_feature(>env, ARM_FEATURE_EL3)) {
> +asidx = ARMASIdx_S;
> +} else {
> +asidx = ARMASIdx_NS;
> +}
> +as = cpu_get_address_space(cs, asidx);
> +
>  if (kernel_filename) {
> -image_size = load_elf(kernel_filename, NULL, NULL, , ,
> -  NULL, big_endian, EM_ARM, 1, 0);
> +image_size = load_elf_as(kernel_filename, NULL, NULL, , 
> ,
> + NULL, big_endian, EM_ARM, 1, 0, as);
>  if (image_size < 0) {
> -image_size = load_image_targphys(kernel_filename, 0, mem_size);
> +image_size = load_image_targphys_as(kernel_filename, 0,
> +mem_size, as);
>  lowaddr = 0;
>  }
>  if (image_size < 0) {
> 

[Qemu-devel] [PATCH 0/3] Arm: honour CPU address space for image loads

[Peter Maydell]

This patchset makes the Arm code for loading kernels, initrds,
etc etc honour the CPU's address space rather than loading
everything via the system address space. This makes a difference
when the image is being loaded to memory or via an alias memory
region which is implemented by an SoC container object (and
which therefore doesn't appear in the system address space).

I needed this in particular for M profile, but I'd written all
the boot.c changes before I remembered that M profile doesn't
use that code at all :-)

thanks
-- PMM

Peter Maydell (3):
  loader: Add new load_ramdisk_as()
  hw/arm/boot: Honour CPU's address space for image loads
  hw/arm/armv7m: Honour CPU's address space for image loads

 include/hw/loader.h |  12 +-
 hw/arm/armv7m.c |  17 ++--
 hw/arm/boot.c   | 119 +---
 hw/core/loader.c|   8 +++-
 4 files changed, 108 insertions(+), 48 deletions(-)

-- 
2.16.1

[Qemu-devel] [PATCH 3/3] hw/arm/armv7m: Honour CPU's address space for image loads

[Peter Maydell]

Instead of loading guest images to the system address space, use the
CPU's address space.  This is important if we're trying to load the
file to memory or via an alias memory region that is provided by an
SoC object and thus not mapped into the system address space.

Signed-off-by: Peter Maydell 
---
 hw/arm/armv7m.c | 17 ++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/hw/arm/armv7m.c b/hw/arm/armv7m.c
index 56770a7048..facc536b07 100644
--- a/hw/arm/armv7m.c
+++ b/hw/arm/armv7m.c
@@ -270,6 +270,9 @@ void armv7m_load_kernel(ARMCPU *cpu, const char 
*kernel_filename, int mem_size)
 uint64_t entry;
 uint64_t lowaddr;
 int big_endian;
+AddressSpace *as;
+int asidx;
+CPUState *cs = CPU(cpu);
 
 #ifdef TARGET_WORDS_BIGENDIAN
 big_endian = 1;
@@ -282,11 +285,19 @@ void armv7m_load_kernel(ARMCPU *cpu, const char 
*kernel_filename, int mem_size)
 exit(1);
 }
 
+if (arm_feature(>env, ARM_FEATURE_EL3)) {
+asidx = ARMASIdx_S;
+} else {
+asidx = ARMASIdx_NS;
+}
+as = cpu_get_address_space(cs, asidx);
+
 if (kernel_filename) {
-image_size = load_elf(kernel_filename, NULL, NULL, , ,
-  NULL, big_endian, EM_ARM, 1, 0);
+image_size = load_elf_as(kernel_filename, NULL, NULL, , ,
+ NULL, big_endian, EM_ARM, 1, 0, as);
 if (image_size < 0) {
-image_size = load_image_targphys(kernel_filename, 0, mem_size);
+image_size = load_image_targphys_as(kernel_filename, 0,
+mem_size, as);
 lowaddr = 0;
 }
 if (image_size < 0) {
-- 
2.16.1

[Qemu-devel] [PATCH 2/3] hw/arm/boot: Honour CPU's address space for image loads

[Peter Maydell]

Instead of loading kernels, device trees, and the like to
the system address space, use the CPU's address space. This
is important if we're trying to load the file to memory or
via an alias memory region that is provided by an SoC
object and thus not mapped into the system address space.

Signed-off-by: Peter Maydell 
---
 hw/arm/boot.c | 119 +-
 1 file changed, 76 insertions(+), 43 deletions(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index 9b174b982c..0eac655c98 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -35,6 +35,25 @@
 #define ARM64_TEXT_OFFSET_OFFSET8
 #define ARM64_MAGIC_OFFSET  56
 
+static AddressSpace *arm_boot_addressspace(ARMCPU *cpu,
+   const struct arm_boot_info *info)
+{
+/* Return the address space to use for bootloader reads and writes.
+ * We prefer the secure address space if the CPU has it and we're
+ * going to boot the guest into it.
+ */
+int asidx;
+CPUState *cs = CPU(cpu);
+
+if (arm_feature(>env, ARM_FEATURE_EL3) && info->secure_boot) {
+asidx = ARMASIdx_S;
+} else {
+asidx = ARMASIdx_NS;
+}
+
+return cpu_get_address_space(cs, asidx);
+}
+
 typedef enum {
 FIXUP_NONE = 0, /* do nothing */
 FIXUP_TERMINATOR,   /* end of insns */
@@ -124,7 +143,8 @@ static const ARMInsnFixup smpboot[] = {
 };
 
 static void write_bootloader(const char *name, hwaddr addr,
- const ARMInsnFixup *insns, uint32_t *fixupcontext)
+ const ARMInsnFixup *insns, uint32_t *fixupcontext,
+ AddressSpace *as)
 {
 /* Fix up the specified bootloader fragment and write it into
  * guest memory using rom_add_blob_fixed(). fixupcontext is
@@ -163,7 +183,7 @@ static void write_bootloader(const char *name, hwaddr addr,
 code[i] = tswap32(insn);
 }
 
-rom_add_blob_fixed(name, code, len * sizeof(uint32_t), addr);
+rom_add_blob_fixed_as(name, code, len * sizeof(uint32_t), addr, as);
 
 g_free(code);
 }
@@ -172,6 +192,7 @@ static void default_write_secondary(ARMCPU *cpu,
 const struct arm_boot_info *info)
 {
 uint32_t fixupcontext[FIXUP_MAX];
+AddressSpace *as = CPU(cpu)->as;
 
 fixupcontext[FIXUP_GIC_CPU_IF] = info->gic_cpu_if_addr;
 fixupcontext[FIXUP_BOOTREG] = info->smp_bootreg_addr;
@@ -182,13 +203,14 @@ static void default_write_secondary(ARMCPU *cpu,
 }
 
 write_bootloader("smpboot", info->smp_loader_start,
- smpboot, fixupcontext);
+ smpboot, fixupcontext, as);
 }
 
 void arm_write_secure_board_setup_dummy_smc(ARMCPU *cpu,
 const struct arm_boot_info *info,
 hwaddr mvbar_addr)
 {
+AddressSpace *as = CPU(cpu)->as;
 int n;
 uint32_t mvbar_blob[] = {
 /* mvbar_addr: secure monitor vectors
@@ -226,22 +248,23 @@ void arm_write_secure_board_setup_dummy_smc(ARMCPU *cpu,
 for (n = 0; n < ARRAY_SIZE(mvbar_blob); n++) {
 mvbar_blob[n] = tswap32(mvbar_blob[n]);
 }
-rom_add_blob_fixed("board-setup-mvbar", mvbar_blob, sizeof(mvbar_blob),
-   mvbar_addr);
+rom_add_blob_fixed_as("board-setup-mvbar", mvbar_blob, sizeof(mvbar_blob),
+  mvbar_addr, as);
 
 for (n = 0; n < ARRAY_SIZE(board_setup_blob); n++) {
 board_setup_blob[n] = tswap32(board_setup_blob[n]);
 }
-rom_add_blob_fixed("board-setup", board_setup_blob,
-   sizeof(board_setup_blob), info->board_setup_addr);
+rom_add_blob_fixed_as("board-setup", board_setup_blob,
+  sizeof(board_setup_blob), info->board_setup_addr, 
as);
 }
 
 static void default_reset_secondary(ARMCPU *cpu,
 const struct arm_boot_info *info)
 {
+AddressSpace *as = arm_boot_addressspace(cpu, info);
 CPUState *cs = CPU(cpu);
 
-address_space_stl_notdirty(_space_memory, info->smp_bootreg_addr,
+address_space_stl_notdirty(as, info->smp_bootreg_addr,
0, MEMTXATTRS_UNSPECIFIED, NULL);
 cpu_set_pc(cs, info->smp_loader_start);
 }
@@ -252,12 +275,12 @@ static inline bool have_dtb(const struct arm_boot_info 
*info)
 }
 
 #define WRITE_WORD(p, value) do { \
-address_space_stl_notdirty(_space_memory, p, value, \
+address_space_stl_notdirty(as, p, value, \
MEMTXATTRS_UNSPECIFIED, NULL);  \
 p += 4;   \
 } while (0)
 
-static void set_kernel_args(const struct arm_boot_info *info)
+static void set_kernel_args(const struct arm_boot_info *info, AddressSpace *as)
 {
 int initrd_size = info->initrd_size;
 hwaddr base = info->loader_start;
@@ -288,8 +311,9 @@ static void set_kernel_args(const struct 

[Qemu-devel] [PULL 7/7] allow to build with older sed

[Daniel P . Berrangé]

From: Jan Beulich 

sed's -E option may not be supported by older distros. As there's no
point using sed here at all, use just shell mechanisms to establish the
variable values, starting from the stem instead of the full target.

Signed-off-by: Jan Beulich 
Signed-off-by: Daniel P. Berrange 
---
 Makefile | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index b5a6d602b2..90e05ac409 100644
--- a/Makefile
+++ b/Makefile
@@ -256,8 +256,7 @@ GENERATED_FILES += $(KEYCODEMAP_FILES)
 
 ui/input-keymap-%.c: $(KEYCODEMAP_GEN) $(KEYCODEMAP_CSV) 
$(SRC_PATH)/ui/Makefile.objs
$(call quiet-command,\
-   src=$$(echo $@ | sed -E -e 
"s,^ui/input-keymap-(.+)-to-(.+)\.c$$,\1,") && \
-   dst=$$(echo $@ | sed -E -e 
"s,^ui/input-keymap-(.+)-to-(.+)\.c$$,\2,") && \
+   stem=$* && src=$${stem%-to-*} dst=$${stem#*-to-} && \
test -e $(KEYCODEMAP_GEN) && \
$(PYTHON) $(KEYCODEMAP_GEN) \
  --lang glib2 \
-- 
2.14.3

[Qemu-devel] [PATCH 1/3] loader: Add new load_ramdisk_as()

[Peter Maydell]

Add a function load_ramdisk_as() which behaves like the existing
load_ramdisk() but allows the caller to specify the AddressSpace
to use. This matches the pattern we have already for various
other loader functions.

Signed-off-by: Peter Maydell 
---
 include/hw/loader.h | 12 +++-
 hw/core/loader.c|  8 +++-
 2 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/include/hw/loader.h b/include/hw/loader.h
index 355fe0f5a2..1fd4256782 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -163,16 +163,26 @@ int load_uimage(const char *filename, hwaddr *ep,
 void *translate_opaque);
 
 /**
- * load_ramdisk:
+ * load_ramdisk_as:
  * @filename: Path to the ramdisk image
  * @addr: Memory address to load the ramdisk to
  * @max_sz: Maximum allowed ramdisk size (for non-u-boot ramdisks)
+ * @as: The AddressSpace to load the ELF to. The value of address_space_memory
+ *  is used if nothing is supplied here.
  *
  * Load a ramdisk image with U-Boot header to the specified memory
  * address.
  *
  * Returns the size of the loaded image on success, -1 otherwise.
  */
+int load_ramdisk_as(const char *filename, hwaddr addr, uint64_t max_sz,
+AddressSpace *as);
+
+/**
+ * load_ramdisk:
+ * Same as load_ramdisk_as(), but doesn't allow the caller to specify
+ * an AddressSpace.
+ */
 int load_ramdisk(const char *filename, hwaddr addr, uint64_t max_sz);
 
 ssize_t gunzip(void *dst, size_t dstlen, uint8_t *src, size_t srclen);
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 91669d65aa..2b9e7394a1 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -729,9 +729,15 @@ int load_uimage_as(const char *filename, hwaddr *ep, 
hwaddr *loadaddr,
 
 /* Load a ramdisk.  */
 int load_ramdisk(const char *filename, hwaddr addr, uint64_t max_sz)
+{
+return load_ramdisk_as(filename, addr, max_sz, NULL);
+}
+
+int load_ramdisk_as(const char *filename, hwaddr addr, uint64_t max_sz,
+AddressSpace *as)
 {
 return load_uboot_image(filename, NULL, , NULL, IH_TYPE_RAMDISK,
-NULL, NULL, NULL);
+NULL, NULL, as);
 }
 
 /* Load a gzip-compressed kernel to a dynamically allocated buffer. */
-- 
2.16.1

[Qemu-devel] [PULL 6/7] io/channel-command: Do not kill the child process after closing the pipe

[Daniel P . Berrangé]

From: Thomas Huth 

We are currently facing some migration failure on s390x when running
certain avocado-vt tests, e.g. when running the test
type_specific.io-github-autotest-qemu.migrate.with_reboot.exec.gzip_exec.
This test is using 'migrate -d "exec:nc localhost 5200"' for the migration.
The problem is detected at the receiving side, where the migration stream
apparently ends too early. However, the cause for the problem is at the
sending side: After writing the migration stream into the pipe to netcat,
the source QEMU calls qio_channel_command_close() which closes the pipe
and immediately (!) kills the child process afterwards (via the function
qio_channel_command_abort()). So if the  sending netcat did not read the
final bytes from the pipe yet, or  if it did not manage to send out all
its buffers yet, it is killed before the whole migration stream is passed
to the destination side.

QEMU can not know how much time is required by the child process to send
over all migration data, so we should not kill it, neither directly nor
after a delay. Let's simply wait for the child process to exit gracefully
instead (this was also the behaviour of pclose() that was used in "exec:"
migration before the QIOChannel rework).

Signed-off-by: Thomas Huth 
Signed-off-by: Daniel P. Berrangé 
---
 io/channel-command.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/io/channel-command.c b/io/channel-command.c
index 319c5ed50c..3e7eb17eff 100644
--- a/io/channel-command.c
+++ b/io/channel-command.c
@@ -301,6 +301,9 @@ static int qio_channel_command_close(QIOChannel *ioc,
 {
 QIOChannelCommand *cioc = QIO_CHANNEL_COMMAND(ioc);
 int rv = 0;
+#ifndef WIN32
+pid_t wp;
+#endif
 
 /* We close FDs before killing, because that
  * gives a better chance of clean shutdown
@@ -315,11 +318,18 @@ static int qio_channel_command_close(QIOChannel *ioc,
 rv = -1;
 }
 cioc->writefd = cioc->readfd = -1;
+
 #ifndef WIN32
-if (qio_channel_command_abort(cioc, errp) < 0) {
+do {
+wp = waitpid(cioc->pid, NULL, 0);
+} while (wp == (pid_t)-1 && errno == EINTR);
+if (wp == (pid_t)-1) {
+error_setg_errno(errp, errno, "Failed to wait for pid %llu",
+ (unsigned long long)cioc->pid);
 return -1;
 }
 #endif
+
 if (rv < 0) {
 error_setg_errno(errp, errno, "%s",
  "Unable to close command");
-- 
2.14.3

[Qemu-devel] [PULL 3/7] io: Fix QIOChannelFile when creating and opening read-write

[Daniel P . Berrangé]

From: Ross Lagerwall 

The code wrongly passes the mode to open() only if O_WRONLY is set.
Instead, the mode should be passed when O_CREAT is set (or O_TMPFILE on
Linux). Fix this by always passing the mode since open() will correctly
ignore the mode if it is not needed. Add a testcase which exercises this
bug and also change the existing testcase to check that the mode of the
created file is correct.

Signed-off-by: Ross Lagerwall 
Signed-off-by: Daniel P. Berrange 
---
 include/io/channel-file.h|  2 +-
 io/channel-file.c|  6 +-
 tests/test-io-channel-file.c | 29 +
 3 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/include/io/channel-file.h b/include/io/channel-file.h
index 79245f1183..ebfe54ec70 100644
--- a/include/io/channel-file.h
+++ b/include/io/channel-file.h
@@ -73,7 +73,7 @@ qio_channel_file_new_fd(int fd);
  * qio_channel_file_new_path:
  * @path: the file path
  * @flags: the open flags (O_RDONLY|O_WRONLY|O_RDWR, etc)
- * @mode: the file creation mode if O_WRONLY is set in @flags
+ * @mode: the file creation mode if O_CREAT is set in @flags
  * @errp: pointer to initialized error object
  *
  * Create a new IO channel object for a file represented
diff --git a/io/channel-file.c b/io/channel-file.c
index b383273201..16bf7ed270 100644
--- a/io/channel-file.c
+++ b/io/channel-file.c
@@ -50,11 +50,7 @@ qio_channel_file_new_path(const char *path,
 
 ioc = QIO_CHANNEL_FILE(object_new(TYPE_QIO_CHANNEL_FILE));
 
-if (flags & O_WRONLY) {
-ioc->fd = open(path, flags, mode);
-} else {
-ioc->fd = open(path, flags);
-}
+ioc->fd = open(path, flags, mode);
 if (ioc->fd < 0) {
 object_unref(OBJECT(ioc));
 error_setg_errno(errp, errno,
diff --git a/tests/test-io-channel-file.c b/tests/test-io-channel-file.c
index 6bfede6bb7..2e94f638f2 100644
--- a/tests/test-io-channel-file.c
+++ b/tests/test-io-channel-file.c
@@ -24,16 +24,21 @@
 #include "io-channel-helpers.h"
 #include "qapi/error.h"
 
-static void test_io_channel_file(void)
+#define TEST_FILE "tests/test-io-channel-file.txt"
+#define TEST_MASK 0600
+
+static void test_io_channel_file_helper(int flags)
 {
 QIOChannel *src, *dst;
 QIOChannelTest *test;
+struct stat st;
+mode_t mask;
+int ret;
 
-#define TEST_FILE "tests/test-io-channel-file.txt"
 unlink(TEST_FILE);
 src = QIO_CHANNEL(qio_channel_file_new_path(
   TEST_FILE,
-  O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0600,
+  flags, TEST_MASK,
   _abort));
 dst = QIO_CHANNEL(qio_channel_file_new_path(
   TEST_FILE,
@@ -45,18 +50,33 @@ static void test_io_channel_file(void)
 qio_channel_test_run_reader(test, dst);
 qio_channel_test_validate(test);
 
+/* Check that the requested mode took effect. */
+mask = umask(0);
+umask(mask);
+ret = stat(TEST_FILE, );
+g_assert_cmpint(ret, >, -1);
+g_assert_cmpuint(TEST_MASK & ~mask, ==, st.st_mode & 0777);
+
 unlink(TEST_FILE);
 object_unref(OBJECT(src));
 object_unref(OBJECT(dst));
 }
 
+static void test_io_channel_file(void)
+{
+test_io_channel_file_helper(O_WRONLY | O_CREAT | O_TRUNC | O_BINARY);
+}
+
+static void test_io_channel_file_rdwr(void)
+{
+test_io_channel_file_helper(O_RDWR | O_CREAT | O_TRUNC | O_BINARY);
+}
 
 static void test_io_channel_fd(void)
 {
 QIOChannel *ioc;
 int fd = -1;
 
-#define TEST_FILE "tests/test-io-channel-file.txt"
 fd = open(TEST_FILE, O_CREAT | O_TRUNC | O_WRONLY, 0600);
 g_assert_cmpint(fd, >, -1);
 
@@ -114,6 +134,7 @@ int main(int argc, char **argv)
 g_test_init(, , NULL);
 
 g_test_add_func("/io/channel/file", test_io_channel_file);
+g_test_add_func("/io/channel/file/rdwr", test_io_channel_file_rdwr);
 g_test_add_func("/io/channel/file/fd", test_io_channel_fd);
 #ifndef _WIN32
 g_test_add_func("/io/channel/pipe/sync", test_io_channel_pipe_sync);
-- 
2.14.3

[Qemu-devel] [PULL 0/7] Qio next patches

[Daniel P . Berrangé]

The following changes since commit 8c5e7bddc22dac9d4dc3526996babce4c7242d9d:

  Merge remote-tracking branch 'remotes/huth/tags/pull-request-2018-02-14' into 
staging (2018-02-15 13:00:44 +)

are available in the Git repository at:

  ssh://g...@github.com/berrange/qemu tags/qio-next-pull-request

for you to fetch changes up to 6809df1df036840d41a0cc9ca77cc6a0214fb1b5:

  allow to build with older sed (2018-02-15 16:54:57 +)





Edgar Kaziakhmedov (1):
  io/channel-websock: handle continuous reads without any data

Jan Beulich (1):
  allow to build with older sed

Paolo Bonzini (1):
  io: fix QIONetListener memory leak

Ross Lagerwall (3):
  io: Fix QIOChannelFile when creating and opening read-write
  io: Don't call close multiple times in QIOChannelFile
  io: Add /dev/fdset/ support to QIOChannelFile

Thomas Huth (1):
  io/channel-command: Do not kill the child process after closing the
pipe

 Makefile |  3 +--
 include/io/channel-file.h|  2 +-
 io/channel-command.c | 12 +++-
 io/channel-file.c| 11 ---
 io/channel-websock.c |  7 +--
 io/net-listener.c|  1 +
 tests/test-io-channel-file.c | 29 +
 7 files changed, 48 insertions(+), 17 deletions(-)

-- 
2.14.3

[Qemu-devel] [PULL 5/7] io: Add /dev/fdset/ support to QIOChannelFile

[Daniel P . Berrangé]

From: Ross Lagerwall 

Add /dev/fdset/ support to QIOChannelFile by calling qemu_open() instead
of open() and qemu_close() instead of close(). There is a subtle
semantic change since qemu_open() automatically sets O_CLOEXEC, but this
doesn't affect any of the users of the function.

Signed-off-by: Ross Lagerwall 
Signed-off-by: Daniel P. Berrange 
---
 io/channel-file.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/io/channel-file.c b/io/channel-file.c
index 1f2f710bf9..db948abc3e 100644
--- a/io/channel-file.c
+++ b/io/channel-file.c
@@ -50,7 +50,7 @@ qio_channel_file_new_path(const char *path,
 
 ioc = QIO_CHANNEL_FILE(object_new(TYPE_QIO_CHANNEL_FILE));
 
-ioc->fd = open(path, flags, mode);
+ioc->fd = qemu_open(path, flags, mode);
 if (ioc->fd < 0) {
 object_unref(OBJECT(ioc));
 error_setg_errno(errp, errno,
@@ -74,7 +74,7 @@ static void qio_channel_file_finalize(Object *obj)
 {
 QIOChannelFile *ioc = QIO_CHANNEL_FILE(obj);
 if (ioc->fd != -1) {
-close(ioc->fd);
+qemu_close(ioc->fd);
 ioc->fd = -1;
 }
 }
@@ -173,7 +173,7 @@ static int qio_channel_file_close(QIOChannel *ioc,
 {
 QIOChannelFile *fioc = QIO_CHANNEL_FILE(ioc);
 
-if (close(fioc->fd) < 0) {
+if (qemu_close(fioc->fd) < 0) {
 error_setg_errno(errp, errno,
  "Unable to close file");
 return -1;
-- 
2.14.3

[Qemu-devel] [PULL 4/7] io: Don't call close multiple times in QIOChannelFile

[Daniel P . Berrangé]

From: Ross Lagerwall 

If the file descriptor underlying QIOChannelFile is closed in the
io_close() method, don't close it again in the finalize() method since
the file descriptor number may have been reused in the meantime.

Signed-off-by: Ross Lagerwall 
Signed-off-by: Daniel P. Berrange 
---
 io/channel-file.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/io/channel-file.c b/io/channel-file.c
index 16bf7ed270..1f2f710bf9 100644
--- a/io/channel-file.c
+++ b/io/channel-file.c
@@ -178,6 +178,7 @@ static int qio_channel_file_close(QIOChannel *ioc,
  "Unable to close file");
 return -1;
 }
+fioc->fd = -1;
 return 0;
 }
 
-- 
2.14.3

[Qemu-devel] [PULL 1/7] io: fix QIONetListener memory leak

[Daniel P . Berrangé]

From: Paolo Bonzini 

The sources array does not escape out of qio_net_listener_wait_client, so
we have to free it.

Reported by Coverity.

Signed-off-by: Paolo Bonzini 
Signed-off-by: Daniel P. Berrange 
---
 io/net-listener.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/io/net-listener.c b/io/net-listener.c
index 77a4e2831c..de38dfae99 100644
--- a/io/net-listener.c
+++ b/io/net-listener.c
@@ -234,6 +234,7 @@ QIOChannelSocket 
*qio_net_listener_wait_client(QIONetListener *listener)
 for (i = 0; i < listener->nsioc; i++) {
 g_source_unref(sources[i]);
 }
+g_free(sources);
 g_main_loop_unref(loop);
 g_main_context_unref(ctxt);
 
-- 
2.14.3

[Qemu-devel] [PULL 2/7] io/channel-websock: handle continuous reads without any data

[Daniel P . Berrangé]

From: Edgar Kaziakhmedov 

According to the current implementation of websocket protocol in QEMU,
qio_channel_websock_handshake_io tries to read handshake from the
channel to start communication over socket. But this approach
doesn't cover scenario when socket was closed while handshaking.
Therefore, if G_IO_IN is caught and qio_channel_read returns zero,
error has to be set and connection has to be done.

Such behaviour causes 100% CPU load in main QEMU loop, because main loop
poll continues to receive and handle G_IO_IN events from websocket.

Step to reproduce 100% CPU load:
1) start qemu with the simplest configuration
$ qemu -vnc [::1]:1,websocket=7500
2) open any vnc listener (which doesn't follow websocket
protocol)
$ vncviewer :7500
3) kill listener
4) qemu main thread eats 100% CPU

Signed-off-by: Edgar Kaziakhmedov 
Signed-off-by: Daniel P. Berrange 
---
 io/channel-websock.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/io/channel-websock.c b/io/channel-websock.c
index 7fd6bb68ba..ec48a305f0 100644
--- a/io/channel-websock.c
+++ b/io/channel-websock.c
@@ -499,9 +499,12 @@ static int 
qio_channel_websock_handshake_read(QIOChannelWebsock *ioc,
 error_setg(errp,
"End of headers not found in first 4096 bytes");
 return 1;
-} else {
-return 0;
+} else if (ret == 0) {
+error_setg(errp,
+   "End of headers not found before connection closed");
+return -1;
 }
+return 0;
 }
 *handshake_end = '\0';
 
-- 
2.14.3

Re: [Qemu-devel] [PATCH 02/11] linux-user/strace: improve sendto() output

[Laurent Vivier]

Le 24/01/2018 à 14:01, Philippe Mathieu-Daudé a écrit :
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  linux-user/strace.c| 16 
>  linux-user/strace.list |  2 +-
>  2 files changed, 17 insertions(+), 1 deletion(-)
> 
> diff --git a/linux-user/strace.c b/linux-user/strace.c
> index 7eb5e2ab48..e7272f4ede 100644
> --- a/linux-user/strace.c
> +++ b/linux-user/strace.c
> @@ -1922,6 +1922,22 @@ print_socketcall(const struct syscallname *name,
>  }
>  #endif
>  
> +#if defined(TARGET_NR_sendto)
> +static void
> +print_sendto(const struct syscallname *name,
> + abi_long arg0, abi_long arg1, abi_long arg2,
> + abi_long arg3, abi_long arg4, abi_long arg5)
> +{
> +print_syscall_prologue(name);
> +print_raw_param(TARGET_ABI_FMT_ld, arg0, 0);

Other strace functions (accept(), fcntl(), ...) use "%d" for fd.

Thanks,
Laurent

Re: [Qemu-devel] [PATCH] docker: dump 'config.log' if ./configure fails

[Philippe Mathieu-Daudé]

On 02/15/2018 02:34 PM, Daniel P. Berrangé wrote:
> On Thu, Feb 15, 2018 at 02:23:06PM -0300, Philippe Mathieu-Daudé wrote:
>> Suggested-by: Eric Blake 
>> Signed-off-by: Philippe Mathieu-Daudé 
>> ---
>>  tests/docker/common.rc | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/docker/common.rc b/tests/docker/common.rc
>> index 7951555e3f..dd011cdf1e 100755
>> --- a/tests/docker/common.rc
>> +++ b/tests/docker/common.rc
>> @@ -30,7 +30,9 @@ build_qemu()
>>   $@"
>>  echo "Configure options:"
>>  echo $config_opts
>> -$QEMU_SRC/configure $config_opts && make $MAKEFLAGS
>> +$QEMU_SRC/configure $config_opts || \
>> +(cat config.log && prep_fail "Failed to run 'configure'")
>> +make $MAKEFLAGS
>>  }
> 
> Reviewed-by: Daniel P. Berrangé 
> 
> Slightly nicer than my patch thanks to the prep_fail addition.

Haha this was a funny experience of parallel programming :)

However your patch has a slightly nicer description!

> 
> 
> Regards,
> Daniel
>