Re: [Qemu-devel] [PATCH v4 04/13] virtio: poll virtqueues for new buffers

[Stefan Hajnoczi]

On Mon, Dec 05, 2016 at 05:59:55AM -0500, Paolo Bonzini wrote:
> 
> 
> - Original Message -
> > From: "Stefan Hajnoczi" 
> > To: "Fam Zheng" 
> > Cc: "Stefan Hajnoczi" , borntrae...@de.ibm.com, "Karl 
> > Rister" ,
> > qemu-devel@nongnu.org, "Paolo Bonzini" 
> > Sent: Monday, December 5, 2016 11:14:52 AM
> > Subject: Re: [Qemu-devel] [PATCH v4 04/13] virtio: poll virtqueues for new 
> > buffers
> > 
> > On Mon, Dec 05, 2016 at 08:54:59AM +0800, Fam Zheng wrote:
> > > On Thu, 12/01 19:26, Stefan Hajnoczi wrote:
> > > > Add an AioContext poll handler to detect new virtqueue buffers without
> > > > waiting for a guest->host notification.
> > > > 
> > > > Signed-off-by: Stefan Hajnoczi 
> > > > ---
> > > >  hw/virtio/virtio.c | 16 +++-
> > > >  1 file changed, 15 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> > > > index 6f8ca25..3870411 100644
> > > > --- a/hw/virtio/virtio.c
> > > > +++ b/hw/virtio/virtio.c
> > > > @@ -2047,13 +2047,27 @@ static void
> > > > virtio_queue_host_notifier_aio_read(EventNotifier *n)
> > > >  }
> > > >  }
> > > >  
> > > > +static bool virtio_queue_host_notifier_aio_poll(void *opaque)
> > > > +{
> > > > +EventNotifier *n = opaque;
> > > > +VirtQueue *vq = container_of(n, VirtQueue, host_notifier);
> > > > +
> > > > +if (virtio_queue_empty(vq)) {
> > > 
> > > I notice this call path gets very hot in the profile:
> > > 
> > > #0  address_space_translate_internal (d=0x7f853c22e970, addr=2140788738,
> > > xlat=xlat@entry=0x7f8549db59e8, plen=plen@entry=0x7f8549db5a68,
> > > resolve_subpage=resolve_subpage@entry=true) at
> > > /root/qemu-nvme/exec.c:419
> > > #1  0x7f854d3e8449 in address_space_translate (as=,
> > > addr=2140788738,
> > > xlat=xlat@entry=0x7f8549db5a70, plen=plen@entry=0x7f8549db5a68,
> > > is_write=is_write@entry=false) at /root/qemu-nvme/exec.c:462
> > > #2  0x7f854d3ee2fd in address_space_lduw_internal
> > > (endian=DEVICE_LITTLE_ENDIAN,
> > > result=0x0, attrs=..., addr=, as=)
> > > at /root/qemu-nvme/exec.c:3299
> > > #3  address_space_lduw_le (result=0x0, attrs=..., addr=,
> > > as=)
> > > at /root/qemu-nvme/exec.c:3351
> > > #4  lduw_le_phys (as=, addr=) at
> > > /root/qemu-nvme/exec.c:3369
> > > #5  0x7f854d475978 in virtio_lduw_phys (vdev=,
> > > pa=)
> > > at /root/qemu-nvme/include/hw/virtio/virtio-access.h:46
> > > #6  vring_avail_idx (vq=, vq=)
> > > at /root/qemu-nvme/hw/virtio/virtio.c:143
> > > #7  virtio_queue_empty (vq=vq@entry=0x7f854fd0c9e0) at
> > > /root/qemu-nvme/hw/virtio/virtio.c:246
> > > #8  0x7f854d475d20 in virtio_queue_empty (vq=0x7f854fd0c9e0)
> > > at /root/qemu-nvme/hw/virtio/virtio.c:2074
> > > #9  virtio_queue_host_notifier_aio_poll (opaque=0x7f854fd0ca48)
> > > at /root/qemu-nvme/hw/virtio/virtio.c:2068
> > > #10 0x7f854d69116e in run_poll_handlers_once (ctx=) at
> > > aio-posix.c:493
> > > #11 0x7f854d691b08 in run_poll_handlers (max_ns=,
> > > ctx=0x7f854e908850)
> > > at aio-posix.c:530
> > > #12 try_poll_mode (blocking=true, ctx=0x7f854e908850) at aio-posix.c:558
> > > #13 aio_poll (ctx=0x7f854e908850, blocking=blocking@entry=true) at
> > > aio-posix.c:601
> > > #14 0x7f854d4ff12e in iothread_run (opaque=0x7f854e9082f0) at
> > > iothread.c:53
> > > #15 0x7f854c249dc5 in start_thread () from /lib64/libpthread.so.0
> > > #16 0x7f854acd973d in clone () from /lib64/libc.so.6
> > > 
> > > Is it too costly to do an address_space_translate per poll?
> > 
> > Good insight.
> > 
> > If we map guest RAM we should consider mapping the vring in
> > hw/virtio/virtio.c.  That way not just polling benefits.
> > 
> > The map/unmap API was not designed for repeated memory accesses.  In the
> > bounce buffer case you have a stale copy - repeated reads will not
> > update it.  Vrings should be in guest RAM so we don't really need to
> > support bounce buffering but it's still a hack to use the map/unmap API.
> > 
> > Paolo: Thoughts about the memory API?
> 
> Yeah, I had discussed this recently with Fam, and prototyped (read: compiled
> the code) an API like this:
> 
> int64_t address_space_cache_init(MemoryRegionCache *cache,
>  AddressSpace *as,
>  hwaddr addr,
>  hwaddr len,
>  bool is_write);
> /* Only for is_write == true.  */
> void address_space_cache_invalidate(MemoryRegionCache *cache,
> hwaddr addr,
> hwaddr access_len);
> uint32_t address_space_lduw_cached(MemoryRegionCache *cache, hwaddr addr,
>MemTxAttrs attrs, MemTxResult *result);
> ...
> 
> It's basically the same API as usual, but with 

Re: [Qemu-devel] [PATCH v4 00/13] aio: experimental virtio-blk polling mode

[Stefan Hajnoczi]

On Mon, Dec 05, 2016 at 06:25:49PM +0800, Fam Zheng wrote:
> On Thu, 12/01 19:26, Stefan Hajnoczi wrote:
> > One way to avoid the costly exit is to use polling instead of notification.
> 
> Testing with fio on virtio-blk backed by NVMe device, I can see significant
> performance improvement with this series:
> 
> poll-max-nsiodepth=1iodepth=32
> --
> 0  24806.94 151430.36
> 1  24435.02 155285.59
> 10024702.41 149937.2
> 1000   24457.08 152936.3
> 1  24605.05 156158.12
> 13000  24412.95 154908.73
> 16000  30654.24 185731.58
> 18000  36365.69 185538.78
> 2  35640.48 188640.61
> 3  37411.05 184198.39
> 6  35230.66 190427.42
> 
> So this definitely helps synchronous I/O with queue depth = 1. Great!

Nice.  Even with iodepth=32 the improvement is significant.

> I have a small concern with high queue depth, though. The more frequent we 
> check
> the virtqueue, the less likely the requests can be batched, and the more
> submissions (both from guest to QEMU and from QEMU to host) are needed to
> achieve the same bandwidth, because we'd do less plug/unplug. This could be a
> problem under heavy workload.  We may want to consider the driver's transient
> state when it is appending requests to the virtqueue. For example, virtio-blk
> driver in Linux updates avail idx after adding each request. If QEMU looks at
> the index in the middle, it will miss the opportunities to plug/unplug and 
> merge
> requests. On the other hand, though virtio-blk driver doesn't have IO 
> scheduler,
> it does have some batch submission semantics passed down by blk-mq (see the
> "notify" condition in drivers/block/virtio_blk.c:virtio_queue_rq()).  So I'm
> wondering whether it makes sense to wait for the whole batch of requests to be
> added to the virtqueue before processing it? This can be done by changing the
> driver to only update "avail" index after all requests are added to the queue,
> or even adding a flag in the virtio ring descriptor to suppress busy polling.

Only benchmarking can tell.  It would be interesting to extract the
vq->vring.avail->idx update from virtqueue_add().  I don't think a new
flag is necessary.

> > The main drawback of polling is that it consumes CPU resources.  In order to
> > benefit performance the host must have extra CPU cycles available on 
> > physical
> > CPUs that aren't used by the guest.
> > 
> > This is an experimental AioContext polling implementation.  It adds a 
> > polling
> > callback into the event loop.  Polling functions are implemented for 
> > virtio-blk
> > virtqueue guest->host kick and Linux AIO completion.
> > 
> > The -object iothread,poll-max-ns=NUM parameter sets the number of 
> > nanoseconds
> > to poll before entering the usual blocking poll(2) syscall.  Try setting 
> > this
> > parameter to the time from old request completion to new virtqueue kick.  By
> > default no polling is done so you must set this parameter to get busy 
> > polling.
> 
> Given the self tuning, should we document the best practice in setting the
> value?  Is it okay for user or even QEMU to use a relatively large value by
> default and expect it to tune itself sensibly?

Karl: do you have time to run a bigger suite of benchmarks to identify a
reasonable default poll-max-ns value?  Both aio=native and aio=threads
are important.

If there is a sweet spot that improves performance without pathological
cases then we could even enable polling by default in QEMU.

Otherwise we'd just document the recommended best polling duration as a
starting point for users.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH for-2.9 22/30] aspeed/smc: handle dummy bytes when doing fast reads

[Cédric Le Goater]

On 12/04/2016 05:46 PM, mar.krzeminski wrote:
> 
> 
> W dniu 29.11.2016 o 16:44, Cédric Le Goater pisze:
>> When doing fast read, a certain amount of dummy bytes should be sent
>> before the read. This number is configurable in the controler CE0
>> Control Register and needs to be modeled using fake transfers the
>> flash module.
>>
>> When the controller is configured for Command mode, the SPI command
>> used to do the read is stored in the CE0 control register but, in User
>> mode, we need to snoop into the flow of bytes to catch the command. It
>> should be the first byte after CS select.
>>
>> Signed-off-by: Cédric Le Goater 
>> Reviewed-by: Andrew Jeffery 
>> ---
>>  hw/ssi/aspeed_smc.c | 58 
>> ++---
>>  include/hw/ssi/aspeed_smc.h |  1 +
>>  2 files changed, 51 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
>> index 9596ea94a3bc..733fd8b09c06 100644
>> --- a/hw/ssi/aspeed_smc.c
>> +++ b/hw/ssi/aspeed_smc.c
>> @@ -71,7 +71,9 @@
>>  #define R_CTRL0   (0x10 / 4)
>>  #define   CTRL_CMD_SHIFT   16
>>  #define   CTRL_CMD_MASK0xff
>> +#define   CTRL_DUMMY_HIGH_SHIFT14
>>  #define   CTRL_AST2400_SPI_4BYTE   (1 << 13)
>> +#define   CTRL_DUMMY_LOW_SHIFT 6 /* 2 bits [7:6] */
>>  #define   CTRL_CE_STOP_ACTIVE  (1 << 2)
>>  #define   CTRL_CMD_MODE_MASK   0x3
>>  #define CTRL_READMODE  0x0
>> @@ -151,6 +153,7 @@
>>  #define SPI_OP_WRDI   0x04/* Write disable */
>>  #define SPI_OP_RDSR   0x05/* Read status register */
>>  #define SPI_OP_WREN   0x06/* Write enable */
>> +#define SPI_OP_READ_FAST  0x0b/* Read data bytes (high frequency) */
>>  
>>  /* Used for Macronix and Winbond flashes. */
>>  #define SPI_OP_EN4B   0xb7/* Enter 4-byte mode */
>> @@ -510,6 +513,12 @@ static void aspeed_smc_flash_setup_read(AspeedSMCFlash 
>> *fl, uint32_t addr)
>>  /* access can not exceed CS segment */
>>  addr = aspeed_smc_check_segment_addr(fl, addr);
>>  
>> +/*
>> + * Remember command as we might need to send dummy bytes before
>> + * reading data
>> + */
>> +fl->cmd = cmd;
>> +
>>  /* TODO: do we have to send 4BYTE each time ? */
>>  if (aspeed_smc_flash_is_4byte(fl)) {
>>  ssi_transfer(s->spi, SPI_OP_EN4B);
>> @@ -525,27 +534,50 @@ static void aspeed_smc_flash_setup_read(AspeedSMCFlash 
>> *fl, uint32_t addr)
>>  ssi_transfer(s->spi, (addr & 0xff));
>>  }
>>  
>> +static int aspeed_smc_flash_dummies(const AspeedSMCFlash *fl)
>> +{
>> +AspeedSMCState *s = fl->controller;
>> +uint32_t r_ctrl0 = s->regs[s->r_ctrl0 + fl->id];
>> +uint32_t dummy_high = (r_ctrl0 >> CTRL_DUMMY_HIGH_SHIFT) & 0x1;
>> +uint32_t dummy_low = (r_ctrl0 >> CTRL_DUMMY_LOW_SHIFT) & 0x3;
>> +
>> +return ((dummy_high << 2) | dummy_low) * 8;
>> +}
>> +
>> +static uint64_t aspeed_smc_flash_do_read(AspeedSMCFlash *fl, unsigned size)
>> +{
>> +AspeedSMCState *s = fl->controller;
>> +uint64_t ret = 0;
>> +int i;
>> +
>> +if (fl->cmd == SPI_OP_READ_FAST) {
>> +for (i = 0; i < aspeed_smc_flash_dummies(fl); i++) {
>> +ssi_transfer(s->spi, 0x0);
>> +}
>> +}
> Generally this is wrong, controller should be not aware of any command in 
> user mode.
> Currently you are forced to do this by m25p80c dummy cycles implementation.
> I had no time to improve this since it need to update in Xilinx models, but 
> maybe it is good place to talk about the solution.
> I was thinking to add to ssi function transferN, and use it in flash models.
> Firs introduce new state for dummy cycles in m25p80 and then:

This new state would depend on the command op ? fastread would put
the object in a dummy_cycle state ? 

> a) in case caller use ssi_transfer(transfer8 in flsh models) dummy count will 
> be incremented by 8.
> This should solve the problem when flash is connected to controller that is 
> not aware about dummy cycles,
> like this mode or clear SPI controller.
> b) when caller use ssi_trasferN length (in bits) will be the number of dummy 
> cycles.
> 
> What is your opinion?

when you have some time, please send a quick rfc patch for the API :) 
so that I can experiment on the aspeed controller.

Thanks,

C. 

