Re: [Qemu-devel] [PATCH] target/ppc: fix mtmsr instruction for icount

2018-11-06 Thread Pavel Dovgalyuk
> From: Richard Henderson [mailto:richard.hender...@linaro.org]
> On 10/30/18 9:30 AM, Pavel Dovgalyuk wrote:
> > This patch fixes processing of mtmsr instructions in icount mode.
> > In this mode writing to interrupt/peripheral state is controlled
> > by can_do_io flag. This flag must be set explicitly before helper
> > function invocation.
> >
> > Signed-off-by: Maria Klimushenkova 
> > Signed-off-by: Pavel Dovgalyuk 
> > ---
> >  target/ppc/translate.c |   12 
> >  1 file changed, 12 insertions(+)
> 
> Reviewed-by: Richard Henderson 

Richard, can you check the another similar patch?

https://patchew.org/QEMU/20181030122134.11055.15711.stgit@pasha-VirtualBox/


Pavel Dovgalyuk




Re: [Qemu-devel] [PATCH for-3.1] replay: Exit on errors reading from replay log

2018-11-06 Thread Pavel Dovgalyuk
> From: Peter Maydell [mailto:peter.mayd...@linaro.org]
> Currently replay_get_byte() does not check for an error
> from getc(). Coverity points out (CID 1390622) that this
> could result in unexpected behaviour (such as looping
> forever, if we use the replay_get_dword() return value
> for a loop count). We don't expect reads from the replay
> log to fail, and if they do there is no way we can
> continue. So make them fatal errors.
> 
> Signed-off-by: Peter Maydell 

Reviewed-by: Pavel Dovgalyuk 

> ---
> Disclaimer: checked only with "make check".
> 
>  replay/replay-internal.c | 16 +---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/replay/replay-internal.c b/replay/replay-internal.c
> index 1cea1d4dc91..8f87e9b957e 100644
> --- a/replay/replay-internal.c
> +++ b/replay/replay-internal.c
> @@ -35,6 +35,12 @@ static void replay_write_error(void)
>  }
>  }
> 
> +static void replay_read_error(void)
> +{
> +error_report("error reading the replay data");
> +exit(1);
> +}
> +
>  void replay_put_byte(uint8_t byte)
>  {
>  if (replay_file) {
> @@ -83,7 +89,11 @@ uint8_t replay_get_byte(void)
>  {
>  uint8_t byte = 0;
>  if (replay_file) {
> -byte = getc(replay_file);
> +int r = getc(replay_file);
> +if (r == EOF) {
> +replay_read_error();
> +}
> +byte = r;
>  }
>  return byte;
>  }
> @@ -126,7 +136,7 @@ void replay_get_array(uint8_t *buf, size_t *size)
>  if (replay_file) {
>  *size = replay_get_dword();
>  if (fread(buf, 1, *size, replay_file) != *size) {
> -error_report("replay read error");
> +replay_read_error();
>  }
>  }
>  }
> @@ -137,7 +147,7 @@ void replay_get_array_alloc(uint8_t **buf, size_t *size)
>  *size = replay_get_dword();
>  *buf = g_malloc(*size);
>  if (fread(*buf, 1, *size, replay_file) != *size) {
> -error_report("replay read error");
> +replay_read_error();
>  }
>  }
>  }
> --
> 2.19.1

Pavel Dovgalyuk




[Qemu-devel] [PATCH] ui/gtk: fix cursor in egl mode

2018-11-06 Thread Gerd Hoffmann
In egl mode the scale_x and scale_y variables are not set, so the
scaling logic in the mouse motion event handler does not work.

Fix that.  Also scale the cursor position in gd_egl_cursor_position().

Reported-by: Chen Zhang 
Signed-off-by: Gerd Hoffmann 
Tested-by: Chen Zhang 
---
 ui/gtk-egl.c | 17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index a77c25b490..5420c2362b 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -68,8 +68,15 @@ void gd_egl_draw(VirtualConsole *vc)
 return;
 }
 
+window = gtk_widget_get_window(vc->gfx.drawing_area);
+ww = gdk_window_get_width(window);
+wh = gdk_window_get_height(window);
+
 if (vc->gfx.scanout_mode) {
 gd_egl_scanout_flush(&vc->gfx.dcl, 0, 0, vc->gfx.w, vc->gfx.h);
+
+vc->gfx.scale_x = (double)ww / vc->gfx.w;
+vc->gfx.scale_y = (double)wh / vc->gfx.h;
 } else {
 if (!vc->gfx.ds) {
 return;
@@ -77,13 +84,13 @@ void gd_egl_draw(VirtualConsole *vc)
 eglMakeCurrent(qemu_egl_display, vc->gfx.esurface,
vc->gfx.esurface, vc->gfx.ectx);
 
-window = gtk_widget_get_window(vc->gfx.drawing_area);
-ww = gdk_window_get_width(window);
-wh = gdk_window_get_height(window);
 surface_gl_setup_viewport(vc->gfx.gls, vc->gfx.ds, ww, wh);
 surface_gl_render_texture(vc->gfx.gls, vc->gfx.ds);
 
 eglSwapBuffers(qemu_egl_display, vc->gfx.esurface);
+
+vc->gfx.scale_x = (double)ww / surface_width(vc->gfx.ds);
+vc->gfx.scale_y = (double)wh / surface_height(vc->gfx.ds);
 }
 }
 
@@ -232,8 +239,8 @@ void gd_egl_cursor_position(DisplayChangeListener *dcl,
 {
 VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);
 
-vc->gfx.cursor_x = pos_x;
-vc->gfx.cursor_y = pos_y;
+vc->gfx.cursor_x = pos_x * vc->gfx.scale_x;
+vc->gfx.cursor_y = pos_y * vc->gfx.scale_y;
 }
 
 void gd_egl_release_dmabuf(DisplayChangeListener *dcl,
-- 
2.9.3




[Qemu-devel] [PATCH] netdev: fix socket backend implementation and docs

2018-11-06 Thread Artem Pisarenko
Changes:
- user documentation and QAPI 'NetdevSocketOptions' comments updated
to match current implementation ('udp' type description added, 'fd'
option separated to exclusive type and described, 'localaddr'-related
description for 'mcast' type fixed, hostname parts in "[host]:port"
options updated to match optional/required syntax, various fixes and
improvements in options breakdown and wording);
- 'fd' type backend: requirement for socket handle being already
binded and connected made explicit and documented;
- 'fd' type backend: fix broken SOCK_DGRAM support;
- 'fd' type backend: removed multicast support and cleaned up broken
workaround for it (never called);
- fix possible broken multicasting in win32 platform;
- fix parsing of "[host]:port" options (added error handling for cases
where "host" part is documented as required but isn't provided);
- some error messages improved;
- other small fixes and refactoring in code.

Signed-off-by: Artem Pisarenko 
---

Notes:
(Since these changes are closely related, I've combined them in one patch.)
This patch addresses all issues I've pointed earlier 
(http://lists.nongnu.org/archive/html/qemu-devel/2018-05/msg06695.html), except 
internal protocol, and plus additional ones.

'fd' transport and its special 'af_inet multicast' case deserve special 
attention.
Firstly, it already wasn't working as expected at least in current qemu 
version, so it isn't supposed to break any compabitibility if someone cares.
The only question is a way of fixing it.
It depends on concept behind 'fd' transport, which is unknown to me. Seems 
like initially it was just optional parameter to any transport allowing to 
replace newly created socket descriptor with user supplied one, but at some 
time someone changed it to be separate transport and not accounted 'af_inet 
multicast' case (code analysis shows that condition "is_connected && mcast != 
NULL" in 'net_socket_fd_init_dgram' function never can be true and 
"s->dgram_dst" value is left unassigned).
I would prefer concept of separate transport when qemu doesn't depend on 
underlying domain, type and protocol of socket (almost). To make it 
universal/flexible, qemu shouldn't handle their specifics, such as 'af_inet 
multicast' one. So I fixed things accordingly. For example, if user needs 
multicasting, it should already know that it cannot share one socket between 
its app and qemu instances, so user should use 'mcast' transport of qemu which 
will create separate socket for qemu instance. And there are maybe other socket 
types (possibly not existing at present moment yet) with their own specifics.
Since this concept restricts usage of sockets which cannot work in 
connected state (such as af_inet multicast), I've prepared and ready to submit 
another version of patch which solves this restriction by checking socket type 
and handling it accordingly. Currently it supports only 'af_inet multicast' 
case by preserving existed hack (slightly modified): it extracts multicast 
address from socket and duplicates socket in proper unconnected state. It also 
requires synchronization with user who will unconnect original socket back. All 
of this is very hacky, but I'm open for discussion...

 include/qemu-common.h  |   3 +
 include/qemu/sockets.h |   2 +-
 net/net.c  |   8 ++-
 net/socket.c   | 148 ++---
 qapi/net.json  |  12 ++--
 qemu-options.hx|  85 +---
 6 files changed, 147 insertions(+), 111 deletions(-)

diff --git a/include/qemu-common.h b/include/qemu-common.h
index ed60ba2..d4e4121 100644
--- a/include/qemu-common.h
+++ b/include/qemu-common.h
@@ -67,6 +67,8 @@ int qemu_openpty_raw(int *aslave, char *pty_name);
 #define qemu_setsockopt(sockfd, level, optname, optval, optlen) \
 setsockopt(sockfd, level, optname, (const void *)optval, optlen)
 #define qemu_recv(sockfd, buf, len, flags) recv(sockfd, (void *)buf, len, 
flags)
+#define qemu_send(sockfd, buf, len, flags) \
+send(sockfd, (const void *)buf, len, flags)
 #define qemu_sendto(sockfd, buf, len, flags, destaddr, addrlen) \
 sendto(sockfd, (const void *)buf, len, flags, destaddr, addrlen)
 #else
@@ -75,6 +77,7 @@ int qemu_openpty_raw(int *aslave, char *pty_name);
 #define qemu_setsockopt(sockfd, level, optname, optval, optlen) \
 setsockopt(sockfd, level, optname, optval, optlen)
 #define qemu_recv(sockfd, buf, len, flags) recv(sockfd, buf, len, flags)
+#define qemu_send(sockfd, buf, len, flags) send(sockfd, buf, len, flags)
 #define qemu_sendto(sockfd, buf, len, flags, destaddr, addrlen) \
 sendto(sockfd, buf, len, flags, destaddr, addrlen)
 #endif
diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
index 8140fea..3fad004 100644
--- a/include/qemu/sockets.h
+++ b/include/qemu/sockets.h
@@ -46,7 +46,7 @@ void socket_listen_cleanup(int fd, Error **errp);
 int socket_dgram(SocketAddress *remote, SocketAddress

Re: [Qemu-devel] [PATCH] block: Make more block drivers compile-time configurable

2018-11-06 Thread Markus Armbruster
Max Reitz  writes:

> On 05.11.18 16:25, Markus Armbruster wrote:
>> Max Reitz  writes:
>> 
>>> On 19.10.18 13:34, Markus Armbruster wrote:
 From: Jeff Cody 

 This adds configure options to control the following block drivers:

 * Bochs
 * Cloop
 * Dmg
 * Qcow (V1)
 * Vdi
 * Vvfat
 * qed
 * parallels
 * sheepdog

 Each of these defaults to being enabled.

 Signed-off-by: Jeff Cody 
 Signed-off-by: Markus Armbruster 
 ---

 Hmm, we got quite a few --enable-BLOCK-DRIVER now.  Perhaps a single
 list-valued option similar --target-list would be better.  Could be
 done on top.

  block/Makefile.objs | 22 ---
  configure   | 91 +
  2 files changed, 107 insertions(+), 6 deletions(-)

 diff --git a/block/Makefile.objs b/block/Makefile.objs
 index c8337bf186..1cad9fc4f1 100644
 --- a/block/Makefile.objs
 +++ b/block/Makefile.objs
>>@@ -1,10 +1,18 @@
>>-block-obj-y += raw-format.o qcow.o vdi.o vmdk.o cloop.o bochs.o vpc.o 
>> vvfat.o dmg.o
>>+block-obj-y += raw-format.o vmdk.o vpc.o
>>+block-obj-$(CONFIG_QCOW1) += qcow.o
>>+block-obj-$(CONFIG_VDI) += vdi.o
>>+block-obj-$(CONFIG_CLOOP) += cloop.o
>>+block-obj-$(CONFIG_BOCHS) += bochs.o
>>+block-obj-$(CONFIG_VVFAT) += vvfat.o
>>+block-obj-$(CONFIG_DMG) += dmg.o
>>+
>> block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o 
>> qcow2-cache.o qcow2-bitmap.o
>>>
>>> [...]
>>>
 @@ -45,7 +54,8 @@ gluster.o-libs := $(GLUSTERFS_LIBS)
  vxhs.o-libs:= $(VXHS_LIBS)
  ssh.o-cflags   := $(LIBSSH2_CFLAGS)
  ssh.o-libs := $(LIBSSH2_LIBS)
 -block-obj-$(if $(CONFIG_BZIP2),m,n) += dmg-bz2.o
 +block-obj-dmg-bz2$(if $(CONFIG_BZIP2),m,n) += dmg-bz2.o
 +block-obj-$(CONFIG_DMG) += $(block-obj-dmg-bz2-y)
>>>
>>> This defines "block-obj-dmg-bz2m" or "block-obj-dmg-bz2n", so
>>> "block-obj-dmg-bz2-y" is never defined (note both the missing hyphen and
>>> the "m" vs. "y").
>>>
>>> How about:
>>>
>>> block-obj-dmg-bz2-$(CONFIG_BZIP2) += dmg-bz2.o
>> 
>> As far as I can tell, CONFIG_BZIP2 is either undefined or "y".  Thus,
>> block-obj-dmg-bz2-y is either left undefined or set to dmg-bz2.o.
>
> Yes.
>
>> Perhaps the '+=' be ':=', but we seem to use '+=' pretty
>> indiscriminately.
>
> Yep.  I don't know.  Whatever works, and both do, so...
>
>>> block-obj-$(if $(CONFIG_DMG),m,n) += $(block-obj-dmg-bz2-y)
>> 
>> As far as I can tell, CONFIG_DMG is also either undefined or "y".  So,
>> this adds dmg-bz2.o to block-obj-m if both CONFIG_BZIP2 and CONFIG_DMG
>> are enabled.
>
> Yes.
>
>> Shouldn't it be added to block-obj-y, like dmg.o, or am I confused?
>
> The behavior before this patch was to add it to block-obj-m.
> 27685a8dd08c051fa6d641fe46106fc0dfa51073 has the explanation: We want
> the bz2 part to be a module so you can launch qemu even without libbz2
> around.  Only when you use dmg will it load that module.
>
> (And if you dig deeper, it was 88d88798b7efe that (intentionally) broke
>  that intended behavior, until it was restored by the above commit.)

Thanks, v2 sent with your fix.



[Qemu-devel] [PATCH v2] block: Make more block drivers compile-time configurable

2018-11-06 Thread Markus Armbruster
From: Jeff Cody 

This adds configure options to control the following block drivers:

* Bochs
* Cloop
* Dmg
* Qcow (V1)
* Vdi
* Vvfat
* qed
* parallels
* sheepdog

Each of these defaults to being enabled.

Signed-off-by: Jeff Cody 
Signed-off-by: Markus Armbruster 
---
v2: Fix handling of dmg-bz2.o [Max]

 block/Makefile.objs | 22 ---
 configure   | 91 +
 2 files changed, 107 insertions(+), 6 deletions(-)

diff --git a/block/Makefile.objs b/block/Makefile.objs
index c8337bf186..46d585cfd0 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -1,10 +1,18 @@
-block-obj-y += raw-format.o qcow.o vdi.o vmdk.o cloop.o bochs.o vpc.o vvfat.o 
dmg.o
+block-obj-y += raw-format.o vmdk.o vpc.o
+block-obj-$(CONFIG_QCOW1) += qcow.o
+block-obj-$(CONFIG_VDI) += vdi.o
+block-obj-$(CONFIG_CLOOP) += cloop.o
+block-obj-$(CONFIG_BOCHS) += bochs.o
+block-obj-$(CONFIG_VVFAT) += vvfat.o
+block-obj-$(CONFIG_DMG) += dmg.o
+
 block-obj-y += qcow2.o qcow2-refcount.o qcow2-cluster.o qcow2-snapshot.o 
qcow2-cache.o qcow2-bitmap.o
-block-obj-y += qed.o qed-l2-cache.o qed-table.o qed-cluster.o
-block-obj-y += qed-check.o
+block-obj-$(CONFIG_QED) += qed.o qed-l2-cache.o qed-table.o qed-cluster.o
+block-obj-$(CONFIG_QED) += qed-check.o
 block-obj-y += vhdx.o vhdx-endian.o vhdx-log.o
 block-obj-y += quorum.o
-block-obj-y += parallels.o blkdebug.o blkverify.o blkreplay.o
+block-obj-y += blkdebug.o blkverify.o blkreplay.o
+block-obj-$(CONFIG_PARALLELS) += parallels.o
 block-obj-y += blklogwrites.o
 block-obj-y += block-backend.o snapshot.o qapi.o
 block-obj-$(CONFIG_WIN32) += file-win32.o win32-aio.o
@@ -14,7 +22,8 @@ block-obj-y += null.o mirror.o commit.o io.o create.o
 block-obj-y += throttle-groups.o
 block-obj-$(CONFIG_LINUX) += nvme.o
 
-block-obj-y += nbd.o nbd-client.o sheepdog.o
+block-obj-y += nbd.o nbd-client.o
+block-obj-$(CONFIG_SHEEPDOG) += sheepdog.o
 block-obj-$(CONFIG_LIBISCSI) += iscsi.o
 block-obj-$(if $(CONFIG_LIBISCSI),y,n) += iscsi-opts.o
 block-obj-$(CONFIG_LIBNFS) += nfs.o
@@ -45,7 +54,8 @@ gluster.o-libs := $(GLUSTERFS_LIBS)
 vxhs.o-libs:= $(VXHS_LIBS)
 ssh.o-cflags   := $(LIBSSH2_CFLAGS)
 ssh.o-libs := $(LIBSSH2_LIBS)
-block-obj-$(if $(CONFIG_BZIP2),m,n) += dmg-bz2.o
+block-obj-dmg-bz2-$(CONFIG_BZIP2) += dmg-bz2.o
+block-obj-$(if $(CONFIG_DMG),m,n) += $(block-obj-dmg-bz2-y)
 dmg-bz2.o-libs := $(BZIP2_LIBS)
 qcow.o-libs:= -lz
 linux-aio.o-libs   := -laio
diff --git a/configure b/configure
index 74e313a810..5b1d83ea26 100755
--- a/configure
+++ b/configure
@@ -470,6 +470,15 @@ tcmalloc="no"
 jemalloc="no"
 replication="yes"
 vxhs=""
+bochs="yes"
+cloop="yes"
+dmg="yes"
+qcow1="yes"
+vdi="yes"
+vvfat="yes"
+qed="yes"
+parallels="yes"
+sheepdog="yes"
 libxml2=""
 docker="no"
 debug_mutex="no"
@@ -1416,6 +1425,42 @@ for opt do
   ;;
   --enable-vxhs) vxhs="yes"
   ;;
+  --disable-bochs) bochs="no"
+  ;;
+  --enable-bochs) bochs="yes"
+  ;;
+  --disable-cloop) cloop="no"
+  ;;
+  --enable-cloop) cloop="yes"
+  ;;
+  --disable-dmg) dmg="no"
+  ;;
+  --enable-dmg) dmg="yes"
+  ;;
+  --disable-qcow1) qcow1="no"
+  ;;
+  --enable-qcow1) qcow1="yes"
+  ;;
+  --disable-vdi) vdi="no"
+  ;;
+  --enable-vdi) vdi="yes"
+  ;;
+  --disable-vvfat) vvfat="no"
+  ;;
+  --enable-vvfat) vvfat="yes"
+  ;;
+  --disable-qed) qed="no"
+  ;;
+  --enable-qed) qed="yes"
+  ;;
+  --disable-parallels) parallels="no"
+  ;;
+  --enable-parallels) parallels="yes"
+  ;;
+  --disable-sheepdog) sheepdog="no"
+  ;;
+  --enable-sheepdog) sheepdog="yes"
+  ;;
   --disable-vhost-user) vhost_user="no"
   ;;
   --enable-vhost-user)
@@ -1718,6 +1763,15 @@ disabled with --disable-FEATURE, default is enabled if 
available:
   qom-cast-debug  cast debugging support
   tools   build qemu-io, qemu-nbd and qemu-image tools
   vxhsVeritas HyperScale vDisk backend support
+  bochs   bochs image format support
+  cloop   cloop image format support
+  dmg dmg image format support
+  qcow1   qcow v1 image format support
+  vdi vdi image format support
+  vvfat   vvfat image format support
+  qed qed image format support
+  parallels   parallels image format support
+  sheepdogsheepdog block driver support
   crypto-afalgLinux AF_ALG crypto backend driver
   vhost-user  vhost-user support
   capstonecapstone disassembler support
@@ -6043,6 +6097,15 @@ echo "jemalloc support  $jemalloc"
 echo "avx2 optimization $avx2_opt"
 echo "replication support $replication"
 echo "VxHS block device $vxhs"
+echo "bochs support $bochs"
+echo "cloop support $cloop"
+echo "dmg support   $dmg"
+echo "qcow v1 support   $qcow1"
+echo "vdi support   $vdi"
+echo "vvfat support $vvfat"
+echo "qed support   $qed"
+echo "parallels support $parallels"
+echo "sheepdog support  $sheepdog"
 echo "capstone  $capstone"
 echo "docker$docker"
 e

Re: [Qemu-devel] [Qemu-block] [PATCH] tests: Fix Python 3 detection on older GNU make versions

2018-11-06 Thread Markus Armbruster
Eduardo Habkost  writes:

> The $(SHELLSTATUS) variable requires GNU make >= 4.2, but Travis
> seems to provide an older version.  Change the existing rules to
> use command output instead of exit code, to make it compatible
> with older GNU make versions.
>
> Signed-off-by: Eduardo Habkost 
> ---
> I think that's the cause of the Travis failures.  I have
> submitted a test job right now, at:
>   https://travis-ci.org/ehabkost/qemu-hacks/jobs/451387962
> Let's see if it fixes the issue.
> ---
>  tests/Makefile.include | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index d2e577eabb..074eece558 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -913,8 +913,8 @@ TESTS_RESULTS_DIR=$(BUILD_DIR)/tests/results
>  # information please refer to "avocado --help".
>  AVOCADO_SHOW=none
>  
> -$(shell $(PYTHON) -c 'import sys; assert sys.version_info >= (3,0)' 
> >/dev/null 2>&1)
> -ifeq ($(.SHELLSTATUS),0)
> +PYTHON3 = $(shell $(PYTHON) -c 'import sys; print(1 if sys.version_info >= 
> (3, 0) else 0)')
> +ifeq ($(PYTHON3), 1)
>  $(TESTS_VENV_DIR): $(TESTS_VENV_REQ)
>   $(call quiet-command, \
>  $(PYTHON) -m venv --system-site-packages $@, \

PEP 394 recommends software distributions install Python 3 into the
default path as python3, and users use that instead of python, except
for programs that are source compatible with both 2 and 3.  So, is
finding out whether python is a Python 3 really appropriate?  Why can't
we just use python3 and be done with it?

If we can't: isn't this a configure problem?



Re: [Qemu-devel] [QEMU-PPC] [PATCH V3 0/3] ppc/spapr: Add support for nested kvm-hv

2018-11-06 Thread David Gibson
On Thu, Oct 11, 2018 at 05:16:06PM +1100, Suraj Jitindar Singh wrote:
> This patch series adds the qemu support for running nested kvm-hv on a
> POWER9 platform with appropriate hypervisor support and migration of
> these guests.
> That is, the ability to run kvm-hv guests as guests of an operating system
> which is itself a kvm-hv guest.
> 
> The host (L0 hypervisor) and level 1 guest (L1 guest hypervisor) require
> the following patch series:
> KVM: PPC: Book3S HV: Nested HV virtualization
> 
> And this patch series in the qemu instance running on the L0 hypervisor.
> 
> Patch series based on: ppc-for-3.1
> 
> The cap number is now in Paolo's tree so hopefully we can rely on it not
> changing.

Now that the kernel changes are in upstream master, can you resend a
final version of this?

> 
> V2 -> V3:
> - The enable cap ioctl no longer takes an enable field to indicate
>   disable/enable, but just enables the cap. So update to match this
>   kernel change. (The cap starts out disable by default)
> 
> Suraj Jitindar Singh (3):
>   target/ppc: Update linux-headers for v4.19-rc7
>   target/ppc: Add one reg id for ptcr
>   ppc/spapr_caps: Add SPAPR_CAP_NESTED_KVM_HV
> 
>  hw/ppc/spapr.c  |  2 ++
>  hw/ppc/spapr_caps.c | 32 
>  include/hw/ppc/spapr.h  |  5 -
>  linux-headers/asm-powerpc/kvm.h |  1 +
>  linux-headers/linux/kvm.h   |  1 +
>  target/ppc/kvm.c| 12 
>  target/ppc/kvm_ppc.h| 12 
>  target/ppc/translate_init.inc.c | 10 +-
>  8 files changed, 69 insertions(+), 6 deletions(-)
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH v2 2/2] spapr_pci: rename some structured types

2018-11-06 Thread David Gibson
On Mon, Oct 15, 2018 at 12:49:53PM +1100, Alexey Kardashevskiy wrote:
> 
> 
> On 12/10/2018 20:05, Greg Kurz wrote:
> > According to CODING_STYLE, structured types names are expected to be
> > in CamelCase but we have:
> > 
> > typedef struct spapr_pci_msi {
> > uint32_t first_irq;
> > uint32_t num;
> > } spapr_pci_msi;
> > 
> > typedef struct spapr_pci_msi_mig {
> > uint32_t key;
> > spapr_pci_msi value;
> > } spapr_pci_msi_mig;
> > 
> > Acronyms are often written in upper-case, but here we would en up with
> > a lot of upper-case letters in a row (ie, sPAPRPCIMSI) which defeats the
> > purpose of CamelCase. It even displays "RPC" which looks awkward.
> 
> Yet more common than this. I vote for sPAPRPCIMSI as PCI is an acronym.
> "pci" is small letters hurts my eyes :)

