[Qemu-devel] [PATCH 3/9] dma-helpers: Do not truncate small qiovs

[Alex Pyrgiotis]

If the size of the qiov is smaller than the sector size, do not truncate
the qiov, which would effectively make it empty. Instead, allow it to
pass as is.

This is necessary for SCSI requests like READ CAPACITY which have small
buffers, e.g. 32 bytes.

Signed-off-by: Alex Pyrgiotis 
Signed-off-by: Dimitris Aragiorgis 

diff --git a/dma-helpers.c b/dma-helpers.c
index e1ea7b3..b8f2ae0 100644
--- a/dma-helpers.c
+++ b/dma-helpers.c
@@ -162,7 +162,16 @@ static void dma_map_sg(DMAAIOCB *dbs)
 return;
 }
 
-if (dbs->iov.size & ~BDRV_SECTOR_MASK) {
+/*
+ * If the size of the qiov is not a multiple of the sector size, truncate
+ * the qiov.
+ *
+ * NOTE: If the qiov is less than a sector, we can assume that there is a
+ * reason for it, e.g., a SCSI request such as READ CAPACITY, and we should
+ * not truncate it.
+ */
+if (dbs->iov.size & ~BDRV_SECTOR_MASK &&
+dbs->iov.size > BDRV_SECTOR_SIZE) {
 qemu_iovec_discard_back(>iov, dbs->iov.size & ~BDRV_SECTOR_MASK);
 }
 }
-- 
2.6.2

[Qemu-devel] [PATCH] os-posix: Log to logfile in case of daemonize

[Alex Pyrgiotis]

From: Dimitris Aragiorgis 

In case of daemonize, use the logfile passed with the -D option in
order to redirect stdout/stderr to a file instead of /dev/null.

Signed-off-by: Dimitris Aragiorgis 
Signed-off-by: Alex Pyrgiotis 

diff --git a/include/qemu/log.h b/include/qemu/log.h
index 362cbc4..b5de1df 100644
--- a/include/qemu/log.h
+++ b/include/qemu/log.h
@@ -175,6 +175,7 @@ static inline void qemu_set_log(int log_flags)
 }
 
 void qemu_set_log_filename(const char *filename);
+char *qemu_get_log_filename(void);
 int qemu_str_to_log_mask(const char *str);
 
 /* Print a usage message listing all the valid logging categories
diff --git a/os-posix.c b/os-posix.c
index e4da406..18052fb 100644
--- a/os-posix.c
+++ b/os-posix.c
@@ -40,6 +40,7 @@
 #include "net/slirp.h"
 #include "qemu-options.h"
 #include "qemu/rcu.h"
+#include "qemu/log.h"
 
 #ifdef CONFIG_LINUX
 #include 
@@ -255,16 +256,25 @@ void os_daemonize(void)
 void os_setup_post(void)
 {
 int fd = 0;
+char *log = NULL;
 
 if (daemonize) {
 if (chdir("/")) {
 perror("not able to chdir to /");
 exit(1);
 }
-TFR(fd = qemu_open("/dev/null", O_RDWR));
+
+log = qemu_get_log_filename();
+if (log != NULL) {
+TFR(fd = qemu_open(log, O_RDWR | O_APPEND | O_CREAT, 0640));
+} else {
+TFR(fd = qemu_open("/dev/null", O_RDWR));
+}
 if (fd == -1) {
+fprintf(stderr, "Cannot open \"%s\" for logging\n", log);
 exit(1);
 }
+g_free(log);
 }
 
 change_root();
diff --git a/qemu-log.c b/qemu-log.c
index 7cb01a8..7bc 100644
--- a/qemu-log.c
+++ b/qemu-log.c
@@ -90,6 +90,11 @@ void qemu_set_log_filename(const char *filename)
 qemu_set_log(qemu_loglevel);
 }
 
+char *qemu_get_log_filename(void)
+{
+return g_strdup(logfilename);
+}
+
 const QEMULogItem qemu_log_items[] = {
 { CPU_LOG_TB_OUT_ASM, "out_asm",
   "show generated host assembly code for each compiled TB" },
-- 
2.6.2

[Qemu-devel] [PATCH v2 3/3] hw/sd: use guest error logging rather than fprintf to stderr

[Andrew Baumann]

Some of these errors may be harmless (e.g. probing unimplemented
commands, or issuing CMD12 in the wrong state), and may also be quite
frequent. Spamming the standard error output isn't desirable in such
cases.

Signed-off-by: Andrew Baumann 
---
It might also be desirable to have a squelch mechanism for these
messages, but at least for my use-case, this is sufficient, since they
only occur during boot time.

 hw/sd/sd.c | 22 +-
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 1011785..9b76b47 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1234,16 +1234,18 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
 
 default:
 bad_cmd:
-fprintf(stderr, "SD: Unknown CMD%i\n", req.cmd);
+qemu_log_mask(LOG_GUEST_ERROR, "SD: Unknown CMD%i\n", req.cmd);
 return sd_illegal;
 
 unimplemented_cmd:
 /* Commands that are recognised but not yet implemented in SPI mode.  
*/
-fprintf(stderr, "SD: CMD%i not implemented in SPI mode\n", req.cmd);
+qemu_log_mask(LOG_GUEST_ERROR,
+  "SD: CMD%i not implemented in SPI mode\n",
+  req.cmd);
 return sd_illegal;
 }
 
-fprintf(stderr, "SD: CMD%i in a wrong state\n", req.cmd);
+qemu_log_mask(LOG_GUEST_ERROR, "SD: CMD%i in a wrong state\n", req.cmd);
 return sd_illegal;
 }
 
@@ -1375,7 +1377,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
 return sd_normal_command(sd, req);
 }
 
-fprintf(stderr, "SD: ACMD%i in a wrong state\n", req.cmd);
+qemu_log_mask(LOG_GUEST_ERROR, "SD: ACMD%i in a wrong state\n", req.cmd);
 return sd_illegal;
 }
 
@@ -1418,7 +1420,7 @@ int sd_do_command(SDState *sd, SDRequest *req,
 if (!cmd_valid_while_locked(sd, req)) {
 sd->card_status |= ILLEGAL_COMMAND;
 sd->expecting_acmd = false;
-fprintf(stderr, "SD: Card is locked\n");
+qemu_log_mask(LOG_GUEST_ERROR, "SD: Card is locked\n");
 rtype = sd_illegal;
 goto send_response;
 }
@@ -1576,7 +1578,8 @@ void sd_write_data(SDState *sd, uint8_t value)
 return;
 
 if (sd->state != sd_receivingdata_state) {
-fprintf(stderr, "sd_write_data: not in Receiving-Data state\n");
+qemu_log_mask(LOG_GUEST_ERROR,
+  "sd_write_data: not in Receiving-Data state\n");
 return;
 }
 
@@ -1695,7 +1698,7 @@ void sd_write_data(SDState *sd, uint8_t value)
 break;
 
 default:
-fprintf(stderr, "sd_write_data: unknown command\n");
+qemu_log_mask(LOG_GUEST_ERROR, "sd_write_data: unknown command\n");
 break;
 }
 }
@@ -1710,7 +1713,8 @@ uint8_t sd_read_data(SDState *sd)
 return 0x00;
 
 if (sd->state != sd_sendingdata_state) {
-fprintf(stderr, "sd_read_data: not in Sending-Data state\n");
+qemu_log_mask(LOG_GUEST_ERROR,
+  "sd_read_data: not in Sending-Data state\n");
 return 0x00;
 }
 
@@ -1821,7 +1825,7 @@ uint8_t sd_read_data(SDState *sd)
 break;
 
 default:
-fprintf(stderr, "sd_read_data: unknown command\n");
+qemu_log_mask(LOG_GUEST_ERROR, "sd_read_data: unknown command\n");
 return 0x00;
 }
 
-- 
2.5.3

[Qemu-devel] [PATCH v2 1/3] hw/sd: implement CMD23 (SET_BLOCK_COUNT) for MMC compatibility

[Andrew Baumann]

CMD23 is optional for SD but required for MMC, and the UEFI bootloader
used for Windows on Raspberry Pi 2 issues it.

Signed-off-by: Andrew Baumann 
---
 hw/sd/sd.c | 37 +
 1 file changed, 37 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 1a9935c..1a24933 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -92,6 +92,7 @@ struct SDState {
 int32_t wpgrps_size;
 uint64_t size;
 uint32_t blk_len;
+uint32_t multi_blk_cnt;
 uint32_t erase_start;
 uint32_t erase_end;
 uint8_t pwd[16];
@@ -423,6 +424,7 @@ static void sd_reset(SDState *sd)
 sd->blk_len = 0x200;
 sd->pwd_len = 0;
 sd->expecting_acmd = false;
+sd->multi_blk_cnt = 0;
 }
 
 static void sd_cardchange(void *opaque, bool load)
@@ -455,6 +457,7 @@ static const VMStateDescription sd_vmstate = {
 VMSTATE_UINT32(vhs, SDState),
 VMSTATE_BITMAP(wp_groups, SDState, 0, wpgrps_size),
 VMSTATE_UINT32(blk_len, SDState),
+VMSTATE_UINT32(multi_blk_cnt, SDState),
 VMSTATE_UINT32(erase_start, SDState),
 VMSTATE_UINT32(erase_end, SDState),
 VMSTATE_UINT8_ARRAY(pwd, SDState, 16),
@@ -671,6 +674,12 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
 if (sd_cmd_type[req.cmd] == sd_ac || sd_cmd_type[req.cmd] == sd_adtc)
 rca = req.arg >> 16;
 
+/* CMD23 (set block count) must be immediately followed by CMD18 or CMD25
+ * if not, its effects are cancelled */
+if (sd->multi_blk_cnt != 0 && !(req.cmd == 18 || req.cmd == 25)) {
+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) */
@@ -966,6 +975,17 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
 }
 break;
 
+case 23:/* CMD23: SET_BLOCK_COUNT */
+switch (sd->state) {
+case sd_transfer_state:
+sd->multi_blk_cnt = req.arg;
+return sd_r1;
+
+default:
+break;
+}
+break;
+
 /* Block write commands (Class 4) */
 case 24:   /* CMD24:  WRITE_SINGLE_BLOCK */
 if (sd->spi)
@@ -1564,6 +1584,14 @@ void sd_write_data(SDState *sd, uint8_t value)
 sd->data_offset = 0;
 sd->csd[14] |= 0x40;
 
+if (sd->multi_blk_cnt != 0) {
+if (--sd->multi_blk_cnt == 0) {
+/* Stop! */
+sd->state = sd_transfer_state;
+break;
+}
+}
+
 /* Bzzztt  Operation complete.  */
 sd->state = sd_receivingdata_state;
 }
@@ -1711,6 +1739,15 @@ uint8_t sd_read_data(SDState *sd)
 if (sd->data_offset >= io_len) {
 sd->data_start += io_len;
 sd->data_offset = 0;
+
+if (sd->multi_blk_cnt != 0) {
+if (--sd->multi_blk_cnt == 0) {
+/* Stop! */
+sd->state = sd_transfer_state;
+break;
+}
+}
+
 if (sd->data_start + io_len > sd->size) {
 sd->card_status |= ADDRESS_ERROR;
 break;
-- 
2.5.3

[Qemu-devel] [PATCH 4/9] scsi-generic: Add common functions

[Alex Pyrgiotis]

In the `scsi_generic_req_ops' struct, instead of pointing to the
implementations of read_data/write_data/send_command, point to wrappers
around these functions, prefixed with "common_".

Also, introduce the concept of "buffer" operations. Buffer operations
are the read/write operations that rely on an intermediate buffer
to do a request. This is the current mode, so we prefix the existing
implementations with "buf_". This paves the way for the introduction of
sg operations, which are operations that rely on scatter-gather lists.

These "common" functions can be used later on as a common entry point
for the read/write operations, in which we can decide whether the device
will use an intermediate buffer for requests or the controller's
scatter-gather list.

Signed-off-by: Alex Pyrgiotis 
Signed-off-by: Dimitris Aragiorgis 

diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index a4626f7..f24f472 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -175,7 +175,7 @@ static int execute_command(BlockBackend *blk,
 return 0;
 }
 
-static void scsi_read_complete(void * opaque, int ret)
+static void scsi_buf_read_complete(void *opaque, int ret)
 {
 SCSIGenericReq *r = (SCSIGenericReq *)opaque;
 SCSIDevice *s = r->req.dev;
@@ -229,13 +229,13 @@ static void scsi_read_complete(void * opaque, int ret)
 }
 
 /* Read more data from scsi device into buffer.  */
-static void scsi_read_data(SCSIRequest *req)
+static void scsi_buf_read_data(SCSIRequest *req)
 {
 SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
 SCSIDevice *s = r->req.dev;
 int ret;
 
-DPRINTF("scsi_read_data 0x%x\n", req->tag);
+DPRINTF("scsi_buf_read_data 0x%x\n", req->tag);
 
 /* The request is used as the AIO opaque value, so add a ref.  */
 scsi_req_ref(>req);
@@ -245,13 +245,18 @@ static void scsi_read_data(SCSIRequest *req)
 }
 
 ret = execute_command(s->conf.blk, r, SG_DXFER_FROM_DEV,
-  scsi_read_complete);
+  scsi_buf_read_complete);
 if (ret < 0) {
 scsi_command_complete_noio(r, ret);
 }
 }
 
-static void scsi_write_complete(void * opaque, int ret)
+static void scsi_common_read_data(SCSIRequest *req)
+{
+scsi_buf_read_data(req);
+}
+
+static void scsi_buf_write_complete(void *opaque, int ret)
 {
 SCSIGenericReq *r = (SCSIGenericReq *)opaque;
 SCSIDevice *s = r->req.dev;
@@ -277,13 +282,13 @@ static void scsi_write_complete(void * opaque, int ret)
 
 /* Write data to a scsi device.  Returns nonzero on failure.
The transfer may complete asynchronously.  */
-static void scsi_write_data(SCSIRequest *req)
+static void scsi_buf_write_data(SCSIRequest *req)
 {
 SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
 SCSIDevice *s = r->req.dev;
 int ret;
 
-DPRINTF("scsi_write_data 0x%x\n", req->tag);
+DPRINTF("scsi_buf_write_data 0x%x\n", req->tag);
 if (r->len == 0) {
 r->len = r->buflen;
 scsi_req_data(>req, r->len);
@@ -292,12 +297,18 @@ static void scsi_write_data(SCSIRequest *req)
 
 /* The request is used as the AIO opaque value, so add a ref.  */
 scsi_req_ref(>req);
-ret = execute_command(s->conf.blk, r, SG_DXFER_TO_DEV, 
scsi_write_complete);
+ret = execute_command(s->conf.blk, r, SG_DXFER_TO_DEV,
+scsi_buf_write_complete);
 if (ret < 0) {
 scsi_command_complete_noio(r, ret);
 }
 }
 
+static void scsi_common_write_data(SCSIRequest *req)
+{
+scsi_buf_write_data(req);
+}
+
 /* Return a pointer to the data buffer.  */
 static uint8_t *scsi_get_buf(SCSIRequest *req)
 {
@@ -311,7 +322,7 @@ static uint8_t *scsi_get_buf(SCSIRequest *req)
(eg. disk reads), negative for transfers to the device (eg. disk writes),
and zero if the command does not transfer any data.  */
 
-static int32_t scsi_send_command(SCSIRequest *req, uint8_t *cmd)
+static int32_t scsi_common_send_command(SCSIRequest *req, uint8_t *cmd)
 {
 SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
 SCSIDevice *s = r->req.dev;
@@ -466,9 +477,9 @@ static void scsi_generic_realize(SCSIDevice *s, Error 
**errp)
 const SCSIReqOps scsi_generic_req_ops = {
 .size = sizeof(SCSIGenericReq),
 .free_req = scsi_free_request,
-.send_command = scsi_send_command,
-.read_data= scsi_read_data,
-.write_data   = scsi_write_data,
+.send_command = scsi_common_send_command,
+.read_data= scsi_common_read_data,
+.write_data   = scsi_common_write_data,
 .get_buf  = scsi_get_buf,
 .load_request = scsi_generic_load_request,
 .save_request = scsi_generic_save_request,
-- 
2.6.2

[Qemu-devel] [PATCH v2 2/3] hw/sd: model a power-up delay, as a workaround for an EDK2 bug

[Andrew Baumann]

The SD spec for ACMD41 says that a zero argument is an "inquiry"
ACMD41, which does not start initialisation and is used only for
retrieving the OCR. However, Tianocore EDK2 (UEFI) has a bug [1]: it
first sends an inquiry (zero) ACMD41. If that first request returns an
OCR value with the power up bit (0x8000) set, it assumes the card
is ready and continues, leaving the card in the wrong state. (My
assumption is that this works on hardware, because no real card is
immediately powered up upon reset.)

This change models a delay of 0.5ms from the first ACMD41 to the power
being up. However, it also immediately sets the power on upon seeing a
non-zero (non-enquiry) ACMD41. This speeds up UEFI boot; it should
also account for guests that simply delay after card reset and then
issue an ACMD41 that they expect will succeed.

[1] 
https://github.com/tianocore/edk2/blob/master/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c#L279
(This is the loop starting with "We need to wait for the MMC or SD
card is ready")

Signed-off-by: Andrew Baumann 
---
Obviously this is a bug that should be fixed in EDK2. However, this
initialisation appears to have been around for quite a while in EDK2
(in various forms), and the fact that it has obviously worked with so
many real SD/MMC cards makes me think that it would be pragmatic to
have the workaround in QEMU as well.

You might argue that the delay timer should start on sd_reset(), and
not the first ACMD41. However, that doesn't work reliably with UEFI,
because a large delay often elapses between the two (particularly in
debug builds that do lots of printing to the serial port). If the
timer fires too early, we'll still hit the bug, but we also don't want
to set a huge timeout value, because some guests may depend on it
expiring.

 hw/sd/sd.c | 45 -
 1 file changed, 40 insertions(+), 5 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 1a24933..1011785 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -33,6 +33,7 @@
 #include "sysemu/block-backend.h"
 #include "hw/sd/sd.h"
 #include "qemu/bitmap.h"
+#include "qemu/timer.h"
 
 //#define DEBUG_SD 1
 
@@ -43,7 +44,9 @@ do { fprintf(stderr, "SD: " fmt , ## __VA_ARGS__); } while (0)
 #define DPRINTF(fmt, ...) do {} while(0)
 #endif
 
-#define ACMD41_ENQUIRY_MASK 0x00ff
+#define ACMD41_ENQUIRY_MASK 0x00ff
+#define OCR_POWER_UP0x8000
+#define OCR_POWER_DELAY (get_ticks_per_sec() / 2000) /* 0.5ms */
 
 typedef enum {
 sd_r0 = 0,/* no response */
@@ -80,6 +83,7 @@ struct SDState {
 uint32_t mode;/* current card mode, one of SDCardModes */
 int32_t state;/* current card state, one of SDCardStates */
 uint32_t ocr;
+QEMUTimer *ocr_power_timer;
 uint8_t scr[8];
 uint8_t cid[16];
 uint8_t csd[16];
@@ -194,8 +198,17 @@ static uint16_t sd_crc16(void *message, size_t width)
 
 static void sd_set_ocr(SDState *sd)
 {
-/* All voltages OK, card power-up OK, Standard Capacity SD Memory Card */
-sd->ocr = 0x8000;
+/* All voltages OK, Standard Capacity SD Memory Card, not yet powered up */
+sd->ocr = 0x0000;
+}
+
+static void sd_ocr_powerup(void *opaque)
+{
+SDState *sd = opaque;
+
+/* Set powered up bit in OCR */
+assert(!(sd->ocr & OCR_POWER_UP));
+sd->ocr |= OCR_POWER_UP;
 }
 
 static void sd_set_scr(SDState *sd)
@@ -449,6 +462,8 @@ static const VMStateDescription sd_vmstate = {
 .fields = (VMStateField[]) {
 VMSTATE_UINT32(mode, SDState),
 VMSTATE_INT32(state, SDState),
+VMSTATE_UINT32(ocr, SDState),
+VMSTATE_TIMER_PTR(ocr_power_timer, SDState),
 VMSTATE_UINT8_ARRAY(cid, SDState, 16),
 VMSTATE_UINT8_ARRAY(csd, SDState, 16),
 VMSTATE_UINT16(rca, SDState),
@@ -493,6 +508,7 @@ SDState *sd_init(BlockBackend *blk, bool is_spi)
 sd->spi = is_spi;
 sd->enable = true;
 sd->blk = blk;
+sd->ocr_power_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, sd_ocr_powerup, sd);
 sd_reset(sd);
 if (sd->blk) {
 /* Attach dev if not already attached.  (This call ignores an
@@ -1295,12 +1311,31 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
 }
 switch (sd->state) {
 case sd_idle_state:
+/* If it's the first ACMD41 since reset, we need to decide
+ * whether to power up. If this is not an enquiry ACMD41,
+ * we immediately report power on and proceed below to the
+ * ready state, but if it is, we set a timer to model a
+ * delay for power up. This works around a bug in EDK2
+ * 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 ((req.arg & ACMD41_ENQUIRY_MASK) != 0) {
+timer_del(sd->ocr_power_timer);

Re: [Qemu-devel] [PATCH 1/3] sd: sdhci: Delete over-zealous power check

[Andrew Baumann]

> From: Peter Crosthwaite [mailto:crosthwaitepe...@gmail.com]
> Sent: Wednesday, 16 December 2015 12:00
> On Wed, Dec 16, 2015 at 11:47 AM, Andrew Baumann
>  wrote:
> > This is exactly equivalent to Peter's patch here:
> > https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg06271.html
> > ... which is why I've copied the signed-off-by lines (I assume that is
> > legit; if not, my apologies!). It is also needed for Pi.
> >
> 
> The correct process it to apply the patch locally as originally,
> authored then append your SoB and any notes/changes using git commit
> --amend. Git am can apply the patch from an mbox, or the patches
> utility can be used to download and apply patches off the list.

Thanks Peter. I'll do this when I redo the patch series with any other feedback.

Andrew

Re: [Qemu-devel] [RFC PATCH v0 2/9] cpu: Store CPU typename in MachineState

[Igor Mammedov]

On Tue, 15 Dec 2015 14:08:09 +0530
Bharata B Rao  wrote:

> On Mon, Dec 14, 2015 at 03:29:49PM -0200, Eduardo Habkost wrote:
> > On Thu, Dec 10, 2015 at 11:45:37AM +0530, Bharata B Rao wrote:
> > > Storing CPU typename in MachineState lets us to create CPU threads
> > > for all architectures in uniform manner from arch-neutral code.
> > > 
> > > TODO: Touching only i386 and spapr targets for now
> > > 
> > > Signed-off-by: Bharata B Rao 
> > 
> > Suggestions:
> > 
> > * Name the field "cpu_base_type" to indicate it is the base CPU
> >   class name, not the actual CPU class name used when creating
> >   CPUs.
> > * Put it in MachineClass, as it may be useful for code that
> >   runs before machine->init(), in the future.
> 
> Ok.
> 
> > * Maybe make it a CPUClass* field instead of a string?
> 
> In the current use case, this base cpu type string is being passed
> to cpu_generic_init(const char *typename, const char *cpu_model)
> to create boot time CPUs with given typename and cpu_mode. So for now
> the string makes sense for use case.
> 
> Making it CPUClass* would necessiate more changes to
> cpu_generic_init().
how about actually leaving it as "cpu_type" and putting in it
actual cpu type that could be used with device_add().

that would get rid of keeping and passing around intermediate cpu_model.

> 
> Regards,
> Bharata.
> 
> 

Re: [Qemu-devel] [PATCH 02/11] alpha: convert "naked" qemu_log to tracepoint

[Richard Henderson]

On 12/12/2015 02:36 AM, Paolo Bonzini wrote:

Cc: Richard Henderson 
Signed-off-by: Paolo Bonzini 
---
  hw/alpha/pci.c | 3 ++-
  trace-events   | 3 +++
  2 files changed, 5 insertions(+), 1 deletion(-)


Acked-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 1/1] i386: expose floppy-related objects in SSDT

[Roman Kagan]

On Wed, Dec 16, 2015 at 05:46:57PM +0100, Igor Mammedov wrote:
> On Wed, 16 Dec 2015 10:45:09 +0300 "Denis V. Lunev" 
> wrote:
> > @@ -1125,6 +1183,26 @@ build_ssdt(GArray *table_data, GArray *linker,
> >  aml_append(ssdt, scope);
> >  }
> >  
> > +{
> add this block only if floppy controller is present

ATM both qemu master (and stable branches) and your patchset define FDC0
unconditionally, making its _STA depend on FDEN field of the containing
ISA device rather than any sort of qemu internal state.  I think it's
more consistent also to always define its _FDE.  The whole definition of
FDC0 can then be made contidional on the presence of the floppy
controller later on top of your patchset.

As for the subdevices representing floppy drives they already are
defined only if present.

> to minimize conflicts with existing series under review,
> I'd suggest to put it before:
> 
>   if (misc->applesmc_io_base) {

No prob, will do in v2.

Thanks,
Roman.

[Qemu-devel] [PATCH] scsi: use scsi_req_cancel_async when purging requests

[Paolo Bonzini]

This avoids calls to aio_poll without having acquired the context first.

Signed-off-by: Paolo Bonzini 
---
 hw/scsi/scsi-bus.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index fd1171e..524a998 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1843,9 +1843,11 @@ void scsi_device_purge_requests(SCSIDevice *sdev, 
SCSISense sense)
 
 while (!QTAILQ_EMPTY(>requests)) {
 req = QTAILQ_FIRST(>requests);
-scsi_req_cancel(req);
+scsi_req_cancel_async(req, NULL);
 }
-
+aio_context_acquire(bdrv_get_aio_context(sdev->conf.blk));
+blk_drain(sdev->conf.blk);
+aio_context_release(bdrv_get_aio_context(sdev->conf.blk));
 scsi_device_set_ua(sdev, sense);
 }
 
-- 
2.5.0

Re: [Qemu-devel] [PATCH v4 3/5] xilinx_spips: Seperate the state struct into a header

[Alistair Francis]

On Tue, Dec 15, 2015 at 10:48 AM, Peter Maydell
 wrote:
> On 23 November 2015 at 08:54, Alistair Francis
>  wrote:
>> Seperate out the XilinxSPIPS struct into a seperate header
>> file.
>
> If you need to respin this series it would be nice to fix
> the typos here and in the subject: it is "separate".

Fixed

Thanks,

Alistair

>
> thanks
> -- PMM
>

[Qemu-devel] Block I/O Tracing

[Luis Pabón]

Hi all,
  I am really interested in the following feature: 
http://wiki.qemu.org/Features/Block/Todo#Trace_guest_block_I.2FO.2C_replay_with_qemu-io 
.  Is there any more information about this feature?


Thanks,

- Luis

[Qemu-devel] [PATCH 2/9] dma-helpers: Add support for ioctl operations

[Alex Pyrgiotis]

Allow ioctl operations to benefit from the DMA functionality created for
the read/write operations. More specifically, create a function called
"dma_blk_ioctl" that uses the existing code for mapping scatter-gather
lists to qiovs and ultimately calls the blk_aio_ioctl() function to
perform the actual ioctl operation.

Also, in order to use the qiov in the ioctl request, the user
can specify a qiov handler when calling dma_blk_ioctl(). This handler
will be called after the creation of the qiov and before the actual
ioctl request. This allows the user to update the ioctl request with the
iovec stored in the qiov.

Signed-off-by: Alex Pyrgiotis 
Signed-off-by: Dimitris Aragiorgis 

diff --git a/dma-helpers.c b/dma-helpers.c
index c38661e..e1ea7b3 100644
--- a/dma-helpers.c
+++ b/dma-helpers.c
@@ -79,10 +79,14 @@ typedef struct {
 QEMUIOVector iov;
 QEMUBH *bh;
 DMAIOFunc *io_func;
+unsigned int ioctl_req_type;
+void *ioctl_req;
+void (*handle_iov)(void *opaque, QEMUIOVector *iov);
 BlockCompletionFunc *dma_cb;
 } DMAAIOCB;
 
 static void dma_blk_io_cb(void *opaque, int ret);
+static void dma_blk_ioctl_cb(void *opaque, int ret);
 
 static void reschedule_dma(void *opaque)
 {
@@ -196,6 +200,42 @@ static void dma_blk_io_cb(void *opaque, int ret)
 assert(dbs->acb);
 }
 
+/*
+ * Callback function for DMA ioctl operations.
+ *
+ * This function is similar to the dma_blk_io_cb() function. The only addition
+ * is that it checks if a caller has specified a handler resulting iovec, and
+ * calls it before starting the ioctl operation.
+ */
+static void dma_blk_ioctl_cb(void *opaque, int ret)
+{
+DMAAIOCB *dbs = (DMAAIOCB *)opaque;
+
+trace_dma_blk_ioctl_cb(dbs, ret);
+
+dbs->acb = NULL;
+
+if (dbs->sg_cur_index == dbs->sg->nsg || ret < 0) {
+dma_complete(dbs, ret);
+return;
+}
+
+dma_map_sg(dbs);
+
+/*
+ * If the user has specified a handler for the resulting iovec, call it
+ * before starting the ioctl.
+ */
+if (dbs->handle_iov) {
+dbs->handle_iov(dbs->common.opaque, >iov);
+}
+
+dbs->acb = blk_aio_ioctl(dbs->blk, dbs->ioctl_req_type, dbs->ioctl_req,
+dma_blk_ioctl_cb, dbs);
+
+assert(dbs->acb);
+}
+
 static void dma_aio_cancel(BlockAIOCB *acb)
 {
 DMAAIOCB *dbs = container_of(acb, DMAAIOCB, common);
@@ -267,6 +307,54 @@ BlockAIOCB *dma_blk_write(BlockBackend *blk,
   DMA_DIRECTION_TO_DEVICE);
 }
 
+/**
+ * dma_blk_ioctl - perform an ioctl request using a scatter-gather list
+ * @blk: The BlockBackend struct of the underlying device
+ * @ioctl_req_type:  The type of the control function to perform
+ * @ioctl_req:   The payload for the action
+ * @dir: The direction of the DMA request
+ * @iov_cb:  The callback for handling the qiov that results from the
+ *   mapping of the scatter gather lists.
+ *   Expected parameters:
+ *   @ opaque:  The opaque struct that is provided
+ *  with this call (see below)
+ *   @ iov: The QEMUIOVector that is mapped to the
+ *  scatter-gather list
+ * @complete_cb: The completion callback for this request.
+ *   Expected parameters:
+ *   @ opaque:  The opaque struct that is provided
+ *  with this call (see below)
+ *   @ ret: The return value of the ioctl call
+ * @opaque:  Private data of the caller
+ *
+ * Description:
+ * This function calls dma_map_sg, which maps the provided scatter-gather
+ * list to a qiov. Next, the iov_cb handler is called with the `opaque' and
+ * qiov as arguments. Then, the ioctl request is enqueued and the function
+ * returns.  The caller will be notified about the completion of the
+ * request with the complete_cb handler.
+ */
+BlockAIOCB *dma_blk_ioctl(BlockBackend *blk,
+  unsigned long int ioctl_req_type,
+  void *ioctl_req,
+  QEMUSGList *sg,
+  DMADirection dir,
+  void (*iov_cb)(void *opaque, QEMUIOVector *iov),
+  void (*complete_cb)(void *opaque, int ret),
+  void *opaque)
+{
+DMAAIOCB *dbs = blk_aio_get(_aiocb_info, blk, complete_cb, opaque);
+
+trace_dma_blk_ioctl(dbs, blk, (dir == DMA_DIRECTION_TO_DEVICE));
+
+dma_init_dbs(dbs, blk, sg, dir);
+dbs->handle_iov = iov_cb;
+dbs->ioctl_req = ioctl_req;
+dbs->ioctl_req_type = ioctl_req_type;
+dbs->dma_cb = dma_blk_ioctl_cb;
+dbs->dma_cb(dbs, 0);
+return >common;
+}
 
 static uint64_t dma_buf_rw(uint8_t *ptr, int32_t len, QEMUSGList *sg,
DMADirection dir)
diff 

[Qemu-devel] [PATCH 6/9] scsi-generic: Make request execution buf-specific

[Alex Pyrgiotis]

Move the request execution logic from execute_command() to
scsi_buf_do_request(), since the way the io header is initialized and
the ioctl is performed is used only for requests that use an
intermediate buffer.

For now, the above is the only request type, but we need to pave the way
for the support of requests with scatter-gather lists.

Signed-off-by: Alex Pyrgiotis 
Signed-off-by: Dimitris Aragiorgis 

diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 71c0110..8e2058d 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -175,20 +175,6 @@ static void scsi_command_complete(void *opaque, int ret)
 scsi_command_complete_noio(r, ret);
 }
 
-static int execute_command(BlockBackend *blk,
-   SCSIGenericReq *r, int direction,
-   BlockCompletionFunc *complete)
-{
-scsi_buf_init_io_header(r, direction);
-
-r->req.aiocb = blk_aio_ioctl(blk, SG_IO, >io_header, complete, r);
-if (r->req.aiocb == NULL) {
-return -EIO;
-}
-
-return 0;
-}
-
 static void scsi_buf_read_complete(void *opaque, int ret)
 {
 SCSIGenericReq *r = (SCSIGenericReq *)opaque;
@@ -242,12 +228,33 @@ static void scsi_buf_read_complete(void *opaque, int ret)
 scsi_req_unref(>req);
 }
 
+/*
+ * Execute the request using an intermediate buffer.
+ *
+ * This function does the following:
+ *
+ * a. Initialize the io header for the ioctl request.
+ * b. Perform the ioctl request using blk_aio_ioctl().
+ */
+static void scsi_buf_do_request(SCSIGenericReq *r, int direction,
+BlockCompletionFunc *complete)
+{
+SCSIDevice *s = r->req.dev;
+
+scsi_buf_init_io_header(r, direction);
+
+r->req.aiocb = blk_aio_ioctl(s->conf.blk, SG_IO,
+>io_header, complete, r);
+
+if (!r->req.aiocb) {
+scsi_command_complete_noio(r, -EIO);
+}
+}
+
 /* Read more data from scsi device into buffer.  */
 static void scsi_buf_read_data(SCSIRequest *req)
 {
 SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
-SCSIDevice *s = r->req.dev;
-int ret;
 
 DPRINTF("scsi_buf_read_data 0x%x\n", req->tag);
 
@@ -258,11 +265,7 @@ static void scsi_buf_read_data(SCSIRequest *req)
 return;
 }
 
-ret = execute_command(s->conf.blk, r, SG_DXFER_FROM_DEV,
-  scsi_buf_read_complete);
-if (ret < 0) {
-scsi_command_complete_noio(r, ret);
-}
+scsi_buf_do_request(r, SG_DXFER_FROM_DEV, scsi_buf_read_complete);
 }
 
 static void scsi_common_read_data(SCSIRequest *req)
