Re: [Qemu-devel] [PATCH] migration: fix bad string passed to error_report()

[Greg Kurz]

On Thu, 4 Feb 2016 15:30:11 +0530
Amit Shah  wrote:

> On (Thu) 04 Feb 2016 [10:20:07], Greg Kurz wrote:
> > state->name does not contain a terminating '\0' and you may get:
> > 
> > Machine type received is 'pseries-2.3y�?' and local is 'pseries-2.4'
> > load of migration failed: Invalid argument
> > 
> > Let's add a precision modifier to fix this.
> > 
> > Signed-off-by: Greg Kurz   
> 
> Reviewed-by: Amit Shah 
> 
>   Amit
> 

I forgot to mention that this patch fixes:

61964c23e5ddd5a33f15699e45ce126f879e3e33 "migration: Add configuration section"

Cc'ing stable since this is a 2.4.0 commit and it may *theorically* lead to
a crash (even if that is very unlikely to occur).

Also Cc'ing trivial.

Thanks !

--
Greg

[Qemu-devel] [PATCH] quorum: fix segfault when read fails in fifo mode

[Changlong Xie]

Signed-off-by: Wen Congyang 
Signed-off-by: Changlong Xie 
---
 block/quorum.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/block/quorum.c b/block/quorum.c
index a5ae4b8..0965277 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -295,6 +295,9 @@ static void quorum_aio_cb(void *opaque, int ret)
 quorum_copy_qiov(acb->qiov, >qcrs[acb->child_iter].qiov);
 }
 acb->vote_ret = ret;
+if (ret < 0) {
+acb->child_iter--;
+}
 quorum_aio_finalize(acb);
 return;
 }
-- 
1.9.3

Re: [Qemu-devel] [PATCH 07/10] virtio: combine the read of a descriptor

[Paolo Bonzini]

On 04/02/2016 08:48, Gonglei (Arei) wrote:
> 11.44%  qemu-kvm [.] memory_region_find
>   6.31%  qemu-kvm [.] qemu_get_ram_ptr
>   4.61%  libpthread-2.19.so   [.] __pthread_mutex_unlock_usercnt
>   3.54%  qemu-kvm [.] qemu_ram_addr_from_host
>   2.80%  libpthread-2.19.so   [.] pthread_mutex_lock
>   2.55%  qemu-kvm [.] object_unref
>   2.49%  libc-2.19.so [.] malloc
>   2.47%  libc-2.19.so [.] _int_malloc
>   2.34%  libc-2.19.so [.] _int_free
>   2.18%  qemu-kvm [.] object_ref
>   2.18%  qemu-kvm [.] address_space_translate
>   2.03%  libc-2.19.so [.] __memcpy_sse2_unaligned
>   1.76%  libc-2.19.so [.] malloc_consolidate
>   1.56%  qemu-kvm [.] addrrange_intersection
>   1.52%  qemu-kvm [.] vring_pop
>   1.36%  qemu-kvm [.] find_next_zero_bit
>   1.30%  [kernel] [k] native_write_msr_safe
>   1.29%  qemu-kvm [.] addrrange_intersects
>   1.21%  qemu-kvm [.] vring_map
>   0.93%  qemu-kvm [.] virtio_notify
> 
> Do you have any thoughts to decrease the cpu overhead and get higher through 
> output? Thanks!

Using bigger chunks than 256 bytes will reduce the overhead in
memory_region_find and qemu_get_ram_ptr.  You could expect
 a further 10-12% improvement.

Paolo

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

[Michael S. Tsirkin]

On Thu, Feb 04, 2016 at 10:04:01AM +0800, Chen Fan wrote:
> 
> On 02/03/2016 09:57 PM, Michael S. Tsirkin wrote:
> >On Wed, Feb 03, 2016 at 04:54:01PM +0800, Chen Fan wrote:
> >>On 01/17/2016 02:34 AM, Michael S. Tsirkin wrote:
> >>>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.
> >>Hi Michael,
> >>
> >>Just think about this more,  I think we also should check the vfio
> >>devices whether on the same bus at the time of function 0 is added.
> >>because we don't know the affected devices by a bus reset have
> >>already all been assigned to VM.
> >This is something vfio in kernel should check.
> >You can't rely on qemu being well behaved, so don't
> >even try to catch cases which would break host in userspace.
> >
> >qemu should only worry about not breaking guest.
> >
> >
> >>for example, the multi-function's hotplug.
> >>devices on same bus in host are added to VM one by one. when we
> >>test one device, we haven't yet added the other devices.
> >>so I think
> >>the patch should like below. then we could add a vfio_is_valid_function in
> >>vfio
> >>to test each device whether the affected devices on the same bus.
> >>
> >>Thanks,
> >>Chen
> >>
> >>diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> >>index d940f79..7163b56 100644
> >>--- a/hw/pci/pci.c
> >>+++ b/hw/pci/pci.c
> >>@@ -1836,6 +1836,38 @@ PCIDevice *pci_find_device(PCIBus *bus, int bus_num,
> >>uint8_t devfn)
> >>  return bus->devices[devfn];
> >>  }
> >>
> >>+static int pci_bus_check_devices(PCIBus *bus)
> >>+{
> >>+PCIDeviceClass *pc;
> >>+int i, ret = 0;
> >>+
> >>+for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) {
> >>+if (!bus->devices[i]) {
> >>+continue;
> >>+}
> >>+
> >>+pc = PCI_DEVICE_GET_CLASS(bus->devices[i]);
> >>+if (!pc->is_valid_func) {
> >>+continue;
> >>+}
> >>+
> >>+ret = pc->is_valid_func(bus->devices[i], bus);
> >>+if (!ret) {
> >>+return -1;
> >>+}
> >>+}
> >>+return 0;
> >>+}
> >>+
> >>+static bool pci_is_valid_function(PCIDevice *pdev, PCIBus *bus)
> >>+{
> >>+if (pdev->bus == bus) {
> >>+return true;
> >>+}
> >>+
> >>+return false;
> >>+}
> >>+
> >I don't really understand what is this one doing.
> >Why do we need a default function?
> if the vfio driver in kernel can handle the bus reset for any one
> device in qemu without the affected devices assigned. I think
> we don't need this default one.
> BTW, IIRC at present the devices on the same bus in host can
> be assigned to different VM, so if we want to support this kind of
> bus reset for an independent device when enable aer, aren't we
> limiting the case that others devices on the same bus must be
> assigned to current VM?
> 
> Thanks,
> Chen

I don't believe this works at the moment, and
I'd expect kernel to prevent this,
so we should not rely on userspace code for this.
Alex, could you comment please?


> >>  static void pci_qdev_realize(DeviceState *qdev, Error **errp)
> >>  {
> >>  PCIDevice *pci_dev = (PCIDevice *)qdev;
> >>@@ -1878,6 +1910,14 @@ static void pci_qdev_realize(DeviceState *qdev, Error
> >>**errp)
> >>  pci_qdev_unrealize(DEVICE(pci_dev), NULL);
> >>  

[Qemu-devel] [PATCH 3/5] pc-dimm: add pc_dimm_build_list()

[Vladimir Sementsov-Ogievskiy]

Like pc_dimm_build_list_sorted but not sorted - for cases where sorting
is not necessary.

Signed-off-by: Vladimir Sementsov-Ogievskiy 

CC: Stefan Hajnoczi 
CC: Xiao Guangrong 
CC: "Michael S. Tsirkin" 
CC: Igor Mammedov 
CC: Eric Blake 
CC: Markus Armbruster 
---
 hw/mem/pc-dimm.c | 15 +++
 include/hw/mem/pc-dimm.h |  3 +++
 2 files changed, 18 insertions(+)

diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 4a681bc..0f102c0 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -266,6 +266,21 @@ static int pc_dimm_build_list_sorted(Object *obj, void 
*opaque)
 return 0;
 }
 
+int pc_dimm_build_list(Object *obj, void *opaque)
+{
+GSList **list = opaque;
+
+if (object_dynamic_cast(obj, TYPE_PC_DIMM)) {
+DeviceState *dev = DEVICE(obj);
+if (dev->realized) { /* only realized DIMMs matter */
+*list = g_slist_prepend(*list, dev);
+}
+}
+
+object_child_foreach(obj, pc_dimm_build_list, opaque);
+return 0;
+}
+
 uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
uint64_t address_space_size,
uint64_t *hint, uint64_t align, uint64_t size,
diff --git a/include/hw/mem/pc-dimm.h b/include/hw/mem/pc-dimm.h
index d83bf30..ab136a3 100644
--- a/include/hw/mem/pc-dimm.h
+++ b/include/hw/mem/pc-dimm.h
@@ -94,4 +94,7 @@ void pc_dimm_memory_plug(DeviceState *dev, MemoryHotplugState 
*hpms,
  MemoryRegion *mr, uint64_t align, Error **errp);
 void pc_dimm_memory_unplug(DeviceState *dev, MemoryHotplugState *hpms,
MemoryRegion *mr);
+
+int pc_dimm_build_list(Object *obj, void *opaque);
+
 #endif
-- 
1.8.3.1

[Qemu-devel] [PATCH 1/5] move get_current_ram_size to virtio-balloon.c

[Vladimir Sementsov-Ogievskiy]

get_current_ram_size() is used only in virtio-balloon.c
This patch moves it into virtio-balloon and make it static, to allow
some balloon-specific tuning.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Denis V. Lunev 
Reviewed-by: Eric Blake 

CC: Stefan Hajnoczi 
CC: Xiao Guangrong 
CC: "Michael S. Tsirkin" 
CC: Igor Mammedov 
CC: Eric Blake 
CC: Markus Armbruster 
---
 hw/mem/pc-dimm.c| 26 --
 hw/virtio/virtio-balloon.c  | 26 ++
 include/exec/cpu-common.h   |  1 -
 stubs/qmp_pc_dimm_device_list.c |  5 -
 4 files changed, 26 insertions(+), 32 deletions(-)

diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index d5cdab2..4f30950 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -191,32 +191,6 @@ int qmp_pc_dimm_device_list(Object *obj, void *opaque)
 return 0;
 }
 
-ram_addr_t get_current_ram_size(void)
-{
-MemoryDeviceInfoList *info_list = NULL;
-MemoryDeviceInfoList **prev = _list;
-MemoryDeviceInfoList *info;
-ram_addr_t size = ram_size;
-
-qmp_pc_dimm_device_list(qdev_get_machine(), );
-for (info = info_list; info; info = info->next) {
-MemoryDeviceInfo *value = info->value;
-
-if (value) {
-switch (value->type) {
-case MEMORY_DEVICE_INFO_KIND_DIMM:
-size += value->u.dimm->size;
-break;
-default:
-break;
-}
-}
-}
-qapi_free_MemoryDeviceInfoList(info_list);
-
-return size;
-}
-
 static int pc_dimm_slot2bitmap(Object *obj, void *opaque)
 {
 unsigned long *bitmap = opaque;
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 9671635..6a4c4d2 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -294,6 +294,32 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, 
uint8_t *config_data)
 memcpy(config_data, , sizeof(struct virtio_balloon_config));
 }
 
+static ram_addr_t get_current_ram_size(void)
+{
+MemoryDeviceInfoList *info_list = NULL;
+MemoryDeviceInfoList **prev = _list;
+MemoryDeviceInfoList *info;
+ram_addr_t size = ram_size;
+
+qmp_pc_dimm_device_list(qdev_get_machine(), );
+for (info = info_list; info; info = info->next) {
+MemoryDeviceInfo *value = info->value;
+
+if (value) {
+switch (value->type) {
+case MEMORY_DEVICE_INFO_KIND_DIMM:
+size += value->u.dimm->size;
+break;
+default:
+break;
+}
+}
+}
+qapi_free_MemoryDeviceInfoList(info_list);
+
+return size;
+}
+
 static void virtio_balloon_set_config(VirtIODevice *vdev,
   const uint8_t *config_data)
 {
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 85aa403..a0ad2ac 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -54,7 +54,6 @@ typedef uintptr_t ram_addr_t;
 #endif
 
 extern ram_addr_t ram_size;
-ram_addr_t get_current_ram_size(void);
 
 /* memory API */
 
diff --git a/stubs/qmp_pc_dimm_device_list.c b/stubs/qmp_pc_dimm_device_list.c
index b584bd8..5cb220c 100644
--- a/stubs/qmp_pc_dimm_device_list.c
+++ b/stubs/qmp_pc_dimm_device_list.c
@@ -5,8 +5,3 @@ int qmp_pc_dimm_device_list(Object *obj, void *opaque)
 {
return 0;
 }
-
-ram_addr_t get_current_ram_size(void)
-{
-return ram_size;
-}
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 06/12] 4byte address mode support added.

[Krzeminski, Marcin (Nokia - PL/Wroclaw)]