>> +fl->cmd = 0;
>> +
>> +for (i = 0; i < size; i++) {
>> +ret |= ssi_transfer(s->spi, 0x0) << (8 * i);
>> +}
>> +return ret;
>> +}
>> +
>>  static uint64_t aspeed_smc_flash_read(void *opaque, hwaddr addr, unsigned 
>> size)
>>  {
>>  AspeedSMCFlash *fl = opaque;
>> -const AspeedSMCState *s = fl->controller;
>>  uint64_t ret = 0;
>> -int i;
>>  
>>  switch (aspeed_smc_flash_mode(fl)) {
>>  case CTRL_USERMODE:
>> -for (i = 0; i < size; i++) {
>> -ret |= ssi_transfer(s->spi, 0x0) << (8 * i);
>> -}
>> +ret = aspeed_smc_flash_do_read(fl, size);
>>  break;
>> 

Re: [Qemu-devel] New wiki page: http://wiki.qemu.org/Hosts

[G 3]

On Dec 5, 2016, at 9:15 AM, Paolo Bonzini wrote:




On 05/12/2016 14:47, G 3 wrote:


So now I have to figure out which commands to use. This script would
have to work on all operating systems that QEMU is built on. This  
page

has some pretty good information on how to do this:
http://stackoverflow.com/questions/12422289/bash-command-to-covert- 
html-page-to-a-text-file


No, I just want to do that once and get rid of the Hosts/ pages.

Paolo


Really? Why? I thought they were very useful. Where would people who  
are doing a web search go for building information? 

Re: [Qemu-devel] New wiki page: http://wiki.qemu.org/Hosts

[G 3]

On Dec 3, 2016, at 3:41 AM, Paolo Bonzini wrote:




On 02/12/2016 23:59, Programmingkid wrote:
I thought we could use a Hosts page to sort all the host  
documentation we have. It is located here: http://wiki.qemu.org/Hosts


Here is what I have so far:

AIX
Darwin
FreeBSD, NetBSD, OpenBSD
Linux
Mac OS X
Solaris
Windows



Since this stuff changes rarely, it should be in the source tree
instead.  You can link to the source tree from the wiki with something
like this:

{{src|path=BUILDING|description=the BUILDING file in the source tree}}

Would you like to make a patch moving the information from the wiki to
such a file?

Paolo


I think I now know what you want. You want a shell script that would  
take each link in the http://wiki.qemu.org/Hosts file, convert the  
html data into text of the linked file, then append that text to a  
file called BUILDING. So in the end we have a file called BUILDING  
that would look like this:



QEMU Building Directions:

AIX
.

Darwin
.

FreeBSD, NetBSD, OpenBSD


Linux
.

Mac OS X


Solaris


Windows



So now I have to figure out which commands to use. This script would  
have to work on all operating systems that QEMU is built on. This  
page has some pretty good information on how to do this: http:// 
stackoverflow.com/questions/12422289/bash-command-to-covert-html-page- 
to-a-text-file


I think I will work on a python script to do this job.

Re: [Qemu-devel] [PATCH for-2.9 17/30] aspeed/smc: handle SPI flash Command mode

[Cédric Le Goater]

On 12/04/2016 05:31 PM, mar.krzeminski wrote:
> Hi Cedric,
> 
> Since there is no public datasheet user guide for SMC I would ask some 
> question
> regarding HW itself because I got impression that you are implementing in this
> model a part functionality that is done by Bootrom.
> 
> W dniu 29.11.2016 o 16:43, Cédric Le Goater pisze:
>> The Aspeed SMC controllers have a mode (Command mode) in which
>> accesses to the flash content are no different than doing MMIOs. The
>> controller generates all the necessary commands to load (or store)
>> data in memory.
>>
>> However, accesses are restricted to the segment window assigned the
>> the flash module by the controller. This window is defined by the
>> Segment Address Register.
>>
>> Signed-off-by: Cédric Le Goater 
>> Reviewed-by: Andrew Jeffery 
>> ---
>>  hw/ssi/aspeed_smc.c | 174 
>> 
>>  1 file changed, 162 insertions(+), 12 deletions(-)
>>
>> diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
>> index 72a44150b0a1..eec087199a22 100644
>> --- a/hw/ssi/aspeed_smc.c
>> +++ b/hw/ssi/aspeed_smc.c
>> @@ -69,6 +69,7 @@
>>  #define R_CTRL0   (0x10 / 4)
>>  #define   CTRL_CMD_SHIFT   16
>>  #define   CTRL_CMD_MASK0xff
>> +#define   CTRL_AST2400_SPI_4BYTE   (1 << 13)
>>  #define   CTRL_CE_STOP_ACTIVE  (1 << 2)
>>  #define   CTRL_CMD_MODE_MASK   0x3
>>  #define CTRL_READMODE  0x0
>> @@ -135,6 +136,16 @@
>>  #define ASPEED_SOC_SPI_FLASH_BASE   0x3000
>>  #define ASPEED_SOC_SPI2_FLASH_BASE  0x3800
>>  
>> +/* Flash opcodes. */
>> +#define SPI_OP_READ   0x03/* Read data bytes (low frequency) */
>> +#define SPI_OP_WRDI   0x04/* Write disable */
>> +#define SPI_OP_RDSR   0x05/* Read status register */
>> +#define SPI_OP_WREN   0x06/* Write enable */
> Are you sure that the controller is aware af all above commands (especially 
> WD/WE and RDS)?

HW is aware of SPI_OP_READ which is the default command for the 
"normal" read mode. For other modes, fast and write, the command 
op is configured in the CEx Control Register. 

These ops are used in the model :

 * SPI_OP_READ_FAST, for dummies
 * SPI_OP_EN4B, might be useless if we expect software to send
   this command before using this mode.
 * SPI_OP_WREN, same comment.

The rest I should remove as it is unused.

>> +
>> +/* Used for Macronix and Winbond flashes. */
>> +#define SPI_OP_EN4B   0xb7/* Enter 4-byte mode */
>> +#define SPI_OP_EX4B   0xe9/* Exit 4-byte mode */
>> +
> Same as above but I think 4byte address mode bit changes onlu nymber of bytes
> that is sent after instruction.
>
>>  /*
>>   * Default segments mapping addresses and size for each slave per
>>   * controller. These can be changed when board is initialized with the
>> @@ -357,6 +368,98 @@ static inline bool aspeed_smc_is_writable(const 
>> AspeedSMCFlash *fl)
>>  return s->regs[s->r_conf] & (1 << (s->conf_enable_w0 + fl->id));
>>  }
>>  
>> +static inline int aspeed_smc_flash_cmd(const AspeedSMCFlash *fl)
>> +{
>> +AspeedSMCState *s = fl->controller;
>> +int cmd = (s->regs[s->r_ctrl0 + fl->id] >> CTRL_CMD_SHIFT) & 
>> CTRL_CMD_MASK;
>> +
>> +/* This is the default value for read mode. In other modes, the
>> + * command should be defined */
>> +if (aspeed_smc_flash_mode(fl) == CTRL_READMODE) {
>> +cmd = SPI_OP_READ;
>> +}
>> +
>> +if (!cmd) {
> cmd == 0 => NOP command for some flash (eg. mx66l1g45g).

So I should use another default value, OxFF ? 

>> +qemu_log_mask(LOG_GUEST_ERROR, "%s: no command defined for mode 
>> %d\n",
>> +  __func__, aspeed_smc_flash_mode(fl));
>> +}
>> +
>> +return cmd;
>> +}
>> +
>> +static inline int aspeed_smc_flash_is_4byte(const AspeedSMCFlash *fl)
>> +{
>> +AspeedSMCState *s = fl->controller;
>> +
>> +if (s->ctrl->segments == aspeed_segments_spi) {
>> +return s->regs[s->r_ctrl0] & CTRL_AST2400_SPI_4BYTE;
>> +} else {
>> +return s->regs[s->r_ce_ctrl] & (1 << (CTRL_EXTENDED0 + fl->id));
>> +}
>> +}
>> +
>> +static void aspeed_smc_flash_select(const AspeedSMCFlash *fl)
>> +{
>> +AspeedSMCState *s = fl->controller;
>> +
>> +s->regs[s->r_ctrl0 + fl->id] &= ~CTRL_CE_STOP_ACTIVE;
>> +qemu_set_irq(s->cs_lines[fl->id], aspeed_smc_is_ce_stop_active(fl));
>> +}
>> +
>> +static void aspeed_smc_flash_unselect(const AspeedSMCFlash *fl)
>> +{
>> +AspeedSMCState *s = fl->controller;
>> +
>> +s->regs[s->r_ctrl0 + fl->id] |= CTRL_CE_STOP_ACTIVE;
>> +qemu_set_irq(s->cs_lines[fl->id], aspeed_smc_is_ce_stop_active(fl));
>> +}
>> +
>> +static uint32_t aspeed_smc_check_segment_addr(AspeedSMCFlash *fl, uint32_t 
>> addr)
>> +{
>> +AspeedSMCState *s = fl->controller;
>> +AspeedSegments seg;
>> +
>> +aspeed_smc_reg_to_segment(s->regs[R_SEG_ADDR0 + fl->id], );
>> +if ((addr & (seg.size - 1)) != addr) {
>> + 

Re: [Qemu-devel] New wiki page: http://wiki.qemu.org/Hosts

[Paolo Bonzini]

On 05/12/2016 14:47, G 3 wrote:
> 
> So now I have to figure out which commands to use. This script would
> have to work on all operating systems that QEMU is built on. This page
> has some pretty good information on how to do this:
> http://stackoverflow.com/questions/12422289/bash-command-to-covert-html-page-to-a-text-file

No, I just want to do that once and get rid of the Hosts/ pages.

Paolo

Re: [Qemu-devel] [PULL 0/3] Net patches

[Stefan Hajnoczi]

On Mon, Dec 05, 2016 at 05:52:00PM +0800, Jason Wang wrote:
> The following changes since commit bd8ef5060dd2124a54578241da9a572faf7658dd:
> 
>   Merge remote-tracking branch 'dgibson/tags/ppc-for-2.8-20161201' into 
> staging (2016-12-01 13:39:29 +)
> 
> are available in the git repository at:
> 
>   https://github.com/jasowang/qemu.git tags/net-pull-request
> 
> for you to fetch changes up to 18766d28848f2a4c309e78c6706b872f2cb32786:
> 
>   fsl_etsec: Fix various small problems in hexdump code (2016-12-05 17:45:14 
> +0800)
> 
> 

Please resend with the coding style violation fixed.  See the patchew
email for details.


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH for-2.9 24/30] aspeed: use first SPI flash as a boot ROM

[Cédric Le Goater]

On 12/05/2016 10:57 AM, Marcin Krzemiński wrote:
> 
> 2016-12-05 10:36 GMT+01:00 Cédric Le Goater  >:
> 
> Hello Marcin,
> 
> On 12/04/2016 06:00 PM, mar.krzeminski wrote:
> > Hi Cedric,
> >
> > it looks like good idea for now to handle boot from flash.
> > As I understand you are trying to omit bootrom code in Qemu model?
> 
> I suppose you mean handling a romd memory region under the m25p80
> object ?
> 
> It could be that I mess up everything because my understanding how the real HW
> work and boot is wrong. Please correct my assumptions:
> 1. You are setting boot source to spi-nor (jumper, resistor whatever)
> 2. There is a small bootrom in SoC (not u-boot) that set eg. reset vector, 
> configure SMC
> and start execude code from flash.
> 3. Memory mapped flash content is at address 0x1c00.

No. The memory flash content CS0 is mapped at 0x0 and starts 
with U-Boot directly, there is no preliminary loader.

U-Boot is not merged in mainline yet. We are "slowly" building a
tree for upstream : 
 
https://github.com/openbmc/u-boot/
https://github.com/legoater/u-boot/


> > This could lead you to some hacks in device models (eg SMC).
> 
> I haven't had to, yet.
> 
> > W dniu 29.11.2016 o 16:44, Cédric Le Goater pisze:
> >> Fill a ROM region with the flash content to support U-Boot. This is a
> >> little hacky but until we can boot from a MMIO region, it seems
> >> difficult to do anything else.
> >>
> >> Signed-off-by: Cédric Le Goater >
> >> Reviewed-by: Joel Stanley >
> >> Reviewed-by: Andrew Jeffery >
> >> ---
> >>  hw/arm/aspeed.c | 41 +
> >>  1 file changed, 41 insertions(+)
> >>
> >> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
> >> index 40c13838fb2d..a92c2f1c362b 100644
> >> --- a/hw/arm/aspeed.c
> >> +++ b/hw/arm/aspeed.c
> >> @@ -20,6 +20,8 @@
> >>  #include "qemu/log.h"
> >>  #include "sysemu/block-backend.h"
> >>  #include "sysemu/blockdev.h"
> >> +#include "hw/loader.h"
> >> +#include "qemu/error-report.h"
> >>
> >>  static struct arm_boot_info aspeed_board_binfo = {
> >>  .board_id = -1, /* device-tree-only board */
> >> @@ -104,6 +106,28 @@ static const AspeedBoardConfig aspeed_boards[] = {
> >>  },
> >>  };
> >>
> >> +#define FIRMWARE_ADDR 0x0
> >> +
> >> +static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, size_t 
> rom_size,
> >> +   Error **errp)
> >> +{
> >> +BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
> >> +uint8_t *storage;
> >> +
> >> +if (rom_size > blk_getlength(blk)) {
> >> +rom_size = blk_getlength(blk);
> >> +}
> > I was not able to attach smaller file as m25p80 storage.
> 
> yes that's most probably because m25p80_realize() does :
> 
>if (blk_pread(s->blk, 0, s->storage, s->size) != s->size) {
> error_setg(errp, "failed to read the initial flash content");
> return;
> }
> 
> my bad. May be, we could relax a bit the test and allow smaller block
> backends ?
> 
>  
> I think not, if you have fs on flash (eg. UBI) and smaller image fs will not 
> mount,
> or in worse case you can issue writes outside the file. 
> 
> 
> 
> >> +
> >> +storage = g_new0(uint8_t, rom_size);
> >> +if (blk_pread(blk, 0, storage, rom_size) < 0) {
> >> +error_setg(errp, "failed to read the initial flash content");
> >> +return;
> >> +}
> >> +
> >> +rom_add_blob_fixed("aspeed.boot_rom", storage, rom_size, addr);
> >> +g_free(storage);
> >> +}
> >> +
> >>  static void aspeed_board_init_flashes(AspeedSMCState *s, const char 
> *flashtype,
> >>Error **errp)
> >>  {
> >> @@ -135,6 +159,7 @@ static void aspeed_board_init(MachineState 
> *machine,
> >>  {
> >>  AspeedBoardState *bmc;
> >>  AspeedSoCClass *sc;
> >> +DriveInfo *drive0 = drive_get(IF_MTD, 0, 0);
> >>
> >>  bmc = g_new0(AspeedBoardState, 1);
> >>  object_initialize(>soc, (sizeof(bmc->soc)), cfg->soc_name);
> >> @@ -168,6 +193,22 @@ static void aspeed_board_init(MachineState 
> *machine,
> >>  aspeed_board_init_flashes(>soc.fmc, cfg->fmc_model, 
> _abort);
> >>  aspeed_board_init_flashes(>soc.spi[0], cfg->spi_model, 
> _abort);
> >>
> >> +/* Install first FMC flash content as a boot rom. */
> >> +if (drive0) {
> >> +AspeedSMCFlash *fl = >soc.fmc.flashes[0];
> >> +MemoryRegion *boot_rom = g_new(MemoryRegion, 1);
> >> +
> >> +/*
> >> + * 

Re: [Qemu-devel] [PATCH v4 01/64] tcg: Add field extraction primitives

[Richard Henderson]

On 12/05/2016 05:17 AM, Alex Bennée wrote:
>> +/* ??? Ideally we'd know what values are available for immediate AND.
>> +   Assume that 8 bits are available, plus the special case of 16,
>> +   so that we get ext8u, ext16u.  */
>> +switch (len) {
>> +case 1 ... 8: case 16:
>> +tcg_gen_shri_i32(ret, arg, ofs);
>> +tcg_gen_andi_i32(ret, ret, (1u << len) - 1);
>> +break;
>> +default:
>> +tcg_gen_shli_i32(ret, arg, 32 - len - ofs);
>> +tcg_gen_shri_i32(ret, ret, 32 - len);
>> +break;
>> +}
> 
> Hmm is this starting to make a case for backend specific optimisation
> passes which have a better idea of the code that can be generated or
> exposing a TCG_TARGET_HAS_8IMM_BITS or some such from the backend to the
> generators?

Thanks for the prod.  In theory the information is already available.

  tcg_target_const_match((1u << len) - 1, TCG_TYPE_I32,
 _op_defs[INDEX_op_and_i32].args_ct[2]);

That's currently static in tcg.c, but that could be fixed.

There could well be a call for backend-specific passes.  I've been thinking of
the problems surrounding constant generation and reverse-endian stores for a
while now, which also sort of fall into this category.


r~

Re: [Qemu-devel] [qemu patch V3 2/2] kvmclock: reduce kvmclock difference on migration

[Eduardo Habkost]

On Fri, Dec 02, 2016 at 08:30:35PM -0200, Marcelo Tosatti wrote:
> On Fri, Dec 02, 2016 at 11:56:09AM -0200, Eduardo Habkost wrote:
> > On Thu, Dec 01, 2016 at 12:39:03PM -0200, Marcelo Tosatti wrote:
> > > On Wed, Nov 30, 2016 at 11:09:28AM -0200, Eduardo Habkost wrote:
> > > > On Tue, Nov 29, 2016 at 09:54:29PM -0200, Marcelo Tosatti wrote:
> > > > > On Tue, Nov 29, 2016 at 10:34:05AM -0200, Eduardo Habkost wrote:
> > > > > > On Tue, Nov 29, 2016 at 08:56:00AM -0200, Marcelo Tosatti wrote:
> > > > > > > On Mon, Nov 28, 2016 at 03:12:01PM -0200, Eduardo Habkost wrote:
> > > > > > > > On Mon, Nov 28, 2016 at 02:45:24PM -0200, Marcelo Tosatti wrote:
> > > > > > > > > On Mon, Nov 28, 2016 at 12:13:22PM -0200, Eduardo Habkost 
> > > > > > > > > wrote:
> > > > > > > > > > Sorry for not noticing the following issues on the previous
> > > > > > > > > > reviews. I was only paying attention to the vmstate and 
> > > > > > > > > > machine
> > > > > > > > > > code:
> > > > > > > > > > 
> > > > > > > > > > On Mon, Nov 21, 2016 at 08:50:04AM -0200, Marcelo Tosatti 
> > > > > > > > > > wrote:
> > > > > > > > > > > Check for KVM_CAP_ADJUST_CLOCK capability 
> > > > > > > > > > > KVM_CLOCK_TSC_STABLE, which
> > > > > > > > > > > indicates that KVM_GET_CLOCK returns a value as seen by 
> > > > > > > > > > > the guest at
> > > > > > > > > > > that moment.
> > > > > > > > > > > 
> > > > > > > > > > > For new machine types, use this value rather than reading 
> > > > > > > > > > > from guest memory.
> > > > > > > > > > > 
> > > > > > > > > > > This reduces kvmclock difference on migration from 5s to 
> > > > > > > > > > > 0.1s
> > > > > > > > > > > (when max_downtime == 5s).
> > > > > > > > > > > 
> > > > > > > > > > > Signed-off-by: Marcelo Tosatti 
> > > > > > > > > > > 
> > > > > > > > > > > ---
> > > > > > > > > > >  hw/i386/kvm/clock.c|  107 
> > > > > > > > > > > ++---
> > > > > > > > > > >  include/hw/i386/pc.h   |5 ++
> > > > > > > > > > >  target-i386/kvm.c  |7 +++
> > > > > > > > > > >  target-i386/kvm_i386.h |1 
> > > > > > > > > > >  4 files changed, 106 insertions(+), 14 deletions(-)
> > > > > > > > > > > 
> > > > > > > > > > > v2: 
> > > > > > > > > > > - improve variable names (Juan)
> > > > > > > > > > > - consolidate code on kvm_get_clock function (Paolo)
> > > > > > > > > > > - return mach_use_reliable_get_clock from needed function 
> > > > > > > > > > > (Paolo)
> > > > > > > > > > > v3: 
> > > > > > > > > > > - simplify check for src_use_reliable_get_clock (Eduardo)
> > > > > > > > > > > - move src_use_reliable_get_clock initialization to 
> > > > > > > > > > > realize (Eduardo)
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > Index: qemu-mig-advance-clock/hw/i386/kvm/clock.c
> > > > > > > > > > > ===
> > > > > > > > > > > --- qemu-mig-advance-clock.orig/hw/i386/kvm/clock.c   
> > > > > > > > > > > 2016-11-17 15:07:11.220632761 -0200
> > > > > > > > > > > +++ qemu-mig-advance-clock/hw/i386/kvm/clock.c
> > > > > > > > > > > 2016-11-17 15:11:51.372048640 -0200
> > > > > > > > > > > @@ -36,6 +36,12 @@
> > > > > > > > > > >  
> > > > > > > > > > >  uint64_t clock;
> > > > > > > > > > >  bool clock_valid;
> > > > > > > > > > > +
> > > > > > > > > > > +/* whether machine supports reliable KVM_GET_CLOCK */
> > > > > > > > > > > +bool mach_use_reliable_get_clock;
> > > > > > > > > > > +
> > > > > > > > > > > +/* whether source host supported reliable 
> > > > > > > > > > > KVM_GET_CLOCK */
> > > > > > > > > > > +bool src_use_reliable_get_clock;
> > > > > > > > > > >  } KVMClockState;
> > > > > > > > > > >  
> > > > > > > > > > >  struct pvclock_vcpu_time_info {
> > > > > > > > > > > @@ -81,6 +87,19 @@
> > > > > > > > > > >  return nsec + time.system_time;
> > > > > > > > > > >  }
> > > > > > > > > > >  
> > > > > > > > > > > +static uint64_t kvm_get_clock(void)
> > > > > > > > > > > +{
> > > > > > > > > > > +struct kvm_clock_data data;
> > > > > > > > > > > +int ret;
> > > > > > > > > > > +
> > > > > > > > > > > +ret = kvm_vm_ioctl(kvm_state, KVM_GET_CLOCK, );
> > > > > > > > > > > +if (ret < 0) {
> > > > > > > > > > > +fprintf(stderr, "KVM_GET_CLOCK failed: %s\n", 
> > > > > > > > > > > strerror(ret));
> > > > > > > > > > > +abort();
> > > > > > > > > > > +}
> > > > > > > > > > > +return data.clock;
> > > > > > > > > > > +}
> > > > > > > > > > > +
> > > > > > > > > > >  static void kvmclock_vm_state_change(void *opaque, int 
> > > > > > > > > > > running,
> > > > > > > > > > >   RunState state)
> > > > > > > > > > >  {
> > > > > > > > > > > @@ -91,15 +110,36 @@
> > > > > > > > > > 
> > > > > > > > > > Can you please use "diff -p" on your patches?
> > > > > > > > > > 
> > > > > > > > > > >  
> > > > > > > > > > >  if 

Re: [Qemu-devel] [PATCH v4 04/13] virtio: poll virtqueues for new buffers

[Paolo Bonzini]

> Anyway, I'm glad the polling series on its own is already showing good
> performance results.  I'd like to merge it at the beginning of QEMU 2.9
> so we can work on further improvements that build on top of it.

Yes, it would be great if you can stage the infrastructure bits, so that
I can rebase my lockcnt patches on top.

Paolo

Re: [Qemu-devel] [PATCH for-2.8] qdev: apply global properties in reverse order

[Cornelia Huck]

On Mon, 05 Dec 2016 16:21:22 +0100
Greg Kurz  wrote:

> The current code recursively applies global properties from child up to
> parent. So, if you have:
> 
> -global virtio-pci.disable-modern=on
> -global virtio-blk-pci.disable-modern=off
> 
> Then the default value of disable-modern for a virtio-blk-pci device is on,
> which looks wrong from an OOP perspective.
> 
> This patch reverses the logic, so that a child property always prevail.

This sounds reasonable...

> 
> This fixes a subtle bug that got introduced in 2.7 with commit "9a4c0e220d8a
> hw/virtio-pci: fix virtio behaviour" for older (< 2.7) machine types: the
> HW_COMPAT_2_6 macro contains global virtio-pci.disable-* properties which
> would silently override global properties passed on the command line for
> virtio subtypes.
> 
> Signed-off-by: Greg Kurz 
> ---
> 
> AFAIK, libvirt's XML doesn't know about modern/legacy modes for virtio
> devices. Early adopters of virtio 1.0 had to rely on the 
> tag to pass global properties to QEMU. This patch ensures that XML files
> used with older machine types remain valid with newer versions of QEMU.
> 
> FWIW I guess it could help to have this fix in 2.8, and also probably in
> 2.7.1.

...but I'm a bit worried about doing that change this late in the
cycle, as we may introduce subtle changes for other configurations. At
the very least, we should look over the existing backwards compat
properties (I'll look at those I'm familiar with).

> 
> Please advise.
> 
>  hw/core/qdev-properties.c |   11 ++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> index 2a8276806721..1345f489d6b1 100644
> --- a/hw/core/qdev-properties.c
> +++ b/hw/core/qdev-properties.c
> @@ -1119,11 +1119,20 @@ static void 
> qdev_prop_set_globals_for_type(DeviceState *dev,
>  void qdev_prop_set_globals(DeviceState *dev)
>  {
>  ObjectClass *class = object_get_class(OBJECT(dev));
> +GSList *class_list = NULL;
> 
>  do {
> -qdev_prop_set_globals_for_type(dev, object_class_get_name(class));
> +class_list = g_slist_prepend(class_list, class);
>  class = object_class_get_parent(class);
>  } while (class);
> +
> +do {
> +GSList *head = class_list;
> +
> +qdev_prop_set_globals_for_type(dev, 
> object_class_get_name(head->data));
> +class_list = g_slist_next(head);
> +g_slist_free_1(head);
> +} while (class_list);
>  }
> 
>  /* --- 64bit unsigned int 'size' type --- */
> 

It is a bit unfortunate that we need a double loop here, but I don't
see any good alternative.

Re: [Qemu-devel] [PATCH for-2.9 24/30] aspeed: use first SPI flash as a boot ROM

[mar.krzeminski]

W dniu 05.12.2016 o 15:53, Cédric Le Goater pisze:

On 12/05/2016 10:57 AM, Marcin Krzemiński wrote:

2016-12-05 10:36 GMT+01:00 Cédric Le Goater >:

 Hello Marcin,

 On 12/04/2016 06:00 PM, mar.krzeminski wrote:
 > Hi Cedric,
 >
 > it looks like good idea for now to handle boot from flash.
 > As I understand you are trying to omit bootrom code in Qemu model?

 I suppose you mean handling a romd memory region under the m25p80
 object ?

It could be that I mess up everything because my understanding how the real HW
work and boot is wrong. Please correct my assumptions:
1. You are setting boot source to spi-nor (jumper, resistor whatever)
2. There is a small bootrom in SoC (not u-boot) that set eg. reset vector, 
configure SMC
and start execude code from flash.
3. Memory mapped flash content is at address 0x1c00.

No. The memory flash content CS0 is mapped at 0x0 and starts
with U-Boot directly, there is no preliminary loader.

U-Boot is not merged in mainline yet. We are "slowly" building a
tree for upstream :
  
	https://github.com/openbmc/u-boot/

https://github.com/legoater/u-boot/

Thanks, I will look at those source in that case and the get back.

Thanks,
Marcin




 > This could lead you to some hacks in device models (eg SMC).

 I haven't had to, yet.

 > W dniu 29.11.2016 o 16:44, Cédric Le Goater pisze:
 >> Fill a ROM region with the flash content to support U-Boot. This is a
 >> little hacky but until we can boot from a MMIO region, it seems
 >> difficult to do anything else.
 >>
 >> Signed-off-by: Cédric Le Goater >
 >> Reviewed-by: Joel Stanley >
 >> Reviewed-by: Andrew Jeffery >
 >> ---
 >>  hw/arm/aspeed.c | 41 +
 >>  1 file changed, 41 insertions(+)
 >>
 >> diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
 >> index 40c13838fb2d..a92c2f1c362b 100644
 >> --- a/hw/arm/aspeed.c
 >> +++ b/hw/arm/aspeed.c
 >> @@ -20,6 +20,8 @@
 >>  #include "qemu/log.h"
 >>  #include "sysemu/block-backend.h"
 >>  #include "sysemu/blockdev.h"
 >> +#include "hw/loader.h"
 >> +#include "qemu/error-report.h"
 >>
 >>  static struct arm_boot_info aspeed_board_binfo = {
 >>  .board_id = -1, /* device-tree-only board */
 >> @@ -104,6 +106,28 @@ static const AspeedBoardConfig aspeed_boards[] = {
 >>  },
 >>  };
 >>
 >> +#define FIRMWARE_ADDR 0x0
 >> +
 >> +static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, size_t 
rom_size,
 >> +   Error **errp)
 >> +{
 >> +BlockBackend *blk = blk_by_legacy_dinfo(dinfo);
 >> +uint8_t *storage;
 >> +
 >> +if (rom_size > blk_getlength(blk)) {
 >> +rom_size = blk_getlength(blk);
 >> +}
 > I was not able to attach smaller file as m25p80 storage.

 yes that's most probably because m25p80_realize() does :

if (blk_pread(s->blk, 0, s->storage, s->size) != s->size) {
 error_setg(errp, "failed to read the initial flash content");
 return;
 }

 my bad. May be, we could relax a bit the test and allow smaller block
 backends ?

  
I think not, if you have fs on flash (eg. UBI) and smaller image fs will not mount,

or in worse case you can issue writes outside the file.



 >> +
 >> +storage = g_new0(uint8_t, rom_size);
 >> +if (blk_pread(blk, 0, storage, rom_size) < 0) {
 >> +error_setg(errp, "failed to read the initial flash content");
 >> +return;
 >> +}
 >> +
 >> +rom_add_blob_fixed("aspeed.boot_rom", storage, rom_size, addr);
 >> +g_free(storage);
 >> +}
 >> +
 >>  static void aspeed_board_init_flashes(AspeedSMCState *s, const char 
*flashtype,
 >>Error **errp)
 >>  {
 >> @@ -135,6 +159,7 @@ static void aspeed_board_init(MachineState *machine,
 >>  {
 >>  AspeedBoardState *bmc;
 >>  AspeedSoCClass *sc;
 >> +DriveInfo *drive0 = drive_get(IF_MTD, 0, 0);
 >>
 >>  bmc = g_new0(AspeedBoardState, 1);
 >>  object_initialize(>soc, (sizeof(bmc->soc)), cfg->soc_name);
 >> @@ -168,6 +193,22 @@ static void aspeed_board_init(MachineState 
*machine,
 >>  aspeed_board_init_flashes(>soc.fmc, cfg->fmc_model, 
_abort);
 >>  aspeed_board_init_flashes(>soc.spi[0], cfg->spi_model, 
_abort);
 >>
 >> +/* Install first FMC flash content as a boot rom. */
 >> +if (drive0) {
 >> +AspeedSMCFlash *fl = >soc.fmc.flashes[0];
 >> +MemoryRegion *boot_rom = g_new(MemoryRegion, 1);
 >> +
 >> +/*
 >> + * create a ROM region 

Re: [Qemu-devel] [PATCH for-2.9 22/30] aspeed/smc: handle dummy bytes when doing fast reads

[mar.krzeminski]

W dniu 05.12.2016 o 15:14, Cédric Le Goater pisze:

On 12/04/2016 05:46 PM, mar.krzeminski wrote:


W dniu 29.11.2016 o 16:44, Cédric Le Goater pisze:

When doing fast read, a certain amount of dummy bytes should be sent
before the read. This number is configurable in the controler CE0
Control Register and needs to be modeled using fake transfers the
flash module.

When the controller is configured for Command mode, the SPI command
used to do the read is stored in the CE0 control register but, in User
mode, we need to snoop into the flow of bytes to catch the command. It
should be the first byte after CS select.

Signed-off-by: Cédric Le Goater 
Reviewed-by: Andrew Jeffery 
---
  hw/ssi/aspeed_smc.c | 58 ++---
  include/hw/ssi/aspeed_smc.h |  1 +
  2 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 9596ea94a3bc..733fd8b09c06 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -71,7 +71,9 @@
  #define R_CTRL0   (0x10 / 4)
  #define   CTRL_CMD_SHIFT   16
  #define   CTRL_CMD_MASK0xff
+#define   CTRL_DUMMY_HIGH_SHIFT14
  #define   CTRL_AST2400_SPI_4BYTE   (1 << 13)
+#define   CTRL_DUMMY_LOW_SHIFT 6 /* 2 bits [7:6] */
  #define   CTRL_CE_STOP_ACTIVE  (1 << 2)
  #define   CTRL_CMD_MODE_MASK   0x3
  #define CTRL_READMODE  0x0
@@ -151,6 +153,7 @@
  #define SPI_OP_WRDI   0x04/* Write disable */
  #define SPI_OP_RDSR   0x05/* Read status register */
  #define SPI_OP_WREN   0x06/* Write enable */
+#define SPI_OP_READ_FAST  0x0b/* Read data bytes (high frequency) */
  
  /* Used for Macronix and Winbond flashes. */

  #define SPI_OP_EN4B   0xb7/* Enter 4-byte mode */
@@ -510,6 +513,12 @@ static void aspeed_smc_flash_setup_read(AspeedSMCFlash 
*fl, uint32_t addr)
  /* access can not exceed CS segment */
  addr = aspeed_smc_check_segment_addr(fl, addr);
  
+/*

+ * Remember command as we might need to send dummy bytes before
+ * reading data
+ */
+fl->cmd = cmd;
+
  /* TODO: do we have to send 4BYTE each time ? */
  if (aspeed_smc_flash_is_4byte(fl)) {
  ssi_transfer(s->spi, SPI_OP_EN4B);
@@ -525,27 +534,50 @@ static void aspeed_smc_flash_setup_read(AspeedSMCFlash 
*fl, uint32_t addr)
  ssi_transfer(s->spi, (addr & 0xff));
  }
  
+static int aspeed_smc_flash_dummies(const AspeedSMCFlash *fl)

+{
+AspeedSMCState *s = fl->controller;
+uint32_t r_ctrl0 = s->regs[s->r_ctrl0 + fl->id];
+uint32_t dummy_high = (r_ctrl0 >> CTRL_DUMMY_HIGH_SHIFT) & 0x1;
+uint32_t dummy_low = (r_ctrl0 >> CTRL_DUMMY_LOW_SHIFT) & 0x3;
+
+return ((dummy_high << 2) | dummy_low) * 8;
+}
+
+static uint64_t aspeed_smc_flash_do_read(AspeedSMCFlash *fl, unsigned size)
+{
+AspeedSMCState *s = fl->controller;
+uint64_t ret = 0;
+int i;
+
+if (fl->cmd == SPI_OP_READ_FAST) {
+for (i = 0; i < aspeed_smc_flash_dummies(fl); i++) {
+ssi_transfer(s->spi, 0x0);
+}
+}

Generally this is wrong, controller should be not aware of any command in user 
mode.
Currently you are forced to do this by m25p80c dummy cycles implementation.
I had no time to improve this since it need to update in Xilinx models, but 
maybe it is good place to talk about the solution.
I was thinking to add to ssi function transferN, and use it in flash models.
Firs introduce new state for dummy cycles in m25p80 and then:

This new state would depend on the command op ? fastread would put
the object in a dummy_cycle state ?
Yes, only when needed (so it will be all fast reads in single/dual/quad 
modes).



a) in case caller use ssi_transfer(transfer8 in flsh models) dummy count will 
be incremented by 8.
This should solve the problem when flash is connected to controller that is not 
aware about dummy cycles,
like this mode or clear SPI controller.
b) when caller use ssi_trasferN length (in bits) will be the number of dummy 
cycles.

What is your opinion?

when you have some time, please send a quick rfc patch for the API :)
so that I can experiment on the aspeed controller.

Will do, can not promise when, but target is 2.9 :)

Thanks,
Marcin


Thanks,

C.


+fl->cmd = 0;
+
+for (i = 0; i < size; i++) {
+ret |= ssi_transfer(s->spi, 0x0) << (8 * i);
+}
+return ret;
+}
+
  static uint64_t aspeed_smc_flash_read(void *opaque, hwaddr addr, unsigned 
size)
  {
  AspeedSMCFlash *fl = opaque;
-const AspeedSMCState *s = fl->controller;
  uint64_t ret = 0;
-int i;
  
  switch (aspeed_smc_flash_mode(fl)) {

  case CTRL_USERMODE:
-for (i = 0; i < size; i++) {
-ret |= ssi_transfer(s->spi, 0x0) << (8 * i);
-}
+ret = aspeed_smc_flash_do_read(fl, size);
  break;
  case CTRL_READMODE:
  case CTRL_FREADMODE:
  aspeed_smc_flash_select(fl);
  

Re: [Qemu-devel] [PATCH for-2.9 17/30] aspeed/smc: handle SPI flash Command mode

[mar.krzeminski]

W dniu 05.12.2016 o 15:07, Cédric Le Goater pisze:

On 12/04/2016 05:31 PM, mar.krzeminski wrote:

Hi Cedric,

Since there is no public datasheet user guide for SMC I would ask some question
regarding HW itself because I got impression that you are implementing in this
model a part functionality that is done by Bootrom.

W dniu 29.11.2016 o 16:43, Cédric Le Goater pisze:

The Aspeed SMC controllers have a mode (Command mode) in which
accesses to the flash content are no different than doing MMIOs. The
controller generates all the necessary commands to load (or store)
data in memory.

However, accesses are restricted to the segment window assigned the
the flash module by the controller. This window is defined by the
Segment Address Register.

Signed-off-by: Cédric Le Goater 
Reviewed-by: Andrew Jeffery 
---
  hw/ssi/aspeed_smc.c | 174 
  1 file changed, 162 insertions(+), 12 deletions(-)

diff --git a/hw/ssi/aspeed_smc.c b/hw/ssi/aspeed_smc.c
index 72a44150b0a1..eec087199a22 100644
--- a/hw/ssi/aspeed_smc.c
+++ b/hw/ssi/aspeed_smc.c
@@ -69,6 +69,7 @@
  #define R_CTRL0   (0x10 / 4)
  #define   CTRL_CMD_SHIFT   16
  #define   CTRL_CMD_MASK0xff
+#define   CTRL_AST2400_SPI_4BYTE   (1 << 13)
  #define   CTRL_CE_STOP_ACTIVE  (1 << 2)
  #define   CTRL_CMD_MODE_MASK   0x3
  #define CTRL_READMODE  0x0
@@ -135,6 +136,16 @@
  #define ASPEED_SOC_SPI_FLASH_BASE   0x3000
  #define ASPEED_SOC_SPI2_FLASH_BASE  0x3800
  
+/* Flash opcodes. */

+#define SPI_OP_READ   0x03/* Read data bytes (low frequency) */
+#define SPI_OP_WRDI   0x04/* Write disable */
+#define SPI_OP_RDSR   0x05/* Read status register */
+#define SPI_OP_WREN   0x06/* Write enable */

Are you sure that the controller is aware af all above commands (especially 
WD/WE and RDS)?

HW is aware of SPI_OP_READ which is the default command for the
"normal" read mode. For other modes, fast and write, the command
op is configured in the CEx Control Register.

These ops are used in the model :

  * SPI_OP_READ_FAST, for dummies
  * SPI_OP_EN4B, might be useless if we expect software to send
this command before using this mode.
  * SPI_OP_WREN, same comment.

The rest I should remove as it is unused.

I think only SPI_OP_READ should stay in the model, rest goes to guest.



+
+/* Used for Macronix and Winbond flashes. */
+#define SPI_OP_EN4B   0xb7/* Enter 4-byte mode */
+#define SPI_OP_EX4B   0xe9/* Exit 4-byte mode */
+

Same as above but I think 4byte address mode bit changes onlu nymber of bytes
that is sent after instruction.


  /*
   * Default segments mapping addresses and size for each slave per
   * controller. These can be changed when board is initialized with the
@@ -357,6 +368,98 @@ static inline bool aspeed_smc_is_writable(const 
AspeedSMCFlash *fl)
  return s->regs[s->r_conf] & (1 << (s->conf_enable_w0 + fl->id));
  }
  
+static inline int aspeed_smc_flash_cmd(const AspeedSMCFlash *fl)

+{
+AspeedSMCState *s = fl->controller;
+int cmd = (s->regs[s->r_ctrl0 + fl->id] >> CTRL_CMD_SHIFT) & CTRL_CMD_MASK;
+
+/* This is the default value for read mode. In other modes, the
+ * command should be defined */
+if (aspeed_smc_flash_mode(fl) == CTRL_READMODE) {
+cmd = SPI_OP_READ;
+}
+
+if (!cmd) {

cmd == 0 => NOP command for some flash (eg. mx66l1g45g).

So I should use another default value, OxFF ?
I believe this check is not needed at all because m25p80c will tell if 
it has

unsupported instruction and HW should accept all register values, isn't it?



+qemu_log_mask(LOG_GUEST_ERROR, "%s: no command defined for mode %d\n",
+  __func__, aspeed_smc_flash_mode(fl));
+}
+
+return cmd;
+}
+
+static inline int aspeed_smc_flash_is_4byte(const AspeedSMCFlash *fl)
+{
+AspeedSMCState *s = fl->controller;
+
+if (s->ctrl->segments == aspeed_segments_spi) {
+return s->regs[s->r_ctrl0] & CTRL_AST2400_SPI_4BYTE;
+} else {
+return s->regs[s->r_ce_ctrl] & (1 << (CTRL_EXTENDED0 + fl->id));
+}
+}
+
+static void aspeed_smc_flash_select(const AspeedSMCFlash *fl)
+{
+AspeedSMCState *s = fl->controller;
+
+s->regs[s->r_ctrl0 + fl->id] &= ~CTRL_CE_STOP_ACTIVE;
+qemu_set_irq(s->cs_lines[fl->id], aspeed_smc_is_ce_stop_active(fl));
+}
+
+static void aspeed_smc_flash_unselect(const AspeedSMCFlash *fl)
+{
+AspeedSMCState *s = fl->controller;
+
+s->regs[s->r_ctrl0 + fl->id] |= CTRL_CE_STOP_ACTIVE;
+qemu_set_irq(s->cs_lines[fl->id], aspeed_smc_is_ce_stop_active(fl));
+}
+
+static uint32_t aspeed_smc_check_segment_addr(AspeedSMCFlash *fl, uint32_t 
addr)
+{
+AspeedSMCState *s = fl->controller;
+AspeedSegments seg;
+
+aspeed_smc_reg_to_segment(s->regs[R_SEG_ADDR0 + fl->id], );
+if ((addr & (seg.size - 1)) != addr) {
+

Re: [Qemu-devel] [PATCH for-2.8] target-arm/translate-a64: fix gen_load_exclusive

[Richard Henderson]

On 12/05/2016 03:09 AM, Alex Bennée wrote:
> 
> Alex Bennée  writes:
> 
>> While testing rth's latest TCG patches with risu I found ldaxp was
>> broken. Investigating further I found it was broken by 1dd089d0 when
>> the cmpxchg atomic work was merged.
> 
> CC'ing Paolo/Richard
> 
> Do you guys have any final TCG fixes planned for 2.8 that can take this
> fix for the ldaxp regression?

I don't have any pending patchs for 2.8, no.

>> As part of that change the code
>> attempted to be clever by doing a single 64 bit load and then shuffle
>> the data around to set the two 32 bit registers.

It's not trying to be "clever", it's trying to be correct, giving an atomic
64-bit load.

The fact that we didn't attempt an atomic 128-bit load here for the 64-bit pair
is a bug.  We have support for that in helper_atomic_ldo_le_mmu; I'm not sure
why I didn't use that here.

>> As I couldn't quite follow the endian magic I've simply partially
>> reverted the change to the original code gen_load_exclusive code. This
>> doesn't affect the cmpxchg functionality as that is all done on in
>> gen_store_exclusive part which is untouched.

We'll have to fix it properly eventually.  But perhaps 2.9 is soon enough,
since that's when mttcg will go in.


r~

Re: [Qemu-devel] [PATCH for-2.9 00/17] target-i386: Implement query-cpu-model-expansion

[David Hildenbrand]

Am 02.12.2016 um 22:17 schrieb Eduardo Habkost:

This series implements query-cpu-model-expansion on target-i386.

QAPI / interface changes


When implementing this, I have noticed that the "host" CPU model
in i386 includes some migration-unsafe features that can't be
translated to any migration-safe representation: "pmu", and
"host-cache-info".

To be able to handle the migration-unsafe features, I have
extended the query-cpu-model-expansion definition to be clear
about what happens to those features when the CPU model is
expanded (in short: static expansion removes them, full expansion
keeps them).


I think this makes sense. The important part is to really keep static 
expansion static :)




I also added "static" and "migration-safe" fields to the return
value of query-cpu-model-expansion, so callers can know if the
the expanded representation is static and migration-safe.


Is static really needed? I can understand why migration-safe might be
of interest, but can't see how "static" could help (I mean we have
static expansion for this purpose). Do you have anything special in
mind regarding exposing "static"?



Test code
-

I have added a Python test script for the feature, that will try
multiple combinations of the expansion operation, and see if the
returned data keeps matches some constratins.

The test script works with the s390x query-cpu-model-expansion
command, except that: 1) I couldn't test it with KVM; 2) qtest.py
error handling when QEMU refuses to run is unreliable (so the
script needs runnability information to be availble in TCG mode,
too, to skip not-runnable CPU models and avoid problems).


Everything except "host" should behave completely the same on s390x
with or without KVM being active. So with !KVM tests we can already
cover most of the interesting parts. Thanks for taking care of s390x.



Future versions of the test script could run a arch-specific
CPUID-dump guest binary, and validate data seen by the guest
directly. While we don't do that, the script validates all QOM
properties on the CPU objects looking for unexpected changes. At
least in the case of x86, the QOM properties will include lots of
the CPUID data seen by the guest, giving us decent coverage.


Something like that would be cool. Unfortunately, e.g. on s390x some 
CPUID-like data (e.g. STFL(E) and SCP info) is only available from

kernel space. So we can't simply run a user space script. However,
something like a kernel module would be possible (or even something like 
the s390x pc-bios to simply read and dump the data).



--

David

[Qemu-devel] [PATCH for-2.8] qdev: apply global properties in reverse order

[Greg Kurz]

The current code recursively applies global properties from child up to
parent. So, if you have:

-global virtio-pci.disable-modern=on
-global virtio-blk-pci.disable-modern=off

Then the default value of disable-modern for a virtio-blk-pci device is on,
which looks wrong from an OOP perspective.

This patch reverses the logic, so that a child property always prevail.

This fixes a subtle bug that got introduced in 2.7 with commit "9a4c0e220d8a
hw/virtio-pci: fix virtio behaviour" for older (< 2.7) machine types: the
HW_COMPAT_2_6 macro contains global virtio-pci.disable-* properties which
would silently override global properties passed on the command line for
virtio subtypes.

Signed-off-by: Greg Kurz 
---

AFAIK, libvirt's XML doesn't know about modern/legacy modes for virtio
devices. Early adopters of virtio 1.0 had to rely on the 
tag to pass global properties to QEMU. This patch ensures that XML files
used with older machine types remain valid with newer versions of QEMU.

FWIW I guess it could help to have this fix in 2.8, and also probably in
2.7.1.

Please advise.

 hw/core/qdev-properties.c |   11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
index 2a8276806721..1345f489d6b1 100644
--- a/hw/core/qdev-properties.c
+++ b/hw/core/qdev-properties.c
@@ -1119,11 +1119,20 @@ static void qdev_prop_set_globals_for_type(DeviceState 
*dev,
 void qdev_prop_set_globals(DeviceState *dev)
 {
 ObjectClass *class = object_get_class(OBJECT(dev));
+GSList *class_list = NULL;
 
 do {
-qdev_prop_set_globals_for_type(dev, object_class_get_name(class));
+class_list = g_slist_prepend(class_list, class);
 class = object_class_get_parent(class);
 } while (class);
+
+do {
+GSList *head = class_list;
+
+qdev_prop_set_globals_for_type(dev, object_class_get_name(head->data));
+class_list = g_slist_next(head);
+g_slist_free_1(head);
+} while (class_list);
 }
 
 /* --- 64bit unsigned int 'size' type --- */

Re: [Qemu-devel] New wiki page: http://wiki.qemu.org/Hosts

[Paolo Bonzini]

> On Dec 5, 2016, at 9:15 AM, Paolo Bonzini wrote:
> > On 05/12/2016 14:47, G 3 wrote:
> > > So now I have to figure out which commands to use. This script would
> > > have to work on all operating systems that QEMU is built on. This
> > > page
> > > has some pretty good information on how to do this:
> > > http://stackoverflow.com/questions/12422289/bash-command-to-covert-
> > > html-page-to-a-text-file
> >
> > No, I just want to do that once and get rid of the Hosts/ pages.
> >
> > Paolo
> 
> Really? Why? I thought they were very useful. Where would people who
> are doing a web search go for building information?

To http://www.quasiquark.com/download/#source in the new website, which
would have a link to the BUILDING file.

Paolo

Re: [Qemu-devel] [RFC 11/15] s390x: Initialize default bus lists

[David Hildenbrand]

Am 22.11.2016 um 02:12 schrieb Eduardo Habkost:

Populate the default_bus_types list for the s390x machines. This
will allow qmp-machine-info.py to run in strict mode for s390x.

Signed-off-by: Eduardo Habkost 
---
 hw/s390x/s390-virtio-ccw.c | 6 ++
 tests/qmp-machine-info.py  | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index e340eab..2532fcb 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -206,6 +206,12 @@ static void ccw_machine_class_init(ObjectClass *oc, void 
*data)
 mc->use_sclp = 1;
 mc->max_cpus = 248;
 mc->get_hotplug_handler = s390_get_hotplug_handler;
+machine_class_add_default_bus(mc, TYPE_VIRTUAL_CSS_BUS);
+machine_class_add_default_bus(mc, TYPE_PCI_BUS);
+machine_class_add_default_bus(mc, TYPE_S390_PCI_BUS);
+machine_class_add_default_bus(mc, "s390-sclp-events-bus");
+machine_class_add_default_bus(mc, TYPE_VIRTIO_BUS);
+


Maybe move and use
#define TYPE_SCLP_EVENTS_BUS "s390-sclp-events-bus"

(maybe this should then become TYPE_S390_SCLP_EVENTS_BUS ...)

?

--

David

[Qemu-devel] Slow qemu guest performance after host is resumed from suspended state

[Ruslan Habalov]

Hi everyone,

I'm having the following qemu issue since about February on Xubuntu.

Steps to reproduce:
1) Boot host and boot qemu guest - everything works perfect
2) Shutdown qemu guest
3) Suspend host and resume host
4) Boot qemu guest again - the guest is now extremely slow

Current system setup:
host: kernel 4.8.0-27-generic (also reproducible with older kernel versions)
qemu guest: Linux or Windows affected (tested both)

I've tried to debug the issue on my own with the following results:
--
https://www.evonide.com/images/qemu_problem.png
- This shows "perf top $QEMU_PID" from guest boot to shutdown for three
different scenarios:
  Left: normal guest behavior after fresh boot of host system
  Middle: very slow guest after host was suspended and resumed
  Right: test run with disabled HPET (since I initially assumed some
problems with HPET)

https://www.evonide.com/images/qemu_hpet_investigation.png
- Results of "perf record -g" and "perf report"
  According to stefanha (qemu IRC) this doesn't seem to indicate any error
sources.

https://www.evonide.com/images/qemu_vm_exit_healthy_vs_unhealthy.png
- Results of running: "perf record -a -e kvm:kvm_exit" and "perf
report" from guest boot to shutdown
  - Left: normal situation (after fresh host boot)
  - Right: bad situation(after suspend and resume on host and boot of
qemu guest)
  There were ~300 out of order events reported by perf for the "bad"
scenario (vs. only 2 for the "good" scenario).
--
I was also recommended to check the host timers (e.g. /proc/timer_list).
However, this file has more than 800 lines and I am unsure what to look for.

Please let me know if you need further information on this issue.

Best regards,
Ruslan Habalov (Evonide)

Re: [Qemu-devel] [PATCH for-2.8] qcow2: Don't strand clusters near 2G intervals during commit

[Eric Blake]

On 12/03/2016 11:34 AM, Eric Blake wrote:
> The qcow2_make_empty() function is reached during 'qemu-img commit',
> in order to clear out ALL clusters of an image.  However, if the
> image cannot use the fast code path (true if the image is format
> 0.10, or if the image contains a snapshot), the cluster size is
> larger than 512, and the image is larger than 2G in size, then our
> choice of sector_step causes problems.  Since it is not cluster
> aligned, but qcow2_discard_clusters() silently ignores an unaligned
> head or tail, we are leaving clusters allocated.
> 
> Enhance the testsuite to expose the flaw, and patch the problem by
> ensuring our step size is aligned.
> 
> [qcow2_discard_clusters() is a GROSS interface: it takes a mix of
> byte offset and sector count to perform cluster operations. But
> fixing it to use a saner byte/byte rather than byte/sector interface,
> and/or asserting that the counts are now aligned thanks to both
> this patch and commit 3482b9b, is material for another day.]
> 
> Signed-off-by: Eric Blake 
> ---
>  block/qcow2.c  |   3 +-
>  tests/qemu-iotests/097 |  41 +---
>  tests/qemu-iotests/097.out | 249 
> +
>  3 files changed, 210 insertions(+), 83 deletions(-)

> +++ b/block/qcow2.c
> @@ -2808,7 +2808,8 @@ static int qcow2_make_empty(BlockDriverState *bs)
>  {
>  BDRVQcow2State *s = bs->opaque;
>  uint64_t start_sector;
> -int sector_step = INT_MAX / BDRV_SECTOR_SIZE;
> +int sector_step = QEMU_ALIGN_DOWN(INT_MAX / BDRV_SECTOR_SIZE,
> +  s->cluster_size);

Oh shoot. I got the units wrong, and made the slow path do more loop
iterations than necessary (rounding sectors to cluster size in bytes is
inappropriate - either the rounding has to occur before division, or the
rounding needs to be by secotrs instead of bytes).  I'll send a v2 that
gets the math right.

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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [RFC 11/15] s390x: Initialize default bus lists

[Cornelia Huck]

On Mon, 5 Dec 2016 16:24:00 +0100
David Hildenbrand  wrote:

> Am 22.11.2016 um 02:12 schrieb Eduardo Habkost:
> > Populate the default_bus_types list for the s390x machines. This
> > will allow qmp-machine-info.py to run in strict mode for s390x.
> >
> > Signed-off-by: Eduardo Habkost 
> > ---
> >  hw/s390x/s390-virtio-ccw.c | 6 ++
> >  tests/qmp-machine-info.py  | 2 +-
> >  2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> > index e340eab..2532fcb 100644
> > --- a/hw/s390x/s390-virtio-ccw.c
> > +++ b/hw/s390x/s390-virtio-ccw.c
> > @@ -206,6 +206,12 @@ static void ccw_machine_class_init(ObjectClass *oc, 
> > void *data)
> >  mc->use_sclp = 1;
> >  mc->max_cpus = 248;
> >  mc->get_hotplug_handler = s390_get_hotplug_handler;
> > +machine_class_add_default_bus(mc, TYPE_VIRTUAL_CSS_BUS);
> > +machine_class_add_default_bus(mc, TYPE_PCI_BUS);
> > +machine_class_add_default_bus(mc, TYPE_S390_PCI_BUS);
> > +machine_class_add_default_bus(mc, "s390-sclp-events-bus");
> > +machine_class_add_default_bus(mc, TYPE_VIRTIO_BUS);
> > +
> 
> Maybe move and use
> #define TYPE_SCLP_EVENTS_BUS "s390-sclp-events-bus"
> 
> (maybe this should then become TYPE_S390_SCLP_EVENTS_BUS ...)
> 
> ?
> 

I'd ack/take such a patch.

[Qemu-devel] [PATCH for-2.8 v2] qcow2: Don't strand clusters near 2G intervals during commit

[Eric Blake]

The qcow2_make_empty() function is reached during 'qemu-img commit',
in order to clear out ALL clusters of an image.  However, if the
image cannot use the fast code path (true if the image is format
0.10, or if the image contains a snapshot), the cluster size is
larger than 512, and the image is larger than 2G in size, then our
choice of sector_step causes problems.  Since it is not cluster
aligned, but qcow2_discard_clusters() silently ignores an unaligned
head or tail, we are leaving clusters allocated.

Enhance the testsuite to expose the flaw, and patch the problem by
ensuring our step size is aligned.

Signed-off-by: Eric Blake 

---
v2: perform rounding correctly
---
 block/qcow2.c  |   3 +-
 tests/qemu-iotests/097 |  41 +---
 tests/qemu-iotests/097.out | 249 +
 3 files changed, 210 insertions(+), 83 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index ed9e0f3..96fb8a8 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -2808,7 +2808,8 @@ static int qcow2_make_empty(BlockDriverState *bs)
 {
 BDRVQcow2State *s = bs->opaque;
 uint64_t start_sector;
-int sector_step = INT_MAX / BDRV_SECTOR_SIZE;
+int sector_step = (QEMU_ALIGN_DOWN(INT_MAX, s->cluster_size) /
+   BDRV_SECTOR_SIZE);
 int l1_clusters, ret = 0;

 l1_clusters = DIV_ROUND_UP(s->l1_size, s->cluster_size / sizeof(uint64_t));
diff --git a/tests/qemu-iotests/097 b/tests/qemu-iotests/097
index 01d8dd0..4c33e80 100755
--- a/tests/qemu-iotests/097
+++ b/tests/qemu-iotests/097
@@ -46,7 +46,7 @@ _supported_proto file
 _supported_os Linux


-# Four passes:
+# Four main passes:
 #  0: Two-layer backing chain, commit to upper backing file (implicitly)
 # (in this case, the top image will be emptied)
 #  1: Two-layer backing chain, commit to upper backing file (explicitly)
@@ -56,22 +56,30 @@ _supported_os Linux
 #  3: Two-layer backing chain, commit to lower backing file
 # (in this case, the top image will implicitly stay unchanged)
 #
+# Each pass is run twice, since qcow2 has different code paths for cleaning
+# an image depending on whether it has a snapshot.
+#
 # 020 already tests committing, so this only tests whether image chains are
 # working properly and that all images above the base are emptied; therefore,
-# no complicated patterns are necessary
+# no complicated patterns are necessary.  Check near the 2G mark, as qcow2
+# has been buggy at that boundary in the past.
 for i in 0 1 2 3; do
+for j in 0 1; do

 echo
-echo "=== Test pass $i ==="
+echo "=== Test pass $i.$j ==="
 echo

-TEST_IMG="$TEST_IMG.base" _make_test_img 64M
-TEST_IMG="$TEST_IMG.itmd" _make_test_img -b "$TEST_IMG.base" 64M
-_make_test_img -b "$TEST_IMG.itmd" 64M
+TEST_IMG="$TEST_IMG.base" _make_test_img 2100M
+TEST_IMG="$TEST_IMG.itmd" _make_test_img -b "$TEST_IMG.base" 2100M
+_make_test_img -b "$TEST_IMG.itmd" 2100M
+if [ $j -eq 0 ]; then
+$QEMU_IMG snapshot -c snap "$TEST_IMG"
+fi

-$QEMU_IO -c 'write -P 1 0 192k' "$TEST_IMG.base" | _filter_qemu_io
-$QEMU_IO -c 'write -P 2 64k 128k' "$TEST_IMG.itmd" | _filter_qemu_io
-$QEMU_IO -c 'write -P 3 128k 64k' "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c 'write -P 1 0x7ffd 192k' "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c 'write -P 2 0x7ffe 128k' "$TEST_IMG.itmd" | _filter_qemu_io
+$QEMU_IO -c 'write -P 3 0x7fff 64k' "$TEST_IMG" | _filter_qemu_io

 if [ $i -lt 3 ]; then
 if [ $i == 0 ]; then
@@ -88,12 +96,12 @@ if [ $i -lt 3 ]; then
 fi

 # Bottom should be unchanged
-$QEMU_IO -c 'read -P 1 0 192k' "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c 'read -P 1 0x7ffd 192k' "$TEST_IMG.base" | _filter_qemu_io

 # Intermediate should contain changes from top
-$QEMU_IO -c 'read -P 1 0 64k' "$TEST_IMG.itmd" | _filter_qemu_io
-$QEMU_IO -c 'read -P 2 64k 64k' "$TEST_IMG.itmd" | _filter_qemu_io
-$QEMU_IO -c 'read -P 3 128k 64k' "$TEST_IMG.itmd" | _filter_qemu_io
+$QEMU_IO -c 'read -P 1 0x7ffd 64k' "$TEST_IMG.itmd" | _filter_qemu_io
+$QEMU_IO -c 'read -P 2 0x7ffe 64k' "$TEST_IMG.itmd" | _filter_qemu_io
+$QEMU_IO -c 'read -P 3 0x7fff 64k' "$TEST_IMG.itmd" | _filter_qemu_io

 # And in pass 0, the top image should be empty, whereas in both other 
passes
 # it should be unchanged (which is both checked by qemu-img map)
@@ -101,9 +109,9 @@ else
 $QEMU_IMG commit -b "$TEST_IMG.base" "$TEST_IMG"

 # Bottom should contain all changes
-$QEMU_IO -c 'read -P 1 0 64k' "$TEST_IMG.base" | _filter_qemu_io
-$QEMU_IO -c 'read -P 2 64k 64k' "$TEST_IMG.base" | _filter_qemu_io
-$QEMU_IO -c 'read -P 3 128k 64k' "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c 'read -P 1 0x7ffd 64k' "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c 'read -P 2 0x7ffe 64k' "$TEST_IMG.base" | _filter_qemu_io
+$QEMU_IO -c 'read -P 3 0x7fff 64k' "$TEST_IMG.base" | _filter_qemu_io

 # Both top and intermediate 

Re: [Qemu-devel] [PATCH for-2.8] target-arm/translate-a64: fix gen_load_exclusive

[Peter Maydell]

On 5 December 2016 at 15:42, Richard Henderson  wrote:
> We'll have to fix it properly eventually.  But perhaps 2.9 is soon enough,
> since that's when mttcg will go in.

Could you provide a reviewed-by (or an explicit nak) for Alex's
patch, please? I didn't look at this area of the code when it
went in so I'm not really in position to review it even if it
goes via my tree.

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-2.8] qdev: apply global properties in reverse order

[Cornelia Huck]

On Mon, 5 Dec 2016 16:42:00 +0100
Cornelia Huck  wrote:

> On Mon, 05 Dec 2016 16:21:22 +0100
> Greg Kurz  wrote:

> > AFAIK, libvirt's XML doesn't know about modern/legacy modes for virtio
> > devices. Early adopters of virtio 1.0 had to rely on the 
> > tag to pass global properties to QEMU. This patch ensures that XML files
> > used with older machine types remain valid with newer versions of QEMU.

I recall some libvirt patches floating around for this legacy/modern
stuff, but I don't know their status.

> > 
> > FWIW I guess it could help to have this fix in 2.8, and also probably in
> > 2.7.1.
> 
> ...but I'm a bit worried about doing that change this late in the
> cycle, as we may introduce subtle changes for other configurations. At
> the very least, we should look over the existing backwards compat
> properties (I'll look at those I'm familiar with).

The s390x properties seem safe.

For virtio-pci, the ability to override extra state might become
problematic for modern devices. Although manually setting this property
is probably a patholotical case...

Re: [Qemu-devel] [RFC v3 0/3] virtio-net: Add support to MTU feature

[Aaron Conole]

"Michael S. Tsirkin"  writes:

> On Wed, Nov 30, 2016 at 01:16:59PM +0100, Maxime Coquelin wrote:
>> 
>> 
>> On 11/30/2016 12:23 PM, Jason Wang wrote:
>> > 
>> > 
>> > On 2016年11月30日 18:10, Maxime Coquelin wrote:
>> > > This series implements Virtio spec update from Aaron Conole which
>> > > defines a way for the host to expose its max MTU to the guest.
>> > > 
>> > > This third version re-desings how MTU value is provided to QEMU.
>> > > Now, host_mtu parameter is added to provide QEMU with the MTU value,
>> > > and the backend, if supported, gets notified of the MTU value when the
>> > > MTU feature neogotiation succeeds.
>> > > 
>> > > Only user backend currently supports MTU notification. A new protocol
>> > > feature has been implemented for sending MTU value to the backend.
>> > > Aaron, Kevin, Flavio, do you confirm this works for OVS if DPDK vhost lib
>> > > adds needed API to get the MTU value?
>> > > 
>> > > For kernel backend, it is expected the management tool also configures
>> > > the tap/macvtap interface with same MTU value.
>> > > Daniel, I would be interrested about your feedback on this implementation
>> > > from management tool point of view.
>> > 
>> > I believe we want management tool to configure both kernel and user
>> > backends.
>> 
>> Yes, I think you are right.
>> 
>> The vhost-user protocol feature would in this case be used to ensure
>> consistency.
>> 
>> Does that make sense, or we should just drop VHOST_USER_SET_MTU?
>> 
>> Thanks,
>> Maxime
>
>
> It doesn't hurt to have a feature and if set send mtu to backend,
> to verify that it can support that mtu.
> But we don't need to require that feature, if not supported
> we can just assume mtu is correct.

Sorry for late reply - I agree, this is fine.

Re: [Qemu-devel] [PATCH v2] scripts: add "git.orderfile" for ordering diff hunks by pathname patterns

[Eric Blake]

On 12/05/2016 04:24 AM, Laszlo Ersek wrote:

> 
> I picked the below lines from Eric's feedback.
> 
>>> +qapi-schema*.json
>>> +qapi/*.json
>>> +include/qapi/visitor.h
>>> +include/qapi/visitor-impl.h
>>> +scripts/qapi.py
>>> +scripts/*.py
>>> +*.h
>>> +qapi/qapi-visit-core.c
>>
>> is the exact order or qapi files that important?
>> I'd rather we stuck to simple wildcards without
>> special casing visitors etc.
> 
> Eric indicated this specific order was helpful for QAPI work.

In fact, what I would recommend is that we do the minimal file now, and
then subsequent series (such as if I have more QAPI work) can add their
further tweaks as part of the series where those tweaks aid review.  So
I'd be just fine omitting the visitor special-casing if it helps get the
patch in faster.

> I suggest we go ahead with this posting (or v1), then people can submit
> whatever improvements they deem fit, from their personal experience. I
> think the initial version should be minimal and non-controversial.
> 
> We've already spent a disproportionate amount of time discussing this item.

Indeed, especially for something that still requires manual
configuration from each developer to make use of it (it's a shame git
doesn't yet have automatic support for .gitorderfile, like it does for
.gitignore or .gitattributes).

-- 
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 00/10] tcg mips64 and mips r6 improvements

[Richard Henderson]

On 12/05/2016 01:41 AM, Jin Guojie wrote:
> -- Original --
> From:  "Richard Henderson";;
> Send time: Thursday, Dec 1, 2016 11:52 PM
> To: "Jin Guojie"; "qemu-devel";
> Cc: "Aurelien Jarno"; "James 
> Hogan";
> Subject:  Re: [PATCH v5 00/10] tcg mips64 and mips r6 improvements
> 
> Thanks a lot for Richard's careful review.
> I have some different opinions for discussion:
> 
>>  @@ -1233,7 +1235,8 @@ static void tcg_out_tlb_load(TCGContext *s, TCGReg 
>> base,
>>  -mask = (target_ulong)TARGET_PAGE_MASK | ((1 << a_bits) - 1);
>>  +mask = TARGET_PAGE_MASK | ((1 << a_bits) - 1);
>> You need the target_ulong cast here for mips64.
>> See patch ebb90a005da67147245cd38fb04a965a87a961b7.
> 
> Since mask is defined as a C type "target_ulong" at the beginning of the 
> function, 
> I guess an implicit type cast should be already done by the compiler.
> So your change is functionally the same with v5 patch. 
> To test my idea, I wrote a small case and compiled it on an x86_64 host:
> 
> main()
> {
>   int a_bits = 2;
>   int page_mask = 0xf000;/* X86 4KB page*/
>   unsigned int mask = page_mask | ((1 << a_bits) - 1);
>   unsigned long m = mask;
>   printf("mask=%lx\n", m);
> }
> 
> $ gcc a.c
> $ file a.out 
> a.out: Mach-O 64-bit executable x86_64
> $ ./a.out
> mask=f003

You misunderstand.  For a 64-bit guest, the result we're looking for is
0xf003.

(1) the type of a_bits is unsigned int, not int,
(2) which means the expression "page_mask | ((1 << a_bits) - 1)"
becomes unsigned int,
(3) which means that the assignment to mask gets truncated.

> The output is exactly an unsigned 32-bit quantity.
> I didn't see a wrong result generated. 
> Could you teach me where is my mistake?

For a 64-bit guest we need a 64-bit quantity.  For a 32-bit guest... we need to
match the load that we issue from cmp_off.  If use use LWU, then we need an
unsigned 32-bit quantity; if we use LW, then we need a signed 32-bit quantity.


r~

Re: [Qemu-devel] [PATCH RFC v2 2/6] replication: add shared-disk and shared-disk-id options

[Eric Blake]

On 12/05/2016 02:35 AM, zhanghailiang wrote:
> We use these two options to identify which disk is
> shared
> 
> Signed-off-by: zhanghailiang 
> Signed-off-by: Wen Congyang 
> Signed-off-by: Zhang Chen 
> ---
> v2:
> - add these two options for BlockdevOptionsReplication to support
>   qmp blockdev-add command.
> - fix a memory leak found by Changlong
> ---

> +++ b/qapi/block-core.json
> @@ -2232,12 +2232,19 @@
>  #  node who owns the replication node chain. Must not be given in
>  #  primary mode.
>  #
> +# @shared-disk-id: #optional The id of shared disk while in replication mode.
> +#
> +# @shared-disk: #optional To indicate whether or not a disk is shared by
> +#   primary VM and secondary VM.

Both of these need a '(since 2.9)' designation since they've missed 2.8,
and could probably also use documentation on the default value assumed
when the parameter is omitted.

> +#
>  # Since: 2.8
>  ##
>  { 'struct': 'BlockdevOptionsReplication',
>'base': 'BlockdevOptionsGenericFormat',
>'data': { 'mode': 'ReplicationMode',
> -'*top-id': 'str' } }
> +'*top-id': 'str',
> +'*shared-disk-id': 'str',
> +'*shared-disk': 'bool' } }
>  
>  ##
>  # @NFSTransport
> 

-- 
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 v2] scripts: add "git.orderfile" for ordering diff hunks by pathname patterns

[Eric Blake]

On 12/04/2016 11:33 AM, Michael S. Tsirkin wrote:

>> +++ b/scripts/git.orderfile
>> @@ -0,0 +1,20 @@
>> +# Apply this diff order to your git configuration with the command
>> +#
>> +#   git config diff.orderFile scripts/git.orderfile
>> +
>> +docs/*
> 
> do we need this now? .txt is just below.
> 
>> +*.txt

I still think we want docs/* first, at which point this *.txt line
currently only adds changes to qdict-test-data.txt (should that file be
moved to live under tests/ ?)

>> +configure
>> +GNUmakefile
>> +makefile
>> +Makefile*
>> +*.mak
> 
> do these rules apply in each directory?

My understanding is that git applies similar logic to these lines as it
does to .gitignore files: if not anchored, the pattern applies to any
file in any directory; or you can start with leading / to force the
match to be anchored to a specific directory.

> I think they do but then I don't think we should list directories
> below.
> 
>> +qapi-schema*.json
>> +qapi/*.json
>> +include/qapi/visitor.h
>> +include/qapi/visitor-impl.h
>> +scripts/qapi.py
>> +scripts/*.py
>> +*.h
>> +qapi/qapi-visit-core.c
> 
> is the exact order or qapi files that important?
> I'd rather we stuck to simple wildcards without
> special casing visitors etc.

On the v1 thread, I had pointed out that I had special-cased visitor
code in some of my patches, as an example, but I'm not sure that it is
special enough to be needed for everyone.  I'm fine doing one-off tweaks
for patch series where a particular file should come first, without
making the official file have to track all those one-off tweaks.


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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [RFC 11/15] s390x: Initialize default bus lists

[Eduardo Habkost]

On Mon, Dec 05, 2016 at 05:03:27PM +0100, Cornelia Huck wrote:
> On Mon, 5 Dec 2016 16:24:00 +0100
> David Hildenbrand  wrote:
> 
> > Am 22.11.2016 um 02:12 schrieb Eduardo Habkost:
> > > Populate the default_bus_types list for the s390x machines. This
> > > will allow qmp-machine-info.py to run in strict mode for s390x.
> > >
> > > Signed-off-by: Eduardo Habkost 
> > > ---
> > >  hw/s390x/s390-virtio-ccw.c | 6 ++
> > >  tests/qmp-machine-info.py  | 2 +-
> > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> > > index e340eab..2532fcb 100644
> > > --- a/hw/s390x/s390-virtio-ccw.c
> > > +++ b/hw/s390x/s390-virtio-ccw.c
> > > @@ -206,6 +206,12 @@ static void ccw_machine_class_init(ObjectClass *oc, 
> > > void *data)
> > >  mc->use_sclp = 1;
> > >  mc->max_cpus = 248;
> > >  mc->get_hotplug_handler = s390_get_hotplug_handler;
> > > +machine_class_add_default_bus(mc, TYPE_VIRTUAL_CSS_BUS);
> > > +machine_class_add_default_bus(mc, TYPE_PCI_BUS);
> > > +machine_class_add_default_bus(mc, TYPE_S390_PCI_BUS);
> > > +machine_class_add_default_bus(mc, "s390-sclp-events-bus");
> > > +machine_class_add_default_bus(mc, TYPE_VIRTIO_BUS);
> > > +
> > 
> > Maybe move and use
> > #define TYPE_SCLP_EVENTS_BUS "s390-sclp-events-bus"
> > 
> > (maybe this should then become TYPE_S390_SCLP_EVENTS_BUS ...)
> > 
> > ?
> > 
> 
> I'd ack/take such a patch.

Agreed. I will address that in final version. This version was
supposed to include a "//FIXME: use macro here" comment.

-- 
Eduardo

Re: [Qemu-devel] [PATCH v4 00/13] aio: experimental virtio-blk polling mode

[Karl Rister]

On 12/05/2016 08:56 AM, Stefan Hajnoczi wrote:


> Karl: do you have time to run a bigger suite of benchmarks to identify a
> reasonable default poll-max-ns value?  Both aio=native and aio=threads
> are important.
> 
> If there is a sweet spot that improves performance without pathological
> cases then we could even enable polling by default in QEMU.
> 
> Otherwise we'd just document the recommended best polling duration as a
> starting point for users.
> 

I have collected a baseline on the latest patches and am currently
collecting poll-max-ns=16384.  I can certainly throw in a few more
scenarios.  Do we want to stick with powers of 2 or some other strategy?

-- 
Karl Rister 

Re: [Qemu-devel] [RFC] target-ppc/fpu_helper.c: Use C99 code to speed up floating point unit

[Eric Blake]

On 12/03/2016 07:32 PM, Richard Henderson wrote:
> On 12/03/2016 09:07 AM, Programmingkid wrote:
>> Yes it would be. The commit message never stated why he wanted to switch
>> to floating point softfloat routines. These are my guesses:
> ...
>> - Different hosts produced different results and instead wanted
>> consistency?
> 
> This is the correct answer.  There are quite a lot of differences
> between floating point across different cpus.  Which means that it's
> actually rare that the host and guest floating point are exactly alike.

Or to frame it in different words:

qemu's goal is to be an architecturally-accurate emulator.  fma() has
specific C99 semantics (mapping to what IEEE floating point requires),
but USUALLY those semantics are not fully implemented in hardware;
rather, the hardware implements 90% and has specific flags or traps that
let the C library recognize when it has to manually perform the 10% of
special cases. But the special cases differ by hardware.

If all hardware implemented C99 semantics 100% of the time, then using
C99 to emulate hardware would be correct.  But since that is not true,
emulating C99 semantics instead of hardware semantics will be WRONG in
the special cases, and cause programs to misbehave under qemu where they
would behave correctly on bare metal. So the decision was that accurate
emulation (guaranteed by using soft-float operations that are tailored
to each hardware's specific quirks on the special cases) is more
important that speed (where the behavior can be different from hardware
on the special cases, even if that behavior is accurate to IEEE and C99
semantics).

If you can completely describe ALL special cases that ANY particular
platform has quirks for, and kick to softmmu for those cases while still
sticking to C99 for the common cases, you may have success. But at this
point, I doubt you have properly identified all the quirks where native
fma instructions differ from C99 fma() requirements, let alone the
differences in emulation that will be required to properly quirk all
possible floating point hardware.  And it's not even guaranteed that
identifying all the inputs that need special casing for various guest
hardware, before calling out to fma() in the remainder of cases, will
not slow things down (since fma() will have its own set of additional
quirking conditionals based on host hardware).

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



signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PULL 0/9] QAPI patches for 2016-12-05

[Markus Armbruster]

A set of fixes for MacOS from Eric, and a little documentation
polishing from Marc-André.  The diffstat for the .json may look scary,
but the generated code is identical.

The following changes since commit bd8ef5060dd2124a54578241da9a572faf7658dd:

  Merge remote-tracking branch 'dgibson/tags/ppc-for-2.8-20161201' into staging 
(2016-12-01 13:39:29 +)

are available in the git repository at:

  git://repo.or.cz/qemu/armbru.git tags/pull-qapi-2016-12-05

for you to fetch changes up to 5072f7b38b1b9b26b8fbe1a89086386a420aded8:

  qapi: add missing colon-ending for section name (2016-12-05 17:41:38 +0100)


QAPI patches for 2016-12-05


Eric Blake (3):
  qmp-event: Avoid qobject_from_jsonf("%"PRId64)
  test-qga: Avoid qobject_from_jsonv("%"PRId64)
  tests: Avoid qobject_from_jsonf("%"PRId64)

Marc-André Lureau (6):
  qga/schema: fix double-return in doc
  qapi: fix schema symbol sections
  qapi: fix missing symbol @prefix
  qapi: fix various symbols mismatch in documentation
  qapi: use one symbol per line
  qapi: add missing colon-ending for section name

 qapi-schema.json   | 346 +++--
 qapi/block-core.json   | 209 +++---
 qapi/block.json|  16 +-
 qapi/common.json   |  14 +-
 qapi/crypto.json   |  36 ++--
 qapi/event.json|  58 +++
 qapi/introspect.json   |  28 +--
 qapi/qmp-event.c   |  17 +-
 qapi/rocker.json   |   2 +-
 qapi/trace.json|   8 +-
 qga/qapi-schema.json   |  52 +++---
 tests/check-qjson.c|   6 +-
 tests/test-qga.c   |   7 +-
 tests/test-qobject-input-visitor.c |   5 +-
 14 files changed, 406 insertions(+), 398 deletions(-)

-- 
2.5.5

[Qemu-devel] [PULL 2/9] test-qga: Avoid qobject_from_jsonv("%"PRId64)

[Markus Armbruster]

From: Eric Blake 

The qobject_from_jsonv() function implements a pseudo-printf
language for creating a QObject; however, it is hard-coded to
only parse a subset of formats understood by -Wformat, and is
not a straight synonym to bare printf().  In particular, any
use of an int64_t integer works only if the system's
definition of PRId64 matches what the parser expects; which
works on glibc (%lld or %ld depending on 32- vs. 64-bit) and
mingw (%I64d), but not on Mac OS (%qd).  Rather than enhance
the parser, it is just as easy to use normal printf() for
this particular conversion, matching what is done elsewhere
in this file [1], which is safe in this instance because the
format does not contain any of the problematic differences
(bare '%' or the '%s' format).

The use of PRId64 for a variable named 'pid' is gross, but it
is a sad reality of the 64-bit mingw environment, which
mistakenly defines pid_t as a 64-bit type even though getpid()
returns 'int' on that platform [2].  Our definition of the
QGA GuestExec type defines 'pid' as a 64-bit entity, and we
can't tighten it to 'int32' unless the mingw header is fixed.
Using 'long long' instead of 'int64_t' just so that we can
stick with qobject_from_jsonv("%lld") instead of printf() is
not any prettier, since we may have later type churn anyways.

[1] see 'git grep -A2 strdup_printf tests/test-qga.c'
[2] https://bugzilla.redhat.com/show_bug.cgi?id=1397787

Reported by: G 3 
Signed-off-by: Eric Blake 
Message-Id: <1479922617-4400-3-git-send-email-ebl...@redhat.com>
Reviewed-by: Markus Armbruster 
Signed-off-by: Markus Armbruster 
---
 tests/test-qga.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tests/test-qga.c b/tests/test-qga.c
index 40af649..868b02a 100644
--- a/tests/test-qga.c
+++ b/tests/test-qga.c
@@ -837,6 +837,7 @@ static void test_qga_guest_exec(gconstpointer fix)
 int64_t pid, now, exitcode;
 gsize len;
 bool exited;
+char *cmd;
 
 /* exec 'echo foo bar' */
 ret = qmp_fd(fixture->fd, "{'execute': 'guest-exec', 'arguments': {"
@@ -851,9 +852,10 @@ static void test_qga_guest_exec(gconstpointer fix)
 
 /* wait for completion */
 now = g_get_monotonic_time();
+cmd = g_strdup_printf("{'execute': 'guest-exec-status',"
+  " 'arguments': { 'pid': %" PRId64 " } }", pid);
 do {
-ret = qmp_fd(fixture->fd, "{'execute': 'guest-exec-status',"
- " 'arguments': { 'pid': %" PRId64 "  } }", pid);
+ret = qmp_fd(fixture->fd, cmd);
 g_assert_nonnull(ret);
 val = qdict_get_qdict(ret, "return");
 exited = qdict_get_bool(val, "exited");
@@ -863,6 +865,7 @@ static void test_qga_guest_exec(gconstpointer fix)
 } while (!exited &&
  g_get_monotonic_time() < now + 5 * G_TIME_SPAN_SECOND);
 g_assert(exited);
+g_free(cmd);
 
 /* check stdout */
 exitcode = qdict_get_int(val, "exitcode");
-- 
2.5.5

[Qemu-devel] [PULL 4/9] qga/schema: fix double-return in doc

[Markus Armbruster]

From: Marc-André Lureau 

guest-get-memory-block-info documentation should have only one
"Returns:".

Signed-off-by: Marc-André Lureau 
Reviewed-by: Eric Blake 
Message-Id: <20161117155504.21843-3-marcandre.lur...@redhat.com>
Reviewed-by: Markus Armbruster 
Signed-off-by: Markus Armbruster 
---
 qga/qapi-schema.json | 1 -
 1 file changed, 1 deletion(-)

diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index c21f308..758803a 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -952,7 +952,6 @@
 #
 # Get information relating to guest memory blocks.
 #
-# Returns: memory block size in bytes.
 # Returns: @GuestMemoryBlockInfo
 #
 # Since 2.3
-- 
2.5.5

[Qemu-devel] [PULL 3/9] tests: Avoid qobject_from_jsonf("%"PRId64)

[Markus Armbruster]

From: Eric Blake 

The qobject_from_jsonf() function implements a pseudo-printf
language for creating a QObject; however, it is hard-coded to
only parse a subset of formats understood by -Wformat, and is
not a straight synonym to bare printf().  In particular, any
use of an int64_t integer works only if the system's
definition of PRId64 matches what the parser expects; which
works on glibc (%lld or %ld depending on 32- vs. 64-bit) and
mingw (%I64d), but not on Mac OS (%qd).  Rather than enhance
the parser, it is just as easy to force the use of int (where
the value is small enough) or long long instead of int64_t,
which we know always works.

This should cover all remaining testsuite uses of
qobject_from_json[fv]() that were trying to rely on PRId64,
although my proof for that was done by adding in asserts and
checking that 'make check' still passed, where such asserts
are inappropriate during hard freeze.  A later series in 2.9
may remove all dynamic JSON parsing, but that's a bigger task.

Reported by: G 3 
Signed-off-by: Eric Blake 
Message-Id: <1479922617-4400-4-git-send-email-ebl...@redhat.com>
Reviewed-by: Markus Armbruster 
[Rename value64 to value_ll]
Signed-off-by: Markus Armbruster 
---
 tests/check-qjson.c| 6 +++---
 tests/test-qobject-input-visitor.c | 5 +++--
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index 8595574..0b21a22 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -964,7 +964,7 @@ static void vararg_number(void)
 QInt *qint;
 QFloat *qfloat;
 int value = 0x2342;
-int64_t value64 = 0x2342342343LL;
+long long value_ll = 0x2342342343LL;
 double valuef = 2.323423423;
 
 obj = qobject_from_jsonf("%d", value);
@@ -976,12 +976,12 @@ static void vararg_number(void)
 
 QDECREF(qint);
 
-obj = qobject_from_jsonf("%" PRId64, value64);
+obj = qobject_from_jsonf("%lld", value_ll);
 g_assert(obj != NULL);
 g_assert(qobject_type(obj) == QTYPE_QINT);
 
 qint = qobject_to_qint(obj);
-g_assert(qint_get_int(qint) == value64);
+g_assert(qint_get_int(qint) == value_ll);
 
 QDECREF(qint);
 
diff --git a/tests/test-qobject-input-visitor.c 
b/tests/test-qobject-input-visitor.c
index 26c5012..945404a 100644
--- a/tests/test-qobject-input-visitor.c
+++ b/tests/test-qobject-input-visitor.c
@@ -83,10 +83,11 @@ static Visitor 
*visitor_input_test_init_raw(TestInputVisitorData *data,
 static void test_visitor_in_int(TestInputVisitorData *data,
 const void *unused)
 {
-int64_t res = 0, value = -42;
+int64_t res = 0;
+int value = -42;
 Visitor *v;
 
-v = visitor_input_test_init(data, "%" PRId64, value);
+v = visitor_input_test_init(data, "%d", value);
 
 visit_type_int(v, NULL, , _abort);
 g_assert_cmpint(res, ==, value);
-- 
2.5.5

[Qemu-devel] [PULL 1/9] qmp-event: Avoid qobject_from_jsonf("%"PRId64)

[Markus Armbruster]

From: Eric Blake 

The qobject_from_jsonf() function implements a pseudo-printf
language for creating a QObject; however, it is hard-coded to
only parse a subset of formats understood by -Wformat, and is
not a straight synonym to bare printf().  In particular, any
use of an int64_t integer works only if the system's
definition of PRId64 matches what the parser expects; which
works on glibc (%lld or %ld depending on 32- vs. 64-bit) and
mingw (%I64d), but not on Mac OS (%qd).  Rather than enhance
the parser, it is just as easy to use 'long long', which we
know always works.  There are few enough callers of
qobject_from_json[fv]() that it is easy to audit that this is
the only non-testsuite caller that was actually relying on
this particular conversion.

Reported by: G 3 
Signed-off-by: Eric Blake 
Message-Id: <1479922617-4400-2-git-send-email-ebl...@redhat.com>
Reviewed-by: Markus Armbruster 
[Cast tv.tv_sec, tv.tv_usec to long long for type correctness]
Signed-off-by: Markus Armbruster 
---
 qapi/qmp-event.c | 17 -
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/qapi/qmp-event.c b/qapi/qmp-event.c
index 8bba165..802ede4 100644
--- a/qapi/qmp-event.c
+++ b/qapi/qmp-event.c
@@ -35,21 +35,12 @@ static void timestamp_put(QDict *qdict)
 int err;
 QObject *obj;
 qemu_timeval tv;
-int64_t sec, usec;
 
 err = qemu_gettimeofday();
-if (err < 0) {
-/* Put -1 to indicate failure of getting host time */
-sec = -1;
-usec = -1;
-} else {
-sec = tv.tv_sec;
-usec = tv.tv_usec;
-}
-
-obj = qobject_from_jsonf("{ 'seconds': %" PRId64 ", "
- "'microseconds': %" PRId64 " }",
- sec, usec);
+/* Put -1 to indicate failure of getting host time */
+obj = qobject_from_jsonf("{ 'seconds': %lld, 'microseconds': %lld }",
+ err < 0 ? -1LL : (long long)tv.tv_sec,
+ err < 0 ? -1LL : (long long)tv.tv_usec);
 qdict_put_obj(qdict, "timestamp", obj);
 }
 
-- 
2.5.5

[Qemu-devel] [PULL 5/9] qapi: fix schema symbol sections

[Markus Armbruster]

From: Marc-André Lureau 

According to docs/qapi-code-gen.txt, there needs to be '##' to start a
and end a symbol section, that's also what the documentation parser
expects.

Signed-off-by: Marc-André Lureau 
Reviewed-by: Eric Blake 
Message-Id: <20161117155504.21843-5-marcandre.lur...@redhat.com>
Reviewed-by: Markus Armbruster 
Signed-off-by: Markus Armbruster 
---
 qapi-schema.json | 18 +-
 qapi/block-core.json |  1 +
 qga/qapi-schema.json |  3 +++
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index f3e9bfc..af488a8 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -65,6 +65,7 @@
 { 'enum': 'LostTickPolicy',
   'data': ['discard', 'delay', 'merge', 'slew' ] }
 
+##
 # @add_client
 #
 # Allow client connections for VNC, Spice and socket based
@@ -443,6 +444,7 @@
'cache-miss': 'int', 'cache-miss-rate': 'number',
'overflow': 'int' } }
 
+##
 # @MigrationStatus:
 #
 # An enumeration of migration status.
@@ -629,6 +631,7 @@
 ##
 { 'command': 'query-migrate-capabilities', 'returns':   
['MigrationCapabilityStatus']}
 
+##
 # @MigrationParameter
 #
 # Migration parameters enumeration
@@ -687,7 +690,7 @@
'tls-creds', 'tls-hostname', 'max-bandwidth',
'downtime-limit', 'x-checkpoint-delay' ] }
 