@@ -299,8 +302,6 @@ static void scsi_buf_write_complete(void *opaque, int ret)
 static void scsi_buf_write_data(SCSIRequest *req)
 {
 SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
-SCSIDevice *s = r->req.dev;
-int ret;
 
 DPRINTF("scsi_buf_write_data 0x%x\n", req->tag);
 if (r->len == 0) {
@@ -311,11 +312,8 @@ static void scsi_buf_write_data(SCSIRequest *req)
 
 /* The request is used as the AIO opaque value, so add a ref.  */
 scsi_req_ref(>req);
-ret = execute_command(s->conf.blk, r, SG_DXFER_TO_DEV,
-scsi_buf_write_complete);
-if (ret < 0) {
-scsi_command_complete_noio(r, ret);
-}
+
+scsi_buf_do_request(r, SG_DXFER_TO_DEV, scsi_buf_write_complete);
 }
 
 static void scsi_common_write_data(SCSIRequest *req)
@@ -339,8 +337,6 @@ static uint8_t *scsi_get_buf(SCSIRequest *req)
 static int32_t scsi_common_send_command(SCSIRequest *req, uint8_t *cmd)
 {
 SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
-SCSIDevice *s = r->req.dev;
-int ret;
 
 #ifdef DEBUG_SCSI
 {
@@ -358,12 +354,7 @@ static int32_t scsi_common_send_command(SCSIRequest *req, 
uint8_t *cmd)
 r->buf = NULL;
 /* The request is used as the AIO opaque value, so add a ref.  */
 scsi_req_ref(>req);
-ret = execute_command(s->conf.blk, r, SG_DXFER_NONE,
-  scsi_command_complete);
-if (ret < 0) {
-scsi_command_complete_noio(r, ret);
-return 0;
-}
+scsi_buf_do_request(r, SG_DXFER_NONE, scsi_command_complete);
 return 0;
 }
 
-- 
2.6.2

Re: [Qemu-devel] [PATCH 1/3] sd: sdhci: Delete over-zealous power check

[Peter Crosthwaite]

On Wed, Dec 16, 2015 at 11:47 AM, Andrew Baumann
 wrote:
> This check was conditionalising SD card operation on the card being
> powered by the SDHCI host controller. It is however possible
> (particularly in embedded systems) for the power control of the SD card
> to be managed outside of SDHCI. This can be as trivial as hard-wiring
> the SD slot VCC to a constant power-rail.
>
> This means the guest SDHCI can validly opt-out of the SDHCI power
> control feature while still using the card. So delete this check to
> allow operation of the card with SDHCI power control.
>
> This is needed for at least Xilinx Zynq and also makes Freescale i.MX25
> work for me. The digilent Zybo board has a public schematic
> which shows SD VCC hardwiring:
>

You should add a note that it is needed for rPI as well.

> http://digilentinc.com/Data/Products/ZYBO/ZYBO_sch_VB.3.pdf
> bottom of page 3.
>
> Signed-off-by: Peter Crosthwaite 
> Reviewed-by: Sai Pavan Boddu 

You should add your tested-by as it is valuable as non-author. Pavan
probably should be CC'd singly (got can be configured to do this
automatically) but I would just CC him whole series as he has does
some SD work.

At this point here you add committable notes about what you have
changed since the original authorship. If you added the note about
fixing rPI, something like:

[ AB changes:
 * Add rPI to list of devices fixed in commit message
]

> Signed-off-by: Andrew Baumann 
> ---
>
> Notes:
> This is exactly equivalent to Peter's patch here:
> https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg06271.html
> ... which is why I've copied the signed-off-by lines (I assume that is
> legit; if not, my apologies!). It is also needed for Pi.
>

The correct process it to apply the patch locally as originally,
authored then append your SoB and any notes/changes using git commit
--amend. Git am can apply the patch from an mbox, or the patches
utility can be used to download and apply patches off the list. You
could in this instance grab the original patch from my github tree:

https://github.com/pcrost/qemu/commit/db8e44ad1d31418fb4b5aaacc8552a39bb2d670f

Regards,
Peter

>  hw/sd/sdhci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
> index 8612760..bc39d48 100644
> --- a/hw/sd/sdhci.c
> +++ b/hw/sd/sdhci.c
> @@ -831,7 +831,7 @@ static void sdhci_data_transfer(void *opaque)
>
>  static bool sdhci_can_issue_command(SDHCIState *s)
>  {
> -if (!SDHC_CLOCK_IS_ON(s->clkcon) || !(s->pwrcon & SDHC_POWER_ON) ||
> +if (!SDHC_CLOCK_IS_ON(s->clkcon) ||
>  (((s->prnsts & SDHC_DATA_INHIBIT) || s->stopped_state) &&
>  ((s->cmdreg & SDHC_CMD_DATA_PRESENT) ||
>  ((s->cmdreg & SDHC_CMD_RESPONSE) == SDHC_CMD_RSP_WITH_BUSY &&
> --
> 2.5.3
>

[Qemu-devel] [Bug 1307225] Re: Running a virtual machine on a Haswell system produces machine check events

[cvbkf]

Minor Update: Bug occurs under Intel Skylake, too.

System-information: Intel Core i7-6700 with 4x8 GB Samsung
M378A1G43DB0-CPB DDR4-2133 RAM, Motherboard: Fujitsu D3401-H1


Dec 15 06:53:30 srv01 kernel: [224214.850599] mce: [Hardware Error]: Machine 
check events logged
Dec 15 06:55:08 srv01 kernel: [224312.001142] mce: [Hardware Error]: Machine 
check events logged
Dec 15 06:57:12 srv01 kernel: [224435.836130] mce: [Hardware Error]: Machine 
check events logged
Dec 15 07:03:35 srv01 kernel: [224818.079136] mce: [Hardware Error]: Machine 
check events logged
Dec 15 07:07:55 srv01 kernel: [225077.697589] mce_notify_irq: 1 callbacks 
suppressed
Dec 15 07:07:55 srv01 kernel: [225077.697592] mce: [Hardware Error]: Machine 
check events logged
Dec 15 07:08:51 srv01 kernel: [225134.136571] mce: [Hardware Error]: Machine 
check events logged
Dec 15 07:12:25 srv01 kernel: [225347.598995] mce_notify_irq: 1 callbacks 
suppressed
Dec 15 07:12:25 srv01 kernel: [225347.598998] mce: [Hardware Error]: Machine 
check events logged
Dec 15 07:15:03 srv01 kernel: [225504.880462] mce: [Hardware Error]: Machine 
check events logged
Dec 15 07:17:49 srv01 kernel: [225670.907609] mce: [Hardware Error]: Machine 
check events logged
Dec 15 07:21:49 srv01 kernel: [225911.163547] mce: [Hardware Error]: Machine 
check events logged
Dec 15 07:22:57 srv01 kernel: [225978.227807] mce: [Hardware Error]: Machine 
check events logged
Dec 15 07:24:32 srv01 kernel: [226073.681985] mce: [Hardware Error]: Machine 
check events logged
Dec 15 07:28:31 srv01 kernel: [226312.111733] mce: [Hardware Error]: Machine 
check events logged
Dec 15 07:34:04 srv01 kernel: [226644.639095] mce: [Hardware Error]: Machine 
check events logged
Dec 15 07:35:58 srv01 kernel: [226757.904937] mce_notify_irq: 2 callbacks 
suppressed
Dec 15 07:35:58 srv01 kernel: [226757.904940] mce: [Hardware Error]: Machine 
check events logged
Dec 15 07:36:10 srv01 kernel: [226770.139237] mce: [Hardware Error]: Machine 
check events logged
Dec 15 07:41:14 srv01 kernel: [227073.719040] mce: [Hardware Error]: Machine 
check events logged
Dec 15 07:41:16 srv01 kernel: [227075.399257] mce: [Hardware Error]: Machine 
check events logged
Dec 15 07:44:14 srv01 kernel: [227253.699541] mce: [Hardware Error]: Machine 
check events logged
Dec 15 07:44:57 srv01 kernel: [227296.490305] mce: [Hardware Error]: Machine 
check events logged
Dec 15 07:52:44 srv01 kernel: [227762.621344] mce: [Hardware Error]: Machine 
check events logged
Dec 15 07:52:49 srv01 kernel: [227767.372259] mce: [Hardware Error]: Machine 
check events logged
Dec 15 07:54:39 srv01 kernel: [227877.219677] mce_notify_irq: 1 callbacks 
suppressed
Dec 15 07:54:39 srv01 kernel: [227877.219680] mce: [Hardware Error]: Machine 
check events logged
...

mcelog: Family 6 Model 5e CPU: only decoding architectural errors
Hardware event. This is not a software error.
MCE 29
CPU 0 BANK 0
TIME 1450162369 Tue Dec 15 07:52:49 2015
MCG status:
MCi status:
Corrected error
Error enabled
MCA: Internal parity error
STATUS 90410005 MCGSTATUS 0
MCGCAP c0a APICID 0 SOCKETID 0
CPUID Vendor Intel Family 6 Model 94
mcelog: Family 6 Model 5e CPU: only decoding architectural errors
Hardware event. This is not a software error.
MCE 30
CPU 2 BANK 0
TIME 1450162422 Tue Dec 15 07:53:42 2015
MCG status:
MCi status:
Corrected error
Error enabled
MCA: Internal parity error
STATUS 90410005 MCGSTATUS 0
MCGCAP c0a APICID 4 SOCKETID 0
CPUID Vendor Intel Family 6 Model 94
mcelog: Family 6 Model 5e CPU: only decoding architectural errors
Hardware event. This is not a software error.
MCE 31
CPU 1 BANK 0
TIME 1450162479 Tue Dec 15 07:54:39 2015
MCG status:
MCi status:
Corrected error
Error enabled
MCA: Internal parity error
STATUS 90410005 MCGSTATUS 0
MCGCAP c0a APICID 2 SOCKETID 0
CPUID Vendor Intel Family 6 Model 94

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1307225

Title:
  Running a virtual machine on a Haswell system produces machine check
  events

Status in QEMU:
  New

Bug description:
  I'm running a virtual Windows SBS 2003 installation on a Xeon E3
  Haswell system running Gentoo Linux. First, I used Qemu 1.5.3 (the
  latest stable version on Gentoo). I got a lot of machine check events
  ("mce: [Hardware Error]: Machine check events logged") in dmesg that
  always looked like (using mcelog):

  Hardware event. This is not a software error.
  MCE 0
  CPU 3 BANK 0
  TIME 1397455091 Mon Apr 14 07:58:11 2014
  MCG status:
  MCi status:
  Corrected error
  Error enabled
  MCA: Internal parity error
  STATUS 904f0005 MCGSTATUS 0
  MCGCAP c09 APICID 6 SOCKETID 0
  CPUID Vendor Intel Family 6 Model 60

  I found this discussion on the vmware community:
  https://communities.vmware.com/thread/452344

  It seems that this is (at least partly) caused by the Qemu machine. I
  switched to Qemu 1.7.0, the first version to use 

Re: [Qemu-devel] [PATCH for-2.6 v2 01/10] fdc: move pick_geometry

[John Snow]

On 12/15/2015 04:51 PM, Hervé Poussineau wrote:
> Le 08/12/2015 00:34, John Snow a écrit :
>> Code motion: I want to refactor this function to work with FDrive
>> directly, so shuffle it below that definition.
>>
>> Signed-off-by: John Snow 
>> ---
>>   hw/block/fdc.c | 90
>> +-
>>   1 file changed, 45 insertions(+), 45 deletions(-)
>>
>> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
>> index 4292ece..246b631 100644
>> --- a/hw/block/fdc.c
>> +++ b/hw/block/fdc.c
>> @@ -114,51 +114,6 @@ static const FDFormat fd_formats[] = {
>>   { FDRIVE_DRV_NONE, -1, -1, 0, 0, },
>>   };
>>
>> -static void pick_geometry(BlockBackend *blk, int *nb_heads,
>> -  int *max_track, int *last_sect,
>> -  FDriveType drive_in, FDriveType *drive,
>> -  FDriveRate *rate)
>> -{
>> -const FDFormat *parse;
>> -uint64_t nb_sectors, size;
>> -int i, first_match, match;
>> -
>> -blk_get_geometry(blk, _sectors);
>> -match = -1;
>> -first_match = -1;
>> -for (i = 0; ; i++) {
>> -parse = _formats[i];
>> -if (parse->drive == FDRIVE_DRV_NONE) {
>> -break;
>> -}
>> -if (drive_in == parse->drive ||
>> -drive_in == FDRIVE_DRV_NONE) {
>> -size = (parse->max_head + 1) * parse->max_track *
>> -parse->last_sect;
>> -if (nb_sectors == size) {
>> -match = i;
>> -break;
>> -}
>> -if (first_match == -1) {
>> -first_match = i;
>> -}
>> -}
>> -}
>> -if (match == -1) {
>> -if (first_match == -1) {
>> -match = 1;
>> -} else {
>> -match = first_match;
>> -}
>> -parse = _formats[match];
>> -}
>> -*nb_heads = parse->max_head + 1;
>> -*max_track = parse->max_track;
>> -*last_sect = parse->last_sect;
>> -*drive = parse->drive;
>> -*rate = parse->rate;
>> -}
>> -
>>   #define GET_CUR_DRV(fdctrl) ((fdctrl)->cur_drv)
>>   #define SET_CUR_DRV(fdctrl, drive) ((fdctrl)->cur_drv = (drive))
>>
>> @@ -286,6 +241,51 @@ static void fd_recalibrate(FDrive *drv)
>>   fd_seek(drv, 0, 0, 1, 1);
>>   }
>>
>> +static void pick_geometry(BlockBackend *blk, int *nb_heads,
>> +  int *max_track, int *last_sect,
>> +  FDriveType drive_in, FDriveType *drive,
>> +  FDriveRate *rate)
>> +{
>> +const FDFormat *parse;
>> +uint64_t nb_sectors, size;
>> +int i, first_match, match;
>> +
>> +blk_get_geometry(blk, _sectors);
>> +match = -1;
>> +first_match = -1;
>> +for (i = 0; ; i++) {
>> +parse = _formats[i];
>> +if (parse->drive == FDRIVE_DRV_NONE) {
>> +break;
>> +}
>> +if (drive_in == parse->drive ||
>> +drive_in == FDRIVE_DRV_NONE) {
>> +size = (parse->max_head + 1) * parse->max_track *
>> +parse->last_sect;
>> +if (nb_sectors == size) {
>> +match = i;
>> +break;
>> +}
>> +if (first_match == -1) {
>> +first_match = i;
>> +}
>> +}
>> +}
>> +if (match == -1) {
>> +if (first_match == -1) {
>> +match = 1;
>> +} else {
>> +match = first_match;
>> +}
>> +parse = _formats[match];
>> +}
>> +*nb_heads = parse->max_head + 1;
>> +*max_track = parse->max_track;
>> +*last_sect = parse->last_sect;
>> +*drive = parse->drive;
>> +*rate = parse->rate;
>> +}
>> +
>>   /* Revalidate a disk drive after a disk change */
>>   static void fd_revalidate(FDrive *drv)
>>   {
> 
> I think it would be better to move the FDiskFlags enum, FDrive, FDCtrl,
> FDCtrlISABus structures to include/hw/block/fdc.h
> That way, we can embed the floppy controller into another chip. I'll
> need it soon for some PIIX4 improvements.
> 
> Regards,
> 
> Hervé
> 

Actually, I'm going to decline on this -- I don't know precisely what
you need moved, and it's trivial for you to cut the headers out when you
need them. It'll be more work re-shuffling this series than it's worth
for me to then not get it completely right anyway.

Just ping me with a patch to move what you need and I will get that done
for you then, thanks.

--js

[Qemu-devel] [PATCH 3/3] sdhci: add optional quirk property to disable card insertion/removal interrupts

[Andrew Baumann]

This is needed for a quirk of the Raspberry Pi (bcm2835/6) MMC
controller, where the card insert bit is documented as unimplemented
(always reads zero, doesn't generate interrupts) but is in fact
observed on hardware as set at power on, but is cleared (and remains
clear) on subsequent controller resets.

Signed-off-by: Andrew Baumann 
---
 hw/sd/sdhci.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index dd83e89..61f919b 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -193,7 +193,9 @@ static void sdhci_reset(SDHCIState *s)
  * initialization */
 memset(>sdmasysad, 0, (uintptr_t)>capareg - 
(uintptr_t)>sdmasysad);
 
-sd_set_cb(s->card, s->ro_cb, s->eject_cb);
+if (!s->noeject_quirk) {
+sd_set_cb(s->card, s->ro_cb, s->eject_cb);
+}
 s->data_count = 0;
 s->stopped_state = sdhc_not_stopped;
 }
@@ -1208,6 +1210,7 @@ const VMStateDescription sdhci_vmstate = {
 VMSTATE_UINT16(data_count, SDHCIState),
 VMSTATE_UINT64(admasysaddr, SDHCIState),
 VMSTATE_UINT8(stopped_state, SDHCIState),
+VMSTATE_BOOL(noeject_quirk, SDHCIState),
 VMSTATE_VBUFFER_UINT32(fifo_buffer, SDHCIState, 1, NULL, 0, buf_maxsz),
 VMSTATE_TIMER_PTR(insert_timer, SDHCIState),
 VMSTATE_TIMER_PTR(transfer_timer, SDHCIState),
@@ -1276,6 +1279,7 @@ static Property sdhci_sysbus_properties[] = {
 DEFINE_PROP_UINT32("capareg", SDHCIState, capareg,
 SDHC_CAPAB_REG_DEFAULT),
 DEFINE_PROP_UINT32("maxcurr", SDHCIState, maxcurr, 0),
+DEFINE_PROP_BOOL("noeject-quirk", SDHCIState, noeject_quirk, false),
 DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.5.3

Re: [Qemu-devel] [RFC PATCH v0 2/9] cpu: Store CPU typename in MachineState

[Eduardo Habkost]

On Wed, Dec 16, 2015 at 05:54:25PM +0100, Igor Mammedov wrote:
> On Tue, 15 Dec 2015 14:08:09 +0530
> Bharata B Rao  wrote:
> 
> > On Mon, Dec 14, 2015 at 03:29:49PM -0200, Eduardo Habkost wrote:
> > > On Thu, Dec 10, 2015 at 11:45:37AM +0530, Bharata B Rao wrote:
> > > > Storing CPU typename in MachineState lets us to create CPU threads
> > > > for all architectures in uniform manner from arch-neutral code.
> > > > 
> > > > TODO: Touching only i386 and spapr targets for now
> > > > 
> > > > Signed-off-by: Bharata B Rao 
> > > 
> > > Suggestions:
> > > 
> > > * Name the field "cpu_base_type" to indicate it is the base CPU
> > >   class name, not the actual CPU class name used when creating
> > >   CPUs.
> > > * Put it in MachineClass, as it may be useful for code that
> > >   runs before machine->init(), in the future.
> > 
> > Ok.
> > 
> > > * Maybe make it a CPUClass* field instead of a string?
> > 
> > In the current use case, this base cpu type string is being passed
> > to cpu_generic_init(const char *typename, const char *cpu_model)
> > to create boot time CPUs with given typename and cpu_mode. So for now
> > the string makes sense for use case.
> > 
> > Making it CPUClass* would necessiate more changes to
> > cpu_generic_init().
> how about actually leaving it as "cpu_type" and putting in it
> actual cpu type that could be used with device_add().
> 
> that would get rid of keeping and passing around intermediate cpu_model.

Makes sense. We only need to save both typename and cpu_model
today because cpu_generic_init() currently encapsulates three
steps: CPU class lookup + CPU creation + CPU feature parsing. But
we shouldn't need to redo CPU class lookup every time.

We could just split cpu_model once, and save the resulting
CPUClass* + featurestr, instead of saving the full cpu_model
string and parsing it again every time.

The only problem is that it would require refactoring multiple
machines/architectures that use a cpu_XXX_init(const char *cpu_model)
helper.

-- 
Eduardo

[Qemu-devel] [PATCH 0/9] Add full scatter-gather support for SCSI generic devices

[Alex Pyrgiotis]

Hi all,

This patch is an attempt to boost the performance of "scsi-generic" and
"scsi-block" device types, by removing an extra data copy and reducing
their memory footprint. More specifically, the problem lies in the
functions in the `scsi-generic_req_ops` struct of scsi-generic.c. These
functions rely on an intermediate buffer to do the SG_IO ioctl request,
without checking if the SCSI controller has provided a scatter-gather
list with the request.

In a nutshell, our proposal is to map the provided scatter-gather list
(if any) to the address space of the QEMU process and use the resulting
iovec as the buffer for the ioctl request. You'll find that the logic is
quite similar to the one used in scsi-disk.c.

Cheers,
Alex

Alex Pyrgiotis (9):
  dma-helpers: Expose the sg mapping logic
  dma-helpers: Add support for ioctl operations
  dma-helpers: Do not truncate small qiovs
  scsi-generic: Add common functions
  scsi-generic: Separate `sg_io_hdr' initializations
  scsi-generic: Make request execution buf-specific
  scsi-generic: Make data-copying logic clearer
  scsi-generic: Factor out response interception
  scsi-generic: Allow full scatter-gather support

 dma-helpers.c  | 184 ++---
 hw/scsi/scsi-generic.c | 422 +
 include/sysemu/dma.h   |   8 +
 trace-events   |   4 +-
 4 files changed, 488 insertions(+), 130 deletions(-)

-- 
2.6.2

[Qemu-devel] [PATCH 5/9] scsi-generic: Separate `sg_io_hdr' initializations

[Alex Pyrgiotis]

Separate the initialization of the `sg_io_hdr' struct in two parts: one
part that fills the struct with sane defaults, and another part that
prepares it for an SG_IO request with DIRECT IO and a single buffer. The
first part can also be reused later on by the code that uses
scatter-gather lists.

Signed-off-by: Alex Pyrgiotis 
Signed-off-by: Dimitris Aragiorgis 

diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index f24f472..71c0110 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -58,6 +58,30 @@ typedef struct SCSIGenericReq {
 sg_io_hdr_t io_header;
 } SCSIGenericReq;
 
+/* Fill an io header with sane defaults. */
+static void _init_io_header(SCSIGenericReq *r, int direction)
+{
+r->io_header.interface_id = 'S';
+r->io_header.dxfer_direction = direction;
+r->io_header.dxfer_len = r->buflen;
+r->io_header.cmdp = r->req.cmd.buf;
+r->io_header.cmd_len = r->req.cmd.len;
+r->io_header.mx_sb_len = sizeof(r->req.sense);
+r->io_header.sbp = r->req.sense;
+r->io_header.timeout = MAX_UINT;
+r->io_header.usr_ptr = r;
+}
+
+/* Create an io_header for buf* operations. */
+static void scsi_buf_init_io_header(SCSIGenericReq *r, int direction)
+{
+_init_io_header(r, direction);
+
+r->io_header.iovec_count = 0;
+r->io_header.dxferp = r->buf;
+r->io_header.flags |= SG_FLAG_DIRECT_IO;
+}
+
 static void scsi_generic_save_request(QEMUFile *f, SCSIRequest *req)
 {
 SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
@@ -155,17 +179,7 @@ static int execute_command(BlockBackend *blk,
SCSIGenericReq *r, int direction,
BlockCompletionFunc *complete)
 {
-r->io_header.interface_id = 'S';
-r->io_header.dxfer_direction = direction;
-r->io_header.dxferp = r->buf;
-r->io_header.dxfer_len = r->buflen;
-r->io_header.cmdp = r->req.cmd.buf;
-r->io_header.cmd_len = r->req.cmd.len;
-r->io_header.mx_sb_len = sizeof(r->req.sense);
-r->io_header.sbp = r->req.sense;
-r->io_header.timeout = MAX_UINT;
-r->io_header.usr_ptr = r;
-r->io_header.flags |= SG_FLAG_DIRECT_IO;
+scsi_buf_init_io_header(r, direction);
 
 r->req.aiocb = blk_aio_ioctl(blk, SG_IO, >io_header, complete, r);
 if (r->req.aiocb == NULL) {
-- 
2.6.2

[Qemu-devel] [PATCH 7/9] scsi-generic: Make data-copying logic clearer

[Alex Pyrgiotis]

The copying of data to/from the intermediate buffer of the device is
done by scsi_req_data(). Internally, scsi_req_data() also restarts the
request with scsi_req_continue(). Therefore, we need a guard variable to
know when the contents of the intermediate buffer are in sync with the
data of the guest kernel, so that we can proceed with the request.

While there is such a guard variable in the `SCSIGenericReq' struct (the
`len' field), its intent is not clear and is assigned various
values, when only two are necessary; 0 and 1.

Rename the `len' field to `synced' and add an explanation for what it
does.

Signed-off-by: Alex Pyrgiotis 
Signed-off-by: Dimitris Aragiorgis 

diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 8e2058d..6c0cfa5 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -54,7 +54,23 @@ typedef struct SCSIGenericReq {
 SCSIRequest req;
 uint8_t *buf;
 int buflen;
-int len;
+
+/*
+ * Note for the `synced' field:
+ *
+ * If the controller does not provide the device with a scatter-gather
+ * list, then the device must create an intermediate buffer for the
+ * request's data.  For write requests, this buffer must be filled with
+ * data from the controller. For read requests, the controller must get the
+ * data from the buffer.
+ *
+ * The above transfers are handled by the scsi_req_data() function. Since
+ * scsi_req_data() effectively restarts the request, we need an indication
+ * so that we don't do the same request twice. This indication is the
+ * `synced' field.
+ */
+int synced;
+
 sg_io_hdr_t io_header;
 } SCSIGenericReq;
 
@@ -192,7 +208,6 @@ static void scsi_buf_read_complete(void *opaque, int ret)
 len = r->io_header.dxfer_len - r->io_header.resid;
 DPRINTF("Data ready tag=0x%x len=%d\n", r->req.tag, len);
 
-r->len = -1;
 if (len == 0) {
 scsi_command_complete_noio(r, 0);
 return;
@@ -224,6 +239,7 @@ static void scsi_buf_read_complete(void *opaque, int ret)
 r->buf[3] |= 0x80;
 }
 }
+r->synced = 1;
 scsi_req_data(>req, len);
 scsi_req_unref(>req);
 }
@@ -260,7 +276,9 @@ static void scsi_buf_read_data(SCSIRequest *req)
 
 /* The request is used as the AIO opaque value, so add a ref.  */
 scsi_req_ref(>req);
-if (r->len == -1) {
+
+/* If we are here due to scsi_req_data(), we can complete the request. */
+if (r->synced) {
 scsi_command_complete_noio(r, 0);
 return;
 }
@@ -304,9 +322,14 @@ static void scsi_buf_write_data(SCSIRequest *req)
 SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
 
 DPRINTF("scsi_buf_write_data 0x%x\n", req->tag);
-if (r->len == 0) {
-r->len = r->buflen;
-scsi_req_data(>req, r->len);
+
+/*
+ * Before performing the write request, we need to transfer the data to
+ * our intermediate buffer.
+ */
+if (!r->synced) {
+r->synced = 1;
+scsi_req_data(req, r->buflen);
 return;
 }
 
@@ -365,9 +388,7 @@ static int32_t scsi_common_send_command(SCSIRequest *req, 
uint8_t *cmd)
 }
 
 memset(r->buf, 0, r->buflen);
-r->len = r->req.cmd.xfer;
 if (r->req.cmd.mode == SCSI_XFER_TO_DEV) {
-r->len = 0;
 return -r->req.cmd.xfer;
 } else {
 return r->req.cmd.xfer;
-- 
2.6.2

[Qemu-devel] [PATCH v2 0/3] SD emulation fixes for Pi2 Tianocore EDK2 UEFI

[Andrew Baumann]

This series contains fixes to the SD card emulation that are needed to
unblock Tianocore EDK2 UEFI (specifically, the bootloader for Windows
on Raspberry Pi 2).

Changes in v2, based on feedback from Peter Crosthwaite:
 * correct implementation of CMD23 to switch to transfer state on completion
 * use an actual timer for the power-up delay, rather than relying on
   the guest polling ACMD41 twice
 * added patch 3: replace fprintfs with guest error logging

(I'm guessing at the CC list here, since this code appears to be
unmaintained. Apologies if I guessed wrong!)

Cheers,
Andrew

Andrew Baumann (3):
  hw/sd: implement CMD23 (SET_BLOCK_COUNT) for MMC compatibility
  hw/sd: model a power-up delay, as a workaround for an EDK2 bug
  hw/sd: use guest error logging rather than fprintf to stderr

 hw/sd/sd.c | 104 -
 1 file changed, 90 insertions(+), 14 deletions(-)

-- 
2.5.3

[Qemu-devel] [PATCH 9/9] scsi-generic: Allow full scatter-gather support

[Alex Pyrgiotis]

If the scsi controller uses scatter-gather lists, do not copy them to an
intermediate buffer. Instead, use them as is via the dma_blk_ioctl()
function.

In order to make this feature possible, the following changes have been
made to the code:

* All I/O functions have been branched into two types of functions:
  "sg_*" and "buf_*", which are used for sg and non-sg operations
  respectively.  The shared code between them is likewise moved in
  "common_*" functions.
* The scsi_req_data() function is predictably used only for non-sg
  operations.
* The `sg_io_hdr' struct for scatter-gather operations does not include
  the `SG_FLAG_DIRECT_IO' flag, since it does not work with iovecs, as
  explained here [1].

[1]: http://www.tldp.org/HOWTO/SCSI-Generic-HOWTO/x192.html

Signed-off-by: Alex Pyrgiotis 
Signed-off-by: Dimitris Aragiorgis 

diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 6704861..5f3c401 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -16,6 +16,7 @@
 #include "hw/scsi/scsi.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
+#include "sysemu/dma.h"
 
 #ifdef __linux__
 
@@ -98,12 +99,57 @@ static void scsi_buf_init_io_header(SCSIGenericReq *r, int 
direction)
 r->io_header.flags |= SG_FLAG_DIRECT_IO;
 }
 
-/* Return a pointer to the data buffer.  */
+/*
+ * Create an io_header for sg* operations.
+ *
+ * Note: The rest of the fields are filled when scsi_sg_handle_iov() is called.
+ */
+static void scsi_sg_init_io_header(SCSIGenericReq *r, int direction)
+{
+_init_io_header(r, direction);
+}
+
+/*
+ * Handle a QEMUIOVector, as created by the dma_blk_ioctl() function.
+ *
+ * As mentioned here [1], in order to use an iovec for the ioctl operation, the
+ * iovec_count field must be filled with the number of vectors, the dxferp
+ * field must point to the iovecs and the dxfer_len field must state the size
+ * of the request, which should amount to the sum of iov_lens.
+ *
+ * [1] http://sg.danny.cz/sg/p/sg_v3_ho.html#id2495162
+ */
+static void scsi_sg_handle_iov(void *opaque, QEMUIOVector *iov)
+{
+SCSIGenericReq *r = (SCSIGenericReq *)opaque;
+
+r->io_header.iovec_count = iov->niov;
+r->io_header.dxferp = iov->iov;
+r->io_header.dxfer_len = iov->size;
+}
+
+/*
+ * Return a pointer to the request's data buffer.
+ *
+ * If the SCSI controller does not provide a scatter-gather list, then the data
+ * buffer of the request is stored in the `buf' field of SCSIGenericReq.
+ *
+ * Else, the data buffer is mapped to a list of iovecs and this function is
+ * never called externally. If called internally, it will be solely for the
+ * manipulation of the first 9 bytes of data of a SCSI response. In this case,
+ * we can safely return a pointer to the data of the first iovec.
+ */
 static uint8_t *scsi_get_buf(SCSIRequest *req)
 {
+struct iovec *iov;
 SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
 
-return r->buf;
+if (req->sg) {
+iov = (struct iovec *)r->io_header.dxferp;
+return iov->iov_base;
+} else {
+return r->buf;
+}
 }
 
 static void scsi_generic_save_request(QEMUFile *f, SCSIRequest *req)
@@ -265,6 +311,18 @@ static int scsi_common_read_complete(SCSIGenericReq *r, 
int ret)
 return len;
 }
 
+static void scsi_sg_read_complete(void *opaque, int ret)
+{
+int len;
+SCSIGenericReq *r = (SCSIGenericReq *)opaque;
+
+len = scsi_common_read_complete(r, ret);
+if (len > 0) {
+r->req.resid = r->io_header.resid;
+scsi_command_complete_noio(r, 0);
+}
+}
+
 static void scsi_buf_read_complete(void *opaque, int ret)
 {
 int len;
@@ -279,6 +337,39 @@ static void scsi_buf_read_complete(void *opaque, int ret)
 }
 
 /*
+ * Execute the request using the provided scatter-gather list.
+ *
+ * This function does the following:
+ *
+ * a. Initialize the io header for the ioctl request.
+ * b. Derive the DMA direction from the provided request direction.
+ * c. Perform the ioctl request using dma_blk_ioctl() from dma-helpers. Also,
+ *register a callback for the handling of the resulting iovector from the
+ *internal mapping of the scatter-gather list.
+ */
+static void scsi_sg_do_request(SCSIGenericReq *r, int direction,
+BlockCompletionFunc *complete)
+{
+SCSIDevice *s = r->req.dev;
+DMADirection dir;
+
+scsi_sg_init_io_header(r, direction);
+
+if (direction == SG_DXFER_TO_DEV) {
+dir = DMA_DIRECTION_TO_DEVICE;
+} else if (direction == SG_DXFER_FROM_DEV) {
+dir = DMA_DIRECTION_FROM_DEVICE;
+}
+
+r->req.aiocb = dma_blk_ioctl(s->conf.blk, SG_IO, >io_header, r->req.sg,
+dir, scsi_sg_handle_iov, complete, r);
+
+if (!r->req.aiocb) {
+scsi_command_complete_noio(r, -EIO);
+}
+}
+
+/*
  * Execute the request using an intermediate buffer.
  *
  * This function does the following:
@@ -301,7 +392,18 @@ static void 

Re: [Qemu-devel] [PATCH v4 4/5] xlnx-zynqmp: Connect the SPI devices

[Alistair Francis]

On Tue, Dec 15, 2015 at 10:46 AM, Peter Maydell
 wrote:
> On 23 November 2015 at 08:54, Alistair Francis
>  wrote:
>> Connect the Xilinx SPI devices to the ZynqMP model.
>>
>> Signed-off-by: Alistair Francis 
>
>> +/* Rename each SPI bus after the SPI device to allow the board
>> + * to access all of the busses from the SoC.
>> + */
>> +spi_bus = qdev_get_child_bus(DEVICE(>spi[i]), "spi0");
>> +snprintf(bus_name, 6, "spi%d", i);
>> +memcpy((char *) spi_bus->name, bus_name, 6 * sizeof(char));
>
> This looks very dubious. If we need to be able to rename buses
> can we provide a proper API to do it please, rather than just
> memcpying into the name field?

Ok, I have added a qdev function which renames busses.

Re-sending now.

Thanks,

Alistair

>
> thanks
> -- PMM
>

[Qemu-devel] [PATCH 8/9] scsi-generic: Factor out response interception

[Alex Pyrgiotis]

The interception of read/write responses is currently done in the main
code that handles the read write response. Move the interception logic
in a function of its own, so that it can be reused from the
scatter-gather path.

Also, instead of altering the response buffer directly, use the
scsi_get_buf() function and alter the buffer that it returns. This is
also required for the support of scatter-gather lists.

Signed-off-by: Alex Pyrgiotis 
Signed-off-by: Dimitris Aragiorgis 

diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index 6c0cfa5..6704861 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -98,6 +98,14 @@ static void scsi_buf_init_io_header(SCSIGenericReq *r, int 
direction)
 r->io_header.flags |= SG_FLAG_DIRECT_IO;
 }
 
+/* Return a pointer to the data buffer.  */
+static uint8_t *scsi_get_buf(SCSIRequest *req)
+{
+SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
+
+return r->buf;
+}
+
 static void scsi_generic_save_request(QEMUFile *f, SCSIRequest *req)
 {
 SCSIGenericReq *r = DO_UPCAST(SCSIGenericReq, req, req);
@@ -127,7 +135,7 @@ static void scsi_free_request(SCSIRequest *req)
 g_free(r->buf);
 }
 
-/* Helper function for command completion.  */
+/* Helper function for command completion. */
 static void scsi_command_complete_noio(SCSIGenericReq *r, int ret)
 {
 int status;
@@ -191,37 +199,23 @@ static void scsi_command_complete(void *opaque, int ret)
 scsi_command_complete_noio(r, ret);
 }
 
-static void scsi_buf_read_complete(void *opaque, int ret)
+/* Intercept the read response in order to snoop or alter it. */
+static void scsi_intercept_read_response(SCSIGenericReq *r)
 {
-SCSIGenericReq *r = (SCSIGenericReq *)opaque;
+uint8_t *res_buf;
 SCSIDevice *s = r->req.dev;
-int len;
 
-assert(r->req.aiocb != NULL);
-r->req.aiocb = NULL;
-
-if (ret || r->req.io_canceled) {
-scsi_command_complete_noio(r, ret);
-return;
-}
-
-len = r->io_header.dxfer_len - r->io_header.resid;
-DPRINTF("Data ready tag=0x%x len=%d\n", r->req.tag, len);
-
-if (len == 0) {
-scsi_command_complete_noio(r, 0);
-return;
-}
+res_buf = scsi_get_buf(>req);
 
 /* Snoop READ CAPACITY output to set the blocksize.  */
 if (r->req.cmd.buf[0] == READ_CAPACITY_10 &&
-(ldl_be_p(>buf[0]) != 0xU || s->max_lba == 0)) {
-s->blocksize = ldl_be_p(>buf[4]);
-s->max_lba = ldl_be_p(>buf[0]) & 0xULL;
+(ldl_be_p(_buf[0]) != 0xU || s->max_lba == 0)) {
+s->blocksize = ldl_be_p(_buf[4]);
+s->max_lba = ldl_be_p(_buf[0]) & 0xULL;
 } else if (r->req.cmd.buf[0] == SERVICE_ACTION_IN_16 &&
(r->req.cmd.buf[1] & 31) == SAI_READ_CAPACITY_16) {
-s->blocksize = ldl_be_p(>buf[8]);
-s->max_lba = ldq_be_p(>buf[0]);
+s->blocksize = ldl_be_p(_buf[8]);
+s->max_lba = ldq_be_p(_buf[0]);
 }
 blk_set_guest_block_size(s->conf.blk, s->blocksize);
 
@@ -234,14 +228,54 @@ static void scsi_buf_read_complete(void *opaque, int ret)
  r->req.cmd.buf[0] == MODE_SENSE_10) &&
 (r->req.cmd.buf[1] & 0x8) == 0) {
 if (r->req.cmd.buf[0] == MODE_SENSE) {
-r->buf[2] |= 0x80;
+res_buf[2] |= 0x80;
 } else  {
-r->buf[3] |= 0x80;
+res_buf[3] |= 0x80;
 }
 }
-r->synced = 1;
-scsi_req_data(>req, len);
-scsi_req_unref(>req);
+}
+
+/*
+ * Perform some common checks and return the number of bytes read.
+ *
+ * If we encounter an error, return -1.
+ */
+static int scsi_common_read_complete(SCSIGenericReq *r, int ret)
+{
+int len;
+
+assert(r->req.aiocb != NULL);
+r->req.aiocb = NULL;
+
+if (ret || r->req.io_canceled) {
+scsi_command_complete_noio(r, ret);
+return -1;
+}
+
+len = r->io_header.dxfer_len - r->io_header.resid;
+DPRINTF("Data ready tag=0x%x len=%d\n", r->req.tag, len);
+
+if (len == 0) {
+scsi_command_complete_noio(r, 0);
+return 0;
+}
+
+scsi_intercept_read_response(r);
+
+return len;
+}
+
+static void scsi_buf_read_complete(void *opaque, int ret)
+{
+int len;
+SCSIGenericReq *r = (SCSIGenericReq *)opaque;
+
+len = scsi_common_read_complete(r, ret);
+if (len > 0) {
+r->synced = 1;
+scsi_req_data(>req, len);
+scsi_req_unref(>req);
+}
 }
 
 /*
@@ -291,12 +325,26 @@ static void scsi_common_read_data(SCSIRequest *req)
 scsi_buf_read_data(req);
 }
 
-static void scsi_buf_write_complete(void *opaque, int ret)
+/* Intercept the write response in order to snoop or alter it */
+static void scsi_intercept_write_response(SCSIGenericReq *r)
 {
-SCSIGenericReq *r = (SCSIGenericReq *)opaque;
+uint8_t *res_buf;
 SCSIDevice *s = r->req.dev;
 
-DPRINTF("scsi_write_complete() ret = %d\n", ret);
+res_buf = scsi_get_buf(>req);

[Qemu-devel] [PATCH v2 1/1] xlnx-zynqmp: Add support for high DDR memory regions

[Alistair Francis]

The Xilinx ZynqMP SoC and EP108 board supports three memory regions:
 - A 2GB region starting at 0
 - A 32GB region starting at 32GB
 - A 256GB region starting at 768GB

This patch adds support for the first two memory regions, which is
automatically created based on the size specified by the QEMU memory
command line argument.

On hardware the physical memory region is one continuous region, it is then
mapped into the three different regions by the DDRC. As we don't model the
DDRC this is done at startup by QEMU. The board creates the memory region and
then passes that memory region to the SoC. The SoC then maps the memory
regions.

Signed-off-by: Alistair Francis 
---
V2:
 - Create one continuous memory region and pass it to the SoC

Also, the Xilinx ZynqMP TRM is avaliable at:
http://www.xilinx.com/products/silicon-devices/soc/zynq-ultrascale-mpsoc.html?resultsTablePreSelect=documenttype:User%20Guides#documentation

 hw/arm/xlnx-ep108.c  | 42 +++---
 hw/arm/xlnx-zynqmp.c | 31 +++
 include/hw/arm/xlnx-zynqmp.h | 12 
 3 files changed, 66 insertions(+), 19 deletions(-)

diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c
index 85b978f..a0174d5 100644
--- a/hw/arm/xlnx-ep108.c
+++ b/hw/arm/xlnx-ep108.c
@@ -22,12 +22,9 @@
 
 typedef struct XlnxEP108 {
 XlnxZynqMPState soc;
-MemoryRegion ddr_ram;
+MemoryRegion ddr_board_ram;
 } XlnxEP108;
 
-/* Max 2GB RAM */
-#define EP108_MAX_RAM_SIZE 0x8000ull
-
 static struct arm_boot_info xlnx_ep108_binfo;
 
 static void xlnx_ep108_init(MachineState *machine)
@@ -35,20 +32,12 @@ static void xlnx_ep108_init(MachineState *machine)
 XlnxEP108 *s = g_new0(XlnxEP108, 1);
 Error *err = NULL;
 
-object_initialize(>soc, sizeof(s->soc), TYPE_XLNX_ZYNQMP);
-object_property_add_child(OBJECT(machine), "soc", OBJECT(>soc),
-  _abort);
-
-object_property_set_bool(OBJECT(>soc), true, "realized", );
-if (err) {
-error_report("%s", error_get_pretty(err));
-exit(1);
-}
-
-if (machine->ram_size > EP108_MAX_RAM_SIZE) {
+/* Create the memory region to pass to the SoC */
+if (machine->ram_size > XLNX_ZYNQMP_MAX_RAM_SIZE) {
 error_report("WARNING: RAM size " RAM_ADDR_FMT " above max supported, "
- "reduced to %llx", machine->ram_size, EP108_MAX_RAM_SIZE);
-machine->ram_size = EP108_MAX_RAM_SIZE;
+ "reduced to %llx", machine->ram_size,
+ XLNX_ZYNQMP_MAX_RAM_SIZE);
+machine->ram_size = XLNX_ZYNQMP_MAX_RAM_SIZE;
 }
 
 if (machine->ram_size < 0x0800) {
@@ -56,9 +45,24 @@ static void xlnx_ep108_init(MachineState *machine)
  machine->ram_size);
 }
 
-memory_region_allocate_system_memory(>ddr_ram, NULL, "ddr-ram",
+memory_region_allocate_system_memory(>ddr_board_ram, NULL,
+ "xlnx-zynqmp-board-ram",
  machine->ram_size);
-memory_region_add_subregion(get_system_memory(), 0, >ddr_ram);
+
+object_initialize(>soc, sizeof(s->soc), TYPE_XLNX_ZYNQMP);
+object_property_add_child(OBJECT(machine), "soc", OBJECT(>soc),
+  _abort);
+
+object_property_set_int(OBJECT(>soc), machine->ram_size,
+"ram-size", _abort);
+object_property_set_link(OBJECT(>soc), OBJECT(>ddr_board_ram),
+ "xlnx-zynqmp-board-ram", _abort);
+
+object_property_set_bool(OBJECT(>soc), true, "realized", );
+if (err) {
+error_report("%s", error_get_pretty(err));
+exit(1);
+}
 
 xlnx_ep108_binfo.ram_size = machine->ram_size;
 xlnx_ep108_binfo.kernel_filename = machine->kernel_filename;
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 87553bb..848d1ff 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -90,6 +90,11 @@ static void xlnx_zynqmp_init(Object *obj)
   _abort);
 }
 
+object_property_add_link(obj, "xlnx-zynqmp-board-ram", TYPE_MEMORY_REGION,
+ (Object **)>ddr_board_ram,
+ qdev_prop_allow_set_link_before_realize,
+ OBJ_PROP_LINK_UNREF_ON_RELEASE, _abort);
+
 object_initialize(>gic, sizeof(s->gic), TYPE_ARM_GIC);
 qdev_set_parent_bus(DEVICE(>gic), sysbus_get_default());
 
@@ -120,9 +125,33 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
**errp)
 MemoryRegion *system_memory = get_system_memory();
 uint8_t i;
 const char *boot_cpu = s->boot_cpu ? s->boot_cpu : "apu-cpu[0]";
+ram_addr_t ddr_low_size, ddr_high_size;
 qemu_irq gic_spi[GIC_NUM_SPI_INTR];
 Error *err = NULL;
 
+/* Create the DDR Memory Regions */
+if (s->ram_size > XLNX_ZYNQMP_MAX_LOW_RAM_SIZE) {
+/* The RAM size is above 

[Qemu-devel] [PATCH 1/3] sd: sdhci: Delete over-zealous power check

[Andrew Baumann]

This check was conditionalising SD card operation on the card being
powered by the SDHCI host controller. It is however possible
(particularly in embedded systems) for the power control of the SD card
to be managed outside of SDHCI. This can be as trivial as hard-wiring
the SD slot VCC to a constant power-rail.

This means the guest SDHCI can validly opt-out of the SDHCI power
control feature while still using the card. So delete this check to
allow operation of the card with SDHCI power control.

This is needed for at least Xilinx Zynq and also makes Freescale i.MX25
work for me. The digilent Zybo board has a public schematic
which shows SD VCC hardwiring:

http://digilentinc.com/Data/Products/ZYBO/ZYBO_sch_VB.3.pdf
bottom of page 3.

Signed-off-by: Peter Crosthwaite 
Reviewed-by: Sai Pavan Boddu 
Signed-off-by: Andrew Baumann 
---

Notes:
This is exactly equivalent to Peter's patch here:
https://lists.gnu.org/archive/html/qemu-devel/2015-11/msg06271.html
... which is why I've copied the signed-off-by lines (I assume that is
legit; if not, my apologies!). It is also needed for Pi.

 hw/sd/sdhci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 8612760..bc39d48 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -831,7 +831,7 @@ static void sdhci_data_transfer(void *opaque)
 
 static bool sdhci_can_issue_command(SDHCIState *s)
 {
-if (!SDHC_CLOCK_IS_ON(s->clkcon) || !(s->pwrcon & SDHC_POWER_ON) ||
+if (!SDHC_CLOCK_IS_ON(s->clkcon) ||
 (((s->prnsts & SDHC_DATA_INHIBIT) || s->stopped_state) &&
 ((s->cmdreg & SDHC_CMD_DATA_PRESENT) ||
 ((s->cmdreg & SDHC_CMD_RESPONSE) == SDHC_CMD_RSP_WITH_BUSY &&
-- 
2.5.3

[Qemu-devel] [PATCH 2/3] sdhci: don't raise a command index error for an unexpected response

[Andrew Baumann]

This deletes a block of code that raised a command index error if a
command returned response data, but the guest did not set the
appropriate bits in the response register to handle such a response. I
cannot find any documentation that suggests the controller should
behave in this way, the error code doesn't make sense (command index
error is defined for the case where the index in a response does not
match that of the issued command), and in at least one case (CMD23
issued by UEFI on Raspberry Pi 2), actual hardware does not do this.

Signed-off-by: Andrew Baumann 
---
 hw/sd/sdhci.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index bc39d48..dd83e89 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -243,9 +243,6 @@ static void sdhci_send_command(SDHCIState *s)
 (s->cmdreg & SDHC_CMD_RESPONSE) == SDHC_CMD_RSP_WITH_BUSY) {
 s->norintsts |= SDHC_NIS_TRSCMP;
 }
-} else if (rlen != 0 && (s->errintstsen & SDHC_EISEN_CMDIDX)) {
-s->errintsts |= SDHC_EIS_CMDIDX;
-s->norintsts |= SDHC_NIS_ERR;
 }
 
 if (s->norintstsen & SDHC_NISEN_CMDCMP) {
-- 
2.5.3

[Qemu-devel] [PATCH 1/9] dma-helpers: Expose the sg mapping logic

[Alex Pyrgiotis]

The mapping of scatter-gather lists from physical addresses (as
perceived by the guest kernel) to the virtual address space of the QEMU
process is a vital step for a DMA operation. This step is currently
implemented, amongst other things, in dma_blk_cb(), making it impossible
to be used by anyone else.

In order to pave the way for the DMA support of ioctl commands, expose
the aforementioned logic in a separate function called "dma_map_sg".
Also, expose some other important pieces too, such as the initialization
of the dbs structure.

Signed-off-by: Alex Pyrgiotis 
Signed-off-by: Dimitris Aragiorgis 

diff --git a/dma-helpers.c b/dma-helpers.c
index 4faec5d..c38661e 100644
--- a/dma-helpers.c
+++ b/dma-helpers.c
@@ -79,9 +79,10 @@ typedef struct {
 QEMUIOVector iov;
 QEMUBH *bh;
 DMAIOFunc *io_func;
+BlockCompletionFunc *dma_cb;
 } DMAAIOCB;
 
-static void dma_blk_cb(void *opaque, int ret);
+static void dma_blk_io_cb(void *opaque, int ret);
 
 static void reschedule_dma(void *opaque)
 {
@@ -89,7 +90,7 @@ static void reschedule_dma(void *opaque)
 
 qemu_bh_delete(dbs->bh);
 dbs->bh = NULL;
-dma_blk_cb(dbs, 0);
+dbs->dma_cb(dbs, 0);
 }
 
 static void dma_blk_unmap(DMAAIOCB *dbs)
@@ -120,21 +121,19 @@ static void dma_complete(DMAAIOCB *dbs, int ret)
 qemu_aio_unref(dbs);
 }
 
-static void dma_blk_cb(void *opaque, int ret)
+/*
+ * Create a QEMUIOVector from a scatter-gather list.
+ *
+ * This function does not copy the data of the scatter-gather list. Instead, it
+ * uses the dma_memory_map() function to map physical memory regions of the
+ * virtual device (as interpreted by the guest kernel) into the address space
+ * of the QEMU process, in order to have access to the data.
+ */
+static void dma_map_sg(DMAAIOCB *dbs)
 {
-DMAAIOCB *dbs = (DMAAIOCB *)opaque;
 dma_addr_t cur_addr, cur_len;
 void *mem;
 
-trace_dma_blk_cb(dbs, ret);
-
-dbs->acb = NULL;
-dbs->sector_num += dbs->iov.size / 512;
-
-if (dbs->sg_cur_index == dbs->sg->nsg || ret < 0) {
-dma_complete(dbs, ret);
-return;
-}
 dma_blk_unmap(dbs);
 
 while (dbs->sg_cur_index < dbs->sg->nsg) {
@@ -162,9 +161,38 @@ static void dma_blk_cb(void *opaque, int ret)
 if (dbs->iov.size & ~BDRV_SECTOR_MASK) {
 qemu_iovec_discard_back(>iov, dbs->iov.size & ~BDRV_SECTOR_MASK);
 }
+}
+
+/*
+ * Callback function for DMA read/write operations.
+ *
+ * This function initiates the read/write operation and also acts as a
+ * completion callback. It uses the dma_map_sg() function to map the
+ * scatter-gather list to a QEMUIOVector and then passes this iovec to the
+ * underlying read/write I/O function.
+ *
+ * If the DMA operation cannot take place in one step, e.g. it couldn't map all
+ * the scatter-gather entries, then this function will do a partial I/O
+ * operation and once done, it will be called and will retry the I/O operation.
+ */
+static void dma_blk_io_cb(void *opaque, int ret)
+{
+DMAAIOCB *dbs = (DMAAIOCB *)opaque;
+
+trace_dma_blk_io_cb(dbs, ret);
+
+dbs->acb = NULL;
+dbs->sector_num += dbs->iov.size / 512;
+
+if (dbs->sg_cur_index == dbs->sg->nsg || ret < 0) {
+dma_complete(dbs, ret);
+return;
+}
+
+dma_map_sg(dbs);
 
 dbs->acb = dbs->io_func(dbs->blk, dbs->sector_num, >iov,
-dbs->iov.size / 512, dma_blk_cb, dbs);
+dbs->iov.size / 512, dma_blk_io_cb, dbs);
 assert(dbs->acb);
 }
 
@@ -190,6 +218,22 @@ static const AIOCBInfo dma_aiocb_info = {
 .cancel_async   = dma_aio_cancel,
 };
 
+/*
+ * Initialize the dbs structure and the QEMUIOVector to sane defaults.
+ */
+static void dma_init_dbs(DMAAIOCB *dbs, BlockBackend *blk, QEMUSGList *sg,
+ DMADirection dir)
+{
+dbs->acb = NULL;
+dbs->blk = blk;
+dbs->sg = sg;
+dbs->sg_cur_index = 0;
+dbs->sg_cur_byte = 0;
+dbs->dir = dir;
+dbs->bh = NULL;
+qemu_iovec_init(>iov, sg->nsg);
+}
+
 BlockAIOCB *dma_blk_io(
 BlockBackend *blk, QEMUSGList *sg, uint64_t sector_num,
 DMAIOFunc *io_func, BlockCompletionFunc *cb,
@@ -199,21 +243,14 @@ BlockAIOCB *dma_blk_io(
 
 trace_dma_blk_io(dbs, blk, sector_num, (dir == DMA_DIRECTION_TO_DEVICE));
 
-dbs->acb = NULL;
-dbs->blk = blk;
-dbs->sg = sg;
+dma_init_dbs(dbs, blk, sg, dir);
 dbs->sector_num = sector_num;
-dbs->sg_cur_index = 0;
-dbs->sg_cur_byte = 0;
-dbs->dir = dir;
 dbs->io_func = io_func;
-dbs->bh = NULL;
-qemu_iovec_init(>iov, sg->nsg);
-dma_blk_cb(dbs, 0);
+dbs->dma_cb = dma_blk_io_cb;
+dbs->dma_cb(dbs, 0);
 return >common;
 }
 
-
 BlockAIOCB *dma_blk_read(BlockBackend *blk,
  QEMUSGList *sg, uint64_t sector,
  void (*cb)(void *opaque, int ret), void *opaque)
diff --git a/trace-events b/trace-events
index 2fce98e..120cdd4 100644
--- 

[Qemu-devel] [PATCH 0/3] sdhci patches to enable Raspberry Pi

[Andrew Baumann]

This is a series of three tweaks needed to enable the generic sdhci
controller to emulate Raspberry Pi (bcm2835/2836), and boot Linux and
Windows.

There was some discussion of these changes in the following thread:
https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg01401.html

Cheers,
Andrew

Andrew Baumann (3):
  sd: sdhci: Delete over-zealous power check
  sdhci: don't raise a command index error for an unexpected response
  sdhci: add optional quirk property to disable card insertion/removal
interrupts

 hw/sd/sdhci.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

-- 
2.5.3

Re: [Qemu-devel] [PATCH v2 11/14] pc: Remove PcGuestInfo.isapc_ram_fw field

[Eduardo Habkost]

On Tue, Dec 15, 2015 at 04:27:51PM +0200, Marcel Apfelbaum wrote:
> On 12/11/2015 08:42 PM, Eduardo Habkost wrote:
[...]
> >@@ -131,8 +130,7 @@ static void pc_q35_init(MachineState *machine)
> >  rom_memory = get_system_memory();
> >  }
> >
> >-guest_info = pc_guest_info_init(pcms);
> >-guest_info->isapc_ram_fw = false;
> 
> This may not be an issue, I just want be sure.
> For Q35 isapc_ram_fw is always false, but now we are always querying
> !pcmc->pci_enabled.
> 
> Now we have a Q35 case when !pcmc->pci_enabled *can* be true.

Do we? pcmc->pci_enabled is always true in Q35.

-- 
Eduardo

Re: [Qemu-devel] [PATCH] target-*: Get rid of "PC advancement" trick

[Sergey Fedorov]

On 12/16/15 20:03, Richard Henderson wrote:

On 12/15/2015 01:02 PM, Sergey Fedorov wrote:
For that, I think it might be interesting to arrange for non-empty 
TBs to
exit prior to recognizing a breakpoint.  So that a breakpoint TB is 
always
just the one operation.  Except for the fact that "generate an 
exception" has
traditionally been a target-specific helper, we could almost make 
the entire

breakpoint generation be done in common code.

I'd think something like a generic "must we end the TB now" 
predicate would
be the proper hook.  It would contain all of the usual stuff: 
tcg_op_buf_full
and checks for singlestep, but then add "is there a breakpoint at 
the next pc".


This could be a next step :)


Or perhaps a first step, since the patch you posted doesn't seem to me 
to be an improvement at all, merely a rearrangement of code.


I should give it a thought. The main motivation of this patch was to 
simply suppress useless disas log and the instruction length mismatch 
disas warning in case of an empty TB. The original reason was that there 
is a place in ARM translation with "PC advancement" tick missed.


Thanks,
Sergey

Re: [Qemu-devel] [PATCH] target-*: Get rid of "PC advancement" trick

[Richard Henderson]

On 12/15/2015 01:02 PM, Sergey Fedorov wrote:

For that, I think it might be interesting to arrange for non-empty TBs to
exit prior to recognizing a breakpoint.  So that a breakpoint TB is always
just the one operation.  Except for the fact that "generate an exception" has
traditionally been a target-specific helper, we could almost make the entire
breakpoint generation be done in common code.

I'd think something like a generic "must we end the TB now" predicate would
be the proper hook.  It would contain all of the usual stuff: tcg_op_buf_full
and checks for singlestep, but then add "is there a breakpoint at the next pc".


This could be a next step :)


