Re: [Qemu-devel] [PATCHv3 2/4] Split serial-isa into its own config option

[Thomas Huth]

On 15.01.2016 13:21, David Gibson wrote:
> At present, the core device model code for 8250-like serial ports
> (serial.c) and the code for serial ports attached to ISA-style legacy IO
> (serial-isa.c) are both controlled by the CONFIG_SERIAL variable.
> 
> There are lots and lots of embedded platforms that have 8250-like serial
> ports but have never had anything resembling ISA legacy IO.  Therefore,
> split serial-isa into its own CONFIG_SERIAL_ISA option so it can be
> disabled for platforms where it's not appropriate.
> 
> For now, I enabled CONFIG_SERIAL_ISA in every default-config where
> CONFIG_SERIAL is enabled, excepting microblaze, moxie, or32, and
> xtensa.  As best as I can tell, those platforms never used legacy ISA,
> and also don't include PCI support (which would allow connection of a
> PCI->ISA bridge and/or a southbridge including legacy ISA serial
> ports).
> 
> Signed-off-by: David Gibson 
> ---
>  default-configs/alpha-softmmu.mak| 1 +
>  default-configs/arm-softmmu.mak  | 1 +
>  default-configs/i386-softmmu.mak | 1 +
>  default-configs/mips-softmmu.mak | 1 +
>  default-configs/mips64-softmmu.mak   | 1 +
>  default-configs/mips64el-softmmu.mak | 1 +
>  default-configs/mipsel-softmmu.mak   | 1 +
>  default-configs/ppc-softmmu.mak  | 1 +
>  default-configs/ppc64-softmmu.mak| 1 +
>  default-configs/ppcemb-softmmu.mak   | 1 +
>  default-configs/sh4-softmmu.mak  | 1 +
>  default-configs/sh4eb-softmmu.mak| 1 +
>  default-configs/sparc64-softmmu.mak  | 1 +
>  default-configs/x86_64-softmmu.mak   | 1 +
>  hw/char/Makefile.objs| 3 ++-
>  15 files changed, 16 insertions(+), 1 deletion(-)
...
> diff --git a/default-configs/ppc-softmmu.mak b/default-configs/ppc-softmmu.mak
> index d4d0f9b..13eb94f 100644
> --- a/default-configs/ppc-softmmu.mak
> +++ b/default-configs/ppc-softmmu.mak
> @@ -45,5 +45,6 @@ CONFIG_PLATFORM_BUS=y
>  CONFIG_ETSEC=y
>  CONFIG_LIBDECNUMBER=y
>  # For PReP
> +CONFIG_SERIAL_ISA=y
>  CONFIG_MC146818RTC=y
>  CONFIG_ISA_TESTDEV=y
> diff --git a/default-configs/ppc64-softmmu.mak 
> b/default-configs/ppc64-softmmu.mak
> index 70a89d1..3e243fd 100644
> --- a/default-configs/ppc64-softmmu.mak
> +++ b/default-configs/ppc64-softmmu.mak
> @@ -50,6 +50,7 @@ CONFIG_LIBDECNUMBER=y
>  CONFIG_XICS=$(CONFIG_PSERIES)
>  CONFIG_XICS_KVM=$(and $(CONFIG_PSERIES),$(CONFIG_KVM))
>  # For PReP
> +CONFIG_SERIAL_ISA=y
>  CONFIG_MC146818RTC=y
>  CONFIG_ISA_TESTDEV=y
>  CONFIG_MEM_HOTPLUG=y

A little bit off-topic ... but maybe we should simply "include
ppc-softmmu.mak" in ppc64-softmmu.mak since the ppc64-softmmu is
supposed to offer all the 32 bit platforms, too? Then changes like this
would only affect one file instead of two.

...
> diff --git a/hw/char/Makefile.objs b/hw/char/Makefile.objs
> index 5931cc8..be42d2f 100644
> --- a/hw/char/Makefile.objs
> +++ b/hw/char/Makefile.objs
> @@ -2,7 +2,8 @@ common-obj-$(CONFIG_IPACK) += ipoctal232.o
>  common-obj-$(CONFIG_ESCC) += escc.o
>  common-obj-$(CONFIG_PARALLEL) += parallel.o
>  common-obj-$(CONFIG_PL011) += pl011.o
> -common-obj-$(CONFIG_SERIAL) += serial.o serial-isa.o
> +common-obj-$(CONFIG_SERIAL) += serial.o
> +common-obj-$(CONFIG_SERIAL_ISA) += serial-isa.o
>  common-obj-$(CONFIG_SERIAL_PCI) += serial-pci.o
>  common-obj-$(CONFIG_VIRTIO) += virtio-console.o
>  common-obj-$(CONFIG_XILINX) += xilinx_uartlite.o