-#
+##
 # @migrate-set-parameters
 #
 # Set various migration parameters.  See MigrationParameters for details.
@@ -699,7 +702,7 @@
 { 'command': 'migrate-set-parameters', 'boxed': true,
   'data': 'MigrationParameters' }
 
-#
+##
 # @MigrationParameters
 #
 # Optional members can be omitted on input ('migrate-set-parameters')
@@ -797,6 +800,7 @@
 # command.
 #
 # Since: 2.5
+##
 { 'command': 'migrate-start-postcopy' }
 
 ##
@@ -2254,6 +2258,7 @@
 ##
 { 'command': 'migrate-incoming', 'data': {'uri': 'str' } }
 
+##
 # @xen-save-devices-state:
 #
 # Save the state of all devices to file. The RAM and the block devices
@@ -3484,6 +3489,7 @@
 'modelb': 'CpuModelInfo' },
   'returns': 'CpuModelBaselineInfo' }
 
+##
 # @AddfdInfo:
 #
 # Information about a file descriptor that was added to an fd set.
@@ -4532,14 +4538,16 @@
 ##
 { 'command': 'query-memory-devices', 'returns': ['MemoryDeviceInfo'] }
 
-## @ACPISlotType
+##
+# @ACPISlotType
 #
 # @DIMM: memory slot
 # @CPU: logical CPU slot (since 2.7)