Or perhaps a first step, since the patch you posted doesn't seem to me to be an 
improvement at all, merely a rearrangement of code.



r~

Re: [Qemu-devel] [PATCH v2 4/6] Migration: Emit event at start of pass

[Dr. David Alan Gilbert]

* Eric Blake (ebl...@redhat.com) wrote:
> On 12/16/2015 04:47 AM, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" 
> > 
> > Emit an event each time we sync the dirty bitmap on the source;
> > this helps libvirt use postcopy by giving it a kick when it
> > might be a good idea to start the postcopy.
> > 
> > Signed-off-by: Dr. David Alan Gilbert 
> > ---
> >  docs/qmp-events.txt | 14 ++
> >  migration/ram.c |  4 
> >  qapi/event.json | 13 +
> >  3 files changed, 31 insertions(+)
> > 
> 
> >  
> > +MIGRATION_PASS
> > +--
> > +
> > +Emitted from the source side of a migration at the start of each pass
> > +(when it syncs the dirty bitmap)
> > +
> > +Data: None.
> > +
> > +  - "pass": An incrementing count (starting at 1 on the first pass)
> > +
> > +Example:
> > +{"timestamp": {"seconds": 1449669631, "microseconds": 239225},
> > + "event": "MIGRATION_PASS", "data": {"pass": 2}}
> 
> How frequently can this event be emitted?  It is not under guest
> control, so it's harder to argue that it could be abused as a way for
> the guest to DoS the host.  And hopefully it is slow enough (considering
> that each pass also requires lots more effort for actually transmitting
> state from source to dest), so I don't think we have to worry about
> adding rate-limiting to avoid overwhelming the monitor client.

The only way that you'd get a rapid repetition of the events
is if the pass was very short.  However a very short pass is normally
the signal to exit the iterative loop of migration and send the
last blob at the end of migration.  And that definition of 'short' is
determined by the downtime set by the management layer anyway.

> 
> Reviewed-by: Eric Blake 

Thanks.

Dave

> 
> -- 
> Eric Blake   eblake redhat com+1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 


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

Re: [Qemu-devel] [PATCH RFC] qemu-char: add an "overlay" backend type

[Paolo Bonzini]

On 16/12/2015 18:56, Daniel P. Berrange wrote:
> Introduce a new QEMU chardev backend called "overlay" which
> allows you to splice together a pair of chardev backends into
> one combined backend. The master backend permits full input/output
> but the slave backend is output only.
> 
> The primary use case for this is to allow arbitrary backends to
> have their data logged to a file, eg a 'file' backend would be
> setup as the slave.
> 
>  $ qemu-system-x86_64 \
>  -chardev socket,host=localhost,port=9000,server=on,nowait,id=char0master 
> \
>  -chardev file,path=/some/log/file.log,id=char0slave \
>  -chardev overlay,id=char0,master=char0master,slave=char0slave  \
>  -device isa-serial,chardev=char0 \
>  ...other args...
> ---
> 
> This idea was suggsted in
> 
>   https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg01256.html
> 
> this patch is a very quick proof of concept impl to illustrate the
> idea.

Hmm, I was a fan of the "do it outside QEMU" idea... It would also fix
the issue you have with qemu_chr_fe_write_all...

Paolo

Re: [Qemu-devel] [PATCH v2 0/3] KVM-UNIT-TESTS: Hyper-V SynIC timers test

[Denis V. Lunev]

On 12/08/2015 07:36 PM, Andrey Smetanin wrote:

The test checks Hyper-V SynIC timers functionality.
The test runs on every vCPU and performs start/stop
of periodic/one-shot timers (with period=1ms) and checks
validity of received expiration messages in appropriate
ISR's.

Changes v2:
* Share generic Hyper-V tests code
* Hyper-V SynIC timers test fixes to improve
readability and output

Signed-off-by: Andrey Smetanin 
Reviewed-by: Roman Kagan 
CC: Paolo Bonzini 
CC: Marcelo Tosatti 
CC: Roman Kagan 
CC: Denis V. Lunev 
CC: qemu-devel@nongnu.org

Andrey Smetanin (3):
   lib/x86: Make free_page() available to call
   x86/hyperv: Move Hyper-V generic code into hyperv.h/hyperv.c
   x86: Hyper-V SynIC timers test

  config/config-x86-common.mak |   8 +-
  lib/x86/msr.h|  23 ---
  lib/x86/vm.h |   1 +
  x86/hyperv.c |  24 +++
  x86/hyperv.h | 183 +
  x86/hyperv_stimer.c  | 376 +++
  x86/hyperv_synic.c   |  42 +
  x86/unittests.cfg|   5 +
  8 files changed, 603 insertions(+), 59 deletions(-)
  create mode 100644 x86/hyperv.c
  create mode 100644 x86/hyperv.h
  create mode 100644 x86/hyperv_stimer.c


ping :)

Re: [Qemu-devel] RFC: Operation Blockers in QEMU Block Nodes

[Jeff Cody]

On Wed, Dec 16, 2015 at 09:31:26AM -0700, Eric Blake wrote:
> On 12/15/2015 11:25 PM, Jeff Cody wrote:
> > Background:
> > 
> > Block jobs, and other QAPI operations, may modify and impact the
> > BlockDriverState graph in QEMU.  In order to support multiple
> > operations safely, we need a mechanism to block and gate operations,
> > 
> > We currently have op blockers, that are attached to each BDS.
> > However, in practice we check this on the device level, rather than on
> > the granularity of the individual BDS.  Also, due to limitations of
> > the current system, we only allow a single block job at a time.
> > 
> > 
> > Proposed New Design Features:
> > --
> > This design would supersede the current op blocker system.
> > 
> > Rather than have the op blockers attached directly to each BDS, Block
> > Job access will be done through a separate Node Access Control system.
> > 
> > This new system will:
> > 
> > * Allow / Disallow operations to BDSs, (generally initiated by QAPI
> >   actions; i.e. BDS node operations other than guest read/writes)
> 
> Doesn't suspending/resuming the guest count as a case where guest
> read/writes affect allowed operations?  That is, a VM in state RUNNING
> counts as one that Requires read-data and write-data, and once it
> transitions to paused, the Requires can go away.
>

That's a good question.  I'm not sure, and I tend to think of the VM
guest as a special case, but I don't know that everyone agrees with
that.

My reasoning on why it is special can be summed up as: the
guest is concerned with the _device_ (i.e. BlockBackend), rather than
the BDS nodes (which are invisible to the guest).

For instance, if the guest had Require/Allow flags set, when we do a
live snapshot, those flags wouldn't necessarily apply to the BDS, but
we would want those to move to the active layer again (but not really,
because again we are concerned with the BlockBackend device moreso
than the BDS).

So I think if we wanted to encapsulate the guest, rather than trying
to do it vis-a-vis the active layer, we should extend this proposed
NAC scheme to also have a BBAC (BlockBackend Access Control) layer as
well.

> > 
> > * Not be limited to "block jobs" (e.g. block-commit, block-stream,
> >   etc..) - will also apply to other operations, such as
> >   blockdev-add, change-backing-file, etc.
> > 
> > * Allow granularity in options, and provide the safety needed to
> >   support multiple block jobs and operations.
> > 
> > * Reference each BDS by its node-name
> > 
> > * Be independent of the bdrv_states graph (i.e. does not reside in
> >   the BDS structure)
> > 
> > 
> > Node Access Control: Jobs
> > --
> > Every QAPI/HMP initiated operation is considered a "job", regardless
> > if it is implemented via coroutines (e.g. block jobs such as
> > block-commit), or handled synchronously in a QAPI handler (e.g.
> > change-backing-file, blockdev-add).
> > 
> > A job consists of:
> > * Unique job ID
> > * A list of all node-names affected by the operation
> 
> Do we need to hold some sort of lock when computing the list of
> node-names to be affected, since some of the very operations involved
> are those that would change the set of node-names impacted? That is, no
> operation can change the graph while another operation is collecting the
> list of nodes to be tied to a job.
> 

Yes, good point, I think you are right: a block-level global mutex of
sorts would be needed here.  I think we could get away with this being
around graph reconfiguration, and computing the list of node-names
affected by an operation.

> > * Action flags (see below) for each node in the above list
> > 
> > 
> > Node Access Control: Action Flags
> > ---
> > Every operation must set action flag pairs, for every node affected by
> > the operation.
> > 
> > Action flags are set as a Require/Allow pair.  If the Require
> > flag is set, then the operation requires the ability to take the
> > specific action.  If the Allow flag is set, then the operation will
> > allow other operations to perform same action in parallel.
> > 
> > The default is to prohibit, not allow, parallel actions.
> > 
> > The proposed actions are:
> > 
> > 1. Modify - Visible Data
> > * This is modification that would be detected by an external
> >   read (for instance, a hypothetical qemu-io read from the
> >   specific node), inclusive of its backing files.  A classic
> >   example would be writing data into another node, as part of
> >   block-commit.
> > 
> > 2. Modify - Metadata
> > * This is a write that is not necessarily visible to an external
> >   read, but still modifies the underlying image format behind the
> >   node.  I.e., change-backing-file to an identical backing file.
> >   (n.b.: if changing the backing file to a 

[Qemu-devel] [PATCH 3/5] target-i386: kvm: Simplify MSR array construction

[Eduardo Habkost]

Add a helper function that appends new entries to the MSR buffer
and checks for the buffer size limit.

Signed-off-by: Eduardo Habkost 
---
 target-i386/kvm.c | 262 ++
 1 file changed, 125 insertions(+), 137 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 1e82400..ada484f 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1359,6 +1359,18 @@ static void kvm_msr_buf_reset(X86CPU *cpu)
 memset(cpu->kvm_msr_buf, 0, MSR_BUF_SIZE);
 }
 