Patch looks fine to me.

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PATCH v7] spec: add qcow2 bitmaps extension specification

[Vladimir Sementsov-Ogievskiy]

On 15.01.2016 02:26, John Snow wrote:


On 01/14/2016 05:08 PM, Eric Blake wrote:

On 01/11/2016 06:05 AM, Vladimir Sementsov-Ogievskiy wrote:

The new feature for qcow2: storing bitmaps.

This patch adds new header extension to qcow2 - Bitmaps Extension. It
provides an ability to store virtual disk related bitmaps in a qcow2
image. For now there is only one type of such bitmaps: Dirty Tracking
Bitmap, which just tracks virtual disk changes from some moment.

Note: Only bitmaps, relative to the virtual disk, stored in qcow2 file,
should be stored in this qcow2 file. The size of each bitmap
(considering its granularity) is equal to virtual disk size.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---

@@ -166,6 +178,34 @@ the header extension data. Each entry look like this:
  terminated if it has full length)
  
  
+== Bitmaps extension ==

+  0 -  3:  nb_bitmaps
+   The number of bitmaps contained in the image. Must be
+   greater than or equal to 1.
+
+   Note: Qemu currently only supports up to 65535 bitmaps per
+   image.
+
+  4 -  7:  bitmap_directory_size
+   Size of the bitmap directory in bytes. It is the cumulative
+   size of all (nb_bitmaps) bitmap headers.

Only 4 bytes - if we ever raise our 64k entry restriction (nb_bitmaps),
could we run into an image that has so many directory entries as to make
the directory itself spill past 4G?  But I don't think it is likely, so
I can live with your choice.


"We'll never need this!"

I hope someone in 2082 is reading this right now and is quite angry.

(But really, I can't foresee needing this many per each drive -- and if
we do, we have external storage mechanisms in development to handle such
wild cases.)


Let's change it to 8 bytes, and add 4bytes reserved field to keep 8b 
alignment.





+
+== Bitmaps ==
+
+As mentioned above, the bitmaps extension provides the ability to store bitmaps
+related a virtual disk. This section describes how these bitmaps are stored.
+
+Note: all bitmaps are related to the virtual disk stored in this image.
+
+=== Bitmap directory ===
+
+Each bitmap saved in the image is described in a bitmap directory entry. The
+bitmap directory is a contiguous area in the image file, whose starting offset
+and length are given by the header extension fields bitmap_directory_offset and
+bitmap_directory_size. The entries of the bitmap directory have variable
+length, depending on the length of the bitmap name and extra data. These
+entries are also called bitmap headers.
+
+Structure of a bitmap directory entry:
+
+Byte 0 -  7:bitmap_table_offset
+Offset into the image file at which the bitmap table
+(described below) for the bitmap starts. Must be aligned to
+a cluster boundary.
+
+ 8 - 11:bitmap_table_size
+Number of entries in the bitmap table of the bitmap.

Should this be the size in bytes, instead of the number of entries? But

For what benefit? We can calculate either from the other, and this gives
us a better resolution.