-#
+##
 { 'enum': 'ACPISlotType', 'data': [ 'DIMM', 'CPU' ] }
 
-## @ACPIOSTInfo
+##
+# @ACPIOSTInfo
 #
 # OSPM Status Indication for a device
 # For description of possible values of @source and @status fields
diff --git a/qapi/block-core.json b/qapi/block-core.json
index c29bef7..39cdaba 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2809,6 +2809,7 @@
 'offset': 'int',
 'speed' : 'int' } }
 
+##
 # @PreallocMode
 #
 # Preallocation mode of QEMU image file
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 758803a..7a35267 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -837,6 +837,7 @@
 { 'command': 'guest-set-user-password',
   'data': { 'username': 'str', 'password': 'str', 'crypted': 'bool' } }
 
+##
 # @GuestMemoryBlock:
 #
 # @phys-index: Arbitrary guest-specific unique identifier of the MEMORY BLOCK.
@@ -936,6 +937,7 @@
   'data':{'mem-blks': ['GuestMemoryBlock'] },
   'returns': ['GuestMemoryBlockResponse'] }
 
+##
 # @GuestMemoryBlockInfo:
 #
 # @size: the size (in bytes) of the guest memory blocks,
@@ -959,6 +961,7 @@
 { 'command': 'guest-get-memory-block-info',
   'returns': 'GuestMemoryBlockInfo' }
 