> -Original Message-
> From: EXT Peter Crosthwaite [mailto:crosthwaitepe...@gmail.com]
> Sent: Tuesday, December 22, 2015 10:29 PM
> To: Cédric Le Goater; g...@xilinx.com
> Cc: Krzeminski, Marcin (Nokia - PL/Wroclaw); qemu-devel@nongnu.org
> Developers; Lenkow, Pawel (Nokia - PL/Wroclaw)
> Subject: Re: [Qemu-devel] [PATCH 06/12] 4byte address mode support
> added.
> 
> On Tue, Dec 22, 2015 at 10:41 AM, Cédric Le Goater  wrote:
> > Hello Marcin,
> >
> >
> > On 12/16/2015 01:57 PM, marcin.krzemin...@nokia.com wrote:
> >> From: Marcin Krzeminski 
> >>
> >> Signed-off-by: Marcin Krzeminski 
> >> ---
> >>  hw/block/m25p80.c | 31 ---
> >>  1 file changed, 28 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c index
> >> 1a547ae..6d5d90d 100644
> >> --- a/hw/block/m25p80.c
> >> +++ b/hw/block/m25p80.c
> >> @@ -237,6 +237,9 @@ typedef enum {
> >>  ERASE_32K = 0x52,
> >>  ERASE_SECTOR = 0xd8,
> >>
> >> +EN_4BYTE_ADDR = 0xB7,
> >> +EX_4BYTE_ADDR = 0xE9,
> >> +
> >>  RESET_ENABLE = 0x66,
> >>  RESET_MEMORY = 0x99,
> >>
> >> @@ -267,6 +270,7 @@ typedef struct Flash {
> >>  uint8_t cmd_in_progress;
> >>  uint64_t cur_addr;
> >>  bool write_enable;
> >> +bool four_bytes_address_mode;
> >>  bool reset_enable;
> >>  bool initialized;
> >>  uint8_t reset_pin;
> >> @@ -405,11 +409,24 @@ void flash_write8(Flash *s, uint64_t addr,
> uint8_t data)
> >>  s->dirty_page = page;
> >>  }
> >>
> >> +static inline int is_4bytes(Flash *s) {
> >> +   return s->four_bytes_address_mode;
> >> +   }
> >> +}
> >> +
> >>  static void complete_collecting_data(Flash *s)  {
> >> -s->cur_addr = s->data[0] << 16;
> >> -s->cur_addr |= s->data[1] << 8;
> >> -s->cur_addr |= s->data[2];
> >> +if (is_4bytes(s)) {
> >> +s->cur_addr = s->data[0] << 24;
> >> +s->cur_addr |= s->data[1] << 16;
> >> +s->cur_addr |= s->data[2] << 8;
> >> +s->cur_addr |= s->data[3];
> >> +} else {
> >> +s->cur_addr = s->data[0] << 16;
> >> +s->cur_addr |= s->data[1] << 8;
> >> +s->cur_addr |= s->data[2];
> >> +}
> >>
> >>  s->state = STATE_IDLE;
> >
> >
> > Don't we need to also change 'needed_bytes' in the decode_new_cmd()
> > routine to increase the number of bytes expected by some commands ?
> >
> 
> I think you are right, and it may be solved later in the series, from patch 
> 10:
> 
>  case QPP:
>  case PP:
> -s->needed_bytes = 3;
> +   s->needed_bytes = is_4bytes(s) ? 4 : 3;
>  s->pos = 0;
>  s->len = 0;
>  s->state = STATE_COLLECTING_DATA;
> 
> This hunk should be brought forward to this patch.
> 
> > If so, we could add a width attribute to 'struct Flash' and to something 
> > like :
> >
> > @@ -260,6 +263,7 @@ typedef struct Flash {
> >  uint8_t cmd_in_progress;
> >  uint64_t cur_addr;
> >  bool write_enable;
> > +uint8_t width;
> >
> >  int64_t dirty_page;
> >
> > @@ -401,6 +405,10 @@ static void complete_collecting_data(Fla
> >  s->cur_addr |= s->data[1] << 8;
> >  s->cur_addr |= s->data[2];
> >
> > +if (s->width == 4) {
> > +s->cur_addr = s->cur_addr << 8 | s->data[4];
> > +}
> > +
> >  s->state = STATE_IDLE;
> >
> >  switch (s->cmd_in_progress) {
> > @@ -446,7 +454,7 @@ static void decode_new_cmd(Flash *s, uin
> >  case DPP:
> >  case QPP:
> >  case PP:
> > -s->needed_bytes = 3;
> > +s->needed_bytes = s->width;
> >  s->pos = 0;
> >  s->len = 0;
> >  s->state = STATE_COLLECTING_DATA;
> > @@ -644,6 +658,7 @@ static int m25p80_init(SSISlave *ss)
> >  memset(s->storage, 0xFF, s->size);
> >  }
> >
> > +s->width = 3;
> >  return 0;
> >  }
> >
> >
> >
> > QOR, DIOR, QIOR command also need a check. I suppose an address and
> > some number of dummy bytes are expected before the fast read
> command is done.
> > I would need to check the datasheets.
> >
> 
> I just checked an n25q256 datasheet, and yes you are right. The same logic as
> in the hunk above needs to apply to these commands. CC Xilinx, this bug is in
> their tree as well I think.
> 
> https://github.com/Xilinx/qemu/blob/pub/2015.2.plnx/hw/block/m25p80.c
> 
> Where PP, READ and friends have the 4 byte correction logic based on
> addr_4b but QIOR does not.
> 
> Nice catch :)
> 
> Regards,
> Peter
> 

Hello Cedric,

Sorry for late response.
As peter has responded, needed bytes for 4bytes address mode/cmd length is 
handled partially (not for all commands).
Dummy cycles are not handled since my QSPI controller model had a 

Re: [Qemu-devel] [RFC PATCH 1/4] tests: Introduce Docker based tests

[Daniel P. Berrange]

On Wed, Feb 03, 2016 at 10:36:01PM +0800, Fam Zheng wrote:

> diff --git a/tests/docker/fedora22.docker b/tests/docker/fedora22.docker
> new file mode 100644
> index 000..5fd48d6
> --- /dev/null
> +++ b/tests/docker/fedora22.docker

Fedora 22 will go end of life in June. I think we should at least
start with the current Fedora 23 since that's the current preferred
Fedora release.

> @@ -0,0 +1,6 @@
> +FROM fedora:22
> +RUN dnf install -y \
> +ccache git mingw{32,64}-{pixman,glib2,gmp,SDL,pkg-config} \
> +glib2-devel pixman-devel zlib-devel SDL-devel \
> +gcc g++ clang make perl which bc findutils

That's a pretty minimal set of mingw packages - many features will
end up disabled with that. Add mingw{32,64} packages for gtk2, gtk3,
gnutls, nettle, libtasn1, libjpeg-turbo, libpng, curl, libssh2,
bzip2 too, which is everything Fedora mingw is capable of providing
for QEMU


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

[Qemu-devel] [PATCH v2 10/11] block: m25p80: n25q256a/n25q512a models

[marcin . krzeminski]

From: Marcin Krzeminski 

Signed-off-by: Marcin Krzeminski 
Reviewed-by: Peter Crosthwaite 
---
 hw/block/m25p80.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 116e098..ec1013d 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -210,8 +210,9 @@ static const FlashPartInfo known_devices[] = {
 { INFO("w25q80bl",0xef4014,  0,  64 << 10,  16, ER_4K) },
 { INFO("w25q256", 0xef4019,  0,  64 << 10, 512, ER_4K) },
 
-/* Numonyx -- n25q128 */
 { INFO("n25q128",  0x20ba18,  0,  64 << 10, 256, 0) },
+{ INFO("n25q256a", 0x20ba19,  0,  64 << 10, 512, ER_4K) },
+{ INFO("n25q512a", 0x20ba20,  0,  64 << 10, 1024, ER_4K) },
 };
 
 typedef enum {
-- 
2.5.0

Re: [Qemu-devel] [PATCH v1 3/5] include/qemu/atomic.h: default to __atomic functions

[Paolo Bonzini]

On 29/01/2016 17:06, Alex Bennée wrote:
> 
> Paolo Bonzini  writes:
> 
>> On 28/01/2016 11:15, Alex Bennée wrote:
>>> +/* atomic_mb_read/set semantics map Java volatile variables. They are
>>> + * less expensive on some platforms (notably POWER & ARM) than fully
>>> + * sequentially consistent operations.
>>> + *
>>> + * As long as they are used as paired operations they are safe to
>>> + * use. See docs/atomic.txt for more discussion.
>>> + */
> 
> The original comment mentioned both POWER and ARM so I wondering if we
> should also special case for the ARMv7?

I don't know the exact feature test, and I think ARMv7's penalty is much
less because processors are slower, with a less deep pipeline and
usually not used in SMP configurations.

In fact, because it doesn't have "dmb st" and friends, the generated
code should be exactly the same for ARMv7.  Looking at
https://www.cl.cam.ac.uk/~pes20/cpp/cpp0xmappings.html confirms this.

> I think we are OK because if cmpxchg succeeds _old was by definition
> what was already there but it is confusing and leads to funny code like
> this:
> 
> if (atomic_cmpxchg([i].n, 0, 3) == 0) {
> data[i].ret = -ECANCELED;
> ...
> 
> and
> 
> if (atomic_cmpxchg(>state, old_state, new_state) == old_state) {
>...
> 
> Which might be easier to read if atomic_cmpxchg used the bool
> semantics, i.e. return true for a successful cmpxchg.

It depends.  When s->state is bool, the bool version is *very* hard to
read.  Then you have two bools and you never know which one it is.  For
example if the expected value is false, atomic_bool_cmpxchg will return
true if the memory location was false...  Aargh! :D

> The old code even has a atomic_bool_cmpxchg which no one seems to use.

Alvise added it, but it's not upstream.

> I wonder if the correct solution is to convert atomic_cmpxchg calls to use
> atomic_cmpxchg_bool calls and remove atomic_cmpxchg from atomic.h?

Yeah, though there are also cases where atomic_cmpxchg_bool is less
efficient.

Not to mention that I don't like the name...  Perhaps atomic_cmpxchg
should be the bool one and atomic_fetch_cmpxchg should return the value.
 Separate patch series though.

Paolo

Re: [Qemu-devel] [PATCH] migration: fix bad string passed to error_report()

[Amit Shah]

On (Thu) 04 Feb 2016 [10:20:07], Greg Kurz wrote:
> state->name does not contain a terminating '\0' and you may get:
> 
> Machine type received is 'pseries-2.3y�?' and local is 'pseries-2.4'
> load of migration failed: Invalid argument
> 
> Let's add a precision modifier to fix this.
> 
> Signed-off-by: Greg Kurz 

Reviewed-by: Amit Shah 

Amit

Re: [Qemu-devel] [PATCH 0/6] Some improvements and small fixes for migration

[Amit Shah]

On (Sat) 30 Jan 2016 [18:32:22], Hailiang Zhang wrote:
> Hi Juan & Amit,
> 
> This series is prerequisite of COLO, and all of them have been reviewed by 
> Dave,
> Could you please review and merge them ?

Yea, I've been away last couple of weeks.  Getting to this now.

Amit

Re: [Qemu-devel] [PULL 00/13] Tracing patches

[Peter Maydell]

On 3 February 2016 at 18:49, Paolo Bonzini  wrote:
>
>
> On 03/02/2016 18:55, Peter Maydell wrote:
>> On 3 February 2016 at 15:47, Stefan Hajnoczi  wrote:
>> I think this is a quirk of this elderly mingw setup that's come up
>> before, but I forget what the resolution of it was.
>> (I now have a more up to date mingw build in the mix too, so I'm
>> not completely averse to the suggestion of "drop this thing".)
>
> Looks like this was always broken, it just required some extra configure
> argument.
>
> I would "drop this thing" or, better, you could compile with
> "--enable-trace-backend=nop" on the old mingw setup and without it on
> the new one.  This has the additional advantage of better coverage.

I added this configure argument and the build does pass, so I've
pushed it to master. I probably will drop the build config
entirely at some point though, IIRC it doesn't actually build
working executables. It's kind of handy to have one elderly
gcc in the mix, though, as long as the mingw foibles don't
cause too much hassle.

thanks
-- PMM

Re: [Qemu-devel] [RFC PATCH 3/4] tests: Add mingw 32/64 cross compiling

[Paolo Bonzini]

On 04/02/2016 03:29, Fam Zheng wrote:
>> > I suggest to add a trace backend as well (my tests run with
>> > --enable-trace-backend=stderr, so maybe you want to choose
>> > a different one).
> Will do.
> 

Note that the stderr (now renamed to "log") is now the default.

Paolo

Re: [Qemu-devel] qdev & hw/core owner? (was Re: [PATCH v19 7/9] machine: add properties to compat_props incrementaly)

[Paolo Bonzini]

On 04/02/2016 12:41, Andreas Färber wrote:
> You're talking about machine, right? Some time ago I had proposed Marcel
> who initially worked on it, but I'm fine with anyone taking it.

Yes.

> For some (but not all) core qdev parts related to the (stalled) QOM
> migration I've been taking care of via qom-next. Last time this came up
> you didn't want anyone to be M: for qdev, so maybe we can use R: so that
> at least people automatically get CC'ed and we avoid this recurring
> discussion?

I might have changed my mind on that.  You definitely should be M: for qdev.

Paolo

Re: [Qemu-devel] qdev & hw/core owner? (was Re: [PATCH v19 7/9] machine: add properties to compat_props incrementaly)

[Marcel Apfelbaum]

On 02/04/2016 01:41 PM, Andreas Färber wrote:

Am 04.02.2016 um 12:31 schrieb Paolo Bonzini:

On 03/02/2016 20:06, Michael S. Tsirkin wrote:

On Wed, Feb 03, 2016 at 03:55:04PM -0200, Eduardo Habkost wrote:

On Thu, Jan 28, 2016 at 06:00:31PM +0100, Igor Mammedov wrote:
[...]

It looks like this series might go nowhere but this patch
is not tied to it and useful to us in general
so perhaps you could pick it up after ACKs from
S390/SPAPR maintainers.



Reviewed-by: Eduardo Habkost 


We don't have a maintainer for hw/core/machine.c, hw/core/qdev*,
and related files.

Assuming we don't have a volunteer to maintain them officially,
can we agree on a default destination for those patches so they
don't linger on the list? Michael? Andreas?


Not me please. Have too much on my plate.
Would you like to maintain it yourself?


That's my suggestion too.  I guess Igor and I could help with reviews,
but testing and sending the pull requests would add too much work.
Since you're the main one touching it, it makes sense for you to handle it.


You're talking about machine, right? Some time ago I had proposed Marcel
who initially worked on it, but I'm fine with anyone taking it.


Hi,

As I previously said I can maintain the machine and the related code.
I'll gladly help Eduardo with reviews or have a tree ready for machine/qdev
and send pull requests if Peter agrees to it.

Eduardo, Peter what is your take on this?

Thanks,
Marcel




For some (but not all) core qdev parts related to the (stalled) QOM
migration I've been taking care of via qom-next. Last time this came up
you didn't want anyone to be M: for qdev, so maybe we can use R: so that
at least people automatically get CC'ed and we avoid this recurring
discussion?

Regards,
Andreas

[Qemu-devel] [PATCH v2 5/6] qmp-commands.hx: Fix the missing options for migration parameters commands

[zhanghailiang]

We didn't document x-cpu-throttle-initial/x-cpu-throttle-increment for
commands migrate-set-parameters and query-migrate-parameters.

Here we add the descriptions for these two options and fix the wrong example
for query-migrate-parameters qmp commands.
Besides, this will also fix the bug that we cann't set x-cpu-throttle-initial
and x-cpu-throttle-increment through migrate-set-parameters qmp command.

Signed-off-by: zhanghailiang 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Amit Shah 
---
 qmp-commands.hx | 18 ++
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/qmp-commands.hx b/qmp-commands.hx
index db072a6..b25b97c 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3700,6 +3700,10 @@ Set migration parameters
 - "compress-level": set compression level during migration (json-int)
 - "compress-threads": set compression thread count for migration (json-int)
 - "decompress-threads": set decompression thread count for migration (json-int)
+- "x-cpu-throttle-initial": set initial percentage of time guest cpus are
+   throttled for auto-converge (json-int)
+- "x-cpu-throttle-increment": set throttle increasing percentage for
+ auto-converge (json-int)
 
 Arguments:
 
@@ -3713,7 +3717,7 @@ EQMP
 {
 .name   = "migrate-set-parameters",
 .args_type  =
-"compress-level:i?,compress-threads:i?,decompress-threads:i?",
+
"compress-level:i?,compress-threads:i?,decompress-threads:i?,x-cpu-throttle-initial:i?,x-cpu-throttle-increment:i?",
 .mhandler.cmd_new = qmp_marshal_migrate_set_parameters,
 },
 SQMP
@@ -3726,6 +3730,10 @@ Query current migration parameters
  - "compress-level" : compression level value (json-int)
  - "compress-threads" : compression thread count value (json-int)
  - "decompress-threads" : decompression thread count value (json-int)
+ - "x-cpu-throttle-initial" : initial percentage of time guest cpus are
+  throttled (json-int)
+ - "x-cpu-throttle-increment" : throttle increasing percentage for
+auto-converge (json-int)
 
 Arguments:
 
@@ -3734,9 +3742,11 @@ Example:
 -> { "execute": "query-migrate-parameters" }
 <- {
   "return": {
- "decompress-threads", 2,
- "compress-threads", 8,
- "compress-level", 1
+ "decompress-threads": 2,
+ "x-cpu-throttle-increment": 10,
+ "compress-threads": 8,
+ "compress-level": 1,
+ "x-cpu-throttle-initial": 20
   }
}
 
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 6/6] qmp-commands.hx: Document the missing options for migration capability commands

[zhanghailiang]

Add the missing descriptions for the options of migration capability commands,
and fix the example for query-migrate-capabilities command.

Signed-off-by: zhanghailiang 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Amit Shah 
---
 qmp-commands.hx | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/qmp-commands.hx b/qmp-commands.hx
index b25b97c..020e5ee 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3647,7 +3647,9 @@ Enable/Disable migration capabilities
 - "rdma-pin-all": pin all pages when using RDMA during migration
 - "auto-converge": throttle down guest to help convergence of migration
 - "zero-blocks": compress zero blocks during block migration
+- "compress": use multiple compression threads to accelerate live migration
 - "events": generate events for each migration state change
+- "x-postcopy-ram": postcopy mode for live migration
 
 Arguments:
 
@@ -3675,13 +3677,24 @@ Query current migration capabilities
  - "rdma-pin-all" : RDMA Pin Page state (json-bool)
  - "auto-converge" : Auto Converge state (json-bool)
  - "zero-blocks" : Zero Blocks state (json-bool)
+ - "compress": Multiple compression threads state (json-bool)
+ - "events": Migration state change event state (json-bool)
+ - "x-postcopy-ram": postcopy ram state (json-bool)
 
 Arguments:
 
 Example:
 
 -> { "execute": "query-migrate-capabilities" }
-<- { "return": [ { "state": false, "capability": "xbzrle" } ] }
+<- {"return": [
+ {"state": false, "capability": "xbzrle"},
+ {"state": false, "capability": "rdma-pin-all"},
+ {"state": false, "capability": "auto-converge"},
+ {"state": false, "capability": "zero-blocks"},
+ {"state": false, "capability": "compress"},
+ {"state": true, "capability": "events"},
+ {"state": false, "capability": "x-postcopy-ram"}
+   ]}
 
 EQMP
 
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 4/5] virtio-balloon: rewrite get_current_ram_size()

[Cornelia Huck]

On Thu,  4 Feb 2016 14:37:22 +0300
Vladimir Sementsov-Ogievskiy  wrote:

> Use pc_dimm_built_list() instead of qmp_pc_dimm_device_list()
> 
> Actually, Qapi is not related to this internal helper.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> 
> CC: Stefan Hajnoczi 
> CC: Xiao Guangrong 
> CC: "Michael S. Tsirkin" 
> CC: Igor Mammedov 
> CC: Eric Blake 
> CC: Markus Armbruster 
> ---
>  hw/virtio/virtio-balloon.c | 23 ++-
>  1 file changed, 6 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index 6a4c4d2..b9c1964 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -296,26 +296,15 @@ static void virtio_balloon_get_config(VirtIODevice 
> *vdev, uint8_t *config_data)
> 
>  static ram_addr_t get_current_ram_size(void)
>  {
> -MemoryDeviceInfoList *info_list = NULL;
> -MemoryDeviceInfoList **prev = _list;
> -MemoryDeviceInfoList *info;
> +GSList *list = NULL, *item;
>  ram_addr_t size = ram_size;
> 
> -qmp_pc_dimm_device_list(qdev_get_machine(), );
> -for (info = info_list; info; info = info->next) {
> -MemoryDeviceInfo *value = info->value;
> -
> -if (value) {
> -switch (value->type) {
> -case MEMORY_DEVICE_INFO_KIND_DIMM:
> -size += value->u.dimm->size;
> -break;
> -default:
> -break;
> -}
> -}
> +pc_dimm_build_list(qdev_get_machine(), );

This will break the build for !CONFIG_MEM_HOTPLUG, as you didn't
provide a stubbed-out version of this function.

> +for (item = list; item; item = g_slist_next(item)) {
> +PCDIMMDevice *dimm = item->data;
> +size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, 
> NULL);
>  }
> -qapi_free_MemoryDeviceInfoList(info_list);
> +g_slist_free(list);
> 
>  return size;
>  }

Re: [Qemu-devel] [PATCH] migration: remove useless code.

[Amit Shah]

On (Wed) 27 Jan 2016 [14:11:05], Liang Li wrote:
> Since 's->state' will be set in migrate_init(), there is no
> need to set it before calling migrate_init(). The code and
> the related comments can be removed.
> 
> Signed-off-by: Liang Li 
> ---
>  migration/migration.c | 6 --
>  1 file changed, 6 deletions(-)
> 
> diff --git a/migration/migration.c b/migration/migration.c
> index aaca451..ae38242 100644
> --- a/migration/migration.c
> +++ b/migration/migration.c
> @@ -1006,12 +1006,6 @@ void qmp_migrate(const char *uri, bool has_blk, bool 
> blk,
>  return;
>  }
>  
> -/* We are starting a new migration, so we want to start in a clean
> -   state.  This change is only needed if previous migration
> -   failed/was cancelled.  We don't use migrate_set_state() because
> -   we are setting the initial state, not changing it. */
> -s->state = MIGRATION_STATUS_NONE;
> -

Looks fine.  This was added to ensure a proper init value, but commit
389775d added the init in migrate_init, so this is now redundant.

Reviewed-by: Amit Shah 


Amit

Re: [Qemu-devel] [PATCH 5/6] qmp-commands.hx: Fix the missing options for migration parameters commands

[Amit Shah]

On (Fri) 15 Jan 2016 [11:37:45], zhanghailiang wrote:
> We didn't document x-cpu-throttle-initial/x-cpu-throttle-increment for
> commands migrate-set-parameters and query-migrate-parameters.
> 
> Here we add the descriptions for these two options and fix the wrong example
> for query-migrate-parameters qmp commands.
> Besides, this will also fix the bug that we can't set x-cpu-throttle-initial
> and x-cpu-throttle-increment through migrate-set-parameters qmp command.
> 
> Signed-off-by: zhanghailiang 
> ---
>  qmp-commands.hx | 18 ++
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index db072a6..0a2cded 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -3700,6 +3700,10 @@ Set migration parameters
>  - "compress-level": set compression level during migration (json-int)
>  - "compress-threads": set compression thread count for migration (json-int)
>  - "decompress-threads": set decompression thread count for migration 
> (json-int)
> +- "x-cpu-throttle-initial": set initial percentage of time guest cpus are
> +   throttled for auto-coverge (json-int)

typo, should be converge.  No need to re-send the series again unless
a respin is required.

> +- "x-cpu-throttle-increment": set throttle increasing percentage for
> + auto-converge (json-int)

Amit

Re: [Qemu-devel] [PATCH 0/6] Some improvements and small fixes for migration

[Amit Shah]

On (Fri) 15 Jan 2016 [11:37:40], zhanghailiang wrote:
> Patch 1 ~ patch 4 are picked from COLO and live memory snapshot series,
> They are just small improvements for migration codes and have been reviewed
> by Dave.
> 
> Patch 5, 6 are small fixes for migration releated documention. 
> 
> Please review.

Reviewed-by: Amit Shah 

Just provide a better commit text for patch 2 and I'll apply.

Thanks,

Amit

Re: [Qemu-devel] qdev & hw/core owner? (was Re: [PATCH v19 7/9] machine: add properties to compat_props incrementaly)

[Paolo Bonzini]

On 03/02/2016 20:06, Michael S. Tsirkin wrote:
> On Wed, Feb 03, 2016 at 03:55:04PM -0200, Eduardo Habkost wrote:
>> On Thu, Jan 28, 2016 at 06:00:31PM +0100, Igor Mammedov wrote:
>> [...]
>>> It looks like this series might go nowhere but this patch
>>> is not tied to it and useful to us in general
>>> so perhaps you could pick it up after ACKs from
>>> S390/SPAPR maintainers.
>>>

 Reviewed-by: Eduardo Habkost 
>>
>> We don't have a maintainer for hw/core/machine.c, hw/core/qdev*,
>> and related files.
>>
>> Assuming we don't have a volunteer to maintain them officially,
>> can we agree on a default destination for those patches so they
>> don't linger on the list? Michael? Andreas?
> 
> Not me please. Have too much on my plate.
> Would you like to maintain it yourself?

That's my suggestion too.  I guess Igor and I could help with reviews,
but testing and sending the pull requests would add too much work.
Since you're the main one touching it, it makes sense for you to handle it.

Paolo

[Qemu-devel] [PATCH 0/9] pc: do not create invalid MADT.LAPIC/Processor entries

[Igor Mammedov]

It's mostly clean up series that removes invalid CPU
entries from MADT/DSDT/SRAT tables when APIC IDs are
sparse distributed*.
Series also removes intermediate present CPUs bitmap
in ACPI tables generation code, replacing it with
machine reported presence status or if not possible*
with directly querying CPU for presence. That should
help later for consolidating and sharing CPU hotplug
codebase and extending supported CPU count above 256
on ACPI side, where I'm going to replace current
"not scalable" bitmap based CPU hotplug MMIO interface
with memory-hotplug like one, which could easily
scale and provide additional info for ACPI CPU device
objects.

Series depends on cotextual changes in following post:
'[PATCH 0/3] merge SSDT into DSDT'
https://www.mail-archive.com/qemu-devel@nongnu.org/msg347960.html

* example topology with sparse APIC IDs:
  -smp X,sockets=2,cores=3,maxcpus=6

* it's not possible to remove notion of apic_ad_limit
  since guest visible interfaces like CPU hoptlug MMIO
  (CPON array in ACPI + corresponding MMIO in QEMU) and
  FWCFG should stay the same for compat reasons with
  current setups and legacy SeaBIOS.

Igor Mammedov (9):
  cpu: rename cpu_exists() to qemu_get_cpu_by_arch_id()
  machine: introduce MachineClass.possible_cpu_arch_ids() hook
  pc: acpi: cleanup qdev_get_machine() calls
  pc: acpi: SRAT: create only valid processor lapic entries
  pc: acpi: create Processor and Notify objects only for valid lapics
  pc: acpi: create MADT.lapic entries only for valid lapics
  pc: acpi: drop not needed intermediate bitmap cpu->found_cpus
  pc: move apic_id_limit to PCMachineState
  pc: acpi: clarify why possible LAPIC entries must be present in MADT

 hw/i386/acpi-build.c | 138 ++-
 hw/i386/pc.c |  67 ++---
 include/hw/boards.h  |  18 +++
 include/hw/i386/pc.h |   4 +-
 include/qom/cpu.h|  10 ++--
 qom/cpu.c|   6 +--
 target-i386/cpu.c|   2 +-
 7 files changed, 138 insertions(+), 107 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 3/9] pc: acpi: cleanup qdev_get_machine() calls

[Igor Mammedov]

call qdev_get_machine() once at acpi_build() and
pass its result to child functions that need it.

Signed-off-by: Igor Mammedov 
---
Later it will also allow to reduce number of arguments
passed around by build_foo() functions called from
acpi_build() once guest_info fields are moved into
PCMachineState.
---
 hw/i386/acpi-build.c | 20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 2028ed7..faf541c 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1937,13 +1937,13 @@ static Aml *build_q35_osc_method(void)
 static void
 build_dsdt(GArray *table_data, GArray *linker,
AcpiCpuInfo *cpu, AcpiPmInfo *pm, AcpiMiscInfo *misc,
-   PcPciInfo *pci, PcGuestInfo *guest_info)
+   PcPciInfo *pci, PcGuestInfo *guest_info,
+   MachineState *machine)
 {
 CrsRangeEntry *entry;
 Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs;
 GPtrArray *mem_ranges = g_ptr_array_new_with_free_func(crs_range_free);
 GPtrArray *io_ranges = g_ptr_array_new_with_free_func(crs_range_free);
-MachineState *machine = MACHINE(qdev_get_machine());
 uint32_t nr_mem = machine->ram_slots;
 int root_bus_limit = 0xFF;
 PCIBus *bus = NULL;
@@ -2365,7 +2365,8 @@ acpi_build_srat_memory(AcpiSratMemoryAffinity *numamem, 
uint64_t base,
 }
 
 static void
-build_srat(GArray *table_data, GArray *linker, PcGuestInfo *guest_info)
+build_srat(GArray *table_data, GArray *linker, PcGuestInfo *guest_info,
+   MachineState *machine)
 {
 AcpiSystemResourceAffinityTable *srat;
 AcpiSratProcessorAffinity *core;
@@ -2375,7 +2376,7 @@ build_srat(GArray *table_data, GArray *linker, 
PcGuestInfo *guest_info)
 uint64_t curnode;
 int srat_start, numa_start, slots;
 uint64_t mem_len, mem_base, next_base;
-PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
+PCMachineState *pcms = PC_MACHINE(machine);
 ram_addr_t hotplugabble_address_space_size =
 object_property_get_int(OBJECT(pcms), PC_MACHINE_MEMHP_REGION_SIZE,
 NULL);
@@ -2579,9 +2580,9 @@ static bool acpi_has_iommu(void)
 return intel_iommu && !ambiguous;
 }
 
-static bool acpi_has_nvdimm(void)
+static bool acpi_has_nvdimm(MachineState *machine)
 {
-PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
+PCMachineState *pcms = PC_MACHINE(machine);
 
 return pcms->nvdimm;
 }
@@ -2599,6 +2600,7 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables 
*tables)
 uint8_t *u;
 size_t aml_len = 0;
 GArray *tables_blob = tables->table_data;
+MachineState *machine = MACHINE(qdev_get_machine());
 
 acpi_get_cpu_info();
 acpi_get_pm_info();
@@ -2624,7 +2626,7 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables 
*tables)
 /* DSDT is pointed to by FADT */
 dsdt = tables_blob->len;
 build_dsdt(tables_blob, tables->linker, , , , ,
-   guest_info);
+   guest_info, machine);
 
 /* Count the size of the DSDT and SSDT, we will need it for legacy
  * sizing of ACPI tables.
@@ -2655,7 +2657,7 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables 
*tables)
 }
 if (guest_info->numa_nodes) {
 acpi_add_table(table_offsets, tables_blob);
-build_srat(tables_blob, tables->linker, guest_info);
+build_srat(tables_blob, tables->linker, guest_info, machine);
 }
 if (acpi_get_mcfg()) {
 acpi_add_table(table_offsets, tables_blob);
@@ -2666,7 +2668,7 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables 
*tables)
 build_dmar_q35(tables_blob, tables->linker);
 }
 
-if (acpi_has_nvdimm()) {
+if (acpi_has_nvdimm(machine)) {
 nvdimm_build_acpi(table_offsets, tables_blob, tables->linker);
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 7/9] pc: acpi: drop not needed intermediate bitmap cpu->found_cpus

[Igor Mammedov]

cpu->found_cpus bitmap is used for setting present
flag in CPON AML package at start up. But it takes
a bunch of code to fill bitmap and cloud be simplified
by calling qemu_get_cpu_by_arch_id(apic_id) directly.

Hence do so and remove not used anymore bitmap
with related utilities, which saves us ~32LOC
and also would simplify consolidating APCI parts
of CPU hotplug.

Signed-off-by: Igor Mammedov 
---
 hw/i386/acpi-build.c | 42 +-
 1 file changed, 5 insertions(+), 37 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 9eeeffa..921830e 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -76,10 +76,6 @@
 #define ACPI_BUILD_DPRINTF(fmt, ...)
 #endif
 
-typedef struct AcpiCpuInfo {
-DECLARE_BITMAP(found_cpus, ACPI_CPU_HOTPLUG_ID_LIMIT);
-} AcpiCpuInfo;
-
 typedef struct AcpiMcfgInfo {
 uint64_t mcfg_base;
 uint32_t mcfg_size;
@@ -121,31 +117,6 @@ typedef struct AcpiBuildPciBusHotplugState {
 bool pcihp_bridge_en;
 } AcpiBuildPciBusHotplugState;
 
-static
-int acpi_add_cpu_info(Object *o, void *opaque)
-{
-AcpiCpuInfo *cpu = opaque;
-uint64_t apic_id;
-
-if (object_dynamic_cast(o, TYPE_CPU)) {
-apic_id = object_property_get_int(o, "apic-id", NULL);
-assert(apic_id < ACPI_CPU_HOTPLUG_ID_LIMIT);
-
-set_bit(apic_id, cpu->found_cpus);
-}
-
-object_child_foreach(o, acpi_add_cpu_info, opaque);
-return 0;
-}
-
-static void acpi_get_cpu_info(AcpiCpuInfo *cpu)
-{
-Object *root = object_get_root();
-
-memset(cpu->found_cpus, 0, sizeof cpu->found_cpus);
-object_child_foreach(root, acpi_add_cpu_info, cpu);
-}
-
 static void acpi_get_pm_info(AcpiPmInfo *pm)
 {
 Object *piix = piix4_pm_find();
@@ -967,8 +938,7 @@ static Aml *build_crs(PCIHostState *host,
 }
 
 static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus,
-AcpiCpuInfo *cpu, AcpiPmInfo *pm,
-MachineState *machine)
+AcpiPmInfo *pm, MachineState *machine)
 {
 int i;
 Aml *dev;
@@ -1063,7 +1033,7 @@ static void build_processor_devices(Aml *sb_scope, 
unsigned acpi_cpus,
  aml_varpackage(acpi_cpus);
 
 for (i = 0; i < acpi_cpus; i++) {
-uint8_t b = test_bit(i, cpu->found_cpus) ? 0x01 : 0x00;
+uint8_t b = qemu_get_cpu_by_arch_id(i) ? 0x01 : 0x00;
 aml_append(pkg, aml_int(b));
 }
 aml_append(sb_scope, aml_name_decl(CPU_ON_BITMAP, pkg));
@@ -1955,7 +1925,7 @@ static Aml *build_q35_osc_method(void)
 
 static void
 build_dsdt(GArray *table_data, GArray *linker,
-   AcpiCpuInfo *cpu, AcpiPmInfo *pm, AcpiMiscInfo *misc,
+   AcpiPmInfo *pm, AcpiMiscInfo *misc,
PcPciInfo *pci, PcGuestInfo *guest_info,
MachineState *machine)
 {
@@ -2263,7 +2233,7 @@ build_dsdt(GArray *table_data, GArray *linker,
 
 sb_scope = aml_scope("\\_SB");
 {
-build_processor_devices(sb_scope, guest_info->apic_id_limit, cpu, pm,
+build_processor_devices(sb_scope, guest_info->apic_id_limit, pm,
 machine);
 
 build_memory_devices(sb_scope, nr_mem, pm->mem_hp_io_base,
@@ -2618,7 +2588,6 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables 
*tables)
 {
 GArray *table_offsets;
 unsigned facs, dsdt, rsdt, fadt;
-AcpiCpuInfo cpu;
 AcpiPmInfo pm;
 AcpiMiscInfo misc;
 AcpiMcfgInfo mcfg;
@@ -2628,7 +2597,6 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables 
*tables)
 GArray *tables_blob = tables->table_data;
 MachineState *machine = MACHINE(qdev_get_machine());
 
-acpi_get_cpu_info();
 acpi_get_pm_info();
 acpi_get_misc_info();
 acpi_get_pci_info();
@@ -2651,7 +2619,7 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables 
*tables)
 
 /* DSDT is pointed to by FADT */
 dsdt = tables_blob->len;
-build_dsdt(tables_blob, tables->linker, , , , ,
+build_dsdt(tables_blob, tables->linker, , , ,
guest_info, machine);
 
 /* Count the size of the DSDT and SSDT, we will need it for legacy
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 0/6] Some improvements and small fixes for migration

[Hailiang Zhang]

On 2016/2/4 19:08, Amit Shah wrote:

On (Fri) 15 Jan 2016 [11:37:40], zhanghailiang wrote:

Patch 1 ~ patch 4 are picked from COLO and live memory snapshot series,
They are just small improvements for migration codes and have been reviewed
by Dave.

Patch 5, 6 are small fixes for migration releated documention.

Please review.


Reviewed-by: Amit Shah 

Just provide a better commit text for patch 2 and I'll apply.



OK, i will send v2.

Thanks,
Hailiang


Thanks,

Amit

.

Re: [Qemu-devel] [PATCH v2 0/6] Some improvments and small fixes for migration

[Amit Shah]

On (Thu) 04 Feb 2016 [20:20:20], zhanghailiang wrote:
> Patch 1 ~ patch 4 are picked from COLO and live memory snapshot series,
> They are just small improvements for migration codes.
> 
> Patch 5, 6 are small fixes for migration releated documention. 
> 
> v2:
>  - Fix a typo (Amit)
>  - A better commit description from Dave for patch 2

Thanks; I've already queued the v1 with these fixes.  Watch for the
pull req tomorrow.

Amit

Re: [Qemu-devel] [PATCH 2/6] migration: Rename the'file' member of MigrationState

[Dr. David Alan Gilbert]

* Amit Shah (amit.s...@redhat.com) wrote:
> On (Thu) 04 Feb 2016 [10:50:16], Dr. David Alan Gilbert wrote:
> > * Amit Shah (amit.s...@redhat.com) wrote:
> > > On (Fri) 15 Jan 2016 [11:37:42], zhanghailiang wrote:
> > > > Rename the 'file' member of MigrationState to 'to_dst_file'.
> > > 
> > > Why?
> > > 
> > > (Use this space to explain why you're doing it, rather than repeating
> > > the first line of the commit.  It helps when reviewing the git logs
> > > later.)
> > 
> > I think the simple answer is consistency; now we've got a bidirectional
> > connection we've already got to_src_file, from_src_file and from_dst_file,
> > so 'file' is an oddity that I was too lazy to rename previously.
> 
> Sure, I just mean we need better commit description.  Just provide
> one, and I'll use that when doing the pull req.

I suggest:

Rename the 'file' memory of MigrationState to 'to_dst_file' to
be consistent with to_src_file, from_src_file and from_dst_file.

Dave

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

[Qemu-devel] [PULL] Update OpenBIOS images

[Mark Cave-Ayland]

Hi Peter,

This pull request updates the OpenBIOS images to latest SVN trunk, most notably 
with fixes for Mac machines
following more testing on OS X/OS 9. Please pull.


ATB,

Mark.


The following changes since commit 87574621b18f86eab295a2c207e0b42c77b5dfa0:

  Merge remote-tracking branch 'remotes/kraxel/tags/pull-vga-20160203-1' into 
staging (2016-02-03 12:23:48 +)

are available in the git repository at:


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

for you to fetch changes up to 44c44eceea6e3705bf7bf8295fde27d1f8a73992:

  Update OpenBIOS images (2016-02-04 11:17:44 +)


Update OpenBIOS images


Mark Cave-Ayland (1):
  Update OpenBIOS images

 pc-bios/openbios-ppc |  Bin 746588 -> 750684 bytes
 pc-bios/openbios-sparc32 |  Bin 381584 -> 381584 bytes
 pc-bios/openbios-sparc64 |  Bin 1616864 -> 1592280 bytes
 roms/openbios|2 +-
 4 files changed, 1 insertion(+), 1 deletion(-)

[Qemu-devel] [PATCH 9/9] pc: acpi: clarify why possible LAPIC entries must be present in MADT

[Igor Mammedov]

Signed-off-by: Igor Mammedov 
---
 hw/i386/acpi-build.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index d6cd06a..2cc3fff 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -361,6 +361,12 @@ build_madt(GArray *table_data, GArray *linker,
 if (id.cpu != NULL) {
 apic->flags = cpu_to_le32(1);
 } else {
+/* ACPI spec says that LAPIC entry for non present
+ * CPU may be omitted from MADT or it must be marked
+ * as disabled. However omitting non present CPU from
+ * MADT breaks hotplug on linux. So possible CPUs
+ * should be put in MADT but kept disabled.
+ */
 apic->flags = cpu_to_le32(0);
 }
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH 8/9] pc: move apic_id_limit to PCMachineState

[Igor Mammedov]

yet another cleanup that replaces multiple apic_id_limit
variables that were initialized at differnt places with
a one initialization at pc_cpus_init() time and letting
other code to just read it. That also allows to reduce
number of a function arguments where it takes
MachineState as an argument.

Signed-off-by: Igor Mammedov 
---
 hw/i386/acpi-build.c | 14 +++---
 hw/i386/pc.c | 49 +
 include/hw/i386/pc.h |  4 +++-
 3 files changed, 31 insertions(+), 36 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 921830e..d6cd06a 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -937,7 +937,7 @@ static Aml *build_crs(PCIHostState *host,
 return crs;
 }
 
