date:20201003

On 02/10/20 22:37, Peter Maydell wrote:
> Found CMake: /usr/bin/cmake (2.8.12.2)
> WARNING: The version of CMake /usr/bin/cmake is 2.8.12.2 but version
>> =3.4 is required
> Run-time dependency capstone found: NO (tried pkgconfig and cmake)
> Configuring capstone-defs.h using configuration
> Configuring config-host.h using configuration
> 
> We shouldn't be looking for or using cmake at all.

This is a missing "method: 'pkg-config'".

Paolo

Re: [PULL v2 00/11] capstone + disassembler patch queue

On 02/10/20 22:37, Peter Maydell wrote:
> ../src/meson.build:753: WARNING: Trying to compare values of different
> types (bool, str) using ==.
> The result of this is undefined and will become a hard error in a
> future Meson release.
> Configuring config-host.h using configuration
> Program scripts/hxtool found: YES
> Program scripts/shaderinclude.pl found: YES
> Program scripts/qapi-gen.py found: YES
> Program scripts/qemu-version.sh found: YES
> Run-time dependency threads found: YES
> Program keycodemapdb/tools/keymap-gen found: YES
> Program scripts/decodetree.py found: YES

This can be rewritten like

-if capstone_opt == 'disabled'
-  capstone_opt = false
-elif capstone_opt in ['enabled', 'auto', 'system']
+if capstone_opt in ['enabled', 'auto', 'system']
   have_internal = fs.exists('capstone/Makefile')
   capstone = dependency('capstone', static: enable_static,
 required: capstone_opt == 'system' or
   capstone_opt == 'enabled' and not 
have_internal)
   if capstone.found()
 capstone_opt = 'system'
   elif have_internal
 capstone_opt = 'internal'
   else
-capstone_opt = false
+capstone_opt = 'disabled'
   endif
 endif
 if capstone_opt == 'internal'

...
-summary_info += {'capstone':  capstone_opt}
+summary_info += {'capstone':  capstone_opt == 'disabled' ? false : 
capstone_opt}


That said, this also showed a bug which can be fixed like this:

-  have_internal = fs.exists('capstone/Makefile')
+  have_internal = fs.exists(meson.current_source_dir() / 'capstone/Makefile')

Paolo

Re: [PATCH] meson.build: Don't look for libudev for static builds

On Fri, Oct 2, 2020 at 9:11 PM Peter Maydell 
wrote:
>
> On Fri, 2 Oct 2020 at 14:05, Paolo Bonzini  wrote:
> >
> > On 02/10/20 14:35, Peter Maydell wrote:
> > >
> > > It would be better to do the "see if a static library is present"
> > > test. This isn't too hard to do in configure (compare that
> > > six line fix to the detection of libgio). Hopefully it is
> > > not too hard to do in meson ?
> >
> > Yes, something like:
> >
> > if enable_static
> >   skeleton = 'int main(void) { return 0; }'
> >   if not cc.links(skeleton, dependencies: libudev)
> > if get_option('mpath').enabled()
> > error('Cannot link with libudev')
> >   else
> > warning('Cannot link with libudev, disabling')
> > libudev = not_found
> >   endif
> > endif
> >   endif
> > endif
>
> This duplicates the information that the thing that depends
> on libudev is mpath. Can we put this in a wrapper around
> dependency() so that we could just say something like
>   libudev = compile_checked_dependency('libudev',
>required: get_option('mpath').enabled(),
>static: enable_static)
>
Hi Bonzini,
This looks like a frequently used function, can we upstrem to meson?


> for those dependencies that want to do the "does this compile"
> check ?
>
> thanks
> -- PMM
>


--
 此致
礼
罗勇刚
Yours
sincerely,
Yonggang Luo

Re: [PATCH] meson.build: Don't look for libudev for static builds

On 03/10/20 09:24, 罗勇刚(Yonggang Luo) wrote:
> 
> 
> On Fri, Oct 2, 2020 at 9:11 PM Peter Maydell  > wrote:
>>
>> On Fri, 2 Oct 2020 at 14:05, Paolo Bonzini  > wrote:
>> >
>> > On 02/10/20 14:35, Peter Maydell wrote:
>> > >
>> > > It would be better to do the "see if a static library is present"
>> > > test. This isn't too hard to do in configure (compare that
>> > > six line fix to the detection of libgio). Hopefully it is
>> > > not too hard to do in meson ?
>> >
>> > Yes, something like:
>> >
>> > if enable_static
>> >   skeleton = 'int main(void) { return 0; }'
>> >   if not cc.links(skeleton, dependencies: libudev)
>> >     if get_option('mpath').enabled()
>> >         error('Cannot link with libudev')
>> >       else
>> >         warning('Cannot link with libudev, disabling')
>> >         libudev = not_found
>> >       endif
>> >     endif
>> >   endif
>> > endif
>>
>> This duplicates the information that the thing that depends
>> on libudev is mpath. Can we put this in a wrapper around
>> dependency() so that we could just say something like
>>   libudev = compile_checked_dependency('libudev',
>>                        required: get_option('mpath').enabled(),
>>                        static: enable_static)
>>
> Hi Bonzini,
> This looks like a frequently used function, can we upstrem to meson?

Yes, I think adding a "links" argument to dependency (similar to
find_library's has_headers argument) makes sense.  That would be written

dependency('libudev',
   required: get_option('mpath').enabled(),
   static: enable_static,
   links: skeleton)

But anyway that shouldn't be a blocker for more improvements to qemu's
meson.build.  Now that we have 5-10 dependencies converted we have a
clearer idea of how to abstract the tests.

Paolo

[Bug 1894804] Re: Second DEVICE_DELETED event missing during virtio-blk disk device detach

2020-10-03 Thread Christian Ehrhardt 

I was seeing in my manual tests that the delete->Event was taking a bit
longer, maybe your retry logic kicks in before it is complete now. And
due to that the problem takes place.

So with the PPA you should get:
a) a warning, but no more the cancelled/undefined behavior on double delete
b) can tune your retry logic to no more trigger the warning messages

So I wasn't sure from the logs - is the fix in the PPA good to overcome
the issue completely or is it missing something?

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

Title:
  Second DEVICE_DELETED event missing during virtio-blk disk device
  detach

Status in QEMU:
  New
Status in qemu package in Ubuntu:
  Incomplete

Bug description:
  We are in the process of moving OpenStack CI across to use 20.04 Focal
  as the underlying OS and encountering the following issue in any test
  attempting to detach disk devices from running QEMU instances.

  We can see a single DEVICE_DELETED event raised to libvirtd for the
  /machine/peripheral/virtio-disk1/virtio-backend device but we do not
  see a second event for the actual disk. As a result the device is
  still marked as present in libvirt but QEMU reports it as missing in
  subsequent attempts to remove the device.

  The following log snippets can also be found in the following pastebin
  that's slightly easier to gork:

  http://paste.openstack.org/show/797564/

  https://review.opendev.org/#/c/746981/ libvirt: Bump
  MIN_{LIBVIRT,QEMU}_VERSION and NEXT_MIN_{LIBVIRT,QEMU}_VERSION

  https://zuul.opendev.org/t/openstack/build/4c56def513884c5eb3ba7b0adf7fa260
  nova-ceph-multistore

  
https://zuul.opendev.org/t/openstack/build/4c56def513884c5eb3ba7b0adf7fa260/log/controller/logs/dpkg-l.txt

  ii  libvirt-daemon   6.0.0-0ubuntu8.3 
 amd64Virtualization daemon
  ii  libvirt-daemon-driver-qemu   6.0.0-0ubuntu8.3 
 amd64Virtualization daemon QEMU connection driver
  ii  libvirt-daemon-system6.0.0-0ubuntu8.3 
 amd64Libvirt daemon configuration files
  ii  libvirt-daemon-system-systemd6.0.0-0ubuntu8.3 
 amd64Libvirt daemon configuration files (systemd)
  ii  libvirt-dev:amd646.0.0-0ubuntu8.3 
 amd64development files for the libvirt library
  ii  libvirt0:amd64   6.0.0-0ubuntu8.3 
 amd64library for interfacing with different virtualization systems
  [..]
  ii  qemu-block-extra:amd64   1:4.2-3ubuntu6.4 
 amd64extra block backend modules for qemu-system and qemu-utils
  ii  qemu-slof20191209+dfsg-1  
 all  Slimline Open Firmware -- QEMU PowerPC version
  ii  qemu-system  1:4.2-3ubuntu6.4 
 amd64QEMU full system emulation binaries
  ii  qemu-system-arm  1:4.2-3ubuntu6.4 
 amd64QEMU full system emulation binaries (arm)
  ii  qemu-system-common   1:4.2-3ubuntu6.4 
 amd64QEMU full system emulation binaries (common files)
  ii  qemu-system-data 1:4.2-3ubuntu6.4 
 all  QEMU full system emulation (data files)
  ii  qemu-system-mips 1:4.2-3ubuntu6.4 
 amd64QEMU full system emulation binaries (mips)
  ii  qemu-system-misc 1:4.2-3ubuntu6.4 
 amd64QEMU full system emulation binaries (miscellaneous)
  ii  qemu-system-ppc  1:4.2-3ubuntu6.4 
 amd64QEMU full system emulation binaries (ppc)
  ii  qemu-system-s390x1:4.2-3ubuntu6.4 
 amd64QEMU full system emulation binaries (s390x)
  ii  qemu-system-sparc1:4.2-3ubuntu6.4 
 amd64QEMU full system emulation binaries (sparc)
  ii  qemu-system-x86  1:4.2-3ubuntu6.4 
 amd64QEMU full system emulation binaries (x86)
  ii  qemu-utils   1:4.2-3ubuntu6.4 
 amd64QEMU utilities

  
https://zuul.opendev.org/t/openstack/build/4c56def513884c5eb3ba7b0adf7fa260/log/controller/logs/libvirt/qemu
  /instance-003a_log.txt

  2020-09-07 19:29:55.021+: starting up libvirt version: 6.0.0, package: 
0ubuntu8.3 (Marc Deslauriers  Thu, 30 Jul 2020 
06:40:28 -0400), qemu version: 4.2.0Debian 1:4.2-3ubuntu6.4, kernel: 
5.4.0-45-generic, hostname: ubuntu-focal-ovh-bhs1-0019682147
  LC_ALL=C \
  PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin \
  HOME=/var/lib/libvirt/qemu/domain-86-instance-003a \
  XDG_DATA

Re: [PATCH] meson.build: Don't look for libudev for static builds