Hrm.  So, yes, I know I kind of started it, but these various
compromises about how to captialize things means this patch is now
"change from non-camelcase to.. something that's also not really
camelcase".

At which point I'm not particularly convinced it's worth the bother.

If we really want to go ahead with CamelCasing this, I think we'd need
to start by fixing up the existing sorta-camelcase-but-not-really
stuff to actual camelcase.  Which would mean the slightly odd reading
"Spapr" and "Pci" and "Msi" and so forth, but it might be worth it for
consistency.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH] MAINTAINERS: PPC: Remove myself

2018-11-06 Thread David Gibson
On Tue, Oct 30, 2018 at 02:34:46PM +, Mark Cave-Ayland wrote:
> On 30/10/2018 09:35, Alexander Graf wrote:
> 
> > I haven't really been maintaining any PowerPC code for quite a while now,
> > so let's reflect reality: David does all the work and embedded PPC is in
> > "Odd Fixes" state rather than supported now.
> 
> It was a pleasure working with you, but of course life moves
> on... :)

[snip]
> >  New World
> > -M: Alexander Graf 
> > +M: David Gibson 
> >  L: qemu-...@nongnu.org
> > -S: Maintained
> > +S: Odd Fixes
> 
> There was actually a patch floating around to add myself to MAINTAINERS for 
> the New
> World and Old World machines, but I think it got lost when trying to set up a
> sub-maintainer queue with David. I'm happy to look after these
> machines for now.

Yeah, it did.  Still need to sort that out...

> 
> >  F: hw/ppc/mac_newworld.c
> >  F: hw/pci-host/uninorth.c
> >  F: hw/pci-bridge/dec.[hc]
> > @@ -823,9 +822,9 @@ F: include/hw/misc/mos6522.h
> >  F: include/hw/ppc/mac_dbdma.h
> >  
> >  Old World
> > -M: Alexander Graf 
> > +M: David Gibson 
> >  L: qemu-...@nongnu.org
> > -S: Maintained
> > +S: Odd Fixes
> >  F: hw/ppc/mac_oldworld.c
> >  F: hw/pci-host/grackle.c
> >  F: hw/misc/macio/
> > @@ -850,7 +849,6 @@ F: pc-bios/ppc_rom.bin
> >  
> >  sPAPR
> >  M: David Gibson 
> > -M: Alexander Graf 
> >  L: qemu-...@nongnu.org
> >  S: Supported
> >  F: hw/*/spapr*
> > @@ -1117,7 +1115,7 @@ F: tests/bios-tables-test.c
> >  F: tests/acpi-utils.[hc]
> >  
> >  ppc4xx
> > -M: Alexander Graf 
> > +M: David Gibson 
> >  L: qemu-...@nongnu.org
> >  S: Odd Fixes
> >  F: hw/ppc/ppc4*.c
> > @@ -1126,9 +1124,9 @@ F: include/hw/ppc/ppc4xx.h
> >  F: include/hw/i2c/ppc4xx_i2c.h
> >  
> >  ppce500
> > -M: Alexander Graf 
> > +M: David Gibson 
> >  L: qemu-...@nongnu.org
> > -S: Supported
> > +S: Odd Fixes
> >  F: hw/ppc/e500*
> >  F: hw/pci-host/ppce500.c
> >  F: hw/net/fsl_etsec/
> 
> 
> ATB,
> 
> Mark.
> 

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH for-3.1] hw/ppc/ppc440_uc: Remove dead code in sdram_size()

2018-11-06 Thread David Gibson
On Tue, Oct 30, 2018 at 05:03:53PM +, Peter Maydell wrote:
> Coverity points out in CID 1390588 that the test for sh == 0
> in sdram_size() can never fire, because we calculate sh with
> sh = 1024 - ((bcr >> 6) & 0x3ff);
> which must result in a value between 1 and 1024 inclusive.
> 
> Without the relevant manual for the SoC, we're not completely
> sure of the correct behaviour here, but we can remove the
> dead code without changing how QEMU currently behaves.
> 
> Signed-off-by: Peter Maydell 

Applied to ppc-for-3.1, thanks.

> ---
> We had a discussion about this coverity error a while back:
> https://lists.nongnu.org/archive/html/qemu-devel/2018-04/msg05187.html
> I'd just like to squash the Coverity warning, I think.
> 
>  hw/ppc/ppc440_uc.c | 6 +-
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
> index 09ccda548f3..9360f781cef 100644
> --- a/hw/ppc/ppc440_uc.c
> +++ b/hw/ppc/ppc440_uc.c
> @@ -559,11 +559,7 @@ static target_ulong sdram_size(uint32_t bcr)
>  int sh;
>  
>  sh = 1024 - ((bcr >> 6) & 0x3ff);
> -if (sh == 0) {
> -size = -1;
> -} else {
> -size = 8 * MiB * sh;
> -}
> +size = 8 * MiB * sh;
>  
>  return size;
>  }

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH v2 10/10] spapr_pci: perform unplug via the hotplug handler

2018-11-06 Thread David Gibson
On Mon, Nov 05, 2018 at 11:20:44AM +0100, David Hildenbrand wrote:
> Introduce and use the "unplug" callback.
> 
> This is a preparation for multi-stage hotplug handlers, whereby the bus
> hotplug handler is overwritten by the machine hotplug handler. This handler
> will then pass control to the bus hotplug handler. So to get this running
> cleanly, we also have to make sure to go via the hotplug handler chain when
> actually unplugging a device after an unplug request. Lookup the hotplug
> handler and call "unplug".
> 
> Signed-off-by: David Hildenbrand 

Acked-by: David Gibson 

> ---
>  hw/ppc/spapr_pci.c | 33 +
>  1 file changed, 21 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 58afa46204..64b8591023 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1370,18 +1370,9 @@ static int spapr_create_pci_child_dt(sPAPRPHBState 
> *phb, PCIDevice *dev,
>  /* Callback to be called during DRC release. */
>  void spapr_phb_remove_pci_device_cb(DeviceState *dev)
>  {
> -/* some version guests do not wait for completion of a device
> - * cleanup (generally done asynchronously by the kernel) before
> - * signaling to QEMU that the device is safe, but instead sleep
> - * for some 'safe' period of time. unfortunately on a busy host
> - * this sleep isn't guaranteed to be long enough, resulting in
> - * bad things like IRQ lines being left asserted during final
> - * device removal. to deal with this we call reset just prior
> - * to finalizing the device, which will put the device back into
> - * an 'idle' state, as the device cleanup code expects.
> - */
> -pci_device_reset(PCI_DEVICE(dev));
> -object_unparent(OBJECT(dev));
> +HotplugHandler *hotplug_ctrl = qdev_get_hotplug_handler(dev);
> +
> +hotplug_handler_unplug(hotplug_ctrl, dev, &error_abort);
>  }
>  
>  static sPAPRDRConnector *spapr_phb_get_pci_func_drc(sPAPRPHBState *phb,
> @@ -1490,6 +1481,23 @@ out:
>  }
>  }
>  
> +static void spapr_pci_unplug(HotplugHandler *plug_handler,
> + DeviceState *plugged_dev, Error **errp)
> +{
> +/* some version guests do not wait for completion of a device
> + * cleanup (generally done asynchronously by the kernel) before
> + * signaling to QEMU that the device is safe, but instead sleep
> + * for some 'safe' period of time. unfortunately on a busy host
> + * this sleep isn't guaranteed to be long enough, resulting in
> + * bad things like IRQ lines being left asserted during final
> + * device removal. to deal with this we call reset just prior
> + * to finalizing the device, which will put the device back into
> + * an 'idle' state, as the device cleanup code expects.
> + */
> +pci_device_reset(PCI_DEVICE(plugged_dev));
> +object_unparent(OBJECT(plugged_dev));
> +}
> +
>  static void spapr_pci_unplug_request(HotplugHandler *plug_handler,
>   DeviceState *plugged_dev, Error **errp)
>  {
> @@ -1965,6 +1973,7 @@ static void spapr_phb_class_init(ObjectClass *klass, 
> void *data)
>  dc->user_creatable = true;
>  set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>  hp->plug = spapr_pci_plug;
> +hp->unplug = spapr_pci_unplug;
>  hp->unplug_request = spapr_pci_unplug_request;
>  }
>  

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PULL 0/8] Monitor patches for 2018-10-30

2018-11-06 Thread Peter Xu
On Tue, Nov 06, 2018 at 04:56:51PM +0100, Markus Armbruster wrote:
> Peter Maydell  writes:
> 
> > On 30 October 2018 at 19:16, Markus Armbruster  wrote:
> >> The following changes since commit 
> >> 3f3285491dd52014852a56135c90e428c8b507ea:
> >>
> >>   Merge remote-tracking branch 'remotes/jnsnow/tags/bitmaps-pull-request' 
> >> into staging (2018-10-30 14:09:25 +)
> >>
> >> are available in the Git repository at:
> >>
> >>   git://repo.or.cz/qemu/armbru.git tags/pull-monitor-2018-10-30
> >>
> >> for you to fetch changes up to 1a8de278dd3c93a7aa40f88486509fe0e2de04da:
> >>
> >>   vl: Avoid crash when -mon is underspecified (2018-10-30 20:13:52 +0100)
> >>
> >> 
> >> Monitor patches for 2018-10-30
> >>
> >> * Fix crash on shutdown with --daemonize
> >> * Change out-of-band execution to stop reading instead of dropping
> >>   commands
> >> * Enable out-of-band execution by default
> >> * Avoid crash when -mon lacks chardev=...
> >>
> >> 
> >
> > Hi; I get test failures on FreeBSD host:
> >
> > TEST: tests/qmp-test... (pid=18305)
> >   /sparc64/qmp/protocol:   OK
> >   /sparc64/qmp/oob:FAIL
> > GTester: last random seed: R02Se36038cab4c051a2cd47374ec9718e98
> > (pid=18337)
> >   /sparc64/qmp/preconfig:  OK
> > FAIL: tests/qmp-test
> >
> > ERROR:tests/qmp-test.c:211:recv_cmd_id: assertion failed
> > (qdict_get_try_str(resp, "id") == id): ("oob-1" == "queue-blocks-1")
> > gmake: *** [/var/tmp/qemu-test.2n2tBU/tests/Makefile.include:805:
> > check-qtest-sparc] Error 1
> >
> > and similarly with i386/qmp/oob and sparc/qmp/oob.
> 
> I haven't been able to reproduce, yet.  I'm going to respin with fewer
> patches, to get the other stuff in while I debug.

Strange, "make check -j8" failed on my hosts (I tried two) with either
Markus's pull tree or qemu master:

hw/core/ptimer.o: In function `timer_new_tl':
/home/xz/git/qemu/include/qemu/timer.h:536: undefined reference to 
`timer_init_tl'
collect2: error: ld returned 1 exit status
make: *** [/home/xz/git/qemu/rules.mak:124: tests/ptimer-test] Error 1

Is that only happening on my hosts?

And I can't reproduce the error too by only running the standalone
qmp-test.

Regaring to the test failure reported - when I saw that I thought
about the patch "monitor: resume the monitor earlier if needed" in my
series since that seems to have the possibility to bring such an
uncertainty, though I noticed that Markus has already removed that
patch from the pull.

-- 
Peter Xu



Re: [Qemu-devel] [PATCH v2 0/5] migration: improve multithreads

2018-11-06 Thread no-reply
Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20181106122025.3487-1-xiaoguangr...@tencent.com
Subject: [Qemu-devel] [PATCH v2 0/5] migration: improve multithreads

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
64aabd11be tests: add threaded-workqueue-bench
e9efc70edd migration: use threaded workqueue for decompression
7753a846ad migration: use threaded workqueue for compression
f6933b5132 util: introduce threaded workqueue
810caaa566 bitops: introduce change_bit_atomic

=== OUTPUT BEGIN ===
Checking PATCH 1/5: bitops: introduce change_bit_atomic...
Checking PATCH 2/5: util: introduce threaded workqueue...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#41: 
new file mode 100644

total: 0 errors, 1 warnings, 566 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
Checking PATCH 3/5: migration: use threaded workqueue for compression...
Checking PATCH 4/5: migration: use threaded workqueue for decompression...
Checking PATCH 5/5: tests: add threaded-workqueue-bench...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#36: 
new file mode 100644