+static void kvm_msr_entry_add(X86CPU *cpu, uint32_t index, uint64_t value)
+{
+struct kvm_msrs *msrs = cpu->kvm_msr_buf;
+void *limit = ((void*)msrs) + MSR_BUF_SIZE;
+struct kvm_msr_entry *entry = >entries[msrs->nmsrs];
+
+assert((void*)(entry + 1) <= limit);
+
+kvm_msr_entry_set(entry, index, value);
+msrs->nmsrs++;
+}
+
 static int kvm_put_tscdeadline_msr(X86CPU *cpu)
 {
 CPUX86State *env = >env;
@@ -1407,46 +1419,45 @@ static int kvm_put_msr_feature_control(X86CPU *cpu)
 static int kvm_put_msrs(X86CPU *cpu, int level)
 {
 CPUX86State *env = >env;
-struct kvm_msr_entry *msrs = cpu->kvm_msr_buf->entries;
-int n = 0, i;
+int i;
 
 kvm_msr_buf_reset(cpu);
 
-kvm_msr_entry_set([n++], MSR_IA32_SYSENTER_CS, env->sysenter_cs);
-kvm_msr_entry_set([n++], MSR_IA32_SYSENTER_ESP, env->sysenter_esp);
-kvm_msr_entry_set([n++], MSR_IA32_SYSENTER_EIP, env->sysenter_eip);
-kvm_msr_entry_set([n++], MSR_PAT, env->pat);
+kvm_msr_entry_add(cpu, MSR_IA32_SYSENTER_CS, env->sysenter_cs);
+kvm_msr_entry_add(cpu, MSR_IA32_SYSENTER_ESP, env->sysenter_esp);
+kvm_msr_entry_add(cpu, MSR_IA32_SYSENTER_EIP, env->sysenter_eip);
+kvm_msr_entry_add(cpu, MSR_PAT, env->pat);
 if (has_msr_star) {
-kvm_msr_entry_set([n++], MSR_STAR, env->star);
+kvm_msr_entry_add(cpu, MSR_STAR, env->star);
 }
 if (has_msr_hsave_pa) {
-kvm_msr_entry_set([n++], MSR_VM_HSAVE_PA, env->vm_hsave);
+kvm_msr_entry_add(cpu, MSR_VM_HSAVE_PA, env->vm_hsave);
 }
 if (has_msr_tsc_aux) {
-kvm_msr_entry_set([n++], MSR_TSC_AUX, env->tsc_aux);
+kvm_msr_entry_add(cpu, MSR_TSC_AUX, env->tsc_aux);
 }
 if (has_msr_tsc_adjust) {
-kvm_msr_entry_set([n++], MSR_TSC_ADJUST, env->tsc_adjust);
+kvm_msr_entry_add(cpu, MSR_TSC_ADJUST, env->tsc_adjust);
 }
 if (has_msr_misc_enable) {
-kvm_msr_entry_set([n++], MSR_IA32_MISC_ENABLE,
+kvm_msr_entry_add(cpu, MSR_IA32_MISC_ENABLE,
   env->msr_ia32_misc_enable);
 }
 if (has_msr_smbase) {
-kvm_msr_entry_set([n++], MSR_IA32_SMBASE, env->smbase);
+kvm_msr_entry_add(cpu, MSR_IA32_SMBASE, env->smbase);
 }
 if (has_msr_bndcfgs) {
-kvm_msr_entry_set([n++], MSR_IA32_BNDCFGS, env->msr_bndcfgs);
+kvm_msr_entry_add(cpu, MSR_IA32_BNDCFGS, env->msr_bndcfgs);
 }
 if (has_msr_xss) {
-kvm_msr_entry_set([n++], MSR_IA32_XSS, env->xss);
+kvm_msr_entry_add(cpu, MSR_IA32_XSS, env->xss);
 }
 #ifdef TARGET_X86_64
 if (lm_capable_kernel) {
-kvm_msr_entry_set([n++], MSR_CSTAR, env->cstar);
-kvm_msr_entry_set([n++], MSR_KERNELGSBASE, env->kernelgsbase);
-kvm_msr_entry_set([n++], MSR_FMASK, env->fmask);
-kvm_msr_entry_set([n++], MSR_LSTAR, env->lstar);
+kvm_msr_entry_add(cpu, MSR_CSTAR, env->cstar);
+kvm_msr_entry_add(cpu, MSR_KERNELGSBASE, env->kernelgsbase);
+kvm_msr_entry_add(cpu, MSR_FMASK, env->fmask);
+kvm_msr_entry_add(cpu, MSR_LSTAR, env->lstar);
 }
 #endif
 /*
@@ -1454,106 +1465,89 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
  * for normal writeback. Limit them to reset or full state updates.
  */
 if (level >= KVM_PUT_RESET_STATE) {
-kvm_msr_entry_set([n++], MSR_IA32_TSC, env->tsc);
-kvm_msr_entry_set([n++], MSR_KVM_SYSTEM_TIME,
-  env->system_time_msr);
-kvm_msr_entry_set([n++], MSR_KVM_WALL_CLOCK, env->wall_clock_msr);
+kvm_msr_entry_add(cpu, MSR_IA32_TSC, env->tsc);
+kvm_msr_entry_add(cpu, MSR_KVM_SYSTEM_TIME, env->system_time_msr);
+kvm_msr_entry_add(cpu, MSR_KVM_WALL_CLOCK, env->wall_clock_msr);
 if (has_msr_async_pf_en) {
-kvm_msr_entry_set([n++], MSR_KVM_ASYNC_PF_EN,
-  env->async_pf_en_msr);
+kvm_msr_entry_add(cpu, MSR_KVM_ASYNC_PF_EN, env->async_pf_en_msr);
 }
 if (has_msr_pv_eoi_en) {
-kvm_msr_entry_set([n++], MSR_KVM_PV_EOI_EN,
-  env->pv_eoi_en_msr);
+kvm_msr_entry_add(cpu, MSR_KVM_PV_EOI_EN, env->pv_eoi_en_msr);
 }
 if (has_msr_kvm_steal_time) {
-kvm_msr_entry_set([n++], MSR_KVM_STEAL_TIME,
-  env->steal_time_msr);
+kvm_msr_entry_add(cpu, MSR_KVM_STEAL_TIME, 

[Qemu-devel] [PATCH 1/5] target-i386: kvm: Allocate kvm_msrs struct once per VCPU

[Eduardo Habkost]

Instead of using 2400 bytes in the stack for 150 MSR entries in
kvm_get_msrs() and kvm_put_msrs(), allocate a buffer once for
each VCPU.

Signed-off-by: Eduardo Habkost 
---
 target-i386/cpu-qom.h |  4 
 target-i386/kvm.c | 37 +++--
 2 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/target-i386/cpu-qom.h b/target-i386/cpu-qom.h
index e3bfe9d..f349b30 100644
--- a/target-i386/cpu-qom.h
+++ b/target-i386/cpu-qom.h
@@ -69,6 +69,8 @@ typedef struct X86CPUClass {
 void (*parent_reset)(CPUState *cpu);
 } X86CPUClass;
 
+struct kvm_msrs;
+
 /**
  * X86CPU:
  * @env: #CPUX86State
@@ -119,6 +121,8 @@ typedef struct X86CPU {
 struct DeviceState *apic_state;
 struct MemoryRegion *cpu_as_root, *cpu_as_mem, *smram;
 Notifier machine_done;
+
+struct kvm_msrs *kvm_msr_buf;
 } X86CPU;
 
 static inline X86CPU *x86_env_get_cpu(CPUX86State *env)
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 6dc9846..660b2d9 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -52,6 +52,9 @@
 #define MSR_KVM_WALL_CLOCK  0x11
 #define MSR_KVM_SYSTEM_TIME 0x12
 
+#define MSR_BUF_SIZE \
+(sizeof(struct kvm_msrs) + 150 * sizeof(struct kvm_msr_entry))
+
 #ifndef BUS_MCEERR_AR
 #define BUS_MCEERR_AR 4
 #endif
@@ -841,6 +844,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
 if (has_xsave) {
 env->kvm_xsave_buf = qemu_memalign(4096, sizeof(struct kvm_xsave));
 }
+cpu->kvm_msr_buf = g_malloc0(MSR_BUF_SIZE);
 
 if (env->features[FEAT_1_EDX] & CPUID_MTRR) {
 has_msr_mtrr = true;
@@ -1349,6 +1353,11 @@ static void kvm_msr_entry_set(struct kvm_msr_entry 
*entry,
 entry->data = value;
 }
 
+static void kvm_msr_buf_reset(X86CPU *cpu)
+{
+memset(cpu->kvm_msr_buf, 0, MSR_BUF_SIZE);
+}
+
 static int kvm_put_tscdeadline_msr(X86CPU *cpu)
 {
 CPUX86State *env = >env;
@@ -1397,13 +1406,11 @@ static int kvm_put_msr_feature_control(X86CPU *cpu)
 static int kvm_put_msrs(X86CPU *cpu, int level)
 {
 CPUX86State *env = >env;
-struct {
-struct kvm_msrs info;
-struct kvm_msr_entry entries[150];
-} msr_data;
-struct kvm_msr_entry *msrs = msr_data.entries;
+struct kvm_msr_entry *msrs = cpu->kvm_msr_buf->entries;
 int n = 0, i;
 
+kvm_msr_buf_reset(cpu);
+
 kvm_msr_entry_set([n++], MSR_IA32_SYSENTER_CS, env->sysenter_cs);
 kvm_msr_entry_set([n++], MSR_IA32_SYSENTER_ESP, env->sysenter_esp);
 kvm_msr_entry_set([n++], MSR_IA32_SYSENTER_EIP, env->sysenter_eip);
@@ -1562,11 +1569,9 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
 }
 }
 
-msr_data.info = (struct kvm_msrs) {
-.nmsrs = n,
-};
+cpu->kvm_msr_buf->nmsrs = n;
 
-return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, _data);
+return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, cpu->kvm_msr_buf);
 
 }
 
@@ -1775,13 +1780,11 @@ static int kvm_get_sregs(X86CPU *cpu)
 static int kvm_get_msrs(X86CPU *cpu)
 {
 CPUX86State *env = >env;
-struct {
-struct kvm_msrs info;
-struct kvm_msr_entry entries[150];
-} msr_data;
-struct kvm_msr_entry *msrs = msr_data.entries;
+struct kvm_msr_entry *msrs = cpu->kvm_msr_buf->entries;
 int ret, i, n;
 
+kvm_msr_buf_reset(cpu);
+
 n = 0;
 msrs[n++].index = MSR_IA32_SYSENTER_CS;
 msrs[n++].index = MSR_IA32_SYSENTER_ESP;
@@ -1904,11 +1907,9 @@ static int kvm_get_msrs(X86CPU *cpu)
 }
 }
 
-msr_data.info = (struct kvm_msrs) {
-.nmsrs = n,
-};
+cpu->kvm_msr_buf->nmsrs = n;
 
-ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MSRS, _data);
+ret = kvm_vcpu_ioctl(CPU(cpu), KVM_GET_MSRS, cpu->kvm_msr_buf);
 if (ret < 0) {
 return ret;
 }
-- 
2.1.0

[Qemu-devel] [PATCH RFC] qemu-char: add an "overlay" backend type

[Daniel P. Berrange]

Introduce a new QEMU chardev backend called "overlay" which
allows you to splice together a pair of chardev backends into
one combined backend. The master backend permits full input/output
but the slave backend is output only.

The primary use case for this is to allow arbitrary backends to
have their data logged to a file, eg a 'file' backend would be
setup as the slave.

 $ qemu-system-x86_64 \
 -chardev socket,host=localhost,port=9000,server=on,nowait,id=char0master \
 -chardev file,path=/some/log/file.log,id=char0slave \
 -chardev overlay,id=char0,master=char0master,slave=char0slave  \
 -device isa-serial,chardev=char0 \
 ...other args...
---

This idea was suggsted in

  https://lists.gnu.org/archive/html/qemu-devel/2015-12/msg01256.html

this patch is a very quick proof of concept impl to illustrate the
idea.

 qapi-schema.json |  14 +
 qemu-char.c  | 152 +++
 2 files changed, 166 insertions(+)

diff --git a/qapi-schema.json b/qapi-schema.json
index 4be151c..1adb220 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3156,6 +3156,19 @@
 { 'struct': 'ChardevRingbuf', 'data': { '*size'  : 'int' } }
 
 ##
+# @ChardevOverlay:
+#
+# Configuration info for overlay chardevs.
+#
+# @master: the master chardev for input / output
+# @slave: the slave chardev for output only
+#
+# Since: 2.6
+##
+{ 'struct': 'ChardevOverlay', 'data': { 'master' : 'str',
+'slave'  : 'str'} }
+
+##
 # @ChardevBackend:
 #
 # Configuration info for the new chardev backend.
@@ -3182,6 +3195,7 @@
'spiceport' : 'ChardevSpicePort',
'vc' : 'ChardevVC',
'ringbuf': 'ChardevRingbuf',
+   'overlay': 'ChardevOverlay',
# next one is just for compatibility
'memory' : 'ChardevRingbuf' } }
 
diff --git a/qemu-char.c b/qemu-char.c
index 45fdb77..0328eb9 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3223,6 +3223,131 @@ char *qmp_ringbuf_read(const char *device, int64_t size,
 return data;
 }
 
+
+typedef struct {
+CharDriverState *master;
+CharDriverState *slave;
+
+IOCanReadHandler *chr_can_read;
+IOReadHandler *chr_read;
+IOEventHandler *chr_event;
+void *handler_opaque;
+} OverlayCharDriver;
+
+static int overlay_chr_write(CharDriverState *chr, const uint8_t *buf, int len)
+{
+OverlayCharDriver *s = chr->opaque;
+int ret;
+
+ret = qemu_chr_fe_write(s->master, buf, len);
+if (ret <= 0) {
+return ret;
+}
+
+/* Write all will block, but how else can we ensure
+ * that all the data we just sent to the master also
+ * gets sent to the slave ? Could buffer data internally
+ * if slave was full and setup a backgroupd I/O watch
+ * to send the data later, but the ChardevDriverState
+ * API design doesn't make that very easy since. Of
+ * course if the slave is a "file" backend, then it
+ * will block regardless thanks to POSIX not handling
+ * O_NONBLOCK on regular files, so this is no worse
+ * in that scenario.
+ */
+qemu_chr_fe_write_all(s->slave, buf, ret);
+return ret;
+}
+
+static GSource *overlay_chr_add_watch(CharDriverState *chr, GIOCondition cond)
+{
+OverlayCharDriver *d = chr->opaque;
+return d->master->chr_add_watch(d->master, cond);
+}
+
+static int overlay_chr_can_read(void *opaque)
+{
+CharDriverState *chr = opaque;
+OverlayCharDriver *d = chr->opaque;
+
+if (d->chr_can_read) {
+return d->chr_can_read(d->handler_opaque);
+}
+return 0;
+}
+
+static void overlay_chr_read(void *opaque, const uint8_t *buf, int size)
+{
+CharDriverState *chr = opaque;
+OverlayCharDriver *d = chr->opaque;
+
+if (d->chr_read) {
+d->chr_read(d->handler_opaque, buf, size);
+}
+}
+
+static void overlay_chr_event(void *opaque, int event)
+{
+CharDriverState *chr = opaque;
+OverlayCharDriver *d = chr->opaque;
+
+if (d->chr_event) {
+d->chr_event(d->handler_opaque, event);
+}
+}
+
+static void overlay_chr_update_read_handler(CharDriverState *chr)
+{
+OverlayCharDriver *d = chr->opaque;
+
+d->handler_opaque = chr->handler_opaque;
+d->chr_can_read = chr->chr_can_read;
+d->chr_read = chr->chr_read;
+d->chr_event = chr->chr_event;
+
+qemu_chr_add_handlers(d->master, overlay_chr_can_read, overlay_chr_read,
+  overlay_chr_event, chr);
+/* Slave is output only, so don't wire it up for input */
+}
+
+static CharDriverState *qemu_chr_open_overlay(const char *id,
+  ChardevBackend *backend,
+  ChardevReturn *ret, Error **errp)
+{
+ChardevOverlay *overlay = backend->u.overlay;
+

Re: [Qemu-devel] [PATCH 0/9] Add full scatter-gather support for SCSI generic devices

[Paolo Bonzini]

On 16/12/2015 17:55, Alex Pyrgiotis wrote:
> Hi all,
> 
> This patch is an attempt to boost the performance of "scsi-generic" and
> "scsi-block" device types, by removing an extra data copy and reducing
> their memory footprint. More specifically, the problem lies in the
> functions in the `scsi-generic_req_ops` struct of scsi-generic.c. These
> functions rely on an intermediate buffer to do the SG_IO ioctl request,
> without checking if the SCSI controller has provided a scatter-gather
> list with the request.
> 
> In a nutshell, our proposal is to map the provided scatter-gather list
> (if any) to the address space of the QEMU process and use the resulting
> iovec as the buffer for the ioctl request. You'll find that the logic is
> quite similar to the one used in scsi-disk.c.

Which commands have large payloads and are on the data path, for
scsi-block?  Or is the use case just scsi-generic (e.g. tape devices?)?

(Just trying to understand before I dive into the patches).

Paolo

Re: [Qemu-devel] [RFC PATCH v0 0/9] Generic cpu-core device

[Igor Mammedov]

On Wed, 16 Dec 2015 16:57:54 +0100
Andreas Färber  wrote:

> Am 16.12.2015 um 16:44 schrieb Igor Mammedov:
> > On Wed, 16 Dec 2015 16:19:06 +0100
> > Andreas Färber  wrote:
> > 
> >> Am 10.12.2015 um 07:15 schrieb Bharata B Rao:
> >>> CPU hotplug granularity
> >>> ---
> >>> CPU hotplug will now be done in cpu-core device granularity.
> >>
> >> Nack.
> >>
> >>> Are there archs that would need thread level CPU addition ?
> >>
> >> Yes, s390. And for x86 people called for socket level.
> > socket level hotplug would be the last resort if we can't agree
> > on thread level one. As it would break existing setups where
> > user can hotplug 1 core, and  I'd like to avoid it if it is
> > possible.
> 
> We still need to keep cpu-add for backwards compatibility, so I am
> discussing solely the new device_add interface. My previous x86 series
> went to severe hacks trying to keep cpu-add working with
> sockets
if possible, it would be better to make cpu-add to use device_add
internally.

> 
> Attendees in Seattle said that thread-level hot-plug were dangerous
> for Linux guests due to assumptions in the (guest's) scheduler
> breaking for any incompletely filled cores or sockets. No one present
There is not such thing as cpu hotplug at socket level in x86 linux so far.
CPUs are plugged at logical(thread) cpu level, one at a time.
And ACPI spec does the same (describes logical CPUs) and hotplug
notification in guest handled per one logical cpu at a time.

> objected to doing it on socket level. Bharata(?) had a recent patch
> to catch such incompletely filled cores on the initial command line
> and I think we should seriously consider doing that even if it breaks
> some fringe use case - hot-added incomplete cores or sockets remain
> to be detected.
interesting question, what MADT/SRAT tables would look like on machine
with Intel CPU when hyperthreading is disabled in BIOS? 
 
> 
> Regards,
> Andreas
> 

Re: [Qemu-devel] Rationalising exit_request, cpu->exit_request and tcg_exit_req?

[Paolo Bonzini]

On 16/12/2015 18:14, Alex Bennée wrote:
> 
> While looking at Fred's current MTTCG WIP branch I ran into a problem
> where:
> 
>   - async_safe_work_pending() was true
>   - this triggered setting cpu->exit_request
>   - however we never left tcg_exec_all()
>   - because the global exit_request wasn't set
>   - hence qemu_tcg_wait_io_event() never drained the async work queue

exit_request should disappear with MTTCG. It should only have
cpu->tcg_exit_req and cpu->exit_request.

> While trying to understand why we have both a cpu and a global
> exit_request I then discovered there is also cpu->tcg_exit_req which is
> the actual variable the TCG examines. This leads to sequences like:

> This seems to me to be slightly insane as we now have 3 variables that
> struggle to be kept in sync. Could all this not be rationalised into a
> single variable or am I missing a subtly in their different semantics?

They do.

cpu->tcg_exit_req is the one that is set from generated code.  It is set
if you do not have to exit cpu_exec.

cpu->exit_request and exit_request are both necessary, in order to
synchronize exits with the setting of tcg_current_cpu.  Again, this is
only needed in single-threaded TCG, because MTTCG gets rid of
tcg_current_cpu.  It's documented here:
http://article.gmane.org/gmane.comp.emulators.qemu/357210.

> I don't know if there is clean-up that can happen in master or if this
> all needs to be done in the mttcg work but would it make sense just to
> keep cpu->exit_request,

Yes, and it's actually necessary. :)

> I did have a brief look at the KVM side of the code and it only seems to
> reference cpu->exit_request so I think the rest of this is a TCG
> problem.

Yes.  With MTTCG you still need cpu->tcg_exit_req because you still have
something like KVM's kernel- and userspace-vmexits.  In TCG the
lightweight exits are those that do not leave cpu_exec.  Those only set
cpu->tcg_exit_req.

Paolo

[Qemu-devel] Rationalising exit_request, cpu->exit_request and tcg_exit_req?

[Alex Bennée]

Hi,

While looking at Fred's current MTTCG WIP branch I ran into a problem
where:

  - async_safe_work_pending() was true
  - this triggered setting cpu->exit_request
  - however we never left tcg_exec_all()
  - because the global exit_request wasn't set
  - hence qemu_tcg_wait_io_event() never drained the async work queue

While trying to understand why we have both a cpu and a global
exit_request I then discovered there is also cpu->tcg_exit_req which is
the actual variable the TCG examines. This leads to sequences like:

void cpu_exit(CPUState *cpu)
{
cpu->exit_request = 1;
/* Ensure cpu_exec will see the exit request after TCG has exited.  */
smp_wmb();
cpu->tcg_exit_req = 1;
}

which itself is amusingly called from:

static void qemu_cpu_kick_no_halt(void)
{
CPUState *cpu;
/* Ensure whatever caused the exit has reached the CPU threads before
 * writing exit_request.
 */
atomic_mb_set(_request, 1);
cpu = atomic_mb_read(_current_cpu);
if (cpu) {
cpu_exit(cpu);
}
}

This seems to me to be slightly insane as we now have 3 variables that
struggle to be kept in sync. Could all this not be rationalised into a
single variable or am I missing a subtly in their different semantics?

One problem I can think of when we get to the MTTCG world is a race when
signalling other CPUs to exit and making sure that request is not
dropped as we clear an old exit_request.

The other complication is the main cpu_exec loop which works hard to
avoid leaving the main loop when processing interrupts (which require
an exit_request to trigger). This means there a potentially multiple
places where exit_requests are drained.

I don't know if there is clean-up that can happen in master or if this
all needs to be done in the mttcg work but would it make sense just to
keep cpu->exit_request, make it visible to the TCG code and make all
exits fall out to qemu_tcg_cpu_thread_fn which would be the only place
to clear the flag?

I did have a brief look at the KVM side of the code and it only seems to
reference cpu->exit_request so I think the rest of this is a TCG
problem.

Thoughts?

--
Alex Bennée

Re: [Qemu-devel] -loadvm and disk only snapshot

[Vasiliy Tolstov]

2015-12-16 19:19 GMT+03:00 Eric Blake :
> Won't work (qemu is not able to load disk snapshots without memory).
> What libvirt does instead is to use qemu-img snapshot -c to change the
> snapshot back to the active layer, then boot qemu fresh on the correct
> contents.
>

qemu-img snapshot -a ? as i see -c creates new snapshot.

> Of course, patches to change behavior aren't out of the question, but
> there's already a lot of cruft there to be aware of, and making sure we
> don't regress libvirt behavior.

My needed use case - create multilayered qcow2 image. Base layer -
clean fresh (for example debian) system, next layer - LAMP, next layer
RAILS (base on top of clean debian system) and so on.
I want to create images from packer and want to write packer plugin
for this case (installer that able to read from qcow2 file i create
later).

Does it possible with qemu-img snapshots with my use case? I don't
want to snapshot memory because i don't need it. To get consistent
snapshots i can sync disk or freeze fs by ioctl.




-- 
Vasiliy Tolstov,
e-mail: v.tols...@selfip.ru

Re: [Qemu-devel] [RFC PATCH 00/40] Sneak peek of virtio and dataplane changes for 2.6

[Paolo Bonzini]

On 16/12/2015 13:54, Christian Borntraeger wrote:
> Just some quick remarks before I leave into vacation:
> 
> Performance seems to be better than the initial version. I have some
> hangs from time to time when shutting down (also with your previous
> version)

Yes, I've seen them too.  To reproduce, just start some "dd" and
repeatedly stop/cont.  It hangs after 3-4 tries typically.

It's like this:

I/O thread  main thread
-
aio_context_acquire
bdrv_drain
bh->scheduled = 0
call bottom half
   aio_context_acquire
   


The bottom half is hung in the I/O thread, and will never be processed
by the main thread because bh->scheduled = 0.

My current idea is that *all* callers of aio_poll have to call it
without the AioContext acquired, including bdrv_drain.  It's not a
problem now that Fam has fixed almost all bdrv_drain users to use
bdrv_drained_begin/end instead.

The nastiness here is two-fold:

1) doing

aio_context_release
aio_poll
aio_context_acquire

in bdrv_drain is not very safe because the mutex is recursive.  (Have I
ever mentioned I don't like recursive mutexes?...)

2) there is still a race in bdrv_drain

bdrv_flush_io_queue(bs);
if (bdrv_requests_pending(bs)) {
busy = true;
aio_context_release(aio_context);
aio_poll(aio_context, busy);
aio_context_acquire(aio_context);

if we release before aio_poll, the last request can be completed
before aio_poll is invoked, and aio_poll will block forever.  In Linux
terms, bdrv_request_pending would be prepare_to_wait() and aio_poll
would be schedule().  But, we don't have prepare_to_wait().  I have a
hackish solution here, but I don't like it really.

Paolo

[Qemu-devel] [PATCH] block: use drained section around bdrv_snapshot_delete

[Paolo Bonzini]

Do not use bdrv_drain, since by itself it does not guarantee
anything.

Signed-off-by: Paolo Bonzini 
---
 block/snapshot.c | 23 ++-
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/block/snapshot.c b/block/snapshot.c
index 6e9fa8d..2d86b88 100644
--- a/block/snapshot.c
+++ b/block/snapshot.c
@@ -229,6 +229,8 @@ int bdrv_snapshot_delete(BlockDriverState *bs,
  Error **errp)
 {
 BlockDriver *drv = bs->drv;
+int ret;
+
 if (!drv) {
 error_setg(errp, QERR_DEVICE_HAS_NO_MEDIUM, bdrv_get_device_name(bs));
 return -ENOMEDIUM;
@@ -239,18 +241,21 @@ int bdrv_snapshot_delete(BlockDriverState *bs,
 }
 
 /* drain all pending i/o before deleting snapshot */
-bdrv_drain(bs);
+bdrv_drained_begin(bs);
 
 if (drv->bdrv_snapshot_delete) {
-return drv->bdrv_snapshot_delete(bs, snapshot_id, name, errp);
+ret = drv->bdrv_snapshot_delete(bs, snapshot_id, name, errp);
+} else if (bs->file) {
+ret = bdrv_snapshot_delete(bs->file->bs, snapshot_id, name, errp);
+} else {
+error_setg(errp, "Block format '%s' used by device '%s' "
+   "does not support internal snapshot deletion",
+   drv->format_name, bdrv_get_device_name(bs));
+ret = -ENOTSUP;
 }
-if (bs->file) {
-return bdrv_snapshot_delete(bs->file->bs, snapshot_id, name, errp);
-}
-error_setg(errp, "Block format '%s' used by device '%s' "
-   "does not support internal snapshot deletion",
-   drv->format_name, bdrv_get_device_name(bs));
-return -ENOTSUP;
+
+bdrv_drained_end(bs);
+return ret;
 }
 
 int bdrv_snapshot_delete_by_id_or_name(BlockDriverState *bs,
-- 
2.5.0

[Qemu-devel] [PATCH] block: fix bdrv_ioctl called from coroutine

[Paolo Bonzini]

When called from a coroutine, bdrv_ioctl must be asynchronous just like
e.g. bdrv_flush.  The code was incorrectly making it synchronous, fix
it.

Signed-off-by: Paolo Bonzini 
---
Fam, any reason why you did it this way?  I don't see
any coroutine caller, but it doesn't make much sense. :)

 block/io.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/block/io.c b/block/io.c
index e00fb5d..841f5b5 100644
--- a/block/io.c
+++ b/block/io.c
@@ -2614,10 +2614,11 @@ int bdrv_ioctl(BlockDriverState *bs, unsigned long int 
req, void *buf)
 bdrv_co_ioctl_entry();
 } else {
 Coroutine *co = qemu_coroutine_create(bdrv_co_ioctl_entry);
+
 qemu_coroutine_enter(co, );
-}
-while (data.ret == -EINPROGRESS) {
-aio_poll(bdrv_get_aio_context(bs), true);
+while (data.ret == -EINPROGRESS) {
+aio_poll(bdrv_get_aio_context(bs), true);
+}
 }
 return data.ret;
 }
-- 
2.5.0

[Qemu-devel] [PATCH] scsi: always call notifier on async cancellation

[Paolo Bonzini]

This was found by code inspection.  If the request is cancelled twice,
the notifier is never called on the second cancellation request,
and hence for example a TMF might never finish.

Signed-off-by: Paolo Bonzini 
---
 hw/scsi/scsi-bus.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c
index 524a998..4c121fe 100644
--- a/hw/scsi/scsi-bus.c
+++ b/hw/scsi/scsi-bus.c
@@ -1759,9 +1759,6 @@ void scsi_req_cancel_async(SCSIRequest *req, Notifier 
*notifier)
 if (notifier) {
 notifier_list_add(>cancel_notifiers, notifier);
 }
-if (req->io_canceled) {
-return;
-}
 scsi_req_ref(req);
 scsi_req_dequeue(req);
 req->io_canceled = true;
-- 
2.5.0

[Qemu-devel] [PATCH 0/5] target-i386: kvm: Increase MSR entry array limits, check for array overrun

[Eduardo Habkost]

We are dangerously close to the array limits in kvm_put_msrs()
and kvm_get_msrs(): with the default mcg_cap configuration, we
can set up to 148 MSRs in kvm_put_msrs(), and if we allow mcg_cap
to be changed, we can write up to 236 MSRs[1].

This series changes the code to allocate a buffer once per VCPU,
increase buffer size to 4096 bytes (that can hold up to 255 MSR
entries), and check array limits before appending new entries.

[1] I have checked the limits by copying and pasting the
kvm_put_msrs() code to a new file, replacing the "if" lines,
copying the macro definitions, and adding a helper macro to
keep track of the kvm_msr_entry_set() calls. The code can be
seen at:
https://gist.github.com/ehabkost/08d4177a33b8648a71ef

Eduardo Habkost (5):
  target-i386: kvm: Allocate kvm_msrs struct once per VCPU
  target-i386: kvm: Increase MSR_BUF_SIZE
  target-i386: kvm: Simplify MSR array construction
  target-i386: kvm: Simplify MSR setting functions
  target-i386: kvm: Eliminate kvm_msr_entry_set()

 target-i386/cpu-qom.h |   4 +
 target-i386/kvm.c | 322 +++---
 2 files changed, 149 insertions(+), 177 deletions(-)

-- 
2.1.0

Re: [Qemu-devel] -loadvm and disk only snapshot

[Eric Blake]

On 12/16/2015 10:25 AM, Vasiliy Tolstov wrote:
> 2015-12-16 19:19 GMT+03:00 Eric Blake :
>> Won't work (qemu is not able to load disk snapshots without memory).
>> What libvirt does instead is to use qemu-img snapshot -c to change the
>> snapshot back to the active layer, then boot qemu fresh on the correct
>> contents.
>>
> 
> qemu-img snapshot -a ? as i see -c creates new snapshot.

Oh, right. The apply operation (not the create operation) (I had in my
mind 'revert', but there is no 'snapshot -r', so I must have been
thinking '-c' meant 'change', without actually reading the full text.
Moral of the story - one-letter mnemonics are not always the best
command line interface).

> 
>> Of course, patches to change behavior aren't out of the question, but
>> there's already a lot of cruft there to be aware of, and making sure we
>> don't regress libvirt behavior.
> 
> My needed use case - create multilayered qcow2 image. Base layer -
> clean fresh (for example debian) system, next layer - LAMP, next layer
> RAILS (base on top of clean debian system) and so on.
> I want to create images from packer and want to write packer plugin
> for this case (installer that able to read from qcow2 file i create
> later).
> 
> Does it possible with qemu-img snapshots with my use case? I don't
> want to snapshot memory because i don't need it. To get consistent
> snapshots i can sync disk or freeze fs by ioctl.

Yes, you can create chains of backing files at known points of time in
system installation, and then revert to booting back from that known
point of time, by using qemu-img commands.  Creating your known points
of time where the guest is powered off, rather than trying to freeze fs
but discard the memory associated with that point of time, is probably
cleaner.

You may also want to investigate if existing tools already do what you
need (libguestfs provides quite a few tools for provisioning templates
that can then be used to fire up easy clones from that point in time).

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v1] kvm/x86: Remove Hyper-V SynIC timer stopping

[Paolo Bonzini]

On 14/12/2015 18:01, Andrey Smetanin wrote:
> hostguest
> start periodic stimer
> start periodic timer
> timer expires after 15ms
> send expiration message into guest
> restart periodic timer
> doing something
> timer expires again after 15 ms
> msg slot is still not cleared so
> setup ->msg_pending
> restart periodic timer
> doing something
> process timer msg and clear slot
> so ->msg_pending was set:
> send EOM into host
> received EOM
> queued call of kvm_hv_process_stimers()
> by KVM_REQ_HV_STIMER
> 
> kvm_hv_process_stimers():
> ...
> stimer_stop()
> if (time_now >= stimer->exp_time)
> stimer_expiration(stimer);
> But time_now  < stimer->exp_time, so stimer_expiration is not called
> in this case and timer is not restarted. so guest lose timer.

Great, this explains it.

Paolo

[Qemu-devel] [PATCH untested] mirror: start drained section earlier

[Paolo Bonzini]

Ensure that the guest does not write anything to disk after cnt is
read for the final time.

Signed-off-by: Paolo Bonzini 
---
Untested.

 block/mirror.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/block/mirror.c b/block/mirror.c
index 0e8f556..e57c246 100644
--- a/block/mirror.c
+++ b/block/mirror.c
@@ -562,8 +562,18 @@ static void coroutine_fn mirror_run(void *opaque)
  * mirror_populate runs.
  */
 trace_mirror_before_drain(s, cnt);
-bdrv_drain(bs);
+bdrv_drained_begin(s->common.bs);
 cnt = bdrv_get_dirty_count(s->dirty_bitmap);
+
+if (cnt == 0) {
+/* The two disks are in sync.  Exit and report successful
+ * completion.
+ */
+assert(QLIST_EMPTY(>tracked_requests));
+s->common.cancelled = false;
+break;
+}
+bdrv_drained_end(s->common.bs);
 }
 
 ret = 0;
@@ -576,13 +586,6 @@ static void coroutine_fn mirror_run(void *opaque)
 } else if (!should_complete) {
 delay_ns = (s->in_flight == 0 && cnt == 0 ? SLICE_TIME : 0);
 block_job_sleep_ns(>common, QEMU_CLOCK_REALTIME, delay_ns);
-} else if (cnt == 0) {
-/* The two disks are in sync.  Exit and report successful
- * completion.
- */
-assert(QLIST_EMPTY(>tracked_requests));
-s->common.cancelled = false;
-break;
 }
 last_pause_ns = qemu_clock_get_ns(QEMU_CLOCK_REALTIME);
 }
@@ -608,9 +611,6 @@ immediate_exit:
 
 data = g_malloc(sizeof(*data));
 data->ret = ret;
-/* Before we switch to target in mirror_exit, make sure data doesn't
- * change. */
-bdrv_drained_begin(s->common.bs);
 block_job_defer_to_main_loop(>common, mirror_exit, data);
 }
 
-- 
2.5.0

[Qemu-devel] [PATCH 4/5] target-i386: kvm: Simplify MSR setting functions

[Eduardo Habkost]

Simplify kvm_put_tscdeadline_msr() and
kvm_put_msr_feature_control() using kvm_msr_buf and the
kvm_msr_entry_add() helper.

Signed-off-by: Eduardo Habkost 
---
 target-i386/kvm.c | 28 ++--
 1 file changed, 6 insertions(+), 22 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index ada484f..3550866 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1374,23 +1374,15 @@ static void kvm_msr_entry_add(X86CPU *cpu, uint32_t 
index, uint64_t value)
 static int kvm_put_tscdeadline_msr(X86CPU *cpu)
 {
 CPUX86State *env = >env;
-struct {
-struct kvm_msrs info;
-struct kvm_msr_entry entries[1];
-} msr_data;
-struct kvm_msr_entry *msrs = msr_data.entries;
 
 if (!has_msr_tsc_deadline) {
 return 0;
 }
 
-kvm_msr_entry_set([0], MSR_IA32_TSCDEADLINE, env->tsc_deadline);
-
-msr_data.info = (struct kvm_msrs) {
-.nmsrs = 1,
-};
+kvm_msr_buf_reset(cpu);
+kvm_msr_entry_add(cpu, MSR_IA32_TSCDEADLINE, env->tsc_deadline);
 
-return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, _data);
+return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, cpu->kvm_msr_buf);
 }
 
 /*
@@ -1401,19 +1393,11 @@ static int kvm_put_tscdeadline_msr(X86CPU *cpu)
  */
 static int kvm_put_msr_feature_control(X86CPU *cpu)
 {
-struct {
-struct kvm_msrs info;
-struct kvm_msr_entry entry;
-} msr_data;
-
-kvm_msr_entry_set(_data.entry, MSR_IA32_FEATURE_CONTROL,
+kvm_msr_buf_reset(cpu);
+kvm_msr_entry_add(cpu, MSR_IA32_FEATURE_CONTROL,
   cpu->env.msr_ia32_feature_control);
 
-msr_data.info = (struct kvm_msrs) {
-.nmsrs = 1,
-};
-
-return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, _data);
+return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_MSRS, cpu->kvm_msr_buf);
 }
 
 static int kvm_put_msrs(X86CPU *cpu, int level)