+##
 # @GuestExecStatus:
 #
 # @exited: true if process has already terminated.
-- 
2.5.5

[Qemu-devel] [PULL 7/9] qapi: fix various symbols mismatch in documentation

[Markus Armbruster]

From: Marc-André Lureau 

There are various mismatch:
- invalid symbols
- section and member symbols mismatch
- enum or union values vs 'type'

The documentation parser catches all these cases.

Signed-off-by: Marc-André Lureau 
Message-Id: <20161117155504.21843-7-marcandre.lur...@redhat.com>
Reviewed-by: Markus Armbruster 
Signed-off-by: Markus Armbruster 
---
 qapi-schema.json | 20 +---
 qapi/block-core.json |  4 
 qapi/common.json |  6 +++---
 qapi/rocker.json |  2 +-
 qga/qapi-schema.json |  6 +++---
 5 files changed, 16 insertions(+), 22 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 824d205..9c0b46a 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -695,8 +695,6 @@
 #
 # Set various migration parameters.  See MigrationParameters for details.
 #
-# @x-checkpoint-delay: the delay time between two checkpoints. (Since 2.8)
-#
 # Since: 2.4
 ##
 { 'command': 'migrate-set-parameters', 'boxed': true,
@@ -1173,7 +1171,7 @@
'*service': 'str', '*auth': 'str', '*clients': ['VncClientInfo']} }
 
 ##