-static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus,
+static void build_processor_devices(Aml *sb_scope,
 AcpiPmInfo *pm, MachineState *machine)
 {
 int i;
@@ -948,12 +948,13 @@ static void build_processor_devices(Aml *sb_scope, 
unsigned acpi_cpus,
 Aml *ifctx;
 Aml *method;
 MachineClass *mc = MACHINE_GET_CLASS(machine);
+PCMachineState *pcms = PC_MACHINE(machine);
 GArray *apic_id_list = mc->possible_cpu_arch_ids();
 
 /* The current AML generator can cover the APIC ID range [0..255],
  * inclusive, for VCPU hotplug. */
 QEMU_BUILD_BUG_ON(ACPI_CPU_HOTPLUG_ID_LIMIT > 256);
-g_assert(acpi_cpus <= ACPI_CPU_HOTPLUG_ID_LIMIT);
+g_assert(pcms->apic_id_limit <= ACPI_CPU_HOTPLUG_ID_LIMIT);
 
 /* create PCI0.PRES device and its _CRS to reserve CPU hotplug MMIO */
 dev = aml_device("PCI0." stringify(CPU_HOTPLUG_RESOURCE_DEVICE));
@@ -1029,10 +1030,10 @@ static void build_processor_devices(Aml *sb_scope, 
unsigned acpi_cpus,
  * ith up to 255 elements. Windows guests up to win2k8 fail when
  * VarPackageOp is used.
  */
-pkg = acpi_cpus <= 255 ? aml_package(acpi_cpus) :
- aml_varpackage(acpi_cpus);
+pkg = pcms->apic_id_limit <= 255 ? aml_package(pcms->apic_id_limit) :
+   aml_varpackage(pcms->apic_id_limit);
 
-for (i = 0; i < acpi_cpus; i++) {
+for (i = 0; i < pcms->apic_id_limit; i++) {
 uint8_t b = qemu_get_cpu_by_arch_id(i) ? 0x01 : 0x00;
 aml_append(pkg, aml_int(b));
 }
@@ -2233,8 +2234,7 @@ build_dsdt(GArray *table_data, GArray *linker,
 
 sb_scope = aml_scope("\\_SB");
 {
-build_processor_devices(sb_scope, guest_info->apic_id_limit, pm,
-machine);
+build_processor_devices(sb_scope, pm, machine);
 
 build_memory_devices(sb_scope, nr_mem, pm->mem_hp_io_base,
  pm->mem_hp_io_len);
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 2fd8fc8..61fbb11 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -700,18 +700,6 @@ static uint32_t x86_cpu_apic_id_from_index(unsigned int 
cpu_index)
 }
 }
 
-/* Calculates the limit to CPU APIC ID values
- *
- * This function returns the limit for the APIC ID value, so that all
- * CPU APIC IDs are < pc_apic_id_limit().
- *
- * This is used for FW_CFG_MAX_CPUS. See comments on bochs_bios_init().
- */
-static unsigned int pc_apic_id_limit(unsigned int max_cpus)
-{
-return x86_cpu_apic_id_from_index(max_cpus - 1) + 1;
-}
-
 static void pc_build_smbios(FWCfgState *fw_cfg)
 {
 uint8_t *smbios_tables, *smbios_anchor;
@@ -749,12 +737,11 @@ static void pc_build_smbios(FWCfgState *fw_cfg)
 }
 }
 
-static FWCfgState *bochs_bios_init(AddressSpace *as)
+static FWCfgState *bochs_bios_init(AddressSpace *as, PCMachineState *pcms)
 {
 FWCfgState *fw_cfg;
 uint64_t *numa_fw_cfg;
 int i, j;
-unsigned int apic_id_limit = pc_apic_id_limit(max_cpus);
 
 fw_cfg = fw_cfg_init_io_dma(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 4, as);
 
@@ -772,7 +759,7 @@ static FWCfgState *bochs_bios_init(AddressSpace *as)
  * [1] The only kind of "CPU identifier" used between SeaBIOS and QEMU is
  * the APIC ID, not the "CPU index"
  */
-fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)apic_id_limit);
+fw_cfg_add_i16(fw_cfg, FW_CFG_MAX_CPUS, (uint16_t)pcms->apic_id_limit);
 fw_cfg_add_i64(fw_cfg, FW_CFG_RAM_SIZE, (uint64_t)ram_size);
 fw_cfg_add_bytes(fw_cfg, FW_CFG_ACPI_TABLES,
  acpi_tables, acpi_tables_len);
@@ -790,11 +777,11 @@ static FWCfgState *bochs_bios_init(AddressSpace *as)
  * of nodes, one word for each VCPU->node and one word for each node to
  * hold the amount of memory.
  */
-numa_fw_cfg = g_new0(uint64_t, 1 + apic_id_limit + nb_numa_nodes);
+numa_fw_cfg = g_new0(uint64_t, 1 + pcms->apic_id_limit + nb_numa_nodes);
 numa_fw_cfg[0] = cpu_to_le64(nb_numa_nodes);
 for (i = 0; i < max_cpus; i++) {
 unsigned int apic_id = x86_cpu_apic_id_from_index(i);
-assert(apic_id < apic_id_limit);
+assert(apic_id < 

Re: [Qemu-devel] [PATCH] kvm-all: trace: strerror fixup

[Paolo Bonzini]

On 01/02/2016 20:37, Andrew Jones wrote:
> Signed-off-by: Andrew Jones 
> ---
>  kvm-all.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kvm-all.c b/kvm-all.c
> index 9148889921197..330f509a0da84 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -2362,7 +2362,7 @@ int kvm_set_one_reg(CPUState *cs, uint64_t id, void 
> *source)
>  reg.addr = (uintptr_t) source;
>  r = kvm_vcpu_ioctl(cs, KVM_SET_ONE_REG, );
>  if (r) {
> -trace_kvm_failed_reg_set(id, strerror(r));
> +trace_kvm_failed_reg_set(id, strerror(-r));
>  }
>  return r;
>  }
> @@ -2376,7 +2376,7 @@ int kvm_get_one_reg(CPUState *cs, uint64_t id, void 
> *target)
>  reg.addr = (uintptr_t) target;
>  r = kvm_vcpu_ioctl(cs, KVM_GET_ONE_REG, );
>  if (r) {
> -trace_kvm_failed_reg_get(id, strerror(r));
> +trace_kvm_failed_reg_get(id, strerror(-r));
>  }
>  return r;
>  }
> 

Applied, thanks!

Paolo

[Qemu-devel] [PATCH v2 06/11] block: m25p80: Add configuration registers

[marcin . krzeminski]

From: Marcin Krzeminski 

This patch adds both volatile and non volatile configuration registers
and commands to allow modify them. It is needed for proper handling
dummy cycles. Power-on initialization of those registers and flash state
has been included as well.
Some of this registers are used by kernel.

Signed-off-by: Marcin Krzeminski 
---
 hw/block/m25p80.c | 109 ++
 1 file changed, 109 insertions(+)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index e306356..a213587 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -26,6 +26,7 @@
 #include "sysemu/block-backend.h"
 #include "sysemu/blockdev.h"
 #include "hw/ssi/ssi.h"
+#include "qemu/bitops.h"
 
 #ifndef M25P80_ERR_DEBUG
 #define M25P80_ERR_DEBUG 0
@@ -245,6 +246,15 @@ typedef enum {
 
 RESET_ENABLE = 0x66,
 RESET_MEMORY = 0x99,
+
+RNVCR = 0xB5,
+WNVCR = 0xB1,
+
+RVCR = 0x85,
+WVCR = 0x81,
+
+REVCR = 0x65,
+WEVCR = 0x61,
 } FlashCMD;
 
 typedef enum {
@@ -271,6 +281,9 @@ typedef struct Flash {
 uint8_t needed_bytes;
 uint8_t cmd_in_progress;
 uint64_t cur_addr;
+uint32_t nonvolatile_cfg;
+uint32_t volatile_cfg;
+uint32_t enh_volatile_cfg;
 bool write_enable;
 bool four_bytes_address_mode;
 bool reset_enable;
@@ -460,6 +473,15 @@ static void complete_collecting_data(Flash *s)
 case EXTEND_ADDR_WRITE:
 s->ear = s->data[0];
 break;
+case WNVCR:
+s->nonvolatile_cfg = s->data[0] | (s->data[1] << 8);
+break;
+case WVCR:
+s->volatile_cfg = s->data[0];
+break;
+case WEVCR:
+s->enh_volatile_cfg = s->data[0];
+break;
 default:
 break;
 }
@@ -618,6 +640,49 @@ static void decode_new_cmd(Flash *s, uint32_t value)
 s->state = STATE_COLLECTING_DATA;
 }
 break;
+case RNVCR:
+s->data[0] = s->nonvolatile_cfg & 0xFF;
+s->data[1] = (s->nonvolatile_cfg >> 8) & 0xFF;
+s->pos = 0;
+s->len = 1;
+s->state = STATE_READING_DATA;
+break;
+case WNVCR:
+if (s->write_enable) {
+s->needed_bytes = 2;
+s->pos = 0;
+s->len = 0;
+s->state = STATE_COLLECTING_DATA;
+}
+break;
+case RVCR:
+s->data[0] = s->volatile_cfg & 0xFF;
+s->pos = 0;
+s->len = 1;
+s->state = STATE_READING_DATA;
+break;
+case WVCR:
+if (s->write_enable) {
+s->needed_bytes = 1;
+s->pos = 0;
+s->len = 0;
+s->state = STATE_COLLECTING_DATA;
+}
+break;
+case REVCR:
+s->data[0] = s->enh_volatile_cfg & 0xFF;
+s->pos = 0;
+s->len = 1;
+s->state = STATE_READING_DATA;
+break;
+case WEVCR:
+if (s->write_enable) {
+s->needed_bytes = 1;
+s->pos = 0;
+s->len = 0;
+s->state = STATE_COLLECTING_DATA;
+}
+break;
 case RESET_ENABLE:
 s->reset_enable = true;
 break;
@@ -710,6 +775,41 @@ static int m25p80_init(SSISlave *ss)
 
 reset_memory(s);
 
+s->volatile_cfg = 0;
+/* WRAP & reserved*/
+s->volatile_cfg |= 0x3;
+/* XIP */
+if (extract32(s->nonvolatile_cfg,9,3) != 0x7)
+{
+s->volatile_cfg |= (1 << 3);
+}
+/* Number of dummy cycles */
+s->volatile_cfg |= 
deposit32(s->volatile_cfg,4,4,extract32(s->nonvolatile_cfg,12,4));
+
+s->enh_volatile_cfg = 0;
+/* Output driver strength */
+s->enh_volatile_cfg |= 0x7;
+/* Vpp accelerator */
+s->enh_volatile_cfg |= (1 << 3);
+/* Reset/hold & reserved */
+s->enh_volatile_cfg |= (1 << 4);
+/* Dual I/O protocol */
+if ( (s->nonvolatile_cfg >> 1) & 0x1) {
+s->enh_volatile_cfg |= (1 << 6);
+}
+/* Quad I/O protocol */
+if ( (s->nonvolatile_cfg >> 3) & 0x1) {
+s->enh_volatile_cfg |= (1 << 7);
+}
+
+if ( !(s->nonvolatile_cfg & 0x1)) {
+s->four_bytes_address_mode = true;
+}
+
+if ( !((s->nonvolatile_cfg >> 1) & 0x1)) {
+s->ear = 0x3;
+}
+
 /* FIXME use a qdev drive property instead of drive_get_next() */
 dinfo = drive_get_next(IF_MTD);
 
@@ -740,6 +840,11 @@ static void m25p80_pre_save(void *opaque)
 flash_sync_dirty((Flash *)opaque, -1);
 }
 
+static Property m25p80_properties[] = {
+DEFINE_PROP_UINT32("nonvolatile-cfg", Flash, nonvolatile_cfg, 0x8FFF),
+DEFINE_PROP_END_OF_LIST(),
+};
+
 static const VMStateDescription vmstate_m25p80 = {
 .name = "xilinx_spi",
 .version_id = 2,
@@ -757,6 +862,9 @@ static const VMStateDescription vmstate_m25p80 = {
 VMSTATE_BOOL(four_bytes_address_mode, Flash),
 VMSTATE_UINT8(ear, Flash),
 VMSTATE_BOOL(reset_enable, Flash),
+VMSTATE_UINT32(nonvolatile_cfg, Flash),
+

[Qemu-devel] [PATCH v2 04/11] block: m25p80: Extend address mode

[marcin . krzeminski]

From: Marcin Krzeminski 

Extend address mode allows to switch flash 16 MiB banks,
allowing user to access all flash sectors.
This access mode is used by u-boot.

Signed-off-by: Marcin Krzeminski 
Reviewed-by: Peter Crosthwaite 
---
 hw/block/m25p80.c | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index f9088c5..6b5f00d 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -48,6 +48,9 @@
  */
 #define WR_1 0x100
 
+/* 16 MiB max in 3 byte address mode */
+#define MAX_3BYTES_SIZE 0x100
+
 typedef struct FlashPartInfo {
 const char *part_name;
 /* jedec code. (jedec >> 16) & 0xff is the 1st byte, >> 8 the 2nd etc */
@@ -234,6 +237,9 @@ typedef enum {
 ERASE_32K = 0x52,
 ERASE_SECTOR = 0xd8,
 
+EXTEND_ADDR_READ = 0xC8,
+EXTEND_ADDR_WRITE = 0xC5,
+
 RESET_ENABLE = 0x66,
 RESET_MEMORY = 0x99,
 } FlashCMD;
@@ -264,6 +270,7 @@ typedef struct Flash {
 uint64_t cur_addr;
 bool write_enable;
 bool reset_enable;
+uint8_t ear;
 
 int64_t dirty_page;
 
@@ -404,6 +411,7 @@ static void complete_collecting_data(Flash *s)
 s->cur_addr = s->data[0] << 16;
 s->cur_addr |= s->data[1] << 8;
 s->cur_addr |= s->data[2];
+s->cur_addr += (s->ear & 0x3) * MAX_3BYTES_SIZE;
 
 s->state = STATE_IDLE;
 
@@ -431,6 +439,9 @@ static void complete_collecting_data(Flash *s)
 s->write_enable = false;
 }
 break;
+case EXTEND_ADDR_WRITE:
+s->ear = s->data[0];
+break;
 default:
 break;
 }
@@ -440,6 +451,7 @@ static void reset_memory(Flash *s)
 {
 s->cmd_in_progress = NOP;
 s->cur_addr = 0;
+s->ear = 0;
 s->len = 0;
 s->needed_bytes = 0;
 s->pos = 0;
@@ -563,6 +575,20 @@ static void decode_new_cmd(Flash *s, uint32_t value)
 break;
 case NOP:
 break;
+case EXTEND_ADDR_READ:
+s->data[0] = s->ear;
+s->pos = 0;
+s->len = 1;
+s->state = STATE_READING_DATA;
+break;
+case EXTEND_ADDR_WRITE:
+if (s->write_enable) {
+s->needed_bytes = 1;
+s->pos = 0;
+s->len = 0;
+s->state = STATE_COLLECTING_DATA;
+}
+break;
 case RESET_ENABLE:
 s->reset_enable = true;
 break;
@@ -699,6 +725,7 @@ static const VMStateDescription vmstate_m25p80 = {
 VMSTATE_UINT8(cmd_in_progress, Flash),
 VMSTATE_UINT64(cur_addr, Flash),
 VMSTATE_BOOL(write_enable, Flash),
+VMSTATE_UINT8(ear, Flash),
 VMSTATE_BOOL(reset_enable, Flash),
 VMSTATE_END_OF_LIST()
 }
-- 
2.5.0

Re: [Qemu-devel] [PATCH 4/5] virtio-balloon: rewrite get_current_ram_size()

[Vladimir Sementsov-Ogievskiy]

On 04.02.2016 15:23, Cornelia Huck wrote:

On Thu,  4 Feb 2016 14:37:22 +0300
Vladimir Sementsov-Ogievskiy  wrote:


Use pc_dimm_built_list() instead of qmp_pc_dimm_device_list()

Actually, Qapi is not related to this internal helper.

Signed-off-by: Vladimir Sementsov-Ogievskiy 

CC: Stefan Hajnoczi 
CC: Xiao Guangrong 
CC: "Michael S. Tsirkin" 
CC: Igor Mammedov 
CC: Eric Blake 
CC: Markus Armbruster 
---
  hw/virtio/virtio-balloon.c | 23 ++-
  1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 6a4c4d2..b9c1964 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -296,26 +296,15 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, 
uint8_t *config_data)

  static ram_addr_t get_current_ram_size(void)
  {
-MemoryDeviceInfoList *info_list = NULL;
-MemoryDeviceInfoList **prev = _list;
-MemoryDeviceInfoList *info;
+GSList *list = NULL, *item;
  ram_addr_t size = ram_size;

-qmp_pc_dimm_device_list(qdev_get_machine(), );
-for (info = info_list; info; info = info->next) {
-MemoryDeviceInfo *value = info->value;
-
-if (value) {
-switch (value->type) {
-case MEMORY_DEVICE_INFO_KIND_DIMM:
-size += value->u.dimm->size;
-break;
-default:
-break;
-}
-}
+pc_dimm_build_list(qdev_get_machine(), );

This will break the build for !CONFIG_MEM_HOTPLUG, as you didn't
provide a stubbed-out version of this function.


Ok, thanks, will add.

Also: it is already broken because of pc_dimm_memory_plug and 
pc_dimm_memory_unplug






+for (item = list; item; item = g_slist_next(item)) {
+PCDIMMDevice *dimm = item->data;
+size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, NULL);
  }
-qapi_free_MemoryDeviceInfoList(info_list);
+g_slist_free(list);

  return size;
  }



--
Best regards,
Vladimir

Re: [Qemu-devel] [PATCH v5 0/7] mips/kvm: Support FPU & SIMD (MSA) in MIPS KVM guests

[Leon Alrae]

On 03/02/16 17:16, James Hogan wrote:
> Here's a v5 refresh of my FPU/MSA patchset for v2.6. Thanks to all who
> have taken the time to review it so far.
> 
> This patchset primarily adds support for FPU and MIPS SIMD Architecture
> (MSA) in MIPS KVM guests to QEMU. It depends on Linux v4.1, specifically
> my KVM patchset to add the corresponding hypervisor support to KVM
> ("[PATCH 00/20] MIPS: KVM: Guest FPU & SIMD (MSA) support").
> 
> All comments welcome.
> 
> Changes in v5:
> - Rebase on master (fixed use of uint64 in patch 5).
> - Use restore_fp_status(env) in patch 6.
> - Restore MSA FP state using restore_msa_fp_status(env) in patch 7
>   (Leon).
> 
> Changes in v4:
> - Rebase on master (dropped patch 1 & 2).
> 
> Changes in v3 (patch 6 only):
> - Fix big endian (the pointer passed to the kernel must be for the
>   actual 32-bit value, not a temporary 64-bit value, otherwise on big
>   endian systems the kernel will only interpret the upper half).
> 
> Changes in v2:
> - Moved most of patch 7 and updates to linux-headers/linux/kvm.h from
>   patches 8 and 9 into a new patch 1, which is purely for reference
>   (Paolo).
> - Add the changes to MIPS_CP0_{32,64} macros from v1 patch 7 to patch 2,
>   since the rest of that patch is now unnecessary and the change is
>   along the same lines as patch 2 (not added Leon's Reviewed-by to this
>   patch due to that non-reviewed change).
> - Fix line wrapping of kvm_mips_get_one_reg() calls from Config4 and
>   Config5 in patch 5 (Leon).
> - Change (1 << x) to (1U << x) in important places in patch 5, 8 & 9 to
>   avoid compiler undefined behaviour (Leon).
> 
> James Hogan (7):
>   mips/kvm: Remove a couple of noisy DPRINTFs
>   mips/kvm: Implement PRid CP0 register
>   mips/kvm: Implement Config CP0 registers
>   mips/kvm: Support unsigned KVM registers
>   mips/kvm: Support signed 64-bit KVM registers
>   mips/kvm: Support FPU in MIPS KVM guests
>   mips/kvm: Support MSA in MIPS KVM guests
> 
>  target-mips/kvm.c | 387 
> --
>  1 file changed, 374 insertions(+), 13 deletions(-)
> 

I've applied the series to my target-mips queue, thanks.

Leon

Re: [Qemu-devel] [PATCH v13 00/10] Block replication for continuous checkpoints

[Changlong Xie]

On 02/04/2016 05:07 PM, Dr. David Alan Gilbert wrote:

* Changlong Xie (xiecl.f...@cn.fujitsu.com) wrote:

On 02/01/2016 09:18 AM, Wen Congyang wrote:

On 01/29/2016 06:47 PM, Dr. David Alan Gilbert wrote:

* Wen Congyang (we...@cn.fujitsu.com) wrote:

On 01/29/2016 06:07 PM, Dr. David Alan Gilbert wrote:

* Wen Congyang (we...@cn.fujitsu.com) wrote:

On 01/27/2016 07:03 PM, Dr. David Alan Gilbert wrote:

Hi,
   I've got a block error if I kill the secondary.

Start both primary & secondary
kill -9 secondary qemu
x_colo_lost_heartbeat on primary

The guest sees a block error and the ext4 root switches to read-only.

I gdb'd the primary with a breakpoint on quorum_report_bad; see
backtrace below.
(This is based on colo-v2.4-periodic-mode of the framework
code with the block and network proxy merged in; so it could be my
merging but I don't think so ?)


(gdb) where
#0  quorum_report_bad (node_name=0x7f2946a0892c "node0", ret=-5, 
acb=0x7f2946cb3910, acb=0x7f2946cb3910)
 at /root/colo/jan-2016/qemu/block/quorum.c:222
#1  0x7f2943b23058 in quorum_aio_cb (opaque=, ret=)
 at /root/colo/jan-2016/qemu/block/quorum.c:315
#2  0x7f2943b311be in bdrv_co_complete (acb=0x7f2946cb3f60) at 
/root/colo/jan-2016/qemu/block/io.c:2122
#3  0x7f2943ae777d in aio_bh_call (bh=) at 
/root/colo/jan-2016/qemu/async.c:64
#4  aio_bh_poll (ctx=ctx@entry=0x7f2945b771d0) at 
/root/colo/jan-2016/qemu/async.c:92
#5  0x7f2943af5090 in aio_dispatch (ctx=0x7f2945b771d0) at 
/root/colo/jan-2016/qemu/aio-posix.c:305
#6  0x7f2943ae756e in aio_ctx_dispatch (source=, 
callback=,
 user_data=) at /root/colo/jan-2016/qemu/async.c:231
#7  0x7f293b84a79a in g_main_context_dispatch () from 
/lib64/libglib-2.0.so.0
#8  0x7f2943af3a00 in glib_pollfds_poll () at 
/root/colo/jan-2016/qemu/main-loop.c:211
#9  os_host_main_loop_wait (timeout=) at 
/root/colo/jan-2016/qemu/main-loop.c:256
#10 main_loop_wait (nonblocking=) at 
/root/colo/jan-2016/qemu/main-loop.c:504
#11 0x7f29438529ee in main_loop () at /root/colo/jan-2016/qemu/vl.c:1945
#12 main (argc=, argv=, envp=) at 
/root/colo/jan-2016/qemu/vl.c:4707

(gdb) p s->num_children
$1 = 2
(gdb) p acb->success_count
$2 = 0
(gdb) p acb->is_read
$5 = false


Sorry for the late reply.


No problem.


What it the value of acb->count?


(gdb) p acb->count
$1 = 1


Note, the count is 1, not 2. Writing to children.0 is in flight. If writing to 
children.0 successes,
the guest doesn't know this error.

If secondary host is down, you should remove quorum's children.1. Otherwise, 
you will get
I/O error event.


Is that safe?  If the secondary fails, do you always have time to issue the 
command to
remove the children.1  before the guest sees the error?


We will write to two children, and expect that writing to children.0 will 
success. If so,
the guest doesn't know this error. You just get the I/O error event.


I think children.0 is the disk, and that should be OK - so only the 
children.1/replication should
be failing - so in that case why do I see the error?


I don't know, and I will check the codes.


The 'node0' in the backtrace above is the name of the replication, so it does 
look like the error
is coming from the replication.


No, the backtrace is just report an I/O error events to the management 
application.




Anyway, I tried removing children.1 but it segfaults now, I guess the 
replication is unhappy:

(qemu) x_block_change colo-disk0 -d children.1
(qemu) x_colo_lost_heartbeat


Hmm, you should not remove the child before failover. I will check it how to 
avoid it in the codes.


  But you said 'If secondary host is down, you should remove quorum's 
children.1' - is that not
what you meant?


Yes, you should excute 'x_colo_lost_heartbeat' fist, and then excute 
'x_block_change ... -d ...'.


Hi david


Hi Xie,
   Thanks for the response.


It seems we missed 'drive_del' command, and will document it in next
version. Here is the right commands order:

{ "execute": "x-colo-lost-heartbeat" }
{ 'execute': 'x-blockdev-change', 'arguments': {'parent': 'colo-disk',
'child': 'children.1'}}
{ 'execute': 'human-monitor-command', 'arguments': {'command-line':
'drive_del x'}}


OK,  however, you should fix the seg fault if you don't issue the drive_del;
qemu should never crash.
(Also I still get the IO error in the guest if I do the x-colo-lost-heartbeat).



Here is a quick fix, i just tested for several times. It work well to me.

bugfix

Signed-off-by: Changlong Xie 

diff --git a/block/quorum.c b/block/quorum.c
index e5a7e4f..f4f1d28 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -458,6 +458,11 @@ static QuorumVoteVersion 
*quorum_get_vote_winner(QuorumVotes *votes)

 if (candidate->vote_count > max) {
 max = candidate->vote_count;
 winner = candidate;
+continue;
+}
+if (candidate->vote_count == max &&
+candidate->value.l > winner->value.l) {
+ 

Re: [Qemu-devel] [PATCH 2/6] migration: Rename the'file' member of MigrationState

[Amit Shah]

On (Fri) 15 Jan 2016 [11:37:42], zhanghailiang wrote:
> Rename the 'file' member of MigrationState to 'to_dst_file'.

Why?

(Use this space to explain why you're doing it, rather than repeating
the first line of the commit.  It helps when reviewing the git logs
later.)

Thanks,

Amit

Re: [Qemu-devel] sda abort with virtio-scsi

[Paolo Bonzini]

On 04/02/2016 07:59, Hannes Reinecke wrote:
> On 02/04/2016 12:19 AM, Paolo Bonzini wrote:
>>
>>
>> On 03/02/2016 22:46, Jim Minter wrote:
>>> I am hitting the following VM lockup issue running a VM with latest
>>> RHEL7 kernel on a host also running latest RHEL7 kernel.  FWIW I'm using
>>> virtio-scsi because I want to use discard=unmap.  I ran the VM as follows:
>>>
>>> /usr/libexec/qemu-kvm -nodefaults \
>>>   -cpu host \
>>>   -smp 4 \
>>>   -m 8192 \
>>>   -drive discard=unmap,file=vm.qcow2,id=disk1,if=none,cache=unsafe \
>>>   -device virtio-scsi-pci \
>>>   -device scsi-disk,drive=disk1 \
>>>   -netdev bridge,id=net0,br=br0 \
>>>   -device virtio-net-pci,netdev=net0,mac=$(utils/random-mac.py) \
>>>   -chardev socket,id=chan0,path=/tmp/rhev.sock,server,nowait \
>>>   -chardev socket,id=chan1,path=/tmp/qemu.sock,server,nowait \
>>>   -monitor unix:tmp/vm.sock,server,nowait \
>>>   -device virtio-serial-pci \
>>>   -device virtserialport,chardev=chan0,name=com.redhat.rhevm.vdsm \
>>>   -device virtserialport,chardev=chan1,name=org.qemu.guest_agent.0 \
>>>   -device cirrus-vga \
>>>   -vnc none \
>>>   -usbdevice tablet
>>>
>>> The host was busyish at the time, but not excessively (IMO).  Nothing
>>> untoward in the host's kernel log; host storage subsystem is fine.  I
>>> didn't get any qemu logs this time around, but I will when the issue
>>> next recurs.  The VM's full kernel log is attached; here are the
>>> highlights:
>>
>> Hannes, were you going to send a patch to disable time outs?
>>
> Rah. Didn't I do it already?
> Seems like I didn't; will be doing so shortly.
> 
>>>
>>> INFO: rcu_sched detected stalls on CPUs/tasks: { 3} (detected by 2, t=60002 
>>> jiffies, g=5253, c=5252, q=0)
>>> sending NMI to all CPUs:
>>> NMI backtrace for cpu 1
>>> CPU: 1 PID: 0 Comm: swapper/1 Not tainted 3.10.0-327.4.5.el7.x86_64 #1
>>> Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
>>> task: 88023417d080 ti: 8802341a4000 task.ti: 8802341a4000
>>> RIP: 0010:[]  [] 
>>> native_safe_halt+0x6/0x10
>>> RSP: 0018:8802341a7e98  EFLAGS: 0286
>>> RAX: ffed RBX: 8802341a4000 RCX: 0100
>>> RDX:  RSI:  RDI: 0046
>>> RBP: 8802341a7e98 R08:  R09: 
>>> R10:  R11:  R12: 0001
>>> R13: 8802341a4000 R14: 8802341a4000 R15: 
>>> FS:  () GS:88023fc8() knlGS:
>>> CS:  0010 DS:  ES:  CR0: 80050033
>>> CR2: 7f4978587008 CR3: 3645e000 CR4: 003407e0
>>> DR0:  DR1:  DR2: 
>>> DR3:  DR6: fffe0ff0 DR7: 0400
>>> Stack:
>>>  8802341a7eb8 8101dbcf 8802341a4000 81a68260
>>>  8802341a7ec8 8101e4d6 8802341a7f20 810d62e5
>>>  8802341a7fd8 8802341a4000 2581685d70de192c 7ba58fdb3a3bc8d4
>>> Call Trace:
>>>  [] default_idle+0x1f/0xc0
>>>  [] arch_cpu_idle+0x26/0x30
>>>  [] cpu_startup_entry+0x245/0x290
>>>  [] start_secondary+0x1ba/0x230
>>> Code: 00 00 00 00 00 55 48 89 e5 fa 5d c3 66 0f 1f 84 00 00 00 00 00 55 48 
>>> 89 e5 fb 5d c3 66 0f 1f 84 00 00 00 00 00 55 48 89 e5 fb f4 <5d> c3 0f 1f 
>>> 84 00 00 00 00 00 55 48 89 e5 f4 5d c3 66 0f 1f 84
>>> NMI backtrace for cpu 0 
>>
>> This is the NMI watchdog firing; the CPU got stuck for 20 seconds.  The
>> issue was not a busy host, but a busy storage (could it be a network
>> partition if the disk was hosted on NFS???)
>>
>> Firing the NMI watchdog is fixed in more recent QEMU, which has
>> asynchronous cancellation, assuming you're running RHEL's QEMU 1.5.3
>> (try /usr/libexec/qemu-kvm --version, or rpm -qf /usr/libexec/qemu-kvm).
>>
> Actually, you still cannot do _real_ async cancellation of I/O; the
> linux aio subsystem implements io_cancel(), but the cancellation
> just aborts the (internal) waitqueue element, not the I/O itself.

Right, but at least the TMF is asynchronous.  Synchronous TMFs keep the
VCPUs in QEMU for many seconds and cause the watchdog to fire.

Paolo

Re: [Qemu-devel] [RFC PATCH 0/4] tests: Introducing docker tests

[Paolo Bonzini]

On 04/02/2016 03:43, Fam Zheng wrote:
> Non-Linux environments can be sorted out in this framework by pulling
> appropriate 3rd party images from docker hub, assuming there isn't too much
> copyright hassle for Mac OSX and Windows stuff. This is theoretically possible
> because with "--privileged -v /dev/kvm:/dev/kvm": we can spawn a kvm guest
> inside the container.  Even without kvm, TCG may also be useful for non-x86
> build env.
> 
> This series also covers mingw cross compiling which might catch some windows
> problems.

And especially 32-bit problems.  Failing on mingw but not arm32 is rare
overall.

Paolo

Re: [Qemu-devel] [PATCH 2/4] vfio: make the 4 bytes aligned for capability size

[Michael S. Tsirkin]

On Thu, Feb 04, 2016 at 09:31:06AM +0800, Cao jin wrote:
> From: Chen Fan 
> 
> this function search the capability from the end, the last
> size should 0x100 - pos, not 0xff - pos.
> 
> Signed-off-by: Chen Fan 
> Reviewed-by: Marcel Apfelbaum 

Reviewed-by: Michael S. Tsirkin 

> ---
>  hw/vfio/pci.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 49f3d2d..e671506 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -1505,7 +1505,8 @@ static void vfio_unmap_bars(VFIOPCIDevice *vdev)
>   */
>  static uint8_t vfio_std_cap_max_size(PCIDevice *pdev, uint8_t pos)
>  {
> -uint8_t tmp, next = 0xff;
> +uint8_t tmp;
> +uint16_t next = PCI_CONFIG_SPACE_SIZE;
>  
>  for (tmp = pdev->config[PCI_CAPABILITY_LIST]; tmp;
>   tmp = pdev->config[tmp + 1]) {
> -- 
> 1.9.3
> 
> 

[Qemu-devel] [PATCH 2/5] pc-dimm: rename pc_dimm_built_list()

[Vladimir Sementsov-Ogievskiy]

s/pc_dimm_built_list/pc_dimm_build_list_sorted

- need for add pc_dimm_build_list (not sorted) in the next patch
- fix typo (built -> build)

Signed-off-by: Vladimir Sementsov-Ogievskiy 

CC: Stefan Hajnoczi 
CC: Xiao Guangrong 
CC: "Michael S. Tsirkin" 
CC: Igor Mammedov 
CC: Eric Blake 
CC: Markus Armbruster 
---
 hw/mem/pc-dimm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/mem/pc-dimm.c b/hw/mem/pc-dimm.c
index 4f30950..4a681bc 100644
--- a/hw/mem/pc-dimm.c
+++ b/hw/mem/pc-dimm.c
@@ -251,7 +251,7 @@ static gint pc_dimm_addr_sort(gconstpointer a, 
gconstpointer b)
 return 0;
 }
 
-static int pc_dimm_built_list(Object *obj, void *opaque)
+static int pc_dimm_build_list_sorted(Object *obj, void *opaque)
 {
 GSList **list = opaque;
 
@@ -262,7 +262,7 @@ static int pc_dimm_built_list(Object *obj, void *opaque)
 }
 }
 
-object_child_foreach(obj, pc_dimm_built_list, opaque);
+object_child_foreach(obj, pc_dimm_build_list_sorted, opaque);
 return 0;
 }
 
@@ -296,7 +296,7 @@ uint64_t pc_dimm_get_free_addr(uint64_t address_space_start,
 }
 
 assert(address_space_end > address_space_start);
-object_child_foreach(qdev_get_machine(), pc_dimm_built_list, );
+object_child_foreach(qdev_get_machine(), pc_dimm_build_list_sorted, );
 
 if (hint) {
 new_addr = *hint;
-- 
1.8.3.1

[Qemu-devel] [PATCH 1/9] cpu: rename cpu_exists() to qemu_get_cpu_by_arch_id()

[Igor Mammedov]

cpu_exists() already does CPU lookup but
discards found CPU and returns boolean instead.
Make it more useful by returning a found CPU
and also rename it more descriptive name.

Signed-off-by: Igor Mammedov 
---
 hw/i386/pc.c  |  2 +-
 include/qom/cpu.h | 10 +-
 qom/cpu.c |  6 +++---
 target-i386/cpu.c |  2 +-
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 942ac06..d72246d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1088,7 +1088,7 @@ void pc_hot_add_cpu(const int64_t id, Error **errp)
 return;
 }
 
-if (cpu_exists(apic_id)) {
+if (qemu_get_cpu_by_arch_id(apic_id)) {
 error_setg(errp, "Unable to add CPU: %" PRIi64
", it already exists", id);
 return;
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 035179c..bd26bf5 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -622,14 +622,14 @@ void async_run_on_cpu(CPUState *cpu, void (*func)(void 
*data), void *data);
 CPUState *qemu_get_cpu(int index);
 
 /**
- * cpu_exists:
- * @id: Guest-exposed CPU ID to lookup.
+ * qemu_get_cpu_by_arch_id:
+ * @id: Guest-exposed CPU ID to lookup returned by CPUState@get_arch_id()
  *
- * Search for CPU with specified ID.
+ * Gets a CPU matching @id.
  *
- * Returns: %true - CPU is found, %false - CPU isn't found.
+ * Returns: The CPU or %NULL if there is no matching CPU.
  */
-bool cpu_exists(int64_t id);
+CPUState *qemu_get_cpu_by_arch_id(int id);
 
 /**
  * cpu_throttle_set:
diff --git a/qom/cpu.c b/qom/cpu.c
index 8f537a4..e66989b 100644
--- a/qom/cpu.c
+++ b/qom/cpu.c
@@ -26,7 +26,7 @@
 #include "qemu/error-report.h"
 #include "sysemu/sysemu.h"
 
-bool cpu_exists(int64_t id)
+CPUState *qemu_get_cpu_by_arch_id(int id)
 {
 CPUState *cpu;
 
@@ -34,10 +34,10 @@ bool cpu_exists(int64_t id)
 CPUClass *cc = CPU_GET_CLASS(cpu);
 
 if (cc->get_arch_id(cpu) == id) {
-return true;
+return cpu;
 }
 }
-return false;
+return NULL;
 }
 
 CPUState *cpu_generic_init(const char *typename, const char *cpu_model)
diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index b255644..3918f01 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1789,7 +1789,7 @@ static void x86_cpuid_set_apic_id(Object *obj, Visitor 
*v, void *opaque,
 return;
 }
 
-if ((value != cpu->apic_id) && cpu_exists(value)) {
+if ((value != cpu->apic_id) && qemu_get_cpu_by_arch_id(value)) {
 error_setg(errp, "CPU with APIC ID %" PRIi64 " exists", value);
 return;
 }
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 2/2] qapi: Allow blockdev-add for NBD

[Paolo Bonzini]

On 03/02/2016 18:16, Max Reitz wrote:
> However, the issue remains that the NBD block driver expects
> flattened options which is syntactically incompatible to
> SocketAddress. Maybe the best way to address this would be to just
> make block/nbd.c directly accept a SocketAddress and keep the old
> flattened @host, @port, and @path options only as a legacy mapping
> to inet.host, inet.port, and unix.path.

Do we need to keep them at all?  The URL-based file is already good
enough as a shortcut for human and command-line use.  Is anyone
actually using host/port/path?

Paolo

Re: [Qemu-devel] [PATCH 2/6] migration: Rename the'file' member of MigrationState

[Hailiang Zhang]

On 2016/2/4 19:10, Dr. David Alan Gilbert wrote:

* Amit Shah (amit.s...@redhat.com) wrote:

On (Thu) 04 Feb 2016 [10:50:16], Dr. David Alan Gilbert wrote:

* Amit Shah (amit.s...@redhat.com) wrote:

On (Fri) 15 Jan 2016 [11:37:42], zhanghailiang wrote:

Rename the 'file' member of MigrationState to 'to_dst_file'.


Why?

(Use this space to explain why you're doing it, rather than repeating
the first line of the commit.  It helps when reviewing the git logs
later.)


I think the simple answer is consistency; now we've got a bidirectional
connection we've already got to_src_file, from_src_file and from_dst_file,
so 'file' is an oddity that I was too lazy to rename previously.


Sure, I just mean we need better commit description.  Just provide
one, and I'll use that when doing the pull req.


I suggest:

Rename the 'file' memory of MigrationState to 'to_dst_file' to
be consistent with to_src_file, from_src_file and from_dst_file.



A very Good suggestion, thanks.


Dave



Thanks,

Amit

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

.

Re: [Qemu-devel] qdev & hw/core owner? (was Re: [PATCH v19 7/9] machine: add properties to compat_props incrementaly)

[Michael S. Tsirkin]

On Thu, Feb 04, 2016 at 12:41:39PM +0100, Andreas Färber wrote:
> Am 04.02.2016 um 12:31 schrieb Paolo Bonzini:
> > On 03/02/2016 20:06, Michael S. Tsirkin wrote:
> >> On Wed, Feb 03, 2016 at 03:55:04PM -0200, Eduardo Habkost wrote:
> >>> On Thu, Jan 28, 2016 at 06:00:31PM +0100, Igor Mammedov wrote:
> >>> [...]
>  It looks like this series might go nowhere but this patch
>  is not tied to it and useful to us in general
>  so perhaps you could pick it up after ACKs from
>  S390/SPAPR maintainers.
> 
> >
> > Reviewed-by: Eduardo Habkost 
> >>>
> >>> We don't have a maintainer for hw/core/machine.c, hw/core/qdev*,
> >>> and related files.
> >>>
> >>> Assuming we don't have a volunteer to maintain them officially,
> >>> can we agree on a default destination for those patches so they
> >>> don't linger on the list? Michael? Andreas?
> >>
> >> Not me please. Have too much on my plate.
> >> Would you like to maintain it yourself?
> > 
> > That's my suggestion too.  I guess Igor and I could help with reviews,
> > but testing and sending the pull requests would add too much work.
> > Since you're the main one touching it, it makes sense for you to handle it.
> 
> You're talking about machine, right? Some time ago I had proposed Marcel
> who initially worked on it, but I'm fine with anyone taking it.

Sure, Marcel can do it too.

> 
> For some (but not all) core qdev parts related to the (stalled) QOM
> migration I've been taking care of via qom-next. Last time this came up
> you didn't want anyone to be M: for qdev, so maybe we can use R: so that
> at least people automatically get CC'ed and we avoid this recurring
> discussion?
> 
> Regards,
> Andreas

I think that was because core was not changing much, so merging through
other trees was more appropriate, but that changed.

So it appears we already can have
M: Eduardo Habkost 
M: Marcel Apfelbaum 

This should spread the review load nicely.

> -- 
> SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)

[Qemu-devel] [PATCH v2 08/11] block: m25p80: Fast read and 4bytes commands

[marcin . krzeminski]

From: Marcin Krzeminski 

Adds fast read and 4bytes commands family.
This work is based on Pawel Lenkow patch from v1.

Signed-off-by: Marcin Krzeminski 
---
 hw/block/m25p80.c | 43 ++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index df1f829..faf1212 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -224,19 +224,28 @@ typedef enum {
 BULK_ERASE = 0xc7,
 
 READ = 0x3,
-FAST_READ = 0xb,
+READ4 = 0x13,
+FAST_READ = 0x0b,
+FAST_READ4 = 0x0c,
 DOR = 0x3b,
+DOR4 = 0x3c,
 QOR = 0x6b,
+QOR4 = 0x6c,
 DIOR = 0xbb,
+DIOR4 = 0xbc,
 QIOR = 0xeb,
+QIOR4 = 0xec,
 
 PP = 0x2,
+PP4 = 0x12,
 DPP = 0xa2,
 QPP = 0x32,
 
 ERASE_4K = 0x20,
+ERASE4_4K = 0x21,
 ERASE_32K = 0x52,
 ERASE_SECTOR = 0xd8,
+ERASE4_SECTOR = 0xdc,
 
 EN_4BYTE_ADDR = 0xB7,
 EX_4BYTE_ADDR = 0xE9,
@@ -359,6 +368,7 @@ static void flash_erase(Flash *s, int offset, FlashCMD cmd)
 
 switch (cmd) {
 case ERASE_4K:
+case ERASE4_4K:
 len = 4 << 10;
 capa_to_assert = ER_4K;
 break;
@@ -367,6 +377,7 @@ static void flash_erase(Flash *s, int offset, FlashCMD cmd)
 capa_to_assert = ER_32K;
 break;
 case ERASE_SECTOR:
+case ERASE4_SECTOR:
 len = s->pi->sector_size;
 break;
 case BULK_ERASE:
@@ -425,6 +436,18 @@ void flash_write8(Flash *s, uint64_t addr, uint8_t data)
 
 static inline int get_cmd_length(Flash *s)
 {
+   switch (s->cmd_in_progress) {
+   case PP4:
+   case READ4:
+   case QIOR4:
+   case ERASE4_4K:
+   case ERASE4_SECTOR:
+   case FAST_READ4:
+   case DOR4:
+   case QOR4:
+   case DIOR4:
+   return 4;
+   default:
return s->four_bytes_address_mode ? 4 : 3;
}
 }
@@ -450,19 +473,28 @@ static void complete_collecting_data(Flash *s)
 case DPP:
 case QPP:
 case PP:
+case PP4:
 s->state = STATE_PAGE_PROGRAM;
 break;
 case READ:
+case READ4:
 case FAST_READ:
+case FAST_READ4:
 case DOR:
+case DOR4:
 case QOR:
+case QOR4:
 case DIOR:
+case DIOR4:
 case QIOR:
+case QIOR4:
 s->state = STATE_READ;
 break;
 case ERASE_4K:
+case ERASE4_4K:
 case ERASE_32K:
 case ERASE_SECTOR:
+case ERASE4_SECTOR:
 flash_erase(s, s->cur_addr, s->cmd_in_progress);
 break;
 case WRSR:
@@ -515,12 +547,16 @@ static void decode_new_cmd(Flash *s, uint32_t value)
 switch (value) {
 
 case ERASE_4K:
+case ERASE4_4K:
 case ERASE_32K:
 case ERASE_SECTOR:
+case ERASE4_SECTOR:
 case READ:
+case READ4:
 case DPP:
 case QPP:
 case PP:
+case PP4:
 s->needed_bytes = get_cmd_length(s);
 s->pos = 0;
 s->len = 0;
@@ -528,8 +564,11 @@ static void decode_new_cmd(Flash *s, uint32_t value)
 break;
 
 case FAST_READ:
+case FAST_READ4:
 case DOR:
+case DOR4:
 case QOR:
+case QOR4:
 s->needed_bytes = get_cmd_length(s);
 if ( ((s->pi->jedec >> 16) & 0xFF) == JEDEC_NUMONYX ) {
 /* Dummy cycles - modeled with bytes writes instead of bits */
@@ -541,6 +580,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
 break;
 
 case DIOR:
+case DIOR4:
 switch ((s->pi->jedec >> 16) & 0xFF) {
 case JEDEC_WINBOND:
 case JEDEC_SPANSION:
@@ -559,6 +599,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
 break;
 
 case QIOR:
+case QIOR4:
 switch ((s->pi->jedec >> 16) & 0xFF) {
 case JEDEC_WINBOND:
 case JEDEC_SPANSION:
-- 
2.5.0

[Qemu-devel] [PATCH v2 05/11] block: m25p80: 4byte address mode

[marcin . krzeminski]

From: Marcin Krzeminski 

This patch adds only 4byte address mode (does not cover dummy cycles).
This mode is needed to access more than 16 MiB of flash.

Signed-off-by: Marcin Krzeminski 
---
 hw/block/m25p80.c | 40 +++-
 1 file changed, 35 insertions(+), 5 deletions(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 6b5f00d..e306356 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -237,6 +237,9 @@ typedef enum {
 ERASE_32K = 0x52,
 ERASE_SECTOR = 0xd8,
 
+EN_4BYTE_ADDR = 0xB7,
+EX_4BYTE_ADDR = 0xE9,
+
 EXTEND_ADDR_READ = 0xC8,
 EXTEND_ADDR_WRITE = 0xC5,
 
@@ -269,6 +272,7 @@ typedef struct Flash {
 uint8_t cmd_in_progress;
 uint64_t cur_addr;
 bool write_enable;
+bool four_bytes_address_mode;
 bool reset_enable;
 uint8_t ear;
 
@@ -406,12 +410,26 @@ void flash_write8(Flash *s, uint64_t addr, uint8_t data)
 s->dirty_page = page;
 }
 
+static inline int get_cmd_length(Flash *s)
+{
+   return s->four_bytes_address_mode ? 4 : 3;
+   }
+}
+
 static void complete_collecting_data(Flash *s)
 {
-s->cur_addr = s->data[0] << 16;
-s->cur_addr |= s->data[1] << 8;
-s->cur_addr |= s->data[2];
+int i;
+
+s->cur_addr = 0;
+
+for ( i=0; i< get_cmd_length(s); ++i ) {
+s->cur_addr <<= 8;
+s->cur_addr |= s->data[i];
+}
+
+if ( get_cmd_length(s) == 3) {
 s->cur_addr += (s->ear & 0x3) * MAX_3BYTES_SIZE;
+}
 
 s->state = STATE_IDLE;
 
@@ -452,6 +470,7 @@ static void reset_memory(Flash *s)
 s->cmd_in_progress = NOP;
 s->cur_addr = 0;
 s->ear = 0;
+s->four_bytes_address_mode = false;
 s->len = 0;
 s->needed_bytes = 0;
 s->pos = 0;
@@ -480,7 +499,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
 case DPP:
 case QPP:
 case PP:
-s->needed_bytes = 3;
+s->needed_bytes = get_cmd_length(s);
 s->pos = 0;
 s->len = 0;
 s->state = STATE_COLLECTING_DATA;
@@ -489,7 +508,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
 case FAST_READ:
 case DOR:
 case QOR:
-s->needed_bytes = 4;
+s->needed_bytes = get_cmd_length(s);
 s->pos = 0;
 s->len = 0;
 s->state = STATE_COLLECTING_DATA;
@@ -502,6 +521,8 @@ static void decode_new_cmd(Flash *s, uint32_t value)
 s->needed_bytes = 4;
 break;
 case JEDEC_NUMONYX:
+s->needed_bytes = get_cmd_length(s);
+break;
 default:
 s->needed_bytes = 5;
 }
@@ -517,6 +538,8 @@ static void decode_new_cmd(Flash *s, uint32_t value)
 s->needed_bytes = 6;
 break;
 case JEDEC_NUMONYX:
+s->needed_bytes = get_cmd_length(s);
+break;
 default:
 s->needed_bytes = 8;
 }
@@ -575,6 +598,12 @@ static void decode_new_cmd(Flash *s, uint32_t value)
 break;
 case NOP:
 break;
+case EN_4BYTE_ADDR:
+s->four_bytes_address_mode = true;
+break;
+case EX_4BYTE_ADDR:
+s->four_bytes_address_mode = false;
+break;
 case EXTEND_ADDR_READ:
 s->data[0] = s->ear;
 s->pos = 0;
@@ -725,6 +754,7 @@ static const VMStateDescription vmstate_m25p80 = {
 VMSTATE_UINT8(cmd_in_progress, Flash),
 VMSTATE_UINT64(cur_addr, Flash),
 VMSTATE_BOOL(write_enable, Flash),
+VMSTATE_BOOL(four_bytes_address_mode, Flash),
 VMSTATE_UINT8(ear, Flash),
 VMSTATE_BOOL(reset_enable, Flash),
 VMSTATE_END_OF_LIST()
-- 
2.5.0

[Qemu-devel] [PATCH v2 3/6] savevm: Split load vm state function qemu_loadvm_state

[zhanghailiang]

qemu_loadvm_state is too long, and we can simplify it by splitting up
with three helper functions.

Signed-off-by: zhanghailiang 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Amit Shah 
---
 migration/savevm.c | 156 +++--
 1 file changed, 92 insertions(+), 64 deletions(-)

diff --git a/migration/savevm.c b/migration/savevm.c
index 8a2cddc..00be5fe 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -1718,90 +1718,118 @@ void loadvm_free_handlers(MigrationIncomingState *mis)
 }
 }
 
+static int
+qemu_loadvm_section_start_full(QEMUFile *f, MigrationIncomingState *mis)
+{
+uint32_t instance_id, version_id, section_id;
+SaveStateEntry *se;
+LoadStateEntry *le;
+char idstr[256];
+int ret;
+
+/* Read section start */
+section_id = qemu_get_be32(f);
+if (!qemu_get_counted_string(f, idstr)) {
+error_report("Unable to read ID string for section %u",
+ section_id);
+return -EINVAL;
+}
+instance_id = qemu_get_be32(f);
+version_id = qemu_get_be32(f);
+
+trace_qemu_loadvm_state_section_startfull(section_id, idstr,
+instance_id, version_id);
+/* Find savevm section */
+se = find_se(idstr, instance_id);
+if (se == NULL) {
+error_report("Unknown savevm section or instance '%s' %d",
+ idstr, instance_id);
+return -EINVAL;
+}
+
+/* Validate version */
+if (version_id > se->version_id) {
+error_report("savevm: unsupported version %d for '%s' v%d",
+ version_id, idstr, se->version_id);
+return -EINVAL;
+}
+
+/* Add entry */
+le = g_malloc0(sizeof(*le));
+
+le->se = se;
+le->section_id = section_id;
+le->version_id = version_id;
+QLIST_INSERT_HEAD(>loadvm_handlers, le, entry);
+
+ret = vmstate_load(f, le->se, le->version_id);
+if (ret < 0) {
+error_report("error while loading state for instance 0x%x of"
+ " device '%s'", instance_id, idstr);
+return ret;
+}
+if (!check_section_footer(f, le)) {
+return -EINVAL;
+}
+
+return 0;
+}
+
+static int
+qemu_loadvm_section_part_end(QEMUFile *f, MigrationIncomingState *mis)
+{
+uint32_t section_id;
+LoadStateEntry *le;
+int ret;
+
+section_id = qemu_get_be32(f);
+
+trace_qemu_loadvm_state_section_partend(section_id);
+QLIST_FOREACH(le, >loadvm_handlers, entry) {
+if (le->section_id == section_id) {
+break;
+}
+}
+if (le == NULL) {
+error_report("Unknown savevm section %d", section_id);
+return -EINVAL;
+}
+
+ret = vmstate_load(f, le->se, le->version_id);
+if (ret < 0) {
+error_report("error while loading state section id %d(%s)",
+ section_id, le->se->idstr);
+return ret;
+}
+if (!check_section_footer(f, le)) {
+return -EINVAL;
+}
+
+return 0;
+}
+
 static int qemu_loadvm_state_main(QEMUFile *f, MigrationIncomingState *mis)
 {
 uint8_t section_type;
 int ret;
 
 while ((section_type = qemu_get_byte(f)) != QEMU_VM_EOF) {
-uint32_t instance_id, version_id, section_id;
-SaveStateEntry *se;
-LoadStateEntry *le;
-char idstr[256];
 
 trace_qemu_loadvm_state_section(section_type);
 switch (section_type) {
 case QEMU_VM_SECTION_START:
 case QEMU_VM_SECTION_FULL:
-/* Read section start */
-section_id = qemu_get_be32(f);
-if (!qemu_get_counted_string(f, idstr)) {
-error_report("Unable to read ID string for section %u",
-section_id);
-return -EINVAL;
-}
-instance_id = qemu_get_be32(f);
-version_id = qemu_get_be32(f);
-
-trace_qemu_loadvm_state_section_startfull(section_id, idstr,
-  instance_id, version_id);
-/* Find savevm section */
-se = find_se(idstr, instance_id);
-if (se == NULL) {
-error_report("Unknown savevm section or instance '%s' %d",
- idstr, instance_id);
-return -EINVAL;
-}
-
-/* Validate version */
-if (version_id > se->version_id) {
-error_report("savevm: unsupported version %d for '%s' v%d",
- version_id, idstr, se->version_id);
-return -EINVAL;
-}
-
-/* Add entry */
-le = g_malloc0(sizeof(*le));
-
-le->se = se;
-le->section_id = section_id;
-le->version_id = version_id;
-QLIST_INSERT_HEAD(>loadvm_handlers, le, entry);
-
-ret = vmstate_load(f, le->se, 

Re: [Qemu-devel] [PATCH] quorum: fix segfault when read fails in fifo mode

[Kevin Wolf]

Am 04.02.2016 um 11:19 hat Changlong Xie geschrieben:
> Signed-off-by: Wen Congyang 
> Signed-off-by: Changlong Xie 
> ---
>  block/quorum.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/block/quorum.c b/block/quorum.c
> index a5ae4b8..0965277 100644
> --- a/block/quorum.c
> +++ b/block/quorum.c
> @@ -295,6 +295,9 @@ static void quorum_aio_cb(void *opaque, int ret)
>  quorum_copy_qiov(acb->qiov, >qcrs[acb->child_iter].qiov);
>  }
>  acb->vote_ret = ret;
> +if (ret < 0) {
> +acb->child_iter--;
> +}
>  quorum_aio_finalize(acb);
>  return;
>  }

This looks semantically correct to me (but I'd like to have an Ack from
Berto), but I would fix it above: We shouldn't do ++acb->child_iter in
the first place if the new value is >= s->num_children. So instead of
decrementing after the fact, just move the increment to inside the then
branch above.

Kevin

Re: [Qemu-devel] [PATCH 3/3] balloon: don't use NVDIMM for ballooning

[Vladimir Sementsov-Ogievskiy]

On 04.02.2016 09:20, Markus Armbruster wrote:

Vladimir Sementsov-Ogievskiy  writes:


On 03.02.2016 18:42, Markus Armbruster wrote:

Eric Blake  writes:


On 01/27/2016 11:51 PM, Vladimir Sementsov-Ogievskiy wrote:

NVDIMM for now is planned to use as a backing store for DAX filesystem
in the guest and thus this memory is excluded from guest memory
management and LRUs.

In this case libvirt running QEMU along with configured balloon almost
immediately inflates balloon and effectively kill the guest as
qemu counts nvdimm as part of the ram.

Counting dimm devices as part of the ram for ballooning was started from
commit 463756d03:
   virtio-balloon: Fix balloon not working correctly when hotplug memory

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Denis V. Lunev 
CC: Stefan Hajnoczi 
CC: Xiao Guangrong 
CC: "Michael S. Tsirkin" 
CC: Igor Mammedov 
CC: Eric Blake 
CC: Markus Armbruster 
---
@@ -308,7 +309,9 @@ static ram_addr_t get_current_ram_size(void)
   if (value) {
   switch (value->type) {
   case MEMORY_DEVICE_INFO_KIND_DIMM:
-size += value->u.dimm->size;
+if (strcmp(value->u.dimm->type, TYPE_NVDIMM)) {

If you fix 2/3 to use a QAPI enum, then this will be an integer compare
instead of a strcmp().

Another option is to add a flag to the subtypes of TYPE_PC_DIMM, set it
for the subtypes that should be counted here, and accumulate the sizes
of devices where the flag is set.  Requires iterating directly over the
devices here (like qmp_pc_dimm_device_list() does under the hood) rather
than the MemoryDeviceInfoList returned by qmp_pc_dimm_device_list(),

It was my first approach but it was rejected)

As another option I can make a function iterating over the devices and
return list of them, and then use it instead of
qmp_pc_dimm_device_list.. Then, I'll have pointers to devices and can
use object_dynamic_cast.

I fail to see how splitting a tree walk doing stuff into a tree walk
creating a list and a list walk doing stuff makes things better :)


It will allow me not touch qapi)



Anyway, you guys figure it out.  The only part where I get involved is
QAPI design.




--
Best regards,
Vladimir

Re: [Qemu-devel] sda abort with virtio-scsi

[Denis V. Lunev]

On 02/04/2016 01:23 PM, Paolo Bonzini wrote:


On 04/02/2016 00:34, Jim Minter wrote:

I was worried there was
some way in which the contention could cause an abort and perhaps thence
the lockup (which does not seem to recover when the host load goes down).

I don't know... It's not the most tested code, but it is not very
complicated either.

The certain points that can be extracted from the kernel messages are:
1) there was a cancellation request that took a long time, >20 seconds;
2) despite taking a long time, it _did_ recover sooner or later because
otherwise you'd not have the lockup splat either.

Paolo


Firing the NMI watchdog is fixed in more recent QEMU, which has
asynchronous cancellation, assuming you're running RHEL's QEMU 1.5.3
(try /usr/libexec/qemu-kvm --version, or rpm -qf /usr/libexec/qemu-kvm).

/usr/libexec/qemu-kvm --version reports QEMU emulator version 1.5.3
(qemu-kvm-1.5.3-105.el7_2.3)


my $0.02 to the account. This could be related or could be
not related.

speaking about NMI we do observe similar problems on older
AMD hosts with big enough number of VCPUs in guest.
On a simple boot we see something like this (the probability to
face the problem is around 1/10). RHEV 2.3 QEMU is used, the same
kernel is running on host node:

[   72.189005] Kernel panic - not syncing: softlockup: hung tasks
[   72.189005] CPU: 5 PID: 593 Comm: systemd-udevd Tainted: G L 
   3.10.0-327.4.4.el7.x86_64 #1
[   72.189005] Hardware name: Red Hat KVM, BIOS seabios-1.7.5-11.vz7.2 
04/01/2014
[   72.189005]  81871a03 50291887 88007fd43e18 
8163515c
[   72.189005]  88007fd43e98 8162e9d7 0008 
88007fd43ea8
[   72.189005]  88007fd43e48 50291887 2710 

[   72.189005] Call Trace:
[   72.189005][] dump_stack+0x19/0x1b
[   72.189005]  [] panic+0xd8/0x1e7
[   72.189005]  [] watchdog_timer_fn+0x1b6/0x1c0
[   72.189005]  [] ? watchdog_enable+0xc0/0xc0
[   72.189005]  [] __hrtimer_run_queues+0xd2/0x260
[   72.189005]  [] hrtimer_interrupt+0xb0/0x1e0
[   72.189005]  [] ? call_softirq+0x1c/0x30
[   72.189005]  [] ? flush_tlb_func+0xb0/0xb0
[   72.189005]  [] local_apic_timer_interrupt+0x37/0x60
[   72.189005]  [] smp_apic_timer_interrupt+0x3f/0x60
[   72.189005]  [] apic_timer_interrupt+0x6d/0x80
[   72.189005][] ? free_cpumask_var+0x9/0x10
[   72.189005]  [] ? smp_call_function_many+0x202/0x260
[   72.189005]  [] ? flush_tlb_func+0xb0/0xb0
[   72.189005]  [] on_each_cpu+0x2d/0x60
[   72.189005]  [] flush_tlb_kernel_range+0x59/0xa0
[   72.189005]  [] __purge_vmap_area_lazy+0x1a0/0x210
[   72.189005]  [] vm_unmap_aliases+0x187/0x1b0
[   72.189005]  [] change_page_attr_set_clr+0xe8/0x4d0
[   72.189005]  [] ? ring_buffer_time_stamp+0x7/0x10
[   72.189005]  [] set_memory_ro+0x2f/0x40
[   72.189005]  [] set_section_ro_nx+0x3a/0x71
[   72.189005]  [] load_module+0x103c/0x1b50
[   72.189005]  [] ? copy_module_from_fd.isra.42+0x53/0x150
[   72.189005]  [] SyS_finit_module+0xa6/0xd0
[   72.189005]  [] system_call_fastpath+0x16/0x1b
[   72.189005] Shutting down cpus with NMI


Timesources changes does not help. Also there is a strange stuff like this:

Having configured the serial port to point at a unix socket, and timestamping
the messages on the host side, we observe

guest timestamp | host timestamp
   0.00 | 20:09:26.805461
   2.587056 | 20:09:30.000993
   7.607329 | 20:09:35.062367
  12.645539 | 20:09:40.057634
  22.608054 | 20:09:50.028727
  32.395499 | 20:10:00.041215
  42.571265 | 20:10:10.041960
  47.606661 | 20:10:15.028973
  48.627059 | 20:10:20.022359
  49.029059 | 20:10:25.047857
  49.399065 | 20:10:30.066884
  49.809077 | 20:10:35.036467
  58.159132 | 20:10:40.013387
  68.043371 | 20:10:40.266714

Note the anomaly around 47 seconds from boot.

anyway, this story is far from the end and we are unable to provide more 
details. Roma is digging this story. Den

Re: [Qemu-devel] qdev & hw/core owner? (was Re: [PATCH v19 7/9] machine: add properties to compat_props incrementaly)

[Andreas Färber]

Am 04.02.2016 um 12:31 schrieb Paolo Bonzini:
> On 03/02/2016 20:06, Michael S. Tsirkin wrote:
>> On Wed, Feb 03, 2016 at 03:55:04PM -0200, Eduardo Habkost wrote:
>>> On Thu, Jan 28, 2016 at 06:00:31PM +0100, Igor Mammedov wrote:
>>> [...]
 It looks like this series might go nowhere but this patch
 is not tied to it and useful to us in general
 so perhaps you could pick it up after ACKs from
 S390/SPAPR maintainers.

>
> Reviewed-by: Eduardo Habkost 
>>>
>>> We don't have a maintainer for hw/core/machine.c, hw/core/qdev*,
>>> and related files.
>>>
>>> Assuming we don't have a volunteer to maintain them officially,
>>> can we agree on a default destination for those patches so they
>>> don't linger on the list? Michael? Andreas?
>>
>> Not me please. Have too much on my plate.
>> Would you like to maintain it yourself?
> 
> That's my suggestion too.  I guess Igor and I could help with reviews,
> but testing and sending the pull requests would add too much work.
> Since you're the main one touching it, it makes sense for you to handle it.

You're talking about machine, right? Some time ago I had proposed Marcel
who initially worked on it, but I'm fine with anyone taking it.

For some (but not all) core qdev parts related to the (stalled) QOM
migration I've been taking care of via qom-next. Last time this came up
you didn't want anyone to be M: for qdev, so maybe we can use R: so that
at least people automatically get CC'ed and we avoid this recurring
discussion?

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)

[Qemu-devel] [PATCH v2 0/6] Some improvments and small fixes for migration

[zhanghailiang]

Patch 1 ~ patch 4 are picked from COLO and live memory snapshot series,
They are just small improvements for migration codes.

Patch 5, 6 are small fixes for migration releated documention. 

v2:
 - Fix a typo (Amit)
 - A better commit description from Dave for patch 2

zhanghailiang (6):
  ram: Split host_from_stream_offset() into two helper functions
  migration: Rename the'file' member of MigrationState
  savevm: Split load vm state function qemu_loadvm_state
  migration/ram: Fix some helper functions' parameter to use
PageSearchStatus
  qmp-commands.hx: Fix the missing options for migration parameters
commands
  qmp-commands.hx: Document the missing options for migration capability
commands

 include/exec/ram_addr.h   |   8 ++-
 include/migration/migration.h |   2 +-
 migration/exec.c  |   4 +-
 migration/fd.c|   4 +-
 migration/migration.c |  72 +--
 migration/postcopy-ram.c  |   6 +-
 migration/ram.c   |  73 +++
 migration/rdma.c  |   2 +-
 migration/savevm.c| 158 +-
 migration/tcp.c   |   4 +-
 migration/unix.c  |   4 +-
 qmp-commands.hx   |  33 +++--
 12 files changed, 222 insertions(+), 148 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH v2 01/11] block: m25p80: Removed unused variable

[marcin . krzeminski]

From: Marcin Krzeminski 

Signed-off-by: Marcin Krzeminski 
Reviewed-by: Peter Crosthwaite 
---
 hw/block/m25p80.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index de24f42..124 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -246,8 +246,6 @@ typedef enum {
 typedef struct Flash {
 SSISlave parent_obj;
 
-uint32_t r;
-
 BlockBackend *blk;
 
 uint8_t *storage;
-- 
2.5.0

[Qemu-devel] [PATCH v2 11/11] block: m25p80: at25128a/at25256a models

[marcin . krzeminski]

From: Marcin Krzeminski 

Signed-off-by: Marcin Krzeminski 
---
 hw/block/m25p80.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index ec1013d..3ff418f 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -99,6 +99,12 @@ static const FlashPartInfo known_devices[] = {
 
 { INFO("at45db081d",  0x1f2500,  0,  64 << 10,  16, ER_4K) },
 
+/* Atmel EEPROMS - it is assumed, that don't care bit in command
+ * is set to 0. Block protection is not supported.
+ */
+{ INFO("at25128a-nonjedec", 0x0, 0, 1, 131072, WR_1) },
+{ INFO("at25256a-nonjedec", 0x0, 0, 1, 262144, WR_1) },
+
 /* EON -- en25xxx */
 { INFO("en25f32", 0x1c3116,  0,  64 << 10,  64, ER_4K) },
 { INFO("en25p32", 0x1c2016,  0,  64 << 10,  64, 0) },
@@ -438,6 +444,11 @@ void flash_write8(Flash *s, uint64_t addr, uint8_t data)
 
 static inline int get_cmd_length(Flash *s)
 {
+   /* check if eeprom is in use */
+   if (s->pi->flags == WR_1) {
+   return 2;
+   }
+
switch (s->cmd_in_progress) {
case PP4:
case READ4:
-- 
2.5.0

[Qemu-devel] [PATCH v2 02/11] block: m25p80: RESET_ENABLE and RESET_MEMORY commnads

[marcin . krzeminski]

From: Marcin Krzeminski 

Signed-off-by: Marcin Krzeminski 
---
 hw/block/m25p80.c | 36 +++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index 124..b986df4 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -233,6 +233,9 @@ typedef enum {
 ERASE_4K = 0x20,
 ERASE_32K = 0x52,
 ERASE_SECTOR = 0xd8,
+
+RESET_ENABLE = 0x66,
+RESET_MEMORY = 0x99,
 } FlashCMD;
 
 typedef enum {
@@ -260,6 +263,7 @@ typedef struct Flash {
 uint8_t cmd_in_progress;
 uint64_t cur_addr;
 bool write_enable;
+bool reset_enable;
 
 int64_t dirty_page;
 
@@ -432,11 +436,29 @@ static void complete_collecting_data(Flash *s)
 }
 }
 
+static void reset_memory(Flash *s)
+{
+s->cmd_in_progress = NOP;
+s->cur_addr = 0;
+s->len = 0;
+s->needed_bytes = 0;
+s->pos = 0;
+s->state = STATE_IDLE;
+s->write_enable = false;
+s->reset_enable = false;
+
+DB_PRINT_L(0, "Reset done.\n");
+}
+
 static void decode_new_cmd(Flash *s, uint32_t value)
 {
 s->cmd_in_progress = value;
 DB_PRINT_L(0, "decoded new command:%x\n", value);
 
+if (value != RESET_MEMORY) {
+s->reset_enable = false;
+}
+
 switch (value) {
 
 case ERASE_4K:
@@ -541,6 +563,14 @@ static void decode_new_cmd(Flash *s, uint32_t value)
 break;
 case NOP:
 break;
+case RESET_ENABLE:
+s->reset_enable = true;
+break;
+case RESET_MEMORY:
+if (s->reset_enable) {
+reset_memory(s);
+}
+break;
 default:
 qemu_log_mask(LOG_GUEST_ERROR, "M25P80: Unknown cmd %x\n", value);
 break;
@@ -555,6 +585,7 @@ static int m25p80_cs(SSISlave *ss, bool select)
 s->len = 0;
 s->pos = 0;
 s->state = STATE_IDLE;
+s->reset_enable = false;
 flash_sync_dirty(s, -1);
 }
 
@@ -622,6 +653,8 @@ static int m25p80_init(SSISlave *ss)
 s->size = s->pi->sector_size * s->pi->n_sectors;
 s->dirty_page = -1;
 
+reset_memory(s);
+
 /* FIXME use a qdev drive property instead of drive_get_next() */
 dinfo = drive_get_next(IF_MTD);
 
@@ -654,7 +687,7 @@ static void m25p80_pre_save(void *opaque)
 
 static const VMStateDescription vmstate_m25p80 = {
 .name = "xilinx_spi",
-.version_id = 1,
+.version_id = 2,
 .minimum_version_id = 1,
 .pre_save = m25p80_pre_save,
 .fields = (VMStateField[]) {
@@ -666,6 +699,7 @@ static const VMStateDescription vmstate_m25p80 = {
 VMSTATE_UINT8(cmd_in_progress, Flash),
 VMSTATE_UINT64(cur_addr, Flash),
 VMSTATE_BOOL(write_enable, Flash),
+VMSTATE_BOOL(reset_enable, Flash),
 VMSTATE_END_OF_LIST()
 }
 };
-- 
2.5.0

Re: [Qemu-devel] [PATCH 2/6] migration: Rename the'file' member of MigrationState

[Dr. David Alan Gilbert]

* Amit Shah (amit.s...@redhat.com) wrote:
> On (Fri) 15 Jan 2016 [11:37:42], zhanghailiang wrote:
> > Rename the 'file' member of MigrationState to 'to_dst_file'.
> 
> Why?
> 
> (Use this space to explain why you're doing it, rather than repeating
> the first line of the commit.  It helps when reviewing the git logs
> later.)

I think the simple answer is consistency; now we've got a bidirectional
connection we've already got to_src_file, from_src_file and from_dst_file,
so 'file' is an oddity that I was too lazy to rename previously.

Dave

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

Re: [Qemu-devel] [PATCH 0/4] vfio-pci: pass the aer error to guest, part1

[Michael S. Tsirkin]

On Thu, Feb 04, 2016 at 09:31:04AM +0800, Cao jin wrote:
> From: Chen Fan 
> 
> this patchset are splited out from my aer series patches,
> this part of aer patches is harmless, and have been reviewed-by.
> In order to avoid blocking it to merge, so I send it out independently
> as part1. the part2 is under discussion. Thanks

I acked the remaining patch.
Alex, will you take it?

> Chen Fan (4):
>   pcie: modify the capability size assert
>   vfio: make the 4 bytes aligned for capability size
>   aer: impove pcie_aer_init to support vfio device
>   pcie_aer: expose pcie_aer_msg() interface
> 
>  hw/pci-bridge/ioh3420.c| 2 +-
>  hw/pci-bridge/xio3130_downstream.c | 2 +-
>  hw/pci-bridge/xio3130_upstream.c   | 2 +-
>  hw/pci/pcie.c  | 2 +-
>  hw/pci/pcie_aer.c  | 6 +++---
>  hw/vfio/pci.c  | 3 ++-
>  include/hw/pci/pcie_aer.h  | 3 ++-
>  7 files changed, 11 insertions(+), 9 deletions(-)
> 
> -- 
> 1.9.3
> 
> 

[Qemu-devel] [PATCH v5 0/5] don't use NVDIMM for balooning

[Vladimir Sementsov-Ogievskiy]

v5: do not use qapi
 0002-0004: new patches
 0005: white list instead of black list

v4:
 0001: Reviewed-by: Eric Blake 
 second patch is splitted to 0002 and 0003
 0002: Add 'type' field instead of 'balloonable' to PCDIMMDeviceInfo
 0003: chec 'type' instead of 'balloonable'

v3:
- do not use additional class variable

NVDIMM for now is planned to use as a backing store for DAX filesystem
in the guest and thus this memory is excluded from guest memory
management and LRUs.

In this case libvirt running QEMU along with configured balloon almost
immediately inflates balloon and effectively kill the guest as
qemu counts nvdimm as part of the ram.

CC: Vladimir Sementsov-Ogievskiy 
CC: Denis V. Lunev 
CC: Stefan Hajnoczi 
CC: Xiao Guangrong 
CC: "Michael S. Tsirkin" 
CC: Igor Mammedov 
CC: Eric Blake 
CC: Markus Armbruster 

Vladimir Sementsov-Ogievskiy (5):
  move get_current_ram_size to virtio-balloon.c
  pc-dimm: rename pc_dimm_built_list()
  pc-dimm: add pc_dimm_build_list()
  virtio-balloon: rewrite get_current_ram_size()
  balloon: Use only 'pc-dimm' type dimm for ballooning

 hw/mem/pc-dimm.c| 47 -
 hw/virtio/virtio-balloon.c  | 19 +
 include/exec/cpu-common.h   |  1 -
 include/hw/mem/pc-dimm.h|  3 +++
 stubs/qmp_pc_dimm_device_list.c |  5 -
 5 files changed, 40 insertions(+), 35 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 4/9] pc: acpi: SRAT: create only valid processor lapic entries

[Igor Mammedov]

When APIC IDs are sparse*, in addition to valid LAPIC
antries the SRAT is also filled invalid ones for non
posiible APIC IDs.
Fix it by asking machine for all possible APIC IDs
instead of wrongly assuming that all APIC IDs in
range 0..apic_id_limit are possible.

* sparse lapic topology CLI:
 -smp x,sockets=2,cores=3,maxcpus=6
Signed-off-by: Igor Mammedov 
---
 hw/i386/acpi-build.c | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index faf541c..3077061 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2376,6 +2376,8 @@ build_srat(GArray *table_data, GArray *linker, 
PcGuestInfo *guest_info,
 uint64_t curnode;
 int srat_start, numa_start, slots;
 uint64_t mem_len, mem_base, next_base;
+MachineClass *mc = MACHINE_GET_CLASS(machine);
+GArray *apic_id_list = mc->possible_cpu_arch_ids();
 PCMachineState *pcms = PC_MACHINE(machine);
 ram_addr_t hotplugabble_address_space_size =
 object_property_get_int(OBJECT(pcms), PC_MACHINE_MEMHP_REGION_SIZE,
@@ -2387,12 +2389,15 @@ build_srat(GArray *table_data, GArray *linker, 
PcGuestInfo *guest_info,
 srat->reserved1 = cpu_to_le32(1);
 core = (void *)(srat + 1);
 
-for (i = 0; i < guest_info->apic_id_limit; ++i) {
+for (i = 0; i < apic_id_list->len; i++) {
+CPUArchId id = FETCH_CPU_ARCH_ID(apic_id_list, i);
+int apic_id = id.arch_id;
+
 core = acpi_data_push(table_data, sizeof *core);
 core->type = ACPI_SRAT_PROCESSOR;
 core->length = sizeof(*core);
-core->local_apic_id = i;
-curnode = guest_info->node_cpu[i];
+core->local_apic_id = apic_id;
+curnode = guest_info->node_cpu[apic_id];
 core->proximity_lo = curnode;
 memset(core->proximity_hi, 0, 3);
 core->local_sapic_eid = 0;
@@ -2457,6 +2462,7 @@ build_srat(GArray *table_data, GArray *linker, 
PcGuestInfo *guest_info,
  (void *)(table_data->data + srat_start),
  "SRAT",
  table_data->len - srat_start, 1, NULL);
+g_array_free(apic_id_list, true);
 }
 
 static void
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 2/2] qapi: Allow blockdev-add for NBD

[Paolo Bonzini]

On 03/02/2016 17:48, Eric Blake wrote:
> On 02/03/2016 09:33 AM, Max Reitz wrote:
>> We have to introduce a new object (BlockdevOptionsNbd) for
>> several reasons: - Neither of InetSocketAddress nor
>> UnixSocketAddress alone is sufficient, because both are
>> supported - We cannot use SocketAddress because NBD does not
>> support an fd, and because it is not a flat union which
>> BlockdevOptionsNbd is
> 
> Can we do it anyways, and just error out/document that fd is
> unsupported?

Especially because there's no reason _not_ to support fd.  Sure, it's
really fringe, but if qemu-socket APIs make it just work...

Paolo

Re: [Qemu-devel] [RFC PATCH 3/4] tests: Add mingw 32/64 cross compiling

[Daniel P. Berrange]

On Wed, Feb 03, 2016 at 10:36:03PM +0800, Fam Zheng wrote:
> Only fedora22 has the required toolchain so it's not run elsewhere.
> 
> Signed-off-by: Fam Zheng 
> ---
>  tests/docker/mingw.sh | 14 ++
>  1 file changed, 14 insertions(+)
>  create mode 100755 tests/docker/mingw.sh
> 
> diff --git a/tests/docker/mingw.sh b/tests/docker/mingw.sh
> new file mode 100755
> index 000..0f103cd
> --- /dev/null
> +++ b/tests/docker/mingw.sh
> @@ -0,0 +1,14 @@
> +#!/bin/bash
> +
> +if [ "$IMAGE_TAG" != "fedora22" ]; then
> +echo "Mingw test skipped"
> +exit 0
> +fi
> +
> +cd $(mktemp -d)
> +for prefix in x86_64-w64-mingw32- i686-w64-mingw32-; do
> +$QEMU_SRC/configuire --cross-prefix=$prefix \
> +--target-list=x86_64-softmmu,aarch64-softmmu
> +make $MAKEFLAGS
> +done

Automated build tests should really pass explicit '--enable-xxx'
flags for every feature we expect to be present. This means that
if someone breaks feature detection in QEMU, we are more likely
to see a build failure, rather than the feature being silently
disabled for months on end.

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

Re: [Qemu-devel] [PATCH 3/9] pc: acpi: cleanup qdev_get_machine() calls

[Marcel Apfelbaum]

On 02/04/2016 01:47 PM, Igor Mammedov wrote:

call qdev_get_machine() once at acpi_build() and
pass its result to child functions that need it.

Signed-off-by: Igor Mammedov 
---
Later it will also allow to reduce number of arguments
passed around by build_foo() functions called from
acpi_build() once guest_info fields are moved into
PCMachineState.
---
  hw/i386/acpi-build.c | 20 +++-
  1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 2028ed7..faf541c 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1937,13 +1937,13 @@ static Aml *build_q35_osc_method(void)
  static void
  build_dsdt(GArray *table_data, GArray *linker,
 AcpiCpuInfo *cpu, AcpiPmInfo *pm, AcpiMiscInfo *misc,
-   PcPciInfo *pci, PcGuestInfo *guest_info)
+   PcPciInfo *pci, PcGuestInfo *guest_info,
+   MachineState *machine)
  {
  CrsRangeEntry *entry;
  Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs;
  GPtrArray *mem_ranges = g_ptr_array_new_with_free_func(crs_range_free);
  GPtrArray *io_ranges = g_ptr_array_new_with_free_func(crs_range_free);
-MachineState *machine = MACHINE(qdev_get_machine());
  uint32_t nr_mem = machine->ram_slots;
  int root_bus_limit = 0xFF;
  PCIBus *bus = NULL;
@@ -2365,7 +2365,8 @@ acpi_build_srat_memory(AcpiSratMemoryAffinity *numamem, 
uint64_t base,
  }

  static void
-build_srat(GArray *table_data, GArray *linker, PcGuestInfo *guest_info)
+build_srat(GArray *table_data, GArray *linker, PcGuestInfo *guest_info,
+   MachineState *machine)
  {
  AcpiSystemResourceAffinityTable *srat;
  AcpiSratProcessorAffinity *core;
@@ -2375,7 +2376,7 @@ build_srat(GArray *table_data, GArray *linker, 
PcGuestInfo *guest_info)
  uint64_t curnode;
  int srat_start, numa_start, slots;
  uint64_t mem_len, mem_base, next_base;
-PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
+PCMachineState *pcms = PC_MACHINE(machine);
  ram_addr_t hotplugabble_address_space_size =
  object_property_get_int(OBJECT(pcms), PC_MACHINE_MEMHP_REGION_SIZE,
  NULL);
@@ -2579,9 +2580,9 @@ static bool acpi_has_iommu(void)
  return intel_iommu && !ambiguous;
  }

-static bool acpi_has_nvdimm(void)
+static bool acpi_has_nvdimm(MachineState *machine)
  {
-PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
+PCMachineState *pcms = PC_MACHINE(machine);

  return pcms->nvdimm;
  }
@@ -2599,6 +2600,7 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables 
*tables)
  uint8_t *u;
  size_t aml_len = 0;
  GArray *tables_blob = tables->table_data;
+MachineState *machine = MACHINE(qdev_get_machine());

  acpi_get_cpu_info();
  acpi_get_pm_info();
@@ -2624,7 +2626,7 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables 
*tables)
  /* DSDT is pointed to by FADT */
  dsdt = tables_blob->len;
  build_dsdt(tables_blob, tables->linker, , , , ,
-   guest_info);
+   guest_info, machine);

  /* Count the size of the DSDT and SSDT, we will need it for legacy
   * sizing of ACPI tables.
@@ -2655,7 +2657,7 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables 
*tables)
  }
  if (guest_info->numa_nodes) {
  acpi_add_table(table_offsets, tables_blob);
-build_srat(tables_blob, tables->linker, guest_info);
+build_srat(tables_blob, tables->linker, guest_info, machine);
  }
  if (acpi_get_mcfg()) {
  acpi_add_table(table_offsets, tables_blob);
@@ -2666,7 +2668,7 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables 
*tables)
  build_dmar_q35(tables_blob, tables->linker);
  }

-if (acpi_has_nvdimm()) {
+if (acpi_has_nvdimm(machine)) {
  nvdimm_build_acpi(table_offsets, tables_blob, tables->linker);
  }




I like the approach to pass the machine as parameter instead of querying it
when the caller has already a reference to the machine.


Reviewed-by: Marcel Apfelbaum 


Thanks,
Marcel

[Qemu-devel] [PATCH v2 03/11] block: m25p80: Widen flags variable

[marcin . krzeminski]

From: Marcin Krzeminski 

Extend the width of the flags variable to support the already existing
(but unused) WR_1 flag, which is above the range of 8 bits.
This allows support of EEPROM emulation which requires the WR_1 feature.

Signed-off-by: Marcin Krzeminski 
Reviewed-by: Peter Crosthwaite 
---
 hw/block/m25p80.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index b986df4..f9088c5 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -61,7 +61,7 @@ typedef struct FlashPartInfo {
 uint32_t sector_size;
 uint32_t n_sectors;
 uint32_t page_size;
-uint8_t flags;
+uint16_t flags;
 } FlashPartInfo;
 
 /* adapted from linux */
-- 
2.5.0

[Qemu-devel] [PATCH v2 00/11] Support for N25Q256/512 and AT25128/256

[marcin . krzeminski]

From: Marcin Krzeminski 

V2:
- Removed support for mx66u51235 and s25fl512s from this series
- Corrected/implemented dummy cycles
- rebased to master

Model was tested on u-boot and kernel using custom qemu machine.

Marcin Krzeminski (11):
  block: m25p80: Removed unused variable
  block: m25p80: RESET_ENABLE and RESET_MEMORY commnads
  block: m25p80: Widen flags variable
  block: m25p80: Extend address mode
  block: m25p80: 4byte address mode
  block: m25p80: Add configuration registers
  block: m25p80: Dummy cycles for N25Q256/512
  block: m25p80: Fast read and 4bytes commands
  block: m25p80: Implemented FSR register
  block: m25p80: n25q256a/n25q512a models
  block: m25p80: at25128a/at25256a models

 hw/block/m25p80.c | 290 +++---
 1 file changed, 279 insertions(+), 11 deletions(-)

-- 
2.5.0

Re: [Qemu-devel] [PATCH] qemu-img: initialize MapEntry object

[Kevin Wolf]

Am 04.02.2016 um 00:38 hat John Snow geschrieben:
> Commit 16b0d555 introduced an issue where we are not initializing
> has_filename for the 'next' MapEntry object, which leads to interesting
> errors in Valgrind and Clang -fsanitize=undefined both.
> 
> Zero the stack object at allocation AND make sure the utility to
> populate the fields properly marks has_filename as false if applicable.
> 
> Signed-off-by: John Snow 
> ---
>  qemu-img.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index f121980..5a85178 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -2231,6 +2231,9 @@ static int get_block_status(BlockDriverState *bs, 
> int64_t sector_num,
>  if (file && e->has_offset) {
>  e->has_filename = true;
>  e->filename = file->filename;
> +} else {
> +e->has_filename = false;
> +e->filename = NULL;
>  }
>  return 0;
>  }

I guess this fixes the bug, but wouldn't it actually be nicer to just
reinitialise the whole object? As everyone knows, I love compound
literals, so I'd make it one big assignment that zeroes everything that
isn't specified:

*e = (MapEntry) {
...
};

> @@ -2264,7 +2267,7 @@ static int img_map(int argc, char **argv)
>  BlockDriverState *bs;
>  const char *filename, *fmt, *output;
>  int64_t length;
> -MapEntry curr = { .length = 0 }, next;
> +MapEntry curr = { .length = 0 }, next = { .length = 0 };
>  int ret = 0;

At first I didn't quite understand what this was for, but I think you
tried to cover newly added fields. If you overwrite the whole struct
above, you wouldn't need to initialise it here any more.

Kevin

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

[Peter Maydell]

On 3 February 2016 at 18:59, Peter Maydell  wrote:
> ARM queue -- most interesting thing here is the raspberry pi 2
> board model.
>
> thanks
> -- PMM
>
>
> The following changes since commit 87574621b18f86eab295a2c207e0b42c77b5dfa0:
>
>   Merge remote-tracking branch 'remotes/kraxel/tags/pull-vga-20160203-1' into 
> staging (2016-02-03 12:23:48 +)
>
> are available in the git repository at:
>
>
>   git://git.linaro.org/people/pmaydell/qemu-arm.git 
> tags/pull-target-arm-20160203
>
> for you to fetch changes up to 1df7d1f9303aef9a2b1f703e887553416b7c0586:
>
>   raspi: add raspberry pi 2 machine (2016-02-03 15:00:47 +)
>
> 
> target-arm queue:
>  * virt-acpi-build: add always-on property for timer
>  * various fixes for EL2 and EL3 behaviour
>  * arm: virt-acpi: each MADT.GICC entry as enabled unconditionally
>  * target-arm: Don't report presence of EL2 if it doesn't exist
>  * raspi: add raspberry pi 2 machine
>

Applied, thanks.

-- PMM

Re: [Qemu-devel] [PATCH v2 00/10] virtio/vring: optimization patches

[Paolo Bonzini]

On 03/02/2016 13:08, Gonglei (Arei) wrote:
> 22.56%  qemu-kvm [.] address_space_translate
>  13.29%  qemu-kvm [.] qemu_get_ram_ptr

We could get rid of qemu_get_ram_ptr by storing the RAMBlock pointer
into the memory region, instead of the ram_addr_t value.  I'm happy to
answer any question if you want to do it.

Paolo

>   4.71%  qemu-kvm [.] phys_page_find
>   4.43%  qemu-kvm [.] address_space_translate_internal
>   3.47%  libpthread-2.19.so   [.] __pthread_mutex_unlock_usercnt
>   3.08%  qemu-kvm [.] qemu_ram_addr_from_host
>   2.62%  qemu-kvm [.] address_space_map
>   2.61%  libc-2.19.so [.] _int_malloc
>   2.58%  libc-2.19.so [.] _int_free
>   2.38%  libc-2.19.so [.] malloc
>   2.06%  libpthread-2.19.so   [.] pthread_mutex_lock
>   1.68%  libc-2.19.so [.] malloc_consolidate
>   1.35%  libc-2.19.so [.] __memcpy_sse2_unaligned
>   1.23%  qemu-kvm [.] lduw_le_phys
>   1.18%  qemu-kvm [.] find_next_zero_bit
>   1.02%  qemu-kvm [.] object_unref

Re: [Qemu-devel] [PATCH] nbd: avoid unaligned uint64_t store

[Paolo Bonzini]

On 04/02/2016 00:48, John Snow wrote:
> cpu_to_be64w can't be used to make unaligned stores, but stq_be_p can.
> The other stores in this routine are left alone, they're aligned already.
> 
> Signed-off-by: John Snow 
> ---
>  nbd/server.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/nbd/server.c b/nbd/server.c
> index 1ec79cf..5b65059 100644
> --- a/nbd/server.c
> +++ b/nbd/server.c
> @@ -441,7 +441,7 @@ static coroutine_fn int nbd_negotiate(NBDClientNewData 
> *data)
>  }
>  
>  assert ((client->exp->nbdflags & ~65535) == 0);
> -cpu_to_be64w((uint64_t*)(buf + 18), client->exp->size);
> +stq_be_p((uint64_t *)(buf + 18), client->exp->size);
>  cpu_to_be16w((uint16_t*)(buf + 26), client->exp->nbdflags | myflags);
>  if (nbd_negotiate_write(csock, buf + 18,
>  sizeof(buf) - 18) != sizeof(buf) - 18) {
> 

Let's change all of them.  But no need to send another patch.

Paolo

[Qemu-devel] [PATCH] migration: move page_cache.c to migration/

[Paolo Bonzini]

page_cache.c is part of the XBZRLE implementation, move it to the right
directory.

Signed-off-by: Paolo Bonzini 
---
 Makefile.objs  | 1 -
 migration/Makefile.objs| 2 +-
 page_cache.c => migration/page_cache.c | 0
 3 files changed, 1 insertion(+), 2 deletions(-)
 rename page_cache.c => migration/page_cache.c (100%)

diff --git a/Makefile.objs b/Makefile.objs
index 06b95c7..f950b0f 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -51,7 +51,6 @@ common-obj-$(CONFIG_LINUX) += fsdev/
 
 common-obj-y += migration/
 common-obj-y += qemu-char.o #aio.o
-common-obj-y += page_cache.o
 common-obj-y += qjson.o
 
 common-obj-$(CONFIG_SPICE) += spice-qemu-char.o
diff --git a/migration/Makefile.objs b/migration/Makefile.objs
index 0cac6d7..e5778de 100644
--- a/migration/Makefile.objs
+++ b/migration/Makefile.objs
@@ -1,7 +1,7 @@
 common-obj-y += migration.o tcp.o
 common-obj-y += vmstate.o
 common-obj-y += qemu-file.o qemu-file-buf.o qemu-file-unix.o qemu-file-stdio.o
-common-obj-y += xbzrle.o postcopy-ram.o
+common-obj-y += xbzrle.o page_cache.o postcopy-ram.o
 
 common-obj-$(CONFIG_RDMA) += rdma.o
 common-obj-$(CONFIG_POSIX) += exec.o unix.o fd.o
diff --git a/page_cache.c b/migration/page_cache.c
similarity index 100%
rename from page_cache.c
rename to migration/page_cache.c
-- 
2.5.0

[Qemu-devel] [PATCH 5/5] balloon: Use only 'pc-dimm' type dimm for ballooning

[Vladimir Sementsov-Ogievskiy]

For now there are only two dimm's: pc-dimm and nvdimm. This patch is
actually needed to disable ballooning on nvdimm. But, to avoid future
bugs, instead of disallowing nvdimm, we allow only pc-dimm. So, if
someone adds new dimm which should be balloon-able, then this ability
should be explicitly specified here.

Why ballooning for nvdimm should be disabled for now:

NVDIMM for now is planned to use as a backing store for DAX filesystem
in the guest and thus this memory is excluded from guest memory
management and LRUs.

In this case libvirt running QEMU along with configured balloon almost
immediately inflates balloon and effectively kill the guest as
qemu counts nvdimm as part of the ram.

Counting dimm devices as part of the ram for ballooning was started from
commit 463756d03:
 virtio-balloon: Fix balloon not working correctly when hotplug memory

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Signed-off-by: Denis V. Lunev 

CC: Stefan Hajnoczi 
CC: Xiao Guangrong 
CC: "Michael S. Tsirkin" 
CC: Igor Mammedov 
CC: Eric Blake 
CC: Markus Armbruster 
---
 hw/virtio/virtio-balloon.c | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index b9c1964..0415e07 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -18,6 +18,7 @@
 #include "qemu-common.h"
 #include "hw/virtio/virtio.h"
 #include "hw/i386/pc.h"
+#include "hw/mem/nvdimm.h"
 #include "cpu.h"
 #include "sysemu/balloon.h"
 #include "hw/virtio/virtio-balloon.h"
@@ -302,7 +303,10 @@ static ram_addr_t get_current_ram_size(void)
 pc_dimm_build_list(qdev_get_machine(), );
 for (item = list; item; item = g_slist_next(item)) {
 PCDIMMDevice *dimm = item->data;
-size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, NULL);
+if (!strcmp(object_get_typename(OBJECT(dimm)), TYPE_PC_DIMM)) {
+size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP,
+NULL);
+}
 }
 g_slist_free(list);
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 4/5] virtio-balloon: rewrite get_current_ram_size()

[Vladimir Sementsov-Ogievskiy]

Use pc_dimm_built_list() instead of qmp_pc_dimm_device_list()

Actually, Qapi is not related to this internal helper.

Signed-off-by: Vladimir Sementsov-Ogievskiy 

CC: Stefan Hajnoczi 
CC: Xiao Guangrong 
CC: "Michael S. Tsirkin" 
CC: Igor Mammedov 
CC: Eric Blake 
CC: Markus Armbruster 
---
 hw/virtio/virtio-balloon.c | 23 ++-
 1 file changed, 6 insertions(+), 17 deletions(-)

diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 6a4c4d2..b9c1964 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -296,26 +296,15 @@ static void virtio_balloon_get_config(VirtIODevice *vdev, 
uint8_t *config_data)
 
 static ram_addr_t get_current_ram_size(void)
 {
-MemoryDeviceInfoList *info_list = NULL;
-MemoryDeviceInfoList **prev = _list;
-MemoryDeviceInfoList *info;
+GSList *list = NULL, *item;
 ram_addr_t size = ram_size;
 
-qmp_pc_dimm_device_list(qdev_get_machine(), );
-for (info = info_list; info; info = info->next) {
-MemoryDeviceInfo *value = info->value;
-
-if (value) {
-switch (value->type) {
-case MEMORY_DEVICE_INFO_KIND_DIMM:
-size += value->u.dimm->size;
-break;
-default:
-break;
-}
-}
+pc_dimm_build_list(qdev_get_machine(), );
+for (item = list; item; item = g_slist_next(item)) {
+PCDIMMDevice *dimm = item->data;
+size += object_property_get_int(OBJECT(dimm), PC_DIMM_SIZE_PROP, NULL);
 }
-qapi_free_MemoryDeviceInfoList(info_list);
+g_slist_free(list);
 
 return size;
 }
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/9] machine: introduce MachineClass.possible_cpu_arch_ids() hook

[Igor Mammedov]

on x86 currently range 0..max_cpus is used to generate
architecture-dependent CPU ID (APIC Id) for each present
and possible CPUs. However architecture-dependent CPU IDs
list could be sparse and code that needs to enumerate
all IDs (ACPI) ended up doing guess work enumerating all
possible and impossible IDs up to
  apic_id_limit = x86_cpu_apic_id_from_index(max_cpus).

That leads to creation of MADT entries and Processor
objects in ACPI tables for not possible CPUs.
Fix it by allowing board specify a concrete list of
CPU IDs accourding its own rules (which for x86 depends
on topology). So that code that needs this list could
request it from board instead of trying to figure out
what IDs are correct on its own.

Signed-off-by: Igor Mammedov 
---
 hw/i386/pc.c| 16 
 include/hw/boards.h | 18 ++
 2 files changed, 34 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index d72246d..2fd8fc8 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1946,6 +1946,21 @@ static unsigned pc_cpu_index_to_socket_id(unsigned 
cpu_index)
 return topo.pkg_id;
 }
 
+static GArray *pc_possible_cpu_arch_ids(void)
+{
+int i;
+GArray *list = g_array_new (FALSE, FALSE, sizeof (CPUArchId));
+
+for (i = 0; i < max_cpus; i++) {
+CPUArchId val;
+
+val.arch_id = x86_cpu_apic_id_from_index(i);
+val.cpu = qemu_get_cpu_by_arch_id(val.arch_id);
+g_array_append_val(list, val);
+}
+return list;
+}
+
 static void pc_machine_class_init(ObjectClass *oc, void *data)
 {
 MachineClass *mc = MACHINE_CLASS(oc);
@@ -1968,6 +1983,7 @@ static void pc_machine_class_init(ObjectClass *oc, void 
*data)
 pcmc->save_tsc_khz = true;
 mc->get_hotplug_handler = pc_get_hotpug_handler;
 mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id;
+mc->possible_cpu_arch_ids = pc_possible_cpu_arch_ids;
 mc->default_boot_order = "cad";
 mc->hot_add_cpu = pc_hot_add_cpu;
 mc->max_cpus = 255;
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 0f30959..bd85f46 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -8,6 +8,7 @@
 #include "sysemu/accel.h"
 #include "hw/qdev.h"
 #include "qom/object.h"
+#include "qom/cpu.h"
 
 void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
   const char *name,
@@ -42,6 +43,16 @@ bool machine_dump_guest_core(MachineState *machine);
 bool machine_mem_merge(MachineState *machine);
 
 /**
+ * CPUArchId:
+ * @arch_id - architecture-dependent CPU ID of present or possible CPU
+ * @cpu - pointer to corresponding CPU object ii it's present on NULL otherwise
+ */
+typedef struct {
+uint64_t arch_id;
+struct CPUState *cpu;
+} CPUArchId;
+
+/**
  * MachineClass:
  * @get_hotplug_handler: this function is called during bus-less
  *device hotplug. If defined it returns pointer to an instance
@@ -57,6 +68,10 @@ bool machine_mem_merge(MachineState *machine);
  *Set only by old machines because they need to keep
  *compatibility on code that exposed QEMU_VERSION to guests in
  *the past (and now use qemu_hw_version()).
+ * @possible_cpu_arch_ids:
+ *Returns an array of @CPUArchId architecture-dependent CPU IDs
+ *which includes CPU IDs for present and possible to hotplug CPUs.
+ *Caller is responsible for freeing returned list.
  */
 struct MachineClass {
 /*< private >*/
@@ -99,8 +114,11 @@ struct MachineClass {
 HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
DeviceState *dev);
 unsigned (*cpu_index_to_socket_id)(unsigned cpu_index);
+GArray *(*possible_cpu_arch_ids)(void);
 };
 
+#define FETCH_CPU_ARCH_ID(array, idx) g_array_index(array, CPUArchId, idx)
+
 /**
  * MachineState:
  */
-- 
1.8.3.1

[Qemu-devel] [PATCH 5/9] pc: acpi: create Processor and Notify objects only for valid lapics

[Igor Mammedov]

do not assume that all lapics in range 0..apic_id_limit
are valid and do not create Processor and Notify objects
for not possible lapics.

Signed-off-by: Igor Mammedov 
---
 hw/i386/acpi-build.c | 33 +++--
 1 file changed, 23 insertions(+), 10 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 3077061..df13c7d 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -960,7 +960,8 @@ static Aml *build_crs(PCIHostState *host,
 }
 
 static void build_processor_devices(Aml *sb_scope, unsigned acpi_cpus,
-AcpiCpuInfo *cpu, AcpiPmInfo *pm)
+AcpiCpuInfo *cpu, AcpiPmInfo *pm,
+MachineState *machine)
 {
 int i;
 Aml *dev;
@@ -969,6 +970,8 @@ static void build_processor_devices(Aml *sb_scope, unsigned 
acpi_cpus,
 Aml *field;
 Aml *ifctx;
 Aml *method;
+MachineClass *mc = MACHINE_GET_CLASS(machine);
+GArray *apic_id_list = mc->possible_cpu_arch_ids();
 
 /* The current AML generator can cover the APIC ID range [0..255],
  * inclusive, for VCPU hotplug. */
@@ -998,22 +1001,27 @@ static void build_processor_devices(Aml *sb_scope, 
unsigned acpi_cpus,
 aml_append(sb_scope, field);
 
 /* build Processor object for each processor */
-for (i = 0; i < acpi_cpus; i++) {
-dev = aml_processor(i, 0, 0, "CP%.02X", i);
+for (i = 0; i < apic_id_list->len; i++) {
+CPUArchId id = FETCH_CPU_ARCH_ID(apic_id_list, i);
+int apic_id = id.arch_id;
+
+assert(apic_id < ACPI_CPU_HOTPLUG_ID_LIMIT);
+dev = aml_processor(apic_id, 0, 0, "CP%.02X", apic_id);
 
 method = aml_method("_MAT", 0, AML_NOTSERIALIZED);
 aml_append(method,
-aml_return(aml_call1(CPU_MAT_METHOD, aml_int(i;
+aml_return(aml_call1(CPU_MAT_METHOD, aml_int(apic_id;
 aml_append(dev, method);
 
 method = aml_method("_STA", 0, AML_NOTSERIALIZED);
 aml_append(method,
-aml_return(aml_call1(CPU_STATUS_METHOD, aml_int(i;
+aml_return(aml_call1(CPU_STATUS_METHOD, aml_int(apic_id;
 aml_append(dev, method);
 
 method = aml_method("_EJ0", 1, AML_NOTSERIALIZED);
 aml_append(method,
-aml_return(aml_call2(CPU_EJECT_METHOD, aml_int(i), aml_arg(0)))
+aml_return(aml_call2(CPU_EJECT_METHOD, aml_int(apic_id),
+aml_arg(0)))
 );
 aml_append(dev, method);
 
@@ -1025,10 +1033,13 @@ static void build_processor_devices(Aml *sb_scope, 
unsigned acpi_cpus,
  */
 /* Arg0 = Processor ID = APIC ID */
 method = aml_method(AML_NOTIFY_METHOD, 2, AML_NOTSERIALIZED);
-for (i = 0; i < acpi_cpus; i++) {
-ifctx = aml_if(aml_equal(aml_arg(0), aml_int(i)));
+for (i = 0; i < apic_id_list->len; i++) {
+CPUArchId id = FETCH_CPU_ARCH_ID(apic_id_list, i);
+int apic_id = id.arch_id;
+
+ifctx = aml_if(aml_equal(aml_arg(0), aml_int(apic_id)));
 aml_append(ifctx,
-aml_notify(aml_name("CP%.02X", i), aml_arg(1))
+aml_notify(aml_name("CP%.02X", apic_id), aml_arg(1))
 );
 aml_append(method, ifctx);
 }
@@ -1049,6 +1060,7 @@ static void build_processor_devices(Aml *sb_scope, 
unsigned acpi_cpus,
 aml_append(pkg, aml_int(b));
 }
 aml_append(sb_scope, aml_name_decl(CPU_ON_BITMAP, pkg));
+g_array_free(apic_id_list, true);
 }
 
 static void build_memory_devices(Aml *sb_scope, int nr_mem,
@@ -2244,7 +2256,8 @@ build_dsdt(GArray *table_data, GArray *linker,
 
 sb_scope = aml_scope("\\_SB");
 {
-build_processor_devices(sb_scope, guest_info->apic_id_limit, cpu, pm);
+build_processor_devices(sb_scope, guest_info->apic_id_limit, cpu, pm,
+machine);
 
 build_memory_devices(sb_scope, nr_mem, pm->mem_hp_io_base,
  pm->mem_hp_io_len);
-- 
1.8.3.1

[Qemu-devel] [PATCH 6/9] pc: acpi: create MADT.lapic entries only for valid lapics

[Igor Mammedov]

do not assume that all lapics in range 0..apic_id_limit
are valid and do not create lapic entries for not
possible lapics in MADT.

Signed-off-by: Igor Mammedov 
---
 hw/i386/acpi-build.c | 21 ++---
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index df13c7d..9eeeffa 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -361,9 +361,11 @@ build_fadt(GArray *table_data, GArray *linker, AcpiPmInfo 
*pm,
 }
 
 static void
-build_madt(GArray *table_data, GArray *linker, AcpiCpuInfo *cpu,
-   PcGuestInfo *guest_info)
+build_madt(GArray *table_data, GArray *linker,
+   MachineState *machine, PcGuestInfo *guest_info)
 {
+MachineClass *mc = MACHINE_GET_CLASS(machine);
+GArray *apic_id_list = mc->possible_cpu_arch_ids();
 int madt_start = table_data->len;
 
 AcpiMultipleApicTable *madt;
@@ -376,18 +378,23 @@ build_madt(GArray *table_data, GArray *linker, 
AcpiCpuInfo *cpu,
 madt->local_apic_address = cpu_to_le32(APIC_DEFAULT_ADDRESS);
 madt->flags = cpu_to_le32(1);
 
-for (i = 0; i < guest_info->apic_id_limit; i++) {
+for (i = 0; i < apic_id_list->len; i++) {
 AcpiMadtProcessorApic *apic = acpi_data_push(table_data, sizeof *apic);
+CPUArchId id = FETCH_CPU_ARCH_ID(apic_id_list, i);
+int apic_id = id.arch_id;
+
 apic->type = ACPI_APIC_PROCESSOR;
 apic->length = sizeof(*apic);
-apic->processor_id = i;
-apic->local_apic_id = i;
-if (test_bit(i, cpu->found_cpus)) {
+apic->processor_id = apic_id;
+apic->local_apic_id = apic_id;
+if (id.cpu != NULL) {
 apic->flags = cpu_to_le32(1);
 } else {
 apic->flags = cpu_to_le32(0);
 }
 }
+g_array_free(apic_id_list, true);
+
 io_apic = acpi_data_push(table_data, sizeof *io_apic);
 io_apic->type = ACPI_APIC_IO;
 io_apic->length = sizeof(*io_apic);
@@ -2659,7 +2666,7 @@ void acpi_build(PcGuestInfo *guest_info, AcpiBuildTables 
*tables)
 aml_len += tables_blob->len - fadt;
 
 acpi_add_table(table_offsets, tables_blob);
-build_madt(tables_blob, tables->linker, , guest_info);
+build_madt(tables_blob, tables->linker, machine, guest_info);
 
 if (misc.has_hpet) {
 acpi_add_table(table_offsets, tables_blob);
-- 
1.8.3.1

Re: [Qemu-devel] qdev & hw/core owner? (was Re: [PATCH v19 7/9] machine: add properties to compat_props incrementaly)

[Michael S. Tsirkin]

On Thu, Feb 04, 2016 at 12:55:22PM +0100, Paolo Bonzini wrote:
> 
> 
> On 04/02/2016 12:41, Andreas Färber wrote:
> > You're talking about machine, right? Some time ago I had proposed Marcel
> > who initially worked on it, but I'm fine with anyone taking it.
> 
> Yes.
> 
> > For some (but not all) core qdev parts related to the (stalled) QOM
> > migration I've been taking care of via qom-next. Last time this came up
> > you didn't want anyone to be M: for qdev, so maybe we can use R: so that
> > at least people automatically get CC'ed and we avoid this recurring
> > discussion?
> 
> I might have changed my mind on that.  You definitely should be M: for qdev.
> 
> Paolo

If Andreas wants to, that's also fine. Several maintainers are
better than one.

-- 
MST

[Qemu-devel] [PATCH v2 2/6] migration: Rename the'file' member of MigrationState

[zhanghailiang]

Rename the 'file' memory of MigrationState to 'to_dst_file' to
be consistent with to_src_file, from_src_file and from_dst_file.

Signed-off-by: zhanghailiang 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Amit Shah 
---
 include/migration/migration.h |  2 +-
 migration/exec.c  |  4 +--
 migration/fd.c|  4 +--
 migration/migration.c | 72 ++-
 migration/postcopy-ram.c  |  6 ++--
 migration/rdma.c  |  2 +-
 migration/savevm.c|  2 +-
 migration/tcp.c   |  4 +--
 migration/unix.c  |  4 +--
 9 files changed, 52 insertions(+), 48 deletions(-)

diff --git a/include/migration/migration.h b/include/migration/migration.h
index 0fc1ffa..74684ad 100644
--- a/include/migration/migration.h
+++ b/include/migration/migration.h
@@ -133,7 +133,7 @@ struct MigrationState
 size_t xfer_limit;
 QemuThread thread;
 QEMUBH *cleanup_bh;
-QEMUFile *file;
+QEMUFile *to_dst_file;
 int parameters[MIGRATION_PARAMETER__MAX];
 
 int state;
diff --git a/migration/exec.c b/migration/exec.c
index f1b5e47..62f892d 100644
--- a/migration/exec.c
+++ b/migration/exec.c
@@ -36,8 +36,8 @@
 
 void exec_start_outgoing_migration(MigrationState *s, const char *command, 
Error **errp)
 {
-s->file = qemu_popen_cmd(command, "w");
-if (s->file == NULL) {
+s->to_dst_file = qemu_popen_cmd(command, "w");
+if (s->to_dst_file == NULL) {
 error_setg_errno(errp, errno, "failed to popen the migration target");
 return;
 }
diff --git a/migration/fd.c b/migration/fd.c
index c0afc96..085dd7c 100644
--- a/migration/fd.c
+++ b/migration/fd.c
@@ -51,9 +51,9 @@ void fd_start_outgoing_migration(MigrationState *s, const 
char *fdname, Error **
 }
 
 if (fd_is_socket(fd)) {
-s->file = qemu_fopen_socket(fd, "wb");
+s->to_dst_file = qemu_fopen_socket(fd, "wb");
 } else {
-s->file = qemu_fdopen(fd, "wb");
+s->to_dst_file = qemu_fdopen(fd, "wb");
 }
 
 migrate_fd_connect(s);
diff --git a/migration/migration.c b/migration/migration.c
index 2c76998..4971722 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -809,7 +809,7 @@ static void migrate_fd_cleanup(void *opaque)
 
 flush_page_queue(s);
 
-if (s->file) {
+if (s->to_dst_file) {
 trace_migrate_fd_cleanup();
 qemu_mutex_unlock_iothread();
 if (s->migration_thread_running) {
@@ -819,8 +819,8 @@ static void migrate_fd_cleanup(void *opaque)
 qemu_mutex_lock_iothread();
 
 migrate_compress_threads_join();
-qemu_fclose(s->file);
-s->file = NULL;
+qemu_fclose(s->to_dst_file);
+s->to_dst_file = NULL;
 }
 
 assert((s->state != MIGRATION_STATUS_ACTIVE) &&
@@ -837,7 +837,7 @@ static void migrate_fd_cleanup(void *opaque)
 void migrate_fd_error(MigrationState *s)
 {
 trace_migrate_fd_error();
-assert(s->file == NULL);
+assert(s->to_dst_file == NULL);
 migrate_set_state(>state, MIGRATION_STATUS_SETUP,
   MIGRATION_STATUS_FAILED);
 notifier_list_notify(_state_notifiers, s);
@@ -846,7 +846,7 @@ void migrate_fd_error(MigrationState *s)
 static void migrate_fd_cancel(MigrationState *s)
 {
 int old_state ;
-QEMUFile *f = migrate_get_current()->file;
+QEMUFile *f = migrate_get_current()->to_dst_file;
 trace_migrate_fd_cancel();
 
 if (s->rp_state.from_dst_file) {
@@ -917,7 +917,7 @@ MigrationState *migrate_init(const MigrationParams *params)
 s->bytes_xfer = 0;
 s->xfer_limit = 0;
 s->cleanup_bh = 0;
-s->file = NULL;
+s->to_dst_file = NULL;
 s->state = MIGRATION_STATUS_NONE;
 s->params = *params;
 s->rp_state.from_dst_file = NULL;
@@ -1096,8 +1096,9 @@ void qmp_migrate_set_speed(int64_t value, Error **errp)
 
 s = migrate_get_current();
 s->bandwidth_limit = value;
-if (s->file) {
-qemu_file_set_rate_limit(s->file, s->bandwidth_limit / 
XFER_LIMIT_RATIO);
+if (s->to_dst_file) {
+qemu_file_set_rate_limit(s->to_dst_file,
+ s->bandwidth_limit / XFER_LIMIT_RATIO);
 }
 }
 
@@ -1367,7 +1368,7 @@ out:
 static int open_return_path_on_source(MigrationState *ms)
 {
 
-ms->rp_state.from_dst_file = qemu_file_get_return_path(ms->file);
+ms->rp_state.from_dst_file = qemu_file_get_return_path(ms->to_dst_file);
 if (!ms->rp_state.from_dst_file) {
 return -1;
 }
@@ -1389,7 +1390,7 @@ static int 
await_return_path_close_on_source(MigrationState *ms)
  * rp_thread will exit, however if there's an error we need to cause
  * it to exit.
  */
-if (qemu_file_get_error(ms->file) && ms->rp_state.from_dst_file) {
+if (qemu_file_get_error(ms->to_dst_file) && ms->rp_state.from_dst_file) {
 /*
  * shutdown(2), if we have it, will cause 

[Qemu-devel] [PATCH v2 4/6] migration/ram: Fix some helper functions' parameter to use PageSearchStatus

[zhanghailiang]

Some helper functions use parameters 'RAMBlock *block' and 'ram_addr_t *offset',
We can use 'PageSearchStatus *pss' directly instead, with this change, we
can reduce the number of parameters for these helper function, also
it is easily to add new parameters for these helper functions.

Signed-off-by: zhanghailiang 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Amit Shah 
---
 migration/ram.c | 33 +++--
 1 file changed, 19 insertions(+), 14 deletions(-)

diff --git a/migration/ram.c b/migration/ram.c
index b541ceb..3cdfea4 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -729,7 +729,7 @@ static int save_zero_page(QEMUFile *f, RAMBlock *block, 
ram_addr_t offset,
  * @last_stage: if we are at the completion stage
  * @bytes_transferred: increase it with the number of transferred bytes
  */
-static int ram_save_page(QEMUFile *f, RAMBlock* block, ram_addr_t offset,
+static int ram_save_page(QEMUFile *f, PageSearchStatus *pss,
  bool last_stage, uint64_t *bytes_transferred)
 {
 int pages = -1;
@@ -738,6 +738,8 @@ static int ram_save_page(QEMUFile *f, RAMBlock* block, 
ram_addr_t offset,
 uint8_t *p;
 int ret;
 bool send_async = true;
+RAMBlock *block = pss->block;
+ram_addr_t offset = pss->offset;
 
 p = block->host + offset;
 
@@ -912,14 +914,16 @@ static int compress_page_with_multi_thread(QEMUFile *f, 
RAMBlock *block,
  * @last_stage: if we are at the completion stage
  * @bytes_transferred: increase it with the number of transferred bytes
  */
-static int ram_save_compressed_page(QEMUFile *f, RAMBlock *block,
-ram_addr_t offset, bool last_stage,
+static int ram_save_compressed_page(QEMUFile *f, PageSearchStatus *pss,
+bool last_stage,
 uint64_t *bytes_transferred)
 {
 int pages = -1;
 uint64_t bytes_xmit;
 uint8_t *p;
 int ret;
+RAMBlock *block = pss->block;
+ram_addr_t offset = pss->offset;
 
 p = block->host + offset;
 
@@ -1229,7 +1233,7 @@ err:
  * Returns: Number of pages written.
  */
 static int ram_save_target_page(MigrationState *ms, QEMUFile *f,
-RAMBlock *block, ram_addr_t offset,
+PageSearchStatus *pss,
 bool last_stage,
 uint64_t *bytes_transferred,
 ram_addr_t dirty_ram_abs)
@@ -1240,11 +1244,11 @@ static int ram_save_target_page(MigrationState *ms, 
QEMUFile *f,
 if (migration_bitmap_clear_dirty(dirty_ram_abs)) {
 unsigned long *unsentmap;
 if (compression_switch && migrate_use_compression()) {
-res = ram_save_compressed_page(f, block, offset,
+res = ram_save_compressed_page(f, pss,
last_stage,
bytes_transferred);
 } else {
-res = ram_save_page(f, block, offset, last_stage,
+res = ram_save_page(f, pss, last_stage,
 bytes_transferred);
 }
 
@@ -1260,7 +1264,7 @@ static int ram_save_target_page(MigrationState *ms, 
QEMUFile *f,
  * to the stream.
  */
 if (res > 0) {
-last_sent_block = block;
+last_sent_block = pss->block;
 }
 }
 
@@ -1284,26 +1288,27 @@ static int ram_save_target_page(MigrationState *ms, 
QEMUFile *f,
  * @bytes_transferred: increase it with the number of transferred bytes
  * @dirty_ram_abs: Address of the start of the dirty page in ram_addr_t space
  */
-static int ram_save_host_page(MigrationState *ms, QEMUFile *f, RAMBlock *block,
-  ram_addr_t *offset, bool last_stage,
+static int ram_save_host_page(MigrationState *ms, QEMUFile *f,
+  PageSearchStatus *pss,
+  bool last_stage,
   uint64_t *bytes_transferred,
   ram_addr_t dirty_ram_abs)
 {
 int tmppages, pages = 0;
 do {
-tmppages = ram_save_target_page(ms, f, block, *offset, last_stage,
+tmppages = ram_save_target_page(ms, f, pss, last_stage,
 bytes_transferred, dirty_ram_abs);
 if (tmppages < 0) {
 return tmppages;
 }
 
 pages += tmppages;
-*offset += TARGET_PAGE_SIZE;
+pss->offset += TARGET_PAGE_SIZE;
 dirty_ram_abs += TARGET_PAGE_SIZE;
-} while (*offset & (qemu_host_page_size - 1));
+} while (pss->offset & (qemu_host_page_size - 1));
 
 /* The offset we leave with is the last one we looked at */
-*offset -= TARGET_PAGE_SIZE;
+pss->offset -= TARGET_PAGE_SIZE;
 return pages;
 }
 
@@ -1351,7 +1356,7 @@ static int 

[Qemu-devel] [PATCH v2 1/6] ram: Split host_from_stream_offset() into two helper functions

[zhanghailiang]

Split host_from_stream_offset() into two parts:
One is to get ram block, which the block idstr may be get from migration
stream, the other is to get hva (host) address from block and the offset.
Besides, we will do the check working in a new helper offset_in_ramblock().

Signed-off-by: zhanghailiang 
Reviewed-by: Dr. David Alan Gilbert 
Reviewed-by: Amit Shah 
---
 include/exec/ram_addr.h |  8 ++--
 migration/ram.c | 40 +---
 2 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
index ef1489d..606e277 100644
--- a/include/exec/ram_addr.h
+++ b/include/exec/ram_addr.h
@@ -38,10 +38,14 @@ struct RAMBlock {
 int fd;
 };
 
+static inline bool offset_in_ramblock(RAMBlock *b, ram_addr_t offset)
+{
+return (b && b->host && offset < b->used_length) ? true : false;
+}
+
 static inline void *ramblock_ptr(RAMBlock *block, ram_addr_t offset)
 {
-assert(offset < block->used_length);
-assert(block->host);
+assert(offset_in_ramblock(block, offset));
 return (char *)block->host + offset;
 }
 
diff --git a/migration/ram.c b/migration/ram.c
index 40d0533..b541ceb 100644
--- a/migration/ram.c
+++ b/migration/ram.c
@@ -2124,28 +2124,24 @@ static int load_xbzrle(QEMUFile *f, ram_addr_t addr, 
void *host)
  * Returns a pointer from within the RCU-protected ram_list.
  */
 /*
- * Read a RAMBlock ID from the stream f, find the host address of the
- * start of that block and add on 'offset'
+ * Read a RAMBlock ID from the stream f.
  *
  * f: Stream to read from
- * offset: Offset within the block
  * flags: Page flags (mostly to see if it's a continuation of previous block)
  */
-static inline void *host_from_stream_offset(QEMUFile *f,
-ram_addr_t offset,
-int flags)
+static inline RAMBlock *ram_block_from_stream(QEMUFile *f,
+  int flags)
 {
 static RAMBlock *block = NULL;
 char id[256];
 uint8_t len;
 
 if (flags & RAM_SAVE_FLAG_CONTINUE) {
-if (!block || block->max_length <= offset) {
+if (!block) {
 error_report("Ack, bad migration stream!");
 return NULL;
 }
-
-return block->host + offset;
+return block;
 }
 
 len = qemu_get_byte(f);
@@ -2153,12 +2149,22 @@ static inline void *host_from_stream_offset(QEMUFile *f,
 id[len] = 0;
 
 block = qemu_ram_block_by_name(id);
-if (block && block->max_length > offset) {
-return block->host + offset;
+if (!block) {
+error_report("Can't find block %s", id);
+return NULL;
 }
 
-error_report("Can't find block %s", id);
-return NULL;
+return block;
+}
+
+static inline void *host_from_ram_block_offset(RAMBlock *block,
+   ram_addr_t offset)
+{
+if (!offset_in_ramblock(block, offset)) {
+return NULL;
+}
+
+return block->host + offset;
 }
 
 /*
@@ -2302,7 +2308,9 @@ static int ram_load_postcopy(QEMUFile *f)
 trace_ram_load_postcopy_loop((uint64_t)addr, flags);
 place_needed = false;
 if (flags & (RAM_SAVE_FLAG_COMPRESS | RAM_SAVE_FLAG_PAGE)) {
-host = host_from_stream_offset(f, addr, flags);
+RAMBlock *block = ram_block_from_stream(f, flags);
+
+host = host_from_ram_block_offset(block, addr);
 if (!host) {
 error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
 ret = -EINVAL;
@@ -2433,7 +2441,9 @@ static int ram_load(QEMUFile *f, void *opaque, int 
version_id)
 
 if (flags & (RAM_SAVE_FLAG_COMPRESS | RAM_SAVE_FLAG_PAGE |
  RAM_SAVE_FLAG_COMPRESS_PAGE | RAM_SAVE_FLAG_XBZRLE)) {
-host = host_from_stream_offset(f, addr, flags);
+RAMBlock *block = ram_block_from_stream(f, flags);
+
+host = host_from_ram_block_offset(block, addr);
 if (!host) {
 error_report("Illegal RAM offset " RAM_ADDR_FMT, addr);
 ret = -EINVAL;
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 09/11] block: m25p80: Implemented FSR register

[marcin . krzeminski]

From: Marcin Krzeminski 

Implements FSR register, it is used for busy waits.

Signed-off-by: Marcin Krzeminski 
---
 hw/block/m25p80.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index faf1212..116e098 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -222,6 +222,7 @@ typedef enum {
 WREN = 0x6,
 JEDEC_READ = 0x9f,
 BULK_ERASE = 0xc7,
+READ_FSR = 0x70,
 
 READ = 0x3,
 READ4 = 0x13,
@@ -640,6 +641,16 @@ static void decode_new_cmd(Flash *s, uint32_t value)
 s->state = STATE_READING_DATA;
 break;
 
+case READ_FSR:
+s->data[0] = (1<<7); /*Indicates flash is ready */
+if (s->four_bytes_address_mode) {
+s->data[0] |= 0x1;
+}
+s->pos = 0;
+s->len = 1;
+s->state = STATE_READING_DATA;
+break;
+
 case JEDEC_READ:
 DB_PRINT_L(0, "populated jedec code\n");
 s->data[0] = (s->pi->jedec >> 16) & 0xff;
-- 
2.5.0

[Qemu-devel] [PATCH v2 07/11] block: m25p80: Dummy cycles for N25Q256/512

[marcin . krzeminski]

From: Marcin Krzeminski 

This patch handles dummy cycles.

Signed-off-by: Marcin Krzeminski 
---
 hw/block/m25p80.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/hw/block/m25p80.c b/hw/block/m25p80.c
index a213587..df1f829 100644
--- a/hw/block/m25p80.c
+++ b/hw/block/m25p80.c
@@ -531,6 +531,10 @@ static void decode_new_cmd(Flash *s, uint32_t value)
 case DOR:
 case QOR:
 s->needed_bytes = get_cmd_length(s);
+if ( ((s->pi->jedec >> 16) & 0xFF) == JEDEC_NUMONYX ) {
+/* Dummy cycles - modeled with bytes writes instead of bits */
+s->needed_bytes += extract32(s->volatile_cfg,4,4);
+}
 s->pos = 0;
 s->len = 0;
 s->state = STATE_COLLECTING_DATA;
@@ -544,6 +548,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
 break;
 case JEDEC_NUMONYX:
 s->needed_bytes = get_cmd_length(s);
+s->needed_bytes += extract32(s->volatile_cfg,4,4);
 break;
 default:
 s->needed_bytes = 5;
@@ -561,6 +566,7 @@ static void decode_new_cmd(Flash *s, uint32_t value)
 break;
 case JEDEC_NUMONYX:
 s->needed_bytes = get_cmd_length(s);
+s->needed_bytes += extract32(s->volatile_cfg,4,4);
 break;
 default:
 s->needed_bytes = 8;
-- 
2.5.0

Re: [Qemu-devel] sda abort with virtio-scsi

[Paolo Bonzini]

On 04/02/2016 00:34, Jim Minter wrote:
> I was worried there was
> some way in which the contention could cause an abort and perhaps thence
> the lockup (which does not seem to recover when the host load goes down).

I don't know... It's not the most tested code, but it is not very
complicated either.

The certain points that can be extracted from the kernel messages are:
1) there was a cancellation request that took a long time, >20 seconds;
2) despite taking a long time, it _did_ recover sooner or later because
otherwise you'd not have the lockup splat either.

Paolo

>> Firing the NMI watchdog is fixed in more recent QEMU, which has
>> asynchronous cancellation, assuming you're running RHEL's QEMU 1.5.3
>> (try /usr/libexec/qemu-kvm --version, or rpm -qf /usr/libexec/qemu-kvm).
> 
> /usr/libexec/qemu-kvm --version reports QEMU emulator version 1.5.3
> (qemu-kvm-1.5.3-105.el7_2.3)

Re: [Qemu-devel] [PATCH 2/6] migration: Rename the'file' member of MigrationState

[Amit Shah]

On (Thu) 04 Feb 2016 [10:50:16], Dr. David Alan Gilbert wrote:
> * Amit Shah (amit.s...@redhat.com) wrote:
> > On (Fri) 15 Jan 2016 [11:37:42], zhanghailiang wrote:
> > > Rename the 'file' member of MigrationState to 'to_dst_file'.
> > 
> > Why?
> > 
> > (Use this space to explain why you're doing it, rather than repeating
> > the first line of the commit.  It helps when reviewing the git logs
> > later.)
> 
> I think the simple answer is consistency; now we've got a bidirectional
> connection we've already got to_src_file, from_src_file and from_dst_file,
> so 'file' is an oddity that I was too lazy to rename previously.

Sure, I just mean we need better commit description.  Just provide
one, and I'll use that when doing the pull req.

Thanks,

Amit

Re: [Qemu-devel] [PATCH v5 0/5] don't use NVDIMM for balooning

[Igor Mammedov]

On Thu,  4 Feb 2016 14:37:18 +0300
Vladimir Sementsov-Ogievskiy  wrote:

CCing Luiz as he might be interested in balloon stuff.

> v5: do not use qapi
>  0002-0004: new patches
>  0005: white list instead of black list
> 
> v4:
>  0001: Reviewed-by: Eric Blake 
>  second patch is splitted to 0002 and 0003
>  0002: Add 'type' field instead of 'balloonable' to PCDIMMDeviceInfo
>  0003: chec 'type' instead of 'balloonable'
> 
> v3:
> - do not use additional class variable
> 
> NVDIMM for now is planned to use as a backing store for DAX filesystem
> in the guest and thus this memory is excluded from guest memory
> management and LRUs.
> 
> In this case libvirt running QEMU along with configured balloon almost
> immediately inflates balloon and effectively kill the guest as
> qemu counts nvdimm as part of the ram.
> 
> CC: Vladimir Sementsov-Ogievskiy 
> CC: Denis V. Lunev 
> CC: Stefan Hajnoczi 
> CC: Xiao Guangrong 
> CC: "Michael S. Tsirkin" 
> CC: Igor Mammedov 
> CC: Eric Blake 
> CC: Markus Armbruster 
> 
> Vladimir Sementsov-Ogievskiy (5):
>   move get_current_ram_size to virtio-balloon.c
>   pc-dimm: rename pc_dimm_built_list()
>   pc-dimm: add pc_dimm_build_list()
>   virtio-balloon: rewrite get_current_ram_size()
>   balloon: Use only 'pc-dimm' type dimm for ballooning
> 
>  hw/mem/pc-dimm.c| 47 
> -
>  hw/virtio/virtio-balloon.c  | 19 +
>  include/exec/cpu-common.h   |  1 -
>  include/hw/mem/pc-dimm.h|  3 +++
>  stubs/qmp_pc_dimm_device_list.c |  5 -
>  5 files changed, 40 insertions(+), 35 deletions(-)
> 

Re: [Qemu-devel] [PATCH 2/9] machine: introduce MachineClass.possible_cpu_arch_ids() hook

[Marcel Apfelbaum]

On 02/04/2016 01:47 PM, Igor Mammedov wrote:

on x86 currently range 0..max_cpus is used to generate
architecture-dependent CPU ID (APIC Id) for each present
and possible CPUs. However architecture-dependent CPU IDs
list could be sparse and code that needs to enumerate
all IDs (ACPI) ended up doing guess work enumerating all
possible and impossible IDs up to
   apic_id_limit = x86_cpu_apic_id_from_index(max_cpus).

That leads to creation of MADT entries and Processor
objects in ACPI tables for not possible CPUs.
Fix it by allowing board specify a concrete list of
CPU IDs accourding its own rules (which for x86 depends
on topology). So that code that needs this list could
request it from board instead of trying to figure out
what IDs are correct on its own.

Signed-off-by: Igor Mammedov 
---
  hw/i386/pc.c| 16 
  include/hw/boards.h | 18 ++
  2 files changed, 34 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index d72246d..2fd8fc8 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1946,6 +1946,21 @@ static unsigned pc_cpu_index_to_socket_id(unsigned 
cpu_index)
  return topo.pkg_id;
  }

+static GArray *pc_possible_cpu_arch_ids(void)
+{
+int i;
+GArray *list = g_array_new (FALSE, FALSE, sizeof (CPUArchId));
+
+for (i = 0; i < max_cpus; i++) {
+CPUArchId val;
+
+val.arch_id = x86_cpu_apic_id_from_index(i);
+val.cpu = qemu_get_cpu_by_arch_id(val.arch_id);
+g_array_append_val(list, val);
+}
+return list;
+}
+
  static void pc_machine_class_init(ObjectClass *oc, void *data)
  {
  MachineClass *mc = MACHINE_CLASS(oc);
@@ -1968,6 +1983,7 @@ static void pc_machine_class_init(ObjectClass *oc, void 
*data)
  pcmc->save_tsc_khz = true;
  mc->get_hotplug_handler = pc_get_hotpug_handler;
  mc->cpu_index_to_socket_id = pc_cpu_index_to_socket_id;
+mc->possible_cpu_arch_ids = pc_possible_cpu_arch_ids;
  mc->default_boot_order = "cad";
  mc->hot_add_cpu = pc_hot_add_cpu;
  mc->max_cpus = 255;
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 0f30959..bd85f46 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -8,6 +8,7 @@
  #include "sysemu/accel.h"
  #include "hw/qdev.h"
  #include "qom/object.h"
+#include "qom/cpu.h"

  void memory_region_allocate_system_memory(MemoryRegion *mr, Object *owner,
const char *name,
@@ -42,6 +43,16 @@ bool machine_dump_guest_core(MachineState *machine);
  bool machine_mem_merge(MachineState *machine);

  /**
+ * CPUArchId:
+ * @arch_id - architecture-dependent CPU ID of present or possible CPU
+ * @cpu - pointer to corresponding CPU object ii it's present on NULL otherwise
+ */
+typedef struct {
+uint64_t arch_id;
+struct CPUState *cpu;
+} CPUArchId;
+
+/**
   * MachineClass:
   * @get_hotplug_handler: this function is called during bus-less
   *device hotplug. If defined it returns pointer to an instance
@@ -57,6 +68,10 @@ bool machine_mem_merge(MachineState *machine);
   *Set only by old machines because they need to keep
   *compatibility on code that exposed QEMU_VERSION to guests in
   *the past (and now use qemu_hw_version()).
+ * @possible_cpu_arch_ids:
+ *Returns an array of @CPUArchId architecture-dependent CPU IDs
+ *which includes CPU IDs for present and possible to hotplug CPUs.
+ *Caller is responsible for freeing returned list.
   */
  struct MachineClass {
  /*< private >*/
@@ -99,8 +114,11 @@ struct MachineClass {
  HotplugHandler *(*get_hotplug_handler)(MachineState *machine,
 DeviceState *dev);
  unsigned (*cpu_index_to_socket_id)(unsigned cpu_index);
+GArray *(*possible_cpu_arch_ids)(void);


Hi Igor,

Can't this be a GArray filled in at machine init time instead of a method?
Just wondering.

Thanks,
Marcel


  };

+#define FETCH_CPU_ARCH_ID(array, idx) g_array_index(array, CPUArchId, idx)
+
  /**
   * MachineState:
   */

[Qemu-devel] [PULL 05/49] virtio: move VirtQueueElement at the beginning of the structs

[Michael S. Tsirkin]

From: Paolo Bonzini 

The next patch will make virtqueue_pop/vring_pop allocate memory for
the VirtQueueElement. In some cases (blk, scsi, gpu) the device wants
to extend VirtQueueElement with device-specific fields and, until now,
the place of the VirtQueueElement within the containing struct didn't
matter. When allocating the entire block in virtqueue_pop/vring_pop,
however, the containing struct must basically be a "subclass" of
VirtQueueElement, with the VirtQueueElement as the first field. Make
that the case for blk and scsi; gpu is already doing it.

Signed-off-by: Paolo Bonzini 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
Reviewed-by: Cornelia Huck 
---
 include/hw/virtio/virtio-blk.h  |  2 +-
 include/hw/virtio/virtio-scsi.h | 13 ++---
 hw/scsi/virtio-scsi.c   |  3 +--
 3 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
index ae11a63..403ab86 100644
--- a/include/hw/virtio/virtio-blk.h
+++ b/include/hw/virtio/virtio-blk.h
@@ -60,9 +60,9 @@ typedef struct VirtIOBlock {
 } VirtIOBlock;
 
 typedef struct VirtIOBlockReq {
+VirtQueueElement elem;
 int64_t sector_num;
 VirtIOBlock *dev;
-VirtQueueElement elem;
 struct virtio_blk_inhdr *in;
 struct virtio_blk_outhdr out;
 QEMUIOVector qiov;
diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h
index 0394eb2..eb9d25b 100644
--- a/include/hw/virtio/virtio-scsi.h
+++ b/include/hw/virtio/virtio-scsi.h
@@ -112,18 +112,17 @@ typedef struct VirtIOSCSI {
 } VirtIOSCSI;
 
 typedef struct VirtIOSCSIReq {
+/* Note:
+ * - fields up to resp_iov are initialized by virtio_scsi_init_req;
+ * - fields starting at vring are zeroed by virtio_scsi_init_req.
+ * */
+VirtQueueElement elem;
+
 VirtIOSCSI *dev;
 VirtQueue *vq;
 QEMUSGList qsgl;
 QEMUIOVector resp_iov;
 
-/* Note:
- * - fields before elem are initialized by virtio_scsi_init_req;
- * - elem is uninitialized at the time of allocation.
- * - fields after elem are zeroed by virtio_scsi_init_req.
- * */
-
-VirtQueueElement elem;
 /* Set by dataplane code. */
 VirtIOSCSIVring *vring;
 
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 1500c42..7fdf6ad 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -45,8 +45,7 @@ VirtIOSCSIReq *virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue 
*vq)
 {
 VirtIOSCSIReq *req;
 VirtIOSCSICommon *vs = (VirtIOSCSICommon *)s;
-const size_t zero_skip = offsetof(VirtIOSCSIReq, elem)
- + sizeof(VirtQueueElement);
+const size_t zero_skip = offsetof(VirtIOSCSIReq, vring);
 
 req = g_malloc(sizeof(*req) + vs->cdb_size);
 req->vq = vq;
-- 
MST

[Qemu-devel] [PULL 25/49] ipmi: add ACPI power and GUID commands

[Michael S. Tsirkin]

From: Cédric Le Goater 

>From the specs (20.8 Get Device GUID Command), the command needs to
return a GUID (Globally Unique ID), or UUID, that should never change
over the lifetime of the device. qemu_uuid looked like a good
candidate to start with but we could use a specific BMC property also
if needed.

Signed-off-by: Cédric Le Goater 
Acked-by: Corey Minyard 
Acked-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/ipmi/ipmi_bmc_sim.c | 49 +
 1 file changed, 49 insertions(+)

diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
index 29224dd..e1ad19b 100644
--- a/hw/ipmi/ipmi_bmc_sim.c
+++ b/hw/ipmi/ipmi_bmc_sim.c
@@ -23,6 +23,7 @@
  */
 
 #include "qemu/osdep.h"
+#include "sysemu/sysemu.h"
 #include "qemu/timer.h"
 #include "hw/ipmi/ipmi.h"
 #include "qemu/error-report.h"
@@ -49,6 +50,9 @@
 #define IPMI_CMD_GET_DEVICE_ID0x01
 #define IPMI_CMD_COLD_RESET   0x02
 #define IPMI_CMD_WARM_RESET   0x03
+#define IPMI_CMD_SET_ACPI_POWER_STATE 0x06
+#define IPMI_CMD_GET_ACPI_POWER_STATE 0x07
+#define IPMI_CMD_GET_DEVICE_GUID  0x08
 #define IPMI_CMD_RESET_WATCHDOG_TIMER 0x22
 #define IPMI_CMD_SET_WATCHDOG_TIMER   0x24
 #define IPMI_CMD_GET_WATCHDOG_TIMER   0x25
@@ -198,6 +202,9 @@ struct IPMIBmcSim {
 
 uint8_t restart_cause;
 
+uint8_t acpi_power_state[2];
+uint8_t uuid[16];
+
 IPMISel sel;
 IPMISdr sdr;
 IPMISensor sensors[MAX_SENSORS];
@@ -826,6 +833,36 @@ static void warm_reset(IPMIBmcSim *ibs,
 k->reset(s, false);
 }
 }
+static void set_acpi_power_state(IPMIBmcSim *ibs,
+  uint8_t *cmd, unsigned int cmd_len,
+  uint8_t *rsp, unsigned int *rsp_len,
+  unsigned int max_rsp_len)
+{
+IPMI_CHECK_CMD_LEN(4);
+ibs->acpi_power_state[0] = cmd[2];
+ibs->acpi_power_state[1] = cmd[3];
+}
+
+static void get_acpi_power_state(IPMIBmcSim *ibs,
+  uint8_t *cmd, unsigned int cmd_len,
+  uint8_t *rsp, unsigned int *rsp_len,
+  unsigned int max_rsp_len)
+{
+IPMI_ADD_RSP_DATA(ibs->acpi_power_state[0]);
+IPMI_ADD_RSP_DATA(ibs->acpi_power_state[1]);
+}
+
+static void get_device_guid(IPMIBmcSim *ibs,
+  uint8_t *cmd, unsigned int cmd_len,
+  uint8_t *rsp, unsigned int *rsp_len,
+  unsigned int max_rsp_len)
+{
+unsigned int i;
+
+for (i = 0; i < 16; i++) {
+IPMI_ADD_RSP_DATA(ibs->uuid[i]);
+}
+}
 
 static void set_bmc_global_enables(IPMIBmcSim *ibs,
uint8_t *cmd, unsigned int cmd_len,
@@ -1608,6 +1645,9 @@ static const IPMICmdHandler app_cmds[] = {
 [IPMI_CMD_GET_DEVICE_ID] = get_device_id,
 [IPMI_CMD_COLD_RESET] = cold_reset,
 [IPMI_CMD_WARM_RESET] = warm_reset,
+[IPMI_CMD_SET_ACPI_POWER_STATE] = set_acpi_power_state,
+[IPMI_CMD_GET_ACPI_POWER_STATE] = get_acpi_power_state,
+[IPMI_CMD_GET_DEVICE_GUID] = get_device_guid,
 [IPMI_CMD_SET_BMC_GLOBAL_ENABLES] = set_bmc_global_enables,
 [IPMI_CMD_GET_BMC_GLOBAL_ENABLES] = get_bmc_global_enables,
 [IPMI_CMD_CLR_MSG_FLAGS] = clr_msg_flags,
@@ -1733,6 +1773,15 @@ static void ipmi_sim_init(Object *obj)
 i += len;
 }
 
+ibs->acpi_power_state[0] = 0;
+ibs->acpi_power_state[1] = 0;
+
+if (qemu_uuid_set) {
+memcpy(>uuid, qemu_uuid, 16);
+} else {
+memset(>uuid, 0, 16);
+}
+
 ipmi_init_sensors_from_sdrs(ibs);
 register_cmds(ibs);
 
-- 
MST

[Qemu-devel] [PULL 03/49] tests: pc: acpi: drop not needed 'expected SSDT' blobs

[Michael S. Tsirkin]

From: Igor Mammedov 

Signed-off-by: Igor Mammedov 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 tests/acpi-test-data/pc/SSDT | Bin 2486 -> 0 bytes
 tests/acpi-test-data/pc/SSDT.bridge  | Bin 4345 -> 0 bytes
 tests/acpi-test-data/q35/SSDT| Bin 691 -> 0 bytes
 tests/acpi-test-data/q35/SSDT.bridge | Bin 708 -> 0 bytes
 4 files changed, 0 insertions(+), 0 deletions(-)
 delete mode 100644 tests/acpi-test-data/pc/SSDT
 delete mode 100644 tests/acpi-test-data/pc/SSDT.bridge
 delete mode 100644 tests/acpi-test-data/q35/SSDT
 delete mode 100644 tests/acpi-test-data/q35/SSDT.bridge

diff --git a/tests/acpi-test-data/pc/SSDT b/tests/acpi-test-data/pc/SSDT
deleted file mode 100644
index 
210d6a71e58aa34ce8e94121d25bcf58c3bd503c..
GIT binary patch
literal 0
HcmV?d1

literal 2486
zcma)-Pjk~o7{=eVMWU=_A{(#pbjB{-fT-EjvFaES~~;d%;?ZzYGyhO1LeTv
zl7zwSyXMF3m{RqAF%6I5Yn^WEQmHaw6upNAm_tEp)XaBFX4tu{Y0Ej=^`$0$h
z?I$YG_)Y)-mFeG+px1B)wTA#{Nd*+lpM4HCiDIt{
zpndMlbag`DUq3Aa6h452$)rLk{z{NL)FS=t$>g_xi{kHpzk$J7VC|>~+wbg#Rh6hl
ztFaAs)<;F}{g=;P9ld_}?Xy?-QukW2QuMSc)^|QR(MGaBzO-=*Jh|
zzy9ujv*06q6y6(f?;KJ~n^}F%64+@hKgjB~B}=lDt1WV90B-BuAx+=l>>KMI&7n#o
zop-@${UPUyF$AjrWzCOaEPNcwiAr@(RA4Oq!KsrAd}K{`@OCp$jYctbo|DC`9()K?
z+yWFhGbD5mylNJ88v;etg`rnVdXJx$-pgNxFrw#-AoKlmx}s2!dm;$0X}r#dvh^$)Qykw-PlGKm$jlBjm*wkVT$6e-k1v9
zeN=zApF_RQYIu

[Qemu-devel] [PULL 07/49] virtio: introduce qemu_get/put_virtqueue_element

[Michael S. Tsirkin]

From: Paolo Bonzini 

Move allocation to virtio functions also when loading/saving a
VirtQueueElement.  This will also let the load/save functions
keep backwards compatibility when the VirtQueueElement layout
is changed.

Reviewed-by: Cornelia Huck 
Signed-off-by: Paolo Bonzini 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 include/hw/virtio/virtio.h  |  2 ++
 hw/block/virtio-blk.c   | 10 +++---
 hw/char/virtio-serial-bus.c | 10 +++---
 hw/scsi/virtio-scsi.c   |  7 ++-
 hw/virtio/virtio.c  | 13 +
 5 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 21fda17..44da9a8 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -153,6 +153,8 @@ void virtqueue_fill(VirtQueue *vq, const VirtQueueElement 
*elem,
 
 void virtqueue_map(VirtQueueElement *elem);
 void *virtqueue_pop(VirtQueue *vq, size_t sz);
+void *qemu_get_virtqueue_element(QEMUFile *f, size_t sz);
+void qemu_put_virtqueue_element(QEMUFile *f, VirtQueueElement *elem);
 int virtqueue_avail_bytes(VirtQueue *vq, unsigned int in_bytes,
   unsigned int out_bytes);
 void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int *in_bytes,
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index bf70b52..c427698 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -808,8 +808,7 @@ static void virtio_blk_save_device(VirtIODevice *vdev, 
QEMUFile *f)
 
 while (req) {
 qemu_put_sbyte(f, 1);
-qemu_put_buffer(f, (unsigned char *)>elem,
-sizeof(VirtQueueElement));
+qemu_put_virtqueue_element(f, >elem);
 req = req->next;
 }
 qemu_put_sbyte(f, 0);
@@ -832,14 +831,11 @@ static int virtio_blk_load_device(VirtIODevice *vdev, 
QEMUFile *f,
 VirtIOBlock *s = VIRTIO_BLK(vdev);
 
 while (qemu_get_sbyte(f)) {
-VirtIOBlockReq *req = g_new(VirtIOBlockReq, 1);
+VirtIOBlockReq *req;
+req = qemu_get_virtqueue_element(f, sizeof(VirtIOBlockReq));
 virtio_blk_init_request(s, req);
-qemu_get_buffer(f, (unsigned char *)>elem,
-sizeof(VirtQueueElement));
 req->next = s->rq;
 s->rq = req;
-
-virtqueue_map(>elem);
 }
 
 return 0;
diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index cf3d12b..99cb683 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -646,9 +646,7 @@ static void virtio_serial_save_device(VirtIODevice *vdev, 
QEMUFile *f)
 if (elem_popped) {
 qemu_put_be32s(f, >iov_idx);
 qemu_put_be64s(f, >iov_offset);
-
-qemu_put_buffer(f, (unsigned char *)port->elem,
-sizeof(VirtQueueElement));
+qemu_put_virtqueue_element(f, port->elem);
 }
 }
 }
@@ -723,10 +721,8 @@ static int fetch_active_ports_list(QEMUFile *f, int 
version_id,
 qemu_get_be32s(f, >iov_idx);
 qemu_get_be64s(f, >iov_offset);
 
-port->elem = g_new(VirtQueueElement, 1);
-qemu_get_buffer(f, (unsigned char *)port->elem,
-sizeof(VirtQueueElement));
-virtqueue_map(port->elem);
+port->elem =
+qemu_get_virtqueue_element(f, sizeof(VirtQueueElement));
 
 /*
  *  Port was throttled on source machine.  Let's
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 50a3cb2..5b29bac 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -189,7 +189,7 @@ static void virtio_scsi_save_request(QEMUFile *f, 
SCSIRequest *sreq)
 
 assert(n < vs->conf.num_queues);
 qemu_put_be32s(f, );
-qemu_put_buffer(f, (unsigned char *)>elem, sizeof(req->elem));
+qemu_put_virtqueue_element(f, >elem);
 }
 
 static void *virtio_scsi_load_request(QEMUFile *f, SCSIRequest *sreq)
@@ -202,12 +202,9 @@ static void *virtio_scsi_load_request(QEMUFile *f, 
SCSIRequest *sreq)
 
 qemu_get_be32s(f, );
 assert(n < vs->conf.num_queues);
-req = g_malloc(sizeof(VirtIOSCSIReq) + vs->cdb_size);
-qemu_get_buffer(f, (unsigned char *)>elem, sizeof(req->elem));
+req = qemu_get_virtqueue_element(f, sizeof(VirtIOSCSIReq) + vs->cdb_size);
 virtio_scsi_init_req(s, vs->cmd_vqs[n], req);
 
-virtqueue_map(>elem);
-
 if (virtio_scsi_parse_req(req, sizeof(VirtIOSCSICmdReq) + vs->cdb_size,
   sizeof(VirtIOSCSICmdResp) + vs->sense_size) < 0) 
{
 error_report("invalid SCSI request migration data");
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 229a092..28fa7fe 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -576,6 +576,19 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz)

[Qemu-devel] [PULL 35/49] pc: Move PcGuestInfo.fw_cfg to PCMachineState

[Michael S. Tsirkin]

From: Eduardo Habkost 

Signed-off-by: Eduardo Habkost 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
Reviewed-by: Marcel Apfelbaum 
---
 include/hw/i386/pc.h |  2 +-
 hw/i386/acpi-build.c |  7 +++
 hw/i386/pc.c | 10 --
 3 files changed, 8 insertions(+), 11 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 6a5c4da..40249b6 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -40,7 +40,6 @@ struct PcGuestInfo {
 uint64_t numa_nodes;
 uint64_t *node_mem;
 uint64_t *node_cpu;
-FWCfgState *fw_cfg;
 };
 
 /**
@@ -62,6 +61,7 @@ struct PCMachineState {
 HotplugHandler *acpi_dev;
 ISADevice *rtc;
 PCIBus *bus;
+FWCfgState *fw_cfg;
 
 /* Configuration options: */
 uint64_t max_ram_below_4g;
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index dac7137..86baf63 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2806,11 +2806,10 @@ void acpi_setup(void)
 {
 PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
 PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
-PcGuestInfo *guest_info = >acpi_guest_info;
 AcpiBuildTables tables;
 AcpiBuildState *build_state;
 
-if (!guest_info->fw_cfg) {
+if (!pcms->fw_cfg) {
 ACPI_BUILD_DPRINTF("No fw cfg. Bailing out.\n");
 return;
 }
@@ -2841,7 +2840,7 @@ void acpi_setup(void)
 build_state->linker_mr =
 acpi_add_rom_blob(build_state, tables.linker, "etc/table-loader", 0);
 
-fw_cfg_add_file(guest_info->fw_cfg, ACPI_BUILD_TPMLOG_FILE,
+fw_cfg_add_file(pcms->fw_cfg, ACPI_BUILD_TPMLOG_FILE,
 tables.tcpalog->data, acpi_data_len(tables.tcpalog));
 
 if (!pcmc->rsdp_in_ram) {
@@ -2853,7 +2852,7 @@ void acpi_setup(void)
 uint32_t rsdp_size = acpi_data_len(tables.rsdp);
 
 build_state->rsdp = g_memdup(tables.rsdp->data, rsdp_size);
-fw_cfg_add_file_callback(guest_info->fw_cfg, ACPI_BUILD_RSDP_FILE,
+fw_cfg_add_file_callback(pcms->fw_cfg, ACPI_BUILD_RSDP_FILE,
  acpi_build_update, build_state,
  build_state->rsdp, rsdp_size);
 build_state->rsdp_mr = NULL;
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index dc0ade4..a47784a 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1172,10 +1172,10 @@ void pc_machine_done(Notifier *notifier, void *data)
 extra_hosts++;
 }
 }
-if (extra_hosts && pcms->acpi_guest_info.fw_cfg) {
+if (extra_hosts && pcms->fw_cfg) {
 uint64_t *val = g_malloc(sizeof(*val));
 *val = cpu_to_le64(extra_hosts);
-fw_cfg_add_file(pcms->acpi_guest_info.fw_cfg,
+fw_cfg_add_file(pcms->fw_cfg,
 "etc/extra-pci-roots", val, sizeof(*val));
 }
 }
@@ -1257,7 +1257,6 @@ void xen_load_linux(PCMachineState *pcms)
 {
 int i;
 FWCfgState *fw_cfg;
-PcGuestInfo *guest_info = >acpi_guest_info;
 
 assert(MACHINE(pcms)->kernel_filename != NULL);
 
@@ -1270,7 +1269,7 @@ void xen_load_linux(PCMachineState *pcms)
!strcmp(option_rom[i].name, "multiboot.bin"));
 rom_add_option(option_rom[i].name, option_rom[i].bootindex);
 }
-guest_info->fw_cfg = fw_cfg;
+pcms->fw_cfg = fw_cfg;
 }
 
 void pc_memory_init(PCMachineState *pcms,
@@ -1278,7 +1277,6 @@ void pc_memory_init(PCMachineState *pcms,
 MemoryRegion *rom_memory,
 MemoryRegion **ram_memory)
 {
-PcGuestInfo *guest_info = >acpi_guest_info;
 int linux_boot, i;
 MemoryRegion *ram, *option_rom_mr;
 MemoryRegion *ram_below_4g, *ram_above_4g;
@@ -1399,7 +1397,7 @@ void pc_memory_init(PCMachineState *pcms,
 for (i = 0; i < nb_option_roms; i++) {
 rom_add_option(option_rom[i].name, option_rom[i].bootindex);
 }
-guest_info->fw_cfg = fw_cfg;
+pcms->fw_cfg = fw_cfg;
 }
 
 qemu_irq pc_allocate_cpu_irq(void)
-- 
MST

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

[Michael S. Tsirkin]

On Thu, Feb 04, 2016 at 03:15:39PM -0500, Alex Williamson wrote:
> 
> 
> - Original Message -
> > On Thu, Feb 04, 2016 at 10:46:52AM -0700, Alex Williamson wrote:
> > > On Thu, 4 Feb 2016 13:21:57 +0200
> > > "Michael S. Tsirkin"  wrote:
> > > 
> > > > On Thu, Feb 04, 2016 at 10:04:01AM +0800, Chen Fan wrote:
> > > > > 
> > > > > On 02/03/2016 09:57 PM, Michael S. Tsirkin wrote:
> > > > > >On Wed, Feb 03, 2016 at 04:54:01PM +0800, Chen Fan wrote:
> > > > > >>On 01/17/2016 02:34 AM, Michael S. Tsirkin wrote:
> > > > > >>>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.
> > > > > >>Hi Michael,
> > > > > >>
> > > > > >>Just think about this more,  I think we also should check the vfio
> > > > > >>devices whether on the same bus at the time of function 0 is
> > > > > >>added. because we don't know the affected devices by a bus reset
> > > > > >>have already all been assigned to VM.
> > > > > >This is something vfio in kernel should check.
> > > > > >You can't rely on qemu being well behaved, so don't
> > > > > >even try to catch cases which would break host in userspace.
> > > > > >
> > > > > >qemu should only worry about not breaking guest.
> > > > > >
> > > > > >  
> > > > > >>for example, the multi-function's hotplug.
> > > > > >>devices on same bus in host are added to VM one by one. when we
> > > > > >>test one device, we haven't yet added the other devices.
> > > > > >>so I think
> > > > > >>the patch should like below. then we could add a
> > > > > >>vfio_is_valid_function in vfio
> > > > > >>to test each device whether the affected devices on the same bus.
> > > > > >>
> > > > > >>Thanks,
> > > > > >>Chen
> > > > > >>
> > > > > >>diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > > > > >>index d940f79..7163b56 100644
> > > > > >>--- a/hw/pci/pci.c
> > > > > >>+++ b/hw/pci/pci.c
> > > > > >>@@ -1836,6 +1836,38 @@ PCIDevice *pci_find_device(PCIBus *bus,
> > > > > >>int bus_num, uint8_t devfn)
> > > > > >>  return bus->devices[devfn];
> > > > > >>  }
> > > > > >>
> > > > > >>+static int pci_bus_check_devices(PCIBus *bus)
> > > > > >>+{
> > > > > >>+PCIDeviceClass *pc;
> > > > > >>+int i, ret = 0;
> > > > > >>+
> > > > > >>+for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) {
> > > > > >>+if (!bus->devices[i]) {
> > > > > >>+continue;
> > > > > >>+}
> > > > > >>+
> > > > > >>+pc = PCI_DEVICE_GET_CLASS(bus->devices[i]);
> > > > > >>+if (!pc->is_valid_func) {
> > > > > >>+continue;
> > > > > >>+}
> > > > > >>+
> > > > > >>+ret = pc->is_valid_func(bus->devices[i], bus);
> > > > > >>+if (!ret) {
> > > > > >>+return -1;
> > > > > >>+}
> > > > > >>+}
> > > > > >>+return 0;
> > > > > >>+}
> > > > > >>+
> > > > > >>+static bool pci_is_valid_function(PCIDevice *pdev, PCIBus *bus)
> > > > > >>+{
> > > > > >>+   

Re: [Qemu-devel] [PATCH v2 03/17] crypto: add support for PBKDF2 algorithm

[Eric Blake]

On 01/20/2016 10:38 AM, Daniel P. Berrange wrote:
> The LUKS data format includes use of PBKDF2 (Password-Based
> Key Derivation Function). The Nettle library can provide
> an implementation of this, but we don't want code directly
> depending on a specific crypto library backend. Introduce
> a include/crypto/pbkdf.h header which defines a QEMU

'an include/...', or maybe 'a new include/...'?

> API for invoking PBKDK2. The initial implementations are
> backed by nettle & gcrypt, which are commonly available
> with distros shipping GNUTLS.
> 
> The test suite data is taken from the cryptsetup codebase
> under the LGPLv2.1+ license. This merely aims to verify
> that whatever backend we provide for this function in QEMU
> will comply with the spec.
> 
> Signed-off-by: Daniel P. Berrange 
> ---

In addition to Fam's review,

> +++ b/crypto/pbkdf-gcrypt.c

> +int qcrypto_pbkdf2(QCryptoHashAlgorithm hash,
> +   const uint8_t *key, size_t nkey,
> +   const uint8_t *salt, size_t nsalt,
> +   unsigned int iterations,
> +   uint8_t *out, size_t nout,
> +   Error **errp)
> +{
> +static const int hash_map[QCRYPTO_HASH_ALG__MAX] = {
> +[QCRYPTO_HASH_ALG_MD5] = GCRY_MD_MD5,
> +[QCRYPTO_HASH_ALG_SHA1] = GCRY_MD_SHA1,
> +[QCRYPTO_HASH_ALG_SHA256] = GCRY_MD_SHA256,
> +};

If QCRYPTO_HASH_ gains future enum values, those elements of the array
will be 0-initialized.

> +int ret;
> +
> +if (hash > G_N_ELEMENTS(hash_map)) {
> +error_setg(errp, "Unexpected hash algorithm %d", hash);
> +return -1;
> +}

This checks for beyond the bounds of the array, but not for an element
that was 0-initialized.  Is that a problem we need to worry about?

> +int qcrypto_pbkdf2(QCryptoHashAlgorithm hash,
> +   const uint8_t *key, size_t nkey,
> +   const uint8_t *salt, size_t nsalt,
> +   unsigned int iterations,
> +   uint8_t *out, size_t nout,
> +   Error **errp);
> +
> +/**
> + * qcrypto_pbkdf2_count_iters:
> + * @hash: the hash algorithm to use
> + * @key: the user password / key
> + * @nkey: the length of @key in bytes
> + * @salt: a random salt
> + * @nsalt: length of @salt in bytes
> + * @errp: pointer to a NULL-initialized error object
> + *
> + * Time the PBKDF2 algorithm to determine how many
> + * iterations are required to derive an encryption
> + * key from a user password provided in @key in 1
> + * second of compute time. The result of this can
> + * be used as a the @iterations parameter of a later
> + * call to qcrypto_pbkdf2().

As machines get faster, will 2^31 still be enough, or do we want a
64-bit iterations counter?


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



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PULL 38/49] acpi: take oem_id in build_header(), optionally

[Laszlo Ersek]

Thank you for picking up my patches, and for rebasing them to the SSDT
removal (Igor's "[PULL 02/49] pc: acpi: merge SSDT into DSDT").

Laszlo

On 02/04/16 22:53, Michael S. Tsirkin wrote:
> From: Laszlo Ersek 
> 
> This patch is the continuation of commit 8870ca0e94f2 ("acpi: support
> specified oem table id for build_header"). It will allow us to control the
> OEM ID field too in the SDT header.
> 
> Cc: "Michael S. Tsirkin"  (supporter:ACPI/SMBIOS)
> Cc: Igor Mammedov  (supporter:ACPI/SMBIOS)
> Cc: Xiao Guangrong  (maintainer:NVDIMM)
> Cc: Shannon Zhao  (maintainer:ARM ACPI Subsystem)
> Cc: Paolo Bonzini  (maintainer:X86)
> Cc: Richard W.M. Jones 
> Cc: Aleksei Kovura 
> Cc: Michael Tokarev 
> Cc: Steven Newbury 
> RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1248758
> LP: https://bugs.launchpad.net/qemu/+bug/1533848
> Signed-off-by: Laszlo Ersek 
> Reviewed-by: Michael S. Tsirkin 
> Signed-off-by: Michael S. Tsirkin 
> Reviewed-by: Shannon Zhao 
> ---
>  include/hw/acpi/aml-build.h |  2 +-
>  hw/acpi/aml-build.c | 11 ---
>  hw/acpi/nvdimm.c|  4 ++--
>  hw/arm/virt-acpi-build.c| 12 ++--
>  hw/i386/acpi-build.c| 18 +-
>  5 files changed, 26 insertions(+), 21 deletions(-)
> 
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index 6d6f705..c460bdd 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -357,7 +357,7 @@ Aml *aml_sizeof(Aml *arg);
>  void
>  build_header(GArray *linker, GArray *table_data,
>   AcpiTableHeader *h, const char *sig, int len, uint8_t rev,
> - const char *oem_table_id);
> + const char *oem_id, const char *oem_table_id);
>  void *acpi_data_push(GArray *table_data, unsigned size);
>  unsigned acpi_data_len(GArray *table);
>  void acpi_add_table(GArray *table_offsets, GArray *table_data);
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 21d2ea0..e4b6c0c 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1426,12 +1426,17 @@ Aml *aml_alias(const char *source_object, const char 
> *alias_object)
>  void
>  build_header(GArray *linker, GArray *table_data,
>   AcpiTableHeader *h, const char *sig, int len, uint8_t rev,
> - const char *oem_table_id)
> + const char *oem_id, const char *oem_table_id)
>  {
>  memcpy(>signature, sig, 4);
>  h->length = cpu_to_le32(len);
>  h->revision = rev;
> -memcpy(h->oem_id, ACPI_BUILD_APPNAME6, 6);
> +
> +if (oem_id) {
> +strncpy((char *)h->oem_id, oem_id, sizeof h->oem_id);
> +} else {
> +memcpy(h->oem_id, ACPI_BUILD_APPNAME6, 6);
> +}
>  
>  if (oem_table_id) {
>  strncpy((char *)h->oem_table_id, oem_table_id, 
> sizeof(h->oem_table_id));
> @@ -1506,5 +1511,5 @@ build_rsdt(GArray *table_data, GArray *linker, GArray 
> *table_offsets)
> sizeof(uint32_t));
>  }
>  build_header(linker, table_data,
> - (void *)rsdt, "RSDT", rsdt_len, 1, NULL);
> + (void *)rsdt, "RSDT", rsdt_len, 1, NULL, NULL);
>  }
> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> index 7ee7e16..49ee68e 100644
> --- a/hw/acpi/nvdimm.c
> +++ b/hw/acpi/nvdimm.c
> @@ -366,7 +366,7 @@ static void nvdimm_build_nfit(GSList *device_list, GArray 
> *table_offsets,
>  
>  build_header(linker, table_data,
>   (void *)(table_data->data + header), "NFIT",
> - sizeof(NvdimmNfitHeader) + structures->len, 1, NULL);
> + sizeof(NvdimmNfitHeader) + structures->len, 1, NULL, NULL);
>  g_array_free(structures, true);
>  }
>  
> @@ -471,7 +471,7 @@ static void nvdimm_build_ssdt(GSList *device_list, GArray 
> *table_offsets,
>  g_array_append_vals(table_data, ssdt->buf->data, ssdt->buf->len);
>  build_header(linker, table_data,
>  (void *)(table_data->data + table_data->len - ssdt->buf->len),
> -"SSDT", ssdt->buf->len, 1, "NVDIMM");
> +"SSDT", ssdt->buf->len, 1, NULL, "NVDIMM");
>  free_aml_allocator();
>  }
>  
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 87fbe7c..8017b12 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -408,7 +408,7 @@ build_spcr(GArray *table_data, GArray *linker, 
> VirtGuestInfo *guest_info)
>  spcr->pci_vendor_id = 0x;  /* PCI Vendor ID: not a PCI device */
>  
>  build_header(linker, table_data, (void *)spcr, "SPCR", sizeof(*spcr), 2,
> - NULL);
> + NULL, NULL);
>  }
>  
>  static void
> @@ -427,7 +427,7 @@ build_mcfg(GArray *table_data, GArray 

Re: [Qemu-devel] [PATCH v5 5/9] qemu-log: support simple pid substitution in logfile

[Richard Henderson]

On 02/05/2016 01:56 AM, Alex Bennée wrote:

+if (g_strrstr(filename, "%d")) {
+/* if we are going to format this we'd better validate first */
+if (g_regex_match_simple("^[^%]+%d[^%]+$", filename, 0, 0)) {


Why g_strrstr instead of strstr?  There should be only one, so why look for the 
last?



r~

Re: [Qemu-devel] [PATCH] linux-user: set ppc64/ppc64le default CPU to POWER8

[Michael Tokarev]

04.02.2016 21:56, Laurent Vivier wrote:
> Set the default to the latest CPU version to have the
> largest set of available features.
> 
> It is also really needed in little-endian mode because
> POWER7 is not really supported in this mode and some distros
> (at least debian) generate POWER8 code for their ppc64le target.
> 
> Fixes: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=813698

Reviewed-by: Michael Tokarev 

Thank you!

/mjt

> ---
>  linux-user/main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/linux-user/main.c b/linux-user/main.c
> index e719a2d..2a692e0 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -4160,7 +4160,7 @@ int main(int argc, char **argv, char **envp)
>  cpu_model = "or1200";
>  #elif defined(TARGET_PPC)
>  # ifdef TARGET_PPC64
> -cpu_model = "POWER7";
> +cpu_model = "POWER8";
>  # else
>  cpu_model = "750";
>  # endif
> 

[Qemu-devel] [PULL 18/49] ipmi: replace goto by a return statement

[Michael S. Tsirkin]

From: Cédric Le Goater 

Each routine using the IPMI_ADD_RSP_DATA, IPMI_CHECK_CMD_LEN or
IPMI_CHECK_RESERVATION macros needs to define a goto label 'out' to
handle hidden errors. Using directly a return statement has the same
effect and it removes the fact that 'out' needs to be defined.

The code exits in ipmi_sim_handle_command() are a little different
from the rest and a "possible" error in the macro IPMI_ADD_RSP_DATA is
handled before making use of it. This might be a bit excessive as a
minimum response len is currently 300 bytes and the patch checks that
at least 3 are available.

Signed-off-by: Cédric Le Goater 
Reviewed-by: Marcel Apfelbaum 
Reviewed-by: Greg Kurz 
Reviewed-by: Corey Minyard 
Acked-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/ipmi/ipmi_bmc_sim.c | 140 +++--
 1 file changed, 41 insertions(+), 99 deletions(-)

diff --git a/hw/ipmi/ipmi_bmc_sim.c b/hw/ipmi/ipmi_bmc_sim.c
index dcdab03..1f06dfc 100644
--- a/hw/ipmi/ipmi_bmc_sim.c
+++ b/hw/ipmi/ipmi_bmc_sim.c
@@ -256,7 +256,7 @@ struct IPMIBmcSim {
 do {   \
 if (*rsp_len >= max_rsp_len) { \
 rsp[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;   \
-goto out;  \
+return;\
 }  \
 rsp[(*rsp_len)++] = (b);   \
 } while (0)
@@ -265,7 +265,7 @@ struct IPMIBmcSim {
 #define IPMI_CHECK_CMD_LEN(l) \
 if (cmd_len < l) { \
 rsp[2] = IPMI_CC_REQUEST_DATA_LENGTH_INVALID;  \
-goto out; \
+return; \
 }
 
 /* Check that the reservation in the command is valid. */
@@ -273,7 +273,7 @@ struct IPMIBmcSim {
 do {   \
 if ((cmd[off] | (cmd[off + 1] << 8)) != r) {   \
 rsp[2] = IPMI_CC_INVALID_RESERVATION;  \
-goto out;  \
+return;\
 }  \
 } while (0)
 
@@ -451,14 +451,12 @@ static void gen_event(IPMIBmcSim *ibs, unsigned int 
sens_num, uint8_t deassert,
 }
 
 if (ibs->msg_flags & IPMI_BMC_MSG_FLAG_EVT_BUF_FULL) {
-goto out;
+return;
 }
 
 memcpy(ibs->evtbuf, evt, 16);
 ibs->msg_flags |= IPMI_BMC_MSG_FLAG_EVT_BUF_FULL;
 k->set_atn(s, 1, attn_irq_enabled(ibs));
- out:
-return;
 }
 
 static void sensor_set_discrete_bit(IPMIBmcSim *ibs, unsigned int sensor,
@@ -579,6 +577,11 @@ static void ipmi_sim_handle_command(IPMIBmc *b,
 
 /* Set up the response, set the low bit of NETFN. */
 /* Note that max_rsp_len must be at least 3 */
+if (max_rsp_len < 3) {
+rsp[2] = IPMI_CC_REQUEST_DATA_TRUNCATED;
+goto out;
+}
+
 IPMI_ADD_RSP_DATA(cmd[0] | 0x04);
 IPMI_ADD_RSP_DATA(cmd[1]);
 IPMI_ADD_RSP_DATA(0); /* Assume success */
@@ -696,8 +699,6 @@ static void chassis_capabilities(IPMIBmcSim *ibs,
 IPMI_ADD_RSP_DATA(ibs->parent.slave_addr);
 IPMI_ADD_RSP_DATA(ibs->parent.slave_addr);
 IPMI_ADD_RSP_DATA(ibs->parent.slave_addr);
- out:
-return;
 }
 
 static void chassis_status(IPMIBmcSim *ibs,
@@ -709,8 +710,6 @@ static void chassis_status(IPMIBmcSim *ibs,
 IPMI_ADD_RSP_DATA(0);
 IPMI_ADD_RSP_DATA(0);
 IPMI_ADD_RSP_DATA(0);
- out:
-return;
 }
 
 static void chassis_control(IPMIBmcSim *ibs,
@@ -744,10 +743,8 @@ static void chassis_control(IPMIBmcSim *ibs,
 break;
 default:
 rsp[2] = IPMI_CC_INVALID_DATA_FIELD;
-goto out;
+return;
 }
- out:
-return;
 }
 
 static void get_device_id(IPMIBmcSim *ibs,
@@ -766,8 +763,6 @@ static void get_device_id(IPMIBmcSim *ibs,
 IPMI_ADD_RSP_DATA(ibs->mfg_id[2]);
 IPMI_ADD_RSP_DATA(ibs->product_id[0]);
 IPMI_ADD_RSP_DATA(ibs->product_id[1]);
- out:
-return;
 }
 
 static void set_global_enables(IPMIBmcSim *ibs, uint8_t val)
@@ -820,8 +815,6 @@ static void set_bmc_global_enables(IPMIBmcSim *ibs,
 {
 IPMI_CHECK_CMD_LEN(3);
 set_global_enables(ibs, cmd[2]);
- out:
-return;
 }
 
 static void get_bmc_global_enables(IPMIBmcSim *ibs,
@@ -830,8 +823,6 @@ static void get_bmc_global_enables(IPMIBmcSim *ibs,
unsigned int max_rsp_len)
 {
 IPMI_ADD_RSP_DATA(ibs->bmc_global_enables);
- out:
-return;
 }
 
 static void clr_msg_flags(IPMIBmcSim *ibs,
@@ -845,8 +836,6 @@ static void clr_msg_flags(IPMIBmcSim *ibs,
 IPMI_CHECK_CMD_LEN(3);
 ibs->msg_flags &= ~cmd[2];
 k->set_atn(s, attn_set(ibs), attn_irq_enabled(ibs));
- out:
-return;
 }
 
 static 

[Qemu-devel] [PULL 11/49] virtio: combine the read of a descriptor

[Michael S. Tsirkin]

From: Paolo Bonzini 

Compared to vring, virtio has a performance penalty of 10%.  Fix it
by combining all the reads for a descriptor in a single address_space_read
call.  This also simplifies the code nicely.

Reviewed-by: Cornelia Huck 
Signed-off-by: Paolo Bonzini 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/virtio/virtio.c | 86 ++
 1 file changed, 35 insertions(+), 51 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index c3e00a8..225ee6d 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -107,35 +107,15 @@ void virtio_queue_update_rings(VirtIODevice *vdev, int n)
   vring->align);
 }
 
-static inline uint64_t vring_desc_addr(VirtIODevice *vdev, hwaddr desc_pa,
-   int i)
+static void vring_desc_read(VirtIODevice *vdev, VRingDesc *desc,
+hwaddr desc_pa, int i)
 {
-hwaddr pa;
-pa = desc_pa + sizeof(VRingDesc) * i + offsetof(VRingDesc, addr);
-return virtio_ldq_phys(vdev, pa);
-}
-
-static inline uint32_t vring_desc_len(VirtIODevice *vdev, hwaddr desc_pa, int 
i)
-{
-hwaddr pa;
-pa = desc_pa + sizeof(VRingDesc) * i + offsetof(VRingDesc, len);
-return virtio_ldl_phys(vdev, pa);
-}
-
-static inline uint16_t vring_desc_flags(VirtIODevice *vdev, hwaddr desc_pa,
-int i)
-{
-hwaddr pa;
-pa = desc_pa + sizeof(VRingDesc) * i + offsetof(VRingDesc, flags);
-return virtio_lduw_phys(vdev, pa);
-}
-
-static inline uint16_t vring_desc_next(VirtIODevice *vdev, hwaddr desc_pa,
-   int i)
-{
-hwaddr pa;
-pa = desc_pa + sizeof(VRingDesc) * i + offsetof(VRingDesc, next);
-return virtio_lduw_phys(vdev, pa);
+address_space_read(_space_memory, desc_pa + i * sizeof(VRingDesc),
+   MEMTXATTRS_UNSPECIFIED, (void *)desc, 
sizeof(VRingDesc));
+virtio_tswap64s(vdev, >addr);
+virtio_tswap32s(vdev, >len);
+virtio_tswap16s(vdev, >flags);
+virtio_tswap16s(vdev, >next);
 }
 
 static inline uint16_t vring_avail_flags(VirtQueue *vq)
@@ -345,18 +325,18 @@ static unsigned int virtqueue_get_head(VirtQueue *vq, 
unsigned int idx)
 return head;
 }
 
-static unsigned virtqueue_next_desc(VirtIODevice *vdev, hwaddr desc_pa,
-unsigned int i, unsigned int max)
+static unsigned virtqueue_read_next_desc(VirtIODevice *vdev, VRingDesc *desc,
+ hwaddr desc_pa, unsigned int max)
 {
 unsigned int next;
 
 /* If this descriptor says it doesn't chain, we're done. */
-if (!(vring_desc_flags(vdev, desc_pa, i) & VRING_DESC_F_NEXT)) {
+if (!(desc->flags & VRING_DESC_F_NEXT)) {
 return max;
 }
 
 /* Check they're not leading us off end of descriptors. */
-next = vring_desc_next(vdev, desc_pa, i);
+next = desc->next;
 /* Make sure compiler knows to grab that: we don't want it changing! */
 smp_wmb();
 
@@ -365,6 +345,7 @@ static unsigned virtqueue_next_desc(VirtIODevice *vdev, 
hwaddr desc_pa,
 exit(1);
 }
 
+vring_desc_read(vdev, desc, desc_pa, next);
 return next;
 }
 
@@ -381,6 +362,7 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int 
*in_bytes,
 while (virtqueue_num_heads(vq, idx)) {
 VirtIODevice *vdev = vq->vdev;
 unsigned int max, num_bufs, indirect = 0;
+VRingDesc desc;
 hwaddr desc_pa;
 int i;
 
@@ -388,9 +370,10 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int 
*in_bytes,
 num_bufs = total_bufs;
 i = virtqueue_get_head(vq, idx++);
 desc_pa = vq->vring.desc;
+vring_desc_read(vdev, , desc_pa, i);
 
-if (vring_desc_flags(vdev, desc_pa, i) & VRING_DESC_F_INDIRECT) {
-if (vring_desc_len(vdev, desc_pa, i) % sizeof(VRingDesc)) {
+if (desc.flags & VRING_DESC_F_INDIRECT) {
+if (desc.len % sizeof(VRingDesc)) {
 error_report("Invalid size for indirect buffer table");
 exit(1);
 }
@@ -403,9 +386,10 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned int 
*in_bytes,
 
 /* loop over the indirect descriptor table */
 indirect = 1;
-max = vring_desc_len(vdev, desc_pa, i) / sizeof(VRingDesc);
-desc_pa = vring_desc_addr(vdev, desc_pa, i);
+max = desc.len / sizeof(VRingDesc);
+desc_pa = desc.addr;
 num_bufs = i = 0;
+vring_desc_read(vdev, , desc_pa, i);
 }
 
 do {
@@ -415,15 +399,15 @@ void virtqueue_get_avail_bytes(VirtQueue *vq, unsigned 
int *in_bytes,
 exit(1);
 }
 
-if (vring_desc_flags(vdev, desc_pa, i) & 

[Qemu-devel] [PULL 09/49] virtio: slim down allocation of VirtQueueElements

[Michael S. Tsirkin]

From: Paolo Bonzini 

Build the addresses and s/g lists on the stack, and then copy them
to a VirtQueueElement that is just as big as required to contain this
particular s/g list.  The cost of the copy is minimal compared to that
of a large malloc.

When virtqueue_map is used on the destination side of migration or on
loadvm, the iovecs have already been split at memory region boundary,
so we can just reuse the out_num/in_num we find in the file.

Reviewed-by: Cornelia Huck 
Signed-off-by: Paolo Bonzini 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Michael S. Tsirkin 
---
 hw/virtio/virtio.c | 82 +-
 1 file changed, 51 insertions(+), 31 deletions(-)

diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index 661a1e1..c3e00a8 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -448,6 +448,32 @@ int virtqueue_avail_bytes(VirtQueue *vq, unsigned int 
in_bytes,
 return in_bytes <= in_total && out_bytes <= out_total;
 }
 
+static void virtqueue_map_desc(unsigned int *p_num_sg, hwaddr *addr, struct 
iovec *iov,
+   unsigned int max_num_sg, bool is_write,
+   hwaddr pa, size_t sz)
+{
+unsigned num_sg = *p_num_sg;
+assert(num_sg <= max_num_sg);
+
+while (sz) {
+hwaddr len = sz;
+
+if (num_sg == max_num_sg) {
+error_report("virtio: too many write descriptors in indirect 
table");
+exit(1);
+}
+
+iov[num_sg].iov_base = cpu_physical_memory_map(pa, , is_write);
+iov[num_sg].iov_len = len;
+addr[num_sg] = pa;
+
+sz -= len;
+pa += len;
+num_sg++;
+}
+*p_num_sg = num_sg;
+}
+
 static void virtqueue_map_iovec(struct iovec *sg, hwaddr *addr,
 unsigned int *num_sg, unsigned int max_size,
 int is_write)
@@ -474,20 +500,10 @@ static void virtqueue_map_iovec(struct iovec *sg, hwaddr 
*addr,
 error_report("virtio: error trying to map MMIO memory");
 exit(1);
 }
-if (len == sg[i].iov_len) {
-continue;
-}
-if (*num_sg >= max_size) {
-error_report("virtio: memory split makes iovec too large");
+if (len != sg[i].iov_len) {
+error_report("virtio: unexpected memory split");
 exit(1);
 }
-memmove(sg + i + 1, sg + i, sizeof(*sg) * (*num_sg - i));
-memmove(addr + i + 1, addr + i, sizeof(*addr) * (*num_sg - i));
-assert(len < sg[i + 1].iov_len);
-sg[i].iov_len = len;
-addr[i + 1] += len;
-sg[i + 1].iov_len -= len;
-++*num_sg;
 }
 }
 
@@ -526,14 +542,16 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz)
 hwaddr desc_pa = vq->vring.desc;
 VirtIODevice *vdev = vq->vdev;
 VirtQueueElement *elem;
+unsigned out_num, in_num;
+hwaddr addr[VIRTQUEUE_MAX_SIZE];
+struct iovec iov[VIRTQUEUE_MAX_SIZE];
 
 if (!virtqueue_num_heads(vq, vq->last_avail_idx)) {
 return NULL;
 }
 
 /* When we start there are none of either input nor output. */
-elem = virtqueue_alloc_element(sz, VIRTQUEUE_MAX_SIZE, VIRTQUEUE_MAX_SIZE);
-elem->out_num = elem->in_num = 0;
+out_num = in_num = 0;
 
 max = vq->vring.num;
 
@@ -556,37 +574,39 @@ void *virtqueue_pop(VirtQueue *vq, size_t sz)
 
 /* Collect all the descriptors */
 do {
-struct iovec *sg;
+hwaddr pa = vring_desc_addr(vdev, desc_pa, i);
+size_t len = vring_desc_len(vdev, desc_pa, i);
 
 if (vring_desc_flags(vdev, desc_pa, i) & VRING_DESC_F_WRITE) {
-if (elem->in_num >= VIRTQUEUE_MAX_SIZE) {
-error_report("Too many write descriptors in indirect table");
-exit(1);
-}
-elem->in_addr[elem->in_num] = vring_desc_addr(vdev, desc_pa, i);
-sg = >in_sg[elem->in_num++];
+virtqueue_map_desc(_num, addr + out_num, iov + out_num,
+   VIRTQUEUE_MAX_SIZE - out_num, true, pa, len);
 } else {
-if (elem->out_num >= VIRTQUEUE_MAX_SIZE) {
-error_report("Too many read descriptors in indirect table");
+if (in_num) {
+error_report("Incorrect order for descriptors");
 exit(1);
 }
-elem->out_addr[elem->out_num] = vring_desc_addr(vdev, desc_pa, i);
-sg = >out_sg[elem->out_num++];
+virtqueue_map_desc(_num, addr, iov,
+   VIRTQUEUE_MAX_SIZE, false, pa, len);
 }
 
-sg->iov_len = vring_desc_len(vdev, desc_pa, i);
-
 /* If we've got too many, that implies a descriptor loop. */
-if ((elem->in_num + elem->out_num) > max) {
+if ((in_num + out_num) > max) {