-- 
2.1.0

Re: [Qemu-devel] [PATCH v1 1/1] xlnx-ep108: Add support for high DDR memory regions

[Alistair Francis]

On Tue, Nov 24, 2015 at 10:33 PM, Peter Crosthwaite
 wrote:
> On Mon, Nov 23, 2015 at 9:00 PM, Alistair Francis
>  wrote:
>> The Xilinx EP108 supports three memory regions:
>>  - A 2GB region starting at 0
>>  - A 32GB region starting at 32GB
>>  - A 256GB region starting at 768GB
>>
>> This patch adds support for the middle memory region, which is
>> automatically created based on the size specified by the QEMU memory
>> command line argument.
>>
>
> Is that the hardware reset value or is it usually uninitialised?

Sorry for the long delay, I was on holidays.

Is what uninitialised?

> Although I'm guessing it is possible that in real HW even the low
> region could be uninitialised (unmapped) too.
>
>> Signed-off-by: Alistair Francis 
>> ---
>> Also, the Xilinx ZynqMP TRM has been released, if anyone is interested
>> it is avalibale at:
>> http://www.xilinx.com/products/silicon-devices/soc/zynq-ultrascale-mpsoc.html?resultsTablePreSelect=documenttype:User%20Guides#documentation
>>
>
> I couldn't find much here on how runtime programmable this is (is
> there more than on pg282?). Are the mappings software controllable?

I can't find much information about this either. I can't image it
being run time configurable, and even if it is that would be the DDRC,
which we don't model.

I also don't have any software cases that changes this during run
time. So for the time being I think it should just be configured at
boot. We can look into changing that if we ever add the DDRC.

>
>>  hw/arm/xlnx-ep108.c | 47 +++
>>  1 file changed, 35 insertions(+), 12 deletions(-)
>>
>> diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c
>> index 2899698..8c59d6d 100644
>> --- a/hw/arm/xlnx-ep108.c
>> +++ b/hw/arm/xlnx-ep108.c
>> @@ -22,17 +22,22 @@
>>
>>  typedef struct XlnxEP108 {
>>  XlnxZynqMPState soc;
>> -MemoryRegion ddr_ram;
>> +MemoryRegion ddr_ram_low;
>> +MemoryRegion ddr_ram_high;
>
> Looking at pg282 of the doc, the external RAM (as known to boards) is
> still only a singleton. So the RAM as instantiated here should still
> only be the one (corresponding to a single DIMM slot?). The mapping of
> that single RAM to multiple windows is a SoC (or DDRC) implemented
> feature. So instead, the machine should create the RAM but not attach
> it to the system memory. The RAM is then handed over to the SoC (MRs
> are QOM objects so it can be a QOM link) which can then create aliases
> into the single RAM for the windows. Those aliases are then in turn
> added to the system memory map by the SoC. This avoid having to dup
> this code when more boards happen and also prepares support for
> runtime remapping of the memory locations (assuming that is
> possible?).

Ok, I think I understand what you are thinking. I'll send a patch today.

>
> Ideally this would all be managed by a DDRC peripheral, but just
> getting it on the SoC level would be a good step.
>
>>  } XlnxEP108;
>>
>> -/* Max 2GB RAM */
>> -#define EP108_MAX_RAM_SIZE 0x8000ull
>> +/* Maximum size of the low memory region */
>> +#define EP108_MAX_LOW_RAM_SIZE 0x8000ull
>> +/* Total maximum size of the EP108 memory */
>> +#define EP108_MAX_RAM_SIZE 0x88000ull
>
> This feels odd. I think both LOW and HIGH should have MAX defined and
> the overall MAX is addition of the two.

Doesn't really matter to me, I'll swap it to your way.

Thanks,

Alistair

>
> Regards,
> Peter
>
>> +#define EP108_HIGH_RAM_START   0x8ull
>>
>>  static struct arm_boot_info xlnx_ep108_binfo;
>>
>>  static void xlnx_ep108_init(MachineState *machine)
>>  {
>>  XlnxEP108 *s = g_new0(XlnxEP108, 1);
>> +ram_addr_t ddr_low_size, ddr_high_size;
>>  Error *err = NULL;
>>
>

[Qemu-devel] [PATCH 2/5] target-i386: kvm: Increase MSR_BUF_SIZE

[Eduardo Habkost]

We are dangerously close to the array limits in kvm_put_msrs()
and kvm_get_msrs(): with the default mcg_cap configuration, we
can set up to 148 MSRs in kvm_put_msrs(), and if we allow mcg_cap
to be changed, we can write up to 236 MSRs.

Use 4096 bytes for the buffer, that can hold 255 kvm_msr_entry
structs.

Signed-off-by: Eduardo Habkost 
---
 target-i386/kvm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 660b2d9..1e82400 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -52,8 +52,9 @@
 #define MSR_KVM_WALL_CLOCK  0x11
 #define MSR_KVM_SYSTEM_TIME 0x12
 
-#define MSR_BUF_SIZE \
-(sizeof(struct kvm_msrs) + 150 * sizeof(struct kvm_msr_entry))
+/* A 4096-byte buffer can hold the 8-byte kvm_msrs header, plus
+ * 255 kvm_msr_entry structs */
+#define MSR_BUF_SIZE 4096
 
 #ifndef BUS_MCEERR_AR
 #define BUS_MCEERR_AR 4
-- 
2.1.0

[Qemu-devel] [PATCH 5/5] target-i386: kvm: Eliminate kvm_msr_entry_set()

[Eduardo Habkost]

Inline the function inside kvm_msr_entry_add().

Signed-off-by: Eduardo Habkost 
---
 target-i386/kvm.c | 12 +++-
 1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 3550866..b328392 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1346,14 +1346,6 @@ static int kvm_put_sregs(X86CPU *cpu)
 return kvm_vcpu_ioctl(CPU(cpu), KVM_SET_SREGS, );
 }
 
-static void kvm_msr_entry_set(struct kvm_msr_entry *entry,
-  uint32_t index, uint64_t value)
-{
-entry->index = index;
-entry->reserved = 0;
-entry->data = value;
-}
-
 static void kvm_msr_buf_reset(X86CPU *cpu)
 {
 memset(cpu->kvm_msr_buf, 0, MSR_BUF_SIZE);
@@ -1367,7 +1359,9 @@ static void kvm_msr_entry_add(X86CPU *cpu, uint32_t 
index, uint64_t value)
 
 assert((void*)(entry + 1) <= limit);
 
-kvm_msr_entry_set(entry, index, value);
+entry->index = index;
+entry->reserved = 0;
+entry->data = value;
 msrs->nmsrs++;
 }
 
-- 
2.1.0

[Qemu-devel] [PATCH v3 08/11] fdc: add physical disk sizes

[John Snow]

2.88MB capable drives can accept 1.44MB floppies,
for instance. To rework the pick_geometry function,
we need to know if our current drive can even accept
the type of disks we're considering.

NB: This allows us to distinguish between all of the
"total sectors" collisions between 1.20MB and 1.44MB
diskette types, by using the physical drive size as a
differentiator.

Signed-off-by: John Snow 
---
 hw/block/fdc.c | 40 
 1 file changed, 32 insertions(+), 8 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index f44472c..1d0c594 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -59,6 +59,12 @@ typedef enum FDriveRate {
 FDRIVE_RATE_1M   = 0x03,  /*   1 Mbps */
 } FDriveRate;
 
+typedef enum FDriveSize {
+FDRIVE_SIZE_UNKNOWN,
+FDRIVE_SIZE_350,
+FDRIVE_SIZE_525,
+} FDriveSize;
+
 typedef struct FDFormat {
 FloppyDriveType drive;
 uint8_t last_sect;
@@ -69,11 +75,15 @@ typedef struct FDFormat {
 
 #define FDRIVE_DEFAULT FLOPPY_DRIVE_TYPE_AUTO
 
+/* In many cases, the total sector size of a format is enough to uniquely
+ * identify it. However, there are some total sector collisions between
+ * formats of different physical size, and these are noted below by
+ * highlighting the total sector size for entries with collisions. */
 static const FDFormat fd_formats[] = {
 /* First entry is default format */
 /* 1.44 MB 3"1/2 floppy disks */
-{ FLOPPY_DRIVE_TYPE_144, 18, 80, 1, FDRIVE_RATE_500K, },
-{ FLOPPY_DRIVE_TYPE_144, 20, 80, 1, FDRIVE_RATE_500K, },
+{ FLOPPY_DRIVE_TYPE_144, 18, 80, 1, FDRIVE_RATE_500K, }, /* 3.5" 2880 */
+{ FLOPPY_DRIVE_TYPE_144, 20, 80, 1, FDRIVE_RATE_500K, }, /* 3.5" 3200 */
 { FLOPPY_DRIVE_TYPE_144, 21, 80, 1, FDRIVE_RATE_500K, },
 { FLOPPY_DRIVE_TYPE_144, 21, 82, 1, FDRIVE_RATE_500K, },
 { FLOPPY_DRIVE_TYPE_144, 21, 83, 1, FDRIVE_RATE_500K, },
@@ -87,7 +97,7 @@ static const FDFormat fd_formats[] = {
 { FLOPPY_DRIVE_TYPE_288, 44, 80, 1, FDRIVE_RATE_1M, },
 { FLOPPY_DRIVE_TYPE_288, 48, 80, 1, FDRIVE_RATE_1M, },
 /* 720 kB 3"1/2 floppy disks */
-{ FLOPPY_DRIVE_TYPE_144,  9, 80, 1, FDRIVE_RATE_250K, },
+{ FLOPPY_DRIVE_TYPE_144,  9, 80, 1, FDRIVE_RATE_250K, }, /* 3.5" 1400 */
 { FLOPPY_DRIVE_TYPE_144, 10, 80, 1, FDRIVE_RATE_250K, },
 { FLOPPY_DRIVE_TYPE_144, 10, 82, 1, FDRIVE_RATE_250K, },
 { FLOPPY_DRIVE_TYPE_144, 10, 83, 1, FDRIVE_RATE_250K, },
@@ -95,15 +105,15 @@ static const FDFormat fd_formats[] = {
 { FLOPPY_DRIVE_TYPE_144, 14, 80, 1, FDRIVE_RATE_250K, },
 /* 1.2 MB 5"1/4 floppy disks */
 { FLOPPY_DRIVE_TYPE_120, 15, 80, 1, FDRIVE_RATE_500K, },
-{ FLOPPY_DRIVE_TYPE_120, 18, 80, 1, FDRIVE_RATE_500K, },
+{ FLOPPY_DRIVE_TYPE_120, 18, 80, 1, FDRIVE_RATE_500K, }, /* 5.25" 2880 */
 { FLOPPY_DRIVE_TYPE_120, 18, 82, 1, FDRIVE_RATE_500K, },
 { FLOPPY_DRIVE_TYPE_120, 18, 83, 1, FDRIVE_RATE_500K, },
-{ FLOPPY_DRIVE_TYPE_120, 20, 80, 1, FDRIVE_RATE_500K, },
+{ FLOPPY_DRIVE_TYPE_120, 20, 80, 1, FDRIVE_RATE_500K, }, /* 5.25" 3200 */
 /* 720 kB 5"1/4 floppy disks */
-{ FLOPPY_DRIVE_TYPE_120,  9, 80, 1, FDRIVE_RATE_250K, },
+{ FLOPPY_DRIVE_TYPE_120,  9, 80, 1, FDRIVE_RATE_250K, }, /* 5.25" 1440 */
 { FLOPPY_DRIVE_TYPE_120, 11, 80, 1, FDRIVE_RATE_250K, },
 /* 360 kB 5"1/4 floppy disks */
-{ FLOPPY_DRIVE_TYPE_120,  9, 40, 1, FDRIVE_RATE_300K, },
+{ FLOPPY_DRIVE_TYPE_120,  9, 40, 1, FDRIVE_RATE_300K, }, /* 5.25" 720 */
 { FLOPPY_DRIVE_TYPE_120,  9, 40, 0, FDRIVE_RATE_300K, },
 { FLOPPY_DRIVE_TYPE_120, 10, 41, 1, FDRIVE_RATE_300K, },
 { FLOPPY_DRIVE_TYPE_120, 10, 42, 1, FDRIVE_RATE_300K, },
@@ -111,11 +121,25 @@ static const FDFormat fd_formats[] = {
 { FLOPPY_DRIVE_TYPE_120,  8, 40, 1, FDRIVE_RATE_250K, },
 { FLOPPY_DRIVE_TYPE_120,  8, 40, 0, FDRIVE_RATE_250K, },
 /* 360 kB must match 5"1/4 better than 3"1/2... */
-{ FLOPPY_DRIVE_TYPE_144,  9, 80, 0, FDRIVE_RATE_250K, },
+{ FLOPPY_DRIVE_TYPE_144,  9, 80, 0, FDRIVE_RATE_250K, }, /* 3.5" 720 */
 /* end */
 { FLOPPY_DRIVE_TYPE_NONE, -1, -1, 0, 0, },
 };
 
+__attribute__((__unused__))
+static FDriveSize drive_size(FloppyDriveType drive)
+{
+switch (drive) {
+case FLOPPY_DRIVE_TYPE_120:
+return FDRIVE_SIZE_525;
+case FLOPPY_DRIVE_TYPE_144:
+case FLOPPY_DRIVE_TYPE_288:
+return FDRIVE_SIZE_350;
+default:
+return FDRIVE_SIZE_UNKNOWN;
+}
+}
+
 #define GET_CUR_DRV(fdctrl) ((fdctrl)->cur_drv)
 #define SET_CUR_DRV(fdctrl, drive) ((fdctrl)->cur_drv = (drive))
 
-- 
2.4.3

[Qemu-devel] [PATCH v3 10/11] qtest/fdc: Support for 2.88MB drives

[John Snow]

The old test assumes a 1.44MB drive.
Assert that the QEMU default drive is now either 1.44 or 2.88.

Signed-off-by: John Snow 
---
 tests/fdc-test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/fdc-test.c b/tests/fdc-test.c
index b5a4696..526d459 100644
--- a/tests/fdc-test.c
+++ b/tests/fdc-test.c
@@ -267,7 +267,7 @@ static void test_cmos(void)
 uint8_t cmos;
 
 cmos = cmos_read(CMOS_FLOPPY);
-g_assert(cmos == 0x40);
+g_assert(cmos == 0x40 || cmos == 0x50);
 }
 
 static void test_no_media_on_start(void)
-- 
2.4.3

[Qemu-devel] [PATCH v5 5/6] xlnx-zynqmp: Connect the SPI devices

[Alistair Francis]

Connect the Xilinx SPI devices to the ZynqMP model.

Signed-off-by: Alistair Francis 
---
V5:
 - Use the bus renaming function
V4:
 - Rename the SPI busses so that they can all be accessed from the SoC
 - Don't set the num-busses property
V3:
 - Expose the SPI Bus as part of the SoC device
V2:
 - Don't connect the SPI flash to the SoC

 hw/arm/xlnx-zynqmp.c | 38 ++
 include/hw/arm/xlnx-zynqmp.h |  3 +++
 2 files changed, 41 insertions(+)

diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 87553bb..bce935d 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -56,6 +56,14 @@ static const int sdhci_intr[XLNX_ZYNQMP_NUM_SDHCI] = {
 48, 49,
 };
 
+static const uint64_t spi_addr[XLNX_ZYNQMP_NUM_SPIS] = {
+0xFF04, 0xFF05,
+};
+
+static const int spi_intr[XLNX_ZYNQMP_NUM_SPIS] = {
+19, 20,
+};
+
 typedef struct XlnxZynqMPGICRegion {
 int region_index;
 uint32_t address;
@@ -112,6 +120,12 @@ static void xlnx_zynqmp_init(Object *obj)
 qdev_set_parent_bus(DEVICE(>sdhci[i]),
 sysbus_get_default());
 }
+
+for (i = 0; i < XLNX_ZYNQMP_NUM_SPIS; i++) {
+object_initialize(>spi[i], sizeof(s->spi[i]),
+  TYPE_XILINX_SPIPS);
+qdev_set_parent_bus(DEVICE(>spi[i]), sysbus_get_default());
+}
 }
 
 static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
@@ -286,6 +300,30 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
**errp)
 sysbus_connect_irq(SYS_BUS_DEVICE(>sdhci[i]), 0,
gic_spi[sdhci_intr[i]]);
 }
+
+for (i = 0; i < XLNX_ZYNQMP_NUM_SPIS; i++) {
+BusState *spi_bus;
+char bus_name[6];
+
+object_property_set_bool(OBJECT(>spi[i]), true, "realized", );
+
+sysbus_mmio_map(SYS_BUS_DEVICE(>spi[i]), 0, spi_addr[i]);
+sysbus_connect_irq(SYS_BUS_DEVICE(>spi[i]), 0,
+   gic_spi[spi_intr[i]]);
+
+/* Rename each SPI bus after the SPI device to allow the board
+ * to access all of the busses from the SoC.
+ */
+spi_bus = qdev_get_child_bus(DEVICE(>spi[i]), "spi0");
+snprintf(bus_name, 6, "spi%d", i);
+qdev_bus_rename(spi_bus, bus_name);
+
+/* Add the SPI buses to the SoC child bus */
+/* FIXME: This causes the later buses to be duplicated in
+ * the SPI devices printout when running qtre.
+ */
+QLIST_INSERT_HEAD(>child_bus, spi_bus, sibling);
+}
 }
 
 static Property xlnx_zynqmp_props[] = {
diff --git a/include/hw/arm/xlnx-zynqmp.h b/include/hw/arm/xlnx-zynqmp.h
index d116092..f598a43 100644
--- a/include/hw/arm/xlnx-zynqmp.h
+++ b/include/hw/arm/xlnx-zynqmp.h
@@ -25,6 +25,7 @@
 #include "hw/ide/pci.h"
 #include "hw/ide/ahci.h"
 #include "hw/sd/sdhci.h"
+#include "hw/ssi/xilinx_spips.h"
 
 #define TYPE_XLNX_ZYNQMP "xlnx,zynqmp"
 #define XLNX_ZYNQMP(obj) OBJECT_CHECK(XlnxZynqMPState, (obj), \
@@ -35,6 +36,7 @@
 #define XLNX_ZYNQMP_NUM_GEMS 4
 #define XLNX_ZYNQMP_NUM_UARTS 2
 #define XLNX_ZYNQMP_NUM_SDHCI 2
+#define XLNX_ZYNQMP_NUM_SPIS 2
 
 #define XLNX_ZYNQMP_NUM_OCM_BANKS 4
 #define XLNX_ZYNQMP_OCM_RAM_0_ADDRESS 0xFFFC
@@ -66,6 +68,7 @@ typedef struct XlnxZynqMPState {
 CadenceUARTState uart[XLNX_ZYNQMP_NUM_UARTS];
 SysbusAHCIState sata;
 SDHCIState sdhci[XLNX_ZYNQMP_NUM_SDHCI];
+XilinxSPIPS spi[XLNX_ZYNQMP_NUM_SPIS];
 
 char *boot_cpu;
 ARMCPU *boot_cpu_ptr;
-- 
2.5.0

[Qemu-devel] [PATCH v5 6/6] xlnx-ep108: Connect the SPI Flash

[Alistair Francis]

Connect the sst25wf080 SPI flash to the EP108 board.

Signed-off-by: Alistair Francis 
---
V4:
 - Only add one SPI flash
V3:
 - Don't reach into the SoC
V2:
 - Use sst25wf080 instead of m25p80

 hw/arm/xlnx-ep108.c | 16 
 1 file changed, 16 insertions(+)

diff --git a/hw/arm/xlnx-ep108.c b/hw/arm/xlnx-ep108.c
index 85b978f..d1912aa 100644
--- a/hw/arm/xlnx-ep108.c
+++ b/hw/arm/xlnx-ep108.c
@@ -33,6 +33,7 @@ static struct arm_boot_info xlnx_ep108_binfo;
 static void xlnx_ep108_init(MachineState *machine)
 {
 XlnxEP108 *s = g_new0(XlnxEP108, 1);
+int i;
 Error *err = NULL;
 
 object_initialize(>soc, sizeof(s->soc), TYPE_XLNX_ZYNQMP);
@@ -60,6 +61,21 @@ static void xlnx_ep108_init(MachineState *machine)
  machine->ram_size);
 memory_region_add_subregion(get_system_memory(), 0, >ddr_ram);
 
+for (i = 0; i < XLNX_ZYNQMP_NUM_SPIS; i++) {
+SSIBus *spi_bus;
+DeviceState *flash_dev;
+qemu_irq cs_line;
+char bus_name[6];
+
+snprintf(bus_name, 6, "spi%d", i);
+spi_bus = (SSIBus *)qdev_get_child_bus(DEVICE(>soc), bus_name);
+
+flash_dev = ssi_create_slave(spi_bus, "sst25wf080");
+cs_line = qdev_get_gpio_in_named(flash_dev, SSI_GPIO_CS, 0);
+
+sysbus_connect_irq(SYS_BUS_DEVICE(>soc.spi[i]), 1, cs_line);
+}
+
 xlnx_ep108_binfo.ram_size = machine->ram_size;
 xlnx_ep108_binfo.kernel_filename = machine->kernel_filename;
 xlnx_ep108_binfo.kernel_cmdline = machine->kernel_cmdline;
-- 
2.5.0

[Qemu-devel] [PATCH v3 04/11] fdc: add default drive type option

[John Snow]

This patch adds a new explicit Floppy Drive Type option. The existing
behavior in QEMU is to automatically guess a drive type based on the
media inserted, or if a diskette is not present, arbitrarily assign one.

This behavior can be described as "auto." This patch adds explicit
behaviors: 120, 144, 288, auto, and none. The new "auto" behavior
is intended to mimick current behavior, while the other types pick
one explicitly.

In a future patch, the goal is to change the FDC's default drive type
from auto (falling back to 1.44MB) to auto (falling back to 2.88MB).

In order to allow users to obtain the old behaviors, though, a mechanism
for specifying the exact type of drive we want is needed.

This patch adds the properties, but it is not acted on yet in favor of
making those changes a little more explicitly clear in standalone patches
later in this patch set.

Signed-off-by: John Snow 
---
 hw/block/fdc.c   | 107 ++-
 hw/core/qdev-properties.c|  11 +
 hw/i386/pc.c |  17 +++
 include/hw/block/fdc.h   |   9 +---
 include/hw/qdev-properties.h |   1 +
 qapi/block.json  |  16 +++
 6 files changed, 102 insertions(+), 59 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 13fef23..ad0e052 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -60,58 +60,60 @@ typedef enum FDriveRate {
 } FDriveRate;
 
 typedef struct FDFormat {
-FDriveType drive;
+FloppyDriveType drive;
 uint8_t last_sect;
 uint8_t max_track;
 uint8_t max_head;
 FDriveRate rate;
 } FDFormat;
 
+#define FDRIVE_DEFAULT FLOPPY_DRIVE_TYPE_AUTO
+
 static const FDFormat fd_formats[] = {
 /* First entry is default format */
 /* 1.44 MB 3"1/2 floppy disks */
-{ FDRIVE_DRV_144, 18, 80, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_144, 20, 80, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_144, 21, 80, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_144, 21, 82, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_144, 21, 83, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_144, 22, 80, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_144, 23, 80, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_144, 24, 80, 1, FDRIVE_RATE_500K, },
+{ FLOPPY_DRIVE_TYPE_144, 18, 80, 1, FDRIVE_RATE_500K, },
+{ FLOPPY_DRIVE_TYPE_144, 20, 80, 1, FDRIVE_RATE_500K, },
+{ FLOPPY_DRIVE_TYPE_144, 21, 80, 1, FDRIVE_RATE_500K, },
+{ FLOPPY_DRIVE_TYPE_144, 21, 82, 1, FDRIVE_RATE_500K, },
+{ FLOPPY_DRIVE_TYPE_144, 21, 83, 1, FDRIVE_RATE_500K, },
+{ FLOPPY_DRIVE_TYPE_144, 22, 80, 1, FDRIVE_RATE_500K, },
+{ FLOPPY_DRIVE_TYPE_144, 23, 80, 1, FDRIVE_RATE_500K, },
+{ FLOPPY_DRIVE_TYPE_144, 24, 80, 1, FDRIVE_RATE_500K, },
 /* 2.88 MB 3"1/2 floppy disks */
-{ FDRIVE_DRV_288, 36, 80, 1, FDRIVE_RATE_1M, },
-{ FDRIVE_DRV_288, 39, 80, 1, FDRIVE_RATE_1M, },
-{ FDRIVE_DRV_288, 40, 80, 1, FDRIVE_RATE_1M, },
-{ FDRIVE_DRV_288, 44, 80, 1, FDRIVE_RATE_1M, },
-{ FDRIVE_DRV_288, 48, 80, 1, FDRIVE_RATE_1M, },
+{ FLOPPY_DRIVE_TYPE_288, 36, 80, 1, FDRIVE_RATE_1M, },
+{ FLOPPY_DRIVE_TYPE_288, 39, 80, 1, FDRIVE_RATE_1M, },
+{ FLOPPY_DRIVE_TYPE_288, 40, 80, 1, FDRIVE_RATE_1M, },
+{ FLOPPY_DRIVE_TYPE_288, 44, 80, 1, FDRIVE_RATE_1M, },
+{ FLOPPY_DRIVE_TYPE_288, 48, 80, 1, FDRIVE_RATE_1M, },
 /* 720 kB 3"1/2 floppy disks */
-{ FDRIVE_DRV_144,  9, 80, 1, FDRIVE_RATE_250K, },
-{ FDRIVE_DRV_144, 10, 80, 1, FDRIVE_RATE_250K, },
-{ FDRIVE_DRV_144, 10, 82, 1, FDRIVE_RATE_250K, },
-{ FDRIVE_DRV_144, 10, 83, 1, FDRIVE_RATE_250K, },
-{ FDRIVE_DRV_144, 13, 80, 1, FDRIVE_RATE_250K, },
-{ FDRIVE_DRV_144, 14, 80, 1, FDRIVE_RATE_250K, },
+{ FLOPPY_DRIVE_TYPE_144,  9, 80, 1, FDRIVE_RATE_250K, },
+{ FLOPPY_DRIVE_TYPE_144, 10, 80, 1, FDRIVE_RATE_250K, },
+{ FLOPPY_DRIVE_TYPE_144, 10, 82, 1, FDRIVE_RATE_250K, },
+{ FLOPPY_DRIVE_TYPE_144, 10, 83, 1, FDRIVE_RATE_250K, },
+{ FLOPPY_DRIVE_TYPE_144, 13, 80, 1, FDRIVE_RATE_250K, },
+{ FLOPPY_DRIVE_TYPE_144, 14, 80, 1, FDRIVE_RATE_250K, },
 /* 1.2 MB 5"1/4 floppy disks */
-{ FDRIVE_DRV_120, 15, 80, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_120, 18, 80, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_120, 18, 82, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_120, 18, 83, 1, FDRIVE_RATE_500K, },
-{ FDRIVE_DRV_120, 20, 80, 1, FDRIVE_RATE_500K, },
+{ FLOPPY_DRIVE_TYPE_120, 15, 80, 1, FDRIVE_RATE_500K, },
+{ FLOPPY_DRIVE_TYPE_120, 18, 80, 1, FDRIVE_RATE_500K, },
+{ FLOPPY_DRIVE_TYPE_120, 18, 82, 1, FDRIVE_RATE_500K, },
+{ FLOPPY_DRIVE_TYPE_120, 18, 83, 1, FDRIVE_RATE_500K, },
+{ FLOPPY_DRIVE_TYPE_120, 20, 80, 1, FDRIVE_RATE_500K, },
 /* 720 kB 5"1/4 floppy disks */
-{ FDRIVE_DRV_120,  9, 80, 1, FDRIVE_RATE_250K, },
-{ FDRIVE_DRV_120, 11, 80, 1, FDRIVE_RATE_250K, },
+{ FLOPPY_DRIVE_TYPE_120,  9, 80, 1, FDRIVE_RATE_250K, },
+{ FLOPPY_DRIVE_TYPE_120, 11, 80, 1, FDRIVE_RATE_250K, },
 /* 360 kB 5"1/4 floppy 

[Qemu-devel] [PATCH v3 00/11] fdc: fix 2.88mb floppy diskette support

[John Snow]

requires: 1448895398-13465-1-git-send-email-ehabk...@redhat.com
  pc: Add pc-*-2.6 machine classes

Yes, it's been broken for ten years.
No, it's not a CVE.

The problem is that QEMU doesn't have a configuration option for the type
of floppy drive you want. It determines that based on the type of
diskette inserted at boot time.

If you don't insert one, it always chooses a 1.44MB type.

If you want to insert a 2.88MB floppy after boot, you simply cannot.

"Wow, who cares?"

Good question -- Unfortunately, the virtio-win floppy disk images that
Red Hat/Fedora ship require a 2.88MB drive, so if you forgot to insert
them at boot, you'd have to change your VM configuration and try again.

For a one-shot operation, that's kind of obnoxious -- it'd be nice to
allow one to just insert the diskette on-demand.

"OK, What are you changing in this decades-old device?"

(1) Add a new property to allow users to specify what kind of drive they
want without relying on magical guessing behavior.
Choices are: 120, 144, 288, auto, and none.

120, 144 and 288 refer to 1.20MB, 1.44MB, and 2.88MB drives.
auto refers to the auto-detect behavior QEMU currently has.
none ... hides the drive. You probably don't want to use this,
but it's there if you feel like creating a drive you can't use.

(2) Add a new "fallback" property for use with the "auto" drive type
that allows us to specify the backup behavior, too. In most cases
this property won't be needed, but it is provided for allowing
QEMU to be fully backwards compatible.

(3) Add the concept of physical diskette size to QEMU, classifying
120-style diskettes as fundamentally different from 144 and 288 ones.

(4) Revamp the automatic guessing heuristic to understand that
2.88MB style drives can accept 1.44MB diskettes.

(5) Change the automatic fallback type for the automatic guessing
heuristic from 1.44MB to 2.88MB for 2.6 machines and beyond,
leaving 2.5- machines set to default to auto/144.

(6) A lot of code cleanup in general.

"Won't this break everything, you madman?"

No: I tested this in MS-DOS 6.22, Fedora 23 and Windows 8.1. All
seemed perfectly happy with 2.88MB drives as the default for 1.44
or 2.88MB floppy diskette images.

And: Older machine types will happily still default to the 1.44
 type just like they used to, so really nothing should change
 at all for most guests.

If there ARE any guests affected in 2.6+ machine types, you are
urged to use an explicit drive type that matches your application
if the automatic behavior is unsuitable.

===
v3:
===

001/11:[] [--] 'fdc: move pick_geometry'
002/11:[] [--] 'fdc: refactor pick_geometry'
003/11:[] [--] 'fdc: add disk field'
004/11:[0037] [FC] 'fdc: add default drive type option'
005/11:[down] 'fdc: Add fallback option'
006/11:[] [-C] 'fdc: do not call revalidate on eject'
007/11:[0030] [FC] 'fdc: implement new drive type property'
008/11:[] [-C] 'fdc: add physical disk sizes'
009/11:[0018] [FC] 'fdc: rework pick_geometry'
010/11:[] [--] 'qtest/fdc: Support for 2.88MB drives'
011/11:[down] 'fdc: change auto fallback drive for ISA FDC to 288'

04: Remove typeA/typeB members of FDCtrl. Store e.g. -fdtypeA options
   directly into FDCtrl.drives[x].drive instead.
05: Add a new fallback= option that controls fdtype{A,B}=auto behavior.
07: replace get_default_drive_type which is no longer needed
add get_fallback_drive_type.
09: Reworked the auto/fallback section of pick_geometry.



For convenience, this branch is available at:
https://github.com/jnsnow/qemu.git branch fdc-default
https://github.com/jnsnow/qemu/tree/fdc-default

This version is tagged fdc-default-v3:
https://github.com/jnsnow/qemu/releases/tag/fdc-default-v3

John Snow (11):
  fdc: move pick_geometry
  fdc: refactor pick_geometry
  fdc: add disk field
  fdc: add default drive type option
  fdc: Add fallback option
  fdc: do not call revalidate on eject
  fdc: implement new drive type property
  fdc: add physical disk sizes
  fdc: rework pick_geometry
  qtest/fdc: Support for 2.88MB drives
  fdc: change auto fallback drive for ISA FDC to 288

 hw/block/fdc.c   | 317 +--
 hw/core/qdev-properties.c|  11 ++
 hw/i386/pc.c |  17 +--
 include/hw/block/fdc.h   |   9 +-
 include/hw/compat.h  |   6 +-
 include/hw/qdev-properties.h |   1 +
 qapi/block.json  |  16 +++
 tests/fdc-test.c |   2 +-
 8 files changed, 260 insertions(+), 119 deletions(-)

-- 
2.4.3

Re: [Qemu-devel] [PATCH v3 11/11] fdc: change auto fallback drive for ISA FDC to 288

[John Snow]

On 12/16/2015 05:16 PM, John Snow wrote:
> The 2.88 drive is more suitable as a default because
> it can still read 1.44 images correctly, but the reverse
> is not true.
> 
> Since there exist virtio-win drivers that are shipped on
> 2.88 floppy images, this patch will allow VMs booted without
> a floppy disk inserted to later insert a 2.88MB floppy and
> have that work.
> 
> This patch has been tested with msdos, freedos, fedora,
> windows 8 and windows 10 without issue: if problems do
> arise for certain guests being unable to cope with 2.88MB
> drives as the default, they are in the minority and can use
> type=144 as needed (or insert a proper boot medium and omit
> type=144/288 or use type=auto) to obtain different drive types.
> 
> As icing, the default will remain auto/144 for any pre-2.6
> machine types, hopefully minimizing the impact of this change
> in legacy hw to basically zero.
> 
> Signed-off-by: John Snow 
> ---
>  hw/block/fdc.c  | 2 +-
>  include/hw/compat.h | 6 +-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index b9b9bf7..745b284 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -2524,7 +2524,7 @@ static Property isa_fdc_properties[] = {
>  FDRIVE_DEFAULT, qdev_prop_fdc_drive_type,
>  FloppyDriveType),
>  DEFINE_PROP_DEFAULT("fallback", FDCtrlISABus, state.fallback,
> -FLOPPY_DRIVE_TYPE_144, qdev_prop_fdc_drive_type,
> +FLOPPY_DRIVE_TYPE_288, qdev_prop_fdc_drive_type,
>  FloppyDriveType),
>  DEFINE_PROP_END_OF_LIST(),
>  };
> diff --git a/include/hw/compat.h b/include/hw/compat.h
> index 24dd2c0..5a53f3c 100644
> --- a/include/hw/compat.h
> +++ b/include/hw/compat.h
> @@ -2,7 +2,11 @@
>  #define HW_COMPAT_H
>  
>  #define HW_COMPAT_2_5 \
> -/* empty */
> +{\
> +.driver   = "fdc",\

This should be "isa-fdc". >:[

> +.property = "fallback",\
> +.value= "144",\
> +},
>  
>  #define HW_COMPAT_2_4 \
>  {\
> 

[Qemu-devel] [PATCH v3 06/11] fdc: do not call revalidate on eject

[John Snow]

Currently, fd_revalidate is called in two different places, with two
different expectations of behavior:

(1) On initialization, as a routine to help pick the drive type and
initial geometries as a side-effect of the pick_geometry routine

(2) On insert/eject, which either sets the geometries to a default value
or chooses new geometries based on the inserted diskette.

Break this nonsense apart by creating a new function dedicated towards
picking the drive type on initialization.

This has a few results:

(1) fd_revalidate does not get called on boot anymore for drives with no
diskette.

(2) pick_geometry will actually get called twice if we have a diskette
inserted, but this is harmless. (Once for the drive type, and once
as part of the media callback.)

Signed-off-by: John Snow 
---
 hw/block/fdc.c | 36 +---
 1 file changed, 29 insertions(+), 7 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index b587de8..e752758 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -167,6 +167,7 @@ static void fd_init(FDrive *drv)
 drv->disk = FLOPPY_DRIVE_TYPE_NONE;
 drv->last_sect = 0;
 drv->max_track = 0;
+drv->ro = true;
 }
 
 #define NUM_SIDES(drv) ((drv)->flags & FDISK_DBL_SIDES ? 2 : 1)
@@ -249,13 +250,21 @@ static void fd_recalibrate(FDrive *drv)
 fd_seek(drv, 0, 0, 1, 1);
 }
 
-static void pick_geometry(FDrive *drv)
+/**
+ * Determine geometry based on inserted diskette.
+ */
+static bool pick_geometry(FDrive *drv)
 {
 BlockBackend *blk = drv->blk;
 const FDFormat *parse;
 uint64_t nb_sectors, size;
 int i, first_match, match;
 
+/* We can only pick a geometry if we have a diskette. */
+if (!drv->media_inserted) {
+return false;
+}
+
 blk_get_geometry(blk, _sectors);
 match = -1;
 first_match = -1;
@@ -293,8 +302,7 @@ static void pick_geometry(FDrive *drv)
 }
 drv->max_track = parse->max_track;
 drv->last_sect = parse->last_sect;
-drv->drive = parse->drive;
-drv->disk = drv->media_inserted ? parse->drive : FLOPPY_DRIVE_TYPE_NONE;
+drv->disk = parse->drive;
 drv->media_rate = parse->rate;
 
 if (drv->media_inserted) {
@@ -303,6 +311,14 @@ static void pick_geometry(FDrive *drv)
drv->max_track, drv->last_sect,
drv->ro ? "ro" : "rw");
 }
+return true;
+}
+
+static void pick_drive_type(FDrive *drv)
+{
+if (pick_geometry(drv)) {
+drv->drive = drv->disk;
+}
 }
 
 /* Revalidate a disk drive after a disk change */
@@ -311,15 +327,18 @@ static void fd_revalidate(FDrive *drv)
 FLOPPY_DPRINTF("revalidate\n");
 if (drv->blk != NULL) {
 drv->ro = blk_is_read_only(drv->blk);
-pick_geometry(drv);
 if (!drv->media_inserted) {
 FLOPPY_DPRINTF("No disk in drive\n");
+drv->disk = FLOPPY_DRIVE_TYPE_NONE;
+} else {
+pick_geometry(drv);
 }
 } else {
 FLOPPY_DPRINTF("No drive connected\n");
 drv->last_sect = 0;
 drv->max_track = 0;
 drv->flags &= ~FDISK_DBL_SIDES;
+drv->disk = FLOPPY_DRIVE_TYPE_NONE;
 }
 }
 