-# @VncPriAuth:
+# @VncPrimaryAuth:
 #
 # vnc primary authentication method.
 #
@@ -3176,7 +3174,7 @@
 #
 # @alias: #optional an alias for the machine name
 #
-# @default: #optional whether the machine is default
+# @is-default: #optional whether the machine is default
 #
 # @cpu-max: maximum number of CPUs supported by the machine type
 #   (since 1.5.0)
@@ -3729,7 +3727,6 @@
 #
 # @device: The name of the special file for the device,
 #  i.e. /dev/ttyS0 on Unix or COM1: on Windows
-# @type: What kind of device this is.
 #
 # Since: 1.4
 ##
@@ -3994,7 +3991,7 @@
 #
 # A union referencing different TPM backend types' configuration options
 #
-# @passthrough: The configuration options for the TPM passthrough type
+# @type: 'passthrough' The configuration options for the TPM passthrough type
 #
 # Since: 1.5
 ##
@@ -4002,7 +3999,7 @@
'data': { 'passthrough' : 'TPMPassthroughOptions' } }
 
 ##
-# @TpmInfo:
+# @TPMInfo:
 #
 # Information about the TPM
 #
@@ -4346,10 +4343,11 @@
 #
 # Input event union.
 #
-# @key: Input event of Keyboard
-# @btn: Input event of pointer buttons
-# @rel: Input event of relative pointer motion
-# @abs: Input event of absolute pointer motion
+# @type: the input type, one of:
+#  - 'key': Input event of Keyboard
+#  - 'btn': Input event of pointer buttons
+#  - 'rel': Input event of relative pointer motion
+#  - 'abs': Input event of absolute pointer motion
 #
 # Since: 2.0
 ##