at least the entries are fixed width of 8 bytes each, so this lets you
get a bitmap table up to 32G bytes rather than just 4G in size.  (Let's
see here - if we have 32G bytes in the bitmap table, that means 4G
clusters occupied by the bitmap itself; in the worst case of 512-byte
clusters and granularity 0, that is a maximum bitmap size of 2T
describing 16T of virtual guest image; with larger cluster size and/or
larger granularity, we cover a lot more virtual guest space with less
bitmap size; so I guess we aren't too worried about running out of space?).


Yes, worst case of g=0 and cluster size of 512 bytes, we can get 2T
bitmaps describing 16T of virtual data.

"default case" of 64K clusters and 64K granularity: 256TiB bitmaps
describing ... let's see ... if my math is right, 128EiB?

We're probably fine :)

(Cue future space-person from 2159 wondering how I could have ever been
so naive. Sorry, future space-person!)


it is just like l1_size from qcow2 header.

 36 - 39:   l1_size
Number of entries in the active L1 table





+20 - 23:extra_data_size
+Size of type-specific extra data.
+
+For now, as no extra data is defined, extra_data_size is
+reserved and must be zero.
+
+variable:   Type-specific extra data for the bitmap.

I'd write this as:
variable:   extra_data
Type-specific extra data for the bitmap,
occupying extra_data_size bytes.


Ok.. then, similar should be done for the name.

About extra data and our discussion of versioning it. For now, we don't 
have concrete cases and requirements for this data, we are only foresee, 
that it may be needed in future, and waste time in 

[Qemu-devel] [PULL] qemu-sparc update

[Mark Cave-Ayland]

Hi Peter,

This is simply your VMStateDescription patchset for SPARC with appropriate
SoBs added. Please pull.


ATB,

Mark.


The following changes since commit 5a57acb66f19ee52723aa05b8afbbc41c3e9ec99:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20160115' 
into staging (2016-01-15 15:49:43 +)

are available in the git repository at:


  https://github.com/mcayland/qemu.git tags/qemu-sparc-signed

for you to fetch changes up to 6d5322442a6904fef51a409ec40f2945581220d6:

  target-sparc: Migrate CWP and PIL for SPARC64 (2016-01-16 12:01:23 +)


qemu-sparc update


Juan Quintela (4):
  vmstate: introduce CPU_DoubleU arrays
  vmstate: Introduce VMSTATE_VARRAY_MULTPLY
  vmstate: define vmstate_info_uinttl
  target-sparc: Convert to VMStateDescription

Peter Maydell (4):
  target-sparc: Split cpu_put_psr into side-effect and no-side-effect parts
  target-sparc: Don't flush TLB in cpu_load function
  target-sparc: Use VMState arrays for SPARC64 TLB/MMU state
  target-sparc: Migrate CWP and PIL for SPARC64

 hw/sparc64/sun4u.c  |   24 ---
 include/hw/hw.h |2 +
 include/migration/vmstate.h |   18 +++
 migration/vmstate.c |   27 
 target-sparc/cpu-qom.h  |4 +
 target-sparc/cpu.c  |1 +
 target-sparc/cpu.h  |7 +-
 target-sparc/machine.c  |  370 ---
 target-sparc/win_helper.c   |   19 ++-
 9 files changed, 236 insertions(+), 236 deletions(-)

Re: [Qemu-devel] [PATCH] migration: not send zero page header in ram bulk stage

[Li, Liang Z]

> On 15/01/2016 10:48, Liang Li wrote:
> > Now that VM's RAM pages are initialized to zero, (VM's RAM is allcated
> > with the mmap() and MAP_ANONYMOUS option, or mmap() without
> MAP_SHARED
> > if hugetlbfs is used.) so there is no need to send the zero page
> > header to destination.
> >
> > For guest just uses a small portions of RAM, this change can avoid
> > allocating all the guest's RAM pages in the destination node after
> > live migration. Another benefit is destination QEMU can save lots of
> > CPU cycles for zero page checking.
> >
> > Signed-off-by: Liang Li 
> 
> This does not work.  Depending on the board, some pages are written by
> QEMU before the guest starts.  If the guest rewrites them with zeroes, this
> change breaks migration.
> 
> Paolo

Hi Paolo,

   Luckily I cc to you.  Could you give an example in which case this patch 
will break migration?
Then I can understand your comments better. Much appreciate!


Liang


> 
> > ---
> >  migration/ram.c | 10 ++
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/migration/ram.c b/migration/ram.c index 4e606ab..c4821d1
> > 100644
> > --- a/migration/ram.c
> > +++ b/migration/ram.c
> > @@ -705,10 +705,12 @@ static int save_zero_page(QEMUFile *f,
> RAMBlock
> > *block, ram_addr_t offset,
> >
> >  if (is_zero_range(p, TARGET_PAGE_SIZE)) {
> >  acct_info.dup_pages++;
> > -*bytes_transferred += save_page_header(f, block,
> > -   offset | 
> > RAM_SAVE_FLAG_COMPRESS);
> > -qemu_put_byte(f, 0);
> > -*bytes_transferred += 1;
> > +if (!ram_bulk_stage) {
> > +*bytes_transferred += save_page_header(f, block, offset |
> > +   RAM_SAVE_FLAG_COMPRESS);
> > +qemu_put_byte(f, 0);
> > +*bytes_transferred += 1;
> > +}
> >  pages = 1;
> >  }
> >
> >

Re: [Qemu-devel] [PATCH] migration: not send zero page header in ram bulk stage

[Li, Liang Z]

> * Liang Li (liang.z...@intel.com) wrote:
> > Now that VM's RAM pages are initialized to zero, (VM's RAM is allcated
> > with the mmap() and MAP_ANONYMOUS option, or mmap() without
> MAP_SHARED
> > if hugetlbfs is used.) so there is no need to send the zero page
> > header to destination.
> >
> > For guest just uses a small portions of RAM, this change can avoid
> > allocating all the guest's RAM pages in the destination node after
> > live migration. Another benefit is destination QEMU can save lots of
> > CPU cycles for zero page checking.
> 
> I think this would break postcopy, because the zero pages wouldn't be filled
> in, so accessing them would still generate a userfault.
> So you'd have to disable this optimisation if postcopy is enabled (even during
> the precopy bulk stage).
> 
> Also, are you sure about the benefits?
>  Destination guests RAM should not be allocated on receiving a zero page;
> see ram_handle_compressed, it doesn't write to the page if it's zero, so it
> shouldn't cause an allocate.  I think you're probably correct about the zero
> page test on the destination, I wonder if we can speed that up.
> 
> Dave