@@ -2196,9 +2215,11 @@ static void fdctrl_change_cb(void *opaque, bool load)
 FDrive *drive = opaque;
 
 drive->media_inserted = load && drive->blk && blk_is_inserted(drive->blk);
-
 drive->media_changed = 1;
-fd_revalidate(drive);
+
+if (load) {
+fd_revalidate(drive);
+}
 }
 
 static bool fdctrl_is_tray_open(void *opaque)
@@ -2234,13 +2255,14 @@ static void fdctrl_connect_drives(FDCtrl *fdctrl, Error 
**errp)
 }
 
 fd_init(drive);
-fdctrl_change_cb(drive, 0);
 if (drive->blk) {
 blk_set_dev_ops(drive->blk, _block_ops, drive);
 drive->media_inserted = blk_is_inserted(drive->blk);
+pick_drive_type(drive);
 } else {
 drive->drive = FLOPPY_DRIVE_TYPE_NONE;
 }
+fdctrl_change_cb(drive, drive->media_inserted);
 }
 }
 
-- 
2.4.3

[Qemu-devel] [PATCH v3 07/11] fdc: implement new drive type property

[John Snow]

Respect the drive type as given via the CLI.

Set the type given by the CLI during fd_init. If the type remains the
default (auto), we'll attempt to scan an inserted diskette if present
to determine a type. If auto is selected but no diskette is present,
we fall back to a predetermined default (currently 1.44MB to match
legacy QEMU behavior.)

The pick_geometry algorithm is modified to only allow matches outside
of the existing drive type for the new auto behavior. If a user specifies
the "none" type, QEMU will not report this drive to the CMOS.

Signed-off-by: John Snow 
---
 hw/block/fdc.c | 23 +--
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index e752758..f44472c 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -155,13 +155,11 @@ typedef struct FDrive {
 bool media_inserted;  /* Is there a medium in the tray */
 } FDrive;
 