diff --git a/qapi/block-core.json b/qapi/block-core.json
index d98fe73..33bc93a 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -2160,10 +2160,6 @@
 #
 # @type:   Transport type used for gluster connection
 #
-# @unix:   socket file
-#
-# @tcp:host address and port number
-#
 # This is similar to SocketAddress, only distinction:
 #
 # 1. GlusterServer is a flat union, SocketAddress is a simple union.
diff --git a/qapi/common.json b/qapi/common.json
index 9353a7b..6987100 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -34,11 +34,11 @@
 #
 # A three-part version number.
 #
-# @qemu.major:  The major version number.
+# @major:  The major version number.
 #
-# @qemu.minor:  The minor version number.
+# @minor:  The minor version number.
 #
-# @qemu.micro:  The micro version number.
+# @micro:  The micro version number.
 #
 # Since: 2.4
 ##
diff --git a/qapi/rocker.json b/qapi/rocker.json
index 2fe7fdf..ace2776 100644
--- a/qapi/rocker.json
+++ b/qapi/rocker.json
@@ -1,5 +1,5 @@
 ##
-# @Rocker:
+# @RockerSwitch:
 #
 # Rocker switch information.
 #
diff --git a/qga/qapi-schema.json b/qga/qapi-schema.json
index 7a35267..ad0a31d 100644
--- a/qga/qapi-schema.json
+++ b/qga/qapi-schema.json
@@ -203,7 +203,7 @@
 #
 # Open a file in the guest and retrieve a file handle for it
 #
-# @filepath: Full path to the file in the guest to open.
+# @path: Full path to the file in the guest to open.
 #
 # @mode: #optional open mode, as per fopen(), "r" is the default.
 #
@@ -378,7 +378,7 @@
   'data': { 'handle': 'int' } }
 
 ##
-# @GuestFsFreezeStatus
+# @GuestFsfreezeStatus
 #
 # An enumeration of filesystem freeze states
 #
@@ -770,7 +770,7 @@
 # @GuestDiskAddress:
 #
 # @pci-controller: controller's PCI address
-# @type: bus type
+# @bus-type: bus type
 # @bus: bus id
 # @target: target id
 # @unit: unit id
-- 
2.5.5

[Qemu-devel] [PULL 6/9] qapi: fix missing symbol @prefix

[Markus Armbruster]

From: Marc-André Lureau 

Signed-off-by: Marc-André Lureau 
Reviewed-by: Eric Blake 
Message-Id: <20161117155504.21843-6-marcandre.lur...@redhat.com>
Reviewed-by: Markus Armbruster 
Signed-off-by: Markus Armbruster 
---
 qapi-schema.json |  4 ++--
 qapi/block-core.json |  4 ++--
 qapi/crypto.json | 36 ++--
 3 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index af488a8..824d205 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -4652,7 +4652,7 @@
 { 'include': 'qapi/rocker.json' }
 
 ##
-# ReplayMode:
+# @ReplayMode:
 #
 # Mode of the replay subsystem.
 #
@@ -4720,7 +4720,7 @@
 { 'command': 'query-gic-capabilities', 'returns': ['GICCapability'] }
 
 ##
-# CpuInstanceProperties
+# @CpuInstanceProperties
 #
 # List of properties to be used for hotplugging a CPU instance,
 # it should be passed by management with device_add command when
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 39cdaba..d98fe73 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1144,7 +1144,7 @@
   'data': 'DriveMirror' }
 
 ##
-# DriveMirror
+# @DriveMirror
 #
 # A set of parameters describing drive mirror setup.
 #
@@ -1368,7 +1368,7 @@
   'data': 'BlockIOThrottle' }
 
 ##
-# BlockIOThrottle
+# @BlockIOThrottle
 #
 # A set of parameters describing block throttling.
 #
diff --git a/qapi/crypto.json b/qapi/crypto.json
index 5c9d7d4..15d296e 100644
--- a/qapi/crypto.json
+++ b/qapi/crypto.json
@@ -3,7 +3,7 @@
 # QAPI crypto definitions
 
 ##
-# QCryptoTLSCredsEndpoint:
+# @QCryptoTLSCredsEndpoint:
 #
 # The type of network endpoint that will be using the credentials.
 # Most types of credential require different setup / structures
@@ -22,7 +22,7 @@
 
 
 ##
-# QCryptoSecretFormat:
+# @QCryptoSecretFormat:
 #
 # The data format that the secret is provided in
 #
@@ -36,7 +36,7 @@
 
 
 ##
-# QCryptoHashAlgorithm:
+# @QCryptoHashAlgorithm:
 #
 # The supported algorithms for computing content digests
 #
@@ -55,7 +55,7 @@
 
 
 ##
-# QCryptoCipherAlgorithm:
+# @QCryptoCipherAlgorithm:
 #
 # The supported algorithms for content encryption ciphers
 #
@@ -82,7 +82,7 @@
 
 
 ##
-# QCryptoCipherMode:
+# @QCryptoCipherMode:
 #
 # The supported modes for content encryption ciphers
 #
@@ -98,7 +98,7 @@
 
 
 ##
-# QCryptoIVGenAlgorithm:
+# @QCryptoIVGenAlgorithm:
 #
 # The supported algorithms for generating initialization
 # vectors for full disk encryption. The 'plain' generator
@@ -116,7 +116,7 @@
   'data': ['plain', 'plain64', 'essiv']}
 
 ##
-# QCryptoBlockFormat:
+# @QCryptoBlockFormat:
 #
 # The supported full disk encryption formats
 #
@@ -131,7 +131,7 @@
   'data': ['qcow', 'luks']}
 
 ##
-# QCryptoBlockOptionsBase:
+# @QCryptoBlockOptionsBase:
 #
 # The common options that apply to all full disk
 # encryption formats
@@ -144,7 +144,7 @@
   'data': { 'format': 'QCryptoBlockFormat' }}
 
 ##
-# QCryptoBlockOptionsQCow:
+# @QCryptoBlockOptionsQCow:
 #
 # The options that apply to QCow/QCow2 AES-CBC encryption format
 #
@@ -158,7 +158,7 @@
   'data': { '*key-secret': 'str' }}
 
 ##
-# QCryptoBlockOptionsLUKS:
+# @QCryptoBlockOptionsLUKS:
 #
 # The options that apply to LUKS encryption format
 #
@@ -172,7 +172,7 @@
 
 
 ##
-# QCryptoBlockCreateOptionsLUKS:
+# @QCryptoBlockCreateOptionsLUKS:
 #
 # The options that apply to LUKS encryption format initialization
 #
@@ -202,7 +202,7 @@
 
 
 ##
-# QCryptoBlockOpenOptions:
+# @QCryptoBlockOpenOptions:
 #
 # The options that are available for all encryption formats
 # when opening an existing volume
@@ -217,7 +217,7 @@
 
 
 ##
-# QCryptoBlockCreateOptions:
+# @QCryptoBlockCreateOptions:
 #
 # The options that are available for all encryption formats
 # when initializing a new volume
@@ -232,7 +232,7 @@
 
 
 ##
-# QCryptoBlockInfoBase:
+# @QCryptoBlockInfoBase:
 #
 # The common information that applies to all full disk
 # encryption formats
@@ -246,7 +246,7 @@
 
 
 ##
-# QCryptoBlockInfoLUKSSlot:
+# @QCryptoBlockInfoLUKSSlot:
 #
 # Information about the LUKS block encryption key
 # slot options
@@ -266,7 +266,7 @@
 
 
 ##
-# QCryptoBlockInfoLUKS:
+# @QCryptoBlockInfoLUKS:
 #
 # Information about the LUKS block encryption options
 #
@@ -294,7 +294,7 @@
'slots': [ 'QCryptoBlockInfoLUKSSlot' ] }}
 
 ##
-# QCryptoBlockInfoQCow:
+# @QCryptoBlockInfoQCow:
 #
 # Information about the QCow block encryption options
 #
@@ -305,7 +305,7 @@
 
 
 ##
-# QCryptoBlockInfo:
+# @QCryptoBlockInfo:
 #
 # Information about the block encryption options
 #
-- 
2.5.5

[Qemu-devel] [PULL 9/9] qapi: add missing colon-ending for section name

[Markus Armbruster]

From: Marc-André Lureau 

The documentation parser we are going to add expects a section name to
end with ':', otherwise the comment is treated as free-form text body.

Signed-off-by: Marc-André Lureau 
Message-Id: <20161117155504.21843-9-marcandre.lur...@redhat.com>
Reviewed-by: Markus Armbruster 
Signed-off-by: Markus Armbruster 
---
 qapi-schema.json | 300 +--
 qapi/block-core.json | 196 -
 qapi/block.json  |  16 +--
 qapi/common.json |   8 +-
 qapi/event.json  |  58 +-
 qapi/introspect.json |  28 ++---
 qapi/trace.json  |   8 +-
 qga/qapi-schema.json |  44 
 8 files changed, 329 insertions(+), 329 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 918a79f..a0d3b5d 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -66,7 +66,7 @@
   'data': ['discard', 'delay', 'merge', 'slew' ] }
 
 ##
-# @add_client
+# @add_client:
 #
 # Allow client connections for VNC, Spice and socket based
 # character devices to be passed in to QEMU via SCM_RIGHTS.
@@ -97,7 +97,7 @@
 #
 # @name: #optional The name of the guest
 #
-# Since 0.14.0
+# Since: 0.14.0
 ##
 { 'struct': 'NameInfo', 'data': {'*name': 'str'} }
 
@@ -108,7 +108,7 @@
 #
 # Returns: @NameInfo of the guest
 #
-# Since 0.14.0
+# Since: 0.14.0
 ##
 { 'command': 'query-name', 'returns': 'NameInfo' }
 
@@ -137,7 +137,7 @@
 { 'command': 'query-kvm', 'returns': 'KvmInfo' }
 
 ##
-# @RunState
+# @RunState:
 #
 # An enumeration of VM run states.
 #
@@ -236,7 +236,7 @@
 #
 # Returns: The @UuidInfo for the guest
 #
-# Since 0.14.0
+# Since: 0.14.0
 ##
 { 'command': 'query-uuid', 'returns': 'UuidInfo' }
 
@@ -383,7 +383,7 @@
 { 'command': 'query-events', 'returns': ['EventInfo'] }
 
 ##
-# @MigrationStats
+# @MigrationStats:
 #
 # Detailed migration status.
 #
@@ -397,7 +397,7 @@
 #
 # @skipped: number of skipped zero pages (since 1.5)
 #