I have test the performance, with a 8G guest just booted, this patch can reduce 
total live migration time about 10%.
Unfortunately, Paolo said this patch would break LM in some case 

For the zero page test on the destination, if the page is really a zero page, 
test is faster than writing a whole page of zero.

Liang

Re: [Qemu-devel] [PATCH v16 00/14] vfio-pci: pass the aer error to guest

[Michael S. Tsirkin]

On Tue, Jan 12, 2016 at 10:43:01AM +0800, Cao jin wrote:
> From: Chen Fan 
> 
> For now, for vfio pci passthough devices when qemu receives
> an error from host aer report, currentlly just terminate the guest,
> but usually user want to know what error occurred but stopping the
> guest, so this patches add aer capability support for vfio device,
> and pass the error to guest, and have guest driver to recover
> from the error.

I would like to see a version of this patchset that doesn't
depend on pci core changes.
I think that if you make this simplifying assumption:

- all devices on same bus in guest are on same bus in host

then you can handle both reset and hotplug simply in function 0
since it will belong to vfio.

So we can have a version without pci core changes that simply assumes
this, and things will just work.


Now, if we wanted to enforce this limitation, I think the
cleanest way would be to add a callback in struct PCIDevice:

bool is_valid_function(PCIDevice *newfunction)

and call it as each function is added.
This way aer function can validate that each function
added shares the same bus.
And this way issues will be detected directly and not when
function 0 is added.

I would prefer this validation code to be a patch on top so we can merge
the functionality directly and avoid blocking it while we figure out the
best api to validate things.

I don't see why making guest topology match host would
ever be a problem, but if it's required to support
configurations where these differ, I'd like to see
an attempt to address that be split out, after aer
is supported.



> v15-v16:
>10/14, 11/14 are new to introduce a reset sequence id to specify the
>vfio devices has been reset for that reset. other patches aren't modified.
> 
> v14-v15:
>1. add device hot reset callback
>2. add bus_in_reset for vfio device to avoid multi do host bus reset
> 
> v13-v14:
>1. for multifunction device, requiring all functions enable AER.(9/13)
>2. due to all affected functions receive error signal, ignore no
>   error occurred function. (12/13)
> 
> v12-v13:
>1. since support multifuncion hotplug, here add callback to enable aer.
>2. add pci device pre+post reset for aer host reset.
> 
> Chen Fan (14):
>   vfio: extract vfio_get_hot_reset_info as a single function
>   vfio: squeeze out vfio_pci_do_hot_reset for support bus reset
>   pcie: modify the capability size assert
>   vfio: make the 4 bytes aligned for capability size
>   vfio: add pcie extanded capability support
>   aer: impove pcie_aer_init to support vfio device
>   vfio: add aer support for vfio device
>   vfio: add check host bus reset is support or not
>   add check reset mechanism when hotplug vfio device
>   pci: introduce pci bus pre reset
>   vfio: introduce last reset sequence id
>   pcie_aer: expose pcie_aer_msg() interface
>   vfio-pci: pass the aer error to guest
>   vfio: add 'aer' property to expose aercap
> 
>  hw/pci-bridge/ioh3420.c|   2 +-
>  hw/pci-bridge/xio3130_downstream.c |   2 +-
>  hw/pci-bridge/xio3130_upstream.c   |   2 +-
>  hw/pci/pci.c   |  42 +++
>  hw/pci/pci_bridge.c|   3 +
>  hw/pci/pcie.c  |   2 +-
>  hw/pci/pcie_aer.c  |   6 +-
>  hw/vfio/pci.c  | 616 
> +
>  hw/vfio/pci.h  |   9 +
>  include/hw/pci/pci.h   |   1 +
>  include/hw/pci/pci_bus.h   |   8 +
>  include/hw/pci/pcie_aer.h  |   3 +-
>  12 files changed, 624 insertions(+), 72 deletions(-)
> 
> -- 
> 1.9.3
> 
> 

[Qemu-devel] [Bug 1534978] [NEW] Windows command line -name cannot use = sign