-
 static FloppyDriveType get_fallback_drive_type(FDrive *drv);
 
 static void fd_init(FDrive *drv)
 {
 /* Drive */
-drv->drive = FLOPPY_DRIVE_TYPE_NONE; /* FIXME: Obey CLI properties */
 drv->perpendicular = 0;
 /* Disk */
 drv->disk = FLOPPY_DRIVE_TYPE_NONE;
@@ -274,7 +272,7 @@ static bool pick_geometry(FDrive *drv)
 break;
 }
 if (drv->drive == parse->drive ||
-drv->drive == FLOPPY_DRIVE_TYPE_NONE) {
+drv->drive == FLOPPY_DRIVE_TYPE_AUTO) {
 size = (parse->max_head + 1) * parse->max_track *
 parse->last_sect;
 if (nb_sectors == size) {
@@ -288,7 +286,10 @@ static bool pick_geometry(FDrive *drv)
 }
 if (match == -1) {
 if (first_match == -1) {
-match = 1;
+/* No entry found: drive_type was NONE or we neglected to add any
+ * candidate geometries for our drive type into the fd_formats 
table
+ */
+match = ARRAY_SIZE(fd_formats) - 1;
 } else {
 match = first_match;
 }
@@ -316,9 +317,19 @@ static bool pick_geometry(FDrive *drv)
 
 static void pick_drive_type(FDrive *drv)
 {
-if (pick_geometry(drv)) {
-drv->drive = drv->disk;
+if (drv->drive != FLOPPY_DRIVE_TYPE_AUTO) {
+return;
 }
+
+if (!drv->media_inserted) {
+drv->drive = get_fallback_drive_type(drv);
+} else {
+if (pick_geometry(drv)) {
+drv->drive = drv->disk;
+}
+}
+
+g_assert(drv->drive != FLOPPY_DRIVE_TYPE_AUTO);
 }
 
 /* Revalidate a disk drive after a disk change */
-- 
2.4.3

Re: [Qemu-devel] [RFC PATCH v0 0/9] Generic cpu-core device

[Igor Mammedov]

On Wed, 16 Dec 2015 18:22:20 +0100
Igor Mammedov  wrote:

> On Wed, 16 Dec 2015 16:57:54 +0100
> Andreas Färber  wrote:
[...]
> > 
> > Attendees in Seattle said that thread-level hot-plug were dangerous
> > for Linux guests due to assumptions in the (guest's) scheduler
> > breaking for any incompletely filled cores or sockets. No one
> > present
> There is not such thing as cpu hotplug at socket level in x86 linux
> so far. CPUs are plugged at logical(thread) cpu level, one at a time.
> And ACPI spec does the same (describes logical CPUs) and hotplug
> notification in guest handled per one logical cpu at a time.
BTW:
from my experience with CPU hotplug, I've had to fix only one
topology+sched related bug in kernel but more bugs were related to fast
CPU online/offline especially if host is over-committed.

And plugging whole socket would lead to a rapid sequence of hotplugging
CPUs which has been causing non deterministic kernel crash/lock up
much more often than topology issues, there are still bugs to fix in
that area.

[Qemu-devel] [PATCH v5 2/6] ssi: Move ssi.h into a separate directory

[Alistair Francis]

Move the ssi.h include file into the ssi directory.

While touching the code also fix the typdef lines as
checkpatch complains.

Signed-off-by: Alistair Francis 
Reviewed-by: Peter Crosthwaite 
---
V2:
 - Change git patch to indicate rename

 hw/arm/pxa2xx.c |  2 +-
 hw/arm/spitz.c  |  2 +-
 hw/arm/stellaris.c  |  2 +-
 hw/arm/strongarm.c  |  2 +-
 hw/arm/tosa.c   |  2 +-
 hw/arm/xilinx_zynq.c|  2 +-
 hw/arm/z2.c |  2 +-
 hw/block/m25p80.c   |  2 +-
 hw/display/ads7846.c|  2 +-
 hw/display/ssd0323.c|  2 +-
 hw/microblaze/petalogix_ml605_mmu.c |  2 +-
 hw/misc/max111x.c   |  2 +-
 hw/sd/ssi-sd.c  |  2 +-
 hw/ssi/pl022.c  |  2 +-
 hw/ssi/ssi.c|  2 +-
 hw/ssi/xilinx_spi.c |  2 +-
 hw/ssi/xilinx_spips.c   |  2 +-
 include/hw/{ => ssi}/ssi.h  | 10 ++
 18 files changed, 23 insertions(+), 21 deletions(-)
 rename include/hw/{ => ssi}/ssi.h (96%)

diff --git a/hw/arm/pxa2xx.c b/hw/arm/pxa2xx.c
index 79d22d9..54bf152 100644
--- a/hw/arm/pxa2xx.c
+++ b/hw/arm/pxa2xx.c
@@ -12,7 +12,7 @@
 #include "sysemu/sysemu.h"
 #include "hw/char/serial.h"
 #include "hw/i2c/i2c.h"
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 #include "sysemu/char.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
diff --git a/hw/arm/spitz.c b/hw/arm/spitz.c
index 8d3cc0b..ee8f889 100644
--- a/hw/arm/spitz.c
+++ b/hw/arm/spitz.c
@@ -16,7 +16,7 @@
 #include "sysemu/sysemu.h"
 #include "hw/pcmcia.h"
 #include "hw/i2c/i2c.h"
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 #include "hw/block/flash.h"
 #include "qemu/timer.h"
 #include "hw/devices.h"
diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index 0114e0a..4e5cfd1 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -8,7 +8,7 @@
  */
 
 #include "hw/sysbus.h"
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 #include "hw/arm/arm.h"
 #include "hw/devices.h"
 #include "qemu/timer.h"
diff --git a/hw/arm/strongarm.c b/hw/arm/strongarm.c
index 9624ecb..4d2ba02 100644
--- a/hw/arm/strongarm.c
+++ b/hw/arm/strongarm.c
@@ -34,7 +34,7 @@
 #include "hw/arm/arm.h"
 #include "sysemu/char.h"
 #include "sysemu/sysemu.h"
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 
 //#define DEBUG
 
diff --git a/hw/arm/tosa.c b/hw/arm/tosa.c
index 02814d7..68ad01e 100644
--- a/hw/arm/tosa.c
+++ b/hw/arm/tosa.c
@@ -19,7 +19,7 @@
 #include "hw/pcmcia.h"
 #include "hw/boards.h"
 #include "hw/i2c/i2c.h"
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 #include "sysemu/block-backend.h"
 #include "hw/sysbus.h"
 #include "exec/address-spaces.h"
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 1c1a445..11a349b 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -25,7 +25,7 @@
 #include "sysemu/block-backend.h"
 #include "hw/loader.h"
 #include "hw/misc/zynq-xadc.h"
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 #include "qemu/error-report.h"
 
 #define NUM_SPI_FLASHES 4
diff --git a/hw/arm/z2.c b/hw/arm/z2.c
index b44eb76..c82fe2c 100644
--- a/hw/arm/z2.c
+++ b/hw/arm/z2.c
@@ -16,7 +16,7 @@
 #include "hw/arm/arm.h"
 #include "hw/devices.h"
 #include "hw/i2c/i2c.h"
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 #include "hw/boards.h"
 #include "sysemu/sysemu.h"
 #include "hw/block/flash.h"
diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 7b9f97c..addd907 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -24,7 +24,7 @@
 #include "hw/hw.h"
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 
 #ifndef M25P80_ERR_DEBUG
 #define M25P80_ERR_DEBUG 0
diff --git a/hw/display/ads7846.c b/hw/display/ads7846.c
index 3f35369..cb82317 100644
--- a/hw/display/ads7846.c
+++ b/hw/display/ads7846.c
@@ -10,7 +10,7 @@
  * GNU GPL, version 2 or (at your option) any later version.
  */
 
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 #include "ui/console.h"
 
 typedef struct {
diff --git a/hw/display/ssd0323.c b/hw/display/ssd0323.c
index 9727007..7545da8 100644
--- a/hw/display/ssd0323.c
+++ b/hw/display/ssd0323.c
@@ -10,7 +10,7 @@
 /* The controller can support a variety of different displays, but we only
implement one.  Most of the commends relating to brightness and geometry
setup are ignored. */
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 #include "ui/console.h"
 
 //#define DEBUG_SSD0323 1
diff --git a/hw/microblaze/petalogix_ml605_mmu.c 
b/hw/microblaze/petalogix_ml605_mmu.c
index 462060f..5366cec 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -35,7 +35,7 @@
 #include "sysemu/block-backend.h"
 #include "hw/char/serial.h"
 #include "exec/address-spaces.h"
-#include "hw/ssi.h"
+#include "hw/ssi/ssi.h"
 
 #include 

[Qemu-devel] [PATCH v5 4/6] qdev: Add a function to rename busses

[Alistair Francis]

Add a function which can be used to rename busses.

Signed-off-by: Alistair Francis 
---

 hw/core/qdev.c | 5 +
 include/hw/qdev-core.h | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index b3ad467..29a3e9d 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -590,6 +590,11 @@ BusState *qdev_get_child_bus(DeviceState *dev, const char 
*name)
 return NULL;
 }
 
+void qdev_bus_rename(BusState *bus, const char *name)
+{
+memcpy((char *) bus->name, name, strlen(name) * sizeof(char));
+}
+
 int qbus_walk_children(BusState *bus,
qdev_walkerfn *pre_devfn, qbus_walkerfn *pre_busfn,
qdev_walkerfn *post_devfn, qbus_walkerfn *post_busfn,
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index c537969..9653f4d 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -297,6 +297,8 @@ qemu_irq qdev_intercept_gpio_out(DeviceState *dev, qemu_irq 
icpt,
 
 BusState *qdev_get_child_bus(DeviceState *dev, const char *name);
 
+void qdev_bus_rename(BusState *bus, const char *name);
+
 /*** Device API.  ***/
 
 /* Register device properties.  */
-- 
2.5.0

Re: [Qemu-devel] [RFC PATCH v0 0/9] Generic cpu-core device

[Igor Mammedov]

On Wed, 16 Dec 2015 16:46:37 +0100
Andreas Färber  wrote:

> Am 10.12.2015 um 13:35 schrieb Igor Mammedov:
> > wrt CLI can't we do something like this?
> > 
> > -device some-cpu-model,socket=x[,core=y[,thread=z]]
> 
> That's problematic and where my x86 remodeling got stuck. It works
> fine (more or less) to model sockets, cores and hyperthreads for
> -smp, but doing it dynamically did not work well. How do you
> determine the instance size a socket with N cores and M threads
> needs?
-smp defines necessary topology, all one need is to find out
instance size of a core or thread object, it would be probably x86
specific but it's doable, the only thing in current X86 thread
needs to fix is to reserve space for largest APIC type
and replace object_new with object_initialize_with_type.

However if I look from x86 point of view, there isn't need to model
sockets nor cores. The threads are sufficient for QEMU needs.
Dummy sockets/cores are just complicating implementation.

What I'm advocating for is let archs to decide if they should create
CPUs per socket, core or thread.

And for x86 do this at thread level, that way we keep compatibility
with cpu-add, but will also allow which cpu thread to plug with
 'node=n,socket=x,core=y,thread=z'.

Another point in favor of thread granularity for x86 is that competing
hyper-visors are doing that at thread level, QEMU would be worse off
in feature parity if minimal hotplug unit would be a socket. 

That also has benefit of being very flexible and would also suit
engineering audience of QEMU, allowing them to build CPUs from
config instead of hardcoding it in code and playing heterogeneous
configurations.

Options 'node=n,socket=x,core=y,thread=z' are just a SMP specific
path, defining where CPU should attached, it could be a QOM path
in the future when we arrive there and have a stable QOM tree. 

> Allocations in instance_init are to be avoided with a view to
> hot-plug.
> So either we have a fully determined socket object or we
> need to wire individual objects on the command line. The latter has
> bad implications for atomicity and thus hot-unplug. That leaves us
what are these bad implication and how they affect unplug?

If for example x86 CPU thread is fixed to embed child APIC then
to avoid allocations as much as possible or fail gracefully there
is 2 options :
 1: like you've said, reserve all needed space at startup, i.e.
pre-create empty sockets
 2: fail gracefully in qdev_device_add() if allocation is not possible

for #2 it's not enough to avoid allocations in instance_init()
we also must teach qdev_device_add() to get the size of to be created
object and replace
object_new() with malloc() + object_initialize_with_type(),
that way it's possible to fail allocation gracefully and report error.

Doing that would benefit not only CPUs but every device_add capable
Device and is sufficient for hotplug purposes without overhead of
reserving space for every possible hotplugged device at startup (which
is impossible anyway in generic)
So I'd go for #2 sane device_add impl. vs #1 preallocated objects one  
 
> with dynamic properties doing allocations and reporting it via
> Error**, something I never finished and could use reviewers and
> contributors.
most of dynamic properties are static, looks like what QOM needs
is really static properties so we don't misuse the former and probably
a way to reserve space for declared number of dynamic ones to avoid
allocations in instance_initialize().

> 
> Anthony's old suggestion had been to use real socket product names
> like Xeon-E5-4242 to get a 6-core, dual-thread socket, without
> parameters - unfortunately I still don't see an easy way to define
> such a thing today with the flexibility users will undoubtedly want.
I don't see it either and for me it is much harder to remember
what Xeon-E5-4242 is while it's much easier to say:
   I want N [cpu-foo] threads
which in SMP world could be expressed via add N tread objects at
specified locations
   device_add cpu-foo, with optional node=n,socket=x,core=y,thread=z
allows to do it.
And well for x86 there lots of these Xeon-foo/whatever-foo codenames,
which would be nightmare to maintain.

> 
> And since the question came up how to detect this, what you guys seem
> to keep forgetting is that somewhere there also needs to be a matching
> link<> property that determines what can be plugged, i.e. QMP
> qom-list. link<>s are the QOM equivalent to qdev's buses. The object
> itself needs to live in /machine/peripheral
> or /machine/peripheral-anon (/machine/unattached is supposed to go
> away after the QOM conversion is done!) and in a machine-specific
> place there will be a /machine/cpu-socket[0]
> -> /machine/peripheral-anon/device[42] link
> property. It might just as well
> be /machine/daughterboard-x/cpu-core[2] -> /machine/peripheral/cpu0.
> (Gentle reminder of the s390 ipi modeling discussion that never came
> to any conclusion iirc.)
QOM view probably is 

[Qemu-devel] [PATCH v3 09/11] fdc: rework pick_geometry

[John Snow]

This one is the crazy one.

fd_revalidate currently uses pick_geometry to tell if the diskette
geometry has changed upon an eject/insert event, but it won't allow us
to insert a 1.44MB diskette into a 2.88MB drive. This is inflexible.

The new algorithm applies a new heuristic to guessing disk geometries
that allows us to switch diskette types as long as the physical size
matches before falling back to the old heuristic.

The old one is roughly:
 - If the size and type matches, choose it.
 - Fall back to the first geometry that matched our type.

The new one is:
 - If the size and type matches, choose it.
 - If the size (sectors) and physical size match, choose it.
 - If the size (sectors) matches at all, choose it begrudgingly.
 - Fall back to the first geometry that matched our type.

Signed-off-by: John Snow 
---
 hw/block/fdc.c | 63 ++
 1 file changed, 41 insertions(+), 22 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 1d0c594..b9b9bf7 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -126,7 +126,6 @@ static const FDFormat fd_formats[] = {
 { FLOPPY_DRIVE_TYPE_NONE, -1, -1, 0, 0, },
 };
 
-__attribute__((__unused__))
 static FDriveSize drive_size(FloppyDriveType drive)
 {
 switch (drive) {
@@ -280,46 +279,66 @@ static bool pick_geometry(FDrive *drv)
 BlockBackend *blk = drv->blk;
 const FDFormat *parse;
 uint64_t nb_sectors, size;
-int i, first_match, match;
+int i;
+int match, size_match, type_match;
+bool magic = drv->drive == FLOPPY_DRIVE_TYPE_AUTO;
 
 /* We can only pick a geometry if we have a diskette. */
 if (!drv->media_inserted) {
 return false;
 }
 
+/* We need to determine the likely geometry of the inserted medium.
+ * In order of preference, we look for:
+ * (1) The same drive type and number of sectors,
+ * (2) The same diskette size and number of sectors,
+ * (3) The same number of sectors,
+ * (4) The same drive type.
+ *
+ * In all cases, matches that occur higher in the drive table will take
+ * precedence over matches that occur later in the table.
+ */
 blk_get_geometry(blk, _sectors);
-match = -1;
-first_match = -1;
+match = size_match = type_match = -1;
 for (i = 0; ; i++) {
 parse = _formats[i];
 if (parse->drive == FLOPPY_DRIVE_TYPE_NONE) {
 break;
 }
-if (drv->drive == parse->drive ||
-drv->drive == FLOPPY_DRIVE_TYPE_AUTO) {
-size = (parse->max_head + 1) * parse->max_track *
-parse->last_sect;
-if (nb_sectors == size) {
-match = i;
-break;
+size = (parse->max_head + 1) * parse->max_track * parse->last_sect;
+if (nb_sectors == size) {
+if (magic || parse->drive == drv->drive) {
+/* (1) perfect match */
+goto out;
+} else if (drive_size(parse->drive) == drive_size(drv->drive)) {
+/* (2) physical size match */
+match = (match == -1) ? i : match;
+} else {
+/* (3) nsectors match only */
+size_match = (size_match == -1) ? i : size_match;
 }
-if (first_match == -1) {
-first_match = i;
+} else if (type_match == -1) {
+if ((parse->drive == drv->drive) ||
+(magic && (parse->drive == get_fallback_drive_type(drv {
+/* (4) type matches, or type matches the autodetect default if
+ * we are using the autodetect mechanism. */
+type_match = i;
 }
 }
 }
+
 if (match == -1) {
-if (first_match == -1) {
-/* No entry found: drive_type was NONE or we neglected to add any
- * candidate geometries for our drive type into the fd_formats 
table
- */
-match = ARRAY_SIZE(fd_formats) - 1;
-} else {
-match = first_match;
-}
-parse = _formats[match];
+match = (size_match != -1) ? size_match : type_match;
+}
+
+if (match == -1) {
+/* No entry found: drive_type was NONE or we neglected to add any
+ * candidate geometries for our drive type into the fd_formats table. 
*/
+match = ARRAY_SIZE(fd_formats) - 1;
 }
+parse = &(fd_formats[match]);
 
+ out:
 if (parse->max_head == 0) {
 drv->flags &= ~FDISK_DBL_SIDES;
 } else {
-- 
2.4.3

Re: [Qemu-devel] [PATCH v5 4/6] qdev: Add a function to rename busses

[Peter Crosthwaite]

On Wed, Dec 16, 2015 at 1:45 PM, Alistair Francis
 wrote:
> Add a function which can be used to rename busses.
>
> Signed-off-by: Alistair Francis 
> ---
>
>  hw/core/qdev.c | 5 +
>  include/hw/qdev-core.h | 2 ++
>  2 files changed, 7 insertions(+)
>
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index b3ad467..29a3e9d 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -590,6 +590,11 @@ BusState *qdev_get_child_bus(DeviceState *dev, const 
> char *name)
>  return NULL;
>  }
>
> +void qdev_bus_rename(BusState *bus, const char *name)
> +{
> +memcpy((char *) bus->name, name, strlen(name) * sizeof(char));

sizeof(char) not needed, but wont this assume the current name is long
enough to hold the new name? Should name just be strduped and the old
name freed?

Regards,
Peter

> +}
> +
>  int qbus_walk_children(BusState *bus,
> qdev_walkerfn *pre_devfn, qbus_walkerfn *pre_busfn,
> qdev_walkerfn *post_devfn, qbus_walkerfn *post_busfn,
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index c537969..9653f4d 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -297,6 +297,8 @@ qemu_irq qdev_intercept_gpio_out(DeviceState *dev, 
> qemu_irq icpt,
>
>  BusState *qdev_get_child_bus(DeviceState *dev, const char *name);
>
> +void qdev_bus_rename(BusState *bus, const char *name);
> +
>  /*** Device API.  ***/
>
>  /* Register device properties.  */
> --
> 2.5.0
>

Re: [Qemu-devel] [PATCH v3 04/11] fdc: add default drive type option

[Eric Blake]

On 12/16/2015 03:16 PM, John Snow wrote:
> This patch adds a new explicit Floppy Drive Type option. The existing
> behavior in QEMU is to automatically guess a drive type based on the
> media inserted, or if a diskette is not present, arbitrarily assign one.
> 
> This behavior can be described as "auto." This patch adds explicit
> behaviors: 120, 144, 288, auto, and none. The new "auto" behavior
> is intended to mimick current behavior, while the other types pick
> one explicitly.
> 
> In a future patch, the goal is to change the FDC's default drive type
> from auto (falling back to 1.44MB) to auto (falling back to 2.88MB).
> 
> In order to allow users to obtain the old behaviors, though, a mechanism
> for specifying the exact type of drive we want is needed.
> 
> This patch adds the properties, but it is not acted on yet in favor of
> making those changes a little more explicitly clear in standalone patches
> later in this patch set.
> 
> Signed-off-by: John Snow 
> ---

> @@ -566,6 +569,7 @@ struct FDCtrl {
>  /* Timers state */
>  uint8_t timer0;
>  uint8_t timer1;
> +
>  };
>  

Spurious blank line?

Otherwise, looks good;
Reviewed-by: Eric Blake 

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v5 5/6] xlnx-zynqmp: Connect the SPI devices

[Paolo Bonzini]

On 16/12/2015 22:45, Alistair Francis wrote:
> +
> +/* Rename each SPI bus after the SPI device to allow the board
> + * to access all of the busses from the SoC.
> + */
> +spi_bus = qdev_get_child_bus(DEVICE(>spi[i]), "spi0");
> +snprintf(bus_name, 6, "spi%d", i);
> +qdev_bus_rename(spi_bus, bus_name);
> +
> +/* Add the SPI buses to the SoC child bus */
> +/* FIXME: This causes the later buses to be duplicated in
> + * the SPI devices printout when running qtre.
> + */
> +QLIST_INSERT_HEAD(>child_bus, spi_bus, sibling);

Isn't the SPI bus accessible with something similar to spi[0-5]/spi0,
even without this hack?

In any case, I would prefer qdev_bus_rename to stay in xlnx-zynqmp.c.

Paolo

Re: [Qemu-devel] [PATCH 0/5] target-i386: kvm: Increase MSR entry array limits, check for array overrun

[Paolo Bonzini]

On 16/12/2015 20:06, Eduardo Habkost wrote:
> We are dangerously close to the array limits in kvm_put_msrs()
> and kvm_get_msrs(): with the default mcg_cap configuration, we
> can set up to 148 MSRs in kvm_put_msrs(), and if we allow mcg_cap
> to be changed, we can write up to 236 MSRs[1].
> 
> This series changes the code to allocate a buffer once per VCPU,
> increase buffer size to 4096 bytes (that can hold up to 255 MSR
> entries), and check array limits before appending new entries.

Thanks, it's a good improvement.

Reviewed-by: Paolo Bonzini 

> [1] I have checked the limits by copying and pasting the
> kvm_put_msrs() code to a new file, replacing the "if" lines,
> copying the macro definitions, and adding a helper macro to
> keep track of the kvm_msr_entry_set() calls. The code can be
> seen at:
> https://gist.github.com/ehabkost/08d4177a33b8648a71ef
> 
> Eduardo Habkost (5):
>   target-i386: kvm: Allocate kvm_msrs struct once per VCPU
>   target-i386: kvm: Increase MSR_BUF_SIZE
>   target-i386: kvm: Simplify MSR array construction
>   target-i386: kvm: Simplify MSR setting functions
>   target-i386: kvm: Eliminate kvm_msr_entry_set()
> 
>  target-i386/cpu-qom.h |   4 +
>  target-i386/kvm.c | 322 
> +++---
>  2 files changed, 149 insertions(+), 177 deletions(-)
> 

[Qemu-devel] [PATCH v5 3/6] xilinx_spips: Separate the state struct into a header

[Alistair Francis]

Separate out the XilinxSPIPS struct into a separate header
file.

Signed-off-by: Alistair Francis 
---
V5:
 - Fix typos
V4:
 - Don't split off R_MOD_ID and hardcode R_MAX
V2:
 - Only split out required #defines
 - Prefix XLNX_SPIPS_

 hw/ssi/xilinx_spips.c | 46 +++
 include/hw/ssi/xilinx_spips.h | 72 +++
 2 files changed, 76 insertions(+), 42 deletions(-)
 create mode 100644 include/hw/ssi/xilinx_spips.h

diff --git a/hw/ssi/xilinx_spips.c b/hw/ssi/xilinx_spips.c
index e9471ff..2111719 100644
--- a/hw/ssi/xilinx_spips.c
+++ b/hw/ssi/xilinx_spips.c
@@ -29,6 +29,7 @@
 #include "qemu/fifo8.h"
 #include "hw/ssi/ssi.h"
 #include "qemu/bitops.h"
+#include "hw/ssi/xilinx_spips.h"
 
 #ifndef XILINX_SPIPS_ERR_DEBUG
 #define XILINX_SPIPS_ERR_DEBUG 0
@@ -103,8 +104,6 @@
 
 #define R_MOD_ID(0xFC / 4)
 
-#define R_MAX (R_MOD_ID+1)
-
 /* size of TXRX FIFOs */
 #define RXFF_A  32
 #define TXFF_A  32
@@ -135,30 +134,6 @@ typedef enum {
 } FlashCMD;
 
 typedef struct {
-SysBusDevice parent_obj;
-
-MemoryRegion iomem;
-MemoryRegion mmlqspi;
-
-qemu_irq irq;
-int irqline;
-
-uint8_t num_cs;
-uint8_t num_busses;
-
-uint8_t snoop_state;
-qemu_irq *cs_lines;
-SSIBus **spi;
-
-Fifo8 rx_fifo;
-Fifo8 tx_fifo;
-
-uint8_t num_txrx_bytes;
-
-uint32_t regs[R_MAX];
-} XilinxSPIPS;
-
-typedef struct {
 XilinxSPIPS parent_obj;
 
 uint8_t lqspi_buf[LQSPI_CACHE_SIZE];
@@ -174,19 +149,6 @@ typedef struct XilinxSPIPSClass {
 uint32_t tx_fifo_size;
 } XilinxSPIPSClass;
 
-#define TYPE_XILINX_SPIPS "xlnx.ps7-spi"
-#define TYPE_XILINX_QSPIPS "xlnx.ps7-qspi"
-
-#define XILINX_SPIPS(obj) \
- OBJECT_CHECK(XilinxSPIPS, (obj), TYPE_XILINX_SPIPS)
-#define XILINX_SPIPS_CLASS(klass) \
- OBJECT_CLASS_CHECK(XilinxSPIPSClass, (klass), TYPE_XILINX_SPIPS)
-#define XILINX_SPIPS_GET_CLASS(obj) \
- OBJECT_GET_CLASS(XilinxSPIPSClass, (obj), TYPE_XILINX_SPIPS)
-
-#define XILINX_QSPIPS(obj) \
- OBJECT_CHECK(XilinxQSPIPS, (obj), TYPE_XILINX_QSPIPS)
-
 static inline int num_effective_busses(XilinxSPIPS *s)
 {
 return (s->regs[R_LQSPI_CFG] & LQSPI_CFG_SEP_BUS &&
@@ -257,7 +219,7 @@ static void xilinx_spips_reset(DeviceState *d)
 XilinxSPIPS *s = XILINX_SPIPS(d);
 
 int i;
-for (i = 0; i < R_MAX; i++) {
+for (i = 0; i < XLNX_SPIPS_R_MAX; i++) {
 s->regs[i] = 0;
 }
 
@@ -664,7 +626,7 @@ static void xilinx_spips_realize(DeviceState *dev, Error 
**errp)
 }
 
 memory_region_init_io(>iomem, OBJECT(s), xsc->reg_ops, s,
-  "spi", R_MAX*4);
+  "spi", XLNX_SPIPS_R_MAX*4);
 sysbus_init_mmio(sbd, >iomem);
 
 s->irqline = -1;
@@ -708,7 +670,7 @@ static const VMStateDescription vmstate_xilinx_spips = {
 .fields = (VMStateField[]) {
 VMSTATE_FIFO8(tx_fifo, XilinxSPIPS),
 VMSTATE_FIFO8(rx_fifo, XilinxSPIPS),
-VMSTATE_UINT32_ARRAY(regs, XilinxSPIPS, R_MAX),
+VMSTATE_UINT32_ARRAY(regs, XilinxSPIPS, XLNX_SPIPS_R_MAX),
 VMSTATE_UINT8(snoop_state, XilinxSPIPS),
 VMSTATE_END_OF_LIST()
 }
diff --git a/include/hw/ssi/xilinx_spips.h b/include/hw/ssi/xilinx_spips.h
new file mode 100644
index 000..dbb9eef
--- /dev/null
+++ b/include/hw/ssi/xilinx_spips.h
@@ -0,0 +1,72 @@
+/*
+ * Header file for the Xilinx Zynq SPI controller
+ *
+ * Copyright (C) 2015 Xilinx Inc
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef XLNX_SPIPS_H
+#define XLNX_SPIPS_H
+
+#include "hw/ssi/ssi.h"
+#include "qemu/fifo8.h"
+
+typedef struct XilinxSPIPS XilinxSPIPS;
+
+#define XLNX_SPIPS_R_MAX(0x100 / 4)
+
+struct XilinxSPIPS {
+SysBusDevice parent_obj;
+
+MemoryRegion iomem;
+MemoryRegion mmlqspi;
+
+qemu_irq irq;
+int irqline;
+
+uint8_t num_cs;
+uint8_t num_busses;
+
+

[Qemu-devel] [PATCH v5 1/6] m25p80.c: Add sst25wf080 SPI flash device

[Alistair Francis]

Add the sst25wf080 SPI flash device.

Signed-off-by: Alistair Francis 
Reviewed-by: Peter Crosthwaite 
---

 hw/block/m25p80.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index efc43dd..7b9f97c 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -163,6 +163,7 @@ static const FlashPartInfo known_devices[] = {
 { INFO("sst25wf010",  0xbf2502,  0,  64 << 10,   2, ER_4K) },
 { INFO("sst25wf020",  0xbf2503,  0,  64 << 10,   4, ER_4K) },
 { INFO("sst25wf040",  0xbf2504,  0,  64 << 10,   8, ER_4K) },
+{ INFO("sst25wf080",  0xbf2505,  0,  64 << 10,  16, ER_4K) },
 
 /* ST Microelectronics -- newer production may have feature updates */
 { INFO("m25p05",  0x202010,  0,  32 << 10,   2, 0) },
-- 
2.5.0

[Qemu-devel] [PATCH v5 0/6] Connect the SPI devices to ZynqMP

[Alistair Francis]

Connect the SPI devices to Xilinx's ZynqMP.

I also need to make some changes to the actual SPI device to
imporove the fuctionality, but for the time being this works.

V5:
 - Fix a typo
 - Use a qdev API to rename the SPI bus
V4:
 - Rebase
 - Rename the SPI busses so that they can all be accessed from the SoC
 - Only create one SPI flash device
V3:
 - Don't reach into the SoC to get the SPI Bus
V2:
 - Connect the SPI flash in the board code
 - Update git patches to properly indicate rename
 - Add sst25wf080 as a SPI flash


Alistair Francis (6):
  m25p80.c: Add sst25wf080 SPI flash device
  ssi: Move ssi.h into a separate directory
  xilinx_spips: Separate the state struct into a header
  qdev: Add a function to rename busses
  xlnx-zynqmp: Connect the SPI devices
  xlnx-ep108: Connect the SPI Flash

 hw/arm/pxa2xx.c |  2 +-
 hw/arm/spitz.c  |  2 +-
 hw/arm/stellaris.c  |  2 +-
 hw/arm/strongarm.c  |  2 +-
 hw/arm/tosa.c   |  2 +-
 hw/arm/xilinx_zynq.c|  2 +-
 hw/arm/xlnx-ep108.c | 16 +
 hw/arm/xlnx-zynqmp.c| 38 
 hw/arm/z2.c |  2 +-
 hw/block/m25p80.c   |  3 +-
 hw/core/qdev.c  |  5 +++
 hw/display/ads7846.c|  2 +-
 hw/display/ssd0323.c|  2 +-
 hw/microblaze/petalogix_ml605_mmu.c |  2 +-
 hw/misc/max111x.c   |  2 +-
 hw/sd/ssi-sd.c  |  2 +-
 hw/ssi/pl022.c  |  2 +-
 hw/ssi/ssi.c|  2 +-
 hw/ssi/xilinx_spi.c |  2 +-
 hw/ssi/xilinx_spips.c   | 48 +++--
 include/hw/arm/xlnx-zynqmp.h|  3 ++
 include/hw/qdev-core.h  |  2 ++
 include/hw/{ => ssi}/ssi.h  | 10 +++---
 include/hw/ssi/xilinx_spips.h   | 72 +
 24 files changed, 164 insertions(+), 63 deletions(-)
 rename include/hw/{ => ssi}/ssi.h (96%)
 create mode 100644 include/hw/ssi/xilinx_spips.h

-- 
2.5.0

Re: [Qemu-devel] [PATCH v2 1/1] i386: expose floppy-related objects in SSDT

[Igor Mammedov]

On Wed, 16 Dec 2015 20:34:55 +0300
Roman Kagan  wrote:

> On Wed, Dec 16, 2015 at 05:46:57PM +0100, Igor Mammedov wrote:
> > On Wed, 16 Dec 2015 10:45:09 +0300 "Denis V. Lunev" 
> > wrote:
> > > @@ -1125,6 +1183,26 @@ build_ssdt(GArray *table_data, GArray
> > > *linker, aml_append(ssdt, scope);
> > >  }
> > >  
> > > +{
> > add this block only if floppy controller is present
> 
> ATM both qemu master (and stable branches) and your patchset define
> FDC0 unconditionally, making its _STA depend on FDEN field of the
> containing ISA device rather than any sort of qemu internal state.  I
> think it's more consistent also to always define its _FDE.  The whole
> definition of FDC0 can then be made contidional on the presence of
> the floppy controller later on top of your patchset.
Ok, I don't insist on it as I'll have to do anyway for whole FDC0 after
merging SSDT into DSDT.

The reason why I haven't done it in conversion patchset, is to keep
DSDT exactly the same as an original one, so 'make check' could prove
conversion correctness.

> 
> As for the subdevices representing floppy drives they already are
> defined only if present.
> 
> > to minimize conflicts with existing series under review,
> > I'd suggest to put it before:
> > 
> >   if (misc->applesmc_io_base) {
> 
> No prob, will do in v2.
> 
> Thanks,
> Roman.

[Qemu-devel] [PATCH v3 03/11] fdc: add disk field

[John Snow]

This allows us to distinguish between the current disk type and the
current drive type. The drive is what's reported to CMOS, the disk is
whatever the pick_geometry function suspects has been inserted.

The drive field maintains the exact same meaning as it did previously,
however pick_geometry/fd_revalidate will be refactored to *never* update
the drive field, considering it frozen in-place during an earlier
initialization call.

Before this patch, pick_geometry/fd_revalidate could only change the
drive field when it was FDRIVE_DRV_NONE, which indicated that we had
not yet reported our drive type to CMOS. After we "pick one," even
though pick_geometry/fd_revalidate re-set drv->drive, it should always
be the same as the value going into these calls, so it is effectively
already static.

As of this patch, disk and drive will always be the same, but that may
not be true by the end of this series.

Disk does not need to be migrated because it is not user-visible state
nor is it currently used for any calculations. It is purely informative,
and will be rebuilt automatically via fd_revalidate on the new host.

Signed-off-by: John Snow 
---
 hw/block/fdc.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 09bb63d..13fef23 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -133,7 +133,8 @@ typedef struct FDrive {
 FDCtrl *fdctrl;
 BlockBackend *blk;
 /* Drive status */
-FDriveType drive;
+FDriveType drive; /* CMOS drive type */
+FDriveType disk;  /* Current disk type */
 uint8_t perpendicular;/* 2.88 MB access mode*/
 /* Position */
 uint8_t head;
@@ -157,6 +158,7 @@ static void fd_init(FDrive *drv)
 drv->drive = FDRIVE_DRV_NONE;
 drv->perpendicular = 0;
 /* Disk */
+drv->disk = FDRIVE_DRV_NONE;
 drv->last_sect = 0;
 drv->max_track = 0;
 }
@@ -286,6 +288,7 @@ static void pick_geometry(FDrive *drv)
 drv->max_track = parse->max_track;
 drv->last_sect = parse->last_sect;
 drv->drive = parse->drive;
+drv->disk = drv->media_inserted ? parse->drive : FDRIVE_DRV_NONE;
 drv->media_rate = parse->rate;
 
 if (drv->media_inserted) {
-- 
2.4.3

Re: [Qemu-devel] [RFC PATCH v0 2/9] cpu: Store CPU typename in MachineState

[Igor Mammedov]

On Wed, 16 Dec 2015 17:39:02 -0200
Eduardo Habkost  wrote:

> On Wed, Dec 16, 2015 at 05:54:25PM +0100, Igor Mammedov wrote:
> > On Tue, 15 Dec 2015 14:08:09 +0530
> > Bharata B Rao  wrote:
> > 
> > > On Mon, Dec 14, 2015 at 03:29:49PM -0200, Eduardo Habkost wrote:
> > > > On Thu, Dec 10, 2015 at 11:45:37AM +0530, Bharata B Rao wrote:
> > > > > Storing CPU typename in MachineState lets us to create CPU
> > > > > threads for all architectures in uniform manner from
> > > > > arch-neutral code.
> > > > > 
> > > > > TODO: Touching only i386 and spapr targets for now
> > > > > 
> > > > > Signed-off-by: Bharata B Rao 
> > > > 
> > > > Suggestions:
> > > > 
> > > > * Name the field "cpu_base_type" to indicate it is the base CPU
> > > >   class name, not the actual CPU class name used when creating
> > > >   CPUs.
> > > > * Put it in MachineClass, as it may be useful for code that
> > > >   runs before machine->init(), in the future.
> > > 
> > > Ok.
> > > 
> > > > * Maybe make it a CPUClass* field instead of a string?
> > > 
> > > In the current use case, this base cpu type string is being passed
> > > to cpu_generic_init(const char *typename, const char *cpu_model)
> > > to create boot time CPUs with given typename and cpu_mode. So for
> > > now the string makes sense for use case.
> > > 
> > > Making it CPUClass* would necessiate more changes to
> > > cpu_generic_init().
> > how about actually leaving it as "cpu_type" and putting in it
> > actual cpu type that could be used with device_add().
> > 
> > that would get rid of keeping and passing around intermediate
> > cpu_model.
> 
> Makes sense. We only need to save both typename and cpu_model
> today because cpu_generic_init() currently encapsulates three
> steps: CPU class lookup + CPU creation + CPU feature parsing. But
> we shouldn't need to redo CPU class lookup every time.
BTW: Eduardo do you know if QEMU could somehow provide a list of
supported CPU types (i.e. not cpumodels) to libvirt?

> 
> We could just split cpu_model once, and save the resulting
> CPUClass* + featurestr, instead of saving the full cpu_model
> string and parsing it again every time.
isn't featurestr as x86/sparc specific?

Could we have field in  x86_cpu_class/sparc_cpu_class for it and set it
when cpu_model is parsed?
That way generic cpu_model parser would handle only cpu names and
target specific overrides would handle both.

> 
> The only problem is that it would require refactoring multiple
> machines/architectures that use a cpu_XXX_init(const char *cpu_model)
> helper.
> 

[Qemu-devel] [PATCH v3 11/11] fdc: change auto fallback drive for ISA FDC to 288

[John Snow]

The 2.88 drive is more suitable as a default because
it can still read 1.44 images correctly, but the reverse
is not true.

Since there exist virtio-win drivers that are shipped on
2.88 floppy images, this patch will allow VMs booted without
a floppy disk inserted to later insert a 2.88MB floppy and
have that work.

This patch has been tested with msdos, freedos, fedora,
windows 8 and windows 10 without issue: if problems do
arise for certain guests being unable to cope with 2.88MB
drives as the default, they are in the minority and can use
type=144 as needed (or insert a proper boot medium and omit
type=144/288 or use type=auto) to obtain different drive types.

As icing, the default will remain auto/144 for any pre-2.6
machine types, hopefully minimizing the impact of this change
in legacy hw to basically zero.

Signed-off-by: John Snow 
---
 hw/block/fdc.c  | 2 +-
 include/hw/compat.h | 6 +-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index b9b9bf7..745b284 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -2524,7 +2524,7 @@ static Property isa_fdc_properties[] = {
 FDRIVE_DEFAULT, qdev_prop_fdc_drive_type,
 FloppyDriveType),
 DEFINE_PROP_DEFAULT("fallback", FDCtrlISABus, state.fallback,
-FLOPPY_DRIVE_TYPE_144, qdev_prop_fdc_drive_type,
+FLOPPY_DRIVE_TYPE_288, qdev_prop_fdc_drive_type,
 FloppyDriveType),
 DEFINE_PROP_END_OF_LIST(),
 };
diff --git a/include/hw/compat.h b/include/hw/compat.h
index 24dd2c0..5a53f3c 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -2,7 +2,11 @@
 #define HW_COMPAT_H
 
 #define HW_COMPAT_2_5 \
-/* empty */
+{\
+.driver   = "fdc",\
+.property = "fallback",\
+.value= "144",\
+},
 
 #define HW_COMPAT_2_4 \
 {\
-- 
2.4.3

[Qemu-devel] [PATCH v3 02/11] fdc: refactor pick_geometry

[John Snow]

Modify this function to operate directly on FDrive objects instead of
unpacking and passing all of those parameters manually.

Helps reduce complexity in each caller, and reduces the number of args.

Signed-off-by: John Snow 
---
 hw/block/fdc.c | 54 +++---
 1 file changed, 23 insertions(+), 31 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 246b631..09bb63d 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -241,11 +241,9 @@ static void fd_recalibrate(FDrive *drv)
 fd_seek(drv, 0, 0, 1, 1);
 }
 
-static void pick_geometry(BlockBackend *blk, int *nb_heads,
-  int *max_track, int *last_sect,
-  FDriveType drive_in, FDriveType *drive,
-  FDriveRate *rate)
+static void pick_geometry(FDrive *drv)
 {
+BlockBackend *blk = drv->blk;
 const FDFormat *parse;
 uint64_t nb_sectors, size;
 int i, first_match, match;
@@ -258,8 +256,8 @@ static void pick_geometry(BlockBackend *blk, int *nb_heads,
 if (parse->drive == FDRIVE_DRV_NONE) {
 break;
 }
-if (drive_in == parse->drive ||
-drive_in == FDRIVE_DRV_NONE) {
+if (drv->drive == parse->drive ||
+drv->drive == FDRIVE_DRV_NONE) {
 size = (parse->max_head + 1) * parse->max_track *
 parse->last_sect;
 if (nb_sectors == size) {
@@ -279,41 +277,35 @@ static void pick_geometry(BlockBackend *blk, int 
*nb_heads,
 }
 parse = _formats[match];
 }
-*nb_heads = parse->max_head + 1;
-*max_track = parse->max_track;
-*last_sect = parse->last_sect;
-*drive = parse->drive;
-*rate = parse->rate;
+
+if (parse->max_head == 0) {
+drv->flags &= ~FDISK_DBL_SIDES;
+} else {
+drv->flags |= FDISK_DBL_SIDES;
+}
+drv->max_track = parse->max_track;
+drv->last_sect = parse->last_sect;
+drv->drive = parse->drive;
+drv->media_rate = parse->rate;
+
+if (drv->media_inserted) {
+FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n",
+   parse->max_head + 1,
+   drv->max_track, drv->last_sect,
+   drv->ro ? "ro" : "rw");
+}
 }
 
 /* Revalidate a disk drive after a disk change */
 static void fd_revalidate(FDrive *drv)
 {
-int nb_heads, max_track, last_sect, ro;
-FDriveType drive;
-FDriveRate rate;
-
 FLOPPY_DPRINTF("revalidate\n");
 if (drv->blk != NULL) {
-ro = blk_is_read_only(drv->blk);
-pick_geometry(drv->blk, _heads, _track,
-  _sect, drv->drive, , );
+drv->ro = blk_is_read_only(drv->blk);
+pick_geometry(drv);
 if (!drv->media_inserted) {
 FLOPPY_DPRINTF("No disk in drive\n");
-} else {
-FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n", nb_heads,
-   max_track, last_sect, ro ? "ro" : "rw");
 }
-if (nb_heads == 1) {
-drv->flags &= ~FDISK_DBL_SIDES;
-} else {
-drv->flags |= FDISK_DBL_SIDES;
-}
-drv->max_track = max_track;
-drv->last_sect = last_sect;
-drv->ro = ro;
-drv->drive = drive;
-drv->media_rate = rate;
 } else {
 FLOPPY_DPRINTF("No drive connected\n");
 drv->last_sect = 0;
-- 
2.4.3

[Qemu-devel] [PATCH v3 01/11] fdc: move pick_geometry

[John Snow]

Code motion: I want to refactor this function to work with FDrive
directly, so shuffle it below that definition.

Signed-off-by: John Snow 
---
 hw/block/fdc.c | 90 +-
 1 file changed, 45 insertions(+), 45 deletions(-)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index 4292ece..246b631 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -114,51 +114,6 @@ static const FDFormat fd_formats[] = {
 { FDRIVE_DRV_NONE, -1, -1, 0, 0, },
 };
 
-static void pick_geometry(BlockBackend *blk, int *nb_heads,
-  int *max_track, int *last_sect,
-  FDriveType drive_in, FDriveType *drive,
-  FDriveRate *rate)
-{
-const FDFormat *parse;
-uint64_t nb_sectors, size;
-int i, first_match, match;
-
-blk_get_geometry(blk, _sectors);
-match = -1;
-first_match = -1;
-for (i = 0; ; i++) {
-parse = _formats[i];
-if (parse->drive == FDRIVE_DRV_NONE) {
-break;
-}
-if (drive_in == parse->drive ||
-drive_in == FDRIVE_DRV_NONE) {
-size = (parse->max_head + 1) * parse->max_track *
-parse->last_sect;
-if (nb_sectors == size) {
-match = i;
-break;
-}
-if (first_match == -1) {
-first_match = i;
-}
-}
-}
-if (match == -1) {
-if (first_match == -1) {
-match = 1;
-} else {
-match = first_match;
-}
-parse = _formats[match];
-}
-*nb_heads = parse->max_head + 1;
-*max_track = parse->max_track;
-*last_sect = parse->last_sect;
-*drive = parse->drive;
-*rate = parse->rate;
-}
-
 #define GET_CUR_DRV(fdctrl) ((fdctrl)->cur_drv)
 #define SET_CUR_DRV(fdctrl, drive) ((fdctrl)->cur_drv = (drive))
 
@@ -286,6 +241,51 @@ static void fd_recalibrate(FDrive *drv)
 fd_seek(drv, 0, 0, 1, 1);
 }
 
+static void pick_geometry(BlockBackend *blk, int *nb_heads,
+  int *max_track, int *last_sect,
+  FDriveType drive_in, FDriveType *drive,
+  FDriveRate *rate)
+{
+const FDFormat *parse;
+uint64_t nb_sectors, size;
+int i, first_match, match;
+
+blk_get_geometry(blk, _sectors);
+match = -1;
+first_match = -1;
+for (i = 0; ; i++) {
+parse = _formats[i];
+if (parse->drive == FDRIVE_DRV_NONE) {
+break;
+}
+if (drive_in == parse->drive ||
+drive_in == FDRIVE_DRV_NONE) {
+size = (parse->max_head + 1) * parse->max_track *
+parse->last_sect;
+if (nb_sectors == size) {
+match = i;
+break;
+}
+if (first_match == -1) {
+first_match = i;
+}
+}
+}
+if (match == -1) {
+if (first_match == -1) {
+match = 1;
+} else {
+match = first_match;
+}
+parse = _formats[match];
+}
+*nb_heads = parse->max_head + 1;
+*max_track = parse->max_track;
+*last_sect = parse->last_sect;
+*drive = parse->drive;
+*rate = parse->rate;
+}
+
 /* Revalidate a disk drive after a disk change */
 static void fd_revalidate(FDrive *drv)
 {
-- 
2.4.3

[Qemu-devel] [PATCH v3 05/11] fdc: Add fallback option

[John Snow]

Add the fallback drive type as an option so we can control
the behavior as a function of the QEMU machine version.

Signed-off-by: John Snow 
---
 hw/block/fdc.c | 23 +++
 1 file changed, 23 insertions(+)

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index ad0e052..b587de8 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -155,6 +155,9 @@ typedef struct FDrive {
 bool media_inserted;  /* Is there a medium in the tray */
 } FDrive;
 
+
+static FloppyDriveType get_fallback_drive_type(FDrive *drv);
+
 static void fd_init(FDrive *drv)
 {
 /* Drive */
@@ -570,8 +573,15 @@ struct FDCtrl {
 uint8_t timer0;
 uint8_t timer1;
 
+FloppyDriveType fallback;
 };
 
+__attribute__((__unused__))
+static FloppyDriveType get_fallback_drive_type(FDrive *drv)
+{
+return drv->fdctrl->fallback;
+}
+
 #define TYPE_SYSBUS_FDC "base-sysbus-fdc"
 #define SYSBUS_FDC(obj) OBJECT_CHECK(FDCtrlSysBus, (obj), TYPE_SYSBUS_FDC)
 
@@ -2302,6 +2312,10 @@ static void fdctrl_realize_common(FDCtrl *fdctrl, Error 
**errp)
 int i, j;
 static int command_tables_inited = 0;
 
+if (fdctrl->fallback == FLOPPY_DRIVE_TYPE_AUTO) {
+error_setg(errp, "Cannot choose a fallback FDrive type of 'auto'");
+}
+
 /* Fill 'command_to_handler' lookup table */
 if (!command_tables_inited) {
 command_tables_inited = 1;
@@ -2433,6 +2447,9 @@ static Property isa_fdc_properties[] = {
 DEFINE_PROP_DEFAULT("fdtypeB", FDCtrlISABus, state.drives[1].drive,
 FDRIVE_DEFAULT, qdev_prop_fdc_drive_type,
 FloppyDriveType),
+DEFINE_PROP_DEFAULT("fallback", FDCtrlISABus, state.fallback,
+FLOPPY_DRIVE_TYPE_144, qdev_prop_fdc_drive_type,
+FloppyDriveType),
 DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -2487,6 +2504,9 @@ static Property sysbus_fdc_properties[] = {
 DEFINE_PROP_DEFAULT("fdtypeB", FDCtrlSysBus, state.drives[1].drive,
 FDRIVE_DEFAULT, qdev_prop_fdc_drive_type,
 FloppyDriveType),
+DEFINE_PROP_DEFAULT("fallback", FDCtrlISABus, state.fallback,
+FLOPPY_DRIVE_TYPE_144, qdev_prop_fdc_drive_type,
+FloppyDriveType),
 DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -2510,6 +2530,9 @@ static Property sun4m_fdc_properties[] = {
 DEFINE_PROP_DEFAULT("fdtype", FDCtrlSysBus, state.drives[0].drive,
 FDRIVE_DEFAULT, qdev_prop_fdc_drive_type,
 FloppyDriveType),
+DEFINE_PROP_DEFAULT("fallback", FDCtrlISABus, state.fallback,
+FLOPPY_DRIVE_TYPE_144, qdev_prop_fdc_drive_type,
+FloppyDriveType),
 DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.4.3

[Qemu-devel] [PATCH v14 Resend 12/13] vfio-pci: pass the aer error to guest

[Cao jin]

From: Chen Fan 

when the vfio device encounters an uncorrectable error in host,
the vfio_pci driver will signal the eventfd registered by this
vfio device, the results in the qemu eventfd handler getting
invoked.

this patch is to pass the error to guest and have the guest driver
recover from the error.

Signed-off-by: Chen Fan 
---
 hw/vfio/pci.c | 53 +++--
 1 file changed, 47 insertions(+), 6 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index df32618..c1300e9 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -2543,18 +2543,59 @@ static void vfio_put_device(VFIOPCIDevice *vdev)
 static void vfio_err_notifier_handler(void *opaque)
 {
 VFIOPCIDevice *vdev = opaque;
+PCIDevice *dev = >pdev;
+PCIEAERMsg msg = {
+.severity = 0,
+.source_id = (pci_bus_num(dev->bus) << 8) | dev->devfn,
+};
 
 if (!event_notifier_test_and_clear(>err_notifier)) {
 return;
 }
 
 /*
- * TBD. Retrieve the error details and decide what action
- * needs to be taken. One of the actions could be to pass
- * the error to the guest and have the guest driver recover
- * from the error. This requires that PCIe capabilities be
- * exposed to the guest. For now, we just terminate the
- * guest to contain the error.
+ * in case the real hardware configration has been changed,
+ * here we should recheck the bus reset capability.
+ */
+if ((vdev->features & VFIO_FEATURE_ENABLE_AER) &&
+vfio_check_host_bus_reset(vdev)) {
+goto stop;
+}
+/*
+ * we should read the error details from the real hardware
+ * configuration spaces, here we only need to do is signaling
+ * to guest an uncorrectable error has occurred.
+ */
+if ((vdev->features & VFIO_FEATURE_ENABLE_AER) &&
+dev->exp.aer_cap) {
+uint8_t *aer_cap = dev->config + dev->exp.aer_cap;
+uint32_t uncor_status;
+bool isfatal;
+
+uncor_status = vfio_pci_read_config(dev,
+   dev->exp.aer_cap + PCI_ERR_UNCOR_STATUS, 4);
+
+/*
+ * if we receive the error signal but not this device, we can
+ * just ignore it.
+ */
+if (!(uncor_status & ~0UL)) {
+return;
+}
+
+isfatal = uncor_status & pci_get_long(aer_cap + PCI_ERR_UNCOR_SEVER);
+
+msg.severity = isfatal ? PCI_ERR_ROOT_CMD_FATAL_EN :
+ PCI_ERR_ROOT_CMD_NONFATAL_EN;
+
+pcie_aer_msg(dev, );
+return;
+}
+
+stop:
+/*
+ * If the aer capability is not exposed to the guest. we just
+ * terminate the guest to contain the error.
  */
 
 error_report("%s(%04x:%02x:%02x.%x) Unrecoverable error detected.  "
-- 
1.9.3

[Qemu-devel] [PATCH v14 Resend 05/13] vfio: add pcie extanded capability support

[Cao jin]

From: Chen Fan 

For vfio pcie device, we could expose the extended capability on
PCIE bus. in order to avoid config space broken, we introduce
a copy config for parsing extended caps. and rebuild the pcie
extended config space.

Signed-off-by: Chen Fan 
---
 hw/vfio/pci.c | 70 ++-
 1 file changed, 69 insertions(+), 1 deletion(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index e305cda..4bc2b51 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1481,6 +1481,21 @@ static uint8_t vfio_std_cap_max_size(PCIDevice *pdev, 
uint8_t pos)
 return next - pos;
 }
 
+
+static uint16_t vfio_ext_cap_max_size(const uint8_t *config, uint16_t pos)
+{
+uint16_t tmp, next = PCIE_CONFIG_SPACE_SIZE;
+
+for (tmp = PCI_CONFIG_SPACE_SIZE; tmp;
+tmp = PCI_EXT_CAP_NEXT(pci_get_long(config + tmp))) {
+if (tmp > pos && tmp < next) {
+next = tmp;
+}
+}
+
+return next - pos;
+}
+
 static void vfio_set_word_bits(uint8_t *buf, uint16_t val, uint16_t mask)
 {
 pci_set_word(buf, (pci_get_word(buf) & ~mask) | val);
@@ -1791,16 +1806,69 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, 
uint8_t pos)
 return 0;
 }
 
+static int vfio_add_ext_cap(VFIOPCIDevice *vdev)
+{
+PCIDevice *pdev = >pdev;
+uint32_t header;
+uint16_t cap_id, next, size;
+uint8_t cap_ver;
+uint8_t *config;
+
+/*
+ * In order to avoid breaking config space, create a copy to
+ * use for parsing extended capabilities.
+ */
+config = g_memdup(pdev->config, vdev->config_size);
+
+for (next = PCI_CONFIG_SPACE_SIZE; next;
+ next = PCI_EXT_CAP_NEXT(pci_get_long(config + next))) {
+header = pci_get_long(config + next);
+cap_id = PCI_EXT_CAP_ID(header);
+cap_ver = PCI_EXT_CAP_VER(header);
+
+/*
+ * If it becomes important to configure extended capabilities to their
+ * actual size, use this as the default when it's something we don't
+ * recognize. Since QEMU doesn't actually handle many of the config
+ * accesses, exact size doesn't seem worthwhile.
+ */
+size = vfio_ext_cap_max_size(config, next);
+
+pcie_add_capability(pdev, cap_id, cap_ver, next, size);
+pci_set_long(dev->config + next, PCI_EXT_CAP(cap_id, cap_ver, 0));
+
+/* Use emulated next pointer to allow dropping extended caps */
+pci_long_test_and_set_mask(vdev->emulated_config_bits + next,
+   PCI_EXT_CAP_NEXT_MASK);
+}
+
+g_free(config);
+return 0;
+}
+
 static int vfio_add_capabilities(VFIOPCIDevice *vdev)
 {
 PCIDevice *pdev = >pdev;
+int ret;
 
 if (!(pdev->config[PCI_STATUS] & PCI_STATUS_CAP_LIST) ||
 !pdev->config[PCI_CAPABILITY_LIST]) {
 return 0; /* Nothing to add */
 }
 
-return vfio_add_std_cap(vdev, pdev->config[PCI_CAPABILITY_LIST]);
+ret = vfio_add_std_cap(vdev, pdev->config[PCI_CAPABILITY_LIST]);
+if (ret) {
+return ret;
+}
+
+/* on PCI bus, it doesn't make sense to expose extended capabilities. */
+if (!pci_is_express(pdev) ||
+!pci_bus_is_express(pdev->bus) ||
+!pci_get_long(pdev->config + PCI_CONFIG_SPACE_SIZE)) {
+return 0;
+}
+
+return vfio_add_ext_cap(vdev);
 }
 
 static void vfio_pci_pre_reset(VFIOPCIDevice *vdev)
-- 
1.9.3

[Qemu-devel] [PATCH v14 Resend 01/13] vfio: extract vfio_get_hot_reset_info as a single function

[Cao jin]

From: Chen Fan 

the function is used to get affected devices by bus reset.
so here extract it, and can used for aer soon.

Signed-off-by: Chen Fan 
---
 hw/vfio/pci.c | 66 +++
 1 file changed, 48 insertions(+), 18 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 8fadbcf..464e6b7 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1628,6 +1628,51 @@ static void vfio_check_af_flr(VFIOPCIDevice *vdev, 
uint8_t pos)
 }
 }
 
+/*
+ * return negative with errno, return 0 on success.
+ * if success, the point of ret_info fill with the affected device reset info.
+ *
+ */
+static int vfio_get_hot_reset_info(VFIOPCIDevice *vdev,
+   struct vfio_pci_hot_reset_info **ret_info)
+{
+struct vfio_pci_hot_reset_info *info;
+int ret, count;
+
+*ret_info = NULL;
+
+info = g_malloc0(sizeof(*info));
+info->argsz = sizeof(*info);
+
+ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_GET_PCI_HOT_RESET_INFO, info);
+if (ret && errno != ENOSPC) {
+ret = -errno;
+goto error;
+}
+
+count = info->count;
+
+info = g_realloc(info, sizeof(*info) +
+ (count * sizeof(struct vfio_pci_dependent_device)));
+info->argsz = sizeof(*info) +
+  (count * sizeof(struct vfio_pci_dependent_device));
+
+ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_GET_PCI_HOT_RESET_INFO, info);
+if (ret) {
+ret = -errno;
+error_report("vfio: hot reset info failed: %m");
+goto error;
+}
+
+*ret_info = info;
+info = NULL;
+
+return 0;
+error:
+g_free(info);
+return ret;
+}
+
 static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos)
 {
 PCIDevice *pdev = >pdev;
@@ -1767,7 +1812,7 @@ static bool vfio_pci_host_match(PCIHostDeviceAddress 
*host1,
 static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool single)
 {
 VFIOGroup *group;
-struct vfio_pci_hot_reset_info *info;
+struct vfio_pci_hot_reset_info *info = NULL;
 struct vfio_pci_dependent_device *devices;
 struct vfio_pci_hot_reset *reset;
 int32_t *fds;
@@ -1779,12 +1824,8 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool 
single)
 vfio_pci_pre_reset(vdev);
 vdev->vbasedev.needs_reset = false;
 
-info = g_malloc0(sizeof(*info));
-info->argsz = sizeof(*info);
-
-ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_GET_PCI_HOT_RESET_INFO, info);
-if (ret && errno != ENOSPC) {
-ret = -errno;
+ret = vfio_get_hot_reset_info(vdev, );
+if (ret) {
 if (!vdev->has_pm_reset) {
 error_report("vfio: Cannot reset device %04x:%02x:%02x.%x, "
  "no available reset mechanism.", vdev->host.domain,
@@ -1793,18 +1834,7 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool 
single)
 goto out_single;
 }
 
-count = info->count;
-info = g_realloc(info, sizeof(*info) + (count * sizeof(*devices)));
-info->argsz = sizeof(*info) + (count * sizeof(*devices));
 devices = >devices[0];
-
-ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_GET_PCI_HOT_RESET_INFO, info);
-if (ret) {
-ret = -errno;
-error_report("vfio: hot reset info failed: %m");
-goto out_single;
-}
-
 trace_vfio_pci_hot_reset_has_dep_devices(vdev->vbasedev.name);
 
 /* Verify that we have all the groups required */
-- 
1.9.3

[Qemu-devel] [PATCH v14 Resend 02/13] vfio: squeeze out vfio_pci_do_hot_reset for support bus reset

[Cao jin]

From: Chen Fan 

squeeze out vfio_pci_do_hot_reset to do host bus reset when AER recovery.

Signed-off-by: Chen Fan 
---
 hw/vfio/pci.c | 75 +++
 1 file changed, 44 insertions(+), 31 deletions(-)

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 464e6b7..f333dfc 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1673,6 +1673,48 @@ error:
 return ret;
 }
 
+static int vfio_pci_do_hot_reset(VFIOPCIDevice *vdev,
+ struct vfio_pci_hot_reset_info *info)
+{
+VFIOGroup *group;
+struct vfio_pci_hot_reset *reset;
+int32_t *fds;
+int ret, i, count;
+struct vfio_pci_dependent_device *devices;
+
+/* Determine how many group fds need to be passed */
+count = 0;
+devices = >devices[0];
+QLIST_FOREACH(group, _group_list, next) {
+for (i = 0; i < info->count; i++) {
+if (group->groupid == devices[i].group_id) {
+count++;
+break;
+}
+}
+}
+
+reset = g_malloc0(sizeof(*reset) + (count * sizeof(*fds)));
+reset->argsz = sizeof(*reset) + (count * sizeof(*fds));
+fds = >group_fds[0];
+
+/* Fill in group fds */
+QLIST_FOREACH(group, _group_list, next) {
+for (i = 0; i < info->count; i++) {
+if (group->groupid == devices[i].group_id) {
+fds[reset->count++] = group->fd;
+break;
+}
+}
+}
+
+/* Bus reset! */
+ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_PCI_HOT_RESET, reset);
+g_free(reset);
+
+return ret;
+}
+
 static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos)
 {
 PCIDevice *pdev = >pdev;
@@ -1814,9 +1856,7 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool 
single)
 VFIOGroup *group;
 struct vfio_pci_hot_reset_info *info = NULL;
 struct vfio_pci_dependent_device *devices;
-struct vfio_pci_hot_reset *reset;
-int32_t *fds;
-int ret, i, count;
+int ret, i;
 bool multi = false;
 
 trace_vfio_pci_hot_reset(vdev->vbasedev.name, single ? "one" : "multi");
@@ -1895,34 +1935,7 @@ static int vfio_pci_hot_reset(VFIOPCIDevice *vdev, bool 
single)
 goto out_single;
 }
 
-/* Determine how many group fds need to be passed */
-count = 0;
-QLIST_FOREACH(group, _group_list, next) {
-for (i = 0; i < info->count; i++) {
-if (group->groupid == devices[i].group_id) {
-count++;
-break;
-}
-}
-}
-
-reset = g_malloc0(sizeof(*reset) + (count * sizeof(*fds)));
-reset->argsz = sizeof(*reset) + (count * sizeof(*fds));
-fds = >group_fds[0];
-
-/* Fill in group fds */
-QLIST_FOREACH(group, _group_list, next) {
-for (i = 0; i < info->count; i++) {
-if (group->groupid == devices[i].group_id) {
-fds[reset->count++] = group->fd;
-break;
-}
-}
-}
-
-/* Bus reset! */
-ret = ioctl(vdev->vbasedev.fd, VFIO_DEVICE_PCI_HOT_RESET, reset);
-g_free(reset);
+ret = vfio_pci_do_hot_reset(vdev, info);
 
 trace_vfio_pci_hot_reset_result(vdev->vbasedev.name,
 ret ? "%m" : "Success");
-- 
1.9.3

Re: [Qemu-devel] [PATCH] change type of pci_bridge_initfn() to void

[Cao jin]

Ping

On 11/30/2015 05:19 PM, Michael S. Tsirkin wrote:

On Mon, Nov 30, 2015 at 05:00:44PM +0800, Cao jin wrote:

It always return 0(success), change its type to void, and modify its caller.
Doing this can reduce a error path of its caller, and it is also good when
convert init() to realize()

Signed-off-by: Cao jin 


Sounds good, but pls remember to ping me after 2.5 is out.


---
  hw/pci-bridge/i82801b11.c  | 5 +
  hw/pci-bridge/ioh3420.c| 6 +-
  hw/pci-bridge/pci_bridge_dev.c | 8 +++-
  hw/pci-bridge/xio3130_downstream.c | 6 +-
  hw/pci-bridge/xio3130_upstream.c   | 6 +-
  hw/pci-host/apb.c  | 5 +
  hw/pci/pci_bridge.c| 3 +--
  include/hw/pci/pci_bridge.h| 2 +-
  8 files changed, 10 insertions(+), 31 deletions(-)

leave DEC 21154 PCI-PCI bridge unchanged because it is better to handle it
when convert init() to realize().

diff --git a/hw/pci-bridge/i82801b11.c b/hw/pci-bridge/i82801b11.c
index 7e79bc0..b21bc2c 100644
--- a/hw/pci-bridge/i82801b11.c
+++ b/hw/pci-bridge/i82801b11.c
@@ -61,10 +61,7 @@ static int i82801b11_bridge_initfn(PCIDevice *d)
  {
  int rc;

-rc = pci_bridge_initfn(d, TYPE_PCI_BUS);
-if (rc < 0) {
-return rc;
-}
+pci_bridge_initfn(d, TYPE_PCI_BUS);

  rc = pci_bridge_ssvid_init(d, I82801ba_SSVID_OFFSET,
 I82801ba_SSVID_SVID, I82801ba_SSVID_SSID);
diff --git a/hw/pci-bridge/ioh3420.c b/hw/pci-bridge/ioh3420.c
index cce2fdd..eead195 100644
--- a/hw/pci-bridge/ioh3420.c
+++ b/hw/pci-bridge/ioh3420.c
@@ -97,11 +97,7 @@ static int ioh3420_initfn(PCIDevice *d)
  PCIESlot *s = PCIE_SLOT(d);
  int rc;

-rc = pci_bridge_initfn(d, TYPE_PCIE_BUS);
-if (rc < 0) {
-return rc;
-}
-
+pci_bridge_initfn(d, TYPE_PCIE_BUS);
  pcie_port_init_reg(d);

  rc = pci_bridge_ssvid_init(d, IOH_EP_SSVID_OFFSET,
diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c
index 26aded9..bc3e1b7 100644
--- a/hw/pci-bridge/pci_bridge_dev.c
+++ b/hw/pci-bridge/pci_bridge_dev.c
@@ -52,10 +52,8 @@ static int pci_bridge_dev_initfn(PCIDevice *dev)
  PCIBridgeDev *bridge_dev = PCI_BRIDGE_DEV(dev);
  int err;

-err = pci_bridge_initfn(dev, TYPE_PCI_BUS);
-if (err) {
-goto bridge_error;
-}
+pci_bridge_initfn(dev, TYPE_PCI_BUS);
+
  if (bridge_dev->flags & (1 << PCI_BRIDGE_DEV_F_SHPC_REQ)) {
  dev->config[PCI_INTERRUPT_PIN] = 0x1;
  memory_region_init(_dev->bar, OBJECT(dev), "shpc-bar",
@@ -94,7 +92,7 @@ slotid_error:
  }
  shpc_error:
  pci_bridge_exitfn(dev);
-bridge_error:
+
  return err;
  }

diff --git a/hw/pci-bridge/xio3130_downstream.c 
b/hw/pci-bridge/xio3130_downstream.c
index 86b7970..012daf3 100644
--- a/hw/pci-bridge/xio3130_downstream.c
+++ b/hw/pci-bridge/xio3130_downstream.c
@@ -61,11 +61,7 @@ static int xio3130_downstream_initfn(PCIDevice *d)
  PCIESlot *s = PCIE_SLOT(d);
  int rc;

-rc = pci_bridge_initfn(d, TYPE_PCIE_BUS);
-if (rc < 0) {
-return rc;
-}
-
+pci_bridge_initfn(d, TYPE_PCIE_BUS);
  pcie_port_init_reg(d);

  rc = msi_init(d, XIO3130_MSI_OFFSET, XIO3130_MSI_NR_VECTOR,
diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
index eada582..434c8fd 100644
--- a/hw/pci-bridge/xio3130_upstream.c
+++ b/hw/pci-bridge/xio3130_upstream.c
@@ -56,11 +56,7 @@ static int xio3130_upstream_initfn(PCIDevice *d)
  PCIEPort *p = PCIE_PORT(d);
  int rc;

-rc = pci_bridge_initfn(d, TYPE_PCIE_BUS);
-if (rc < 0) {
-return rc;
-}
-
+pci_bridge_initfn(d, TYPE_PCIE_BUS);
  pcie_port_init_reg(d);

  rc = msi_init(d, XIO3130_MSI_OFFSET, XIO3130_MSI_NR_VECTOR,
diff --git a/hw/pci-host/apb.c b/hw/pci-host/apb.c
index 599768e..e9117b9 100644
--- a/hw/pci-host/apb.c
+++ b/hw/pci-host/apb.c
@@ -636,10 +636,7 @@ static int apb_pci_bridge_initfn(PCIDevice *dev)
  {
  int rc;

-rc = pci_bridge_initfn(dev, TYPE_PCI_BUS);
-if (rc < 0) {
-return rc;
-}
+pci_bridge_initfn(dev, TYPE_PCI_BUS);

  /*
   * command register:
diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
index 40c97b1..5c30795 100644
--- a/hw/pci/pci_bridge.c
+++ b/hw/pci/pci_bridge.c
@@ -332,7 +332,7 @@ void pci_bridge_reset(DeviceState *qdev)
  }

  /* default qdev initialization function for PCI-to-PCI bridge */
-int pci_bridge_initfn(PCIDevice *dev, const char *typename)
+void pci_bridge_initfn(PCIDevice *dev, const char *typename)
  {
  PCIBus *parent = dev->bus;
  PCIBridge *br = PCI_BRIDGE(dev);
@@ -378,7 +378,6 @@ int pci_bridge_initfn(PCIDevice *dev, const char *typename)
  br->windows = pci_bridge_region_init(br);
  QLIST_INIT(_bus->child);
  QLIST_INSERT_HEAD(>child, sec_bus, sibling);
-return 0;
  }

  /* default qdev clean up function for PCI-to-PCI bridge */
diff --git 

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 2/4] spapr: Handle "ibm, nmi-register" and "ibm, nmi-interlock" RTAS calls

[Aravinda Prasad]

On Thursday 17 December 2015 09:21 AM, David Gibson wrote:
> On Wed, Dec 16, 2015 at 11:38:22AM +0530, Aravinda Prasad wrote:
>> This patch adds support in QEMU to handle "ibm,nmi-register"
>> and "ibm,nmi-interlock" RTAS calls.
>>
>> The machine check notification address is saved when the
>> OS issues "ibm,nmi-register" RTAS call.
>>
>> This patch also handles the case when multiple processors
>> experience machine check at or about the same time by
>> handling "ibm,nmi-interlock" call. In such cases, as per
>> PAPR, subsequent processors serialize waiting for the first
>> processor to issue the "ibm,nmi-interlock" call. The second
>> processor waits till the first processor, which also
>> received a machine check error, is done reading the error
>> log. The first processor issues "ibm,nmi-interlock" call
>> when the error log is consumed. This patch implements the
>> releasing part of the error-log while subsequent patch
>> (which builds error log) handles the locking part.
>>
>> Signed-off-by: Aravinda Prasad 
>> ---
>>  hw/ppc/spapr_rtas.c|   36 
>>  include/hw/ppc/spapr.h |   10 +-
>>  2 files changed, 45 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>> index 9869bc9..17c4672 100644
>> --- a/hw/ppc/spapr_rtas.c
>> +++ b/hw/ppc/spapr_rtas.c
>> @@ -597,6 +597,38 @@ out:
>>  rtas_st(rets, 0, rc);
>>  }
>>  
>> +static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
>> +  sPAPRMachineState *spapr,
>> +  uint32_t token, uint32_t nargs,
>> +  target_ulong args,
>> +  uint32_t nret, target_ulong rets)
>> +{
>> +spapr->mc_in_progress = false;
>> +qemu_cond_init(>mc_delivery_cond);
> 
> I think initializing mc_in_progress and the condition variable should
> go in the overall initialization (in fact during system reset) instead
> of here.  Things using these shouldn't be invoked until after the
> nmi_register, but it's safer to have the qemu internal variables
> initialized beforehand.

sure

> 
>> +spapr->guest_machine_check_addr = rtas_ld(args, 1);
>> +rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>> +}
> 
> It also looks like you need to add code to clear
> guest_machine_check_addr if the system is reset

will add it

> 
>> +static void rtas_ibm_nmi_interlock(PowerPCCPU *cpu,
>> +   sPAPRMachineState *spapr,
>> +   uint32_t token, uint32_t nargs,
>> +   target_ulong args,
>> +   uint32_t nret, target_ulong rets)
>> +{
>> +if (!spapr->guest_machine_check_addr) {
>> +/* NMI register not called */
>> +rtas_st(rets, 0, RTAS_OUT_PARAM_ERROR);
>> +} else {
>> +/*
>> + * VCPU issuing "ibm,nmi-interlock" is done with NMI handling,
>> + * hence unset mc_in_progress.
>> + */
>> +spapr->mc_in_progress = false;
>> +qemu_cond_signal(>mc_delivery_cond);
>> +rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>> +}
>> +}
>> +
>>  static struct rtas_call {
>>  const char *name;
>>  spapr_rtas_fn fn;
>> @@ -747,6 +779,10 @@ static void core_rtas_register_types(void)
>>  rtas_get_sensor_state);
>>  spapr_rtas_register(RTAS_IBM_CONFIGURE_CONNECTOR, 
>> "ibm,configure-connector",
>>  rtas_ibm_configure_connector);
>> +spapr_rtas_register(RTAS_IBM_NMI_REGISTER, "ibm,nmi-register",
>> +rtas_ibm_nmi_register);
>> +spapr_rtas_register(RTAS_IBM_NMI_INTERLOCK, "ibm,nmi-interlock",
>> +rtas_ibm_nmi_interlock);
>>  }
>>  
>>  type_init(core_rtas_register_types)
>> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
>> index b5cadd7..de84a4e 100644
>> --- a/include/hw/ppc/spapr.h
>> +++ b/include/hw/ppc/spapr.h
>> @@ -74,6 +74,12 @@ struct sPAPRMachineState {
>>  /* RTAS state */
>>  QTAILQ_HEAD(, sPAPRConfigureConnectorState) ccs_list;
>>  
>> +/* State related to "ibm,nmi-register" and "ibm,nmi-interlock" calls */
>> +target_ulong guest_machine_check_addr;
>> +bool mc_in_progress;
>> +int mc_cpu;
> 
> mc_cpu doesn't appear to be used.

mc_cpu is used in patch 3.

> 
> I think guest_machine_check_addr and mc_in_progress will also need to
> be added to migration state.

sure. I have not looked into migration state before, but I think I need
to check if the KVM on target after migration has the KVM_CAP_PPC_FWNMI
capability.

Regards,
Aravinda

> 
>> +QemuCond mc_delivery_cond;
>> +
>>  /*< public >*/
>>  char *kvm_type;
>>  };
>> @@ -458,8 +464,10 @@ int spapr_allocate_irq_block(int num, bool lsi, bool 
>> msi);
>>  #define RTAS_IBM_SET_SLOT_RESET (RTAS_TOKEN_BASE + 0x23)
>>  #define RTAS_IBM_CONFIGURE_PE  

Re: [Qemu-devel] [Qemu-ppc] [PATCH v2 3/4] target-ppc: Handle NMI guest exit

[Aravinda Prasad]

On Thursday 17 December 2015 09:30 AM, David Gibson wrote:
> On Wed, Dec 16, 2015 at 11:38:37AM +0530, Aravinda Prasad wrote:
>> Memory error such as bit flips that cannot be corrected
>> by hardware are passed on to the kernel for handling.
>> If the memory address in error belongs to guest then
>> guest kernel is responsible for taking suitable action.
>> Patch [1] enhances KVM to exit guest with exit reason
>> set to KVM_EXIT_NMI in such cases.
>>
>> This patch handles KVM_EXIT_NMI exit. If the guest OS
>> has registered the machine check handling routine by
>> calling "ibm,nmi-register", then the handler builds
>> the error log and invokes the registered handler else
>> invokes the handler at 0x200.
>>
>> [1] http://marc.info/?l=kvm=145024538523497
>>
>> Signed-off-by: Aravinda Prasad 
>> ---
>>  cpus.c   |5 +++
>>  hw/ppc/spapr.c   |6 +++
>>  include/qemu/main-loop.h |8 
>>  target-ppc/kvm.c |   86 
>> ++
>>  target-ppc/kvm_ppc.h |   81 +++
>>  5 files changed, 186 insertions(+)
>>
>> diff --git a/cpus.c b/cpus.c
>> index 056..7b7dd0f 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -1154,6 +1154,11 @@ void qemu_mutex_unlock_iothread(void)
>>  qemu_mutex_unlock(_global_mutex);
>>  }
>>  
>> +void qemu_cond_wait_iothread(QemuCond *cond)
>> +{
>> +qemu_cond_wait(cond, _global_mutex);
>> +}
>> +
> 
> Even though it's trivial, this generic change should probably go in a
> separate patch from the papr specific pieces.
> 
> Speaking of which, I think it's sufficiently trivial you could just
> inline it in the header.

I had it in a separate patch before. I changed my mind later as it is
trivial.

I will include it in a separate patch.

> 
>>  static int all_vcpus_paused(void)
>>  {
>>  CPUState *cpu;
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 05926a3..501dd70 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -1556,6 +1556,12 @@ static void ppc_spapr_init(MachineState *machine)
>>  exit(1);
>>  }
>>  spapr->rtas_size = get_image_size(filename);
>> +
>> +assert(spapr->rtas_size < RTAS_ERRLOG_OFFSET);
>> +
>> +/* Resize blob to accommodate error log. */
>> +spapr->rtas_size = RTAS_ERRLOG_OFFSET + sizeof(struct RtasMCELog);
>> +
>>  spapr->rtas_blob = g_malloc(spapr->rtas_size);
>>  if (load_image_size(filename, spapr->rtas_blob, spapr->rtas_size) < 0) {
>>  error_report("Could not load LPAR rtas '%s'", filename);
>> diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
>> index 9976909..c4d4446 100644
>> --- a/include/qemu/main-loop.h
>> +++ b/include/qemu/main-loop.h
>> @@ -263,6 +263,14 @@ void qemu_mutex_lock_iothread(void);
>>   */
>>  void qemu_mutex_unlock_iothread(void);
>>  
>> +/**
>> + * qemu_cond_wait_iothread: Wait on condition for the main loop mutex
>> + *
>> + * This function atomically releases the main loop mutex and causes
>> + * the calling thread to block on the condition.
>> + */
>> +void qemu_cond_wait_iothread(QemuCond *cond);
>> +
>>  /* internal interfaces */
>>  
>>  void qemu_fd_register(int fd);
>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>> index 110436d..2bbb46d 100644
>> --- a/target-ppc/kvm.c
>> +++ b/target-ppc/kvm.c
>> @@ -1665,6 +1665,11 @@ int kvm_arch_handle_exit(CPUState *cs, struct kvm_run 
>> *run)
>>  ret = 0;
>>  break;
>>  
>> +case KVM_EXIT_NMI:
>> +DPRINTF("handle NMI exception\n");
>> +ret = kvm_handle_nmi(cpu);
>> +break;
>> +
>>  default:
>>  fprintf(stderr, "KVM: unknown exit reason %d\n", run->exit_reason);
>>  ret = -1;
>> @@ -2484,3 +2489,84 @@ int kvm_arch_msi_data_to_gsi(uint32_t data)
>>  {
>>  return data & 0x;
>>  }
>> +
>> +int kvm_handle_nmi(PowerPCCPU *cpu)
>> +{
>> +struct RtasMCELog mc_log;
>> +CPUPPCState *env = >env;
> 
> You go from cpu to env here..
> 
>> +sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>> +PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
>> +target_ulong msr = 0;
>> +
>> +cpu_synchronize_state(CPU(ppc_env_get_cpu(env)));
> 
> Then back again awkwardly, although you still have the cpu variable.

ah.. I overlooked it.

> 
>> +
>> +/*
>> + * Properly set bits in MSR before we invoke the handler.
>> + * SRR0/1, DAR and DSISR are properly set by KVM
>> + */
>> +if (!(*pcc->interrupts_big_endian)(cpu)) {
>> +msr |= (1ULL << MSR_LE);
>> +}
>> +
>> +if (env->msr && (1ULL << MSR_SF)) {
>> +msr |= (1ULL << MSR_SF);
>> +}
>> +
>> +msr |= (1ULL << MSR_ME);
> 
> Based on earlier discussions, sounds like assert(msr & (1ULL <<
> MSR_ME)) would actually be correct here.

Based on
http://lists.nongnu.org/archive/html/qemu-ppc/2015-11/msg00306.html, I
always set MSR_ME and don't assert if not set. Or am I missing anything

[Qemu-devel] [PATCH 1/2] block: Remove prototype of bdrv_swap from header

[Fam Zheng]

The function has gone.

Signed-off-by: Fam Zheng 
---
 include/block/block.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/block/block.h b/include/block/block.h
index 3477328..028f2fb 100644
--- a/include/block/block.h
+++ b/include/block/block.h
@@ -197,7 +197,6 @@ int bdrv_create_file(const char *filename, QemuOpts *opts, 
Error **errp);
 BlockDriverState *bdrv_new_root(void);
 BlockDriverState *bdrv_new(void);
 void bdrv_make_anon(BlockDriverState *bs);
-void bdrv_swap(BlockDriverState *bs_new, BlockDriverState *bs_old);
 void bdrv_append(BlockDriverState *bs_new, BlockDriverState *bs_top);
 void bdrv_replace_in_backing_chain(BlockDriverState *old,
BlockDriverState *new);
-- 
2.4.3

[Qemu-devel] [PATCH v2] coverity: Model g_poll()

[Markus Armbruster]

In my testing, Coverity reported two more CHECKED_RETURN:

* qemu-char.c:1248: fixed in commit c1f2448: "qemu-char: retry g_poll
  on EINTR".

* migration/qemu-file-unix.c:75: harmless, cleaned up in commit
  4e39f57 "migration: Clean up use of g_poll() in
  socket_writev_buffer()

Signed-off-by: Markus Armbruster 
---
v2: Commit message body rewritten, because fixes have been committed
meanwhile.

 scripts/coverity-model.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/scripts/coverity-model.c b/scripts/coverity-model.c
index e1d5f45..ee5bf9d 100644
--- a/scripts/coverity-model.c
+++ b/scripts/coverity-model.c
@@ -342,6 +342,15 @@ char *g_strconcat(const char *s, ...)
 
 /* Other glib functions */
 
+typedef struct pollfd GPollFD;
+
+int poll();
+
+int g_poll (GPollFD *fds, unsigned nfds, int timeout)
+{
+return poll(fds, nfds, timeout);
+}
+
 typedef struct _GIOChannel GIOChannel;
 GIOChannel *g_io_channel_unix_new(int fd)
 {
-- 
2.4.3

Re: [Qemu-devel] [TCG only][Migration Bug? ] Occasionally, the content of VM's memory is inconsistent between Source and Destination of migration

[Amit Shah]

On (Fri) 04 Dec 2015 [01:43:07], Li, Liang Z wrote:
> > 
> > Thanks for describing how to reproduce the bug.
> > If some pages are not transferred to destination then it is a bug, so we 
> > need
> > to know what the problem is, notice that the problem can be that TCG is not
> > marking dirty some page, that Migration code "forgets" about that page, or
> > anything eles altogether, that is what we need to find.
> > 
> > There are more posibilities, I am not sure that memtest is on 32bit mode, 
> > and
> > it is inside posibility that we are missing some state when we are on real
> > mode.
> > 
> > Will try to take a look at this.
> > 
> > THanks, again.
> > 
> 
> Hi Juan & Amit
> 
>  Do you think we should add a mechanism to check the data integrity during LM 
> like Zhijian's patch did?  it may be very helpful for developers. 
>  Actually, I did the similar thing before in order to make sure that I did 
> the right thing we I change the code related to LM.

If you mean for debugging, something that's not always on, then I'm
fine with it.

A script that goes along that shows the result of comparison of the
diff will be helpful too, something that shows how many pages are
differnt, how many bytes in a page on average, and so on.

Amit

[Qemu-devel] [PATCH 0/2] Clean up the remainders of bdrv_swap

[Fam Zheng]

Fam Zheng (2):
  block: Remove prototype of bdrv_swap from header
  iotests: Don't mention bdrv_swap in comments

 include/block/block.h  | 1 -
 tests/qemu-iotests/094 | 4 +---
 2 files changed, 1 insertion(+), 4 deletions(-)

-- 
2.4.3

[Qemu-devel] [PATCH 2/2] iotests: Don't mention bdrv_swap in comments

[Fam Zheng]

Signed-off-by: Fam Zheng 
---
 tests/qemu-iotests/094 | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/tests/qemu-iotests/094 b/tests/qemu-iotests/094
index 27a2be2..d30c78d 100755
--- a/tests/qemu-iotests/094
+++ b/tests/qemu-iotests/094
@@ -1,6 +1,6 @@
 #!/bin/bash
 #
-# Test case for drive-mirror to NBD (especially bdrv_swap() on NBD BDS)
+# Test case for drive-mirror to NBD
 #
 # Copyright (C) 2015 Red Hat, Inc.
 #
@@ -50,8 +50,6 @@ _send_qemu_cmd $QEMU_HANDLE \
 "{'execute': 'qmp_capabilities'}" \
 'return'
 
-# 'format': 'nbd' is not actually "correct", but this is probably the only way
-# to test bdrv_swap() on an NBD BDS
 _send_qemu_cmd $QEMU_HANDLE  \
 "{'execute': 'drive-mirror',
   'arguments': {'device': 'src',
-- 
2.4.3

Re: [Qemu-devel] [PATCH v2 4/4] spapr: Introduce FWNMI KVM capability

[Aravinda Prasad]

On Thursday 17 December 2015 09:32 AM, David Gibson wrote:
> On Wed, Dec 16, 2015 at 11:38:47AM +0530, Aravinda Prasad wrote:
>> Introduce a new KVM capability to control how KVM
>> behaves on machine check exception.
>>
>> Without this capability, KVM redirects machine check
>> exceptions to guest's 0x200 vector if the address in
>> error belongs to guest. With this capability KVM
>> causes a guest exit with NMI exit reason.
>>
>> This is required to avoid problem if a new kernel/KVM
>> is used with an old QEMU. As old QEMU might not
>> understand the new NMI exit type and treat it as a
>> fatal error, even though the guest could have actually
>> handled the error if the exception was delivered to
>> guest's 0x200 interrupt vector.
>>
>> PS: KVM_CAP_PPC_FWNMI is set to 121 as 119 and 120 are
>> used by KVM_CAP_GUEST_DEBUG_HW_BPS and KVM_CAP_GUEST_DEBUG_HW_WPS
>> in KVM code, but still not reflected in QEMU code.
> 
> The commit message seems to be written as if this were the kernel
> patch adding the capability there, rather than the qemu patch using
> it.
> 

I will reword it.

Regards,
Aravinda

>> Signed-off-by: Aravinda Prasad 
>> ---
>>  hw/ppc/spapr_rtas.c   |   17 +
>>  linux-headers/linux/kvm.h |1 +
>>  target-ppc/kvm.c  |2 ++
>>  3 files changed, 20 insertions(+)
>>
>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>> index 17c4672..53319da 100644
>> --- a/hw/ppc/spapr_rtas.c
>> +++ b/hw/ppc/spapr_rtas.c
>> @@ -38,6 +38,8 @@
>>  #include 
>>  #include "hw/ppc/spapr_drc.h"
>>  
>> +extern int cap_fwnmi;
>> +
>>  /* #define DEBUG_SPAPR */
>>  
>>  #ifdef DEBUG_SPAPR
>> @@ -603,9 +605,24 @@ static void rtas_ibm_nmi_register(PowerPCCPU *cpu,
>>target_ulong args,
>>uint32_t nret, target_ulong rets)
>>  {
>> +int ret;
>> +CPUState *cs = CPU(cpu);
>> +
>> +if (!cap_fwnmi) {
>> +rtas_st(rets, 0, RTAS_OUT_NOT_SUPPORTED);
>> +return;
>> +}
>> +
>>  spapr->mc_in_progress = false;
>>  qemu_cond_init(>mc_delivery_cond);
>>  spapr->guest_machine_check_addr = rtas_ld(args, 1);
>> +
>> +ret = kvm_vcpu_enable_cap(cs, KVM_CAP_PPC_FWNMI, 0);
>> +if (ret < 0) {
>> +rtas_st(rets, 0, RTAS_OUT_HW_ERROR);
>> +return;
>> +}
>> +
>>  rtas_st(rets, 0, RTAS_OUT_SUCCESS);
>>  }
>>  
>> diff --git a/linux-headers/linux/kvm.h b/linux-headers/linux/kvm.h
>> index 683f713..2db1fba 100644
>> --- a/linux-headers/linux/kvm.h
>> +++ b/linux-headers/linux/kvm.h
>> @@ -819,6 +819,7 @@ struct kvm_ppc_smmu_info {
>>  #define KVM_CAP_DISABLE_QUIRKS 116
>>  #define KVM_CAP_X86_SMM 117
>>  #define KVM_CAP_MULTI_ADDRESS_SPACE 118
>> +#define KVM_CAP_PPC_FWNMI 121
>>  
>>  #ifdef KVM_CAP_IRQ_ROUTING
>>  
>> diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
>> index 2bbb46d..5339c04 100644
>> --- a/target-ppc/kvm.c
>> +++ b/target-ppc/kvm.c
>> @@ -74,6 +74,7 @@ static int cap_ppc_watchdog;
>>  static int cap_papr;
>>  static int cap_htab_fd;
>>  static int cap_fixup_hcalls;
>> +int cap_fwnmi;
>>  
>>  static uint32_t debug_inst_opcode;
>>  
>> @@ -116,6 +117,7 @@ int kvm_arch_init(MachineState *ms, KVMState *s)
>>   * only activated after this by kvmppc_set_papr() */
>>  cap_htab_fd = kvm_check_extension(s, KVM_CAP_PPC_HTAB_FD);
>>  cap_fixup_hcalls = kvm_check_extension(s, KVM_CAP_PPC_FIXUP_HCALL);
>> +cap_fwnmi = kvm_check_extension(s, KVM_CAP_PPC_FWNMI);
>>  
>>  if (!cap_interrupt_level) {
>>  fprintf(stderr, "KVM: Couldn't find level irq capability. Expect 
>> the "
>>
> 

-- 
Regards,
Aravinda