-# @normal : number of normal pages (since 1.2)
+# @normal: number of normal pages (since 1.2)
 #
 # @normal-bytes: number of normal bytes sent (since 1.2)
 #
@@ -421,7 +421,7 @@
'postcopy-requests' : 'int' } }
 
 ##
-# @XBZRLECacheStats
+# @XBZRLECacheStats:
 #
 # Detailed XBZRLE migration cache statistics
 #
@@ -476,7 +476,7 @@
 'active', 'postcopy-active', 'completed', 'failed', 'colo' ] }
 
 ##
-# @MigrationInfo
+# @MigrationInfo:
 #
 # Information about current migration process.
 #
@@ -536,7 +536,7 @@
'*error-desc': 'str'} }
 
 ##
-# @query-migrate
+# @query-migrate:
 #
 # Returns information about current migration process.
 #
@@ -547,7 +547,7 @@
 { 'command': 'query-migrate', 'returns': 'MigrationInfo' }
 
 ##
-# @MigrationCapability
+# @MigrationCapability:
 #
 # Migration capabilities enumeration
 #
@@ -595,7 +595,7 @@
'compress', 'events', 'postcopy-ram', 'x-colo'] }
 
 ##
-# @MigrationCapabilityStatus
+# @MigrationCapabilityStatus:
 #
 # Migration capability information
 #
@@ -609,7 +609,7 @@
   'data': { 'capability' : 'MigrationCapability', 'state' : 'bool' } }
 
 ##
-# @migrate-set-capabilities
+# @migrate-set-capabilities:
 #
 # Enable/Disable the following migration capabilities (like xbzrle)
 #
@@ -621,7 +621,7 @@
   'data': { 'capabilities': ['MigrationCapabilityStatus'] } }
 
 ##
-# @query-migrate-capabilities
+# @query-migrate-capabilities:
 #
 # Returns information about the current migration capabilities status
 #
@@ -632,7 +632,7 @@
 { 'command': 'query-migrate-capabilities', 'returns':   
['MigrationCapabilityStatus']}
 
 ##
-# @MigrationParameter
+# @MigrationParameter:
 #
 # Migration parameters enumeration
 #
@@ -691,7 +691,7 @@
'downtime-limit', 'x-checkpoint-delay' ] }
 
 ##
-# @migrate-set-parameters
+# @migrate-set-parameters:
 #
 # Set various migration parameters.  See MigrationParameters for details.
 #
@@ -701,7 +701,7 @@
   'data': 'MigrationParameters' }
 
 ##
-# @MigrationParameters
+# @MigrationParameters:
 #
 # Optional members can be omitted on input ('migrate-set-parameters')
 # but most members will always be present on output
@@ -760,7 +760,7 @@
 '*x-checkpoint-delay': 'int'} }
 
 ##
-# @query-migrate-parameters
+# @query-migrate-parameters:
 #
 # Returns information about the current migration parameters
 #
@@ -772,7 +772,7 @@
   'returns': 'MigrationParameters' }
 
 ##
-# @client_migrate_info
+# @client_migrate_info:
 #
 # Set migration information for remote display.  This makes the server
 # ask the client to automatically reconnect using the new parameters
@@ -791,7 +791,7 @@
 '*tls-port': 'int', '*cert-subject': 'str' } }
 
 ##
-# @migrate-start-postcopy
+# @migrate-start-postcopy:
 #
 # Followup to a migration command to switch the migration to postcopy mode.
 # The postcopy-ram capability must be set before the original migration
@@ -802,7 +802,7 @@
 { 'command': 

Re: [Qemu-devel] [PATCH v5 00/17] qapi doc generation (whole version, squashed)

[Markus Armbruster]

Marc-André Lureau  writes:

> Add a qapi2texi script to generate the documentation from the qapi
> schemas.
>
> The 15th patch in this series is a squashed version of the
> documentation move from qmp-commands.txt to the schemas. The whole
> version (not sent on the ML to avoid spamming) is in the following git
> branch: https://github.com/elmarco/qemu/commits/qapi-doc
>
> PDF preview:
> https://fedorapeople.org/~elmarco/qemu-qmp-ref.pdf

PATCH 2,4-8 applied to qapi-next for 2.8.

PATCH 1,10 are ready, but I'm punting them to 2.9 because they affect
generated code.  Probably overcautious.

[Qemu-devel] [PULL 8/9] qapi: use one symbol per line

[Markus Armbruster]

From: Marc-André Lureau 

The documentation parser we are going to add only handles a single
symbol per line.

Signed-off-by: Marc-André Lureau 
Message-Id: <20161117155504.21843-8-marcandre.lur...@redhat.com>
Reviewed-by: Markus Armbruster 
Signed-off-by: Markus Armbruster 
---
 qapi-schema.json | 10 +++---
 qapi/block-core.json |  8 ++--
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/qapi-schema.json b/qapi-schema.json
index 9c0b46a..918a79f 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3612,15 +3612,19 @@
 ##
 # @QKeyCode:
 #
+# @unmapped: since 2.0
+# @pause: since 2.0
+# @ro: since 2.4
+# @kp_comma: since 2.4
+# @kp_equals: since 2.6
+# @power: since 2.6
+#
 # An enumeration of key name.
 #
 # This is used by the send-key command.
 #
 # Since: 1.3.0
 #
-# 'unmapped' and 'pause' since 2.0
-# 'ro' and 'kp_comma' since 2.4
-# 'kp_equals' and 'power' since 2.6
 ##
 { 'enum': 'QKeyCode',
   'data': [ 'unmapped',
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 33bc93a..96d0859 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1712,9 +1712,13 @@
 #
 # Drivers that are supported in block device operations.
 #
-# @host_device, @host_cdrom: Since 2.1
+# @host_device: Since 2.1
+# @host_cdrom: Since 2.1
 # @gluster: Since 2.7
-# @nbd, @nfs, @replication, @ssh: Since 2.8
+# @nbd: Since 2.8
+# @nfs: Since 2.8
+# @replication: Since 2.8
+# @ssh: Since 2.8
 #
 # Since: 2.0
 ##
-- 
2.5.5

Re: [Qemu-devel] [PATCH for-2.8] qdev: apply global properties in reverse order

[Eduardo Habkost]

On Mon, Dec 05, 2016 at 04:42:00PM +0100, Cornelia Huck wrote:
> On Mon, 05 Dec 2016 16:21:22 +0100
> Greg Kurz  wrote:
> 
> > The current code recursively applies global properties from child up to
> > parent. So, if you have:
> > 
> > -global virtio-pci.disable-modern=on
> > -global virtio-blk-pci.disable-modern=off
> > 
> > Then the default value of disable-modern for a virtio-blk-pci device is on,
> > which looks wrong from an OOP perspective.
> > 
> > This patch reverses the logic, so that a child property always prevail.
> 
> This sounds reasonable...
> 
> > 
> > This fixes a subtle bug that got introduced in 2.7 with commit "9a4c0e220d8a
> > hw/virtio-pci: fix virtio behaviour" for older (< 2.7) machine types: the
> > HW_COMPAT_2_6 macro contains global virtio-pci.disable-* properties which
> > would silently override global properties passed on the command line for
> > virtio subtypes.
> > 
> > Signed-off-by: Greg Kurz 
> > ---
> > 
> > AFAIK, libvirt's XML doesn't know about modern/legacy modes for virtio
> > devices. Early adopters of virtio 1.0 had to rely on the 
> > tag to pass global properties to QEMU. This patch ensures that XML files
> > used with older machine types remain valid with newer versions of QEMU.
> > 
> > FWIW I guess it could help to have this fix in 2.8, and also probably in
> > 2.7.1.
> 
> ...but I'm a bit worried about doing that change this late in the
> cycle, as we may introduce subtle changes for other configurations. At
> the very least, we should look over the existing backwards compat
> properties (I'll look at those I'm familiar with).

This patch would change the behavior for:
 -global virtio-blk-pci.disable-modern=on
 -global virtio-pci.disable-modern=off

And I am not sure the new behavior would be correct. Shouldn't we
apply the properties in the order specified in the command-line?

On either case, changing the semantics of the command-line can
break existing configurations. Let's do it more carefully in the
2.9 cycle, and fix the existing bug by changing the HW_COMPAT_*
macros?

> 
> > 
> > Please advise.
> > 
> >  hw/core/qdev-properties.c |   11 ++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c
> > index 2a8276806721..1345f489d6b1 100644
> > --- a/hw/core/qdev-properties.c
> > +++ b/hw/core/qdev-properties.c
> > @@ -1119,11 +1119,20 @@ static void 
> > qdev_prop_set_globals_for_type(DeviceState *dev,
> >  void qdev_prop_set_globals(DeviceState *dev)
> >  {
> >  ObjectClass *class = object_get_class(OBJECT(dev));
> > +GSList *class_list = NULL;
> > 
> >  do {
> > -qdev_prop_set_globals_for_type(dev, object_class_get_name(class));
> > +class_list = g_slist_prepend(class_list, class);
> >  class = object_class_get_parent(class);
> >  } while (class);
> > +
> > +do {
> > +GSList *head = class_list;
> > +
> > +qdev_prop_set_globals_for_type(dev, 
> > object_class_get_name(head->data));
> > +class_list = g_slist_next(head);
> > +g_slist_free_1(head);
> > +} while (class_list);
> >  }
> > 
> >  /* --- 64bit unsigned int 'size' type --- */
> > 
> 
> It is a bit unfortunate that we need a double loop here, but I don't
> see any good alternative.
> 
> 

-- 
Eduardo

Re: [Qemu-devel] [PATCH] nbd: Implement NBD_CMD_HAS_ZERO_INIT

[Eric Blake]

On 12/05/2016 05:09 AM, Stefan Hajnoczi wrote:
> On Sun, Dec 04, 2016 at 11:44:57PM +, Yi Fang wrote:
>> NBD client has not implemented callback for .bdrv_has_zero_init. So
>> bdrv_has_zero_init always returns 0 when doing non-shared storage
>> migration.
>> This patch implemented NBD_CMD_HAS_ZERO_INIT and will avoid unnecessary
>> set-dirty.
> 
> Please link to the NBD spec where this new command is defined.  Has it
> already been accepted by the NBD community?

No. This is the first I've seen of such a proposal.

> 
> I think this is a weird command because it's information that doesn't
> change over the lifetime of an NBD session.  Your patch sends a command
> and waits for the reply every time has_zero_init() is queried.
> 
> Is there a better place to put this information in the NBD protocols,
> like some export information that is retried during connection?  (I
> haven't checked the protocol.)  It seems weird to send a command and
> wait for the response.

Agreed - this patch is wrong. Even if you DO get the NBD community to
buy into this, it should be done as a new NBD_FLAG_* sent once up-front
during handshake phase in reply to NBD_OPT_EXPORT_NAME/NBD_OPT_GO, and
NOT a new command that can be arbitrarily invoked multiple times during
transmission phase.

Is the goal of the flag for the server to be able to advertise to the
client that upon initial connection, the server asserts that the volume
currently reads as all zeroes (and therefore the client can optimize by
not writing zeroes)?

Do you need me to help re-write this into a proposal to the NBD
community that might stand a chance of being accepted?

-- 
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 v2 for-2.8 0/4] Fix MacOS runtime failure of qobject_from_jsonf()

[Markus Armbruster]

Eric Blake  writes:

> programmingk...@gmail.com[*] reported a runtime failure on a
> 32-bit Mac OS compilation, where "%"PRId64 expands to "%qd".
> Fortunately, we had very few spots that were relying on our
> pseudo-printf JSON parsing of int64_t numbers, so it was
> easier to just convert callers to stick to safer %lld.
>
> The remaining uses of pseudo-printf handling are more complex;
> there are only 3 users in the released codebased, but LOTS of
> users in the testsuite (via wrapper functions like qmp()); I
> will be posting a followup series that rips out the remaining
> uses of dynamic JSON, but it will be 2.9 material, while
> these (first three) patches qualify for 2.8.  The fourth patch
> is RFC; not intended to be applied now, but shows how I tested
> patch 3/4; it will probably reappear in the later 2.9 series.
>
> [*] git log shows the name John, but the particular email that
> sparked this only stated the non-descript name 'G 3', which
> makes it a bit hard for me to know which form is preferred
> when lending credit.

PATCH 1-3 applied to qapi-next for 2.8 with minor touch-ups.  Thanks!

Re: [Qemu-devel] [PATCH for-2.9 1/3] crypto: add standard des support

[Daniel P. Berrange]

On Mon, Dec 05, 2016 at 09:29:59AM +, Gonglei (Arei) wrote:
> >
> > >  switch (alg) {
> > > +case QCRYPTO_CIPHER_ALG_DES:
> > >  case QCRYPTO_CIPHER_ALG_DES_RFB:
> > >  case QCRYPTO_CIPHER_ALG_AES_128:
> > >  case QCRYPTO_CIPHER_ALG_AES_192:
> > > @@ -256,11 +257,17 @@ QCryptoCipher
> > *qcrypto_cipher_new(QCryptoCipherAlgorithm alg,
> > >  ctx = g_new0(QCryptoCipherNettle, 1);
> > >
> > >  switch (alg) {
> > > +case QCRYPTO_CIPHER_ALG_DES:
> > >  case QCRYPTO_CIPHER_ALG_DES_RFB:
> > >  ctx->ctx = g_new0(struct des_ctx, 1);
> > > -rfbkey = qcrypto_cipher_munge_des_rfb_key(key, nkey);
> > > -des_set_key(ctx->ctx, rfbkey);
> > > -g_free(rfbkey);
> > > +
> > > +if (alg == QCRYPTO_CIPHER_ALG_DES_RFB) {
> > > +rfbkey = qcrypto_cipher_munge_des_rfb_key(key, nkey);
> > > +des_set_key(ctx->ctx, rfbkey);
> > > +g_free(rfbkey);
> > > +} else {
> > > +des_set_key(ctx->ctx, key);
> > > +}
> > >
> > >  ctx->alg_encrypt_native = des_encrypt_native;
> > >  ctx->alg_decrypt_native = des_decrypt_native;
> > > diff --git a/crypto/cipher.c b/crypto/cipher.c
> > > index a9bca41..00d9682 100644
> > > --- a/crypto/cipher.c
> > > +++ b/crypto/cipher.c
> > > @@ -27,6 +27,7 @@ static size_t
> > alg_key_len[QCRYPTO_CIPHER_ALG__MAX] = {
> > >  [QCRYPTO_CIPHER_ALG_AES_128] = 16,
> > >  [QCRYPTO_CIPHER_ALG_AES_192] = 24,
> > >  [QCRYPTO_CIPHER_ALG_AES_256] = 32,
> > > +[QCRYPTO_CIPHER_ALG_DES] = 8,
> > >  [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
> > >  [QCRYPTO_CIPHER_ALG_CAST5_128] = 16,
> > >  [QCRYPTO_CIPHER_ALG_SERPENT_128] = 16,
> > > @@ -41,6 +42,7 @@ static size_t
> > alg_block_len[QCRYPTO_CIPHER_ALG__MAX] = {
> > >  [QCRYPTO_CIPHER_ALG_AES_128] = 16,
> > >  [QCRYPTO_CIPHER_ALG_AES_192] = 16,
> > >  [QCRYPTO_CIPHER_ALG_AES_256] = 16,
> > > +[QCRYPTO_CIPHER_ALG_DES] = 8,
> > >  [QCRYPTO_CIPHER_ALG_DES_RFB] = 8,
> > >  [QCRYPTO_CIPHER_ALG_CAST5_128] = 8,
> > >  [QCRYPTO_CIPHER_ALG_SERPENT_128] = 16,
> > > @@ -107,7 +109,8 @@
> > qcrypto_cipher_validate_key_length(QCryptoCipherAlgorithm alg,
> > >  }
> > >
> > >  if (mode == QCRYPTO_CIPHER_MODE_XTS) {
> > > -if (alg == QCRYPTO_CIPHER_ALG_DES_RFB) {
> > > +if (alg == QCRYPTO_CIPHER_ALG_DES_RFB
> > > +|| alg == QCRYPTO_CIPHER_ALG_DES) {
> > >  error_setg(errp, "XTS mode not compatible with DES-RFB");
> > >  return false;
> > >  }
> > > diff --git a/qapi/crypto.json b/qapi/crypto.json
> > > index 5c9d7d4..d403ab9 100644
> > > --- a/qapi/crypto.json
> > > +++ b/qapi/crypto.json
> > > @@ -75,7 +75,7 @@
> > >  { 'enum': 'QCryptoCipherAlgorithm',
> > >'prefix': 'QCRYPTO_CIPHER_ALG',
> > >'data': ['aes-128', 'aes-192', 'aes-256',
> > > -   'des-rfb',
> > > +   'des-rfb', 'des',
> > 
> > Can we call this '3des' to make it clear that this is Triple-DES and not
> > the single-DES (which des-rfb is)
> > 
> Actually the current des is not triple-DES, just the single-DES, and des-rfb 
> in QEMU is just a variant of
> single DES, which change the standard key by calling 
> qcrypto_cipher_munge_des_rfb_key().
> 
> I think we can add the 3des support as well in the next step.
> 
> The current single-DES in the patch set is ok to me. :)

Per my othre reply in this thread, I don't think we should be supporting
single-DES at all in QEMU / cryptodev. So IMHO, the correct fix is to
remove the single-DES support from cryptodev entirely

Regards,
Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://entangle-photo.org   -o-http://search.cpan.org/~danberr/ :|