[Steve Si]

Public bug reported:

Windows command line:

qemu.exe -L . -name "32-bit Emulation Session RAM=500MB" -boot c -m 500
-drive file=\\.\PhysicalDrive2

This fails to run.
If I remove the = sign in the -name quoted string it runs OK.

** Affects: qemu
 Importance: Undecided
 Status: New

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

Title:
  Windows command line -name cannot use = sign

Status in QEMU:
  New

Bug description:
  Windows command line:

  qemu.exe -L . -name "32-bit Emulation Session RAM=500MB" -boot c -m
  500 -drive file=\\.\PhysicalDrive2

  This fails to run.
  If I remove the = sign in the -name quoted string it runs OK.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1534978/+subscriptions

Re: [Qemu-devel] [PATCH v1 1/2] tcg: Add support for constant value promises

[Lluís Vilanova]

Richard Henderson writes:

> On 01/15/2016 12:12 PM, Lluís Vilanova wrote:
>> Richard Henderson writes:
>> 
>>> On 01/15/2016 07:35 AM, Lluís Vilanova wrote:
 +TCGv_i64 tcg_promise_i64(TCGv_promise_i64 *promise)
 +{
 +int pi = tcg_ctx.gen_next_parm_idx;
 +*promise = (TCGv_promise_i64)_ctx.gen_opparam_buf[pi];
 +return tcg_const_i64(0xdeadcafe);
 +}
>> 
>>> This doesn't work for a 32-bit host.  The constant may be split across two
>>> different parameter indices, and you don't know exactly where the second 
>>> will be.
>> 
>>> Because of that, I think this is over-engineered, and really prefer the 
>>> simpler
>>> interface that Edgar posted last week.
>> 
>> In this case, 'tcg_set_promise_i64' sets the two arguments accordingly on 
>> 32-bit
>> targets. Both solutions depend on TCG internals (in this specific case the
>> implementation of 'tcg_gen_movi_i64'), but now it's all implemented inside 
>> TCG.
>> 
>> Alternatively, promises could use the longer route of recording the opcode 
>> index
>> (as Edgar did AFAIR), and retrieve the argument pointer from there. Still, 
>> for
>> 32-bit targets we have to assume the two immediate moves are gonna generate 
>> two
>> consecutive opcodes.

> Your solution also doesn't help Edgar, since he's interested in modifying an
> argument to the insn_start opcode, not modifying a literal constant in a move.

I wasn't aware of that. If the idea was to use this for more than immediates
stored in TCGv values, I see two options. First, modify the necessary opcodes to
use a TCGv argument instead of an immediate. Second, generalize this patch to
to select any opcode argument.

An example of the generalization when used to reimplement icount:

   // insn count placeholder
   TCGv_i32 imm = tcg_const_i32(0xcafecafe);
   // insn count promise
   TCGv_promise_i32 imm_promise = tcg_promise_i32(
   1,  // how many opcodes to go "backwards"
   1); // what argument to modify on that opcode
   // operate with imm
   ...
   // resolve value
   tcg_set_promise_i32(imm_promise, insn_count);

The question still stands on how to cleanly handle promises for opcodes like a
64-bit mov on a 32-bit host (it's generated as two opcodes). Using this
interface would still be cleaner than directly manipulating the low-level TCG
arrays, and makes it easier to adopt it in future changes.


Cheers,
  Lluis

Re: [Qemu-devel] bug with io/channel-socket.c - variable-sized object may not be initialized

[Programmingkid]

On Jan 15, 2016, at 6:56 PM, Eric Blake wrote:

> On 01/13/2016 02:19 PM, Programmingkid wrote:
>> This code causes an error to occur during compiling: 
>> 
>> char control[CMSG_SPACE(sizeof(int) * SOCKET_MAX_FDS)] = { 0 };
>> 
>> It is located at line 496 in io/channel-socket.c. 
>> 
>> Here is the full error message:
>> io/channel-socket.c: In function 'qio_channel_socket_writev':
>> io/channel-socket.c:496:18: error: variable-sized object may not be 
>> initialized
>> char control[CMSG_SPACE(sizeof(int) * SOCKET_MAX_FDS)] = { 0 };
>> 
>> This is from gcc 4.9 running on Mac OS 10.6.8. 
> 
> Uggh. That sounds like a bug in the Mac OS headers, for making
> CMSG_SPACE() not be a compile-time constant.  We do NOT want to be using
> variable-sized objects here, so we need a compile-time constant for the
> array size, even if we have to work around your platform's borked headers.
> Can you capture what that line expands to after pre-processing, to see
> if my guess is right?

It expands to 76.