On Sat, Oct 3, 2020 at 3:50 PM Paolo Bonzini  wrote:
>
> On 03/10/20 09:24, 罗勇刚(Yonggang Luo) wrote:
> >
> >
> > On Fri, Oct 2, 2020 at 9:11 PM Peter Maydell  > > wrote:
> >>
> >> On Fri, 2 Oct 2020 at 14:05, Paolo Bonzini  > > wrote:
> >> >
> >> > On 02/10/20 14:35, Peter Maydell wrote:
> >> > >
> >> > > It would be better to do the "see if a static library is present"
> >> > > test. This isn't too hard to do in configure (compare that
> >> > > six line fix to the detection of libgio). Hopefully it is
> >> > > not too hard to do in meson ?
> >> >
> >> > Yes, something like:
> >> >
> >> > if enable_static
> >> >   skeleton = 'int main(void) { return 0; }'
> >> >   if not cc.links(skeleton, dependencies: libudev)
> >> > if get_option('mpath').enabled()
> >> > error('Cannot link with libudev')
> >> >   else
> >> > warning('Cannot link with libudev, disabling')
> >> > libudev = not_found
> >> >   endif
> >> > endif
> >> >   endif
> >> > endif
> >>
> >> This duplicates the information that the thing that depends
> >> on libudev is mpath. Can we put this in a wrapper around
> >> dependency() so that we could just say something like
> >>   libudev = compile_checked_dependency('libudev',
> >>required: get_option('mpath').enabled(),
> >>static: enable_static)
> >>
> > Hi Bonzini,
> > This looks like a frequently used function, can we upstrem to meson?
>
> Yes, I think adding a "links" argument to dependency (similar to
> find_library's has_headers argument) makes sense.  That would be written
>
> dependency('libudev',
>required: get_option('mpath').enabled(),
>static: enable_static,
>links: skeleton)
make sense, may also need extra cflags and link flags.
>
> But anyway that shouldn't be a blocker for more improvements to qemu's
> meson.build.  Now that we have 5-10 dependencies converted we have a
> clearer idea of how to abstract the tests.
>
> Paolo
>


--
 此致
礼
罗勇刚
Yours
sincerely,
Yonggang Luo

Re: [PATCH] meson.build: Don't look for libudev for static builds

On Sat, Oct 3, 2020 at 3:50 PM Paolo Bonzini  wrote:
>
> On 03/10/20 09:24, 罗勇刚(Yonggang Luo) wrote:
> >
> >
> > On Fri, Oct 2, 2020 at 9:11 PM Peter Maydell  > > wrote:
> >>
> >> On Fri, 2 Oct 2020 at 14:05, Paolo Bonzini  > > wrote:
> >> >
> >> > On 02/10/20 14:35, Peter Maydell wrote:
> >> > >
> >> > > It would be better to do the "see if a static library is present"
> >> > > test. This isn't too hard to do in configure (compare that
> >> > > six line fix to the detection of libgio). Hopefully it is
> >> > > not too hard to do in meson ?
> >> >
> >> > Yes, something like:
> >> >
> >> > if enable_static
> >> >   skeleton = 'int main(void) { return 0; }'
> >> >   if not cc.links(skeleton, dependencies: libudev)
> >> > if get_option('mpath').enabled()
> >> > error('Cannot link with libudev')
> >> >   else
> >> > warning('Cannot link with libudev, disabling')
> >> > libudev = not_found
> >> >   endif
> >> > endif
> >> >   endif
> >> > endif
> >>
> >> This duplicates the information that the thing that depends
> >> on libudev is mpath. Can we put this in a wrapper around
> >> dependency() so that we could just say something like
> >>   libudev = compile_checked_dependency('libudev',
> >>required: get_option('mpath').enabled(),
> >>static: enable_static)
> >>
> > Hi Bonzini,
> > This looks like a frequently used function, can we upstrem to meson?
>
> Yes, I think adding a "links" argument to dependency (similar to
> find_library's has_headers argument) makes sense.  That would be written
>
> dependency('libudev',
>required: get_option('mpath').enabled(),
>static: enable_static,
>links: skeleton)
>
For some meson script like this:
curses = not_found
if iconv.found() and not get_option('curses').disabled()
  curses_libname_list = ['ncursesw', 'ncurses', 'cursesw', 'pdcurses']
  curses_test = '''
#include 
#include 
#include 
int main(void) {
  wchar_t wch = L'w';
  setlocale(LC_ALL, "");
  resize_term(0, 0);
  addwstr(L"wide chars\n");
  addnwstr(&wch, 1);
  add_wch(WACS_DEGREE);
  return 0;
}'''
  foreach curses_libname : curses_libname_list
  libcurses = dependency(curses_libname,
 required: false,
 method: 'pkg-config',
 static: enable_static)

  if not libcurses.found()
dirs = ['/usr/include/ncursesw']
if targetos == 'windows'
  dirs = []
endif
libcurses = cc.find_library(curses_libname,
required: false,
dirs: dirs,
static: enable_static)
  endif
  if libcurses.found()
if cc.links(curses_test, dependencies: [libcurses])
  curses = declare_dependency(compile_args: '-DNCURSES_WIDECHAR',
dependencies: [libcurses])
  break
endif
  endif
  endforeach
endif

We also need to define extra  compile_args  '-DNCURSES_WIDECHAR' as the
part of dependencies.

> But anyway that shouldn't be a blocker for more improvements to qemu's
> meson.build.  Now that we have 5-10 dependencies converted we have a
> clearer idea of how to abstract the tests.
>
> Paolo
>


--
 此致
礼
罗勇刚
Yours
sincerely,
Yonggang Luo

Re: [PATCH v2 2/2] vhost: Don't call log_access_ok() when using IOTLB

On Sat, 3 Oct 2020 09:58:59 +0800
Jason Wang  wrote:

> 
> On 2020/9/30 上午12:30, Greg Kurz wrote:
> > When the IOTLB device is enabled, the log_guest_addr that is passed by
> > userspace to the VHOST_SET_VRING_ADDR ioctl, and which is then written
> > to vq->log_addr, is a GIOVA. All writes to this address are translated
> > by log_user() to writes to an HVA, and then ultimately logged through
> > the corresponding GPAs in log_write_hva(). No logging will ever occur
> > with vq->log_addr in this case. It is thus wrong to pass vq->log_addr
> > and log_guest_addr to log_access_vq() which assumes they are actual
> > GPAs.
> >
> > Introduce a new vq_log_used_access_ok() helper that only checks accesses
> > to the log for the used structure when there isn't an IOTLB device around.
> >
> > Signed-off-by: Greg Kurz 
> > ---
> >   drivers/vhost/vhost.c |   23 +++
> >   1 file changed, 19 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > index c3b49975dc28..5996e32fa818 100644
> > --- a/drivers/vhost/vhost.c
> > +++ b/drivers/vhost/vhost.c
> > @@ -1370,6 +1370,20 @@ bool vhost_log_access_ok(struct vhost_dev *dev)
> >   }
> >   EXPORT_SYMBOL_GPL(vhost_log_access_ok);
> >   
> > +static bool vq_log_used_access_ok(struct vhost_virtqueue *vq,
> > + void __user *log_base,
> > + bool log_used,
> > + u64 log_addr,
> > + size_t log_size)
> > +{
> > +   /* If an IOTLB device is present, log_addr is a GIOVA that
> > +* will never be logged by log_used(). */
> > +   if (vq->iotlb)
> > +   return true;
> > +
> > +   return !log_used || log_access_ok(log_base, log_addr, log_size);
> > +}
> > +
> >   /* Verify access for write logging. */
> >   /* Caller should have vq mutex and device mutex */
> >   static bool vq_log_access_ok(struct vhost_virtqueue *vq,
> > @@ -1377,8 +1391,8 @@ static bool vq_log_access_ok(struct vhost_virtqueue 
> > *vq,
> >   {
> > return vq_memory_access_ok(log_base, vq->umem,
> >vhost_has_feature(vq, VHOST_F_LOG_ALL)) &&
> > -   (!vq->log_used || log_access_ok(log_base, vq->log_addr,
> > - vhost_get_used_size(vq, vq->num)));
> > +   vq_log_used_access_ok(vq, log_base, vq->log_used, vq->log_addr,
> > + vhost_get_used_size(vq, vq->num));
> >   }
> >   
> >   /* Can we start vq? */
> > @@ -1517,8 +1531,9 @@ static long vhost_vring_set_addr(struct vhost_dev *d,
> > return -EINVAL;
> >   
> > /* Also validate log access for used ring if enabled. */
> > -   if ((a.flags & (0x1 << VHOST_VRING_F_LOG)) &&
> > -   !log_access_ok(vq->log_base, a.log_guest_addr,
> > +   if (!vq_log_used_access_ok(vq, vq->log_base,
> > +   a.flags & (0x1 << VHOST_VRING_F_LOG),
> > +   a.log_guest_addr,
> > sizeof *vq->used +
> > vq->num * sizeof *vq->used->ring))
> 
> 
> It looks to me that we should use vhost_get_used_size() which takes 
> event into account.
> 
> Any reason that we can't reuse vq_log_access_ok() here?
> 

No reason indeed but I'll fix this in a preliminary patch, and
send a v2 shortly.

Cheers,

--
Greg

> Thanks
> 
> 
> > return -EINVAL;
> >
> >
>

Re: [PATCH] meson.build: Don't look for libudev for static builds

On 03/10/20 10:28, 罗勇刚(Yonggang Luo) wrote:
>> Yes, I think adding a "links" argument to dependency (similar to
>> find_library's has_headers argument) makes sense.  That would be written
>>
>>     dependency('libudev',
>>                required: get_option('mpath').enabled(),
>>                static: enable_static,
>>                links: skeleton)
> make sense, may also need extra cflags and link flags.

They would be provided by the dependency, in general.

Paolo

Re: [PATCH] meson.build: Don't look for libudev for static builds

On 03/10/20 10:29, 罗勇刚(Yonggang Luo) wrote:
> For some meson script like this:
> curses = not_found
> if iconv.found() and not get_option('curses').disabled()
>   curses_libname_list = ['ncursesw', 'ncurses', 'cursesw', 'pdcurses']
>   curses_test = '''
>     #include 
>     #include 
>     #include 
>     int main(void) {
>       wchar_t wch = L'w';
>       setlocale(LC_ALL, "");
>       resize_term(0, 0);
>       addwstr(L"wide chars\n");
>       addnwstr(&wch, 1);
>       add_wch(WACS_DEGREE);
>       return 0;
>     }'''
>   foreach curses_libname : curses_libname_list
>       libcurses = dependency(curses_libname,
>                              required: false,
>                              method: 'pkg-config',
>                              static: enable_static)
> 
>       if not libcurses.found()
>         dirs = ['/usr/include/ncursesw']
>         if targetos == 'windows'
>           dirs = []
>         endif
>         libcurses = cc.find_library(curses_libname,
>                                     required: false,
>                                     dirs: dirs,
>                                     static: enable_static)
>       endif
>       if libcurses.found()
>         if cc.links(curses_test, dependencies: [libcurses])
>           curses = declare_dependency(compile_args:
> '-DNCURSES_WIDECHAR', dependencies: [libcurses])
>           break
>         endif
>       endif
>   endforeach
> endif
> 
> We also need to define extra  compile_args  '-DNCURSES_WIDECHAR' as the
> part of dependencies.

You can do that with #define before including .

Paolo

[Bug 1898084] Re: Assertion failed: (buf_len != 0), function soread, file socket.c, line 183.

libslirp is a separate project now, and I see that you already reported
it there (https://gitlab.freedesktop.org/slirp/libslirp/-/issues/29), so
I'm closing this QEMU ticket now.

** Bug watch added: gitlab.freedesktop.org/slirp/libslirp/-/issues #29
   https://gitlab.freedesktop.org/slirp/libslirp/-/issues/29

** Changed in: qemu
   Status: New => Invalid

** Changed in: qemu
   Status: Invalid => Triaged

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

Title:
  Assertion failed: (buf_len != 0), function soread, file socket.c, line
  183.

Status in QEMU:
  Triaged

Bug description:
  I have a virtual raspberry py that I am running qemu 5.1.0 for MacOS.

  Here is the command line I used:

  qemu-system-arm \
-M versatilepb \
-cpu arm1176 \
-m 256 \
-drive 
file=2020-08-20-raspios-buster-armhf-lite.img,if=none,index=0,media=disk,format=raw,id=disk0
 \
-device virtio-blk-pci,drive=disk0,disable-modern=on,disable-legacy=off \
-net nic -net user,hostfwd=tcp::5022-:22 \
-dtb versatile-pb-buster-5.4.51.dtb \
-kernel kernel-qemu-5.4.51-buster \
-append "root=/dev/vda2 panic=1" \
-no-reboot \
-serial stdio

  When trying to ssh from another machine while docker was running
  inside the VM, I got the following error:

  Assertion failed: (buf_len != 0), function soread, file 
/private/tmp/qemu-20200813-13289-1g95loa/qemu-5.1.0/slirp/src/socket.c, line 183
  ../boot.sh: line 12:  8592 Abort trap: 6

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

[Bug 1898084] Re: Assertion failed: (buf_len != 0), function soread, file socket.c, line 183.

Hmm, thinking about it twice, maybe let's rather keep this open, to make
sure that we update to the right version of libslirp in QEMU once the
fix is released there.

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

Title:
  Assertion failed: (buf_len != 0), function soread, file socket.c, line
  183.

Status in QEMU:
  Triaged

Bug description:
  I have a virtual raspberry py that I am running qemu 5.1.0 for MacOS.

  Here is the command line I used:

  qemu-system-arm \
-M versatilepb \
-cpu arm1176 \
-m 256 \
-drive 
file=2020-08-20-raspios-buster-armhf-lite.img,if=none,index=0,media=disk,format=raw,id=disk0
 \
-device virtio-blk-pci,drive=disk0,disable-modern=on,disable-legacy=off \
-net nic -net user,hostfwd=tcp::5022-:22 \
-dtb versatile-pb-buster-5.4.51.dtb \
-kernel kernel-qemu-5.4.51-buster \
-append "root=/dev/vda2 panic=1" \
-no-reboot \
-serial stdio

  When trying to ssh from another machine while docker was running
  inside the VM, I got the following error:

  Assertion failed: (buf_len != 0), function soread, file 
/private/tmp/qemu-20200813-13289-1g95loa/qemu-5.1.0/slirp/src/socket.c, line 183
  ../boot.sh: line 12:  8592 Abort trap: 6

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

Re: [PATCH v3] qemu/atomic.h: rename atomic_ to qatomic_

On 02/10/20 18:43, Matthew Rosato wrote:
>> diff --git
>> a/include/standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h
>> b/include/standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h
>> index acd4c8346d..7b4062a1a1 100644
>> ---
>> a/include/standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h
>> +++
>> b/include/standard-headers/drivers/infiniband/hw/vmw_pvrdma/pvrdma_ring.h
>> @@ -68,7 +68,7 @@ static inline int pvrdma_idx_valid(uint32_t idx,
>> uint32_t max_elems)
>>     static inline int32_t pvrdma_idx(int *var, uint32_t max_elems)
>>   {
>> -    const unsigned int idx = atomic_read(var);
>> +    const unsigned int idx = qatomic_read(var);
>>     if (pvrdma_idx_valid(idx, max_elems))
>>   return idx & (max_elems - 1);
>> @@ -77,17 +77,17 @@ static inline int32_t pvrdma_idx(int *var,
>> uint32_t max_elems)
>>     static inline void pvrdma_idx_ring_inc(int *var, uint32_t max_elems)
>>   {
>> -    uint32_t idx = atomic_read(var) + 1;    /* Increment. */
>> +    uint32_t idx = qatomic_read(var) + 1;    /* Increment. */
>>     idx &= (max_elems << 1) - 1;    /* Modulo size, flip gen. */
>> -    atomic_set(var, idx);
>> +    qatomic_set(var, idx);
>>   }
>>     static inline int32_t pvrdma_idx_ring_has_space(const struct
>> pvrdma_ring *r,
>>     uint32_t max_elems, uint32_t *out_tail)
>>   {
>> -    const uint32_t tail = atomic_read(&r->prod_tail);
>> -    const uint32_t head = atomic_read(&r->cons_head);
>> +    const uint32_t tail = qatomic_read(&r->prod_tail);
>> +    const uint32_t head = qatomic_read(&r->cons_head);
>>     if (pvrdma_idx_valid(tail, max_elems) &&
>>   pvrdma_idx_valid(head, max_elems)) {
>> @@ -100,8 +100,8 @@ static inline int32_t
>> pvrdma_idx_ring_has_space(const struct pvrdma_ring *r,
>>   static inline int32_t pvrdma_idx_ring_has_data(const struct
>> pvrdma_ring *r,
>>    uint32_t max_elems, uint32_t *out_head)
>>   {
>> -    const uint32_t tail = atomic_read(&r->prod_tail);
>> -    const uint32_t head = atomic_read(&r->cons_head);
>> +    const uint32_t tail = qatomic_read(&r->prod_tail);
>> +    const uint32_t head = qatomic_read(&r->cons_head);
>>     if (pvrdma_idx_valid(tail, max_elems) &&
>>   pvrdma_idx_valid(head, max_elems)) {
> 
> 
> It looks like the changes in this file are going to get reverted the
> next time someone does a linux header sync.

Source code should not be at all imported from Linux.  The hacks that
accomodate pvrdma in update-linux-headers.sh (like s/atomic_t/u32/)
really have no place there; the files in
include/standard-headers/drivers/infiniband/hw/vmw_pvrdma should all be
moved in hw/.

Paolo

[PATCH] dockerfiles: add diffutils to Fedora

For some reason diffutils is not included in the Fedora containers anymore,
causing the build to fail.

Signed-off-by: Paolo Bonzini 
---
 tests/docker/dockerfiles/fedora.docker | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/docker/dockerfiles/fedora.docker 
b/tests/docker/dockerfiles/fedora.docker
index 71e4b56977..ec783418c8 100644
--- a/tests/docker/dockerfiles/fedora.docker
+++ b/tests/docker/dockerfiles/fedora.docker
@@ -11,6 +11,7 @@ ENV PACKAGES \
 cyrus-sasl-devel \
 dbus-daemon \
 device-mapper-multipath-devel \
+diffutils \
 findutils \
 gcc \
 gcc-c++ \
-- 
2.26.2

[PATCH] tests: tcg: do not use implicit rules

Use pattern rules to clarify which targets are going to match the
rule and to provide clearer error messages.

Signed-off-by: Paolo Bonzini 
---
 tests/Makefile.include | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/Makefile.include b/tests/Makefile.include
index 40d909badc..5aca98e60c 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -50,21 +50,21 @@ RUN_TCG_TARGET_RULES=$(patsubst %,run-tcg-tests-%, 
$(TARGET_DIRS))
 $(foreach PROBE_TARGET,$(TARGET_DIRS), \
$(eval -include $(SRC_PATH)/tests/tcg/Makefile.prereqs))
 
-build-tcg-tests-%: $(if $(CONFIG_PLUGIN),test-plugins)
+$(BUILD_TCG_TARGET_RULES): build-tcg-tests-%: $(if 
$(CONFIG_PLUGIN),test-plugins)
$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) \
-f $(SRC_PATH)/tests/tcg/Makefile.qemu \
SRC_PATH=$(SRC_PATH) \
V="$(V)" TARGET="$*" guest-tests, \
"BUILD", "TCG tests for $*")
 
-run-tcg-tests-%: build-tcg-tests-% all
+$(RUN_TCG_TARGET_RULES): run-tcg-tests-%: build-tcg-tests-% all
$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) \
-f $(SRC_PATH)/tests/tcg/Makefile.qemu \
SRC_PATH=$(SRC_PATH) SPEED="$(SPEED)" \
V="$(V)" TARGET="$*" run-guest-tests, \
"RUN", "TCG tests for $*")
 
-clean-tcg-tests-%:
+$(CLEAN_TCG_TARGET_RULES): clean-tcg-tests-%:
$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) \
-f $(SRC_PATH)/tests/tcg/Makefile.qemu \
SRC_PATH=$(SRC_PATH) TARGET="$*" clean-guest-tests, \
-- 
2.26.2

Re: [PATCH 0/5] kernel-doc ixes

On 03/10/20 04:41, Eduardo Habkost wrote:
> Among other fixes in kernel-doc, this series get rid of
> QEMU-specific $decl_type='type name' hack in kernel-doc, because
> it made it impossible to document macros with names starting with
> uppercase letters (like most of the macros at qom/object.h).

Thanks, it seemed like a good idea but... it wasn't. :)

Reviewed-by: Paolo Bonzini 

Paolo

> Eduardo Habkost (5):
>   kernel-doc: Handle function typedefs that return pointers
>   kernel-doc: Handle function typedefs without asterisks
>   qom: Explicitly tag doc comments for typedefs and structs
>   memory: Explicitly tag doc comments for structs
>   kernel-doc: Remove $decl_type='type name' hack
> 
>  include/exec/memory.h |  6 +++---
>  include/qom/object.h  | 22 +++---
>  scripts/kernel-doc| 16 +++-
>  3 files changed, 17 insertions(+), 27 deletions(-)
>

Re: [PATCH 5/6] docs/devel/qom: Remove usage of

On 03/10/20 04:54, Eduardo Habkost wrote:
>  is not valid reST syntax.
> 
> Function @argument references don't need additional markup, so
> just remove .
> 
> Constants were changed to use reST ``code`` syntax
> 
> Signed-off-by: Eduardo Habkost 
> ---
>  include/qom/object.h | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/include/qom/object.h b/include/qom/object.h
> index e738dfc6744..16c9bcdf3b0 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -1256,7 +1256,7 @@ char *object_property_get_str(Object *obj, const char 
> *name,
>   * Writes an object's canonical path to a property.
>   *
>   * If the link property was created with
> - * OBJ_PROP_LINK_STRONG bit, the old target object is
> + * ``OBJ_PROP_LINK_STRONG`` bit, the old target object is
>   * unreferenced, and a reference is added to the new target object.
>   *
>   * Returns: %true on success, %false on failure.
> @@ -1603,16 +1603,16 @@ void object_property_allow_set_link(const Object 
> *obj, const char *name,
>   *
>   * Links form the graph in the object model.
>   *
> - * The @check() callback is invoked when
> + * The @check() callback is invoked when
>   * object_property_set_link() is called and can raise an error to prevent the
> - * link being set.  If @check is NULL, the property is read-only
> + * link being set.  If @check is NULL, the property is read-only
>   * and cannot be set.
>   *
>   * Ownership of the pointer that @child points to is transferred to the
> - * link property.  The reference count for *@child is
> + * link property.  The reference count for *@child is
>   * managed by the property from after the function returns till the
>   * property is deleted with object_property_del().  If the
> - * @flags OBJ_PROP_LINK_STRONG bit is set,
> + * @flags ``OBJ_PROP_LINK_STRONG`` bit is set,
>   * the reference count is decremented when the property is deleted or
>   * modified.

You can use % too in this case and in the first hunk above; there's
actually no difference between ``a`` and %a except the latter is
shorter.  Linux generally prefers %a, though kernel-doc also knows about
``a`` so that ``%x`` is interpreted correctly when the percent sign
should be a literal.

Apart from that.

Reviewed-by: Paolo Bonzini 

Paolo

>   *
> @@ -1823,7 +1823,7 @@ ObjectProperty 
> *object_class_property_add_uint64_ptr(ObjectClass *klass,
>   * Add an alias for a property on an object.  This function will add a 
> property
>   * of the same type as the forwarded property.
>   *
> - * The caller must ensure that @target_obj stays alive as long 
> as
> + * The caller must ensure that @target_obj stays alive as long as
>   * this property exists.  In the case of a child object or an alias on the 
> same
>   * object this will be the case.  For aliases to other objects the caller is
>   * responsible for taking a reference.
>

Re: [PATCH 0/6] qom documentation fixes

On 03/10/20 04:54, Eduardo Habkost wrote:
> A few fixes to the QOM documentation in docs/devel/qom.rst and
> include/qom/object.h.
> 
> Eduardo Habkost (6):
>   qom: Fix DECLARE_*CHECKER documentation
>   docs/devel/qom: Fix indentation of bulleted list
>   docs/devel/qom: Fix indentation of code blocks
>   docs/devel/qom: Use *emphasis* for emphasis
>   docs/devel/qom: Remove usage of 
>   docs/devel/qom: Avoid long lines
> 
>  docs/devel/qom.rst   | 91 +++-
>  include/qom/object.h | 16 
>  2 files changed, 55 insertions(+), 52 deletions(-)
> 

Reviewed-by: Paolo Bonzini 

though see patch 5 for a small note that can even be fixed without a v2.

Paolo

Re: [PATCH v4 01/12] accel/tcg: Add stub for cpu_loop_exit()

On 9/29/20 5:43 PM, Philippe Mathieu-Daudé wrote:
> Since the support of SYS_READC in commit 8de702cb67 the
> semihosting code is strongly depedent of the TCG accelerator
> via a call to cpu_loop_exit().
> 
> Ideally we would only build semihosting support when TCG
> is available, but unfortunately this is not trivial because
> semihosting is used by many targets in different configurations.
> For now add a simple stub to avoid link failure when building
> with --disable-tcg:
> 
>   hw/semihosting/console.c:160: undefined reference to `cpu_loop_exit'
> 
> Cc: Keith Packard 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  accel/stubs/tcg-stub.c | 5 +
>  1 file changed, 5 insertions(+)

Reviewed-by: Richard Henderson 

r~

Re: [PATCH v4 02/12] meson: Allow optional target/${ARCH}/Kconfig

On 9/29/20 5:43 PM, Philippe Mathieu-Daudé wrote:
> +++ b/meson.build
> @@ -529,6 +529,7 @@ kconfig_external_symbols = [
>  ]
>  ignored = ['TARGET_XML_FILES', 'TARGET_ABI_DIR', 'TARGET_DIRS']
>  
> +fs = import('fs')

Note that I have this in the capstone update, and I placed it closer to the top
of the file with some other imports.


>  foreach target : target_dirs
>config_target = keyval.load(meson.current_build_dir() / target / 
> 'config-target.mak')
>  
> @@ -569,8 +570,13 @@ foreach target : target_dirs
>  endforeach
>  
>  config_devices_mak = target + '-config-devices.mak'
> +target_kconfig = 'target' / config_target['TARGET_BASE_ARCH'] / 'Kconfig'
> +minikconf_input = ['default-configs' / target + '.mak', 'Kconfig']
> +if fs.is_file(target_kconfig)

Missing a meson.current_source_dir()?
Leastwise that was a comment that Paolo had for me.


r~

Re: [PATCH v4 02/12] meson: Allow optional target/${ARCH}/Kconfig

On 03/10/20 11:13, Richard Henderson wrote:
>> +target_kconfig = 'target' / config_target['TARGET_BASE_ARCH'] / 
>> 'Kconfig'
>> +minikconf_input = ['default-configs' / target + '.mak', 'Kconfig']
>> +if fs.is_file(target_kconfig)
> Missing a meson.current_source_dir()?
> Leastwise that was a comment that Paolo had for me.

Not sure, but it was the only way I thought the BSD build could fail;
unless the capstone submodule really was not present in Peter's checkout
and submodule update was disabled.

Paolo

Re: [PATCH v4 04/12] target/arm: Restrict ARMv4 cpus to TCG accel


On 30/09/2020 10.03, Philippe Mathieu-Daudé wrote:

On 9/30/20 12:43 AM, Philippe Mathieu-Daudé wrote:

KVM requires a cpu based on (at least) the ARMv7 architecture.

Only enable the following ARMv4 CPUs when TCG is available:

   - StrongARM (SA1100/1110)
   - OMAP1510 (TI925T)

Signed-off-by: Philippe Mathieu-Daudé 
---
  hw/arm/Kconfig | 6 ++
  1 file changed, 6 insertions(+)

diff --git a/hw/arm/Kconfig b/hw/arm/Kconfig
index 7d040827af..b546b20654 100644
--- a/hw/arm/Kconfig
+++ b/hw/arm/Kconfig
@@ -1,3 +1,7 @@
+config ARM_V4
+bool
+select TCG


This should be 'depends on TCG' because we can not
*select* TCG, either we enabled it or not.

The problem is the machines are already selected in
default-configs/arm-softmmu.mak, so we can not build
the current config without TCG.


Is it really a problem? If the users disabled TCG and still have these 
machines in their arm-softmmu.mak, it's a configuration issue on their side, 
so it's ok if they get an error in that case.


 Thomas

Re: [PULL v2 00/11] capstone + disassembler patch queue

On 10/3/20 2:06 AM, Paolo Bonzini wrote:
> On 02/10/20 22:37, Peter Maydell wrote:
>> Found CMake: /usr/bin/cmake (2.8.12.2)
>> WARNING: The version of CMake /usr/bin/cmake is 2.8.12.2 but version
>>> =3.4 is required
>> Run-time dependency capstone found: NO (tried pkgconfig and cmake)
>> Configuring capstone-defs.h using configuration
>> Configuring config-host.h using configuration
>>
>> We shouldn't be looking for or using cmake at all.
> 
> This is a missing "method: 'pkg-config'".

Ah, thanks.


r~

Re: [PATCH v6 00/10] Fix scsi devices plug/unplug races w.r.t virtio-scsi iothread

Patchew URL: 
https://patchew.org/QEMU/20201002173558.232960-1-pbonz...@redhat.com/



Hi,

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

Type: series
Message-id: 20201002173558.232960-1-pbonz...@redhat.com
Subject: [PATCH v6 00/10] Fix scsi devices plug/unplug races w.r.t virtio-scsi 
iothread

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 - [tag update]  patchew/20200929224355.1224017-1-phi...@redhat.com -> 
patchew/20200929224355.1224017-1-phi...@redhat.com
 - [tag update]  patchew/20201003024123.193840-1-ehabk...@redhat.com -> 
patchew/20201003024123.193840-1-ehabk...@redhat.com
 - [tag update]  patchew/20201003025424.199291-1-ehabk...@redhat.com -> 
patchew/20201003025424.199291-1-ehabk...@redhat.com
 * [new tag] patchew/20201003085054.332992-1-pbonz...@redhat.com -> 
patchew/20201003085054.332992-1-pbonz...@redhat.com
 * [new tag] patchew/20201003085054.332992-2-pbonz...@redhat.com -> 
patchew/20201003085054.332992-2-pbonz...@redhat.com
Switched to a new branch 'test'
72355ec scsi/scsi_bus: fix races in REPORT LUNS
78baf0c virtio-scsi: use scsi_device_get
03b4bc4 scsi/scsi_bus: Add scsi_device_get
2a615ef scsi/scsi-bus: scsi_device_find: don't return unrealized devices
59dbbdb device-core: use atomic_set on .realized property
0f7ff0e device-core: use RCU for list of children of a bus
5d17a9e device_core: use drain_call_rcu in in qmp_device_add
7fda4cb scsi/scsi_bus: switch search direction in scsi_device_find
7d8c3e8 scsi: switch to bus->check_address
7aacab0 qdev: add "check if address free" callback for buses

=== OUTPUT BEGIN ===
1/10 Checking commit 7aacab0c0d1b (qdev: add "check if address free" callback 
for buses)
2/10 Checking commit 7d8c3e85267a (scsi: switch to bus->check_address)
ERROR: code indent should never use tabs
#53: FILE: hw/scsi/scsi-bus.c:137:
+^I^I^I^I int channel, int target, int lun,$

ERROR: code indent should never use tabs
#54: FILE: hw/scsi/scsi-bus.c:138:
+^I^I^I^I SCSIDevice **p_dev)$

WARNING: line over 80 characters
#69: FILE: hw/scsi/scsi-bus.c:153:
+static bool scsi_bus_check_address(BusState *qbus, DeviceState *qdev, Error 
**errp)

WARNING: line over 80 characters
#89: FILE: hw/scsi/scsi-bus.c:173:
+if (!scsi_bus_is_address_free(bus, dev->channel, dev->id, dev->lun, 
&d)) {

WARNING: line over 80 characters
#128: FILE: hw/scsi/scsi-bus.c:195:
+is_free = scsi_bus_is_address_free(bus, dev->channel, ++id, 
dev->lun, NULL);

WARNING: line over 80 characters
#141: FILE: hw/scsi/scsi-bus.c:205:
+is_free = scsi_bus_is_address_free(bus, dev->channel, dev->id, 
++lun, NULL);

total: 2 errors, 4 warnings, 182 lines checked

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

3/10 Checking commit 7fda4cb2ba3d (scsi/scsi_bus: switch search direction in 
scsi_device_find)
4/10 Checking commit 5d17a9e06136 (device_core: use drain_call_rcu in in 
qmp_device_add)
5/10 Checking commit 0f7ff0e67f1c (device-core: use RCU for list of children of 
a bus)
6/10 Checking commit 59dbbdb8dc7f (device-core: use atomic_set on .realized 
property)
7/10 Checking commit 2a615ef87ec0 (scsi/scsi-bus: scsi_device_find: don't 
return unrealized devices)
8/10 Checking commit 03b4bc43c886 (scsi/scsi_bus: Add scsi_device_get)
9/10 Checking commit 78baf0c272a0 (virtio-scsi: use scsi_device_get)
10/10 Checking commit 72355ecf7e7e (scsi/scsi_bus: fix races in REPORT LUNS)
=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20201002173558.232960-1-pbonz...@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [PATCH] meson.build: Don't look for libudev for static builds

On Sat, Oct 3, 2020 at 4:43 PM Paolo Bonzini  wrote:
>
> On 03/10/20 10:29, 罗勇刚(Yonggang Luo) wrote:
> > For some meson script like this:
> > curses = not_found
> > if iconv.found() and not get_option('curses').disabled()
> >   curses_libname_list = ['ncursesw', 'ncurses', 'cursesw', 'pdcurses']
> >   curses_test = '''
> > #include 
> > #include 
> > #include 
> > int main(void) {
> >   wchar_t wch = L'w';
> >   setlocale(LC_ALL, "");
> >   resize_term(0, 0);
> >   addwstr(L"wide chars\n");
> >   addnwstr(&wch, 1);
> >   add_wch(WACS_DEGREE);
> >   return 0;
> > }'''
> >   foreach curses_libname : curses_libname_list
> >   libcurses = dependency(curses_libname,
> >  required: false,
> >  method: 'pkg-config',
> >  static: enable_static)
> >
> >   if not libcurses.found()
> > dirs = ['/usr/include/ncursesw']
> > if targetos == 'windows'
> >   dirs = []
> > endif
> > libcurses = cc.find_library(curses_libname,
> > required: false,
> > dirs: dirs,
> > static: enable_static)
> >   endif
> >   if libcurses.found()
> > if cc.links(curses_test, dependencies: [libcurses])
> >   curses = declare_dependency(compile_args:
> > '-DNCURSES_WIDECHAR', dependencies: [libcurses])
> >   break
> > endif
> >   endif
> >   endforeach
> > endif
> >
> > We also need to define extra  compile_args  '-DNCURSES_WIDECHAR' as the
> > part of dependencies.
>
> You can do that with #define before including .
Yeap, but the  -DNCURSES_WIDECHAR not only used for testing compile of
#include but also
as a dependencies of qemu.
>
> Paolo
>


--
 此致
礼
罗勇刚
Yours
sincerely,
Yonggang Luo

Re: [PATCH v4 02/12] meson: Allow optional target/${ARCH}/Kconfig

On 10/3/20 4:15 AM, Paolo Bonzini wrote:
> On 03/10/20 11:13, Richard Henderson wrote:
>>> +target_kconfig = 'target' / config_target['TARGET_BASE_ARCH'] / 
>>> 'Kconfig'
>>> +minikconf_input = ['default-configs' / target + '.mak', 'Kconfig']
>>> +if fs.is_file(target_kconfig)
>> Missing a meson.current_source_dir()?
>> Leastwise that was a comment that Paolo had for me.
> 
> Not sure, but it was the only way I thought the BSD build could fail;
> unless the capstone submodule really was not present in Peter's checkout
> and submodule update was disabled.

I don't think the build actually failed, I think it was just the cmake warning
from the missing method: to which Peter objected.

FWIW with and without source_dir work for me when testing, and I'm about to
include it in the v3 pull for an abundance of caution.

r~

[PULL v3 00/11] capstone + disassembler patch queue

Version 3 adds the method: to the dependency(), which avoids the
cmake warning, and also matches the bulk of the other dependency()
invocations throughout meson.build.

I also added the suggested current_source_dir() out of an abundance
of caution, even though it works for me either way.


r~


The following changes since commit dd8c1e808f1ca311e1f50bff218c3ee3198b1f02:

  Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20201002' into 
staging (2020-10-02 14:29:49 +0100)

are available in the Git repository at:

  https://gitlab.com/rth7680/qemu.git tags/pull-cap-20201003

for you to fetch changes up to c6d3da962f058bca09b25f99da35436816fb6de8:

  disas/capstone: Add skipdata hook for s390x (2020-10-03 04:25:14 -0500)


Update capstone submodule from v3.0.5 to v5 ("next").
Convert submodule build to meson.
Enable capstone disassembly for s390x.
Code cleanups in disas.c


Richard Henderson (11):
  capstone: Convert Makefile bits to meson bits
  capstone: Update to upstream "next" branch
  capstone: Require version 4.0 from a system library
  disas: Move host asm annotations to tb_gen_code
  disas: Clean up CPUDebug initialization
  disas: Use qemu/bswap.h for bfd endian loads
  disas: Cleanup plugin_disas
  disas: Configure capstone for aarch64 host without libvixl
  disas: Split out capstone code to disas/capstone.c
  disas: Enable capstone disassembly for s390x
  disas/capstone: Add skipdata hook for s390x

 configure |  68 +
 Makefile  |  18 +-
 meson.build   | 123 +++-
 include/disas/dis-asm.h   | 104 +++
 include/disas/disas.h |   2 +-
 include/exec/log.h|   4 +-
 accel/tcg/translate-all.c |  24 +-
 disas.c   | 707 +++---
 disas/capstone.c  | 326 +
 target/s390x/cpu.c|   4 +
 tcg/tcg.c |   4 +-
 capstone  |   2 +-
 disas/meson.build |   1 +
 meson_options.txt |   4 +
 14 files changed, 686 insertions(+), 705 deletions(-)
 create mode 100644 disas/capstone.c

[PULL v3 01/11] capstone: Convert Makefile bits to meson bits

There are better ways to do this, e.g. meson cmake subproject,
but that requires cmake 3.7 and some of our CI environments
only provide cmake 3.5.

Nor can we add a meson.build file to capstone/, because the git
submodule would then always report "untracked files".  Fixing that
would require creating our own branch on the qemu git mirror, at
which point we could just as easily create a native meson subproject.

Instead, build the library via the main meson.build.

This improves the current state of affairs in that we will re-link
the qemu executables against a changed libcapstone.a, which we wouldn't
do before-hand.  In addition, the use of the configuration header file
instead of command-line -DEFINES means that we will rebuild the
capstone objects with changes to meson.build.

Acked-by: Paolo Bonzini 
Reviewed-by: Alex Bennée 
Signed-off-by: Richard Henderson 
---
 configure |  68 
 Makefile  |  18 ++--
 meson.build   | 110 +++---
 meson_options.txt |   4 ++
 4 files changed, 120 insertions(+), 80 deletions(-)

diff --git a/configure b/configure
index a5841241be..f46f433649 100755
--- a/configure
+++ b/configure
@@ -478,7 +478,7 @@ opengl=""
 opengl_dmabuf="no"
 cpuid_h="no"
 avx2_opt=""
-capstone=""
+capstone="auto"
 lzo=""
 snappy=""
 bzip2=""
@@ -1575,11 +1575,11 @@ for opt do
   ;;
   --enable-vhost-kernel) vhost_kernel="yes"
   ;;
-  --disable-capstone) capstone="no"
+  --disable-capstone) capstone="disabled"
   ;;
-  --enable-capstone) capstone="yes"
+  --enable-capstone) capstone="enabled"
   ;;
-  --enable-capstone=git) capstone="git"
+  --enable-capstone=git) capstone="internal"
   ;;
   --enable-capstone=system) capstone="system"
   ;;
@@ -5017,51 +5017,11 @@ fi
 # capstone
 
 case "$capstone" in
-  "" | yes)
-if $pkg_config capstone; then
-  capstone=system
-elif test -e "${source_path}/.git" && test $git_update = 'yes' ; then
-  capstone=git
-elif test -e "${source_path}/capstone/Makefile" ; then
-  capstone=internal
-elif test -z "$capstone" ; then
-  capstone=no
-else
-  feature_not_found "capstone" "Install capstone devel or git submodule"
-fi
-;;
-
-  system)
-if ! $pkg_config capstone; then
-  feature_not_found "capstone" "Install capstone devel"
-fi
-;;
-esac
-
-case "$capstone" in
-  git | internal)
-if test "$capstone" = git; then
+  auto | enabled | internal)
+# Simpler to always update submodule, even if not needed.
+if test -e "${source_path}/.git" && test $git_update = 'yes' ; then
   git_submodules="${git_submodules} capstone"
 fi
-mkdir -p capstone
-if test "$mingw32" = "yes"; then
-  LIBCAPSTONE=capstone.lib
-else
-  LIBCAPSTONE=libcapstone.a
-fi
-capstone_libs="-Lcapstone -lcapstone"
-capstone_cflags="-I${source_path}/capstone/include"
-;;
-
-  system)
-capstone_libs="$($pkg_config --libs capstone)"
-capstone_cflags="$($pkg_config --cflags capstone)"
-;;
-
-  no)
-;;
-  *)
-error_exit "Unknown state for capstone: $capstone"
 ;;
 esac
 
@@ -7142,11 +7102,6 @@ fi
 if test "$ivshmem" = "yes" ; then
   echo "CONFIG_IVSHMEM=y" >> $config_host_mak
 fi
-if test "$capstone" != "no" ; then
-  echo "CONFIG_CAPSTONE=y" >> $config_host_mak
-  echo "CAPSTONE_CFLAGS=$capstone_cflags" >> $config_host_mak
-  echo "CAPSTONE_LIBS=$capstone_libs" >> $config_host_mak
-fi
 if test "$debug_mutex" = "yes" ; then
   echo "CONFIG_DEBUG_MUTEX=y" >> $config_host_mak
 fi
@@ -7664,13 +7619,7 @@ done # for target in $targets
 if [ "$fdt" = "git" ]; then
   subdirs="$subdirs dtc"
 fi
-if [ "$capstone" = "git" -o "$capstone" = "internal" ]; then
-  subdirs="$subdirs capstone"
-fi
 echo "SUBDIRS=$subdirs" >> $config_host_mak
-if test -n "$LIBCAPSTONE"; then
-  echo "LIBCAPSTONE=$LIBCAPSTONE" >> $config_host_mak
-fi
 
 if test "$numa" = "yes"; then
   echo "CONFIG_NUMA=y" >> $config_host_mak
@@ -7846,7 +7795,8 @@ NINJA=${ninja:-$PWD/ninjatool} $meson setup \
-Dmalloc=$malloc -Dmalloc_trim=$malloc_trim \
-Dcocoa=$cocoa -Dmpath=$mpath -Dsdl=$sdl -Dsdl_image=$sdl_image \
-Dvnc=$vnc -Dvnc_sasl=$vnc_sasl -Dvnc_jpeg=$vnc_jpeg -Dvnc_png=$vnc_png 
\
-   -Dgettext=$gettext -Dxkbcommon=$xkbcommon -Du2f=$u2f\
+   -Dgettext=$gettext -Dxkbcommon=$xkbcommon -Du2f=$u2f \
+   -Dcapstone=$capstone \
 $cross_arg \
 "$PWD" "$source_path"
 
diff --git a/Makefile b/Makefile
index 54fc1a9d10..f27bd4b2eb 100644
--- a/Makefile
+++ b/Makefile
@@ -156,21 +156,11 @@ dtc/all: .git-submodule-status dtc/libfdt
 dtc/%: .git-submodule-status
@mkdir -p $@
 
-# Overriding CFLAGS causes us to lose defines added in the sub-makefile.
-# Not overriding CFLAGS leads to mis-matches between compilation modes.
-# Therefore we replicate some of the logic in the sub-makefile.
-# Remove all the extra -Warning flags that QEMU uses that Capstone doesn't;
-# no need

Re: [PATCH v4 03/12] target/arm: Select SEMIHOSTING if TCG is available

On 9/29/20 5:43 PM, Philippe Mathieu-Daudé wrote:
> Add a kconfig entry which not explicitly selected by another
> entry, but which selects SEMIHOSTING if TCG is available.
> 
> This avoids:
> 
>   /usr/bin/ld: libqemu-aarch64-softmmu.fa.p/target_arm_arm-semi.c.o: in 
> function `do_arm_semihosting':
>   target/arm/arm-semi.c:784: undefined reference to 
> `qemu_semihosting_console_outc'
>   target/arm/arm-semi.c:787: undefined reference to 
> `qemu_semihosting_console_outs'
>   /usr/bin/ld: target/arm/arm-semi.c:815: undefined reference to 
> `qemu_semihosting_console_inc'
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  target/arm/Kconfig | 4 
>  1 file changed, 4 insertions(+)
>  create mode 100644 target/arm/Kconfig

Reviewed-by: Richard Henderson 

r~

Re: [PATCH v4 09/12] target/arm: Make m_helper.c optional via CONFIG_ARM_V7M

On 9/29/20 5:43 PM, Philippe Mathieu-Daudé wrote:
> +arm_ss.add(when: 'CONFIG_ARM_V7M', if_true: files('m_helper.c'), if_false: 
> files('m_helper-stub.c'))
> +
>  arm_ss.add(zlib)
>  
>  arm_ss.add(when: 'CONFIG_TCG', if_true: files('arm-semi.c'))
> +arm_ss.add(when: 'CONFIG_TCG', if_false: files('m_helper-stub.c'))

I'm a bit surprised about adding the file twice.
Since ARM_V7M depends on TCG, isn't the second line redundant?


r~

Re: [PATCH v4 10/12] target/arm: Do not build TCG objects when TCG is off

On 9/29/20 5:43 PM, Philippe Mathieu-Daudé wrote:
> -arm_ss.add(when: 'CONFIG_TCG', if_true: files('arm-semi.c'))

Aha, so you remove the line in the next patch anyway.
I suspect that you can not add it in the previous.

Reviewed-by: Richard Henderson 

r~

Re: [PATCH v4 11/12] target/arm: Reorder meson.build rules

On 9/29/20 5:43 PM, Philippe Mathieu-Daudé wrote:
> Reorder the rules to make this file easier to modify.
> 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> I prefer to not squash that with the previous patch,
> as it makes it harder to review.
> ---
>  target/arm/meson.build | 22 ++
>  1 file changed, 10 insertions(+), 12 deletions(-)

Reviewed-by: Richard Henderson 

r~

[PATCH v3 1/3] vhost: Don't call access_ok() when using IOTLB

When the IOTLB device is enabled, the vring addresses we get
from userspace are GIOVAs. It is thus wrong to pass them down
to access_ok() which only takes HVAs.

Access validation is done at prefetch time with IOTLB. Teach
vq_access_ok() about that by moving the (vq->iotlb) check
from vhost_vq_access_ok() to vq_access_ok(). This prevents
vhost_vring_set_addr() to fail when verifying the accesses.
No behavior change for vhost_vq_access_ok().

BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1883084
Fixes: 6b1e6cc7855b ("vhost: new device IOTLB API")
Cc: jasow...@redhat.com
CC: sta...@vger.kernel.org # 4.14+
Signed-off-by: Greg Kurz 
Acked-by: Jason Wang 
---
 drivers/vhost/vhost.c |9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index b45519ca66a7..c3b49975dc28 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1290,6 +1290,11 @@ static bool vq_access_ok(struct vhost_virtqueue *vq, 
unsigned int num,
 vring_used_t __user *used)
 
 {
+   /* If an IOTLB device is present, the vring addresses are
+* GIOVAs. Access validation occurs at prefetch time. */
+   if (vq->iotlb)
+   return true;
+
return access_ok(desc, vhost_get_desc_size(vq, num)) &&
   access_ok(avail, vhost_get_avail_size(vq, num)) &&
   access_ok(used, vhost_get_used_size(vq, num));
@@ -1383,10 +1388,6 @@ bool vhost_vq_access_ok(struct vhost_virtqueue *vq)
if (!vq_log_access_ok(vq, vq->log_base))
return false;
 
-   /* Access validation occurs at prefetch time with IOTLB */
-   if (vq->iotlb)
-   return true;
-
return vq_access_ok(vq, vq->num, vq->desc, vq->avail, vq->used);
 }
 EXPORT_SYMBOL_GPL(vhost_vq_access_ok);

[PATCH v3 0/3] vhost: Skip access checks on GIOVAs

This series addresses some misuse around vring addresses provided by
userspace when using an IOTLB device. The misuse cause failures of
the VHOST_SET_VRING_ADDR ioctl on POWER, which in turn causes QEMU
to crash at migration time.

Jason suggested that we should use vhost_get_used_size() during the
review of v2. Fixed this in a preliminary patch (patch 2) and rebased
the vq_log_used_access_ok() helper on top (patch 3).

Note that I've also posted a patch for QEMU so that it skips the used
structure GIOVA when allocating the log bitmap. Otherwise QEMU fails to
allocate it because POWER puts GIOVAs very high in the address space (ie.
over 0x800ULL).

https://patchwork.ozlabs.org/project/qemu-devel/patch/160105498386.68108.2145229309875282336.st...@bahia.lan/

v3:
 - patch 1: added Jason's ack
 - patch 2: new patch to use vhost_get_used_size()
 - patch 3: rebased patch 2 from v2

v2:
 - patch 1: move the (vq->ioltb) check from vhost_vq_access_ok() to
vq_access_ok() as suggested by MST
 - patch 2: new patch

---

Greg Kurz (3):
  vhost: Don't call access_ok() when using IOTLB
  vhost: Use vhost_get_used_size() in vhost_vring_set_addr()
  vhost: Don't call log_access_ok() when using IOTLB


 drivers/vhost/vhost.c |   33 +++--
 1 file changed, 23 insertions(+), 10 deletions(-)

--
Greg

[PATCH v3 2/3] vhost: Use vhost_get_used_size() in vhost_vring_set_addr()

The open-coded computation of the used size doesn't take the event
into account when the VIRTIO_RING_F_EVENT_IDX feature is present.
Fix that by using vhost_get_used_size().

Signed-off-by: Greg Kurz 
---
 drivers/vhost/vhost.c |3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index c3b49975dc28..9d2c225fb518 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1519,8 +1519,7 @@ static long vhost_vring_set_addr(struct vhost_dev *d,
/* Also validate log access for used ring if enabled. */
if ((a.flags & (0x1 << VHOST_VRING_F_LOG)) &&
!log_access_ok(vq->log_base, a.log_guest_addr,
-   sizeof *vq->used +
-   vq->num * sizeof *vq->used->ring))
+  vhost_get_used_size(vq, vq->num)))
return -EINVAL;
}

Re: [PATCH v4 12/12] .travis.yml: Add a KVM-only Aarch64 job

On 9/29/20 5:43 PM, Philippe Mathieu-Daudé wrote:
> Add a job to build QEMU on Aarch64 with TCG disabled, so
> this configuration won't bitrot over time.
> 
> We explicitly modify default-configs/aarch64-softmmu.mak to
> only select the 'virt' and 'SBSA-REF' machines.

I really wish we didn't have to do this.

Can't we e.g. *not* list all of the arm boards explicitly in default-configs,
but use the Kconfig "default y if ..."?

Seems like that would let --disable-tcg work as expected.
One should still be able to create custom configs with e.g.
CONFIG_EXYNOS4=n or CONIFIG_ARM_V4=n, correct?

r~

[PATCH v3 3/3] vhost: Don't call log_access_ok() when using IOTLB

When the IOTLB device is enabled, the log_guest_addr that is passed by
userspace to the VHOST_SET_VRING_ADDR ioctl, and which is then written
to vq->log_addr, is a GIOVA. All writes to this address are translated
by log_user() to writes to an HVA, and then ultimately logged through
the corresponding GPAs in log_write_hva(). No logging will ever occur
with vq->log_addr in this case. It is thus wrong to pass vq->log_addr
and log_guest_addr to log_access_vq() which assumes they are actual
GPAs.

Introduce a new vq_log_used_access_ok() helper that only checks accesses
to the log for the used structure when there isn't an IOTLB device around.

Signed-off-by: Greg Kurz 
---
 drivers/vhost/vhost.c |   23 ++-
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 9d2c225fb518..9ad45e1d27f0 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1370,6 +1370,20 @@ bool vhost_log_access_ok(struct vhost_dev *dev)
 }
 EXPORT_SYMBOL_GPL(vhost_log_access_ok);
 
+static bool vq_log_used_access_ok(struct vhost_virtqueue *vq,
+ void __user *log_base,
+ bool log_used,
+ u64 log_addr)
+{
+   /* If an IOTLB device is present, log_addr is a GIOVA that
+* will never be logged by log_used(). */
+   if (vq->iotlb)
+   return true;
+
+   return !log_used || log_access_ok(log_base, log_addr,
+ vhost_get_used_size(vq, vq->num));
+}
+
 /* Verify access for write logging. */
 /* Caller should have vq mutex and device mutex */
 static bool vq_log_access_ok(struct vhost_virtqueue *vq,
@@ -1377,8 +1391,7 @@ static bool vq_log_access_ok(struct vhost_virtqueue *vq,
 {
return vq_memory_access_ok(log_base, vq->umem,
   vhost_has_feature(vq, VHOST_F_LOG_ALL)) &&
-   (!vq->log_used || log_access_ok(log_base, vq->log_addr,
- vhost_get_used_size(vq, vq->num)));
+   vq_log_used_access_ok(vq, log_base, vq->log_used, vq->log_addr);
 }
 
 /* Can we start vq? */
@@ -1517,9 +1530,9 @@ static long vhost_vring_set_addr(struct vhost_dev *d,
return -EINVAL;
 
/* Also validate log access for used ring if enabled. */
-   if ((a.flags & (0x1 << VHOST_VRING_F_LOG)) &&
-   !log_access_ok(vq->log_base, a.log_guest_addr,
-  vhost_get_used_size(vq, vq->num)))
+   if (!vq_log_used_access_ok(vq, vq->log_base,
+   a.flags & (0x1 << VHOST_VRING_F_LOG),
+   a.log_guest_addr))
return -EINVAL;
}

Re: [PATCH v4 12/12] .travis.yml: Add a KVM-only Aarch64 job


On 03/10/2020 12.03, Richard Henderson wrote:

On 9/29/20 5:43 PM, Philippe Mathieu-Daudé wrote:

Add a job to build QEMU on Aarch64 with TCG disabled, so
this configuration won't bitrot over time.

We explicitly modify default-configs/aarch64-softmmu.mak to
only select the 'virt' and 'SBSA-REF' machines.


I really wish we didn't have to do this.

Can't we e.g. *not* list all of the arm boards explicitly in default-configs,
but use the Kconfig "default y if ..."?

Seems like that would let --disable-tcg work as expected.
One should still be able to create custom configs with e.g.
CONFIG_EXYNOS4=n or CONIFIG_ARM_V4=n, correct?


But that would be different from how we handle all other targets currently...
IMHO we shoud go into a different direction instead, e.g. by adding a 
"--kconfig-dir" switch to the configure script. If it has not been 
specified, the configs will be read from default-configs/ (or maybe we 
should then rename it to configs/default/). But if the switch has been 
specified with a directory as parameter, the config files will be read from 
that directory instead. We could then have folders like:


- configs/default (current default-configs)
- configs/no-tcg (all machines that work without tcg)
- configs/lean-kvm (for "nemu"-style minimalistic settings)

etc.

What do you think?

 Thomas

Re: [PATCH v4 12/12] .travis.yml: Add a KVM-only Aarch64 job

On 10/3/20 5:14 AM, Thomas Huth wrote:
> On 03/10/2020 12.03, Richard Henderson wrote:
>> On 9/29/20 5:43 PM, Philippe Mathieu-Daudé wrote:
>>> Add a job to build QEMU on Aarch64 with TCG disabled, so
>>> this configuration won't bitrot over time.
>>>
>>> We explicitly modify default-configs/aarch64-softmmu.mak to
>>> only select the 'virt' and 'SBSA-REF' machines.
>>
>> I really wish we didn't have to do this.
>>
>> Can't we e.g. *not* list all of the arm boards explicitly in default-configs,
>> but use the Kconfig "default y if ..."?
>>
>> Seems like that would let --disable-tcg work as expected.
>> One should still be able to create custom configs with e.g.
>> CONFIG_EXYNOS4=n or CONIFIG_ARM_V4=n, correct?
> 
> But that would be different from how we handle all other targets currently...

So?  Does that automatically mean they're golden?
Perhaps they should be doing it the other way around too.


> IMHO we shoud go into a different direction instead, e.g. by adding a
> "--kconfig-dir" switch to the configure script. If it has not been specified,
> the configs will be read from default-configs/ (or maybe we should then rename
> it to configs/default/). But if the switch has been specified with a directory
> as parameter, the config files will be read from that directory instead. We
> could then have folders like:
> 
> - configs/default (current default-configs)
> - configs/no-tcg (all machines that work without tcg)
> - configs/lean-kvm (for "nemu"-style minimalistic settings)
> 
> etc.
> 
> What do you think?

I don't really understand the suggestion.  My first reaction is that this is
more confusing than --disable-tcg, and trying to automatically dtrt with that.


r~

[PATCH v2] hw/avr: Add limited support for avr gpio registers

2020-10-03 Thread Hee-cheol Yang

Hello, Thank you very much for your prompt and kind review.
This is my next version of the patch. Here are what I changed from the previous 
one:


  *   Remove unnecessary header inclusions
  *   Replace codes for unreachable conditions with g_assert_not_reached() 
function
  *   Remove 'enable' field from AVRGPIOState structure: It is actually 
unnecessary. I copied this field from AVRUSARTState structure.

I am afraid that there is an encoding issue even though I tried to do it 
correctly.
Sorry in advance for this issue again.

With best regards
Heecheol Yang.

diff --git a/hw/avr/Kconfig b/hw/avr/Kconfig
index d31298c3cc..16a57ced11 100644
--- a/hw/avr/Kconfig
+++ b/hw/avr/Kconfig
@@ -3,6 +3,7 @@ config AVR_ATMEGA_MCU
 select AVR_TIMER16
 select AVR_USART
 select AVR_POWER
+select AVR_GPIO

 config ARDUINO
 select AVR_ATMEGA_MCU
diff --git a/hw/avr/atmega.c b/hw/avr/atmega.c
index 44c6afebbb..ad942028fd 100644
--- a/hw/avr/atmega.c
+++ b/hw/avr/atmega.c
@@ -283,8 +283,11 @@ static void atmega_realize(DeviceState *dev, Error **errp)
 continue;
 }
 devname = g_strdup_printf("atmega-gpio-%c", 'a' + (char)i);
-create_unimplemented_device(devname,
-OFFSET_DATA + mc->dev[idx].addr, 3);
+object_initialize_child(OBJECT(dev), devname, &s->gpio[i],
+TYPE_AVR_GPIO);
+sysbus_realize(SYS_BUS_DEVICE(&s->gpio[i]), &error_abort);
+sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio[i]), 0,
+OFFSET_DATA + mc->dev[idx].addr);
 g_free(devname);
 }

diff --git a/hw/avr/atmega.h b/hw/avr/atmega.h
index a99ee15c7e..e2289d5744 100644
--- a/hw/avr/atmega.h
+++ b/hw/avr/atmega.h
@@ -13,6 +13,7 @@

 #include "hw/char/avr_usart.h"
 #include "hw/timer/avr_timer16.h"
+#include "hw/gpio/avr_gpio.h"
 #include "hw/misc/avr_power.h"
 #include "target/avr/cpu.h"
 #include "qom/object.h"
@@ -44,6 +45,7 @@ struct AtmegaMcuState {
 DeviceState *io;
 AVRMaskState pwr[POWER_MAX];
 AVRUsartState usart[USART_MAX];
+AVRGPIOState gpio[GPIO_MAX];
 AVRTimer16State timer[TIMER_MAX];
 uint64_t xtal_freq_hz;
 };
diff --git a/hw/gpio/Kconfig b/hw/gpio/Kconfig
index b6fdaa2586..1752d0ce56 100644
--- a/hw/gpio/Kconfig
+++ b/hw/gpio/Kconfig
@@ -10,3 +10,6 @@ config GPIO_KEY

 config SIFIVE_GPIO
 bool
+
+config AVR_GPIO
+bool
diff --git a/hw/gpio/avr_gpio.c b/hw/gpio/avr_gpio.c
new file mode 100644
index 00..6ca8e8703a
--- /dev/null
+++ b/hw/gpio/avr_gpio.c
@@ -0,0 +1,112 @@
+/*
+ * AVR processors GPIO registers emulation.
+ *
+ * Copyright (C) 2020 Heecheol Yang 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 or
+ * (at your option) version 3 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see .
+ */
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "qapi/error.h"
+#include "hw/sysbus.h"
+#include "hw/irq.h"
+#include "hw/gpio/avr_gpio.h"
+#include "hw/qdev-properties.h"
+
+static void avr_gpio_reset(DeviceState *dev)
+{
+AVRGPIOState *gpio = AVR_GPIO(dev);
+gpio->ddr_val = 0u;
+gpio->port_val = 0u;
+}
+static uint64_t avr_gpio_read(void *opaque, hwaddr offset, unsigned int size)
+{
+AVRGPIOState *s = (AVRGPIOState *)opaque;
+switch (offset) {
+case GPIO_PIN:
+/* Not implemented yet */
+break;
+case GPIO_DDR:
+return s->ddr_val;
+break;
+case GPIO_PORT:
+return s->port_val;
+default:
+g_assert_not_reached();
+break;
+}
+return 0;
+}
+
+static void avr_gpio_write(void *opaque, hwaddr offset, uint64_t value,
+unsigned int size)
+{
+AVRGPIOState *s = (AVRGPIOState *)opaque;
+switch (offset) {
+case GPIO_PIN:
+/* Not implemented yet */
+break;
+case GPIO_DDR:
+s->ddr_val = value & 0xF;
+break;
+case GPIO_PORT:
+s->port_val = value & 0xF;
+break;
+default:
+g_assert_not_reached();
+break;
+}
+}
+
+static const MemoryRegionOps avr_gpio_ops = {
+.read = avr_gpio_read,
+.write = avr_gpio_write,
+.endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void avr_gpio_init(Object *obj)
+{
+AVRGPIOState *s = AVR_GPIO(obj);
+memory_region_init_io(&s->mmio, obj, &avr_gpio_ops, s, TYPE_AVR_GPIO, 3);
+sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
+}
+static void avr_gpio_realize(DeviceState *

Emulate Rpi with QEMU fails

2020-10-03 Thread Thomas

Hi,

I'm trying to emulate Rpi with QEMU.
I found
[url=https://www.raspberry-pi-geek.de/ausgaben/rpg/2014/04/raspberry-pi-emulieren/]this[/url]
arcticle in Raspberry Pi Geek documenting the steps including persistent
storage on host.

However when starting the emulation with command
qemu-system-arm -M versatilepb -cpu arm1176 -m 256 -serial stdio -hda
2020-08-20-raspios-buster-armhf-lite.img -net
"user,hostfwd=tcp::5022-:22" -dtb versatile-pb-buster.dtb -kernel
kernel-qemu-5.4.51-buster -append "root=/dev/sda2 rootfstype=ext4 rw
panic=1" -no-reboot
I get this error:
VFS: Cannot open root device "sda2" or unknown-block(0,0): error -6
Please append a correct "root=" boot option; here are the available
partitions:
0100    4096 ram0
 (driver?)
0101    4096 ram1
 (driver?)
0102    4096 ram2
 (driver?)
0103    4096 ram3
 (driver?)
0104    4096 ram4
 (driver?)
0105    4096 ram5
 (driver?)
0106    4096 ram6
 (driver?)
0107    4096 ram7
 (driver?)
0108    4096 ram8
 (driver?)
0109    4096 ram9
 (driver?)
010a    4096 ram10
 (driver?)
010b    4096 ram11
 (driver?)
010c    4096 ram12
 (driver?)
010d    4096 ram13
 (driver?)
010e    4096 ram14
 (driver?)
010f    4096 ram15
 (driver?)
1f00   65536 mtdblock0
 (driver?)
Kernel panic - not syncing: VFS: Unable to mount root fs on
unknown-block(0,0)

I assume this is related to the content in fstab of RaspiOS:
proc    /proc   proc    defaults  0   0
PARTUUID=907af7d0-01  /boot   vfat    defaults  0   2
PARTUUID=907af7d0-02  /   ext4    defaults,noatime  0   1

Can you please advise how to fix this error?

THX

[PATCH v3] hw/avr: Add limited support for avr gpio registers

2020-10-03 Thread Hee-cheol Yang

I am very sorry for your inconvenience. This is 3rd version of the patch for 
what I did.
I should have read the contribution guide again and again. I am very sorry 
again.

The contents of the patch is the same with the v2. This mail is just for 
patchew and making a new thread.
Here are what I changed from the v1:

  *   Remove unnecessary header inclusions
  *   Replace codes for unreachable conditions with g_assert_not_reached() 
function
  *   Remove 'enable' field from AVRGPIOState structure: It is actually 
unnecessary. I copied this field from AVRUSARTState structure.

Thanks a lot!
With best regards
Heecheol Yang.

Add some of these features for avr gpio:

  - GPIO I/O : PORTx registers
  - Data Direction : DDRx registers

Following things are not supported yet:
  - PINx registers
  - MCUR registers
  - Even though read/write for DDRx registers are
implemented, actual direction controls are not
supported yet.

Signed-off-by: Heecheol Yang 
---
 hw/avr/Kconfig |   1 +
 hw/avr/atmega.c|   7 ++-
 hw/avr/atmega.h|   2 +
 hw/gpio/Kconfig|   3 +
 hw/gpio/avr_gpio.c | 112 +
 hw/gpio/meson.build|   2 +
 include/hw/gpio/avr_gpio.h |  46 +++
 7 files changed, 171 insertions(+), 2 deletions(-)
 create mode 100644 hw/gpio/avr_gpio.c
 create mode 100644 include/hw/gpio/avr_gpio.h

diff --git a/hw/avr/Kconfig b/hw/avr/Kconfig
index d31298c3cc..16a57ced11 100644
--- a/hw/avr/Kconfig
+++ b/hw/avr/Kconfig
@@ -3,6 +3,7 @@ config AVR_ATMEGA_MCU
 select AVR_TIMER16
 select AVR_USART
 select AVR_POWER
+select AVR_GPIO

 config ARDUINO
 select AVR_ATMEGA_MCU
diff --git a/hw/avr/atmega.c b/hw/avr/atmega.c
index 44c6afebbb..ad942028fd 100644
--- a/hw/avr/atmega.c
+++ b/hw/avr/atmega.c
@@ -283,8 +283,11 @@ static void atmega_realize(DeviceState *dev, Error **errp)
 continue;
 }
 devname = g_strdup_printf("atmega-gpio-%c", 'a' + (char)i);
-create_unimplemented_device(devname,
-OFFSET_DATA + mc->dev[idx].addr, 3);
+object_initialize_child(OBJECT(dev), devname, &s->gpio[i],
+TYPE_AVR_GPIO);
+sysbus_realize(SYS_BUS_DEVICE(&s->gpio[i]), &error_abort);
+sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio[i]), 0,
+OFFSET_DATA + mc->dev[idx].addr);
 g_free(devname);
 }

diff --git a/hw/avr/atmega.h b/hw/avr/atmega.h
index a99ee15c7e..e2289d5744 100644
--- a/hw/avr/atmega.h
+++ b/hw/avr/atmega.h
@@ -13,6 +13,7 @@

 #include "hw/char/avr_usart.h"
 #include "hw/timer/avr_timer16.h"
+#include "hw/gpio/avr_gpio.h"
 #include "hw/misc/avr_power.h"
 #include "target/avr/cpu.h"
 #include "qom/object.h"
@@ -44,6 +45,7 @@ struct AtmegaMcuState {
 DeviceState *io;
 AVRMaskState pwr[POWER_MAX];
 AVRUsartState usart[USART_MAX];
+AVRGPIOState gpio[GPIO_MAX];
 AVRTimer16State timer[TIMER_MAX];
 uint64_t xtal_freq_hz;
 };
diff --git a/hw/gpio/Kconfig b/hw/gpio/Kconfig
index b6fdaa2586..1752d0ce56 100644
--- a/hw/gpio/Kconfig
+++ b/hw/gpio/Kconfig
@@ -10,3 +10,6 @@ config GPIO_KEY

 config SIFIVE_GPIO
 bool
+
+config AVR_GPIO
+bool
diff --git a/hw/gpio/avr_gpio.c b/hw/gpio/avr_gpio.c
new file mode 100644
index 00..6ca8e8703a
--- /dev/null
+++ b/hw/gpio/avr_gpio.c
@@ -0,0 +1,112 @@
+/*
+ * AVR processors GPIO registers emulation.
+ *
+ * Copyright (C) 2020 Heecheol Yang 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 or
+ * (at your option) version 3 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see .
+ */
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "qapi/error.h"
+#include "hw/sysbus.h"
+#include "hw/irq.h"
+#include "hw/gpio/avr_gpio.h"
+#include "hw/qdev-properties.h"
+
+static void avr_gpio_reset(DeviceState *dev)
+{
+AVRGPIOState *gpio = AVR_GPIO(dev);
+gpio->ddr_val = 0u;
+gpio->port_val = 0u;
+}
+static uint64_t avr_gpio_read(void *opaque, hwaddr offset, unsigned int size)
+{
+AVRGPIOState *s = (AVRGPIOState *)opaque;
+switch (offset) {
+case GPIO_PIN:
+/* Not implemented yet */
+break;
+case GPIO_DDR:
+return s->ddr_val;
+break;
+case GPIO_PORT:
+return s->port_val;
+default:
+g_assert_not_reached();
+break;
+}
+return 0;
+}
+
+static void avr_gpio_write(void *opaque, hwaddr offse

Re: [PATCH 04/14] hw/arm/raspi: add a skeleton implementation of the cprman

2020-10-03 Thread Luc Michel

On 16:37 Fri 02 Oct , Philippe Mathieu-Daudé wrote:

[snip]

> >>> +struct BCM2835CprmanState {
> >>> +/*< private >*/
> >>> +SysBusDevice parent_obj;
> >>> +
> >>> +/*< public >*/
> >>> +MemoryRegion iomem;
> >>> +
> >>> +uint32_t regs[CPRMAN_NUM_REGS];
> >>> +uint32_t xosc_freq;
> >>> +
> >>> +Clock *xosc;
> 
> Isn't it xosc external to the CPRMAN?
> 
Yes on real hardware I'm pretty sure it's the oscillator we can see on
the board itself, near the SoC (on the bottom side). This is how I first
planned to implement it. I then realized that would add complexity to
the BCM2835Peripherals model for no good reasons IMHO (mainly because of
migration). So at the end I put it inside the CPRMAN for simplicity, and
added a property to set its frequency.

> >>> +};

[snip]

> >>> +static const MemoryRegionOps cprman_ops = {
> >>> +.read = cprman_read,
> >>> +.write = cprman_write,
> >>> +.endianness = DEVICE_LITTLE_ENDIAN,
> >>> +.valid  = {
> >>> +.min_access_size= 4,
> >>> +.max_access_size= 4,
> >>
> >> I couldn't find this in the public datasheets (any pointer?).
> >>
> >> Since your implementation is 32bit, can you explicit .impl
> >> min/max = 4?
> > 
> > I could not find this information either, but I assumed this is the
> > case, mainly because of the 'PASSWORD' field in all registers.
> 
> Good point. Do you mind adding a comment about it here please?
> 

OK

> > 
> > Regarding .impl, I thought that having .valid was enough?
> 
> Until we eventually figure out we can do 64-bit accesses,
> so someone change .valid.max to 8 and your model is broken :/

OK, I'll add the .impl constraints.

[snip]

-- 
Luc

[PATCH v3] hw/avr: Add limited support for avr gpio registers

2020-10-03 Thread Heecheol Yang

I am very sorry for your inconvenience. This is 3rd version of the patch 
for what I did.
I should have read the contribution guide again and again. I am very 
sorry again.


The contents of the patch is the same with the v2. This mail is just for 
patchew and making a new thread.

Here are what I changed from the v1:

 * Remove unnecessary header inclusions
 * Replace codes for unreachable conditions with g_assert_not_reached()
   function
 * Remove 'enable' field from AVRGPIOState structure: It is actually
   unnecessary. I copied this field from AVRUSARTState structure.


Thanks a lot!
With best regards
Heecheol Yang.

Add some of these features for avr gpio:

  - GPIO I/O : PORTx registers
  - Data Direction : DDRx registers

Following things are not supported yet:
  - PINx registers
  - MCUR registers
  - Even though read/write for DDRx registers are
    implemented, actual direction controls are not
    supported yet.

Signed-off-by: Heecheol Yang 
---
 hw/avr/Kconfig |   1 +
 hw/avr/atmega.c    |   7 ++-
 hw/avr/atmega.h    |   2 +
 hw/gpio/Kconfig    |   3 +
 hw/gpio/avr_gpio.c | 112 +
 hw/gpio/meson.build    |   2 +
 include/hw/gpio/avr_gpio.h |  46 +++
 7 files changed, 171 insertions(+), 2 deletions(-)
 create mode 100644 hw/gpio/avr_gpio.c
 create mode 100644 include/hw/gpio/avr_gpio.h

diff --git a/hw/avr/Kconfig b/hw/avr/Kconfig
index d31298c3cc..16a57ced11 100644
--- a/hw/avr/Kconfig
+++ b/hw/avr/Kconfig
@@ -3,6 +3,7 @@ config AVR_ATMEGA_MCU
 select AVR_TIMER16
 select AVR_USART
 select AVR_POWER
+    select AVR_GPIO

 config ARDUINO
 select AVR_ATMEGA_MCU
diff --git a/hw/avr/atmega.c b/hw/avr/atmega.c
index 44c6afebbb..ad942028fd 100644
--- a/hw/avr/atmega.c
+++ b/hw/avr/atmega.c
@@ -283,8 +283,11 @@ static void atmega_realize(DeviceState *dev, Error 
**errp)

 continue;
 }
 devname = g_strdup_printf("atmega-gpio-%c", 'a' + (char)i);
-    create_unimplemented_device(devname,
-    OFFSET_DATA + mc->dev[idx].addr, 3);
+    object_initialize_child(OBJECT(dev), devname, &s->gpio[i],
+    TYPE_AVR_GPIO);
+ sysbus_realize(SYS_BUS_DEVICE(&s->gpio[i]), &error_abort);
+ sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio[i]), 0,
+    OFFSET_DATA + mc->dev[idx].addr);
 g_free(devname);
 }

diff --git a/hw/avr/atmega.h b/hw/avr/atmega.h
index a99ee15c7e..e2289d5744 100644
--- a/hw/avr/atmega.h
+++ b/hw/avr/atmega.h
@@ -13,6 +13,7 @@

 #include "hw/char/avr_usart.h"
 #include "hw/timer/avr_timer16.h"
+#include "hw/gpio/avr_gpio.h"
 #include "hw/misc/avr_power.h"
 #include "target/avr/cpu.h"
 #include "qom/object.h"
@@ -44,6 +45,7 @@ struct AtmegaMcuState {
 DeviceState *io;
 AVRMaskState pwr[POWER_MAX];
 AVRUsartState usart[USART_MAX];
+    AVRGPIOState gpio[GPIO_MAX];
 AVRTimer16State timer[TIMER_MAX];
 uint64_t xtal_freq_hz;
 };
diff --git a/hw/gpio/Kconfig b/hw/gpio/Kconfig
index b6fdaa2586..1752d0ce56 100644
--- a/hw/gpio/Kconfig
+++ b/hw/gpio/Kconfig
@@ -10,3 +10,6 @@ config GPIO_KEY

 config SIFIVE_GPIO
 bool
+
+config AVR_GPIO
+    bool
diff --git a/hw/gpio/avr_gpio.c b/hw/gpio/avr_gpio.c
new file mode 100644
index 00..6ca8e8703a
--- /dev/null
+++ b/hw/gpio/avr_gpio.c
@@ -0,0 +1,112 @@
+/*
+ * AVR processors GPIO registers emulation.
+ *
+ * Copyright (C) 2020 Heecheol Yang 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 or
+ * (at your option) version 3 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see .
+ */
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "qapi/error.h"
+#include "hw/sysbus.h"
+#include "hw/irq.h"
+#include "hw/gpio/avr_gpio.h"
+#include "hw/qdev-properties.h"
+
+static void avr_gpio_reset(DeviceState *dev)
+{
+    AVRGPIOState *gpio = AVR_GPIO(dev);
+    gpio->ddr_val = 0u;
+    gpio->port_val = 0u;
+}
+static uint64_t avr_gpio_read(void *opaque, hwaddr offset, unsigned int 
size)

+{
+    AVRGPIOState *s = (AVRGPIOState *)opaque;
+    switch (offset) {
+    case GPIO_PIN:
+    /* Not implemented yet */
+    break;
+    case GPIO_DDR:
+    return s->ddr_val;
+    break;
+    case GPIO_PORT:
+    return s->port_val;
+    default:
+    g_assert_not_reached();
+    break;
+    }
+    return 0;
+}
+
+static void avr_gpio_write(void *opaque, hwaddr offset, uint64_t

[PATCH v2] hw/avr: Add limited support for avr gpio registers

2020-10-03 Thread Heecheol Yang

Add some of these features for avr gpio:

  - GPIO I/O : PORTx registers
  - Data Direction : DDRx registers

Following things are not supported yet:
  - PINx registers
  - MCUR registers
  - Even though read/write for DDRx registers are
implemented, actual direction controls are not
supported yet.

Signed-off-by: Heecheol Yang 
---
 hw/avr/Kconfig |   1 +
 hw/avr/atmega.c|   7 ++-
 hw/avr/atmega.h|   2 +
 hw/gpio/Kconfig|   3 +
 hw/gpio/avr_gpio.c | 112 +
 hw/gpio/meson.build|   2 +
 include/hw/gpio/avr_gpio.h |  46 +++
 7 files changed, 171 insertions(+), 2 deletions(-)
 create mode 100644 hw/gpio/avr_gpio.c
 create mode 100644 include/hw/gpio/avr_gpio.h

diff --git a/hw/avr/Kconfig b/hw/avr/Kconfig
index d31298c3cc..16a57ced11 100644
--- a/hw/avr/Kconfig
+++ b/hw/avr/Kconfig
@@ -3,6 +3,7 @@ config AVR_ATMEGA_MCU
 select AVR_TIMER16
 select AVR_USART
 select AVR_POWER
+select AVR_GPIO
 
 config ARDUINO
 select AVR_ATMEGA_MCU
diff --git a/hw/avr/atmega.c b/hw/avr/atmega.c
index 44c6afebbb..ad942028fd 100644
--- a/hw/avr/atmega.c
+++ b/hw/avr/atmega.c
@@ -283,8 +283,11 @@ static void atmega_realize(DeviceState *dev, Error **errp)
 continue;
 }
 devname = g_strdup_printf("atmega-gpio-%c", 'a' + (char)i);
-create_unimplemented_device(devname,
-OFFSET_DATA + mc->dev[idx].addr, 3);
+object_initialize_child(OBJECT(dev), devname, &s->gpio[i],
+TYPE_AVR_GPIO);
+sysbus_realize(SYS_BUS_DEVICE(&s->gpio[i]), &error_abort);
+sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio[i]), 0,
+OFFSET_DATA + mc->dev[idx].addr);
 g_free(devname);
 }
 
diff --git a/hw/avr/atmega.h b/hw/avr/atmega.h
index a99ee15c7e..e2289d5744 100644
--- a/hw/avr/atmega.h
+++ b/hw/avr/atmega.h
@@ -13,6 +13,7 @@
 
 #include "hw/char/avr_usart.h"
 #include "hw/timer/avr_timer16.h"
+#include "hw/gpio/avr_gpio.h"
 #include "hw/misc/avr_power.h"
 #include "target/avr/cpu.h"
 #include "qom/object.h"
@@ -44,6 +45,7 @@ struct AtmegaMcuState {
 DeviceState *io;
 AVRMaskState pwr[POWER_MAX];
 AVRUsartState usart[USART_MAX];
+AVRGPIOState gpio[GPIO_MAX];
 AVRTimer16State timer[TIMER_MAX];
 uint64_t xtal_freq_hz;
 };
diff --git a/hw/gpio/Kconfig b/hw/gpio/Kconfig
index b6fdaa2586..1752d0ce56 100644
--- a/hw/gpio/Kconfig
+++ b/hw/gpio/Kconfig
@@ -10,3 +10,6 @@ config GPIO_KEY
 
 config SIFIVE_GPIO
 bool
+
+config AVR_GPIO
+bool
diff --git a/hw/gpio/avr_gpio.c b/hw/gpio/avr_gpio.c
new file mode 100644
index 00..6ca8e8703a
--- /dev/null
+++ b/hw/gpio/avr_gpio.c
@@ -0,0 +1,112 @@
+/*
+ * AVR processors GPIO registers emulation.
+ *
+ * Copyright (C) 2020 Heecheol Yang 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 or
+ * (at your option) version 3 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see .
+ */
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "qemu/module.h"
+#include "qapi/error.h"
+#include "hw/sysbus.h"
+#include "hw/irq.h"
+#include "hw/gpio/avr_gpio.h"
+#include "hw/qdev-properties.h"
+
+static void avr_gpio_reset(DeviceState *dev)
+{
+AVRGPIOState *gpio = AVR_GPIO(dev);
+gpio->ddr_val = 0u;
+gpio->port_val = 0u;
+}
+static uint64_t avr_gpio_read(void *opaque, hwaddr offset, unsigned int size)
+{
+AVRGPIOState *s = (AVRGPIOState *)opaque;
+switch (offset) {
+case GPIO_PIN:
+/* Not implemented yet */
+break;
+case GPIO_DDR:
+return s->ddr_val;
+break;
+case GPIO_PORT:
+return s->port_val;
+default:
+g_assert_not_reached();
+break;
+}
+return 0;
+}
+
+static void avr_gpio_write(void *opaque, hwaddr offset, uint64_t value,
+unsigned int size)
+{
+AVRGPIOState *s = (AVRGPIOState *)opaque;
+switch (offset) {
+case GPIO_PIN:
+/* Not implemented yet */
+break;
+case GPIO_DDR:
+s->ddr_val = value & 0xF;
+break;
+case GPIO_PORT:
+s->port_val = value & 0xF;
+break;
+default:
+g_assert_not_reached();
+break;
+}
+}
+
+static const MemoryRegionOps avr_gpio_ops = {
+.read = avr_gpio_read,
+.write = avr_gpio_write,
+.endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void avr_gpio_init(Object *obj)
+{

Re: [PATCH v3 1/4] hw/timer/bcm2835: Introduce BCM2835_SYSTIMER_COUNT definition

On 10/2/20 11:42 AM, Philippe Mathieu-Daudé wrote:
> Use the BCM2835_SYSTIMER_COUNT definition instead of the
> magic '4' value.
> 
> Reviewed-by: Luc Michel 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  include/hw/timer/bcm2835_systmr.h | 4 +++-
>  hw/timer/bcm2835_systmr.c | 3 ++-
>  2 files changed, 5 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [PATCH v3 2/4] hw/timer/bcm2835: Rename variable holding CTRL_STATUS register

On 10/2/20 11:42 AM, Philippe Mathieu-Daudé wrote:
> The variable holding the CTRL_STATUS register is misnamed
> 'status'. Rename it 'ctrl_status' to make it more obvious
> this register is also used to control the peripheral.
> 
> Reviewed-by: Luc Michel 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  include/hw/timer/bcm2835_systmr.h | 2 +-
>  hw/timer/bcm2835_systmr.c | 8 
>  2 files changed, 5 insertions(+), 5 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [PATCH v2] hw/avr: Add limited support for avr gpio registers

2020-10-03 Thread Michael Rolnik

On Sat, Oct 3, 2020 at 3:39 PM Heecheol Yang 
wrote:

> Add some of these features for avr gpio:
>
>   - GPIO I/O : PORTx registers
>   - Data Direction : DDRx registers
>
> Following things are not supported yet:
>   - PINx registers
>   - MCUR registers
>   - Even though read/write for DDRx registers are
> implemented, actual direction controls are not
> supported yet.
>
> Signed-off-by: Heecheol Yang 
> ---
>  hw/avr/Kconfig |   1 +
>  hw/avr/atmega.c|   7 ++-
>  hw/avr/atmega.h|   2 +
>  hw/gpio/Kconfig|   3 +
>  hw/gpio/avr_gpio.c | 112 +
>  hw/gpio/meson.build|   2 +
>  include/hw/gpio/avr_gpio.h |  46 +++
>  7 files changed, 171 insertions(+), 2 deletions(-)
>  create mode 100644 hw/gpio/avr_gpio.c
>  create mode 100644 include/hw/gpio/avr_gpio.h
>
> diff --git a/hw/avr/Kconfig b/hw/avr/Kconfig
> index d31298c3cc..16a57ced11 100644
> --- a/hw/avr/Kconfig
> +++ b/hw/avr/Kconfig
> @@ -3,6 +3,7 @@ config AVR_ATMEGA_MCU
>  select AVR_TIMER16
>  select AVR_USART
>  select AVR_POWER
> +select AVR_GPIO
>
>  config ARDUINO
>  select AVR_ATMEGA_MCU
> diff --git a/hw/avr/atmega.c b/hw/avr/atmega.c
> index 44c6afebbb..ad942028fd 100644
> --- a/hw/avr/atmega.c
> +++ b/hw/avr/atmega.c
> @@ -283,8 +283,11 @@ static void atmega_realize(DeviceState *dev, Error
> **errp)
>  continue;
>  }
>  devname = g_strdup_printf("atmega-gpio-%c", 'a' + (char)i);
> -create_unimplemented_device(devname,
> -OFFSET_DATA + mc->dev[idx].addr, 3);
> +object_initialize_child(OBJECT(dev), devname, &s->gpio[i],
> +TYPE_AVR_GPIO);
> +sysbus_realize(SYS_BUS_DEVICE(&s->gpio[i]), &error_abort);
> +sysbus_mmio_map(SYS_BUS_DEVICE(&s->gpio[i]), 0,
> +OFFSET_DATA + mc->dev[idx].addr);
>  g_free(devname);
>  }
>
> diff --git a/hw/avr/atmega.h b/hw/avr/atmega.h
> index a99ee15c7e..e2289d5744 100644
> --- a/hw/avr/atmega.h
> +++ b/hw/avr/atmega.h
> @@ -13,6 +13,7 @@
>
>  #include "hw/char/avr_usart.h"
>  #include "hw/timer/avr_timer16.h"
> +#include "hw/gpio/avr_gpio.h"
>  #include "hw/misc/avr_power.h"
>  #include "target/avr/cpu.h"
>  #include "qom/object.h"
> @@ -44,6 +45,7 @@ struct AtmegaMcuState {
>  DeviceState *io;
>  AVRMaskState pwr[POWER_MAX];
>  AVRUsartState usart[USART_MAX];
> +AVRGPIOState gpio[GPIO_MAX];
>  AVRTimer16State timer[TIMER_MAX];
>  uint64_t xtal_freq_hz;
>  };
> diff --git a/hw/gpio/Kconfig b/hw/gpio/Kconfig
> index b6fdaa2586..1752d0ce56 100644
> --- a/hw/gpio/Kconfig
> +++ b/hw/gpio/Kconfig
> @@ -10,3 +10,6 @@ config GPIO_KEY
>
>  config SIFIVE_GPIO
>  bool
> +
> +config AVR_GPIO
> +bool
> diff --git a/hw/gpio/avr_gpio.c b/hw/gpio/avr_gpio.c
> new file mode 100644
> index 00..6ca8e8703a
> --- /dev/null
> +++ b/hw/gpio/avr_gpio.c
> @@ -0,0 +1,112 @@
> +/*
> + * AVR processors GPIO registers emulation.
> + *
> + * Copyright (C) 2020 Heecheol Yang 
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 or
> + * (at your option) version 3 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, see .
> + */
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +#include "qemu/module.h"
> +#include "qapi/error.h"
> +#include "hw/sysbus.h"
> +#include "hw/irq.h"
> +#include "hw/gpio/avr_gpio.h"
> +#include "hw/qdev-properties.h"
> +
> +static void avr_gpio_reset(DeviceState *dev)
> +{
> +AVRGPIOState *gpio = AVR_GPIO(dev);
> +gpio->ddr_val = 0u;
> +gpio->port_val = 0u;
> +}
> +static uint64_t avr_gpio_read(void *opaque, hwaddr offset, unsigned int
> size)
> +{
> +AVRGPIOState *s = (AVRGPIOState *)opaque;
> +switch (offset) {
> +case GPIO_PIN:
> +/* Not implemented yet */
> +break;
> +case GPIO_DDR:
> +return s->ddr_val;
>
Why do you need `break` after `return` ?

> +break;
> +case GPIO_PORT:
> +return s->port_val;
> +default:
> +g_assert_not_reached();
> +break;
> +}
> +return 0;
> +}
> +
> +static void avr_gpio_write(void *opaque, hwaddr offset, uint64_t value,
> +unsigned int size)
> +{
> +AVRGPIOState *s = (AVRGPIOState *)opaque;
> +switch (offset) {
> +case GPIO_PIN:
> +/* Not implemented yet */
> +break;
> +case GPIO_DD

[PATCH v7 01/14] replay: don't record interrupt poll

Interrupt poll is not a real interrupt event. It is needed only for
thread safety. This interrupt is used for i386 and converted
to hardware interrupt by cpu_handle_interrupt function.
Therefore it is not needed to be recorded, because hardware
interrupt will be recorded after converting.

Signed-off-by: Pavel Dovgalyuk 
Reviewed-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 

--

v4 changes:
 - Condition check refactoring (suggested by Alex Bennée)
---
 accel/tcg/cpu-exec.c |   21 ++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index e10b46283c..a2b913c72f 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -430,8 +430,7 @@ static inline bool cpu_handle_halt(CPUState *cpu)
 {
 if (cpu->halted) {
 #if defined(TARGET_I386) && !defined(CONFIG_USER_ONLY)
-if ((cpu->interrupt_request & CPU_INTERRUPT_POLL)
-&& replay_interrupt()) {
+if (cpu->interrupt_request & CPU_INTERRUPT_POLL) {
 X86CPU *x86_cpu = X86_CPU(cpu);
 qemu_mutex_lock_iothread();
 apic_poll_irq(x86_cpu->apic_state);
@@ -527,6 +526,20 @@ static inline bool cpu_handle_exception(CPUState *cpu, int 
*ret)
 return false;
 }
 
+/*
+ * CPU_INTERRUPT_POLL is a virtual event which gets converted into a
+ * "real" interrupt event later. It does not need to be recorded for
+ * replay purposes.
+ */
+static inline bool need_replay_interrupt(int interrupt_request)
+{
+#if defined(TARGET_I386)
+return !(interrupt_request & CPU_INTERRUPT_POLL);
+#else
+return true;
+#endif
+}
+
 static inline bool cpu_handle_interrupt(CPUState *cpu,
 TranslationBlock **last_tb)
 {
@@ -588,7 +601,9 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
and via longjmp via cpu_loop_exit.  */
 else {
 if (cc->cpu_exec_interrupt(cpu, interrupt_request)) {
-replay_interrupt();
+if (need_replay_interrupt(interrupt_request)) {
+replay_interrupt();
+}
 /*
  * After processing the interrupt, ensure an EXCP_DEBUG is
  * raised when single-stepping so that GDB doesn't miss the

[PATCH v7 00/14] Reverse debugging

GDB remote protocol supports reverse debugging of the targets.
It includes 'reverse step' and 'reverse continue' operations.
The first one finds the previous step of the execution,
and the second one is intended to stop at the last breakpoint that
would happen when the program is executed normally.

Reverse debugging is possible in the replay mode, when at least
one snapshot was created at the record or replay phase.
QEMU can use these snapshots for travelling back in time with GDB.

Running the execution in replay mode allows using GDB reverse debugging
commands:
 - reverse-stepi (or rsi): Steps one instruction to the past.
   QEMU loads on of the prior snapshots and proceeds to the desired
   instruction forward. When that step is reaches, execution stops.
 - reverse-continue (or rc): Runs execution "backwards".
   QEMU tries to find breakpoint or watchpoint by loaded prior snapshot
   and replaying the execution. Then QEMU loads snapshots again and
   replays to the latest breakpoint. When there are no breakpoints in
   the examined section of the execution, QEMU finds one more snapshot
   and tries again. After the first snapshot is processed, execution
   stops at this snapshot.

The set of patches include the following modifications:
 - gdbstub update for reverse debugging support
 - functions that automatically perform reverse step and reverse
   continue operations
 - hmp/qmp commands for manipulating the replay process
 - improvement of the snapshotting for saving the execution step
   in the snapshot parameters
 - avocado-based acceptance tests for reverse debugging

The patches are available in the repository:
https://github.com/ispras/qemu/tree/rr-200901

v7 changes:
 - updated snapshot info output format
 - fixed qcow2 snapshot-related tests
v6 changes:
 - removed passing err variable without checking it's value after
v5 changes:
 - disabled reverse debugging tests for gitlab-based testing
   due to the unidentified timeout problem
v4 changes:
 - added VM snapshot creation on gdb connect (suggested by Alex Bennée)
 - removed useless calls to error_free
 - updated poll interrupt processing
 - minor changes
v3 changes:
 - rebased to support the new build system
 - bumped avocado framework version for using fixed remote gdb client
v2 changes:
 - rebased to the latest upstream version
 - fixed replaying of the POLL interrupts after the latest debug changes

---

Pavel Dovgaluk (10):
  replay: provide an accessor for rr filename
  qapi: introduce replay.json for record/replay-related stuff
  replay: introduce info hmp/qmp command
  replay: introduce breakpoint at the specified step
  replay: implement replay-seek command
  replay: flush rr queue before loading the vmstate
  gdbstub: add reverse step support in replay mode
  gdbstub: add reverse continue support in replay mode
  replay: describe reverse debugging in docs/replay.txt
  tests/acceptance: add reverse debugging test

Pavel Dovgalyuk (4):
  replay: don't record interrupt poll
  qcow2: introduce icount field for snapshots
  migration: introduce icount field for snapshots
  replay: create temporary snapshot at debugger connection


 MAINTAINERS   |2 
 accel/tcg/cpu-exec.c  |   21 ++
 accel/tcg/translator.c|1 
 block/qapi.c  |   18 +-
 block/qcow2-snapshot.c|9 +
 block/qcow2.h |3 
 blockdev.c|   10 +
 docs/interop/qcow2.txt|5 
 docs/replay.txt   |   46 +
 exec.c|8 +
 gdbstub.c |   64 ++
 hmp-commands-info.hx  |   11 +
 hmp-commands.hx   |   50 +
 include/block/snapshot.h  |1 
 include/monitor/hmp.h |4 
 include/sysemu/replay.h   |   26 +++
 migration/savevm.c|   17 +-
 qapi/block-core.json  |   11 +
 qapi/meson.build  |1 
 qapi/misc.json|   18 --
 qapi/qapi-schema.json |1 
 qapi/replay.json  |  121 
 replay/meson.build|1 
 replay/replay-debugging.c |  334 +
 replay/replay-events.c|4 
 replay/replay-internal.h  |6 -
 replay/replay.c   |   22 ++
 softmmu/cpus.c|   19 ++
 stubs/replay.c|   15 +
 tests/acceptance/reverse_debugging.py |  208 +
 tests/qemu-iotests/261|   19 +-
 tests/qemu-iotests/261.out|   51 +++--
 tests/qemu-iotests/267.out|   48 ++---
 33 files changed, 1086 insertions(+), 89 deletions(-)
 create mode 100644 qapi/replay.json
 create mode 100644 replay/re

[PATCH v7 03/14] qcow2: introduce icount field for snapshots

This patch introduces the icount field for saving within the snapshot.
It is required for navigation between the snapshots in record/replay mode.

Signed-off-by: Pavel Dovgalyuk 
Acked-by: Kevin Wolf 

--

v7 changes:
 - also fix the test which checks qcow2 snapshot extra data
---
 block/qcow2-snapshot.c |7 ++
 block/qcow2.h  |3 +++
 docs/interop/qcow2.txt |5 
 tests/qemu-iotests/261 |   15 -
 tests/qemu-iotests/261.out |   51 +---
 5 files changed, 58 insertions(+), 23 deletions(-)

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 9b68690f56..d68b25e0c5 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -164,6 +164,12 @@ static int qcow2_do_read_snapshots(BlockDriverState *bs, 
bool repair,
 sn->disk_size = bs->total_sectors * BDRV_SECTOR_SIZE;
 }
 
+if (sn->extra_data_size >= endof(QCowSnapshotExtraData, icount)) {
+sn->icount = be64_to_cpu(extra.icount);
+} else {
+sn->icount = -1ULL;
+}
+
 if (sn->extra_data_size > sizeof(extra)) {
 uint64_t extra_data_end;
 size_t unknown_extra_data_size;
@@ -333,6 +339,7 @@ int qcow2_write_snapshots(BlockDriverState *bs)
 memset(&extra, 0, sizeof(extra));
 extra.vm_state_size_large = cpu_to_be64(sn->vm_state_size);
 extra.disk_size = cpu_to_be64(sn->disk_size);
+extra.icount = cpu_to_be64(sn->icount);
 
 id_str_size = strlen(sn->id_str);
 name_size = strlen(sn->name);
diff --git a/block/qcow2.h b/block/qcow2.h
index b71e444fca..125ea9679b 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -206,6 +206,7 @@ typedef struct QEMU_PACKED QCowSnapshotHeader {
 typedef struct QEMU_PACKED QCowSnapshotExtraData {
 uint64_t vm_state_size_large;
 uint64_t disk_size;
+uint64_t icount;
 } QCowSnapshotExtraData;
 
 
@@ -219,6 +220,8 @@ typedef struct QCowSnapshot {
 uint32_t date_sec;
 uint32_t date_nsec;
 uint64_t vm_clock_nsec;
+/* icount value for the moment when snapshot was taken */
+uint64_t icount;
 /* Size of all extra data, including QCowSnapshotExtraData if available */
 uint32_t extra_data_size;
 /* Data beyond QCowSnapshotExtraData, if any */
diff --git a/docs/interop/qcow2.txt b/docs/interop/qcow2.txt
index 7da0d81df8..0463f761ef 100644
--- a/docs/interop/qcow2.txt
+++ b/docs/interop/qcow2.txt
@@ -707,6 +707,11 @@ Snapshot table entry:
 
 Byte 48 - 55:   Virtual disk size of the snapshot in bytes
 
+Byte 56 - 63:   icount value which corresponds to
+the record/replay instruction count
+when the snapshot was taken. Set to -1
+if icount was disabled
+
 Version 3 images must include extra data at least up to
 byte 55.
 
diff --git a/tests/qemu-iotests/261 b/tests/qemu-iotests/261
index ddcb04f285..848ffa760d 100755
--- a/tests/qemu-iotests/261
+++ b/tests/qemu-iotests/261
@@ -91,7 +91,10 @@ print_snapshot_table()
 if [ $extra_len -ge 16 ]; then
 echo "Disk size: $(peek_file_be "$1" $((extra_ofs + 8)) 8)"
 fi
-if [ $extra_len -gt 16 ]; then
+if [ $extra_len -ge 24 ]; then
+echo "Icount: $(peek_file_be "$1" $((extra_ofs + 16)) 8)"
+fi
+if [ $extra_len -gt 24 ]; then
 echo 'Unknown extra data:' \
 "$(peek_file_raw "$1" $((extra_ofs + 16)) $((extra_len - 16)) \
| tr -d '\0')"
@@ -198,12 +201,12 @@ truncate -s 0 "$TEST_DIR/sn0-extra"
 truncate -s $(($(snapshot_table_entry_size "$TEST_DIR/sn0-pre") - 40)) \
 "$TEST_DIR/sn0-post"
 
-# Set sn1's extra data size to 42
-poke_file "$TEST_DIR/sn1-pre" 36 '\x00\x00\x00\x2a'
-truncate -s 42 "$TEST_DIR/sn1-extra"
-poke_file "$TEST_DIR/sn1-extra" 16 'very important data'
+# Set sn1's extra data size to 50
+poke_file "$TEST_DIR/sn1-pre" 36 '\x00\x00\x00\x32'
+truncate -s 50 "$TEST_DIR/sn1-extra"
+poke_file "$TEST_DIR/sn1-extra" 24 'very important data'
 # Grow sn1-post to pad
-truncate -s $(($(snapshot_table_entry_size "$TEST_DIR/sn1-pre") - 82)) \
+truncate -s $(($(snapshot_table_entry_size "$TEST_DIR/sn1-pre") - 90)) \
 "$TEST_DIR/sn1-post"
 
 # Set sn2's extra data size to 8
diff --git a/tests/qemu-iotests/261.out b/tests/qemu-iotests/261.out
index 2600354566..612433ae40 100644
--- a/tests/qemu-iotests/261.out
+++ b/tests/qemu-iotests/261.out
@@ -12,9 +12,10 @@ Snapshots in TEST_DIR/t.IMGFMT.v2.orig:
   [1]
 ID: 2
 Name: sn1
-Extra data size: 42
+Extra data size: 50
 VM state size: 0
 Disk size: 67108864
+Icount: 0
 Unknown extra data: very important data
   [2]
 ID: 3
@@ -29,22 +30,25 @@ Snapshots in TEST_DIR/t.IMGFMT.v3.orig:
   [0]
 ID: 1
 Name: sn0
-

[PATCH v7 07/14] replay: introduce breakpoint at the specified step

From: Pavel Dovgalyuk 

This patch introduces replay_break, replay_delete_break
qmp and hmp commands.
These commands allow stopping at the specified instruction.
It may be useful for debugging when there are some known
events that should be investigated.
replay_break command has one argument - number of instructions
executed since the start of the replay.
replay_delete_break removes previously set breakpoint.

Signed-off-by: Pavel Dovgalyuk 
Acked-by: Markus Armbruster 

--

v4 changes:
 - removed useless error_free call
---
 hmp-commands.hx   |   32 +
 include/monitor/hmp.h |2 +
 qapi/replay.json  |   36 +++
 replay/replay-debugging.c |   84 +
 replay/replay-internal.h  |4 ++
 replay/replay.c   |   17 +
 6 files changed, 175 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 1088d64503..7680d0b380 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1804,6 +1804,38 @@ SRST
   Set QOM property *property* of object at location *path* to value *value*
 ERST
 
+{
+.name   = "replay_break",
+.args_type  = "icount:i",
+.params = "icount",
+.help   = "set breakpoint at the specified instruction count",
+.cmd= hmp_replay_break,
+},
+
+SRST
+``replay_break`` *icount*
+  Set replay breakpoint at instruction count *icount*.
+  Execution stops when the specified instruction is reached.
+  There can be at most one breakpoint. When breakpoint is set, any prior
+  one is removed.  The breakpoint may be set only in replay mode and only
+  "in the future", i.e. at instruction counts greater than the current one.
+  The current instruction count can be observed with ``info replay``.
+ERST
+
+{
+.name   = "replay_delete_break",
+.args_type  = "",
+.params = "",
+.help   = "remove replay breakpoint",
+.cmd= hmp_replay_delete_break,
+},
+
+SRST
+``replay_delete_break``
+  Remove replay breakpoint which was previously set with ``replay_break``.
+  The command is ignored when there are no replay breakpoints.
+ERST
+
 {
 .name   = "info",
 .args_type  = "item:s?",
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index f297fccce8..809ad638bb 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -130,5 +130,7 @@ void hmp_info_vm_generation_id(Monitor *mon, const QDict 
*qdict);
 void hmp_info_memory_size_summary(Monitor *mon, const QDict *qdict);
 void hmp_info_sev(Monitor *mon, const QDict *qdict);
 void hmp_info_replay(Monitor *mon, const QDict *qdict);
+void hmp_replay_break(Monitor *mon, const QDict *qdict);
+void hmp_replay_delete_break(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi/replay.json b/qapi/replay.json
index e6b3f6001d..173ba76107 100644
--- a/qapi/replay.json
+++ b/qapi/replay.json
@@ -63,3 +63,39 @@
 ##
 { 'command': 'query-replay',
   'returns': 'ReplayInfo' }
+
+##
+# @replay-break:
+#
+# Set replay breakpoint at instruction count @icount.
+# Execution stops when the specified instruction is reached.
+# There can be at most one breakpoint. When breakpoint is set, any prior
+# one is removed.  The breakpoint may be set only in replay mode and only
+# "in the future", i.e. at instruction counts greater than the current one.
+# The current instruction count can be observed with @query-replay.
+#
+# @icount: instruction count to stop at
+#
+# Since: 5.2
+#
+# Example:
+#
+# -> { "execute": "replay-break", "data": { "icount": 220414 } }
+#
+##
+{ 'command': 'replay-break', 'data': { 'icount': 'int' } }
+
+##
+# @replay-delete-break:
+#
+# Remove replay breakpoint which was set with @replay-break.
+# The command is ignored when there are no replay breakpoints.
+#
+# Since: 5.2
+#
+# Example:
+#
+# -> { "execute": "replay-delete-break" }
+#
+##
+{ 'command': 'replay-delete-break' }
diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
index 51a6de4e81..3dc23b84fc 100644
--- a/replay/replay-debugging.c
+++ b/replay/replay-debugging.c
@@ -12,10 +12,13 @@
 #include "qemu/osdep.h"
 #include "qapi/error.h"
 #include "sysemu/replay.h"
+#include "sysemu/runstate.h"
 #include "replay-internal.h"
 #include "monitor/hmp.h"
 #include "monitor/monitor.h"
 #include "qapi/qapi-commands-replay.h"
+#include "qapi/qmp/qdict.h"
+#include "qemu/timer.h"
 
 void hmp_info_replay(Monitor *mon, const QDict *qdict)
 {
@@ -41,3 +44,84 @@ ReplayInfo *qmp_query_replay(Error **errp)
 retval->icount = replay_get_current_icount();
 return retval;
 }
+
+static void replay_break(uint64_t icount, QEMUTimerCB callback, void *opaque)
+{
+assert(replay_mode == REPLAY_MODE_PLAY);
+assert(replay_mutex_locked());
+assert(replay_break_icount >= replay_get_current_icount());
+assert(callback);
+
+replay_break_icount = icount;
+
+if (replay_break_timer) {
+timer_del(replay_bre

[PATCH v7 02/14] replay: provide an accessor for rr filename

From: Pavel Dovgalyuk 

This patch adds an accessor function for the name of the record/replay
log file. Adding an accessor instead of making variable global,
prevents accidental modification of this variable by other modules.

Signed-off-by: Pavel Dovgalyuk 
Reviewed-by: Alex Bennée 
Reviewed-by: Philippe Mathieu-Daudé 
---
 include/sysemu/replay.h |2 ++
 replay/replay.c |5 +
 2 files changed, 7 insertions(+)

diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 5471bb514d..c9c896ae8d 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -72,6 +72,8 @@ void replay_start(void);
 void replay_finish(void);
 /*! Adds replay blocker with the specified error description */
 void replay_add_blocker(Error *reason);
+/* Returns name of the replay log file */
+const char *replay_get_filename(void);
 
 /* Processing the instructions */
 
diff --git a/replay/replay.c b/replay/replay.c
index 83ed9e0e24..42e82f7bc7 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -399,3 +399,8 @@ void replay_add_blocker(Error *reason)
 {
 replay_blockers = g_slist_prepend(replay_blockers, reason);
 }
+
+const char *replay_get_filename(void)
+{
+return replay_filename;
+}

[PATCH v7 12/14] replay: describe reverse debugging in docs/replay.txt

From: Pavel Dovgalyuk 

This patch updates the documentation and describes usage of the reverse
debugging in QEMU+GDB.

Signed-off-by: Pavel Dovgalyuk 
Reviewed-by: Alex Bennée 

--

v4 changes:
 - added an example of the command line for reverse debugging of
   the diskless machine
---
 docs/replay.txt |   46 ++
 1 file changed, 46 insertions(+)

diff --git a/docs/replay.txt b/docs/replay.txt
index 70c27edb36..39fe5e9740 100644
--- a/docs/replay.txt
+++ b/docs/replay.txt
@@ -265,6 +265,16 @@ of the original disk image, use overlay files linked to 
the original images.
 Therefore all new snapshots (including the starting one) will be saved in
 overlays and the original image remains unchanged.
 
+When you need to use snapshots with diskless virtual machine,
+it must be started with 'orphan' qcow2 image. This image will be used
+for storing VM snapshots. Here is the example of the command line for this:
+
+  qemu-system-i386 -icount shift=3,rr=replay,rrfile=record.bin,rrsnapshot=init 
\
+-net none -drive file=empty.qcow2,if=none,id=rr
+
+empty.qcow2 drive does not connected to any virtual block device and used
+for VM snapshots only.
+
 Network devices
 ---
 
@@ -294,6 +304,42 @@ for recording and replaying must contain identical number 
of ports in record
 and replay modes, but their backends may differ.
 E.g., '-serial stdio' in record mode, and '-serial null' in replay mode.
 
+Reverse debugging
+-
+
+Reverse debugging allows "executing" the program in reverse direction.
+GDB remote protocol supports "reverse step" and "reverse continue"
+commands. The first one steps single instruction backwards in time,
+and the second one finds the last breakpoint in the past.
+
+Recorded executions may be used to enable reverse debugging. QEMU can't
+execute the code in backwards direction, but can load a snapshot and
+replay forward to find the desired position or breakpoint.
+
+The following GDB commands are supported:
+ - reverse-stepi (or rsi) - step one instruction backwards
+ - reverse-continue (or rc) - find last breakpoint in the past
+
+Reverse step loads the nearest snapshot and replays the execution until
+the required instruction is met.
+
+Reverse continue may include several passes of examining the execution
+between the snapshots. Each of the passes include the following steps:
+ 1. loading the snapshot
+ 2. replaying to examine the breakpoints
+ 3. if breakpoint or watchpoint was met
+- loading the snaphot again
+- replaying to the required breakpoint
+ 4. else
+- proceeding to the p.1 with the earlier snapshot
+
+Therefore usage of the reverse debugging requires at least one snapshot
+created in advance. This can be done by omitting 'snapshot' option
+for the block drives and adding 'rrsnapshot' for both record and replay
+command lines.
+See the "Snapshotting" section to learn more about running record/replay
+and creating the snapshot in these modes.
+
 Replay log format
 -

[PATCH v7 04/14] migration: introduce icount field for snapshots

Saving icount as a parameters of the snapshot allows navigation between
them in the execution replay scenario.
This information can be used for finding a specific snapshot for proceeding
the recorded execution to the specific moment of the time.
E.g., 'reverse step' action (introduced in one of the following patches)
needs to load the nearest snapshot which is prior to the current moment
of time.
This patch also updates snapshot test which verifies qemu monitor output.

Signed-off-by: Pavel Dovgalyuk 
Acked-by: Markus Armbruster 
Acked-by: Kevin Wolf 

--

v4 changes:
 - squashed format update with test output update
v7 changes:
 - introduced the spaces between the fields in snapshot info output
 - updated the test to match new field widths
---
 block/qapi.c   |   18 +
 block/qcow2-snapshot.c |2 ++
 blockdev.c |   10 +
 include/block/snapshot.h   |1 +
 migration/savevm.c |5 +
 qapi/block-core.json   |   10 +++--
 stubs/replay.c |5 +
 tests/qemu-iotests/261 |4 ++--
 tests/qemu-iotests/267.out |   48 ++--
 9 files changed, 71 insertions(+), 32 deletions(-)

diff --git a/block/qapi.c b/block/qapi.c
index f423ece98c..036da085ee 100644
--- a/block/qapi.c
+++ b/block/qapi.c
@@ -230,6 +230,8 @@ int bdrv_query_snapshot_info_list(BlockDriverState *bs,
 info->date_nsec = sn_tab[i].date_nsec;
 info->vm_clock_sec  = sn_tab[i].vm_clock_nsec / 10;
 info->vm_clock_nsec = sn_tab[i].vm_clock_nsec % 10;
+info->icount= sn_tab[i].icount;
+info->has_icount= sn_tab[i].icount != -1ULL;
 
 info_list = g_new0(SnapshotInfoList, 1);
 info_list->value = info;
@@ -694,14 +696,15 @@ BlockStatsList *qmp_query_blockstats(bool has_query_nodes,
 void bdrv_snapshot_dump(QEMUSnapshotInfo *sn)
 {
 char date_buf[128], clock_buf[128];
+char icount_buf[128] = {0};
 struct tm tm;
 time_t ti;
 int64_t secs;
 char *sizing = NULL;
 
 if (!sn) {
-qemu_printf("%-10s%-20s%11s%20s%15s",
-"ID", "TAG", "VM SIZE", "DATE", "VM CLOCK");
+qemu_printf("%-10s%-18s%7s%20s%13s%11s",
+"ID", "TAG", "VM SIZE", "DATE", "VM CLOCK", "ICOUNT");
 } else {
 ti = sn->date_sec;
 localtime_r(&ti, &tm);
@@ -715,11 +718,16 @@ void bdrv_snapshot_dump(QEMUSnapshotInfo *sn)
  (int)(secs % 60),
  (int)((sn->vm_clock_nsec / 100) % 1000));
 sizing = size_to_str(sn->vm_state_size);
-qemu_printf("%-10s%-20s%11s%20s%15s",
+if (sn->icount != -1ULL) {
+snprintf(icount_buf, sizeof(icount_buf),
+"%"PRId64, sn->icount);
+}
+qemu_printf("%-9s %-17s %7s%20s%13s%11s",
 sn->id_str, sn->name,
 sizing,
 date_buf,
-clock_buf);
+clock_buf,
+icount_buf);
 }
 g_free(sizing);
 }
@@ -881,6 +889,8 @@ void bdrv_image_info_dump(ImageInfo *info)
 .date_nsec = elem->value->date_nsec,
 .vm_clock_nsec = elem->value->vm_clock_sec * 10ULL +
  elem->value->vm_clock_nsec,
+.icount = elem->value->has_icount ?
+  elem->value->icount : -1ULL,
 };
 
 pstrcpy(sn.id_str, sizeof(sn.id_str), elem->value->id);
diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index d68b25e0c5..2e98c7f4b6 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -663,6 +663,7 @@ int qcow2_snapshot_create(BlockDriverState *bs, 
QEMUSnapshotInfo *sn_info)
 sn->date_sec = sn_info->date_sec;
 sn->date_nsec = sn_info->date_nsec;
 sn->vm_clock_nsec = sn_info->vm_clock_nsec;
+sn->icount = sn_info->icount;
 sn->extra_data_size = sizeof(QCowSnapshotExtraData);
 
 /* Allocate the L1 table of the snapshot and copy the current one there. */
@@ -1007,6 +1008,7 @@ int qcow2_snapshot_list(BlockDriverState *bs, 
QEMUSnapshotInfo **psn_tab)
 sn_info->date_sec = sn->date_sec;
 sn_info->date_nsec = sn->date_nsec;
 sn_info->vm_clock_nsec = sn->vm_clock_nsec;
+sn_info->icount = sn->icount;
 }
 *psn_tab = sn_tab;
 return s->nb_snapshots;
diff --git a/blockdev.c b/blockdev.c
index bebd3ba1c3..a6ae475dac 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -59,6 +59,7 @@
 #include "sysemu/arch_init.h"
 #include "sysemu/qtest.h"
 #include "sysemu/runstate.h"
+#include "sysemu/replay.h"
 #include "qemu/cutils.h"
 #include "qemu/help_option.h"
 #include "qemu/main-loop.h"
@@ -1190,6 +1191,10 @@ SnapshotInfo 
*qmp_blockdev_snapshot_delete_internal_sync(const char *device,
 info->vm_state_size = sn.vm_state_size;
 info->vm_clock_nsec = sn.vm_clock_nsec % 10;
 info->vm

[PATCH v7 05/14] qapi: introduce replay.json for record/replay-related stuff

From: Pavel Dovgalyuk 

This patch adds replay.json file. It will be
used for adding record/replay-related data structures and commands.

Signed-off-by: Pavel Dovgalyuk 
Reviewed-by: Markus Armbruster 
Reviewed-by: Alex Bennée 
---
 MAINTAINERS |1 +
 include/sysemu/replay.h |1 +
 qapi/meson.build|1 +
 qapi/misc.json  |   18 --
 qapi/qapi-schema.json   |1 +
 qapi/replay.json|   26 ++
 6 files changed, 30 insertions(+), 18 deletions(-)
 create mode 100644 qapi/replay.json

diff --git a/MAINTAINERS b/MAINTAINERS
index b76fb31861..ea4fa3e481 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2693,6 +2693,7 @@ F: include/sysemu/replay.h
 F: docs/replay.txt
 F: stubs/replay.c
 F: tests/acceptance/replay_kernel.py
+F: qapi/replay.json
 
 IOVA Tree
 M: Peter Xu 
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index c9c896ae8d..e00ed2f4a5 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -14,6 +14,7 @@
 
 #include "qapi/qapi-types-misc.h"
 #include "qapi/qapi-types-run-state.h"
+#include "qapi/qapi-types-replay.h"
 #include "qapi/qapi-types-ui.h"
 #include "block/aio.h"
 
diff --git a/qapi/meson.build b/qapi/meson.build
index 7c4a89a882..8ae085a17d 100644
--- a/qapi/meson.build
+++ b/qapi/meson.build
@@ -38,6 +38,7 @@ qapi_all_modules = [
   'pci',
   'qom',
   'rdma',
+  'replay',
   'rocker',
   'run-state',
   'sockets',
diff --git a/qapi/misc.json b/qapi/misc.json
index 694d2142f3..7d1e2e9aae 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -757,24 +757,6 @@
  'returns': ['CommandLineOptionInfo'],
  'allow-preconfig': true }
 
-##
-# @ReplayMode:
-#
-# Mode of the replay subsystem.
-#
-# @none: normal execution mode. Replay or record are not enabled.
-#
-# @record: record mode. All non-deterministic data is written into the
-#  replay log.
-#
-# @play: replay mode. Non-deterministic data required for system execution
-#is read from the log.
-#
-# Since: 2.5
-##
-{ 'enum': 'ReplayMode',
-  'data': [ 'none', 'record', 'play' ] }
-
 ##
 # @xen-load-devices-state:
 #
diff --git a/qapi/qapi-schema.json b/qapi/qapi-schema.json
index 0c6ca5c000..d0ea66ca05 100644
--- a/qapi/qapi-schema.json
+++ b/qapi/qapi-schema.json
@@ -84,6 +84,7 @@
 { 'include': 'qdev.json' }
 { 'include': 'machine.json' }
 { 'include': 'machine-target.json' }
+{ 'include': 'replay.json' }
 { 'include': 'misc.json' }
 { 'include': 'misc-target.json' }
 { 'include': 'audio.json' }
diff --git a/qapi/replay.json b/qapi/replay.json
new file mode 100644
index 00..9e13551d20
--- /dev/null
+++ b/qapi/replay.json
@@ -0,0 +1,26 @@
+# -*- Mode: Python -*-
+#
+
+##
+# = Record/replay
+##
+
+{ 'include': 'common.json' }
+
+##
+# @ReplayMode:
+#
+# Mode of the replay subsystem.
+#
+# @none: normal execution mode. Replay or record are not enabled.
+#
+# @record: record mode. All non-deterministic data is written into the
+#  replay log.
+#
+# @play: replay mode. Non-deterministic data required for system execution
+#is read from the log.
+#
+# Since: 2.5
+##
+{ 'enum': 'ReplayMode',
+  'data': [ 'none', 'record', 'play' ] }

[PATCH v7 14/14] tests/acceptance: add reverse debugging test

From: Pavel Dovgalyuk 

This is a test for GDB reverse debugging commands: reverse step and reverse 
continue.
Every test in this suite consists of two phases: record and replay.
Recording saves the execution of some instructions and makes an initial
VM snapshot to allow reverse execution.
Replay saves the order of the first instructions and then checks that they
are executed backwards in the correct order.
After that the execution is replayed to the end, and reverse continue
command is checked by setting several breakpoints, and asserting
that the execution is stopped at the last of them.

Signed-off-by: Pavel Dovgalyuk 
Reviewed-by: Philippe Mathieu-Daudé 
Reviewed-by: Willian Rampazzo 

--

v5:
 - disabled (as some other tests) when running on gitlab
   due to the unidentified timeout problem
---
 MAINTAINERS   |1 
 tests/acceptance/reverse_debugging.py |  208 +
 2 files changed, 209 insertions(+)
 create mode 100644 tests/acceptance/reverse_debugging.py

diff --git a/MAINTAINERS b/MAINTAINERS
index ea4fa3e481..bd3a7efb75 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2693,6 +2693,7 @@ F: include/sysemu/replay.h
 F: docs/replay.txt
 F: stubs/replay.c
 F: tests/acceptance/replay_kernel.py
+F: tests/acceptance/reverse_debugging.py
 F: qapi/replay.json
 
 IOVA Tree
diff --git a/tests/acceptance/reverse_debugging.py 
b/tests/acceptance/reverse_debugging.py
new file mode 100644
index 00..b72fdf6cdc
--- /dev/null
+++ b/tests/acceptance/reverse_debugging.py
@@ -0,0 +1,208 @@
+# Reverse debugging test
+#
+# Copyright (c) 2020 ISP RAS
+#
+# Author:
+#  Pavel Dovgalyuk 
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+import os
+import logging
+
+from avocado import skipIf
+from avocado_qemu import BUILD_DIR
+from avocado.utils import gdb
+from avocado.utils import process
+from avocado.utils.path import find_command
+from boot_linux_console import LinuxKernelTest
+
+class ReverseDebugging(LinuxKernelTest):
+"""
+Test GDB reverse debugging commands: reverse step and reverse continue.
+Recording saves the execution of some instructions and makes an initial
+VM snapshot to allow reverse execution.
+Replay saves the order of the first instructions and then checks that they
+are executed backwards in the correct order.
+After that the execution is replayed to the end, and reverse continue
+command is checked by setting several breakpoints, and asserting
+that the execution is stopped at the last of them.
+"""
+
+timeout = 10
+STEPS = 10
+endian_is_le = True
+
+def run_vm(self, record, shift, args, replay_path, image_path):
+logger = logging.getLogger('replay')
+vm = self.get_vm()
+vm.set_console()
+if record:
+logger.info('recording the execution...')
+mode = 'record'
+else:
+logger.info('replaying the execution...')
+mode = 'replay'
+vm.add_args('-s', '-S')
+vm.add_args('-icount', 'shift=%s,rr=%s,rrfile=%s,rrsnapshot=init' %
+(shift, mode, replay_path),
+'-net', 'none')
+vm.add_args('-drive', 'file=%s,if=none' % image_path)
+if args:
+vm.add_args(*args)
+vm.launch()
+return vm
+
+@staticmethod
+def get_reg_le(g, reg):
+res = g.cmd(b'p%x' % reg)
+num = 0
+for i in range(len(res))[-2::-2]:
+num = 0x100 * num + int(res[i:i + 2], 16)
+return num
+
+@staticmethod
+def get_reg_be(g, reg):
+res = g.cmd(b'p%x' % reg)
+return int(res, 16)
+
+def get_reg(self, g, reg):
+# value may be encoded in BE or LE order
+if self.endian_is_le:
+return self.get_reg_le(g, reg)
+else:
+return self.get_reg_be(g, reg)
+
+def get_pc(self, g):
+return self.get_reg(g, self.REG_PC)
+
+def check_pc(self, g, addr):
+pc = self.get_pc(g)
+if pc != addr:
+self.fail('Invalid PC (read %x instead of %x)' % (pc, addr))
+
+@staticmethod
+def gdb_step(g):
+g.cmd(b's', b'T05thread:01;')
+
+@staticmethod
+def gdb_bstep(g):
+g.cmd(b'bs', b'T05thread:01;')
+
+@staticmethod
+def vm_get_icount(vm):
+return vm.qmp('query-replay')['return']['icount']
+
+def reverse_debugging(self, shift=7, args=None):
+logger = logging.getLogger('replay')
+
+# create qcow2 for snapshots
+logger.info('creating qcow2 image for VM snapshots')
+image_path = os.path.join(self.workdir, 'disk.qcow2')
+qemu_img = os.path.join(BUILD_DIR, 'qemu-img')
+if not os.path.exists(qemu_img):
+qemu_img = find_command('qemu-img', False)
+if qemu_img is False:
+self.cancel('Could not find "qemu-img", which is

[PATCH v7 08/14] replay: implement replay-seek command

From: Pavel Dovgalyuk 

This patch adds hmp/qmp commands replay_seek/replay-seek that proceed
the execution to the specified instruction count.
The command automatically loads nearest snapshot and replays the execution
to find the desired instruction count.

Signed-off-by: Pavel Dovgalyuk 
Acked-by: Markus Armbruster 

--

v4 changes:
 - fixed HMP command description indent
 - removed useless error_free call
---
 hmp-commands.hx   |   18 +
 include/monitor/hmp.h |1 +
 qapi/replay.json  |   20 ++
 replay/replay-debugging.c |   87 +
 4 files changed, 126 insertions(+)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 7680d0b380..e43ce600b8 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -1836,6 +1836,24 @@ SRST
   The command is ignored when there are no replay breakpoints.
 ERST
 
+{
+.name   = "replay_seek",
+.args_type  = "icount:i",
+.params = "icount",
+.help   = "replay execution to the specified instruction count",
+.cmd= hmp_replay_seek,
+},
+
+SRST
+``replay_seek`` *icount*
+  Automatically proceed to the instruction count *icount*, when
+  replaying the execution. The command automatically loads nearest
+  snapshot and replays the execution to find the desired instruction.
+  When there is no preceding snapshot or the execution is not replayed,
+  then the command fails.
+  *icount* for the reference may be observed with ``info replay`` command.
+ERST
+
 {
 .name   = "info",
 .args_type  = "item:s?",
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index 809ad638bb..ed2913fd18 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -132,5 +132,6 @@ void hmp_info_sev(Monitor *mon, const QDict *qdict);
 void hmp_info_replay(Monitor *mon, const QDict *qdict);
 void hmp_replay_break(Monitor *mon, const QDict *qdict);
 void hmp_replay_delete_break(Monitor *mon, const QDict *qdict);
+void hmp_replay_seek(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi/replay.json b/qapi/replay.json
index 173ba76107..bfd83d7591 100644
--- a/qapi/replay.json
+++ b/qapi/replay.json
@@ -99,3 +99,23 @@
 #
 ##
 { 'command': 'replay-delete-break' }
+
+##
+# @replay-seek:
+#
+# Automatically proceed to the instruction count @icount, when
+# replaying the execution. The command automatically loads nearest
+# snapshot and replays the execution to find the desired instruction.
+# When there is no preceding snapshot or the execution is not replayed,
+# then the command fails.
+# icount for the reference may be obtained with @query-replay command.
+#
+# @icount: target instruction count
+#
+# Since: 5.2
+#
+# Example:
+#
+# -> { "execute": "replay-seek", "data": { "icount": 220414 } }
+##
+{ 'command': 'replay-seek', 'data': { 'icount': 'int' } }
diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
index 3dc23b84fc..e1fe6b8661 100644
--- a/replay/replay-debugging.c
+++ b/replay/replay-debugging.c
@@ -19,6 +19,8 @@
 #include "qapi/qapi-commands-replay.h"
 #include "qapi/qmp/qdict.h"
 #include "qemu/timer.h"
+#include "block/snapshot.h"
+#include "migration/snapshot.h"
 
 void hmp_info_replay(Monitor *mon, const QDict *qdict)
 {
@@ -125,3 +127,88 @@ void hmp_replay_delete_break(Monitor *mon, const QDict 
*qdict)
 return;
 }
 }
+
+static char *replay_find_nearest_snapshot(int64_t icount,
+  int64_t *snapshot_icount)
+{
+BlockDriverState *bs;
+QEMUSnapshotInfo *sn_tab;
+QEMUSnapshotInfo *nearest = NULL;
+char *ret = NULL;
+int nb_sns, i;
+AioContext *aio_context;
+
+*snapshot_icount = -1;
+
+bs = bdrv_all_find_vmstate_bs();
+if (!bs) {
+goto fail;
+}
+aio_context = bdrv_get_aio_context(bs);
+
+aio_context_acquire(aio_context);
+nb_sns = bdrv_snapshot_list(bs, &sn_tab);
+aio_context_release(aio_context);
+
+for (i = 0; i < nb_sns; i++) {
+if (bdrv_all_find_snapshot(sn_tab[i].name, &bs) == 0) {
+if (sn_tab[i].icount != -1ULL
+&& sn_tab[i].icount <= icount
+&& (!nearest || nearest->icount < sn_tab[i].icount)) {
+nearest = &sn_tab[i];
+}
+}
+}
+if (nearest) {
+ret = g_strdup(nearest->name);
+*snapshot_icount = nearest->icount;
+}
+g_free(sn_tab);
+
+fail:
+return ret;
+}
+
+static void replay_seek(int64_t icount, QEMUTimerCB callback, Error **errp)
+{
+char *snapshot = NULL;
+int64_t snapshot_icount;
+
+if (replay_mode != REPLAY_MODE_PLAY) {
+error_setg(errp, "replay must be enabled to seek");
+return;
+}
+
+snapshot = replay_find_nearest_snapshot(icount, &snapshot_icount);
+if (snapshot) {
+if (icount < replay_get_current_icount()
+|| replay_get_current_icount() < snapshot_icount) {
+vm_stop(RUN_STAT

Re: [PATCH v3 3/4] hw/timer/bcm2835: Support the timer COMPARE registers

On 10/2/20 11:42 AM, Philippe Mathieu-Daudé wrote:
> @@ -78,16 +71,29 @@ static void bcm2835_systmr_write(void *opaque, hwaddr 
> offset,
>   uint64_t value, unsigned size)
>  {
>  BCM2835SystemTimerState *s = BCM2835_SYSTIMER(opaque);
> +int index;
> +uint64_t now;
> +uint64_t triggers_delay_us;
>  
>  trace_bcm2835_systmr_write(offset, value);
>  switch (offset) {
>  case A_CTRL_STATUS:
>  s->reg.ctrl_status &= ~value; /* Ack */
> -bcm2835_systmr_update_irq(s);
> +for (index = 0; index < ARRAY_SIZE(s->tmr); index++) {
> +if (extract32(value, index, 1)) {
> +trace_bcm2835_systmr_irq_ack(index);
> +qemu_set_irq(s->tmr[index].irq, 0);
> +}

I think it might be instructive to have the parameter be uint64_t value64, and
the immediately do

uint32_t value = value64;

That matches up better with extract32, the trace arguments...

> +}
>  break;
>  case A_COMPARE0 ... A_COMPARE3:
> -s->reg.compare[(offset - A_COMPARE0) >> 2] = value;
> -bcm2835_systmr_update_compare(s, (offset - A_COMPARE0) >> 2);
> +index = (offset - A_COMPARE0) >> 2;
> +s->reg.compare[index] = value;
> +now = qemu_clock_get_us(QEMU_CLOCK_VIRTUAL);
> +/* Compare lower 32-bits of the free-running counter. */
> +triggers_delay_us = value - (now & UINT32_MAX);
> +trace_bcm2835_systmr_run(index, triggers_delay_us);
> +timer_mod(&s->tmr[index].timer, now + triggers_delay_us);

... and here.

Also, the arithmetic looks off.

Consider when you want a long timeout, and pass in a value slightly below now.
 So, e.g.

  now   = 0xabcd;
  value = 0xfffe;

since triggers_delay_us is uint64_t, that comparison becomes

  triggers_delay_us = 0xfffe - 0x;
= 0x;

Then you add back in now, and do *not* get a value in the future:

now + triggers_delay_us
  = 0xabcd + 0x
  = 0xabcdfffe

What I think you want is

  uint32_t triggers_delay_us = value - now
 = 0x;

which then zero-extends when you add back to now to get

now + triggers_delay_us
  = 0xabcd + 0x
  = 0xabcefffe

which is indeed in the future.


r~

[PATCH v7 09/14] replay: flush rr queue before loading the vmstate

From: Pavel Dovgalyuk 

Non-empty record/replay queue prevents saving and loading the VM state,
because it includes pending bottom halves and block coroutines.
But when the new VM state is loaded, we don't have to preserve the consistency
of the current state anymore. Therefore this patch just flushes the queue
allowing the coroutines to finish and removes checking for empty rr queue
for load_snapshot function.

Signed-off-by: Pavel Dovgalyuk 
Reviewed-by: Alex Bennée 
---
 include/sysemu/replay.h  |2 ++
 migration/savevm.c   |   12 ++--
 replay/replay-events.c   |4 
 replay/replay-internal.h |2 --
 4 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index e00ed2f4a5..239c01e7df 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -149,6 +149,8 @@ void replay_disable_events(void);
 void replay_enable_events(void);
 /*! Returns true when saving events is enabled */
 bool replay_events_enabled(void);
+/* Flushes events queue */
+void replay_flush_events(void);
 /*! Adds bottom half event to the queue */
 void replay_bh_schedule_event(QEMUBH *bh);
 /* Adds oneshot bottom half event to the queue */
diff --git a/migration/savevm.c b/migration/savevm.c
index 0e8dc78684..d2e141f7b1 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -2881,12 +2881,6 @@ int load_snapshot(const char *name, Error **errp)
 AioContext *aio_context;
 MigrationIncomingState *mis = migration_incoming_get_current();
 
-if (!replay_can_snapshot()) {
-error_setg(errp, "Record/replay does not allow loading snapshot "
-   "right now. Try once more later.");
-return -EINVAL;
-}
-
 if (!bdrv_all_can_snapshot(&bs)) {
 error_setg(errp,
"Device '%s' is writable but does not support snapshots",
@@ -2920,6 +2914,12 @@ int load_snapshot(const char *name, Error **errp)
 return -EINVAL;
 }
 
+/*
+ * Flush the record/replay queue. Now the VM state is going
+ * to change. Therefore we don't need to preserve its consistency
+ */
+replay_flush_events();
+
 /* Flush all IO requests so they don't interfere with the new state.  */
 bdrv_drain_all_begin();
 
diff --git a/replay/replay-events.c b/replay/replay-events.c
index 302b84043a..a1c6bb934e 100644
--- a/replay/replay-events.c
+++ b/replay/replay-events.c
@@ -77,6 +77,10 @@ bool replay_has_events(void)
 
 void replay_flush_events(void)
 {
+if (replay_mode == REPLAY_MODE_NONE) {
+return;
+}
+
 g_assert(replay_mutex_locked());
 
 while (!QTAILQ_EMPTY(&events_list)) {
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index 2f6145ec7c..97649ed8d7 100644
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -149,8 +149,6 @@ void replay_read_next_clock(unsigned int kind);
 void replay_init_events(void);
 /*! Clears internal data structures for events handling */
 void replay_finish_events(void);
-/*! Flushes events queue */
-void replay_flush_events(void);
 /*! Returns true if there are any unsaved events in the queue */
 bool replay_has_events(void);
 /*! Saves events from queue into the file */

Re: [PATCH v3 4/4] hw/arm/bcm2835_peripherals: Correctly wire the SYS_timer IRQs

On 10/2/20 11:42 AM, Philippe Mathieu-Daudé wrote:
> The SYS_timer is not directly wired to the ARM core, but to the
> SoC (peripheral) interrupt controller.
> 
> Fixes: 0e5bbd74064 ("hw/arm/bcm2835_peripherals: Use the SYS_timer")
> Reviewed-by: Luc Michel 
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
>  hw/arm/bcm2835_peripherals.c | 13 +++--
>  1 file changed, 11 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson 


r~

[PATCH v7 06/14] replay: introduce info hmp/qmp command

From: Pavel Dovgalyuk 

This patch introduces 'info replay' monitor command and
corresponding qmp request.
These commands request the current record/replay mode, replay log file
name, and the instruction count (number of recorded/replayed
instructions).  The instruction count can be used with the
replay_seek/replay_break commands added in the next two patches.

Signed-off-by: Pavel Dovgalyuk 
Acked-by: Dr. David Alan Gilbert 
Acked-by: Markus Armbruster 
---
 hmp-commands-info.hx  |   11 +++
 include/monitor/hmp.h |1 +
 qapi/block-core.json  |3 ++-
 qapi/replay.json  |   39 +++
 replay/meson.build|1 +
 replay/replay-debugging.c |   43 +++
 6 files changed, 97 insertions(+), 1 deletion(-)
 create mode 100644 replay/replay-debugging.c

diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 30209e3903..117ba25f91 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -881,4 +881,15 @@ SRST
 Show SEV information.
 ERST
 
+{
+.name   = "replay",
+.args_type  = "",
+.params = "",
+.help   = "show record/replay information",
+.cmd= hmp_info_replay,
+},
 
+SRST
+  ``info replay``
+Display the record/replay information: mode and the current icount.
+ERST
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index 642e9e91f9..f297fccce8 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -129,5 +129,6 @@ void hmp_hotpluggable_cpus(Monitor *mon, const QDict 
*qdict);
 void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict);
 void hmp_info_memory_size_summary(Monitor *mon, const QDict *qdict);
 void hmp_info_sev(Monitor *mon, const QDict *qdict);
+void hmp_info_replay(Monitor *mon, const QDict *qdict);
 
 #endif
diff --git a/qapi/block-core.json b/qapi/block-core.json
index 015cc0afa4..a4c6c59bd8 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -29,7 +29,8 @@
 #
 # @icount: Current instruction count. Appears when execution record/replay
 #  is enabled. Used for "time-traveling" to match the moment
-#  in the recorded execution with the snapshots. (since 5.2)
+#  in the recorded execution with the snapshots. This counter may
+#  be obtained through @query-replay command (since 5.2)
 #
 # Since: 1.3
 #
diff --git a/qapi/replay.json b/qapi/replay.json
index 9e13551d20..e6b3f6001d 100644
--- a/qapi/replay.json
+++ b/qapi/replay.json
@@ -24,3 +24,42 @@
 ##
 { 'enum': 'ReplayMode',
   'data': [ 'none', 'record', 'play' ] }
+
+##
+# @ReplayInfo:
+#
+# Record/replay information.
+#
+# @mode: current mode.
+#
+# @filename: name of the record/replay log file.
+#It is present only in record or replay modes, when the log
+#is recorded or replayed.
+#
+# @icount: current number of executed instructions.
+#
+# Since: 5.2
+#
+##
+{ 'struct': 'ReplayInfo',
+  'data': { 'mode': 'ReplayMode', '*filename': 'str', 'icount': 'int' } }
+
+##
+# @query-replay:
+#
+# Retrieve the record/replay information.
+# It includes current instruction count which may be used for
+# @replay-break and @replay-seek commands.
+#
+# Returns: record/replay information.
+#
+# Since: 5.2
+#
+# Example:
+#
+# -> { "execute": "query-replay" }
+# <- { "return": { "mode": "play", "filename": "log.rr", "icount": 220414 } }
+#
+##
+{ 'command': 'query-replay',
+  'returns': 'ReplayInfo' }
diff --git a/replay/meson.build b/replay/meson.build
index 8783aea7c8..f91163fb1e 100644
--- a/replay/meson.build
+++ b/replay/meson.build
@@ -9,4 +9,5 @@ softmmu_ss.add(files(
   'replay-net.c',
   'replay-audio.c',
   'replay-random.c',
+  'replay-debugging.c',
 ))
diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
new file mode 100644
index 00..51a6de4e81
--- /dev/null
+++ b/replay/replay-debugging.c
@@ -0,0 +1,43 @@
+/*
+ * replay-debugging.c
+ *
+ * Copyright (c) 2010-2020 Institute for System Programming
+ * of the Russian Academy of Sciences.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "sysemu/replay.h"
+#include "replay-internal.h"
+#include "monitor/hmp.h"
+#include "monitor/monitor.h"
+#include "qapi/qapi-commands-replay.h"
+
+void hmp_info_replay(Monitor *mon, const QDict *qdict)
+{
+if (replay_mode == REPLAY_MODE_NONE) {
+monitor_printf(mon, "Record/replay is not active\n");
+} else {
+monitor_printf(mon,
+"%s execution '%s': instruction count = %"PRId64"\n",
+replay_mode == REPLAY_MODE_RECORD ? "Recording" : "Replaying",
+replay_get_filename(), replay_get_current_icount());
+}
+}
+
+ReplayInfo *qmp_query_replay(Error **errp)
+{
+ReplayInfo *retval = g_new0(ReplayInfo, 1);
+
+retval-

[PATCH v7 10/14] gdbstub: add reverse step support in replay mode

From: Pavel Dovgalyuk 

GDB remote protocol supports two reverse debugging commands:
reverse step and reverse continue.
This patch adds support of the first one to the gdbstub.
Reverse step is intended to step one instruction in the backwards
direction. This is not possible in regular execution.
But replayed execution is deterministic, therefore we can load one of
the prior snapshots and proceed to the desired step. It is equivalent
to stepping one instruction back.
There should be at least one snapshot preceding the debugged part of
the replay log.

Signed-off-by: Pavel Dovgalyuk 
Reviewed-by: Alex Bennée 

--

v4 changes:
 - inverted condition in cpu_handle_guest_debug (suggested by Alex Bennée)
---
 accel/tcg/translator.c|1 +
 exec.c|7 ++
 gdbstub.c |   55 +++--
 include/sysemu/replay.h   |   11 +
 replay/replay-debugging.c |   34 
 softmmu/cpus.c|   14 +--
 stubs/replay.c|5 
 7 files changed, 122 insertions(+), 5 deletions(-)

diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 603d17ff83..fb1e19c585 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -17,6 +17,7 @@
 #include "exec/log.h"
 #include "exec/translator.h"
 #include "exec/plugin-gen.h"
+#include "sysemu/replay.h"
 
 /* Pairs with tcg_clear_temp_count.
To be called by #TranslatorOps.{translate_insn,tb_stop} if
diff --git a/exec.c b/exec.c
index c088559d94..80db218c99 100644
--- a/exec.c
+++ b/exec.c
@@ -2752,6 +2752,13 @@ void cpu_check_watchpoint(CPUState *cpu, vaddr addr, 
vaddr len,
 QTAILQ_FOREACH(wp, &cpu->watchpoints, entry) {
 if (watchpoint_address_matches(wp, addr, len)
 && (wp->flags & flags)) {
+if (replay_running_debug()) {
+/*
+ * Don't process the watchpoints when we are
+ * in a reverse debugging operation.
+ */
+return;
+}
 if (flags == BP_MEM_READ) {
 wp->flags |= BP_WATCHPOINT_HIT_READ;
 } else {
diff --git a/gdbstub.c b/gdbstub.c
index 9dfb6e4142..79e8ccc050 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -51,6 +51,7 @@
 #include "sysemu/runstate.h"
 #include "hw/semihosting/semihost.h"
 #include "exec/exec-all.h"
+#include "sysemu/replay.h"
 
 #ifdef CONFIG_USER_ONLY
 #define GDB_ATTACHED "0"
@@ -375,6 +376,20 @@ typedef struct GDBState {
  */
 static int sstep_flags = SSTEP_ENABLE|SSTEP_NOIRQ|SSTEP_NOTIMER;
 
+/* Retrieves flags for single step mode. */
+static int get_sstep_flags(void)
+{
+/*
+ * In replay mode all events written into the log should be replayed.
+ * That is why NOIRQ flag is removed in this mode.
+ */
+if (replay_mode != REPLAY_MODE_NONE) {
+return SSTEP_ENABLE;
+} else {
+return sstep_flags;
+}
+}
+
 static GDBState gdbserver_state;
 
 static void init_gdbserver_state(void)
@@ -501,7 +516,7 @@ static int gdb_continue_partial(char *newstates)
 break; /* nothing to do here */
 case 's':
 trace_gdbstub_op_stepping(cpu->cpu_index);
-cpu_single_step(cpu, sstep_flags);
+cpu_single_step(cpu, get_sstep_flags());
 cpu_resume(cpu);
 flag = 1;
 break;
@@ -1874,10 +1889,31 @@ static void handle_step(GdbCmdContext *gdb_ctx, void 
*user_ctx)
 gdb_set_cpu_pc((target_ulong)gdb_ctx->params[0].val_ull);
 }
 
-cpu_single_step(gdbserver_state.c_cpu, sstep_flags);
+cpu_single_step(gdbserver_state.c_cpu, get_sstep_flags());
 gdb_continue();
 }
 
+static void handle_backward(GdbCmdContext *gdb_ctx, void *user_ctx)
+{
+if (replay_mode != REPLAY_MODE_PLAY) {
+put_packet("E22");
+}
+if (gdb_ctx->num_params == 1) {
+switch (gdb_ctx->params[0].opcode) {
+case 's':
+if (replay_reverse_step()) {
+gdb_continue();
+} else {
+put_packet("E14");
+}
+return;
+}
+}
+
+/* Default invalid command */
+put_packet("");
+}
+
 static void handle_v_cont_query(GdbCmdContext *gdb_ctx, void *user_ctx)
 {
 put_packet("vCont;c;C;s;S");
@@ -2124,6 +2160,10 @@ static void handle_query_supported(GdbCmdContext 
*gdb_ctx, void *user_ctx)
 g_string_append(gdbserver_state.str_buf, ";qXfer:features:read+");
 }
 
+if (replay_mode == REPLAY_MODE_PLAY) {
+g_string_append(gdbserver_state.str_buf, ";ReverseStep+");
+}
+
 if (gdb_ctx->num_params &&
 strstr(gdb_ctx->params[0].data, "multiprocess+")) {
 gdbserver_state.multiprocess = true;
@@ -2460,6 +2500,17 @@ static int gdb_handle_packet(const char *line_buf)
 cmd_parser = &step_cmd_desc;
 }
 break;
+case 'b':
+{
+static const GdbC

[PATCH v7 11/14] gdbstub: add reverse continue support in replay mode

From: Pavel Dovgalyuk 

This patch adds support of the reverse continue operation for gdbstub.
Reverse continue finds the last breakpoint that would happen in normal
execution from the beginning to the current moment.
Implementation of the reverse continue replays the execution twice:
to find the breakpoints that were hit and to seek to the last breakpoint.
Reverse continue loads the previous snapshot and tries to find the breakpoint
since that moment. If there are no such breakpoints, it proceeds to
the earlier snapshot, and so on. When no breakpoints or watchpoints were
hit at all, execution stops at the beginning of the replay log.

Signed-off-by: Pavel Dovgalyuk 
---
 exec.c|1 +
 gdbstub.c |   10 ++
 include/sysemu/replay.h   |8 +
 replay/replay-debugging.c |   72 +
 softmmu/cpus.c|5 +++
 stubs/replay.c|5 +++
 6 files changed, 100 insertions(+), 1 deletion(-)

diff --git a/exec.c b/exec.c
index 80db218c99..f4a6d4cf62 100644
--- a/exec.c
+++ b/exec.c
@@ -2757,6 +2757,7 @@ void cpu_check_watchpoint(CPUState *cpu, vaddr addr, 
vaddr len,
  * Don't process the watchpoints when we are
  * in a reverse debugging operation.
  */
+replay_breakpoint();
 return;
 }
 if (flags == BP_MEM_READ) {
diff --git a/gdbstub.c b/gdbstub.c
index 79e8ccc050..ac92273018 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -1907,6 +1907,13 @@ static void handle_backward(GdbCmdContext *gdb_ctx, void 
*user_ctx)
 put_packet("E14");
 }
 return;
+case 'c':
+if (replay_reverse_continue()) {
+gdb_continue();
+} else {
+put_packet("E14");
+}
+return;
 }
 }
 
@@ -2161,7 +2168,8 @@ static void handle_query_supported(GdbCmdContext 
*gdb_ctx, void *user_ctx)
 }
 
 if (replay_mode == REPLAY_MODE_PLAY) {
-g_string_append(gdbserver_state.str_buf, ";ReverseStep+");
+g_string_append(gdbserver_state.str_buf,
+";ReverseStep+;ReverseContinue+");
 }
 
 if (gdb_ctx->num_params &&
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index 13a8123b09..b6cac175c4 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -81,11 +81,19 @@ const char *replay_get_filename(void);
  * Returns true on success.
  */
 bool replay_reverse_step(void);
+/*
+ * Start searching the last breakpoint/watchpoint.
+ * Used by gdbstub for backwards debugging.
+ * Returns true if the process successfully started.
+ */
+bool replay_reverse_continue(void);
 /*
  * Returns true if replay module is processing
  * reverse_continue or reverse_step request
  */
 bool replay_running_debug(void);
+/* Called in reverse debugging mode to collect breakpoint information */
+void replay_breakpoint(void);
 
 /* Processing the instructions */
 
diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
index 1e1dec0295..30ca38e5dd 100644
--- a/replay/replay-debugging.c
+++ b/replay/replay-debugging.c
@@ -23,6 +23,8 @@
 #include "migration/snapshot.h"
 
 static bool replay_is_debugging;
+static int64_t replay_last_breakpoint;
+static int64_t replay_last_snapshot;
 
 bool replay_running_debug(void)
 {
@@ -246,3 +248,73 @@ bool replay_reverse_step(void)
 
 return false;
 }
+
+static void replay_continue_end(void)
+{
+replay_is_debugging = false;
+vm_stop(RUN_STATE_DEBUG);
+replay_delete_break();
+}
+
+static void replay_continue_stop(void *opaque)
+{
+Error *err = NULL;
+if (replay_last_breakpoint != -1LL) {
+replay_seek(replay_last_breakpoint, replay_stop_vm_debug, &err);
+if (err) {
+error_free(err);
+replay_continue_end();
+}
+return;
+}
+/*
+ * No breakpoints since the last snapshot.
+ * Find previous snapshot and try again.
+ */
+if (replay_last_snapshot != 0) {
+replay_seek(replay_last_snapshot - 1, replay_continue_stop, &err);
+if (err) {
+error_free(err);
+replay_continue_end();
+}
+replay_last_snapshot = replay_get_current_icount();
+return;
+} else {
+/* Seek to the very first step */
+replay_seek(0, replay_stop_vm_debug, &err);
+if (err) {
+error_free(err);
+replay_continue_end();
+}
+return;
+}
+replay_continue_end();
+}
+
+bool replay_reverse_continue(void)
+{
+Error *err = NULL;
+
+assert(replay_mode == REPLAY_MODE_PLAY);
+
+if (replay_get_current_icount() != 0) {
+replay_seek(replay_get_current_icount() - 1,
+replay_continue_stop, &err);
+if (err) {
+error_free(err);
+return false;
+}
+replay_last_breakpoint = -1LL;
+

[PATCH v7 13/14] replay: create temporary snapshot at debugger connection

When record/replay does not uses overlays for storing the snapshots,
user is not capable of issuing reverse debugging commands.
This patch adds creation of the VM snapshot on the temporary
overlay image, when the debugger connects to QEMU.
Therefore the execution can be rewind to the moment
of the debugger connection while debugging the virtual machine.

Signed-off-by: Pavel Dovgalyuk 

--

v6:
 - dropped unused error processing (suggested by Philippe Mathieu-Daudé)
---
 gdbstub.c |1 +
 include/sysemu/replay.h   |2 ++
 replay/replay-debugging.c |   14 ++
 3 files changed, 17 insertions(+)

diff --git a/gdbstub.c b/gdbstub.c
index ac92273018..f19f98ab1a 100644
--- a/gdbstub.c
+++ b/gdbstub.c
@@ -3321,6 +3321,7 @@ static void gdb_chr_event(void *opaque, QEMUChrEvent 
event)
 s->g_cpu = s->c_cpu;
 
 vm_stop(RUN_STATE_PAUSED);
+replay_gdb_attached();
 gdb_has_xml = false;
 break;
 default:
diff --git a/include/sysemu/replay.h b/include/sysemu/replay.h
index b6cac175c4..2aa34b8919 100644
--- a/include/sysemu/replay.h
+++ b/include/sysemu/replay.h
@@ -94,6 +94,8 @@ bool replay_reverse_continue(void);
 bool replay_running_debug(void);
 /* Called in reverse debugging mode to collect breakpoint information */
 void replay_breakpoint(void);
+/* Called when gdb is attached to gdbstub */
+void replay_gdb_attached(void);
 
 /* Processing the instructions */
 
diff --git a/replay/replay-debugging.c b/replay/replay-debugging.c
index 30ca38e5dd..ee9e86daa9 100644
--- a/replay/replay-debugging.c
+++ b/replay/replay-debugging.c
@@ -318,3 +318,17 @@ void replay_breakpoint(void)
 assert(replay_mode == REPLAY_MODE_PLAY);
 replay_last_breakpoint = replay_get_current_icount();
 }
+
+void replay_gdb_attached(void)
+{
+/*
+ * Create VM snapshot on temporary overlay to allow reverse
+ * debugging even if snapshots were not enabled.
+ */
+if (replay_mode == REPLAY_MODE_PLAY
+&& !replay_snapshot) {
+if (save_snapshot("start_debugging", NULL) != 0) {
+/* Can't create the snapshot. Continue conventional debugging. */
+}
+}
+}

Re: [PATCH v6 00/14] Reverse debugging


On 02.10.2020 18:39, Paolo Bonzini wrote:

On 29/09/20 13:01, Pavel Dovgalyuk wrote:

GDB remote protocol supports reverse debugging of the targets.
It includes 'reverse step' and 'reverse continue' operations.
The first one finds the previous step of the execution,
and the second one is intended to stop at the last breakpoint that
would happen when the program is executed normally.

Reverse debugging is possible in the replay mode, when at least
one snapshot was created at the record or replay phase.
QEMU can use these snapshots for travelling back in time with GDB.

Running the execution in replay mode allows using GDB reverse debugging
commands:
  - reverse-stepi (or rsi): Steps one instruction to the past.
QEMU loads on of the prior snapshots and proceeds to the desired
instruction forward. When that step is reaches, execution stops.
  - reverse-continue (or rc): Runs execution "backwards".
QEMU tries to find breakpoint or watchpoint by loaded prior snapshot
and replaying the execution. Then QEMU loads snapshots again and
replays to the latest breakpoint. When there are no breakpoints in
the examined section of the execution, QEMU finds one more snapshot
and tries again. After the first snapshot is processed, execution
stops at this snapshot.

The set of patches include the following modifications:
  - gdbstub update for reverse debugging support
  - functions that automatically perform reverse step and reverse
continue operations
  - hmp/qmp commands for manipulating the replay process
  - improvement of the snapshotting for saving the execution step
in the snapshot parameters
  - avocado-based acceptance tests for reverse debugging

The patches are available in the repository:
https://github.com/ispras/qemu/tree/rr-200901


Hi Pavel,

I'm still seeing failures in "make check-block":

https://gitlab.com/bonzini/qemu/-/jobs/769653852


Please, find the new version.
The patches 3 and 4 were updated.

However, is there any reason that not all the tests are executed?
I ran 'check -qcow2' without list of the tests, and encountered some 
problems with test 286 (that are fixed now).
But this test was not executed in your gitlab repository and with 'make 
check' on my local machine.




Paolo


v6 changes:
  - removed passing err variable without checking it's value after
v5 changes:
  - disabled reverse debugging tests for gitlab-based testing
due to the unidentified timeout problem
v4 changes:
  - added VM snapshot creation on gdb connect (suggested by Alex Bennée)
  - removed useless calls to error_free
  - updated poll interrupt processing
  - minor changes
v3 changes:
  - rebased to support the new build system
  - bumped avocado framework version for using fixed remote gdb client
v2 changes:
  - rebased to the latest upstream version
  - fixed replaying of the POLL interrupts after the latest debug changes

---

Pavel Dovgaluk (11):
   replay: provide an accessor for rr filename
   qcow2: introduce icount field for snapshots
   qapi: introduce replay.json for record/replay-related stuff
   replay: introduce info hmp/qmp command
   replay: introduce breakpoint at the specified step
   replay: implement replay-seek command
   replay: flush rr queue before loading the vmstate
   gdbstub: add reverse step support in replay mode
   gdbstub: add reverse continue support in replay mode
   replay: describe reverse debugging in docs/replay.txt
   tests/acceptance: add reverse debugging test

Pavel Dovgalyuk (3):
   replay: don't record interrupt poll
   migration: introduce icount field for snapshots
   replay: create temporary snapshot at debugger connection


  MAINTAINERS   |2
  accel/tcg/cpu-exec.c  |   21 ++
  accel/tcg/translator.c|1
  block/qapi.c  |   18 +-
  block/qcow2-snapshot.c|9 +
  block/qcow2.h |3
  blockdev.c|   10 +
  docs/interop/qcow2.txt|5
  docs/replay.txt   |   46 +
  exec.c|8 +
  gdbstub.c |   64 ++
  hmp-commands-info.hx  |   11 +
  hmp-commands.hx   |   50 +
  include/block/snapshot.h  |1
  include/monitor/hmp.h |4
  include/sysemu/replay.h   |   26 +++
  migration/savevm.c|   17 +-
  qapi/block-core.json  |   11 +
  qapi/meson.build  |1
  qapi/misc.json|   18 --
  qapi/qapi-schema.json |1
  qapi/replay.json  |  121 
  replay/meson.build|1
  replay/replay-debugging.c |  332 +
  replay/replay-events.c|4
  replay/

Re: [PATCH v2] scripts/qmp/qom-set: Allow setting integer value

Hi Jonatan,

On 10/2/20 10:52 PM, Jonatan Pålsson wrote:
> If the value appears to be an integer, parse it as such.
> 
> This allows the following:
> 
> qmp/qom-set -s ~/qmp.sock sensor.temperature 2

Maybe instead:

Fix the following error:

  $ scripts/qmp/qom-set -s ~/qmp.sock sensor.temperature 2
  Traceback (most recent call last):
File "scripts/qmp/qom-set", line 66, in 
  print(srv.command('qom-set', path=path, property=prop, value=value))
File "scripts/qmp/../../python/qemu/qmp.py", line 274, in command
  raise QMPResponseError(ret)
  qemu.qmp.QMPResponseError: Invalid parameter type for 'temperature',
expected: integer

> 
> .. where sensor is a tmp105 device, and temperature is an integer
> property.
> 
> Signed-off-by: Jonatan Pålsson 
> ---
>  scripts/qmp/qom-set | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/qmp/qom-set b/scripts/qmp/qom-set
> index 240a78187f..49eebe4924 100755
> --- a/scripts/qmp/qom-set
> +++ b/scripts/qmp/qom-set
> @@ -56,7 +56,10 @@ if len(args) > 1:
>  path, prop = args[0].rsplit('.', 1)
>  except:
>  usage_error("invalid format for path/property/value")
> -value = args[1]
> +try:
> +value = int(args[1])

Maybe 'long' is safer?

> +except ValueError:
> +value = args[1]
>  else:
>  usage_error("not enough arguments")
>  
>

Re: [PATCH v10 4/8] linux-user/elfload: Fix coding style in load_elf_image

On 10/2/20 11:59 PM, Richard Henderson wrote:
> Fixing this now will clarify following patches.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  linux-user/elfload.c | 12 +---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index f6022fd704..7572a32a30 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -2531,9 +2531,15 @@ static void load_elf_image(const char *image_name, int 
> image_fd,
>  abi_ulong vaddr, vaddr_po, vaddr_ps, vaddr_ef, vaddr_em, 
> vaddr_len;
>  int elf_prot = 0;
>  
> -if (eppnt->p_flags & PF_R) elf_prot =  PROT_READ;
> -if (eppnt->p_flags & PF_W) elf_prot |= PROT_WRITE;
> -if (eppnt->p_flags & PF_X) elf_prot |= PROT_EXEC;
> +if (eppnt->p_flags & PF_R) {
> +elf_prot |= PROT_READ;
> +}
> +if (eppnt->p_flags & PF_W) {
> +elf_prot |= PROT_WRITE;
> +}
> +if (eppnt->p_flags & PF_X) {
> +elf_prot |= PROT_EXEC;
> +}
>  
>  vaddr = load_bias + eppnt->p_vaddr;
>  vaddr_po = TARGET_ELF_PAGEOFFSET(vaddr);
>

Re: [PATCH 5/5] memory: Skip bad range assertion if notifier is DEVIOTLB_UNMAP type

2020-10-03 Thread Michael S. Tsirkin

On Mon, Sep 28, 2020 at 01:48:57PM -0400, Peter Xu wrote:
> On Mon, Sep 28, 2020 at 11:05:01AM +0200, Eugenio Perez Martin wrote:
> > On Fri, Sep 4, 2020 at 6:34 AM Jason Wang  wrote:
> > >
> > >
> > > On 2020/9/4 上午12:14, Eugenio Pérez wrote:
> > > > Device IOTLB invalidations can unmap arbitrary ranges, eiter outside of
> > > > the memory region or even [0, ~0ULL] for all the space. The assertion
> > > > could be hit by a guest, and rhel7 guest effectively hit it.
> > > >
> > > > Signed-off-by: Eugenio Pérez 
> > > > Reviewed-by: Peter Xu 
> > > > Reviewed-by: Juan Quintela 
> > > > ---
> > > >   softmmu/memory.c | 13 +++--
> > > >   1 file changed, 11 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/softmmu/memory.c b/softmmu/memory.c
> > > > index 8694fc7cf7..e723fcbaa1 100644
> > > > --- a/softmmu/memory.c
> > > > +++ b/softmmu/memory.c
> > > > @@ -1895,6 +1895,7 @@ void memory_region_notify_iommu_one(IOMMUNotifier 
> > > > *notifier,
> > > >   {
> > > >   IOMMUTLBEntry *entry = &event->entry;
> > > >   hwaddr entry_end = entry->iova + entry->addr_mask;
> > > > +IOMMUTLBEntry tmp = *entry;
> > > >
> > > >   if (event->type == IOMMU_NOTIFIER_UNMAP) {
> > > >   assert(entry->perm == IOMMU_NONE);
> > > > @@ -1908,10 +1909,18 @@ void 
> > > > memory_region_notify_iommu_one(IOMMUNotifier *notifier,
> > > >   return;
> > > >   }
> > > >
> > > > -assert(entry->iova >= notifier->start && entry_end <= 
> > > > notifier->end);
> > > > +if (notifier->notifier_flags & IOMMU_NOTIFIER_DEVIOTLB_UNMAP) {
> > > > +/* Crop (iova, addr_mask) to range */
> > > > +tmp.iova = MAX(tmp.iova, notifier->start);
> > > > +tmp.addr_mask = MIN(entry_end, notifier->end) - tmp.iova;
> > > > +/* Confirm no underflow */
> > > > +assert(MIN(entry_end, notifier->end) >= tmp.iova);
> > >
> > >
> > > It's still not clear to me why we need such assert. Consider
> > > notifier->end is the possible IOVA range but not possible device IOTLB
> > > invalidation range (e.g it allows [0, ULLONG_MAX]).
> > >
> > > Thanks
> > >
> > 
> > As far as I understood the device should admit that out of bounds
> > notifications in that case,
> > and the assert just makes sure that there was no underflow in
> > tmp.addr_mask, i.e., that something
> > very wrong that should never happen in production happened.
> > 
> > Peter, would you mind to confirm/correct it?
> 
> I think Jason is right - since we have checked at the entry that the two
> regions cross over each other:
> 
> /*
>  * Skip the notification if the notification does not overlap
>  * with registered range.
>  */
> if (notifier->start > entry_end || notifier->end < entry->iova) {
> return;
> }
> 
> Then I don't see how this assertion can fail any more.
> 
> But imho not a big problem either, and it shouldn't hurt to even keep the
> assertion of above isn't that straightforward.
> 
> > 
> > Is there anything else needed to pull this patch?
> 
> I didn't post a pull for this only because I shouldn't :) - the plan was that
> all vt-d patches will still go via Michael's tree, iiuc.  Though at least to 
> me
> I think this series is acceptable for merging.

Sure, that's ok.

Eugenio, you sent patch 0 as a response to another series, which
made me miss the series. Pls don't do that in the future.

Looks like Jason reviewed the series - Jason, is that right? -
so I'd like his ack if possible.


> Though it would always be good too if Jason would still like to review it.
> 
> Jason, what's your opinion?
> 
> Thanks,
> 
> -- 
> Peter Xu

Re: [PATCH v10 6/8] linux-user/elfload: Move PT_INTERP detection to first loop

On 10/2/20 11:59 PM, Richard Henderson wrote:
> For BTI, we need to know if the executable is static or dynamic,
> which means looking for PT_INTERP earlier.
> 
> Signed-off-by: Richard Henderson 
> ---
>  linux-user/elfload.c | 60 +++-
>  1 file changed, 31 insertions(+), 29 deletions(-)
> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 735ebfa190..6b422990ff 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -2421,8 +2421,10 @@ static void load_elf_image(const char *image_name, int 
> image_fd,
>  
>  mmap_lock();
>  
> -/* Find the maximum size of the image and allocate an appropriate
> -   amount of memory to handle that.  */
> +/*
> + * Find the maximum size of the image and allocate an appropriate
> + * amount of memory to handle that.  Locate the interpreter, if any.
> + */
>  loaddr = -1, hiaddr = 0;
>  info->alignment = 0;
>  for (i = 0; i < ehdr->e_phnum; ++i) {
> @@ -2438,6 +2440,33 @@ static void load_elf_image(const char *image_name, int 
> image_fd,
>  }
>  ++info->nsegs;
>  info->alignment |= eppnt->p_align;
> +} else if (eppnt->p_type == PT_INTERP && pinterp_name) {
> +char *interp_name;
> +
> +if (*pinterp_name) {
> +errmsg = "Multiple PT_INTERP entries";
> +goto exit_errmsg;
> +}
> +interp_name = malloc(eppnt->p_filesz);
> +if (!interp_name) {
> +goto exit_perror;
> +}
> +
> +if (eppnt->p_offset + eppnt->p_filesz <= BPRM_BUF_SIZE) {
> +memcpy(interp_name, bprm_buf + eppnt->p_offset,
> +   eppnt->p_filesz);
> +} else {
> +retval = pread(image_fd, interp_name, eppnt->p_filesz,
> +   eppnt->p_offset);
> +if (retval != eppnt->p_filesz) {

Preexisting, free(interp_name)?

> +goto exit_perror;
> +}
> +}
> +if (interp_name[eppnt->p_filesz - 1] != 0) {
> +errmsg = "Invalid PT_INTERP entry";

Ditto, otherwise:
Reviewed-by: Philippe Mathieu-Daudé 

> +goto exit_errmsg;
> +}
> +*pinterp_name = interp_name;
>  }
>  }
>  
> @@ -2590,33 +2619,6 @@ static void load_elf_image(const char *image_name, int 
> image_fd,
>  if (vaddr_em > info->brk) {
>  info->brk = vaddr_em;
>  }
> -} else if (eppnt->p_type == PT_INTERP && pinterp_name) {
> -char *interp_name;
> -
> -if (*pinterp_name) {
> -errmsg = "Multiple PT_INTERP entries";
> -goto exit_errmsg;
> -}
> -interp_name = malloc(eppnt->p_filesz);
> -if (!interp_name) {
> -goto exit_perror;
> -}
> -
> -if (eppnt->p_offset + eppnt->p_filesz <= BPRM_BUF_SIZE) {
> -memcpy(interp_name, bprm_buf + eppnt->p_offset,
> -   eppnt->p_filesz);
> -} else {
> -retval = pread(image_fd, interp_name, eppnt->p_filesz,
> -   eppnt->p_offset);
> -if (retval != eppnt->p_filesz) {
> -goto exit_perror;
> -}
> -}
> -if (interp_name[eppnt->p_filesz - 1] != 0) {
> -errmsg = "Invalid PT_INTERP entry";
> -goto exit_errmsg;
> -}
> -*pinterp_name = interp_name;
>  #ifdef TARGET_MIPS
>  } else if (eppnt->p_type == PT_MIPS_ABIFLAGS) {
>  Mips_elf_abiflags_v0 abiflags;
>

[PATCH] linux-user/elfload: Avoid leaking interp_name using GLib memory API

Fix an unlikely memory leak in load_elf_image().

Fixes: bf858897b7 ("linux-user: Re-use load_elf_image for the main binary.")
Signed-off-by: Philippe Mathieu-Daudé 
---
 linux-user/elfload.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index f6022fd704..1a3150df7c 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2584,13 +2584,13 @@ static void load_elf_image(const char *image_name, int 
image_fd,
 info->brk = vaddr_em;
 }
 } else if (eppnt->p_type == PT_INTERP && pinterp_name) {
-char *interp_name;
+g_autofree char *interp_name = NULL;
 
 if (*pinterp_name) {
 errmsg = "Multiple PT_INTERP entries";
 goto exit_errmsg;
 }
-interp_name = malloc(eppnt->p_filesz);
+interp_name = g_malloc(eppnt->p_filesz);
 if (!interp_name) {
 goto exit_perror;
 }
@@ -2609,7 +2609,7 @@ static void load_elf_image(const char *image_name, int 
image_fd,
 errmsg = "Invalid PT_INTERP entry";
 goto exit_errmsg;
 }
-*pinterp_name = interp_name;
+*pinterp_name = g_steal_pointer(&interp_name);
 #ifdef TARGET_MIPS
 } else if (eppnt->p_type == PT_MIPS_ABIFLAGS) {
 Mips_elf_abiflags_v0 abiflags;
@@ -2961,7 +2961,7 @@ int load_elf_binary(struct linux_binprm *bprm, struct 
image_info *info)
 if (elf_interpreter) {
 info->load_bias = interp_info.load_bias;
 info->entry = interp_info.entry;
-free(elf_interpreter);
+g_free(elf_interpreter);
 }
 
 #ifdef USE_ELF_CORE_DUMP
-- 
2.26.2

Re: [PATCH v10 5/8] linux-user/elfload: Adjust iteration over phdr

On 10/2/20 11:59 PM, Richard Henderson wrote:
> The second loop uses a loop induction variable, and the first
> does not.  Transform the first to match the second, to simplify
> a following patch moving code between them.
> 
> Signed-off-by: Richard Henderson 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  linux-user/elfload.c | 9 +
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 7572a32a30..735ebfa190 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -2426,17 +2426,18 @@ static void load_elf_image(const char *image_name, 
> int image_fd,
>  loaddr = -1, hiaddr = 0;
>  info->alignment = 0;
>  for (i = 0; i < ehdr->e_phnum; ++i) {
> -if (phdr[i].p_type == PT_LOAD) {
> -abi_ulong a = phdr[i].p_vaddr - phdr[i].p_offset;
> +struct elf_phdr *eppnt = phdr + i;
> +if (eppnt->p_type == PT_LOAD) {
> +abi_ulong a = eppnt->p_vaddr - eppnt->p_offset;
>  if (a < loaddr) {
>  loaddr = a;
>  }
> -a = phdr[i].p_vaddr + phdr[i].p_memsz;
> +a = eppnt->p_vaddr + eppnt->p_memsz;
>  if (a > hiaddr) {
>  hiaddr = a;
>  }
>  ++info->nsegs;
> -info->alignment |= phdr[i].p_align;
> +info->alignment |= eppnt->p_align;
>  }
>  }
>  
>

Re: [PATCH 4/6] docs/devel/qom: Use emphasis for emphasis

On 10/3/20 4:54 AM, Eduardo Habkost wrote:
>  is not valid reST syntax.
> 
> Signed-off-by: Eduardo Habkost 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  docs/devel/qom.rst | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst
> index a47e1b9a239..0c610e20d62 100644
> --- a/docs/devel/qom.rst
> +++ b/docs/devel/qom.rst
> @@ -174,17 +174,17 @@ dynamically cast it to an object that implements the 
> interface.
>  Methods
>  ===
>  
> -A method is a function within the namespace scope of
> +A *method* is a function within the namespace scope of
>  a class. It usually operates on the object instance by passing it as a
>  strongly-typed first argument.
>  If it does not operate on an object instance, it is dubbed
> -class method.
> +*class method*.
>  
>  Methods cannot be overloaded. That is, the #ObjectClass and method name
>  uniquely identity the function to be called; the signature does not vary
>  except for trailing varargs.
>  
> -Methods are always virtual. Overriding a method in
> +Methods are always *virtual*. Overriding a method in
>  #TypeInfo.class_init of a subclass leads to any user of the class obtained
>  via OBJECT_GET_CLASS() accessing the overridden function.
>  The original function is not automatically invoked. It is the responsibility
>

Re: [PATCH 6/6] docs/devel/qom: Avoid long lines

On 10/3/20 4:54 AM, Eduardo Habkost wrote:
> Long code lines don't look good in the rendered documents, make
> them shorter.
> 
> Signed-off-by: Eduardo Habkost 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  docs/devel/qom.rst | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/devel/qom.rst b/docs/devel/qom.rst
> index 0c610e20d62..42d0dc4f4da 100644
> --- a/docs/devel/qom.rst
> +++ b/docs/devel/qom.rst
> @@ -284,7 +284,8 @@ in the header file:
>  .. code-block:: c
> :caption: Declaring a simple type
>  
> -   OBJECT_DECLARE_SIMPLE_TYPE(MyDevice, my_device, MY_DEVICE, DEVICE)
> +   OBJECT_DECLARE_SIMPLE_TYPE(MyDevice, my_device,
> +  MY_DEVICE, DEVICE)
>  
>  This is equivalent to the following:
>  
> @@ -360,7 +361,8 @@ This accepts an array of interface type names.
>  
> OBJECT_DEFINE_TYPE_WITH_INTERFACES(MyDevice, my_device,
>MY_DEVICE, DEVICE,
> -  { TYPE_USER_CREATABLE }, { NULL })
> +  { TYPE_USER_CREATABLE },
> +  { NULL })
>  
>  If the type is not intended to be instantiated, then then
>  the OBJECT_DEFINE_ABSTRACT_TYPE() macro can be used instead:
> @@ -368,7 +370,8 @@ the OBJECT_DEFINE_ABSTRACT_TYPE() macro can be used 
> instead:
>  .. code-block:: c
> :caption: Defining a simple abstract type
>  
> -   OBJECT_DEFINE_ABSTRACT_TYPE(MyDevice, my_device, MY_DEVICE, DEVICE)
> +   OBJECT_DEFINE_ABSTRACT_TYPE(MyDevice, my_device,
> +   MY_DEVICE, DEVICE)
>  
>  
>  
>

Re: [PATCH] tests: tcg: do not use implicit rules

On 10/3/20 10:50 AM, Paolo Bonzini wrote:
> Use pattern rules to clarify which targets are going to match the
> rule and to provide clearer error messages.
> 
> Signed-off-by: Paolo Bonzini 

Reviewed-by: Philippe Mathieu-Daudé 

> ---
>  tests/Makefile.include | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/Makefile.include b/tests/Makefile.include
> index 40d909badc..5aca98e60c 100644
> --- a/tests/Makefile.include
> +++ b/tests/Makefile.include
> @@ -50,21 +50,21 @@ RUN_TCG_TARGET_RULES=$(patsubst %,run-tcg-tests-%, 
> $(TARGET_DIRS))
>  $(foreach PROBE_TARGET,$(TARGET_DIRS),   \
>   $(eval -include $(SRC_PATH)/tests/tcg/Makefile.prereqs))
>  
> -build-tcg-tests-%: $(if $(CONFIG_PLUGIN),test-plugins)
> +$(BUILD_TCG_TARGET_RULES): build-tcg-tests-%: $(if 
> $(CONFIG_PLUGIN),test-plugins)
>   $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) \
>   -f $(SRC_PATH)/tests/tcg/Makefile.qemu \
>   SRC_PATH=$(SRC_PATH) \
>   V="$(V)" TARGET="$*" guest-tests, \
>   "BUILD", "TCG tests for $*")
>  
> -run-tcg-tests-%: build-tcg-tests-% all
> +$(RUN_TCG_TARGET_RULES): run-tcg-tests-%: build-tcg-tests-% all
>   $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) \
>   -f $(SRC_PATH)/tests/tcg/Makefile.qemu \
>   SRC_PATH=$(SRC_PATH) SPEED="$(SPEED)" \
>   V="$(V)" TARGET="$*" run-guest-tests, \
>   "RUN", "TCG tests for $*")
>  
> -clean-tcg-tests-%:
> +$(CLEAN_TCG_TARGET_RULES): clean-tcg-tests-%:
>   $(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) \
>   -f $(SRC_PATH)/tests/tcg/Makefile.qemu \
>   SRC_PATH=$(SRC_PATH) TARGET="$*" clean-guest-tests, \
>

Re: [PATCH 1/6] qom: Fix DECLARE_*CHECKER documentation

On 10/3/20 4:54 AM, Eduardo Habkost wrote:
> Correct copy/paste mistake in the DECLARE_INSTANCE_CHECKER and
> DECLARE_CLASS_CHECKERS documentation.
> 
> Signed-off-by: Eduardo Habkost 
> ---
>  include/qom/object.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/qom/object.h b/include/qom/object.h
> index 27aaa67e63f..e738dfc6744 100644
> --- a/include/qom/object.h
> +++ b/include/qom/object.h
> @@ -170,7 +170,7 @@ struct Object
>   * Direct usage of this macro should be avoided, and the complete
>   * OBJECT_DECLARE_TYPE macro is recommended instead.
>   *
> - * This macro will provide the three standard type cast functions for a
> + * This macro will provide the the instance type cast functions for a

Using a single "the":
Reviewed-by: Philippe Mathieu-Daudé 

>   * QOM type.
>   */
>  #define DECLARE_INSTANCE_CHECKER(InstanceType, OBJ_NAME, TYPENAME) \
> @@ -187,7 +187,7 @@ struct Object
>   * Direct usage of this macro should be avoided, and the complete
>   * OBJECT_DECLARE_TYPE macro is recommended instead.
>   *
> - * This macro will provide the three standard type cast functions for a
> + * This macro will provide the class type cast functions for a
>   * QOM type.
>   */
>  #define DECLARE_CLASS_CHECKERS(ClassType, OBJ_NAME, TYPENAME) \
>

Re: [PATCH] dockerfiles: add diffutils to Fedora

On 10/3/20 10:50 AM, Paolo Bonzini wrote:
> For some reason diffutils is not included in the Fedora containers anymore,
> causing the build to fail.
> 
> Signed-off-by: Paolo Bonzini 
> ---
>  tests/docker/dockerfiles/fedora.docker | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/tests/docker/dockerfiles/fedora.docker 
> b/tests/docker/dockerfiles/fedora.docker
> index 71e4b56977..ec783418c8 100644
> --- a/tests/docker/dockerfiles/fedora.docker
> +++ b/tests/docker/dockerfiles/fedora.docker
> @@ -11,6 +11,7 @@ ENV PACKAGES \
>  cyrus-sasl-devel \
>  dbus-daemon \
>  device-mapper-multipath-devel \
> +diffutils \
>  findutils \
>  gcc \
>  gcc-c++ \
> 

What about tests/docker/dockerfiles/fedora-cris-cross.docker
and tests/docker/dockerfiles/fedora-i386-cross.docker?

Re: [PATCH v2] hw/avr: Add limited support for avr gpio registers

On 10/3/20 2:38 PM, Heecheol Yang wrote:
> Add some of these features for avr gpio:
> 
>   - GPIO I/O : PORTx registers
>   - Data Direction : DDRx registers
> 
> Following things are not supported yet:
>   - PINx registers
>   - MCUR registers
>   - Even though read/write for DDRx registers are
> implemented, actual direction controls are not
> supported yet.
> 
> Signed-off-by: Heecheol Yang 
> ---
>  hw/avr/Kconfig |   1 +
>  hw/avr/atmega.c|   7 ++-
>  hw/avr/atmega.h|   2 +
>  hw/gpio/Kconfig|   3 +
>  hw/gpio/avr_gpio.c | 112 +
>  hw/gpio/meson.build|   2 +
>  include/hw/gpio/avr_gpio.h |  46 +++
>  7 files changed, 171 insertions(+), 2 deletions(-)
>  create mode 100644 hw/gpio/avr_gpio.c
>  create mode 100644 include/hw/gpio/avr_gpio.h

FYI this one is posted correctly, I can read it and patchew
successfully applied it:
https://patchew.org/QEMU/dm6pr16mb2473c5a77e009ca2fef3d8ece6...@dm6pr16mb2473.namprd16.prod.outlook.com/

But v3 is broken again...

Re: [PATCH v10 6/8] linux-user/elfload: Move PT_INTERP detection to first loop

On 10/3/20 7:38 PM, Philippe Mathieu-Daudé wrote:
> On 10/2/20 11:59 PM, Richard Henderson wrote:
>> For BTI, we need to know if the executable is static or dynamic,
>> which means looking for PT_INTERP earlier.
>>
>> Signed-off-by: Richard Henderson 
>> ---
>>  linux-user/elfload.c | 60 +++-
>>  1 file changed, 31 insertions(+), 29 deletions(-)
>>
>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
>> index 735ebfa190..6b422990ff 100644
>> --- a/linux-user/elfload.c
>> +++ b/linux-user/elfload.c
>> @@ -2421,8 +2421,10 @@ static void load_elf_image(const char *image_name, 
>> int image_fd,
>>  
>>  mmap_lock();
>>  
>> -/* Find the maximum size of the image and allocate an appropriate
>> -   amount of memory to handle that.  */
>> +/*
>> + * Find the maximum size of the image and allocate an appropriate
>> + * amount of memory to handle that.  Locate the interpreter, if any.
>> + */
>>  loaddr = -1, hiaddr = 0;
>>  info->alignment = 0;
>>  for (i = 0; i < ehdr->e_phnum; ++i) {
>> @@ -2438,6 +2440,33 @@ static void load_elf_image(const char *image_name, 
>> int image_fd,
>>  }
>>  ++info->nsegs;
>>  info->alignment |= eppnt->p_align;
>> +} else if (eppnt->p_type == PT_INTERP && pinterp_name) {
>> +char *interp_name;
>> +
>> +if (*pinterp_name) {
>> +errmsg = "Multiple PT_INTERP entries";
>> +goto exit_errmsg;
>> +}
>> +interp_name = malloc(eppnt->p_filesz);
>> +if (!interp_name) {
>> +goto exit_perror;
>> +}
>> +
>> +if (eppnt->p_offset + eppnt->p_filesz <= BPRM_BUF_SIZE) {
>> +memcpy(interp_name, bprm_buf + eppnt->p_offset,
>> +   eppnt->p_filesz);
>> +} else {
>> +retval = pread(image_fd, interp_name, eppnt->p_filesz,
>> +   eppnt->p_offset);
>> +if (retval != eppnt->p_filesz) {
> 
> Preexisting, free(interp_name)?

I just sent a patch using g_steal_pointer() instead:
https://lists.gnu.org/archive/html/qemu-devel/2020-10/msg00792.html
(Maybe I should have tagged it RFC as this is the first
time I try this API).

> 
>> +goto exit_perror;
>> +}
>> +}
>> +if (interp_name[eppnt->p_filesz - 1] != 0) {
>> +errmsg = "Invalid PT_INTERP entry";
> 
> Ditto, otherwise:
> Reviewed-by: Philippe Mathieu-Daudé 
> 
>> +goto exit_errmsg;
>> +}
>> +*pinterp_name = interp_name;
>>  }
>>  }
>>  
>> @@ -2590,33 +2619,6 @@ static void load_elf_image(const char *image_name, 
>> int image_fd,
>>  if (vaddr_em > info->brk) {
>>  info->brk = vaddr_em;
>>  }
>> -} else if (eppnt->p_type == PT_INTERP && pinterp_name) {
>> -char *interp_name;
>> -
>> -if (*pinterp_name) {
>> -errmsg = "Multiple PT_INTERP entries";
>> -goto exit_errmsg;
>> -}
>> -interp_name = malloc(eppnt->p_filesz);
>> -if (!interp_name) {
>> -goto exit_perror;
>> -}
>> -
>> -if (eppnt->p_offset + eppnt->p_filesz <= BPRM_BUF_SIZE) {
>> -memcpy(interp_name, bprm_buf + eppnt->p_offset,
>> -   eppnt->p_filesz);
>> -} else {
>> -retval = pread(image_fd, interp_name, eppnt->p_filesz,
>> -   eppnt->p_offset);
>> -if (retval != eppnt->p_filesz) {
>> -goto exit_perror;
>> -}
>> -}
>> -if (interp_name[eppnt->p_filesz - 1] != 0) {
>> -errmsg = "Invalid PT_INTERP entry";
>> -goto exit_errmsg;
>> -}
>> -*pinterp_name = interp_name;
>>  #ifdef TARGET_MIPS
>>  } else if (eppnt->p_type == PT_MIPS_ABIFLAGS) {
>>  Mips_elf_abiflags_v0 abiflags;
>>
> 
>

Re: [PATCH 04/14] hw/arm/raspi: add a skeleton implementation of the cprman

On 10/3/20 1:54 PM, Luc Michel wrote:
> On 16:37 Fri 02 Oct , Philippe Mathieu-Daudé wrote:
> 
> [snip]
> 
> +struct BCM2835CprmanState {
> +/*< private >*/
> +SysBusDevice parent_obj;
> +
> +/*< public >*/
> +MemoryRegion iomem;
> +
> +uint32_t regs[CPRMAN_NUM_REGS];
> +uint32_t xosc_freq;
> +
> +Clock *xosc;
>>
>> Isn't it xosc external to the CPRMAN?
>>
> Yes on real hardware I'm pretty sure it's the oscillator we can see on
> the board itself, near the SoC (on the bottom side). This is how I first
> planned to implement it. I then realized that would add complexity to
> the BCM2835Peripherals model for no good reasons IMHO (mainly because of
> migration). So at the end I put it inside the CPRMAN for simplicity, and
> added a property to set its frequency.

OK as long as all boards have a 19.2MHz crystal, but if the property
is not easily accessible why not use a #define in
"hw/arm/raspi_platform.h" instead?

Else we should alias the property using object_property_add_alias()
in TYPE_BCM2835_PERIPHERALS.

> 
> +};
> 
> [snip]
> 
> +static const MemoryRegionOps cprman_ops = {
> +.read = cprman_read,
> +.write = cprman_write,
> +.endianness = DEVICE_LITTLE_ENDIAN,
> +.valid  = {
> +.min_access_size= 4,
> +.max_access_size= 4,

 I couldn't find this in the public datasheets (any pointer?).

 Since your implementation is 32bit, can you explicit .impl
 min/max = 4?
>>>
>>> I could not find this information either, but I assumed this is the
>>> case, mainly because of the 'PASSWORD' field in all registers.
>>
>> Good point. Do you mind adding a comment about it here please?
>>
> 
> OK
> 
>>>
>>> Regarding .impl, I thought that having .valid was enough?
>>
>> Until we eventually figure out we can do 64-bit accesses,
>> so someone change .valid.max to 8 and your model is broken :/
> 
> OK, I'll add the .impl constraints.
> 
> [snip]
>

Re: [PATCH v2] scripts/qmp/qom-set: Allow setting integer value

2020-10-03 Thread John Snow


On 10/3/20 1:33 PM, Philippe Mathieu-Daudé wrote:

Hi Jonatan,

On 10/2/20 10:52 PM, Jonatan Pålsson wrote:

If the value appears to be an integer, parse it as such.

This allows the following:

 qmp/qom-set -s ~/qmp.sock sensor.temperature 2


Maybe instead:

Fix the following error:

   $ scripts/qmp/qom-set -s ~/qmp.sock sensor.temperature 2
   Traceback (most recent call last):
 File "scripts/qmp/qom-set", line 66, in 
   print(srv.command('qom-set', path=path, property=prop, value=value))
 File "scripts/qmp/../../python/qemu/qmp.py", line 274, in command
   raise QMPResponseError(ret)
   qemu.qmp.QMPResponseError: Invalid parameter type for 'temperature',
expected: integer



No, this is just relaying the error that QMP returned. QMP is telling 
you it doesn't want string data for this parameter. His diagnosis of the 
problem is accurate.




.. where sensor is a tmp105 device, and temperature is an integer
property.

Signed-off-by: Jonatan Pålsson 
---
  scripts/qmp/qom-set | 5 -
  1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/scripts/qmp/qom-set b/scripts/qmp/qom-set
index 240a78187f..49eebe4924 100755
--- a/scripts/qmp/qom-set
+++ b/scripts/qmp/qom-set
@@ -56,7 +56,10 @@ if len(args) > 1:
  path, prop = args[0].rsplit('.', 1)
  except:
  usage_error("invalid format for path/property/value")
-value = args[1]
+try:
+value = int(args[1])


Maybe 'long' is safer?



This is a Python patch, what's a "long"?


+except ValueError:
+value = args[1]
  else:
  usage_error("not enough arguments")

QEMU API cleanup initiative - Let's chat during the KVM call

2020-10-03 Thread John Snow

Hi, everyone! I'd like to discuss some of this in the upstream KVM call.

TLDR: I would like to begin an organized effort to consolidate our CLI
parsing, moving it onto QAPI. I'd like to talk about how we should
proceed on the KVM call, prior to KVM Forum, where we should continue
these discussions.

Background
--

You may recall late last December that Stefan started a big discussion
thread about Modernizing the QEMU API:

"Making QEMU easier for management tools and applications" [1]

There were lots of opinions and directions proposed for this, with many
competing visions for where QEMU should go, or what it ought endeavor to be.

Though many of these visions conflict in terms of the implementations
for their end goal, many shared a similar logical end-goal and shared
some concrete intermediate steps. One of those concrete intermediate
steps is the consolidation of our configuration and startup mechanisms
into a unified API.

The QEMU API, Today
--

At the moment, QAPI is our most formal system for declaring types,
structures and interfaces. I believe QAPI is not going anywhere, and so
I am doubling down and committing to improving and expanding the QAPI
subsystem.

I wanted to understand what QEMU's existing interface even *was*. We can
understand QEMU's interface to be four components presently:

1. The QEMU Monitor Protocol (QMP)

QMP is based explicitly on top of QAPI, which we do indeed have formal
specifications for in ./qapi. They are not standards-compliant, but they
are at least unified.

2. The GTK UI

The GTK UI offers very minimal interface, and does not offer any feature
that is not available through one of the other interfaces or standard
operating system UI. Good!

3. The Human Monitor Protocol (HMP)

HMP is increasingly based on QMP, though the conversion is not complete
and it is not clear if it will be "complete". This was a major sub-topic
of the thread last December with no clear consensus. Some work continues
to bring major HMP features over to QMP; notably Daniel Berrange has
been trying to port savevm/loadvm over to QMP [2]. For now, it seems
like HMP will be with us at least as a debugging and development
interface. There is work to be done to audit and inventory any remaining
features that must be ported to QMP, which are reundant with QMP, and
which are uniquely useful as debugging interfaces.

4. The QEMU command-line

Last, we have the QEMU CLI. This interface was grown organically over
time and features many different parser subsystems and loosely federated
components with no unified specification document that explains the
entire shape of the CLI.

Auditing the CLI interface
--

I wanted to know what our CLI really looked like. Not trusting any of
our existing documentation to be complete/authoritative, I used the QEMU
5.1 release as a basis and audited the entirety of that interface. [3]

Here's what I found:

- QEMU 5.1 has 131 command line flags
- 93 of these take an argument
- 38 of them do not.

If we want to unify the parsing into a single formal declaration, it
would be helpful to know what we're dealing with. Of those flags that
take arguments:

- 3 use QAPI to parse their argument directly
- 51 use QemuOpts in some way:
- 36 use qemu_opts_parse[_noisily] directly
- 10 desugar to `qemu_opts_parse_noisily` (-fdc, -hda, et al)
- 5 add a single option using `qemu_opt_set`.
- 1 is parsed rather directly by QOM (-tb-size)
- 14 are stored directly as (const char *)
- 3 are parsed into numerical types with atoi/strtol/etc.
- 21 are parsed by custom parsing mechanisms.

For full, gory details, please see the document referenced at [3]. It's
about 4000 lines of markdown detailing the QAPI/C structures that define
each command line argument, as well as some fairly detailed analyses of
the custom parsers and exactly which values they accept.

I'm not reading a 4,000+ line markdown document
---

Good news! I made a summary spreadsheet to summarize what I found. [4]

This spreadsheet summarizes the types of arguments we have and what
parsers they utilize and their support status. The spreadsheet follows
the order of flags as defined in qemu-options.hx, category-by-category.

I also tried to broadly assign "topics" to each flag for the purposes of
laying out a better manual in the future, but I wasn't fully confident
in many flags that affect things like boot, firmware, chipset, etc, so
this is a work in progress.

https://docs.google.com/spreadsheets/d/1OJvzDwLpo2XsGytNp9Pr-vYIrb0pWrFKBNPtOyH80ew/edit?usp=sharing

If you don't have google, I have an ODS exported version of this
spreadsheet too -- feel free to relay your feedback back to me here. [5]

Paolo Bonzini helped re-organize my initial draft and used it to
identify flags perhaps most in need of attention to bring onto a new
standard, annotated in yellow.

Re: [PATCH v7 00/14] Reverse debugging

Patchew URL: 
https://patchew.org/QEMU/160174516520.12451.10785284392438702137.stgit@pasha-ThinkPad-X280/



Hi,

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

Type: series
Message-id: 160174516520.12451.10785284392438702137.stgit@pasha-ThinkPad-X280
Subject: [PATCH v7 00/14] Reverse debugging

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
ba53a9d tests/acceptance: add reverse debugging test
c6aa9c5 replay: create temporary snapshot at debugger connection
17d5c46 replay: describe reverse debugging in docs/replay.txt
1f88bff gdbstub: add reverse continue support in replay mode
72ef5d6 gdbstub: add reverse step support in replay mode
42bf7cc replay: flush rr queue before loading the vmstate
4285666 replay: implement replay-seek command
653aa62 replay: introduce breakpoint at the specified step
59ab65a replay: introduce info hmp/qmp command
1bc0b45 qapi: introduce replay.json for record/replay-related stuff
c4b17f7 migration: introduce icount field for snapshots
03d28c5 qcow2: introduce icount field for snapshots
6de69ce replay: provide an accessor for rr filename
8ba3d42 replay: don't record interrupt poll

=== OUTPUT BEGIN ===
1/14 Checking commit 8ba3d42631d9 (replay: don't record interrupt poll)
2/14 Checking commit 6de69cee86b9 (replay: provide an accessor for rr filename)
3/14 Checking commit 03d28c50b445 (qcow2: introduce icount field for snapshots)
4/14 Checking commit c4b17f7373f0 (migration: introduce icount field for 
snapshots)
ERROR: trailing whitespace
#251: FILE: tests/qemu-iotests/267.out:37:
+--snap0SIZE -mm-dd hh:mm:ss 00:00:00.000   
$

ERROR: trailing whitespace
#262: FILE: tests/qemu-iotests/267.out:48:
+--snap0SIZE -mm-dd hh:mm:ss 00:00:00.000   
$

ERROR: trailing whitespace
#273: FILE: tests/qemu-iotests/267.out:73:
+--snap0SIZE -mm-dd hh:mm:ss 00:00:00.000   
$

ERROR: trailing whitespace
#284: FILE: tests/qemu-iotests/267.out:98:
+--snap0SIZE -mm-dd hh:mm:ss 00:00:00.000   
$

ERROR: trailing whitespace
#295: FILE: tests/qemu-iotests/267.out:109:
+--snap0SIZE -mm-dd hh:mm:ss 00:00:00.000   
$

ERROR: trailing whitespace
#306: FILE: tests/qemu-iotests/267.out:123:
+--snap0SIZE -mm-dd hh:mm:ss 00:00:00.000   
$

ERROR: trailing whitespace
#317: FILE: tests/qemu-iotests/267.out:138:
+--snap0SIZE -mm-dd hh:mm:ss 00:00:00.000   
$

ERROR: trailing whitespace
#328: FILE: tests/qemu-iotests/267.out:149:
+--snap0SIZE -mm-dd hh:mm:ss 00:00:00.000   
$

ERROR: trailing whitespace
#337: FILE: tests/qemu-iotests/267.out:156:
+1 snap0SIZE -mm-dd hh:mm:ss 00:00:00.000   
$

ERROR: trailing whitespace
#348: FILE: tests/qemu-iotests/267.out:170:
+--snap0SIZE -mm-dd hh:mm:ss 00:00:00.000   
$

ERROR: trailing whitespace
#357: FILE: tests/qemu-iotests/267.out:177:
+1 snap0SIZE -mm-dd hh:mm:ss 00:00:00.000   
$

ERROR: trailing whitespace
#363: FILE: tests/qemu-iotests/267.out:181:
+1 snap0SIZE -mm-dd hh:mm:ss 00:00:00.000   
$

total: 12 errors, 0 warnings, 275 lines checked

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

5/14 Checking commit 1bc0b45203ea (qapi: introduce replay.json for 
record/replay-related stuff)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#93: 
new file mode 100644

total: 0 errors, 1 warnings, 78 lines checked

Patch 5/14 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/14 Checking commit 59ab65a00e3b (replay: introduce info hmp/qmp command)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#122: 
new file mode 100644

total: 0 errors, 1 warnings, 120 lines checked

Patch 6/14 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
7/14 Checking commit 653aa622c001 (replay: introduce breakpoint at the 
specified step)
8/14 Checking commit 4285666198ee (replay: implement replay-seek command)
9/14 Checking commit 42bf7cc3ae4e (replay: flush rr queue before loading the 
vmstate)
10/14 Checking commit 72ef5d64fb17 (gdbstub: add reverse step support in replay 
mode)
11/14 Checking comm

Re: [PATCH v10 0/8] linux-user: User support for AArch64 BTI

Patchew URL: 
https://patchew.org/QEMU/20201002215955.254866-1-richard.hender...@linaro.org/



Hi,

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

Type: series
Message-id: 20201002215955.254866-1-richard.hender...@linaro.org
Subject: [PATCH v10 0/8] linux-user: User support for AArch64 BTI

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
eaf5edb tests/tcg/aarch64: Add bti smoke test
c74e202 linux-user/elfload: Parse NT_GNU_PROPERTY_TYPE_0 notes
7e7c534 linux-user/elfload: Move PT_INTERP detection to first loop
1ded2cd linux-user/elfload: Adjust iteration over phdr
9a313b3 linux-user/elfload: Fix coding style in load_elf_image
ddc27b7 include/elf: Add defines related to GNU property notes for AArch64
eca4240 linux-user: Set PAGE_TARGET_1 for TARGET_PROT_BTI
6b3e8e3 linux-user/aarch64: Reset btype for signals

=== OUTPUT BEGIN ===
1/8 Checking commit 6b3e8e369613 (linux-user/aarch64: Reset btype for signals)
2/8 Checking commit eca424067459 (linux-user: Set PAGE_TARGET_1 for 
TARGET_PROT_BTI)
3/8 Checking commit ddc27b75549d (include/elf: Add defines related to GNU 
property notes for AArch64)
4/8 Checking commit 9a313b30265c (linux-user/elfload: Fix coding style in 
load_elf_image)
5/8 Checking commit 1ded2cdcd8ed (linux-user/elfload: Adjust iteration over 
phdr)
6/8 Checking commit 7e7c5343dde5 (linux-user/elfload: Move PT_INTERP detection 
to first loop)
7/8 Checking commit c74e202361a9 (linux-user/elfload: Parse 
NT_GNU_PROPERTY_TYPE_0 notes)
8/8 Checking commit eaf5edb50de6 (tests/tcg/aarch64: Add bti smoke test)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#32: 
new file mode 100644

ERROR: externs should be avoided in .c files
#117: FILE: tests/tcg/aarch64/bti-crt.inc.c:13:
+int main(void);

total: 1 errors, 1 warnings, 136 lines checked

Patch 8/8 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


The full log is available at
http://patchew.org/logs/20201002215955.254866-1-richard.hender...@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [PATCH] tests: tcg: do not use implicit rules

Patchew URL: 
https://patchew.org/QEMU/20201003085054.332992-1-pbonz...@redhat.com/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing 
commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===




The full log is available at
http://patchew.org/logs/20201003085054.332992-1-pbonz...@redhat.com/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-de...@redhat.com

Re: [PATCH] dockerfiles: add diffutils to Fedora