WARNING: line over 80 characters
#237: FILE: tests/threaded-workqueue-bench.c:197:
+printf("   -r:   the number of requests handled by each thread 
(default %d).\n",

WARNING: line over 80 characters
#239: FILE: tests/threaded-workqueue-bench.c:199:
+printf("   -m:   the size of the memory (G) used to test (default 
%dG).\n",

ERROR: line over 90 characters
#285: FILE: tests/threaded-workqueue-bench.c:245:
+printf("Run the benchmark: threads %d requests-per-thread: %d memory %ldG 
repeat %d.\n",

total: 1 errors, 3 warnings, 273 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [Qemu-devel] [PATCH] MAINTAINERS: PPC: Remove myself

2018-11-06 Thread David Gibson
On Tue, Oct 30, 2018 at 10:35:31AM +0100, Alexander Graf wrote:
> I haven't really been maintaining any PowerPC code for quite a while now,
> so let's reflect reality: David does all the work and embedded PPC is in
> "Odd Fixes" state rather than supported now.
> 
> Signed-off-by: Alexander Graf 

Applied to ppc-for-3.1.

> ---
>  MAINTAINERS | 30 ++
>  1 file changed, 14 insertions(+), 16 deletions(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index d794bd7a66..d1a6d3c740 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -230,7 +230,6 @@ F: tests/tcg/openrisc/
>  
>  PowerPC
>  M: David Gibson 
> -M: Alexander Graf 
>  L: qemu-...@nongnu.org
>  S: Maintained
>  F: target/ppc/
> @@ -340,7 +339,7 @@ S: Maintained
>  F: target/mips/kvm.c
>  
>  PPC
> -M: Alexander Graf 
> +M: David Gibson 
>  S: Maintained
>  F: target/ppc/kvm.c
>  
> @@ -780,21 +779,21 @@ F: hw/openrisc/openrisc_sim.c
>  PowerPC Machines
>  
>  405
> -M: Alexander Graf 
> +M: David Gibson 
>  L: qemu-...@nongnu.org
>  S: Odd Fixes
>  F: hw/ppc/ppc405_boards.c
>  
>  Bamboo
> -M: Alexander Graf 
> +M: David Gibson 
>  L: qemu-...@nongnu.org
>  S: Odd Fixes
>  F: hw/ppc/ppc440_bamboo.c
>  
>  e500
> -M: Alexander Graf 
> +M: David Gibson 
>  L: qemu-...@nongnu.org
> -S: Supported
> +S: Odd Fixes
>  F: hw/ppc/e500.[hc]
>  F: hw/ppc/e500plat.c
>  F: include/hw/ppc/ppc_e500.h
> @@ -802,16 +801,16 @@ F: include/hw/pci-host/ppce500.h
>  F: pc-bios/u-boot.e500
>  
>  mpc8544ds
> -M: Alexander Graf 
> +M: David Gibson 
>  L: qemu-...@nongnu.org
> -S: Supported
> +S: Odd Fixes
>  F: hw/ppc/mpc8544ds.c
>  F: hw/ppc/mpc8544_guts.c
>  
>  New World
> -M: Alexander Graf 
> +M: David Gibson 
>  L: qemu-...@nongnu.org
> -S: Maintained
> +S: Odd Fixes
>  F: hw/ppc/mac_newworld.c
>  F: hw/pci-host/uninorth.c
>  F: hw/pci-bridge/dec.[hc]
> @@ -823,9 +822,9 @@ F: include/hw/misc/mos6522.h
>  F: include/hw/ppc/mac_dbdma.h
>  
>  Old World
> -M: Alexander Graf 
> +M: David Gibson 
>  L: qemu-...@nongnu.org
> -S: Maintained
> +S: Odd Fixes
>  F: hw/ppc/mac_oldworld.c
>  F: hw/pci-host/grackle.c
>  F: hw/misc/macio/
> @@ -850,7 +849,6 @@ F: pc-bios/ppc_rom.bin
>  
>  sPAPR
>  M: David Gibson 
> -M: Alexander Graf 
>  L: qemu-...@nongnu.org
>  S: Supported
>  F: hw/*/spapr*
> @@ -1117,7 +1115,7 @@ F: tests/bios-tables-test.c
>  F: tests/acpi-utils.[hc]
>  
>  ppc4xx
> -M: Alexander Graf 
> +M: David Gibson 
>  L: qemu-...@nongnu.org
>  S: Odd Fixes
>  F: hw/ppc/ppc4*.c
> @@ -1126,9 +1124,9 @@ F: include/hw/ppc/ppc4xx.h
>  F: include/hw/i2c/ppc4xx_i2c.h
>  
>  ppce500
> -M: Alexander Graf 
> +M: David Gibson 
>  L: qemu-...@nongnu.org
> -S: Supported
> +S: Odd Fixes
>  F: hw/ppc/e500*
>  F: hw/pci-host/ppce500.c
>  F: hw/net/fsl_etsec/

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH] gtk: Fix mouse offset in scaled gtk-gl display for VFIO/iGVT-g DMA Buf mode

2018-11-06 Thread Chen Zhang via Qemu-devel

Yes, this patch just works.

Thank you.

Best regards

On Nov 06, 2018, at 08:22 PM, Gerd Hoffmann  wrote:

On Wed, Oct 31, 2018 at 06:24:56AM +, Chen Zhang wrote:
The issue was reported as in https://bugs.launchpad.net/qemu/+bug/1793859

When an OpenGL accelerated GTK window is used for iGVT-g DMA Buf device,
window scaling would cause guest cursor to move in undesirable velocity.

To fix this usability issue, the gtk mouse motion events was modified to
scale the position of event to match the coordinates of the scaled GL
surface.

Hmm, we already have logic in place to deal with that. I suspect the
root cause is simply that the scale_{x,y} variables are not set properly
in egl mode. Can you try the patch below?

thanks,
Gerd

== [ cut here ] ===
From ebecaab0102aca37a3101131c20f45576835b6b3 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann 
Date: Tue, 6 Nov 2018 13:18:39 +0100
Subject: [PATCH] try fix gtk egl cursor

---
ui/gtk-egl.c | 17 -
1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/ui/gtk-egl.c b/ui/gtk-egl.c
index a77c25b490..5420c2362b 100644
--- a/ui/gtk-egl.c
+++ b/ui/gtk-egl.c
@@ -68,8 +68,15 @@ void gd_egl_draw(VirtualConsole *vc)
return;
}

+ window = gtk_widget_get_window(vc->gfx.drawing_area);
+ ww = gdk_window_get_width(window);
+ wh = gdk_window_get_height(window);
+
if (vc->gfx.scanout_mode) {
gd_egl_scanout_flush(&vc->gfx.dcl, 0, 0, vc->gfx.w, vc->gfx.h);
+
+ vc->gfx.scale_x = (double)ww / vc->gfx.w;
+ vc->gfx.scale_y = (double)wh / vc->gfx.h;
} else {
if (!vc->gfx.ds) {
return;
@@ -77,13 +84,13 @@ void gd_egl_draw(VirtualConsole *vc)
eglMakeCurrent(qemu_egl_display, vc->gfx.esurface,
vc->gfx.esurface, vc->gfx.ectx);

- window = gtk_widget_get_window(vc->gfx.drawing_area);
- ww = gdk_window_get_width(window);
- wh = gdk_window_get_height(window);
surface_gl_setup_viewport(vc->gfx.gls, vc->gfx.ds, ww, wh);
surface_gl_render_texture(vc->gfx.gls, vc->gfx.ds);

eglSwapBuffers(qemu_egl_display, vc->gfx.esurface);
+
+ vc->gfx.scale_x = (double)ww / surface_width(vc->gfx.ds);
+ vc->gfx.scale_y = (double)wh / surface_height(vc->gfx.ds);
}
}

@@ -232,8 +239,8 @@ void gd_egl_cursor_position(DisplayChangeListener *dcl,
{
VirtualConsole *vc = container_of(dcl, VirtualConsole, gfx.dcl);

- vc->gfx.cursor_x = pos_x;
- vc->gfx.cursor_y = pos_y;
+ vc->gfx.cursor_x = pos_x * vc->gfx.scale_x;
+ vc->gfx.cursor_y = pos_y * vc->gfx.scale_y;
}

void gd_egl_release_dmabuf(DisplayChangeListener *dcl,
--
2.9.3



Re: [Qemu-devel] [PATCH v2 09/10] pci/shpc: perform unplug via the hotplug handler

2018-11-06 Thread David Gibson
On Mon, Nov 05, 2018 at 11:20:43AM +0100, David Hildenbrand wrote:
> Introduce and use the "unplug" callback.
> 
> This is a preparation for multi-stage hotplug handlers, whereby the bus
> hotplug handler is overwritten by the machine hotplug handler. This handler
> will then pass control to the bus hotplug handler. So to get this running
> cleanly, we also have to make sure to go via the hotplug handler chain when
> actually unplugging a device after an unplug request. Lookup the hotplug
> handler and call "unplug".
> 
> Signed-off-by: David Hildenbrand 

Reviewed-by: David Gibson 

> ---
>  hw/pci-bridge/pci_bridge_dev.c  | 14 ++
>  hw/pci-bridge/pcie_pci_bridge.c | 14 ++
>  hw/pci/shpc.c   | 11 ++-
>  include/hw/pci/shpc.h   |  2 ++
>  4 files changed, 40 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c
> index e1df9a52ac..91415f114c 100644
> --- a/hw/pci-bridge/pci_bridge_dev.c
> +++ b/hw/pci-bridge/pci_bridge_dev.c
> @@ -219,6 +219,19 @@ static void pci_bridge_dev_plug_cb(HotplugHandler 
> *hotplug_dev,
>  shpc_device_plug_cb(hotplug_dev, dev, errp);
>  }
>  
> +static void pci_bridge_dev_unplug_cb(HotplugHandler *hotplug_dev,
> + DeviceState *dev, Error **errp)
> +{
> +PCIDevice *pci_hotplug_dev = PCI_DEVICE(hotplug_dev);
> +
> +if (!shpc_present(pci_hotplug_dev)) {
> +error_setg(errp, "standard hotplug controller has been disabled for "
> +   "this %s", TYPE_PCI_BRIDGE_DEV);
> +return;
> +}
> +shpc_device_unplug_cb(hotplug_dev, dev, errp);
> +}
> +
>  static void pci_bridge_dev_unplug_request_cb(HotplugHandler *hotplug_dev,
>   DeviceState *dev, Error **errp)
>  {
> @@ -251,6 +264,7 @@ static void pci_bridge_dev_class_init(ObjectClass *klass, 
> void *data)
>  dc->vmsd = &pci_bridge_dev_vmstate;
>  set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>  hc->plug = pci_bridge_dev_plug_cb;
> +hc->unplug = pci_bridge_dev_unplug_cb;
>  hc->unplug_request = pci_bridge_dev_unplug_request_cb;
>  }
>  
> diff --git a/hw/pci-bridge/pcie_pci_bridge.c b/hw/pci-bridge/pcie_pci_bridge.c
> index c634353b06..7c667bc97c 100644
> --- a/hw/pci-bridge/pcie_pci_bridge.c
> +++ b/hw/pci-bridge/pcie_pci_bridge.c
> @@ -150,6 +150,19 @@ static void pcie_pci_bridge_plug_cb(HotplugHandler 
> *hotplug_dev,
>  shpc_device_plug_cb(hotplug_dev, dev, errp);
>  }
>  
> +static void pcie_pci_bridge_unplug_cb(HotplugHandler *hotplug_dev,
> +  DeviceState *dev, Error **errp)
> +{
> +PCIDevice *pci_hotplug_dev = PCI_DEVICE(hotplug_dev);
> +
> +if (!shpc_present(pci_hotplug_dev)) {
> +error_setg(errp, "standard hotplug controller has been disabled for "
> +   "this %s", TYPE_PCIE_PCI_BRIDGE_DEV);
> +return;
> +}
> +shpc_device_unplug_cb(hotplug_dev, dev, errp);
> +}
> +
>  static void pcie_pci_bridge_unplug_request_cb(HotplugHandler *hotplug_dev,
>DeviceState *dev, Error **errp)
>  {
> @@ -180,6 +193,7 @@ static void pcie_pci_bridge_class_init(ObjectClass 
> *klass, void *data)
>  dc->reset = &pcie_pci_bridge_reset;
>  set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
>  hc->plug = pcie_pci_bridge_plug_cb;
> +hc->unplug = pcie_pci_bridge_unplug_cb;
>  hc->unplug_request = pcie_pci_bridge_unplug_request_cb;
>  }
>  
> diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
> index 098ffaef1d..5de905cb56 100644
> --- a/hw/pci/shpc.c
> +++ b/hw/pci/shpc.c
> @@ -238,6 +238,7 @@ static void shpc_invalid_command(SHPCDevice *shpc)
>  
>  static void shpc_free_devices_in_slot(SHPCDevice *shpc, int slot)
>  {
> +HotplugHandler *hotplug_ctrl;
>  int devfn;
>  int pci_slot = SHPC_IDX_TO_PCI(slot);
>  for (devfn = PCI_DEVFN(pci_slot, 0);
> @@ -245,7 +246,9 @@ static void shpc_free_devices_in_slot(SHPCDevice *shpc, 
> int slot)
>   ++devfn) {
>  PCIDevice *affected_dev = shpc->sec_bus->devices[devfn];
>  if (affected_dev) {
> -object_unparent(OBJECT(affected_dev));
> +hotplug_ctrl = qdev_get_hotplug_handler(DEVICE(affected_dev));
> +hotplug_handler_unplug(hotplug_ctrl, DEVICE(affected_dev),
> +   &error_abort);
>  }
>  }
>  }
> @@ -540,6 +543,12 @@ void shpc_device_plug_cb(HotplugHandler *hotplug_dev, 
> DeviceState *dev,
>  shpc_interrupt_update(pci_hotplug_dev);
>  }
>  
> +void shpc_device_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> +   Error **errp)
> +{
> +object_unparent(OBJECT(dev));
> +}
> +
>  void shpc_device_unplug_request_cb(HotplugHandler *hotplug_dev,
> DeviceState *dev, Error **errp)
>  {
> diff --git a/include/hw/pci/shpc.h b/include/

Re: [Qemu-devel] [PATCH v2 08/10] pci/pcie: perform unplug via the hotplug handler

2018-11-06 Thread David Gibson
On Mon, Nov 05, 2018 at 11:20:42AM +0100, David Hildenbrand wrote:
> Introduce and use the "unplug" callback.
> 
> This is a preparation for multi-stage hotplug handlers, whereby the bus
> hotplug handler is overwritten by the machine hotplug handler. This handler
> will then pass control to the bus hotplug handler. So to get this running
> cleanly, we also have to make sure to go via the hotplug handler chain when
> actually unplugging a device after an unplug request. Lookup the hotplug
> handler and call "unplug".
> 
> Signed-off-by: David Hildenbrand 

Reviewed-by: David Gibson 

> ---
>  hw/pci/pcie.c | 10 +-
>  hw/pci/pcie_port.c|  1 +
>  include/hw/pci/pcie.h |  2 ++
>  3 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index ccba29269e..ba3ea925e9 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -364,11 +364,19 @@ void pcie_cap_slot_plug_cb(HotplugHandler *hotplug_dev, 
> DeviceState *dev,
>  }
>  }
>  
> -static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque)
> +void pcie_cap_slot_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> + Error **errp)
>  {
>  object_unparent(OBJECT(dev));
>  }
>  
> +static void pcie_unplug_device(PCIBus *bus, PCIDevice *dev, void *opaque)
> +{
> +HotplugHandler *hotplug_ctrl = qdev_get_hotplug_handler(DEVICE(dev));
> +
> +hotplug_handler_unplug(hotplug_ctrl, DEVICE(dev), &error_abort);
> +}
> +
>  void pcie_cap_slot_unplug_request_cb(HotplugHandler *hotplug_dev,
>   DeviceState *dev, Error **errp)
>  {
> diff --git a/hw/pci/pcie_port.c b/hw/pci/pcie_port.c
> index 7982a87880..a30291ef54 100644
> --- a/hw/pci/pcie_port.c
> +++ b/hw/pci/pcie_port.c
> @@ -156,6 +156,7 @@ static void pcie_slot_class_init(ObjectClass *oc, void 
> *data)
>  dc->props = pcie_slot_props;
>  hc->pre_plug = pcie_cap_slot_pre_plug_cb;
>  hc->plug = pcie_cap_slot_plug_cb;
> +hc->unplug = pcie_cap_slot_unplug_cb;
>  hc->unplug_request = pcie_cap_slot_unplug_request_cb;
>  }
>  
> diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
> index d9fbcf4a4a..9ae6482658 100644
> --- a/include/hw/pci/pcie.h
> +++ b/include/hw/pci/pcie.h
> @@ -135,6 +135,8 @@ void pcie_cap_slot_pre_plug_cb(HotplugHandler 
> *hotplug_dev, DeviceState *dev,
> Error **errp);
>  void pcie_cap_slot_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> Error **errp);
> +void pcie_cap_slot_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> + Error **errp);
>  void pcie_cap_slot_unplug_request_cb(HotplugHandler *hotplug_dev,
>   DeviceState *dev, Error **errp);
>  #endif /* QEMU_PCIE_H */

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH v3 26/35] target/riscv: Remove shift and slt insn manual decoding

2018-11-06 Thread Palmer Dabbelt

On Mon, 05 Nov 2018 09:00:10 PST (-0800), Bastian Koppelmann wrote:


On 11/1/18 4:59 PM, Palmer Dabbelt wrote:

On Wed, 31 Oct 2018 15:38:08 PDT (-0700), richard.hender...@linaro.org
wrote:

On 10/31/18 1:20 PM, Bastian Koppelmann wrote:

 static bool trans_slt(DisasContext *ctx, arg_slt *a)
 {
-    gen_arith(ctx, OPC_RISC_SLT, a->rd, a->rs1, a->rs2);
+    TCGv source1 = tcg_temp_new();
+    TCGv source2 = tcg_temp_new();
+
+    gen_get_gpr(source1, a->rs1);
+    gen_get_gpr(source2, a->rs2);
+
+    tcg_gen_setcond_tl(TCG_COND_LT, source1, source1, source2);


I do wonder about extracting this one line to gen_slt so that you can
re-use
gen_arith and gen_arithi.


+    tcg_gen_setcond_tl(TCG_COND_LTU, source1, source1, source2);


Similarly.


 static bool trans_sllw(DisasContext *ctx, arg_sllw *a)
 {
-    gen_arith(ctx, OPC_RISC_SLLW, a->rd, a->rs1, a->rs2);
+    TCGv source1 = tcg_temp_new();
+    TCGv source2 = tcg_temp_new();
+
+    gen_get_gpr(source1, a->rs1);
+    gen_get_gpr(source2, a->rs2);
+
+    tcg_gen_andi_tl(source2, source2, 0x1F);
+    tcg_gen_shl_tl(source1, source1, source2);
+
+    gen_set_gpr(a->rd, source1);


Missing the ext32s after the shift.


 static bool trans_srlw(DisasContext *ctx, arg_srlw *a)
 {
-    gen_arith(ctx, OPC_RISC_SRLW, a->rd, a->rs1, a->rs2);
+    TCGv source1 = tcg_temp_new();
+    TCGv source2 = tcg_temp_new();
+
+    gen_get_gpr(source1, a->rs1);
+    gen_get_gpr(source2, a->rs2);
+
+    /* clear upper 32 */
+    tcg_gen_ext32u_tl(source1, source1);
+    tcg_gen_andi_tl(source2, source2, 0x1F);
+    tcg_gen_shr_tl(source1, source1, source2);


Likewise.  (Consider source2 == 0.)


-    case OPC_RISC_SRL:
-    tcg_gen_andi_tl(source2, source2, TARGET_LONG_BITS - 1);
-    tcg_gen_shr_tl(source1, source1, source2);
-    break;

...

-    case OPC_RISC_SRA:
-    tcg_gen_andi_tl(source2, source2, TARGET_LONG_BITS - 1);
-    tcg_gen_sar_tl(source1, source1, source2);
-    break;


I see that the bugs are in the original though, so fixing them in a
separate
patch is certainly ok.


Since we're in the soft freeze now, I think it would be great to get
the bug fixes broken out as separate patches at the start of this
series that we can pick up for this release.  It would be great if the
decodetree conversion was a non-functional change, as that will make
it easier to review.



Sounds good. Unfortunately, I don't have much time right now. I try at
least to extract the bugfixes, such that they make it for this release.


Thanks.  You're welcome to just enumerate the bug fixes so I can dig them out 
if that's easier.




[Qemu-devel] [PATCH] 9p: write lock path in v9fs_co_open2()

2018-11-06 Thread Greg Kurz
The assumption that the fid cannot be used by any other operation is
wrong. At least, nothing prevents a misbehaving client to create a
file with a given fid, and to pass this fid to some other operation
at the same time (ie, without waiting for the response to the creation
request). The call to v9fs_path_copy() performed by the worker thread
after the file was created can race with any access to the fid path
performed by some other thread. This causes use-after-free issues that
can be detected by ASAN with a custom 9p client.

Unlike other operations that only read the fid path, v9fs_co_open2()
does modify it. It should hence take the write lock.

Cc: P J P 
Reported-by: zhibin hu 
Signed-off-by: Greg Kurz 
---
 hw/9pfs/cofile.c |6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/9pfs/cofile.c b/hw/9pfs/cofile.c
index 88791bc327ac..9c22837cda32 100644
--- a/hw/9pfs/cofile.c
+++ b/hw/9pfs/cofile.c
@@ -140,10 +140,10 @@ int coroutine_fn v9fs_co_open2(V9fsPDU *pdu, V9fsFidState 
*fidp,
 cred.fc_gid = gid;
 /*
  * Hold the directory fid lock so that directory path name
- * don't change. Read lock is fine because this fid cannot
- * be used by any other operation.
+ * don't change. Take the write lock to be sure this fid
+ * cannot be used by another operation.
  */
-v9fs_path_read_lock(s);
+v9fs_path_write_lock(s);
 v9fs_co_run_in_worker(
 {
 err = s->ops->open2(&s->ctx, &fidp->path,




Re: [Qemu-devel] [PATCH v2 04/10] pci/pcie: stop plug/unplug if the slot is locked

2018-11-06 Thread David Gibson
On Mon, Nov 05, 2018 at 11:20:38AM +0100, David Hildenbrand wrote:
> We better stop right away. While at it, properly move the check
> to the pre_plug handler.
> 
> Reviewed-by: Igor Mammedov 
> Signed-off-by: David Hildenbrand 

Reviewed-by: David Gibson 

Although I'm not sure the commit message gives a terribly clear
picture of what's going on here.

> ---
>  hw/pci/pcie.c | 25 +
>  hw/pci/pcie_port.c|  1 +
>  include/hw/pci/pcie.h |  2 ++
>  3 files changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 44737cc1cd..ccba29269e 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -316,10 +316,10 @@ static void pcie_cap_slot_event(PCIDevice *dev, 
> PCIExpressHotPlugEvent event)
>  }
>  
>  static void pcie_cap_slot_plug_common(PCIDevice *hotplug_dev, DeviceState 
> *dev,
> -  uint8_t **exp_cap, Error **errp)
> +  Error **errp)
>  {
> -*exp_cap = hotplug_dev->config + hotplug_dev->exp.exp_cap;
> -uint16_t sltsta = pci_get_word(*exp_cap + PCI_EXP_SLTSTA);
> +uint8_t *exp_cap = hotplug_dev->config + hotplug_dev->exp.exp_cap;
> +uint16_t sltsta = pci_get_word(exp_cap + PCI_EXP_SLTSTA);
>  
>  PCIE_DEV_PRINTF(PCI_DEVICE(dev), "hotplug state: 0x%x\n", sltsta);
>  if (sltsta & PCI_EXP_SLTSTA_EIS) {
> @@ -330,14 +330,19 @@ static void pcie_cap_slot_plug_common(PCIDevice 
> *hotplug_dev, DeviceState *dev,
>  }
>  }
>  
> +void pcie_cap_slot_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> +   Error **errp)
> +{
> +pcie_cap_slot_plug_common(PCI_DEVICE(hotplug_dev), dev, errp);
> +}
> +
>  void pcie_cap_slot_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> Error **errp)
>  {
> -uint8_t *exp_cap;
> +PCIDevice *hotplug_pdev = PCI_DEVICE(hotplug_dev);
> +uint8_t *exp_cap = hotplug_pdev->config + hotplug_pdev->exp.exp_cap;
>  PCIDevice *pci_dev = PCI_DEVICE(dev);
>  
> -pcie_cap_slot_plug_common(PCI_DEVICE(hotplug_dev), dev, &exp_cap, errp);
> -
>  /* Don't send event when device is enabled during qemu machine creation:
>   * it is present on boot, no hotplug event is necessary. We do send an
>   * event when the device is disabled later. */
> @@ -367,11 +372,15 @@ static void pcie_unplug_device(PCIBus *bus, PCIDevice 
> *dev, void *opaque)
>  void pcie_cap_slot_unplug_request_cb(HotplugHandler *hotplug_dev,
>   DeviceState *dev, Error **errp)
>  {
> -uint8_t *exp_cap;
> +Error *local_err = NULL;
>  PCIDevice *pci_dev = PCI_DEVICE(dev);
>  PCIBus *bus = pci_get_bus(pci_dev);
>  
> -pcie_cap_slot_plug_common(PCI_DEVICE(hotplug_dev), dev, &exp_cap, errp);
> +pcie_cap_slot_plug_common(PCI_DEVICE(hotplug_dev), dev, &local_err);
> +if (local_err) {
> +error_propagate(errp, local_err);
> +return;
> +}
>  
>  /* In case user cancel the operation of multi-function hot-add,
>   * remove the function that is unexposed to guest individually,
> diff --git a/hw/pci/pcie_port.c b/hw/pci/pcie_port.c
> index 73e81e5847..7982a87880 100644
> --- a/hw/pci/pcie_port.c
> +++ b/hw/pci/pcie_port.c
> @@ -154,6 +154,7 @@ static void pcie_slot_class_init(ObjectClass *oc, void 
> *data)
>  HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
>  
>  dc->props = pcie_slot_props;
> +hc->pre_plug = pcie_cap_slot_pre_plug_cb;
>  hc->plug = pcie_cap_slot_plug_cb;
>  hc->unplug_request = pcie_cap_slot_unplug_request_cb;
>  }
> diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
> index 735f8e8154..d9fbcf4a4a 100644
> --- a/include/hw/pci/pcie.h
> +++ b/include/hw/pci/pcie.h
> @@ -131,6 +131,8 @@ void pcie_ari_init(PCIDevice *dev, uint16_t offset, 
> uint16_t nextfn);
>  void pcie_dev_ser_num_init(PCIDevice *dev, uint16_t offset, uint64_t 
> ser_num);
>  void pcie_ats_init(PCIDevice *dev, uint16_t offset);
>  
> +void pcie_cap_slot_pre_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> +   Error **errp);
>  void pcie_cap_slot_plug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
> Error **errp);
>  void pcie_cap_slot_unplug_request_cb(HotplugHandler *hotplug_dev,

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PATCH v1 1/1] riscv: spike: Fix memory leak in the board init

2018-11-06 Thread Palmer Dabbelt

On Mon, 05 Nov 2018 11:44:41 PST (-0800), Alistair Francis wrote:

Coverity caught a malloc() call that was never freed. This patch ensures
that we free the memory but also updates the allocation to use
g_strdup_printf() instead of malloc().

Signed-off-by: Alistair Francis 
Suggested-by: Peter Maydell 
---
 hw/riscv/spike.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c
index 8a712ed490..268df04c3c 100644
--- a/hw/riscv/spike.c
+++ b/hw/riscv/spike.c
@@ -316,9 +316,7 @@ static void spike_v1_09_1_board_init(MachineState *machine)
 
 /* build config string with supplied memory size */

 char *isa = riscv_isa_string(&s->soc.harts[0]);
-size_t config_string_size = strlen(config_string_tmpl) + 48;
-char *config_string = malloc(config_string_size);
-snprintf(config_string, config_string_size, config_string_tmpl,
+char *config_string = g_strdup_printf(config_string_tmpl,
 (uint64_t)memmap[SPIKE_CLINT].base + SIFIVE_TIME_BASE,
 (uint64_t)memmap[SPIKE_DRAM].base,
 (uint64_t)ram_size, isa,
@@ -345,6 +343,8 @@ static void spike_v1_09_1_board_init(MachineState *machine)
 /* Core Local Interruptor (timer and IPI) */
 sifive_clint_create(memmap[SPIKE_CLINT].base, memmap[SPIKE_CLINT].size,
 smp_cpus, SIFIVE_SIP_BASE, SIFIVE_TIMECMP_BASE, SIFIVE_TIME_BASE);
+
+g_free(config_string);
 }
 
 static void spike_v1_09_1_machine_init(MachineClass *mc)


Reviewed-by: Palmer Dabbelt 



Re: [Qemu-devel] [PATCH 7/7] qcow2: do decompression in threads

2018-11-06 Thread Paolo Bonzini
On 01/11/2018 19:27, Vladimir Sementsov-Ogievskiy wrote:
> -/* See qcow2_compress definition for parameters description */
> -static ssize_t qcow2_co_compress(BlockDriverState *bs,
> - void *dest, size_t dest_size,
> - const void *src, size_t src_size)
> +static ssize_t qcow2_co_do_compress(BlockDriverState *bs,
> +void *dest, size_t dest_size,
> +const void *src, size_t src_size,
> +Qcow2CompressFunc func)
>  {
>  BDRVQcow2State *s = bs->opaque;
>  BlockAIOCB *acb;
> @@ -3838,6 +3842,7 @@ static ssize_t qcow2_co_compress(BlockDriverState *bs,
>  .dest_size = dest_size,
>  .src = src,
>  .src_size = src_size,
> +.func = func,
>  };
>  
>  while (s->nb_compress_threads >= MAX_COMPRESS_THREADS) {
> @@ -3860,6 +3865,22 @@ static ssize_t qcow2_co_compress(BlockDriverState *bs,
>  return arg.ret;
>  }
>  

Not a blocker for this patch, but I'll note that qcow2_co_do_compress
could use thread_pool_submit_co.  Also, qcow2_co_do_compress should be a
"coroutine_fn".

Paolo



Re: [Qemu-devel] [PATCH v2 1/6] move ObjectClass to typedefs.h

2018-11-06 Thread David Gibson
On Tue, Nov 06, 2018 at 11:23:30AM +0100, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann 

Reviewed-by: David Gibson 

> ---
>  include/qemu/typedefs.h | 1 +
>  include/qom/object.h| 1 -
>  2 files changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h
> index 3ec0e13a96..fed53f6de2 100644
> --- a/include/qemu/typedefs.h
> +++ b/include/qemu/typedefs.h
> @@ -62,6 +62,7 @@ typedef struct NetClientState NetClientState;
>  typedef struct NetFilterState NetFilterState;
>  typedef struct NICInfo NICInfo;
>  typedef struct NumaNodeMem NumaNodeMem;
> +typedef struct ObjectClass ObjectClass;
>  typedef struct PCIBridge PCIBridge;
>  typedef struct PCIBus PCIBus;
>  typedef struct PCIDevice PCIDevice;
> diff --git a/include/qom/object.h b/include/qom/object.h
> index f0b0bf39cc..499e1fd8b7 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -20,7 +20,6 @@
>  struct TypeImpl;
>  typedef struct TypeImpl *Type;
>  
> -typedef struct ObjectClass ObjectClass;
>  typedef struct Object Object;
>  
>  typedef struct TypeInfo TypeInfo;

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


Re: [Qemu-devel] [PULL 12/15] Acceptance tests: add make rule for running them

2018-11-06 Thread Paolo Bonzini
On 31/10/2018 01:31, Eduardo Habkost wrote:
> From: Cleber Rosa 
> 
> The acceptance (aka functional, aka Avocado-based) tests are
> Python files located in "tests/acceptance" that need to be run
> with the Avocado libs and test runner.
> 
> Let's provide a convenient way for QEMU developers to run them,
> by making use of the tests-venv with the required setup.
> 
> Also, while the Avocado test runner will take care of creating a
> location to save test results to, it was understood that it's better
> if the results are kept within the build tree.
> 
> Signed-off-by: Cleber Rosa 
> Acked-by: Stefan Hajnoczi 
> Acked-by: Wainer dos Santos Moschetta 
> Reviewed-by: Caio Carrara 
> Message-Id: <20181018153134.8493-3-cr...@redhat.com>
> Signed-off-by: Eduardo Habkost 
> ---
>  docs/devel/testing.rst | 43 +-
>  tests/requirements.txt |  1 +
>  tests/Makefile.include | 21 +++--
>  3 files changed, 58 insertions(+), 7 deletions(-)
> 
> diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst
> index a227754f86..18e2c0868a 100644
> --- a/docs/devel/testing.rst
> +++ b/docs/devel/testing.rst
> @@ -545,10 +545,39 @@ Tests based on ``avocado_qemu.Test`` can easily:
> - 
> http://avocado-framework.readthedocs.io/en/latest/api/test/avocado.html#avocado.Test
> - 
> http://avocado-framework.readthedocs.io/en/latest/api/utils/avocado.utils.html
>  
> -Installation
> -
> +Running tests
> +-
> +
> +You can run the acceptance tests simply by executing:
> +
> +.. code::
> +
> +  make check-acceptance
> +
> +This involves the automatic creation of Python virtual environment
> +within the build tree (at ``tests/venv``) which will have all the
> +right dependencies, and will save tests results also within the
> +build tree (at ``tests/results``).
>  
> -To install Avocado and its dependencies, run:
> +Note: the build environment must be using a Python 3 stack, and have
> +the ``venv`` and ``pip`` packages installed.  If necessary, make sure
> +``configure`` is called with ``--python=`` and that those modules are
> +available.  On Debian and Ubuntu based systems, depending on the
> +specific version, they may be on packages named ``python3-venv`` and
> +``python3-pip``.
> +
> +The scripts installed inside the virtual environment may be used
> +without an "activation".  For instance, the Avocado test runner
> +may be invoked by running:
> +
> + .. code::
> +
> +  tests/venv/bin/avocado run $OPTION1 $OPTION2 tests/acceptance/
> +
> +Manual Installation
> +---
> +
> +To manually install Avocado and its dependencies, run:
>  
>  .. code::
>  
> @@ -689,11 +718,15 @@ The exact QEMU binary to be used on QEMUMachine.
>  Uninstalling Avocado
>  
>  
> -If you've followed the installation instructions above, you can easily
> -uninstall Avocado.  Start by listing the packages you have installed::
> +If you've followed the manual installation instructions above, you can
> +easily uninstall Avocado.  Start by listing the packages you have
> +installed::
>  
>pip list --user
>  
>  And remove any package you want with::
>  
>pip uninstall 
> +
> +If you've used ``make check-acceptance``, the Python virtual environment 
> where
> +Avocado is installed will be cleaned up as part of ``make check-clean``.
> diff --git a/tests/requirements.txt b/tests/requirements.txt
> index d39f9d1576..64c6e27a94 100644
> --- a/tests/requirements.txt
> +++ b/tests/requirements.txt
> @@ -1,3 +1,4 @@
>  # Add Python module requirements, one per line, to be installed
>  # in the tests/venv Python virtual environment. For more info,
>  # refer to: https://pip.pypa.io/en/stable/user_guide/#id1
> +avocado-framework==65.0
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index eabc1da2f3..d2e577eabb 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -11,6 +11,7 @@ check-help:
>   @echo " $(MAKE) check-qapi-schemaRun QAPI schema tests"
>   @echo " $(MAKE) check-block  Run block tests"
>   @echo " $(MAKE) check-tcgRun TCG tests"
> + @echo " $(MAKE) check-acceptance Run all acceptance (functional) 
> tests"
>   @echo " $(MAKE) check-report.htmlGenerates an HTML test report"
>   @echo " $(MAKE) check-venv   Creates a Python venv for tests"
>   @echo " $(MAKE) check-clean  Clean the tests"
> @@ -902,10 +903,15 @@ check-decodetree:
>  
>  # Python venv for running tests
>  
> -.PHONY: check-venv
> +.PHONY: check-venv check-acceptance
>  
>  TESTS_VENV_DIR=$(BUILD_DIR)/tests/venv
>  TESTS_VENV_REQ=$(SRC_PATH)/tests/requirements.txt
> +TESTS_RESULTS_DIR=$(BUILD_DIR)/tests/results
> +# Controls the output generated by Avocado when running tests.
> +# Any number of command separated loggers are accepted.  For more
> +# information please refer to "avocado --help".
> +AVOCADO_SHOW=none
>  
>  $(shell $(PYTHON) -c 'import sys; assert 

Re: [Qemu-devel] List of files containing devices which have not been QOMified

2018-11-06 Thread Paolo Bonzini
On 07/11/2018 00:05, Peter Maydell wrote:
> On 6 November 2018 at 19:46, Paolo Bonzini  wrote:
>> On 06/11/2018 19:43, Peter Maydell wrote:
>>> hw/core/ptimer.c
>>
>> Not a device.
> 
> Indeed not, but it could be a QOM object I guess (would
> that gain us anything?)

I don't know, it seems to me more like a generic high-level abstraction
around QEMUTimer.

>>> hw/i2c/bitbang_i2c.c
>>
>> TYPE_GPIO_I2C?
> 
> That part is, but bitbang_i2c_init() creates an object
> which isn't a QOM object and is used by some other i2c devices.

Ah, I see.  Then I think it is in the same family as AHCIState.

>>> hw/ide/ahci.c
>>
>> Even though AHCIState is not a QOM object, all of its users are
>> (TYPE_SYSBUS_AHCI is in this file, TYPE_ICH9_AHCI is in hw/ide/ich.c)
> 
> Mmm, this is one of those which I was unsure about so
> put on the list anyway.
> 
> Overall something that occurs to me is that I'm not sure
> what exactly (other than tidiness) we gain from converting
> remaining non-QOM devices. In some of the other cases I've
> looked at (like sysbus init methods or old_mmio users) we
> get to complete an API transition and remove the old code.
> For a non-QOM device, how much does it hurt us that they're
> lying around in the codebase? We might do better to
> specifically target APIs we'd like to deal with (like
> direct uses of vmstate_register, maybe?).

Yes, those are ugly and are definitely a sign of a legacy device
(non-qdev even before QOM).  serial_mm_init is the main example as you
point out below.

Paolo

> Some bits I would definitely like to see cleaned up are
> the things like the mmio version of the 16550 UART code
> in hw/char/serial.c -- that not being QOMified has
> knock-on effects in making other devices that would
> like to basically just be 16550-wrappers harder to
> write in a clean way.





Re: [Qemu-devel] [PATCH for-3.1 3/4] slirp: Remove code that handles socreate() failure

2018-11-06 Thread Samuel Thibault
Peter Maydell, le mar. 06 nov. 2018 15:13:22 +, a ecrit:
> Now that socreate() can never fail, we can remove the code
> that was trying to handle that situation.
> 
> In particular this removes code in tcp_connect() that
> provoked Coverity to complain (CID 1005724): in
>  closesocket(accept(inso->s, (struct sockaddr *)&addr, &addrlen));
> if the accept() call fails then we pass closesocket() -1
> instead of a valid file descriptor.
> 
> Signed-off-by: Peter Maydell 

Applied, thanks!



Re: [Qemu-devel] [PULL 00/17] Misc patches for QEMU 3.1 hard freeze (?)

2018-11-06 Thread Peter Maydell
On 6 November 2018 at 21:37, Paolo Bonzini  wrote:
> The following changes since commit fc3d1bad1edf08871275cf469a64e12dae4eba96:
>
>   oslib-posix: Use MAP_STACK in qemu_alloc_stack() on OpenBSD (2018-11-06 
> 10:52:23 +)
>
> are available in the git repository at:
>
>   git://github.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to a458774ad711bceabefbf01e8f0b91d86ec72e0c:
>
>   util/qemu-thread-posix: Fix qemu_thread_atexit* for OSX (2018-11-06 
> 21:35:06 +0100)
>
> Vitaly's patch is a bit late; unfortunately I couldn't resend the pull
> request yesterday because of a long weekend.  On the other hand this
> meant I could include three more bugfix series, and also fix some nits
> pointed out by Laszlo who just came back from his own vacation.

Missed rc0 by four hours or so, but never mind -- it'll go into rc1.

thanks
-- PMM



Re: [Qemu-devel] [PATCH for-3.1 4/4] slirp: fork_exec(): create and connect child socket before fork()

2018-11-06 Thread Samuel Thibault
Peter Maydell, le mar. 06 nov. 2018 15:13:23 +, a ecrit:
> Currently fork_exec() fork()s, and then creates and connects the
> child socket which it uses for communication with the parent in
> the child process. This is awkward because the child has no
> mechanism to report failure back to the parent, which might end
> up blocked forever in accept(). The child code also has an issue
> pointed out by Coverity (CID 1005727), where if the qemu_socket()
> call fails it will pass -1 as a file descriptor to connect().
> 
> Fix these issues by moving the creation of the child's end of
> the socket to before the fork(), where we are in a position to
> handle a possible failure.
> 
> Signed-off-by: Peter Maydell 

Applied, thanks!

Samuel



Re: [Qemu-devel] [PATCH v2 2/6] add QemuSupportState

2018-11-06 Thread Paolo Bonzini
On 06/11/2018 15:26, Eduardo Habkost wrote:
> On Tue, Nov 06, 2018 at 11:23:31AM +0100, Gerd Hoffmann wrote:
>> Indicates support state for something (device, backend, subsystem, ...)
>> in qemu.  Add QemuSupportState field to ObjectClass.  Add some support
>> code.
>>
>> TODO: wire up to qom-list-types
>>
>> Signed-off-by: Gerd Hoffmann 
>> ---
> [...]
>> +##
>> +# @SupportState:
>> +#
>> +# Indicate Support level of qemu devices, backends, subsystems, ...
>> +#
>> +# @unspecified: not specified (zero-initialized).
>> +#
>> +# @experimental: in development, can be unstable or incomplete.
> 
> People reading this document would ask: what would appear on
> MAINTAINERS if SupportState is `experimental`?

Probably Maintained.  It's something that is on its way towards becoming
"supported", but still too immature ("unstable or incomplete").

>> +#
>> +# @supported: works stable and is fully supported.
>> +# (supported + maintained in MAINTAINERS).
>> +#
>> +# @unsupported: should work, support is weak or not present.
>> +#   (odd-fixes + orphan in MAINTAINERS).
> 
> What's the difference in practice between unsupported and
> experimental?
> 
>> +#
>> +# @obsolete: is obsolete, still present for compatibility reasons,
>> +#will likely be removed at some point in the future.

I am not sure this is necessarily true.  I don't see Cirrus or adlib or
pcnet disappearing anytime soon.

Paolo

>> +#Not deprecated (yet).
>> +#(obsolete in MAINTAINERS).
>> +#
>> +# @deprecated: is deprecated, according to qemu deprecation policy.
> 
> I believe we want to differentiate "deprecated, but still safe to
> use in production if you have a migration plan" from "deprecated,
> and also unstable and unsafe for production".
> 
> I expect enterprise distributions to have a strict policy of not
> allowing unsupported and experimental devices to be enabled, but
> still allow deprecated devices to be enabled (but only if they
> are stable/supported).
> 




Re: [Qemu-devel] [PATCH for-3.1 2/4] slirp: Use g_new() to allocate sockets in socreate()

2018-11-06 Thread Samuel Thibault
Peter Maydell, le mar. 06 nov. 2018 15:13:21 +, a ecrit:
> The slirp socreate() function can only fail if the attempt
> to malloc() the struct socket fails. Switch to using
> g_new() instead, which will allow us to remove the
> error-handling code from its callers.
> 
> Signed-off-by: Peter Maydell 
> ---
> We already use g_new/g_malloc in slirp, including for
> mbuf buffers which are larger than these socket structs.
> The motivation here is that we can render moot a Coverity
> complaint about an issue in an error-handling path.
> 
> As usual, indenting in slirp code is a bit of a mess;
> I've opted for "keep checkpatch happy".

Applied, thanks!

Samuel



Re: [Qemu-devel] List of files containing devices which have not been QOMified

2018-11-06 Thread Peter Maydell
On 6 November 2018 at 19:16, Philippe Mathieu-Daudé  wrote:
> On 6/11/18 19:43, Peter Maydell wrote:
>>
>> I had an idea for how to get a rough list of source files
>> containing devices that haven't been QOMified. The theory
>> is that a pre-QOM device generally has an "init" function
>> which allocates memory for the device struct. So looking in
>> hw/ for files which call g_new*() or g_malloc*() should get
>> us all the non-QOM devices (as well as a pile of false
>> positives, of course). The following link is the result of
>> doing that and then eyeballing the results for false positives
>> and throwing those out. It might have missed one or two
>> files or included one or two by mistake. But I think it's
>> pretty close, and it seems to have caught all the obvious
>> ones I knew about. There are 61 files on this list.
>>
>> I am also suspicious about hw/bt/ but don't know enough
>> about that subsystem to say if it could benefit from
>> using QOM objects more.
>>
>
>> hw/arm/exynos4210.c
>
> I already did this one.
>
>> hw/sd/omap_mmc.c
> I will do this one.

I have some out-of-tree stuff that deals with this device
(part of the omap3 patchset tries to do some QOMification,
but it was a bit tangled with adding omap3 features),
so if you could hold off on working on the various omap
devices in this set that might be better, til I see
whether any of the out-of-tree code is usefully salvageable.

thanks
-- PMM



Re: [Qemu-devel] List of files containing devices which have not been QOMified

2018-11-06 Thread Peter Maydell
On 6 November 2018 at 19:46, Paolo Bonzini  wrote:
> On 06/11/2018 19:43, Peter Maydell wrote:
>> hw/core/ptimer.c
>
> Not a device.

Indeed not, but it could be a QOM object I guess (would
that gain us anything?)

>> hw/i2c/bitbang_i2c.c
>
> TYPE_GPIO_I2C?

That part is, but bitbang_i2c_init() creates an object
which isn't a QOM object and is used by some other i2c devices.

>> hw/ide/ahci.c
>
> Even though AHCIState is not a QOM object, all of its users are
> (TYPE_SYSBUS_AHCI is in this file, TYPE_ICH9_AHCI is in hw/ide/ich.c)

Mmm, this is one of those which I was unsure about so
put on the list anyway.

Overall something that occurs to me is that I'm not sure
what exactly (other than tidiness) we gain from converting
remaining non-QOM devices. In some of the other cases I've
looked at (like sysbus init methods or old_mmio users) we
get to complete an API transition and remove the old code.
For a non-QOM device, how much does it hurt us that they're
lying around in the codebase? We might do better to
specifically target APIs we'd like to deal with (like
direct uses of vmstate_register, maybe?).

Some bits I would definitely like to see cleaned up are
the things like the mmio version of the 16550 UART code
in hw/char/serial.c -- that not being QOMified has
knock-on effects in making other devices that would
like to basically just be 16550-wrappers harder to
write in a clean way.

thanks
-- PMM



Re: [Qemu-devel] [PATCH for-3.1 1/4] slirp: Don't pass possibly -1 fd to send()

2018-11-06 Thread Samuel Thibault
Peter Maydell, le mar. 06 nov. 2018 15:13:20 +, a ecrit:
> Coverity complains (CID 1005726) that we might pass -1 as the fd
> argument to send() in slirp_send(), because we previously checked for
> "so->s == -1 && so->extra".  The case of "so->s == -1 but so->extra
> NULL" should not in theory happen, but it is hard to guarantee
> because various places in the code do so->s = qemu_socket(...) and so
> will end up with so->s == -1 on failure, and not all the paths which
> call that always throw away the socket in that case (eg
> tcp_fconnect()).  So just check specifically for the condition and
> fail slirp_send().
> 
> Signed-off-by: Peter Maydell 
> ---
> This is to some extent just placating Coverity.

Applied, thanks!



Re: [Qemu-devel] [Qemu-block] ping Re: [PATCH v4 00/11] backup-top filter driver for backup

2018-11-06 Thread John Snow



On 11/06/2018 12:21 PM, Kevin Wolf wrote:
> Am 02.11.2018 um 17:41 hat Vladimir Sementsov-Ogievskiy geschrieben:
>> ping
>>
>> 15.10.2018 19:06, Vladimir Sementsov-Ogievskiy wrote:
>>> Hi all!
>>>
>>> These series introduce backup-top driver. It's a filter-node, which
>>> do copy-before-write operation. Mirror uses filter-node for handling
>>> guest writes, let's move to filter-node (from write-notifiers) for
>>> backup too (patch 16)
>>>
>>> v4:
>>> fixes, rewrite driver to be implicit, drop new interfaces and
>>> don't move to BdrvDirtyBitmap for now, as it's not obvious will
>>> it be really needed and don't relate to these series more.
>>>
>>> v3 was "[PATCH v3 00/18] fleecing-hook driver for backup"
>>>
>>> v2 was "[RFC v2] new, node-graph-based fleecing and backup"
>>>
>>> These series are based on
>>>   [PATCH v4 0/8] dirty-bitmap: rewrite bdrv_dirty_iter_next_area
>>> and
>>>   [PATCH 0/2] replication: drop extra sync
> 
> Before we can move forward here, we need to deal with the dependencies.
> I merged the second one now (because there wasn't any feedback from the
> replication maintainers), but the first one looks like it was going
> through John's or Eric's tree?
> 

I was expecting it to go through Eric's because it was predicated on NBD
patches that he was staging.

Is that no longer true?
--js



Re: [Qemu-devel] [PATCH for-3.1] replay: Exit on errors reading from replay log

2018-11-06 Thread Paolo Bonzini
On 06/11/2018 16:33, Peter Maydell wrote:
> Currently replay_get_byte() does not check for an error
> from getc(). Coverity points out (CID 1390622) that this
> could result in unexpected behaviour (such as looping
> forever, if we use the replay_get_dword() return value
> for a loop count). We don't expect reads from the replay
> log to fail, and if they do there is no way we can
> continue. So make them fatal errors.
> 
> Signed-off-by: Peter Maydell 
> ---
> Disclaimer: checked only with "make check".
> 
>  replay/replay-internal.c | 16 +---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/replay/replay-internal.c b/replay/replay-internal.c
> index 1cea1d4dc91..8f87e9b957e 100644
> --- a/replay/replay-internal.c
> +++ b/replay/replay-internal.c
> @@ -35,6 +35,12 @@ static void replay_write_error(void)
>  }
>  }
>  
> +static void replay_read_error(void)
> +{
> +error_report("error reading the replay data");
> +exit(1);
> +}
> +
>  void replay_put_byte(uint8_t byte)
>  {
>  if (replay_file) {
> @@ -83,7 +89,11 @@ uint8_t replay_get_byte(void)
>  {
>  uint8_t byte = 0;
>  if (replay_file) {
> -byte = getc(replay_file);
> +int r = getc(replay_file);
> +if (r == EOF) {
> +replay_read_error();
> +}
> +byte = r;
>  }
>  return byte;
>  }
> @@ -126,7 +136,7 @@ void replay_get_array(uint8_t *buf, size_t *size)
>  if (replay_file) {
>  *size = replay_get_dword();
>  if (fread(buf, 1, *size, replay_file) != *size) {
> -error_report("replay read error");
> +replay_read_error();
>  }
>  }
>  }
> @@ -137,7 +147,7 @@ void replay_get_array_alloc(uint8_t **buf, size_t *size)
>  *size = replay_get_dword();
>  *buf = g_malloc(*size);
>  if (fread(*buf, 1, *size, replay_file) != *size) {
> -error_report("replay read error");
> +replay_read_error();
>  }
>  }
>  }
> 

Makes sense, can you apply it directly to qemu.git as soon as Pavel
reviews it (or in some time if he doesn't)?

Thanks,

Paolo



[Qemu-devel] [PULL 07/17] memory: learn about non-volatile memory region

2018-11-06 Thread Paolo Bonzini
From: Marc-André Lureau 

Add a new flag to mark memory region that are used as non-volatile, by
NVDIMM for example. That bit is propagated down to the flat view, and
reflected in HMP info mtree with a "nv-" prefix on the memory type.

This way, guest_phys_blocks_region_add() can skip the NV memory
regions for dumps and TCG memory clear in a following patch.

Cc: dgilb...@redhat.com
Cc: imamm...@redhat.com
Cc: pbonz...@redhat.com
Cc: guangrong.x...@linux.intel.com
Cc: m...@redhat.com
Cc: xiaoguangrong.e...@gmail.com
Signed-off-by: Marc-André Lureau 
Message-Id: <20181003114454.5662-2-marcandre.lur...@redhat.com>
Signed-off-by: Paolo Bonzini 
---
 docs/devel/migration.rst |  1 +
 include/exec/memory.h| 25 +
 memory.c | 45 +++--
 3 files changed, 61 insertions(+), 10 deletions(-)

diff --git a/docs/devel/migration.rst b/docs/devel/migration.rst
index 6875707..e7658ab 100644
--- a/docs/devel/migration.rst
+++ b/docs/devel/migration.rst
@@ -435,6 +435,7 @@ Examples of such memory API functions are:
   - memory_region_add_subregion()
   - memory_region_del_subregion()
   - memory_region_set_readonly()
+  - memory_region_set_nonvolatile()
   - memory_region_set_enabled()
   - memory_region_set_address()
   - memory_region_set_alias_offset()
diff --git a/include/exec/memory.h b/include/exec/memory.h
index d0c7f0d..8e61450 100644
--- a/include/exec/memory.h
+++ b/include/exec/memory.h
@@ -355,6 +355,7 @@ struct MemoryRegion {
 bool ram;
 bool subpage;
 bool readonly; /* For RAM regions */
+bool nonvolatile;
 bool rom_device;
 bool flush_coalesced_mmio;
 bool global_locking;
@@ -480,6 +481,7 @@ static inline FlatView 
*address_space_to_flatview(AddressSpace *as)
  * @offset_within_address_space: the address of the first byte of the section
  * relative to the region's address space
  * @readonly: writes to this section are ignored
+ * @nonvolatile: this section is non-volatile
  */
 struct MemoryRegionSection {
 MemoryRegion *mr;
@@ -488,6 +490,7 @@ struct MemoryRegionSection {
 Int128 size;
 hwaddr offset_within_address_space;
 bool readonly;
+bool nonvolatile;
 };
 
 /**
@@ -1170,6 +1173,17 @@ static inline bool memory_region_is_rom(MemoryRegion *mr)
 return mr->ram && mr->readonly;
 }
 
+/**
+ * memory_region_is_nonvolatile: check whether a memory region is non-volatile
+ *
+ * Returns %true is a memory region is non-volatile memory.
+ *
+ * @mr: the memory region being queried
+ */
+static inline bool memory_region_is_nonvolatile(MemoryRegion *mr)
+{
+return mr->nonvolatile;
+}
 
 /**
  * memory_region_get_fd: Get a file descriptor backing a RAM memory region.
@@ -1342,6 +1356,17 @@ void memory_region_reset_dirty(MemoryRegion *mr, hwaddr 
addr,
 void memory_region_set_readonly(MemoryRegion *mr, bool readonly);
 
 /**
+ * memory_region_set_nonvolatile: Turn a memory region non-volatile
+ *
+ * Allows a memory region to be marked as non-volatile.
+ * only useful on RAM regions.
+ *
+ * @mr: the region being updated.
+ * @nonvolatile: whether rhe region is to be non-volatile.
+ */
+void memory_region_set_nonvolatile(MemoryRegion *mr, bool nonvolatile);
+
+/**
  * memory_region_rom_device_set_romd: enable/disable ROMD mode
  *
  * Allows a ROM device (initialized with memory_region_init_rom_device() to
diff --git a/memory.c b/memory.c
index 51204aa..d14c6de 100644
--- a/memory.c
+++ b/memory.c
@@ -216,6 +216,7 @@ struct FlatRange {
 uint8_t dirty_log_mask;
 bool romd_mode;
 bool readonly;
+bool nonvolatile;
 };
 
 #define FOR_EACH_FLAT_RANGE(var, view)  \
@@ -231,6 +232,7 @@ section_from_flat_range(FlatRange *fr, FlatView *fv)
 .size = fr->addr.size,
 .offset_within_address_space = int128_get64(fr->addr.start),
 .readonly = fr->readonly,
+.nonvolatile = fr->nonvolatile,
 };
 }
 
@@ -240,7 +242,8 @@ static bool flatrange_equal(FlatRange *a, FlatRange *b)
 && addrrange_equal(a->addr, b->addr)
 && a->offset_in_region == b->offset_in_region
 && a->romd_mode == b->romd_mode
-&& a->readonly == b->readonly;
+&& a->readonly == b->readonly
+&& a->nonvolatile == b->nonvolatile;
 }
 
 static FlatView *flatview_new(MemoryRegion *mr_root)
@@ -312,7 +315,8 @@ static bool can_merge(FlatRange *r1, FlatRange *r2)
  int128_make64(r2->offset_in_region))
 && r1->dirty_log_mask == r2->dirty_log_mask
 && r1->romd_mode == r2->romd_mode
-&& r1->readonly == r2->readonly;
+&& r1->readonly == r2->readonly
+&& r1->nonvolatile == r2->nonvolatile;
 }
 
 /* Attempt to simplify a view by merging adjacent ranges */
@@ -592,7 +596,8 @@ static void render_memory_region(FlatView *view,
  MemoryRegion *mr,
  Int128 base,
  AddrRange clip,
-   

[Qemu-devel] [PULL 08/17] nvdimm: set non-volatile on the memory region

2018-11-06 Thread Paolo Bonzini
From: Marc-André Lureau 

qemu-system-x86_64 -machine pc,nvdimm -m 2G,slots=4,maxmem=16G -enable-kvm 
-monitor stdio -object 
memory-backend-file,id=mem1,share=on,mem-path=/tmp/foo,size=1G -device 
nvdimm,id=nvdimm1,memdev=mem1

HMP info mtree command reflects the flag with "nv-" prefix on memory type:

(qemu) info mtree
0001-00013fff (prio 0, nv-i/o): alias nvdimm-memory 
@/objects/mem1 -3fff

(qemu) info mtree -f
0001-00013fff (prio 0, nv-ram): /objects/mem1

Signed-off-by: Marc-André Lureau 
Message-Id: <20181003114454.5662-3-marcandre.lur...@redhat.com>
Signed-off-by: Paolo Bonzini 
---
 hw/mem/nvdimm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/mem/nvdimm.c b/hw/mem/nvdimm.c
index 49324f3..bf2adf5 100644
--- a/hw/mem/nvdimm.c
+++ b/hw/mem/nvdimm.c
@@ -116,6 +116,7 @@ static void nvdimm_prepare_memory_region(NVDIMMDevice 
*nvdimm, Error **errp)
 nvdimm->nvdimm_mr = g_new(MemoryRegion, 1);
 memory_region_init_alias(nvdimm->nvdimm_mr, OBJECT(dimm),
  "nvdimm-memory", mr, 0, pmem_size);
+memory_region_set_nonvolatile(nvdimm->nvdimm_mr, true);
 nvdimm->nvdimm_mr->align = align;
 }
 
-- 
1.8.3.1





Re: [Qemu-devel] [PATCH v4 00/16] gdbstub: support for the multiprocess extension

2018-11-06 Thread no-reply
Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 20181106110548.4209-1-luc.mic...@greensocs.com
Subject: [Qemu-devel] [PATCH v4 00/16] gdbstub: support for the multiprocess 
extension

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
5b37d36270 arm/xlnx-zynqmp: put APUs and RPUs in separate CPU clusters
9cd6ba6c0c gdbstub: add multiprocess extension support
93d14b3254 gdbstub: gdb_set_stop_cpu: ignore request when process is not 
attached
22126cda18 gdbstub: processes initialization on new peer connection
931e6cce36 gdbstub: add support for vAttach packets
6cafd391d1 gdbstub: add support for extended mode packet
de385a573d gdbstub: add multiprocess support to 'D' packets
12491e88da gdbstub: add multiprocess support to gdb_vm_state_change()
1e8b86e03a gdbstub: add multiprocess support to Xfer:features:read:
e37f5c9f3c gdbstub: add multiprocess support to (f|s)ThreadInfo and 
ThreadExtraInfo
344eb3e2f1 gdbstub: add multiprocess support to 'sC' packets
ff95bcac11 gdbstub: add multiprocess support to vCont packets
5320b221a3 gdbstub: add multiprocess support to 'H' and 'T' packets
19980f2997 gdbstub: add multiprocess support to '?' packets
dc64a6c997 gdbstub: introduce GDB processes
2e2a60cc51 hw/cpu: introduce CPU clusters

=== OUTPUT BEGIN ===
Checking PATCH 1/16: hw/cpu: introduce CPU clusters...
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#26: 
new file mode 100644

ERROR: do not initialise statics to 0 or NULL
#58: FILE: hw/cpu/cluster.c:28:
+static uint64_t cluster_id_auto_increment = 0;

total: 1 errors, 1 warnings, 102 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 2/16: gdbstub: introduce GDB processes...
Checking PATCH 3/16: gdbstub: add multiprocess support to '?' packets...
Checking PATCH 4/16: gdbstub: add multiprocess support to 'H' and 'T' packets...
Checking PATCH 5/16: gdbstub: add multiprocess support to vCont packets...
Checking PATCH 6/16: gdbstub: add multiprocess support to 'sC' packets...
Checking PATCH 7/16: gdbstub: add multiprocess support to (f|s)ThreadInfo and 
ThreadExtraInfo...
Checking PATCH 8/16: gdbstub: add multiprocess support to Xfer:features:read:...
Checking PATCH 9/16: gdbstub: add multiprocess support to 
gdb_vm_state_change()...
Checking PATCH 10/16: gdbstub: add multiprocess support to 'D' packets...
Checking PATCH 11/16: gdbstub: add support for extended mode packet...
Checking PATCH 12/16: gdbstub: add support for vAttach packets...
Checking PATCH 13/16: gdbstub: processes initialization on new peer 
connection...
Checking PATCH 14/16: gdbstub: gdb_set_stop_cpu: ignore request when process is 
not attached...
Checking PATCH 15/16: gdbstub: add multiprocess extension support...
Checking PATCH 16/16: arm/xlnx-zynqmp: put APUs and RPUs in separate CPU 
clusters...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

[Qemu-devel] [PULL 04/17] ivshmem: fix memory backend leak

2018-11-06 Thread Paolo Bonzini
From: Igor Mammedov 

object_new() returns a new backend with refcount == 1 and
then later object_property_add_child() increases refcount to 2
So when ivshmem is destroyed, the backend it has created isn't
destroyed along with it as children cleanup will bring
backend's refcount only to 1, which leaks backend including
resources it is using.

Drop the original reference from object_new() once backend
is attached to its parent.

Signed-off-by: Igor Mammedov 
Message-Id: <1541069086-167036-1-git-send-email-imamm...@redhat.com>
Reviewed-by: Marc-André Lureau 
Fixes: 5503e285041979dd29698ecb41729b3b22622e8d
Signed-off-by: Paolo Bonzini 
---
 hw/misc/ivshmem.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index f88910e..ecfd10a 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -1279,6 +1279,7 @@ static void desugar_shm(IVShmemState *s)
 object_property_set_bool(obj, true, "share", &error_abort);
 object_property_add_child(OBJECT(s), "internal-shm-backend", obj,
   &error_abort);
+object_unref(obj);
 user_creatable_complete(obj, &error_abort);
 s->hostmem = MEMORY_BACKEND(obj);
 }
-- 
1.8.3.1





[Qemu-devel] [PULL 15/17] scsi-generic: do not do VPD emulation for sense other than ILLEGAL_REQUEST

2018-11-06 Thread Paolo Bonzini
Pass other sense, such as UNIT_ATTENTION or BUSY, directly to the
guest.

Reported-by: Max Reitz 
Signed-off-by: Paolo Bonzini 
---
 hw/scsi/scsi-generic.c | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index b50ce64..7237b41 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -246,7 +246,6 @@ static void scsi_read_complete(void * opaque, int ret)
 {
 SCSIGenericReq *r = (SCSIGenericReq *)opaque;
 SCSIDevice *s = r->req.dev;
-SCSISense sense;
 int len;
 
 assert(r->req.aiocb != NULL);
@@ -269,11 +268,14 @@ static void scsi_read_complete(void * opaque, int ret)
  * resulted in sense error but would need emulation.
  * In this case, emulate a valid VPD response.
  */
-if (s->needs_vpd_bl_emulation &&
+if (s->needs_vpd_bl_emulation && ret == 0 &&
+(r->io_header.driver_status & SG_ERR_DRIVER_SENSE) &&
 r->req.cmd.buf[0] == INQUIRY &&
 (r->req.cmd.buf[1] & 0x01) &&
 r->req.cmd.buf[2] == 0xb0) {
-if (sg_io_sense_from_errno(-ret, &r->io_header, &sense)) {
+SCSISense sense =
+scsi_parse_sense_buf(r->req.sense, r->io_header.sb_len_wr);
+if (sense.key == ILLEGAL_REQUEST) {
 len = scsi_generic_emulate_block_limits(r, s);
 /*
  * No need to let scsi_read_complete go on and handle an
-- 
1.8.3.1





[Qemu-devel] [PULL 10/17] scripts/dump-guest-memory: Synchronize with guest_phys_blocks_region_add

2018-11-06 Thread Paolo Bonzini
Recent patches have removed ram_device and nonvolatile RAM
from dump-guest-memory's output.  Do the same for dumps
that are extracted from a QEMU core file.

Reviewed-by: Marc-André Lureau 
Signed-off-by: Paolo Bonzini 
---
 scripts/dump-guest-memory.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py
index 5a857ce..198cd0f 100644
--- a/scripts/dump-guest-memory.py
+++ b/scripts/dump-guest-memory.py
@@ -417,7 +417,9 @@ def get_guest_phys_blocks():
 memory_region = flat_range["mr"].dereference()
 
 # we only care about RAM
-if not memory_region["ram"]:
+if (not memory_region["ram"] or
+memory_region["ram_device"] or
+memory_region["nonvolatile"]):
 continue
 
 section_size = int128_get64(flat_range["addr"]["size"])
-- 
1.8.3.1





[Qemu-devel] [PULL 01/17] icount: fix deadlock when all cpus are sleeping

2018-11-06 Thread Paolo Bonzini
From: Clement Deschamps 

When all cpus are sleeping (e.g in WFI), to avoid a deadlock
in the main_loop, wake it up in order to start the warp timer.

Signed-off-by: Clement Deschamps 
Message-Id: <20181021142103.19014-1-clement.descha...@greensocs.com>
Signed-off-by: Paolo Bonzini 
---
 cpus.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/cpus.c b/cpus.c
index 3978f63..a2b33cc 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1554,6 +1554,14 @@ static void *qemu_tcg_rr_cpu_thread_fn(void *arg)
 atomic_mb_set(&cpu->exit_request, 0);
 }
 
+if (use_icount && all_cpu_threads_idle()) {
+/*
+ * When all cpus are sleeping (e.g in WFI), to avoid a deadlock
+ * in the main_loop, wake it up in order to start the warp timer.
+ */
+qemu_notify_event();
+}
+
 qemu_tcg_rr_wait_io_event(cpu ? cpu : first_cpu);
 deal_with_unplugged_cpus();
 }
-- 
1.8.3.1





[Qemu-devel] [PULL 09/17] memory-mapping: skip non-volatile memory regions in GuestPhysBlockList

2018-11-06 Thread Paolo Bonzini
From: Marc-André Lureau 

GuestPhysBlockList is currently used to produce dumps. Given the size
and the typical usage of NVDIMM for storage, they are not a good idea
to have in the dumps. We may want to have an extra dump option to
include them. For now, skip non-volatile regions.

The TCG memory clear function is going to use the GuestPhysBlockList
as well, and will thus skip NVDIMM for similar reasons.

Signed-off-by: Marc-André Lureau 
Message-Id: <20181003114454.5662-4-marcandre.lur...@redhat.com>
Reviewed-by: Laszlo Ersek 
Reviewed-by: David Hildenbrand 
Signed-off-by: Paolo Bonzini 
---
 memory_mapping.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/memory_mapping.c b/memory_mapping.c
index 775466f..724dd0b 100644
--- a/memory_mapping.c
+++ b/memory_mapping.c
@@ -206,7 +206,8 @@ static void guest_phys_blocks_region_add(MemoryListener 
*listener,
 
 /* we only care about RAM */
 if (!memory_region_is_ram(section->mr) ||
-memory_region_is_ram_device(section->mr)) {
+memory_region_is_ram_device(section->mr) ||
+memory_region_is_nonvolatile(section->mr)) {
 return;
 }
 
-- 
1.8.3.1





[Qemu-devel] [PULL 17/17] util/qemu-thread-posix: Fix qemu_thread_atexit* for OSX

2018-11-06 Thread Paolo Bonzini
From: Peter Maydell 

Our current implementation of qemu_thread_atexit* is broken on OSX.
This is because it works by cerating a piece of thread-specific
data with pthread_key_create() and using the destructor function
for that data to run the notifier function passed to it by
the caller of qemu_thread_atexit_add(). The expected use case
is that the caller uses a __thread variable as the notifier,
and uses the callback to clean up information that it is
keeping per-thread in __thread variables.

Unfortunately, on OSX this does not work, because on OSX
a __thread variable may be destroyed (freed) before the
pthread_key_create() destructor runs. (POSIX imposes no
ordering constraint here; the OSX implementation happens
to implement __thread variables in terms of pthread_key_create((),
whereas Linux uses different mechanisms that mean the __thread
variables will still be present when the pthread_key_create()
destructor is run.)

Fix this by switching to a scheme similar to the one qemu-thread-win32
uses for qemu_thread_atexit: keep the thread's notifiers on a
__thread variable, and run the notifiers on calls to
qemu_thread_exit() and on return from the start routine passed
to qemu_thread_start(). We do this with the pthread_cleanup_push()
API.

We take advantage of the qemu_thread_atexit_add() API
permission not to run thread notifiers on process exit to
avoid having to special case the main thread.

Suggested-by: Paolo Bonzini 
Signed-off-by: Peter Maydell 
Reviewed-by: Eric Blake 
Message-Id: <20181105135538.28025-3-peter.mayd...@linaro.org>
Signed-off-by: Paolo Bonzini 
---
 util/qemu-thread-posix.c | 44 
 1 file changed, 20 insertions(+), 24 deletions(-)

diff --git a/util/qemu-thread-posix.c b/util/qemu-thread-posix.c
index dfa66ff..865e476 100644
--- a/util/qemu-thread-posix.c
+++ b/util/qemu-thread-posix.c
@@ -443,42 +443,34 @@ void qemu_event_wait(QemuEvent *ev)
 }
 }
 
-static pthread_key_t exit_key;
-
-union NotifierThreadData {
-void *ptr;
-NotifierList list;
-};
-QEMU_BUILD_BUG_ON(sizeof(union NotifierThreadData) != sizeof(void *));
+static __thread NotifierList thread_exit;
 
+/*
+ * Note that in this implementation you can register a thread-exit
+ * notifier for the main thread, but it will never be called.
+ * This is OK because main thread exit can only happen when the
+ * entire process is exiting, and the API allows notifiers to not
+ * be called on process exit.
+ */
 void qemu_thread_atexit_add(Notifier *notifier)
 {
-union NotifierThreadData ntd;
-ntd.ptr = pthread_getspecific(exit_key);
-notifier_list_add(&ntd.list, notifier);
-pthread_setspecific(exit_key, ntd.ptr);
+notifier_list_add(&thread_exit, notifier);
 }
 
 void qemu_thread_atexit_remove(Notifier *notifier)
 {
-union NotifierThreadData ntd;
-ntd.ptr = pthread_getspecific(exit_key);
 notifier_remove(notifier);
-pthread_setspecific(exit_key, ntd.ptr);
-}
-
-static void qemu_thread_atexit_run(void *arg)
-{
-union NotifierThreadData ntd = { .ptr = arg };
-notifier_list_notify(&ntd.list, NULL);
 }
 
-static void __attribute__((constructor)) qemu_thread_atexit_init(void)
+static void qemu_thread_atexit_notify(void *arg)
 {
-pthread_key_create(&exit_key, qemu_thread_atexit_run);
+/*
+ * Called when non-main thread exits (via qemu_thread_exit()
+ * or by returning from its start routine.)
+ */
+notifier_list_notify(&thread_exit, NULL);
 }
 
-
 typedef struct {
 void *(*start_routine)(void *);
 void *arg;
@@ -490,6 +482,7 @@ static void *qemu_thread_start(void *args)
 QemuThreadArgs *qemu_thread_args = args;
 void *(*start_routine)(void *) = qemu_thread_args->start_routine;
 void *arg = qemu_thread_args->arg;
+void *r;
 
 #ifdef CONFIG_PTHREAD_SETNAME_NP
 /* Attempt to set the threads name; note that this is for debug, so
@@ -501,7 +494,10 @@ static void *qemu_thread_start(void *args)
 #endif
 g_free(qemu_thread_args->name);
 g_free(qemu_thread_args);
-return start_routine(arg);
+pthread_cleanup_push(qemu_thread_atexit_notify, NULL);
+r = start_routine(arg);
+pthread_cleanup_pop(1);
+return r;
 }
 
 void qemu_thread_create(QemuThread *thread, const char *name,
-- 
1.8.3.1




[Qemu-devel] [PULL 11/17] lsi53c895a: check message length value is valid

2018-11-06 Thread Paolo Bonzini
From: Prasad J Pandit 

While writing a message in 'lsi_do_msgin', message length value
in 'msg_len' could be invalid due to an invalid migration stream.
Add an assertion to avoid an out of bounds access, and reject
the incoming migration data if it contains an invalid message
length.

Discovered by Deja vu Security. Reported by Oracle.

Signed-off-by: Prasad J Pandit 
Message-Id: <20181026194314.18663-1-ppan...@redhat.com>
Signed-off-by: Paolo Bonzini 
---
 hw/scsi/lsi53c895a.c | 19 +--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/hw/scsi/lsi53c895a.c b/hw/scsi/lsi53c895a.c
index d1e6534..3f207f6 100644
--- a/hw/scsi/lsi53c895a.c
+++ b/hw/scsi/lsi53c895a.c
@@ -861,10 +861,11 @@ static void lsi_do_status(LSIState *s)
 
 static void lsi_do_msgin(LSIState *s)
 {
-int len;
+uint8_t len;
 trace_lsi_do_msgin(s->dbc, s->msg_len);
 s->sfbr = s->msg[0];
 len = s->msg_len;
+assert(len > 0 && len <= LSI_MAX_MSGIN_LEN);
 if (len > s->dbc)
 len = s->dbc;
 pci_dma_write(PCI_DEVICE(s), s->dnad, s->msg, len);
@@ -1705,8 +1706,10 @@ static uint8_t lsi_reg_readb(LSIState *s, int offset)
 break;
 case 0x58: /* SBDL */
 /* Some drivers peek at the data bus during the MSG IN phase.  */
-if ((s->sstat1 & PHASE_MASK) == PHASE_MI)
+if ((s->sstat1 & PHASE_MASK) == PHASE_MI) {
+assert(s->msg_len > 0);
 return s->msg[0];
+}
 ret = 0;
 break;
 case 0x59: /* SBDL high */
@@ -2103,11 +2106,23 @@ static int lsi_pre_save(void *opaque)
 return 0;
 }
 
+static int lsi_post_load(void *opaque, int version_id)
+{
+LSIState *s = opaque;
+
+if (s->msg_len < 0 || s->msg_len > LSI_MAX_MSGIN_LEN) {
+return -EINVAL;
+}
+
+return 0;
+}
+
 static const VMStateDescription vmstate_lsi_scsi = {
 .name = "lsiscsi",
 .version_id = 0,
 .minimum_version_id = 0,
 .pre_save = lsi_pre_save,
+.post_load = lsi_post_load,
 .fields = (VMStateField[]) {
 VMSTATE_PCI_DEVICE(parent_obj, LSIState),
 
-- 
1.8.3.1





[Qemu-devel] [PULL 16/17] include/qemu/thread.h: Document qemu_thread_atexit* API

2018-11-06 Thread Paolo Bonzini
From: Peter Maydell 

Add documentation for the qemu_thread_atexit_add() and
qemu_thread_atexit_remove() functions.

We include a (previously undocumented) constraint that notifiers
may not be called if a thread is exiting because the entire
process is exiting. This is fine for our current use because
the callers use it only for cleaning up resources which go away
on process exit (memory, Win32 fibers), and we will need the
flexibility for the new posix implementation.

Signed-off-by: Peter Maydell 
Reviewed-by: Eric Blake 
Message-Id: <20181105135538.28025-2-peter.mayd...@linaro.org>
Signed-off-by: Paolo Bonzini 
---
 include/qemu/thread.h | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/include/qemu/thread.h b/include/qemu/thread.h
index b2661b6..55d83a9 100644
--- a/include/qemu/thread.h
+++ b/include/qemu/thread.h
@@ -162,7 +162,29 @@ void qemu_thread_exit(void *retval);
 void qemu_thread_naming(bool enable);
 
 struct Notifier;
+/**
+ * qemu_thread_atexit_add:
+ * @notifier: Notifier to add
+ *
+ * Add the specified notifier to a list which will be run via
+ * notifier_list_notify() when this thread exits (either by calling
+ * qemu_thread_exit() or by returning from its start_routine).
+ * The usual usage is that the caller passes a Notifier which is
+ * a per-thread variable; it can then use the callback to free
+ * other per-thread data.
+ *
+ * If the thread exits as part of the entire process exiting,
+ * it is unspecified whether notifiers are called or not.
+ */
 void qemu_thread_atexit_add(struct Notifier *notifier);
+/**
+ * qemu_thread_atexit_remove:
+ * @notifier: Notifier to remove
+ *
+ * Remove the specified notifier from the thread-exit notification
+ * list. It is not valid to try to remove a notifier which is not
+ * on the list.
+ */
 void qemu_thread_atexit_remove(struct Notifier *notifier);
 
 struct QemuSpin {
-- 
1.8.3.1





[Qemu-devel] [PULL 02/17] x86: hv_evmcs CPU flag support

2018-11-06 Thread Paolo Bonzini
From: Vitaly Kuznetsov 

Adds a new CPU flag to enable the Enlightened VMCS KVM feature.
QEMU enables KVM_CAP_HYPERV_ENLIGHTENED_VMCS and gets back the
version to be advertised in lower 16 bits of CPUID.0x400A:EAX.

Suggested-by: Ladi Prosek 
Signed-off-by: Vitaly Kuznetsov 
Message-Id: <20181022165506.30332-3-vkuzn...@redhat.com>
Reviewed-by: Roman Kagan 
Signed-off-by: Paolo Bonzini 
---
 target/i386/cpu.c  |  1 +
 target/i386/cpu.h  |  1 +
 target/i386/hyperv-proto.h |  2 ++
 target/i386/kvm.c  | 30 --
 4 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index af7e9f0..f81d35e 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5732,6 +5732,7 @@ static Property x86_cpu_properties[] = {
 DEFINE_PROP_BOOL("hv-frequencies", X86CPU, hyperv_frequencies, false),
 DEFINE_PROP_BOOL("hv-reenlightenment", X86CPU, hyperv_reenlightenment, 
false),
 DEFINE_PROP_BOOL("hv-tlbflush", X86CPU, hyperv_tlbflush, false),
+DEFINE_PROP_BOOL("hv-evmcs", X86CPU, hyperv_evmcs, false),
 DEFINE_PROP_BOOL("hv-ipi", X86CPU, hyperv_ipi, false),
 DEFINE_PROP_BOOL("check", X86CPU, check_cpuid, true),
 DEFINE_PROP_BOOL("enforce", X86CPU, enforce_cpuid, false),
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index ad0e0b4..9c52d0c 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1391,6 +1391,7 @@ struct X86CPU {
 bool hyperv_frequencies;
 bool hyperv_reenlightenment;
 bool hyperv_tlbflush;
+bool hyperv_evmcs;
 bool hyperv_ipi;
 bool check_cpuid;
 bool enforce_cpuid;
diff --git a/target/i386/hyperv-proto.h b/target/i386/hyperv-proto.h
index 8c572cd..c0272b3 100644
--- a/target/i386/hyperv-proto.h
+++ b/target/i386/hyperv-proto.h
@@ -18,6 +18,7 @@
 #define HV_CPUID_FEATURES 0x4003
 #define HV_CPUID_ENLIGHTMENT_INFO 0x4004
 #define HV_CPUID_IMPLEMENT_LIMITS 0x4005
+#define HV_CPUID_NESTED_FEATURES  0x400A
 #define HV_CPUID_MIN  0x4005
 #define HV_CPUID_MAX  0x4000
 #define HV_HYPERVISOR_PRESENT_BIT 0x8000
@@ -60,6 +61,7 @@
 #define HV_RELAXED_TIMING_RECOMMENDED   (1u << 5)
 #define HV_CLUSTER_IPI_RECOMMENDED  (1u << 10)
 #define HV_EX_PROCESSOR_MASKS_RECOMMENDED   (1u << 11)
+#define HV_ENLIGHTENED_VMCS_RECOMMENDED (1u << 14)
 
 /*
  * Basic virtualized MSRs
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 796a049..f524e7d 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -869,6 +869,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
 uint32_t unused;
 struct kvm_cpuid_entry2 *c;
 uint32_t signature[3];
+uint16_t evmcs_version;
 int kvm_base = KVM_CPUID_SIGNATURE;
 int r;
 Error *local_err = NULL;
@@ -912,7 +913,8 @@ int kvm_arch_init_vcpu(CPUState *cs)
 memset(signature, 0, 12);
 memcpy(signature, cpu->hyperv_vendor_id, len);
 }
-c->eax = HV_CPUID_MIN;
+c->eax = cpu->hyperv_evmcs ?
+HV_CPUID_NESTED_FEATURES : HV_CPUID_IMPLEMENT_LIMITS;
 c->ebx = signature[0];
 c->ecx = signature[1];
 c->edx = signature[2];
@@ -970,7 +972,16 @@ int kvm_arch_init_vcpu(CPUState *cs)
 c->eax |= HV_CLUSTER_IPI_RECOMMENDED;
 c->eax |= HV_EX_PROCESSOR_MASKS_RECOMMENDED;
 }
-
+if (cpu->hyperv_evmcs) {
+if (kvm_vcpu_enable_cap(cs, KVM_CAP_HYPERV_ENLIGHTENED_VMCS, 0,
+(uintptr_t)&evmcs_version)) {
+fprintf(stderr, "Hyper-V Enlightened VMCS "
+"(requested by 'hv-evmcs' cpu flag) "
+"is not supported by kernel\n");
+return -ENOSYS;
+}
+c->eax |= HV_ENLIGHTENED_VMCS_RECOMMENDED;
+}
 c->ebx = cpu->hyperv_spinlock_attempts;
 
 c = &cpuid_data.entries[cpuid_i++];
@@ -981,6 +992,21 @@ int kvm_arch_init_vcpu(CPUState *cs)
 
 kvm_base = KVM_CPUID_SIGNATURE_NEXT;
 has_msr_hv_hypercall = true;
+
+if (cpu->hyperv_evmcs) {
+__u32 function;
+
+/* Create zeroed 0x4006..0x4009 leaves */
+for (function = HV_CPUID_IMPLEMENT_LIMITS + 1;
+ function < HV_CPUID_NESTED_FEATURES; function++) {
+c = &cpuid_data.entries[cpuid_i++];
+c->function = function;
+}
+
+c = &cpuid_data.entries[cpuid_i++];
+c->function = HV_CPUID_NESTED_FEATURES;
+c->eax = evmcs_version;
+}
 }
 
 if (cpu->expose_kvm) {
-- 
1.8.3.1





[Qemu-devel] [PULL 06/17] target/i386: Clear RF on SYSCALL instruction

2018-11-06 Thread Paolo Bonzini
From: Rudolf Marek 

Fix the SYSCALL instruction in 64-bit (long mode). The RF flag
should be cleared in R11 as well as in the RFLAGS. Intel
and AMD CPUs behave same. AMD has this documented in the
APM vol 3.

Signed-off-by: Roman Kapl 
Signed-off-by: Rudolf Marek 
Message-Id: <20181019122449.26387-1-...@sysgo.com>
Signed-off-by: Paolo Bonzini 
---
 target/i386/seg_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/i386/seg_helper.c b/target/i386/seg_helper.c
index 33714bc..63e265c 100644
--- a/target/i386/seg_helper.c
+++ b/target/i386/seg_helper.c
@@ -991,11 +991,11 @@ void helper_syscall(CPUX86State *env, int next_eip_addend)
 int code64;
 
 env->regs[R_ECX] = env->eip + next_eip_addend;
-env->regs[11] = cpu_compute_eflags(env);
+env->regs[11] = cpu_compute_eflags(env) & ~RF_MASK;
 
 code64 = env->hflags & HF_CS64_MASK;
 
-env->eflags &= ~env->fmask;
+env->eflags &= ~(env->fmask | RF_MASK);
 cpu_load_eflags(env, env->eflags, 0);
 cpu_x86_load_seg_cache(env, R_CS, selector & 0xfffc,
0, 0x,
-- 
1.8.3.1





[Qemu-devel] [PULL 13/17] scsi-generic: avoid out-of-bounds access to VPD page list

2018-11-06 Thread Paolo Bonzini
A device can report an excessive number of VPD pages when asked for a
list; this can cause an out-of-bounds access to buf in
scsi_generic_set_vpd_bl_emulation.  It should not happen, but
it is technically not incorrect so handle it: do not check any byte
past the allocation length that was sent to the INQUIRY command.

Reported-by: Max Reitz 
Reviewed-by: Max Reitz 
Signed-off-by: Paolo Bonzini 
---
 hw/scsi/scsi-generic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index aebb7cd..c5497bb 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -538,7 +538,7 @@ static void scsi_generic_set_vpd_bl_emulation(SCSIDevice *s)
 }
 
 page_len = buf[3];
-for (i = 4; i < page_len + 4; i++) {
+for (i = 4; i < MIN(sizeof(buf), page_len + 4); i++) {
 if (buf[i] == 0xb0) {
 s->needs_vpd_bl_emulation = false;
 return;
-- 
1.8.3.1





[Qemu-devel] [PULL 00/17] Misc patches for QEMU 3.1 hard freeze (?)

2018-11-06 Thread Paolo Bonzini
The following changes since commit fc3d1bad1edf08871275cf469a64e12dae4eba96:

  oslib-posix: Use MAP_STACK in qemu_alloc_stack() on OpenBSD (2018-11-06 
10:52:23 +)

are available in the git repository at:

  git://github.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to a458774ad711bceabefbf01e8f0b91d86ec72e0c:

  util/qemu-thread-posix: Fix qemu_thread_atexit* for OSX (2018-11-06 21:35:06 
+0100)

Vitaly's patch is a bit late; unfortunately I couldn't resend the pull
request yesterday because of a long weekend.  On the other hand this
meant I could include three more bugfix series, and also fix some nits
pointed out by Laszlo who just came back from his own vacation.


* icount fix (Clement)
* dumping fixes for non-volatile memory (Marc-André, myself)
* x86 emulation fix (Rudolf)
* recent Hyper-V CPUID flag (Vitaly)
* Q35 doc fix (Daniel)
* lsi fix (Prasad)
* SCSI block limits emulation fixes (myself)
* qemu_thread_atexit rework (Peter)
* ivshmem memory leak fix (Igor)


Clement Deschamps (1):
  icount: fix deadlock when all cpus are sleeping

Daniel P. Berrangé (1):
  i386: clarify that the Q35 machine type implements a P35 chipset

Igor Mammedov (1):
  ivshmem: fix memory backend leak

Marc-André Lureau (3):
  memory: learn about non-volatile memory region
  nvdimm: set non-volatile on the memory region
  memory-mapping: skip non-volatile memory regions in GuestPhysBlockList

Paolo Bonzini (6):
  MAINTAINERS: remove or downgrade myself to reviewer from some subsystems
  scripts/dump-guest-memory: Synchronize with guest_phys_blocks_region_add
  scsi-generic: keep VPD page list sorted
  scsi-generic: avoid out-of-bounds access to VPD page list
  scsi-generic: avoid invalid access to struct when emulating block limits
  scsi-generic: do not do VPD emulation for sense other than ILLEGAL_REQUEST

Peter Maydell (2):
  include/qemu/thread.h: Document qemu_thread_atexit* API
  util/qemu-thread-posix: Fix qemu_thread_atexit* for OSX

Prasad J Pandit (1):
  lsi53c895a: check message length value is valid

Rudolf Marek (1):
  target/i386: Clear RF on SYSCALL instruction

Vitaly Kuznetsov (1):
  x86: hv_evmcs CPU flag support

 MAINTAINERS  | 13 +++
 cpus.c   |  8 
 docs/devel/migration.rst |  1 +
 hw/mem/nvdimm.c  |  1 +
 hw/misc/ivshmem.c|  1 +
 hw/pci-host/q35.c| 10 -
 hw/scsi/Makefile.objs|  2 +-
 hw/scsi/emulation.c  | 42 
 hw/scsi/lsi53c895a.c | 19 -
 hw/scsi/scsi-disk.c  | 92 ++--
 hw/scsi/scsi-generic.c   | 60 +
 include/exec/memory.h| 25 
 include/hw/pci/pci_ids.h |  2 +-
 include/hw/scsi/emulation.h  | 16 
 include/hw/scsi/scsi.h   |  1 -
 include/qemu/thread.h| 22 +++
 memory.c | 45 +-
 memory_mapping.c |  3 +-
 scripts/dump-guest-memory.py |  4 +-
 target/i386/cpu.c|  1 +
 target/i386/cpu.h|  1 +
 target/i386/hyperv-proto.h   |  2 +
 target/i386/kvm.c| 30 ++-
 target/i386/seg_helper.c |  4 +-
 util/qemu-thread-posix.c | 44 ++---
 25 files changed, 308 insertions(+), 141 deletions(-)
 create mode 100644 hw/scsi/emulation.c
 create mode 100644 include/hw/scsi/emulation.h
-- 
1.8.3.1




[Qemu-devel] [PULL 14/17] scsi-generic: avoid invalid access to struct when emulating block limits

2018-11-06 Thread Paolo Bonzini
Emulation of the block limits VPD page called back into scsi-disk.c,
which however expected the request to be for a SCSIDiskState and
accessed a scsi-generic device outside the bounds of its struct
(namely to retrieve s->max_unmap_size and s->max_io_size).

To avoid this, move the emulation code to a separate function that
takes a new SCSIBlockLimits struct and marshals it into the VPD
response format.

Reported-by: Max Reitz 
Reviewed-by: Max Reitz 
Signed-off-by: Paolo Bonzini 
---
 hw/scsi/Makefile.objs   |  2 +-
 hw/scsi/emulation.c | 42 +
 hw/scsi/scsi-disk.c | 92 ++---
 hw/scsi/scsi-generic.c  | 35 -
 include/hw/scsi/emulation.h | 16 
 include/hw/scsi/scsi.h  |  1 -
 6 files changed, 104 insertions(+), 84 deletions(-)
 create mode 100644 hw/scsi/emulation.c
 create mode 100644 include/hw/scsi/emulation.h

diff --git a/hw/scsi/Makefile.objs b/hw/scsi/Makefile.objs
index 718b4c2..45167ba 100644
--- a/hw/scsi/Makefile.objs
+++ b/hw/scsi/Makefile.objs
@@ -1,4 +1,4 @@
-common-obj-y += scsi-disk.o
+common-obj-y += scsi-disk.o emulation.o
 common-obj-y += scsi-generic.o scsi-bus.o
 common-obj-$(CONFIG_LSI_SCSI_PCI) += lsi53c895a.o
 common-obj-$(CONFIG_MPTSAS_SCSI_PCI) += mptsas.o mptconfig.o mptendian.o
diff --git a/hw/scsi/emulation.c b/hw/scsi/emulation.c
new file mode 100644
index 000..06d62f3
--- /dev/null
+++ b/hw/scsi/emulation.c
@@ -0,0 +1,42 @@
+#include "qemu/osdep.h"
+#include "qemu/units.h"
+#include "qemu/bswap.h"
+#include "hw/scsi/emulation.h"
+
+int scsi_emulate_block_limits(uint8_t *outbuf, const SCSIBlockLimits *bl)
+{
+/* required VPD size with unmap support */
+memset(outbuf, 0, 0x3c);
+
+outbuf[0] = bl->wsnz; /* wsnz */
+
+if (bl->max_io_sectors) {
+/* optimal transfer length granularity.  This field and the optimal
+ * transfer length can't be greater than maximum transfer length.
+ */
+stw_be_p(outbuf + 2, MIN(bl->min_io_size, bl->max_io_sectors));
+
+/* maximum transfer length */
+stl_be_p(outbuf + 4, bl->max_io_sectors);
+
+/* optimal transfer length */
+stl_be_p(outbuf + 8, MIN(bl->opt_io_size, bl->max_io_sectors));
+} else {
+stw_be_p(outbuf + 2, bl->min_io_size);
+stl_be_p(outbuf + 8, bl->opt_io_size);
+}
+
+/* max unmap LBA count */
+stl_be_p(outbuf + 16, bl->max_unmap_sectors);
+
+/* max unmap descriptors */
+stl_be_p(outbuf + 20, bl->max_unmap_descr);
+
+/* optimal unmap granularity; alignment is zero */
+stl_be_p(outbuf + 24, bl->unmap_sectors);
+
+/* max write same size, make it the same as maximum transfer length */
+stl_be_p(outbuf + 36, bl->max_io_sectors);
+
+return 0x3c;
+}
diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c
index e2c5408..6eb258d 100644
--- a/hw/scsi/scsi-disk.c
+++ b/hw/scsi/scsi-disk.c
@@ -33,6 +33,7 @@ do { printf("scsi-disk: " fmt , ## __VA_ARGS__); } while (0)
 #include "qapi/error.h"
 #include "qemu/error-report.h"
 #include "hw/scsi/scsi.h"
+#include "hw/scsi/emulation.h"
 #include "scsi/constants.h"
 #include "sysemu/sysemu.h"
 #include "sysemu/block-backend.h"
@@ -589,7 +590,7 @@ static uint8_t *scsi_get_buf(SCSIRequest *req)
 return (uint8_t *)r->iov.iov_base;
 }
 
-int scsi_disk_emulate_vpd_page(SCSIRequest *req, uint8_t *outbuf)
+static int scsi_disk_emulate_vpd_page(SCSIRequest *req, uint8_t *outbuf)
 {
 SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, req->dev);
 uint8_t page_code = req->cmd.buf[2];
@@ -691,89 +692,36 @@ int scsi_disk_emulate_vpd_page(SCSIRequest *req, uint8_t 
*outbuf)
 }
 case 0xb0: /* block limits */
 {
-unsigned int unmap_sectors =
-s->qdev.conf.discard_granularity / s->qdev.blocksize;
-unsigned int min_io_size =
-s->qdev.conf.min_io_size / s->qdev.blocksize;
-unsigned int opt_io_size =
-s->qdev.conf.opt_io_size / s->qdev.blocksize;
-unsigned int max_unmap_sectors =
-s->max_unmap_size / s->qdev.blocksize;
-unsigned int max_io_sectors =
-s->max_io_size / s->qdev.blocksize;
+SCSIBlockLimits bl = {};
 
 if (s->qdev.type == TYPE_ROM) {
 DPRINTF("Inquiry (EVPD[%02X] not supported for CDROM\n",
 page_code);
 return -1;
 }
+bl.wsnz = 1;
+bl.unmap_sectors =
+s->qdev.conf.discard_granularity / s->qdev.blocksize;
+bl.min_io_size =
+s->qdev.conf.min_io_size / s->qdev.blocksize;
+bl.opt_io_size =
+s->qdev.conf.opt_io_size / s->qdev.blocksize;
+bl.max_unmap_sectors =
+s->max_unmap_size / s->qdev.blocksize;
+bl.max_io_sectors =
+s->max_io_size / s->qdev.blocksize;
+/* 255 descriptors fit in 4 KiB with an 8-byte header */
+bl.max_unmap_descr = 255;
+
 if (s->qd

[Qemu-devel] [PULL 05/17] MAINTAINERS: remove or downgrade myself to reviewer from some subsystems

2018-11-06 Thread Paolo Bonzini
Other people are doing a much better work than myself at handling some
subsystems.  For those files it is better if I downgrade myself to
reviewer or recognize that I am not actually doing any work there.

Cc: Daniel P. Berrange 
Cc: Gerd Hoffmann 
Cc: Eric Blake 
Cc: Thomas Huth 
Cc: Laurent Vivier 
Cc: Marc-André Lureau 
Signed-off-by: Paolo Bonzini 
---
 MAINTAINERS | 13 ++---
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8ec2281..d411521 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -105,9 +105,9 @@ Guest CPU cores (TCG):
 --
 Overall
 L: qemu-devel@nongnu.org
-M: Paolo Bonzini 
 M: Peter Crosthwaite 
 M: Richard Henderson 
+R: Paolo Bonzini 
 S: Maintained
 F: cpus.c
 F: exec.c
@@ -1139,7 +1139,8 @@ F: hw/pci-host/ppce500.c
 F: hw/net/fsl_etsec/
 
 Character devices
-M: Paolo Bonzini 
+M: Marc-André Lureau 
+R: Paolo Bonzini 
 S: Odd Fixes
 F: hw/char/
 
@@ -1526,8 +1527,8 @@ T: git git://github.com/famz/qemu.git bitmaps
 T: git git://github.com/jnsnow/qemu.git bitmaps
 
 Character device backends
-M: Paolo Bonzini 
 M: Marc-André Lureau 
+R: Paolo Bonzini 
 S: Maintained
 F: chardev/
 F: include/chardev/
@@ -1760,9 +1761,9 @@ F: tests/qmp-cmd-test.c
 T: git git://repo.or.cz/qemu/armbru.git qapi-next
 
 qtest
-M: Paolo Bonzini 
 M: Thomas Huth 
 M: Laurent Vivier 
+R: Paolo Bonzini 
 S: Maintained
 F: qtest.c
 F: tests/libqtest.*
@@ -1869,7 +1870,6 @@ F: tests/test-io-*
 Sockets
 M: Daniel P. Berrange 
 M: Gerd Hoffmann 
-M: Paolo Bonzini 
 S: Maintained
 F: include/qemu/sockets.h
 F: util/qemu-sockets.c
@@ -2056,13 +2056,12 @@ M: Ronnie Sahlberg 
 M: Paolo Bonzini 
 M: Peter Lieven 
 L: qemu-bl...@nongnu.org
-S: Supported
+S: Odd Fixes
 F: block/iscsi.c
 F: block/iscsi-opts.c
 
 Network Block Device (NBD)
 M: Eric Blake 
-M: Paolo Bonzini 
 L: qemu-bl...@nongnu.org
 S: Maintained
 F: block/nbd*
-- 
1.8.3.1





[Qemu-devel] [PULL 12/17] scsi-generic: keep VPD page list sorted

2018-11-06 Thread Paolo Bonzini
Block limits emulation is just placing 0xb0 as the final byte of the
VPD pages list.  However, VPD page numbers must be sorted, so change
that to an in-place insert.  Since I couldn't find any disk that triggered
the loop more than once, this was tested by adding manually 0xb1
at the end of the list and checking that 0xb0 was added before.

Reported-by: Max Reitz 
Reviewed-by: Max Reitz 
Signed-off-by: Paolo Bonzini 
---
 hw/scsi/scsi-generic.c | 19 +++
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/hw/scsi/scsi-generic.c b/hw/scsi/scsi-generic.c
index d60c4d0..aebb7cd 100644
--- a/hw/scsi/scsi-generic.c
+++ b/hw/scsi/scsi-generic.c
@@ -144,7 +144,7 @@ static int execute_command(BlockBackend *blk,
 
 static void scsi_handle_inquiry_reply(SCSIGenericReq *r, SCSIDevice *s)
 {
-uint8_t page, page_len;
+uint8_t page, page_idx;
 
 /*
  *  EVPD set to zero returns the standard INQUIRY data.
@@ -190,10 +190,21 @@ static void scsi_handle_inquiry_reply(SCSIGenericReq *r, 
SCSIDevice *s)
  *
  * This way, the guest kernel will be aware of the support
  * and will use it to proper setup the SCSI device.
+ *
+ * VPD page numbers must be sorted, so insert 0xb0 at the
+ * right place with an in-place insert.  After the initialization
+ * part of the for loop is executed, the device response is
+ * at r[0] to r[page_idx - 1].
  */
-page_len = r->buf[3];
-r->buf[page_len + 4] = 0xb0;
-r->buf[3] = ++page_len;
+for (page_idx = lduw_be_p(r->buf + 2) + 4;
+ page_idx > 4 && r->buf[page_idx - 1] >= 0xb0;
+ page_idx--) {
+if (page_idx < r->buflen) {
+r->buf[page_idx] = r->buf[page_idx - 1];
+}
+}
+r->buf[page_idx] = 0xb0;
+stw_be_p(r->buf + 2, lduw_be_p(r->buf + 2) + 1);
 }
 }
 }
-- 
1.8.3.1





[Qemu-devel] [PULL 03/17] i386: clarify that the Q35 machine type implements a P35 chipset

2018-11-06 Thread Paolo Bonzini
From: Daniel P. Berrangé 

The 'q35' machine type implements an Intel Series 3 chipset,
of which there are several variants:

  https://www.intel.com/Assets/PDF/datasheet/316966.pdf

The key difference between the 82P35 MCH ('p35', PCI device ID 0x29c0)
and 82Q35 GMCH ('q35', PCI device ID 0x29b0) variants is that the latter
has an integrated graphics adapter. QEMU does not implement integrated
graphics, so uses the PCI ID for the 82P35 chipset, despite calling the
machine type 'q35'. Thus we rename the PCI device ID constant to reflect
reality, to avoid confusing future developers. The new name more closely
matches what pci.ids reports it to be:

$ grep  P35 /usr/share/hwdata/pci.ids  | grep 29
29c0  82G33/G31/P35/P31 Express DRAM Controller
29c1  82G33/G31/P35/P31 Express PCI Express Root Port
29c4  82G33/G31/P35/P31 Express MEI Controller
29c5  82G33/G31/P35/P31 Express MEI Controller
29c6  82G33/G31/P35/P31 Express PT IDER Controller
29c7  82G33/G31/P35/P31 Express Serial KT Controller

$ grep  Q35 /usr/share/hwdata/pci.ids  | grep 29
29b0  82Q35 Express DRAM Controller
29b1  82Q35 Express PCI Express Root Port
29b2  82Q35 Express Integrated Graphics Controller
29b3  82Q35 Express Integrated Graphics Controller
29b4  82Q35 Express MEI Controller
29b5  82Q35 Express MEI Controller
29b6  82Q35 Express PT IDER Controller
29b7  82Q35 Express Serial KT Controller

Arguably the QEMU machine type should be named 'p35'. At this point in
time, however, it is not worth the churn for management applications &
documentation to worry about renaming it.

Signed-off-by: Daniel P. Berrangé 
Message-Id: <20180830105757.10577-1-berra...@redhat.com>
Signed-off-by: Paolo Bonzini 
---
 hw/pci-host/q35.c| 10 +-
 include/hw/pci/pci_ids.h |  2 +-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/pci-host/q35.c b/hw/pci-host/q35.c
index 966a7cf..71e4ca5 100644
--- a/hw/pci-host/q35.c
+++ b/hw/pci-host/q35.c
@@ -622,7 +622,15 @@ static void mch_class_init(ObjectClass *klass, void *data)
 dc->desc = "Host bridge";
 dc->vmsd = &vmstate_mch;
 k->vendor_id = PCI_VENDOR_ID_INTEL;
-k->device_id = PCI_DEVICE_ID_INTEL_Q35_MCH;
+/*
+ * The 'q35' machine type implements an Intel Series 3 chipset,
+ * of which there are several variants. The key difference between
+ * the 82P35 MCH ('p35') and 82Q35 GMCH ('q35') variants is that
+ * the latter has an integrated graphics adapter. QEMU does not
+ * implement integrated graphics, so uses the PCI ID for the 82P35
+ * chipset.
+ */
+k->device_id = PCI_DEVICE_ID_INTEL_P35_MCH;
 k->revision = MCH_HOST_BRIDGE_REVISION_DEFAULT;
 k->class_id = PCI_CLASS_BRIDGE_HOST;
 /*
diff --git a/include/hw/pci/pci_ids.h b/include/hw/pci/pci_ids.h
index 63acc72..eeb3301 100644
--- a/include/hw/pci/pci_ids.h
+++ b/include/hw/pci/pci_ids.h
@@ -255,7 +255,7 @@
 #define PCI_DEVICE_ID_INTEL_82801I_EHCI2 0x293c
 #define PCI_DEVICE_ID_INTEL_82599_SFP_VF 0x10ed
 
-#define PCI_DEVICE_ID_INTEL_Q35_MCH  0x29c0
+#define PCI_DEVICE_ID_INTEL_P35_MCH  0x29c0
 
 #define PCI_VENDOR_ID_XEN0x5853
 #define PCI_DEVICE_ID_XEN_PLATFORM   0x0001
-- 
1.8.3.1





Re: [Qemu-devel] List of files containing devices which have not been QOMified

2018-11-06 Thread Paolo Bonzini
On 06/11/2018 22:23, John Snow wrote:
> 
> 
> On 11/06/2018 02:46 PM, Paolo Bonzini wrote:
>> On 06/11/2018 19:43, Peter Maydell wrote:
> 
>>> hw/ide/ahci.c
>>
>> Even though AHCIState is not a QOM object, all of its users are
>> (TYPE_SYSBUS_AHCI is in this file, TYPE_ICH9_AHCI is in hw/ide/ich.c).
> 
> Yeah, if there's something *further* that needs to happen, let me know.
> I'm not sure what the QOM ideal is.

I think it's okay.  The virtio-*-device approach (which is contained in
virtio-*-pci and virtio-*-ccw) made sense for virtio where you could
have "socket" virtio-mmio devices and the virtio-*-device "plugs" are
specified in the command line.  However, for AHCI it's overkill.

Paolo



Re: [Qemu-devel] List of files containing devices which have not been QOMified

2018-11-06 Thread John Snow



On 11/06/2018 02:46 PM, Paolo Bonzini wrote:
> On 06/11/2018 19:43, Peter Maydell wrote:

>> hw/ide/ahci.c
> 
> Even though AHCIState is not a QOM object, all of its users are
> (TYPE_SYSBUS_AHCI is in this file, TYPE_ICH9_AHCI is in hw/ide/ich.c).
> 

Yeah, if there's something *further* that needs to happen, let me know.
I'm not sure what the QOM ideal is.

Is there a reference implementation for what we consider to be a proper
QOM device that I can measure against?

A list of forbidden ABIs I ought not use?

(S/ATA device realization is in itself already particularly bizarre
where we have a skeleton list of properties in QOM and then we just
malloc the bulk of the structure we actually use, but I think it's not
worth fixing to be nicer because I haven't figured out a way to do it
without breaking backwards compatibility, so I've just left it alone.)

--js



Re: [Qemu-devel] List of files containing devices which have not been QOMified

2018-11-06 Thread Alistair Francis
On Tue, Nov 6, 2018 at 11:47 AM Paolo Bonzini  wrote:
>
> On 06/11/2018 19:43, Peter Maydell wrote:
> > hw/core/ptimer.c
>
> Not a device.
>
> > hw/i2c/bitbang_i2c.c
>
> TYPE_GPIO_I2C?
>
> > hw/ide/ahci.c
>
> Even though AHCIState is not a QOM object, all of its users are
> (TYPE_SYSBUS_AHCI is in this file, TYPE_ICH9_AHCI is in hw/ide/ich.c).

Should this list go on the wiki somewhere? Then we can cross off ones
as they are converted.

Alistair

>
> Paolo
>



Re: [Qemu-devel] List of files containing devices which have not been QOMified

2018-11-06 Thread Paolo Bonzini
On 06/11/2018 19:43, Peter Maydell wrote:
> hw/core/ptimer.c

Not a device.

> hw/i2c/bitbang_i2c.c

TYPE_GPIO_I2C?

> hw/ide/ahci.c

Even though AHCIState is not a QOM object, all of its users are
(TYPE_SYSBUS_AHCI is in this file, TYPE_ICH9_AHCI is in hw/ide/ich.c).

Paolo



Re: [Qemu-devel] List of files containing devices which have not been QOMified

2018-11-06 Thread Paolo Bonzini
On 06/11/2018 19:43, Peter Maydell wrote:
> hw/pci/shpc.c ?

This is not a device, it is a PCI capability.

Paolo



Re: [Qemu-devel] List of files containing devices which have not been QOMified

2018-11-06 Thread Philippe Mathieu-Daudé

On 6/11/18 19:43, Peter Maydell wrote:

I had an idea for how to get a rough list of source files
containing devices that haven't been QOMified. The theory
is that a pre-QOM device generally has an "init" function
which allocates memory for the device struct. So looking in
hw/ for files which call g_new*() or g_malloc*() should get
us all the non-QOM devices (as well as a pile of false
positives, of course). The following link is the result of
doing that and then eyeballing the results for false positives
and throwing those out. It might have missed one or two
files or included one or two by mistake. But I think it's
pretty close, and it seems to have caught all the obvious
ones I knew about. There are 61 files on this list.

I am also suspicious about hw/bt/ but don't know enough
about that subsystem to say if it could benefit from
using QOM objects more.




hw/arm/exynos4210.c

I already did this one.

> hw/sd/omap_mmc.c
I will do this one.

> hw/mips/mips_malta.c
> hw/timer/mips_gictimer.c
I'm interested in doing those two.


hw/char/parallel.c
hw/char/serial.c
hw/display/vga-isa-mm.c
hw/input/pckbd.c
hw/input/ps2.c

I can do those too when QOM'ifying the Malta.


hw/char/sh_serial.c
hw/sh4/r2d.c
hw/sh4/sh7750.c

> hw/timer/sh_timer.c
If nobody volonteers, I can take those.

Regards,

Phil.



Re: [Qemu-devel] [PATCH v4] lsi_scsi: Reselection needed to remove pending commands from queue

2018-11-06 Thread Paolo Bonzini
On 06/11/2018 20:13, george kennedy wrote:
> 
> On 11/6/2018 1:58 PM, Paolo Bonzini wrote:
>> On 31/10/2018 22:03, George Kennedy wrote:
>>> +#define SCRIPTS_LOAD_AND_STORE  0xe2340004
>> I'm very confused.  Why did this constant reappear?
> 
> Ok. Me too.
> 
> What are you proposing instead and I'll change it to that?
> 
> Did I have what you're after in a prior patch? If so, can you point to
> that clip?

No.  You need to do what you're doing here after the WAIT DISCONNECT
command (which, as far as I can see, comes before the LOAD AND STORE
you're looking for).  I wrote that in the reply to v3 I think.

Paolo



Re: [Qemu-devel] [PATCH v4] lsi_scsi: Reselection needed to remove pending commands from queue

2018-11-06 Thread george kennedy



On 11/6/2018 1:58 PM, Paolo Bonzini wrote:

On 31/10/2018 22:03, George Kennedy wrote:

+#define SCRIPTS_LOAD_AND_STORE  0xe2340004

I'm very confused.  Why did this constant reappear?


Ok. Me too.

What are you proposing instead and I'll change it to that?

Did I have what you're after in a prior patch? If so, can you point to 
that clip?


Thank you,
George



Paolo







Re: [Qemu-devel] [PATCH] MAINTAINERS: Add an entry for the 'collie' machine

2018-11-06 Thread Peter Maydell
On 6 November 2018 at 18:52, Thomas Huth  wrote:
> On 2018-11-06 19:49, Philippe Mathieu-Daudé wrote:
>> On 6/11/18 19:17, Thomas Huth wrote:
>>> There is no active maintainer, but since Peter is picking up
>>> patches via qemu-...@nongnu.org, I think we could at least use
>>> "Odd Fixes" as status here.
>>
>> This looks more as "Orphan" to me...
>
> I'll leave it up to Peter for the final decision...

I think we're not very consistent[*] in our usage of the various
statuses in the MAINTAINERS file. I guess "Odd Fixes" makes
sense in that, well, if you send a patch to this
code and cc me I'll review it and put it in the tree. (This
is true of any of the arm boards we have.)

[*] We have one thing tagged Orphan, which is bsd-user/,
and some things tagged Odd Fixes with no listed maintainer,
and some things tagged Odd Fixes which are in practice more
like Orphan (for instance sh4), and we list "fpu/" as
Odd Fixes despite having given it a pretty thorough
overhaul very recently, and so on...

If you wanted a mechanizable rule, you could try something
like "every file which is in status Odd Fixes or better
must list with M: at least one named individual who has
submitted a pull request in the last nine months" :-)

thanks
-- PMM



Re: [Qemu-devel] [PATCH v4] lsi_scsi: Reselection needed to remove pending commands from queue

2018-11-06 Thread Paolo Bonzini
On 31/10/2018 22:03, George Kennedy wrote:
> +#define SCRIPTS_LOAD_AND_STORE  0xe2340004

I'm very confused.  Why did this constant reappear?

Paolo




Re: [Qemu-devel] [PATCH v1] bt: use size_t type for length parameters instead of int

2018-11-06 Thread Paolo Bonzini
On 06/11/2018 15:52, Thomas Huth wrote:
> The bluetooth subsystem is completely unmaintained, so if Paolo does not
> want to pick it up through his "misc" tree, maybe Peter could apply this
> patch directly? Or maybe it could go through the trivial tree since it
> does not look very complicated?

I wanted to check that there were no possible underflows, but that takes
time.

> FWIW, the patch looks OK to me at a first glance, so:
> 
> Acked-by: Thomas Huth 
> 
> PS: I still think we should deprecate the bt subsystem, since nobody
> really touched it within years...

Well, apparently there _are_ people using it, and it worked for them.
Or at least they were using it in 2014.  I'm sure that if people
starting poking at it, some holes would appear, but it appears to work
decently.

Paolo



Re: [Qemu-devel] [PATCH 0/4] scsi-generic: fixes for Block Limits emulation

2018-11-06 Thread Paolo Bonzini
On 06/11/2018 16:54, Daniel Henrique Barboza wrote:
> Hi,
> 
> How did you find all those issues, Max? First patch is something that
> I missed out from the SCSI spec (ordering of the VPD pages)
> and could have been detected by code inspection,  but I am curious about
> the other fixes.

It was found just by code inspection.

Paolo



Re: [Qemu-devel] [PATCH] MAINTAINERS: Add an entry for the 'collie' machine

2018-11-06 Thread Thomas Huth
On 2018-11-06 19:49, Philippe Mathieu-Daudé wrote:
> On 6/11/18 19:17, Thomas Huth wrote:
>> There is no active maintainer, but since Peter is picking up
>> patches via qemu-...@nongnu.org, I think we could at least use
>> "Odd Fixes" as status here.
> 
> This looks more as "Orphan" to me...

I'll leave it up to Peter for the final decision...

> Regardless the one choosed:
> Reviewed-by: Philippe Mathieu-Daudé 

 Thanks,
  Thomas




Re: [Qemu-devel] [PATCH] MAINTAINERS: Add an entry for the 'collie' machine

2018-11-06 Thread Philippe Mathieu-Daudé

On 6/11/18 19:17, Thomas Huth wrote:

There is no active maintainer, but since Peter is picking up
patches via qemu-...@nongnu.org, I think we could at least use
"Odd Fixes" as status here.


This looks more as "Orphan" to me...

Regardless the one choosed:
Reviewed-by: Philippe Mathieu-Daudé 



Signed-off-by: Thomas Huth 
---
  MAINTAINERS | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 0499e11..471cf72 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -592,6 +592,12 @@ F: hw/*/pxa2xx*
  F: hw/misc/mst_fpga.c
  F: include/hw/arm/pxa.h
  
+Sharp SL-5500 (Collie) PDA

+L: qemu-...@nongnu.org
+S: Odd Fixes
+F: hw/arm/collie.c
+F: hw/arm/strongarm*
+
  Stellaris
  M: Peter Maydell 
  L: qemu-...@nongnu.org





[Qemu-devel] List of files containing devices which have not been QOMified

2018-11-06 Thread Peter Maydell
I had an idea for how to get a rough list of source files
containing devices that haven't been QOMified. The theory
is that a pre-QOM device generally has an "init" function
which allocates memory for the device struct. So looking in
hw/ for files which call g_new*() or g_malloc*() should get
us all the non-QOM devices (as well as a pile of false
positives, of course). The following link is the result of
doing that and then eyeballing the results for false positives
and throwing those out. It might have missed one or two
files or included one or two by mistake. But I think it's
pretty close, and it seems to have caught all the obvious
ones I knew about. There are 61 files on this list.

I am also suspicious about hw/bt/ but don't know enough
about that subsystem to say if it could benefit from
using QOM objects more.

hw/arm/exynos4210.c
hw/arm/nseries.c
hw/arm/omap1.c
hw/arm/omap2.c
hw/arm/pxa2xx.c
hw/arm/stellaris.c
hw/arm/strongarm.c
hw/char/omap_uart.c
hw/char/parallel.c
hw/char/serial.c
hw/char/sh_serial.c
hw/core/ptimer.c
hw/cris/axis_dev88.c
hw/display/blizzard.c
hw/display/omap_dss.c
hw/display/omap_lcdc.c
hw/display/pxa2xx_lcd.c
hw/display/tc6393xb.c
hw/display/vga-isa-mm.c
hw/dma/etraxfs_dma.c
hw/dma/omap_dma.c
hw/dma/rc4030.c
hw/dma/soc_dma.c
hw/i2c/bitbang_i2c.c
hw/ide/ahci.c
hw/input/pckbd.c
hw/input/ps2.c
hw/input/pxa2xx_keypad.c
hw/input/stellaris_input.c
hw/input/tsc2005.c
hw/input/tsc210x.c
hw/m68k/mcf5206.c
hw/m68k/mcf5208.c
hw/mips/mips_malta.c
hw/misc/cbus.c
hw/misc/omap_clk.c
hw/misc/omap_gpmc.c
hw/misc/omap_l4.c
hw/misc/omap_sdrc.c
hw/openrisc/cputimer.c
hw/pci/shpc.c ?
hw/ppc/ppc405_boards.c
hw/ppc/ppc405_uc.c
hw/ppc/ppc440_uc.c
hw/ppc/ppc4xx_devs.c
hw/ppc/ppc_booke.c
hw/ppc/prep.c
hw/riscv/riscv_htif.c
hw/riscv/sifive_uart.c
hw/sd/omap_mmc.c
hw/sh4/r2d.c
hw/sh4/sh7750.c
hw/sparc64/sparc64.c
hw/ssi/omap_spi.c
hw/timer/arm_timer.c
hw/timer/mips_gictimer.c
hw/timer/omap_gptimer.c
hw/timer/omap_synctimer.c
hw/timer/sh_timer.c
hw/usb/hcd-musb.c
hw/xtensa/xtfpga.c

thanks
-- PMM



[Qemu-devel] [PATCH v2 for-3.1] hw/arm/sysbus-fdt: Only call match_fn callback if the type matches

2018-11-06 Thread Eric Auger
Commit af7d64ede0b9 (hw/arm/sysbus-fdt: Allow device matching with DT
compatible value) introduced a match_fn callback which gets called
for each registered combo to check whether a sysbus device can be
dynamically instantiated. However the callback gets called even if
the device type does not match the binding combo typename field.
This causes an assert when passing "-device ramfb" to the qemu
command line as vfio_platform_match() gets called on a non
vfio-platform device.

To fix this regression, let's change the add_fdt_node() logic so
that we first check the type and if the match_fn callback is defined,
then we also call it.

Binding combos only requesting a type check do not define the
match_fn callback.

Fixes: af7d64ede0b9 (hw/arm/sysbus-fdt: Allow device matching with
DT compatible value)

Signed-off-by: Eric Auger 
Reported-by: Thomas Huth 

---

v1 -> v2:
- use "if (!iter->match_fn || iter->match_fn(sbdev, iter)) {"
  as suggested by Peter to avoid code duplication
- mention the ramfb regression fixed by this patch in the
  commit message.
---
 hw/arm/sysbus-fdt.c | 12 +++-
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
index 0e24c803a1..ad698d4832 100644
--- a/hw/arm/sysbus-fdt.c
+++ b/hw/arm/sysbus-fdt.c
@@ -449,7 +449,7 @@ static bool type_match(SysBusDevice *sbdev, const 
BindingEntry *entry)
 return !strcmp(object_get_typename(OBJECT(sbdev)), entry->typename);
 }
 
-#define TYPE_BINDING(type, add_fn) {(type), NULL, (add_fn), type_match}
+#define TYPE_BINDING(type, add_fn) {(type), NULL, (add_fn), NULL}
 
 /* list of supported dynamic sysbus bindings */
 static const BindingEntry bindings[] = {
@@ -481,10 +481,12 @@ static void add_fdt_node(SysBusDevice *sbdev, void 
*opaque)
 for (i = 0; i < ARRAY_SIZE(bindings); i++) {
 const BindingEntry *iter = &bindings[i];
 
-if (iter->match_fn(sbdev, iter)) {
-ret = iter->add_fn(sbdev, opaque);
-assert(!ret);
-return;
+if (type_match(sbdev, iter)) {
+if (!iter->match_fn || iter->match_fn(sbdev, iter)) {
+ret = iter->add_fn(sbdev, opaque);
+assert(!ret);
+return;
+}
 }
 }
 error_report("Device %s can not be dynamically instantiated",
-- 
2.17.2




Re: [Qemu-devel] [PULL v2 0/3] Monitor patches for 2018-10-30

2018-11-06 Thread Peter Maydell
On 6 November 2018 at 17:39, Markus Armbruster  wrote:
> The following changes since commit 0ca70f19c010ccf0b10cf7cc1d2b9a87193fe787:
>
>   tests: Fix Python 3 detection on older GNU make versions (2018-11-06 
> 14:58:13 +)
>
> are available in the Git repository at:
>
>   git://repo.or.cz/qemu/armbru.git tags/pull-monitor-2018-10-30-v2
>
> for you to fetch changes up to 0c57893d62596d1d91779452be3b38b4b72ecd04:
>
>   vl: Avoid crash when -mon is underspecified (2018-11-06 17:03:29 +0100)
>
> 
> Monitor patches for 2018-10-30
>
> * Fix crash on shutdown with --daemonize
> * Avoid crash when -mon lacks chardev=...
>
> 

Applied, thanks.

-- PMM



[Qemu-devel] [PATCH] MAINTAINERS: Add an entry for the 'collie' machine

2018-11-06 Thread Thomas Huth
There is no active maintainer, but since Peter is picking up
patches via qemu-...@nongnu.org, I think we could at least use
"Odd Fixes" as status here.

Signed-off-by: Thomas Huth 
---
 MAINTAINERS | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 0499e11..471cf72 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -592,6 +592,12 @@ F: hw/*/pxa2xx*
 F: hw/misc/mst_fpga.c
 F: include/hw/arm/pxa.h
 
+Sharp SL-5500 (Collie) PDA
+L: qemu-...@nongnu.org
+S: Odd Fixes
+F: hw/arm/collie.c
+F: hw/arm/strongarm*
+
 Stellaris
 M: Peter Maydell 
 L: qemu-...@nongnu.org
-- 
1.8.3.1




Re: [Qemu-devel] [PATCH v4 04/11] block: improve should_update_child

2018-11-06 Thread Kevin Wolf
Am 15.10.2018 um 18:06 hat Vladimir Sementsov-Ogievskiy geschrieben:
> As it already said in the comment, we don't want to create loops in
> parent->child relations. So, when we try to append @to to @c, we should
> check that @c is not in @to children subtree, and we should check it
> recursively, not only the first level. The patch provides BFS-based
> search, to check the relations.
> 
> This is needed for further fleecing-hook filter usage: we need to
> append it to source, when the hook is already a parent of target, and
> source may be in a backing chain of target (fleecing-scheme). So, on
> appending, the hook should not became a child (direct or through
> children subtree) of the target.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 
> ---
>  block.c | 32 +++-
>  1 file changed, 27 insertions(+), 5 deletions(-)
> 
> diff --git a/block.c b/block.c
> index c298ca6a19..7f605b0bf0 100644
> --- a/block.c
> +++ b/block.c
> @@ -3432,7 +3432,7 @@ void bdrv_close_all(void)
>  
>  static bool should_update_child(BdrvChild *c, BlockDriverState *to)
>  {
> -BdrvChild *to_c;
> +GList *queue = NULL, *pos;
>  
>  if (c->role->stay_at_node) {
>  return false;
> @@ -3468,13 +3468,35 @@ static bool should_update_child(BdrvChild *c, 
> BlockDriverState *to)
>   * if A is a child of B, that means we cannot replace A by B there
>   * because that would create a loop.  Silently detaching A from B
>   * is also not really an option.  So overall just leaving A in
> - * place there is the most sensible choice. */
> -QLIST_FOREACH(to_c, &to->children, next) {
> -if (to_c == c) {
> -return false;
> + * place there is the most sensible choice.
> + *
> + * upd: If the child @c belongs to the @to's children, or children of 
> it's
> + * children and so on - this would create a loop to. To prevent it let's 
> do
> + * a BFS search on @to children subtree.
> + */

I don't like the "upd:" thing. Let me try to rephrase the addition:

We would also create a loop in any cases where @c is only indirectly
referenced by @to. Prevent this by returning false if @c is found
(by breadth-first search) anywhere in the whole subtree of @to.

Also, even though the coding style says 80 characters per line, the
existing comment seems to use only 70 characters. Let's try to stay
consistent here, otherwise it looks a bit odd.

> +pos = queue = g_list_append(queue, to);
> +while (pos) {
> +BlockDriverState *v = pos->data;
> +BdrvChild *c2;
> +
> +QLIST_FOREACH(c2, &v->children, next) {
> +if (c2 == c) {
> +g_list_free(queue);
> +return false;
> +}
> +
> +if (g_list_find(queue, c2->bs)) {
> +continue;
> +}
> +
> +queue = g_list_append(queue, c2->bs);
>  }
> +
> +pos = pos->next;
>  }
>  
> +g_list_free(queue);
>  return true;
>  }

The functional change looks good to me.

Kevin



Re: [Qemu-devel] [PATCH v4 03/11] block: allow serialized reads to intersect

2018-11-06 Thread Kevin Wolf
Am 15.10.2018 um 18:06 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Otherwise, if we have serialized read-part in copy_range from backing
> file to its parent if CoW take place, this CoW's sub-reads will
> intersect with firstly created serialized read request.
> 
> Anyway, reads should not influence on disk view, let's allow them to
> intersect.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 

What about reads that internally trigger writes, such as copy-on-read?

Kevin



[Qemu-devel] [PULL v2 0/3] Monitor patches for 2018-10-30

2018-11-06 Thread Markus Armbruster
The following changes since commit 0ca70f19c010ccf0b10cf7cc1d2b9a87193fe787:

  tests: Fix Python 3 detection on older GNU make versions (2018-11-06 14:58:13 
+)

are available in the Git repository at:

  git://repo.or.cz/qemu/armbru.git tags/pull-monitor-2018-10-30-v2

for you to fetch changes up to 0c57893d62596d1d91779452be3b38b4b72ecd04:

  vl: Avoid crash when -mon is underspecified (2018-11-06 17:03:29 +0100)


Monitor patches for 2018-10-30

* Fix crash on shutdown with --daemonize
* Avoid crash when -mon lacks chardev=...


Eric Blake (1):
  vl: Avoid crash when -mon is underspecified

Wolfgang Bumiller (2):
  monitor: guard iothread access by mon->use_io_thread
  monitor: delay monitor iothread creation

 monitor.c | 37 ++---
 vl.c  |  4 
 2 files changed, 26 insertions(+), 15 deletions(-)

Eric Blake (1):
  vl: Avoid crash when -mon is underspecified

Wolfgang Bumiller (2):
  monitor: guard iothread access by mon->use_io_thread
  monitor: delay monitor iothread creation

 monitor.c | 37 ++---
 vl.c  |  4 
 2 files changed, 26 insertions(+), 15 deletions(-)

-- 
2.17.2




[Qemu-devel] [PULL v2 2/3] monitor: delay monitor iothread creation

2018-11-06 Thread Markus Armbruster
From: Wolfgang Bumiller 

Commit d32749deb615 moved the call to monitor_init_globals()
to before os_daemonize(), making it an unsuitable place to
spawn the monitor iothread as it won't be inherited over the
fork() in os_daemonize().

We now spawn the thread the first time we instantiate a
monitor which actually has use_io_thread == true.
Instantiation of monitors happens only after os_daemonize().
We still need to create the qmp_dispatcher_bh when not using
iothreads, so this now still happens in
monitor_init_globals().

Signed-off-by: Wolfgang Bumiller 
Fixes: d32749deb615 ("monitor: move init global earlier")
Message-Id: <20180925081507.11873-3-w.bumil...@proxmox.com>
Reviewed-by: Eric Blake 
Reviewed-by: Peter Xu 
Tested-by: Peter Xu 
[This fixes a crash on shutdown with --daemonize]
Signed-off-by: Markus Armbruster 
---
 monitor.c | 35 +--
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/monitor.c b/monitor.c
index 66f149c1dc..d39390c2f2 100644
--- a/monitor.c
+++ b/monitor.c
@@ -708,9 +708,14 @@ static void monitor_qapi_event_init(void)
 
 static void handle_hmp_command(Monitor *mon, const char *cmdline);
 
+static void monitor_iothread_init(void);
+
 static void monitor_data_init(Monitor *mon, bool skip_flush,
   bool use_io_thread)
 {
+if (use_io_thread && !mon_iothread) {
+monitor_iothread_init();
+}
 memset(mon, 0, sizeof(Monitor));
 qemu_mutex_init(&mon->mon_lock);
 qemu_mutex_init(&mon->qmp.qmp_queue_lock);
@@ -4461,15 +4466,6 @@ static AioContext *monitor_get_aio_context(void)
 static void monitor_iothread_init(void)
 {
 mon_iothread = iothread_create("mon_iothread", &error_abort);
-
-/*
- * The dispatcher BH must run in the main loop thread, since we
- * have commands assuming that context.  It would be nice to get
- * rid of those assumptions.
- */
-qmp_dispatcher_bh = aio_bh_new(iohandler_get_aio_context(),
-   monitor_qmp_bh_dispatcher,
-   NULL);
 }
 
 void monitor_init_globals(void)
@@ -4479,7 +4475,15 @@ void monitor_init_globals(void)
 sortcmdlist();
 qemu_mutex_init(&monitor_lock);
 qemu_mutex_init(&mon_fdsets_lock);
-monitor_iothread_init();
+
+/*
+ * The dispatcher BH must run in the main loop thread, since we
+ * have commands assuming that context.  It would be nice to get
+ * rid of those assumptions.
+ */
+qmp_dispatcher_bh = aio_bh_new(iohandler_get_aio_context(),
+   monitor_qmp_bh_dispatcher,
+   NULL);
 }
 
 /* These functions just adapt the readline interface in a typesafe way.  We
@@ -4624,7 +4628,9 @@ void monitor_cleanup(void)
  * we need to unregister from chardev below in
  * monitor_data_destroy(), and chardev is not thread-safe yet
  */
-iothread_stop(mon_iothread);
+if (mon_iothread) {
+iothread_stop(mon_iothread);
+}
 
 /* Flush output buffers and destroy monitors */
 qemu_mutex_lock(&monitor_lock);
@@ -4639,9 +4645,10 @@ void monitor_cleanup(void)
 /* QEMUBHs needs to be deleted before destroying the I/O thread */
 qemu_bh_delete(qmp_dispatcher_bh);
 qmp_dispatcher_bh = NULL;
-
-iothread_destroy(mon_iothread);
-mon_iothread = NULL;
+if (mon_iothread) {
+iothread_destroy(mon_iothread);
+mon_iothread = NULL;
+}
 }
 
 QemuOptsList qemu_mon_opts = {
-- 
2.17.2




[Qemu-devel] [PULL v2 3/3] vl: Avoid crash when -mon is underspecified

2018-11-06 Thread Markus Armbruster
From: Eric Blake 

A quick coredump on an incomplete command line:
./x86_64-softmmu/qemu-system-x86_64 -mon mode=control,pretty=on

 #0  0x7723d9e4 in g_str_hash () at /lib64/libglib-2.0.so.0
 #1  0x7723ce38 in g_hash_table_lookup () at /lib64/libglib-2.0.so.0
 #2  0x55cc0073 in object_class_property_find (klass=0x566a94b0, 
name=0x0, errp=0x0) at qom/object.c:1135
 #3  0x55cc004b in object_class_property_find (klass=0x566a9440, 
name=0x0, errp=0x0) at qom/object.c:1129
 #4  0x55cbfe6e in object_property_find (obj=0x568348c0, name=0x0, 
errp=0x0) at qom/object.c:1080
 #5  0x55cc183d in object_resolve_path_component 
(parent=0x568348c0, part=0x0) at qom/object.c:1762
 #6  0x55d82071 in qemu_chr_find (name=0x0) at chardev/char.c:802
 #7  0x559d77cb in mon_init_func (opaque=0x0, opts=0x566b65a0, 
errp=0x0) at vl.c:2291

Fix it to instead fail gracefully.

Signed-off-by: Eric Blake 
Message-Id: <20181023213600.364086-1-ebl...@redhat.com>
Reviewed-by: Paolo Bonzini 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Peter Xu 
Signed-off-by: Markus Armbruster 
---
 vl.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/vl.c b/vl.c
index 03ed215d7b..55bab005b6 100644
--- a/vl.c
+++ b/vl.c
@@ -2323,6 +2323,10 @@ static int mon_init_func(void *opaque, QemuOpts *opts, 
Error **errp)
 }
 
 chardev = qemu_opt_get(opts, "chardev");
+if (!chardev) {
+error_report("chardev is required");
+exit(1);
+}
 chr = qemu_chr_find(chardev);
 if (chr == NULL) {
 error_setg(errp, "chardev \"%s\" not found", chardev);
-- 
2.17.2




[Qemu-devel] [PULL v2 1/3] monitor: guard iothread access by mon->use_io_thread

2018-11-06 Thread Markus Armbruster
From: Wolfgang Bumiller 

monitor_resume() and monitor_suspend() both want to
"kick" the I/O thread if it is there, but in
monitor_suspend() lacked the use_io_thread flag condition.
This is required when we later only spawn the thread on
first use.

Signed-off-by: Wolfgang Bumiller 
Reviewed-by: Eric Blake 
Reviewed-by: Peter Xu 
Message-Id: <20180925081507.11873-2-w.bumil...@proxmox.com>
Signed-off-by: Markus Armbruster 
---
 monitor.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/monitor.c b/monitor.c
index 823b5a1099..66f149c1dc 100644
--- a/monitor.c
+++ b/monitor.c
@@ -4292,7 +4292,7 @@ int monitor_suspend(Monitor *mon)
 
 atomic_inc(&mon->suspend_cnt);
 
-if (monitor_is_qmp(mon)) {
+if (monitor_is_qmp(mon) && mon->use_io_thread) {
 /*
  * Kick I/O thread to make sure this takes effect.  It'll be
  * evaluated again in prepare() of the watch object.
-- 
2.17.2




Re: [Qemu-devel] [PULL 1/1] update seabios to master snapshot

2018-11-06 Thread Stefan Berger

On 11/6/18 6:28 AM, Gerd Hoffmann wrote:

seabios 1.12 release is planned for november.

update seabios to a master branch snapshot so it gets more testing
and to make the delta smaller when updating to -final during freeze.

git shortlog rel-1.11.2..14221cd86e
===

[...]

Paul Menzel (1):
   docs/Download: Use more secure HTTPS URLs where possible

Stefan Berger (5):
   tpm: Add support for TPM2 ACPI table
   tpm: Wait for tpmRegValidSts flag on CRB interface before probing
   tpm: revert return values for successful/failed CRB probing
   tpm: when CRB is active, select, lock it, and check addresses
   tpm: Request access to locality 0

Stephen Douthit (3):
   tpm: Refactor duplicated wait code in tis_wait_sts() & crb_wait_reg()
   tpm: Wait for interface startup when probing
   tpm: Handle unimplemented TIS_REG_IFACE_ID in tis_get_tpm_version()

Signed-off-by: Gerd Hoffmann 


Thanks for taking the latest version. If this works out well, would it 
be possible to use this version of Seabios also for QEMU 3.0 stable? I 
am asking primarily due to the TPM patches and the fact that QEMU 3.0 is 
packaged in F29, with the CRB port missing in Seabios...



   Stefan




Re: [Qemu-devel] [PATCH for-3.1] target/arm: Remove workaround for small SAU regions

2018-11-06 Thread Philippe Mathieu-Daudé

On 6/11/18 17:38, Peter Maydell wrote:

Before we supported direct execution from MMIO regions, we
implemented workarounds in commit 720424359917887c926a33d2
which let us avoid doing so, even if the SAU or MPU region
was less than page-sized.

Once we implemented execute-from-MMIO, we removed part
of those workarounds in commit d4b6275df320cee76; but
we forgot the one in get_phys_addr_pmsav8() which
suppressed use of small SAU regions in executable regions.
Remove that workaround now.

Signed-off-by: Peter Maydell 


Reviewed-by: Philippe Mathieu-Daudé 


---
  target/arm/helper.c | 12 
  1 file changed, 12 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 96301930cc8..ec56becc394 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -10560,18 +10560,6 @@ static bool get_phys_addr_pmsav8(CPUARMState *env, 
uint32_t address,
  
  ret = pmsav8_mpu_lookup(env, address, access_type, mmu_idx, phys_ptr,

  txattrs, prot, &mpu_is_subpage, fi, NULL);
-/*
- * TODO: this is a temporary hack to ignore the fact that the SAU region
- * is smaller than a page if this is an executable region. We never
- * supported small MPU regions, but we did (accidentally) allow small
- * SAU regions, and if we now made small SAU regions not be executable
- * then this would break previously working guest code. We can't
- * remove this until/unless we implement support for execution from
- * small regions.
- */
-if (*prot & PAGE_EXEC) {
-sattrs.subpage = false;
-}
  *page_size = sattrs.subpage || mpu_is_subpage ? 1 : TARGET_PAGE_SIZE;
  return ret;
  }





Re: [Qemu-devel] [PATCH v4 11/11] block/backup: use backup-top instead of write notifiers

2018-11-06 Thread Kevin Wolf
Am 15.10.2018 um 18:06 hat Vladimir Sementsov-Ogievskiy geschrieben:
> Drop write notifiers and use filter node instead. Changes:
> 
> 1. copy-before-writes now handled by filter node, so, drop all
>is_write_notifier arguments.
> 
> 2. we don't have intersecting requests, so their handling is dropped.
> Instead, synchronization works as follows:
> when backup or backup-top starts copying of some area it firstly
> clears copy-bitmap bits, and nobody touches areas, not marked with
> dirty bits in copy-bitmap, so there is no intersection. Also, backup
> job copy operations are surrounded by bdrv region lock, which is
> actually serializing request, to not interfer with guest writes and
> not read changed data from source (before reading we clear
> corresponding bit in copy-bitmap, so, this area is not more handled by
> backup-top).
> 
> 3. To sync with in-flight requests we now just drain hook node, we
> don't need rw-lock.
> 
> 4. After the whole backup loop (top, full, incremental modes), we need
> to check for not copied clusters, which were under backup-top operation
> and we skipped them, but backup-top operation failed, error returned to
> the guest and dirty bits set back.
> 
> 5. Don't create additional blk, use backup-top children for copy
> operations.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy 

Haven't reviewed it yet, but gcc complains correctly here:

block/backup.c: In function 'backup_job_create':
block/backup.c:717:9: error: 'backup_top' may be used uninitialized in this 
function [-Werror=maybe-uninitialized]
 bdrv_backup_top_drop(backup_top);
 ^~~~
cc1: all warnings being treated as errors

> @@ -653,24 +648,29 @@ BlockJob *backup_job_create(const char *job_id, 
> BlockDriverState *bs,
>  
>  copy_bitmap = hbitmap_alloc(len, ctz32(cluster_size));
>  
> +/* bdrv_get_device_name will not help to find device name starting from
> + * @bs after backup-top append, so let's calculate job_id before. Do
> + * it in the same way like block_job_create
> + */
> +if (job_id == NULL && !(creation_flags & JOB_INTERNAL)) {
> +job_id = bdrv_get_device_name(bs);
> +}
> +
> +backup_top = bdrv_backup_top_append(bs, target, copy_bitmap, errp);
> +if (!backup_top) {
> +return NULL;

This needs to be goto error, just like in the bdrv_getlength() failure a
few lines above (which is where the uninitialised backup_top warning
comes from).

Kevin



Re: [Qemu-devel] ping Re: [PATCH v4 00/11] backup-top filter driver for backup

2018-11-06 Thread Kevin Wolf
Am 02.11.2018 um 17:41 hat Vladimir Sementsov-Ogievskiy geschrieben:
> ping
> 
> 15.10.2018 19:06, Vladimir Sementsov-Ogievskiy wrote:
> > Hi all!
> >
> > These series introduce backup-top driver. It's a filter-node, which
> > do copy-before-write operation. Mirror uses filter-node for handling
> > guest writes, let's move to filter-node (from write-notifiers) for
> > backup too (patch 16)
> >
> > v4:
> > fixes, rewrite driver to be implicit, drop new interfaces and
> > don't move to BdrvDirtyBitmap for now, as it's not obvious will
> > it be really needed and don't relate to these series more.
> >
> > v3 was "[PATCH v3 00/18] fleecing-hook driver for backup"
> >
> > v2 was "[RFC v2] new, node-graph-based fleecing and backup"
> >
> > These series are based on
> >   [PATCH v4 0/8] dirty-bitmap: rewrite bdrv_dirty_iter_next_area
> > and
> >   [PATCH 0/2] replication: drop extra sync

Before we can move forward here, we need to deal with the dependencies.
I merged the second one now (because there wasn't any feedback from the
replication maintainers), but the first one looks like it was going
through John's or Eric's tree?

Kevin



Re: [Qemu-devel] ping2 Re: [PATCH 0/2] replication: drop extra sync

2018-11-06 Thread Kevin Wolf
Am 31.10.2018 um 10:30 hat Vladimir Sementsov-Ogievskiy geschrieben:
> ping2
> 
> Hi, it's a first step to backup refactoring and improving. Just get rid 
> of this extra and unnatural synchronization.

The replication maintainers don't seem to have an opinion about this, so
thanks, applied to block-next (for 3.2).

Kevin



Re: [Qemu-devel] [PATCH v3 0/4] Adding LZFSE compression support for DMG block driver.

2018-11-06 Thread Kevin Wolf
Am 05.11.2018 um 16:08 hat Julio Faracco geschrieben:
> Since Mac OS X El Capitain (v10.11), Apple uses LZFSE compression to 
> generate compressed DMGs as an alternative to BZIP2. Possible, Apple
> want to keep this algorithm as default in long term. Some years ago, 
> Apple opened the LZFSE algorithm to opensource and the main source (or 
> the most active repo), can be found at: https://github.com/lzfse/lzfse
> 
> v1-v2: Fixing some error handlings from dmg-lzfse.c
> v2-v3: Master rebasing suggestion from Stefan.

Thanks, applied to the block-next branch (for 3.2).

Stefan, if you have some more comments, we can still modify/dequeue the
patches, but this seems to address all of my review comments for v1.

Kevin



Re: [Qemu-devel] [PATCH] blockdev: handle error on block latency histogram set error

2018-11-06 Thread Kevin Wolf
Am 05.11.2018 um 04:04 hat zhenwei pi geschrieben:
> Function block_latency_histogram_set may return error, but qapi ignore this.
> This can be reproduced easily by qmp command:
> virsh qemu-monitor-command INSTANCE 
> '{"execute":"x-block-latency-histogram-set",
> "arguments":{"device":"drive-virtio-disk1","boundaries":[10,200,40]}}'
> In fact this command does not work, but we still get success result.
> 
> qmp_x_block_latency_histogram_set is a batch setting API, report error ASAP.
> 
> Signed-off-by: zhenwei pi 

Thanks, applied to the block branch.

Kevin



Re: [Qemu-devel] [PATCH for-3.1] blockdev: Consistently use snapshot_node_name in external_snapshot_prepare()

2018-11-06 Thread Kevin Wolf
Am 01.11.2018 um 17:30 hat Peter Maydell geschrieben:
> In the function external_snapshot_prepare() we have a
> BlockdevSnapshotSync struct, which has the usual combination
> of has_snapshot_node_name and snapshot_node_name fields for an
> optional field. We set up a local variable
> const char *snapshot_node_name =
> s->has_snapshot_node_name ? s->snapshot_node_name : NULL;
> 
> and then mostly use "if (!snapshot_node_name)" for checking
> whether we have a snapshot node name. The exception is that in
> one place we check s->has_snapshot_node_name instead. This
> confuses Coverity (CID 1396473), which thinks it might be
> possible to get here with s->has_snapshot_node_name true but
> snapshot_node_name NULL, and warns that the call to
> qdict_put_str() will segfault in that case.
> 
> Make the code consistent and unconfuse Coverity by using
> the same check for this conditional that we do in the rest
> of the surrounding code.
> 
> Signed-off-by: Peter Maydell 

Thanks, applied to the block branch.

Kevin



[Qemu-devel] [PATCH for-3.1] target/arm: Remove antique TODO comment

2018-11-06 Thread Peter Maydell
Remove a TODO comment about implementing the vectored interrupt
controller. We have had an implementation of that for a decade;
it's in hw/intc/pl190.c.

Signed-off-by: Peter Maydell 
---
 target/arm/helper.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index ec56becc394..851ea9aa977 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -8378,7 +8378,6 @@ static void arm_cpu_do_interrupt_aarch32(CPUState *cs)
 return;
 }
 
-/* TODO: Vectored interrupt controller.  */
 switch (cs->exception_index) {
 case EXCP_UDEF:
 new_mode = ARM_CPU_MODE_UND;
-- 
2.19.1




[Qemu-devel] [PATCH for-3.1] target/arm: Remove workaround for small SAU regions

2018-11-06 Thread Peter Maydell
Before we supported direct execution from MMIO regions, we
implemented workarounds in commit 720424359917887c926a33d2
which let us avoid doing so, even if the SAU or MPU region
was less than page-sized.

Once we implemented execute-from-MMIO, we removed part
of those workarounds in commit d4b6275df320cee76; but
we forgot the one in get_phys_addr_pmsav8() which
suppressed use of small SAU regions in executable regions.
Remove that workaround now.

Signed-off-by: Peter Maydell 
---
 target/arm/helper.c | 12 
 1 file changed, 12 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 96301930cc8..ec56becc394 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -10560,18 +10560,6 @@ static bool get_phys_addr_pmsav8(CPUARMState *env, 
uint32_t address,
 
 ret = pmsav8_mpu_lookup(env, address, access_type, mmu_idx, phys_ptr,
 txattrs, prot, &mpu_is_subpage, fi, NULL);
-/*
- * TODO: this is a temporary hack to ignore the fact that the SAU region
- * is smaller than a page if this is an executable region. We never
- * supported small MPU regions, but we did (accidentally) allow small
- * SAU regions, and if we now made small SAU regions not be executable
- * then this would break previously working guest code. We can't
- * remove this until/unless we implement support for execution from
- * small regions.
- */
-if (*prot & PAGE_EXEC) {
-sattrs.subpage = false;
-}
 *page_size = sattrs.subpage || mpu_is_subpage ? 1 : TARGET_PAGE_SIZE;
 return ret;
 }
-- 
2.19.1




Re: [Qemu-devel] [Qemu-block] [PATCH 6/7] qcow2: aio support for compressed cluster read

2018-11-06 Thread Vladimir Sementsov-Ogievskiy
06.11.2018 18:30, Alberto Garcia wrote:
> On Tue 06 Nov 2018 04:13:58 PM CET, Vladimir Sementsov-Ogievskiy wrote:
>> 06.11.2018 18:06, Alberto Garcia wrote:
>>> On Thu 01 Nov 2018 07:27:37 PM CET, Vladimir Sementsov-Ogievskiy wrote:
>>>
 +buf = g_try_malloc(csize);
 +if (!buf) {
 +return -ENOMEM;
 +}
 +iov.iov_base = buf;
 +iov.iov_len = csize;
 +qemu_iovec_init_external(&local_qiov, &iov, 1);

 -iov.iov_base = s->cluster_data;
 -iov.iov_len = csize;
 -qemu_iovec_init_external(&local_qiov, &iov, 1);
 +out_buf = qemu_blockalign(bs, s->cluster_size);
>>> You should also check whether out_buf is NULL, shouldn't you?
>> No, it will abort on fail. qemu_try_blockalign result should be
>> checked.
> Is there any reason why some parts of the QEMU code use qemu_blockalign
> and others qemu_try_blockalign() ? From what I can see it seems to be up
> to whoever wrote it...
>
> Berto

As I understand, the good reason to use _try_ versions, is when we are 
allocating some size, taken from user input, so it may be unpredictable 
large (hm, or just any really large allocation), so, I use try_malloc 
for compressed size, which may be very large in somehow corrupted image. 
And it looks a common practice to use not-failing (aborting) allocation 
functions for cluster_size allocations in qcow2.c

-- 
Best regards,
Vladimir



Re: [Qemu-devel] [PULL 02/33] tests: Move tests/hex-loader-check-data/ to tests/data/hex-loader/

2018-11-06 Thread Laurent Vivier
On 06/11/2018 16:15, Philippe Mathieu-Daudé wrote:
> On 6/11/18 15:13, Michael S. Tsirkin wrote:
>> On Tue, Nov 06, 2018 at 02:27:18PM +0100, Philippe Mathieu-Daudé wrote:
>>> On 5/11/18 19:14, Michael S. Tsirkin wrote:
 From: Peter Maydell 

 Currently tests/hex-loader-check-data contains data files used
 by the hexloader-test, and configure individually symlinks those
 data files into the build directory using a wildcard.

 Using a wildcard like this is a bad idea, because if a new
 data file is added, nothing causes configure to be rerun,
 and so no symlink is added for the new file. This can cause
 tests to spuriously fail when they can't find their data.
 Instead, it's better to symlink an entire directory of
 data files. We already have such a directory: tests/data.

 Move the data files from tests/hex-loader-check-data/ to
 tests/data/hex-loader/, and remove the unnecessary symlinking.

 Signed-off-by: Peter Maydell 
>>>
>>> I reviewed/tested this patch too.
>>
>>
>> Thanks a lot Philippe!
>> It is unfortunately too late to update this patch info in git
>> commit history, however your help is still greatly appreciated!
> 
> No worry, I'm not mad at all, but there might be an issue in your git PR
> workflow, this series also missed your maintainer S-o-b.
> 
> Peter: Can you add a such check in your scripts? (during next merge
> window, no hurry).
> 
> Rather than your scripts, this should be in scripts a maintainer can run
> locally, such ./scripts/checkpatch.pl --maintainer or
> ./scripts/checkseries.xx.

I think such tool already exists: with git-publish you can configure a
"pre-publish-send-email" hook, and check your S-o-B is present.

Thanks,
Laurent



[Qemu-devel] [PATCH 1/1] virtio-blk: rename iov to out_iov in virtio_blk_handle_request()

2018-11-06 Thread Dongli Zhang
In virtio_blk_handle_request(), in_iov is used for input header while iov
is used for output header. Rename iov to out_iov to pair output header's
name with in_iov to avoid confusing people when reading source code.

Signed-off-by: Dongli Zhang 
---
 hw/block/virtio-blk.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index 83cf5c0..fb0d74d 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -482,7 +482,7 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, 
MultiReqBuffer *mrb)
 {
 uint32_t type;
 struct iovec *in_iov = req->elem.in_sg;
-struct iovec *iov = req->elem.out_sg;
+struct iovec *out_iov = req->elem.out_sg;
 unsigned in_num = req->elem.in_num;
 unsigned out_num = req->elem.out_num;
 VirtIOBlock *s = req->dev;
@@ -493,13 +493,13 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, 
MultiReqBuffer *mrb)
 return -1;
 }
 
-if (unlikely(iov_to_buf(iov, out_num, 0, &req->out,
+if (unlikely(iov_to_buf(out_iov, out_num, 0, &req->out,
 sizeof(req->out)) != sizeof(req->out))) {
 virtio_error(vdev, "virtio-blk request outhdr too short");
 return -1;
 }
 
-iov_discard_front(&iov, &out_num, sizeof(req->out));
+iov_discard_front(&out_iov, &out_num, sizeof(req->out));
 
 if (in_iov[in_num - 1].iov_len < sizeof(struct virtio_blk_inhdr)) {
 virtio_error(vdev, "virtio-blk request inhdr too short");
@@ -526,7 +526,7 @@ static int virtio_blk_handle_request(VirtIOBlockReq *req, 
MultiReqBuffer *mrb)
&req->out.sector);
 
 if (is_write) {
-qemu_iovec_init_external(&req->qiov, iov, out_num);
+qemu_iovec_init_external(&req->qiov, out_iov, out_num);
 trace_virtio_blk_handle_write(vdev, req, req->sector_num,
   req->qiov.size / BDRV_SECTOR_SIZE);
 } else {
-- 
2.7.4




Re: [Qemu-devel] [PULL 02/33] tests: Move tests/hex-loader-check-data/ to tests/data/hex-loader/

2018-11-06 Thread Michael S. Tsirkin
On Tue, Nov 06, 2018 at 04:15:03PM +0100, Philippe Mathieu-Daudé wrote:
> On 6/11/18 15:13, Michael S. Tsirkin wrote:
> > On Tue, Nov 06, 2018 at 02:27:18PM +0100, Philippe Mathieu-Daudé wrote:
> > > On 5/11/18 19:14, Michael S. Tsirkin wrote:
> > > > From: Peter Maydell 
> > > > 
> > > > Currently tests/hex-loader-check-data contains data files used
> > > > by the hexloader-test, and configure individually symlinks those
> > > > data files into the build directory using a wildcard.
> > > > 
> > > > Using a wildcard like this is a bad idea, because if a new
> > > > data file is added, nothing causes configure to be rerun,
> > > > and so no symlink is added for the new file. This can cause
> > > > tests to spuriously fail when they can't find their data.
> > > > Instead, it's better to symlink an entire directory of
> > > > data files. We already have such a directory: tests/data.
> > > > 
> > > > Move the data files from tests/hex-loader-check-data/ to
> > > > tests/data/hex-loader/, and remove the unnecessary symlinking.
> > > > 
> > > > Signed-off-by: Peter Maydell 
> > > 
> > > I reviewed/tested this patch too.
> > 
> > 
> > Thanks a lot Philippe!
> > It is unfortunately too late to update this patch info in git
> > commit history, however your help is still greatly appreciated!
> 
> No worry, I'm not mad at all, but there might be an issue in your git PR
> workflow, this series also missed your maintainer S-o-b.

It's just that I could not figure out the failures that were blocking
the pull, so when I saw that Peter finally posted the fix I rushed to
merge and test it and didn't look for any acks.  My mistake, sorry about
that. That's also why I forgot to sign it.

> Peter: Can you add a such check in your scripts? (during next merge window,
> no hurry).
> 
> Rather than your scripts, this should be in scripts a maintainer can run
> locally, such ./scripts/checkpatch.pl --maintainer or
> ./scripts/checkseries.xx.
> 
> > 
> > 
> > > > ---
> > > >configure | 4 
> > > >tests/hexloader-test.c| 2 +-
> > > >MAINTAINERS   | 2 +-
> > > >tests/{hex-loader-check-data => data/hex-loader}/test.hex | 0
> > > >4 files changed, 2 insertions(+), 6 deletions(-)
> > > >rename tests/{hex-loader-check-data => data/hex-loader}/test.hex 
> > > > (100%)
> > > > 
> > > > diff --git a/configure b/configure
> > > > index 895b7483b8..bfdca8b814 100755
> > > > --- a/configure
> > > > +++ b/configure
> > > > @@ -7421,10 +7421,6 @@ for bios_file in \
> > > >do
> > > >FILES="$FILES pc-bios/$(basename $bios_file)"
> > > >done
> > > > -for test_file in $(find $source_path/tests/hex-loader-check-data -type 
> > > > f)
> > > > -do
> > > > -FILES="$FILES tests/hex-loader-check-data$(echo $test_file | sed 
> > > > -e 's/.*hex-loader-check-data//')"
> > > > -done
> > > >mkdir -p $DIRS
> > > >for f in $FILES ; do
> > > >if [ -e "$source_path/$f" ] && [ "$pwd_is_source_path" != "y" ]; 
> > > > then
> > > > diff --git a/tests/hexloader-test.c b/tests/hexloader-test.c
> > > > index b653d44ba1..834ed52c22 100644
> > > > --- a/tests/hexloader-test.c
> > > > +++ b/tests/hexloader-test.c
> > > > @@ -23,7 +23,7 @@ static void hex_loader_test(void)
> > > >const unsigned int base_addr = 0x0001;
> > > >QTestState *s = qtest_initf(
> > > > -"-M vexpress-a9 -nographic -device 
> > > > loader,file=tests/hex-loader-check-data/test.hex");
> > > > +"-M vexpress-a9 -nographic -device 
> > > > loader,file=tests/data/hex-loader/test.hex");
> > > >for (i = 0; i < 256; ++i) {
> > > >uint8_t val = qtest_readb(s, base_addr + i);
> > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > index 98a1856afc..cfabc14b59 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -1370,7 +1370,7 @@ Intel Hexadecimal Object File Loader
> > > >M: Su Hang 
> > > >S: Maintained
> > > >F: tests/hexloader-test.c
> > > > -F: tests/hex-loader-check-data/test.hex
> > > > +F: tests/data/hex-loader/test.hex
> > > >CHRP NVRAM
> > > >M: Thomas Huth 
> > > > diff --git a/tests/hex-loader-check-data/test.hex 
> > > > b/tests/data/hex-loader/test.hex
> > > > similarity index 100%
> > > > rename from tests/hex-loader-check-data/test.hex
> > > > rename to tests/data/hex-loader/test.hex
> > > > 



Re: [Qemu-devel] [PULL 02/33] tests: Move tests/hex-loader-check-data/ to tests/data/hex-loader/

2018-11-06 Thread Michael S. Tsirkin
On Tue, Nov 06, 2018 at 03:31:08PM +, Peter Maydell wrote:
> On 6 November 2018 at 15:15, Philippe Mathieu-Daudé  wrote:
> > No worry, I'm not mad at all, but there might be an issue in your git PR
> > workflow, this series also missed your maintainer S-o-b.
> >
> > Peter: Can you add a such check in your scripts? (during next merge window,
> > no hurry).
> >
> > Rather than your scripts, this should be in scripts a maintainer can run
> > locally, such ./scripts/checkpatch.pl --maintainer or
> > ./scripts/checkseries.xx.
> 
> I have such a check in my own make-pullreq script which I use
> for arm pull requests:
> https://git.linaro.org/people/peter.maydell/misc-scripts.git/tree/make-pullreq#n98
> 
> I can't do much about other submaintainers' workflows
> (and the block tree's "sub-submaintainers" setup means
> "check all commits have a signoff from the same person as
> the merge commit did" won't work either.)
> 
> thanks
> -- PMM

I don't have anything validating that but I agree it's a good idea.
Will add, thanks!




Re: [Qemu-devel] [PATCH 1/2] nvme: don't unref ctrl_mem when device unrealized

2018-11-06 Thread Igor Mammedov
On Sun, 28 Oct 2018 23:29:40 -0700
Li Qiang  wrote:

> Currently, when hotplug/unhotplug nvme device, it will cause an
> assert in object.c. Following is the backtrack:
> 
> ERROR:qom/object.c:981:object_unref: assertion failed: (obj->ref > 0)
> 
> Thread 2 "qemu-system-x86" received signal SIGABRT, Aborted.
> [Switching to Thread 0x7fffcbd32700 (LWP 18844)]
> 0x7fffdb9e4fff in raise () from /lib/x86_64-linux-gnu/libc.so.6
> (gdb) bt
> /lib/x86_64-linux-gnu/libglib-2.0.so.0
> /lib/x86_64-linux-gnu/libglib-2.0.so.0
> qom/object.c:981
> /home/liqiang02/qemu-upstream/qemu/memory.c:1732
> /home/liqiang02/qemu-upstream/qemu/memory.c:285
> util/qemu-thread-posix.c:504
> /lib/x86_64-linux-gnu/libpthread.so.0
> 
> This is caused by memory_region_unref in nvme_exit.
> 
> Remove it to make the PCIdevice refcount correct.
> 
> Signed-off-by: Li Qiang 
nvme device holds a reference to ctrl_mem MemoryRegion as a parent
so MemoryRegion will be destroyed later during destruction of
nvme object when its cildren are un-parented.

Reviewed-by: Igor Mammedov 

> ---
>  hw/block/nvme.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/hw/block/nvme.c b/hw/block/nvme.c
> index fc7dacb816..359a06d0ad 100644
> --- a/hw/block/nvme.c
> +++ b/hw/block/nvme.c
> @@ -1331,9 +1331,6 @@ static void nvme_exit(PCIDevice *pci_dev)
>  g_free(n->namespaces);
>  g_free(n->cq);
>  g_free(n->sq);
> -if (n->cmbsz) {
> -memory_region_unref(&n->ctrl_mem);
> -}
>  
>  msix_uninit_exclusive_bar(pci_dev);
>  }




Re: [Qemu-devel] [PULL 0/8] Monitor patches for 2018-10-30

2018-11-06 Thread Markus Armbruster
Peter Maydell  writes:

> On 30 October 2018 at 19:16, Markus Armbruster  wrote:
>> The following changes since commit 3f3285491dd52014852a56135c90e428c8b507ea:
>>
>>   Merge remote-tracking branch 'remotes/jnsnow/tags/bitmaps-pull-request' 
>> into staging (2018-10-30 14:09:25 +)
>>
>> are available in the Git repository at:
>>
>>   git://repo.or.cz/qemu/armbru.git tags/pull-monitor-2018-10-30
>>
>> for you to fetch changes up to 1a8de278dd3c93a7aa40f88486509fe0e2de04da:
>>
>>   vl: Avoid crash when -mon is underspecified (2018-10-30 20:13:52 +0100)
>>
>> 
>> Monitor patches for 2018-10-30
>>
>> * Fix crash on shutdown with --daemonize
>> * Change out-of-band execution to stop reading instead of dropping
>>   commands
>> * Enable out-of-band execution by default
>> * Avoid crash when -mon lacks chardev=...
>>
>> 
>
> Hi; I get test failures on FreeBSD host:
>
> TEST: tests/qmp-test... (pid=18305)
>   /sparc64/qmp/protocol:   OK
>   /sparc64/qmp/oob:FAIL
> GTester: last random seed: R02Se36038cab4c051a2cd47374ec9718e98
> (pid=18337)
>   /sparc64/qmp/preconfig:  OK
> FAIL: tests/qmp-test
>
> ERROR:tests/qmp-test.c:211:recv_cmd_id: assertion failed
> (qdict_get_try_str(resp, "id") == id): ("oob-1" == "queue-blocks-1")
> gmake: *** [/var/tmp/qemu-test.2n2tBU/tests/Makefile.include:805:
> check-qtest-sparc] Error 1
>
> and similarly with i386/qmp/oob and sparc/qmp/oob.

I haven't been able to reproduce, yet.  I'm going to respin with fewer
patches, to get the other stuff in while I debug.



Re: [Qemu-devel] [PATCH 0/4] scsi-generic: fixes for Block Limits emulation

2018-11-06 Thread Daniel Henrique Barboza

Hi,

How did you find all those issues, Max? First patch is something that
I missed out from the SCSI spec (ordering of the VPD pages)
and could have been detected by code inspection,  but I am curious about
the other fixes.


Thanks,


Daniel

On 10/29/18 2:34 PM, Paolo Bonzini wrote:

scsi-generic (pass through) devices are able to inject an artificial Block
Limits VPD page in order to communicate host HBA limits to the guest.
However, Max Reitz found a few issues with the implementation of this
feature; this series should fix them all.

Paolo

Paolo Bonzini (4):
   scsi-generic: keep VPD page list sorted
   scsi-generic: avoid out-of-bounds access to VPD page list
   scsi-generic: avoid invalid access to struct when emulating block
 limits
   scsi-generic: do not do VPD emulation for sense other than
 ILLEGAL_REQUEST

  hw/scsi/Makefile.objs   |  2 +-
  hw/scsi/emulation.c | 42 +
  hw/scsi/scsi-disk.c | 92 -
  hw/scsi/scsi-generic.c  | 51 +++-
  include/hw/scsi/emulation.h | 16 +++
  include/hw/scsi/scsi.h  |  1 -
  6 files changed, 119 insertions(+), 85 deletions(-)
  create mode 100644 hw/scsi/emulation.c
  create mode 100644 include/hw/scsi/emulation.h





Re: [Qemu-devel] [Libguestfs] How to emulate block I/O timeout on qemu side?

2018-11-06 Thread Richard W.M. Jones
On Tue, Nov 06, 2018 at 09:43:06AM +, Richard W.M. Jones wrote:
> On Tue, Nov 06, 2018 at 09:14:57AM +, Richard W.M. Jones wrote:
> > This link shows how to combine delay and error filters together:
> > 
> >   https://rwmj.wordpress.com/2018/11/04/nbd-graphical-viewer/
> 
> Oops, that's in a forthcoming blog post not this one.  Not enough
> caffeine this morning.

Up now:

https://rwmj.wordpress.com/2018/11/06/nbd-graphical-viewer-raid-5-edition/

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org



Re: [Qemu-devel] [PULL 00/33] pci, pc, virtio: fixes, features

2018-11-06 Thread Laszlo Ersek
On 11/06/18 13:39, Peter Maydell wrote:
> On 6 November 2018 at 11:20, Peter Maydell  wrote:
>> On 6 November 2018 at 11:07, Michael S. Tsirkin  wrote:
>>> On Tue, Nov 06, 2018 at 09:18:49AM +0100, Thomas Huth wrote:
 On 2018-11-05 19:14, Michael S. Tsirkin wrote:
> The following changes since commit 
> b2f7a038bb4c4fc5ce6b8486e8513dfd97665e2a:
>
>   Merge remote-tracking branch 'remotes/rth/tags/pull-softfloat-20181104' 
> into staging (2018-11-05 10:32:49 +)
>
> are available in the Git repository at:
>
>   git://git.kernel.org/pub/scm/virt/kvm/mst/qemu.git tags/for_upstream
>
> for you to fetch changes up to 6196df5c8e6688c1c3f06f73442820066335337c:
>
>   vhost-scsi: prevent using uninitialized vqs (2018-11-05 12:59:35 -0500)
>
> 
> pci, pc, virtio: fixes, features
>
> AMD IOMMU VAPIC support + fixes all over the place.
>
> Signed-off-by: Michael S. Tsirkin 
>
> 
> Gerd Hoffmann (1):
>   pci-testdev: add optional memory bar
>
> Laszlo Ersek (4):
>   MAINTAINERS: list "tests/acpi-test-data" files in ACPI/SMBIOS 
> section
 [...]
>  tests/{acpi-test-data => data/acpi}/pc/APIC| Bin
>  tests/{acpi-test-data => data/acpi}/pc/APIC.cphp   | Bin
>  .../{acpi-test-data => data/acpi}/pc/APIC.dimmpxm  | Bin
>  tests/{acpi-test-data => data/acpi}/pc/DSDT| Bin

 So patch 1 moves tests/acpi-test-data/ to tests/data/acpi/ and patch 20
 adds an entry for tests/acpi-test-data/ ? Does not make much sense to me
 ... I think patch 20 needs to be adapted now.

  Thomas
>>>
>>> Oh right, MAINTAINERS needs to be fixed. Can be done with a patch on top
>>> though.
>>
>> Yeah, given the timing for rc0 I'll just apply this version of
>> the pullreq, and we can fix up MAINTAINERS afterwards.
> 
> ...applied.

Thanks!
Laszlo



Re: [Qemu-devel] [RESEND PATCH for-3.1] hw/arm/sysbus-fdt: Only call match_fn callback if the type matches

2018-11-06 Thread Peter Maydell
On 5 November 2018 at 15:35, Eric Auger  wrote:
> Commit af7d64ede0b9 (hw/arm/sysbus-fdt: Allow device matching with DT
> compatible value) introduced a match_fn callback which gets called
> for each registered combo to check whether a sysbus device can be
> dynamically instantiated. However the callback gets called even if
> the device type does not match the binding combo typename field.
>
> Let's change the add_fdt_node() logic so that we first check the
> type and if the match_fn callback is defined, then we also call it.
>
> Binding combos only requesting a type check do not define the
> match_fn callback.
>
> Fixes: af7d64ede0b9 (hw/arm/sysbus-fdt: Allow device matching with
> DT compatible value)
>
> Signed-off-by: Eric Auger 
> Reported-by: Thomas Huth 
> ---
>  hw/arm/sysbus-fdt.c | 18 +-
>  1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/hw/arm/sysbus-fdt.c b/hw/arm/sysbus-fdt.c
> index 0e24c803a1..a44cf7f255 100644
> --- a/hw/arm/sysbus-fdt.c
> +++ b/hw/arm/sysbus-fdt.c
> @@ -449,7 +449,7 @@ static bool type_match(SysBusDevice *sbdev, const 
> BindingEntry *entry)
>  return !strcmp(object_get_typename(OBJECT(sbdev)), entry->typename);
>  }
>
> -#define TYPE_BINDING(type, add_fn) {(type), NULL, (add_fn), type_match}
> +#define TYPE_BINDING(type, add_fn) {(type), NULL, (add_fn), NULL}
>
>  /* list of supported dynamic sysbus bindings */
>  static const BindingEntry bindings[] = {
> @@ -481,10 +481,18 @@ static void add_fdt_node(SysBusDevice *sbdev, void 
> *opaque)
>  for (i = 0; i < ARRAY_SIZE(bindings); i++) {
>  const BindingEntry *iter = &bindings[i];
>
> -if (iter->match_fn(sbdev, iter)) {
> -ret = iter->add_fn(sbdev, opaque);
> -assert(!ret);
> -return;
> +if (type_match(sbdev, iter)) {
> +if (iter->match_fn) {
> +if (iter->match_fn(sbdev, iter)) {

"if (!iter->match_fn || iter->match_fn(sbdev, iter)) {"

would let you avoid duplicating the code in the body
of this if and the else clause later.

(Or you could have a match_always() function that always
returns true instead of having to special case NULL.)

> +ret = iter->add_fn(sbdev, opaque);
> +assert(!ret);
> +return;
> +}
> +} else {
> +ret = iter->add_fn(sbdev, opaque);
> +assert(!ret);
> +return;
> +}
>  }

thanks
-- PMM



  1   2   3   >